Re: [PATCH 2/2] soc: imx: gpcv2: make pgc driver more generic for other i.MX platforms

[Andrey Smirnov]

On Sun, Aug 5, 2018 at 11:45 PM Anson Huang <Anson.Huang@xxxxxxx> 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.Huang@xxxxxxx>
> ---
>  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

> +
> +       if (!imx_pgc_domains) {
> +               dev_err(&pdev->dev, "no device match found\n");
> +               return -ENODEV;
> +       }

And doing so would also allow you to drop the check above.

Other that this seems like a reasonable change:

Acked-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx>

Thanks,
Andrey Smirnov