lkml.org 
[lkml]   [2022]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8
On Thu, May 19, 2022 at 08:43:23AM +0300, Pavel Skripkin wrote:
> > > +
> > > + if (reg & RAM_DL_SEL) { /* 8051 RAM code */
> > > rtw_write8(padapter, REG_MCUFWDL, 0x00);
> > > rtw_reset_8051(padapter);
> > > }
> > > @@ -278,7 +303,14 @@ int rtl8188e_firmware_download(struct adapter *padapter)
> > > fwdl_timeout = jiffies + msecs_to_jiffies(500);
> > > while (1) {
> > > /* reset the FWDL chksum */
> > > - rtw_write8(padapter, REG_MCUFWDL, rtw_read8(padapter, REG_MCUFWDL) | FWDL_CHKSUM_RPT);
> > > + res = rtw_read8(padapter, REG_MCUFWDL, &reg);
> > > + if (res == -ENODEV)
> > > + break;
> > > +
> > > + if (res)
> > > + continue;
> >
> > This continue is wrong. If res = -EPERM then it's a forever loop.
> > Let's just break for every error.
> >
>
> I was trying to avoid strict breaking the loop on any error, since I am
> afraid this might break the driver.
>
> What about:
>
> do {
> /* reset the FWDL chksum */
> ret = rtw_read8(padapter, REG_MCUFWDL, &reg);
> if (ret == -ENODEV || ret == -EPERM)
> break;
>
> if (ret) {
> ret == _FAIL;
> continue;
> }
>
> rtw_write8(padapter, REG_MCUFWDL, reg | FWDL_CHKSUM_RPT);
>
> ret = write_fw(padapter, fw_data, fw_size);
> } while (!(ret == _SUCCESS ||
> (time_after(jiffies, fwdl_timeout) && write_fw_retry++ >= 3)))
>
> The idea is to break only on fatal errors to make things less strict
>

This is too complicated.

Treat all the errors the same, and use one time out condition. Either
based on the jiffies or the retry count.

regards,
dan carpenter

\
 
 \ /
  Last update: 2022-05-19 07:52    [W:0.078 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site