[PATCH v2 2/9] pinctrl: single: support gpio request and free
Haojian Zhuang
haojian.zhuang at gmail.com
Sun Oct 28 21:55:48 EDT 2012
On Tue, Oct 23, 2012 at 5:37 AM, Tony Lindgren <tony at atomide.com> wrote:
> * Tony Lindgren <tony at atomide.com> [121022 13:29]:
>> * Haojian Zhuang <haojian.zhuang at gmail.com> [121022 09:11]:
>> > --- a/drivers/pinctrl/pinctrl-single.c
>> > +++ b/drivers/pinctrl/pinctrl-single.c
>> > @@ -28,8 +28,10 @@
>> > #define DRIVER_NAME "pinctrl-single"
>> > #define PCS_MUX_PINS_NAME "pinctrl-single,pins"
>> > #define PCS_MUX_BITS_NAME "pinctrl-single,bits"
>> > +#define PCS_GPIO_FUNC_NAME "pinctrl-single,gpio-func"
>>
>> I think we can now get rid of these defines, I initially added
>> them as we had a bit hard time finding a suitable name for the
>> driver. These are only used in one location, so let's not add
>> new ones here.
>>
>> > static int pcs_request_gpio(struct pinctrl_dev *pctldev,
>> > - struct pinctrl_gpio_range *range, unsigned offset)
>> > + struct pinctrl_gpio_range *range, unsigned pin)
>> > {
>> > - return -ENOTSUPP;
>> > + struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
>> > + struct pcs_gpio_range *gpio = NULL;
>> > + int end, mux_bytes;
>> > + unsigned data;
>> > +
>> > + gpio = container_of(range, struct pcs_gpio_range, range);
>> > + if (!gpio->func_en)
>> > + return 0;
>> > + end = range->pin_base + range->npins - 1;
>> > + if (pin < range->pin_base || pin > end) {
>> > + dev_err(pctldev->dev, "pin %d isn't in the range of "
>> > + "%d to %d\n", pin, range->pin_base, end);
>> > + return -EINVAL;
>> > + }
>> > + mux_bytes = pcs->width / BITS_PER_BYTE;
>> > + data = pcs_readl(pcs->base + pin * mux_bytes) & ~pcs->fmask;
>> > + data |= gpio->gpio_func;
>> > + pcs_writel(data, pcs->base + pin * mux_bytes);
>> > + return 0;
>> > }
>>
>> I think I already commented on this one.. Is this safe if you don't
>> have GPIOs configured? Or should you return -ENODEV in that case?
>
> Oops also you should not use pcs_readl/pcs_writel in the driver
> directly but use pcs_read instead as you can have register width other
> than 32-bits.
>
I think you're meaning pcs->read(). I'll use this interface.
More information about the linux-arm-kernel
mailing list