Re: [PATCH v3 6/6] gpio: tps65086: Add GPIO driver for the TPS65086 PMIC

From: Andrew F. Davis
Date: Wed Nov 18 2015 - 11:41:30 EST


On 11/18/2015 03:44 AM, Linus Walleij wrote:
On Tue, Nov 17, 2015 at 5:11 PM, Andrew F. Davis <afd@xxxxxx> wrote:
On 11/17/2015 03:17 AM, Linus Walleij wrote:

On Wed, Nov 4, 2015 at 6:12 PM, Andrew F. Davis <afd@xxxxxx> 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@xxxxxx>


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>;
};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/