| Message ID | 20230913060433.48373-3-tony@atomide.com |
|---|---|
| State | Accepted |
| Commit | 3b384cc74b00b5ac21d18e4c1efc3c1da5300971 |
| Headers | show |
| Series | [v2,1/3] phy: mapphone-mdm6600: Fix runtime disable on probe | expand |
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@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> > --- > > 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@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
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;
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@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> --- Changes since v1: - Limit the pinctrl state changes to remove for now based on what Sebastian was wondering about --- drivers/phy/motorola/phy-mapphone-mdm6600.c | 29 +++++++++------------ 1 file changed, 12 insertions(+), 17 deletions(-)