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
[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 a72132cacf..54866aff6b 100644
--- a/drivers/net/dwc_eth_qos.c
+++ b/drivers/net/dwc_eth_qos.c
@@ -298,6 +298,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;
@@ -701,6 +702,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",
@@ -708,7 +718,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) {
@@ -716,6 +726,8 @@ static int eqos_start_resets_stm32(struct udevice *dev)
ret);
return ret;
}
+
+ udelay(eqos->reset_delays[2]);
}
debug("%s: OK\n", __func__);
@@ -1703,11 +1715,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
