Messages in this thread |  | | From | "Eads, Gage" <> | Subject | RE: [PATCH 04/20] dlb2: add device ioctl layer and first 4 ioctls | Date | Tue, 4 Aug 2020 22:20:47 +0000 |
| |
> > +/* [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
|  |