[RFC PATCH 0/3] pinctrl: at91: add support for generic pinconf

[RFC PATCH 0/3] pinctrl: at91: add support for generic pinconf

[Boris BREZILLON]

Hello,

This patch series is an attempt to add support for generic pin config
syntax to at91 pinctrl driver.

My primary goal is to add support for output configuration from dt definition.
This is needed to fully move at91rm9200ek board to dt (other boards may have
the same needs).
This board use a pin to drive an external switch which select between 2
functionnalities:
 - mmc interface
 - spi interface
The pin level is currently configured in the board init (init_machine) function
based on user config choices (CONFIG_MTD_AT91_DATAFLASH_CARD).

Instead of adding a new flag to the current (native) pin config binding, I
tried to add support for the generic pin config used by some pinctrl drivers
(i.e. rockchip).

Is this the right way to do this or should I add a new at91 native flags for
output config (OUTPUT_HIGH/LOW) ?

The second patch introduce a new config parameter to add a glitch filter on a
specific pin.
Glitch filter is similar to bounce filter (or debounce) but with a smaller
delay (expressed in nsecs ?).

I'm not sure this is the right approach.
Maybe we should reuse the debounce parameter and add a flag to specify the delay
unit (usec or nsec).

What do you think ?

The third patch migrate sama5 dt boards to the new generic config syntax.

Please feel free to share your thoughts.

Best Regards,

Boris


Boris BREZILLON (3):
  pinctrl: add new generic pinconf config for deglitch filter
  pinctrl: at91: add support for generic pinconf
  ARM: at91/dt: move sama5 to generic pinconf

 .../bindings/pinctrl/atmel,at91-pinctrl.txt        |   43 ++-
 .../bindings/pinctrl/pinctrl-bindings.txt          |    1 +
 arch/arm/boot/dts/sama5d3.dtsi                     |  363 ++++++++++----------
 arch/arm/boot/dts/sama5d3xdm.dtsi                  |    2 +-
 arch/arm/boot/dts/sama5d3xmb.dtsi                  |   12 +-
 drivers/pinctrl/Kconfig                            |    2 +-
 drivers/pinctrl/pinconf-generic.c                  |    2 +
 drivers/pinctrl/pinctrl-at91.c                     |  265 +++++++++++++-
 include/linux/pinctrl/pinconf-generic.h            |    5 +
 9 files changed, 494 insertions(+), 201 deletions(-)

-- 
1.7.9.5

[RFC PATCH 1/3] pinctrl: add new generic pinconf config for deglitch filter

[Boris BREZILLON]

Add a new parameter to support deglitch filter configuration.
A deglitch filter works like a debounce filter but with a smaller
delay (nanoseconds).

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
---
 .../bindings/pinctrl/pinctrl-bindings.txt          |    1 +
 drivers/pinctrl/pinconf-generic.c                  |    2 ++
 include/linux/pinctrl/pinconf-generic.h            |    5 +++++
 3 files changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
index 1958ca9..2f7613e 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -154,6 +154,7 @@ drive-strength		- sink or source at most X mA
 input-schmitt-enable	- enable schmitt-trigger mode
 input-schmitt-disable	- disable schmitt-trigger mode
 input-debounce		- debounce mode with debound time X
+input-deglitch		- deglitch mode
 low-power-enable	- enable low power mode
 low-power-disable	- disable low power mode
 output-low		- set the pin to output mode with low level
diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 2c62225..f14a386 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -51,6 +51,7 @@ static struct pin_config_item conf_items[] = {
 	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT_ENABLE, "input schmitt enabled", NULL),
 	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT, "input schmitt trigger", NULL),
 	PCONFDUMP(PIN_CONFIG_INPUT_DEBOUNCE, "input debounce", "usec"),
+	PCONFDUMP(PIN_CONFIG_INPUT_DEGLITCH, "input deglitch", NULL),
 	PCONFDUMP(PIN_CONFIG_POWER_SOURCE, "pin power source", "selector"),
 	PCONFDUMP(PIN_CONFIG_SLEW_RATE, "slew rate", NULL),
 	PCONFDUMP(PIN_CONFIG_LOW_POWER_MODE, "pin low power", "mode"),
@@ -163,6 +164,7 @@ static struct pinconf_generic_dt_params dt_params[] = {
 	{ "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 },
 	{ "input-schmitt-disable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0 },
 	{ "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 },
+	{ "input-deglitch", PIN_CONFIG_INPUT_DEGLITCH, 0 },
 	{ "low-power-enable", PIN_CONFIG_LOW_POWER_MODE, 1 },
 	{ "low-power-disable", PIN_CONFIG_LOW_POWER_MODE, 0 },
 	{ "output-low", PIN_CONFIG_OUTPUT, 0, },
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index fb90ef5..aa06535 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -72,6 +72,10 @@
  *	which means it will wait for signals to settle when reading inputs. The
  *	argument gives the debounce time in usecs. Setting the
  *	argument to zero turns debouncing off.
+  * @PIN_CONFIG_INPUT_DEGLITCH: this will configure the pin to deglitch mode,
+ *	which means it will wait for signals to settle when reading inputs. The
+ *      If the argument != 0, deglitch mode is enabled. If it's 0, deglitch
+ *      mode is disabled.
  * @PIN_CONFIG_POWER_SOURCE: if the pin can select between different power
  *	supplies, the argument to this parameter (on a custom format) tells
  *	the driver which alternative power source to use.
@@ -102,6 +106,7 @@ enum pin_config_param {
 	PIN_CONFIG_INPUT_SCHMITT_ENABLE,
 	PIN_CONFIG_INPUT_SCHMITT,
 	PIN_CONFIG_INPUT_DEBOUNCE,
+	PIN_CONFIG_INPUT_DEGLITCH,
 	PIN_CONFIG_POWER_SOURCE,
 	PIN_CONFIG_SLEW_RATE,
 	PIN_CONFIG_LOW_POWER_MODE,
-- 
1.7.9.5

Re: [RFC PATCH 1/3] pinctrl: add new generic pinconf config for deglitch filter

[Stephen Warren]

On 08/24/2013 03:35 PM, Boris BREZILLON wrote:
> Add a new parameter to support deglitch filter configuration.
> A deglitch filter works like a debounce filter but with a smaller
> delay (nanoseconds).

Why not use the existing debounce property, just with a small delay
specified. It seems like that's exactly what the property is for?

Re: [RFC PATCH 1/3] pinctrl: add new generic pinconf config for deglitch filter

[boris brezillon]

Hello Stephen,

On 26/08/2013 18:50, Stephen Warren wrote:
> On 08/24/2013 03:35 PM, Boris BREZILLON wrote:
>> Add a new parameter to support deglitch filter configuration.
>> A deglitch filter works like a debounce filter but with a smaller
>> delay (nanoseconds).
> Why not use the existing debounce property, just with a small delay
> specified. It seems like that's exactly what the property is for?
That's one of the question I asked in my cover letter :-)

Indeed the at91 deglitch filter delay is not configurable and is statically
assigned to half a master clk cycle (if master clk = 133MHz -> 8 ns).
The debounce property argument is currently expressed in usecs.

This will result in always selecting the debounce filter (which is also
available on at91 SoCs) over the deglitch filter.

Could we add a flag in the deglitch argument to specify the delay unit
(nsecs or usecs) ?


Best Regards,

Boris

Re: [RFC PATCH 1/3] pinctrl: add new generic pinconf config for deglitch filter

[Stephen Warren]

On 08/26/2013 11:01 AM, boris brezillon wrote:
> Hello Stephen,
> 
> On 26/08/2013 18:50, Stephen Warren wrote:
>> On 08/24/2013 03:35 PM, Boris BREZILLON wrote:
>>> Add a new parameter to support deglitch filter configuration.
>>> A deglitch filter works like a debounce filter but with a smaller
>>> delay (nanoseconds).
>> Why not use the existing debounce property, just with a small delay
>> specified. It seems like that's exactly what the property is for?
> That's one of the question I asked in my cover letter :-)
> 
> Indeed the at91 deglitch filter delay is not configurable and is statically
> assigned to half a master clk cycle (if master clk = 133MHz -> 8 ns).
> The debounce property argument is currently expressed in usecs.
> 
> This will result in always selecting the debounce filter (which is also
> available on at91 SoCs) over the deglitch filter.
> 
> Could we add a flag in the deglitch argument to specify the delay unit
> (nsecs or usecs) ?

If the value is hard-coded in HW, why not use non-zero (or 1) to enable
and zero to disable?

(this kind of thing is why I'm not convinced that generic pinconf works
so well... What if we need psecs in the future?)

Re: [RFC PATCH 1/3] pinctrl: add new generic pinconf config for deglitch filter

[boris brezillon]

On 27/08/2013 05:55, Stephen Warren wrote:
> On 08/26/2013 11:01 AM, boris brezillon wrote:
>> Hello Stephen,
>>
>> On 26/08/2013 18:50, Stephen Warren wrote:
>>> On 08/24/2013 03:35 PM, Boris BREZILLON wrote:
>>>> Add a new parameter to support deglitch filter configuration.
>>>> A deglitch filter works like a debounce filter but with a smaller
>>>> delay (nanoseconds).
>>> Why not use the existing debounce property, just with a small delay
>>> specified. It seems like that's exactly what the property is for?
>> That's one of the question I asked in my cover letter :-)
>>
>> Indeed the at91 deglitch filter delay is not configurable and is statically
>> assigned to half a master clk cycle (if master clk = 133MHz -> 8 ns).
>> The debounce property argument is currently expressed in usecs.
>>
>> This will result in always selecting the debounce filter (which is also
>> available on at91 SoCs) over the deglitch filter.
>>
>> Could we add a flag in the deglitch argument to specify the delay unit
>> (nsecs or usecs) ?
> If the value is hard-coded in HW, why not use non-zero (or 1) to enable
> and zero to disable?

Indeed at91 pins support both deglitch and debounce filter and I have to 
choose
between the two given the argument value (in usec).

Here's what I can do:

if (arg >= 1/2 * slowclock)    /* debounce case */
     /* choose debounce filter and configure the delay
         according to the given argument value */
else                                          /* deglitch case */
     /* choose deglitch filter */


Slow clock is running at 32KHz which gives a 30 usec clock cycle.

>
> (this kind of thing is why I'm not convinced that generic pinconf works
> so well... What if we need psecs in the future?)

Should I keep the at91 native pinconf binding and add the missing flags 
to this binding
(OUTPUT configuration flags) ?

This was another question I asked in my cover letter: wether or not the 
generic pinconf
binding should be used.

Re: [RFC PATCH 1/3] pinctrl: add new generic pinconf config for deglitch filter

[Nicolas Ferre]

On 27/08/2013 08:16, boris brezillon :
> On 27/08/2013 05:55, Stephen Warren wrote:
>> On 08/26/2013 11:01 AM, boris brezillon wrote:
>>> Hello Stephen,
>>>
>>> On 26/08/2013 18:50, Stephen Warren wrote:
>>>> On 08/24/2013 03:35 PM, Boris BREZILLON wrote:
>>>>> Add a new parameter to support deglitch filter configuration.
>>>>> A deglitch filter works like a debounce filter but with a smaller
>>>>> delay (nanoseconds).
>>>> Why not use the existing debounce property, just with a small delay
>>>> specified. It seems like that's exactly what the property is for?
>>> That's one of the question I asked in my cover letter :-)
>>>
>>> Indeed the at91 deglitch filter delay is not configurable and is statically
>>> assigned to half a master clk cycle (if master clk = 133MHz -> 8 ns).
>>> The debounce property argument is currently expressed in usecs.
>>>
>>> This will result in always selecting the debounce filter (which is also
>>> available on at91 SoCs) over the deglitch filter.
>>>
>>> Could we add a flag in the deglitch argument to specify the delay unit
>>> (nsecs or usecs) ?
>> If the value is hard-coded in HW, why not use non-zero (or 1) to enable
>> and zero to disable?
>
> Indeed at91 pins support both deglitch and debounce filter and I have to
> choose
> between the two given the argument value (in usec).
>
> Here's what I can do:
>
> if (arg >= 1/2 * slowclock)    /* debounce case */
>       /* choose debounce filter and configure the delay
>           according to the given argument value */
> else                                          /* deglitch case */
>       /* choose deglitch filter */
>
>
> Slow clock is running at 32KHz which gives a 30 usec clock cycle.

I am not in favor for this kind of complicated heuristic. Deglitch and 
Debounce filters are different features in at91 (even if they pursuit 
the same goal). So I do prefer to let the user choose which feature is 
preferred for his application and add a different flag.


>> (this kind of thing is why I'm not convinced that generic pinconf works
>> so well... What if we need psecs in the future?)
>
> Should I keep the at91 native pinconf binding and add the missing flags
> to this binding
> (OUTPUT configuration flags) ?
>
> This was another question I asked in my cover letter: wether or not the
> generic pinconf
> binding should be used.

The question is: how much this "generic" pinconf is... well... generic! 
And it is not a answer I can give.
On the other hand, if the "generic" is not going to overcome the native 
pinctrl, I do not feel like switching to this at the cost of changing 
the whole dtsi/dts entries that we already have.

So, it is more to Linus and Stephen to give us clues about this...

Bye,
-- 
Nicolas Ferre

Re: [RFC PATCH 1/3] pinctrl: add new generic pinconf config for deglitch filter

[boris brezillon]

On 27/08/2013 09:42, Nicolas Ferre wrote:
> On 27/08/2013 08:16, boris brezillon :
>> On 27/08/2013 05:55, Stephen Warren wrote:
>>> On 08/26/2013 11:01 AM, boris brezillon wrote:
>>>> Hello Stephen,
>>>>
>>>> On 26/08/2013 18:50, Stephen Warren wrote:
>>>>> On 08/24/2013 03:35 PM, Boris BREZILLON wrote:
>>>>>> Add a new parameter to support deglitch filter configuration.
>>>>>> A deglitch filter works like a debounce filter but with a smaller
>>>>>> delay (nanoseconds).
>>>>> Why not use the existing debounce property, just with a small delay
>>>>> specified. It seems like that's exactly what the property is for?
>>>> That's one of the question I asked in my cover letter :-)
>>>>
>>>> Indeed the at91 deglitch filter delay is not configurable and is 
>>>> statically
>>>> assigned to half a master clk cycle (if master clk = 133MHz -> 8 ns).
>>>> The debounce property argument is currently expressed in usecs.
>>>>
>>>> This will result in always selecting the debounce filter (which is 
>>>> also
>>>> available on at91 SoCs) over the deglitch filter.
>>>>
>>>> Could we add a flag in the deglitch argument to specify the delay unit
>>>> (nsecs or usecs) ?
>>> If the value is hard-coded in HW, why not use non-zero (or 1) to enable
>>> and zero to disable?
>>
>> Indeed at91 pins support both deglitch and debounce filter and I have to
>> choose
>> between the two given the argument value (in usec).
>>
>> Here's what I can do:
>>
>> if (arg >= 1/2 * slowclock)    /* debounce case */
>>       /* choose debounce filter and configure the delay
>>           according to the given argument value */
>> else                                          /* deglitch case */
>>       /* choose deglitch filter */
>>
>>
>> Slow clock is running at 32KHz which gives a 30 usec clock cycle.
>
> I am not in favor for this kind of complicated heuristic. Deglitch and 
> Debounce filters are different features in at91 (even if they pursuit 
> the same goal). So I do prefer to let the user choose which feature is 
> preferred for his application and add a different flag.
>
>
>>> (this kind of thing is why I'm not convinced that generic pinconf works
>>> so well... What if we need psecs in the future?)
>>
>> Should I keep the at91 native pinconf binding and add the missing flags
>> to this binding
>> (OUTPUT configuration flags) ?
>>
>> This was another question I asked in my cover letter: wether or not the
>> generic pinconf
>> binding should be used.
>
> The question is: how much this "generic" pinconf is... well... 
> generic! And it is not a answer I can give.
> On the other hand, if the "generic" is not going to overcome the 
> native pinctrl, I do not feel like switching to this at the cost of 
> changing the whole dtsi/dts entries that we already have.
>
> So, it is more to Linus and Stephen to give us clues about this...

Okay.

I'll propose a new patch series adding native support for OUTPUT 
configuration of at91 pins (add OUTPUT_HIGH/LOW),
and put this series in stand-by until a clear decision is made about 
generic pinconf.

Thanks,

Best Regards,

Boris
>
> Bye,

Re: [RFC PATCH 1/3] pinctrl: add new generic pinconf config for deglitch filter

[Stephen Warren]

On 08/27/2013 01:42 AM, Nicolas Ferre wrote:
> On 27/08/2013 08:16, boris brezillon :
>> On 27/08/2013 05:55, Stephen Warren wrote:
>>> On 08/26/2013 11:01 AM, boris brezillon wrote:
>>>> Hello Stephen,
>>>>
>>>> On 26/08/2013 18:50, Stephen Warren wrote:
>>>>> On 08/24/2013 03:35 PM, Boris BREZILLON wrote:
>>>>>> Add a new parameter to support deglitch filter configuration.
>>>>>> A deglitch filter works like a debounce filter but with a smaller
>>>>>> delay (nanoseconds).
>>>>> Why not use the existing debounce property, just with a small delay
>>>>> specified. It seems like that's exactly what the property is for?
>>>> That's one of the question I asked in my cover letter :-)
>>>>
>>>> Indeed the at91 deglitch filter delay is not configurable and is
>>>> statically
>>>> assigned to half a master clk cycle (if master clk = 133MHz -> 8 ns).
>>>> The debounce property argument is currently expressed in usecs.
>>>>
>>>> This will result in always selecting the debounce filter (which is also
>>>> available on at91 SoCs) over the deglitch filter.
>>>>
>>>> Could we add a flag in the deglitch argument to specify the delay unit
>>>> (nsecs or usecs) ?
>>> If the value is hard-coded in HW, why not use non-zero (or 1) to enable
>>> and zero to disable?
>>
>> Indeed at91 pins support both deglitch and debounce filter and I have to
>> choose
>> between the two given the argument value (in usec).
>>
>> Here's what I can do:
>>
>> if (arg >= 1/2 * slowclock)    /* debounce case */
>>       /* choose debounce filter and configure the delay
>>           according to the given argument value */
>> else                                          /* deglitch case */
>>       /* choose deglitch filter */
>>
>>
>> Slow clock is running at 32KHz which gives a 30 usec clock cycle.
> 
> I am not in favor for this kind of complicated heuristic. Deglitch and
> Debounce filters are different features in at91 (even if they pursuit
> the same goal). So I do prefer to let the user choose which feature is
> preferred for his application and add a different flag.
> 
> 
>>> (this kind of thing is why I'm not convinced that generic pinconf works
>>> so well... What if we need psecs in the future?)
>>
>> Should I keep the at91 native pinconf binding and add the missing flags
>> to this binding
>> (OUTPUT configuration flags) ?
>>
>> This was another question I asked in my cover letter: wether or not the
>> generic pinconf
>> binding should be used.
> 
> The question is: how much this "generic" pinconf is... well... generic!

This is why I don't really like the concept of generic pinconf; it ends
up being more: whoever defines something first imposes their SoCs'
viewpoint on that feature/property, and then everything else is declared
non-generic.

In many cases, I think bindings will need to add SoC-specific properties
beyond generic pinconf (or perhaps only use SoC-specific properties and
ignore generic pinconf).

In the case of deglitch here, perhaps that's the best answer.

Re: [RFC PATCH 1/3] pinctrl: add new generic pinconf config for deglitch filter

[Linus Walleij]

On Tue, Aug 27, 2013 at 11:33 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/27/2013 01:42 AM, Nicolas Ferre wrote:

>> The question is: how much this "generic" pinconf is... well... generic!
>
> This is why I don't really like the concept of generic pinconf; it ends
> up being more: whoever defines something first imposes their SoCs'
> viewpoint on that feature/property, and then everything else is declared
> non-generic.

I do not see it that way. For pinconf what we're dealing with is a very
small community of electrical engineers that produce the cell libraries
for the pads of these ASICs, and connect some of the control lines to
software-controlled registers and sometimes hard-code their
characteristics. Or a mix.

They are all learning from each other and reproducing the design
patterns of other engineers, much in the same way as software
engineers do. That is why everyone is implementing some things
like pull-up/pull-down/drive strength/schmitt-trigger etc.

We already have 7 drivers using GENERIC_PINCONF without
any ontological conflicts like this so even if the rest of the world
end up not using it we have already saved a few thousand lines
of code by not reimplementing this (including DT bindings and
parsing code) over and over again for each.

And given that pinctrl-single is one of these, I do hope and think
that the ACPI people are taking notice and in their case, since
standardized ACPI tables must describe all systems out there,
a top-down ten commandments type of pin config is necessary
for their specs. (My interpretation though.)

Yours,
Linus Walleij

Re: [RFC PATCH 1/3] pinctrl: add new generic pinconf config for deglitch filter

[Linus Walleij]

On Tue, Aug 27, 2013 at 9:42 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
> On 27/08/2013 08:16, boris brezillon :

> Deglitch and
> Debounce filters are different features in at91 (even if they pursuit the
> same goal). So I do prefer to let the user choose which feature is preferred
> for his application and add a different flag.

Electrically speaking debounce and deglitch are totally different things.

Debounce is for, well debouncing. To even out the effect of pressing
a button which due to mechanical characteristics create a sharp
series of spikes like that:
  _   _
_| |_| |_

I highly suspect that "deglitch" is either:

- A one-spike version of the above (in which case a custom config
  may be warranted) or

- What we usually call schmitt-trigger, i.e. handling of analog
  swing-in.

Can you describe exactly what the two features do, in electrical
terms?

> The question is: how much this "generic" pinconf is... well... generic! And
> it is not a answer I can give.

The generic configs are defined from electrical use-cases that
appear in practice and also have the character of appearing in
similar implementations in I/O cells of several vendors, to the point
that from a software perspective they are identical.

> On the other hand, if the "generic" is not going to overcome the native
> pinctrl, I do not feel like switching to this at the cost of changing the
> whole dtsi/dts entries that we already have.

The question we ask in this case is whether the electrical construction
is so fantastically unique and ingenious that no other ASIC pad
implementer sitting in his chamber working on I/O cells would
possibly ever come up with the same concept.

If that is true, then it warrants its own, custom binding.

If you ask some other randong cell implementer whether this is
something they would do, and they say "yeah I have that in the
next version of my cell library" then it is generic, because we will
see the same thing in other systems as time moves on.

Yours,
Linus Walleij

[RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[Boris BREZILLON]

Add support for generic pin configuration to pinctrl-at91 driver.

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
---
 .../bindings/pinctrl/atmel,at91-pinctrl.txt        |   43 +++-
 drivers/pinctrl/Kconfig                            |    2 +-
 drivers/pinctrl/pinctrl-at91.c                     |  265 ++++++++++++++++++--
 3 files changed, 289 insertions(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
index 7ccae49..7a7c0c4 100644
--- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
@@ -18,7 +18,9 @@ mode) this pin can work on and the 'config' configures various pad settings
 such as pull-up, multi drive, etc.
 
 Required properties for iomux controller:
-- compatible: "atmel,at91rm9200-pinctrl"
+- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".
+  Add "generic-pinconf" to the compatible string list to use the generic pin
+  configuration syntax.
 - atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
   configured in this periph mode. All the periph and bank need to be describe.
 
@@ -83,6 +85,11 @@ Required properties for pin configuration node:
   setting. The format is atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>.
   The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B...
   PIN_BANK 0 is pioA, PIN_BANK 1 is pioB...
+  Dependending on the presence of the "generic-pinconf" string in the
+  compatible property the 4th cell is:
+   * a phandle referencing a generic pin config node (refer to
+     pinctrl-bindings.txt)
+   * an integer defining the pin config (see the following description)
 
 Bits used for CONFIG:
 PULL_UP		(1 << 0): indicate this pin need a pull up.
@@ -132,6 +139,40 @@ pinctrl@fffff400 {
 	};
 };
 
+or
+
+pinctrl@fffff400 {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+	compatible = "atmel,at91rm9200-pinctrl", "generic-pinconf", "simple-bus";
+	reg = <0xfffff400 0x600>;
+
+	atmel,mux-mask = <
+	      /*    A         B     */
+	       0xffffffff 0xffc00c3b  /* pioA */
+	       0xffffffff 0x7fff3ccf  /* pioB */
+	       0xffffffff 0x007fffff  /* pioC */
+	      >;
+
+	pcfg_none: pcfg_none {
+		bias-disable;
+	};
+
+	pcfg_pull_up: pcfg_pull_up {
+		bias-pullup;
+	};
+
+	/* shared pinctrl settings */
+	dbgu {
+		pinctrl_dbgu: dbgu-0 {
+			atmel,pins =
+				<1 14 0x1 &pcfg_none		/* PB14 periph A */
+				 1 15 0x1 &pcfg_pull_up>;	/* PB15 periph A with pullup */
+		};
+	};
+};
+
 dbgu: serial@fffff200 {
 	compatible = "atmel,at91sam9260-usart";
 	reg = <0xfffff200 0x200>;
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index bdb1a87..55a4f2c 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -54,7 +54,7 @@ config PINCTRL_AT91
 	depends on OF
 	depends on ARCH_AT91
 	select PINMUX
-	select PINCONF
+	select GENERIC_PINCONF
 	help
 	  Say Y here to enable the at91 pinctrl driver
 
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 7cce066..1994dd2 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -23,6 +23,7 @@
 #include <linux/gpio.h>
 #include <linux/pinctrl/machine.h>
 #include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 /* Since we request GPIOs from ourself */
@@ -32,6 +33,7 @@
 #include <mach/at91_pio.h>
 
 #include "core.h"
+#include "pinconf.h"
 
 #define MAX_NB_GPIO_PER_BANK	32
 
@@ -85,6 +87,21 @@ enum at91_mux {
 	AT91_MUX_PERIPH_D = 4,
 };
 
+struct at91_generic_pinconf {
+	unsigned long	*configs;
+	unsigned int	nconfigs;
+};
+
+enum at91_pinconf_type {
+	AT91_PINCONF_NATIVE,
+	AT91_PINCONF_GENERIC,
+};
+
+union at91_pinconf {
+	unsigned long			native;
+	struct at91_generic_pinconf	generic;
+};
+
 /**
  * struct at91_pmx_pin - describes an At91 pin mux
  * @bank: the bank of the pin
@@ -93,10 +110,11 @@ enum at91_mux {
  * @conf: the configuration of the pin: PULL_UP, MULTIDRIVE etc...
  */
 struct at91_pmx_pin {
-	uint32_t	bank;
-	uint32_t	pin;
-	enum at91_mux	mux;
-	unsigned long	conf;
+	uint32_t		bank;
+	uint32_t		pin;
+	enum at91_mux		mux;
+	enum at91_pinconf_type	conftype;
+	union at91_pinconf	conf;
 };
 
 /**
@@ -278,8 +296,16 @@ static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
 		new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
 		new_map[i].data.configs.group_or_pin =
 				pin_get_name(pctldev, grp->pins[i]);
-		new_map[i].data.configs.configs = &grp->pins_conf[i].conf;
-		new_map[i].data.configs.num_configs = 1;
+		if (!pctldev->desc->confops->is_generic) {
+			new_map[i].data.configs.configs =
+				&grp->pins_conf[i].conf.native;
+			new_map[i].data.configs.num_configs = 1;
+		} else {
+			new_map[i].data.configs.configs =
+				grp->pins_conf[i].conf.generic.configs;
+			new_map[i].data.configs.num_configs =
+				grp->pins_conf[i].conf.generic.nconfigs;
+		}
 	}
 
 	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
@@ -325,7 +351,7 @@ static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
 
 static unsigned at91_mux_get_pullup(void __iomem *pio, unsigned pin)
 {
-	return (readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1;
+	return !((readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1);
 }
 
 static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, bool on)
@@ -407,6 +433,16 @@ static enum at91_mux at91_mux_get_periph(void __iomem *pio, unsigned mask)
 	return select + 1;
 }
 
+static bool at91_mux_get_output(void __iomem *pio, unsigned pin)
+{
+	return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
+}
+
+static void at91_mux_set_output(void __iomem *pio, unsigned mask, bool is_on)
+{
+	__raw_writel(mask, pio + (is_on ? PIO_OER : PIO_ODR));
+}
+
 static bool at91_mux_get_deglitch(void __iomem *pio, unsigned pin)
 {
 	return (__raw_readl(pio + PIO_IFSR) >> pin) & 0x1;
@@ -445,7 +481,7 @@ static void at91_mux_pio3_set_debounce(void __iomem *pio, unsigned mask,
 
 static bool at91_mux_pio3_get_pulldown(void __iomem *pio, unsigned pin)
 {
-	return (__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1;
+	return !((__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1);
 }
 
 static void at91_mux_pio3_set_pulldown(void __iomem *pio, unsigned mask, bool is_on)
@@ -492,12 +528,17 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
 static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin)
 {
 	if (pin->mux) {
-		dev_dbg(dev, "pio%c%d configured as periph%c with conf = 0x%lu\n",
-			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A', pin->conf);
+		dev_dbg(dev, "pio%c%d configured as periph%c",
+			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A');
 	} else {
-		dev_dbg(dev, "pio%c%d configured as gpio with conf = 0x%lu\n",
-			pin->bank + 'A', pin->pin, pin->conf);
+		dev_dbg(dev, "pio%c%d configured as gpio",
+			pin->bank + 'A', pin->pin);
 	}
+
+	if (pin->conftype == AT91_PINCONF_NATIVE)
+		dev_dbg(dev, " with conf = 0x%lu\n", pin->conf.native);
+	else
+		dev_dbg(dev, "\n");
 }
 
 static int pin_check_config(struct at91_pinctrl *info, const char *name,
@@ -782,6 +823,157 @@ static const struct pinconf_ops at91_pinconf_ops = {
 	.pin_config_group_dbg_show	= at91_pinconf_group_dbg_show,
 };
 
+static int at91_generic_pinconf_get(struct pinctrl_dev *pctldev,
+				    unsigned pin_id, unsigned long *config)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param param = pinconf_to_config_param(*config);
+	void __iomem *pio = pin_to_controller(info, pin_to_bank(pin_id));
+	unsigned pin = pin_id % MAX_NB_GPIO_PER_BANK;
+	int div;
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		if (at91_mux_get_pullup(pio, pin) &&
+		    (info->ops->get_pulldown ||
+		     !info->ops->get_pulldown(pio, pin)))
+			return -EINVAL;
+
+		*config = 0;
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		if (!at91_mux_get_pullup(pio, pin))
+			return -EINVAL;
+
+		*config = 0;
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		if (!info->ops->get_pulldown)
+			return -ENOTSUPP;
+		if (!info->ops->get_pulldown(pio, pin))
+			return -EINVAL;
+
+		*config = 0;
+		break;
+	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+		if (!at91_mux_get_multidrive(pio, pin))
+			return -EINVAL;
+
+		*config = 0;
+		break;
+	case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+		if (!info->ops->get_schmitt_trig)
+			return -ENOTSUPP;
+
+		if (!(info->ops->get_schmitt_trig(pio, pin)))
+			*config = 1;
+		else
+			*config = 0;
+		break;
+	case PIN_CONFIG_INPUT_DEBOUNCE:
+		if (!info->ops->get_debounce)
+			return -ENOTSUPP;
+
+		if (info->ops->get_debounce(pio, pin, &div)) {
+			/* TODO: replace 32768 with clk_get_rate(slck) return */
+			*config = ((div + 1) * 2) * 1000000 / 32768;
+			if (*config > 0xffff)
+				*config = 0xffff;
+		} else
+			*config = 0;
+		break;
+	case PIN_CONFIG_INPUT_DEGLITCH:
+		if (!info->ops->get_deglitch)
+			return -ENOTSUPP;
+
+		*config = info->ops->get_deglitch(pio, pin);
+		break;
+	case PIN_CONFIG_OUTPUT:
+		if (info->ops->get_periph(pio, pin) != AT91_MUX_GPIO)
+			return -EINVAL;
+
+		*config = at91_mux_get_output(pio, pin);
+		break;
+	default:
+		return -ENOTSUPP;
+		break;
+	}
+
+	return 0;
+}
+
+static int at91_generic_pinconf_set(struct pinctrl_dev *pctldev,
+				    unsigned pin_id, unsigned long config)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param param = pinconf_to_config_param(config);
+	u16 arg = pinconf_to_config_argument(config);
+	u32 div = 0;
+	unsigned pin = pin_id % MAX_NB_GPIO_PER_BANK;
+	unsigned mask = pin_to_mask(pin);
+	void __iomem *pio = pin_to_controller(info, pin_to_bank(pin_id));
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		at91_mux_set_pullup(pio, mask, 0);
+		if (info->ops->set_pulldown)
+			info->ops->set_pulldown(pio, mask, 0);
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		at91_mux_set_pullup(pio, mask, arg);
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		if (!info->ops->set_pulldown)
+			return -ENOTSUPP;
+		info->ops->set_pulldown(pio, mask, arg);
+		break;
+	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+		at91_mux_set_multidrive(pio, mask, 1);
+		break;
+	case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+		if (!info->ops->disable_schmitt_trig)
+			return -ENOTSUPP;
+		if (arg)
+			mask = ~mask;
+		info->ops->disable_schmitt_trig(pio, mask);
+		break;
+	case PIN_CONFIG_INPUT_DEBOUNCE:
+		if (!info->ops->set_debounce)
+			return -ENOTSUPP;
+
+		/* TODO: replace 32768 with clk_get_rate(slck) return */
+		if (arg) {
+			div = (arg * 32768 / (2 * 1000000));
+			if (div)
+				div--;
+		}
+		info->ops->set_debounce(pio, mask, arg ? 1 : 0, div);
+		break;
+	case PIN_CONFIG_INPUT_DEGLITCH:
+		if (!info->ops->set_deglitch)
+			return -ENOTSUPP;
+
+		info->ops->set_deglitch(pio, mask, arg ? 1 : 0);
+		break;
+	case PIN_CONFIG_OUTPUT:
+		if (info->ops->get_periph(pio, pin) != AT91_MUX_GPIO)
+			return -EINVAL;
+		at91_mux_set_output(pio, mask, arg);
+		break;
+	default:
+		return -ENOTSUPP;
+		break;
+	}
+
+	return 0;
+}
+
+static const struct pinconf_ops at91_generic_pinconf_ops = {
+	.is_generic			= true,
+	.pin_config_get			= at91_generic_pinconf_get,
+	.pin_config_set			= at91_generic_pinconf_set,
+};
+
 static struct pinctrl_desc at91_pinctrl_desc = {
 	.pctlops	= &at91_pctrl_ops,
 	.pmxops		= &at91_pmx_ops,
@@ -847,6 +1039,7 @@ static int at91_pinctrl_parse_groups(struct device_node *np,
 	int size;
 	const __be32 *list;
 	int i, j;
+	int err;
 
 	dev_dbg(info->dev, "group(%d): %s\n", index, np->name);
 
@@ -870,21 +1063,48 @@ static int at91_pinctrl_parse_groups(struct device_node *np,
 				GFP_KERNEL);
 	grp->pins = devm_kzalloc(info->dev, grp->npins * sizeof(unsigned int),
 				GFP_KERNEL);
-	if (!grp->pins_conf || !grp->pins)
-		return -ENOMEM;
+	if (!grp->pins_conf || !grp->pins) {
+		err = -ENOMEM;
+		goto out_err;
+	}
 
 	for (i = 0, j = 0; i < size; i += 4, j++) {
 		pin->bank = be32_to_cpu(*list++);
 		pin->pin = be32_to_cpu(*list++);
 		grp->pins[j] = pin->bank * MAX_NB_GPIO_PER_BANK + pin->pin;
 		pin->mux = be32_to_cpu(*list++);
-		pin->conf = be32_to_cpu(*list++);
+		if (at91_pinctrl_desc.confops->is_generic) {
+			struct device_node *np_config;
+			const __be32 *phandle =	list++;
+
+			if (!phandle) {
+				err = -EINVAL;
+				goto out_err;
+			}
+			np_config =
+				of_find_node_by_phandle(be32_to_cpup(phandle));
+			pin->conftype = AT91_PINCONF_GENERIC;
+			err = pinconf_generic_parse_dt_config(np_config,
+						&pin->conf.generic.configs,
+						&pin->conf.generic.nconfigs);
+			if (err)
+				goto out_err;
+
+		} else {
+			pin->conftype = AT91_PINCONF_NATIVE;
+			pin->conf.native = be32_to_cpu(*list++);
+		}
 
 		at91_pin_dbg(info->dev, pin);
 		pin++;
 	}
 
 	return 0;
+
+out_err:
+	kfree(grp->pins_conf);
+	kfree(grp->pins);
+	return err;
 }
 
 static int at91_pinctrl_parse_functions(struct device_node *np,
@@ -904,10 +1124,10 @@ static int at91_pinctrl_parse_functions(struct device_node *np,
 	/* Initialise function */
 	func->name = np->name;
 	func->ngroups = of_get_child_count(np);
-	if (func->ngroups <= 0) {
-		dev_err(info->dev, "no groups defined\n");
-		return -EINVAL;
-	}
+	/* This node might be a generic config definition: silently ignore it */
+	if (func->ngroups <= 0)
+		return 0;
+
 	func->groups = devm_kzalloc(info->dev,
 			func->ngroups * sizeof(char *), GFP_KERNEL);
 	if (!func->groups)
@@ -930,6 +1150,11 @@ static struct of_device_id at91_pinctrl_of_match[] = {
 	{ /* sentinel */ }
 };
 
+static struct of_device_id at91_pinconf_of_match[] = {
+	{ .compatible = "generic-pinconf" },
+	{ /* sentinel */ }
+};
+
 static int at91_pinctrl_probe_dt(struct platform_device *pdev,
 				 struct at91_pinctrl *info)
 {
@@ -945,6 +1170,8 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
 	info->dev = &pdev->dev;
 	info->ops = (struct at91_pinctrl_mux_ops *)
 		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+	if (of_match_device(at91_pinconf_of_match, &pdev->dev))
+		at91_pinctrl_desc.confops = &at91_generic_pinconf_ops;
 	at91_pinctrl_child_count(info, np);
 
 	if (info->nbanks < 1) {
-- 
1.7.9.5

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[Stephen Warren]

On 08/24/2013 03:37 PM, Boris BREZILLON wrote:
> Add support for generic pin configuration to pinctrl-at91 driver.

> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt

>  Required properties for iomux controller:
> -- compatible: "atmel,at91rm9200-pinctrl"
> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".

You seem to also be adding a second chip name to the list here, which is
more than the patch subject/description imply you're doing...

> +  Add "generic-pinconf" to the compatible string list to use the generic pin
> +  configuration syntax.

"generic-pinconf" is too generic of a compatible value for this binding
to define.

Instead, I think you want to either:

a)

Use compatible="atmel,at91rm9200-pinctrl" for the old binding,
use compatible="atmel,at91rm9200-pinctrl-generic" for the new binding

or:

b)

Define Boolean property atmel,generic-pinconf (perhaps a better name
could be chosen?). If it's not present, parse the node assuming the old
binding. If it is present, parse the node assuming the new binding.

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[boris brezillon]

On 26/08/2013 18:53, Stephen Warren wrote:
> On 08/24/2013 03:37 PM, Boris BREZILLON wrote:
>> Add support for generic pin configuration to pinctrl-at91 driver.
>> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>   Required properties for iomux controller:
>> -- compatible: "atmel,at91rm9200-pinctrl"
>> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".
> You seem to also be adding a second chip name to the list here, which is
> more than the patch subject/description imply you're doing...

This is an update of the documentation:
"atmel,at91sam9x5-pinctrl" compatible is already used in the pinctrl 
driver but the documention
was not updated.

But I agree, this should not be part of this series.

>> +  Add "generic-pinconf" to the compatible string list to use the generic pin
>> +  configuration syntax.
> "generic-pinconf" is too generic of a compatible value for this binding
> to define.
>
> Instead, I think you want to either:
>
> a)
>
> Use compatible="atmel,at91rm9200-pinctrl" for the old binding,
> use compatible="atmel,at91rm9200-pinctrl-generic" for the new binding
>
> or:
>
> b)
>
> Define Boolean property atmel,generic-pinconf (perhaps a better name
> could be chosen?). If it's not present, parse the node assuming the old
> binding. If it is present, parse the node assuming the new binding.
>
Okay.

I thought this property string could be generic as it may concern other 
drivers too
(in order to keep compatibility with old dt ABI and add support the 
generic pinconf binding).

Anyway, I prefer the first proposition.

pinctrl single driver is already using these names:

|compatible = "pinctrl-single" for non generic pinconf binding
||compatible = "pinconf-single" ||for generic pinconf binding|

So I think we should use something similar:

|compatible = "atmel,at91xx-pinctrl" for non generic pinconf binding
||compatible = "|||atmel,at91xx-|pinconf" ||for generic pinconf binding|

What do you think ?

Best Regards,

Boris

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[Stephen Warren]

On 08/26/2013 11:17 AM, boris brezillon wrote:
> On 26/08/2013 18:53, Stephen Warren wrote:
>> On 08/24/2013 03:37 PM, Boris BREZILLON wrote:
>>> Add support for generic pin configuration to pinctrl-at91 driver.
>>> diff --git
>>> a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>> b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>   Required properties for iomux controller:
>>> -- compatible: "atmel,at91rm9200-pinctrl"
>>> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".
>> You seem to also be adding a second chip name to the list here, which is
>> more than the patch subject/description imply you're doing...
> 
> This is an update of the documentation:
> "atmel,at91sam9x5-pinctrl" compatible is already used in the pinctrl
> driver but the documention
> was not updated.
> 
> But I agree, this should not be part of this series.
> 
>>> +  Add "generic-pinconf" to the compatible string list to use the
>>> generic pin
>>> +  configuration syntax.
>> "generic-pinconf" is too generic of a compatible value for this binding
>> to define.
>>
>> Instead, I think you want to either:
>>
>> a)
>>
>> Use compatible="atmel,at91rm9200-pinctrl" for the old binding,
>> use compatible="atmel,at91rm9200-pinctrl-generic" for the new binding
>>
>> or:
>>
>> b)
>>
>> Define Boolean property atmel,generic-pinconf (perhaps a better name
>> could be chosen?). If it's not present, parse the node assuming the old
>> binding. If it is present, parse the node assuming the new binding.
>>
> Okay.
> 
> I thought this property string could be generic as it may concern other
> drivers too
> (in order to keep compatibility with old dt ABI and add support the
> generic pinconf binding).
> 
> Anyway, I prefer the first proposition.
> 
> pinctrl single driver is already using these names:
> 
> |compatible = "pinctrl-single" for non generic pinconf binding
> ||compatible = "pinconf-single" ||for generic pinconf binding|
> 
> So I think we should use something similar:
> 
> |compatible = "atmel,at91xx-pinctrl" for non generic pinconf binding
> ||compatible = "|||atmel,at91xx-|pinconf" ||for generic pinconf binding|
> 
> What do you think ?

Hmmm. It is a little odd to switch out the compatible value and invent a
new binding for the same HW. Isn't it possible to define both sets of
properties in the binding, and have drivers look for either?

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[boris brezillon]

On 27/08/2013 05:57, Stephen Warren wrote:
> On 08/26/2013 11:17 AM, boris brezillon wrote:
>> On 26/08/2013 18:53, Stephen Warren wrote:
>>> On 08/24/2013 03:37 PM, Boris BREZILLON wrote:
>>>> Add support for generic pin configuration to pinctrl-at91 driver.
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>> b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>>    Required properties for iomux controller:
>>>> -- compatible: "atmel,at91rm9200-pinctrl"
>>>> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".
>>> You seem to also be adding a second chip name to the list here, which is
>>> more than the patch subject/description imply you're doing...
>> This is an update of the documentation:
>> "atmel,at91sam9x5-pinctrl" compatible is already used in the pinctrl
>> driver but the documention
>> was not updated.
>>
>> But I agree, this should not be part of this series.
>>
>>>> +  Add "generic-pinconf" to the compatible string list to use the
>>>> generic pin
>>>> +  configuration syntax.
>>> "generic-pinconf" is too generic of a compatible value for this binding
>>> to define.
>>>
>>> Instead, I think you want to either:
>>>
>>> a)
>>>
>>> Use compatible="atmel,at91rm9200-pinctrl" for the old binding,
>>> use compatible="atmel,at91rm9200-pinctrl-generic" for the new binding
>>>
>>> or:
>>>
>>> b)
>>>
>>> Define Boolean property atmel,generic-pinconf (perhaps a better name
>>> could be chosen?). If it's not present, parse the node assuming the old
>>> binding. If it is present, parse the node assuming the new binding.
>>>
>> Okay.
>>
>> I thought this property string could be generic as it may concern other
>> drivers too
>> (in order to keep compatibility with old dt ABI and add support the
>> generic pinconf binding).
>>
>> Anyway, I prefer the first proposition.
>>
>> pinctrl single driver is already using these names:
>>
>> |compatible = "pinctrl-single" for non generic pinconf binding
>> ||compatible = "pinconf-single" ||for generic pinconf binding|
>>
>> So I think we should use something similar:
>>
>> |compatible = "atmel,at91xx-pinctrl" for non generic pinconf binding
>> ||compatible = "|||atmel,at91xx-|pinconf" ||for generic pinconf binding|
>>
>> What do you think ?
> Hmmm. It is a little odd to switch out the compatible value and invent a
> new binding for the same HW. Isn't it possible to define both sets of
> properties in the binding, and have drivers look for either?
>

Do you mean something like:

atmel,pins = <xxx>;                /* current dt binding */
atmel,generic-pins = <yyy>;    /* new dt binding */

If that's what you had in mind, it will be a little bit tricky to 
handle, because AFAIK the pinconf_ops
callbacks do not give me any element I could use to deduce the type of 
pinconf (generic or
native).
This implies I have to know early during the probe process which kind of 
binding is in use.

Please tell me if I missed some key points, and this can be easily done.

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[Stephen Warren]

On 08/27/2013 12:40 AM, boris brezillon wrote:
> On 27/08/2013 05:57, Stephen Warren wrote:
>> On 08/26/2013 11:17 AM, boris brezillon wrote:
>>> On 26/08/2013 18:53, Stephen Warren wrote:
>>>> On 08/24/2013 03:37 PM, Boris BREZILLON wrote:
>>>>> Add support for generic pin configuration to pinctrl-at91 driver.
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>>> b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>>>    Required properties for iomux controller:
>>>>> -- compatible: "atmel,at91rm9200-pinctrl"
>>>>> +- compatible: "atmel,at91rm9200-pinctrl" or
>>>>> "atmel,at91sam9x5-pinctrl".
>>>> You seem to also be adding a second chip name to the list here,
>>>> which is
>>>> more than the patch subject/description imply you're doing...
>>> This is an update of the documentation:
>>> "atmel,at91sam9x5-pinctrl" compatible is already used in the pinctrl
>>> driver but the documention
>>> was not updated.
>>>
>>> But I agree, this should not be part of this series.
>>>
>>>>> +  Add "generic-pinconf" to the compatible string list to use the
>>>>> generic pin
>>>>> +  configuration syntax.
>>>> "generic-pinconf" is too generic of a compatible value for this binding
>>>> to define.
>>>>
>>>> Instead, I think you want to either:
>>>>
>>>> a)
>>>>
>>>> Use compatible="atmel,at91rm9200-pinctrl" for the old binding,
>>>> use compatible="atmel,at91rm9200-pinctrl-generic" for the new binding
>>>>
>>>> or:
>>>>
>>>> b)
>>>>
>>>> Define Boolean property atmel,generic-pinconf (perhaps a better name
>>>> could be chosen?). If it's not present, parse the node assuming the old
>>>> binding. If it is present, parse the node assuming the new binding.
>>>>
>>> Okay.
>>>
>>> I thought this property string could be generic as it may concern other
>>> drivers too
>>> (in order to keep compatibility with old dt ABI and add support the
>>> generic pinconf binding).
>>>
>>> Anyway, I prefer the first proposition.
>>>
>>> pinctrl single driver is already using these names:
>>>
>>> |compatible = "pinctrl-single" for non generic pinconf binding
>>> ||compatible = "pinconf-single" ||for generic pinconf binding|
>>>
>>> So I think we should use something similar:
>>>
>>> |compatible = "atmel,at91xx-pinctrl" for non generic pinconf binding
>>> ||compatible = "|||atmel,at91xx-|pinconf" ||for generic pinconf binding|
>>>
>>> What do you think ?
>> Hmmm. It is a little odd to switch out the compatible value and invent a
>> new binding for the same HW. Isn't it possible to define both sets of
>> properties in the binding, and have drivers look for either?
>>
> 
> Do you mean something like:
> 
> atmel,pins = <xxx>;                /* current dt binding */
> atmel,generic-pins = <yyy>;    /* new dt binding */
> 
> If that's what you had in mind, it will be a little bit tricky to
> handle, because AFAIK the pinconf_ops
> callbacks do not give me any element I could use to deduce the type of
> pinconf (generic or
> native).
> This implies I have to know early during the probe process which kind of
> binding is in use.
> 
> Please tell me if I missed some key points, and this can be easily done.

It's probably most compatible to keep all the existing properties, and
just add new properties for new features.

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[Jean-Christophe PLAGNIOL-VILLARD]

On 23:37 Sat 24 Aug     , Boris BREZILLON wrote:
> Add support for generic pin configuration to pinctrl-at91 driver.
> 
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> ---
>  .../bindings/pinctrl/atmel,at91-pinctrl.txt        |   43 +++-
>  drivers/pinctrl/Kconfig                            |    2 +-
>  drivers/pinctrl/pinctrl-at91.c                     |  265 ++++++++++++++++++--
>  3 files changed, 289 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> index 7ccae49..7a7c0c4 100644
> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> @@ -18,7 +18,9 @@ mode) this pin can work on and the 'config' configures various pad settings
>  such as pull-up, multi drive, etc.
>  
>  Required properties for iomux controller:
> -- compatible: "atmel,at91rm9200-pinctrl"
> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".
> +  Add "generic-pinconf" to the compatible string list to use the generic pin
> +  configuration syntax.
>  - atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
>    configured in this periph mode. All the periph and bank need to be describe.
>  
> @@ -83,6 +85,11 @@ Required properties for pin configuration node:
>    setting. The format is atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>.
>    The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B...
>    PIN_BANK 0 is pioA, PIN_BANK 1 is pioB...
> +  Dependending on the presence of the "generic-pinconf" string in the
> +  compatible property the 4th cell is:
> +   * a phandle referencing a generic pin config node (refer to
> +     pinctrl-bindings.txt)
> +   * an integer defining the pin config (see the following description)
>  
>  Bits used for CONFIG:
>  PULL_UP		(1 << 0): indicate this pin need a pull up.
> @@ -132,6 +139,40 @@ pinctrl@fffff400 {
>  	};
>  };
>  
> +or
> +
> +pinctrl@fffff400 {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	ranges;
> +	compatible = "atmel,at91rm9200-pinctrl", "generic-pinconf", "simple-bus";
nack your break the backword compatibility

if we use a old kernel with this new dt nothing will work
as the old kernel will never known the the "generic-pinconf" means anything

if we want to use generic-pinconf support you *CAN NOT* use atmel,at91rm9200-pinctrl
in the compatible
> +	reg = <0xfffff400 0x600>;
> +
> +	atmel,mux-mask = <
> +	      /*    A         B     */
> +	       0xffffffff 0xffc00c3b  /* pioA */
> +	       0xffffffff 0x7fff3ccf  /* pioB */
> +	       0xffffffff 0x007fffff  /* pioC */
> +	      >;
> +
> +	pcfg_none: pcfg_none {
> +		bias-disable;
> +	};
> +
> +	pcfg_pull_up: pcfg_pull_up {
> +		bias-pullup;
> +	};
> +
> +	/* shared pinctrl settings */
> +	dbgu {
> +		pinctrl_dbgu: dbgu-0 {
> +			atmel,pins =
> +				<1 14 0x1 &pcfg_none		/* PB14 periph A */
> +				 1 15 0x1 &pcfg_pull_up>;	/* PB15 periph A with pullup */
> +		};
> +	};
> +};
> +
>  dbgu: serial@fffff200 {
>  	compatible = "atmel,at91sam9260-usart";
>  	reg = <0xfffff200 0x200>;
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index bdb1a87..55a4f2c 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -54,7 +54,7 @@ config PINCTRL_AT91
>  	depends on OF
>  	depends on ARCH_AT91
>  	select PINMUX
> -	select PINCONF
> +	select GENERIC_PINCONF
>  	help
>  	  Say Y here to enable the at91 pinctrl driver
>  
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 7cce066..1994dd2 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -23,6 +23,7 @@
>  #include <linux/gpio.h>
>  #include <linux/pinctrl/machine.h>
>  #include <linux/pinctrl/pinconf.h>
> +#include <linux/pinctrl/pinconf-generic.h>
>  #include <linux/pinctrl/pinctrl.h>
>  #include <linux/pinctrl/pinmux.h>
>  /* Since we request GPIOs from ourself */
> @@ -32,6 +33,7 @@
>  #include <mach/at91_pio.h>
>  
>  #include "core.h"
> +#include "pinconf.h"
>  
>  #define MAX_NB_GPIO_PER_BANK	32
>  
> @@ -85,6 +87,21 @@ enum at91_mux {
>  	AT91_MUX_PERIPH_D = 4,
>  };
>  
> +struct at91_generic_pinconf {
> +	unsigned long	*configs;
> +	unsigned int	nconfigs;
> +};
> +
> +enum at91_pinconf_type {
> +	AT91_PINCONF_NATIVE,
> +	AT91_PINCONF_GENERIC,
> +};
> +
> +union at91_pinconf {
> +	unsigned long			native;
> +	struct at91_generic_pinconf	generic;
> +};
> +
>  /**
>   * struct at91_pmx_pin - describes an At91 pin mux
>   * @bank: the bank of the pin
> @@ -93,10 +110,11 @@ enum at91_mux {
>   * @conf: the configuration of the pin: PULL_UP, MULTIDRIVE etc...
>   */
>  struct at91_pmx_pin {
> -	uint32_t	bank;
> -	uint32_t	pin;
> -	enum at91_mux	mux;
> -	unsigned long	conf;
> +	uint32_t		bank;
> +	uint32_t		pin;
> +	enum at91_mux		mux;
> +	enum at91_pinconf_type	conftype;
> +	union at91_pinconf	conf;
>  };
>  
>  /**
> @@ -278,8 +296,16 @@ static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
>  		new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
>  		new_map[i].data.configs.group_or_pin =
>  				pin_get_name(pctldev, grp->pins[i]);
> -		new_map[i].data.configs.configs = &grp->pins_conf[i].conf;
> -		new_map[i].data.configs.num_configs = 1;
> +		if (!pctldev->desc->confops->is_generic) {
> +			new_map[i].data.configs.configs =
> +				&grp->pins_conf[i].conf.native;
> +			new_map[i].data.configs.num_configs = 1;
> +		} else {
> +			new_map[i].data.configs.configs =
> +				grp->pins_conf[i].conf.generic.configs;
> +			new_map[i].data.configs.num_configs =
> +				grp->pins_conf[i].conf.generic.nconfigs;
> +		}
>  	}
>  
>  	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
> @@ -325,7 +351,7 @@ static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
>  
>  static unsigned at91_mux_get_pullup(void __iomem *pio, unsigned pin)
>  {
> -	return (readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1;
> +	return !((readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1);
>  }
>  
>  static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, bool on)
> @@ -407,6 +433,16 @@ static enum at91_mux at91_mux_get_periph(void __iomem *pio, unsigned mask)
>  	return select + 1;
>  }
>  
> +static bool at91_mux_get_output(void __iomem *pio, unsigned pin)
> +{
> +	return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
> +}
> +
> +static void at91_mux_set_output(void __iomem *pio, unsigned mask, bool is_on)
> +{
> +	__raw_writel(mask, pio + (is_on ? PIO_OER : PIO_ODR));
> +}
> +
>  static bool at91_mux_get_deglitch(void __iomem *pio, unsigned pin)
>  {
>  	return (__raw_readl(pio + PIO_IFSR) >> pin) & 0x1;
> @@ -445,7 +481,7 @@ static void at91_mux_pio3_set_debounce(void __iomem *pio, unsigned mask,
>  
>  static bool at91_mux_pio3_get_pulldown(void __iomem *pio, unsigned pin)
>  {
> -	return (__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1;
> +	return !((__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1);
>  }
>  
>  static void at91_mux_pio3_set_pulldown(void __iomem *pio, unsigned mask, bool is_on)
> @@ -492,12 +528,17 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
>  static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin)
>  {
>  	if (pin->mux) {
> -		dev_dbg(dev, "pio%c%d configured as periph%c with conf = 0x%lu\n",
> -			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A', pin->conf);
> +		dev_dbg(dev, "pio%c%d configured as periph%c",
> +			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A');
>  	} else {
> -		dev_dbg(dev, "pio%c%d configured as gpio with conf = 0x%lu\n",
> -			pin->bank + 'A', pin->pin, pin->conf);
> +		dev_dbg(dev, "pio%c%d configured as gpio",
> +			pin->bank + 'A', pin->pin);
>  	}
> +
> +	if (pin->conftype == AT91_PINCONF_NATIVE)
> +		dev_dbg(dev, " with conf = 0x%lu\n", pin->conf.native);
> +	else
> +		dev_dbg(dev, "\n");

do not change debug output
>  }
>  
>  static int pin_check_config(struct at91_pinctrl *info, const char *name,
> @@ -782,6 +823,157 @@ static const struct pinconf_ops at91_pinconf_ops = {
>  	.pin_config_group_dbg_show	= at91_pinconf_group_dbg_show,
>  };
>  
> +static int at91_generic_pinconf_get(struct pinctrl_dev *pctldev,
> +				    unsigned pin_id, unsigned long *config)
> +{
> +	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +	enum pin_config_param param = pinconf_to_config_param(*config);
> +	void __iomem *pio = pin_to_controller(info, pin_to_bank(pin_id));
> +	unsigned pin = pin_id % MAX_NB_GPIO_PER_BANK;
> +	int div;
> +
> +	switch (param) {
> +	case PIN_CONFIG_BIAS_DISABLE:
> +		if (at91_mux_get_pullup(pio, pin) &&
> +		    (info->ops->get_pulldown ||
> +		     !info->ops->get_pulldown(pio, pin)))
> +			return -EINVAL;
> +
> +		*config = 0;
> +		break;
> +	case PIN_CONFIG_BIAS_PULL_UP:
> +		if (!at91_mux_get_pullup(pio, pin))
> +			return -EINVAL;
> +
> +		*config = 0;
> +		break;
> +	case PIN_CONFIG_BIAS_PULL_DOWN:
> +		if (!info->ops->get_pulldown)
> +			return -ENOTSUPP;
> +		if (!info->ops->get_pulldown(pio, pin))
> +			return -EINVAL;
> +
> +		*config = 0;
> +		break;
> +	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> +		if (!at91_mux_get_multidrive(pio, pin))
> +			return -EINVAL;
> +
> +		*config = 0;
> +		break;
> +	case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
> +		if (!info->ops->get_schmitt_trig)
> +			return -ENOTSUPP;
> +
> +		if (!(info->ops->get_schmitt_trig(pio, pin)))
> +			*config = 1;
> +		else
> +			*config = 0;
> +		break;
> +	case PIN_CONFIG_INPUT_DEBOUNCE:
> +		if (!info->ops->get_debounce)
> +			return -ENOTSUPP;
> +
> +		if (info->ops->get_debounce(pio, pin, &div)) {
> +			/* TODO: replace 32768 with clk_get_rate(slck) return */
> +			*config = ((div + 1) * 2) * 1000000 / 32768;
> +			if (*config > 0xffff)
> +				*config = 0xffff;
> +		} else
> +			*config = 0;
> +		break;
> +	case PIN_CONFIG_INPUT_DEGLITCH:
> +		if (!info->ops->get_deglitch)
> +			return -ENOTSUPP;
> +
> +		*config = info->ops->get_deglitch(pio, pin);
> +		break;
> +	case PIN_CONFIG_OUTPUT:
> +		if (info->ops->get_periph(pio, pin) != AT91_MUX_GPIO)
> +			return -EINVAL;
> +
> +		*config = at91_mux_get_output(pio, pin);
> +		break;
> +	default:
> +		return -ENOTSUPP;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int at91_generic_pinconf_set(struct pinctrl_dev *pctldev,
> +				    unsigned pin_id, unsigned long config)
> +{
> +	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +	enum pin_config_param param = pinconf_to_config_param(config);
> +	u16 arg = pinconf_to_config_argument(config);
> +	u32 div = 0;
> +	unsigned pin = pin_id % MAX_NB_GPIO_PER_BANK;
> +	unsigned mask = pin_to_mask(pin);
> +	void __iomem *pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> +	switch (param) {
> +	case PIN_CONFIG_BIAS_DISABLE:
> +		at91_mux_set_pullup(pio, mask, 0);
> +		if (info->ops->set_pulldown)
> +			info->ops->set_pulldown(pio, mask, 0);
> +		break;
> +	case PIN_CONFIG_BIAS_PULL_UP:
> +		at91_mux_set_pullup(pio, mask, arg);
> +		break;
> +	case PIN_CONFIG_BIAS_PULL_DOWN:
> +		if (!info->ops->set_pulldown)
> +			return -ENOTSUPP;
> +		info->ops->set_pulldown(pio, mask, arg);
> +		break;
> +	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> +		at91_mux_set_multidrive(pio, mask, 1);
> +		break;
> +	case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
> +		if (!info->ops->disable_schmitt_trig)
> +			return -ENOTSUPP;
> +		if (arg)
> +			mask = ~mask;
> +		info->ops->disable_schmitt_trig(pio, mask);
> +		break;
> +	case PIN_CONFIG_INPUT_DEBOUNCE:
> +		if (!info->ops->set_debounce)
> +			return -ENOTSUPP;
> +
> +		/* TODO: replace 32768 with clk_get_rate(slck) return */
> +		if (arg) {
> +			div = (arg * 32768 / (2 * 1000000));
> +			if (div)
> +				div--;
> +		}
> +		info->ops->set_debounce(pio, mask, arg ? 1 : 0, div);
> +		break;
> +	case PIN_CONFIG_INPUT_DEGLITCH:
> +		if (!info->ops->set_deglitch)
> +			return -ENOTSUPP;
> +
> +		info->ops->set_deglitch(pio, mask, arg ? 1 : 0);
> +		break;
> +	case PIN_CONFIG_OUTPUT:
> +		if (info->ops->get_periph(pio, pin) != AT91_MUX_GPIO)
> +			return -EINVAL;
> +		at91_mux_set_output(pio, mask, arg);
> +		break;
> +	default:
> +		return -ENOTSUPP;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct pinconf_ops at91_generic_pinconf_ops = {
> +	.is_generic			= true,
> +	.pin_config_get			= at91_generic_pinconf_get,
> +	.pin_config_set			= at91_generic_pinconf_set,
> +};
> +
>  static struct pinctrl_desc at91_pinctrl_desc = {
>  	.pctlops	= &at91_pctrl_ops,
>  	.pmxops		= &at91_pmx_ops,
> @@ -847,6 +1039,7 @@ static int at91_pinctrl_parse_groups(struct device_node *np,
>  	int size;
>  	const __be32 *list;
>  	int i, j;
> +	int err;
>  
>  	dev_dbg(info->dev, "group(%d): %s\n", index, np->name);
>  
> @@ -870,21 +1063,48 @@ static int at91_pinctrl_parse_groups(struct device_node *np,
>  				GFP_KERNEL);
>  	grp->pins = devm_kzalloc(info->dev, grp->npins * sizeof(unsigned int),
>  				GFP_KERNEL);
> -	if (!grp->pins_conf || !grp->pins)
> -		return -ENOMEM;
> +	if (!grp->pins_conf || !grp->pins) {
> +		err = -ENOMEM;
> +		goto out_err;
> +	}

why ???
>  
>  	for (i = 0, j = 0; i < size; i += 4, j++) {
>  		pin->bank = be32_to_cpu(*list++);
>  		pin->pin = be32_to_cpu(*list++);
>  		grp->pins[j] = pin->bank * MAX_NB_GPIO_PER_BANK + pin->pin;
>  		pin->mux = be32_to_cpu(*list++);
> -		pin->conf = be32_to_cpu(*list++);
> +		if (at91_pinctrl_desc.confops->is_generic) {
> +			struct device_node *np_config;
> +			const __be32 *phandle =	list++;
> +
> +			if (!phandle) {
> +				err = -EINVAL;
> +				goto out_err;
> +			}
> +			np_config =
> +				of_find_node_by_phandle(be32_to_cpup(phandle));
> +			pin->conftype = AT91_PINCONF_GENERIC;
> +			err = pinconf_generic_parse_dt_config(np_config,
> +						&pin->conf.generic.configs,
> +						&pin->conf.generic.nconfigs);
> +			if (err)
> +				goto out_err;
> +
> +		} else {
> +			pin->conftype = AT91_PINCONF_NATIVE;
> +			pin->conf.native = be32_to_cpu(*list++);
> +		}
>  
>  		at91_pin_dbg(info->dev, pin);
>  		pin++;
>  	}
>  
>  	return 0;
> +
> +out_err:
> +	kfree(grp->pins_conf);
> +	kfree(grp->pins);
use devm and drop those kfree
> +	return err;
>  }
>  
>  static int at91_pinctrl_parse_functions(struct device_node *np,
> @@ -904,10 +1124,10 @@ static int at91_pinctrl_parse_functions(struct device_node *np,
>  	/* Initialise function */
>  	func->name = np->name;
>  	func->ngroups = of_get_child_count(np);
> -	if (func->ngroups <= 0) {
> -		dev_err(info->dev, "no groups defined\n");
> -		return -EINVAL;
> -	}
> +	/* This node might be a generic config definition: silently ignore it */
> +	if (func->ngroups <= 0)
> +		return 0;
> +
>  	func->groups = devm_kzalloc(info->dev,
>  			func->ngroups * sizeof(char *), GFP_KERNEL);
>  	if (!func->groups)
> @@ -930,6 +1150,11 @@ static struct of_device_id at91_pinctrl_of_match[] = {
>  	{ /* sentinel */ }
>  };
>  
> +static struct of_device_id at91_pinconf_of_match[] = {
> +	{ .compatible = "generic-pinconf" },
> +	{ /* sentinel */ }
> +};
> +
>  static int at91_pinctrl_probe_dt(struct platform_device *pdev,
>  				 struct at91_pinctrl *info)
>  {
> @@ -945,6 +1170,8 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
>  	info->dev = &pdev->dev;
>  	info->ops = (struct at91_pinctrl_mux_ops *)
>  		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> +	if (of_match_device(at91_pinconf_of_match, &pdev->dev))
> +		at91_pinctrl_desc.confops = &at91_generic_pinconf_ops;
>  	at91_pinctrl_child_count(info, np);
>  
>  	if (info->nbanks < 1) {
> -- 
> 1.7.9.5
> 

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[boris brezillon]

Hello Jean-Christophe,

Le 26/08/2013 19:53, Jean-Christophe PLAGNIOL-VILLARD a écrit :
> On 23:37 Sat 24 Aug     , Boris BREZILLON wrote:
>> Add support for generic pin configuration to pinctrl-at91 driver.
>>
>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>> ---
>>   .../bindings/pinctrl/atmel,at91-pinctrl.txt        |   43 +++-
>>   drivers/pinctrl/Kconfig                            |    2 +-
>>   drivers/pinctrl/pinctrl-at91.c                     |  265 ++++++++++++++++++--
>>   3 files changed, 289 insertions(+), 21 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>> index 7ccae49..7a7c0c4 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>> @@ -18,7 +18,9 @@ mode) this pin can work on and the 'config' configures various pad settings
>>   such as pull-up, multi drive, etc.
>>   
>>   Required properties for iomux controller:
>> -- compatible: "atmel,at91rm9200-pinctrl"
>> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".
>> +  Add "generic-pinconf" to the compatible string list to use the generic pin
>> +  configuration syntax.
>>   - atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
>>     configured in this periph mode. All the periph and bank need to be describe.
>>   
>> @@ -83,6 +85,11 @@ Required properties for pin configuration node:
>>     setting. The format is atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>.
>>     The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B...
>>     PIN_BANK 0 is pioA, PIN_BANK 1 is pioB...
>> +  Dependending on the presence of the "generic-pinconf" string in the
>> +  compatible property the 4th cell is:
>> +   * a phandle referencing a generic pin config node (refer to
>> +     pinctrl-bindings.txt)
>> +   * an integer defining the pin config (see the following description)
>>   
>>   Bits used for CONFIG:
>>   PULL_UP		(1 << 0): indicate this pin need a pull up.
>> @@ -132,6 +139,40 @@ pinctrl@fffff400 {
>>   	};
>>   };
>>   
>> +or
>> +
>> +pinctrl@fffff400 {
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +	ranges;
>> +	compatible = "atmel,at91rm9200-pinctrl", "generic-pinconf", "simple-bus";
> nack your break the backword compatibility
>
> if we use a old kernel with this new dt nothing will work
> as the old kernel will never known the the "generic-pinconf" means anything

Your're right, I didn't think of this case (old kernel with new dt).

> if we want to use generic-pinconf support you *CAN NOT* use atmel,at91rm9200-pinctrl
> in the compatible

What about using "atmel,at91xx-pinconf" instead of 
"atmel,at91xx-pinctrl" to notify
the generic pinconf compatibility (as done by single pinctrl driver) ?

>> +	reg = <0xfffff400 0x600>;
>> +
>> +	atmel,mux-mask = <
>> +	      /*    A         B     */
>> +	       0xffffffff 0xffc00c3b  /* pioA */
>> +	       0xffffffff 0x7fff3ccf  /* pioB */
>> +	       0xffffffff 0x007fffff  /* pioC */
>> +	      >;
>> +
>> +	pcfg_none: pcfg_none {
>> +		bias-disable;
>> +	};
>> +
>> +	pcfg_pull_up: pcfg_pull_up {
>> +		bias-pullup;
>> +	};
>> +
>> +	/* shared pinctrl settings */
>> +	dbgu {
>> +		pinctrl_dbgu: dbgu-0 {
>> +			atmel,pins =
>> +				<1 14 0x1 &pcfg_none		/* PB14 periph A */
>> +				 1 15 0x1 &pcfg_pull_up>;	/* PB15 periph A with pullup */
>> +		};
>> +	};
>> +};
>> +
>>   dbgu: serial@fffff200 {
>>   	compatible = "atmel,at91sam9260-usart";
>>   	reg = <0xfffff200 0x200>;
>> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
>> index bdb1a87..55a4f2c 100644
>> --- a/drivers/pinctrl/Kconfig
>> +++ b/drivers/pinctrl/Kconfig
>> @@ -54,7 +54,7 @@ config PINCTRL_AT91
>>   	depends on OF
>>   	depends on ARCH_AT91
>>   	select PINMUX
>> -	select PINCONF
>> +	select GENERIC_PINCONF
>>   	help
>>   	  Say Y here to enable the at91 pinctrl driver
>>   
>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>> index 7cce066..1994dd2 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -23,6 +23,7 @@
>>   #include <linux/gpio.h>
>>   #include <linux/pinctrl/machine.h>
>>   #include <linux/pinctrl/pinconf.h>
>> +#include <linux/pinctrl/pinconf-generic.h>
>>   #include <linux/pinctrl/pinctrl.h>
>>   #include <linux/pinctrl/pinmux.h>
>>   /* Since we request GPIOs from ourself */
>> @@ -32,6 +33,7 @@
>>   #include <mach/at91_pio.h>
>>   
>>   #include "core.h"
>> +#include "pinconf.h"
>>   
>>   #define MAX_NB_GPIO_PER_BANK	32
>>   
>> @@ -85,6 +87,21 @@ enum at91_mux {
>>   	AT91_MUX_PERIPH_D = 4,
>>   };
>>   
>> +struct at91_generic_pinconf {
>> +	unsigned long	*configs;
>> +	unsigned int	nconfigs;
>> +};
>> +
>> +enum at91_pinconf_type {
>> +	AT91_PINCONF_NATIVE,
>> +	AT91_PINCONF_GENERIC,
>> +};
>> +
>> +union at91_pinconf {
>> +	unsigned long			native;
>> +	struct at91_generic_pinconf	generic;
>> +};
>> +
>>   /**
>>    * struct at91_pmx_pin - describes an At91 pin mux
>>    * @bank: the bank of the pin
>> @@ -93,10 +110,11 @@ enum at91_mux {
>>    * @conf: the configuration of the pin: PULL_UP, MULTIDRIVE etc...
>>    */
>>   struct at91_pmx_pin {
>> -	uint32_t	bank;
>> -	uint32_t	pin;
>> -	enum at91_mux	mux;
>> -	unsigned long	conf;
>> +	uint32_t		bank;
>> +	uint32_t		pin;
>> +	enum at91_mux		mux;
>> +	enum at91_pinconf_type	conftype;
>> +	union at91_pinconf	conf;
>>   };
>>   
>>   /**
>> @@ -278,8 +296,16 @@ static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
>>   		new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
>>   		new_map[i].data.configs.group_or_pin =
>>   				pin_get_name(pctldev, grp->pins[i]);
>> -		new_map[i].data.configs.configs = &grp->pins_conf[i].conf;
>> -		new_map[i].data.configs.num_configs = 1;
>> +		if (!pctldev->desc->confops->is_generic) {
>> +			new_map[i].data.configs.configs =
>> +				&grp->pins_conf[i].conf.native;
>> +			new_map[i].data.configs.num_configs = 1;
>> +		} else {
>> +			new_map[i].data.configs.configs =
>> +				grp->pins_conf[i].conf.generic.configs;
>> +			new_map[i].data.configs.num_configs =
>> +				grp->pins_conf[i].conf.generic.nconfigs;
>> +		}
>>   	}
>>   
>>   	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
>> @@ -325,7 +351,7 @@ static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
>>   
>>   static unsigned at91_mux_get_pullup(void __iomem *pio, unsigned pin)
>>   {
>> -	return (readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1;
>> +	return !((readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1);
>>   }
>>   
>>   static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, bool on)
>> @@ -407,6 +433,16 @@ static enum at91_mux at91_mux_get_periph(void __iomem *pio, unsigned mask)
>>   	return select + 1;
>>   }
>>   
>> +static bool at91_mux_get_output(void __iomem *pio, unsigned pin)
>> +{
>> +	return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
>> +}
>> +
>> +static void at91_mux_set_output(void __iomem *pio, unsigned mask, bool is_on)
>> +{
>> +	__raw_writel(mask, pio + (is_on ? PIO_OER : PIO_ODR));
>> +}
>> +
>>   static bool at91_mux_get_deglitch(void __iomem *pio, unsigned pin)
>>   {
>>   	return (__raw_readl(pio + PIO_IFSR) >> pin) & 0x1;
>> @@ -445,7 +481,7 @@ static void at91_mux_pio3_set_debounce(void __iomem *pio, unsigned mask,
>>   
>>   static bool at91_mux_pio3_get_pulldown(void __iomem *pio, unsigned pin)
>>   {
>> -	return (__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1;
>> +	return !((__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1);
>>   }
>>   
>>   static void at91_mux_pio3_set_pulldown(void __iomem *pio, unsigned mask, bool is_on)
>> @@ -492,12 +528,17 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
>>   static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin)
>>   {
>>   	if (pin->mux) {
>> -		dev_dbg(dev, "pio%c%d configured as periph%c with conf = 0x%lu\n",
>> -			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A', pin->conf);
>> +		dev_dbg(dev, "pio%c%d configured as periph%c",
>> +			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A');
>>   	} else {
>> -		dev_dbg(dev, "pio%c%d configured as gpio with conf = 0x%lu\n",
>> -			pin->bank + 'A', pin->pin, pin->conf);
>> +		dev_dbg(dev, "pio%c%d configured as gpio",
>> +			pin->bank + 'A', pin->pin);
>>   	}
>> +
>> +	if (pin->conftype == AT91_PINCONF_NATIVE)
>> +		dev_dbg(dev, " with conf = 0x%lu\n", pin->conf.native);
>> +	else
>> +		dev_dbg(dev, "\n");
> do not change debug output

I do not change the debug output for the native pinconf binding, but I 
cannot print the config as
a single interger in hex format if the generic pinconf is used.
I must translate each config entry to a "property=value" string, or 
translate the generic config
array to a single native config integer.

Do you have something easier in mind ?

>>   }
>>   
>>   static int pin_check_config(struct at91_pinctrl *info, const char *name,
>> @@ -782,6 +823,157 @@ static const struct pinconf_ops at91_pinconf_ops = {
>>   	.pin_config_group_dbg_show	= at91_pinconf_group_dbg_show,
>>   };
>>   
>> +static int at91_generic_pinconf_get(struct pinctrl_dev *pctldev,
>> +				    unsigned pin_id, unsigned long *config)
>> +{
>> +	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
>> +	enum pin_config_param param = pinconf_to_config_param(*config);
>> +	void __iomem *pio = pin_to_controller(info, pin_to_bank(pin_id));
>> +	unsigned pin = pin_id % MAX_NB_GPIO_PER_BANK;
>> +	int div;
>> +
>> +	switch (param) {
>> +	case PIN_CONFIG_BIAS_DISABLE:
>> +		if (at91_mux_get_pullup(pio, pin) &&
>> +		    (info->ops->get_pulldown ||
>> +		     !info->ops->get_pulldown(pio, pin)))
>> +			return -EINVAL;
>> +
>> +		*config = 0;
>> +		break;
>> +	case PIN_CONFIG_BIAS_PULL_UP:
>> +		if (!at91_mux_get_pullup(pio, pin))
>> +			return -EINVAL;
>> +
>> +		*config = 0;
>> +		break;
>> +	case PIN_CONFIG_BIAS_PULL_DOWN:
>> +		if (!info->ops->get_pulldown)
>> +			return -ENOTSUPP;
>> +		if (!info->ops->get_pulldown(pio, pin))
>> +			return -EINVAL;
>> +
>> +		*config = 0;
>> +		break;
>> +	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
>> +		if (!at91_mux_get_multidrive(pio, pin))
>> +			return -EINVAL;
>> +
>> +		*config = 0;
>> +		break;
>> +	case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
>> +		if (!info->ops->get_schmitt_trig)
>> +			return -ENOTSUPP;
>> +
>> +		if (!(info->ops->get_schmitt_trig(pio, pin)))
>> +			*config = 1;
>> +		else
>> +			*config = 0;
>> +		break;
>> +	case PIN_CONFIG_INPUT_DEBOUNCE:
>> +		if (!info->ops->get_debounce)
>> +			return -ENOTSUPP;
>> +
>> +		if (info->ops->get_debounce(pio, pin, &div)) {
>> +			/* TODO: replace 32768 with clk_get_rate(slck) return */
>> +			*config = ((div + 1) * 2) * 1000000 / 32768;
>> +			if (*config > 0xffff)
>> +				*config = 0xffff;
>> +		} else
>> +			*config = 0;
>> +		break;
>> +	case PIN_CONFIG_INPUT_DEGLITCH:
>> +		if (!info->ops->get_deglitch)
>> +			return -ENOTSUPP;
>> +
>> +		*config = info->ops->get_deglitch(pio, pin);
>> +		break;
>> +	case PIN_CONFIG_OUTPUT:
>> +		if (info->ops->get_periph(pio, pin) != AT91_MUX_GPIO)
>> +			return -EINVAL;
>> +
>> +		*config = at91_mux_get_output(pio, pin);
>> +		break;
>> +	default:
>> +		return -ENOTSUPP;
>> +		break;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int at91_generic_pinconf_set(struct pinctrl_dev *pctldev,
>> +				    unsigned pin_id, unsigned long config)
>> +{
>> +	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
>> +	enum pin_config_param param = pinconf_to_config_param(config);
>> +	u16 arg = pinconf_to_config_argument(config);
>> +	u32 div = 0;
>> +	unsigned pin = pin_id % MAX_NB_GPIO_PER_BANK;
>> +	unsigned mask = pin_to_mask(pin);
>> +	void __iomem *pio = pin_to_controller(info, pin_to_bank(pin_id));
>> +
>> +	switch (param) {
>> +	case PIN_CONFIG_BIAS_DISABLE:
>> +		at91_mux_set_pullup(pio, mask, 0);
>> +		if (info->ops->set_pulldown)
>> +			info->ops->set_pulldown(pio, mask, 0);
>> +		break;
>> +	case PIN_CONFIG_BIAS_PULL_UP:
>> +		at91_mux_set_pullup(pio, mask, arg);
>> +		break;
>> +	case PIN_CONFIG_BIAS_PULL_DOWN:
>> +		if (!info->ops->set_pulldown)
>> +			return -ENOTSUPP;
>> +		info->ops->set_pulldown(pio, mask, arg);
>> +		break;
>> +	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
>> +		at91_mux_set_multidrive(pio, mask, 1);
>> +		break;
>> +	case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
>> +		if (!info->ops->disable_schmitt_trig)
>> +			return -ENOTSUPP;
>> +		if (arg)
>> +			mask = ~mask;
>> +		info->ops->disable_schmitt_trig(pio, mask);
>> +		break;
>> +	case PIN_CONFIG_INPUT_DEBOUNCE:
>> +		if (!info->ops->set_debounce)
>> +			return -ENOTSUPP;
>> +
>> +		/* TODO: replace 32768 with clk_get_rate(slck) return */
>> +		if (arg) {
>> +			div = (arg * 32768 / (2 * 1000000));
>> +			if (div)
>> +				div--;
>> +		}
>> +		info->ops->set_debounce(pio, mask, arg ? 1 : 0, div);
>> +		break;
>> +	case PIN_CONFIG_INPUT_DEGLITCH:
>> +		if (!info->ops->set_deglitch)
>> +			return -ENOTSUPP;
>> +
>> +		info->ops->set_deglitch(pio, mask, arg ? 1 : 0);
>> +		break;
>> +	case PIN_CONFIG_OUTPUT:
>> +		if (info->ops->get_periph(pio, pin) != AT91_MUX_GPIO)
>> +			return -EINVAL;
>> +		at91_mux_set_output(pio, mask, arg);
>> +		break;
>> +	default:
>> +		return -ENOTSUPP;
>> +		break;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct pinconf_ops at91_generic_pinconf_ops = {
>> +	.is_generic			= true,
>> +	.pin_config_get			= at91_generic_pinconf_get,
>> +	.pin_config_set			= at91_generic_pinconf_set,
>> +};
>> +
>>   static struct pinctrl_desc at91_pinctrl_desc = {
>>   	.pctlops	= &at91_pctrl_ops,
>>   	.pmxops		= &at91_pmx_ops,
>> @@ -847,6 +1039,7 @@ static int at91_pinctrl_parse_groups(struct device_node *np,
>>   	int size;
>>   	const __be32 *list;
>>   	int i, j;
>> +	int err;
>>   
>>   	dev_dbg(info->dev, "group(%d): %s\n", index, np->name);
>>   
>> @@ -870,21 +1063,48 @@ static int at91_pinctrl_parse_groups(struct device_node *np,
>>   				GFP_KERNEL);
>>   	grp->pins = devm_kzalloc(info->dev, grp->npins * sizeof(unsigned int),
>>   				GFP_KERNEL);
>> -	if (!grp->pins_conf || !grp->pins)
>> -		return -ENOMEM;
>> +	if (!grp->pins_conf || !grp->pins) {
>> +		err = -ENOMEM;
>> +		goto out_err;
>> +	}
> why ???

Right, I didn't notice the devm_kzalloc, I thought it was allocated 
using kzalloc.

>>   
>>   	for (i = 0, j = 0; i < size; i += 4, j++) {
>>   		pin->bank = be32_to_cpu(*list++);
>>   		pin->pin = be32_to_cpu(*list++);
>>   		grp->pins[j] = pin->bank * MAX_NB_GPIO_PER_BANK + pin->pin;
>>   		pin->mux = be32_to_cpu(*list++);
>> -		pin->conf = be32_to_cpu(*list++);
>> +		if (at91_pinctrl_desc.confops->is_generic) {
>> +			struct device_node *np_config;
>> +			const __be32 *phandle =	list++;
>> +
>> +			if (!phandle) {
>> +				err = -EINVAL;
>> +				goto out_err;
>> +			}
>> +			np_config =
>> +				of_find_node_by_phandle(be32_to_cpup(phandle));
>> +			pin->conftype = AT91_PINCONF_GENERIC;
>> +			err = pinconf_generic_parse_dt_config(np_config,
>> +						&pin->conf.generic.configs,
>> +						&pin->conf.generic.nconfigs);
>> +			if (err)
>> +				goto out_err;
>> +
>> +		} else {
>> +			pin->conftype = AT91_PINCONF_NATIVE;
>> +			pin->conf.native = be32_to_cpu(*list++);
>> +		}
>>   
>>   		at91_pin_dbg(info->dev, pin);
>>   		pin++;
>>   	}
>>   
>>   	return 0;
>> +
>> +out_err:
>> +	kfree(grp->pins_conf);
>> +	kfree(grp->pins);
> use devm and drop those kfree

Same mistake (devm_kzalloc is already used).
I'll drop this part for next version.

>> +	return err;
>>   }
>>   
>>   static int at91_pinctrl_parse_functions(struct device_node *np,
>> @@ -904,10 +1124,10 @@ static int at91_pinctrl_parse_functions(struct device_node *np,
>>   	/* Initialise function */
>>   	func->name = np->name;
>>   	func->ngroups = of_get_child_count(np);
>> -	if (func->ngroups <= 0) {
>> -		dev_err(info->dev, "no groups defined\n");
>> -		return -EINVAL;
>> -	}
>> +	/* This node might be a generic config definition: silently ignore it */
>> +	if (func->ngroups <= 0)
>> +		return 0;
>> +
>>   	func->groups = devm_kzalloc(info->dev,
>>   			func->ngroups * sizeof(char *), GFP_KERNEL);
>>   	if (!func->groups)
>> @@ -930,6 +1150,11 @@ static struct of_device_id at91_pinctrl_of_match[] = {
>>   	{ /* sentinel */ }
>>   };
>>   
>> +static struct of_device_id at91_pinconf_of_match[] = {
>> +	{ .compatible = "generic-pinconf" },
>> +	{ /* sentinel */ }
>> +};
>> +
>>   static int at91_pinctrl_probe_dt(struct platform_device *pdev,
>>   				 struct at91_pinctrl *info)
>>   {
>> @@ -945,6 +1170,8 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
>>   	info->dev = &pdev->dev;
>>   	info->ops = (struct at91_pinctrl_mux_ops *)
>>   		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
>> +	if (of_match_device(at91_pinconf_of_match, &pdev->dev))
>> +		at91_pinctrl_desc.confops = &at91_generic_pinconf_ops;
>>   	at91_pinctrl_child_count(info, np);
>>   
>>   	if (info->nbanks < 1) {
>> -- 
>> 1.7.9.5
>>

Thanks for your review.

Best Regards,

Boris

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[Jean-Christophe PLAGNIOL-VILLARD]

On 20:45 Mon 26 Aug     , boris brezillon wrote:
> Hello Jean-Christophe,
> 
> Le 26/08/2013 19:53, Jean-Christophe PLAGNIOL-VILLARD a écrit :
> >On 23:37 Sat 24 Aug     , Boris BREZILLON wrote:
> >>Add support for generic pin configuration to pinctrl-at91 driver.
> >>
> >>Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> >>---
> >>  .../bindings/pinctrl/atmel,at91-pinctrl.txt        |   43 +++-
> >>  drivers/pinctrl/Kconfig                            |    2 +-
> >>  drivers/pinctrl/pinctrl-at91.c                     |  265 ++++++++++++++++++--
> >>  3 files changed, 289 insertions(+), 21 deletions(-)
> >>
> >>diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> >>index 7ccae49..7a7c0c4 100644
> >>--- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> >>+++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> >>@@ -18,7 +18,9 @@ mode) this pin can work on and the 'config' configures various pad settings
> >>  such as pull-up, multi drive, etc.
> >>  Required properties for iomux controller:
> >>-- compatible: "atmel,at91rm9200-pinctrl"
> >>+- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".
> >>+  Add "generic-pinconf" to the compatible string list to use the generic pin
> >>+  configuration syntax.
> >>  - atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
> >>    configured in this periph mode. All the periph and bank need to be describe.
> >>@@ -83,6 +85,11 @@ Required properties for pin configuration node:
> >>    setting. The format is atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>.
> >>    The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B...
> >>    PIN_BANK 0 is pioA, PIN_BANK 1 is pioB...
> >>+  Dependending on the presence of the "generic-pinconf" string in the
> >>+  compatible property the 4th cell is:
> >>+   * a phandle referencing a generic pin config node (refer to
> >>+     pinctrl-bindings.txt)
> >>+   * an integer defining the pin config (see the following description)
> >>  Bits used for CONFIG:
> >>  PULL_UP		(1 << 0): indicate this pin need a pull up.
> >>@@ -132,6 +139,40 @@ pinctrl@fffff400 {
> >>  	};
> >>  };
> >>+or
> >>+
> >>+pinctrl@fffff400 {
> >>+	#address-cells = <1>;
> >>+	#size-cells = <1>;
> >>+	ranges;
> >>+	compatible = "atmel,at91rm9200-pinctrl", "generic-pinconf", "simple-bus";
> >nack your break the backword compatibility
> >
> >if we use a old kernel with this new dt nothing will work
> >as the old kernel will never known the the "generic-pinconf" means anything
> 
> Your're right, I didn't think of this case (old kernel with new dt).
> 
> >if we want to use generic-pinconf support you *CAN NOT* use atmel,at91rm9200-pinctrl
> >in the compatible
> 
> What about using "atmel,at91xx-pinconf" instead of
> "atmel,at91xx-pinctrl" to notify
> the generic pinconf compatibility (as done by single pinctrl driver) ?
no as the rm9200 IP and sam9x5 IP are only partially compatible
you MUST distinguish them

> 
> >>+	reg = <0xfffff400 0x600>;
> >>+
> >>+	atmel,mux-mask = <
> >>+	      /*    A         B     */
> >>+	       0xffffffff 0xffc00c3b  /* pioA */
> >>+	       0xffffffff 0x7fff3ccf  /* pioB */
> >>+	       0xffffffff 0x007fffff  /* pioC */
> >>+	      >;
> >>+
> >>+	pcfg_none: pcfg_none {
> >>+		bias-disable;
> >>+	};
> >>+
> >>+	pcfg_pull_up: pcfg_pull_up {
> >>+		bias-pullup;
> >>+	};
> >>+
> >>+	/* shared pinctrl settings */
> >>+	dbgu {
> >>+		pinctrl_dbgu: dbgu-0 {
> >>+			atmel,pins =
> >>+				<1 14 0x1 &pcfg_none		/* PB14 periph A */
> >>+				 1 15 0x1 &pcfg_pull_up>;	/* PB15 periph A with pullup */
> >>+		};
> >>+	};
> >>+};
> >>+
> >>  dbgu: serial@fffff200 {
> >>  	compatible = "atmel,at91sam9260-usart";
> >>  	reg = <0xfffff200 0x200>;
> >>diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> >>index bdb1a87..55a4f2c 100644
> >>--- a/drivers/pinctrl/Kconfig
> >>+++ b/drivers/pinctrl/Kconfig
> >>@@ -54,7 +54,7 @@ config PINCTRL_AT91
> >>  	depends on OF
> >>  	depends on ARCH_AT91
> >>  	select PINMUX
> >>-	select PINCONF
> >>+	select GENERIC_PINCONF
> >>  	help
> >>  	  Say Y here to enable the at91 pinctrl driver
> >>diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> >>index 7cce066..1994dd2 100644
> >>--- a/drivers/pinctrl/pinctrl-at91.c
> >>+++ b/drivers/pinctrl/pinctrl-at91.c
> >>@@ -23,6 +23,7 @@
> >>  #include <linux/gpio.h>
> >>  #include <linux/pinctrl/machine.h>
> >>  #include <linux/pinctrl/pinconf.h>
> >>+#include <linux/pinctrl/pinconf-generic.h>
> >>  #include <linux/pinctrl/pinctrl.h>
> >>  #include <linux/pinctrl/pinmux.h>
> >>  /* Since we request GPIOs from ourself */
> >>@@ -32,6 +33,7 @@
> >>  #include <mach/at91_pio.h>
> >>  #include "core.h"
> >>+#include "pinconf.h"
> >>  #define MAX_NB_GPIO_PER_BANK	32
> >>@@ -85,6 +87,21 @@ enum at91_mux {
> >>  	AT91_MUX_PERIPH_D = 4,
> >>  };
> >>+struct at91_generic_pinconf {
> >>+	unsigned long	*configs;
> >>+	unsigned int	nconfigs;
> >>+};
> >>+
> >>+enum at91_pinconf_type {
> >>+	AT91_PINCONF_NATIVE,
> >>+	AT91_PINCONF_GENERIC,
> >>+};
> >>+
> >>+union at91_pinconf {
> >>+	unsigned long			native;
> >>+	struct at91_generic_pinconf	generic;
> >>+};
> >>+
> >>  /**
> >>   * struct at91_pmx_pin - describes an At91 pin mux
> >>   * @bank: the bank of the pin
> >>@@ -93,10 +110,11 @@ enum at91_mux {
> >>   * @conf: the configuration of the pin: PULL_UP, MULTIDRIVE etc...
> >>   */
> >>  struct at91_pmx_pin {
> >>-	uint32_t	bank;
> >>-	uint32_t	pin;
> >>-	enum at91_mux	mux;
> >>-	unsigned long	conf;
> >>+	uint32_t		bank;
> >>+	uint32_t		pin;
> >>+	enum at91_mux		mux;
> >>+	enum at91_pinconf_type	conftype;
> >>+	union at91_pinconf	conf;
> >>  };
> >>  /**
> >>@@ -278,8 +296,16 @@ static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
> >>  		new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
> >>  		new_map[i].data.configs.group_or_pin =
> >>  				pin_get_name(pctldev, grp->pins[i]);
> >>-		new_map[i].data.configs.configs = &grp->pins_conf[i].conf;
> >>-		new_map[i].data.configs.num_configs = 1;
> >>+		if (!pctldev->desc->confops->is_generic) {
> >>+			new_map[i].data.configs.configs =
> >>+				&grp->pins_conf[i].conf.native;
> >>+			new_map[i].data.configs.num_configs = 1;
> >>+		} else {
> >>+			new_map[i].data.configs.configs =
> >>+				grp->pins_conf[i].conf.generic.configs;
> >>+			new_map[i].data.configs.num_configs =
> >>+				grp->pins_conf[i].conf.generic.nconfigs;
> >>+		}
> >>  	}
> >>  	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
> >>@@ -325,7 +351,7 @@ static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
> >>  static unsigned at91_mux_get_pullup(void __iomem *pio, unsigned pin)
> >>  {
> >>-	return (readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1;
> >>+	return !((readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1);
> >>  }
> >>  static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, bool on)
> >>@@ -407,6 +433,16 @@ static enum at91_mux at91_mux_get_periph(void __iomem *pio, unsigned mask)
> >>  	return select + 1;
> >>  }
> >>+static bool at91_mux_get_output(void __iomem *pio, unsigned pin)
> >>+{
> >>+	return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
> >>+}
> >>+
> >>+static void at91_mux_set_output(void __iomem *pio, unsigned mask, bool is_on)
> >>+{
> >>+	__raw_writel(mask, pio + (is_on ? PIO_OER : PIO_ODR));
> >>+}
> >>+
> >>  static bool at91_mux_get_deglitch(void __iomem *pio, unsigned pin)
> >>  {
> >>  	return (__raw_readl(pio + PIO_IFSR) >> pin) & 0x1;
> >>@@ -445,7 +481,7 @@ static void at91_mux_pio3_set_debounce(void __iomem *pio, unsigned mask,
> >>  static bool at91_mux_pio3_get_pulldown(void __iomem *pio, unsigned pin)
> >>  {
> >>-	return (__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1;
> >>+	return !((__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1);
> >>  }
> >>  static void at91_mux_pio3_set_pulldown(void __iomem *pio, unsigned mask, bool is_on)
> >>@@ -492,12 +528,17 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
> >>  static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin)
> >>  {
> >>  	if (pin->mux) {
> >>-		dev_dbg(dev, "pio%c%d configured as periph%c with conf = 0x%lu\n",
> >>-			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A', pin->conf);
> >>+		dev_dbg(dev, "pio%c%d configured as periph%c",
> >>+			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A');
> >>  	} else {
> >>-		dev_dbg(dev, "pio%c%d configured as gpio with conf = 0x%lu\n",
> >>-			pin->bank + 'A', pin->pin, pin->conf);
> >>+		dev_dbg(dev, "pio%c%d configured as gpio",
> >>+			pin->bank + 'A', pin->pin);
> >>  	}
> >>+
> >>+	if (pin->conftype == AT91_PINCONF_NATIVE)
> >>+		dev_dbg(dev, " with conf = 0x%lu\n", pin->conf.native);
> >>+	else
> >>+		dev_dbg(dev, "\n");
> >do not change debug output
> 
> I do not change the debug output for the native pinconf binding, but
> I cannot print the config as
> a single interger in hex format if the generic pinconf is used.
> I must translate each config entry to a "property=value" string, or
> translate the generic config
> array to a single native config integer.
> 
> Do you have something easier in mind ?

no but I do not want to brake current automatic test tools

propose something with this in mind

Best Regards,
J.

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[boris brezillon]

Le 26/08/2013 21:18, Jean-Christophe PLAGNIOL-VILLARD a écrit :
> On 20:45 Mon 26 Aug     , boris brezillon wrote:
>> Hello Jean-Christophe,
>>
>> Le 26/08/2013 19:53, Jean-Christophe PLAGNIOL-VILLARD a écrit :
>>> On 23:37 Sat 24 Aug     , Boris BREZILLON wrote:
>>>> Add support for generic pin configuration to pinctrl-at91 driver.
>>>>
>>>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>>>> ---
>>>>   .../bindings/pinctrl/atmel,at91-pinctrl.txt        |   43 +++-
>>>>   drivers/pinctrl/Kconfig                            |    2 +-
>>>>   drivers/pinctrl/pinctrl-at91.c                     |  265 ++++++++++++++++++--
>>>>   3 files changed, 289 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>> index 7ccae49..7a7c0c4 100644
>>>> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>> @@ -18,7 +18,9 @@ mode) this pin can work on and the 'config' configures various pad settings
>>>>   such as pull-up, multi drive, etc.
>>>>   Required properties for iomux controller:
>>>> -- compatible: "atmel,at91rm9200-pinctrl"
>>>> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".
>>>> +  Add "generic-pinconf" to the compatible string list to use the generic pin
>>>> +  configuration syntax.
>>>>   - atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
>>>>     configured in this periph mode. All the periph and bank need to be describe.
>>>> @@ -83,6 +85,11 @@ Required properties for pin configuration node:
>>>>     setting. The format is atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>.
>>>>     The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B...
>>>>     PIN_BANK 0 is pioA, PIN_BANK 1 is pioB...
>>>> +  Dependending on the presence of the "generic-pinconf" string in the
>>>> +  compatible property the 4th cell is:
>>>> +   * a phandle referencing a generic pin config node (refer to
>>>> +     pinctrl-bindings.txt)
>>>> +   * an integer defining the pin config (see the following description)
>>>>   Bits used for CONFIG:
>>>>   PULL_UP		(1 << 0): indicate this pin need a pull up.
>>>> @@ -132,6 +139,40 @@ pinctrl@fffff400 {
>>>>   	};
>>>>   };
>>>> +or
>>>> +
>>>> +pinctrl@fffff400 {
>>>> +	#address-cells = <1>;
>>>> +	#size-cells = <1>;
>>>> +	ranges;
>>>> +	compatible = "atmel,at91rm9200-pinctrl", "generic-pinconf", "simple-bus";
>>> nack your break the backword compatibility
>>>
>>> if we use a old kernel with this new dt nothing will work
>>> as the old kernel will never known the the "generic-pinconf" means anything
>> Your're right, I didn't think of this case (old kernel with new dt).
>>
>>> if we want to use generic-pinconf support you *CAN NOT* use atmel,at91rm9200-pinctrl
>>> in the compatible
>> What about using "atmel,at91xx-pinconf" instead of
>> "atmel,at91xx-pinctrl" to notify
>> the generic pinconf compatibility (as done by single pinctrl driver) ?
> no as the rm9200 IP and sam9x5 IP are only partially compatible
> you MUST distinguish them

What I meant is use the "-pinctrl" and "-pinconf" suffixes to 
differentiate between native and generic
pinconf bindings and keep the IP names as it is right now (replace xx 
with the IP name) to differentiate
the IP versions.

This gives us the following compatible strings:

"atmel,at91rm9200-pinctrl"
"atmel,at91rm9200-pinconf"
"atmel,at91sam9x5-pinctrl"
"atmel,at91sam9x5-pinconf"

>>>> +	reg = <0xfffff400 0x600>;
>>>> +
>>>> +	atmel,mux-mask = <
>>>> +	      /*    A         B     */
>>>> +	       0xffffffff 0xffc00c3b  /* pioA */
>>>> +	       0xffffffff 0x7fff3ccf  /* pioB */
>>>> +	       0xffffffff 0x007fffff  /* pioC */
>>>> +	      >;
>>>> +
>>>> +	pcfg_none: pcfg_none {
>>>> +		bias-disable;
>>>> +	};
>>>> +
>>>> +	pcfg_pull_up: pcfg_pull_up {
>>>> +		bias-pullup;
>>>> +	};
>>>> +
>>>> +	/* shared pinctrl settings */
>>>> +	dbgu {
>>>> +		pinctrl_dbgu: dbgu-0 {
>>>> +			atmel,pins =
>>>> +				<1 14 0x1 &pcfg_none		/* PB14 periph A */
>>>> +				 1 15 0x1 &pcfg_pull_up>;	/* PB15 periph A with pullup */
>>>> +		};
>>>> +	};
>>>> +};
>>>> +
>>>>   dbgu: serial@fffff200 {
>>>>   	compatible = "atmel,at91sam9260-usart";
>>>>   	reg = <0xfffff200 0x200>;
>>>> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
>>>> index bdb1a87..55a4f2c 100644
>>>> --- a/drivers/pinctrl/Kconfig
>>>> +++ b/drivers/pinctrl/Kconfig
>>>> @@ -54,7 +54,7 @@ config PINCTRL_AT91
>>>>   	depends on OF
>>>>   	depends on ARCH_AT91
>>>>   	select PINMUX
>>>> -	select PINCONF
>>>> +	select GENERIC_PINCONF
>>>>   	help
>>>>   	  Say Y here to enable the at91 pinctrl driver
>>>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>>>> index 7cce066..1994dd2 100644
>>>> --- a/drivers/pinctrl/pinctrl-at91.c
>>>> +++ b/drivers/pinctrl/pinctrl-at91.c
>>>> @@ -23,6 +23,7 @@
>>>>   #include <linux/gpio.h>
>>>>   #include <linux/pinctrl/machine.h>
>>>>   #include <linux/pinctrl/pinconf.h>
>>>> +#include <linux/pinctrl/pinconf-generic.h>
>>>>   #include <linux/pinctrl/pinctrl.h>
>>>>   #include <linux/pinctrl/pinmux.h>
>>>>   /* Since we request GPIOs from ourself */
>>>> @@ -32,6 +33,7 @@
>>>>   #include <mach/at91_pio.h>
>>>>   #include "core.h"
>>>> +#include "pinconf.h"
>>>>   #define MAX_NB_GPIO_PER_BANK	32
>>>> @@ -85,6 +87,21 @@ enum at91_mux {
>>>>   	AT91_MUX_PERIPH_D = 4,
>>>>   };
>>>> +struct at91_generic_pinconf {
>>>> +	unsigned long	*configs;
>>>> +	unsigned int	nconfigs;
>>>> +};
>>>> +
>>>> +enum at91_pinconf_type {
>>>> +	AT91_PINCONF_NATIVE,
>>>> +	AT91_PINCONF_GENERIC,
>>>> +};
>>>> +
>>>> +union at91_pinconf {
>>>> +	unsigned long			native;
>>>> +	struct at91_generic_pinconf	generic;
>>>> +};
>>>> +
>>>>   /**
>>>>    * struct at91_pmx_pin - describes an At91 pin mux
>>>>    * @bank: the bank of the pin
>>>> @@ -93,10 +110,11 @@ enum at91_mux {
>>>>    * @conf: the configuration of the pin: PULL_UP, MULTIDRIVE etc...
>>>>    */
>>>>   struct at91_pmx_pin {
>>>> -	uint32_t	bank;
>>>> -	uint32_t	pin;
>>>> -	enum at91_mux	mux;
>>>> -	unsigned long	conf;
>>>> +	uint32_t		bank;
>>>> +	uint32_t		pin;
>>>> +	enum at91_mux		mux;
>>>> +	enum at91_pinconf_type	conftype;
>>>> +	union at91_pinconf	conf;
>>>>   };
>>>>   /**
>>>> @@ -278,8 +296,16 @@ static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
>>>>   		new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
>>>>   		new_map[i].data.configs.group_or_pin =
>>>>   				pin_get_name(pctldev, grp->pins[i]);
>>>> -		new_map[i].data.configs.configs = &grp->pins_conf[i].conf;
>>>> -		new_map[i].data.configs.num_configs = 1;
>>>> +		if (!pctldev->desc->confops->is_generic) {
>>>> +			new_map[i].data.configs.configs =
>>>> +				&grp->pins_conf[i].conf.native;
>>>> +			new_map[i].data.configs.num_configs = 1;
>>>> +		} else {
>>>> +			new_map[i].data.configs.configs =
>>>> +				grp->pins_conf[i].conf.generic.configs;
>>>> +			new_map[i].data.configs.num_configs =
>>>> +				grp->pins_conf[i].conf.generic.nconfigs;
>>>> +		}
>>>>   	}
>>>>   	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
>>>> @@ -325,7 +351,7 @@ static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
>>>>   static unsigned at91_mux_get_pullup(void __iomem *pio, unsigned pin)
>>>>   {
>>>> -	return (readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1;
>>>> +	return !((readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1);
>>>>   }
>>>>   static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, bool on)
>>>> @@ -407,6 +433,16 @@ static enum at91_mux at91_mux_get_periph(void __iomem *pio, unsigned mask)
>>>>   	return select + 1;
>>>>   }
>>>> +static bool at91_mux_get_output(void __iomem *pio, unsigned pin)
>>>> +{
>>>> +	return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
>>>> +}
>>>> +
>>>> +static void at91_mux_set_output(void __iomem *pio, unsigned mask, bool is_on)
>>>> +{
>>>> +	__raw_writel(mask, pio + (is_on ? PIO_OER : PIO_ODR));
>>>> +}
>>>> +
>>>>   static bool at91_mux_get_deglitch(void __iomem *pio, unsigned pin)
>>>>   {
>>>>   	return (__raw_readl(pio + PIO_IFSR) >> pin) & 0x1;
>>>> @@ -445,7 +481,7 @@ static void at91_mux_pio3_set_debounce(void __iomem *pio, unsigned mask,
>>>>   static bool at91_mux_pio3_get_pulldown(void __iomem *pio, unsigned pin)
>>>>   {
>>>> -	return (__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1;
>>>> +	return !((__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1);
>>>>   }
>>>>   static void at91_mux_pio3_set_pulldown(void __iomem *pio, unsigned mask, bool is_on)
>>>> @@ -492,12 +528,17 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
>>>>   static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin)
>>>>   {
>>>>   	if (pin->mux) {
>>>> -		dev_dbg(dev, "pio%c%d configured as periph%c with conf = 0x%lu\n",
>>>> -			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A', pin->conf);
>>>> +		dev_dbg(dev, "pio%c%d configured as periph%c",
>>>> +			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A');
>>>>   	} else {
>>>> -		dev_dbg(dev, "pio%c%d configured as gpio with conf = 0x%lu\n",
>>>> -			pin->bank + 'A', pin->pin, pin->conf);
>>>> +		dev_dbg(dev, "pio%c%d configured as gpio",
>>>> +			pin->bank + 'A', pin->pin);
>>>>   	}
>>>> +
>>>> +	if (pin->conftype == AT91_PINCONF_NATIVE)
>>>> +		dev_dbg(dev, " with conf = 0x%lu\n", pin->conf.native);
>>>> +	else
>>>> +		dev_dbg(dev, "\n");
>>> do not change debug output
>> I do not change the debug output for the native pinconf binding, but
>> I cannot print the config as
>> a single interger in hex format if the generic pinconf is used.
>> I must translate each config entry to a "property=value" string, or
>> translate the generic config
>> array to a single native config integer.
>>
>> Do you have something easier in mind ?
> no but I do not want to brake current automatic test tools
>
> propose something with this in mind
Okay.

If I understand it correctly you want to have the same traces for both 
pinconf dt-bindings
(generic and native).

I am right ?

If so, this only leaves us the 2nd solution: convert generic pinconf to 
native pinconf before printing.

>
> Best Regards,
> J.

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[Nicolas Ferre]

On 26/08/2013 21:18, Jean-Christophe PLAGNIOL-VILLARD :
> On 20:45 Mon 26 Aug     , boris brezillon wrote:
>> Hello Jean-Christophe,
>>
>> Le 26/08/2013 19:53, Jean-Christophe PLAGNIOL-VILLARD a écrit :
>>> On 23:37 Sat 24 Aug     , Boris BREZILLON wrote:
>>>> Add support for generic pin configuration to pinctrl-at91 driver.
>>>>
>>>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>>>> ---
>>>>   .../bindings/pinctrl/atmel,at91-pinctrl.txt        |   43 +++-
>>>>   drivers/pinctrl/Kconfig                            |    2 +-
>>>>   drivers/pinctrl/pinctrl-at91.c                     |  265 ++++++++++++++++++--
>>>>   3 files changed, 289 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>> index 7ccae49..7a7c0c4 100644
>>>> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>> @@ -18,7 +18,9 @@ mode) this pin can work on and the 'config' configures various pad settings
>>>>   such as pull-up, multi drive, etc.
>>>>   Required properties for iomux controller:
>>>> -- compatible: "atmel,at91rm9200-pinctrl"
>>>> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".
>>>> +  Add "generic-pinconf" to the compatible string list to use the generic pin
>>>> +  configuration syntax.
>>>>   - atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
>>>>     configured in this periph mode. All the periph and bank need to be describe.
>>>> @@ -83,6 +85,11 @@ Required properties for pin configuration node:
>>>>     setting. The format is atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>.
>>>>     The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B...
>>>>     PIN_BANK 0 is pioA, PIN_BANK 1 is pioB...
>>>> +  Dependending on the presence of the "generic-pinconf" string in the
>>>> +  compatible property the 4th cell is:
>>>> +   * a phandle referencing a generic pin config node (refer to
>>>> +     pinctrl-bindings.txt)
>>>> +   * an integer defining the pin config (see the following description)
>>>>   Bits used for CONFIG:
>>>>   PULL_UP		(1 << 0): indicate this pin need a pull up.
>>>> @@ -132,6 +139,40 @@ pinctrl@fffff400 {
>>>>   	};
>>>>   };
>>>> +or
>>>> +
>>>> +pinctrl@fffff400 {
>>>> +	#address-cells = <1>;
>>>> +	#size-cells = <1>;
>>>> +	ranges;
>>>> +	compatible = "atmel,at91rm9200-pinctrl", "generic-pinconf", "simple-bus";
>>> nack your break the backword compatibility
>>>
>>> if we use a old kernel with this new dt nothing will work
>>> as the old kernel will never known the the "generic-pinconf" means anything
>>
>> Your're right, I didn't think of this case (old kernel with new dt).
>>
>>> if we want to use generic-pinconf support you *CAN NOT* use atmel,at91rm9200-pinctrl

One.
I did not spot.

>>> in the compatible
>>
>> What about using "atmel,at91xx-pinconf" instead of
>> "atmel,at91xx-pinctrl" to notify
>> the generic pinconf compatibility (as done by single pinctrl driver) ?
> no as the rm9200 IP and sam9x5 IP are only partially compatible
> you MUST distinguish them

Two? WTF!

Jean-Christophe, YOU MUST NOT SCREAM in emails, okay?!


>>>> +	reg = <0xfffff400 0x600>;
>>>> +
>>>> +	atmel,mux-mask = <
>>>> +	      /*    A         B     */
>>>> +	       0xffffffff 0xffc00c3b  /* pioA */
>>>> +	       0xffffffff 0x7fff3ccf  /* pioB */
>>>> +	       0xffffffff 0x007fffff  /* pioC */
>>>> +	      >;
>>>> +
>>>> +	pcfg_none: pcfg_none {
>>>> +		bias-disable;
>>>> +	};
>>>> +
>>>> +	pcfg_pull_up: pcfg_pull_up {
>>>> +		bias-pullup;
>>>> +	};
>>>> +
>>>> +	/* shared pinctrl settings */
>>>> +	dbgu {
>>>> +		pinctrl_dbgu: dbgu-0 {
>>>> +			atmel,pins =
>>>> +				<1 14 0x1 &pcfg_none		/* PB14 periph A */
>>>> +				 1 15 0x1 &pcfg_pull_up>;	/* PB15 periph A with pullup */
>>>> +		};
>>>> +	};
>>>> +};
>>>> +
>>>>   dbgu: serial@fffff200 {
>>>>   	compatible = "atmel,at91sam9260-usart";
>>>>   	reg = <0xfffff200 0x200>;
>>>> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
>>>> index bdb1a87..55a4f2c 100644
>>>> --- a/drivers/pinctrl/Kconfig
>>>> +++ b/drivers/pinctrl/Kconfig
>>>> @@ -54,7 +54,7 @@ config PINCTRL_AT91
>>>>   	depends on OF
>>>>   	depends on ARCH_AT91
>>>>   	select PINMUX
>>>> -	select PINCONF
>>>> +	select GENERIC_PINCONF
>>>>   	help
>>>>   	  Say Y here to enable the at91 pinctrl driver
>>>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>>>> index 7cce066..1994dd2 100644
>>>> --- a/drivers/pinctrl/pinctrl-at91.c
>>>> +++ b/drivers/pinctrl/pinctrl-at91.c
>>>> @@ -23,6 +23,7 @@
>>>>   #include <linux/gpio.h>
>>>>   #include <linux/pinctrl/machine.h>
>>>>   #include <linux/pinctrl/pinconf.h>
>>>> +#include <linux/pinctrl/pinconf-generic.h>
>>>>   #include <linux/pinctrl/pinctrl.h>
>>>>   #include <linux/pinctrl/pinmux.h>
>>>>   /* Since we request GPIOs from ourself */
>>>> @@ -32,6 +33,7 @@
>>>>   #include <mach/at91_pio.h>
>>>>   #include "core.h"
>>>> +#include "pinconf.h"
>>>>   #define MAX_NB_GPIO_PER_BANK	32
>>>> @@ -85,6 +87,21 @@ enum at91_mux {
>>>>   	AT91_MUX_PERIPH_D = 4,
>>>>   };
>>>> +struct at91_generic_pinconf {
>>>> +	unsigned long	*configs;
>>>> +	unsigned int	nconfigs;
>>>> +};
>>>> +
>>>> +enum at91_pinconf_type {
>>>> +	AT91_PINCONF_NATIVE,
>>>> +	AT91_PINCONF_GENERIC,
>>>> +};
>>>> +
>>>> +union at91_pinconf {
>>>> +	unsigned long			native;
>>>> +	struct at91_generic_pinconf	generic;
>>>> +};
>>>> +
>>>>   /**
>>>>    * struct at91_pmx_pin - describes an At91 pin mux
>>>>    * @bank: the bank of the pin
>>>> @@ -93,10 +110,11 @@ enum at91_mux {
>>>>    * @conf: the configuration of the pin: PULL_UP, MULTIDRIVE etc...
>>>>    */
>>>>   struct at91_pmx_pin {
>>>> -	uint32_t	bank;
>>>> -	uint32_t	pin;
>>>> -	enum at91_mux	mux;
>>>> -	unsigned long	conf;
>>>> +	uint32_t		bank;
>>>> +	uint32_t		pin;
>>>> +	enum at91_mux		mux;
>>>> +	enum at91_pinconf_type	conftype;
>>>> +	union at91_pinconf	conf;
>>>>   };
>>>>   /**
>>>> @@ -278,8 +296,16 @@ static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
>>>>   		new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
>>>>   		new_map[i].data.configs.group_or_pin =
>>>>   				pin_get_name(pctldev, grp->pins[i]);
>>>> -		new_map[i].data.configs.configs = &grp->pins_conf[i].conf;
>>>> -		new_map[i].data.configs.num_configs = 1;
>>>> +		if (!pctldev->desc->confops->is_generic) {
>>>> +			new_map[i].data.configs.configs =
>>>> +				&grp->pins_conf[i].conf.native;
>>>> +			new_map[i].data.configs.num_configs = 1;
>>>> +		} else {
>>>> +			new_map[i].data.configs.configs =
>>>> +				grp->pins_conf[i].conf.generic.configs;
>>>> +			new_map[i].data.configs.num_configs =
>>>> +				grp->pins_conf[i].conf.generic.nconfigs;
>>>> +		}
>>>>   	}
>>>>   	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
>>>> @@ -325,7 +351,7 @@ static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
>>>>   static unsigned at91_mux_get_pullup(void __iomem *pio, unsigned pin)
>>>>   {
>>>> -	return (readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1;
>>>> +	return !((readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1);
>>>>   }
>>>>   static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, bool on)
>>>> @@ -407,6 +433,16 @@ static enum at91_mux at91_mux_get_periph(void __iomem *pio, unsigned mask)
>>>>   	return select + 1;
>>>>   }
>>>> +static bool at91_mux_get_output(void __iomem *pio, unsigned pin)
>>>> +{
>>>> +	return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
>>>> +}
>>>> +
>>>> +static void at91_mux_set_output(void __iomem *pio, unsigned mask, bool is_on)
>>>> +{
>>>> +	__raw_writel(mask, pio + (is_on ? PIO_OER : PIO_ODR));
>>>> +}
>>>> +
>>>>   static bool at91_mux_get_deglitch(void __iomem *pio, unsigned pin)
>>>>   {
>>>>   	return (__raw_readl(pio + PIO_IFSR) >> pin) & 0x1;
>>>> @@ -445,7 +481,7 @@ static void at91_mux_pio3_set_debounce(void __iomem *pio, unsigned mask,
>>>>   static bool at91_mux_pio3_get_pulldown(void __iomem *pio, unsigned pin)
>>>>   {
>>>> -	return (__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1;
>>>> +	return !((__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1);
>>>>   }
>>>>   static void at91_mux_pio3_set_pulldown(void __iomem *pio, unsigned mask, bool is_on)
>>>> @@ -492,12 +528,17 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
>>>>   static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin)
>>>>   {
>>>>   	if (pin->mux) {
>>>> -		dev_dbg(dev, "pio%c%d configured as periph%c with conf = 0x%lu\n",
>>>> -			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A', pin->conf);
>>>> +		dev_dbg(dev, "pio%c%d configured as periph%c",
>>>> +			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A');
>>>>   	} else {
>>>> -		dev_dbg(dev, "pio%c%d configured as gpio with conf = 0x%lu\n",
>>>> -			pin->bank + 'A', pin->pin, pin->conf);
>>>> +		dev_dbg(dev, "pio%c%d configured as gpio",
>>>> +			pin->bank + 'A', pin->pin);
>>>>   	}
>>>> +
>>>> +	if (pin->conftype == AT91_PINCONF_NATIVE)
>>>> +		dev_dbg(dev, " with conf = 0x%lu\n", pin->conf.native);
>>>> +	else
>>>> +		dev_dbg(dev, "\n");
>>> do not change debug output
>>
>> I do not change the debug output for the native pinconf binding, but
>> I cannot print the config as
>> a single interger in hex format if the generic pinconf is used.
>> I must translate each config entry to a "property=value" string, or
>> translate the generic config
>> array to a single native config integer.
>>
>> Do you have something easier in mind ?
>
> no but I do not want to brake current automatic test tools
>
> propose something with this in mind
>
> Best Regards,
> J.
>


-- 
Nicolas Ferre

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[Stephen Warren]

On 08/26/2013 12:45 PM, boris brezillon wrote:
> Hello Jean-Christophe,
> 
> Le 26/08/2013 19:53, Jean-Christophe PLAGNIOL-VILLARD a écrit :
>> On 23:37 Sat 24 Aug     , Boris BREZILLON wrote:
>>> Add support for generic pin configuration to pinctrl-at91 driver.
...
>>> a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
...
>>> configures various pad settings
>>>   such as pull-up, multi drive, etc.
>>>     Required properties for iomux controller:
>>> -- compatible: "atmel,at91rm9200-pinctrl"
>>> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".
>>> +  Add "generic-pinconf" to the compatible string list to use the
>>> generic pin
...
>>> +pinctrl@fffff400 {
>>> +    #address-cells = <1>;
>>> +    #size-cells = <1>;
>>> +    ranges;
>>> +    compatible = "atmel,at91rm9200-pinctrl", "generic-pinconf",
>>> "simple-bus";
>> nack your break the backword compatibility
>>
>> if we use a old kernel with this new dt nothing will work
>> as the old kernel will never known the the "generic-pinconf" means
>> anything
> 
> Your're right, I didn't think of this case (old kernel with new dt).

Well, just to be clear: If a new DT uses a new compatible value of any
kind, be it adding "generic-pinconf" or switching to "foo-yyy" rather
than "foo-yyy", it won't be compatible... That somewhat implies that you
can't ever replace an old binding with something new.

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[boris brezillon]

On 27/08/2013 05:54, Stephen Warren wrote:
> On 08/26/2013 12:45 PM, boris brezillon wrote:
>> Hello Jean-Christophe,
>>
>> Le 26/08/2013 19:53, Jean-Christophe PLAGNIOL-VILLARD a écrit :
>>> On 23:37 Sat 24 Aug     , Boris BREZILLON wrote:
>>>> Add support for generic pin configuration to pinctrl-at91 driver.
> ...
>>>> a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> ...
>>>> configures various pad settings
>>>>    such as pull-up, multi drive, etc.
>>>>      Required properties for iomux controller:
>>>> -- compatible: "atmel,at91rm9200-pinctrl"
>>>> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl".
>>>> +  Add "generic-pinconf" to the compatible string list to use the
>>>> generic pin
> ...
>>>> +pinctrl@fffff400 {
>>>> +    #address-cells = <1>;
>>>> +    #size-cells = <1>;
>>>> +    ranges;
>>>> +    compatible = "atmel,at91rm9200-pinctrl", "generic-pinconf",
>>>> "simple-bus";
>>> nack your break the backword compatibility
>>>
>>> if we use a old kernel with this new dt nothing will work
>>> as the old kernel will never known the the "generic-pinconf" means
>>> anything
>> Your're right, I didn't think of this case (old kernel with new dt).
> Well, just to be clear: If a new DT uses a new compatible value of any
> kind, be it adding "generic-pinconf" or switching to "foo-yyy" rather
> than "foo-yyy", it won't be compatible... That somewhat implies that you
> can't ever replace an old binding with something new.

That's absolutely right, however the behaviour won't be the same in both 
cases.

1) If your (new) dt defines its pinctrl using the "foo-pinconf" 
compatible string and
     your (old) kernel does not support it, the pinctrl will never probe 
the pinctrl definitions.
     Moreover, if you want to define both old ("foo-pinctrl") and new 
("foo-pinconf") pinctrl
     definitions in your dt in order to support several kernel versions, 
nothing prevents you
     from doing it.

2) In the other hand, if you use an additional "generic-pinconf" 
compatible string to signify
     wether or not the pinctrl definition use the generic pinconf dt 
binding, the (old) kernel
     will probe the pinctrl definitions, ignore the "generic-pinconf" 
string, and fail when parsing
     the pinctrl configuration nodes (which are invalid pinctrl function 
nodes in the current dt binding).
     We have the same problem when using the 'atmel,generic-pinconf' 
property inside a pinctrl node:
     old kernels won't take this property into account.

Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf

[Stephen Warren]

On 08/27/2013 12:04 AM, boris brezillon wrote:
> On 27/08/2013 05:54, Stephen Warren wrote:
>> On 08/26/2013 12:45 PM, boris brezillon wrote:
>>> Hello Jean-Christophe,
>>>
>>> Le 26/08/2013 19:53, Jean-Christophe PLAGNIOL-VILLARD a écrit :
>>>> On 23:37 Sat 24 Aug     , Boris BREZILLON wrote:
>>>>> Add support for generic pin configuration to pinctrl-at91 driver.
>> ...
>>>>> a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>> ...
>>>>> configures various pad settings
>>>>>    such as pull-up, multi drive, etc.
>>>>>      Required properties for iomux controller:
>>>>> -- compatible: "atmel,at91rm9200-pinctrl"
>>>>> +- compatible: "atmel,at91rm9200-pinctrl" or
>>>>> "atmel,at91sam9x5-pinctrl".
>>>>> +  Add "generic-pinconf" to the compatible string list to use the
>>>>> generic pin
>> ...
>>>>> +pinctrl@fffff400 {
>>>>> +    #address-cells = <1>;
>>>>> +    #size-cells = <1>;
>>>>> +    ranges;
>>>>> +    compatible = "atmel,at91rm9200-pinctrl", "generic-pinconf",
>>>>> "simple-bus";
>>>> nack your break the backword compatibility
>>>>
>>>> if we use a old kernel with this new dt nothing will work
>>>> as the old kernel will never known the the "generic-pinconf" means
>>>> anything
>>> Your're right, I didn't think of this case (old kernel with new dt).
>> Well, just to be clear: If a new DT uses a new compatible value of any
>> kind, be it adding "generic-pinconf" or switching to "foo-yyy" rather
>> than "foo-yyy", it won't be compatible... That somewhat implies that you
>> can't ever replace an old binding with something new.
> 
> That's absolutely right, however the behaviour won't be the same in both
> cases.
> 
> 1) If your (new) dt defines its pinctrl using the "foo-pinconf"
> compatible string and
>     your (old) kernel does not support it, the pinctrl will never probe
> the pinctrl definitions.

True.

>     Moreover, if you want to define both old ("foo-pinctrl") and new
> ("foo-pinconf") pinctrl
>     definitions in your dt in order to support several kernel versions,
> nothing prevents you
>     from doing it.

I don't know if that will work well; what happens if a kernel supports
both the old and new compatible values, so that the new kernel is
compatible with old DTs?

> 2) In the other hand, if you use an additional "generic-pinconf"
> compatible string to signify
>     wether or not the pinctrl definition use the generic pinconf dt
> binding, the (old) kernel
>     will probe the pinctrl definitions, ignore the "generic-pinconf"
> string, and fail when parsing
>     the pinctrl configuration nodes (which are invalid pinctrl function
> nodes in the current dt binding).
>     We have the same problem when using the 'atmel,generic-pinconf'
> property inside a pinctrl node:
>     old kernels won't take this property into account.

True.

[RFC PATCH 3/3] ARM: at91/dt: move sama5 to generic pinconf

[Boris BREZILLON]

Add generic pinconf definitions and reference appropriate configs in
atmel,pins properties.

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
---
 arch/arm/boot/dts/sama5d3.dtsi    |  363 +++++++++++++++++++------------------
 arch/arm/boot/dts/sama5d3xdm.dtsi |    2 +-
 arch/arm/boot/dts/sama5d3xmb.dtsi |   12 +-
 3 files changed, 197 insertions(+), 180 deletions(-)

diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index a1d5e25..3e38383 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -402,7 +402,7 @@
 			pinctrl@fffff200 {
 				#address-cells = <1>;
 				#size-cells = <1>;
-				compatible = "atmel,at91sam9x5-pinctrl", "atmel,at91rm9200-pinctrl", "simple-bus";
+				compatible = "atmel,at91sam9x5-pinctrl", "atmel,at91rm9200-pinctrl", "generic-pinconf", "simple-bus";
 				ranges = <0xfffff200 0xfffff200 0xa00>;
 				atmel,mux-mask = <
 					/*   A          B          C  */
@@ -413,206 +413,223 @@
 					0xffffffff 0xbf9f8000 0x18000000	/* pioE */
 					>;
 
+				pcfg_none: pcfg_none {
+					bias-disable;
+				};
+
+				pcfg_pull_up: pcfg_pull_up {
+					bias-pull-up;
+				};
+
+				pcfg_deglitch: pcfg_deglitch {
+					input-deglitch = <1>;
+				};
+
+				pcfg_pull_up_deglitch: pcfg_pull_up_deglitch {
+					bias-pull-up;
+					input-deglitch = <1>;
+				};
+
 				/* shared pinctrl settings */
 				adc0 {
 					pinctrl_adc0_adtrg: adc0_adtrg {
 						atmel,pins =
-							<AT91_PIOD 19 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD19 periph A ADTRG */
+							<AT91_PIOD 19 AT91_PERIPH_A &pcfg_none>;	/* PD19 periph A ADTRG */
 					};
 					pinctrl_adc0_ad0: adc0_ad0 {
 						atmel,pins =
-							<AT91_PIOD 20 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD20 periph A AD0 */
+							<AT91_PIOD 20 AT91_PERIPH_A &pcfg_none>;	/* PD20 periph A AD0 */
 					};
 					pinctrl_adc0_ad1: adc0_ad1 {
 						atmel,pins =
-							<AT91_PIOD 21 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD21 periph A AD1 */
+							<AT91_PIOD 21 AT91_PERIPH_A &pcfg_none>;	/* PD21 periph A AD1 */
 					};
 					pinctrl_adc0_ad2: adc0_ad2 {
 						atmel,pins =
-							<AT91_PIOD 22 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD22 periph A AD2 */
+							<AT91_PIOD 22 AT91_PERIPH_A &pcfg_none>;	/* PD22 periph A AD2 */
 					};
 					pinctrl_adc0_ad3: adc0_ad3 {
 						atmel,pins =
-							<AT91_PIOD 23 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD23 periph A AD3 */
+							<AT91_PIOD 23 AT91_PERIPH_A &pcfg_none>;	/* PD23 periph A AD3 */
 					};
 					pinctrl_adc0_ad4: adc0_ad4 {
 						atmel,pins =
-							<AT91_PIOD 24 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD24 periph A AD4 */
+							<AT91_PIOD 24 AT91_PERIPH_A &pcfg_none>;	/* PD24 periph A AD4 */
 					};
 					pinctrl_adc0_ad5: adc0_ad5 {
 						atmel,pins =
-							<AT91_PIOD 25 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD25 periph A AD5 */
+							<AT91_PIOD 25 AT91_PERIPH_A &pcfg_none>;	/* PD25 periph A AD5 */
 					};
 					pinctrl_adc0_ad6: adc0_ad6 {
 						atmel,pins =
-							<AT91_PIOD 26 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD26 periph A AD6 */
+							<AT91_PIOD 26 AT91_PERIPH_A &pcfg_none>;	/* PD26 periph A AD6 */
 					};
 					pinctrl_adc0_ad7: adc0_ad7 {
 						atmel,pins =
-							<AT91_PIOD 27 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD27 periph A AD7 */
+							<AT91_PIOD 27 AT91_PERIPH_A &pcfg_none>;	/* PD27 periph A AD7 */
 					};
 					pinctrl_adc0_ad8: adc0_ad8 {
 						atmel,pins =
-							<AT91_PIOD 28 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD28 periph A AD8 */
+							<AT91_PIOD 28 AT91_PERIPH_A &pcfg_none>;	/* PD28 periph A AD8 */
 					};
 					pinctrl_adc0_ad9: adc0_ad9 {
 						atmel,pins =
-							<AT91_PIOD 29 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD29 periph A AD9 */
+							<AT91_PIOD 29 AT91_PERIPH_A &pcfg_none>;	/* PD29 periph A AD9 */
 					};
 					pinctrl_adc0_ad10: adc0_ad10 {
 						atmel,pins =
-							<AT91_PIOD 30 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD30 periph A AD10, conflicts with PCK0 */
+							<AT91_PIOD 30 AT91_PERIPH_A &pcfg_none>;	/* PD30 periph A AD10, conflicts with PCK0 */
 					};
 					pinctrl_adc0_ad11: adc0_ad11 {
 						atmel,pins =
-							<AT91_PIOD 31 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD31 periph A AD11, conflicts with PCK1 */
+							<AT91_PIOD 31 AT91_PERIPH_A &pcfg_none>;	/* PD31 periph A AD11, conflicts with PCK1 */
 					};
 				};
 
 				can0 {
 					pinctrl_can0_rx_tx: can0_rx_tx {
 						atmel,pins =
-							<AT91_PIOD 14 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PD14 periph C RX, conflicts with SCK0, SPI0_NPCS1 */
-							 AT91_PIOD 15 AT91_PERIPH_C AT91_PINCTRL_NONE>;	/* PD15 periph C TX, conflicts with CTS0, SPI0_NPCS2 */
+							<AT91_PIOD 14 AT91_PERIPH_C &pcfg_none	/* PD14 periph C RX, conflicts with SCK0, SPI0_NPCS1 */
+							 AT91_PIOD 15 AT91_PERIPH_C &pcfg_none>;	/* PD15 periph C TX, conflicts with CTS0, SPI0_NPCS2 */
 					};
 				};
 
 				can1 {
 					pinctrl_can1_rx_tx: can1_rx_tx {
 						atmel,pins =
-							<AT91_PIOB 14 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB14 periph B RX, conflicts with GCRS */
-							 AT91_PIOB 15 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* PB15 periph B TX, conflicts with GCOL */
+							<AT91_PIOB 14 AT91_PERIPH_B &pcfg_none	/* PB14 periph B RX, conflicts with GCRS */
+							 AT91_PIOB 15 AT91_PERIPH_B &pcfg_none>;	/* PB15 periph B TX, conflicts with GCOL */
 					};
 				};
 
 				dbgu {
 					pinctrl_dbgu: dbgu-0 {
 						atmel,pins =
-							<AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB30 periph A */
-							 AT91_PIOB 31 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;	/* PB31 periph A with pullup */
+							<AT91_PIOB 30 AT91_PERIPH_A &pcfg_none	/* PB30 periph A */
+							 AT91_PIOB 31 AT91_PERIPH_A &pcfg_pull_up>;	/* PB31 periph A with pullup */
 					};
 				};
 
 				i2c0 {
 					pinctrl_i2c0: i2c0-0 {
 						atmel,pins =
-							<AT91_PIOA 30 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA30 periph A TWD0 pin, conflicts with URXD1, ISI_VSYNC */
-							 AT91_PIOA 31 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PA31 periph A TWCK0 pin, conflicts with UTXD1, ISI_HSYNC */
+							<AT91_PIOA 30 AT91_PERIPH_A &pcfg_none	/* PA30 periph A TWD0 pin, conflicts with URXD1, ISI_VSYNC */
+							 AT91_PIOA 31 AT91_PERIPH_A &pcfg_none>;	/* PA31 periph A TWCK0 pin, conflicts with UTXD1, ISI_HSYNC */
 					};
 				};
 
 				i2c1 {
 					pinctrl_i2c1: i2c1-0 {
 						atmel,pins =
-							<AT91_PIOC 26 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PC26 periph B TWD1 pin, conflicts with SPI1_NPCS1, ISI_D11 */
-							 AT91_PIOC 27 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* PC27 periph B TWCK1 pin, conflicts with SPI1_NPCS2, ISI_D10 */
+							<AT91_PIOC 26 AT91_PERIPH_B &pcfg_none	/* PC26 periph B TWD1 pin, conflicts with SPI1_NPCS1, ISI_D11 */
+							 AT91_PIOC 27 AT91_PERIPH_B &pcfg_none>;	/* PC27 periph B TWCK1 pin, conflicts with SPI1_NPCS2, ISI_D10 */
 					};
 				};
 
 				isi {
 					pinctrl_isi: isi-0 {
 						atmel,pins =
-							<AT91_PIOA 16 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PA16 periph C ISI_D0, conflicts with LCDDAT16 */
-							 AT91_PIOA 17 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PA17 periph C ISI_D1, conflicts with LCDDAT17 */
-							 AT91_PIOA 18 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PA18 periph C ISI_D2, conflicts with LCDDAT18, TWD2 */
-							 AT91_PIOA 19 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PA19 periph C ISI_D3, conflicts with LCDDAT19, TWCK2 */
-							 AT91_PIOA 20 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PA20 periph C ISI_D4, conflicts with LCDDAT20, PWMH0 */
-							 AT91_PIOA 21 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PA21 periph C ISI_D5, conflicts with LCDDAT21, PWML0 */
-							 AT91_PIOA 22 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PA22 periph C ISI_D6, conflicts with LCDDAT22, PWMH1 */
-							 AT91_PIOA 23 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PA23 periph C ISI_D7, conflicts with LCDDAT23, PWML1 */
-							 AT91_PIOC 30 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PC30 periph C ISI_PCK, conflicts with UTXD0 */
-							 AT91_PIOA 31 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PA31 periph C ISI_HSYNC, conflicts with TWCK0, UTXD1 */
-							 AT91_PIOA 30 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PA30 periph C ISI_VSYNC, conflicts with TWD0, URXD1 */
-							 AT91_PIOC 29 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PC29 periph C ISI_PD8, conflicts with URXD0, PWMFI2 */
-							 AT91_PIOC 28 AT91_PERIPH_C AT91_PINCTRL_NONE>;	/* PC28 periph C ISI_PD9, conflicts with SPI1_NPCS3, PWMFI0 */
+							<AT91_PIOA 16 AT91_PERIPH_C &pcfg_none	/* PA16 periph C ISI_D0, conflicts with LCDDAT16 */
+							 AT91_PIOA 17 AT91_PERIPH_C &pcfg_none	/* PA17 periph C ISI_D1, conflicts with LCDDAT17 */
+							 AT91_PIOA 18 AT91_PERIPH_C &pcfg_none	/* PA18 periph C ISI_D2, conflicts with LCDDAT18, TWD2 */
+							 AT91_PIOA 19 AT91_PERIPH_C &pcfg_none	/* PA19 periph C ISI_D3, conflicts with LCDDAT19, TWCK2 */
+							 AT91_PIOA 20 AT91_PERIPH_C &pcfg_none	/* PA20 periph C ISI_D4, conflicts with LCDDAT20, PWMH0 */
+							 AT91_PIOA 21 AT91_PERIPH_C &pcfg_none	/* PA21 periph C ISI_D5, conflicts with LCDDAT21, PWML0 */
+							 AT91_PIOA 22 AT91_PERIPH_C &pcfg_none	/* PA22 periph C ISI_D6, conflicts with LCDDAT22, PWMH1 */
+							 AT91_PIOA 23 AT91_PERIPH_C &pcfg_none	/* PA23 periph C ISI_D7, conflicts with LCDDAT23, PWML1 */
+							 AT91_PIOC 30 AT91_PERIPH_C &pcfg_none	/* PC30 periph C ISI_PCK, conflicts with UTXD0 */
+							 AT91_PIOA 31 AT91_PERIPH_C &pcfg_none	/* PA31 periph C ISI_HSYNC, conflicts with TWCK0, UTXD1 */
+							 AT91_PIOA 30 AT91_PERIPH_C &pcfg_none	/* PA30 periph C ISI_VSYNC, conflicts with TWD0, URXD1 */
+							 AT91_PIOC 29 AT91_PERIPH_C &pcfg_none	/* PC29 periph C ISI_PD8, conflicts with URXD0, PWMFI2 */
+							 AT91_PIOC 28 AT91_PERIPH_C &pcfg_none>;	/* PC28 periph C ISI_PD9, conflicts with SPI1_NPCS3, PWMFI0 */
 					};
 					pinctrl_isi_pck_as_mck: isi_pck_as_mck-0 {
 						atmel,pins =
-							<AT91_PIOD 31 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* PD31 periph B ISI_MCK */
+							<AT91_PIOD 31 AT91_PERIPH_B &pcfg_none>;	/* PD31 periph B ISI_MCK */
 					};
 				};
 
 				lcd {
 					pinctrl_lcd: lcd-0 {
 						atmel,pins =
-							<AT91_PIOA 24 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA24 periph A LCDPWM */
-							 AT91_PIOA 26 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA26 periph A LCDVSYNC */
-							 AT91_PIOA 27 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA27 periph A LCDHSYNC */
-							 AT91_PIOA 25 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA25 periph A LCDDISP */
-							 AT91_PIOA 29 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA29 periph A LCDDEN */
-							 AT91_PIOA 28 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA28 periph A LCDPCK */
-							 AT91_PIOA 0 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA0 periph A LCDD0 pin */
-							 AT91_PIOA 1 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA1 periph A LCDD1 pin */
-							 AT91_PIOA 2 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA2 periph A LCDD2 pin */
-							 AT91_PIOA 3 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA3 periph A LCDD3 pin */
-							 AT91_PIOA 4 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA4 periph A LCDD4 pin */
-							 AT91_PIOA 5 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA5 periph A LCDD5 pin */
-							 AT91_PIOA 6 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA6 periph A LCDD6 pin */
-							 AT91_PIOA 7 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA7 periph A LCDD7 pin */
-							 AT91_PIOA 8 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA8 periph A LCDD8 pin */
-							 AT91_PIOA 9 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA9 periph A LCDD9 pin */
-							 AT91_PIOA 10 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA10 periph A LCDD10 pin */
-							 AT91_PIOA 11 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA11 periph A LCDD11 pin */
-							 AT91_PIOA 12 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA12 periph A LCDD12 pin */
-							 AT91_PIOA 13 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA13 periph A LCDD13 pin */
-							 AT91_PIOA 14 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA14 periph A LCDD14 pin */
-							 AT91_PIOA 15 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA15 periph A LCDD15 pin */
-							 AT91_PIOC 14 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PC14 periph C LCDD16 pin */
-							 AT91_PIOC 13 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PC13 periph C LCDD17 pin */
-							 AT91_PIOC 12 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PC12 periph C LCDD18 pin */
-							 AT91_PIOC 11 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PC11 periph C LCDD19 pin */
-							 AT91_PIOC 10 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PC10 periph C LCDD20 pin */
-							 AT91_PIOC 15 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PC15 periph C LCDD21 pin */
-							 AT91_PIOE 27 AT91_PERIPH_C AT91_PINCTRL_NONE	/* PE27 periph C LCDD22 pin */
-							 AT91_PIOE 28 AT91_PERIPH_C AT91_PINCTRL_NONE>;	/* PE28 periph C LCDD23 pin */
+							<AT91_PIOA 24 AT91_PERIPH_A &pcfg_none	/* PA24 periph A LCDPWM */
+							 AT91_PIOA 26 AT91_PERIPH_A &pcfg_none	/* PA26 periph A LCDVSYNC */
+							 AT91_PIOA 27 AT91_PERIPH_A &pcfg_none	/* PA27 periph A LCDHSYNC */
+							 AT91_PIOA 25 AT91_PERIPH_A &pcfg_none	/* PA25 periph A LCDDISP */
+							 AT91_PIOA 29 AT91_PERIPH_A &pcfg_none	/* PA29 periph A LCDDEN */
+							 AT91_PIOA 28 AT91_PERIPH_A &pcfg_none	/* PA28 periph A LCDPCK */
+							 AT91_PIOA 0 AT91_PERIPH_A &pcfg_none	/* PA0 periph A LCDD0 pin */
+							 AT91_PIOA 1 AT91_PERIPH_A &pcfg_none	/* PA1 periph A LCDD1 pin */
+							 AT91_PIOA 2 AT91_PERIPH_A &pcfg_none	/* PA2 periph A LCDD2 pin */
+							 AT91_PIOA 3 AT91_PERIPH_A &pcfg_none	/* PA3 periph A LCDD3 pin */
+							 AT91_PIOA 4 AT91_PERIPH_A &pcfg_none	/* PA4 periph A LCDD4 pin */
+							 AT91_PIOA 5 AT91_PERIPH_A &pcfg_none	/* PA5 periph A LCDD5 pin */
+							 AT91_PIOA 6 AT91_PERIPH_A &pcfg_none	/* PA6 periph A LCDD6 pin */
+							 AT91_PIOA 7 AT91_PERIPH_A &pcfg_none	/* PA7 periph A LCDD7 pin */
+							 AT91_PIOA 8 AT91_PERIPH_A &pcfg_none	/* PA8 periph A LCDD8 pin */
+							 AT91_PIOA 9 AT91_PERIPH_A &pcfg_none	/* PA9 periph A LCDD9 pin */
+							 AT91_PIOA 10 AT91_PERIPH_A &pcfg_none	/* PA10 periph A LCDD10 pin */
+							 AT91_PIOA 11 AT91_PERIPH_A &pcfg_none	/* PA11 periph A LCDD11 pin */
+							 AT91_PIOA 12 AT91_PERIPH_A &pcfg_none	/* PA12 periph A LCDD12 pin */
+							 AT91_PIOA 13 AT91_PERIPH_A &pcfg_none	/* PA13 periph A LCDD13 pin */
+							 AT91_PIOA 14 AT91_PERIPH_A &pcfg_none	/* PA14 periph A LCDD14 pin */
+							 AT91_PIOA 15 AT91_PERIPH_A &pcfg_none	/* PA15 periph A LCDD15 pin */
+							 AT91_PIOC 14 AT91_PERIPH_C &pcfg_none	/* PC14 periph C LCDD16 pin */
+							 AT91_PIOC 13 AT91_PERIPH_C &pcfg_none	/* PC13 periph C LCDD17 pin */
+							 AT91_PIOC 12 AT91_PERIPH_C &pcfg_none	/* PC12 periph C LCDD18 pin */
+							 AT91_PIOC 11 AT91_PERIPH_C &pcfg_none	/* PC11 periph C LCDD19 pin */
+							 AT91_PIOC 10 AT91_PERIPH_C &pcfg_none	/* PC10 periph C LCDD20 pin */
+							 AT91_PIOC 15 AT91_PERIPH_C &pcfg_none	/* PC15 periph C LCDD21 pin */
+							 AT91_PIOE 27 AT91_PERIPH_C &pcfg_none	/* PE27 periph C LCDD22 pin */
+							 AT91_PIOE 28 AT91_PERIPH_C &pcfg_none>;	/* PE28 periph C LCDD23 pin */
 					};
 				};
 
 				macb0 {
 					pinctrl_macb0_data_rgmii: macb0_data_rgmii {
 						atmel,pins =
-							<AT91_PIOB 0 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB0 periph A GTX0, conflicts with PWMH0 */
-							 AT91_PIOB 1 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB1 periph A GTX1, conflicts with PWML0 */
-							 AT91_PIOB 2 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB2 periph A GTX2, conflicts with TK1 */
-							 AT91_PIOB 3 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB3 periph A GTX3, conflicts with TF1 */
-							 AT91_PIOB 4 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB4 periph A GRX0, conflicts with PWMH1 */
-							 AT91_PIOB 5 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB5 periph A GRX1, conflicts with PWML1 */
-							 AT91_PIOB 6 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB6 periph A GRX2, conflicts with TD1 */
-							 AT91_PIOB 7 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PB7 periph A GRX3, conflicts with RK1 */
+							<AT91_PIOB 0 AT91_PERIPH_A &pcfg_none	/* PB0 periph A GTX0, conflicts with PWMH0 */
+							 AT91_PIOB 1 AT91_PERIPH_A &pcfg_none	/* PB1 periph A GTX1, conflicts with PWML0 */
+							 AT91_PIOB 2 AT91_PERIPH_A &pcfg_none	/* PB2 periph A GTX2, conflicts with TK1 */
+							 AT91_PIOB 3 AT91_PERIPH_A &pcfg_none	/* PB3 periph A GTX3, conflicts with TF1 */
+							 AT91_PIOB 4 AT91_PERIPH_A &pcfg_none	/* PB4 periph A GRX0, conflicts with PWMH1 */
+							 AT91_PIOB 5 AT91_PERIPH_A &pcfg_none	/* PB5 periph A GRX1, conflicts with PWML1 */
+							 AT91_PIOB 6 AT91_PERIPH_A &pcfg_none	/* PB6 periph A GRX2, conflicts with TD1 */
+							 AT91_PIOB 7 AT91_PERIPH_A &pcfg_none>;	/* PB7 periph A GRX3, conflicts with RK1 */
 					};
 					pinctrl_macb0_data_gmii: macb0_data_gmii {
 						atmel,pins =
-							<AT91_PIOB 19 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB19 periph B GTX4, conflicts with MCI1_CDA */
-							 AT91_PIOB 20 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB20 periph B GTX5, conflicts with MCI1_DA0 */
-							 AT91_PIOB 21 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB21 periph B GTX6, conflicts with MCI1_DA1 */
-							 AT91_PIOB 22 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB22 periph B GTX7, conflicts with MCI1_DA2 */
-							 AT91_PIOB 23 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB23 periph B GRX4, conflicts with MCI1_DA3 */
-							 AT91_PIOB 24 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB24 periph B GRX5, conflicts with MCI1_CK */
-							 AT91_PIOB 25 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB25 periph B GRX6, conflicts with SCK1 */
-							 AT91_PIOB 26 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* PB26 periph B GRX7, conflicts with CTS1 */
+							<AT91_PIOB 19 AT91_PERIPH_B &pcfg_none	/* PB19 periph B GTX4, conflicts with MCI1_CDA */
+							 AT91_PIOB 20 AT91_PERIPH_B &pcfg_none	/* PB20 periph B GTX5, conflicts with MCI1_DA0 */
+							 AT91_PIOB 21 AT91_PERIPH_B &pcfg_none	/* PB21 periph B GTX6, conflicts with MCI1_DA1 */
+							 AT91_PIOB 22 AT91_PERIPH_B &pcfg_none	/* PB22 periph B GTX7, conflicts with MCI1_DA2 */
+							 AT91_PIOB 23 AT91_PERIPH_B &pcfg_none	/* PB23 periph B GRX4, conflicts with MCI1_DA3 */
+							 AT91_PIOB 24 AT91_PERIPH_B &pcfg_none	/* PB24 periph B GRX5, conflicts with MCI1_CK */
+							 AT91_PIOB 25 AT91_PERIPH_B &pcfg_none	/* PB25 periph B GRX6, conflicts with SCK1 */
+							 AT91_PIOB 26 AT91_PERIPH_B &pcfg_none>;	/* PB26 periph B GRX7, conflicts with CTS1 */
 					};
 					pinctrl_macb0_signal_rgmii: macb0_signal_rgmii {
 						atmel,pins =
-							<AT91_PIOB 8 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB8 periph A GTXCK, conflicts with PWMH2 */
-							 AT91_PIOB 9 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB9 periph A GTXEN, conflicts with PWML2 */
-							 AT91_PIOB 11 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB11 periph A GRXCK, conflicts with RD1 */
-							 AT91_PIOB 13 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB13 periph A GRXER, conflicts with PWML3 */
-							 AT91_PIOB 16 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB16 periph A GMDC */
-							 AT91_PIOB 17 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB17 periph A GMDIO */
-							 AT91_PIOB 18 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PB18 periph A G125CK */
+							<AT91_PIOB 8 AT91_PERIPH_A &pcfg_none	/* PB8 periph A GTXCK, conflicts with PWMH2 */
+							 AT91_PIOB 9 AT91_PERIPH_A &pcfg_none	/* PB9 periph A GTXEN, conflicts with PWML2 */
+							 AT91_PIOB 11 AT91_PERIPH_A &pcfg_none	/* PB11 periph A GRXCK, conflicts with RD1 */
+							 AT91_PIOB 13 AT91_PERIPH_A &pcfg_none	/* PB13 periph A GRXER, conflicts with PWML3 */
+							 AT91_PIOB 16 AT91_PERIPH_A &pcfg_none	/* PB16 periph A GMDC */
+							 AT91_PIOB 17 AT91_PERIPH_A &pcfg_none	/* PB17 periph A GMDIO */
+							 AT91_PIOB 18 AT91_PERIPH_A &pcfg_none>;	/* PB18 periph A G125CK */
 					};
 					pinctrl_macb0_signal_gmii: macb0_signal_gmii {
 						atmel,pins =
-							<AT91_PIOB 9 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB9 periph A GTXEN, conflicts with PWML2 */
-							 AT91_PIOB 10 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB10 periph A GTXER, conflicts with RF1 */
-							 AT91_PIOB 11 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB11 periph A GRXCK, conflicts with RD1 */
-							 AT91_PIOB 12 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB12 periph A GRXDV, conflicts with PWMH3 */
-							 AT91_PIOB 13 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB13 periph A GRXER, conflicts with PWML3 */
-							 AT91_PIOB 14 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB14 periph A GCRS, conflicts with CANRX1 */
-							 AT91_PIOB 15 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB15 periph A GCOL, conflicts with CANTX1 */
-							 AT91_PIOB 16 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB16 periph A GMDC */
-							 AT91_PIOB 17 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB17 periph A GMDIO */
-							 AT91_PIOB 27 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* PB27 periph B G125CKO */
+							<AT91_PIOB 9 AT91_PERIPH_A &pcfg_none	/* PB9 periph A GTXEN, conflicts with PWML2 */
+							 AT91_PIOB 10 AT91_PERIPH_A &pcfg_none	/* PB10 periph A GTXER, conflicts with RF1 */
+							 AT91_PIOB 11 AT91_PERIPH_A &pcfg_none	/* PB11 periph A GRXCK, conflicts with RD1 */
+							 AT91_PIOB 12 AT91_PERIPH_A &pcfg_none	/* PB12 periph A GRXDV, conflicts with PWMH3 */
+							 AT91_PIOB 13 AT91_PERIPH_A &pcfg_none	/* PB13 periph A GRXER, conflicts with PWML3 */
+							 AT91_PIOB 14 AT91_PERIPH_A &pcfg_none	/* PB14 periph A GCRS, conflicts with CANRX1 */
+							 AT91_PIOB 15 AT91_PERIPH_A &pcfg_none	/* PB15 periph A GCOL, conflicts with CANTX1 */
+							 AT91_PIOB 16 AT91_PERIPH_A &pcfg_none	/* PB16 periph A GMDC */
+							 AT91_PIOB 17 AT91_PERIPH_A &pcfg_none	/* PB17 periph A GMDIO */
+							 AT91_PIOB 27 AT91_PERIPH_B &pcfg_none>;	/* PB27 periph B G125CKO */
 					};
 
 				};
@@ -620,198 +637,198 @@
 				macb1 {
 					pinctrl_macb1_rmii: macb1_rmii-0 {
 						atmel,pins =
-							<AT91_PIOC 0 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC0 periph A ETX0, conflicts with TIOA3 */
-							 AT91_PIOC 1 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC1 periph A ETX1, conflicts with TIOB3 */
-							 AT91_PIOC 2 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC2 periph A ERX0, conflicts with TCLK3 */
-							 AT91_PIOC 3 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC3 periph A ERX1, conflicts with TIOA4 */
-							 AT91_PIOC 4 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC4 periph A ETXEN, conflicts with TIOB4 */
-							 AT91_PIOC 5 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC5 periph A ECRSDV,conflicts with TCLK4 */
-							 AT91_PIOC 6 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC6 periph A ERXER, conflicts with TIOA5 */
-							 AT91_PIOC 7 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC7 periph A EREFCK, conflicts with TIOB5 */
-							 AT91_PIOC 8 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC8 periph A EMDC, conflicts with TCLK5 */
-							 AT91_PIOC 9 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PC9 periph A EMDIO  */
+							<AT91_PIOC 0 AT91_PERIPH_A &pcfg_none	/* PC0 periph A ETX0, conflicts with TIOA3 */
+							 AT91_PIOC 1 AT91_PERIPH_A &pcfg_none	/* PC1 periph A ETX1, conflicts with TIOB3 */
+							 AT91_PIOC 2 AT91_PERIPH_A &pcfg_none	/* PC2 periph A ERX0, conflicts with TCLK3 */
+							 AT91_PIOC 3 AT91_PERIPH_A &pcfg_none	/* PC3 periph A ERX1, conflicts with TIOA4 */
+							 AT91_PIOC 4 AT91_PERIPH_A &pcfg_none	/* PC4 periph A ETXEN, conflicts with TIOB4 */
+							 AT91_PIOC 5 AT91_PERIPH_A &pcfg_none	/* PC5 periph A ECRSDV,conflicts with TCLK4 */
+							 AT91_PIOC 6 AT91_PERIPH_A &pcfg_none	/* PC6 periph A ERXER, conflicts with TIOA5 */
+							 AT91_PIOC 7 AT91_PERIPH_A &pcfg_none	/* PC7 periph A EREFCK, conflicts with TIOB5 */
+							 AT91_PIOC 8 AT91_PERIPH_A &pcfg_none	/* PC8 periph A EMDC, conflicts with TCLK5 */
+							 AT91_PIOC 9 AT91_PERIPH_A &pcfg_none>;	/* PC9 periph A EMDIO  */
 					};
 				};
 
 				mmc0 {
 					pinctrl_mmc0_clk_cmd_dat0: mmc0_clk_cmd_dat0 {
 						atmel,pins =
-							<AT91_PIOD 9 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PD9 periph A MCI0_CK */
-							 AT91_PIOD 0 AT91_PERIPH_A AT91_PINCTRL_PULL_UP	/* PD0 periph A MCI0_CDA with pullup */
-							 AT91_PIOD 1 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;	/* PD1 periph A MCI0_DA0 with pullup */
+							<AT91_PIOD 9 AT91_PERIPH_A &pcfg_none	/* PD9 periph A MCI0_CK */
+							 AT91_PIOD 0 AT91_PERIPH_A &pcfg_pull_up	/* PD0 periph A MCI0_CDA with pullup */
+							 AT91_PIOD 1 AT91_PERIPH_A &pcfg_pull_up>;	/* PD1 periph A MCI0_DA0 with pullup */
 					};
 					pinctrl_mmc0_dat1_3: mmc0_dat1_3 {
 						atmel,pins =
-							<AT91_PIOD 2 AT91_PERIPH_A AT91_PINCTRL_PULL_UP	/* PD2 periph A MCI0_DA1 with pullup */
-							 AT91_PIOD 3 AT91_PERIPH_A AT91_PINCTRL_PULL_UP	/* PD3 periph A MCI0_DA2 with pullup */
-							 AT91_PIOD 4 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;	/* PD4 periph A MCI0_DA3 with pullup */
+							<AT91_PIOD 2 AT91_PERIPH_A &pcfg_pull_up	/* PD2 periph A MCI0_DA1 with pullup */
+							 AT91_PIOD 3 AT91_PERIPH_A &pcfg_pull_up	/* PD3 periph A MCI0_DA2 with pullup */
+							 AT91_PIOD 4 AT91_PERIPH_A &pcfg_pull_up>;	/* PD4 periph A MCI0_DA3 with pullup */
 					};
 					pinctrl_mmc0_dat4_7: mmc0_dat4_7 {
 						atmel,pins =
-							<AT91_PIOD 5 AT91_PERIPH_A AT91_PINCTRL_PULL_UP	/* PD5 periph A MCI0_DA4 with pullup, conflicts with TIOA0, PWMH2 */
-							 AT91_PIOD 6 AT91_PERIPH_A AT91_PINCTRL_PULL_UP	/* PD6 periph A MCI0_DA5 with pullup, conflicts with TIOB0, PWML2 */
-							 AT91_PIOD 7 AT91_PERIPH_A AT91_PINCTRL_PULL_UP	/* PD7 periph A MCI0_DA6 with pullup, conlicts with TCLK0, PWMH3 */
-							 AT91_PIOD 8 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;	/* PD8 periph A MCI0_DA7 with pullup, conflicts with PWML3 */
+							<AT91_PIOD 5 AT91_PERIPH_A &pcfg_pull_up	/* PD5 periph A MCI0_DA4 with pullup, conflicts with TIOA0, PWMH2 */
+							 AT91_PIOD 6 AT91_PERIPH_A &pcfg_pull_up	/* PD6 periph A MCI0_DA5 with pullup, conflicts with TIOB0, PWML2 */
+							 AT91_PIOD 7 AT91_PERIPH_A &pcfg_pull_up	/* PD7 periph A MCI0_DA6 with pullup, conlicts with TCLK0, PWMH3 */
+							 AT91_PIOD 8 AT91_PERIPH_A &pcfg_pull_up>;	/* PD8 periph A MCI0_DA7 with pullup, conflicts with PWML3 */
 					};
 				};
 
 				mmc1 {
 					pinctrl_mmc1_clk_cmd_dat0: mmc1_clk_cmd_dat0 {
 						atmel,pins =
-							<AT91_PIOB 24 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB24 periph A MCI1_CK, conflicts with GRX5 */
-							 AT91_PIOB 19 AT91_PERIPH_A AT91_PINCTRL_PULL_UP	/* PB19 periph A MCI1_CDA with pullup, conflicts with GTX4 */
-							 AT91_PIOB 20 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;	/* PB20 periph A MCI1_DA0 with pullup, conflicts with GTX5 */
+							<AT91_PIOB 24 AT91_PERIPH_A &pcfg_none	/* PB24 periph A MCI1_CK, conflicts with GRX5 */
+							 AT91_PIOB 19 AT91_PERIPH_A &pcfg_pull_up	/* PB19 periph A MCI1_CDA with pullup, conflicts with GTX4 */
+							 AT91_PIOB 20 AT91_PERIPH_A &pcfg_pull_up>;	/* PB20 periph A MCI1_DA0 with pullup, conflicts with GTX5 */
 					};
 					pinctrl_mmc1_dat1_3: mmc1_dat1_3 {
 						atmel,pins =
-							<AT91_PIOB 21 AT91_PERIPH_A AT91_PINCTRL_PULL_UP	/* PB21 periph A MCI1_DA1 with pullup, conflicts with GTX6 */
-							 AT91_PIOB 22 AT91_PERIPH_A AT91_PINCTRL_PULL_UP	/* PB22 periph A MCI1_DA2 with pullup, conflicts with GTX7 */
-							 AT91_PIOB 23 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;	/* PB23 periph A MCI1_DA3 with pullup, conflicts with GRX4 */
+							<AT91_PIOB 21 AT91_PERIPH_A &pcfg_pull_up	/* PB21 periph A MCI1_DA1 with pullup, conflicts with GTX6 */
+							 AT91_PIOB 22 AT91_PERIPH_A &pcfg_pull_up	/* PB22 periph A MCI1_DA2 with pullup, conflicts with GTX7 */
+							 AT91_PIOB 23 AT91_PERIPH_A &pcfg_pull_up>;	/* PB23 periph A MCI1_DA3 with pullup, conflicts with GRX4 */
 					};
 				};
 
 				mmc2 {
 					pinctrl_mmc2_clk_cmd_dat0: mmc2_clk_cmd_dat0 {
 						atmel,pins =
-							<AT91_PIOC 15 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC15 periph A MCI2_CK, conflicts with PCK2 */
-							 AT91_PIOC 10 AT91_PERIPH_A AT91_PINCTRL_PULL_UP	/* PC10 periph A MCI2_CDA with pullup */
-							 AT91_PIOC 11 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;	/* PC11 periph A MCI2_DA0 with pullup */
+							<AT91_PIOC 15 AT91_PERIPH_A &pcfg_none	/* PC15 periph A MCI2_CK, conflicts with PCK2 */
+							 AT91_PIOC 10 AT91_PERIPH_A &pcfg_pull_up	/* PC10 periph A MCI2_CDA with pullup */
+							 AT91_PIOC 11 AT91_PERIPH_A &pcfg_pull_up>;	/* PC11 periph A MCI2_DA0 with pullup */
 					};
 					pinctrl_mmc2_dat1_3: mmc2_dat1_3 {
 						atmel,pins =
-							<AT91_PIOC 12 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC12 periph A MCI2_DA1 with pullup, conflicts with TIOA1 */
-							 AT91_PIOC 13 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC13 periph A MCI2_DA2 with pullup, conflicts with TIOB1 */
-							 AT91_PIOC 14 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PC14 periph A MCI2_DA3 with pullup, conflicts with TCLK1 */
+							<AT91_PIOC 12 AT91_PERIPH_A &pcfg_none	/* PC12 periph A MCI2_DA1 with pullup, conflicts with TIOA1 */
+							 AT91_PIOC 13 AT91_PERIPH_A &pcfg_none	/* PC13 periph A MCI2_DA2 with pullup, conflicts with TIOB1 */
+							 AT91_PIOC 14 AT91_PERIPH_A &pcfg_none>;	/* PC14 periph A MCI2_DA3 with pullup, conflicts with TCLK1 */
 					};
 				};
 
 				nand0 {
 					pinctrl_nand0_ale_cle: nand0_ale_cle-0 {
 						atmel,pins =
-							<AT91_PIOE 21 AT91_PERIPH_A AT91_PINCTRL_PULL_UP	/* PE21 periph A with pullup */
-							 AT91_PIOE 22 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;	/* PE22 periph A with pullup */
+							<AT91_PIOE 21 AT91_PERIPH_A &pcfg_pull_up	/* PE21 periph A with pullup */
+							 AT91_PIOE 22 AT91_PERIPH_A &pcfg_pull_up>;	/* PE22 periph A with pullup */
 					};
 				};
 
 				spi0 {
 					pinctrl_spi0: spi0-0 {
 						atmel,pins =
-							<AT91_PIOD 10 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PD10 periph A SPI0_MISO pin */
-							 AT91_PIOD 11 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PD11 periph A SPI0_MOSI pin */
-							 AT91_PIOD 12 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD12 periph A SPI0_SPCK pin */
+							<AT91_PIOD 10 AT91_PERIPH_A &pcfg_none	/* PD10 periph A SPI0_MISO pin */
+							 AT91_PIOD 11 AT91_PERIPH_A &pcfg_none	/* PD11 periph A SPI0_MOSI pin */
+							 AT91_PIOD 12 AT91_PERIPH_A &pcfg_none>;	/* PD12 periph A SPI0_SPCK pin */
 					};
 				};
 
 				spi1 {
 					pinctrl_spi1: spi1-0 {
 						atmel,pins =
-							<AT91_PIOC 22 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC22 periph A SPI1_MISO pin */
-							 AT91_PIOC 23 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC23 periph A SPI1_MOSI pin */
-							 AT91_PIOC 24 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PC24 periph A SPI1_SPCK pin */
+							<AT91_PIOC 22 AT91_PERIPH_A &pcfg_none	/* PC22 periph A SPI1_MISO pin */
+							 AT91_PIOC 23 AT91_PERIPH_A &pcfg_none	/* PC23 periph A SPI1_MOSI pin */
+							 AT91_PIOC 24 AT91_PERIPH_A &pcfg_none>;	/* PC24 periph A SPI1_SPCK pin */
 					};
 				};
 
 				ssc0 {
 					pinctrl_ssc0_tx: ssc0_tx {
 						atmel,pins =
-							<AT91_PIOC 16 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC16 periph A TK0 */
-							 AT91_PIOC 17 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC17 periph A TF0 */
-							 AT91_PIOC 18 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PC18 periph A TD0 */
+							<AT91_PIOC 16 AT91_PERIPH_A &pcfg_none	/* PC16 periph A TK0 */
+							 AT91_PIOC 17 AT91_PERIPH_A &pcfg_none	/* PC17 periph A TF0 */
+							 AT91_PIOC 18 AT91_PERIPH_A &pcfg_none>;	/* PC18 periph A TD0 */
 					};
 
 					pinctrl_ssc0_rx: ssc0_rx {
 						atmel,pins =
-							<AT91_PIOC 19 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC19 periph A RK0 */
-							 AT91_PIOC 20 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC20 periph A RF0 */
-							 AT91_PIOC 21 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PC21 periph A RD0 */
+							<AT91_PIOC 19 AT91_PERIPH_A &pcfg_none	/* PC19 periph A RK0 */
+							 AT91_PIOC 20 AT91_PERIPH_A &pcfg_none	/* PC20 periph A RF0 */
+							 AT91_PIOC 21 AT91_PERIPH_A &pcfg_none>;	/* PC21 periph A RD0 */
 					};
 				};
 
 				ssc1 {
 					pinctrl_ssc1_tx: ssc1_tx {
 						atmel,pins =
-							<AT91_PIOB 2 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB2 periph B TK1, conflicts with GTX2 */
-							 AT91_PIOB 3 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB3 periph B TF1, conflicts with GTX3 */
-							 AT91_PIOB 6 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* PB6 periph B TD1, conflicts with TD1 */
+							<AT91_PIOB 2 AT91_PERIPH_B &pcfg_none	/* PB2 periph B TK1, conflicts with GTX2 */
+							 AT91_PIOB 3 AT91_PERIPH_B &pcfg_none	/* PB3 periph B TF1, conflicts with GTX3 */
+							 AT91_PIOB 6 AT91_PERIPH_B &pcfg_none>;	/* PB6 periph B TD1, conflicts with TD1 */
 					};
 
 					pinctrl_ssc1_rx: ssc1_rx {
 						atmel,pins =
-							<AT91_PIOB 7 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB7 periph B RK1, conflicts with EREFCK */
-							 AT91_PIOB 10 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PB10 periph B RF1, conflicts with GTXER */
-							 AT91_PIOB 11 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* PB11 periph B RD1, conflicts with GRXCK */
+							<AT91_PIOB 7 AT91_PERIPH_B &pcfg_none	/* PB7 periph B RK1, conflicts with EREFCK */
+							 AT91_PIOB 10 AT91_PERIPH_B &pcfg_none	/* PB10 periph B RF1, conflicts with GTXER */
+							 AT91_PIOB 11 AT91_PERIPH_B &pcfg_none>;	/* PB11 periph B RD1, conflicts with GRXCK */
 					};
 				};
 
 				uart0 {
 					pinctrl_uart0: uart0-0 {
 						atmel,pins =
-							<AT91_PIOC 29 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC29 periph A, conflicts with PWMFI2, ISI_D8 */
-							 AT91_PIOC 30 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;	/* PC30 periph A with pullup, conflicts with ISI_PCK */
+							<AT91_PIOC 29 AT91_PERIPH_A &pcfg_none	/* PC29 periph A, conflicts with PWMFI2, ISI_D8 */
+							 AT91_PIOC 30 AT91_PERIPH_A &pcfg_pull_up>;	/* PC30 periph A with pullup, conflicts with ISI_PCK */
 					};
 				};
 
 				uart1 {
 					pinctrl_uart1: uart1-0 {
 						atmel,pins =
-							<AT91_PIOA 30 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PA30 periph B, conflicts with TWD0, ISI_VSYNC */
-							 AT91_PIOA 31 AT91_PERIPH_B AT91_PINCTRL_PULL_UP>;	/* PA31 periph B with pullup, conflicts with TWCK0, ISI_HSYNC */
+							<AT91_PIOA 30 AT91_PERIPH_B &pcfg_none	/* PA30 periph B, conflicts with TWD0, ISI_VSYNC */
+							 AT91_PIOA 31 AT91_PERIPH_B &pcfg_pull_up>;	/* PA31 periph B with pullup, conflicts with TWCK0, ISI_HSYNC */
 					};
 				};
 
 				usart0 {
 					pinctrl_usart0: usart0-0 {
 						atmel,pins =
-							<AT91_PIOD 17 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PD17 periph A */
-							 AT91_PIOD 18 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;	/* PD18 periph A with pullup */
+							<AT91_PIOD 17 AT91_PERIPH_A &pcfg_none	/* PD17 periph A */
+							 AT91_PIOD 18 AT91_PERIPH_A &pcfg_pull_up>;	/* PD18 periph A with pullup */
 					};
 
 					pinctrl_usart0_rts_cts: usart0_rts_cts-0 {
 						atmel,pins =
-							<AT91_PIOD 15 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PD15 periph A, conflicts with SPI0_NPCS2, CANTX0 */
-							 AT91_PIOD 16 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PD16 periph A, conflicts with SPI0_NPCS3, PWMFI3 */
+							<AT91_PIOD 15 AT91_PERIPH_A &pcfg_none	/* PD15 periph A, conflicts with SPI0_NPCS2, CANTX0 */
+							 AT91_PIOD 16 AT91_PERIPH_A &pcfg_none>;	/* PD16 periph A, conflicts with SPI0_NPCS3, PWMFI3 */
 					};
 				};
 
 				usart1 {
 					pinctrl_usart1: usart1-0 {
 						atmel,pins =
-							<AT91_PIOB 28 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB28 periph A */
-							 AT91_PIOB 29 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;	/* PB29 periph A with pullup */
+							<AT91_PIOB 28 AT91_PERIPH_A &pcfg_none	/* PB28 periph A */
+							 AT91_PIOB 29 AT91_PERIPH_A &pcfg_pull_up>;	/* PB29 periph A with pullup */
 					};
 
 					pinctrl_usart1_rts_cts: usart1_rts_cts-0 {
 						atmel,pins =
-							<AT91_PIOB 26 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PB26 periph A, conflicts with GRX7 */
-							 AT91_PIOB 27 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PB27 periph A, conflicts with G125CKO */
+							<AT91_PIOB 26 AT91_PERIPH_A &pcfg_none	/* PB26 periph A, conflicts with GRX7 */
+							 AT91_PIOB 27 AT91_PERIPH_A &pcfg_none>;	/* PB27 periph A, conflicts with G125CKO */
 					};
 				};
 
 				usart2 {
 					pinctrl_usart2: usart2-0 {
 						atmel,pins =
-							<AT91_PIOE 25 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PE25 periph B, conflicts with A25 */
-							 AT91_PIOE 26 AT91_PERIPH_B AT91_PINCTRL_PULL_UP>;	/* PE26 periph B with pullup, conflicts NCS0 */
+							<AT91_PIOE 25 AT91_PERIPH_B &pcfg_none	/* PE25 periph B, conflicts with A25 */
+							 AT91_PIOE 26 AT91_PERIPH_B &pcfg_pull_up>;	/* PE26 periph B with pullup, conflicts NCS0 */
 					};
 
 					pinctrl_usart2_rts_cts: usart2_rts_cts-0 {
 						atmel,pins =
-							<AT91_PIOE 23 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PE23 periph B, conflicts with A23 */
-							 AT91_PIOE 24 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* PE24 periph B, conflicts with A24 */
+							<AT91_PIOE 23 AT91_PERIPH_B &pcfg_none	/* PE23 periph B, conflicts with A23 */
+							 AT91_PIOE 24 AT91_PERIPH_B &pcfg_none>;	/* PE24 periph B, conflicts with A24 */
 					};
 				};
 
 				usart3 {
 					pinctrl_usart3: usart3-0 {
 						atmel,pins =
-							<AT91_PIOE 18 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PE18 periph B, conflicts with A18 */
-							 AT91_PIOE 19 AT91_PERIPH_B AT91_PINCTRL_PULL_UP>;	/* PE19 periph B with pullup, conflicts with A19 */
+							<AT91_PIOE 18 AT91_PERIPH_B &pcfg_none	/* PE18 periph B, conflicts with A18 */
+							 AT91_PIOE 19 AT91_PERIPH_B &pcfg_pull_up>;	/* PE19 periph B with pullup, conflicts with A19 */
 					};
 
 					pinctrl_usart3_rts_cts: usart3_rts_cts-0 {
 						atmel,pins =
-							<AT91_PIOE 16 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PE16 periph B, conflicts with A16 */
-							 AT91_PIOE 17 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* PE17 periph B, conflicts with A17 */
+							<AT91_PIOE 16 AT91_PERIPH_B &pcfg_none	/* PE16 periph B, conflicts with A16 */
+							 AT91_PIOE 17 AT91_PERIPH_B &pcfg_none>;	/* PE17 periph B, conflicts with A17 */
 					};
 				};
 
diff --git a/arch/arm/boot/dts/sama5d3xdm.dtsi b/arch/arm/boot/dts/sama5d3xdm.dtsi
index 1c296d6..a8d5f77 100644
--- a/arch/arm/boot/dts/sama5d3xdm.dtsi
+++ b/arch/arm/boot/dts/sama5d3xdm.dtsi
@@ -33,7 +33,7 @@
 				board {
 					pinctrl_qt1070_irq: qt1070_irq {
 						atmel,pins =
-							<AT91_PIOE 31 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP_DEGLITCH>; /* PE31 GPIO with pull up deglith */
+							<AT91_PIOE 31 AT91_PERIPH_GPIO &pcfg_pull_up_deglitch>; /* PE31 GPIO with pull up deglith */
 					};
 				};
 			};
diff --git a/arch/arm/boot/dts/sama5d3xmb.dtsi b/arch/arm/boot/dts/sama5d3xmb.dtsi
index 8a9e05d..126a3a8 100644
--- a/arch/arm/boot/dts/sama5d3xmb.dtsi
+++ b/arch/arm/boot/dts/sama5d3xmb.dtsi
@@ -87,32 +87,32 @@
 				board {
 					pinctrl_mmc0_cd: mmc0_cd {
 						atmel,pins =
-							<AT91_PIOD 17 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP_DEGLITCH>; /* PD17 GPIO with pullup deglitch */
+							<AT91_PIOD 17 AT91_PERIPH_GPIO &pcfg_pull_up_deglitch>; /* PD17 GPIO with pullup deglitch */
 					};
 
 					pinctrl_mmc1_cd: mmc1_cd {
 						atmel,pins =
-							<AT91_PIOD 18 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP_DEGLITCH>; /* PD18 GPIO with pullup deglitch */
+							<AT91_PIOD 18 AT91_PERIPH_GPIO &pcfg_pull_up_deglitch>; /* PD18 GPIO with pullup deglitch */
 					};
 
 					pinctrl_pck0_as_audio_mck: pck0_as_audio_mck {
 						atmel,pins =
-							<AT91_PIOD 30 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* PD30 periph B */
+							<AT91_PIOD 30 AT91_PERIPH_B &pcfg_none>;	/* PD30 periph B */
 					};
 
 					pinctrl_isi_reset: isi_reset-0 {
 						atmel,pins =
-							<AT91_PIOE 24 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;   /* PE24 gpio */
+							<AT91_PIOE 24 AT91_PERIPH_GPIO &pcfg_none>;   /* PE24 gpio */
 					};
 
 					pinctrl_isi_power: isi_power-0 {
 						atmel,pins =
-							<AT91_PIOE 29 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; /* PE29 gpio */
+							<AT91_PIOE 29 AT91_PERIPH_GPIO &pcfg_none>; /* PE29 gpio */
 					};
 
 					pinctrl_usba_vbus: usba_vbus {
 						atmel,pins =
-							<AT91_PIOD 29 AT91_PERIPH_GPIO AT91_PINCTRL_DEGLITCH>; /* PD29 GPIO with deglitch */
+							<AT91_PIOD 29 AT91_PERIPH_GPIO &pcfg_deglitch>; /* PD29 GPIO with deglitch */
 					};
 				};
 			};
-- 
1.7.9.5

Re: [RFC PATCH 3/3] ARM: at91/dt: move sama5 to generic pinconf

[Linus Walleij]

On Sat, Aug 24, 2013 at 11:40 PM, Boris BREZILLON
<b.brezillon@overkiz.com> wrote:

> Add generic pinconf definitions and reference appropriate configs in
> atmel,pins properties.
>
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
(...)
>                         pinctrl@fffff200 {
>                                 #address-cells = <1>;
>                                 #size-cells = <1>;
> -                               compatible = "atmel,at91sam9x5-pinctrl", "atmel,at91rm9200-pinctrl", "simple-bus";
> +                               compatible = "atmel,at91sam9x5-pinctrl", "atmel,at91rm9200-pinctrl", "generic-pinconf", "simple-bus";

What kind of compatible string is that "generic-pinconf"?

There is no driver that can instantiate against this string but I'm not
100% sure about such things. Is there some other driver doing this?

Else I think it'd just be removed.

> +                               pcfg_none: pcfg_none {
> +                                       bias-disable;
> +                               };
> +
> +                               pcfg_pull_up: pcfg_pull_up {
> +                                       bias-pull-up;
> +                               };

Nice.

> +                               pcfg_deglitch: pcfg_deglitch {
> +                                       input-deglitch = <1>;
> +                               };
> +
> +                               pcfg_pull_up_deglitch: pcfg_pull_up_deglitch {
> +                                       bias-pull-up;
> +                                       input-deglitch = <1>;
> +                               };

input-deglitch seems like a proposed generic binding but I haven't seen
these yet?

(It might be in my violently exploding INBOX though sorry in that case.)

This would need adding to
Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
Plus changes to pinctrl core for handling.

BTW: this is really moving in the right direction!

Yours,
Linus Walleij

Re: [RFC PATCH 3/3] ARM: at91/dt: move sama5 to generic pinconf

[boris brezillon]

Hello Linus,

On 28/08/2013 14:28, Linus Walleij wrote:
> On Sat, Aug 24, 2013 at 11:40 PM, Boris BREZILLON
> <b.brezillon@overkiz.com> wrote:
>
>> Add generic pinconf definitions and reference appropriate configs in
>> atmel,pins properties.
>>
>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> (...)
>>                          pinctrl@fffff200 {
>>                                  #address-cells = <1>;
>>                                  #size-cells = <1>;
>> -                               compatible = "atmel,at91sam9x5-pinctrl", "atmel,at91rm9200-pinctrl", "simple-bus";
>> +                               compatible = "atmel,at91sam9x5-pinctrl", "atmel,at91rm9200-pinctrl", "generic-pinconf", "simple-bus";
> What kind of compatible string is that "generic-pinconf"?
>
> There is no driver that can instantiate against this string but I'm not
> 100% sure about such things. Is there some other driver doing this?
>
> Else I think it'd just be removed.

It did not exist before this patch series.

I thought it would be good idea to add a compatible string to tell if 
the pinctrl subnodes support the generic-pinconf binding,
without modifying the current compatible strings:
if compatible string contains the "generic-pinconf" then the pinconf 
definitions should be considered generic.

However, after discussing it with Stephen, Jean-Christophe and Nicolas, 
I no longer think this is a good idea
(backward compatibility issues).



>
>> +                               pcfg_none: pcfg_none {
>> +                                       bias-disable;
>> +                               };
>> +
>> +                               pcfg_pull_up: pcfg_pull_up {
>> +                                       bias-pull-up;
>> +                               };
> Nice.
>
>> +                               pcfg_deglitch: pcfg_deglitch {
>> +                                       input-deglitch = <1>;
>> +                               };
>> +
>> +                               pcfg_pull_up_deglitch: pcfg_pull_up_deglitch {
>> +                                       bias-pull-up;
>> +                                       input-deglitch = <1>;
>> +                               };
> input-deglitch seems like a proposed generic binding but I haven't seen
> these yet?
> (It might be in my violently exploding INBOX though sorry in that case.)
>
> This would need adding to
> Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> Plus changes to pinctrl core for handling.

This was added in the first patch of this series:
https://lkml.org/lkml/2013/8/24/99

> BTW: this is really moving in the right direction!
>
> Yours,
> Linus Walleij

Thanks.

Best Regards,

Boris

Re: [RFC PATCH 0/3] pinctrl: at91: add support for generic pinconf

[boris brezillon]

On 24/08/2013 23:32, Boris BREZILLON wrote:
> Hello,
>
> This patch series is an attempt to add support for generic pin config
> syntax to at91 pinctrl driver.
>
> My primary goal is to add support for output configuration from dt definition.
> This is needed to fully move at91rm9200ek board to dt (other boards may have
> the same needs).
> This board use a pin to drive an external switch which select between 2
> functionnalities:
>   - mmc interface
>   - spi interface
> The pin level is currently configured in the board init (init_machine) function
> based on user config choices (CONFIG_MTD_AT91_DATAFLASH_CARD).
>
> Instead of adding a new flag to the current (native) pin config binding, I
> tried to add support for the generic pin config used by some pinctrl drivers
> (i.e. rockchip).
>
> Is this the right way to do this or should I add a new at91 native flags for
> output config (OUTPUT_HIGH/LOW) ?
>
> The second patch introduce a new config parameter to add a glitch filter on a
> specific pin.
The first patch, not the second.
> Glitch filter is similar to bounce filter (or debounce) but with a smaller
> delay (expressed in nsecs ?).
>
> I'm not sure this is the right approach.
> Maybe we should reuse the debounce parameter and add a flag to specify the delay
> unit (usec or nsec).
>
> What do you think ?
>
> The third patch migrate sama5 dt boards to the new generic config syntax.
>
> Please feel free to share your thoughts.
>
> Best Regards,
>
> Boris
>
>
> Boris BREZILLON (3):
>    pinctrl: add new generic pinconf config for deglitch filter
>    pinctrl: at91: add support for generic pinconf
>    ARM: at91/dt: move sama5 to generic pinconf
>
>   .../bindings/pinctrl/atmel,at91-pinctrl.txt        |   43 ++-
>   .../bindings/pinctrl/pinctrl-bindings.txt          |    1 +
>   arch/arm/boot/dts/sama5d3.dtsi                     |  363 ++++++++++----------
>   arch/arm/boot/dts/sama5d3xdm.dtsi                  |    2 +-
>   arch/arm/boot/dts/sama5d3xmb.dtsi                  |   12 +-
>   drivers/pinctrl/Kconfig                            |    2 +-
>   drivers/pinctrl/pinconf-generic.c                  |    2 +
>   drivers/pinctrl/pinctrl-at91.c                     |  265 +++++++++++++-
>   include/linux/pinctrl/pinconf-generic.h            |    5 +
>   9 files changed, 494 insertions(+), 201 deletions(-)
>