[U-Boot] [PATCH] ARM: socfpga: Convert to DM serial

Marek Vasut marex at denx.de
Fri Aug 3 23:14:27 UTC 2018


On 08/03/2018 10:24 PM, Simon Goldschmidt wrote:
> 
> 
> On 03.08.2018 13:01, Simon Goldschmidt wrote:
>>
>>
>> Marek Vasut <marex at denx.de <mailto:marex at denx.de>> schrieb am Fr., 3.
>> Aug. 2018, 11:22:
>>
>>     On 08/03/2018 11:10 AM, Simon Goldschmidt wrote:
>>      >
>>      >
>>      > Marek Vasut <marex at denx.de <mailto:marex at denx.de>
>>     <mailto:marex at denx.de <mailto:marex at denx.de>>> schrieb am Fr., 3.
>>      > Aug. 2018, 11:00:
>>      >
>>      >     On 08/03/2018 10:40 AM, Simon Goldschmidt wrote:
>>      >     >
>>      >     >
>>      >     > On 01.08.2018 09:34, Marek Vasut wrote:
>>      >     >> On 08/01/2018 09:29 AM, Goldschmidt Simon wrote:
>>      >     >>>
>>      >     >>> On 30.07.2018 16:04, Marek Vasut wrote:
>>      >     >>>> On 07/30/2018 04:03 PM, Simon Goldschmidt wrote:
>>      >     >>>>>
>>      >     >>>>> On 12.05.2018 22:28, Marek Vasut wrote:
>>      >     >>>>>> Pull the serial port configuration from DT and use DM
>>     serial
>>      >     instead
>>      >     >>>>>> of having the serial configuration in two places, DT
>>     and board
>>      >     >>>>>> config.
>>      >     >>>>>>
>>      >     >>>>>> Signed-off-by: Marek Vasut <marex at denx.de
>>     <mailto:marex at denx.de> <mailto:marex at denx.de <mailto:marex at denx.de>>>
>>      >     >>>>>> Cc: Chin Liang See <chin.liang.see at intel.com
>>     <mailto:chin.liang.see at intel.com>
>>      >     <mailto:chin.liang.see at intel.com
>>     <mailto:chin.liang.see at intel.com>>>
>>      >     >>>>>> Cc: Dinh Nguyen <dinguyen at kernel.org
>>     <mailto:dinguyen at kernel.org>
>>      >     <mailto:dinguyen at kernel.org <mailto:dinguyen at kernel.org>>>
>>      >     >>>>>> ---
>>      >     >>>>>>     arch/arm/Kconfig                             | 3
>> +++
>>      >     >>>>>>     arch/arm/dts/socfpga.dtsi                    | 2 ++
>>      >     >>>>>>     arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 1 +
>>      >     >>>>>>     include/configs/socfpga_common.h             | 8
>>     --------
>>      >     >>>>>>     4 files changed, 6 insertions(+), 8 deletions(-)
>>      >     >>>>>>
>>      >     >>>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>      >     >>>>>> index 532aa41a87..2012ac6410 100644
>>      >     >>>>>> --- a/arch/arm/Kconfig
>>      >     >>>>>> +++ b/arch/arm/Kconfig
>>      >     >>>>>> @@ -737,6 +737,7 @@ config ARCH_SOCFPGA
>>      >     >>>>>>         select ARCH_MISC_INIT
>>      >     >>>>>>         select CPU_V7A
>>      >     >>>>>>         select DM
>>      >     >>>>>> +    select DM_SERIAL
>>      >     >>>>>>         select ENABLE_ARM_SOC_BOOT0_HOOK
>>      >     >>>>>>         select OF_CONTROL
>>      >     >>>>>>         select SPL_LIBCOMMON_SUPPORT
>>      >     >>>>>> @@ -746,11 +747,13 @@ config ARCH_SOCFPGA
>>      >     >>>>>>         select SPL_NAND_SUPPORT if SPL_NAND_DENALI
>>      >     >>>>>>         select SPL_OF_CONTROL
>>      >     >>>>>>         select SPL_SERIAL_SUPPORT
>>      >     >>>>>> +    select SPL_DM_SERIAL
>>      >     >>>>>>         select SPL_SPI_FLASH_SUPPORT if SPL_SPI_SUPPORT
>>      >     >>>>>>         select SPL_SPI_SUPPORT if DM_SPI
>>      >     >>>>>>         select SPL_WATCHDOG_SUPPORT
>>      >     >>>>>>         select SUPPORT_SPL
>>      >     >>>>>>         select
>>     SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
>>      >     >>>>>> +    select SYS_NS16550
>>      >     >>>>>>         select SYS_THUMB_BUILD
>>      >     >>>>>>         imply CMD_MTDPARTS
>>      >     >>>>>>         imply CRC32_VERIFY
>>      >     >>>>>> diff --git a/arch/arm/dts/socfpga.dtsi
>>      >     b/arch/arm/dts/socfpga.dtsi
>>      >     >>>>>> index e64127fcb2..314449478d 100644
>>      >     >>>>>> --- a/arch/arm/dts/socfpga.dtsi
>>      >     >>>>>> +++ b/arch/arm/dts/socfpga.dtsi
>>      >     >>>>>> @@ -737,6 +737,7 @@
>>      >     >>>>>>                 reg-shift = <2>;
>>      >     >>>>>>                 reg-io-width = <4>;
>>      >     >>>>>>                 clocks = <&l4_sp_clk>;
>>      >     >>>>>> +            clock-frequency = <100000000>;
>>      >     >>>>>>             };
>>      >     >>>>>>               uart1: serial1 at ffc03000 {
>>      >     >>>>>> @@ -746,6 +747,7 @@
>>      >     >>>>>>                 reg-shift = <2>;
>>      >     >>>>>>                 reg-io-width = <4>;
>>      >     >>>>>>                 clocks = <&l4_sp_clk>;
>>      >     >>>>>> +            clock-frequency = <100000000>;
>>      >     >>>>>>             };
>>      >     >>>>>>               rst: rstmgr at ffd05000 {
>>      >     >>>>>> diff --git
>> a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
>>      >     >>>>>> b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
>>      >     >>>>>> index b573d0e658..06b61cb0af 100644
>>      >     >>>>>> --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
>>      >     >>>>>> +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
>>      >     >>>>>> @@ -24,6 +24,7 @@
>>      >     >>>>>>     };
>>      >     >>>>>>       &uart1 {
>>      >     >>>>>> +    clock-frequency = <50000000>;
>>      >     >>>>>>         u-boot,dm-pre-reloc;
>>      >     >>>>>>         status = "okay";
>>      >     >>>>>>     };
>>      >     >>>>>> diff --git a/include/configs/socfpga_common.h
>>      >     >>>>>> b/include/configs/socfpga_common.h
>>      >     >>>>>> index 54b9edc97c..a60da85499 100644
>>      >     >>>>>> --- a/include/configs/socfpga_common.h
>>      >     >>>>>> +++ b/include/configs/socfpga_common.h
>>      >     >>>>>> @@ -173,14 +173,6 @@ unsigned int
>>      >     >>>>>> cm_get_qspi_controller_clk_hz(void);
>>      >     >>>>>>      * Serial Driver
>>      >     >>>>>>      */
>>      >     >>>>>>     #define CONFIG_SYS_NS16550_SERIAL
>>      >     >>>>>> -#define CONFIG_SYS_NS16550_REG_SIZE    -4
>>      >     >>>>>> -#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
>>      >     >>>>>> -#define CONFIG_SYS_NS16550_COM1          
>> SOCFPGA_UART0_ADDRESS
>>      >     >>>>>> -#define CONFIG_SYS_NS16550_CLK        100000000
>>      >     >>>>>> -#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
>>      >     >>>>>> -#define CONFIG_SYS_NS16550_COM1          
>> SOCFPGA_UART1_ADDRESS
>>      >     >>>>>> -#define CONFIG_SYS_NS16550_CLK        50000000
>>      >     >>>>>> -#endif
>>      >     >>>>>>       /*
>>      >     >>>>>>      * USB
>>      >     >>>>>>
>>      >     >>>>> Unfortunately I saw this just now, but it seems this
>>     breaks GEN5
>>      >     >>>>> SPL? At
>>      >     >>>>> least git-bisect told me that
>>      >     73172753f4f3351ed1c9d2f6586fc599ce4e728c
>>      >     >>>>> is the first bad commit.
>>      >     >>>>>
>>      >     >>>>> I tested socfpga_socrates_defconfig on my socrates
>> board.
>>      >     >>>>>
>>      >     >>>>> Any idea what's wrong there?
>>      >     >>>> Nope, this should work fine. Can you investigate ?
>>      >     >>>
>>      >     >>> Ok, so after adding "u-boot,dm-pre-reloc" to uart0 in
>>      >     >>> socfpga_cyclone5_socrates.dts, U-Boot works (combined
>> with an
>>      >     old SPL).
>>      >     >>
>>      >     >> Good!
>>      >     >>
>>      >     >>> SPL still does not work. Any idea? How does SPL get the
>> uart?
>>      >     >>>
>>      >     >>> Thanks for any pointers.
>>      >     >>
>>      >     >> Dig around the NS16550 driver .
>>      >     > OK, so my DS-5 debugger shows me it's not the driver that
>>     fails but
>>      >     > "malloc_simple()" returns 0x0 when being called from
>>      >     > uclass_get_device_by_seq(). Which is strange because
>>      >     gd->malloc_base is
>>      >     > 0xffffe000.
>>      >
>>      >     malloc() returns NULL because it ran out of space ?
>>      >
>>      >
>>      > No, the malloc limit is not even set in gd.
>>      > It just somehow fails to add malloc_base. I don't get why, as it
>>     should
>>      > do so given the disassembly.
>>      >
>>      > The next step is then zeroing the memory from calloc() and it
>> doesn't
>>      > return from there.
>>      >
>>      > I guess next I could check if malloc_simple is called at all
>> without
>>      > serial DM. That would at least explain how this error is
>> connected to
>>      > your commit...
>>
>>     Maybe it's not, I recall that the SPL sometimes failed when it
>> grew too
>>     large and noone explained in a satisfactory way why. Maybe the commit
>>     just triggers this.
>>
>>
>> I know. That's why I just tested with a smaller config where I disable
>> spi and spi-flash drivers. The SPL binary is now only 40kbyte but the
>> error remains the same.
>>
>> I wonder if the cause might be that now malloc_simple() is called
>> earlier than before?
> 
> And this was the right direction: malloc_simple just did not work at
> that time in the boot. But after fixing this (patches pending) other
> things (e.g. in dm core) did not work.
> 
> The solution was to call 'spl_early_init()' from socfpga's
> 'boart_init_f()', which not only correctly inits the heap but also inits
> the dm code.
> 
> Now the SPL is working for me, too! :-)
> 
> I'll have to prepare patches on top of the current git master though:
> for now, I have only tested changes on this patch.
> 
> I really don't know how this could have worked for other socfpga boards,
> but I saw the SPL split coming later after this patch, and I must say I
> don't think it's such a good idea. I'd prefer if all the socfpga
> platforms could stay with one SPL. That would greatly reduce that chance
> of old platforms getting broken like this.

I agree, then again the Arria10 has quite different needs in SPL and
than the Gen5 . Obviously, if the differences could be somehow pushed
into drivers, that'd be nice.

-- 
Best regards,
Marek Vasut


More information about the U-Boot mailing list