Re: [RESEND PATCH v2 02/11] net: dwc_eth_qos: Add option "snps, reset-gpio" phy-rst gpio for stm32
Hi Tom, 在 2020/6/12 下午10:48, Tom Rini 写道: On Tue, May 12, 2020 at 05:56:01PM +0800, David Wu wrote: It can be seen that most of the Socs using STM mac, "snps,reset-gpio" gpio is used, adding this option makes reset function more general. Signed-off-by: David Wu Reviewed-by: Patrice Chotard --- Changes in v2: - Remove the code is not related (Patrice) Please note that based on Patrick's feedback I am expecting a v3 of this patch or further answers about the obsolete binding being used. Thanks! I think I will submit v3 after the following two patches, it looks like I need to add a rockchip config. [1] net: dwc_eth_qos: update the compatible supported for STM32 http://patchwork.ozlabs.org/project/uboot/patch/20200514130023.15030-1-patrick.delau...@st.com/ [2] net: dwc_eth_qos: add Kconfig option to select supported configuration http://patchwork.ozlabs.org/project/uboot/list/?series=181931
Re: [RESEND PATCH v2 02/11] net: dwc_eth_qos: Add option "snps,reset-gpio" phy-rst gpio for stm32
Hi Patrick, Yes, this is the case, it should be add at PHY node, and I also used the original writing "snps,reset*" at MAC node. Anyway, I will try to put the reset gpio in the PHY node. 在 2020/5/13 下午8:55, Patrick DELAUNAY 写道: Hi David From: David Wu Sent: mardi 12 mai 2020 11:56 It can be seen that most of the Socs using STM mac, "snps,reset-gpio" gpio is used, adding this option makes reset function more general. Signed-off-by: David Wu --- Changes in v2: - Remove the code is not related (Patrice) drivers/net/dwc_eth_qos.c | 32 +++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 66a02aa80b..92dab678c7 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -314,6 +314,7 @@ struct eqos_priv { struct eqos_tegra186_regs *tegra186_regs; struct reset_ctl reset_ctl; struct gpio_desc phy_reset_gpio; + u32 reset_delays[3]; struct clk clk_master_bus; struct clk clk_rx; struct clk clk_ptp_ref; @@ -739,6 +740,15 @@ static int eqos_start_resets_stm32(struct udevice *dev) debug("%s(dev=%p):\n", __func__, dev); if (dm_gpio_is_valid(>phy_reset_gpio)) { + ret = dm_gpio_set_value(>phy_reset_gpio, 0); + if (ret < 0) { + pr_err("dm_gpio_set_value(phy_reset, deassert) failed: %d", + ret); + return ret; + } + + udelay(eqos->reset_delays[0]); + ret = dm_gpio_set_value(>phy_reset_gpio, 1); if (ret < 0) { pr_err("dm_gpio_set_value(phy_reset, assert) failed: %d", @@ -746,7 +756,7 @@ static int eqos_start_resets_stm32(struct udevice *dev) return ret; } - udelay(2); + udelay(eqos->reset_delays[1]); ret = dm_gpio_set_value(>phy_reset_gpio, 0); if (ret < 0) { @@ -754,6 +764,8 @@ static int eqos_start_resets_stm32(struct udevice *dev) ret); return ret; } + + udelay(eqos->reset_delays[2]); } debug("%s: OK\n", __func__); @@ -1864,11 +1876,29 @@ static int eqos_probe_resources_stm32(struct udevice *dev) if (ret) pr_warn("gpio_request_by_name(phy reset) not provided %d", ret); + else + eqos->reset_delays[1] = 2; eqos->phyaddr = ofnode_read_u32_default(phandle_args.node, "reg", -1); } + if (!dm_gpio_is_valid(>phy_reset_gpio)) { + int reset_flags = GPIOD_IS_OUT; + + if (dev_read_bool(dev, "snps,reset-active-low")) + reset_flags |= GPIOD_ACTIVE_LOW; + + ret = gpio_request_by_name(dev, "snps,reset-gpio", 0, + >phy_reset_gpio, reset_flags); + if (ret == 0) + ret = dev_read_u32_array(dev, "snps,reset-delays-us", +eqos->reset_delays, 3); + else + pr_warn("gpio_request_by_name(snps,reset-gpio) failed: %d", + ret); + } + debug("%s: OK\n", __func__); return 0; -- 2.19.1 This obsolete binding isn't expected to be supported in stm32 glue for dwmac (and it tis the purpose of eqos_stm32_config) Reference in linux binding ./Documentation/devicetree/bindings/net/stm32-dwmac.txt (the glue) ./Documentation/devicetree/bindings/net/snps,dwmac.yaml snps,reset-gpio: deprecated: true snps,reset-active-low: deprecated: true snps,reset-delays-us: deprecated: true I expected that gpio reset in future device tree should be managed by only by PHY generic binding (upstream in progress on Linux side for STM32MP15x), as described in: 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 alsoU-Boot: doc/device-tree-bindings/net/phy.txt Something as { status = "okay"; pinctrl-0 = <_mii>; pinctrl-names = "default"; phy-mode= "mii"; phy-handle = <>; mdio0 { #address-cells = <1>; #size-cells = <0>; compatible =
Re: [RESEND PATCH v2 02/11] net: dwc_eth_qos: Add option "snps, reset-gpio" phy-rst gpio for stm32
On Tue, May 12, 2020 at 05:56:01PM +0800, David Wu wrote: > It can be seen that most of the Socs using STM mac, "snps,reset-gpio" > gpio is used, adding this option makes reset function more general. > > Signed-off-by: David Wu > Reviewed-by: Patrice Chotard > --- > > Changes in v2: > - Remove the code is not related (Patrice) Please note that based on Patrick's feedback I am expecting a v3 of this patch or further answers about the obsolete binding being used. Thanks! -- Tom signature.asc Description: PGP signature
RE: [RESEND PATCH v2 02/11] net: dwc_eth_qos: Add option "snps,reset-gpio" phy-rst gpio for stm32
Hi David > From: David Wu > Sent: mardi 12 mai 2020 11:56 > > It can be seen that most of the Socs using STM mac, "snps,reset-gpio" > gpio is used, adding this option makes reset function more general. > > Signed-off-by: David Wu > --- > > Changes in v2: > - Remove the code is not related (Patrice) > > drivers/net/dwc_eth_qos.c | 32 +++- > 1 file changed, 31 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index > 66a02aa80b..92dab678c7 100644 > --- a/drivers/net/dwc_eth_qos.c > +++ b/drivers/net/dwc_eth_qos.c > @@ -314,6 +314,7 @@ struct eqos_priv { > struct eqos_tegra186_regs *tegra186_regs; > struct reset_ctl reset_ctl; > struct gpio_desc phy_reset_gpio; > + u32 reset_delays[3]; > struct clk clk_master_bus; > struct clk clk_rx; > struct clk clk_ptp_ref; > @@ -739,6 +740,15 @@ static int eqos_start_resets_stm32(struct udevice *dev) > > debug("%s(dev=%p):\n", __func__, dev); > if (dm_gpio_is_valid(>phy_reset_gpio)) { > + ret = dm_gpio_set_value(>phy_reset_gpio, 0); > + if (ret < 0) { > + pr_err("dm_gpio_set_value(phy_reset, deassert) failed: > %d", > +ret); > + return ret; > + } > + > + udelay(eqos->reset_delays[0]); > + > ret = dm_gpio_set_value(>phy_reset_gpio, 1); > if (ret < 0) { > pr_err("dm_gpio_set_value(phy_reset, assert) failed: > %d", > @@ -746,7 +756,7 @@ static int eqos_start_resets_stm32(struct udevice *dev) > return ret; > } > > - udelay(2); > + udelay(eqos->reset_delays[1]); > > ret = dm_gpio_set_value(>phy_reset_gpio, 0); > if (ret < 0) { > @@ -754,6 +764,8 @@ static int eqos_start_resets_stm32(struct udevice *dev) > ret); > return ret; > } > + > + udelay(eqos->reset_delays[2]); > } > debug("%s: OK\n", __func__); > > @@ -1864,11 +1876,29 @@ static int eqos_probe_resources_stm32(struct > udevice *dev) > if (ret) > pr_warn("gpio_request_by_name(phy reset) not provided > %d", > ret); > + else > + eqos->reset_delays[1] = 2; > > eqos->phyaddr = ofnode_read_u32_default(phandle_args.node, > "reg", -1); > } > > + if (!dm_gpio_is_valid(>phy_reset_gpio)) { > + int reset_flags = GPIOD_IS_OUT; > + > + if (dev_read_bool(dev, "snps,reset-active-low")) > + reset_flags |= GPIOD_ACTIVE_LOW; > + > + ret = gpio_request_by_name(dev, "snps,reset-gpio", 0, > +>phy_reset_gpio, reset_flags); > + if (ret == 0) > + ret = dev_read_u32_array(dev, "snps,reset-delays-us", > + eqos->reset_delays, 3); > + else > + pr_warn("gpio_request_by_name(snps,reset-gpio) failed: > %d", > + ret); > + } > + > debug("%s: OK\n", __func__); > return 0; > > -- > 2.19.1 > > This obsolete binding isn't expected to be supported in stm32 glue for dwmac (and it tis the purpose of eqos_stm32_config) Reference in linux binding ./Documentation/devicetree/bindings/net/stm32-dwmac.txt (the glue) ./Documentation/devicetree/bindings/net/snps,dwmac.yaml snps,reset-gpio: deprecated: true snps,reset-active-low: deprecated: true snps,reset-delays-us: deprecated: true I expected that gpio reset in future device tree should be managed by only by PHY generic binding (upstream in progress on Linux side for STM32MP15x), as described in: 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 alsoU-Boot: doc/device-tree-bindings/net/phy.txt Something as { status = "okay"; pinctrl-0 = <_mii>; pinctrl-names = "default"; phy-mode= "mii"; phy-handle = <>; mdio0 { #address-cells = <1>; #size-cells = <0>; compatible = "snps,dwmac-mdio"; phy1: ethernet-phy@1 { reg = <1>; + reset-assert-us = <1>; +
Re: [RESEND PATCH v2 02/11] net: dwc_eth_qos: Add option "snps,reset-gpio" phy-rst gpio for stm32
Hi David On 5/12/20 11:56 AM, David Wu wrote: > It can be seen that most of the Socs using STM mac, "snps,reset-gpio" > gpio is used, adding this option makes reset function more general. > > Signed-off-by: David Wu > --- > > Changes in v2: > - Remove the code is not related (Patrice) > > drivers/net/dwc_eth_qos.c | 32 +++- > 1 file changed, 31 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c > index 66a02aa80b..92dab678c7 100644 > --- a/drivers/net/dwc_eth_qos.c > +++ b/drivers/net/dwc_eth_qos.c > @@ -314,6 +314,7 @@ struct eqos_priv { > struct eqos_tegra186_regs *tegra186_regs; > struct reset_ctl reset_ctl; > struct gpio_desc phy_reset_gpio; > + u32 reset_delays[3]; > struct clk clk_master_bus; > struct clk clk_rx; > struct clk clk_ptp_ref; > @@ -739,6 +740,15 @@ static int eqos_start_resets_stm32(struct udevice *dev) > > debug("%s(dev=%p):\n", __func__, dev); > if (dm_gpio_is_valid(>phy_reset_gpio)) { > + ret = dm_gpio_set_value(>phy_reset_gpio, 0); > + if (ret < 0) { > + pr_err("dm_gpio_set_value(phy_reset, deassert) failed: > %d", > +ret); > + return ret; > + } > + > + udelay(eqos->reset_delays[0]); > + > ret = dm_gpio_set_value(>phy_reset_gpio, 1); > if (ret < 0) { > pr_err("dm_gpio_set_value(phy_reset, assert) failed: > %d", > @@ -746,7 +756,7 @@ static int eqos_start_resets_stm32(struct udevice *dev) > return ret; > } > > - udelay(2); > + udelay(eqos->reset_delays[1]); > > ret = dm_gpio_set_value(>phy_reset_gpio, 0); > if (ret < 0) { > @@ -754,6 +764,8 @@ static int eqos_start_resets_stm32(struct udevice *dev) > ret); > return ret; > } > + > + udelay(eqos->reset_delays[2]); > } > debug("%s: OK\n", __func__); > > @@ -1864,11 +1876,29 @@ static int eqos_probe_resources_stm32(struct udevice > *dev) > if (ret) > pr_warn("gpio_request_by_name(phy reset) not provided > %d", > ret); > + else > + eqos->reset_delays[1] = 2; > > eqos->phyaddr = ofnode_read_u32_default(phandle_args.node, > "reg", -1); > } > > + if (!dm_gpio_is_valid(>phy_reset_gpio)) { > + int reset_flags = GPIOD_IS_OUT; > + > + if (dev_read_bool(dev, "snps,reset-active-low")) > + reset_flags |= GPIOD_ACTIVE_LOW; > + > + ret = gpio_request_by_name(dev, "snps,reset-gpio", 0, > +>phy_reset_gpio, reset_flags); > + if (ret == 0) > + ret = dev_read_u32_array(dev, "snps,reset-delays-us", > + eqos->reset_delays, 3); > + else > + pr_warn("gpio_request_by_name(snps,reset-gpio) failed: > %d", > + ret); > + } > + > debug("%s: OK\n", __func__); > return 0; > Reviewed-by: Patrice Chotard Thanks
[RESEND PATCH v2 02/11] net: dwc_eth_qos: Add option "snps, reset-gpio" phy-rst gpio for stm32
It can be seen that most of the Socs using STM mac, "snps,reset-gpio" gpio is used, adding this option makes reset function more general. Signed-off-by: David Wu --- Changes in v2: - Remove the code is not related (Patrice) drivers/net/dwc_eth_qos.c | 32 +++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 66a02aa80b..92dab678c7 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -314,6 +314,7 @@ struct eqos_priv { struct eqos_tegra186_regs *tegra186_regs; struct reset_ctl reset_ctl; struct gpio_desc phy_reset_gpio; + u32 reset_delays[3]; struct clk clk_master_bus; struct clk clk_rx; struct clk clk_ptp_ref; @@ -739,6 +740,15 @@ static int eqos_start_resets_stm32(struct udevice *dev) debug("%s(dev=%p):\n", __func__, dev); if (dm_gpio_is_valid(>phy_reset_gpio)) { + ret = dm_gpio_set_value(>phy_reset_gpio, 0); + if (ret < 0) { + pr_err("dm_gpio_set_value(phy_reset, deassert) failed: %d", + ret); + return ret; + } + + udelay(eqos->reset_delays[0]); + ret = dm_gpio_set_value(>phy_reset_gpio, 1); if (ret < 0) { pr_err("dm_gpio_set_value(phy_reset, assert) failed: %d", @@ -746,7 +756,7 @@ static int eqos_start_resets_stm32(struct udevice *dev) return ret; } - udelay(2); + udelay(eqos->reset_delays[1]); ret = dm_gpio_set_value(>phy_reset_gpio, 0); if (ret < 0) { @@ -754,6 +764,8 @@ static int eqos_start_resets_stm32(struct udevice *dev) ret); return ret; } + + udelay(eqos->reset_delays[2]); } debug("%s: OK\n", __func__); @@ -1864,11 +1876,29 @@ static int eqos_probe_resources_stm32(struct udevice *dev) if (ret) pr_warn("gpio_request_by_name(phy reset) not provided %d", ret); + else + eqos->reset_delays[1] = 2; eqos->phyaddr = ofnode_read_u32_default(phandle_args.node, "reg", -1); } + if (!dm_gpio_is_valid(>phy_reset_gpio)) { + int reset_flags = GPIOD_IS_OUT; + + if (dev_read_bool(dev, "snps,reset-active-low")) + reset_flags |= GPIOD_ACTIVE_LOW; + + ret = gpio_request_by_name(dev, "snps,reset-gpio", 0, + >phy_reset_gpio, reset_flags); + if (ret == 0) + ret = dev_read_u32_array(dev, "snps,reset-delays-us", +eqos->reset_delays, 3); + else + pr_warn("gpio_request_by_name(snps,reset-gpio) failed: %d", + ret); + } + debug("%s: OK\n", __func__); return 0; -- 2.19.1