Re: [PATCH v4 1/3] dt-bindings: rng: Add Rockchip RNG bindings

[Krzysztof Kozlowski <krzk@kernel.org>]

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