Re: [PATCH 1/7] iio: adc: adi-axi-adc: simplify devm_adi_axi_adc_conv_register

Jonathan Cameron
2021-04-24T12:31:06Z

On Mon, 12 Apr 2021 17:10:17 +0800
Yicong Yang <yangyicong@hisilicon.com> wrote:

> On 2021/4/11 22:12, Jonathan Cameron wrote:
> > On Thu, 8 Apr 2021 19:38:10 +0800
> > Yicong Yang <yangyicong@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: Yicong Yang <yangyicong@hisilicon.com>
> >> ---
> >> drivers/iio/adc/adi-axi-adc.c | 22 +++++++++-------------
> >> 1 file changed, 9 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> >> index 9109da2..575a63f 100644
> >> --- a/drivers/iio/adc/adi-axi-adc.c
> >> +++ b/drivers/iio/adc/adi-axi-adc.c
> >> @@ -210,29 +210,25 @@ static void adi_axi_adc_conv_unregister(struct adi_axi_adc_conv *conv)
> >> kfree(cl);
> >> }
> >>
> >> -static void devm_adi_axi_adc_conv_release(struct device *dev, void *res)
> >> +static void devm_adi_axi_adc_conv_release(void *conv)
> >> {
> >> - adi_axi_adc_conv_unregister(*(struct adi_axi_adc_conv **)res);
> >> + adi_axi_adc_conv_unregister(conv);
> >> }
> >>
> >> struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device *dev,
> >> size_t sizeof_priv)
> >> {
> >> - struct adi_axi_adc_conv **ptr, *conv;
> >> -
> >> - ptr = devres_alloc(devm_adi_axi_adc_conv_release, sizeof(*ptr),
> >> - GFP_KERNEL);
> >> - if (!ptr)
> >> - return ERR_PTR(-ENOMEM);
> >> + struct adi_axi_adc_conv *conv;
> >> + int ret;
> >>
> >> conv = adi_axi_adc_conv_register(dev, sizeof_priv);
> >> - if (IS_ERR(conv)) {
> >> - devres_free(ptr);
> >> + if (IS_ERR(conv))
> >> return ERR_CAST(conv);
> >
> > Is that ERR_CAST() needed here? conv is already of the
> > right type so we don't need to cast it to a void * and back gain.
> > Obviously was there before an not needed either, but might as well
> > tidy it up whilst we are here!
>
> sure. thanks for the hint. I didn't notice this. will drop the ERR_CAST.
>
> thanks!

Rather than waste your time doing a v2 for such a trivial change, I've
applied the patch and tweaked that whilst doing so.

Applied to the togreg branch of iio.git and pushed out as testing to
let 0-day poke at it etc.

Thanks,

Jonathan

>
> >
> > Thanks,
> >
> > Jonathan
> >
> >
> >
> >> - }
> >>
> >> - *ptr = conv;
> >> - devres_add(dev, ptr);
> >> + ret = devm_add_action_or_reset(dev, devm_adi_axi_adc_conv_release,
> >> + conv);
> >> + if (ret)
> >> + return ERR_PTR(ret);
> >>
> >> return conv;
> >> }
> >
> >
> > .
> >
>