Board files constitute a significant part of the users of the legacy
GPIO framework. In many cases they only export a line and set its
desired value. We could use GPIO hogs for that like we do for DT and
ACPI but there's no support for that in machine code.
This patch proposes to extend the machine.h API with support for
registering hog tables in board files.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
v1 -> v2:
- kbuild bot complains about enum gpiod_flags having incomplete type
although it builds fine for me locally: change the type of dflags
to int
Documentation/driver-api/gpio/board.rst | 16 ++++++
drivers/gpio/gpiolib.c | 67 +++++++++++++++++++++++++
include/linux/gpio/machine.h | 31 ++++++++++++
3 files changed, 114 insertions(+)
diff --git a/Documentation/driver-api/gpio/board.rst b/Documentation/driver-api/gpio/board.rst
index 25d62b2e9fd0..2c112553df84 100644
--- a/Documentation/driver-api/gpio/board.rst
+++ b/Documentation/driver-api/gpio/board.rst
@@ -177,3 +177,19 @@ mapping and is thus transparent to GPIO consumers.
A set of functions such as gpiod_set_value() is available to work with
the new descriptor-oriented interface.
+
+Boards using platform data can also hog GPIO lines by defining GPIO hog tables.
+
+.. code-block:: c
+
+ struct gpiod_hog gpio_hog_table[] = {
+ GPIO_HOG("gpio.0", 10, "foo", GPIO_ACTIVE_LOW, GPIOD_OUT_HIGH),
+ { }
+ };
+
+And the table can be added to the board code as follows::
+
+ gpiod_add_hogs(gpio_hog_table);
+
+The line will be hogged as soon as the gpiochip is created or - in case the
+chip was created earlier - when the hog table is registered.
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 43aeb07343ec..547adc149b62 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -71,6 +71,9 @@ static DEFINE_MUTEX(gpio_lookup_lock);
static LIST_HEAD(gpio_lookup_list);
LIST_HEAD(gpio_devices);
+static DEFINE_MUTEX(gpio_machine_hogs_mutex);
+static LIST_HEAD(gpio_machine_hogs);
+
static void gpiochip_free_hogs(struct gpio_chip *chip);
static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
struct lock_class_key *lock_key,
@@ -1171,6 +1174,41 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
return status;
}
+static void gpiochip_machine_hog(struct gpio_chip *chip, struct gpiod_hog *hog)
+{
+ struct gpio_desc *desc;
+ int rv;
+
+ desc = gpiochip_get_desc(chip, hog->chip_hwnum);
+ if (IS_ERR(desc)) {
+ pr_err("%s: unable to get GPIO desc: %ld\n",
+ __func__, PTR_ERR(desc));
+ return;
+ }
+
+ if (desc->flags & FLAG_IS_HOGGED)
+ return;
+
+ rv = gpiod_hog(desc, hog->line_name, hog->lflags, hog->dflags);
+ if (rv)
+ pr_err("%s: unable to hog GPIO line (%s:%u): %d\n",
+ __func__, chip->label, hog->chip_hwnum, rv);
+}
+
+static void machine_gpiochip_add(struct gpio_chip *chip)
+{
+ struct gpiod_hog *hog;
+
+ mutex_lock(&gpio_machine_hogs_mutex);
+
+ list_for_each_entry(hog, &gpio_machine_hogs, list) {
+ if (!strcmp(chip->label, hog->chip_label))
+ gpiochip_machine_hog(chip, hog);
+ }
+
+ mutex_unlock(&gpio_machine_hogs_mutex);
+}
+
static void gpiochip_setup_devs(void)
{
struct gpio_device *gdev;
@@ -1326,6 +1364,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
acpi_gpiochip_add(chip);
+ machine_gpiochip_add(chip);
+
/*
* By first adding the chardev, and then adding the device,
* we get a device node entry in sysfs under
@@ -3462,6 +3502,33 @@ void gpiod_remove_lookup_table(struct gpiod_lookup_table *table)
}
EXPORT_SYMBOL_GPL(gpiod_remove_lookup_table);
+/**
+ * gpiod_add_hogs() - register a set of GPIO hogs from machine code
+ * @hogs: table of gpio hog entries with a zeroed sentinel at the end
+ */
+void gpiod_add_hogs(struct gpiod_hog *hogs)
+{
+ struct gpio_chip *chip;
+ struct gpiod_hog *hog;
+
+ mutex_lock(&gpio_machine_hogs_mutex);
+
+ for (hog = &hogs[0]; hog->chip_label; hog++) {
+ list_add_tail(&hog->list, &gpio_machine_hogs);
+
+ /*
+ * The chip may have been registered earlier, so check if it
+ * exists and, if so, try to hog the line now.
+ */
+ chip = find_chip_by_name(hog->chip_label);
+ if (chip)
+ gpiochip_machine_hog(chip, hog);
+ }
+
+ mutex_unlock(&gpio_machine_hogs_mutex);
+}
+EXPORT_SYMBOL_GPL(gpiod_add_hogs);
+
static struct gpiod_lookup_table *gpiod_find_lookup_table(struct device *dev)
{
const char *dev_id = dev ? dev_name(dev) : NULL;
diff --git a/include/linux/gpio/machine.h b/include/linux/gpio/machine.h
index b2f2dc638463..daa44eac9241 100644
--- a/include/linux/gpio/machine.h
+++ b/include/linux/gpio/machine.h
@@ -39,6 +39,23 @@ struct gpiod_lookup_table {
struct gpiod_lookup table[];
};
+/**
+ * struct gpiod_hog - GPIO line hog table
+ * @chip_label: name of the chip the GPIO belongs to
+ * @chip_hwnum: hardware number (i.e. relative to the chip) of the GPIO
+ * @line_name: consumer name for the hogged line
+ * @lflags: mask of GPIO lookup flags
+ * @dflags: GPIO flags used to specify the direction and value
+ */
+struct gpiod_hog {
+ struct list_head list;
+ const char *chip_label;
+ u16 chip_hwnum;
+ const char *line_name;
+ enum gpio_lookup_flags lflags;
+ int dflags;
+};
+
/*
* Simple definition of a single GPIO under a con_id
*/
@@ -59,10 +76,23 @@ struct gpiod_lookup_table {
.flags = _flags, \
}
+/*
+ * Simple definition of a single GPIO hog in an array.
+ */
+#define GPIO_HOG(_chip_label, _chip_hwnum, _line_name, _lflags, _dflags) \
+{ \
+ .chip_label = _chip_label, \
+ .chip_hwnum = _chip_hwnum, \
+ .line_name = _line_name, \
+ .lflags = _lflags, \
+ .dflags = _dflags, \
+}
+
#ifdef CONFIG_GPIOLIB
void gpiod_add_lookup_table(struct gpiod_lookup_table *table);
void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n);
void gpiod_remove_lookup_table(struct gpiod_lookup_table *table);
+void gpiod_add_hogs(struct gpiod_hog *hogs);
#else
static inline
void gpiod_add_lookup_table(struct gpiod_lookup_table *table) {}
@@ -70,6 +100,7 @@ static inline
void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n) {}
static inline
void gpiod_remove_lookup_table(struct gpiod_lookup_table *table) {}
+static inline void gpiod_add_hogs(struct gpiod_hog *hogs) {}
#endif
#endif /* __LINUX_GPIO_MACHINE_H */
--
2.17.0
On Dienstag, 10. April 2018 22:30:28 CEST Bartosz Golaszewski wrote:
> Board files constitute a significant part of the users of the legacy
> GPIO framework. In many cases they only export a line and set its
> desired value. We could use GPIO hogs for that like we do for DT and
> ACPI but there's no support for that in machine code.
>
> This patch proposes to extend the machine.h API with support for
> registering hog tables in board files.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> @@ -1326,6 +1364,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
>
> acpi_gpiochip_add(chip);
>
> + machine_gpiochip_add(chip);
> +
> /*
> * By first adding the chardev, and then adding the device,
> * we get a device node entry in sysfs under
> @@ -3462,6 +3502,33 @@ void gpiod_remove_lookup_table(struct gpiod_lookup_table *table)
I think I see the same problem right here in regards to pinctrls
and gpiohogs that have with DeviceTree:
<https://patchwork.kernel.org/patch/10313767/>
The problem is that unlike native gpio-controllers, pinctrls need
to have a "pin/gpio range" defined before any gpio-hogs can be added.
If this is not the case the generic pinctrl_gpio_reguest() [0] will
fail with -EPROBE_DEFER at this point. (see the call chain in the
"pinctrl: msm: fix gpio-hog related boot issueslogin register" mail
starting from gpiod_hog).
And now the crux of the matter is that currently in order for pinctrl
drivers to register the range they have to call gpiochip_add_pin_range() [1].
But they only can do it after the gpiochip_add_data_with_key() [2], since
this function initializes the pin_ranges list [3].
So what will happen is that you'll get an
"gpiochip_machine_hog: unable to hog GPIO line $LABEL $GPIONR -517" error
for every single gpio-hog and wonder why :(.
Regards,
Christian
[0] <https://elixir.bootlin.com/linux/v4.16.2/source/drivers/pinctrl/core.c#L743>
[1] <https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpiolib.c#L2078>
[2] <https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpiolib.c#L1136>
[3] <https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpiolib.c#L1253>
On Tue, Apr 10, 2018 at 10:30 PM, Bartosz Golaszewski <[email protected]> wrote:
> Board files constitute a significant part of the users of the legacy
> GPIO framework. In many cases they only export a line and set its
> desired value. We could use GPIO hogs for that like we do for DT and
> ACPI but there's no support for that in machine code.
>
> This patch proposes to extend the machine.h API with support for
> registering hog tables in board files.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> v1 -> v2:
> - kbuild bot complains about enum gpiod_flags having incomplete type
> although it builds fine for me locally: change the type of dflags
> to int
I like the idea and thinking behind this patch, so patch applied.
It's a bit of code to carry, so if it doesn't see any use, I will simply
revert it :)
But I bet you intend to follow up with some machine patches
and then it is immediately worth it.
Yours,
Linus Walleij
On Thu, Apr 12, 2018 at 10:00 PM, Christian Lamparter
<[email protected]> wrote:
> The problem is that unlike native gpio-controllers, pinctrls need
> to have a "pin/gpio range" defined before any gpio-hogs can be added.
Indeed. But the primary use case (correct me if I am wrong Bartosz)
is to clean up old boardfile code.
Old boardfiles belong to equally old boards. They very often do not
have pin control, just GPIO, and they very often have custom code
that just issue gpio_get(), gpio_* etc to set up hogs.
So this will be able to replace all such boilerplate with some
hog table for each boardfile and be done with it.
I.e. they have only drivers/gpio/gpio-foo.c and no pin control
driver in 9 cases out of 10. Cases do exist where they use
pin control with board files. Those are rare. But they will have
problems.
Some machine descriptor tables are used on modern archs
and the most prominent is x86 Intel. However the Intel pin control
driver is one of those that (IIRC) will actually survive this (i.e. it
doesn not have this bug). They are not even using DT, they
use ACPI.
> So what will happen is that you'll get an
> "gpiochip_machine_hog: unable to hog GPIO line $LABEL $GPIONR -517" error
> for every single gpio-hog and wonder why :(.
Hm maybe we can simply improbe the error messages so
people realize they have to go and fix their pin control driver(s)?
OK maybe a bit whimsical comment from me here... :/
Yours,
Linus Walleij
2018-04-26 14:07 GMT+02:00 Linus Walleij <[email protected]>:
> On Tue, Apr 10, 2018 at 10:30 PM, Bartosz Golaszewski <[email protected]> wrote:
>
>> Board files constitute a significant part of the users of the legacy
>> GPIO framework. In many cases they only export a line and set its
>> desired value. We could use GPIO hogs for that like we do for DT and
>> ACPI but there's no support for that in machine code.
>>
>> This patch proposes to extend the machine.h API with support for
>> registering hog tables in board files.
>>
>> Signed-off-by: Bartosz Golaszewski <[email protected]>
>> ---
>> v1 -> v2:
>> - kbuild bot complains about enum gpiod_flags having incomplete type
>> although it builds fine for me locally: change the type of dflags
>> to int
>
> I like the idea and thinking behind this patch, so patch applied.
>
> It's a bit of code to carry, so if it doesn't see any use, I will simply
> revert it :)
>
> But I bet you intend to follow up with some machine patches
> and then it is immediately worth it.
Yes, I'll be submitting patches removing the legacy gpio calls for
boards in mach-davinci.
Thanks,
Bartosz
On Thu, Apr 26, 2018 at 6:42 PM, Bartosz Golaszewski <[email protected]> wrote:
> 2018-04-26 14:07 GMT+02:00 Linus Walleij <[email protected]>:
>> On Tue, Apr 10, 2018 at 10:30 PM, Bartosz Golaszewski <[email protected]> wrote:
>>
>>> Board files constitute a significant part of the users of the legacy
>>> GPIO framework. In many cases they only export a line and set its
>>> desired value. We could use GPIO hogs for that like we do for DT and
>>> ACPI but there's no support for that in machine code.
>>>
>>> This patch proposes to extend the machine.h API with support for
>>> registering hog tables in board files.
>>>
>>> Signed-off-by: Bartosz Golaszewski <[email protected]>
>>> ---
>>> v1 -> v2:
>>> - kbuild bot complains about enum gpiod_flags having incomplete type
>>> although it builds fine for me locally: change the type of dflags
>>> to int
>>
>> I like the idea and thinking behind this patch, so patch applied.
>>
>> It's a bit of code to carry, so if it doesn't see any use, I will simply
>> revert it :)
>>
>> But I bet you intend to follow up with some machine patches
>> and then it is immediately worth it.
>
> Yes, I'll be submitting patches removing the legacy gpio calls for
> boards in mach-davinci.
Awesome, thanks!
(I guess they should all be converted to device tree though, hope that
will happen...)
Yours,
Linus Walleij