[PATCHv1 0/6] rtc: rtc-isl12057: fixes and alarm support

[PATCHv1 0/6] rtc: rtc-isl12057: fixes and alarm support

[Arnaud Ebalard]

Hi,

This series includes six patches for Intersil ISL12057 driver.

 - First patch is a fix for masking issues which dates back to driver
   inclusion. Even if those issues are not critical per se, the patch is
   a good candidate for stable down to 3.14.

 - As suggested by Uwe, second patch adds proper support for century bit
   provided by the driver. This will allow to use the chip until 2199
   if we manage to pass 2038.

 - Following another comment by Uwe, third patch corrects the handling
   of oscillator failure bit.

 - fourth patch fixes remaining places where 'isl' is used instead of the
   expected NASDAQ symbol for Intersil (i.e. isil) after commit
   7a6540ca856ae ("ARM: mvebu: Change vendor prefix for Intersil 
   Corporation to isil"). 

 - As suggested by Mark, fifth patch improves failure reports by providing
   error code in dev_err() code.

 - Sixth patch provides alarm support for Intersil ISL12057. This
   support was not added when the driver was initially pushed in 3.14
   kernel due to the inability to check interrupt support. After some
   soldering, this tests have been performed. More time was also
   spent on testing.

Comments are welcome,

Cheers,

a+

Changes since v0:

- Kept an "isl,isl12057" compatible string for out-of-tree users w/
  a comment this is obsolete as suggested by Mark and Uwe.
- Changed function name from toggle to update for helper as suggested
  by Uwe and Guenter.
- As suggested by Mark, added a patch to improve error reporting and
  log the error code 
- Driver now handles lack of IRQ correctly by returning -ENOTTY
  if alarm_irq_enable() handler is called and no IRQ has been
  provided. Additionally, the device is marked as not supporting UIE
  when no IRQ is available.
- Following comments from Mark, added a local copy of IRQ (or lack
  of) in driver's private structure
- Following comments from Mark, removed a useless call to free_irq()
  (driver makes use of devm_request_threaded_irq() which makes the
  call pointless)
- Following report from Uwe, added a patch to handle oscillator
  failure bit properly
- Following suggestions from Uwe, added a patch to handle the century
  bit the device provides

Arnaud Ebalard (6):
  rtc: rtc-isl12057: fix masking of register values
  rtc: rtc-isl12057: add support for century bit
  rtc: rtc-isl12057: add proper handling of oscillator failure bit
  rtc: rtc-isl12057: fix isil vs isl naming for intersil
  rtc: rtc-isl12057: report error code upon failure in dev_err() calls
  rtc: rtc-isl12057: add alarm support to Intersil ISL12057 RTC driver

 .../devicetree/bindings/i2c/trivial-devices.txt    |   2 +-
 .../devicetree/bindings/vendor-prefixes.txt        |   2 +-
 drivers/rtc/rtc-isl12057.c                         | 408 +++++++++++++++++++--
 3 files changed, 377 insertions(+), 35 deletions(-)

-- 
2.1.1

[PATCHv1 1/6] rtc: rtc-isl12057: fix masking of register values

[Arnaud Ebalard]

When Intersil ISL12057 support was added by commit 70e123373c05 ("rtc:
Add support for Intersil ISL12057 I2C RTC chip"), two masks for time
registers values imported from the device were either wrong or
omitted, leading to additional bits from those registers to impact
read values:

 - mask for hour register value when reading it in AM/PM mode. As
   AM/PM mode is not the usual mode used by the driver, this error
   would only have an impact on an externally configured RTC hour
   later read by the driver.
 - mask for month value. The lack of masking would provide an
   erroneous value if century bit is set.

This patch fixes those two masks.

Fixes: 70e123373c05 ("rtc: Add support for Intersil ISL12057 I2C RTC chip")
Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
---
 drivers/rtc/rtc-isl12057.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-isl12057.c b/drivers/rtc/rtc-isl12057.c
index 455b601d731d..8c3f60737df8 100644
--- a/drivers/rtc/rtc-isl12057.c
+++ b/drivers/rtc/rtc-isl12057.c
@@ -88,7 +88,7 @@ static void isl12057_rtc_regs_to_tm(struct rtc_time *tm, u8 *regs)
 	tm->tm_min = bcd2bin(regs[ISL12057_REG_RTC_MN]);
 
 	if (regs[ISL12057_REG_RTC_HR] & ISL12057_REG_RTC_HR_MIL) { /* AM/PM */
-		tm->tm_hour = bcd2bin(regs[ISL12057_REG_RTC_HR] & 0x0f);
+		tm->tm_hour = bcd2bin(regs[ISL12057_REG_RTC_HR] & 0x1f);
 		if (regs[ISL12057_REG_RTC_HR] & ISL12057_REG_RTC_HR_PM)
 			tm->tm_hour += 12;
 	} else {					    /* 24 hour mode */
@@ -97,7 +97,7 @@ static void isl12057_rtc_regs_to_tm(struct rtc_time *tm, u8 *regs)
 
 	tm->tm_mday = bcd2bin(regs[ISL12057_REG_RTC_DT]);
 	tm->tm_wday = bcd2bin(regs[ISL12057_REG_RTC_DW]) - 1; /* starts at 1 */
-	tm->tm_mon  = bcd2bin(regs[ISL12057_REG_RTC_MO]) - 1; /* starts at 1 */
+	tm->tm_mon  = bcd2bin(regs[ISL12057_REG_RTC_MO] & 0x1f) - 1; /* ditto */
 	tm->tm_year = bcd2bin(regs[ISL12057_REG_RTC_YR]) + 100;
 }
 
-- 
2.1.1

[PATCHv1 2/6] rtc: rtc-isl12057: add support for century bit

[Arnaud Ebalard]

The month register of ISL12057 RTC chip includes a century bit
which reports overflow of year register from 99 to 0. This bit
can also be written, which allows using it to extend the time
interval the chip can support from 99 to 199 years.

This patch adds support for century overflow bit in tm to regs
and regs to tm helpers in ISL12057 driver.

This was tested by putting a device 100 years in the future (using
a specific kernel due to the inability of userland tools such as
date or hwclock to pass year 2038), rebooting on a kernel w/ this
patch applied and verifying the device was still 100 years in the
future.

Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
---
 drivers/rtc/rtc-isl12057.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-isl12057.c b/drivers/rtc/rtc-isl12057.c
index 8c3f60737df8..b538fabfcfd3 100644
--- a/drivers/rtc/rtc-isl12057.c
+++ b/drivers/rtc/rtc-isl12057.c
@@ -41,6 +41,7 @@
 #define ISL12057_REG_RTC_DW	0x03	/* Day of the Week */
 #define ISL12057_REG_RTC_DT	0x04	/* Date */
 #define ISL12057_REG_RTC_MO	0x05	/* Month */
+#define ISL12057_REG_RTC_MO_CEN	BIT(7)	/* Century bit */
 #define ISL12057_REG_RTC_YR	0x06	/* Year */
 #define ISL12057_RTC_SEC_LEN	7
 
@@ -99,24 +100,35 @@ static void isl12057_rtc_regs_to_tm(struct rtc_time *tm, u8 *regs)
 	tm->tm_wday = bcd2bin(regs[ISL12057_REG_RTC_DW]) - 1; /* starts at 1 */
 	tm->tm_mon  = bcd2bin(regs[ISL12057_REG_RTC_MO] & 0x1f) - 1; /* ditto */
 	tm->tm_year = bcd2bin(regs[ISL12057_REG_RTC_YR]) + 100;
+
+	/* Check if years register has overflown from 99 to 00 */
+	if (regs[ISL12057_REG_RTC_MO] & ISL12057_REG_RTC_MO_CEN)
+		tm->tm_year += 100;
 }
 
 static int isl12057_rtc_tm_to_regs(u8 *regs, struct rtc_time *tm)
 {
+	u8 century_bit;
+
 	/*
 	 * The clock has an 8 bit wide bcd-coded register for the year.
+	 * It also has a century bit encoded in MO flag which provides
+	 * information about overflow of year register from 99 to 00.
 	 * tm_year is an offset from 1900 and we are interested in the
-	 * 2000-2099 range, so any value less than 100 is invalid.
+	 * 2000-2199 range, so any value less than 100 or larger than
+	 * 299 is invalid.
 	 */
-	if (tm->tm_year < 100)
+	if (tm->tm_year < 100 || tm->tm_year > 299)
 		return -EINVAL;
 
+	century_bit = (tm->tm_year > 199) ? ISL12057_REG_RTC_MO_CEN : 0;
+
 	regs[ISL12057_REG_RTC_SC] = bin2bcd(tm->tm_sec);
 	regs[ISL12057_REG_RTC_MN] = bin2bcd(tm->tm_min);
 	regs[ISL12057_REG_RTC_HR] = bin2bcd(tm->tm_hour); /* 24-hour format */
 	regs[ISL12057_REG_RTC_DT] = bin2bcd(tm->tm_mday);
-	regs[ISL12057_REG_RTC_MO] = bin2bcd(tm->tm_mon + 1);
-	regs[ISL12057_REG_RTC_YR] = bin2bcd(tm->tm_year - 100);
+	regs[ISL12057_REG_RTC_MO] = bin2bcd(tm->tm_mon + 1) | century_bit;
+	regs[ISL12057_REG_RTC_YR] = bin2bcd(tm->tm_year % 100);
 	regs[ISL12057_REG_RTC_DW] = bin2bcd(tm->tm_wday + 1);
 
 	return 0;
-- 
2.1.1

[PATCHv1 3/6] rtc: rtc-isl12057: add proper handling of oscillator failure bit

[Arnaud Ebalard]

As suggested by Uwe, instead of clearing oscillator failure bit
unconditionally at driver load, this patch adds proper handling
of the flag. The driver now returns -ENODATA when reading time
from the device and oscillator failure bit is set. The flag
is now cleared only when the a new time value is pushed to the
device.

Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
---
 drivers/rtc/rtc-isl12057.c | 47 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/drivers/rtc/rtc-isl12057.c b/drivers/rtc/rtc-isl12057.c
index b538fabfcfd3..fe562820a54a 100644
--- a/drivers/rtc/rtc-isl12057.c
+++ b/drivers/rtc/rtc-isl12057.c
@@ -164,17 +164,32 @@ static int isl12057_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct isl12057_rtc_data *data = dev_get_drvdata(dev);
 	u8 regs[ISL12057_RTC_SEC_LEN];
+	unsigned int sr;
 	int ret;
 
 	mutex_lock(&data->lock);
+	ret = regmap_read(data->regmap, ISL12057_REG_SR, &sr);
+	if (ret) {
+		dev_err(dev, "%s: unable to read oscillator status flag\n",
+			__func__);
+		goto out;
+	} else {
+		if (sr & ISL12057_REG_SR_OSF) {
+			ret = -ENODATA;
+			goto out;
+		}
+	}
+
 	ret = regmap_bulk_read(data->regmap, ISL12057_REG_RTC_SC, regs,
 			       ISL12057_RTC_SEC_LEN);
+	if (ret)
+		dev_err(dev, "%s: unable to read RTC time\n", __func__);
+
+out:
 	mutex_unlock(&data->lock);
 
-	if (ret) {
-		dev_err(dev, "%s: RTC read failed\n", __func__);
+	if (ret)
 		return ret;
-	}
 
 	isl12057_rtc_regs_to_tm(tm, regs);
 
@@ -194,10 +209,22 @@ static int isl12057_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	mutex_lock(&data->lock);
 	ret = regmap_bulk_write(data->regmap, ISL12057_REG_RTC_SC, regs,
 				ISL12057_RTC_SEC_LEN);
-	mutex_unlock(&data->lock);
+	if (ret) {
+		dev_err(dev, "%s: writing RTC time failed\n", __func__);
+		goto out;
+	}
 
-	if (ret)
-		dev_err(dev, "%s: RTC write failed\n", __func__);
+	/*
+	 * Now that RTC time has been updated, let's clear oscillator
+	 * failure flag, if needed.
+	 */
+	ret = regmap_update_bits(data->regmap, ISL12057_REG_SR,
+				 ISL12057_REG_SR_OSF, 0);
+	if (ret < 0)
+		dev_err(dev, "Unable to clear oscillator failure bit\n");
+
+out:
+	mutex_unlock(&data->lock);
 
 	return ret;
 }
@@ -219,14 +246,6 @@ static int isl12057_check_rtc_status(struct device *dev, struct regmap *regmap)
 		return ret;
 	}
 
-	/* Clear oscillator failure bit if needed */
-	ret = regmap_update_bits(regmap, ISL12057_REG_SR,
-				 ISL12057_REG_SR_OSF, 0);
-	if (ret < 0) {
-		dev_err(dev, "Unable to clear oscillator failure bit\n");
-		return ret;
-	}
-
 	/* Clear alarm bit if needed */
 	ret = regmap_update_bits(regmap, ISL12057_REG_SR,
 				 ISL12057_REG_SR_A1F, 0);
-- 
2.1.1

[PATCHv1 4/6] rtc: rtc-isl12057: fix isil vs isl naming for intersil

[Arnaud Ebalard]

When Intersil ISL12057 driver was introduced by commit 70e123373c05
("rtc: Add support for Intersil ISL12057 I2C RTC chip"), the vendor
prefix 'isl' was used instead of the expected 'isil' (Intersil
NASDAQ symbol). Recently, a patch from Philip Zabel (7a6540ca856a,
"ARM: mvebu: Change vendor prefix for Intersil Corporation to isil")
fixed that prefix in ReadyNAS devices .dts files (AFAICT, the only
users).

That patch completes that work to change the remaining values in:

 - Intersil ISL12057 driver (compatible string) and also associated
   Documentation/devicetree/bindings/i2c/trivial-devices.txt file
   to reflect the changes introduced by previous commit from Philip
 - Documentation/devicetree/bindings/vendor-prefixes.txt files to
   avoid future mistakes

Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
---
 Documentation/devicetree/bindings/i2c/trivial-devices.txt | 2 +-
 Documentation/devicetree/bindings/vendor-prefixes.txt     | 2 +-
 drivers/rtc/rtc-isl12057.c                                | 3 ++-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index fbde415078e6..edac97c0f756 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -55,7 +55,7 @@ fsl,sgtl5000		SGTL5000: Ultra Low-Power Audio Codec
 gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
 infineon,slb9635tt	Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
 infineon,slb9645tt	Infineon SLB9645 I2C TPM (new protocol, max 400khz)
-isl,isl12057		Intersil ISL12057 I2C RTC Chip
+isil,isl12057		Intersil ISL12057 I2C RTC Chip
 maxim,ds1050		5 Bit Programmable, Pulse-Width Modulator
 maxim,max1237		Low-Power, 4-/12-Channel, 2-Wire Serial, 12-Bit ADCs
 maxim,max6625		9-Bit/12-Bit Temperature Sensors with I²C-Compatible Serial Interface
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 723999d73744..84193ecdc41c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -77,7 +77,7 @@ innolux	Innolux Corporation
 intel	Intel Corporation
 intercontrol	Inter Control Group
 isee	ISEE 2007 S.L.
-isl	Intersil
+isil	Intersil
 karo	Ka-Ro electronics GmbH
 keymile	Keymile GmbH
 lacie	LaCie
diff --git a/drivers/rtc/rtc-isl12057.c b/drivers/rtc/rtc-isl12057.c
index fe562820a54a..31eaa6a9c2b2 100644
--- a/drivers/rtc/rtc-isl12057.c
+++ b/drivers/rtc/rtc-isl12057.c
@@ -310,7 +310,8 @@ static int isl12057_probe(struct i2c_client *client,
 
 #ifdef CONFIG_OF
 static const struct of_device_id isl12057_dt_match[] = {
-	{ .compatible = "isl,isl12057" },
+	{ .compatible = "isl,isl12057" },  /* obsolete */
+	{ .compatible = "isil,isl12057" },
 	{ },
 };
 #endif
-- 
2.1.1

Re: [rtc-linux] [PATCHv1 4/6] rtc: rtc-isl12057: fix isil vs isl naming for intersil

[Andrew Morton]

On Sat, 15 Nov 2014 00:07:05 +0100 Arnaud Ebalard <arno@natisbad.org> wrote:

> 
> When Intersil ISL12057 driver was introduced by commit 70e123373c05
> ("rtc: Add support for Intersil ISL12057 I2C RTC chip"), the vendor
> prefix 'isl' was used instead of the expected 'isil' (Intersil
> NASDAQ symbol). Recently, a patch from Philip Zabel (7a6540ca856a,
> "ARM: mvebu: Change vendor prefix for Intersil Corporation to isil")
> fixed that prefix in ReadyNAS devices .dts files (AFAICT, the only
> users).
> 
> That patch completes that work to change the remaining values in:
> 
>  - Intersil ISL12057 driver (compatible string) and also associated
>    Documentation/devicetree/bindings/i2c/trivial-devices.txt file
>    to reflect the changes introduced by previous commit from Philip
>  - Documentation/devicetree/bindings/vendor-prefixes.txt files to
>    avoid future mistakes
> 
> ...
>
> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> index fbde415078e6..edac97c0f756 100644
> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> @@ -55,7 +55,7 @@ fsl,sgtl5000		SGTL5000: Ultra Low-Power Audio Codec
>  gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
>  infineon,slb9635tt	Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
>  infineon,slb9645tt	Infineon SLB9645 I2C TPM (new protocol, max 400khz)
> -isl,isl12057		Intersil ISL12057 I2C RTC Chip
> +isil,isl12057		Intersil ISL12057 I2C RTC Chip
>  maxim,ds1050		5 Bit Programmable, Pulse-Width Modulator
>  maxim,max1237		Low-Power, 4-/12-Channel, 2-Wire Serial, 12-Bit ADCs
>  maxim,max6625		9-Bit/12-Bit Temperature Sensors with I__C-Compatible Serial Interface
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 723999d73744..84193ecdc41c 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -77,7 +77,7 @@ innolux	Innolux Corporation
>  intel	Intel Corporation
>  intercontrol	Inter Control Group
>  isee	ISEE 2007 S.L.
> -isl	Intersil
> +isil	Intersil
>  karo	Ka-Ro electronics GmbH
>  keymile	Keymile GmbH
>  lacie	LaCie
> diff --git a/drivers/rtc/rtc-isl12057.c b/drivers/rtc/rtc-isl12057.c
> index fe562820a54a..31eaa6a9c2b2 100644
> --- a/drivers/rtc/rtc-isl12057.c
> +++ b/drivers/rtc/rtc-isl12057.c
> @@ -310,7 +310,8 @@ static int isl12057_probe(struct i2c_client *client,
>  
>  #ifdef CONFIG_OF
>  static const struct of_device_id isl12057_dt_match[] = {
> -	{ .compatible = "isl,isl12057" },
> +	{ .compatible = "isl,isl12057" },  /* obsolete */
> +	{ .compatible = "isil,isl12057" },
>  	{ },
>  };
>  #endif

Your patch conflicts both textually and materially with the two below
patches, which are now upstream.

I don't know what to do about this so I'll drop "rtc: rtc-isl12057: fix
isil vs isl naming for intersil".  Could you folks please sort this all
out?



commit 7c75c1d5e72be6736aa558efd33ca2b31d77e425
Author:     Darshana Padmadas <darshanapadmadas@gmail.com>
AuthorDate: Sat Nov 8 22:52:06 2014 +0530
Commit:     Arnd Bergmann <arnd@arndb.de>
CommitDate: Thu Nov 20 12:20:18 2014 +0100

    dt-bindings: Document deprecated device vendor name to fix related warning
    
    This patch documents deprecated vendor name "isil" to fix warning of
    undocumented string for device isl29028 as reported while running checkpatch.pl
    on drivers/staging/iio/light/isl29028.c. This is done to maintain compatibility
    with older kernels.
    
    Signed-off-by: Darshana Padmadas <darshanapadmadas@gmail.com>
    Acked-by: Arnd Bergmann <arnd@arndb.de>
    Acked-by: Mark Rutland <mark.rutland@arm.com>
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 723999d..bfbd93e 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -77,6 +77,7 @@ innolux	Innolux Corporation
 intel	Intel Corporation
 intercontrol	Inter Control Group
 isee	ISEE 2007 S.L.
+isil    Intersil (deprecated, use isl)
 isl	Intersil
 karo	Ka-Ro electronics GmbH
 keymile	Keymile GmbH


commit b2ea3f82e7984d855c5e61fef18746771d90c73c
Author:     Darshana Padmadas <darshanapadmadas@gmail.com>
AuthorDate: Sat Nov 8 22:52:05 2014 +0530
Commit:     Arnd Bergmann <arnd@arndb.de>
CommitDate: Thu Nov 20 12:19:59 2014 +0100

    dt-bindings: Document correct and deprecated vendor-prefix with device isl29028
    
    This patch documents the device isl29028 with its vendor-prefix. Undocumented deprecated vendor-prefix
    found by checkpatch also documented for compatibility reasons.
    
    Signed-off-by: Darshana Padmadas <darshanapadmadas@gmail.com>
    Acked-by: Arnd Bergmann <arnd@arndb.de>
    Acked-by: Mark Rutland <mark.rutland@arm.com>
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index fbde415..605dcca 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -56,6 +56,8 @@ gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire In
 infineon,slb9635tt	Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
 infineon,slb9645tt	Infineon SLB9645 I2C TPM (new protocol, max 400khz)
 isl,isl12057		Intersil ISL12057 I2C RTC Chip
+isil,isl29028           (deprecated, use isl)
+isl,isl29028            Intersil ISL29028 Ambient Light and Proximity Sensor
 maxim,ds1050		5 Bit Programmable, Pulse-Width Modulator
 maxim,max1237		Low-Power, 4-/12-Channel, 2-Wire Serial, 12-Bit ADCs
 maxim,max6625		9-Bit/12-Bit Temperature Sensors with I²C-Compatible Serial Interface

Re: [rtc-linux] [PATCHv1 4/6] rtc: rtc-isl12057: fix isil vs isl naming for intersil

[Arnaud Ebalard]

Hi Arnd and Darshana,

Sorry for somehow top-posting, but let me provide some context to what
follows. While pushing a set of patches for ISL 12057 RTC chip, I tried
and continue what had been started by Philip Zabel (7a6540ca856a,
"ARM: mvebu: Change vendor prefix for Intersil Corporation to isil")
in one of the patches (see below) by switching the vendor prefix for
Intersil from isl to isil. This specific patch was handled by Andrew via
his -mm tree and now conflicts with what you pushed upstream.

I do not have a strong advice on what is better and will follow what you
decide by providing an updated patch to Andrew but:

 - your changes somehow revert what has been done by philip
 - three .dts files (those updated by Philip for isil) now have a deprecated prefix
 - we now have both isil and isl in vendor-prefixes.txt
 - FWIW, isil is the nasdaq symbol

Cheers,

a+

Andrew Morton <akpm@linux-foundation.org> writes:

> On Sat, 15 Nov 2014 00:07:05 +0100 Arnaud Ebalard <arno@natisbad.org> wrote:
>
>> 
>> When Intersil ISL12057 driver was introduced by commit 70e123373c05
>> ("rtc: Add support for Intersil ISL12057 I2C RTC chip"), the vendor
>> prefix 'isl' was used instead of the expected 'isil' (Intersil
>> NASDAQ symbol). Recently, a patch from Philip Zabel (7a6540ca856a,
>> "ARM: mvebu: Change vendor prefix for Intersil Corporation to isil")
>> fixed that prefix in ReadyNAS devices .dts files (AFAICT, the only
>> users).
>> 
>> That patch completes that work to change the remaining values in:
>> 
>>  - Intersil ISL12057 driver (compatible string) and also associated
>>    Documentation/devicetree/bindings/i2c/trivial-devices.txt file
>>    to reflect the changes introduced by previous commit from Philip
>>  - Documentation/devicetree/bindings/vendor-prefixes.txt files to
>>    avoid future mistakes
>> 
>> ...
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> index fbde415078e6..edac97c0f756 100644
>> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> @@ -55,7 +55,7 @@ fsl,sgtl5000		SGTL5000: Ultra Low-Power Audio Codec
>>  gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
>>  infineon,slb9635tt	Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
>>  infineon,slb9645tt	Infineon SLB9645 I2C TPM (new protocol, max 400khz)
>> -isl,isl12057		Intersil ISL12057 I2C RTC Chip
>> +isil,isl12057		Intersil ISL12057 I2C RTC Chip
>>  maxim,ds1050		5 Bit Programmable, Pulse-Width Modulator
>>  maxim,max1237		Low-Power, 4-/12-Channel, 2-Wire Serial, 12-Bit ADCs
>>  maxim,max6625		9-Bit/12-Bit Temperature Sensors with I__C-Compatible Serial Interface
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> index 723999d73744..84193ecdc41c 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> @@ -77,7 +77,7 @@ innolux	Innolux Corporation
>>  intel	Intel Corporation
>>  intercontrol	Inter Control Group
>>  isee	ISEE 2007 S.L.
>> -isl	Intersil
>> +isil	Intersil
>>  karo	Ka-Ro electronics GmbH
>>  keymile	Keymile GmbH
>>  lacie	LaCie
>> diff --git a/drivers/rtc/rtc-isl12057.c b/drivers/rtc/rtc-isl12057.c
>> index fe562820a54a..31eaa6a9c2b2 100644
>> --- a/drivers/rtc/rtc-isl12057.c
>> +++ b/drivers/rtc/rtc-isl12057.c
>> @@ -310,7 +310,8 @@ static int isl12057_probe(struct i2c_client *client,
>>  
>>  #ifdef CONFIG_OF
>>  static const struct of_device_id isl12057_dt_match[] = {
>> -	{ .compatible = "isl,isl12057" },
>> +	{ .compatible = "isl,isl12057" },  /* obsolete */
>> +	{ .compatible = "isil,isl12057" },
>>  	{ },
>>  };
>>  #endif
>
> Your patch conflicts both textually and materially with the two below
> patches, which are now upstream.
>
> I don't know what to do about this so I'll drop "rtc: rtc-isl12057: fix
> isil vs isl naming for intersil".  Could you folks please sort this all
> out?
>
>
>
> commit 7c75c1d5e72be6736aa558efd33ca2b31d77e425
> Author:     Darshana Padmadas <darshanapadmadas@gmail.com>
> AuthorDate: Sat Nov 8 22:52:06 2014 +0530
> Commit:     Arnd Bergmann <arnd@arndb.de>
> CommitDate: Thu Nov 20 12:20:18 2014 +0100
>
>     dt-bindings: Document deprecated device vendor name to fix related warning
>     
>     This patch documents deprecated vendor name "isil" to fix warning of
>     undocumented string for device isl29028 as reported while running checkpatch.pl
>     on drivers/staging/iio/light/isl29028.c. This is done to maintain compatibility
>     with older kernels.
>     
>     Signed-off-by: Darshana Padmadas <darshanapadmadas@gmail.com>
>     Acked-by: Arnd Bergmann <arnd@arndb.de>
>     Acked-by: Mark Rutland <mark.rutland@arm.com>
>     Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 723999d..bfbd93e 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -77,6 +77,7 @@ innolux	Innolux Corporation
>  intel	Intel Corporation
>  intercontrol	Inter Control Group
>  isee	ISEE 2007 S.L.
> +isil    Intersil (deprecated, use isl)
>  isl	Intersil
>  karo	Ka-Ro electronics GmbH
>  keymile	Keymile GmbH
>
>
> commit b2ea3f82e7984d855c5e61fef18746771d90c73c
> Author:     Darshana Padmadas <darshanapadmadas@gmail.com>
> AuthorDate: Sat Nov 8 22:52:05 2014 +0530
> Commit:     Arnd Bergmann <arnd@arndb.de>
> CommitDate: Thu Nov 20 12:19:59 2014 +0100
>
>     dt-bindings: Document correct and deprecated vendor-prefix with device isl29028
>     
>     This patch documents the device isl29028 with its vendor-prefix. Undocumented deprecated vendor-prefix
>     found by checkpatch also documented for compatibility reasons.
>     
>     Signed-off-by: Darshana Padmadas <darshanapadmadas@gmail.com>
>     Acked-by: Arnd Bergmann <arnd@arndb.de>
>     Acked-by: Mark Rutland <mark.rutland@arm.com>
>     Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> index fbde415..605dcca 100644
> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> @@ -56,6 +56,8 @@ gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire In
>  infineon,slb9635tt	Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
>  infineon,slb9645tt	Infineon SLB9645 I2C TPM (new protocol, max 400khz)
>  isl,isl12057		Intersil ISL12057 I2C RTC Chip
> +isil,isl29028           (deprecated, use isl)
> +isl,isl29028            Intersil ISL29028 Ambient Light and Proximity Sensor
>  maxim,ds1050		5 Bit Programmable, Pulse-Width Modulator
>  maxim,max1237		Low-Power, 4-/12-Channel, 2-Wire Serial, 12-Bit ADCs
>  maxim,max6625		9-Bit/12-Bit Temperature Sensors with I²C-Compatible Serial Interface

[PATCHv1 5/6] rtc: rtc-isl12057: report error code upon failure in dev_err() calls

[Arnaud Ebalard]

As pointed by Mark, it is generally useful to log the error code when
reporting a failure. This patch improves existing calls to dev_err()
in ISL12057 driver to also report error code.

Suggested-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
---
 drivers/rtc/rtc-isl12057.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-isl12057.c b/drivers/rtc/rtc-isl12057.c
index 31eaa6a9c2b2..a783e16cfd2f 100644
--- a/drivers/rtc/rtc-isl12057.c
+++ b/drivers/rtc/rtc-isl12057.c
@@ -170,8 +170,8 @@ static int isl12057_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	mutex_lock(&data->lock);
 	ret = regmap_read(data->regmap, ISL12057_REG_SR, &sr);
 	if (ret) {
-		dev_err(dev, "%s: unable to read oscillator status flag\n",
-			__func__);
+		dev_err(dev, "%s: unable to read oscillator status flag (%d)\n",
+			__func__, ret);
 		goto out;
 	} else {
 		if (sr & ISL12057_REG_SR_OSF) {
@@ -183,7 +183,8 @@ static int isl12057_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	ret = regmap_bulk_read(data->regmap, ISL12057_REG_RTC_SC, regs,
 			       ISL12057_RTC_SEC_LEN);
 	if (ret)
-		dev_err(dev, "%s: unable to read RTC time\n", __func__);
+		dev_err(dev, "%s: unable to read RTC time section (%d)\n",
+			__func__, ret);
 
 out:
 	mutex_unlock(&data->lock);
@@ -210,7 +211,8 @@ static int isl12057_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	ret = regmap_bulk_write(data->regmap, ISL12057_REG_RTC_SC, regs,
 				ISL12057_RTC_SEC_LEN);
 	if (ret) {
-		dev_err(dev, "%s: writing RTC time failed\n", __func__);
+		dev_err(dev, "%s: unable to write RTC time section (%d)\n",
+			__func__, ret);
 		goto out;
 	}
 
@@ -221,7 +223,8 @@ static int isl12057_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	ret = regmap_update_bits(data->regmap, ISL12057_REG_SR,
 				 ISL12057_REG_SR_OSF, 0);
 	if (ret < 0)
-		dev_err(dev, "Unable to clear oscillator failure bit\n");
+		dev_err(dev, "%s: unable to clear osc. failure bit (%d)\n",
+			__func__, ret);
 
 out:
 	mutex_unlock(&data->lock);
@@ -242,7 +245,8 @@ static int isl12057_check_rtc_status(struct device *dev, struct regmap *regmap)
 	ret = regmap_update_bits(regmap, ISL12057_REG_INT,
 				 ISL12057_REG_INT_EOSC, 0);
 	if (ret < 0) {
-		dev_err(dev, "Unable to enable oscillator\n");
+		dev_err(dev, "%s: unable to enable oscillator (%d)\n",
+			__func__, ret);
 		return ret;
 	}
 
@@ -250,7 +254,8 @@ static int isl12057_check_rtc_status(struct device *dev, struct regmap *regmap)
 	ret = regmap_update_bits(regmap, ISL12057_REG_SR,
 				 ISL12057_REG_SR_A1F, 0);
 	if (ret < 0) {
-		dev_err(dev, "Unable to clear alarm bit\n");
+		dev_err(dev, "%s: unable to clear alarm bit (%d)\n",
+			__func__, ret);
 		return ret;
 	}
 
@@ -284,7 +289,8 @@ static int isl12057_probe(struct i2c_client *client,
 	regmap = devm_regmap_init_i2c(client, &isl12057_rtc_regmap_config);
 	if (IS_ERR(regmap)) {
 		ret = PTR_ERR(regmap);
-		dev_err(dev, "regmap allocation failed: %d\n", ret);
+		dev_err(dev, "%s: regmap allocation failed (%d)\n",
+			__func__, ret);
 		return ret;
 	}
 
-- 
2.1.1

[PATCHv1 6/6] rtc: rtc-isl12057: add alarm support to Intersil ISL12057 RTC driver

[Arnaud Ebalard]

This patch adds alarm support to Intersil ISL12057 driver. This allows
to configure the chip to generate an interrupt when the alarm matches
current time value. Alarm can be programmed up to one month in the
future and is accurate to the second.

The patch was developed to support two different configurations:
systems w/ and w/o RTC chip IRQ line connected to the main CPU.

The latter is the one found on current 3 kernel users of the chip
for which support was initially developed (Netgear ReadyNAS 102,
104 and 2120 NAS). On those devices, the IRQ#2 pin of the chip is not
connected to the SoC but to a PMIC. This allows setting an alarm,
powering off the device and have it wake up when the alarm rings.
To support that configuration the driver does the following:

 1. it has alarm_irq_enable() function returns -ENOTTY when no IRQ
    is passed to the driver.
 2. it marks the device as a wakeup source in all cases (whether an
    IRQ is passed to the driver or not) to have 'wakealarm' sysfs
    entry created.
 3. it marks the device has not supporting UIE mode when no IRQ is
    passed to the driver (see the commmit message of c9f5c7e7a84f)

This specific configuration was tested on a ReadyNAS 102 by setting
an alarm, powering off the device and see it reboot as expected
when the alarm rang.

The former configuration was tested on a Netgear ReadyNAS 102 after
some soldering of the IRQ#2 pin of the RTC chip to a MPP line of the
SoC (the one used usually handles the reset button). The test was
performed using a modified .dts file reflecting this change (see below)
and rtc-test.c program available in Documentation/rtc.txt. This test
program ran as expected, which validates alarm supports, including
interrupt support.

As a side note, the ISL12057 remains in the list of trivial devices,
i.e. no specific DT binding being added by this patch: i2c core
automatically handles extraction of IRQ line info from .dts file. For
instance, if one wants to reference the interrupt line for the
alarm in its .dts file, adding interrupt and interrupt-parent
properties works as expected:

          isl12057: isl12057@68 {
                  compatible = "isil,isl12057";
                  interrupt-parent = <&gpio0>;
                  interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
                  reg = <0x68>;
          };

FWIW, if someone is looking for a way to test alarm support on a system
on which the chip IRQ line has the ability to boot the system (e.g.
ReadyNAS 102, 104, etc):

    # echo 0 > /sys/class/rtc/rtc0/wakealarm
    # echo `date '+%s' -d '+ 1 minutes'` > /sys/class/rtc/rtc0/wakealarm
    # shutdown -h now

With the commandes above, after a minute, the system comes back to life.

Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
---
 .../devicetree/bindings/i2c/trivial-devices.txt    |   2 +-
 drivers/rtc/rtc-isl12057.c                         | 324 ++++++++++++++++++++-
 2 files changed, 315 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index edac97c0f756..f716e7da44a9 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -55,7 +55,7 @@ fsl,sgtl5000		SGTL5000: Ultra Low-Power Audio Codec
 gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
 infineon,slb9635tt	Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
 infineon,slb9645tt	Infineon SLB9645 I2C TPM (new protocol, max 400khz)
-isil,isl12057		Intersil ISL12057 I2C RTC Chip
+isil,isl12057		Intersil ISL12057 I2C RTC/Alarm Chip
 maxim,ds1050		5 Bit Programmable, Pulse-Width Modulator
 maxim,max1237		Low-Power, 4-/12-Channel, 2-Wire Serial, 12-Bit ADCs
 maxim,max6625		9-Bit/12-Bit Temperature Sensors with I²C-Compatible Serial Interface
diff --git a/drivers/rtc/rtc-isl12057.c b/drivers/rtc/rtc-isl12057.c
index a783e16cfd2f..2c5b5ecb30b6 100644
--- a/drivers/rtc/rtc-isl12057.c
+++ b/drivers/rtc/rtc-isl12057.c
@@ -1,5 +1,5 @@
 /*
- * rtc-isl12057 - Driver for Intersil ISL12057 I2C Real Time Clock
+ * rtc-isl12057 - Driver for Intersil ISL12057 I2C Real Time Clock / Alarm
  *
  * Copyright (C) 2013, Arnaud EBALARD <arno@natisbad.org>
  *
@@ -79,8 +79,10 @@
 #define ISL12057_MEM_MAP_LEN	0x10
 
 struct isl12057_rtc_data {
+	struct rtc_device *rtc;
 	struct regmap *regmap;
 	struct mutex lock;
+	int irq;
 };
 
 static void isl12057_rtc_regs_to_tm(struct rtc_time *tm, u8 *regs)
@@ -160,14 +162,47 @@ static int isl12057_i2c_validate_chip(struct regmap *regmap)
 	return 0;
 }
 
-static int isl12057_rtc_read_time(struct device *dev, struct rtc_time *tm)
+static int _isl12057_rtc_clear_alarm(struct device *dev)
+{
+	struct isl12057_rtc_data *data = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regmap_update_bits(data->regmap, ISL12057_REG_SR,
+				 ISL12057_REG_SR_A1F, 0);
+	if (ret)
+		dev_err(dev, "%s: clearing alarm failed (%d)\n", __func__, ret);
+
+	return ret;
+}
+
+static int _isl12057_rtc_update_alarm(struct device *dev, int enable)
+{
+	struct isl12057_rtc_data *data = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regmap_update_bits(data->regmap, ISL12057_REG_INT,
+				 ISL12057_REG_INT_A1IE,
+				 enable ? ISL12057_REG_INT_A1IE : 0);
+	if (ret)
+		dev_err(dev, "%s: changing alarm interrupt flag failed (%d)\n",
+			__func__, ret);
+
+	return ret;
+}
+
+/*
+ * Note: as we only read from device and do not perform any update, there is
+ * no need for an equivalent function which would try and get driver's main
+ * lock. Here, it is safe for everyone if we just use regmap internal lock
+ * on the device when reading.
+ */
+static int _isl12057_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct isl12057_rtc_data *data = dev_get_drvdata(dev);
 	u8 regs[ISL12057_RTC_SEC_LEN];
 	unsigned int sr;
 	int ret;
 
-	mutex_lock(&data->lock);
 	ret = regmap_read(data->regmap, ISL12057_REG_SR, &sr);
 	if (ret) {
 		dev_err(dev, "%s: unable to read oscillator status flag (%d)\n",
@@ -187,8 +222,6 @@ static int isl12057_rtc_read_time(struct device *dev, struct rtc_time *tm)
 			__func__, ret);
 
 out:
-	mutex_unlock(&data->lock);
-
 	if (ret)
 		return ret;
 
@@ -197,6 +230,168 @@ out:
 	return rtc_valid_tm(tm);
 }
 
+static int isl12057_rtc_update_alarm(struct device *dev, int enable)
+{
+	struct isl12057_rtc_data *data = dev_get_drvdata(dev);
+	int ret;
+
+	mutex_lock(&data->lock);
+	ret = _isl12057_rtc_update_alarm(dev, enable);
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int isl12057_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+	struct isl12057_rtc_data *data = dev_get_drvdata(dev);
+	struct rtc_time rtc_tm, *alarm_tm = &alarm->time;
+	unsigned long rtc_secs, alarm_secs;
+	u8 regs[ISL12057_A1_SEC_LEN];
+	unsigned int ir;
+	int ret;
+
+	mutex_lock(&data->lock);
+	ret = regmap_bulk_read(data->regmap, ISL12057_REG_A1_SC, regs,
+			       ISL12057_A1_SEC_LEN);
+	if (ret) {
+		dev_err(dev, "%s: reading alarm section failed (%d)\n",
+			__func__, ret);
+		goto err_unlock;
+	}
+
+	alarm_tm->tm_sec  = bcd2bin(regs[0] & 0x7f);
+	alarm_tm->tm_min  = bcd2bin(regs[1] & 0x7f);
+	alarm_tm->tm_hour = bcd2bin(regs[2] & 0x3f);
+	alarm_tm->tm_mday = bcd2bin(regs[3] & 0x3f);
+	alarm_tm->tm_wday = -1;
+
+	/*
+	 * The alarm section does not store year/month. We use the ones in rtc
+	 * section as a basis and increment month and then year if needed to get
+	 * alarm after current time.
+	 */
+	ret = _isl12057_rtc_read_time(dev, &rtc_tm);
+	if (ret)
+		goto err_unlock;
+
+	alarm_tm->tm_year = rtc_tm.tm_year;
+	alarm_tm->tm_mon = rtc_tm.tm_mon;
+
+	ret = rtc_tm_to_time(&rtc_tm, &rtc_secs);
+	if (ret)
+		goto err_unlock;
+
+	ret = rtc_tm_to_time(alarm_tm, &alarm_secs);
+	if (ret)
+		goto err_unlock;
+
+	if (alarm_secs < rtc_secs) {
+		if (alarm_tm->tm_mon == 11) {
+			alarm_tm->tm_mon = 0;
+			alarm_tm->tm_year += 1;
+		} else {
+			alarm_tm->tm_mon += 1;
+		}
+	}
+
+	ret = regmap_read(data->regmap, ISL12057_REG_INT, &ir);
+	if (ret) {
+		dev_err(dev, "%s: reading alarm interrupt flag failed (%d)\n",
+			__func__, ret);
+		goto err_unlock;
+	}
+
+	alarm->enabled = !!(ir & ISL12057_REG_INT_A1IE);
+
+err_unlock:
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int isl12057_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+	struct isl12057_rtc_data *data = dev_get_drvdata(dev);
+	struct rtc_time *alarm_tm = &alarm->time;
+	unsigned long rtc_secs, alarm_secs;
+	u8 regs[ISL12057_A1_SEC_LEN];
+	struct rtc_time rtc_tm;
+	int ret, enable = 1;
+
+	mutex_lock(&data->lock);
+	ret = _isl12057_rtc_read_time(dev, &rtc_tm);
+	if (ret)
+		goto err_unlock;
+
+	ret = rtc_tm_to_time(&rtc_tm, &rtc_secs);
+	if (ret)
+		goto err_unlock;
+
+	ret = rtc_tm_to_time(alarm_tm, &alarm_secs);
+	if (ret)
+		goto err_unlock;
+
+	/* If alarm time is before current time, disable the alarm */
+	if (!alarm->enabled || alarm_secs <= rtc_secs) {
+		enable = 0;
+	} else {
+		/*
+		 * Chip only support alarms up to one month in the future. Let's
+		 * return an error if we get something after that limit.
+		 * Comparison is done by incrementing rtc_tm month field by one
+		 * and checking alarm value is still below.
+		 */
+		if (rtc_tm.tm_mon == 11) { /* handle year wrapping */
+			rtc_tm.tm_mon = 0;
+			rtc_tm.tm_year += 1;
+		} else {
+			rtc_tm.tm_mon += 1;
+		}
+
+		ret = rtc_tm_to_time(&rtc_tm, &rtc_secs);
+		if (ret)
+			goto err_unlock;
+
+		if (alarm_secs > rtc_secs) {
+			dev_err(dev, "%s: max for alarm is one month (%d)\n",
+				__func__, ret);
+			ret = -EINVAL;
+			goto err_unlock;
+		}
+	}
+
+	/* Disable the alarm before modifying it */
+	ret = _isl12057_rtc_update_alarm(dev, 0);
+	if (ret < 0) {
+		dev_err(dev, "%s: unable to disable the alarm (%d)\n",
+			__func__, ret);
+		goto err_unlock;
+	}
+
+	/* Program alarm registers */
+	regs[0] = bin2bcd(alarm_tm->tm_sec) & 0x7f;
+	regs[1] = bin2bcd(alarm_tm->tm_min) & 0x7f;
+	regs[2] = bin2bcd(alarm_tm->tm_hour) & 0x3f;
+	regs[3] = bin2bcd(alarm_tm->tm_mday) & 0x3f;
+
+	ret = regmap_bulk_write(data->regmap, ISL12057_REG_A1_SC, regs,
+				ISL12057_A1_SEC_LEN);
+	if (ret < 0) {
+		dev_err(dev, "%s: writing alarm section failed (%d)\n",
+			__func__, ret);
+		goto err_unlock;
+	}
+
+	/* Enable or disable alarm */
+	ret = _isl12057_rtc_update_alarm(dev, enable);
+
+err_unlock:
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
 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(struct device *dev, struct regmap *regmap)
 	return 0;
 }
 
+static int isl12057_rtc_alarm_irq_enable(struct device *dev,
+					 unsigned int enable)
+{
+	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(dev);
+	int ret = -ENOTTY;
+
+	if (rtc_data->irq)
+		ret = isl12057_rtc_update_alarm(dev, enable);
+
+	return ret;
+}
+
+static irqreturn_t isl12057_rtc_interrupt(int irq, void *data)
+{
+	struct i2c_client *client = data;
+	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(&client->dev);
+	struct rtc_device *rtc = rtc_data->rtc;
+	int ret, handled = IRQ_NONE;
+	unsigned int sr;
+
+	ret = regmap_read(rtc_data->regmap, ISL12057_REG_SR, &sr);
+	if (!ret && (sr & ISL12057_REG_SR_A1F)) {
+		dev_dbg(&client->dev, "RTC alarm!\n");
+
+		rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
+
+		/* Acknowledge and disable the alarm */
+		_isl12057_rtc_clear_alarm(&client->dev);
+		_isl12057_rtc_update_alarm(&client->dev, 0);
+
+		handled = IRQ_HANDLED;
+	}
+
+	return handled;
+}
+
 static const struct rtc_class_ops rtc_ops = {
-	.read_time = isl12057_rtc_read_time,
+	.read_time = _isl12057_rtc_read_time,
 	.set_time = isl12057_rtc_set_time,
+	.read_alarm = isl12057_rtc_read_alarm,
+	.set_alarm = isl12057_rtc_set_alarm,
+	.alarm_irq_enable = isl12057_rtc_alarm_irq_enable,
 };
 
 static struct regmap_config isl12057_rtc_regmap_config = {
@@ -277,7 +511,6 @@ static int isl12057_probe(struct i2c_client *client,
 {
 	struct device *dev = &client->dev;
 	struct isl12057_rtc_data *data;
-	struct rtc_device *rtc;
 	struct regmap *regmap;
 	int ret;
 
@@ -310,10 +543,79 @@ static int isl12057_probe(struct i2c_client *client,
 	data->regmap = regmap;
 	dev_set_drvdata(dev, data);
 
-	rtc = devm_rtc_device_register(dev, DRV_NAME, &rtc_ops, THIS_MODULE);
-	return PTR_ERR_OR_ZERO(rtc);
+	if (client->irq > 0) {
+		ret = devm_request_threaded_irq(dev, client->irq, NULL,
+						isl12057_rtc_interrupt,
+						IRQF_SHARED|IRQF_ONESHOT,
+						DRV_NAME, client);
+		if (!ret)
+			data->irq = client->irq;
+		else
+			dev_err(dev, "%s: irq %d unavailable (%d)\n", __func__,
+				client->irq, ret);
+	}
+
+	/*
+	 * 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);
+
+	data->rtc = devm_rtc_device_register(dev, DRV_NAME, &rtc_ops,
+					     THIS_MODULE);
+	ret = PTR_ERR_OR_ZERO(data->rtc);
+	if (ret) {
+		dev_err(dev, "%s: unable to register RTC device (%d)\n",
+			__func__, ret);
+		goto err;
+	}
+
+	/* We cannot support UIE mode if we do not have an IRQ line */
+	if (!data->irq)
+		data->rtc->uie_unsupported = 1;
+
+err:
+	return ret;
 }
 
+static int isl12057_remove(struct i2c_client *client)
+{
+	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(&client->dev);
+
+	if (rtc_data->irq > 0)
+		device_init_wakeup(&client->dev, false);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int isl12057_rtc_suspend(struct device *dev)
+{
+	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		return enable_irq_wake(rtc_data->irq);
+
+	return 0;
+}
+
+static int isl12057_rtc_resume(struct device *dev)
+{
+	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		return disable_irq_wake(rtc_data->irq);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(isl12057_rtc_pm_ops, isl12057_rtc_suspend,
+			 isl12057_rtc_resume);
+
 #ifdef CONFIG_OF
 static const struct of_device_id isl12057_dt_match[] = {
 	{ .compatible = "isl,isl12057" },  /* obsolete */
@@ -332,13 +634,15 @@ static struct i2c_driver isl12057_driver = {
 	.driver = {
 		.name = DRV_NAME,
 		.owner = THIS_MODULE,
+		.pm = &isl12057_rtc_pm_ops,
 		.of_match_table = of_match_ptr(isl12057_dt_match),
 	},
 	.probe	  = isl12057_probe,
+	.remove	  = isl12057_remove,
 	.id_table = isl12057_id,
 };
 module_i2c_driver(isl12057_driver);
 
 MODULE_AUTHOR("Arnaud EBALARD <arno@natisbad.org>");
-MODULE_DESCRIPTION("Intersil ISL12057 RTC driver");
+MODULE_DESCRIPTION("Intersil ISL12057 RTC/Alarm driver");
 MODULE_LICENSE("GPL");
-- 
2.1.1

Re: [PATCHv1 6/6] rtc: rtc-isl12057: add alarm support to Intersil ISL12057 RTC driver

[Uwe Kleine-König]

Hello,

finally I managed to test this series on my (unmodified) rn104.

For patch 1: Maybe point out that the issue with the century bit isn't
that critical, because this bit is not expected to be set before year 2100.

For patch 3: This patch adds a few dev_err calls that get later amended
in patch 5 to also include an error code. IMHO these should already be
added in patch 3. Patch 5 should only add it to the already existing
strings (if applicable).

For patch 4: Maybe
s/obsolete/for backwards compatibility, don't use in new code/.

Some further comments inline ...

On 11/15/2014 12:07 AM, Arnaud Ebalard wrote:
> The latter is the one found on current 3 kernel users of the chip
> for which support was initially developed (Netgear ReadyNAS 102,
> 104 and 2120 NAS). On those devices, the IRQ#2 pin of the chip is not
> connected to the SoC but to a PMIC. This allows setting an alarm,
> powering off the device and have it wake up when the alarm rings.
> To support that configuration the driver does the following:
> 
>  1. it has alarm_irq_enable() function returns -ENOTTY when no IRQ
>     is passed to the driver.
>  2. it marks the device as a wakeup source in all cases (whether an
>     IRQ is passed to the driver or not) to have 'wakealarm' sysfs
>     entry created.
This is not pretty, but I don't know if there is a nicer alternative.
Maybe this should only be done in the presence of a flag in the device
tree (say: can-wakeup-machine, a prefix would be nice, but "isil," feels
wrong).

> [...]
> FWIW, if someone is looking for a way to test alarm support on a system
> on which the chip IRQ line has the ability to boot the system (e.g.
> ReadyNAS 102, 104, etc):
> 
>     # echo 0 > /sys/class/rtc/rtc0/wakealarm
Why is this needed?

>     # echo `date '+%s' -d '+ 1 minutes'` > /sys/class/rtc/rtc0/wakealarm
>     # shutdown -h now
> 
> With the commandes above, after a minute, the system comes back to life.
s/commandes/commands/

> +	ret = regmap_update_bits(data->regmap, ISL12057_REG_INT,
> +				 ISL12057_REG_INT_A1IE,
> +				 enable ? ISL12057_REG_INT_A1IE : 0);
> +	if (ret)
> +		dev_err(dev, "%s: changing alarm interrupt flag failed (%d)\n",
> +			__func__, ret);
> +
> +	return ret;
> +}
Maybe point out in the commit log that the first alarm (of two) is used,
and the event is signaled on pin $Ididntlookitup.
(IIRC the 2nd alarm register set doesn't support seconds, but in return
has year and month field.)

> [...]
> +	/*
> +	 * 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.
Maybe add the machines you know of that have this setup to the comment.

> +	 */
> +	device_init_wakeup(dev, true);
> +
> +	data->rtc = devm_rtc_device_register(dev, DRV_NAME, &rtc_ops,
> +					     THIS_MODULE);
> +	ret = PTR_ERR_OR_ZERO(data->rtc);
> +	if (ret) {
> +		dev_err(dev, "%s: unable to register RTC device (%d)\n",
> +			__func__, ret);
> +		goto err;
> +	}
> +
> +	/* We cannot support UIE mode if we do not have an IRQ line */
> +	if (!data->irq)
> +		data->rtc->uie_unsupported = 1;
> +
> +err:
> +	return ret;
>  }
>  
> +static int isl12057_remove(struct i2c_client *client)
> +{
> +	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(&client->dev);
> +
> +	if (rtc_data->irq > 0)
> +		device_init_wakeup(&client->dev, false);
I didn't check, but I wonder it that could be/is done by the device
core already?

> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int isl12057_rtc_suspend(struct device *dev)
> +{
> +	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev))
> +		return enable_irq_wake(rtc_data->irq);
Does this need a check for data->irq?

> +
> +	return 0;
> +}
> +
> +static int isl12057_rtc_resume(struct device *dev)
> +{
> +	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev))
> +		return disable_irq_wake(rtc_data->irq);
ditto.

Thanks for your efforts to improve my NAS :-)
Uwe

Re: [PATCHv1 6/6] rtc: rtc-isl12057: add alarm support to Intersil ISL12057 RTC driver

[Arnaud Ebalard]

Hi Uwe,

Uwe Kleine-König <uwe@kleine-koenig.org> writes:

> Hello,
>
> finally I managed to test this series on my (unmodified) rn104.
>
> For patch 1: Maybe point out that the issue with the century bit isn't
> that critical, because this bit is not expected to be set before year 2100.
>
> For patch 3: This patch adds a few dev_err calls that get later amended
> in patch 5 to also include an error code. IMHO these should already be
> added in patch 3. Patch 5 should only add it to the already existing
> strings (if applicable).
>
> For patch 4: Maybe
> s/obsolete/for backwards compatibility, don't use in new code/.
>
> Some further comments inline ...

Thanks for the tests. I'll take a look at your comments this evening.

Cheers,

a+

Re: [PATCHv1 6/6] rtc: rtc-isl12057: add alarm support to Intersil ISL12057 RTC driver

[Arnaud Ebalard]

Hi Uwe,

Uwe Kleine-König <uwe@kleine-koenig.org> writes:

> finally I managed to test this series on my (unmodified) rn104.
>
> For patch 1: Maybe point out that the issue with the century bit isn't
> that critical, because this bit is not expected to be set before year
> 2100.

It has:

    This was tested by putting a device 100 years in the future (using a
    specific kernel due to the inability of userland tools such as date or
    hwclock to pass year 2038), rebooting on a kernel w/ this patch applied
    and verifying the device was still 100 years in the future.


> For patch 3: This patch adds a few dev_err calls that get later amended
> in patch 5 to also include an error code. IMHO these should already be
> added in patch 3. Patch 5 should only add it to the already existing
> strings (if applicable).

will do that next time.

> For patch 4: Maybe
> s/obsolete/for backwards compatibility, don't use in new code/.
>
>
> Some further comments inline ...
>
> On 11/15/2014 12:07 AM, Arnaud Ebalard wrote:
>> The latter is the one found on current 3 kernel users of the chip
>> for which support was initially developed (Netgear ReadyNAS 102,
>> 104 and 2120 NAS). On those devices, the IRQ#2 pin of the chip is not
>> connected to the SoC but to a PMIC. This allows setting an alarm,
>> powering off the device and have it wake up when the alarm rings.
>> To support that configuration the driver does the following:
>> 
>>  1. it has alarm_irq_enable() function returns -ENOTTY when no IRQ
>>     is passed to the driver.
>>  2. it marks the device as a wakeup source in all cases (whether an
>>     IRQ is passed to the driver or not) to have 'wakealarm' sysfs
>>     entry created.
> This is not pretty, but I don't know if there is a nicer alternative.
> Maybe this should only be done in the presence of a flag in the device
> tree (say: can-wakeup-machine, a prefix would be nice, but "isil," feels
> wrong).

I can prepare a v0 patch for a "can-wakeup-machine" property to mark the
device as a wakeup source when the IRQ is absent. Will fix the prefix
in a v1.


>> [...]
>> FWIW, if someone is looking for a way to test alarm support on a system
>> on which the chip IRQ line has the ability to boot the system (e.g.
>> ReadyNAS 102, 104, etc):
>> 
>>     # echo 0 > /sys/class/rtc/rtc0/wakealarm
> Why is this needed?

It disable the alarm. It's not required.


>>     # echo `date '+%s' -d '+ 1 minutes'` > /sys/class/rtc/rtc0/wakealarm
>>     # shutdown -h now
>> 
>> With the commandes above, after a minute, the system comes back to life.
> s/commandes/commands/

useless french letter.


>> +	ret = regmap_update_bits(data->regmap, ISL12057_REG_INT,
>> +				 ISL12057_REG_INT_A1IE,
>> +				 enable ? ISL12057_REG_INT_A1IE : 0);
>> +	if (ret)
>> +		dev_err(dev, "%s: changing alarm interrupt flag failed (%d)\n",
>> +			__func__, ret);
>> +
>> +	return ret;
>> +}
> Maybe point out in the commit log that the first alarm (of two) is used,
> and the event is signaled on pin $Ididntlookitup.
> (IIRC the 2nd alarm register set doesn't support seconds, but in return
> has year and month field.)

I can add that in the documentation.


>> [...]
>> +	/*
>> +	 * 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.
> Maybe add the machines you know of that have this setup to the
> comment.

ditto.


>> +	 */
>> +	device_init_wakeup(dev, true);
>> +
>> +	data->rtc = devm_rtc_device_register(dev, DRV_NAME, &rtc_ops,
>> +					     THIS_MODULE);
>> +	ret = PTR_ERR_OR_ZERO(data->rtc);
>> +	if (ret) {
>> +		dev_err(dev, "%s: unable to register RTC device (%d)\n",
>> +			__func__, ret);
>> +		goto err;
>> +	}
>> +
>> +	/* We cannot support UIE mode if we do not have an IRQ line */
>> +	if (!data->irq)
>> +		data->rtc->uie_unsupported = 1;
>> +
>> +err:
>> +	return ret;
>>  }
>>  
>> +static int isl12057_remove(struct i2c_client *client)
>> +{
>> +	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(&client->dev);
>> +
>> +	if (rtc_data->irq > 0)
>> +		device_init_wakeup(&client->dev, false);
> I didn't check, but I wonder it that could be/is done by the device
> core already?
>
>> +	return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int isl12057_rtc_suspend(struct device *dev)
>> +{
>> +	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(dev);
>> +
>> +	if (device_may_wakeup(dev))
>> +		return enable_irq_wake(rtc_data->irq);
> Does this need a check for data->irq?
>
>> +	return 0;
>> +}
>> +
>> +static int isl12057_rtc_resume(struct device *dev)
>> +{
>> +	struct isl12057_rtc_data *rtc_data = dev_get_drvdata(dev);
>> +
>> +	if (device_may_wakeup(dev))
>> +		return disable_irq_wake(rtc_data->irq);
> ditto.

Hum, I will take a look at those irq vs wakeup cpabilities when adding
support for "can-wakeup-machine" property to consolidate the behavior.

Cheers,

a+

Re: [PATCHv1 6/6] rtc: rtc-isl12057: add alarm support to Intersil ISL12057 RTC driver

[Uwe Kleine-König]

On December 1, 2014 9:11:21 PM CET, arno@natisbad.org wrote:
>Uwe Kleine-König <uwe@kleine-koenig.org> writes:
>> For patch 1: Maybe point out that the issue with the century bit
>isn't
>> that critical, because this bit is not expected to be set before year
>> 2100.
>
>It has:
>
>   This was tested by putting a device 100 years in the future (using a
> specific kernel due to the inability of userland tools such as date or
>hwclock to pass year 2038), rebooting on a kernel w/ this patch applied
>    and verifying the device was still 100 years in the future.
>
That describes your test, but it doesn't rule out that someone might use
the century bit to distinguish 19** and 20**.

Best regards
Uwe

Re: [PATCHv1 0/6] rtc: rtc-isl12057: fixes and alarm support

[Arnaud Ebalard]

Hi Jason and Mark,

Arnaud Ebalard <arno@natisbad.org> writes:

> This series includes six patches for Intersil ISL12057 driver.
>
>  - First patch is a fix for masking issues which dates back to driver
>    inclusion. Even if those issues are not critical per se, the patch is
>    a good candidate for stable down to 3.14.
>
>  - As suggested by Uwe, second patch adds proper support for century bit
>    provided by the driver. This will allow to use the chip until 2199
>    if we manage to pass 2038.
>
>  - Following another comment by Uwe, third patch corrects the handling
>    of oscillator failure bit.
>
>  - fourth patch fixes remaining places where 'isl' is used instead of the
>    expected NASDAQ symbol for Intersil (i.e. isil) after commit
>    7a6540ca856ae ("ARM: mvebu: Change vendor prefix for Intersil 
>    Corporation to isil"). 
>
>  - As suggested by Mark, fifth patch improves failure reports by providing
>    error code in dev_err() code.
>
>  - Sixth patch provides alarm support for Intersil ISL12057. This
>    support was not added when the driver was initially pushed in 3.14
>    kernel due to the inability to check interrupt support. After some
>    soldering, this tests have been performed. More time was also
>    spent on testing.

I wonder if you could take a look at this series I sent providing fixes
(patches #1-5) and alarm support (patch #6) for ISL12057 RTC Chip. As
all people do for RTC patches - due to lack of feedback from maintainer
- I am looking for someone to review the patches (Mark, patch 6 has a
sticker w/ regmap-inside ;-)) and then push those upstream if they are
ok (Jason, you handled that part for 70e123373c05: "rtc: Add support for
Intersil ISL12057 I2C RTC chip" in 2013 for the same reason).

Note that I addressed previous comments and feedback received from Uwe
and also spent time to do some soldering in order to support and test
the last patch for both my use case (Alarm IRQ signal not routed to the
SoC on ReadyNAS RN102, RN104 and RN2120) and the common case.

Do not hesitate to ask if you want me to resend!

Cheers,

a+

Thread: http://thread.gmane.org/gmane.linux.documentation/28237
Patch 1/6: https://patchwork.kernel.org/patch/5310101/raw/
Patch 2/6: https://patchwork.kernel.org/patch/5310111/raw/
Patch 3/6: https://patchwork.kernel.org/patch/5310121/raw/
Patch 4/6: https://patchwork.kernel.org/patch/5310131/raw/
Patch 5/6: https://patchwork.kernel.org/patch/5310141/raw/
Patch 6/6: https://patchwork.kernel.org/patch/5310151/raw/

Re: [PATCHv1 0/6] rtc: rtc-isl12057: fixes and alarm support

[Uwe Kleine-König]

Hello Arnaud,

On 11/26/2014 07:35 PM, Arnaud Ebalard wrote:
> I wonder if you could take a look at this series I sent providing fixes
> (patches #1-5) and alarm support (patch #6) for ISL12057 RTC Chip. As
> all people do for RTC patches - due to lack of feedback from maintainer
> - I am looking for someone to review the patches (Mark, patch 6 has a
> sticker w/ regmap-inside ;-)) and then push those upstream if they are
> ok (Jason, you handled that part for 70e123373c05: "rtc: Add support for
> Intersil ISL12057 I2C RTC chip" in 2013 for the same reason).
> 
> Note that I addressed previous comments and feedback received from Uwe
> and also spent time to do some soldering in order to support and test
> the last patch for both my use case (Alarm IRQ signal not routed to the
> SoC on ReadyNAS RN102, RN104 and RN2120) and the common case.
> 
> Do not hesitate to ask if you want me to resend!
akpm already applied the series, doesn't he?

I'm about to test the patches, just fighting with my NAS to boot my
custom kernel.

Apart from that patches 1 - 5 can have my Ack. Didn't look deep enough
into patch 6 yet. @akpm: No need to hurry adding them, I hope to be
able to also provide Tested-by:-tags later today.

Best regards
Uwe

Re: [PATCHv1 0/6] rtc: rtc-isl12057: fixes and alarm support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 164 bytes --]

On Wed, Nov 26, 2014 at 07:48:54PM +0100, Uwe Kleine-König wrote:

> akpm already applied the series, doesn't he?

Yeah, I didn't look since they got applied.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCHv1 0/6] rtc: rtc-isl12057: fixes and alarm support

[Arnaud Ebalard]

Hi,

Uwe Kleine-König <uwe@kleine-koenig.org> writes:

> On 11/26/2014 07:35 PM, Arnaud Ebalard wrote:
>> I wonder if you could take a look at this series I sent providing fixes
>> (patches #1-5) and alarm support (patch #6) for ISL12057 RTC Chip. As
>> all people do for RTC patches - due to lack of feedback from maintainer
>> - I am looking for someone to review the patches (Mark, patch 6 has a
>> sticker w/ regmap-inside ;-)) and then push those upstream if they are
>> ok (Jason, you handled that part for 70e123373c05: "rtc: Add support for
>> Intersil ISL12057 I2C RTC chip" in 2013 for the same reason).
>> 
>> Note that I addressed previous comments and feedback received from Uwe
>> and also spent time to do some soldering in order to support and test
>> the last patch for both my use case (Alarm IRQ signal not routed to the
>> SoC on ReadyNAS RN102, RN104 and RN2120) and the common case.
>> 
>> Do not hesitate to ask if you want me to resend!
> akpm already applied the series, doesn't he?

I am not really familiar w/ -mm tree workflow. It was not clear to me
whether it was sitting in -mm tree waiting for reviewed-by and
acked-by.

> I'm about to test the patches, just fighting with my NAS to boot my
> custom kernel.
>
> Apart from that patches 1 - 5 can have my Ack. Didn't look deep enough
> into patch 6 yet. @akpm: No need to hurry adding them, I hope to be
> able to also provide Tested-by:-tags later today.

I'll be happy to handle comments and possible requests for additional
changes if needed.

Cheers,

a+

ps: anyway, thanks Andrew!

Re: [PATCHv1 0/6] rtc: rtc-isl12057: fixes and alarm support

[Andrew Morton]

On Wed, 26 Nov 2014 20:28:53 +0100 arno@natisbad.org (Arnaud Ebalard) wrote:

> Hi,
> 
> Uwe Kleine-K__nig <uwe@kleine-koenig.org> writes:
> 
> > On 11/26/2014 07:35 PM, Arnaud Ebalard wrote:
> >> I wonder if you could take a look at this series I sent providing fixes
> >> (patches #1-5) and alarm support (patch #6) for ISL12057 RTC Chip. As
> >> all people do for RTC patches - due to lack of feedback from maintainer
> >> - I am looking for someone to review the patches (Mark, patch 6 has a
> >> sticker w/ regmap-inside ;-)) and then push those upstream if they are
> >> ok (Jason, you handled that part for 70e123373c05: "rtc: Add support for
> >> Intersil ISL12057 I2C RTC chip" in 2013 for the same reason).
> >> 
> >> Note that I addressed previous comments and feedback received from Uwe
> >> and also spent time to do some soldering in order to support and test
> >> the last patch for both my use case (Alarm IRQ signal not routed to the
> >> SoC on ReadyNAS RN102, RN104 and RN2120) and the common case.
> >> 
> >> Do not hesitate to ask if you want me to resend!
> > akpm already applied the series, doesn't he?
> 
> I am not really familiar w/ -mm tree workflow. It was not clear to me
> whether it was sitting in -mm tree waiting for reviewed-by and
> acked-by.

-mm is a collection of several hundred trees (most of which are empty
at a particular time).  The workflow depends on which tree we're
talking about!

For RTC patches I attempt to review them myself but I'm by no means an
expert on RTC, so I'm careful to cc various people who I think might
help with review and test.  Often people do this.  Generally when they
don't, the patches are sufficiently obvious for me to proceed under my
own steam.  If something breaks (this is rare), the number of people
who are affected is usually quite small and they tend to be system
developers who can fix it up for us anyway.

Re: [PATCHv1 0/6] rtc: rtc-isl12057: fixes and alarm support

[Arnaud Ebalard]

Hi Andrew,

Andrew Morton <akpm@linux-foundation.org> writes:

>> > akpm already applied the series, doesn't he?
>> 
>> I am not really familiar w/ -mm tree workflow. It was not clear to me
>> whether it was sitting in -mm tree waiting for reviewed-by and
>> acked-by.
>
> -mm is a collection of several hundred trees (most of which are empty
> at a particular time).  The workflow depends on which tree we're
> talking about!
>
> For RTC patches I attempt to review them myself but I'm by no means an
> expert on RTC, so I'm careful to cc various people who I think might
> help with review and test.  Often people do this.  Generally when they
> don't, the patches are sufficiently obvious for me to proceed under my
> own steam.  If something breaks (this is rare), the number of people
> who are affected is usually quite small and they tend to be system
> developers who can fix it up for us anyway.

Thanks for the explanation.

Cheers,

a+

Re: [PATCHv1 0/6] rtc: rtc-isl12057: fixes and alarm support

[Andrew Morton]

On Wed, 26 Nov 2014 19:48:54 +0100 Uwe Kleine-K__nig <uwe@kleine-koenig.org> wrote:

> Hello Arnaud,
> 
> On 11/26/2014 07:35 PM, Arnaud Ebalard wrote:
> > I wonder if you could take a look at this series I sent providing fixes
> > (patches #1-5) and alarm support (patch #6) for ISL12057 RTC Chip. As
> > all people do for RTC patches - due to lack of feedback from maintainer
> > - I am looking for someone to review the patches (Mark, patch 6 has a
> > sticker w/ regmap-inside ;-)) and then push those upstream if they are
> > ok (Jason, you handled that part for 70e123373c05: "rtc: Add support for
> > Intersil ISL12057 I2C RTC chip" in 2013 for the same reason).
> > 
> > Note that I addressed previous comments and feedback received from Uwe
> > and also spent time to do some soldering in order to support and test
> > the last patch for both my use case (Alarm IRQ signal not routed to the
> > SoC on ReadyNAS RN102, RN104 and RN2120) and the common case.
> > 
> > Do not hesitate to ask if you want me to resend!
> akpm already applied the series, doesn't he?
> 
> I'm about to test the patches, just fighting with my NAS to boot my
> custom kernel.
> 
> Apart from that patches 1 - 5 can have my Ack. Didn't look deep enough
> into patch 6 yet. @akpm: No need to hurry adding them, I hope to be
> able to also provide Tested-by:-tags later today.

Thanks, it really helps.