Re: [PATCH net-next 02/15] net/mlx5: Change parameters for PTP internal functions

[Tariq Toukan <ttoukan.linux@gmail.com>]

On 04/02/2025 10:51, Mateusz Polchlopek wrote:
> 
> 
> On 2/3/2025 10:35 PM, Tariq Toukan wrote:
>> From: Jianbo Liu <jianbol@nvidia.com>
>>
>> In later patch, the mlx5_clock will be allocated dynamically, its
>> address can be obtained from mlx5_core_dev struct, but mdev can't be
>> obtained from mlx5_clock because it can be shared by multiple
>> interfaces. So change the parameter for such internal functions, only
>> mdev is passed down from the callers.
>>
>> Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
>> Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
>> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
>> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
>> ---
>>   .../ethernet/mellanox/mlx5/core/lib/clock.c   | 19 ++++++++-----------
>>   1 file changed, 8 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c b/ 
>> drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
>> index eaf343756026..e7e4bdba02a3 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
>> @@ -878,10 +878,8 @@ static int mlx5_query_mtpps_pin_mode(struct 
>> mlx5_core_dev *mdev, u8 pin,
>>                       mtpps_size, MLX5_REG_MTPPS, 0, 0);
>>   }
>> -static int mlx5_get_pps_pin_mode(struct mlx5_clock *clock, u8 pin)
>> +static int mlx5_get_pps_pin_mode(struct mlx5_core_dev *mdev, u8 pin)
>>   {
>> -    struct mlx5_core_dev *mdev = container_of(clock, struct 
>> mlx5_core_dev, clock);
>> -
>>       u32 out[MLX5_ST_SZ_DW(mtpps_reg)] = {};
>>       u8 mode;
>>       int err;
>> @@ -900,8 +898,9 @@ static int mlx5_get_pps_pin_mode(struct mlx5_clock 
>> *clock, u8 pin)
>>       return PTP_PF_NONE;
>>   }
>> -static void mlx5_init_pin_config(struct mlx5_clock *clock)
>> +static void mlx5_init_pin_config(struct mlx5_core_dev *mdev)
>>   {
>> +    struct mlx5_clock *clock = &mdev->clock;
>>       int i;
>>       if (!clock->ptp_info.n_pins)
>> @@ -922,7 +921,7 @@ static void mlx5_init_pin_config(struct mlx5_clock 
>> *clock)
>>                sizeof(clock->ptp_info.pin_config[i].name),
>>                "mlx5_pps%d", i);
>>           clock->ptp_info.pin_config[i].index = i;
>> -        clock->ptp_info.pin_config[i].func = 
>> mlx5_get_pps_pin_mode(clock, i);
>> +        clock->ptp_info.pin_config[i].func = 
>> mlx5_get_pps_pin_mode(mdev, i);
>>           clock->ptp_info.pin_config[i].chan = 0;
>>       }
>>   }
>> @@ -1041,10 +1040,10 @@ static void mlx5_timecounter_init(struct 
>> mlx5_core_dev *mdev)
>>                ktime_to_ns(ktime_get_real()));
>>   }
>> -static void mlx5_init_overflow_period(struct mlx5_clock *clock)
>> +static void mlx5_init_overflow_period(struct mlx5_core_dev *mdev)
>>   {
>> -    struct mlx5_core_dev *mdev = container_of(clock, struct 
>> mlx5_core_dev, clock);
>>       struct mlx5_ib_clock_info *clock_info = mdev->clock_info;
>> +    struct mlx5_clock *clock = &mdev->clock;
>>       struct mlx5_timer *timer = &clock->timer;
> 
> It seems that because of the refactor the RCT rule has been violated.
> I think you have to split *timer into two lines.
> 

This is an existing line of code.
Due to dependency, clock had to come before.
I wouldn't split 'timer' just for this reason. Readability is not hurt.
Let's keep it as is.

>>       u64 overflow_cycles;
>>       u64 frac = 0;
>> @@ -1135,7 +1134,7 @@ static void mlx5_init_timer_clock(struct 
>> mlx5_core_dev *mdev)
>>       mlx5_timecounter_init(mdev);
>>       mlx5_init_clock_info(mdev);
>> -    mlx5_init_overflow_period(clock);
>> +    mlx5_init_overflow_period(mdev);
>>       if (mlx5_real_time_mode(mdev)) {
>>           struct timespec64 ts;
>> @@ -1147,13 +1146,11 @@ static void mlx5_init_timer_clock(struct 
>> mlx5_core_dev *mdev)
>>   static void mlx5_init_pps(struct mlx5_core_dev *mdev)
>>   {
>> -    struct mlx5_clock *clock = &mdev->clock;
>> -
>>       if (!MLX5_PPS_CAP(mdev))
>>           return;
>>       mlx5_get_pps_caps(mdev);
>> -    mlx5_init_pin_config(clock);
>> +    mlx5_init_pin_config(mdev);
>>   }
>>   void mlx5_init_clock(struct mlx5_core_dev *mdev)
> 
> Overall if you fix that RCT issue then feel free to add my RB tag,
> thanks.
> 

Thanks for your review.