From: Marek Vasut <marex@denx.de>
To: linux-gpio@vger.kernel.org
Cc: Marek Vasut <marex@denx.de>,
Roman Guskov <rguskov@dh-electronics.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Bartosz Golaszewski <bgolaszewski@baylibre.com>,
Linus Walleij <linus.walleij@linaro.org>,
Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: [PATCH] Revert "gpiolib: generalize devprop_gpiochip_set_names() for device properties"
Date: Fri, 5 Mar 2021 00:09:56 +0100 [thread overview]
Message-ID: <20210304230956.154626-1-marex@denx.de> (raw)
This reverts 7cba1a4d5e162 ("gpiolib: generalize devprop_gpiochip_set_names()
for device properties") because this breaks OF "gpio-line-names" behavior on
STM32MP1 platform.
On STM32MP1, the GPIO banks are subnodes of pin-controller@50002000, see
arch/arm/boot/dts/stm32mp151.dtsi. The driver for pin-controller@50002000
is in drivers/pinctrl/stm32/pinctrl-stm32.c and iterates over all of its DT
subnodes when registering each GPIO bank gpiochip. Each gpiochip has:
- gpio_chip.parent = dev where dev is the device node of the pin controller
- gpio_chip.of_node = np which is the OF node of the GPIO bank
Therefore, dev_fwnode(chip->parent) != of_fwnode_handle(chip.of_node) , i.e.
pin-controller@50002000 != pin-controller@50002000/gpio@5000*000.
The original code behaved correctly, as it extracted the "gpio-line-names"
from of_fwnode_handle(chip.of_node) = pin-controller@50002000/gpio@5000*000.
This patch reinstates the original correct behavior.
Signed-off-by: Marek Vasut <marex@denx.de>
Reported-by: Roman Guskov <rguskov@dh-electronics.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Roman Guskov <rguskov@dh-electronics.com>
---
drivers/gpio/gpiolib-acpi.c | 3 +++
drivers/gpio/gpiolib-of.c | 5 +++++
drivers/gpio/gpiolib.c | 32 ++++++++++++++------------------
include/linux/gpio/driver.h | 3 +++
4 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index e37a57d0a2f07..7a7618edfa198 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1252,6 +1252,9 @@ void acpi_gpiochip_add(struct gpio_chip *chip)
return;
}
+ if (!chip->names)
+ devprop_gpiochip_set_names(chip, dev_fwnode(chip->parent));
+
acpi_gpiochip_request_regions(acpi_gpio);
acpi_gpiochip_scan_gpios(acpi_gpio);
acpi_walk_dep_device_list(handle);
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index baf0153b7bca3..3b21c999172e6 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -1026,6 +1026,11 @@ int of_gpiochip_add(struct gpio_chip *chip)
if (ret)
return ret;
+ /* If the chip defines names itself, these take precedence */
+ if (!chip->names)
+ devprop_gpiochip_set_names(chip,
+ of_fwnode_handle(chip->of_node));
+
of_node_get(chip->of_node);
ret = of_gpiochip_scan_gpios(chip);
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index adf55db080d86..12362cd9b3c98 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -343,6 +343,9 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc)
struct gpio_device *gdev = gc->gpiodev;
int i;
+ if (!gc->names)
+ return 0;
+
/* First check all names if they are unique */
for (i = 0; i != gc->ngpio; ++i) {
struct gpio_desc *gpio;
@@ -364,27 +367,25 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc)
/*
* devprop_gpiochip_set_names - Set GPIO line names using device properties
* @chip: GPIO chip whose lines should be named, if possible
+ * @fwnode: Property Node containing the gpio-line-names property
*
* Looks for device property "gpio-line-names" and if it exists assigns
* GPIO line names for the chip. The memory allocated for the assigned
- * names belong to the underlying software node and should not be released
+ * names belong to the underlying firmware node and should not be released
* by the caller.
*/
-static int devprop_gpiochip_set_names(struct gpio_chip *chip)
+void devprop_gpiochip_set_names(struct gpio_chip *chip,
+ const struct fwnode_handle *fwnode)
{
struct gpio_device *gdev = chip->gpiodev;
- struct device *dev = chip->parent;
const char **names;
int ret, i;
int count;
- /* GPIO chip may not have a parent device whose properties we inspect. */
- if (!dev)
- return 0;
-
- count = device_property_string_array_count(dev, "gpio-line-names");
+ count = fwnode_property_read_string_array(fwnode, "gpio-line-names",
+ NULL, 0);
if (count < 0)
- return 0;
+ return;
if (count > gdev->ngpio) {
dev_warn(&gdev->dev, "gpio-line-names is length %d but should be at most length %d",
@@ -394,22 +395,20 @@ static int devprop_gpiochip_set_names(struct gpio_chip *chip)
names = kcalloc(count, sizeof(*names), GFP_KERNEL);
if (!names)
- return -ENOMEM;
+ return;
- ret = device_property_read_string_array(dev, "gpio-line-names",
+ ret = fwnode_property_read_string_array(fwnode, "gpio-line-names",
names, count);
if (ret < 0) {
dev_warn(&gdev->dev, "failed to read GPIO line names\n");
kfree(names);
- return ret;
+ return;
}
for (i = 0; i < count; i++)
gdev->descs[i].name = names[i];
kfree(names);
-
- return 0;
}
static unsigned long *gpiochip_allocate_mask(struct gpio_chip *gc)
@@ -682,10 +681,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
INIT_LIST_HEAD(&gdev->pin_ranges);
#endif
- if (gc->names)
- ret = gpiochip_set_desc_names(gc);
- else
- ret = devprop_gpiochip_set_names(gc);
+ ret = gpiochip_set_desc_names(gc);
if (ret)
goto err_remove_from_list;
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 286de0520574e..adc1ddcd1b739 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -685,6 +685,9 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc,
enum gpiod_flags dflags);
void gpiochip_free_own_desc(struct gpio_desc *desc);
+void devprop_gpiochip_set_names(struct gpio_chip *gc,
+ const struct fwnode_handle *fwnode);
+
#ifdef CONFIG_GPIOLIB
/* lock/unlock as IRQ */
--
2.30.1
next reply other threads:[~2021-03-04 23:10 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-04 23:09 Marek Vasut [this message]
2021-03-05 10:44 ` [PATCH] Revert "gpiolib: generalize devprop_gpiochip_set_names() for device properties" Andy Shevchenko
2021-03-05 10:51 ` Marek Vasut
2021-03-05 12:03 ` Andy Shevchenko
2021-03-05 12:12 ` Marek Vasut
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210304230956.154626-1-marex@denx.de \
--to=marex@denx.de \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bgolaszewski@baylibre.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=mika.westerberg@linux.intel.com \
--cc=rguskov@dh-electronics.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).