diff mbox series

[01/11] mt76: mt7615: fix MT_ANT_SWITCH_CON register definition

Message ID 20200908211756.15998-1-nbd@nbd.name
State New
Headers show
Series [01/11] mt76: mt7615: fix MT_ANT_SWITCH_CON register definition | expand

Commit Message

Felix Fietkau Sept. 8, 2020, 9:17 p.m. UTC
This is used for testmode tx antenna selection

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 drivers/net/wireless/mediatek/mt76/mt7615/regs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kalle Valo Sept. 11, 2020, 8:15 a.m. UTC | #1
Felix Fietkau <nbd@nbd.name> writes:

> In order to avoid keeping work like tx scheduling pinned to the CPU it was
> scheduled from, it makes sense to switch from tasklets to kernel threads.
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>

[...]

> --- a/drivers/net/wireless/mediatek/mt76/util.c
> +++ b/drivers/net/wireless/mediatek/mt76/util.c
> @@ -110,4 +110,32 @@ int mt76_get_min_avg_rssi(struct mt76_dev *dev, bool ext_phy)
>  }
>  EXPORT_SYMBOL_GPL(mt76_get_min_avg_rssi);
>  
> +int __mt76_worker_fn(void *ptr)
> +{
> +	struct mt76_worker *w = ptr;
> +
> +	while (!kthread_should_stop()) {
> +		set_current_state(TASK_INTERRUPTIBLE);
> +
> +		if (kthread_should_park()) {
> +			kthread_parkme();
> +			continue;
> +		}
> +
> +		if (!test_and_clear_bit(MT76_WORKER_SCHEDULED, &w->state)) {
> +			schedule();
> +			continue;
> +		}
> +
> +		set_bit(MT76_WORKER_RUNNING, &w->state);
> +		set_current_state(TASK_RUNNING);
> +		w->fn(w);
> +		cond_resched();
> +		clear_bit(MT76_WORKER_RUNNING, &w->state);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(__mt76_worker_fn);

So how is this better than, for example,
create_singlethread_workqueue()? And if this is better, shouldn't it be
part of workqueue.h instead of every driver reinventing the wheel?
Kalle Valo Sept. 14, 2020, 7:55 a.m. UTC | #2
Felix Fietkau <nbd@nbd.name> writes:

> On 2020-09-11 10:15, Kalle Valo wrote:
>> Felix Fietkau <nbd@nbd.name> writes:
>> 
>>> In order to avoid keeping work like tx scheduling pinned to the CPU it was
>>> scheduled from, it makes sense to switch from tasklets to kernel threads.
>>>
>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>> 
>> [...]
>> 
>>> --- a/drivers/net/wireless/mediatek/mt76/util.c
>>> +++ b/drivers/net/wireless/mediatek/mt76/util.c
>>> @@ -110,4 +110,32 @@ int mt76_get_min_avg_rssi(struct mt76_dev *dev, bool ext_phy)
>>>  }
>>>  EXPORT_SYMBOL_GPL(mt76_get_min_avg_rssi);
>>>  
>>> +int __mt76_worker_fn(void *ptr)
>>> +{
>>> +	struct mt76_worker *w = ptr;
>>> +
>>> +	while (!kthread_should_stop()) {
>>> +		set_current_state(TASK_INTERRUPTIBLE);
>>> +
>>> +		if (kthread_should_park()) {
>>> +			kthread_parkme();
>>> +			continue;
>>> +		}
>>> +
>>> +		if (!test_and_clear_bit(MT76_WORKER_SCHEDULED, &w->state)) {
>>> +			schedule();
>>> +			continue;
>>> +		}
>>> +
>>> +		set_bit(MT76_WORKER_RUNNING, &w->state);
>>> +		set_current_state(TASK_RUNNING);
>>> +		w->fn(w);
>>> +		cond_resched();
>>> +		clear_bit(MT76_WORKER_RUNNING, &w->state);
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(__mt76_worker_fn);
>> 
>> So how is this better than, for example,
>> create_singlethread_workqueue()? And if this is better, shouldn't it be
>> part of workqueue.h instead of every driver reinventing the wheel?
>
> Unlike a workqueue, this one only allows one fixed worker function to be
> executed by the worker thread. Because of that, there is less locking
> and less code for scheduling involved.
> In fact, the function that schedules the worker is small enough that
> it's just a simple inline function.
> The difference matters in this case, because the tx worker is scheduled
> very often in a hot path.
> I don't think it fits into workqueue.h (because of the lack of
> separation between workqueue and work struct), and I don't know if other
> drivers need this, so let's keep it in mt76 and only move to a generic
> API if we find another user for it.

Ok, fair enough. But please add this info to the commit log so the
reasoning is properly documented.
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/regs.h b/drivers/net/wireless/mediatek/mt76/mt7615/regs.h
index 9137d9e6b51d..61623f480806 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/regs.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/regs.h
@@ -575,7 +575,7 @@  enum mt7615_reg_base {
 #define MT_MCU_PTA_BASE			0x81060000
 #define MT_MCU_PTA(_n)			(MT_MCU_PTA_BASE + (_n))
 
-#define MT_ANT_SWITCH_CON(n)		MT_MCU_PTA(0x0c8)
+#define MT_ANT_SWITCH_CON(_n)		MT_MCU_PTA(0x0c8 + ((_n) - 1) * 4)
 #define MT_ANT_SWITCH_CON_MODE(_n)	(GENMASK(4, 0) << (_n * 8))
 #define MT_ANT_SWITCH_CON_MODE1(_n)	(GENMASK(3, 0) << (_n * 8))