2021-02-08 13:00:05

by Leo Liou

[permalink] [raw]
Subject: [PATCH] scsi: ufs: create a hook for unipro dme control

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

Create a hook for unipro vendor-specific commands.

Signed-off-by: Leo Liou <[email protected]>
---
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,
--
2.30.0.478.g8a0d178c01-goog


2021-02-08 21:35:52

by Bean Huo

[permalink] [raw]
Subject: Re: [PATCH] scsi: ufs: create a hook for unipro dme control

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

> Create a hook for unipro vendor-specific commands.
>
> Signed-off-by: Leo Liou <[email protected]>
> ---
> 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,

2021-02-09 16:21:50

by Leo Liou

[permalink] [raw]
Subject: Re: [PATCH] scsi: ufs: create a hook for unipro dme control

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 <[email protected]>
> > ---
> > 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,
>