[alsa-devel] [PATCH 2/4] ASoC: s3c64xx/smartq: use dynamic registration

[Alexandre Courbot]

On Mon, Jul 21, 2014 at 7:04 PM, Mark Brown <broonie at kernel.org> wrote:
> On Mon, Jul 21, 2014 at 12:36:43PM +0900, Alexandre Courbot wrote:
>> On Thu, Jul 17, 2014 at 8:05 PM, Mark Brown <broonie at kernel.org> wrote:
>
>> > For set and get, sure - but it's still useful to be able to do bulk
>> > requests for GPIOs especially since that's the only bit of the interface
>> > that requires error handling.
>
>> I foresee many problems if people start using gpiod_array_get() as a
>> way to spare a few lines of error-checking code. First all the GPIOs
>> would end into an array instead of members with meaningful names -
>> unless they are moved later on, but doing so would add extra code and
>> somewhat kill the purpose. It also becomes more difficult to maintain
>> as you are dealing with array indexes to update all over the code.
>
> You just need a few defines for the names, it's not a big deal.
>
>> Finally, it will make it more difficult to use gpiod_array_*() the way
>> it is intended to be used, as you would have to discriminate between
>> GPIOs of the same function and the rest by yourself.
>
> Yes, you probably shouldn't mix and match here but that's fine.
>
>> Also, if such a convenience function is legitimate for GPIO, shouldn't
>> it also apply to other sub-systems? E.g. regulator_array_get()?
>
> It's certainly a totally reasonable and expected way of using
> regulator_bulk_get().
>
>> Maybe I am missing your point, but I still think some error-handling
>> code really doesn't hurt here, and the few drivers that would actually
>> benefit from a more automated GPIO request error handling can easily
>> implement it themselves. Let's keep gpiod_array_*() single-purposed
>> and to the point.
>
> I'm not sure I see the massive complication TBH - it's not so much about
> complexity as it is about reducing the amount of boilerplate that people
> need to get right.

I guess our disagreement came from the fact that we want the same
function to perform two different things. GPIOs are different from
regulators in that the former are really requested using a (dev,
function, index) vs. a (dev, function) tuple for regulators. I want a
convenience function to easily request all the GPIOs that match (dev,
function) so that functionally identical GPIOs can be manipulated as
an "atomic" set using the rest of the gpiod_array API (which would
make no sense for regulators which have no "index". You want an
equivalent to regulator_bulk_get() so a driver can conveniently
request all its GPIOs no matter the function they fullfil.

These should really be two different functions for two different
use-cases - gpiod_array_get() that returns an array suitable for being
manipulated using the rest of the gpiod_array API ; gpiod_bulk_get()
that takes the equivalent of regulator_bulk_data for GPIOs and fills
it the same way.