* [PATCH v2 1/3] pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN,UP} argument
2016-11-11 2:44 [PATCH v2 0/3] pinctrl: sunxi: Support generic pinconf functions Chen-Yu Tsai
@ 2016-11-11 2:44 ` Chen-Yu Tsai
2016-11-11 8:34 ` Maxime Ripard
2016-11-15 9:16 ` Linus Walleij
2016-11-11 2:44 ` [PATCH v2 2/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware Chen-Yu Tsai
2016-11-11 2:44 ` [PATCH v2 3/3] pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper Chen-Yu Tsai
2 siblings, 2 replies; 10+ messages in thread
From: Chen-Yu Tsai @ 2016-11-11 2:44 UTC (permalink / raw)
To: Linus Walleij, Maxime Ripard
Cc: Chen-Yu Tsai, linux-gpio, linux-arm-kernel, linux-kernel, linux-sunxi
According to pinconf-generic.h, the argument for
PIN_CONFIG_BIAS_PULL_{DOWN,UP} is non-zero if the bias is enabled
with a pull up/down resistor, zero if it is directly connected
to VDD or ground.
Since Allwinner hardware uses a weak pull resistor internally,
the argument should be 1.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index e199d95af8c0..e04edda8629d 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -291,12 +291,16 @@ static unsigned long *sunxi_pctrl_build_pin_config(struct device_node *node,
if (sunxi_pctrl_has_bias_prop(node)) {
int pull = sunxi_pctrl_parse_bias_prop(node);
+ int arg = 0;
if (pull < 0) {
ret = pull;
goto err_free;
}
- pinconfig[idx++] = pinconf_to_config_packed(pull, 0);
+ if (pull != PIN_CONFIG_BIAS_DISABLE)
+ arg = 1; /* hardware uses weak pull resistors */
+
+ pinconfig[idx++] = pinconf_to_config_packed(pull, arg);
}
--
2.10.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN,UP} argument
2016-11-11 2:44 ` [PATCH v2 1/3] pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN,UP} argument Chen-Yu Tsai
@ 2016-11-11 8:34 ` Maxime Ripard
2016-11-15 9:16 ` Linus Walleij
1 sibling, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2016-11-11 8:34 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Linus Walleij, linux-gpio, linux-arm-kernel, linux-kernel, linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 606 bytes --]
On Fri, Nov 11, 2016 at 10:44:53AM +0800, Chen-Yu Tsai wrote:
> According to pinconf-generic.h, the argument for
> PIN_CONFIG_BIAS_PULL_{DOWN,UP} is non-zero if the bias is enabled
> with a pull up/down resistor, zero if it is directly connected
> to VDD or ground.
>
> Since Allwinner hardware uses a weak pull resistor internally,
> the argument should be 1.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN,UP} argument
2016-11-11 2:44 ` [PATCH v2 1/3] pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN,UP} argument Chen-Yu Tsai
2016-11-11 8:34 ` Maxime Ripard
@ 2016-11-15 9:16 ` Linus Walleij
1 sibling, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2016-11-15 9:16 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Maxime Ripard, linux-gpio, linux-arm-kernel, linux-kernel, linux-sunxi
On Fri, Nov 11, 2016 at 3:44 AM, Chen-Yu Tsai <wens@csie.org> wrote:
> According to pinconf-generic.h, the argument for
> PIN_CONFIG_BIAS_PULL_{DOWN,UP} is non-zero if the bias is enabled
> with a pull up/down resistor, zero if it is directly connected
> to VDD or ground.
>
> Since Allwinner hardware uses a weak pull resistor internally,
> the argument should be 1.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Patch applied with Maxime's ACK.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware
2016-11-11 2:44 [PATCH v2 0/3] pinctrl: sunxi: Support generic pinconf functions Chen-Yu Tsai
2016-11-11 2:44 ` [PATCH v2 1/3] pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN,UP} argument Chen-Yu Tsai
@ 2016-11-11 2:44 ` Chen-Yu Tsai
2016-11-11 8:36 ` Maxime Ripard
2016-11-11 2:44 ` [PATCH v2 3/3] pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper Chen-Yu Tsai
2 siblings, 1 reply; 10+ messages in thread
From: Chen-Yu Tsai @ 2016-11-11 2:44 UTC (permalink / raw)
To: Linus Walleij, Maxime Ripard
Cc: Chen-Yu Tsai, linux-gpio, linux-arm-kernel, linux-kernel, linux-sunxi
The sunxi pinctrl driver only caches whatever pinconf setting was last
set on a given pingroup. This is not particularly helpful, nor is it
correct.
Fix this by actually reading the hardware registers and returning
the correct results or error codes. Also filter out unsupported
pinconf settings. Since this driver has a peculiar setup of 1 pin
per group, we can support both pin and pingroup pinconf setting
read back with the same code. The sunxi_pconf_reg helper and code
structure is inspired by pinctrl-msm.
With this done we can also claim to support generic pinconf, by
setting .is_generic = true in pinconf_ops.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 84 +++++++++++++++++++++++++++++++++--
1 file changed, 81 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index e04edda8629d..3e9f7c675d36 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -438,15 +438,91 @@ static const struct pinctrl_ops sunxi_pctrl_ops = {
.get_group_pins = sunxi_pctrl_get_group_pins,
};
+static int sunxi_pconf_reg(unsigned pin, enum pin_config_param param,
+ u32 *offset, u32 *shift, u32 *mask)
+{
+ switch (param) {
+ case PIN_CONFIG_DRIVE_STRENGTH:
+ *offset = sunxi_dlevel_reg(pin);
+ *shift = sunxi_dlevel_offset(pin);
+ *mask = DLEVEL_PINS_MASK;
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_UP:
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ case PIN_CONFIG_BIAS_DISABLE:
+ *offset = sunxi_pull_reg(pin);
+ *shift = sunxi_pull_offset(pin);
+ *mask = PULL_PINS_MASK;
+ break;
+
+ default:
+ return -ENOTSUPP;
+ }
+
+ return 0;
+}
+
+static int sunxi_pconf_get(struct pinctrl_dev *pctldev, unsigned pin,
+ unsigned long *config)
+{
+ struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ enum pin_config_param param = pinconf_to_config_param(*config);
+ u32 offset, shift, mask, val;
+ u16 arg;
+ int ret;
+
+ pin -= pctl->desc->pin_base;
+
+ ret = sunxi_pconf_reg(pin, param, &offset, &shift, &mask);
+ if (ret < 0)
+ return ret;
+
+ val = (readl(pctl->membase + offset) >> shift) & mask;
+
+ switch (pinconf_to_config_param(*config)) {
+ case PIN_CONFIG_DRIVE_STRENGTH:
+ arg = (val + 1) * 10;
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_UP:
+ if (val != SUN4I_PINCTRL_PULL_UP)
+ return -EINVAL;
+ arg = 1; /* hardware is weak pull-up */
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ if (val != SUN4I_PINCTRL_PULL_DOWN)
+ return -EINVAL;
+ arg = 1; /* hardware is weak pull-down */
+ break;
+
+ case PIN_CONFIG_BIAS_DISABLE:
+ if (val != SUN4I_PINCTRL_NO_PULL)
+ return -EINVAL;
+ arg = 0;
+ break;
+
+ default:
+ /* sunxi_pconf_reg should catch anything unsupported */
+ WARN_ON(1);
+ return -ENOTSUPP;
+ }
+
+ *config = pinconf_to_config_packed(param, arg);
+
+ return 0;
+}
+
static int sunxi_pconf_group_get(struct pinctrl_dev *pctldev,
unsigned group,
unsigned long *config)
{
struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ struct sunxi_pinctrl_group *g = &pctl->groups[group];
- *config = pctl->groups[group].config;
-
- return 0;
+ /* We only support 1 pin per group. Chain it to the pin callback */
+ return sunxi_pconf_get(pctldev, g->pin, config);
}
static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
@@ -518,6 +594,8 @@ static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
}
static const struct pinconf_ops sunxi_pconf_ops = {
+ .is_generic = true,
+ .pin_config_get = sunxi_pconf_get,
.pin_config_group_get = sunxi_pconf_group_get,
.pin_config_group_set = sunxi_pconf_group_set,
};
--
2.10.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware
2016-11-11 2:44 ` [PATCH v2 2/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware Chen-Yu Tsai
@ 2016-11-11 8:36 ` Maxime Ripard
2016-11-11 8:39 ` Chen-Yu Tsai
0 siblings, 1 reply; 10+ messages in thread
From: Maxime Ripard @ 2016-11-11 8:36 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Linus Walleij, linux-gpio, linux-arm-kernel, linux-kernel, linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 3881 bytes --]
On Fri, Nov 11, 2016 at 10:44:54AM +0800, Chen-Yu Tsai wrote:
> The sunxi pinctrl driver only caches whatever pinconf setting was last
> set on a given pingroup. This is not particularly helpful, nor is it
> correct.
>
> Fix this by actually reading the hardware registers and returning
> the correct results or error codes. Also filter out unsupported
> pinconf settings. Since this driver has a peculiar setup of 1 pin
> per group, we can support both pin and pingroup pinconf setting
> read back with the same code. The sunxi_pconf_reg helper and code
> structure is inspired by pinctrl-msm.
>
> With this done we can also claim to support generic pinconf, by
> setting .is_generic = true in pinconf_ops.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 84 +++++++++++++++++++++++++++++++++--
> 1 file changed, 81 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index e04edda8629d..3e9f7c675d36 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -438,15 +438,91 @@ static const struct pinctrl_ops sunxi_pctrl_ops = {
> .get_group_pins = sunxi_pctrl_get_group_pins,
> };
>
> +static int sunxi_pconf_reg(unsigned pin, enum pin_config_param param,
> + u32 *offset, u32 *shift, u32 *mask)
> +{
> + switch (param) {
> + case PIN_CONFIG_DRIVE_STRENGTH:
> + *offset = sunxi_dlevel_reg(pin);
> + *shift = sunxi_dlevel_offset(pin);
> + *mask = DLEVEL_PINS_MASK;
> + break;
> +
> + case PIN_CONFIG_BIAS_PULL_UP:
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + case PIN_CONFIG_BIAS_DISABLE:
> + *offset = sunxi_pull_reg(pin);
> + *shift = sunxi_pull_offset(pin);
> + *mask = PULL_PINS_MASK;
> + break;
> +
> + default:
> + return -ENOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int sunxi_pconf_get(struct pinctrl_dev *pctldev, unsigned pin,
> + unsigned long *config)
> +{
> + struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
> + enum pin_config_param param = pinconf_to_config_param(*config);
> + u32 offset, shift, mask, val;
> + u16 arg;
> + int ret;
> +
> + pin -= pctl->desc->pin_base;
> +
> + ret = sunxi_pconf_reg(pin, param, &offset, &shift, &mask);
> + if (ret < 0)
> + return ret;
> +
> + val = (readl(pctl->membase + offset) >> shift) & mask;
> +
> + switch (pinconf_to_config_param(*config)) {
> + case PIN_CONFIG_DRIVE_STRENGTH:
> + arg = (val + 1) * 10;
> + break;
> +
> + case PIN_CONFIG_BIAS_PULL_UP:
> + if (val != SUN4I_PINCTRL_PULL_UP)
> + return -EINVAL;
> + arg = 1; /* hardware is weak pull-up */
> + break;
> +
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + if (val != SUN4I_PINCTRL_PULL_DOWN)
> + return -EINVAL;
> + arg = 1; /* hardware is weak pull-down */
> + break;
> +
> + case PIN_CONFIG_BIAS_DISABLE:
> + if (val != SUN4I_PINCTRL_NO_PULL)
> + return -EINVAL;
> + arg = 0;
> + break;
> +
> + default:
> + /* sunxi_pconf_reg should catch anything unsupported */
> + WARN_ON(1);
> + return -ENOTSUPP;
> + }
> +
> + *config = pinconf_to_config_packed(param, arg);
> +
> + return 0;
> +}
> +
> static int sunxi_pconf_group_get(struct pinctrl_dev *pctldev,
> unsigned group,
> unsigned long *config)
> {
> struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
> + struct sunxi_pinctrl_group *g = &pctl->groups[group];
>
> - *config = pctl->groups[group].config;
Do we still need this variable? Looking at the code, it doesn't look
that way, and we can remove the caching in the _group_set function and
the variable itslef in the sunxi_pincttrl_group structure.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware
2016-11-11 8:36 ` Maxime Ripard
@ 2016-11-11 8:39 ` Chen-Yu Tsai
0 siblings, 0 replies; 10+ messages in thread
From: Chen-Yu Tsai @ 2016-11-11 8:39 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, Linus Walleij, linux-gpio, linux-arm-kernel,
linux-kernel, linux-sunxi
On Fri, Nov 11, 2016 at 4:36 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Fri, Nov 11, 2016 at 10:44:54AM +0800, Chen-Yu Tsai wrote:
>> The sunxi pinctrl driver only caches whatever pinconf setting was last
>> set on a given pingroup. This is not particularly helpful, nor is it
>> correct.
>>
>> Fix this by actually reading the hardware registers and returning
>> the correct results or error codes. Also filter out unsupported
>> pinconf settings. Since this driver has a peculiar setup of 1 pin
>> per group, we can support both pin and pingroup pinconf setting
>> read back with the same code. The sunxi_pconf_reg helper and code
>> structure is inspired by pinctrl-msm.
>>
>> With this done we can also claim to support generic pinconf, by
>> setting .is_generic = true in pinconf_ops.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 84 +++++++++++++++++++++++++++++++++--
>> 1 file changed, 81 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> index e04edda8629d..3e9f7c675d36 100644
>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> @@ -438,15 +438,91 @@ static const struct pinctrl_ops sunxi_pctrl_ops = {
>> .get_group_pins = sunxi_pctrl_get_group_pins,
>> };
>>
>> +static int sunxi_pconf_reg(unsigned pin, enum pin_config_param param,
>> + u32 *offset, u32 *shift, u32 *mask)
>> +{
>> + switch (param) {
>> + case PIN_CONFIG_DRIVE_STRENGTH:
>> + *offset = sunxi_dlevel_reg(pin);
>> + *shift = sunxi_dlevel_offset(pin);
>> + *mask = DLEVEL_PINS_MASK;
>> + break;
>> +
>> + case PIN_CONFIG_BIAS_PULL_UP:
>> + case PIN_CONFIG_BIAS_PULL_DOWN:
>> + case PIN_CONFIG_BIAS_DISABLE:
>> + *offset = sunxi_pull_reg(pin);
>> + *shift = sunxi_pull_offset(pin);
>> + *mask = PULL_PINS_MASK;
>> + break;
>> +
>> + default:
>> + return -ENOTSUPP;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int sunxi_pconf_get(struct pinctrl_dev *pctldev, unsigned pin,
>> + unsigned long *config)
>> +{
>> + struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
>> + enum pin_config_param param = pinconf_to_config_param(*config);
>> + u32 offset, shift, mask, val;
>> + u16 arg;
>> + int ret;
>> +
>> + pin -= pctl->desc->pin_base;
>> +
>> + ret = sunxi_pconf_reg(pin, param, &offset, &shift, &mask);
>> + if (ret < 0)
>> + return ret;
>> +
>> + val = (readl(pctl->membase + offset) >> shift) & mask;
>> +
>> + switch (pinconf_to_config_param(*config)) {
>> + case PIN_CONFIG_DRIVE_STRENGTH:
>> + arg = (val + 1) * 10;
>> + break;
>> +
>> + case PIN_CONFIG_BIAS_PULL_UP:
>> + if (val != SUN4I_PINCTRL_PULL_UP)
>> + return -EINVAL;
>> + arg = 1; /* hardware is weak pull-up */
>> + break;
>> +
>> + case PIN_CONFIG_BIAS_PULL_DOWN:
>> + if (val != SUN4I_PINCTRL_PULL_DOWN)
>> + return -EINVAL;
>> + arg = 1; /* hardware is weak pull-down */
>> + break;
>> +
>> + case PIN_CONFIG_BIAS_DISABLE:
>> + if (val != SUN4I_PINCTRL_NO_PULL)
>> + return -EINVAL;
>> + arg = 0;
>> + break;
>> +
>> + default:
>> + /* sunxi_pconf_reg should catch anything unsupported */
>> + WARN_ON(1);
>> + return -ENOTSUPP;
>> + }
>> +
>> + *config = pinconf_to_config_packed(param, arg);
>> +
>> + return 0;
>> +}
>> +
>> static int sunxi_pconf_group_get(struct pinctrl_dev *pctldev,
>> unsigned group,
>> unsigned long *config)
>> {
>> struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
>> + struct sunxi_pinctrl_group *g = &pctl->groups[group];
>>
>> - *config = pctl->groups[group].config;
>
> Do we still need this variable? Looking at the code, it doesn't look
> that way, and we can remove the caching in the _group_set function and
> the variable itslef in the sunxi_pincttrl_group structure.
It's actually removed in the next patch. :)
ChenYu
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper
2016-11-11 2:44 [PATCH v2 0/3] pinctrl: sunxi: Support generic pinconf functions Chen-Yu Tsai
2016-11-11 2:44 ` [PATCH v2 1/3] pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN,UP} argument Chen-Yu Tsai
2016-11-11 2:44 ` [PATCH v2 2/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware Chen-Yu Tsai
@ 2016-11-11 2:44 ` Chen-Yu Tsai
2016-11-11 8:38 ` Maxime Ripard
2 siblings, 1 reply; 10+ messages in thread
From: Chen-Yu Tsai @ 2016-11-11 2:44 UTC (permalink / raw)
To: Linus Walleij, Maxime Ripard
Cc: Chen-Yu Tsai, linux-gpio, linux-arm-kernel, linux-kernel, linux-sunxi
The sunxi_pconf_reg helper introduced in the last patch gives us the
chance to rework sunxi_pconf_group_set to have it match the structure
of sunxi_pconf_(group_)get and make it easier to understand.
For each config to set, it:
1. checks if the parameter is supported.
2. checks if the argument is within limits.
3. converts argument to the register value.
4. writes to the register with spinlock held.
As a result the function now blocks unsupported config parameters,
instead of silently ignoring them.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 66 +++++++++++++++++------------------
drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 -
2 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 3e9f7c675d36..fa11a3100346 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -532,23 +532,27 @@ static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
{
struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
struct sunxi_pinctrl_group *g = &pctl->groups[group];
- unsigned long flags;
unsigned pin = g->pin - pctl->desc->pin_base;
- u32 val, mask;
- u16 strength;
- u8 dlevel;
int i;
- spin_lock_irqsave(&pctl->lock, flags);
-
for (i = 0; i < num_configs; i++) {
- switch (pinconf_to_config_param(configs[i])) {
+ enum pin_config_param param;
+ unsigned long flags;
+ u32 offset, shift, mask, reg;
+ u16 arg, val;
+ int ret;
+
+ param = pinconf_to_config_param(configs[i]);
+ arg = pinconf_to_config_argument(configs[i]);
+
+ ret = sunxi_pconf_reg(pin, param, &offset, &shift, &mask);
+ if (ret < 0)
+ return ret;
+
+ switch (param) {
case PIN_CONFIG_DRIVE_STRENGTH:
- strength = pinconf_to_config_argument(configs[i]);
- if (strength > 40) {
- spin_unlock_irqrestore(&pctl->lock, flags);
+ if (arg < 10 || arg > 40)
return -EINVAL;
- }
/*
* We convert from mA to what the register expects:
* 0: 10mA
@@ -556,39 +560,33 @@ static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
* 2: 30mA
* 3: 40mA
*/
- dlevel = strength / 10 - 1;
- val = readl(pctl->membase + sunxi_dlevel_reg(pin));
- mask = DLEVEL_PINS_MASK << sunxi_dlevel_offset(pin);
- writel((val & ~mask)
- | dlevel << sunxi_dlevel_offset(pin),
- pctl->membase + sunxi_dlevel_reg(pin));
+ val = arg / 10 - 1;
break;
case PIN_CONFIG_BIAS_DISABLE:
- val = readl(pctl->membase + sunxi_pull_reg(pin));
- mask = PULL_PINS_MASK << sunxi_pull_offset(pin);
- writel((val & ~mask),
- pctl->membase + sunxi_pull_reg(pin));
+ val = 0;
break;
case PIN_CONFIG_BIAS_PULL_UP:
- val = readl(pctl->membase + sunxi_pull_reg(pin));
- mask = PULL_PINS_MASK << sunxi_pull_offset(pin);
- writel((val & ~mask) | 1 << sunxi_pull_offset(pin),
- pctl->membase + sunxi_pull_reg(pin));
+ if (arg == 0)
+ return -EINVAL;
+ val = 1;
break;
case PIN_CONFIG_BIAS_PULL_DOWN:
- val = readl(pctl->membase + sunxi_pull_reg(pin));
- mask = PULL_PINS_MASK << sunxi_pull_offset(pin);
- writel((val & ~mask) | 2 << sunxi_pull_offset(pin),
- pctl->membase + sunxi_pull_reg(pin));
+ if (arg == 0)
+ return -EINVAL;
+ val = 2;
break;
default:
- break;
+ /* sunxi_pconf_reg should catch anything unsupported */
+ WARN_ON(1);
+ return -ENOTSUPP;
}
- /* cache the config value */
- g->config = configs[i];
- } /* for each config */
- spin_unlock_irqrestore(&pctl->lock, flags);
+ spin_lock_irqsave(&pctl->lock, flags);
+ reg = readl(pctl->membase + offset);
+ reg &= ~(mask << shift);
+ writel(reg | val << shift, pctl->membase + offset);
+ spin_unlock_irqrestore(&pctl->lock, flags);
+ } /* for each config */
return 0;
}
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
index 0afce1ab12d0..a7efb31d6523 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
@@ -109,7 +109,6 @@ struct sunxi_pinctrl_function {
struct sunxi_pinctrl_group {
const char *name;
- unsigned long config;
unsigned pin;
};
--
2.10.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper
2016-11-11 2:44 ` [PATCH v2 3/3] pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper Chen-Yu Tsai
@ 2016-11-11 8:38 ` Maxime Ripard
2016-11-11 8:45 ` Chen-Yu Tsai
0 siblings, 1 reply; 10+ messages in thread
From: Maxime Ripard @ 2016-11-11 8:38 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Linus Walleij, linux-gpio, linux-arm-kernel, linux-kernel, linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 953 bytes --]
On Fri, Nov 11, 2016 at 10:44:55AM +0800, Chen-Yu Tsai wrote:
> The sunxi_pconf_reg helper introduced in the last patch gives us the
> chance to rework sunxi_pconf_group_set to have it match the structure
> of sunxi_pconf_(group_)get and make it easier to understand.
>
> For each config to set, it:
>
> 1. checks if the parameter is supported.
> 2. checks if the argument is within limits.
> 3. converts argument to the register value.
> 4. writes to the register with spinlock held.
>
> As a result the function now blocks unsupported config parameters,
> instead of silently ignoring them.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
But I think the config variable removal should be part of patch 2, as
discussed there.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper
2016-11-11 8:38 ` Maxime Ripard
@ 2016-11-11 8:45 ` Chen-Yu Tsai
0 siblings, 0 replies; 10+ messages in thread
From: Chen-Yu Tsai @ 2016-11-11 8:45 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, Linus Walleij, linux-gpio, linux-arm-kernel,
linux-kernel, linux-sunxi
On Fri, Nov 11, 2016 at 4:38 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Fri, Nov 11, 2016 at 10:44:55AM +0800, Chen-Yu Tsai wrote:
>> The sunxi_pconf_reg helper introduced in the last patch gives us the
>> chance to rework sunxi_pconf_group_set to have it match the structure
>> of sunxi_pconf_(group_)get and make it easier to understand.
>>
>> For each config to set, it:
>>
>> 1. checks if the parameter is supported.
>> 2. checks if the argument is within limits.
>> 3. converts argument to the register value.
>> 4. writes to the register with spinlock held.
>>
>> As a result the function now blocks unsupported config parameters,
>> instead of silently ignoring them.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> But I think the config variable removal should be part of patch 2, as
> discussed there.
OK. I think that makes sense. Re-reading my patches, I can't figure out,
which patch I meant for it to go in. :(
I'll send out a v3.
ChenYu
^ permalink raw reply [flat|nested] 10+ messages in thread