| Message ID | 20230814112539.70453-1-sriranjani.p@samsung.com |
|---|---|
| Headers | show |
| Series | net: stmmac: dwc-qos: Add FSD EQoS support | expand |
On 14/08/2023 13:25, Sriranjani P wrote: > The FSD SoC contains two instances of Synopsys DWC QoS Ethernet IP, one > in FSYS0 block and other in PERIC block. ... > > cpus { > @@ -984,6 +985,27 @@ > clocks = <&clock_fsys0 UFS0_MPHY_REFCLK_IXTAL26>; > clock-names = "ref_clk"; > }; > + > + ethernet_0: ethernet@15300000 { > + compatible = "tesla,dwc-qos-ethernet-4.21"; The requirement for entire Samsung and its flavors is to pass dtbs_check. Since some months. Does it pass? Best regards, Krzysztof
On 14/08/2023 13:25, 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 PERIC Block and enables the same for > FSD platform. > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > Signed-off-by: Jayati Sahu <jayati.sahu@samsung.com> > Signed-off-by: Swathi K S <swathi.ks@samsung.com> > Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> > --- > 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 | 29 +++++++++++ > 3 files changed, 94 insertions(+) Looks duplicated. Best regards, Krzysztof
On 14/08/2023 21:41, Krzysztof Kozlowski wrote: > On 14/08/2023 13:25, 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 PERIC Block and enables the same for >> FSD platform. >> >> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> >> Signed-off-by: Jayati Sahu <jayati.sahu@samsung.com> >> Signed-off-by: Swathi K S <swathi.ks@samsung.com> >> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> >> --- >> 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 | 29 +++++++++++ >> 3 files changed, 94 insertions(+) > > Looks duplicated. Ah, not, it's another block. My question whether this was tested remains... Best regards, Krzysztof
On 14/08/2023 13:25, Sriranjani P wrote: > The FSD SoC contains two instance of the Synopsys DWC ethernet QOS IP core. > The binding that it uses is slightly different from existing ones because > of the integration (clocks, resets). > > For FSD SoC, a mux switch is needed between internal and external clocks. > By default after reset internal clock is used but for receiving packets > properly, external clock is needed. Mux switch to external clock happens > only when the external clock is present. > > Signed-off-by: Chandrasekar R <rcsekar@samsung.com> > Signed-off-by: Suresh Siddha <ssiddha@tesla.com> > Signed-off-by: Swathi K S <swathi.ks@samsung.com> > Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> > --- > +static int dwc_eqos_setup_rxclock(struct platform_device *pdev, int ins_num) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct regmap *syscon; > + unsigned int reg; > + > + if (np && of_property_read_bool(np, "fsd-rx-clock-skew")) { > + syscon = syscon_regmap_lookup_by_phandle_args(np, > + "fsd-rx-clock-skew", > + 1, ®); > + if (IS_ERR(syscon)) { > + dev_err(&pdev->dev, > + "couldn't get the rx-clock-skew syscon!\n"); > + return PTR_ERR(syscon); > + } > + > + regmap_write(syscon, reg, rx_clock_skew_val[ins_num]); > + } > + > + return 0; > +} > + > +static int fsd_eqos_clk_init(struct fsd_eqos_plat_data *plat, > + struct plat_stmmacenet_data *data) > +{ > + int ret = 0, i; > + > + const struct fsd_eqos_variant *fsd_eqos_v_data = > + plat->fsd_eqos_inst_var; > + > + plat->clks = devm_kcalloc(plat->dev, fsd_eqos_v_data->num_clks, > + sizeof(*plat->clks), GFP_KERNEL); > + if (!plat->clks) > + return -ENOMEM; > + > + for (i = 0; i < fsd_eqos_v_data->num_clks; i++) > + plat->clks[i].id = fsd_eqos_v_data->clk_list[i]; > + > + ret = devm_clk_bulk_get(plat->dev, fsd_eqos_v_data->num_clks, > + plat->clks); Instead of duplicating entire clock management with existing code, you should extend/rework existing one. This code is unfortunately great example how not to stuff vendor code into upstream project. :( > + > + return ret; > +} > + > +static int fsd_clks_endisable(void *priv, bool enabled) > +{ > + int ret, num_clks; > + struct fsd_eqos_plat_data *plat = priv; > + > + num_clks = plat->fsd_eqos_inst_var->num_clks; > + > + if (enabled) { > + ret = clk_bulk_prepare_enable(num_clks, plat->clks); > + if (ret) { > + dev_err(plat->dev, "Clock enable failed, err = %d\n", ret); > + return ret; > + } > + } else { > + clk_bulk_disable_unprepare(num_clks, plat->clks); > + } > + > + return 0; > +} > + > +static int fsd_eqos_probe(struct platform_device *pdev, > + struct plat_stmmacenet_data *data, > + struct stmmac_resources *res) > +{ > + struct fsd_eqos_plat_data *priv_plat; > + struct device_node *np = pdev->dev.of_node; > + int ret = 0; > + > + priv_plat = devm_kzalloc(&pdev->dev, sizeof(*priv_plat), GFP_KERNEL); > + if (!priv_plat) { > + ret = -ENOMEM; return -ENOMEM > + goto error; > + } > + > + priv_plat->dev = &pdev->dev; > + data->bus_id = of_alias_get_id(np, "eth"); No, you cannot do like this. Aliases are board specific and are based on labeling on the board. > + > + priv_plat->fsd_eqos_inst_var = &fsd_eqos_clk_info[data->bus_id]; > + > + ret = fsd_eqos_clk_init(priv_plat, data); > + > + data->bsp_priv = priv_plat; > + data->clks_config = fsd_clks_endisable; > + data->rxmux_setup = dwc_eqos_rxmux_setup; > + > + ret = fsd_clks_endisable(priv_plat, true); > + if (ret) > + goto error; > + > + ret = dwc_eqos_setup_rxclock(pdev, data->bus_id); > + if (ret) { > + fsd_clks_endisable(priv_plat, false); > + dev_err_probe(&pdev->dev, ret, "Unable to setup rxclock\n"); The syntax is: return dev_err_probe(). > + } > + > +error: > + return ret; > +} .... Best regards, Krzysztof
On 14/08/2023 13:25, 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. > ... > &pinctrl_peric { > diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi > index 1c53c68efd53..9a991f021711 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 = ðernet_0; One more thing - I said it two times already. Patch v1 and then in v2. You responded now without waiting for my further feedback and immediately sent the same stuff. Let's be clear: NAK for the reasons I said multiple times. Best regards, Krzysztof
> +static const int rx_clock_skew_val[] = {0x2, 0x0}; > +static int dwc_eqos_setup_rxclock(struct platform_device *pdev, int ins_num) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct regmap *syscon; > + unsigned int reg; > + > + if (np && of_property_read_bool(np, "fsd-rx-clock-skew")) { > + syscon = syscon_regmap_lookup_by_phandle_args(np, > + "fsd-rx-clock-skew", > + 1, ®); > + if (IS_ERR(syscon)) { > + dev_err(&pdev->dev, > + "couldn't get the rx-clock-skew syscon!\n"); > + return PTR_ERR(syscon); > + } > + > + regmap_write(syscon, reg, rx_clock_skew_val[ins_num]); Please could you explain what this is doing. Andrew
> +ðernet_0 { > + status = "okay"; > + > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > +}; A fixed link on its own is pretty unusual. Normally it is combined with an Ethernet switch. What is the link peer here? Andrew
> diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi > index 1c53c68efd53..9a991f021711 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 = ðernet_0; > }; > > cpus { > @@ -984,6 +985,27 @@ > clocks = <&clock_fsys0 UFS0_MPHY_REFCLK_IXTAL26>; > clock-names = "ref_clk"; > }; > + > + ethernet_0: ethernet@15300000 { > + compatible = "tesla,dwc-qos-ethernet-4.21"; > + reg = <0x0 0x15300000 0x0 0x10000>; > + interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clock_fsys0 FSYS0_EQOS_TOP0_IPCLKPORT_CLK_PTP_REF_I>, > + <&clock_fsys0 FSYS0_EQOS_TOP0_IPCLKPORT_ACLK_I>, > + <&clock_fsys0 FSYS0_EQOS_TOP0_IPCLKPORT_HCLK_I>, > + <&clock_fsys0 FSYS0_EQOS_TOP0_IPCLKPORT_RGMII_CLK_I>, > + <&clock_fsys0 FSYS0_EQOS_TOP0_IPCLKPORT_CLK_RX_I>; > + clock-names = "ptp_ref", "master_bus", "slave_bus", "tx", "rx"; > + pinctrl-names = "default"; > + pinctrl-0 = <ð0_tx_clk>, <ð0_tx_data>, <ð0_tx_ctrl>, > + <ð0_phy_intr>, <ð0_rx_clk>, <ð0_rx_data>, > + <ð0_rx_ctrl>, <ð0_mdio>; > + local-mac-address = [00 00 00 00 00 00]; > + fsd-rx-clock-skew = <&sysreg_fsys0 0x0>; > + iommus = <&smmu_fsys0 0x0 0x1>; > + phy-mode = "rgmii"; What is inserting the RGMII delays? Andrew
> +ðernet_1 { > + status = "okay"; > + > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > +}; So the exact same comments i made for patch 3/4 apply here. Andrew
> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] > Sent: 15 August 2023 01:12 > To: Sriranjani P <sriranjani.p@samsung.com>; 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; Jayati Sahu <jayati.sahu@samsung.com> > Subject: Re: [PATCH v3 4/4] arm64: dts: fsd: Add Ethernet support for PERIC > Block of FSD SoC > > On 14/08/2023 21:41, Krzysztof Kozlowski wrote: > > On 14/08/2023 13:25, 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 PERIC Block and enables the > >> same for FSD platform. > >> > >> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > >> Signed-off-by: Jayati Sahu <jayati.sahu@samsung.com> > >> Signed-off-by: Swathi K S <swathi.ks@samsung.com> > >> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> > >> --- > >> 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 | 29 +++++++++++ > >> 3 files changed, 94 insertions(+) > > > > Looks duplicated. > > Ah, not, it's another block. > > My question whether this was tested remains... I understand your concern. It was tested but just before posting I addressed one of earlier review comment of compatible and I missed to reflect it in all the patches. I will take care. > > Best regards, > Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] > Sent: 15 August 2023 01:21 > To: Sriranjani P <sriranjani.p@samsung.com>; 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; Chandrasekar R <rcsekar@samsung.com>; > Suresh Siddha <ssiddha@tesla.com> > Subject: Re: [PATCH v3 2/4] net: stmmac: dwc-qos: Add FSD EQoS support > > On 14/08/2023 13:25, Sriranjani P wrote: > > The FSD SoC contains two instance of the Synopsys DWC ethernet QOS IP > core. > > The binding that it uses is slightly different from existing ones > > because of the integration (clocks, resets). > > > > For FSD SoC, a mux switch is needed between internal and external clocks. > > By default after reset internal clock is used but for receiving > > packets properly, external clock is needed. Mux switch to external > > clock happens only when the external clock is present. > > > > Signed-off-by: Chandrasekar R <rcsekar@samsung.com> > > Signed-off-by: Suresh Siddha <ssiddha@tesla.com> > > Signed-off-by: Swathi K S <swathi.ks@samsung.com> > > Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> > > --- > > > > +static int dwc_eqos_setup_rxclock(struct platform_device *pdev, int > > +ins_num) { > > + struct device_node *np = pdev->dev.of_node; > > + struct regmap *syscon; > > + unsigned int reg; > > + > > + if (np && of_property_read_bool(np, "fsd-rx-clock-skew")) { > > + syscon = syscon_regmap_lookup_by_phandle_args(np, > > + "fsd-rx-clock- > skew", > > + 1, ®); > > + if (IS_ERR(syscon)) { > > + dev_err(&pdev->dev, > > + "couldn't get the rx-clock-skew syscon!\n"); > > + return PTR_ERR(syscon); > > + } > > + > > + regmap_write(syscon, reg, rx_clock_skew_val[ins_num]); > > + } > > + > > + return 0; > > +} > > + > > +static int fsd_eqos_clk_init(struct fsd_eqos_plat_data *plat, > > + struct plat_stmmacenet_data *data) { > > + int ret = 0, i; > > + > > + const struct fsd_eqos_variant *fsd_eqos_v_data = > > + plat->fsd_eqos_inst_var; > > + > > + plat->clks = devm_kcalloc(plat->dev, fsd_eqos_v_data->num_clks, > > + sizeof(*plat->clks), GFP_KERNEL); > > + if (!plat->clks) > > + return -ENOMEM; > > + > > + for (i = 0; i < fsd_eqos_v_data->num_clks; i++) > > + plat->clks[i].id = fsd_eqos_v_data->clk_list[i]; > > + > > + ret = devm_clk_bulk_get(plat->dev, fsd_eqos_v_data->num_clks, > > + plat->clks); > > Instead of duplicating entire clock management with existing code, you > should extend/rework existing one. > > This code is unfortunately great example how not to stuff vendor code into > upstream project. :( I will check again if I can extend existing one to support FSD platform specific requirement. > > > + > > + return ret; > > +} > > + > > +static int fsd_clks_endisable(void *priv, bool enabled) { > > + int ret, num_clks; > > + struct fsd_eqos_plat_data *plat = priv; > > + > > + num_clks = plat->fsd_eqos_inst_var->num_clks; > > + > > + if (enabled) { > > + ret = clk_bulk_prepare_enable(num_clks, plat->clks); > > + if (ret) { > > + dev_err(plat->dev, "Clock enable failed, err = %d\n", > ret); > > + return ret; > > + } > > + } else { > > + clk_bulk_disable_unprepare(num_clks, plat->clks); > > + } > > + > > + return 0; > > +} > > + > > +static int fsd_eqos_probe(struct platform_device *pdev, > > + struct plat_stmmacenet_data *data, > > + struct stmmac_resources *res) > > +{ > > + struct fsd_eqos_plat_data *priv_plat; > > + struct device_node *np = pdev->dev.of_node; > > + int ret = 0; > > + > > + priv_plat = devm_kzalloc(&pdev->dev, sizeof(*priv_plat), > GFP_KERNEL); > > + if (!priv_plat) { > > + ret = -ENOMEM; > > return -ENOMEM Will fix this in v4. > > > + goto error; > > + } > > + > > + priv_plat->dev = &pdev->dev; > > + data->bus_id = of_alias_get_id(np, "eth"); > > No, you cannot do like this. Aliases are board specific and are based on > labeling on the board. So if I understood this correctly, I need to move alias in the board specific DTS file and I can use this, because we have to handle rx-clock-skew differently for the two instances in the FSD platform. Another approach we took in v1, by specifying the value to be programmed in rx-clock-skew property itself, but it seems it is not a preferred approach. I can see that in drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +436 common code is already using this API and getting alias id, so I can probably use the same rather getting same again here, but I have to specify alias in DTS file. > > > + > > + priv_plat->fsd_eqos_inst_var = &fsd_eqos_clk_info[data->bus_id]; > > + > > + ret = fsd_eqos_clk_init(priv_plat, data); > > + > > + data->bsp_priv = priv_plat; > > + data->clks_config = fsd_clks_endisable; > > + data->rxmux_setup = dwc_eqos_rxmux_setup; > > + > > + ret = fsd_clks_endisable(priv_plat, true); > > + if (ret) > > + goto error; > > + > > + ret = dwc_eqos_setup_rxclock(pdev, data->bus_id); > > + if (ret) { > > + fsd_clks_endisable(priv_plat, false); > > + dev_err_probe(&pdev->dev, ret, "Unable to setup > rxclock\n"); > > The syntax is: return dev_err_probe(). Will fix it in v4. > > > + } > > + > > +error: > > + return ret; > > +} > > .... > > > Best regards, > Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] > Sent: 15 August 2023 01:27 > To: Sriranjani P <sriranjani.p@samsung.com>; 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; Jayati Sahu <jayati.sahu@samsung.com> > Subject: Re: [PATCH v3 3/4] arm64: dts: fsd: Add Ethernet support for FSYS0 > Block of FSD SoC > > On 14/08/2023 13:25, 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. > > > > ... > > > &pinctrl_peric { > > diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi > > b/arch/arm64/boot/dts/tesla/fsd.dtsi > > index 1c53c68efd53..9a991f021711 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 = ðernet_0; > > One more thing - I said it two times already. Patch v1 and then in v2. > You responded now without waiting for my further feedback and > immediately sent the same stuff. > > Let's be clear: > > NAK for the reasons I said multiple times. Got it, will move this alias in board specific dts file. > > Best regards, > Krzysztof
On 16/08/2023 08:38, Sriranjani P wrote: > > >> -----Original Message----- >> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] >> Sent: 15 August 2023 01:21 >> To: Sriranjani P <sriranjani.p@samsung.com>; 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; Chandrasekar R <rcsekar@samsung.com>; >> Suresh Siddha <ssiddha@tesla.com> >> Subject: Re: [PATCH v3 2/4] net: stmmac: dwc-qos: Add FSD EQoS support >> >> On 14/08/2023 13:25, Sriranjani P wrote: >>> The FSD SoC contains two instance of the Synopsys DWC ethernet QOS IP >> core. >>> The binding that it uses is slightly different from existing ones >>> because of the integration (clocks, resets). >>> >>> For FSD SoC, a mux switch is needed between internal and external clocks. >>> By default after reset internal clock is used but for receiving >>> packets properly, external clock is needed. Mux switch to external >>> clock happens only when the external clock is present. >>> >>> Signed-off-by: Chandrasekar R <rcsekar@samsung.com> >>> Signed-off-by: Suresh Siddha <ssiddha@tesla.com> >>> Signed-off-by: Swathi K S <swathi.ks@samsung.com> >>> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> >>> --- >> >> >>> +static int dwc_eqos_setup_rxclock(struct platform_device *pdev, int >>> +ins_num) { >>> + struct device_node *np = pdev->dev.of_node; >>> + struct regmap *syscon; >>> + unsigned int reg; >>> + >>> + if (np && of_property_read_bool(np, "fsd-rx-clock-skew")) { >>> + syscon = syscon_regmap_lookup_by_phandle_args(np, >>> + "fsd-rx-clock- >> skew", >>> + 1, ®); >>> + if (IS_ERR(syscon)) { >>> + dev_err(&pdev->dev, >>> + "couldn't get the rx-clock-skew syscon!\n"); >>> + return PTR_ERR(syscon); >>> + } >>> + >>> + regmap_write(syscon, reg, rx_clock_skew_val[ins_num]); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static int fsd_eqos_clk_init(struct fsd_eqos_plat_data *plat, >>> + struct plat_stmmacenet_data *data) { >>> + int ret = 0, i; >>> + >>> + const struct fsd_eqos_variant *fsd_eqos_v_data = >>> + plat->fsd_eqos_inst_var; >>> + >>> + plat->clks = devm_kcalloc(plat->dev, fsd_eqos_v_data->num_clks, >>> + sizeof(*plat->clks), GFP_KERNEL); >>> + if (!plat->clks) >>> + return -ENOMEM; >>> + >>> + for (i = 0; i < fsd_eqos_v_data->num_clks; i++) >>> + plat->clks[i].id = fsd_eqos_v_data->clk_list[i]; >>> + >>> + ret = devm_clk_bulk_get(plat->dev, fsd_eqos_v_data->num_clks, >>> + plat->clks); >> >> Instead of duplicating entire clock management with existing code, you >> should extend/rework existing one. >> >> This code is unfortunately great example how not to stuff vendor code into >> upstream project. :( > > I will check again if I can extend existing one to support FSD platform specific requirement. > >> >>> + >>> + return ret; >>> +} >>> + >>> +static int fsd_clks_endisable(void *priv, bool enabled) { >>> + int ret, num_clks; >>> + struct fsd_eqos_plat_data *plat = priv; >>> + >>> + num_clks = plat->fsd_eqos_inst_var->num_clks; >>> + >>> + if (enabled) { >>> + ret = clk_bulk_prepare_enable(num_clks, plat->clks); >>> + if (ret) { >>> + dev_err(plat->dev, "Clock enable failed, err = %d\n", >> ret); >>> + return ret; >>> + } >>> + } else { >>> + clk_bulk_disable_unprepare(num_clks, plat->clks); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static int fsd_eqos_probe(struct platform_device *pdev, >>> + struct plat_stmmacenet_data *data, >>> + struct stmmac_resources *res) >>> +{ >>> + struct fsd_eqos_plat_data *priv_plat; >>> + struct device_node *np = pdev->dev.of_node; >>> + int ret = 0; >>> + >>> + priv_plat = devm_kzalloc(&pdev->dev, sizeof(*priv_plat), >> GFP_KERNEL); >>> + if (!priv_plat) { >>> + ret = -ENOMEM; >> >> return -ENOMEM > > Will fix this in v4. > >> >>> + goto error; >>> + } >>> + >>> + priv_plat->dev = &pdev->dev; >>> + data->bus_id = of_alias_get_id(np, "eth"); >> >> No, you cannot do like this. Aliases are board specific and are based on >> labeling on the board. > > So if I understood this correctly, I need to move alias in the board specific DTS file This part: yes > and I can use this, because we have to handle rx-clock-skew differently for the two instances in the FSD platform. Not really. Do you expect it to work correctly if given EQoS instance receives different alias, e.g. 5? > Another approach we took in v1, by specifying the value to be programmed in rx-clock-skew property itself, but it seems it is not a preferred approach. > I can see that in drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +436 common code is already using this API and getting alias id, so I can probably use the same rather getting same again here, but I have to specify alias in DTS file. Getting alias ID is okay in general. It is used to provide user-visible ID of the devices (see mmc). Using such alias to configure hardware is abuse of the alias, because of the reasons I said - how is it supposed to work if alias is 5 (this is perfectly valid alias)? I suspect that all this is to substitute missing abstractions, like clocks, phys or resets... Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] > Sent: 18 August 2023 14:57 > To: Sriranjani P <sriranjani.p@samsung.com>; 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; 'Chandrasekar R' <rcsekar@samsung.com>; > 'Suresh Siddha' <ssiddha@tesla.com> > Subject: Re: [PATCH v3 2/4] net: stmmac: dwc-qos: Add FSD EQoS support > > On 16/08/2023 08:38, Sriranjani P wrote: > > > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] > >> Sent: 15 August 2023 01:21 > >> To: Sriranjani P <sriranjani.p@samsung.com>; 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; Chandrasekar R <rcsekar@samsung.com>; > >> Suresh Siddha <ssiddha@tesla.com> > >> Subject: Re: [PATCH v3 2/4] net: stmmac: dwc-qos: Add FSD EQoS > >> support > >> > >> On 14/08/2023 13:25, Sriranjani P wrote: > >>> The FSD SoC contains two instance of the Synopsys DWC ethernet QOS > >>> IP > >> core. > >>> The binding that it uses is slightly different from existing ones > >>> because of the integration (clocks, resets). > >>> > >>> For FSD SoC, a mux switch is needed between internal and external > clocks. > >>> By default after reset internal clock is used but for receiving > >>> packets properly, external clock is needed. Mux switch to external > >>> clock happens only when the external clock is present. > >>> > >>> Signed-off-by: Chandrasekar R <rcsekar@samsung.com> > >>> Signed-off-by: Suresh Siddha <ssiddha@tesla.com> > >>> Signed-off-by: Swathi K S <swathi.ks@samsung.com> > >>> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> > >>> --- > >> > >> > >>> +static int dwc_eqos_setup_rxclock(struct platform_device *pdev, int > >>> +ins_num) { > >>> + struct device_node *np = pdev->dev.of_node; > >>> + struct regmap *syscon; > >>> + unsigned int reg; > >>> + > >>> + if (np && of_property_read_bool(np, "fsd-rx-clock-skew")) { > >>> + syscon = syscon_regmap_lookup_by_phandle_args(np, > >>> + "fsd-rx-clock- > >> skew", > >>> + 1, ®); > >>> + if (IS_ERR(syscon)) { > >>> + dev_err(&pdev->dev, > >>> + "couldn't get the rx-clock-skew syscon!\n"); > >>> + return PTR_ERR(syscon); > >>> + } > >>> + > >>> + regmap_write(syscon, reg, rx_clock_skew_val[ins_num]); > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static int fsd_eqos_clk_init(struct fsd_eqos_plat_data *plat, > >>> + struct plat_stmmacenet_data *data) { > >>> + int ret = 0, i; > >>> + > >>> + const struct fsd_eqos_variant *fsd_eqos_v_data = > >>> + plat->fsd_eqos_inst_var; > >>> + > >>> + plat->clks = devm_kcalloc(plat->dev, fsd_eqos_v_data->num_clks, > >>> + sizeof(*plat->clks), GFP_KERNEL); > >>> + if (!plat->clks) > >>> + return -ENOMEM; > >>> + > >>> + for (i = 0; i < fsd_eqos_v_data->num_clks; i++) > >>> + plat->clks[i].id = fsd_eqos_v_data->clk_list[i]; > >>> + > >>> + ret = devm_clk_bulk_get(plat->dev, fsd_eqos_v_data->num_clks, > >>> + plat->clks); > >> > >> Instead of duplicating entire clock management with existing code, > >> you should extend/rework existing one. > >> > >> This code is unfortunately great example how not to stuff vendor code > >> into upstream project. :( > > > > I will check again if I can extend existing one to support FSD platform > specific requirement. > > > >> > >>> + > >>> + return ret; > >>> +} > >>> + > >>> +static int fsd_clks_endisable(void *priv, bool enabled) { > >>> + int ret, num_clks; > >>> + struct fsd_eqos_plat_data *plat = priv; > >>> + > >>> + num_clks = plat->fsd_eqos_inst_var->num_clks; > >>> + > >>> + if (enabled) { > >>> + ret = clk_bulk_prepare_enable(num_clks, plat->clks); > >>> + if (ret) { > >>> + dev_err(plat->dev, "Clock enable failed, err = %d\n", > >> ret); > >>> + return ret; > >>> + } > >>> + } else { > >>> + clk_bulk_disable_unprepare(num_clks, plat->clks); > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static int fsd_eqos_probe(struct platform_device *pdev, > >>> + struct plat_stmmacenet_data *data, > >>> + struct stmmac_resources *res) > >>> +{ > >>> + struct fsd_eqos_plat_data *priv_plat; > >>> + struct device_node *np = pdev->dev.of_node; > >>> + int ret = 0; > >>> + > >>> + priv_plat = devm_kzalloc(&pdev->dev, sizeof(*priv_plat), > >> GFP_KERNEL); > >>> + if (!priv_plat) { > >>> + ret = -ENOMEM; > >> > >> return -ENOMEM > > > > Will fix this in v4. > > > >> > >>> + goto error; > >>> + } > >>> + > >>> + priv_plat->dev = &pdev->dev; > >>> + data->bus_id = of_alias_get_id(np, "eth"); > >> > >> No, you cannot do like this. Aliases are board specific and are based > >> on labeling on the board. > > > > So if I understood this correctly, I need to move alias in the board > > specific DTS file > > This part: yes > > > and I can use this, because we have to handle rx-clock-skew differently for > the two instances in the FSD platform. > > Not really. Do you expect it to work correctly if given EQoS instance receives > different alias, e.g. 5? > > > Another approach we took in v1, by specifying the value to be programmed > in rx-clock-skew property itself, but it seems it is not a preferred approach. > > I can see that in drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > +436 common code is already using this API and getting alias id, so I can > probably use the same rather getting same again here, but I have to specify > alias in DTS file. > > Getting alias ID is okay in general. It is used to provide user-visible ID of the > devices (see mmc). Using such alias to configure hardware is abuse of the > alias, because of the reasons I said - how is it supposed to work if alias is 5 > (this is perfectly valid alias)? > > I suspect that all this is to substitute missing abstractions, like clocks, phys or > resets... Will avoid using the API to get alias id to configure the HW. Will share the new implementation in v4. > > Best regards, > Krzysztof Regards, Swathi
> -----Original Message----- > From: Andrew Lunn [mailto:andrew@lunn.ch] > Sent: 15 August 2023 02:20 > To: Sriranjani P <sriranjani.p@samsung.com> > Cc: 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; 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; Jayati Sahu > <jayati.sahu@samsung.com> > Subject: Re: [PATCH v3 3/4] arm64: dts: fsd: Add Ethernet support for FSYS0 > Block of FSD SoC > > > +ðernet_0 { > > + status = "okay"; > > + > > + fixed-link { > > + speed = <1000>; > > + full-duplex; > > + }; > > +}; > > A fixed link on its own is pretty unusual. Normally it is combined with an > Ethernet switch. What is the link peer here? It is a direct connection to the Ethernet switch managed by an external management unit. > > Andrew Regards, Swathi
> -----Original Message----- > From: Andrew Lunn [mailto:andrew@lunn.ch] > Sent: 15 August 2023 02:17 > To: Sriranjani P <sriranjani.p@samsung.com> > Cc: 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; 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; Chandrasekar R > <rcsekar@samsung.com>; Suresh Siddha <ssiddha@tesla.com> > Subject: Re: [PATCH v3 2/4] net: stmmac: dwc-qos: Add FSD EQoS support > > > +static const int rx_clock_skew_val[] = {0x2, 0x0}; > > > +static int dwc_eqos_setup_rxclock(struct platform_device *pdev, int > > +ins_num) { > > + struct device_node *np = pdev->dev.of_node; > > + struct regmap *syscon; > > + unsigned int reg; > > + > > + if (np && of_property_read_bool(np, "fsd-rx-clock-skew")) { > > + syscon = syscon_regmap_lookup_by_phandle_args(np, > > + "fsd-rx-clock- > skew", > > + 1, ®); > > + if (IS_ERR(syscon)) { > > + dev_err(&pdev->dev, > > + "couldn't get the rx-clock-skew syscon!\n"); > > + return PTR_ERR(syscon); > > + } > > + > > + regmap_write(syscon, reg, rx_clock_skew_val[ins_num]); > > Please could you explain what this is doing. As per customer requirement, we need to provide a delay of 2ns in FSYS in both TX and RX path and no delay in peric block > > Andrew Regards, Swathi
On 06/06/2024 11:14, Swathi K S wrote: > > >> -----Original Message----- >> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] >> Sent: 18 August 2023 14:57 >> To: Sriranjani P <sriranjani.p@samsung.com>; 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; 'Chandrasekar R' <rcsekar@samsung.com>; >> 'Suresh Siddha' <ssiddha@tesla.com> >> Subject: Re: [PATCH v3 2/4] net: stmmac: dwc-qos: Add FSD EQoS support >> >> On 16/08/2023 08:38, Sriranjani P wrote: >>> >>> >>>> -----Original Message----- >>>> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] >>>> Sent: 15 August 2023 01:21 >>>> To: Sriranjani P <sriranjani.p@samsung.com>; 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; Chandrasekar R <rcsekar@samsung.com>; >>>> Suresh Siddha <ssiddha@tesla.com> >>>> Subject: Re: [PATCH v3 2/4] net: stmmac: dwc-qos: Add FSD EQoS >>>> support >>>> >>>> On 14/08/2023 13:25, Sriranjani P wrote: >>>>> The FSD SoC contains two instance of the Synopsys DWC ethernet QOS >>>>> IP >>>> core. >>>>> The binding that it uses is slightly different from existing ones >>>>> because of the integration (clocks, resets). >>>>> >>>>> For FSD SoC, a mux switch is needed between internal and external >> clocks. >>>>> By default after reset internal clock is used but for receiving >>>>> packets properly, external clock is needed. Mux switch to external >>>>> clock happens only when the external clock is present. >>>>> >>>>> Signed-off-by: Chandrasekar R <rcsekar@samsung.com> >>>>> Signed-off-by: Suresh Siddha <ssiddha@tesla.com> >>>>> Signed-off-by: Swathi K S <swathi.ks@samsung.com> >>>>> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> >>>>> --- >>>> >>>> >>>>> +static int dwc_eqos_setup_rxclock(struct platform_device *pdev, int >>>>> +ins_num) { >>>>> + struct device_node *np = pdev->dev.of_node; >>>>> + struct regmap *syscon; >>>>> + unsigned int reg; >>>>> + >>>>> + if (np && of_property_read_bool(np, "fsd-rx-clock-skew")) { >>>>> + syscon = syscon_regmap_lookup_by_phandle_args(np, >>>>> + "fsd-rx-clock- >>>> skew", >>>>> + 1, ®); >>>>> + if (IS_ERR(syscon)) { >>>>> + dev_err(&pdev->dev, >>>>> + "couldn't get the rx-clock-skew syscon!\n"); >>>>> + return PTR_ERR(syscon); >>>>> + } >>>>> + >>>>> + regmap_write(syscon, reg, rx_clock_skew_val[ins_num]); >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int fsd_eqos_clk_init(struct fsd_eqos_plat_data *plat, >>>>> + struct plat_stmmacenet_data *data) { >>>>> + int ret = 0, i; >>>>> + >>>>> + const struct fsd_eqos_variant *fsd_eqos_v_data = >>>>> + plat->fsd_eqos_inst_var; >>>>> + >>>>> + plat->clks = devm_kcalloc(plat->dev, fsd_eqos_v_data->num_clks, >>>>> + sizeof(*plat->clks), GFP_KERNEL); >>>>> + if (!plat->clks) >>>>> + return -ENOMEM; >>>>> + >>>>> + for (i = 0; i < fsd_eqos_v_data->num_clks; i++) >>>>> + plat->clks[i].id = fsd_eqos_v_data->clk_list[i]; >>>>> + >>>>> + ret = devm_clk_bulk_get(plat->dev, fsd_eqos_v_data->num_clks, >>>>> + plat->clks); >>>> >>>> Instead of duplicating entire clock management with existing code, >>>> you should extend/rework existing one. >>>> >>>> This code is unfortunately great example how not to stuff vendor code >>>> into upstream project. :( >>> >>> I will check again if I can extend existing one to support FSD platform >> specific requirement. >>> >>>> >>>>> + >>>>> + return ret; >>>>> +} >>>>> + >>>>> +static int fsd_clks_endisable(void *priv, bool enabled) { >>>>> + int ret, num_clks; >>>>> + struct fsd_eqos_plat_data *plat = priv; >>>>> + >>>>> + num_clks = plat->fsd_eqos_inst_var->num_clks; >>>>> + >>>>> + if (enabled) { >>>>> + ret = clk_bulk_prepare_enable(num_clks, plat->clks); >>>>> + if (ret) { >>>>> + dev_err(plat->dev, "Clock enable failed, err = %d\n", >>>> ret); >>>>> + return ret; >>>>> + } >>>>> + } else { >>>>> + clk_bulk_disable_unprepare(num_clks, plat->clks); >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int fsd_eqos_probe(struct platform_device *pdev, >>>>> + struct plat_stmmacenet_data *data, >>>>> + struct stmmac_resources *res) >>>>> +{ >>>>> + struct fsd_eqos_plat_data *priv_plat; >>>>> + struct device_node *np = pdev->dev.of_node; >>>>> + int ret = 0; >>>>> + >>>>> + priv_plat = devm_kzalloc(&pdev->dev, sizeof(*priv_plat), >>>> GFP_KERNEL); >>>>> + if (!priv_plat) { >>>>> + ret = -ENOMEM; >>>> >>>> return -ENOMEM >>> >>> Will fix this in v4. >>> >>>> >>>>> + goto error; >>>>> + } >>>>> + >>>>> + priv_plat->dev = &pdev->dev; >>>>> + data->bus_id = of_alias_get_id(np, "eth"); >>>> >>>> No, you cannot do like this. Aliases are board specific and are based >>>> on labeling on the board. >>> >>> So if I understood this correctly, I need to move alias in the board >>> specific DTS file >> >> This part: yes >> >>> and I can use this, because we have to handle rx-clock-skew differently for >> the two instances in the FSD platform. >> >> Not really. Do you expect it to work correctly if given EQoS instance receives >> different alias, e.g. 5? >> >>> Another approach we took in v1, by specifying the value to be programmed >> in rx-clock-skew property itself, but it seems it is not a preferred approach. >>> I can see that in drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c >> +436 common code is already using this API and getting alias id, so I can >> probably use the same rather getting same again here, but I have to specify >> alias in DTS file. >> >> Getting alias ID is okay in general. It is used to provide user-visible ID of the >> devices (see mmc). Using such alias to configure hardware is abuse of the >> alias, because of the reasons I said - how is it supposed to work if alias is 5 >> (this is perfectly valid alias)? >> >> I suspect that all this is to substitute missing abstractions, like clocks, phys or >> resets... > > Will avoid using the API to get alias id to configure the HW. Will share the new implementation in v4. That was August 2023, almost year ago. Whatever you plan, expect having same questions in the discussion because we forgot everything said that year ago... Best regards, Krzysztof
> > > +ðernet_0 { > > > + status = "okay"; > > > + > > > + fixed-link { > > > + speed = <1000>; > > > + full-duplex; > > > + }; > > > +}; > > > > A fixed link on its own is pretty unusual. Normally it is combined with an > > Ethernet switch. What is the link peer here? > > It is a direct connection to the Ethernet switch managed by an external > management unit. Ah, interesting. This is the third example of this in about a month. Take a look at the Realtek and TI work in this area. So, i will ask the same questions i put to Realtek and TI. Does Linux know about the switch in any way? Can it manage the switch, other than SNMP, HTTP from user space? Does it know about the state of the ports, etc? If you say this is just a colocated management switch, which Linux is not managing in any way, that is O.K. If you have Linux involved in some way, please join the discussion with TI about adding a new model for semi-autonomous switches. Andrew