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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100915182145.GF2597@sortiz-mobl>
Date:	Wed, 15 Sep 2010 20:21:46 +0200
From:	Samuel Ortiz <sameo@...ux.intel.com>
To:	Randy Dunlap <randy.dunlap@...cle.com>
Cc:	Arun MURTHY <arun.murthy@...ricsson.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linus WALLEIJ <linus.walleij@...ricsson.com>,
	Mattias WALLIN <mattias.wallin@...ricsson.com>,
	Srinidhi KASAGAR <srinidhi.kasagar@...ricsson.com>
Subject: Re: [PATCH] mfd: ab8500: update kconfig for ab8500 core driver

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/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/