On Tue, Oct 20, 2015 at 3:39 AM, Russell King - ARM Linux
[off-list ref] wrote:

On Mon, Oct 19, 2015 at 08:27:44PM +0200, Uwe Kleine-König wrote:

quoted

Hello,

On Mon, Oct 19, 2015 at 04:43:24PM +0100, Russell King - ARM Linux wrote:

quoted

It's a bit ironic that you've chosen GPIO as an example there.  The
"new" GPIO API (the gpiod_* stuff) only has a fwnode way to get the
gpio descriptor.  There's no of_* method.

Without following all that fwnode discussion:
gpiod_get et al. should work for you here, doesn't it? It just takes a
struct device * and I'm happy with it.

What if you don't have a struct device?  I had that problem recently
when modifying the mvebu PCIe code.  The 'struct device' node doesn't
contain the GPIOs, it's the PCIe controller.  Individual ports on the
controller are described in DT as sub-nodes, and the sub-nodes can
have a GPIO for card reset purposes.  These sub-nodes don't have a
struct device.

Right now, I'm having to do this to work around this issue:

        reset_gpio = of_get_named_gpio_flags(child, "reset-gpios", 0, &flags);
        if (reset_gpio == -EPROBE_DEFER) {
                ret = reset_gpio;
                goto err;
        }

        if (gpio_is_valid(reset_gpio)) {
                unsigned long gpio_flags;

                port->reset_name = devm_kasprintf(dev, GFP_KERNEL, "%s-reset",
                                                  port->name);
                if (!port->reset_name) {
                        ret = -ENOMEM;
                        goto err;
                }

                if (flags & OF_GPIO_ACTIVE_LOW) {
                        dev_info(dev, "%s: reset gpio is active low\n",
                                 of_node_full_name(child));
                        gpio_flags = GPIOF_ACTIVE_LOW |
                                     GPIOF_OUT_INIT_LOW;
                } else {
                        gpio_flags = GPIOF_OUT_INIT_HIGH;
                }

                ret = devm_gpio_request_one(dev, reset_gpio, gpio_flags,
                                            port->reset_name);
                if (ret) {
                        if (ret == -EPROBE_DEFER)
                                goto err;
                        goto skip;
                }

                port->reset_gpio = gpio_to_desc(reset_gpio);
        }

Not nice, is it?  Not nice to have that in lots of drivers either.

However, switching to use any of_* or fwnode_* thing also carries with
it another problem: you can't control the name appearing in the
allocation, so you end up with a bunch of GPIOs requested with a "reset"
name - meaning you lose any identification of which port the GPIO was
bound to.

There are a few holes in the gpiod API. I see two solutions here:

1) extend devm_get_gpiod_from_child() to take an optional name argument
2) add a function to explicitly change a GPIO's name

2) seems to be the most generic solution, would that do the trick?

(sorry for the off-topic)