Re: [PATCH v5 net-next 06/12] net: ethernet: ti: introduce cpsw switchdev based driver part 1 - dual-emac

From: Andrew Lunn
Date: Fri Nov 01 2019 - 16:39:38 EST


> > > +static const struct devlink_ops cpsw_devlink_ops;
> >
> > It would be nice to avoid this forward declaration.
>
> It's not declaration, it's definition of devlink_ops without any standard callbacks implemented.

Ho Grygorii

Ah, yes.

How about

= {
};

to make it clearer?

> > > +static const struct devlink_param cpsw_devlink_params[] = {
> > > + DEVLINK_PARAM_DRIVER(CPSW_DL_PARAM_ALE_BYPASS,
> > > + "ale_bypass", DEVLINK_PARAM_TYPE_BOOL,
> > > + BIT(DEVLINK_PARAM_CMODE_RUNTIME),
> > > + cpsw_dl_ale_ctrl_get, cpsw_dl_ale_ctrl_set, NULL),
> > > +};
> >
> > Is this documented?
>
> In patch 9. But I'll update it and add standard devlink parameter definition, like:
>
> ale_bypass [DEVICE, DRIVER-SPECIFIC]
> Allows to enable ALE_CONTROL(4).BYPASS mode for debug purposes
> Type: bool
> Configuration mode: runtime

And please you the standard file naming and location,
Documentation/networking/devlink-params-foo.txt

Thanks
Andrew