* [PATCH 1/3] phy: mapphone-mdm6600: Fix runtime disable on probe
@ 2023-09-11 3:54 Tony Lindgren
2023-09-11 3:54 ` [PATCH 2/3] phy: mapphone-mdm6600: Fix runtime PM for remove Tony Lindgren
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Tony Lindgren @ 2023-09-11 3:54 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Miaoqian Lin
Cc: Ivaylo Dimitrov, Merlijn Wajer, Pavel Machek, Sebastian Reichel,
linux-phy
Commit d644e0d79829 ("phy: mapphone-mdm6600: Fix PM error handling in
phy_mdm6600_probe") caused a regression where we now unconditionally
disable runtime PM at the end of the probe while it is only needed on
errors.
Cc: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Cc: Merlijn Wajer <merlijn@wizzup.org>
Cc: Miaoqian Lin <linmq006@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Sebastian Reichel <sre@kernel.org>
Fixes: d644e0d79829 ("phy: mapphone-mdm6600: Fix PM error handling in phy_mdm6600_probe")
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/phy/motorola/phy-mapphone-mdm6600.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
--- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -627,10 +627,12 @@ static int phy_mdm6600_probe(struct platform_device *pdev)
pm_runtime_put_autosuspend(ddata->dev);
cleanup:
- if (error < 0)
+ if (error < 0) {
phy_mdm6600_device_power_off(ddata);
- pm_runtime_disable(ddata->dev);
- pm_runtime_dont_use_autosuspend(ddata->dev);
+ pm_runtime_disable(ddata->dev);
+ pm_runtime_dont_use_autosuspend(ddata->dev);
+ }
+
return error;
}
--
2.42.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] phy: mapphone-mdm6600: Fix runtime PM for remove
2023-09-11 3:54 [PATCH 1/3] phy: mapphone-mdm6600: Fix runtime disable on probe Tony Lindgren
@ 2023-09-11 3:54 ` Tony Lindgren
2023-09-12 15:00 ` Sebastian Reichel
2023-09-11 3:54 ` [PATCH 3/3] phy: mapphone-mdm6600: Fix pinctrl_pm handling for sleep pins Tony Lindgren
2023-09-12 15:00 ` [PATCH 1/3] phy: mapphone-mdm6600: Fix runtime disable on probe Sebastian Reichel
2 siblings, 1 reply; 8+ messages in thread
From: Tony Lindgren @ 2023-09-11 3:54 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I
Cc: Ivaylo Dimitrov, Merlijn Wajer, Miaoqian Lin, Pavel Machek,
Sebastian Reichel, linux-phy, Kishon Vijay Abraham I
Otherwise we will get an underflow on remove.
Cc: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Cc: Merlijn Wajer <merlijn@wizzup.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Sebastian Reichel <sre@kernel.org>
Fixes: f7f50b2a7b05 ("phy: mapphone-mdm6600: Add runtime PM support for n_gsm on USB suspend")
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/phy/motorola/phy-mapphone-mdm6600.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
--- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -640,6 +640,11 @@ static void phy_mdm6600_remove(struct platform_device *pdev)
{
struct phy_mdm6600 *ddata = platform_get_drvdata(pdev);
struct gpio_desc *reset_gpio = ddata->ctrl_gpios[PHY_MDM6600_RESET];
+ int error;
+
+ error = pm_runtime_resume_and_get(ddata->dev);
+ if (error)
+ return;
pm_runtime_dont_use_autosuspend(ddata->dev);
pm_runtime_put_sync(ddata->dev);
--
2.42.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] phy: mapphone-mdm6600: Fix pinctrl_pm handling for sleep pins
2023-09-11 3:54 [PATCH 1/3] phy: mapphone-mdm6600: Fix runtime disable on probe Tony Lindgren
2023-09-11 3:54 ` [PATCH 2/3] phy: mapphone-mdm6600: Fix runtime PM for remove Tony Lindgren
@ 2023-09-11 3:54 ` Tony Lindgren
2023-09-12 15:14 ` Sebastian Reichel
2023-09-12 15:00 ` [PATCH 1/3] phy: mapphone-mdm6600: Fix runtime disable on probe Sebastian Reichel
2 siblings, 1 reply; 8+ messages in thread
From: Tony Lindgren @ 2023-09-11 3:54 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I
Cc: Ivaylo Dimitrov, Merlijn Wajer, Miaoqian Lin, Pavel Machek,
Sebastian Reichel, linux-phy, Kishon Vijay Abraham I
Looks like the driver sleep pins configuration is unusable. Adding the
sleep pins causes the usb phy to not respond. We need to use the default
pins in probe, and only set sleep pins at phy_mdm6600_device_power_off().
The sleep pins are needed as otherwise the modem hardware can wake up even
with the phy driver unloaded as the reset gpio pin can glitch during the
deeper SoC idle states.
Cc: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Cc: Merlijn Wajer <merlijn@wizzup.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Sebastian Reichel <sre@kernel.org>
Fixes: 2ad2af081622 ("phy: mapphone-mdm6600: Improve phy related runtime PM calls")
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/phy/motorola/phy-mapphone-mdm6600.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
--- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -456,6 +456,7 @@ static void phy_mdm6600_device_power_off(struct phy_mdm6600 *ddata)
{
struct gpio_desc *reset_gpio =
ddata->ctrl_gpios[PHY_MDM6600_RESET];
+ int error;
ddata->enabled = false;
phy_mdm6600_cmd(ddata, PHY_MDM6600_CMD_BP_SHUTDOWN_REQ);
@@ -471,6 +472,11 @@ static void phy_mdm6600_device_power_off(struct phy_mdm6600 *ddata)
} else {
dev_err(ddata->dev, "Timed out powering down\n");
}
+
+ error = pinctrl_pm_select_sleep_state(ddata->dev);
+ if (error)
+ dev_warn(ddata->dev, "%s: error with sleep_state: %i\n",
+ __func__, error);
}
static void phy_mdm6600_deferred_power_on(struct work_struct *work)
@@ -571,12 +577,6 @@ static int phy_mdm6600_probe(struct platform_device *pdev)
ddata->dev = &pdev->dev;
platform_set_drvdata(pdev, ddata);
- /* Active state selected in phy_mdm6600_power_on() */
- error = pinctrl_pm_select_sleep_state(ddata->dev);
- if (error)
- dev_warn(ddata->dev, "%s: error with sleep_state: %i\n",
- __func__, error);
-
error = phy_mdm6600_init_lines(ddata);
if (error)
return error;
--
2.42.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] phy: mapphone-mdm6600: Fix runtime PM for remove
2023-09-11 3:54 ` [PATCH 2/3] phy: mapphone-mdm6600: Fix runtime PM for remove Tony Lindgren
@ 2023-09-12 15:00 ` Sebastian Reichel
2023-09-13 4:36 ` Tony Lindgren
0 siblings, 1 reply; 8+ messages in thread
From: Sebastian Reichel @ 2023-09-12 15:00 UTC (permalink / raw)
To: Tony Lindgren
Cc: Vinod Koul, Kishon Vijay Abraham I, Ivaylo Dimitrov,
Merlijn Wajer, Miaoqian Lin, Pavel Machek, linux-phy,
Kishon Vijay Abraham I
[-- Attachment #1.1: Type: text/plain, Size: 1308 bytes --]
Hi,
On Mon, Sep 11, 2023 at 06:54:56AM +0300, Tony Lindgren wrote:
> Otherwise we will get an underflow on remove.
>
> Cc: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Cc: Merlijn Wajer <merlijn@wizzup.org>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Sebastian Reichel <sre@kernel.org>
> Fixes: f7f50b2a7b05 ("phy: mapphone-mdm6600: Add runtime PM support for n_gsm on USB suspend")
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> drivers/phy/motorola/phy-mapphone-mdm6600.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> --- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
> +++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> @@ -640,6 +640,11 @@ static void phy_mdm6600_remove(struct platform_device *pdev)
> {
> struct phy_mdm6600 *ddata = platform_get_drvdata(pdev);
> struct gpio_desc *reset_gpio = ddata->ctrl_gpios[PHY_MDM6600_RESET];
> + int error;
> +
> + error = pm_runtime_resume_and_get(ddata->dev);
> + if (error)
> + return;
Isn't this just about the use counters, i.e.
pm_runtime_get_noresume(); should be enough?
-- Sebastian
> pm_runtime_dont_use_autosuspend(ddata->dev);
> pm_runtime_put_sync(ddata->dev);
> --
> 2.42.0
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 112 bytes --]
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] phy: mapphone-mdm6600: Fix runtime disable on probe
2023-09-11 3:54 [PATCH 1/3] phy: mapphone-mdm6600: Fix runtime disable on probe Tony Lindgren
2023-09-11 3:54 ` [PATCH 2/3] phy: mapphone-mdm6600: Fix runtime PM for remove Tony Lindgren
2023-09-11 3:54 ` [PATCH 3/3] phy: mapphone-mdm6600: Fix pinctrl_pm handling for sleep pins Tony Lindgren
@ 2023-09-12 15:00 ` Sebastian Reichel
2 siblings, 0 replies; 8+ messages in thread
From: Sebastian Reichel @ 2023-09-12 15:00 UTC (permalink / raw)
To: Tony Lindgren
Cc: Vinod Koul, Kishon Vijay Abraham I, Miaoqian Lin, Ivaylo Dimitrov,
Merlijn Wajer, Pavel Machek, linux-phy
[-- Attachment #1.1: Type: text/plain, Size: 1550 bytes --]
Hi,
On Mon, Sep 11, 2023 at 06:54:55AM +0300, Tony Lindgren wrote:
> Commit d644e0d79829 ("phy: mapphone-mdm6600: Fix PM error handling in
> phy_mdm6600_probe") caused a regression where we now unconditionally
> disable runtime PM at the end of the probe while it is only needed on
> errors.
>
> Cc: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Cc: Merlijn Wajer <merlijn@wizzup.org>
> Cc: Miaoqian Lin <linmq006@gmail.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Sebastian Reichel <sre@kernel.org>
> Fixes: d644e0d79829 ("phy: mapphone-mdm6600: Fix PM error handling in phy_mdm6600_probe")
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
-- Sebastian
> drivers/phy/motorola/phy-mapphone-mdm6600.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> --- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
> +++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> @@ -627,10 +627,12 @@ static int phy_mdm6600_probe(struct platform_device *pdev)
> pm_runtime_put_autosuspend(ddata->dev);
>
> cleanup:
> - if (error < 0)
> + if (error < 0) {
> phy_mdm6600_device_power_off(ddata);
> - pm_runtime_disable(ddata->dev);
> - pm_runtime_dont_use_autosuspend(ddata->dev);
> + pm_runtime_disable(ddata->dev);
> + pm_runtime_dont_use_autosuspend(ddata->dev);
> + }
> +
> return error;
> }
>
> --
> 2.42.0
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 112 bytes --]
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] phy: mapphone-mdm6600: Fix pinctrl_pm handling for sleep pins
2023-09-11 3:54 ` [PATCH 3/3] phy: mapphone-mdm6600: Fix pinctrl_pm handling for sleep pins Tony Lindgren
@ 2023-09-12 15:14 ` Sebastian Reichel
2023-09-13 4:59 ` Tony Lindgren
0 siblings, 1 reply; 8+ messages in thread
From: Sebastian Reichel @ 2023-09-12 15:14 UTC (permalink / raw)
To: Tony Lindgren
Cc: Vinod Koul, Kishon Vijay Abraham I, Ivaylo Dimitrov,
Merlijn Wajer, Miaoqian Lin, Pavel Machek, linux-phy,
Kishon Vijay Abraham I
[-- Attachment #1.1: Type: text/plain, Size: 2804 bytes --]
Hi,
On Mon, Sep 11, 2023 at 06:54:57AM +0300, Tony Lindgren wrote:
> Looks like the driver sleep pins configuration is unusable. Adding the
> sleep pins causes the usb phy to not respond. We need to use the default
> pins in probe, and only set sleep pins at phy_mdm6600_device_power_off().
>
> The sleep pins are needed as otherwise the modem hardware can wake up even
> with the phy driver unloaded as the reset gpio pin can glitch during the
> deeper SoC idle states.
>
> Cc: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Cc: Merlijn Wajer <merlijn@wizzup.org>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Sebastian Reichel <sre@kernel.org>
> Fixes: 2ad2af081622 ("phy: mapphone-mdm6600: Improve phy related runtime PM calls")
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
Apparently phy_power_off is not called on device removal, so I
understand the need to setup sleep pins in phy_mdm6600_device_power_off()
in addition to the exsting setup in phy_mdm6600_power_off().
But I'm a bit confused about the change required in probe(), since
phy_mdm6600_power_on() selects the default state. I wouldn't expect
any access before the phy is powered on? Anyways,
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
-- Sebastian
> drivers/phy/motorola/phy-mapphone-mdm6600.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> --- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
> +++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> @@ -456,6 +456,7 @@ static void phy_mdm6600_device_power_off(struct phy_mdm6600 *ddata)
> {
> struct gpio_desc *reset_gpio =
> ddata->ctrl_gpios[PHY_MDM6600_RESET];
> + int error;
>
> ddata->enabled = false;
> phy_mdm6600_cmd(ddata, PHY_MDM6600_CMD_BP_SHUTDOWN_REQ);
> @@ -471,6 +472,11 @@ static void phy_mdm6600_device_power_off(struct phy_mdm6600 *ddata)
> } else {
> dev_err(ddata->dev, "Timed out powering down\n");
> }
> +
> + error = pinctrl_pm_select_sleep_state(ddata->dev);
> + if (error)
> + dev_warn(ddata->dev, "%s: error with sleep_state: %i\n",
> + __func__, error);
> }
>
> static void phy_mdm6600_deferred_power_on(struct work_struct *work)
> @@ -571,12 +577,6 @@ static int phy_mdm6600_probe(struct platform_device *pdev)
> ddata->dev = &pdev->dev;
> platform_set_drvdata(pdev, ddata);
>
> - /* Active state selected in phy_mdm6600_power_on() */
> - error = pinctrl_pm_select_sleep_state(ddata->dev);
> - if (error)
> - dev_warn(ddata->dev, "%s: error with sleep_state: %i\n",
> - __func__, error);
> -
> error = phy_mdm6600_init_lines(ddata);
> if (error)
> return error;
> --
> 2.42.0
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 112 bytes --]
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] phy: mapphone-mdm6600: Fix runtime PM for remove
2023-09-12 15:00 ` Sebastian Reichel
@ 2023-09-13 4:36 ` Tony Lindgren
0 siblings, 0 replies; 8+ messages in thread
From: Tony Lindgren @ 2023-09-13 4:36 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Vinod Koul, Kishon Vijay Abraham I, Ivaylo Dimitrov,
Merlijn Wajer, Miaoqian Lin, Pavel Machek, linux-phy,
Kishon Vijay Abraham I
* Sebastian Reichel <sebastian.reichel@collabora.com> [230912 15:00]:
> Isn't this just about the use counters, i.e.
> pm_runtime_get_noresume(); should be enough?
Good point I'll check. We are toggling the out-of-bandwidth modem wake
gpio line with runtime PM, it may not matter on remove.
Regards,
Tony
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] phy: mapphone-mdm6600: Fix pinctrl_pm handling for sleep pins
2023-09-12 15:14 ` Sebastian Reichel
@ 2023-09-13 4:59 ` Tony Lindgren
0 siblings, 0 replies; 8+ messages in thread
From: Tony Lindgren @ 2023-09-13 4:59 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Vinod Koul, Kishon Vijay Abraham I, Ivaylo Dimitrov,
Merlijn Wajer, Miaoqian Lin, Pavel Machek, linux-phy,
Kishon Vijay Abraham I
* Sebastian Reichel <sebastian.reichel@collabora.com> [230912 15:14]:
> Apparently phy_power_off is not called on device removal, so I
> understand the need to setup sleep pins in phy_mdm6600_device_power_off()
> in addition to the exsting setup in phy_mdm6600_power_off().
>
> But I'm a bit confused about the change required in probe(), since
> phy_mdm6600_power_on() selects the default state. I wouldn't expect
> any access before the phy is powered on? Anyways,
Maybe we should just set the sleep state in remove and leave it out of the
phy functions. If a separate state is needed for the phy_mdm6600_power_off(),
it could be the pinctrl idle state.
We need the reset pin in probe to get the modem started. The modem may
also be started in uart mode for firmware flashing depending on how the
gpio pins are set.
Regards,
Tony
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-09-13 4:59 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-11 3:54 [PATCH 1/3] phy: mapphone-mdm6600: Fix runtime disable on probe Tony Lindgren
2023-09-11 3:54 ` [PATCH 2/3] phy: mapphone-mdm6600: Fix runtime PM for remove Tony Lindgren
2023-09-12 15:00 ` Sebastian Reichel
2023-09-13 4:36 ` Tony Lindgren
2023-09-11 3:54 ` [PATCH 3/3] phy: mapphone-mdm6600: Fix pinctrl_pm handling for sleep pins Tony Lindgren
2023-09-12 15:14 ` Sebastian Reichel
2023-09-13 4:59 ` Tony Lindgren
2023-09-12 15:00 ` [PATCH 1/3] phy: mapphone-mdm6600: Fix runtime disable on probe Sebastian Reichel
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).