[2/2] gpiolib: add support for fetching descriptors from static properties
diff mbox series

Message ID 20190713075259.243565-3-dmitry.torokhov@gmail.com
State New
Headers show
Series
  • Make gpiolib work with static device properties
Related show

Commit Message

Dmitry Torokhov July 13, 2019, 7:52 a.m. UTC
Now that static device properties understand notion of child nodes, let's
teach gpiolib to tie such children and machine GPIO descriptor tables.
We will continue using a single table for entire device, but instead of
using connection ID as a lookup key in the GPIO descriptor table directly,
we will perform additional translation: fwnode_get_named_gpiod() when
dealing with property_set-backed fwnodes will try parsing string property
with name matching connection ID and use result of the lookup as the key in
the table:

static const struct property_entry dev_child1_props[] __initconst = {
	...
	PROPERTY_ENTRY_STRING("gpios",		"child-1-gpios"),
	{ }
};

static struct gpiod_lookup_table dev_gpiod_table = {
	.dev_id = "some-device",
	.table = {
		...
		GPIO_LOOKUP_IDX("B", 1, "child-1-gpios", 1, GPIO_ACTIVE_LOW),
		...
	},
};

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib.c | 108 ++++++++++++++++++++++++++++++-----------
 1 file changed, 79 insertions(+), 29 deletions(-)

Comments

Linus Walleij July 28, 2019, 10:15 p.m. UTC | #1
On Sat, Jul 13, 2019 at 9:53 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> Now that static device properties understand notion of child nodes, let's
> teach gpiolib to tie such children and machine GPIO descriptor tables.
> We will continue using a single table for entire device, but instead of
> using connection ID as a lookup key in the GPIO descriptor table directly,
> we will perform additional translation: fwnode_get_named_gpiod() when
> dealing with property_set-backed fwnodes will try parsing string property
> with name matching connection ID and use result of the lookup as the key in
> the table:
>
> static const struct property_entry dev_child1_props[] __initconst = {
>         ...
>         PROPERTY_ENTRY_STRING("gpios",          "child-1-gpios"),
>         { }
> };
>
> static struct gpiod_lookup_table dev_gpiod_table = {
>         .dev_id = "some-device",
>         .table = {
>                 ...
>                 GPIO_LOOKUP_IDX("B", 1, "child-1-gpios", 1, GPIO_ACTIVE_LOW),
>                 ...
>         },
> };
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I'm pretty grateful for this since I think at one point I provoked this whole
series. :)

> +static struct gpio_desc *__fwnode_get_named_gpiod(struct fwnode_handle *fwnode,

I am allergic to __underscore_with_unclear_semantics() so I will
change this when applying to something with meaning (I even
like "inner_" better.)

Otherwise it's good to go when I get an ACK on the first patch.

Yours,
Linus Walleij
Heikki Krogerus July 30, 2019, 11:30 a.m. UTC | #2
Hi Dmitry,

On Sat, Jul 13, 2019 at 12:52:59AM -0700, Dmitry Torokhov wrote:
> Now that static device properties understand notion of child nodes, let's
> teach gpiolib to tie such children and machine GPIO descriptor tables.
> We will continue using a single table for entire device, but instead of
> using connection ID as a lookup key in the GPIO descriptor table directly,
> we will perform additional translation: fwnode_get_named_gpiod() when
> dealing with property_set-backed fwnodes will try parsing string property
> with name matching connection ID and use result of the lookup as the key in
> the table:
> 
> static const struct property_entry dev_child1_props[] __initconst = {
> 	...
> 	PROPERTY_ENTRY_STRING("gpios",		"child-1-gpios"),
> 	{ }
> };
> 
> static struct gpiod_lookup_table dev_gpiod_table = {
> 	.dev_id = "some-device",
> 	.table = {
> 		...
> 		GPIO_LOOKUP_IDX("B", 1, "child-1-gpios", 1, GPIO_ACTIVE_LOW),
> 		...
> 	},
> };

We don't need struct gpiod_lookup_table anymore. We can mimic DT with
the software nodes now that we have those "reference properties". A
gpio reference with the software nodes would look something like this:

        enum {
                GPIO_CONTROLLER,
                MY_DEVICE
        };

        static const struct software_node nodes[];

        static const struct software_node_ref_args reset_gpio_ref = {
                .node = &nodes[GPIO_CONTROLLER],
                .nargs = 2,
                .args = {
                        14,                     /* line number */
                        GPIO_ACTIVE_HIGH        /* flags */
                }
        };

        static const struct software_node_reference my_refs[] = {
                { "reset-gpios", 1, &reset_gpio_ref }
        };

        /* Optionally, we could support gpiochip finding by name... */
        static const struct property_entry my_props[] = {
                PROPERTY_ENTRY_STRING("gpio-controller", "name_of_the_controller")
        };

        static const struct software_node nodes[] = {
                [GPIO_CONTROLLER]       = { "gpio_controller" },
                [MY_DEVICE]             = { "my_device", NULL, my_props, my_refs }
        };

        void my_init(void)
        {
                ...
                ret = software_node_register_nodes(nodes);
                ...
        }

In gpiolib we should now be able to access that reference with
fwnode_property_get_references_args():

        static int gpiochip_match_fwnode(struct gpio_chip *chip, void *fwnode)
        {
                /* The fwnode member needs to be added to struct gpio_chip */
                return chip->fwnode == fwnode;
        }

        static struct gpio_desc *gpiod_find(struct device *dev,
                                            const char *con_id,
                                            unsigned int idx,
                                            unsigned long flags)
        {
                struct fwnode_reference_args args;
                struct gpio_chip *chip;
                struct gpio_desc *desc;
                const char *name;
                int ret;

                ret = fwnode_property_get_refernce_args(dev_fwnode(dev), con_id,
                                                        NULL, idx, &args);
                ...

                /* Let's find the gpiochip */
                chip = gpiochip_find(args.fwnode, gpiochip_match_fwnode);
                ...

                /* Or optionally with find_chip_by_name() */
                //ret = device_property_read_string(dev, "gpio-controller", &name);
                ...
                //chip = find_chip_by_name(name);
                ...

                /* I'm assuming hwnum is the same as line number? */
                desc = gpiochip_get_desc(chip, args.args[0]);
                *flags = args.args[1];

                return desc;
        }

The above is just an example, but I'm pretty sure that something like
it (with a little bit of tuning) is all that we need.

thanks,

Patch
diff mbox series

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e013d417a936..b6574febe2b8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4307,39 +4307,44 @@  struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
 }
 EXPORT_SYMBOL(gpiod_get_from_of_node);
 
-/**
- * fwnode_get_named_gpiod - obtain a GPIO from firmware node
- * @fwnode:	handle of the firmware node
- * @propname:	name of the firmware property representing the GPIO
- * @index:	index of the GPIO to obtain for the consumer
- * @dflags:	GPIO initialization flags
- * @label:	label to attach to the requested GPIO
- *
- * This function can be used for drivers that get their configuration
- * from opaque firmware.
- *
- * The function properly finds the corresponding GPIO using whatever is the
- * underlying firmware interface and then makes sure that the GPIO
- * descriptor is requested before it is returned to the caller.
- *
- * Returns:
- * On successful request the GPIO pin is configured in accordance with
- * provided @dflags.
- *
- * In case of error an ERR_PTR() is returned.
- */
-struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
-					 const char *propname, int index,
-					 enum gpiod_flags dflags,
-					 const char *label)
+static struct gpio_desc *
+software_node_get_gpiod(struct fwnode_handle *fwnode,
+			const char *propname, int index, unsigned long *flags)
+{
+	struct device *dev;
+	const char *con_id;
+
+	dev = software_node_get_linked_device(fwnode);
+	if (IS_ERR_OR_NULL(dev))
+		return ERR_PTR(-EINVAL);
+
+	if (fwnode_property_read_string(fwnode, propname, &con_id)) {
+		/*
+		 * We could not find string mapping property name to
+		 * entry in gpio lookup table. Let's see if we are
+		 * dealing with firmware node corresponding to the
+		 * device (and not a child node): for such nodes we can
+		 * try doing lookup directly with property name.
+		 */
+		if (fwnode_get_parent(fwnode))
+			return ERR_PTR(-ENOENT);
+
+		con_id = propname;
+	}
+
+	return gpiod_find(dev, con_id, index, flags);
+}
+
+static struct gpio_desc *__fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
+						  const char *propname,
+						  int index,
+						  enum gpiod_flags dflags,
+						  const char *label)
 {
 	unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
 	struct gpio_desc *desc = ERR_PTR(-ENODEV);
 	int ret;
 
-	if (!fwnode)
-		return ERR_PTR(-EINVAL);
-
 	if (is_of_node(fwnode)) {
 		desc = gpiod_get_from_of_node(to_of_node(fwnode),
 					      propname, index,
@@ -4355,9 +4360,13 @@  struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 
 		acpi_gpio_update_gpiod_flags(&dflags, &info);
 		acpi_gpio_update_gpiod_lookup_flags(&lflags, &info);
+	} else if (is_software_node(fwnode)) {
+		desc = software_node_get_gpiod(fwnode, propname, index,
+					       &lflags);
+		if (IS_ERR(desc))
+			return desc;
 	}
 
-	/* Currently only ACPI takes this path */
 	ret = gpiod_request(desc, label);
 	if (ret)
 		return ERR_PTR(ret);
@@ -4370,6 +4379,47 @@  struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 
 	return desc;
 }
+
+/**
+ * fwnode_get_named_gpiod - obtain a GPIO from firmware node
+ * @fwnode:	handle of the firmware node
+ * @propname:	name of the firmware property representing the GPIO
+ * @index:	index of the GPIO to obtain for the consumer
+ * @dflags:	GPIO initialization flags
+ * @label:	label to attach to the requested GPIO
+ *
+ * This function can be used for drivers that get their configuration
+ * from opaque firmware.
+ *
+ * The function properly finds the corresponding GPIO using whatever is the
+ * underlying firmware interface and then makes sure that the GPIO
+ * descriptor is requested before it is returned to the caller.
+ *
+ * Returns:
+ * On successful request the GPIO pin is configured in accordance with
+ * provided @dflags.
+ *
+ * In case of error an ERR_PTR() is returned.
+ */
+struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
+					 const char *propname, int index,
+					 enum gpiod_flags dflags,
+					 const char *label)
+{
+	struct gpio_desc *desc;
+
+	if (!fwnode)
+		return ERR_PTR(-EINVAL);
+
+	desc = __fwnode_get_named_gpiod(fwnode, propname, index, dflags, label);
+	if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT &&
+	    !IS_ERR_OR_NULL(fwnode->secondary)) {
+		desc = __fwnode_get_named_gpiod(fwnode->secondary,
+						propname, index, dflags, label);
+	}
+
+	return desc;
+}
 EXPORT_SYMBOL_GPL(fwnode_get_named_gpiod);
 
 /**