Skip to contentSkip to conversation

rtc: rs5c372: let the alarm to be used as wakeup source

PATCH In review Daniel González Cabanelas 2 participants 3 messages Nov 23, 2020 – Mar 8, 2021 Real time clock (RTC)

lore
Share & tools
lore
#0 lore raw

PATCH 2/2 rtc: rs5c372: let the alarm to be used as wakeup source

Currently there is no use for the interrupts on the rs5c372 RTC and the wakealarm isn't enabled. There are some devices like NAS which use this RTC to wake up from the power off state when the INTR pin is activated by the alarm clock.

Enable the alarm and let to be used as a wakeup source. Tested on a Buffalo LS421DE NAS.

---
 drivers/rtc/rtc-rs5c372.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
drivers/rtc/rtc-rs5c372.c +16 -0
index 94b778c6e..76775d66e 100644
@@ -654,6 +654,7 @@ static int rs5c372_probe(struct i2c_client *client,
 	int err = 0;
 	int smbus_mode = 0;
 	struct rs5c372 *rs5c372;
+	bool rs5c372_can_wakeup_device = false;
 
 	dev_dbg(&client->dev, "%s\n", __func__);
 
@@ -689,6 +690,12 @@ static int rs5c372_probe(struct i2c_client *client,
 	else
 		rs5c372->type = id->driver_data;
 
+#ifdef CONFIG_OF
+	if(of_property_read_bool(client->dev.of_node,
+					      "wakeup-source"))
+		rs5c372_can_wakeup_device = true;
+#endif
+
 	/* we read registers 0x0f then 0x00-0x0f; skip the first one */
 	rs5c372->regs = &rs5c372->buf[1];
 	rs5c372->smbus = smbus_mode;
@@ -722,6 +729,8 @@ static int rs5c372_probe(struct i2c_client *client,
 		goto exit;
 	}
 
+	rs5c372->has_irq = 1;
+
 	/* if the oscillator lost power and no other software (like
 	 * the bootloader) set it up, do it here.
 	 *
@@ -748,6 +757,10 @@ static int rs5c372_probe(struct i2c_client *client,
 			);
 
 	/* REVISIT use client->irq to register alarm irq ... */
+	if (rs5c372_can_wakeup_device) {
+		device_init_wakeup(&client->dev, true);
+	}
+
 	rs5c372->rtc = devm_rtc_device_register(&client->dev,
 					rs5c372_driver.driver.name,
 					&rs5c372_rtc_ops, THIS_MODULE);
@@ -761,6 +774,9 @@ static int rs5c372_probe(struct i2c_client *client,
 	if (err)
 		goto exit;
 
+	/* the rs5c372 alarm only supports a minute accuracy */
+	rs5c372->rtc->uie_unsupported = 1;
+
 	return 0;
 
 exit:
Signature · 1 line
2.29.2
#1 lore raw
Quoted 26 lines · Daniel González Cabanelas

Currently there is no use for the interrupts on the rs5c372 RTC and the wakealarm isn't enabled. There are some devices like NAS which use this RTC to wake up from the power off state when the INTR pin is activated by the alarm clock.

Enable the alarm and let to be used as a wakeup source. Tested on a Buffalo LS421DE NAS.

---
 drivers/rtc/rtc-rs5c372.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
drivers/rtc/rtc-rs5c372.c +1 -0
index 94b778c6e..76775d66e 100644
@@ -654,6 +654,7 @@ static int rs5c372_probe(struct i2c_client *client,
 	int err = 0;
 	int smbus_mode = 0;
 	struct rs5c372 *rs5c372;
+	bool rs5c372_can_wakeup_device = false;
 
 	dev_dbg(&client->dev, "%s\n", __func__);
 

drivers/rtc/rtc-rs5c372.c static int rs5c372_probe(struct i2c_client *client, · line 693

Quoted 4 lines
drivers/rtc/rtc-rs5c372.c +1 -0
@@ -689,6 +690,12 @@ static int rs5c372_probe(struct i2c_client *client,
 	else
 		rs5c372->type = id->driver_data;
 
+#ifdef CONFIG_OF

I don't think you need to protect this section

drivers/rtc/rtc-rs5c372.c static int rs5c372_probe(struct i2c_client *client, · line 694

Quoted 1 line
drivers/rtc/rtc-rs5c372.c +1 -0
+	if(of_property_read_bool(client->dev.of_node,

but you'll have to check client->dev.of_node is not null.

drivers/rtc/rtc-rs5c372.c static int rs5c372_probe(struct i2c_client *client, · line 733

Quoted 13 lines
drivers/rtc/rtc-rs5c372.c +6 -0
+					      "wakeup-source"))
+		rs5c372_can_wakeup_device = true;
+#endif
+
 	/* we read registers 0x0f then 0x00-0x0f; skip the first one */
 	rs5c372->regs = &rs5c372->buf[1];
 	rs5c372->smbus = smbus_mode;
@@ -722,6 +729,8 @@ static int rs5c372_probe(struct i2c_client *client,
 		goto exit;
 	}
 
+	rs5c372->has_irq = 1;
+

This is an unrelated change but is this actually useful? I guess you should simply remove has_irq.

Quoted 15 lines
drivers/rtc/rtc-rs5c372.c +4 -0
 	/* if the oscillator lost power and no other software (like
 	 * the bootloader) set it up, do it here.
 	 *
@@ -748,6 +757,10 @@ static int rs5c372_probe(struct i2c_client *client,
 			);
 
 	/* REVISIT use client->irq to register alarm irq ... */
+	if (rs5c372_can_wakeup_device) {
+		device_init_wakeup(&client->dev, true);
+	}
+
 	rs5c372->rtc = devm_rtc_device_register(&client->dev,
 					rs5c372_driver.driver.name,
 					&rs5c372_rtc_ops, THIS_MODULE);

drivers/rtc/rtc-rs5c372.c static int rs5c372_probe(struct i2c_client *client, · line 779

Quoted 6 lines
drivers/rtc/rtc-rs5c372.c +3 -0
@@ -761,6 +774,9 @@ static int rs5c372_probe(struct i2c_client *client,
 	if (err)
 		goto exit;
 
+	/* the rs5c372 alarm only supports a minute accuracy */
+	rs5c372->rtc->uie_unsupported = 1;
+

Honestly, the whole driver would need some modernization so if you have time to test, I can propose a set of patches.

Signature · 3 lines
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
#2 lore raw

Hi Alexandre,

Quoted 73 lines · Alexandre Belloni
Quoted 26 lines · Daniel González Cabanelas

Currently there is no use for the interrupts on the rs5c372 RTC and the wakealarm isn't enabled. There are some devices like NAS which use this RTC to wake up from the power off state when the INTR pin is activated by the alarm clock.

Enable the alarm and let to be used as a wakeup source. Tested on a Buffalo LS421DE NAS.

---
 drivers/rtc/rtc-rs5c372.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
drivers/rtc/rtc-rs5c372.c +1 -0
index 94b778c6e..76775d66e 100644
@@ -654,6 +654,7 @@ static int rs5c372_probe(struct i2c_client *client,
      int err = 0;
      int smbus_mode = 0;
      struct rs5c372 *rs5c372;
+     bool rs5c372_can_wakeup_device = false;
 
      dev_dbg(&client->dev, "%s\n", __func__);
 

drivers/rtc/rtc-rs5c372.c static int rs5c372_probe(struct i2c_client *client, · line 693

Quoted 4 lines
drivers/rtc/rtc-rs5c372.c +1 -0
@@ -689,6 +690,12 @@ static int rs5c372_probe(struct i2c_client *client,
      else
              rs5c372->type = id->driver_data;
 
+#ifdef CONFIG_OF

I don't think you need to protect this section

drivers/rtc/rtc-rs5c372.c static int rs5c372_probe(struct i2c_client *client, · line 694

Quoted 1 line
drivers/rtc/rtc-rs5c372.c +1 -0
+     if(of_property_read_bool(client->dev.of_node,

but you'll have to check client->dev.of_node is not null.

drivers/rtc/rtc-rs5c372.c static int rs5c372_probe(struct i2c_client *client, · line 733

Quoted 13 lines
drivers/rtc/rtc-rs5c372.c +6 -0
+                                           "wakeup-source"))
+             rs5c372_can_wakeup_device = true;
+#endif
+
      /* we read registers 0x0f then 0x00-0x0f; skip the first one */
      rs5c372->regs = &rs5c372->buf[1];
      rs5c372->smbus = smbus_mode;
@@ -722,6 +729,8 @@ static int rs5c372_probe(struct i2c_client *client,
              goto exit;
      }
 
+     rs5c372->has_irq = 1;
+

This is an unrelated change but is this actually useful? I guess you should simply remove has_irq.

Quoted 18 lines
drivers/rtc/rtc-rs5c372.c +4 -0
      /* if the oscillator lost power and no other software (like
       * the bootloader) set it up, do it here.
       *
@@ -748,6 +757,10 @@ static int rs5c372_probe(struct i2c_client *client,
                      );
 
      /* REVISIT use client->irq to register alarm irq ... */
+     if (rs5c372_can_wakeup_device) {
+             device_init_wakeup(&client->dev, true);
+     }
+
      rs5c372->rtc = devm_rtc_device_register(&client->dev,
                                      rs5c372_driver.driver.name,
                                      &rs5c372_rtc_ops, THIS_MODULE);
@@ -761,6 +774,9 @@ static int rs5c372_probe(struct i2c_client *client,
      if (err)
              goto exit;
 
Quoted 7 lines

drivers/rtc/rtc-rs5c372.c static int rs5c372_probe(struct i2c_client *client, · line 779

Quoted 3 lines
drivers/rtc/rtc-rs5c372.c +3 -0
+     /* the rs5c372 alarm only supports a minute accuracy */
+     rs5c372->rtc->uie_unsupported = 1;
+

Honestly, the whole driver would need some modernization so if you have time to test, I can propose a set of patches.

Of course I can test them. I'll reply with ACK once tested.

Regards
Daniel

Quoted 5 lines

--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com