Re: [PATCH v6 1/3] scsi: ufs: add ioctl interface for query request

2015-03-12 Thread Akinobu Mita 
2015-03-12 21:27 GMT+09:00 Gilad Broner gbro...@codeaurora.org:
 From: Dolev Raviv dra...@codeaurora.org

 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 dra...@codeaurora.org
 Signed-off-by: Noa Rubens n...@codeaurora.org
 Signed-off-by: Raviv Shvili rshv...@codeaurora.org
 Signed-off-by: Yaniv Gardi yga...@codeaurora.org

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)

 +   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,

...

 diff --git a/include/uapi/scsi/ufs/ioctl.h b/include/uapi/scsi/ufs/ioctl.h
 new file mode 100644
 index 000..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_QUERY0x5388

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?
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v6 1/3] scsi: ufs: add ioctl interface for query request

2015-03-12 Thread Gilad Broner 
From: Dolev Raviv dra...@codeaurora.org

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 dra...@codeaurora.org
Signed-off-by: Noa Rubens n...@codeaurora.org
Signed-off-by: Raviv Shvili rshv...@codeaurora.org
Signed-off-by: Yaniv Gardi yga...@codeaurora.org
---
 drivers/scsi/ufs/ufs.h|  53 +++---
 drivers/scsi/ufs/ufshcd.c | 225 +-
 include/uapi/scsi/Kbuild  |   1 +
 include/uapi/scsi/ufs/Kbuild  |   3 +
 include/uapi/scsi/ufs/ioctl.h |  57 +++
 include/uapi/scsi/ufs/ufs.h   |  66 +
 6 files changed, 361 insertions(+), 44 deletions(-)
 create mode 100644 include/uapi/scsi/ufs/Kbuild
 create mode 100644 include/uapi/scsi/ufs/ioctl.h
 create mode 100644 include/uapi/scsi/ufs/ufs.h

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 42c459a..1f023c4 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -38,6 +38,7 @@
 
 #include linux/mutex.h
 #include linux/types.h
+#include scsi/ufs/ufs.h
 
 #define MAX_CDB_SIZE   16
 #define GENERAL_UPIU_REQUEST_SIZE 32
@@ -71,6 +72,16 @@ enum {
UFS_UPIU_RPMB_WLUN  = 0xC4,
 };
 
+/**
+ * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit descriptor
+ * @lun: LU number to check
+ * @return: true if the lun has a matching unit descriptor, false otherwise
+ */
+static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
+{
+   return (lun == UFS_UPIU_RPMB_WLUN || (lun  UFS_UPIU_MAX_GENERAL_LUN));
+}
+
 /*
  * UFS Protocol Information Unit related definitions
  */
@@ -126,35 +137,6 @@ enum {
UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST  = 0x81,
 };
 
-/* Flag idn for Query Requests*/
-enum flag_idn {
-   QUERY_FLAG_IDN_FDEVICEINIT  = 0x01,
-   QUERY_FLAG_IDN_PWR_ON_WPE   = 0x03,
-   QUERY_FLAG_IDN_BKOPS_EN = 0x04,
-};
-
-/* Attribute idn for Query requests */
-enum attr_idn {
-   QUERY_ATTR_IDN_ACTIVE_ICC_LVL   = 0x03,
-   QUERY_ATTR_IDN_BKOPS_STATUS = 0x05,
-   QUERY_ATTR_IDN_EE_CONTROL   = 0x0D,
-   QUERY_ATTR_IDN_EE_STATUS= 0x0E,
-};
-
-/* Descriptor idn for Query requests */
-enum desc_idn {
-   QUERY_DESC_IDN_DEVICE   = 0x0,
-   QUERY_DESC_IDN_CONFIGURAION = 0x1,
-   QUERY_DESC_IDN_UNIT = 0x2,
-   QUERY_DESC_IDN_RFU_0= 0x3,
-   QUERY_DESC_IDN_INTERCONNECT = 0x4,
-   QUERY_DESC_IDN_STRING   = 0x5,
-   QUERY_DESC_IDN_RFU_1= 0x6,
-   QUERY_DESC_IDN_GEOMETRY = 0x7,
-   QUERY_DESC_IDN_POWER= 0x8,
-   QUERY_DESC_IDN_MAX,
-};
-
 enum desc_header_offset {
QUERY_DESC_LENGTH_OFFSET= 0x00,
QUERY_DESC_DESC_TYPE_OFFSET = 0x01,
@@ -247,19 +229,6 @@ enum bkops_status {
BKOPS_STATUS_MAX = BKOPS_STATUS_CRITICAL,
 };
 
-/* UTP QUERY Transaction Specific Fields OpCode */
-enum query_opcode {
-   UPIU_QUERY_OPCODE_NOP   = 0x0,
-   UPIU_QUERY_OPCODE_READ_DESC = 0x1,
-   UPIU_QUERY_OPCODE_WRITE_DESC= 0x2,
-   UPIU_QUERY_OPCODE_READ_ATTR = 0x3,
-   UPIU_QUERY_OPCODE_WRITE_ATTR= 0x4,
-   UPIU_QUERY_OPCODE_READ_FLAG = 0x5,
-   UPIU_QUERY_OPCODE_SET_FLAG  = 0x6,
-   UPIU_QUERY_OPCODE_CLEAR_FLAG= 0x7,
-   UPIU_QUERY_OPCODE_TOGGLE_FLAG   = 0x8,
-};
-
 /* Query response result code */
 enum {
QUERY_RESULT_SUCCESS= 0x00,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5d60a86..cb357f8 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3,7 +3,7 @@
  *
  * This code is based on drivers/scsi/ufs/ufshcd.c
  * Copyright (C) 2011-2013 Samsung India Software Operations
- * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
  *
  * Authors:
  * Santosh Yaraganavi santosh...@samsung.com
@@ -39,6 +39,7 @@
 
 #include linux/async.h
 #include linux/devfreq.h
+#include scsi/ufs/ioctl.h
 
 #include ufshcd.h
 #include unipro.h
@@ -74,6 +75,9 @@
 /* Interrupt aggregation default timeout, unit: 40us */
 #define INT_AGGR_DEF_TO0x02
 
+/* IOCTL opcode for command - ufs set device read only */
+#define UFS_IOCTL_BLKROSET  BLKROSET
+
 #define ufshcd_toggle_vreg(_dev, _vreg, _on)   \
({  \
int _ret;   \
@@ -1882,7 +1886,7 @@ static inline int ufshcd_read_unit_desc_param(struct 
ufs_hba *hba,
 * Unit descriptors are only available for general purpose LUs (LUN id
 * from 0 to 7) and RPMB Well known LU.
 */
-   if (lun != UFS_UPIU_RPMB_WLUN  (lun =