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@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) 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@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 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
