[PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay

[PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay

[Patrick Delaunay]

The gpio reset assert/deassert delay are today harcoded in U-Boot driver
but the value should be read from DT.

STM32 use the generic binding defined in linux:
Documentation/devicetree/bindings/net/ethernet-phy.yaml

  reset-gpios:
    maxItems: 1
    description:
      The GPIO phandle and specifier for the PHY reset signal.

  reset-assert-us:
    description:
      Delay after the reset was asserted in microseconds. If this
      property is missing the delay will be skipped.

  reset-deassert-us:
    description:
      Delay after the reset was deasserted in microseconds. If
      this property is missing the delay will be skipped.

See also U-Boot: doc/device-tree-bindings/net/phy.txt

This patch moves these 2 delay configuration in the driver platdata
and manages this ethernet phy binding for STM32 variant;
when the properties are absent, the driver use the existing value = 2
and the minimal supported udelay = 1 to avoid issue with udelay(0)
when the the property is absent.

This patch also updates the tegra186 part to use the modified platdata,
even if the GPIO reset delay value is hardcoded in probe function.

Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---

 drivers/net/dwc_eth_qos.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
index e8242ca4e1..6ef750a348 100644
--- a/drivers/net/dwc_eth_qos.c
+++ b/drivers/net/dwc_eth_qos.c
@@ -293,6 +293,13 @@ struct eqos_ops {
 	ulong (*eqos_get_tick_clk_rate)(struct udevice *dev);
 };
 
+/* extend the u-class platdata (eth_pdata) with EQOS driver platdata */
+struct eqos_pdata {
+	struct eth_pdata eth_pdata;
+	u32 reset_assert_us;
+	u32 reset_deassert_us;
+};
+
 struct eqos_priv {
 	struct udevice *dev;
 	const struct eqos_config *config;
@@ -662,6 +669,7 @@ static void eqos_stop_clks_imx(struct udevice *dev)
 
 static int eqos_start_resets_tegra186(struct udevice *dev)
 {
+	struct eqos_pdata *eqos_plat = dev_get_plat(dev);
 	struct eqos_priv *eqos = dev_get_priv(dev);
 	int ret;
 
@@ -672,14 +680,14 @@ static int eqos_start_resets_tegra186(struct udevice *dev)
 		pr_err("dm_gpio_set_value(phy_reset, assert) failed: %d", ret);
 		return ret;
 	}
-
-	udelay(2);
+	udelay(eqos_plat->reset_assert_us);
 
 	ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 0);
 	if (ret < 0) {
 		pr_err("dm_gpio_set_value(phy_reset, deassert) failed: %d", ret);
 		return ret;
 	}
+	udelay(eqos_plat->reset_deassert_us);
 
 	ret = reset_assert(&eqos->reset_ctl);
 	if (ret < 0) {
@@ -701,6 +709,7 @@ static int eqos_start_resets_tegra186(struct udevice *dev)
 
 static int eqos_start_resets_stm32(struct udevice *dev)
 {
+	struct eqos_pdata *eqos_plat = dev_get_plat(dev);
 	struct eqos_priv *eqos = dev_get_priv(dev);
 	int ret;
 
@@ -712,8 +721,7 @@ static int eqos_start_resets_stm32(struct udevice *dev)
 			       ret);
 			return ret;
 		}
-
-		udelay(2);
+		udelay(eqos_plat->reset_assert_us);
 
 		ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 0);
 		if (ret < 0) {
@@ -721,6 +729,7 @@ static int eqos_start_resets_stm32(struct udevice *dev)
 			       ret);
 			return ret;
 		}
+		udelay(eqos_plat->reset_deassert_us);
 	}
 	debug("%s: OK\n", __func__);
 
@@ -1070,7 +1079,8 @@ static int eqos_adjust_link(struct udevice *dev)
 
 static int eqos_write_hwaddr(struct udevice *dev)
 {
-	struct eth_pdata *plat = dev_get_plat(dev);
+	struct eqos_pdata *eqos_plat = dev_get_plat(dev);
+	struct eth_pdata *plat = &eqos_plat->eth_pdata;
 	struct eqos_priv *eqos = dev_get_priv(dev);
 	uint32_t val;
 
@@ -1114,7 +1124,8 @@ static int eqos_write_hwaddr(struct udevice *dev)
 
 static int eqos_read_rom_hwaddr(struct udevice *dev)
 {
-	struct eth_pdata *pdata = dev_get_plat(dev);
+	struct eqos_pdata *eqos_plat = dev_get_plat(dev);
+	struct eth_pdata *pdata = &eqos_plat->eth_pdata;
 
 #ifdef CONFIG_ARCH_IMX8M
 	imx_get_mac_from_fuse(dev_seq(dev), pdata->enetaddr);
@@ -1704,6 +1715,7 @@ static int eqos_remove_resources_core(struct udevice *dev)
 
 static int eqos_probe_resources_tegra186(struct udevice *dev)
 {
+	struct eqos_pdata *eqos_plat = dev_get_plat(dev);
 	struct eqos_priv *eqos = dev_get_priv(dev);
 	int ret;
 
@@ -1722,6 +1734,8 @@ static int eqos_probe_resources_tegra186(struct udevice *dev)
 		pr_err("gpio_request_by_name(phy reset) failed: %d", ret);
 		goto err_free_reset_eqos;
 	}
+	eqos_plat->reset_assert_us = 2;
+	eqos_plat->reset_deassert_us = 1;
 
 	ret = clk_get_by_name(dev, "slave_bus", &eqos->clk_slave_bus);
 	if (ret) {
@@ -1783,6 +1797,7 @@ __weak int board_interface_eth_init(struct udevice *dev,
 
 static int eqos_probe_resources_stm32(struct udevice *dev)
 {
+	struct eqos_pdata *eqos_plat = dev_get_plat(dev);
 	struct eqos_priv *eqos = dev_get_priv(dev);
 	int ret;
 	phy_interface_t interface;
@@ -1839,6 +1854,10 @@ static int eqos_probe_resources_stm32(struct udevice *dev)
 		if (ret)
 			pr_warn("gpio_request_by_name(phy reset) not provided %d",
 				ret);
+		eqos_plat->reset_assert_us = ofnode_read_u32_default(phandle_args.node,
+								     "reset-assert-us", 2);
+		eqos_plat->reset_deassert_us = ofnode_read_u32_default(phandle_args.node,
+								       "reset-deassert-us", 1);
 
 		eqos->phyaddr = ofnode_read_u32_default(phandle_args.node,
 							"reg", -1);
@@ -2167,5 +2186,5 @@ U_BOOT_DRIVER(eth_eqos) = {
 	.remove = eqos_remove,
 	.ops = &eqos_ops,
 	.priv_auto	= sizeof(struct eqos_priv),
-	.plat_auto	= sizeof(struct eth_pdata),
+	.plat_auto	= sizeof(struct eqos_pdata),
 };
-- 
2.17.1

[PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay

[Marek Vasut]

On 4/9/21 10:00 AM, Patrick Delaunay wrote:
> The gpio reset assert/deassert delay are today harcoded in U-Boot driver
> but the value should be read from DT.
> 
> STM32 use the generic binding defined in linux:
> Documentation/devicetree/bindings/net/ethernet-phy.yaml
> 
>    reset-gpios:
>      maxItems: 1
>      description:
>        The GPIO phandle and specifier for the PHY reset signal.
> 
>    reset-assert-us:
>      description:
>        Delay after the reset was asserted in microseconds. If this
>        property is missing the delay will be skipped.
> 
>    reset-deassert-us:
>      description:
>        Delay after the reset was deasserted in microseconds. If
>        this property is missing the delay will be skipped.
> 
> See also U-Boot: doc/device-tree-bindings/net/phy.txt

Since this is a PHY property, shouldn't that be handled in 
drivers/net/phy/ instead of MAC driver ?

[PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay

[Patrick DELAUNAY]

Hi,

On 4/9/21 2:22 PM, Marek Vasut wrote:
> On 4/9/21 10:00 AM, Patrick Delaunay wrote:
>> The gpio reset assert/deassert delay are today harcoded in U-Boot driver
>> but the value should be read from DT.
>>
>> STM32 use the generic binding defined in linux:
>> Documentation/devicetree/bindings/net/ethernet-phy.yaml
>>
>> ?? reset-gpios:
>> ???? maxItems: 1
>> ???? description:
>> ?????? The GPIO phandle and specifier for the PHY reset signal.
>>
>> ?? reset-assert-us:
>> ???? description:
>> ?????? Delay after the reset was asserted in microseconds. If this
>> ?????? property is missing the delay will be skipped.
>>
>> ?? reset-deassert-us:
>> ???? description:
>> ?????? Delay after the reset was deasserted in microseconds. If
>> ?????? this property is missing the delay will be skipped.
>>
>> See also U-Boot: doc/device-tree-bindings/net/phy.txt
>
> Since this is a PHY property, shouldn't that be handled in 
> drivers/net/phy/ instead of MAC driver ?


I was my first idea but I don't found found the correct location in phy 
(driver or uclass)

to manage these generic property and the generic property "reset-gpios" 
was already

managed in eth driver, so I continue to patch the driver.


But I come back to this idea after your remark....

=> in linux these property are managed in

 ??? drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register

 ??? ??? parse DT node and add info in mdio

 ??? drivers/net/phy/mdio_device.c::mdio_device_reset

 ??? ??? called in? mdio_probe / mdio_remove


In my first search, I don't found the same level in the U-Boot drivers 
in drivers/net/phy/

but I miss the uclass defined in drivers/net/eth-phy-uclass.c


Finally I think I need to manage the generic binding property

(reset-gpios, reset-assert-us, reset-deassert-us) directly? in

=> drivers/net/mdio-uclass


The GPIO RESET will be managed in mdio? ops: pre_probe/ post_remove

as it is done in linux

warning: today post_remove ops don't exit in u-class.


Do you think it is the correct location ?


Do you think it should be a new serie (migrate the eqos property in mdio)

after this eqos is accepted

or I shoudl sent a new serie to replace this serie.


Regards


Patrick

[PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay

[Marek Vasut]

On 4/14/21 4:07 PM, Patrick DELAUNAY wrote:
> Hi,

Hi,

> On 4/9/21 2:22 PM, Marek Vasut wrote:
>> On 4/9/21 10:00 AM, Patrick Delaunay wrote:
>>> The gpio reset assert/deassert delay are today harcoded in U-Boot driver
>>> but the value should be read from DT.
>>>
>>> STM32 use the generic binding defined in linux:
>>> Documentation/devicetree/bindings/net/ethernet-phy.yaml
>>>
>>> ?? reset-gpios:
>>> ???? maxItems: 1
>>> ???? description:
>>> ?????? The GPIO phandle and specifier for the PHY reset signal.
>>>
>>> ?? reset-assert-us:
>>> ???? description:
>>> ?????? Delay after the reset was asserted in microseconds. If this
>>> ?????? property is missing the delay will be skipped.
>>>
>>> ?? reset-deassert-us:
>>> ???? description:
>>> ?????? Delay after the reset was deasserted in microseconds. If
>>> ?????? this property is missing the delay will be skipped.
>>>
>>> See also U-Boot: doc/device-tree-bindings/net/phy.txt
>>
>> Since this is a PHY property, shouldn't that be handled in 
>> drivers/net/phy/ instead of MAC driver ?
> 
> 
> I was my first idea but I don't found found the correct location in phy 
> (driver or uclass)
> 
> to manage these generic property and the generic property "reset-gpios" 
> was already
> 
> managed in eth driver, so I continue to patch the driver.
> 
> 
> But I come back to this idea after your remark....
> 
> => in linux these property are managed in
> 
>  ??? drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register
> 
>  ??? ??? parse DT node and add info in mdio
> 
>  ??? drivers/net/phy/mdio_device.c::mdio_device_reset
> 
>  ??? ??? called in? mdio_probe / mdio_remove
> 
> 
> In my first search, I don't found the same level in the U-Boot drivers 
> in drivers/net/phy/

Note that this is MDIO-wide PHY reset (e.g. you can have single reset 
line connected to multiple PHYs on single MDIO bus), this is not 
PHY-specific reset.

> but I miss the uclass defined in drivers/net/eth-phy-uclass.c
> 
> 
> Finally I think I need to manage the generic binding property
> 
> (reset-gpios, reset-assert-us, reset-deassert-us) directly? in
> 
> => drivers/net/mdio-uclass
> 
> 
> The GPIO RESET will be managed in mdio? ops: pre_probe/ post_remove
> 
> as it is done in linux
> 
> warning: today post_remove ops don't exit in u-class.
> 
> 
> Do you think it is the correct location ?

For single-PHY reset, the correct location is in drivers/net/phy/ somewhere.

> Do you think it should be a new serie (migrate the eqos property in mdio)
> 
> after this eqos is accepted
> 
> or I shoudl sent a new serie to replace this serie.

I'll leave that decision to Ramon/Joe.

[PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay

[Ramon Fried]

On Wed, Apr 14, 2021 at 5:36 PM Marek Vasut <marex@denx.de> wrote:
>
> On 4/14/21 4:07 PM, Patrick DELAUNAY wrote:
> > Hi,
>
> Hi,
>
> > On 4/9/21 2:22 PM, Marek Vasut wrote:
> >> On 4/9/21 10:00 AM, Patrick Delaunay wrote:
> >>> The gpio reset assert/deassert delay are today harcoded in U-Boot driver
> >>> but the value should be read from DT.
> >>>
> >>> STM32 use the generic binding defined in linux:
> >>> Documentation/devicetree/bindings/net/ethernet-phy.yaml
> >>>
> >>>    reset-gpios:
> >>>      maxItems: 1
> >>>      description:
> >>>        The GPIO phandle and specifier for the PHY reset signal.
> >>>
> >>>    reset-assert-us:
> >>>      description:
> >>>        Delay after the reset was asserted in microseconds. If this
> >>>        property is missing the delay will be skipped.
> >>>
> >>>    reset-deassert-us:
> >>>      description:
> >>>        Delay after the reset was deasserted in microseconds. If
> >>>        this property is missing the delay will be skipped.
> >>>
> >>> See also U-Boot: doc/device-tree-bindings/net/phy.txt
> >>
> >> Since this is a PHY property, shouldn't that be handled in
> >> drivers/net/phy/ instead of MAC driver ?
> >
> >
> > I was my first idea but I don't found found the correct location in phy
> > (driver or uclass)
> >
> > to manage these generic property and the generic property "reset-gpios"
> > was already
> >
> > managed in eth driver, so I continue to patch the driver.
> >
> >
> > But I come back to this idea after your remark....
> >
> > => in linux these property are managed in
> >
> >      drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register
> >
> >          parse DT node and add info in mdio
> >
> >      drivers/net/phy/mdio_device.c::mdio_device_reset
> >
> >          called in  mdio_probe / mdio_remove
> >
> >
> > In my first search, I don't found the same level in the U-Boot drivers
> > in drivers/net/phy/
>
> Note that this is MDIO-wide PHY reset (e.g. you can have single reset
> line connected to multiple PHYs on single MDIO bus), this is not
> PHY-specific reset.
>
> > but I miss the uclass defined in drivers/net/eth-phy-uclass.c
> >
> >
> > Finally I think I need to manage the generic binding property
> >
> > (reset-gpios, reset-assert-us, reset-deassert-us) directly  in
> >
> > => drivers/net/mdio-uclass
> >
> >
> > The GPIO RESET will be managed in mdio  ops: pre_probe/ post_remove
> >
> > as it is done in linux
> >
> > warning: today post_remove ops don't exit in u-class.
> >
> >
> > Do you think it is the correct location ?
>
> For single-PHY reset, the correct location is in drivers/net/phy/ somewhere.
>
> > Do you think it should be a new serie (migrate the eqos property in mdio)
> >
> > after this eqos is accepted
> >
> > or I shoudl sent a new serie to replace this serie.
>
> I'll leave that decision to Ramon/Joe.
Joe, I'll leave this to you.

[PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay

[Ramon Fried]

On Thu, Apr 15, 2021 at 4:41 AM Ramon Fried <rfried.dev@gmail.com> wrote:
>
> On Wed, Apr 14, 2021 at 5:36 PM Marek Vasut <marex@denx.de> wrote:
> >
> > On 4/14/21 4:07 PM, Patrick DELAUNAY wrote:
> > > Hi,
> >
> > Hi,
> >
> > > On 4/9/21 2:22 PM, Marek Vasut wrote:
> > >> On 4/9/21 10:00 AM, Patrick Delaunay wrote:
> > >>> The gpio reset assert/deassert delay are today harcoded in U-Boot driver
> > >>> but the value should be read from DT.
> > >>>
> > >>> STM32 use the generic binding defined in linux:
> > >>> Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > >>>
> > >>>    reset-gpios:
> > >>>      maxItems: 1
> > >>>      description:
> > >>>        The GPIO phandle and specifier for the PHY reset signal.
> > >>>
> > >>>    reset-assert-us:
> > >>>      description:
> > >>>        Delay after the reset was asserted in microseconds. If this
> > >>>        property is missing the delay will be skipped.
> > >>>
> > >>>    reset-deassert-us:
> > >>>      description:
> > >>>        Delay after the reset was deasserted in microseconds. If
> > >>>        this property is missing the delay will be skipped.
> > >>>
> > >>> See also U-Boot: doc/device-tree-bindings/net/phy.txt
> > >>
> > >> Since this is a PHY property, shouldn't that be handled in
> > >> drivers/net/phy/ instead of MAC driver ?
> > >
> > >
> > > I was my first idea but I don't found found the correct location in phy
> > > (driver or uclass)
> > >
> > > to manage these generic property and the generic property "reset-gpios"
> > > was already
> > >
> > > managed in eth driver, so I continue to patch the driver.
> > >
> > >
> > > But I come back to this idea after your remark....
> > >
> > > => in linux these property are managed in
> > >
> > >      drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register
> > >
> > >          parse DT node and add info in mdio
> > >
> > >      drivers/net/phy/mdio_device.c::mdio_device_reset
> > >
> > >          called in  mdio_probe / mdio_remove
> > >
> > >
> > > In my first search, I don't found the same level in the U-Boot drivers
> > > in drivers/net/phy/
> >
> > Note that this is MDIO-wide PHY reset (e.g. you can have single reset
> > line connected to multiple PHYs on single MDIO bus), this is not
> > PHY-specific reset.
> >
> > > but I miss the uclass defined in drivers/net/eth-phy-uclass.c
> > >
> > >
> > > Finally I think I need to manage the generic binding property
> > >
> > > (reset-gpios, reset-assert-us, reset-deassert-us) directly  in
> > >
> > > => drivers/net/mdio-uclass
> > >
> > >
> > > The GPIO RESET will be managed in mdio  ops: pre_probe/ post_remove
> > >
> > > as it is done in linux
> > >
> > > warning: today post_remove ops don't exit in u-class.
> > >
> > >
> > > Do you think it is the correct location ?
> >
> > For single-PHY reset, the correct location is in drivers/net/phy/ somewhere.
> >
> > > Do you think it should be a new serie (migrate the eqos property in mdio)
> > >
> > > after this eqos is accepted
> > >
> > > or I shoudl sent a new serie to replace this serie.
> >
> > I'll leave that decision to Ramon/Joe.
> Joe, I'll leave this to you.

You know what, let's go with the new series, please migrate it to the net/phy.
Thanks,
Ramon

[PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay

[Patrick DELAUNAY]

Hi,

On 4/15/21 8:08 AM, Ramon Fried wrote:
> On Thu, Apr 15, 2021 at 4:41 AM Ramon Fried <rfried.dev@gmail.com> wrote:
>> On Wed, Apr 14, 2021 at 5:36 PM Marek Vasut <marex@denx.de> wrote:
>>> On 4/14/21 4:07 PM, Patrick DELAUNAY wrote:
>>>> Hi,
>>> Hi,
>>>
>>>> On 4/9/21 2:22 PM, Marek Vasut wrote:
>>>>> On 4/9/21 10:00 AM, Patrick Delaunay wrote:
>>>>>> The gpio reset assert/deassert delay are today harcoded in U-Boot driver
>>>>>> but the value should be read from DT.
>>>>>>
>>>>>> STM32 use the generic binding defined in linux:
>>>>>> Documentation/devicetree/bindings/net/ethernet-phy.yaml
>>>>>>
>>>>>>     reset-gpios:
>>>>>>       maxItems: 1
>>>>>>       description:
>>>>>>         The GPIO phandle and specifier for the PHY reset signal.
>>>>>>
>>>>>>     reset-assert-us:
>>>>>>       description:
>>>>>>         Delay after the reset was asserted in microseconds. If this
>>>>>>         property is missing the delay will be skipped.
>>>>>>
>>>>>>     reset-deassert-us:
>>>>>>       description:
>>>>>>         Delay after the reset was deasserted in microseconds. If
>>>>>>         this property is missing the delay will be skipped.
>>>>>>
>>>>>> See also U-Boot: doc/device-tree-bindings/net/phy.txt
>>>>> Since this is a PHY property, shouldn't that be handled in
>>>>> drivers/net/phy/ instead of MAC driver ?
>>>>
>>>> I was my first idea but I don't found found the correct location in phy
>>>> (driver or uclass)
>>>>
>>>> to manage these generic property and the generic property "reset-gpios"
>>>> was already
>>>>
>>>> managed in eth driver, so I continue to patch the driver.
>>>>
>>>>
>>>> But I come back to this idea after your remark....
>>>>
>>>> => in linux these property are managed in
>>>>
>>>>       drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register
>>>>
>>>>           parse DT node and add info in mdio
>>>>
>>>>       drivers/net/phy/mdio_device.c::mdio_device_reset
>>>>
>>>>           called in  mdio_probe / mdio_remove
>>>>
>>>>
>>>> In my first search, I don't found the same level in the U-Boot drivers
>>>> in drivers/net/phy/
>>> Note that this is MDIO-wide PHY reset (e.g. you can have single reset
>>> line connected to multiple PHYs on single MDIO bus), this is not
>>> PHY-specific reset.
>>>
>>>> but I miss the uclass defined in drivers/net/eth-phy-uclass.c
>>>>
>>>>
>>>> Finally I think I need to manage the generic binding property
>>>>
>>>> (reset-gpios, reset-assert-us, reset-deassert-us) directly  in
>>>>
>>>> => drivers/net/mdio-uclass
>>>>
>>>>
>>>> The GPIO RESET will be managed in mdio  ops: pre_probe/ post_remove
>>>>
>>>> as it is done in linux
>>>>
>>>> warning: today post_remove ops don't exit in u-class.
>>>>
>>>>
>>>> Do you think it is the correct location ?
>>> For single-PHY reset, the correct location is in drivers/net/phy/ somewhere.
>>>
>>>> Do you think it should be a new serie (migrate the eqos property in mdio)
>>>>
>>>> after this eqos is accepted
>>>>
>>>> or I shoudl sent a new serie to replace this serie.
>>> I'll leave that decision to Ramon/Joe.
>> Joe, I'll leave this to you.
> You know what, let's go with the new series, please migrate it to the net/phy.
> Thanks,
> Ramon


I am preparing V2 with DT parsing in drivers/net/eth-phy-uclass.c

with CONFIG_DM_ETH_PHY enabled for stm32 variant of dwc_eth_qos driver.

Regards

Patrick