[PATCH 1/1] firmware: imx: scu-pd: do not power off console domain

[PATCH 1/1] firmware: imx: scu-pd: do not power off console domain

[Dong Aisheng]

Do not power off console domain in runtime pm.

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/firmware/imx/scu-pd.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
index b556612207e5..770e3681bbbf 100644
--- a/drivers/firmware/imx/scu-pd.c
+++ b/drivers/firmware/imx/scu-pd.c
@@ -85,6 +85,8 @@ struct imx_sc_pd_soc {
 	u8 num_ranges;
 };
 
+int imx_con_rsrc;
+
 static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
 	/* LSIO SS */
 	{ "pwm", IMX_SC_R_PWM_0, 8, true, 0 },
@@ -173,6 +175,23 @@ to_imx_sc_pd(struct generic_pm_domain *genpd)
 	return container_of(genpd, struct imx_sc_pm_domain, pd);
 }
 
+static void imx_sc_pd_get_console_rsrc(void)
+{
+	struct of_phandle_args specs;
+	int ret;
+
+	if (!of_stdout)
+		return;
+
+	ret = of_parse_phandle_with_args(of_stdout, "power-domains",
+					 "#power-domain-cells",
+					 0, &specs);
+	if (ret)
+		return;
+
+	imx_con_rsrc = specs.args[0];
+}
+
 static int imx_sc_pd_power(struct generic_pm_domain *domain, bool power_on)
 {
 	struct imx_sc_msg_req_set_resource_power_mode msg;
@@ -233,6 +252,7 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
 		      const struct imx_sc_pd_range *pd_ranges)
 {
 	struct imx_sc_pm_domain *sc_pd;
+	bool is_off = true;
 	int ret;
 
 	sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
@@ -251,6 +271,10 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
 			 "%s", pd_ranges->name);
 
 	sc_pd->pd.name = sc_pd->name;
+	if (imx_con_rsrc == sc_pd->rsrc) {
+		sc_pd->pd.flags = GENPD_FLAG_RPM_ALWAYS_ON;
+		is_off = false;
+	}
 
 	if (sc_pd->rsrc >= IMX_SC_R_LAST) {
 		dev_warn(dev, "invalid pd %s rsrc id %d found",
@@ -260,7 +284,7 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
 		return NULL;
 	}
 
-	ret = pm_genpd_init(&sc_pd->pd, NULL, true);
+	ret = pm_genpd_init(&sc_pd->pd, NULL, is_off);
 	if (ret) {
 		dev_warn(dev, "failed to init pd %s rsrc id %d",
 			 sc_pd->name, sc_pd->rsrc);
@@ -326,6 +350,8 @@ static int imx_sc_pd_probe(struct platform_device *pdev)
 	if (!pd_soc)
 		return -ENODEV;
 
+	imx_sc_pd_get_console_rsrc();
+
 	return imx_scu_init_pm_domains(&pdev->dev, pd_soc);
 }
 
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/1] firmware: imx: scu-pd: do not power off console domain

[kbuild test robot]

Hi Dong,

I love your patch! Perhaps something to improve:

[auto build test WARNING on shawnguo/for-next]
[also build test WARNING on v5.4-rc7 next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Dong-Aisheng/firmware-imx-scu-pd-do-not-power-off-console-domain/20191117-211100
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-32-g233d4e1-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/firmware/imx/scu-pd.c:88:5: sparse: sparse: symbol 'imx_con_rsrc' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH] firmware: imx: scu-pd: imx_con_rsrc can be static

[kbuild test robot]

Fixes: 3eb6739fc067 ("firmware: imx: scu-pd: do not power off console domain")
Signed-off-by: kbuild test robot <lkp@intel.com>
---
 scu-pd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
index 770e3681bbbf6..cae678feda645 100644
--- a/drivers/firmware/imx/scu-pd.c
+++ b/drivers/firmware/imx/scu-pd.c
@@ -85,7 +85,7 @@ struct imx_sc_pd_soc {
 	u8 num_ranges;
 };
 
-int imx_con_rsrc;
+static int imx_con_rsrc;
 
 static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
 	/* LSIO SS */

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/1] firmware: imx: scu-pd: do not power off console domain

[Ulf Hansson]

On Sun, 17 Nov 2019 at 14:07, Dong Aisheng <aisheng.dong@nxp.com> wrote:
>
> Do not power off console domain in runtime pm.

Would it be possible to configure a remote wakeup for the console
while the device becomes runtime suspended, rather than preventing the
power off altogether?

Kind regards
Uffe

>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
>  drivers/firmware/imx/scu-pd.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
> index b556612207e5..770e3681bbbf 100644
> --- a/drivers/firmware/imx/scu-pd.c
> +++ b/drivers/firmware/imx/scu-pd.c
> @@ -85,6 +85,8 @@ struct imx_sc_pd_soc {
>         u8 num_ranges;
>  };
>
> +int imx_con_rsrc;
> +
>  static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
>         /* LSIO SS */
>         { "pwm", IMX_SC_R_PWM_0, 8, true, 0 },
> @@ -173,6 +175,23 @@ to_imx_sc_pd(struct generic_pm_domain *genpd)
>         return container_of(genpd, struct imx_sc_pm_domain, pd);
>  }
>
> +static void imx_sc_pd_get_console_rsrc(void)
> +{
> +       struct of_phandle_args specs;
> +       int ret;
> +
> +       if (!of_stdout)
> +               return;
> +
> +       ret = of_parse_phandle_with_args(of_stdout, "power-domains",
> +                                        "#power-domain-cells",
> +                                        0, &specs);
> +       if (ret)
> +               return;
> +
> +       imx_con_rsrc = specs.args[0];
> +}
> +
>  static int imx_sc_pd_power(struct generic_pm_domain *domain, bool power_on)
>  {
>         struct imx_sc_msg_req_set_resource_power_mode msg;
> @@ -233,6 +252,7 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
>                       const struct imx_sc_pd_range *pd_ranges)
>  {
>         struct imx_sc_pm_domain *sc_pd;
> +       bool is_off = true;
>         int ret;
>
>         sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
> @@ -251,6 +271,10 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
>                          "%s", pd_ranges->name);
>
>         sc_pd->pd.name = sc_pd->name;
> +       if (imx_con_rsrc == sc_pd->rsrc) {
> +               sc_pd->pd.flags = GENPD_FLAG_RPM_ALWAYS_ON;
> +               is_off = false;
> +       }
>
>         if (sc_pd->rsrc >= IMX_SC_R_LAST) {
>                 dev_warn(dev, "invalid pd %s rsrc id %d found",
> @@ -260,7 +284,7 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
>                 return NULL;
>         }
>
> -       ret = pm_genpd_init(&sc_pd->pd, NULL, true);
> +       ret = pm_genpd_init(&sc_pd->pd, NULL, is_off);
>         if (ret) {
>                 dev_warn(dev, "failed to init pd %s rsrc id %d",
>                          sc_pd->name, sc_pd->rsrc);
> @@ -326,6 +350,8 @@ static int imx_sc_pd_probe(struct platform_device *pdev)
>         if (!pd_soc)
>                 return -ENODEV;
>
> +       imx_sc_pd_get_console_rsrc();
> +
>         return imx_scu_init_pm_domains(&pdev->dev, pd_soc);
>  }
>
> --
> 2.23.0
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [EXT] Re: [PATCH 1/1] firmware: imx: scu-pd: do not power off console domain

[Andy Duan]

From: Dong Aisheng <dongas86@gmail.com> Sent: Thursday, January 2, 2020 6:03 PM
> On Tue, Nov 19, 2019 at 12:13 AM Ulf Hansson <ulf.hansson@linaro.org>
> wrote:
> >
> > On Sun, 17 Nov 2019 at 14:07, Dong Aisheng <aisheng.dong@nxp.com>
> wrote:
> > >
> > > Do not power off console domain in runtime pm.
> >
> > Would it be possible to configure a remote wakeup for the console
> > while the device becomes runtime suspended, rather than preventing the
> > power off altogether?

From HW view, the wakeup features requires power on, but clocks can be off.

In fact, the standard flow is power domain driver should not power off the devices
that enable the wakeup feature.

NXP internal tree already supported runtime pm. I will upstream the patch.

Andy
> >
> 
> Remote wakeup usually requires clock and power.
> Not sure if this device can support it. Copy module owner (Fugang
> Duan) to confirm.
> 
> The current lpuart driver seems not support.
> 
> Regards
> Aisheng
> 
> > Kind regards
> > Uffe
> >
> > >
> > > Cc: Shawn Guo <shawnguo@kernel.org>
> > > Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> > > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > > ---
> > >  drivers/firmware/imx/scu-pd.c | 28 +++++++++++++++++++++++++++-
> > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/firmware/imx/scu-pd.c
> > > b/drivers/firmware/imx/scu-pd.c index b556612207e5..770e3681bbbf
> > > 100644
> > > --- a/drivers/firmware/imx/scu-pd.c
> > > +++ b/drivers/firmware/imx/scu-pd.c
> > > @@ -85,6 +85,8 @@ struct imx_sc_pd_soc {
> > >         u8 num_ranges;
> > >  };
> > >
> > > +int imx_con_rsrc;
> > > +
> > >  static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
> > >         /* LSIO SS */
> > >         { "pwm", IMX_SC_R_PWM_0, 8, true, 0 }, @@ -173,6 +175,23
> @@
> > > to_imx_sc_pd(struct generic_pm_domain *genpd)
> > >         return container_of(genpd, struct imx_sc_pm_domain, pd);  }
> > >
> > > +static void imx_sc_pd_get_console_rsrc(void) {
> > > +       struct of_phandle_args specs;
> > > +       int ret;
> > > +
> > > +       if (!of_stdout)
> > > +               return;
> > > +
> > > +       ret = of_parse_phandle_with_args(of_stdout, "power-domains",
> > > +
> "#power-domain-cells",
> > > +                                        0, &specs);
> > > +       if (ret)
> > > +               return;
> > > +
> > > +       imx_con_rsrc = specs.args[0]; }
> > > +
> > >  static int imx_sc_pd_power(struct generic_pm_domain *domain, bool
> > > power_on)  {
> > >         struct imx_sc_msg_req_set_resource_power_mode msg; @@
> -233,6
> > > +252,7 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
> > >                       const struct imx_sc_pd_range *pd_ranges)  {
> > >         struct imx_sc_pm_domain *sc_pd;
> > > +       bool is_off = true;
> > >         int ret;
> > >
> > >         sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL); @@
> > > -251,6 +271,10 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
> > >                          "%s", pd_ranges->name);
> > >
> > >         sc_pd->pd.name = sc_pd->name;
> > > +       if (imx_con_rsrc == sc_pd->rsrc) {
> > > +               sc_pd->pd.flags = GENPD_FLAG_RPM_ALWAYS_ON;
> > > +               is_off = false;
> > > +       }
> > >
> > >         if (sc_pd->rsrc >= IMX_SC_R_LAST) {
> > >                 dev_warn(dev, "invalid pd %s rsrc id %d found", @@
> > > -260,7 +284,7 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
> > >                 return NULL;
> > >         }
> > >
> > > -       ret = pm_genpd_init(&sc_pd->pd, NULL, true);
> > > +       ret = pm_genpd_init(&sc_pd->pd, NULL, is_off);
> > >         if (ret) {
> > >                 dev_warn(dev, "failed to init pd %s rsrc id %d",
> > >                          sc_pd->name, sc_pd->rsrc); @@ -326,6
> +350,8
> > > @@ static int imx_sc_pd_probe(struct platform_device *pdev)
> > >         if (!pd_soc)
> > >                 return -ENODEV;
> > >
> > > +       imx_sc_pd_get_console_rsrc();
> > > +
> > >         return imx_scu_init_pm_domains(&pdev->dev, pd_soc);  }
> > >
> > > --
> > > 2.23.0
> > >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel