diff mbox series

[4.19,178/191] rtc: rx8010: dont modify the global rtc ops

Message ID 20201103203249.312789260@linuxfoundation.org
State Superseded
Headers show
Series None | expand

Commit Message

Greg Kroah-Hartman Nov. 3, 2020, 8:37 p.m. UTC 
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

commit d3b14296da69adb7825022f3224ac6137eb30abf upstream.

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")
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20200914154601.32245-2-brgl@bgdev.pl
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


---
 drivers/rtc/rtc-rx8010.c |   24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

Comments

Pavel Machek Nov. 5, 2020, 7:25 p.m. UTC | #1 
Hi!

> commit d3b14296da69adb7825022f3224ac6137eb30abf upstream.

> 

> 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.


While this fixes very unlikely configuration with two RTCs...

> 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().


...it contains unrelated changes and in particular will break
operation when IRQ can not be requested.

I don't believe we need it in -stable.

Best regards,
								Pavel

> @@ -468,16 +478,16 @@ static int rx8010_probe(struct i2c_clien

>  

>  		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;

>  		}


-- 
http://www.livejournal.com/~pavelmachek
diff mbox series

Patch

--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -428,16 +428,26 @@  static int rx8010_ioctl(struct device *d
 	}
 }
 
-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 = to_i2c_adapter(client->dev.parent);
+	const struct rtc_class_ops *rtc_ops;
 	struct rx8010_data *rx8010;
 	int err = 0;
 
@@ -468,16 +478,16 @@  static int rx8010_probe(struct i2c_clien
 
 		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");