2021-12-01 18:07:06

by Al Cooper

[permalink] [raw]
Subject: [PATCH 0/3] phy: phy-brcm-usb: Fixes for phy-brcm-usb driver

A few fixes for the phy-brcm-usb driver.

Al Cooper (3):
phy: usb: Leave some clocks running during suspend
usb: Add "wake on" functionality for newer Synopsis XHCI controllers
phy: broadcom: Kconfig: Fix PHY_BRCM_USB config option

drivers/phy/broadcom/Kconfig | 3 +-
.../phy/broadcom/phy-brcm-usb-init-synopsys.c | 46 +++++++++++++++----
drivers/phy/broadcom/phy-brcm-usb.c | 38 +++++++++++++++
3 files changed, 77 insertions(+), 10 deletions(-)


base-commit: f080815fdb3e3cff5a004ca83b3815ac17ef71b1
--
2.17.1



2021-12-01 18:07:11

by Al Cooper

[permalink] [raw]
Subject: [PATCH 1/3] phy: usb: Leave some clocks running during suspend

The PHY client driver does a phy_exit() call on suspend or rmmod and
the PHY driver needs to know the difference because some clocks need
to be kept running for suspend but can be shutdown on unbind/rmmod
(or if there are no PHY clients at all).

The fix is to use a PM notifier so the driver can tell if a PHY
client is calling exit() because of a system suspend or a driver
unbind/rmmod.

Signed-off-by: Al Cooper <[email protected]>
---
drivers/phy/broadcom/phy-brcm-usb.c | 38 +++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)

diff --git a/drivers/phy/broadcom/phy-brcm-usb.c b/drivers/phy/broadcom/phy-brcm-usb.c
index 116fb23aebd9..0f1deb6e0eab 100644
--- a/drivers/phy/broadcom/phy-brcm-usb.c
+++ b/drivers/phy/broadcom/phy-brcm-usb.c
@@ -18,6 +18,7 @@
#include <linux/soc/brcmstb/brcmstb.h>
#include <dt-bindings/phy/phy.h>
#include <linux/mfd/syscon.h>
+#include <linux/suspend.h>

#include "phy-brcm-usb-init.h"

@@ -70,12 +71,35 @@ struct brcm_usb_phy_data {
int init_count;
int wake_irq;
struct brcm_usb_phy phys[BRCM_USB_PHY_ID_MAX];
+ struct notifier_block pm_notifier;
+ bool pm_active;
};

static s8 *node_reg_names[BRCM_REGS_MAX] = {
"crtl", "xhci_ec", "xhci_gbl", "usb_phy", "usb_mdio", "bdc_ec"
};

+static int brcm_pm_notifier(struct notifier_block *notifier,
+ unsigned long pm_event,
+ void *unused)
+{
+ struct brcm_usb_phy_data *priv =
+ container_of(notifier, struct brcm_usb_phy_data, pm_notifier);
+
+ switch (pm_event) {
+ case PM_HIBERNATION_PREPARE:
+ case PM_SUSPEND_PREPARE:
+ priv->pm_active = true;
+ break;
+ case PM_POST_RESTORE:
+ case PM_POST_HIBERNATION:
+ case PM_POST_SUSPEND:
+ priv->pm_active = false;
+ break;
+ }
+ return NOTIFY_DONE;
+}
+
static irqreturn_t brcm_usb_phy_wake_isr(int irq, void *dev_id)
{
struct phy *gphy = dev_id;
@@ -91,6 +115,9 @@ static int brcm_usb_phy_init(struct phy *gphy)
struct brcm_usb_phy_data *priv =
container_of(phy, struct brcm_usb_phy_data, phys[phy->id]);

+ if (priv->pm_active)
+ return 0;
+
/*
* Use a lock to make sure a second caller waits until
* the base phy is inited before using it.
@@ -120,6 +147,9 @@ static int brcm_usb_phy_exit(struct phy *gphy)
struct brcm_usb_phy_data *priv =
container_of(phy, struct brcm_usb_phy_data, phys[phy->id]);

+ if (priv->pm_active)
+ return 0;
+
dev_dbg(&gphy->dev, "EXIT\n");
if (phy->id == BRCM_USB_PHY_2_0)
brcm_usb_uninit_eohci(&priv->ini);
@@ -488,6 +518,9 @@ static int brcm_usb_phy_probe(struct platform_device *pdev)
if (err)
return err;

+ priv->pm_notifier.notifier_call = brcm_pm_notifier;
+ register_pm_notifier(&priv->pm_notifier);
+
mutex_init(&priv->mutex);

/* make sure invert settings are correct */
@@ -528,7 +561,10 @@ static int brcm_usb_phy_probe(struct platform_device *pdev)

static int brcm_usb_phy_remove(struct platform_device *pdev)
{
+ struct brcm_usb_phy_data *priv = dev_get_drvdata(&pdev->dev);
+
sysfs_remove_group(&pdev->dev.kobj, &brcm_usb_phy_group);
+ unregister_pm_notifier(&priv->pm_notifier);

return 0;
}
@@ -539,6 +575,7 @@ static int brcm_usb_phy_suspend(struct device *dev)
struct brcm_usb_phy_data *priv = dev_get_drvdata(dev);

if (priv->init_count) {
+ dev_dbg(dev, "SUSPEND\n");
priv->ini.wake_enabled = device_may_wakeup(dev);
if (priv->phys[BRCM_USB_PHY_3_0].inited)
brcm_usb_uninit_xhci(&priv->ini);
@@ -578,6 +615,7 @@ static int brcm_usb_phy_resume(struct device *dev)
* Uninitialize anything that wasn't previously initialized.
*/
if (priv->init_count) {
+ dev_dbg(dev, "RESUME\n");
if (priv->wake_irq >= 0)
disable_irq_wake(priv->wake_irq);
brcm_usb_init_common(&priv->ini);
--
2.17.1


2021-12-01 18:07:13

by Al Cooper

[permalink] [raw]
Subject: [PATCH 2/3] usb: Add "wake on" functionality for newer Synopsis XHCI controllers

Add "wake on" support for the newer Synopsis based XHCI only controller.
This works on the 72165 and 72164 and newer chips and does not work
on 7216 based systems. Also switch the USB sysclk to a slower clock
on suspend to save additional power in S2. The clock switch will only
save power on the 72165b0 and newer chips and is a nop on older chips.

Signed-off-by: Al Cooper <[email protected]>
---
.../phy/broadcom/phy-brcm-usb-init-synopsys.c | 46 +++++++++++++++----
1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
index e63457e145c7..d2524b70ea16 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
+++ b/drivers/phy/broadcom/phy-brcm-usb-init-synopsys.c
@@ -47,6 +47,8 @@
#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_STATUS 0x08
#define USB_CTRL_USB_DEVICE_CTL1 0x10
#define USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK 0x00000003
@@ -190,10 +192,6 @@ static void usb_init_common(struct brcm_usb_init_params *params)

pr_debug("%s\n", __func__);

- USB_CTRL_UNSET(ctrl, USB_PM, USB_PWRDN);
- /* 1 millisecond - for USB clocks to settle down */
- usleep_range(1000, 2000);
-
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);
@@ -222,6 +220,17 @@ static void usb_wake_enable_7211b0(struct brcm_usb_init_params *params,
USB_CTRL_UNSET(ctrl, CTLR_CSHCR, ctl_pme_en);
}

+static void usb_wake_enable_7216(struct brcm_usb_init_params *params,
+ bool enable)
+{
+ void __iomem *ctrl = params->regs[BRCM_REGS_CTRL];
+
+ if (enable)
+ USB_CTRL_SET(ctrl, USB_PM, XHC_PME_EN);
+ else
+ USB_CTRL_UNSET(ctrl, USB_PM, XHC_PME_EN);
+}
+
static void usb_init_common_7211b0(struct brcm_usb_init_params *params)
{
void __iomem *ctrl = params->regs[BRCM_REGS_CTRL];
@@ -295,6 +304,20 @@ static void usb_init_common_7211b0(struct brcm_usb_init_params *params)
usb2_eye_fix_7211b0(params);
}

+static void usb_init_common_7216(struct brcm_usb_init_params *params)
+{
+ void __iomem *ctrl = params->regs[BRCM_REGS_CTRL];
+
+ USB_CTRL_UNSET(ctrl, USB_PM, XHC_S2_CLK_SWITCH_EN);
+ USB_CTRL_UNSET(ctrl, USB_PM, USB_PWRDN);
+
+ /* 1 millisecond - for USB clocks to settle down */
+ usleep_range(1000, 2000);
+
+ usb_wake_enable_7216(params, false);
+ usb_init_common(params);
+}
+
static void usb_init_xhci(struct brcm_usb_init_params *params)
{
pr_debug("%s\n", __func__);
@@ -302,14 +325,20 @@ static void usb_init_xhci(struct brcm_usb_init_params *params)
xhci_soft_reset(params, 0);
}

-static void usb_uninit_common(struct brcm_usb_init_params *params)
+static void usb_uninit_common_7216(struct brcm_usb_init_params *params)
{
void __iomem *ctrl = params->regs[BRCM_REGS_CTRL];

pr_debug("%s\n", __func__);

- USB_CTRL_SET(ctrl, USB_PM, USB_PWRDN);
+ if (!params->wake_enabled) {
+ USB_CTRL_SET(ctrl, USB_PM, USB_PWRDN);

+ /* 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);
+ }
}

static void usb_uninit_common_7211b0(struct brcm_usb_init_params *params)
@@ -371,9 +400,9 @@ static void usb_set_dual_select(struct brcm_usb_init_params *params, int mode)

static const struct brcm_usb_init_ops bcm7216_ops = {
.init_ipp = usb_init_ipp,
- .init_common = usb_init_common,
+ .init_common = usb_init_common_7216,
.init_xhci = usb_init_xhci,
- .uninit_common = usb_uninit_common,
+ .uninit_common = usb_uninit_common_7216,
.uninit_xhci = usb_uninit_xhci,
.get_dual_select = usb_get_dual_select,
.set_dual_select = usb_set_dual_select,
@@ -396,6 +425,7 @@ 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)
--
2.17.1


2021-12-01 18:07:15

by Al Cooper

[permalink] [raw]
Subject: [PATCH 3/3] phy: broadcom: Kconfig: Fix PHY_BRCM_USB config option

The previous commit 4b402fa8e0b7 ("phy: phy-brcm-usb: support PHY on
the BCM4908") added a second "default" line for ARCH_BCM_4908 above
the original "default" line for ARCH_BRCMSTB. When two "default"
lines are used, only the first is used and this change stopped
the PHY_BRCM_USB option for being enabled for ARCH_BRCMSTB.
The fix is to use one "default line with "||".

Fixes: 4b402fa8e0b7 ("phy: phy-brcm-usb: support PHY on the BCM4908")
Signed-off-by: Al Cooper <[email protected]>
---
drivers/phy/broadcom/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/phy/broadcom/Kconfig b/drivers/phy/broadcom/Kconfig
index f81e23742079..849c4204f550 100644
--- a/drivers/phy/broadcom/Kconfig
+++ b/drivers/phy/broadcom/Kconfig
@@ -97,8 +97,7 @@ config PHY_BRCM_USB
depends on OF
select GENERIC_PHY
select SOC_BRCMSTB if ARCH_BRCMSTB
- default ARCH_BCM4908
- default ARCH_BRCMSTB
+ default ARCH_BCM4908 || ARCH_BRCMSTB
help
Enable this to support the Broadcom STB USB PHY.
This driver is required by the USB XHCI, EHCI and OHCI
--
2.17.1


2021-12-01 20:42:04

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 1/3] phy: usb: Leave some clocks running during suspend

On 12/1/21 10:06 AM, Al Cooper wrote:
> The PHY client driver does a phy_exit() call on suspend or rmmod and
> the PHY driver needs to know the difference because some clocks need
> to be kept running for suspend but can be shutdown on unbind/rmmod
> (or if there are no PHY clients at all).
>
> The fix is to use a PM notifier so the driver can tell if a PHY
> client is calling exit() because of a system suspend or a driver
> unbind/rmmod.
>
> Signed-off-by: Al Cooper <[email protected]>

Acked-by: Florian Fainelli <[email protected]>
--
Florian

2021-12-01 20:42:21

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 3/3] phy: broadcom: Kconfig: Fix PHY_BRCM_USB config option

On 12/1/21 10:06 AM, Al Cooper wrote:
> The previous commit 4b402fa8e0b7 ("phy: phy-brcm-usb: support PHY on
> the BCM4908") added a second "default" line for ARCH_BCM_4908 above
> the original "default" line for ARCH_BRCMSTB. When two "default"
> lines are used, only the first is used and this change stopped
> the PHY_BRCM_USB option for being enabled for ARCH_BRCMSTB.
> The fix is to use one "default line with "||".
>
> Fixes: 4b402fa8e0b7 ("phy: phy-brcm-usb: support PHY on the BCM4908")
> Signed-off-by: Al Cooper <[email protected]>

Acked-by: Florian Fainelli <[email protected]>

Thanks Al!
--
Florian

2021-12-01 20:43:41

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 2/3] usb: Add "wake on" functionality for newer Synopsis XHCI controllers

On 12/1/21 10:06 AM, Al Cooper wrote:
> Add "wake on" support for the newer Synopsis based XHCI only controller.
> This works on the 72165 and 72164 and newer chips and does not work
> on 7216 based systems. Also switch the USB sysclk to a slower clock
> on suspend to save additional power in S2. The clock switch will only
> save power on the 72165b0 and newer chips and is a nop on older chips.
>
> Signed-off-by: Al Cooper <[email protected]>

Acked-by: Florian Fainelli <[email protected]>
--
Florian

2021-12-01 22:26:01

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 3/3] phy: broadcom: Kconfig: Fix PHY_BRCM_USB config option

On 01.12.2021 19:06, Al Cooper wrote:
> The previous commit 4b402fa8e0b7 ("phy: phy-brcm-usb: support PHY on
> the BCM4908") added a second "default" line for ARCH_BCM_4908 above
> the original "default" line for ARCH_BRCMSTB. When two "default"
> lines are used, only the first is used and this change stopped
> the PHY_BRCM_USB option for being enabled for ARCH_BRCMSTB.
> The fix is to use one "default line with "||".
>
> Fixes: 4b402fa8e0b7 ("phy: phy-brcm-usb: support PHY on the BCM4908")
> Signed-off-by: Al Cooper <[email protected]>

Acked-by: Rafał Miłecki <[email protected]>

2021-12-02 04:37:03

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 1/3] phy: usb: Leave some clocks running during suspend

On 01-12-21, 13:06, Al Cooper wrote:
> The PHY client driver does a phy_exit() call on suspend or rmmod and
> the PHY driver needs to know the difference because some clocks need
> to be kept running for suspend but can be shutdown on unbind/rmmod
> (or if there are no PHY clients at all).
>
> The fix is to use a PM notifier so the driver can tell if a PHY
> client is calling exit() because of a system suspend or a driver
> unbind/rmmod.
>
> Signed-off-by: Al Cooper <[email protected]>
> ---
> drivers/phy/broadcom/phy-brcm-usb.c | 38 +++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/drivers/phy/broadcom/phy-brcm-usb.c b/drivers/phy/broadcom/phy-brcm-usb.c
> index 116fb23aebd9..0f1deb6e0eab 100644
> --- a/drivers/phy/broadcom/phy-brcm-usb.c
> +++ b/drivers/phy/broadcom/phy-brcm-usb.c
> @@ -18,6 +18,7 @@
> #include <linux/soc/brcmstb/brcmstb.h>
> #include <dt-bindings/phy/phy.h>
> #include <linux/mfd/syscon.h>
> +#include <linux/suspend.h>
>
> #include "phy-brcm-usb-init.h"
>
> @@ -70,12 +71,35 @@ struct brcm_usb_phy_data {
> int init_count;
> int wake_irq;
> struct brcm_usb_phy phys[BRCM_USB_PHY_ID_MAX];
> + struct notifier_block pm_notifier;
> + bool pm_active;
> };
>
> static s8 *node_reg_names[BRCM_REGS_MAX] = {
> "crtl", "xhci_ec", "xhci_gbl", "usb_phy", "usb_mdio", "bdc_ec"
> };
>
> +static int brcm_pm_notifier(struct notifier_block *notifier,
> + unsigned long pm_event,
> + void *unused)
> +{
> + struct brcm_usb_phy_data *priv =
> + container_of(notifier, struct brcm_usb_phy_data, pm_notifier);
> +
> + switch (pm_event) {
> + case PM_HIBERNATION_PREPARE:
> + case PM_SUSPEND_PREPARE:
> + priv->pm_active = true;
> + break;
> + case PM_POST_RESTORE:
> + case PM_POST_HIBERNATION:
> + case PM_POST_SUSPEND:
> + priv->pm_active = false;
> + break;
> + }
> + return NOTIFY_DONE;
> +}
> +
> static irqreturn_t brcm_usb_phy_wake_isr(int irq, void *dev_id)
> {
> struct phy *gphy = dev_id;
> @@ -91,6 +115,9 @@ static int brcm_usb_phy_init(struct phy *gphy)
> struct brcm_usb_phy_data *priv =
> container_of(phy, struct brcm_usb_phy_data, phys[phy->id]);
>
> + if (priv->pm_active)
> + return 0;
> +
> /*
> * Use a lock to make sure a second caller waits until
> * the base phy is inited before using it.
> @@ -120,6 +147,9 @@ static int brcm_usb_phy_exit(struct phy *gphy)
> struct brcm_usb_phy_data *priv =
> container_of(phy, struct brcm_usb_phy_data, phys[phy->id]);
>
> + if (priv->pm_active)
> + return 0;
> +
> dev_dbg(&gphy->dev, "EXIT\n");
> if (phy->id == BRCM_USB_PHY_2_0)
> brcm_usb_uninit_eohci(&priv->ini);
> @@ -488,6 +518,9 @@ static int brcm_usb_phy_probe(struct platform_device *pdev)
> if (err)
> return err;
>
> + priv->pm_notifier.notifier_call = brcm_pm_notifier;
> + register_pm_notifier(&priv->pm_notifier);
> +
> mutex_init(&priv->mutex);
>
> /* make sure invert settings are correct */
> @@ -528,7 +561,10 @@ static int brcm_usb_phy_probe(struct platform_device *pdev)
>
> static int brcm_usb_phy_remove(struct platform_device *pdev)
> {
> + struct brcm_usb_phy_data *priv = dev_get_drvdata(&pdev->dev);
> +
> sysfs_remove_group(&pdev->dev.kobj, &brcm_usb_phy_group);
> + unregister_pm_notifier(&priv->pm_notifier);
>
> return 0;
> }
> @@ -539,6 +575,7 @@ static int brcm_usb_phy_suspend(struct device *dev)
> struct brcm_usb_phy_data *priv = dev_get_drvdata(dev);
>
> if (priv->init_count) {
> + dev_dbg(dev, "SUSPEND\n");

debug artifact?

> priv->ini.wake_enabled = device_may_wakeup(dev);
> if (priv->phys[BRCM_USB_PHY_3_0].inited)
> brcm_usb_uninit_xhci(&priv->ini);
> @@ -578,6 +615,7 @@ static int brcm_usb_phy_resume(struct device *dev)
> * Uninitialize anything that wasn't previously initialized.
> */
> if (priv->init_count) {
> + dev_dbg(dev, "RESUME\n");

here too

> if (priv->wake_irq >= 0)
> disable_irq_wake(priv->wake_irq);
> brcm_usb_init_common(&priv->ini);
> --
> 2.17.1

--
~Vinod

2022-01-06 21:00:28

by Al Cooper

[permalink] [raw]
Subject: Re: [PATCH 1/3] phy: usb: Leave some clocks running during suspend

On Wed, Dec 1, 2021 at 11:35 PM Vinod Koul <[email protected]> wrote:
>
> On 01-12-21, 13:06, Al Cooper wrote:
> > The PHY client driver does a phy_exit() call on suspend or rmmod and
> > the PHY driver needs to know the difference because some clocks need
> > to be kept running for suspend but can be shutdown on unbind/rmmod
> > (or if there are no PHY clients at all).
> >
> > The fix is to use a PM notifier so the driver can tell if a PHY
> > client is calling exit() because of a system suspend or a driver
> > unbind/rmmod.
> >
> > Signed-off-by: Al Cooper <[email protected]>
> > ---
> > drivers/phy/broadcom/phy-brcm-usb.c | 38 +++++++++++++++++++++++++++++
> > 1 file changed, 38 insertions(+)
> >
> > diff --git a/drivers/phy/broadcom/phy-brcm-usb.c b/drivers/phy/broadcom/phy-brcm-usb.c
> > index 116fb23aebd9..0f1deb6e0eab 100644
> > --- a/drivers/phy/broadcom/phy-brcm-usb.c
> > +++ b/drivers/phy/broadcom/phy-brcm-usb.c
> > @@ -18,6 +18,7 @@
> > #include <linux/soc/brcmstb/brcmstb.h>
> > #include <dt-bindings/phy/phy.h>
> > #include <linux/mfd/syscon.h>
> > +#include <linux/suspend.h>
> >
> > #include "phy-brcm-usb-init.h"
> >
> > @@ -70,12 +71,35 @@ struct brcm_usb_phy_data {
> > int init_count;
> > int wake_irq;
> > struct brcm_usb_phy phys[BRCM_USB_PHY_ID_MAX];
> > + struct notifier_block pm_notifier;
> > + bool pm_active;
> > };
> >
> > static s8 *node_reg_names[BRCM_REGS_MAX] = {
> > "crtl", "xhci_ec", "xhci_gbl", "usb_phy", "usb_mdio", "bdc_ec"
> > };
> >
> > +static int brcm_pm_notifier(struct notifier_block *notifier,
> > + unsigned long pm_event,
> > + void *unused)
> > +{
> > + struct brcm_usb_phy_data *priv =
> > + container_of(notifier, struct brcm_usb_phy_data, pm_notifier);
> > +
> > + switch (pm_event) {
> > + case PM_HIBERNATION_PREPARE:
> > + case PM_SUSPEND_PREPARE:
> > + priv->pm_active = true;
> > + break;
> > + case PM_POST_RESTORE:
> > + case PM_POST_HIBERNATION:
> > + case PM_POST_SUSPEND:
> > + priv->pm_active = false;
> > + break;
> > + }
> > + return NOTIFY_DONE;
> > +}
> > +
> > static irqreturn_t brcm_usb_phy_wake_isr(int irq, void *dev_id)
> > {
> > struct phy *gphy = dev_id;
> > @@ -91,6 +115,9 @@ static int brcm_usb_phy_init(struct phy *gphy)
> > struct brcm_usb_phy_data *priv =
> > container_of(phy, struct brcm_usb_phy_data, phys[phy->id]);
> >
> > + if (priv->pm_active)
> > + return 0;
> > +
> > /*
> > * Use a lock to make sure a second caller waits until
> > * the base phy is inited before using it.
> > @@ -120,6 +147,9 @@ static int brcm_usb_phy_exit(struct phy *gphy)
> > struct brcm_usb_phy_data *priv =
> > container_of(phy, struct brcm_usb_phy_data, phys[phy->id]);
> >
> > + if (priv->pm_active)
> > + return 0;
> > +
> > dev_dbg(&gphy->dev, "EXIT\n");
> > if (phy->id == BRCM_USB_PHY_2_0)
> > brcm_usb_uninit_eohci(&priv->ini);
> > @@ -488,6 +518,9 @@ static int brcm_usb_phy_probe(struct platform_device *pdev)
> > if (err)
> > return err;
> >
> > + priv->pm_notifier.notifier_call = brcm_pm_notifier;
> > + register_pm_notifier(&priv->pm_notifier);
> > +
> > mutex_init(&priv->mutex);
> >
> > /* make sure invert settings are correct */
> > @@ -528,7 +561,10 @@ static int brcm_usb_phy_probe(struct platform_device *pdev)
> >
> > static int brcm_usb_phy_remove(struct platform_device *pdev)
> > {
> > + struct brcm_usb_phy_data *priv = dev_get_drvdata(&pdev->dev);
> > +
> > sysfs_remove_group(&pdev->dev.kobj, &brcm_usb_phy_group);
> > + unregister_pm_notifier(&priv->pm_notifier);
> >
> > return 0;
> > }
> > @@ -539,6 +575,7 @@ static int brcm_usb_phy_suspend(struct device *dev)
> > struct brcm_usb_phy_data *priv = dev_get_drvdata(dev);
> >
> > if (priv->init_count) {
> > + dev_dbg(dev, "SUSPEND\n");
>
> debug artifact?
>
> > priv->ini.wake_enabled = device_may_wakeup(dev);
> > if (priv->phys[BRCM_USB_PHY_3_0].inited)
> > brcm_usb_uninit_xhci(&priv->ini);
> > @@ -578,6 +615,7 @@ static int brcm_usb_phy_resume(struct device *dev)
> > * Uninitialize anything that wasn't previously initialized.
> > */
> > if (priv->init_count) {
> > + dev_dbg(dev, "RESUME\n");
>
> here too

I left these in intentionally because they are very useful for debug.

>
> > if (priv->wake_irq >= 0)
> > disable_irq_wake(priv->wake_irq);
> > brcm_usb_init_common(&priv->ini);
> > --
> > 2.17.1
>
> --
> ~Vinod

2022-01-21 20:11:32

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 0/3] phy: phy-brcm-usb: Fixes for phy-brcm-usb driver

Hi Vinod,

On 12/1/21 10:06 AM, Al Cooper wrote:
> A few fixes for the phy-brcm-usb driver.
>
> Al Cooper (3):
> phy: usb: Leave some clocks running during suspend
> usb: Add "wake on" functionality for newer Synopsis XHCI controllers
> phy: broadcom: Kconfig: Fix PHY_BRCM_USB config option

Are you able to pick up those patches? Thank you

>
> drivers/phy/broadcom/Kconfig | 3 +-
> .../phy/broadcom/phy-brcm-usb-init-synopsys.c | 46 +++++++++++++++----
> drivers/phy/broadcom/phy-brcm-usb.c | 38 +++++++++++++++
> 3 files changed, 77 insertions(+), 10 deletions(-)
>
>
> base-commit: f080815fdb3e3cff5a004ca83b3815ac17ef71b1
>


--
Florian

2022-01-24 08:42:49

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 0/3] phy: phy-brcm-usb: Fixes for phy-brcm-usb driver

On 19-01-22, 15:26, Florian Fainelli wrote:
> Hi Vinod,
>
> On 12/1/21 10:06 AM, Al Cooper wrote:
> > A few fixes for the phy-brcm-usb driver.
> >
> > Al Cooper (3):
> > phy: usb: Leave some clocks running during suspend
> > usb: Add "wake on" functionality for newer Synopsis XHCI controllers
> > phy: broadcom: Kconfig: Fix PHY_BRCM_USB config option
>
> Are you able to pick up those patches? Thank you

Applied phy patches to phy-fixes, thanks

--
~Vinod

2022-01-29 05:55:17

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 0/3] phy: phy-brcm-usb: Fixes for phy-brcm-usb driver



On 1/23/2022 3:45 AM, Vinod Koul wrote:
> On 19-01-22, 15:26, Florian Fainelli wrote:
>> Hi Vinod,
>>
>> On 12/1/21 10:06 AM, Al Cooper wrote:
>>> A few fixes for the phy-brcm-usb driver.
>>>
>>> Al Cooper (3):
>>> phy: usb: Leave some clocks running during suspend
>>> usb: Add "wake on" functionality for newer Synopsis XHCI controllers
>>> phy: broadcom: Kconfig: Fix PHY_BRCM_USB config option
>>
>> Are you able to pick up those patches? Thank you
>
> Applied phy patches to phy-fixes, thanks

Thanks! I can see patches 1 and 3 there but patch 2 is not in phy-fixes
or your next branch. Can you pick it up if it is satisfactory? Thanks!
--
Florian

2022-02-02 09:14:36

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 0/3] phy: phy-brcm-usb: Fixes for phy-brcm-usb driver

On 27-01-22, 17:01, Florian Fainelli wrote:
>
>
> On 1/23/2022 3:45 AM, Vinod Koul wrote:
> > On 19-01-22, 15:26, Florian Fainelli wrote:
> > > Hi Vinod,
> > >
> > > On 12/1/21 10:06 AM, Al Cooper wrote:
> > > > A few fixes for the phy-brcm-usb driver.
> > > >
> > > > Al Cooper (3):
> > > > phy: usb: Leave some clocks running during suspend
> > > > usb: Add "wake on" functionality for newer Synopsis XHCI controllers
> > > > phy: broadcom: Kconfig: Fix PHY_BRCM_USB config option
> > >
> > > Are you able to pick up those patches? Thank you
> >
> > Applied phy patches to phy-fixes, thanks
>
> Thanks! I can see patches 1 and 3 there but patch 2 is not in phy-fixes or
> your next branch. Can you pick it up if it is satisfactory? Thanks!

That is usb patch right. I wont pick unless Greg acks it to go thru phy
tree

--
~Vinod

2022-02-02 22:36:35

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 0/3] phy: phy-brcm-usb: Fixes for phy-brcm-usb driver



On 2/1/2022 8:57 PM, Vinod Koul wrote:
> On 27-01-22, 17:01, Florian Fainelli wrote:
>>
>>
>> On 1/23/2022 3:45 AM, Vinod Koul wrote:
>>> On 19-01-22, 15:26, Florian Fainelli wrote:
>>>> Hi Vinod,
>>>>
>>>> On 12/1/21 10:06 AM, Al Cooper wrote:
>>>>> A few fixes for the phy-brcm-usb driver.
>>>>>
>>>>> Al Cooper (3):
>>>>> phy: usb: Leave some clocks running during suspend
>>>>> usb: Add "wake on" functionality for newer Synopsis XHCI controllers
>>>>> phy: broadcom: Kconfig: Fix PHY_BRCM_USB config option
>>>>
>>>> Are you able to pick up those patches? Thank you
>>>
>>> Applied phy patches to phy-fixes, thanks
>>
>> Thanks! I can see patches 1 and 3 there but patch 2 is not in phy-fixes or
>> your next branch. Can you pick it up if it is satisfactory? Thanks!
>
> That is usb patch right. I wont pick unless Greg acks it to go thru phy
> tree

This is an USB PHY patch, I suppose the subject including "usb: " did
somewhat mislead you. Here is the diffstat just for that patch:

.../phy/broadcom/phy-brcm-usb-init-synopsys.c | 46 +++++++++++++++----
1 file changed, 38 insertions(+), 8 deletions(-)

definitively falling into the PHY subsystem.
--
Florian

2022-02-03 20:39:11

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 0/3] phy: phy-brcm-usb: Fixes for phy-brcm-usb driver

On 02-02-22, 08:24, Florian Fainelli wrote:
>
>
> On 2/1/2022 8:57 PM, Vinod Koul wrote:
> > On 27-01-22, 17:01, Florian Fainelli wrote:
> > >
> > >
> > > On 1/23/2022 3:45 AM, Vinod Koul wrote:
> > > > On 19-01-22, 15:26, Florian Fainelli wrote:
> > > > > Hi Vinod,
> > > > >
> > > > > On 12/1/21 10:06 AM, Al Cooper wrote:
> > > > > > A few fixes for the phy-brcm-usb driver.
> > > > > >
> > > > > > Al Cooper (3):
> > > > > > phy: usb: Leave some clocks running during suspend
> > > > > > usb: Add "wake on" functionality for newer Synopsis XHCI controllers
> > > > > > phy: broadcom: Kconfig: Fix PHY_BRCM_USB config option
> > > > >
> > > > > Are you able to pick up those patches? Thank you
> > > >
> > > > Applied phy patches to phy-fixes, thanks
> > >
> > > Thanks! I can see patches 1 and 3 there but patch 2 is not in phy-fixes or
> > > your next branch. Can you pick it up if it is satisfactory? Thanks!
> >
> > That is usb patch right. I wont pick unless Greg acks it to go thru phy
> > tree
>
> This is an USB PHY patch, I suppose the subject including "usb: " did
> somewhat mislead you. Here is the diffstat just for that patch:
>
> .../phy/broadcom/phy-brcm-usb-init-synopsys.c | 46 +++++++++++++++----
> 1 file changed, 38 insertions(+), 8 deletions(-)
>
> definitively falling into the PHY subsystem.

Yeah that was it! Can you please change the title and resend...

> --
> Florian

--
~Vinod