Re: [PATCH -next] remoteproc: stm32: Clean up redundant dev_err_probe()

["chenjiahao (C)" <chenjiahao16@huawei.com>]

On 2023/8/17 15:02, Arnaud POULIQUEN wrote:
>
> On 8/17/23 04:33, chenjiahao (C) wrote:
>> On 2023/8/16 20:46, Arnaud POULIQUEN wrote:
>>> Hi,
>>>
>>> On 8/2/23 11:51, Chen Jiahao wrote:
>>>> Referring to platform_get_irq()'s definition, the return value has
>>>> already been checked if ret < 0, and printed via dev_err_probe().
>>>> Calling dev_err_probe() one more time outside platform_get_irq()
>>>> is obviously redundant.
>>>>
>>>> Removing dev_err_probe() outside platform_get_irq() to clean up
>>>> above problem.
>>>>
>>>> Signed-off-by: Chen Jiahao <chenjiahao16@huawei.com>
>>>> ---
>>>>    drivers/remoteproc/stm32_rproc.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
>>>> index 98234b44f038..a09eeb83ea5c 100644
>>>> --- a/drivers/remoteproc/stm32_rproc.c
>>>> +++ b/drivers/remoteproc/stm32_rproc.c
>>>> @@ -714,7 +714,7 @@ static int stm32_rproc_parse_dt(struct platform_device
>>>> *pdev,
>>>>          irq = platform_get_irq(pdev, 0);
>>>>        if (irq == -EPROBE_DEFER)
>>>> -        return dev_err_probe(dev, irq, "failed to get interrupt\n");
>>>> +        return irq;
>>> The IRQ is optional so using platform_get_irq_optional seems a better option
>>> to me.
>>>
>>> -      irq = platform_get_irq(pdev, 0);
>>> +      irq = platform_get_irq_optional(pdev, 0);
>> Hi,
>>
>> Thanks for your reminding.
>>
>> It seems that platform_get_irq is nothing more than platform_get_irq_optional,
>> except wrapping a dev_err_probe for error code checking. So using the former
>> one should also be OK.
>>
>> Or have I missed anywhere else?
> The main difference is that platform_get_irq prints an error message, while
> platform_get_irq just returns without print.
>
>
> The IRQ is optional, It would be better to only include traces that is necessary
> and useful for debugging purposes.

Make sense. I will update a v2 patch soon.

Thanks

>
> Best Regards,
> Arnaud
>
>
>> Best Regards,
>> Jiahao
>>
>>>>        if (irq == -EPROBE_DEFER)
>>>>          return dev_err_probe(dev, irq, "failed to get interrupt\n");
>>> Thanks,
>>> Arnaud
>>>
>>>>          if (irq > 0) {
>>>>            err = devm_request_irq(dev, irq, stm32_rproc_wdg, 0,