Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-11-05 Thread Tony Lindgren 
* Linus Walleij linus.wall...@linaro.org [121104 09:39]:
 On Tue, Oct 30, 2012 at 3:12 PM, AnilKumar, Chimata anilku...@ti.com wrote:
  I have two options now
  - add only default states for now, I can add reset of
  the state details once suspend/resume is supported.
  - add same values in all the states, modify those once
  suspend/resume is added for AM335x.
 
 The OMAP maintainer gets to choose how this is to be done,
 it's none of my business :-)

Either way is fine with me as long as the changes have been
tested and you don't have to redo things once you have
suspend and resume working.

Regards,

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-11-04 Thread Linus Walleij 
On Tue, Oct 30, 2012 at 3:12 PM, AnilKumar, Chimata anilku...@ti.com wrote:

 I completely understood this named modes, I have added named
 modes like this in am335x-xxx.dts files

I do not understand how the pinctrl-single dts files work actually,
so please get Tony to review this part.

 I think pinctrl-1 state will be used in driver
 suspend/resume calls.

Hopefully, I think you should test the code and monitor the
system to make sure the right thing happens.

 I have the pinmux/conf settings for default state but fully
 optimized pinmux/conf values in idle  suspend states are not
 available yet.

You have defined a sleep state which is what should be used
for suspend? Or do you mean that you do have a state but
you're just not defining it to the most optimal setting yet?

 Even though if I add here, I am unable to test
 these pins in suspend state because suspend/resume of AM35xx
 is not added yet

Aha.

 I have two options now
 - add only default states for now, I can add reset of
 the state details once suspend/resume is supported.
 - add same values in all the states, modify those once
 suspend/resume is added for AM335x.

The OMAP maintainer gets to choose how this is to be done,
it's none of my business :-)

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-11-04 Thread AnilKumar, Chimata 
On Sun, Nov 04, 2012 at 23:07:44, Linus Walleij wrote:
 On Tue, Oct 30, 2012 at 3:12 PM, AnilKumar, Chimata anilku...@ti.com wrote:
 
  I completely understood this named modes, I have added named
  modes like this in am335x-xxx.dts files
 
 I do not understand how the pinctrl-single dts files work actually,
 so please get Tony to review this part.
 
  I think pinctrl-1 state will be used in driver
  suspend/resume calls.
 
 Hopefully, I think you should test the code and monitor the
 system to make sure the right thing happens.

I will test while adding pinctrl-1 data to .dts files.

 
  I have the pinmux/conf settings for default state but fully
  optimized pinmux/conf values in idle  suspend states are not
  available yet.
 
 You have defined a sleep state which is what should be used
 for suspend? Or do you mean that you do have a state but
 you're just not defining it to the most optimal setting yet?

Yes, sleep state is used for suspend. Regarding to this suspend
state fully optimized pinmux/conf values are not available.

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

RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-10-30 Thread AnilKumar, Chimata 
On Wed, Oct 03, 2012 at 18:06:10, Linus Walleij wrote:
 On Wed, Oct 3, 2012 at 12:52 PM, AnilKumar, Chimata anilku...@ti.com wrote:
  On Tue, Oct 02, 2012 at 01:29:37, Linus Walleij wrote:
 
  This is what we're doing for ux500 and should be a good model.
 
  I have looked into this, but not seen any named modes.
 
 OK maybe it's not easy to find. If you look into:
 arch/arm/mach-ux500/board-mop500-pins.c
 you find our work in progress. Note that this is not (yet)
 using device tree. (We want to migrate all our pinctrl
 stuff first, then do DT.)
 
 So for example this macro:
 
 #define DB8500_PIN(pin,conf,dev) \
 PIN_MAP_CONFIGS_PIN_DEFAULT(dev, pinctrl-db8500, pin, conf)
 
 Will define a config for the default mode for a certain
 pin.
 
 This:
 
 #define DB8500_PIN_SLEEP(pin, conf, dev) \
 PIN_MAP_CONFIGS_PIN(dev, PINCTRL_STATE_SLEEP, pinctrl-db8500, \
 pin, conf)
 
 Will mutatis mutandis define a sleep mode for a pin.
 
 Sorry for the macros. We'll get rid of them in the DT.
 (Now that Stephen has patched in preprocessing it will
 even look good.)
 

Hi Linus Walleij/Tony,

I completely understood this named modes, I have added named
modes like this in am335x-xxx.dts files

am33xx_pinmux: pinmux@44e10800 {
pinctrl-names = default, sleep;
pinctrl-0 = user_leds_s0;
pinctrl-1 = user_leds_s1;

user_leds_s0: user_leds_s0 {
pinctrl-single,pins = 
0x54 0x7/* gpmc_a5.gpio1_21, OUTPUT | MODE7 */
0x58 0x17   /* gpmc_a6.gpio1_22, OUT PULLUP | MODE7 
*/
0x5c 0x7/* gpmc_a7.gpio1_23, OUTPUT | MODE7 */
0x60 0x17   /* gpmc_a8.gpio1_24, OUT PULLUP | MODE7 
*/
;
};

user_leds_s1: user_leds_s1 {
pinctrl-single,pins = 
0x54 0x2e   /* gpmc_a5.gpio1_21, INPUT | MODE7 */
0x58 0x2e   /* gpmc_a6.gpio1_22, INPUT | MODE7 */
0x5c 0x2e   /* gpmc_a7.gpio1_23, INPUT | MODE7 */
0x60 0x2e   /* gpmc_a8.gpio1_24, INPUT | MODE7 */
;
};
};

I think pinctrl-1 state will be used in driver
suspend/resume calls.

I have the pinmux/conf settings for default state but fully
optimized pinmux/conf values in idle  suspend states are not
available yet. Even though if I add here, I am unable to test
these pins in suspend state because suspend/resume of AM35xx
is not added yet

I have two options now
- add only default states for now, I can add reset of
the state details once suspend/resume is supported. 
- add same values in all the states, modify those once
suspend/resume is added for AM335x.

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

RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-10-03 Thread AnilKumar, Chimata 
On Tue, Oct 02, 2012 at 01:29:37, Linus Walleij wrote:
 On Mon, Oct 1, 2012 at 5:44 PM, Tony Lindgren t...@atomide.com wrote:
 
  OK that is typical pinctrl driver implementation work.
  I hope Tony can advice on this?
 
  I think we're best off to just stick to alternative named modes
  passed from device tree. For example, for GPIO wake-ups you can
  have named modes such as default, enabled and idle where
  idle muxes things for GPIO wake-ups for the duration of idle.
 

In this case we need to add three different values according
to three modes (default, enabled, idle) and for each node.

  It seems that should also work for leds-gpio. And you can
  define more named modes as needed.

If we want to implement pinctrl_gpio functionality we have to
separate function-mask bits to

1. pinmux-mask
2. pinconf-mask, to make it generic we need following bit masks
a. receiver enable/disable bit
b. slew rate fast/slow bit
c. pull-up/down bit


I have gone through nvidia pinctrl dt data (tegra20-seaboard.dts,
node drive_sdio1) which has different pinconfig values, those
are mapping to pinconf values.

With the above bit masks and function-mask we can identify
pull-up/down, slow/high speed slew rate and direction in/out.

(or)

Named modes:-

Are you saying named modes like this?
default-input-up
default-input-down
default-output-up
default-output-down

This 1, 2 and 2.a or named modes are required to implement
pinctrl_gpio_direction_input/output and
pinctrl_request/free_gpio.

 
 
 This is what we're doing for ux500 and should be a good model.

I have looked into this, but not seen any named modes.

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-10-03 Thread Linus Walleij 
On Wed, Oct 3, 2012 at 12:52 PM, AnilKumar, Chimata anilku...@ti.com wrote:
 On Tue, Oct 02, 2012 at 01:29:37, Linus Walleij wrote:

 This is what we're doing for ux500 and should be a good model.

 I have looked into this, but not seen any named modes.

OK maybe it's not easy to find. If you look into:
arch/arm/mach-ux500/board-mop500-pins.c
you find our work in progress. Note that this is not (yet)
using device tree. (We want to migrate all our pinctrl
stuff first, then do DT.)

So for example this macro:

#define DB8500_PIN(pin,conf,dev) \
PIN_MAP_CONFIGS_PIN_DEFAULT(dev, pinctrl-db8500, pin, conf)

Will define a config for the default mode for a certain
pin.

This:

#define DB8500_PIN_SLEEP(pin, conf, dev) \
PIN_MAP_CONFIGS_PIN(dev, PINCTRL_STATE_SLEEP, pinctrl-db8500, \
pin, conf)

Will mutatis mutandis define a sleep mode for a pin.

Sorry for the macros. We'll get rid of them in the DT.
(Now that Stephen has patched in preprocessing it will
even look good.)

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-10-03 Thread Tony Lindgren 
* AnilKumar, Chimata anilku...@ti.com [121003 03:53]:
 On Tue, Oct 02, 2012 at 01:29:37, Linus Walleij wrote:
  On Mon, Oct 1, 2012 at 5:44 PM, Tony Lindgren t...@atomide.com wrote:
  
   OK that is typical pinctrl driver implementation work.
   I hope Tony can advice on this?
  
   I think we're best off to just stick to alternative named modes
   passed from device tree. For example, for GPIO wake-ups you can
   have named modes such as default, enabled and idle where
   idle muxes things for GPIO wake-ups for the duration of idle.
  
 
 In this case we need to add three different values according
 to three modes (default, enabled, idle) and for each node.

Yes those make sense from the generic leds-gpio point of view
for the platforms that implement pinctrl.
 
   It seems that should also work for leds-gpio. And you can
   define more named modes as needed.
 
 If we want to implement pinctrl_gpio functionality we have to
 separate function-mask bits to
 
 1. pinmux-mask
 2. pinconf-mask, to make it generic we need following bit masks
   a. receiver enable/disable bit
   b. slew rate fast/slow bit
   c. pull-up/down bit
   

Yes those can be implemented, but the problem will always be
that the driver will not know if the board is using external
pulls. If you implement the board specific settings in the .dts
file for default, enabled and idle, the leds-gpio does not need
to care if the pull is internal or external. So that seems like
a more generic way to do it.
 
 I have gone through nvidia pinctrl dt data (tegra20-seaboard.dts,
 node drive_sdio1) which has different pinconfig values, those
 are mapping to pinconf values.
 
 With the above bit masks and function-mask we can identify
 pull-up/down, slow/high speed slew rate and direction in/out.
 
 (or)
 
 Named modes:-
 
 Are you saying named modes like this?
 default-input-up
 default-input-down
 default-output-up
 default-output-down

Hmm no, you want to implement named modes that make sense
from the client driver point of view. It seems that default,
enabled and idle should do here? Then for the enabled mode
you can set the LED specific pins to whatever pull mode
you want for the board, and then leds-gpio does the rest
using the gpio framework.
 
 This 1, 2 and 2.a or named modes are required to implement
 pinctrl_gpio_direction_input/output and
 pinctrl_request/free_gpio.

I would just add the named modes to leds-gpio because 2a
does not work for the case where you use external pulls.

Regards,

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

RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-10-01 Thread AnilKumar, Chimata 
+Don Aisheng

On Tue, Sep 11, 2012 at 01:10:12, Linus Walleij wrote:
 On Sat, Sep 1, 2012 at 10:16 AM, AnilKumar Ch anilku...@ti.com wrote:
 
  Adopt pinctrl support to leds-gpio driver based on leds-gpio
  device pointer, pinctrl driver configure SoC pins to GPIO
  mode according to definitions provided in .dts file.
 
  Signed-off-by: AnilKumar Ch anilku...@ti.com
 
 So looking back at this after Stephen posed a real good
 question, when you say configure SoC pins to GPIO
 mode, does that mean anything else than to mux them into
 GPIO mode?


pinctrl-single.c driver sets mux mode as well as pin configuration
like pull-up/pull-down, input/output and slew rate.

 In that case, have you considered augmenting
 pinctrl-single.c to implement .gpio_request_enable()
 .gpio_disable_free() and maybe also .gpio_set_direction()
 in its struct pinmux_ops pinmux backend?
 
 If not, why?

Recently, I have gone through the details on implementing
gpio_request_enable in pinctrl-single driver. To add this
functionality we have to add gpio_range's to pinctrl driver.

AM335x EVM supports total 128 GPIO pins (4 banks) and these
are randomly distributed across AM33XX SoC pins.

These are the concerns/questions I have:-
1. I haven't added yet but it will come to more than 30-40
pinctrl_gpio_range entries
2. If the silicon package is changed then these will change.
3. If the GPIO driver is common for multiple SoCs then these
entries will be more  more over SoC specific and driver has
to change every time the gpio_range changes. 

   I have gone through the Don Aisheng patch series, which
adds pinctrl_dt_add_gpio_ranges support but not accepted
yet. With this patch series we can overcome the driver changes.

4. The current pinctrl driver has support for these APIs
pinctrl_request_gpio(), pinctrl_free_gpio(),
pinctrl_gpio_direction_input/output()
no API for slew rate control, pulled down/up control
how can we handle this?
5. pinctrl-single driver has to modify to provide separate handles
for pinmux and pinconfig. And we need separate pin configuration
bit masks and values/flags to take care of pull-up/down, slew rate,
receiver in/out and mux mode control.
6. This is for my understanding, on which node do we have to add
pinctrl data i.e., pin mux-mode data. In leds-gpio node (mostly not)
and if it is in gpio node then how can we pass pinmux data with
the existing API pinctrl_request_gpio(gpio);? Here we are passing
only gpio number.

Few more driver patches are pending along with this (leds-gpio DT
data additions according to this patch, similarly other drivers
like matrix keypad and volume keys)

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-10-01 Thread Linus Walleij 
On Mon, Oct 1, 2012 at 9:03 AM, AnilKumar, Chimata anilku...@ti.com wrote:

I have gone through the Don Aisheng patch series, which
 adds pinctrl_dt_add_gpio_ranges support but not accepted
 yet. With this patch series we can overcome the driver changes.

OK then this is the direction we need to go.

 4. The current pinctrl driver has support for these APIs
 pinctrl_request_gpio(), pinctrl_free_gpio(),
 pinctrl_gpio_direction_input/output()
 no API for slew rate control, pulled down/up control
 how can we handle this?

You are not supposed to handle that from the GPIO level
of the API. That is supposed to be handled by pinctrl.

These two subsystems are orthogonal, with the exception
of the above calls. If you need to pass more information
between the GPIO and pinctrl interfaces it usually means
you're doing something wrong.

The reason why pinctrl was created in the first place
was that Grant didn't like that we started to shoehorn all
kind of pin control into the GPIO subsystem.

 5. pinctrl-single driver has to modify to provide separate handles
 for pinmux and pinconfig. And we need separate pin configuration
 bit masks and values/flags to take care of pull-up/down, slew rate,
 receiver in/out and mux mode control.

OK that is typical pinctrl driver implementation work.
I hope Tony can advice on this?

 6. This is for my understanding, on which node do we have to add
 pinctrl data i.e., pin mux-mode data. In leds-gpio node (mostly not)
 and if it is in gpio node then how can we pass pinmux data with
 the existing API pinctrl_request_gpio(gpio);? Here we are passing
 only gpio number.

So with the pinctrl_request_gpio() call you are requesting a single
pin to be used as GPIO, nothing else.

No additional data should be passed with that call.

Implementing it is up to the pinctrl driver, the pinctrl subsystem
API does not say anything about how this should be done, but
there are a few examples.

The pinctrl maps of muxes and config from the pin control
subsystem are for entire devices, and the single-pin GPIO
reservation API is orthogonal to this, please consult
Documentation/pinctrl.txt if you are uncertain about what
I mean with this, I've really tried to explain it there.

The docs were recently amended to reflect some corner-case
GPIO uses, see e.g.:
http://marc.info/?l=linux-arm-kernelm=134763067415678w=2

 Few more driver patches are pending along with this (leds-gpio DT
 data additions according to this patch, similarly other drivers
 like matrix keypad and volume keys)

OK so the threshold is that we need to get it right for the first
one and then the others will look good too.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-10-01 Thread Tony Lindgren 
* Linus Walleij linus.wall...@linaro.org [121001 01:25]:
 On Mon, Oct 1, 2012 at 9:03 AM, AnilKumar, Chimata anilku...@ti.com wrote:
 
 I have gone through the Don Aisheng patch series, which
  adds pinctrl_dt_add_gpio_ranges support but not accepted
  yet. With this patch series we can overcome the driver changes.
 
 OK then this is the direction we need to go.
 
  4. The current pinctrl driver has support for these APIs
  pinctrl_request_gpio(), pinctrl_free_gpio(),
  pinctrl_gpio_direction_input/output()
  no API for slew rate control, pulled down/up control
  how can we handle this?
 
 You are not supposed to handle that from the GPIO level
 of the API. That is supposed to be handled by pinctrl.
 
 These two subsystems are orthogonal, with the exception
 of the above calls. If you need to pass more information
 between the GPIO and pinctrl interfaces it usually means
 you're doing something wrong.
 
 The reason why pinctrl was created in the first place
 was that Grant didn't like that we started to shoehorn all
 kind of pin control into the GPIO subsystem.

Agreed.
 
  5. pinctrl-single driver has to modify to provide separate handles
  for pinmux and pinconfig. And we need separate pin configuration
  bit masks and values/flags to take care of pull-up/down, slew rate,
  receiver in/out and mux mode control.
 
 OK that is typical pinctrl driver implementation work.
 I hope Tony can advice on this?

I think we're best off to just stick to alternative named modes
passed from device tree. For example, for GPIO wake-ups you can
have named modes such as default, enabled and idle where
idle muxes things for GPIO wake-ups for the duration of idle.

It seems that should also work for leds-gpio. And you can
define more named modes as needed.

You really don't want the client driver or the GPIO driver doing
things like pull-up/down automatically as that is board specific and
can also depend on things like externall pull resistors.

  6. This is for my understanding, on which node do we have to add
  pinctrl data i.e., pin mux-mode data. In leds-gpio node (mostly not)
  and if it is in gpio node then how can we pass pinmux data with
  the existing API pinctrl_request_gpio(gpio);? Here we are passing
  only gpio number.
 
 So with the pinctrl_request_gpio() call you are requesting a single
 pin to be used as GPIO, nothing else.
 
 No additional data should be passed with that call.

Yeah I agree.
 
 Implementing it is up to the pinctrl driver, the pinctrl subsystem
 API does not say anything about how this should be done, but
 there are a few examples.

 The pinctrl maps of muxes and config from the pin control
 subsystem are for entire devices, and the single-pin GPIO
 reservation API is orthogonal to this, please consult
 Documentation/pinctrl.txt if you are uncertain about what
 I mean with this, I've really tried to explain it there.
 
 The docs were recently amended to reflect some corner-case
 GPIO uses, see e.g.:
 http://marc.info/?l=linux-arm-kernelm=134763067415678w=2
 
  Few more driver patches are pending along with this (leds-gpio DT
  data additions according to this patch, similarly other drivers
  like matrix keypad and volume keys)
 
 OK so the threshold is that we need to get it right for the first
 one and then the others will look good too.

It seems we want to keep leds-gpio, gpio framework and pinctrl
framework generic. It also seems you should be able to do
what you're describing using the pinctrl named modes.

Regards,

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-10-01 Thread Linus Walleij 
On Mon, Oct 1, 2012 at 5:44 PM, Tony Lindgren t...@atomide.com wrote:

 OK that is typical pinctrl driver implementation work.
 I hope Tony can advice on this?

 I think we're best off to just stick to alternative named modes
 passed from device tree. For example, for GPIO wake-ups you can
 have named modes such as default, enabled and idle where
 idle muxes things for GPIO wake-ups for the duration of idle.

 It seems that should also work for leds-gpio. And you can
 define more named modes as needed.


This is what we're doing for ux500 and should be a good model.

 You really don't want the client driver or the GPIO driver doing
 things like pull-up/down automatically as that is board specific and
 can also depend on things like externall pull resistors.

Nope. We've had instances of people getting bad leakage
because of pulling down a line where there is already a
pull-down resistor on the board :-(

 OK so the threshold is that we need to get it right for the first
 one and then the others will look good too.

 It seems we want to keep leds-gpio, gpio framework and pinctrl
 framework generic. It also seems you should be able to do
 what you're describing using the pinctrl named modes.

I think so too.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-10 Thread Linus Walleij 
On Sun, Sep 9, 2012 at 1:44 AM, Domenico Andreoli cav...@gmail.com wrote:
 On Fri, Sep 07, 2012 at 11:57:59PM +0200, Linus Walleij wrote:

 If all you need to to is to multiplex the pins into GPIO mode,
 then the gpio_get() call on this driver *can* call through to
 pinctrl_request_gpio() which will in turn fall through to the
 above pinmux driver calls (.gpio_request_enable, etc).

 So if the GPIO driver doesn't coordinate with the pinctrl driver, it's
 all left to the GPIO user to configure the pin before using it, right?

Yes, more or less, or should I say that certain aspects of pinctrl
are orthogonal to GPIO and the two mostly do not know of each
other due to a separation of concerns.

So the driver may need to tie things up and request its pinctrl
and GPIOs independently.

 I can understand the concerns of Tony, whether a pin must be requested
 or not before the gpio then depends on the GPIO driver implementation,
 which may or may not call through the pinctrl layer, isn't it?.

Yes that is a sematic limitation, indeed.

I think the best way of trying to eliminate that is to bring the two
subsystems closer (which is some long-term project) but in the
meantime we could propose a documentation fixup to make the
semantics clear, what about this:

From a92d754367861cf564c09e0b15746e02f0a96f3f Mon Sep 17 00:00:00 2001
From: Linus Walleij linus.wall...@linaro.org
Date: Mon, 10 Sep 2012 17:22:00 +0200
Subject: [PATCH] pinctrl: document semantics vs GPIO

The semantics of the interactions between GPIO and pinctrl may be
unclear, e.g. which one do you request first? This amends the
documentation to make this clear.

Reported-by: Domenico Andreoli cav...@gmail.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
 Documentation/pinctrl.txt | 24 
 1 file changed, 24 insertions(+)

diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt
index 1479aca..941e783 100644
--- a/Documentation/pinctrl.txt
+++ b/Documentation/pinctrl.txt
@@ -289,6 +289,29 @@ Interaction with the GPIO subsystem
 The GPIO drivers may want to perform operations of various types on the same
 physical pins that are also registered as pin controller pins.

+First and foremost, the two subsystems can be used as completely orthogonal,
+so say that your driver is fetching its resources like this:
+
+#include linux/pinctrl/consumer.h
+#include linux/gpio.h
+
+struct pinctrl *pinctrl;
+int gpio;
+
+pinctrl = devm_pinctrl_get_select_default(dev);
+gpio = devm_gpio_request(dev, 14, foo);
+
+Here we first request a certain pin state and then request GPIO 14 to be
+used. If you're using the subsystems orthogonally like this, always get
+your pinctrl handle and select the desired pinctrl state BEFORE requesting
+the GPIO. This is a semantic convention to avoid situations that can be
+electrically unpleasant, you may certainly want to mux in and bias pins
+a certain way before the GPIO subsystems starts to deal with them.
+
+But there are also situations where it makes sense for the GPIO subsystem
+to communicate with with the pinctrl subsystem, using the latter as a
+back-end.
+
 Since the pin controller subsystem have its pinspace local to the pin
 controller we need a mapping so that the pin control subsystem can figure out
 which pin controller handles control of a certain GPIO pin. Since a single
@@ -359,6 +382,7 @@ will get an pin number into its handled number
range. Further it is also passed
 the range ID value, so that the pin controller knows which range it should
 deal with.

+
 PINMUX interfaces
 =

-- 
1.7.11.3


Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-10 Thread Stephen Warren 
On 09/10/2012 09:23 AM, Linus Walleij wrote:
 On Sun, Sep 9, 2012 at 1:44 AM, Domenico Andreoli cav...@gmail.com wrote:
 On Fri, Sep 07, 2012 at 11:57:59PM +0200, Linus Walleij wrote:

 If all you need to to is to multiplex the pins into GPIO mode,
 then the gpio_get() call on this driver *can* call through to
 pinctrl_request_gpio() which will in turn fall through to the
 above pinmux driver calls (.gpio_request_enable, etc).

 So if the GPIO driver doesn't coordinate with the pinctrl driver, it's
 all left to the GPIO user to configure the pin before using it, right?
 
 Yes, more or less, or should I say that certain aspects of pinctrl
 are orthogonal to GPIO and the two mostly do not know of each
 other due to a separation of concerns.
 
 So the driver may need to tie things up and request its pinctrl
 and GPIOs independently.

That seems like exactly what we were trying to avoid when we added the
possibility for GPIO to call into pinctrl.

Documentation/gpio.txt already contains:

 For GPIOs that use pins known to the pinctrl subsystem, that subsystem should
 be informed of their use; a gpiolib driver's .request() operation may call
 pinctrl_request_gpio(), and a gpiolib driver's .free() operation may call
 pinctrl_free_gpio(). The pinctrl subsystem allows a pinctrl_request_gpio()
 to succeed concurrently with a pin or pingroup being owned by a device for
 pin multiplexing.

In order to resolve this, shouldn't we simply change the should at the
end of the first line I quoted to must? That way, there'd never be any
need to use pinctrl if you're only relying on gpiolib APIs.

(and I'd argue that the text was already meant to say must, so this
isn't actually a change to the intent, just a clarification).
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-10 Thread Linus Walleij 
On Sat, Sep 1, 2012 at 10:16 AM, AnilKumar Ch anilku...@ti.com wrote:

 Adopt pinctrl support to leds-gpio driver based on leds-gpio
 device pointer, pinctrl driver configure SoC pins to GPIO
 mode according to definitions provided in .dts file.

 Signed-off-by: AnilKumar Ch anilku...@ti.com

So looking back at this after Stephen posed a real good
question, when you say configure SoC pins to GPIO
mode, does that mean anything else than to mux them into
GPIO mode?

In that case, have you considered augmenting
pinctrl-single.c to implement .gpio_request_enable()
.gpio_disable_free() and maybe also .gpio_set_direction()
in its struct pinmux_ops pinmux backend?

If not, why?

Currently it looks like this:

static int pcs_request_gpio(struct pinctrl_dev *pctldev,
struct pinctrl_gpio_range *range, unsigned offset)
{
return -ENOTSUPP;
}

static struct pinmux_ops pcs_pinmux_ops = {
.get_functions_count = pcs_get_functions_count,
.get_function_name = pcs_get_function_name,
.get_function_groups = pcs_get_function_groups,
.enable = pcs_enable,
.disable = pcs_disable,
.gpio_request_enable = pcs_request_gpio,
};

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-10 Thread Stephen Warren 
On 09/10/2012 01:34 PM, Linus Walleij wrote:
 On Mon, Sep 10, 2012 at 7:41 PM, Stephen Warren swar...@wwwdotorg.org wrote:
 On 09/10/2012 09:23 AM, Linus Walleij wrote:
 
 That seems like exactly what we were trying to avoid when we added the
 possibility for GPIO to call into pinctrl.

 Documentation/gpio.txt already contains:

 For GPIOs that use pins known to the pinctrl subsystem, that subsystem 
 should
 be informed of their use; a gpiolib driver's .request() operation may call
 pinctrl_request_gpio(), and a gpiolib driver's .free() operation may call
 pinctrl_free_gpio(). The pinctrl subsystem allows a pinctrl_request_gpio()
 to succeed concurrently with a pin or pingroup being owned by a device for
 pin multiplexing.

 In order to resolve this, shouldn't we simply change the should at the
 end of the first line I quoted to must? That way, there'd never be any
 need to use pinctrl if you're only relying on gpiolib APIs.

 (and I'd argue that the text was already meant to say must, so this
 isn't actually a change to the intent, just a clarification).
 
 It should deal with all the simple muxing use cases yes. And
 I am uncertain about the scope for this patch, if it only pertains
 to muxing, and in that case it would be solved by adding
 a proper GPIO backend to pinctrl-single.c.
 
 But it doesn't help with some real-world usecases if I'm
 not mistaken.
 
 If you want to set up a certain GPIO pin as pull-down (I guess
 this could be the case for a LED array), this cannot be done
 through any of these functions:
 
 extern int pinctrl_request_gpio(unsigned gpio);
 extern void pinctrl_free_gpio(unsigned gpio);
 extern int pinctrl_gpio_direction_input(unsigned gpio);
 extern int pinctrl_gpio_direction_output(unsigned gpio);
 
 So either we have to use a pin config hog to do this,

I'd certainly expect that to be the common case; I'd imagine it's pretty
common you'd never want to change the pulls at runtime, so hogging would
be appropriate.

 or we have
 to use devm_pinctrl_get_select_default(pdev-dev); from the
 driver (as this patch does).

Yes, true.

 Either way it is using the pinctrl
 system orthogonally to the GPIO system, it doesn't happen
 from pinctrl_request_gpio() or so.
 
 An alternative solution would be to add functions for
 controlling pinconfig and whatnot to the GPIO glue, which
 in turn would require adding frontends all over linux/gpio.h
 which in turn was the thing that Grant nixed to I got
 started with pinctrl instead...

Maybe the first gpio_request that GPIO passes to pinctrl could activate
some default gpio state or similar? But then you'd get into issues
with: what if the driver selects a pinctrl state for other reasons -
then you'd end up wanting multiple states active at once; the
gpiolib-requested state and the driver-requested state, and maybe they
conflict, ... probably madness ensues!

 But I'm open to any other suggestions. Would it be possible
 for pinctrl_request_gpio() to activate a pin config in the
 map for example? Currently it can only do muxing.
 
 It's also possible to have the driver do something custom
 behind the back of pinctrl altogether as a response to
 pinctrl_request_gpio() but it wouldn't be
 any elegant...

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-08 Thread Domenico Andreoli 
On Fri, Sep 07, 2012 at 11:57:59PM +0200, Linus Walleij wrote:
 On Fri, Sep 7, 2012 at 6:00 PM, Domenico Andreoli cav...@gmail.com wrote:
  On Fri, Sep 07, 2012 at 02:30:59PM +, AnilKumar, Chimata wrote:
 
  How can gpio driver knows that leds-gpio driver require
  these 4 pins?
 
  because leds-gpio requests each gpio (specified in the DT against a specific
  gpio controller) before assuming it is available?  gpiolib then asks to
  pinctrl if those pins are available for gpio and possibly reserve them
  (configuring the mux, if necessary) for the device.
 
 So this is not an either-or situation but a both-and case.

...
 
 If all you need to to is to multiplex the pins into GPIO mode,
 then the gpio_get() call on this driver *can* call through to
 pinctrl_request_gpio() which will in turn fall through to the
 above pinmux driver calls (.gpio_request_enable, etc).

So if the GPIO driver doesn't coordinate with the pinctrl driver, it's
all left to the GPIO user to configure the pin before using it, right?

I can understand the concerns of Tony, whether a pin must be requested
or not before the gpio then depends on the GPIO driver implementation,
which may or may not call through the pinctrl layer, isn't it?.

 But that's as far as it goes! The GPIO abstraction cannot
 call through to e.g. set some specific biasing on the pins
 (pull up etc). Doing that would require us to reimplement
 every interface that pinctrl already has again in the
 GPIO layer, which is not a good idea.

Yes, clear. Never meant that, I thought that the pinctrl was anyway
available for stuff not modeled by the GPIO layer, as you say below.

 So the pinctrl handle can be used for such config, and it
 can also be used for multiplexing if that is desired - if not
 done by the fall through functions pinctrl_gpio_*().
 
 You can use a combination of both too, Stephen patched
 pinctrl some time back so that a pin can be muxed for a
 certain function and used as GPIO at the same time, so
 these two are now orthogonal, it's a bit relaxed and gives some
 feeling of loss of control but was necessary for certain
 usecases. (For example we can snoop on a I2C pin using
 its corresponding GPIO registers in the U300...)

 There is some flexibility here, I hope it's not too confusing :-/

Thank you for clarifying :)

Regards,
Domenico
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread AnilKumar, Chimata 
On Fri, Sep 07, 2012 at 05:39:35, Marek Vasut wrote:
 Dear Tony Lindgren,
 
  * Marek Vasut ma...@denx.de [120905 19:05]:
   Hi Tony,
   
* Marek Vasut ma...@denx.de [120904 20:13]:
 Dear Bryan Wu,
 
  On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch anilku...@ti.com 
  wrote:
   Adopt pinctrl support to leds-gpio driver based on leds-gpio
   device pointer, pinctrl driver configure SoC pins to GPIO
   mode according to definitions provided in .dts file.
  
  Thanks for this, actually Marek Vasut submitted a similar patch
  before. I'm pretty fine with this patch.
 
 Thanks for submitting this actually ... I didn't have time to
 properly investigate this.
 
  But without proper DT setting, it will also give us warning I
  think. or we can provide some dummy functions as a temp solution
  as Shawn pointed out before.
 
 But this driver is also used on hardware that's not yet coverted to
 DT, so I'd say dev_warn() if CONFIG_OF is enabled and otherwise
 simply go on ? Actually, can we not skip whole this pinctrl thing if
 CONFIG_OF is disabled? Actually (2), what's the relationship between
 OF and pinctrl?

The warning should be pinctrl related as the pinctrl drivers may not be
device tree based drivers.
   
   Exactly my concern. Also the warning shouldnt be present on systems where
   pinctrl is disabled.
  
  But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if
  CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)
 
 Oh all right then.
 

Bryan,

If this patch looks fine, can you queue this for 3.7?

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread Marek Vasut 
Dear AnilKumar, Chimata,

 On Fri, Sep 07, 2012 at 05:39:35, Marek Vasut wrote:
  Dear Tony Lindgren,
  
   * Marek Vasut ma...@denx.de [120905 19:05]:
Hi Tony,

 * Marek Vasut ma...@denx.de [120904 20:13]:
  Dear Bryan Wu,
  
   On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch anilku...@ti.com 
wrote:
Adopt pinctrl support to leds-gpio driver based on leds-gpio
device pointer, pinctrl driver configure SoC pins to GPIO
mode according to definitions provided in .dts file.
   
   Thanks for this, actually Marek Vasut submitted a similar patch
   before. I'm pretty fine with this patch.
  
  Thanks for submitting this actually ... I didn't have time to
  properly investigate this.
  
   But without proper DT setting, it will also give us warning I
   think. or we can provide some dummy functions as a temp
   solution as Shawn pointed out before.
  
  But this driver is also used on hardware that's not yet coverted
  to DT, so I'd say dev_warn() if CONFIG_OF is enabled and
  otherwise simply go on ? Actually, can we not skip whole this
  pinctrl thing if CONFIG_OF is disabled? Actually (2), what's the
  relationship between OF and pinctrl?
 
 The warning should be pinctrl related as the pinctrl drivers may
 not be device tree based drivers.

Exactly my concern. Also the warning shouldnt be present on systems
where pinctrl is disabled.
   
   But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h
   if CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)
  
  Oh all right then.
 
 Bryan,
 
 If this patch looks fine, can you queue this for 3.7?

Looks good to me.

 Thanks
 AnilKumar

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread AnilKumar, Chimata 
Hi Domenico,

On Fri, Sep 07, 2012 at 14:18:39, Domenico Andreoli wrote:
 On Sat, Sep 1, 2012 at 10:16 AM, AnilKumar Ch anilku...@ti.com wrote:
  Adopt pinctrl support to leds-gpio driver based on leds-gpio
  device pointer, pinctrl driver configure SoC pins to GPIO
  mode according to definitions provided in .dts file.
 
 Shouldn't be the interaction with the pinctrl layer left to gpiolib?
 

No, these gpio's are configured specifically for user leds.

So, leds-gpio driver should have this call, because these gpio
pins are used by leds-gpio driver.

+   am33xx_pinmux: pinmux@44e10800 {
+   userled_pins: pinmux_userled_pins {
+   pinctrl-single,pins = 
+   0x54 0x7   
+   0x58 0x17   
+   0x5c 0x7
+   0x60 0x17   
+   ;
+   };
+   };
+

[...]

+   leds {
+   compatible = gpio-leds;
+   pinctrl-names = default;
+   pinctrl-0 = userled_pins;
  

This devm_pinctrl_get_select_default() call in leds-gpio driver
will internally take userled_pins node and configure those pins
according to the above definitions.

Lets take gpio-keypad driver, in that case we have to configure
pins as INPUT mode (generic gpio driver might not know what
the end usecase is) and this leds case we configure as OUTPUT
mode.

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread Domenico Andreoli 
On Fri, Sep 07, 2012 at 09:10:50AM +, AnilKumar, Chimata wrote:
 Hi Domenico,

Hi,

 On Fri, Sep 07, 2012 at 14:18:39, Domenico Andreoli wrote:
  On Sat, Sep 1, 2012 at 10:16 AM, AnilKumar Ch anilku...@ti.com wrote:
   Adopt pinctrl support to leds-gpio driver based on leds-gpio
   device pointer, pinctrl driver configure SoC pins to GPIO
   mode according to definitions provided in .dts file.
  
  Shouldn't be the interaction with the pinctrl layer left to gpiolib?
  
 
 No, these gpio's are configured specifically for user leds.

So there are some special pad configs to make the leds work which are not
only muxing and direction setting? Because I expect these to be managed
privately between gpiolib and pinctrl but now I'm not sure any more,
I'll look the code.

 So, leds-gpio driver should have this call, because these gpio
 pins are used by leds-gpio driver.
 
 +   am33xx_pinmux: pinmux@44e10800 {
 +   userled_pins: pinmux_userled_pins {
 +   pinctrl-single,pins = 
 +   0x54 0x7   
 +   0x58 0x17   
 +   0x5c 0x7
 +   0x60 0x17   
 +   ;
 +   };
 +   };
 +
 
 [...]
 
 +   leds {
 +   compatible = gpio-leds;
 +   pinctrl-names = default;
 +   pinctrl-0 = userled_pins;
   

I'm surprised to not see any gpio controller (ala irq) involved.

 Lets take gpio-keypad driver, in that case we have to configure
 pins as INPUT mode (generic gpio driver might not know what
 the end usecase is) and this leds case we configure as OUTPUT
 mode.

gpio direction is modeled by gpiolib so, if no other out-of-gpiolib
capabilities are required for that led gpio, there is no need to directly
use pinctrl.

maybe I've just got it wrong. sorry.

thanks,
Domenico
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread AnilKumar, Chimata 
On Fri, Sep 07, 2012 at 16:32:51, Domenico Andreoli wrote:
 On Fri, Sep 07, 2012 at 09:10:50AM +, AnilKumar, Chimata wrote:
  Hi Domenico,
 
 Hi,
 
  On Fri, Sep 07, 2012 at 14:18:39, Domenico Andreoli wrote:
   On Sat, Sep 1, 2012 at 10:16 AM, AnilKumar Ch anilku...@ti.com wrote:
Adopt pinctrl support to leds-gpio driver based on leds-gpio
device pointer, pinctrl driver configure SoC pins to GPIO
mode according to definitions provided in .dts file.
   
   Shouldn't be the interaction with the pinctrl layer left to gpiolib?
   
  
  No, these gpio's are configured specifically for user leds.
 
 So there are some special pad configs to make the leds work which are not
 only muxing and direction setting? Because I expect these to be managed
 privately between gpiolib and pinctrl but now I'm not sure any more,
 I'll look the code.

How can gpio driver knows that leds-gpio driver require
these 4 pins?

 
  So, leds-gpio driver should have this call, because these gpio
  pins are used by leds-gpio driver.
  
  +   am33xx_pinmux: pinmux@44e10800 {
  +   userled_pins: pinmux_userled_pins {
  +   pinctrl-single,pins = 
  +   0x54 0x7   
  +   0x58 0x17   
  +   0x5c 0x7
  +   0x60 0x17   
  +   ;
  +   };
  +   };
  +
  
  [...]
  
  +   leds {
  +   compatible = gpio-leds;
  +   pinctrl-names = default;
  +   pinctrl-0 = userled_pins;

 
 I'm surprised to not see any gpio controller (ala irq) involved.

GPIO controller data will be in GPIO node, should not be here.

 
  Lets take gpio-keypad driver, in that case we have to configure
  pins as INPUT mode (generic gpio driver might not know what
  the end usecase is) and this leds case we configure as OUTPUT
  mode.
 
 gpio direction is modeled by gpiolib so, if no other out-of-gpiolib
 capabilities are required for that led gpio, there is no need to directly
 use pinctrl.
 

Here leds-gpio driver requirement is to set mux configuration of
those 4 pins to GPIO mode (mode 7) as well direction OUTPUT/INPUT.

Set mux mode to 7 (GPIO usage) should be from led driver, because
this driver have the requirement.

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread Domenico Andreoli 
On Fri, Sep 07, 2012 at 02:30:59PM +, AnilKumar, Chimata wrote:
 On Fri, Sep 07, 2012 at 16:32:51, Domenico Andreoli wrote:
  On Fri, Sep 07, 2012 at 09:10:50AM +, AnilKumar, Chimata wrote:
   On Fri, Sep 07, 2012 at 14:18:39, Domenico Andreoli wrote:
On Sat, Sep 1, 2012 at 10:16 AM, AnilKumar Ch anilku...@ti.com wrote:
 Adopt pinctrl support to leds-gpio driver based on leds-gpio
 device pointer, pinctrl driver configure SoC pins to GPIO
 mode according to definitions provided in .dts file.

Shouldn't be the interaction with the pinctrl layer left to gpiolib?

   
   No, these gpio's are configured specifically for user leds.
  
  So there are some special pad configs to make the leds work which are not
  only muxing and direction setting? Because I expect these to be managed
  privately between gpiolib and pinctrl but now I'm not sure any more,
  I'll look the code.
 
 How can gpio driver knows that leds-gpio driver require
 these 4 pins?

because leds-gpio requests each gpio (specified in the DT against a specific
gpio controller) before assuming it is available?  gpiolib then asks to
pinctrl if those pins are available for gpio and possibly reserve them
(configuring the mux, if necessary) for the device.

But this idea does not correspond to the code, so I must have filled in
the blanks of something I didn't fully understand.

   So, leds-gpio driver should have this call, because these gpio
   pins are used by leds-gpio driver.
   
   +   am33xx_pinmux: pinmux@44e10800 {
   +   userled_pins: pinmux_userled_pins {
   +   pinctrl-single,pins = 
   +   0x54 0x7   
   +   0x58 0x17   
   +   0x5c 0x7
   +   0x60 0x17   
   +   ;
   +   };
   +   };
   +
   
   [...]
   
   +   leds {
   +   compatible = gpio-leds;
   +   pinctrl-names = default;
   +   pinctrl-0 = userled_pins;
 
  
  I'm surprised to not see any gpio controller (ala irq) involved.
 
 GPIO controller data will be in GPIO node, should not be here.

So is this the preferred way to attach gpio users to gpio provides in DT
whenever gpios are muxed?

I would well see these info hidden in the gpio controller so, at least
for gpios, no magic numbers would be required in the DT (except the gpio
number relative to the owning controller).

   Lets take gpio-keypad driver, in that case we have to configure
   pins as INPUT mode (generic gpio driver might not know what
   the end usecase is) and this leds case we configure as OUTPUT
   mode.
  
  gpio direction is modeled by gpiolib so, if no other out-of-gpiolib
  capabilities are required for that led gpio, there is no need to directly
  use pinctrl.
  
 
 Here leds-gpio driver requirement is to set mux configuration of
 those 4 pins to GPIO mode (mode 7) as well direction OUTPUT/INPUT.

Direction info is passed to gpiolib in the exact instant
gpio_set_direction_*() is invoked.

 Set mux mode to 7 (GPIO usage) should be from led driver, because
 this driver have the requirement.

This GPIO mode value is known to the pinctrl driver, actually it's _specific_
for that driver. So the only info pinctrl would really need to know is which
pin is requested to be used as GPIO, something that gpiolib already manages.

So I really don't see why the gpiolib could not be (I've understood it isn't)
the one-stop for GPIO users. Of course direct pinctrl configuration would be
required for all those specific gpio parameters not modeled by the gpiolib
(like debounce times, etc).

cheers,
Domenico
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread Bryan Wu 
On Fri, Sep 7, 2012 at 3:59 PM, AnilKumar, Chimata anilku...@ti.com wrote:
 On Fri, Sep 07, 2012 at 05:39:35, Marek Vasut wrote:
 Dear Tony Lindgren,

  * Marek Vasut ma...@denx.de [120905 19:05]:
   Hi Tony,
  
* Marek Vasut ma...@denx.de [120904 20:13]:
 Dear Bryan Wu,

  On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch anilku...@ti.com 
  wrote:
   Adopt pinctrl support to leds-gpio driver based on leds-gpio
   device pointer, pinctrl driver configure SoC pins to GPIO
   mode according to definitions provided in .dts file.
 
  Thanks for this, actually Marek Vasut submitted a similar patch
  before. I'm pretty fine with this patch.

 Thanks for submitting this actually ... I didn't have time to
 properly investigate this.

  But without proper DT setting, it will also give us warning I
  think. or we can provide some dummy functions as a temp solution
  as Shawn pointed out before.

 But this driver is also used on hardware that's not yet coverted to
 DT, so I'd say dev_warn() if CONFIG_OF is enabled and otherwise
 simply go on ? Actually, can we not skip whole this pinctrl thing if
 CONFIG_OF is disabled? Actually (2), what's the relationship between
 OF and pinctrl?
   
The warning should be pinctrl related as the pinctrl drivers may not be
device tree based drivers.
  
   Exactly my concern. Also the warning shouldnt be present on systems where
   pinctrl is disabled.
 
  But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if
  CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)

 Oh all right then.


 Bryan,

 If this patch looks fine, can you queue this for 3.7?


I've applied this to my for-next branch.

Thanks,
-Bryan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread Tony Lindgren 
* Domenico Andreoli cav...@gmail.com [120907 09:01]:
 
 So is this the preferred way to attach gpio users to gpio provides in DT
 whenever gpios are muxed?
 
 I would well see these info hidden in the gpio controller so, at least
 for gpios, no magic numbers would be required in the DT (except the gpio
 number relative to the owning controller).

In the pure GPIO pins only case it could be all done in the GPIO controller,
but it's probably best to have the pins muxed by the drivers using them.

Some drivers doing runtime PM may need to dynamically toggle the pins
for idle mode to stop leakage, enable wakeup events for rx pins etc.
Probably no need for that in the gpio leds case, but it would be confusing
to have some pins claimed by the GPIO driver and some by the drivers
using the pins.

Regards,

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread Linus Walleij 
On Sat, Sep 1, 2012 at 10:16 AM, AnilKumar Ch anilku...@ti.com wrote:

 Adopt pinctrl support to leds-gpio driver based on leds-gpio
 device pointer, pinctrl driver configure SoC pins to GPIO
 mode according to definitions provided in .dts file.

 Signed-off-by: AnilKumar Ch anilku...@ti.com

Looks good from a pinctrl point of view!
Reviewed-by: Linus Walleij linus.wall...@linaro.org

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread Linus Walleij 
On Thu, Sep 6, 2012 at 7:45 PM, Tony Lindgren t...@atomide.com wrote:

  The warning should be pinctrl related as the pinctrl drivers may not be
  device tree based drivers.

 Exactly my concern. Also the warning shouldnt be present on systems where
 pinctrl is disabled.

 But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if
 CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)

This is correct, nothing to worry about.

The one troublesome case is if a pinctrl driver is present but not
being used, then you might need to call pinctrl_provide_dummies().

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread Tony Lindgren 
* Linus Walleij linus.wall...@linaro.org [120907 14:40]:
 On Thu, Sep 6, 2012 at 7:45 PM, Tony Lindgren t...@atomide.com wrote:
 
   The warning should be pinctrl related as the pinctrl drivers may not be
   device tree based drivers.
 
  Exactly my concern. Also the warning shouldnt be present on systems where
  pinctrl is disabled.
 
  But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if
  CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)
 
 This is correct, nothing to worry about.
 
 The one troublesome case is if a pinctrl driver is present but not
 being used, then you might need to call pinctrl_provide_dummies().

Thanks that's good to know.

Regards,

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread Linus Walleij 
On Fri, Sep 7, 2012 at 6:00 PM, Domenico Andreoli cav...@gmail.com wrote:
 On Fri, Sep 07, 2012 at 02:30:59PM +, AnilKumar, Chimata wrote:

 How can gpio driver knows that leds-gpio driver require
 these 4 pins?

 because leds-gpio requests each gpio (specified in the DT against a specific
 gpio controller) before assuming it is available?  gpiolib then asks to
 pinctrl if those pins are available for gpio and possibly reserve them
 (configuring the mux, if necessary) for the device.

So this is not an either-or situation but a both-and case.

So all muxing and configuration of pins can be handled by
the pinctrl handle, and that may require setting up a single
pinctrl function for every single pin, and that list can get long.
But it works fine.

In that case you don't write your pinctrl driver to do anything
special with the GPIO callbacks, leave  the
.gpio_request_enable(), .gpio_disable_free() and
.gpio_set_direction() callbacks in the vtable undefined.

If all you need to to is to multiplex the pins into GPIO mode,
then the gpio_get() call on this driver *can* call through to
pinctrl_request_gpio() which will in turn fall through to the
above pinmux driver calls (.gpio_request_enable, etc).

Likewise for pinctrl_free_gpio(), pinctrl_gpio_direction_input()
and pinctrl_gpio_direction_output().

But that's as far as it goes! The GPIO abstraction cannot
call through to e.g. set some specific biasing on the pins
(pull up etc). Doing that would require us to reimplement
every interface that pinctrl already has again in the
GPIO layer, which is not a good idea.

So the pinctrl handle can be used for such config, and it
can also be used for multiplexing if that is desired - if not
done by the fall through functions pinctrl_gpio_*().

You can use a combination of both too, Stephen patched
pinctrl some time back so that a pin can be muxed for a
certain function and used as GPIO at the same time, so
these two are now orthogonal, it's a bit relaxed and gives some
feeling of loss of control but was necessary for certain
usecases. (For example we can snoop on a I2C pin using
its corresponding GPIO registers in the U300...)

There is some flexibility here, I hope it's not too confusing :-/

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-07 Thread Linus Walleij 
On Fri, Sep 7, 2012 at 6:35 PM, Tony Lindgren t...@atomide.com wrote:

 In the pure GPIO pins only case it could be all done in the GPIO controller,
 but it's probably best to have the pins muxed by the drivers using them.

Yes, that's an easier way to say the long unreadable thing I just
posted ...

 Some drivers doing runtime PM may need to dynamically toggle the pins
 for idle mode to stop leakage, enable wakeup events for rx pins etc.
 Probably no need for that in the gpio leds case, but it would be confusing
 to have some pins claimed by the GPIO driver and some by the drivers
 using the pins.

True. drivers/tty/serial/amba-pl011.c provides a simple example.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-06 Thread Tony Lindgren 
* Marek Vasut ma...@denx.de [120905 19:05]:
 Hi Tony,
 
  * Marek Vasut ma...@denx.de [120904 20:13]:
   Dear Bryan Wu,
   
On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch anilku...@ti.com wrote:
 Adopt pinctrl support to leds-gpio driver based on leds-gpio
 device pointer, pinctrl driver configure SoC pins to GPIO
 mode according to definitions provided in .dts file.

Thanks for this, actually Marek Vasut submitted a similar patch
before. I'm pretty fine with this patch.
   
   Thanks for submitting this actually ... I didn't have time to properly
   investigate this.
   
But without proper DT setting, it will also give us warning I think.
or we can provide some dummy functions as a temp solution as Shawn
pointed out before.
   
   But this driver is also used on hardware that's not yet coverted to DT,
   so I'd say dev_warn() if CONFIG_OF is enabled and otherwise simply go on
   ? Actually, can we not skip whole this pinctrl thing if CONFIG_OF is
   disabled? Actually (2), what's the relationship between OF and pinctrl?
  
  The warning should be pinctrl related as the pinctrl drivers may not be
  device tree based drivers.
 
 Exactly my concern. Also the warning shouldnt be present on systems where 
 pinctrl is disabled.

But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if
CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)

Or do you get some warning if CONFIG_PINCTRL is not selected for your
hardware?

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-06 Thread Marek Vasut 
Dear Tony Lindgren,

 * Marek Vasut ma...@denx.de [120905 19:05]:
  Hi Tony,
  
   * Marek Vasut ma...@denx.de [120904 20:13]:
Dear Bryan Wu,

 On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch anilku...@ti.com wrote:
  Adopt pinctrl support to leds-gpio driver based on leds-gpio
  device pointer, pinctrl driver configure SoC pins to GPIO
  mode according to definitions provided in .dts file.
 
 Thanks for this, actually Marek Vasut submitted a similar patch
 before. I'm pretty fine with this patch.

Thanks for submitting this actually ... I didn't have time to
properly investigate this.

 But without proper DT setting, it will also give us warning I
 think. or we can provide some dummy functions as a temp solution
 as Shawn pointed out before.

But this driver is also used on hardware that's not yet coverted to
DT, so I'd say dev_warn() if CONFIG_OF is enabled and otherwise
simply go on ? Actually, can we not skip whole this pinctrl thing if
CONFIG_OF is disabled? Actually (2), what's the relationship between
OF and pinctrl?
   
   The warning should be pinctrl related as the pinctrl drivers may not be
   device tree based drivers.
  
  Exactly my concern. Also the warning shouldnt be present on systems where
  pinctrl is disabled.
 
 But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if
 CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)

Oh all right then.

 Or do you get some warning if CONFIG_PINCTRL is not selected for your
 hardware?

No, I don't have much hardware like such anymore :-(

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-05 Thread Tony Lindgren 
* Marek Vasut ma...@denx.de [120904 20:13]:
 Dear Bryan Wu,
 
  On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch anilku...@ti.com wrote:
   Adopt pinctrl support to leds-gpio driver based on leds-gpio
   device pointer, pinctrl driver configure SoC pins to GPIO
   mode according to definitions provided in .dts file.
  
  Thanks for this, actually Marek Vasut submitted a similar patch
  before. I'm pretty fine with this patch.
 
 Thanks for submitting this actually ... I didn't have time to properly 
 investigate this.
 
  But without proper DT setting, it will also give us warning I think.
  or we can provide some dummy functions as a temp solution as Shawn
  pointed out before.
 
 But this driver is also used on hardware that's not yet coverted to DT, so 
 I'd 
 say dev_warn() if CONFIG_OF is enabled and otherwise simply go on ? Actually, 
 can we not skip whole this pinctrl thing if CONFIG_OF is disabled? Actually 
 (2), 
 what's the relationship between OF and pinctrl?

The warning should be pinctrl related as the pinctrl drivers may not be
device tree based drivers.

Regards,

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

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-04 Thread Bryan Wu 
On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch anilku...@ti.com wrote:
 Adopt pinctrl support to leds-gpio driver based on leds-gpio
 device pointer, pinctrl driver configure SoC pins to GPIO
 mode according to definitions provided in .dts file.


Thanks for this, actually Marek Vasut submitted a similar patch
before. I'm pretty fine with this patch.
But without proper DT setting, it will also give us warning I think.
or we can provide some dummy functions as a temp solution as Shawn
pointed out before.

-Bryan

 Signed-off-by: AnilKumar Ch anilku...@ti.com
 ---
 Changes from v1:
 - Seperated from Add DT for AM33XX devices patch series
 - Incorporated Tony's comments on v1
   * Changed to warning message instead od error return

  drivers/leds/leds-gpio.c |7 +++
  1 file changed, 7 insertions(+)

 diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
 index c032b21..ad577f4 100644
 --- a/drivers/leds/leds-gpio.c
 +++ b/drivers/leds/leds-gpio.c
 @@ -20,6 +20,7 @@
  #include linux/slab.h
  #include linux/workqueue.h
  #include linux/module.h
 +#include linux/pinctrl/consumer.h

  struct gpio_led_data {
 struct led_classdev cdev;
 @@ -236,8 +237,14 @@ static int __devinit gpio_led_probe(struct 
 platform_device *pdev)
  {
 struct gpio_led_platform_data *pdata = pdev-dev.platform_data;
 struct gpio_leds_priv *priv;
 +   struct pinctrl *pinctrl;
 int i, ret = 0;

 +   pinctrl = devm_pinctrl_get_select_default(pdev-dev);
 +   if (IS_ERR(pinctrl))
 +   dev_warn(pdev-dev,
 +   pins are not configured from the driver\n);
 +
 if (pdata  pdata-num_leds) {
 priv = devm_kzalloc(pdev-dev,
 sizeof_gpio_leds_priv(pdata-num_leds),
 --
 1.7.9.5

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



-- 
Bryan Wu bryan...@canonical.com
Kernel Developer+86.186-168-78255 Mobile
Canonical Ltd.  www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

2012-09-04 Thread Marek Vasut 
Dear Bryan Wu,

 On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch anilku...@ti.com wrote:
  Adopt pinctrl support to leds-gpio driver based on leds-gpio
  device pointer, pinctrl driver configure SoC pins to GPIO
  mode according to definitions provided in .dts file.
 
 Thanks for this, actually Marek Vasut submitted a similar patch
 before. I'm pretty fine with this patch.

Thanks for submitting this actually ... I didn't have time to properly 
investigate this.

 But without proper DT setting, it will also give us warning I think.
 or we can provide some dummy functions as a temp solution as Shawn
 pointed out before.

But this driver is also used on hardware that's not yet coverted to DT, so I'd 
say dev_warn() if CONFIG_OF is enabled and otherwise simply go on ? Actually, 
can we not skip whole this pinctrl thing if CONFIG_OF is disabled? Actually 
(2), 
what's the relationship between OF and pinctrl?

 -Bryan
 
  Signed-off-by: AnilKumar Ch anilku...@ti.com
  ---
  
  Changes from v1:
  - Seperated from Add DT for AM33XX devices patch series
  - Incorporated Tony's comments on v1
  
* Changed to warning message instead od error return
   
   drivers/leds/leds-gpio.c |7 +++
   1 file changed, 7 insertions(+)
  
  diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
  index c032b21..ad577f4 100644
  --- a/drivers/leds/leds-gpio.c
  +++ b/drivers/leds/leds-gpio.c
  @@ -20,6 +20,7 @@
  
   #include linux/slab.h
   #include linux/workqueue.h
   #include linux/module.h
  
  +#include linux/pinctrl/consumer.h
  
   struct gpio_led_data {
   
  struct led_classdev cdev;
  
  @@ -236,8 +237,14 @@ static int __devinit gpio_led_probe(struct
  platform_device *pdev)
  
   {
   
  struct gpio_led_platform_data *pdata = pdev-dev.platform_data;
  struct gpio_leds_priv *priv;
  
  +   struct pinctrl *pinctrl;
  
  int i, ret = 0;
  
  +   pinctrl = devm_pinctrl_get_select_default(pdev-dev);
  +   if (IS_ERR(pinctrl))
  +   dev_warn(pdev-dev,
  +   pins are not configured from the driver\n);
  +
  
  if (pdata  pdata-num_leds) {
  
  priv = devm_kzalloc(pdev-dev,
  
  sizeof_gpio_leds_priv(pdata-num_leds),
  
  --
  1.7.9.5
  
  --
  To unsubscribe from this list: send the line unsubscribe linux-leds in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html