[PATCH] arm: mach-snapdragon: pinctrl: Place pin_name in .data section

Jorge Ramirez-Ortiz, Gmail jorge.ramirez.ortiz at gmail.com
Thu Jul 15 09:24:48 CEST 2021


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 <stephan at 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 at 74a4900: 0                                                                                
Loading Environment from EXT4... MMC: no card present                                                  
In:    serial at 75b0000                                                                                  
Out:   serial at 75b0000                                                                                  
Err:   serial at 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.dev at gmail.com>
> > > Signed-off-by: Stephan Gerhold <stephan at 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.dev at gmail.com>


More information about the U-Boot mailing list