RE: [PATCH v2] net: stmmac: Wait a bit for the reset to take effect

From: Jose Abreu
Date: Mon Jan 22 2024 - 12:15:02 EST


From: Bernd Edlinger <bernd.edlinger@xxxxxxxxxx>
Date: Mon, Jan 22, 2024 at 06:41:46

> On 1/19/24 11:38, Jose Abreu wrote:
> > I understand your point, but the delay should be on reset function itself, since it depends
> > on the SoC that stmmac is integrated.
> >
> > Please refer to reset_simple_reset(), where usleep_range() is used.
> >
>
> Okay, in my case the SOC is an Altera CycloneV and reset control seems to be an altr,rst-mgr
> which is indeed based on this reset_simple_reset.
>
> So it implements reset_control_assert, reset_control_deassert, and reset_control_reset.
> But the above mentioned delay affects only the width of the reset pulse that is generated
> by the reset_control_reset method.
>
> However if you look at the code in stmmac_dvr_proble where the reset pulse is generated,
> you will see that the reset pulse is only generated with reset_control_assert/deassert:
>
> if (priv->plat->stmmac_rst) {
> ret = reset_control_assert(priv->plat->stmmac_rst);
> reset_control_deassert(priv->plat->stmmac_rst);
> /* Some reset controllers have only reset callback instead of
> * assert + deassert callbacks pair.
> */
> if (ret == -ENOTSUPP)
> reset_control_reset(priv->plat->stmmac_rst);
> }
>
> I don't know which reset controller that would be, where only a reset_control_reset is
> available, but in my case ret == 0, and even if I could get the reset_control_reset
> to be used, the issue is not the duration how long the reset line is in active state,
> but the duration that is needed for the device to recover from the reset.

Sorry, I indeed missed the fact that on simple_reset the deassert is done
after the delay. But my point was that what you are facing is expected since
most of SoC chips need some time to recover from reset, and this time is
greatly dependent on integration' reference clock value (lower reference
values cause "recover" to take longer).

I have no objection to your patch since it's indeed a very small value, but
I believe reset framework and/or Altera' reset manager should take these delays
into account on deassert, since they are expected to happen.

Thanks,
Jose