Re: [PATCH 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver

[Rob Herring]

On Wed, Nov 02, 2022 at 06:13:35PM +0100, Andreas Färber wrote:
> Hi Rob,
> 
> On 02.11.22 16:55, Rob Herring wrote:
> > On Mon, Oct 31, 2022 at 06:10:49PM +0800, Chester Lin wrote:
> > > Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
> > > Chassis.
> > > 
> > > Signed-off-by: Jan Petrous <jan.petrous@xxxxxxx>
> > > Signed-off-by: Chester Lin <clin@xxxxxxxx>
> > > ---
> > >   .../bindings/net/nxp,s32cc-dwmac.yaml         | 145 ++++++++++++++++++
> > >   1 file changed, 145 insertions(+)
> > >   create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> > > new file mode 100644
> > > index 000000000000..f6b8486f9d42
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> > > @@ -0,0 +1,145 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +# Copyright 2021-2022 NXP
> > > +%YAML 1.2
> > > +---
> > > +$id: "http://devicetree.org/schemas/net/nxp,s32cc-dwmac.yaml#";
> > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#";
> > > +
> > > +title: NXP S32CC DWMAC Ethernet controller
> > > +
> > > +maintainers:
> > > +  - Jan Petrous <jan.petrous@xxxxxxx>
> > > +  - Chester Lin <clin@xxxxxxxx>
> [...]
> > > +properties:
> > > +  compatible:
> > > +    contains:
> > 
> > Drop 'contains'.
> > 
> > > +      enum:
> > > +        - nxp,s32cc-dwmac
> 
> In the past you were adamant that we use concrete SoC-specific strings. Here
> that would mean s32g2 or s32g274 instead of s32cc (which aims to share with
> S32G3 IIUC).

Yes they should be SoC specific. Really, 1 per maskset or die is fine if 
that level of detail is known. No need for different compatibles for 
different part numbers created by fused off features or package pinout 
differences.


> [...]
> > > +  clocks:
> > > +    items:
> > > +      - description: Main GMAC clock
> > > +      - description: Peripheral registers clock
> > > +      - description: Transmit SGMII clock
> > > +      - description: Transmit RGMII clock
> > > +      - description: Transmit RMII clock
> > > +      - description: Transmit MII clock
> > > +      - description: Receive SGMII clock
> > > +      - description: Receive RGMII clock
> > > +      - description: Receive RMII clock
> > > +      - description: Receive MII clock
> > > +      - description:
> > > +          PTP reference clock. This clock is used for programming the
> > > +          Timestamp Addend Register. If not passed then the system
> > > +          clock will be used.
> > 
> > If optional, then you need 'minItems'.
> [snip]
> 
> Do we have any precedence of bindings with *MII clocks like these?

Don't know...

> AFAIU the reason there are so many here is that there are in fact physically
> just five, but different parent clock configurations that SCMI does not
> currently expose to Linux. Thus I was raising that we may want to extend the
> SCMI protocol with some SET_PARENT operation that could allow us to use less
> input clocks here, but obviously such a standardization process will take
> time...
> 
> What are your thoughts on how to best handle this here?

Perhaps use assigned-clocks if it is static for a board.

> Not clear to me has been whether the PHY mode can be switched at runtime
> (like DPAA2 on Layerscape allows for SFPs) or whether this is fixed by board
> design. If the latter, the two out of six SCMI IDs could get selected in
> TF-A, to have only physical clocks here in the binding.
> 
> Regards,
> Andreas