| Message ID | 20200221140812.476338-3-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] ufshcd: remove unused quirks | expand |
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
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.
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>
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.
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. */
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(-)