Willem de Bruijn [off-list ref] writes:
On Wed, Jan 27, 2021 at 5:23 AM Emil Renner Berthing [off-list ref] wrote:
quoted
In commit d3ccc14dfe95 most of the tasklets in this driver was
updated to the new API. However for the rx_work_tasklet only the
type of the callback was changed from
void _rtl_rx_work(unsigned long data)
to
void _rtl_rx_work(struct tasklet_struct *t).
The initialization of rx_work_tasklet was still open-coded and the
function pointer just cast into the old type, and hence nothing sets
rx_work_tasklet.use_callback = true and the callback was still called as
t->func(t->data);
with uninitialized/zero t->data.
Commit 6b8c7574a5f8 changed the casting of _rtl_rx_work a bit and
initialized t->data to a pointer to the tasklet cast to an unsigned
long.
This way calling t->func(t->data) might actually work through all the
casting, but it still doesn't update the code to use the new tasklet
API.
Let's use the new tasklet_setup to initialize rx_work_tasklet properly
and set rx_work_tasklet.use_callback = true so that the callback is
called as
t->callback(t);
without all the casting.
Fixes: 6b8c7574a5f8 ("rtlwifi: fix build warning")
Fixes: d3ccc14dfe95 ("rtlwifi/rtw88: convert tasklets to use new
tasklet_setup() API")
Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Since the current code works, this could target net-next
This should go to wireless-drivers-next, not net-next.
without Fixes tags.
Correct, no need for Fixes tag as there's no bug to fix. This is only
cleanup AFAICS.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Wed, 27 Jan 2021 at 16:20, Kalle Valo [off-list ref] wrote:
Willem de Bruijn [off-list ref] writes:
quoted
On Wed, Jan 27, 2021 at 5:23 AM Emil Renner Berthing [off-list ref] wrote:
quoted
In commit d3ccc14dfe95 most of the tasklets in this driver was
updated to the new API. However for the rx_work_tasklet only the
type of the callback was changed from
void _rtl_rx_work(unsigned long data)
to
void _rtl_rx_work(struct tasklet_struct *t).
The initialization of rx_work_tasklet was still open-coded and the
function pointer just cast into the old type, and hence nothing sets
rx_work_tasklet.use_callback = true and the callback was still called as
t->func(t->data);
with uninitialized/zero t->data.
Commit 6b8c7574a5f8 changed the casting of _rtl_rx_work a bit and
initialized t->data to a pointer to the tasklet cast to an unsigned
long.
This way calling t->func(t->data) might actually work through all the
casting, but it still doesn't update the code to use the new tasklet
API.
Let's use the new tasklet_setup to initialize rx_work_tasklet properly
and set rx_work_tasklet.use_callback = true so that the callback is
called as
t->callback(t);
without all the casting.
Fixes: 6b8c7574a5f8 ("rtlwifi: fix build warning")
Fixes: d3ccc14dfe95 ("rtlwifi/rtw88: convert tasklets to use new
tasklet_setup() API")
Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Since the current code works, this could target net-next
This should go to wireless-drivers-next, not net-next.
quoted
without Fixes tags.
Correct, no need for Fixes tag as there's no bug to fix. This is only
cleanup AFAICS.
I can definitely see how you can reasonably disagree, but I would not
be comfortable having code that only works because the calling
conventions of all relevant architectures happen to put the first
unsigned long argument and the first pointer argument in the same
register.
If you want to be conservative I'd much rather revert all the changes
around rx_work_tasklet including the new type of the _rtl_rx_work
callback, so we don't have to rely on calling conventions matching up.
In any case: as long as this fix eventually ends up upstream I'm fine.
/Emil