2020-09-08 16:41:50

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH 0/3] gpiolib: generalize GPIO line names property

From: Bartosz Golaszewski <[email protected]>

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()

drivers/gpio/Makefile | 1 -
drivers/gpio/gpiolib-acpi.c | 3 --
drivers/gpio/gpiolib-devprop.c | 20 ++++++-------
drivers/gpio/gpiolib-of.c | 5 ----
drivers/gpio/gpiolib.c | 55 +++++++++++++++++++++++++++++++---
include/linux/gpio/driver.h | 3 --
include/linux/property.h | 13 ++++++++
7 files changed, 74 insertions(+), 26 deletions(-)

--
2.26.1


2020-09-08 20:07:38

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH 1/3] device: property: add helpers to count items in string arrays

From: Bartosz Golaszewski <[email protected]>

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 <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
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

2020-09-08 20:08:59

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH 2/3] gpiolib: generalize devprop_gpiochip_set_names() for device properties

From: Bartosz Golaszewski <[email protected]>

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 <[email protected]>
---
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

2020-09-15 12:21:10

by Anders Roxell

[permalink] [raw]
Subject: Re: [PATCH 0/3] gpiolib: generalize GPIO line names property

On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <[email protected]> wrote:
>
> From: Bartosz Golaszewski <[email protected]>
>
> 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

2020-09-15 13:25:33

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH 0/3] gpiolib: generalize GPIO line names property

On Tue, Sep 15, 2020 at 3:12 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Tue, Sep 15, 2020 at 02:01:56PM +0200, Anders Roxell wrote:
> > On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <[email protected]> wrote:
> > >
> > > From: Bartosz Golaszewski <[email protected]>
> > >
> > > 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

2020-09-15 13:27:37

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 0/3] gpiolib: generalize GPIO line names property

On Tue, Sep 15, 2020 at 02:01:56PM +0200, Anders Roxell wrote:
> On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <[email protected]> wrote:
> >
> > From: Bartosz Golaszewski <[email protected]>
> >
> > 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


2020-09-15 13:43:57

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 0/3] gpiolib: generalize GPIO line names property

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 <[email protected]> wrote:
> > >
> > > From: Bartosz Golaszewski <[email protected]>
> > >
> > > 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


2020-09-15 16:07:26

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 0/3] gpiolib: generalize GPIO line names property

On Tue, Sep 15, 2020 at 03:16:42PM +0200, Bartosz Golaszewski wrote:
> On Tue, Sep 15, 2020 at 3:12 PM Andy Shevchenko
> <[email protected]> wrote:
> >
> > On Tue, Sep 15, 2020 at 02:01:56PM +0200, Anders Roxell wrote:
> > > On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <[email protected]> wrote:
> > > >
> > > > From: Bartosz Golaszewski <[email protected]>
> > > >
> > > > 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


2020-09-15 22:23:56

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH 0/3] gpiolib: generalize GPIO line names property

On Tue, Sep 15, 2020 at 3:16 PM Andy Shevchenko
<[email protected]> 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 <[email protected]> wrote:
> > > >
> > > > From: Bartosz Golaszewski <[email protected]>
> > > >
> > > > 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

2020-09-16 20:25:59

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH 0/3] gpiolib: generalize GPIO line names property

On Tue, Sep 15, 2020 at 2:02 PM Anders Roxell <[email protected]> wrote:
>
> On Tue, 8 Sep 2020 at 18:40, Bartosz Golaszewski <[email protected]> wrote:
> >
> > From: Bartosz Golaszewski <[email protected]>
> >
> > 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