On Wed, Jan 23, 2019 at 2:04 PM Jiri Pirko <j...@resnulli.us> wrote:
>
> Fri, Jan 18, 2019 at 08:09:38AM CET, vasundhara-v.vo...@broadcom.com wrote:
> >Add functions to register and unregister for the driver supported
> >configuration parameters table per port.
> >
> >v2->v3:
> >- Add a helper __devlink_params_register() with common code used by
> > both devlink_params_register() and devlink_port_params_register().
> >
>
> [...]
>
>
> >+
> >+static int __devlink_params_register(struct devlink *devlink,
> >+ struct list_head *param_list,
> >+ const struct devlink_param *params,
> >+ size_t params_count)
> > {
> > const struct devlink_param *param = params;
> > int i;
> >@@ -4490,20 +4493,11 @@ int devlink_params_register(struct devlink *devlink,
> >
> > mutex_lock(&devlink->lock);
> > for (i = 0; i < params_count; i++, param++) {
> >- if (!param || !param->name || !param->supported_cmodes) {
> >- err = -EINVAL;
> >+ err = devlink_param_verify(param);
> >+ if (err)
> > goto rollback;
> >- }
> >- if (param->generic) {
> >- err = devlink_param_generic_verify(param);
> >- if (err)
> >- goto rollback;
> >- } else {
> >- err = devlink_param_driver_verify(param);
> >- if (err)
> >- goto rollback;
> >- }
> >- err = devlink_param_register_one(devlink, param);
> >+
> >+ err = devlink_param_register_one(devlink, param_list, param);
> > if (err)
> > goto rollback;
> > }
> >@@ -4515,31 +4509,57 @@ int devlink_params_register(struct devlink *devlink,
> > if (!i)
> > goto unlock;
> > for (param--; i > 0; i--, param--)
> >- devlink_param_unregister_one(devlink, param);
> >+ devlink_param_unregister_one(devlink, param_list, param);
> > unlock:
> > mutex_unlock(&devlink->lock);
> > return err;
> > }
> >-EXPORT_SYMBOL_GPL(devlink_params_register);
> >
> > /**
> >- * devlink_params_unregister - unregister configuration parameters
> >+ * devlink_params_register - register configuration parameters
> >+ *
> > * @devlink: devlink
> >- * @params: configuration parameters to unregister
> >+ * @params: configuration parameters array
> > * @params_count: number of parameters provided
> >+ *
> >+ * Register the configuration parameters supported by the driver.
> > */
> >-void devlink_params_unregister(struct devlink *devlink,
> >- const struct devlink_param *params,
> >- size_t params_count)
> >+int devlink_params_register(struct devlink *devlink,
>
> The order of the functions should be:
> __devlink_params_register
> __devlink_params_unregister
> devlink_params_register
> devlink_params_unregister
> devlink_port_params_register
> devlink_port_params_unregister
>
> Your order is a bit confusing.
>
> Other than that:
> Acked-by: Jiri Pirko <j...@mellanox.com>
Thank you Jiri. I will update the order in the next version.
>
>
> [...]
