On 15/07/21, Jorge Ramirez-Ortiz, Gmail wrote:
> On 06/07/21, Ramon Fried wrote:
> > On Mon, Jul 5, 2021 at 3:19 PM Stephan Gerhold <step...@gerhold.net> wrote:
> > >
> > > According to arch/arm/lib/crt0_64.S, the BSS section is "UNAVAILABLE"
> > > and uninitialized before relocation. Also, it overlaps with the
> > > appended DTB before relocation, so writing data into a variable
> > > in the BSS section might corrupt the appended DTB.
> > >
> > > Unfortunately, pinctrl-apq8016.c and pinctrl-apq8096.c do place the
> > > "pin_name" variable in the BSS section (since it's uninitialized).
> > > It's also used before relocation, when setting up the pinctrl for
> > > the serial driver.
> > >
> > > On DB410c this causes "GPIO_5" to be written into some part of an
> > > appended DTB, e.g.:
> > >
> > > 80111820: edfe0dd0 9f100000 38000000 c00e0000 ...........8....
> > > 80111830: 28000000 11000000 10000000 00000000 ...(............
> > > 80111840: 4f495047 8800355f 00000000 00000000 GPIO_5..........
> > > 80111850: 00000000 00000000 01000000 00000000 ................
> > > 80111860: 03000000 04000000 00000000 02000000 ................
> > > 80111870: 03000000 04000000 0f000000 02000000 ................
> > > 80111880: 03000000 2d000000 1b000000 6c617551 .......-....Qual
> > > 80111890: 6d6d6f63 63655420 6c6f6e68 6569676f comm Technologie
> > >
> > > Depending on the part of the DTB that is corrupted this might not
> > > cause any problems, but it can also result in strange reboots
> > > without any serial output.
> > >
> > > Fortunately, in practice this does not cause issues on DB410c yet
> > > because board_fdt_blob_setup() in dragonboard410c.c currently
> > > overrides the appended DTB with the one passed by the previous
> > > bootloader (LK) (which does not get corrupted).
> > >
> > > DB820c does not have board_fdt_blob_setup() so I would expect it to
> > > be affected by this problem. Perhaps everyone was just fortunate to
> > > not compile an U-Boot configuration where the pin_name corrupts an
> > > important part of the DTB.
>
> I'll try to confirm during the week.
>
> After Ramon's pinctrl changes db820c did indeed break - was able to
> test a few months back- but we didnt have a board to test back then
> (and nobody complained)
>
> will come back to you on this but from what you are saying, this is
> probably the fix we needed for db820c
yep, all good. just tested it. thanks for the fix!
U-Boot 2021.07-00467-gc11f5abce8 (Jul 15 2021 - 09:20:22 +0200)
Qualcomm-DragonBoard 820C
DRAM: 3 GiB
PSCI: v1.0
MMC: sdhci@74a4900: 0
Loading Environment from EXT4... MMC: no card present
In: serial@75b0000
Out: serial@75b0000
Err: serial@75b0000
Net: Net Initialization Skipped
No ethernet found.
Hit any key to stop autoboot: 0
MMC: no card present
dragonboard820c =>
dragonboard820c => bdinfo
boot_params = 0x0000000000000000
DRAM bank = 0x0000000000000000
-> start = 0x0000000080000000
-> size = 0x0000000060000000
DRAM bank = 0x0000000000000001
-> start = 0x0000000100000000
-> size = 0x000000005ea4ffff
flashstart = 0x0000000000000000
flashsize = 0x0000000000000000
flashoffset = 0x0000000000000000
baudrate = 115200 bps
relocaddr = 0x000000013ff70000
reloc off = 0x00000000bfef0000
Build = 64-bit
current eth = unknown
ethaddr = (not set)
IP addr = <NULL>
fdt_blob = 0x000000013f7673c0
new_fdt = 0x000000013f7673c0
fdt_size = 0x0000000000000a20
lmb_dump_all:
memory.cnt = 0x2
memory[0] [0x80000000-0xdfffffff], 0x60000000 bytes flags: 0
memory[1] [0x100000000-0x15ea4fffe], 0x5ea4ffff bytes flags: 0
reserved.cnt = 0x1
reserved[0] [0x86300000-0x864fffff], 0x00200000 bytes flags: 4
arch_number = 0x0000000000000000
TLB addr = 0x000000013fff0000
irq_sp = 0x000000013f7673b0
sp start = 0x000000013f7673b0
Early malloc usage: 5d8 / 2000
>
>
> > >
> > > Make sure "pin_name" is explicitly placed in the .data section
> > > instead of .bss to fix this.
> > >
> > > Cc: Ramon Fried <rfried....@gmail.com>
> > > Signed-off-by: Stephan Gerhold <step...@gerhold.net>
> > > ---
> > >
> > > arch/arm/mach-snapdragon/pinctrl-apq8016.c | 3 +--
> > > arch/arm/mach-snapdragon/pinctrl-apq8096.c | 2 +-
> > > 2 files changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c
> > > b/arch/arm/mach-snapdragon/pinctrl-apq8016.c
> > > index 1042b564c3..70c0be0bca 100644
> > > --- a/arch/arm/mach-snapdragon/pinctrl-apq8016.c
> > > +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c
> > > @@ -10,7 +10,7 @@
> > > #include <common.h>
> > >
> > > #define MAX_PIN_NAME_LEN 32
> > > -static char pin_name[MAX_PIN_NAME_LEN];
> > > +static char pin_name[MAX_PIN_NAME_LEN] __section(".data");
> > > static const char * const msm_pinctrl_pins[] = {
> > > "SDC1_CLK",
> > > "SDC1_CMD",
> > > @@ -59,4 +59,3 @@ struct msm_pinctrl_data apq8016_data = {
> > > .get_function_mux = apq8016_get_function_mux,
> > > .get_pin_name = apq8016_get_pin_name,
> > > };
> > > -
> > > diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8096.c
> > > b/arch/arm/mach-snapdragon/pinctrl-apq8096.c
> > > index 20a71c319b..45462f01c2 100644
> > > --- a/arch/arm/mach-snapdragon/pinctrl-apq8096.c
> > > +++ b/arch/arm/mach-snapdragon/pinctrl-apq8096.c
> > > @@ -10,7 +10,7 @@
> > > #include <common.h>
> > >
> > > #define MAX_PIN_NAME_LEN 32
> > > -static char pin_name[MAX_PIN_NAME_LEN];
> > > +static char pin_name[MAX_PIN_NAME_LEN] __section(".data");
> > > static const char * const msm_pinctrl_pins[] = {
> > > "SDC1_CLK",
> > > "SDC1_CMD",
> > > --
> > > 2.32.0
> > >
> > Reviewed-by: Ramon Fried <rfried....@gmail.com>
