From mboxrd@z Thu Jan 1 00:00:00 1970 From: Subhash Jadavani Subject: Re: [PATCH 6/7] scsi: ufs: add operation for the uic power mode change Date: Mon, 29 Jul 2013 15:23:50 +0530 Message-ID: <51F63BAE.4080709@codeaurora.org> References: <1374280885-11526-1-git-send-email-mita@fixstars.com> <002101ce8a06$dac280e0$904782a0$%jun@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org ([198.145.11.231]:35571 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752066Ab3G2Jx4 (ORCPT ); Mon, 29 Jul 2013 05:53:56 -0400 In-Reply-To: <002101ce8a06$dac280e0$904782a0$%jun@samsung.com> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: Seungwon Jeon Cc: linux-scsi@vger.kernel.org, 'Vinayak Holikatti' , 'Santosh Y' , "'James E.J. Bottomley'" Change looks good except few minor comments. On 7/26/2013 7:18 PM, Seungwon Jeon wrote: > Setting PA_PWRMode using DME_SET triggers the power mode > change. And then the result will be given by the HCS.UPMCRS. > This operation should be done atomically. > > Signed-off-by: Seungwon Jeon > --- > drivers/scsi/ufs/ufshcd.c | 80 ++++++++++++++++++++++++++++++++++++++++++-- > drivers/scsi/ufs/ufshcd.h | 3 ++ > drivers/scsi/ufs/ufshci.h | 12 +++++++ > 3 files changed, 91 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 8277c40..ffda72d 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -36,9 +36,11 @@ > #include > > #include "ufshcd.h" > +#include "unipro.h" > > #define UFSHCD_ENABLE_INTRS (UTP_TRANSFER_REQ_COMPL |\ > UTP_TASK_REQ_COMPL |\ > + UIC_POWER_MODE |\ > UFSHCD_ERROR_MASK) > /* UIC command timeout, unit: ms */ > #define UIC_CMD_TIMEOUT 500 > @@ -359,6 +361,18 @@ static inline bool ufshcd_ready_for_uic_cmd(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_dispatch_uic_cmd - Dispatch UIC commands to unipro layers > * @hba: per adapter instance > * @uic_cmd: UIC command > @@ -907,6 +921,60 @@ out: > EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr); > > /** > + * ufshcd_uic_change_pwr_mode - Perform the UIC power mode chage > + * using DME_SET primitives. > + * @hba: per adapter instance > + * @mode: powr mode value > + * > + * Returns 0 on success, non-zero value on failure > + */ > +int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode) > +{ > + struct uic_command uic_cmd = {0}; > + struct completion pwr_done; > + unsigned long flags; > + u8 status; > + int ret; > + > + uic_cmd.command = UIC_CMD_DME_SET; > + uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE); > + uic_cmd.argument3 = mode; > + init_completion(&pwr_done); > + > + mutex_lock(&hba->uic_cmd_mutex); > + > + spin_lock_irqsave(hba->host->host_lock, flags); > + hba->pwr_done = &pwr_done; > + spin_unlock_irqrestore(hba->host->host_lock, flags); > + ret = __ufshcd_send_uic_cmd(hba, &uic_cmd); > + if (ret) { > + dev_err(hba->dev, "pwr mode change uic error %d\n", ret); we should also print the power "mode" which we were trying to set here. > + goto out; > + } > + > + if (!wait_for_completion_timeout(hba->pwr_done, > + msecs_to_jiffies(UIC_CMD_TIMEOUT))) { > + dev_err(hba->dev, "pwr mode change completion timeout\n"); we should also print the power "mode" which we were trying to set here. > + ret = -ETIMEDOUT; > + goto out; > + } > + > + status = ufshcd_get_upmcrs(hba); > + if (status != PWR_LOCAL) { > + dev_err(hba->dev, > + "pwr mode change failed, host umpcrs:0x%x\n", > + status); > + ret = (status != PWR_OK) ? status : -1; > + } > +out: > + spin_lock_irqsave(hba->host->host_lock, flags); > + hba->pwr_done = NULL; > + spin_unlock_irqrestore(hba->host->host_lock, flags); > + mutex_unlock(&hba->uic_cmd_mutex); > + return ret; > +} > + > +/** > * ufshcd_make_hba_operational - Make UFS controller operational > * @hba: per adapter instance > * > @@ -1336,16 +1404,20 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp) > /** > * ufshcd_uic_cmd_compl - handle completion of uic command > * @hba: per adapter instance > + * @intr_status: interrupt status generated by the controller > */ > -static void ufshcd_uic_cmd_compl(struct ufs_hba *hba) > +static void ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status) > { > - if (hba->active_uic_cmd) { > + if ((intr_status & UIC_COMMAND_COMPL) && hba->active_uic_cmd) { > 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); > } > + > + if ((intr_status & UIC_POWER_MODE) && hba->pwr_done) > + complete(hba->pwr_done); > } > > /** > @@ -1447,8 +1519,8 @@ static void ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status) > if (hba->errors) > ufshcd_err_handler(hba); > > - if (intr_status & UIC_COMMAND_COMPL) > - ufshcd_uic_cmd_compl(hba); > + if (intr_status & UFSHCD_UIC_MASK) > + ufshcd_uic_cmd_compl(hba, intr_status); > > if (intr_status & UTP_TASK_REQ_COMPL) > ufshcd_tmc_handler(hba); > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > index 50bcd29..246c0e1 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -142,6 +142,7 @@ struct ufshcd_lrb { > * @uic_cmd_mutex: mutex for uic command > * @ufshcd_tm_wait_queue: wait queue for task management > * @tm_condition: condition variable for task management > + * @pwr_done: completion for powr mode change typo: s/powr/power > * @ufshcd_state: UFSHCD states > * @intr_mask: Interrupt Mask Bits > * @feh_workq: Work queue for fatal controller error handling > @@ -180,6 +181,8 @@ struct ufs_hba { > wait_queue_head_t ufshcd_tm_wait_queue; > unsigned long tm_condition; > > + struct completion *pwr_done; > + > u32 ufshcd_state; > u32 intr_mask; > > diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h > index df4901e..d92fd9c 100644 > --- a/drivers/scsi/ufs/ufshci.h > +++ b/drivers/scsi/ufs/ufshci.h > @@ -124,6 +124,9 @@ enum { > #define CONTROLLER_FATAL_ERROR UFS_BIT(16) > #define SYSTEM_BUS_FATAL_ERROR UFS_BIT(17) > > +#define UFSHCD_UIC_MASK (UIC_COMMAND_COMPL |\ > + UIC_POWER_MODE) > + > #define UFSHCD_ERROR_MASK (UIC_ERROR |\ > DEVICE_FATAL_ERROR |\ > CONTROLLER_FATAL_ERROR |\ > @@ -142,6 +145,15 @@ enum { > #define DEVICE_ERROR_INDICATOR UFS_BIT(5) > #define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK UFS_MASK(0x7, 8) > > +enum { > + PWR_OK = 0x0, > + PWR_LOCAL = 0x01, > + PWR_REMOTE = 0x02, > + PWR_BUSY = 0x03, > + PWR_ERROR_CAP = 0x04, > + PWR_FATAL_ERROR = 0x05, > +}; > + > /* HCE - Host Controller Enable 34h */ > #define CONTROLLER_ENABLE UFS_BIT(0) > #define CONTROLLER_DISABLE 0x0