devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] dt-bindings: rtc: m41t80: Convert text schema to YAML one
@ 2022-12-11 20:51 Marek Vasut
  2022-12-11 20:51 ` [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated Marek Vasut
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Marek Vasut @ 2022-12-11 20:51 UTC (permalink / raw)
  To: devicetree
  Cc: Marek Vasut, Alessandro Zummo, Alexandre Belloni, Rob Herring,
	Krzysztof Kozlowski, linux-rtc

Convert the m41t80 text schema to YAML schema.

Add "#clock-cells" requirement, which is required by clock-output-names.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: linux-rtc@vger.kernel.org
To: devicetree@vger.kernel.org
---
V2: - Add maxItems:1 to clock-output-names
    - Skip wakeup-source property, comes from rtc.yaml
    - Mention the new clock-cells requirement coming from
      clock-output-names property.
    - Rename rtc-m41t80.txt to st,m41t80.yaml
V3: - Update license to (GPL-2.0-only OR BSD-2-Clause)
---
 .../devicetree/bindings/rtc/rtc-m41t80.txt    | 39 ----------
 .../devicetree/bindings/rtc/st,m41t80.yaml    | 73 +++++++++++++++++++
 2 files changed, 73 insertions(+), 39 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/rtc/rtc-m41t80.txt
 create mode 100644 Documentation/devicetree/bindings/rtc/st,m41t80.yaml

diff --git a/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt b/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt
deleted file mode 100644
index cdd196b1e9bdb..0000000000000
--- a/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt
+++ /dev/null
@@ -1,39 +0,0 @@
-ST M41T80 family of RTC and compatible
-
-Required properties:
-- compatible: should be one of:
-	"st,m41t62",
-	"st,m41t65",
-	"st,m41t80",
-	"st,m41t81",
-	"st,m41t81s",
-	"st,m41t82",
-	"st,m41t83",
-	"st,m41t84",
-	"st,m41t85",
-	"st,m41t87",
-	"microcrystal,rv4162",
-- reg: I2C bus address of the device
-
-Optional properties:
-- interrupts: rtc alarm interrupt.
-- clock-output-names: From common clock binding to override the default output
-                      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/Documentation/devicetree/bindings/rtc/st,m41t80.yaml b/Documentation/devicetree/bindings/rtc/st,m41t80.yaml
new file mode 100644
index 0000000000000..fc9c6da6483f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/st,m41t80.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/st,m41t80.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ST M41T80 family of RTC and compatible
+
+maintainers:
+  - Alexandre Belloni <alexandre.belloni@bootlin.com>
+
+properties:
+  compatible:
+    enum:
+      - st,m41t62
+      - st,m41t65
+      - st,m41t80
+      - st,m41t81
+      - st,m41t81s
+      - st,m41t82
+      - st,m41t83
+      - st,m41t84
+      - st,m41t85
+      - st,m41t87
+      - microcrystal,rv4162
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 1
+
+  clock-output-names:
+    maxItems: 1
+    description: From common clock binding to override the default output clock name.
+
+  clock:
+    type: object
+    $ref: /schemas/clock/fixed-clock.yaml#
+    properties:
+      clock-frequency:
+        const: 32768
+
+allOf:
+  - $ref: rtc.yaml
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      rtc@68 {
+        compatible = "st,m41t80";
+        reg = <0x68>;
+        interrupt-parent = <&UIC0>;
+        interrupts = <0x9 0x8>;
+
+        clock {
+          compatible = "fixed-clock";
+          #clock-cells = <0>;
+          clock-frequency = <32768>;
+        };
+      };
+    };
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated
  2022-12-11 20:51 [PATCH v3 1/2] dt-bindings: rtc: m41t80: Convert text schema to YAML one Marek Vasut
@ 2022-12-11 20:51 ` Marek Vasut
  2022-12-15 18:06   ` Sebastian Reichel
  2022-12-12  8:57 ` [PATCH v3 1/2] dt-bindings: rtc: m41t80: Convert text schema to YAML one Krzysztof Kozlowski
  2022-12-12  9:25 ` Alexandre Belloni
  2 siblings, 1 reply; 11+ messages in thread
From: Marek Vasut @ 2022-12-11 20:51 UTC (permalink / raw)
  To: devicetree
  Cc: Marek Vasut, Krzysztof Kozlowski, Alessandro Zummo,
	Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, linux-rtc

The clock {} subnode seems like it is describing an always-on clock
generated by the PMIC. This should rather be modeled by consumer of
the clock taking phandle to the RTC node itself, since it already
does have clock-cells and all. Since there are no users of the clock
subnode in tree anyway, mark it as deprecated to avoid proliferation
of this approach.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: linux-rtc@vger.kernel.org
To: devicetree@vger.kernel.org
---
V2: - Add AB from Krzysztof
V3: - No change
---
 Documentation/devicetree/bindings/rtc/st,m41t80.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/rtc/st,m41t80.yaml b/Documentation/devicetree/bindings/rtc/st,m41t80.yaml
index fc9c6da6483f5..03ff833f5fe9d 100644
--- a/Documentation/devicetree/bindings/rtc/st,m41t80.yaml
+++ b/Documentation/devicetree/bindings/rtc/st,m41t80.yaml
@@ -40,6 +40,7 @@ properties:
   clock:
     type: object
     $ref: /schemas/clock/fixed-clock.yaml#
+    deprecated: true
     properties:
       clock-frequency:
         const: 32768
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] dt-bindings: rtc: m41t80: Convert text schema to YAML one
  2022-12-11 20:51 [PATCH v3 1/2] dt-bindings: rtc: m41t80: Convert text schema to YAML one Marek Vasut
  2022-12-11 20:51 ` [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated Marek Vasut
@ 2022-12-12  8:57 ` Krzysztof Kozlowski
  2022-12-12  9:25 ` Alexandre Belloni
  2 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-12  8:57 UTC (permalink / raw)
  To: Marek Vasut, devicetree
  Cc: Alessandro Zummo, Alexandre Belloni, Rob Herring,
	Krzysztof Kozlowski, linux-rtc

On 11/12/2022 21:51, Marek Vasut wrote:
> Convert the m41t80 text schema to YAML schema.
> 
> Add "#clock-cells" requirement, which is required by clock-output-names.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] dt-bindings: rtc: m41t80: Convert text schema to YAML one
  2022-12-11 20:51 [PATCH v3 1/2] dt-bindings: rtc: m41t80: Convert text schema to YAML one Marek Vasut
  2022-12-11 20:51 ` [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated Marek Vasut
  2022-12-12  8:57 ` [PATCH v3 1/2] dt-bindings: rtc: m41t80: Convert text schema to YAML one Krzysztof Kozlowski
@ 2022-12-12  9:25 ` Alexandre Belloni
  2 siblings, 0 replies; 11+ messages in thread
From: Alexandre Belloni @ 2022-12-12  9:25 UTC (permalink / raw)
  To: Marek Vasut, devicetree
  Cc: Krzysztof Kozlowski, linux-rtc, Rob Herring, Alessandro Zummo

On Sun, 11 Dec 2022 21:51:23 +0100, Marek Vasut wrote:
> Convert the m41t80 text schema to YAML schema.
> 
> Add "#clock-cells" requirement, which is required by clock-output-names.
> 
> 

Applied, thanks!

[1/2] dt-bindings: rtc: m41t80: Convert text schema to YAML one
      commit: 3e39f7971d75cafe1c90dec60526ad45484657c0
[2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated
      commit: b279e3276c247a060ca73784e6655472b74c3f27

Best regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated
  2022-12-11 20:51 ` [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated Marek Vasut
@ 2022-12-15 18:06   ` Sebastian Reichel
  2022-12-15 19:39     ` Marek Vasut
  0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Reichel @ 2022-12-15 18:06 UTC (permalink / raw)
  To: Marek Vasut
  Cc: devicetree, Krzysztof Kozlowski, Alessandro Zummo,
	Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, linux-rtc

[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]

Hi,

On Sun, Dec 11, 2022 at 09:51:24PM +0100, Marek Vasut wrote:
> The clock {} subnode seems like it is describing an always-on clock
> generated by the PMIC. This should rather be modeled by consumer of
> the clock taking phandle to the RTC node itself, since it already
> does have clock-cells and all. Since there are no users of the clock
> subnode in tree anyway, mark it as deprecated to avoid proliferation
> of this approach.
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: linux-rtc@vger.kernel.org
> To: devicetree@vger.kernel.org
> ---
> V2: - Add AB from Krzysztof
> V3: - No change
> ---

I just noticed this by accident. Basically everything in the patch
description is wrong:

1. There is a in-tree user: arch/arm/boot/dts/imx6dl-qmx6.dtsi
2. The PMIC has nothing to do with this
3. Directly referencing the RTC does not work, since that introduces
   an unsolvable dependency loop on QMX6. This was the solution accepted
   by Rob and Saravana:

[v1] https://lore.kernel.org/lkml/20210222171247.97609-1-sebastian.reichel@collabora.com/
[v2] https://lore.kernel.org/all/20210428222953.235280-1-sebastian.reichel@collabora.com/

-- Sebastian

>  Documentation/devicetree/bindings/rtc/st,m41t80.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/st,m41t80.yaml b/Documentation/devicetree/bindings/rtc/st,m41t80.yaml
> index fc9c6da6483f5..03ff833f5fe9d 100644
> --- a/Documentation/devicetree/bindings/rtc/st,m41t80.yaml
> +++ b/Documentation/devicetree/bindings/rtc/st,m41t80.yaml
> @@ -40,6 +40,7 @@ properties:
>    clock:
>      type: object
>      $ref: /schemas/clock/fixed-clock.yaml#
> +    deprecated: true
>      properties:
>        clock-frequency:
>          const: 32768
> -- 
> 2.35.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated
  2022-12-15 18:06   ` Sebastian Reichel
@ 2022-12-15 19:39     ` Marek Vasut
  2022-12-16 14:24       ` Sebastian Reichel
  0 siblings, 1 reply; 11+ messages in thread
From: Marek Vasut @ 2022-12-15 19:39 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: devicetree, Krzysztof Kozlowski, Alessandro Zummo,
	Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, linux-rtc

On 12/15/22 19:06, Sebastian Reichel wrote:
> Hi,

Hi,

> On Sun, Dec 11, 2022 at 09:51:24PM +0100, Marek Vasut wrote:
>> The clock {} subnode seems like it is describing an always-on clock
>> generated by the PMIC. This should rather be modeled by consumer of
>> the clock taking phandle to the RTC node itself, since it already
>> does have clock-cells and all. Since there are no users of the clock
>> subnode in tree anyway, mark it as deprecated to avoid proliferation
>> of this approach.
>>
>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>> Cc: linux-rtc@vger.kernel.org
>> To: devicetree@vger.kernel.org
>> ---
>> V2: - Add AB from Krzysztof
>> V3: - No change
>> ---
> 
> I just noticed this by accident. Basically everything in the patch
> description is wrong:
> 
> 1. There is a in-tree user: arch/arm/boot/dts/imx6dl-qmx6.dtsi

Sorry, I missed this one.

> 2. The PMIC has nothing to do with this

In [3] the commit message claims the PMIC supplies 32kHz clock to i.MX6 
CKIL , which per IMX6DQRM rev.6 Table 18-3 row SNVS indirectly supplies 
SNVS RTC . This reminded me of commit:

9509593f327ac ("arm64: dts: imx8mm: Model PMIC to SNVS RTC clock path on 
Data Modul i.MX8M Mini eDM SBC")

which solves exactly the same problem, system hangs when 32 kHz clock 
are stopped, except this time on i.MX8MM, clock are generated by PMIC on 
I2C (notice how the PMIC is referenced directly) and the clock are 
supplied to the SVNS RTC XTal terminals.

I wonder if this could be reused on the QMX6 board too ?

> 3. Directly referencing the RTC does not work, since that introduces
>     an unsolvable dependency loop on QMX6. This was the solution accepted
>     by Rob and Saravana:
> 
> [v1] https://lore.kernel.org/lkml/20210222171247.97609-1-sebastian.reichel@collabora.com/
> [v2] https://lore.kernel.org/all/20210428222953.235280-1-sebastian.reichel@collabora.com/

[3] 
https://lore.kernel.org/linux-clk/20191108170135.9053-1-sebastian.reichel@collabora.com/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated
  2022-12-15 19:39     ` Marek Vasut
@ 2022-12-16 14:24       ` Sebastian Reichel
  2022-12-16 14:36         ` Marek Vasut
  0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Reichel @ 2022-12-16 14:24 UTC (permalink / raw)
  To: Marek Vasut
  Cc: devicetree, Krzysztof Kozlowski, Alessandro Zummo,
	Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, linux-rtc

[-- Attachment #1: Type: text/plain, Size: 2993 bytes --]

Hi,

On Thu, Dec 15, 2022 at 08:39:47PM +0100, Marek Vasut wrote:
> On 12/15/22 19:06, Sebastian Reichel wrote:
> > On Sun, Dec 11, 2022 at 09:51:24PM +0100, Marek Vasut wrote:
> > > The clock {} subnode seems like it is describing an always-on clock
> > > generated by the PMIC. This should rather be modeled by consumer of
> > > the clock taking phandle to the RTC node itself, since it already
> > > does have clock-cells and all. Since there are no users of the clock
> > > subnode in tree anyway, mark it as deprecated to avoid proliferation
> > > of this approach.
> > > 
> > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > ---
> > > Cc: Alessandro Zummo <a.zummo@towertech.it>
> > > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > > Cc: linux-rtc@vger.kernel.org
> > > To: devicetree@vger.kernel.org
> > > ---
> > > V2: - Add AB from Krzysztof
> > > V3: - No change
> > > ---
> > 
> > I just noticed this by accident. Basically everything in the patch
> > description is wrong:
> > 
> > 1. There is a in-tree user: arch/arm/boot/dts/imx6dl-qmx6.dtsi
> 
> Sorry, I missed this one.
> 
> > 2. The PMIC has nothing to do with this
> 
> In [3] the commit message claims the PMIC supplies 32kHz clock to i.MX6 CKIL,
> which per IMX6DQRM rev.6 Table 18-3 row SNVS indirectly supplies SNVS RTC.
> This reminded me of commit:

The word PMIC is not mentioned once in [3]. PMIC is not involved.
The QMX6 32khz chain is like this:

32kHz crystal -> m41t62 crystal input
m41t62 clock output -> i.MX6 CKIL

> 9509593f327ac ("arm64: dts: imx8mm: Model PMIC to SNVS RTC clock path on
> Data Modul i.MX8M Mini eDM SBC")
> 
> which solves exactly the same problem, system hangs when 32 kHz clock are
> stopped, except this time on i.MX8MM, clock are generated by PMIC on I2C
> (notice how the PMIC is referenced directly) and the clock are supplied to
> the SVNS RTC XTal terminals.
> 
> I wonder if this could be reused on the QMX6 board too?

IIRC On i.MX6 referencing the I2C connected RTC results in boot
hanging forever when trying to get the ckil clock in
imx6q_clocks_init. At least it used to be the case when I was
working on this - I no longer have access to the boards. Of course
properly referencing the RTC clock was the first route I tried.

-- Sebastian

> > 3. Directly referencing the RTC does not work, since that introduces
> >     an unsolvable dependency loop on QMX6. This was the solution accepted
> >     by Rob and Saravana:
> > 
> > [v1] https://lore.kernel.org/lkml/20210222171247.97609-1-sebastian.reichel@collabora.com/
> > [v2] https://lore.kernel.org/all/20210428222953.235280-1-sebastian.reichel@collabora.com/
> 
> [3] https://lore.kernel.org/linux-clk/20191108170135.9053-1-sebastian.reichel@collabora.com/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated
  2022-12-16 14:24       ` Sebastian Reichel
@ 2022-12-16 14:36         ` Marek Vasut
  2022-12-16 15:05           ` Sebastian Reichel
  0 siblings, 1 reply; 11+ messages in thread
From: Marek Vasut @ 2022-12-16 14:36 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: devicetree, Krzysztof Kozlowski, Alessandro Zummo,
	Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, linux-rtc

On 12/16/22 15:24, Sebastian Reichel wrote:
> Hi,

Hi,

> On Thu, Dec 15, 2022 at 08:39:47PM +0100, Marek Vasut wrote:
>> On 12/15/22 19:06, Sebastian Reichel wrote:
>>> On Sun, Dec 11, 2022 at 09:51:24PM +0100, Marek Vasut wrote:
>>>> The clock {} subnode seems like it is describing an always-on clock
>>>> generated by the PMIC. This should rather be modeled by consumer of
>>>> the clock taking phandle to the RTC node itself, since it already
>>>> does have clock-cells and all. Since there are no users of the clock
>>>> subnode in tree anyway, mark it as deprecated to avoid proliferation
>>>> of this approach.
>>>>
>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> ---
>>>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>>>> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>>>> Cc: linux-rtc@vger.kernel.org
>>>> To: devicetree@vger.kernel.org
>>>> ---
>>>> V2: - Add AB from Krzysztof
>>>> V3: - No change
>>>> ---
>>>
>>> I just noticed this by accident. Basically everything in the patch
>>> description is wrong:
>>>
>>> 1. There is a in-tree user: arch/arm/boot/dts/imx6dl-qmx6.dtsi
>>
>> Sorry, I missed this one.
>>
>>> 2. The PMIC has nothing to do with this
>>
>> In [3] the commit message claims the PMIC supplies 32kHz clock to i.MX6 CKIL,
>> which per IMX6DQRM rev.6 Table 18-3 row SNVS indirectly supplies SNVS RTC.
>> This reminded me of commit:
> 
> The word PMIC is not mentioned once in [3].

s@PMIC@m41t62 RTC@, sorry.

> PMIC is not involved.
> The QMX6 32khz chain is like this:
> 
> 32kHz crystal -> m41t62 crystal input
> m41t62 clock output -> i.MX6 CKIL
> 
>> 9509593f327ac ("arm64: dts: imx8mm: Model PMIC to SNVS RTC clock path on
>> Data Modul i.MX8M Mini eDM SBC")
>>
>> which solves exactly the same problem, system hangs when 32 kHz clock are
>> stopped, except this time on i.MX8MM, clock are generated by PMIC on I2C
>> (notice how the PMIC is referenced directly) and the clock are supplied to
>> the SVNS RTC XTal terminals.
>>
>> I wonder if this could be reused on the QMX6 board too?
> 
> IIRC On i.MX6 referencing the I2C connected RTC results in boot
> hanging forever when trying to get the ckil clock in
> imx6q_clocks_init. At least it used to be the case when I was
> working on this - I no longer have access to the boards. Of course
> properly referencing the RTC clock was the first route I tried.

Hmmmmm, what shall we do, un-deprecate the clock sub-node ?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated
  2022-12-16 14:36         ` Marek Vasut
@ 2022-12-16 15:05           ` Sebastian Reichel
  2022-12-16 16:24             ` Alexandre Belloni
  0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Reichel @ 2022-12-16 15:05 UTC (permalink / raw)
  To: Marek Vasut
  Cc: devicetree, Krzysztof Kozlowski, Alessandro Zummo,
	Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, linux-rtc

[-- Attachment #1: Type: text/plain, Size: 3324 bytes --]

Hi,

On Fri, Dec 16, 2022 at 03:36:17PM +0100, Marek Vasut wrote:
> On 12/16/22 15:24, Sebastian Reichel wrote:
> > On Thu, Dec 15, 2022 at 08:39:47PM +0100, Marek Vasut wrote:
> > > On 12/15/22 19:06, Sebastian Reichel wrote:
> > > > On Sun, Dec 11, 2022 at 09:51:24PM +0100, Marek Vasut wrote:
> > > > > The clock {} subnode seems like it is describing an always-on clock
> > > > > generated by the PMIC. This should rather be modeled by consumer of
> > > > > the clock taking phandle to the RTC node itself, since it already
> > > > > does have clock-cells and all. Since there are no users of the clock
> > > > > subnode in tree anyway, mark it as deprecated to avoid proliferation
> > > > > of this approach.
> > > > > 
> > > > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > > ---
> > > > > Cc: Alessandro Zummo <a.zummo@towertech.it>
> > > > > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > > > > Cc: linux-rtc@vger.kernel.org
> > > > > To: devicetree@vger.kernel.org
> > > > > ---
> > > > > V2: - Add AB from Krzysztof
> > > > > V3: - No change
> > > > > ---
> > > > 
> > > > I just noticed this by accident. Basically everything in the patch
> > > > description is wrong:
> > > > 
> > > > 1. There is a in-tree user: arch/arm/boot/dts/imx6dl-qmx6.dtsi
> > > 
> > > Sorry, I missed this one.
> > > 
> > > > 2. The PMIC has nothing to do with this
> > > 
> > > In [3] the commit message claims the PMIC supplies 32kHz clock to i.MX6 CKIL,
> > > which per IMX6DQRM rev.6 Table 18-3 row SNVS indirectly supplies SNVS RTC.
> > > This reminded me of commit:
> > 
> > The word PMIC is not mentioned once in [3].
> 
> s@PMIC@m41t62 RTC@, sorry.
> 
> > PMIC is not involved.
> > The QMX6 32khz chain is like this:
> > 
> > 32kHz crystal -> m41t62 crystal input
> > m41t62 clock output -> i.MX6 CKIL
> > 
> > > 9509593f327ac ("arm64: dts: imx8mm: Model PMIC to SNVS RTC clock path on
> > > Data Modul i.MX8M Mini eDM SBC")
> > > 
> > > which solves exactly the same problem, system hangs when 32 kHz clock are
> > > stopped, except this time on i.MX8MM, clock are generated by PMIC on I2C
> > > (notice how the PMIC is referenced directly) and the clock are supplied to
> > > the SVNS RTC XTal terminals.
> > > 
> > > I wonder if this could be reused on the QMX6 board too?
> > 
> > IIRC On i.MX6 referencing the I2C connected RTC results in boot
> > hanging forever when trying to get the ckil clock in
> > imx6q_clocks_init. At least it used to be the case when I was
> > working on this - I no longer have access to the boards. Of course
> > properly referencing the RTC clock was the first route I tried.
> 
> Hmmmmm, what shall we do, un-deprecate the clock sub-node?

Depends on the exact meaning of "deprecate: true". I think we all
agree, that it's better to avoid the sub-node and only use it when
it's really required. But having a deprecation warning for an
in-tree user without a clear path forward also seems to be annoying.
I think it makes sense for the DT binding maintainers (Rob/Krzysztof)
to comment on this.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated
  2022-12-16 15:05           ` Sebastian Reichel
@ 2022-12-16 16:24             ` Alexandre Belloni
  2022-12-16 20:31               ` Marek Vasut
  0 siblings, 1 reply; 11+ messages in thread
From: Alexandre Belloni @ 2022-12-16 16:24 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Marek Vasut, devicetree, Krzysztof Kozlowski, Alessandro Zummo,
	Rob Herring, Krzysztof Kozlowski, linux-rtc

On 16/12/2022 16:05:48+0100, Sebastian Reichel wrote:
> > > IIRC On i.MX6 referencing the I2C connected RTC results in boot
> > > hanging forever when trying to get the ckil clock in
> > > imx6q_clocks_init. At least it used to be the case when I was
> > > working on this - I no longer have access to the boards. Of course
> > > properly referencing the RTC clock was the first route I tried.
> > 
> > Hmmmmm, what shall we do, un-deprecate the clock sub-node?
> 
> Depends on the exact meaning of "deprecate: true". I think we all
> agree, that it's better to avoid the sub-node and only use it when
> it's really required. But having a deprecation warning for an
> in-tree user without a clear path forward also seems to be annoying.
> I think it makes sense for the DT binding maintainers (Rob/Krzysztof)
> to comment on this.
> 

I dropped the commit from the PR I'm going to send Linus later today

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated
  2022-12-16 16:24             ` Alexandre Belloni
@ 2022-12-16 20:31               ` Marek Vasut
  0 siblings, 0 replies; 11+ messages in thread
From: Marek Vasut @ 2022-12-16 20:31 UTC (permalink / raw)
  To: Alexandre Belloni, Sebastian Reichel
  Cc: devicetree, Krzysztof Kozlowski, Alessandro Zummo, Rob Herring,
	Krzysztof Kozlowski, linux-rtc

On 12/16/22 17:24, Alexandre Belloni wrote:
> On 16/12/2022 16:05:48+0100, Sebastian Reichel wrote:
>>>> IIRC On i.MX6 referencing the I2C connected RTC results in boot
>>>> hanging forever when trying to get the ckil clock in
>>>> imx6q_clocks_init. At least it used to be the case when I was
>>>> working on this - I no longer have access to the boards. Of course
>>>> properly referencing the RTC clock was the first route I tried.
>>>
>>> Hmmmmm, what shall we do, un-deprecate the clock sub-node?
>>
>> Depends on the exact meaning of "deprecate: true". I think we all
>> agree, that it's better to avoid the sub-node and only use it when
>> it's really required. But having a deprecation warning for an
>> in-tree user without a clear path forward also seems to be annoying.
>> I think it makes sense for the DT binding maintainers (Rob/Krzysztof)
>> to comment on this.
>>
> 
> I dropped the commit from the PR I'm going to send Linus later today

That works, thank you !

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-12-16 20:31 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-11 20:51 [PATCH v3 1/2] dt-bindings: rtc: m41t80: Convert text schema to YAML one Marek Vasut
2022-12-11 20:51 ` [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated Marek Vasut
2022-12-15 18:06   ` Sebastian Reichel
2022-12-15 19:39     ` Marek Vasut
2022-12-16 14:24       ` Sebastian Reichel
2022-12-16 14:36         ` Marek Vasut
2022-12-16 15:05           ` Sebastian Reichel
2022-12-16 16:24             ` Alexandre Belloni
2022-12-16 20:31               ` Marek Vasut
2022-12-12  8:57 ` [PATCH v3 1/2] dt-bindings: rtc: m41t80: Convert text schema to YAML one Krzysztof Kozlowski
2022-12-12  9:25 ` Alexandre Belloni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).