From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756681AbeAHKvl (ORCPT + 1 other); Mon, 8 Jan 2018 05:51:41 -0500 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:34159 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756536AbeAHKvk (ORCPT ); Mon, 8 Jan 2018 05:51:40 -0500 Message-ID: <1515408692.12538.10.camel@pengutronix.de> Subject: Re: [PATCH] soc: imx: gpc: de-register power domains only if initialized From: Lucas Stach To: Dong Aisheng , Stefan Agner Cc: shawnguo@kernel.org, kernel@pengutronix.de, fabio.estevam@nxp.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-imx@nxp.com Date: Mon, 08 Jan 2018 11:51:32 +0100 In-Reply-To: <20180108102855.GA32635@b29396-OptiPlex-7040> References: <20180107134905.15624-1-stefan@agner.ch> <20180108102855.GA32635@b29396-OptiPlex-7040> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.6-1+deb9u1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-SA-Exim-Connect-IP: 2001:67c:670:100:fa0f:41ff:fe58:4010 X-SA-Exim-Mail-From: l.stach@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: Am Montag, den 08.01.2018, 18:28 +0800 schrieb Dong Aisheng: > On Sun, Jan 07, 2018 at 02:49:05PM +0100, Stefan Agner wrote: > > If power domain information are missing in the device tree, no > > power domains get initialized. However, imx_gpc_remove tries to > > remove power domains always in the old DT binding case. Only > > remove power domains when imx_gpc_probe initialized them in > > first place. > > > > Fixes: 721cabf6c660 ("soc: imx: move PGC handling to a new GPC > > driver") > > Cc: Lucas Stach > > Signed-off-by: Stefan Agner > > --- > >  drivers/soc/imx/gpc.c | 10 +++++++++- > >  1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c > > index 53f7275d6cbd..62bb724726d9 100644 > > --- a/drivers/soc/imx/gpc.c > > +++ b/drivers/soc/imx/gpc.c > > @@ -470,13 +470,21 @@ static int imx_gpc_probe(struct > > platform_device *pdev) > >   > >  static int imx_gpc_remove(struct platform_device *pdev) > >  { > > What's the original purpose of imx_gpc_remove? > ARM power domain can't be removed. Why? As long as it stays powered on there is not reason why we wouldn't be able to remove the driver. > And why current imx_gpc_remove only remove domains for old DT but not > for new ones? With the new binding the power domains will be removed by the sub- drivers for the domains. > How about make it un-removable? > e.g. I don't see why this would be a good idea. Once more device-dependency handling is in place we might need to unbind the power domains when the regulator driver for the domain is unbound. Do you intend to make them non-removable, too? Regards, Lucas > diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c > index 47e7aa9..7fc6737 100644 > --- a/drivers/soc/imx/gpc.c > +++ b/drivers/soc/imx/gpc.c > @@ -454,36 +454,17 @@ static int imx_gpc_probe(struct platform_device > *pdev) >         return 0; >  } >   > -static int imx_gpc_remove(struct platform_device *pdev) > -{ > -       int ret; > - > -       /* > -        * If the old DT binding is used the toplevel driver needs to > -        * de-register the power domains > -        */ > -       if (!of_get_child_by_name(pdev->dev.of_node, "pgc")) { > -               of_genpd_del_provider(pdev->dev.of_node); > - > -               ret = > pm_genpd_remove(&imx_gpc_domains[GPC_PGC_DOMAIN_PU].base); > -               if (ret) > -                       return ret; > -               imx_pgc_put_clocks(&imx_gpc_domains[GPC_PGC_DOMAIN_PU > ]); > - > -               ret = > pm_genpd_remove(&imx_gpc_domains[GPC_PGC_DOMAIN_ARM].base); > -               if (ret) > -                       return ret; > -       } > - > -       return 0; > -} > - >  static struct platform_driver imx_gpc_driver = { >         .driver = { >                 .name = "imx-gpc", >                 .of_match_table = imx_gpc_dt_ids, > +                /* > +                 * We can't forcibly eject devices form power > domain, > +                 * so we can't really remove power domains once they > +                 * were added. > +                 */ > +                .suppress_bind_attrs = true, >         }, >         .probe = imx_gpc_probe, > -       .remove = imx_gpc_remove, >  }; >  builtin_platform_driver(imx_gpc_driver) > > Regards > Dong Aisheng > > > + struct device_node *pgc_node; > >   int ret; > >   > > + pgc_node = of_get_child_by_name(pdev->dev.of_node, "pgc"); > > + > > + /* bail out if DT too old and doesn't provide the > > necessary info */ > > + if (!of_property_read_bool(pdev->dev.of_node, "#power- > > domain-cells") && > > +     !pgc_node) > > + return 0; > > + > >   /* > >    * If the old DT binding is used the toplevel driver needs > > to > >    * de-register the power domains > >    */ > > - if (!of_get_child_by_name(pdev->dev.of_node, "pgc")) { > > + if (!pgc_node) { > >   of_genpd_del_provider(pdev->dev.of_node); > >   > >   ret = > > pm_genpd_remove(&imx_gpc_domains[GPC_PGC_DOMAIN_PU].base); > > --  > > 2.15.1 > >