All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leo Liou <leoliou@google.com>
To: Bean Huo <huobean@gmail.com>
Cc: agross@kernel.org, bjorn.andersson@linaro.org,
	alim.akhtar@samsung.com, avri.altman@wdc.com, jejb@linux.ibm.com,
	martin.petersen@oracle.com, stanley.chu@mediatek.com,
	cang@codeaurora.org, asutoshd@codeaurora.org, beanhuo@micron.com,
	jaegeuk@kernel.org, adrian.hunter@intel.com, satyat@google.com,
	essuuj@gmail.com, linux-arm-msm@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] scsi: ufs: create a hook for unipro dme control
Date: Wed, 10 Feb 2021 00:15:32 +0800	[thread overview]
Message-ID: <YCK1JMsC3D1BqU6Q@google.com> (raw)
In-Reply-To: <4cea12c5c2a1c42ab6c1b96b82489cc59da39517.camel@gmail.com>

On Mon, Feb 08, 2021 at 09:42:05PM +0100, Bean Huo wrote:
> On Mon, 2021-02-08 at 20:56 +0800, Leo Liou wrote:
> > Based on ufshci spec, it defines that "Offset C0h to FFh" belong
> > to vendor specific. If cpu vendor doesn't support these commands, it
> > makes the dme errors:
> > 
> > ufs: dme-set: attr-id 0xd041 val 0x1fff failed 0 retries
> > ufs: dme-set: attr-id 0xd042 val 0xffff failed 0 retries
> > ufs: dme-set: attr-id 0xd043 val 0x7fff failed 0 retries
> > 
> 
> Hi Leo
> "Offset C0h to FFh" registers belong to the UFSHCI, but the attribtes
> you moved belong to "DME Attributes for DME_SET-based High Level Power
> Mode Control" defined in Unipro and doesen't say they are vendor-
> defined attributes. How these two are associated?
> 
> Kind regards,
> Bean
> 
Hi Bean,

Thanks for your remind:)
I thought the vendor means cpu-related, and it's incorrect.
Actually it doesn't make sense.
I'll check the error with unipro layer.

Best Regards,
Leo

> > Create a hook for unipro vendor-specific commands.
> > 
> > Signed-off-by: Leo Liou <leoliou@google.com>
> > ---
> >  drivers/scsi/ufs/ufs-qcom.c | 11 +++++++++++
> >  drivers/scsi/ufs/ufs-qcom.h |  5 +++++
> >  drivers/scsi/ufs/ufshcd.c   |  7 +------
> >  drivers/scsi/ufs/ufshcd.h   |  8 ++++++++
> >  drivers/scsi/ufs/unipro.h   |  4 ----
> >  5 files changed, 25 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-
> > qcom.c
> > index 2206b1e4b774..f2a925587029 100644
> > --- a/drivers/scsi/ufs/ufs-qcom.c
> > +++ b/drivers/scsi/ufs/ufs-qcom.c
> > @@ -759,6 +759,16 @@ static int ufs_qcom_pwr_change_notify(struct
> > ufs_hba *hba,
> >  	return ret;
> >  }
> >  
> > +static void ufs_qcom_unipro_dme_set(struct ufs_hba *hba)
> > +{
> > +	ufshcd_dme_set(hba,
> > UIC_ARG_MIB(DME_LocalFC0ProtectionTimeOutVal),
> > +		       DL_FC0ProtectionTimeOutVal_Default);
> > +	ufshcd_dme_set(hba, UIC_ARG_MIB(DME_LocalTC0ReplayTimeOutVal),
> > +		       DL_TC0ReplayTimeOutVal_Default);
> > +	ufshcd_dme_set(hba, UIC_ARG_MIB(DME_LocalAFC0ReqTimeOutVal),
> > +		       DL_AFC0ReqTimeOutVal_Default);
> > +}
> > +
> >  static int ufs_qcom_quirk_host_pa_saveconfigtime(struct ufs_hba
> > *hba)
> >  {
> >  	int err;
> > @@ -1469,6 +1479,7 @@ static const struct ufs_hba_variant_ops
> > ufs_hba_qcom_vops = {
> >  	.hce_enable_notify      = ufs_qcom_hce_enable_notify,
> >  	.link_startup_notify    = ufs_qcom_link_startup_notify,
> >  	.pwr_change_notify	= ufs_qcom_pwr_change_notify,
> > +	.unipro_dme_set		= ufs_qcom_unipro_dme_set,
> >  	.apply_dev_quirks	= ufs_qcom_apply_dev_quirks,
> >  	.suspend		= ufs_qcom_suspend,
> >  	.resume			= ufs_qcom_resume,
> > diff --git a/drivers/scsi/ufs/ufs-qcom.h b/drivers/scsi/ufs/ufs-
> > qcom.h
> > index 8208e3a3ef59..83db97caaa1b 100644
> > --- a/drivers/scsi/ufs/ufs-qcom.h
> > +++ b/drivers/scsi/ufs/ufs-qcom.h
> > @@ -124,6 +124,11 @@ enum {
> >  /* QUniPro Vendor specific attributes */
> >  #define PA_VS_CONFIG_REG1	0x9000
> >  #define DME_VS_CORE_CLK_CTRL	0xD002
> > +
> > +#define DME_LocalFC0ProtectionTimeOutVal	0xD041
> > +#define DME_LocalTC0ReplayTimeOutVal		0xD042
> > +#define DME_LocalAFC0ReqTimeOutVal		0xD043
> > +
> >  /* bit and mask definitions for DME_VS_CORE_CLK_CTRL attribute */
> >  #define DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT		BIT(8)
> >  #define DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK	0xFF
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index fb32d122f2e3..8ba2ce8c5d0c 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -4231,12 +4231,7 @@ static int ufshcd_change_power_mode(struct
> > ufs_hba *hba,
> >  	ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PWRMODEUSERDATA5),
> >  			DL_AFC1ReqTimeOutVal_Default);
> >  
> > -	ufshcd_dme_set(hba,
> > UIC_ARG_MIB(DME_LocalFC0ProtectionTimeOutVal),
> > -			DL_FC0ProtectionTimeOutVal_Default);
> > -	ufshcd_dme_set(hba, UIC_ARG_MIB(DME_LocalTC0ReplayTimeOutVal),
> > -			DL_TC0ReplayTimeOutVal_Default);
> > -	ufshcd_dme_set(hba, UIC_ARG_MIB(DME_LocalAFC0ReqTimeOutVal),
> > -			DL_AFC0ReqTimeOutVal_Default);
> > +	ufshcd_vops_unipro_dme_set(hba);
> >  
> >  	ret = ufshcd_uic_change_pwr_mode(hba, pwr_mode->pwr_rx << 4
> >  			| pwr_mode->pwr_tx);
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index aa9ea3552323..b8c90bee7503 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -311,6 +311,7 @@ struct ufs_pwr_mode_info {
> >   * @pwr_change_notify: called before and after a power mode change
> >   *			is carried out to allow vendor spesific
> > capabilities
> >   *			to be set.
> > + * @unipro_dme_set: called when vendor speicific control is needed
> >   * @setup_xfer_req: called before any transfer request is issued
> >   *                  to set some things
> >   * @setup_task_mgmt: called before any task management request is
> > issued
> > @@ -342,6 +343,7 @@ struct ufs_hba_variant_ops {
> >  					enum ufs_notify_change_status
> > status,
> >  					struct ufs_pa_layer_attr *,
> >  					struct ufs_pa_layer_attr *);
> > +	void    (*unipro_dme_set)(struct ufs_hba *hba);
> >  	void	(*setup_xfer_req)(struct ufs_hba *, int, bool);
> >  	void	(*setup_task_mgmt)(struct ufs_hba *, int, u8);
> >  	void    (*hibern8_notify)(struct ufs_hba *, enum uic_cmd_dme,
> > @@ -1161,6 +1163,12 @@ static inline int
> > ufshcd_vops_pwr_change_notify(struct ufs_hba *hba,
> >  	return -ENOTSUPP;
> >  }
> >  
> > +static inline void ufshcd_vops_unipro_dme_set(struct ufs_hba *hba)
> > +{
> > +	if (hba->vops && hba->vops->unipro_dme_set)
> > +		return hba->vops->unipro_dme_set(hba);
> > +}
> > +
> >  static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba,
> > int tag,
> >  					bool is_scsi_cmd)
> >  {
> > diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
> > index 8e9e486a4f7b..224162e5afd8 100644
> > --- a/drivers/scsi/ufs/unipro.h
> > +++ b/drivers/scsi/ufs/unipro.h
> > @@ -192,10 +192,6 @@
> >  #define DL_TC1ReplayTimeOutVal_Default		65535
> >  #define DL_AFC1ReqTimeOutVal_Default		32767
> >  
> > -#define DME_LocalFC0ProtectionTimeOutVal	0xD041
> > -#define DME_LocalTC0ReplayTimeOutVal		0xD042
> > -#define DME_LocalAFC0ReqTimeOutVal		0xD043
> > -
> >  /* PA power modes */
> >  enum {
> >  	FAST_MODE	= 1,
> 

      reply	other threads:[~2021-02-09 16:16 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-08 12:56 [PATCH] scsi: ufs: create a hook for unipro dme control Leo Liou
2021-02-08 20:42 ` Bean Huo
2021-02-09 16:15   ` Leo Liou [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YCK1JMsC3D1BqU6Q@google.com \
    --to=leoliou@google.com \
    --cc=adrian.hunter@intel.com \
    --cc=agross@kernel.org \
    --cc=alim.akhtar@samsung.com \
    --cc=asutoshd@codeaurora.org \
    --cc=avri.altman@wdc.com \
    --cc=beanhuo@micron.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=cang@codeaurora.org \
    --cc=essuuj@gmail.com \
    --cc=huobean@gmail.com \
    --cc=jaegeuk@kernel.org \
    --cc=jejb@linux.ibm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=satyat@google.com \
    --cc=stanley.chu@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.