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

[Sebastian Reichel]

Hi,

On Wed, Sep 13, 2023 at 09:04:29AM +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().
> 
> As the modem can also be booted to a serial port mode for firmware
> flashing, let's make the pin changes limited to probe and remove. For
> probe, we get the default pins automatically. We only need to set the
> sleep pins in phy_mdm6600_device_power_off() to prevent the modem from
> waking up because the gpio line glitches.
> 
> If it turns out that we need a separate state for phy_mdm6600_power_on()
> and phy_mdm6600_power_off(), we can use the pinctrl idle state.
> 
> 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>
> ---
> 
> Changes since v1:
> 
> - Limit the pinctrl state changes to remove for now based on what
>   Sebastian was wondering about
> 
> ---

LGTM.

Reviewed-by: Sebastian Reichel <sebastian.reichel at collabora.com>

-- Sebastian

>  drivers/phy/motorola/phy-mapphone-mdm6600.c | 29 +++++++++------------
>  1 file changed, 12 insertions(+), 17 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
> @@ -122,16 +122,10 @@ static int phy_mdm6600_power_on(struct phy *x)
>  {
>  	struct phy_mdm6600 *ddata = phy_get_drvdata(x);
>  	struct gpio_desc *enable_gpio = ddata->ctrl_gpios[PHY_MDM6600_ENABLE];
> -	int error;
>  
>  	if (!ddata->enabled)
>  		return -ENODEV;
>  
> -	error = pinctrl_pm_select_default_state(ddata->dev);
> -	if (error)
> -		dev_warn(ddata->dev, "%s: error with default_state: %i\n",
> -			 __func__, error);
> -
>  	gpiod_set_value_cansleep(enable_gpio, 1);
>  
>  	/* Allow aggressive PM for USB, it's only needed for n_gsm port */
> @@ -160,11 +154,6 @@ static int phy_mdm6600_power_off(struct phy *x)
>  
>  	gpiod_set_value_cansleep(enable_gpio, 0);
>  
> -	error = pinctrl_pm_select_sleep_state(ddata->dev);
> -	if (error)
> -		dev_warn(ddata->dev, "%s: error with sleep_state: %i\n",
> -			 __func__, error);
> -
>  	return 0;
>  }
>  
> @@ -456,6 +445,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 +461,17 @@ static void phy_mdm6600_device_power_off(struct phy_mdm6600 *ddata)
>  	} else {
>  		dev_err(ddata->dev, "Timed out powering down\n");
>  	}
> +
> +	/*
> +	 * Keep reset gpio high with padconf internal pull-up resistor to
> +	 * prevent modem from waking up during deeper SoC idle states. The
> +	 * gpio bank lines can have glitches if not in the always-on wkup
> +	 * domain.
> +	 */
> +	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 +572,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/20230913/05079c18/attachment.sig>