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@f0000:bus@4000000:r5fss@79000000: reserved memory > > > init failed, ret = -22 > > > [ 7.517549] remoteproc remoteproc0: releasing 79000000.r5f > > > [ 7.523338] k3_r5_rproc bus@f0000:bus@4000000:r5fss@79000000: > > > k3_r5_cluster_rproc_init failed, ret = -22 > > > [ 7.532993] k3_r5_rproc: probe of bus@f0000:bus@4000000:r5fss@79000000 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