Re: [ANNOUNCE] scsi patch queue tree
On 5/28/2014 4:24 PM, Christoph Hellwig wrote: I've pushed the following changes to the drivers-for-3.16 tree. I've there's anyting matching the rules that I did forget please resend and/or ping me. Benoit Taine (2): qla4xxx: Use kmemdup instead of kmalloc + memcpy qla2xxx: Use kmemdup instead of kmalloc + memcpy Dan Carpenter (1): qla2xxx: fix incorrect debug printk Dolev Raviv (4): scsi: ufs: query descriptor API scsi: ufs: device query status and size check scsi: ufs: Logical Unit (LU) command queue depth scsi: ufs: Fix queue depth handling for best effort cases The above 4 patches are just posted to mailing lists with no review/ack's yet. I believe it still went in because the sender has modified the author name to himself and signed-off by is present by the original author which worked as a positive review for your rules. I have asked the sender to check why the original author name is changed. Meanwhile, I believe these patches should get some more time for review. -- Regards, Sujit -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ANNOUNCE] scsi patch queue tree
On 5/28/2014 4:24 PM, Christoph Hellwig wrote: I've pushed the following changes to the drivers-for-3.16 tree. I've there's anyting matching the rules that I did forget please resend and/or ping me. Benoit Taine (2): qla4xxx: Use kmemdup instead of kmalloc + memcpy qla2xxx: Use kmemdup instead of kmalloc + memcpy Dan Carpenter (1): qla2xxx: fix incorrect debug printk Dolev Raviv (4): scsi: ufs: query descriptor API scsi: ufs: device query status and size check scsi: ufs: Logical Unit (LU) command queue depth scsi: ufs: Fix queue depth handling for best effort cases The above 4 patches are just posted to mailing lists with no review/ack's yet. I believe it still went in because the sender has modified the author name to himself and signed-off by is present by the original author which worked as a positive review for your rules. I have asked the sender to check why the original author name is changed. Meanwhile, I believe these patches should get some more time for review. -- Regards, Sujit -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build warning after merge of the scsi tree
On 9/4/2013 2:48 PM, Sujit Reddy Thumma wrote:
On 9/2/2013 1:58 PM, Stephen Rothwell wrote:
Hi James,
After merging the scsi tree, today's linux-next build (x86_64
allmodconfig) produced this warning:
drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_eh_host_reset_handler':
drivers/scsi/ufs/ufshcd.c:2740:3: warning: 'flush_work_sync' is
deprecated (declared at
/scratch/sfr/next/include/linux/workqueue.h:624)
[-Wdeprecated-declarations]
flush_work_sync(>eh_work);
^
James, would you like to pick up follow-up patch @
http://marc.info/?l=linux-scsi=137819519527432=2
or let me update the commit c1e846ab4422 fixing this?
Ignore this.
I saw you have dropped the patchset already. I will fix and post the
updated patches soon. Thanks.
Introduced by commit c1e846ab4422 ("[SCSI] ufs: Fix device and host reset
methods"). flush_work_sync was deprecated by commit 43829731dd37
("workqueue: deprecate flush[_delayed]_work_sync()") before v3.7-rc1.
--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build warning after merge of the scsi tree
On 9/2/2013 1:58 PM, Stephen Rothwell wrote:
Hi James,
After merging the scsi tree, today's linux-next build (x86_64
allmodconfig) produced this warning:
drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_eh_host_reset_handler':
drivers/scsi/ufs/ufshcd.c:2740:3: warning: 'flush_work_sync' is deprecated
(declared at /scratch/sfr/next/include/linux/workqueue.h:624)
[-Wdeprecated-declarations]
flush_work_sync(>eh_work);
^
James, would you like to pick up follow-up patch @
http://marc.info/?l=linux-scsi=137819519527432=2
or let me update the commit c1e846ab4422 fixing this?
Introduced by commit c1e846ab4422 ("[SCSI] ufs: Fix device and host reset
methods"). flush_work_sync was deprecated by commit 43829731dd37
("workqueue: deprecate flush[_delayed]_work_sync()") before v3.7-rc1.
--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build warning after merge of the scsi tree
On 9/2/2013 1:58 PM, Stephen Rothwell wrote: Hi James, After merging the scsi tree, today's linux-next build (x86_64 allmodconfig) produced this warning: drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_eh_host_reset_handler': drivers/scsi/ufs/ufshcd.c:2740:3: warning: 'flush_work_sync' is deprecated (declared at /scratch/sfr/next/include/linux/workqueue.h:624) [-Wdeprecated-declarations] flush_work_sync(hba-eh_work); ^ James, would you like to pick up follow-up patch @ http://marc.info/?l=linux-scsim=137819519527432w=2 or let me update the commit c1e846ab4422 fixing this? Introduced by commit c1e846ab4422 ([SCSI] ufs: Fix device and host reset methods). flush_work_sync was deprecated by commit 43829731dd37 (workqueue: deprecate flush[_delayed]_work_sync()) before v3.7-rc1. -- Regards, Sujit -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build warning after merge of the scsi tree
On 9/4/2013 2:48 PM, Sujit Reddy Thumma wrote: On 9/2/2013 1:58 PM, Stephen Rothwell wrote: Hi James, After merging the scsi tree, today's linux-next build (x86_64 allmodconfig) produced this warning: drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_eh_host_reset_handler': drivers/scsi/ufs/ufshcd.c:2740:3: warning: 'flush_work_sync' is deprecated (declared at /scratch/sfr/next/include/linux/workqueue.h:624) [-Wdeprecated-declarations] flush_work_sync(hba-eh_work); ^ James, would you like to pick up follow-up patch @ http://marc.info/?l=linux-scsim=137819519527432w=2 or let me update the commit c1e846ab4422 fixing this? Ignore this. I saw you have dropped the patchset already. I will fix and post the updated patches soon. Thanks. Introduced by commit c1e846ab4422 ([SCSI] ufs: Fix device and host reset methods). flush_work_sync was deprecated by commit 43829731dd37 (workqueue: deprecate flush[_delayed]_work_sync()) before v3.7-rc1. -- Regards, Sujit -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Fix possible sleep in invalid context
On 7/2/2013 8:34 AM, Aaron Lu wrote: Fix this by releasing spin_lock_irq() before calling >pm_runtime_autosuspend() in blk_post_runtime_resume(). Hi Sujit, Thanks for testing out block layer runtime PM! As for the problem here, it is already fixed by: commit c60855cdb976c632b3bf8922eeab8a0e78edfc04 Author: Aaron Lu Date: Fri May 17 15:47:20 2013 +0800 blkpm: avoid sleep when holding queue lock Thanks Aaron. I see that is merged in 3.10-rc6. Please ignore this patch. -- Regards, Sujit -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] block: Fix possible sleep in invalid context
When block runtime PM is enabled following warning is seen
while resuming the device.
BUG: sleeping function called from invalid context at
.../drivers/base/power/runtime.c:923
in_atomic(): 1, irqs_disabled(): 128, pid: 12, name: kworker/0:1
[] (unwind_backtrace+0x0/0x120) from
[] (__pm_runtime_suspend+0x34/0xa0) from
[] (blk_post_runtime_resume+0x4c/0x5c) from
[] (scsi_runtime_resume+0x90/0xb4) from
[] (__rpm_callback+0x30/0x58) from
[] (rpm_callback+0x18/0x28) from
[] (rpm_resume+0x3dc/0x540) from
[] (pm_runtime_work+0x8c/0x98) from
[] (process_one_work+0x238/0x3e4) from
[] (worker_thread+0x1ac/0x2ac) from
[] (kthread+0x88/0x94) from
[] (kernel_thread_exit+0x0/0x8)
Fix this by releasing spin_lock_irq() before calling
pm_runtime_autosuspend() in blk_post_runtime_resume().
Signed-off-by: Sujit Reddy Thumma
Cc: sta...@vger.kernel.org
---
block/blk-core.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 33c33bc..2456116 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -3159,16 +3159,18 @@ EXPORT_SYMBOL(blk_pre_runtime_resume);
*/
void blk_post_runtime_resume(struct request_queue *q, int err)
{
- spin_lock_irq(q->queue_lock);
if (!err) {
+ spin_lock_irq(q->queue_lock);
q->rpm_status = RPM_ACTIVE;
__blk_run_queue(q);
pm_runtime_mark_last_busy(q->dev);
+ spin_unlock_irq(q->queue_lock);
pm_runtime_autosuspend(q->dev);
} else {
+ spin_lock_irq(q->queue_lock);
q->rpm_status = RPM_SUSPENDED;
+ spin_unlock_irq(q->queue_lock);
}
- spin_unlock_irq(q->queue_lock);
}
EXPORT_SYMBOL(blk_post_runtime_resume);
#endif
--
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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] block: Fix possible sleep in invalid context
When block runtime PM is enabled following warning is seen
while resuming the device.
BUG: sleeping function called from invalid context at
.../drivers/base/power/runtime.c:923
in_atomic(): 1, irqs_disabled(): 128, pid: 12, name: kworker/0:1
[c0014448] (unwind_backtrace+0x0/0x120) from
[c03120e4] (__pm_runtime_suspend+0x34/0xa0) from
[c021c33c] (blk_post_runtime_resume+0x4c/0x5c) from
[c03297cc] (scsi_runtime_resume+0x90/0xb4) from
[c0310940] (__rpm_callback+0x30/0x58) from
[c0310980] (rpm_callback+0x18/0x28) from
[c0311ab0] (rpm_resume+0x3dc/0x540) from
[c03120a4] (pm_runtime_work+0x8c/0x98) from
[c007767c] (process_one_work+0x238/0x3e4) from
[c0077b90] (worker_thread+0x1ac/0x2ac) from
[c007cfdc] (kthread+0x88/0x94) from
[c000ece0] (kernel_thread_exit+0x0/0x8)
Fix this by releasing spin_lock_irq() before calling
pm_runtime_autosuspend() in blk_post_runtime_resume().
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
Cc: sta...@vger.kernel.org
---
block/blk-core.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 33c33bc..2456116 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -3159,16 +3159,18 @@ EXPORT_SYMBOL(blk_pre_runtime_resume);
*/
void blk_post_runtime_resume(struct request_queue *q, int err)
{
- spin_lock_irq(q-queue_lock);
if (!err) {
+ spin_lock_irq(q-queue_lock);
q-rpm_status = RPM_ACTIVE;
__blk_run_queue(q);
pm_runtime_mark_last_busy(q-dev);
+ spin_unlock_irq(q-queue_lock);
pm_runtime_autosuspend(q-dev);
} else {
+ spin_lock_irq(q-queue_lock);
q-rpm_status = RPM_SUSPENDED;
+ spin_unlock_irq(q-queue_lock);
}
- spin_unlock_irq(q-queue_lock);
}
EXPORT_SYMBOL(blk_post_runtime_resume);
#endif
--
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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Fix possible sleep in invalid context
On 7/2/2013 8:34 AM, Aaron Lu wrote: Fix this by releasing spin_lock_irq() before calling pm_runtime_autosuspend() in blk_post_runtime_resume(). Hi Sujit, Thanks for testing out block layer runtime PM! As for the problem here, it is already fixed by: commit c60855cdb976c632b3bf8922eeab8a0e78edfc04 Author: Aaron Luaaron...@intel.com Date: Fri May 17 15:47:20 2013 +0800 blkpm: avoid sleep when holding queue lock Thanks Aaron. I see that is merged in 3.10-rc6. Please ignore this patch. -- Regards, Sujit -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
On 6/13/2013 10:03 AM, Sujit Reddy Thumma wrote:
static struct scsi_host_template ufshcd_driver_template = {
@@ -1771,8 +2064,8 @@ int ufshcd_init(struct device *dev, struct
ufs_hba **hba_handle,
/* Configure LRB */
ufshcd_host_memory_configure(hba);
- host->can_queue = hba->nutrs;
- host->cmd_per_lun = hba->nutrs;
+ host->can_queue = SCSI_CMD_QUEUE_SIZE;
I don't think this is appropriate. Reserving a slot exclusively for
query/DM requests is not optimal. can_queue should be changed
dynamically, scsi_adjust_queue_depth() maybe?
The motivation to change the design for this patch is that routing
query command through SCSI layer raised problems when we are trying to
improve the fatal error handling as we need to block the SCSI layer and
recover the link. Hence, the need to have the query/DM command send as
internal commands. Which probably makes sense.
One possible option was to look for a free command slot whenever we
try to send an internal command, but getting a free slot is not always
guaranteed.
Even if we get hold of a tag, there is no way we can explain this to
SCSI/block layer that particular tag is in use internally (case where
normal query requests are sent in tandem with SCSI requests). In which
case, other option is to use tag[31] as you have said on need basis
and change the queue depth to 31. This again has problems - one
changing queue depth doesn't take effect immediately but for the next
command. Second, if the command in tag[31] is the root cause of the
fatal error and is stuck, then the internal command has to wait forever
(until scsi timesout) to plan recovery. Considering, all these factors
it is better to reserve a tag and use it for internal commands instead
of playing around with the tags internally without/partially informing
SCSI/block layers.
I am open for discussion, if there are any suggestions to handle such
LLD requirements in the SCSI layer optimally.
Coming to how optimal is to work with 31 slots instead of h/w defined 32
is something which we can answer when we have true multi queueing.
AFAIK, there may not exist real-world applications which utilize full 32
tags at particular instant. SATA AHCI controller driver which is
ubiquitous in modern systems also reserves a slot for internal commands
which is used only in case of error handling and AFAIK, no one has ever
reported performance problems with this (its about 7 years the commit to
reserve a slot is merged into Linux tree).
I hope this explains the intent. Please let me know what do you think.
--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
On 6/13/2013 10:03 AM, Sujit Reddy Thumma wrote:
static struct scsi_host_template ufshcd_driver_template = {
@@ -1771,8 +2064,8 @@ int ufshcd_init(struct device *dev, struct
ufs_hba **hba_handle,
/* Configure LRB */
ufshcd_host_memory_configure(hba);
- host-can_queue = hba-nutrs;
- host-cmd_per_lun = hba-nutrs;
+ host-can_queue = SCSI_CMD_QUEUE_SIZE;
I don't think this is appropriate. Reserving a slot exclusively for
query/DM requests is not optimal. can_queue should be changed
dynamically, scsi_adjust_queue_depth() maybe?
The motivation to change the design for this patch is that routing
query command through SCSI layer raised problems when we are trying to
improve the fatal error handling as we need to block the SCSI layer and
recover the link. Hence, the need to have the query/DM command send as
internal commands. Which probably makes sense.
One possible option was to look for a free command slot whenever we
try to send an internal command, but getting a free slot is not always
guaranteed.
Even if we get hold of a tag, there is no way we can explain this to
SCSI/block layer that particular tag is in use internally (case where
normal query requests are sent in tandem with SCSI requests). In which
case, other option is to use tag[31] as you have said on need basis
and change the queue depth to 31. This again has problems - one
changing queue depth doesn't take effect immediately but for the next
command. Second, if the command in tag[31] is the root cause of the
fatal error and is stuck, then the internal command has to wait forever
(until scsi timesout) to plan recovery. Considering, all these factors
it is better to reserve a tag and use it for internal commands instead
of playing around with the tags internally without/partially informing
SCSI/block layers.
I am open for discussion, if there are any suggestions to handle such
LLD requirements in the SCSI layer optimally.
Coming to how optimal is to work with 31 slots instead of h/w defined 32
is something which we can answer when we have true multi queueing.
AFAIK, there may not exist real-world applications which utilize full 32
tags at particular instant. SATA AHCI controller driver which is
ubiquitous in modern systems also reserves a slot for internal commands
which is used only in case of error handling and AFAIK, no one has ever
reported performance problems with this (its about 7 years the commit to
reserve a slot is merged into Linux tree).
I hope this explains the intent. Please let me know what do you think.
--
Regards,
Sujit
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] scsi: ufs: Set fDeviceInit flag to initiate device initialization
On 6/12/2013 11:04 AM, Santosh Y wrote:
/**
+ * ufshcd_query_request() - API for issuing query request to the device.
+ * @hba: ufs driver context
+ * @query: params for query request
+ * @descriptor: buffer for sending/receiving descriptor
+ * @retries: number of times to try executing the command
+ *
+ * All necessary fields for issuing a query and receiving its response
+ * are stored in the UFS hba struct. We can use this method since we know
+ * there is only one active query request or any internal command at all
+ * times.
+ */
+static int ufshcd_send_query_request(struct ufs_hba *hba,
+ struct ufs_query_req *query,
+ u8 *descriptor,
+ struct ufs_query_res *response)
+{
+ int ret;
+
+ BUG_ON(!hba);
+ if (!query || !response) {
+ dev_err(hba->dev,
+ "%s: NULL pointer query = %p, response = %p\n",
+ __func__, query, response);
+ return -EINVAL;
+ }
+
+ mutex_lock(>i_cmd.dev_cmd_lock);
+ hba->i_cmd.query.request = query;
+ hba->i_cmd.query.response = response;
+ hba->i_cmd.query.descriptor = descriptor;
+
+ ret = ufshcd_exec_internal_cmd(hba, DEV_CMD_TYPE_QUERY,
+ QUERY_REQ_TIMEOUT);
Can this be generic, as external query commands might also use it?
External query commands can call ufshcd_send_query_request() directly,
without going into hassle of taking mutex lock and filling internal cmd
structure.
+
+ hba->i_cmd.query.request = NULL;
+ hba->i_cmd.query.response = NULL;
+ hba->i_cmd.query.descriptor = NULL;
+ mutex_unlock(>i_cmd.dev_cmd_lock);
+
+ return ret;
+}
+
+/**
--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
On 6/12/2013 11:00 AM, Santosh Y wrote:
+/*
+ * 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 register value after iteration
+ */
+static u32 ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
+ u32 val, unsigned long interval_us, unsigned long timeout_ms)
+{
+ u32 tmp;
+ ktime_t start;
+ unsigned long diff;
+
+ tmp = ufshcd_readl(hba, reg);
+
+ start = ktime_get();
+ while ((tmp & mask) == val) {
...as now I notice it, 'val' is the wait condition and the loop
continues if the wait condition is met. I feel it's a bit confusing.
Wouldn't something like (x != wait_condition) be appropriate?
Makes sense.
+ /* wakeup within 50us of expiry */
+ usleep_range(interval_us, interval_us + 50);
+ tmp = ufshcd_readl(hba, reg);
+ diff = ktime_to_ms(ktime_sub(ktime_get(), start));
+ if (diff > timeout_ms) {
+ tmp = ufshcd_readl(hba, reg);
Why this extra read? The register value might have changed during the
execution of 2 previous statements, is that the assumption?
Yes, if there is a preemption between the last register read and the
diff calculation and the CPU comes back after long time, it can happen
that diff is greater than timeout and we enter the if condition. So, it
is better to read the value after a timeout and return to the caller.
+ break;
+ }
+ }
+
+ return tmp;
+}
+
/**
* ufshcd_get_intr_mask - Get the interrupt bit mask
* @hba - Pointer to adapter instance
@@ -223,18 +267,13 @@ static inline void ufshcd_free_hba_memory(struct ufs_hba
*hba)
}
/**
- * ufshcd_is_valid_req_rsp - checks if controller TR response is valid
+ * ufshcd_get_req_rsp - returns the TR response
* @ucd_rsp_ptr: pointer to response UPIU
- *
- * This function checks the response UPIU for valid transaction type in
- * response field
- * Returns 0 on success, non-zero on failure
*/
static inline int
-ufshcd_is_valid_req_rsp(struct utp_upiu_rsp *ucd_rsp_ptr)
+ufshcd_get_req_rsp(struct utp_upiu_rsp *ucd_rsp_ptr)
{
- return ((be32_to_cpu(ucd_rsp_ptr->header.dword_0) >> 24) ==
-UPIU_TRANSACTION_RESPONSE) ? 0 : DID_ERROR << 16;
+ return be32_to_cpu(ucd_rsp_ptr->header.dword_0) >> 24;
}
/**
@@ -331,9 +370,9 @@ static inline void ufshcd_copy_sense_data(struct ufshcd_lrb
*lrbp)
{
int len;
if (lrbp->sense_buffer) {
- len = be16_to_cpu(lrbp->ucd_rsp_ptr->sense_data_len);
+ len = be16_to_cpu(lrbp->ucd_rsp_ptr->sc.sense_data_len);
memcpy(lrbp->sense_buffer,
- lrbp->ucd_rsp_ptr->sense_data,
+ lrbp->ucd_rsp_ptr->sc.sense_data,
min_t(int, len, SCSI_SENSE_BUFFERSIZE));
}
}
@@ -551,76 +590,128 @@ static void ufshcd_disable_intr(struct ufs_hba *hba, u32
intrs)
}
/**
+ * ufshcd_prepare_req_desc() - Fills the requests header
+ * descriptor according to request
+ * @lrbp: pointer to local reference block
+ * @upiu_flags: flags required in the header
+ * @cmd_dir: requests data direction
+ */
+static void ufshcd_prepare_req_desc(struct ufshcd_lrb *lrbp, u32 *upiu_flags,
+ enum dma_data_direction cmd_dir)
+{
+ struct utp_transfer_req_desc *req_desc = lrbp->utr_descriptor_ptr;
+ u32 data_direction;
+ u32 dword_0;
+
+ if (cmd_dir == DMA_FROM_DEVICE) {
+ data_direction = UTP_DEVICE_TO_HOST;
+ *upiu_flags = UPIU_CMD_FLAGS_READ;
+ } else if (cmd_dir == DMA_TO_DEVICE) {
+ data_direction = UTP_HOST_TO_DEVICE;
+ *upiu_flags = UPIU_CMD_FLAGS_WRITE;
+ } else {
+ data_direction = UTP_NO_DATA_TRANSFER;
+ *upiu_flags = UPIU_CMD_FLAGS_NONE;
+ }
+
+ dword_0 = data_direction | (lrbp->command_type
+ << UPIU_COMMAND_TYPE_OFFSET);
+ if (lrbp->intr_cmd)
+ dword_0 |= UTP_REQ_DESC_INT_CMD;
+
+ /* Transfer request descriptor header fields */
+ req_desc->header.dword_0 = cpu_to_le32(dword_0);
+
+ /*
+* assigning invalid value for command status. Controller
+* updates OCS on command completion, with the command
+* status
+*/
+ req_desc->header.dword_2 =
+ cpu_to_le32(OCS_INVALID_COMMAND_STATUS);
+}
+
+/**
+ * ufshcd_prepare_utp_scsi_cmd_upiu() - fills the utp_transfer_req_desc,
+ * for scsi commands
+ * @lrbp - local reference block pointer
+ * @upiu_flags - flags
+ */
+static
+void ufshcd_prepare_utp_scsi_cmd_upiu(struct ufshcd_lrb
Re: [PATCH 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
On 6/12/2013 11:00 AM, Santosh Y wrote:
+/*
+ * 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 register value after iteration
+ */
+static u32 ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
+ u32 val, unsigned long interval_us, unsigned long timeout_ms)
+{
+ u32 tmp;
+ ktime_t start;
+ unsigned long diff;
+
+ tmp = ufshcd_readl(hba, reg);
+
+ start = ktime_get();
+ while ((tmp mask) == val) {
...as now I notice it, 'val' is the wait condition and the loop
continues if the wait condition is met. I feel it's a bit confusing.
Wouldn't something like (x != wait_condition) be appropriate?
Makes sense.
+ /* wakeup within 50us of expiry */
+ usleep_range(interval_us, interval_us + 50);
+ tmp = ufshcd_readl(hba, reg);
+ diff = ktime_to_ms(ktime_sub(ktime_get(), start));
+ if (diff timeout_ms) {
+ tmp = ufshcd_readl(hba, reg);
Why this extra read? The register value might have changed during the
execution of 2 previous statements, is that the assumption?
Yes, if there is a preemption between the last register read and the
diff calculation and the CPU comes back after long time, it can happen
that diff is greater than timeout and we enter the if condition. So, it
is better to read the value after a timeout and return to the caller.
+ break;
+ }
+ }
+
+ return tmp;
+}
+
/**
* ufshcd_get_intr_mask - Get the interrupt bit mask
* @hba - Pointer to adapter instance
@@ -223,18 +267,13 @@ static inline void ufshcd_free_hba_memory(struct ufs_hba
*hba)
}
/**
- * ufshcd_is_valid_req_rsp - checks if controller TR response is valid
+ * ufshcd_get_req_rsp - returns the TR response
* @ucd_rsp_ptr: pointer to response UPIU
- *
- * This function checks the response UPIU for valid transaction type in
- * response field
- * Returns 0 on success, non-zero on failure
*/
static inline int
-ufshcd_is_valid_req_rsp(struct utp_upiu_rsp *ucd_rsp_ptr)
+ufshcd_get_req_rsp(struct utp_upiu_rsp *ucd_rsp_ptr)
{
- return ((be32_to_cpu(ucd_rsp_ptr-header.dword_0) 24) ==
-UPIU_TRANSACTION_RESPONSE) ? 0 : DID_ERROR 16;
+ return be32_to_cpu(ucd_rsp_ptr-header.dword_0) 24;
}
/**
@@ -331,9 +370,9 @@ static inline void ufshcd_copy_sense_data(struct ufshcd_lrb
*lrbp)
{
int len;
if (lrbp-sense_buffer) {
- len = be16_to_cpu(lrbp-ucd_rsp_ptr-sense_data_len);
+ len = be16_to_cpu(lrbp-ucd_rsp_ptr-sc.sense_data_len);
memcpy(lrbp-sense_buffer,
- lrbp-ucd_rsp_ptr-sense_data,
+ lrbp-ucd_rsp_ptr-sc.sense_data,
min_t(int, len, SCSI_SENSE_BUFFERSIZE));
}
}
@@ -551,76 +590,128 @@ static void ufshcd_disable_intr(struct ufs_hba *hba, u32
intrs)
}
/**
+ * ufshcd_prepare_req_desc() - Fills the requests header
+ * descriptor according to request
+ * @lrbp: pointer to local reference block
+ * @upiu_flags: flags required in the header
+ * @cmd_dir: requests data direction
+ */
+static void ufshcd_prepare_req_desc(struct ufshcd_lrb *lrbp, u32 *upiu_flags,
+ enum dma_data_direction cmd_dir)
+{
+ struct utp_transfer_req_desc *req_desc = lrbp-utr_descriptor_ptr;
+ u32 data_direction;
+ u32 dword_0;
+
+ if (cmd_dir == DMA_FROM_DEVICE) {
+ data_direction = UTP_DEVICE_TO_HOST;
+ *upiu_flags = UPIU_CMD_FLAGS_READ;
+ } else if (cmd_dir == DMA_TO_DEVICE) {
+ data_direction = UTP_HOST_TO_DEVICE;
+ *upiu_flags = UPIU_CMD_FLAGS_WRITE;
+ } else {
+ data_direction = UTP_NO_DATA_TRANSFER;
+ *upiu_flags = UPIU_CMD_FLAGS_NONE;
+ }
+
+ dword_0 = data_direction | (lrbp-command_type
+UPIU_COMMAND_TYPE_OFFSET);
+ if (lrbp-intr_cmd)
+ dword_0 |= UTP_REQ_DESC_INT_CMD;
+
+ /* Transfer request descriptor header fields */
+ req_desc-header.dword_0 = cpu_to_le32(dword_0);
+
+ /*
+* assigning invalid value for command status. Controller
+* updates OCS on command completion, with the command
+* status
+*/
+ req_desc-header.dword_2 =
+ cpu_to_le32(OCS_INVALID_COMMAND_STATUS);
+}
+
+/**
+ * ufshcd_prepare_utp_scsi_cmd_upiu() - fills the utp_transfer_req_desc,
+ * for scsi commands
+ * @lrbp - local reference block pointer
+ * @upiu_flags - flags
+ */
+static
+void ufshcd_prepare_utp_scsi_cmd_upiu(struct ufshcd_lrb *lrbp, u32 upiu_flags)
+{
+
Re: [PATCH 2/2] scsi: ufs: Set fDeviceInit flag to initiate device initialization
On 6/12/2013 11:04 AM, Santosh Y wrote:
/**
+ * ufshcd_query_request() - API for issuing query request to the device.
+ * @hba: ufs driver context
+ * @query: params for query request
+ * @descriptor: buffer for sending/receiving descriptor
+ * @retries: number of times to try executing the command
+ *
+ * All necessary fields for issuing a query and receiving its response
+ * are stored in the UFS hba struct. We can use this method since we know
+ * there is only one active query request or any internal command at all
+ * times.
+ */
+static int ufshcd_send_query_request(struct ufs_hba *hba,
+ struct ufs_query_req *query,
+ u8 *descriptor,
+ struct ufs_query_res *response)
+{
+ int ret;
+
+ BUG_ON(!hba);
+ if (!query || !response) {
+ dev_err(hba-dev,
+ %s: NULL pointer query = %p, response = %p\n,
+ __func__, query, response);
+ return -EINVAL;
+ }
+
+ mutex_lock(hba-i_cmd.dev_cmd_lock);
+ hba-i_cmd.query.request = query;
+ hba-i_cmd.query.response = response;
+ hba-i_cmd.query.descriptor = descriptor;
+
+ ret = ufshcd_exec_internal_cmd(hba, DEV_CMD_TYPE_QUERY,
+ QUERY_REQ_TIMEOUT);
Can this be generic, as external query commands might also use it?
External query commands can call ufshcd_send_query_request() directly,
without going into hassle of taking mutex lock and filling internal cmd
structure.
+
+ hba-i_cmd.query.request = NULL;
+ hba-i_cmd.query.response = NULL;
+ hba-i_cmd.query.descriptor = NULL;
+ mutex_unlock(hba-i_cmd.dev_cmd_lock);
+
+ return ret;
+}
+
+/**
--
Regards,
Sujit
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] scsi: ufs: Generalize UFS Interconnect Layer (UIC) command support
On 5/2/2013 12:49 PM, Santosh Y wrote:
-static inline void
-ufshcd_send_uic_command(struct ufs_hba *hba, struct uic_command *uic_cmnd)
+static int ufshcd_send_uic_command(struct ufs_hba *hba,
+ struct uic_command *uic_cmnd, int retries)
{
+ int err = 0;
+ unsigned long flags;
+
+ if (unlikely(mutex_trylock(>uic_cmd_mutex))) {
+ mutex_unlock(>uic_cmd_mutex);
+ dev_err(hba->dev, "%s: called without prepare command\n",
+ __func__);
+ BUG();
+ }
+
+retry:
+ /* check if controller is ready to accept UIC commands */
+ if (!(readl(hba->mmio_base + REG_CONTROLLER_STATUS) &
+ UIC_COMMAND_READY)) {
+ dev_err(hba->dev, "Controller not ready to accept UIC
commands\n");
+ err = -EIO;
+ goto out;
+ }
+
+ init_completion(_cmnd->completion);
+
+ spin_lock_irqsave(hba->host->host_lock, flags);
+
+ /* enable UIC related interrupts */
+ if (!(hba->int_enable_mask & UIC_COMMAND_COMPL)) {
+ hba->int_enable_mask |= UIC_COMMAND_COMPL;
+ ufshcd_int_config(hba, UFSHCD_INT_ENABLE);
+ }
+
No need to check if the interrupt is already enabled, it can be
directly enabled.
That would be unnecessary device access (writel) every time we send a
UIC command.
/* Write Args */
writel(uic_cmnd->argument1,
(hba->mmio_base + REG_UIC_COMMAND_ARG_1));
@@ -415,6 +491,45 @@ ufshcd_send_uic_command(struct ufs_hba *hba, struct
uic_command *uic_cmnd)
/* Write UIC Cmd */
writel((uic_cmnd->command & COMMAND_OPCODE_MASK),
(hba->mmio_base + REG_UIC_COMMAND));
+
+ /* flush to make sure h/w sees the write */
+ readl(hba->mmio_base + REG_UIC_COMMAND);
+
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+
mutex lock/unlock can be done in the send_uic_command, It will
simplify the process for the caller.
ufshcd_dme_get_attr()
{
1) ufshcd_prepare_uic_command_lck();
2) ufshcd_send_uic_command();
3) Read arg3 to know the value of Uni-pro attribute.
4) ufshcd_unprepare_uic_command_unlck();
}
Step 3 above is optional for some of the UIC commands but when it is
required it should be done with mutex lock held as the
hba->active_uic_cmd might be modified by some one else before value
present hba->active_uic_cmd->argument3 is read.
But thinking more on this holding lock only in send_uic_command() is
feasible.
+ err = wait_for_completion_timeout(
+ _cmnd->completion, UIC_COMMAND_TIMEOUT);
+ if (!err) {
+ err = -ETIMEDOUT;
+ } else if (uic_cmnd->argument2 & MASK_UIC_COMMAND_RESULT) {
+ /* something bad with h/w or arguments, try again */
+ if (--retries)
+ goto retry;
+
--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] scsi: ufs: Add support for sending NOP OUT UPIU
On 5/2/2013 12:57 PM, Santosh Y wrote:
+
+/**
+ * ufshcd_validate_dev_connection() - Check device connection status
+ * @hba: per-adapter instance
+ *
+ * Send NOP OUT UPIU and wait for NOP IN response to check whether the
+ * device Transport Protocol (UTP) layer is ready after a reset.
+ * If the UTP layer at the device side is not initialized, it may
+ * not respond with NOP IN UPIU within timeout of %NOP_OUT_TIMEOUT
+ * and we retry sending NOP OUT for %NOP_OUT_RETRIES iterations.
+ */
+static int ufshcd_validate_dev_connection(struct ufs_hba *hba)
+{
+ int err;
+ struct ufshcd_lrb *lrbp;
+ unsigned long timeout;
+ unsigned long flags;
+ struct completion wait;
+ int retries = NOP_OUT_RETRIES;
+
+retry:
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ lrbp = >lrb[INTERNAL_CMD_TAG];
+ init_completion();
+
+ err = ufshcd_compose_nop_out_upiu(hba, lrbp);
+ if (err)
+ goto may_retry;
+
+ lrbp->completion =
+ ufshcd_send_command(hba, INTERNAL_CMD_TAG);
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+
+ timeout = wait_for_completion_timeout(
+ , msecs_to_jiffies(NOP_OUT_TIMEOUT));
+
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ if (timeout > 0) {
+ int ocs;
+
+ ocs = ufshcd_get_tr_ocs(lrbp);
+ switch (ocs) {
+ case OCS_SUCCESS:
+ goto out;
+ default:
+ dev_dbg(hba->dev, "%s: OCS error %d\n", __func__, ocs);
+ err = -EIO;
+ goto may_retry;
+ }
+ } else {
+ u32 reg;
+
+ err = -ETIMEDOUT;
+
+ /* clear outstanding transaction before retry */
+ ufshcd_utrl_clear(hba, INTERNAL_CMD_TAG);
+ __clear_bit(INTERNAL_CMD_TAG, >outstanding_reqs);
+
+ /* poll for max. 1 sec to clear door bell register by h/w */
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ if (readl_poll_timeout(
+ hba->mmio_base + REG_UTP_TRANSFER_REQ_DOOR_BELL,
+ reg, !(reg & INTERNAL_CMD_TAG), 1000, 1000))
Condition is always true here, change it to !(reg & (1 << INTERNAL_CMD_TAG)).
Good catch. I will update.
+ retries = 0;
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ goto may_retry;
+ }
+
--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] scsi: ufs: Add support for sending NOP OUT UPIU
On 5/2/2013 12:57 PM, Santosh Y wrote:
+
+/**
+ * ufshcd_validate_dev_connection() - Check device connection status
+ * @hba: per-adapter instance
+ *
+ * Send NOP OUT UPIU and wait for NOP IN response to check whether the
+ * device Transport Protocol (UTP) layer is ready after a reset.
+ * If the UTP layer at the device side is not initialized, it may
+ * not respond with NOP IN UPIU within timeout of %NOP_OUT_TIMEOUT
+ * and we retry sending NOP OUT for %NOP_OUT_RETRIES iterations.
+ */
+static int ufshcd_validate_dev_connection(struct ufs_hba *hba)
+{
+ int err;
+ struct ufshcd_lrb *lrbp;
+ unsigned long timeout;
+ unsigned long flags;
+ struct completion wait;
+ int retries = NOP_OUT_RETRIES;
+
+retry:
+ spin_lock_irqsave(hba-host-host_lock, flags);
+ lrbp = hba-lrb[INTERNAL_CMD_TAG];
+ init_completion(wait);
+
+ err = ufshcd_compose_nop_out_upiu(hba, lrbp);
+ if (err)
+ goto may_retry;
+
+ lrbp-completion = wait;
+ ufshcd_send_command(hba, INTERNAL_CMD_TAG);
+ spin_unlock_irqrestore(hba-host-host_lock, flags);
+
+ timeout = wait_for_completion_timeout(
+ wait, msecs_to_jiffies(NOP_OUT_TIMEOUT));
+
+ spin_lock_irqsave(hba-host-host_lock, flags);
+ if (timeout 0) {
+ int ocs;
+
+ ocs = ufshcd_get_tr_ocs(lrbp);
+ switch (ocs) {
+ case OCS_SUCCESS:
+ goto out;
+ default:
+ dev_dbg(hba-dev, %s: OCS error %d\n, __func__, ocs);
+ err = -EIO;
+ goto may_retry;
+ }
+ } else {
+ u32 reg;
+
+ err = -ETIMEDOUT;
+
+ /* clear outstanding transaction before retry */
+ ufshcd_utrl_clear(hba, INTERNAL_CMD_TAG);
+ __clear_bit(INTERNAL_CMD_TAG, hba-outstanding_reqs);
+
+ /* poll for max. 1 sec to clear door bell register by h/w */
+ spin_unlock_irqrestore(hba-host-host_lock, flags);
+ if (readl_poll_timeout(
+ hba-mmio_base + REG_UTP_TRANSFER_REQ_DOOR_BELL,
+ reg, !(reg INTERNAL_CMD_TAG), 1000, 1000))
Condition is always true here, change it to !(reg (1 INTERNAL_CMD_TAG)).
Good catch. I will update.
+ retries = 0;
+ spin_lock_irqsave(hba-host-host_lock, flags);
+ goto may_retry;
+ }
+
--
Regards,
Sujit
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] scsi: ufs: Generalize UFS Interconnect Layer (UIC) command support
On 5/2/2013 12:49 PM, Santosh Y wrote:
-static inline void
-ufshcd_send_uic_command(struct ufs_hba *hba, struct uic_command *uic_cmnd)
+static int ufshcd_send_uic_command(struct ufs_hba *hba,
+ struct uic_command *uic_cmnd, int retries)
{
+ int err = 0;
+ unsigned long flags;
+
+ if (unlikely(mutex_trylock(hba-uic_cmd_mutex))) {
+ mutex_unlock(hba-uic_cmd_mutex);
+ dev_err(hba-dev, %s: called without prepare command\n,
+ __func__);
+ BUG();
+ }
+
+retry:
+ /* check if controller is ready to accept UIC commands */
+ if (!(readl(hba-mmio_base + REG_CONTROLLER_STATUS)
+ UIC_COMMAND_READY)) {
+ dev_err(hba-dev, Controller not ready to accept UIC
commands\n);
+ err = -EIO;
+ goto out;
+ }
+
+ init_completion(uic_cmnd-completion);
+
+ spin_lock_irqsave(hba-host-host_lock, flags);
+
+ /* enable UIC related interrupts */
+ if (!(hba-int_enable_mask UIC_COMMAND_COMPL)) {
+ hba-int_enable_mask |= UIC_COMMAND_COMPL;
+ ufshcd_int_config(hba, UFSHCD_INT_ENABLE);
+ }
+
No need to check if the interrupt is already enabled, it can be
directly enabled.
That would be unnecessary device access (writel) every time we send a
UIC command.
/* Write Args */
writel(uic_cmnd-argument1,
(hba-mmio_base + REG_UIC_COMMAND_ARG_1));
@@ -415,6 +491,45 @@ ufshcd_send_uic_command(struct ufs_hba *hba, struct
uic_command *uic_cmnd)
/* Write UIC Cmd */
writel((uic_cmnd-command COMMAND_OPCODE_MASK),
(hba-mmio_base + REG_UIC_COMMAND));
+
+ /* flush to make sure h/w sees the write */
+ readl(hba-mmio_base + REG_UIC_COMMAND);
+
+ spin_unlock_irqrestore(hba-host-host_lock, flags);
+
mutex lock/unlock can be done in the send_uic_command, It will
simplify the process for the caller.
ufshcd_dme_get_attr()
{
1) ufshcd_prepare_uic_command_lck();
2) ufshcd_send_uic_command();
3) Read arg3 to know the value of Uni-pro attribute.
4) ufshcd_unprepare_uic_command_unlck();
}
Step 3 above is optional for some of the UIC commands but when it is
required it should be done with mutex lock held as the
hba-active_uic_cmd might be modified by some one else before value
present hba-active_uic_cmd-argument3 is read.
But thinking more on this holding lock only in send_uic_command() is
feasible.
+ err = wait_for_completion_timeout(
+ uic_cmnd-completion, UIC_COMMAND_TIMEOUT);
+ if (!err) {
+ err = -ETIMEDOUT;
+ } else if (uic_cmnd-argument2 MASK_UIC_COMMAND_RESULT) {
+ /* something bad with h/w or arguments, try again */
+ if (--retries)
+ goto retry;
+
--
Regards,
Sujit
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] 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.
Signed-off-by: Sujit Reddy Thumma
---
drivers/scsi/ufs/ufshcd.c | 167 ++---
drivers/scsi/ufs/ufshcd.h |4 +
2 files changed, 162 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index ced421a..1a8cba3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -34,12 +34,21 @@
*/
#include "ufshcd.h"
+#include
+#include
/* Timeout after 10 secs if UIC command hangs */
#define UIC_COMMAND_TIMEOUT(10 * HZ)
/* Retry for 3 times in case of UIC failures */
#define UIC_COMMAND_RETRIES3
+/* 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 */
+/* Reserved tag for internal commands */
+#define INTERNAL_CMD_TAG 0
+
enum {
UFSHCD_MAX_CHANNEL = 0,
UFSHCD_MAX_ID = 1,
@@ -607,7 +616,7 @@ static void ufshcd_prepare_req_desc(struct ufshcd_lrb
*lrbp, u32 *upiu_flags)
{
struct utp_transfer_req_desc *req_desc = lrbp->utr_descriptor_ptr;
enum dma_data_direction cmd_dir =
- lrbp->cmd->sc_data_direction;
+ lrbp->cmd ? lrbp->cmd->sc_data_direction : DMA_NONE;
u32 data_direction;
u32 dword_0;
@@ -624,6 +633,8 @@ static void ufshcd_prepare_req_desc(struct ufshcd_lrb
*lrbp, u32 *upiu_flags)
dword_0 = data_direction | (lrbp->command_type
<< UPIU_COMMAND_TYPE_OFFSET);
+ if (lrbp->intr_cmd)
+ dword_0 |= UTP_REQ_DESC_INT_CMD;
/* Transfer request descriptor header fields */
req_desc->header.dword_0 = cpu_to_le32(dword_0);
@@ -712,6 +723,18 @@ static inline void
ufshcd_prepare_utp_query_req_upiu(struct ufs_hba *hba,
}
+static inline void ufshcd_prepare_utp_nop_upiu(struct ufshcd_lrb *lrbp)
+{
+ struct utp_upiu_req *ucd_req_ptr = lrbp->ucd_req_ptr;
+
+ memset(ucd_req_ptr, 0, sizeof(struct utp_upiu_req));
+
+ /* command descriptor fields */
+ ucd_req_ptr->header.dword_0 =
+ UPIU_HEADER_DWORD(
+ UPIU_TRANSACTION_NOP_OUT, 0, 0, lrbp->task_tag);
+}
+
/**
* ufshcd_compose_upiu - form UFS Protocol Information Unit(UPIU)
* @hba - UFS hba
@@ -741,12 +764,14 @@ static int ufshcd_compose_upiu(struct ufs_hba *hba,
struct ufshcd_lrb *lrbp)
case UTP_CMD_TYPE_SCSI:
case UTP_CMD_TYPE_DEV_MANAGE:
ufshcd_prepare_req_desc(lrbp, _flags);
- if (lrbp->cmd && lrbp->command_type == UTP_CMD_TYPE_SCSI)
+ if (lrbp->cmd && lrbp->command_type == UTP_CMD_TYPE_SCSI) {
ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);
- else if (lrbp->cmd)
+ } else if (lrbp->cmd && ufshcd_is_query_req(lrbp)) {
ufshcd_prepare_utp_query_req_upiu(hba, lrbp,
upiu_flags);
- else {
+ } else if (!lrbp->cmd) {
+ ufshcd_prepare_utp_nop_upiu(lrbp);
+ } else {
dev_err(hba->dev, "%s: Invalid UPIU request\n",
__func__);
ret = -EINVAL;
@@ -799,6 +824,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *cmd)
lrbp->sense_buffer = cmd->sense_buffer;
lrbp->task_tag = tag;
lrbp->lun = cmd->device->lun;
+ lrbp->intr_cmd = false;
if (ufshcd_is_query_req(lrbp))
lrbp->command_type = UTP_CMD_TYPE_DEV_MANAGE;
@@ -1112,6 +1138,113 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba)
return err;
}
+static inline int
+ufshcd_compose_nop_out_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
+{
+ lrbp->cmd = NULL;
+ lrbp->sense_bufflen = 0;
+ lrbp->sense_buffer = NULL;
+ lrbp->task_tag = INTERNAL_CMD_TAG;
+ lrbp->lun = 0; /* NOP OUT is not specific to any LUN */
+ lrbp->command_type = UTP_CMD_TYPE_DEV_MANAGE;
+ lrbp->intr_cmd = true; /* No interrupt aggregation */
+
+ return ufshcd_compose_upiu(hba, lrbp);
+}
+
+/**
+ * ufshcd_validate_dev_connection() - Check device connection status
+ * @hba: per-adapter instance
+ *
+ * Send NOP OUT UPIU and wait for NOP IN respons
[PATCH 1/1] 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.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufshcd.c | 167 ++---
drivers/scsi/ufs/ufshcd.h |4 +
2 files changed, 162 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index ced421a..1a8cba3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -34,12 +34,21 @@
*/
#include ufshcd.h
+#include linux/iopoll.h
+#include linux/async.h
/* Timeout after 10 secs if UIC command hangs */
#define UIC_COMMAND_TIMEOUT(10 * HZ)
/* Retry for 3 times in case of UIC failures */
#define UIC_COMMAND_RETRIES3
+/* 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 */
+/* Reserved tag for internal commands */
+#define INTERNAL_CMD_TAG 0
+
enum {
UFSHCD_MAX_CHANNEL = 0,
UFSHCD_MAX_ID = 1,
@@ -607,7 +616,7 @@ static void ufshcd_prepare_req_desc(struct ufshcd_lrb
*lrbp, u32 *upiu_flags)
{
struct utp_transfer_req_desc *req_desc = lrbp-utr_descriptor_ptr;
enum dma_data_direction cmd_dir =
- lrbp-cmd-sc_data_direction;
+ lrbp-cmd ? lrbp-cmd-sc_data_direction : DMA_NONE;
u32 data_direction;
u32 dword_0;
@@ -624,6 +633,8 @@ static void ufshcd_prepare_req_desc(struct ufshcd_lrb
*lrbp, u32 *upiu_flags)
dword_0 = data_direction | (lrbp-command_type
UPIU_COMMAND_TYPE_OFFSET);
+ if (lrbp-intr_cmd)
+ dword_0 |= UTP_REQ_DESC_INT_CMD;
/* Transfer request descriptor header fields */
req_desc-header.dword_0 = cpu_to_le32(dword_0);
@@ -712,6 +723,18 @@ static inline void
ufshcd_prepare_utp_query_req_upiu(struct ufs_hba *hba,
}
+static inline void ufshcd_prepare_utp_nop_upiu(struct ufshcd_lrb *lrbp)
+{
+ struct utp_upiu_req *ucd_req_ptr = lrbp-ucd_req_ptr;
+
+ memset(ucd_req_ptr, 0, sizeof(struct utp_upiu_req));
+
+ /* command descriptor fields */
+ ucd_req_ptr-header.dword_0 =
+ UPIU_HEADER_DWORD(
+ UPIU_TRANSACTION_NOP_OUT, 0, 0, lrbp-task_tag);
+}
+
/**
* ufshcd_compose_upiu - form UFS Protocol Information Unit(UPIU)
* @hba - UFS hba
@@ -741,12 +764,14 @@ static int ufshcd_compose_upiu(struct ufs_hba *hba,
struct ufshcd_lrb *lrbp)
case UTP_CMD_TYPE_SCSI:
case UTP_CMD_TYPE_DEV_MANAGE:
ufshcd_prepare_req_desc(lrbp, upiu_flags);
- if (lrbp-cmd lrbp-command_type == UTP_CMD_TYPE_SCSI)
+ if (lrbp-cmd lrbp-command_type == UTP_CMD_TYPE_SCSI) {
ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);
- else if (lrbp-cmd)
+ } else if (lrbp-cmd ufshcd_is_query_req(lrbp)) {
ufshcd_prepare_utp_query_req_upiu(hba, lrbp,
upiu_flags);
- else {
+ } else if (!lrbp-cmd) {
+ ufshcd_prepare_utp_nop_upiu(lrbp);
+ } else {
dev_err(hba-dev, %s: Invalid UPIU request\n,
__func__);
ret = -EINVAL;
@@ -799,6 +824,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *cmd)
lrbp-sense_buffer = cmd-sense_buffer;
lrbp-task_tag = tag;
lrbp-lun = cmd-device-lun;
+ lrbp-intr_cmd = false;
if (ufshcd_is_query_req(lrbp))
lrbp-command_type = UTP_CMD_TYPE_DEV_MANAGE;
@@ -1112,6 +1138,113 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba)
return err;
}
+static inline int
+ufshcd_compose_nop_out_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
+{
+ lrbp-cmd = NULL;
+ lrbp-sense_bufflen = 0;
+ lrbp-sense_buffer = NULL;
+ lrbp-task_tag = INTERNAL_CMD_TAG;
+ lrbp-lun = 0; /* NOP OUT is not specific to any LUN */
+ lrbp-command_type = UTP_CMD_TYPE_DEV_MANAGE;
+ lrbp-intr_cmd = true; /* No interrupt aggregation */
+
+ return ufshcd_compose_upiu(hba, lrbp);
+}
+
+/**
+ * ufshcd_validate_dev_connection() - Check device connection status
+ * @hba: per-adapter instance
+ *
+ * Send NOP OUT UPIU and wait for NOP IN response to check whether the
+ * device Transport Protocol (UTP) layer is ready after a reset.
+ * If the UTP layer at the device side is not initialized
[PATCH 1/1] scsi: ufs: Generalize UFS Interconnect Layer (UIC) command support
Currently, only DME_LINKSTARTUP UIC command is fully supported,
generalize this to send any UIC command. A new uic_cmd_mutex
is introduced and the callers are expected to hold this mutex
lock before preparing and sending UIC command as the specification
doesn't allow more than one UIC command at a time.
Further, the command completion for DME_LINKSTARTUP is modified
and the command completes in the context of the caller instead
of a separate work.
Signed-off-by: Sujit Reddy Thumma
---
drivers/scsi/ufs/ufshcd.c | 257 +
drivers/scsi/ufs/ufshcd.h |7 +-
2 files changed, 194 insertions(+), 70 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 3f67225..ced421a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -35,6 +35,11 @@
#include "ufshcd.h"
+/* Timeout after 10 secs if UIC command hangs */
+#define UIC_COMMAND_TIMEOUT(10 * HZ)
+/* Retry for 3 times in case of UIC failures */
+#define UIC_COMMAND_RETRIES3
+
enum {
UFSHCD_MAX_CHANNEL = 0,
UFSHCD_MAX_ID = 1,
@@ -51,7 +56,7 @@ enum {
};
/* Interrupt configuration options */
-enum {
+enum ufshcd_int_cfg {
UFSHCD_INT_DISABLE,
UFSHCD_INT_ENABLE,
UFSHCD_INT_CLEAR,
@@ -63,6 +68,8 @@ enum {
INT_AGGR_CONFIG,
};
+static void ufshcd_int_config(struct ufs_hba *hba, enum ufshcd_int_cfg option);
+
/**
* ufshcd_get_ufs_version - Get the UFS version supported by the HBA
* @hba - Pointer to adapter instance
@@ -157,19 +164,6 @@ static inline int ufshcd_get_lists_status(u32 reg)
}
/**
- * ufshcd_get_uic_cmd_result - Get the UIC command result
- * @hba: Pointer to adapter instance
- *
- * This function gets the result of UIC command completion
- * Returns 0 on success, non zero value on error
- */
-static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
-{
- return readl(hba->mmio_base + REG_UIC_COMMAND_ARG_2) &
- MASK_UIC_COMMAND_RESULT;
-}
-
-/**
* ufshcd_free_hba_memory - Free allocated memory for LRB, request
* and task lists
* @hba: Pointer to adapter instance
@@ -397,13 +391,95 @@ static inline void ufshcd_hba_capabilities(struct ufs_hba
*hba)
}
/**
+ * ufshcd_prepare_uic_command_lck() - prepare UIC command
+ * @hba: per-adapter instance
+ * @uic_cmd: pointer to UIC command structure
+ * @cmd: UIC command
+ * @arg1: argument 1
+ * @arg2: argument 2
+ * @arg3: argument 3
+ *
+ * Hold UIC command mutex and prepare UIC command structure.
+ * Proper sequence for sending UIC command is:
+ * 1) ufshcd_prepare_uic_command_lck()
+ * 2) ufshcd_send_uic_command()
+ * 3) check argument 2 and argument 3 for results
+ * 4) ufshcd_unprepare_uic_command_unlck()
+ *
+ * Note: Step 3 is optional. But when it is required it should be
+ * with mutex lock held.
+ */
+static void ufshcd_prepare_uic_command_lck(struct ufs_hba *hba,
+ struct uic_command *uic_cmd, u32 cmd,
+ u32 arg1, u32 arg2, u32 arg3)
+{
+ mutex_lock(>uic_cmd_mutex);
+ WARN_ON(hba->active_uic_cmd);
+
+ /* form UIC command */
+ uic_cmd->command = cmd;
+ uic_cmd->argument1 = arg1;
+ uic_cmd->argument2 = arg2;
+ uic_cmd->argument3 = arg3;
+
+ hba->active_uic_cmd = uic_cmd;
+}
+
+/**
+ * ufshcd_unprepare_uic_command_unlck() - unprepare UIC command
+ * @hba: per-adapter instance
+ *
+ * Release UIC mutex and point active uic command to NULL.
+ *
+ * See comments in ufshcd_prepare_uic_command_lck().
+ */
+static void ufshcd_unprepare_uic_command_unlck(struct ufs_hba *hba)
+{
+ hba->active_uic_cmd = NULL;
+ mutex_unlock(>uic_cmd_mutex);
+}
+
+/**
* ufshcd_send_uic_command - Send UIC commands to unipro layers
* @hba: per adapter instance
* @uic_command: UIC command
+ * @retries: number of retries in case of failure.
+ *
+ * See comments in ufshcd_prepare_uic_command_lck().
+ * Returns 0 on success, negative value on failure.
*/
-static inline void
-ufshcd_send_uic_command(struct ufs_hba *hba, struct uic_command *uic_cmnd)
+static int ufshcd_send_uic_command(struct ufs_hba *hba,
+ struct uic_command *uic_cmnd, int retries)
{
+ int err = 0;
+ unsigned long flags;
+
+ if (unlikely(mutex_trylock(>uic_cmd_mutex))) {
+ mutex_unlock(>uic_cmd_mutex);
+ dev_err(hba->dev, "%s: called without prepare command\n",
+ __func__);
+ BUG();
+ }
+
+retry:
+ /* check if controller is ready to accept UIC commands */
+ if (!(readl(hba->mmio_base + REG_CONTROLLER_STATUS) &
+ UIC_COMMAND_READY)) {
+ dev_err(hba->dev, "Controller not ready to accept UIC
commands\n");
+ err = -EIO;
+ goto out;
+ }
[PATCH 1/1] scsi: ufs: Generalize UFS Interconnect Layer (UIC) command support
Currently, only DME_LINKSTARTUP UIC command is fully supported,
generalize this to send any UIC command. A new uic_cmd_mutex
is introduced and the callers are expected to hold this mutex
lock before preparing and sending UIC command as the specification
doesn't allow more than one UIC command at a time.
Further, the command completion for DME_LINKSTARTUP is modified
and the command completes in the context of the caller instead
of a separate work.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufshcd.c | 257 +
drivers/scsi/ufs/ufshcd.h |7 +-
2 files changed, 194 insertions(+), 70 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 3f67225..ced421a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -35,6 +35,11 @@
#include ufshcd.h
+/* Timeout after 10 secs if UIC command hangs */
+#define UIC_COMMAND_TIMEOUT(10 * HZ)
+/* Retry for 3 times in case of UIC failures */
+#define UIC_COMMAND_RETRIES3
+
enum {
UFSHCD_MAX_CHANNEL = 0,
UFSHCD_MAX_ID = 1,
@@ -51,7 +56,7 @@ enum {
};
/* Interrupt configuration options */
-enum {
+enum ufshcd_int_cfg {
UFSHCD_INT_DISABLE,
UFSHCD_INT_ENABLE,
UFSHCD_INT_CLEAR,
@@ -63,6 +68,8 @@ enum {
INT_AGGR_CONFIG,
};
+static void ufshcd_int_config(struct ufs_hba *hba, enum ufshcd_int_cfg option);
+
/**
* ufshcd_get_ufs_version - Get the UFS version supported by the HBA
* @hba - Pointer to adapter instance
@@ -157,19 +164,6 @@ static inline int ufshcd_get_lists_status(u32 reg)
}
/**
- * ufshcd_get_uic_cmd_result - Get the UIC command result
- * @hba: Pointer to adapter instance
- *
- * This function gets the result of UIC command completion
- * Returns 0 on success, non zero value on error
- */
-static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
-{
- return readl(hba-mmio_base + REG_UIC_COMMAND_ARG_2)
- MASK_UIC_COMMAND_RESULT;
-}
-
-/**
* ufshcd_free_hba_memory - Free allocated memory for LRB, request
* and task lists
* @hba: Pointer to adapter instance
@@ -397,13 +391,95 @@ static inline void ufshcd_hba_capabilities(struct ufs_hba
*hba)
}
/**
+ * ufshcd_prepare_uic_command_lck() - prepare UIC command
+ * @hba: per-adapter instance
+ * @uic_cmd: pointer to UIC command structure
+ * @cmd: UIC command
+ * @arg1: argument 1
+ * @arg2: argument 2
+ * @arg3: argument 3
+ *
+ * Hold UIC command mutex and prepare UIC command structure.
+ * Proper sequence for sending UIC command is:
+ * 1) ufshcd_prepare_uic_command_lck()
+ * 2) ufshcd_send_uic_command()
+ * 3) check argument 2 and argument 3 for results
+ * 4) ufshcd_unprepare_uic_command_unlck()
+ *
+ * Note: Step 3 is optional. But when it is required it should be
+ * with mutex lock held.
+ */
+static void ufshcd_prepare_uic_command_lck(struct ufs_hba *hba,
+ struct uic_command *uic_cmd, u32 cmd,
+ u32 arg1, u32 arg2, u32 arg3)
+{
+ mutex_lock(hba-uic_cmd_mutex);
+ WARN_ON(hba-active_uic_cmd);
+
+ /* form UIC command */
+ uic_cmd-command = cmd;
+ uic_cmd-argument1 = arg1;
+ uic_cmd-argument2 = arg2;
+ uic_cmd-argument3 = arg3;
+
+ hba-active_uic_cmd = uic_cmd;
+}
+
+/**
+ * ufshcd_unprepare_uic_command_unlck() - unprepare UIC command
+ * @hba: per-adapter instance
+ *
+ * Release UIC mutex and point active uic command to NULL.
+ *
+ * See comments in ufshcd_prepare_uic_command_lck().
+ */
+static void ufshcd_unprepare_uic_command_unlck(struct ufs_hba *hba)
+{
+ hba-active_uic_cmd = NULL;
+ mutex_unlock(hba-uic_cmd_mutex);
+}
+
+/**
* ufshcd_send_uic_command - Send UIC commands to unipro layers
* @hba: per adapter instance
* @uic_command: UIC command
+ * @retries: number of retries in case of failure.
+ *
+ * See comments in ufshcd_prepare_uic_command_lck().
+ * Returns 0 on success, negative value on failure.
*/
-static inline void
-ufshcd_send_uic_command(struct ufs_hba *hba, struct uic_command *uic_cmnd)
+static int ufshcd_send_uic_command(struct ufs_hba *hba,
+ struct uic_command *uic_cmnd, int retries)
{
+ int err = 0;
+ unsigned long flags;
+
+ if (unlikely(mutex_trylock(hba-uic_cmd_mutex))) {
+ mutex_unlock(hba-uic_cmd_mutex);
+ dev_err(hba-dev, %s: called without prepare command\n,
+ __func__);
+ BUG();
+ }
+
+retry:
+ /* check if controller is ready to accept UIC commands */
+ if (!(readl(hba-mmio_base + REG_CONTROLLER_STATUS)
+ UIC_COMMAND_READY)) {
+ dev_err(hba-dev, Controller not ready to accept UIC
commands\n);
+ err = -EIO;
+ goto out;
+ }
+
+ init_completion(uic_cmnd-completion
