linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree
@ 2022-10-11  8:29 Sascha Hauer
  2022-10-11  8:29 ` [PATCH 1/6] usb: chipidea: usbmisc_imx: Fix i.MX53 clock sel masks Sascha Hauer
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Sascha Hauer @ 2022-10-11  8:29 UTC (permalink / raw)
  To: linux-usb
  Cc: linux-arm-kernel, Peter Chen, Peng Fan, Pengutronix Kernel Team,
	devicetree, Sascha Hauer

This series exports more phy tuning settings to device tree. There are
some values already exported and I follow that example in this series.

Patches 1 and 2 contain two small bugfixes for issues I stumbled upon
along the way. Patches 3 and 4 are cleanups. These patches could be
applied without the remaining two patches.

The binding patch is based on
https://lore.kernel.org/linux-arm-kernel/20221010101816.298334-3-peng.fan@oss.nxp.com/t/,
so this will need a rebase once this series settles.

Sascha

Sascha Hauer (6):
  usb: chipidea: usbmisc_imx: Fix i.MX53 clock sel masks
  usb: chipidea: usbmisc_imx: Fix setting i.MX6SX wakeup source
  usb: chipidea: usbmisc_imx: Use GENMASK/FIELD_PREP for bitfields
  usb: chipidea: usbmisc_imx: Add prefix to register defines
  usb: chipidea: usbmisc_imx: Add device tree properties for i.MX7 phy
    tuning
  dt-bindings: usb: ci-hdrc-usb2: Add more phy tuning properties

 .../devicetree/bindings/usb/ci-hdrc-usb2.yaml |  79 ++++++++
 drivers/usb/chipidea/ci_hdrc_imx.c            |  14 ++
 drivers/usb/chipidea/ci_hdrc_imx.h            |   7 +
 drivers/usb/chipidea/usbmisc_imx.c            | 186 ++++++++++++------
 4 files changed, 230 insertions(+), 56 deletions(-)

-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/6] usb: chipidea: usbmisc_imx: Fix i.MX53 clock sel masks
  2022-10-11  8:29 [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree Sascha Hauer
@ 2022-10-11  8:29 ` Sascha Hauer
  2022-10-18  8:45   ` Xu Yang
  2022-10-11  8:29 ` [PATCH 2/6] usb: chipidea: usbmisc_imx: Fix setting i.MX6SX wakeup source Sascha Hauer
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2022-10-11  8:29 UTC (permalink / raw)
  To: linux-usb
  Cc: linux-arm-kernel, Peter Chen, Peng Fan, Pengutronix Kernel Team,
	devicetree, Sascha Hauer

According to the reference manual the masks for the
MX53_USB_CTRL_1_H*_XCVR_CLK_SEL bits are 0x3, not 0x11 (which were
probably meant as 0b11).

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/chipidea/usbmisc_imx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index bac0f5458cab9..8f805aa9c383c 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -42,9 +42,9 @@
 #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 (0x11 << 2)
+#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 (0x11 << 6)
+#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_UH2_CTRL_OFFSET	0x14
 #define MX53_USB_UH3_CTRL_OFFSET	0x18
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/6] usb: chipidea: usbmisc_imx: Fix setting i.MX6SX wakeup source
  2022-10-11  8:29 [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree Sascha Hauer
  2022-10-11  8:29 ` [PATCH 1/6] usb: chipidea: usbmisc_imx: Fix i.MX53 clock sel masks Sascha Hauer
@ 2022-10-11  8:29 ` Sascha Hauer
  2022-10-18  8:46   ` Xu Yang
  2022-10-11  8:29 ` [PATCH 3/6] usb: chipidea: usbmisc_imx: Use GENMASK/FIELD_PREP for bitfields Sascha Hauer
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2022-10-11  8:29 UTC (permalink / raw)
  To: linux-usb
  Cc: linux-arm-kernel, Peter Chen, Peng Fan, Pengutronix Kernel Team,
	devicetree, Sascha Hauer

MX6SX_USB_VBUS_WAKEUP_SOURCE are two bits containing an enum value,
so when read/modify/write that we have to clear both bits bits before
setting the desired bits. The clearing of the bits was forgotten, add
it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/chipidea/usbmisc_imx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 8f805aa9c383c..e1b4b7f9b3f31 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -551,6 +551,7 @@ 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);
 		/*
 		 * Disable dp/dm wakeup in device mode when vbus is
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/6] usb: chipidea: usbmisc_imx: Use GENMASK/FIELD_PREP for bitfields
  2022-10-11  8:29 [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree Sascha Hauer
  2022-10-11  8:29 ` [PATCH 1/6] usb: chipidea: usbmisc_imx: Fix i.MX53 clock sel masks Sascha Hauer
  2022-10-11  8:29 ` [PATCH 2/6] usb: chipidea: usbmisc_imx: Fix setting i.MX6SX wakeup source Sascha Hauer
@ 2022-10-11  8:29 ` Sascha Hauer
  2022-10-18  8:39   ` xu yang
  2022-10-11  8:29 ` [PATCH 4/6] usb: chipidea: usbmisc_imx: Add prefix to register defines Sascha Hauer
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2022-10-11  8:29 UTC (permalink / raw)
  To: linux-usb
  Cc: linux-arm-kernel, Peter Chen, Peng Fan, Pengutronix Kernel Team,
	devicetree, Sascha Hauer

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>
---
 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..95f2ba01c0df1 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(5, 4)
+#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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/6] usb: chipidea: usbmisc_imx: Add prefix to register defines
  2022-10-11  8:29 [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree Sascha Hauer
                   ` (2 preceding siblings ...)
  2022-10-11  8:29 ` [PATCH 3/6] usb: chipidea: usbmisc_imx: Use GENMASK/FIELD_PREP for bitfields Sascha Hauer
@ 2022-10-11  8:29 ` Sascha Hauer
  2022-10-18  8:49   ` Xu Yang
  2022-10-11  8:29 ` [PATCH 5/6] usb: chipidea: usbmisc_imx: Add device tree properties for i.MX7 phy tuning Sascha Hauer
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2022-10-11  8:29 UTC (permalink / raw)
  To: linux-usb
  Cc: linux-arm-kernel, Peter Chen, Peng Fan, Pengutronix Kernel Team,
	devicetree, Sascha Hauer

The driver is used for a broad range of i.MX SoCs and most of the
register defines have a SoC/regname specific prefix to make clear
in which context they should be used. Add such a prefix to the
MX7D_USB_OTG_PHY_CFG1 defines as well.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/chipidea/usbmisc_imx.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 95f2ba01c0df1..63de7d6fea427 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -126,8 +126,8 @@
 #define MX7D_USB_OTG_PHY_STATUS_CHRGDET		BIT(29)
 
 #define MX7D_USB_OTG_PHY_CFG1		0x30
-#define TXPREEMPAMPTUNE0		GENMASK(29, 28)
-#define TXVREFTUNE0			GENMASK(23, 20)
+#define MX7D_USB_OTG_PHY_CFG1_TXPREEMPAMPTUNE0	GENMASK(29, 28)
+#define MX7D_USB_OTG_PHY_CFG1_TXVREFTUNE0	GENMASK(23, 20)
 
 #define MX6_USB_OTG_WAKEUP_BITS (MX6_BM_WAKEUP_ENABLE | MX6_BM_VBUS_WAKEUP | \
 				 MX6_BM_ID_WAKEUP)
@@ -659,15 +659,19 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
 		/* PHY tuning for signal quality */
 		reg = readl(usbmisc->base + MX7D_USB_OTG_PHY_CFG1);
 		if (data->emp_curr_control &&
-		    FIELD_FIT(TXPREEMPAMPTUNE0, data->emp_curr_control)) {
-			reg &= ~TXPREEMPAMPTUNE0;
-			reg |= FIELD_PREP(TXPREEMPAMPTUNE0, data->emp_curr_control);
+		    FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_TXPREEMPAMPTUNE0,
+			      data->emp_curr_control)) {
+			reg &= ~MX7D_USB_OTG_PHY_CFG1_TXPREEMPAMPTUNE0;
+			reg |= FIELD_PREP(MX7D_USB_OTG_PHY_CFG1_TXPREEMPAMPTUNE0,
+					  data->emp_curr_control);
 		}
 
 		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);
+		    FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_TXVREFTUNE0,
+			      data->dc_vol_level_adjust)) {
+			reg &= ~MX7D_USB_OTG_PHY_CFG1_TXVREFTUNE0;
+			reg |= FIELD_PREP(MX7D_USB_OTG_PHY_CFG1_TXVREFTUNE0,
+					  data->dc_vol_level_adjust);
 		}
 
 		writel(reg, usbmisc->base + MX7D_USB_OTG_PHY_CFG1);
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/6] usb: chipidea: usbmisc_imx: Add device tree properties for i.MX7 phy tuning
  2022-10-11  8:29 [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree Sascha Hauer
                   ` (3 preceding siblings ...)
  2022-10-11  8:29 ` [PATCH 4/6] usb: chipidea: usbmisc_imx: Add prefix to register defines Sascha Hauer
@ 2022-10-11  8:29 ` Sascha Hauer
  2022-10-11  8:29 ` [PATCH 6/6] dt-bindings: usb: ci-hdrc-usb2: Add more phy tuning properties Sascha Hauer
  2022-10-17  0:41 ` [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree Peter Chen
  6 siblings, 0 replies; 15+ messages in thread
From: Sascha Hauer @ 2022-10-11  8:29 UTC (permalink / raw)
  To: linux-usb
  Cc: linux-arm-kernel, Peter Chen, Peng Fan, Pengutronix Kernel Team,
	devicetree, Sascha Hauer

This adds support for configuring the remaining phy tuning options
from device tree. Some properties are already configurable, the
remaining properties are added following the same pattern.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 14 ++++++
 drivers/usb/chipidea/ci_hdrc_imx.h |  7 +++
 drivers/usb/chipidea/usbmisc_imx.c | 71 +++++++++++++++++++++++++++++-
 3 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 9ffcecd3058c1..a7c8c0065b9b7 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -172,8 +172,22 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
 
 	of_property_read_u32(np, "samsung,picophy-pre-emp-curr-control",
 			&data->emp_curr_control);
+	of_property_read_u32(np, "samsung,picophy-usb-source-impedance-adjust",
+			&data->usb_source_impedance_adjust);
+	of_property_read_u32(np, "samsung,picophy-hs-rise-time-adjust",
+			&data->hs_transmitter_rise_time_adjust);
 	of_property_read_u32(np, "samsung,picophy-dc-vol-level-adjust",
 			&data->dc_vol_level_adjust);
+	of_property_read_u32(np, "samsung,picophy-fs-ls-source-impedance-adjust",
+			&data->fs_ls_source_impedance_adjust);
+	of_property_read_u32(np, "samsung,picophy-transmitter-hs-crossover-adjust",
+			&data->transmitter_hs_crossover_adjust);
+	of_property_read_u32(np, "samsung,picophy-vbus-valid-threshold-adjust",
+			&data->vbus_valid_threshold_adjust);
+	of_property_read_u32(np, "samsung,picophy-squelsh-threshold-adjust",
+			&data->squelsh_threshold_adjust);
+	of_property_read_u32(np, "samsung,picophy-disconnect-threshold-adjust",
+			&data->disconnect_threshold_adjust);
 
 	return data;
 }
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
index 7daccb9c5006a..c38f4746d6903 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -27,7 +27,14 @@ struct imx_usbmisc_data {
 	struct usb_phy *usb_phy;
 	enum usb_dr_mode available_role; /* runtime usb dr mode */
 	int emp_curr_control;
+	int hs_transmitter_rise_time_adjust;
 	int dc_vol_level_adjust;
+	int usb_source_impedance_adjust;
+	int fs_ls_source_impedance_adjust;
+	int transmitter_hs_crossover_adjust;
+	int vbus_valid_threshold_adjust;
+	int squelsh_threshold_adjust;
+	int disconnect_threshold_adjust;
 };
 
 int imx_usbmisc_init(struct imx_usbmisc_data *data);
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 63de7d6fea427..23dea390bf99b 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -126,8 +126,19 @@
 #define MX7D_USB_OTG_PHY_STATUS_CHRGDET		BIT(29)
 
 #define MX7D_USB_OTG_PHY_CFG1		0x30
-#define MX7D_USB_OTG_PHY_CFG1_TXPREEMPAMPTUNE0	GENMASK(29, 28)
-#define MX7D_USB_OTG_PHY_CFG1_TXVREFTUNE0	GENMASK(23, 20)
+#define MX7D_USB_OTG_PHY_CFG1_CHRGDET_MEGAMIX		BIT(31)
+#define MX7D_USB_OTG_PHY_CFG1_TXPREEMPPULSETUNE0	BIT(30)
+#define MX7D_USB_OTG_PHY_CFG1_TXPREEMPAMPTUNE0		GENMASK(29, 28)
+#define MX7D_USB_OTG_PHY_CFG1_TXRESTUNE0		GENMASK(27, 26)
+#define MX7D_USB_OTG_PHY_CFG1_TXRISETUNE0		GENMASK(25, 24)
+#define MX7D_USB_OTG_PHY_CFG1_TXVREFTUNE0		GENMASK(23, 20)
+#define MX7D_USB_OTG_PHY_CFG1_TXFSLSTUNE0		GENMASK(19, 16)
+#define MX7D_USB_OTG_PHY_CFG1_TXHSXVTUNE0		GENMASK(14, 13)
+#define MX7D_USB_OTG_PHY_CFG1_OTGTUNE0			GENMASK(12, 10)
+#define MX7D_USB_OTG_PHY_CFG1_SQRTUNE0			GENMASK(9, 7)
+#define MX7D_USB_OTG_PHY_CFG1_COMPDISTUNE0		GENMASK(6, 4)
+#define MX7D_USB_OTG_PHY_CFG1_FSEL			GENMASK(3, 1)
+#define MX7D_USB_OTG_PHY_CFG1_COMMONONN			BIT(0)
 
 #define MX6_USB_OTG_WAKEUP_BITS (MX6_BM_WAKEUP_ENABLE | MX6_BM_VBUS_WAKEUP | \
 				 MX6_BM_ID_WAKEUP)
@@ -666,6 +677,22 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
 					  data->emp_curr_control);
 		}
 
+		if (data->usb_source_impedance_adjust &&
+		    FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_TXRESTUNE0,
+			      data->usb_source_impedance_adjust)) {
+			reg &= ~MX7D_USB_OTG_PHY_CFG1_TXRESTUNE0;
+			reg |= FIELD_PREP(MX7D_USB_OTG_PHY_CFG1_TXRESTUNE0,
+					  data->usb_source_impedance_adjust);
+		}
+
+		if (data->hs_transmitter_rise_time_adjust &&
+		    FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_TXRISETUNE0,
+			      data->hs_transmitter_rise_time_adjust)) {
+			reg &= ~MX7D_USB_OTG_PHY_CFG1_TXRISETUNE0;
+			reg |= FIELD_PREP(MX7D_USB_OTG_PHY_CFG1_TXRISETUNE0,
+					  data->hs_transmitter_rise_time_adjust);
+		}
+
 		if (data->dc_vol_level_adjust &&
 		    FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_TXVREFTUNE0,
 			      data->dc_vol_level_adjust)) {
@@ -674,6 +701,46 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
 					  data->dc_vol_level_adjust);
 		}
 
+		if (data->fs_ls_source_impedance_adjust &&
+		    FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_TXFSLSTUNE0,
+			      data->fs_ls_source_impedance_adjust)) {
+			reg &= ~MX7D_USB_OTG_PHY_CFG1_TXFSLSTUNE0;
+			reg |= FIELD_PREP(MX7D_USB_OTG_PHY_CFG1_TXFSLSTUNE0,
+					  data->fs_ls_source_impedance_adjust);
+		}
+
+		if (data->transmitter_hs_crossover_adjust &&
+		    FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_TXHSXVTUNE0,
+			      data->transmitter_hs_crossover_adjust)) {
+			reg &= ~MX7D_USB_OTG_PHY_CFG1_TXHSXVTUNE0;
+			reg |= FIELD_PREP(MX7D_USB_OTG_PHY_CFG1_TXHSXVTUNE0,
+					  data->transmitter_hs_crossover_adjust);
+		}
+
+		if (data->vbus_valid_threshold_adjust &&
+		    FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_OTGTUNE0,
+			      data->vbus_valid_threshold_adjust)) {
+			reg &= ~MX7D_USB_OTG_PHY_CFG1_OTGTUNE0;
+			reg |= FIELD_PREP(MX7D_USB_OTG_PHY_CFG1_OTGTUNE0,
+					  data->vbus_valid_threshold_adjust);
+		}
+
+		if (data->squelsh_threshold_adjust &&
+		    FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_SQRTUNE0,
+			      data->squelsh_threshold_adjust)) {
+			reg &= ~MX7D_USB_OTG_PHY_CFG1_SQRTUNE0;
+			reg |= FIELD_PREP(MX7D_USB_OTG_PHY_CFG1_SQRTUNE0,
+					  data->squelsh_threshold_adjust);
+		}
+
+		if (data->disconnect_threshold_adjust &&
+		    FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_COMPDISTUNE0,
+			      data->disconnect_threshold_adjust)) {
+			reg &= ~MX7D_USB_OTG_PHY_CFG1_COMPDISTUNE0;
+			reg |= FIELD_PREP(MX7D_USB_OTG_PHY_CFG1_COMPDISTUNE0,
+					  data->disconnect_threshold_adjust);
+		}
+
 		writel(reg, usbmisc->base + MX7D_USB_OTG_PHY_CFG1);
 	}
 
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/6] dt-bindings: usb: ci-hdrc-usb2: Add more phy tuning properties
  2022-10-11  8:29 [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree Sascha Hauer
                   ` (4 preceding siblings ...)
  2022-10-11  8:29 ` [PATCH 5/6] usb: chipidea: usbmisc_imx: Add device tree properties for i.MX7 phy tuning Sascha Hauer
@ 2022-10-11  8:29 ` Sascha Hauer
  2022-10-12 16:08   ` Rob Herring
  2022-10-17  0:41 ` [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree Peter Chen
  6 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2022-10-11  8:29 UTC (permalink / raw)
  To: linux-usb
  Cc: linux-arm-kernel, Peter Chen, Peng Fan, Pengutronix Kernel Team,
	devicetree, Sascha Hauer

Following the example of samsung,picophy-dc-vol-level-adjust more
phy tuning properties are added for configuring the remaining bitfields
in the USBNC_n_PHY_CFG1 register.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
index 11d08ffeb1e9c..c467924235759 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
@@ -255,15 +255,94 @@ allOf:
           minimum: 0x0
           maximum: 0x3
 
+        samsung,picophy-usb-source-impedance-adjust:
+          description: |
+            USB Source Impedance Adjustment. In some applications, there can be
+            significant series resistance on the USB DP/DN path between the
+            USB_OTG*_DP/USB_OTG*_DN pins tns and the USB cable. This bus adjusts
+            the driver source impedance to compensate for that added resistance.
+            The default value is 0x1. For more details refer to TXRESTUNE0 bits of
+            USBNC_n_PHY_CFG1.
+
+          $ref: /schemas/types.yaml#/definitions/uint32
+          minimum: 0x0
+          maximum: 0x3
+
+        samsung,picophy-hs-rise-time-adjust:
+          description: |
+            This bus adjust the rise/fall times of the high-speed transmitter
+            waveform. The default value is 0x1. For more details refer to
+            TXRISETUNE0 bits of USBNC_n_PHY_CFG1.
+
+          $ref: /schemas/types.yaml#/definitions/uint32
+          minimum: 0x0
+          maximum: 0x3
+
         samsung,picophy-dc-vol-level-adjust:
           description: |
             HS DC Voltage Level Adjustment. Adjust the high-speed transmitter DC
             level voltage. The range is from 0x0 to 0xf, the default value is
             0x3. Details can refer to TXVREFTUNE0 bits of USBNC_n_PHY_CFG1.
+
+        $ref: /schemas/types.yaml#/definitions/uint32
+          minimum: 0x0
+          maximum: 0xf
+
+        samsung,picophy-fs-ls-source-impedance-adjust:
+          description: |
+            FS/LS Source Impedance Adjustment. This bus adjusts the low- and
+            full-speed single-ended source impedance while driving high. The
+            adjustment values listed are based on nominal process, voltage, and
+            temperature conditions. The default value is 0x3. For more details
+            refer to TXFSLSTUNE0 bits of USBNC_n_PHY_CFG1.
+
           $ref: /schemas/types.yaml#/definitions/uint32
           minimum: 0x0
           maximum: 0xf
 
+        samsung,picophy-transmitter-hs-crossover-adjust:
+          description: |
+            Transmitter High-Speed Crossover Adjustment. This bus adjusts the
+            voltage at which the USB_OTG*_DP and USB_OTG*_DN signals cross
+            while transmitting in HS mode. The default value is 0x3. For more
+            details refer to TXHSXVTUNE0 bits of USBNC_n_PHY_CFG1.
+
+          $ref: /schemas/types.yaml#/definitions/uint32
+          minimum: 0x0
+          maximum: 0x3
+
+        samsung,picophy-vbus-valid-threshold-adjust:
+          description: |
+            VBUS Valid Threshold Adjustment. This bus adjust the voltage level
+            for the VBUS VALID threshold. The default value is 0x4. For more
+            details refer to OTGTUNE0 bits of USBNC_n_PHY_CFG1.
+
+          $ref: /schemas/types.yaml#/definitions/uint32
+          minimum: 0x0
+          maximum: 0x7
+
+        samsung,picophy-squelsh-threshold-adjust:
+          description: |
+            Squelch Threshold Adjustment. This bus adjusts the voltage level for
+            the receiver threshold used to detect valid high-speed data. The
+            default value is 0x3. For more details refer to SQRXTUNE0 bits of
+            USBNC_n_PHY_CFG1.
+
+          $ref: /schemas/types.yaml#/definitions/uint32
+          minimum: 0x0
+          maximum: 0x7
+
+        samsung,picophy-disconnect-threshold-adjust:
+          description: |
+            Disconnect Threshold Adjustment. This bus adjusts the voltage level for
+            the receiver threshold used to detect a disconnect event at the host.
+            The default value is 0x4. For more details refer to COMPDISTUNE0 bits of
+            USBNC_n_PHY_CFG1.
+
+          $ref: /schemas/types.yaml#/definitions/uint32
+          minimum: 0x0
+          maximum: 0x7
+
 additionalProperties: true
 
 examples:
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 6/6] dt-bindings: usb: ci-hdrc-usb2: Add more phy tuning properties
  2022-10-11  8:29 ` [PATCH 6/6] dt-bindings: usb: ci-hdrc-usb2: Add more phy tuning properties Sascha Hauer
@ 2022-10-12 16:08   ` Rob Herring
  2022-10-13 10:14     ` Sascha Hauer
  0 siblings, 1 reply; 15+ messages in thread
From: Rob Herring @ 2022-10-12 16:08 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-usb, linux-arm-kernel, Peter Chen, Peng Fan,
	Pengutronix Kernel Team, devicetree

On Tue, Oct 11, 2022 at 10:29:24AM +0200, Sascha Hauer wrote:
> Following the example of samsung,picophy-dc-vol-level-adjust more
> phy tuning properties are added for configuring the remaining bitfields
> in the USBNC_n_PHY_CFG1 register.

All these properties really doesn't scale. These properties should go 
in the phy node as they are properties or the phy. There's no rule that 
you can only read properties from the driver's device node.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6/6] dt-bindings: usb: ci-hdrc-usb2: Add more phy tuning properties
  2022-10-12 16:08   ` Rob Herring
@ 2022-10-13 10:14     ` Sascha Hauer
  2022-10-13 20:03       ` Rob Herring
  0 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2022-10-13 10:14 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-usb, linux-arm-kernel, Peter Chen, Peng Fan,
	Pengutronix Kernel Team, devicetree

On Wed, Oct 12, 2022 at 11:08:06AM -0500, Rob Herring wrote:
> On Tue, Oct 11, 2022 at 10:29:24AM +0200, Sascha Hauer wrote:
> > Following the example of samsung,picophy-dc-vol-level-adjust more
> > phy tuning properties are added for configuring the remaining bitfields
> > in the USBNC_n_PHY_CFG1 register.
> 
> All these properties really doesn't scale. These properties should go 
> in the phy node as they are properties or the phy. There's no rule that 
> you can only read properties from the driver's device node.

I understand and agree.

On i.MX8M we currently use the usb-nop-xceiv. I guess it's not an option
to just add these properties there, so we'll need a phy node with a new
compatible like fsl,imx8mm-usbphy. The driver would basically just
register a usb-nop-xceiv and the node would be a container for the new
property. Does this sound sane?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6/6] dt-bindings: usb: ci-hdrc-usb2: Add more phy tuning properties
  2022-10-13 10:14     ` Sascha Hauer
@ 2022-10-13 20:03       ` Rob Herring
  0 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2022-10-13 20:03 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-usb, linux-arm-kernel, Peter Chen, Peng Fan,
	Pengutronix Kernel Team, devicetree

On Thu, Oct 13, 2022 at 12:14:12PM +0200, Sascha Hauer wrote:
> On Wed, Oct 12, 2022 at 11:08:06AM -0500, Rob Herring wrote:
> > On Tue, Oct 11, 2022 at 10:29:24AM +0200, Sascha Hauer wrote:
> > > Following the example of samsung,picophy-dc-vol-level-adjust more
> > > phy tuning properties are added for configuring the remaining bitfields
> > > in the USBNC_n_PHY_CFG1 register.
> > 
> > All these properties really doesn't scale. These properties should go 
> > in the phy node as they are properties or the phy. There's no rule that 
> > you can only read properties from the driver's device node.
> 
> I understand and agree.
> 
> On i.MX8M we currently use the usb-nop-xceiv. I guess it's not an option
> to just add these properties there, so we'll need a phy node with a new
> compatible like fsl,imx8mm-usbphy. The driver would basically just
> register a usb-nop-xceiv and the node would be a container for the new
> property. Does this sound sane?

I think it would be fine if you do:

compatible = "fsl,imx8mm-usbphy", "usb-nop-xceiv";

You'll have to rework the usb-nop-xceiv schema 'select' like we have to 
do for any compatible appearing in multiple schema files.

Or don't have the fallback and add "fsl,imx8mm-usbphy" to the 
phy-generic.c driver. But that should be marked for stable for at least 
some compatibility with old kernels.

For existing kernel binaries to work, you have to go with the former 
option.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree
  2022-10-11  8:29 [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree Sascha Hauer
                   ` (5 preceding siblings ...)
  2022-10-11  8:29 ` [PATCH 6/6] dt-bindings: usb: ci-hdrc-usb2: Add more phy tuning properties Sascha Hauer
@ 2022-10-17  0:41 ` Peter Chen
  6 siblings, 0 replies; 15+ messages in thread
From: Peter Chen @ 2022-10-17  0:41 UTC (permalink / raw)
  To: Sascha Hauer, Xu Yang, Li Jun
  Cc: linux-usb, linux-arm-kernel, Peng Fan, Pengutronix Kernel Team,
	devicetree

On 22-10-11 10:29:18, Sascha Hauer wrote:
> This series exports more phy tuning settings to device tree. There are
> some values already exported and I follow that example in this series.
> 
> Patches 1 and 2 contain two small bugfixes for issues I stumbled upon
> along the way. Patches 3 and 4 are cleanups. These patches could be
> applied without the remaining two patches.
> 
> The binding patch is based on
> https://lore.kernel.org/linux-arm-kernel/20221010101816.298334-3-peng.fan@oss.nxp.com/t/,
> so this will need a rebase once this series settles.

I have left NXP, maybe Jun Li or Yang Xu could have a review.

Peter
> 
> Sascha
> 
> Sascha Hauer (6):
>   usb: chipidea: usbmisc_imx: Fix i.MX53 clock sel masks
>   usb: chipidea: usbmisc_imx: Fix setting i.MX6SX wakeup source
>   usb: chipidea: usbmisc_imx: Use GENMASK/FIELD_PREP for bitfields
>   usb: chipidea: usbmisc_imx: Add prefix to register defines
>   usb: chipidea: usbmisc_imx: Add device tree properties for i.MX7 phy
>     tuning
>   dt-bindings: usb: ci-hdrc-usb2: Add more phy tuning properties
> 
>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml |  79 ++++++++
>  drivers/usb/chipidea/ci_hdrc_imx.c            |  14 ++
>  drivers/usb/chipidea/ci_hdrc_imx.h            |   7 +
>  drivers/usb/chipidea/usbmisc_imx.c            | 186 ++++++++++++------
>  4 files changed, 230 insertions(+), 56 deletions(-)
> 
> -- 
> 2.30.2
> 

-- 

Thanks,
Peter Chen

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/6] usb: chipidea: usbmisc_imx: Use GENMASK/FIELD_PREP for bitfields
  2022-10-11  8:29 ` [PATCH 3/6] usb: chipidea: usbmisc_imx: Use GENMASK/FIELD_PREP for bitfields Sascha Hauer
@ 2022-10-18  8:39   ` xu yang
  0 siblings, 0 replies; 15+ messages in thread
From: xu yang @ 2022-10-18  8:39 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-usb, linux-arm-kernel, Peter Chen, Peng Fan,
	Pengutronix Kernel Team, devicetree, Xu Yang, jun.li

Hi Sascha,

On Tue, Oct 11, 2022 at 4:55 PM Sascha Hauer <s.hauer@pengutronix.de> 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>
> ---
>  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..95f2ba01c0df1 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(5, 4)

Should be GENMASK(7, 6)

Thanks,
Xu Yang

> +#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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/6] usb: chipidea: usbmisc_imx: Fix i.MX53 clock sel masks
  2022-10-11  8:29 ` [PATCH 1/6] usb: chipidea: usbmisc_imx: Fix i.MX53 clock sel masks Sascha Hauer
@ 2022-10-18  8:45   ` Xu Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Xu Yang @ 2022-10-18  8:45 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-usb, linux-arm-kernel, Peter Chen, Peng Fan,
	Pengutronix Kernel Team, devicetree, Xu Yang, jun.li

On Tue, Oct 11, 2022 at 4:50 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> According to the reference manual the masks for the
> MX53_USB_CTRL_1_H*_XCVR_CLK_SEL bits are 0x3, not 0x11 (which were
> probably meant as 0b11).
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Xu Yang <xu.yang_2@nxp.com>

Thanks,
Xu Yang

> ---
>  drivers/usb/chipidea/usbmisc_imx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index bac0f5458cab9..8f805aa9c383c 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -42,9 +42,9 @@
>  #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 (0x11 << 2)
> +#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 (0x11 << 6)
> +#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_UH2_CTRL_OFFSET       0x14
>  #define MX53_USB_UH3_CTRL_OFFSET       0x18
> --
> 2.30.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/6] usb: chipidea: usbmisc_imx: Fix setting i.MX6SX wakeup source
  2022-10-11  8:29 ` [PATCH 2/6] usb: chipidea: usbmisc_imx: Fix setting i.MX6SX wakeup source Sascha Hauer
@ 2022-10-18  8:46   ` Xu Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Xu Yang @ 2022-10-18  8:46 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-usb, linux-arm-kernel, Peter Chen, Peng Fan,
	Pengutronix Kernel Team, devicetree, jun.li, Xu Yang

On Tue, Oct 11, 2022 at 4:41 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> MX6SX_USB_VBUS_WAKEUP_SOURCE are two bits containing an enum value,
> so when read/modify/write that we have to clear both bits bits before
> setting the desired bits. The clearing of the bits was forgotten, add
> it.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Xu Yang <xu.yang_2@nxp.com>

Thanks,
Xu Yang

> ---
>  drivers/usb/chipidea/usbmisc_imx.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 8f805aa9c383c..e1b4b7f9b3f31 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -551,6 +551,7 @@ 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);
>                 /*
>                  * Disable dp/dm wakeup in device mode when vbus is
> --
> 2.30.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/6] usb: chipidea: usbmisc_imx: Add prefix to register defines
  2022-10-11  8:29 ` [PATCH 4/6] usb: chipidea: usbmisc_imx: Add prefix to register defines Sascha Hauer
@ 2022-10-18  8:49   ` Xu Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Xu Yang @ 2022-10-18  8:49 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-usb, linux-arm-kernel, Peter Chen, Peng Fan,
	Pengutronix Kernel Team, devicetree, Xu Yang, jun.li

On Tue, Oct 11, 2022 at 4:49 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> The driver is used for a broad range of i.MX SoCs and most of the
> register defines have a SoC/regname specific prefix to make clear
> in which context they should be used. Add such a prefix to the
> MX7D_USB_OTG_PHY_CFG1 defines as well.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Xu Yang <xu.yang_2@nxp.com>

Thanks,
Xu Yang

> ---
>  drivers/usb/chipidea/usbmisc_imx.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 95f2ba01c0df1..63de7d6fea427 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -126,8 +126,8 @@
>  #define MX7D_USB_OTG_PHY_STATUS_CHRGDET                BIT(29)
>
>  #define MX7D_USB_OTG_PHY_CFG1          0x30
> -#define TXPREEMPAMPTUNE0               GENMASK(29, 28)
> -#define TXVREFTUNE0                    GENMASK(23, 20)
> +#define MX7D_USB_OTG_PHY_CFG1_TXPREEMPAMPTUNE0 GENMASK(29, 28)
> +#define MX7D_USB_OTG_PHY_CFG1_TXVREFTUNE0      GENMASK(23, 20)
>
>  #define MX6_USB_OTG_WAKEUP_BITS (MX6_BM_WAKEUP_ENABLE | MX6_BM_VBUS_WAKEUP | \
>                                  MX6_BM_ID_WAKEUP)
> @@ -659,15 +659,19 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
>                 /* PHY tuning for signal quality */
>                 reg = readl(usbmisc->base + MX7D_USB_OTG_PHY_CFG1);
>                 if (data->emp_curr_control &&
> -                   FIELD_FIT(TXPREEMPAMPTUNE0, data->emp_curr_control)) {
> -                       reg &= ~TXPREEMPAMPTUNE0;
> -                       reg |= FIELD_PREP(TXPREEMPAMPTUNE0, data->emp_curr_control);
> +                   FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_TXPREEMPAMPTUNE0,
> +                             data->emp_curr_control)) {
> +                       reg &= ~MX7D_USB_OTG_PHY_CFG1_TXPREEMPAMPTUNE0;
> +                       reg |= FIELD_PREP(MX7D_USB_OTG_PHY_CFG1_TXPREEMPAMPTUNE0,
> +                                         data->emp_curr_control);
>                 }
>
>                 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);
> +                   FIELD_FIT(MX7D_USB_OTG_PHY_CFG1_TXVREFTUNE0,
> +                             data->dc_vol_level_adjust)) {
> +                       reg &= ~MX7D_USB_OTG_PHY_CFG1_TXVREFTUNE0;
> +                       reg |= FIELD_PREP(MX7D_USB_OTG_PHY_CFG1_TXVREFTUNE0,
> +                                         data->dc_vol_level_adjust);
>                 }
>
>                 writel(reg, usbmisc->base + MX7D_USB_OTG_PHY_CFG1);
> --
> 2.30.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-10-18  8:50 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-11  8:29 [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree Sascha Hauer
2022-10-11  8:29 ` [PATCH 1/6] usb: chipidea: usbmisc_imx: Fix i.MX53 clock sel masks Sascha Hauer
2022-10-18  8:45   ` Xu Yang
2022-10-11  8:29 ` [PATCH 2/6] usb: chipidea: usbmisc_imx: Fix setting i.MX6SX wakeup source Sascha Hauer
2022-10-18  8:46   ` Xu Yang
2022-10-11  8:29 ` [PATCH 3/6] usb: chipidea: usbmisc_imx: Use GENMASK/FIELD_PREP for bitfields Sascha Hauer
2022-10-18  8:39   ` xu yang
2022-10-11  8:29 ` [PATCH 4/6] usb: chipidea: usbmisc_imx: Add prefix to register defines Sascha Hauer
2022-10-18  8:49   ` Xu Yang
2022-10-11  8:29 ` [PATCH 5/6] usb: chipidea: usbmisc_imx: Add device tree properties for i.MX7 phy tuning Sascha Hauer
2022-10-11  8:29 ` [PATCH 6/6] dt-bindings: usb: ci-hdrc-usb2: Add more phy tuning properties Sascha Hauer
2022-10-12 16:08   ` Rob Herring
2022-10-13 10:14     ` Sascha Hauer
2022-10-13 20:03       ` Rob Herring
2022-10-17  0:41 ` [PATCH 0/6] usb: chipidea: Export more phy tuning parameters to device tree Peter Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).