linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] staging: r8188eu: clean up workers
@ 2021-11-25 16:25 Martin Kaiser
  2021-11-25 16:25 ` [PATCH 1/4] staging: r8188eu: remove the _set_workitem wrapper Martin Kaiser
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Martin Kaiser @ 2021-11-25 16:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
	linux-kernel, Martin Kaiser

Remove the wrapper functions for work items. Use a delayed worker in the
led layer.

I tested this series on top of "clean up efuse / eeprom code".

Martin Kaiser (4):
  staging: r8188eu: remove the _set_workitem wrapper
  staging: r8188eu: remove the _init_workitem wrapper
  staging: r8188eu: remove the _cancel_workitem_sync wrapper
  staging: r8188eu: use a delayed worker for led updates

 drivers/staging/r8188eu/core/rtw_cmd.c        |   4 +-
 drivers/staging/r8188eu/core/rtw_led.c        | 106 ++++++++----------
 .../staging/r8188eu/include/osdep_service.h   |  14 ---
 drivers/staging/r8188eu/include/rtw_led.h     |  16 +--
 4 files changed, 55 insertions(+), 85 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] staging: r8188eu: remove the _set_workitem wrapper
  2021-11-25 16:25 [PATCH 0/4] staging: r8188eu: clean up workers Martin Kaiser
@ 2021-11-25 16:25 ` Martin Kaiser
  2021-11-25 16:25 ` [PATCH 2/4] staging: r8188eu: remove the _init_workitem wrapper Martin Kaiser
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Martin Kaiser @ 2021-11-25 16:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
	linux-kernel, Martin Kaiser

Remove the _set_workitem wrapper and call schedule_work directly.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_led.c          | 2 +-
 drivers/staging/r8188eu/include/osdep_service.h | 5 -----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
index 0e3453639a8b..d48ed98453f5 100644
--- a/drivers/staging/r8188eu/core/rtw_led.c
+++ b/drivers/staging/r8188eu/core/rtw_led.c
@@ -12,7 +12,7 @@ void BlinkTimerCallback(struct timer_list *t)
 	if ((padapter->bSurpriseRemoved) || (padapter->bDriverStopped))
 		return;
 
-	_set_workitem(&pLed->BlinkWorkItem);
+	schedule_work(&pLed->BlinkWorkItem);
 }
 
 void BlinkWorkItemCallback(struct work_struct *work)
diff --git a/drivers/staging/r8188eu/include/osdep_service.h b/drivers/staging/r8188eu/include/osdep_service.h
index f6f5e4581212..766440461a0c 100644
--- a/drivers/staging/r8188eu/include/osdep_service.h
+++ b/drivers/staging/r8188eu/include/osdep_service.h
@@ -79,11 +79,6 @@ static inline void _init_workitem(struct work_struct *pwork, void *pfunc, void *
 	INIT_WORK(pwork, pfunc);
 }
 
-static inline void _set_workitem(struct work_struct *pwork)
-{
-	schedule_work(pwork);
-}
-
 static inline void _cancel_workitem_sync(struct work_struct *pwork)
 {
 	cancel_work_sync(pwork);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] staging: r8188eu: remove the _init_workitem wrapper
  2021-11-25 16:25 [PATCH 0/4] staging: r8188eu: clean up workers Martin Kaiser
  2021-11-25 16:25 ` [PATCH 1/4] staging: r8188eu: remove the _set_workitem wrapper Martin Kaiser
@ 2021-11-25 16:25 ` Martin Kaiser
  2021-11-25 16:25 ` [PATCH 3/4] staging: r8188eu: remove the _cancel_workitem_sync wrapper Martin Kaiser
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Martin Kaiser @ 2021-11-25 16:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
	linux-kernel, Martin Kaiser

Remove the _init_workitem wrapper and call INIT_WORK directly.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_cmd.c          | 2 +-
 drivers/staging/r8188eu/core/rtw_led.c          | 2 +-
 drivers/staging/r8188eu/include/osdep_service.h | 5 -----
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_cmd.c b/drivers/staging/r8188eu/core/rtw_cmd.c
index 1f4cc321bd1a..fb8ba7ded489 100644
--- a/drivers/staging/r8188eu/core/rtw_cmd.c
+++ b/drivers/staging/r8188eu/core/rtw_cmd.c
@@ -69,7 +69,7 @@ static int _rtw_init_evt_priv(struct evt_priv *pevtpriv)
 	atomic_set(&pevtpriv->event_seq, 0);
 	pevtpriv->evt_done_cnt = 0;
 
-	_init_workitem(&pevtpriv->c2h_wk, c2h_wk_callback, NULL);
+	INIT_WORK(&pevtpriv->c2h_wk, c2h_wk_callback);
 	pevtpriv->c2h_wk_alive = false;
 	pevtpriv->c2h_queue = rtw_cbuf_alloc(C2H_QUEUE_MAX_LEN + 1);
 
diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
index d48ed98453f5..0aebdc3c497d 100644
--- a/drivers/staging/r8188eu/core/rtw_led.c
+++ b/drivers/staging/r8188eu/core/rtw_led.c
@@ -46,7 +46,7 @@ void InitLed871x(struct adapter *padapter, struct LED_871x *pLed, enum LED_PIN_8
 	ResetLedStatus(pLed);
 
 	timer_setup(&pLed->BlinkTimer, BlinkTimerCallback, 0);
-	_init_workitem(&pLed->BlinkWorkItem, BlinkWorkItemCallback, pLed);
+	INIT_WORK(&pLed->BlinkWorkItem, BlinkWorkItemCallback);
 }
 
 void DeInitLed871x(struct LED_871x *pLed)
diff --git a/drivers/staging/r8188eu/include/osdep_service.h b/drivers/staging/r8188eu/include/osdep_service.h
index 766440461a0c..21e5cacbd893 100644
--- a/drivers/staging/r8188eu/include/osdep_service.h
+++ b/drivers/staging/r8188eu/include/osdep_service.h
@@ -74,11 +74,6 @@ static inline void _cancel_timer(struct timer_list *ptimer,u8 *bcancelled)
 #define RTW_TIMER_HDL_NAME(name) rtw_##name##_timer_hdl
 #define RTW_DECLARE_TIMER_HDL(name) void RTW_TIMER_HDL_NAME(name)(RTW_TIMER_HDL_ARGS)
 
-static inline void _init_workitem(struct work_struct *pwork, void *pfunc, void * cntx)
-{
-	INIT_WORK(pwork, pfunc);
-}
-
 static inline void _cancel_workitem_sync(struct work_struct *pwork)
 {
 	cancel_work_sync(pwork);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] staging: r8188eu: remove the _cancel_workitem_sync wrapper
  2021-11-25 16:25 [PATCH 0/4] staging: r8188eu: clean up workers Martin Kaiser
  2021-11-25 16:25 ` [PATCH 1/4] staging: r8188eu: remove the _set_workitem wrapper Martin Kaiser
  2021-11-25 16:25 ` [PATCH 2/4] staging: r8188eu: remove the _init_workitem wrapper Martin Kaiser
@ 2021-11-25 16:25 ` Martin Kaiser
  2021-11-25 16:25 ` [PATCH 4/4] staging: r8188eu: use a delayed worker for led updates Martin Kaiser
  2021-11-26 11:41 ` [PATCH v2] " Martin Kaiser
  4 siblings, 0 replies; 9+ messages in thread
From: Martin Kaiser @ 2021-11-25 16:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
	linux-kernel, Martin Kaiser

Remove the _cancel_workitem_sync wrapper and call
cancel_work_sync directly.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_cmd.c          | 2 +-
 drivers/staging/r8188eu/core/rtw_led.c          | 2 +-
 drivers/staging/r8188eu/include/osdep_service.h | 4 ----
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_cmd.c b/drivers/staging/r8188eu/core/rtw_cmd.c
index fb8ba7ded489..d0a61331b883 100644
--- a/drivers/staging/r8188eu/core/rtw_cmd.c
+++ b/drivers/staging/r8188eu/core/rtw_cmd.c
@@ -78,7 +78,7 @@ static int _rtw_init_evt_priv(struct evt_priv *pevtpriv)
 
 void rtw_free_evt_priv(struct	evt_priv *pevtpriv)
 {
-	_cancel_workitem_sync(&pevtpriv->c2h_wk);
+	cancel_work_sync(&pevtpriv->c2h_wk);
 	while (pevtpriv->c2h_wk_alive)
 		msleep(10);
 
diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
index 0aebdc3c497d..ae46fd48f940 100644
--- a/drivers/staging/r8188eu/core/rtw_led.c
+++ b/drivers/staging/r8188eu/core/rtw_led.c
@@ -51,7 +51,7 @@ void InitLed871x(struct adapter *padapter, struct LED_871x *pLed, enum LED_PIN_8
 
 void DeInitLed871x(struct LED_871x *pLed)
 {
-	_cancel_workitem_sync(&pLed->BlinkWorkItem);
+	cancel_work_sync(&pLed->BlinkWorkItem);
 	_cancel_timer_ex(&pLed->BlinkTimer);
 	ResetLedStatus(pLed);
 }
diff --git a/drivers/staging/r8188eu/include/osdep_service.h b/drivers/staging/r8188eu/include/osdep_service.h
index 21e5cacbd893..5d8b567a3165 100644
--- a/drivers/staging/r8188eu/include/osdep_service.h
+++ b/drivers/staging/r8188eu/include/osdep_service.h
@@ -74,10 +74,6 @@ static inline void _cancel_timer(struct timer_list *ptimer,u8 *bcancelled)
 #define RTW_TIMER_HDL_NAME(name) rtw_##name##_timer_hdl
 #define RTW_DECLARE_TIMER_HDL(name) void RTW_TIMER_HDL_NAME(name)(RTW_TIMER_HDL_ARGS)
 
-static inline void _cancel_workitem_sync(struct work_struct *pwork)
-{
-	cancel_work_sync(pwork);
-}
 /*  */
 /*  Global Mutex: can only be used at PASSIVE level. */
 /*  */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] staging: r8188eu: use a delayed worker for led updates
  2021-11-25 16:25 [PATCH 0/4] staging: r8188eu: clean up workers Martin Kaiser
                   ` (2 preceding siblings ...)
  2021-11-25 16:25 ` [PATCH 3/4] staging: r8188eu: remove the _cancel_workitem_sync wrapper Martin Kaiser
@ 2021-11-25 16:25 ` Martin Kaiser
  2021-11-26 11:41 ` [PATCH v2] " Martin Kaiser
  4 siblings, 0 replies; 9+ messages in thread
From: Martin Kaiser @ 2021-11-25 16:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
	linux-kernel, Martin Kaiser

The led layer uses a combination of timer and worker for periodic led
updates, e.g. for blinking. The reason seems to be that blocking
operations like a usb read are not allowed in a timer handler.

Replace the combination of timer and worker with a delayed worker.

Convert the timeout defines from milliseconds to jiffies to make them
usable as delays for the delayed worker. Shorten the names of the defines
and rename the work item to make checkpatch happy.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_led.c    | 106 ++++++++++------------
 drivers/staging/r8188eu/include/rtw_led.h |  16 ++--
 2 files changed, 53 insertions(+), 69 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
index ae46fd48f940..31994d6ba26f 100644
--- a/drivers/staging/r8188eu/core/rtw_led.c
+++ b/drivers/staging/r8188eu/core/rtw_led.c
@@ -4,20 +4,10 @@
 #include "../include/drv_types.h"
 #include "../include/rtw_led.h"
 
-void BlinkTimerCallback(struct timer_list *t)
-{
-	struct LED_871x *pLed = from_timer(pLed, t, BlinkTimer);
-	struct adapter *padapter = pLed->padapter;
-
-	if ((padapter->bSurpriseRemoved) || (padapter->bDriverStopped))
-		return;
-
-	schedule_work(&pLed->BlinkWorkItem);
-}
-
 void BlinkWorkItemCallback(struct work_struct *work)
 {
-	struct LED_871x *pLed = container_of(work, struct LED_871x, BlinkWorkItem);
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct LED_871x *pLed = container_of(dwork, struct LED_871x, blink_work);
 	BlinkHandler(pLed);
 }
 
@@ -45,14 +35,12 @@ void InitLed871x(struct adapter *padapter, struct LED_871x *pLed, enum LED_PIN_8
 
 	ResetLedStatus(pLed);
 
-	timer_setup(&pLed->BlinkTimer, BlinkTimerCallback, 0);
-	INIT_WORK(&pLed->BlinkWorkItem, BlinkWorkItemCallback);
+	INIT_DELAYED_WORK(&pLed->blink_work, BlinkWorkItemCallback);
 }
 
 void DeInitLed871x(struct LED_871x *pLed)
 {
-	cancel_work_sync(&pLed->BlinkWorkItem);
-	_cancel_timer_ex(&pLed->BlinkTimer);
+	cancel_delayed_work_sync(&pLed->blink_work);
 	ResetLedStatus(pLed);
 }
 
@@ -80,14 +68,14 @@ static void SwLedBlink1(struct LED_871x *pLed)
 			pLed->BlinkingLedState = RTW_LED_OFF;
 		else
 			pLed->BlinkingLedState = RTW_LED_ON;
-		_set_timer(&pLed->BlinkTimer, LED_BLINK_NO_LINK_INTERVAL_ALPHA);
+		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;
-		_set_timer(&pLed->BlinkTimer, LED_BLINK_LINK_INTERVAL_ALPHA);
+		schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
 		break;
 	case LED_BLINK_SCAN:
 		pLed->BlinkTimes--;
@@ -101,7 +89,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 					pLed->BlinkingLedState = RTW_LED_OFF;
 				else
 					pLed->BlinkingLedState = RTW_LED_ON;
-				_set_timer(&pLed->BlinkTimer, LED_BLINK_LINK_INTERVAL_ALPHA);
+				schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
 			} else if (!check_fwstate(pmlmepriv, _FW_LINKED)) {
 				pLed->bLedNoLinkBlinkInProgress = true;
 				pLed->CurrLedState = LED_BLINK_SLOWLY;
@@ -109,7 +97,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 					pLed->BlinkingLedState = RTW_LED_OFF;
 				else
 					pLed->BlinkingLedState = RTW_LED_ON;
-				_set_timer(&pLed->BlinkTimer, LED_BLINK_NO_LINK_INTERVAL_ALPHA);
+				schedule_delayed_work(&pLed->blink_work, LED_BLINK_NO_LINK_INTVL);
 			}
 			pLed->bLedScanBlinkInProgress = false;
 		} else {
@@ -117,7 +105,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_SCAN_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_SCAN_INTVL);
 		}
 		break;
 	case LED_BLINK_TXRX:
@@ -132,7 +120,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 					pLed->BlinkingLedState = RTW_LED_OFF;
 				else
 					pLed->BlinkingLedState = RTW_LED_ON;
-				_set_timer(&pLed->BlinkTimer, LED_BLINK_LINK_INTERVAL_ALPHA);
+				schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
 			} else if (!check_fwstate(pmlmepriv, _FW_LINKED)) {
 				pLed->bLedNoLinkBlinkInProgress = true;
 				pLed->CurrLedState = LED_BLINK_SLOWLY;
@@ -140,7 +128,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 					pLed->BlinkingLedState = RTW_LED_OFF;
 				else
 					pLed->BlinkingLedState = RTW_LED_ON;
-				_set_timer(&pLed->BlinkTimer, LED_BLINK_NO_LINK_INTERVAL_ALPHA);
+				schedule_delayed_work(&pLed->blink_work, LED_BLINK_NO_LINK_INTVL);
 			}
 			pLed->bLedBlinkInProgress = false;
 		} else {
@@ -148,7 +136,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_FASTER_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_FASTER_INTVL);
 		}
 		break;
 	case LED_BLINK_WPS:
@@ -156,7 +144,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 			pLed->BlinkingLedState = RTW_LED_OFF;
 		else
 			pLed->BlinkingLedState = RTW_LED_ON;
-		_set_timer(&pLed->BlinkTimer, LED_BLINK_SCAN_INTERVAL_ALPHA);
+		schedule_delayed_work(&pLed->blink_work, LED_BLINK_SCAN_INTVL);
 		break;
 	case LED_BLINK_WPS_STOP:	/* WPS success */
 		if (pLed->BlinkingLedState == RTW_LED_ON)
@@ -171,12 +159,12 @@ static void SwLedBlink1(struct LED_871x *pLed)
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_LINK_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
 
 			pLed->bLedWPSBlinkInProgress = false;
 		} else {
 			pLed->BlinkingLedState = RTW_LED_OFF;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_WPS_SUCESS_INTVL);
 		}
 		break;
 	default:
@@ -198,11 +186,11 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 			if (pLed->CurrLedState == LED_BLINK_SCAN || IS_LED_WPS_BLINKING(pLed))
 				return;
 			if (pLed->bLedLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedBlinkInProgress = false;
 			}
 
@@ -212,7 +200,7 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_NO_LINK_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_NO_LINK_INTVL);
 		}
 		break;
 	case LED_CTL_LINK:
@@ -220,11 +208,11 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 			if (pLed->CurrLedState == LED_BLINK_SCAN || IS_LED_WPS_BLINKING(pLed))
 				return;
 			if (pLed->bLedNoLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedNoLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedBlinkInProgress = false;
 			}
 			pLed->bLedLinkBlinkInProgress = true;
@@ -233,7 +221,7 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_LINK_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
 		}
 		break;
 	case LED_CTL_SITE_SURVEY:
@@ -243,15 +231,15 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 			if (IS_LED_WPS_BLINKING(pLed))
 				return;
 			if (pLed->bLedNoLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedNoLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedBlinkInProgress = false;
 			}
 			pLed->bLedScanBlinkInProgress = true;
@@ -261,7 +249,7 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_SCAN_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_SCAN_INTVL);
 		 }
 		break;
 	case LED_CTL_TX:
@@ -270,11 +258,11 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 			if (pLed->CurrLedState == LED_BLINK_SCAN || IS_LED_WPS_BLINKING(pLed))
 				return;
 			if (pLed->bLedNoLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedNoLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedLinkBlinkInProgress = false;
 			}
 			pLed->bLedBlinkInProgress = true;
@@ -284,26 +272,26 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_FASTER_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_FASTER_INTVL);
 		}
 		break;
 	case LED_CTL_START_WPS: /* wait until xinpin finish */
 	case LED_CTL_START_WPS_BOTTON:
 		 if (!pLed->bLedWPSBlinkInProgress) {
 			if (pLed->bLedNoLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedNoLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedBlinkInProgress = false;
 			}
 			if (pLed->bLedScanBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work_sync(&pLed->blink_work);
 				pLed->bLedScanBlinkInProgress = false;
 			}
 			pLed->bLedWPSBlinkInProgress = true;
@@ -312,42 +300,42 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_SCAN_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_SCAN_INTVL);
 		 }
 		break;
 	case LED_CTL_STOP_WPS:
 		if (pLed->bLedNoLinkBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work_sync(&pLed->blink_work);
 			pLed->bLedNoLinkBlinkInProgress = false;
 		}
 		if (pLed->bLedLinkBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work_sync(&pLed->blink_work);
 			pLed->bLedLinkBlinkInProgress = false;
 		}
 		if (pLed->bLedBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work_sync(&pLed->blink_work);
 			pLed->bLedBlinkInProgress = false;
 		}
 		if (pLed->bLedScanBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work_sync(&pLed->blink_work);
 			pLed->bLedScanBlinkInProgress = false;
 		}
 		if (pLed->bLedWPSBlinkInProgress)
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work_sync(&pLed->blink_work);
 		else
 			pLed->bLedWPSBlinkInProgress = true;
 		pLed->CurrLedState = LED_BLINK_WPS_STOP;
 		if (pLed->bLedOn) {
 			pLed->BlinkingLedState = RTW_LED_OFF;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_WPS_SUCESS_INTVL);
 		} else {
 			pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, 0);
+			schedule_delayed_work(&pLed->blink_work, 0);
 		}
 		break;
 	case LED_CTL_STOP_WPS_FAIL:
 		if (pLed->bLedWPSBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work_sync(&pLed->blink_work);
 			pLed->bLedWPSBlinkInProgress = false;
 		}
 		pLed->bLedNoLinkBlinkInProgress = true;
@@ -356,29 +344,29 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 			pLed->BlinkingLedState = RTW_LED_OFF;
 		else
 			pLed->BlinkingLedState = RTW_LED_ON;
-		_set_timer(&pLed->BlinkTimer, LED_BLINK_NO_LINK_INTERVAL_ALPHA);
+		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;
 		if (pLed->bLedNoLinkBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work_sync(&pLed->blink_work);
 			pLed->bLedNoLinkBlinkInProgress = false;
 		}
 		if (pLed->bLedLinkBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work_sync(&pLed->blink_work);
 			pLed->bLedLinkBlinkInProgress = false;
 		}
 		if (pLed->bLedBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work_sync(&pLed->blink_work);
 			pLed->bLedBlinkInProgress = false;
 		}
 		if (pLed->bLedWPSBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work_sync(&pLed->blink_work);
 			pLed->bLedWPSBlinkInProgress = false;
 		}
 		if (pLed->bLedScanBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work_sync(&pLed->blink_work);
 			pLed->bLedScanBlinkInProgress = false;
 		}
 		SwLedOff(padapter, pLed);
diff --git a/drivers/staging/r8188eu/include/rtw_led.h b/drivers/staging/r8188eu/include/rtw_led.h
index c035fe267635..7e901aae92fb 100644
--- a/drivers/staging/r8188eu/include/rtw_led.h
+++ b/drivers/staging/r8188eu/include/rtw_led.h
@@ -13,11 +13,11 @@
 #define LED_BLINK_SLOWLY_INTERVAL		200
 #define LED_BLINK_LONG_INTERVAL			400
 
-#define LED_BLINK_NO_LINK_INTERVAL_ALPHA	1000
-#define LED_BLINK_LINK_INTERVAL_ALPHA		500	/* 500 */
-#define LED_BLINK_SCAN_INTERVAL_ALPHA		180	/* 150 */
-#define LED_BLINK_FASTER_INTERVAL_ALPHA		50
-#define LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA	5000
+#define LED_BLINK_NO_LINK_INTVL			msecs_to_jiffies(1000)
+#define LED_BLINK_LINK_INTVL			msecs_to_jiffies(500)
+#define LED_BLINK_SCAN_INTVL			msecs_to_jiffies(180)
+#define LED_BLINK_FASTER_INTVL			msecs_to_jiffies(50)
+#define LED_BLINK_WPS_SUCESS_INTVL		msecs_to_jiffies(5000)
 
 #define LED_BLINK_NORMAL_INTERVAL_NETTRONIX	100
 #define LED_BLINK_SLOWLY_INTERVAL_NETTRONIX	2000
@@ -105,15 +105,12 @@ struct LED_871x {
 
 	u32 BlinkTimes; /*  Number of times to toggle led state for blinking. */
 
-	struct timer_list BlinkTimer; /*  Timer object for led blinking. */
-
 	/*  ALPHA, added by chiyoko, 20090106 */
 	u8 bLedNoLinkBlinkInProgress;
 	u8 bLedLinkBlinkInProgress;
 	u8 bLedStartToLinkBlinkInProgress;
 	u8 bLedScanBlinkInProgress;
-	struct work_struct BlinkWorkItem; /* Workitem used by BlinkTimer to
-					   * manipulate H/W to blink LED. */
+	struct delayed_work blink_work;
 };
 
 #define IS_LED_WPS_BLINKING(_LED_871x)					\
@@ -143,7 +140,6 @@ struct led_priv{
 			(adapt)->ledpriv.LedControlHandler((adapt), (action)); \
 	} while (0)
 
-void BlinkTimerCallback(struct timer_list *t);
 void BlinkWorkItemCallback(struct work_struct *work);
 
 void ResetLedStatus(struct LED_871x * pLed);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2] staging: r8188eu: use a delayed worker for led updates
  2021-11-25 16:25 [PATCH 0/4] staging: r8188eu: clean up workers Martin Kaiser
                   ` (3 preceding siblings ...)
  2021-11-25 16:25 ` [PATCH 4/4] staging: r8188eu: use a delayed worker for led updates Martin Kaiser
@ 2021-11-26 11:41 ` Martin Kaiser
  2021-11-27  7:42   ` Michael Straube
  2021-11-29 11:04   ` Dan Carpenter
  4 siblings, 2 replies; 9+ messages in thread
From: Martin Kaiser @ 2021-11-26 11:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
	linux-kernel, Martin Kaiser

The led layer uses a combination of timer and worker for periodic led
updates, e.g. for blinking. The reason seems to be that blocking
operations like a usb read are not allowed in a timer handler.

Replace the combination of timer and worker with a delayed worker.

Convert the timeout defines from milliseconds to jiffies to make them
usable as delays for the delayed worker. Shorten the names of the defines
and rename the work item to make checkpatch happy.

Other layers may call SwLedControlMode1 to update the led state. Such
an update may result in cancelling the delayed worker. SwLedControlMode1
might be called in interrupt context, we must use cancel_delayed_work to
cancel the worker. cancel_delayed_work_sync waits until the worker is
finished, this is not allowed in interrupt context.

DeInitLed871x is called when the driver is removed or when the system
goes into standby. We may use cancel_delayed_work_sync here to cancel
the delayed worker.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v2:
 - do not block if we cancel a delayed worker in interrupt context

 drivers/staging/r8188eu/core/rtw_led.c    | 106 ++++++++++------------
 drivers/staging/r8188eu/include/rtw_led.h |  16 ++--
 2 files changed, 53 insertions(+), 69 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
index ae46fd48f940..1eda366c61aa 100644
--- a/drivers/staging/r8188eu/core/rtw_led.c
+++ b/drivers/staging/r8188eu/core/rtw_led.c
@@ -4,20 +4,10 @@
 #include "../include/drv_types.h"
 #include "../include/rtw_led.h"
 
-void BlinkTimerCallback(struct timer_list *t)
-{
-	struct LED_871x *pLed = from_timer(pLed, t, BlinkTimer);
-	struct adapter *padapter = pLed->padapter;
-
-	if ((padapter->bSurpriseRemoved) || (padapter->bDriverStopped))
-		return;
-
-	schedule_work(&pLed->BlinkWorkItem);
-}
-
 void BlinkWorkItemCallback(struct work_struct *work)
 {
-	struct LED_871x *pLed = container_of(work, struct LED_871x, BlinkWorkItem);
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct LED_871x *pLed = container_of(dwork, struct LED_871x, blink_work);
 	BlinkHandler(pLed);
 }
 
@@ -45,14 +35,12 @@ void InitLed871x(struct adapter *padapter, struct LED_871x *pLed, enum LED_PIN_8
 
 	ResetLedStatus(pLed);
 
-	timer_setup(&pLed->BlinkTimer, BlinkTimerCallback, 0);
-	INIT_WORK(&pLed->BlinkWorkItem, BlinkWorkItemCallback);
+	INIT_DELAYED_WORK(&pLed->blink_work, BlinkWorkItemCallback);
 }
 
 void DeInitLed871x(struct LED_871x *pLed)
 {
-	cancel_work_sync(&pLed->BlinkWorkItem);
-	_cancel_timer_ex(&pLed->BlinkTimer);
+	cancel_delayed_work_sync(&pLed->blink_work);
 	ResetLedStatus(pLed);
 }
 
@@ -80,14 +68,14 @@ static void SwLedBlink1(struct LED_871x *pLed)
 			pLed->BlinkingLedState = RTW_LED_OFF;
 		else
 			pLed->BlinkingLedState = RTW_LED_ON;
-		_set_timer(&pLed->BlinkTimer, LED_BLINK_NO_LINK_INTERVAL_ALPHA);
+		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;
-		_set_timer(&pLed->BlinkTimer, LED_BLINK_LINK_INTERVAL_ALPHA);
+		schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
 		break;
 	case LED_BLINK_SCAN:
 		pLed->BlinkTimes--;
@@ -101,7 +89,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 					pLed->BlinkingLedState = RTW_LED_OFF;
 				else
 					pLed->BlinkingLedState = RTW_LED_ON;
-				_set_timer(&pLed->BlinkTimer, LED_BLINK_LINK_INTERVAL_ALPHA);
+				schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
 			} else if (!check_fwstate(pmlmepriv, _FW_LINKED)) {
 				pLed->bLedNoLinkBlinkInProgress = true;
 				pLed->CurrLedState = LED_BLINK_SLOWLY;
@@ -109,7 +97,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 					pLed->BlinkingLedState = RTW_LED_OFF;
 				else
 					pLed->BlinkingLedState = RTW_LED_ON;
-				_set_timer(&pLed->BlinkTimer, LED_BLINK_NO_LINK_INTERVAL_ALPHA);
+				schedule_delayed_work(&pLed->blink_work, LED_BLINK_NO_LINK_INTVL);
 			}
 			pLed->bLedScanBlinkInProgress = false;
 		} else {
@@ -117,7 +105,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_SCAN_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_SCAN_INTVL);
 		}
 		break;
 	case LED_BLINK_TXRX:
@@ -132,7 +120,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 					pLed->BlinkingLedState = RTW_LED_OFF;
 				else
 					pLed->BlinkingLedState = RTW_LED_ON;
-				_set_timer(&pLed->BlinkTimer, LED_BLINK_LINK_INTERVAL_ALPHA);
+				schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
 			} else if (!check_fwstate(pmlmepriv, _FW_LINKED)) {
 				pLed->bLedNoLinkBlinkInProgress = true;
 				pLed->CurrLedState = LED_BLINK_SLOWLY;
@@ -140,7 +128,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 					pLed->BlinkingLedState = RTW_LED_OFF;
 				else
 					pLed->BlinkingLedState = RTW_LED_ON;
-				_set_timer(&pLed->BlinkTimer, LED_BLINK_NO_LINK_INTERVAL_ALPHA);
+				schedule_delayed_work(&pLed->blink_work, LED_BLINK_NO_LINK_INTVL);
 			}
 			pLed->bLedBlinkInProgress = false;
 		} else {
@@ -148,7 +136,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_FASTER_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_FASTER_INTVL);
 		}
 		break;
 	case LED_BLINK_WPS:
@@ -156,7 +144,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 			pLed->BlinkingLedState = RTW_LED_OFF;
 		else
 			pLed->BlinkingLedState = RTW_LED_ON;
-		_set_timer(&pLed->BlinkTimer, LED_BLINK_SCAN_INTERVAL_ALPHA);
+		schedule_delayed_work(&pLed->blink_work, LED_BLINK_SCAN_INTVL);
 		break;
 	case LED_BLINK_WPS_STOP:	/* WPS success */
 		if (pLed->BlinkingLedState == RTW_LED_ON)
@@ -171,12 +159,12 @@ static void SwLedBlink1(struct LED_871x *pLed)
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_LINK_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
 
 			pLed->bLedWPSBlinkInProgress = false;
 		} else {
 			pLed->BlinkingLedState = RTW_LED_OFF;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_WPS_SUCESS_INTVL);
 		}
 		break;
 	default:
@@ -198,11 +186,11 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 			if (pLed->CurrLedState == LED_BLINK_SCAN || IS_LED_WPS_BLINKING(pLed))
 				return;
 			if (pLed->bLedLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedBlinkInProgress = false;
 			}
 
@@ -212,7 +200,7 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_NO_LINK_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_NO_LINK_INTVL);
 		}
 		break;
 	case LED_CTL_LINK:
@@ -220,11 +208,11 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 			if (pLed->CurrLedState == LED_BLINK_SCAN || IS_LED_WPS_BLINKING(pLed))
 				return;
 			if (pLed->bLedNoLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedNoLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedBlinkInProgress = false;
 			}
 			pLed->bLedLinkBlinkInProgress = true;
@@ -233,7 +221,7 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_LINK_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_LINK_INTVL);
 		}
 		break;
 	case LED_CTL_SITE_SURVEY:
@@ -243,15 +231,15 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 			if (IS_LED_WPS_BLINKING(pLed))
 				return;
 			if (pLed->bLedNoLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedNoLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedBlinkInProgress = false;
 			}
 			pLed->bLedScanBlinkInProgress = true;
@@ -261,7 +249,7 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_SCAN_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_SCAN_INTVL);
 		 }
 		break;
 	case LED_CTL_TX:
@@ -270,11 +258,11 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 			if (pLed->CurrLedState == LED_BLINK_SCAN || IS_LED_WPS_BLINKING(pLed))
 				return;
 			if (pLed->bLedNoLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedNoLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedLinkBlinkInProgress = false;
 			}
 			pLed->bLedBlinkInProgress = true;
@@ -284,26 +272,26 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_FASTER_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_FASTER_INTVL);
 		}
 		break;
 	case LED_CTL_START_WPS: /* wait until xinpin finish */
 	case LED_CTL_START_WPS_BOTTON:
 		 if (!pLed->bLedWPSBlinkInProgress) {
 			if (pLed->bLedNoLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedNoLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedLinkBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedBlinkInProgress = false;
 			}
 			if (pLed->bLedScanBlinkInProgress) {
-				_cancel_timer_ex(&pLed->BlinkTimer);
+				cancel_delayed_work(&pLed->blink_work);
 				pLed->bLedScanBlinkInProgress = false;
 			}
 			pLed->bLedWPSBlinkInProgress = true;
@@ -312,42 +300,42 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 				pLed->BlinkingLedState = RTW_LED_OFF;
 			else
 				pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_SCAN_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_SCAN_INTVL);
 		 }
 		break;
 	case LED_CTL_STOP_WPS:
 		if (pLed->bLedNoLinkBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work(&pLed->blink_work);
 			pLed->bLedNoLinkBlinkInProgress = false;
 		}
 		if (pLed->bLedLinkBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work(&pLed->blink_work);
 			pLed->bLedLinkBlinkInProgress = false;
 		}
 		if (pLed->bLedBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work(&pLed->blink_work);
 			pLed->bLedBlinkInProgress = false;
 		}
 		if (pLed->bLedScanBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work(&pLed->blink_work);
 			pLed->bLedScanBlinkInProgress = false;
 		}
 		if (pLed->bLedWPSBlinkInProgress)
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work(&pLed->blink_work);
 		else
 			pLed->bLedWPSBlinkInProgress = true;
 		pLed->CurrLedState = LED_BLINK_WPS_STOP;
 		if (pLed->bLedOn) {
 			pLed->BlinkingLedState = RTW_LED_OFF;
-			_set_timer(&pLed->BlinkTimer, LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA);
+			schedule_delayed_work(&pLed->blink_work, LED_BLINK_WPS_SUCESS_INTVL);
 		} else {
 			pLed->BlinkingLedState = RTW_LED_ON;
-			_set_timer(&pLed->BlinkTimer, 0);
+			schedule_delayed_work(&pLed->blink_work, 0);
 		}
 		break;
 	case LED_CTL_STOP_WPS_FAIL:
 		if (pLed->bLedWPSBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work(&pLed->blink_work);
 			pLed->bLedWPSBlinkInProgress = false;
 		}
 		pLed->bLedNoLinkBlinkInProgress = true;
@@ -356,29 +344,29 @@ static void SwLedControlMode1(struct adapter *padapter, enum LED_CTL_MODE LedAct
 			pLed->BlinkingLedState = RTW_LED_OFF;
 		else
 			pLed->BlinkingLedState = RTW_LED_ON;
-		_set_timer(&pLed->BlinkTimer, LED_BLINK_NO_LINK_INTERVAL_ALPHA);
+		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;
 		if (pLed->bLedNoLinkBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work(&pLed->blink_work);
 			pLed->bLedNoLinkBlinkInProgress = false;
 		}
 		if (pLed->bLedLinkBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work(&pLed->blink_work);
 			pLed->bLedLinkBlinkInProgress = false;
 		}
 		if (pLed->bLedBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work(&pLed->blink_work);
 			pLed->bLedBlinkInProgress = false;
 		}
 		if (pLed->bLedWPSBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work(&pLed->blink_work);
 			pLed->bLedWPSBlinkInProgress = false;
 		}
 		if (pLed->bLedScanBlinkInProgress) {
-			_cancel_timer_ex(&pLed->BlinkTimer);
+			cancel_delayed_work(&pLed->blink_work);
 			pLed->bLedScanBlinkInProgress = false;
 		}
 		SwLedOff(padapter, pLed);
diff --git a/drivers/staging/r8188eu/include/rtw_led.h b/drivers/staging/r8188eu/include/rtw_led.h
index c035fe267635..7e901aae92fb 100644
--- a/drivers/staging/r8188eu/include/rtw_led.h
+++ b/drivers/staging/r8188eu/include/rtw_led.h
@@ -13,11 +13,11 @@
 #define LED_BLINK_SLOWLY_INTERVAL		200
 #define LED_BLINK_LONG_INTERVAL			400
 
-#define LED_BLINK_NO_LINK_INTERVAL_ALPHA	1000
-#define LED_BLINK_LINK_INTERVAL_ALPHA		500	/* 500 */
-#define LED_BLINK_SCAN_INTERVAL_ALPHA		180	/* 150 */
-#define LED_BLINK_FASTER_INTERVAL_ALPHA		50
-#define LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA	5000
+#define LED_BLINK_NO_LINK_INTVL			msecs_to_jiffies(1000)
+#define LED_BLINK_LINK_INTVL			msecs_to_jiffies(500)
+#define LED_BLINK_SCAN_INTVL			msecs_to_jiffies(180)
+#define LED_BLINK_FASTER_INTVL			msecs_to_jiffies(50)
+#define LED_BLINK_WPS_SUCESS_INTVL		msecs_to_jiffies(5000)
 
 #define LED_BLINK_NORMAL_INTERVAL_NETTRONIX	100
 #define LED_BLINK_SLOWLY_INTERVAL_NETTRONIX	2000
@@ -105,15 +105,12 @@ struct LED_871x {
 
 	u32 BlinkTimes; /*  Number of times to toggle led state for blinking. */
 
-	struct timer_list BlinkTimer; /*  Timer object for led blinking. */
-
 	/*  ALPHA, added by chiyoko, 20090106 */
 	u8 bLedNoLinkBlinkInProgress;
 	u8 bLedLinkBlinkInProgress;
 	u8 bLedStartToLinkBlinkInProgress;
 	u8 bLedScanBlinkInProgress;
-	struct work_struct BlinkWorkItem; /* Workitem used by BlinkTimer to
-					   * manipulate H/W to blink LED. */
+	struct delayed_work blink_work;
 };
 
 #define IS_LED_WPS_BLINKING(_LED_871x)					\
@@ -143,7 +140,6 @@ struct led_priv{
 			(adapt)->ledpriv.LedControlHandler((adapt), (action)); \
 	} while (0)
 
-void BlinkTimerCallback(struct timer_list *t);
 void BlinkWorkItemCallback(struct work_struct *work);
 
 void ResetLedStatus(struct LED_871x * pLed);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] staging: r8188eu: use a delayed worker for led updates
  2021-11-26 11:41 ` [PATCH v2] " Martin Kaiser
@ 2021-11-27  7:42   ` Michael Straube
  2021-11-29 11:04   ` Dan Carpenter
  1 sibling, 0 replies; 9+ messages in thread
From: Michael Straube @ 2021-11-27  7:42 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

On 11/26/21 12:41, Martin Kaiser wrote:
> The led layer uses a combination of timer and worker for periodic led
> updates, e.g. for blinking. The reason seems to be that blocking
> operations like a usb read are not allowed in a timer handler.
> 
> Replace the combination of timer and worker with a delayed worker.
> 
> Convert the timeout defines from milliseconds to jiffies to make them
> usable as delays for the delayed worker. Shorten the names of the defines
> and rename the work item to make checkpatch happy.
> 
> Other layers may call SwLedControlMode1 to update the led state. Such
> an update may result in cancelling the delayed worker. SwLedControlMode1
> might be called in interrupt context, we must use cancel_delayed_work to
> cancel the worker. cancel_delayed_work_sync waits until the worker is
> finished, this is not allowed in interrupt context.
> 
> DeInitLed871x is called when the driver is removed or when the system
> goes into standby. We may use cancel_delayed_work_sync here to cancel
> the delayed worker.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> v2:
>   - do not block if we cancel a delayed worker in interrupt context
> 

Hi Martin,

v2 works for me without any sleeping/scheduling while atomic bugs.

Thanks,
Michael

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] staging: r8188eu: use a delayed worker for led updates
  2021-11-26 11:41 ` [PATCH v2] " Martin Kaiser
  2021-11-27  7:42   ` Michael Straube
@ 2021-11-29 11:04   ` Dan Carpenter
  2021-12-03 14:14     ` Greg Kroah-Hartman
  1 sibling, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2021-11-29 11:04 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Phillip Potter,
	Michael Straube, linux-staging, linux-kernel

This was confusing becuase it should have been [PATCH 4/4 v2].  These
days I think the prefered way is to just resend the whole series as a
new thread.

Greg doesn't use patchwork, but these rules especially apply for
subsystems which use patchwork.  People say that patchwork gets confused
when people use the --in-reply-to option and I guess it's hard to
apply individual patches in patchwork?  Anyway, just always start a new
thread and resend everything.

Send a reply to the original thread to say "Don't apply this one, it has
sleeping in atomic bugs", otherwise it might get applied by mistake.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] staging: r8188eu: use a delayed worker for led updates
  2021-11-29 11:04   ` Dan Carpenter
@ 2021-12-03 14:14     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2021-12-03 14:14 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Martin Kaiser, Larry Finger, Phillip Potter, Michael Straube,
	linux-staging, linux-kernel

On Mon, Nov 29, 2021 at 02:04:27PM +0300, Dan Carpenter wrote:
> This was confusing becuase it should have been [PATCH 4/4 v2].  These
> days I think the prefered way is to just resend the whole series as a
> new thread.
> 
> Greg doesn't use patchwork, but these rules especially apply for
> subsystems which use patchwork.  People say that patchwork gets confused
> when people use the --in-reply-to option and I guess it's hard to
> apply individual patches in patchwork?  Anyway, just always start a new
> thread and resend everything.
> 
> Send a reply to the original thread to say "Don't apply this one, it has
> sleeping in atomic bugs", otherwise it might get applied by mistake.

I had already reverted that patch from my tree, so I would not have
applied it again :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-12-03 14:14 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-25 16:25 [PATCH 0/4] staging: r8188eu: clean up workers Martin Kaiser
2021-11-25 16:25 ` [PATCH 1/4] staging: r8188eu: remove the _set_workitem wrapper Martin Kaiser
2021-11-25 16:25 ` [PATCH 2/4] staging: r8188eu: remove the _init_workitem wrapper Martin Kaiser
2021-11-25 16:25 ` [PATCH 3/4] staging: r8188eu: remove the _cancel_workitem_sync wrapper Martin Kaiser
2021-11-25 16:25 ` [PATCH 4/4] staging: r8188eu: use a delayed worker for led updates Martin Kaiser
2021-11-26 11:41 ` [PATCH v2] " Martin Kaiser
2021-11-27  7:42   ` Michael Straube
2021-11-29 11:04   ` Dan Carpenter
2021-12-03 14:14     ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).