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

[Chester Lin <clin@suse.com>]

Hi Rob and Andreas,

On Wed, Nov 02, 2022 at 04:44:56PM -0500, Rob Herring wrote:
> 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@nxp.com>
> > > > Signed-off-by: Chester Lin <clin@suse.com>
> > > > ---
> > > >   .../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@nxp.com>
> > > > +  - Chester Lin <clin@suse.com>
> > [...]
> > > > +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.
> 

If I understand correctly from NXP, the GMAC0 belongs to a common hardware
sub-architecture called "S32 Common Chassis", which is a common IP set applied
in many S32 SoC series, such as S32G2/G3 and S32R45. Therefore this binding is
not specifically for S32G2 but it supports all S32 SoC series who adopt S32CC
sub-arch if they could all be upstreamed in the future.

Logically S32G2 and S32R45 have the same subset *S32CC* but it doesn't mean that
S32G2 and S32R45 are derived from each other.

Regards,
Chester

> > [...]
> > > > +  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