Re: [RFC PATCH v2 net-next 13/15] dt-bindings: lynx-pcs: add properties for backplane mode

[Vladimir Oltean <vladimir.oltean@nxp.com>]

Hi Krzysztof,

On Sun, Sep 24, 2023 at 01:49:24PM +0200, Krzysztof Kozlowski wrote:
> On 23/09/2023 15:49, Vladimir Oltean wrote:
> > When the Lynx PCS is deployed on a copper backplane link, it must be
> > prepared to handle clause 73 autoneg and clause 72 link training, which
> > it can do using a dedicated AN/LT block. The latter doesn't need to be
> > described in the device tree, because it is discoverable from the SerDes
> > lanes.
> > 
> > The media type that is deployed on the link is not discoverable though,
> > so the introduction of a fsl,backplane-mode boolean property appears
> > necessary to determine whether the AN/LT block should be employed, or
> > left bypassed.
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> > v1->v2: patch is new
> > 
> >  .../devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml b/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
> > index fbedf696c555..40fbcd80ee2a 100644
> > --- a/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
> > +++ b/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
> > @@ -16,11 +16,24 @@ description: |
> >  
> >  properties:
> >    compatible:
> > -    const: fsl,lynx-pcs
> > +    enum:
> > +      - fsl,lx2160a-lynx-pcs
> > +      - fsl,lynx-pcs
> >  
> >    reg:
> >      maxItems: 1
> >  
> > +  phys:
> > +    maxItems: 4
> > +    description:
> > +      phandle for the SerDes lanes that act as PMA/PMD layer when the PCS is
> > +      part of a copper backplane PHY.
> > +
> > +  fsl,backplane-mode:
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description:
> > +      Indicates that the PCS is deployed over a copper backplane link.
> > +
> 
> Please extend also existing example.

Ok. Snippet for attention (a dtsi I was working with - applies over fsl-lx2160a-qds.dts):

&dpmac2 {
	phy-connection-type = "internal";
	managed = "in-band-status";
	/delete-property/ phys;
};

&pcs_mdio2 {
	status = "okay";
};

&pcs2 {
	fsl,backplane-mode;
	phys = <&serdes_1 3>, /* lane D */
	       <&serdes_1 2>, /* lane C */
	       <&serdes_1 1>, /* lane B */
	       <&serdes_1 0>; /* lane A */
};

The thing is that the RFC v2 bindings are still very much WIP. For v3,
I will try to remove the "phys" property from the pcs node, and process
the ones from the MAC node (client of PCS).

For example, arch/arm64/boot/dts/freescale/fsl-lx2160a-clearfog-itx.dtsi
has "phys" under &dpmac7:

&dpmac7 {
	sfp = <&sfp0>;
	managed = "in-band-status";
	phys = <&serdes_1 3>;
};

but &dpmac7 also has pcs-handle = <&pcs7>; in fsl-lx2160a.dtsi.
Thus, if I'm able to pass the "phys" phandle from &dpmac7 to &pcs7
through code (argument to lynx_pcs_create_fwnode()), then the location
of the "phys" property could be the same regardless of use case
(backplane or not), and the dt-bindings of the lynx pcs would be simpler.

The only dilemma that has stopped me from doing it is that the dpmac node
may have other "phys" in the signal path (for example external redrivers/
retimers). With up to 4 SerDes lanes per MAC and with optional retimer
phys on each lane, it becomes a question of how can we distinguish the
SerDes phys from the other phys (the PCS wants the SerDes PHY)? Would it
be okay to add a phy-names property, and parse it for "serdes-%d" to
indicate a SerDes PHY, and anything else can be named in any other way
("retimer-%d")?

> If these do not apply to lynx-pcs, then they should be disallowed in
> allOf:if:then.

They do: the "fsl,lynx-pcs" compatible string + the bool "fsl,backplane-mode"
is supposed to instantiate a MTIP_MODEL_AUTODETECT backplane AN/LT block
(as opposed to LX2160A where it isn't autodetectable). It's just that I
didn't get to implement support for the other models yet.