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@xxxxxxxxxxx> > > --- > > .../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@xxxxxxxxxxx> > > + > > +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 -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@xxxxxxxxxxx http://www.aurel32.net
