diff mbox series

[v6] soc: fsl: enable acpi support in RCPM driver

Message ID 20210312085822.12136-1-ran.wang_1@nxp.com (mailing list archive)
State Not Applicable
Headers show
Series [v6] soc: fsl: enable acpi support in RCPM driver | expand
Related show

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (91966823812efbd175f904599e5cf2a854b39809)
snowpatch_ozlabs/build-ppc64le success Build succeeded
snowpatch_ozlabs/build-ppc64be success Build succeeded
snowpatch_ozlabs/build-ppc64e success Build succeeded
snowpatch_ozlabs/build-pmac32 success Build succeeded
snowpatch_ozlabs/checkpatch success total: 0 errors, 0 warnings, 0 checks, 42 lines checked
snowpatch_ozlabs/needsstable success Patch has no Fixes tags

Commit Message

Ran Wang March 12, 2021, 8:58 a.m. UTC 
From: Peng Ma <peng.ma@nxp.com>

This patch enables ACPI support in RCPM driver.

Signed-off-by: Peng Ma <peng.ma@nxp.com>
Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
---
Change in v6:
 - Remove copyright udpate to rebase on latest mainline

Change in v5:
 - Fix panic when dev->of_node is null

Change in v4:
 - Make commit subject more accurate
 - Remove unrelated new blank line

Change in v3:
 - Add #ifdef CONFIG_ACPI for acpi_device_id
 - Rename rcpm_acpi_imx_ids to rcpm_acpi_ids

Change in v2:
 - Update acpi_device_id to fix conflict with other driver

 drivers/soc/fsl/rcpm.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Leo Li April 6, 2021, 9:45 p.m. UTC | #1 
On Fri, Mar 12, 2021 at 2:56 AM Ran Wang <ran.wang_1@nxp.com> wrote:
>
> From: Peng Ma <peng.ma@nxp.com>
>
> This patch enables ACPI support in RCPM driver.
>
> Signed-off-by: Peng Ma <peng.ma@nxp.com>
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> ---
> Change in v6:
>  - Remove copyright udpate to rebase on latest mainline
>
> Change in v5:
>  - Fix panic when dev->of_node is null
>
> Change in v4:
>  - Make commit subject more accurate
>  - Remove unrelated new blank line
>
> Change in v3:
>  - Add #ifdef CONFIG_ACPI for acpi_device_id
>  - Rename rcpm_acpi_imx_ids to rcpm_acpi_ids
>
> Change in v2:
>  - Update acpi_device_id to fix conflict with other driver
>
>  drivers/soc/fsl/rcpm.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
> index 4ace28cab314..7aa997b932d1 100644
> --- a/drivers/soc/fsl/rcpm.c
> +++ b/drivers/soc/fsl/rcpm.c
> @@ -13,6 +13,7 @@
>  #include <linux/slab.h>
>  #include <linux/suspend.h>
>  #include <linux/kernel.h>
> +#include <linux/acpi.h>
>
>  #define RCPM_WAKEUP_CELL_MAX_SIZE      7
>
> @@ -78,10 +79,14 @@ static int rcpm_pm_prepare(struct device *dev)
>                                 "fsl,rcpm-wakeup", value,
>                                 rcpm->wakeup_cells + 1);
>
> -               /*  Wakeup source should refer to current rcpm device */
> -               if (ret || (np->phandle != value[0]))
> +               if (ret)
>                         continue;
>
> +               if (is_of_node(dev->fwnode))
> +                       /*  Should refer to current rcpm device */
> +                       if (np->phandle != value[0])
> +                               continue;

It looks like that we assume that in the ACPI scenario there will only
be one RCPM controller and all devices are controlled by this single
PM controller.  This probably is true for all existing SoCs with a
RCPM.  But since the driver tried to support multiple RCPMs, maybe we
should continue to support multiple RCPM controllers or at least
mention that in the comment.

> +
>                 /* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the
>                  * number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup"
>                  * of wakeup source IP contains an integer array: <phandle to
> @@ -172,10 +177,19 @@ static const struct of_device_id rcpm_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rcpm_of_match);
>
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id rcpm_acpi_ids[] = {
> +       {"NXP0015",},
> +       { }
> +};
> +MODULE_DEVICE_TABLE(acpi, rcpm_acpi_ids);
> +#endif
> +
>  static struct platform_driver rcpm_driver = {
>         .driver = {
>                 .name = "rcpm",
>                 .of_match_table = rcpm_of_match,
> +               .acpi_match_table = ACPI_PTR(rcpm_acpi_ids),
>                 .pm     = &rcpm_pm_ops,
>         },
>         .probe = rcpm_probe,
> --
> 2.25.1
>
Ran Wang April 7, 2021, 1:31 a.m. UTC | #2 
Hi Leo,

On Wednesday, April 7, 2021 5:45 AM, Li Yang wrote:
> 
> On Fri, Mar 12, 2021 at 2:56 AM Ran Wang <ran.wang_1@nxp.com> wrote:
> >
> > From: Peng Ma <peng.ma@nxp.com>
> >
> > This patch enables ACPI support in RCPM driver.
> >
> > Signed-off-by: Peng Ma <peng.ma@nxp.com>
> > Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> > ---
> > Change in v6:
> >  - Remove copyright udpate to rebase on latest mainline
> >
> > Change in v5:
> >  - Fix panic when dev->of_node is null
> >
> > Change in v4:
> >  - Make commit subject more accurate
> >  - Remove unrelated new blank line
> >
> > Change in v3:
> >  - Add #ifdef CONFIG_ACPI for acpi_device_id
> >  - Rename rcpm_acpi_imx_ids to rcpm_acpi_ids
> >
> > Change in v2:
> >  - Update acpi_device_id to fix conflict with other driver
> >
> >  drivers/soc/fsl/rcpm.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c index
> > 4ace28cab314..7aa997b932d1 100644
> > --- a/drivers/soc/fsl/rcpm.c
> > +++ b/drivers/soc/fsl/rcpm.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/suspend.h>
> >  #include <linux/kernel.h>
> > +#include <linux/acpi.h>
> >
> >  #define RCPM_WAKEUP_CELL_MAX_SIZE      7
> >
> > @@ -78,10 +79,14 @@ static int rcpm_pm_prepare(struct device *dev)
> >                                 "fsl,rcpm-wakeup", value,
> >                                 rcpm->wakeup_cells + 1);
> >
> > -               /*  Wakeup source should refer to current rcpm device */
> > -               if (ret || (np->phandle != value[0]))
> > +               if (ret)
> >                         continue;
> >
> > +               if (is_of_node(dev->fwnode))
> > +                       /*  Should refer to current rcpm device */
> > +                       if (np->phandle != value[0])
> > +                               continue;
> 
> It looks like that we assume that in the ACPI scenario there will only be one RCPM controller and all devices are controlled by this single
> PM controller.  This probably is true for all existing SoCs with a RCPM.  But since the driver tried to support multiple RCPMs, maybe we
> should continue to support multiple RCPM controllers or at least mention that in the comment.

How about adding some comment as below:

/* For ACPI mode, currently we assume there is only one RCPM controller existing */

Regards,
Ran

> 
> > +
> >                 /* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the
> >                  * number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup"
> >                  * of wakeup source IP contains an integer array:
> > <phandle to @@ -172,10 +177,19 @@ static const struct of_device_id
> > rcpm_of_match[] = {  };  MODULE_DEVICE_TABLE(of, rcpm_of_match);
> >
> > +#ifdef CONFIG_ACPI
> > +static const struct acpi_device_id rcpm_acpi_ids[] = {
> > +       {"NXP0015",},
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(acpi, rcpm_acpi_ids); #endif
> > +
> >  static struct platform_driver rcpm_driver = {
> >         .driver = {
> >                 .name = "rcpm",
> >                 .of_match_table = rcpm_of_match,
> > +               .acpi_match_table = ACPI_PTR(rcpm_acpi_ids),
> >                 .pm     = &rcpm_pm_ops,
> >         },
> >         .probe = rcpm_probe,
> > --
> > 2.25.1
> >
Leo Li April 7, 2021, 3:38 a.m. UTC | #3 
> -----Original Message-----
> From: Ran Wang <ran.wang_1@nxp.com>
> Sent: Tuesday, April 6, 2021 8:32 PM
> To: Leo Li <leoyang.li@nxp.com>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>; linuxppc-dev
> <linuxppc-dev@lists.ozlabs.org>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; lkml <linux-
> kernel@vger.kernel.org>
> Subject: RE: [PATCH v6] soc: fsl: enable acpi support in RCPM driver
> 
> Hi Leo,
> 
> On Wednesday, April 7, 2021 5:45 AM, Li Yang wrote:
> >
> > On Fri, Mar 12, 2021 at 2:56 AM Ran Wang <ran.wang_1@nxp.com> wrote:
> > >
> > > From: Peng Ma <peng.ma@nxp.com>
> > >
> > > This patch enables ACPI support in RCPM driver.
> > >
> > > Signed-off-by: Peng Ma <peng.ma@nxp.com>
> > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> > > ---
> > > Change in v6:
> > >  - Remove copyright udpate to rebase on latest mainline
> > >
> > > Change in v5:
> > >  - Fix panic when dev->of_node is null
> > >
> > > Change in v4:
> > >  - Make commit subject more accurate
> > >  - Remove unrelated new blank line
> > >
> > > Change in v3:
> > >  - Add #ifdef CONFIG_ACPI for acpi_device_id
> > >  - Rename rcpm_acpi_imx_ids to rcpm_acpi_ids
> > >
> > > Change in v2:
> > >  - Update acpi_device_id to fix conflict with other driver
> > >
> > >  drivers/soc/fsl/rcpm.c | 18 ++++++++++++++++--
> > >  1 file changed, 16 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c index
> > > 4ace28cab314..7aa997b932d1 100644
> > > --- a/drivers/soc/fsl/rcpm.c
> > > +++ b/drivers/soc/fsl/rcpm.c
> > > @@ -13,6 +13,7 @@
> > >  #include <linux/slab.h>
> > >  #include <linux/suspend.h>
> > >  #include <linux/kernel.h>
> > > +#include <linux/acpi.h>
> > >
> > >  #define RCPM_WAKEUP_CELL_MAX_SIZE      7
> > >
> > > @@ -78,10 +79,14 @@ static int rcpm_pm_prepare(struct device *dev)
> > >                                 "fsl,rcpm-wakeup", value,
> > >                                 rcpm->wakeup_cells + 1);
> > >
> > > -               /*  Wakeup source should refer to current rcpm device */
> > > -               if (ret || (np->phandle != value[0]))
> > > +               if (ret)
> > >                         continue;
> > >
> > > +               if (is_of_node(dev->fwnode))
> > > +                       /*  Should refer to current rcpm device */

Better to be /* Only handle devices with fsl,rcpm-wakeup pointing to the current rcpm node*/
> > > +                       if (np->phandle != value[0])
> > > +                               continue;
> >
> > It looks like that we assume that in the ACPI scenario there will only
> > be one RCPM controller and all devices are controlled by this single
> > PM controller.  This probably is true for all existing SoCs with a RCPM.  But
> since the driver tried to support multiple RCPMs, maybe we should continue
> to support multiple RCPM controllers or at least mention that in the
> comment.
> 
> How about adding some comment as below:
> 
> /* For ACPI mode, currently we assume there is only one RCPM controller
> existing */

Ok.  On the other hand, it will be clearer to update the existing comment above.

> 
> Regards,
> Ran
> 
> >
> > > +
> > >                 /* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the
> > >                  * number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup"
> > >                  * of wakeup source IP contains an integer array:
> > > <phandle to @@ -172,10 +177,19 @@ static const struct of_device_id
> > > rcpm_of_match[] = {  };  MODULE_DEVICE_TABLE(of, rcpm_of_match);
> > >
> > > +#ifdef CONFIG_ACPI
> > > +static const struct acpi_device_id rcpm_acpi_ids[] = {
> > > +       {"NXP0015",},
> > > +       { }
> > > +};
> > > +MODULE_DEVICE_TABLE(acpi, rcpm_acpi_ids); #endif
> > > +
> > >  static struct platform_driver rcpm_driver = {
> > >         .driver = {
> > >                 .name = "rcpm",
> > >                 .of_match_table = rcpm_of_match,
> > > +               .acpi_match_table = ACPI_PTR(rcpm_acpi_ids),
> > >                 .pm     = &rcpm_pm_ops,
> > >         },
> > >         .probe = rcpm_probe,
> > > --
> > > 2.25.1
> > >
diff mbox series

Patch

diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
index 4ace28cab314..7aa997b932d1 100644
--- a/drivers/soc/fsl/rcpm.c
+++ b/drivers/soc/fsl/rcpm.c
@@ -13,6 +13,7 @@ 
 #include <linux/slab.h>
 #include <linux/suspend.h>
 #include <linux/kernel.h>
+#include <linux/acpi.h>
 
 #define RCPM_WAKEUP_CELL_MAX_SIZE	7
 
@@ -78,10 +79,14 @@  static int rcpm_pm_prepare(struct device *dev)
 				"fsl,rcpm-wakeup", value,
 				rcpm->wakeup_cells + 1);
 
-		/*  Wakeup source should refer to current rcpm device */
-		if (ret || (np->phandle != value[0]))
+		if (ret)
 			continue;
 
+		if (is_of_node(dev->fwnode))
+			/*  Should refer to current rcpm device */
+			if (np->phandle != value[0])
+				continue;
+
 		/* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the
 		 * number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup"
 		 * of wakeup source IP contains an integer array: <phandle to
@@ -172,10 +177,19 @@  static const struct of_device_id rcpm_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, rcpm_of_match);
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id rcpm_acpi_ids[] = {
+	{"NXP0015",},
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, rcpm_acpi_ids);
+#endif
+
 static struct platform_driver rcpm_driver = {
 	.driver = {
 		.name = "rcpm",
 		.of_match_table = rcpm_of_match,
+		.acpi_match_table = ACPI_PTR(rcpm_acpi_ids),
 		.pm	= &rcpm_pm_ops,
 	},
 	.probe = rcpm_probe,