All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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.