Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To:     Larry.Finger@lwfinger.net, florian.c.schilhabel@googlemail.com,
        gregkh@linuxfoundation.org, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org, Sathish Kumar <skumark1902@gmail.com>
Cc:     Sathish Kumar <skumark1902@gmail.com>
Subject: Re: [PATCH v3] staging: rtl8712: Use completions for signaling
Date:   Thu, 24 Mar 2022 10:02:17 +0100
Message-ID: <3422492.iIbC2pHGDl@leap>
In-Reply-To: <20220323045515.2513-1-skumark1902@gmail.com>
References: <20220323045515.2513-1-skumark1902@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="ISO-8859-1"
Precedence: bulk

On mercoled=EC 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.
>=20
> Use completion variables because they are better suited for the purpose.
>=20
> This patch fixes the checkpatch.pl warnings like:
> CHECK: Avoid CamelCase: <blnEnableRxFF0Filter>
> +   u8 blnEnableRxFF0Filter;
>=20
> Signed-off-by: Sathish Kumar <skumark1902@gmail.com>
> ---
> Changes in v2:
>   - Remove the "bln" prefix
>=20
> 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(-)
>=20
Hi Sathish,

=46irst 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=20
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=20
of experience than me and because I'm just a spare time type of kernel=20
developer.=20

You chose to use wait_for_completion() / complete() (notice that these API=
=20
use barriers indirectly). What I can say is that they look to be suited for=
=20
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=
=20
one of the API that were suggested, you have my...

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

Thanks,

=46abio