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

Yicong Yang
2021-04-12T09:13:06Z

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!

>
> 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;
>> }
>
>
> .
>