Re: [PATCH] mfd: ab8500: update kconfig for ab8500 core driver

[Samuel Ortiz <sameo@linux.intel.com>]

Hi Randy,

On Wed, Sep 15, 2010 at 10:41:05AM -0700, Randy Dunlap wrote:
> I don't know what the __crc_* symbols are (I can't find them anywhere).
> 
> The other functions (pwm_config, pwm_free, pwm_request, pwm_disable,
> pwm_enable) exist in multiple places.  This is not good.
> They are very generically named.  The instances of these that are
> provided by platform code are OK (these):
> 
> ./include/linux/pwm.h:19:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
> ./arch/arm/plat-pxa/pwm.c:64:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> ./arch/arm/plat-pxa/pwm.c:101:EXPORT_SYMBOL(pwm_config);
> ./arch/arm/plat-samsung/pwm.c:194:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> ./arch/arm/plat-samsung/pwm.c:281:EXPORT_SYMBOL(pwm_config);
> ./arch/arm/plat-mxc/pwm.c:55:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> ./arch/arm/plat-mxc/pwm.c:114:EXPORT_SYMBOL(pwm_config);
> ./arch/mips/jz4740/pwm.c:94:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> 
> but the instances of these that are defined in
> drivers/mfd/twl6030-pwm.c should not be named so generically.
> 
> Changing (Fixing) the function names in twl6030-pwm.c should fix the build problem
> that you reported, I think.  
The pwm "API" relies on someone providing the pwm_* symbols, and then you have
leds_pwm or backlight_pwm calling those symbols out of the blue.
If we change the twl6030-pwm.c function names, no pwm users (backlight, led or
input) will actually be able to use the twl6030 PWM driver.
The pwm API is very poorly designed in my opinion and should provide a way for
pwm drivers to register against it. pwm users will then call into the pwm
framework who would select which driver to use.

> And it will still allow the ab8500 driver to be
> built on other platforms, which is what we prefer when that is possible.
I agree with that, but couldnt see any other fix with the current pwm
situation.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/