[PATCH] qla4xxx: skip error recovery in case of register disconnect.

2018-02-11 Thread Manish Rangankar
A system crashes when continuously removing/re-adding
the storage controller.

Signed-off-by: Manish Rangankar 
---
 drivers/scsi/qla4xxx/ql4_def.h |  2 ++
 drivers/scsi/qla4xxx/ql4_os.c  | 46 ++
 2 files changed, 48 insertions(+)

diff --git a/drivers/scsi/qla4xxx/ql4_def.h b/drivers/scsi/qla4xxx/ql4_def.h
index fc23371..817f312 100644
--- a/drivers/scsi/qla4xxx/ql4_def.h
+++ b/drivers/scsi/qla4xxx/ql4_def.h
@@ -168,6 +168,8 @@
 #define DEV_DB_NON_PERSISTENT  0
 #define DEV_DB_PERSISTENT  1
 
+#define QL4_ISP_REG_DISCONNECT 0xU
+
 #define COPY_ISID(dst_isid, src_isid) {\
int i, j;   \
for (i = 0, j = ISID_SIZE - 1; i < ISID_SIZE;)  \
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 2b8a8ce..efaa7f1 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -262,6 +262,24 @@ static int qla4xxx_sysfs_ddb_logout(struct 
iscsi_bus_flash_session *fnode_sess,
 
 static struct scsi_transport_template *qla4xxx_scsi_transport;
 
+static int qla4xxx_isp_check_reg(struct scsi_qla_host *ha)
+{
+   u32 reg_val = 0;
+   int rval = QLA_SUCCESS;
+
+   if (is_qla8022(ha))
+   reg_val = readl(&ha->qla4_82xx_reg->host_status);
+   else if (is_qla8032(ha) || is_qla8042(ha))
+   reg_val = qla4_8xxx_rd_direct(ha, QLA8XXX_PEG_ALIVE_COUNTER);
+   else
+   reg_val = readw(&ha->reg->ctrl_status);
+
+   if (reg_val == QL4_ISP_REG_DISCONNECT)
+   rval = QLA_ERROR;
+
+   return rval;
+}
+
 static int qla4xxx_send_ping(struct Scsi_Host *shost, uint32_t iface_num,
 uint32_t iface_type, uint32_t payload_size,
 uint32_t pid, struct sockaddr *dst_addr)
@@ -9188,10 +9206,17 @@ static int qla4xxx_eh_abort(struct scsi_cmnd *cmd)
struct srb *srb = NULL;
int ret = SUCCESS;
int wait = 0;
+   int rval;
 
ql4_printk(KERN_INFO, ha, "scsi%ld:%d:%llu: Abort command issued 
cmd=%p, cdb=0x%x\n",
   ha->host_no, id, lun, cmd, cmd->cmnd[0]);
 
+   rval = qla4xxx_isp_check_reg(ha);
+   if (rval != QLA_SUCCESS) {
+   ql4_printk(KERN_INFO, ha, "PCI/Register disconnect, 
exiting.\n");
+   return FAILED;
+   }
+
spin_lock_irqsave(&ha->hardware_lock, flags);
srb = (struct srb *) CMD_SP(cmd);
if (!srb) {
@@ -9243,6 +9268,7 @@ static int qla4xxx_eh_device_reset(struct scsi_cmnd *cmd)
struct scsi_qla_host *ha = to_qla_host(cmd->device->host);
struct ddb_entry *ddb_entry = cmd->device->hostdata;
int ret = FAILED, stat;
+   int rval;
 
if (!ddb_entry)
return ret;
@@ -9262,6 +9288,12 @@ static int qla4xxx_eh_device_reset(struct scsi_cmnd *cmd)
  cmd, jiffies, cmd->request->timeout / HZ,
  ha->dpc_flags, cmd->result, cmd->allowed));
 
+   rval = qla4xxx_isp_check_reg(ha);
+   if (rval != QLA_SUCCESS) {
+   ql4_printk(KERN_INFO, ha, "PCI/Register disconnect, 
exiting.\n");
+   return FAILED;
+   }
+
/* FIXME: wait for hba to go online */
stat = qla4xxx_reset_lun(ha, ddb_entry, cmd->device->lun);
if (stat != QLA_SUCCESS) {
@@ -9305,6 +9337,7 @@ static int qla4xxx_eh_target_reset(struct scsi_cmnd *cmd)
struct scsi_qla_host *ha = to_qla_host(cmd->device->host);
struct ddb_entry *ddb_entry = cmd->device->hostdata;
int stat, ret;
+   int rval;
 
if (!ddb_entry)
return FAILED;
@@ -9322,6 +9355,12 @@ static int qla4xxx_eh_target_reset(struct scsi_cmnd *cmd)
  ha->host_no, cmd, jiffies, cmd->request->timeout / HZ,
  ha->dpc_flags, cmd->result, cmd->allowed));
 
+   rval = qla4xxx_isp_check_reg(ha);
+   if (rval != QLA_SUCCESS) {
+   ql4_printk(KERN_INFO, ha, "PCI/Register disconnect, 
exiting.\n");
+   return FAILED;
+   }
+
stat = qla4xxx_reset_target(ha, ddb_entry);
if (stat != QLA_SUCCESS) {
starget_printk(KERN_INFO, scsi_target(cmd->device),
@@ -9376,9 +9415,16 @@ static int qla4xxx_eh_host_reset(struct scsi_cmnd *cmd)
 {
int return_status = FAILED;
struct scsi_qla_host *ha;
+   int rval;
 
ha = to_qla_host(cmd->device->host);
 
+   rval = qla4xxx_isp_check_reg(ha);
+   if (rval != QLA_SUCCESS) {
+   ql4_printk(KERN_INFO, ha, "PCI/Register disconnect, 
exiting.\n");
+   return FAILED;
+   }
+
if ((is_qla8032(ha) || is_qla8042(ha)) && ql4xdontresethba)
qla4_83xx_set_idc_dontreset(ha);
 
-- 
1.8.3.1



RE: [PATCH v2] scsi: mpt3sas: fix oops in error handlers after shutdown/unload

2018-02-11 Thread Sreekanth Reddy
Mauricio,

Instead of returning the scmd with DID_NO_CONNECT in TM path, can we wait
for some time (10 seconds) in shutdown/unload path for the outstanding
commands to complete and even then the scmds are outstanding then return
all the outstanding scmds with DID_NO_CONNECT in the shutdown/unload path
itself as shown below,


diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 13d6e4e..f62ce50 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -6294,14 +6294,14 @@ _base_reset_handler(struct MPT3SAS_ADAPTER *ioc,
int reset_phase)
 }

 /**
- * _wait_for_commands_to_complete - reset controller
+ * mpt3sas_wait_for_commands_to_complete - reset controller
  * @ioc: Pointer to MPT_ADAPTER structure
  *
  * This function is waiting 10s for all pending commands to complete
  * prior to putting controller in reset.
  */
-static void
-_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc)
+void
+mpt3sas_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc)
 {
u32 ioc_state;

@@ -6374,7 +6374,7 @@ mpt3sas_base_hard_reset_handler(struct
MPT3SAS_ADAPTER *ioc,
is_fault = 1;
}
_base_reset_handler(ioc, MPT3_IOC_PRE_RESET);
-   _wait_for_commands_to_complete(ioc);
+   mpt3sas_wait_for_commands_to_complete(ioc);
_base_mask_interrupts(ioc);
r = _base_make_ioc_ready(ioc, type);
if (r)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h
b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 789bc42..99ccf83 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1433,6 +1433,9 @@ void mpt3sas_base_update_missing_delay(struct
MPT3SAS_ADAPTER *ioc,

 int mpt3sas_port_enable(struct MPT3SAS_ADAPTER *ioc);

+void
+mpt3sas_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc);
+

 /* scsih shared API */
 struct scsi_cmnd *mpt3sas_scsih_scsi_lookup_get(struct MPT3SAS_ADAPTER
*ioc,
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 74fca18..458709e 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -4453,7 +4453,7 @@ _scsih_flush_running_cmds(struct MPT3SAS_ADAPTER
*ioc)
st = scsi_cmd_priv(scmd);
mpt3sas_base_clear_st(ioc, st);
scsi_dma_unmap(scmd);
-   if (ioc->pci_error_recovery)
+   if (ioc->pci_error_recovery || ioc->remove_host)
scmd->result = DID_NO_CONNECT << 16;
else
scmd->result = DID_RESET << 16;
@@ -9739,6 +9739,10 @@ static void scsih_remove(struct pci_dev *pdev)
unsigned long flags;

ioc->remove_host = 1;
+
+   mpt3sas_wait_for_commands_to_complete(ioc);
+   _scsih_flush_running_cmds(ioc);
+
_scsih_fw_event_cleanup_queue(ioc);

spin_lock_irqsave(&ioc->fw_event_lock, flags);
@@ -9815,6 +9819,10 @@ scsih_shutdown(struct pci_dev *pdev)
unsigned long flags;

ioc->remove_host = 1;
+
+   mpt3sas_wait_for_commands_to_complete(ioc);
+   _scsih_flush_running_cmds(ioc);
+
_scsih_fw_event_cleanup_queue(ioc);

spin_lock_irqsave(&ioc->fw_event_lock, flags);
---

-Original Message-
From: linux-scsi-ow...@vger.kernel.org
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Mauricio Faria de
Oliveira
Sent: Friday, February 2, 2018 3:46 AM
To: linux-scsi@vger.kernel.org; bart.vanass...@wdc.com
Cc: sathya.prak...@broadcom.com; chaitra.basa...@broadcom.com;
suganath-prabu.subram...@broadcom.com; j...@linux.vnet.ibm.com;
martin.peter...@oracle.com; dougm...@linux.vnet.ibm.com
Subject: [PATCH v2] scsi: mpt3sas: fix oops in error handlers after
shutdown/unload

This patch adds checks for 'ioc->remove_host' in the SCSI error handlers,
so not to access pointers/resources potentially freed in the PCI
shutdown/module unload path.  The error handlers may be invoked after
shutdown/unload, depending on other components.

This problem was observed with kexec on a system with a mpt3sas based
adapter and an infiniband adapter which takes long enough to shutdown:

The mpt3sas driver finished shutting down / disabled interrupt handling,
thus some commands have not finished and timed out.

Since the system was still running (waiting for the infiniband adapter to
shutdown), the scsi error handler for task abort of mpt3sas was invoked,
and hit an oops -- either in scsih_abort() because 'ioc->scsi_lookup' was
NULL (without commit dbec4c9040ed
("scsi: mpt3sas: lockless command submission")), or later up in
scsih_host_reset() (with or without that commit), because it eventually
called mpt3sas_base_get_iocstate().

After that commit, the oops in scsih_abort() does not occur anymore
(_scsih_scsi_lookup_find_by_scmd() is no longer called), but that commit
is too big and out of the scope of linux-stable, where this patch might
help, so still go for the

[PATCH] mpt3sas: Do not use 32-bit atomic request descriptor for Ventura controllers

2018-02-11 Thread Shivasharan S
From: Suganath Prabu S 

Problem Statement:
Sending I/O through 32 bit descriptors to Ventura series of controller
results in IO timeout on certain conditions.
 
This error only occurs on systems with high I/O activity on
Ventura series controllers.
  
Changes in this patch will prevent driver from using 32 bit descriptor
and use 64 bit Descriptors.

Cc: 
Signed-off-by: Suganath Prabu S 
Signed-off-by: Shivasharan S 
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 121 
 drivers/scsi/mpt3sas/mpt3sas_base.h |   1 -
 2 files changed, 122 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 13d6e4e..6051469 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -3108,116 +3108,6 @@ _base_put_smid_default(struct MPT3SAS_ADAPTER *ioc, u16 
smid)
 }
 
 /**
-* _base_put_smid_scsi_io_atomic - send SCSI_IO request to firmware using
-*   Atomic Request Descriptor
-* @ioc: per adapter object
-* @smid: system request message index
-* @handle: device handle, unused in this function, for function type match
-*
-* Return nothing.
-*/
-static void
-_base_put_smid_scsi_io_atomic(struct MPT3SAS_ADAPTER *ioc, u16 smid,
-   u16 handle)
-{
-   Mpi26AtomicRequestDescriptor_t descriptor;
-   u32 *request = (u32 *)&descriptor;
-
-   descriptor.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_SCSI_IO;
-   descriptor.MSIxIndex = _base_get_msix_index(ioc);
-   descriptor.SMID = cpu_to_le16(smid);
-
-   writel(cpu_to_le32(*request), &ioc->chip->AtomicRequestDescriptorPost);
-}
-
-/**
- * _base_put_smid_fast_path_atomic - send fast path request to firmware
- * using Atomic Request Descriptor
- * @ioc: per adapter object
- * @smid: system request message index
- * @handle: device handle, unused in this function, for function type match
- * Return nothing
- */
-static void
-_base_put_smid_fast_path_atomic(struct MPT3SAS_ADAPTER *ioc, u16 smid,
-   u16 handle)
-{
-   Mpi26AtomicRequestDescriptor_t descriptor;
-   u32 *request = (u32 *)&descriptor;
-
-   descriptor.RequestFlags = MPI25_REQ_DESCRIPT_FLAGS_FAST_PATH_SCSI_IO;
-   descriptor.MSIxIndex = _base_get_msix_index(ioc);
-   descriptor.SMID = cpu_to_le16(smid);
-
-   writel(cpu_to_le32(*request), &ioc->chip->AtomicRequestDescriptorPost);
-}
-
-/**
- * _base_put_smid_hi_priority_atomic - send Task Management request to
- * firmware using Atomic Request Descriptor
- * @ioc: per adapter object
- * @smid: system request message index
- * @msix_task: msix_task will be same as msix of IO incase of task abort else 0
- *
- * Return nothing.
- */
-static void
-_base_put_smid_hi_priority_atomic(struct MPT3SAS_ADAPTER *ioc, u16 smid,
-   u16 msix_task)
-{
-   Mpi26AtomicRequestDescriptor_t descriptor;
-   u32 *request = (u32 *)&descriptor;
-
-   descriptor.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_HIGH_PRIORITY;
-   descriptor.MSIxIndex = msix_task;
-   descriptor.SMID = cpu_to_le16(smid);
-
-   writel(cpu_to_le32(*request), &ioc->chip->AtomicRequestDescriptorPost);
-}
-
-/**
- * _base_put_smid_nvme_encap_atomic - send NVMe encapsulated request to
- *   firmware using Atomic Request Descriptor
- * @ioc: per adapter object
- * @smid: system request message index
- *
- * Return nothing.
- */
-static void
-_base_put_smid_nvme_encap_atomic(struct MPT3SAS_ADAPTER *ioc, u16 smid)
-{
-   Mpi26AtomicRequestDescriptor_t descriptor;
-   u32 *request = (u32 *)&descriptor;
-
-   descriptor.RequestFlags = MPI26_REQ_DESCRIPT_FLAGS_PCIE_ENCAPSULATED;
-   descriptor.MSIxIndex = _base_get_msix_index(ioc);
-   descriptor.SMID = cpu_to_le16(smid);
-
-   writel(cpu_to_le32(*request), &ioc->chip->AtomicRequestDescriptorPost);
-}
-
-/**
- * _base_put_smid_default - Default, primarily used for config pages
- * use Atomic Request Descriptor
- * @ioc: per adapter object
- * @smid: system request message index
- *
- * Return nothing.
- */
-static void
-_base_put_smid_default_atomic(struct MPT3SAS_ADAPTER *ioc, u16 smid)
-{
-   Mpi26AtomicRequestDescriptor_t descriptor;
-   u32 *request = (u32 *)&descriptor;
-
-   descriptor.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_DEFAULT_TYPE;
-   descriptor.MSIxIndex = _base_get_msix_index(ioc);
-   descriptor.SMID = cpu_to_le16(smid);
-
-   writel(cpu_to_le32(*request), &ioc->chip->AtomicRequestDescriptorPost);
-}
-
-/**
  * _base_display_OEMs_branding - Display branding string
  * @ioc: per adapter object
  *
@@ -5071,8 +4961,6 @@ _base_get_ioc_facts(struct MPT3SAS_ADAPTER *ioc)
if ((facts->IOCCapabilities &
  MPI2_IOCFACTS_CAPABILITY_RDPQ_ARRAY_CAPABLE) && (!reset_devices))
ioc->rdpq_array_capable = 1;
-   if (facts->IOCCapabilities & MPI26_IOCFACTS_CAPABILITY_ATOMIC_REQ)
-   ioc->atomic_desc_capable = 1;
facts->FWVersion.Word = le32_to_cpu(mpi_reply.FWVersion.Word);
facts->IOCR

[PATCH] megaraid_sas: Do not use 32-bit atomic request descriptor for Ventura controllers

2018-02-11 Thread Shivasharan S
Problem Statement:
Sending I/O through 32 bit descriptors to Ventura series of controller
results in IO timeout on certain conditions.
 
This error only occurs on systems with high I/O activity on
Ventura series controllers.
  
Changes in this patch will prevent driver from using 32 bit descriptor
and use 64 bit Descriptors.

Cc: 
Signed-off-by: Kashyap Desai 
Signed-off-by: Shivasharan S 
---
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 37 ++---
 1 file changed, 12 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c 
b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 073ced07e662..09f2bdb14b0a 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -226,26 +226,21 @@ static void
 megasas_fire_cmd_fusion(struct megasas_instance *instance,
union MEGASAS_REQUEST_DESCRIPTOR_UNION *req_desc)
 {
-   if (instance->adapter_type == VENTURA_SERIES)
-   writel(le32_to_cpu(req_desc->u.low),
-   &instance->reg_set->inbound_single_queue_port);
-   else {
 #if defined(writeq) && defined(CONFIG_64BIT)
-   u64 req_data = (((u64)le32_to_cpu(req_desc->u.high) << 32) |
-   le32_to_cpu(req_desc->u.low));
+   u64 req_data = (((u64)le32_to_cpu(req_desc->u.high) << 32) |
+   le32_to_cpu(req_desc->u.low));
 
-   writeq(req_data, &instance->reg_set->inbound_low_queue_port);
+   writeq(req_data, &instance->reg_set->inbound_low_queue_port);
 #else
-   unsigned long flags;
-   spin_lock_irqsave(&instance->hba_lock, flags);
-   writel(le32_to_cpu(req_desc->u.low),
-   &instance->reg_set->inbound_low_queue_port);
-   writel(le32_to_cpu(req_desc->u.high),
-   &instance->reg_set->inbound_high_queue_port);
-   mmiowb();
-   spin_unlock_irqrestore(&instance->hba_lock, flags);
+   unsigned long flags;
+   spin_lock_irqsave(&instance->hba_lock, flags);
+   writel(le32_to_cpu(req_desc->u.low),
+   &instance->reg_set->inbound_low_queue_port);
+   writel(le32_to_cpu(req_desc->u.high),
+   &instance->reg_set->inbound_high_queue_port);
+   mmiowb();
+   spin_unlock_irqrestore(&instance->hba_lock, flags);
 #endif
-   }
 }
 
 /**
@@ -982,7 +977,6 @@ megasas_ioc_init_fusion(struct megasas_instance *instance)
const char *sys_info;
MFI_CAPABILITIES *drv_ops;
u32 scratch_pad_2;
-   unsigned long flags;
ktime_t time;
bool cur_fw_64bit_dma_capable;
 
@@ -1121,14 +1115,7 @@ megasas_ioc_init_fusion(struct megasas_instance 
*instance)
break;
}
 
-   /* For Ventura also IOC INIT required 64 bit Descriptor write. */
-   spin_lock_irqsave(&instance->hba_lock, flags);
-   writel(le32_to_cpu(req_desc.u.low),
-  &instance->reg_set->inbound_low_queue_port);
-   writel(le32_to_cpu(req_desc.u.high),
-  &instance->reg_set->inbound_high_queue_port);
-   mmiowb();
-   spin_unlock_irqrestore(&instance->hba_lock, flags);
+   megasas_fire_cmd_fusion(instance, &req_desc);
 
wait_and_poll(instance, cmd, MFI_POLL_TIMEOUT_SECS);
 
-- 
2.14.1.dirty



Re: [PATCH v5 02/11] scsi: ufs: sysfs: device descriptor

2018-02-11 Thread Jaegeuk Kim
On 02/06, Stanislav Nijnikov wrote:
> This patch introduces a sysfs group entry for the UFS device descriptor
> parameters. The group adds "device_descriptor" folder under the UFS driver
> sysfs entry (/sys/bus/platform/drivers/ufshcd/*). The parameters are shown
> as hexadecimal numbers. The full information about the parameters could be
> found at UFS specifications 2.1.
> 
> Signed-off-by: Stanislav Nijnikov 
> ---
>  Documentation/ABI/testing/sysfs-driver-ufs | 223 
> +
>  drivers/scsi/ufs/ufs-sysfs.c   | 116 +++
>  drivers/scsi/ufs/ufs.h |   8 ++
>  drivers/scsi/ufs/ufshcd.c  |  12 +-
>  drivers/scsi/ufs/ufshcd.h  |   6 +
>  5 files changed, 359 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-ufs
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-ufs 
> b/Documentation/ABI/testing/sysfs-driver-ufs
> new file mode 100644
> index 000..8da7b84
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-ufs
> @@ -0,0 +1,223 @@
> +What:
> /sys/bus/platform/drivers/ufshcd/*/device_descriptor/device_type
> +Date:February 2018
> +Contact: Stanislav Nijnikov 
> +Description: This file shows the device type. This is one of the UFS
> + device descriptor parameters. The full information about
> + the descriptor could be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/device_descriptor/device_class
> +Date:February 2018
> +Contact: Stanislav Nijnikov 
> +Description: This file shows the device class. This is one of the UFS
> + device descriptor parameters. The full information about
> + the descriptor could be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/device_descriptor/device_sub_class
> +Date:February 2018
> +Contact: Stanislav Nijnikov 
> +Description: This file shows the UFS storage subclass. This is one of
> + the UFS device descriptor parameters. The full information
> + about the descriptor could be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/device_descriptor/protocol
> +Date:February 2018
> +Contact: Stanislav Nijnikov 
> +Description: This file shows the protocol supported by an UFS device.
> + This is one of the UFS device descriptor parameters.
> + The full information about the descriptor could be found
> + at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/device_descriptor/number_of_luns
> +Date:February 2018
> +Contact: Stanislav Nijnikov 
> +Description: This file shows number of logical units. This is one of
> + the UFS device descriptor parameters. The full information
> + about the descriptor could be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/device_descriptor/number_of_wluns
> +Date:February 2018
> +Contact: Stanislav Nijnikov 
> +Description: This file shows number of well known logical units.
> + This is one of the UFS device descriptor parameters.
> + The full information about the descriptor could be found
> + at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/device_descriptor/boot_enable
> +Date:February 2018
> +Contact: Stanislav Nijnikov 
> +Description: This file shows value that indicates whether the device is
> + enabled for boot. This is one of the UFS device descriptor
> + parameters. The full information about the descriptor could
> + be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/device_descriptor/descriptor_access_enable
> +Date:February 2018
> +Contact: Stanislav Nijnikov 
> +Description: This file shows value that indicates whether the device
> + descriptor could be read after partial initialization phase
> + of the boot sequence. This is one of the UFS device descriptor
> + parameters. The full information about the descriptor could
> + be found at UFS specifications 2.1.
> + The file is read only.
> +
> +What:
> /sys/bus/platform/drivers/ufshcd/*/device_descriptor/initial_power_mode
> +Date:February 2018
> +Contact: Stanislav Nijni

Re: [PATCH v5 01/11] scsi: ufs: sysfs: attribute group for existing sysfs entries.

2018-02-11 Thread Jaegeuk Kim
On 02/06, Stanislav Nijnikov wrote:
> This patch introduces attribute group to show existing sysfs entries.
> 
> Signed-off-by: Stanislav Nijnikov 
> ---
>  drivers/scsi/ufs/Makefile|   3 +-
>  drivers/scsi/ufs/ufs-sysfs.c | 156 
> +++
>  drivers/scsi/ufs/ufs-sysfs.h |  14 
>  drivers/scsi/ufs/ufshcd.c| 156 
> ++-
>  drivers/scsi/ufs/ufshcd.h|   2 +
>  5 files changed, 178 insertions(+), 153 deletions(-)
>  create mode 100644 drivers/scsi/ufs/ufs-sysfs.c
>  create mode 100644 drivers/scsi/ufs/ufs-sysfs.h
> 
> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> index 9310c6c..918f579 100644
> --- a/drivers/scsi/ufs/Makefile
> +++ b/drivers/scsi/ufs/Makefile
> @@ -3,6 +3,7 @@
>  obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o 
> tc-dwc-g210.o
>  obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o 
> tc-dwc-g210.o
>  obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
> -obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
> +obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> +ufshcd-core-objs := ufshcd.o ufs-sysfs.o
>  obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
>  obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
> diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
> new file mode 100644
> index 000..ce8dcb6
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-sysfs.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright (C) 2018 Western Digital Corporation
> +
> +#include 
> +#include 
> +
> +#include "ufs-sysfs.h"
> +
> +static const char *ufschd_uic_link_state_to_string(
> + enum uic_link_state state)
> +{
> + switch (state) {
> + case UIC_LINK_OFF_STATE:return "OFF";
> + case UIC_LINK_ACTIVE_STATE: return "ACTIVE";
> + case UIC_LINK_HIBERN8_STATE:return "HIBERN8";
> + default:return "UNKNOWN";
> + }
> +}
> +
> +static const char *ufschd_ufs_dev_pwr_mode_to_string(
> + enum ufs_dev_pwr_mode state)
> +{
> + switch (state) {
> + case UFS_ACTIVE_PWR_MODE:   return "ACTIVE";
> + case UFS_SLEEP_PWR_MODE:return "SLEEP";
> + case UFS_POWERDOWN_PWR_MODE:return "POWERDOWN";
> + default:return "UNKNOWN";
> + }
> +}
> +
> +static inline ssize_t ufs_sysfs_pm_lvl_store(struct device *dev,
> +  struct device_attribute *attr,
> +  const char *buf, size_t count,
> +  bool rpm)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + unsigned long flags, value;
> +
> + if (kstrtoul(buf, 0, &value))
> + return -EINVAL;
> +
> + if (value >= UFS_PM_LVL_MAX)
> + return -EINVAL;
> +
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + if (rpm)
> + hba->rpm_lvl = value;
> + else
> + hba->spm_lvl = value;
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> + return count;
> +}
> +
> +static ssize_t rpm_lvl_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + int curr_len;
> + u8 lvl;
> +
> + curr_len = snprintf(buf, PAGE_SIZE,
> + "\nCurrent Runtime PM level [%d] => dev_state [%s] 
> link_state [%s]\n",
> + hba->rpm_lvl,
> + ufschd_ufs_dev_pwr_mode_to_string(
> + ufs_pm_lvl_states[hba->rpm_lvl].dev_state),
> + ufschd_uic_link_state_to_string(
> + ufs_pm_lvl_states[hba->rpm_lvl].link_state));

If there is no objection regarding to backward compatibility, can we also
clean this up by adding multiple entries having single string each, as Greg
recommended?

For example,

/rpm_level
1

/rpm_dev_state
ACTIVE

/rpm_link_state
HIBERN8

/spm_level
2

/spm_dev_state
SLEEP

/spm_link_state
ACTIVE

/avail_dev_states
0:ACTIVE 1:ACTIVE 2:SLEEP 3:SLEEP 4:POWERDOWN 5:POWERDOWN

/avail_link_states
0:ACTIVE 1:HIBERN8 2:ACTIVE 3:HIBERN8 4:HIBERN8 5:OFF

Thanks,

> +
> + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> +  "\nAll available Runtime PM levels info:\n");
> + for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> +  "\tRuntime PM level [%d] => dev_state [%s] 
> link_state [%s]\n",
> + lvl,
> + ufschd_ufs_dev_pwr_mode_to_string(
> + ufs_pm_lvl_states[lvl].dev_state),
> + ufschd_uic_link_state_to_string(
> + ufs_pm_lvl