On Tue, Oct 04, 2016 at 09:51:12AM +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@xxxxxxxx>
> ---
> 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)
This is a nitpick, but I'd really like to store the value in a
separate variable, to have a distinction between the value that was
given us as an argument, and what we're going to write.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Attachment:
signature.asc
Description: PGP signature
