Re: [PATCH net-next v3 2/8] dt-bindings: phy: add the "fsl,lynx-28g" compatible

[Ioana Ciornei <ioana.ciornei@nxp.com>]

On Thu, Mar 10, 2022 at 05:58:07PM +0000, Russell King (Oracle) wrote:
> On Thu, Mar 10, 2022 at 05:47:31PM +0100, Krzysztof Kozlowski wrote:
> > > +patternProperties:
> > > +  '^phy@[0-9a-f]$':
> > > +    type: object
> > > +    properties:
> > > +      reg:
> > > +        description:
> > > +          Number of the SerDes lane.
> > > +        minimum: 0
> > > +        maximum: 7
> > > +
> > > +      "#phy-cells":
> > > +        const: 0
> > 
> > Why do you need all these children? You just enumerated them, without
> > statuses, resources or any properties. This should be rather just index
> > of lynx-28g phy.
> 
> There is good reason why the Marvell driver does it this way, and that
> is because there are shared registers amongst all the comphys on the
> SoC.
> 

The Lynx SerDes block also has shared registers between the lanes as
well as per lane registers.
For example, I can configure the PLL to be used, the equalization
parameters etc by using per lane registers but the protocol registers
are shared among all the lanes.

> Where that isn't the case, and there is no other reason, I would suggest
> creating multiple phy modes,

I suppose here you intended 'multiple phy nodes', right?

> one per physical PHY in DT, giving their
> address would be a saner approach. That way, the driver isn't locked
> in to a model of "we have N PHYs which are spaced by such-and-such
> apart", and you don't have this "maximum: 7" thing above either.
> 

I don't think the model of separate driver instances per lane is
applicable here.

Ioana