Re: [PATCH v4 12/17] dt-bindings: PCI: dwc: Add Baikal-T1 PCIe Root Port bindings

[Serge Semin]

On Tue, Aug 09, 2022 at 02:06:16PM -0600, Rob Herring wrote:
> On Tue, Aug 9, 2022 at 1:28 PM Serge Semin <fancer.lancer@xxxxxxxxx> wrote:
> >
> > On Tue, Aug 09, 2022 at 09:12:31AM -0600, Rob Herring wrote:
> > > On Mon, Aug 8, 2022 at 10:01 AM Serge Semin <fancer.lancer@xxxxxxxxx> wrote:
> > > >
> > > > On Mon, Aug 01, 2022 at 12:13:11PM -0600, Rob Herring wrote:
> > > > > On Thu, Jul 28, 2022 at 05:34:22PM +0300, Serge Semin wrote:
> > > > > > Baikal-T1 SoC is equipped with DWC PCIe v4.60a Root Port controller, which
> > > > > > link can be trained to work on up to Gen.3 speed over up to x4 lanes. The
> > > > > > controller is supposed to be fed up with four clock sources: DBI
> > > > > > peripheral clock, AXI application Tx/Rx clocks and external PHY/core
> > > > > > reference clock generating the 100MHz signal. In addition to that the
> > > > > > platform provide a way to reset each part of the controller:
> > > > > > sticky/non-sticky bits, host controller core, PIPE interface, PCS/PHY and
> > > > > > Hot/Power reset signal. The Root Port controller is equipped with multiple
> > > > > > IRQ lines like MSI, system AER, PME, HP, Bandwidth change, Link
> > > > > > equalization request and eDMA ones. The registers space is accessed over
> > > > > > the DBI interface. There can be no more than four inbound or outbound iATU
> > > > > > windows configured.
> > > > > >
> > > > > > Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx>
> > > > > >
> 
> [...]
> 
> > > > > > +  reg-names:
> > > > > > +    minItems: 3
> > > > > > +    maxItems: 3
> > > > > > +    items:
> > > > > > +      enum: [ dbi, dbi2, config ]
> > > > >
> > > >
> > > > > Define the order. Here, and the rest.
> > > >
> > > > Ok. I will, but please answer to my question, I asked you in the
> > > > previous email thread:
> > > >
> > > > Serge Semin wrote:
> > > > > Rob Herring wrote:
> > > > > > ...
> > > > > > Tell me why you need random order.
> > > > >
> > > > > Because I don't see a need in constraining the order. If we get to set
> > > > > the order requirement, then why do we need to have the "*-names"
> > > > > property at all?
> > >
> > > Originally, it was for cases where you have a variable number of
> > > entries and can't determine what each entry is. IOW, when you have
> > > optional entries in the middle of required entries. But then everyone
> > > *loves* -names even when not needed or useful such as 'phy-names =
> > > "pcie"' (the phy subsys requiring names was part of the problem there,
> > > but that's been fixed).
> > >
> 
> > > > > IMO having "reg" with max/minItems restriction plus generic
> > > > > description and "reg-names" with possible values enumerated seems very
> > > > > suitable pattern in this case. Don't you think?
> > >
> > > No, I think this is just as concise and defines the order too:
> > >
> > > reg-names:
> > >   items:
> > >     - const: dbi
> > >     - const: dbi2
> > >     - const: config
> > >
> > > >
> > > > In addition to that what about optional names? How would you suggest
> > > > to handle such case without the non-ordered pattern?
> > >
> >
> > > Sorry, I don't follow.
> >
> > I meant exactly the case you've described as the main goal of the
> > named properties. My worry was that by using the pattern:
> >
> > reg-names:
> >   items:
> >     - const: name
> >     - const: another_name
> >     - const: one_more_name
> >
> > you get to fix the names order, which they were invented to get rid
> > from. If you get to use that pattern the only optional names could be
> > the names at the tail of the array, which isn't always applicable. In
> > that case you'd have no choice but to use the pattern suggested by
> > me.
> 
> For this binding, we use reg-names because the order and what's
> present varies by platform. But for a given platform the order is
> fixed.

Got it. Thanks for your time and for the detailed case clarification.

-Sergey

> 
> Rob