2022-09-18 18:26:07

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 0/6] staging: r8188eu: another round of led layer cleanups

Here's some more cleanups for the led layer. This series should be applied
on top of the "staging: r8188eu: more led cleanups" series I sent on Sep
11th.

Martin Kaiser (6):
staging: r8188eu: cancel blink_work during wps stop
staging: r8188eu: update status before wps success blinking
staging: r8188eu: remove bLedNoLinkBlinkInProgress
staging: r8188eu: remove BlinkingLedState
staging: r8188eu: remove duplicate bSurpriseRemoved check
staging: r8188eu: remove two unused enum entries

drivers/staging/r8188eu/core/rtw_led.c | 104 +++-------------------
drivers/staging/r8188eu/include/rtw_led.h | 8 --
2 files changed, 12 insertions(+), 100 deletions(-)

--
2.30.2


2022-09-18 18:26:08

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 5/6] staging: r8188eu: remove duplicate bSurpriseRemoved check

We don't have to check bSurpriseRemoved in the SwLedOn function.

SwLedOn calls rtw_read8 which in turn calls usb_read. This function checks
bSurpriseRemoved for us.

Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/staging/r8188eu/core/rtw_led.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
index 744247af5956..989808a2b171 100644
--- a/drivers/staging/r8188eu/core/rtw_led.c
+++ b/drivers/staging/r8188eu/core/rtw_led.c
@@ -35,7 +35,7 @@ static void SwLedOn(struct adapter *padapter, struct led_priv *pLed)
u8 LedCfg;
int res;

- if (padapter->bSurpriseRemoved || padapter->bDriverStopped)
+ if (padapter->bDriverStopped)
return;

res = rtw_read8(padapter, REG_LEDCFG2, &LedCfg);
--
2.30.2

2022-09-18 18:26:07

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 4/6] staging: r8188eu: remove BlinkingLedState

Both bLedOn and BlinkingLedState in struct led_priv store the same
information.

The boolean bLedOn stores the curent led state while BlinkingLedState
stores the next led state to be set during blinking, which is the inverse
of the current led state. (The led is either off or blinking, it's never
continuously on.)

This patch removes BlinkingLedState and uses bLedOn instead.

The LED_BLINK_WPS_STOP case in blink_work checked for
pLed->BlinkingLedState != RTW_LED_ON. This is true if the next led
blinking state is ON, i.e. if the led has just been switched off by
blink_work, i.e. if (!pLed->bLedOn).

Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/staging/r8188eu/core/rtw_led.c | 62 ++---------------------
drivers/staging/r8188eu/include/rtw_led.h | 4 --
2 files changed, 4 insertions(+), 62 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
index aee3ea3613a9..744247af5956 100644
--- a/drivers/staging/r8188eu/core/rtw_led.c
+++ b/drivers/staging/r8188eu/core/rtw_led.c
@@ -25,7 +25,6 @@ static void ResetLedStatus(struct led_priv *pLed)
pLed->bLedWPSBlinkInProgress = false;

pLed->BlinkTimes = 0; /* Number of times to toggle led state for blinking. */
- pLed->BlinkingLedState = LED_UNKNOWN; /* Next state for blinking, either RTW_LED_ON or RTW_LED_OFF are. */

pLed->bLedLinkBlinkInProgress = false;
pLed->bLedScanBlinkInProgress = false;
@@ -87,32 +86,19 @@ static void blink_work(struct work_struct *work)
return;
}

- /* Change LED according to BlinkingLedState specified. */
- if (pLed->BlinkingLedState == RTW_LED_ON)
- SwLedOn(padapter, pLed);
- else
+ if (pLed->bLedOn)
SwLedOff(padapter, pLed);
+ else
+ SwLedOn(padapter, pLed);

switch (pLed->CurrLedState) {
case LED_BLINK_SLOWLY:
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_NO_LINK_INTVL);
break;
case LED_BLINK_NORMAL:
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
break;
case LED_BLINK_SCAN:
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
pLed->BlinkTimes--;
if (pLed->BlinkTimes == 0) {
if (check_fwstate(pmlmepriv, _FW_LINKED)) {
@@ -129,10 +115,6 @@ static void blink_work(struct work_struct *work)
}
break;
case LED_BLINK_TXRX:
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
pLed->BlinkTimes--;
if (pLed->BlinkTimes == 0) {
if (check_fwstate(pmlmepriv, _FW_LINKED)) {
@@ -149,25 +131,16 @@ static void blink_work(struct work_struct *work)
}
break;
case LED_BLINK_WPS:
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_SCAN_INTVL);
break;
case LED_BLINK_WPS_STOP: /* WPS success */
- if (pLed->BlinkingLedState != RTW_LED_ON) {
+ if (!pLed->bLedOn) {
pLed->bLedLinkBlinkInProgress = true;
pLed->CurrLedState = LED_BLINK_NORMAL;
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);

pLed->bLedWPSBlinkInProgress = false;
} else {
- pLed->BlinkingLedState = RTW_LED_OFF;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_WPS_SUCESS_INTVL);
}
break;
@@ -223,10 +196,6 @@ void rtw_led_control(struct adapter *padapter, enum LED_CTL_MODE LedAction)
pLed->bLedBlinkInProgress = false;

pLed->CurrLedState = LED_BLINK_SLOWLY;
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_NO_LINK_INTVL);
break;
case LED_CTL_LINK:
@@ -242,10 +211,6 @@ void rtw_led_control(struct adapter *padapter, enum LED_CTL_MODE LedAction)
pLed->bLedLinkBlinkInProgress = true;

pLed->CurrLedState = LED_BLINK_NORMAL;
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
break;
case LED_CTL_SITE_SURVEY:
@@ -266,10 +231,6 @@ void rtw_led_control(struct adapter *padapter, enum LED_CTL_MODE LedAction)

pLed->CurrLedState = LED_BLINK_SCAN;
pLed->BlinkTimes = 24;
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_SCAN_INTVL);
break;
case LED_CTL_TX:
@@ -287,10 +248,6 @@ void rtw_led_control(struct adapter *padapter, enum LED_CTL_MODE LedAction)

pLed->CurrLedState = LED_BLINK_TXRX;
pLed->BlinkTimes = 2;
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_FASTER_INTVL);
break;
case LED_CTL_START_WPS: /* wait until xinpin finish */
@@ -304,10 +261,6 @@ void rtw_led_control(struct adapter *padapter, enum LED_CTL_MODE LedAction)
pLed->bLedScanBlinkInProgress = false;
pLed->bLedWPSBlinkInProgress = true;
pLed->CurrLedState = LED_BLINK_WPS;
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_SCAN_INTVL);
break;
case LED_CTL_STOP_WPS:
@@ -320,10 +273,8 @@ void rtw_led_control(struct adapter *padapter, enum LED_CTL_MODE LedAction)

pLed->CurrLedState = LED_BLINK_WPS_STOP;
if (pLed->bLedOn) {
- pLed->BlinkingLedState = RTW_LED_OFF;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_WPS_SUCESS_INTVL);
} else {
- pLed->BlinkingLedState = RTW_LED_ON;
schedule_delayed_work(&pLed->blink_work, 0);
}
break;
@@ -331,15 +282,10 @@ void rtw_led_control(struct adapter *padapter, enum LED_CTL_MODE LedAction)
cancel_delayed_work(&pLed->blink_work);
pLed->bLedWPSBlinkInProgress = false;
pLed->CurrLedState = LED_BLINK_SLOWLY;
- if (pLed->bLedOn)
- pLed->BlinkingLedState = RTW_LED_OFF;
- else
- pLed->BlinkingLedState = RTW_LED_ON;
schedule_delayed_work(&pLed->blink_work, LED_BLINK_NO_LINK_INTVL);
break;
case LED_CTL_POWER_OFF:
pLed->CurrLedState = RTW_LED_OFF;
- pLed->BlinkingLedState = RTW_LED_OFF;
pLed->bLedLinkBlinkInProgress = false;
pLed->bLedBlinkInProgress = false;
pLed->bLedWPSBlinkInProgress = false;
diff --git a/drivers/staging/r8188eu/include/rtw_led.h b/drivers/staging/r8188eu/include/rtw_led.h
index 6a0881990394..7b1775b9fa64 100644
--- a/drivers/staging/r8188eu/include/rtw_led.h
+++ b/drivers/staging/r8188eu/include/rtw_led.h
@@ -21,8 +21,6 @@ enum LED_CTL_MODE {
};

enum LED_STATE_871x {
- LED_UNKNOWN = 0,
- RTW_LED_ON = 1,
RTW_LED_OFF = 2,
LED_BLINK_NORMAL = 3,
LED_BLINK_SLOWLY = 4,
@@ -43,8 +41,6 @@ struct led_priv {
bool bRegUseLed;

enum LED_STATE_871x CurrLedState; /* Current LED state. */
- enum LED_STATE_871x BlinkingLedState; /* Next state for blinking,
- * either RTW_LED_ON or RTW_LED_OFF are. */

bool bLedOn; /* true if LED is ON, false if LED is OFF. */

--
2.30.2

2022-09-19 01:39:29

by Philipp Hortmann

[permalink] [raw]
Subject: Re: [PATCH 0/6] staging: r8188eu: another round of led layer cleanups

On 9/18/22 19:56, Martin Kaiser wrote:
> Here's some more cleanups for the led layer. This series should be applied
> on top of the "staging: r8188eu: more led cleanups" series I sent on Sep
> 11th.
>
> Martin Kaiser (6):
> staging: r8188eu: cancel blink_work during wps stop
> staging: r8188eu: update status before wps success blinking
> staging: r8188eu: remove bLedNoLinkBlinkInProgress
> staging: r8188eu: remove BlinkingLedState
> staging: r8188eu: remove duplicate bSurpriseRemoved check
> staging: r8188eu: remove two unused enum entries
>
> drivers/staging/r8188eu/core/rtw_led.c | 104 +++-------------------
> drivers/staging/r8188eu/include/rtw_led.h | 8 --
> 2 files changed, 12 insertions(+), 100 deletions(-)
>

Observed LED. All OK

Tested-by: Philipp Hortmann <[email protected]> # Edimax N150

2022-09-19 05:50:04

by Michael Straube

[permalink] [raw]
Subject: Re: [PATCH 5/6] staging: r8188eu: remove duplicate bSurpriseRemoved check

On 9/18/22 19:56, Martin Kaiser wrote:
> We don't have to check bSurpriseRemoved in the SwLedOn function.
>
> SwLedOn calls rtw_read8 which in turn calls usb_read. This function checks
> bSurpriseRemoved for us.
>
> Signed-off-by: Martin Kaiser <[email protected]>
> ---
> drivers/staging/r8188eu/core/rtw_led.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
> index 744247af5956..989808a2b171 100644
> --- a/drivers/staging/r8188eu/core/rtw_led.c
> +++ b/drivers/staging/r8188eu/core/rtw_led.c
> @@ -35,7 +35,7 @@ static void SwLedOn(struct adapter *padapter, struct led_priv *pLed)
> u8 LedCfg;
> int res;
>
> - if (padapter->bSurpriseRemoved || padapter->bDriverStopped)
> + if (padapter->bDriverStopped)
> return;
>
> res = rtw_read8(padapter, REG_LEDCFG2, &LedCfg);

Hi Martin,

same could be done in SwLedOff.

regards,
Michael

2022-09-19 20:41:13

by Martin Kaiser

[permalink] [raw]
Subject: Re: [PATCH 5/6] staging: r8188eu: remove duplicate bSurpriseRemoved check

Hi Michael,

Thus wrote Michael Straube ([email protected]):

> On 9/18/22 19:56, Martin Kaiser wrote:
> > We don't have to check bSurpriseRemoved in the SwLedOn function.

> > SwLedOn calls rtw_read8 which in turn calls usb_read. This function checks
> > bSurpriseRemoved for us.

> > Signed-off-by: Martin Kaiser <[email protected]>
> > ---
> > drivers/staging/r8188eu/core/rtw_led.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)

> > diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
> > index 744247af5956..989808a2b171 100644
> > --- a/drivers/staging/r8188eu/core/rtw_led.c
> > +++ b/drivers/staging/r8188eu/core/rtw_led.c
> > @@ -35,7 +35,7 @@ static void SwLedOn(struct adapter *padapter, struct led_priv *pLed)
> > u8 LedCfg;
> > int res;
> > - if (padapter->bSurpriseRemoved || padapter->bDriverStopped)
> > + if (padapter->bDriverStopped)
> > return;
> > res = rtw_read8(padapter, REG_LEDCFG2, &LedCfg);

> Hi Martin,

> same could be done in SwLedOff.

you're right. I wanted to kick off the bSurpriseRemoved cleanup with a
simple patch and wait for people's comments. I'll do SwLedOff when this
one's accepted.

Thanks,
Martin