add pcf2123 RTC overlay on SPI0.0

[Kriechi]

I'm not sure is this is the correct way to add it, maybe someone could help me get this into a cleaner form and fix the dtc warnings.
Thanks.

[pelwell]

Most of your warnings were due to specifiying '#address-cells' and '#size-cells' at the wrong location. There is also a subtle problem with including a node that overwrites an existing one - this won't work when the overlay is dynamically loaded. Instead you should create a separate fragment that targets the existing node and overwrites its properties.

But even when the overlay is correct the latest dtc still produces a warning message for each fragment because it uses the fragment@n notation but the the fragments don't contain reg = <n>; properties.

Try the patch here - it replaces your device-specific overlay with a container for multiple SPI RTC devices (c.f. i2c-rtc), although at the moment there is only one device. If it works, replace your commit with the new one and use git push -f to update the PR, then comment here to let me know you are done.

[Kriechi]

Thanks @pelwell - this worked perfectly.
I've updated to commit to reflect your patch.

Just to clarify, with this change, I still have to load the kernel module rtc-pcf2123. So simply enabling the overlay in /boot/config.txt is not enough, right?

[pelwell]

Although I haven't been able to test it I would have expected the module to be loaded automatically, so I don't know why it isn't. Are you saying that you need an explicit modprobe rtc-pcf2123?

Is it possible that you have blacklisted the driver? That would explain the behaviour, although it seems unlikely. What do you get from this command?:

grep -E "(spi|rtc)" /etc/modprobe.d/*

[Kriechi]

Yes, I have to manually load it (either with modprobe, or by adding it to /etc/modules)
grep -E "(spi|rtc)" /etc/modprobe.d/* does not show any results.
I will try with a fresh system next week - maybe I have some old stuff interfering...

[Kriechi]

Without loading the rtc-pcf2123 module explicitly:

sudo vcdbg log msg tells me:

...
001516.615: dtparam: spi=on
001539.316: Loaded overlay 'spi-rtc-overlay'
001539.331: dtparam: pcf2123=true
...

dmesg does not show any signs of RTC related entries.

Does the i2c overlay automatically load correct the kernel module?

[pelwell]

It ought to. Try this command:

grep pcf2123 /lib/modules/`uname -r`/modules.alias

If that doesn't return anything, try:

sudo depmod -a

and repeat the grep.

[Kriechi]

$ sudo grep pcf2123 /lib/modules/`uname -r`/modules.alias
alias of:N*T*Cnxp,rtc-pcf2123* rtc_pcf2123

$ sudo depmod -a

$ sudo grep pcf2123 /lib/modules/`uname -r`/modules.alias
alias of:N*T*Cnxp,rtc-pcf2123* rtc_pcf2123

[pelwell]

That looks fine - buried in that string you can see the "compatible" string for the pcf2123, which is what should cause the module to be loaded.

Perhaps there is a timing problem - if the device isn't ready at boot time the module may not be loaded. You could try loading the overlay from the command line instead:

1. Comment out dtoverlay=spi-rtc,pcf2123 in config.txt
2. Reboot
3. sudo dtoverlay spi-rtc pcf2123
4. See if the module is loaded already

[Kriechi]

I assume we should load spi-rtc-overlay? Or is the dts filename wrong?

$ sudo dtoverlay spi-rtc pcf2123
DTOVERLAY[error]: Failed to open '/boot/overlays/spi-rtc.dtbo'
* Failed to read '/boot/overlays/spi-rtc.dtbo'
$ sudo dtoverlay spi-rtc-overlay pcf2123
$ lsmod
< does not show rtc-pcf2123 module>
$ modprobe rtc-pcf2123
Module                  Size  Used by
rtc_pcf2123             3920  0

[pelwell]

uname -a
ls /sys/kernel/config
ls /boot/overlays/*rtc*

[Kriechi]

$ uname -a                                                                                                                                     [0]
Linux myhostname 4.4.11-v7+ #888 SMP Mon May 23 20:10:33 BST 2016 armv7l GNU/Linux
$ ls /sys/kernel/config
device-tree
$ ls /boot/overlays/*rtc*
/boot/overlays/i2c-rtc.dtbo  /boot/overlays/spi-rtc-overlay.dtbo

With the overlay loaded:

$ tree /sys/kernel/config/device-tree/
/sys/kernel/config/device-tree/
└── overlays
    └── 0_spi-rtc-overlay
        ├── dtbo
        ├── path
        └── status

[pelwell]

OK - you've just named the overlay inconsistently. The old scheme was name-overlay.dtb, and the new way is name.dtbo.

When you have modprobed the driver, does it appear to the system as an RTC? I'm asking because you can modprobe any module but it won't necessarily do anything useful.

[Kriechi]

Yes, when I modprobe the module (or have it automatically loaded at boot by adding it to /etc/modules), and the overlay is loaded, I get /dev/rtc0 and I can access it with sudo hwclock -r. So everything works.

I will rename the file to the new scheme.

[Kriechi]

From dmesg:

[  253.368116] rtc-pcf2123 spi0.0: chip found, driver version 0.6
[  253.368138] rtc-pcf2123 spi0.0: spiclk 5000 KHz.
[  253.368571] rtc-pcf2123 spi0.0: rtc core: registered rtc-pcf2123 as rtc0

[pelwell]

For reasons I haven't figured out yet, the SPI bus doesn't seem to use compatible strings when locating a driver. There is a code to create a DT compatible lookup string but it is an ACPI code path that doesn't get called for the PCF2123 driver.

Unlike most of the other SPI drivers, the PCF2123 driver does not register a module alias and doesn't include a MODULE_DEVICE_TABLE(spi, ..) declaration (which ends up creating an SPI alias). Adding MODULE_ALIAS("spi:rtc-pcf2123"); to the module source allows it to be loaded correctly.

If this was a Pi-specific driver I would patch it, but it is an unmodified upstream driver. You could try emailing the maintainer - Alessandro Zummo a.zummo@towertech.it.

[Kriechi]

@pelwell I contacted Alessandro Zummo (no response yet), and also the original author from the pcf2123.c driver (email seems to be discontinued or invalid).

Apart from the missing auto-load issue, are there any other concerns with this PR?
Did I get the filenames correct this time? :-)

[pelwell]

That was the only issue. I think the overlay is correct as it stands - would you like me to merge it even though the end result of applying it isn't what it ought to be?

[Kriechi]

I guess it's better than nothing :-)

Maybe we should add a note to the readme, that manual loading of the kernel module is required?
Where would be a good place to add it - or feel free to add it yourself.

[pelwell]

Actually I had made a few small changes, so I've merged the updated version. And the lack of that alias is such an annoyingly small glitch I've pushed a patch to add it. Should it get fixed upstream our patch will get dropped.

[Kriechi]

Yeah! 🎉
Thanks - much appreciated!