Re: [PATCH 2/7] dt-bindings: rng: add binding for Rockchip RK3588 RNG

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Olivia Mackall <olivia@selenic.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Daniel Golle <daniel@makrotopia.org>,
	Aurelien Jarno <aurelien@aurel32.net>
Cc: Sebastian Reichel <sebastian.reichel@collabora.com>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-crypto@vger.kernel.org
Subject: Re: [PATCH 2/7] dt-bindings: rng: add binding for Rockchip RK3588 RNG
Date: Fri, 31 Jan 2025 09:30:51 +0100	[thread overview]
Message-ID: <979564a4-c8e9-4427-8019-349d0794d9af@kernel.org> (raw)
In-Reply-To: <20250130-rk3588-trng-submission-v1-2-97ff76568e49@collabora.com>

On 30/01/2025 17:31, Nicolas Frattaroli wrote:
> +title: Rockchip RK3588 TRNG
> +
> +description: True Random Number Generator on Rockchip RK3588 SoC
> +
> +maintainers:
> +  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - rockchip,rk3588-rng
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: TRNG AHB clock
> +
> +  # Optional, not used by some driver implementations

What driver implementations? Downstream? They do not matter, because
they are full of all sort of crap.

Can this block have interrupt really disconnected? This is the question
you should answer.

> +  interrupts:
> +    maxItems: 1
> +
> +  # Optional, hardware works without explicit reset

Just because bootloader did something? With that reasoning nothing is
ever required because firmware can abstract it. Either you have there a
reset or not. In this particular case your driver is irrelevant.

> +  resets:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +

BTW, there is a binding for Rockchip TRNG, with a bit different clocks
so I have feeling yours is incomplete here.

> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> +    bus {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      rng@fe378000 {
> +        compatible = "rockchip,rk3588-rng";
> +        reg = <0x0 0xfe378000 0x0 0x200>;
> +        interrupts = <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH 0>;
> +        clocks = <&scmi_clk SCMI_HCLK_SECURE_NS>;
> +        resets = <&scmi_reset SCMI_SRST_H_TRNG_NS>;
> +        status = "disabled";

Examples cannot be disabled.

> +      };
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bc8ce7af3303f747e0ef028e5a7b29b0bbba99f4..7daf9bfeb0cb4e9e594b809012c7aa243b0558ae 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20420,8 +20420,10 @@ F:	include/uapi/linux/rkisp1-config.h
>  ROCKCHIP RK3568 RANDOM NUMBER GENERATOR SUPPORT
>  M:	Daniel Golle <daniel@makrotopia.org>
>  M:	Aurelien Jarno <aurelien@aurel32.net>
> +M:	Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Like Conor said, this is not really relevant and should be a separate patch.

Best regards,
Krzysztof

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip