Messages in this thread | | | From | Ryosuke Saito <> | Subject | Re: [PATCH] net: netsec: replace cpu_relax() with timeout handling for register checks | Date | Mon, 20 Nov 2023 11:19:48 +0900 |
| |
[Resend again after removing an HTML format. Sorry for that.]
Hi Simon-san,
On Mon, Nov 20, 2023 at 3:53 AM Simon Horman <horms@kernel.org> wrote: > > On Fri, Nov 17, 2023 at 05:10:02PM +0900, Ryosuke Saito wrote: > > The cpu_relax() loops have the potential to hang if the specified > > register bits are not met on condition. The patch replaces it with > > usleep_range() and netsec_wait_while_busy() which includes timeout > > logic. > > > > Additionally, if the error condition is met during interrupting DMA > > transfer, there's no recovery mechanism available. In that case, any > > frames being sent or received will be discarded, which leads to > > potential frame loss as indicated in the comments. > > > > Signed-off-by: Ryosuke Saito <ryosuke.saito@linaro.org> > > --- > > drivers/net/ethernet/socionext/netsec.c | 35 ++++++++++++++++--------- > > 1 file changed, 23 insertions(+), 12 deletions(-) > > ... > > > @@ -1476,9 +1483,13 @@ static int netsec_reset_hardware(struct netsec_priv *priv, > > netsec_write(priv, NETSEC_REG_DMA_MH_CTRL, MH_CTRL__MODE_TRANS); > > netsec_write(priv, NETSEC_REG_PKT_CTRL, value); > > > > - while ((netsec_read(priv, NETSEC_REG_MODE_TRANS_COMP_STATUS) & > > - NETSEC_MODE_TRANS_COMP_IRQ_T2N) == 0) > > - cpu_relax(); > > + usleep_range(100000, 120000); > > + > > + if ((netsec_read(priv, NETSEC_REG_MODE_TRANS_COMP_STATUS) & > > + NETSEC_MODE_TRANS_COMP_IRQ_T2N) == 0) { > > + dev_warn(priv->dev, > > + "%s: trans comp timeout.\n", __func__); > > + } > > Hi Saito-san, > > could you add some colour to how the new code satisfies the > requirements of the hardware? In particular, the use of > usleep_range(), and the values passed to it.
For the h/w requirements, I followed U-Boot upstream: https://elixir.bootlin.com/u-boot/latest/source/drivers/net/sni_netsec.c
It has the same function as well, netsec_reset_hardware(), and the corresponding potion is the following read-check loop:
1012 value = 100; 1013 while ((netsec_read_reg(priv, NETSEC_REG_MODE_TRANS_COMP_STATUS) & 1014 NETSEC_MODE_TRANS_COMP_IRQ_T2N) == 0) { 1015 udelay(1000); 1016 if (--value == 0) { 1017 value = netsec_read_reg(priv, NETSEC_REG_MODE_TRANS_COMP_STATUS); 1018 pr_err("%s:%d timeout! val=%x\n", __func__, __LINE__, value); 1019 break; 1020 } 1021 }
The maximum t/o = 1000us * 100 + read time
Regards, Ryo
| |