Thread (1 message) 1 message, 1 author, 2011-12-02

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

From: Mark Brown <hidden>
Date: 2011-12-02 01:05:51
Also in: alsa-devel, linux-tegra

On Thu, Dec 01, 2011 at 04:48:11PM -0800, Stephen Warren wrote:
Mark Brown wrote at Thursday, December 01, 2011 5:28 PM:
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help