Re: [PATCH v1 1/2] ata: libahci_platform: Get rid of dup message when IRQ can't be retrieved
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2021-12-13 22:03:50
Also in:
lkml
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2021-12-13 22:03:50
Also in:
lkml
On Tue, Dec 14, 2021 at 06:36:00AM +0900, Damien Le Moal wrote:
On 2021/12/13 20:52, Andy Shevchenko wrote:quoted
On Mon, Dec 13, 2021 at 07:39:31AM +0900, Damien Le Moal wrote:quoted
On 2021/12/11 19:25, Sergey Shtylyov wrote:
...
quoted
quoted
The problem I see is that the current behavior is unclear: what does platform_get_irq_optional() returning 0 mean ? IRQ == 0 ? or "no IRQ" ? I think it should be the latter rather than the former. Note that the function could return ENOENT (or similar) for the "no IRQ" case. With that, case (2) goes away, but then I do not see any difference between platform_get_irq_optional() and platform_get_irq(). If the preferred API semantic is to allow returning IRQ 0 with a warning, then the kdoc comments of platform_get_irq_optional() and platform_get_irq() are totally broken, and the code for many drivers is probably wrong too.Yeah, what we need to do is that (roughly a roadmap): - revisit callers of platform_get_irq_optional() to be prepared for new behaviour - rewrite platform_get_irq() to return -ENOENT - rewrite platform_get_irq_optional() to return 0 on -ENOENT This is how other similar (i.e. _optional) APIs do.Sounds like a good plan to me. In the mean time though, your patch 1/2 should keep the "if (!irq)" test and return an error for that case. No ?
No problem. I can send a v2. -- With Best Regards, Andy Shevchenko