| Message ID | 20240514200534.73847-1-shenwei.wang@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pwm: imx-tpm: Enable pinctrl setting for sleep state | expand |
> -----Original Message----- > From: Shenwei Wang <shenwei.wang@nxp.com> > Sent: Tuesday, May 14, 2024 3:06 PM > To: Uwe Kleine-König <ukleinek@kernel.org>; Shawn Guo > <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de> > Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam > <festevam@gmail.com>; linux-pwm@vger.kernel.org; imx@lists.linux.dev; > linux-arm-kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; > Shenwei Wang <shenwei.wang@nxp.com> > Subject: [PATCH] pwm: imx-tpm: Enable pinctrl setting for sleep state > > Apply the pinctrl setting of sleep state when system enters suspend state. > Restore to the default pinctrl setting when system resumes. > Ping @Uwe Thanks, Shenwei > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com> > --- > drivers/pwm/pwm-imx-tpm.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c index > c50ddbac43c8..19245790c67c 100644 > --- a/drivers/pwm/pwm-imx-tpm.c > +++ b/drivers/pwm/pwm-imx-tpm.c > @@ -393,7 +393,7 @@ static int pwm_imx_tpm_suspend(struct device *dev) > > clk_disable_unprepare(tpm->clk); > > - return 0; > + return pinctrl_pm_select_sleep_state(dev); > } > > static int pwm_imx_tpm_resume(struct device *dev) @@ -401,6 +401,10 @@ > static int pwm_imx_tpm_resume(struct device *dev) > struct imx_tpm_pwm_chip *tpm = dev_get_drvdata(dev); > int ret = 0; > > + ret = pinctrl_pm_select_default_state(dev); > + if (ret) > + return ret; > + > ret = clk_prepare_enable(tpm->clk); > if (ret) > dev_err(dev, "failed to prepare or enable clock: %d\n", ret); > -- > 2.34.1
> -----Original Message----- > From: Shenwei Wang > Sent: Friday, June 14, 2024 1:43 PM > To: Uwe Kleine-König <ukleinek@kernel.org>; Shawn Guo > <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de> > > -----Original Message----- > > From: Shenwei Wang <shenwei.wang@nxp.com> > > Sent: Tuesday, May 14, 2024 3:06 PM > > To: Uwe Kleine-König <ukleinek@kernel.org>; Shawn Guo > > <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de> > > Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam > > <festevam@gmail.com>; linux-pwm@vger.kernel.org; imx@lists.linux.dev; > > linux-arm-kernel@lists.infradead.org; dl-linux-imx > > <linux-imx@nxp.com>; Shenwei Wang <shenwei.wang@nxp.com> > > Subject: [PATCH] pwm: imx-tpm: Enable pinctrl setting for sleep state > > > > Apply the pinctrl setting of sleep state when system enters suspend state. > > Restore to the default pinctrl setting when system resumes. > > > > Ping @Uwe 2nd Ping. Hi Uwe, Can you please provide your feedback on this patch? Thanks, Shenwei > > Thanks, > Shenwei > > > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com> > > --- > > drivers/pwm/pwm-imx-tpm.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c > > index c50ddbac43c8..19245790c67c 100644 > > --- a/drivers/pwm/pwm-imx-tpm.c > > +++ b/drivers/pwm/pwm-imx-tpm.c > > @@ -393,7 +393,7 @@ static int pwm_imx_tpm_suspend(struct device > *dev) > > > > clk_disable_unprepare(tpm->clk); > > > > - return 0; > > + return pinctrl_pm_select_sleep_state(dev); > > } > > > > static int pwm_imx_tpm_resume(struct device *dev) @@ -401,6 +401,10 > > @@ static int pwm_imx_tpm_resume(struct device *dev) > > struct imx_tpm_pwm_chip *tpm = dev_get_drvdata(dev); > > int ret = 0; > > > > + ret = pinctrl_pm_select_default_state(dev); > > + if (ret) > > + return ret; > > + > > ret = clk_prepare_enable(tpm->clk); > > if (ret) > > dev_err(dev, "failed to prepare or enable clock: %d\n", ret); > > -- > > 2.34.1
Hello, On Tue, May 14, 2024 at 03:05:34PM -0500, Shenwei Wang wrote: > Apply the pinctrl setting of sleep state when system enters > suspend state. > Restore to the default pinctrl setting when system resumes. > > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com> > --- > drivers/pwm/pwm-imx-tpm.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c > index c50ddbac43c8..19245790c67c 100644 > --- a/drivers/pwm/pwm-imx-tpm.c > +++ b/drivers/pwm/pwm-imx-tpm.c > @@ -393,7 +393,7 @@ static int pwm_imx_tpm_suspend(struct device *dev) > > clk_disable_unprepare(tpm->clk); > > - return 0; > + return pinctrl_pm_select_sleep_state(dev); > } > > static int pwm_imx_tpm_resume(struct device *dev) > @@ -401,6 +401,10 @@ static int pwm_imx_tpm_resume(struct device *dev) > struct imx_tpm_pwm_chip *tpm = dev_get_drvdata(dev); > int ret = 0; > > + ret = pinctrl_pm_select_default_state(dev); > + if (ret) > + return ret; > + > ret = clk_prepare_enable(tpm->clk); > if (ret) > dev_err(dev, "failed to prepare or enable clock: %d\n", ret); If .resume() failed (consider clk_prepare_enable() failing), and the resume is retried later: Is there an unexpected (though maybe harmless) imbalance because pinctrl_pm_select_default_state() isn't undone in .resume()'s error path? Best regards Uwe
> -----Original Message----- > From: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > Sent: Monday, July 1, 2024 3:46 PM > To: Shenwei Wang <shenwei.wang@nxp.com> > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Pengutronix Kernel Team > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; linux- > pwm@vger.kernel.org; imx@lists.linux.dev; linux-arm- > kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com> > Subject: [EXT] Re: [PATCH] pwm: imx-tpm: Enable pinctrl setting for sleep state > > Hello, > > On Tue, May 14, 2024 at 03:05:34PM -0500, Shenwei Wang wrote: > > Apply the pinctrl setting of sleep state when system enters suspend > > state. > > Restore to the default pinctrl setting when system resumes. > > > > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com> > > --- > > drivers/pwm/pwm-imx-tpm.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c > > index c50ddbac43c8..19245790c67c 100644 > > --- a/drivers/pwm/pwm-imx-tpm.c > > +++ b/drivers/pwm/pwm-imx-tpm.c > > @@ -393,7 +393,7 @@ static int pwm_imx_tpm_suspend(struct device > *dev) > > > > clk_disable_unprepare(tpm->clk); > > > > - return 0; > > + return pinctrl_pm_select_sleep_state(dev); > > } > > > > static int pwm_imx_tpm_resume(struct device *dev) @@ -401,6 +401,10 > > @@ static int pwm_imx_tpm_resume(struct device *dev) > > struct imx_tpm_pwm_chip *tpm = dev_get_drvdata(dev); > > int ret = 0; > > > > + ret = pinctrl_pm_select_default_state(dev); > > + if (ret) > > + return ret; > > + > > ret = clk_prepare_enable(tpm->clk); > > if (ret) > > dev_err(dev, "failed to prepare or enable clock: %d\n", ret); > > If .resume() failed (consider clk_prepare_enable() failing), and the resume is > retried later: Is there an unexpected (though maybe > harmless) imbalance because pinctrl_pm_select_default_state() isn't undone > in .resume()'s error path? > If we need to take care the imbalance of pinctrl states in the error path in .resume function, should I also take care of the imbalance of clk states in the error path in .suspend function? Thank you for the review! Shenwei > Best regards > Uwe
diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c index c50ddbac43c8..19245790c67c 100644 --- a/drivers/pwm/pwm-imx-tpm.c +++ b/drivers/pwm/pwm-imx-tpm.c @@ -393,7 +393,7 @@ static int pwm_imx_tpm_suspend(struct device *dev) clk_disable_unprepare(tpm->clk); - return 0; + return pinctrl_pm_select_sleep_state(dev); } static int pwm_imx_tpm_resume(struct device *dev) @@ -401,6 +401,10 @@ static int pwm_imx_tpm_resume(struct device *dev) struct imx_tpm_pwm_chip *tpm = dev_get_drvdata(dev); int ret = 0; + ret = pinctrl_pm_select_default_state(dev); + if (ret) + return ret; + ret = clk_prepare_enable(tpm->clk); if (ret) dev_err(dev, "failed to prepare or enable clock: %d\n", ret);
Apply the pinctrl setting of sleep state when system enters suspend state. Restore to the default pinctrl setting when system resumes. Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com> --- drivers/pwm/pwm-imx-tpm.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)