Re: [PATCH net-next 3/4] mlx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 23/09/2025 18:44, Carolina Jubran wrote:
> 
> On 22/09/2025 19:51, Vadim Fedorenko wrote:
> Hi Vadim, thanks for the patch!
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/ 
>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> index 5e007bb3bad1..74a63371ab69 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> @@ -4755,9 +4755,11 @@ static int mlx5e_hwstamp_config_ptp_rx(struct 
>> mlx5e_priv *priv, bool ptp_rx)
>>                       &new_params.ptp_rx, true);
>>   }
>> -int mlx5e_hwstamp_set(struct mlx5e_priv *priv, struct ifreq *ifr)
>> +int mlx5e_hwstamp_set(struct net_device *dev,
>> +              struct kernel_hwtstamp_config *config,
>> +              struct netlink_ext_ack *extack)
>>   {
>> -    struct hwtstamp_config config;
>> +    struct mlx5e_priv *priv = netdev_priv(dev);
>>       bool rx_cqe_compress_def;
>>       bool ptp_rx;
>>       int err;
>> @@ -4766,11 +4768,8 @@ int mlx5e_hwstamp_set(struct mlx5e_priv *priv, 
>> struct ifreq *ifr)
>>           (mlx5_clock_get_ptp_index(priv->mdev) == -1))
> 
> 
> I would add an |extack| message here.

Yeah, but the !MLX5_CAP_GEN(priv->mdev, device_frequency_khz) check
looks redundant as mdev->clock->ptp will be null in case of absent of
device_frequency_khz, according to mlx5_init_clock()

> 
>> @@ -4814,47 +4813,34 @@ int mlx5e_hwstamp_set(struct mlx5e_priv *priv, 
>> struct ifreq *ifr)
>>       if (!mlx5e_profile_feature_cap(priv->profile, PTP_RX))
>>           err = mlx5e_hwstamp_config_no_ptp_rx(priv,
>> -                             config.rx_filter != HWTSTAMP_FILTER_NONE);
>> +                             config->rx_filter != HWTSTAMP_FILTER_NONE);
>>       else
>>           err = mlx5e_hwstamp_config_ptp_rx(priv, ptp_rx);
>>       if (err)
>>           goto err_unlock;
>> -    memcpy(&priv->tstamp, &config, sizeof(config));
>> +    memcpy(&priv->tstamp, config, sizeof(*config));
> 
> 
> A direct assignment would be cleaner.

Just wanted to follow original style. I'll change it in the next ver

> 
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c b/ 
>> drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
>> index 79ae3a51a4b3..ff8ffd997b17 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
>> @@ -52,7 +52,8 @@ static const struct net_device_ops mlx5i_netdev_ops = {
>>       .ndo_init                = mlx5i_dev_init,
>>       .ndo_uninit              = mlx5i_dev_cleanup,
>>       .ndo_change_mtu          = mlx5i_change_mtu,
>> -    .ndo_eth_ioctl            = mlx5i_ioctl,
>> +    .ndo_hwtstamp_get        = mlx5e_hwstamp_get,
>> +    .ndo_hwtstamp_set        = mlx5e_hwstamp_set,
>>   };
>>   /* IPoIB mlx5 netdev profile */
>> @@ -557,20 +558,6 @@ int mlx5i_dev_init(struct net_device *dev)
>>       return 0;
>>   }
>> -int mlx5i_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>> -{
>> -    struct mlx5e_priv *priv = mlx5i_epriv(dev);
>> -
> 
> 
> mlx5i_epriv should still be used here. on IPoIB netdev_priv gives you a
> struct mlx5i_priv .

Oh, I see... we have to have slightly different hwstamp functions for
mlx5i and mlx5e because of different netdev->priv type. Let me see how
it can be factorized.