* ufshcd quirk cleanup v2
@ 2020-02-21 14:08 Christoph Hellwig
2020-02-21 14:08 ` [PATCH 1/2] ufshcd: remove unused quirks Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-02-21 14:08 UTC (permalink / raw)
To: linux-scsi; +Cc: Alim Akhtar, Avri Altman
Hi all,
this removes lots of dead code from the ufs drivers, an cleans
up the quirk handling a little bit.
Changes since v1:
- fix ufshcd_utrl_clear
- minor cleanups
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] ufshcd: remove unused quirks 2020-02-21 14:08 ufshcd quirk cleanup v2 Christoph Hellwig @ 2020-02-21 14:08 ` Christoph Hellwig 2020-02-22 2:46 ` Bart Van Assche 2020-02-22 3:05 ` Alim Akhtar 2020-02-21 14:08 ` [PATCH 2/2] ufshcd: use an enum for quirks Christoph Hellwig 2020-02-29 1:37 ` ufshcd quirk cleanup v2 Martin K. Petersen 2 siblings, 2 replies; 11+ messages in thread From: Christoph Hellwig @ 2020-02-21 14:08 UTC (permalink / raw) To: linux-scsi; +Cc: Alim Akhtar, Avri Altman Remove various quirks that don't have users, as well as the dead code keyed off them. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/scsi/ufs/ufshcd.c | 119 ++++---------------------------------- drivers/scsi/ufs/ufshcd.h | 22 ------- 2 files changed, 12 insertions(+), 129 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index abd0e6b05f79..886a8a597e6a 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -642,11 +642,7 @@ static inline int ufshcd_get_tr_ocs(struct ufshcd_lrb *lrbp) */ static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos) { - if (hba->quirks & UFSHCI_QUIRK_BROKEN_REQ_LIST_CLR) - ufshcd_writel(hba, (1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR); - else - ufshcd_writel(hba, ~(1 << pos), - REG_UTP_TRANSFER_REQ_LIST_CLEAR); + ufshcd_writel(hba, ~(1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR); } /** @@ -656,10 +652,7 @@ static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos) */ static inline void ufshcd_utmrl_clear(struct ufs_hba *hba, u32 pos) { - if (hba->quirks & UFSHCI_QUIRK_BROKEN_REQ_LIST_CLR) - ufshcd_writel(hba, (1 << pos), REG_UTP_TASK_REQ_LIST_CLEAR); - else - ufshcd_writel(hba, ~(1 << pos), REG_UTP_TASK_REQ_LIST_CLEAR); + ufshcd_writel(hba, ~(1 << pos), REG_UTP_TASK_REQ_LIST_CLEAR); } /** @@ -2093,13 +2086,8 @@ static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp) return sg_segments; if (sg_segments) { - if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN) - lrbp->utr_descriptor_ptr->prd_table_length = - cpu_to_le16((u16)(sg_segments * - sizeof(struct ufshcd_sg_entry))); - else - lrbp->utr_descriptor_ptr->prd_table_length = - cpu_to_le16((u16) (sg_segments)); + lrbp->utr_descriptor_ptr->prd_table_length = + cpu_to_le16((u16)sg_segments); prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr; @@ -3403,21 +3391,11 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba) cpu_to_le32(upper_32_bits(cmd_desc_element_addr)); /* Response upiu and prdt offset should be in double words */ - if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN) { - utrdlp[i].response_upiu_offset = - cpu_to_le16(response_offset); - utrdlp[i].prd_table_offset = - cpu_to_le16(prdt_offset); - utrdlp[i].response_upiu_length = - cpu_to_le16(ALIGNED_UPIU_SIZE); - } else { - utrdlp[i].response_upiu_offset = - cpu_to_le16((response_offset >> 2)); - utrdlp[i].prd_table_offset = - cpu_to_le16((prdt_offset >> 2)); - utrdlp[i].response_upiu_length = - cpu_to_le16(ALIGNED_UPIU_SIZE >> 2); - } + utrdlp[i].response_upiu_offset = + cpu_to_le16(response_offset >> 2); + utrdlp[i].prd_table_offset = cpu_to_le16(prdt_offset >> 2); + utrdlp[i].response_upiu_length = + cpu_to_le16(ALIGNED_UPIU_SIZE >> 2); hba->lrb[i].utr_descriptor_ptr = (utrdlp + i); hba->lrb[i].utrd_dma_addr = hba->utrdl_dma_addr + @@ -3460,52 +3438,6 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba) "dme-link-startup: error code %d\n", ret); return ret; } -/** - * ufshcd_dme_reset - UIC command for DME_RESET - * @hba: per adapter instance - * - * DME_RESET command is issued in order to reset UniPro stack. - * This function now deal with cold reset. - * - * Returns 0 on success, non-zero value on failure - */ -static int ufshcd_dme_reset(struct ufs_hba *hba) -{ - struct uic_command uic_cmd = {0}; - int ret; - - uic_cmd.command = UIC_CMD_DME_RESET; - - ret = ufshcd_send_uic_cmd(hba, &uic_cmd); - if (ret) - dev_err(hba->dev, - "dme-reset: error code %d\n", ret); - - return ret; -} - -/** - * ufshcd_dme_enable - UIC command for DME_ENABLE - * @hba: per adapter instance - * - * DME_ENABLE command is issued in order to enable UniPro stack. - * - * Returns 0 on success, non-zero value on failure - */ -static int ufshcd_dme_enable(struct ufs_hba *hba) -{ - struct uic_command uic_cmd = {0}; - int ret; - - uic_cmd.command = UIC_CMD_DME_ENABLE; - - ret = ufshcd_send_uic_cmd(hba, &uic_cmd); - if (ret) - dev_err(hba->dev, - "dme-reset: error code %d\n", ret); - - return ret; -} static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba) { @@ -4217,7 +4149,7 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba, bool can_sleep) } /** - * ufshcd_hba_execute_hce - initialize the controller + * ufshcd_hba_enable - initialize the controller * @hba: per adapter instance * * The controller resets itself and controller firmware initialization @@ -4226,7 +4158,7 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba, bool can_sleep) * * Returns 0 on success, non-zero value on failure */ -static int ufshcd_hba_execute_hce(struct ufs_hba *hba) +int ufshcd_hba_enable(struct ufs_hba *hba) { int retry; @@ -4274,32 +4206,6 @@ static int ufshcd_hba_execute_hce(struct ufs_hba *hba) return 0; } - -int ufshcd_hba_enable(struct ufs_hba *hba) -{ - int ret; - - if (hba->quirks & UFSHCI_QUIRK_BROKEN_HCE) { - ufshcd_set_link_off(hba); - ufshcd_vops_hce_enable_notify(hba, PRE_CHANGE); - - /* enable UIC related interrupts */ - ufshcd_enable_intr(hba, UFSHCD_UIC_MASK); - ret = ufshcd_dme_reset(hba); - if (!ret) { - ret = ufshcd_dme_enable(hba); - if (!ret) - ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); - if (ret) - dev_err(hba->dev, - "Host controller enable failed with non-hce\n"); - } - } else { - ret = ufshcd_hba_execute_hce(hba); - } - - return ret; -} EXPORT_SYMBOL_GPL(ufshcd_hba_enable); static int ufshcd_disable_tx_lcc(struct ufs_hba *hba, bool peer) @@ -4869,8 +4775,7 @@ static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba) * false interrupt if device completes another request after resetting * aggregation and before reading the DB. */ - if (ufshcd_is_intr_aggr_allowed(hba) && - !(hba->quirks & UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR)) + if (ufshcd_is_intr_aggr_allowed(hba)) ufshcd_reset_intr_aggr(hba); tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 2ae6c7c8528c..9c2b80f87b9f 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -612,28 +612,6 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION 0x20 - /* - * This quirk needs to be enabled if the host contoller regards - * resolution of the values of PRDTO and PRDTL in UTRD as byte. - */ - #define UFSHCD_QUIRK_PRDT_BYTE_GRAN 0x80 - - /* - * Clear handling for transfer/task request list is just opposite. - */ - #define UFSHCI_QUIRK_BROKEN_REQ_LIST_CLR 0x100 - - /* - * This quirk needs to be enabled if host controller doesn't allow - * that the interrupt aggregation timer and counter are reset by s/w. - */ - #define UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR 0x200 - - /* - * This quirks needs to be enabled if host controller cannot be - * enabled via HCE register. - */ - #define UFSHCI_QUIRK_BROKEN_HCE 0x400 unsigned int quirks; /* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device spec. */ -- 2.24.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ufshcd: remove unused quirks 2020-02-21 14:08 ` [PATCH 1/2] ufshcd: remove unused quirks Christoph Hellwig @ 2020-02-22 2:46 ` Bart Van Assche 2020-02-22 3:05 ` Alim Akhtar 1 sibling, 0 replies; 11+ messages in thread From: Bart Van Assche @ 2020-02-22 2:46 UTC (permalink / raw) To: Christoph Hellwig, linux-scsi; +Cc: Alim Akhtar, Avri Altman On 2020-02-21 06:08, Christoph Hellwig wrote: > Remove various quirks that don't have users, as well as the dead code > keyed off them. Reviewed-by: Bart Van Assche <bvanassche@acm.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ufshcd: remove unused quirks 2020-02-21 14:08 ` [PATCH 1/2] ufshcd: remove unused quirks Christoph Hellwig 2020-02-22 2:46 ` Bart Van Assche @ 2020-02-22 3:05 ` Alim Akhtar 2020-02-22 12:55 ` Avri Altman 1 sibling, 1 reply; 11+ messages in thread From: Alim Akhtar @ 2020-02-22 3:05 UTC (permalink / raw) To: Christoph Hellwig Cc: linux-scsi, Alim Akhtar, Avri Altman, Kiwoong Kim, Eric Biggers, Martin K. Petersen On Fri, Feb 21, 2020 at 7:38 PM Christoph Hellwig <hch@lst.de> wrote: > > Remove various quirks that don't have users, as well as the dead code > keyed off them. > > Signed-off-by: Christoph Hellwig <hch@lst.de> This does not make sense to remove it now and revert the patch a few week later. Martin / Avri your thought please. > --- > drivers/scsi/ufs/ufshcd.c | 119 ++++---------------------------------- > drivers/scsi/ufs/ufshcd.h | 22 ------- > 2 files changed, 12 insertions(+), 129 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index abd0e6b05f79..886a8a597e6a 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -642,11 +642,7 @@ static inline int ufshcd_get_tr_ocs(struct ufshcd_lrb *lrbp) > */ > static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos) > { > - if (hba->quirks & UFSHCI_QUIRK_BROKEN_REQ_LIST_CLR) > - ufshcd_writel(hba, (1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR); > - else > - ufshcd_writel(hba, ~(1 << pos), > - REG_UTP_TRANSFER_REQ_LIST_CLEAR); > + ufshcd_writel(hba, ~(1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR); > } > > /** > @@ -656,10 +652,7 @@ static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos) > */ > static inline void ufshcd_utmrl_clear(struct ufs_hba *hba, u32 pos) > { > - if (hba->quirks & UFSHCI_QUIRK_BROKEN_REQ_LIST_CLR) > - ufshcd_writel(hba, (1 << pos), REG_UTP_TASK_REQ_LIST_CLEAR); > - else > - ufshcd_writel(hba, ~(1 << pos), REG_UTP_TASK_REQ_LIST_CLEAR); > + ufshcd_writel(hba, ~(1 << pos), REG_UTP_TASK_REQ_LIST_CLEAR); > } > > /** > @@ -2093,13 +2086,8 @@ static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp) > return sg_segments; > > if (sg_segments) { > - if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN) > - lrbp->utr_descriptor_ptr->prd_table_length = > - cpu_to_le16((u16)(sg_segments * > - sizeof(struct ufshcd_sg_entry))); > - else > - lrbp->utr_descriptor_ptr->prd_table_length = > - cpu_to_le16((u16) (sg_segments)); > + lrbp->utr_descriptor_ptr->prd_table_length = > + cpu_to_le16((u16)sg_segments); > > prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr; > > @@ -3403,21 +3391,11 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba) > cpu_to_le32(upper_32_bits(cmd_desc_element_addr)); > > /* Response upiu and prdt offset should be in double words */ > - if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN) { > - utrdlp[i].response_upiu_offset = > - cpu_to_le16(response_offset); > - utrdlp[i].prd_table_offset = > - cpu_to_le16(prdt_offset); > - utrdlp[i].response_upiu_length = > - cpu_to_le16(ALIGNED_UPIU_SIZE); > - } else { > - utrdlp[i].response_upiu_offset = > - cpu_to_le16((response_offset >> 2)); > - utrdlp[i].prd_table_offset = > - cpu_to_le16((prdt_offset >> 2)); > - utrdlp[i].response_upiu_length = > - cpu_to_le16(ALIGNED_UPIU_SIZE >> 2); > - } > + utrdlp[i].response_upiu_offset = > + cpu_to_le16(response_offset >> 2); > + utrdlp[i].prd_table_offset = cpu_to_le16(prdt_offset >> 2); > + utrdlp[i].response_upiu_length = > + cpu_to_le16(ALIGNED_UPIU_SIZE >> 2); > > hba->lrb[i].utr_descriptor_ptr = (utrdlp + i); > hba->lrb[i].utrd_dma_addr = hba->utrdl_dma_addr + > @@ -3460,52 +3438,6 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba) > "dme-link-startup: error code %d\n", ret); > return ret; > } > -/** > - * ufshcd_dme_reset - UIC command for DME_RESET > - * @hba: per adapter instance > - * > - * DME_RESET command is issued in order to reset UniPro stack. > - * This function now deal with cold reset. > - * > - * Returns 0 on success, non-zero value on failure > - */ > -static int ufshcd_dme_reset(struct ufs_hba *hba) > -{ > - struct uic_command uic_cmd = {0}; > - int ret; > - > - uic_cmd.command = UIC_CMD_DME_RESET; > - > - ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > - if (ret) > - dev_err(hba->dev, > - "dme-reset: error code %d\n", ret); > - > - return ret; > -} > - > -/** > - * ufshcd_dme_enable - UIC command for DME_ENABLE > - * @hba: per adapter instance > - * > - * DME_ENABLE command is issued in order to enable UniPro stack. > - * > - * Returns 0 on success, non-zero value on failure > - */ > -static int ufshcd_dme_enable(struct ufs_hba *hba) > -{ > - struct uic_command uic_cmd = {0}; > - int ret; > - > - uic_cmd.command = UIC_CMD_DME_ENABLE; > - > - ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > - if (ret) > - dev_err(hba->dev, > - "dme-reset: error code %d\n", ret); > - > - return ret; > -} > > static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba) > { > @@ -4217,7 +4149,7 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba, bool can_sleep) > } > > /** > - * ufshcd_hba_execute_hce - initialize the controller > + * ufshcd_hba_enable - initialize the controller > * @hba: per adapter instance > * > * The controller resets itself and controller firmware initialization > @@ -4226,7 +4158,7 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba, bool can_sleep) > * > * Returns 0 on success, non-zero value on failure > */ > -static int ufshcd_hba_execute_hce(struct ufs_hba *hba) > +int ufshcd_hba_enable(struct ufs_hba *hba) > { > int retry; > > @@ -4274,32 +4206,6 @@ static int ufshcd_hba_execute_hce(struct ufs_hba *hba) > > return 0; > } > - > -int ufshcd_hba_enable(struct ufs_hba *hba) > -{ > - int ret; > - > - if (hba->quirks & UFSHCI_QUIRK_BROKEN_HCE) { > - ufshcd_set_link_off(hba); > - ufshcd_vops_hce_enable_notify(hba, PRE_CHANGE); > - > - /* enable UIC related interrupts */ > - ufshcd_enable_intr(hba, UFSHCD_UIC_MASK); > - ret = ufshcd_dme_reset(hba); > - if (!ret) { > - ret = ufshcd_dme_enable(hba); > - if (!ret) > - ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); > - if (ret) > - dev_err(hba->dev, > - "Host controller enable failed with non-hce\n"); > - } > - } else { > - ret = ufshcd_hba_execute_hce(hba); > - } > - > - return ret; > -} > EXPORT_SYMBOL_GPL(ufshcd_hba_enable); > > static int ufshcd_disable_tx_lcc(struct ufs_hba *hba, bool peer) > @@ -4869,8 +4775,7 @@ static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba) > * false interrupt if device completes another request after resetting > * aggregation and before reading the DB. > */ > - if (ufshcd_is_intr_aggr_allowed(hba) && > - !(hba->quirks & UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR)) > + if (ufshcd_is_intr_aggr_allowed(hba)) > ufshcd_reset_intr_aggr(hba); > > tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > index 2ae6c7c8528c..9c2b80f87b9f 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -612,28 +612,6 @@ struct ufs_hba { > */ > #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION 0x20 > > - /* > - * This quirk needs to be enabled if the host contoller regards > - * resolution of the values of PRDTO and PRDTL in UTRD as byte. > - */ > - #define UFSHCD_QUIRK_PRDT_BYTE_GRAN 0x80 > - > - /* > - * Clear handling for transfer/task request list is just opposite. > - */ > - #define UFSHCI_QUIRK_BROKEN_REQ_LIST_CLR 0x100 > - > - /* > - * This quirk needs to be enabled if host controller doesn't allow > - * that the interrupt aggregation timer and counter are reset by s/w. > - */ > - #define UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR 0x200 > - > - /* > - * This quirks needs to be enabled if host controller cannot be > - * enabled via HCE register. > - */ > - #define UFSHCI_QUIRK_BROKEN_HCE 0x400 > unsigned int quirks; /* Deviations from standard UFSHCI spec. */ > > /* Device deviations from standard UFS device spec. */ > -- > 2.24.1 > -- Regards, Alim ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/2] ufshcd: remove unused quirks 2020-02-22 3:05 ` Alim Akhtar @ 2020-02-22 12:55 ` Avri Altman 0 siblings, 0 replies; 11+ messages in thread From: Avri Altman @ 2020-02-22 12:55 UTC (permalink / raw) To: Alim Akhtar, Christoph Hellwig Cc: linux-scsi, Alim Akhtar, Kiwoong Kim, Eric Biggers, Martin K. Petersen > > On Fri, Feb 21, 2020 at 7:38 PM Christoph Hellwig <hch@lst.de> wrote: > > > > Remove various quirks that don't have users, as well as the dead code > > keyed off them. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > This does not make sense to remove it now and revert the patch a few week > later. > Martin / Avri your thought please. Yeah - I agree with you, provided that indeed it is just few weeks. Thanks, Avri ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ufshcd: use an enum for quirks 2020-02-21 14:08 ufshcd quirk cleanup v2 Christoph Hellwig 2020-02-21 14:08 ` [PATCH 1/2] ufshcd: remove unused quirks Christoph Hellwig @ 2020-02-21 14:08 ` Christoph Hellwig 2020-02-21 18:18 ` Asutosh Das (asd) 2020-02-22 2:46 ` Bart Van Assche 2020-02-29 1:37 ` ufshcd quirk cleanup v2 Martin K. Petersen 2 siblings, 2 replies; 11+ messages in thread From: Christoph Hellwig @ 2020-02-21 14:08 UTC (permalink / raw) To: linux-scsi; +Cc: Alim Akhtar, Avri Altman Use an enum to specify the various quirks instead of #defines inside the structure definition. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/scsi/ufs/ufshcd.h | 82 ++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 9c2b80f87b9f..f68b3cd2e465 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -469,6 +469,48 @@ struct ufs_stats { struct ufs_err_reg_hist task_abort; }; +enum ufshcd_quirks { + /* Interrupt aggregation support is broken */ + UFSHCD_QUIRK_BROKEN_INTR_AGGR = 1 << 0, + + /* + * delay before each dme command is required as the unipro + * layer has shown instabilities + */ + UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS = 1 << 1, + + /* + * If UFS host controller is having issue in processing LCC (Line + * Control Command) coming from device then enable this quirk. + * When this quirk is enabled, host controller driver should disable + * the LCC transmission on UFS device (by clearing TX_LCC_ENABLE + * attribute of device to 0). + */ + UFSHCD_QUIRK_BROKEN_LCC = 1 << 2, + + /* + * The attribute PA_RXHSUNTERMCAP specifies whether or not the + * inbound Link supports unterminated line in HS mode. Setting this + * attribute to 1 fixes moving to HS gear. + */ + UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP = 1 << 3, + + /* + * This quirk needs to be enabled if the host contoller only allows + * accessing the peer dme attributes in AUTO mode (FAST AUTO or + * SLOW AUTO). + */ + UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE = 1 << 4, + + /* + * This quirk needs to be enabled if the host contoller doesn't + * advertise the correct version in UFS_VER register. If this quirk + * is enabled, standard UFS host driver will call the vendor specific + * ops (get_ufs_hci_version) to get the correct version. + */ + UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION = 1 << 5, +}; + /** * struct ufs_hba - per adapter private structure * @mmio_base: UFSHCI base register address @@ -572,46 +614,6 @@ struct ufs_hba { bool is_irq_enabled; enum ufs_ref_clk_freq dev_ref_clk_freq; - /* Interrupt aggregation support is broken */ - #define UFSHCD_QUIRK_BROKEN_INTR_AGGR 0x1 - - /* - * delay before each dme command is required as the unipro - * layer has shown instabilities - */ - #define UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS 0x2 - - /* - * If UFS host controller is having issue in processing LCC (Line - * Control Command) coming from device then enable this quirk. - * When this quirk is enabled, host controller driver should disable - * the LCC transmission on UFS device (by clearing TX_LCC_ENABLE - * attribute of device to 0). - */ - #define UFSHCD_QUIRK_BROKEN_LCC 0x4 - - /* - * The attribute PA_RXHSUNTERMCAP specifies whether or not the - * inbound Link supports unterminated line in HS mode. Setting this - * attribute to 1 fixes moving to HS gear. - */ - #define UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP 0x8 - - /* - * This quirk needs to be enabled if the host contoller only allows - * accessing the peer dme attributes in AUTO mode (FAST AUTO or - * SLOW AUTO). - */ - #define UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE 0x10 - - /* - * This quirk needs to be enabled if the host contoller doesn't - * advertise the correct version in UFS_VER register. If this quirk - * is enabled, standard UFS host driver will call the vendor specific - * ops (get_ufs_hci_version) to get the correct version. - */ - #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION 0x20 - unsigned int quirks; /* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device spec. */ -- 2.24.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ufshcd: use an enum for quirks 2020-02-21 14:08 ` [PATCH 2/2] ufshcd: use an enum for quirks Christoph Hellwig @ 2020-02-21 18:18 ` Asutosh Das (asd) 2020-02-22 2:45 ` Bart Van Assche 2020-02-22 2:46 ` Bart Van Assche 1 sibling, 1 reply; 11+ messages in thread From: Asutosh Das (asd) @ 2020-02-21 18:18 UTC (permalink / raw) To: Christoph Hellwig, linux-scsi; +Cc: Alim Akhtar, Avri Altman Hi Christoph On 2/21/2020 6:08 AM, Christoph Hellwig wrote: > Use an enum to specify the various quirks instead of #defines inside > the structure definition. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/scsi/ufs/ufshcd.h | 82 ++++++++++++++++++++------------------- > 1 file changed, 42 insertions(+), 40 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > index 9c2b80f87b9f..f68b3cd2e465 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -469,6 +469,48 @@ struct ufs_stats { > struct ufs_err_reg_hist task_abort; > }; > > +enum ufshcd_quirks { > + /* Interrupt aggregation support is broken */ > + UFSHCD_QUIRK_BROKEN_INTR_AGGR = 1 << 0, > + How about using BIT() here? > + /* > + * delay before each dme command is required as the unipro > + * layer has shown instabilities > + */ > + UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS = 1 << 1, > + > + /* > + * If UFS host controller is having issue in processing LCC (Line > + * Control Command) coming from device then enable this quirk. > + * When this quirk is enabled, host controller driver should disable > + * the LCC transmission on UFS device (by clearing TX_LCC_ENABLE > + * attribute of device to 0). > + */ > + UFSHCD_QUIRK_BROKEN_LCC = 1 << 2, > + > + /* > + * The attribute PA_RXHSUNTERMCAP specifies whether or not the > + * inbound Link supports unterminated line in HS mode. Setting this > + * attribute to 1 fixes moving to HS gear. > + */ > + UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP = 1 << 3, > + > + /* > + * This quirk needs to be enabled if the host contoller only allows > + * accessing the peer dme attributes in AUTO mode (FAST AUTO or > + * SLOW AUTO). > + */ > + UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE = 1 << 4, > + > + /* > + * This quirk needs to be enabled if the host contoller doesn't > + * advertise the correct version in UFS_VER register. If this quirk > + * is enabled, standard UFS host driver will call the vendor specific > + * ops (get_ufs_hci_version) to get the correct version. > + */ > + UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION = 1 << 5, > +}; > + > /** > * struct ufs_hba - per adapter private structure > * @mmio_base: UFSHCI base register address > @@ -572,46 +614,6 @@ struct ufs_hba { > bool is_irq_enabled; > enum ufs_ref_clk_freq dev_ref_clk_freq; > > - /* Interrupt aggregation support is broken */ > - #define UFSHCD_QUIRK_BROKEN_INTR_AGGR 0x1 > - > - /* > - * delay before each dme command is required as the unipro > - * layer has shown instabilities > - */ > - #define UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS 0x2 > - > - /* > - * If UFS host controller is having issue in processing LCC (Line > - * Control Command) coming from device then enable this quirk. > - * When this quirk is enabled, host controller driver should disable > - * the LCC transmission on UFS device (by clearing TX_LCC_ENABLE > - * attribute of device to 0). > - */ > - #define UFSHCD_QUIRK_BROKEN_LCC 0x4 > - > - /* > - * The attribute PA_RXHSUNTERMCAP specifies whether or not the > - * inbound Link supports unterminated line in HS mode. Setting this > - * attribute to 1 fixes moving to HS gear. > - */ > - #define UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP 0x8 > - > - /* > - * This quirk needs to be enabled if the host contoller only allows > - * accessing the peer dme attributes in AUTO mode (FAST AUTO or > - * SLOW AUTO). > - */ > - #define UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE 0x10 > - > - /* > - * This quirk needs to be enabled if the host contoller doesn't > - * advertise the correct version in UFS_VER register. If this quirk > - * is enabled, standard UFS host driver will call the vendor specific > - * ops (get_ufs_hci_version) to get the correct version. > - */ > - #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION 0x20 > - > unsigned int quirks; /* Deviations from standard UFSHCI spec. */ > > /* Device deviations from standard UFS device spec. */ > -asd -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ufshcd: use an enum for quirks 2020-02-21 18:18 ` Asutosh Das (asd) @ 2020-02-22 2:45 ` Bart Van Assche 2020-02-24 17:16 ` Christoph Hellwig 0 siblings, 1 reply; 11+ messages in thread From: Bart Van Assche @ 2020-02-22 2:45 UTC (permalink / raw) To: Asutosh Das (asd), Christoph Hellwig, linux-scsi; +Cc: Alim Akhtar, Avri Altman On 2020-02-21 10:18, Asutosh Das (asd) wrote: > On 2/21/2020 6:08 AM, Christoph Hellwig wrote: >> + /* Interrupt aggregation support is broken */ >> + UFSHCD_QUIRK_BROKEN_INTR_AGGR = 1 << 0, >> + > > How about using BIT() here? Not everyone is convinced that using BIT() improves code readability. Bart. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ufshcd: use an enum for quirks 2020-02-22 2:45 ` Bart Van Assche @ 2020-02-24 17:16 ` Christoph Hellwig 0 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2020-02-24 17:16 UTC (permalink / raw) To: Bart Van Assche Cc: Asutosh Das (asd), Christoph Hellwig, linux-scsi, Alim Akhtar, Avri Altman On Fri, Feb 21, 2020 at 06:45:37PM -0800, Bart Van Assche wrote: > On 2020-02-21 10:18, Asutosh Das (asd) wrote: > > On 2/21/2020 6:08 AM, Christoph Hellwig wrote: > >> + /* Interrupt aggregation support is broken */ > >> + UFSHCD_QUIRK_BROKEN_INTR_AGGR = 1 << 0, > >> + > > > > How about using BIT() here? > > Not everyone is convinced that using BIT() improves code readability. I for one am not. 1 << N shoud be obvious to anyone with a basic understanding of C code, BIT() needs to be looked up. And it isn't actually any shorter. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ufshcd: use an enum for quirks 2020-02-21 14:08 ` [PATCH 2/2] ufshcd: use an enum for quirks Christoph Hellwig 2020-02-21 18:18 ` Asutosh Das (asd) @ 2020-02-22 2:46 ` Bart Van Assche 1 sibling, 0 replies; 11+ messages in thread From: Bart Van Assche @ 2020-02-22 2:46 UTC (permalink / raw) To: Christoph Hellwig, linux-scsi; +Cc: Alim Akhtar, Avri Altman On 2020-02-21 06:08, Christoph Hellwig wrote: > Use an enum to specify the various quirks instead of #defines inside > the structure definition. Reviewed-by: Bart Van Assche <bvanassche@acm.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ufshcd quirk cleanup v2 2020-02-21 14:08 ufshcd quirk cleanup v2 Christoph Hellwig 2020-02-21 14:08 ` [PATCH 1/2] ufshcd: remove unused quirks Christoph Hellwig 2020-02-21 14:08 ` [PATCH 2/2] ufshcd: use an enum for quirks Christoph Hellwig @ 2020-02-29 1:37 ` Martin K. Petersen 2 siblings, 0 replies; 11+ messages in thread From: Martin K. Petersen @ 2020-02-29 1:37 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-scsi, Alim Akhtar, Avri Altman Christoph, > this removes lots of dead code from the ufs drivers, an cleans up the > quirk handling a little bit. Applied to 5.7/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-02-29 1:37 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-21 14:08 ufshcd quirk cleanup v2 Christoph Hellwig 2020-02-21 14:08 ` [PATCH 1/2] ufshcd: remove unused quirks Christoph Hellwig 2020-02-22 2:46 ` Bart Van Assche 2020-02-22 3:05 ` Alim Akhtar 2020-02-22 12:55 ` Avri Altman 2020-02-21 14:08 ` [PATCH 2/2] ufshcd: use an enum for quirks Christoph Hellwig 2020-02-21 18:18 ` Asutosh Das (asd) 2020-02-22 2:45 ` Bart Van Assche 2020-02-24 17:16 ` Christoph Hellwig 2020-02-22 2:46 ` Bart Van Assche 2020-02-29 1:37 ` ufshcd quirk cleanup v2 Martin K. Petersen
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.