Anson Huang Best Regards!
> -----Original Message----- > From: Andrey Smirnov <andrew.smir...@gmail.com> > Sent: Tuesday, August 28, 2018 4:01 PM > To: Anson Huang <anson.hu...@nxp.com> > Cc: Shawn Guo <shawn...@kernel.org>; Sascha Hauer > <s.ha...@pengutronix.de>; Sascha Hauer <ker...@pengutronix.de>; Fabio > Estevam <fabio.este...@nxp.com>; linux-arm-kernel > <linux-arm-ker...@lists.infradead.org>; linux-kernel > <linux-kernel@vger.kernel.org>; dl-linux-imx <linux-...@nxp.com> > Subject: Re: [PATCH 2/2] soc: imx: gpcv2: make pgc driver more generic for > other i.MX platforms > > On Tue, Aug 28, 2018 at 12:28 AM Anson Huang <anson.hu...@nxp.com> > wrote: > > > > > > > > Anson Huang > > Best Regards! > > > > > > > -----Original Message----- > > > From: Andrey Smirnov <andrew.smir...@gmail.com> > > > Sent: Tuesday, August 28, 2018 6:51 AM > > > To: Anson Huang <anson.hu...@nxp.com> > > > Cc: Shawn Guo <shawn...@kernel.org>; Sascha Hauer > > > <s.ha...@pengutronix.de>; Sascha Hauer <ker...@pengutronix.de>; > > > Fabio Estevam <fabio.este...@nxp.com>; linux-arm-kernel > > > <linux-arm-ker...@lists.infradead.org>; linux-kernel > > > <linux-kernel@vger.kernel.org>; dl-linux-imx <linux-...@nxp.com> > > > Subject: Re: [PATCH 2/2] soc: imx: gpcv2: make pgc driver more > > > generic for other i.MX platforms > > > > > > On Sun, Aug 5, 2018 at 11:45 PM Anson Huang <anson.hu...@nxp.com> > > > wrote: > > > > > > > > i.MX8MQ and i.MX8MM share same gpc module with i.MX7D, they can > > > > reuse > > > > gpcv2 pgc driver for power domain control, this patch renames all > > > > functions and structure definitions started with "imx7" to "imx", > > > > and check machine type to pass platform specific power domain data > > > > for power domain driver, thus make > > > > gpcv2 pgc driver more generic for i.MX platforms. > > > > > > > > > > Just for the sake of > > > > > > > > > > Signed-off-by: Anson Huang <anson.hu...@nxp.com> > > > > --- > > > > drivers/soc/imx/gpcv2.c | 68 > > > > +++++++++++++++++++++++++++++-------------------- > > > > 1 file changed, 40 insertions(+), 28 deletions(-) > > > > > > > > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c > > > > index > > > > 0e31465..0e33cb5 100644 > > > > --- a/drivers/soc/imx/gpcv2.c > > > > +++ b/drivers/soc/imx/gpcv2.c > > > > @@ -53,7 +53,7 @@ > > > > > > > > #define GPC_PGC_CTRL_PCR BIT(0) > > > > > > > > -struct imx7_pgc_domain { > > > > +struct imx_pgc_domain { > > > > struct generic_pm_domain genpd; > > > > struct regmap *regmap; > > > > struct regulator *regulator; @@ -69,11 +69,11 @@ struct > > > > imx7_pgc_domain { > > > > struct device *dev; > > > > }; > > > > > > > > -static int imx7_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain > > > > *genpd, > > > > +static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain > > > > +*genpd, > > > > bool on) { > > > > - struct imx7_pgc_domain *domain = container_of(genpd, > > > > - struct > > > imx7_pgc_domain, > > > > + struct imx_pgc_domain *domain = container_of(genpd, > > > > + struct > > > > + imx_pgc_domain, > > > > > genpd); > > > > unsigned int offset = on ? > > > > GPC_PU_PGC_SW_PUP_REQ : > > > GPC_PU_PGC_SW_PDN_REQ; @@ > > > > -150,17 +150,17 @@ static int imx7_gpc_pu_pgc_sw_pxx_req(struct > > > generic_pm_domain *genpd, > > > > return ret; > > > > } > > > > > > > > -static int imx7_gpc_pu_pgc_sw_pup_req(struct generic_pm_domain > > > > *genpd) > > > > +static int imx_gpc_pu_pgc_sw_pup_req(struct generic_pm_domain > > > > +*genpd) > > > > { > > > > - return imx7_gpc_pu_pgc_sw_pxx_req(genpd, true); > > > > + return imx_gpc_pu_pgc_sw_pxx_req(genpd, true); > > > > } > > > > > > > > -static int imx7_gpc_pu_pgc_sw_pdn_req(struct generic_pm_domain > > > > *genpd) > > > > +static int imx_gpc_pu_pgc_sw_pdn_req(struct generic_pm_domain > > > > +*genpd) > > > > { > > > > - return imx7_gpc_pu_pgc_sw_pxx_req(genpd, false); > > > > + return imx_gpc_pu_pgc_sw_pxx_req(genpd, false); > > > > } > > > > > > > > -static const struct imx7_pgc_domain imx7_pgc_domains[] = { > > > > +static const struct imx_pgc_domain imx7_pgc_domains[] = { > > > > [IMX7_POWER_DOMAIN_MIPI_PHY] = { > > > > .genpd = { > > > > .name = "mipi-phy", > > > > @@ -198,9 +198,9 @@ static const struct imx7_pgc_domain > > > imx7_pgc_domains[] = { > > > > }, > > > > }; > > > > > > > > -static int imx7_pgc_domain_probe(struct platform_device *pdev) > > > > +static int imx_pgc_domain_probe(struct platform_device *pdev) > > > > { > > > > - struct imx7_pgc_domain *domain = pdev->dev.platform_data; > > > > + struct imx_pgc_domain *domain = pdev->dev.platform_data; > > > > int ret; > > > > > > > > domain->dev = &pdev->dev; > > > > @@ -233,9 +233,9 @@ static int imx7_pgc_domain_probe(struct > > > platform_device *pdev) > > > > return ret; > > > > } > > > > > > > > -static int imx7_pgc_domain_remove(struct platform_device *pdev) > > > > +static int imx_pgc_domain_remove(struct platform_device *pdev) > > > > { > > > > - struct imx7_pgc_domain *domain = pdev->dev.platform_data; > > > > + struct imx_pgc_domain *domain = pdev->dev.platform_data; > > > > > > > > of_genpd_del_provider(domain->dev->of_node); > > > > pm_genpd_remove(&domain->genpd); @@ -243,23 +243,24 > @@ > > > static > > > > int imx7_pgc_domain_remove(struct platform_device *pdev) > > > > return 0; > > > > } > > > > > > > > -static const struct platform_device_id imx7_pgc_domain_id[] = { > > > > - { "imx7-pgc-domain", }, > > > > +static const struct platform_device_id imx_pgc_domain_id[] = { > > > > + { "imx-pgc-domain", }, > > > > { }, > > > > }; > > > > > > > > -static struct platform_driver imx7_pgc_domain_driver = { > > > > +static struct platform_driver imx_pgc_domain_driver = { > > > > .driver = { > > > > - .name = "imx7-pgc", > > > > + .name = "imx-pgc", > > > > }, > > > > - .probe = imx7_pgc_domain_probe, > > > > - .remove = imx7_pgc_domain_remove, > > > > - .id_table = imx7_pgc_domain_id, > > > > + .probe = imx_pgc_domain_probe, > > > > + .remove = imx_pgc_domain_remove, > > > > + .id_table = imx_pgc_domain_id, > > > > }; > > > > -builtin_platform_driver(imx7_pgc_domain_driver) > > > > +builtin_platform_driver(imx_pgc_domain_driver) > > > > > > > > static int imx_gpcv2_probe(struct platform_device *pdev) { > > > > + static const struct imx_pgc_domain *imx_pgc_domains; > > > > static const struct regmap_range yes_ranges[] = { > > > > regmap_reg_range(GPC_LPCR_A_CORE_BSC, > > > > GPC_M4_PU_PDN_FLG), @@ > > > -287,6 +288,7 > > > > @@ static int imx_gpcv2_probe(struct platform_device *pdev) > > > > struct regmap *regmap; > > > > struct resource *res; > > > > void __iomem *base; > > > > + int pgc_max_index; > > > > int ret; > > > > > > > > pgc_np = of_get_child_by_name(dev->of_node, "pgc"); @@ > > > > -307,9 > > > > +309,19 @@ static int imx_gpcv2_probe(struct platform_device > > > > +*pdev) > > > > return ret; > > > > } > > > > > > > > + if (of_machine_is_compatible("fsl,imx7d")) { > > > > + pgc_max_index = ARRAY_SIZE(imx7_pgc_domains); > > > > + imx_pgc_domains = imx7_pgc_domains; > > > > + } > > > > > > Is there any reason to do this explicit call to > > > of_machine_is_compatible() as opposed to passing necessary data via > > > .data in imx_gpcv2_dt_ids[]? The latter seems like a more > > > straightforward way of passing variant specific driver info > > > > The reason of NOT using .data is that currently gpcv2 driver uses a > > structure array of imx7_pgc_domains[] which has no fixed length, the > > .data can ONLY pass the pointer of the starting address of > > imx7_pgc_domains[], and we need to know the array size of > > imx7_pgc_domains, how can we know it if we ONLY got the starting address > of this array passed from .data. Please advise, thanks. > > > > Unless I am missing something, you should be able to do something like: > > struct imx_pgc_domain_data { > struct imx_pgc_domain *domains; > size_t domains_num; > }; > > const struct imx_pgc_domain_data imx7_pgc_domain_data = { > .domains = imx7_pgc_domains, > .domains_num = ARRAY_SIZE(imx7_pgc_domains), }; > > and then just pass imx7_pgc_domain_data via .data. That's a pretty common > idiom in kernel driver. > > Thanks, > Andrey Smirnov Thanks Andrey. I just thought we can avoid another warp for the structure, if we can NOT, I will add a wrap in V2 patch set as you suggested, thanks. Anson.