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...@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;
>
