> Subject: Re: [RFC 04/20] RDMA/irdma: Add driver framework definitions
>
<...............>
> > +/**
> > + * i40iw_l2param_change - handle qs handles for QoS and MSS change
> > + * @ldev: LAN device information
> > + * @client: client for parameter change
> > + * @params: new parameters from L2
> > + */
> > +static void i40iw_l2param_change(struct i40e_info *ldev,
> > + struct i40e_client *client,
> > + struct i40e_params *params)
> > +{
> > + struct irdma_l2params *l2params;
> > + struct l2params_work *work;
> > + struct irdma_device *iwdev;
> > + struct irdma_handler *hdl;
> > + int i;
> > +
> > + hdl = irdma_find_handler(ldev->pcidev);
> > + if (!hdl)
> > + return;
> > +
> > + iwdev = (struct irdma_device *)((u8 *)hdl + sizeof(*hdl));
> > +
> > + if (atomic_read(&iwdev->params_busy))
> > + return;
> > + work = kzalloc(sizeof(*work), GFP_KERNEL);
> > + if (!work)
> > + return;
> > +
> > + atomic_inc(&iwdev->params_busy);
>
> Changing parameters through workqueue and perform locking with atomic_t,
> exciting.
> Please do proper locking scheme and better to avoid workqueue at all.
Hmmm....Yes, this is buggy. Will come up with a better solution.
>
> <...>
>
> > +/* client interface functions */
> > +static const struct i40e_client_ops i40e_ops = {
> > + .open = i40iw_open,
> > + .close = i40iw_close,
> > + .l2_param_change = i40iw_l2param_change };
> > +
> > +static struct i40e_client i40iw_client = {
> > + .name = "irdma",
> > + .ops = &i40e_ops,
> > + .version.major = I40E_CLIENT_VERSION_MAJOR,
> > + .version.minor = I40E_CLIENT_VERSION_MINOR,
> > + .version.build = I40E_CLIENT_VERSION_BUILD,
> > + .type = I40E_CLIENT_IWARP,
> > +};
> > +
> > +int i40iw_probe(struct platform_device *pdev) {
> > + struct i40e_peer_dev_platform_data *pdata =
> > + dev_get_platdata(&pdev->dev);
> > + struct i40e_info *ldev;
> > +
> > + if (!pdata)
> > + return -EINVAL;
> > +
> > + ldev = pdata->ldev;
> > +
> > + if (ldev->version.major != I40E_CLIENT_VERSION_MAJOR ||
> > + ldev->version.minor != I40E_CLIENT_VERSION_MINOR) {
> > + pr_err("version mismatch:\n");
> > + pr_err("expected major ver %d, caller specified major ver %d\n",
> > + I40E_CLIENT_VERSION_MAJOR, ldev->version.major);
> > + pr_err("expected minor ver %d, caller specified minor ver %d\n",
> > + I40E_CLIENT_VERSION_MINOR, ldev->version.minor);
> > + return -EINVAL;
> > + }
>
> This is can't be in upstream code, we don't support out-of-tree modules,
> everything else will have proper versions.
>
OK.
