Re: [PATCH net-next v6 11/17] dt-bindings: net: pse-pd: Add another way of describing several PSE PIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 3 Apr 2024 09:31:42 -0500
Rob Herring <robh@xxxxxxxxxx> wrote:

> On Wed, Apr 03, 2024 at 11:15:48AM +0200, Kory Maincent wrote:
> > On Tue, 2 Apr 2024 08:26:37 -0500
> > Rob Herring <robh@xxxxxxxxxx> wrote:
> >   
> > > > +          pairset-names:
> > > > +            $ref: /schemas/types.yaml#/definitions/string-array
> > > > +            description:
> > > > +              Names of the pairsets as per IEEE 802.3-2022, Section
> > > > 145.2.4.
> > > > +              Valid values are "alternative-a" and "alternative-b".
> > > > Each name    
> > > 
> > > Don't state constraints in prose which are defined as schema 
> > > constraints.  
> > 
> > Ok, I will remove the line.
> >   
> > > > +          pairsets:
> > > > +            $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > +            description:
> > > > +              List of phandles, each pointing to the power supply for
> > > > the
> > > > +              corresponding pairset named in 'pairset-names'. This
> > > > property
> > > > +              aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4.
> > > > +              PSE Pinout Alternatives (as per IEEE 802.3-2022 Table
> > > > 145\u20133)
> > > > +
> > > > |-----------|---------------|---------------|---------------|---------------|
> > > > +              | Conductor | Alternative A | Alternative A |
> > > > Alternative B | Alternative B |
> > > > +              |           |    (MDI-X)    |     (MDI)     |      (X)
> > > > |      (S)      |
> > > > +
> > > > |-----------|---------------|---------------|---------------|---------------|
> > > > +              | 1         | Negative VPSE | Positive VPSE | \u2014
> > > >     | \u2014             |
> > > > +              | 2         | Negative VPSE | Positive VPSE | \u2014
> > > >     | \u2014             |
> > > > +              | 3         | Positive VPSE | Negative VPSE | \u2014
> > > >     | \u2014             |
> > > > +              | 4         | \u2014             | \u2014             |
> > > > Negative VPSE | Positive VPSE |
> > > > +              | 5         | \u2014             | \u2014             |
> > > > Negative VPSE | Positive VPSE |
> > > > +              | 6         | Positive VPSE | Negative VPSE | \u2014
> > > >     | \u2014             |
> > > > +              | 7         | \u2014             | \u2014             |
> > > > Positive VPSE | Negative VPSE |
> > > > +              | 8         | \u2014             | \u2014             |
> > > > Positive VPSE | Negative VPSE |
> > > > +            minItems: 1
> > > > +            maxItems: 2    
> > > 
> > > "pairsets" does not follow the normal design pattern of foos, foo-names, 
> > > and #foo-cells. You could add #foo-cells I suppose, but what would cells 
> > > convey? I don't think it's a good fit for what you need.
> > > 
> > > The other oddity is the number of entries and the names are fixed. That 
> > > is usually defined per consumer.   
> > 
> > Theoretically if the RJ45 port binding was supported it would make more
> > sense, but in reality it's not feasible as the PSE controller need this
> > information in its init process.
> > The PSE controller reset all its port to apply a configuration so we can't
> > do it when the consumer (RJ45) probe. It would reset the other ports if one
> > consumer is probed later in the process.  
> 
> There is no reason other than convenience that all information some 
> driver needs has to be in one node or one hierarchy of nodes. You can 
> fetch anything from anywhere in the DT. It does feel like some of this 
> belongs in a connector node. We often haven't described connectors in DT 
> and stick connector properties in the controller node associated with 
> the connector. Then as things get more complicated, it becomes a mess. 

Right, we could indeed put all the informations of the pse_pi node in the future
RJ45 port abstraction node. Then, this series will be put aside until we manage
to have the port abstraction get merged.
I am not glad about this as it will stuck my work until then, but indeed
removing this pse_pi wrapper node which is between the pse_controller node and
the connector node seems cleaner.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux