Re: [PATCH v6 1/3] scsi: ufs: add ioctl interface for query request
From: Akinobu Mita <hidden>
Date: 2015-03-12 13:30:42
Also in:
linux-arm-msm, linux-scsi, lkml
2015-03-12 21:27 GMT+09:00 Gilad Broner [off-list ref]:
From: Dolev Raviv <redacted> This patch exposes the ioctl interface for UFS driver via SCSI device ioctl interface. As of now UFS driver would provide the ioctl for query interface to connected UFS device. Signed-off-by: Dolev Raviv <redacted> Signed-off-by: Noa Rubens <redacted> Signed-off-by: Raviv Shvili <redacted> Signed-off-by: Yaniv Gardi <redacted>
Sorry to bother you again. Could you read two comments below?
+/**
+ * ufshcd_ioctl - ufs ioctl callback registered in scsi_host
+ * @dev: scsi device required for per LUN queries
+ * @cmd: command opcode
+ * @buffer: user space buffer for transferring data
+ *
+ * Supported commands:
+ * UFS_IOCTL_QUERY
+ */
+static int ufshcd_ioctl(struct scsi_device *dev, int cmd, void __user *buffer)
+{
+ struct ufs_hba *hba = shost_priv(dev->host);
+ int err = 0;
+
+ BUG_ON(!hba);
+ if (!buffer) {
+ dev_err(hba->dev, "%s: User buffer is NULL!\n", __func__);
+ return -EINVAL;
+ }
+Should we remove this check or move it into ufshcd_query_ioctl()? For example, BLKFLS ioctl without argument is correct usage, but it always triggers this message. (blkdev_ioctl -> __blkdev_driver_ioctl -> sd_ioctl -> scsi_ioctl -> ufshcd_ioctl)
quoted hunk ↗ jump to hunk
+ switch (cmd) { + case UFS_IOCTL_QUERY: + pm_runtime_get_sync(hba->dev); + err = ufshcd_query_ioctl(hba, ufshcd_scsi_to_upiu_lun(dev->lun), + buffer); + pm_runtime_put_sync(hba->dev); + break; + case UFS_IOCTL_BLKROSET: + err = -ENOIOCTLCMD; + break; + default: + err = -EINVAL; + dev_err(hba->dev, "%s: Illegal ufs-IOCTL cmd %d\n", __func__, + cmd); + break; + } + + return err; +} + static struct scsi_host_template ufshcd_driver_template = { .module = THIS_MODULE, .name = UFSHCD,@@ -4213,6 +4433,7 @@ static struct scsi_host_template ufshcd_driver_template = { .eh_abort_handler = ufshcd_abort, .eh_device_reset_handler = ufshcd_eh_device_reset_handler, .eh_host_reset_handler = ufshcd_eh_host_reset_handler, + .ioctl = ufshcd_ioctl, .this_id = -1, .sg_tablesize = SG_ALL, .cmd_per_lun = UFSHCD_CMD_PER_LUN,
...
quoted hunk ↗ jump to hunk
diff --git a/include/uapi/scsi/ufs/ioctl.h b/include/uapi/scsi/ufs/ioctl.h new file mode 100644 index 0000000..bc4eed7 --- /dev/null +++ b/include/uapi/scsi/ufs/ioctl.h@@ -0,0 +1,57 @@ +#ifndef UAPI_UFS_IOCTL_H_ +#define UAPI_UFS_IOCTL_H_ + +#include <linux/types.h> + +/* + * IOCTL opcode for ufs queries has the following opcode after + * SCSI_IOCTL_GET_PCI + */ +#define UFS_IOCTL_QUERY 0x5388
Should we also need some comments near SCSI_IOCTL_GET_PCI in include/scsi/scsi.h in order to avoid someone trying to define the same ioctl code in the future?