All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary of_node_put() in error handling
@ 2019-05-21  9:18 ` Anson Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Anson Huang @ 2019-05-21  9:18 UTC (permalink / raw)
  To: shawnguo, s.hauer, kernel, festevam, Leonard Crestez, Abel Vesa,
	viresh.kumar, linux-arm-kernel, linux-kernel
  Cc: dl-linux-imx

of_node_put() is called after of_match_node() successfully called,
then in the following error handling, of_node_put() is called again
which is unnecessary, this patch adjusts the location of of_node_put()
to avoid such scenario.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/soc/imx/soc-imx8.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/imx/soc-imx8.c b/drivers/soc/imx/soc-imx8.c
index b1bd8e2..944add2 100644
--- a/drivers/soc/imx/soc-imx8.c
+++ b/drivers/soc/imx/soc-imx8.c
@@ -86,8 +86,6 @@ static int __init imx8_soc_init(void)
 	if (!id)
 		goto free_soc;
 
-	of_node_put(root);
-
 	data = id->data;
 	if (data) {
 		soc_dev_attr->soc_id = data->name;
@@ -106,6 +104,8 @@ static int __init imx8_soc_init(void)
 	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
 		platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
 
+	of_node_put(root);
+
 	return 0;
 
 free_rev:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary of_node_put() in error handling
@ 2019-05-21  9:18 ` Anson Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Anson Huang @ 2019-05-21  9:18 UTC (permalink / raw)
  To: shawnguo, s.hauer, kernel, festevam, Leonard Crestez, Abel Vesa,
	viresh.kumar, linux-arm-kernel, linux-kernel
  Cc: dl-linux-imx

of_node_put() is called after of_match_node() successfully called,
then in the following error handling, of_node_put() is called again
which is unnecessary, this patch adjusts the location of of_node_put()
to avoid such scenario.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/soc/imx/soc-imx8.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/imx/soc-imx8.c b/drivers/soc/imx/soc-imx8.c
index b1bd8e2..944add2 100644
--- a/drivers/soc/imx/soc-imx8.c
+++ b/drivers/soc/imx/soc-imx8.c
@@ -86,8 +86,6 @@ static int __init imx8_soc_init(void)
 	if (!id)
 		goto free_soc;
 
-	of_node_put(root);
-
 	data = id->data;
 	if (data) {
 		soc_dev_attr->soc_id = data->name;
@@ -106,6 +104,8 @@ static int __init imx8_soc_init(void)
 	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
 		platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
 
+	of_node_put(root);
+
 	return 0;
 
 free_rev:
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/2] soc: imx: soc-imx8: Correct return value of error handle
  2019-05-21  9:18 ` Anson Huang
@ 2019-05-21  9:18   ` Anson Huang
  -1 siblings, 0 replies; 10+ messages in thread
From: Anson Huang @ 2019-05-21  9:18 UTC (permalink / raw)
  To: shawnguo, s.hauer, kernel, festevam, Leonard Crestez, Abel Vesa,
	viresh.kumar, linux-arm-kernel, linux-kernel
  Cc: dl-linux-imx

Current implementation of i.MX8 SoC driver returns -ENODEV
for all cases of error during initialization, this is incorrect.
This patch fixes them using correct return value according
to different errors.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/soc/imx/soc-imx8.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/imx/soc-imx8.c b/drivers/soc/imx/soc-imx8.c
index 944add2..9dd088f 100644
--- a/drivers/soc/imx/soc-imx8.c
+++ b/drivers/soc/imx/soc-imx8.c
@@ -73,7 +73,7 @@ static int __init imx8_soc_init(void)
 
 	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
 	if (!soc_dev_attr)
-		return -ENODEV;
+		return -ENOMEM;
 
 	soc_dev_attr->family = "Freescale i.MX";
 
@@ -83,8 +83,10 @@ static int __init imx8_soc_init(void)
 		goto free_soc;
 
 	id = of_match_node(imx8_soc_match, root);
-	if (!id)
+	if (!id) {
+		ret = -ENODEV;
 		goto free_soc;
+	}
 
 	data = id->data;
 	if (data) {
@@ -94,12 +96,16 @@ static int __init imx8_soc_init(void)
 	}
 
 	soc_dev_attr->revision = imx8_revision(soc_rev);
-	if (!soc_dev_attr->revision)
+	if (!soc_dev_attr->revision) {
+		ret = -ENOMEM;
 		goto free_soc;
+	}
 
 	soc_dev = soc_device_register(soc_dev_attr);
-	if (IS_ERR(soc_dev))
+	if (IS_ERR(soc_dev)) {
+		ret = PTR_ERR(soc_dev);
 		goto free_rev;
+	}
 
 	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
 		platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
@@ -113,6 +119,6 @@ static int __init imx8_soc_init(void)
 free_soc:
 	kfree(soc_dev_attr);
 	of_node_put(root);
-	return -ENODEV;
+	return ret;
 }
 device_initcall(imx8_soc_init);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/2] soc: imx: soc-imx8: Correct return value of error handle
@ 2019-05-21  9:18   ` Anson Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Anson Huang @ 2019-05-21  9:18 UTC (permalink / raw)
  To: shawnguo, s.hauer, kernel, festevam, Leonard Crestez, Abel Vesa,
	viresh.kumar, linux-arm-kernel, linux-kernel
  Cc: dl-linux-imx

Current implementation of i.MX8 SoC driver returns -ENODEV
for all cases of error during initialization, this is incorrect.
This patch fixes them using correct return value according
to different errors.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/soc/imx/soc-imx8.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/imx/soc-imx8.c b/drivers/soc/imx/soc-imx8.c
index 944add2..9dd088f 100644
--- a/drivers/soc/imx/soc-imx8.c
+++ b/drivers/soc/imx/soc-imx8.c
@@ -73,7 +73,7 @@ static int __init imx8_soc_init(void)
 
 	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
 	if (!soc_dev_attr)
-		return -ENODEV;
+		return -ENOMEM;
 
 	soc_dev_attr->family = "Freescale i.MX";
 
@@ -83,8 +83,10 @@ static int __init imx8_soc_init(void)
 		goto free_soc;
 
 	id = of_match_node(imx8_soc_match, root);
-	if (!id)
+	if (!id) {
+		ret = -ENODEV;
 		goto free_soc;
+	}
 
 	data = id->data;
 	if (data) {
@@ -94,12 +96,16 @@ static int __init imx8_soc_init(void)
 	}
 
 	soc_dev_attr->revision = imx8_revision(soc_rev);
-	if (!soc_dev_attr->revision)
+	if (!soc_dev_attr->revision) {
+		ret = -ENOMEM;
 		goto free_soc;
+	}
 
 	soc_dev = soc_device_register(soc_dev_attr);
-	if (IS_ERR(soc_dev))
+	if (IS_ERR(soc_dev)) {
+		ret = PTR_ERR(soc_dev);
 		goto free_rev;
+	}
 
 	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
 		platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
@@ -113,6 +119,6 @@ static int __init imx8_soc_init(void)
 free_soc:
 	kfree(soc_dev_attr);
 	of_node_put(root);
-	return -ENODEV;
+	return ret;
 }
 device_initcall(imx8_soc_init);
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary of_node_put() in error handling
  2019-05-21  9:18 ` Anson Huang
@ 2019-05-21 11:03   ` Leonard Crestez
  -1 siblings, 0 replies; 10+ messages in thread
From: Leonard Crestez @ 2019-05-21 11:03 UTC (permalink / raw)
  To: Anson Huang, shawnguo
  Cc: s.hauer, kernel, festevam, Abel Vesa, viresh.kumar,
	linux-arm-kernel, linux-kernel, dl-linux-imx

On 5/21/2019 12:18 PM, Anson Huang wrote:
> of_node_put() is called after of_match_node() successfully called,
> then in the following error handling, of_node_put() is called again
> which is unnecessary, this patch adjusts the location of of_node_put()
> to avoid such scenario.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

For both:

Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>

I was thinking that maybe you could of_node_put as soon as you were done 
with it but the model is read straight into soc_dev_attr so just freeing 
everything at the end makes more sense.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary of_node_put() in error handling
@ 2019-05-21 11:03   ` Leonard Crestez
  0 siblings, 0 replies; 10+ messages in thread
From: Leonard Crestez @ 2019-05-21 11:03 UTC (permalink / raw)
  To: Anson Huang, shawnguo
  Cc: Abel Vesa, viresh.kumar, s.hauer, linux-kernel, dl-linux-imx,
	kernel, festevam, linux-arm-kernel

On 5/21/2019 12:18 PM, Anson Huang wrote:
> of_node_put() is called after of_match_node() successfully called,
> then in the following error handling, of_node_put() is called again
> which is unnecessary, this patch adjusts the location of of_node_put()
> to avoid such scenario.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

For both:

Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>

I was thinking that maybe you could of_node_put as soon as you were done 
with it but the model is read straight into soc_dev_attr so just freeing 
everything at the end makes more sense.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary of_node_put() in error handling
  2019-05-21  9:18 ` Anson Huang
@ 2019-05-23 12:40   ` Shawn Guo
  -1 siblings, 0 replies; 10+ messages in thread
From: Shawn Guo @ 2019-05-23 12:40 UTC (permalink / raw)
  To: Anson Huang
  Cc: s.hauer, kernel, festevam, Leonard Crestez, Abel Vesa,
	viresh.kumar, linux-arm-kernel, linux-kernel, dl-linux-imx

On Tue, May 21, 2019 at 09:18:43AM +0000, Anson Huang wrote:
> of_node_put() is called after of_match_node() successfully called,
> then in the following error handling, of_node_put() is called again
> which is unnecessary, this patch adjusts the location of of_node_put()
> to avoid such scenario.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Again, there are '=20' in the patch content and I cannot apply it.

Shawn

> ---
>  drivers/soc/imx/soc-imx8.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/imx/soc-imx8.c b/drivers/soc/imx/soc-imx8.c
> index b1bd8e2..944add2 100644
> --- a/drivers/soc/imx/soc-imx8.c
> +++ b/drivers/soc/imx/soc-imx8.c
> @@ -86,8 +86,6 @@ static int __init imx8_soc_init(void)
>  	if (!id)
>  		goto free_soc;
>  
> -	of_node_put(root);
> -
>  	data = id->data;
>  	if (data) {
>  		soc_dev_attr->soc_id = data->name;
> @@ -106,6 +104,8 @@ static int __init imx8_soc_init(void)
>  	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
>  		platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
>  
> +	of_node_put(root);
> +
>  	return 0;
>  
>  free_rev:
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary of_node_put() in error handling
@ 2019-05-23 12:40   ` Shawn Guo
  0 siblings, 0 replies; 10+ messages in thread
From: Shawn Guo @ 2019-05-23 12:40 UTC (permalink / raw)
  To: Anson Huang
  Cc: Abel Vesa, viresh.kumar, s.hauer, linux-kernel, dl-linux-imx,
	kernel, Leonard Crestez, festevam, linux-arm-kernel

On Tue, May 21, 2019 at 09:18:43AM +0000, Anson Huang wrote:
> of_node_put() is called after of_match_node() successfully called,
> then in the following error handling, of_node_put() is called again
> which is unnecessary, this patch adjusts the location of of_node_put()
> to avoid such scenario.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Again, there are '=20' in the patch content and I cannot apply it.

Shawn

> ---
>  drivers/soc/imx/soc-imx8.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/imx/soc-imx8.c b/drivers/soc/imx/soc-imx8.c
> index b1bd8e2..944add2 100644
> --- a/drivers/soc/imx/soc-imx8.c
> +++ b/drivers/soc/imx/soc-imx8.c
> @@ -86,8 +86,6 @@ static int __init imx8_soc_init(void)
>  	if (!id)
>  		goto free_soc;
>  
> -	of_node_put(root);
> -
>  	data = id->data;
>  	if (data) {
>  		soc_dev_attr->soc_id = data->name;
> @@ -106,6 +104,8 @@ static int __init imx8_soc_init(void)
>  	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
>  		platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
>  
> +	of_node_put(root);
> +
>  	return 0;
>  
>  free_rev:
> -- 
> 2.7.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread
* RE: [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary of_node_put() in error handling
  2019-05-23 12:40   ` Shawn Guo
@ 2019-05-24  5:53     ` Anson Huang
  -1 siblings, 0 replies; 10+ messages in thread
From: Anson Huang @ 2019-05-24  5:53 UTC (permalink / raw)
  To: Shawn Guo
  Cc: s.hauer, kernel, festevam, Leonard Crestez, Abel Vesa,
	viresh.kumar, linux-arm-kernel, linux-kernel, dl-linux-imx

Hi, Shawn

> -----Original Message-----
> From: Shawn Guo [mailto:shawnguo@kernel.org]
> Sent: Thursday, May 23, 2019 8:41 PM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> Leonard Crestez <leonard.crestez@nxp.com>; Abel Vesa
> <abel.vesa@nxp.com>; viresh.kumar@linaro.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary
> of_node_put() in error handling
> 
> On Tue, May 21, 2019 at 09:18:43AM +0000, Anson Huang wrote:
> > of_node_put() is called after of_match_node() successfully called,
> > then in the following error handling, of_node_put() is called again
> > which is unnecessary, this patch adjusts the location of of_node_put()
> > to avoid such scenario.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> 
> Again, there are '=20' in the patch content and I cannot apply it.

I resent the patch set, please pick them up, thanks.

https://patchwork.kernel.org/patch/10959101/
https://patchwork.kernel.org/patch/10959099/

Anson.

> 
> Shawn
> 
> > ---
> >  drivers/soc/imx/soc-imx8.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/soc/imx/soc-imx8.c b/drivers/soc/imx/soc-imx8.c
> > index b1bd8e2..944add2 100644
> > --- a/drivers/soc/imx/soc-imx8.c
> > +++ b/drivers/soc/imx/soc-imx8.c
> > @@ -86,8 +86,6 @@ static int __init imx8_soc_init(void)
> >  	if (!id)
> >  		goto free_soc;
> >
> > -	of_node_put(root);
> > -
> >  	data = id->data;
> >  	if (data) {
> >  		soc_dev_attr->soc_id = data->name;
> > @@ -106,6 +104,8 @@ static int __init imx8_soc_init(void)
> >  	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
> >  		platform_device_register_simple("imx-cpufreq-dt", -1, NULL,
> 0);
> >
> > +	of_node_put(root);
> > +
> >  	return 0;
> >
> >  free_rev:
> > --
> > 2.7.4
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread
* RE: [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary of_node_put() in error handling
@ 2019-05-24  5:53     ` Anson Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Anson Huang @ 2019-05-24  5:53 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Abel Vesa, viresh.kumar, s.hauer, linux-kernel, dl-linux-imx,
	kernel, Leonard Crestez, festevam, linux-arm-kernel

Hi, Shawn

> -----Original Message-----
> From: Shawn Guo [mailto:shawnguo@kernel.org]
> Sent: Thursday, May 23, 2019 8:41 PM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> Leonard Crestez <leonard.crestez@nxp.com>; Abel Vesa
> <abel.vesa@nxp.com>; viresh.kumar@linaro.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary
> of_node_put() in error handling
> 
> On Tue, May 21, 2019 at 09:18:43AM +0000, Anson Huang wrote:
> > of_node_put() is called after of_match_node() successfully called,
> > then in the following error handling, of_node_put() is called again
> > which is unnecessary, this patch adjusts the location of of_node_put()
> > to avoid such scenario.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> 
> Again, there are '=20' in the patch content and I cannot apply it.

I resent the patch set, please pick them up, thanks.

https://patchwork.kernel.org/patch/10959101/
https://patchwork.kernel.org/patch/10959099/

Anson.

> 
> Shawn
> 
> > ---
> >  drivers/soc/imx/soc-imx8.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/soc/imx/soc-imx8.c b/drivers/soc/imx/soc-imx8.c
> > index b1bd8e2..944add2 100644
> > --- a/drivers/soc/imx/soc-imx8.c
> > +++ b/drivers/soc/imx/soc-imx8.c
> > @@ -86,8 +86,6 @@ static int __init imx8_soc_init(void)
> >  	if (!id)
> >  		goto free_soc;
> >
> > -	of_node_put(root);
> > -
> >  	data = id->data;
> >  	if (data) {
> >  		soc_dev_attr->soc_id = data->name;
> > @@ -106,6 +104,8 @@ static int __init imx8_soc_init(void)
> >  	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
> >  		platform_device_register_simple("imx-cpufreq-dt", -1, NULL,
> 0);
> >
> > +	of_node_put(root);
> > +
> >  	return 0;
> >
> >  free_rev:
> > --
> > 2.7.4
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-05-24  5:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-21  9:18 [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary of_node_put() in error handling Anson Huang
2019-05-21  9:18 ` Anson Huang
2019-05-21  9:18 ` [PATCH 2/2] soc: imx: soc-imx8: Correct return value of error handle Anson Huang
2019-05-21  9:18   ` Anson Huang
2019-05-21 11:03 ` [PATCH 1/2] soc: imx: soc-imx8: Avoid unnecessary of_node_put() in error handling Leonard Crestez
2019-05-21 11:03   ` Leonard Crestez
2019-05-23 12:40 ` Shawn Guo
2019-05-23 12:40   ` Shawn Guo
2019-05-24  5:53   ` Anson Huang
2019-05-24  5:53     ` Anson Huang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.