From: tony@atomide•com (Tony Lindgren)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v4 3/9] pinctrl: single: support pinconf generic
Date: Thu, 8 Nov 2012 14:55:48 -0800 [thread overview]
Message-ID: <20121108225547.GL6801@atomide.com> (raw)
In-Reply-To: <1352301582-12244-4-git-send-email-haojian.zhuang@gmail.com>
Hi,
Noticed few more things while playing with this.
* Haojian Zhuang <haojian.zhuang@gmail•com> [121107 07:21]:
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> static int pcs_pinconf_get(struct pinctrl_dev *pctldev,
> unsigned pin, unsigned long *config)
> {
> + struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
> + enum pin_config_param param = pinconf_to_config_param(*config);
> + unsigned data;
> + u32 offset;
I suggest adding:
int res = -ENOTSUPP;
> +
> + offset = pin * (pcs->width / BITS_PER_BYTE);
> + data = pcs->read(pcs->base + offset);
> +
> + switch (param) {
> + case PIN_CONFIG_POWER_SOURCE:
> + if (pcs->psmask == PCS_OFF_DISABLED
> + || pcs->psshift == PCS_OFF_DISABLED)
> + return -ENOTSUPP;
> + data &= pcs->psmask;
> + data = data >> pcs->psshift;
> + *config = data;
> + return 0;
> + break;
then you can do:
res = 0;
break;
instead of the return 0 and break.
> + case PIN_CONFIG_BIAS_DISABLE:
> + if (pcs->bmask == PCS_OFF_DISABLED
> + || pcs->bshift == PCS_OFF_DISABLED
> + || pcs->bdis == PCS_OFF_DISABLED)
> + return -ENOTSUPP;
> + data &= pcs->bmask;
> + *config = 0;
> + if (data == pcs->bdis)
> + return 0;
> + else
> + return -EINVAL;
> + break;
> + case PIN_CONFIG_BIAS_PULL_UP:
> + if (pcs->bmask == PCS_OFF_DISABLED
> + || pcs->bshift == PCS_OFF_DISABLED
> + || pcs->bpullup == PCS_OFF_DISABLED)
> + return -ENOTSUPP;
> + data &= pcs->bmask;
> + *config = 0;
> + if (data == pcs->bpullup)
> + return 0;
> + else
> + return -EINVAL;
> + break;
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + if (pcs->bmask == PCS_OFF_DISABLED
> + || pcs->bshift == PCS_OFF_DISABLED
> + || pcs->bpulldown == PCS_OFF_DISABLED)
> + return -ENOTSUPP;
> + data &= pcs->bmask;
> + *config = 0;
> + if (data == pcs->bpulldown)
> + return 0;
> + else
> + return -EINVAL;
> + break;
> + default:
> + break;
> + }
> return -ENOTSUPP;
return res;
> }
Did you forget to add the input schmitt handling to pcs_pinconf_get()
above?
> static int pcs_pinconf_set(struct pinctrl_dev *pctldev,
> unsigned pin, unsigned long config)
> {
> - return -ENOTSUPP;
> + struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
> + enum pin_config_param config_param = pinconf_to_config_param(config);
> + unsigned ret, mask = ~0UL;
> + u32 offset, data;
> +
> + switch (config_param) {
> + case PIN_CONFIG_POWER_SOURCE:
> + if (pcs->psmask == PCS_OFF_DISABLED
> + || pcs->psshift == PCS_OFF_DISABLED)
> + return 0;
> + mask = pcs->psmask;
> + data = (pinconf_to_config_argument(config) << pcs->psshift)
> + & pcs->psmask;
> + break;
> + case PIN_CONFIG_BIAS_DISABLE:
> + if (pcs->bmask == PCS_OFF_DISABLED
> + || pcs->bshift == PCS_OFF_DISABLED)
> + return 0;
> + mask = pcs->bmask;
> + data = (pinconf_to_config_argument(config) << pcs->bshift)
> + & pcs->bmask;
> + break;
> + default:
> + return 0;
Here we should probably return -ENOTSUPP instead for the unhandled
ones?
> + }
> + offset = pin * (pcs->width / BITS_PER_BYTE);
> + ret = pcs->read(pcs->base + offset) & ~mask;
> + pcs->write(ret | data, pcs->base + offset);
> + return 0;
> }
Then looks like pcs_pinconf_set() is also missing handling for few
of the things configured in the probe?
Regards,
Tony
next prev parent reply other threads:[~2012-11-08 22:55 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-07 15:19 [PATCH v4 0/9] use pinctrl-single in arch mmp Haojian Zhuang
2012-11-07 15:19 ` [PATCH v4 1/9] ARM: mmp: select pinctrl driver Haojian Zhuang
2012-11-08 1:38 ` Tony Lindgren
2012-11-10 14:53 ` Haojian Zhuang
2012-11-13 13:37 ` Linus Walleij
2012-11-07 15:19 ` [PATCH v4 2/9] pinctrl: single: support gpio request and free Haojian Zhuang
2012-11-07 22:27 ` Tony Lindgren
2012-11-13 13:07 ` Linus Walleij
2012-11-13 17:34 ` Tony Lindgren
2012-11-14 1:44 ` Haojian Zhuang
2012-11-07 15:19 ` [PATCH v4 3/9] pinctrl: single: support pinconf generic Haojian Zhuang
2012-11-08 0:25 ` Tony Lindgren
2012-11-08 22:55 ` Tony Lindgren [this message]
2012-11-08 23:13 ` Tony Lindgren
2012-11-09 21:53 ` Tony Lindgren
2012-11-07 15:19 ` [PATCH v4 4/9] ARM: dts: support pinctrl single in pxa910 Haojian Zhuang
2012-11-09 22:48 ` Tony Lindgren
2012-11-10 5:37 ` Haojian Zhuang
2012-11-10 20:04 ` Tony Lindgren
2012-11-07 15:19 ` [PATCH v4 5/9] document: devicetree: bind pinconf with pin-single Haojian Zhuang
2012-11-08 1:37 ` Tony Lindgren
2012-11-12 20:37 ` Stephen Warren
2012-11-15 8:27 ` Haojian Zhuang
2012-11-15 8:30 ` Haojian Zhuang
2012-11-07 15:19 ` [PATCH v4 6/9] tty: pxa: configure pin Haojian Zhuang
2012-11-07 15:19 ` [PATCH v4 7/9] i2c: pxa: use devm_kzalloc Haojian Zhuang
2012-11-07 15:19 ` [PATCH v4 8/9] i2c: pxa: configure pinmux Haojian Zhuang
2012-11-07 15:19 ` [PATCH v4 9/9] pinctrl: single: dump pinmux register value Haojian Zhuang
2012-11-08 1:25 ` Tony Lindgren
2012-11-13 13:08 ` Linus Walleij
2012-11-13 17:34 ` Tony Lindgren
2012-11-14 1:45 ` Haojian Zhuang
2012-11-15 14:11 ` Linus Walleij
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=20121108225547.GL6801@atomide.com \
--to=tony@atomide$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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