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 |
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?
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 --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))
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(-)