linux-kernel - Re: [PATCH v2] staging: rtl8712: Fix CamelCase warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1786742.atdPhlSkOF@leap>
Date:   Tue, 22 Mar 2022 11:42:21 +0100
From:   "Fabio M. De Francesco" <fmdefrancesco@...il.com>
To:     Greg KH <gregkh@...uxfoundation.org>,
        Sathish Kumar <skumark1902@...il.com>
Cc:     Larry.Finger@...inger.net, florian.c.schilhabel@...glemail.com,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] staging: rtl8712: Fix CamelCase warnings

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@...il.com>
> >> ---
> >> 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".

Take some time to research and understand what the Linuc kernel uses for
waiting for completion or state changes: struct completion, 
wait_for_completion(), complete(), and others.

Another related mechanism are the Linux kernel "Wait Queues". Do some 
research and understand how to use wait_event{,_interruptible,timeout} 
and wake_up{,all,interruptible,interruptible_all}.

If I recall correctly you may find one or more of my own patches to
r8188eu where I use those API (git-log is your friend).

I hope that all the above will be sufficient to start with.

Again, Greg please correct my words if I'm misinterpreting what you
requested to Sathish.

Thanks,

Fabio M. De Francesco

> > I know it's not caused by your change here, but perhaps you might
> > want to fix this up to work properly?
> >
> > thanks,
> >
> > greg k-h
> 
> Do i need to replace the above code with some other mechanism?
> 
> If yes, please let me know which mechanism i should use? Or what should 
> I do here?
> 
> Note : I am new to Linux kernel development and looking forward to learn 
> and contribute.
> 
> Thanks,
> Sathish
> 
>