| Message ID | 20210428222953.235280-2-sebastian.reichel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support for GE B1x5v2 and B1x5Pv2 | expand |
On Wed, Apr 28, 2021 at 3:29 PM Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > > Congatec's QMX6 system on module (SoM) uses a m41t62 as RTC. The > modules SQW clock output defaults to 32768 Hz. This behaviour is > used to provide the i.MX6 CKIL clock. Once the RTC driver is probed, > the clock is disabled and all i.MX6 functionality depending on > the 32 KHz clock has undefined behaviour. For example when using > the hardware watchdog the system will likely do arbitrary reboots. > > Referencing the m41t62 directly results in a deadlock. The kernel > will see, that i.MX6 system clock needs the RTC clock and do probe > deferral. But the i.MX6 I2C module never becomes usable without the > i.MX6 CKIL clock and thus the RTC's clock will not be probed. So > from the kernel's perspective this is a chicken-and-egg problem. > > Technically everything is fine by not touching anything, since > the RTC clock correctly enables the clock on reset (i.e. on > battery backup power loss) and also the bootloader enables it > in case an something (e.g. an unpatched kernel) disabled this > incorrectly. > > A workaround for this issue is describing the square wave pin > as fixed-clock, which is registered early and basically how > this pin is used on the i.MX6. > > Suggested-by: Saravana Kannan <saravanak@google.com> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > --- > Documentation/devicetree/bindings/rtc/rtc-m41t80.txt | 9 +++++++++ > drivers/rtc/rtc-m41t80.c | 12 ++++++++++++ > 2 files changed, 21 insertions(+) > > diff --git a/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt b/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt > index c746cb221210..cdd196b1e9bd 100644 > --- a/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt > +++ b/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt > @@ -21,10 +21,19 @@ Optional properties: > clock name > - wakeup-source: Enables wake up of host system on alarm > > +Optional child node: > +- clock: Provide this if the square wave pin is used as boot-enabled fixed clock. > + > Example: > rtc@68 { > compatible = "st,m41t80"; > reg = <0x68>; > interrupt-parent = <&UIC0>; > interrupts = <0x9 0x8>; > + > + clock { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32768>; > + }; > }; > diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c > index 89128fc29ccc..b3ece42b6f90 100644 > --- a/drivers/rtc/rtc-m41t80.c > +++ b/drivers/rtc/rtc-m41t80.c > @@ -544,10 +544,22 @@ static struct clk *m41t80_sqw_register_clk(struct m41t80_data *m41t80) > { > struct i2c_client *client = m41t80->client; > struct device_node *node = client->dev.of_node; > + struct device_node *fixed_clock; > struct clk *clk; > struct clk_init_data init; > int ret; > > + fixed_clock = of_get_child_by_name(node, "clock"); > + if (fixed_clock) { > + /* > + * skip registering square wave clock when a fixed > + * clock has been registered. The fixed clock is > + * registered automatically when being referenced. > + */ > + of_node_put(fixed_clock); > + return 0; > + } > + > /* First disable the clock */ > ret = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_MON); > if (ret < 0) Reviewed-by: Saravana Kannan <saravanak@google.com> -Saravana
On Thu, 29 Apr 2021 00:29:49 +0200, Sebastian Reichel wrote: > Congatec's QMX6 system on module (SoM) uses a m41t62 as RTC. The > modules SQW clock output defaults to 32768 Hz. This behaviour is > used to provide the i.MX6 CKIL clock. Once the RTC driver is probed, > the clock is disabled and all i.MX6 functionality depending on > the 32 KHz clock has undefined behaviour. For example when using > the hardware watchdog the system will likely do arbitrary reboots. > > Referencing the m41t62 directly results in a deadlock. The kernel > will see, that i.MX6 system clock needs the RTC clock and do probe > deferral. But the i.MX6 I2C module never becomes usable without the > i.MX6 CKIL clock and thus the RTC's clock will not be probed. So > from the kernel's perspective this is a chicken-and-egg problem. > > Technically everything is fine by not touching anything, since > the RTC clock correctly enables the clock on reset (i.e. on > battery backup power loss) and also the bootloader enables it > in case an something (e.g. an unpatched kernel) disabled this > incorrectly. > > A workaround for this issue is describing the square wave pin > as fixed-clock, which is registered early and basically how > this pin is used on the i.MX6. > > Suggested-by: Saravana Kannan <saravanak@google.com> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > --- > Documentation/devicetree/bindings/rtc/rtc-m41t80.txt | 9 +++++++++ > drivers/rtc/rtc-m41t80.c | 12 ++++++++++++ > 2 files changed, 21 insertions(+) > Reviewed-by: Rob Herring <robh@kernel.org>
diff --git a/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt b/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt index c746cb221210..cdd196b1e9bd 100644 --- a/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt +++ b/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt @@ -21,10 +21,19 @@ Optional properties: clock name - wakeup-source: Enables wake up of host system on alarm +Optional child node: +- clock: Provide this if the square wave pin is used as boot-enabled fixed clock. + Example: rtc@68 { compatible = "st,m41t80"; reg = <0x68>; interrupt-parent = <&UIC0>; interrupts = <0x9 0x8>; + + clock { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <32768>; + }; }; diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c index 89128fc29ccc..b3ece42b6f90 100644 --- a/drivers/rtc/rtc-m41t80.c +++ b/drivers/rtc/rtc-m41t80.c @@ -544,10 +544,22 @@ static struct clk *m41t80_sqw_register_clk(struct m41t80_data *m41t80) { struct i2c_client *client = m41t80->client; struct device_node *node = client->dev.of_node; + struct device_node *fixed_clock; struct clk *clk; struct clk_init_data init; int ret; + fixed_clock = of_get_child_by_name(node, "clock"); + if (fixed_clock) { + /* + * skip registering square wave clock when a fixed + * clock has been registered. The fixed clock is + * registered automatically when being referenced. + */ + of_node_put(fixed_clock); + return 0; + } + /* First disable the clock */ ret = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_MON); if (ret < 0)
Congatec's QMX6 system on module (SoM) uses a m41t62 as RTC. The modules SQW clock output defaults to 32768 Hz. This behaviour is used to provide the i.MX6 CKIL clock. Once the RTC driver is probed, the clock is disabled and all i.MX6 functionality depending on the 32 KHz clock has undefined behaviour. For example when using the hardware watchdog the system will likely do arbitrary reboots. Referencing the m41t62 directly results in a deadlock. The kernel will see, that i.MX6 system clock needs the RTC clock and do probe deferral. But the i.MX6 I2C module never becomes usable without the i.MX6 CKIL clock and thus the RTC's clock will not be probed. So from the kernel's perspective this is a chicken-and-egg problem. Technically everything is fine by not touching anything, since the RTC clock correctly enables the clock on reset (i.e. on battery backup power loss) and also the bootloader enables it in case an something (e.g. an unpatched kernel) disabled this incorrectly. A workaround for this issue is describing the square wave pin as fixed-clock, which is registered early and basically how this pin is used on the i.MX6. Suggested-by: Saravana Kannan <saravanak@google.com> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> --- Documentation/devicetree/bindings/rtc/rtc-m41t80.txt | 9 +++++++++ drivers/rtc/rtc-m41t80.c | 12 ++++++++++++ 2 files changed, 21 insertions(+)