Re: [PATCH net-next 4/4] net/mlx5: Add sensor name to temperature event message

[Saeed Mahameed <saeed@kernel.org>]

On 19 Feb 07:28, Jakub Kicinski wrote:
>On Wed, 19 Feb 2025 15:00:57 +0200 Tariq Toukan wrote:
>> >> If you have to respin for some other reason, please consider limiting lines
>> >> to 80 columns wide or less here and elsewhere in this patch where it
>> >> doesn't reduce readability (subjective I know).
>> >
>> > +1, please try to catch such situations going forward
>>
>> This was not missed.
>> This is not a new thing...
>> We've been enforcing a max line length of 100 chars in mlx5 driver for
>> the past few years.
>> I don't have the full image now, but I'm convinced that this dates back
>> to an agreement between the mlx5 and netdev maintainers at that time.
>>
>> 80 chars could be too restrictive, especially with today's large
>> monitors, while 100-chars is still highly readable.
>> This is subjective of course...
>>
>> If you don't have a strong preference, we'll keep the current 100 chars
>> limit. Otherwise, just let me know and we'll start enforcing the
>> 80-chars limit for future patches.
>
>Right, I think mlx5 is the only exception to the 80 column guidance.
>I don't think it's resulting in more readable code, so yes, my
>preference is to end this experiment.
>

The reason in mlx5 was that we wanted to preserve the official HW spec
auto-generated fields names and they are really long.
100 chars worked very well with us for example the following sequence of
code setting up a FW command buffer would have to be broken in every line
if we were to restrict 80 chars per line.

  MLX5_SET(modify_vhca_state_in, in, opcode, MLX5_CMD_OP_MODIFY_VHCA_STATE);
  MLX5_SET(modify_vhca_state_in, in, vhca_state_field_select.sw_function_id, 1);
  MLX5_SET(modify_vhca_state_in, in, vhca_state_context.sw_function_id, sw_fn_id);
  MLX5_SET(modify_vhca_state_in, in, vhca_state_context.arm_change_event, 1);
  MLX5_SET(modify_vhca_state_in, in, vhca_state_field_select.arm_change_event, 1);

But I believe the driver grow larger than caring about those lines too
much, I just did a quick check and it seems less than 2% of the lines are
actually > 80, not sure this is due to being more strict in the past few
years or that we don't really need more than 80 lines.

I also check the interesting cases with macros such
MLX5_SET/MLX5_GET/MLX5_CAP and also the percentile of long lines was very
minor just about 5% in all cases.. 

So I kinda agree mlx5 doesn't need be so special anymore. 
Tariq up to you, you are the main  reviewer now.

Thanks
Saeed.