* [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe @ 2021-04-09 8:34 Tian Tao 2021-04-09 11:58 ` Sven Van Asbroeck 0 siblings, 1 reply; 7+ messages in thread From: Tian Tao @ 2021-04-09 8:34 UTC (permalink / raw) To: TheSven73, gregkh; +Cc: linux-staging, Tian Tao Use devm_add_action_or_reset() instead of devres_alloc() and devres_add(), which works the same. This will simplify the code. There is no functional changes. Signed-off-by: Tian Tao <tiantao6@hisilicon.com> --- drivers/staging/fieldbus/anybuss/host.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/staging/fieldbus/anybuss/host.c b/drivers/staging/fieldbus/anybuss/host.c index 549cb7d5..2924e68 100644 --- a/drivers/staging/fieldbus/anybuss/host.c +++ b/drivers/staging/fieldbus/anybuss/host.c @@ -1406,7 +1406,7 @@ void anybuss_host_common_remove(struct anybuss_host *host) } EXPORT_SYMBOL_GPL(anybuss_host_common_remove); -static void host_release(struct device *dev, void *res) +static void host_release(void *res) { struct anybuss_host **dr = res; @@ -1417,21 +1417,17 @@ struct anybuss_host * __must_check devm_anybuss_host_common_probe(struct device *dev, const struct anybuss_ops *ops) { - struct anybuss_host **dr; struct anybuss_host *host; - - dr = devres_alloc(host_release, sizeof(struct anybuss_host *), - GFP_KERNEL); - if (!dr) - return ERR_PTR(-ENOMEM); + int ret; host = anybuss_host_common_probe(dev, ops); - if (IS_ERR(host)) { - devres_free(dr); + if (IS_ERR(host)) return host; - } - *dr = host; - devres_add(dev, dr); + + ret = devm_add_action_or_reset(dev, host_release, host); + if (ret) + return ERR_PTR(ret); + return host; } EXPORT_SYMBOL_GPL(devm_anybuss_host_common_probe); -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe 2021-04-09 8:34 [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe Tian Tao @ 2021-04-09 11:58 ` Sven Van Asbroeck 0 siblings, 0 replies; 7+ messages in thread From: Sven Van Asbroeck @ 2021-04-09 11:58 UTC (permalink / raw) To: Tian Tao; +Cc: Greg KH, linux-staging, Linux Kernel Mailing List Hello Tian, thank you for the contribution. See below. On Fri, Apr 9, 2021 at 4:33 AM Tian Tao <tiantao6@hisilicon.com> wrote: > > Use devm_add_action_or_reset() instead of devres_alloc() and > devres_add(), which works the same. This will simplify the > code. There is no functional changes. > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com> > --- > drivers/staging/fieldbus/anybuss/host.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/drivers/staging/fieldbus/anybuss/host.c b/drivers/staging/fieldbus/anybuss/host.c > index 549cb7d5..2924e68 100644 > --- a/drivers/staging/fieldbus/anybuss/host.c > +++ b/drivers/staging/fieldbus/anybuss/host.c > @@ -1406,7 +1406,7 @@ void anybuss_host_common_remove(struct anybuss_host *host) > } > EXPORT_SYMBOL_GPL(anybuss_host_common_remove); > > -static void host_release(struct device *dev, void *res) > +static void host_release(void *res) > { > struct anybuss_host **dr = res; You're expecting a double pointer as the argument here... > > @@ -1417,21 +1417,17 @@ struct anybuss_host * __must_check > devm_anybuss_host_common_probe(struct device *dev, > const struct anybuss_ops *ops) > { > - struct anybuss_host **dr; > struct anybuss_host *host; > - > - dr = devres_alloc(host_release, sizeof(struct anybuss_host *), > - GFP_KERNEL); > - if (!dr) > - return ERR_PTR(-ENOMEM); > + int ret; > > host = anybuss_host_common_probe(dev, ops); > - if (IS_ERR(host)) { > - devres_free(dr); > + if (IS_ERR(host)) > return host; > - } > - *dr = host; > - devres_add(dev, dr); > + > + ret = devm_add_action_or_reset(dev, host_release, host); ... yet you pass in a single pointer here. Is that going to work? > + if (ret) > + return ERR_PTR(ret); > + > return host; > } > EXPORT_SYMBOL_GPL(devm_anybuss_host_common_probe); > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe @ 2021-04-09 11:58 ` Sven Van Asbroeck 0 siblings, 0 replies; 7+ messages in thread From: Sven Van Asbroeck @ 2021-04-09 11:58 UTC (permalink / raw) To: Tian Tao; +Cc: Greg KH, linux-staging, Linux Kernel Mailing List Hello Tian, thank you for the contribution. See below. On Fri, Apr 9, 2021 at 4:33 AM Tian Tao <tiantao6@hisilicon.com> wrote: > > Use devm_add_action_or_reset() instead of devres_alloc() and > devres_add(), which works the same. This will simplify the > code. There is no functional changes. > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com> > --- > drivers/staging/fieldbus/anybuss/host.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/drivers/staging/fieldbus/anybuss/host.c b/drivers/staging/fieldbus/anybuss/host.c > index 549cb7d5..2924e68 100644 > --- a/drivers/staging/fieldbus/anybuss/host.c > +++ b/drivers/staging/fieldbus/anybuss/host.c > @@ -1406,7 +1406,7 @@ void anybuss_host_common_remove(struct anybuss_host *host) > } > EXPORT_SYMBOL_GPL(anybuss_host_common_remove); > > -static void host_release(struct device *dev, void *res) > +static void host_release(void *res) > { > struct anybuss_host **dr = res; You're expecting a double pointer as the argument here... > > @@ -1417,21 +1417,17 @@ struct anybuss_host * __must_check > devm_anybuss_host_common_probe(struct device *dev, > const struct anybuss_ops *ops) > { > - struct anybuss_host **dr; > struct anybuss_host *host; > - > - dr = devres_alloc(host_release, sizeof(struct anybuss_host *), > - GFP_KERNEL); > - if (!dr) > - return ERR_PTR(-ENOMEM); > + int ret; > > host = anybuss_host_common_probe(dev, ops); > - if (IS_ERR(host)) { > - devres_free(dr); > + if (IS_ERR(host)) > return host; > - } > - *dr = host; > - devres_add(dev, dr); > + > + ret = devm_add_action_or_reset(dev, host_release, host); ... yet you pass in a single pointer here. Is that going to work? > + if (ret) > + return ERR_PTR(ret); > + > return host; > } > EXPORT_SYMBOL_GPL(devm_anybuss_host_common_probe); > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe 2021-04-09 11:58 ` Sven Van Asbroeck (?) @ 2021-04-12 1:14 ` tiantao (H) 2021-04-12 12:40 ` Sven Van Asbroeck -1 siblings, 1 reply; 7+ messages in thread From: tiantao (H) @ 2021-04-12 1:14 UTC (permalink / raw) To: Sven Van Asbroeck, Tian Tao Cc: Greg KH, linux-staging, Linux Kernel Mailing List 在 2021/4/9 19:58, Sven Van Asbroeck 写道: > Hello Tian, thank you for the contribution. See below. > > On Fri, Apr 9, 2021 at 4:33 AM Tian Tao <tiantao6@hisilicon.com> wrote: >> Use devm_add_action_or_reset() instead of devres_alloc() and >> devres_add(), which works the same. This will simplify the >> code. There is no functional changes. >> >> Signed-off-by: Tian Tao <tiantao6@hisilicon.com> >> --- >> drivers/staging/fieldbus/anybuss/host.c | 20 ++++++++------------ >> 1 file changed, 8 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/staging/fieldbus/anybuss/host.c b/drivers/staging/fieldbus/anybuss/host.c >> index 549cb7d5..2924e68 100644 >> --- a/drivers/staging/fieldbus/anybuss/host.c >> +++ b/drivers/staging/fieldbus/anybuss/host.c >> @@ -1406,7 +1406,7 @@ void anybuss_host_common_remove(struct anybuss_host *host) >> } >> EXPORT_SYMBOL_GPL(anybuss_host_common_remove); >> >> -static void host_release(struct device *dev, void *res) >> +static void host_release(void *res) >> { >> struct anybuss_host **dr = res; > You're expecting a double pointer as the argument here... What about doing it like this? diff --git a/drivers/staging/fieldbus/anybuss/host.c b/drivers/staging/fieldbus/anybuss/host.c index 549cb7d5..c97df91 100644 --- a/drivers/staging/fieldbus/anybuss/host.c +++ b/drivers/staging/fieldbus/anybuss/host.c @@ -1406,32 +1406,26 @@ void anybuss_host_common_remove(struct anybuss_host *host) } EXPORT_SYMBOL_GPL(anybuss_host_common_remove); -static void host_release(struct device *dev, void *res) +static void host_release(void *res) { - struct anybuss_host **dr = res; - - anybuss_host_common_remove(*dr); + anybuss_host_common_remove(res); } struct anybuss_host * __must_check devm_anybuss_host_common_probe(struct device *dev, const struct anybuss_ops *ops) { - struct anybuss_host **dr; struct anybuss_host *host; - - dr = devres_alloc(host_release, sizeof(struct anybuss_host *), - GFP_KERNEL); - if (!dr) - return ERR_PTR(-ENOMEM); + int ret; host = anybuss_host_common_probe(dev, ops); - if (IS_ERR(host)) { - devres_free(dr); + if (IS_ERR(host)) return host; - } - *dr = host; - devres_add(dev, dr); + + ret = devm_add_action_or_reset(dev, host_release, host); + if (ret) + return ERR_PTR(ret); + return host; } > >> @@ -1417,21 +1417,17 @@ struct anybuss_host * __must_check >> devm_anybuss_host_common_probe(struct device *dev, >> const struct anybuss_ops *ops) >> { >> - struct anybuss_host **dr; >> struct anybuss_host *host; >> - >> - dr = devres_alloc(host_release, sizeof(struct anybuss_host *), >> - GFP_KERNEL); >> - if (!dr) >> - return ERR_PTR(-ENOMEM); >> + int ret; >> >> host = anybuss_host_common_probe(dev, ops); >> - if (IS_ERR(host)) { >> - devres_free(dr); >> + if (IS_ERR(host)) >> return host; >> - } >> - *dr = host; >> - devres_add(dev, dr); >> + >> + ret = devm_add_action_or_reset(dev, host_release, host); > ... yet you pass in a single pointer here. Is that going to work? > >> + if (ret) >> + return ERR_PTR(ret); >> + >> return host; >> } >> EXPORT_SYMBOL_GPL(devm_anybuss_host_common_probe); >> -- >> 2.7.4 >> > . > ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe 2021-04-12 1:14 ` tiantao (H) @ 2021-04-12 12:40 ` Sven Van Asbroeck 0 siblings, 0 replies; 7+ messages in thread From: Sven Van Asbroeck @ 2021-04-12 12:40 UTC (permalink / raw) To: tiantao (H); +Cc: Tian Tao, Greg KH, linux-staging, Linux Kernel Mailing List On Sun, Apr 11, 2021 at 9:14 PM tiantao (H) <tiantao6@huawei.com> wrote: > > What about doing it like this? > > -static void host_release(struct device *dev, void *res) > +static void host_release(void *res) > { > - struct anybuss_host **dr = res; > - > - anybuss_host_common_remove(*dr); > + anybuss_host_common_remove(res); > } That looks like it could work. Can you resend as a "proper" patch, please? To tell the versions apart, simply specify [PATCH v1] [PATCH v2] etc in the patch title/subject line. Then below the patch's "--- line", add the version history. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe @ 2021-04-12 12:40 ` Sven Van Asbroeck 0 siblings, 0 replies; 7+ messages in thread From: Sven Van Asbroeck @ 2021-04-12 12:40 UTC (permalink / raw) To: tiantao (H); +Cc: Tian Tao, Greg KH, linux-staging, Linux Kernel Mailing List On Sun, Apr 11, 2021 at 9:14 PM tiantao (H) <tiantao6@huawei.com> wrote: > > What about doing it like this? > > -static void host_release(struct device *dev, void *res) > +static void host_release(void *res) > { > - struct anybuss_host **dr = res; > - > - anybuss_host_common_remove(*dr); > + anybuss_host_common_remove(res); > } That looks like it could work. Can you resend as a "proper" patch, please? To tell the versions apart, simply specify [PATCH v1] [PATCH v2] etc in the patch title/subject line. Then below the patch's "--- line", add the version history. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe 2021-04-12 12:40 ` Sven Van Asbroeck (?) @ 2021-04-12 13:05 ` tiantao (H) -1 siblings, 0 replies; 7+ messages in thread From: tiantao (H) @ 2021-04-12 13:05 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Tian Tao, Greg KH, linux-staging, Linux Kernel Mailing List 在 2021/4/12 20:40, Sven Van Asbroeck 写道: > On Sun, Apr 11, 2021 at 9:14 PM tiantao (H) <tiantao6@huawei.com> wrote: >> What about doing it like this? >> >> -static void host_release(struct device *dev, void *res) >> +static void host_release(void *res) >> { >> - struct anybuss_host **dr = res; >> - >> - anybuss_host_common_remove(*dr); >> + anybuss_host_common_remove(res); >> } > That looks like it could work. Can you resend as a "proper" patch, > please? To tell the versions apart, simply specify [PATCH v1] [PATCH > v2] etc in the patch title/subject line. Then below the patch's "--- > line", add the version history. > . sure. thanks for helping review。 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-12 13:05 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-09 8:34 [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe Tian Tao 2021-04-09 11:58 ` Sven Van Asbroeck 2021-04-09 11:58 ` Sven Van Asbroeck 2021-04-12 1:14 ` tiantao (H) 2021-04-12 12:40 ` Sven Van Asbroeck 2021-04-12 12:40 ` Sven Van Asbroeck 2021-04-12 13:05 ` tiantao (H)
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.