RE: [PATCH 04/20] dlb2: add device ioctl layer and first 4 ioctls

[Eads, Gage]

> > +/* [7:0]: device revision, [15:8]: device version */
> > +#define DLB2_SET_DEVICE_VERSION(ver, rev) (((ver) << 8) | (rev))
> > +
> > +static int dlb2_ioctl_get_device_version(struct dlb2_dev *dev,
> > +                                        unsigned long user_arg,
> > +                                        u16 size)
> > +{
> > +       struct dlb2_get_device_version_args arg;
> > +       struct dlb2_cmd_response response;
> > +       int ret;
> > +
> > +       dev_dbg(dev->dlb2_device, "Entering %s()\n", __func__);
> > +
> > +       response.status = 0;
> > +       response.id = DLB2_SET_DEVICE_VERSION(2, DLB2_REV_A0);
> > +
> > +       ret = dlb2_copy_from_user(dev, user_arg, size, &arg, sizeof(arg));
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = dlb2_copy_resp_to_user(dev, arg.response, &response);
> 
> Better avoid any indirect pointers. As you always return a constant
> here, I think the entire ioctl command can be removed until you
> actually need it. If you have an ioctl command that needs both
> input and output, use _IOWR() to define it and put all arguments
> into the same structure.

I should've caught this in my earlier response, sorry. The device version
command is intentionally the first in the user interface enum. My
goal is for all device versions (e.g. DLB 1.0 in the future) to be accessible
through a /dev/dlb%d node. To allow this, all drivers would support the same
device-version command as command 0, then the subsequent commands can be
tailored to that particular device. User-space would query the version first
to determine which set of ioctl commands it needs to use.

So even though the response is constant (for now), it must occupy command 0 for
this design to work.

Thanks,
Gage