Re: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
On 7/19/2013 7:17 PM, Seungwon Jeon wrote:
On Thu, July 18, 2013, Sujit Reddy Thumma 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)
I feel like this function's role is to wait for clearing register (specially,
doorbells).
If you really don't intend to apply other all register, I think it would better
to change the
function name.
ex> ufshcd_wait_for_clear_doorbell or ufshcd_wait_for_clear_reg?
Although, this API is introduced in this patch and used only for
clearing the doorbell, I have intentionally made it generic to avoid
duplication of wait condition code in future (not just clearing but
also for setting a bit). For example, when we write to HCE.ENABLE we
wait for it turn to 1.
And if you like it, it could be more simple like below
static bool ufshcd_wait_for_clear_reg(struct ufs_hba *hba, u32 reg, u32 mask,
unsigned long interval_us,
unsigned int timeout_ms)
{
unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
Jiffies on 100Hz systems have minimum granularity of 10ms. So we really
wait for more 10ms even though the timeout_ms < 10ms.
Yes. Real timeout depends on system.
But normally actual wait will be maintained until 'ufshcd_readl' is done.
Timeout is valid for failure case. If we don't need to apply a strict timeout
value, it's not bad.
Hmm.. make sense. Will take care in the next patchset.
/* wakeup within 50us of expiry */
const unsigned int expiry = 50;
while (ufshcd_readl(hba, reg) & mask) {
usleep_range(interval_us, interval_us + expiry);
if (time_after(jiffies, timeout)) {
if (ufshcd_readl(hba, reg) & mask)
return false;
I really want the caller to decide on what to do after the timeout.
This helps in error handling cases where we try to clear off the entire
door-bell register but only a few of the bits are cleared after timeout.
I don't know what we can do with the report of the partial success on clearing
bit.
It's just failure case. Isn't enough with false/true?
The point is, if a bit can't be cleared it can be regarded as a
permanent failure (only for that request), otherwise, it can be
retried with the same tag value.
else
return true;
}
}
return true;
}
+{
+ u32 tmp;
+ ktime_t start;
+ unsigned long diff;
+
+ tmp = ufshcd_readl(hba, reg);
+
+ if ((val & mask) != val) {
+ dev_err(hba->dev, "%s: Invalid wait condition 0x%x\n",
+ __func__, val);
+ goto out;
+ }
+
+ start = ktime_get();
+ while ((tmp & mask) != val) {
+ /* 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);
+ break;
+ }
+ }
+out:
+ return tmp;
+}
+
..
+static int
+ufshcd_clear_cmd(struct ufs_hba *hba, int tag)
+{
+ int err = 0;
+ unsigned long flags;
+ u32 reg;
+ u32 mask = 1 << tag;
+
+ /* clear outstanding transaction before retry */
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ ufshcd_utrl_clear(hba, tag);
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+
+ /*
+* wait for for h/w to clear corresponding bit in door-bell.
+* max. wait is 1 sec.
+*/
+ reg = ufshcd_wait_for_register(hba,
+ REG_UTP_TRANSFER_REQ_DOOR_BELL,
+ mask, 0, 1000, 1000);
4th argument should be (~mask) instead of '0', right?
True, but not really for this implementation. It breaks the check in
in wait_for_register -
if ((val & mask) != val)
dev_err(...);
Hmm, it seems complicated to use.
Ok. Is right the following about val as 4th argument?
- clear: val should be '0' regardless corresponding bit.
- set: val should be same with mask.
If the related comment is added, it will be helpful.
Thinking again it looks like it is complicated. How about changing
the check to -
val = val & mask; /* ignore the bits we don't intend to wait on */
while (ufshcd
RE: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
On Thu, July 18, 2013, Sujit Reddy Thumma 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)
> >>> I feel like this function's role is to wait for clearing register
> >>> (specially, doorbells).
> >>> If you really don't intend to apply other all register, I think it would
> >>> better to change the
> >> function name.
> >>> ex> ufshcd_wait_for_clear_doorbell or ufshcd_wait_for_clear_reg?
> >>
> >> Although, this API is introduced in this patch and used only for
> >> clearing the doorbell, I have intentionally made it generic to avoid
> >> duplication of wait condition code in future (not just clearing but
> >> also for setting a bit). For example, when we write to HCE.ENABLE we
> >> wait for it turn to 1.
> >>
> >>
> >>> And if you like it, it could be more simple like below
> >>>
> >>> static bool ufshcd_wait_for_clear_reg(struct ufs_hba *hba, u32 reg, u32
> >>> mask,
> >>>unsigned long interval_us,
> >>>unsigned int timeout_ms)
> >>> {
> >>> unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
> >>
> >> Jiffies on 100Hz systems have minimum granularity of 10ms. So we really
> >> wait for more 10ms even though the timeout_ms < 10ms.
> > Yes. Real timeout depends on system.
> > But normally actual wait will be maintained until 'ufshcd_readl' is done.
> > Timeout is valid for failure case. If we don't need to apply a strict
> > timeout value, it's not bad.
>
> Hmm.. make sense. Will take care in the next patchset.
>
> >
> >>
> >>> /* wakeup within 50us of expiry */
> >>> const unsigned int expiry = 50;
> >>>
> >>> while (ufshcd_readl(hba, reg) & mask) {
> >>> usleep_range(interval_us, interval_us + expiry);
> >>> if (time_after(jiffies, timeout)) {
> >>> if (ufshcd_readl(hba, reg) & mask)
> >>> return false;
> >>
> >> I really want the caller to decide on what to do after the timeout.
> >> This helps in error handling cases where we try to clear off the entire
> >> door-bell register but only a few of the bits are cleared after timeout.
> > I don't know what we can do with the report of the partial success on
> > clearing bit.
> > It's just failure case. Isn't enough with false/true?
>
> The point is, if a bit can't be cleared it can be regarded as a
> permanent failure (only for that request), otherwise, it can be
> retried with the same tag value.
>
> >
> >>
> >>> else
> >>> return true;
> >>> }
> >>> }
> >>>
> >>> return true;
> >>> }
> +{
> +u32 tmp;
> +ktime_t start;
> +unsigned long diff;
> +
> +tmp = ufshcd_readl(hba, reg);
> +
> +if ((val & mask) != val) {
> +dev_err(hba->dev, "%s: Invalid wait condition 0x%x\n",
> +__func__, val);
> +goto out;
> +}
> +
> +start = ktime_get();
> +while ((tmp & mask) != val) {
> +/* 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);
> +break;
> +}
> +}
> +out:
> +return tmp;
> +}
> +
> ..
> +static int
> +ufshcd_clear_cmd(struct ufs_hba *hba, int tag)
> +{
> +int err = 0;
> +unsigned long flags;
> +u32 reg;
> +u32 mask = 1 << tag;
> +
> +/* clear outstanding transaction before retry */
> +spin_lock_irqsave(hba->host->host_lock, flags);
> +ufshcd_utrl_clear(hba, tag);
> +spin_unlock_irqrestore(hba->host->host_lock, flags);
> +
> +/*
> + * wait for for h/w to clear corresponding bit in door-bell.
> + * max. wait is 1 sec.
> + */
> +reg = ufshcd_wait_for_register(hba,
> +
Re: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
+ * 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)
I feel like this function's role is to wait for clearing register (specially,
doorbells).
If you really don't intend to apply other all register, I think it would better
to change the
function name.
ex> ufshcd_wait_for_clear_doorbell or ufshcd_wait_for_clear_reg?
Although, this API is introduced in this patch and used only for
clearing the doorbell, I have intentionally made it generic to avoid
duplication of wait condition code in future (not just clearing but
also for setting a bit). For example, when we write to HCE.ENABLE we
wait for it turn to 1.
And if you like it, it could be more simple like below
static bool ufshcd_wait_for_clear_reg(struct ufs_hba *hba, u32 reg, u32 mask,
unsigned long interval_us,
unsigned int timeout_ms)
{
unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
Jiffies on 100Hz systems have minimum granularity of 10ms. So we really
wait for more 10ms even though the timeout_ms < 10ms.
Yes. Real timeout depends on system.
But normally actual wait will be maintained until 'ufshcd_readl' is done.
Timeout is valid for failure case. If we don't need to apply a strict timeout
value, it's not bad.
Hmm.. make sense. Will take care in the next patchset.
/* wakeup within 50us of expiry */
const unsigned int expiry = 50;
while (ufshcd_readl(hba, reg) & mask) {
usleep_range(interval_us, interval_us + expiry);
if (time_after(jiffies, timeout)) {
if (ufshcd_readl(hba, reg) & mask)
return false;
I really want the caller to decide on what to do after the timeout.
This helps in error handling cases where we try to clear off the entire
door-bell register but only a few of the bits are cleared after timeout.
I don't know what we can do with the report of the partial success on clearing
bit.
It's just failure case. Isn't enough with false/true?
The point is, if a bit can't be cleared it can be regarded as a
permanent failure (only for that request), otherwise, it can be
retried with the same tag value.
else
return true;
}
}
return true;
}
+{
+ u32 tmp;
+ ktime_t start;
+ unsigned long diff;
+
+ tmp = ufshcd_readl(hba, reg);
+
+ if ((val & mask) != val) {
+ dev_err(hba->dev, "%s: Invalid wait condition 0x%x\n",
+ __func__, val);
+ goto out;
+ }
+
+ start = ktime_get();
+ while ((tmp & mask) != val) {
+ /* 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);
+ break;
+ }
+ }
+out:
+ return tmp;
+}
+
..
+static int
+ufshcd_clear_cmd(struct ufs_hba *hba, int tag)
+{
+ int err = 0;
+ unsigned long flags;
+ u32 reg;
+ u32 mask = 1 << tag;
+
+ /* clear outstanding transaction before retry */
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ ufshcd_utrl_clear(hba, tag);
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+
+ /*
+* wait for for h/w to clear corresponding bit in door-bell.
+* max. wait is 1 sec.
+*/
+ reg = ufshcd_wait_for_register(hba,
+ REG_UTP_TRANSFER_REQ_DOOR_BELL,
+ mask, 0, 1000, 1000);
4th argument should be (~mask) instead of '0', right?
True, but not really for this implementation. It breaks the check in
in wait_for_register -
if ((val & mask) != val)
dev_err(...);
Hmm, it seems complicated to use.
Ok. Is right the following about val as 4th argument?
- clear: val should be '0' regardless corresponding bit.
- set: val should be same with mask.
If the related comment is added, it will be helpful.
Thinking again it looks like it is complicated. How about changing
the check to -
val = val & mask; /* ignore the bits we don't intend to wait on */
while (ufshcd_readl() & mask != val) {
sleep
}
Usage will be ~mask for clearing the bits, mask for setting the bits
in
RE: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
On Thu, July 11, 2013, Sujit Reddy Thumma wrote:
> On 7/10/2013 6:58 PM, Seungwon Jeon wrote:
> > On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
> >> 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
> >> Signed-off-by: Dolev Raviv
> >> ---
> >> 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
> >> +#include
> >> +
> >> #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 retri
Re: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
On 7/10/2013 6:58 PM, Seungwon Jeon wrote:
> On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
>> 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
>> Signed-off-by: Dolev Raviv
>> ---
>> 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
>> +#include
>> +
>> #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_OFFSET28
>> +
>> +/* Offset of the response code in the UPIU header */
>> +#define UPIU_RSP_CODE_OFFSET8
>> +
>> 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
RE: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
> 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
> Signed-off-by: Dolev Raviv
> ---
> 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
> +#include
> +
> #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
> + * @re
Re: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU
Tested-by: Maya Erez
> 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
> Signed-off-by: Dolev Raviv
> ---
> 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
> +#include
> +
> #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 - m
