[PATCH] pinctrl: at91: add support for OUTPUT config

[PATCH] pinctrl: at91: add support for OUTPUT config

[Boris BREZILLON]

Add support for pin output control through the pinctrl config:
 - support enabling/disabling output on a given pin
 - support output level setting (high or low)

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
---
 .../bindings/pinctrl/atmel,at91-pinctrl.txt        |    2 ++
 drivers/pinctrl/pinctrl-at91.c                     |   22 ++++++++++++++++++++
 include/dt-bindings/pinctrl/at91.h                 |    2 ++
 3 files changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
index cf7c7bc..5a22e0d 100644
--- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
@@ -90,6 +90,8 @@ MULTIDRIVE	(1 << 1): indicate this pin need to be configured as multidrive.
 DEGLITCH	(1 << 2): indicate this pin need deglitch.
 PULL_DOWN	(1 << 3): indicate this pin need a pull down.
 DIS_SCHMIT	(1 << 4): indicate this pin need to disable schmit trigger.
+OUTPUT		(1 << 5): indicate this pin need to be configured as an output.
+OUTPUT_VAL	(0x1 << 6): output val (1 = high, 0 = low)
 DEBOUNCE	(1 << 16): indicate this pin need debounce.
 DEBOUNCE_VAL	(0x3fff << 17): debounce val.
 
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 19afb9a..1578a0d 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -61,6 +61,9 @@ static int gpio_banks;
 #define DEGLITCH	(1 << 2)
 #define PULL_DOWN	(1 << 3)
 #define DIS_SCHMIT	(1 << 4)
+#define OUTPUT		(1 << 5)
+#define OUTPUT_VAL_SHIFT	6
+#define OUTPUT_VAL	(0x1 << OUTPUT_VAL_SHIFT)
 #define DEBOUNCE	(1 << 16)
 #define DEBOUNCE_VAL_SHIFT	17
 #define DEBOUNCE_VAL	(0x3fff << DEBOUNCE_VAL_SHIFT)
@@ -333,6 +336,19 @@ static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, bool on)
 	writel_relaxed(mask, pio + (on ? PIO_PUER : PIO_PUDR));
 }
 
+static bool at91_mux_get_output(void __iomem *pio, unsigned pin, bool *val)
+{
+	*val = (readl_relaxed(pio + PIO_ODSR) >> pin) & 0x1;
+	return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
+}
+
+static void at91_mux_set_output(void __iomem *pio, unsigned mask, bool is_on,
+				bool val)
+{
+	writel_relaxed(mask, pio + (val ? PIO_SODR : PIO_CODR));
+	writel_relaxed(mask, pio + (is_on ? PIO_OER : PIO_ODR));
+}
+
 static unsigned at91_mux_get_multidrive(void __iomem *pio, unsigned pin)
 {
 	return (readl_relaxed(pio + PIO_MDSR) >> pin) & 0x1;
@@ -712,6 +728,7 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
 	void __iomem *pio;
 	unsigned pin;
 	int div;
+	bool out;
 
 	dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, __LINE__, pin_id, *config);
 	pio = pin_to_controller(info, pin_to_bank(pin_id));
@@ -732,6 +749,9 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
 	if (info->ops->get_schmitt_trig && info->ops->get_schmitt_trig(pio, pin))
 		*config |= DIS_SCHMIT;
 
+	if (at91_mux_get_output(pio, pin, &out))
+		*config |= OUTPUT | (out << OUTPUT_VAL_SHIFT);
+
 	return 0;
 }
 
@@ -749,6 +769,8 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
 	if (config & PULL_UP && config & PULL_DOWN)
 		return -EINVAL;
 
+	at91_mux_set_output(pio, mask, config & OUTPUT,
+			    (config & OUTPUT_VAL) >> OUTPUT_VAL_SHIFT);
 	at91_mux_set_pullup(pio, mask, config & PULL_UP);
 	at91_mux_set_multidrive(pio, mask, config & MULTI_DRIVE);
 	if (info->ops->set_deglitch)
diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h
index d7988b4..9fd4d48 100644
--- a/include/dt-bindings/pinctrl/at91.h
+++ b/include/dt-bindings/pinctrl/at91.h
@@ -15,6 +15,8 @@
 #define AT91_PINCTRL_DEGLITCH		(1 << 2)
 #define AT91_PINCTRL_PULL_DOWN		(1 << 3)
 #define AT91_PINCTRL_DIS_SCHMIT		(1 << 4)
+#define AT91_PINCTRL_OUTPUT		(1 << 5)
+#define AT91_PINCTRL_OUTPUT_VAL(x)	((x & 0x1) << 6)
 #define AT91_PINCTRL_DEBOUNCE		(1 << 16)
 #define AT91_PINCTRL_DEBOUNCE_VA(x)	(x << 17)
 
-- 
1.7.9.5

Re: [PATCH] pinctrl: at91: add support for OUTPUT config

[boris brezillon]

This was tested on sama5d31ek.

On 27/08/2013 16:51, Boris BREZILLON wrote:
> Add support for pin output control through the pinctrl config:
>   - support enabling/disabling output on a given pin
>   - support output level setting (high or low)
>
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> ---
>   .../bindings/pinctrl/atmel,at91-pinctrl.txt        |    2 ++
>   drivers/pinctrl/pinctrl-at91.c                     |   22 ++++++++++++++++++++
>   include/dt-bindings/pinctrl/at91.h                 |    2 ++
>   3 files changed, 26 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> index cf7c7bc..5a22e0d 100644
> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> @@ -90,6 +90,8 @@ MULTIDRIVE	(1 << 1): indicate this pin need to be configured as multidrive.
>   DEGLITCH	(1 << 2): indicate this pin need deglitch.
>   PULL_DOWN	(1 << 3): indicate this pin need a pull down.
>   DIS_SCHMIT	(1 << 4): indicate this pin need to disable schmit trigger.
> +OUTPUT		(1 << 5): indicate this pin need to be configured as an output.
> +OUTPUT_VAL	(0x1 << 6): output val (1 = high, 0 = low)
>   DEBOUNCE	(1 << 16): indicate this pin need debounce.
>   DEBOUNCE_VAL	(0x3fff << 17): debounce val.
>   
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 19afb9a..1578a0d 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -61,6 +61,9 @@ static int gpio_banks;
>   #define DEGLITCH	(1 << 2)
>   #define PULL_DOWN	(1 << 3)
>   #define DIS_SCHMIT	(1 << 4)
> +#define OUTPUT		(1 << 5)
> +#define OUTPUT_VAL_SHIFT	6
> +#define OUTPUT_VAL	(0x1 << OUTPUT_VAL_SHIFT)
>   #define DEBOUNCE	(1 << 16)
>   #define DEBOUNCE_VAL_SHIFT	17
>   #define DEBOUNCE_VAL	(0x3fff << DEBOUNCE_VAL_SHIFT)
> @@ -333,6 +336,19 @@ static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, bool on)
>   	writel_relaxed(mask, pio + (on ? PIO_PUER : PIO_PUDR));
>   }
>   
> +static bool at91_mux_get_output(void __iomem *pio, unsigned pin, bool *val)
> +{
> +	*val = (readl_relaxed(pio + PIO_ODSR) >> pin) & 0x1;
> +	return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
> +}
> +
> +static void at91_mux_set_output(void __iomem *pio, unsigned mask, bool is_on,
> +				bool val)
> +{
> +	writel_relaxed(mask, pio + (val ? PIO_SODR : PIO_CODR));
> +	writel_relaxed(mask, pio + (is_on ? PIO_OER : PIO_ODR));
> +}
> +
>   static unsigned at91_mux_get_multidrive(void __iomem *pio, unsigned pin)
>   {
>   	return (readl_relaxed(pio + PIO_MDSR) >> pin) & 0x1;
> @@ -712,6 +728,7 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
>   	void __iomem *pio;
>   	unsigned pin;
>   	int div;
> +	bool out;
>   
>   	dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, __LINE__, pin_id, *config);
>   	pio = pin_to_controller(info, pin_to_bank(pin_id));
> @@ -732,6 +749,9 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
>   	if (info->ops->get_schmitt_trig && info->ops->get_schmitt_trig(pio, pin))
>   		*config |= DIS_SCHMIT;
>   
> +	if (at91_mux_get_output(pio, pin, &out))
> +		*config |= OUTPUT | (out << OUTPUT_VAL_SHIFT);
> +
>   	return 0;
>   }
>   
> @@ -749,6 +769,8 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
>   	if (config & PULL_UP && config & PULL_DOWN)
>   		return -EINVAL;
>   
> +	at91_mux_set_output(pio, mask, config & OUTPUT,
> +			    (config & OUTPUT_VAL) >> OUTPUT_VAL_SHIFT);
>   	at91_mux_set_pullup(pio, mask, config & PULL_UP);
>   	at91_mux_set_multidrive(pio, mask, config & MULTI_DRIVE);
>   	if (info->ops->set_deglitch)
> diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h
> index d7988b4..9fd4d48 100644
> --- a/include/dt-bindings/pinctrl/at91.h
> +++ b/include/dt-bindings/pinctrl/at91.h
> @@ -15,6 +15,8 @@
>   #define AT91_PINCTRL_DEGLITCH		(1 << 2)
>   #define AT91_PINCTRL_PULL_DOWN		(1 << 3)
>   #define AT91_PINCTRL_DIS_SCHMIT		(1 << 4)
> +#define AT91_PINCTRL_OUTPUT		(1 << 5)
> +#define AT91_PINCTRL_OUTPUT_VAL(x)	((x & 0x1) << 6)
>   #define AT91_PINCTRL_DEBOUNCE		(1 << 16)
>   #define AT91_PINCTRL_DEBOUNCE_VA(x)	(x << 17)
>   

Re: [PATCH] pinctrl: at91: add support for OUTPUT config

[Nicolas Ferre]

On 27/08/2013 16:51, Boris BREZILLON :
> Add support for pin output control through the pinctrl config:
>   - support enabling/disabling output on a given pin
>   - support output level setting (high or low)
>
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> ---
>   .../bindings/pinctrl/atmel,at91-pinctrl.txt        |    2 ++
>   drivers/pinctrl/pinctrl-at91.c                     |   22 ++++++++++++++++++++
>   include/dt-bindings/pinctrl/at91.h                 |    2 ++
>   3 files changed, 26 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> index cf7c7bc..5a22e0d 100644
> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> @@ -90,6 +90,8 @@ MULTIDRIVE	(1 << 1): indicate this pin need to be configured as multidrive.
>   DEGLITCH	(1 << 2): indicate this pin need deglitch.
>   PULL_DOWN	(1 << 3): indicate this pin need a pull down.
>   DIS_SCHMIT	(1 << 4): indicate this pin need to disable schmit trigger.
> +OUTPUT		(1 << 5): indicate this pin need to be configured as an output.
> +OUTPUT_VAL	(0x1 << 6): output val (1 = high, 0 = low)
>   DEBOUNCE	(1 << 16): indicate this pin need debounce.
>   DEBOUNCE_VAL	(0x3fff << 17): debounce val.
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 19afb9a..1578a0d 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -61,6 +61,9 @@ static int gpio_banks;
>   #define DEGLITCH	(1 << 2)
>   #define PULL_DOWN	(1 << 3)
>   #define DIS_SCHMIT	(1 << 4)
> +#define OUTPUT		(1 << 5)
> +#define OUTPUT_VAL_SHIFT	6
> +#define OUTPUT_VAL	(0x1 << OUTPUT_VAL_SHIFT)
>   #define DEBOUNCE	(1 << 16)
>   #define DEBOUNCE_VAL_SHIFT	17
>   #define DEBOUNCE_VAL	(0x3fff << DEBOUNCE_VAL_SHIFT)
> @@ -333,6 +336,19 @@ static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, bool on)
>   	writel_relaxed(mask, pio + (on ? PIO_PUER : PIO_PUDR));
>   }
>
> +static bool at91_mux_get_output(void __iomem *pio, unsigned pin, bool *val)
> +{
> +	*val = (readl_relaxed(pio + PIO_ODSR) >> pin) & 0x1;
> +	return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
> +}
> +
> +static void at91_mux_set_output(void __iomem *pio, unsigned mask, bool is_on,
> +				bool val)
> +{
> +	writel_relaxed(mask, pio + (val ? PIO_SODR : PIO_CODR));
> +	writel_relaxed(mask, pio + (is_on ? PIO_OER : PIO_ODR));
> +}
> +
>   static unsigned at91_mux_get_multidrive(void __iomem *pio, unsigned pin)
>   {
>   	return (readl_relaxed(pio + PIO_MDSR) >> pin) & 0x1;
> @@ -712,6 +728,7 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
>   	void __iomem *pio;
>   	unsigned pin;
>   	int div;
> +	bool out;
>
>   	dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, __LINE__, pin_id, *config);
>   	pio = pin_to_controller(info, pin_to_bank(pin_id));
> @@ -732,6 +749,9 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
>   	if (info->ops->get_schmitt_trig && info->ops->get_schmitt_trig(pio, pin))
>   		*config |= DIS_SCHMIT;
>
> +	if (at91_mux_get_output(pio, pin, &out))
> +		*config |= OUTPUT | (out << OUTPUT_VAL_SHIFT);
> +
>   	return 0;
>   }
>
> @@ -749,6 +769,8 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
>   	if (config & PULL_UP && config & PULL_DOWN)
>   		return -EINVAL;
>
> +	at91_mux_set_output(pio, mask, config & OUTPUT,
> +			    (config & OUTPUT_VAL) >> OUTPUT_VAL_SHIFT);
>   	at91_mux_set_pullup(pio, mask, config & PULL_UP);
>   	at91_mux_set_multidrive(pio, mask, config & MULTI_DRIVE);
>   	if (info->ops->set_deglitch)
> diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h
> index d7988b4..9fd4d48 100644
> --- a/include/dt-bindings/pinctrl/at91.h
> +++ b/include/dt-bindings/pinctrl/at91.h
> @@ -15,6 +15,8 @@
>   #define AT91_PINCTRL_DEGLITCH		(1 << 2)
>   #define AT91_PINCTRL_PULL_DOWN		(1 << 3)
>   #define AT91_PINCTRL_DIS_SCHMIT		(1 << 4)
> +#define AT91_PINCTRL_OUTPUT		(1 << 5)
> +#define AT91_PINCTRL_OUTPUT_VAL(x)	((x & 0x1) << 6)

Can you add this change to the documentation as well:
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt


>   #define AT91_PINCTRL_DEBOUNCE		(1 << 16)
>   #define AT91_PINCTRL_DEBOUNCE_VA(x)	(x << 17)

Oh, we have a nice typo here! ---^^^^^^
And moreover it would be good to add the mask as well.

>
>

Once the documentation added, you can stick my:

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Thanks, bye,
-- 
Nicolas Ferre

Re: [PATCH] pinctrl: at91: add support for OUTPUT config

[boris brezillon]

On 28/08/2013 09:31, Nicolas Ferre wrote:
> On 27/08/2013 16:51, Boris BREZILLON :
>> Add support for pin output control through the pinctrl config:
>>   - support enabling/disabling output on a given pin
>>   - support output level setting (high or low)
>>
>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>> ---
>>   .../bindings/pinctrl/atmel,at91-pinctrl.txt        |    2 ++
>>   drivers/pinctrl/pinctrl-at91.c                     |   22 
>> ++++++++++++++++++++
>>   include/dt-bindings/pinctrl/at91.h                 |    2 ++
>>   3 files changed, 26 insertions(+)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt 
>> b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>> index cf7c7bc..5a22e0d 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>> @@ -90,6 +90,8 @@ MULTIDRIVE    (1 << 1): indicate this pin need to 
>> be configured as multidrive.
>>   DEGLITCH    (1 << 2): indicate this pin need deglitch.
>>   PULL_DOWN    (1 << 3): indicate this pin need a pull down.
>>   DIS_SCHMIT    (1 << 4): indicate this pin need to disable schmit 
>> trigger.
>> +OUTPUT        (1 << 5): indicate this pin need to be configured as 
>> an output.
>> +OUTPUT_VAL    (0x1 << 6): output val (1 = high, 0 = low)
>>   DEBOUNCE    (1 << 16): indicate this pin need debounce.
>>   DEBOUNCE_VAL    (0x3fff << 17): debounce val.
>>
>> diff --git a/drivers/pinctrl/pinctrl-at91.c 
>> b/drivers/pinctrl/pinctrl-at91.c
>> index 19afb9a..1578a0d 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -61,6 +61,9 @@ static int gpio_banks;
>>   #define DEGLITCH    (1 << 2)
>>   #define PULL_DOWN    (1 << 3)
>>   #define DIS_SCHMIT    (1 << 4)
>> +#define OUTPUT        (1 << 5)
>> +#define OUTPUT_VAL_SHIFT    6
>> +#define OUTPUT_VAL    (0x1 << OUTPUT_VAL_SHIFT)
>>   #define DEBOUNCE    (1 << 16)
>>   #define DEBOUNCE_VAL_SHIFT    17
>>   #define DEBOUNCE_VAL    (0x3fff << DEBOUNCE_VAL_SHIFT)
>> @@ -333,6 +336,19 @@ static void at91_mux_set_pullup(void __iomem 
>> *pio, unsigned mask, bool on)
>>       writel_relaxed(mask, pio + (on ? PIO_PUER : PIO_PUDR));
>>   }
>>
>> +static bool at91_mux_get_output(void __iomem *pio, unsigned pin, 
>> bool *val)
>> +{
>> +    *val = (readl_relaxed(pio + PIO_ODSR) >> pin) & 0x1;
>> +    return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
>> +}
>> +
>> +static void at91_mux_set_output(void __iomem *pio, unsigned mask, 
>> bool is_on,
>> +                bool val)
>> +{
>> +    writel_relaxed(mask, pio + (val ? PIO_SODR : PIO_CODR));
>> +    writel_relaxed(mask, pio + (is_on ? PIO_OER : PIO_ODR));
>> +}
>> +
>>   static unsigned at91_mux_get_multidrive(void __iomem *pio, unsigned 
>> pin)
>>   {
>>       return (readl_relaxed(pio + PIO_MDSR) >> pin) & 0x1;
>> @@ -712,6 +728,7 @@ static int at91_pinconf_get(struct pinctrl_dev 
>> *pctldev,
>>       void __iomem *pio;
>>       unsigned pin;
>>       int div;
>> +    bool out;
>>
>>       dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, 
>> __LINE__, pin_id, *config);
>>       pio = pin_to_controller(info, pin_to_bank(pin_id));
>> @@ -732,6 +749,9 @@ static int at91_pinconf_get(struct pinctrl_dev 
>> *pctldev,
>>       if (info->ops->get_schmitt_trig && 
>> info->ops->get_schmitt_trig(pio, pin))
>>           *config |= DIS_SCHMIT;
>>
>> +    if (at91_mux_get_output(pio, pin, &out))
>> +        *config |= OUTPUT | (out << OUTPUT_VAL_SHIFT);
>> +
>>       return 0;
>>   }
>>
>> @@ -749,6 +769,8 @@ static int at91_pinconf_set(struct pinctrl_dev 
>> *pctldev,
>>       if (config & PULL_UP && config & PULL_DOWN)
>>           return -EINVAL;
>>
>> +    at91_mux_set_output(pio, mask, config & OUTPUT,
>> +                (config & OUTPUT_VAL) >> OUTPUT_VAL_SHIFT);
>>       at91_mux_set_pullup(pio, mask, config & PULL_UP);
>>       at91_mux_set_multidrive(pio, mask, config & MULTI_DRIVE);
>>       if (info->ops->set_deglitch)
>> diff --git a/include/dt-bindings/pinctrl/at91.h 
>> b/include/dt-bindings/pinctrl/at91.h
>> index d7988b4..9fd4d48 100644
>> --- a/include/dt-bindings/pinctrl/at91.h
>> +++ b/include/dt-bindings/pinctrl/at91.h
>> @@ -15,6 +15,8 @@
>>   #define AT91_PINCTRL_DEGLITCH        (1 << 2)
>>   #define AT91_PINCTRL_PULL_DOWN        (1 << 3)
>>   #define AT91_PINCTRL_DIS_SCHMIT        (1 << 4)
>> +#define AT91_PINCTRL_OUTPUT        (1 << 5)
>> +#define AT91_PINCTRL_OUTPUT_VAL(x)    ((x & 0x1) << 6)
>
> Can you add this change to the documentation as well:
> Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>
>
>>   #define AT91_PINCTRL_DEBOUNCE        (1 << 16)
>>   #define AT91_PINCTRL_DEBOUNCE_VA(x)    (x << 17)
>
> Oh, we have a nice typo here! ---^^^^^^
> And moreover it would be good to add the mask as well.

Do you want me to fix this typo and add mask for DEBOUNCE
in a separate patch ?

>
>>
>>
>
> Once the documentation added, you can stick my:
>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>
> Thanks, bye,

Re: [PATCH] pinctrl: at91: add support for OUTPUT config

[Nicolas Ferre]

On 28/08/2013 09:35, boris brezillon :
> On 28/08/2013 09:31, Nicolas Ferre wrote:
>> On 27/08/2013 16:51, Boris BREZILLON :
>>> Add support for pin output control through the pinctrl config:
>>>    - support enabling/disabling output on a given pin
>>>    - support output level setting (high or low)
>>>
>>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>>> ---
>>>    .../bindings/pinctrl/atmel,at91-pinctrl.txt        |    2 ++
>>>    drivers/pinctrl/pinctrl-at91.c                     |   22
>>> ++++++++++++++++++++
>>>    include/dt-bindings/pinctrl/at91.h                 |    2 ++
>>>    3 files changed, 26 insertions(+)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>> b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>> index cf7c7bc..5a22e0d 100644
>>> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>> @@ -90,6 +90,8 @@ MULTIDRIVE    (1 << 1): indicate this pin need to
>>> be configured as multidrive.
>>>    DEGLITCH    (1 << 2): indicate this pin need deglitch.
>>>    PULL_DOWN    (1 << 3): indicate this pin need a pull down.
>>>    DIS_SCHMIT    (1 << 4): indicate this pin need to disable schmit
>>> trigger.
>>> +OUTPUT        (1 << 5): indicate this pin need to be configured as
>>> an output.
>>> +OUTPUT_VAL    (0x1 << 6): output val (1 = high, 0 = low)
>>>    DEBOUNCE    (1 << 16): indicate this pin need debounce.
>>>    DEBOUNCE_VAL    (0x3fff << 17): debounce val.
>>>
>>> diff --git a/drivers/pinctrl/pinctrl-at91.c
>>> b/drivers/pinctrl/pinctrl-at91.c
>>> index 19afb9a..1578a0d 100644
>>> --- a/drivers/pinctrl/pinctrl-at91.c
>>> +++ b/drivers/pinctrl/pinctrl-at91.c
>>> @@ -61,6 +61,9 @@ static int gpio_banks;
>>>    #define DEGLITCH    (1 << 2)
>>>    #define PULL_DOWN    (1 << 3)
>>>    #define DIS_SCHMIT    (1 << 4)
>>> +#define OUTPUT        (1 << 5)
>>> +#define OUTPUT_VAL_SHIFT    6
>>> +#define OUTPUT_VAL    (0x1 << OUTPUT_VAL_SHIFT)
>>>    #define DEBOUNCE    (1 << 16)
>>>    #define DEBOUNCE_VAL_SHIFT    17
>>>    #define DEBOUNCE_VAL    (0x3fff << DEBOUNCE_VAL_SHIFT)
>>> @@ -333,6 +336,19 @@ static void at91_mux_set_pullup(void __iomem
>>> *pio, unsigned mask, bool on)
>>>        writel_relaxed(mask, pio + (on ? PIO_PUER : PIO_PUDR));
>>>    }
>>>
>>> +static bool at91_mux_get_output(void __iomem *pio, unsigned pin,
>>> bool *val)
>>> +{
>>> +    *val = (readl_relaxed(pio + PIO_ODSR) >> pin) & 0x1;
>>> +    return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
>>> +}
>>> +
>>> +static void at91_mux_set_output(void __iomem *pio, unsigned mask,
>>> bool is_on,
>>> +                bool val)
>>> +{
>>> +    writel_relaxed(mask, pio + (val ? PIO_SODR : PIO_CODR));
>>> +    writel_relaxed(mask, pio + (is_on ? PIO_OER : PIO_ODR));
>>> +}
>>> +
>>>    static unsigned at91_mux_get_multidrive(void __iomem *pio, unsigned
>>> pin)
>>>    {
>>>        return (readl_relaxed(pio + PIO_MDSR) >> pin) & 0x1;
>>> @@ -712,6 +728,7 @@ static int at91_pinconf_get(struct pinctrl_dev
>>> *pctldev,
>>>        void __iomem *pio;
>>>        unsigned pin;
>>>        int div;
>>> +    bool out;
>>>
>>>        dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__,
>>> __LINE__, pin_id, *config);
>>>        pio = pin_to_controller(info, pin_to_bank(pin_id));
>>> @@ -732,6 +749,9 @@ static int at91_pinconf_get(struct pinctrl_dev
>>> *pctldev,
>>>        if (info->ops->get_schmitt_trig &&
>>> info->ops->get_schmitt_trig(pio, pin))
>>>            *config |= DIS_SCHMIT;
>>>
>>> +    if (at91_mux_get_output(pio, pin, &out))
>>> +        *config |= OUTPUT | (out << OUTPUT_VAL_SHIFT);
>>> +
>>>        return 0;
>>>    }
>>>
>>> @@ -749,6 +769,8 @@ static int at91_pinconf_set(struct pinctrl_dev
>>> *pctldev,
>>>        if (config & PULL_UP && config & PULL_DOWN)
>>>            return -EINVAL;
>>>
>>> +    at91_mux_set_output(pio, mask, config & OUTPUT,
>>> +                (config & OUTPUT_VAL) >> OUTPUT_VAL_SHIFT);
>>>        at91_mux_set_pullup(pio, mask, config & PULL_UP);
>>>        at91_mux_set_multidrive(pio, mask, config & MULTI_DRIVE);
>>>        if (info->ops->set_deglitch)
>>> diff --git a/include/dt-bindings/pinctrl/at91.h
>>> b/include/dt-bindings/pinctrl/at91.h
>>> index d7988b4..9fd4d48 100644
>>> --- a/include/dt-bindings/pinctrl/at91.h
>>> +++ b/include/dt-bindings/pinctrl/at91.h
>>> @@ -15,6 +15,8 @@
>>>    #define AT91_PINCTRL_DEGLITCH        (1 << 2)
>>>    #define AT91_PINCTRL_PULL_DOWN        (1 << 3)
>>>    #define AT91_PINCTRL_DIS_SCHMIT        (1 << 4)
>>> +#define AT91_PINCTRL_OUTPUT        (1 << 5)
>>> +#define AT91_PINCTRL_OUTPUT_VAL(x)    ((x & 0x1) << 6)
>>
>> Can you add this change to the documentation as well:
>> Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>
>>
>>>    #define AT91_PINCTRL_DEBOUNCE        (1 << 16)
>>>    #define AT91_PINCTRL_DEBOUNCE_VA(x)    (x << 17)
>>
>> Oh, we have a nice typo here! ---^^^^^^
>> And moreover it would be good to add the mask as well.
>
> Do you want me to fix this typo and add mask for DEBOUNCE
> in a separate patch ?

Yep, if you want.

>> Once the documentation added, you can stick my:
>>
>> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>
>> Thanks, bye,
>
>
>


-- 
Nicolas Ferre

Re: [PATCH] pinctrl: at91: add support for OUTPUT config

[Nicolas Ferre]

On 28/08/2013 09:43, Nicolas Ferre :
> On 28/08/2013 09:35, boris brezillon :
>> On 28/08/2013 09:31, Nicolas Ferre wrote:
>>> On 27/08/2013 16:51, Boris BREZILLON :
>>>> Add support for pin output control through the pinctrl config:
>>>>     - support enabling/disabling output on a given pin
>>>>     - support output level setting (high or low)
>>>>
>>>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>>>> ---
>>>>     .../bindings/pinctrl/atmel,at91-pinctrl.txt        |    2 ++
>>>>     drivers/pinctrl/pinctrl-at91.c                     |   22
>>>> ++++++++++++++++++++
>>>>     include/dt-bindings/pinctrl/at91.h                 |    2 ++
>>>>     3 files changed, 26 insertions(+)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>> b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>> index cf7c7bc..5a22e0d 100644
>>>> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>>> @@ -90,6 +90,8 @@ MULTIDRIVE    (1 << 1): indicate this pin need to
>>>> be configured as multidrive.
>>>>     DEGLITCH    (1 << 2): indicate this pin need deglitch.
>>>>     PULL_DOWN    (1 << 3): indicate this pin need a pull down.
>>>>     DIS_SCHMIT    (1 << 4): indicate this pin need to disable schmit
>>>> trigger.
>>>> +OUTPUT        (1 << 5): indicate this pin need to be configured as
>>>> an output.
>>>> +OUTPUT_VAL    (0x1 << 6): output val (1 = high, 0 = low)
>>>>     DEBOUNCE    (1 << 16): indicate this pin need debounce.
>>>>     DEBOUNCE_VAL    (0x3fff << 17): debounce val.
>>>>
>>>> diff --git a/drivers/pinctrl/pinctrl-at91.c
>>>> b/drivers/pinctrl/pinctrl-at91.c
>>>> index 19afb9a..1578a0d 100644
>>>> --- a/drivers/pinctrl/pinctrl-at91.c
>>>> +++ b/drivers/pinctrl/pinctrl-at91.c
>>>> @@ -61,6 +61,9 @@ static int gpio_banks;
>>>>     #define DEGLITCH    (1 << 2)
>>>>     #define PULL_DOWN    (1 << 3)
>>>>     #define DIS_SCHMIT    (1 << 4)
>>>> +#define OUTPUT        (1 << 5)
>>>> +#define OUTPUT_VAL_SHIFT    6
>>>> +#define OUTPUT_VAL    (0x1 << OUTPUT_VAL_SHIFT)
>>>>     #define DEBOUNCE    (1 << 16)
>>>>     #define DEBOUNCE_VAL_SHIFT    17
>>>>     #define DEBOUNCE_VAL    (0x3fff << DEBOUNCE_VAL_SHIFT)
>>>> @@ -333,6 +336,19 @@ static void at91_mux_set_pullup(void __iomem
>>>> *pio, unsigned mask, bool on)
>>>>         writel_relaxed(mask, pio + (on ? PIO_PUER : PIO_PUDR));
>>>>     }
>>>>
>>>> +static bool at91_mux_get_output(void __iomem *pio, unsigned pin,
>>>> bool *val)
>>>> +{
>>>> +    *val = (readl_relaxed(pio + PIO_ODSR) >> pin) & 0x1;
>>>> +    return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1;
>>>> +}
>>>> +
>>>> +static void at91_mux_set_output(void __iomem *pio, unsigned mask,
>>>> bool is_on,
>>>> +                bool val)
>>>> +{
>>>> +    writel_relaxed(mask, pio + (val ? PIO_SODR : PIO_CODR));
>>>> +    writel_relaxed(mask, pio + (is_on ? PIO_OER : PIO_ODR));
>>>> +}
>>>> +
>>>>     static unsigned at91_mux_get_multidrive(void __iomem *pio, unsigned
>>>> pin)
>>>>     {
>>>>         return (readl_relaxed(pio + PIO_MDSR) >> pin) & 0x1;
>>>> @@ -712,6 +728,7 @@ static int at91_pinconf_get(struct pinctrl_dev
>>>> *pctldev,
>>>>         void __iomem *pio;
>>>>         unsigned pin;
>>>>         int div;
>>>> +    bool out;
>>>>
>>>>         dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__,
>>>> __LINE__, pin_id, *config);
>>>>         pio = pin_to_controller(info, pin_to_bank(pin_id));
>>>> @@ -732,6 +749,9 @@ static int at91_pinconf_get(struct pinctrl_dev
>>>> *pctldev,
>>>>         if (info->ops->get_schmitt_trig &&
>>>> info->ops->get_schmitt_trig(pio, pin))
>>>>             *config |= DIS_SCHMIT;
>>>>
>>>> +    if (at91_mux_get_output(pio, pin, &out))
>>>> +        *config |= OUTPUT | (out << OUTPUT_VAL_SHIFT);
>>>> +
>>>>         return 0;
>>>>     }
>>>>
>>>> @@ -749,6 +769,8 @@ static int at91_pinconf_set(struct pinctrl_dev
>>>> *pctldev,
>>>>         if (config & PULL_UP && config & PULL_DOWN)
>>>>             return -EINVAL;
>>>>
>>>> +    at91_mux_set_output(pio, mask, config & OUTPUT,
>>>> +                (config & OUTPUT_VAL) >> OUTPUT_VAL_SHIFT);
>>>>         at91_mux_set_pullup(pio, mask, config & PULL_UP);
>>>>         at91_mux_set_multidrive(pio, mask, config & MULTI_DRIVE);
>>>>         if (info->ops->set_deglitch)
>>>> diff --git a/include/dt-bindings/pinctrl/at91.h
>>>> b/include/dt-bindings/pinctrl/at91.h
>>>> index d7988b4..9fd4d48 100644
>>>> --- a/include/dt-bindings/pinctrl/at91.h
>>>> +++ b/include/dt-bindings/pinctrl/at91.h
>>>> @@ -15,6 +15,8 @@
>>>>     #define AT91_PINCTRL_DEGLITCH        (1 << 2)
>>>>     #define AT91_PINCTRL_PULL_DOWN        (1 << 3)
>>>>     #define AT91_PINCTRL_DIS_SCHMIT        (1 << 4)
>>>> +#define AT91_PINCTRL_OUTPUT        (1 << 5)
>>>> +#define AT91_PINCTRL_OUTPUT_VAL(x)    ((x & 0x1) << 6)
>>>
>>> Can you add this change to the documentation as well:
>>> Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt

Oops! it is already in this patch: sorry for the noise!


>>>>     #define AT91_PINCTRL_DEBOUNCE        (1 << 16)
>>>>     #define AT91_PINCTRL_DEBOUNCE_VA(x)    (x << 17)
>>>
>>> Oh, we have a nice typo here! ---^^^^^^
>>> And moreover it would be good to add the mask as well.
>>
>> Do you want me to fix this typo and add mask for DEBOUNCE
>> in a separate patch ?
>
> Yep, if you want.
>
>>> Once the documentation added, you can stick my:

No condition anymore here then.

>>>
>>> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>
>>> Thanks, bye,
>>
>>
>>
>
>


-- 
Nicolas Ferre

Re: [PATCH] pinctrl: at91: add support for OUTPUT config

[Linus Walleij]

On Tue, Aug 27, 2013 at 4:51 PM, Boris BREZILLON
<b.brezillon@overkiz.com> wrote:

> Add support for pin output control through the pinctrl config:
>  - support enabling/disabling output on a given pin
>  - support output level setting (high or low)
>
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>

NAK.

We already have this:

 * @PIN_CONFIG_OUTPUT: this will configure the pin in output, use argument
 *      1 to indicate high level, argument 0 to indicate low level.

It also has device tree bindings:

output-low              - set the pin to output mode with low level
output-high             - set the pin to output mode with high level

Yours,
Linus Walleij

Re: [PATCH] pinctrl: at91: add support for OUTPUT config

[Linus Walleij]

On Wed, Aug 28, 2013 at 3:46 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Aug 27, 2013 at 4:51 PM, Boris BREZILLON
> <b.brezillon@overkiz.com> wrote:
>
>> Add support for pin output control through the pinctrl config:
>>  - support enabling/disabling output on a given pin
>>  - support output level setting (high or low)
>>
>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>
> NAK.
>
> We already have this:
>
>  * @PIN_CONFIG_OUTPUT: this will configure the pin in output, use argument
>  *      1 to indicate high level, argument 0 to indicate low level.
>
> It also has device tree bindings:
>
> output-low              - set the pin to output mode with low level
> output-high             - set the pin to output mode with high level

Bah maybe I'm misunderstanding :-(

So this is not part of the generic pin config series, but something
stand-alone to augment the existing driver to do this?

So I just apply this patch right off then?

Yours,
Linus Walleij

Re: [PATCH] pinctrl: at91: add support for OUTPUT config

[boris brezillon]

On 28/08/2013 15:48, Linus Walleij wrote:
> On Wed, Aug 28, 2013 at 3:46 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Tue, Aug 27, 2013 at 4:51 PM, Boris BREZILLON
>> <b.brezillon@overkiz.com> wrote:
>>
>>> Add support for pin output control through the pinctrl config:
>>>   - support enabling/disabling output on a given pin
>>>   - support output level setting (high or low)
>>>
>>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>> NAK.
>>
>> We already have this:
>>
>>   * @PIN_CONFIG_OUTPUT: this will configure the pin in output, use argument
>>   *      1 to indicate high level, argument 0 to indicate low level.
>>
>> It also has device tree bindings:
>>
>> output-low              - set the pin to output mode with low level
>> output-high             - set the pin to output mode with high level
> Bah maybe I'm misunderstanding :-(
>
> So this is not part of the generic pin config series, but something
> stand-alone to augment the existing driver to do this?

This is a proposal to add support for OUTPUT config using native at91 
pinconf binding (not generic pinconf binding).

I did this to get support for OUTPUT config quickly.

The generic pinconf binding is still in discussion, and will be added as 
soon as at91 maintainers and pinctrl
maintainers agree on how this should be done.

>
> So I just apply this patch right off then?

I don't know, I think we should wait for Jean-Christophe approval.

>
> Yours,
> Linus Walleij