Linux Serial subsystem development

* Re: [PATCH 000/102] Convert drivers to explicit reset API
From: Dmitry Torokhov @ 2017-07-20 20:46 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Thomas Petazzoni, lkml, Andrew Lunn, Prashant Gaikwad,
	Heiko Stuebner, Peter Chen, Linus Walleij, DRI, Marc Dietrich,
	Rakesh Iyer, Peter Meerwald-Stadler, linux-clk, Wim Van Sebroeck,
	Wolfram Sang, Xinliang Liu, Chanwoo Choi, Alan Stern, Jiri Slaby,
	Michael Turquette, Guenter Roeck
In-Reply-To: <1500555312.2354.75.camel@pengutronix.de>

On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Hi Thomas,
>
> On Thu, 2017-07-20 at 12:36 +0200, Thomas Petazzoni wrote:
>> Hello,
>>
>> On Thu, 20 Jul 2017 11:36:55 +0200, Philipp Zabel wrote:
>>
>> > > I don't know if it has been discussed in the past, so forgive me if it
>> > > has been. Have you considered adding a "int flags" argument to the
>> > > existing reset_control_get_*() functions, rather than introducing
>> > > separate exclusive variants ?
>> > >
>> > > Indeed, with a "int flags" argument you could in the future add more
>> > > variants/behaviors without actually multiplying the number of
>> > > functions. Something like the "flags" argument for request_irq() for
>> > > example.
>> >
>> > I can't find the discussion right now, but I remember we had talked
>> > about this in the past.
>> > Behind the scenes, all the inline API functions already call common
>> > entry points with flags (well, currently separate bool parameters for
>> > shared and optional).
>> > One reason against exposing those as an int flags in the user facing API
>> > is the possibility to accidentally provide a wrong value.
>>
>> This is a quite strange argument. You could also accidentally use the
>> wrong variant of the function, just like you could use the wrong flag.
>
> You can't accidentally use no flag at all or a completely bogus value
> with the "plethora of inline functions" variant.
>
>> Once again, the next time you have another parameter for those reset
>> functions, beyond the exclusive/shared variant, you will multiply again
>> by two the number of functions ? You already have the  exclusive/shared
>> and optional/mandatory variants, so 4 variants. When you'll add a new
>> parameter, you'll have 8 variants. Doesn't seem really good.
>
> I'd rather avoid adding more variants, if possible. The complexity
> increases regardless of whether the API is expressed as a bunch of
> functions or as a single function with a bunch of flags.
>
>> What about reset_control_get(struct device *, const char *, int flags)
>> to replace all those variants ?
>
> While I like how this looks, unfortunately (devm_)reset_control_get
> already exists without the flags, so we can't change to that with a
> gentle transition.

This was done for gpiod_get() and its flags argument with horrifying
#define-ry, which thankfully was completely hidden from users.

-- 
Dmitry

^ permalink raw reply