Re: [patch net-next v2 02/15] ice: register devlink port for PF with ops

[Jiri Pirko <jiri@resnulli.us>]

Fri, May 26, 2023 at 12:52:21PM CEST, michal.wilczynski@intel.com wrote:
>
>
>On 5/26/2023 12:28 PM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Use newly introduce devlink port registration function variant and
>> register devlink port passing ops.
>>
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
>> ---
>>  drivers/net/ethernet/intel/ice/ice_devlink.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
>> index bc44cc220818..6661d12772a3 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
>> @@ -1512,6 +1512,9 @@ ice_devlink_set_port_split_options(struct ice_pf *pf,
>>  	ice_active_port_option = active_idx;
>>  }
>>  
>> +static const struct devlink_port_ops ice_devlink_port_ops = {
>> +};
>
>I can see that you're doing this everywhere, but aren't those braces redundant ?
>This struct would be initialized to zero anyway.

Well, yeah, but 3 patches later, they are not going to be empty anymore.


>
>> +
>>  /**
>>   * ice_devlink_create_pf_port - Create a devlink port for this PF
>>   * @pf: the PF to create a devlink port for
>> @@ -1551,7 +1554,8 @@ int ice_devlink_create_pf_port(struct ice_pf *pf)
>>  	devlink_port_attrs_set(devlink_port, &attrs);
>>  	devlink = priv_to_devlink(pf);
>>  
>> -	err = devlink_port_register(devlink, devlink_port, vsi->idx);
>> +	err = devlink_port_register_with_ops(devlink, devlink_port, vsi->idx,
>> +					     &ice_devlink_port_ops);
>>  	if (err) {
>>  		dev_err(dev, "Failed to create devlink port for PF %d, error %d\n",
>>  			pf->hw.pf_id, err);
>
>Looks good to me,
>
>Reviewed-by: Michal Wilczynski <michal.wilczynski@intel.com>
>
>