Motorola CPCAP PMIC RTC

This driver supports the Motorola CPCAP PMIC found on some of Motorola's mobile phones, such as the Droid 4.

Quoted 2 lines · Sebastian Reichel

This driver supports the Motorola CPCAP PMIC found on some of Motorola's mobile phones, such as the Droid 4.

Hey that's cool, works for me for reading and setting date :) Just noticed one thing, see below.

Documentation/devicetree/bindings/rtc/cpcap-rtc.txt line 13

Quoted 10 lines

Documentation/devicetree/bindings/rtc/cpcap-rtc.txt new file +10 -0

+Requires node properties:
+- compatible: should contain "motorola,cpcap-rtc"
+- interrupts: An interrupt specifier for alarm and 1 Hz irq
+
+Example:
+
+cpcap_rtc: rtc {
+	compatible = "motorola,cpcap-rtc";
+	interrupts = <39 IRQ_TYPE_NONE>, <26 IRQ_TYPE_NONE>;
+};

I tried rtcwake -s 3, but got the following:

rtcwake: cannot open /sys/class/rtc/rtc0/device/power/wakeup:
No such file or directory
rtcwake: /dev/rtc0 not enabled for wakeup events

Then tried adding "wakeup-source" to above but I think the driver needs something, maybe probably just PM ops?

Quoted 24 lines · Tony Lindgren

Quoted 2 lines · Sebastian Reichel

This driver supports the Motorola CPCAP PMIC found on some of Motorola's mobile phones, such as the Droid 4.

Hey that's cool, works for me for reading and setting date :) Just noticed one thing, see below.

Documentation/devicetree/bindings/rtc/cpcap-rtc.txt line 13

Quoted 10 lines

Documentation/devicetree/bindings/rtc/cpcap-rtc.txt new file +10 -0

+Requires node properties:
+- compatible: should contain "motorola,cpcap-rtc"
+- interrupts: An interrupt specifier for alarm and 1 Hz irq
+
+Example:
+
+cpcap_rtc: rtc {
+	compatible = "motorola,cpcap-rtc";
+	interrupts = <39 IRQ_TYPE_NONE>, <26 IRQ_TYPE_NONE>;
+};

I tried rtcwake -s 3, but got the following:

rtcwake: cannot open /sys/class/rtc/rtc0/device/power/wakeup:
No such file or directory
rtcwake: /dev/rtc0 not enabled for wakeup events

Quoted 3 lines

Then tried adding "wakeup-source" to above but I think the driver needs something, maybe probably just PM ops?

Probably device_init_wakeup(dev, 1) ?

Quoted 29 lines · Alexandre Belloni

Quoted 27 lines · Tony Lindgren

Quoted 2 lines · Sebastian Reichel

This driver supports the Motorola CPCAP PMIC found on some of Motorola's mobile phones, such as the Droid 4.

Hey that's cool, works for me for reading and setting date :) Just noticed one thing, see below.

Documentation/devicetree/bindings/rtc/cpcap-rtc.txt line 13

Quoted 10 lines

Documentation/devicetree/bindings/rtc/cpcap-rtc.txt new file +10 -0

+Requires node properties:
+- compatible: should contain "motorola,cpcap-rtc"
+- interrupts: An interrupt specifier for alarm and 1 Hz irq
+
+Example:
+
+cpcap_rtc: rtc {
+	compatible = "motorola,cpcap-rtc";
+	interrupts = <39 IRQ_TYPE_NONE>, <26 IRQ_TYPE_NONE>;
+};

I tried rtcwake -s 3, but got the following:

rtcwake: cannot open /sys/class/rtc/rtc0/device/power/wakeup:
No such file or directory
rtcwake: /dev/rtc0 not enabled for wakeup events

Then tried adding "wakeup-source" to above but I think the driver needs something, maybe probably just PM ops?

Quoted 1 line

Probably device_init_wakeup(dev, 1) ?

That gets us a bit closer but now produces:

rtcwake: write error

Quoted 30 lines · Tony Lindgren

Quoted 29 lines · Alexandre Belloni

Quoted 27 lines · Tony Lindgren

Quoted 2 lines · Sebastian Reichel

This driver supports the Motorola CPCAP PMIC found on some of Motorola's mobile phones, such as the Droid 4.

Hey that's cool, works for me for reading and setting date :) Just noticed one thing, see below.

Documentation/devicetree/bindings/rtc/cpcap-rtc.txt line 13

Quoted 10 lines

Documentation/devicetree/bindings/rtc/cpcap-rtc.txt new file +10 -0

+Requires node properties:
+- compatible: should contain "motorola,cpcap-rtc"
+- interrupts: An interrupt specifier for alarm and 1 Hz irq
+
+Example:
+
+cpcap_rtc: rtc {
+	compatible = "motorola,cpcap-rtc";
+	interrupts = <39 IRQ_TYPE_NONE>, <26 IRQ_TYPE_NONE>;
+};

I tried rtcwake -s 3, but got the following:

rtcwake: cannot open /sys/class/rtc/rtc0/device/power/wakeup:
No such file or directory
rtcwake: /dev/rtc0 not enabled for wakeup events

Then tried adding "wakeup-source" to above but I think the driver needs something, maybe probably just PM ops?

Quoted 5 lines

Quoted 1 line

Probably device_init_wakeup(dev, 1) ?

That gets us a bit closer but now produces:

rtcwake: write error

Oh user error.. It needs to be done with:

# rtcwake -s 3 -m mem

And with the addition of device_init_wakeup(dev, 1) to the probe it works for me. So Sebastian, with that change, please feel free to add:

Tested-by Tony Lindgren <[email protected]>

drivers/rtc/rtc-cpcap.c new file +8 -0

+static int cpcap_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct cpcap_rtc *rtc;
+	struct cpcap_time cpcap_tm;
+	int ret = 0;
+
+	rtc = dev_get_drvdata(dev);
+

drivers/rtc/rtc-cpcap.c new file +15 -0

+		ret |= regmap_update_bits(rtc->regmap, CPCAP_REG_TOD1,
+					  TOD1_MASK, cpcap_tm.tod1);
+		ret |= regmap_update_bits(rtc->regmap, CPCAP_REG_TOD2,
+					  TOD2_MASK, cpcap_tm.tod2);
+		ret |= regmap_update_bits(rtc->regmap, CPCAP_REG_DAY,
+					  DAY_MASK, cpcap_tm.day);
+	} else {
+		/* Clearing the upper lower 8 bits of the TOD guarantees that
+		 * the upper half of TOD (TOD2) will not increment for 0xFF RTC
+		 * ticks (255 seconds).  During this time we can safely write
+		 * to DAY, TOD2, then TOD1 (in that order) and expect RTC to be
+		 * synchronized to the exact time requested upon the final write
+		 * to TOD1. */
+		ret |= regmap_update_bits(rtc->regmap, CPCAP_REG_TOD1,
+					  TOD1_MASK, 0);

drivers/rtc/rtc-cpcap.c new file +7 -0

+	if (err)
+		return err;
+
+	rtc->alarm_irq= platform_get_irq(pdev, 0);
+	err = devm_request_threaded_irq(dev, rtc->alarm_irq, NULL,
+					cpcap_rtc_alarm_irq, IRQ_NONE,
+					"rtc_alarm", rtc);

The patch has a few checkpatch issues. Some of those should really be fixed. Can you do that?

Else, it is mostly fine, a few comments below.

Quoted 12 lines · Sebastian Reichel

drivers/rtc/rtc-cpcap.c new file +12 -0

+static int cpcap_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct cpcap_rtc *rtc;
+	struct cpcap_time cpcap_tm;
+	int ret = 0;
+
+	rtc = dev_get_drvdata(dev);
+
+	rtc2cpcap_time(&cpcap_tm, tm);
+
+	if (rtc->alarm_enabled)
+		disable_irq(rtc->alarm_irq);

drivers/rtc/rtc-cpcap.c line 142

Quoted 6 lines

drivers/rtc/rtc-cpcap.c new file +6 -0

+	if (rtc->update_enabled)
+		disable_irq(rtc->update_irq);
+
+	if (rtc->vendor == CPCAP_VENDOR_ST) {
+		/* The TOD1 and TOD2 registers MUST be written in this order
+		 * for the change to properly set. */

Does this mean there is a race condition?

Quoted 23 lines

drivers/rtc/rtc-cpcap.c new file +23 -0

+		ret |= regmap_update_bits(rtc->regmap, CPCAP_REG_TOD1,
+					  TOD1_MASK, cpcap_tm.tod1);
+		ret |= regmap_update_bits(rtc->regmap, CPCAP_REG_TOD2,
+					  TOD2_MASK, cpcap_tm.tod2);
+		ret |= regmap_update_bits(rtc->regmap, CPCAP_REG_DAY,
+					  DAY_MASK, cpcap_tm.day);
+	} else {
+		/* Clearing the upper lower 8 bits of the TOD guarantees that
+		 * the upper half of TOD (TOD2) will not increment for 0xFF RTC
+		 * ticks (255 seconds).  During this time we can safely write
+		 * to DAY, TOD2, then TOD1 (in that order) and expect RTC to be
+		 * synchronized to the exact time requested upon the final write
+		 * to TOD1. */
+		ret |= regmap_update_bits(rtc->regmap, CPCAP_REG_TOD1,
+					  TOD1_MASK, 0);
+		ret |= regmap_update_bits(rtc->regmap, CPCAP_REG_DAY,
+					  DAY_MASK, cpcap_tm.day);
+		ret |= regmap_update_bits(rtc->regmap, CPCAP_REG_TOD2,
+					  TOD2_MASK, cpcap_tm.tod2);
+		ret |= regmap_update_bits(rtc->regmap, CPCAP_REG_TOD1,
+					  TOD1_MASK, cpcap_tm.tod1);
+	}
+

drivers/rtc/rtc-cpcap.c line 272

Quoted 1 line

drivers/rtc/rtc-cpcap.c new file +1 -0

+	err = cpcap_get_vendor(dev, rtc->regmap, &rtc->vendor);

I think this means it depends on the mfd tree.

Quoted 12 lines

drivers/rtc/rtc-cpcap.c new file +12 -0

+	if (err)
+		return err;
+
+	rtc->alarm_irq= platform_get_irq(pdev, 0);
+	err = devm_request_threaded_irq(dev, rtc->alarm_irq, NULL,
+					cpcap_rtc_alarm_irq, IRQ_NONE,
+					"rtc_alarm", rtc);
+	if (err) {
+		dev_err(dev, "Could not request alarm irq: %d\n", err);
+		return err;
+	}
+	disable_irq(rtc->alarm_irq);

drivers/rtc/rtc-cpcap.c line 289

Quoted 5 lines

drivers/rtc/rtc-cpcap.c new file +5 -0

+
+	rtc->update_irq= platform_get_irq(pdev, 1);
+	err = devm_request_threaded_irq(dev, rtc->update_irq, NULL,
+					cpcap_rtc_update_irq, IRQ_NONE,
+					"rtc_1hz", rtc);

I don't think this IRQ is actually useful. It doesn't really harm but the tests should pass without it.

drivers/rtc/rtc-cpcap.c line 296

Quoted 7 lines

drivers/rtc/rtc-cpcap.c new file +7 -0

+	if (err) {
+		dev_err(dev, "Could not request update irq: %d\n", err);
+		return err;
+	}
+	disable_irq(rtc->update_irq);
+
+	err = device_init_wakeup(dev, 1);

If you use device_init_wakeup, I think it needs to be called before devm_rtc_device_register() to properly work.

Quoted 1 line

Does this mean there is a race condition?

The logic (incl. comments) in this section are from the vendor kernel driver and there is no documentation for CPCAP as far as I know. I don't know if the hardware has logic to prevent a race condition for the cpcap_tm.tod1 == 255 case.

Quoted 2 lines

drivers/rtc/rtc-cpcap.c line 272

Quoted 1 line

drivers/rtc/rtc-cpcap.c new file +1 -0

+	err = cpcap_get_vendor(dev, rtc->regmap, &rtc->vendor);

I think this means it depends on the mfd tree.

Yes, but cpcap_get_vendor should get into mainline with the 4.11 mfd pull request. So if you base your 4.12 for-next tree on 4.11-rc1 everything should be fine.

Quoted 12 lines

Quoted 12 lines

drivers/rtc/rtc-cpcap.c new file +12 -0

+	if (err)
+		return err;
+
+	rtc->alarm_irq= platform_get_irq(pdev, 0);
+	err = devm_request_threaded_irq(dev, rtc->alarm_irq, NULL,
+					cpcap_rtc_alarm_irq, IRQ_NONE,
+					"rtc_alarm", rtc);
+	if (err) {
+		dev_err(dev, "Could not request alarm irq: %d\n", err);
+		return err;
+	}
+	disable_irq(rtc->alarm_irq);

Quoted 8 lines

drivers/rtc/rtc-cpcap.c line 289

Quoted 5 lines

drivers/rtc/rtc-cpcap.c new file +5 -0

+
+	rtc->update_irq= platform_get_irq(pdev, 1);
+	err = devm_request_threaded_irq(dev, rtc->update_irq, NULL,
+					cpcap_rtc_update_irq, IRQ_NONE,
+					"rtc_1hz", rtc);

I don't think this IRQ is actually useful. It doesn't really harm but the tests should pass without it.

Yes. RTC works perfectly fine with just the alarm irq. It also works perfectly fine with just the 1 Hz irq (except for wakeup).

I would like to keep the irq in the driver, so that it's explicitly disabled. On Droid 4 mainline kernel is booted via kexec from Android (AKA bootloader) and Motorola's Android kernel uses the 1 Hz IRQ for some proprietary "secure clock daemon".

I will add a comment.

Quoted 10 lines

drivers/rtc/rtc-cpcap.c line 296

Quoted 7 lines

drivers/rtc/rtc-cpcap.c new file +7 -0

+	if (err) {
+		dev_err(dev, "Could not request update irq: %d\n", err);
+		return err;
+	}
+	disable_irq(rtc->update_irq);
+
+	err = device_init_wakeup(dev, 1);

If you use device_init_wakeup, I think it needs to be called before devm_rtc_device_register() to properly work.

I successfully tested wakeup before sending this. But in case your prefer it to be called before registering the RTC I can move the call accordingly.

This driver supports the Motorola CPCAP PMIC found on some of Motorola's mobile phones, such as the Droid 4.

Tested-by Tony Lindgren <[email protected]>

Signed-off-by Sebastian Reichel <[email protected]>

---

Changes since PATCHv2:
- checkpatch fixes
- add comment for update irq
- introduced vendor prefix patch
Changes since PATCHv1:
- added device_init_wakeup() at the end of probe
- added Tested-by from Tony

---
 .../devicetree/bindings/rtc/cpcap-rtc.txt          |  13 +
 drivers/rtc/Kconfig                                |   7 +
 drivers/rtc/Makefile                               |   1 +
 drivers/rtc/rtc-cpcap.c                            | 332 +++++++++++++++++++++
 4 files changed, 353 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/cpcap-rtc.txt
 create mode 100644 drivers/rtc/rtc-cpcap.c

Documentation/devicetree/bindings/rtc/cpcap-rtc.txt new file +0 -0

new file mode 100644
index 000000000000..2709c32baf2c

Documentation/devicetree/bindings/rtc/cpcap-rtc.txt new file +11 -0

+
+Requires node properties:
+- compatible: should contain "motorola,cpcap-rtc"
+- interrupts: An interrupt specifier for alarm and 1 Hz irq
+
+Example:
+
+cpcap_rtc: rtc {
+	compatible = "motorola,cpcap-rtc";
+	interrupts = <39 IRQ_TYPE_NONE>, <26 IRQ_TYPE_NONE>;
+};

Motorola was involved in semiconductor and mobile phone business. The "motrola," prefix is already used by a couple of bindings:

* rtc/rtc-cmos.txt
* mfd/motorola-cpcap.txt
* regulator/cpcap-regulator.txt

Apart from that it is used in the DT file for the Droid 4 mobile phone.

Signed-off-by Sebastian Reichel <[email protected]>

---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

This driver supports the Motorola CPCAP PMIC found on some of Motorola's mobile phones, such as the Droid 4.

Tested-by Tony Lindgren <[email protected]>

Signed-off-by Sebastian Reichel <[email protected]>

---
Changes since PATCHv3:
- Modified DT binding document to mention parent device

---
 .../devicetree/bindings/rtc/cpcap-rtc.txt          |  18 ++

 drivers/rtc/Kconfig                                |   7 +
 drivers/rtc/Makefile                               |   1 +
 drivers/rtc/rtc-cpcap.c                            | 332 +++++++++++++++++++++
 4 files changed, 358 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/cpcap-rtc.txt
 create mode 100644 drivers/rtc/rtc-cpcap.c

This driver supports the Motorola CPCAP PMIC found on some of Motorola's mobile phones, such as the Droid 4.

Tested-by Tony Lindgren <[email protected]>

Signed-off-by Sebastian Reichel <[email protected]>

---
Changes since PATCHv3:
- Modified DT binding document to mention parent device

---
 .../devicetree/bindings/rtc/cpcap-rtc.txt          |  18 ++
 drivers/rtc/Kconfig                                |   7 +
 drivers/rtc/Makefile                               |   1 +
 drivers/rtc/rtc-cpcap.c                            | 332 +++++++++++++++++++++
 4 files changed, 358 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/cpcap-rtc.txt
 create mode 100644 drivers/rtc/rtc-cpcap.c

Motorola was involved in semiconductor and mobile phone business. The "motorola," prefix is already used by a couple of bindings:

* rtc/rtc-cmos.txt
* mfd/motorola-cpcap.txt
* regulator/cpcap-regulator.txt

Apart from that it is used in the DT file for the Droid 4 mobile phone.

Acked-by Rob Herring <[email protected]>

Signed-off-by Sebastian Reichel <[email protected]>

---
Changes since PATCHv3:
- Fix typo
- Add Acked-By from Rob Herring

---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)