On Wed, Jan 26, 2022 at 01:26:08PM +0300, Pavel Skripkin wrote:
> Hi Phillip,
>
> On 1/26/22 04:13, Phillip Potter wrote:
>
> [snip]
>
> }
> > >
> > > And here you also removes the reads. I guess, some kind of magic pattern is
> > > used
> > >
> >
> > So these calls are macro arguments, they would never be executed under
> > normal circumstances anyway, unless the rtw_debug kernel module was
> > passed in as 5 or more - it is 1 by default. The DBG_88E macro would
> > expand during preprocessing phase to (for example):
> >
> > do {
> > if (5 <= GlobalDebugLevel)
> > pr_info("R8188EU: " "dbg(0x450) = 0x%x\n", rtw_read32(padapter, 0x450));
> > } while (0)
> >
> > As this is never executed under normal circumstances anyway, I would say
> > calls like these are therefore safe to remove. Happy to be convinced
> > though :-) Many thanks.
> >
>
> I see your point, thanks for explanation.
>
> Well, in this case, you may left all reads, that are executed during normal
> lifetime of a driver. We know, that there is at least 1 place, where read()
> call removal can break things. Might be there are couple of other places we
> don't know about.
>
> IMHO the best thing you can do is to leave these reads and leave a comment
> like "hey, please remove me and test". One day useless reads should be
> anyway removed, since ideally rtw_read family must get __must_check
> annotation + normal error handling.
>
Yeah, not a bad idea - I've tried to remove only those which don't look
like they'd have side effects (such as fwstate checks etc.), but by all
means I can put them back in with the next revision. Thanks.
Regards,
Phil
