Re: [PATCH v2] staging: rtl8712: Fix CamelCase warnings

[Sathish Kumar]

On 22/03/22 5:01 pm, Greg KH wrote:
> On Tue, Mar 22, 2022 at 11:42:21AM +0100, Fabio M. De Francesco wrote:
> > On martedì 22 marzo 2022 05:30:29 CET Sathish Kumar wrote:
> > > On 18/03/22 4:58 pm, Greg KH wrote:
> > > > On Fri, Mar 18, 2022 at 03:44:40PM +0530, Sathish Kumar wrote:
> > > > > This patch fixes the checkpatch.pl warnings like:
> > > > > CHECK: Avoid CamelCase: <blnEnableRxFF0Filter>
> > > > > +   u8 blnEnableRxFF0Filter;
> > > > >
> > > > > Signed-off-by: Sathish Kumar <skumark1902@xxxxxxxxx>
> > > > > ---
> > > > > Changes in v2:
> > > > >       - Remove the "bln" prefix
> > > > > ---
> > > > >    drivers/staging/rtl8712/drv_types.h   | 2 +-
> > > > >    drivers/staging/rtl8712/rtl871x_cmd.c | 2 +-
> > > > >    drivers/staging/rtl8712/xmit_linux.c  | 4 ++--
> > > > >    3 files changed, 4 insertions(+), 4 deletions(-)
> > > > >
> > > > > [...]
> > > > >
> > > > >    	do {
> > > > >    		msleep(100);
> > > > > -	} while (adapter->blnEnableRxFF0Filter == 1);
> > > > > +	} while (adapter->enable_rx_ff0_filter == 1);
> > > > Ah, that's funny.  It's amazing it works at all and that the compiler
> > > > doesn't optimize this away.  This isn't a good pattern to use in kernel
> > > Do you mean the following code is not a good pattern in kernel?
> > > do {
> > > msleep();
> > > } while(condition);
> > Exactly, this is not a pattern that works as you expect :)
> >
> > I was waiting for Greg to detail something more about this subject but,
> > since it looks like he has no time yet to respond, I'll try to interpret
> > his words.
> >
> > (@Greg, please forgive me if I saying something different from what you
> > intended to convey :)).
> >
> > The reason why this pattern does not work as expected is too long to be
> > explained here. However, I think that Greg is suggesting to you to research
> > and use what are called "Condition variables".
> Kind of.  The problem is that "condition" here is just looking at a
> random variable.  There is no sort of assurance that the variable will
> actually change or that that compiler even has to read from memory for
> it.  It could cache the value the first time it is read and then never
> update it for the whole loop logic.
>
> Please read Documentation/memory-barriers.txt for how to fix this all up
> and do it properly.
>
> Again, it's amazing that the current code even works at all.  So maybe
> it doesn't!  :)
>
> thansks,
>
> greg k-h

Thank you Greg and Fabio for your inputs. Will check and fix this up.

Regards,

Sathish