linux-kernel - Re: [PATCH 2/3] arm64: dts: ti: Introduce J722S family of SoCs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231214183356.hi5d6cuqmqltiy6i@footnote>
Date:   Thu, 14 Dec 2023 12:33:56 -0600
From:   Nishanth Menon <nm@...com>
To:     Andrew Davis <afd@...com>
CC:     Vaishnav Achath <vaishnav.a@...com>, <vigneshr@...com>,
        <kristo@...nel.org>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <j-choudhary@...com>, <u-kumar1@...com>
Subject: Re: [PATCH 2/3] arm64: dts: ti: Introduce J722S family of SoCs

On 09:43-20231214, Andrew Davis wrote:
> On 12/14/23 6:27 AM, Nishanth Menon wrote:
> > On 14:07-20231214, Vaishnav Achath wrote:
> > [..]
> > > > Trim this down to what is different from AM62P?
> > > > 
> > > 
> > > Thanks for the review, I will trim this down in next revision, but the above is
> > > just a summary of the main features of this SoC, pointing to AM62P feature set
> > > here seems confusing to me. why does a new user/developer using J722S need to be
> > > aware of the existence of AM62P to just understand a high level summary about
> > > this device?
> > 
> > Since this is a reuse device. Helps with review and focus on deltas.
> > 
> > [...]
> > 
> > > > > +	l2_0: l2-cache0 {
> > > > > +		compatible = "cache";
> > > > > +		cache-unified;
> > > > > +		cache-level = <2>;
> > > > > +		cache-size = <0x80000>;
> > > > > +		cache-line-size = <64>;
> > > > > +		cache-sets = <512>;
> > > > > +	};
> > > > 
> > > > ^^ this is a duplication of am62p5.dtsi? what about the spins with
> > > > different CPUs enabled?
> > > > 
> > > 
> > > Yes it is a duplicate, as of now we are not aware of plan for spins with cores
> > > disabled, so just followed the pattern followed for other Jacinto devices
> > > (J721e, J7200, J721s2, J784s4).
> > 
> > None of the devices have been as close a reuse device as this has been.
> 
> I'd argue J721e and J7200 are more similar in terms of reuse. It was a
> mistake to model them as simple super/subsets of each other, only causes
> confusion later. Let's keep at least this top level file, we will end up
> using it more as more features/deltas are enabled/found.
> 

yes, we do need a top level dtsi for the SoC. just minimize the amount
of duplication.

[...]
> > > 
> > > [    7.492406] platform 79000000.r5f: configured R5F for remoteproc mode
> > > [    7.499887] platform 79000000.r5f: device does not have reserved memory
> > > regions, ret = -22
> > > [    7.508271] k3_r5_rproc bus@...00:bus@...0000:r5fss@...00000: reserved memory
> > > init failed, ret = -22
> > > [    7.517549] remoteproc remoteproc0: releasing 79000000.r5f
> > > [    7.523338] k3_r5_rproc bus@...00:bus@...0000:r5fss@...00000:
> > > k3_r5_cluster_rproc_init failed, ret = -22
> > > [    7.532993] k3_r5_rproc: probe of bus@...00:bus@...0000:r5fss@...00000 failed
> > > with error -22
> > 
> > Yes, and the approach should rather be to disable the remote procs in
> > the board or at the SoC dtsi in a consistent manner. I had previously
> > suggested to do that SoC level (which means at am62p dtsi) since the remoteprocs have direct
> > dependency on how the memory layouts are partitioned in board.dts - but
> > i had asked folks working on remote procs to do that consistently across
> > SoCs. I don't see that having been done so far.
> > 
> 
> I fixed this for a couple SoCs way back last year (7e48b665100ee), seems
> folks kept adding mailboxes/rprocs un-disabled in the base .dtbi for
> new SoCs anyway :( This needs fixed in AM62p .dtsi first, then these
> disables can be removed from here.
> 

Yes, hence blocking it from here on. Cleanup, then add.

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