All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Sascha Hauer <kernel@pengutronix.de>,
	Grant Likely <grant.likely@secretlab.ca>,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org
Subject: Re: [PATCH] gpio: twl4030: Use only TWL4030_MODULE_LED for LED configuration
Date: Mon, 19 Nov 2012 13:19:16 +0100	[thread overview]
Message-ID: <50AA23C4.1000706@ti.com> (raw)
In-Reply-To: <CACRpkdbvXFtHoOXThUrT4Z+8nO_HJCFq-May+4zaOsNv-886LA@mail.gmail.com>

Hi Linus,

On 11/19/2012 11:40 AM, Linus Walleij wrote:
> On Mon, Nov 19, 2012 at 9:52 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> 
>> I was actually tempted to remove the whole LED (PWM) thing from the
>> gpio-twl4030 driver. It was a big surprise to me to see something like this in
>> there.
> 
> It looks wrong. In theory I think this should be ripped out and moved to
> drivers/leds/leds-twl4030.c. But that would only be in case it was *always*
> exclusively used for a LED and as you say:
> 
>> It turns out that on BeagleBoard the USB host enable signal is connected to
>> LEDA (PWMA) of twl4030... It is an enable signal. Seriously. So what we do
>> here is either configure the PWMs as full on, or turn it off.
> 
> Sounds like some hardware engineer has been having fun or was
> just out of GPIOs to use... So this LED PWM is also used as a
> GPIO.

My guess it was out of fun. On BealgeBoard GPIO13 from twl4030 is free
(LEDSYNC/GPIO.13) so it could have been used for this, or GPIO16/17 as well.

> I think this part of the driver should be moved to
> drivers/pwm/pwm-twl4030.c and modeled as a PWM. If some
> platform need to use the PWM as if it was a GPIO then that's just a
> special usecase. (Setting duty cycles to something like
> INT_MAX and just switching polarity to toggle it...)
> 
> If it needs to be used by a LED there needs to be some generic
> LED type just using a standard pwm_request() to get some
> specific PWM, such as leds/leds-pwm.c or so. This way the
> PWM can be used for LEDs if need be or other things...

I have already sent a series to support the PWMs in twl4030/6030:
https://lkml.org/lkml/2012/11/8/219
It adds support for both type of PWMs (the one named as PWM and the ones named
as LED since they are just PWMs).

We already have leds/leds-pwm driver in upstream so we are going to use that
if there is a led connected to one of them:
https://lkml.org/lkml/2012/11/12/222

We can use the backlight-pwm if the LCD backlight has been hooked to one of
the PWM (as it is done on SDP4430).

This is why I was thinking of adding gpio/gpio-pwm driver. With that we can
use the PWM as GPO line.

> Sounds correct, Sascha?
> 
>> Either way this is wrong IMHO to handle the LEDA/B via the gpio-twl4030 driver.
> 
> It's confusing indeed.
> 
> So what we're dealing with is: a LED-specific PWM, being used
> as a PWM with eternal dutycycle and then being used as GPIO.
> 
> Well, we get to deal with it ... :-/

gpio/gpio-pwm driver?

-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Sascha Hauer <kernel@pengutronix.de>,
	Grant Likely <grant.likely@secretlab.ca>,
	<linux-kernel@vger.kernel.org>, <linux-omap@vger.kernel.org>
Subject: Re: [PATCH] gpio: twl4030: Use only TWL4030_MODULE_LED for LED configuration
Date: Mon, 19 Nov 2012 13:19:16 +0100	[thread overview]
Message-ID: <50AA23C4.1000706@ti.com> (raw)
In-Reply-To: <CACRpkdbvXFtHoOXThUrT4Z+8nO_HJCFq-May+4zaOsNv-886LA@mail.gmail.com>

Hi Linus,

On 11/19/2012 11:40 AM, Linus Walleij wrote:
> On Mon, Nov 19, 2012 at 9:52 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> 
>> I was actually tempted to remove the whole LED (PWM) thing from the
>> gpio-twl4030 driver. It was a big surprise to me to see something like this in
>> there.
> 
> It looks wrong. In theory I think this should be ripped out and moved to
> drivers/leds/leds-twl4030.c. But that would only be in case it was *always*
> exclusively used for a LED and as you say:
> 
>> It turns out that on BeagleBoard the USB host enable signal is connected to
>> LEDA (PWMA) of twl4030... It is an enable signal. Seriously. So what we do
>> here is either configure the PWMs as full on, or turn it off.
> 
> Sounds like some hardware engineer has been having fun or was
> just out of GPIOs to use... So this LED PWM is also used as a
> GPIO.

My guess it was out of fun. On BealgeBoard GPIO13 from twl4030 is free
(LEDSYNC/GPIO.13) so it could have been used for this, or GPIO16/17 as well.

> I think this part of the driver should be moved to
> drivers/pwm/pwm-twl4030.c and modeled as a PWM. If some
> platform need to use the PWM as if it was a GPIO then that's just a
> special usecase. (Setting duty cycles to something like
> INT_MAX and just switching polarity to toggle it...)
> 
> If it needs to be used by a LED there needs to be some generic
> LED type just using a standard pwm_request() to get some
> specific PWM, such as leds/leds-pwm.c or so. This way the
> PWM can be used for LEDs if need be or other things...

I have already sent a series to support the PWMs in twl4030/6030:
https://lkml.org/lkml/2012/11/8/219
It adds support for both type of PWMs (the one named as PWM and the ones named
as LED since they are just PWMs).

We already have leds/leds-pwm driver in upstream so we are going to use that
if there is a led connected to one of them:
https://lkml.org/lkml/2012/11/12/222

We can use the backlight-pwm if the LCD backlight has been hooked to one of
the PWM (as it is done on SDP4430).

This is why I was thinking of adding gpio/gpio-pwm driver. With that we can
use the PWM as GPO line.

> Sounds correct, Sascha?
> 
>> Either way this is wrong IMHO to handle the LEDA/B via the gpio-twl4030 driver.
> 
> It's confusing indeed.
> 
> So what we're dealing with is: a LED-specific PWM, being used
> as a PWM with eternal dutycycle and then being used as GPIO.
> 
> Well, we get to deal with it ... :-/

gpio/gpio-pwm driver?

-- 
Péter

  parent reply	other threads:[~2012-11-19 12:19 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-13  9:35 [PATCH] gpio: twl4030: Use only TWL4030_MODULE_LED for LED configuration Peter Ujfalusi
2012-11-13  9:35 ` Peter Ujfalusi
2012-11-17 20:16 ` Linus Walleij
2012-11-19  8:52   ` Peter Ujfalusi
2012-11-19  8:52     ` Peter Ujfalusi
2012-11-19 10:40     ` Linus Walleij
2012-11-19 10:57       ` Sascha Hauer
2012-11-19 12:22         ` Peter Ujfalusi
2012-11-19 12:22           ` Peter Ujfalusi
2012-11-19 12:19       ` Peter Ujfalusi [this message]
2012-11-19 12:19         ` Peter Ujfalusi
2012-11-20 18:42         ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50AA23C4.1000706@ti.com \
    --to=peter.ujfalusi@ti.com \
    --cc=grant.likely@secretlab.ca \
    --cc=kernel@pengutronix.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.