On 3/3/2025 10:54 AM, Simon Horman wrote:
On Wed, Feb 26, 2025 at 12:33:56PM +0100, Mateusz Polchlopek wrote:
Embed ice_get_tx_topo_user_sel() inside the only caller:
ice_devlink_tx_sched_layers_get().
Instead of jump from the wrapper to the function that does "get" operation
it does "get" itself.
Remove unnecessary comment and make usage of str_enabled_disabled()
in ice_init_tx_topology().
Hi Mateusz,
These changes seem reasonable to me.
But I wonder if they could be motivated in the commit message.
What I mean is, the commit message explains what has been done.
But I think it should explain why it has been done.
Hi Simon,
I'm replying on behalf of Mateusz since he's on leave, and we didn't
want to keep this issue waiting too long.
Would such extended commit message make sense and address your concerns?
"Simplify the code by eliminating an unnecessary wrapper function.
Previously, ice_devlink_tx_sched_layers_get() acted as a thin wrapper
around ice_get_tx_topo_user_sel(), adding no real value but increasing
code complexity. Since both functions were only used once, the wrapper
was redundant and contributed approximately 20 lines of unnecessary
code. By directly calling ice_get_tx_topo_user_sel(), improve
readability and reduce function jumps, without altering functionality.
Also remove unnecessary comment and make usage of str_enabled_disabled()
in ice_init_tx_topology()."
Thank you,
Martyna
Suggested-by: Marcin Szycik <[email protected]>
Reviewed-by: Michal Swiatkowski <[email protected]>
Reviewed-by: Jedrzej Jagielski <[email protected]>
Reviewed-by: Przemek Kitszel <[email protected]>
Reviewed-by: Aleksandr Loktionov <[email protected]>
Signed-off-by: Mateusz Polchlopek <[email protected]>
...
