UPGRADE YOUR BROWSER

We have detected your current browser version is not the latest one. Xilinx.com uses the latest web technologies to bring you the best online experience possible. Please upgrade to a Xilinx.com supported browser:Chrome, Firefox, Internet Explorer 11, Safari. Thank you!

cancel
Showing results for 
Search instead for 
Did you mean: 
Scholar wzab
Scholar
383 Views
Registered: ‎08-24-2011

Bug in of_get_named_gpiod_flags function in the kernel - random switching of OPEN DRAIN or OPEN SOURCE property in GPIOs

Jump to solution

Hi,

Last time we have ported our project utilizing my multi-gpio driver with Xilinx AXI GPIO from Vivado 2016.4 to Vivado 2017.4.

We were surprised, that certain GPIO lines were treated as OPEN SOURCE.

After thorough investigation of the kernel code, I have found that the reason of the problem is as follows:

 

The function of_find_gpio declares the of_flags variable, but does not initialize it. It is later on passed through of_get_named_gpiod_flags and of_xlate_and_get_gpiod_flags finally to the xgpio_xlate which also leaves it unmodified.

So the random value from the stack is treated as flag, randomly switching the OPEN DRAIN or OPEN SOURCE mode.

 

The problem was not present in 2016.4, because its version of of_get_named_gpiod_flags was prepared for such problem:

 

/* .of_xlate might decide to not fill in the flags, so clear it. */
if (flags)
    *flags = 0;

Unfortunaly that code disappeared somewhere between 2016.4 and 2017.4

Quick look at the 2018.2 version of of_get_named_gpiod_flags suggests, that it may also suffer from the same bug.

 

 

0 Kudos
1 Solution

Accepted Solutions
Scholar wzab
Scholar
287 Views
Registered: ‎08-24-2011

Re: Bug in of_get_named_gpiod_flags function in the kernel - random switching of OPEN DRAIN or OPEN SOURCE property in GPIOs

Jump to solution

I've reported the problem to the LKML. However, the final statement was, that it is the fault of the driver if it does not initialize the flags.

The correct solution would be then to modify the xilinx GPIO driver:

static int xgpio_xlate(struct gpio_chip *gc,
		       const struct of_phandle_args *gpiospec, u32 *flags)
{
	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
	struct xgpio_instance *chip = container_of(mm_gc, struct xgpio_instance,
						   mmchip);
        if(flags)
              * flags = 0;

	if (gpiospec->args[1] == chip->offset)
		return gpiospec->args[0];

	return -EINVAL;
}

 So the correct patch meta-user/recipes-kernel/linux/linux-xlnx/0001-gpio-xilinx-xgpio-xlate-fix.patch should look like below:

diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
index 72fb3cd..1e50dd5 100644 --- a/drivers/gpio/gpio-xilinx.c +++ b/drivers/gpio/gpio-xilinx.c @@ -261,10 +261,10 @@ static int xgpio_xlate(struct gpio_chip *gc, struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); struct xgpio_instance *chip = container_of(mm_gc, struct xgpio_instance, mmchip); - + if (flags) + *flags = 0; if (gpiospec->args[1] == chip->offset) return gpiospec->args[0]; - return -EINVAL; }

 and meta-user/recipes-kernel/linux/linux-xlnx_%.bbappend should be modified as follows (of course your file may be different, depending on the configuration. It is important that you add the line with patch description).

SRC_URI += "file://bsp.cfg \
SRC_URI +=  file://0001-gpio-xilinx-xgpio-xlate-fix.patch \
           "

FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:""
0 Kudos
2 Replies
Scholar wzab
Scholar
332 Views
Registered: ‎08-24-2011

Re: Bug in of_get_named_gpiod_flags function in the kernel - random switching of OPEN DRAIN or OPEN SOURCE property in GPIOs

Jump to solution

The following modifications solve the problem for 2017.4 (probably similar solution should be possible for newer version as well):

The following patch should be added (as meta-user/recipes-kernel/linux/linux-xlnx/0001-of-get-named-gpiod-flags-fix.patch):

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 193f15d5..a9b9cfc3 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -75,6 +75,10 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
 	struct gpio_desc *desc;
 	int ret;
 
+	/* .of_xlate might decide to not fill in the flags, so clear it. */
+	if (flags)
+		*flags = 0;
+
 	ret = of_parse_phandle_with_args(np, propname, "#gpio-cells", index,
 					 &gpiospec);
 	if (ret) {

The meta-user/recipes-kernel/linux/linux-xlnx_%.bbappend should be modified to use this patch. Below is just a template (your file may already contain references to different kernel configuration changes):

 

SRC_URI += "file://bsp.cfg \
SRC_URI +=  file://0001-of-get-named-gpiod-flags-fix.patch \
           "

FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:""

 

0 Kudos
Scholar wzab
Scholar
288 Views
Registered: ‎08-24-2011

Re: Bug in of_get_named_gpiod_flags function in the kernel - random switching of OPEN DRAIN or OPEN SOURCE property in GPIOs

Jump to solution

I've reported the problem to the LKML. However, the final statement was, that it is the fault of the driver if it does not initialize the flags.

The correct solution would be then to modify the xilinx GPIO driver:

static int xgpio_xlate(struct gpio_chip *gc,
		       const struct of_phandle_args *gpiospec, u32 *flags)
{
	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
	struct xgpio_instance *chip = container_of(mm_gc, struct xgpio_instance,
						   mmchip);
        if(flags)
              * flags = 0;

	if (gpiospec->args[1] == chip->offset)
		return gpiospec->args[0];

	return -EINVAL;
}

 So the correct patch meta-user/recipes-kernel/linux/linux-xlnx/0001-gpio-xilinx-xgpio-xlate-fix.patch should look like below:

diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
index 72fb3cd..1e50dd5 100644 --- a/drivers/gpio/gpio-xilinx.c +++ b/drivers/gpio/gpio-xilinx.c @@ -261,10 +261,10 @@ static int xgpio_xlate(struct gpio_chip *gc, struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); struct xgpio_instance *chip = container_of(mm_gc, struct xgpio_instance, mmchip); - + if (flags) + *flags = 0; if (gpiospec->args[1] == chip->offset) return gpiospec->args[0]; - return -EINVAL; }

 and meta-user/recipes-kernel/linux/linux-xlnx_%.bbappend should be modified as follows (of course your file may be different, depending on the configuration. It is important that you add the line with patch description).

SRC_URI += "file://bsp.cfg \
SRC_URI +=  file://0001-gpio-xilinx-xgpio-xlate-fix.patch \
           "

FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:""
0 Kudos