Re: [patch net-next 01/15] devlink: introduce port ops placeholder

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 24 May 2023 14:18:22 +0200 Jiri Pirko wrote:
> @@ -6799,6 +6799,7 @@ EXPORT_SYMBOL_GPL(devlink_port_fini);
>   * @devlink: devlink
>   * @devlink_port: devlink port
>   * @port_index: driver-specific numerical identifier of the port
> + * @ops: port ops
>   *
>   * Register devlink port with provided port index. User can use
>   * any indexing, even hw-related one. devlink_port structure
> @@ -6806,9 +6807,10 @@ EXPORT_SYMBOL_GPL(devlink_port_fini);
>   * Note that the caller should take care of zeroing the devlink_port
>   * structure.
>   */
> -int devl_port_register(struct devlink *devlink,
> -		       struct devlink_port *devlink_port,
> -		       unsigned int port_index)
> +int devl_port_register_with_ops(struct devlink *devlink,
> +				struct devlink_port *devlink_port,
> +				unsigned int port_index,
> +				const struct devlink_port_ops *ops)
>  {
>  	int err;

function name in kdoc needs an update

> @@ -6819,6 +6821,7 @@ int devl_port_register(struct devlink *devlink,
>  	devlink_port_init(devlink, devlink_port);
>  	devlink_port->registered = true;
>  	devlink_port->index = port_index;
> +	devlink_port->ops = ops;
>  	spin_lock_init(&devlink_port->type_lock);
>  	INIT_LIST_HEAD(&devlink_port->reporter_list);
>  	err = xa_insert(&devlink->ports, port_index, devlink_port, GFP_KERNEL);
> @@ -6830,7 +6833,7 @@ int devl_port_register(struct devlink *devlink,
>  	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(devl_port_register);
> +EXPORT_SYMBOL_GPL(devl_port_register_with_ops);
>  
>  /**
>   *	devlink_port_register - Register devlink port
> @@ -6838,6 +6841,7 @@ EXPORT_SYMBOL_GPL(devl_port_register);
>   *	@devlink: devlink
>   *	@devlink_port: devlink port
>   *	@port_index: driver-specific numerical identifier of the port
> + *	@ops: port ops
>   *
>   *	Register devlink port with provided port index. User can use
>   *	any indexing, even hw-related one. devlink_port structure
> @@ -6847,18 +6851,20 @@ EXPORT_SYMBOL_GPL(devl_port_register);
>   *
>   *	Context: Takes and release devlink->lock <mutex>.
>   */
> -int devlink_port_register(struct devlink *devlink,
> -			  struct devlink_port *devlink_port,
> -			  unsigned int port_index)
> +int devlink_port_register_with_ops(struct devlink *devlink,
> +				   struct devlink_port *devlink_port,
> +				   unsigned int port_index,
> +				   const struct devlink_port_ops *ops)

same here.

BTW do we need to provide the "devlink_*" form of this API or can we
use this as an opportunity to move everyone to devl_*. Even if the
driver just wraps the call with devl_lock(), sooner or later people
will coalesce the locking in the drivers, I hope.
-- 
pw-bot: cr