Re: [PATCH v4 07/17] clocksource: bcm2835: Switch to sched_clock_register()
On 07/18/2013 05:21 PM, Stephen Boyd wrote: > The 32 bit sched_clock interface now supports 64 bits. Upgrade to > the 64 bit function to allow us to remove the 32 bit registration > interface. Acked-by: Stephen Warren -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 13/17] clocksource: tegra: Switch to sched_clock_register()
On 07/18/2013 05:21 PM, Stephen Boyd wrote: > The 32 bit sched_clock interface now supports 64 bits. Upgrade to > the 64 bit function to allow us to remove the 32 bit registration > interface. Acked-by: Stephen Warren Tested-by: Stephen Warren -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 1/2] scsi: ufs: Add support for host assisted background operations
I'm not sure that BKOPS with runtime-pm associates.
Do you think it's helpful for power management?
How about hibernation scheme for runtime-pm?
I'm testing and I can introduce soon.
Well, I am thinking on following approach when we introduce
power management.
ufshcd_runtime_suspend() {
if (bkops_status >= NON_CRITICAL) { /* 0x1 */
ufshcd_enable_auto_bkops();
hibernate(); /* only the link and the device
should be able to cary out bkops */
} else {
hibernate(); /* Link and the device for more savings */
}
}
Let me know if this is okay.
I still consider whether BKOPS is proper behavior with runtime-pm or not.
The BKOPS is something that host allows the card to carry out
when the host knows it is idle and not expecting back to back requests.
Runtime PM idle is the only way to know whether the device is
idle (unless we want to reinvent the wheel to detect the idleness of
host and trigger bkops). There was a discussion on this in MMC mailing
list as well, and folks have agreed to move idle time bkops to runtime
PM (http://thread.gmane.org/gmane.linux.kernel.mmc/19444/)
It looks like different.
eMMC cannot execute BKOPS itself unlike UFS.
That's the way eMMC's host should trigger the BKOPS manually.
I guess it is not much of a difference for UFS as far as we concern
only about idle time bkops. In UFS one can still disallow the device to
not carry out BKOPS and hence the case where UFS device also cannot
execute BKOPS itself and a idle timer is needed to allow BKOPS
sporadically so that device doesn't go into URGENT_BKOPS state.
How about this scenario? It seems more simple.
> > If we concern a response latency of transfer requests, BKOPS can be
disabled by default.
And then BKOPS can be enabled whenever device requests in some exception.
If you have any idea, let me know.
Exceptions are raised only when the device is in critical need for
bkops. Also the spec. recommends, host should ensure that the device
doesn't go into such states.
With your suggestion, when we disable bkops, the exception is raised and
we enable bkops after which there is no way to disable it again?
Yes, it's difficult to find proper time.
Maybe, BKOPS can be disabled when request comes up.
In cases where there are back-to-back heavy data write requests then it
is not proper to enable/disable BKOPS as soon as the new request comes
up. It may happen that the card will then move from PERFORMANCE_IMPACT
state to CRITICAL and ultimately start failing the requests. The point
is that we should never get into state where we need URGENT_BKOPS.
--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 3/4] scsi: ufs: Fix device and host reset methods
On 7/19/2013 7:27 PM, Seungwon Jeon wrote:
> On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
>> 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
>> ---
>> 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) {
> Lock is no needed for 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
Re: [PATCH V3 1/4] scsi: ufs: Fix broken task management command implementation
On 7/19/2013 7:26 PM, Seungwon Jeon wrote:
> On Tue, July 09, 2013 Sujit Reddy Thumma wrote:
>> 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
>> ---
>> 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,
Re: [PATCH V3 4/4] scsi: ufs: Improve UFS fatal error handling
On 7/19/2013 7:28 PM, Seungwon Jeon wrote:
> On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
>> 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
> Attention'i', typo.
Okay.
>
>>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
>> ---
>> 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
Re: [PATCH V3 2/4] scsi: ufs: Fix hardware race conditions while aborting a command
On 7/19/2013 7:26 PM, Seungwon Jeon wrote:
> On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
>> 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.
> It's interesting.
> I wonder when we can meet this situation.
> Is it possible if SCSI mid layer should send abort command as soon as the
> transfer command is issued?
> AFAIK abort command is followed if one command has timed out.
> That means command have been already issued and no response?
> If you had some problem, could you share?
You are right. This is a very rare case and probably don't happen at all
until and unless the SCSI error handling is changed. We found it just by
static analysis. Probably, at some point I may push a patch that tries
to reduce the read latencies while aborting a large write transfer when
I come across a UFS device that has command per LU as 1 :-)
I guess this is good to have change. The path chosen is according to
SCSI SAM-5 architecture specification, so I don't expect any issues
here.
>
>> - If the device recieves abort command first then it returns success
> receives
>
>>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
>> ---
>> 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 respons
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 v4 04/17] sched_clock: Add support for >32 bit sched_clock
On 07/19, Baruch Siach wrote: > On Thu, Jul 18, 2013 at 04:21:17PM -0700, Stephen Boyd wrote: > > @@ -110,14 +123,13 @@ void __init setup_sched_clock(u32 (*read)(void), int > > bits, unsigned long rate) > > if (cd.rate > rate) > > return; > > > > - BUG_ON(bits > 32); > > WARN_ON(!irqs_disabled()); > > read_sched_clock = read; > > - sched_clock_mask = (1 << bits) - 1; > > + sched_clock_mask = CLOCKSOURCE_MASK(bits); > > Note that this conflicts with my integer overflow fix > (http://article.gmane.org/gmane.linux.ports.arm.kernel/252498) that I hope > will get merged before 3.11 is out. > Thanks for the heads up. The conflict looks minor and I'll rebase if necessary. -- 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-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 02/17] sched_clock: Use seqcount instead of rolling our own
On Fri, 19 Jul 2013, Will Deacon wrote: > Looks good to me. The current scheme would be very fiddly to extend to > 64-bit values on 32-bit architectures without cheap atomic doubleword > accesses. You should have a look at include/linux/cnt32_to_63.h. This could be applied to pure software counters if the low part is atomically increased. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 02/17] sched_clock: Use seqcount instead of rolling our own
On Fri, Jul 19, 2013 at 10:20:19AM -0400, Nicolas Pitre wrote: > On Fri, 19 Jul 2013, Will Deacon wrote: > > > Looks good to me. The current scheme would be very fiddly to extend to > > 64-bit values on 32-bit architectures without cheap atomic doubleword > > accesses. > > You should have a look at include/linux/cnt32_to_63.h. > This could be applied to pure software counters if the low part is > atomically increased. But this can't be used for sched_clock(). That's exactly why I originally had to rewrite that thing in the first place. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 4/4] scsi: ufs: Improve UFS fatal error handling
On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
> 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
Attention'i', typo.
> 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
> ---
> 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);
>
> +
RE: [PATCH V3 2/4] scsi: ufs: Fix hardware race conditions while aborting a command
On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
> 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.
It's interesting.
I wonder when we can meet this situation.
Is it possible if SCSI mid layer should send abort command as soon as the
transfer command is issued?
AFAIK abort command is followed if one command has timed out.
That means command have been already issued and no response?
If you had some problem, could you share?
> - If the device recieves abort command first then it returns success
receives
> 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
> ---
> 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 = SUCC
RE: [PATCH V3 3/4] scsi: ufs: Fix device and host reset methods
On Tue, July 09, 2013, Sujit Reddy Thumma wrote:
> 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
> ---
> 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) {
Lock is no needed for 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,
RE: [PATCH V3 1/4] scsi: ufs: Fix broken task management command implementation
On Tue, July 09, 2013 Sujit Reddy Thumma wrote:
> 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
> ---
> 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);
> - re
RE: [PATCH V3 1/2] scsi: ufs: Add support for host assisted background operations
On Thu, July 18, 2013, Sujit Reddy Thumma wrote:
> >>> > >I'm not sure that BKOPS with runtime-pm associates.
> >>> > >Do you think it's helpful for power management?
> >>> > >How about hibernation scheme for runtime-pm?
> >>> > >I'm testing and I can introduce soon.
> >> >
> >> >Well, I am thinking on following approach when we introduce
> >> >power management.
> >> >
> >> >ufshcd_runtime_suspend() {
> >> > if (bkops_status >= NON_CRITICAL) { /* 0x1 */
> >> > ufshcd_enable_auto_bkops();
> >> > hibernate(); /* only the link and the device
> >> > should be able to cary out bkops */
> >> > } else {
> >> > hibernate(); /* Link and the device for more savings */
> >> > }
> >> >}
> >> >
> >> >Let me know if this is okay.
> > I still consider whether BKOPS is proper behavior with runtime-pm or not.
>
> The BKOPS is something that host allows the card to carry out
> when the host knows it is idle and not expecting back to back requests.
> Runtime PM idle is the only way to know whether the device is
> idle (unless we want to reinvent the wheel to detect the idleness of
> host and trigger bkops). There was a discussion on this in MMC mailing
> list as well, and folks have agreed to move idle time bkops to runtime
> PM (http://thread.gmane.org/gmane.linux.kernel.mmc/19444/)
It looks like different.
eMMC cannot execute BKOPS itself unlike UFS.
That's the way eMMC's host should trigger the BKOPS manually.
>
> > How about this scenario? It seems more simple.
> > If we concern a response latency of transfer requests, BKOPS can be
disabled by default.
> > And then BKOPS can be enabled whenever device requests in some exception.
> > If you have any idea, let me know.
>
> Exceptions are raised only when the device is in critical need for
> bkops. Also the spec. recommends, host should ensure that the device
> doesn't go into such states.
>
> With your suggestion, when we disable bkops, the exception is raised and
> we enable bkops after which there is no way to disable it again?
Yes, it's difficult to find proper time.
Maybe, BKOPS can be disabled when request comes up.
Thanks,
Seungwon Jeon
>
> --
> Regards,
> Sujit
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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
On Thursday, July 18, 2013, Dolev Raviv wrote:
> > Hi,
> >
> > Sorry for the late response. I had a few days off.
> >
> > On Thu, July 11, 2013, Dolev Raviv wrote:
> >> > On Tuesday, July 09, 2013, Sujit Reddy Thumma wrote:
> >> >> From: Dolev Raviv
> >> >> 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
> >> >> Signed-off-by: Sujit Reddy Thumma
> >> >> ---
> >> >> 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;
>
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 v4 04/17] sched_clock: Add support for >32 bit sched_clock
Hi Stephen,
On Thu, Jul 18, 2013 at 04:21:17PM -0700, Stephen Boyd wrote:
> The ARM architected system counter has at least 56 usable bits.
> Add support for counters with more than 32 bits to the generic
> sched_clock implementation so we can increase the time between
> wakeups due to dealing with wrap-around on these devices while
> benefiting from the irqtime accounting and suspend/resume
> handling that the generic sched_clock code already has. On my
> system using 56 bits over 32 bits changes the wraparound time
> from a few minutes to an hour. For faster running counters (GHz
> range) this is even more important because we may not be able to
> execute the timer in time to deal with the wraparound if only 32
> bits are used.
>
> We choose a maxsec value of 3600 seconds because we assume no
> system will go idle for more than an hour. In the future we may
> need to increase this value.
>
> Note: All users should switch over to the 64-bit read function so
> we can remove setup_sched_clock() in favor of sched_clock_register().
>
> Cc: Russell King
> Signed-off-by: Stephen Boyd
> ---
[..]
> @@ -110,14 +123,13 @@ void __init setup_sched_clock(u32 (*read)(void), int
> bits, unsigned long rate)
> if (cd.rate > rate)
> return;
>
> - BUG_ON(bits > 32);
> WARN_ON(!irqs_disabled());
> read_sched_clock = read;
> - sched_clock_mask = (1 << bits) - 1;
> + sched_clock_mask = CLOCKSOURCE_MASK(bits);
Note that this conflicts with my integer overflow fix
(http://article.gmane.org/gmane.linux.ports.arm.kernel/252498) that I hope
will get merged before 3.11 is out.
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}ooO--U--Ooo{=
- bar...@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 02/17] sched_clock: Use seqcount instead of rolling our own
On Fri, Jul 19, 2013 at 12:21:15AM +0100, Stephen Boyd wrote: > We're going to increase the cyc value to 64 bits in the near > future. Doing that is going to break the custom seqcount > implementation in the sched_clock code because 64 bit numbers > aren't guaranteed to be atomic. Replace the cyc_copy with a > seqcount to avoid this problem. > > Cc: Russell King > Signed-off-by: Stephen Boyd > --- > kernel/time/sched_clock.c | 27 --- > 1 file changed, 8 insertions(+), 19 deletions(-) Looks good to me. The current scheme would be very fiddly to extend to 64-bit values on 32-bit architectures without cheap atomic doubleword accesses. Acked-by: Will Deacon Will -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
