Re: [PATCH v3 3/6] ASoC: WM8903: Pass pdata to wm8903_init_gpio

[Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>]

On Thu, Dec 01, 2011 at 04:48:11PM -0800, Stephen Warren wrote:
> Mark Brown wrote at Thursday, December 01, 2011 5:28 PM:

> > This will break existing users in counjuntion with the previous patch.
> > Previously if the user provided platform data but left gpio_base as zero
> > we'd use -1 and let gpiolib pick for us.  Now instead the driver will
> > take that zero and pass it on to gpiolib, probably failing as the SoC
> > will have taken the low numbered GPIOs.

> Yes, I suppose that's true. However, I don't see it as a problem.

> Surely if the user provided pdata, it's their responsibility to fill
> it in correctly and completely. It seems a little random to take the
> pdata, and try to guess whether 0 means 0 or "I didn't set the value,
> so use the default". I think the same comment applies w.r.t to your

No, that's not been the general approach as it avoids breaking existing
users when you add new elements to the platform data and it means that
users don't have to worry about every single field which is much more
friendly as the number of fields gets larger.  In the case of GPIOs it
was frankly a bad idea to have 0 be a valid GPIO value in the first
place; it makes the API that little bit more fiddly to work with.

Device tree is much nicer in this regard as you can just omit properties
which is unambiguous.

> comment on patch 2 (gpio_cfg); 0 is a perfectly legitimate value for
> the register; why should the driver double-guess that value and assume
> 0 means "don't touch the pin"?

Yeah, that's annoying.  There's a reason why most of the chips do the
write of zero by setting an out of bounds bit in the pdata.  At least
for the devices I deal with directly 0 is fortunately usually a
nonsensical value to want to set anyway so it's not so important.