Messages in this thread | | | Subject | Re: [PATCH v3 6/6] gpio: tps65086: Add GPIO driver for the TPS65086 PMIC | From | "Andrew F. Davis" <> | Date | Wed, 18 Nov 2015 10:41:10 -0600 |
| |
On 11/18/2015 03:44 AM, Linus Walleij wrote: > On Tue, Nov 17, 2015 at 5:11 PM, Andrew F. Davis <afd@ti.com> wrote: >> On 11/17/2015 03:17 AM, Linus Walleij wrote: >>> >>> On Wed, Nov 4, 2015 at 6:12 PM, Andrew F. Davis <afd@ti.com> wrote: >>> >>>> Add support for the TPS65086 PMIC GPOs. >>>> >>>> TPS65086 has four configurable GPOs that can be used for several >>>> purposes. >>>> >>>> Signed-off-by: Andrew F. Davis <afd@ti.com> >>> >>> >>> OK... >>> >>>> +static int tps65086_gpio_get(struct gpio_chip *gc, unsigned offset) >>>> +static void tps65086_gpio_set(struct gpio_chip *gc, unsigned offset, >>> >>> >>> Just get/set and no get_direction/direction_input/direction_output? >>> Are you sure? >>> >> >> Yeah, these are output only, I could probably add get_direction and just >> always return output, but setters wouldn't make sense here. > > It's important that you note in the driver, commit blurb and maybe even > Kconfig that these GPIOs are output-only. Someone could get confused. > > You should implement .direction_output() and return 0, and implement > .direction_input and return negative error code. As it is now, it will > seem like input is working, while it's not, right? > > Yours, > Linus Walleij >
Right, will add.
Also I would hold off on taking the GPIO bindings just yet. Mark Brown has stated he would like the compatible string removed from the regulator sub-node before he takes the regulator components, so for consistency sake I am also removing the string from GPIO as well. With this, it is difficult to have the sub-node still be a GPIO controller.
( gpiochip->of_node vs gpiochip->dev->of_node gpiolib.c:694 etc.. )
So the main pmic node will also be the GPIO controller node.
pmic: tps65912@2d { compatible = "ti,tps65912"; reg = <0x2d>;
interrupt-parent = <&gpio1>; interrupts = <28 8>;
interrupt-controller; #interrupt-cells = <2>;
gpio-controller; #gpio-cells = <2>; };
| |