Re: [PATCH 0/2] UFS driver fixes for v3.18

[merez]

Hi Chris and Mita,

We will review and test the below patches early next week.

Thanks,
Maya
> On Tue, Nov 18, 2014 at 11:02:44PM +0900, Akinobu Mita wrote:
>> This patch series includes two fixes for UFS driver.
>>
>> The first patch already got reviewed and it was a part of Dolev's
>> various
>> fixes for UFS-PM series
>> (http://marc.info/?l=linux-scsi&m=141405993104015),
>> but they are not merged scsi-queue yet.  So this series also includes
>> it.
>
> Actually that series is missing reviews for this and mother other
> patches.
>
> Can the UFS community please start reviewing each others patches?
> Thanks!
>
> --
> 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
>


-- 
Maya Erez
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/6] various fixes for UFS-PM series

[merez]

Reviewed-by: Maya Erez 

> Contains a couple of bug fixes reported by Akinobu Mita, In adition to
> "static checker" warnings reported by Dan Carpenter.
>
> Akinobu Mita (1):
>   scsi: ufs: fix reference counting of W-LUs
>
> Dolev Raviv (4):
>   scsi: ufs: fix static checker errors in ufshcd_system_suspend
>   scsi: ufs: fix static checker warning in ufshcd_populate_vreg
>   scsi: ufs: fix static checker warning in __ufshcd_setup_clocks
>   scsi: ufs: fix static checker warning in ufshcd_parse_clock_info
>
> Yaniv Gardi (1):
>   scsi: ufs: fix power info after link start-up
>
>  drivers/scsi/ufs/ufshcd-pltfrm.c | 15 +++
>  drivers/scsi/ufs/ufshcd.c| 96
> +++-
>  drivers/scsi/ufs/ufshcd.h|  2 -
>  3 files changed, 61 insertions(+), 52 deletions(-)
>
> --
> 1.8.5.2
> --
> Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
>
> --
> 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
>


-- 
Maya Erez
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 4/4] scsi: ufs: Improve UFS fatal error handling

[merez]

Tested with error injection.

Tested-by: Maya Erez 

> Error handling in UFS driver is broken and resets the host controller
> for fatal errors without re-initialization. Correct the fatal error
> handling sequence according to UFS Host Controller Interface (HCI)
> v1.1 specification.
>
> o Upon determining fatal error condition the host controller may hang
>   forever until a reset is applied, so just retrying the command doesn't
>   work without a reset. So, the reset is applied in the driver context
>   in a separate work and SCSI mid-layer isn't informed until reset is
>   applied.
>
> o Processed requests which are completed without error are reported to
>   SCSI layer as successful and any pending commands that are not started
>   yet or are not cause of the error are re-queued into scsi midlayer
> queue.
>   For the command that caused error, host controller or device is reset
>   and DID_ERROR is returned for command retry after applying reset.
>
> o SCSI is informed about the expected Unit-Attentioni exception from the
>   device for the immediate command after a reset so that the SCSI layer
>   take necessary steps to establish communication with the device.
>
> Signed-off-by: Sujit Reddy Thumma 
> ---
>  drivers/scsi/ufs/ufshcd.c |  349
> +++-
>  drivers/scsi/ufs/ufshcd.h |2 +
>  drivers/scsi/ufs/ufshci.h |   19 ++-
>  3 files changed, 295 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b4c9910..2a3874f 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -80,6 +80,14 @@ enum {
>   UFSHCD_EH_DEVICE_RESET_PENDING = (1 << 1),
>  };
>
> +/* UFSHCD UIC layer error flags */
> +enum {
> + UFSHCD_UIC_DL_PA_INIT_ERROR = (1 << 0), /* Data link layer error */
> + UFSHCD_UIC_NL_ERROR = (1 << 1), /* Network layer error */
> + UFSHCD_UIC_TL_ERROR = (1 << 2), /* Transport Layer error */
> + UFSHCD_UIC_DME_ERROR = (1 << 3), /* DME error */
> +};
> +
>  /* Interrupt configuration options */
>  enum {
>   UFSHCD_INT_DISABLE,
> @@ -108,6 +116,7 @@ enum {
>
>  static void ufshcd_tmc_handler(struct ufs_hba *hba);
>  static void ufshcd_async_scan(void *data, async_cookie_t cookie);
> +static int ufshcd_reset_and_restore(struct ufs_hba *hba);
>
>  /*
>   * ufshcd_wait_for_register - wait for register value to change
> @@ -1605,9 +1614,6 @@ static int ufshcd_make_hba_operational(struct
> ufs_hba *hba)
>   goto out;
>   }
>
> - if (hba->ufshcd_state == UFSHCD_STATE_RESET)
> - scsi_unblock_requests(hba->host);
> -
>  out:
>   return err;
>  }
> @@ -1733,66 +1739,6 @@ static int ufshcd_validate_dev_connection(struct
> ufs_hba *hba)
>  }
>
>  /**
> - * ufshcd_do_reset - reset the host controller
> - * @hba: per adapter instance
> - *
> - * Returns SUCCESS/FAILED
> - */
> -static int ufshcd_do_reset(struct ufs_hba *hba)
> -{
> - struct ufshcd_lrb *lrbp;
> - unsigned long flags;
> - int tag;
> -
> - /* block commands from midlayer */
> - scsi_block_requests(hba->host);
> -
> - spin_lock_irqsave(hba->host->host_lock, flags);
> - hba->ufshcd_state = UFSHCD_STATE_RESET;
> -
> - /* send controller to reset state */
> - ufshcd_hba_stop(hba);
> - spin_unlock_irqrestore(hba->host->host_lock, flags);
> -
> - /* abort outstanding commands */
> - for (tag = 0; tag < hba->nutrs; tag++) {
> - if (test_bit(tag, &hba->outstanding_reqs)) {
> - lrbp = &hba->lrb[tag];
> - if (lrbp->cmd) {
> - scsi_dma_unmap(lrbp->cmd);
> - lrbp->cmd->result = DID_RESET << 16;
> - lrbp->cmd->scsi_done(lrbp->cmd);
> - lrbp->cmd = NULL;
> - clear_bit_unlock(tag, &hba->lrb_in_use);
> - }
> - }
> - }
> -
> - /* complete device management command */
> - if (hba->dev_cmd.complete)
> - complete(hba->dev_cmd.complete);
> -
> - /* clear outstanding request/task bit maps */
> - hba->outstanding_reqs = 0;
> - hba->outstanding_tasks = 0;
> -
> - /* Host controller enable */
> - if (ufshcd_hba_enable(hba)) {
> - dev_err(hba->dev,
> - "Reset: Controller initialization failed\n");
> - return FAILED;
> - }
> -
> - if (ufshcd_link_startup(hba)) {
> - dev_err(hba->dev,
> - "Reset: Link start-up failed\n");
> - return FAILED;
> - }
> -
> - return SUCCESS;
> -}
> -
> -/**
>   * ufshcd_slave_alloc - handle initial SCSI device configurations
>   * @sdev: pointer to SCSI device
>   *
> @@ -1809,6 +1755,9 @@ static int ufshcd_slave_alloc(struct scsi_device
> *sdev)
>   sdev->use_10_for_ms = 1;
>   scsi_set_tag_type(sdev, MSG_SIMPLE_TAG);
>
> + /* allow SCSI layer to re

Re: [PATCH V3 3/4] scsi: ufs: Fix device and host reset methods

[merez]

Tested with error injection.

Tested-by: Maya Erez 

> 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) {
> + case UFSHCD_STATE_OPERATIONAL:
> + break;
> + case UFSHCD_STATE_RESET:
>   err = SCSI_MLQUEUE_HOST_BUSY;
>   goto out;
> + case UFSHCD_STATE_ERROR:
> + set_host_byte(cmd, DID_ERROR);
> + cmd->scsi_done(cmd);
> + goto out;
> + default:
> + dev_WARN_ONCE(hba->dev, 1, "%s: invalid state %d\n",
> + __func__, hba->ufshcd_state);
> + set_host_byte(cmd, DID_BAD_TARGET);
> + cmd->scsi_done(cmd);
> + goto out;
>   }
>
>   /* acquire the tag to make sure device cmds don't use it */
> @@ -1573,8 +1608,6 @@ static int ufshcd_make_hba_operational(struct
> ufs_hba *hba)
>   if (hba->ufshcd_state == UFSHCD_STATE_RESET)
>   scsi_unblock_requests(hba->host);
>
> - hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
> -
>  out:
>   return err;
>  }
> @@ -2273,6 +2306,106 @@ out:
>  }
>
>  /**
> + * ufshcd_utrl_is_rsr_enabled - check if run-stop register is enabled
> + * @hba: per-adapter instance
> + */
> +static bool ufshcd_utrl_is_rsr_enabled(struct ufs_hba *hba)
> +{
> + return ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_LIST_RUN_STOP) & 0x1;
> +}
> +
> +/**
> + * ufshcd_utmrl_is_rsr_enabled - check if run-stop register is enabled
> + * @hba: per-adapter instance
> + */
> +static bool ufshcd_utmrl_is_rsr_enabled(struct ufs_hba *hba)
> +{
> + return ufshcd_readl(hba, REG_UTP_TASK_REQ_LIST_RUN_STOP) & 0x1;
> +}
> +
> +/**
> + * ufshcd_complete_pending_tasks - complete outstanding tasks
> + * @hba: per adapter instance
> + *
> + * Abort in-progress task management commands and wakeup
> + * waiting threads.
> + *
> + * Returns non-zero error value when failed to clear all the commands.
> + */
> +static int ufshcd_complete_pending_tasks(struct ufs_hba *hba)
> +{
> + u32 reg;
> + int err = 0;
> + unsigned long flags;
> +
> + if (!hba->outstanding_tasks)
> + goto out;
> +
> + /* Clear UTMRL only when run-stop is enabled */
> + if (ufshcd_utmrl_is_rsr_enabled(hba))
> + ufshcd_writel(hba, ~hba->outstanding_tasks,
> + REG_UTP_TASK_REQ_LIST_CLEAR);
> +
> + /* poll for max. 1 sec to clear door bell register by h/w */
> + reg = ufshcd_wait_for_register(hba,
> + REG_UTP_TASK_REQ_DOOR_BELL,
> + hba->outstanding_tasks, 0, 1000, 1000);
> + if (reg & hba->outstanding_tasks)
> + err = -ETIMEDOUT;
> +
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + /* complete commands that were 

Re: [PATCH V3 1/4] scsi: ufs: Fix broken task management command implementation

[merez]

Tested-by: Maya Erez 

> 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);
> - return task_result;
> +
> + return 

Re: [PATCH V3 2/4] scsi: ufs: Fix hardware race conditions while aborting a command

[merez]

Tested-by: Maya Erez 

> There is a possible race condition in the hardware when the abort
> command is issued to terminate the ongoing SCSI command as described
> below:
>
> - A bit in the door-bell register is set in the controller for a
>   new SCSI command.
> - In some rare situations, before controller get a chance to issue
>   the command to the device, the software issued an abort command.
> - If the device recieves abort command first then it returns success
>   because the command itself is not present.
> - Now if the controller commits the command to device it will be
>   processed.
> - Software thinks that command is aborted and proceed while still
>   the device is processing it.
> - The software, controller and device may go out of sync because of
>   this race condition.
>
> To avoid this, query task presence in the device before sending abort
> task command so that after the abort operation, the command is guaranteed
> to be non-existent in both controller and the device.
>
> Signed-off-by: Sujit Reddy Thumma 
> ---
>  drivers/scsi/ufs/ufshcd.c |   70
> +++-
>  1 files changed, 55 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index a176421..51ce096 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -2550,6 +2550,12 @@ static int ufshcd_host_reset(struct scsi_cmnd *cmd)
>   * ufshcd_abort - abort a specific command
>   * @cmd: SCSI command pointer
>   *
> + * Abort the pending command in device by sending UFS_ABORT_TASK task
> management
> + * command, and in host controller by clearing the door-bell register.
> There can
> + * be race between controller sending the command to the device while
> abort is
> + * issued. To avoid that, first issue UFS_QUERY_TASK to check if the
> command is
> + * really issued and then try to abort it.
> + *
>   * Returns SUCCESS/FAILED
>   */
>  static int ufshcd_abort(struct scsi_cmnd *cmd)
> @@ -2558,7 +2564,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>   struct ufs_hba *hba;
>   unsigned long flags;
>   unsigned int tag;
> - int err;
> + int err = 0;
> + int poll_cnt;
>   u8 resp;
>   struct ufshcd_lrb *lrbp;
>
> @@ -2566,33 +2573,59 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>   hba = shost_priv(host);
>   tag = cmd->request->tag;
>
> - spin_lock_irqsave(host->host_lock, flags);
> + /* If command is already aborted/completed, return SUCCESS */
> + if (!(test_bit(tag, &hba->outstanding_reqs)))
> + goto out;
>
> - /* check if command is still pending */
> - if (!(test_bit(tag, &hba->outstanding_reqs))) {
> - err = FAILED;
> - spin_unlock_irqrestore(host->host_lock, flags);
> + lrbp = &hba->lrb[tag];
> + for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> + UFS_QUERY_TASK, &resp);
> + if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> + /* cmd pending in the device */
> + break;
> + } else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> + u32 reg;
> +
> + /*
> +  * cmd not pending in the device, check if it is
> +  * in transition.
> +  */
> + reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> + if (reg & (1 << tag)) {
> + /* sleep for max. 2ms to stabilize */
> + usleep_range(1000, 2000);
> + continue;
> + }
> + /* command completed already */
> + goto out;
> + } else {
> + if (!err)
> + err = resp; /* service response error */
> + goto out;
> + }
> + }
> +
> + if (!poll_cnt) {
> + err = -EBUSY;
>   goto out;
>   }
> - spin_unlock_irqrestore(host->host_lock, flags);
>
> - lrbp = &hba->lrb[tag];
>   err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
>   UFS_ABORT_TASK, &resp);
>   if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> - err = FAILED;
> + if (!err)
> + err = resp; /* service response error */
>   goto out;
> - } else {
> - err = SUCCESS;
>   }
>
> + err = ufshcd_clear_cmd(hba, tag);
> + if (err)
> + goto out;
> +
>   scsi_dma_unmap(cmd);
>
>   spin_lock_irqsave(host->host_lock, flags);
> -
> - /* clear the respective UTRLCLR register bit */
> - ufshcd_utrl_clear(hba, tag);
> -
>   __clear_bit(tag, &hba->outstanding_reqs);
>   hba->lrb[tag].cmd = NULL;
> 

Re: [PATCH V3 2/2] scsi: ufs: Add runtime PM support for UFS host controller driver

[merez]

Tested-by: Maya Erez 

> Add runtime PM helpers to suspend/resume UFS controller at runtime.
> Enable runtime PM by default for pci and platform drivers as the
> initialized hardware can suspend if it is not used after bootup.
>
> Signed-off-by: Sujit Reddy Thumma 
> ---
>  drivers/scsi/ufs/ufshcd-pci.c|   65
> ++---
>  drivers/scsi/ufs/ufshcd-pltfrm.c |   51 +-
>  drivers/scsi/ufs/ufshcd.c|8 +
>  3 files changed, 117 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
> index 48be39a..7bd8faa 100644
> --- a/drivers/scsi/ufs/ufshcd-pci.c
> +++ b/drivers/scsi/ufs/ufshcd-pci.c
> @@ -35,6 +35,7 @@
>
>  #include "ufshcd.h"
>  #include 
> +#include 
>
>  #ifdef CONFIG_PM
>  /**
> @@ -44,7 +45,7 @@
>   *
>   * Returns -ENOSYS
>   */
> -static int ufshcd_pci_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int ufshcd_pci_suspend(struct device *dev)
>  {
>   /*
>* TODO:
> @@ -61,7 +62,7 @@ static int ufshcd_pci_suspend(struct pci_dev *pdev,
> pm_message_t state)
>   *
>   * Returns -ENOSYS
>   */
> -static int ufshcd_pci_resume(struct pci_dev *pdev)
> +static int ufshcd_pci_resume(struct device *dev)
>  {
>   /*
>* TODO:
> @@ -71,8 +72,48 @@ static int ufshcd_pci_resume(struct pci_dev *pdev)
>
>   return -ENOSYS;
>  }
> +#else
> +#define ufshcd_pci_suspend   NULL
> +#define ufshcd_pci_resumeNULL
>  #endif /* CONFIG_PM */
>
> +#ifdef CONFIG_PM_RUNTIME
> +static int ufshcd_pci_runtime_suspend(struct device *dev)
> +{
> + struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
> + struct ufs_hba *hba = pci_get_drvdata(pdev);
> +
> + if (!hba)
> + return 0;
> +
> + return ufshcd_runtime_suspend(hba);
> +}
> +static int ufshcd_pci_runtime_resume(struct device *dev)
> +{
> + struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
> + struct ufs_hba *hba = pci_get_drvdata(pdev);
> +
> + if (!hba)
> + return 0;
> +
> + return ufshcd_runtime_resume(hba);
> +}
> +static int ufshcd_pci_runtime_idle(struct device *dev)
> +{
> + struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
> + struct ufs_hba *hba = pci_get_drvdata(pdev);
> +
> + if (!hba)
> + return 0;
> +
> + return ufshcd_runtime_idle(hba);
> +}
> +#else /* !CONFIG_PM_RUNTIME */
> +#define ufshcd_pci_runtime_suspend   NULL
> +#define ufshcd_pci_runtime_resumeNULL
> +#define ufshcd_pci_runtime_idle  NULL
> +#endif /* CONFIG_PM_RUNTIME */
> +
>  /**
>   * ufshcd_pci_shutdown - main function to put the controller in reset
> state
>   * @pdev: pointer to PCI device handle
> @@ -91,6 +132,9 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
>  {
>   struct ufs_hba *hba = pci_get_drvdata(pdev);
>
> + pm_runtime_forbid(&pdev->dev);
> + pm_runtime_get_noresume(&pdev->dev);
> +
>   disable_irq(pdev->irq);
>   ufshcd_remove(hba);
>   pci_release_regions(pdev);
> @@ -168,6 +212,8 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *id)
>   }
>
>   pci_set_drvdata(pdev, hba);
> + pm_runtime_put_noidle(&pdev->dev);
> + pm_runtime_allow(&pdev->dev);
>
>   return 0;
>
> @@ -182,6 +228,14 @@ out_error:
>   return err;
>  }
>
> +static const struct dev_pm_ops ufshcd_pci_pm_ops = {
> + .suspend= ufshcd_pci_suspend,
> + .resume = ufshcd_pci_resume,
> + .runtime_suspend = ufshcd_pci_runtime_suspend,
> + .runtime_resume  = ufshcd_pci_runtime_resume,
> + .runtime_idle= ufshcd_pci_runtime_idle,
> +};
> +
>  static DEFINE_PCI_DEVICE_TABLE(ufshcd_pci_tbl) = {
>   { PCI_VENDOR_ID_SAMSUNG, 0xC00C, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
>   { } /* terminate list */
> @@ -195,10 +249,9 @@ static struct pci_driver ufshcd_pci_driver = {
>   .probe = ufshcd_pci_probe,
>   .remove = ufshcd_pci_remove,
>   .shutdown = ufshcd_pci_shutdown,
> -#ifdef CONFIG_PM
> - .suspend = ufshcd_pci_suspend,
> - .resume = ufshcd_pci_resume,
> -#endif
> + .driver = {
> + .pm = &ufshcd_pci_pm_ops
> + },
>  };
>
>  module_pci_driver(ufshcd_pci_driver);
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c
> b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index c42db40..b1f2605 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -34,6 +34,7 @@
>   */
>
>  #include 
> +#include 
>
>  #include "ufshcd.h"
>
> @@ -87,6 +88,43 @@ static int ufshcd_pltfrm_resume(struct device *dev)
>  #define ufshcd_pltfrm_resume NULL
>  #endif
>
> +#ifdef CONFIG_PM_RUNTIME
> +static int ufshcd_pltfrm_runtime_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct ufs_hba *hba =  platform_get_drvdata(pdev);
> +
> + if (!hba)
> + return 0;
> +
> + return ufshcd_runtime_suspend(hba);
> +}
> +static 

Re: [PATCH V3 1/2] scsi: ufs: Add support for host assisted background operations

[merez]

Tested-by: Maya Erez 

> Background operations in the UFS device can be disabled by
> the host to reduce the response latency of transfer requests.
> Add support for enabling/disabling the background operations
> during runtime suspend/resume of the device.
>
> If the device is in critical need of BKOPS it will raise an
> URGENT_BKOPS exception which should be handled by the host to
> make sure the device performs as expected.
>
> During bootup, the BKOPS is enabled in the device by default.
> The disable of BKOPS is supported only when the driver supports
> runtime suspend/resume operations as the runtime PM framework
> provides a way to determine the device idleness and hence BKOPS
> can be managed effectively. During runtime resume the BKOPS is
> disabled to reduce latency and during runtime suspend the BKOPS
> is enabled to allow device to carry out idle time BKOPS.
>
> In some cases where the BKOPS is disabled during runtime resume
> and due to continuous data transfers the runtime suspend is not
> triggered, the BKOPS is enabled when the device raises a level-2
> exception (outstanding operations - performance impact).
>
> Signed-off-by: Sujit Reddy Thumma 
> ---
>  drivers/scsi/ufs/ufs.h|   25 -
>  drivers/scsi/ufs/ufshcd.c |  338
> +
>  drivers/scsi/ufs/ufshcd.h |   10 ++
>  3 files changed, 372 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index db5bde4..549a652 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -107,7 +107,29 @@ enum {
>
>  /* Flag idn for Query Requests*/
>  enum flag_idn {
> - QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
> + QUERY_FLAG_IDN_FDEVICEINIT  = 0x01,
> + QUERY_FLAG_IDN_BKOPS_EN = 0x04,
> +};
> +
> +/* Attribute idn for Query requests */
> +enum attr_idn {
> + QUERY_ATTR_IDN_BKOPS_STATUS = 0x05,
> + QUERY_ATTR_IDN_EE_CONTROL   = 0x0D,
> + QUERY_ATTR_IDN_EE_STATUS= 0x0E,
> +};
> +
> +/* Exception event mask values */
> +enum {
> + MASK_EE_STATUS  = 0x,
> + MASK_EE_URGENT_BKOPS= (1 << 2),
> +};
> +
> +/* Background operation status */
> +enum {
> + BKOPS_STATUS_NO_OP   = 0x0,
> + BKOPS_STATUS_NON_CRITICAL= 0x1,
> + BKOPS_STATUS_PERF_IMPACT = 0x2,
> + BKOPS_STATUS_CRITICAL= 0x3,
>  };
>
>  /* UTP QUERY Transaction Specific Fields OpCode */
> @@ -156,6 +178,7 @@ enum {
>   MASK_TASK_RESPONSE  = 0xFF00,
>   MASK_RSP_UPIU_RESULT= 0x,
>   MASK_QUERY_DATA_SEG_LEN = 0x,
> + MASK_RSP_EXCEPTION_EVENT = 0x1,
>  };
>
>  /* Task management service response */
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 96ccb28..a25de66 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -268,6 +268,21 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp
> *ucd_rsp_ptr)
>  }
>
>  /**
> + * ufshcd_is_exception_event - Check if the device raised an exception
> event
> + * @ucd_rsp_ptr: pointer to response UPIU
> + *
> + * The function checks if the device raised an exception event indicated
> in
> + * the Device Information field of response UPIU.
> + *
> + * Returns true if exception is raised, false otherwise.
> + */
> +static inline bool ufshcd_is_exception_event(struct utp_upiu_rsp
> *ucd_rsp_ptr)
> +{
> + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
> + MASK_RSP_EXCEPTION_EVENT ? true : false;
> +}
> +
> +/**
>   * ufshcd_config_int_aggr - Configure interrupt aggregation values.
>   *   Currently there is no use case where we want to configure
>   *   interrupt aggregation dynamically. So to configure interrupt
> @@ -1174,6 +1189,86 @@ out_no_mem:
>  }
>
>  /**
> + * ufshcd_query_attr - Helper function for composing attribute requests
> + * hba: per-adapter instance
> + * opcode: attribute opcode
> + * idn: attribute idn to access
> + * index: index field
> + * selector: selector field
> + * attr_val: the attribute value after the query request completes
> + *
> + * Returns 0 for success, non-zero in case of failure
> +*/
> +int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
> + enum attr_idn idn, u8 index, u8 selector, u32 *attr_val)
> +{
> + struct ufs_query_req *query;
> + struct ufs_query_res *response;
> + int err = -ENOMEM;
> +
> + if (!attr_val) {
> + dev_err(hba->dev, "%s: attribute value required for write 
> request\n",
> + __func__);
> + err = -EINVAL;
> + goto out;
> + }
> +
> + query = kzalloc(sizeof(struct ufs_query_req), GFP_KERNEL);
> + if (!query) {
> + dev_err(hba->dev,
> + "%s: Failed allocating ufs_query_req instance\n",
> + __func__);
> + goto out;
> + }
> +
> + response = kzalloc(sizeof(struct ufs_

Re: [PATCH V3 1/2] scsi: ufs: Add support for sending NOP OUT UPIU

[merez]

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

Re: [PATCH V3 2/2] scsi: ufs: Set fDeviceInit flag to initiate device initialization

[merez]

Tested-by: Maya Erez 

> 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;
> + u32 value;
> + u32 reserved[2];
> +};
> +
> +/**
>   * struct utp_upiu_req - general upiu request structure
>   * @header:UPIU header structure DW-0 to DW-2
>   * @sc: fields structure for scsi command DW-3 to DW-7
> + * @qr: fields structure for query request DW-3 to DW-7
>   */
>  struct utp_upiu_req {
>   struct utp_upiu_header header;
> - struct utp_upiu_cmd sc;
> + union {
> + struct utp_upiu_cmd sc;
> + struct utp_upiu_query qr;
> + };
>  };
>
>  /**
> @@ -187,10 +243,14 @@ struct utp_cmd_rsp {
>   * struct utp_upiu_rsp - general upiu response structure
>   * @header: UPIU header structure DW-0 to DW-2
>   * @sr: fields structure for scsi command DW-3 to DW-12
> + * @qr: fields structure for query request DW-3 to DW-7
>   */
>  struct utp_upiu_rsp {
>   struct utp_upiu_header header;
> - struct utp_cmd_rsp sr

Re: [PATCH v4 1/6] scsi: ufs: wrap the i/o access operations

[merez]

Hi,

I tested the new set of patches (v4 1-6) and it works.

Thanks,
Maya
> Simplify operations with hiding mmio_base.
>
> Signed-off-by: Seungwon Jeon 
> Tested-by: Maya Erez 
> ---
>  drivers/scsi/ufs/ufshcd.c |  105
> +++--
>  drivers/scsi/ufs/ufshcd.h |7 +++-
>  2 files changed, 50 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index f244812..cf7c8e4 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -71,7 +71,7 @@ enum {
>   */
>  static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba)
>  {
> - return readl(hba->mmio_base + REG_UFS_VERSION);
> + return ufshcd_readl(hba, REG_UFS_VERSION);
>  }
>
>  /**
> @@ -130,8 +130,7 @@ static inline int ufshcd_get_tm_free_slot(struct
> ufs_hba *hba)
>   */
>  static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos)
>  {
> - writel(~(1 << pos),
> - (hba->mmio_base + REG_UTP_TRANSFER_REQ_LIST_CLEAR));
> + ufshcd_writel(hba, ~(1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR);
>  }
>
>  /**
> @@ -165,7 +164,7 @@ static inline int ufshcd_get_lists_status(u32 reg)
>   */
>  static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
>  {
> - return readl(hba->mmio_base + REG_UIC_COMMAND_ARG_2) &
> + return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_2) &
>  MASK_UIC_COMMAND_RESULT;
>  }
>
> @@ -238,18 +237,15 @@ ufshcd_config_int_aggr(struct ufs_hba *hba, int
> option)
>  {
>   switch (option) {
>   case INT_AGGR_RESET:
> - writel((INT_AGGR_ENABLE |
> - INT_AGGR_COUNTER_AND_TIMER_RESET),
> - (hba->mmio_base +
> -  REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL));
> + ufshcd_writel(hba, INT_AGGR_ENABLE |
> +   INT_AGGR_COUNTER_AND_TIMER_RESET,
> +   REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
>   break;
>   case INT_AGGR_CONFIG:
> - writel((INT_AGGR_ENABLE |
> - INT_AGGR_PARAM_WRITE |
> - INT_AGGR_COUNTER_THRESHOLD_VALUE |
> - INT_AGGR_TIMEOUT_VALUE),
> - (hba->mmio_base +
> -  REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL));
> + ufshcd_writel(hba, INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
> +   INT_AGGR_COUNTER_THRESHOLD_VALUE |
> +   INT_AGGR_TIMEOUT_VALUE,
> +   REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
>   break;
>   }
>  }
> @@ -262,12 +258,10 @@ ufshcd_config_int_aggr(struct ufs_hba *hba, int
> option)
>   */
>  static void ufshcd_enable_run_stop_reg(struct ufs_hba *hba)
>  {
> - writel(UTP_TASK_REQ_LIST_RUN_STOP_BIT,
> -(hba->mmio_base +
> - REG_UTP_TASK_REQ_LIST_RUN_STOP));
> - writel(UTP_TRANSFER_REQ_LIST_RUN_STOP_BIT,
> -(hba->mmio_base +
> - REG_UTP_TRANSFER_REQ_LIST_RUN_STOP));
> + ufshcd_writel(hba, UTP_TASK_REQ_LIST_RUN_STOP_BIT,
> +   REG_UTP_TASK_REQ_LIST_RUN_STOP);
> + ufshcd_writel(hba, UTP_TRANSFER_REQ_LIST_RUN_STOP_BIT,
> +   REG_UTP_TRANSFER_REQ_LIST_RUN_STOP);
>  }
>
>  /**
> @@ -276,7 +270,7 @@ static void ufshcd_enable_run_stop_reg(struct ufs_hba
> *hba)
>   */
>  static inline void ufshcd_hba_start(struct ufs_hba *hba)
>  {
> - writel(CONTROLLER_ENABLE , (hba->mmio_base + REG_CONTROLLER_ENABLE));
> + ufshcd_writel(hba, CONTROLLER_ENABLE, REG_CONTROLLER_ENABLE);
>  }
>
>  /**
> @@ -287,7 +281,7 @@ static inline void ufshcd_hba_start(struct ufs_hba
> *hba)
>   */
>  static inline int ufshcd_is_hba_active(struct ufs_hba *hba)
>  {
> - return (readl(hba->mmio_base + REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
> + return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
>  }
>
>  /**
> @@ -299,8 +293,7 @@ static inline
>  void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
>  {
>   __set_bit(task_tag, &hba->outstanding_reqs);
> - writel((1 << task_tag),
> -(hba->mmio_base + REG_UTP_TRANSFER_REQ_DOOR_BELL));
> + ufshcd_writel(hba, 1 << task_tag, REG_UTP_TRANSFER_REQ_DOOR_BELL);
>  }
>
>  /**
> @@ -381,8 +374,7 @@ void ufshcd_copy_query_response(struct ufs_hba *hba,
> struct ufshcd_lrb *lrbp)
>   */
>  static inline void ufshcd_hba_capabilities(struct ufs_hba *hba)
>  {
> - hba->capabilities =
> - readl(hba->mmio_base + REG_CONTROLLER_CAPABILITIES);
> + hba->capabilities = ufshcd_readl(hba, REG_CONTROLLER_CAPABILITIES);
>
>   /* nutrs and nutmrs are 0 based values */
>   hba->nutrs = (hba->capabilities & MASK_TRANSFER_REQUESTS_SLOTS) + 1;
> @@ -399,16 +391,13 @@ static inline void
>  ufshcd_send_uic_command(struct ufs_hba *hba, struct uic_command
> *uic_cmnd)
>  {
>   /* Write Args */
> - writel(uic_cmnd->argument1,
> -   (hba->mmio_base +

Re: [PATCH 1/1] scsi: ufs: Set fDeviceInit flag to initiate device initialization

[merez]

Tested-by: Maya Erez 

> 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|   10 -
>  drivers/scsi/ufs/ufshcd.c |  102
> +
>  drivers/scsi/ufs/ufshcd.h |7 +++
>  3 files changed, 118 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index 086ff03..742363d 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -107,8 +107,13 @@ enum {
>   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 {
> +enum query_opcode {
>   UPIU_QUERY_OPCODE_NOP   = 0x0,
>   UPIU_QUERY_OPCODE_READ_DESC = 0x1,
>   UPIU_QUERY_OPCODE_WRITE_DESC= 0x2,
> @@ -208,6 +213,9 @@ struct utp_upiu_query {
>   u32 reserved[2];
>  };
>
> +/* Expose the flag value from utp_upiu_query.value */
> +#define MASK_QUERY_UPIU_FLAG_LOC 0xFF
> +
>  /**
>   * struct utp_upiu_req - general upiu request structure
>   * @header:UPIU header structure DW-0 to DW-2
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 4090f6c..21aa0b4 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -52,6 +52,15 @@
>  /* Reserved tag for internal commands */
>  #define INTERNAL_CMD_TAG   0
>
> +/* Query request standard retries */
> +#define QUERY_REQ_RETRIES 10
> +/* Query request standart timeout in seconds */
> +#define QUERY_REQ_TIMEOUT 5
> +/* Send Query Requst with default parameters */
> +#define send_query_request(ARG1, ARG2, ARG3, ARG4)   \
> + ufshcd_query_request(ARG1, ARG2, ARG3, ARG4,\
> + QUERY_REQ_TIMEOUT, QUERY_REQ_RETRIES)
> +
>  enum {
>   UFSHCD_MAX_CHANNEL  = 0,
>   UFSHCD_MAX_ID   = 1,
> @@ -952,6 +961,57 @@ int ufshcd_query_request(struct ufs_hba *hba,
>  }
>
>  /**
> + * ufshcd_query_flag() - Helper function for composing flag query
> requests
> + * hba: per-adapter instance
> + * query_opcode: flag query to perform
> + * idn: flag idn to access
> + * flag_res: the flag value after the query request completes
> + *
> + * Returns 0 for success, non-zero in case of failure
> + */
> +int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
> + enum flag_idn idn, bool *flag_res)
> +{
> + struct ufs_query_req query = {0};
> + struct ufs_query_res response = {0};
> + int err;
> +
> + switch (opcode) {
> + case UPIU_QUERY_OPCODE_SET_FLAG:
> + case UPIU_QUERY_OPCODE_CLEAR_FLAG:
> + case UPIU_QUERY_OPCODE_TOGGLE_FLAG:
> + query.query_func = UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST;
> + break;
> + case UPIU_QUERY_OPCODE_READ_FLAG:
> + query.query_func = UPIU_QUERY_FUNC_STANDARD_READ_REQUEST;
> + break;
> + default:
> + dev_err(hba->dev,
> + "%s: Expected query flag opcode but got = %d\n",
> + __func__, opcode);
> + err = -EINVAL;
> + goto out;
> + }
> + query.upiu_req.opcode = opcode;
> + query.upiu_req.idn = idn;
> +
> + /* Send query request */
> + err = send_query_request(hba, &query, NULL, &response);
> +
> + if (err) {
> + dev_err(hba->dev,
> + "%s: Sending flag query for idn %d failed, err = %d\n",
> + __func__, idn, err);
> + goto out;
> + }
> +
> + *flag_res = (response.upiu_res.value & MASK_QUERY_UPIU_FLAG_LOC) & 0x1;
> +
> +out:
> + return err;
> +}
> +
> +/**
>   * ufshcd_memory_alloc - allocate memory for host memory space data
> structures
>   * @hba: per adapter instance
>   *
> @@ -1384,6 +1444,44 @@ out:
>  }
>
>  /**
> + * ufshcd_validate_device_init() - checks device readines
> + * hba: per-adapter instance
> + *
> + * Set fDeviceInit flag, than, query the flag untill the device clears
> the
> + * flag.
> + */
> +static int ufshcd_validate_device_init(struct ufs_hba *hba)
> +{
> + int i, err;
> + bool flag_res = 0;
> +
> + err = ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_SET_FLAG,
> + QUERY_FLAG_IDN_FDEVICEINIT, &flag_res);
> + if (err) {
> + dev_err(hba->dev,
> + "%s setting fDeviceInit flag failed with error %d\n",
> + __func__, err);
> + goto out;
> + }
> +
> + for (i = 0; i < QUERY_REQ_RETRIES && !err && flag_res; i++) {
> + err

RE: [PATCH v3 4/6] scsi: ufs: rework link start-up process

[merez]

> On Monday, May 06, 2013, Sujit Reddy Thumma wrote:
>> On 5/6/2013 11:07 AM, Seungwon Jeon wrote:
>> > Link start-up requires long time with multiphase handshakes
>> > between UFS host and device. This affects driver's probe time.
>> > This patch let link start-up run asynchronously. Link start-up
>> > will be executed at the end of prove separately.
>> > Along with this change, the following is worked.
>> >
>> > Defined completion time of uic command to avoid a permanent wait.
>> We have seen a kernel crash when the timeout happens in our internal
>> testing. This is basically because the the UIC_CMD_TIMEOUT of 500msec is
>> less for our platform. We see UIC_COMMAND_COMPL interrupt after this
>> timeout and since uic_cmd is freed we see a kernel crash.
>>
>> [6.026094] ufshcd :01:00.0: link startup: error code -110
>> [6.030886] ufshcd :01:00.0: link startup failed -110
>> ...
>>
>> [6.487288] BUG: spinlock bad magic on CPU#0, swapper/0/0
>> [6.491683] Unable to handle kernel paging request at virtual address
>> 612b
>> ...
>> [7.625270] [] (spin_dump+0x4c/0x88) from []
>> (do_raw_spin_lock+0x20/0x154)
>> [7.633877] [] (do_raw_spin_lock+0x20/0x154) from
>> [] (_raw_spin_lock_irqsave+0x28/0x30)
>> [7.643583] [] (_raw_spin_lock_irqsave+0x28/0x30) from
>> [] (complete+0x1c/0x58)
>> [7.652525] [] (complete+0x1c/0x58) from []
>> (ufshcd_intr+0x1a8/0x498)
>> [7.660705] [] (ufshcd_intr+0x1a8/0x498) from []
>> (handle_irq_event_percpu+0xb0/0x290)
>> [7.670227] [] (handle_irq_event_percpu+0xb0/0x290) from
>> [] (handle_irq_event+0x3c/0x5c)
>> [7.680054] [] (handle_irq_event+0x3c/0x5c) from
>> [] (handle_fasteoi_irq+0xdc/0x148)
>> [7.689424] [] (handle_fasteoi_irq+0xdc/0x148) from
>> [] (generic_handle_irq+0x30/0x44)
>> [7.698977] [] (generic_handle_irq+0x30/0x44) from
>> [] (handle_IRQ+0x7c/0xc0)
>> [7.707736] [] (handle_IRQ+0x7c/0xc0) from []
>> (gic_handle_irq+0x94/0x110)
>> [7.716252] [] (gic_handle_irq+0x94/0x110) from
>> [] (__irq_svc+0x40/0x70)
>>
>>
>> Following would fix this. Please check. Also, we need to increase the
>> timeout to say 5 secs to avoid such races.
> Thank you for test.
> I have seen the completion of link startup within current timeout ever.
> It should be fixed for that case.
> By the way, I feel 5 seconds looks like a little too long.
> Currently, is your platform situation with UFS in normal stage?
> The UNIPRO spec. mentions the 100 ms as timeout internally for link
> startup.
> Anyway, do you mean that you want to change timeout value to 5 seconds?
> I think it has a strong dependency with your platform.
> 1 sec seems proper though the worst case is considered.
>
> Thanks,
> Seungwon Jeon

I agree, you can keep a timeout of 500ms. I got the timeout due to a
unipro issue and not due to low timeout value.

Thanks,
Maya
>
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index ecbae40..ae22a8e 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -485,6 +485,7 @@ static int
>>  ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct uic_command
>> *uic_cmd)
>>  {
>>  int ret;
>> +unsigned long flags;
>>
>>  if (wait_for_completion_timeout(&uic_cmd->done,
>>  msecs_to_jiffies(UIC_CMD_TIMEOUT)))
>> @@ -492,6 +493,10 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct
>> uic_command *uic_cmd)
>>  else
>>  ret = -ETIMEDOUT;
>>
>> +spin_lock_irqsave(hba->host->host_lock, flags);
>> +hba->active_uic_cmd = NULL;
>> +spin_unlock_irqrestore(hba->host->host_lock, flags);
>> +
>>  return ret;
>>  }
>>
>> @@ -1842,7 +1847,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
>> struct ufshcd_lrb *lrbp)
>>   */
>>  static void ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
>>  {
>> -if (intr_status & UIC_COMMAND_COMPL) {
>> +if (hba->active_uic_cmd && (intr_status & UIC_COMMAND_COMPL)) {
>>  hba->active_uic_cmd->argument2 |=
>>  ufshcd_get_uic_cmd_result(hba);
>>  hba->active_uic_cmd->argument3 =
>>
>>
>> > Added mutex to guarantee of uic command at a time.
>> > Adapted some sequence of controller initialization after link statup
>> > according to HCI standard.
>> >
>> > Signed-off-by: Seungwon Jeon 
>> > Signed-off-by: Sujit Reddy Thumma 
>> > ---
>> >   drivers/scsi/ufs/ufshcd.c |  287
>> ++---
>> >   drivers/scsi/ufs/ufshcd.h |   10 +-
>> >   2 files changed, 200 insertions(+), 97 deletions(-)
>> >
>> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> > index e04c74e..118f104 100644
>> > --- a/drivers/scsi/ufs/ufshcd.c
>> > +++ b/drivers/scsi/ufs/ufshcd.c
>> > @@ -33,11 +33,15 @@
>> >* this program.
>> >*/
>> >
>> > +#include 
>> > +
>> >   #include "ufshcd.h"
>> >
>> >   #define UFSHCD_ENABLE_INTRS  (UTP_TRANSFER_REQ_COMPL |\
>> >

Re: [PATCH v3 3/6] scsi: ufs: fix interrupt status clears

[merez]

Tested-by: Maya Erez 

> There is no need to check the version to clear
> the interrupt status. And the order is changed
> prior to actual handling.
>
> Signed-off-by: Seungwon Jeon 
> ---
>  drivers/scsi/ufs/ufshcd.c |5 +
>  1 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index feaf221..e04c74e 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1582,11 +1582,8 @@ static irqreturn_t ufshcd_intr(int irq, void
> *__hba)
>   intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
>
>   if (intr_status) {
> + ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
>   ufshcd_sl_intr(hba, intr_status);
> -
> - /* If UFSHCI 1.0 then clear interrupt status register */
> - if (hba->ufs_version == UFSHCI_VERSION_10)
> - ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
>   retval = IRQ_HANDLED;
>   }
>   spin_unlock(hba->host->host_lock);
> --
> 1.7.0.4
>
>
> --
> 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
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/1] scsi: ufs: Add support for sending NOP OUT UPIU

[merez]

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.
>
> Signed-off-by: Sujit Reddy Thumma 
>
> ---
> v2:
>- fixed INTERNAL_CMD_TAG check in readl_poll_timeout
>- minor cleanup from v1
>- rebased on Seungwon Jeon's UFS V3 patchset
> ---
>  drivers/scsi/ufs/ufshcd.c |  161
> ++---
>  drivers/scsi/ufs/ufshcd.h |4 +
>  2 files changed, 156 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 3607ffb..ecbae40 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -34,6 +34,7 @@
>   */
>
>  #include 
> +#include 
>
>  #include "ufshcd.h"
>
> @@ -44,6 +45,13 @@
>  /* 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 */
> +/* Reserved tag for internal commands */
> +#define INTERNAL_CMD_TAG   0
> +
>  enum {
>   UFSHCD_MAX_CHANNEL  = 0,
>   UFSHCD_MAX_ID   = 1,
> @@ -630,7 +638,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;
>
> @@ -647,6 +655,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);
> @@ -735,6 +745,18 @@ static 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
> @@ -749,11 +771,13 @@ 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->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
> + else if (lrbp->cmd && ufshcd_is_query_req(lrbp))
>   ufshcd_prepare_utp_query_req_upiu(hba, lrbp,
>   upiu_flags);
> + else if (!lrbp->cmd)
> + ufshcd_prepare_utp_nop_upiu(lrbp);
>   break;
>   case UTP_CMD_TYPE_UFS:
>   /* For UFS native command implementation */
> @@ -802,6 +826,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;
> @@ -1261,6 +1286,101 @@ int ufshcd_dme_endpt_reset(struct ufs_hba *hba)
>  }
>  EXPORT_SYMBOL_GPL(ufshcd_dme_endpt_reset);
>
> +static 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);
> +}
> +
> +static int ufshcd_wait_for_nop_cmd(struct ufs_hba *hba, struct ufshcd_lrb
> *lrbp)
> +{
> + int err = 0;
> + unsigned long timeout;
> + unsigned long flags;
> +
> + timeout = wait_for_completion_timeout(
> +  

Re: [PATCH 3/3] scsi: ufs: add support for query requests

[merez]

Tested-by: Maya Erez 

>>> diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
>>> index 35faf24..82417d6 100644
>>> --- a/drivers/scsi/ufs/Kconfig
>>> +++ b/drivers/scsi/ufs/Kconfig
>>> @@ -35,6 +35,7 @@
>>>  config SCSI_UFSHCD
>>>   tristate "Universal Flash Storage Controller Driver Core"
>>>   depends on SCSI
>>> + default y
>>>   ---help---
>>>   This selects the support for UFS devices in Linux, say Y and make
>>> sure that you know the name of your UFS host adapter (the card
>>
>> I don't think you want to force this on in every configuration since
>> it's only appropriate to non server/desktop.  I stripped this hunk.
>>
>
> Hi James,
>
> Please apply the patch without the particular change or you can apply
> the latest patch sent by Dolev -
> http://marc.info/?l=linux-scsi&m=136774922024179&w=1.
>
> Thanks,
> Santosh
> --
> 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
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 7/7] scsi: ufs: add dme control primitives

[merez]

Tested-by: Maya Erez 

> Implements to support the following operations.
> Currently, this patch doesn't introduce DME_ENABLE and DME_RESET
> because host controller's HCE enable contains these roles.
>
> DME_POWERON{OFF}, DME_HIBERNATE_ENTER{EXIT}, DME_ENDPOINTRESET.
>
> Signed-off-by: Seungwon Jeon 
> ---
>  drivers/scsi/ufs/ufshcd.c |  133
> ++---
>  drivers/scsi/ufs/ufshcd.h |   26 +
>  drivers/scsi/ufs/ufshci.h |   15 +
>  3 files changed, 166 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 956041c..54cd61a 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -39,6 +39,7 @@
>
>  #define UFSHCD_ENABLE_INTRS  (UTP_TRANSFER_REQ_COMPL |\
>UTP_TASK_REQ_COMPL |\
> +  UFSHCD_HIBERNATE_MASK |\
>UFSHCD_ERROR_MASK)
>  /* UIC command timeout, unit: ms */
>  #define UIC_CMD_TIMEOUT  500
> @@ -203,6 +204,18 @@ static inline u32 ufshcd_get_dme_attr_val(struct
> ufs_hba *hba)
>  }
>
>  /**
> + * ufshcd_get_upmcrs - Get the power mode change request status
> + * @hba: Pointer to adapter instance
> + *
> + * This function gets the UPMCRS field of HCS register
> + * Returns value of UPMCRS field
> + */
> +static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
> +{
> + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
> +}
> +
> +/**
>   * ufshcd_free_hba_memory - Free allocated memory for LRB, request
>   *   and task lists
>   * @hba: Pointer to adapter instance
> @@ -1161,6 +1174,104 @@ int ufshcd_dme_xxx_get(struct ufs_hba *hba, u32
> attr_sel,
>  EXPORT_SYMBOL_GPL(ufshcd_dme_xxx_get);
>
>  /**
> + * ufshcd_dme_power_xxx - UIC command for DME_POWERON, DME_POWEROFF
> + * @hba: per adapter instance
> + * @on: indicate wherter power_on or power_off
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +int ufshcd_dme_power_xxx(struct ufs_hba *hba, u8 on)
> +{
> + struct uic_command uic_cmd = {0};
> + static const char *const action[] = {
> + "dme-power-off",
> + "dme-power-on"
> + };
> + const char *power = action[!!on];
> + int ret;
> +
> + uic_cmd.command = on ?
> + UIC_CMD_DME_POWERON : UIC_CMD_DME_POWEROFF;
> +
> + ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
> + if (ret)
> + dev_err(hba->dev, "%s: error code %d\n", power, ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_dme_power_xxx);
> +
> +/**
> + * ufshcd_dme_hibern8_xxx - UIC command for DME_HIBERNATE_ENTER,
> + *   DME_HIBERNATE_EXIT
> + * @hba: per adapter instance
> + * @enter: indicate wherter hibernation enter or exit
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +int ufshcd_dme_hibern8_xxx(struct ufs_hba *hba, u8 enter)
> +{
> + struct uic_command uic_cmd = {0};
> + static const char *const action[] = {
> + "dme-hibernate-exit",
> + "dme-hibernate-enter"
> + };
> + const char *hibern8 = action[!!enter];
> + u8 status;
> + int ret;
> +
> + uic_cmd.command = enter ?
> + UIC_CMD_DME_HIBER_ENTER : UIC_CMD_DME_HIBER_EXIT;
> +
> + mutex_lock(&hba->uic_cmd_mutex);
> + ret = __ufshcd_send_uic_cmd(hba, &uic_cmd);
> + if (ret) {
> + dev_err(hba->dev, "%s: error code %d\n", hibern8, ret);
> + goto out;
> + }
> +
> + init_completion(&hba->hibern8_done);
> +
> + if (wait_for_completion_timeout(&hba->hibern8_done,
> + msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
> + status = ufshcd_get_upmcrs(hba);
> + if (status != PWR_LOCAL) {
> + dev_err(hba->dev, "%s: failed, host upmcrs:%x\n",
> + hibern8, status);
> + ret = status;
> + }
> + } else {
> + dev_err(hba->dev, "%s: timeout\n", hibern8);
> + ret = -ETIMEDOUT;
> + }
> +out:
> + mutex_unlock(&hba->uic_cmd_mutex);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_dme_hibern8_xxx);
> +
> +/**
> + * ufshcd_dme_endpt_reset - UIC command for DME_ENDPOINTRESET
> + * @hba: per adapter instance
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +int ufshcd_dme_endpt_reset(struct ufs_hba *hba)
> +{
> + struct uic_command uic_cmd = {0};
> + int ret;
> +
> + uic_cmd.command = UIC_CMD_DME_END_PT_RST;
> +
> + ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
> + if (ret)
> + dev_err(hba->dev, "endpoint reset: error code %d\n", ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_dme_endpt_reset);
> +
> +/**
>   * ufshcd_make_hba_operational - Make UFS controller operational
>   * @hba: per adapter instance
>   *
> @@ -1617,14 +1728,20 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
> s

Re: [PATCH v2 6/7] scsi: ufs: add dme configuration primitives

[merez]

Tested-by: Maya Erez 

> Implements to support Get and Set operation of the DME.
> And along with these operations, IDs of Attribute are added.
> It is used to configure the behavior of the UNIPRO.
>
> Signed-off-by: Seungwon Jeon 
> ---
>  drivers/scsi/ufs/ufshcd.c |   84 +
>  drivers/scsi/ufs/ufshcd.h |   29 ++
>  drivers/scsi/ufs/ufshci.h |4 ++
>  drivers/scsi/ufs/unipro.h |  129
> +
>  4 files changed, 246 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/scsi/ufs/unipro.h
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 9c94d42..956041c 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -191,6 +191,18 @@ static inline int ufshcd_get_uic_cmd_result(struct
> ufs_hba *hba)
>  }
>
>  /**
> + * ufshcd_get_dme_attr_val - Get the value of attribute returned by UIC
> command
> + * @hba: Pointer to adapter instance
> + *
> + * This function gets UIC command argument3
> + * Returns 0 on success, non zero value on error
> + */
> +static inline u32 ufshcd_get_dme_attr_val(struct ufs_hba *hba)
> +{
> + return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_3);
> +}
> +
> +/**
>   * ufshcd_free_hba_memory - Free allocated memory for LRB, request
>   *   and task lists
>   * @hba: Pointer to adapter instance
> @@ -1079,6 +1091,76 @@ static int ufshcd_dme_link_startup(struct ufs_hba
> *hba)
>  }
>
>  /**
> + * ufshcd_dme_xxx_set - UIC command for DME_SET, DME_PEER_SET
> + * @hba: per adapter instance
> + * @attr_sel: uic command argument1
> + * @attr_set: attribute set type as uic command argument2
> + * @mib_val: setting value as uic command argument3
> + * @peer: indicate wherter peer or non-peer
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +int ufshcd_dme_xxx_set(struct ufs_hba *hba, u32 attr_sel,
> +u8 attr_set, u32 mib_val, u8 peer)
> +{
> + struct uic_command uic_cmd = {0};
> + static const char *const action[] = {
> + "dme-set",
> + "dme-peer-set"
> + };
> + const char *set = action[!!peer];
> + int ret;
> +
> + uic_cmd.command = peer ?
> + UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET;
> + uic_cmd.argument1 = attr_sel;
> + uic_cmd.argument2 = UIC_ARG_ATTR_SET(attr_set);
> + uic_cmd.argument3 = mib_val;
> +
> + ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
> +
> + dev_dbg(hba->dev, "%s: error code %d\n", set, ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_dme_xxx_set);
> +
> +/**
> + * ufshcd_dme_xxx_get - UIC command for DME_GET, DME_PEER_GET
> + * @hba: per adapter instance
> + * @attr_sel: uic command argument1
> + * @mib_val: the value of the attribute as returned by the UIC command
> + * @peer: indicate wherter peer or non-peer
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +int ufshcd_dme_xxx_get(struct ufs_hba *hba, u32 attr_sel,
> +u32 *mib_val, u8 peer)
> +{
> + struct uic_command uic_cmd = {0};
> + static const char *const action[] = {
> + "dme-get",
> + "dme-peer-get"
> + };
> + const char *get = action[!!peer];
> + int ret;
> +
> + uic_cmd.command = peer ?
> + UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET;
> + uic_cmd.argument1 = attr_sel;
> +
> + ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
> +
> + if (mib_val)
> + *mib_val = uic_cmd.argument3;
> +
> + dev_dbg(hba->dev, "%s: error code %d\n", get, ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_dme_xxx_get);
> +
> +/**
>   * ufshcd_make_hba_operational - Make UFS controller operational
>   * @hba: per adapter instance
>   *
> @@ -1540,6 +1622,8 @@ static void ufshcd_uic_cmd_compl(struct ufs_hba
> *hba)
>  {
>   hba->active_uic_cmd->argument2 |=
>   ufshcd_get_uic_cmd_result(hba);
> + hba->active_uic_cmd->argument3 =
> + ufshcd_get_dme_attr_val(hba);
>   complete(&hba->active_uic_cmd->done);
>  }
>
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 6095b3b..b47de70 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -217,4 +217,33 @@ int ufshcd_init(struct device *, struct ufs_hba ** ,
> void __iomem * ,
>   unsigned int);
>  void ufshcd_remove(struct ufs_hba *);
>
> +extern int ufshcd_dme_xxx_set(struct ufs_hba *hba, u32 attr_sel,
> +   u8 attr_set, u32 mib_val, u8 peer);
> +extern int ufshcd_dme_xxx_get(struct ufs_hba *hba, u32 attr_sel,
> +   u32 *mib_val, u8 peer);
> +
> +static inline int ufshcd_dme_set(struct ufs_hba *hba, u32 attr_sel,
> +  u8 attr_set, u32 mib_val)
> +{
> + return ufshcd_dme_xxx_set(hba, attr_sel, attr_set, mib_val, 0);
> +}
> +
> +static inline int ufshcd_dme_peer_set(struct ufs_hba *hba, u32 attr_sel,
> +

Re: [PATCH v2 3/7] scsi: ufs: amend interrupt configuration

[merez]

Hi James,

Can you please merge this patch?

Thanks,
Maya
> It makes interrupt setting more flexible especially
> for disabling. And wrong bit mask is fixed for ver 1.0.
> [17:16] is added for mask.
>
> Signed-off-by: Seungwon Jeon 
> Tested-by: Maya Erez 
> ---
>  drivers/scsi/ufs/ufshcd.c |   84
> +++-
>  drivers/scsi/ufs/ufshcd.h |4 +-
>  drivers/scsi/ufs/ufshci.h |5 ++-
>  3 files changed, 64 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index af7b81b..bc956e8 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -35,6 +35,10 @@
>
>  #include "ufshcd.h"
>
> +#define UFSHCD_ENABLE_INTRS  (UTP_TRANSFER_REQ_COMPL |\
> +  UTP_TASK_REQ_COMPL |\
> +  UFSHCD_ERROR_MASK)
> +
>  enum {
>   UFSHCD_MAX_CHANNEL  = 0,
>   UFSHCD_MAX_ID   = 1,
> @@ -64,6 +68,20 @@ enum {
>  };
>
>  /**
> + * ufshcd_get_intr_mask - Get the interrupt bit mask
> + * @hba - Pointer to adapter instance
> + *
> + * Returns interrupt bit mask per version
> + */
> +static inline u32 ufshcd_get_intr_mask(struct ufs_hba *hba)
> +{
> + if (hba->ufs_version == UFSHCI_VERSION_10)
> + return INTERRUPT_MASK_ALL_VER_10;
> + else
> + return INTERRUPT_MASK_ALL_VER_11;
> +}
> +
> +/**
>   * ufshcd_get_ufs_version - Get the UFS version supported by the HBA
>   * @hba - Pointer to adapter instance
>   *
> @@ -450,25 +468,45 @@ static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
>  }
>
>  /**
> - * ufshcd_int_config - enable/disable interrupts
> + * ufshcd_enable_intr - enable interrupts
>   * @hba: per adapter instance
> - * @option: interrupt option
> + * @intrs: interrupt bits
>   */
> -static void ufshcd_int_config(struct ufs_hba *hba, u32 option)
> +static void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs)
>  {
> - switch (option) {
> - case UFSHCD_INT_ENABLE:
> - ufshcd_writel(hba, hba->int_enable_mask, REG_INTERRUPT_ENABLE);
> - break;
> - case UFSHCD_INT_DISABLE:
> - if (hba->ufs_version == UFSHCI_VERSION_10)
> - ufshcd_writel(hba, INTERRUPT_DISABLE_MASK_10,
> -   REG_INTERRUPT_ENABLE);
> - else
> - ufshcd_writel(hba, INTERRUPT_DISABLE_MASK_11,
> -   REG_INTERRUPT_ENABLE);
> - break;
> + u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
> +
> + if (hba->ufs_version == UFSHCI_VERSION_10) {
> + u32 rw;
> + rw = set & INTERRUPT_MASK_RW_VER_10;
> + set = rw | ((set ^ intrs) & intrs);
> + } else {
> + set |= intrs;
> + }
> +
> + ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
> +}
> +
> +/**
> + * ufshcd_disable_intr - disable interrupts
> + * @hba: per adapter instance
> + * @intrs: interrupt bits
> + */
> +static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
> +{
> + u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
> +
> + if (hba->ufs_version == UFSHCI_VERSION_10) {
> + u32 rw;
> + rw = (set & INTERRUPT_MASK_RW_VER_10) &
> + ~(intrs & INTERRUPT_MASK_RW_VER_10);
> + set = rw | ((set & intrs) & ~INTERRUPT_MASK_RW_VER_10);
> +
> + } else {
> + set &= ~intrs;
>   }
> +
> + ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
>  }
>
>  /**
> @@ -955,8 +993,7 @@ static int ufshcd_dme_link_startup(struct ufs_hba
> *hba)
>   uic_cmd->argument3 = 0;
>
>   /* enable UIC related interrupts */
> - hba->int_enable_mask |= UIC_COMMAND_COMPL;
> - ufshcd_int_config(hba, UFSHCD_INT_ENABLE);
> + ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
>
>   /* sending UIC commands to controller */
>   ufshcd_send_uic_command(hba, uic_cmd);
> @@ -1003,13 +1040,7 @@ static int ufshcd_make_hba_operational(struct
> ufs_hba *hba)
>   }
>
>   /* Enable required interrupts */
> - hba->int_enable_mask |= (UTP_TRANSFER_REQ_COMPL |
> -  UIC_ERROR |
> -  UTP_TASK_REQ_COMPL |
> -  DEVICE_FATAL_ERROR |
> -  CONTROLLER_FATAL_ERROR |
> -  SYSTEM_BUS_FATAL_ERROR);
> - ufshcd_int_config(hba, UFSHCD_INT_ENABLE);
> + ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
>
>   /* Configure interrupt aggregation */
>   ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG);
> @@ -1837,7 +1868,7 @@ static void ufshcd_hba_free(struct ufs_hba *hba)
>  void ufshcd_remove(struct ufs_hba *hba)
>  {
>   /* disable interrupts */
> - ufshcd_int_config(hba, UFSHCD_INT_DISABLE);
> + ufshcd_disable_intr(hba, hba->intr_mask);
>
>   ufshcd_hba_stop(hba);
>   ufshcd_hba_free(hba);
> @@ -1895,6 +1926,9 @@ int ufshcd_init(struct device *dev, struct uf

Re: [PATCH v3 2/6] scsi: ufs: amend interrupt configuration

[merez]

Hi James,

Can you please merge this patch?

Thanks,
Maya
> It makes interrupt setting more flexible especially
> for disabling. And wrong bit mask is fixed for ver 1.0.
> [17:16] is added for mask.
>
> Signed-off-by: Seungwon Jeon 
> Tested-by: Maya Erez 
> ---
>  drivers/scsi/ufs/ufshcd.c |   84
> +++-
>  drivers/scsi/ufs/ufshcd.h |4 +-
>  drivers/scsi/ufs/ufshci.h |5 ++-
>  3 files changed, 64 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index cf7c8e4..feaf221 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -35,6 +35,10 @@
>
>  #include "ufshcd.h"
>
> +#define UFSHCD_ENABLE_INTRS  (UTP_TRANSFER_REQ_COMPL |\
> +  UTP_TASK_REQ_COMPL |\
> +  UFSHCD_ERROR_MASK)
> +
>  enum {
>   UFSHCD_MAX_CHANNEL  = 0,
>   UFSHCD_MAX_ID   = 1,
> @@ -64,6 +68,20 @@ enum {
>  };
>
>  /**
> + * ufshcd_get_intr_mask - Get the interrupt bit mask
> + * @hba - Pointer to adapter instance
> + *
> + * Returns interrupt bit mask per version
> + */
> +static inline u32 ufshcd_get_intr_mask(struct ufs_hba *hba)
> +{
> + if (hba->ufs_version == UFSHCI_VERSION_10)
> + return INTERRUPT_MASK_ALL_VER_10;
> + else
> + return INTERRUPT_MASK_ALL_VER_11;
> +}
> +
> +/**
>   * ufshcd_get_ufs_version - Get the UFS version supported by the HBA
>   * @hba - Pointer to adapter instance
>   *
> @@ -441,25 +459,45 @@ static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
>  }
>
>  /**
> - * ufshcd_int_config - enable/disable interrupts
> + * ufshcd_enable_intr - enable interrupts
>   * @hba: per adapter instance
> - * @option: interrupt option
> + * @intrs: interrupt bits
>   */
> -static void ufshcd_int_config(struct ufs_hba *hba, u32 option)
> +static void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs)
>  {
> - switch (option) {
> - case UFSHCD_INT_ENABLE:
> - ufshcd_writel(hba, hba->int_enable_mask, REG_INTERRUPT_ENABLE);
> - break;
> - case UFSHCD_INT_DISABLE:
> - if (hba->ufs_version == UFSHCI_VERSION_10)
> - ufshcd_writel(hba, INTERRUPT_DISABLE_MASK_10,
> -   REG_INTERRUPT_ENABLE);
> - else
> - ufshcd_writel(hba, INTERRUPT_DISABLE_MASK_11,
> -   REG_INTERRUPT_ENABLE);
> - break;
> + u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
> +
> + if (hba->ufs_version == UFSHCI_VERSION_10) {
> + u32 rw;
> + rw = set & INTERRUPT_MASK_RW_VER_10;
> + set = rw | ((set ^ intrs) & intrs);
> + } else {
> + set |= intrs;
> + }
> +
> + ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
> +}
> +
> +/**
> + * ufshcd_disable_intr - disable interrupts
> + * @hba: per adapter instance
> + * @intrs: interrupt bits
> + */
> +static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
> +{
> + u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
> +
> + if (hba->ufs_version == UFSHCI_VERSION_10) {
> + u32 rw;
> + rw = (set & INTERRUPT_MASK_RW_VER_10) &
> + ~(intrs & INTERRUPT_MASK_RW_VER_10);
> + set = rw | ((set & intrs) & ~INTERRUPT_MASK_RW_VER_10);
> +
> + } else {
> + set &= ~intrs;
>   }
> +
> + ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
>  }
>
>  /**
> @@ -946,8 +984,7 @@ static int ufshcd_dme_link_startup(struct ufs_hba
> *hba)
>   uic_cmd->argument3 = 0;
>
>   /* enable UIC related interrupts */
> - hba->int_enable_mask |= UIC_COMMAND_COMPL;
> - ufshcd_int_config(hba, UFSHCD_INT_ENABLE);
> + ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
>
>   /* sending UIC commands to controller */
>   ufshcd_send_uic_command(hba, uic_cmd);
> @@ -994,13 +1031,7 @@ static int ufshcd_make_hba_operational(struct
> ufs_hba *hba)
>   }
>
>   /* Enable required interrupts */
> - hba->int_enable_mask |= (UTP_TRANSFER_REQ_COMPL |
> -  UIC_ERROR |
> -  UTP_TASK_REQ_COMPL |
> -  DEVICE_FATAL_ERROR |
> -  CONTROLLER_FATAL_ERROR |
> -  SYSTEM_BUS_FATAL_ERROR);
> - ufshcd_int_config(hba, UFSHCD_INT_ENABLE);
> + ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
>
>   /* Configure interrupt aggregation */
>   ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG);
> @@ -1828,7 +1859,7 @@ static void ufshcd_hba_free(struct ufs_hba *hba)
>  void ufshcd_remove(struct ufs_hba *hba)
>  {
>   /* disable interrupts */
> - ufshcd_int_config(hba, UFSHCD_INT_DISABLE);
> + ufshcd_disable_intr(hba, hba->intr_mask);
>
>   ufshcd_hba_stop(hba);
>   ufshcd_hba_free(hba);
> @@ -1886,6 +1917,9 @@ int ufshcd_init(struct device *dev, struct ufs

Re: [PATCH v3 1/6] scsi: ufs: wrap the i/o access operations

[merez]

Hi James,

Can you please merge this patch on top of the query requests patch?

Thanks,
Maya
> Simplify operations with hiding mmio_base.
>
> Signed-off-by: Seungwon Jeon 
> Tested-by: Maya Erez 
> ---
>  drivers/scsi/ufs/ufshcd.c |  105
> +++--
>  drivers/scsi/ufs/ufshcd.h |7 +++-
>  2 files changed, 50 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index f244812..cf7c8e4 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -71,7 +71,7 @@ enum {
>   */
>  static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba)
>  {
> - return readl(hba->mmio_base + REG_UFS_VERSION);
> + return ufshcd_readl(hba, REG_UFS_VERSION);
>  }
>
>  /**
> @@ -130,8 +130,7 @@ static inline int ufshcd_get_tm_free_slot(struct
> ufs_hba *hba)
>   */
>  static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos)
>  {
> - writel(~(1 << pos),
> - (hba->mmio_base + REG_UTP_TRANSFER_REQ_LIST_CLEAR));
> + ufshcd_writel(hba, ~(1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR);
>  }
>
>  /**
> @@ -165,7 +164,7 @@ static inline int ufshcd_get_lists_status(u32 reg)
>   */
>  static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
>  {
> - return readl(hba->mmio_base + REG_UIC_COMMAND_ARG_2) &
> + return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_2) &
>  MASK_UIC_COMMAND_RESULT;
>  }
>
> @@ -238,18 +237,15 @@ ufshcd_config_int_aggr(struct ufs_hba *hba, int
> option)
>  {
>   switch (option) {
>   case INT_AGGR_RESET:
> - writel((INT_AGGR_ENABLE |
> - INT_AGGR_COUNTER_AND_TIMER_RESET),
> - (hba->mmio_base +
> -  REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL));
> + ufshcd_writel(hba, INT_AGGR_ENABLE |
> +   INT_AGGR_COUNTER_AND_TIMER_RESET,
> +   REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
>   break;
>   case INT_AGGR_CONFIG:
> - writel((INT_AGGR_ENABLE |
> - INT_AGGR_PARAM_WRITE |
> - INT_AGGR_COUNTER_THRESHOLD_VALUE |
> - INT_AGGR_TIMEOUT_VALUE),
> - (hba->mmio_base +
> -  REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL));
> + ufshcd_writel(hba, INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
> +   INT_AGGR_COUNTER_THRESHOLD_VALUE |
> +   INT_AGGR_TIMEOUT_VALUE,
> +   REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
>   break;
>   }
>  }
> @@ -262,12 +258,10 @@ ufshcd_config_int_aggr(struct ufs_hba *hba, int
> option)
>   */
>  static void ufshcd_enable_run_stop_reg(struct ufs_hba *hba)
>  {
> - writel(UTP_TASK_REQ_LIST_RUN_STOP_BIT,
> -(hba->mmio_base +
> - REG_UTP_TASK_REQ_LIST_RUN_STOP));
> - writel(UTP_TRANSFER_REQ_LIST_RUN_STOP_BIT,
> -(hba->mmio_base +
> - REG_UTP_TRANSFER_REQ_LIST_RUN_STOP));
> + ufshcd_writel(hba, UTP_TASK_REQ_LIST_RUN_STOP_BIT,
> +   REG_UTP_TASK_REQ_LIST_RUN_STOP);
> + ufshcd_writel(hba, UTP_TRANSFER_REQ_LIST_RUN_STOP_BIT,
> +   REG_UTP_TRANSFER_REQ_LIST_RUN_STOP);
>  }
>
>  /**
> @@ -276,7 +270,7 @@ static void ufshcd_enable_run_stop_reg(struct ufs_hba
> *hba)
>   */
>  static inline void ufshcd_hba_start(struct ufs_hba *hba)
>  {
> - writel(CONTROLLER_ENABLE , (hba->mmio_base + REG_CONTROLLER_ENABLE));
> + ufshcd_writel(hba, CONTROLLER_ENABLE, REG_CONTROLLER_ENABLE);
>  }
>
>  /**
> @@ -287,7 +281,7 @@ static inline void ufshcd_hba_start(struct ufs_hba
> *hba)
>   */
>  static inline int ufshcd_is_hba_active(struct ufs_hba *hba)
>  {
> - return (readl(hba->mmio_base + REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
> + return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
>  }
>
>  /**
> @@ -299,8 +293,7 @@ static inline
>  void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
>  {
>   __set_bit(task_tag, &hba->outstanding_reqs);
> - writel((1 << task_tag),
> -(hba->mmio_base + REG_UTP_TRANSFER_REQ_DOOR_BELL));
> + ufshcd_writel(hba, 1 << task_tag, REG_UTP_TRANSFER_REQ_DOOR_BELL);
>  }
>
>  /**
> @@ -381,8 +374,7 @@ void ufshcd_copy_query_response(struct ufs_hba *hba,
> struct ufshcd_lrb *lrbp)
>   */
>  static inline void ufshcd_hba_capabilities(struct ufs_hba *hba)
>  {
> - hba->capabilities =
> - readl(hba->mmio_base + REG_CONTROLLER_CAPABILITIES);
> + hba->capabilities = ufshcd_readl(hba, REG_CONTROLLER_CAPABILITIES);
>
>   /* nutrs and nutmrs are 0 based values */
>   hba->nutrs = (hba->capabilities & MASK_TRANSFER_REQUESTS_SLOTS) + 1;
> @@ -399,16 +391,13 @@ static inline void
>  ufshcd_send_uic_command(struct ufs_hba *hba, struct uic_command
> *uic_cmnd)
>  {
>   /* Write Args */
> - writel(uic_cmnd->argument1,
> - 

Re: [PATCH v3 4/6] scsi: ufs: rework link start-up process

[merez]

Hi,

The proposed fix fixes the timeout crash.
In normal operation the patch works, so after applying the fix you can add
my Tested-By.

One additional minor fix that is required:
In ufshcd_link_startup the following print-out is printed in success as well:
dev_err(hba->dev, "link startup failed %d\n", ret);

Please move it to the error cases only.

Thanks,
Maya

> On 5/6/2013 11:07 AM, Seungwon Jeon wrote:
>> Link start-up requires long time with multiphase handshakes
>> between UFS host and device. This affects driver's probe time.
>> This patch let link start-up run asynchronously. Link start-up
>> will be executed at the end of prove separately.
>> Along with this change, the following is worked.
>>
>> Defined completion time of uic command to avoid a permanent wait.
> We have seen a kernel crash when the timeout happens in our internal
> testing. This is basically because the the UIC_CMD_TIMEOUT of 500msec is
> less for our platform. We see UIC_COMMAND_COMPL interrupt after this
> timeout and since uic_cmd is freed we see a kernel crash.
>
> [6.026094] ufshcd :01:00.0: link startup: error code -110
> [6.030886] ufshcd :01:00.0: link startup failed -110
> ...
>
> [6.487288] BUG: spinlock bad magic on CPU#0, swapper/0/0
> [6.491683] Unable to handle kernel paging request at virtual address
> 612b
> ...
> [7.625270] [] (spin_dump+0x4c/0x88) from []
> (do_raw_spin_lock+0x20/0x154)
> [7.633877] [] (do_raw_spin_lock+0x20/0x154) from
> [] (_raw_spin_lock_irqsave+0x28/0x30)
> [7.643583] [] (_raw_spin_lock_irqsave+0x28/0x30) from
> [] (complete+0x1c/0x58)
> [7.652525] [] (complete+0x1c/0x58) from []
> (ufshcd_intr+0x1a8/0x498)
> [7.660705] [] (ufshcd_intr+0x1a8/0x498) from []
> (handle_irq_event_percpu+0xb0/0x290)
> [7.670227] [] (handle_irq_event_percpu+0xb0/0x290) from
> [] (handle_irq_event+0x3c/0x5c)
> [7.680054] [] (handle_irq_event+0x3c/0x5c) from
> [] (handle_fasteoi_irq+0xdc/0x148)
> [7.689424] [] (handle_fasteoi_irq+0xdc/0x148) from
> [] (generic_handle_irq+0x30/0x44)
> [7.698977] [] (generic_handle_irq+0x30/0x44) from
> [] (handle_IRQ+0x7c/0xc0)
> [7.707736] [] (handle_IRQ+0x7c/0xc0) from []
> (gic_handle_irq+0x94/0x110)
> [7.716252] [] (gic_handle_irq+0x94/0x110) from
> [] (__irq_svc+0x40/0x70)
>
>
> Following would fix this. Please check. Also, we need to increase the
> timeout to say 5 secs to avoid such races.
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index ecbae40..ae22a8e 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -485,6 +485,7 @@ static int
>  ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
>  {
>   int ret;
> + unsigned long flags;
>
>   if (wait_for_completion_timeout(&uic_cmd->done,
>   msecs_to_jiffies(UIC_CMD_TIMEOUT)))
> @@ -492,6 +493,10 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct
> uic_command *uic_cmd)
>   else
>   ret = -ETIMEDOUT;
>
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + hba->active_uic_cmd = NULL;
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> +
>   return ret;
>  }
>
> @@ -1842,7 +1847,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
> struct ufshcd_lrb *lrbp)
>   */
>  static void ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
>  {
> - if (intr_status & UIC_COMMAND_COMPL) {
> + if (hba->active_uic_cmd && (intr_status & UIC_COMMAND_COMPL)) {
>   hba->active_uic_cmd->argument2 |=
>   ufshcd_get_uic_cmd_result(hba);
>   hba->active_uic_cmd->argument3 =
>
>
>> Added mutex to guarantee of uic command at a time.
>> Adapted some sequence of controller initialization after link statup
>> according to HCI standard.
>>
>> Signed-off-by: Seungwon Jeon 
>> Signed-off-by: Sujit Reddy Thumma 
>> ---
>>   drivers/scsi/ufs/ufshcd.c |  287
>> ++---
>>   drivers/scsi/ufs/ufshcd.h |   10 +-
>>   2 files changed, 200 insertions(+), 97 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index e04c74e..118f104 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -33,11 +33,15 @@
>>* this program.
>>*/
>>
>> +#include 
>> +
>>   #include "ufshcd.h"
>>
>>   #define UFSHCD_ENABLE_INTRS(UTP_TRANSFER_REQ_COMPL |\
>>   UTP_TASK_REQ_COMPL |\
>>   UFSHCD_ERROR_MASK)
>> +/* UIC command timeout, unit: ms */
>> +#define UIC_CMD_TIMEOUT 500
>>
>>   enum {
>>  UFSHCD_MAX_CHANNEL  = 0,
>> @@ -401,24 +405,115 @@ static inline void ufshcd_hba_capabilities(struct
>> ufs_hba *hba)
>>   }
>>
>>   /**
>> - * ufshcd_send_uic_command - Send UIC commands to unipro layers
>> + * ufshcd_ready_for_uic_cmd - Check if controller is ready
>> + *to accept UIC commands

Re: [PATCH v2 1/7] scsi: ufs: move the ufshcd_hba_stop to ufshcd.c

[merez]

Hi,

ufshcd_pci_shutdown (in ufshcd_pci.c) is using ufshcd_hba_stop so it
cannot be moved to ufshcd.c.
Sorry for missing it last time I tested, I returned ufshcd_hba_stop to
ufshcd.h by mistake when I resolved conflicts of the next patch so I
didn't hit the compilation errors.

Thanks,
Maya
> Move the ufshcd_hba_stop from header file.
>
> Signed-off-by: Seungwon Jeon 
> Reviewed-by: Subhash Jadavani 
> Tested-by: Maya Erez 
> ---
>  drivers/scsi/ufs/ufshcd.c |9 +
>  drivers/scsi/ufs/ufshcd.h |9 -
>  2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index f244812..d932239 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -280,6 +280,15 @@ static inline void ufshcd_hba_start(struct ufs_hba
> *hba)
>  }
>
>  /**
> + * ufshcd_hba_stop - Send controller to reset state
> + * @hba: per adapter instance
> + */
> +static inline void ufshcd_hba_stop(struct ufs_hba *hba)
> +{
> + writel(CONTROLLER_DISABLE, (hba->mmio_base + REG_CONTROLLER_ENABLE));
> +}
> +
> +/**
>   * ufshcd_is_hba_active - Get controller state
>   * @hba: per adapter instance
>   *
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 336980b..3a1052d 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -208,13 +208,4 @@ int ufshcd_init(struct device *, struct ufs_hba ** ,
> void __iomem * ,
>   unsigned int);
>  void ufshcd_remove(struct ufs_hba *);
>
> -/**
> - * ufshcd_hba_stop - Send controller to reset state
> - * @hba: per adapter instance
> - */
> -static inline void ufshcd_hba_stop(struct ufs_hba *hba)
> -{
> - writel(CONTROLLER_DISABLE, (hba->mmio_base + REG_CONTROLLER_ENABLE));
> -}
> -
>  #endif /* End of Header */
> --
> 1.7.0.4
>
>
> --
> 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
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] scsi: ufs: move the ufshcd_hba_stop to ufshcd.c

[merez]

Hi,

Can you please rebase the next version on top of the following patches
which are the next candidates for merge?
Vinayak Holikatti (2):
  ufs: Add Platform glue driver for ufshcd
  ufs: Correct the expected data transfersize
Dolev Raviv (1):
  scsi: ufs: add support for query requests

Thanks,
Maya
> On Wed, Apr 24, 2013 at 9:36 PM, Seungwon Jeon 
> wrote:
>> Move the ufshcd_hba_stop from header file.
>>
>> Signed-off-by: Seungwon Jeon 
>> ---
>>  drivers/scsi/ufs/ufshcd.c |9 +
>>  drivers/scsi/ufs/ufshcd.h |9 -
>>  2 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 60fd40c..41b9639 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -285,6 +285,15 @@ static inline void ufshcd_hba_start(struct ufs_hba
>> *hba)
>>  }
>>
>>  /**
>> + * ufshcd_hba_stop - Send controller to reset state
>> + * @hba: per adapter instance
>> + */
>> +static inline void ufshcd_hba_stop(struct ufs_hba *hba)
>> +{
>> +   writel(CONTROLLER_DISABLE, (hba->mmio_base +
>> REG_CONTROLLER_ENABLE));
>> +}
>> +
>> +/**
>>   * ufshcd_is_hba_active - Get controller state
>>   * @hba: per adapter instance
>>   *
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index 6b99a42..1680394 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -190,13 +190,4 @@ int ufshcd_init(struct device *, struct ufs_hba **
>> , void __iomem * ,
>> unsigned int);
>>  void ufshcd_remove(struct ufs_hba *);
>>
>> -/**
>> - * ufshcd_hba_stop - Send controller to reset state
>> - * @hba: per adapter instance
>> - */
>> -static inline void ufshcd_hba_stop(struct ufs_hba *hba)
>> -{
>> -   writel(CONTROLLER_DISABLE, (hba->mmio_base +
>> REG_CONTROLLER_ENABLE));
>> -}
>> -
>>  #endif /* End of Header */
>> --
>> 1.7.0.4
>>
>>
>
> The patch doesn't apply due to the character set.
>
> "fatal: cannot convert from ks_c_5601-1987 to UTF-8"
>
>
> --
> ~Santosh
> --
> 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
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] scsi: ufs: add dme operations

[merez]

Tested-by: Maya Erez 

> Add uic command operations including DME_XXX series.
>
> Signed-off-by: Seungwon Jeon 
> ---
>  drivers/scsi/ufs/ufs-attrs.h |  129 
>  drivers/scsi/ufs/ufshcd.c|  220
> +-
>  drivers/scsi/ufs/ufshcd.h|   55 +++
>  drivers/scsi/ufs/ufshci.h|   19 
>  4 files changed, 422 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/scsi/ufs/ufs-attrs.h
>
> diff --git a/drivers/scsi/ufs/ufs-attrs.h b/drivers/scsi/ufs/ufs-attrs.h
> new file mode 100644
> index 000..562bb49
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-attrs.h
> @@ -0,0 +1,129 @@
> +/*
> + * drivers/scsi/ufs/ufs-attrs.h
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef _UFS_ATTRS_H_
> +#define _UFS_ATTRS_H_
> +
> +/*
> + * PHY Adpater attributes
> + */
> +#define PA_ACTIVETXDATALANES 0x1560
> +#define PA_ACTIVERXDATALANES 0x1580
> +#define PA_TXTRAILINGCLOCKS  0x1564
> +#define PA_PHY_TYPE  0x1500
> +#define PA_AVAILTXDATALANES  0x1520
> +#define PA_AVAILRXDATALANES  0x1540
> +#define PA_MINRXTRAILINGCLOCKS   0x1543
> +#define PA_TXPWRSTATUS   0x1567
> +#define PA_RXPWRSTATUS   0x1582
> +#define PA_TXFORCECLOCK  0x1562
> +#define PA_TXPWRMODE 0x1563
> +#define PA_LEGACYDPHYESCDL   0x1570
> +#define PA_MAXTXSPEEDFAST0x1521
> +#define PA_MAXTXSPEEDSLOW0x1522
> +#define PA_MAXRXSPEEDFAST0x1541
> +#define PA_MAXRXSPEEDSLOW0x1542
> +#define PA_TXLINKSTARTUPHS   0x1544
> +#define PA_TXSPEEDFAST   0x1565
> +#define PA_TXSPEEDSLOW   0x1566
> +#define PA_REMOTEVERINFO 0x15A0
> +#define PA_TXGEAR0x1568
> +#define PA_TXTERMINATION 0x1569
> +#define PA_HSSERIES  0x156A
> +#define PA_PWRMODE   0x1571
> +#define PA_RXGEAR0x1583
> +#define PA_RXTERMINATION 0x1584
> +#define PA_MAXRXPWMGEAR  0x1586
> +#define PA_MAXRXHSGEAR   0x1587
> +#define PA_RXHSUNTERMCAP 0x15A5
> +#define PA_RXLSTERMCAP   0x15A6
> +#define PA_PACPREQTIMEOUT0x1590
> +#define PA_PACPREQEOBTIMEOUT 0x1591
> +#define PA_LOCALVERINFO  0x15A9
> +#define PA_TACTIVATE 0x15A8
> +#define PA_PACPFRAMECOUNT0x15C0
> +#define PA_PACPERRORCOUNT0x15C1
> +#define PA_PHYTESTCONTROL0x15C2
> +#define PA_PWRMODEUSERDATA0  0x15B0
> +#define PA_PWRMODEUSERDATA1  0x15B1
> +#define PA_PWRMODEUSERDATA2  0x15B2
> +#define PA_PWRMODEUSERDATA3  0x15B3
> +#define PA_PWRMODEUSERDATA4  0x15B4
> +#define PA_PWRMODEUSERDATA5  0x15B5
> +#define PA_PWRMODEUSERDATA6  0x15B6
> +#define PA_PWRMODEUSERDATA7  0x15B7
> +#define PA_PWRMODEUSERDATA8  0x15B8
> +#define PA_PWRMODEUSERDATA9  0x15B9
> +#define PA_PWRMODEUSERDATA10 0x15BA
> +#define PA_PWRMODEUSERDATA11 0x15BB
> +#define PA_CONNECTEDTXDATALANE   0x1561
> +#define PA_CONNECTEDRXDATALANE   0x1581
> +#define PA_LOGICALLANEMAP0x15A1
> +#define PA_SLEEPNOCONFIGTIME 0x15A2
> +#define PA_STALLNOCONFIGTIME 0x15A3
> +#define PA_SAVECONFIGTIME0x15A4
> +
> +/*
> + * Data Link Layer Attributes
> + */
> +#define DL_TC0TXFCTHRESHOLD  0x2040
> +#define DL_FC0PROTTIMEOUTVAL 0x2041
> +#define DL_TC0REPLAYTIMEOUTVAL   0x2042
> +#define DL_AFC0REQTIMEOUTVAL 0x2043
> +#define DL_AFC0CREDITTHRESHOLD   0x2044
> +#define DL_TC0OUTACKTHRESHOLD0x2045
> +#define DL_TC1TXFCTHRESHOLD  0x2060
> +#define DL_FC1PROTTIMEOUTVAL 0x2061
> +#define DL_TC1REPLAYTIMEOUTVAL   0x2062
> +#define DL_AFC1REQTIMEOUTVAL 0x2063
> +#define DL_AFC1CREDITTHRESHOLD   0x2064
> +#define DL_TC1OUTACKTHRESHOLD0x2065
> +#define DL_TXPREEMPTIONCAP   0x2000
> +#define DL_TC0TXMAXSDUSIZE   0x2001
> +#define DL_TC0RXINITCREDITVAL0x2002
> +#define DL_TC0TXBUFFERSIZE   0x2005
> +#define DL_PEERTC0PRESENT0x2046
> +#define DL_PEERTC0RXINITCREVAL   0x2047
> +#define DL_TC1TXMAXSDUSIZE   0x2003
> +#define DL_TC1RXINITCREDITVAL0x2004
> +#define DL_TC1TXBUFFERSIZE   0x2006
> +#define DL_PEERTC1PRESENT0x2066
> +#define DL_PEERTC1RXINITCREVAL   0x2067
> +
> +/*
> + * Network Layer Attributes
> + */
> +#define N_DEVICEID   0x3000
> +#define N_DEVICEID_VALID 0x3001
> +#define N_TC0TXMAXSDUSIZE0x3020
> +#define N_TC1TXMAXSDUSIZE0x3021
> +
> +/*
> + * Transport Layer Attributes
> + */
> +#define T_NUMCPORTS  0x4000
> +#define T_NUMTESTFEATURES0x4001
> +#define T_CONNECTIONSTATE0x4020
> +#define T_PEERDEVICEID   0x4021
> +#define T_PEERCPORTID0x4022
> +#define T_TRAFFICCLASS   0x4023
> +#define T_PROTOCOLID 0x4024
> +#define T_CPORTFLAGS 0x402

Re: [PATCH 3/5] scsi: ufs: amend interrupt configuration

[merez]

Tested-by: Maya Erez 

> Patch looks good but one minor comment below.
>
> On 4/24/2013 9:36 PM, Seungwon Jeon wrote:
>> It makes interrupt setting more flexible especially
>> for disabling. And wrong bit mask is fixed for ver 1.0.
>> [17:16] is added for mask.
>>
>> Signed-off-by: Seungwon Jeon 
>> ---
>>   drivers/scsi/ufs/ufshcd.c |   86
>> -
>>   drivers/scsi/ufs/ufshcd.h |4 +-
>>   drivers/scsi/ufs/ufshci.h |5 ++-
>>   3 files changed, 66 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index b6c19b0..efe2256 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -35,6 +35,10 @@
>>
>>   #include "ufshcd.h"
>>
>> +#define UFSHCD_ENABLE_INTRS (UTP_TRANSFER_REQ_COMPL |\
>> + UTP_TASK_REQ_COMPL |\
>> + UFSHCD_ERROR_MASK)
>> +
>
> I don't see any use of this macro in this patch. could you please remove
> it or move it to the patch-set where its really being used.
>
>>   enum {
>>  UFSHCD_MAX_CHANNEL  = 0,
>>  UFSHCD_MAX_ID   = 1,
>> @@ -64,6 +68,20 @@ enum {
>>   };
>>
>>   /**
>> + * ufshcd_get_intr_mask - Get the interrupt bit mask
>> + * @hba - Pointer to adapter instance
>> + *
>> + * Returns interrupt bit mask per version
>> + */
>> +static inline u32 ufshcd_get_intr_mask(struct ufs_hba *hba)
>> +{
>> +if (hba->ufs_version == UFSHCI_VERSION_10)
>> +return INTERRUPT_MASK_ALL_VER_10;
>> +else
>> +return INTERRUPT_MASK_ALL_VER_11;
>> +}
>> +
>> +/**
>>* ufshcd_get_ufs_version - Get the UFS version supported by the HBA
>>* @hba - Pointer to adapter instance
>>*
>> @@ -397,25 +415,45 @@ static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
>>   }
>>
>>   /**
>> - * ufshcd_int_config - enable/disable interrupts
>> + * ufshcd_enable_intr - enable interrupts
>>* @hba: per adapter instance
>> - * @option: interrupt option
>> + * @intrs: interrupt bits
>>*/
>> -static void ufshcd_int_config(struct ufs_hba *hba, u32 option)
>> +static void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs)
>>   {
>> -switch (option) {
>> -case UFSHCD_INT_ENABLE:
>> -ufshcd_writel(hba, REG_INTERRUPT_ENABLE, hba->int_enable_mask);
>> -break;
>> -case UFSHCD_INT_DISABLE:
>> -if (hba->ufs_version == UFSHCI_VERSION_10)
>> -ufshcd_writel(hba, REG_INTERRUPT_ENABLE,
>> -  INTERRUPT_DISABLE_MASK_10);
>> -else
>> -ufshcd_writel(hba, REG_INTERRUPT_ENABLE,
>> -  INTERRUPT_DISABLE_MASK_11);
>> -break;
>> +u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
>> +
>> +if (hba->ufs_version == UFSHCI_VERSION_10) {
>> +u32 rw;
>> +rw = set & INTERRUPT_MASK_RW_VER_10;
>> +set = rw | ((set ^ intrs) & intrs);
>> +} else {
>> +set |= intrs;
>> +}
>> +
>> +ufshcd_writel(hba, REG_INTERRUPT_ENABLE, set);
>> +}
>> +
>> +/**
>> + * ufshcd_disable_intr - disable interrupts
>> + * @hba: per adapter instance
>> + * @intrs: interrupt bits
>> + */
>> +static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
>> +{
>> +u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
>> +
>> +if (hba->ufs_version == UFSHCI_VERSION_10) {
>> +u32 rw;
>> +rw = (set & INTERRUPT_MASK_RW_VER_10) &
>> +~(intrs & INTERRUPT_MASK_RW_VER_10);
>> +set = rw | ((set & intrs) & ~INTERRUPT_MASK_RW_VER_10);
>> +
>> +} else {
>> +set &= ~intrs;
>>  }
>> +
>> +ufshcd_writel(hba, REG_INTERRUPT_ENABLE, set);
>>   }
>>
>>   /**
>> @@ -717,8 +755,7 @@ static int ufshcd_dme_link_startup(struct ufs_hba
>> *hba)
>>  uic_cmd->argument3 = 0;
>>
>>  /* enable UIC related interrupts */
>> -hba->int_enable_mask |= UIC_COMMAND_COMPL;
>> -ufshcd_int_config(hba, UFSHCD_INT_ENABLE);
>> +ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
>>
>>  /* sending UIC commands to controller */
>>  ufshcd_send_uic_command(hba, uic_cmd);
>> @@ -765,13 +802,9 @@ static int ufshcd_make_hba_operational(struct
>> ufs_hba *hba)
>>  }
>>
>>  /* Enable required interrupts */
>> -hba->int_enable_mask |= (UTP_TRANSFER_REQ_COMPL |
>> - UIC_ERROR |
>> - UTP_TASK_REQ_COMPL |
>> - DEVICE_FATAL_ERROR |
>> - CONTROLLER_FATAL_ERROR |
>> - SYSTEM_BUS_FATAL_ERROR);
>> -ufshcd_int_config(hba, UFSHCD_INT_ENABLE);
>> +ufshcd_enable_intr(hba, UTP_TRANSFER_REQ_COMPL | UIC_ERROR |
>> +   UTP_TASK_REQ_COMPL | DEVICE_FATAL_ERROR |
>> +   CONTROLLER_FATAL_ERROR | SYSTEM_BUS_FATAL_ERROR);
>>
>>  /* Configure interrupt aggregation */
>>  u

Re: [PATCH 2/5] scsi: ufs: wrap the i/o access operations

[merez]

Tested-by: Maya Erez 

I also tend to agree with Sujit on the order of the wrappers parameters.

Thanks,
Maya

> On 4/26/2013 10:36 AM, Seungwon Jeon wrote:
>> Hi,
>>
>> On Thursday, April 25, 2013, Sujit Reddy Thumma wrote:
>>> On 4/24/2013 9:36 PM, Seungwon Jeon wrote:
 Simplify operations with hiding mmio_base.

 Signed-off-by: Seungwon Jeon 
 ---
drivers/scsi/ufs/ufshcd.c |  106
 +++--
drivers/scsi/ufs/ufshcd.h |5 ++
2 files changed, 49 insertions(+), 62 deletions(-)

 diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
 index 1680394..6728450 100644
 --- a/drivers/scsi/ufs/ufshcd.h
 +++ b/drivers/scsi/ufs/ufshcd.h
 @@ -190,4 +190,9 @@ int ufshcd_init(struct device *, struct ufs_hba **
 , void __iomem * ,
unsigned int);
void ufshcd_remove(struct ufs_hba *);

 +#define ufshcd_writel(hba, reg, val)  \
>>> Let this be consistent with writel() arguments - "val" as second arg
>>> and
>>> "reg" as third?
>> You got a point there.
>> When considering an array of arguments in two functions and value part
>> can be some long expression,
>> I think it seems more coherent.
>>  ufshcd_readl(hba, reg);
>>  ufshcd_writel(hba, reg, val);
>> How about keeping these?
>
> I somehow tend to agree with what Sujit suggested. Its good to be
> consitent with writel() for better code readability.
>
>>
>> Thanks,
>> Seungwon Jeon
>>
 +  writel((val), (hba)->mmio_base + (reg))
 +#define ufshcd_readl(hba, reg)\
 +  readl((hba)->mmio_base + (reg))
 +
#endif /* End of Header */

>>> --
>>> 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-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-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] scsi: ufs: move the ufshcd_hba_stop to ufshcd.c

[merez]

Tested-by: Maya Erez 

>
> Looks good. Reviewed-by: Subhash Jadavani 
>
> On 4/24/2013 9:36 PM, Seungwon Jeon wrote:
>> Move the ufshcd_hba_stop from header file.
>>
>> Signed-off-by: Seungwon Jeon 
>> ---
>>  drivers/scsi/ufs/ufshcd.c |9 +
>>  drivers/scsi/ufs/ufshcd.h |9 -
>>  2 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 60fd40c..41b9639 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -285,6 +285,15 @@ static inline void ufshcd_hba_start(struct ufs_hba
>> *hba)
>>  }
>>
>>  /**
>> + * ufshcd_hba_stop - Send controller to reset state
>> + * @hba: per adapter instance
>> + */
>> +static inline void ufshcd_hba_stop(struct ufs_hba *hba)
>> +{
>> +writel(CONTROLLER_DISABLE, (hba->mmio_base + REG_CONTROLLER_ENABLE));
>> +}
>> +
>> +/**
>>   * ufshcd_is_hba_active - Get controller state
>>   * @hba: per adapter instance
>>   *
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index 6b99a42..1680394 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -190,13 +190,4 @@ int ufshcd_init(struct device *, struct ufs_hba **
>> , void __iomem * ,
>>  unsigned int);
>>  void ufshcd_remove(struct ufs_hba *);
>>
>> -/**
>> - * ufshcd_hba_stop - Send controller to reset state
>> - * @hba: per adapter instance
>> - */
>> -static inline void ufshcd_hba_stop(struct ufs_hba *hba)
>> -{
>> -writel(CONTROLLER_DISABLE, (hba->mmio_base + REG_CONTROLLER_ENABLE));
>> -}
>> -
>>  #endif /* End of Header */
>
> --
> 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
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V6 Resend2 2/2] [SCSI] ufs: Correct the expected data transfersize

[merez]

Hi James,

Can this patch be poushed to 3.9+ merge window?

Thnaks,
Maya
> This patch corrects the expected data transfer size of the
> command UPIU. The current implementation of cmd->transfersize
> is wrong as it probably equal to sector size. With this
> implementation the transfer size is updated correctly
>
> Reported-by: KOBAYASHI Yoshitake 
> Reviewed-by: Arnd Bergmann 
> Reviewed-by: Namjae Jeon 
> Reviewed-by: Subhash Jadavani 
> Tested-by: Maya Erez 
> Signed-off-by: Santosh Yaraganavi 
> Signed-off-by: Vinayak Holikatti 
> ---
>  drivers/scsi/ufs/ufshcd.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 60fd40c..c32a478 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -478,7 +478,7 @@ static void ufshcd_compose_upiu(struct ufshcd_lrb
> *lrbp)
>   ucd_cmd_ptr->header.dword_2 = 0;
>
>   ucd_cmd_ptr->exp_data_transfer_len =
> - cpu_to_be32(lrbp->cmd->transfersize);
> + cpu_to_be32(lrbp->cmd->sdb.length);
>
>   memcpy(ucd_cmd_ptr->cdb,
>  lrbp->cmd->cmnd,
> --
> 1.7.5.4
>
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V6 Resend2 1/2] [SCSI] ufs: Add Platform glue driver for ufshcd

[merez]

Hi James,

Can this patch be poushed to 3.9+ merge window?

Thnaks,
Maya
> This patch adds Platform glue driver for ufshcd.
>
> Reviewed-by: Arnd Bergmann 
> Reviewed-by: Namjae Jeon 
> Reviewed-by: Subhash Jadavani 
> Reviewed-by: Sujit Reddy Thumma 
> Tested-by: Maya Erez 
> Signed-off-by: Vinayak Holikatti 
> Signed-off-by: Santosh Yaraganavi 
> ---
>  drivers/scsi/ufs/Kconfig |   11 ++
>  drivers/scsi/ufs/Makefile|1 +
>  drivers/scsi/ufs/ufshcd-pltfrm.c |  217
> ++
>  3 files changed, 229 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/scsi/ufs/ufshcd-pltfrm.c
>
> diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
> index 0371047..35faf24 100644
> --- a/drivers/scsi/ufs/Kconfig
> +++ b/drivers/scsi/ufs/Kconfig
> @@ -57,3 +57,14 @@ config SCSI_UFSHCD_PCI
> If you have a controller with this interface, say Y or M here.
>
> If unsure, say N.
> +
> +config SCSI_UFSHCD_PLATFORM
> + tristate "Platform bus based UFS Controller support"
> + depends on SCSI_UFSHCD
> + ---help---
> + This selects the UFS host controller support. Select this if
> + you have an UFS controller on Platform bus.
> +
> + If you have a controller with this interface, say Y or M here.
> +
> +   If unsure, say N.
> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> index 9eda0df..1e5bd48 100644
> --- a/drivers/scsi/ufs/Makefile
> +++ b/drivers/scsi/ufs/Makefile
> @@ -1,3 +1,4 @@
>  # UFSHCD makefile
>  obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
>  obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
> +obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c
> b/drivers/scsi/ufs/ufshcd-pltfrm.c
> new file mode 100644
> index 000..03319ac
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -0,0 +1,217 @@
> +/*
> + * Universal Flash Storage Host controller Platform bus based glue driver
> + *
> + * This code is based on drivers/scsi/ufs/ufshcd-pltfrm.c
> + * Copyright (C) 2011-2013 Samsung India Software Operations
> + *
> + * Authors:
> + *   Santosh Yaraganavi 
> + *   Vinayak Holikatti 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + * See the COPYING file in the top-level directory or visit
> + * 
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This program is provided "AS IS" and "WITH ALL FAULTS" and
> + * without warranty of any kind. You are solely responsible for
> + * determining the appropriateness of using and distributing
> + * the program and assume all risks associated with your exercise
> + * of rights with respect to the program, including but not limited
> + * to infringement of third party rights, the risks and costs of
> + * program errors, damage to or loss of data, programs or equipment,
> + * and unavailability or interruption of operations. Under no
> + * circumstances will the contributor of this Program be liable for
> + * any damages of any kind arising from your use or distribution of
> + * this program.
> + */
> +
> +#include "ufshcd.h"
> +#include 
> +
> +#ifdef CONFIG_PM
> +/**
> + * ufshcd_pltfrm_suspend - suspend power management function
> + * @dev: pointer to device handle
> + *
> + *
> + * Returns 0
> + */
> +static int ufshcd_pltfrm_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct ufs_hba *hba =  platform_get_drvdata(pdev);
> +
> + /*
> +  * TODO:
> +  * 1. Call ufshcd_suspend
> +  * 2. Do bus specific power management
> +  */
> +
> + disable_irq(hba->irq);
> +
> + return 0;
> +}
> +
> +/**
> + * ufshcd_pltfrm_resume - resume power management function
> + * @dev: pointer to device handle
> + *
> + * Returns 0
> + */
> +static int ufshcd_pltfrm_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct ufs_hba *hba =  platform_get_drvdata(pdev);
> +
> + /*
> +  * TODO:
> +  * 1. Call ufshcd_resume.
> +  * 2. Do bus specific wake up
> +  */
> +
> + enable_irq(hba->irq);
> +
> + return 0;
> +}
> +#else
> +#define ufshcd_pltfrm_suspendNULL
> +#define ufshcd_pltfrm_resume NULL
> +#endif
> +
> +/**
> + * ufshcd_pltfrm_probe - probe routine of the driver
> + * @pdev: pointer to Platform device handle
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +static int ufshcd_pltfrm_probe(struct platform_device *pdev)
> +{
> + struct ufs_hba *hba

Re: [PATCH V6 0/4] [SCSI] ufs: Adds glue drivers to ufshcd

[merez]

Tested-by: Maya Erez 

> Hi James,
>
> look like these patches are fine. Can you please integrate these
> patches in your tree.
>
>
> On Tue, Feb 5, 2013 at 10:17 PM,   wrote:
>> From: Vinayak Holikatti 
>>
>> This patch set adds following features
>>  - Seggregate PCI specific code in ufshcd.c and corrected copyright
>>  - Adds PCI glue driver ufshcd-pci.c and ufshcd.c become core module
>>  - Adds Platform glue driver ufshcd-pltfrm.c
>>  - Update correct transfer size in Command UPIU
>>
>> Vinayak Holikatti (4):
>>   [SCSI] drivers/scsi/ufs: Seggregate PCI Specific Code
>>   [SCSI] drivers/scsi/ufs: Separate PCI code into glue driver
>>   [SCSI] ufs: Add Platform glue driver for ufshcd
>>   [SCSI] ufs: Correct the expected data transfersize
>>
>>  drivers/scsi/ufs/Kconfig |   87 +---
>>  drivers/scsi/ufs/Makefile|2 +
>>  drivers/scsi/ufs/ufs.h   |   44 ++---
>>  drivers/scsi/ufs/ufshcd-pci.c|  211 +++
>>  drivers/scsi/ufs/ufshcd-pltfrm.c |  217 +++
>>  drivers/scsi/ufs/ufshcd.c|  426
>> --
>>  drivers/scsi/ufs/ufshcd.h|  202 ++
>>  drivers/scsi/ufs/ufshci.h|   44 ++---
>>  8 files changed, 811 insertions(+), 422 deletions(-)
>>  create mode 100644 drivers/scsi/ufs/ufshcd-pci.c
>>  create mode 100644 drivers/scsi/ufs/ufshcd-pltfrm.c
>>  create mode 100644 drivers/scsi/ufs/ufshcd.h
>>
>> --
>> 1.7.5.4
>>
> --
> 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
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html