Re: [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe
在 2021/4/12 20:40, Sven Van Asbroeck 写道: On Sun, Apr 11, 2021 at 9:14 PM tiantao (H) 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。
Re: [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe
On Sun, Apr 11, 2021 at 9:14 PM tiantao (H) 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.
Re: [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe
在 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 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 --- 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 .
Re: [PATCH] staging: fieldbus: simplify devm_anybuss_host_common_probe
Hello Tian, thank you for the contribution. See below. On Fri, Apr 9, 2021 at 4:33 AM Tian Tao 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 > --- > 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 >