[PATCH 3/3] phy: mapphone-mdm6600: Fix pinctrl_pm handling for sleep pins

[Sebastian Reichel]

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 at gmail.com>
> Cc: Merlijn Wajer <merlijn at wizzup.org>
> Cc: Pavel Machek <pavel at ucw.cz>
> Cc: Sebastian Reichel <sre at kernel.org>
> Fixes: 2ad2af081622 ("phy: mapphone-mdm6600: Improve phy related runtime PM calls")
> Signed-off-by: Tony Lindgren <tony at 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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-phy/attachments/20230912/9ab0bf01/attachment.sig>