[PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
As part of device initialization sequence, sending NOP OUT UPIU and
waiting for NOP IN UPIU response is mandatory. This confirms that the
device UFS Transport (UTP) layer is functional and the host can configure
the device with further commands. Add support for sending NOP OUT UPIU to
check the device connection path and test whether the UTP layer on the
device side is functional during initialization.
A tag is acquired from the SCSI tag map space in order to send the device
management command. When the tag is acquired by internal command the scsi
command is rejected with host busy flag in order to requeue the request.
To avoid frequent collisions between internal commands and scsi commands
the device management command tag is allocated in the opposite direction
w.r.t block layer tag allocation.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
Signed-off-by: Dolev Raviv dra...@codeaurora.org
---
drivers/scsi/ufs/ufs.h| 43 +++-
drivers/scsi/ufs/ufshcd.c | 596 +
drivers/scsi/ufs/ufshcd.h | 29 ++-
3 files changed, 552 insertions(+), 116 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 139bc06..14c0a4e 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -36,10 +36,16 @@
#ifndef _UFS_H
#define _UFS_H
+#include linux/mutex.h
+#include linux/types.h
+
#define MAX_CDB_SIZE 16
+#define GENERAL_UPIU_REQUEST_SIZE 32
+#define UPIU_HEADER_DATA_SEGMENT_MAX_SIZE ((ALIGNED_UPIU_SIZE) - \
+ (GENERAL_UPIU_REQUEST_SIZE))
#define UPIU_HEADER_DWORD(byte3, byte2, byte1, byte0)\
- ((byte3 24) | (byte2 16) |\
+ cpu_to_be32((byte3 24) | (byte2 16) |\
(byte1 8) | (byte0))
/*
@@ -73,6 +79,7 @@ enum {
UPIU_TRANSACTION_TASK_RSP = 0x24,
UPIU_TRANSACTION_READY_XFER = 0x31,
UPIU_TRANSACTION_QUERY_RSP = 0x36,
+ UPIU_TRANSACTION_REJECT_UPIU= 0x3F,
};
/* UPIU Read/Write flags */
@@ -110,6 +117,12 @@ enum {
UPIU_COMMAND_SET_TYPE_QUERY = 0x2,
};
+/* UTP Transfer Request Command Offset */
+#define UPIU_COMMAND_TYPE_OFFSET 28
+
+/* Offset of the response code in the UPIU header */
+#define UPIU_RSP_CODE_OFFSET 8
+
enum {
MASK_SCSI_STATUS= 0xFF,
MASK_TASK_RESPONSE = 0xFF00,
@@ -138,26 +151,32 @@ struct utp_upiu_header {
/**
* struct utp_upiu_cmd - Command UPIU structure
- * @header: UPIU header structure DW-0 to DW-2
* @data_transfer_len: Data Transfer Length DW-3
* @cdb: Command Descriptor Block CDB DW-4 to DW-7
*/
struct utp_upiu_cmd {
- struct utp_upiu_header header;
u32 exp_data_transfer_len;
u8 cdb[MAX_CDB_SIZE];
};
/**
- * struct utp_upiu_rsp - Response UPIU structure
- * @header: UPIU header DW-0 to DW-2
+ * struct utp_upiu_req - general upiu request structure
+ * @header:UPIU header structure DW-0 to DW-2
+ * @sc: fields structure for scsi command DW-3 to DW-7
+ */
+struct utp_upiu_req {
+ struct utp_upiu_header header;
+ struct utp_upiu_cmd sc;
+};
+
+/**
+ * struct utp_cmd_rsp - Response UPIU structure
* @residual_transfer_count: Residual transfer count DW-3
* @reserved: Reserved double words DW-4 to DW-7
* @sense_data_len: Sense data length DW-8 U16
* @sense_data: Sense data field DW-8 to DW-12
*/
-struct utp_upiu_rsp {
- struct utp_upiu_header header;
+struct utp_cmd_rsp {
u32 residual_transfer_count;
u32 reserved[4];
u16 sense_data_len;
@@ -165,6 +184,16 @@ struct utp_upiu_rsp {
};
/**
+ * struct utp_upiu_rsp - general upiu response structure
+ * @header: UPIU header structure DW-0 to DW-2
+ * @sr: fields structure for scsi command DW-3 to DW-12
+ */
+struct utp_upiu_rsp {
+ struct utp_upiu_header header;
+ struct utp_cmd_rsp sr;
+};
+
+/**
* struct utp_upiu_task_req - Task request UPIU structure
* @header - UPIU header structure DW0 to DW-2
* @input_param1: Input parameter 1 DW-3
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..3f482b6 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -43,6 +43,11 @@
/* UIC command timeout, unit: ms */
#define UIC_CMD_TIMEOUT500
+/* NOP OUT retries waiting for NOP IN response */
+#define NOP_OUT_RETRIES10
+/* Timeout after 30 msecs if NOP OUT hangs without response */
+#define NOP_OUT_TIMEOUT30 /* msecs */
+
enum {
UFSHCD_MAX_CHANNEL = 0,
UFSHCD_MAX_ID = 1,
@@ -71,6 +76,47 @@ enum {
INT_AGGR_CONFIG,
};
+/*
+ * ufshcd_wait_for_register - wait for register value to change
+ * @hba - per-adapter interface
+ * @reg - mmio register offset
+ * @mask - mask to apply to read register value
+ * @val - wait condition
+ * @interval_us - polling interval in microsecs
+ * @timeout_ms - timeout in millisecs
+ *
+ * Returns final
[PATCH V3 2/2] scsi: ufs: Set fDeviceInit flag to initiate device initialization
From: Dolev Raviv dra...@codeaurora.org
Allow UFS device to complete its initialization and accept
SCSI commands by setting fDeviceInit flag. The device may take
time for this operation and hence the host should poll until
fDeviceInit flag is toggled to zero. This step is mandated by
UFS device specification for device initialization completion.
Signed-off-by: Dolev Raviv dra...@codeaurora.org
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufs.h| 88 +-
drivers/scsi/ufs/ufshcd.c | 292 -
drivers/scsi/ufs/ufshcd.h | 14 ++
drivers/scsi/ufs/ufshci.h |2 +-
4 files changed, 390 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 14c0a4e..db5bde4 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -43,6 +43,8 @@
#define GENERAL_UPIU_REQUEST_SIZE 32
#define UPIU_HEADER_DATA_SEGMENT_MAX_SIZE ((ALIGNED_UPIU_SIZE) - \
(GENERAL_UPIU_REQUEST_SIZE))
+#define QUERY_OSF_SIZE ((GENERAL_UPIU_REQUEST_SIZE) - \
+ (sizeof(struct utp_upiu_header)))
#define UPIU_HEADER_DWORD(byte3, byte2, byte1, byte0)\
cpu_to_be32((byte3 24) | (byte2 16) |\
@@ -68,7 +70,7 @@ enum {
UPIU_TRANSACTION_COMMAND= 0x01,
UPIU_TRANSACTION_DATA_OUT = 0x02,
UPIU_TRANSACTION_TASK_REQ = 0x04,
- UPIU_TRANSACTION_QUERY_REQ = 0x26,
+ UPIU_TRANSACTION_QUERY_REQ = 0x16,
};
/* UTP UPIU Transaction Codes Target to Initiator */
@@ -97,8 +99,19 @@ enum {
UPIU_TASK_ATTR_ACA = 0x03,
};
-/* UTP QUERY Transaction Specific Fields OpCode */
+/* UPIU Query request function */
enum {
+ UPIU_QUERY_FUNC_STANDARD_READ_REQUEST = 0x01,
+ UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST = 0x81,
+};
+
+/* Flag idn for Query Requests*/
+enum flag_idn {
+ QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
+};
+
+/* 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,
@@ -110,6 +123,21 @@ enum {
UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
};
+/* Query response result code */
+enum {
+ QUERY_RESULT_SUCCESS= 0x00,
+ QUERY_RESULT_NOT_READABLE = 0xF6,
+ QUERY_RESULT_NOT_WRITEABLE = 0xF7,
+ QUERY_RESULT_ALREADY_WRITTEN= 0xF8,
+ QUERY_RESULT_INVALID_LENGTH = 0xF9,
+ QUERY_RESULT_INVALID_VALUE = 0xFA,
+ QUERY_RESULT_INVALID_SELECTOR = 0xFB,
+ QUERY_RESULT_INVALID_INDEX = 0xFC,
+ QUERY_RESULT_INVALID_IDN= 0xFD,
+ QUERY_RESULT_INVALID_OPCODE = 0xFE,
+ QUERY_RESULT_GENERAL_FAILURE= 0xFF,
+};
+
/* UTP Transfer Request Command Type (CT) */
enum {
UPIU_COMMAND_SET_TYPE_SCSI = 0x0,
@@ -127,6 +155,7 @@ enum {
MASK_SCSI_STATUS= 0xFF,
MASK_TASK_RESPONSE = 0xFF00,
MASK_RSP_UPIU_RESULT= 0x,
+ MASK_QUERY_DATA_SEG_LEN = 0x,
};
/* Task management service response */
@@ -160,13 +189,40 @@ struct utp_upiu_cmd {
};
/**
+ * struct utp_upiu_query - upiu request buffer structure for
+ * query request.
+ * @opcode: command to perform B-0
+ * @idn: a value that indicates the particular type of data B-1
+ * @index: Index to further identify data B-2
+ * @selector: Index to further identify data B-3
+ * @reserved_osf: spec reserved field B-4,5
+ * @length: number of descriptor bytes to read/write B-6,7
+ * @value: Attribute value to be written DW-5
+ * @reserved: spec reserved DW-6,7
+ */
+struct utp_upiu_query {
+ u8 opcode;
+ u8 idn;
+ u8 index;
+ u8 selector;
+ u16 reserved_osf;
+ u16 length;
+ u32 value;
+ u32 reserved[2];
+};
+
+/**
* struct utp_upiu_req - general upiu request structure
* @header:UPIU header structure DW-0 to DW-2
* @sc: fields structure for scsi command DW-3 to DW-7
+ * @qr: fields structure for query request DW-3 to DW-7
*/
struct utp_upiu_req {
struct utp_upiu_header header;
- struct utp_upiu_cmd sc;
+ union {
+ struct utp_upiu_cmd sc;
+ struct utp_upiu_query qr;
+ };
};
/**
@@ -187,10 +243,14 @@ struct utp_cmd_rsp {
* struct utp_upiu_rsp - general upiu response structure
* @header: UPIU header structure DW-0 to DW-2
* @sr: fields structure for scsi command DW-3 to DW-12
+ * @qr: fields structure for query request DW-3 to DW-7
*/
struct utp_upiu_rsp {
struct utp_upiu_header header;
- struct utp_cmd_rsp sr;
+ union {
+ struct utp_cmd_rsp sr;
+ struct utp_upiu_query qr;
+ };
};
/**
@@ -223,4
[PATCH V3 0/2] Add suport for internal request (NOP and Query Request)
This patch series replace the previous Query Request and NOP patches: [PATCH 1/8] scsi: ufs: add support for query [PATCH 6/8] scsi: ufs: Add support for sending NOP OUT UPIU [PATCH 7/8] scsi: ufs: Set fDeviceInit flag to initiate device initialization Major difference - Sending the query request via the SCSI vendor specific command can cause a deadlock in case the SCSI command queue is blocked and we would like to send a query request (for example fDeviceInit in case of re-initialization). In addition, usage of a vendor specific SCSI command for UFS can cause future conflicts if this vendor specific command will be allocated for a different usage. The new patch series gets an internal tag for NOP and query requests and do not involve the SCSI layer in UFS specific requests transfers. This design also resolves the possible deadlock mentioned above. Changes from v2: - Rebased on scsi-misc branche (commit 8c0eb596baa5) - Minor addition to structure documentation in ufshcd.h Changes from v1 - Allocate a tag for device management commands dynamically instead of reserving tag[MAX_QUEUE - 1]. - Changed the internal_cmd naming to dev_cmd to avoid confusion with other type of internal commands other than NOP and QUERY. Dolev Raviv (1): scsi: ufs: Set fDeviceInit flag to initiate device initialization Sujit Reddy Thumma (1): scsi: ufs: Add support for sending NOP OUT UPIU drivers/scsi/ufs/ufs.h| 127 ++- drivers/scsi/ufs/ufshcd.c | 886 +++-- drivers/scsi/ufs/ufshcd.h | 43 ++- drivers/scsi/ufs/ufshci.h |2 +- 4 files changed, 939 insertions(+), 119 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. -- 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 V3 0/2] scsi: ufs: Add support to control UFS device background operations
Add host assisted background operations for UFS device and runtime PM helpers for ufshcd platform and pci glue drivers. The background operations are disabled during runtime resume and enabled when the device is idle and runtime suspended. These patches depends on: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU [PATCH V3 2/2] scsi: ufs: Set fDeviceInit flag to initiate device initialization Changes from v2: - Enable auto bkops by default explicitly during bootup so that we may not assume it as enabled after reset. - Enable runtime PM support for contexts that are not part of SCSI, so that the host is not suspended while running other contexts like bkops exception handling. Changes from v1: - Minor cleanup and rebase - Forced enable of auto bkops during initialization to make sure device and driver state are matched. Sujit Reddy Thumma (2): scsi: ufs: Add support for host assisted background operations scsi: ufs: Add runtime PM support for UFS host controller driver drivers/scsi/ufs/ufs.h | 25 +++- drivers/scsi/ufs/ufshcd-pci.c| 65 +++- drivers/scsi/ufs/ufshcd-pltfrm.c | 51 ++- drivers/scsi/ufs/ufshcd.c| 346 ++ drivers/scsi/ufs/ufshcd.h| 10 + 5 files changed, 489 insertions(+), 8 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. -- 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 V3 4/4] scsi: ufs: Improve UFS fatal error handling
Error handling in UFS driver is broken and resets the host controller
for fatal errors without re-initialization. Correct the fatal error
handling sequence according to UFS Host Controller Interface (HCI)
v1.1 specification.
o Upon determining fatal error condition the host controller may hang
forever until a reset is applied, so just retrying the command doesn't
work without a reset. So, the reset is applied in the driver context
in a separate work and SCSI mid-layer isn't informed until reset is
applied.
o Processed requests which are completed without error are reported to
SCSI layer as successful and any pending commands that are not started
yet or are not cause of the error are re-queued into scsi midlayer queue.
For the command that caused error, host controller or device is reset
and DID_ERROR is returned for command retry after applying reset.
o SCSI is informed about the expected Unit-Attentioni exception from the
device for the immediate command after a reset so that the SCSI layer
take necessary steps to establish communication with the device.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufshcd.c | 349 +++-
drivers/scsi/ufs/ufshcd.h |2 +
drivers/scsi/ufs/ufshci.h | 19 ++-
3 files changed, 295 insertions(+), 75 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b4c9910..2a3874f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -80,6 +80,14 @@ enum {
UFSHCD_EH_DEVICE_RESET_PENDING = (1 1),
};
+/* UFSHCD UIC layer error flags */
+enum {
+ UFSHCD_UIC_DL_PA_INIT_ERROR = (1 0), /* Data link layer error */
+ UFSHCD_UIC_NL_ERROR = (1 1), /* Network layer error */
+ UFSHCD_UIC_TL_ERROR = (1 2), /* Transport Layer error */
+ UFSHCD_UIC_DME_ERROR = (1 3), /* DME error */
+};
+
/* Interrupt configuration options */
enum {
UFSHCD_INT_DISABLE,
@@ -108,6 +116,7 @@ enum {
static void ufshcd_tmc_handler(struct ufs_hba *hba);
static void ufshcd_async_scan(void *data, async_cookie_t cookie);
+static int ufshcd_reset_and_restore(struct ufs_hba *hba);
/*
* ufshcd_wait_for_register - wait for register value to change
@@ -1605,9 +1614,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba
*hba)
goto out;
}
- if (hba-ufshcd_state == UFSHCD_STATE_RESET)
- scsi_unblock_requests(hba-host);
-
out:
return err;
}
@@ -1733,66 +1739,6 @@ static int ufshcd_validate_dev_connection(struct ufs_hba
*hba)
}
/**
- * ufshcd_do_reset - reset the host controller
- * @hba: per adapter instance
- *
- * Returns SUCCESS/FAILED
- */
-static int ufshcd_do_reset(struct ufs_hba *hba)
-{
- struct ufshcd_lrb *lrbp;
- unsigned long flags;
- int tag;
-
- /* block commands from midlayer */
- scsi_block_requests(hba-host);
-
- spin_lock_irqsave(hba-host-host_lock, flags);
- hba-ufshcd_state = UFSHCD_STATE_RESET;
-
- /* send controller to reset state */
- ufshcd_hba_stop(hba);
- spin_unlock_irqrestore(hba-host-host_lock, flags);
-
- /* abort outstanding commands */
- for (tag = 0; tag hba-nutrs; tag++) {
- if (test_bit(tag, hba-outstanding_reqs)) {
- lrbp = hba-lrb[tag];
- if (lrbp-cmd) {
- scsi_dma_unmap(lrbp-cmd);
- lrbp-cmd-result = DID_RESET 16;
- lrbp-cmd-scsi_done(lrbp-cmd);
- lrbp-cmd = NULL;
- clear_bit_unlock(tag, hba-lrb_in_use);
- }
- }
- }
-
- /* complete device management command */
- if (hba-dev_cmd.complete)
- complete(hba-dev_cmd.complete);
-
- /* clear outstanding request/task bit maps */
- hba-outstanding_reqs = 0;
- hba-outstanding_tasks = 0;
-
- /* Host controller enable */
- if (ufshcd_hba_enable(hba)) {
- dev_err(hba-dev,
- Reset: Controller initialization failed\n);
- return FAILED;
- }
-
- if (ufshcd_link_startup(hba)) {
- dev_err(hba-dev,
- Reset: Link start-up failed\n);
- return FAILED;
- }
-
- return SUCCESS;
-}
-
-/**
* ufshcd_slave_alloc - handle initial SCSI device configurations
* @sdev: pointer to SCSI device
*
@@ -1809,6 +1755,9 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
sdev-use_10_for_ms = 1;
scsi_set_tag_type(sdev, MSG_SIMPLE_TAG);
+ /* allow SCSI layer to restart the device in case of errors */
+ sdev-allow_restart = 1;
+
/*
* Inform SCSI Midlayer that the LUN queue depth is same as the
* controller queue depth. If a LUN queue depth is less than the
@@
[PATCH V3 3/4] scsi: ufs: Fix device and host reset methods
As of now SCSI initiated error handling is broken because,
the reset APIs don't try to bring back the device initialized and
ready for further transfers.
In case of timeouts, the scsi error handler takes care of handling aborts
and resets. Improve the error handling in such scenario by resetting the
device and host and re-initializing them in proper manner.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufshcd.c | 467 +++--
drivers/scsi/ufs/ufshcd.h |2 +
2 files changed, 411 insertions(+), 58 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 51ce096..b4c9910 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -69,9 +69,15 @@ enum {
/* UFSHCD states */
enum {
- UFSHCD_STATE_OPERATIONAL,
UFSHCD_STATE_RESET,
UFSHCD_STATE_ERROR,
+ UFSHCD_STATE_OPERATIONAL,
+};
+
+/* UFSHCD error handling flags */
+enum {
+ UFSHCD_EH_HOST_RESET_PENDING = (1 0),
+ UFSHCD_EH_DEVICE_RESET_PENDING = (1 1),
};
/* Interrupt configuration options */
@@ -87,6 +93,22 @@ enum {
INT_AGGR_CONFIG,
};
+#define ufshcd_set_device_reset_pending(h) \
+ (h-eh_flags |= UFSHCD_EH_DEVICE_RESET_PENDING)
+#define ufshcd_set_host_reset_pending(h) \
+ (h-eh_flags |= UFSHCD_EH_HOST_RESET_PENDING)
+#define ufshcd_device_reset_pending(h) \
+ (h-eh_flags UFSHCD_EH_DEVICE_RESET_PENDING)
+#define ufshcd_host_reset_pending(h) \
+ (h-eh_flags UFSHCD_EH_HOST_RESET_PENDING)
+#define ufshcd_clear_device_reset_pending(h) \
+ (h-eh_flags = ~UFSHCD_EH_DEVICE_RESET_PENDING)
+#define ufshcd_clear_host_reset_pending(h) \
+ (h-eh_flags = ~UFSHCD_EH_HOST_RESET_PENDING)
+
+static void ufshcd_tmc_handler(struct ufs_hba *hba);
+static void ufshcd_async_scan(void *data, async_cookie_t cookie);
+
/*
* ufshcd_wait_for_register - wait for register value to change
* @hba - per-adapter interface
@@ -851,9 +873,22 @@ static int ufshcd_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *cmd)
tag = cmd-request-tag;
- if (hba-ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
+ switch (hba-ufshcd_state) {
+ case UFSHCD_STATE_OPERATIONAL:
+ break;
+ case UFSHCD_STATE_RESET:
err = SCSI_MLQUEUE_HOST_BUSY;
goto out;
+ case UFSHCD_STATE_ERROR:
+ set_host_byte(cmd, DID_ERROR);
+ cmd-scsi_done(cmd);
+ goto out;
+ default:
+ dev_WARN_ONCE(hba-dev, 1, %s: invalid state %d\n,
+ __func__, hba-ufshcd_state);
+ set_host_byte(cmd, DID_BAD_TARGET);
+ cmd-scsi_done(cmd);
+ goto out;
}
/* acquire the tag to make sure device cmds don't use it */
@@ -1573,8 +1608,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba
*hba)
if (hba-ufshcd_state == UFSHCD_STATE_RESET)
scsi_unblock_requests(hba-host);
- hba-ufshcd_state = UFSHCD_STATE_OPERATIONAL;
-
out:
return err;
}
@@ -2273,6 +2306,106 @@ out:
}
/**
+ * ufshcd_utrl_is_rsr_enabled - check if run-stop register is enabled
+ * @hba: per-adapter instance
+ */
+static bool ufshcd_utrl_is_rsr_enabled(struct ufs_hba *hba)
+{
+ return ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_LIST_RUN_STOP) 0x1;
+}
+
+/**
+ * ufshcd_utmrl_is_rsr_enabled - check if run-stop register is enabled
+ * @hba: per-adapter instance
+ */
+static bool ufshcd_utmrl_is_rsr_enabled(struct ufs_hba *hba)
+{
+ return ufshcd_readl(hba, REG_UTP_TASK_REQ_LIST_RUN_STOP) 0x1;
+}
+
+/**
+ * ufshcd_complete_pending_tasks - complete outstanding tasks
+ * @hba: per adapter instance
+ *
+ * Abort in-progress task management commands and wakeup
+ * waiting threads.
+ *
+ * Returns non-zero error value when failed to clear all the commands.
+ */
+static int ufshcd_complete_pending_tasks(struct ufs_hba *hba)
+{
+ u32 reg;
+ int err = 0;
+ unsigned long flags;
+
+ if (!hba-outstanding_tasks)
+ goto out;
+
+ /* Clear UTMRL only when run-stop is enabled */
+ if (ufshcd_utmrl_is_rsr_enabled(hba))
+ ufshcd_writel(hba, ~hba-outstanding_tasks,
+ REG_UTP_TASK_REQ_LIST_CLEAR);
+
+ /* poll for max. 1 sec to clear door bell register by h/w */
+ reg = ufshcd_wait_for_register(hba,
+ REG_UTP_TASK_REQ_DOOR_BELL,
+ hba-outstanding_tasks, 0, 1000, 1000);
+ if (reg hba-outstanding_tasks)
+ err = -ETIMEDOUT;
+
+ spin_lock_irqsave(hba-host-host_lock, flags);
+ /* complete commands that were cleared out */
+ ufshcd_tmc_handler(hba);
+ spin_unlock_irqrestore(hba-host-host_lock, flags);
+out:
+ if (err)
+ dev_err(hba-dev, %s: failed, still pending = 0x%.8x\n,
+
[PATCH V3 1/4] scsi: ufs: Fix broken task management command implementation
Currently, sending Task Management (TM) command to the card might
be broken in some scenarios as listed below:
Problem: If there are more than 8 TM commands the implementation
returns error to the caller.
Fix: Wait for one of the slots to be emptied and send the command.
Problem: Sometimes it is necessary for the caller to know the TM service
response code to determine the task status.
Fix: Propogate the service response to the caller.
Problem: If the TM command times out no proper error recovery is
implemented.
Fix: Clear the command in the controller door-bell register, so that
further commands for the same slot don't fail.
Problem: While preparing the TM command descriptor, the task tag used
should be unique across SCSI/NOP/QUERY/TM commands and not the
task tag of the command which the TM command is trying to manage.
Fix: Use a unique task tag instead of task tag of SCSI command.
Problem: Since the TM command involves H/W communication, abruptly ending
the request on kill interrupt signal might cause h/w malfunction.
Fix: Wait for hardware completion interrupt with TASK_UNINTERRUPTIBLE
set.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufshcd.c | 177 ++---
drivers/scsi/ufs/ufshcd.h |8 ++-
2 files changed, 126 insertions(+), 59 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index af7d01d..a176421 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -53,6 +53,9 @@
/* Query request timeout */
#define QUERY_REQ_TIMEOUT 30 /* msec */
+/* Task management command timeout */
+#define TM_CMD_TIMEOUT 100 /* msecs */
+
/* Expose the flag value from utp_upiu_query.value */
#define MASK_QUERY_UPIU_FLAG_LOC 0xFF
@@ -190,13 +193,35 @@ ufshcd_get_tmr_ocs(struct utp_task_req_desc
*task_req_descp)
/**
* ufshcd_get_tm_free_slot - get a free slot for task management request
* @hba: per adapter instance
+ * @free_slot: pointer to variable with available slot value
*
- * Returns maximum number of task management request slots in case of
- * task management queue full or returns the free slot number
+ * Get a free tag and lock it until ufshcd_put_tm_slot() is called.
+ * Returns 0 if free slot is not available, else return 1 with tag value
+ * in @free_slot.
*/
-static inline int ufshcd_get_tm_free_slot(struct ufs_hba *hba)
+static bool ufshcd_get_tm_free_slot(struct ufs_hba *hba, int *free_slot)
+{
+ int tag;
+ bool ret = false;
+
+ if (!free_slot)
+ goto out;
+
+ do {
+ tag = find_first_zero_bit(hba-tm_slots_in_use, hba-nutmrs);
+ if (tag = hba-nutmrs)
+ goto out;
+ } while (test_and_set_bit_lock(tag, hba-tm_slots_in_use));
+
+ *free_slot = tag;
+ ret = true;
+out:
+ return ret;
+}
+
+static inline void ufshcd_put_tm_slot(struct ufs_hba *hba, int slot)
{
- return find_first_zero_bit(hba-outstanding_tasks, hba-nutmrs);
+ clear_bit_unlock(slot, hba-tm_slots_in_use);
}
/**
@@ -1778,10 +1803,11 @@ static void ufshcd_slave_destroy(struct scsi_device
*sdev)
* ufshcd_task_req_compl - handle task management request completion
* @hba: per adapter instance
* @index: index of the completed request
+ * @resp: task management service response
*
- * Returns SUCCESS/FAILED
+ * Returns non-zero value on error, zero on success
*/
-static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index)
+static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8 *resp)
{
struct utp_task_req_desc *task_req_descp;
struct utp_upiu_task_rsp *task_rsp_upiup;
@@ -1802,19 +1828,15 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba,
u32 index)
task_req_descp[index].task_rsp_upiu;
task_result = be32_to_cpu(task_rsp_upiup-header.dword_1);
task_result = ((task_result MASK_TASK_RESPONSE) 8);
-
- if (task_result != UPIU_TASK_MANAGEMENT_FUNC_COMPL
- task_result != UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED)
- task_result = FAILED;
- else
- task_result = SUCCESS;
+ if (resp)
+ *resp = (u8)task_result;
} else {
- task_result = FAILED;
- dev_err(hba-dev,
- trc: Invalid ocs = %x\n, ocs_value);
+ dev_err(hba-dev, %s: failed, ocs = 0x%x\n,
+ __func__, ocs_value);
}
spin_unlock_irqrestore(hba-host-host_lock, flags);
- return task_result;
+
+ return ocs_value;
}
/**
@@ -2298,7 +2320,7 @@ static void ufshcd_tmc_handler(struct ufs_hba *hba)
tm_doorbell = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL);
hba-tm_condition =
[PATCH V3 2/2] scsi: ufs: Add runtime PM support for UFS host controller driver
Add runtime PM helpers to suspend/resume UFS controller at runtime.
Enable runtime PM by default for pci and platform drivers as the
initialized hardware can suspend if it is not used after bootup.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufshcd-pci.c| 65 ++---
drivers/scsi/ufs/ufshcd-pltfrm.c | 51 +-
drivers/scsi/ufs/ufshcd.c|8 +
3 files changed, 117 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 48be39a..7bd8faa 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -35,6 +35,7 @@
#include ufshcd.h
#include linux/pci.h
+#include linux/pm_runtime.h
#ifdef CONFIG_PM
/**
@@ -44,7 +45,7 @@
*
* Returns -ENOSYS
*/
-static int ufshcd_pci_suspend(struct pci_dev *pdev, pm_message_t state)
+static int ufshcd_pci_suspend(struct device *dev)
{
/*
* TODO:
@@ -61,7 +62,7 @@ static int ufshcd_pci_suspend(struct pci_dev *pdev,
pm_message_t state)
*
* Returns -ENOSYS
*/
-static int ufshcd_pci_resume(struct pci_dev *pdev)
+static int ufshcd_pci_resume(struct device *dev)
{
/*
* TODO:
@@ -71,8 +72,48 @@ static int ufshcd_pci_resume(struct pci_dev *pdev)
return -ENOSYS;
}
+#else
+#define ufshcd_pci_suspend NULL
+#define ufshcd_pci_resume NULL
#endif /* CONFIG_PM */
+#ifdef CONFIG_PM_RUNTIME
+static int ufshcd_pci_runtime_suspend(struct device *dev)
+{
+ struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+ struct ufs_hba *hba = pci_get_drvdata(pdev);
+
+ if (!hba)
+ return 0;
+
+ return ufshcd_runtime_suspend(hba);
+}
+static int ufshcd_pci_runtime_resume(struct device *dev)
+{
+ struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+ struct ufs_hba *hba = pci_get_drvdata(pdev);
+
+ if (!hba)
+ return 0;
+
+ return ufshcd_runtime_resume(hba);
+}
+static int ufshcd_pci_runtime_idle(struct device *dev)
+{
+ struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+ struct ufs_hba *hba = pci_get_drvdata(pdev);
+
+ if (!hba)
+ return 0;
+
+ return ufshcd_runtime_idle(hba);
+}
+#else /* !CONFIG_PM_RUNTIME */
+#define ufshcd_pci_runtime_suspend NULL
+#define ufshcd_pci_runtime_resume NULL
+#define ufshcd_pci_runtime_idleNULL
+#endif /* CONFIG_PM_RUNTIME */
+
/**
* ufshcd_pci_shutdown - main function to put the controller in reset state
* @pdev: pointer to PCI device handle
@@ -91,6 +132,9 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
{
struct ufs_hba *hba = pci_get_drvdata(pdev);
+ pm_runtime_forbid(pdev-dev);
+ pm_runtime_get_noresume(pdev-dev);
+
disable_irq(pdev-irq);
ufshcd_remove(hba);
pci_release_regions(pdev);
@@ -168,6 +212,8 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
}
pci_set_drvdata(pdev, hba);
+ pm_runtime_put_noidle(pdev-dev);
+ pm_runtime_allow(pdev-dev);
return 0;
@@ -182,6 +228,14 @@ out_error:
return err;
}
+static const struct dev_pm_ops ufshcd_pci_pm_ops = {
+ .suspend= ufshcd_pci_suspend,
+ .resume = ufshcd_pci_resume,
+ .runtime_suspend = ufshcd_pci_runtime_suspend,
+ .runtime_resume = ufshcd_pci_runtime_resume,
+ .runtime_idle= ufshcd_pci_runtime_idle,
+};
+
static DEFINE_PCI_DEVICE_TABLE(ufshcd_pci_tbl) = {
{ PCI_VENDOR_ID_SAMSUNG, 0xC00C, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
{ } /* terminate list */
@@ -195,10 +249,9 @@ static struct pci_driver ufshcd_pci_driver = {
.probe = ufshcd_pci_probe,
.remove = ufshcd_pci_remove,
.shutdown = ufshcd_pci_shutdown,
-#ifdef CONFIG_PM
- .suspend = ufshcd_pci_suspend,
- .resume = ufshcd_pci_resume,
-#endif
+ .driver = {
+ .pm = ufshcd_pci_pm_ops
+ },
};
module_pci_driver(ufshcd_pci_driver);
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index c42db40..b1f2605 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -34,6 +34,7 @@
*/
#include linux/platform_device.h
+#include linux/pm_runtime.h
#include ufshcd.h
@@ -87,6 +88,43 @@ static int ufshcd_pltfrm_resume(struct device *dev)
#define ufshcd_pltfrm_resume NULL
#endif
+#ifdef CONFIG_PM_RUNTIME
+static int ufshcd_pltfrm_runtime_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct ufs_hba *hba = platform_get_drvdata(pdev);
+
+ if (!hba)
+ return 0;
+
+ return ufshcd_runtime_suspend(hba);
+}
+static int ufshcd_pltfrm_runtime_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct
[PATCH V3 0/4] scsi: ufs: Improve UFS error handling
The first patch fixes many issues with current task management handling in UFSHCD driver. Others improve error handling in various scenarios. These patches depends on: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU [PATCH V3 2/2] scsi: ufs: Set fDeviceInit flag to initiate device initialization [PATCH V3 1/2] scsi: ufs: Add support for host assisted background operations [PATCH V3 2/2] scsi: ufs: Add runtime PM support for UFS host controller driver Changes from v2: - [PATCH V3 1/4]: Make the task management command task tag unique across SCSI/NOP/QUERY request tags. - [PATCH V3 3/4]: While handling device/host reset, wait for pending fatal handler to return if running. Changes from v1: - [PATCH V2 1/4]: Fix a race condition because of overloading outstanding_tasks variable to lock the slots. A new variable tm_slots_in_use will track which slots are in use by the driver. - [PATCH V2 2/4]: Commit text update to clarify the hardware race with more details. - [PATCH V2 3/4]: Minor cleanup and rebase - [PATCH V2 4/4]: Fix a bug - sleeping in atomic context Sujit Reddy Thumma (4): scsi: ufs: Fix broken task management command implementation scsi: ufs: Fix hardware race conditions while aborting a command scsi: ufs: Fix device and host reset methods scsi: ufs: Improve UFS fatal error handling drivers/scsi/ufs/ufshcd.c | 1035 + drivers/scsi/ufs/ufshcd.h | 12 +- drivers/scsi/ufs/ufshci.h | 19 +- 3 files changed, 873 insertions(+), 193 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. -- 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
Re: [PATCH V3 2/2] scsi: ufs: Set fDeviceInit flag to initiate device initialization
Tested-by: Maya Erez me...@codeaurora.org
From: Dolev Raviv dra...@codeaurora.org
Allow UFS device to complete its initialization and accept
SCSI commands by setting fDeviceInit flag. The device may take
time for this operation and hence the host should poll until
fDeviceInit flag is toggled to zero. This step is mandated by
UFS device specification for device initialization completion.
Signed-off-by: Dolev Raviv dra...@codeaurora.org
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufs.h| 88 +-
drivers/scsi/ufs/ufshcd.c | 292
-
drivers/scsi/ufs/ufshcd.h | 14 ++
drivers/scsi/ufs/ufshci.h |2 +-
4 files changed, 390 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 14c0a4e..db5bde4 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -43,6 +43,8 @@
#define GENERAL_UPIU_REQUEST_SIZE 32
#define UPIU_HEADER_DATA_SEGMENT_MAX_SIZE((ALIGNED_UPIU_SIZE) - \
(GENERAL_UPIU_REQUEST_SIZE))
+#define QUERY_OSF_SIZE ((GENERAL_UPIU_REQUEST_SIZE) - \
+ (sizeof(struct utp_upiu_header)))
#define UPIU_HEADER_DWORD(byte3, byte2, byte1, byte0)\
cpu_to_be32((byte3 24) | (byte2 16) |\
@@ -68,7 +70,7 @@ enum {
UPIU_TRANSACTION_COMMAND= 0x01,
UPIU_TRANSACTION_DATA_OUT = 0x02,
UPIU_TRANSACTION_TASK_REQ = 0x04,
- UPIU_TRANSACTION_QUERY_REQ = 0x26,
+ UPIU_TRANSACTION_QUERY_REQ = 0x16,
};
/* UTP UPIU Transaction Codes Target to Initiator */
@@ -97,8 +99,19 @@ enum {
UPIU_TASK_ATTR_ACA = 0x03,
};
-/* UTP QUERY Transaction Specific Fields OpCode */
+/* UPIU Query request function */
enum {
+ UPIU_QUERY_FUNC_STANDARD_READ_REQUEST = 0x01,
+ UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST = 0x81,
+};
+
+/* Flag idn for Query Requests*/
+enum flag_idn {
+ QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
+};
+
+/* 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,
@@ -110,6 +123,21 @@ enum {
UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
};
+/* Query response result code */
+enum {
+ QUERY_RESULT_SUCCESS= 0x00,
+ QUERY_RESULT_NOT_READABLE = 0xF6,
+ QUERY_RESULT_NOT_WRITEABLE = 0xF7,
+ QUERY_RESULT_ALREADY_WRITTEN= 0xF8,
+ QUERY_RESULT_INVALID_LENGTH = 0xF9,
+ QUERY_RESULT_INVALID_VALUE = 0xFA,
+ QUERY_RESULT_INVALID_SELECTOR = 0xFB,
+ QUERY_RESULT_INVALID_INDEX = 0xFC,
+ QUERY_RESULT_INVALID_IDN= 0xFD,
+ QUERY_RESULT_INVALID_OPCODE = 0xFE,
+ QUERY_RESULT_GENERAL_FAILURE= 0xFF,
+};
+
/* UTP Transfer Request Command Type (CT) */
enum {
UPIU_COMMAND_SET_TYPE_SCSI = 0x0,
@@ -127,6 +155,7 @@ enum {
MASK_SCSI_STATUS= 0xFF,
MASK_TASK_RESPONSE = 0xFF00,
MASK_RSP_UPIU_RESULT= 0x,
+ MASK_QUERY_DATA_SEG_LEN = 0x,
};
/* Task management service response */
@@ -160,13 +189,40 @@ struct utp_upiu_cmd {
};
/**
+ * struct utp_upiu_query - upiu request buffer structure for
+ * query request.
+ * @opcode: command to perform B-0
+ * @idn: a value that indicates the particular type of data B-1
+ * @index: Index to further identify data B-2
+ * @selector: Index to further identify data B-3
+ * @reserved_osf: spec reserved field B-4,5
+ * @length: number of descriptor bytes to read/write B-6,7
+ * @value: Attribute value to be written DW-5
+ * @reserved: spec reserved DW-6,7
+ */
+struct utp_upiu_query {
+ u8 opcode;
+ u8 idn;
+ u8 index;
+ u8 selector;
+ u16 reserved_osf;
+ u16 length;
+ u32 value;
+ u32 reserved[2];
+};
+
+/**
* struct utp_upiu_req - general upiu request structure
* @header:UPIU header structure DW-0 to DW-2
* @sc: fields structure for scsi command DW-3 to DW-7
+ * @qr: fields structure for query request DW-3 to DW-7
*/
struct utp_upiu_req {
struct utp_upiu_header header;
- struct utp_upiu_cmd sc;
+ union {
+ struct utp_upiu_cmd sc;
+ struct utp_upiu_query qr;
+ };
};
/**
@@ -187,10 +243,14 @@ struct utp_cmd_rsp {
* struct utp_upiu_rsp - general upiu response structure
* @header: UPIU header structure DW-0 to DW-2
* @sr: fields structure for scsi command DW-3 to DW-12
+ * @qr: fields structure for query request DW-3 to DW-7
*/
struct utp_upiu_rsp {
struct utp_upiu_header header;
- struct utp_cmd_rsp sr;
+ union {
+ struct utp_cmd_rsp sr;
+
Re: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
Tested-by: Maya Erez me...@codeaurora.org
As part of device initialization sequence, sending NOP OUT UPIU and
waiting for NOP IN UPIU response is mandatory. This confirms that the
device UFS Transport (UTP) layer is functional and the host can configure
the device with further commands. Add support for sending NOP OUT UPIU to
check the device connection path and test whether the UTP layer on the
device side is functional during initialization.
A tag is acquired from the SCSI tag map space in order to send the device
management command. When the tag is acquired by internal command the scsi
command is rejected with host busy flag in order to requeue the request.
To avoid frequent collisions between internal commands and scsi commands
the device management command tag is allocated in the opposite direction
w.r.t block layer tag allocation.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
Signed-off-by: Dolev Raviv dra...@codeaurora.org
---
drivers/scsi/ufs/ufs.h| 43 +++-
drivers/scsi/ufs/ufshcd.c | 596
+
drivers/scsi/ufs/ufshcd.h | 29 ++-
3 files changed, 552 insertions(+), 116 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 139bc06..14c0a4e 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -36,10 +36,16 @@
#ifndef _UFS_H
#define _UFS_H
+#include linux/mutex.h
+#include linux/types.h
+
#define MAX_CDB_SIZE 16
+#define GENERAL_UPIU_REQUEST_SIZE 32
+#define UPIU_HEADER_DATA_SEGMENT_MAX_SIZE((ALIGNED_UPIU_SIZE) - \
+ (GENERAL_UPIU_REQUEST_SIZE))
#define UPIU_HEADER_DWORD(byte3, byte2, byte1, byte0)\
- ((byte3 24) | (byte2 16) |\
+ cpu_to_be32((byte3 24) | (byte2 16) |\
(byte1 8) | (byte0))
/*
@@ -73,6 +79,7 @@ enum {
UPIU_TRANSACTION_TASK_RSP = 0x24,
UPIU_TRANSACTION_READY_XFER = 0x31,
UPIU_TRANSACTION_QUERY_RSP = 0x36,
+ UPIU_TRANSACTION_REJECT_UPIU= 0x3F,
};
/* UPIU Read/Write flags */
@@ -110,6 +117,12 @@ enum {
UPIU_COMMAND_SET_TYPE_QUERY = 0x2,
};
+/* UTP Transfer Request Command Offset */
+#define UPIU_COMMAND_TYPE_OFFSET 28
+
+/* Offset of the response code in the UPIU header */
+#define UPIU_RSP_CODE_OFFSET 8
+
enum {
MASK_SCSI_STATUS= 0xFF,
MASK_TASK_RESPONSE = 0xFF00,
@@ -138,26 +151,32 @@ struct utp_upiu_header {
/**
* struct utp_upiu_cmd - Command UPIU structure
- * @header: UPIU header structure DW-0 to DW-2
* @data_transfer_len: Data Transfer Length DW-3
* @cdb: Command Descriptor Block CDB DW-4 to DW-7
*/
struct utp_upiu_cmd {
- struct utp_upiu_header header;
u32 exp_data_transfer_len;
u8 cdb[MAX_CDB_SIZE];
};
/**
- * struct utp_upiu_rsp - Response UPIU structure
- * @header: UPIU header DW-0 to DW-2
+ * struct utp_upiu_req - general upiu request structure
+ * @header:UPIU header structure DW-0 to DW-2
+ * @sc: fields structure for scsi command DW-3 to DW-7
+ */
+struct utp_upiu_req {
+ struct utp_upiu_header header;
+ struct utp_upiu_cmd sc;
+};
+
+/**
+ * struct utp_cmd_rsp - Response UPIU structure
* @residual_transfer_count: Residual transfer count DW-3
* @reserved: Reserved double words DW-4 to DW-7
* @sense_data_len: Sense data length DW-8 U16
* @sense_data: Sense data field DW-8 to DW-12
*/
-struct utp_upiu_rsp {
- struct utp_upiu_header header;
+struct utp_cmd_rsp {
u32 residual_transfer_count;
u32 reserved[4];
u16 sense_data_len;
@@ -165,6 +184,16 @@ struct utp_upiu_rsp {
};
/**
+ * struct utp_upiu_rsp - general upiu response structure
+ * @header: UPIU header structure DW-0 to DW-2
+ * @sr: fields structure for scsi command DW-3 to DW-12
+ */
+struct utp_upiu_rsp {
+ struct utp_upiu_header header;
+ struct utp_cmd_rsp sr;
+};
+
+/**
* struct utp_upiu_task_req - Task request UPIU structure
* @header - UPIU header structure DW0 to DW-2
* @input_param1: Input parameter 1 DW-3
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..3f482b6 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -43,6 +43,11 @@
/* UIC command timeout, unit: ms */
#define UIC_CMD_TIMEOUT 500
+/* NOP OUT retries waiting for NOP IN response */
+#define NOP_OUT_RETRIES10
+/* Timeout after 30 msecs if NOP OUT hangs without response */
+#define NOP_OUT_TIMEOUT30 /* msecs */
+
enum {
UFSHCD_MAX_CHANNEL = 0,
UFSHCD_MAX_ID = 1,
@@ -71,6 +76,47 @@ enum {
INT_AGGR_CONFIG,
};
+/*
+ * ufshcd_wait_for_register - wait for register value to change
+ * @hba - per-adapter interface
+ * @reg - mmio register offset
+ * @mask - mask to apply to read register value
+ * @val - wait condition
+ *
Re: [PATCH V3 1/2] scsi: ufs: Add support for host assisted background operations
Tested-by: Maya Erez me...@codeaurora.org
Background operations in the UFS device can be disabled by
the host to reduce the response latency of transfer requests.
Add support for enabling/disabling the background operations
during runtime suspend/resume of the device.
If the device is in critical need of BKOPS it will raise an
URGENT_BKOPS exception which should be handled by the host to
make sure the device performs as expected.
During bootup, the BKOPS is enabled in the device by default.
The disable of BKOPS is supported only when the driver supports
runtime suspend/resume operations as the runtime PM framework
provides a way to determine the device idleness and hence BKOPS
can be managed effectively. During runtime resume the BKOPS is
disabled to reduce latency and during runtime suspend the BKOPS
is enabled to allow device to carry out idle time BKOPS.
In some cases where the BKOPS is disabled during runtime resume
and due to continuous data transfers the runtime suspend is not
triggered, the BKOPS is enabled when the device raises a level-2
exception (outstanding operations - performance impact).
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufs.h| 25 -
drivers/scsi/ufs/ufshcd.c | 338
+
drivers/scsi/ufs/ufshcd.h | 10 ++
3 files changed, 372 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index db5bde4..549a652 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -107,7 +107,29 @@ enum {
/* Flag idn for Query Requests*/
enum flag_idn {
- QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
+ QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
+ QUERY_FLAG_IDN_BKOPS_EN = 0x04,
+};
+
+/* Attribute idn for Query requests */
+enum attr_idn {
+ QUERY_ATTR_IDN_BKOPS_STATUS = 0x05,
+ QUERY_ATTR_IDN_EE_CONTROL = 0x0D,
+ QUERY_ATTR_IDN_EE_STATUS= 0x0E,
+};
+
+/* Exception event mask values */
+enum {
+ MASK_EE_STATUS = 0x,
+ MASK_EE_URGENT_BKOPS= (1 2),
+};
+
+/* Background operation status */
+enum {
+ BKOPS_STATUS_NO_OP = 0x0,
+ BKOPS_STATUS_NON_CRITICAL= 0x1,
+ BKOPS_STATUS_PERF_IMPACT = 0x2,
+ BKOPS_STATUS_CRITICAL= 0x3,
};
/* UTP QUERY Transaction Specific Fields OpCode */
@@ -156,6 +178,7 @@ enum {
MASK_TASK_RESPONSE = 0xFF00,
MASK_RSP_UPIU_RESULT= 0x,
MASK_QUERY_DATA_SEG_LEN = 0x,
+ MASK_RSP_EXCEPTION_EVENT = 0x1,
};
/* Task management service response */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 96ccb28..a25de66 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -268,6 +268,21 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp
*ucd_rsp_ptr)
}
/**
+ * ufshcd_is_exception_event - Check if the device raised an exception
event
+ * @ucd_rsp_ptr: pointer to response UPIU
+ *
+ * The function checks if the device raised an exception event indicated
in
+ * the Device Information field of response UPIU.
+ *
+ * Returns true if exception is raised, false otherwise.
+ */
+static inline bool ufshcd_is_exception_event(struct utp_upiu_rsp
*ucd_rsp_ptr)
+{
+ return be32_to_cpu(ucd_rsp_ptr-header.dword_2)
+ MASK_RSP_EXCEPTION_EVENT ? true : false;
+}
+
+/**
* ufshcd_config_int_aggr - Configure interrupt aggregation values.
* Currently there is no use case where we want to configure
* interrupt aggregation dynamically. So to configure interrupt
@@ -1174,6 +1189,86 @@ out_no_mem:
}
/**
+ * ufshcd_query_attr - Helper function for composing attribute requests
+ * hba: per-adapter instance
+ * opcode: attribute opcode
+ * idn: attribute idn to access
+ * index: index field
+ * selector: selector field
+ * attr_val: the attribute value after the query request completes
+ *
+ * Returns 0 for success, non-zero in case of failure
+*/
+int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
+ enum attr_idn idn, u8 index, u8 selector, u32 *attr_val)
+{
+ struct ufs_query_req *query;
+ struct ufs_query_res *response;
+ int err = -ENOMEM;
+
+ if (!attr_val) {
+ dev_err(hba-dev, %s: attribute value required for write
request\n,
+ __func__);
+ err = -EINVAL;
+ goto out;
+ }
+
+ query = kzalloc(sizeof(struct ufs_query_req), GFP_KERNEL);
+ if (!query) {
+ dev_err(hba-dev,
+ %s: Failed allocating ufs_query_req instance\n,
+ __func__);
+ goto out;
+ }
+
+ response = kzalloc(sizeof(struct ufs_query_res), GFP_KERNEL);
+ if (!response) {
+ dev_err(hba-dev,
+ %s:
Re: [PATCH V3 2/2] scsi: ufs: Add runtime PM support for UFS host controller driver
Tested-by: Maya Erez me...@codeaurora.org
Add runtime PM helpers to suspend/resume UFS controller at runtime.
Enable runtime PM by default for pci and platform drivers as the
initialized hardware can suspend if it is not used after bootup.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufshcd-pci.c| 65
++---
drivers/scsi/ufs/ufshcd-pltfrm.c | 51 +-
drivers/scsi/ufs/ufshcd.c|8 +
3 files changed, 117 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 48be39a..7bd8faa 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -35,6 +35,7 @@
#include ufshcd.h
#include linux/pci.h
+#include linux/pm_runtime.h
#ifdef CONFIG_PM
/**
@@ -44,7 +45,7 @@
*
* Returns -ENOSYS
*/
-static int ufshcd_pci_suspend(struct pci_dev *pdev, pm_message_t state)
+static int ufshcd_pci_suspend(struct device *dev)
{
/*
* TODO:
@@ -61,7 +62,7 @@ static int ufshcd_pci_suspend(struct pci_dev *pdev,
pm_message_t state)
*
* Returns -ENOSYS
*/
-static int ufshcd_pci_resume(struct pci_dev *pdev)
+static int ufshcd_pci_resume(struct device *dev)
{
/*
* TODO:
@@ -71,8 +72,48 @@ static int ufshcd_pci_resume(struct pci_dev *pdev)
return -ENOSYS;
}
+#else
+#define ufshcd_pci_suspend NULL
+#define ufshcd_pci_resumeNULL
#endif /* CONFIG_PM */
+#ifdef CONFIG_PM_RUNTIME
+static int ufshcd_pci_runtime_suspend(struct device *dev)
+{
+ struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+ struct ufs_hba *hba = pci_get_drvdata(pdev);
+
+ if (!hba)
+ return 0;
+
+ return ufshcd_runtime_suspend(hba);
+}
+static int ufshcd_pci_runtime_resume(struct device *dev)
+{
+ struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+ struct ufs_hba *hba = pci_get_drvdata(pdev);
+
+ if (!hba)
+ return 0;
+
+ return ufshcd_runtime_resume(hba);
+}
+static int ufshcd_pci_runtime_idle(struct device *dev)
+{
+ struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+ struct ufs_hba *hba = pci_get_drvdata(pdev);
+
+ if (!hba)
+ return 0;
+
+ return ufshcd_runtime_idle(hba);
+}
+#else /* !CONFIG_PM_RUNTIME */
+#define ufshcd_pci_runtime_suspend NULL
+#define ufshcd_pci_runtime_resumeNULL
+#define ufshcd_pci_runtime_idle NULL
+#endif /* CONFIG_PM_RUNTIME */
+
/**
* ufshcd_pci_shutdown - main function to put the controller in reset
state
* @pdev: pointer to PCI device handle
@@ -91,6 +132,9 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
{
struct ufs_hba *hba = pci_get_drvdata(pdev);
+ pm_runtime_forbid(pdev-dev);
+ pm_runtime_get_noresume(pdev-dev);
+
disable_irq(pdev-irq);
ufshcd_remove(hba);
pci_release_regions(pdev);
@@ -168,6 +212,8 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
}
pci_set_drvdata(pdev, hba);
+ pm_runtime_put_noidle(pdev-dev);
+ pm_runtime_allow(pdev-dev);
return 0;
@@ -182,6 +228,14 @@ out_error:
return err;
}
+static const struct dev_pm_ops ufshcd_pci_pm_ops = {
+ .suspend= ufshcd_pci_suspend,
+ .resume = ufshcd_pci_resume,
+ .runtime_suspend = ufshcd_pci_runtime_suspend,
+ .runtime_resume = ufshcd_pci_runtime_resume,
+ .runtime_idle= ufshcd_pci_runtime_idle,
+};
+
static DEFINE_PCI_DEVICE_TABLE(ufshcd_pci_tbl) = {
{ PCI_VENDOR_ID_SAMSUNG, 0xC00C, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
{ } /* terminate list */
@@ -195,10 +249,9 @@ static struct pci_driver ufshcd_pci_driver = {
.probe = ufshcd_pci_probe,
.remove = ufshcd_pci_remove,
.shutdown = ufshcd_pci_shutdown,
-#ifdef CONFIG_PM
- .suspend = ufshcd_pci_suspend,
- .resume = ufshcd_pci_resume,
-#endif
+ .driver = {
+ .pm = ufshcd_pci_pm_ops
+ },
};
module_pci_driver(ufshcd_pci_driver);
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c
b/drivers/scsi/ufs/ufshcd-pltfrm.c
index c42db40..b1f2605 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -34,6 +34,7 @@
*/
#include linux/platform_device.h
+#include linux/pm_runtime.h
#include ufshcd.h
@@ -87,6 +88,43 @@ static int ufshcd_pltfrm_resume(struct device *dev)
#define ufshcd_pltfrm_resume NULL
#endif
+#ifdef CONFIG_PM_RUNTIME
+static int ufshcd_pltfrm_runtime_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct ufs_hba *hba = platform_get_drvdata(pdev);
+
+ if (!hba)
+ return 0;
+
+ return ufshcd_runtime_suspend(hba);
+}
+static int ufshcd_pltfrm_runtime_resume(struct device *dev)
+{
+ struct
Re: [PATCH V3 1/4] scsi: ufs: Fix broken task management command implementation
Tested-by: Maya Erez me...@codeaurora.org
Currently, sending Task Management (TM) command to the card might
be broken in some scenarios as listed below:
Problem: If there are more than 8 TM commands the implementation
returns error to the caller.
Fix: Wait for one of the slots to be emptied and send the command.
Problem: Sometimes it is necessary for the caller to know the TM service
response code to determine the task status.
Fix: Propogate the service response to the caller.
Problem: If the TM command times out no proper error recovery is
implemented.
Fix: Clear the command in the controller door-bell register, so that
further commands for the same slot don't fail.
Problem: While preparing the TM command descriptor, the task tag used
should be unique across SCSI/NOP/QUERY/TM commands and not the
task tag of the command which the TM command is trying to manage.
Fix: Use a unique task tag instead of task tag of SCSI command.
Problem: Since the TM command involves H/W communication, abruptly ending
the request on kill interrupt signal might cause h/w malfunction.
Fix: Wait for hardware completion interrupt with TASK_UNINTERRUPTIBLE
set.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufshcd.c | 177
++---
drivers/scsi/ufs/ufshcd.h |8 ++-
2 files changed, 126 insertions(+), 59 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index af7d01d..a176421 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -53,6 +53,9 @@
/* Query request timeout */
#define QUERY_REQ_TIMEOUT 30 /* msec */
+/* Task management command timeout */
+#define TM_CMD_TIMEOUT 100 /* msecs */
+
/* Expose the flag value from utp_upiu_query.value */
#define MASK_QUERY_UPIU_FLAG_LOC 0xFF
@@ -190,13 +193,35 @@ ufshcd_get_tmr_ocs(struct utp_task_req_desc
*task_req_descp)
/**
* ufshcd_get_tm_free_slot - get a free slot for task management request
* @hba: per adapter instance
+ * @free_slot: pointer to variable with available slot value
*
- * Returns maximum number of task management request slots in case of
- * task management queue full or returns the free slot number
+ * Get a free tag and lock it until ufshcd_put_tm_slot() is called.
+ * Returns 0 if free slot is not available, else return 1 with tag value
+ * in @free_slot.
*/
-static inline int ufshcd_get_tm_free_slot(struct ufs_hba *hba)
+static bool ufshcd_get_tm_free_slot(struct ufs_hba *hba, int *free_slot)
+{
+ int tag;
+ bool ret = false;
+
+ if (!free_slot)
+ goto out;
+
+ do {
+ tag = find_first_zero_bit(hba-tm_slots_in_use, hba-nutmrs);
+ if (tag = hba-nutmrs)
+ goto out;
+ } while (test_and_set_bit_lock(tag, hba-tm_slots_in_use));
+
+ *free_slot = tag;
+ ret = true;
+out:
+ return ret;
+}
+
+static inline void ufshcd_put_tm_slot(struct ufs_hba *hba, int slot)
{
- return find_first_zero_bit(hba-outstanding_tasks, hba-nutmrs);
+ clear_bit_unlock(slot, hba-tm_slots_in_use);
}
/**
@@ -1778,10 +1803,11 @@ static void ufshcd_slave_destroy(struct
scsi_device *sdev)
* ufshcd_task_req_compl - handle task management request completion
* @hba: per adapter instance
* @index: index of the completed request
+ * @resp: task management service response
*
- * Returns SUCCESS/FAILED
+ * Returns non-zero value on error, zero on success
*/
-static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index)
+static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8
*resp)
{
struct utp_task_req_desc *task_req_descp;
struct utp_upiu_task_rsp *task_rsp_upiup;
@@ -1802,19 +1828,15 @@ static int ufshcd_task_req_compl(struct ufs_hba
*hba, u32 index)
task_req_descp[index].task_rsp_upiu;
task_result = be32_to_cpu(task_rsp_upiup-header.dword_1);
task_result = ((task_result MASK_TASK_RESPONSE) 8);
-
- if (task_result != UPIU_TASK_MANAGEMENT_FUNC_COMPL
- task_result != UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED)
- task_result = FAILED;
- else
- task_result = SUCCESS;
+ if (resp)
+ *resp = (u8)task_result;
} else {
- task_result = FAILED;
- dev_err(hba-dev,
- trc: Invalid ocs = %x\n, ocs_value);
+ dev_err(hba-dev, %s: failed, ocs = 0x%x\n,
+ __func__, ocs_value);
}
spin_unlock_irqrestore(hba-host-host_lock, flags);
- return task_result;
+
+ return ocs_value;
}
/**
@@ -2298,7 +2320,7 @@ static void ufshcd_tmc_handler(struct ufs_hba *hba)
Re: [PATCH V3 2/4] scsi: ufs: Fix hardware race conditions while aborting a command
Tested-by: Maya Erez me...@codeaurora.org
There is a possible race condition in the hardware when the abort
command is issued to terminate the ongoing SCSI command as described
below:
- A bit in the door-bell register is set in the controller for a
new SCSI command.
- In some rare situations, before controller get a chance to issue
the command to the device, the software issued an abort command.
- If the device recieves abort command first then it returns success
because the command itself is not present.
- Now if the controller commits the command to device it will be
processed.
- Software thinks that command is aborted and proceed while still
the device is processing it.
- The software, controller and device may go out of sync because of
this race condition.
To avoid this, query task presence in the device before sending abort
task command so that after the abort operation, the command is guaranteed
to be non-existent in both controller and the device.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufshcd.c | 70
+++-
1 files changed, 55 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index a176421..51ce096 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2550,6 +2550,12 @@ static int ufshcd_host_reset(struct scsi_cmnd *cmd)
* ufshcd_abort - abort a specific command
* @cmd: SCSI command pointer
*
+ * Abort the pending command in device by sending UFS_ABORT_TASK task
management
+ * command, and in host controller by clearing the door-bell register.
There can
+ * be race between controller sending the command to the device while
abort is
+ * issued. To avoid that, first issue UFS_QUERY_TASK to check if the
command is
+ * really issued and then try to abort it.
+ *
* Returns SUCCESS/FAILED
*/
static int ufshcd_abort(struct scsi_cmnd *cmd)
@@ -2558,7 +2564,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
struct ufs_hba *hba;
unsigned long flags;
unsigned int tag;
- int err;
+ int err = 0;
+ int poll_cnt;
u8 resp;
struct ufshcd_lrb *lrbp;
@@ -2566,33 +2573,59 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
hba = shost_priv(host);
tag = cmd-request-tag;
- spin_lock_irqsave(host-host_lock, flags);
+ /* If command is already aborted/completed, return SUCCESS */
+ if (!(test_bit(tag, hba-outstanding_reqs)))
+ goto out;
- /* check if command is still pending */
- if (!(test_bit(tag, hba-outstanding_reqs))) {
- err = FAILED;
- spin_unlock_irqrestore(host-host_lock, flags);
+ lrbp = hba-lrb[tag];
+ for (poll_cnt = 100; poll_cnt; poll_cnt--) {
+ err = ufshcd_issue_tm_cmd(hba, lrbp-lun, lrbp-task_tag,
+ UFS_QUERY_TASK, resp);
+ if (!err resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
+ /* cmd pending in the device */
+ break;
+ } else if (!err resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
+ u32 reg;
+
+ /*
+ * cmd not pending in the device, check if it is
+ * in transition.
+ */
+ reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
+ if (reg (1 tag)) {
+ /* sleep for max. 2ms to stabilize */
+ usleep_range(1000, 2000);
+ continue;
+ }
+ /* command completed already */
+ goto out;
+ } else {
+ if (!err)
+ err = resp; /* service response error */
+ goto out;
+ }
+ }
+
+ if (!poll_cnt) {
+ err = -EBUSY;
goto out;
}
- spin_unlock_irqrestore(host-host_lock, flags);
- lrbp = hba-lrb[tag];
err = ufshcd_issue_tm_cmd(hba, lrbp-lun, lrbp-task_tag,
UFS_ABORT_TASK, resp);
if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
- err = FAILED;
+ if (!err)
+ err = resp; /* service response error */
goto out;
- } else {
- err = SUCCESS;
}
+ err = ufshcd_clear_cmd(hba, tag);
+ if (err)
+ goto out;
+
scsi_dma_unmap(cmd);
spin_lock_irqsave(host-host_lock, flags);
-
- /* clear the respective UTRLCLR register bit */
- ufshcd_utrl_clear(hba, tag);
-
__clear_bit(tag, hba-outstanding_reqs);
hba-lrb[tag].cmd = NULL;
spin_unlock_irqrestore(host-host_lock, flags);
@@ -2600,6 +2633,13 @@ static int ufshcd_abort(struct scsi_cmnd
Re: [PATCH V3 3/4] scsi: ufs: Fix device and host reset methods
Tested with error injection.
Tested-by: Maya Erez me...@codeaurora.org
As of now SCSI initiated error handling is broken because,
the reset APIs don't try to bring back the device initialized and
ready for further transfers.
In case of timeouts, the scsi error handler takes care of handling aborts
and resets. Improve the error handling in such scenario by resetting the
device and host and re-initializing them in proper manner.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufshcd.c | 467
+++--
drivers/scsi/ufs/ufshcd.h |2 +
2 files changed, 411 insertions(+), 58 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 51ce096..b4c9910 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -69,9 +69,15 @@ enum {
/* UFSHCD states */
enum {
- UFSHCD_STATE_OPERATIONAL,
UFSHCD_STATE_RESET,
UFSHCD_STATE_ERROR,
+ UFSHCD_STATE_OPERATIONAL,
+};
+
+/* UFSHCD error handling flags */
+enum {
+ UFSHCD_EH_HOST_RESET_PENDING = (1 0),
+ UFSHCD_EH_DEVICE_RESET_PENDING = (1 1),
};
/* Interrupt configuration options */
@@ -87,6 +93,22 @@ enum {
INT_AGGR_CONFIG,
};
+#define ufshcd_set_device_reset_pending(h) \
+ (h-eh_flags |= UFSHCD_EH_DEVICE_RESET_PENDING)
+#define ufshcd_set_host_reset_pending(h) \
+ (h-eh_flags |= UFSHCD_EH_HOST_RESET_PENDING)
+#define ufshcd_device_reset_pending(h) \
+ (h-eh_flags UFSHCD_EH_DEVICE_RESET_PENDING)
+#define ufshcd_host_reset_pending(h) \
+ (h-eh_flags UFSHCD_EH_HOST_RESET_PENDING)
+#define ufshcd_clear_device_reset_pending(h) \
+ (h-eh_flags = ~UFSHCD_EH_DEVICE_RESET_PENDING)
+#define ufshcd_clear_host_reset_pending(h) \
+ (h-eh_flags = ~UFSHCD_EH_HOST_RESET_PENDING)
+
+static void ufshcd_tmc_handler(struct ufs_hba *hba);
+static void ufshcd_async_scan(void *data, async_cookie_t cookie);
+
/*
* ufshcd_wait_for_register - wait for register value to change
* @hba - per-adapter interface
@@ -851,9 +873,22 @@ static int ufshcd_queuecommand(struct Scsi_Host
*host, struct scsi_cmnd *cmd)
tag = cmd-request-tag;
- if (hba-ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
+ switch (hba-ufshcd_state) {
+ case UFSHCD_STATE_OPERATIONAL:
+ break;
+ case UFSHCD_STATE_RESET:
err = SCSI_MLQUEUE_HOST_BUSY;
goto out;
+ case UFSHCD_STATE_ERROR:
+ set_host_byte(cmd, DID_ERROR);
+ cmd-scsi_done(cmd);
+ goto out;
+ default:
+ dev_WARN_ONCE(hba-dev, 1, %s: invalid state %d\n,
+ __func__, hba-ufshcd_state);
+ set_host_byte(cmd, DID_BAD_TARGET);
+ cmd-scsi_done(cmd);
+ goto out;
}
/* acquire the tag to make sure device cmds don't use it */
@@ -1573,8 +1608,6 @@ static int ufshcd_make_hba_operational(struct
ufs_hba *hba)
if (hba-ufshcd_state == UFSHCD_STATE_RESET)
scsi_unblock_requests(hba-host);
- hba-ufshcd_state = UFSHCD_STATE_OPERATIONAL;
-
out:
return err;
}
@@ -2273,6 +2306,106 @@ out:
}
/**
+ * ufshcd_utrl_is_rsr_enabled - check if run-stop register is enabled
+ * @hba: per-adapter instance
+ */
+static bool ufshcd_utrl_is_rsr_enabled(struct ufs_hba *hba)
+{
+ return ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_LIST_RUN_STOP) 0x1;
+}
+
+/**
+ * ufshcd_utmrl_is_rsr_enabled - check if run-stop register is enabled
+ * @hba: per-adapter instance
+ */
+static bool ufshcd_utmrl_is_rsr_enabled(struct ufs_hba *hba)
+{
+ return ufshcd_readl(hba, REG_UTP_TASK_REQ_LIST_RUN_STOP) 0x1;
+}
+
+/**
+ * ufshcd_complete_pending_tasks - complete outstanding tasks
+ * @hba: per adapter instance
+ *
+ * Abort in-progress task management commands and wakeup
+ * waiting threads.
+ *
+ * Returns non-zero error value when failed to clear all the commands.
+ */
+static int ufshcd_complete_pending_tasks(struct ufs_hba *hba)
+{
+ u32 reg;
+ int err = 0;
+ unsigned long flags;
+
+ if (!hba-outstanding_tasks)
+ goto out;
+
+ /* Clear UTMRL only when run-stop is enabled */
+ if (ufshcd_utmrl_is_rsr_enabled(hba))
+ ufshcd_writel(hba, ~hba-outstanding_tasks,
+ REG_UTP_TASK_REQ_LIST_CLEAR);
+
+ /* poll for max. 1 sec to clear door bell register by h/w */
+ reg = ufshcd_wait_for_register(hba,
+ REG_UTP_TASK_REQ_DOOR_BELL,
+ hba-outstanding_tasks, 0, 1000, 1000);
+ if (reg hba-outstanding_tasks)
+ err = -ETIMEDOUT;
+
+ spin_lock_irqsave(hba-host-host_lock, flags);
+ /* complete commands that were cleared out */
+ ufshcd_tmc_handler(hba);
+ spin_unlock_irqrestore(hba-host-host_lock, flags);
+out:
+ if (err)
+
Re: [PATCH V3 4/4] scsi: ufs: Improve UFS fatal error handling
Tested with error injection.
Tested-by: Maya Erez me...@codeaurora.org
Error handling in UFS driver is broken and resets the host controller
for fatal errors without re-initialization. Correct the fatal error
handling sequence according to UFS Host Controller Interface (HCI)
v1.1 specification.
o Upon determining fatal error condition the host controller may hang
forever until a reset is applied, so just retrying the command doesn't
work without a reset. So, the reset is applied in the driver context
in a separate work and SCSI mid-layer isn't informed until reset is
applied.
o Processed requests which are completed without error are reported to
SCSI layer as successful and any pending commands that are not started
yet or are not cause of the error are re-queued into scsi midlayer
queue.
For the command that caused error, host controller or device is reset
and DID_ERROR is returned for command retry after applying reset.
o SCSI is informed about the expected Unit-Attentioni exception from the
device for the immediate command after a reset so that the SCSI layer
take necessary steps to establish communication with the device.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufshcd.c | 349
+++-
drivers/scsi/ufs/ufshcd.h |2 +
drivers/scsi/ufs/ufshci.h | 19 ++-
3 files changed, 295 insertions(+), 75 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b4c9910..2a3874f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -80,6 +80,14 @@ enum {
UFSHCD_EH_DEVICE_RESET_PENDING = (1 1),
};
+/* UFSHCD UIC layer error flags */
+enum {
+ UFSHCD_UIC_DL_PA_INIT_ERROR = (1 0), /* Data link layer error */
+ UFSHCD_UIC_NL_ERROR = (1 1), /* Network layer error */
+ UFSHCD_UIC_TL_ERROR = (1 2), /* Transport Layer error */
+ UFSHCD_UIC_DME_ERROR = (1 3), /* DME error */
+};
+
/* Interrupt configuration options */
enum {
UFSHCD_INT_DISABLE,
@@ -108,6 +116,7 @@ enum {
static void ufshcd_tmc_handler(struct ufs_hba *hba);
static void ufshcd_async_scan(void *data, async_cookie_t cookie);
+static int ufshcd_reset_and_restore(struct ufs_hba *hba);
/*
* ufshcd_wait_for_register - wait for register value to change
@@ -1605,9 +1614,6 @@ static int ufshcd_make_hba_operational(struct
ufs_hba *hba)
goto out;
}
- if (hba-ufshcd_state == UFSHCD_STATE_RESET)
- scsi_unblock_requests(hba-host);
-
out:
return err;
}
@@ -1733,66 +1739,6 @@ static int ufshcd_validate_dev_connection(struct
ufs_hba *hba)
}
/**
- * ufshcd_do_reset - reset the host controller
- * @hba: per adapter instance
- *
- * Returns SUCCESS/FAILED
- */
-static int ufshcd_do_reset(struct ufs_hba *hba)
-{
- struct ufshcd_lrb *lrbp;
- unsigned long flags;
- int tag;
-
- /* block commands from midlayer */
- scsi_block_requests(hba-host);
-
- spin_lock_irqsave(hba-host-host_lock, flags);
- hba-ufshcd_state = UFSHCD_STATE_RESET;
-
- /* send controller to reset state */
- ufshcd_hba_stop(hba);
- spin_unlock_irqrestore(hba-host-host_lock, flags);
-
- /* abort outstanding commands */
- for (tag = 0; tag hba-nutrs; tag++) {
- if (test_bit(tag, hba-outstanding_reqs)) {
- lrbp = hba-lrb[tag];
- if (lrbp-cmd) {
- scsi_dma_unmap(lrbp-cmd);
- lrbp-cmd-result = DID_RESET 16;
- lrbp-cmd-scsi_done(lrbp-cmd);
- lrbp-cmd = NULL;
- clear_bit_unlock(tag, hba-lrb_in_use);
- }
- }
- }
-
- /* complete device management command */
- if (hba-dev_cmd.complete)
- complete(hba-dev_cmd.complete);
-
- /* clear outstanding request/task bit maps */
- hba-outstanding_reqs = 0;
- hba-outstanding_tasks = 0;
-
- /* Host controller enable */
- if (ufshcd_hba_enable(hba)) {
- dev_err(hba-dev,
- Reset: Controller initialization failed\n);
- return FAILED;
- }
-
- if (ufshcd_link_startup(hba)) {
- dev_err(hba-dev,
- Reset: Link start-up failed\n);
- return FAILED;
- }
-
- return SUCCESS;
-}
-
-/**
* ufshcd_slave_alloc - handle initial SCSI device configurations
* @sdev: pointer to SCSI device
*
@@ -1809,6 +1755,9 @@ static int ufshcd_slave_alloc(struct scsi_device
*sdev)
sdev-use_10_for_ms = 1;
scsi_set_tag_type(sdev, MSG_SIMPLE_TAG);
+ /* allow SCSI layer to restart the device in case of errors */
+ sdev-allow_restart = 1;
+
/*
* Inform SCSI Midlayer that the LUN queue depth is same
Re: [scsi:misc 34/37] include/uapi/linux/swab.h:106:23: warning: 'csum' may be used uninitialized in this function
On Tue, 2013-07-09 at 17:38 +0800, kbuild test robot wrote: tree: git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git misc head: 06ba9fa7f8cec5b9c04e511bfe21c5aa7cc3044c commit: eb7b211bac8e0aad45c2349a853be13cef20048a [34/37] [SCSI] scsi_debug: reduce duplication between prot_verify_read and prot_verify_write config: x86_64-randconfig-x027-0707 (attached as .config) Note: it may well be a FALSE warning. FWIW you are at least aware of it now. http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings All warnings: In file included from include/linux/swab.h:4:0, from include/uapi/linux/byteorder/little_endian.h:12, from include/linux/byteorder/little_endian.h:4, from arch/x86/include/uapi/asm/byteorder.h:4, from include/asm-generic/bitops/le.h:5, from arch/x86/include/asm/bitops.h:516, from include/linux/bitops.h:22, from include/linux/kernel.h:10, from include/linux/cache.h:4, from include/linux/time.h:4, from include/linux/stat.h:18, from include/linux/module.h:10, from drivers/scsi/scsi_debug.c:28: drivers/scsi/scsi_debug.c: In function 'dif_verify': include/uapi/linux/swab.h:106:23: warning: 'csum' may be used uninitialized in this function [-Wmaybe-uninitialized] (__builtin_constant_p((__u16)(x)) ? \ ^ drivers/scsi/scsi_debug.c:1715:6: note: 'csum' was declared here u16 csum; ^ This is probably because the randconfig has CONFIG_BUG=n, so the default case is a nop. The original had an initialiser in there, so the fix is either to drop the impossible default case or add the initialiser. James -- 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 0/8] libfc, libfcoe, fcoe updates for 3.11(+)
The following series implements a few random fixes. I know this series may be a bit late for the merge window, but they're fairly simple fixes and I think they should be fine for RC if I've missed the merge window. --- Bart Van Assche (1): fcoe: Reduce number of sparse warnings Mark Rustad (2): libfc: Reject PLOGI from nodes with incompatible role fcoe: Stop fc_rport_priv structure leak Neerav Parikh (1): fcoe: Fix smatch warning in fcoe_fdmi_info function Robert Love (3): libfc: Remove extra space in fc_exch_timer_cancel definition libfc: Differentiate echange timer cancellation debug statements libfcoe: Fix meaningless log statement Yi Zou (1): fcoe: fix the link error status block sparse warnings drivers/scsi/fcoe/fcoe.c | 26 +++--- drivers/scsi/fcoe/fcoe_ctlr.c |4 drivers/scsi/fcoe/fcoe_sysfs.c | 24 drivers/scsi/fcoe/fcoe_transport.c | 28 ++-- drivers/scsi/libfc/fc_exch.c |4 ++-- drivers/scsi/libfc/fc_rport.c | 27 +++ 6 files changed, 66 insertions(+), 47 deletions(-) -- Thanks, //Rob -- 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 2/8] fcoe: Fix smatch warning in fcoe_fdmi_info function
From: Neerav Parikh neerav.par...@intel.com
This patch fixes a smatch warning as below:
smatch warnings:
drivers/scsi/fcoe/fcoe.c:782 fcoe_fdmi_info() warn: 'fdmi' puts 896 bytes on
stack
Reported-by: Fengguang Wu fengguang...@intel.com
Signed-off-by: Neerav Parikh neerav.par...@intel.com
Tested-by: Jack Morgan jack.mor...@intel.com
Signed-off-by: Robert Love robert.w.l...@intel.com
---
drivers/scsi/fcoe/fcoe.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 32ae6c6..3336e57 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -774,7 +774,6 @@ static void fcoe_fdmi_info(struct fc_lport *lport, struct
net_device *netdev)
struct fcoe_port *port;
struct net_device *realdev;
int rc;
- struct netdev_fcoe_hbainfo fdmi;
port = lport_priv(lport);
fcoe = port-priv;
@@ -788,9 +787,13 @@ static void fcoe_fdmi_info(struct fc_lport *lport, struct
net_device *netdev)
return;
if (realdev-netdev_ops-ndo_fcoe_get_hbainfo) {
- memset(fdmi, 0, sizeof(fdmi));
+ struct netdev_fcoe_hbainfo *fdmi;
+ fdmi = kzalloc(sizeof(*fdmi), GFP_KERNEL);
+ if (!fdmi)
+ return;
+
rc = realdev-netdev_ops-ndo_fcoe_get_hbainfo(realdev,
- fdmi);
+ fdmi);
if (rc) {
printk(KERN_INFO fcoe: Failed to retrieve FDMI
information from netdev.\n);
@@ -800,38 +803,39 @@ static void fcoe_fdmi_info(struct fc_lport *lport, struct
net_device *netdev)
snprintf(fc_host_serial_number(lport-host),
FC_SERIAL_NUMBER_SIZE,
%s,
-fdmi.serial_number);
+fdmi-serial_number);
snprintf(fc_host_manufacturer(lport-host),
FC_SERIAL_NUMBER_SIZE,
%s,
-fdmi.manufacturer);
+fdmi-manufacturer);
snprintf(fc_host_model(lport-host),
FC_SYMBOLIC_NAME_SIZE,
%s,
-fdmi.model);
+fdmi-model);
snprintf(fc_host_model_description(lport-host),
FC_SYMBOLIC_NAME_SIZE,
%s,
-fdmi.model_description);
+fdmi-model_description);
snprintf(fc_host_hardware_version(lport-host),
FC_VERSION_STRING_SIZE,
%s,
-fdmi.hardware_version);
+fdmi-hardware_version);
snprintf(fc_host_driver_version(lport-host),
FC_VERSION_STRING_SIZE,
%s,
-fdmi.driver_version);
+fdmi-driver_version);
snprintf(fc_host_optionrom_version(lport-host),
FC_VERSION_STRING_SIZE,
%s,
-fdmi.optionrom_version);
+fdmi-optionrom_version);
snprintf(fc_host_firmware_version(lport-host),
FC_VERSION_STRING_SIZE,
%s,
-fdmi.firmware_version);
+fdmi-firmware_version);
/* Enable FDMI lport states */
lport-fdmi_enabled = 1;
+ kfree(fdmi);
} else {
lport-fdmi_enabled = 0;
printk(KERN_INFO fcoe: No FDMI support.\n);
--
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 3/8] fcoe: fix the link error status block sparse warnings
From: Yi Zou yi@intel.com
Both fcoe_fc_els_lesb and fc_els_lesb are in __be32 already, and both are
exactly the same size in bytes, with somewhat different member names to
reflect the fact the former is for Ethernet media the latter is for Fiber
Channel, so, remove conversion and use __be32 directly. This fixes the warning
from sparse check.
Signed-off-by: Yi Zou yi@intel.com
Reported-by: Fengguang Wu fengguang...@intel.com
Tested-by: Jack Morgan jack.mor...@intel.com
Signed-off-by: Robert Love robert.w.l...@intel.com
---
drivers/scsi/fcoe/fcoe_transport.c | 22 --
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe_transport.c
b/drivers/scsi/fcoe/fcoe_transport.c
index f3a5a53..bedd422 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -180,24 +180,10 @@ void fcoe_ctlr_get_lesb(struct fcoe_ctlr_device *ctlr_dev)
{
struct fcoe_ctlr *fip = fcoe_ctlr_device_priv(ctlr_dev);
struct net_device *netdev = fcoe_get_netdev(fip-lp);
- struct fcoe_fc_els_lesb *fcoe_lesb;
- struct fc_els_lesb fc_lesb;
-
- __fcoe_get_lesb(fip-lp, fc_lesb, netdev);
- fcoe_lesb = (struct fcoe_fc_els_lesb *)(fc_lesb);
-
- ctlr_dev-lesb.lesb_link_fail =
- ntohl(fcoe_lesb-lesb_link_fail);
- ctlr_dev-lesb.lesb_vlink_fail =
- ntohl(fcoe_lesb-lesb_vlink_fail);
- ctlr_dev-lesb.lesb_miss_fka =
- ntohl(fcoe_lesb-lesb_miss_fka);
- ctlr_dev-lesb.lesb_symb_err =
- ntohl(fcoe_lesb-lesb_symb_err);
- ctlr_dev-lesb.lesb_err_block =
- ntohl(fcoe_lesb-lesb_err_block);
- ctlr_dev-lesb.lesb_fcs_error =
- ntohl(fcoe_lesb-lesb_fcs_error);
+ struct fc_els_lesb *fc_lesb;
+
+ fc_lesb = (struct fc_els_lesb *)(ctlr_dev-lesb);
+ __fcoe_get_lesb(fip-lp, fc_lesb, netdev);
}
EXPORT_SYMBOL_GPL(fcoe_ctlr_get_lesb);
--
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 5/8] libfc: Differentiate echange timer cancellation debug statements
There are two debug statements with the same output string regarding
echange timer cancellation. This patch simply changes the output of
one string so that they can be differentiated.
Signed-off-by: Robert Love robert.w.l...@intel.com
Tested-by: Jack Morgan jack.mor...@intel.com
Acked-by: Neil Horman nhor...@tuxdriver.com
---
drivers/scsi/libfc/fc_exch.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index e98ea6a..5879929 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -1567,7 +1567,7 @@ static void fc_exch_abts_resp(struct fc_exch *ep, struct
fc_frame *fp)
fc_exch_rctl_name(fh-fh_r_ctl));
if (cancel_delayed_work_sync(ep-timeout_work)) {
- FC_EXCH_DBG(ep, Exchange timer canceled\n);
+ FC_EXCH_DBG(ep, Exchange timer canceled due to ABTS
response\n);
fc_exch_release(ep);/* release from pending timer hold */
}
--
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 4/8] libfc: Remove extra space in fc_exch_timer_cancel definition
Simply remove an extra space that violates coding style.
Signed-off-by: Robert Love robert.w.l...@intel.com
Tested-by: Jack Morgan jack.mor...@intel.com
Acked-by: Neil Horman nhor...@tuxdriver.com
---
drivers/scsi/libfc/fc_exch.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 8b928c6..e98ea6a 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -337,7 +337,7 @@ static void fc_exch_release(struct fc_exch *ep)
* fc_exch_timer_cancel() - cancel exch timer
* @ep:The exchange whose timer to be canceled
*/
-static inline void fc_exch_timer_cancel(struct fc_exch *ep)
+static inline void fc_exch_timer_cancel(struct fc_exch *ep)
{
if (cancel_delayed_work(ep-timeout_work)) {
FC_EXCH_DBG(ep, Exchange timer canceled\n);
--
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 1/8] libfc: Reject PLOGI from nodes with incompatible role
From: Mark Rustad mark.d.rus...@intel.com
Reject a PLOGI from a node with an incompatible role,
that is, initiator-to-initiator or target-to-target.
Signed-off-by: Mark Rustad mark.d.rus...@intel.com
Reviewed-by: Yi Zou yi@intel.com
Tested-by: Jack Morgan jack.mor...@intel.com
Signed-off-by: Robert Love robert.w.l...@intel.com
---
drivers/scsi/libfc/fc_rport.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
index 6bbb944..c710d90 100644
--- a/drivers/scsi/libfc/fc_rport.c
+++ b/drivers/scsi/libfc/fc_rport.c
@@ -926,6 +926,20 @@ err:
kref_put(rdata-kref, rdata-local_port-tt.rport_destroy);
}
+static bool
+fc_rport_compatible_roles(struct fc_lport *lport, struct fc_rport_priv *rdata)
+{
+ if (rdata-ids.roles == FC_PORT_ROLE_UNKNOWN)
+ return true;
+ if ((rdata-ids.roles FC_PORT_ROLE_FCP_TARGET)
+ (lport-service_params FCP_SPPF_INIT_FCN))
+ return true;
+ if ((rdata-ids.roles FC_PORT_ROLE_FCP_INITIATOR)
+ (lport-service_params FCP_SPPF_TARG_FCN))
+ return true;
+ return false;
+}
+
/**
* fc_rport_enter_plogi() - Send Port Login (PLOGI) request
* @rdata: The remote port to send a PLOGI to
@@ -938,6 +952,12 @@ static void fc_rport_enter_plogi(struct fc_rport_priv
*rdata)
struct fc_lport *lport = rdata-local_port;
struct fc_frame *fp;
+ if (!fc_rport_compatible_roles(lport, rdata)) {
+ FC_RPORT_DBG(rdata, PLOGI suppressed for incompatible role\n);
+ fc_rport_state_enter(rdata, RPORT_ST_PLOGI_WAIT);
+ return;
+ }
+
FC_RPORT_DBG(rdata, Port entered PLOGI state from %s state\n,
fc_rport_state(rdata));
@@ -1646,6 +1666,13 @@ static void fc_rport_recv_plogi_req(struct fc_lport
*lport,
rjt_data.explan = ELS_EXPL_NONE;
goto reject;
}
+ if (!fc_rport_compatible_roles(lport, rdata)) {
+ FC_RPORT_DBG(rdata, Received PLOGI for incompatible role\n);
+ mutex_unlock(rdata-rp_mutex);
+ rjt_data.reason = ELS_RJT_LOGIC;
+ rjt_data.explan = ELS_EXPL_NONE;
+ goto reject;
+ }
/*
* Get session payload size from incoming PLOGI.
--
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
fcoe pull-request - fixes for 3.11 merge window or RC
The following changes since commit 8bb495e3f02401ee6f76d1b1d77f3ac9f079e376: Linux 3.10 (2013-06-30 15:13:29 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/rwlove/fcoe.git tags/fixes_for_3.10 for you to fetch changes up to 7a5ed75a782a1f2de7fc773981ecd88bfa7595b1: fcoe: Reduce number of sparse warnings (2013-07-09 11:19:00 -0700) A short series of fixes to libfc, libfcoe and fcoe. Most patches fix formatting problems, one changes the behavior of which discovered ports can/will be logged into and another fixes a memory leak. Bart Van Assche (1): fcoe: Reduce number of sparse warnings Mark Rustad (2): libfc: Reject PLOGI from nodes with incompatible role fcoe: Stop fc_rport_priv structure leak Neerav Parikh (1): fcoe: Fix smatch warning in fcoe_fdmi_info function Robert Love (3): libfc: Remove extra space in fc_exch_timer_cancel definition libfc: Differentiate echange timer cancellation debug statements libfcoe: Fix meaningless log statement Yi Zou (1): fcoe: fix the link error status block sparse warnings drivers/scsi/fcoe/fcoe.c | 26 +++--- drivers/scsi/fcoe/fcoe_ctlr.c |4 drivers/scsi/fcoe/fcoe_sysfs.c | 24 drivers/scsi/fcoe/fcoe_transport.c | 28 ++-- drivers/scsi/libfc/fc_exch.c |4 ++-- drivers/scsi/libfc/fc_rport.c | 27 +++ 6 files changed, 66 insertions(+), 47 deletions(-) -- 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 6/8] libfcoe: Fix meaningless log statement
ctlr_dev was initialized to NULL, and never re-assigned. This
caused the log statement to always report failure. This patch
removes the unused variable and fixes the log statement to always
report 'success', as that is what should be logged if the code
reaches this point.
Signed-off-by: Robert Love robert.w.l...@intel.com
Tested-by: Jack Morgan jack.mor...@intel.com
Acked-by: Neil Horman nhor...@tuxdriver.com
---
drivers/scsi/fcoe/fcoe_transport.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe_transport.c
b/drivers/scsi/fcoe/fcoe_transport.c
index bedd422..f1ae5ed 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -707,7 +707,6 @@ ssize_t fcoe_ctlr_create_store(struct bus_type *bus,
{
struct net_device *netdev = NULL;
struct fcoe_transport *ft = NULL;
- struct fcoe_ctlr_device *ctlr_dev = NULL;
int rc = 0;
int err;
@@ -754,9 +753,8 @@ ssize_t fcoe_ctlr_create_store(struct bus_type *bus,
goto out_putdev;
}
- LIBFCOE_TRANSPORT_DBG(transport %s %s to create fcoe on %s.\n,
- ft-name, (ctlr_dev) ? succeeded : failed,
- netdev-name);
+ LIBFCOE_TRANSPORT_DBG(transport %s succeeded to create fcoe on %s.\n,
+ ft-name, netdev-name);
out_putdev:
dev_put(netdev);
--
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 7/8] fcoe: Stop fc_rport_priv structure leak
From: Mark Rustad mark.d.rus...@intel.com
When repeatedly doing rmmod and modprobe on the ixgbe
driver while FCoE is active in a VN2VN configuration,
memory leaks would be discovered by kmemleak with the
following backtrace:
unreferenced object 0x88003d076000 (size 1024):
comm kworker/0:3, pid 2998, jiffies 4295436448 (age 1015.332s)
hex dump (first 32 bytes):
48 8a fe 6f 00 88 ff ff 00 00 00 00 00 00 00 00 H..o
01 00 00 00 02 00 00 00 7b ac 87 21 1b 00 00 10 {..!
backtrace:
[814b308b] kmemleak_alloc+0x5b/0xc0
[8115c6e8] __kmalloc+0xd8/0x1b0
[a0216638] fc_rport_create+0x48/0x1f0 [libfc]
[a023cd86] fcoe_ctlr_vn_add.isra.10+0x56/0x1a0 [libfcoe]
[a023f440] fcoe_ctlr_vn_recv+0x8b0/0xab0 [libfcoe]
[a023fb06] fcoe_ctlr_recv_work+0x4c6/0xf60 [libfcoe]
[81067404] process_one_work+0x1e4/0x4d0
[81068def] worker_thread+0x10f/0x380
[8107019a] kthread+0xea/0xf0
[814d32ec] ret_from_fork+0x7c/0xb0
[] 0x
This patch stops the leak of the fc_rport_priv structure.
Signed-off-by: Mark Rustad mark.d.rus...@intel.com
Tested-by: Jack Morgan jack.mor...@intel.com
Signed-off-by: Robert Love robert.w.l...@intel.com
---
drivers/scsi/fcoe/fcoe_ctlr.c |4
1 file changed, 4 insertions(+)
diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 795843d..203415e 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -2090,7 +2090,11 @@ static struct fc_rport_operations fcoe_ctlr_vn_rport_ops
= {
*/
static void fcoe_ctlr_disc_stop_locked(struct fc_lport *lport)
{
+ struct fc_rport_priv *rdata;
+
mutex_lock(lport-disc.disc_mutex);
+ list_for_each_entry_rcu(rdata, lport-disc.rports, peers)
+ lport-tt.rport_logoff(rdata);
lport-disc.disc_callback = NULL;
mutex_unlock(lport-disc.disc_mutex);
}
--
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 8/8] fcoe: Reduce number of sparse warnings
From: Bart Van Assche bvanass...@acm.org
Declare local variables and functions 'static'. This patch does not
change any functionality.
Signed-off-by: Bart Van Assche bvanass...@acm.org
Signed-off-by: Robert Love robert.w.l...@intel.com
---
drivers/scsi/fcoe/fcoe_sysfs.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c
index 8c05ae01..c9382d6 100644
--- a/drivers/scsi/fcoe/fcoe_sysfs.c
+++ b/drivers/scsi/fcoe/fcoe_sysfs.c
@@ -507,7 +507,7 @@ static const struct attribute_group *fcoe_fcf_attr_groups[]
= {
NULL,
};
-struct bus_type fcoe_bus_type;
+static struct bus_type fcoe_bus_type;
static int fcoe_bus_match(struct device *dev,
struct device_driver *drv)
@@ -541,25 +541,25 @@ static void fcoe_fcf_device_release(struct device *dev)
kfree(fcf);
}
-struct device_type fcoe_ctlr_device_type = {
+static struct device_type fcoe_ctlr_device_type = {
.name = fcoe_ctlr,
.groups = fcoe_ctlr_attr_groups,
.release = fcoe_ctlr_device_release,
};
-struct device_type fcoe_fcf_device_type = {
+static struct device_type fcoe_fcf_device_type = {
.name = fcoe_fcf,
.groups = fcoe_fcf_attr_groups,
.release = fcoe_fcf_device_release,
};
-struct bus_attribute fcoe_bus_attr_group[] = {
+static struct bus_attribute fcoe_bus_attr_group[] = {
__ATTR(ctlr_create, S_IWUSR, NULL, fcoe_ctlr_create_store),
__ATTR(ctlr_destroy, S_IWUSR, NULL, fcoe_ctlr_destroy_store),
__ATTR_NULL
};
-struct bus_type fcoe_bus_type = {
+static struct bus_type fcoe_bus_type = {
.name = fcoe,
.match = fcoe_bus_match,
.bus_attrs = fcoe_bus_attr_group,
@@ -569,7 +569,7 @@ struct bus_type fcoe_bus_type = {
* fcoe_ctlr_device_flush_work() - Flush a FIP ctlr's workqueue
* @ctlr: Pointer to the FIP ctlr whose workqueue is to be flushed
*/
-void fcoe_ctlr_device_flush_work(struct fcoe_ctlr_device *ctlr)
+static void fcoe_ctlr_device_flush_work(struct fcoe_ctlr_device *ctlr)
{
if (!fcoe_ctlr_work_q(ctlr)) {
printk(KERN_ERR
@@ -590,8 +590,8 @@ void fcoe_ctlr_device_flush_work(struct fcoe_ctlr_device
*ctlr)
* Return value:
* 1 on success / 0 already queued / 0 for error
*/
-int fcoe_ctlr_device_queue_work(struct fcoe_ctlr_device *ctlr,
- struct work_struct *work)
+static int fcoe_ctlr_device_queue_work(struct fcoe_ctlr_device *ctlr,
+ struct work_struct *work)
{
if (unlikely(!fcoe_ctlr_work_q(ctlr))) {
printk(KERN_ERR
@@ -609,7 +609,7 @@ int fcoe_ctlr_device_queue_work(struct fcoe_ctlr_device
*ctlr,
* fcoe_ctlr_device_flush_devloss() - Flush a FIP ctlr's devloss workqueue
* @ctlr: Pointer to FIP ctlr whose workqueue is to be flushed
*/
-void fcoe_ctlr_device_flush_devloss(struct fcoe_ctlr_device *ctlr)
+static void fcoe_ctlr_device_flush_devloss(struct fcoe_ctlr_device *ctlr)
{
if (!fcoe_ctlr_devloss_work_q(ctlr)) {
printk(KERN_ERR
@@ -631,9 +631,9 @@ void fcoe_ctlr_device_flush_devloss(struct fcoe_ctlr_device
*ctlr)
* Return value:
* 1 on success / 0 already queued / 0 for error
*/
-int fcoe_ctlr_device_queue_devloss_work(struct fcoe_ctlr_device *ctlr,
- struct delayed_work *work,
- unsigned long delay)
+static int fcoe_ctlr_device_queue_devloss_work(struct fcoe_ctlr_device *ctlr,
+ struct delayed_work *work,
+ unsigned long delay)
{
if (unlikely(!fcoe_ctlr_devloss_work_q(ctlr))) {
printk(KERN_ERR
--
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
Re: [PATCH 4/8] libfc: Remove extra space in fc_exch_timer_cancel definition
On Tue, Jul 09, 2013 at 12:47:26PM -0700, Robert Love wrote:
Simply remove an extra space that violates coding style.
Signed-off-by: Robert Love robert.w.l...@intel.com
Tested-by: Jack Morgan jack.mor...@intel.com
Acked-by: Neil Horman nhor...@tuxdriver.com
---
drivers/scsi/libfc/fc_exch.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 8b928c6..e98ea6a 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -337,7 +337,7 @@ static void fc_exch_release(struct fc_exch *ep)
* fc_exch_timer_cancel() - cancel exch timer
* @ep: The exchange whose timer to be canceled
*/
-static inline void fc_exch_timer_cancel(struct fc_exch *ep)
+static inline void fc_exch_timer_cancel(struct fc_exch *ep)
{
if (cancel_delayed_work(ep-timeout_work)) {
FC_EXCH_DBG(ep, Exchange timer canceled\n);
Acked-by: Neil Horman nhor...@tuxdriver.com
--
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
Re: [PATCH 6/8] libfcoe: Fix meaningless log statement
On Tue, Jul 09, 2013 at 12:47:37PM -0700, Robert Love wrote:
ctlr_dev was initialized to NULL, and never re-assigned. This
caused the log statement to always report failure. This patch
removes the unused variable and fixes the log statement to always
report 'success', as that is what should be logged if the code
reaches this point.
Signed-off-by: Robert Love robert.w.l...@intel.com
Tested-by: Jack Morgan jack.mor...@intel.com
Acked-by: Neil Horman nhor...@tuxdriver.com
---
drivers/scsi/fcoe/fcoe_transport.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe_transport.c
b/drivers/scsi/fcoe/fcoe_transport.c
index bedd422..f1ae5ed 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -707,7 +707,6 @@ ssize_t fcoe_ctlr_create_store(struct bus_type *bus,
{
struct net_device *netdev = NULL;
struct fcoe_transport *ft = NULL;
- struct fcoe_ctlr_device *ctlr_dev = NULL;
int rc = 0;
int err;
@@ -754,9 +753,8 @@ ssize_t fcoe_ctlr_create_store(struct bus_type *bus,
goto out_putdev;
}
- LIBFCOE_TRANSPORT_DBG(transport %s %s to create fcoe on %s.\n,
- ft-name, (ctlr_dev) ? succeeded : failed,
- netdev-name);
+ LIBFCOE_TRANSPORT_DBG(transport %s succeeded to create fcoe on %s.\n,
+ ft-name, netdev-name);
out_putdev:
dev_put(netdev);
Acked-by: Neil Horman nhor...@tuxdriver.com
--
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
Re: [PATCH 5/8] libfc: Differentiate echange timer cancellation debug statements
On Tue, Jul 09, 2013 at 12:47:31PM -0700, Robert Love wrote:
There are two debug statements with the same output string regarding
echange timer cancellation. This patch simply changes the output of
one string so that they can be differentiated.
Signed-off-by: Robert Love robert.w.l...@intel.com
Tested-by: Jack Morgan jack.mor...@intel.com
Acked-by: Neil Horman nhor...@tuxdriver.com
---
drivers/scsi/libfc/fc_exch.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index e98ea6a..5879929 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -1567,7 +1567,7 @@ static void fc_exch_abts_resp(struct fc_exch *ep,
struct fc_frame *fp)
fc_exch_rctl_name(fh-fh_r_ctl));
if (cancel_delayed_work_sync(ep-timeout_work)) {
- FC_EXCH_DBG(ep, Exchange timer canceled\n);
+ FC_EXCH_DBG(ep, Exchange timer canceled due to ABTS
response\n);
fc_exch_release(ep);/* release from pending timer hold */
}
Acked-by: Neil Horman nhor...@tuxdriver.com
--
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 1/3] [SCSI] megaraid: Remove 64-bit DMA_BIT_MASK capability
If the megaraid device is 64-bit DMA capable then, once it is setup, any
subsequent DMA allocations for internal commands would not be properly
restricted due to megaraid_probe_one() having called pci_set_dma_mask() on
pdev with DMA_BIT_MASK(64). The driver attempts to solve this by using
make_local_pdev() to dynamically create local pci_dev structures which are
then set and used for allocating 32-bit address space restricted DMA
buffers but I don't believe that the implementation works as intended.
For a 64-bit DMA capable device, the originating pdev will have its
'dma_mask' set to 0x after the driver attaches.
Subsequently, when an internal command is initiated, make_local_pdev() is
called. make_local_pdev() uses the PCI's core to allocate a local pdev
and then copies the originating pdev content into the newly allocated
local pdev. As a result of copying the originating pdev content into
the local pdev, pdev-dev.dma_mask will be pointing back to the
originating pdev's 'dma_mask' member, not the local pdev's as
intended. Thus, when make_local_pdev() calls pci_set_dma_mask() in an
attempt to set the local pdev's DMA mask to 32 it will instead overwrite
the originating pdev's DMA mask. So, after any user initiated commands
are issued, all subsequent DMA allocations will be 32-bit restricted from
that point onward regardless of whether they are internal commands or
otherwise.
This patch fixes the issue by removing the setup of DMA_BIT_MASK to 64 in
megaraid_probe_one(), leaving the driver setup for default 32-bit DMA
capabilities, as it currently ends up in such a state anyway after any
internal commands are initiated.
Signed-off-by: Myron Stowe myron.st...@redhat.com
---
drivers/scsi/megaraid.c | 11 +++
1 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 846f475..32cca61 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -4535,14 +4535,9 @@ megaraid_probe_one(struct pci_dev *pdev, const struct
pci_device_id *id)
mcontroller[i].uid = (pci_bus 8) | pci_dev_func;
- /* Set the Mode of addressing to 64 bit if we can */
- if ((adapter-flag BOARD_64BIT) (sizeof(dma_addr_t) == 8)) {
- pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
- adapter-has_64bit_addr = 1;
- } else {
- pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
- adapter-has_64bit_addr = 0;
- }
+ /* Set the Mode of addressing to 32 bit */
+ pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+ adapter-has_64bit_addr = 0;
mutex_init(adapter-int_mtx);
init_completion(adapter-int_waitq);
--
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 2/3] [SCSI] megaraid: Remove local pdev's
With the driver now setup for default 32-bit DMA capabilities, remove the
broken make_local_pdev() implementation and usages.
Signed-off-by: Myron Stowe myron.st...@redhat.com
---
drivers/scsi/megaraid.c | 98 ---
1 files changed, 16 insertions(+), 82 deletions(-)
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 32cca61..316924c 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -2023,29 +2023,6 @@ megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd
*cmd, int aor)
return FALSE;
}
-static inline int
-make_local_pdev(adapter_t *adapter, struct pci_dev **pdev)
-{
- *pdev = alloc_pci_dev();
-
- if( *pdev == NULL ) return -1;
-
- memcpy(*pdev, adapter-dev, sizeof(struct pci_dev));
-
- if( pci_set_dma_mask(*pdev, DMA_BIT_MASK(32)) != 0 ) {
- kfree(*pdev);
- return -1;
- }
-
- return 0;
-}
-
-static inline void
-free_local_pdev(struct pci_dev *pdev)
-{
- kfree(pdev);
-}
-
/**
* mega_allocate_inquiry()
* @dma_handle - handle returned for dma address
@@ -2209,13 +2186,10 @@ proc_show_rebuild_rate(struct seq_file *m, void *v)
adapter_t *adapter = m-private;
dma_addr_t dma_handle;
caddr_t inquiry;
- struct pci_dev *pdev;
-
- if( make_local_pdev(adapter, pdev) != 0 )
- return 0;
+ struct pci_dev *pdev = adapter-dev;
if( (inquiry = mega_allocate_inquiry(dma_handle, pdev)) == NULL )
- goto free_pdev;
+ return 0;
if( mega_adapinq(adapter, dma_handle) != 0 ) {
seq_puts(m, Adapter inquiry failed.\n);
@@ -2233,8 +2207,6 @@ proc_show_rebuild_rate(struct seq_file *m, void *v)
free_inquiry:
mega_free_inquiry(inquiry, dma_handle, pdev);
-free_pdev:
- free_local_pdev(pdev);
return 0;
}
@@ -2252,14 +2224,11 @@ proc_show_battery(struct seq_file *m, void *v)
adapter_t *adapter = m-private;
dma_addr_t dma_handle;
caddr_t inquiry;
- struct pci_dev *pdev;
+ struct pci_dev *pdev = adapter-dev;
u8 battery_status;
- if( make_local_pdev(adapter, pdev) != 0 )
- return 0;
-
if( (inquiry = mega_allocate_inquiry(dma_handle, pdev)) == NULL )
- goto free_pdev;
+ return 0;
if( mega_adapinq(adapter, dma_handle) != 0 ) {
seq_printf(m, Adapter inquiry failed.\n);
@@ -2308,8 +2277,6 @@ proc_show_battery(struct seq_file *m, void *v)
free_inquiry:
mega_free_inquiry(inquiry, dma_handle, pdev);
-free_pdev:
- free_local_pdev(pdev);
return 0;
}
@@ -2357,18 +2324,15 @@ proc_show_pdrv(struct seq_file *m, adapter_t *adapter,
int channel)
char*scsi_inq;
dma_addr_t scsi_inq_dma_handle;
caddr_t inquiry;
- struct pci_dev *pdev;
+ struct pci_dev *pdev = adapter-dev;
u8 *pdrv_state;
u8 state;
int tgt;
int max_channels;
int i;
- if( make_local_pdev(adapter, pdev) != 0 )
- return 0;
-
if( (inquiry = mega_allocate_inquiry(dma_handle, pdev)) == NULL )
- goto free_pdev;
+ return 0;
if( mega_adapinq(adapter, dma_handle) != 0 ) {
seq_puts(m, Adapter inquiry failed.\n);
@@ -2453,8 +2417,6 @@ free_pci:
pci_free_consistent(pdev, 256, scsi_inq, scsi_inq_dma_handle);
free_inquiry:
mega_free_inquiry(inquiry, dma_handle, pdev);
-free_pdev:
- free_local_pdev(pdev);
return 0;
}
@@ -2533,17 +2495,14 @@ proc_show_rdrv(struct seq_file *m, adapter_t *adapter,
int start, int end )
char*disk_array;
dma_addr_t disk_array_dma_handle;
caddr_t inquiry;
- struct pci_dev *pdev;
+ struct pci_dev *pdev = adapter-dev;
u8 *rdrv_state;
int num_ldrv;
u32 array_sz;
int i;
- if( make_local_pdev(adapter, pdev) != 0 )
- return 0;
-
if( (inquiry = mega_allocate_inquiry(dma_handle, pdev)) == NULL )
- goto free_pdev;
+ return 0;
if( mega_adapinq(adapter, dma_handle) != 0 ) {
seq_puts(m, Adapter inquiry failed.\n);
@@ -2694,8 +2653,6 @@ free_pci:
disk_array_dma_handle);
free_inquiry:
mega_free_inquiry(inquiry, dma_handle, pdev);
-free_pdev:
- free_local_pdev(pdev);
return 0;
}
@@ -3164,6 +3121,7 @@ megadev_ioctl(struct file *filep, unsigned int cmd,
unsigned long arg)
return (-ENODEV);
adapter = hba_soft_state[adapno];
+ pdev = adapter-dev;
/*
* Deletion of logical drive is a special case. The adapter
@@ -3208,12
[PATCH 0/3] [SCSI] megaraid: Remove local (struct pci_dev) pdev's
Is the megaraid driver still actively used and maintained? I originally posted this series on 06.07.2013 and after receiving no comments, pinged the list again on 06.17.2013 and still received no comments/feedback. Trying again as I believe there is a real issue here, which I'd like confirmation on, and we really should remove the local copy/usage of 'struct pci_dev' that this driver currently maintains. While the megaraid device itself may be 64-bit DMA capable, 32-bit address restricted DMA buffers are apparently required for internal commands as is denoted by a couple of comments - For all internal commands, the buffer must be allocated in 4GB address range - within the driver. If the device is 64-bit DMA capable then, once it is setup, any subsequent DMA allocations for internal commands would not be properly restricted due to megaraid_probe_one() having called pci_set_dma_mask() on pdev with DMA_BIT_MASK(64). The driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures which are then set and used for allocating 32-bit address space restricted DMA buffers[1] but I don't believe that the implementation works as intended. Assume that the megaraid device is 64-bit DMA capable. While probing the device and attaching the megaraid driver, pci_set_dma_mask() is called with the originating pdev and a DMA_BIT_MASK of 64. As a result, any subsequent dynamic DMA related allocations associated with the originating pdev will acquire 64-bit based buffers, which do not meet the addressing restrictions for internal commands. megaraid_probe_one(struct pci_dev *pdev, ...) ... pci_set_dma_mask(pdev, DMA_BIT_MASK(64)); As mentioned, the driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures - local pdev's - which are set with a DMA_BIT_MASK of 32. make_local_pdev alloc_pci_dev memcpy pci_set_dma_mask dma_set_mask *dev-dma_mask = mask; The local pdev is then used in allocating a DMA buffer in an attempt to meet the 4 GB restriction. For a 64-bit DMA capable device, the originating pdev will have its 'dma_mask' set to 0x after the driver attaches. Subsequently, when an internal command is initiated, make_local_pdev() is called. make_local_pdev() uses the PCI's core to allocate a local pdev and then copies the originating pdev content into the newly allocated local pdev. As a result of copying the originating pdev content into the local pdev, pdev-dev.dma_mask will be pointing back to the originating pdev's 'dma_mask' member, not the local pdev's as intended. Thus, when make_local_pdev() calls pci_set_dma_mask() in an attempt to set the local pdev's DMA mask to 32 it will instead overwrite the originating pdev's DMA mask. Thus, after any user initiated commands are issued, all subsequent DMA allocations will be 32-bit restricted from that point onward regardless of whether they are internal commands or otherwise. This patch fixes the issue by removing the setup of DMA_BIT_MASK to 64 in megaraid_probe_one(), leaving the driver with default 32-bit DMA capabilities, as it currently ends up in such a state anyway after any internal commands are initiated. [1] It seems strange that both mega_buffer/buf_dma_handle and make_local_pdev() both exist for internal commands but this has been the case for a long time - at least since 2.6.12-rc2. Perhaps there is some coalescing that could be done. --- Myron Stowe (3): [SCSI] megaraid: Remove 64-bit DMA related dead code [SCSI] megaraid: Remove local pdev's [SCSI] megaraid: Remove 64-bit DMA_BIT_MASK capability drivers/scsi/megaraid.c | 152 --- drivers/scsi/megaraid.h |1 2 files changed, 27 insertions(+), 126 deletions(-) -- -- 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 3/3] [SCSI] megaraid: Remove 64-bit DMA related dead code
With the driver now setup for default 32-bit DMA capabilities, remove the
64-bit DMA related dead code.
Signed-off-by: Myron Stowe myron.st...@redhat.com
---
drivers/scsi/megaraid.c | 45 +
drivers/scsi/megaraid.h |1 -
2 files changed, 9 insertions(+), 37 deletions(-)
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 316924c..58953ab 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -713,12 +713,7 @@ mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int
*busy)
pthru-cdblen = cmd-cmd_len;
memcpy(pthru-cdb, cmd-cmnd, cmd-cmd_len);
- if( adapter-has_64bit_addr ) {
- mbox-m_out.cmd = MEGA_MBOXCMD_PASSTHRU64;
- }
- else {
- mbox-m_out.cmd = MEGA_MBOXCMD_PASSTHRU;
- }
+ mbox-m_out.cmd = MEGA_MBOXCMD_PASSTHRU;
scb-dma_direction = PCI_DMA_FROMDEVICE;
@@ -750,16 +745,9 @@ mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int
*busy)
* A little hack: 2nd bit is zero for all scsi read
* commands and is set for all scsi write commands
*/
- if( adapter-has_64bit_addr ) {
- mbox-m_out.cmd = (*cmd-cmnd 0x02) ?
- MEGA_MBOXCMD_LWRITE64:
- MEGA_MBOXCMD_LREAD64 ;
- }
- else {
- mbox-m_out.cmd = (*cmd-cmnd 0x02) ?
- MEGA_MBOXCMD_LWRITE:
- MEGA_MBOXCMD_LREAD ;
- }
+ mbox-m_out.cmd = (*cmd-cmnd 0x02) ?
+ MEGA_MBOXCMD_LWRITE:
+ MEGA_MBOXCMD_LREAD ;
/*
* 6-byte READ(0x08) or WRITE(0x0A) cdb
@@ -929,13 +917,7 @@ mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int
*busy)
channel, target);
/* Initialize mailbox */
- if( adapter-has_64bit_addr ) {
- mbox-m_out.cmd = MEGA_MBOXCMD_PASSTHRU64;
- }
- else {
- mbox-m_out.cmd = MEGA_MBOXCMD_PASSTHRU;
- }
-
+ mbox-m_out.cmd = MEGA_MBOXCMD_PASSTHRU;
mbox-m_out.xferaddr = scb-pthru_dma_addr;
}
@@ -1763,7 +1745,7 @@ mega_build_sglist(adapter_t *adapter, scb_t *scb, u32
*buf, u32 *len)
*len = 0;
- if (scsi_sg_count(cmd) == 1 !adapter-has_64bit_addr) {
+ if (scsi_sg_count(cmd) == 1) {
sg = scsi_sglist(cmd);
scb-dma_h_bulkdata = sg_dma_address(sg);
*buf = (u32)scb-dma_h_bulkdata;
@@ -1772,13 +1754,8 @@ mega_build_sglist(adapter_t *adapter, scb_t *scb, u32
*buf, u32 *len)
}
scsi_for_each_sg(cmd, sg, sgcnt, idx) {
- if (adapter-has_64bit_addr) {
- scb-sgl64[idx].address = sg_dma_address(sg);
- *len += scb-sgl64[idx].length = sg_dma_len(sg);
- } else {
- scb-sgl[idx].address = sg_dma_address(sg);
- *len += scb-sgl[idx].length = sg_dma_len(sg);
- }
+ scb-sgl[idx].address = sg_dma_address(sg);
+ *len += scb-sgl[idx].length = sg_dma_len(sg);
}
/* Reset pointer and length fields */
@@ -2076,10 +2053,7 @@ proc_show_config(struct seq_file *m, void *v)
if(adapter-flag BOARD_64BIT)
seq_puts(m, Controller capable of 64-bit memory addressing\n);
- if( adapter-has_64bit_addr )
- seq_puts(m, Controller using 64-bit memory addressing\n);
- else
- seq_puts(m, Controller is not using 64-bit memory
addressing\n);
+ seq_puts(m, Controller is not using 64-bit memory addressing\n);
seq_printf(m, Base = %08lx, Irq = %d, ,
adapter-base, adapter-host-irq);
@@ -4471,7 +4445,6 @@ megaraid_probe_one(struct pci_dev *pdev, const struct
pci_device_id *id)
/* Set the Mode of addressing to 32 bit */
pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
- adapter-has_64bit_addr = 0;
mutex_init(adapter-int_mtx);
init_completion(adapter-int_waitq);
diff --git a/drivers/scsi/megaraid.h b/drivers/scsi/megaraid.h
index 4d0ce4e..17e449b 100644
--- a/drivers/scsi/megaraid.h
+++ b/drivers/scsi/megaraid.h
@@ -827,7 +827,6 @@ typedef struct {
#endif
- int has_64bit_addr; /* are we using
Re: [PATCH 0/3] [SCSI] megaraid: Remove local (struct pci_dev) pdev's
On Tue, 2013-07-09 at 14:39 -0600, Myron Stowe wrote: Is the megaraid driver still actively used and maintained? I originally posted this series on 06.07.2013 and after receiving no comments, pinged the list again on 06.17.2013 and still received no comments/feedback. Trying again as I believe there is a real issue here, which I'd like confirmation on, and we really should remove the local copy/usage of 'struct pci_dev' that this driver currently maintains. While the megaraid device itself may be 64-bit DMA capable, 32-bit address restricted DMA buffers are apparently required for internal commands as is denoted by a couple of comments - For all internal commands, the buffer must be allocated in 4GB address range - within the driver. If the device is 64-bit DMA capable then, once it is setup, any subsequent DMA allocations for internal commands would not be properly restricted due to megaraid_probe_one() having called pci_set_dma_mask() on pdev with DMA_BIT_MASK(64). The driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures which are then set and used for allocating 32-bit address space restricted DMA buffers[1] but I don't believe that the implementation works as intended. Assume that the megaraid device is 64-bit DMA capable. While probing the device and attaching the megaraid driver, pci_set_dma_mask() is called with the originating pdev and a DMA_BIT_MASK of 64. As a result, any subsequent dynamic DMA related allocations associated with the originating pdev will acquire 64-bit based buffers, which do not meet the addressing restrictions for internal commands. megaraid_probe_one(struct pci_dev *pdev, ...) ... pci_set_dma_mask(pdev, DMA_BIT_MASK(64)); As mentioned, the driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures - local pdev's - which are set with a DMA_BIT_MASK of 32. make_local_pdev alloc_pci_dev memcpy pci_set_dma_mask dma_set_mask *dev-dma_mask = mask; The local pdev is then used in allocating a DMA buffer in an attempt to meet the 4 GB restriction. For a 64-bit DMA capable device, the originating pdev will have its 'dma_mask' set to 0x after the driver attaches. Subsequently, when an internal command is initiated, make_local_pdev() is called. make_local_pdev() uses the PCI's core to allocate a local pdev and then copies the originating pdev content into the newly allocated local pdev. As a result of copying the originating pdev content into the local pdev, pdev-dev.dma_mask will be pointing back to the originating pdev's 'dma_mask' member, not the local pdev's as intended. Thus, when make_local_pdev() calls pci_set_dma_mask() in an attempt to set the local pdev's DMA mask to 32 it will instead overwrite the originating pdev's DMA mask. Thus, after any user initiated commands are issued, all subsequent DMA allocations will be 32-bit restricted from that point onward regardless of whether they are internal commands or otherwise. This patch fixes the issue by removing the setup of DMA_BIT_MASK to 64 in megaraid_probe_one(), leaving the driver with default 32-bit DMA capabilities, as it currently ends up in such a state anyway after any internal commands are initiated. [1] It seems strange that both mega_buffer/buf_dma_handle and make_local_pdev() both exist for internal commands but this has been the case for a long time - at least since 2.6.12-rc2. Perhaps there is some coalescing that could be done. --- Myron Stowe (3): [SCSI] megaraid: Remove 64-bit DMA related dead code [SCSI] megaraid: Remove local pdev's [SCSI] megaraid: Remove 64-bit DMA_BIT_MASK capability Adam, you do drive by coding on this for LSI ... ack or reject, please. James -- 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
Re: [PATCH 0/3] [SCSI] megaraid: Remove local (struct pci_dev) pdev's
On Tue, Jul 9, 2013 at 2:18 PM, James Bottomley james.bottom...@hansenpartnership.com wrote: On Tue, 2013-07-09 at 14:39 -0600, Myron Stowe wrote: Is the megaraid driver still actively used and maintained? I originally posted this series on 06.07.2013 and after receiving no comments, pinged the list again on 06.17.2013 and still received no comments/feedback. Trying again as I believe there is a real issue here, which I'd like confirmation on, and we really should remove the local copy/usage of 'struct pci_dev' that this driver currently maintains. While the megaraid device itself may be 64-bit DMA capable, 32-bit address restricted DMA buffers are apparently required for internal commands as is denoted by a couple of comments - For all internal commands, the buffer must be allocated in 4GB address range - within the driver. If the device is 64-bit DMA capable then, once it is setup, any subsequent DMA allocations for internal commands would not be properly restricted due to megaraid_probe_one() having called pci_set_dma_mask() on pdev with DMA_BIT_MASK(64). The driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures which are then set and used for allocating 32-bit address space restricted DMA buffers[1] but I don't believe that the implementation works as intended. Assume that the megaraid device is 64-bit DMA capable. While probing the device and attaching the megaraid driver, pci_set_dma_mask() is called with the originating pdev and a DMA_BIT_MASK of 64. As a result, any subsequent dynamic DMA related allocations associated with the originating pdev will acquire 64-bit based buffers, which do not meet the addressing restrictions for internal commands. megaraid_probe_one(struct pci_dev *pdev, ...) ... pci_set_dma_mask(pdev, DMA_BIT_MASK(64)); As mentioned, the driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures - local pdev's - which are set with a DMA_BIT_MASK of 32. make_local_pdev alloc_pci_dev memcpy pci_set_dma_mask dma_set_mask *dev-dma_mask = mask; The local pdev is then used in allocating a DMA buffer in an attempt to meet the 4 GB restriction. For a 64-bit DMA capable device, the originating pdev will have its 'dma_mask' set to 0x after the driver attaches. Subsequently, when an internal command is initiated, make_local_pdev() is called. make_local_pdev() uses the PCI's core to allocate a local pdev and then copies the originating pdev content into the newly allocated local pdev. As a result of copying the originating pdev content into the local pdev, pdev-dev.dma_mask will be pointing back to the originating pdev's 'dma_mask' member, not the local pdev's as intended. Thus, when make_local_pdev() calls pci_set_dma_mask() in an attempt to set the local pdev's DMA mask to 32 it will instead overwrite the originating pdev's DMA mask. Thus, after any user initiated commands are issued, all subsequent DMA allocations will be 32-bit restricted from that point onward regardless of whether they are internal commands or otherwise. This patch fixes the issue by removing the setup of DMA_BIT_MASK to 64 in megaraid_probe_one(), leaving the driver with default 32-bit DMA capabilities, as it currently ends up in such a state anyway after any internal commands are initiated. [1] It seems strange that both mega_buffer/buf_dma_handle and make_local_pdev() both exist for internal commands but this has been the case for a long time - at least since 2.6.12-rc2. Perhaps there is some coalescing that could be done. --- Myron Stowe (3): [SCSI] megaraid: Remove 64-bit DMA related dead code [SCSI] megaraid: Remove local pdev's [SCSI] megaraid: Remove 64-bit DMA_BIT_MASK capability Adam, you do drive by coding on this for LSI ... ack or reject, please. James James, I have just now located my box of MegaRAID Parallel SCSI controllers. I will review and test the patch series from Myron and respond by next Monday. -Adam -- 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
