Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   "Sriranjani P" <sriranjani.p@samsung.com>
To:     "'Krzysztof Kozlowski'" <krzysztof.kozlowski@linaro.org>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
        <richardcochran@gmail.com>, <alexandre.torgue@foss.st.com>,
        <joabreu@synopsys.com>, <mcoquelin.stm32@gmail.com>,
        <alim.akhtar@samsung.com>, <linux-fsd@tesla.com>,
        <pankaj.dubey@samsung.com>, <swathi.ks@samsung.com>,
        <ravi.patel@samsung.com>
Cc:     <netdev@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>,
        <linux-samsung-soc@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>
In-Reply-To: <16eab776-07d4-3c31-7e82-444863303102@linaro.org>
Subject: RE: [PATCH v3 1/4] dt-bindings: net: Add FSD EQoS device tree
 bindings
Date:   Wed, 16 Aug 2023 11:28:05 +0530
Message-ID: <000d01d9d006$a211d880$e6358980$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQG0kE2cByMDcfrFjxkR49X5VWx9JAKGcKNgAXd78/0Bp/St37AJuAwg
Content-Language: en-in
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 105P
DLP-Filter: Pass
References: <20230814112539.70453-1-sriranjani.p@samsung.com>
        <CGME20230814112605epcas5p31aca7b23e70e8d93df11414291f7ce66@epcas5p3.samsung.com>
        <20230814112539.70453-2-sriranjani.p@samsung.com>
        <16eab776-07d4-3c31-7e82-444863303102@linaro.org>
Precedence: bulk


> -----Original Message-----
> From: Krzysztof Kozlowski =5Bmailto:krzysztof.kozlowski=40linaro.org=5D
> Sent: 15 August 2023 01:10
> To: Sriranjani P <sriranjani.p=40samsung.com>; davem=40davemloft.net;
> edumazet=40google.com; kuba=40kernel.org; pabeni=40redhat.com;
> robh+dt=40kernel.org; krzysztof.kozlowski+dt=40linaro.org;
> conor+dt=40kernel.org; richardcochran=40gmail.com;
> alexandre.torgue=40foss.st.com; joabreu=40synopsys.com;
> mcoquelin.stm32=40gmail.com; alim.akhtar=40samsung.com; linux-
> fsd=40tesla.com; pankaj.dubey=40samsung.com; swathi.ks=40samsung.com;
> ravi.patel=40samsung.com
> Cc: netdev=40vger.kernel.org; devicetree=40vger.kernel.org; linux-
> kernel=40vger.kernel.org; linux-samsung-soc=40vger.kernel.org; linux-arm-
> kernel=40lists.infradead.org
> Subject: Re: =5BPATCH v3 1/4=5D dt-bindings: net: Add FSD EQoS device tre=
e
> bindings
>=20
> On 14/08/2023 13:25, Sriranjani P wrote:
> > Add FSD Ethernet compatible in Synopsys dt-bindings document. Add FSD
> > Ethernet YAML schema to enable the DT validation.
> >
> > Signed-off-by: Pankaj Dubey <pankaj.dubey=40samsung.com>
> > Signed-off-by: Ravi Patel <ravi.patel=40samsung.com>
> > Signed-off-by: Swathi K S <swathi.ks=40samsung.com>
> > Signed-off-by: Sriranjani P <sriranjani.p=40samsung.com>
> > ---
> >  .../devicetree/bindings/net/snps,dwmac.yaml   =7C   5 +-
> >  .../devicetree/bindings/net/tesla,ethqos.yaml =7C 114
> > ++++++++++++++++++
> >  2 files changed, 117 insertions(+), 2 deletions(-)  create mode
> > 100644 Documentation/devicetree/bindings/net/tesla,ethqos.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > index ddf9522a5dc2..0ced7901e644 100644
> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > =40=40 -96,6 +96,7 =40=40 properties:
> >          - snps,dwxgmac
> >          - snps,dwxgmac-2.10
> >          - starfive,jh7110-dwmac
> > +        - tesla,fsd-ethqos-4.21
>=20
> I don't think one given SoC - and I was told FSD is strictly defined one =
specific
> SoC - can have different versions of the same block, so drop the block
> versioning.
>=20

Ok, will remove versioning.

> >
> >    reg:
> >      minItems: 1
> > =40=40 -117,7 +118,7 =40=40 properties:
> >
> >    clocks:
> >      minItems: 1
> > -    maxItems: 8
> > +    maxItems: 10
> >      additionalItems: true
> >      items:
> >        - description: GMAC main clock
> > =40=40 -129,7 +130,7 =40=40 properties:
> >
> >    clock-names:
> >      minItems: 1
> > -    maxItems: 8
> > +    maxItems: 10
> >      additionalItems: true
> >      contains:
> >        enum:
> > diff --git a/Documentation/devicetree/bindings/net/tesla,ethqos.yaml
> > b/Documentation/devicetree/bindings/net/tesla,ethqos.yaml
> > new file mode 100644
> > index 000000000000..b78829246364
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/tesla,ethqos.yaml
> > =40=40 -0,0 +1,114 =40=40
> > +=23 SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > +---
> > +=24id:
> > +https://protect2.fireeye.com/v1/url?k=3D0d5f9085-6cd485b3-0d5e1bca-
> 74fe
> > +485cbff1-9835d59b137d73e5&q=3D1&e=3D93f28da2-6d86-4cc2-a07a-
> 9be1380616cc&
> >
> +u=3Dhttp%3A%2F%2Fdevicetree.org%2Fschemas%2Fnet%2Ftesla%2Cethqos.
> yaml%2
> > +3
> > +=24schema:
> > +https://protect2.fireeye.com/v1/url?k=3D67d3522a-0658471c-67d2d965-
> 74fe
> > +485cbff1-b9570dbbedf33f81&q=3D1&e=3D93f28da2-6d86-4cc2-a07a-
> 9be1380616cc&
> > +u=3Dhttp%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
> > +
> > +title: FSD Ethernet Quality of Service
> > +
> > +maintainers:
> > +  - Sriranjani P <sriranjani.p=40samsung.com>
> > +  - Swathi K S <swathi.ks=40samsung.com>
> > +
> > +description:
> > +  dwmmac based tesla ethernet devices which support Gigabit
> > +  ethernet.
> > +
> > +allOf:
> > +  - =24ref: snps,dwmac.yaml=23
> > +
> > +properties:
> > +  compatible:
> > +    const: tesla,fsd-ethqos-4.21.yaml
>=20
> ?

Will fix this to tesla,fsd-ethqos.yaml=20

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 5
>=20
> Why? I expect it to be specific.

Sorry, I could not understood this comment. In FSD we have two instances of=
 EQoS IP, one in PERIC block, which requires total 10 clocks  to be configu=
red and another instance exist in FSYS0 block which needs 5 clocks to be co=
nfigured, so we kept minItems as 5 and maxItems as 10, but looks like lates=
t items schema do not need maxItems entry so we will drop maxItems entry. I=
n my understanding minItems still required so it should be kept with minimu=
m number of clock requirements.

>=20
> > +    maxItems: 10
> > +
> > +  clock-names:
> > +    minItems: 5
> > +    maxItems: 10
> > +    items:
> > +      - const: ptp_ref
> > +      - const: master_bus
> > +      - const: slave_bus
> > +      - const: tx
> > +      - const: rx
> > +      - const: master2_bus
> > +      - const: slave2_bus
> > +      - const: eqos_rxclk_mux
> > +      - const: eqos_phyrxclk
> > +      - const: dout_peric_rgmii_clk
> > +
> > +  fsd-rx-clock-skew:
> > +    =24ref: /schemas/types.yaml=23/definitions/phandle-array
> > +    items:
> > +      - items:
> > +          - description: phandle to the syscon node
> > +          - description: offset of the control register
> > +    description:
> > +      Should be phandle/offset pair. The phandle to the syscon node.
> > +
> > +  iommus:
> > +    maxItems: 1
> > +
> > +  phy-mode:
> > +    =24ref: ethernet-controller.yaml=23/properties/phy-connection-type
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - rx-clock-skew
>=20
> Eee? Isn't it fsd-rx-clock-skew which anyway is not correct?

Sorry, I missed to change this in DT schema before posting, I will make thi=
s to fsd-rx-clock-skew.=20

>=20
> > +  - iommus
> > +  - phy-mode
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - =7C
> > +    =23include <dt-bindings/clock/fsd-clk.h>
> > +    =23include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    ethernet_1: ethernet=4014300000 =7B
> > +              compatible =3D =22tesla,dwc-qos-ethernet-4.21=22;
>=20
> Three different compatibles for the same.
>=20
> No, please test your patches before sending.
>=20
> I am not even checking if previous feedback was applied... Did you really=
 go
> through it?
>=20

Sorry, I missed this will take care.=20

> Best regards,
> Krzysztof