Observations with AT91SAM9263-EK

[Sam Ravnborg]

Hi all.

I recently purchased an AT91SAM9263-EK from ebay and
have played around a little with barebox.
For now only some observations.

I had the impression that I could
drop AT91BootStrap when using barebox.

But I could not make it boot until I deployed
at91bootstrap (named BOOT.BIN) on the SD-card.
And I named arch/arm/pbl/zbarebox.bin => u-boot.bin
on the SD-card.

I wanted to boot barebox - but nothing happened.
So I tried older versions af barebox:

v2017.06.0 => Boots OK (did not try to load a kernel)

v2017.07.0 => Boots but emits:
NULL pointer dereference at address 0x00000014
### Please RESET the board ###
(A bit more was written to the serial console)

master from git => Nothing written to the console at all

I will as time permits dig deeper into this.
Seems like we are facing two bugs:
One that causes the NULL pointer, and another that
prevents any output.

As barebox is quick to build and the bug is simple to spot
I will likely just try to bisect and see where I end up.

Sam

[Jean-Christophe PLAGNIOL-VILLARD]

Post by Sam Ravnborg
Hi all.
I recently purchased an AT91SAM9263-EK from ebay and
have played around a little with barebox.
For now only some observations.
I had the impression that I could
drop AT91BootStrap when using barebox.

Yes Wrote the support for that but on the 9263 you are limited.

you have 2 choice boot a small barebox <= 72kiB

or use a console less barebox to boot the second barebox

as the rom code on at91 will load the barebox from the SD or Nand
into SRAM

Post by Sam Ravnborg
But I could not make it boot until I deployed
at91bootstrap (named BOOT.BIN) on the SD-card.
And I named arch/arm/pbl/zbarebox.bin => u-boot.bin
on the SD-card.

on 9263ek you have a nor flash so why not flash it

in this case barebox will boot by itself

without the bootstrap

Best Regards,
J.

Post by Sam Ravnborg
I wanted to boot barebox - but nothing happened.
v2017.06.0 => Boots OK (did not try to load a kernel)
NULL pointer dereference at address 0x00000014
### Please RESET the board ###
(A bit more was written to the serial console)
master from git => Nothing written to the console at all
I will as time permits dig deeper into this.
One that causes the NULL pointer, and another that
prevents any output.
As barebox is quick to build and the bug is simple to spot
I will likely just try to bisect and see where I end up.
Sam
_______________________________________________
barebox mailing list
http://lists.infradead.org/mailman/listinfo/barebox

[Sam Ravnborg]

Post by Jean-Christophe PLAGNIOL-VILLARD

Post by Sam Ravnborg
Hi all.
I recently purchased an AT91SAM9263-EK from ebay and
have played around a little with barebox.
For now only some observations.
I had the impression that I could
drop AT91BootStrap when using barebox.

Yes Wrote the support for that but on the 9263 you are limited.
you have 2 choice boot a small barebox <= 72kiB
or use a console less barebox to boot the second barebox
as the rom code on at91 will load the barebox from the SD or Nand
into SRAM

Thanks for the info.
I will try to collect this in a at91.rst file in Documentation/boards/
When there is something useful I will post a patch for review.

Post by Jean-Christophe PLAGNIOL-VILLARD

Post by Sam Ravnborg
But I could not make it boot until I deployed
at91bootstrap (named BOOT.BIN) on the SD-card.
And I named arch/arm/pbl/zbarebox.bin => u-boot.bin
on the SD-card.

on 9263ek you have a nor flash so why not flash it
in this case barebox will boot by itself
without the bootstrap

The EK only have NAND, the NOR is not mounted.
But anyway - the idea here is to have a testbed for my at91
patches that I can use before I send them upstream.

I have a proprietary at91sam9263 based board that
I will try to add support for in barebox.
But only the parts touching the atmel boards are relevant for the
upstream barebox.

Step 1 was to get barebox workign with the EK, and with
the two small patches I already sent it looks like I
had some luck there.

Sam

[Sam Ravnborg]

From feaf9f379a1100b3e56faa83e22360b25a5eb904 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <***@skov.dk>
Date: Mon, 3 Jul 2017 21:21:02 +0200
Subject: [PATCH 1/2] 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.

Fix this by modifying the logic to explicitly check for -EPROBE_DEFER
like we do in the kernel.

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..939b067cc 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) == -EPROBE_DEFER)
return clk;
}

[Uwe Kleine-König]

Hello Sam,

Post by Sam Ravnborg
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.
Fix this by modifying the logic to explicitly check for -EPROBE_DEFER
like we do in the kernel.
Fixes: 90f7eacb ("clk: let clk_get return errors from of_clk_get_by_name")
---
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..939b067cc 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) == -EPROBE_DEFER)

Hmm, I don't remember which was the failure I saw that lead me to create
this patch. Probably I wanted to see EPROBE_DEFER.

Looking into the tree of functions that can be called from
of_clk_get_by_name I didn't find a function that returns ENODEV.

But consider a clock provider that tries to give you the clock and
triggers a EIO or ETIMEOUT. IMHO this should be given to the caller
instead of continuing with clk_get_sys. So I suggest

- if (!IS_ERR(clk) || PTR_ERR(clk) != -ENODEV)
+ if (!IS_ERR(clk) || PTR_ERR(clk) != -ENOENT)

instead of your patch.

Best regards
Uwe

[Sam Ravnborg]

Hi Uwe.

Thanks for the quick feedback!

Post by Uwe Kleine-König
Looking into the tree of functions that can be called from
of_clk_get_by_name I didn't find a function that returns ENODEV.
But consider a clock provider that tries to give you the clock and
triggers a EIO or ETIMEOUT. IMHO this should be given to the caller
instead of continuing with clk_get_sys. So I suggest
- if (!IS_ERR(clk) || PTR_ERR(clk) != -ENODEV)
+ if (!IS_ERR(clk) || PTR_ERR(clk) != -ENOENT)
instead of your patch.

Makes good sense and it was only because the kernel did otherwise
I explicitly tested for -EPROBE_DEFER.
Will rework and send an updated patch later today.

Sam

[Sam Ravnborg]

From 500c564285890fd0c9c47dc68f7fe6bc916e4589 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <***@skov.dk>
Date: Mon, 3 Jul 2017 22:07:41 +0200
Subject: [PATCH 2/2] 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.

Fixes: 37e6bee7 ("gpiolib: Add support for GPIO "hog" nodes")
Cc: Alexander Kurz <***@blala.de>
Signed-off-by: Sam Ravnborg <***@ravnborg.org>
---


Alexander posted a patch to fix this problem earlier this week.
I only looked at it after I had cooked up this patch.

Not too happy about the approach here, but seems to me to be
the best way.

Also the two static functions in this file may be wrapped in
#ifdef CONFIG_OFTREE
#else
static int of_gpiochip_scan_hogs(struct gpio_chip *chip) { return 0; }
#endif
Or maybe moved to of/of_gpio.c?

So they do not waste binary size with no OFTREE support.
I could not from existing code base see what was the preferred
approach here, likely because I looked in the wrong places.

Sam



drivers/gpio/gpiolib.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a3e17ada0..2d0b778c8 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;