linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] soc: imx: gpc: de-register power domains only if initialized
@ 2018-01-07 13:49 Stefan Agner
  2018-01-08 10:28 ` Dong Aisheng
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Stefan Agner @ 2018-01-07 13:49 UTC (permalink / raw)
  To: shawnguo, kernel
  Cc: fabio.estevam, linux-arm-kernel, linux-kernel, Stefan Agner, Lucas Stach

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 <l.stach@pengutronix.de>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 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)
 {
+	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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] soc: imx: gpc: de-register power domains only if initialized
  2018-01-07 13:49 [PATCH] soc: imx: gpc: de-register power domains only if initialized Stefan Agner
@ 2018-01-08 10:28 ` Dong Aisheng
  2018-01-08 10:51   ` Lucas Stach
  2018-01-09 15:25 ` Lucas Stach
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Dong Aisheng @ 2018-01-08 10:28 UTC (permalink / raw)
  To: Stefan Agner
  Cc: shawnguo, kernel, fabio.estevam, linux-arm-kernel, linux-kernel,
	Lucas Stach, linux-imx

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 <l.stach@pengutronix.de>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  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.

And why current imx_gpc_remove only remove domains for old DT but not
for new ones?

How about make it un-removable?
e.g.

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
> 

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] soc: imx: gpc: de-register power domains only if initialized
  2018-01-08 10:28 ` Dong Aisheng
@ 2018-01-08 10:51   ` Lucas Stach
  2018-01-08 21:17     ` Stefan Agner
  0 siblings, 1 reply; 9+ messages in thread
From: Lucas Stach @ 2018-01-08 10:51 UTC (permalink / raw)
  To: Dong Aisheng, Stefan Agner
  Cc: shawnguo, kernel, fabio.estevam, linux-arm-kernel, linux-kernel,
	linux-imx

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 <l.stach@pengutronix.de>
> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > ---
> >  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
> > 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soc: imx: gpc: de-register power domains only if initialized
  2018-01-08 10:51   ` Lucas Stach
@ 2018-01-08 21:17     ` Stefan Agner
  2018-01-09  9:28       ` Lucas Stach
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Agner @ 2018-01-08 21:17 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Dong Aisheng, shawnguo, kernel, fabio.estevam, linux-arm-kernel,
	linux-kernel, linux-imx

On 2018-01-08 11:51, Lucas Stach wrote:
> 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 <l.stach@pengutronix.de>
>> > Signed-off-by: Stefan Agner <stefan@agner.ch>
>> > ---
>> >  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.
> 

Is it really safe to make assumptions of the hardware state when drivers
get removed? At least some drivers disable the hardware on remove (e.g.
i.MX SPI 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?

I think it would be preferable to keep the ability to remote the driver.

However, I noticed that even with this fix, with device trees which do
use the power domains capabilities (e.g. i.MX6DL) it leads to a stack
trace when using DEBUG_TEST_DRIVER_REMOVE=y, see:
https://marc.info/?l=linux-arm-kernel&m=151544599904423&w=4

--
Stefan

> 
> 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
>> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soc: imx: gpc: de-register power domains only if initialized
  2018-01-08 21:17     ` Stefan Agner
@ 2018-01-09  9:28       ` Lucas Stach
  0 siblings, 0 replies; 9+ messages in thread
From: Lucas Stach @ 2018-01-09  9:28 UTC (permalink / raw)
  To: Stefan Agner
  Cc: Dong Aisheng, shawnguo, kernel, fabio.estevam, linux-arm-kernel,
	linux-kernel, linux-imx

Am Montag, den 08.01.2018, 22:17 +0100 schrieb Stefan Agner:
> On 2018-01-08 11:51, Lucas Stach wrote:
> > 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 <l.stach@pengutronix.de>
> > > > > > > > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > > > ---
> > > >  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.
> > 
> 
> Is it really safe to make assumptions of the hardware state when drivers
> get removed? At least some drivers disable the hardware on remove (e.g.
> i.MX SPI driver).

You are completely right that we should do something more sensible on
remove. Like making sure the domain is powered on or (preferably for
the non ARM domains) unbinding the consumers.
 
> > > 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?
> 
> I think it would be preferable to keep the ability to remote the driver.
> 
> However, I noticed that even with this fix, with device trees which do
> use the power domains capabilities (e.g. i.MX6DL) it leads to a stack
> trace when using DEBUG_TEST_DRIVER_REMOVE=y, see:
> https://marc.info/?l=linux-arm-kernel&m=151544599904423&w=4

Urgh. Yes, we should fix this. This is really missing a device
dependency between the core GPC driver and the PM domain drivers. With
this dependency in place we can make sure to unbind the domain driver
before the core driver goes away.

Regards,
Lucas

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soc: imx: gpc: de-register power domains only if initialized
  2018-01-07 13:49 [PATCH] soc: imx: gpc: de-register power domains only if initialized Stefan Agner
  2018-01-08 10:28 ` Dong Aisheng
@ 2018-01-09 15:25 ` Lucas Stach
  2018-02-10 15:46 ` Stefan Agner
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Lucas Stach @ 2018-01-09 15:25 UTC (permalink / raw)
  To: Stefan Agner, shawnguo, kernel
  Cc: fabio.estevam, linux-kernel, linux-arm-kernel

Am Sonntag, den 07.01.2018, 14:49 +0100 schrieb Stefan Agner:
> 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 <l.stach@pengutronix.de>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  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)
>  {
> +	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);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soc: imx: gpc: de-register power domains only if initialized
  2018-01-07 13:49 [PATCH] soc: imx: gpc: de-register power domains only if initialized Stefan Agner
  2018-01-08 10:28 ` Dong Aisheng
  2018-01-09 15:25 ` Lucas Stach
@ 2018-02-10 15:46 ` Stefan Agner
  2018-02-10 16:24 ` Fabio Estevam
  2018-02-22  3:21 ` Shawn Guo
  4 siblings, 0 replies; 9+ messages in thread
From: Stefan Agner @ 2018-02-10 15:46 UTC (permalink / raw)
  To: shawnguo, kernel
  Cc: fabio.estevam, linux-arm-kernel, linux-kernel, Lucas Stach

On 07.01.2018 14:49, 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.

Shawn, I think this patch can be merged. The discussion branches a bit
off into a second problem, but this patch should be fine.

--
Stefan

> 
> Fixes: 721cabf6c660 ("soc: imx: move PGC handling to a new GPC driver")
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  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)
>  {
> +	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);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soc: imx: gpc: de-register power domains only if initialized
  2018-01-07 13:49 [PATCH] soc: imx: gpc: de-register power domains only if initialized Stefan Agner
                   ` (2 preceding siblings ...)
  2018-02-10 15:46 ` Stefan Agner
@ 2018-02-10 16:24 ` Fabio Estevam
  2018-02-22  3:21 ` Shawn Guo
  4 siblings, 0 replies; 9+ messages in thread
From: Fabio Estevam @ 2018-02-10 16:24 UTC (permalink / raw)
  To: Stefan Agner
  Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, linux-kernel,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Lucas Stach

On Sun, Jan 7, 2018 at 11:49 AM, Stefan Agner <stefan@agner.ch> 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 <l.stach@pengutronix.de>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soc: imx: gpc: de-register power domains only if initialized
  2018-01-07 13:49 [PATCH] soc: imx: gpc: de-register power domains only if initialized Stefan Agner
                   ` (3 preceding siblings ...)
  2018-02-10 16:24 ` Fabio Estevam
@ 2018-02-22  3:21 ` Shawn Guo
  4 siblings, 0 replies; 9+ messages in thread
From: Shawn Guo @ 2018-02-22  3:21 UTC (permalink / raw)
  To: Stefan Agner
  Cc: kernel, fabio.estevam, linux-arm-kernel, linux-kernel, Lucas Stach

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 <l.stach@pengutronix.de>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Applied, thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-02-22  3:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-07 13:49 [PATCH] soc: imx: gpc: de-register power domains only if initialized Stefan Agner
2018-01-08 10:28 ` Dong Aisheng
2018-01-08 10:51   ` Lucas Stach
2018-01-08 21:17     ` Stefan Agner
2018-01-09  9:28       ` Lucas Stach
2018-01-09 15:25 ` Lucas Stach
2018-02-10 15:46 ` Stefan Agner
2018-02-10 16:24 ` Fabio Estevam
2018-02-22  3:21 ` Shawn Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).