Re: [PATCH] dt-bindings: memory-controllers: arm,pl353-smc: Extend to support 'arm,pl354' SMC

[Miquel Raynal]

Hi Rob,

robh@xxxxxxxxxx wrote on Mon, 24 Oct 2022 09:31:41 -0500:

> On Mon, Oct 24, 2022 at 3:14 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> >
> > Hi Rob,
> >
> > robh@xxxxxxxxxx wrote on Fri, 21 Oct 2022 15:39:28 -0500:
> >  
> > > Add support for the Arm PL354 static memory controller to the existing
> > > Arm PL353 binding. Both are different configurations of the same IP with
> > > support for different types of memory interfaces.
> > >
> > > The 'arm,pl354' binding has already been in use upstream for a long time
> > > in Arm development boards. The existing users have only the controller
> > > without any child devices, so drop the required address properties
> > > (ranges, #address-cells, #size-cells). The schema for 'ranges' is too
> > > constrained as the order is not important and the PL354 has 8
> > > chipselects (And the PL353 actually has up to 8 too).  
> >
> > I'm not convinced the ranges constraint should be soften. For me
> > the order was important (and the description in the yaml useful, but
> > that's a personal opinion). What makes you think the ranges order is
> > not relevant on PL353?  
> 
> Address translation looks for a matching entry, so order doesn't
> matter. However, we have seen cases in PCI hosts where the driver
> populates registers based on the order of ranges. That's a driver
> problem IMO. For PCI, it was multiple entries of the same type. For
> this, we have the chip select number in the entry, so we shouldn't
> have the same sort of problem. Except there is another issue that the
> SRAM interface chipselects are numbered 1 and 2. The PL353 can have 4
> NAND chipselects, I don't think the host addresses are necessarily in
> order or contiguous either, so you could need 4 entries for NAND. The
> existing description doesn't handle that, and the chipselects for the
> SRAM interface should have been numbered 4-7. I don't mind saying the
> entries should be in order by chipselect, but we can't define indices
> of the entries as was done. It's all kind of academic because we don't
> have any h/w needing anything else though the Arm boards would if the
> child nodes actually got defined (not likely at this point).

Alright, thanks for the feedback.

Cheers,
Miquèl