Module request: rtc-rv3028

[Gadgetoid]

I would very much appreciate inclusion of the rv3028 RTC driver module if possible.

The PR that brought it into upstream is here: torvalds/linux@e6e7376

It looks like it requires (or requires updates to) 'rtc_add_group' and rtc_nvram_register along with some constants not defined in rtc.h for 4.14.y so I'm not sure how straight forward this is?

Thank you.

[pelwell]

Is 4.14 a requirement for you? 4.19 is our current master branch.

[Gadgetoid]

Not at all! In my case it happened to be the kernel available via apt and I was attempting to build the module out-of-tree for testing.

[pelwell]

Can you build and test #2913?

[Gadgetoid]

Thank you- on it.

[Gadgetoid]

Built and testing against 4.19.30 using (as root):

modprobe rtc-rv3028
echo rv3028 0x52 > /sys/class/i2c-adapter/i2c-1/new_device
hwclock --systohc
hwclock --show

I believe the following dtoverlay is required for battery backup functionality to be enabled:

/dts-v1/;
/plugin/;

/ {
    compatible = "brcm,bcm2708";

    fragment@0 {
        target = <&i2c_arm>;
        __overlay__ {
            #address-cells = <1>;
            #size-cells = <0>;
            status = "okay";

            rv3028@52 {
                compatible = "microcrystal,rv3028";
                reg = <0x52>;
                trickle-resistor-ohms = <11000>;
                status = "okay";
            };
        };
    };
};

Could this please be merged into https://github.com/raspberrypi/linux/blob/e2d2941326922b63d722ebc46520c3a2287b675f/arch/arm/boot/dts/overlays/i2c-rtc-overlay.dts ?

I believe the existing address, and trickle-resistor-ohms parameters would apply.

Thank you for cranking this out so quickly, much appreciated!

[pelwell]

Could this please be merged into https://github.com/raspberrypi/linux/blob/e2d2941326922b63d722ebc46520c3a2287b675f/arch/arm/boot/dts/overlays/i2c-rtc-overlay.dts ?

Already done. 8-)

I believe the existing address, and trickle-resistor-ohms parameters would apply.

Not the address - that's fixed - but I could add the trickle-resistor.

[pelwell]

Now updated with trickle-resistor support - use the trickle-resistor-ohms parameter.

[Gadgetoid]

Actually I'm wrong regarding backup battery functionality. Looking at the datasheet for details on register 0x37 suggests it requires BSM (Backup Switchover Mode) to be changed, but this driver currently does not support that at all. I'll have to add a backup-switchover-mode property to make this settable.

[Gadgetoid]

Brilliant! Thank you.

This patch adds suggested functionality for backup-switchover-mode.

Without this parameter supplied, the RTC is never told to use its backup battery and thus loses the time on power off.

Possible options are:

• 0 - Switchover Disabled
• 1 - Enable Direct Switching Mode (switchover when Vdd < Vbackup)
• 2 - Enable Standby Mode (go into standby when Vdd < Vbackup, does not draw current)
• 3 - Enable level Switching Mode (switchover when Vdd < Vbackup and Vdd < Vddsw and Vbackup > Vddsw.

I'll also ping it over to the module author:

diff --git a/drivers/rtc/rtc-rv3028.c b/drivers/rtc/rtc-rv3028.c
index d04c2d481..d674a8257 100644
--- a/drivers/rtc/rtc-rv3028.c
+++ b/drivers/rtc/rtc-rv3028.c
@@ -74,6 +74,7 @@

 #define RV3028_BACKUP_TCE              BIT(5)
 #define RV3028_BACKUP_TCR_MASK         GENMASK(1,0)
+#define RV3028_BACKUP_BSM_MASK  0b00001100

 #define OFFSET_STEP_PPT                        953674

@@ -601,6 +602,7 @@ static int rv3028_probe(struct i2c_client *client)
        struct rv3028_data *rv3028;
        int ret, status;
        u32 ohms;
+       u8 bsm;
        struct nvmem_config nvmem_cfg = {
                .name = "rv3028_nvram",
                .word_size = 1,
@@ -671,6 +673,17 @@ static int rv3028_probe(struct i2c_client *client)
        if (ret)
                return ret;

+       /* setup backup switchover mode */
+       if (!device_property_read_u8(&client->dev, "backup-switchover-mode",
+                                 &bsm))  {
+               ret = regmap_update_bits(rv3028->regmap, RV3028_BACKUP,
+                       RV3028_BACKUP_BSM_MASK,
+                       (bsm & 0b11) << 2);
+
+               if (ret)
+                   return ret;
+       }
+
        /* setup trickle charger */
        if (!device_property_read_u32(&client->dev, "trickle-resistor-ohms",
                                      &ohms)) {

[pelwell]

Can you submit this as a PR, ideally with the required overlay parameter?

If you're happy with the rest of the functionality I'll merge it now.

[Gadgetoid]

No problem, will get a PR under way once it's merged.

And, yes, everything else seems to work as expected.

Thanks again!

[pelwell]

Have at it!