| Message ID | b28ccedac0a51f8a437f7ceb5175e3b70696c8c2.1719106472.git.daniel@makrotopia.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hwrng: add hwrng support for Rockchip RK3568 | expand |
On 23/06/2024 05:32, Daniel Golle wrote: > From: Aurelien Jarno <aurelien@aurel32.net> > > Add the True Random Number Generator on the Rockchip RK3568 SoC. > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > Signed-off-by: Daniel Golle <daniel@makrotopia.org> My comments from v2, which I reminded at v3, were not addressed. Respond to each of them and acknowledge that you are going to implement the change. Best regards, Krzysztof
Hi Krzysztof, thank you for your patiente and repeated review of this series. On Sun, Jun 23, 2024 at 09:03:15AM +0200, Krzysztof Kozlowski wrote: > On 23/06/2024 05:32, Daniel Golle wrote: > > From: Aurelien Jarno <aurelien@aurel32.net> > > > > Add the True Random Number Generator on the Rockchip RK3568 SoC. > > > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > > My comments from v2, which I reminded at v3, were not addressed. > > Respond to each of them and acknowledge that you are going to implement > the change. Your comments to v1which I'm aware of are: https://patchwork.kernel.org/comment/25087874/ > > +++ b/Documentation/devicetree/bindings/rng/rockchip-rng.yaml > Filename matching compatible, so "rockchip,rk3568-rng.yaml" I've changed the filename. > > +title: Rockchip TRNG bindings > Drop "bindings" I've changed the title accordingly (now: "Rockchip TRNG" in v4). > > +description: > > + This driver interface with the True Random Number Generator present in some > > Drop "This driver interface" and make it a proper sentence. Bindings are > not about drivers. This has been addressed by Aurelien and further improved by me in v3. > > + clocks: > > + minItems: 2 > Drop minItems. Aurelien did that in v2. > > + clock-names: > > + items: > > + - const: clk > > + - const: hclk > > You need to explain what are these in clocks. Also you need better > names. A clock name "clk" is useless. Clocks now have meaningful names and descriptions. > > + reset-names: > > + items: > > + - const: reset > > Drop reset-names entirely, not useful. Aurelien did so in v2. Your comments to v2 which I'm aware of are: https://patchwork.kernel.org/comment/25111597/ > > 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". I've changed 'RNG' into 'rng' in the subject and spelled it out in the commit message. > > +description: True Random Number Generator for some Rockchip SoCs > > s/for some Rockchip SoCs/on Rokchip RK3568 SoC/ I've adopted your suggestion in v3 and then fixed the typo in v4. > > > + 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. If changed the names to the suggested 'core' and 'ahb'. Before sending another round of patches, just to make sure we are on the same page, please confirm that what remains is Subject: dt-bindings: rng: Add Rockchip RNG bindings which not only should be 'rng' in small letters but also name the exact chip, eg.: Subject: dt-bindings: rng: add TRNG on the Rockchip RK3568 SoC If there are any other comments you made which I'm not aware of, please point me to them. Cheers Daniel
On 23/06/2024 15:08, Daniel Golle wrote: > Hi Krzysztof, > > thank you for your patiente and repeated review of this series. > > On Sun, Jun 23, 2024 at 09:03:15AM +0200, Krzysztof Kozlowski wrote: >> On 23/06/2024 05:32, Daniel Golle wrote: >>> From: Aurelien Jarno <aurelien@aurel32.net> >>> >>> Add the True Random Number Generator on the Rockchip RK3568 SoC. >>> >>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> >>> Signed-off-by: Daniel Golle <daniel@makrotopia.org> >> >> My comments from v2, which I reminded at v3, were not addressed. >> >> Respond to each of them and acknowledge that you are going to implement >> the change. > > Your comments to v1which I'm aware of are: > https://patchwork.kernel.org/comment/25087874/ We talk about comments from v2, not v1. > >>> +++ b/Documentation/devicetree/bindings/rng/rockchip-rng.yaml >> Filename matching compatible, so "rockchip,rk3568-rng.yaml" > > I've changed the filename. > >>> +title: Rockchip TRNG bindings > >> Drop "bindings" > > I've changed the title accordingly (now: "Rockchip TRNG" in v4). > >>> +description: >>> + This driver interface with the True Random Number Generator present in some >> >> Drop "This driver interface" and make it a proper sentence. Bindings are >> not about drivers. > > This has been addressed by Aurelien and further improved by me in v3. > >>> + clocks: >>> + minItems: 2 > >> Drop minItems. > > Aurelien did that in v2. > >>> + clock-names: >>> + items: >>> + - const: clk >>> + - const: hclk >> >> You need to explain what are these in clocks. Also you need better >> names. A clock name "clk" is useless. > > Clocks now have meaningful names and descriptions. > >>> + reset-names: >>> + items: >>> + - const: reset >> >> Drop reset-names entirely, not useful. > > Aurelien did so in v2. > > Your comments to v2 which I'm aware of are: > https://patchwork.kernel.org/comment/25111597/ > >>> 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". > > I've changed 'RNG' into 'rng' in the subject and spelled it out in the > commit message. And where did you drop the redundant bindings? You just quoted the sentence and ignored it. 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..ad3648b96f82 --- /dev/null +++ b/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml @@ -0,0 +1,61 @@ +# 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 on Rockchip RK3568 SoC + +maintainers: + - Aurelien Jarno <aurelien@aurel32.net> + - Daniel Golle <daniel@makrotopia.org> + +properties: + compatible: + enum: + - rockchip,rk3568-rng + + reg: + maxItems: 1 + + clocks: + items: + - description: TRNG clock + - description: TRNG AHB clock + + clock-names: + items: + - const: core + - const: ahb + + 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 = "core", "ahb"; + resets = <&cru SRST_TRNG_NS>; + }; + }; + +... diff --git a/MAINTAINERS b/MAINTAINERS index 807feae089c4..5cd3bc2b034f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -19498,6 +19498,12 @@ F: Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst F: drivers/media/platform/rockchip/rkisp1 F: include/uapi/linux/rkisp1-config.h +ROCKCHIP RANDOM NUMBER GENERATOR SUPPORT +M: Daniel Golle <daniel@makrotopia.org> +M: Aurelien Jarno <aurelien@aurel32.net> +S: Maintained +F: Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml + ROCKCHIP RASTER 2D GRAPHIC ACCELERATION UNIT DRIVER M: Jacob Chen <jacob-chen@iotwrt.com> M: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>