Re: [PATCH 1/2] platform: make platform_get_irq_optional() optional

Sergey Shtylyov
2022-01-20T11:27:32Z

On 1/17/22 4:08 PM, Geert Uytterhoeven wrote:

[...]
>>> But as we were all in a hurry to kill the non-applicable error
>>> message, we went for the quick and dirty fix.
>>>
>>>> Also I fail to see how a caller of (today's) platform_get_irq_optional()
>>>> is simpler than a caller of platform_get_irq() given that there is no
>>>> semantic difference between the two. Please show me a single
>>>> conversion from platform_get_irq to platform_get_irq_optional that
>>>> yielded a simplification.
>>>
>>> That's exactly why we want to change the latter to return 0 ;-)
>>
>> OK. So you agree to my statement "The reason for
>> platform_get_irq_optional()'s existence is just that platform_get_irq()
>> emits an error message [...]". Actually you don't want to oppose but
>> say: It's unfortunate that the silent variant of platform_get_irq() took
>> the obvious name of a function that could have an improved return code
>> semantic.
>>
>> So my suggestion to rename todays platform_get_irq_optional() to
>> platform_get_irq_silently() and then introducing
>> platform_get_irq_optional() with your suggested semantic seems
>> intriguing and straigt forward to me.
>
> I don't really see the point of needing platform_get_irq_silently(),
> unless as an intermediary step, where it's going to be removed again
> once the conversion has completed.
> Still, the rename would touch all users at once anyway.
>
>> Another thought: platform_get_irq emits an error message for all
>> problems. Wouldn't it be consistent to let platform_get_irq_optional()
>> emit an error message for all problems but "not found"?
>> Alternatively remove the error printk from platform_get_irq().
>
> Yes, all problems but not found are real errors.

ACK for using dev_err_probe() in platfrom_get_irq_optional()
for the real errors...
I've also noted that only platfrom_get_irq_optional() got converted
from dev_err() to dev_err_probe() but not platfrom_get_irq_byname_optional()...

[...]

> Gr{oetje,eeting}s,
>
> Geert

MBR, Sergey