Re: [PATCH v4 4/6] nfc: s3fwrn5: enable the PVDD supply

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The NCI device is registered and exposed to userspace before its IRQ handler is requested, creating a race condition.
--

--- Patch [4]: [PATCH v4 4/6] nfc: s3fwrn5: enable the PVDD supply ---
commit 3e46e47e27236d4ca640efa94f0c2940f401ccd7
Author: Jorijn van der Graaf <[email protected]>

nfc: s3fwrn5: enable the PVDD supply

This commit retrieves and enables the PVDD supply in the I2C PHY probe
function. This ensures the controller is powered before any of its pins
are driven or its clock is started.

> diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
> index c528c085b43ef..f93d8b47b65e3 100644
> --- a/drivers/nfc/s3fwrn5/i2c.c
> +++ b/drivers/nfc/s3fwrn5/i2c.c

[ ... ]

> @@ -158,6 +159,11 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client)
>  	phy->i2c_dev = client;
>  	i2c_set_clientdata(client, phy);
>  
> +	ret = devm_regulator_get_enable(&client->dev, "pvdd");
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret,
> +				     "failed to enable pvdd\n");
> +

[Severity: High]
This isn't a bug introduced by this patch, but does this function expose
the device to userspace before the IRQ handler is set up?

Looking further down in s3fwrn5_i2c_probe():

	ret = s3fwrn5_probe(&phy->common.ndev, phy, &phy->i2c_dev->dev,
			    &i2c_phy_ops, variant);
	if (ret < 0)
		return ret;

	ret = devm_request_threaded_irq(&client->dev, phy->i2c_dev->irq, NULL,
		s3fwrn5_i2c_irq_thread_fn, IRQF_ONESHOT,
		"s3fwrn5_i2c", phy);

If userspace (like udev or neard) immediately opens the NCI device and
starts sending commands after s3fwrn5_probe() returns, can the hardware
generate an interrupt that is missed because devm_request_threaded_irq()
has not been called yet?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4