gpiolib: Fix of_node inconsistency

Message ID 1533548881-35174-1-git-send-email-biju.das@bp.renesas.com
State New
Headers show
Series
  • gpiolib: Fix of_node inconsistency
Related show

Commit Message

Biju Das Aug. 6, 2018, 9:48 a.m.
Some platforms are not setting of_node in the driver. On these platforms
defining gpio-reserved-ranges on device tree leads to kernel crash.

It is due to some parts of the gpio core relying on the driver to set up
of_node,while other parts do themselves.This inconsistent behaviour leads 
to a crash.

gpiochip_add_data_with_key() calls gpiochip_init_valid_mask() with of_node
as NULL. of_gpiochip_add() fills "of_node" and calls 
of_gpiochip_init_valid_mask().

The fix is to move the assignment to chip->of_node from of_gpiochip_add()
to gpiochip_add_data_with_key().

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 drivers/gpio/gpiolib-of.c | 3 ---
 drivers/gpio/gpiolib.c    | 2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

Comments

Linus Walleij Aug. 6, 2018, 10:57 a.m. | #1
On Mon, Aug 6, 2018 at 11:53 AM Biju Das <biju.das@bp.renesas.com> wrote:

> Some platforms are not setting of_node in the driver. On these platforms
> defining gpio-reserved-ranges on device tree leads to kernel crash.
>
> It is due to some parts of the gpio core relying on the driver to set up
> of_node,while other parts do themselves.This inconsistent behaviour leads
> to a crash.
>
> gpiochip_add_data_with_key() calls gpiochip_init_valid_mask() with of_node
> as NULL. of_gpiochip_add() fills "of_node" and calls
> of_gpiochip_init_valid_mask().
>
> The fix is to move the assignment to chip->of_node from of_gpiochip_add()
> to gpiochip_add_data_with_key().
>
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>

After reading through context this seems like the right fix so patch applied!

In the long run I want to get rid of this extra of_node in the chip, but it
kind of requires every driver in the kernel to pass in a valid parent device
representing the OF node. And there are so many hairy corner cases
where they don't.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven Aug. 7, 2018, 7:49 a.m. | #2
On Mon, Aug 6, 2018 at 11:54 AM Biju Das <biju.das@bp.renesas.com> wrote:
> Some platforms are not setting of_node in the driver. On these platforms
> defining gpio-reserved-ranges on device tree leads to kernel crash.
>
> It is due to some parts of the gpio core relying on the driver to set up
> of_node,while other parts do themselves.This inconsistent behaviour leads
> to a crash.
>
> gpiochip_add_data_with_key() calls gpiochip_init_valid_mask() with of_node
> as NULL. of_gpiochip_add() fills "of_node" and calls
> of_gpiochip_init_valid_mask().
>
> The fix is to move the assignment to chip->of_node from of_gpiochip_add()
> to gpiochip_add_data_with_key().
>
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

Patch

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 28d9680..91174bf 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -620,9 +620,6 @@  int of_gpiochip_add(struct gpio_chip *chip)
 {
 	int status;
 
-	if ((!chip->of_node) && (chip->parent))
-		chip->of_node = chip->parent->of_node;
-
 	if (!chip->of_node)
 		return 0;
 
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e11a3bb..20fe531 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1256,6 +1256,8 @@  int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
 	/* If the gpiochip has an assigned OF node this takes precedence */
 	if (chip->of_node)
 		gdev->dev.of_node = chip->of_node;
+	else
+		chip->of_node = gdev->dev.of_node;
 #endif
 
 	gdev->id = ida_simple_get(&gpio_ida, 0, 0, GFP_KERNEL);