On Fri, Jan 14, 2022 at 10:58:51AM +0100, Geert Uytterhoeven wrote: > Hi Uwe, > > On Thu, Jan 13, 2022 at 11:43 PM Uwe Kleine-König > <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > On Thu, Jan 13, 2022 at 11:57:43PM +0300, Sergey Shtylyov wrote: > > > On 1/13/22 11:17 PM, Mark Brown wrote: > > > >> The subsystems regulator, clk and gpio have the concept of a dummy > > > >> resource. For regulator, clk and gpio there is a semantic difference > > > >> between the regular _get() function and the _get_optional() variant. > > > >> (One might return the dummy resource, the other won't. Unfortunately > > > >> which one implements which isn't the same for these three.) The > > > >> difference between platform_get_irq() and platform_get_irq_optional() is > > > >> only that the former might emit an error message and the later won't. > > > > > > This is only a current difference but I'm still going to return 0 ISO > > > -ENXIO from latform_get_irq_optional(), no way I'd leave that -ENXIO there > > > alone... :-) > > > > This would address a bit of the critic in my commit log. But as 0 isn't > > a dummy value like the dummy values that exist for clk, gpiod and > > regulator I still think that the naming is a bad idea because it's not > > in the spirit of the other *_get_optional functions. > > > > Seeing you say that -ENXIO is a bad return value for > > platform_get_irq_optional() and 0 should be used instead, I wonder why > > not changing platform_get_irq() to return 0 instead of -ENXIO, too. > > This question is for now only about a sensible semantic. That actually > > changing platform_get_irq() is probably harder than changing > > platform_get_irq_optional() is a different story. > > > > If only platform_get_irq_optional() is changed and given that the > > callers have to do something like: > > > > if (this_irq_exists()): > > ... (e.g. request_irq) > > else: > > ... (e.g. setup polling) > > > > I really think it's a bad idea that this_irq_exists() has to be > > different for platform_get_irq() vs. platform_get_irq_optional(). > > For platform_get_irq(), the IRQ being absent is an error condition, > hence it should return an error code. > For platform_get_irq_optional(), the IRQ being absent is not an error > condition, hence it should not return an error code, and 0 is OK. Please show a few examples how this simplifies the code. If it's only that a driver has to check for == 0 instead of == -ENXIO, than that's not a good enough motivation to make platform_get_irq_optional() different to platform_get_irq(). Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
