Re: [PATCH 3/8] net: dwc_eth_qos: Add option "snps,reset-gpio" phy-rst gpio for stm32

Hi David

On 4/30/20 12:36 PM, 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 david.wu@rock-chips.com

drivers/net/dwc_eth_qos.c | 40 ++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 16988f6bdc..06a8d924a7 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(&eqos->phy_reset_gpio)) {

• 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->reset_delays[0]);
  

not related to this patch subject

ret = dm_gpio_set_value(&eqos->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(&eqos->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__);

@@ -1065,16 +1077,16 @@ static int eqos_start(struct udevice *dev) val |= EQOS_DMA_MODE_SWR; writel(val, &eqos->dma_regs->mode); limit = eqos->config->swr_wait / 10;

• while (limit--) {

• do { if (!(readl(&eqos->dma_regs->mode) & EQOS_DMA_MODE_SWR)) break; mdelay(10000);

• }

• } while (limit--);

why are you updating this ? it's not related to the purpose of this patch

if (limit < 0) { pr_err("EQOS_DMA_MODE_SWR stuck");

• goto err_stop_clks;
  

• return -ETIMEDOUT;
  

• ret = -ETIMEDOUT;
  

• goto err_stop_resets;
  

ditto

}

ret = eqos->config->ops->eqos_calibrate_pads(dev); @@ -1712,11 +1724,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;
  

this is not the correct place to set default value. It must be set in case we can't get value from DT below

eqos->phyaddr = ofnode_read_u32_default(phandle_args.node,
					"reg", -1);

}

• if (!dm_gpio_is_valid(&eqos->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,
  

• 			   &eqos->phy_reset_gpio, reset_flags);
  

• if (ret == 0)
  

• 	ret = dev_read_u32_array(dev, "snps,reset-delays-us",
  

• 				 eqos->reset_delays, 3);
  

in case "snps,reset-delays-us" is not in present DT, all resets-delays are set to 0, see my remark above

• else
  

• 	pr_warn("gpio_request_by_name(snps,reset-gpio) failed: %d",
  

• 		ret);
  

• }
• debug("%s: OK\n", __func__); return 0;