lkml.org 
[lkml]   [2011]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] s3c/s3c24xx: arm: leds: Make s3c24xx LEDS driver use gpiolib
On 11/22/2011 12:03 AM, Stephen Warren wrote:
> Denis Kuzmenko wrote at Monday, November 21, 2011 12:38 PM:
>> On 11/21/2011 08:07 PM, Stephen Warren wrote:
>>> Denis Kuzmenko wrote at Friday, November 18, 2011 4:17 PM:
>>>> On 11/19/2011 12:44 AM, Stephen Warren wrote:
> ...
>>>>> OK, I see the need for a pull of some kind (although aren't there meant
>>>>> to be ESD protection diodes for this purpose; relying on what are probably
>>>>> pretty weak pullup/down resistors doesn't seem like it will provide much
>>>>> protection at all).
>>>>
>>>> I don't mean pull as any kind of good protection. But it's much better
>>>> to have it than not.
>>>
>>> Hmm. I'm not entirely convinced. If the board already has a pull-up/down,
>>> it seems like it won't really make much difference to ESD, and you can't
>>> make any assumptions in the core driver about whether such an external
>>> resistor is already present. In fact, adding another pull resistor inside
>>> the SoC in parallel will reduce the overall resistance, and increase wasted
>>> power.
>>>
>>
>> I don't think it's a real protection. It's rather "mistake-proofing"
>> (Poka-Yoke).
>> You are right, I didn't considered additional pulls (however I can't
>> imagine tristate LED usage with additional external pull) and power
>> consumptions.
>> I was just wondering, why was pull needed in previous implementation.
>> Additional ESD protection was the only thing I could imagine. I don't
>> think it's needed there and I'm OK to remove pull-related code.
>> So I'll remove it, test and send patch V3?
>
> I don't see any pulls being configured in the original code at all,
> unless some of the s3c2410_* function have unexpected side-effect. The
> only related thing is in probe:
>
> /* no point in having a pull-up if we are always driving */
>
> if (pdata->flags & S3C24XX_LEDF_TRISTATE) {
> ..
> } else {
> s3c2410_gpio_pullup(pdata->gpio, 0);
>
> which I assume disables an pull in the case where the pin is always driven.
>
> So, yes, I'd say submit v3 without any pull manipulation at all.
>

Actually, "s3c2410_gpio_pullup(pdata->gpio, 0);" enables pull in the
same way I've done that. Here is it's code:

/* gpiolib wrappers until these are totally eliminated */

void s3c2410_gpio_pullup(unsigned int pin, unsigned int to)
{
int ret;

WARN_ON(to); /* should be none of these left */

if (!to) {
/* if pull is enabled, try first with up, and if that
* fails, try using down */

ret = s3c_gpio_setpull(pin, S3C_GPIO_PULL_UP);
if (ret)
s3c_gpio_setpull(pin, S3C_GPIO_PULL_DOWN);
} else {
s3c_gpio_setpull(pin, S3C_GPIO_PULL_NONE);
}
}

So pull is enabled in same "random" way as I did but for *opposite*
state of S3C24XX_LEDF_TRISTATE flag.
And again:

>> I was just wondering, why was pull needed in previous implementation.
>> Additional ESD protection was the only thing I could imagine. I don't
>> think it's needed there and I'm OK to remove pull-related code.
>> So I'll remove it, test and send patch V3?

--
Best regards, Denis Kuzmenko.


\
 
 \ /
  Last update: 2011-11-21 23:53    [W:0.059 / U:0.920 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site