Re: [PATCH v4 1/2] arm64: dts: meson-g12b-odroid-n2: Enable RTC controller node

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/09/2020 12:14, Anand Moon wrote:
> Hi Kevin,
> 
> Thanks for your review comments and testing.
> 
> On Tue, 1 Sep 2020 at 01:36, Kevin Hilman <khilman@xxxxxxxxxxxx> wrote:
>>
>> Anand Moon <linux.amoon@xxxxxxxxx> writes:
>>
>>> Enable RTC PCF8563 node on Odroid-N2 SBC, In order to
>>> support the RTC wakealarm feature for suspend and resume.
>>> Also assign an alias to the pcf8563 to rtc0 and meson-vrtc to rtc1
>>> timer device to prevent it being assigned to /dev/rtc0
>>> which disto userspace tools assume is a clock device.
>>>
>>> Cc: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
>>> Cc: Kevin Hilman <khilman@xxxxxxxxxxxx>
>>> Suggested-by: Christian Hewitt <christianshewitt@xxxxxxxxx>
>>> Signed-off-by: Anand Moon <linux.amoon@xxxxxxxxx>
>>> ---
>>> Changes v4
>>> --Add gpio interrupt for GPIOAO.BIT7 as suggested by Neil.
>>> Changes v3
>>> --Drop the INI GPIOAO.BIT7 pinctrl.
>>> --Added missing RTC alias so that rtc get assigned correcly,
>>>   as suggested by Chris Hewitt.
>>> changes v2
>>> --Fix the missing INT (GPIOAO.BIT7) pinctrl.
>>> --Fix the missing rtcwakeup.
>>> --Drop the clock not required clock property by the PCF8563 driver.
>>> ---
>>>  .../boot/dts/amlogic/meson-g12b-odroid-n2.dts   | 17 +++++++++++++++++
>>>  1 file changed, 17 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
>>> index 34fffa6d859d..3e2aaa6f48e5 100644
>>> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
>>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
>>> @@ -19,6 +19,8 @@ / {
>>>       aliases {
>>>               serial0 = &uart_AO;
>>>               ethernet0 = &ethmac;
>>> +             rtc0 = &rtc0;
>>> +             rtc1 = &vrtc;
>>>       };
>>>
>>>       dioo2133: audio-amplifier-0 {
>>> @@ -477,6 +479,21 @@ hdmi_tx_tmds_out: endpoint {
>>>       };
>>>  };
>>>
>>> +&i2c3 {
>>> +     pinctrl-0 = <&i2c3_sda_a_pins>, <&i2c3_sck_a_pins>;
>>> +     pinctrl-names = "default";
>>> +     status = "okay";
>>> +
>>> +     rtc0: rtc@51 {
>>> +             reg = <0x51>;
>>> +             compatible = "nxp,pcf8563";
>>> +             /* RTC INT */
>>> +             interrupts = <GPIOAO_7 IRQ_TYPE_LEVEL_LOW>;
>>> +             interrupt-parent = <&gpio_intc>;
>>> +             wakeup-source;
>>> +     };
>>> +};
>>
>> There's still no pinctrl definition for the GPIO pin being used as the
>> IRQ.  It looks like you discussed this with Martin and he pointed you in
>> the right direction in your v3 series, but I don't see it in this
>> patch.
>>
> Yes I had followed the approach suggested by Martin on previous email and IRC.
> but it really did not work out for me in the testing.
> 
> rtc-pcf8563 driver does not handle such gpio configuration.
> so the rtc probe will fail if we add gpio pinctl to *pinctrl-0*.

No need for multiple pinctrl-*, simple add a new pinctrl to the rtc node like:


@@ -18,6 +18,8 @@
        aliases {
                serial0 = &uart_AO;
                ethernet0 = &ethmac;
+               rtc0 = &rtc0;
+               rtc1 = &vrtc;
        };

        chosen {
@@ -266,6 +268,17 @@
        status = "okay";
 };

+&ao_pinctrl {
+       rtc_int_pins: rtc-int {
+               mux {
+                       groups = "GPIOAO_7";
+                       function = "gpio_aobus";
+                       bias-disable;
+                       output-disable;
+               };
+       };
+};
+
 &cec_AO {
        pinctrl-0 = <&cec_ao_a_h_pins>;
        pinctrl-names = "default";
@@ -391,6 +404,23 @@
        };
 };

+&i2c3 {
+       pinctrl-0 = <&i2c3_sda_a_pins>, <&i2c3_sck_a_pins>;
+       pinctrl-names = "default";
+       status = "okay";
+
+       rtc0: rtc@51 {
+             pinctrl-0 = <&rtc_int_pins>;
+             pinctrl-names = "default";
+             reg = <0x51>;
+             compatible = "nxp,pcf8563";
+             /* RTC INT */
+             interrupts = <GPIOAO_7 IRQ_TYPE_LEVEL_LOW>;
+             interrupt-parent = <&gpio_intc>;
+             wakeup-source;
+       };
+};
+
 &ir {
        status = "okay";
        pinctrl-0 = <&remote_input_ao_pins>;

DISCALIMER: not built or run tested

Neil

> 
> So there could be an internal way the wakeup gets triggered for the u-boot.
> _This is the reason I have opted for Neil's suggestion._
> 
>> You can see the GPIOs that the kernel knows about using the GPIO
>> debugfs.  For example:
>>
> 
> Yes I had already checked this /sys/kernel/debug/gpio
> it's not reflecting at my end as well.
> 
>> / # cat /sys/kernel/debug/gpio
>> gpiochip1: GPIOs 412-426, parent: platform/ff800000.sys-ctrl:pinctrl@14, aobus-banks:
>>  gpio-414 (                    |enable              ) out lo
>>  gpio-420 (                    |regulator-tflash_vdd) out hi
>>  gpio-421 (                    |TF_IO               ) out lo
>>  gpio-423 (                    |n2:blue             ) out lo
>>
>> gpiochip0: GPIOs 427-511, parent: platform/ff634400.bus:pinctrl@40, periphs-banks:
>>  gpio-442 (                    |PHY reset           ) out hi ACTIVE LOW
>>  gpio-447 (                    |usb-hub-reset       ) out hi
>>  gpio-448 (                    |regulator-hub_5v    ) out hi
>>  gpio-449 (                    |regulator-usb_pwr_en) out lo
>>  gpio-464 (                    |reset               ) out hi ACTIVE LOW
>>  gpio-474 (                    |cd                  ) in  lo ACTIVE LOW
>>
>>
>> Also, I tested this on my odroid-n2, and it does not fully wakeup[1].
>> At the end of the log, you can see the "resume rate" of the big and
>> little cores, which suggests that the SoC has woken and trying to
>> resume, but it never makes it back to the kernel.
> 
> I feel the RTC wakeup is handled by the u-boot bl30
> since virtual RTC we pass the seconds as sleep
> is reflected in the logs.
> 
> # rtcwake -d /dev/rtc1 -s 5 -m mem
> ...
> bl30 get wakeup sources!
> process command 00000006
> bl30 enter suspend!
> Little core clk suspend rate 1398000000
> Big core clk suspend rate 24000000
> store restore gp0 pll
> suspend_counter: 1
> Enter ddr suspend
> ddr suspend time: 16us
> *alarm=6S*
> process command 00000001
> GPIOA_11/13 off
> cec ver:2018/04/19
> CEC cfg:0x0000
> WAKEUP GPIO cfg:0x00000000
> ...
> 
>>
>> Could you be more specific with exactly what u-boot you're running
>> (mainline version and Khadas version.)
>>
> 
> On u-boot Mainline
> U-Boot 2020.10-rc2-00133-g60d5402950-dirty (Aug 16 2020 - 20:25:26
> +0530) odroid-n2
> 
> [0] https://pastebin.com/GGUM7k8Q
> 
> On u-boot Hardkernel
> U-Boot 2015.01-10 (Dec 08 2019 - 14:54:07) Arch Linux ARM (Hardkernel U-boot)
> 
> [1] https://pastebin.com/WbHGFmH2
> 
> Note: Yes I have observed there is some off sync in sleep timeout.
> 
>> I'm running an older version of mainline u-boot:
>> U-Boot 2019.07-rc3-00029-g47bebaa4a3-dirty (Jun 04 2019 - 17:16:32 +0200) odroid-n2
>>
>> Kevin
>>
> 
> Yes I have observed this at my end on Hardkernel u-boot.
> This message is because you have not sync the hwclock with the timezone.
> 
> # sudo hwclock -w -f /dev/rtc0
> # sudo  hwclock --systohc
> 
> Once you sync with the timezone these messages are resolved.
>>
>> [1]
>> / # dmesg |grep -i rtc
>> [   14.799773] meson-vrtc ff8000a8.rtc: registered as rtc1
>> [   14.871365] rtc-pcf8563 0-0051: low voltage detected, date/time is not reliable.
>> [   14.871519] rtc-pcf8563 0-0051: registered as rtc0
>> [   14.873536] rtc-pcf8563 0-0051: low voltage detected, date/time is not reliable.
>> [   14.886474] rtc-pcf8563 0-0051: hctosys: unable to read the hardware clock
>> / # rtcwake -d rtc0 -m mem -s5
>> rtcwake: assuming RTC uses UTC ...
>> rtcwake: wakeup from "mem" using rtc0 at Mon Aug 31 19:58:15 2020
>> [  119.297633] PM: suspend entry (deep)
>> [  119.297722] Filesystems sync: 0.000 seconds
>> [  119.300330] Freezing user space processes ... (elapsed 0.003 seconds) done.
>> [  119.306667] OOM killer disabled.
>> [  119.309828] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
>> [  119.317184] printk: Suspending console(s) (use no_console_suspend to debug)
>> bl30 get wakeup sources!
>> process command 00000006
>> bl30 enter suspend!
>> Little core clk suspend rate 1200000000
>> Big core clk suspend rate 24000000
>> store restore gp0 pll
>> suspend_counter: 1
>> Enter ddr suspend
>> ddr suspend time: 17us
>> alarm=0S
>> process command 00000001
>> cec ver:2018/04/19
>> CEC cfg:0x0000
>> WAKEUP GPIO cfg:0x00000000
>> use vddee new table!
>> WAKEUP GPIO FAIL - invalid key
>> fffffe71
>> use vddee new table!
>> exit_reason:0x03
>> Enter ddr resume
>> ddr resume time: 125us
>> store restore gp0 pll
>> cfg15 3b00000
>> cfg15 33b00000
>> Little core clk resume rate 1200000000
>> Big core clk resume rate 50000000
>>
>>
> 
> Best Regards
> -Anand
> 




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux