* [PATCH 0/3] reset: Miscellaneous improvements
@ 2019-11-20 14:26 Geert Uytterhoeven
2019-11-20 14:26 ` [PATCH 1/3] reset: Do not register resource data for missing resets Geert Uytterhoeven
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2019-11-20 14:26 UTC (permalink / raw)
To: Philipp Zabel; +Cc: Vivek Gautam, linux-kernel, Geert Uytterhoeven
Hi Philipp,
This patch series contains a resource optimization for the managed
helpers, a kerneldoc fix, and an enhancement to make the reset
controller code more uniform.
Thanks!
Geert Uytterhoeven (3):
reset: Do not register resource data for missing resets
reset: Fix {of,devm}_reset_control_array_get kerneldoc return types
reset: Align logic and flow in managed helpers
drivers/reset/core.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)
--
2.17.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] reset: Do not register resource data for missing resets
2019-11-20 14:26 [PATCH 0/3] reset: Miscellaneous improvements Geert Uytterhoeven
@ 2019-11-20 14:26 ` Geert Uytterhoeven
2019-11-20 14:43 ` Philipp Zabel
2019-11-20 14:26 ` [PATCH 2/3] reset: Fix {of,devm}_reset_control_array_get kerneldoc return types Geert Uytterhoeven
2019-11-20 14:26 ` [PATCH 3/3] reset: Align logic and flow in managed helpers Geert Uytterhoeven
2 siblings, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2019-11-20 14:26 UTC (permalink / raw)
To: Philipp Zabel; +Cc: Vivek Gautam, linux-kernel, Geert Uytterhoeven
When an optional reset is not present, __devm_reset_control_get() and
devm_reset_control_array_get() still register resource data to release
the non-existing reset on cleanup, which is futile.
Fix this by skipping NULL reset control pointers.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/reset/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index ca1d49146f611435..55245f485b3819da 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -787,7 +787,7 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
return ERR_PTR(-ENOMEM);
rstc = __reset_control_get(dev, id, index, shared, optional, acquired);
- if (!IS_ERR(rstc)) {
+ if (rstc && !IS_ERR(rstc)) {
*ptr = rstc;
devres_add(dev, ptr);
} else {
@@ -930,7 +930,7 @@ devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
return ERR_PTR(-ENOMEM);
rstc = of_reset_control_array_get(dev->of_node, shared, optional, true);
- if (IS_ERR(rstc)) {
+ if (!rstc || IS_ERR(rstc)) {
devres_free(devres);
return rstc;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] reset: Fix {of,devm}_reset_control_array_get kerneldoc return types
2019-11-20 14:26 [PATCH 0/3] reset: Miscellaneous improvements Geert Uytterhoeven
2019-11-20 14:26 ` [PATCH 1/3] reset: Do not register resource data for missing resets Geert Uytterhoeven
@ 2019-11-20 14:26 ` Geert Uytterhoeven
2019-11-20 14:43 ` Philipp Zabel
2019-11-20 14:26 ` [PATCH 3/3] reset: Align logic and flow in managed helpers Geert Uytterhoeven
2 siblings, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2019-11-20 14:26 UTC (permalink / raw)
To: Philipp Zabel; +Cc: Vivek Gautam, linux-kernel, Geert Uytterhoeven
of_reset_control_array_get() and devm_reset_control_array_get() return
struct reset_control pointers, not internal struct reset_control_array
pointers, just like all other reset control API calls.
Correct the kerneldoc to match the code.
Fixes: 17c82e206d2a3cd8 ("reset: Add APIs to manage array of resets")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/reset/core.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 55245f485b3819da..4ea62aa00260f82c 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -861,8 +861,7 @@ static int of_reset_control_get_count(struct device_node *node)
* @acquired: only one reset control may be acquired for a given controller
* and ID
*
- * Returns pointer to allocated reset_control_array on success or
- * error on failure
+ * Returns pointer to allocated reset_control on success or error on failure
*/
struct reset_control *
of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
@@ -915,8 +914,7 @@ EXPORT_SYMBOL_GPL(of_reset_control_array_get);
* that just have to be asserted or deasserted, without any
* requirements on the order.
*
- * Returns pointer to allocated reset_control_array on success or
- * error on failure
+ * Returns pointer to allocated reset_control on success or error on failure
*/
struct reset_control *
devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] reset: Align logic and flow in managed helpers
2019-11-20 14:26 [PATCH 0/3] reset: Miscellaneous improvements Geert Uytterhoeven
2019-11-20 14:26 ` [PATCH 1/3] reset: Do not register resource data for missing resets Geert Uytterhoeven
2019-11-20 14:26 ` [PATCH 2/3] reset: Fix {of,devm}_reset_control_array_get kerneldoc return types Geert Uytterhoeven
@ 2019-11-20 14:26 ` Geert Uytterhoeven
2 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2019-11-20 14:26 UTC (permalink / raw)
To: Philipp Zabel; +Cc: Vivek Gautam, linux-kernel, Geert Uytterhoeven
__devm_reset_control_get() and devm_reset_control_array_get() are very
similar, but they do not look similar, due to inverted logic.
Make them more similar, following the "bail out early" paradigm.
Adjust the logic and flow in devm_reset_controller_register() to match
the two other functions.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/reset/core.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 4ea62aa00260f82c..763f1353d386519b 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -150,13 +150,14 @@ int devm_reset_controller_register(struct device *dev,
return -ENOMEM;
ret = reset_controller_register(rcdev);
- if (!ret) {
- *rcdevp = rcdev;
- devres_add(dev, rcdevp);
- } else {
+ if (ret) {
devres_free(rcdevp);
+ return ret;
}
+ *rcdevp = rcdev;
+ devres_add(dev, rcdevp);
+
return ret;
}
EXPORT_SYMBOL_GPL(devm_reset_controller_register);
@@ -787,13 +788,14 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
return ERR_PTR(-ENOMEM);
rstc = __reset_control_get(dev, id, index, shared, optional, acquired);
- if (rstc && !IS_ERR(rstc)) {
- *ptr = rstc;
- devres_add(dev, ptr);
- } else {
+ if (!rstc || IS_ERR(rstc)) {
devres_free(ptr);
+ return rstc;
}
+ *ptr = rstc;
+ devres_add(dev, ptr);
+
return rstc;
}
EXPORT_SYMBOL_GPL(__devm_reset_control_get);
@@ -919,22 +921,21 @@ EXPORT_SYMBOL_GPL(of_reset_control_array_get);
struct reset_control *
devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
{
- struct reset_control **devres;
- struct reset_control *rstc;
+ struct reset_control **ptr, *rstc;
- devres = devres_alloc(devm_reset_control_release, sizeof(*devres),
- GFP_KERNEL);
- if (!devres)
+ ptr = devres_alloc(devm_reset_control_release, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
return ERR_PTR(-ENOMEM);
rstc = of_reset_control_array_get(dev->of_node, shared, optional, true);
if (!rstc || IS_ERR(rstc)) {
- devres_free(devres);
+ devres_free(ptr);
return rstc;
}
- *devres = rstc;
- devres_add(dev, devres);
+ *ptr = rstc;
+ devres_add(dev, ptr);
return rstc;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] reset: Do not register resource data for missing resets
2019-11-20 14:26 ` [PATCH 1/3] reset: Do not register resource data for missing resets Geert Uytterhoeven
@ 2019-11-20 14:43 ` Philipp Zabel
2019-11-20 14:45 ` Geert Uytterhoeven
0 siblings, 1 reply; 7+ messages in thread
From: Philipp Zabel @ 2019-11-20 14:43 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Vivek Gautam, linux-kernel
Hi Geert,
thank you for the patches!
On Wed, 2019-11-20 at 15:26 +0100, Geert Uytterhoeven wrote:
> When an optional reset is not present, __devm_reset_control_get() and
> devm_reset_control_array_get() still register resource data to release
> the non-existing reset on cleanup, which is futile.
>
> Fix this by skipping NULL reset control pointers.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> drivers/reset/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index ca1d49146f611435..55245f485b3819da 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -787,7 +787,7 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
> return ERR_PTR(-ENOMEM);
>
> rstc = __reset_control_get(dev, id, index, shared, optional, acquired);
> - if (!IS_ERR(rstc)) {
> + if (rstc && !IS_ERR(rstc)) {
Could you change this to use IS_ERR_OR_NULL, both here and below?
> *ptr = rstc;
> devres_add(dev, ptr);
> } else {
> @@ -930,7 +930,7 @@ devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
> return ERR_PTR(-ENOMEM);
>
> rstc = of_reset_control_array_get(dev->of_node, shared, optional, true);
> - if (IS_ERR(rstc)) {
> + if (!rstc || IS_ERR(rstc)) {
> devres_free(devres);
> return rstc;
> }
Same for patch 3, which otherwise looks fine.
regards
Philipp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] reset: Fix {of,devm}_reset_control_array_get kerneldoc return types
2019-11-20 14:26 ` [PATCH 2/3] reset: Fix {of,devm}_reset_control_array_get kerneldoc return types Geert Uytterhoeven
@ 2019-11-20 14:43 ` Philipp Zabel
0 siblings, 0 replies; 7+ messages in thread
From: Philipp Zabel @ 2019-11-20 14:43 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Vivek Gautam, linux-kernel
On Wed, 2019-11-20 at 15:26 +0100, Geert Uytterhoeven wrote:
> of_reset_control_array_get() and devm_reset_control_array_get() return
> struct reset_control pointers, not internal struct reset_control_array
> pointers, just like all other reset control API calls.
>
> Correct the kerneldoc to match the code.
>
> Fixes: 17c82e206d2a3cd8 ("reset: Add APIs to manage array of resets")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> drivers/reset/core.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 55245f485b3819da..4ea62aa00260f82c 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -861,8 +861,7 @@ static int of_reset_control_get_count(struct device_node *node)
> * @acquired: only one reset control may be acquired for a given controller
> * and ID
> *
> - * Returns pointer to allocated reset_control_array on success or
> - * error on failure
> + * Returns pointer to allocated reset_control on success or error on failure
> */
> struct reset_control *
> of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
> @@ -915,8 +914,7 @@ EXPORT_SYMBOL_GPL(of_reset_control_array_get);
> * that just have to be asserted or deasserted, without any
> * requirements on the order.
> *
> - * Returns pointer to allocated reset_control_array on success or
> - * error on failure
> + * Returns pointer to allocated reset_control on success or error on failure
> */
> struct reset_control *
> devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
Thank you, applied to reset/fixes.
regards
Philipp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] reset: Do not register resource data for missing resets
2019-11-20 14:43 ` Philipp Zabel
@ 2019-11-20 14:45 ` Geert Uytterhoeven
0 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2019-11-20 14:45 UTC (permalink / raw)
To: Philipp Zabel; +Cc: Geert Uytterhoeven, Vivek Gautam, Linux Kernel Mailing List
Hi Philipp,
On Wed, Nov 20, 2019 at 3:43 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> On Wed, 2019-11-20 at 15:26 +0100, Geert Uytterhoeven wrote:
> > When an optional reset is not present, __devm_reset_control_get() and
> > devm_reset_control_array_get() still register resource data to release
> > the non-existing reset on cleanup, which is futile.
> >
> > Fix this by skipping NULL reset control pointers.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > drivers/reset/core.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> > index ca1d49146f611435..55245f485b3819da 100644
> > --- a/drivers/reset/core.c
> > +++ b/drivers/reset/core.c
> > @@ -787,7 +787,7 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
> > return ERR_PTR(-ENOMEM);
> >
> > rstc = __reset_control_get(dev, id, index, shared, optional, acquired);
> > - if (!IS_ERR(rstc)) {
> > + if (rstc && !IS_ERR(rstc)) {
>
> Could you change this to use IS_ERR_OR_NULL, both here and below?
Sure. Silly me...
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-11-20 14:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-20 14:26 [PATCH 0/3] reset: Miscellaneous improvements Geert Uytterhoeven
2019-11-20 14:26 ` [PATCH 1/3] reset: Do not register resource data for missing resets Geert Uytterhoeven
2019-11-20 14:43 ` Philipp Zabel
2019-11-20 14:45 ` Geert Uytterhoeven
2019-11-20 14:26 ` [PATCH 2/3] reset: Fix {of,devm}_reset_control_array_get kerneldoc return types Geert Uytterhoeven
2019-11-20 14:43 ` Philipp Zabel
2019-11-20 14:26 ` [PATCH 3/3] reset: Align logic and flow in managed helpers Geert Uytterhoeven
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).