[PATCH] staging: r8188eu: core: remove condition with no effect

[PATCH] staging: r8188eu: core: remove condition with no effect

[Saurav Girepunje]

Remove the condition with no effect (if == else) in rtw_led.c
file.

Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_led.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
index 22d4df9c92a5..76cbd5f19f90 100644
--- a/drivers/staging/r8188eu/core/rtw_led.c
+++ b/drivers/staging/r8188eu/core/rtw_led.c
@@ -148,10 +148,7 @@ static void SwLedBlink(struct LED_871x *pLed)
 			_set_timer(&(pLed->BlinkTimer), LED_BLINK_SLOWLY_INTERVAL);
 			break;
 		case LED_BLINK_WPS:
-			if (pLed->BlinkingLedState == RTW_LED_ON)
-				_set_timer(&(pLed->BlinkTimer), LED_BLINK_LONG_INTERVAL);
-			else
-				_set_timer(&(pLed->BlinkTimer), LED_BLINK_LONG_INTERVAL);
+			_set_timer(&(pLed->BlinkTimer), LED_BLINK_LONG_INTERVAL);
 			break;
 		default:
 			_set_timer(&(pLed->BlinkTimer), LED_BLINK_SLOWLY_INTERVAL);
--
2.30.2

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[Phillip Potter]

On Sat, 21 Aug 2021 at 21:23, Saurav Girepunje
<saurav.girepunje@gmail.com> wrote:
>
> Remove the condition with no effect (if == else) in rtw_led.c
> file.
>
> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> ---
>  drivers/staging/r8188eu/core/rtw_led.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
> index 22d4df9c92a5..76cbd5f19f90 100644
> --- a/drivers/staging/r8188eu/core/rtw_led.c
> +++ b/drivers/staging/r8188eu/core/rtw_led.c
> @@ -148,10 +148,7 @@ static void SwLedBlink(struct LED_871x *pLed)
>                         _set_timer(&(pLed->BlinkTimer), LED_BLINK_SLOWLY_INTERVAL);
>                         break;
>                 case LED_BLINK_WPS:
> -                       if (pLed->BlinkingLedState == RTW_LED_ON)
> -                               _set_timer(&(pLed->BlinkTimer), LED_BLINK_LONG_INTERVAL);
> -                       else
> -                               _set_timer(&(pLed->BlinkTimer), LED_BLINK_LONG_INTERVAL);
> +                       _set_timer(&(pLed->BlinkTimer), LED_BLINK_LONG_INTERVAL);
>                         break;
>                 default:
>                         _set_timer(&(pLed->BlinkTimer), LED_BLINK_SLOWLY_INTERVAL);
> --
> 2.30.2
>

Thanks for this, looks good.

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[Michael Straube]

On 8/21/21 10:23 PM, Saurav Girepunje wrote:
> Remove the condition with no effect (if == else) in rtw_led.c
> file.
> 
> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> ---
>   drivers/staging/r8188eu/core/rtw_led.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
> index 22d4df9c92a5..76cbd5f19f90 100644
> --- a/drivers/staging/r8188eu/core/rtw_led.c
> +++ b/drivers/staging/r8188eu/core/rtw_led.c
> @@ -148,10 +148,7 @@ static void SwLedBlink(struct LED_871x *pLed)
>   			_set_timer(&(pLed->BlinkTimer), LED_BLINK_SLOWLY_INTERVAL);
>   			break;
>   		case LED_BLINK_WPS:
> -			if (pLed->BlinkingLedState == RTW_LED_ON)
> -				_set_timer(&(pLed->BlinkTimer), LED_BLINK_LONG_INTERVAL);
> -			else
> -				_set_timer(&(pLed->BlinkTimer), LED_BLINK_LONG_INTERVAL);
> +			_set_timer(&(pLed->BlinkTimer), LED_BLINK_LONG_INTERVAL);
>   			break;
>   		default:
>   			_set_timer(&(pLed->BlinkTimer), LED_BLINK_SLOWLY_INTERVAL);
> --
> 2.30.2
> 
> 

Hi Saurav,

this does not apply to staging-testing. Please rebase against
staging-testing and send v2.

Thanks,

Michael

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[Fabio M. De Francesco]

On Sunday, August 22, 2021 1:58:10 AM CEST Phillip Potter wrote:
> On Sat, 21 Aug 2021 at 21:23, Saurav Girepunje
> 
> <saurav.girepunje@gmail.com> wrote:
> > Remove the condition with no effect (if == else) in rtw_led.c
> > file.
> > 
> > Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> > ---
> > 
> >  drivers/staging/r8188eu/core/rtw_led.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> Thanks for this, looks good.
> 
> Acked-by: Phillip Potter <phil@philpotter.co.uk>
> 
> Regards,
> Phil

Dear Philip,

Before acking, please check at least if it applies to the current version of 
the tree and check if it compiles without adding warnings and / or errors. :-)

Thanks,

Fabio
 

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[Greg KH]

On Sun, Aug 22, 2021 at 01:06:09PM +0200, Fabio M. De Francesco wrote:
> On Sunday, August 22, 2021 1:58:10 AM CEST Phillip Potter wrote:
> > On Sat, 21 Aug 2021 at 21:23, Saurav Girepunje
> > 
> > <saurav.girepunje@gmail.com> wrote:
> > > Remove the condition with no effect (if == else) in rtw_led.c
> > > file.
> > > 
> > > Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> > > ---
> > > 
> > >  drivers/staging/r8188eu/core/rtw_led.c | 5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > Thanks for this, looks good.
> > 
> > Acked-by: Phillip Potter <phil@philpotter.co.uk>
> > 
> > Regards,
> > Phil
> 
> Dear Philip,
> 
> Before acking, please check at least if it applies to the current version of 
> the tree and check if it compiles without adding warnings and / or errors. :-)

That is not necessary or needed here, I can do that when I apply the
patch.

greg k-h

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[Phillip Potter]

On Sun, 22 Aug 2021 at 12:06, Fabio M. De Francesco
<fmdefrancesco@gmail.com> wrote:
>
> On Sunday, August 22, 2021 1:58:10 AM CEST Phillip Potter wrote:
> > On Sat, 21 Aug 2021 at 21:23, Saurav Girepunje
> >
> > <saurav.girepunje@gmail.com> wrote:
> > > Remove the condition with no effect (if == else) in rtw_led.c
> > > file.
> > >
> > > Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> > > ---
> > >
> > >  drivers/staging/r8188eu/core/rtw_led.c | 5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > Thanks for this, looks good.
> >
> > Acked-by: Phillip Potter <phil@philpotter.co.uk>
> >
> > Regards,
> > Phil
>
> Dear Philip,
>
> Before acking, please check at least if it applies to the current version of
> the tree and check if it compiles without adding warnings and / or errors. :-)
>
> Thanks,
>
> Fabio
>
>
>
>

Dear Fabio,

An Acked-by merely signals acknowledgement of the patch, and that is
looks OK to the person offering the tag. Please see the following
quote from the kernel.org documentation:
"Acked-by: is not as formal as Signed-off-by:. It is a record that the
acker has at least reviewed the patch and has indicated acceptance."
It is not, to my knowledge, a commitment from the reviewer that the
patch applies to the given tree at that precise moment in time.

I reviewed the patch, and indicated my acceptance - the content of the
patch is fine. Whilst I will often make an effort to merge + build
test many patches, I will not do this with all of them, I simply don't
have the time due to other commitments. You can be assured that if I
have offered this tag I have at least read the patch and it looks
correct to me.

Particularly with a driver as in flux as this one, there are going to
be many merge conflicts. Advice such as this to me is not particularly
helpful, as I can promise you I'm trying :-)

Regards,
Phil

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[Fabio M. De Francesco]

On Sunday, August 22, 2021 4:58:10 PM CEST Phillip Potter wrote:
> Dear Fabio,
> 
> An Acked-by merely signals acknowledgement of the patch, and that is
> looks OK to the person offering the tag. Please see the following
> quote from the kernel.org documentation:
> "Acked-by: is not as formal as Signed-off-by:. It is a record that the
> acker has at least reviewed the patch and has indicated acceptance."
> It is not, to my knowledge, a commitment from the reviewer that the
> patch applies to the given tree at that precise moment in time.

Dear Philip,

I didn't mean to be harsh with you, I apologize if this is the message 
I conveyed. Really!

> I reviewed the patch, and indicated my acceptance - the content of the
> patch is fine. Whilst I will often make an effort to merge + build
> test many patches, I will not do this with all of them, I simply don't
> have the time due to other commitments. You can be assured that if I
> have offered this tag I have at least read the patch and it looks
> correct to me.

Now it is clearer to me what acking means. I've given only a handful of
acks because I thought I should also check if they applied and if they
build. It takes time. Now I understand it is not required. Thanks.
 
> Particularly with a driver as in flux as this one, there are going to
> be many merge conflicts. Advice such as this to me is not particularly
> helpful, as I can promise you I'm trying :-)

Please, don't ever think I'm not more than sure that you give a lot
of your _unpaid_ time to the kernel and I thank you very much
I know what it means, because I too have other commitments :-)

Cheers,

Fabio 

> Regards,
> Phil
> 

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[Phillip Potter]

On Sun, 22 Aug 2021 at 17:24, Fabio M. De Francesco
<fmdefrancesco@gmail.com> wrote:
>
>
> Dear Philip,
>
> I didn't mean to be harsh with you, I apologize if this is the message
> I conveyed. Really!

Dear Fabio,

No apology necessary :-)

>
>
> Now it is clearer to me what acking means. I've given only a handful of
> acks because I thought I should also check if they applied and if they
> build. It takes time. Now I understand it is not required. Thanks.
>

Yeah I try to ack as many as I can now, but couldn't possibly do as
many as I do if I had to build test them all, especially with my own
code to write and a family/full time non-kernel dev role. It is
certainly desirable for the majority, but if the code looks
good/correct and is a simple patch then in my mind an Acked-by is fine
to give in any case. That said, I will no doubt make mistakes on that.

>
> Please, don't ever think I'm not more than sure that you give a lot
> of your _unpaid_ time to the kernel and I thank you very much
> I know what it means, because I too have other commitments :-)

Thank you, and I appreciate your involvement so keep up the good work :-)

Regards,
Phil

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[SAURAV GIREPUNJE]

On 23 Aug 2021 00:58, Phillip Potter wrote:
> On Sun, 22 Aug 2021 at 17:24, Fabio M. De Francesco
> <fmdefrancesco@gmail.com> wrote:
> >
> >
> > Dear Philip,
> >
> > I didn't mean to be harsh with you, I apologize if this is the message
> > I conveyed. Really!
>
> Dear Fabio,
>
> No apology necessary :-)
>
> >
> >
> > Now it is clearer to me what acking means. I've given only a handful of
> > acks because I thought I should also check if they applied and if they
> > build. It takes time. Now I understand it is not required. Thanks.
> >
>
> Yeah I try to ack as many as I can now, but couldn't possibly do as
> many as I do if I had to build test them all, especially with my own
> code to write and a family/full time non-kernel dev role. It is
> certainly desirable for the majority, but if the code looks
> good/correct and is a simple patch then in my mind an Acked-by is fine
> to give in any case. That said, I will no doubt make mistakes on that.
>
> >
> > Please, don't ever think I'm not more than sure that you give a lot
> > of your _unpaid_ time to the kernel and I thank you very much
> > I know what it means, because I too have other commitments :-)
>
> Thank you, and I appreciate your involvement so keep up the good work :-)
>
> Regards,
> Phil


Thanks Michael, Phil, Fabio for review.

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[Greg KH]

On Sun, Aug 22, 2021 at 02:36:11PM +0200, Greg KH wrote:
> On Sun, Aug 22, 2021 at 01:06:09PM +0200, Fabio M. De Francesco wrote:
> > On Sunday, August 22, 2021 1:58:10 AM CEST Phillip Potter wrote:
> > > On Sat, 21 Aug 2021 at 21:23, Saurav Girepunje
> > > 
> > > <saurav.girepunje@gmail.com> wrote:
> > > > Remove the condition with no effect (if == else) in rtw_led.c
> > > > file.
> > > > 
> > > > Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> > > > ---
> > > > 
> > > >  drivers/staging/r8188eu/core/rtw_led.c | 5 +----
> > > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > 
> > > Thanks for this, looks good.
> > > 
> > > Acked-by: Phillip Potter <phil@philpotter.co.uk>
> > > 
> > > Regards,
> > > Phil
> > 
> > Dear Philip,
> > 
> > Before acking, please check at least if it applies to the current version of 
> > the tree and check if it compiles without adding warnings and / or errors. :-)
> 
> That is not necessary or needed here, I can do that when I apply the
> patch.

As proof of that, this patch applied just fine to my tree right now...

thanks,

greg k-h

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[SAURAV GIREPUNJE]

On 26 Aug 2021 12:34, Greg KH wrote:
> On Sun, Aug 22, 2021 at 02:36:11PM +0200, Greg KH wrote:
> > On Sun, Aug 22, 2021 at 01:06:09PM +0200, Fabio M. De Francesco wrote:
> > > On Sunday, August 22, 2021 1:58:10 AM CEST Phillip Potter wrote:
> > > > On Sat, 21 Aug 2021 at 21:23, Saurav Girepunje
> > > >
> > > > <saurav.girepunje@gmail.com> wrote:
> > > > > Remove the condition with no effect (if == else) in rtw_led.c
> > > > > file.
> > > > >
> > > > > Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> > > > > ---
> > > > >
> > > > >  drivers/staging/r8188eu/core/rtw_led.c | 5 +----
> > > > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > >
> > > > Thanks for this, looks good.
> > > >
> > > > Acked-by: Phillip Potter <phil@philpotter.co.uk>
> > > >
> > > > Regards,
> > > > Phil
> > >
> > > Dear Philip,
> > >
> > > Before acking, please check at least if it applies to the current version of
> > > the tree and check if it compiles without adding warnings and / or errors. :-)
> >
> > That is not necessary or needed here, I can do that when I apply the
> > patch.
>
> As proof of that, this patch applied just fine to my tree right now...
>
> thanks,
>
> greg k-h

Thanks greg

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[Michael Straube]

On 8/29/21 11:01, Saurav Girepunje wrote:
> Remove the condition with no effect (if == else) and group multiple
> cases which execute same statement in rtw_mlme_ext.c
> 
> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> ---
>   drivers/staging/r8188eu/core/rtw_mlme_ext.c | 9 ---------
>   1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> index 5a472a4954b0..4178b3c1ff57 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> @@ -453,17 +453,8 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
>   		fallthrough;
>   	case WIFI_ASSOCREQ:
>   	case WIFI_REASSOCREQ:
> -		_mgt_dispatcher(padapter, ptable, precv_frame);
> -		break;
>   	case WIFI_PROBEREQ:
> -		if (check_fwstate(pmlmepriv, WIFI_AP_STATE))
> -			_mgt_dispatcher(padapter, ptable, precv_frame);
> -		else
> -			_mgt_dispatcher(padapter, ptable, precv_frame);
> -		break;
>   	case WIFI_BEACON:
> -		_mgt_dispatcher(padapter, ptable, precv_frame);
> -		break;
>   	case WIFI_ACTION:
>   		_mgt_dispatcher(padapter, ptable, precv_frame);
>   		break;
> --
> 2.32.0
> 

Acked-by: Michael Straube <straube.linux@gmail.com>

Thanks,
Michael

Re: [PATCH] staging: r8188eu: core: remove condition with no effect

[Pavel Skripkin]

On 8/29/21 12:01 PM, Saurav Girepunje wrote:
> Remove the condition with no effect (if == else) and group multiple
> cases which execute same statement in rtw_mlme_ext.c
> 
> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>

Reviewed-by: Pavel Skripkin <paskripkin@gmail.com>

[PATCH] staging: r8188eu: core: remove condition with no effect

[Saurav Girepunje]

Remove the condition with no effect (if == else) and group multiple
cases which execute same statement in rtw_mlme_ext.c

Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 5a472a4954b0..4178b3c1ff57 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -453,17 +453,8 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 		fallthrough;
 	case WIFI_ASSOCREQ:
 	case WIFI_REASSOCREQ:
-		_mgt_dispatcher(padapter, ptable, precv_frame);
-		break;
 	case WIFI_PROBEREQ:
-		if (check_fwstate(pmlmepriv, WIFI_AP_STATE))
-			_mgt_dispatcher(padapter, ptable, precv_frame);
-		else
-			_mgt_dispatcher(padapter, ptable, precv_frame);
-		break;
 	case WIFI_BEACON:
-		_mgt_dispatcher(padapter, ptable, precv_frame);
-		break;
 	case WIFI_ACTION:
 		_mgt_dispatcher(padapter, ptable, precv_frame);
 		break;
--
2.32.0