| Message ID | 20221128184718.1963353-2-aurelien@aurel32.net |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | hwrng: add hwrng support for Rockchip RK3568 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 28/11/2022 19:47, Aurelien Jarno wrote: > Add the RNG bindings for the RK3568 SoC from Rockchip Use subject prefixes matching the subsystem (git log --oneline -- ...), so it is rng, not RNG. Also, you are not adding all-Rockhip RNG but a specific device. Subject: drop second, redundant "bindings". > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > --- > .../bindings/rng/rockchip,rk3568-rng.yaml | 60 +++++++++++++++++++ > 1 file changed, 60 insertions(+) > create mode 100644 Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml > > diff --git a/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml b/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml > new file mode 100644 > index 000000000000..c2f5ef69cf07 > --- /dev/null > +++ b/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml > @@ -0,0 +1,60 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/rng/rockchip,rk3568-rng.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Rockchip TRNG > + > +description: True Random Number Generator for some Rockchip SoCs s/for some Rockchip SoCs/on Rokchip RK3568 SoC/ > + > +maintainers: > + - Aurelien Jarno <aurelien@aurel32.net> > + > +properties: > + compatible: > + enum: > + - rockchip,rk3568-rng > + > + reg: > + maxItems: 1 > + > + clocks: > + items: > + - description: TRNG clock > + - description: TRNG AHB clock > + > + clock-names: > + items: > + - const: trng_clk > + - const: trng_hclk These are too vague names. Everything is a clk in clock-names, so no need usually to add it as name suffix. Give them some descriptive names, e.g. core and ahb. > + > + resets: > + maxItems: 1 > + Best regards, Krzysztof
Hi, Thanks for your feedback. On 2022-11-29 10:24, Krzysztof Kozlowski wrote: > On 28/11/2022 19:47, Aurelien Jarno wrote: > > Add the RNG bindings for the RK3568 SoC from Rockchip > > Use subject prefixes matching the subsystem (git log --oneline -- ...), > so it is rng, not RNG. Also, you are not adding all-Rockhip RNG but a > specific device. > > Subject: drop second, redundant "bindings". > > > > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > > --- > > .../bindings/rng/rockchip,rk3568-rng.yaml | 60 +++++++++++++++++++ > > 1 file changed, 60 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml > > > > diff --git a/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml b/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml > > new file mode 100644 > > index 000000000000..c2f5ef69cf07 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml > > @@ -0,0 +1,60 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/rng/rockchip,rk3568-rng.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Rockchip TRNG > > + > > +description: True Random Number Generator for some Rockchip SoCs > > s/for some Rockchip SoCs/on Rokchip RK3568 SoC/ My point there is that this driver should also work for other Rockchip SoCs like the RK3588, but 1) it support for this SoC is being added and not yet available in the Linux kernel 2) it hasn't been tested. Should we mark it as RK3568 specific (or rather RK356x) and change that once a compatible entry is added for the RK3588? > > + > > +maintainers: > > + - Aurelien Jarno <aurelien@aurel32.net> > > + > > +properties: > > + compatible: > > + enum: > > + - rockchip,rk3568-rng > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + items: > > + - description: TRNG clock > > + - description: TRNG AHB clock > > + > > + clock-names: > > + items: > > + - const: trng_clk > > + - const: trng_hclk > > These are too vague names. Everything is a clk in clock-names, so no > need usually to add it as name suffix. Give them some descriptive names, > e.g. core and ahb. Those names are based on <include/dt-bindings/clock/rk3568-cru.h> and other drivers seems to have used those for the names. But I understand that broken things could have been merged, so I am fine changing that to core and ahb. > > + > > + resets: > > + maxItems: 1 > > + Regards Aurelien
On 02/12/2022 20:20, Aurelien Jarno wrote: > Hi, > > Thanks for your feedback. > > On 2022-11-29 10:24, Krzysztof Kozlowski wrote: >> On 28/11/2022 19:47, Aurelien Jarno wrote: >>> Add the RNG bindings for the RK3568 SoC from Rockchip >> >> Use subject prefixes matching the subsystem (git log --oneline -- ...), >> so it is rng, not RNG. Also, you are not adding all-Rockhip RNG but a >> specific device. >> >> Subject: drop second, redundant "bindings". >> >>> >>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> >>> --- >>> .../bindings/rng/rockchip,rk3568-rng.yaml | 60 +++++++++++++++++++ >>> 1 file changed, 60 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml b/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml >>> new file mode 100644 >>> index 000000000000..c2f5ef69cf07 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml >>> @@ -0,0 +1,60 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/rng/rockchip,rk3568-rng.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Rockchip TRNG >>> + >>> +description: True Random Number Generator for some Rockchip SoCs >> >> s/for some Rockchip SoCs/on Rokchip RK3568 SoC/ > > My point there is that this driver should also work for other Rockchip > SoCs like the RK3588, but 1) Driver maybe less, but bindings might not. > it support for this SoC is being added and > not yet available in the Linux kernel 2) it hasn't been tested. > > Should we mark it as RK3568 specific (or rather RK356x) and change that > once a compatible entry is added for the RK3588? Describe what you are adding here, not something else. > >>> + >>> +maintainers: >>> + - Aurelien Jarno <aurelien@aurel32.net> >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - rockchip,rk3568-rng >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + clocks: >>> + items: >>> + - description: TRNG clock >>> + - description: TRNG AHB clock >>> + >>> + clock-names: >>> + items: >>> + - const: trng_clk >>> + - const: trng_hclk >> >> These are too vague names. Everything is a clk in clock-names, so no >> need usually to add it as name suffix. Give them some descriptive names, >> e.g. core and ahb. > > Those names are based on <include/dt-bindings/clock/rk3568-cru.h> and clock-names is not for the actual name of the clock feeding it, but rather name of input of the device. Reader-friendly. > other drivers seems to have used those for the names. But I understand > that broken things could have been merged, so I am fine changing that to > core and ahb. > >>> + >>> + resets: >>> + maxItems: 1 >>> + > > Regards > Aurelien > Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml b/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml new file mode 100644 index 000000000000..c2f5ef69cf07 --- /dev/null +++ b/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml @@ -0,0 +1,60 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/rng/rockchip,rk3568-rng.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Rockchip TRNG + +description: True Random Number Generator for some Rockchip SoCs + +maintainers: + - Aurelien Jarno <aurelien@aurel32.net> + +properties: + compatible: + enum: + - rockchip,rk3568-rng + + reg: + maxItems: 1 + + clocks: + items: + - description: TRNG clock + - description: TRNG AHB clock + + clock-names: + items: + - const: trng_clk + - const: trng_hclk + + resets: + maxItems: 1 + +required: + - compatible + - reg + - clocks + - clock-names + - resets + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/rk3568-cru.h> + bus { + #address-cells = <2>; + #size-cells = <2>; + + rng@fe388000 { + compatible = "rockchip,rk3568-rng"; + reg = <0x0 0xfe388000 0x0 0x4000>; + clocks = <&cru CLK_TRNG_NS>, <&cru HCLK_TRNG_NS>; + clock-names = "trng_clk", "trng_hclk"; + resets = <&cru SRST_TRNG_NS>; + }; + }; + +...
Add the RNG bindings for the RK3568 SoC from Rockchip Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- .../bindings/rng/rockchip,rk3568-rng.yaml | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml