* [PATCH 9/9] ARM: sunxi: Convert pinctrl nodes to generic bindings
From: Chen-Yu Tsai @ 2016-10-04 2:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1f67fc945364c4347108d5c82e50de3d2abe4e8d.1475489558.git-series.maxime.ripard@free-electrons.com>
On Mon, Oct 3, 2016 at 6:21 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Now that we can handle the generic pinctrl bindings, convert our DT to it.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Same comment as the last 2 patches.
ChenYu
^ permalink raw reply
* [PATCH 8/9] ARM: sunxi: Remove useless allwinner,pull property
From: Chen-Yu Tsai @ 2016-10-04 2:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7b4c2580fc98e0877c14736b7edf3738128591ae.1475489558.git-series.maxime.ripard@free-electrons.com>
On Mon, Oct 3, 2016 at 6:21 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The allwinner,pull property set to NO_PULL was really considered our
> default (and wasn't even changing the default value in the code).
>
> Remove these properties to make it obvious that we do not set anything in
> such a case.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Same comment as the last patch.
ChenYu
^ permalink raw reply
* [PATCH 7/9] ARM: sunxi: Remove useless allwinner,drive property
From: Chen-Yu Tsai @ 2016-10-04 2:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <01cd77ccced851276fcb3625b3d7805f5260fb59.1475489558.git-series.maxime.ripard@free-electrons.com>
On Mon, Oct 3, 2016 at 6:21 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The allwinner,drive property set to 10mA was really considered as our
> default. Remove all those properties entirely to make that obvious.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Did you use sed or some other scripting tool to do this patch?
Including the command should make it easier to verify the result,
instead of having to go through the 93 files here.
ChenYu
^ permalink raw reply
* [PATCH 6/9] dt-bindings: pinctrl: Deprecate sunxi pinctrl bindings
From: Chen-Yu Tsai @ 2016-10-04 2:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <fad5a977d8ecdd43b37d7028b1bc32d8667b65a2.1475489558.git-series.maxime.ripard@free-electrons.com>
On Mon, Oct 3, 2016 at 6:21 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The generic pin configuration and multiplexing should be preferred now,
> even though we still support the old one.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt | 5 +++++
> 1 file changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt
> index 69617220c5d6..e8b7cf64fdf1 100644
> --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt
> @@ -36,6 +36,11 @@ pins it needs, and how they should be configured, with regard to muxer
> configuration, drive strength and pullups. If one of these options is
> not set, its actual value will be unspecified.
>
> +This driver supports the generic pin multiplexing and configuration
> +bindings.
Should we list which options are supported? We don't support them all.
ChenYu
> +
> +*** Deprecated pin configuration and multiplexing binding
> +
> Required subnode-properties:
>
> - allwinner,pins: List of strings containing the pin name.
> --
> git-series 0.8.10
^ permalink raw reply
* [PATCH 5/9] pinctrl: sunxi: Support generic binding
From: Chen-Yu Tsai @ 2016-10-04 2:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1b45160fb37e2dcabd855d2192fc403a1c4db324.1475489558.git-series.maxime.ripard@free-electrons.com>
On Mon, Oct 3, 2016 at 6:21 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Our bindings are mostly irrelevant now that we have generic pinctrl
> bindings that cover exactly the same uses cases.
>
> Add support for the new ones, and obviously keep our old binding support in
> order to keep the ABI stable.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Chen-Yu Tsai <wens@csie.org>
^ permalink raw reply
* [PATCH 4/9] pinctrl: sunxi: Deal with configless pins
From: Chen-Yu Tsai @ 2016-10-04 2:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8b72a134d957afdee6de48ec2c1891d8bba1b9e3.1475489558.git-series.maxime.ripard@free-electrons.com>
On Mon, Oct 3, 2016 at 6:21 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Even though the our binding had the assumption that the allwinner,pull and
> allwinner,drive properties were optional, the code never took that into
> account.
>
> Fix that.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 50 +++++++++++++++++++---------
> 1 file changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 6f6f1e0011e2..cec977fcf98c 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -218,20 +218,29 @@ static unsigned long *sunxi_pctrl_build_pin_config(struct device_node *node,
> {
> unsigned long *pinconfig;
> unsigned int configlen = 0, idx = 0;
> + int ret;
>
> if (sunxi_pctrl_has_drive_prop(node))
> configlen++;
> if (sunxi_pctrl_has_bias_prop(node))
> configlen++;
>
> + /*
> + * If we don't have any configuration, bail out
> + */
> + if (!configlen)
> + return NULL;
> +
> pinconfig = kzalloc(configlen * sizeof(*pinconfig), GFP_KERNEL);
> if (!pinconfig)
> - return NULL;
> + return ERR_PTR(-ENOMEM);
>
> if (sunxi_pctrl_has_drive_prop(node)) {
> int drive = sunxi_pctrl_parse_drive_prop(node);
> - if (drive < 0)
> + if (drive < 0) {
> + ret = -EINVAL;
Why not just pass the error code returned from the parse function?
> goto err_free;
> + }
>
> pinconfig[idx++] = pinconf_to_config_packed(PIN_CONFIG_DRIVE_STRENGTH,
> drive);
> @@ -239,8 +248,10 @@ 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);
> - if (pull < 0)
> + if (pull < 0) {
> + ret = -EINVAL;
Same here.
> goto err_free;
> + }
>
> pinconfig[idx++] = pinconf_to_config_packed(pull, 0);
> }
> @@ -251,7 +262,7 @@ static unsigned long *sunxi_pctrl_build_pin_config(struct device_node *node,
>
> err_free:
> kfree(pinconfig);
> - return NULL;
> + return ERR_PTR(ret);
> }
>
> static int sunxi_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> @@ -285,7 +296,10 @@ static int sunxi_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
>
> /*
> * We have two maps for each pin: one for the function, one
> - * for the configuration (bias, strength, etc)
> + * for the configuration (bias, strength, etc).
> + *
> + * We might be slightly overshooting, since we might not have
> + * any configuration.
> */
> nmaps = npins * 2;
> *map = kmalloc(nmaps * sizeof(struct pinctrl_map), GFP_KERNEL);
> @@ -293,8 +307,8 @@ static int sunxi_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> return -ENOMEM;
>
> pinconfig = sunxi_pctrl_build_pin_config(node, &configlen);
> - if (!pinconfig) {
> - ret = -EINVAL;
> + if (IS_ERR(pinconfig)) {
> + ret = PTR_ERR(pinconfig);
> goto err_free_map;
> }
>
> @@ -321,15 +335,16 @@ static int sunxi_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
>
> i++;
>
> - (*map)[i].type = PIN_MAP_TYPE_CONFIGS_GROUP;
> - (*map)[i].data.configs.group_or_pin = group;
> - (*map)[i].data.configs.configs = pinconfig;
> - (*map)[i].data.configs.num_configs = configlen;
> -
> - i++;
> + if (pinconfig) {
> + (*map)[i].type = PIN_MAP_TYPE_CONFIGS_GROUP;
> + (*map)[i].data.configs.group_or_pin = group;
> + (*map)[i].data.configs.configs = pinconfig;
> + (*map)[i].data.configs.num_configs = configlen;
> + i++;
> + }
> }
>
> - *num_maps = nmaps;
> + *num_maps = i;
Thought: should we do a krealloc to shrink the array?
>
> return 0;
>
> @@ -342,8 +357,13 @@ static void sunxi_pctrl_dt_free_map(struct pinctrl_dev *pctldev,
> struct pinctrl_map *map,
> unsigned num_maps)
> {
> + unsigned long *pinconfig;
> +
> /* All the maps have the same pin config, free only the first one */
> - kfree(map[0].data.configs.configs);
> + pinconfig = map[0].data.configs.configs;
> + if (pinconfig)
> + kfree(pinconfig);
Passing NULL to kfree is allowed. (It becomes a no-op.)
So you could leave this function alone.
ChenYu
> +
> kfree(map);
> }
>
> --
> git-series 0.8.10
^ permalink raw reply
* [PATCH 3/9] pinctrl: sunxi: Handle bias disable
From: Chen-Yu Tsai @ 2016-10-04 2:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f4de32e1b124b25c27e34efcadc1d332cb93ee59.1475489558.git-series.maxime.ripard@free-electrons.com>
On Mon, Oct 3, 2016 at 6:21 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> So far, putting NO_PULL in allwinner,pull was ignored, behaving like if
> that property was not there at all.
>
> Obviously, this is not the right thing to do, and in that case, we really
> need to just disable the bias.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
I've done the same in my patches.
Acked-by: Chen-Yu Tsai <wens@csie.org>
> ---
> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 8 ++++++++
> 1 file changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 5be455d5e252..6f6f1e0011e2 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -166,6 +166,8 @@ static int sunxi_pctrl_parse_bias_prop(struct device_node *node)
> return -EINVAL;
>
> switch (val) {
> + case SUN4I_PINCTRL_NO_PULL:
> + return PIN_CONFIG_BIAS_DISABLE;
> case SUN4I_PINCTRL_PULL_UP:
> return PIN_CONFIG_BIAS_PULL_UP;
> case SUN4I_PINCTRL_PULL_DOWN:
> @@ -402,6 +404,12 @@ static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
> | dlevel << sunxi_dlevel_offset(pin),
> pctl->membase + sunxi_dlevel_reg(pin));
> 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));
> + break;
> case PIN_CONFIG_BIAS_PULL_UP:
> val = readl(pctl->membase + sunxi_pull_reg(pin));
> mask = PULL_PINS_MASK << sunxi_pull_offset(pin);
> --
> git-series 0.8.10
^ permalink raw reply
* [PATCH] ARM64: dts: meson-gxbb-odroidc2: Enable USB Nodes
From: Brian Kim @ 2016-10-04 2:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7h7f9tgzg8.fsf@baylibre.com>
On 2016? 10? 01? 00:49, Kevin Hilman wrote:
> Brian Kim <brian.kim@hardkernel.com> writes:
>
>> Enable both gxbb USB controller and add a 5V regulator for the OTG port
>> VBUS
>>
>> Signed-off-by: Brian Kim <brian.kim@hardkernel.com>
> Thanks for the patch.
>
> In the future, please state what branch the patch should apply to when
> not using mainline. Because of the sd_emmc nodes in your patch, I could
> tell that it was based on my integ branch so was able to figure it out,
> but it's very helpful to maintainers if you state the branch and/or any
> dependencies explicity.
Okay, I will do next time.
I guessed the branch is your latest working branch by the commit logs.
>
>> ---
>> .../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 29 ++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
>> index 8d89edc..997c671 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
>> @@ -64,6 +64,18 @@
>> reg = <0x0 0x0 0x0 0x80000000>;
>> };
>>
>> + usb_pwr: regulator-usb-pwrs {
> minor nit: since this is specific to the OTG part, can you call this
> usb_otg_pwr? ...
Sure.
>
>> + compatible = "regulator-fixed";
>> +
>> + regulator-name = "USB_PWR";
> ... and rename this also?
Yes, I will change the name to "USB_OTG_PWR".
>
>> + regulator-min-microvolt = <5000000>;
>> + regulator-max-microvolt = <5000000>;
>> +
>> + gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
>> + enable-active-high;
>> + };
>> +
> Thanks
>
> Kevin
>
^ permalink raw reply
* [PATCH 2/9] pinctrl: sunxi: Add bindings define
From: Chen-Yu Tsai @ 2016-10-04 1:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f56e271107221a2f41f256e2759a92cfe29624a8.1475489558.git-series.maxime.ripard@free-electrons.com>
On Mon, Oct 3, 2016 at 6:21 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Since we have some bindings header for our hardcoded flags, let's use them
> when we can.
"Use macros from bindings header file for DT parsing"
would make the subject clearer.
Otherwise,
Acked-by: Chen-Yu Tsai <wens@csie.org>
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 64f7f6dcc027..5be455d5e252 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -28,6 +28,8 @@
> #include <linux/platform_device.h>
> #include <linux/slab.h>
>
> +#include <dt-bindings/pinctrl/sun4i-a10.h>
> +
> #include "../core.h"
> #include "../../gpio/gpiolib.h"
> #include "pinctrl-sunxi.h"
> @@ -164,9 +166,9 @@ static int sunxi_pctrl_parse_bias_prop(struct device_node *node)
> return -EINVAL;
>
> switch (val) {
> - case 1:
> + case SUN4I_PINCTRL_PULL_UP:
> return PIN_CONFIG_BIAS_PULL_UP;
> - case 2:
> + case SUN4I_PINCTRL_PULL_DOWN:
> return PIN_CONFIG_BIAS_PULL_DOWN;
> }
>
> --
> git-series 0.8.10
^ permalink raw reply
* [PATCH 3/3] pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper
From: Chen-Yu Tsai @ 2016-10-04 1:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004015112.20833-1-wens@csie.org>
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 | 65 +++++++++++++++++++----------------
drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 -
2 files changed, 35 insertions(+), 31 deletions(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 236272a2339d..1f02c4cd55c7 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -364,23 +364,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, val;
+ u16 arg;
+ 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
@@ -388,33 +392,34 @@ 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));
+ arg = arg / 10 - 1;
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;
+ arg = 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;
+ arg = 2;
break;
- default:
+ case PIN_CONFIG_BIAS_DISABLE:
+ arg = 0;
break;
+
+ default:
+ /* 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);
+ val = readl(pctl->membase + offset);
+ val &= ~(mask << shift);
+ writel(val | arg << 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.9.3
^ permalink raw reply related
* [PATCH 2/3] pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN, UP} argument
From: Chen-Yu Tsai @ 2016-10-04 1:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004015112.20833-1-wens@csie.org>
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 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 609843c9a65c..236272a2339d 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -234,7 +234,7 @@ static int sunxi_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
pull = PIN_CONFIG_BIAS_PULL_UP;
else if (val == 2)
pull = PIN_CONFIG_BIAS_PULL_DOWN;
- pinconfig[j++] = pinconf_to_config_packed(pull, 0);
+ pinconfig[j++] = pinconf_to_config_packed(pull, 1);
}
(*map)[i].data.configs.configs = pinconfig;
--
2.9.3
^ permalink raw reply related
* [PATCH 1/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware
From: Chen-Yu Tsai @ 2016-10-04 1:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004015112.20833-1-wens@csie.org>
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 | 85 +++++++++++++++++++++++++++++++++--
1 file changed, 82 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 54455af566ec..609843c9a65c 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -10,6 +10,7 @@
* warranty of any kind, whether express or implied.
*/
+#include <dt-bindings/pinctrl/sun4i-a10.h>
#include <linux/io.h>
#include <linux/clk.h>
#include <linux/gpio/driver.h>
@@ -269,15 +270,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,
@@ -343,6 +420,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.9.3
^ permalink raw reply related
* [PATCH 0/3] pinctrl: sunxi: Support generic pinconf functions
From: Chen-Yu Tsai @ 2016-10-04 1:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi everyone,
This series fixes up generic pinconf support for the sunxi pinctrl driver
library. The driver was doing some bits wrong, like a) storing the pinconf
config value in its struct, and not actually reading the hardware to get
the current config, and b) not using the right arguments for the bias
parameters.
Patch 1 makes the driver read out pinconf settings from the hardware, and
returns the correct value for unsupported features and disable features.
With this in place it also declares itself as generic pinconf compatible,
which enables us to read the config through the debugfs pinconf interface.
Patch 2 fixes the pin bias parameter arguments.
Patch 3 makes the sunxi_pconf_group_set callback use the helper function
introduced in patch 1.
The patches will likely conflict with Maxime's generic pinctrl bindings
series. Lets figure something out.
Regards
ChenYu
Chen-Yu Tsai (3):
pinctrl: sunxi: Add support for fetching pinconf settings from
hardware
pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN,UP} argument
pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 152 ++++++++++++++++++++++++++--------
drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 -
2 files changed, 118 insertions(+), 35 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH 7/8] pinctrl: aspeed-g4: Add mux configuration for all pins
From: Andrew Jeffery @ 2016-10-04 1:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161003190833.GA3065@rob-hp-laptop>
On Mon, 2016-10-03 at 14:08 -0500, Rob Herring wrote:
> On Wed, Sep 28, 2016 at 12:20:19AM +0930, Andrew Jeffery wrote:
> >
> > The patch introducing the g4 pinctrl driver implemented a smattering of
> > pins to flesh out the implementation of the core and provide bare-bones
> > support for some OpenPOWER platforms. Now, update the bindings document
> > to reflect the complete functionality and implement the necessary pin
> > configuration tables in the driver.
> We prefer bindings to be complete if possible where as drivers can be?
> expanded over time.
Noted.
>
> >
> >
> > Cc: Timothy Pearson <tpearson@raptorengineering.com>
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > ?Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt |???19 +-
> Acked-by: Rob Herring <robh@kernel.org>
Thanks,
Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161004/746c8627/attachment.sig>
^ permalink raw reply
* [PATCH] efi/arm: fix absolute relocation detection for older toolchains
From: Matt Fleming @ 2016-10-03 20:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475276515-21801-1-git-send-email-ard.biesheuvel@linaro.org>
On Fri, 30 Sep, at 04:01:55PM, Ard Biesheuvel wrote:
>
> This is a workaround for now. We can revisit this when a need arises to copy
> more kernel code into the stub, by which time we could put in a more elaborate
> fix, or decide to no longer care about 'older' versions of objcopy.
>
> Since this fixes an ARM specific issue and only affects ARM specific Makefile
> variables, I am happy for this to go on top of the arm-soc patch that enables
> CONFIG_EFI for ARM's multi_v7_defconfig (queued for v4.9), given that we have
> no other changes queued in linux-efi that should conflict with this patch.
>
> Matt, any concerns?
Not with the patch, but could we clarify the user-visible effects of
not applying it? Are the absolute relocations harmless, or will they
lead to crashes?
^ permalink raw reply
* [PATCH v4 7/7] i2c: bcm2835: Add support for dynamic clock
From: Noralf Trønnes @ 2016-10-03 20:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475525174-28604-1-git-send-email-noralf@tronnes.org>
Support a dynamic clock by reading the frequency and setting the
divisor in the transfer function instead of during probe.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
Reviewed-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/i2c/busses/i2c-bcm2835.c | 51 +++++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index d2085dd..c3436f6 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -58,6 +58,7 @@ struct bcm2835_i2c_dev {
void __iomem *regs;
struct clk *clk;
int irq;
+ u32 bus_clk_rate;
struct i2c_adapter adapter;
struct completion completion;
struct i2c_msg *curr_msg;
@@ -78,6 +79,30 @@ static inline u32 bcm2835_i2c_readl(struct bcm2835_i2c_dev *i2c_dev, u32 reg)
return readl(i2c_dev->regs + reg);
}
+static int bcm2835_i2c_set_divider(struct bcm2835_i2c_dev *i2c_dev)
+{
+ u32 divider;
+
+ divider = DIV_ROUND_UP(clk_get_rate(i2c_dev->clk),
+ i2c_dev->bus_clk_rate);
+ /*
+ * Per the datasheet, the register is always interpreted as an even
+ * number, by rounding down. In other words, the LSB is ignored. So,
+ * if the LSB is set, increment the divider to avoid any issue.
+ */
+ if (divider & 1)
+ divider++;
+ if ((divider < BCM2835_I2C_CDIV_MIN) ||
+ (divider > BCM2835_I2C_CDIV_MAX)) {
+ dev_err_ratelimited(i2c_dev->dev, "Invalid clock-frequency\n");
+ return -EINVAL;
+ }
+
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DIV, divider);
+
+ return 0;
+}
+
static void bcm2835_fill_txfifo(struct bcm2835_i2c_dev *i2c_dev)
{
u32 val;
@@ -224,7 +249,7 @@ static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
{
struct bcm2835_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
unsigned long time_left;
- int i;
+ int i, ret;
for (i = 0; i < (num - 1); i++)
if (msgs[i].flags & I2C_M_RD) {
@@ -233,6 +258,10 @@ static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
return -EOPNOTSUPP;
}
+ ret = bcm2835_i2c_set_divider(i2c_dev);
+ if (ret)
+ return ret;
+
i2c_dev->curr_msg = msgs;
i2c_dev->num_msgs = num;
reinit_completion(&i2c_dev->completion);
@@ -282,7 +311,6 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
{
struct bcm2835_i2c_dev *i2c_dev;
struct resource *mem, *irq;
- u32 bus_clk_rate, divider;
int ret;
struct i2c_adapter *adap;
@@ -306,27 +334,12 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
}
ret = of_property_read_u32(pdev->dev.of_node, "clock-frequency",
- &bus_clk_rate);
+ &i2c_dev->bus_clk_rate);
if (ret < 0) {
dev_warn(&pdev->dev,
"Could not read clock-frequency property\n");
- bus_clk_rate = 100000;
- }
-
- divider = DIV_ROUND_UP(clk_get_rate(i2c_dev->clk), bus_clk_rate);
- /*
- * Per the datasheet, the register is always interpreted as an even
- * number, by rounding down. In other words, the LSB is ignored. So,
- * if the LSB is set, increment the divider to avoid any issue.
- */
- if (divider & 1)
- divider++;
- if ((divider < BCM2835_I2C_CDIV_MIN) ||
- (divider > BCM2835_I2C_CDIV_MAX)) {
- dev_err(&pdev->dev, "Invalid clock-frequency\n");
- return -ENODEV;
+ i2c_dev->bus_clk_rate = 100000;
}
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DIV, divider);
irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!irq) {
--
2.8.2
^ permalink raw reply related
* [PATCH v4 6/7] i2c: bcm2835: Support i2c-dev ioctl I2C_TIMEOUT
From: Noralf Trønnes @ 2016-10-03 20:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475525174-28604-1-git-send-email-noralf@tronnes.org>
Use i2c_adapter->timeout for the completion timeout value. The core
default is 1 second.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
---
drivers/i2c/busses/i2c-bcm2835.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index 241e08a..d2085dd 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -53,8 +53,6 @@
#define BCM2835_I2C_CDIV_MIN 0x0002
#define BCM2835_I2C_CDIV_MAX 0xFFFE
-#define BCM2835_I2C_TIMEOUT (msecs_to_jiffies(1000))
-
struct bcm2835_i2c_dev {
struct device *dev;
void __iomem *regs;
@@ -242,7 +240,7 @@ static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
bcm2835_i2c_start_transfer(i2c_dev);
time_left = wait_for_completion_timeout(&i2c_dev->completion,
- BCM2835_I2C_TIMEOUT);
+ adap->timeout);
if (!time_left) {
bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C,
BCM2835_I2C_C_CLEAR);
--
2.8.2
^ permalink raw reply related
* [PATCH v4 5/7] i2c: bcm2835: Add support for Repeated Start Condition
From: Noralf Trønnes @ 2016-10-03 20:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475525174-28604-1-git-send-email-noralf@tronnes.org>
Documentation/i2c/i2c-protocol states that Combined transactions should
separate messages with a Start bit and end the whole transaction with a
Stop bit. This patch adds support for issuing only a Start between
messages instead of a Stop followed by a Start.
This implementation differs from downstream i2c-bcm2708 in 2 respects:
- it uses an interrupt to detect that the transfer is active instead
of using polling. There is no interrupt for Transfer Active, but by
not prefilling the FIFO it's possible to use the TXW interrupt.
- when resetting/disabling the controller between transfers it writes
CLEAR to the control register instead of just zero.
Using just zero gave many errors. This might be the reason why
downstream had to disable this feature and make it available with a
module parameter.
I have run thousands of transfers to a DS1307 (rtc), MMA8451 (accel)
and AT24C32 (eeprom) in parallel without problems.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
Acked-by: Eric Anholt <eric@anholt.net>
---
drivers/i2c/busses/i2c-bcm2835.c | 101 ++++++++++++++++++++++++---------------
1 file changed, 63 insertions(+), 38 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index 565ef69..241e08a 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -63,6 +63,7 @@ struct bcm2835_i2c_dev {
struct i2c_adapter adapter;
struct completion completion;
struct i2c_msg *curr_msg;
+ int num_msgs;
u32 msg_err;
u8 *msg_buf;
size_t msg_buf_remaining;
@@ -110,6 +111,45 @@ static void bcm2835_drain_rxfifo(struct bcm2835_i2c_dev *i2c_dev)
}
/*
+ * Repeated Start Condition (Sr)
+ * The BCM2835 ARM Peripherals datasheet mentions a way to trigger a Sr when it
+ * talks about reading from a slave with 10 bit address. This is achieved by
+ * issuing a write, poll the I2CS.TA flag and wait for it to be set, and then
+ * issue a read.
+ * A comment in https://github.com/raspberrypi/linux/issues/254 shows how the
+ * firmware actually does it using polling and says that it's a workaround for
+ * a problem in the state machine.
+ * It turns out that it is possible to use the TXW interrupt to know when the
+ * transfer is active, provided the FIFO has not been prefilled.
+ */
+
+static void bcm2835_i2c_start_transfer(struct bcm2835_i2c_dev *i2c_dev)
+{
+ u32 c = BCM2835_I2C_C_ST | BCM2835_I2C_C_I2CEN;
+ struct i2c_msg *msg = i2c_dev->curr_msg;
+ bool last_msg = (i2c_dev->num_msgs == 1);
+
+ if (!i2c_dev->num_msgs)
+ return;
+
+ i2c_dev->num_msgs--;
+ i2c_dev->msg_buf = msg->buf;
+ i2c_dev->msg_buf_remaining = msg->len;
+
+ if (msg->flags & I2C_M_RD)
+ c |= BCM2835_I2C_C_READ | BCM2835_I2C_C_INTR;
+ else
+ c |= BCM2835_I2C_C_INTT;
+
+ if (last_msg)
+ c |= BCM2835_I2C_C_INTD;
+
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_A, msg->addr);
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DLEN, msg->len);
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, c);
+}
+
+/*
* Note about I2C_C_CLEAR on error:
* The I2C_C_CLEAR on errors will take some time to resolve -- if you were in
* non-idle state and I2C_C_READ, it sets an abort_rx flag and runs through
@@ -151,6 +191,12 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
}
bcm2835_fill_txfifo(i2c_dev);
+
+ if (i2c_dev->num_msgs && !i2c_dev->msg_buf_remaining) {
+ i2c_dev->curr_msg++;
+ bcm2835_i2c_start_transfer(i2c_dev);
+ }
+
return IRQ_HANDLED;
}
@@ -175,30 +221,25 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
return IRQ_HANDLED;
}
-static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
- struct i2c_msg *msg)
+static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
+ int num)
{
- u32 c;
+ struct bcm2835_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
unsigned long time_left;
+ int i;
- i2c_dev->curr_msg = msg;
- i2c_dev->msg_buf = msg->buf;
- i2c_dev->msg_buf_remaining = msg->len;
- reinit_completion(&i2c_dev->completion);
-
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, BCM2835_I2C_C_CLEAR);
+ for (i = 0; i < (num - 1); i++)
+ if (msgs[i].flags & I2C_M_RD) {
+ dev_warn_once(i2c_dev->dev,
+ "only one read message supported, has to be last\n");
+ return -EOPNOTSUPP;
+ }
- if (msg->flags & I2C_M_RD) {
- c = BCM2835_I2C_C_READ | BCM2835_I2C_C_INTR;
- } else {
- c = BCM2835_I2C_C_INTT;
- bcm2835_fill_txfifo(i2c_dev);
- }
- c |= BCM2835_I2C_C_ST | BCM2835_I2C_C_INTD | BCM2835_I2C_C_I2CEN;
+ i2c_dev->curr_msg = msgs;
+ i2c_dev->num_msgs = num;
+ reinit_completion(&i2c_dev->completion);
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_A, msg->addr);
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DLEN, msg->len);
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, c);
+ bcm2835_i2c_start_transfer(i2c_dev);
time_left = wait_for_completion_timeout(&i2c_dev->completion,
BCM2835_I2C_TIMEOUT);
@@ -209,31 +250,15 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
return -ETIMEDOUT;
}
- if (likely(!i2c_dev->msg_err))
- return 0;
+ if (!i2c_dev->msg_err)
+ return num;
dev_dbg(i2c_dev->dev, "i2c transfer failed: %x\n", i2c_dev->msg_err);
if (i2c_dev->msg_err & BCM2835_I2C_S_ERR)
return -EREMOTEIO;
- else
- return -EIO;
-}
-
-static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
- int num)
-{
- struct bcm2835_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
- int i;
- int ret = 0;
-
- for (i = 0; i < num; i++) {
- ret = bcm2835_i2c_xfer_msg(i2c_dev, &msgs[i]);
- if (ret)
- break;
- }
- return ret ?: i;
+ return -EIO;
}
static u32 bcm2835_i2c_func(struct i2c_adapter *adap)
--
2.8.2
^ permalink raw reply related
* [PATCH v4 4/7] i2c: bcm2835: Can't support I2C_M_IGNORE_NAK
From: Noralf Trønnes @ 2016-10-03 20:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475525174-28604-1-git-send-email-noralf@tronnes.org>
The controller can't support this flag, so remove it.
Documentation/i2c/i2c-protocol states that all of the message is sent:
I2C_M_IGNORE_NAK:
Normally message is interrupted immediately if there is [NA] from the
client. Setting this flag treats any [NA] as [A], and all of
message is sent.
>From the BCM2835 ARM Peripherals datasheet:
The ERR field is set when the slave fails to acknowledge either
its address or a data byte written to it.
So when the controller doesn't receive an ack, it sets ERR and raises
an interrupt. In other words, the whole message is not sent.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
---
drivers/i2c/busses/i2c-bcm2835.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index 54d510a..565ef69 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -212,10 +212,6 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
if (likely(!i2c_dev->msg_err))
return 0;
- if ((i2c_dev->msg_err & BCM2835_I2C_S_ERR) &&
- (msg->flags & I2C_M_IGNORE_NAK))
- return 0;
-
dev_dbg(i2c_dev->dev, "i2c transfer failed: %x\n", i2c_dev->msg_err);
if (i2c_dev->msg_err & BCM2835_I2C_S_ERR)
--
2.8.2
^ permalink raw reply related
* [PATCH v4 3/7] i2c: bcm2835: Use dev_dbg logging on transfer errors
From: Noralf Trønnes @ 2016-10-03 20:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475525174-28604-1-git-send-email-noralf@tronnes.org>
Writing to an AT24C32 generates on average 2x i2c transfer errors per
32-byte page write. Which amounts to a lot for a 4k write. This is due
to the fact that the chip doesn't respond during it's internal write
cycle when the at24 driver tries and retries the next write.
Only a handful drivers use dev_err() on transfer error, so switch to
dev_dbg() instead.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
---
drivers/i2c/busses/i2c-bcm2835.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index d2ba1a4..54d510a 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -216,7 +216,7 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
(msg->flags & I2C_M_IGNORE_NAK))
return 0;
- dev_err(i2c_dev->dev, "i2c transfer failed: %x\n", i2c_dev->msg_err);
+ dev_dbg(i2c_dev->dev, "i2c transfer failed: %x\n", i2c_dev->msg_err);
if (i2c_dev->msg_err & BCM2835_I2C_S_ERR)
return -EREMOTEIO;
--
2.8.2
^ permalink raw reply related
* [PATCH v4 2/7] i2c: bcm2835: Protect against unexpected TXW/RXR interrupts
From: Noralf Trønnes @ 2016-10-03 20:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475525174-28604-1-git-send-email-noralf@tronnes.org>
If an unexpected TXW or RXR interrupt occurs (msg_buf_remaining == 0),
the driver has no way to fill/drain the FIFO to stop the interrupts.
In this case the controller has to be disabled and the transfer
completed to avoid hang.
(CLKT | ERR) and DONE interrupts are completed in their own paths, and
the controller is disabled in the transfer function after completion.
Unite the code paths and do disabling inside the interrupt routine.
Clear interrupt status bits in the united completion path instead of
trying to do it on every interrupt which isn't necessary.
Only CLKT, ERR and DONE can be cleared that way.
Add the status value to the error value in case of TXW/RXR errors to
distinguish them from the other S_LEN error.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
---
Changes since v3:
- Add comment about I2C_C_CLEAR on error
drivers/i2c/busses/i2c-bcm2835.c | 40 +++++++++++++++++++++++++++++++---------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index f283b71..d2ba1a4 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -50,8 +50,6 @@
#define BCM2835_I2C_S_CLKT BIT(9)
#define BCM2835_I2C_S_LEN BIT(10) /* Fake bit for SW error reporting */
-#define BCM2835_I2C_BITMSK_S 0x03FF
-
#define BCM2835_I2C_CDIV_MIN 0x0002
#define BCM2835_I2C_CDIV_MAX 0xFFFE
@@ -111,20 +109,26 @@ static void bcm2835_drain_rxfifo(struct bcm2835_i2c_dev *i2c_dev)
}
}
+/*
+ * Note about I2C_C_CLEAR on error:
+ * The I2C_C_CLEAR on errors will take some time to resolve -- if you were in
+ * non-idle state and I2C_C_READ, it sets an abort_rx flag and runs through
+ * the state machine to send a NACK and a STOP. Since we're setting CLEAR
+ * without I2CEN, that NACK will be hanging around queued up for next time
+ * we start the engine.
+ */
+
static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
{
struct bcm2835_i2c_dev *i2c_dev = data;
u32 val, err;
val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S);
- val &= BCM2835_I2C_BITMSK_S;
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_S, val);
err = val & (BCM2835_I2C_S_CLKT | BCM2835_I2C_S_ERR);
if (err) {
i2c_dev->msg_err = err;
- complete(&i2c_dev->completion);
- return IRQ_HANDLED;
+ goto complete;
}
if (val & BCM2835_I2C_S_DONE) {
@@ -137,21 +141,38 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
i2c_dev->msg_err = BCM2835_I2C_S_LEN;
else
i2c_dev->msg_err = 0;
- complete(&i2c_dev->completion);
- return IRQ_HANDLED;
+ goto complete;
}
if (val & BCM2835_I2C_S_TXW) {
+ if (!i2c_dev->msg_buf_remaining) {
+ i2c_dev->msg_err = val | BCM2835_I2C_S_LEN;
+ goto complete;
+ }
+
bcm2835_fill_txfifo(i2c_dev);
return IRQ_HANDLED;
}
if (val & BCM2835_I2C_S_RXR) {
+ if (!i2c_dev->msg_buf_remaining) {
+ i2c_dev->msg_err = val | BCM2835_I2C_S_LEN;
+ goto complete;
+ }
+
bcm2835_drain_rxfifo(i2c_dev);
return IRQ_HANDLED;
}
return IRQ_NONE;
+
+complete:
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, BCM2835_I2C_C_CLEAR);
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_S, BCM2835_I2C_S_CLKT |
+ BCM2835_I2C_S_ERR | BCM2835_I2C_S_DONE);
+ complete(&i2c_dev->completion);
+
+ return IRQ_HANDLED;
}
static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
@@ -181,8 +202,9 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
time_left = wait_for_completion_timeout(&i2c_dev->completion,
BCM2835_I2C_TIMEOUT);
- bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, BCM2835_I2C_C_CLEAR);
if (!time_left) {
+ bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C,
+ BCM2835_I2C_C_CLEAR);
dev_err(i2c_dev->dev, "i2c transfer timed out\n");
return -ETIMEDOUT;
}
--
2.8.2
^ permalink raw reply related
* [PATCH v4 1/7] i2c: bcm2835: Fix hang for writing messages larger than 16 bytes
From: Noralf Trønnes @ 2016-10-03 20:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475525174-28604-1-git-send-email-noralf@tronnes.org>
Writing messages larger than the FIFO size results in a hang, rendering
the machine unusable. This is because the RXD status flag is set on the
first interrupt which results in bcm2835_drain_rxfifo() stealing bytes
from the buffer. The controller continues to trigger interrupts waiting
for the missing bytes, but bcm2835_fill_txfifo() has none to give.
In this situation wait_for_completion_timeout() apparently is unable to
stop the madness.
The BCM2835 ARM Peripherals datasheet has this to say about the flags:
TXD: is set when the FIFO has space for at least one byte of data.
RXD: is set when the FIFO contains at least one byte of data.
TXW: is set during a write transfer and the FIFO is less than full.
RXR: is set during a read transfer and the FIFO is or more full.
Implementing the logic from the downstream i2c-bcm2708 driver solved
the hang problem.
Signed-off-by: Noralf Tr?nnes <noralf@tronnes.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/i2c/busses/i2c-bcm2835.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index d4f3239..f283b71 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -64,6 +64,7 @@ struct bcm2835_i2c_dev {
int irq;
struct i2c_adapter adapter;
struct completion completion;
+ struct i2c_msg *curr_msg;
u32 msg_err;
u8 *msg_buf;
size_t msg_buf_remaining;
@@ -126,14 +127,13 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
return IRQ_HANDLED;
}
- if (val & BCM2835_I2C_S_RXD) {
- bcm2835_drain_rxfifo(i2c_dev);
- if (!(val & BCM2835_I2C_S_DONE))
- return IRQ_HANDLED;
- }
-
if (val & BCM2835_I2C_S_DONE) {
- if (i2c_dev->msg_buf_remaining)
+ if (i2c_dev->curr_msg->flags & I2C_M_RD) {
+ bcm2835_drain_rxfifo(i2c_dev);
+ val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S);
+ }
+
+ if ((val & BCM2835_I2C_S_RXD) || i2c_dev->msg_buf_remaining)
i2c_dev->msg_err = BCM2835_I2C_S_LEN;
else
i2c_dev->msg_err = 0;
@@ -141,11 +141,16 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
return IRQ_HANDLED;
}
- if (val & BCM2835_I2C_S_TXD) {
+ if (val & BCM2835_I2C_S_TXW) {
bcm2835_fill_txfifo(i2c_dev);
return IRQ_HANDLED;
}
+ if (val & BCM2835_I2C_S_RXR) {
+ bcm2835_drain_rxfifo(i2c_dev);
+ return IRQ_HANDLED;
+ }
+
return IRQ_NONE;
}
@@ -155,6 +160,7 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
u32 c;
unsigned long time_left;
+ i2c_dev->curr_msg = msg;
i2c_dev->msg_buf = msg->buf;
i2c_dev->msg_buf_remaining = msg->len;
reinit_completion(&i2c_dev->completion);
--
2.8.2
^ permalink raw reply related
* [PATCH v4 0/7] i2c: bcm2835: Bring in changes from downstream
From: Noralf Trønnes @ 2016-10-03 20:06 UTC (permalink / raw)
To: linux-arm-kernel
This patchset tries to bring in the lessons learned in the downstream
driver i2c-bcm2708. The downstream clock stretcing timeout patch has
been left out since clock stretching is broken/unreliable on this
controller, so no point in setting it.
Changes since version 3:
- Add comment about I2C_C_CLEAR on error
Noralf.
Noralf Tr?nnes (7):
i2c: bcm2835: Fix hang for writing messages larger than 16 bytes
i2c: bcm2835: Protect against unexpected TXW/RXR interrupts
i2c: bcm2835: Use dev_dbg logging on transfer errors
i2c: bcm2835: Can't support I2C_M_IGNORE_NAK
i2c: bcm2835: Add support for Repeated Start Condition
i2c: bcm2835: Support i2c-dev ioctl I2C_TIMEOUT
i2c: bcm2835: Add support for dynamic clock
drivers/i2c/busses/i2c-bcm2835.c | 218 +++++++++++++++++++++++++--------------
1 file changed, 139 insertions(+), 79 deletions(-)
--
2.8.2
^ permalink raw reply
* [PATCH -next v2] arm*/efi: efi_init error handling fix
From: Matt Fleming @ 2016-10-03 19:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474371593-10250-1-git-send-email-xieyisheng1@huawei.com>
On Tue, 20 Sep, at 07:39:53PM, Yisheng Xie wrote:
> There's an early memmap leak in efi_init error path, fix it.
>
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> drivers/firmware/efi/arm-init.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index 9f8d996..7c2fe57 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -245,8 +245,10 @@ void __init efi_init(void)
> "Unexpected EFI_MEMORY_DESCRIPTOR version %ld",
> efi.memmap.desc_version);
>
> - if (uefi_init() < 0)
> + if (uefi_init() < 0) {
> + efi_memmap_unmap();
> return;
> + }
>
> reserve_regions();
> efi_memattr_init();
Thanks, applied to 'next'.
^ permalink raw reply
* [PATCH] i2c: bcm2835: Set up the clock stretching timeout at boot.
From: Eric Anholt @ 2016-10-03 19:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160722072740.GG1605@katana>
Wolfram Sang <wsa@the-dreams.de> writes:
> On Sun, Jul 03, 2016 at 06:02:32PM -0700, Eric Anholt wrote:
>> Wolfram Sang <wsa@the-dreams.de> writes:
>>
>> >> + /*
>> >> + * SMBUS says "Devices participating in a transfer will
>> >> + * timeout when any clock low exceeds the value of
>> >> + * T_TIMEOUT,MIN of 25 ms."
>> >> + */
>> >
>> > SMBus has that timeout, but I2C doesn't. How about disabling the timeout
>> > simply? Or using the max value if you want to keep the timeout
>> > detection?
>>
>> Disabling the timeout seems fine to me. We still have a 1-second
>> timeout around the entire transfer. I'll be back on my DSI branch this
>> week and test it out then.
>
> Did it work?
Sorry for the long-delayed feedback: It turned out that the reason I was
getting timeouts and looking into i2c in the first place was that the
firmware was driving that controller behind my back, so I couldn't do
useful testing anyway.
I put together a patch
(https://github.com/anholt/linux/commit/894200276239d2e4c60b378bdc52164fcb13af8d)
but I'm a bit concerned by it: I don't see a way to get the controller
back to its idle state without continuing through the I2C state machine,
and if the clock is still being stretched it doesn't continue unless CLK
is triggered.
What is supposed to happen when adap->timeout times out while the clock
is being stretched? Should we be able to try starting a fresh new I2C
transaction cleanly?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161003/880af017/attachment.sig>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox