2020-09-14 15:49:16

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v3 01/14] rtc: rx8010: don't modify the global rtc ops

From: Bartosz Golaszewski <[email protected]>

The way the driver is implemented is buggy for the (admittedly unlikely)
use case where there are two RTCs with one having an interrupt configured
and the second not. This is caused by the fact that we use a global
rtc_class_ops struct which we modify depending on whether the irq number
is present or not.

Fix it by using two const ops structs with and without alarm operations.
While at it: not being able to request a configured interrupt is an error
so don't ignore it and bail out of probe().

Fixes: ed13d89b08e3 ("rtc: Add Epson RX8010SJ RTC driver")
Cc: [email protected]
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/rtc/rtc-rx8010.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index fe010151ec8f..08c93d492494 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -407,16 +407,26 @@ static int rx8010_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
}
}

-static struct rtc_class_ops rx8010_rtc_ops = {
+static const struct rtc_class_ops rx8010_rtc_ops_default = {
.read_time = rx8010_get_time,
.set_time = rx8010_set_time,
.ioctl = rx8010_ioctl,
};

+static const struct rtc_class_ops rx8010_rtc_ops_alarm = {
+ .read_time = rx8010_get_time,
+ .set_time = rx8010_set_time,
+ .ioctl = rx8010_ioctl,
+ .read_alarm = rx8010_read_alarm,
+ .set_alarm = rx8010_set_alarm,
+ .alarm_irq_enable = rx8010_alarm_irq_enable,
+};
+
static int rx8010_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct i2c_adapter *adapter = client->adapter;
+ const struct rtc_class_ops *rtc_ops;
struct rx8010_data *rx8010;
int err = 0;

@@ -447,16 +457,16 @@ static int rx8010_probe(struct i2c_client *client,

if (err) {
dev_err(&client->dev, "unable to request IRQ\n");
- client->irq = 0;
- } else {
- rx8010_rtc_ops.read_alarm = rx8010_read_alarm;
- rx8010_rtc_ops.set_alarm = rx8010_set_alarm;
- rx8010_rtc_ops.alarm_irq_enable = rx8010_alarm_irq_enable;
+ return err;
}
+
+ rtc_ops = &rx8010_rtc_ops_alarm;
+ } else {
+ rtc_ops = &rx8010_rtc_ops_default;
}

rx8010->rtc = devm_rtc_device_register(&client->dev, client->name,
- &rx8010_rtc_ops, THIS_MODULE);
+ rtc_ops, THIS_MODULE);

if (IS_ERR(rx8010->rtc)) {
dev_err(&client->dev, "unable to register the class device\n");
--
2.26.1


2020-09-17 18:57:43

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH v3 01/14] rtc: rx8010: don't modify the global rtc ops

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: ed13d89b08e3 ("rtc: Add Epson RX8010SJ RTC driver").

The bot has tested the following trees: v5.8.9, v5.4.65, v4.19.145, v4.14.198, v4.9.236.

v5.8.9: Build OK!
v5.4.65: Build OK!
v4.19.145: Failed to apply! Possible dependencies:
9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")

v4.14.198: Failed to apply! Possible dependencies:
9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")

v4.9.236: Failed to apply! Possible dependencies:
9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

--
Thanks
Sasha

2020-09-17 19:56:12

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v3 01/14] rtc: rx8010: don't modify the global rtc ops

On Thu, Sep 17, 2020 at 5:53 PM Sasha Levin <[email protected]> wrote:
>
> Hi
>
> [This is an automated email]
>
> This commit has been processed because it contains a "Fixes:" tag
> fixing commit: ed13d89b08e3 ("rtc: Add Epson RX8010SJ RTC driver").
>
> The bot has tested the following trees: v5.8.9, v5.4.65, v4.19.145, v4.14.198, v4.9.236.
>
> v5.8.9: Build OK!
> v5.4.65: Build OK!
> v4.19.145: Failed to apply! Possible dependencies:
> 9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")
>
> v4.14.198: Failed to apply! Possible dependencies:
> 9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")
>
> v4.9.236: Failed to apply! Possible dependencies:
> 9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?
>
> --
> Thanks
> Sasha

I sent out a backport for v4.X branches.

Bartosz