Add support for Intersil ISL12057 I2C RTC/Alarm chip
PATCH RESEND 1/1 Add support for Intersil ISL12057 I2C RTC/Alarm chip
Hi,
Netgear ReadyNAS 102 (Armada 370-base) NAS includes an Intersil ISL12057 I2C RTC/Alarm chip. As part of an effort to get full support for the NAS in mainline kernel, I wrote a driver for the chip based on ISL1208 one developed by Hebert Valerio Riedel. The patch below is an updated resend of the one I submitted on August 3 (i.e. holidays), but got no reply for. See
https://groups.google.com/forum/#!topic/rtc-linux/ypqMloOezj0
If possible, I would like to be able to get it mainline in 3.13, and pushed required DT bindings for ReadyNAS 102 to avoid the NAS to wake up in the 70s.
Setting and retrieving time works as expected at startup and shutdown. Also, alarm bit is set in status register when the alarm is triggered.
Some things worth mentioning:
- on my platform, the Alarm IRQ is not connected to the SoC but to a power chip, so I cannot test the alarm IRQ handling code of the patch. I'd be happy to get feedback if someone has the same hardware.
- nonetheless, when the alarm is set and the NAS is powered off, it gets woken up, i.e. the alarm setting code does work.
- trying to interact w/ the clock using userland tools (Debian jessie) makes me wonder what I missed:
root@humble:/sys/class/rtc/rtc0# hwclock -r hwclock: select() to /dev/rtc0 to wait for clock tick timed out: Success
root@humble:/tmp# gcc rtctest.c root@humble:/tmp# ./a.out
RTC Driver Test Example.
Counting 5 update (1/sec) interrupts from reading /dev/rtc0:
[ nothing happens ]
I tried and find some documentation on the topic to understand where the issue comes from but found nothing obvious.
I intend to provide some documentation in next round and fix reported issues. Additionally, I wonder why ISL1208 code has no specific mutex protection for access and changes. I also followed that path in attached driver but I guess this is something that should be improved.
Comments welcome,
Cheers,
a+
--- drivers/rtc/Kconfig | 9 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-isl12057.c | 641 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 651 insertions(+) create mode 100644 drivers/rtc/rtc-isl12057.c
drivers/rtc/Kconfig +9 -0
@@ -294,6 +294,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/Calendar chip. + + 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
@@ -57,6 +57,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 +641 -0
641 lines not on this page · raw message
Signature · 6 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
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 33 lines · Arnaud Ebalard
Hi,
Netgear ReadyNAS 102 (Armada 370-base) NAS includes an Intersil ISL12057 I2C RTC/Alarm chip. As part of an effort to get full support for the NAS in mainline kernel, I wrote a driver for the chip based on ISL1208 one developed by Hebert Valerio Riedel. The patch below is an updated resend of the one I submitted on August 3 (i.e. holidays), but got no reply for. See
https://groups.google.com/forum/#!topic/rtc-linux/ypqMloOezj0
If possible, I would like to be able to get it mainline in 3.13, and pushed required DT bindings for ReadyNAS 102 to avoid the NAS to wake up in the 70s.
Setting and retrieving time works as expected at startup and shutdown. Also, alarm bit is set in status register when the alarm is triggered.
Some things worth mentioning:
- on my platform, the Alarm IRQ is not connected to the SoC but to a power chip, so I cannot test the alarm IRQ handling code of the patch. I'd be happy to get feedback if someone has the same hardware.
- nonetheless, when the alarm is set and the NAS is powered off, it gets woken up, i.e. the alarm setting code does work.
- trying to interact w/ the clock using userland tools (Debian jessie) makes me wonder what I missed:root@humble:/sys/class/rtc/rtc0# hwclock -r hwclock: select() to /dev/rtc0 to wait for clock tick timed out: Successroot@humble:/tmp# gcc rtctest.c root@humble:/tmp# ./a.out
Quoted 8 lines
RTC Driver Test Example.
Counting 5 update (1/sec) interrupts from reading
/dev/rtc0:[ nothing happens ]
I tried and find some documentation on the topic to understand where the issue comes from but found nothing obvious.
Forgive me if I'm mistaken, but you mentioned above that interrupts aren't wired up on your platform, and judging by the output the test program seems to be relying on them.
Quoted 5 lines
I intend to provide some documentation in next round and fix reported issues. Additionally, I wonder why ISL1208 code has no specific mutex protection for access and changes. I also followed that path in attached driver but I guess this is something that should be improved.
The devicetree binding must go in when the driver is added, rather than later.
Given the above known issue, I think that should be fixed up before the driver gets merged --
If this driver has known issues then should those not be fixed before merging it?
Quoted 14 lines
Comments welcome,
Cheers,
a+
Signed-off-by Arnaud Ebalard <[email protected]>--- drivers/rtc/Kconfig | 9 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-isl12057.c | 641 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 651 insertions(+) create mode 100644 drivers/rtc/rtc-isl12057.c
[...]
drivers/rtc/rtc-isl12057.c line 617
Quoted 6 lines
drivers/rtc/rtc-isl12057.c new file +6
-0+#ifdef CONFIG_OF +static struct of_device_id isl12057_dt_match[] = { + { .compatible ="isil,isl12057" }, + { }, +}; +#endif
It looks like this is a simple device with only an interrupt (no clocks, regulators, other elements), so that can go under Documentation/devicetree/bindings/i2c/trivial-devices.txt.
Also, the "isil" vendor prefix doesn't seem to be documented. Could you please create a patch adding it to Documentation/devicetree/bindings/vendor-prefixes.txt?
Cheers,
Mark.
--
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
Hi,
Quoted 38 lines · Mark Rutland
Quoted 37 lines · Arnaud Ebalard
Hi,
Netgear ReadyNAS 102 (Armada 370-base) NAS includes an Intersil ISL12057 I2C RTC/Alarm chip. As part of an effort to get full support for the NAS in mainline kernel, I wrote a driver for the chip based on ISL1208 one developed by Hebert Valerio Riedel. The patch below is an updated resend of the one I submitted on August 3 (i.e. holidays), but got no reply for. See
https://groups.google.com/forum/#!topic/rtc-linux/ypqMloOezj0
If possible, I would like to be able to get it mainline in 3.13, and pushed required DT bindings for ReadyNAS 102 to avoid the NAS to wake up in the 70s.
Setting and retrieving time works as expected at startup and shutdown. Also, alarm bit is set in status register when the alarm is triggered.
Some things worth mentioning:
- on my platform, the Alarm IRQ is not connected to the SoC but to a power chip, so I cannot test the alarm IRQ handling code of the patch. I'd be happy to get feedback if someone has the same hardware.
- nonetheless, when the alarm is set and the NAS is powered off, it gets woken up, i.e. the alarm setting code does work.
- trying to interact w/ the clock using userland tools (Debian jessie) makes me wonder what I missed:root@humble:/sys/class/rtc/rtc0# hwclock -r hwclock: select() to /dev/rtc0 to wait for clock tick timed out: Successroot@humble:/tmp# gcc rtctest.c root@humble:/tmp# ./a.outRTC Driver Test Example.
Counting 5 update (1/sec) interrupts from reading
/dev/rtc0:
Quoted 8 lines
Quoted 4 lines
[ nothing happens ]
I tried and find some documentation on the topic to understand where the issue comes from but found nothing obvious.
Forgive me if I'm mistaken, but you mentioned above that interrupts aren't wired up on your platform, and judging by the output the test program seems to be relying on them.
This is correct. I spent some more time digging about what I should do an it's not very clear to me. I found some informative elements about IRQ handling and RTC in
commit c9f5c7e7a8 rtc: rtc-spear: Provide flag for no support of UIE mode commit 4a649903f9 rtc: Provide flag for rtc devices that don't support UIE
So I modified the driver to have uie_unsupported set to 1 for testing. This makes hwclock happy but does not please rtctest.c, because commit 4a649903f9 modified the interface to have -EINVAL returned.
root@humble:/tmp# hwclock -r Sun 06 Oct 2013 09:38:12 PM CEST -0.107813 seconds root@humble:/tmp# ./a.out
RTC Driver Test Example.
RTC_UIE_ON ioctl: Invalid argument
Even if I modify the code to pass the UIE related test (i.e. s/ENOTTY/EINVAL/) then I get stuck on RTC_AIE_ON test.
Anyway, to come back to my initial issue, I thinks my problem is that the ReadyNAS 102 has the ISL 12057 Alarm IRQ pin connected to a power chip instead of the SoC, i.e. there is no way for the SoC to get interrupts from the chips and the RTC interface does not seem (at least to me) to provide a trivial whay to handle that. As I am unable to understand what I should do and not do from the documentation, I am basically asking RTC experts for guidelines on what to do.
One solution^Wworkaround would be for me to completely remove the alarm related code (at least initially) and provide a basic driver that only support the clock function of the chip. Alarm support could then be added later by someone with an IRQ pin routed to its SoC.
Another possible path could be to provide some DT (i.e. OF) knob in order to make ISL 12057 code (or possibly generic RTC code) aware that alarm IRQ are indeed supported by the chip but not routed to the processor itself. Regarding the IRQ handling code, care should be taken in that case to review it as I cannot test it myself.
Quoted 7 lines
Quoted 4 lines
I intend to provide some documentation in next round and fix reported issues. Additionally, I wonder why ISL1208 code has no specific mutex protection for access and changes. I also followed that path in attached driver but I guess this is something that should be improved.
The devicetree binding must go in when the driver is added, rather than later.
As soon as I know what should be fixed I will write some documentation if needed. As you write below, if the driver remains as is (trivial interface), it will not be needed. On the contrary, if some DT knob is needed, I will provide documentation once the code in a next round of the patch set.
Quoted 4 lines
Given the above known issue, I think that should be fixed up before the driver gets merged --
If this driver has known issues then should those not be fixed before merging it?
I don't think the driver has issues per se. All the problems come from the fact that the platform I have does not have the chip alarm interrupt line routed to the SoC:
- someone with an ISL 12057 with a routed alarm interrupt pin could test the IRQ-related code of the driver; otherwise, that code - based on ISL 1208 one - should be carefully reviewed.
- some direction is needed about RTC interface in order to be able to express the fact that the chip supports setting an alarm which can trigger an interrupt at a given time but that this interrupt is for someone else than the SoC.
Quoted 10 lines
drivers/rtc/rtc-isl12057.c line 617
Quoted 6 lines
drivers/rtc/rtc-isl12057.c new file +6
-0+#ifdef CONFIG_OF +static struct of_device_id isl12057_dt_match[] = { + { .compatible ="isil,isl12057" }, + { }, +}; +#endif It looks like this is a simple device with only an interrupt (no clocks, regulators, other elements), so that can go under
Documentation/devicetree/bindings/i2c/trivial-devices.txt.
Will do that if the interface remains as is.
Quoted 3 lines
Also, the "isil" vendor prefix doesn't seem to be documented. Could you please create a patch adding it to
Documentation/devicetree/bindings/vendor-prefixes.txt?
will do. I went for "isil" because it is the stock ticker symbol for Intersil (FWIW, https://lkml.org/lkml/2013/7/11/170).
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
A message that is not in the archive (sent off-list or not yet received): 20131002111423.GO22259-NuALmloUBlrZROr8t4l/[email protected]
Hi,
Quoted 38 lines · Mark Rutland
Quoted 37 lines · Arnaud Ebalard
Hi,
Netgear ReadyNAS 102 (Armada 370-base) NAS includes an Intersil ISL12057 I2C RTC/Alarm chip. As part of an effort to get full support for the NAS in mainline kernel, I wrote a driver for the chip based on ISL1208 one developed by Hebert Valerio Riedel. The patch below is an updated resend of the one I submitted on August 3 (i.e. holidays), but got no reply for. See
https://groups.google.com/forum/#!topic/rtc-linux/ypqMloOezj0
If possible, I would like to be able to get it mainline in 3.13, and pushed required DT bindings for ReadyNAS 102 to avoid the NAS to wake up in the 70s.
Setting and retrieving time works as expected at startup and shutdown. Also, alarm bit is set in status register when the alarm is triggered.
Some things worth mentioning:
- on my platform, the Alarm IRQ is not connected to the SoC but to a power chip, so I cannot test the alarm IRQ handling code of the patch. I'd be happy to get feedback if someone has the same hardware.
- nonetheless, when the alarm is set and the NAS is powered off, it gets woken up, i.e. the alarm setting code does work.
- trying to interact w/ the clock using userland tools (Debian jessie) makes me wonder what I missed:root@humble:/sys/class/rtc/rtc0# hwclock -r hwclock: select() to /dev/rtc0 to wait for clock tick timed out: Successroot@humble:/tmp# gcc rtctest.c root@humble:/tmp# ./a.outRTC Driver Test Example.
Counting 5 update (1/sec) interrupts from reading
/dev/rtc0:
Quoted 8 lines
Quoted 4 lines
[ nothing happens ]
I tried and find some documentation on the topic to understand where the issue comes from but found nothing obvious.
Forgive me if I'm mistaken, but you mentioned above that interrupts aren't wired up on your platform, and judging by the output the test program seems to be relying on them.
This is correct. I spent some more time digging about what I should do an it's not very clear to me. I found some informative elements about IRQ handling and RTC in
commit c9f5c7e7a8 rtc: rtc-spear: Provide flag for no support of UIE mode commit 4a649903f9 rtc: Provide flag for rtc devices that don't support UIE
So I modified the driver to have uie_unsupported set to 1 for testing. This makes hwclock happy but does not please rtctest.c, because commit 4a649903f9 modified the interface to have -EINVAL returned.
root@humble:/tmp# hwclock -r Sun 06 Oct 2013 09:38:12 PM CEST -0.107813 seconds root@humble:/tmp# ./a.out
RTC Driver Test Example.
RTC_UIE_ON ioctl: Invalid argument
Even if I modify the code to pass the UIE related test (i.e. s/ENOTTY/EINVAL/) then I get stuck on RTC_AIE_ON test.
Anyway, to come back to my initial issue, I thinks my problem is that the ReadyNAS 102 has the ISL 12057 Alarm IRQ pin connected to a power chip instead of the SoC, i.e. there is no way for the SoC to get interrupts from the chips and the RTC interface does not seem (at least to me) to provide a trivial whay to handle that. As I am unable to understand what I should do and not do from the documentation, I am basically asking RTC experts for guidelines on what to do.
One solution^Wworkaround would be for me to completely remove the alarm related code (at least initially) and provide a basic driver that only support the clock function of the chip. Alarm support could then be added later by someone with an IRQ pin routed to its SoC.
Another possible path could be to provide some DT (i.e. OF) knob in order to make ISL 12057 code (or possibly generic RTC code) aware that alarm IRQ are indeed supported by the chip but not routed to the processor itself. Regarding the IRQ handling code, care should be taken in that case to review it as I cannot test it myself.
Quoted 7 lines
Quoted 4 lines
I intend to provide some documentation in next round and fix reported issues. Additionally, I wonder why ISL1208 code has no specific mutex protection for access and changes. I also followed that path in attached driver but I guess this is something that should be improved.
The devicetree binding must go in when the driver is added, rather than later.
As soon as I know what should be fixed I will write some documentation if needed. As you write below, if the driver remains as is (trivial interface), it will not be needed. On the contrary, if some DT knob is needed, I will provide documentation once the code in a next round of the patch set.
Quoted 4 lines
Given the above known issue, I think that should be fixed up before the driver gets merged --
If this driver has known issues then should those not be fixed before merging it?
I don't think the driver has issues per se. All the problems come from the fact that the platform I have does not have the chip alarm interrupt line routed to the SoC:
- someone with an ISL 12057 with a routed alarm interrupt pin could test the IRQ-related code of the driver; otherwise, that code - based on ISL 1208 one - should be carefully reviewed.
- some direction is needed about RTC interface in order to be able to express the fact that the chip supports setting an alarm which can trigger an interrupt at a given time but that this interrupt is for someone else than the SoC.
Quoted 10 lines
drivers/rtc/rtc-isl12057.c line 617
Quoted 6 lines
drivers/rtc/rtc-isl12057.c new file +6
-0+#ifdef CONFIG_OF +static struct of_device_id isl12057_dt_match[] = { + { .compatible ="isil,isl12057" }, + { }, +}; +#endif It looks like this is a simple device with only an interrupt (no clocks, regulators, other elements), so that can go under
Documentation/devicetree/bindings/i2c/trivial-devices.txt.
Will do that if the interface remains as is.
Quoted 3 lines
Also, the "isil" vendor prefix doesn't seem to be documented. Could you please create a patch adding it to
Documentation/devicetree/bindings/vendor-prefixes.txt?
will do. I went for "isil" because it is the stock ticker symbol for Intersil (FWIW, https://lkml.org/lkml/2013/7/11/170).
Cheers,
a+
ps: sorry if your received the mail twice but google smtp servers decide to drop my first email.
--
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