From: Sean Wang <[email protected]>
Use dev_info instead for the informative messages.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt7921/mac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
index eb1bfb682e02..2ce3a833176e 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
@@ -740,7 +740,7 @@ void mt7921_mac_reset_work(struct work_struct *work)
struct mt76_connac_pm *pm = &dev->pm;
int i;
- dev_err(dev->mt76.dev, "chip reset\n");
+ dev_info(dev->mt76.dev, "chip reset\n");
dev->hw_full_reset = true;
ieee80211_stop_queues(hw);
--
2.25.1
From: Sean Wang <[email protected]>
Reduce the mutex lock scope for reset to get rid of possible task hung
e.g wpa_supplicant and to allow the user-space process to keep running
during we need more retries to complete the reset.
Suggested-by: YN Chen <[email protected]>
Signed-off-by: Sean Wang <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt7921/mac.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
index 2ce3a833176e..c7bca83e7686 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
@@ -738,7 +738,7 @@ void mt7921_mac_reset_work(struct work_struct *work)
reset_work);
struct ieee80211_hw *hw = mt76_hw(dev);
struct mt76_connac_pm *pm = &dev->pm;
- int i;
+ int i, ret;
dev_info(dev->mt76.dev, "chip reset\n");
dev->hw_full_reset = true;
@@ -748,11 +748,14 @@ void mt7921_mac_reset_work(struct work_struct *work)
cancel_delayed_work_sync(&pm->ps_work);
cancel_work_sync(&pm->wake_work);
- mutex_lock(&dev->mt76.mutex);
- for (i = 0; i < 10; i++)
- if (!mt7921_dev_reset(dev))
+ for (i = 0; i < 10; i++) {
+ mutex_lock(&dev->mt76.mutex);
+ ret = mt7921_dev_reset(dev);
+ mutex_unlock(&dev->mt76.mutex);
+
+ if (!ret)
break;
- mutex_unlock(&dev->mt76.mutex);
+ }
if (i == 10)
dev_err(dev->mt76.dev, "chip reset failed\n");
--
2.25.1
Il 23/06/22 02:35, [email protected] ha scritto:
> From: Sean Wang <[email protected]>
>
> Use dev_info instead for the informative messages.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/net/wireless/mediatek/mt76/mt7921/mac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
> index eb1bfb682e02..2ce3a833176e 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
> @@ -740,7 +740,7 @@ void mt7921_mac_reset_work(struct work_struct *work)
> struct mt76_connac_pm *pm = &dev->pm;
> int i;
>
> - dev_err(dev->mt76.dev, "chip reset\n");
> + dev_info(dev->mt76.dev, "chip reset\n");
Since this function is normally expected to be called and this message is
describing the wanted flow and/or this worker function starting, I'd say
that this is not a really important information anyway...
What about changing that one to a dev_dbg() instead?
Regards,
Angelo
> dev->hw_full_reset = true;
> ieee80211_stop_queues(hw);
>
Il 23/06/22 02:35, [email protected] ha scritto:
> From: Sean Wang <[email protected]>
>
> Reduce the mutex lock scope for reset to get rid of possible task hung
> e.g wpa_supplicant and to allow the user-space process to keep running
> during we need more retries to complete the reset.
>
To actually, effectively, reduce the chance to get a hung task, and also
to improve the general responsiveness, I think that the best way would be
to manage the locking inside of the reset callback(s) for each dev/bus.
This is especially because some of these reset functions (like the SDIO one)
may end up waiting for more than *two seconds*.
However, I get that this proposal requires way more effort, and this commit
will anyway improve the situation as it is... so, you get my:
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
> Suggested-by: YN Chen <[email protected]>
> Signed-off-by: Sean Wang <[email protected]>
AngeloGioacchino Del Regno <[email protected]>
writes:
> Il 23/06/22 02:35, [email protected] ha scritto:
>> From: Sean Wang <[email protected]>
>>
>> Use dev_info instead for the informative messages.
>>
>> Signed-off-by: Sean Wang <[email protected]>
>> ---
>> drivers/net/wireless/mediatek/mt76/mt7921/mac.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
>> index eb1bfb682e02..2ce3a833176e 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
>> @@ -740,7 +740,7 @@ void mt7921_mac_reset_work(struct work_struct *work)
>> struct mt76_connac_pm *pm = &dev->pm;
>> int i;
>> - dev_err(dev->mt76.dev, "chip reset\n");
>> + dev_info(dev->mt76.dev, "chip reset\n");
>
> Since this function is normally expected to be called and this message is
> describing the wanted flow and/or this worker function starting, I'd say
> that this is not a really important information anyway...
>
> What about changing that one to a dev_dbg() instead?
Yeah, that would be better.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches