| Message ID | 20260811194034.28937-6-apokusinski01@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | rtc: abx80x: add support for abx81x | expand |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Failure to clear client->irq on IRQ request error leaves a dangling positive IRQ value that is erroneously disabled during system shutdown.
--
--- Patch [5]: [PATCH v5 5/9] rtc: abx80x: add irq to struct abx80x_priv ---
commit e5e02251b0d732530fe4bd8df1edc078a44d0fe3
Author: Antoni Pokusinski <apokusinski01@gmail.com>
rtc: abx80x: add irq to struct abx80x_priv
This commit adds the irq number to struct abx80x_priv so it is not tied
to the i2c_client. This allows the irq to be easily accessed in the case
of ABX81X SPI clocks.
> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> index 485d1ea08030d..12a4dbfec6396 100644
> --- a/drivers/rtc/rtc-abx80x.c
> +++ b/drivers/rtc/rtc-abx80x.c
[ ... ]
> @@ -994,19 +997,19 @@ static int abx80x_probe(struct i2c_client *client)
> return -EIO;
> }
>
> - if (client->irq > 0) {
> - dev_info(&client->dev, "IRQ %d supplied\n", client->irq);
> - err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> + if (priv->irq > 0) {
> + dev_info(&client->dev, "IRQ %d supplied\n", priv->irq);
> + err = devm_request_threaded_irq(&client->dev, priv->irq, NULL,
> abx80x_handle_irq,
> IRQF_SHARED | IRQF_ONESHOT,
> "abx8xx",
> client);
> if (err) {
> dev_err(&client->dev, "unable to request IRQ, alarms disabled\n");
> - client->irq = 0;
> + priv->irq = 0;
[Severity: Medium]
By removing the client->irq = 0 assignment here, doesn't this leave a
dangling positive IRQ value in the I2C client if the request fails?
If devm_request_threaded_irq() fails, the driver gracefully falls back to
running with alarms disabled. However, during system shutdown, the I2C core
in i2c_device_shutdown() relies on client->irq to disable interrupts:
drivers/i2c/i2c-core-base.c:i2c_device_shutdown() {
...
if (client->irq > 0)
disable_irq(client->irq);
...
}
Since this driver uses IRQF_SHARED and has no .shutdown callback to
override the default behavior, leaving client->irq > 0 means the I2C core
will unconditionally call disable_irq() on an IRQ line the driver failed
to acquire.
Could this erroneously mask the shared IRQ line for other active devices
during system shutdown?
> }
> }
diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c index 485d1ea08030..12a4dbfec639 100644 --- a/drivers/rtc/rtc-abx80x.c +++ b/drivers/rtc/rtc-abx80x.c @@ -135,6 +135,7 @@ struct abx80x_priv { struct i2c_client *client; struct watchdog_device wdog; struct mutex lock; + int irq; }; static int abx80x_write_config_key(struct i2c_client *client, u8 key) @@ -305,11 +306,12 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id) static int abx80x_read_alarm(struct device *dev, struct rtc_wkalrm *t) { struct i2c_client *client = to_i2c_client(dev); + struct abx80x_priv *priv = i2c_get_clientdata(client); unsigned char buf[7]; int irq_mask, err; - if (client->irq <= 0) + if (priv->irq <= 0) return -EINVAL; err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_ASC, @@ -341,7 +343,7 @@ static int abx80x_set_alarm(struct device *dev, struct rtc_wkalrm *t) u8 alarm[6]; int err; - if (client->irq <= 0) + if (priv->irq <= 0) return -EINVAL; alarm[0] = 0x0; @@ -947,6 +949,7 @@ static int abx80x_probe(struct i2c_client *client) priv->rtc->ops = &abx80x_rtc_ops; priv->client = client; + priv->irq = client->irq; err = devm_mutex_init(&client->dev, &priv->lock); if (err) return err; @@ -994,19 +997,19 @@ static int abx80x_probe(struct i2c_client *client) return -EIO; } - if (client->irq > 0) { - dev_info(&client->dev, "IRQ %d supplied\n", client->irq); - err = devm_request_threaded_irq(&client->dev, client->irq, NULL, + if (priv->irq > 0) { + dev_info(&client->dev, "IRQ %d supplied\n", priv->irq); + err = devm_request_threaded_irq(&client->dev, priv->irq, NULL, abx80x_handle_irq, IRQF_SHARED | IRQF_ONESHOT, "abx8xx", client); if (err) { dev_err(&client->dev, "unable to request IRQ, alarms disabled\n"); - client->irq = 0; + priv->irq = 0; } } - if (client->irq <= 0) + if (priv->irq <= 0) clear_bit(RTC_FEATURE_ALARM, priv->rtc->features); err = rtc_add_group(priv->rtc, &rtc_calib_attr_group);
This way the irq number is not tied to the i2c_client and can be easily accessed also in the case of ABX81X SPI clocks. Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com> --- drivers/rtc/rtc-abx80x.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)