Re: [PATCH] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On Mon, May 14, 2012 at 8:38 PM, Stephen Warren swar...@wwwdotorg.org wrote: On 05/12/2012 05:49 PM, Linus Walleij wrote: We have some pinctrl drivers implementing gpiolib too already, and it's unavoidable I think, as some recent discussion about matcing struct gpio_chip and pinctrl GPIO ranges shows. I strongly believe we should only do this when the exact same HW module is both pinctrl and GPIO. Yep so this is what I have done for pinctrl-nomadik.c When there are separate HW modules, we should have separate drivers. The fact that the two drivers need to co-ordinate with each-other isn't a good argument to make them one driver. So this matches the pinctrl-u300.c/pinctrl-coh901.c design pattern. Maybe -simple isn't such a good name for this thing. Noone thinks any kind of pin control is simple in any sense of the word anyway :-D Tony, would pinctrl-dt-only.c be a better name perhaps? That might be OK for the filename, but it doesn't seem like a useful change for the DT compatible value. Oh I didn't think of these. Hm from a generic world running Windows Mobile etc I take it that this is seen as simple mappings then? 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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 05/16/2012 01:14 AM, Linus Walleij wrote: On Mon, May 14, 2012 at 8:38 PM, Stephen Warren swar...@wwwdotorg.org wrote: On 05/12/2012 05:49 PM, Linus Walleij wrote: ... Maybe -simple isn't such a good name for this thing. Noone thinks any kind of pin control is simple in any sense of the word anyway :-D Tony, would pinctrl-dt-only.c be a better name perhaps? That might be OK for the filename, but it doesn't seem like a useful change for the DT compatible value. Oh I didn't think of these. Hm from a generic world running Windows Mobile etc I take it that this is seen as simple mappings then? I can't think of anything that's much better than pinctrl-simple. Perhaps pinctrl-generic; at least that doesn't imply it's simple if we keep adding features:-) -- 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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
* Stephen Warren swar...@wwwdotorg.org [120514 11:42]: On 05/12/2012 05:49 PM, Linus Walleij wrote: On Thu, May 10, 2012 at 7:05 PM, Stephen Warren swar...@wwwdotorg.org wrote: Also, were you intending pinctrl-simple to actually be the GPIO controller itself? That'd be another case that one might consider fairly simple, but then extends to being gpio-simple as well as pinctrl-simple... We have some pinctrl drivers implementing gpiolib too already, and it's unavoidable I think, as some recent discussion about matcing struct gpio_chip and pinctrl GPIO ranges shows. I strongly believe we should only do this when the exact same HW module is both pinctrl and GPIO. When there are separate HW modules, we should have separate drivers. The fact that the two drivers need to co-ordinate with each-other isn't a good argument to make them one driver. And irrespective of how the drivers are structured, if there are two HW modules, we really need two separate nodes in DT to describe them, since the SW architecture (1 vs. 2 drivers) shouldn't influence the DT representation unduly. Yes. Maybe -simple isn't such a good name for this thing. Noone thinks any kind of pin control is simple in any sense of the word anyway :-D Tony, would pinctrl-dt-only.c be a better name perhaps? That might be OK for the filename, but it doesn't seem like a useful change for the DT compatible value. Yeah let's see if we can come up with some better name. 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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 05/12/2012 05:49 PM, Linus Walleij wrote: On Thu, May 10, 2012 at 7:05 PM, Stephen Warren swar...@wwwdotorg.org wrote: Also, were you intending pinctrl-simple to actually be the GPIO controller itself? That'd be another case that one might consider fairly simple, but then extends to being gpio-simple as well as pinctrl-simple... We have some pinctrl drivers implementing gpiolib too already, and it's unavoidable I think, as some recent discussion about matcing struct gpio_chip and pinctrl GPIO ranges shows. I strongly believe we should only do this when the exact same HW module is both pinctrl and GPIO. When there are separate HW modules, we should have separate drivers. The fact that the two drivers need to co-ordinate with each-other isn't a good argument to make them one driver. And irrespective of how the drivers are structured, if there are two HW modules, we really need two separate nodes in DT to describe them, since the SW architecture (1 vs. 2 drivers) shouldn't influence the DT representation unduly. Maybe -simple isn't such a good name for this thing. Noone thinks any kind of pin control is simple in any sense of the word anyway :-D Tony, would pinctrl-dt-only.c be a better name perhaps? That might be OK for the filename, but it doesn't seem like a useful change for the DT compatible value. -- 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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On Thu, May 10, 2012 at 7:05 PM, Stephen Warren swar...@wwwdotorg.org wrote: Also, were you intending pinctrl-simple to actually be the GPIO controller itself? That'd be another case that one might consider fairly simple, but then extends to being gpio-simple as well as pinctrl-simple... We have some pinctrl drivers implementing gpiolib too already, and it's unavoidable I think, as some recent discussion about matcing struct gpio_chip and pinctrl GPIO ranges shows. Maybe -simple isn't such a good name for this thing. Noone thinks any kind of pin control is simple in any sense of the word anyway :-D Tony, would pinctrl-dt-only.c be a better name perhaps? 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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 05/10/2012 11:27 AM, Tony Lindgren wrote:
* Stephen Warren swar...@wwwdotorg.org [120510 10:09]:
On 05/09/2012 02:49 PM, Tony Lindgren wrote:
* Stephen Warren swar...@wwwdotorg.org [120509 13:22]:
On 05/04/2012 04:08 PM, Tony Lindgren wrote:
* Stephen Warren swar...@wwwdotorg.org [120504 11:59]:
On 05/04/2012 10:34 AM, Tony Lindgren wrote:
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120504
08:58]:
On 08:03 Fri 04 May , Tony Lindgren wrote:
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
no pin number in the bank because it could not be gpio
Yes OK, but pin number 12 in the gpio bank, not in the mux register.
Got it.
I'd prefer to avoid any references to GPIOs here; not all muxable pins
are GPIOs and not all GPIOs are muxable pins. Lets keep the two concepts
independent.
And it seems that pioA 12 is not always enough information for the
pinctrl
driver to request a GPIO. So it's best to specify it separately.
Why would a pinctrl driver request a GPIO?
Hmm what would pinctrl_request_gpio do if the GPIO driver is separate
driver?
Well, that's a GPIO driver requesting a GPIO from the pinctrl system,
rather than the pinctrl driver requesting a GPIO (sorry to be picky).
Oh sorry maybe I misunderstood what pinctrl_request_gpio is doing.
Seems like that should work the same way from binding point of view.
It wasn't at all obvious to me from your binding proposal that you
intended the pinctrl-simple driver to support the GPIO operations at
all. If you do want this, I think you'd need some properties (perhaps
some kind of explicit table) in order to set up the GPIO ID - pinctrl
pin ID mapping. I don't recall seeing those; did I just miss them? I
think we'd want this to be explicit because:
a) It may well be the case that not all users of pinctrl-simple actually
mux/control GPIOs at all. It's certainly possible to only mux special
functions, and have dedicated pins for a GPIO controller.
b) Even when GPIOs do come into the picture, it may be that only some of
the pins are available as GPIOs.
Right, we need some pinctrl to gpio mapping eventually. That comes
automatically from DT for the pins and gpios we care about.
And that's why the binding needs to be flexible enough so we can add
optional pin specific options as needed in addition to parsing a larger
set of pins with no options.
The mapping of GPIO to pinctrl pins would presumably be driven solely by
the HW design of the pin controller and GPIO, and not by the mux
selection in the pin controller (otherwise, I'd argue this isn't a
simple case that should be handled by pinctrl-simple).
As such, I'd expect some properties/table at the top-level of the pin
controller object to describe the GPIO mapping. In turn, that implies
that the individual per-pin mux-selection/configuration nodes don't need
to describe any GPIO-related information.
--
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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
* Stephen Warren swar...@wwwdotorg.org [120511 12:21]: The mapping of GPIO to pinctrl pins would presumably be driven solely by the HW design of the pin controller and GPIO, and not by the mux selection in the pin controller (otherwise, I'd argue this isn't a simple case that should be handled by pinctrl-simple). As such, I'd expect some properties/table at the top-level of the pin controller object to describe the GPIO mapping. In turn, that implies that the individual per-pin mux-selection/configuration nodes don't need to describe any GPIO-related information. Yes good point. I agree it's a HW design issue, and could be in the properties for the pin controller object. Just to summarize, the things to consider with the GPIO to mux mapping are: 1. Having this table as static data in the driver is is not a nice solution as it seems that we'd currently need six mapping tables for omap2+ alone. 2. This table is not needed for most of the (hundreds of) pins, it's only needed for a few selected pins, let's say ten or so on an average device. So there's no need to stuff the kernel with information about the unused GPIO pins. It seems that the conclusion here is that we don't need to worry about GPIOs in the pinctrl-simple binding for now, and it can be added later without having to change the basic binding. 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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 05/11/2012 01:51 PM, Tony Lindgren wrote: * Stephen Warren swar...@wwwdotorg.org [120511 12:21]: The mapping of GPIO to pinctrl pins would presumably be driven solely by the HW design of the pin controller and GPIO, and not by the mux selection in the pin controller (otherwise, I'd argue this isn't a simple case that should be handled by pinctrl-simple). As such, I'd expect some properties/table at the top-level of the pin controller object to describe the GPIO mapping. In turn, that implies that the individual per-pin mux-selection/configuration nodes don't need to describe any GPIO-related information. Yes good point. I agree it's a HW design issue, and could be in the properties for the pin controller object. Just to summarize, the things to consider with the GPIO to mux mapping are: 1. Having this table as static data in the driver is is not a nice solution as it seems that we'd currently need six mapping tables for omap2+ alone. 2. This table is not needed for most of the (hundreds of) pins, it's only needed for a few selected pins, let's say ten or so on an average device. So there's no need to stuff the kernel with information about the unused GPIO pins. It seems that the conclusion here is that we don't need to worry about GPIOs in the pinctrl-simple binding for now, and it can be added later without having to change the basic binding. The one thing I wanted to resolve here wasn't so much the binding for GPIO interaction here, but the following comment: You wrote: I wrote: From a binding perspective, I don't see why you'd want to allow two cases: 1) One node with multiple entries in pinctrl-simple,cells 2) Multiple nodes each with a single entry in pinctrl-simple,cells Why not only allow (1)? Because we need to specify GPIO for some pins. There may be additional flags too, we do have external DMA request lines for few pins available.. I'm not saying pinctrl fwk should know about that, but it's a similar mapping of pins to GPIO lines. I'm asserting that since any GPIO mapping information would be at the top-level of the pinctrl-simple binding, we can in fact only allow option (1) above for the individual pin configuration nodes. -- 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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 05/09/2012 02:49 PM, Tony Lindgren wrote:
* Stephen Warren swar...@wwwdotorg.org [120509 13:22]:
On 05/04/2012 04:08 PM, Tony Lindgren wrote:
* Stephen Warren swar...@wwwdotorg.org [120504 11:59]:
On 05/04/2012 10:34 AM, Tony Lindgren wrote:
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120504 08:58]:
On 08:03 Fri 04 May , Tony Lindgren wrote:
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
no pin number in the bank because it could not be gpio
Yes OK, but pin number 12 in the gpio bank, not in the mux register.
Got it.
I'd prefer to avoid any references to GPIOs here; not all muxable pins
are GPIOs and not all GPIOs are muxable pins. Lets keep the two concepts
independent.
And it seems that pioA 12 is not always enough information for the pinctrl
driver to request a GPIO. So it's best to specify it separately.
Why would a pinctrl driver request a GPIO?
Hmm what would pinctrl_request_gpio do if the GPIO driver is separate driver?
Well, that's a GPIO driver requesting a GPIO from the pinctrl system,
rather than the pinctrl driver requesting a GPIO (sorry to be picky).
It wasn't at all obvious to me from your binding proposal that you
intended the pinctrl-simple driver to support the GPIO operations at
all. If you do want this, I think you'd need some properties (perhaps
some kind of explicit table) in order to set up the GPIO ID - pinctrl
pin ID mapping. I don't recall seeing those; did I just miss them? I
think we'd want this to be explicit because:
a) It may well be the case that not all users of pinctrl-simple actually
mux/control GPIOs at all. It's certainly possible to only mux special
functions, and have dedicated pins for a GPIO controller.
b) Even when GPIOs do come into the picture, it may be that only some of
the pins are available as GPIOs.
Also, were you intending pinctrl-simple to actually be the GPIO
controller itself? That'd be another case that one might consider fairly
simple, but then extends to being gpio-simple as well as pinctrl-simple...
--
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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 05/04/2012 03:57 PM, Tony Lindgren wrote:
* Stephen Warren swar...@wwwdotorg.org [120504 12:27]:
On 05/02/2012 11:24 AM, Tony Lindgren wrote:
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt
b/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt
...
On the other hand, I worry about whether using pinctrl-simple here as
the compatible value is going to cause issues:
Certainly, this is a pretty simple driver, and most likely reasonably
generic an applicable to many SoCs. However, it doesn't cover a bunch of
cases that I'd still consider simple. For example, what if each pin
has a separate mux and pinconf register? There are probably many such
small cases that would add up to something more complex. to cover those
cases, will we be able to extend pinctrl-simple to cover them, and
continue to be backwards compatible, or will we need to create a
binding/driver for pinctrl-simple-1, pinctrl-simple-2, pinctrl-simple-3
each of which covers some different, yet still simple, configuration?
Yes getting the binding right is the critical part here, everything else
can be added as needed. I was thinking about using separate properties
for auxilary registers, but now thinking about it a bit more, it may not
be sufficient.
How about we make some of these properties into arrays? For example:
#pinctrl-cells = 6;
pinctrl-simple,function-mask = 0x 0x 0x;
pinctrl-simple,function-off = 0x7 0x7 0x7;
pinctrl-simple,pinconf-mask = 0x 0x 0x;
I'm not sure what the 3 entries in the array are meant to describe?
Then for handling the set/clear bits case of registers, we could do that
automatically if both masks are 0 for some registers. We actually have
that for some omap1 where there are three registers per mux with bits
to set or clear. And these bits are not the same across all registers..
+ 0x6c 0xf/* CSI21_DX3 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ 0x6e 0xf/* CSI21_DY3 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ 0x70 0xf/* CSI21_DX4 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ 0x72 0xf/* CSI21_DY4 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ ;
+ };
+ };
+
+
+ /* map all uart2 pins as a single function */
+ uart2_pins: pinmux_uart2_pins {
+ uart2_pins {
+ pinctrl-simple,cells =
+ 0xd8 0x118 /* UART2_CTS
OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0 */
+ 0xda 0 /* UART2_RTS OMAP_PIN_OUTPUT |
OMAP_MUX_MODE0 */
+ 0xdc 0x118 /* UART2_RX
OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0 */
+ 0xde 0 /* UART2_TX OMAP_PIN_OUTPUT |
OMAP_MUX_MODE0 */
+ ;
+ };
+ };
+
+ /* map all uart3 pins as separate functions */
+ uart3_pins: pinmux_uart3_pins {
From a binding perspective, I don't see why you'd want to allow two cases:
1) One node with multiple entries in pinctrl-simple,cells
2) Multiple nodes each with a single entry in pinctrl-simple,cells
Why not only allow (1)?
Because we need to specify GPIO for some pins. There may be additional flags
What do you mean by specify GPIO?
Nothing in this pinctrl-simple binding seems to imply that it's also a
GPIO controller.
If GPIO is one of the functions that can be mux'd onto a pin, then I'd
expect that to be represented in exactly the same way as any other
function that could be mux'd onto the pin.
So, I'm not sure what GPIO-related information you want to represent.
too, we do have external DMA request lines for few pins available.. I'm not
saying pinctrl fwk should know about that, but it's a similar mapping of pins
to GPIO lines.
Aren't DMA request lines also just another function that can be mux'd
onto a pin?
--
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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 05/04/2012 04:08 PM, Tony Lindgren wrote:
* Stephen Warren swar...@wwwdotorg.org [120504 11:59]:
On 05/04/2012 10:34 AM, Tony Lindgren wrote:
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120504 08:58]:
On 08:03 Fri 04 May , Tony Lindgren wrote:
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
no pin number in the bank because it could not be gpio
Yes OK, but pin number 12 in the gpio bank, not in the mux register.
Got it.
I'd prefer to avoid any references to GPIOs here; not all muxable pins
are GPIOs and not all GPIOs are muxable pins. Lets keep the two concepts
independent.
And it seems that pioA 12 is not always enough information for the pinctrl
driver to request a GPIO. So it's best to specify it separately.
Why would a pinctrl driver request a GPIO?
--
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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 05/04/2012 08:04 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 12:55 Fri 04 May , Stephen Warren wrote:
On 05/04/2012 10:34 AM, Tony Lindgren wrote:
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120504 08:58]:
On 08:03 Fri 04 May , Tony Lindgren wrote:
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
no pin number in the bank because it could not be gpio
Yes OK, but pin number 12 in the gpio bank, not in the mux register.
Got it.
I'd prefer to avoid any references to GPIOs here; not all muxable pins
are GPIOs and not all GPIOs are muxable pins. Lets keep the two concepts
independent.
my idea was to have a phandle pinctrl specific to represent the bank
and use it in the same way as done on gpio
OK, so when you're talking about GPIOs in this thread, you really mean
pins; nothing to do with real GPIOs? You're just using the existing GPIO
binding as an example/base for your pinctrl pin proposal.
...
I don't really see how that DT format represents pins, functions, and
nodes directly, and separately from which of those a board chooses to
use. I think this binding and the one Tony originally proposed are
eseentially semantically identical.
Going back to my idea of separating SoC and board configurations, if we
did that, I'd expect to see something more like:
soc.dtsi or board.dts:
This is the data that the pin controller driver needs to export to
pinctrl core. This can be completely enumerated in the soc.dtsi, or
perhaps for uncommonly used pins/groups/functions, only included in the
board.dts that actually uses it to cut down on soc.dtsi's size:
This data is not needed for SoCs whose pinctrl drivers include it in
their driver file instead of DT.
I agree on at91 I propose exactly this but get the following comment tat we
are going to have too much node.
That's why I put it in the Tegra driver not DT:-)
so the idea I propoose with the pins array is to avoid this issue
OK, to some extent that makes sense, but it doesn't allow you to do
stuff like have the correct names for each pin, since each pin would be
something like Bank 1 Pin 5 or Bank 6 Pin 2; not very descriptive.
my first bindings on at91
...
1) we describe one functin per pin
functions {
rxd_pb12 {
atmel,pin-id = pioB 12;
atmel,mux = 0;
};
txd_pb13 {
atmel,pin-id = piaB 13;
atmel,pull = 2;
atmel,mux = 0;
};
A function is a specific mux value you select on a pin or group. As
such, specifying pull within the definition of a function doesn't
really make sense.
Now, I know that many people want to auto-generate the functions and
groups from device tree, but actually calling them functions in the DT
binding isn't right, because those nodes represent the entire pin
configuration of a pin, not a function, which is some signal you can mux
onto a pin.
--
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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
* Stephen Warren swar...@wwwdotorg.org [120509 13:22]:
On 05/04/2012 04:08 PM, Tony Lindgren wrote:
* Stephen Warren swar...@wwwdotorg.org [120504 11:59]:
On 05/04/2012 10:34 AM, Tony Lindgren wrote:
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120504 08:58]:
On 08:03 Fri 04 May , Tony Lindgren wrote:
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
no pin number in the bank because it could not be gpio
Yes OK, but pin number 12 in the gpio bank, not in the mux register.
Got it.
I'd prefer to avoid any references to GPIOs here; not all muxable pins
are GPIOs and not all GPIOs are muxable pins. Lets keep the two concepts
independent.
And it seems that pioA 12 is not always enough information for the pinctrl
driver to request a GPIO. So it's best to specify it separately.
Why would a pinctrl driver request a GPIO?
Hmm what would pinctrl_request_gpio do if the GPIO driver is separate driver?
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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
* Linus Walleij linus.wall...@linaro.org [120509 02:13]: On Fri, May 4, 2012 at 5:03 PM, Tony Lindgren t...@atomide.com wrote: Also I think we should let the pinctrl core eventually manage the pins more too. Right now the pins are a static array in the driver, which makes things unnecessarily complex for the DT case. It would be nice to also have something like pinctrl_register/unregister_pin instead of requiring them all be registered while registering with the framework initially. Not instead of but in addition to :-) One does not exclude the other. But a good general idea if you need this, just introduce it. Sure agreed it does not exclude the other. Will take a look when I have a chance. 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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
* Stephen Warren swar...@wwwdotorg.org [120509 13:19]: On 05/04/2012 03:57 PM, Tony Lindgren wrote: * Stephen Warren swar...@wwwdotorg.org [120504 12:27]: On 05/02/2012 11:24 AM, Tony Lindgren wrote: diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt ... On the other hand, I worry about whether using pinctrl-simple here as the compatible value is going to cause issues: Certainly, this is a pretty simple driver, and most likely reasonably generic an applicable to many SoCs. However, it doesn't cover a bunch of cases that I'd still consider simple. For example, what if each pin has a separate mux and pinconf register? There are probably many such small cases that would add up to something more complex. to cover those cases, will we be able to extend pinctrl-simple to cover them, and continue to be backwards compatible, or will we need to create a binding/driver for pinctrl-simple-1, pinctrl-simple-2, pinctrl-simple-3 each of which covers some different, yet still simple, configuration? Yes getting the binding right is the critical part here, everything else can be added as needed. I was thinking about using separate properties for auxilary registers, but now thinking about it a bit more, it may not be sufficient. How about we make some of these properties into arrays? For example: #pinctrl-cells = 6; pinctrl-simple,function-mask = 0x 0x 0x; pinctrl-simple,function-off = 0x7 0x7 0x7; pinctrl-simple,pinconf-mask = 0x 0x 0x; I'm not sure what the 3 entries in the array are meant to describe? If you have let's say three registers per pin, those would be the related function and pinconf masks for those registers. Because we need to specify GPIO for some pins. There may be additional flags What do you mean by specify GPIO? Nothing in this pinctrl-simple binding seems to imply that it's also a GPIO controller. It is not a GPIO controller, but eventually needs to deal with existing GPIO controllers. If GPIO is one of the functions that can be mux'd onto a pin, then I'd expect that to be represented in exactly the same way as any other function that could be mux'd onto the pin. Right. But additionally we also need to know the mux register to GPIO mapping for things like irq_set_irq_wake()/enable_irq_wake()/disable_irq_wake() that may be set dynamically depending on what the user wants. So, I'm not sure what GPIO-related information you want to represent. It seems that we should be able to do pinctrl_request_gpio that uses an external GPIO controller and also sets up the desired wake-up flags as needed. Anyways, not needed yet. too, we do have external DMA request lines for few pins available.. I'm not saying pinctrl fwk should know about that, but it's a similar mapping of pins to GPIO lines. Aren't DMA request lines also just another function that can be mux'd onto a pin? Yes it's a function for routing the signal. But that also needs to be configured in the DMA controller. Right now there's no need to have that mapping in the binding. 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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120503 22:08]:
In my mind in the driver we do not have to care how to list
register/unregister the group. We just need to be able to do this
pinctrl_register_group(...)
or
pinctrl_unregistewr_group(...)
On at91 we have this type of controller
Ah I see. Yeah makes sense. Also I think we should let the pinctrl
core eventually manage the pins more too. Right now the pins are
a static array in the driver, which makes things unnecessarily
complex for the DT case. It would be nice to also have something like
pinctrl_register/unregister_pin instead of requiring them all
be registered while registering with the framework initially.
But all that can be improved later on once we get the binding down..
one pin can have multiple function and each function can be on different pin
and we need to program and represent each of them one by one
And each pin have different parameter
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
and use macro as basicaly we are just this
and this can be applied to tegra too as you will just refer the pin in this hw
pin block
I was thinking of adding gpio eventually as a separate attribute with
something like the following. Here cam_d10 pin is used as gpio109:
cam_d10.gpio_109 {
pinctrl-simple,cells = 0xfa 0x104;/* OMAP_PIN_INPUT |
OMAP_MUX_MODE4 */
gpio = gpio4 13 0; /* gpio109 */
};
The reasoning for this is that as we may not care about the gpio number
for all pins, it should be optional. Would that work for you?
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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 08:03 Fri 04 May , Tony Lindgren wrote:
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120503 22:08]:
In my mind in the driver we do not have to care how to list
register/unregister the group. We just need to be able to do this
pinctrl_register_group(...)
or
pinctrl_unregistewr_group(...)
On at91 we have this type of controller
Ah I see. Yeah makes sense. Also I think we should let the pinctrl
core eventually manage the pins more too. Right now the pins are
a static array in the driver, which makes things unnecessarily
complex for the DT case. It would be nice to also have something like
pinctrl_register/unregister_pin instead of requiring them all
be registered while registering with the framework initially.
But all that can be improved later on once we get the binding down..
agreed at 100%
one pin can have multiple function and each function can be on different pin
and we need to program and represent each of them one by one
And each pin have different parameter
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
no pin number in the bank because it could not be gpio
evenif on at91 and nearly on the controller I known it is the case
and use macro as basicaly we are just this
and this can be applied to tegra too as you will just refer the pin in this
hw
pin block
I was thinking of adding gpio eventually as a separate attribute with
something like the following. Here cam_d10 pin is used as gpio109:
cam_d10.gpio_109 {
pinctrl-simple,cells = 0xfa 0x104;/* OMAP_PIN_INPUT |
OMAP_MUX_MODE4 */
gpio = gpio4 13 0; /* gpio109 */
};
The reasoning for this is that as we may not care about the gpio number
for all pins, it should be optional. Would that work for you?
yes
but I was thinking to put it as a param but why not
my idea was this
pinctrl@f200 {
#address-cells = 1;
#size-cells = 0;
compatible = atmel,at91rm9200-pinctrl;
atmel,mux-mask =
/*A B */
0x 0xffc003ff /* pioA */
0x 0x800f8f00 /* pioB */
0x 0x0e00 /* pioC */
0x 0xff0c1381 /* pioD */
0x 0x8181 /* pioE */
;
pioA: gpio@f200 {
compatible = atmel,at91rm9200-gpio;
reg = 0xf200 0x100;
interrupts = 2 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
pioB: gpio@f400 {
compatible = atmel,at91rm9200-gpio;
reg = 0xf400 0x100;
interrupts = 3 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
pioC: gpio@f600 {
compatible = atmel,at91rm9200-gpio;
reg = 0xf600 0x100;
interrupts = 4 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
pioD: gpio@f800 {
compatible = atmel,at91rm9200-gpio;
reg = 0xf800 0x100;
interrupts = 5 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
pioE: gpio@fa00 {
compatible = atmel,at91rm9200-gpio;
reg = 0xfa00 0x100;
interrupts = 5 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
dbgu {
pins = pioB 12 0 0
pioB 13 0 2 ;
/* with macro */
pins = pioB 12 MUX_A NO_PULL_UP
pioB 13 MUX_A PULL_UP ;
};
/* and also the notion of linked group
* as on uart of network you have often the same subset of pin use.
*
* As example on uart rxd/txd is use for the group without rts/cts
* and the one with it
* on ethernet the RMII pin are use also on MII
*/
uart0_rxd_txd {
pins = pioB 19 MUX_A PULL_UP /* rxd */
pioB 18 MUX_A NO_PULL_UP ; /* txd */
};
uart0_rts_cts {
groups = uart0_rxd_txd ;
pins = pioB 17 MUX_B NO_PULL_UP /* rts */
pioB 15 MUX_B NO_PULL_UP ; /* cts */
};
uart0_rts_cts_external_pull_up {
groups = uart0_rts_cts ;
gpios = pioC 1 0;
};
};
The idea is to avoid duplication the xlate for pins will be driver specific
with maybe a common implementation
the 3 or 4 first fix as done on gpio
Best Regards,
Re: [PATCH] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120504 08:58]:
On 08:03 Fri 04 May , Tony Lindgren wrote:
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
no pin number in the bank because it could not be gpio
Yes OK, but pin number 12 in the gpio bank, not in the mux register.
Got it.
pioD: gpio@f800 {
compatible = atmel,at91rm9200-gpio;
reg = 0xf800 0x100;
interrupts = 5 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
pioE: gpio@fa00 {
compatible = atmel,at91rm9200-gpio;
reg = 0xfa00 0x100;
interrupts = 5 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
dbgu {
pins = pioB 12 0 0
pioB 13 0 2 ;
/* with macro */
pins = pioB 12 MUX_A NO_PULL_UP
pioB 13 MUX_A PULL_UP ;
};
I could change to use this too no problem. The only concern I have is
that is pioB 12 immutable for all gpio controllers?
Grepping the *.dts* files, at least exynos is using the following
for gpios:
gpios = gpx2 0 0 0 2;
If we can conclude that phandle to the gpio controller instance and
the register offset is always enough here, then I'm OK changing to
that format. It would actually save some parsing in most cases.
/* and also the notion of linked group
* as on uart of network you have often the same subset of pin use.
*
* As example on uart rxd/txd is use for the group without rts/cts
* and the one with it
* on ethernet the RMII pin are use also on MII
*/
uart0_rxd_txd {
pins = pioB 19 MUX_A PULL_UP /* rxd */
pioB 18 MUX_A NO_PULL_UP ; /* txd */
};
uart0_rts_cts {
groups = uart0_rxd_txd ;
pins = pioB 17 MUX_B NO_PULL_UP /* rts */
pioB 15 MUX_B NO_PULL_UP ; /* cts */
};
uart0_rts_cts_external_pull_up {
groups = uart0_rts_cts ;
gpios = pioC 1 0;
};
};
The idea is to avoid duplication the xlate for pins will be driver specific
with maybe a common implementation
the 3 or 4 first fix as done on gpio
Yeah sounds doable to me, but can probably be added later?
Regarding grouping, basically for most cases it makes sense to have
three states: default, active, idle instead of just active and idle.
The reason is that for most cases the default pins only need to be
set once for each devices. Only few pins need to toggle between
active and idle states.
For example, omap2 uart needs to toggle rx pin to gpio input everytime
it hits idle to provide wake-up events. Other pins do not need to change.
As this is done all the time, the active and idle toggling should be kept
to minimum.
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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 09:34 Fri 04 May , Tony Lindgren wrote:
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120504 08:58]:
On 08:03 Fri 04 May , Tony Lindgren wrote:
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
no pin number in the bank because it could not be gpio
Yes OK, but pin number 12 in the gpio bank, not in the mux register.
Got it.
pioD: gpio@f800 {
compatible = atmel,at91rm9200-gpio;
reg = 0xf800 0x100;
interrupts = 5 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
pioE: gpio@fa00 {
compatible = atmel,at91rm9200-gpio;
reg = 0xfa00 0x100;
interrupts = 5 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
dbgu {
pins = pioB 12 0 0
pioB 13 0 2 ;
/* with macro */
pins = pioB 12 MUX_A NO_PULL_UP
pioB 13 MUX_A PULL_UP ;
};
I could change to use this too no problem. The only concern I have is
that is pioB 12 immutable for all gpio controllers?
Grepping the *.dts* files, at least exynos is using the following
for gpios:
gpios = gpx2 0 0 0 2;
If we can conclude that phandle to the gpio controller instance and
the register offset is always enough here, then I'm OK changing to
that format. It would actually save some parsing in most cases.
I would said yes but we could use the same notion to create pin-bank
the idea is to refer to the bank and then the pin inside
after if a driver need more argumement as on exynos they will have to
implement their own xlate
as we did on at91 for the irq xlate
/* and also the notion of linked group
* as on uart of network you have often the same subset of pin use.
*
* As example on uart rxd/txd is use for the group without rts/cts
* and the one with it
* on ethernet the RMII pin are use also on MII
*/
uart0_rxd_txd {
pins = pioB 19 MUX_A PULL_UP /* rxd */
pioB 18 MUX_A NO_PULL_UP ; /* txd */
};
uart0_rts_cts {
groups = uart0_rxd_txd ;
pins = pioB 17 MUX_B NO_PULL_UP /* rts */
pioB 15 MUX_B NO_PULL_UP ; /* cts */
};
uart0_rts_cts_external_pull_up {
groups = uart0_rts_cts ;
gpios = pioC 1 0;
};
};
The idea is to avoid duplication the xlate for pins will be driver specific
with maybe a common implementation
the 3 or 4 first fix as done on gpio
Yeah sounds doable to me, but can probably be added later?
for the sub-group stuff yes agreed
Regarding grouping, basically for most cases it makes sense to have
three states: default, active, idle instead of just active and idle.
The reason is that for most cases the default pins only need to be
set once for each devices. Only few pins need to toggle between
active and idle states.
yeah agreed
Best Regards,
J.
--
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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 05/04/2012 10:34 AM, Tony Lindgren wrote:
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120504 08:58]:
On 08:03 Fri 04 May , Tony Lindgren wrote:
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
no pin number in the bank because it could not be gpio
Yes OK, but pin number 12 in the gpio bank, not in the mux register.
Got it.
I'd prefer to avoid any references to GPIOs here; not all muxable pins
are GPIOs and not all GPIOs are muxable pins. Lets keep the two concepts
independent.
pioD: gpio@f800 {
compatible = atmel,at91rm9200-gpio;
reg = 0xf800 0x100;
interrupts = 5 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
pioE: gpio@fa00 {
compatible = atmel,at91rm9200-gpio;
reg = 0xfa00 0x100;
interrupts = 5 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
dbgu {
pins = pioB 12 0 0
pioB 13 0 2 ;
/* with macro */
pins = pioB 12 MUX_A NO_PULL_UP
pioB 13 MUX_A PULL_UP ;
};
I could change to use this too no problem. The only concern I have is
that is pioB 12 immutable for all gpio controllers?
You mean is the number of cells used to specify a GPIO the same
everywhere? No. It's defined by #gpio-cells in the GPIO controller node.
But again, the GPIO binding shouldn't be related to the pinctrl binding
directly.
Grepping the *.dts* files, at least exynos is using the following
for gpios:
gpios = gpx2 0 0 0 2;
If we can conclude that phandle to the gpio controller instance and
the register offset is always enough here, then I'm OK changing to
that format. It would actually save some parsing in most cases.
/* and also the notion of linked group
* as on uart of network you have often the same subset of pin use.
*
* As example on uart rxd/txd is use for the group without rts/cts
* and the one with it
* on ethernet the RMII pin are use also on MII
*/
uart0_rxd_txd {
pins = pioB 19 MUX_A PULL_UP /* rxd */
pioB 18 MUX_A NO_PULL_UP ; /* txd */
};
I don't really see how that DT format represents pins, functions, and
nodes directly, and separately from which of those a board chooses to
use. I think this binding and the one Tony originally proposed are
eseentially semantically identical.
Going back to my idea of separating SoC and board configurations, if we
did that, I'd expect to see something more like:
soc.dtsi or board.dts:
This is the data that the pin controller driver needs to export to
pinctrl core. This can be completely enumerated in the soc.dtsi, or
perhaps for uncommonly used pins/groups/functions, only included in the
board.dts that actually uses it to cut down on soc.dtsi's size:
This data is not needed for SoCs whose pinctrl drivers include it in
their driver file instead of DT.
/ {
pinctrl@... {
pins {
uart1_rx_pin: uart1_rx {
/* register to program the pin if per-pin muxing*/
reg = ...;
name = UART1_RX;
...;
}
uart1_tx_pin: uart1_tx {
reg = ...;
name = UART1_TX;
...;
}
uart2_rx_pin: uart2_rx {
reg = ...;
name = UART2_RX;
...;
}
uart2_tx_pin: uart2_tx {
reg = ...;
name = UART2_TX;
...;
}
};
pingroups {
uart1_pg: uart1 {
/* register to program the group, if per-grouop muxing */
reg = ...;
pins = pin_uart1_rx pin_uart1_tx;
}
uart2_pg: uart2 {
pins = pin_uart2_rx pin_uart2_tx;
}
};
functions {
uart1_func: uart1 {
selectable-on = uart1_pg 0; /* where, mux value */
};
uart2_func: uart2 {
selectable-on = uart2_pg 0;
};
spi_func: spi {
selectable-on = uart1_pg 1 uart2_pg 1;
};
};
soc.dtsi or board.dts:
This is part of the data used to construct the mapping table. Common
options for function X on pin/pingroup Y can be included in soc.dtsi to
reduce duplication in board files. Uncommon options can be included
directly in the board.dts that uses them, to avoid bloating soc.dtsi:
This data is needed irrespective of whether a SoC pinctrl driver stores
the pin/function/group information in DT or in the driver itself.
/ {
pinctrl@... {
uart1_uart1 {
muxing = uart1_pg
Re: [PATCH] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 05/02/2012 11:24 AM, Tony Lindgren wrote:
Add simple pinctrl driver using device tree data.
Currently this driver only works on omap2+ series of
processors, where there is either an 8 or 16-bit padconf
register for each pin. Support for other similar pinmux
controllers could be added.
Note that this patch does not yet support pinconf_ops
or GPIO. Further.
Signed-off-by: Tony Lindgren t...@atomide.com
---
Here's this one finally updated to use the common pinctrl bindings.
That sure cleaned up a bunch of the nasty things in this driver :)
Nice job Stephen!
Thanks.
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt
b/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt
+Required properties:
+- compatible : one of:
+ - pinctrl-simple
+ - ti,omap2420-padconf
+ - ti,omap2430-padconf
+ - ti,omap3-padconf
+ - ti,omap4-padconf
I'm in two minds about this.
If this is truly intended to be generic, I would not document any of the
ti compatible values here. Instead, I'd create a binding document for
the TI controllers that basically just says this uses the bindings in
pinctrl-simple.txt, with the following additions, and list the
compatible values as an addition.
On the other hand, I worry about whether using pinctrl-simple here as
the compatible value is going to cause issues:
Certainly, this is a pretty simple driver, and most likely reasonably
generic an applicable to many SoCs. However, it doesn't cover a bunch of
cases that I'd still consider simple. For example, what if each pin
has a separate mux and pinconf register? There are probably many such
small cases that would add up to something more complex. to cover those
cases, will we be able to extend pinctrl-simple to cover them, and
continue to be backwards compatible, or will we need to create a
binding/driver for pinctrl-simple-1, pinctrl-simple-2, pinctrl-simple-3
each of which covers some different, yet still simple, configuration?
To resolve this, perhaps it would be best to rework this binding and
driver as a set of utility code that other bindings and drivers can
build upon, rather than making it a standalone directly usable driver
and binding.
Honestly, I'm not really sure which way to go here.
+- reg : offset and length of the register set for the mux registers
+- #pinctrl-cells : width of the pinctrl array, currently only 2 is supported
+- pinctrl-simple,register-width : pinmux register access width
+- pinctrl-simple,function-mask : mask of allowed pinmux function bits
+- pinctrl-simple,function-off : function off mode for disabled state
+- pinctrl-simple,pinconf-mask : mask of allowed pinconf bits
+
+This driver uses the common pinctrl bindings as specified in
+pinctrl-bindings.txt document in this directory. The common bindings are used
+to specify the client device states using pinctrl-0 and pinctrl-names
entries.
I think just remove the second sentence there; it's implicit given the
first.
+/* board specific .dts file, such as omap4-sdp.dts */
+omap4_pmx_core {
+
+ /*
+ * map all board specific static pins enabled by the pinctrl driver
+ * itself during the boot (or just set them up in the bootloader)
+ */
+ pinctrl-names = default;
+ pinctrl-0 = board_pins;
+
+ board_pins: pinmux_board_pins {
+ board_static_pins {
I'd like to see a little more explanation of the node structure here.
I.e. what does each node represent, why are there two levels of nodes, etc.
Given that the pinctrl-simple,cells can specify both mux function and
pin configuration for each pin, i.e. everything you need to, I don't see
why you'd ever need two levels of nodes here. I'd expect each pin
configuration node (in pinctrl core bindings terminology) to be a
single level:
node {
pinctrl-simple,cells = ...;
};
where node is what's references in the pinctrl-0 property by pinctrl
client drivers.
+ pinctrl-simple,cells =
cells here doesn't seem like the right word. Perhaps pins or
configuration? Cells to me seems semi-reserved for binding cell
count description, like #gpio-cells, #address-cells, etc.
Also, there's nothing in the free-form text part of this binding
document that says describes the set of properties that need to exist in
these pin configuration nodes, nor what their content is.
+ 0x6c 0xf/* CSI21_DX3 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ 0x6e 0xf/* CSI21_DY3 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ 0x70 0xf/* CSI21_DX4 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ 0x72 0xf/* CSI21_DY4 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ ;
+ };
+ };
+
+
+ /* map all uart2 pins as a single function */
+ uart2_pins: pinmux_uart2_pins {
+ uart2_pins {
+
Re: [PATCH] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
* Stephen Warren swar...@wwwdotorg.org [120504 12:27]:
On 05/02/2012 11:24 AM, Tony Lindgren wrote:
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt
b/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt
+Required properties:
+- compatible : one of:
+ - pinctrl-simple
+ - ti,omap2420-padconf
+ - ti,omap2430-padconf
+ - ti,omap3-padconf
+ - ti,omap4-padconf
I'm in two minds about this.
If this is truly intended to be generic, I would not document any of the
ti compatible values here. Instead, I'd create a binding document for
the TI controllers that basically just says this uses the bindings in
pinctrl-simple.txt, with the following additions, and list the
compatible values as an addition.
Hmm sure makes sense. I'll move the TI specific stuff into a separate
doc.
On the other hand, I worry about whether using pinctrl-simple here as
the compatible value is going to cause issues:
Certainly, this is a pretty simple driver, and most likely reasonably
generic an applicable to many SoCs. However, it doesn't cover a bunch of
cases that I'd still consider simple. For example, what if each pin
has a separate mux and pinconf register? There are probably many such
small cases that would add up to something more complex. to cover those
cases, will we be able to extend pinctrl-simple to cover them, and
continue to be backwards compatible, or will we need to create a
binding/driver for pinctrl-simple-1, pinctrl-simple-2, pinctrl-simple-3
each of which covers some different, yet still simple, configuration?
Yes getting the binding right is the critical part here, everything else
can be added as needed. I was thinking about using separate properties
for auxilary registers, but now thinking about it a bit more, it may not
be sufficient.
How about we make some of these properties into arrays? For example:
#pinctrl-cells = 6;
pinctrl-simple,function-mask = 0x 0x 0x;
pinctrl-simple,function-off = 0x7 0x7 0x7;
pinctrl-simple,pinconf-mask = 0x 0x 0x;
Then for handling the set/clear bits case of registers, we could do that
automatically if both masks are 0 for some registers. We actually have
that for some omap1 where there are three registers per mux with bits
to set or clear. And these bits are not the same across all registers..
To resolve this, perhaps it would be best to rework this binding and
driver as a set of utility code that other bindings and drivers can
build upon, rather than making it a standalone directly usable driver
and binding.
Honestly, I'm not really sure which way to go here.
Sure that's doable, but sounds like that can be also done later after
the binding is agreed on.
+- reg : offset and length of the register set for the mux registers
+- #pinctrl-cells : width of the pinctrl array, currently only 2 is
supported
+- pinctrl-simple,register-width : pinmux register access width
+- pinctrl-simple,function-mask : mask of allowed pinmux function bits
+- pinctrl-simple,function-off : function off mode for disabled state
+- pinctrl-simple,pinconf-mask : mask of allowed pinconf bits
+
+This driver uses the common pinctrl bindings as specified in
+pinctrl-bindings.txt document in this directory. The common bindings are
used
+to specify the client device states using pinctrl-0 and pinctrl-names
entries.
I think just remove the second sentence there; it's implicit given the
first.
Sure.
+/* board specific .dts file, such as omap4-sdp.dts */
+omap4_pmx_core {
+
+ /*
+* map all board specific static pins enabled by the pinctrl driver
+* itself during the boot (or just set them up in the bootloader)
+*/
+ pinctrl-names = default;
+ pinctrl-0 = board_pins;
+
+ board_pins: pinmux_board_pins {
+ board_static_pins {
I'd like to see a little more explanation of the node structure here.
I.e. what does each node represent, why are there two levels of nodes, etc.
Given that the pinctrl-simple,cells can specify both mux function and
pin configuration for each pin, i.e. everything you need to, I don't see
why you'd ever need two levels of nodes here. I'd expect each pin
configuration node (in pinctrl core bindings terminology) to be a
single level:
node {
pinctrl-simple,cells = ...;
};
where node is what's references in the pinctrl-0 property by pinctrl
client drivers.
Good point, it seems we can remove the subnodes for the cases where we
parse multiple registers in one entry. Two levels of nodes are needed for
the cases where we need to specify GPIO for a pin.
+ pinctrl-simple,cells =
cells here doesn't seem like the right word. Perhaps pins or
configuration? Cells to me seems semi-reserved for binding cell
count description, like #gpio-cells, #address-cells, etc.
Sure, pins works just as well here, it's shorter than
Re: [PATCH] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
* Stephen Warren swar...@wwwdotorg.org [120504 11:59]:
On 05/04/2012 10:34 AM, Tony Lindgren wrote:
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120504 08:58]:
On 08:03 Fri 04 May , Tony Lindgren wrote:
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
no pin number in the bank because it could not be gpio
Yes OK, but pin number 12 in the gpio bank, not in the mux register.
Got it.
I'd prefer to avoid any references to GPIOs here; not all muxable pins
are GPIOs and not all GPIOs are muxable pins. Lets keep the two concepts
independent.
And it seems that pioA 12 is not always enough information for the pinctrl
driver to request a GPIO. So it's best to specify it separately.
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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 12:55 Fri 04 May , Stephen Warren wrote:
On 05/04/2012 10:34 AM, Tony Lindgren wrote:
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120504 08:58]:
On 08:03 Fri 04 May , Tony Lindgren wrote:
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
Hmm I assume the 12 above the gpio number?
no pin number in the bank because it could not be gpio
Yes OK, but pin number 12 in the gpio bank, not in the mux register.
Got it.
I'd prefer to avoid any references to GPIOs here; not all muxable pins
are GPIOs and not all GPIOs are muxable pins. Lets keep the two concepts
independent.
my idea was to have a phandle pinctrl specific to represent the bank
and use it in the same way as done on gpio
pioD: gpio@f800 {
compatible = atmel,at91rm9200-gpio;
reg = 0xf800 0x100;
interrupts = 5 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
pioE: gpio@fa00 {
compatible = atmel,at91rm9200-gpio;
reg = 0xfa00 0x100;
interrupts = 5 4;
#gpio-cells = 2;
gpio-controller;
interrupt-controller;
};
dbgu {
pins = pioB 12 0 0
pioB 13 0 2 ;
/* with macro */
pins = pioB 12 MUX_A NO_PULL_UP
pioB 13 MUX_A PULL_UP ;
};
I could change to use this too no problem. The only concern I have is
that is pioB 12 immutable for all gpio controllers?
You mean is the number of cells used to specify a GPIO the same
everywhere? No. It's defined by #gpio-cells in the GPIO controller node.
But again, the GPIO binding shouldn't be related to the pinctrl binding
directly.
Grepping the *.dts* files, at least exynos is using the following
for gpios:
gpios = gpx2 0 0 0 2;
If we can conclude that phandle to the gpio controller instance and
the register offset is always enough here, then I'm OK changing to
that format. It would actually save some parsing in most cases.
/* and also the notion of linked group
* as on uart of network you have often the same subset of pin use.
*
* As example on uart rxd/txd is use for the group without rts/cts
* and the one with it
* on ethernet the RMII pin are use also on MII
*/
uart0_rxd_txd {
pins = pioB 19 MUX_A PULL_UP /* rxd */
pioB 18 MUX_A NO_PULL_UP ; /* txd */
};
I don't really see how that DT format represents pins, functions, and
nodes directly, and separately from which of those a board chooses to
use. I think this binding and the one Tony originally proposed are
eseentially semantically identical.
Going back to my idea of separating SoC and board configurations, if we
did that, I'd expect to see something more like:
soc.dtsi or board.dts:
This is the data that the pin controller driver needs to export to
pinctrl core. This can be completely enumerated in the soc.dtsi, or
perhaps for uncommonly used pins/groups/functions, only included in the
board.dts that actually uses it to cut down on soc.dtsi's size:
This data is not needed for SoCs whose pinctrl drivers include it in
their driver file instead of DT.
I agree on at91 I propose exactly this but get the following comment tat we
are going to have too much node.
so the idea I propoose with the pins array is to avoid this issue
my first bindings on at91
functions {
};
1) we describe one functin per pin
functions {
rxd_pb12 {
atmel,pin-id = pioB 12;
atmel,mux = 0;
};
txd_pb13 {
atmel,pin-id = piaB 13;
atmel,pull = 2;
atmel,mux = 0;
};
txd0_pb19 {
atmel,pin-id = pioB 19;
atmel,pull = 2;
atmel,mux = 0;
};
rxd0_pb18 {
atmel,pin-id = pioB 18;
atmel,mux = 0;
};
rts0_pb17 {
atmel,pin-id = pioB 17;
atmel,mux = 1;
};
cts0_pb15 {
atmel,pin-id = pioB 15;
atmel,mux = 1;
};
};
groups {
dbgu {
pinctrl,functions =
rxd_pb12 txd_pb13 ;
};
uart0_rxd_txd {
pinctrl,functions =
rxd0_pb18 txd0_pb19 ;
};
uart0_rts_cts {
pinctrl,functions =
rxd0_pb18 txd0_pb19
rts0_pb17 cts0_pb15 ;
};
};
Best Regards,
J.
--
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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
Hi,
I really like it
I was working on something simillar
but can we split the group management so we can use it on other
bindings
Best Regards,
J.
On 10:24 Wed 02 May , Tony Lindgren wrote:
Add simple pinctrl driver using device tree data.
Currently this driver only works on omap2+ series of
processors, where there is either an 8 or 16-bit padconf
register for each pin. Support for other similar pinmux
controllers could be added.
Note that this patch does not yet support pinconf_ops
or GPIO. Further.
Signed-off-by: Tony Lindgren t...@atomide.com
---
Here's this one finally updated to use the common pinctrl bindings.
That sure cleaned up a bunch of the nasty things in this driver :)
Nice job Stephen!
---
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt
b/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt
new file mode 100644
index 000..7b32263
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt
@@ -0,0 +1,125 @@
+Generic simple device tree based pinctrl driver
+
+Required properties:
+- compatible : one of:
+ - pinctrl-simple
+ - ti,omap2420-padconf
+ - ti,omap2430-padconf
+ - ti,omap3-padconf
+ - ti,omap4-padconf
+- reg : offset and length of the register set for the mux registers
+- #pinctrl-cells : width of the pinctrl array, currently only 2 is supported
+- pinctrl-simple,register-width : pinmux register access width
+- pinctrl-simple,function-mask : mask of allowed pinmux function bits
+- pinctrl-simple,function-off : function off mode for disabled state
+- pinctrl-simple,pinconf-mask : mask of allowed pinconf bits
+
+This driver uses the common pinctrl bindings as specified in
+pinctrl-bindings.txt document in this directory. The common bindings are used
+to specify the client device states using pinctrl-0 and pinctrl-names
entries.
+
+This driver supports parsing one or more pinctrl functions as the subnodes of
+the pinctrl driver entry. One or more registers can be specified for each
+function, see uart2 and uart3 examples below. If you are concerned about boot
+time, parsing multiple registers in a single function is slightly faster.
+
+For setting all static board specific pins, see the pinmux_board_pins example
+below. If you are concerned about the boot time, set up the static pins in
+the bootloader, and only set up selected pins as device tree entries.
+
+This driver assumes currently that there is one register for each pin. If you
+have some pins with more complicated configuration, you can set up a separate
+hardware specific driver for those pins.
+
+Example:
+
+/* SoC common file, such as omap4.dtsi */
+omap4_pmx_core: pinmux@4a100040 {
+ compatible = ti,omap4-padconf;
+ reg = 0x4a100040 0x0196;
+ #address-cells = 1;
+ #size-cells = 0;
+ #pinctrl-cells = 2;
+ pinctrl-simple,register-width = 16;
+ pinctrl-simple,function-mask = 0x7;
+ pinctrl-simple,function-off = 0x;
+ pinctrl-simple,pinconf-mask = 0xfff8;
+};
+
+omap4_pmx_wkup: pinmux@4a31e040 {
+ compatible = ti,omap4-padconf;
+ reg = 0x4a31e040 0x0038;
+ #address-cells = 1;
+ #size-cells = 0;
+ #pinctrl-cells = 2;
+ pinctrl-simple,register-width = 16;
+ pinctrl-simple,function-mask = 0x7;
+ pinctrl-simple,function-off = 0x;
+ pinctrl-simple,pinconf-mask = 0xfff8;
+};
+
+
+/* board specific .dts file, such as omap4-sdp.dts */
+omap4_pmx_core {
+
+ /*
+ * map all board specific static pins enabled by the pinctrl driver
+ * itself during the boot (or just set them up in the bootloader)
+ */
+ pinctrl-names = default;
+ pinctrl-0 = board_pins;
+
+ board_pins: pinmux_board_pins {
+ board_static_pins {
+ pinctrl-simple,cells =
+ 0x6c 0xf/* CSI21_DX3 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ 0x6e 0xf/* CSI21_DY3 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ 0x70 0xf/* CSI21_DX4 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ 0x72 0xf/* CSI21_DY4 OMAP_PIN_OUTPUT |
OMAP_MUX_MODE7 */
+ ;
+ };
+ };
+
+
+ /* map all uart2 pins as a single function */
+ uart2_pins: pinmux_uart2_pins {
+ uart2_pins {
+ pinctrl-simple,cells =
+ 0xd8 0x118 /* UART2_CTS
OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0 */
+ 0xda 0 /* UART2_RTS OMAP_PIN_OUTPUT |
OMAP_MUX_MODE0 */
+ 0xdc 0x118 /* UART2_RX
OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0 */
+ 0xde 0 /* UART2_TX OMAP_PIN_OUTPUT |
OMAP_MUX_MODE0 */
+ ;
+
Re: [PATCH] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
Hi, * Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120503 00:16]: I really like it I was working on something simillar but can we split the group management so we can use it on other bindings Hmm I'm not sure I follow on the group management splitting, can you specify what you have in mind here? If you mean moving more things into pinctrl fwk, then yeah I'd assume that will happen eventually and drivers like this will end up becoming more minimal. 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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 05/03/2012 09:27 AM, Tony Lindgren wrote: Hi, * Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120503 00:16]: I really like it I was working on something simillar but can we split the group management so we can use it on other bindings Hmm I'm not sure I follow on the group management splitting, can you specify what you have in mind here? If you mean moving more things into pinctrl fwk, then yeah I'd assume that will happen eventually and drivers like this will end up becoming more minimal. Jean-Christophe, forgive me if I'm putting words in your mouth, but I assume the following is what you mean: There are two pieces of data required by the pinctrl subsystem: a) The set of pins, functions, and groups that exist. b) The specific function to select for each pin/group on a given board. Item (a) can be represented in the pinctrl driver (e.g. as in the Tegra driver), or can be represented in device tree in order to avoid large tables in the driver. Item (b) has to be represented in device tree, since the whole point is that it's board-specific. For all DT bindings I've seen that choose to represent (a) in the DT rather than in the driver, the DT represents (b) directly, and (a) is implicitly extracted/created based on (a). When I was first thinking about DT bindings for pinctrl, I had hoped that even if (a) was represented in DT, the DT nodes/properties for (a) and (b) would be entirely separate, so that the binding for (b) could be completely common across all SoCs, even though the binding for (a) would perhaps be different across SoCs (if it existed at all). So, perhaps Jean-Christophe is talking about splitting up (a) and (b) in device tree? Or perhaps Jean-Christophe only refers to the code that creates the group and function definitions from (b), and not the actual DT binding itself? -- 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] pinctrl: Add generic pinctrl-simple driver that supports omap2+ padconf
On 16:34 Thu 03 May , Stephen Warren wrote:
On 05/03/2012 09:27 AM, Tony Lindgren wrote:
Hi,
* Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com [120503 00:16]:
I really like it
I was working on something simillar
but can we split the group management so we can use it on other
bindings
Hmm I'm not sure I follow on the group management splitting, can you specify
what you have in mind here?
If you mean moving more things into pinctrl fwk, then yeah I'd assume that
will happen eventually and drivers like this will end up becoming more
minimal.
Jean-Christophe, forgive me if I'm putting words in your mouth, but I
assume the following is what you mean:
There are two pieces of data required by the pinctrl subsystem:
a) The set of pins, functions, and groups that exist.
b) The specific function to select for each pin/group on a given board.
Item (a) can be represented in the pinctrl driver (e.g. as in the Tegra
driver), or can be represented in device tree in order to avoid large
tables in the driver.
Item (b) has to be represented in device tree, since the whole point is
that it's board-specific.
For all DT bindings I've seen that choose to represent (a) in the DT
rather than in the driver, the DT represents (b) directly, and (a) is
implicitly extracted/created based on (a).
When I was first thinking about DT bindings for pinctrl, I had hoped
that even if (a) was represented in DT, the DT nodes/properties for (a)
and (b) would be entirely separate, so that the binding for (b) could be
completely common across all SoCs, even though the binding for (a) would
perhaps be different across SoCs (if it existed at all).
So, perhaps Jean-Christophe is talking about splitting up (a) and (b) in
device tree?
Or perhaps Jean-Christophe only refers to the code that creates the
group and function definitions from (b), and not the actual DT binding
itself?
yes you are right Stephen
I was thinking of both but the second could be a first step
today we tend all to represent the group of pin in DT
for TI, IM, MXC, at91, ST and others
the way to represend the groups are exactly the same
a group is a node with a set on pin
uart {
pincfg..
}
and the group is uart
so we do need to have a common way to handle it in c
the code propose by Tony is really what I'm working on to acheive this
In my mind in the driver we do not have to care how to list
register/unregister the group. We just need to be able to do this
pinctrl_register_group(...)
or
pinctrl_unregistewr_group(...)
On at91 we have this type of controller
one pin can have multiple function and each function can be on different pin
and we need to program and represent each of them one by one
And each pin have different parameter
so I was thinking to do like on gpio
uart {
pin = pioA 12 {pararms}
}
and use macro as basicaly we are just this
and this can be applied to tegra too as you will just refer the pin in this hw
pin block
Best Regards,
J.
--
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
