From: Vinod Koul <vkoul@kernel.org>
To: Al Cooper <alcooperx@gmail.com>
Cc: linux-kernel@vger.kernel.org,
bcm-kernel-feedback-list@broadcom.com,
"Florian Fainelli" <f.fainelli@gmail.com>,
"Kishon Vijay Abraham I" <kishon@ti.com>,
linux-phy@lists.infradead.org, "Rafał Miłecki" <rafal@milecki.pl>
Subject: Re: [PATCH 1/3] phy: usb: Leave some clocks running during suspend
Date: Thu, 2 Dec 2021 10:05:46 +0530 [thread overview]
Message-ID: <YahNIinbEwB569C1@matsya> (raw)
In-Reply-To: <20211201180653.35097-2-alcooperx@gmail.com>
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 <alcooperx@gmail.com>
> ---
> 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
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
WARNING: multiple messages have this Message-ID (diff)
From: Vinod Koul <vkoul@kernel.org>
To: Al Cooper <alcooperx@gmail.com>
Cc: linux-kernel@vger.kernel.org,
bcm-kernel-feedback-list@broadcom.com,
"Florian Fainelli" <f.fainelli@gmail.com>,
"Kishon Vijay Abraham I" <kishon@ti.com>,
linux-phy@lists.infradead.org, "Rafał Miłecki" <rafal@milecki.pl>
Subject: Re: [PATCH 1/3] phy: usb: Leave some clocks running during suspend
Date: Thu, 2 Dec 2021 10:05:46 +0530 [thread overview]
Message-ID: <YahNIinbEwB569C1@matsya> (raw)
In-Reply-To: <20211201180653.35097-2-alcooperx@gmail.com>
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 <alcooperx@gmail.com>
> ---
> 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
next prev parent reply other threads:[~2021-12-02 4:35 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-01 18:06 [PATCH 0/3] phy: phy-brcm-usb: Fixes for phy-brcm-usb driver Al Cooper
2021-12-01 18:06 ` Al Cooper
2021-12-01 18:06 ` [PATCH 1/3] phy: usb: Leave some clocks running during suspend Al Cooper
2021-12-01 18:06 ` Al Cooper
2021-12-01 20:39 ` Florian Fainelli
2021-12-01 20:39 ` Florian Fainelli
2021-12-02 4:35 ` Vinod Koul [this message]
2021-12-02 4:35 ` Vinod Koul
2022-01-06 21:01 ` Alan Cooper
2022-01-06 21:01 ` Alan Cooper
2021-12-01 18:06 ` [PATCH 2/3] usb: Add "wake on" functionality for newer Synopsis XHCI controllers Al Cooper
2021-12-01 18:06 ` Al Cooper
2021-12-01 20:40 ` Florian Fainelli
2021-12-01 20:40 ` Florian Fainelli
2021-12-01 18:06 ` [PATCH 3/3] phy: broadcom: Kconfig: Fix PHY_BRCM_USB config option Al Cooper
2021-12-01 18:06 ` Al Cooper
2021-12-01 20:40 ` Florian Fainelli
2021-12-01 20:40 ` Florian Fainelli
2021-12-01 21:08 ` Rafał Miłecki
2021-12-01 21:08 ` Rafał Miłecki
2022-01-19 23:26 ` [PATCH 0/3] phy: phy-brcm-usb: Fixes for phy-brcm-usb driver Florian Fainelli
2022-01-19 23:26 ` Florian Fainelli
2022-01-23 11:45 ` Vinod Koul
2022-01-23 11:45 ` Vinod Koul
2022-01-28 1:01 ` Florian Fainelli
2022-01-28 1:01 ` Florian Fainelli
2022-02-02 4:57 ` Vinod Koul
2022-02-02 4:57 ` Vinod Koul
2022-02-02 16:24 ` Florian Fainelli
2022-02-02 16:24 ` Florian Fainelli
2022-02-03 0:12 ` Vinod Koul
2022-02-03 0:12 ` Vinod Koul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YahNIinbEwB569C1@matsya \
--to=vkoul@kernel.org \
--cc=alcooperx@gmail.com \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=f.fainelli@gmail.com \
--cc=kishon@ti.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=rafal@milecki.pl \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.