Re: [PATCH 1/3] arm64: dts: ti: k3-am65-main: drop RNG clock

[Jayesh Choudhary]

On 03/11/22 01:14, Nishanth Menon wrote:
> On 12:04-20221102, Andrew Davis wrote:
> > On 11/2/22 10:17 AM, Nishanth Menon wrote:
> > > On 03:02-20221101, Jayesh Choudhary wrote:
> > > > Drop RNG clock property as it is not controlled by rng-driver.
> > >
> > > Does'nt tell me what is the alternative? why is the hardware description
> > > not sufficient for control?
> > >
> > > https://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am65x_sr2/clocks.html#clocks-for-sa2-ul0-device
> > > Looks like a perfectly valid description - do we have a bug and firmware
> > > does'nt allow control here?
> >
> > We have three input clocks feeding the SA2UL module, x1, x2, pka. PKA goes
> > to the PKA sub-module (isn't it nice when they make things simple). But x1 and
> > x2 are miscellaneous and bus clocks respectively and route to several sub-modules.
> >
> > All we drop here is the clock handle in the RNG sub-module, as that sub-module is
> > not the owner of that clock (the parent SA2UL is). The alternative we could implement
> > is to move the clock node up to the parent SA2UL node.
> >
> > > > Fixes: b366b2409c97 ("arm64: dts: ti: k3-am6: Add crypto accelarator node")
> > > > Signed-off-by: Jayesh Choudhary <j-choudhary@xxxxxx>
> > > > ---
> > > >    arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 1 -
> > > >    1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > index 4005a73cfea9..e166d7b7e3a1 100644
> > > > --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > @@ -126,7 +126,6 @@ rng: rng@4e10000 {
> > > >    			compatible = "inside-secure,safexcel-eip76";
> > > >    			reg = <0x0 0x4e10000 0x0 0x7d>;
> > > >    			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> > > > -			clocks = <&k3_clks 136 1>;
> > >
> > > Does this mean that the crypto module's power-domains property should be
> > > dropped as well?
> >
> > Why? the power-domains property is in the correct spot (up in the parent node).
> >
> > Now it is true we cant actually shut the SA2UL down since it is owned
> > by the security processor, but since it is marked TI_SCI_PD_SHARED this
> > should be fine.
>
> The idea of the descriptions were to describe what is controllable by
> firmware, if there is no control due to the specified reason, it is a
> device tree bug, and should be documented when dropping it. If it serves
> a purpose in the firmware by indicating usage for example - it has valid
> reason to stick around as it is expected to be used by firmware for some
> specific reason.

The x1-clk to sa2ul is always running and fixed. What we can do is gate
off the clock specific to trng module (x1-clk) by using TRNG_EN MMR to
disable TRNG.
Hence, uncontrollable.

> The commit description does bring up the above mentioned questions and
> must be explained appropriately.

Okay, I will update the description properly in v2 for dropping this
clock and for each patch (and not just in the cover-letter).