Mark Brown wrote at Thursday, December 01, 2011 5:28 PM:
On Thu, Dec 01, 2011 at 01:49:21PM -0700, Stephen Warren wrote:
- if (pdata && pdata->gpio_base)
wm8903->gpio_chip.base = pdata->gpio_base;- else
wm8903->gpio_chip.base = -1;
- wm8903->gpio_chip.base = pdata->gpio_base;
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 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"?