RE: [PATCH v2 1/1] dt-bindings: net: convert enetc to yaml

[Wei Fang]

> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: 2024年7月9日 22:11
> To: Wei Fang <wei.fang@xxxxxxx>
> Cc: Frank Li <frank.li@xxxxxxx>; krzk@xxxxxxxxxx; conor+dt@xxxxxxxxxx;
> davem@xxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; edumazet@xxxxxxxxxx;
> imx@xxxxxxxxxxxxxxx; krzk+dt@xxxxxxxxxx; kuba@xxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; pabeni@xxxxxxxxxx;
> Vladimir Oltean <vladimir.oltean@xxxxxxx>; Claudiu Manoil
> <claudiu.manoil@xxxxxxx>; Clark Wang <xiaoning.wang@xxxxxxx>
> Subject: Re: [PATCH v2 1/1] dt-bindings: net: convert enetc to yaml
> 
> On Mon, Jul 8, 2024 at 4:07 AM Wei Fang <wei.fang@xxxxxxx> wrote:
> >
> > > -----Original Message-----
> > > From: Frank Li <Frank.Li@xxxxxxx>
> > > Sent: 2024年6月27日 0:23
> > > To: krzk@xxxxxxxxxx
> > > Cc: Frank Li <frank.li@xxxxxxx>; conor+dt@xxxxxxxxxx;
> > > davem@xxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> > > edumazet@xxxxxxxxxx; imx@xxxxxxxxxxxxxxx; krzk+dt@xxxxxxxxxx;
> > > kuba@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > > netdev@xxxxxxxxxxxxxxx; pabeni@xxxxxxxxxx; robh@xxxxxxxxxx
> > > Subject: [PATCH v2 1/1] dt-bindings: net: convert enetc to yaml
> > >
> > > Convert enetc device binding file to yaml. Split to 3 yaml files,
> > > 'fsl,enetc.yaml', 'fsl,enetc-mdio.yaml', 'fsl,enetc-ierb.yaml'.
> > >
> >
> > Sorry I didn't see this patch until now, I was planning to make this
> > conversion but didn't realize you had started it first. It's very nice, thanks!
> >
> > > Additional Changes:
> > > - Add pci<vendor id>,<production id> in compatible string.
> > > - Ref to common ethernet-controller.yaml and mdio.yaml.
> > > - Remove fixed-link part.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@xxxxxxx>
> > > ---
> > > Change from v1 to v2
> > > - renamee file as fsl,enetc-mdio.yaml, fsl,enetc-ierb.yaml,
> > > fsl,enetc.yaml
> > > - example include pcie node
> > > ---
> > >  .../bindings/net/fsl,enetc-ierb.yaml          |  35 ++++++
> > >  .../bindings/net/fsl,enetc-mdio.yaml          |  53 ++++++++
> > >  .../devicetree/bindings/net/fsl,enetc.yaml    |  50 ++++++++
> > >  .../devicetree/bindings/net/fsl-enetc.txt     | 119 ------------------
> > >  4 files changed, 138 insertions(+), 119 deletions(-)  create mode
> > > 100644 Documentation/devicetree/bindings/net/fsl,enetc-ierb.yaml
> > >  create mode 100644
> > > Documentation/devicetree/bindings/net/fsl,enetc-mdio.yaml
> > >  create mode 100644
> > > Documentation/devicetree/bindings/net/fsl,enetc.yaml
> > >  delete mode 100644
> > > Documentation/devicetree/bindings/net/fsl-enetc.txt
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/net/fsl,enetc-mdio.yaml
> > > b/Documentation/devicetree/bindings/net/fsl,enetc-mdio.yaml
> > > new file mode 100644
> > > index 0000000000000..60740ea56cb08
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/fsl,enetc-mdio.yaml
> >
> > I suggest changing the file name to nxp,netc-emdio.yaml. "fsl" is a
> > very outdated prefix. For new files, I think "nxp" is a better prefix.
> 
> Convention is filenames use the compatible string. So no.
Actually, the "fsl,enetc-mdio" compatible is not used in the driver, I'm not
sure whether we should still keep it in the DTS, maybe we can remove it
from DTS first, or rename the compatible string, IMO, "fsl,enetc-mdio" is
not quiet right, this MDIO controller not only provide MDIO bus to ENETC,
but also to NETC switch.

> 
> > > @@ -0,0 +1,53 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +
> > > +title: ENETC the central MDIO PCIe endpoint device
> > external is better, that is why we call it EMDIO.
> >
> > > +
> > > +description:
> > > +  In this case, the mdio node should be defined as another PCIe
> > > +  endpoint node, at the same level with the ENETC port nodes
> > > +
> > This my local description, excerpted from NETC's block guide, FYI.
> > description: |
> >   NETC provides an external master MDIO interface (EMDIO) for managing
> external
> >   devices (PHYs). EMDIO supports both Clause 22 and 45 protocols. And
> the EMDIO
> >   provides a means for different software modules to share a single set of
> MDIO
> >   signals to access their PHYs.
> >
> > > +maintainers:
> > > +  - Frank Li <Frank.Li@xxxxxxx>.
> > Vladimir and Claudiu as the driver maintainer, it is best to add them
> > to this list
> >
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - enum:
> > > +          - pci1957,ee01
> > > +      - const: fsl,enetc-mdio
> >
> > " fsl,enetc-mdio" is meaningless, we did not use it ever.
> 
> arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi:
>  compatible = "pci1957,ee01", "fsl,enetc-mdio";
> 
> In fact, until I recently added the standard PCI compatibles, these were the
> only compatible strings used.
> 
>