Re: [PATCH v2 1/4] arm64: dts: ti: k3-am65-main: Disable RNG node

[Nishanth Menon <nm@ti.com>]

On 12:36-20220707, Andrew Davis wrote:
> On 7/7/22 12:17 PM, Nishanth Menon wrote:
> > On 10:01-20220707, Andrew Davis wrote:
> > > On 7/7/22 9:44 AM, Bryan Brattlof wrote:
> > > > Hi Andrew
> > > > 
> > > > On July  6, 2022 thus sayeth Andrew Davis:
> > > > > The hardware random number generator is used by OP-TEE and is access is
> > > > > denied to other users with SoC level bus firewalls. Any access to this
> > > > > device from Linux will result in firewall errors. Disable this node.
> > > > > 
> > > > > Signed-off-by: Andrew Davis <afd@ti.com>
> > > > > ---
> > > > > 
> > > > > Changes from v1:
> > > > >    - Added comment in dtsi file
> > > > > 
> > > > >    arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 1 +
> > > > >    1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > > index e749343accedd..9de5a8294acd6 100644
> > > > > --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > > +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > > @@ -127,6 +127,7 @@ rng: rng@4e10000 {
> > > > >    			reg = <0x0 0x4e10000 0x0 0x7d>;
> > > > >    			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> > > > >    			clocks = <&k3_clks 136 1>;
> > > > > +			status = "disabled"; /* Used by OP-TEE */
> > > > 
> > > > Just curious about how we should document disabling nodes. I was
> > > > assuming the reasoning should be described in the bindings?
> > > > 
> > > > I would like to start disabling nodes by default in our dtsi files and
> > > > enabling them in our top dts file, making it easier for others to use
> > > > our device tree for a more focused purpose than our dev boards. I just
> > > > didn't know where I should document why I disabled the nodes :)
> > > > 
> > > 
> > > This got push-back last time this was suggested, I'll have to lookup
> > > the history. IIRC we landed on the other way around, all things should
> > > be described by default, then the specific board can enable/disable
> > > what is not used as needed.
> > 
> > See thread https://lore.kernel.org/linux-arm-kernel/YiizsYnKB0X9bDY2@atomide.com/
> > 
> > > 
> > > I was worried this topic would come up with this patch series and was
> > > almost just going to delete the whole RNG node instead of disabling it
> > > to avoid that. My reasoning for disabling here anyway is that this device
> > > *cannot* be used by *any* board, it is not just a board level configuration
> > > decision like disabling I2C nodes by default or similar that was proposed
> > > last time we had the "nodes disabled by default" discussion.
> > 
> > Hmm.. If that is the case, then why even have it in dts - is that
> > because of cases where OPTEE is'nt the TEE and users may want to
> > directly use it? OR is it because OPTEE can potentially use device tree
> > itself and discover the rng location from dt?
> > 
> 
> Hadn't thought about the second case, but it could also be valid if we
> started using DT in our OPTEE. The intention was for the first case,
> this node has valid hardware description, didn't feel right to delete
> it due to it being unusable by Linux.


Fair enough, it is a bit late for me to let this series cook in list and
then apply and wait for linux-next stability before I tag for next
kernel, I suggest improving the commit message with rationale in the
next rev.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D