[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ddwtgxbtzy6vtc2bn5gvuskpco7mhmsmakt4gqk6ksppkmmtp2@w7eti3prlpke>
Date: Wed, 24 Jan 2024 16:31:31 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Bernd Edlinger <bernd.edlinger@...mail.de>
Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>,
Jose Abreu <joabreu@...opsys.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, linux-stm32@...md-mailman.stormreply.com,
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>
Subject: Re: [PATCH net v3] net: stmmac: Wait a bit for the reset to take
effect
On Mon, Jan 22, 2024 at 07:19:09PM +0100, Bernd Edlinger wrote:
> otherwise the synopsys_id value may be read out wrong,
> because the GMAC_VERSION register might still be in reset
> state, for at least 1 us after the reset is de-asserted.
>
> Add a wait for 10 us before continuing to be on the safe side.
>
> > From what have you got that delay value?
>
> Just try and error, with very old linux versions and old gcc versions
> the synopsys_id was read out correctly most of the time (but not always),
> with recent linux versions and recnet gcc versions it was read out
> wrongly most of the time, but again not always.
> I don't have access to the VHDL code in question, so I cannot
> tell why it takes so long to get the correct values, I also do not
> have more than a few hardware samples, so I cannot tell how long
> this timeout must be in worst case.
> Experimentally I can tell that the register is read several times
> as zero immediately after the reset is de-asserted, also adding several
> no-ops is not enough, adding a printk is enough, also udelay(1) seems to
> be enough but I tried that not very often, and I have not access to many
> hardware samples to be 100% sure about the necessary delay.
> And since the udelay here is only executed once per device instance,
> it seems acceptable to delay the boot for 10 us.
>
> BTW: my hardware's synopsys id is 0x37.
>
> Fixes: c5e4ddbdfa11 ("net: stmmac: Add support for optional reset control")
> Signed-off-by: Bernd Edlinger <bernd.edlinger@...mail.de>
> Reviewed-by: Jiri Pirko <jiri@...dia.com>
Thanks for taking all the notes into account. No objections from my
side:
Reviewed-by: Serge Semin <fancer.lancer@...il.com>
-Serge(y)
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> v2: rebased to v6.7, retested and updated the commit message
> as suggested Serge Semins review comment:
> https://lore.kernel.org/lkml/b4mpa62b2juln47374x6xxnbozb7fcfgztrc5ounk4tvscs3wg@mixnvsoqno7j/
> and retained Jiri Pirkos Reviwed-by from:
> https://lore.kernel.org/lkml/ZT+Zq4j9iQj1+Xai@nanopsycho/
>
> v3: addressed review comments.
>
>
> Thanks
> Bernd.
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index a0e46369ae15..b334eb16da23 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7542,6 +7542,9 @@ int stmmac_dvr_probe(struct device *device,
> dev_err(priv->device, "unable to bring out of ahb reset: %pe\n",
> ERR_PTR(ret));
>
> + /* Wait a bit for the reset to take effect */
> + udelay(10);
> +
> /* Init MAC and get the capabilities */
> ret = stmmac_hw_init(priv);
> if (ret)
> --
> 2.39.2
Powered by blists - more mailing lists