Add support for Intersil ISL12057 I2C RTC chip
PATCH 1/1 Add support for Intersil ISL12057 I2C RTC chip
Intersil ISL12057 I2C RTC chip also supports two alarms. This patch only adds support for basic RTC functionalities (i.e. getting and setting time). Tests have been performed on NETGEAR ReadyNAS 102 w/ startup/shutdown scripts, hwclock, ntpdate and openntpd.
---
Hi,
In order to get at least some initial support for the chip available in mainline kernel (3.14 will have three users waiting for it: ReadyNAS 102, 104 and 2120), I decided to temporarily remove the alarm and IRQ related code I pushed earlier (see [1] below). I will submit a patch later to add back Alarm/IRQ support once I have understood how to correctly handle my use case described via existing RTC logic.
Anyway, comments are welcome on the patch!
Changes since previous v0:
- removed alarm and IRQ related code
- Switched to isl for intersil (no existing driver has any reference to isil, even though this would be the recommended choice)
- Added intersil info in vendor-prefixes.txt file
- Added entry for ISL 12057 in I2C trivial-devices.txt file
- Added mutex protection for non atomic read/write
Now, regarding later submission of Alarm/IRQ functionality, here is the use case: on all 3 NETGEAR NAS above (102, 104 and RN2120), the Alarm interrupt pins of the ISL 12057 are not connected to the SoC, i.e. Linux kernel never gets warned via an interrupt when the alarm goes off. The reason is that the alarm interrupt pin is connected to some regulator which will power up the (previously off) device when the alarm goes off. It raises some questions for which I am looking for some supports from RTC maintainer and people familiar w/ RTC framework:
- Are there no previous example of such need from existing platform?
- How can this use case be supported? Tested?
- in usual context, what is the purpose of having an IRQ handler for such an alarm event: just acknowledge it and remove the alarm? Or are there other reasons?
As a side note, I want to repeat that previous code (the one I dropped from current patch) worked fine to get alarm installed (i.e. once the device powered off, it woke up as expected using the alarm). I was just unable to test the interrupt handling part, for the reason described above.
Cheers,
a+
[1]: http://thread.gmane.org/gmane.linux.drivers.devicetree/46581
.../devicetree/bindings/i2c/trivial-devices.txt | 1 + .../devicetree/bindings/vendor-prefixes.txt | 1 + drivers/rtc/Kconfig | 9 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-isl12057.c | 397 +++++++++++++++++++++ 5 files changed, 409 insertions(+) create mode 100644 drivers/rtc/rtc-isl12057.c
Documentation/devicetree/bindings/i2c/trivial-devices.txt +1 -0
@@ -37,6 +37,7 @@ fsl,mpr121 MPR121: Proximity Capacitive Touch Sensor Controller fsl,sgtl5000 SGTL5000: Ultra Low-Power Audio Codec 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 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
Documentation/devicetree/bindings/vendor-prefixes.txt +1 -0
@@ -38,6 +38,7 @@ ibm International Business Machines (IBM) idt Integrated Device Technologies, Inc. img Imagination Technologies Ltd. intercontrol Inter Control Group +isl Intersil linux Linux-specific binding lsi LSI Corp. (LSI Logic) marvell Marvell Technology Group Ltd.
drivers/rtc/Kconfig +9 -0
@@ -304,6 +304,15 @@ config RTC_DRV_ISL12022 This driver can also be built as a module. If so, the module will be called rtc-isl12022. +config RTC_DRV_ISL12057 + tristate "Intersil ISL12057" + help + If you say yes here you get support for the Intersil ISL12057 + I2C RTC chip. Alarm functionality is not supported. + + This driver can also be built as a module. If so, the module + will be called rtc-isl12057. + config RTC_DRV_X1205 tristate "Xicor/Intersil X1205" help
drivers/rtc/Makefile +1 -0
@@ -58,6 +58,7 @@ obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += rtc-hid-sensor-time.o obj-$(CONFIG_RTC_DRV_IMXDI) += rtc-imxdi.o obj-$(CONFIG_RTC_DRV_ISL1208) += rtc-isl1208.o obj-$(CONFIG_RTC_DRV_ISL12022) += rtc-isl12022.o +obj-$(CONFIG_RTC_DRV_ISL12057) += rtc-isl12057.o obj-$(CONFIG_RTC_DRV_JZ4740) += rtc-jz4740.o obj-$(CONFIG_RTC_DRV_LP8788) += rtc-lp8788.o obj-$(CONFIG_RTC_DRV_LPC32XX) += rtc-lpc32xx.o
drivers/rtc/rtc-isl12057.c new file +397 -0
397 lines not on this page · raw message
Signature · 9 lines
1.8.4.rc3 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
drivers/rtc/Kconfig
Quoted 90 lines · Arnaud Ebalard
Intersil ISL12057 I2C RTC chip also supports two alarms. This patch only adds support for basic RTC functionalities (i.e. getting and setting time). Tests have been performed on NETGEAR ReadyNAS 102 w/ startup/shutdown scripts, hwclock, ntpdate and openntpd.
Signed-off-by Arnaud Ebalard <[email protected]>---
Hi,In order to get at least some initial support for the chip available in mainline kernel (3.14 will have three users waiting for it: ReadyNAS 102, 104 and 2120), I decided to temporarily remove the alarm and IRQ related code I pushed earlier (see [1] below). I will submit a patch later to add back Alarm/IRQ support once I have understood how to correctly handle my use case described via existing RTC logic.
Anyway, comments are welcome on the patch!
Changes since previous v0:
- removed alarm and IRQ related code
- Switched to isl for intersil (no existing driver has any reference to isil, even though this would be the recommended choice)
- Added intersil info in vendor-prefixes.txt file
- Added entry for ISL 12057 in I2C trivial-devices.txt file
- Added mutex protection for non atomic read/writeNow, regarding later submission of Alarm/IRQ functionality, here is the use case: on all 3 NETGEAR NAS above (102, 104 and RN2120), the Alarm interrupt pins of the ISL 12057 are not connected to the SoC, i.e. Linux kernel never gets warned via an interrupt when the alarm goes off. The reason is that the alarm interrupt pin is connected to some regulator which will power up the (previously off) device when the alarm goes off. It raises some questions for which I am looking for some supports from RTC maintainer and people familiar w/ RTC framework:
- Are there no previous example of such need from existing platform?
- How can this use case be supported? Tested?
- in usual context, what is the purpose of having an IRQ handler for such an alarm event: just acknowledge it and remove the alarm? Or are there other reasons?As a side note, I want to repeat that previous code (the one I dropped from current patch) worked fine to get alarm installed (i.e. once the device powered off, it woke up as expected using the alarm). I was just unable to test the interrupt handling part, for the reason described above.
Cheers,
a+
[1]: http://thread.gmane.org/gmane.linux.drivers.devicetree/46581
.../devicetree/bindings/i2c/trivial-devices.txt | 1 + .../devicetree/bindings/vendor-prefixes.txt | 1 + drivers/rtc/Kconfig | 9 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-isl12057.c | 397 +++++++++++++++++++++ 5 files changed, 409 insertions(+) create mode 100644 drivers/rtc/rtc-isl12057.cDocumentation/devicetree/bindings/i2c/trivial-devices.txt +1
-0@@ -37,6 +37,7 @@ fsl,mpr121 MPR121: Proximity Capacitive Touch Sensor Controller fsl,sgtl5000 SGTL5000: Ultra Low-Power Audio Codec 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 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 InterfaceDocumentation/devicetree/bindings/vendor-prefixes.txt +1
-0@@ -38,6 +38,7 @@ ibm International Business Machines (IBM) idt Integrated Device Technologies, Inc. img Imagination Technologies Ltd. intercontrol Inter Control Group +isl Intersil linux Linux-specific binding lsi LSI Corp. (LSI Logic) marvell Marvell Technology Group Ltd.drivers/rtc/Kconfig +0
-0
The binding additions look fine to me.
[...]
Quoted 15 lines
drivers/rtc/Kconfig +15
-0+static int isl12057_i2c_read_regs(struct i2c_client *client, u8 reg, u8 buf[], + unsigned len) +{ + struct i2c_msg msgs[2] = { + { + .addr = client->addr, + .flags = 0, + .len = sizeof(u8), + .buf = buf + }, + { + .addr = client->addr, + .flags = I2C_M_RD, + .len = len, + .buf = buf
drivers/rtc/Kconfig
Quoted 10 lines
drivers/rtc/Kconfig +10
-0+ } + }; + int ret; + + BUG_ON(reg > ISL12057_REG_SR); + BUG_ON(reg + len > ISL12057_REG_SR + 1); + + ret = i2c_transfer(client->adapter, msgs, 2); + if (ret != 2) + return ret;
This might return 1.
Given that in isl12057_rtc_read_time you check if the return value is negative (also indirectly via isl12057_i2c_validate_client), I think it would make sense to always either return a negative error code or 0.
Thanks,
Mark.
A message that is not in the archive (sent off-list or not yet received): 20131118152012.GA3759-NuALmloUBlrZROr8t4l/[email protected]
Hi Mark,
Quoted 28 lines · Mark Rutland
Quoted 15 lines
drivers/rtc/rtc-isl12057.c new file +15
-0+static int isl12057_i2c_read_regs (struct i2c_client *client,u8 reg,u8 buf[], +unsigned len) +{ +struct i2c_msg msgs[2 ] = { + { + .addr = client->addr, + .flags =0 , + .len =sizeof (u8), + .buf = buf + }, + { + .addr = client->addr, + .flags =I2C_M_RD , + .len = len, + .buf = bufdrivers/rtc/rtc-isl12057.c line 118
Quoted 10 lines
drivers/rtc/rtc-isl12057.c new file +10
-0+ } + }; +int ret; + +BUG_ON (reg >ISL12057_REG_SR ); +BUG_ON (reg + len >ISL12057_REG_SR +1 ); + + ret =i2c_transfer (client->adapter, msgs,2 ); +if (ret !=2 ) +return ret;This might return 1.
Quoted 3 lines
Given that in
isl12057_rtc_read_timeyou check if the return value is negative (also indirectly viaisl12057_i2c_validate_client), I think it would make sense to always either return a negative error code or 0.
Good catch. Will fix that and spend some more time on return paths, and then send a v2.
Cheers,
a+
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html