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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: 
 <DM4PR12MB50880D31E415D0DD95D991F9D3752@DM4PR12MB5088.namprd12.prod.outlook.com>
Date: Mon, 22 Jan 2024 15:44:19 +0000
From: Jose Abreu <Jose.Abreu@...opsys.com>
To: Bernd Edlinger <bernd.edlinger@...mail.de>, Andrew Lunn <andrew@...n.ch>
CC: Alexandre Torgue <alexandre.torgue@...s.st.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-stm32@...md-mailman.stormreply.com" <linux-stm32@...md-mailman.stormreply.com>,
        "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jiri Pirko <jiri@...dia.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jose Abreu <Jose.Abreu@...opsys.com>
Subject: RE: [PATCH v2] net: stmmac: Wait a bit for the reset to take effect

From: Bernd Edlinger <bernd.edlinger@...mail.de>
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