RE: [PATCH v2 3/4] arm64: dts: fsd: Add Ethernet support for FSYS0 Block of FSD SoC

["Sriranjani P" <sriranjani.p@xxxxxxxxxxx>]

> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@xxxxxxxxxx]
> Sent: 12 January 2023 15:05
> To: Sriranjani P <sriranjani.p@xxxxxxxxxxx>; davem@xxxxxxxxxxxxx;
> edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx;
> alexandre.torgue@xxxxxxxxxxx; peppe.cavallaro@xxxxxx;
> joabreu@xxxxxxxxxxxx; mcoquelin.stm32@xxxxxxxxx
> Cc: netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; pankaj.dubey@xxxxxxxxxxx;
> alim.akhtar@xxxxxxxxxxx; ravi.patel@xxxxxxxxxxx; Jayati Sahu
> <jayati.sahu@xxxxxxxxxxx>
> Subject: Re: [PATCH v2 3/4] arm64: dts: fsd: Add Ethernet support for FSYS0
> Block of FSD SoC
> 
> On 11/01/2023 08:54, Sriranjani P wrote:
> > The FSD SoC contains two instances of Synopsys DWC QoS Ethernet IP,
> > one in FSYS0 block and other in PERIC block.
> >
> > Adds device tree node for Ethernet in FSYS0 Block and enables the same
> > for FSD platform.
> >
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@xxxxxxxxxxx>
> > Signed-off-by: Jayati Sahu <jayati.sahu@xxxxxxxxxxx>
> > Signed-off-by: Sriranjani P <sriranjani.p@xxxxxxxxxxx>
> > ---
> >  arch/arm64/boot/dts/tesla/fsd-evb.dts      |  9 ++++
> >  arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 56
> ++++++++++++++++++++++
> >  arch/arm64/boot/dts/tesla/fsd.dtsi         | 22 +++++++++
> >  3 files changed, 87 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > index 1db6ddf03f01..ca0c1a28d562 100644
> > --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > @@ -30,6 +30,15 @@
> >  	};
> >  };
> >
> > +&ethernet_0 {
> > +	status = "okay";
> > +
> > +	fixed-link {
> > +		speed = <1000>;
> > +		full-duplex;
> > +	};
> > +};
> > +
> >  &fin_pll {
> >  	clock-frequency = <24000000>;
> >  };
> > diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> > b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> > index d0abb9aa0e9e..7ccc0738a149 100644
> > --- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> > +++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> > @@ -64,6 +64,62 @@
> >  		samsung,pin-pud = <FSD_PIN_PULL_NONE>;
> >  		samsung,pin-drv = <FSD_PIN_DRV_LV2>;
> >  	};
> > +
> > +		eth0_tx_clk: eth0-tx-clk-pins {
> 
> Wrong indentation.
Will fix in the next version.
> 
> > +		samsung,pins = "gpf0-0";
> > +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> > +		samsung,pin-pud = <FSD_PIN_PULL_DOWN>;
> > +		samsung,pin-drv = <FSD_PIN_DRV_LV6>;
> > +	};
> > +
> > +	eth0_tx_data: eth0-tx-data-pins {
> > +		samsung,pins = "gpf0-1", "gpf0-2", "gpf0-3", "gpf0-4";
> > +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> > +		samsung,pin-pud = <FSD_PIN_PULL_UP>;
> > +		samsung,pin-drv = <FSD_PIN_DRV_LV6>;
> > +	};
> > +
> > +	eth0_tx_ctrl: eth0-tx-ctrl-pins {
> > +		samsung,pins = "gpf0-5";
> > +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> > +		samsung,pin-pud = <FSD_PIN_PULL_UP>;
> > +		samsung,pin-drv = <FSD_PIN_DRV_LV6>;
> > +	};
> > +
> > +	eth0_phy_intr: eth0-phy-intr-pins {
> > +		samsung,pins = "gpf0-6";
> > +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> > +		samsung,pin-pud = <FSD_PIN_PULL_NONE>;
> > +		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
> > +	};
> > +
> > +	eth0_rx_clk: eth0-rx-clk-pins {
> > +		samsung,pins = "gpf1-0";
> > +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> > +		samsung,pin-pud = <FSD_PIN_PULL_UP>;
> > +		samsung,pin-drv = <FSD_PIN_DRV_LV6>;
> > +	};
> > +
> > +	eth0_rx_data: eth0-rx-data-pins {
> > +		samsung,pins = "gpf1-1", "gpf1-2", "gpf1-3", "gpf1-4";
> > +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> > +		samsung,pin-pud = <FSD_PIN_PULL_UP>;
> > +		samsung,pin-drv = <FSD_PIN_DRV_LV6>;
> > +	};
> > +
> > +	eth0_rx_ctrl: eth0-rx-ctrl-pins {
> > +		samsung,pins = "gpf1-5";
> > +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> > +		samsung,pin-pud = <FSD_PIN_PULL_UP>;
> > +		samsung,pin-drv = <FSD_PIN_DRV_LV6>;
> > +	};
> > +
> > +	eth0_mdio: eth0-mdio-pins {
> > +		samsung,pins = "gpf1-6", "gpf1-7";
> > +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> > +		samsung,pin-pud = <FSD_PIN_PULL_NONE>;
> > +		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
> > +	};
> >  };
> >
> >  &pinctrl_peric {
> > diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi
> > b/arch/arm64/boot/dts/tesla/fsd.dtsi
> > index f35bc5a288c2..ade707cc646b 100644
> > --- a/arch/arm64/boot/dts/tesla/fsd.dtsi
> > +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
> > @@ -32,6 +32,7 @@
> >  		spi0 = &spi_0;
> >  		spi1 = &spi_1;
> >  		spi2 = &spi_2;
> > +		eth0 = &ethernet_0;
> 
> This is a friendly reminder during the review process.
> 
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all requested
> changes or keep discussing them.
Sorry somehow I'm not able to find my previous mail chain.
I had replied to your comment in the previous version of the mail.
In this case alias id is used to differentiate between Ethernet instance 0 and 1 in the driver code.
> 
> Thank you.
> 
> 
> Best regards,
> Krzysztof