Skip to content

rtc-ds1307.c: add DT property 'wakeup-source' #1260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

vitalogy
Copy link

This PR will add the DT property 'wakeup-source' to the rtc driverl rtc-ds1307 and will add an DT overlay for the Witty Pi extension board by UUGear, that has the rtc ds1337.
To support the DT property 'wakeup-source' to get the wakealarm sysfs entry I have taken the modul-source from linux-4.4 and add the necessary changes, because the irq_handling was easier to drop.
With the supplied DT overlay you get an rtc (with the wakealarm sysfs entry) for your Raspberry A+, B+ or 2. The DT property 'wakeup-source' is also used by other kernel-drivers in the mainline kernel. I will also try to bring this upstream to rtc-ds1307.

Please have a look.

regards
MichaeL

@pelwell
Copy link
Contributor

pelwell commented Jan 18, 2016

That's quite a substantial change. Although I normally prefer fewer commits, I would rather see this code change split into two parts - a clearly labelled back-port of the 4.4 version (with an explanation of why it is necessary), and a separate commit containing your changes. We are likely to switch to 4.4 fairly soon - weeks, not months - and this split will make both the code reviewing and the rebasing much easier.

The overlay looks fine.

vitalogy added 2 commits January 19, 2016 07:02
contains the following changes:
 * Convert to threaded IRQ
   with this, it is easier to drop the irq-handling for chips with no irq connected to the soc
   changes for threaded IRQ: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/rtc/rtc-ds1307.c?id=2fb07a10e0aa699ddb12aba1459208579bdc9802
 * Fix kernel splat due to wakeup irq handling
 * Enable the mcp794xx alarm after programming time
 * Fix alarm programming for mcp794xx
 * Clean up ds1307_nvram_read and ds1307_nvram_write()
 * Sorting the headers

NOTE: This is a backport from linux-4.4 to rpi-4.1.y, expect the irq handling, these are small changes
@vitalogy
Copy link
Author

Splitted the PR as suggested.

And here a little explanation:
The driver rtc-ds1307 in rpi-4.1.y is using his own workqueue for his IRQ handling. The already existing threaded IRQ handling in rpi-4.1.y (and introduced in ds-1307.c in linux-4.3) is easier to drop in the driver, because there are no queues initialised. There are small fixes from linux-4.3 to version 4.4 that makes the patch for the "wakeup-source" easier to include.
The threaded IRQ handling in rpi-4.1.y is also used by the driver rtc-isl12057.c. You can use such a ISL12057-chip for wakeup a device without an IRQ connected to the SOC by stating "isil,irq2-can-wakeup-machine" in the .dts file. In newer kernel-releases the rtc-isl12057.c is also using "wakeup-source".

… wakeup the device without an IRQ

Tested with the Witty Pi extension board by UUGear that has an ds1337.
@vitalogy
Copy link
Author

corrected minor issues

@popcornmix
Copy link
Collaborator

Can you try submitting your patch upstream and give us a link to the discussion.
We'd be a lot happier accepting it once we know it's been okayed by the upstream maintainers.

@vitalogy
Copy link
Author

Send to the mailinglist rtc-linux:
https://patchwork.ozlabs.org/patch/571268/

@popcornmix
Copy link
Collaborator

@vitalogy thanks. Feel free to ping here if the patch is accepted, or some other solution emerges and we can cherry-pick whatever is merged upstream.

@vitalogy
Copy link
Author

https://patchwork.ozlabs.org/patch/571268/
Patch was applied and is now in rtc-next, queued probably for linux-4.6

@popcornmix
Copy link
Collaborator

I've cherry-picked the upstream commit and the overlay to rpi-4.4.y tree.
Can you confirm that works okay?
We could backport it to 4.1.y, but as we will be moving to 4.4 kernel shortly, that may not be essential.

@vitalogy
Copy link
Author

So sorry, I have only tested this on rpi-4.1.y yet, but it should also work with rpi-4.4.y. The rtc-ds1307 was taken from linux-4.4 and then patched with the wakeup-source changes.
My RPi is running Arch Linux ARM with rpi-4.1.15. First i must setup distcc on my home PC to compile rpi-4.4.y and test this. The last compilation has taken hours :) . Do I need some other/newer firmware/bootloader to boot rpi-4.4.y?

@nboullis
Copy link
Contributor

nboullis commented Apr 3, 2016

wakeup-source.patch.txt

For what it’s worth, I am using @vitalogy’s patches with linux 4.1.18 and OpenELEC 6.0.3.
With my home-made DS1339-based card, I had to patch the driver a little bit to avoid a reboot when the alarm was set; I have submitted the patches upstream:
https://patchwork.ozlabs.org/patch/605492/
https://patchwork.ozlabs.org/patch/605493/

Moreover, I had to enable wakeup-source in the i2c-rtc-overlay.dts file. I tried the attached patch but, unfortunately, it does not work for me: if I load this overlay with the wakeup-source option, when I look in /proc/device-tree, I see that the option is only set for the ds1307 device, but not for the ds1339 device (that I am using) and others… Any idea what is wrong with my patch?

@vitalogy
Copy link
Author

Tested rpi-4.4 with wittypi-overlay.dtb and it works without problems.
The overlay file wittypi.dtbo could not be loaded! With /opt/vc/bin/vcdbg log msg I get this:

001915.093: Failed to load overlay 'wittypi'

Do I miss something in kernel config or a new binary to load the overlay files dtbo?
I build the kernel by my self for Arch Linux ARM with make oldconfig.

Testing the wakealarm, using wittypi-overlay.dtb

# echo `date '+%s' -d '+ 5 minutes'` > /sys/class/rtc/rtc0/wakealarm
# cat /proc/driver/rtc 
rtc_time    : 16:34:42
rtc_date    : 2016-04-12
alrm_time   : 16:39:38
alrm_date   : 2016-04-12
alarm_IRQ   : yes
alrm_pending    : no
update IRQ enabled  : no
periodic IRQ enabled    : no
periodic IRQ frequency  : 1
max user IRQ frequency  : 64
24hr        : yes
# shutdown -h now

The Raspberry comes up again 👍

@nboullis
With this little patch (based on yours) I got also the wakealarm entry in sysfs for my rtc (ds1337), but only when I using i2c-rtc-overlay.dtb. The overlay i2c-rtc.dtbo could also not be loaded.

--- i2c-rtc-overlay.dts 2016-04-11 18:43:09.000000000 +0200
+++ i2c-rtc-overlay.dts 2016-04-12 18:17:15.437624445 +0200
@@ -17,6 +17,11 @@
                reg = <0x68>;
                status = "disable";
            };
+           ds1337: ds1337@68 {
+               compatible = "dallas,ds1337";
+               reg = <0x68>;
+               status = "disable";
+           };
            ds1339: ds1339@68 {
                compatible = "dallas,ds1339";
                trickle-resistor-ohms = <0>;
@@ -52,6 +57,7 @@
    };
    __overrides__ {
        ds1307 = <&ds1307>,"status";
+       ds1337 = <&ds1337>,"status";
        ds1339 = <&ds1339>,"status";
        ds3231 = <&ds3231>,"status";
        mcp7941x = <&mcp7941x>,"status";
@@ -59,5 +65,9 @@
        pcf8523 = <&pcf8523>,"status";
        pcf8563 = <&pcf8563>,"status";
        trickle-resistor-ohms = <&ds1339>,"trickle-resistor-ohms:0";
+       wakeup-source = <&ds1337>,"wakeup-source?",
+                       <&ds1339>,"wakeup-source?",
+                       <&ds3231>,"wakeup-source?",
+                       <&mcp7941x>,"wakeup-source?";
    };
 };

/boot/config.txt should contain this

dtoverlay=i2c-rtc,ds1337,wakeup-source

There is also an error in the log, but it works.

001773.240: dtparam: i2c_arm=on
001781.179: dtparam: spi=on
001802.425: Loaded overlay 'i2c-rtc'
001802.441: dtparam: ds1337=true
001803.038: dtparam: wakeup-source=true
001804.898: dterror:   phandle 1702195245 not found     <===== ???
001954.227: dtparam: arm_freq=900000000
001992.650: dtparam: core_freq=250000000
002002.731: dtparam: cache_line_size=64

@pelwell
Copy link
Contributor

pelwell commented Apr 13, 2016

I think you've been hit by two different issues. The failure to load the overlay was a recent glitch that should be fixed in the latest firmware. The phandle issue is the result of you running into a long-standing bug.

Boolean attributes cause a property to be created or deleted, which in turn can cause items after the property in question to move. The code that applies the dtparams/overrides has been using the description of the parameter in-place, walking through it and acting on it one target at a time. Your series of wakeup-source booleans causes it to lose track of where it was, hence the error.

I'll address this in an update soon.

@nboullis
Copy link
Contributor

@pelwell
Thanks for the explanation. Does it mean that it should work fine if the boolean properties where updated backwards (from mcp7941x to ds1307)?

@pelwell
Copy link
Contributor

pelwell commented Apr 13, 2016

No - the problem is that the description of which properties to modify appears after the properties themselves. As an experiment you could change the order in the overlay files so that overrides appears before fragment@0, but the correct solution is to fix the firmware (I have a patch ready for that).

@vitalogy
Copy link
Author

@pelwell @popcornmix
Updated today to new firmware and rpi-4.4.7 provided by Arch Linux ARM and it works now with the dtbo files. Thx folks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants