diff mbox series

[v2,1/3] dt-bindings: clock: add rk3576 cru bindings

Message ID 20240802214053.433493-2-detlev.casanova@collabora.com (mailing list archive)
State New
Headers show
Series Add CRU support for rk3576 SoC | expand

Commit Message

Detlev Casanova Aug. 2, 2024, 9:35 p.m. UTC 
Document the device tree bindings of the rockchip rk3576 SoC
clock and reset unit.

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 .../bindings/clock/rockchip,rk3576-cru.yaml   | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml

Comments

Krzysztof Kozlowski Aug. 4, 2024, 9:52 a.m. UTC | #1 
On 02/08/2024 23:35, Detlev Casanova wrote:
> Document the device tree bindings of the rockchip rk3576 SoC
> clock and reset unit.
> 
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> ---
>  .../bindings/clock/rockchip,rk3576-cru.yaml   | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml b/Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml
> new file mode 100644
> index 0000000000000..929eb6183bf18
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/rockchip,rk3576-cru.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip rk3576 Family Clock and Reset Control Module
> +
> +maintainers:
> +  - Elaine Zhang <zhangqing@rock-chips.com>
> +  - Heiko Stuebner <heiko@sntech.de>
> +
> +description: |
> +  The RK3576 clock controller generates the clock and also implements a reset
> +  controller for SoC peripherals. For example it provides SCLK_UART2 and
> +  PCLK_UART2, as well as SRST_P_UART2 and SRST_S_UART2 for the second UART
> +  module.
> +  Each clock is assigned an identifier and client nodes can use this identifier
> +  to specify the clock which they consume. All available clock and reset IDs
> +  are defined as preprocessor macros in dt-binding headers.

Drop paragraph, it is obvious. You could provide here the name of the
header...

> +
> +properties:
> +  compatible:
> +    enum:
> +      - rockchip,rk3576-cru
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  "#reset-cells":
> +    const: 1
> +
> +  clocks:
> +    minItems: 2

You can drop minitems

> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: xin24m
> +      - const: xin32k
> +
> +  assigned-clocks: true
> +
> +  assigned-clock-rates: true
> +
> +  assigned-clock-parents: true

Drop  all these three

> +
> +  rockchip,grf:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: >
> +      phandle to the syscon managing the "general register files". It is used
> +      for GRF muxes, if missing any muxes present in the GRF will not be
> +      available.
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"
> +  - "#reset-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    cru: clock-controller@27200000 {

Drop unused label

> +      compatible = "rockchip,rk3576-cru";
> +      reg = <0xfd7c0000 0x5c000>;
> +      #clock-cells = <1>;
> +      #reset-cells = <1>;

Make the example complete.

> +    };

Best regards,
Krzysztof
Detlev Casanova Aug. 6, 2024, 9:22 p.m. UTC | #2 
Hi Krzysztof,

On Sunday, 4 August 2024 05:52:53 EDT Krzysztof Kozlowski wrote:
> On 02/08/2024 23:35, Detlev Casanova wrote:
> > Document the device tree bindings of the rockchip rk3576 SoC
> > clock and reset unit.
> > 
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> 
> A nit, subject: drop second/last, redundant "bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bi
> ndings/submitting-patches.rst#L18
> > ---
> > 
> >  .../bindings/clock/rockchip,rk3576-cru.yaml   | 73 +++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml> 
> > diff --git
> > a/Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml
> > b/Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml new
> > file mode 100644
> > index 0000000000000..929eb6183bf18
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml
> > @@ -0,0 +1,73 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/rockchip,rk3576-cru.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Rockchip rk3576 Family Clock and Reset Control Module
> > +
> > +maintainers:
> > +  - Elaine Zhang <zhangqing@rock-chips.com>
> > +  - Heiko Stuebner <heiko@sntech.de>
> > +
> > +description: |
> > +  The RK3576 clock controller generates the clock and also implements a
> > reset +  controller for SoC peripherals. For example it provides
> > SCLK_UART2 and +  PCLK_UART2, as well as SRST_P_UART2 and SRST_S_UART2
> > for the second UART +  module.
> > +  Each clock is assigned an identifier and client nodes can use this
> > identifier +  to specify the clock which they consume. All available
> > clock and reset IDs +  are defined as preprocessor macros in dt-binding
> > headers.
> 
> Drop paragraph, it is obvious. You could provide here the name of the
> header...
> 
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - rockchip,rk3576-cru
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  "#clock-cells":
> > +    const: 1
> > +
> > +  "#reset-cells":
> > +    const: 1
> > +
> > +  clocks:
> > +    minItems: 2
> 
> You can drop minitems
> 
> > +    maxItems: 2
> > +
> > +  clock-names:
> > +    items:
> > +      - const: xin24m
> > +      - const: xin32k
> > +
> > +  assigned-clocks: true
> > +
> > +  assigned-clock-rates: true
> > +
> > +  assigned-clock-parents: true
> 
> Drop  all these three

Why dro pthese if I need them in the device tree ? Should I remove them from 
there as well ? It seems to be working without it.

> > +
> > +  rockchip,grf:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: >
> > +      phandle to the syscon managing the "general register files". It is
> > used +      for GRF muxes, if missing any muxes present in the GRF will
> > not be +      available.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#clock-cells"
> > +  - "#reset-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    cru: clock-controller@27200000 {
> 
> Drop unused label
> 
> > +      compatible = "rockchip,rk3576-cru";
> > +      reg = <0xfd7c0000 0x5c000>;
> > +      #clock-cells = <1>;
> > +      #reset-cells = <1>;
> 
> Make the example complete.
> 
> > +    };
> 

Detlev.
Krzysztof Kozlowski Aug. 7, 2024, 5:50 a.m. UTC | #3 
On 06/08/2024 23:22, Detlev Casanova wrote:
>>> +    maxItems: 2
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: xin24m
>>> +      - const: xin32k
>>> +
>>> +  assigned-clocks: true
>>> +
>>> +  assigned-clock-rates: true
>>> +
>>> +  assigned-clock-parents: true
>>
>> Drop  all these three
> 
> Why dro pthese if I need them in the device tree ? Should I remove them from 
> there as well ? It seems to be working without it.

Because they are already accepted via dependency of clocks. This is just
redundant. Please trim your replies to relevant content.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml b/Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml
new file mode 100644
index 0000000000000..929eb6183bf18
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/rockchip,rk3576-cru.yaml
@@ -0,0 +1,73 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/rockchip,rk3576-cru.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip rk3576 Family Clock and Reset Control Module
+
+maintainers:
+  - Elaine Zhang <zhangqing@rock-chips.com>
+  - Heiko Stuebner <heiko@sntech.de>
+
+description: |
+  The RK3576 clock controller generates the clock and also implements a reset
+  controller for SoC peripherals. For example it provides SCLK_UART2 and
+  PCLK_UART2, as well as SRST_P_UART2 and SRST_S_UART2 for the second UART
+  module.
+  Each clock is assigned an identifier and client nodes can use this identifier
+  to specify the clock which they consume. All available clock and reset IDs
+  are defined as preprocessor macros in dt-binding headers.
+
+properties:
+  compatible:
+    enum:
+      - rockchip,rk3576-cru
+
+  reg:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 1
+
+  "#reset-cells":
+    const: 1
+
+  clocks:
+    minItems: 2
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: xin24m
+      - const: xin32k
+
+  assigned-clocks: true
+
+  assigned-clock-rates: true
+
+  assigned-clock-parents: true
+
+  rockchip,grf:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: >
+      phandle to the syscon managing the "general register files". It is used
+      for GRF muxes, if missing any muxes present in the GRF will not be
+      available.
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    cru: clock-controller@27200000 {
+      compatible = "rockchip,rk3576-cru";
+      reg = <0xfd7c0000 0x5c000>;
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+    };