linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chen-Yu Tsai <wens@csie.org>
To: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Chen-Yu Tsai <wens@csie.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-sunxi <linux-sunxi@googlegroups.com>
Subject: Re: [PATCH v2 2/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware
Date: Fri, 11 Nov 2016 16:39:01 +0800	[thread overview]
Message-ID: <CAGb2v65UfxZP4R5_=QF-=P1ebsKb9SbRNT10gr5yi7qdnrd4hg@mail.gmail.com> (raw)
In-Reply-To: <20161111083627.yooctcxqs4ow7sn3@lukather>

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

  reply	other threads:[~2016-11-11  8:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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  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  8:36   ` Maxime Ripard
2016-11-11  8:39     ` Chen-Yu Tsai [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAGb2v65UfxZP4R5_=QF-=P1ebsKb9SbRNT10gr5yi7qdnrd4hg@mail.gmail.com' \
    --to=wens@csie.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@googlegroups.com \
    --cc=maxime.ripard@free-electrons.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).