Thread (7 messages) flat view 7 messages, 3 authors, 2022-10-22

Re: [PATCH v2 3/4] usb: chipidea: usbmisc_imx: Use GENMASK/FIELD_PREP for bitfields

From: Xu Yang <hidden>
Date: 2022-10-18 10:52:41
Also in: linux-usb

On Tue, Oct 18, 2022 at 6:37 PM Sascha Hauer [off-list ref] wrote:
GENMASK and FIELD_PREP offer a convenient unified way to manipulate
bitfields in registers without having to define mask and shift
separately. Broaden the use of this mechanism by converting usbmisc_imx
over to it. No functional change intended.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Link: https://lore.kernel.org/r/20221011082924.884123-4-s.hauer@pengutronix.de (local)
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Xu Yang <xu.yang_2@nxp.com>

Thanks,
Xu Yang
quoted hunk ↗ jump to hunk
---
 drivers/usb/chipidea/usbmisc_imx.c | 116 +++++++++++++++--------------
 1 file changed, 59 insertions(+), 57 deletions(-)
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index e1b4b7f9b3f31..09979264958fc 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -9,24 +9,23 @@
 #include <linux/io.h>
 #include <linux/delay.h>
 #include <linux/usb/otg.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>

 #include "ci_hdrc_imx.h"

 #define MX25_USB_PHY_CTRL_OFFSET       0x08
 #define MX25_BM_EXTERNAL_VBUS_DIVIDER  BIT(23)

-#define MX25_EHCI_INTERFACE_SINGLE_UNI (2 << 0)
-#define MX25_EHCI_INTERFACE_DIFF_UNI   (0 << 0)
-#define MX25_EHCI_INTERFACE_MASK       (0xf)
+#define MX25_EHCI_INTERFACE_SINGLE_UNI 2
+#define MX25_EHCI_INTERFACE_DIFF_UNI   0

-#define MX25_OTG_SIC_SHIFT             29
-#define MX25_OTG_SIC_MASK              (0x3 << MX25_OTG_SIC_SHIFT)
+#define MX25_OTG_SIC                   GENMASK(30, 29)
 #define MX25_OTG_PM_BIT                        BIT(24)
 #define MX25_OTG_PP_BIT                        BIT(11)
 #define MX25_OTG_OCPOL_BIT             BIT(3)

-#define MX25_H1_SIC_SHIFT              21
-#define MX25_H1_SIC_MASK               (0x3 << MX25_H1_SIC_SHIFT)
+#define MX25_H1_SIC                    GENMASK(22, 21)
 #define MX25_H1_PP_BIT                 BIT(18)
 #define MX25_H1_PM_BIT                 BIT(16)
 #define MX25_H1_IPPUE_UP_BIT           BIT(7)
@@ -42,10 +41,10 @@
 #define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08
 #define MX53_USB_OTG_PHY_CTRL_1_OFFSET 0x0c
 #define MX53_USB_CTRL_1_OFFSET         0x10
-#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK (0x3 << 2)
-#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI BIT(2)
-#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK (0x3 << 6)
-#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
+#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL        GENMASK(3, 2)
+#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI 1
+#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL        GENMASK(7, 6)
+#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI 1
 #define MX53_USB_UH2_CTRL_OFFSET       0x14
 #define MX53_USB_UH3_CTRL_OFFSET       0x18
 #define MX53_USB_CLKONOFF_CTRL_OFFSET  0x24
@@ -85,26 +84,24 @@
 #define MX6_USB_OTG1_PHY_CTRL          0x18
 /* For imx6dql, it is host-only controller, for later imx6, it is otg's */
 #define MX6_USB_OTG2_PHY_CTRL          0x1c
-#define MX6SX_USB_VBUS_WAKEUP_SOURCE(v)        (v << 8)
-#define MX6SX_USB_VBUS_WAKEUP_SOURCE_VBUS      MX6SX_USB_VBUS_WAKEUP_SOURCE(0)
-#define MX6SX_USB_VBUS_WAKEUP_SOURCE_AVALID    MX6SX_USB_VBUS_WAKEUP_SOURCE(1)
-#define MX6SX_USB_VBUS_WAKEUP_SOURCE_BVALID    MX6SX_USB_VBUS_WAKEUP_SOURCE(2)
-#define MX6SX_USB_VBUS_WAKEUP_SOURCE_SESS_END  MX6SX_USB_VBUS_WAKEUP_SOURCE(3)
+#define MX6SX_USB_VBUS_WAKEUP_SOURCE           GENMASK(9, 8)
+#define MX6SX_USB_VBUS_WAKEUP_SOURCE_VBUS      0
+#define MX6SX_USB_VBUS_WAKEUP_SOURCE_AVALID    1
+#define MX6SX_USB_VBUS_WAKEUP_SOURCE_BVALID    2
+#define MX6SX_USB_VBUS_WAKEUP_SOURCE_SESS_END  3

 #define VF610_OVER_CUR_DIS             BIT(7)

 #define MX7D_USBNC_USB_CTRL2           0x4
-#define MX7D_USB_VBUS_WAKEUP_SOURCE_MASK       0x3
-#define MX7D_USB_VBUS_WAKEUP_SOURCE(v)         (v << 0)
-#define MX7D_USB_VBUS_WAKEUP_SOURCE_VBUS       MX7D_USB_VBUS_WAKEUP_SOURCE(0)
-#define MX7D_USB_VBUS_WAKEUP_SOURCE_AVALID     MX7D_USB_VBUS_WAKEUP_SOURCE(1)
-#define MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID     MX7D_USB_VBUS_WAKEUP_SOURCE(2)
-#define MX7D_USB_VBUS_WAKEUP_SOURCE_SESS_END   MX7D_USB_VBUS_WAKEUP_SOURCE(3)
+#define MX7D_USB_VBUS_WAKEUP_SOURCE            GENMASK(1, 0)
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_VBUS       0
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_AVALID     1
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID     2
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_SESS_END   3
 #define MX7D_USBNC_AUTO_RESUME                         BIT(2)
 /* The default DM/DP value is pull-down */
-#define MX7D_USBNC_USB_CTRL2_OPMODE(v)                 (v << 6)
-#define MX7D_USBNC_USB_CTRL2_OPMODE_NON_DRIVING        MX7D_USBNC_USB_CTRL2_OPMODE(1)
-#define MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_MASK      (BIT(7) | BIT(6))
+#define MX7D_USBNC_USB_CTRL2_OPMODE            GENMASK(7, 6)
+#define MX7D_USBNC_USB_CTRL2_OPMODE_NON_DRIVING        1
 #define MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_EN                BIT(8)
 #define MX7D_USBNC_USB_CTRL2_DP_OVERRIDE_VAL           BIT(12)
 #define MX7D_USBNC_USB_CTRL2_DP_OVERRIDE_EN            BIT(13)
@@ -129,10 +126,8 @@
 #define MX7D_USB_OTG_PHY_STATUS_CHRGDET                BIT(29)

 #define MX7D_USB_OTG_PHY_CFG1          0x30
-#define TXPREEMPAMPTUNE0_BIT           28
-#define TXPREEMPAMPTUNE0_MASK          (3 << 28)
-#define TXVREFTUNE0_BIT                        20
-#define TXVREFTUNE0_MASK               (0xf << 20)
+#define TXPREEMPAMPTUNE0               GENMASK(29, 28)
+#define TXVREFTUNE0                    GENMASK(23, 20)

 #define MX6_USB_OTG_WAKEUP_BITS (MX6_BM_WAKEUP_ENABLE | MX6_BM_VBUS_WAKEUP | \
                                 MX6_BM_ID_WAKEUP)
@@ -173,8 +168,8 @@ static int usbmisc_imx25_init(struct imx_usbmisc_data *data)
        switch (data->index) {
        case 0:
                val = readl(usbmisc->base);
-               val &= ~(MX25_OTG_SIC_MASK | MX25_OTG_PP_BIT);
-               val |= (MX25_EHCI_INTERFACE_DIFF_UNI & MX25_EHCI_INTERFACE_MASK) << MX25_OTG_SIC_SHIFT;
+               val &= ~(MX25_OTG_SIC | MX25_OTG_PP_BIT);
+               val |= FIELD_PREP(MX25_OTG_SIC, MX25_EHCI_INTERFACE_DIFF_UNI);
                val |= (MX25_OTG_PM_BIT | MX25_OTG_OCPOL_BIT);

                /*
@@ -188,8 +183,8 @@ static int usbmisc_imx25_init(struct imx_usbmisc_data *data)
                break;
        case 1:
                val = readl(usbmisc->base);
-               val &= ~(MX25_H1_SIC_MASK | MX25_H1_PP_BIT |  MX25_H1_IPPUE_UP_BIT);
-               val |= (MX25_EHCI_INTERFACE_SINGLE_UNI & MX25_EHCI_INTERFACE_MASK) << MX25_H1_SIC_SHIFT;
+               val &= ~(MX25_H1_SIC | MX25_H1_PP_BIT |  MX25_H1_IPPUE_UP_BIT);
+               val |= FIELD_PREP(MX25_H1_SIC, MX25_EHCI_INTERFACE_SINGLE_UNI);
                val |= (MX25_H1_PM_BIT | MX25_H1_OCPOL_BIT | MX25_H1_TLL_BIT |
                        MX25_H1_USBTE_BIT | MX25_H1_IPPUE_DOWN_BIT);
@@ -308,8 +303,9 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
                        reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
                        val = readl(reg) | MX53_USB_CTRL_1_UH2_ULPI_EN;
                        /* select ULPI clock */
-                       val &= ~MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK;
-                       val |= MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI;
+                       val &= ~MX53_USB_CTRL_1_H2_XCVR_CLK_SEL;
+                       val |= FIELD_PREP(MX53_USB_CTRL_1_H2_XCVR_CLK_SEL,
+                                         MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI);
                        writel(val, reg);
                        /* Set interrupt wake up enable */
                        reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
@@ -338,8 +334,9 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
                        reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
                        val = readl(reg) | MX53_USB_CTRL_1_UH3_ULPI_EN;
                        /* select ULPI clock */
-                       val &= ~MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK;
-                       val |= MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI;
+                       val &= ~MX53_USB_CTRL_1_H3_XCVR_CLK_SEL;
+                       val |= FIELD_PREP(MX53_USB_CTRL_1_H3_XCVR_CLK_SEL,
+                                         MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI);
                        writel(val, reg);
                        /* Set interrupt wake up enable */
                        reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
@@ -551,8 +548,10 @@ static int usbmisc_imx6sx_init(struct imx_usbmisc_data *data)
                spin_lock_irqsave(&usbmisc->lock, flags);
                /* Set vbus wakeup source as bvalid */
                val = readl(reg);
-               val &= ~MX6SX_USB_VBUS_WAKEUP_SOURCE(3);
-               writel(val | MX6SX_USB_VBUS_WAKEUP_SOURCE_BVALID, reg);
+               val &= ~MX6SX_USB_VBUS_WAKEUP_SOURCE;
+               val |= FIELD_PREP(MX6SX_USB_VBUS_WAKEUP_SOURCE,
+                                 MX6SX_USB_VBUS_WAKEUP_SOURCE_BVALID);
+               writel(val, reg);
                /*
                 * Disable dp/dm wakeup in device mode when vbus is
                 * not there.
@@ -652,22 +651,23 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)

        if (!data->hsic) {
                reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
-               reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK;
-               writel(reg | MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID
-                       | MX7D_USBNC_AUTO_RESUME,
-                       usbmisc->base + MX7D_USBNC_USB_CTRL2);
+               reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE;
+               reg |= FIELD_PREP(MX7D_USB_VBUS_WAKEUP_SOURCE,
+                                 MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID);
+               reg |= MX7D_USBNC_AUTO_RESUME;
+               writel(reg, usbmisc->base + MX7D_USBNC_USB_CTRL2);
                /* PHY tuning for signal quality */
                reg = readl(usbmisc->base + MX7D_USB_OTG_PHY_CFG1);
-               if (data->emp_curr_control && data->emp_curr_control <=
-                       (TXPREEMPAMPTUNE0_MASK >> TXPREEMPAMPTUNE0_BIT)) {
-                       reg &= ~TXPREEMPAMPTUNE0_MASK;
-                       reg |= (data->emp_curr_control << TXPREEMPAMPTUNE0_BIT);
+               if (data->emp_curr_control &&
+                   FIELD_FIT(TXPREEMPAMPTUNE0, data->emp_curr_control)) {
+                       reg &= ~TXPREEMPAMPTUNE0;
+                       reg |= FIELD_PREP(TXPREEMPAMPTUNE0, data->emp_curr_control);
                }

-               if (data->dc_vol_level_adjust && data->dc_vol_level_adjust <=
-                       (TXVREFTUNE0_MASK >> TXVREFTUNE0_BIT)) {
-                       reg &= ~TXVREFTUNE0_MASK;
-                       reg |= (data->dc_vol_level_adjust << TXVREFTUNE0_BIT);
+               if (data->dc_vol_level_adjust &&
+                   FIELD_FIT(TXVREFTUNE0, data->dc_vol_level_adjust)) {
+                       reg &= ~TXVREFTUNE0;
+                       reg |= FIELD_PREP(TXVREFTUNE0, data->dc_vol_level_adjust);
                }

                writel(reg, usbmisc->base + MX7D_USB_OTG_PHY_CFG1);
@@ -742,7 +742,7 @@ static void imx7_disable_charger_detector(struct imx_usbmisc_data *data)

        /* Set OPMODE to be 2'b00 and disable its override */
        val = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
-       val &= ~MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_MASK;
+       val &= ~MX7D_USBNC_USB_CTRL2_OPMODE;
        writel(val, usbmisc->base + MX7D_USBNC_USB_CTRL2);

        val = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
@@ -852,8 +852,9 @@ static int imx7d_charger_detection(struct imx_usbmisc_data *data)
         */
        spin_lock_irqsave(&usbmisc->lock, flags);
        val = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
-       val &= ~MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_MASK;
-       val |= MX7D_USBNC_USB_CTRL2_OPMODE_NON_DRIVING;
+       val &= ~MX7D_USBNC_USB_CTRL2_OPMODE;
+       val |= FIELD_PREP(MX7D_USBNC_USB_CTRL2_OPMODE,
+                         MX7D_USBNC_USB_CTRL2_OPMODE_NON_DRIVING);
        writel(val, usbmisc->base + MX7D_USBNC_USB_CTRL2);

        val = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
@@ -926,9 +927,10 @@ static int usbmisc_imx7ulp_init(struct imx_usbmisc_data *data)
                        usbmisc->base + MX7D_USBNC_USB_CTRL2);
        } else {
                reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
-               reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK;
-               writel(reg | MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID,
-                        usbmisc->base + MX7D_USBNC_USB_CTRL2);
+               reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE;
+               reg |= FIELD_PREP(MX7D_USB_VBUS_WAKEUP_SOURCE,
+                                 MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID);
+               writel(reg, usbmisc->base + MX7D_USBNC_USB_CTRL2);
        }

        spin_unlock_irqrestore(&usbmisc->lock, flags);
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help