* Re: [PATCH] pinctrl: at91: add support for OUTPUT config
2013-08-27 14:51 [PATCH] pinctrl: at91: add support for OUTPUT config Boris BREZILLON
@ 2013-08-27 15:01 ` boris brezillon
2013-08-28 7:31 ` Nicolas Ferre
2013-08-28 13:46 ` Linus Walleij
2 siblings, 0 replies; 9+ messages in thread
From: boris brezillon @ 2013-08-27 15:01 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Jean-Christophe PLAGNIOL-VILLARD,
Linus Walleij, Nicolas Ferre, Richard Genoud, Jiri Kosina,
devicetree, linux-doc, linux-kernel, linux-arm-kernel
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)
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] pinctrl: at91: add support for OUTPUT config
2013-08-27 14:51 [PATCH] pinctrl: at91: add support for OUTPUT config Boris BREZILLON
2013-08-27 15:01 ` boris brezillon
@ 2013-08-28 7:31 ` Nicolas Ferre
2013-08-28 7:35 ` boris brezillon
2013-08-28 13:46 ` Linus Walleij
2 siblings, 1 reply; 9+ messages in thread
From: Nicolas Ferre @ 2013-08-28 7:31 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Jean-Christophe PLAGNIOL-VILLARD,
Linus Walleij, Richard Genoud, Jiri Kosina, devicetree, linux-doc,
linux-kernel, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] pinctrl: at91: add support for OUTPUT config
2013-08-28 7:31 ` Nicolas Ferre
@ 2013-08-28 7:35 ` boris brezillon
2013-08-28 7:43 ` Nicolas Ferre
0 siblings, 1 reply; 9+ messages in thread
From: boris brezillon @ 2013-08-28 7:35 UTC (permalink / raw)
To: Nicolas Ferre
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Jean-Christophe PLAGNIOL-VILLARD,
Linus Walleij, Richard Genoud, Jiri Kosina, devicetree, linux-doc,
linux-kernel, linux-arm-kernel
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,
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] pinctrl: at91: add support for OUTPUT config
2013-08-28 7:35 ` boris brezillon
@ 2013-08-28 7:43 ` Nicolas Ferre
2013-08-28 8:08 ` Nicolas Ferre
0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Ferre @ 2013-08-28 7:43 UTC (permalink / raw)
To: boris brezillon
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Jean-Christophe PLAGNIOL-VILLARD,
Linus Walleij, Richard Genoud, Jiri Kosina, devicetree, linux-doc,
linux-kernel, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] pinctrl: at91: add support for OUTPUT config
2013-08-28 7:43 ` Nicolas Ferre
@ 2013-08-28 8:08 ` Nicolas Ferre
0 siblings, 0 replies; 9+ messages in thread
From: Nicolas Ferre @ 2013-08-28 8:08 UTC (permalink / raw)
To: boris brezillon, Linus Walleij
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Jean-Christophe PLAGNIOL-VILLARD,
Richard Genoud, Jiri Kosina, devicetree, linux-doc, linux-kernel,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pinctrl: at91: add support for OUTPUT config
2013-08-27 14:51 [PATCH] pinctrl: at91: add support for OUTPUT config Boris BREZILLON
2013-08-27 15:01 ` boris brezillon
2013-08-28 7:31 ` Nicolas Ferre
@ 2013-08-28 13:46 ` Linus Walleij
2013-08-28 13:48 ` Linus Walleij
2 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2013-08-28 13:46 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Jean-Christophe PLAGNIOL-VILLARD,
Nicolas Ferre, Richard Genoud, Jiri Kosina,
devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pinctrl: at91: add support for OUTPUT config
2013-08-28 13:46 ` Linus Walleij
@ 2013-08-28 13:48 ` Linus Walleij
2013-08-28 14:01 ` boris brezillon
0 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2013-08-28 13:48 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Jean-Christophe PLAGNIOL-VILLARD,
Nicolas Ferre, Richard Genoud, Jiri Kosina,
devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pinctrl: at91: add support for OUTPUT config
2013-08-28 13:48 ` Linus Walleij
@ 2013-08-28 14:01 ` boris brezillon
0 siblings, 0 replies; 9+ messages in thread
From: boris brezillon @ 2013-08-28 14:01 UTC (permalink / raw)
To: Linus Walleij
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Jean-Christophe PLAGNIOL-VILLARD,
Nicolas Ferre, Richard Genoud, Jiri Kosina,
devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
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
^ permalink raw reply [flat|nested] 9+ messages in thread