linux-kernel - Re: [PATCH v2 1/3] dt-bindings: clock: add rk3576 cru bindings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1975336.PYKUYFuaPT@trenzalore>
Date: Tue, 06 Aug 2024 17:22:03 -0400
From: Detlev Casanova <detlev.casanova@...labora.com>
To: linux-kernel@...r.kernel.org, Krzysztof Kozlowski <krzk@...nel.org>
Cc: Michael Turquette <mturquette@...libre.com>,
 Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Heiko Stuebner <heiko@...ech.de>, Philipp Zabel <p.zabel@...gutronix.de>,
 Elaine Zhang <zhangqing@...k-chips.com>, linux-clk@...r.kernel.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH v2 1/3] dt-bindings: clock: add rk3576 cru bindings

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@...labora.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@...k-chips.com>
> > +  - Heiko Stuebner <heiko@...ech.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@...00000 {
> 
> Drop unused label
> 
> > +      compatible = "rockchip,rk3576-cru";
> > +      reg = <0xfd7c0000 0x5c000>;
> > +      #clock-cells = <1>;
> > +      #reset-cells = <1>;
> 
> Make the example complete.
> 
> > +    };
> 

Detlev.