| Message ID | 20250203213516.227902-3-tariqt@nvidia.com (mailing list archive) |
|---|---|
| State | Needs ACK |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Support one PTP device per hardware clock | expand |
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. > 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.
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.
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; 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)