On Mon, July 29, 2013, Subhash Jadavani wrote:
> Change looks good except few minor comments.
Thank you for 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 <tgih.jun@xxxxxxxxxxx>
> > ---
> > 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 <linux/async.h>
> >
> > #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.
Ok with above.
>
> > + 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
Ok.
Thanks,
Seungwon Jeon
>
> > * @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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
