* [PATCH 1/3] device: property: add helpers to count items in string arrays
2020-09-08 12:58 [PATCH 0/3] gpiolib: generalize GPIO line names property Bartosz Golaszewski
@ 2020-09-08 12:58 ` Bartosz Golaszewski
2020-09-08 13:47 ` Andy Shevchenko
2020-09-08 12:58 ` [PATCH 2/3] gpiolib: generalize devprop_gpiochip_set_names() for device properties Bartosz Golaszewski
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Bartosz Golaszewski @ 2020-09-08 12:58 UTC (permalink / raw)
To: Linus Walleij, Mika Westerberg, Andy Shevchenko, Kent Gibson,
Greg Kroah-Hartman
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Instead of doing the following:
count = device_property_read_string_array(dev, propname, NULL, 0);
Let's provide inline helpers with hardcoded arguments for counting
strings in property arrays.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
include/linux/property.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/linux/property.h b/include/linux/property.h
index 9f805c442819..1fa5e250a8ea 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -170,6 +170,12 @@ static inline int device_property_count_u64(struct device *dev, const char *prop
return device_property_read_u64_array(dev, propname, NULL, 0);
}
+static inline int device_property_count_strings(struct device *dev,
+ const char *propname)
+{
+ return device_property_read_string_array(dev, propname, NULL, 0);
+}
+
static inline bool fwnode_property_read_bool(const struct fwnode_handle *fwnode,
const char *propname)
{
@@ -224,6 +230,13 @@ static inline int fwnode_property_count_u64(const struct fwnode_handle *fwnode,
return fwnode_property_read_u64_array(fwnode, propname, NULL, 0);
}
+static inline int
+fwnode_property_count_strings(const struct fwnode_handle *fwnode,
+ const char *propname)
+{
+ return fwnode_property_read_string_array(fwnode, propname, NULL, 0);
+}
+
struct software_node;
/**
--
2.26.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] device: property: add helpers to count items in string arrays
2020-09-08 12:58 ` [PATCH 1/3] device: property: add helpers to count items in string arrays Bartosz Golaszewski
@ 2020-09-08 13:47 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2020-09-08 13:47 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Mika Westerberg, Kent Gibson, Greg Kroah-Hartman,
linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
On Tue, Sep 08, 2020 at 02:58:11PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Instead of doing the following:
>
> count = device_property_read_string_array(dev, propname, NULL, 0);
>
> Let's provide inline helpers with hardcoded arguments for counting
> strings in property arrays.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Thanks!
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> include/linux/property.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 9f805c442819..1fa5e250a8ea 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -170,6 +170,12 @@ static inline int device_property_count_u64(struct device *dev, const char *prop
> return device_property_read_u64_array(dev, propname, NULL, 0);
> }
>
> +static inline int device_property_count_strings(struct device *dev,
> + const char *propname)
> +{
> + return device_property_read_string_array(dev, propname, NULL, 0);
> +}
> +
> static inline bool fwnode_property_read_bool(const struct fwnode_handle *fwnode,
> const char *propname)
> {
> @@ -224,6 +230,13 @@ static inline int fwnode_property_count_u64(const struct fwnode_handle *fwnode,
> return fwnode_property_read_u64_array(fwnode, propname, NULL, 0);
> }
>
> +static inline int
> +fwnode_property_count_strings(const struct fwnode_handle *fwnode,
> + const char *propname)
> +{
> + return fwnode_property_read_string_array(fwnode, propname, NULL, 0);
> +}
> +
> struct software_node;
>
> /**
> --
> 2.26.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] gpiolib: generalize devprop_gpiochip_set_names() for device properties
2020-09-08 12:58 [PATCH 0/3] gpiolib: generalize GPIO line names property Bartosz Golaszewski
2020-09-08 12:58 ` [PATCH 1/3] device: property: add helpers to count items in string arrays Bartosz Golaszewski
@ 2020-09-08 12:58 ` Bartosz Golaszewski
2020-09-08 12:58 ` [PATCH 3/3] gpiolib: unexport devprop_gpiochip_set_names() Bartosz Golaszewski
2020-09-15 12:01 ` [PATCH 0/3] gpiolib: generalize GPIO line names property Anders Roxell
3 siblings, 0 replies; 14+ messages in thread
From: Bartosz Golaszewski @ 2020-09-08 12:58 UTC (permalink / raw)
To: Linus Walleij, Mika Westerberg, Andy Shevchenko, Kent Gibson,
Greg Kroah-Hartman
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
devprop_gpiochip_set_names() is overly complicated with taking the
fwnode argument (which requires using dev_fwnode() & of_fwnode_handle()
in ACPI and OF GPIO code respectively). Let's just switch to using the
generic device properties.
This allows us to pull the code setting line names directly into
gpiochip_add_data_with_key() instead of handling it separately for
ACPI and OF.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/gpio/gpiolib-acpi.c | 3 ---
drivers/gpio/gpiolib-devprop.c | 20 ++++++++++----------
drivers/gpio/gpiolib-of.c | 5 -----
drivers/gpio/gpiolib.c | 8 ++++----
include/linux/gpio/driver.h | 3 +--
5 files changed, 15 insertions(+), 24 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 54ca3c18b291..834a12f3219e 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1221,9 +1221,6 @@ 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-devprop.c b/drivers/gpio/gpiolib-devprop.c
index 26741032fa9e..29ee00d7730e 100644
--- a/drivers/gpio/gpiolib-devprop.c
+++ b/drivers/gpio/gpiolib-devprop.c
@@ -17,25 +17,23 @@
/**
* 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 firmware node and should not be released
+ * names belong to the underlying software node and should not be released
* by the caller.
*/
-void devprop_gpiochip_set_names(struct gpio_chip *chip,
- const struct fwnode_handle *fwnode)
+int devprop_gpiochip_set_names(struct gpio_chip *chip)
{
struct gpio_device *gdev = chip->gpiodev;
+ struct device *dev = chip->parent;
const char **names;
int ret, i;
int count;
- count = fwnode_property_read_string_array(fwnode, "gpio-line-names",
- NULL, 0);
+ count = device_property_count_strings(dev, "gpio-line-names");
if (count < 0)
- return;
+ return 0;
if (count > gdev->ngpio) {
dev_warn(&gdev->dev, "gpio-line-names is length %d but should be at most length %d",
@@ -45,19 +43,21 @@ void devprop_gpiochip_set_names(struct gpio_chip *chip,
names = kcalloc(count, sizeof(*names), GFP_KERNEL);
if (!names)
- return;
+ return -ENOMEM;
- ret = fwnode_property_read_string_array(fwnode, "gpio-line-names",
+ ret = device_property_read_string_array(dev, "gpio-line-names",
names, count);
if (ret < 0) {
dev_warn(&gdev->dev, "failed to read GPIO line names\n");
kfree(names);
- return;
+ return ret;
}
for (i = 0; i < count; i++)
gdev->descs[i].name = names[i];
kfree(names);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(devprop_gpiochip_set_names);
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index bd31dd3b6a75..2f895a2b8411 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -1026,11 +1026,6 @@ 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 80137c1b3cdc..0d390f0ec32c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -340,9 +340,6 @@ 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;
@@ -621,7 +618,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
INIT_LIST_HEAD(&gdev->pin_ranges);
#endif
- ret = gpiochip_set_desc_names(gc);
+ if (gc->names)
+ ret = gpiochip_set_desc_names(gc);
+ else
+ ret = devprop_gpiochip_set_names(gc);
if (ret)
goto err_remove_from_list;
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index d1cef5c2715c..56485a040b82 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -756,8 +756,7 @@ 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);
+int devprop_gpiochip_set_names(struct gpio_chip *gc);
#ifdef CONFIG_GPIOLIB
--
2.26.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 3/3] gpiolib: unexport devprop_gpiochip_set_names()
2020-09-08 12:58 [PATCH 0/3] gpiolib: generalize GPIO line names property Bartosz Golaszewski
2020-09-08 12:58 ` [PATCH 1/3] device: property: add helpers to count items in string arrays Bartosz Golaszewski
2020-09-08 12:58 ` [PATCH 2/3] gpiolib: generalize devprop_gpiochip_set_names() for device properties Bartosz Golaszewski
@ 2020-09-08 12:58 ` Bartosz Golaszewski
2020-09-08 13:49 ` Andy Shevchenko
2020-09-15 12:01 ` [PATCH 0/3] gpiolib: generalize GPIO line names property Anders Roxell
3 siblings, 1 reply; 14+ messages in thread
From: Bartosz Golaszewski @ 2020-09-08 12:58 UTC (permalink / raw)
To: Linus Walleij, Mika Westerberg, Andy Shevchenko, Kent Gibson,
Greg Kroah-Hartman
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Now that devprop_gpiochip_set_names() is only used in a single place
inside drivers/gpio/gpiolib.c, there's no need anymore for it to be
exported or to even live in its own source file. Pull this function into
the core source file for gpiolib.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/gpio/Makefile | 1 -
drivers/gpio/gpiolib.c | 47 +++++++++++++++++++++++++++++++++++++
include/linux/gpio/driver.h | 2 --
3 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4f9abff4f2dc..639275eb4e4d 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -6,7 +6,6 @@ ccflags-$(CONFIG_DEBUG_GPIO) += -DDEBUG
obj-$(CONFIG_GPIOLIB) += gpiolib.o
obj-$(CONFIG_GPIOLIB) += gpiolib-devres.o
obj-$(CONFIG_GPIOLIB) += gpiolib-legacy.o
-obj-$(CONFIG_GPIOLIB) += gpiolib-devprop.o
obj-$(CONFIG_GPIOLIB) += gpiolib-cdev.o
obj-$(CONFIG_OF_GPIO) += gpiolib-of.o
obj-$(CONFIG_GPIO_SYSFS) += gpiolib-sysfs.o
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 0d390f0ec32c..2b2526c5de51 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -358,6 +358,53 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc)
return 0;
}
+/*
+ * devprop_gpiochip_set_names - Set GPIO line names using device properties
+ * @chip: GPIO chip whose lines should be named, if possible
+ *
+ * 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
+ * by the caller.
+ */
+static int devprop_gpiochip_set_names(struct gpio_chip *chip)
+{
+ struct gpio_device *gdev = chip->gpiodev;
+ struct device *dev = chip->parent;
+ const char **names;
+ int ret, i;
+ int count;
+
+ count = device_property_count_strings(dev, "gpio-line-names");
+ if (count < 0)
+ return 0;
+
+ if (count > gdev->ngpio) {
+ dev_warn(&gdev->dev, "gpio-line-names is length %d but should be at most length %d",
+ count, gdev->ngpio);
+ count = gdev->ngpio;
+ }
+
+ names = kcalloc(count, sizeof(*names), GFP_KERNEL);
+ if (!names)
+ return -ENOMEM;
+
+ ret = device_property_read_string_array(dev, "gpio-line-names",
+ names, count);
+ if (ret < 0) {
+ dev_warn(&gdev->dev, "failed to read GPIO line names\n");
+ kfree(names);
+ return ret;
+ }
+
+ 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)
{
unsigned long *p;
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 56485a040b82..4a7e295c3640 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -756,8 +756,6 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc,
enum gpiod_flags dflags);
void gpiochip_free_own_desc(struct gpio_desc *desc);
-int devprop_gpiochip_set_names(struct gpio_chip *gc);
-
#ifdef CONFIG_GPIOLIB
/* lock/unlock as IRQ */
--
2.26.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/3] gpiolib: unexport devprop_gpiochip_set_names()
2020-09-08 12:58 ` [PATCH 3/3] gpiolib: unexport devprop_gpiochip_set_names() Bartosz Golaszewski
@ 2020-09-08 13:49 ` Andy Shevchenko
2020-09-08 14:01 ` Bartosz Golaszewski
0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2020-09-08 13:49 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Mika Westerberg, Kent Gibson, Greg Kroah-Hartman,
linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
On Tue, Sep 08, 2020 at 02:58:13PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Now that devprop_gpiochip_set_names() is only used in a single place
> inside drivers/gpio/gpiolib.c, there's no need anymore for it to be
> exported or to even live in its own source file. Pull this function into
> the core source file for gpiolib.
Did you miss to remove old file?
> drivers/gpio/Makefile | 1 -
> drivers/gpio/gpiolib.c | 47 +++++++++++++++++++++++++++++++++++++
> include/linux/gpio/driver.h | 2 --
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] gpiolib: unexport devprop_gpiochip_set_names()
2020-09-08 13:49 ` Andy Shevchenko
@ 2020-09-08 14:01 ` Bartosz Golaszewski
0 siblings, 0 replies; 14+ messages in thread
From: Bartosz Golaszewski @ 2020-09-08 14:01 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Mika Westerberg, Kent Gibson, Greg Kroah-Hartman,
open list:GPIO SUBSYSTEM, Linux Kernel Mailing List,
ACPI Devel Maling List, Bartosz Golaszewski
On Tue, Sep 8, 2020 at 3:52 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Sep 08, 2020 at 02:58:13PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Now that devprop_gpiochip_set_names() is only used in a single place
> > inside drivers/gpio/gpiolib.c, there's no need anymore for it to be
> > exported or to even live in its own source file. Pull this function into
> > the core source file for gpiolib.
>
> Did you miss to remove old file?
>
> > drivers/gpio/Makefile | 1 -
> > drivers/gpio/gpiolib.c | 47 +++++++++++++++++++++++++++++++++++++
> > include/linux/gpio/driver.h | 2 --
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Arr yes, rebase failed me. :(
Bart
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] gpiolib: generalize GPIO line names property
2020-09-08 12:58 [PATCH 0/3] gpiolib: generalize GPIO line names property Bartosz Golaszewski
` (2 preceding siblings ...)
2020-09-08 12:58 ` [PATCH 3/3] gpiolib: unexport devprop_gpiochip_set_names() Bartosz Golaszewski
@ 2020-09-15 12:01 ` Anders Roxell
2020-09-15 13:12 ` Andy Shevchenko
2020-09-16 13:29 ` Bartosz Golaszewski
3 siblings, 2 replies; 14+ messages in thread
From: Anders Roxell @ 2020-09-15 12:01 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Mika Westerberg, Andy Shevchenko, Kent Gibson,
Greg Kroah-Hartman, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, linux-acpi, Bartosz Golaszewski,
lkft-triage, Linux-Next Mailing List, Stephen Rothwell
On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> I initially sent this as part of the gpio-mockup overhaul but since
> these patches are indepentent and the work on gpio-mockup may become
> more complicated - I'm sending these separately.
>
> The only change is adding additional property helpers to count strings
> in array.
>
> Bartosz Golaszewski (3):
> device: property: add helpers to count items in string arrays
> gpiolib: generalize devprop_gpiochip_set_names() for device properties
> gpiolib: unexport devprop_gpiochip_set_names()
I do an arm64 allmodconfig build fron linux-next (tag: next-20200915) and
run that in qemu. When I run I see the following output (see full log [1]):
"BUG: KASAN: null-ptr-deref in device_property_read_string_array".
[ 6186.339462][ T1] unittest-gpio
testcase-data:overlay-node:test-bus:gpio@0: no pinctrl handle
[ 6186.346148][ T1] gpiochip_find_base: found new base at 507
[ 6186.348684][ T1]
==================================================================
[ 6186.351563][ T1] BUG: KASAN: null-ptr-deref in
device_property_read_string_array+0x40/0xa0
[ 6186.355157][ T1] Read of size 8 at addr 0000000000000570 by task
swapper/0/1
[ 6186.358212][ T1]
[ 6186.359361][ T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G
W 5.9.0-rc5-next-20200915-00006-g104c8fe4916b #1
[ 6186.363877][ T1] Hardware name: linux,dummy-virt (DT)
[ 6186.366156][ T1] Call trace:
[ 6186.367540][ T1] dump_backtrace+0x0/0x320
[ 6186.369446][ T1] show_stack+0x38/0x60
[ 6186.371282][ T1] dump_stack+0x1d4/0x278
[ 6186.373193][ T1] __kasan_report+0x148/0x180
[ 6186.375265][ T1] kasan_report+0x44/0xe0
[ 6186.377168][ T1] __asan_load8+0xbc/0xe0
[ 6186.379069][ T1] device_property_read_string_array+0x40/0xa0
[ 6186.381741][ T1] devprop_gpiochip_set_names.isra.0+0x4c/0x200
[ 6186.384394][ T1] gpiochip_add_data_with_key+0x75c/0xf80
[ 6186.386876][ T1] unittest_gpio_probe+0xf4/0x1e0
[ 6186.389049][ T1] platform_drv_probe+0xac/0x160
[ 6186.391184][ T1] really_probe+0x430/0xaa0
[ 6186.393136][ T1] really_probe_debug+0x3c/0xe0
[ 6186.395238][ T1] driver_probe_device+0x134/0x1c0
[ 6186.397443][ T1] device_driver_attach+0xec/0x180
[ 6186.399639][ T1] __driver_attach+0x1f0/0x220
[ 6186.401718][ T1] bus_for_each_dev+0x104/0x1c0
[ 6186.403796][ T1] driver_attach+0x44/0x60
[ 6186.405731][ T1] bus_add_driver+0x214/0x3c0
[ 6186.407745][ T1] driver_register+0x1a8/0x240
[ 6186.409835][ T1] __platform_driver_register+0x90/0xa0
[ 6186.412207][ T1] of_unittest_overlay_gpio+0x20c/0x7cc
[ 6186.414595][ T1] of_unittest_overlay+0x748/0x7c0
[ 6186.416810][ T1] of_unittest+0x148/0x184
[ 6186.418732][ T1] do_one_initcall+0xc4/0x280
[ 6186.420782][ T1] do_initcalls+0x148/0x1ac
[ 6186.422758][ T1] kernel_init_freeable+0x158/0x1a0
[ 6186.425023][ T1] kernel_init+0x24/0x1f0
[ 6186.426938][ T1] ret_from_fork+0x10/0x18
[ 6186.428894][ T1]
==================================================================
[ 6186.433241][ T1] Unable to handle kernel read from unreadable
memory at virtual address 0000000000000570
[ 6186.437207][ T1] Mem abort info:
[ 6186.438639][ T1] ESR = 0x96000004
[ 6186.440536][ T1] EC = 0x25: DABT (current EL), IL = 32 bits
[ 6186.442791][ T1] SET = 0, FnV = 0
[ 6186.444660][ T1] EA = 0, S1PTW = 0
[ 6186.446233][ T1] Data abort info:
[ 6186.447938][ T1] ISV = 0, ISS = 0x00000004
[ 6186.449749][ T1] CM = 0, WnR = 0
[ 6186.451222][ T1] [0000000000000570] user address but active_mm is swapper
[ 6186.454000][ T1] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[ 6186.456422][ T1] Modules linked in:
[ 6186.458232][ T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G B
W 5.9.0-rc5-next-20200915-00006-g104c8fe4916b #1
[ 6186.462833][ T1] Hardware name: linux,dummy-virt (DT)
[ 6186.465170][ T1] pstate: 60400005 (nZCv daif +PAN -UAO BTYPE=--)
[ 6186.467910][ T1] pc : device_property_read_string_array+0x40/0xa0
[ 6186.470653][ T1] lr : device_property_read_string_array+0x40/0xa0
[ 6186.473380][ T1] sp : ffff000069827770
[ 6186.475138][ T1] x29: ffff000069827770 x28: ffffa00014a2cc20
[ 6186.477806][ T1] x27: ffff000068794760 x26: ffff000068794800
[ 6186.480444][ T1] x25: ffff000068794000 x24: ffff0000674e1094
[ 6186.483107][ T1] x23: 0000000000000000 x22: 0000000000000000
[ 6186.485794][ T1] x21: ffffa00012d61ca0 x20: ffffa00012d61200
[ 6186.488457][ T1] x19: 0000000000000000 x18: 00000000000014b8
[ 6186.491100][ T1] x17: 00000000000014f8 x16: 0000000000001438
[ 6186.493779][ T1] x15: 00000000f1f1f1f1 x14: 0000000000000003
[ 6186.496405][ T1] x13: 00000000000ca688 x12: ffff80000d304e7b
[ 6186.499084][ T1] x11: 1fffe0000d304e7a x10: ffff80000d304e7a
[ 6186.501775][ T1] x9 : ffffa00012702b2c x8 : ffff0000698273d7
[ 6186.504409][ T1] x7 : 0000000000000001 x6 : 00007ffff2cfb186
[ 6186.507074][ T1] x5 : 0000000000000000 x4 : dfffa00000000000
[ 6186.509706][ T1] x3 : ffffa000126f85c4 x2 : 0000000000000007
[ 6186.512352][ T1] x1 : ffff00006981c040 x0 : 0000000000000001
[ 6186.515009][ T1] Call trace:
[ 6186.516511][ T1] device_property_read_string_array+0x40/0xa0
[ 6186.519155][ T1] devprop_gpiochip_set_names.isra.0+0x4c/0x200
[ 6186.521806][ T1] gpiochip_add_data_with_key+0x75c/0xf80
[ 6186.524294][ T1] unittest_gpio_probe+0xf4/0x1e0
[ 6186.526518][ T1] platform_drv_probe+0xac/0x160
[ 6186.528632][ T1] really_probe+0x430/0xaa0
[ 6186.530600][ T1] really_probe_debug+0x3c/0xe0
[ 6186.532679][ T1] driver_probe_device+0x134/0x1c0
[ 6186.534936][ T1] device_driver_attach+0xec/0x180
[ 6186.537119][ T1] __driver_attach+0x1f0/0x220
[ 6186.539182][ T1] bus_for_each_dev+0x104/0x1c0
[ 6186.541315][ T1] driver_attach+0x44/0x60
[ 6186.543233][ T1] bus_add_driver+0x214/0x3c0
[ 6186.545307][ T1] driver_register+0x1a8/0x240
[ 6186.547373][ T1] __platform_driver_register+0x90/0xa0
[ 6186.549754][ T1] of_unittest_overlay_gpio+0x20c/0x7cc
[ 6186.552105][ T1] of_unittest_overlay+0x748/0x7c0
[ 6186.554272][ T1] of_unittest+0x148/0x184
[ 6186.556193][ T1] do_one_initcall+0xc4/0x280
[ 6186.558248][ T1] do_initcalls+0x148/0x1ac
[ 6186.560227][ T1] kernel_init_freeable+0x158/0x1a0
[ 6186.562492][ T1] kernel_init+0x24/0x1f0
[ 6186.564395][ T1] ret_from_fork+0x10/0x18
[ 6186.566404][ T1] Code: aa0303f7 97b54003 9115c260 97c3ca39 (f942ba74)
[ 6186.569375][ T1] ---[ end trace f489669ae669dad0 ]---
[ 6186.571688][ T1] Kernel panic - not syncing: Oops: Fatal exception
[ 6186.574448][ T1] Kernel Offset: disabled
[ 6186.576306][ T1] CPU features: 0x0240002,20002004
[ 6186.578453][ T1] Memory Limit: none
[ 6186.580215][ T1] ---[ end Kernel panic - not syncing: Oops:
Fatal exception ]---
Cheers,
Anders
[1] http://ix.io/2xDy
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 0/3] gpiolib: generalize GPIO line names property
2020-09-15 12:01 ` [PATCH 0/3] gpiolib: generalize GPIO line names property Anders Roxell
@ 2020-09-15 13:12 ` Andy Shevchenko
2020-09-15 13:16 ` Bartosz Golaszewski
2020-09-15 13:16 ` Andy Shevchenko
2020-09-16 13:29 ` Bartosz Golaszewski
1 sibling, 2 replies; 14+ messages in thread
From: Andy Shevchenko @ 2020-09-15 13:12 UTC (permalink / raw)
To: Anders Roxell
Cc: Bartosz Golaszewski, Linus Walleij, Mika Westerberg, Kent Gibson,
Greg Kroah-Hartman, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, linux-acpi, Bartosz Golaszewski,
lkft-triage, Linux-Next Mailing List, Stephen Rothwell
On Tue, Sep 15, 2020 at 02:01:56PM +0200, Anders Roxell wrote:
> On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > I initially sent this as part of the gpio-mockup overhaul but since
> > these patches are indepentent and the work on gpio-mockup may become
> > more complicated - I'm sending these separately.
> >
> > The only change is adding additional property helpers to count strings
> > in array.
> >
> > Bartosz Golaszewski (3):
> > device: property: add helpers to count items in string arrays
> > gpiolib: generalize devprop_gpiochip_set_names() for device properties
> > gpiolib: unexport devprop_gpiochip_set_names()
Ha-ha, OF unittest is of_node centric. definitely there is no backed device.
Bart, it seems we are stuck with fwnode interface.
> [ 6186.379069][ T1] device_property_read_string_array+0x40/0xa0
> [ 6186.381741][ T1] devprop_gpiochip_set_names.isra.0+0x4c/0x200
> [ 6186.384394][ T1] gpiochip_add_data_with_key+0x75c/0xf80
> [ 6186.386876][ T1] unittest_gpio_probe+0xf4/0x1e0
> [ 6186.433241][ T1] Unable to handle kernel read from unreadable
> memory at virtual address 0000000000000570
> [ 6186.437207][ T1] Mem abort info:
> [ 6186.438639][ T1] ESR = 0x96000004
> [ 6186.440536][ T1] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 6186.442791][ T1] SET = 0, FnV = 0
> [ 6186.444660][ T1] EA = 0, S1PTW = 0
> [ 6186.446233][ T1] Data abort info:
> [ 6186.447938][ T1] ISV = 0, ISS = 0x00000004
> [ 6186.449749][ T1] CM = 0, WnR = 0
> [ 6186.451222][ T1] [0000000000000570] user address but active_mm is swapper
> [ 6186.454000][ T1] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [ 6186.456422][ T1] Modules linked in:
> [ 6186.458232][ T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G B
> W 5.9.0-rc5-next-20200915-00006-g104c8fe4916b #1
> [ 6186.462833][ T1] Hardware name: linux,dummy-virt (DT)
> [ 6186.465170][ T1] pstate: 60400005 (nZCv daif +PAN -UAO BTYPE=--)
> [ 6186.467910][ T1] pc : device_property_read_string_array+0x40/0xa0
> [ 6186.470653][ T1] lr : device_property_read_string_array+0x40/0xa0
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] gpiolib: generalize GPIO line names property
2020-09-15 13:12 ` Andy Shevchenko
@ 2020-09-15 13:16 ` Bartosz Golaszewski
2020-09-15 15:16 ` Andy Shevchenko
2020-09-15 13:16 ` Andy Shevchenko
1 sibling, 1 reply; 14+ messages in thread
From: Bartosz Golaszewski @ 2020-09-15 13:16 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Anders Roxell, Linus Walleij, Mika Westerberg, Kent Gibson,
Greg Kroah-Hartman, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, ACPI Devel Maling List,
Bartosz Golaszewski, lkft-triage, Linux-Next Mailing List,
Stephen Rothwell
On Tue, Sep 15, 2020 at 3:12 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Sep 15, 2020 at 02:01:56PM +0200, Anders Roxell wrote:
> > On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > I initially sent this as part of the gpio-mockup overhaul but since
> > > these patches are indepentent and the work on gpio-mockup may become
> > > more complicated - I'm sending these separately.
> > >
> > > The only change is adding additional property helpers to count strings
> > > in array.
> > >
> > > Bartosz Golaszewski (3):
> > > device: property: add helpers to count items in string arrays
> > > gpiolib: generalize devprop_gpiochip_set_names() for device properties
> > > gpiolib: unexport devprop_gpiochip_set_names()
>
> Ha-ha, OF unittest is of_node centric. definitely there is no backed device.
>
> Bart, it seems we are stuck with fwnode interface.
>
Wait what?! This means the implementation is wrong - the whole concept
of device properties is to be generic and to hide the underlying
fwnode or OF properties. If anything we should fix
device/base/property.c to fall back to OF.
What is happening exactly? If all fwnode code compiled out?
I'll try to give it a spin and see what can be done but I don't like
that device_property_* functions fail if you have OF but not fwnode.
Bart
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] gpiolib: generalize GPIO line names property
2020-09-15 13:16 ` Bartosz Golaszewski
@ 2020-09-15 15:16 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2020-09-15 15:16 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Anders Roxell, Linus Walleij, Mika Westerberg, Kent Gibson,
Greg Kroah-Hartman, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, ACPI Devel Maling List,
Bartosz Golaszewski, lkft-triage, Linux-Next Mailing List,
Stephen Rothwell
On Tue, Sep 15, 2020 at 03:16:42PM +0200, Bartosz Golaszewski wrote:
> On Tue, Sep 15, 2020 at 3:12 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Tue, Sep 15, 2020 at 02:01:56PM +0200, Anders Roxell wrote:
> > > On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > >
> > > > I initially sent this as part of the gpio-mockup overhaul but since
> > > > these patches are indepentent and the work on gpio-mockup may become
> > > > more complicated - I'm sending these separately.
> > > >
> > > > The only change is adding additional property helpers to count strings
> > > > in array.
> > > >
> > > > Bartosz Golaszewski (3):
> > > > device: property: add helpers to count items in string arrays
> > > > gpiolib: generalize devprop_gpiochip_set_names() for device properties
> > > > gpiolib: unexport devprop_gpiochip_set_names()
> >
> > Ha-ha, OF unittest is of_node centric. definitely there is no backed device.
> >
> > Bart, it seems we are stuck with fwnode interface.
> >
>
> Wait what?! This means the implementation is wrong - the whole concept
> of device properties is to be generic and to hide the underlying
> fwnode or OF properties. If anything we should fix
> device/base/property.c to fall back to OF.
>
> What is happening exactly? If all fwnode code compiled out?
>
> I'll try to give it a spin and see what can be done but I don't like
> that device_property_* functions fail if you have OF but not fwnode.
Read my next reply. It seems only OF unittest issue
% git grep -n -w fwnode -- drivers/of/unittest* | wc -l
0
% git grep -n -w fwnode -- drivers/of/ | wc -l
50
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] gpiolib: generalize GPIO line names property
2020-09-15 13:12 ` Andy Shevchenko
2020-09-15 13:16 ` Bartosz Golaszewski
@ 2020-09-15 13:16 ` Andy Shevchenko
2020-09-15 16:03 ` Bartosz Golaszewski
1 sibling, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2020-09-15 13:16 UTC (permalink / raw)
To: Anders Roxell
Cc: Bartosz Golaszewski, Linus Walleij, Mika Westerberg, Kent Gibson,
Greg Kroah-Hartman, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, linux-acpi, Bartosz Golaszewski,
lkft-triage, Linux-Next Mailing List, Stephen Rothwell
On Tue, Sep 15, 2020 at 04:12:28PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 15, 2020 at 02:01:56PM +0200, Anders Roxell wrote:
> > On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > I initially sent this as part of the gpio-mockup overhaul but since
> > > these patches are indepentent and the work on gpio-mockup may become
> > > more complicated - I'm sending these separately.
> > >
> > > The only change is adding additional property helpers to count strings
> > > in array.
> > >
> > > Bartosz Golaszewski (3):
> > > device: property: add helpers to count items in string arrays
> > > gpiolib: generalize devprop_gpiochip_set_names() for device properties
> > > gpiolib: unexport devprop_gpiochip_set_names()
>
> Ha-ha, OF unittest is of_node centric. definitely there is no backed device.
>
> Bart, it seems we are stuck with fwnode interface.
Hmm... There is a platform device. So, it means that it fails along these
lines:
return IS_ENABLED(CONFIG_OF) && dev->of_node ? &dev->of_node->fwnode : dev->fwnode;
so, who should set fwnode for of_node?
> > [ 6186.379069][ T1] device_property_read_string_array+0x40/0xa0
> > [ 6186.381741][ T1] devprop_gpiochip_set_names.isra.0+0x4c/0x200
> > [ 6186.384394][ T1] gpiochip_add_data_with_key+0x75c/0xf80
> > [ 6186.386876][ T1] unittest_gpio_probe+0xf4/0x1e0
>
> > [ 6186.433241][ T1] Unable to handle kernel read from unreadable
> > memory at virtual address 0000000000000570
> > [ 6186.437207][ T1] Mem abort info:
> > [ 6186.438639][ T1] ESR = 0x96000004
> > [ 6186.440536][ T1] EC = 0x25: DABT (current EL), IL = 32 bits
> > [ 6186.442791][ T1] SET = 0, FnV = 0
> > [ 6186.444660][ T1] EA = 0, S1PTW = 0
> > [ 6186.446233][ T1] Data abort info:
> > [ 6186.447938][ T1] ISV = 0, ISS = 0x00000004
> > [ 6186.449749][ T1] CM = 0, WnR = 0
> > [ 6186.451222][ T1] [0000000000000570] user address but active_mm is swapper
> > [ 6186.454000][ T1] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> > [ 6186.456422][ T1] Modules linked in:
> > [ 6186.458232][ T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G B
> > W 5.9.0-rc5-next-20200915-00006-g104c8fe4916b #1
> > [ 6186.462833][ T1] Hardware name: linux,dummy-virt (DT)
> > [ 6186.465170][ T1] pstate: 60400005 (nZCv daif +PAN -UAO BTYPE=--)
> > [ 6186.467910][ T1] pc : device_property_read_string_array+0x40/0xa0
> > [ 6186.470653][ T1] lr : device_property_read_string_array+0x40/0xa0
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] gpiolib: generalize GPIO line names property
2020-09-15 13:16 ` Andy Shevchenko
@ 2020-09-15 16:03 ` Bartosz Golaszewski
0 siblings, 0 replies; 14+ messages in thread
From: Bartosz Golaszewski @ 2020-09-15 16:03 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Anders Roxell, Bartosz Golaszewski, Linus Walleij,
Mika Westerberg, Kent Gibson, Greg Kroah-Hartman,
open list:GPIO SUBSYSTEM, Linux Kernel Mailing List,
ACPI Devel Maling List, lkft-triage, Linux-Next Mailing List,
Stephen Rothwell
On Tue, Sep 15, 2020 at 3:16 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Sep 15, 2020 at 04:12:28PM +0300, Andy Shevchenko wrote:
> > On Tue, Sep 15, 2020 at 02:01:56PM +0200, Anders Roxell wrote:
> > > On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > >
> > > > I initially sent this as part of the gpio-mockup overhaul but since
> > > > these patches are indepentent and the work on gpio-mockup may become
> > > > more complicated - I'm sending these separately.
> > > >
> > > > The only change is adding additional property helpers to count strings
> > > > in array.
> > > >
> > > > Bartosz Golaszewski (3):
> > > > device: property: add helpers to count items in string arrays
> > > > gpiolib: generalize devprop_gpiochip_set_names() for device properties
> > > > gpiolib: unexport devprop_gpiochip_set_names()
> >
> > Ha-ha, OF unittest is of_node centric. definitely there is no backed device.
> >
> > Bart, it seems we are stuck with fwnode interface.
>
> Hmm... There is a platform device. So, it means that it fails along these
> lines:
> return IS_ENABLED(CONFIG_OF) && dev->of_node ? &dev->of_node->fwnode : dev->fwnode;
> so, who should set fwnode for of_node?
>
It's strange because the device for this unittest is registered
similarly to how we do it in gpio-mockup where this function works.
I've not investigated the internals but somewhere someone sets the
fwnode for these platform devices.
Bartosz
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] gpiolib: generalize GPIO line names property
2020-09-15 12:01 ` [PATCH 0/3] gpiolib: generalize GPIO line names property Anders Roxell
2020-09-15 13:12 ` Andy Shevchenko
@ 2020-09-16 13:29 ` Bartosz Golaszewski
1 sibling, 0 replies; 14+ messages in thread
From: Bartosz Golaszewski @ 2020-09-16 13:29 UTC (permalink / raw)
To: Anders Roxell
Cc: Linus Walleij, Mika Westerberg, Andy Shevchenko, Kent Gibson,
Greg Kroah-Hartman, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, ACPI Devel Maling List,
Bartosz Golaszewski, lkft-triage, Linux-Next Mailing List,
Stephen Rothwell
On Tue, Sep 15, 2020 at 2:02 PM Anders Roxell <anders.roxell@linaro.org> wrote:
>
> On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > I initially sent this as part of the gpio-mockup overhaul but since
> > these patches are indepentent and the work on gpio-mockup may become
> > more complicated - I'm sending these separately.
> >
> > The only change is adding additional property helpers to count strings
> > in array.
> >
> > Bartosz Golaszewski (3):
> > device: property: add helpers to count items in string arrays
> > gpiolib: generalize devprop_gpiochip_set_names() for device properties
> > gpiolib: unexport devprop_gpiochip_set_names()
>
> I do an arm64 allmodconfig build fron linux-next (tag: next-20200915) and
> run that in qemu. When I run I see the following output (see full log [1]):
> "BUG: KASAN: null-ptr-deref in device_property_read_string_array".
>
>
FYI: this fails because someone passes a NULL struct device * to
dev_fwnode() - this is probably caused by some ordering issues in this
patch. I'm working on it.
Bartosz
^ permalink raw reply [flat|nested] 14+ messages in thread