[PATCH 0/2] J2 Turtle Board fixes

[PATCH 0/2] J2 Turtle Board fixes

[Artur Rojek]

Hi all,

this series fixes boot issues and allows J2 Turtle Board to boot
upstream Linux again.

Patch [1/2] enforces 8-byte alignment for the dtb offset.

Patch [2/2] resolves a problem with PIT interrupts failing to register.

Even with the above fixes, Turtle Board is prone to occasional freezes
related to clock source transition from periodic to hrtimers. I however
decided to send those two patches ahead and debug the third issue at a
later time. 

Cheers,
Artur

Artur Rojek (2):
  sh: align .bss section padding to 8-byte boundary
  irqchip: clocksource: fix jcore-pit irq request

 arch/sh/kernel/vmlinux.lds.S    | 15 ++++++++++++++-
 drivers/clocksource/jcore-pit.c | 15 ++++++++++++++-
 drivers/irqchip/irq-jcore-aic.c |  2 +-
 3 files changed, 29 insertions(+), 3 deletions(-)

-- 
2.48.1

[PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[Artur Rojek]

J2 based devices expect to find a devicetree blob at the end of the bss
section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
dtb, causing J2 devices to fail early in sh_fdt_init.

As J2 loader firmware calculates the dtb location based on the kernel
image .bss section size, rather than the __bss_stop symbol offset, the
required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE, 8).
Instead, inline modified version of the above macro, which grows .bss
by the required size.

While this change affects all existing SH boards, it should be benign on
platforms which don't need this alignment.

Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---
 arch/sh/kernel/vmlinux.lds.S | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/sh/kernel/vmlinux.lds.S b/arch/sh/kernel/vmlinux.lds.S
index 9644fe187a3f..008c30289eaa 100644
--- a/arch/sh/kernel/vmlinux.lds.S
+++ b/arch/sh/kernel/vmlinux.lds.S
@@ -71,7 +71,20 @@ SECTIONS
 
 	. = ALIGN(PAGE_SIZE);
 	__init_end = .;
-	BSS_SECTION(0, PAGE_SIZE, 4)
+	__bss_start = .;
+	SBSS(0)
+	. = ALIGN(PAGE_SIZE);
+	.bss : AT(ADDR(.bss) - LOAD_OFFSET) {
+		BSS_FIRST_SECTIONS
+		. = ALIGN(PAGE_SIZE);
+		*(.bss..page_aligned)
+		. = ALIGN(PAGE_SIZE);
+		*(.dynbss)
+		*(BSS_MAIN)
+		*(COMMON)
+		. = ALIGN(8);
+	}
+	__bss_stop = .;
 	_end = . ;
 
 	STABS_DEBUG
-- 
2.48.1

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[Rob Landley]

On 2/16/25 11:55, Artur Rojek wrote:
> J2 based devices expect to find a devicetree blob at the end of the bss
> section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
> dtb, causing J2 devices to fail early in sh_fdt_init.
> 
> As J2 loader firmware calculates the dtb location based on the kernel
> image .bss section size, rather than the __bss_stop symbol offset, the
> required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE, 8).
> Instead, inline modified version of the above macro, which grows .bss
> by the required size.
> 
> While this change affects all existing SH boards, it should be benign on
> platforms which don't need this alignment.
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>

Tested-by: Rob Landley <rob@landley.net>

Rob

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[John Paul Adrian Glaubitz]

Hi Artur,

I'm currently trying to review this patch, but I'm not 100% sure how it
this change helps grows the .bss section, see below. Maybe you can help
me understand what's happening.

On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
> J2 based devices expect to find a devicetree blob at the end of the bss
> section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
> dtb, causing J2 devices to fail early in sh_fdt_init.
> 
> As J2 loader firmware calculates the dtb location based on the kernel
> image .bss section size, rather than the __bss_stop symbol offset, the
> required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE, 8).
> Instead, inline modified version of the above macro, which grows .bss
> by the required size.
> 
> While this change affects all existing SH boards, it should be benign on
> platforms which don't need this alignment.
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
>  arch/sh/kernel/vmlinux.lds.S | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/sh/kernel/vmlinux.lds.S b/arch/sh/kernel/vmlinux.lds.S
> index 9644fe187a3f..008c30289eaa 100644
> --- a/arch/sh/kernel/vmlinux.lds.S
> +++ b/arch/sh/kernel/vmlinux.lds.S
> @@ -71,7 +71,20 @@ SECTIONS
>  
>  	. = ALIGN(PAGE_SIZE);
>  	__init_end = .;
> -	BSS_SECTION(0, PAGE_SIZE, 4)
> +	__bss_start = .;
> +	SBSS(0)
> +	. = ALIGN(PAGE_SIZE);

What this effectively does is removing ". = ALIGN(sbss_align);" first from BSS_SECTION().

Then it inserts ". = ALIGN(PAGE_SIZE);" after the "SBSS(0)".

If I understand this correctly, SBSS() inserts a zero-padding and if I'm not mistaken,
inserting ". = ALIGN(PAGE_SIZE);" will cause this padding to grow to at least PAGE_SIZE
due the alignment.

Is this correct?

> +	.bss : AT(ADDR(.bss) - LOAD_OFFSET) {
> +		BSS_FIRST_SECTIONS
> +		. = ALIGN(PAGE_SIZE);
> +		*(.bss..page_aligned)
> +		. = ALIGN(PAGE_SIZE);
> +		*(.dynbss)
> +		*(BSS_MAIN)
> +		*(COMMON)
> +		. = ALIGN(8);

If my understanding above is correct, why do we will need an additional ". = ALIGN(8)"
here?

> +	}
> +	__bss_stop = .;
>  	_end = . ;
>  
>  	STABS_DEBUG

Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[Artur Rojek]

On 2025-03-11 18:28, John Paul Adrian Glaubitz wrote:
> Hi Artur,

Hey Adrian,
thanks for looking into this patch.

> 
> I'm currently trying to review this patch, but I'm not 100% sure how it
> this change helps grows the .bss section, see below. Maybe you can help
> me understand what's happening.
> 
> On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
>> J2 based devices expect to find a devicetree blob at the end of the 
>> bss
>> section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
>> dtb, causing J2 devices to fail early in sh_fdt_init.
>> 
>> As J2 loader firmware calculates the dtb location based on the kernel
>> image .bss section size, rather than the __bss_stop symbol offset, the
>> required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE, 
>> 8).
>> Instead, inline modified version of the above macro, which grows .bss
>> by the required size.
>> 
>> While this change affects all existing SH boards, it should be benign 
>> on
>> platforms which don't need this alignment.
>> 
>> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
>> ---
>>  arch/sh/kernel/vmlinux.lds.S | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/sh/kernel/vmlinux.lds.S 
>> b/arch/sh/kernel/vmlinux.lds.S
>> index 9644fe187a3f..008c30289eaa 100644
>> --- a/arch/sh/kernel/vmlinux.lds.S
>> +++ b/arch/sh/kernel/vmlinux.lds.S
>> @@ -71,7 +71,20 @@ SECTIONS
>> 
>>  	. = ALIGN(PAGE_SIZE);
>>  	__init_end = .;
>> -	BSS_SECTION(0, PAGE_SIZE, 4)
>> +	__bss_start = .;
>> +	SBSS(0)
>> +	. = ALIGN(PAGE_SIZE);
> 
> What this effectively does is removing ". = ALIGN(sbss_align);" first 
> from BSS_SECTION().
> 
> Then it inserts ". = ALIGN(PAGE_SIZE);" after the "SBSS(0)".
> 
> If I understand this correctly, SBSS() inserts a zero-padding and if 
> I'm not mistaken,
> inserting ". = ALIGN(PAGE_SIZE);" will cause this padding to grow to at 
> least PAGE_SIZE
> due the alignment.
> 
> Is this correct?
> 
>> +	.bss : AT(ADDR(.bss) - LOAD_OFFSET) {
>> +		BSS_FIRST_SECTIONS
>> +		. = ALIGN(PAGE_SIZE);
>> +		*(.bss..page_aligned)
>> +		. = ALIGN(PAGE_SIZE);
>> +		*(.dynbss)
>> +		*(BSS_MAIN)
>> +		*(COMMON)
>> +		. = ALIGN(8);
> 
> If my understanding above is correct, why do we will need an additional 
> ". = ALIGN(8)"
> here?

I'll tackle both of the above questions at once.
I'm by no means an expert at GNU Linker syntax, but the intention of
this patch is to put . = ALIGN(8) inside the .bss : { ... } section
definition, so that the section itself grows by the requested padding.

In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts
a 4 byte padding after the closing brace of .bss section definition,
causing the __bss_stop symbol offset to grow, but not the .bss section
itself:

#define BSS_SECTION(sbss_align, bss_align, stop_align)			\
	. = ALIGN(sbss_align);						\
	__bss_start = .;						\
	SBSS(sbss_align)						\
	BSS(bss_align)							\
	. = ALIGN(stop_align);						\
	__bss_stop = .;

TurtleBoard loader is only concerned with the .bss section size - it
doesn't care about any symbol offsets - and hence this seemingly cryptic
change (you can display the section size information with
readelf -t kernel_image).
The rest of the changes are simply to "inline" the BSS() macro (as I
needed to access that closing brace), and the former sbss_align,
bss_align (that's your PAGE_SIZE) and stop_align arguments are passed
accordingly, the same way they used to be passed before. The only
visible effect should be the move of ALIGN(stop_align) inside of .bss
section definition, and the change of stop_align value from 4 to 8.

Arguably the TurtleBoard loader should read the __bss_stop symbol offset
instead, but in this patch I'm trying to solve the issue from kernel's
point of view.

Cheers,
Artur

> 
>> +	}
>> +	__bss_stop = .;
>>  	_end = . ;
>> 
>>  	STABS_DEBUG
> 
> Thanks,
> Adrian

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[Geert Uytterhoeven]

Hi Artur,

On Wed, 12 Mar 2025 at 00:40, Artur Rojek <contact@artur-rojek.eu> wrote:
> On 2025-03-11 18:28, John Paul Adrian Glaubitz wrote:
> > I'm currently trying to review this patch, but I'm not 100% sure how it
> > this change helps grows the .bss section, see below. Maybe you can help
> > me understand what's happening.
> >
> > On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
> >> J2 based devices expect to find a devicetree blob at the end of the
> >> bss
> >> section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
> >> dtb, causing J2 devices to fail early in sh_fdt_init.
> >>
> >> As J2 loader firmware calculates the dtb location based on the kernel
> >> image .bss section size, rather than the __bss_stop symbol offset, the
> >> required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE,
> >> 8).
> >> Instead, inline modified version of the above macro, which grows .bss
> >> by the required size.
> >>
> >> While this change affects all existing SH boards, it should be benign
> >> on
> >> platforms which don't need this alignment.
> >>
> >> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> >> ---
> >>  arch/sh/kernel/vmlinux.lds.S | 15 ++++++++++++++-
> >>  1 file changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/sh/kernel/vmlinux.lds.S
> >> b/arch/sh/kernel/vmlinux.lds.S
> >> index 9644fe187a3f..008c30289eaa 100644
> >> --- a/arch/sh/kernel/vmlinux.lds.S
> >> +++ b/arch/sh/kernel/vmlinux.lds.S
> >> @@ -71,7 +71,20 @@ SECTIONS
> >>
> >>      . = ALIGN(PAGE_SIZE);
> >>      __init_end = .;
> >> -    BSS_SECTION(0, PAGE_SIZE, 4)
> >> +    __bss_start = .;
> >> +    SBSS(0)
> >> +    . = ALIGN(PAGE_SIZE);
> >
> > What this effectively does is removing ". = ALIGN(sbss_align);" first
> > from BSS_SECTION().
> >
> > Then it inserts ". = ALIGN(PAGE_SIZE);" after the "SBSS(0)".
> >
> > If I understand this correctly, SBSS() inserts a zero-padding and if
> > I'm not mistaken,
> > inserting ". = ALIGN(PAGE_SIZE);" will cause this padding to grow to at
> > least PAGE_SIZE
> > due the alignment.
> >
> > Is this correct?
> >
> >> +    .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
> >> +            BSS_FIRST_SECTIONS
> >> +            . = ALIGN(PAGE_SIZE);
> >> +            *(.bss..page_aligned)
> >> +            . = ALIGN(PAGE_SIZE);
> >> +            *(.dynbss)
> >> +            *(BSS_MAIN)
> >> +            *(COMMON)
> >> +            . = ALIGN(8);
> >
> > If my understanding above is correct, why do we will need an additional
> > ". = ALIGN(8)"
> > here?
>
> I'll tackle both of the above questions at once.
> I'm by no means an expert at GNU Linker syntax, but the intention of
> this patch is to put . = ALIGN(8) inside the .bss : { ... } section
> definition, so that the section itself grows by the requested padding.
>
> In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts
> a 4 byte padding after the closing brace of .bss section definition,
> causing the __bss_stop symbol offset to grow, but not the .bss section
> itself:
>
> #define BSS_SECTION(sbss_align, bss_align, stop_align)                  \
>         . = ALIGN(sbss_align);                                          \
>         __bss_start = .;                                                \
>         SBSS(sbss_align)                                                \
>         BSS(bss_align)                                                  \
>         . = ALIGN(stop_align);                                          \
>         __bss_stop = .;
>
> TurtleBoard loader is only concerned with the .bss section size - it
> doesn't care about any symbol offsets - and hence this seemingly cryptic
> change (you can display the section size information with
> readelf -t kernel_image).
> The rest of the changes are simply to "inline" the BSS() macro (as I
> needed to access that closing brace), and the former sbss_align,
> bss_align (that's your PAGE_SIZE) and stop_align arguments are passed
> accordingly, the same way they used to be passed before. The only
> visible effect should be the move of ALIGN(stop_align) inside of .bss
> section definition, and the change of stop_align value from 4 to 8.
>
> Arguably the TurtleBoard loader should read the __bss_stop symbol offset
> instead, but in this patch I'm trying to solve the issue from kernel's
> point of view.

What about moving (or duplicating, e.g. sbss_align alignment is
done before and after __bss_start)  the stop_align alignment
from BSS_SECTION() into BSS() instead, i.e. just changing
include/asm-generic/vmlinux.lds.h for everyone?  I don't think that
would hurt any platforms, while fixing the issue for good.
IMHO it is a bit strange that the size of the bss section can differ
from __bss_stop - __bss_start.
One last question though: what about sbss? How does the TurtleBoard
loader handle that?  __bss_stop - __bss_start is not the size of bss,
but the sum of the sizes of sbss and bss, plus extra alignment in
between. The latter might cause trouble, too.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[John Paul Adrian Glaubitz]

Hi Geert,

On Wed, 2025-03-12 at 09:06 +0100, Geert Uytterhoeven wrote:
> What about moving (or duplicating, e.g. sbss_align alignment is
> done before and after __bss_start)  the stop_align alignment
> from BSS_SECTION() into BSS() instead, i.e. just changing
> include/asm-generic/vmlinux.lds.h for everyone?  I don't think that
> would hurt any platforms, while fixing the issue for good.
> IMHO it is a bit strange that the size of the bss section can differ
> from __bss_stop - __bss_start.

This sounds reasonable. Could you send a patch? I assume that would go
through a different tree as we're touching generic code.

> One last question though: what about sbss? How does the TurtleBoard
> loader handle that?  __bss_stop - __bss_start is not the size of bss,
> but the sum of the sizes of sbss and bss, plus extra alignment in
> between. The latter might cause trouble, too.

Does the compiler actually generate the SBSS section on SH?

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[John Paul Adrian Glaubitz]

Hi Artur,

On Wed, 2025-03-12 at 00:40 +0100, Artur Rojek wrote:
> On 2025-03-11 18:28, John Paul Adrian Glaubitz wrote:
> > Hi Artur,
> 
> Hey Adrian,
> thanks for looking into this patch.

Sure. I just want to understand what's going on before signing it with "Reviewed-by",
I wouldn't dare that without fully understanding what the proposed change does ;-).

> > What this effectively does is removing ". = ALIGN(sbss_align);" first 
> > from BSS_SECTION().
> > 
> > Then it inserts ". = ALIGN(PAGE_SIZE);" after the "SBSS(0)".
> > 
> > If I understand this correctly, SBSS() inserts a zero-padding and if 
> > I'm not mistaken,
> > inserting ". = ALIGN(PAGE_SIZE);" will cause this padding to grow to at 
> > least PAGE_SIZE
> > due the alignment.
> > 
> > Is this correct?
> > 
> > > +	.bss : AT(ADDR(.bss) - LOAD_OFFSET) {
> > > +		BSS_FIRST_SECTIONS
> > > +		. = ALIGN(PAGE_SIZE);
> > > +		*(.bss..page_aligned)
> > > +		. = ALIGN(PAGE_SIZE);
> > > +		*(.dynbss)
> > > +		*(BSS_MAIN)
> > > +		*(COMMON)
> > > +		. = ALIGN(8);
> > 
> > If my understanding above is correct, why do we will need an additional 
> > ". = ALIGN(8)"
> > here?
> 
> I'll tackle both of the above questions at once.
> I'm by no means an expert at GNU Linker syntax, but the intention of
> this patch is to put . = ALIGN(8) inside the .bss : { ... } section
> definition, so that the section itself grows by the requested padding.

Makes sense.

> In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts
> a 4 byte padding after the closing brace of .bss section definition,
> causing the __bss_stop symbol offset to grow, but not the .bss section
> itself:
> 
> #define BSS_SECTION(sbss_align, bss_align, stop_align)			\
> 	. = ALIGN(sbss_align);						\
> 	__bss_start = .;						\
> 	SBSS(sbss_align)						\
> 	BSS(bss_align)							\
> 	. = ALIGN(stop_align);						\
> 	__bss_stop = .;

OK, that's really odd. So, the __bss_stop would be moved to the desired
position but the section itself still remains small? What exactly does the
linker fill the region with? Sounds very strange.

> TurtleBoard loader is only concerned with the .bss section size - it
> doesn't care about any symbol offsets - and hence this seemingly cryptic
> change (you can display the section size information with
> readelf -t kernel_image).

Looking at the actual kernel image with readelf is a very good suggestion!

> The rest of the changes are simply to "inline" the BSS() macro (as I
> needed to access that closing brace), and the former sbss_align,
> bss_align (that's your PAGE_SIZE) and stop_align arguments are passed
> accordingly, the same way they used to be passed before. The only
> visible effect should be the move of ALIGN(stop_align) inside of .bss
> section definition, and the change of stop_align value from 4 to 8.

OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation
for it.

> Arguably the TurtleBoard loader should read the __bss_stop symbol offset
> instead, but in this patch I'm trying to solve the issue from kernel's
> point of view.

That's absolutely sensible as this avoids having to update the firmware.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[Uros Bizjak]

On Wed, Mar 12, 2025 at 9:22 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:

> > In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts
> > a 4 byte padding after the closing brace of .bss section definition,
> > causing the __bss_stop symbol offset to grow, but not the .bss section
> > itself:
> >
> > #define BSS_SECTION(sbss_align, bss_align, stop_align)                        \
> >       . = ALIGN(sbss_align);                                          \
> >       __bss_start = .;                                                \
> >       SBSS(sbss_align)                                                \
> >       BSS(bss_align)                                                  \
> >       . = ALIGN(stop_align);                                          \
> >       __bss_stop = .;
>
> OK, that's really odd. So, the __bss_stop would be moved to the desired
> position but the section itself still remains small? What exactly does the
> linker fill the region with? Sounds very strange.
>
> > TurtleBoard loader is only concerned with the .bss section size - it
> > doesn't care about any symbol offsets - and hence this seemingly cryptic
> > change (you can display the section size information with
> > readelf -t kernel_image).
>
> Looking at the actual kernel image with readelf is a very good suggestion!
>
> > The rest of the changes are simply to "inline" the BSS() macro (as I
> > needed to access that closing brace), and the former sbss_align,
> > bss_align (that's your PAGE_SIZE) and stop_align arguments are passed
> > accordingly, the same way they used to be passed before. The only
> > visible effect should be the move of ALIGN(stop_align) inside of .bss
> > section definition, and the change of stop_align value from 4 to 8.
>
> OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation
> for it.

Small BSS section. The compiler can put data objects under a certain
size threshold to the .sbss section. Looking at GCC sh config, sh does
not use this section.

Uros.

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[John Paul Adrian Glaubitz]

Hi Uros,

On Wed, 2025-03-12 at 09:32 +0100, Uros Bizjak wrote:
> > OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation
> > for it.
> 
> Small BSS section. The compiler can put data objects under a certain
> size threshold to the .sbss section. Looking at GCC sh config, sh does
> not use this section.

Thank you, that helps me understand what's going a bit more!

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[Geert Uytterhoeven]

Hi Uros,

On Wed, 12 Mar 2025 at 09:32, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Mar 12, 2025 at 9:22 AM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
> > > In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts
> > > a 4 byte padding after the closing brace of .bss section definition,
> > > causing the __bss_stop symbol offset to grow, but not the .bss section
> > > itself:
> > >
> > > #define BSS_SECTION(sbss_align, bss_align, stop_align)                        \
> > >       . = ALIGN(sbss_align);                                          \
> > >       __bss_start = .;                                                \
> > >       SBSS(sbss_align)                                                \
> > >       BSS(bss_align)                                                  \
> > >       . = ALIGN(stop_align);                                          \
> > >       __bss_stop = .;
> >
> > OK, that's really odd. So, the __bss_stop would be moved to the desired
> > position but the section itself still remains small? What exactly does the
> > linker fill the region with? Sounds very strange.
> >
> > > TurtleBoard loader is only concerned with the .bss section size - it
> > > doesn't care about any symbol offsets - and hence this seemingly cryptic
> > > change (you can display the section size information with
> > > readelf -t kernel_image).
> >
> > Looking at the actual kernel image with readelf is a very good suggestion!
> >
> > > The rest of the changes are simply to "inline" the BSS() macro (as I
> > > needed to access that closing brace), and the former sbss_align,
> > > bss_align (that's your PAGE_SIZE) and stop_align arguments are passed
> > > accordingly, the same way they used to be passed before. The only
> > > visible effect should be the move of ALIGN(stop_align) inside of .bss
> > > section definition, and the change of stop_align value from 4 to 8.
> >
> > OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation
> > for it.
>
> Small BSS section. The compiler can put data objects under a certain
> size threshold to the .sbss section. Looking at GCC sh config, sh does
> not use this section.

Hence the moment gcc (or clang) starts using that section, the
TurtleBoard loader is broken again...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[Uros Bizjak]

On Wed, Mar 12, 2025 at 10:47 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Uros,
>
> On Wed, 12 Mar 2025 at 09:32, Uros Bizjak <ubizjak@gmail.com> wrote:
> > On Wed, Mar 12, 2025 at 9:22 AM John Paul Adrian Glaubitz
> > <glaubitz@physik.fu-berlin.de> wrote:
> > > > In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts
> > > > a 4 byte padding after the closing brace of .bss section definition,
> > > > causing the __bss_stop symbol offset to grow, but not the .bss section
> > > > itself:
> > > >
> > > > #define BSS_SECTION(sbss_align, bss_align, stop_align)                        \
> > > >       . = ALIGN(sbss_align);                                          \
> > > >       __bss_start = .;                                                \
> > > >       SBSS(sbss_align)                                                \
> > > >       BSS(bss_align)                                                  \
> > > >       . = ALIGN(stop_align);                                          \
> > > >       __bss_stop = .;
> > >
> > > OK, that's really odd. So, the __bss_stop would be moved to the desired
> > > position but the section itself still remains small? What exactly does the
> > > linker fill the region with? Sounds very strange.
> > >
> > > > TurtleBoard loader is only concerned with the .bss section size - it
> > > > doesn't care about any symbol offsets - and hence this seemingly cryptic
> > > > change (you can display the section size information with
> > > > readelf -t kernel_image).
> > >
> > > Looking at the actual kernel image with readelf is a very good suggestion!
> > >
> > > > The rest of the changes are simply to "inline" the BSS() macro (as I
> > > > needed to access that closing brace), and the former sbss_align,
> > > > bss_align (that's your PAGE_SIZE) and stop_align arguments are passed
> > > > accordingly, the same way they used to be passed before. The only
> > > > visible effect should be the move of ALIGN(stop_align) inside of .bss
> > > > section definition, and the change of stop_align value from 4 to 8.
> > >
> > > OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation
> > > for it.
> >
> > Small BSS section. The compiler can put data objects under a certain
> > size threshold to the .sbss section. Looking at GCC sh config, sh does
> > not use this section.
>
> Hence the moment gcc (or clang) starts using that section, the
> TurtleBoard loader is broken again...

Rest assured that the compiler won't just magically start using SBSS.
This is part of an ABI and in case ABI allows SBSS, the compiler needs
something like:

  if (in_small_data)
    switch_to_section (get_named_section (NULL, ".sbss", 0));

when emitting the declaration.

Uros.

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[David Laight]

On Wed, 12 Mar 2025 10:47:00 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Uros,
> 
> On Wed, 12 Mar 2025 at 09:32, Uros Bizjak <ubizjak@gmail.com> wrote:
> > On Wed, Mar 12, 2025 at 9:22 AM John Paul Adrian Glaubitz
> > <glaubitz@physik.fu-berlin.de> wrote:  
....
> > > OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation
> > > for it.  
> >
> > Small BSS section. The compiler can put data objects under a certain
> > size threshold to the .sbss section. Looking at GCC sh config, sh does
> > not use this section.  

The .sbss (and .sdata) sections are used by some architectures (eg Nios2)
for data that can be accessed using a 16bit offset from a fixed register.
(Although using the register as global register variable generates better code.)
Historically they may have been used for data at the top/bottom of the
addresses space (for the same reason).

There is no reason to just use it for 'small' (eg 8 bytes or less)
data, that is just some empirical default.

I guess they could also be used on cpu like the strongarm for memory
that used the 'small data cache' (useful for screen buffer memory!).

	David

Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

[John Paul Adrian Glaubitz]

Hi Artur,

On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
> J2 based devices expect to find a devicetree blob at the end of the bss
> section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
> dtb, causing J2 devices to fail early in sh_fdt_init.
> 
> As J2 loader firmware calculates the dtb location based on the kernel
> image .bss section size, rather than the __bss_stop symbol offset, the
> required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE, 8).
> Instead, inline modified version of the above macro, which grows .bss
> by the required size.
> 
> While this change affects all existing SH boards, it should be benign on
> platforms which don't need this alignment.
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
>  arch/sh/kernel/vmlinux.lds.S | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/sh/kernel/vmlinux.lds.S b/arch/sh/kernel/vmlinux.lds.S
> index 9644fe187a3f..008c30289eaa 100644
> --- a/arch/sh/kernel/vmlinux.lds.S
> +++ b/arch/sh/kernel/vmlinux.lds.S
> @@ -71,7 +71,20 @@ SECTIONS
>  
>  	. = ALIGN(PAGE_SIZE);
>  	__init_end = .;
> -	BSS_SECTION(0, PAGE_SIZE, 4)
> +	__bss_start = .;
> +	SBSS(0)
> +	. = ALIGN(PAGE_SIZE);
> +	.bss : AT(ADDR(.bss) - LOAD_OFFSET) {
> +		BSS_FIRST_SECTIONS
> +		. = ALIGN(PAGE_SIZE);
> +		*(.bss..page_aligned)
> +		. = ALIGN(PAGE_SIZE);
> +		*(.dynbss)
> +		*(BSS_MAIN)
> +		*(COMMON)
> +		. = ALIGN(8);
> +	}
> +	__bss_stop = .;
>  	_end = . ;
>  
>  	STABS_DEBUG

I'll pick this up for now since Uros has confirmed that the compiler
won't just use SBSS without breaking the ABI, so I think to use this
fix for now.

If it breaks in the future, we can change it again.

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

[PATCH 2/2] irqchip: clocksource: fix jcore-pit irq request

[Artur Rojek]

The jcore-aic irqchip does not have separate interrupt numbers reserved
for cpu-local vs global interrupts. Instead, the task of selecting this
property is being delegated to the device drivers requesting the given
irq.

This quirk has not been taken into account while migrating jcore-pit to
request_percpu_irq(), resulting in a failure to register PIT interrupts.

Fix this behavior by making the following changes:
1) Explicitly register irq_set_percpu_devid() in jcore-pit.
2) Provide enable_percpu_irq()/disable_percpu_irq() calls in jcore-pit.
3) Make jcore-aic pass the correct per-cpu cookie to the irq handler by
   using handle_percpu_devid_irq() instead of handle_percpu_irq().

Fixes: 69a9dcbd2d65 ("clocksource/drivers/jcore: Use request_percpu_irq()")

Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---
 drivers/clocksource/jcore-pit.c | 15 ++++++++++++++-
 drivers/irqchip/irq-jcore-aic.c |  2 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/jcore-pit.c b/drivers/clocksource/jcore-pit.c
index a3fe98cd3838..82815428f8f9 100644
--- a/drivers/clocksource/jcore-pit.c
+++ b/drivers/clocksource/jcore-pit.c
@@ -114,6 +114,18 @@ static int jcore_pit_local_init(unsigned cpu)
 	pit->periodic_delta = DIV_ROUND_CLOSEST(NSEC_PER_SEC, HZ * buspd);
 
 	clockevents_config_and_register(&pit->ced, freq, 1, ULONG_MAX);
+	enable_percpu_irq(pit->ced.irq, IRQ_TYPE_NONE);
+
+	return 0;
+}
+
+static int jcore_pit_local_teardown(unsigned cpu)
+{
+	struct jcore_pit *pit = this_cpu_ptr(jcore_pit_percpu);
+
+	pr_info("Local J-Core PIT teardown on cpu %u\n", cpu);
+
+	disable_percpu_irq(pit->ced.irq);
 
 	return 0;
 }
@@ -168,6 +180,7 @@ static int __init jcore_pit_init(struct device_node *node)
 		return -ENOMEM;
 	}
 
+	irq_set_percpu_devid(pit_irq);
 	err = request_percpu_irq(pit_irq, jcore_timer_interrupt,
 				 "jcore_pit", jcore_pit_percpu);
 	if (err) {
@@ -237,7 +250,7 @@ static int __init jcore_pit_init(struct device_node *node)
 
 	cpuhp_setup_state(CPUHP_AP_JCORE_TIMER_STARTING,
 			  "clockevents/jcore:starting",
-			  jcore_pit_local_init, NULL);
+			  jcore_pit_local_init, jcore_pit_local_teardown);
 
 	return 0;
 }
diff --git a/drivers/irqchip/irq-jcore-aic.c b/drivers/irqchip/irq-jcore-aic.c
index b9dcc8e78c75..1f613eb7b7f0 100644
--- a/drivers/irqchip/irq-jcore-aic.c
+++ b/drivers/irqchip/irq-jcore-aic.c
@@ -38,7 +38,7 @@ static struct irq_chip jcore_aic;
 static void handle_jcore_irq(struct irq_desc *desc)
 {
 	if (irqd_is_per_cpu(irq_desc_get_irq_data(desc)))
-		handle_percpu_irq(desc);
+		handle_percpu_devid_irq(desc);
 	else
 		handle_simple_irq(desc);
 }
-- 
2.48.1

Re: [PATCH 2/2] irqchip: clocksource: fix jcore-pit irq request

[Uros Bizjak]

On Sun, Feb 16, 2025 at 6:56 PM Artur Rojek <contact@artur-rojek.eu> wrote:
>
> The jcore-aic irqchip does not have separate interrupt numbers reserved
> for cpu-local vs global interrupts. Instead, the task of selecting this
> property is being delegated to the device drivers requesting the given
> irq.
>
> This quirk has not been taken into account while migrating jcore-pit to
> request_percpu_irq(), resulting in a failure to register PIT interrupts.
>
> Fix this behavior by making the following changes:
> 1) Explicitly register irq_set_percpu_devid() in jcore-pit.
> 2) Provide enable_percpu_irq()/disable_percpu_irq() calls in jcore-pit.
> 3) Make jcore-aic pass the correct per-cpu cookie to the irq handler by
>    using handle_percpu_devid_irq() instead of handle_percpu_irq().
>
> Fixes: 69a9dcbd2d65 ("clocksource/drivers/jcore: Use request_percpu_irq()")
>
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>

I can confirm that this change compiles OK with strict percpu address
space checks on x86 [1].

Acked-by: Uros Bizjak <ubizjak@gmail.com>

[1] https://lore.kernel.org/lkml/20250127160709.80604-1-ubizjak@gmail.com/

Uros.

> ---
>  drivers/clocksource/jcore-pit.c | 15 ++++++++++++++-
>  drivers/irqchip/irq-jcore-aic.c |  2 +-
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/jcore-pit.c b/drivers/clocksource/jcore-pit.c
> index a3fe98cd3838..82815428f8f9 100644
> --- a/drivers/clocksource/jcore-pit.c
> +++ b/drivers/clocksource/jcore-pit.c
> @@ -114,6 +114,18 @@ static int jcore_pit_local_init(unsigned cpu)
>         pit->periodic_delta = DIV_ROUND_CLOSEST(NSEC_PER_SEC, HZ * buspd);
>
>         clockevents_config_and_register(&pit->ced, freq, 1, ULONG_MAX);
> +       enable_percpu_irq(pit->ced.irq, IRQ_TYPE_NONE);
> +
> +       return 0;
> +}
> +
> +static int jcore_pit_local_teardown(unsigned cpu)
> +{
> +       struct jcore_pit *pit = this_cpu_ptr(jcore_pit_percpu);
> +
> +       pr_info("Local J-Core PIT teardown on cpu %u\n", cpu);
> +
> +       disable_percpu_irq(pit->ced.irq);
>
>         return 0;
>  }
> @@ -168,6 +180,7 @@ static int __init jcore_pit_init(struct device_node *node)
>                 return -ENOMEM;
>         }
>
> +       irq_set_percpu_devid(pit_irq);
>         err = request_percpu_irq(pit_irq, jcore_timer_interrupt,
>                                  "jcore_pit", jcore_pit_percpu);
>         if (err) {
> @@ -237,7 +250,7 @@ static int __init jcore_pit_init(struct device_node *node)
>
>         cpuhp_setup_state(CPUHP_AP_JCORE_TIMER_STARTING,
>                           "clockevents/jcore:starting",
> -                         jcore_pit_local_init, NULL);
> +                         jcore_pit_local_init, jcore_pit_local_teardown);
>
>         return 0;
>  }
> diff --git a/drivers/irqchip/irq-jcore-aic.c b/drivers/irqchip/irq-jcore-aic.c
> index b9dcc8e78c75..1f613eb7b7f0 100644
> --- a/drivers/irqchip/irq-jcore-aic.c
> +++ b/drivers/irqchip/irq-jcore-aic.c
> @@ -38,7 +38,7 @@ static struct irq_chip jcore_aic;
>  static void handle_jcore_irq(struct irq_desc *desc)
>  {
>         if (irqd_is_per_cpu(irq_desc_get_irq_data(desc)))
> -               handle_percpu_irq(desc);
> +               handle_percpu_devid_irq(desc);
>         else
>                 handle_simple_irq(desc);
>  }
> --
> 2.48.1
>

Re: [PATCH 2/2] irqchip: clocksource: fix jcore-pit irq request

[Rob Landley]

On 2/16/25 11:55, Artur Rojek wrote:
> The jcore-aic irqchip does not have separate interrupt numbers reserved
> for cpu-local vs global interrupts. Instead, the task of selecting this
> property is being delegated to the device drivers requesting the given
> irq.
> 
> This quirk has not been taken into account while migrating jcore-pit to
> request_percpu_irq(), resulting in a failure to register PIT interrupts.
> 
> Fix this behavior by making the following changes:
> 1) Explicitly register irq_set_percpu_devid() in jcore-pit.
> 2) Provide enable_percpu_irq()/disable_percpu_irq() calls in jcore-pit.
> 3) Make jcore-aic pass the correct per-cpu cookie to the irq handler by
>     using handle_percpu_devid_irq() instead of handle_percpu_irq().
> 
> Fixes: 69a9dcbd2d65 ("clocksource/drivers/jcore: Use request_percpu_irq()")
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>

Tested-by: Rob Landley <rob@landley.net>

Rob

Re: [PATCH 2/2] irqchip: clocksource: fix jcore-pit irq request

[Daniel Lezcano]

On 16/02/2025 18:55, Artur Rojek wrote:
> The jcore-aic irqchip does not have separate interrupt numbers reserved
> for cpu-local vs global interrupts. Instead, the task of selecting this
> property is being delegated to the device drivers requesting the given
> irq.
> 
> This quirk has not been taken into account while migrating jcore-pit to
> request_percpu_irq(), resulting in a failure to register PIT interrupts.
> 
> Fix this behavior by making the following changes:
> 1) Explicitly register irq_set_percpu_devid() in jcore-pit.
> 2) Provide enable_percpu_irq()/disable_percpu_irq() calls in jcore-pit.
> 3) Make jcore-aic pass the correct per-cpu cookie to the irq handler by
>     using handle_percpu_devid_irq() instead of handle_percpu_irq().
> 
> Fixes: 69a9dcbd2d65 ("clocksource/drivers/jcore: Use request_percpu_irq()")
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---

How this patch should be merged ?

It is touching irqchip and clocksource at the same time.

May I pick it in the clocksource tree ?


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 2/2] irqchip: clocksource: fix jcore-pit irq request

[Geert Uytterhoeven]

Hi Daniel,

On Wed, 19 Feb 2025 at 15:43, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> On 16/02/2025 18:55, Artur Rojek wrote:
> > The jcore-aic irqchip does not have separate interrupt numbers reserved
> > for cpu-local vs global interrupts. Instead, the task of selecting this
> > property is being delegated to the device drivers requesting the given
> > irq.
> >
> > This quirk has not been taken into account while migrating jcore-pit to
> > request_percpu_irq(), resulting in a failure to register PIT interrupts.
> >
> > Fix this behavior by making the following changes:
> > 1) Explicitly register irq_set_percpu_devid() in jcore-pit.
> > 2) Provide enable_percpu_irq()/disable_percpu_irq() calls in jcore-pit.
> > 3) Make jcore-aic pass the correct per-cpu cookie to the irq handler by
> >     using handle_percpu_devid_irq() instead of handle_percpu_irq().
> >
> > Fixes: 69a9dcbd2d65 ("clocksource/drivers/jcore: Use request_percpu_irq()")
> >
> > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> > ---
>
> How this patch should be merged ?
>
> It is touching irqchip and clocksource at the same time.
>
> May I pick it in the clocksource tree ?

Thomas already took it, cfr. commit d7e3fd658248f257
("irqchip/jcore-aic, clocksource/drivers/jcore: Fix jcore-pit interrupt
request") in next-20250219.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] irqchip: clocksource: fix jcore-pit irq request

[Daniel Lezcano]

On 19/02/2025 15:50, Geert Uytterhoeven wrote:
> Hi Daniel,
> 
> On Wed, 19 Feb 2025 at 15:43, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>> On 16/02/2025 18:55, Artur Rojek wrote:
>>> The jcore-aic irqchip does not have separate interrupt numbers reserved
>>> for cpu-local vs global interrupts. Instead, the task of selecting this
>>> property is being delegated to the device drivers requesting the given
>>> irq.
>>>
>>> This quirk has not been taken into account while migrating jcore-pit to
>>> request_percpu_irq(), resulting in a failure to register PIT interrupts.
>>>
>>> Fix this behavior by making the following changes:
>>> 1) Explicitly register irq_set_percpu_devid() in jcore-pit.
>>> 2) Provide enable_percpu_irq()/disable_percpu_irq() calls in jcore-pit.
>>> 3) Make jcore-aic pass the correct per-cpu cookie to the irq handler by
>>>      using handle_percpu_devid_irq() instead of handle_percpu_irq().
>>>
>>> Fixes: 69a9dcbd2d65 ("clocksource/drivers/jcore: Use request_percpu_irq()")
>>>
>>> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
>>> ---
>>
>> How this patch should be merged ?
>>
>> It is touching irqchip and clocksource at the same time.
>>
>> May I pick it in the clocksource tree ?
> 
> Thomas already took it, cfr. commit d7e3fd658248f257
> ("irqchip/jcore-aic, clocksource/drivers/jcore: Fix jcore-pit interrupt
> request") in next-20250219.

Oh, ok, thanks for letting me know

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 0/2] J2 Turtle Board fixes

[John Paul Adrian Glaubitz]

Hi Artur,

On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
> this series fixes boot issues and allows J2 Turtle Board to boot
> upstream Linux again.
> 
> Patch [1/2] enforces 8-byte alignment for the dtb offset.
> 
> Patch [2/2] resolves a problem with PIT interrupts failing to register.

I can confirm that this series makes my J2 Turtle Board boot again!

> Even with the above fixes, Turtle Board is prone to occasional freezes
> related to clock source transition from periodic to hrtimers. I however
> decided to send those two patches ahead and debug the third issue at a
> later time. 

Yep, it just got stuck for me right after these messages at my first boot attempt:

clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
futex hash table entries: 512 (order: 1, 8192 bytes, linear)
NET: Registered PF_NETLINK/PF_ROUTE protocol family
clocksource: Switched to clocksource jcore_pit_cs

It boots past these messages on second attempt, although it's now stuck trying to start
/init. However, it's still echoing <RETURN> strokes, so it might be an issue with Toybox.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 0/2] J2 Turtle Board fixes

[Rob Landley]

On 2/27/25 01:52, John Paul Adrian Glaubitz wrote:
> Hi Artur,
> 
> On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
>> this series fixes boot issues and allows J2 Turtle Board to boot
>> upstream Linux again.
>>
>> Patch [1/2] enforces 8-byte alignment for the dtb offset.
>>
>> Patch [2/2] resolves a problem with PIT interrupts failing to register.
> 
> I can confirm that this series makes my J2 Turtle Board boot again!
> 
>> Even with the above fixes, Turtle Board is prone to occasional freezes
>> related to clock source transition from periodic to hrtimers. I however
>> decided to send those two patches ahead and debug the third issue at a
>> later time.
> 
> Yep, it just got stuck for me right after these messages at my first boot attempt:
> 
> clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
> futex hash table entries: 512 (order: 1, 8192 bytes, linear)
> NET: Registered PF_NETLINK/PF_ROUTE protocol family
> clocksource: Switched to clocksource jcore_pit_cs
> 
> It boots past these messages on second attempt, although it's now stuck trying to start
> /init. However, it's still echoing <RETURN> strokes, so it might be an issue with Toybox.

Which was fixed a year ago, which is why I told you to use the new 
toolchain with a current musl-libc:

http://lists.landley.net/pipermail/toybox-landley.net/2024-February/030040.html

Unless you're hitting the OTHER issue I fixed last year...

https://github.com/landley/toybox/commit/0b2d5c2bb3f1

Rob

Re: [PATCH 0/2] J2 Turtle Board fixes

[John Paul Adrian Glaubitz]

Hi Rob,

On Thu, 2025-02-27 at 21:03 -0600, Rob Landley wrote:
> On 2/27/25 01:52, John Paul Adrian Glaubitz wrote:
> > Hi Artur,
> > 
> > On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
> > > this series fixes boot issues and allows J2 Turtle Board to boot
> > > upstream Linux again.
> > > 
> > > Patch [1/2] enforces 8-byte alignment for the dtb offset.
> > > 
> > > Patch [2/2] resolves a problem with PIT interrupts failing to register.
> > 
> > I can confirm that this series makes my J2 Turtle Board boot again!
> > 
> > > Even with the above fixes, Turtle Board is prone to occasional freezes
> > > related to clock source transition from periodic to hrtimers. I however
> > > decided to send those two patches ahead and debug the third issue at a
> > > later time.
> > 
> > Yep, it just got stuck for me right after these messages at my first boot attempt:
> > 
> > clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
> > futex hash table entries: 512 (order: 1, 8192 bytes, linear)
> > NET: Registered PF_NETLINK/PF_ROUTE protocol family
> > clocksource: Switched to clocksource jcore_pit_cs
> > 
> > It boots past these messages on second attempt, although it's now stuck trying to start
> > /init. However, it's still echoing <RETURN> strokes, so it might be an issue with Toybox.
> 
> Which was fixed a year ago, which is why I told you to use the new 
> toolchain with a current musl-libc:
> 
> http://lists.landley.net/pipermail/toybox-landley.net/2024-February/030040.html
> 
> Unless you're hitting the OTHER issue I fixed last year...
> 
> https://github.com/landley/toybox/commit/0b2d5c2bb3f1

I just downloaded the latest toolchain from:

https://landley.net/bin/toolchains/latest/sh2eb-linux-muslfdpic-cross.tar.xz

and the issue still persists.

Am I missing anything?

Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 0/2] J2 Turtle Board fixes

[Rob Landley]

On 2/28/25 02:34, John Paul Adrian Glaubitz wrote:
> Hi Rob,
> 
> On Thu, 2025-02-27 at 21:03 -0600, Rob Landley wrote:
>> On 2/27/25 01:52, John Paul Adrian Glaubitz wrote:
>>> Hi Artur,
>>>
>>> On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
>>>> this series fixes boot issues and allows J2 Turtle Board to boot
>>>> upstream Linux again.
>>>>
>>>> Patch [1/2] enforces 8-byte alignment for the dtb offset.
>>>>
>>>> Patch [2/2] resolves a problem with PIT interrupts failing to register.
>>>
>>> I can confirm that this series makes my J2 Turtle Board boot again!
>>>
>>>> Even with the above fixes, Turtle Board is prone to occasional freezes
>>>> related to clock source transition from periodic to hrtimers. I however
>>>> decided to send those two patches ahead and debug the third issue at a
>>>> later time.
>>>
>>> Yep, it just got stuck for me right after these messages at my first boot attempt:
>>>
>>> clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
>>> futex hash table entries: 512 (order: 1, 8192 bytes, linear)
>>> NET: Registered PF_NETLINK/PF_ROUTE protocol family
>>> clocksource: Switched to clocksource jcore_pit_cs
>>>
>>> It boots past these messages on second attempt, although it's now stuck trying to start
>>> /init. However, it's still echoing <RETURN> strokes, so it might be an issue with Toybox.
>>
>> Which was fixed a year ago, which is why I told you to use the new
>> toolchain with a current musl-libc:
>>
>> http://lists.landley.net/pipermail/toybox-landley.net/2024-February/030040.html
>>
>> Unless you're hitting the OTHER issue I fixed last year...
>>
>> https://github.com/landley/toybox/commit/0b2d5c2bb3f1
> 
> I just downloaded the latest toolchain from:
> 
> https://landley.net/bin/toolchains/latest/sh2eb-linux-muslfdpic-cross.tar.xz
> 
> and the issue still persists.
> 
> Am I missing anything?

The march 2024 rebuild was in response to that Feb 2024 bugfix, so it 
_should_ have the fix? (I'm waiting for another musl release to rebuild 
them again...)

I just downloaded the toolchain currently at that URL and built mkroot 
and it worked for me:

Run /init as init process
sntp: time.google.com:123: Try again
Type exit when done.
$ cat /proc/version
Linux version 6.14.0-rc3 (landley@driftwood) (sh2eb-linux-muslfdpic-cc 
(GCC) 11.2.0, GNU ld (GNU Binutils) 2.33.1) #1 SMP Fri Feb 28 15:47:36 
CST 2025

And the failure _without_ the fix was deterministic rather than 
intermittent, so...

Keep in mind the init script has a 3 second timeout trying to call sntp 
to set the clock, which will fail if the ethernet isn't connected (or no 
driver, or no internet...)

Rob

P.S. Speaking of intermittent, I hit that hang after "clocksource: 
Switched to clocksource jcore_pit_cs" on one attempt just now. I should 
sit down with the engineers next time I'm in japan and try to root cause 
it. The scheduler fires reliably, so it's _probably_ not a hardware 
issue? We've had Linux uptime of over a year, not just idle but running 
an energy monitoring app, so it's pretty stable in our systems...

Re: [PATCH 0/2] J2 Turtle Board fixes

[John Paul Adrian Glaubitz]

Hi,

On Fri, 2025-02-28 at 16:19 -0600, Rob Landley wrote:
> The march 2024 rebuild was in response to that Feb 2024 bugfix, so it 
> _should_ have the fix? (I'm waiting for another musl release to rebuild 
> them again...)
> 
> I just downloaded the toolchain currently at that URL and built mkroot 
> and it worked for me:
> 
> Run /init as init process
> sntp: time.google.com:123: Try again
> Type exit when done.
> $ cat /proc/version
> Linux version 6.14.0-rc3 (landley@driftwood) (sh2eb-linux-muslfdpic-cc 
> (GCC) 11.2.0, GNU ld (GNU Binutils) 2.33.1) #1 SMP Fri Feb 28 15:47:36 
> CST 2025

Is that on Toybox git HEAD?

> And the failure _without_ the fix was deterministic rather than 
> intermittent, so...
> 
> Keep in mind the init script has a 3 second timeout trying to call sntp 
> to set the clock, which will fail if the ethernet isn't connected (or no 
> driver, or no internet...)

I'll try again this weekend. Also, I will review and pick up the fix.

> P.S. Speaking of intermittent, I hit that hang after "clocksource: 
> Switched to clocksource jcore_pit_cs" on one attempt just now. I should 
> sit down with the engineers next time I'm in japan and try to root cause 
> it. The scheduler fires reliably, so it's _probably_ not a hardware 
> issue? We've had Linux uptime of over a year, not just idle but running 
> an energy monitoring app, so it's pretty stable in our systems...

I thought it was a software issue?

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 0/2] J2 Turtle Board fixes

[Rob Landley]

On 2/28/25 16:34, John Paul Adrian Glaubitz wrote:
> Hi,
> 
> On Fri, 2025-02-28 at 16:19 -0600, Rob Landley wrote:
>> The march 2024 rebuild was in response to that Feb 2024 bugfix, so it
>> _should_ have the fix? (I'm waiting for another musl release to rebuild
>> them again...)
>>
>> I just downloaded the toolchain currently at that URL and built mkroot
>> and it worked for me:
>>
>> Run /init as init process
>> sntp: time.google.com:123: Try again
>> Type exit when done.
>> $ cat /proc/version
>> Linux version 6.14.0-rc3 (landley@driftwood) (sh2eb-linux-muslfdpic-cc
>> (GCC) 11.2.0, GNU ld (GNU Binutils) 2.33.1) #1 SMP Fri Feb 28 15:47:36
>> CST 2025
> 
> Is that on Toybox git HEAD?

Yes. Commit b4fd71d18f84.

>> And the failure _without_ the fix was deterministic rather than
>> intermittent, so...
>>
>> Keep in mind the init script has a 3 second timeout trying to call sntp
>> to set the clock, which will fail if the ethernet isn't connected (or no
>> driver, or no internet...)
> 
> I'll try again this weekend. Also, I will review and pick up the fix.

Ok. (I'm available Saturday if you need to poke me, but traveling sunday.)

>> P.S. Speaking of intermittent, I hit that hang after "clocksource:
>> Switched to clocksource jcore_pit_cs" on one attempt just now. I should
>> sit down with the engineers next time I'm in japan and try to root cause
>> it. The scheduler fires reliably, so it's _probably_ not a hardware
>> issue? We've had Linux uptime of over a year, not just idle but running
>> an energy monitoring app, so it's pretty stable in our systems...
> 
> I thought it was a software issue?

I agree. That's why I said it's probably not a hardware issue. (The 
config has NO_HZ_IDLE so if the PIT didn't fire reliably when 
reprogrammed the scheduler would flake, and it's not, so...)

> Adrian

Rob

Re: [PATCH 0/2] J2 Turtle Board fixes

[John Paul Adrian Glaubitz]

Hi Rob,

On Fri, 2025-02-28 at 16:19 -0600, Rob Landley wrote:
> > > Which was fixed a year ago, which is why I told you to use the new
> > > toolchain with a current musl-libc:
> > > 
> > > http://lists.landley.net/pipermail/toybox-landley.net/2024-February/030040.html
> > > 
> > > Unless you're hitting the OTHER issue I fixed last year...
> > > 
> > > https://github.com/landley/toybox/commit/0b2d5c2bb3f1
> > 
> > I just downloaded the latest toolchain from:
> > 
> > https://landley.net/bin/toolchains/latest/sh2eb-linux-muslfdpic-cross.tar.xz
> > 
> > and the issue still persists.
> > 
> > Am I missing anything?
> 
> The march 2024 rebuild was in response to that Feb 2024 bugfix, so it 
> _should_ have the fix? (I'm waiting for another musl release to rebuild 
> them again...)
> 
> I just downloaded the toolchain currently at that URL and built mkroot 
> and it worked for me:
> 
> Run /init as init process
> sntp: time.google.com:123: Try again
> Type exit when done.
> $ cat /proc/version
> Linux version 6.14.0-rc3 (landley@driftwood) (sh2eb-linux-muslfdpic-cc 
> (GCC) 11.2.0, GNU ld (GNU Binutils) 2.33.1) #1 SMP Fri Feb 28 15:47:36 
> CST 2025
> 
> And the failure _without_ the fix was deterministic rather than 
> intermittent, so...
> 
> Keep in mind the init script has a 3 second timeout trying to call sntp 
> to set the clock, which will fail if the ethernet isn't connected (or no 
> driver, or no internet...)

I just gave it another try and it still hangs for me at:

	Run /init as init process

with the latest toolchain, toybox and kernel (v6.15-rc-1).

FWIW, I did not connect an ethernet cable.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 0/2] J2 Turtle Board fixes

[Rob Landley]

On 4/8/25 10:23, John Paul Adrian Glaubitz wrote:
> Hi Rob,
> 
> On Fri, 2025-02-28 at 16:19 -0600, Rob Landley wrote:
>>>> Which was fixed a year ago, which is why I told you to use the new
>>>> toolchain with a current musl-libc:
>>>>
>>>> http://lists.landley.net/pipermail/toybox-landley.net/2024-February/030040.html
>>>>
>>>> Unless you're hitting the OTHER issue I fixed last year...
>>>>
>>>> https://github.com/landley/toybox/commit/0b2d5c2bb3f1
>>>
>>> I just downloaded the latest toolchain from:
>>>
>>> https://landley.net/bin/toolchains/latest/sh2eb-linux-muslfdpic-cross.tar.xz
>>>
>>> and the issue still persists.
>>>
>>> Am I missing anything?
>>
>> The march 2024 rebuild was in response to that Feb 2024 bugfix, so it
>> _should_ have the fix? (I'm waiting for another musl release to rebuild
>> them again...)
>>
>> I just downloaded the toolchain currently at that URL and built mkroot
>> and it worked for me:
>>
>> Run /init as init process
>> sntp: time.google.com:123: Try again
>> Type exit when done.
>> $ cat /proc/version
>> Linux version 6.14.0-rc3 (landley@driftwood) (sh2eb-linux-muslfdpic-cc
>> (GCC) 11.2.0, GNU ld (GNU Binutils) 2.33.1) #1 SMP Fri Feb 28 15:47:36
>> CST 2025
>>
>> And the failure _without_ the fix was deterministic rather than
>> intermittent, so...
>>
>> Keep in mind the init script has a 3 second timeout trying to call sntp
>> to set the clock, which will fail if the ethernet isn't connected (or no
>> driver, or no internet...)
> 
> I just gave it another try and it still hangs for me at:
> 
> 	Run /init as init process
> 
> with the latest toolchain, toybox and kernel (v6.15-rc-1).

FYI I reproduced this but haven't tracked it down yet.

Rob

Re: [PATCH 0/2] J2 Turtle Board fixes

[John Paul Adrian Glaubitz]

Hi Rob,

On Fri, 2025-04-11 at 06:25 -0500, Rob Landley wrote:
> > I just gave it another try and it still hangs for me at:
> > 
> > 	Run /init as init process
> > 
> > with the latest toolchain, toybox and kernel (v6.15-rc-1).
> 
> FYI I reproduced this but haven't tracked it down yet.

I have updated to the latest git revision now with the result that it
fails executing /init now. Can you confirm this error?

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 0/2] J2 Turtle Board fixes

[Rob Landley]

On 6/9/25 05:01, John Paul Adrian Glaubitz wrote:
> Hi Rob,
> 
> On Fri, 2025-04-11 at 06:25 -0500, Rob Landley wrote:
>>> I just gave it another try and it still hangs for me at:
>>>
>>> 	Run /init as init process
>>>
>>> with the latest toolchain, toybox and kernel (v6.15-rc-1).
>>
>> FYI I reproduced this but haven't tracked it down yet.
> 
> I have updated to the latest git revision now with the result that it
> fails executing /init now. Can you confirm this error?

6.15 works for me? How did I reproduce it last time... Ah right, I 
always build turtle with my patch stack applied:

https://landley.net/bin/mkroot/latest/linux-patches/

Sorry, fell off my todo list. I'll take another look. Thanks for the poke.

Rob