Re: [PATCH v1] ARM: tegra: Fix misplaced tegra_uart_config in decompressor

[Dmitry Osipenko]

15.12.2020 22:20, Florian Fainelli пишет:
> 
> 
> On 12/15/2020 10:56 AM, Dmitry Osipenko wrote:
>> 15.12.2020 21:22, Florian Fainelli пишет:
>>>
>>>
>>> On 12/15/2020 8:53 AM, Dmitry Osipenko wrote:
>>>> 15.12.2020 19:40, Florian Fainelli пишет:
>>>>>
>>>>>
>>>>> On 12/15/2020 8:17 AM, Dmitry Osipenko wrote:
>>>>>> 15.12.2020 19:04, Florian Fainelli пишет:
>>>>>>>
>>>>>>>
>>>>>>> On 12/15/2020 5:52 AM, Dmitry Osipenko wrote:
>>>>>>>> The tegra_uart_config of the DEBUG_LL code is now placed right at the
>>>>>>>> start of the .text section after commit which enabled debug output in the
>>>>>>>> decompressor. Tegra devices are not booting anymore if DEBUG_LL is enabled
>>>>>>>> since tegra_uart_config data is executes as a code. Fix the misplaced
>>>>>>>> tegra_uart_config storage by embedding it into the code.
>>>>>>>>
>>>>>>>> Cc: stable@xxxxxxxxxxxxxxx
>>>>>>>> Fixes: 2596a72d3384 ("ARM: 9009/1: uncompress: Enable debug in head.S")
>>>>>>>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
>>>>>>>> ---
>>>>>>>>  arch/arm/include/debug/tegra.S | 54 +++++++++++++++++-----------------
>>>>>>>>  1 file changed, 27 insertions(+), 27 deletions(-)
>>>>>>>
>>>>>>> Looks like arch/arm/include/debug/brcmstb.S would need the same
>>>>>>> treatment since the implementation was copied from tegra.S.
>>>>>>>
>>>>>>
>>>>>> Good catch, will you be able to test the brcm and make a patch?
>>>>>
>>>>> Yes, absolutely, building a kernel to test right now.
>>>>>
>>>>
>>>> Thank you.
>>>>
>>>> BTW, I noticed that the problem is more visible on a thumb2 kernel
>>>> build, i.e. you should get a more reliable hang on boot. On a non-thumb2
>>>> kernel the hanging behaviour seems depends on a device / bootloader. I
>>>> haven't tried to figure out what exactly makes the difference, perhaps
>>>> it should be a memory layout / state.
>>>
>>> To build with a CONFIG_THUMB2_KERNEL I had to fetch:
>>>
>>> https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9018/2
>>>
>>> to avoid a build error, too bad this missed v5.10 final but hopefully it
>>> can make it soon.
>>
>> The VFP fix was applied to the -next very recently, it should propagate
>> to v5.10 eventually.
>>
>>> With CONFIG_THUMB2_KERNEL=y, I am not getting the head.S output where it
>>> prints the start/end of the compressed kernel:
>>>
>>> C:0x420800C0-0x4321B0E0->0x4212AB00-0x432C5B20
>>> Uncompressing Linux... done, booting the kernel.
>>> [    0.000000] Booting Linux on physical CPU 0x0
>>> [    0.000000] Linux version 5.10.0-g148842c98a24
>>> (fainelli@fainelli-desktop) (arm-linux-gcc (GCC) 8.3.0, GNU ld (GNU
>>> Binutils) 2.32) #71 SMP Tue Dec 15 09:53:09 PST 2020
>>>
>>> I am only getting:
>>>
>>> Uncompressing Linux... done, booting the kernel.
>>>
>>> Is that the same for you?
>>
>> No, start/end are printed for both THUMB2 and ARM kernels here.
>>
>>> Looking at the disassembly of head.o it definitively has
>>> brcmstb_uart_config in the .text section as the beginning just like you
>>> mentioned in your commit message.
>>>
>>> Disassembly of section .text:
>>>
>>> 00000000 <brcmstb_uart_config>:
>>>    0:   00000001        andeq   r0, r0, r1
>>>         ...
>>>    c:   467c            mov     r4, pc
>>>    e:   f004 4478       and.w   r4, r4, #4160749568     ; 0xf8000000
>>>   12:   f504 4400       add.w   r4, r4, #32768  ; 0x8000
>>>   16:   4678            mov     r0, pc
>>>   18:   42a0            cmp     r0, r4
>>>   1a:   bf3f            itttt   cc
>>>   1c:   48d4            ldrcc   r0, [pc, #848]  ; (370 <LC1+0x8>)
>>>   1e:   4478            addcc   r0, pc
>>>   20:   4284            cmpcc   r4, r0
>>>   22:   f044 0401       orrcc.w r4, r4, #1
>>>   26:   bf28            it      cs
>>>   28:   f000 f9aa       blcs    380 <cache_on>
>>>
>>> however after applying a fix similar to yours, we do end-up with the
>>> expected data embedded within the code and given brcmstb.S would be
>>> subject to the same issue as tegra.S, it would not hurt.
>>>
>>
>> Have you checked whether start/end printed after applying the fix?
> 
> Yes I did, it is not printed when CONFIG_THUMB2_KERNEL=y, but it is when
> CONFIG_THUMB2_KERNEL=n. I don't have a JTAG adapter on this board right
> now to single step.
> 

Okay, I don't know what's wrong there.

I also noticed that brcmstb doesn't check whether UART address is 0x0 in
senduart / busyuart, perhaps this needs to be fixed too.