Skip to content

ISL12057 alarm support and isil vs isl fix

Arnaud Ebalard linux-arm-kernel linux-devicetree 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v0 Latest5 patches 2 repliesIn review

Patches in v0 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/5rtc: rtc-isl12057: add alarm support to Intersil ISL12057 RTC driverDocumentation/devicetree/bindings/rtc/isil,isl12057.txt drivers/rtc/rtc-isl12057.c+411 -102 replies
2/5ARM: mvebu: ISL12057 rtc chip can be used to wake up RN102arch/arm/boot/dts/armada-370-netgear-rn102.dts+1 -01 reply
2/5ARM: mvebu: ISL12057 rtc chip can be used to wake up RN102+0 -00 replies
3/5ARM: mvebu: ISL12057 rtc chip can be used to wake up RN104arch/arm/boot/dts/armada-370-netgear-rn104.dts+1 -00 replies
4/5ARM: mvebu: ISL12057 rtc chip can be used to wake up RN2120arch/arm/boot/dts/armada-xp-netgear-rn2120.dts+1 -00 replies
5/5dt-bindings: fix isl vs isil prefix issue for IntersilDocumentation/devicetree/bindings/i2c/trivial-devices.txt Documentation/devicetree/bindings/regulator/isl9305.txt Documentation/devicetree/bindings/vendor-prefixes.txt +6 more+17 -151 reply
5/5dt-bindings: fix isl vs isil prefix issue for Intersil+0 -01 reply

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v0 yet.

Read the v0 thread

Cover letter [PATCHv0 0/5] ISL12057 alarm support and isil vs isl fix

Hi Andrew, Uwe,

Andrew, the first patch of this series is a new version of the one you have in your tree, providing the changes asked by Uwe (i.e. you can just replace yours by that one). If you prefer an additional commit over what you already have, do not hesitate to tell me. Below, you have a diff between previous (the one you have) and this first patch, w/ some inline comments. Uwe, tell me if it is what you expected.

The three patches for RN102, RN104 and RN2120 .dts files just add a boolean property to the ISL12057 node to mark it has a wakeup source, as proposed by Uwe. If first patch is ok, I think those three can either be merged by Jason or directly by you, Andrew.

The last patch tries and fix the isl vs isil issue which led to the conflict between the previous version of this patch and two patches merged upstream by Darshana, *by using isil everywhere and deprecating isl*. Arnd is also in copy to provide some feedback. At the moment, this last patch and the first one of this series can be applied in whatever order but I think this is just luck (first and last patch both touch rtc-isl12057.c file) so we'll have to be careful if it's merged by different maintainers.

Cheers,

a+

Some comments on the changes in first patch:

-diff -puN drivers/rtc/rtc-isl12057.c~rtc-rtc-isl12057-add-alarm-support-to-intersil-isl12057-rtc-driver drivers/rtc/rtc-isl12057.c ---- a/drivers/rtc/rtc-isl12057.c~rtc-rtc-isl12057-add-alarm-support-to-intersil-isl12057-rtc-driver
-+++ a/drivers/rtc/rtc-isl12057.c
+Index: linux/drivers/rtc/rtc-isl12057.c

+===================================================================
+--- linux.orig/drivers/rtc/rtc-isl12057.c

++++ linux/drivers/rtc/rtc-isl12057.c
@@ -1,5 +1,5 @@
/*
- * rtc-isl12057 - Driver for Intersil ISL12057 I2C Real Time Clock
@@ -335,10 +310,44 @@
static int isl12057_rtc_set_time(struct device *dev, struct rtc_time *tm) {
struct isl12057_rtc_data *data = dev_get_drvdata(dev);
-@@ -262,9 +457,48 @@ static int isl12057_check_rtc_status(str +@@ -262,9 +457,82 @@ static int isl12057_check_rtc_status(str
return 0;
}

Quoted 26 lines

++#ifdef CONFIG_OF
++/*
++ * One would expect the device to be marked as a wakeup source only ++ * when an IRQ pin of the RTC is routed to an interrupt line of the ++ * CPU. In practice, such an IRQ pin can be connected to a PMIC and ++ * this allows the device to be powered up when RTC alarm rings. This ++ * is for instance the case on ReadyNAS 102, 104 and 2120. On those ++ * devices with no IRQ driectly connected to the SoC, the RTC chip ++ * can be forced as a wakeup source by stating that explicitly in ++ * the device's .dts file using the "can-wakeup-machine" boolean ++ * property. This will guarantee 'wakealarm' sysfs entry is available ++ * on the device.
++ *
++ * The function below returns 1, i.e. the capability of the chip to ++ * wakeup the device, based on IRQ availability or if the boolean ++ * property has been set in the .dts file. Otherwise, it returns 0. ++ */
++
++static bool isl12057_can_wakeup_machine(struct device *dev) ++{
++ struct isl12057_rtc_data *data = dev_get_drvdata(dev);
++
++ return (data->irq ||
++ of_property_read_bool(dev->of_node, "can-wakeup-machine"));
++}
++#else

Quoted 8 lines

++static bool isl12057_can_wakeup_machine(struct device *dev) ++{
++ struct isl12057_rtc_data *data = dev_get_drvdata(dev);
++
++ return !!data->irq;
++}
++#endif
++

a small helper. Note: the only way to mark the device is a wakeup source is via system .dts file. Uwe, now is the time to decide if we put a prefix before the property and which ;-)

I have also added a Documentation/devicetree/bindings/rtc/isil,isl12057.txt in first patch to describe the purpose of the property and give some example. I decided to let the device in the list of trivial I2C device though, as this property is not per see required by the device, nor specific to ISL12057 but to the way the chip IRQ line is connected (or not connected) to the SoC.

Quoted 25 lines

+static int isl12057_rtc_alarm_irq_enable(struct device *dev,
+ unsigned int enable)
+{

(quoted diff) +2 -2
@@ -385,7 +394,7 @@
  };
 
  static struct regmap_config isl12057_rtc_regmap_config = {
-@@ -277,7 +511,6 @@ static int isl12057_probe(struct i2c_cli
+@@ -277,7 +545,6 @@ static int isl12057_probe(struct i2c_cli
  {
  	struct device *dev = &client->dev;
  	struct isl12057_rtc_data *data;

@@ -393,7 +402,7 @@
  	struct regmap *regmap;
  	int ret;
 
-@@ -310,10 +543,79 @@ static int isl12057_probe(struct i2c_cli
+@@ -310,9 +577,70 @@ static int isl12057_probe(struct i2c_cli
  	data->regmap = regmap;
  	dev_set_drvdata(dev, data);
 

@@ -411,14 +420,8 @@
 +				client->irq, ret);
 +	}
 +

Review line 424

Quoted 10 lines
(quoted diff) +2 -8
-+	/*
-+	 * This is needed to have 'wakealarm' sysfs entry available. One
-+	 * would expect the device to be marked as a wakeup source only
-+	 * when an IRQ pin of the RTC is routed to an interrupt line of the
-+	 * CPU. In practice, such an IRQ pin can be connected to a PMIC and
-+	 * this allows the device to be powered up when RTC alarm rings.
-+	 */
-+	device_init_wakeup(dev, true);
++	if (isl12057_can_wakeup_machine(dev))
++		device_init_wakeup(dev, true);

We mark the device as a wakeup source if irq is available or the .dts file explicitly require it via our new boolean can-wakeup-machine property.

Review

Quoted 9 lines
(quoted diff) +3 -3
++}
++
 +static int isl12057_remove(struct i2c_client *client)
 +{
-+	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(&client->dev);
-+
-+	if (rtc_data->irq > 0)
++	if (isl12057_can_wakeup_machine(&client->dev))
 +		device_init_wakeup(&client->dev, false);

Same reason is used to remove the device as from wakeup sources.

Quoted 12 lines

+
+ return 0;

(quoted diff) +1 -1
@@ -452,7 +453,7 @@
 +{
 +	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(dev);
 +
-+	if (device_may_wakeup(dev))
++	if (rtc_data->irq && device_may_wakeup(dev))
 +		return enable_irq_wake(rtc_data->irq);
 +
 +	return 0;

Review line 467

Quoted 6 lines
(quoted diff) +1 -1
@@ -462,20 +463,19 @@
 +{
 +	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(dev);
 +
-+	if (device_may_wakeup(dev))
++	if (rtc_data->irq && device_may_wakeup(dev))
 +		return disable_irq_wake(rtc_data->irq);

check we have a valid irq before passing it to enable/disable_irq_wake().

Arnaud Ebalard (5):
rtc: rtc-isl12057: add alarm support to Intersil ISL12057 RTC driver
ARM: mvebu: ISL12057 rtc chip can be used to wake up RN102
ARM: mvebu: ISL12057 rtc chip can be used to wake up RN104
ARM: mvebu: ISL12057 rtc chip can be used to wake up RN2120
dt-bindings: fix isl vs isil prefix issue for Intersil

 .../devicetree/bindings/i2c/trivial-devices.txt    |   5 +-
 .../devicetree/bindings/regulator/isl9305.txt      |   4 +-
 .../devicetree/bindings/rtc/isil,isl12057.txt      |  71 +++++
 .../devicetree/bindings/vendor-prefixes.txt        |   3 +-
 arch/arm/boot/dts/armada-370-netgear-rn102.dts     |   1 +
 arch/arm/boot/dts/armada-370-netgear-rn104.dts     |   1 +
 arch/arm/boot/dts/armada-xp-netgear-rn2120.dts     |   1 +
 arch/arm/boot/dts/tegra30-cardhu.dtsi              |   2 +-
 arch/arm/boot/dts/zynq-parallella.dts              |   2 +-
 drivers/regulator/isl9305.c                        |   6 +-
 drivers/rtc/rtc-isl12022.c                         |   3 +-
 drivers/rtc/rtc-isl12057.c                         | 353 ++++++++++++++++++++-
 drivers/staging/iio/light/isl29028.c               |   4 +-
 13 files changed, 431 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12057.txt
Signature · 1 line
2.1.1

Read the cover letter in its thread