Re: [Intel-wired-lan] [PATCH iwl-next v1] ice: refactor the Tx scheduler feature

["Szapar-Mudlaw, Martyna" <martyna.szapar-mudlaw@linux.intel.com>]

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 <marcin.szycik@linux.intel.com>
>> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
>> Reviewed-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
> 
> ...
>