Re: [PATCH v3] staging: rtl8712: Use completions for signaling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On mercoledì 23 marzo 2022 05:55:15 CET Sathish Kumar wrote:
> r8712_sitesurvey_cmd() uses a variable to notify r8712_SetFilter() that it
> has completed operation. There is no sort of assurance that the variable will
> actually change and it could cache the value the first time it is read and
> then never update it for the whole loop logic.
> 
> Use completion variables because they are better suited for the purpose.
> 
> 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
> 
> Changes in v3:
>   - Replace the variable used for signaling with completion
> ---
>  drivers/staging/rtl8712/drv_types.h   | 3 +--
>  drivers/staging/rtl8712/rtl871x_cmd.c | 2 +-
>  drivers/staging/rtl8712/usb_intf.c    | 2 +-
>  drivers/staging/rtl8712/xmit_linux.c  | 8 +-------
>  4 files changed, 4 insertions(+), 11 deletions(-)
> 
Hi Sathish,

First of all, I must admit that you have impressed me :)

Aside from the specific code of this driver that I don't know and aside
from the specific problem that you've been suggested to fix, I see that
you have been able to research and understand the subjects that Greg and 
I talked to you about.

By reading what Greg wrote soon after me, I believe that he was expecting
a slight different solution. I suggest you to read carefully what he writes
rather than what I write, just because he has at least 20 or more years 
of experience than me and because I'm just a spare time type of kernel 
developer. 

You chose to use wait_for_completion() / complete() (notice that these API 
use barriers indirectly). What I can say is that they look to be suited for 
solving the issues that you have here, even if there are other approaches.

Since, as said, it looks like you have understood how to select and use the 
one of the API that were suggested, you have my...

Reviewed-by: Fabio M. De Francesco <fmdefrancesco@xxxxxxxxx>

Thanks,

Fabio







[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux