[PATCH 0/6] Various bug fixes/improvements for phy-brcm-usb driver

[PATCH 0/6] Various bug fixes/improvements for phy-brcm-usb driver

[justinpopo6]

From: Justin Chen <justin.chen@broadcom.com>

A few suspend/resume bug fixes with clocks and wake counter. Also improve
port mode selection.

Al Cooper (1):
  phy: usb: s2 WoL wakeup_count not incremented for USB->Eth devices

Justin Chen (5):
  phy: usb: Improve port mode selection
  phy: usb: Migrate to BIT and BITMASK macros
  phy: usb: Disable phy auto-suspend
  phy: usb: Use slow clock for wake enabled suspend
  phy: usb: Fix clock imbalance for suspend/resume

 drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c | 95 ++++++++++++-----------
 drivers/phy/broadcom/phy-brcm-usb-init.c          | 90 ++++++++++-----------
 drivers/phy/broadcom/phy-brcm-usb-init.h          | 11 ++-
 drivers/phy/broadcom/phy-brcm-usb.c               | 28 ++++---
 4 files changed, 116 insertions(+), 108 deletions(-)

-- 
2.7.4

[PATCH 1/6] phy: usb: Improve port mode selection

[justinpopo6]

From: Justin Chen <justinpopo6@gmail.com>

Split port modes into two different variables. Supported port modes
is what the hardware supports. While port mode is how the hardware
is currently configured and can be dynamically changed through the
sysfs. We initialize all supported port modes on init even though
the port mode may not be selected because we cannot guarantee the
downstream interface from the phy will be active or not.

This also fixes an issue where port modes selected via sysfs were
not being saved through suspend/resume.

Signed-off-by: Justin Chen <justinpopo6@gmail.com>
---
 drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c | 12 ++++++------
 drivers/phy/broadcom/phy-brcm-usb-init.c          | 10 +++++-----
 drivers/phy/broadcom/phy-brcm-usb-init.h          | 10 +++++-----
 drivers/phy/broadcom/phy-brcm-usb.c               | 14 +++++++++-----
 4 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
index d2524b7..430a8ae 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
+++ b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
@@ -195,10 +195,10 @@ static void usb_init_common(struct brcm_usb_init_params *params)
 	if (USB_CTRL_MASK(USB_DEVICE_CTL1, PORT_MODE)) {
 		reg = brcm_usb_readl(USB_CTRL_REG(ctrl, USB_DEVICE_CTL1));
 		reg &= ~USB_CTRL_MASK(USB_DEVICE_CTL1, PORT_MODE);
-		reg |= params->mode;
+		reg |= params->port_mode;
 		brcm_usb_writel(reg, USB_CTRL_REG(ctrl, USB_DEVICE_CTL1));
 	}
-	switch (params->mode) {
+	switch (params->supported_port_modes) {
 	case USB_CTLR_MODE_HOST:
 		USB_CTRL_UNSET(ctrl, USB_PM, BDC_SOFT_RESETB);
 		break;
@@ -276,7 +276,7 @@ static void usb_init_common_7211b0(struct brcm_usb_init_params *params)
 	/* Set the PHY_MODE */
 	reg = brcm_usb_readl(usb_phy + USB_PHY_UTMI_CTL_1);
 	reg &= ~USB_PHY_UTMI_CTL_1_PHY_MODE_MASK;
-	reg |= params->mode << USB_PHY_UTMI_CTL_1_PHY_MODE_SHIFT;
+	reg |= params->supported_port_modes << USB_PHY_UTMI_CTL_1_PHY_MODE_SHIFT;
 	brcm_usb_writel(reg, usb_phy + USB_PHY_UTMI_CTL_1);
 
 	usb_init_common(params);
@@ -286,7 +286,7 @@ static void usb_init_common_7211b0(struct brcm_usb_init_params *params)
 	 * the default "Read Transaction Size" of 6 (1024 bytes).
 	 * Set it to 4 (256 bytes).
 	 */
-	if ((params->mode != USB_CTLR_MODE_HOST) && bdc_ec) {
+	if ((params->supported_port_modes != USB_CTLR_MODE_HOST) && bdc_ec) {
 		reg = brcm_usb_readl(bdc_ec + BDC_EC_AXIRDA);
 		reg &= ~BDC_EC_AXIRDA_RTS_MASK;
 		reg |= (0x4 << BDC_EC_AXIRDA_RTS_SHIFT);
@@ -385,7 +385,7 @@ static int usb_get_dual_select(struct brcm_usb_init_params *params)
 	return reg;
 }
 
-static void usb_set_dual_select(struct brcm_usb_init_params *params, int mode)
+static void usb_set_dual_select(struct brcm_usb_init_params *params)
 {
 	void __iomem *ctrl = params->regs[BRCM_REGS_CTRL];
 	u32 reg;
@@ -394,7 +394,7 @@ static void usb_set_dual_select(struct brcm_usb_init_params *params, int mode)
 
 	reg = brcm_usb_readl(USB_CTRL_REG(ctrl, USB_DEVICE_CTL1));
 	reg &= ~USB_CTRL_MASK(USB_DEVICE_CTL1, PORT_MODE);
-	reg |= mode;
+	reg |= params->port_mode;
 	brcm_usb_writel(reg, USB_CTRL_REG(ctrl, USB_DEVICE_CTL1));
 }
 
diff --git a/drivers/phy/broadcom/phy-brcm-usb-init.c b/drivers/phy/broadcom/phy-brcm-usb-init.c
index dddcbd3..a7f8b3d 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init.c
+++ b/drivers/phy/broadcom/phy-brcm-usb-init.c
@@ -876,11 +876,11 @@ static void usb_init_common(struct brcm_usb_init_params *params)
 		reg = brcm_usb_readl(USB_CTRL_REG(ctrl, USB_DEVICE_CTL1));
 		reg &= ~USB_CTRL_MASK_FAMILY(params, USB_DEVICE_CTL1,
 					PORT_MODE);
-		reg |= params->mode;
+		reg |= params->port_mode;
 		brcm_usb_writel(reg, USB_CTRL_REG(ctrl, USB_DEVICE_CTL1));
 	}
 	if (USB_CTRL_MASK_FAMILY(params, USB_PM, BDC_SOFT_RESETB)) {
-		switch (params->mode) {
+		switch (params->supported_port_modes) {
 		case USB_CTLR_MODE_HOST:
 			USB_CTRL_UNSET_FAMILY(params, USB_PM, BDC_SOFT_RESETB);
 			break;
@@ -891,7 +891,7 @@ static void usb_init_common(struct brcm_usb_init_params *params)
 		}
 	}
 	if (USB_CTRL_MASK_FAMILY(params, SETUP, CC_DRD_MODE_ENABLE)) {
-		if (params->mode == USB_CTLR_MODE_TYPEC_PD)
+		if (params->supported_port_modes == USB_CTLR_MODE_TYPEC_PD)
 			USB_CTRL_SET_FAMILY(params, SETUP, CC_DRD_MODE_ENABLE);
 		else
 			USB_CTRL_UNSET_FAMILY(params, SETUP,
@@ -1000,7 +1000,7 @@ static int usb_get_dual_select(struct brcm_usb_init_params *params)
 	return reg;
 }
 
-static void usb_set_dual_select(struct brcm_usb_init_params *params, int mode)
+static void usb_set_dual_select(struct brcm_usb_init_params *params)
 {
 	void __iomem *ctrl = params->regs[BRCM_REGS_CTRL];
 	u32 reg;
@@ -1011,7 +1011,7 @@ static void usb_set_dual_select(struct brcm_usb_init_params *params, int mode)
 		reg = brcm_usb_readl(USB_CTRL_REG(ctrl, USB_DEVICE_CTL1));
 		reg &= ~USB_CTRL_MASK_FAMILY(params, USB_DEVICE_CTL1,
 					PORT_MODE);
-		reg |= mode;
+		reg |= params->port_mode;
 		brcm_usb_writel(reg, USB_CTRL_REG(ctrl, USB_DEVICE_CTL1));
 	}
 }
diff --git a/drivers/phy/broadcom/phy-brcm-usb-init.h b/drivers/phy/broadcom/phy-brcm-usb-init.h
index 1ccb5dd..bedf2b8 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init.h
+++ b/drivers/phy/broadcom/phy-brcm-usb-init.h
@@ -45,14 +45,15 @@ struct brcm_usb_init_ops {
 	void (*uninit_eohci)(struct brcm_usb_init_params *params);
 	void (*uninit_xhci)(struct brcm_usb_init_params *params);
 	int  (*get_dual_select)(struct brcm_usb_init_params *params);
-	void (*set_dual_select)(struct brcm_usb_init_params *params, int mode);
+	void (*set_dual_select)(struct brcm_usb_init_params *params);
 };
 
 struct  brcm_usb_init_params {
 	void __iomem *regs[BRCM_REGS_MAX];
 	int ioc;
 	int ipp;
-	int mode;
+	int supported_port_modes;
+	int port_mode;
 	u32 family_id;
 	u32 product_id;
 	int selected_family;
@@ -153,11 +154,10 @@ static inline int brcm_usb_get_dual_select(struct brcm_usb_init_params *ini)
 	return 0;
 }
 
-static inline void brcm_usb_set_dual_select(struct brcm_usb_init_params *ini,
-	int mode)
+static inline void brcm_usb_set_dual_select(struct brcm_usb_init_params *ini)
 {
 	if (ini->ops->set_dual_select)
-		ini->ops->set_dual_select(ini, mode);
+		ini->ops->set_dual_select(ini);
 }
 
 #endif /* _USB_BRCM_COMMON_INIT_H */
diff --git a/drivers/phy/broadcom/phy-brcm-usb.c b/drivers/phy/broadcom/phy-brcm-usb.c
index 2cb3779..99d4dea 100644
--- a/drivers/phy/broadcom/phy-brcm-usb.c
+++ b/drivers/phy/broadcom/phy-brcm-usb.c
@@ -233,7 +233,7 @@ static ssize_t dr_mode_show(struct device *dev,
 	return sprintf(buf, "%s\n",
 		value_to_name(&brcm_dr_mode_to_name[0],
 			      ARRAY_SIZE(brcm_dr_mode_to_name),
-			      priv->ini.mode));
+			      priv->ini.supported_port_modes));
 }
 static DEVICE_ATTR_RO(dr_mode);
 
@@ -249,7 +249,8 @@ static ssize_t dual_select_store(struct device *dev,
 	res = name_to_value(&brcm_dual_mode_to_name[0],
 			    ARRAY_SIZE(brcm_dual_mode_to_name), buf, &value);
 	if (!res) {
-		brcm_usb_set_dual_select(&priv->ini, value);
+		priv->ini.port_mode = value;
+		brcm_usb_set_dual_select(&priv->ini);
 		res = len;
 	}
 	mutex_unlock(&sysfs_lock);
@@ -495,13 +496,16 @@ static int brcm_usb_phy_probe(struct platform_device *pdev)
 	of_property_read_u32(dn, "brcm,ipp", &priv->ini.ipp);
 	of_property_read_u32(dn, "brcm,ioc", &priv->ini.ioc);
 
-	priv->ini.mode = USB_CTLR_MODE_HOST;
+	priv->ini.supported_port_modes = USB_CTLR_MODE_HOST;
 	err = of_property_read_string(dn, "dr_mode", &mode);
 	if (err == 0) {
 		name_to_value(&brcm_dr_mode_to_name[0],
 			      ARRAY_SIZE(brcm_dr_mode_to_name),
-			mode, &priv->ini.mode);
+			mode, &priv->ini.supported_port_modes);
 	}
+	/* Default port_mode to supported port_modes */
+	priv->ini.port_mode = priv->ini.supported_port_modes;
+
 	if (of_property_read_bool(dn, "brcm,has-xhci"))
 		priv->has_xhci = true;
 	if (of_property_read_bool(dn, "brcm,has-eohci"))
@@ -539,7 +543,7 @@ static int brcm_usb_phy_probe(struct platform_device *pdev)
 	 * Create sysfs entries for mode.
 	 * Remove "dual_select" attribute if not in dual mode
 	 */
-	if (priv->ini.mode != USB_CTLR_MODE_DRD)
+	if (priv->ini.supported_port_modes != USB_CTLR_MODE_DRD)
 		brcm_usb_phy_attrs[1] = NULL;
 	err = sysfs_create_group(&dev->kobj, &brcm_usb_phy_group);
 	if (err)
-- 
2.7.4

Re: [PATCH 1/6] phy: usb: Improve port mode selection

[Florian Fainelli]

On 10/5/2022 2:30 PM, justinpopo6@gmail.com wrote:
> From: Justin Chen <justinpopo6@gmail.com>
> 
> Split port modes into two different variables. Supported port modes
> is what the hardware supports. While port mode is how the hardware
> is currently configured and can be dynamically changed through the
> sysfs. We initialize all supported port modes on init even though
> the port mode may not be selected because we cannot guarantee the
> downstream interface from the phy will be active or not.
> 
> This also fixes an issue where port modes selected via sysfs were
> not being saved through suspend/resume.
> 
> Signed-off-by: Justin Chen <justinpopo6@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH 2/6] phy: usb: s2 WoL wakeup_count not incremented for USB->Eth devices

[justinpopo6]

From: Al Cooper <alcooperx@gmail.com>

The PHY's "wakeup_count" is not incrementing when waking from
WoL. The wakeup count can be found in sysfs at:
/sys/bus/platform/devices/rdb/*.usb-phy/power/wakeup_count.
The problem is that the system wakup event handler was being passed
the wrong "device" by the PHY driver.

Fixes: f1c0db40a3ad ("usb: Add "wake on" functionality")
Signed-off-by: Al Cooper <alcooperx@gmail.com>
Signed-off-by: Justin Chen <justinpopo6@gmail.com>
---
 drivers/phy/broadcom/phy-brcm-usb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/broadcom/phy-brcm-usb.c b/drivers/phy/broadcom/phy-brcm-usb.c
index 99d4dea..d97fa58 100644
--- a/drivers/phy/broadcom/phy-brcm-usb.c
+++ b/drivers/phy/broadcom/phy-brcm-usb.c
@@ -102,9 +102,9 @@ static int brcm_pm_notifier(struct notifier_block *notifier,
 
 static irqreturn_t brcm_usb_phy_wake_isr(int irq, void *dev_id)
 {
-	struct phy *gphy = dev_id;
+	struct device *dev = dev_id;
 
-	pm_wakeup_event(&gphy->dev, 0);
+	pm_wakeup_event(dev, 0);
 
 	return IRQ_HANDLED;
 }
@@ -452,7 +452,7 @@ static int brcm_usb_phy_dvr_init(struct platform_device *pdev,
 	if (priv->wake_irq >= 0) {
 		err = devm_request_irq(dev, priv->wake_irq,
 				       brcm_usb_phy_wake_isr, 0,
-				       dev_name(dev), gphy);
+				       dev_name(dev), dev);
 		if (err < 0)
 			return err;
 		device_set_wakeup_capable(dev, 1);
-- 
2.7.4

Re: [PATCH 2/6] phy: usb: s2 WoL wakeup_count not incremented for USB->Eth devices

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 609 bytes --]



On 10/5/2022 2:30 PM, justinpopo6@gmail.com wrote:
> From: Al Cooper <alcooperx@gmail.com>
> 
> The PHY's "wakeup_count" is not incrementing when waking from
> WoL. The wakeup count can be found in sysfs at:
> /sys/bus/platform/devices/rdb/*.usb-phy/power/wakeup_count.
> The problem is that the system wakup event handler was being passed
> the wrong "device" by the PHY driver.
> 
> Fixes: f1c0db40a3ad ("usb: Add "wake on" functionality")
> Signed-off-by: Al Cooper <alcooperx@gmail.com>
> Signed-off-by: Justin Chen <justinpopo6@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

[PATCH 3/6] phy: usb: Migrate to BIT and BITMASK macros

[justinpopo6]

From: Justin Chen <justinpopo6@gmail.com>

Using BIT and BITMASK macros makes it much easier to read and make
modifications. Also reordered some constants to be in numerical order.

Signed-off-by: Justin Chen <justinpopo6@gmail.com>
---
 drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c | 68 +++++++++----------
 drivers/phy/broadcom/phy-brcm-usb-init.c          | 80 +++++++++++------------
 2 files changed, 74 insertions(+), 74 deletions(-)

diff --git a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
index 430a8ae..26e9585e 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
+++ b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
@@ -18,14 +18,14 @@
 #define PIARBCTL_CAM			0x00
 #define PIARBCTL_SPLITTER		0x04
 #define PIARBCTL_MISC			0x08
-#define   PIARBCTL_MISC_SECURE_MASK			0x80000000
-#define   PIARBCTL_MISC_USB_SELECT_MASK			0x40000000
-#define   PIARBCTL_MISC_USB_4G_SDRAM_MASK		0x20000000
-#define   PIARBCTL_MISC_USB_PRIORITY_MASK		0x000f0000
-#define   PIARBCTL_MISC_USB_MEM_PAGE_MASK		0x0000f000
-#define   PIARBCTL_MISC_CAM1_MEM_PAGE_MASK		0x00000f00
-#define   PIARBCTL_MISC_CAM0_MEM_PAGE_MASK		0x000000f0
-#define   PIARBCTL_MISC_SATA_PRIORITY_MASK		0x0000000f
+#define   PIARBCTL_MISC_SATA_PRIORITY_MASK		GENMASK(3, 0)
+#define   PIARBCTL_MISC_CAM0_MEM_PAGE_MASK		GENMASK(7, 4)
+#define   PIARBCTL_MISC_CAM1_MEM_PAGE_MASK		GENMASK(11, 8)
+#define   PIARBCTL_MISC_USB_MEM_PAGE_MASK		GENMASK(15, 12)
+#define   PIARBCTL_MISC_USB_PRIORITY_MASK		GENMASK(19, 16)
+#define   PIARBCTL_MISC_USB_4G_SDRAM_MASK		BIT(29)
+#define   PIARBCTL_MISC_USB_SELECT_MASK			BIT(30)
+#define   PIARBCTL_MISC_SECURE_MASK			BIT(31)
 
 #define PIARBCTL_MISC_USB_ONLY_MASK		\
 	(PIARBCTL_MISC_USB_SELECT_MASK |	\
@@ -35,46 +35,46 @@
 
 /* Register definitions for the USB CTRL block */
 #define USB_CTRL_SETUP			0x00
-#define   USB_CTRL_SETUP_STRAP_IPP_SEL_MASK		0x02000000
-#define   USB_CTRL_SETUP_SCB2_EN_MASK			0x00008000
-#define   USB_CTRL_SETUP_tca_drv_sel_MASK		0x01000000
-#define   USB_CTRL_SETUP_SCB1_EN_MASK			0x00004000
-#define   USB_CTRL_SETUP_SOFT_SHUTDOWN_MASK		0x00000200
-#define   USB_CTRL_SETUP_IPP_MASK			0x00000020
-#define   USB_CTRL_SETUP_IOC_MASK			0x00000010
+#define   USB_CTRL_SETUP_IOC_MASK			BIT(4)
+#define   USB_CTRL_SETUP_IPP_MASK			BIT(5)
+#define   USB_CTRL_SETUP_SOFT_SHUTDOWN_MASK		BIT(9)
+#define   USB_CTRL_SETUP_SCB1_EN_MASK			BIT(14)
+#define   USB_CTRL_SETUP_SCB2_EN_MASK			BIT(15)
+#define   USB_CTRL_SETUP_tca_drv_sel_MASK		BIT(24)
+#define   USB_CTRL_SETUP_STRAP_IPP_SEL_MASK		BIT(25)
 #define USB_CTRL_USB_PM			0x04
-#define   USB_CTRL_USB_PM_USB_PWRDN_MASK		0x80000000
-#define   USB_CTRL_USB_PM_SOFT_RESET_MASK		0x40000000
-#define   USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK		0x00800000
-#define   USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK		0x00400000
-#define   USB_CTRL_USB_PM_XHC_PME_EN_MASK		0x00000010
-#define   USB_CTRL_USB_PM_XHC_S2_CLK_SWITCH_EN_MASK	0x00000008
+#define   USB_CTRL_USB_PM_XHC_S2_CLK_SWITCH_EN_MASK	BIT(3)
+#define   USB_CTRL_USB_PM_XHC_PME_EN_MASK		BIT(4)
+#define   USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK		BIT(22)
+#define   USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK		BIT(23)
+#define   USB_CTRL_USB_PM_SOFT_RESET_MASK		BIT(30)
+#define   USB_CTRL_USB_PM_USB_PWRDN_MASK		BIT(31)
 #define USB_CTRL_USB_PM_STATUS		0x08
 #define USB_CTRL_USB_DEVICE_CTL1	0x10
-#define   USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK	0x00000003
+#define   USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK	GENMASK(1, 0)
 #define USB_CTRL_TEST_PORT_CTL		0x30
-#define   USB_CTRL_TEST_PORT_CTL_TPOUT_SEL_MASK	0x000000ff
+#define   USB_CTRL_TEST_PORT_CTL_TPOUT_SEL_MASK		GENMASK(7, 0)
 #define   USB_CTRL_TEST_PORT_CTL_TPOUT_SEL_PME_GEN_MASK	0x0000002e
 #define USB_CTRL_TP_DIAG1		0x34
-#define   USB_CTLR_TP_DIAG1_wake_MASK	0x00000002
+#define   USB_CTLR_TP_DIAG1_wake_MASK			BIT(1)
 #define USB_CTRL_CTLR_CSHCR		0x50
-#define   USB_CTRL_CTLR_CSHCR_ctl_pme_en_MASK	0x00040000
+#define   USB_CTRL_CTLR_CSHCR_ctl_pme_en_MASK		BIT(18)
 
 /* Register definitions for the USB_PHY block in 7211b0 */
 #define USB_PHY_PLL_CTL			0x00
-#define   USB_PHY_PLL_CTL_PLL_RESETB_MASK		0x40000000
+#define   USB_PHY_PLL_CTL_PLL_RESETB_MASK		BIT(30)
 #define USB_PHY_PLL_LDO_CTL		0x08
-#define   USB_PHY_PLL_LDO_CTL_AFE_CORERDY_MASK		0x00000004
-#define   USB_PHY_PLL_LDO_CTL_AFE_LDO_PWRDWNB_MASK	0x00000002
-#define   USB_PHY_PLL_LDO_CTL_AFE_BG_PWRDWNB_MASK	0x00000001
+#define   USB_PHY_PLL_LDO_CTL_AFE_BG_PWRDWNB_MASK	BIT(0)
+#define   USB_PHY_PLL_LDO_CTL_AFE_LDO_PWRDWNB_MASK	BIT(1)
+#define   USB_PHY_PLL_LDO_CTL_AFE_CORERDY_MASK		BIT(2)
 #define USB_PHY_UTMI_CTL_1		0x04
-#define   USB_PHY_UTMI_CTL_1_POWER_UP_FSM_EN_MASK	0x00000800
-#define   USB_PHY_UTMI_CTL_1_PHY_MODE_MASK		0x0000000c
+#define   USB_PHY_UTMI_CTL_1_PHY_MODE_MASK		GENMASK(3, 2)
 #define   USB_PHY_UTMI_CTL_1_PHY_MODE_SHIFT		2
+#define   USB_PHY_UTMI_CTL_1_POWER_UP_FSM_EN_MASK	BIT(11)
 #define USB_PHY_IDDQ			0x1c
-#define   USB_PHY_IDDQ_phy_iddq_MASK			0x00000001
+#define   USB_PHY_IDDQ_phy_iddq_MASK			BIT(0)
 #define USB_PHY_STATUS			0x20
-#define   USB_PHY_STATUS_pll_lock_MASK			0x00000001
+#define   USB_PHY_STATUS_pll_lock_MASK			BIT(0)
 
 /* Register definitions for the MDIO registers in the DWC2 block of
  * the 7211b0.
@@ -86,7 +86,7 @@
 
 /* Register definitions for the BDC EC block in 7211b0 */
 #define BDC_EC_AXIRDA			0x0c
-#define   BDC_EC_AXIRDA_RTS_MASK			0xf0000000
+#define   BDC_EC_AXIRDA_RTS_MASK			GENMASK(31, 28)
 #define   BDC_EC_AXIRDA_RTS_SHIFT			28
 
 
diff --git a/drivers/phy/broadcom/phy-brcm-usb-init.c b/drivers/phy/broadcom/phy-brcm-usb-init.c
index a7f8b3d..a1ca833 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init.c
+++ b/drivers/phy/broadcom/phy-brcm-usb-init.c
@@ -21,57 +21,57 @@
 
 /* Register definitions for the USB CTRL block */
 #define USB_CTRL_SETUP			0x00
-#define   USB_CTRL_SETUP_IOC_MASK			0x00000010
-#define   USB_CTRL_SETUP_IPP_MASK			0x00000020
-#define   USB_CTRL_SETUP_BABO_MASK			0x00000001
-#define   USB_CTRL_SETUP_FNHW_MASK			0x00000002
-#define   USB_CTRL_SETUP_FNBO_MASK			0x00000004
-#define   USB_CTRL_SETUP_WABO_MASK			0x00000008
-#define   USB_CTRL_SETUP_SCB_CLIENT_SWAP_MASK		0x00002000 /* option */
-#define   USB_CTRL_SETUP_SCB1_EN_MASK			0x00004000 /* option */
-#define   USB_CTRL_SETUP_SCB2_EN_MASK			0x00008000 /* option */
-#define   USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK		0X00020000 /* option */
-#define   USB_CTRL_SETUP_SS_EHCI64BIT_EN_VAR_MASK	0x00010000 /* option */
-#define   USB_CTRL_SETUP_STRAP_IPP_SEL_MASK		0x02000000 /* option */
-#define   USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK	0x04000000 /* option */
-#define   USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK 0x08000000 /* opt */
-#define   USB_CTRL_SETUP_OC3_DISABLE_MASK		0xc0000000 /* option */
+#define   USB_CTRL_SETUP_BABO_MASK			BIT(0)
+#define   USB_CTRL_SETUP_FNHW_MASK			BIT(1)
+#define   USB_CTRL_SETUP_FNBO_MASK			BIT(2)
+#define   USB_CTRL_SETUP_WABO_MASK			BIT(3)
+#define   USB_CTRL_SETUP_IOC_MASK			BIT(4)
+#define   USB_CTRL_SETUP_IPP_MASK			BIT(5)
+#define   USB_CTRL_SETUP_SCB_CLIENT_SWAP_MASK		BIT(13) /* option */
+#define   USB_CTRL_SETUP_SCB1_EN_MASK			BIT(14) /* option */
+#define   USB_CTRL_SETUP_SCB2_EN_MASK			BIT(15) /* option */
+#define   USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK		BIT(17) /* option */
+#define   USB_CTRL_SETUP_SS_EHCI64BIT_EN_VAR_MASK	BIT(16) /* option */
+#define   USB_CTRL_SETUP_STRAP_IPP_SEL_MASK		BIT(25) /* option */
+#define   USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK	BIT(26) /* option */
+#define   USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK BIT(27) /* opt */
+#define   USB_CTRL_SETUP_OC3_DISABLE_MASK		GENMASK(31, 30) /* option */
 #define USB_CTRL_PLL_CTL		0x04
-#define   USB_CTRL_PLL_CTL_PLL_SUSPEND_EN_MASK		0x08000000
-#define   USB_CTRL_PLL_CTL_PLL_RESETB_MASK		0x40000000
-#define   USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK		0x80000000 /* option */
+#define   USB_CTRL_PLL_CTL_PLL_SUSPEND_EN_MASK		BIT(27)
+#define   USB_CTRL_PLL_CTL_PLL_RESETB_MASK		BIT(30)
+#define   USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK		BIT(31) /* option */
 #define USB_CTRL_EBRIDGE		0x0c
-#define   USB_CTRL_EBRIDGE_ESTOP_SCB_REQ_MASK		0x00020000 /* option */
-#define   USB_CTRL_EBRIDGE_EBR_SCB_SIZE_MASK		0x00000f80 /* option */
+#define   USB_CTRL_EBRIDGE_EBR_SCB_SIZE_MASK		GENMASK(11, 7) /* option */
+#define   USB_CTRL_EBRIDGE_ESTOP_SCB_REQ_MASK		BIT(17) /* option */
 #define USB_CTRL_OBRIDGE		0x10
-#define   USB_CTRL_OBRIDGE_LS_KEEP_ALIVE_MASK		0x08000000
+#define   USB_CTRL_OBRIDGE_LS_KEEP_ALIVE_MASK		BIT(27)
 #define USB_CTRL_MDIO			0x14
 #define USB_CTRL_MDIO2			0x18
 #define USB_CTRL_UTMI_CTL_1		0x2c
-#define   USB_CTRL_UTMI_CTL_1_POWER_UP_FSM_EN_MASK	0x00000800
-#define   USB_CTRL_UTMI_CTL_1_POWER_UP_FSM_EN_P1_MASK	0x08000000
+#define   USB_CTRL_UTMI_CTL_1_POWER_UP_FSM_EN_MASK	BIT(11)
+#define   USB_CTRL_UTMI_CTL_1_POWER_UP_FSM_EN_P1_MASK	BIT(27)
 #define USB_CTRL_USB_PM			0x34
-#define   USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK		0x00800000 /* option */
-#define   USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK		0x00400000 /* option */
-#define   USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK	0x40000000 /* option */
-#define   USB_CTRL_USB_PM_USB_PWRDN_MASK		0x80000000 /* option */
-#define   USB_CTRL_USB_PM_SOFT_RESET_MASK		0x40000000 /* option */
-#define   USB_CTRL_USB_PM_USB20_HC_RESETB_MASK		0x30000000 /* option */
-#define   USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK	0x00300000 /* option */
-#define   USB_CTRL_USB_PM_RMTWKUP_EN_MASK		0x00000001
+#define   USB_CTRL_USB_PM_RMTWKUP_EN_MASK		BIT(0)
+#define   USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK	GENMASK(21, 20) /* option */
+#define   USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK		BIT(22) /* option */
+#define   USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK		BIT(23) /* option */
+#define   USB_CTRL_USB_PM_USB20_HC_RESETB_MASK		GENMASK(29, 28) /* option */
+#define   USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK	BIT(30) /* option */
+#define   USB_CTRL_USB_PM_SOFT_RESET_MASK		BIT(30) /* option */
+#define   USB_CTRL_USB_PM_USB_PWRDN_MASK		BIT(31) /* option */
 #define USB_CTRL_USB_PM_STATUS		0x38
 #define USB_CTRL_USB30_CTL1		0x60
-#define   USB_CTRL_USB30_CTL1_PHY3_PLL_SEQ_START_MASK	0x00000010
-#define   USB_CTRL_USB30_CTL1_PHY3_RESETB_MASK		0x00010000
-#define   USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK	0x00020000 /* option */
-#define   USB_CTRL_USB30_CTL1_USB3_IOC_MASK		0x10000000 /* option */
-#define   USB_CTRL_USB30_CTL1_USB3_IPP_MASK		0x20000000 /* option */
+#define   USB_CTRL_USB30_CTL1_PHY3_PLL_SEQ_START_MASK	BIT(4)
+#define   USB_CTRL_USB30_CTL1_PHY3_RESETB_MASK		BIT(16)
+#define   USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK	BIT(17) /* option */
+#define   USB_CTRL_USB30_CTL1_USB3_IOC_MASK		BIT(28) /* option */
+#define   USB_CTRL_USB30_CTL1_USB3_IPP_MASK		BIT(29) /* option */
 #define USB_CTRL_USB30_PCTL		0x70
-#define   USB_CTRL_USB30_PCTL_PHY3_SOFT_RESETB_MASK	0x00000002
-#define   USB_CTRL_USB30_PCTL_PHY3_IDDQ_OVERRIDE_MASK	0x00008000
-#define   USB_CTRL_USB30_PCTL_PHY3_SOFT_RESETB_P1_MASK	0x00020000
+#define   USB_CTRL_USB30_PCTL_PHY3_SOFT_RESETB_MASK	BIT(1)
+#define   USB_CTRL_USB30_PCTL_PHY3_IDDQ_OVERRIDE_MASK	BIT(15)
+#define   USB_CTRL_USB30_PCTL_PHY3_SOFT_RESETB_P1_MASK	BIT(17)
 #define USB_CTRL_USB_DEVICE_CTL1	0x90
-#define   USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK	0x00000003 /* option */
+#define   USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK	GENMASK(1, 0) /* option */
 
 /* Register definitions for the XHCI EC block */
 #define USB_XHCI_EC_IRAADR 0x658
-- 
2.7.4

Re: [PATCH 3/6] phy: usb: Migrate to BIT and BITMASK macros

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]



On 10/5/2022 2:30 PM, justinpopo6@gmail.com wrote:
> From: Justin Chen <justinpopo6@gmail.com>
> 
> Using BIT and BITMASK macros makes it much easier to read and make
> modifications. Also reordered some constants to be in numerical order.
> 
> Signed-off-by: Justin Chen <justinpopo6@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

[PATCH 4/6] phy: usb: Disable phy auto-suspend

[justinpopo6]

From: Justin Chen <justinpopo6@gmail.com>

The BDC block requires the PLL lock in order to grab the PLL clock.
The phy auto-suspend feature turns off the phy when nothing is attached
leading to the PLL to not lock. This leads the BDC block to grab the AUX
clock instead of the PLL clock. This is not ideal, so lets turn this
feature off.

Signed-off-by: Justin Chen <justinpopo6@gmail.com>
---
 drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
index 26e9585e..6a4d478 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
+++ b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
@@ -62,6 +62,7 @@
 
 /* Register definitions for the USB_PHY block in 7211b0 */
 #define USB_PHY_PLL_CTL			0x00
+#define   USB_PHY_PLL_CTL_PLL_SUSPEND_MASK		BIT(27)
 #define   USB_PHY_PLL_CTL_PLL_RESETB_MASK		BIT(30)
 #define USB_PHY_PLL_LDO_CTL		0x08
 #define   USB_PHY_PLL_LDO_CTL_AFE_BG_PWRDWNB_MASK	BIT(0)
@@ -259,6 +260,11 @@ static void usb_init_common_7211b0(struct brcm_usb_init_params *params)
 		brcm_usb_writel(reg, usb_phy + USB_PHY_UTMI_CTL_1);
 	}
 
+	/* Disable PLL auto suspend */
+	reg = brcm_usb_readl(usb_phy + USB_PHY_PLL_CTL);
+	reg |= USB_PHY_PLL_CTL_PLL_SUSPEND_MASK;
+	brcm_usb_writel(reg, usb_phy + USB_PHY_PLL_CTL);
+
 	/* Init the PHY */
 	reg = USB_PHY_PLL_LDO_CTL_AFE_CORERDY_MASK |
 		USB_PHY_PLL_LDO_CTL_AFE_LDO_PWRDWNB_MASK |
-- 
2.7.4

Re: [PATCH 4/6] phy: usb: Disable phy auto-suspend

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 524 bytes --]



On 10/5/2022 2:30 PM, justinpopo6@gmail.com wrote:
> From: Justin Chen <justinpopo6@gmail.com>
> 
> The BDC block requires the PLL lock in order to grab the PLL clock.
> The phy auto-suspend feature turns off the phy when nothing is attached
> leading to the PLL to not lock. This leads the BDC block to grab the AUX
> clock instead of the PLL clock. This is not ideal, so lets turn this
> feature off.
> 
> Signed-off-by: Justin Chen <justinpopo6@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

[PATCH 5/6] phy: usb: Use slow clock for wake enabled suspend

[justinpopo6]

From: Justin Chen <justinpopo6@gmail.com>

The logic was incorrect when switching to slow clock. We want the slow
clock if wake_enabled is set.

Fixes: ae532b2b7aa5 ("usb: Add "wake on" functionality for newer Synopsis XHCI controllers")
Signed-off-by: Justin Chen <justinpopo6@gmail.com>
---
 drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
index 6a4d478..f78cff2 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
+++ b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
@@ -337,13 +337,12 @@ static void usb_uninit_common_7216(struct brcm_usb_init_params *params)
 
 	pr_debug("%s\n", __func__);
 
-	if (!params->wake_enabled) {
-		USB_CTRL_SET(ctrl, USB_PM, USB_PWRDN);
-
+	if (params->wake_enabled) {
 		/* Switch to using slower clock during suspend to save power */
 		USB_CTRL_SET(ctrl, USB_PM, XHC_S2_CLK_SWITCH_EN);
-	} else {
 		usb_wake_enable_7216(params, true);
+	} else {
+		USB_CTRL_SET(ctrl, USB_PM, USB_PWRDN);
 	}
 }
 
-- 
2.7.4

Re: [PATCH 5/6] phy: usb: Use slow clock for wake enabled suspend

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]



On 10/5/2022 2:30 PM, justinpopo6@gmail.com wrote:
> From: Justin Chen <justinpopo6@gmail.com>
> 
> The logic was incorrect when switching to slow clock. We want the slow
> clock if wake_enabled is set.
> 
> Fixes: ae532b2b7aa5 ("usb: Add "wake on" functionality for newer Synopsis XHCI controllers")
> Signed-off-by: Justin Chen <justinpopo6@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

[PATCH 6/6] phy: usb: Fix clock imbalance for suspend/resume

[justinpopo6]

From: Justin Chen <justinpopo6@gmail.com>

We should be disabling clocks when wake from USB is not needed. Since
this wasn't done, we had a clock imbalance since clocks were always
being enabled on resume.

Fixes: ae532b2b7aa5 ("usb: Add "wake on" functionality for newer Synopsis XHCI controllers")
Fixes: b0c0b66c0b43 ("phy: usb: Add support for wake and USB low power mode for 7211 S2/S5")
Signed-off-by: Justin Chen <justinpopo6@gmail.com>
---
 drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c | 2 --
 drivers/phy/broadcom/phy-brcm-usb-init.h          | 1 -
 drivers/phy/broadcom/phy-brcm-usb.c               | 8 +++++---
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
index f78cff2..76cf428 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
+++ b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
@@ -430,7 +430,6 @@ void brcm_usb_dvr_init_7216(struct brcm_usb_init_params *params)
 
 	params->family_name = "7216";
 	params->ops = &bcm7216_ops;
-	params->suspend_with_clocks = true;
 }
 
 void brcm_usb_dvr_init_7211b0(struct brcm_usb_init_params *params)
@@ -440,5 +439,4 @@ void brcm_usb_dvr_init_7211b0(struct brcm_usb_init_params *params)
 
 	params->family_name = "7211";
 	params->ops = &bcm7211b0_ops;
-	params->suspend_with_clocks = true;
 }
diff --git a/drivers/phy/broadcom/phy-brcm-usb-init.h b/drivers/phy/broadcom/phy-brcm-usb-init.h
index bedf2b8..f9fbf8f 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init.h
+++ b/drivers/phy/broadcom/phy-brcm-usb-init.h
@@ -62,7 +62,6 @@ struct  brcm_usb_init_params {
 	const struct brcm_usb_init_ops *ops;
 	struct regmap *syscon_piarbctl;
 	bool wake_enabled;
-	bool suspend_with_clocks;
 };
 
 void brcm_usb_dvr_init_4908(struct brcm_usb_init_params *params);
diff --git a/drivers/phy/broadcom/phy-brcm-usb.c b/drivers/phy/broadcom/phy-brcm-usb.c
index d97fa58..aafba4a 100644
--- a/drivers/phy/broadcom/phy-brcm-usb.c
+++ b/drivers/phy/broadcom/phy-brcm-usb.c
@@ -602,7 +602,7 @@ static int brcm_usb_phy_suspend(struct device *dev)
 		 * and newer XHCI->2.0-clks/3.0-clks.
 		 */
 
-		if (!priv->ini.suspend_with_clocks) {
+		if (!priv->ini.wake_enabled) {
 			if (priv->phys[BRCM_USB_PHY_3_0].inited)
 				clk_disable_unprepare(priv->usb_30_clk);
 			if (priv->phys[BRCM_USB_PHY_2_0].inited ||
@@ -619,8 +619,10 @@ static int brcm_usb_phy_resume(struct device *dev)
 {
 	struct brcm_usb_phy_data *priv = dev_get_drvdata(dev);
 
-	clk_prepare_enable(priv->usb_20_clk);
-	clk_prepare_enable(priv->usb_30_clk);
+	if (!priv->ini.wake_enabled) {
+		clk_prepare_enable(priv->usb_20_clk);
+		clk_prepare_enable(priv->usb_30_clk);
+	}
 	brcm_usb_init_ipp(&priv->ini);
 
 	/*
-- 
2.7.4

Re: [PATCH 6/6] phy: usb: Fix clock imbalance for suspend/resume

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]



On 10/5/2022 2:30 PM, justinpopo6@gmail.com wrote:
> From: Justin Chen <justinpopo6@gmail.com>
> 
> We should be disabling clocks when wake from USB is not needed. Since
> this wasn't done, we had a clock imbalance since clocks were always
> being enabled on resume.
> 
> Fixes: ae532b2b7aa5 ("usb: Add "wake on" functionality for newer Synopsis XHCI controllers")
> Fixes: b0c0b66c0b43 ("phy: usb: Add support for wake and USB low power mode for 7211 S2/S5")
> Signed-off-by: Justin Chen <justinpopo6@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH 0/6] Various bug fixes/improvements for phy-brcm-usb driver

[Justin Chen]

On Wed, Oct 5, 2022 at 2:30 PM <justinpopo6@gmail.com> wrote:
>
> From: Justin Chen <justin.chen@broadcom.com>
>
> A few suspend/resume bug fixes with clocks and wake counter. Also improve
> port mode selection.
>
> Al Cooper (1):
>   phy: usb: s2 WoL wakeup_count not incremented for USB->Eth devices
>
> Justin Chen (5):
>   phy: usb: Improve port mode selection
>   phy: usb: Migrate to BIT and BITMASK macros
>   phy: usb: Disable phy auto-suspend
>   phy: usb: Use slow clock for wake enabled suspend
>   phy: usb: Fix clock imbalance for suspend/resume
>
>  drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c | 95 ++++++++++++-----------
>  drivers/phy/broadcom/phy-brcm-usb-init.c          | 90 ++++++++++-----------
>  drivers/phy/broadcom/phy-brcm-usb-init.h          | 11 ++-
>  drivers/phy/broadcom/phy-brcm-usb.c               | 28 ++++---
>  4 files changed, 116 insertions(+), 108 deletions(-)
>
> --
> 2.7.4
>

Friendly ping. Any updates on this patch set?

Thanks,
Justin

Re: [PATCH 0/6] Various bug fixes/improvements for phy-brcm-usb driver

[Vinod Koul]

On 05-10-22, 14:30, justinpopo6@gmail.com wrote:
> From: Justin Chen <justin.chen@broadcom.com>
> 
> A few suspend/resume bug fixes with clocks and wake counter. Also improve
> port mode selection.

Pls order fixes first, that way they can go into -rc.

I have applied these, thanks

-- 
~Vinod