Discussion:
[PATCH v2 0/3] diverse at91 related fixes
Sam Ravnborg
2017-07-05 18:27:47 UTC
Permalink
This is a rework of the two patches sent earlier + an extra patch.
With these patches my AT91SAM9263EK board boots barebox.

clk: fix clk_get error handling
- changed based on feedback from Uwe, test for -ENOENT

gpiolib: fix: do not access oftree on non-OFDEVICE boards
- rebased on top of Alexenaders original fix

And a third patch that just adds a missing static (found while browsing code)

diffstat:

arch/arm/mach-at91/bootstrap.c | 2 +-
drivers/clk/clkdev.c | 2 +-
drivers/gpio/gpiolib.c | 8 +++++++-
3 files changed, 9 insertions(+), 3 deletions(-)

Sam
Sam Ravnborg
2017-07-05 18:32:35 UTC
Permalink
From 20397009e13e6111c8b17e5cbaf0addf3aaf593a Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <***@skov.dk>
Date: Mon, 3 Jul 2017 21:21:02 +0200
Subject: [PATCH 1/3] clk: fix clk_get error handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If there is no OFTREE support of_clk_get_by_name failed with
-ENOENT, which caused clk_get to bail out.
This had the effect that nothing was printed on the serial console
with at91sam9263-ek.

There are no error paths that will return -ENODEV as we test for today,
so change this to -ENOENT which is in use.
This allows us to contine with clk_get_sys() in case of other
errors as was the intention of the original fix.

Fixes: 90f7eacb ("clk: let clk_get return errors from of_clk_get_by_name")
Cc: Uwe Kleine-König <u.kleine-***@pengutronix.de>
Signed-off-by: Sam Ravnborg <***@ravnborg.org>
---
drivers/clk/clkdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 6b1666355..abdc41527 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -181,7 +181,7 @@ struct clk *clk_get(struct device_d *dev, const char *con_id)

if (dev) {
clk = of_clk_get_by_name(dev->device_node, con_id);
- if (!IS_ERR(clk) || PTR_ERR(clk) != -ENODEV)
+ if (!IS_ERR(clk) || PTR_ERR(clk) != -ENOENT)
return clk;
}
--
2.12.0
Sam Ravnborg
2017-07-05 18:33:15 UTC
Permalink
From d10f426e1b8cec7de257dabf59e7fe53a591b3c1 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <***@skov.dk>
Date: Mon, 3 Jul 2017 22:07:41 +0200
Subject: [PATCH 2/3] gpio: fix null pointer exception when there is no oftree

In a system with oftree support enabled but with no oftree the
of_gpiochip_scan_hogs() would fail due to device_node equals NULL.

Check device_node and return with 0 in this situation, as this
mirrors what would have happened before we added support for gpio-hogs.

Use IS_ENABLED(CONFIG_OFDEVICE) to teach compiler to leave
out the of_* specific functions if not needed.

Fixes: 37e6bee7 ("gpiolib: Add support for GPIO "hog" nodes")
Signed-off-by: Alexander Kurz <***@blala.de>
Signed-off-by: Sam Ravnborg <***@ravnborg.org>
---
drivers/gpio/gpiolib.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a3e17ada0..2bd8ef2a8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -379,6 +379,9 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
struct device_node *np;
int ret, i;

+ if (!chip->dev->device_node)
+ return 0;
+
for_each_available_child_of_node(chip->dev->device_node, np) {
if (!of_property_read_bool(np, "gpio-hog"))
continue;
@@ -416,7 +419,10 @@ int gpiochip_add(struct gpio_chip *chip)
for (i = chip->base; i < chip->base + chip->ngpio; i++)
gpio_desc[i].chip = chip;

- return of_gpiochip_scan_hogs(chip);
+ if (IS_ENABLED(CONFIG_OFDEVICE))
+ return of_gpiochip_scan_hogs(chip);
+ else
+ return 0;
}

void gpiochip_remove(struct gpio_chip *chip)
--
2.12.0
Lucas Stach
2017-07-06 12:46:20 UTC
Permalink
Post by Sam Ravnborg
From d10f426e1b8cec7de257dabf59e7fe53a591b3c1 Mon Sep 17 00:00:00 2001
Date: Mon, 3 Jul 2017 22:07:41 +0200
Subject: [PATCH 2/3] gpio: fix null pointer exception when there is no oftree
In a system with oftree support enabled but with no oftree the
of_gpiochip_scan_hogs() would fail due to device_node equals NULL.
Check device_node and return with 0 in this situation, as this
mirrors what would have happened before we added support for gpio-hogs.
Use IS_ENABLED(CONFIG_OFDEVICE) to teach compiler to leave
out the of_* specific functions if not needed.
Fixes: 37e6bee7 ("gpiolib: Add support for GPIO "hog" nodes")
---
drivers/gpio/gpiolib.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a3e17ada0..2bd8ef2a8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -379,6 +379,9 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
struct device_node *np;
int ret, i;
+ if (!chip->dev->device_node)
+ return 0;
+
for_each_available_child_of_node(chip->dev->device_node, np) {
if (!of_property_read_bool(np, "gpio-hog"))
continue;
@@ -416,7 +419,10 @@ int gpiochip_add(struct gpio_chip *chip)
for (i = chip->base; i < chip->base + chip->ngpio; i++)
gpio_desc[i].chip = chip;
- return of_gpiochip_scan_hogs(chip);
+ if (IS_ENABLED(CONFIG_OFDEVICE))
+ return of_gpiochip_scan_hogs(chip);
+ else
+ return 0;
}
void gpiochip_remove(struct gpio_chip *chip)
I think this can be simplified to:

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a3e17ada0d39..1a373ef149a5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -379,6 +379,9 @@ static int of_gpiochip_scan_hogs(struct gpio_chip
*chip)
struct device_node *np;
int ret, i;

+ if (!IS_ENABLED(CONFIG_OFDEVICE) || !chip->dev->device_node)
+ return 0;
+
for_each_available_child_of_node(chip->dev->device_node, np) {
if (!of_property_read_bool(np, "gpio-hog"))
continue;

The optimizer should be able to work this out. Can you check that this
works for you? No need to resend, if it works I'll just commit this.

Regards,
Lucas
Alexander Kurz
2017-07-06 20:53:38 UTC
Permalink
Post by Sam Ravnborg
Post by Sam Ravnborg
From d10f426e1b8cec7de257dabf59e7fe53a591b3c1 Mon Sep 17 00:00:00 2001
Date: Mon, 3 Jul 2017 22:07:41 +0200
Subject: [PATCH 2/3] gpio: fix null pointer exception when there is no oftree
In a system with oftree support enabled but with no oftree the
of_gpiochip_scan_hogs() would fail due to device_node equals NULL.
Check device_node and return with 0 in this situation, as this
mirrors what would have happened before we added support for gpio-hogs.
Use IS_ENABLED(CONFIG_OFDEVICE) to teach compiler to leave
out the of_* specific functions if not needed.
Fixes: 37e6bee7 ("gpiolib: Add support for GPIO "hog" nodes")
---
drivers/gpio/gpiolib.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a3e17ada0..2bd8ef2a8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -379,6 +379,9 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
struct device_node *np;
int ret, i;
+ if (!chip->dev->device_node)
+ return 0;
+
for_each_available_child_of_node(chip->dev->device_node, np) {
if (!of_property_read_bool(np, "gpio-hog"))
continue;
@@ -416,7 +419,10 @@ int gpiochip_add(struct gpio_chip *chip)
for (i = chip->base; i < chip->base + chip->ngpio; i++)
gpio_desc[i].chip = chip;
- return of_gpiochip_scan_hogs(chip);
+ if (IS_ENABLED(CONFIG_OFDEVICE))
+ return of_gpiochip_scan_hogs(chip);
+ else
+ return 0;
}
void gpiochip_remove(struct gpio_chip *chip)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a3e17ada0d39..1a373ef149a5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -379,6 +379,9 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
struct device_node *np;
int ret, i;
+ if (!IS_ENABLED(CONFIG_OFDEVICE) || !chip->dev->device_node)
+ return 0;
+
for_each_available_child_of_node(chip->dev->device_node, np) {
if (!of_property_read_bool(np, "gpio-hog"))
continue;
The optimizer should be able to work this out. Can you check that this
works for you? No need to resend, if it works I'll just commit this.
This works perfecty fine with gcc-5.2 and gcc-4.6
(although gcc-4.6 compatibilty broke with commit 4ef026c3048)

Thanks, Alexander
Post by Sam Ravnborg
Regards,
Lucas
Lucas Stach
2017-07-07 12:42:51 UTC
Permalink
Post by Alexander Kurz
Post by Sam Ravnborg
Post by Sam Ravnborg
From d10f426e1b8cec7de257dabf59e7fe53a591b3c1 Mon Sep 17 00:00:00 2001
Date: Mon, 3 Jul 2017 22:07:41 +0200
Subject: [PATCH 2/3] gpio: fix null pointer exception when there is no oftree
In a system with oftree support enabled but with no oftree the
of_gpiochip_scan_hogs() would fail due to device_node equals NULL.
Check device_node and return with 0 in this situation, as this
mirrors what would have happened before we added support for gpio-hogs.
Use IS_ENABLED(CONFIG_OFDEVICE) to teach compiler to leave
out the of_* specific functions if not needed.
Fixes: 37e6bee7 ("gpiolib: Add support for GPIO "hog" nodes")
---
drivers/gpio/gpiolib.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a3e17ada0..2bd8ef2a8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -379,6 +379,9 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
struct device_node *np;
int ret, i;
+ if (!chip->dev->device_node)
+ return 0;
+
for_each_available_child_of_node(chip->dev->device_node, np) {
if (!of_property_read_bool(np, "gpio-hog"))
continue;
@@ -416,7 +419,10 @@ int gpiochip_add(struct gpio_chip *chip)
for (i = chip->base; i < chip->base + chip->ngpio; i++)
gpio_desc[i].chip = chip;
- return of_gpiochip_scan_hogs(chip);
+ if (IS_ENABLED(CONFIG_OFDEVICE))
+ return of_gpiochip_scan_hogs(chip);
+ else
+ return 0;
}
void gpiochip_remove(struct gpio_chip *chip)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a3e17ada0d39..1a373ef149a5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -379,6 +379,9 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
struct device_node *np;
int ret, i;
+ if (!IS_ENABLED(CONFIG_OFDEVICE) || !chip->dev->device_node)
+ return 0;
+
for_each_available_child_of_node(chip->dev->device_node, np) {
if (!of_property_read_bool(np, "gpio-hog"))
continue;
The optimizer should be able to work this out. Can you check that this
works for you? No need to resend, if it works I'll just commit this.
This works perfecty fine with gcc-5.2 and gcc-4.6
(although gcc-4.6 compatibilty broke with commit 4ef026c3048)
Thanks, all. I've pushed the reworked patch together with 1/3 to master.
Patch 3/3 is applied for -next.

Regards,
Lucas

Sam Ravnborg
2017-07-05 18:34:00 UTC
Permalink
From 8eb65eccbdd304ba4fb06e8de2a958791e0077f0 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <***@skov.dk>
Date: Wed, 5 Jul 2017 08:52:19 +0200
Subject: [PATCH 3/3] arm: at91 bootstrap: declare local function static

Function is only used in this file and no prototype exist
in any header.

Signed-off-by: Sam Ravnborg <***@ravnborg.org>
---
arch/arm/mach-at91/bootstrap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-at91/bootstrap.c b/arch/arm/mach-at91/bootstrap.c
index 9dd575b74..5d21b2d02 100644
--- a/arch/arm/mach-at91/bootstrap.c
+++ b/arch/arm/mach-at91/bootstrap.c
@@ -149,7 +149,7 @@ static void boot_reset_action(struct menu *m, struct menu_entry *me)
restart_machine();
}

-void at91_bootstrap_menu(void)
+static void at91_bootstrap_menu(void)
{
struct menu *m;
struct menu_entry *me;
--
2.12.0
Loading...