Received-SPF: pass (google.com: domain of linux-kernel+bounces-204025-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: "Swathi K S" <swathi.ks@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>, <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: <c9344953-9367-0ab0-fa42-3117d17643eb@linaro.org>
Subject: RE: [PATCH v3 1/4] dt-bindings: net: Add FSD EQoS device tree
 bindings
Date: Thu, 6 Jun 2024 14:44:40 +0530
Message-ID: <000301dab7f2$1e684710$5b38d530$@samsung.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQG0kE2cByMDcfrFjxkR49X5VWx9JAKGcKNgAXd78/0Bp/St3wILmJFrAs2Q6cmxp49FYA==
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>
	<000d01d9d006$a211d880$e6358980$@samsung.com>
	<c9344953-9367-0ab0-fa42-3117d17643eb@linaro.org>

Hi Krzysztof,=20

Sorry for the delay in response.
Starting now, I will be taking over this task.
I have gone through your comments and feedback and will be implementing the=
m in v4 of this patch.

> -----Original Message-----
> From: Krzysztof Kozlowski =5Bmailto:krzysztof.kozlowski=40linaro.org=5D
> Sent: 16 August 2023 11:48
> 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 16/08/2023 07:58, Sriranjani P wrote:
> >>> +
> >>> +allOf:
> >>> +  - =24ref: snps,dwmac.yaml=23
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    const: tesla,fsd-ethqos-4.21.yaml
> >>
> >> ?
> >
> > Will fix this to tesla,fsd-ethqos.yaml
>=20
> Test your patches before sending. REALLY TEST.

Sure. Will fix this to tesla,fsd-ethqos.yaml and test the same.

>=20
> >
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +  interrupts:
> >>> +    maxItems: 1
> >>> +
> >>> +  clocks:
> >>> +    minItems: 5
> >>
> >> Why? I expect it to be specific.
> >
> > Sorry, I could not understood this comment. In FSD we have two instance=
s
> of EQoS IP, one in PERIC block, which requires total 10 clocks  to be
> configured and another instance exist in FSYS0 block which needs 5 clocks=
 to
> be configured, so we kept minItems as 5 and maxItems as 10, but looks lik=
e
> latest items schema do not need maxItems entry so we will drop maxItems
> entry. In my understanding minItems still required so it should be kept w=
ith
> minimum number of clock requirements.
>=20
> No, the code is fine then.
>=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-ty=
pe
> >>> +
> >>> +required:
> >>> +  - compatible
> >>> +  - reg
> >>> +  - interrupts
> >>> +  - clocks
> >>> +  - clock-names
> >>> +  - rx-clock-skew
> >>
> >> 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=
 this
> to fsd-rx-clock-skew.
>=20
> Remember about vendor prefixes for every custom property.

Sure, will fix this in v4.

>=20
>=20
> Best regards,
> Krzysztof

Regards,=20
Swathi