[PATCH 0/3] Updates for simple-pm-bus and ti-sysc bindings

[PATCH 0/3] Updates for simple-pm-bus and ti-sysc bindings

[Tony Lindgren]

Hi,

Turns out we added a bunch of new devicetree warnings for omaps when we
updated the dts files to use simple-pm-bus. Here are the binding changes
to fix most of the introduced simple-pm-bus related warnings.

I'll post the dts changes separately as we have ranges missing for some
interconnect segments, and can also now rename the ocp node.

Regards,

Tony


Tony Lindgren (3):
  dt-bindings: bus: simple-pm-bus: Make clocks and power-domains
    optional
  dt-bindings: bus: simple-pm-bus: Add more matches for node name
  dt-bindings: bus: ti-sysc: Update to use yaml binding

 .../bindings/bus/simple-pm-bus.yaml           |  19 +--
 .../devicetree/bindings/bus/ti-sysc.txt       | 139 ----------------
 .../devicetree/bindings/bus/ti-sysc.yaml      | 150 ++++++++++++++++++
 3 files changed, 156 insertions(+), 152 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/bus/ti-sysc.txt
 create mode 100644 Documentation/devicetree/bindings/bus/ti-sysc.yaml

-- 
2.33.0

[PATCH 1/3] dt-bindings: bus: simple-pm-bus: Make clocks and power-domains optional

[Tony Lindgren]

Clocks and power domains are not required by the simple-pm-bus driver.
There are buses with read-only registers for clocks and power domains
that are always on.

Even without clocks and power domains configured, simple-pm-bus is still
different from simple-bus as simple-pm-bus enables runtime PM for the bus
driver.

Let's update the binding accordingly as this remove the related warnings
for dt_binding_check for omaps.

Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Rob Herring <robh@kernel.org>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Suman Anna <s-anna@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 .../devicetree/bindings/bus/simple-pm-bus.yaml  | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml b/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
--- a/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
+++ b/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
@@ -13,10 +13,9 @@ description: |
   A Simple Power-Managed Bus is a transparent bus that doesn't need a real
   driver, as it's typically initialized by the boot loader.
 
-  However, its bus controller is part of a PM domain, or under the control
-  of a functional clock.  Hence, the bus controller's PM domain and/or
-  clock must be enabled for child devices connected to the bus (either
-  on-SoC or externally) to function.
+  However, its bus controller is typically part of a PM domain, or under
+  the control of a functional clock.  Without PM domain or functional clock,
+  it still enables runtime PM for the bus driver unlike "simple-bus".
 
   While "simple-pm-bus" follows the "simple-bus" set of properties, as
   specified in the Devicetree Specification, it is not an extension of
@@ -43,10 +42,10 @@ properties:
 
   clocks: true
     # Functional clocks
-    # Required if power-domains is absent, optional otherwise
+    # Typically used if power-domains is absent
 
   power-domains:
-    # Required if clocks is absent, optional otherwise
+    # Typically used if clocks is absent
     minItems: 1
 
 required:
@@ -55,12 +54,6 @@ required:
   - '#size-cells'
   - ranges
 
-anyOf:
-  - required:
-      - clocks
-  - required:
-      - power-domains
-
 additionalProperties: true
 
 examples:
-- 
2.33.0

Re: [PATCH 1/3] dt-bindings: bus: simple-pm-bus: Make clocks and power-domains optional

[Geert Uytterhoeven]

Hi Tony,

Thanks for your patch!

On Thu, Oct 7, 2021 at 2:49 PM Tony Lindgren <tony@atomide.com> wrote:
> Clocks and power domains are not required by the simple-pm-bus driver.
> There are buses with read-only registers for clocks and power domains
> that are always on.

The presence of clocks or power-domains properties is the only
distinguishing factor between simple-pm-bus and simple-bus, from a
DT point of view.  So if there has to be a distinguishment, the
properties should be required

If you don't have clocks and power-domains, you should use simple-bus.

> Even without clocks and power domains configured, simple-pm-bus is still
> different from simple-bus as simple-pm-bus enables runtime PM for the bus
> driver.

Which you need to have working Runtime PM for child devices, right? ;-)

This is not specific to DT, but to Linux.
One more reason to let Linux treat simple-pm-bus and simple-bus exactly
the same.  Linux handles the clocks and power-domains (if present)
transparently anyway, through PM Domains

> Let's update the binding accordingly as this remove the related warnings
> for dt_binding_check for omaps.
>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> Cc: Suman Anna <s-anna@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>

> --- a/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
> +++ b/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
> @@ -13,10 +13,9 @@ description: |
>    A Simple Power-Managed Bus is a transparent bus that doesn't need a real
>    driver, as it's typically initialized by the boot loader.
>
> -  However, its bus controller is part of a PM domain, or under the control
> -  of a functional clock.  Hence, the bus controller's PM domain and/or
> -  clock must be enabled for child devices connected to the bus (either
> -  on-SoC or externally) to function.
> +  However, its bus controller is typically part of a PM domain, or under
> +  the control of a functional clock.  Without PM domain or functional clock,
> +  it still enables runtime PM for the bus driver unlike "simple-bus".
>
>    While "simple-pm-bus" follows the "simple-bus" set of properties, as
>    specified in the Devicetree Specification, it is not an extension of
> @@ -43,10 +42,10 @@ properties:
>
>    clocks: true
>      # Functional clocks
> -    # Required if power-domains is absent, optional otherwise
> +    # Typically used if power-domains is absent
>
>    power-domains:
> -    # Required if clocks is absent, optional otherwise
> +    # Typically used if clocks is absent
>      minItems: 1
>
>  required:
> @@ -55,12 +54,6 @@ required:
>    - '#size-cells'
>    - ranges
>
> -anyOf:
> -  - required:
> -      - clocks
> -  - required:
> -      - power-domains
> -
>  additionalProperties: true
>
>  examples:

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] dt-bindings: bus: simple-pm-bus: Make clocks and power-domains optional

[Tony Lindgren]

* Geert Uytterhoeven <geert@linux-m68k.org> [211007 13:27]:
> Hi Tony,
> 
> Thanks for your patch!
> 
> On Thu, Oct 7, 2021 at 2:49 PM Tony Lindgren <tony@atomide.com> wrote:
> > Clocks and power domains are not required by the simple-pm-bus driver.
> > There are buses with read-only registers for clocks and power domains
> > that are always on.
> 
> The presence of clocks or power-domains properties is the only
> distinguishing factor between simple-pm-bus and simple-bus, from a
> DT point of view.  So if there has to be a distinguishment, the
> properties should be required

Heh seems there is no need for distinguishment beyond the compatible
property here though :)

> If you don't have clocks and power-domains, you should use simple-bus.

Except simple-bus is not the same as simple-pm-bus. We do not have
simple-bus do pm_runtime_enable() as you well know having written it :)

> > Even without clocks and power domains configured, simple-pm-bus is still
> > different from simple-bus as simple-pm-bus enables runtime PM for the bus
> > driver.
> 
> Which you need to have working Runtime PM for child devices, right? ;-)

Right. And based on what I remember we simply cannot do pm_runtime_enable()
for simple-bus without breaking tons of devices.

> This is not specific to DT, but to Linux.
> One more reason to let Linux treat simple-pm-bus and simple-bus exactly
> the same.  Linux handles the clocks and power-domains (if present)
> transparently anyway, through PM Domains

I agree they should be treated the same way with simple-pm-bus just
doing the pm_runtime_enable() being the only difference.

But the clocks and power domain still should be optional. They are
not required by simple-pm-bus.c driver, and may not be required by
the hardware.

Got any better solutions in mind? Adding yet another compatible or
another driver does not seem to get us anywhere either with this :)

Regards,

Tony

Re: [PATCH 1/3] dt-bindings: bus: simple-pm-bus: Make clocks and power-domains optional

[Geert Uytterhoeven]

Hi Tony,

On Thu, Oct 7, 2021 at 7:24 PM Tony Lindgren <tony@atomide.com> wrote:
> * Geert Uytterhoeven <geert@linux-m68k.org> [211007 13:27]:
> > On Thu, Oct 7, 2021 at 2:49 PM Tony Lindgren <tony@atomide.com> wrote:
> > > Even without clocks and power domains configured, simple-pm-bus is still
> > > different from simple-bus as simple-pm-bus enables runtime PM for the bus
> > > driver.
> >
> > Which you need to have working Runtime PM for child devices, right? ;-)
>
> Right. And based on what I remember we simply cannot do pm_runtime_enable()
> for simple-bus without breaking tons of devices.

Why not? Do you have an example of what would break?
The only reason I created simple-pm-bus was because the DT people
objected to adding PM to simple-bus, as they considered it wrong
conceptually.  AFAIK this wouldn't have caused any actual breakage.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] dt-bindings: bus: simple-pm-bus: Make clocks and power-domains optional

[Tony Lindgren]

* Geert Uytterhoeven <geert@linux-m68k.org> [211007 17:57]:
> Hi Tony,
> 
> On Thu, Oct 7, 2021 at 7:24 PM Tony Lindgren <tony@atomide.com> wrote:
> > * Geert Uytterhoeven <geert@linux-m68k.org> [211007 13:27]:
> > > On Thu, Oct 7, 2021 at 2:49 PM Tony Lindgren <tony@atomide.com> wrote:
> > > > Even without clocks and power domains configured, simple-pm-bus is still
> > > > different from simple-bus as simple-pm-bus enables runtime PM for the bus
> > > > driver.
> > >
> > > Which you need to have working Runtime PM for child devices, right? ;-)
> >
> > Right. And based on what I remember we simply cannot do pm_runtime_enable()
> > for simple-bus without breaking tons of devices.
> 
> Why not? Do you have an example of what would break?
> The only reason I created simple-pm-bus was because the DT people
> objected to adding PM to simple-bus, as they considered it wrong
> conceptually.  AFAIK this wouldn't have caused any actual breakage.

Oh OK, then I just remember the reasons wrong for the need for
adding it as a driver.

Regards,

Tony

[PATCH 2/3] dt-bindings: bus: simple-pm-bus: Add more matches for node name

[Tony Lindgren]

At least omaps use simple-pm-bus for interconnects and interconnect
segments. While the interconnects do have configurable registers, there is
currently no need for a dedicated interconnect driver.

Let's update the list of allowed node names to remove the related
dt_binding_check warnings for omaps.

Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Rob Herring <robh@kernel.org>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Suman Anna <s-anna@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 Documentation/devicetree/bindings/bus/simple-pm-bus.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml b/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
--- a/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
+++ b/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
@@ -23,7 +23,7 @@ description: |
 
 properties:
   $nodename:
-    pattern: "^bus(@[0-9a-f]+)?$"
+    pattern: "^(bus|interconnect|segment)(@[0-9a-f]+)?$"
 
   compatible:
     contains:
-- 
2.33.0

[PATCH 3/3] dt-bindings: bus: ti-sysc: Update to use yaml binding

[Tony Lindgren]

Update the binding for ti-sysc interconnect target module driver to yaml
format.

Cc: Rob Herring <robh@kernel.org>
Cc: Suman Anna <s-anna@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 .../devicetree/bindings/bus/ti-sysc.txt       | 139 ----------------
 .../devicetree/bindings/bus/ti-sysc.yaml      | 150 ++++++++++++++++++
 2 files changed, 150 insertions(+), 139 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/bus/ti-sysc.txt
 create mode 100644 Documentation/devicetree/bindings/bus/ti-sysc.yaml

diff --git a/Documentation/devicetree/bindings/bus/ti-sysc.txt b/Documentation/devicetree/bindings/bus/ti-sysc.txt
deleted file mode 100644
--- a/Documentation/devicetree/bindings/bus/ti-sysc.txt
+++ /dev/null
@@ -1,139 +0,0 @@
-Texas Instruments sysc interconnect target module wrapper binding
-
-Texas Instruments SoCs can have a generic interconnect target module
-hardware for devices connected to various interconnects such as L3
-interconnect (Arteris NoC) and L4 interconnect (Sonics s3220). The sysc
-is mostly used for interaction between module and PRCM. It participates
-in the OCP Disconnect Protocol but other than that is mostly independent
-of the interconnect.
-
-Each interconnect target module can have one or more devices connected to
-it. There is a set of control registers for managing interconnect target
-module clocks, idle modes and interconnect level resets for the module.
-
-These control registers are sprinkled into the unused register address
-space of the first child device IP block managed by the interconnect
-target module and typically are named REVISION, SYSCONFIG and SYSSTATUS.
-
-Required standard properties:
-
-- compatible	shall be one of the following generic types:
-
-		"ti,sysc"
-		"ti,sysc-omap2"
-		"ti,sysc-omap4"
-		"ti,sysc-omap4-simple"
-
-		or one of the following derivative types for hardware
-		needing special workarounds:
-
-		"ti,sysc-omap2-timer"
-		"ti,sysc-omap4-timer"
-		"ti,sysc-omap3430-sr"
-		"ti,sysc-omap3630-sr"
-		"ti,sysc-omap4-sr"
-		"ti,sysc-omap3-sham"
-		"ti,sysc-omap-aes"
-		"ti,sysc-mcasp"
-		"ti,sysc-dra7-mcasp"
-		"ti,sysc-usb-host-fs"
-		"ti,sysc-dra7-mcan"
-		"ti,sysc-pruss"
-
-- reg		shall have register areas implemented for the interconnect
-		target module in question such as revision, sysc and syss
-
-- reg-names	shall contain the register names implemented for the
-		interconnect target module in question such as
-		"rev, "sysc", and "syss"
-
-- ranges	shall contain the interconnect target module IO range
-		available for one or more child device IP blocks managed
-		by the interconnect target module, the ranges may include
-		multiple ranges such as device L4 range for control and
-		parent L3 range for DMA access
-
-Optional properties:
-
-- ti,sysc-mask	shall contain mask of supported register bits for the
-		SYSCONFIG register as documented in the Technical Reference
-		Manual (TRM) for the interconnect target module
-
-- ti,sysc-midle	list of master idle modes supported by the interconnect
-		target module as documented in the TRM for SYSCONFIG
-		register MIDLEMODE bits
-
-- ti,sysc-sidle	list of slave idle modes supported by the interconnect
-		target module as documented in the TRM for SYSCONFIG
-		register SIDLEMODE bits
-
-- ti,sysc-delay-us	delay needed after OCP softreset before accssing
-			SYSCONFIG register again
-
-- ti,syss-mask	optional mask of reset done status bits as described in the
-		TRM for SYSSTATUS registers, typically 1 with some devices
-		having separate reset done bits for children like OHCI and
-		EHCI
-
-- clocks	clock specifier for each name in the clock-names as
-		specified in the binding documentation for ti-clkctrl,
-		typically available for all interconnect targets on TI SoCs
-		based on omap4 except if it's read-only register in hwauto
-		mode as for example omap4 L4_CFG_CLKCTRL
-
-- clock-names	should contain at least "fck", and optionally also "ick"
-		depending on the SoC and the interconnect target module,
-		some interconnect target modules also need additional
-		optional clocks that can be specified as listed in TRM
-		for the related CLKCTRL register bits 8 to 15 such as
-		"dbclk" or "clk32k" depending on their role
-
-- ti,hwmods	optional TI interconnect module name to use legacy
-		hwmod platform data
-
-- ti,no-reset-on-init	interconnect target module should not be reset at init
-
-- ti,no-idle-on-init	interconnect target module should not be idled at init
-
-- ti,no-idle		interconnect target module should not be idled
-
-Example: Single instance of MUSB controller on omap4 using interconnect ranges
-using offsets from l4_cfg second segment (0x4a000000 + 0x80000 = 0x4a0ab000):
-
-	target-module@2b000 {		/* 0x4a0ab000, ap 84 12.0 */
-		compatible = "ti,sysc-omap2";
-		ti,hwmods = "usb_otg_hs";
-		reg = <0x2b400 0x4>,
-		      <0x2b404 0x4>,
-		      <0x2b408 0x4>;
-		reg-names = "rev", "sysc", "syss";
-		clocks = <&l3_init_clkctrl OMAP4_USB_OTG_HS_CLKCTRL 0>;
-		clock-names = "fck";
-		ti,sysc-mask = <(SYSC_OMAP2_ENAWAKEUP |
-				 SYSC_OMAP2_SOFTRESET |
-				 SYSC_OMAP2_AUTOIDLE)>;
-		ti,sysc-midle = <SYSC_IDLE_FORCE>,
-				<SYSC_IDLE_NO>,
-				<SYSC_IDLE_SMART>;
-		ti,sysc-sidle = <SYSC_IDLE_FORCE>,
-				<SYSC_IDLE_NO>,
-				<SYSC_IDLE_SMART>,
-				<SYSC_IDLE_SMART_WKUP>;
-		ti,syss-mask = <1>;
-		#address-cells = <1>;
-		#size-cells = <1>;
-		ranges = <0 0x2b000 0x1000>;
-
-		usb_otg_hs: otg@0 {
-			compatible = "ti,omap4-musb";
-			reg = <0x0 0x7ff>;
-			interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
-			usb-phy = <&usb2_phy>;
-			...
-		};
-	};
-
-Note that other SoCs, such as am335x can have multiple child devices. On am335x
-there are two MUSB instances, two USB PHY instances, and a single CPPI41 DMA
-instance as children of a single interconnect target module.
diff --git a/Documentation/devicetree/bindings/bus/ti-sysc.yaml b/Documentation/devicetree/bindings/bus/ti-sysc.yaml
new file mode 100644
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/ti-sysc.yaml
@@ -0,0 +1,150 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/ti-sysc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments interconnect target module binding
+
+maintainers:
+  - Tony Lindgren <tony@atomide.com>
+
+description: |
+  Texas Instruments SoCs can have a generic interconnect target module
+  for devices connected to various interconnects such as L3 interconnect
+  using Arteris NoC, and L4 interconnect using Sonics s3220. This module
+  is mostly used for interaction between module and Power, Reset and Clock
+  Manager PRCM. It participates in the OCP Disconnect Protocol, but other
+  than that it is mostly independent of the interconnect.
+
+  Each interconnect target module can have one or more devices connected to
+  it. There is a set of control registers for managing the interconnect target
+  module clocks, idle modes and interconnect level resets.
+
+  The interconnect target module control registers are sprinkled into the
+  unused register address space of the first child device IP block managed by
+  the interconnect target module. Typically the register names are REVISION,
+  SYSCONFIG and SYSSTATUS.
+
+properties:
+  $nodename:
+    pattern: "^target-module(@[0-9a-f]+)?$"
+
+  compatible:
+    oneOf:
+      - items:
+        - enum:
+            - ti,sysc-omap2
+            - ti,sysc-omap2
+            - ti,sysc-omap4
+            - ti,sysc-omap4-simple
+            - ti,sysc-omap2-timer
+            - ti,sysc-omap4-timer
+            - ti,sysc-omap3430-sr
+            - ti,sysc-omap3630-sr
+            - ti,sysc-omap4-sr
+            - ti,sysc-omap3-sham
+            - ti,sysc-omap-aes
+            - ti,sysc-mcasp
+            - ti,sysc-dra7-mcasp
+            - ti,sysc-usb-host-fs
+            - ti,sysc-dra7-mcan
+            - ti,sysc-pruss
+        - const: ti,sysc
+      - items:
+        - const: ti,sysc
+
+  reg: true
+
+  reg-names: true
+
+  clocks: true
+
+  clock-names: true
+
+  power-domains: true
+
+  '#address-cells':
+    enum: [ 1, 2 ]
+
+  '#size-cells':
+    enum: [ 1, 2 ]
+
+  ranges: true
+
+  ti,sysc-mask:
+    description: Mask of supported register bits for the SYSCONFIG register
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  ti,sysc-midle:
+    description: List of hardware supported idle modes
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  ti,sysc-sidle:
+    description: List of hardware supported idle modes
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  ti,syss-mask:
+    description: Mask of supported register bits for the SYSSTATUS register
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  ti,sysc-delay-us:
+    description: Delay needed after OCP softreset before accessing SYCONFIG
+    default: 0
+    minimum: 0
+    maximum: 2
+
+  ti,no-reset-on-init:
+    description: Interconnect target module shall not be reset at init
+    type: boolean
+
+  ti,no-idle-on-init:
+    description: Interconnect target module shall not be idled at init
+    type: boolean
+
+  ti,no-idle:
+    description: Interconnect target module shall not be idled
+    type: boolean
+
+  ti,hwmods:
+    description: Interconnect module name to use with legacy hwmod data
+    $ref: /schemas/types.yaml#/definitions/string
+    deprecated: true
+
+required:
+  - compatible
+  - '#address-cells'
+  - '#size-cells'
+  - ranges
+
+additionalProperties: true
+
+examples:
+  - |
+    #include <dt-bindings/bus/ti-sysc.h>
+    #include <dt-bindings/clock/omap4.h>
+
+    target-module@2b000 {
+        compatible = "ti,sysc-omap2", "ti,sysc";
+        ti,hwmods = "usb_otg_hs";
+        reg = <0x2b400 0x4>,
+              <0x2b404 0x4>,
+              <0x2b408 0x4>;
+        reg-names = "rev", "sysc", "syss";
+        clocks = <&l3_init_clkctrl OMAP4_USB_OTG_HS_CLKCTRL 0>;
+        clock-names = "fck";
+        ti,sysc-mask = <(SYSC_OMAP2_ENAWAKEUP |
+                         SYSC_OMAP2_SOFTRESET |
+                         SYSC_OMAP2_AUTOIDLE)>;
+        ti,sysc-midle = <SYSC_IDLE_FORCE>,
+                        <SYSC_IDLE_NO>,
+                        <SYSC_IDLE_SMART>;
+        ti,sysc-sidle = <SYSC_IDLE_FORCE>,
+                        <SYSC_IDLE_NO>,
+                        <SYSC_IDLE_SMART>,
+                        <SYSC_IDLE_SMART_WKUP>;
+        ti,syss-mask = <1>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges = <0 0x2b000 0x1000>;
+    };
-- 
2.33.0

Re: [PATCH 3/3] dt-bindings: bus: ti-sysc: Update to use yaml binding

[Rob Herring]

On Thu, 07 Oct 2021 15:48:58 +0300, Tony Lindgren wrote:
> Update the binding for ti-sysc interconnect target module driver to yaml
> format.
> 
> Cc: Rob Herring <robh@kernel.org>
> Cc: Suman Anna <s-anna@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  .../devicetree/bindings/bus/ti-sysc.txt       | 139 ----------------
>  .../devicetree/bindings/bus/ti-sysc.yaml      | 150 ++++++++++++++++++
>  2 files changed, 150 insertions(+), 139 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/bus/ti-sysc.txt
>  create mode 100644 Documentation/devicetree/bindings/bus/ti-sysc.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/bus/ti-sysc.yaml:36:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/bus/ti-sysc.yaml:55:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1537683

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 3/3] dt-bindings: bus: ti-sysc: Update to use yaml binding

[Rob Herring]

On Thu, Oct 07, 2021 at 03:48:58PM +0300, Tony Lindgren wrote:
> Update the binding for ti-sysc interconnect target module driver to yaml
> format.
> 
> Cc: Rob Herring <robh@kernel.org>
> Cc: Suman Anna <s-anna@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  .../devicetree/bindings/bus/ti-sysc.txt       | 139 ----------------
>  .../devicetree/bindings/bus/ti-sysc.yaml      | 150 ++++++++++++++++++
>  2 files changed, 150 insertions(+), 139 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/bus/ti-sysc.txt
>  create mode 100644 Documentation/devicetree/bindings/bus/ti-sysc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/bus/ti-sysc.txt b/Documentation/devicetree/bindings/bus/ti-sysc.txt
> deleted file mode 100644
> --- a/Documentation/devicetree/bindings/bus/ti-sysc.txt
> +++ /dev/null
> @@ -1,139 +0,0 @@
> -Texas Instruments sysc interconnect target module wrapper binding
> -
> -Texas Instruments SoCs can have a generic interconnect target module
> -hardware for devices connected to various interconnects such as L3
> -interconnect (Arteris NoC) and L4 interconnect (Sonics s3220). The sysc
> -is mostly used for interaction between module and PRCM. It participates
> -in the OCP Disconnect Protocol but other than that is mostly independent
> -of the interconnect.
> -
> -Each interconnect target module can have one or more devices connected to
> -it. There is a set of control registers for managing interconnect target
> -module clocks, idle modes and interconnect level resets for the module.
> -
> -These control registers are sprinkled into the unused register address
> -space of the first child device IP block managed by the interconnect
> -target module and typically are named REVISION, SYSCONFIG and SYSSTATUS.
> -
> -Required standard properties:
> -
> -- compatible	shall be one of the following generic types:
> -
> -		"ti,sysc"
> -		"ti,sysc-omap2"
> -		"ti,sysc-omap4"
> -		"ti,sysc-omap4-simple"
> -
> -		or one of the following derivative types for hardware
> -		needing special workarounds:
> -
> -		"ti,sysc-omap2-timer"
> -		"ti,sysc-omap4-timer"
> -		"ti,sysc-omap3430-sr"
> -		"ti,sysc-omap3630-sr"
> -		"ti,sysc-omap4-sr"
> -		"ti,sysc-omap3-sham"
> -		"ti,sysc-omap-aes"
> -		"ti,sysc-mcasp"
> -		"ti,sysc-dra7-mcasp"
> -		"ti,sysc-usb-host-fs"
> -		"ti,sysc-dra7-mcan"
> -		"ti,sysc-pruss"
> -
> -- reg		shall have register areas implemented for the interconnect
> -		target module in question such as revision, sysc and syss
> -
> -- reg-names	shall contain the register names implemented for the
> -		interconnect target module in question such as
> -		"rev, "sysc", and "syss"
> -
> -- ranges	shall contain the interconnect target module IO range
> -		available for one or more child device IP blocks managed
> -		by the interconnect target module, the ranges may include
> -		multiple ranges such as device L4 range for control and
> -		parent L3 range for DMA access
> -
> -Optional properties:
> -
> -- ti,sysc-mask	shall contain mask of supported register bits for the
> -		SYSCONFIG register as documented in the Technical Reference
> -		Manual (TRM) for the interconnect target module
> -
> -- ti,sysc-midle	list of master idle modes supported by the interconnect
> -		target module as documented in the TRM for SYSCONFIG
> -		register MIDLEMODE bits
> -
> -- ti,sysc-sidle	list of slave idle modes supported by the interconnect
> -		target module as documented in the TRM for SYSCONFIG
> -		register SIDLEMODE bits
> -
> -- ti,sysc-delay-us	delay needed after OCP softreset before accssing
> -			SYSCONFIG register again
> -
> -- ti,syss-mask	optional mask of reset done status bits as described in the
> -		TRM for SYSSTATUS registers, typically 1 with some devices
> -		having separate reset done bits for children like OHCI and
> -		EHCI
> -
> -- clocks	clock specifier for each name in the clock-names as
> -		specified in the binding documentation for ti-clkctrl,
> -		typically available for all interconnect targets on TI SoCs
> -		based on omap4 except if it's read-only register in hwauto
> -		mode as for example omap4 L4_CFG_CLKCTRL
> -
> -- clock-names	should contain at least "fck", and optionally also "ick"
> -		depending on the SoC and the interconnect target module,
> -		some interconnect target modules also need additional
> -		optional clocks that can be specified as listed in TRM
> -		for the related CLKCTRL register bits 8 to 15 such as
> -		"dbclk" or "clk32k" depending on their role
> -
> -- ti,hwmods	optional TI interconnect module name to use legacy
> -		hwmod platform data
> -
> -- ti,no-reset-on-init	interconnect target module should not be reset at init
> -
> -- ti,no-idle-on-init	interconnect target module should not be idled at init
> -
> -- ti,no-idle		interconnect target module should not be idled
> -
> -Example: Single instance of MUSB controller on omap4 using interconnect ranges
> -using offsets from l4_cfg second segment (0x4a000000 + 0x80000 = 0x4a0ab000):
> -
> -	target-module@2b000 {		/* 0x4a0ab000, ap 84 12.0 */
> -		compatible = "ti,sysc-omap2";
> -		ti,hwmods = "usb_otg_hs";
> -		reg = <0x2b400 0x4>,
> -		      <0x2b404 0x4>,
> -		      <0x2b408 0x4>;
> -		reg-names = "rev", "sysc", "syss";
> -		clocks = <&l3_init_clkctrl OMAP4_USB_OTG_HS_CLKCTRL 0>;
> -		clock-names = "fck";
> -		ti,sysc-mask = <(SYSC_OMAP2_ENAWAKEUP |
> -				 SYSC_OMAP2_SOFTRESET |
> -				 SYSC_OMAP2_AUTOIDLE)>;
> -		ti,sysc-midle = <SYSC_IDLE_FORCE>,
> -				<SYSC_IDLE_NO>,
> -				<SYSC_IDLE_SMART>;
> -		ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> -				<SYSC_IDLE_NO>,
> -				<SYSC_IDLE_SMART>,
> -				<SYSC_IDLE_SMART_WKUP>;
> -		ti,syss-mask = <1>;
> -		#address-cells = <1>;
> -		#size-cells = <1>;
> -		ranges = <0 0x2b000 0x1000>;
> -
> -		usb_otg_hs: otg@0 {
> -			compatible = "ti,omap4-musb";
> -			reg = <0x0 0x7ff>;
> -			interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>,
> -				     <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
> -			usb-phy = <&usb2_phy>;
> -			...
> -		};
> -	};
> -
> -Note that other SoCs, such as am335x can have multiple child devices. On am335x
> -there are two MUSB instances, two USB PHY instances, and a single CPPI41 DMA
> -instance as children of a single interconnect target module.
> diff --git a/Documentation/devicetree/bindings/bus/ti-sysc.yaml b/Documentation/devicetree/bindings/bus/ti-sysc.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/ti-sysc.yaml
> @@ -0,0 +1,150 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/ti-sysc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments interconnect target module binding
> +
> +maintainers:
> +  - Tony Lindgren <tony@atomide.com>
> +
> +description: |
> +  Texas Instruments SoCs can have a generic interconnect target module
> +  for devices connected to various interconnects such as L3 interconnect
> +  using Arteris NoC, and L4 interconnect using Sonics s3220. This module
> +  is mostly used for interaction between module and Power, Reset and Clock
> +  Manager PRCM. It participates in the OCP Disconnect Protocol, but other
> +  than that it is mostly independent of the interconnect.
> +
> +  Each interconnect target module can have one or more devices connected to
> +  it. There is a set of control registers for managing the interconnect target
> +  module clocks, idle modes and interconnect level resets.
> +
> +  The interconnect target module control registers are sprinkled into the
> +  unused register address space of the first child device IP block managed by
> +  the interconnect target module. Typically the register names are REVISION,
> +  SYSCONFIG and SYSSTATUS.
> +
> +properties:
> +  $nodename:
> +    pattern: "^target-module(@[0-9a-f]+)?$"
> +
> +  compatible:
> +    oneOf:
> +      - items:
> +        - enum:
> +            - ti,sysc-omap2
> +            - ti,sysc-omap2
> +            - ti,sysc-omap4
> +            - ti,sysc-omap4-simple
> +            - ti,sysc-omap2-timer
> +            - ti,sysc-omap4-timer
> +            - ti,sysc-omap3430-sr
> +            - ti,sysc-omap3630-sr
> +            - ti,sysc-omap4-sr
> +            - ti,sysc-omap3-sham
> +            - ti,sysc-omap-aes
> +            - ti,sysc-mcasp
> +            - ti,sysc-dra7-mcasp
> +            - ti,sysc-usb-host-fs
> +            - ti,sysc-dra7-mcan
> +            - ti,sysc-pruss
> +        - const: ti,sysc
> +      - items:
> +        - const: ti,sysc

This doesn't really match what the old doc said nor the old example. 
Fine to fix in the conversion if wrong, but just highlight that in the 
commit msg.

> +
> +  reg: true

Any number of register areas is valid?

> +
> +  reg-names: true

You've thrown out the names defined before.

> +
> +  clocks: true

Any number of clocks is valid? 

> +
> +  clock-names: true

You've thrown out the names defined before.

> +
> +  power-domains: true

How many?

> +
> +  '#address-cells':
> +    enum: [ 1, 2 ]
> +
> +  '#size-cells':
> +    enum: [ 1, 2 ]
> +
> +  ranges: true
> +
> +  ti,sysc-mask:
> +    description: Mask of supported register bits for the SYSCONFIG register
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  ti,sysc-midle:
> +    description: List of hardware supported idle modes
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +  ti,sysc-sidle:
> +    description: List of hardware supported idle modes
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +  ti,syss-mask:
> +    description: Mask of supported register bits for the SYSSTATUS register
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  ti,sysc-delay-us:
> +    description: Delay needed after OCP softreset before accessing SYCONFIG
> +    default: 0
> +    minimum: 0
> +    maximum: 2
> +
> +  ti,no-reset-on-init:
> +    description: Interconnect target module shall not be reset at init
> +    type: boolean
> +
> +  ti,no-idle-on-init:
> +    description: Interconnect target module shall not be idled at init
> +    type: boolean
> +
> +  ti,no-idle:
> +    description: Interconnect target module shall not be idled
> +    type: boolean
> +
> +  ti,hwmods:
> +    description: Interconnect module name to use with legacy hwmod data
> +    $ref: /schemas/types.yaml#/definitions/string
> +    deprecated: true
> +
> +required:
> +  - compatible
> +  - '#address-cells'
> +  - '#size-cells'
> +  - ranges
> +
> +additionalProperties: true

This can be restricted to child nodes only? If so:

additionalProperties:
  type: object

> +
> +examples:
> +  - |
> +    #include <dt-bindings/bus/ti-sysc.h>
> +    #include <dt-bindings/clock/omap4.h>
> +
> +    target-module@2b000 {
> +        compatible = "ti,sysc-omap2", "ti,sysc";
> +        ti,hwmods = "usb_otg_hs";
> +        reg = <0x2b400 0x4>,
> +              <0x2b404 0x4>,
> +              <0x2b408 0x4>;
> +        reg-names = "rev", "sysc", "syss";
> +        clocks = <&l3_init_clkctrl OMAP4_USB_OTG_HS_CLKCTRL 0>;
> +        clock-names = "fck";
> +        ti,sysc-mask = <(SYSC_OMAP2_ENAWAKEUP |
> +                         SYSC_OMAP2_SOFTRESET |
> +                         SYSC_OMAP2_AUTOIDLE)>;
> +        ti,sysc-midle = <SYSC_IDLE_FORCE>,
> +                        <SYSC_IDLE_NO>,
> +                        <SYSC_IDLE_SMART>;
> +        ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> +                        <SYSC_IDLE_NO>,
> +                        <SYSC_IDLE_SMART>,
> +                        <SYSC_IDLE_SMART_WKUP>;
> +        ti,syss-mask = <1>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges = <0 0x2b000 0x1000>;
> +    };
> -- 
> 2.33.0
> 

Re: [PATCH 3/3] dt-bindings: bus: ti-sysc: Update to use yaml binding

[Tony Lindgren]

Hi,

* Rob Herring <robh@kernel.org> [211008 20:39]:
> On Thu, Oct 07, 2021 at 03:48:58PM +0300, Tony Lindgren wrote:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +        - enum:
> > +            - ti,sysc-omap2
> > +            - ti,sysc-omap2
> > +            - ti,sysc-omap4
> > +            - ti,sysc-omap4-simple
> > +            - ti,sysc-omap2-timer
> > +            - ti,sysc-omap4-timer
> > +            - ti,sysc-omap3430-sr
> > +            - ti,sysc-omap3630-sr
> > +            - ti,sysc-omap4-sr
> > +            - ti,sysc-omap3-sham
> > +            - ti,sysc-omap-aes
> > +            - ti,sysc-mcasp
> > +            - ti,sysc-dra7-mcasp
> > +            - ti,sysc-usb-host-fs
> > +            - ti,sysc-dra7-mcan
> > +            - ti,sysc-pruss
> > +        - const: ti,sysc
> > +      - items:
> > +        - const: ti,sysc
> 
> This doesn't really match what the old doc said nor the old example. 
> Fine to fix in the conversion if wrong, but just highlight that in the 
> commit msg.

OK yes we currently always need "ti,sysc" as the clockdomain autoidle
handling still relies on auxdata.

> > +
> > +  reg: true
> 
> Any number of register areas is valid?

Yes the number of registers is known, I'll update to use max three reg.

> > +
> > +  reg-names: true
> 
> You've thrown out the names defined before.

Yup these are known too.

> > +
> > +  clocks: true
> 
> Any number of clocks is valid? 

I think 8 clocks is the highest seen :)

> > +  clock-names: true
> 
> You've thrown out the names defined before.

Hmm fck and ick can be named, the rest are device specific though.

> > +
> > +  power-domains: true
> 
> How many?

Just one should be enough here.

> > +additionalProperties: true
> 
> This can be restricted to child nodes only? If so:
> 
> additionalProperties:
>   type: object

OK great, this can be limited to child nodes only.

Regards,

Tony