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.