| Message ID | 1524855063-14996-3-git-send-email-aisheng.dong@nxp.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
[...] > + > +static int __init imx_sc_init_pm_domains(void) > +{ > + struct generic_pm_domain *pd; > + struct device_node *np; > + sc_err_t sci_err; > + > + if (!of_machine_is_compatible("fsl,imx8qxp")) > + return 0; > + > + sci_err = sc_ipc_get_handle(&pm_ipc_handle); > + if (sci_err != SC_ERR_NONE) { > + pr_err("imx_sc_pd: can't get sc ipc handle\n"); > + return -ENODEV; > + } > + > + for_each_matching_node(np, imx_sc_pm_domain_of_match) { > + pd = imx_sc_pm_add_one_domain(np, NULL); > + if (!IS_ERR(pd)) > + imx_sc_pm_add_subdomains(np, pd); > + } Perhaps using of_genpd_add_subdomain() may help here and possibly could avoid some open coding!? > + > + return 0; > +} > +early_initcall(imx_sc_init_pm_domains); Otherwise this looks good to me! Kind regards Uffe
Hi Ulf, Thanks for the review. > -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > Sent: Wednesday, May 9, 2018 3:16 AM > To: A.s. Dong <aisheng.dong@nxp.com> > Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng > <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn > Guo <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>; > dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki <rjw@rjwysocki.net>; > Kevin Hilman <khilman@kernel.org>; Linux PM <linux-pm@vger.kernel.org> > Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver > > [...] > > > + > > +static int __init imx_sc_init_pm_domains(void) { > > + struct generic_pm_domain *pd; > > + struct device_node *np; > > + sc_err_t sci_err; > > + > > + if (!of_machine_is_compatible("fsl,imx8qxp")) > > + return 0; > > + > > + sci_err = sc_ipc_get_handle(&pm_ipc_handle); > > + if (sci_err != SC_ERR_NONE) { > > + pr_err("imx_sc_pd: can't get sc ipc handle\n"); > > + return -ENODEV; > > + } > > + > > + for_each_matching_node(np, imx_sc_pm_domain_of_match) { > > + pd = imx_sc_pm_add_one_domain(np, NULL); > > + if (!IS_ERR(pd)) > > + imx_sc_pm_add_subdomains(np, pd); > > + } > > Perhaps using of_genpd_add_subdomain() may help here and possibly could > avoid some open coding!? > Thanks for the suggestion. I thought of it a lot and the result is that I'm not sure If it's quite suitable for i.MX cases. Currently seems there's only one user, Samsung, in kernel using that API which takes two struct of_phandle_args as arguments, parent and child. It looks needs special handling in code before using it, e.g. register both parent and child domain first, which is somehow not like i.MX flow of registration. It looks like to me not see much benefits to enforce a big change to switch to it. Or am I missed anything? If you have better idea please let me know. > > + > > + return 0; > > +} > > +early_initcall(imx_sc_init_pm_domains); > > Otherwise this looks good to me! > Thanks Regards Dong Aisheng > Kind regards > Uffe
On 24 May 2018 at 10:37, A.s. Dong <aisheng.dong@nxp.com> wrote: > Hi Ulf, > > Thanks for the review. > >> -----Original Message----- >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] >> Sent: Wednesday, May 9, 2018 3:16 AM >> To: A.s. Dong <aisheng.dong@nxp.com> >> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng >> <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn >> Guo <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>; >> dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki <rjw@rjwysocki.net>; >> Kevin Hilman <khilman@kernel.org>; Linux PM <linux-pm@vger.kernel.org> >> Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver >> >> [...] >> >> > + >> > +static int __init imx_sc_init_pm_domains(void) { >> > + struct generic_pm_domain *pd; >> > + struct device_node *np; >> > + sc_err_t sci_err; >> > + >> > + if (!of_machine_is_compatible("fsl,imx8qxp")) >> > + return 0; >> > + >> > + sci_err = sc_ipc_get_handle(&pm_ipc_handle); >> > + if (sci_err != SC_ERR_NONE) { >> > + pr_err("imx_sc_pd: can't get sc ipc handle\n"); >> > + return -ENODEV; >> > + } >> > + >> > + for_each_matching_node(np, imx_sc_pm_domain_of_match) { >> > + pd = imx_sc_pm_add_one_domain(np, NULL); >> > + if (!IS_ERR(pd)) >> > + imx_sc_pm_add_subdomains(np, pd); >> > + } >> >> Perhaps using of_genpd_add_subdomain() may help here and possibly could >> avoid some open coding!? >> > > Thanks for the suggestion. I thought of it a lot and the result is that I'm not sure > If it's quite suitable for i.MX cases. Currently seems there's only one user, Samsung, > in kernel using that API which takes two struct of_phandle_args as arguments, > parent and child. It looks needs special handling in code before using it, e.g. > register both parent and child domain first, which is somehow not like i.MX flow > of registration. It looks like to me not see much benefits to enforce a big > change to switch to it. Or am I missed anything? It's up to you. The idea was to make the code easier and a bit future proof. Perhaps that isn't suitable here. In any case, if you have a device node and all power-domain providers are listed below it, one can call for_each_child_of_node() and search for provider nodes hierarchically. I thought that is kind of nice. Something along the lines of what I done in psci_dt_set_genpd_topology(), from a patch I posted a while ago: https://patchwork.kernel.org/patch/10338399/ [...] If you decide to sticking to the existing way, anyway feel free to add: Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Uffe
Hi Ulf, > -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > Sent: Thursday, May 24, 2018 5:00 PM > To: A.s. Dong <aisheng.dong@nxp.com> > Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng > <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn > Guo <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>; > dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki <rjw@rjwysocki.net>; > Kevin Hilman <khilman@kernel.org>; Linux PM <linux-pm@vger.kernel.org> > Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver > > On 24 May 2018 at 10:37, A.s. Dong <aisheng.dong@nxp.com> wrote: > > Hi Ulf, > > > > Thanks for the review. > > > >> -----Original Message----- > >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > >> Sent: Wednesday, May 9, 2018 3:16 AM > >> To: A.s. Dong <aisheng.dong@nxp.com> > >> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng > >> <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn > Guo > >> <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>; > >> dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki > >> <rjw@rjwysocki.net>; Kevin Hilman <khilman@kernel.org>; Linux PM > >> <linux-pm@vger.kernel.org> > >> Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver > >> > >> [...] > >> > >> > + > >> > +static int __init imx_sc_init_pm_domains(void) { > >> > + struct generic_pm_domain *pd; > >> > + struct device_node *np; > >> > + sc_err_t sci_err; > >> > + > >> > + if (!of_machine_is_compatible("fsl,imx8qxp")) > >> > + return 0; > >> > + > >> > + sci_err = sc_ipc_get_handle(&pm_ipc_handle); > >> > + if (sci_err != SC_ERR_NONE) { > >> > + pr_err("imx_sc_pd: can't get sc ipc handle\n"); > >> > + return -ENODEV; > >> > + } > >> > + > >> > + for_each_matching_node(np, imx_sc_pm_domain_of_match) { > >> > + pd = imx_sc_pm_add_one_domain(np, NULL); > >> > + if (!IS_ERR(pd)) > >> > + imx_sc_pm_add_subdomains(np, pd); > >> > + } > >> > >> Perhaps using of_genpd_add_subdomain() may help here and possibly > >> could avoid some open coding!? > >> > > > > Thanks for the suggestion. I thought of it a lot and the result is > > that I'm not sure If it's quite suitable for i.MX cases. Currently > > seems there's only one user, Samsung, in kernel using that API which > > takes two struct of_phandle_args as arguments, parent and child. It looks > needs special handling in code before using it, e.g. > > register both parent and child domain first, which is somehow not like > > i.MX flow of registration. It looks like to me not see much benefits > > to enforce a big change to switch to it. Or am I missed anything? > > It's up to you. The idea was to make the code easier and a bit future proof. > Perhaps that isn't suitable here. > > In any case, if you have a device node and all power-domain providers are > listed below it, one can call for_each_child_of_node() and search for > provider nodes hierarchically. I thought that is kind of nice. > > Something along the lines of what I done in psci_dt_set_genpd_topology(), > from a patch I posted a while ago: > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat > chwork.kernel.org%2Fpatch%2F10338399%2F&data=02%7C01%7Caisheng.do > ng%40nxp.com%7C0ce43cb8f66740a0f95908d5c154c330%7C686ea1d3bc2b4c > 6fa92cd99c5c301635%7C0%7C1%7C636627492163349262&sdata=sk9U7c5VQ > WKJN1UMOMUIXZh0nnpiWGX7ildx2Zzo4dc%3D&reserved=0 > Thanks for the info. Will have a look at it. > [...] > > If you decide to sticking to the existing way, anyway feel free to add: > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Thanks Regards Dong Aisheng > Kind regards > Uffe
diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig index 4cad3c0..8d56e9b 100644 --- a/drivers/soc/imx/Kconfig +++ b/drivers/soc/imx/Kconfig @@ -14,4 +14,8 @@ config HAVE_IMX_SCU bool depends on HAVE_IMX_MU +config HAVE_IMX_SC_PD + bool + depends on HAVE_IMX_SCU + endmenu diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile index f931984..40ad2a5 100644 --- a/drivers/soc/imx/Makefile +++ b/drivers/soc/imx/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o obj-$(CONFIG_IMX7_PM_DOMAINS) += gpcv2.o obj-$(CONFIG_HAVE_IMX_MU) += imx_mu.o obj-$(CONFIG_HAVE_IMX_SCU) += sc/ +obj-$(CONFIG_HAVE_IMX_SC_PD) += sc-pd.o diff --git a/drivers/soc/imx/sc-pd.c b/drivers/soc/imx/sc-pd.c new file mode 100644 index 0000000..ecbfd2c --- /dev/null +++ b/drivers/soc/imx/sc-pd.c @@ -0,0 +1,151 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2016 Freescale Semiconductor, Inc. + * Copyright 2017~2018 NXP + * Dong Aisheng <aisheng.dong@nxp.com> + * + * Implementation of the SCU based Power Domains + * + */ + +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/platform_device.h> +#include <linux/pm.h> +#include <linux/pm_domain.h> +#include <linux/slab.h> + +#include <soc/imx/sc/sci.h> + +struct imx_sc_pm_domain { + struct generic_pm_domain pd; + sc_rsrc_t rsrc_id; +}; + +static sc_ipc_t pm_ipc_handle; + +static int imx_sc_pd_power(struct generic_pm_domain *domain, bool power_on) +{ + struct imx_sc_pm_domain *pd; + sc_err_t sci_err; + + pd = container_of(domain, struct imx_sc_pm_domain, pd); + + sci_err = sc_pm_set_resource_power_mode(pm_ipc_handle, pd->rsrc_id, + power_on ? SC_PM_PW_MODE_ON : + SC_PM_PW_MODE_LP); + if (sci_err) { + pr_err("imx_sc_pm: failed to power %s resource %d ret %d\n", + power_on ? "up" : "off", pd->rsrc_id, sci_err); + return -EPERM; + } + + return 0; +} + +static int imx_sc_pd_power_on(struct generic_pm_domain *domain) +{ + return imx_sc_pd_power(domain, true); +} + +static int imx_sc_pd_power_off(struct generic_pm_domain *domain) +{ + return imx_sc_pd_power(domain, false); +} + +static struct __init generic_pm_domain * imx_sc_pm_add_one_domain(struct device_node *np, + struct generic_pm_domain *genpd_parent) +{ + struct imx_sc_pm_domain *imx_sc_pd; + sc_rsrc_t rsrc_id; + int ret; + + imx_sc_pd = kzalloc(sizeof(*imx_sc_pd), GFP_KERNEL); + if (!imx_sc_pd) + return ERR_PTR(-ENOMEM); + + if (!of_property_read_u32(np, "reg", &rsrc_id)) { + if (rsrc_id > SC_R_LAST) { + pr_warn("%pOF: invalid rsrc id %d found", np, rsrc_id); + ret = -EINVAL; + goto err; + } + imx_sc_pd->rsrc_id = rsrc_id; + } else { + imx_sc_pd->rsrc_id = SC_R_LAST; + } + + if (imx_sc_pd->rsrc_id != SC_R_LAST) { + imx_sc_pd->pd.power_off = imx_sc_pd_power_off; + imx_sc_pd->pd.power_on = imx_sc_pd_power_on; + } + + imx_sc_pd->pd.name = np->name; + + ret = pm_genpd_init(&imx_sc_pd->pd, NULL, true); + if (ret < 0) + goto err; + + if (genpd_parent) { + ret = pm_genpd_add_subdomain(genpd_parent, &imx_sc_pd->pd); + if (ret) + goto err; + } + + ret = of_genpd_add_provider_simple(np, &imx_sc_pd->pd); + if (!ret) + return &imx_sc_pd->pd; + + pm_genpd_remove_subdomain(genpd_parent, &imx_sc_pd->pd); +err: + pr_warn("imx_sc_pm: failed to add PM domain %pOF: %d\n", np, ret); + kfree(imx_sc_pd); + return ERR_PTR(ret); +} + +static void __init imx_sc_pm_add_subdomains(struct device_node *parent, + struct generic_pm_domain *genpd_parent) +{ + struct generic_pm_domain *pd; + struct device_node *np; + + for_each_child_of_node(parent, np) { + pd = imx_sc_pm_add_one_domain(np, genpd_parent); + if (!IS_ERR(pd)) + imx_sc_pm_add_subdomains(np, pd); + } +} + +static const struct of_device_id imx_sc_pm_domain_of_match[] __initconst = { + { + .compatible = "nxp,imx8qxp-pd", + }, + { }, +}; + +static int __init imx_sc_init_pm_domains(void) +{ + struct generic_pm_domain *pd; + struct device_node *np; + sc_err_t sci_err; + + if (!of_machine_is_compatible("fsl,imx8qxp")) + return 0; + + sci_err = sc_ipc_get_handle(&pm_ipc_handle); + if (sci_err != SC_ERR_NONE) { + pr_err("imx_sc_pd: can't get sc ipc handle\n"); + return -ENODEV; + } + + for_each_matching_node(np, imx_sc_pm_domain_of_match) { + pd = imx_sc_pm_add_one_domain(np, NULL); + if (!IS_ERR(pd)) + imx_sc_pm_add_subdomains(np, pd); + } + + return 0; +} +early_initcall(imx_sc_init_pm_domains);
Some i.MX SoCs contain a system controller that is responsible for controlling the state of the IPs that are present. Communication between the host processor running an OS and the system controller happens through a SCU protocol. This patch adds SCU protocol based power domains drivers. Cc: Shawn Guo <shawnguo@kernel.org> Cc: Sascha Hauer <kernel@pengutronix.de> Cc: Fabio Estevam <fabio.estevam@nxp.com> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Cc: Kevin Hilman <khilman@kernel.org> Cc: Ulf Hansson <ulf.hansson@linaro.org> Cc: linux-pm@vger.kernel.org Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> --- drivers/soc/imx/Kconfig | 4 ++ drivers/soc/imx/Makefile | 1 + drivers/soc/imx/sc-pd.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+) create mode 100644 drivers/soc/imx/sc-pd.c