On Wed, Oct 30, 2024 at 11:49:49AM +0100, Oliver Hartkopp wrote:
On 28.10.24 08:29, Nam Cao wrote:
The hrtimer "thrtimer" is not used for TX. But this timer is initialized
regardless.
Remove the hrtimer_init() for the unused hrtimer and change bcm_remove_op()
to make sure hrtimer_cancel() is not called with the uninitialized hrtimer.
NAK.
There are several other occurrences of thrtimer that are not covered by
RX/TX distinction, where the second timer is canceled.
This one-time init and cancel of an unused hrtimer costs nearly nothing and
is not even in any hot path.
So this incomplete patch only adds complexity and potential error cases in
some 20 y/o code for nothing.
The "real" motivation is preparing to use hrtimer_setup() instead of
hrtimer_init() [1] and deleting hrtimer_init() [2]. The new function
mandates a callback function, and since the TX thrtimer doesn't have a
callback function, hrtimer_setup() cannot be used.
Your concerns are also valid. So I can drop this patch, and use a dummy
function to make hrtimer_setup() happy, like how it's done for the rt2x00
driver [3]. It will make the driver a bit ugly, but it's obvious that it
won't cause any regression.
Best regards,
Nam
[1] https://lore.kernel.org/lkml/e4ce3a3a28625d54ef93e47bfb02f7ffb741758a.1729865232.git.namcao@xxxxxxxxxxxxx/
[2] https://lore.kernel.org/lkml/7bde2762d82d30dab184c7a747e76afc41208da0.1729865740.git.namcao@xxxxxxxxxxxxx/
[3] https://lore.kernel.org/lkml/49f2bce487f56eb2a3ff572ea6d7de0a43560c0f.1729865232.git.namcao@xxxxxxxxxxxxx/