* [PATCH 1/3] pinctrl: at91: correct a few typos
@ 2013-12-07 13:08 ` Alexandre Belloni
0 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-07 13:08 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard
Cc: Nicolas Ferre, linux-kernel, linux-arm-kernel, Linus Walleij,
Alexandre Belloni
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
drivers/pinctrl/pinctrl-at91.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index a7549c4c83b4..6446dc804aa7 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -118,7 +118,7 @@ struct at91_pin_group {
};
/**
- * struct at91_pinctrl_mux_ops - describes an At91 mux ops group
+ * struct at91_pinctrl_mux_ops - describes an AT91 mux ops group
* on new IP with support for periph C and D the way to mux in
* periph A and B has changed
* So provide the right call back
@@ -1396,7 +1396,7 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
chained_irq_enter(chip, desc);
for (;;) {
/* Reading ISR acks pending (edge triggered) GPIO interrupts.
- * When there none are pending, we're finished unless we need
+ * When there are none pending, we're finished unless we need
* to process multiple banks (like ID_PIOCDE on sam9263).
*/
isr = readl_relaxed(pio + PIO_ISR) & readl_relaxed(pio + PIO_IMR);
@@ -1505,7 +1505,7 @@ static int at91_gpio_of_irq_setup(struct device_node *node,
prev = gpio_chips[at91_gpio->pioc_idx - 1];
/* The top level handler handles one bank of GPIOs, except
- * on some SoC it can handles up to three...
+ * on some SoC it can handle up to three...
* We only set up the handler for the first of the list.
*/
if (prev && prev->next == at91_gpio)
--
1.8.3.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 1/3] pinctrl: at91: correct a few typos
@ 2013-12-07 13:08 ` Alexandre Belloni
0 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-07 13:08 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
drivers/pinctrl/pinctrl-at91.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index a7549c4c83b4..6446dc804aa7 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -118,7 +118,7 @@ struct at91_pin_group {
};
/**
- * struct at91_pinctrl_mux_ops - describes an At91 mux ops group
+ * struct at91_pinctrl_mux_ops - describes an AT91 mux ops group
* on new IP with support for periph C and D the way to mux in
* periph A and B has changed
* So provide the right call back
@@ -1396,7 +1396,7 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
chained_irq_enter(chip, desc);
for (;;) {
/* Reading ISR acks pending (edge triggered) GPIO interrupts.
- * When there none are pending, we're finished unless we need
+ * When there are none pending, we're finished unless we need
* to process multiple banks (like ID_PIOCDE on sam9263).
*/
isr = readl_relaxed(pio + PIO_ISR) & readl_relaxed(pio + PIO_IMR);
@@ -1505,7 +1505,7 @@ static int at91_gpio_of_irq_setup(struct device_node *node,
prev = gpio_chips[at91_gpio->pioc_idx - 1];
/* The top level handler handles one bank of GPIOs, except
- * on some SoC it can handles up to three...
+ * on some SoC it can handle up to three...
* We only set up the handler for the first of the list.
*/
if (prev && prev->next == at91_gpio)
--
1.8.3.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
2013-12-07 13:08 ` Alexandre Belloni
@ 2013-12-07 13:08 ` Alexandre Belloni
-1 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-07 13:08 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard
Cc: Nicolas Ferre, linux-kernel, linux-arm-kernel, Linus Walleij,
Alexandre Belloni
When passing a not initialized config parameter, at91_pinconf_get() would return
a bogus value. Fix that by initializing it to zero before using it.
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
drivers/pinctrl/pinctrl-at91.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 6446dc804aa7..b0b78f3468ae 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -722,7 +722,8 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
unsigned pin;
int div;
- dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, __LINE__, pin_id, *config);
+ *config = 0;
+ dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
pio = pin_to_controller(info, pin_to_bank(pin_id));
pin = pin_id % MAX_NB_GPIO_PER_BANK;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
@ 2013-12-07 13:08 ` Alexandre Belloni
0 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-07 13:08 UTC (permalink / raw)
To: linux-arm-kernel
When passing a not initialized config parameter, at91_pinconf_get() would return
a bogus value. Fix that by initializing it to zero before using it.
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
drivers/pinctrl/pinctrl-at91.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 6446dc804aa7..b0b78f3468ae 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -722,7 +722,8 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
unsigned pin;
int div;
- dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, __LINE__, pin_id, *config);
+ *config = 0;
+ dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
pio = pin_to_controller(info, pin_to_bank(pin_id));
pin = pin_id % MAX_NB_GPIO_PER_BANK;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
2013-12-07 13:08 ` Alexandre Belloni
@ 2013-12-07 13:08 ` Alexandre Belloni
-1 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-07 13:08 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard
Cc: Nicolas Ferre, linux-kernel, linux-arm-kernel, Linus Walleij,
Alexandre Belloni
This allows to get the pin configuration by using debugfs. On my system:
# cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
drivers/pinctrl/pinctrl-at91.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index b0b78f3468ae..bcfc8a2eebca 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -784,10 +784,35 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
return 0;
}
+#define DBG_SHOW_FLAG(flag) do { \
+ if (config & flag) { \
+ if (num_conf) \
+ seq_puts(s, "|"); \
+ seq_puts(s, #flag); \
+ num_conf++; \
+ } \
+} while (0)
+
static void at91_pinconf_dbg_show(struct pinctrl_dev *pctldev,
struct seq_file *s, unsigned pin_id)
{
+ unsigned long config;
+ int ret, val, num_conf = 0;
+
+ ret = at91_pinconf_get(pctldev, pin_id, &config);
+
+ DBG_SHOW_FLAG(MULTI_DRIVE);
+ DBG_SHOW_FLAG(PULL_UP);
+ DBG_SHOW_FLAG(PULL_DOWN);
+ DBG_SHOW_FLAG(DIS_SCHMIT);
+ DBG_SHOW_FLAG(DEGLITCH);
+ DBG_SHOW_FLAG(DEBOUNCE);
+ if (config & DEBOUNCE) {
+ val = config >> DEBOUNCE_VAL_SHIFT;
+ seq_printf(s, "(%d)", val);
+ }
+ return;
}
static void at91_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
--
1.8.3.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
@ 2013-12-07 13:08 ` Alexandre Belloni
0 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-07 13:08 UTC (permalink / raw)
To: linux-arm-kernel
This allows to get the pin configuration by using debugfs. On my system:
# cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
drivers/pinctrl/pinctrl-at91.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index b0b78f3468ae..bcfc8a2eebca 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -784,10 +784,35 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
return 0;
}
+#define DBG_SHOW_FLAG(flag) do { \
+ if (config & flag) { \
+ if (num_conf) \
+ seq_puts(s, "|"); \
+ seq_puts(s, #flag); \
+ num_conf++; \
+ } \
+} while (0)
+
static void at91_pinconf_dbg_show(struct pinctrl_dev *pctldev,
struct seq_file *s, unsigned pin_id)
{
+ unsigned long config;
+ int ret, val, num_conf = 0;
+
+ ret = at91_pinconf_get(pctldev, pin_id, &config);
+
+ DBG_SHOW_FLAG(MULTI_DRIVE);
+ DBG_SHOW_FLAG(PULL_UP);
+ DBG_SHOW_FLAG(PULL_DOWN);
+ DBG_SHOW_FLAG(DIS_SCHMIT);
+ DBG_SHOW_FLAG(DEGLITCH);
+ DBG_SHOW_FLAG(DEBOUNCE);
+ if (config & DEBOUNCE) {
+ val = config >> DEBOUNCE_VAL_SHIFT;
+ seq_printf(s, "(%d)", val);
+ }
+ return;
}
static void at91_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
--
1.8.3.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
2013-12-07 13:08 ` Alexandre Belloni
@ 2013-12-09 8:24 ` Nicolas Ferre
-1 siblings, 0 replies; 36+ messages in thread
From: Nicolas Ferre @ 2013-12-09 8:24 UTC (permalink / raw)
To: Alexandre Belloni, Jean-Christophe Plagniol-Villard, Linus Walleij
Cc: linux-kernel, linux-arm-kernel
On 07/12/2013 14:08, Alexandre Belloni :
> When passing a not initialized config parameter, at91_pinconf_get() would return
> a bogus value. Fix that by initializing it to zero before using it.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
> drivers/pinctrl/pinctrl-at91.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 6446dc804aa7..b0b78f3468ae 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -722,7 +722,8 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> unsigned pin;
> int div;
>
> - dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, __LINE__, pin_id, *config);
> + *config = 0;
> + dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
Beyond this patch, I must say that I am puzzled by this function.
What I read from the prototype documentation and what I see in different
implementations is different...
Linus, can we have a review of this function because it seems not in
line with what is used for u300 (but on the other hand looks like the
what is returned by pinctrl-exynos5440.c driver for example).
What would be the consequences if we change this function's behavior: I
mean use of -EINVAL for pin configuration "available but disabled" as
said in include/linux/pinctrl/pinconf.h?
Bye,
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
@ 2013-12-09 8:24 ` Nicolas Ferre
0 siblings, 0 replies; 36+ messages in thread
From: Nicolas Ferre @ 2013-12-09 8:24 UTC (permalink / raw)
To: linux-arm-kernel
On 07/12/2013 14:08, Alexandre Belloni :
> When passing a not initialized config parameter, at91_pinconf_get() would return
> a bogus value. Fix that by initializing it to zero before using it.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
> drivers/pinctrl/pinctrl-at91.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 6446dc804aa7..b0b78f3468ae 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -722,7 +722,8 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> unsigned pin;
> int div;
>
> - dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, __LINE__, pin_id, *config);
> + *config = 0;
> + dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
Beyond this patch, I must say that I am puzzled by this function.
What I read from the prototype documentation and what I see in different
implementations is different...
Linus, can we have a review of this function because it seems not in
line with what is used for u300 (but on the other hand looks like the
what is returned by pinctrl-exynos5440.c driver for example).
What would be the consequences if we change this function's behavior: I
mean use of -EINVAL for pin configuration "available but disabled" as
said in include/linux/pinctrl/pinconf.h?
Bye,
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
2013-12-09 8:24 ` Nicolas Ferre
@ 2013-12-09 9:55 ` Alexandre Belloni
-1 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-09 9:55 UTC (permalink / raw)
To: Nicolas Ferre, Jean-Christophe Plagniol-Villard, Linus Walleij
Cc: linux-kernel, linux-arm-kernel
Hi,
On 09/12/2013 09:24, Nicolas Ferre wrote:
> On 07/12/2013 14:08, Alexandre Belloni :
>> When passing a not initialized config parameter, at91_pinconf_get()
>> would return
>> a bogus value. Fix that by initializing it to zero before using it.
>>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> ---
>> drivers/pinctrl/pinctrl-at91.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-at91.c
>> b/drivers/pinctrl/pinctrl-at91.c
>> index 6446dc804aa7..b0b78f3468ae 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -722,7 +722,8 @@ static int at91_pinconf_get(struct pinctrl_dev
>> *pctldev,
>> unsigned pin;
>> int div;
>>
>> - dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__,
>> __LINE__, pin_id, *config);
>> + *config = 0;
>> + dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
>> pio = pin_to_controller(info, pin_to_bank(pin_id));
>> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
> Beyond this patch, I must say that I am puzzled by this function.
>
> What I read from the prototype documentation and what I see in
> different implementations is different...
>
> Linus, can we have a review of this function because it seems not in
> line with what is used for u300 (but on the other hand looks like the
> what is returned by pinctrl-exynos5440.c driver for example).
>
> What would be the consequences if we change this function's behavior:
> I mean use of -EINVAL for pin configuration "available but disabled"
> as said in include/linux/pinctrl/pinconf.h?
>
>From what I understand, it doesn't really matter because that function
is never used. I guess the best implementation is the tegra one ;)
It is only called in one specific case:
- you have ops->is_generic == true (that is only true for a few
implmentations)
- and you are dumping the pinconf state using debugfs
I'm actually wondering if the checks for the ops->pin_config_get are not
a bit overkill. We check for that function in:
- pinconf_check_ops()
- before calling it in pin_config_get_for_pin() which is only used
once, in the same path : dump using debugfs and having ops->is_generic
== true
- in pinconf_pins_show() which is the function calling also in the same
path
What I would do is:
- remove all the checks in pinconf_check_ops() and pinconf_pins_show()
so that people are not pressured to implement a function that is simply
never used.
- modify pin_config_get_for_pin() by removing the dev_err() call and
returning -ENOTSUPP instead of -EINVAL (it doesn't change the behaviour
but I feel -ENOTSUPP is more appropriate)
I have a patch ready but I can't test it as I don't own any of the
is_generic platforms.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
@ 2013-12-09 9:55 ` Alexandre Belloni
0 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-09 9:55 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 09/12/2013 09:24, Nicolas Ferre wrote:
> On 07/12/2013 14:08, Alexandre Belloni :
>> When passing a not initialized config parameter, at91_pinconf_get()
>> would return
>> a bogus value. Fix that by initializing it to zero before using it.
>>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> ---
>> drivers/pinctrl/pinctrl-at91.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-at91.c
>> b/drivers/pinctrl/pinctrl-at91.c
>> index 6446dc804aa7..b0b78f3468ae 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -722,7 +722,8 @@ static int at91_pinconf_get(struct pinctrl_dev
>> *pctldev,
>> unsigned pin;
>> int div;
>>
>> - dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__,
>> __LINE__, pin_id, *config);
>> + *config = 0;
>> + dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
>> pio = pin_to_controller(info, pin_to_bank(pin_id));
>> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
> Beyond this patch, I must say that I am puzzled by this function.
>
> What I read from the prototype documentation and what I see in
> different implementations is different...
>
> Linus, can we have a review of this function because it seems not in
> line with what is used for u300 (but on the other hand looks like the
> what is returned by pinctrl-exynos5440.c driver for example).
>
> What would be the consequences if we change this function's behavior:
> I mean use of -EINVAL for pin configuration "available but disabled"
> as said in include/linux/pinctrl/pinconf.h?
>
>From what I understand, it doesn't really matter because that function
is never used. I guess the best implementation is the tegra one ;)
It is only called in one specific case:
- you have ops->is_generic == true (that is only true for a few
implmentations)
- and you are dumping the pinconf state using debugfs
I'm actually wondering if the checks for the ops->pin_config_get are not
a bit overkill. We check for that function in:
- pinconf_check_ops()
- before calling it in pin_config_get_for_pin() which is only used
once, in the same path : dump using debugfs and having ops->is_generic
== true
- in pinconf_pins_show() which is the function calling also in the same
path
What I would do is:
- remove all the checks in pinconf_check_ops() and pinconf_pins_show()
so that people are not pressured to implement a function that is simply
never used.
- modify pin_config_get_for_pin() by removing the dev_err() call and
returning -ENOTSUPP instead of -EINVAL (it doesn't change the behaviour
but I feel -ENOTSUPP is more appropriate)
I have a patch ready but I can't test it as I don't own any of the
is_generic platforms.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
2013-12-09 8:24 ` Nicolas Ferre
@ 2013-12-12 14:38 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 14:38 UTC (permalink / raw)
To: Nicolas Ferre
Cc: Alexandre Belloni, Jean-Christophe Plagniol-Villard,
linux-kernel, linux-arm-kernel
On Mon, Dec 9, 2013 at 9:24 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>> - dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__,
>> __LINE__, pin_id, *config);
>> + *config = 0;
>> + dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__,
>> pin_id);
>> pio = pin_to_controller(info, pin_to_bank(pin_id));
>> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
>
> Beyond this patch, I must say that I am puzzled by this function.
>
> What I read from the prototype documentation and what I see in different
> implementations is different...
Yeah, we need to fix this mess.
> Linus, can we have a review of this function because it seems not in line
> with what is used for u300 (but on the other hand looks like the what is
> returned by pinctrl-exynos5440.c driver for example).
It is supposed to read out one config at the time, if and only if used
with the generic pin config.
Typically:
enum pin_config_param param = (enum pin_config_param) *config;
switch (param) {
case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
*config = 0;
if (biasmode)
return 0;
else
return -EINVAL;
break;
(...)
return -ENOTSUPP;
However AT91 is *not* using generic pin config, so the semantics of
this call is driver-dependent. In your case the implementation get all
the configs at once, which is an efficient shortcut if you don't need
to be general and enumerat all possible configs.
> What would be the consequences if we change this function's behavior: I mean
> use of -EINVAL for pin configuration "available but disabled" as said in
> include/linux/pinctrl/pinconf.h?
That code will propagate back ... I guess you'd have to test it :-/
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
@ 2013-12-12 14:38 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 14:38 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Dec 9, 2013 at 9:24 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>> - dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__,
>> __LINE__, pin_id, *config);
>> + *config = 0;
>> + dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__,
>> pin_id);
>> pio = pin_to_controller(info, pin_to_bank(pin_id));
>> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
>
> Beyond this patch, I must say that I am puzzled by this function.
>
> What I read from the prototype documentation and what I see in different
> implementations is different...
Yeah, we need to fix this mess.
> Linus, can we have a review of this function because it seems not in line
> with what is used for u300 (but on the other hand looks like the what is
> returned by pinctrl-exynos5440.c driver for example).
It is supposed to read out one config at the time, if and only if used
with the generic pin config.
Typically:
enum pin_config_param param = (enum pin_config_param) *config;
switch (param) {
case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
*config = 0;
if (biasmode)
return 0;
else
return -EINVAL;
break;
(...)
return -ENOTSUPP;
However AT91 is *not* using generic pin config, so the semantics of
this call is driver-dependent. In your case the implementation get all
the configs at once, which is an efficient shortcut if you don't need
to be general and enumerat all possible configs.
> What would be the consequences if we change this function's behavior: I mean
> use of -EINVAL for pin configuration "available but disabled" as said in
> include/linux/pinctrl/pinconf.h?
That code will propagate back ... I guess you'd have to test it :-/
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
2013-12-09 9:55 ` Alexandre Belloni
@ 2013-12-12 14:40 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 14:40 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-kernel,
linux-arm-kernel
On Mon, Dec 9, 2013 at 10:55 AM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> I'm actually wondering if the checks for the ops->pin_config_get are not
> a bit overkill. We check for that function in:
> - pinconf_check_ops()
> - before calling it in pin_config_get_for_pin() which is only used
> once, in the same path : dump using debugfs and having ops->is_generic
> == true
> - in pinconf_pins_show() which is the function calling also in the same
> path
>
> What I would do is:
> - remove all the checks in pinconf_check_ops() and pinconf_pins_show()
> so that people are not pressured to implement a function that is simply
> never used.
> - modify pin_config_get_for_pin() by removing the dev_err() call and
> returning -ENOTSUPP instead of -EINVAL (it doesn't change the behaviour
> but I feel -ENOTSUPP is more appropriate)
>
> I have a patch ready but I can't test it as I don't own any of the
> is_generic platforms.
Mail it out with a [CFT: ] "call for testing" prefix.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
@ 2013-12-12 14:40 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 14:40 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Dec 9, 2013 at 10:55 AM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> I'm actually wondering if the checks for the ops->pin_config_get are not
> a bit overkill. We check for that function in:
> - pinconf_check_ops()
> - before calling it in pin_config_get_for_pin() which is only used
> once, in the same path : dump using debugfs and having ops->is_generic
> == true
> - in pinconf_pins_show() which is the function calling also in the same
> path
>
> What I would do is:
> - remove all the checks in pinconf_check_ops() and pinconf_pins_show()
> so that people are not pressured to implement a function that is simply
> never used.
> - modify pin_config_get_for_pin() by removing the dev_err() call and
> returning -ENOTSUPP instead of -EINVAL (it doesn't change the behaviour
> but I feel -ENOTSUPP is more appropriate)
>
> I have a patch ready but I can't test it as I don't own any of the
> is_generic platforms.
Mail it out with a [CFT: ] "call for testing" prefix.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/3] pinctrl: at91: correct a few typos
2013-12-07 13:08 ` Alexandre Belloni
@ 2013-12-12 14:42 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 14:42 UTC (permalink / raw)
To: Alexandre Belloni, Laurent Pinchart
Cc: Jean-Christophe Plagniol-Villard, Nicolas Ferre, linux-kernel,
linux-arm-kernel
On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Patch applied.
Please include Laurent Pinchart on the To: line on AT91 pin control
patches.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/3] pinctrl: at91: correct a few typos
@ 2013-12-12 14:42 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 14:42 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Patch applied.
Please include Laurent Pinchart on the To: line on AT91 pin control
patches.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
2013-12-07 13:08 ` Alexandre Belloni
@ 2013-12-12 14:44 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 14:44 UTC (permalink / raw)
To: Alexandre Belloni, Laurent Pinchart
Cc: Jean-Christophe Plagniol-Villard, Nicolas Ferre, linux-kernel,
linux-arm-kernel
On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> When passing a not initialized config parameter, at91_pinconf_get() would return
> a bogus value. Fix that by initializing it to zero before using it.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
@ 2013-12-12 14:44 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> When passing a not initialized config parameter, at91_pinconf_get() would return
> a bogus value. Fix that by initializing it to zero before using it.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
2013-12-07 13:08 ` Alexandre Belloni
@ 2013-12-12 14:45 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 14:45 UTC (permalink / raw)
To: Alexandre Belloni, Laurent Pinchart
Cc: Jean-Christophe Plagniol-Villard, Nicolas Ferre, linux-kernel,
linux-arm-kernel
On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> This allows to get the pin configuration by using debugfs. On my system:
> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
This is a more invasive patch so I prefer if Laurent ACK this before
I apply it.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
@ 2013-12-12 14:45 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 14:45 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> This allows to get the pin configuration by using debugfs. On my system:
> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
This is a more invasive patch so I prefer if Laurent ACK this before
I apply it.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/3] pinctrl: at91: correct a few typos
2013-12-12 14:42 ` Linus Walleij
@ 2013-12-12 14:47 ` Laurent Pinchart
-1 siblings, 0 replies; 36+ messages in thread
From: Laurent Pinchart @ 2013-12-12 14:47 UTC (permalink / raw)
To: Linus Walleij
Cc: Alexandre Belloni, Jean-Christophe Plagniol-Villard,
Nicolas Ferre, linux-kernel, linux-arm-kernel
Hi Linus,
On Thursday 12 December 2013 15:42:28 Linus Walleij wrote:
> On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni wrote:
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>
> Patch applied.
>
> Please include Laurent Pinchart on the To: line on AT91 pin control
> patches.
I understand that you would like patch to have a wide review coverage, but why
me ? :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/3] pinctrl: at91: correct a few typos
@ 2013-12-12 14:47 ` Laurent Pinchart
0 siblings, 0 replies; 36+ messages in thread
From: Laurent Pinchart @ 2013-12-12 14:47 UTC (permalink / raw)
To: linux-arm-kernel
Hi Linus,
On Thursday 12 December 2013 15:42:28 Linus Walleij wrote:
> On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni wrote:
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>
> Patch applied.
>
> Please include Laurent Pinchart on the To: line on AT91 pin control
> patches.
I understand that you would like patch to have a wide review coverage, but why
me ? :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
2013-12-12 14:40 ` Linus Walleij
@ 2013-12-12 15:33 ` Alexandre Belloni
-1 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-12 15:33 UTC (permalink / raw)
To: Linus Walleij
Cc: Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-kernel,
linux-arm-kernel
On 12/12/2013 15:40, Linus Walleij wrote:
> Mail it out with a [CFT: ] "call for testing" prefix.
>
I actually already sent it but without the CFT. Do you want me to resend ?
Regards,
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/3] pinctrl: at91: initialize config parameter to 0
@ 2013-12-12 15:33 ` Alexandre Belloni
0 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-12 15:33 UTC (permalink / raw)
To: linux-arm-kernel
On 12/12/2013 15:40, Linus Walleij wrote:
> Mail it out with a [CFT: ] "call for testing" prefix.
>
I actually already sent it but without the CFT. Do you want me to resend ?
Regards,
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
2013-12-12 14:45 ` Linus Walleij
@ 2013-12-12 15:50 ` Alexandre Belloni
-1 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-12 15:50 UTC (permalink / raw)
To: Linus Walleij, Laurent Pinchart
Cc: Jean-Christophe Plagniol-Villard, Nicolas Ferre, linux-kernel,
linux-arm-kernel
On 12/12/2013 15:45, Linus Walleij wrote:
> On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
>
>> This allows to get the pin configuration by using debugfs. On my system:
>> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
>>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> This is a more invasive patch so I prefer if Laurent ACK this before
> I apply it.
Hum, just to be clear, this is an Atmel chip and the Atmel maintainers
were already in copy. I believe Laurent already has a lot on his plate
with CDF ;)
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
@ 2013-12-12 15:50 ` Alexandre Belloni
0 siblings, 0 replies; 36+ messages in thread
From: Alexandre Belloni @ 2013-12-12 15:50 UTC (permalink / raw)
To: linux-arm-kernel
On 12/12/2013 15:45, Linus Walleij wrote:
> On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
>
>> This allows to get the pin configuration by using debugfs. On my system:
>> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
>>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> This is a more invasive patch so I prefer if Laurent ACK this before
> I apply it.
Hum, just to be clear, this is an Atmel chip and the Atmel maintainers
were already in copy. I believe Laurent already has a lot on his plate
with CDF ;)
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
2013-12-12 15:50 ` Alexandre Belloni
@ 2013-12-12 16:04 ` Laurent Pinchart
-1 siblings, 0 replies; 36+ messages in thread
From: Laurent Pinchart @ 2013-12-12 16:04 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Linus Walleij, Jean-Christophe Plagniol-Villard, Nicolas Ferre,
linux-kernel, linux-arm-kernel
On Thursday 12 December 2013 16:50:20 Alexandre Belloni wrote:
> On 12/12/2013 15:45, Linus Walleij wrote:
> > On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
> >
> > <alexandre.belloni@free-electrons.com> wrote:
> >> This allows to get the pin configuration by using debugfs. On my system:
> >> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
> >>
> >> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> >
> > This is a more invasive patch so I prefer if Laurent ACK this before
> > I apply it.
>
> Hum, just to be clear, this is an Atmel chip and the Atmel maintainers
> were already in copy. I believe Laurent already has a lot on his plate
> with CDF ;)
Not to mention the rest. I'd love to help with at91 pinctrl, but I think I
have to draw the line somewhere. I hope it won't come as a shock that I can't
review the whole kernel :-D
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
@ 2013-12-12 16:04 ` Laurent Pinchart
0 siblings, 0 replies; 36+ messages in thread
From: Laurent Pinchart @ 2013-12-12 16:04 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 12 December 2013 16:50:20 Alexandre Belloni wrote:
> On 12/12/2013 15:45, Linus Walleij wrote:
> > On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
> >
> > <alexandre.belloni@free-electrons.com> wrote:
> >> This allows to get the pin configuration by using debugfs. On my system:
> >> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
> >>
> >> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> >
> > This is a more invasive patch so I prefer if Laurent ACK this before
> > I apply it.
>
> Hum, just to be clear, this is an Atmel chip and the Atmel maintainers
> were already in copy. I believe Laurent already has a lot on his plate
> with CDF ;)
Not to mention the rest. I'd love to help with at91 pinctrl, but I think I
have to draw the line somewhere. I hope it won't come as a shock that I can't
review the whole kernel :-D
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/3] pinctrl: at91: correct a few typos
2013-12-12 14:47 ` Laurent Pinchart
@ 2013-12-12 21:13 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 21:13 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Alexandre Belloni, Jean-Christophe Plagniol-Villard,
Nicolas Ferre, linux-kernel, linux-arm-kernel
On Thu, Dec 12, 2013 at 3:47 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Linus,
>
> On Thursday 12 December 2013 15:42:28 Linus Walleij wrote:
>> On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni wrote:
>> > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>
>> Patch applied.
>>
>> Please include Laurent Pinchart on the To: line on AT91 pin control
>> patches.
>
> I understand that you would like patch to have a wide review coverage, but why
> me ? :-)
Bah nevermind, too much in my head, I confused AT91 for
SH mobile nothing else... Nicolas and Jean-Christophe is on the
list and all is well.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/3] pinctrl: at91: correct a few typos
@ 2013-12-12 21:13 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 21:13 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 12, 2013 at 3:47 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Linus,
>
> On Thursday 12 December 2013 15:42:28 Linus Walleij wrote:
>> On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni wrote:
>> > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>
>> Patch applied.
>>
>> Please include Laurent Pinchart on the To: line on AT91 pin control
>> patches.
>
> I understand that you would like patch to have a wide review coverage, but why
> me ? :-)
Bah nevermind, too much in my head, I confused AT91 for
SH mobile nothing else... Nicolas and Jean-Christophe is on the
list and all is well.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
2013-12-12 16:04 ` Laurent Pinchart
@ 2013-12-12 21:14 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 21:14 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Alexandre Belloni, Jean-Christophe Plagniol-Villard,
Nicolas Ferre, linux-kernel, linux-arm-kernel
On Thu, Dec 12, 2013 at 5:04 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Thursday 12 December 2013 16:50:20 Alexandre Belloni wrote:
>> On 12/12/2013 15:45, Linus Walleij wrote:
>> > On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
>> >
>> > <alexandre.belloni@free-electrons.com> wrote:
>> >> This allows to get the pin configuration by using debugfs. On my system:
>> >> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
>> >>
>> >> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> >
>> > This is a more invasive patch so I prefer if Laurent ACK this before
>> > I apply it.
>>
>> Hum, just to be clear, this is an Atmel chip and the Atmel maintainers
>> were already in copy. I believe Laurent already has a lot on his plate
>> with CDF ;)
>
> Not to mention the rest. I'd love to help with at91 pinctrl, but I think I
> have to draw the line somewhere. I hope it won't come as a shock that I can't
> review the whole kernel :-D
Sorry about this mess, forget about it, I don't know what is wrong
we me today, probably trying to process too many patches :-)
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
@ 2013-12-12 21:14 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-12 21:14 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 12, 2013 at 5:04 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Thursday 12 December 2013 16:50:20 Alexandre Belloni wrote:
>> On 12/12/2013 15:45, Linus Walleij wrote:
>> > On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
>> >
>> > <alexandre.belloni@free-electrons.com> wrote:
>> >> This allows to get the pin configuration by using debugfs. On my system:
>> >> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
>> >>
>> >> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> >
>> > This is a more invasive patch so I prefer if Laurent ACK this before
>> > I apply it.
>>
>> Hum, just to be clear, this is an Atmel chip and the Atmel maintainers
>> were already in copy. I believe Laurent already has a lot on his plate
>> with CDF ;)
>
> Not to mention the rest. I'd love to help with at91 pinctrl, but I think I
> have to draw the line somewhere. I hope it won't come as a shock that I can't
> review the whole kernel :-D
Sorry about this mess, forget about it, I don't know what is wrong
we me today, probably trying to process too many patches :-)
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
2013-12-07 13:08 ` Alexandre Belloni
@ 2013-12-13 8:50 ` Nicolas Ferre
-1 siblings, 0 replies; 36+ messages in thread
From: Nicolas Ferre @ 2013-12-13 8:50 UTC (permalink / raw)
To: Alexandre Belloni, Jean-Christophe Plagniol-Villard,
Linus Walleij, Laurent Pinchart
Cc: linux-kernel, linux-arm-kernel
On 07/12/2013 14:08, Alexandre Belloni :
> This allows to get the pin configuration by using debugfs. On my system:
> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
I am fine with these helpers:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
But still a little disappointed that Laurent announced that he wouldn't
review the whole kernel ;-)
Bye guys and thanks a lot!
> ---
> drivers/pinctrl/pinctrl-at91.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index b0b78f3468ae..bcfc8a2eebca 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -784,10 +784,35 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> return 0;
> }
>
> +#define DBG_SHOW_FLAG(flag) do { \
> + if (config & flag) { \
> + if (num_conf) \
> + seq_puts(s, "|"); \
> + seq_puts(s, #flag); \
> + num_conf++; \
> + } \
> +} while (0)
> +
> static void at91_pinconf_dbg_show(struct pinctrl_dev *pctldev,
> struct seq_file *s, unsigned pin_id)
> {
> + unsigned long config;
> + int ret, val, num_conf = 0;
> +
> + ret = at91_pinconf_get(pctldev, pin_id, &config);
> +
> + DBG_SHOW_FLAG(MULTI_DRIVE);
> + DBG_SHOW_FLAG(PULL_UP);
> + DBG_SHOW_FLAG(PULL_DOWN);
> + DBG_SHOW_FLAG(DIS_SCHMIT);
> + DBG_SHOW_FLAG(DEGLITCH);
> + DBG_SHOW_FLAG(DEBOUNCE);
> + if (config & DEBOUNCE) {
> + val = config >> DEBOUNCE_VAL_SHIFT;
> + seq_printf(s, "(%d)", val);
> + }
>
> + return;
> }
>
> static void at91_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
@ 2013-12-13 8:50 ` Nicolas Ferre
0 siblings, 0 replies; 36+ messages in thread
From: Nicolas Ferre @ 2013-12-13 8:50 UTC (permalink / raw)
To: linux-arm-kernel
On 07/12/2013 14:08, Alexandre Belloni :
> This allows to get the pin configuration by using debugfs. On my system:
> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
I am fine with these helpers:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
But still a little disappointed that Laurent announced that he wouldn't
review the whole kernel ;-)
Bye guys and thanks a lot!
> ---
> drivers/pinctrl/pinctrl-at91.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index b0b78f3468ae..bcfc8a2eebca 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -784,10 +784,35 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> return 0;
> }
>
> +#define DBG_SHOW_FLAG(flag) do { \
> + if (config & flag) { \
> + if (num_conf) \
> + seq_puts(s, "|"); \
> + seq_puts(s, #flag); \
> + num_conf++; \
> + } \
> +} while (0)
> +
> static void at91_pinconf_dbg_show(struct pinctrl_dev *pctldev,
> struct seq_file *s, unsigned pin_id)
> {
> + unsigned long config;
> + int ret, val, num_conf = 0;
> +
> + ret = at91_pinconf_get(pctldev, pin_id, &config);
> +
> + DBG_SHOW_FLAG(MULTI_DRIVE);
> + DBG_SHOW_FLAG(PULL_UP);
> + DBG_SHOW_FLAG(PULL_DOWN);
> + DBG_SHOW_FLAG(DIS_SCHMIT);
> + DBG_SHOW_FLAG(DEGLITCH);
> + DBG_SHOW_FLAG(DEBOUNCE);
> + if (config & DEBOUNCE) {
> + val = config >> DEBOUNCE_VAL_SHIFT;
> + seq_printf(s, "(%d)", val);
> + }
>
> + return;
> }
>
> static void at91_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
2013-12-07 13:08 ` Alexandre Belloni
@ 2013-12-13 9:34 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-13 9:34 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Jean-Christophe Plagniol-Villard, Nicolas Ferre, linux-kernel,
linux-arm-kernel
On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> This allows to get the pin configuration by using debugfs. On my system:
> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Patch applied with Nicolas' ACK.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show
@ 2013-12-13 9:34 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2013-12-13 9:34 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 7, 2013 at 2:08 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> This allows to get the pin configuration by using debugfs. On my system:
> # cat /sys/kernel/debug/pinctrl/pinctrl.3/pinconf-pins
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Patch applied with Nicolas' ACK.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2013-12-13 9:34 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-07 13:08 [PATCH 1/3] pinctrl: at91: correct a few typos Alexandre Belloni
2013-12-07 13:08 ` Alexandre Belloni
2013-12-07 13:08 ` [PATCH 2/3] pinctrl: at91: initialize config parameter to 0 Alexandre Belloni
2013-12-07 13:08 ` Alexandre Belloni
2013-12-09 8:24 ` Nicolas Ferre
2013-12-09 8:24 ` Nicolas Ferre
2013-12-09 9:55 ` Alexandre Belloni
2013-12-09 9:55 ` Alexandre Belloni
2013-12-12 14:40 ` Linus Walleij
2013-12-12 14:40 ` Linus Walleij
2013-12-12 15:33 ` Alexandre Belloni
2013-12-12 15:33 ` Alexandre Belloni
2013-12-12 14:38 ` Linus Walleij
2013-12-12 14:38 ` Linus Walleij
2013-12-12 14:44 ` Linus Walleij
2013-12-12 14:44 ` Linus Walleij
2013-12-07 13:08 ` [PATCH 3/3] pinctrl: at91: implement at91_pinconf_dbg_show Alexandre Belloni
2013-12-07 13:08 ` Alexandre Belloni
2013-12-12 14:45 ` Linus Walleij
2013-12-12 14:45 ` Linus Walleij
2013-12-12 15:50 ` Alexandre Belloni
2013-12-12 15:50 ` Alexandre Belloni
2013-12-12 16:04 ` Laurent Pinchart
2013-12-12 16:04 ` Laurent Pinchart
2013-12-12 21:14 ` Linus Walleij
2013-12-12 21:14 ` Linus Walleij
2013-12-13 8:50 ` Nicolas Ferre
2013-12-13 8:50 ` Nicolas Ferre
2013-12-13 9:34 ` Linus Walleij
2013-12-13 9:34 ` Linus Walleij
2013-12-12 14:42 ` [PATCH 1/3] pinctrl: at91: correct a few typos Linus Walleij
2013-12-12 14:42 ` Linus Walleij
2013-12-12 14:47 ` Laurent Pinchart
2013-12-12 14:47 ` Laurent Pinchart
2013-12-12 21:13 ` Linus Walleij
2013-12-12 21:13 ` Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.