diff mbox

[00/16] Refactor the architecture parts of mt7628

Message ID 8f80b516-d402-1f84-5dd1-487883b33f64@denx.de
State New
Headers show

Commit Message

Stefan Roese Jan. 9, 2020, 12:36 p.m. UTC
Hi Weijie,

On 09.01.20 09:49, Weijie Gao wrote:
> On Thu, 2020-01-09 at 08:52 +0100, Stefan Roese wrote:
>> Hi Weijie,
>>
>> On 08.01.20 03:59, Weijie Gao wrote:
>>> This patch series are divided into two parts:
>>>
>>> The main part is to rewrite the whole architecture code of mt7628:
>>> 1. Lock parts of the d-cache for initial stack so the rest of the code can
>>>      be reimplemented in C.
>>> 2. Memory controller & DDR initialization have been fully written to support
>>>      detecting DDR size automatically.
>>> 3. DDR calibration has also been reimplemented with a clear logic.
>>> 4. Implemented a new sysreset driver to take advantage of the reset
>>>      controller so we can drop the use of syscon-based sysreset to reduce size.
>>>
>>> The second part is to add SPL support for mt7628:
>>> 1. With SPL enabled we can build the ROM-bootable and RAM-bootable binary
>>>      simultaneously, and we can drop RAM boot related configs and defconfig
>>>      files.
>>> 2. Generate compressed u-boot.bin image for SPL to reduce size of final
>>>      combined binary.
>>> 3. Add a demo board (mt7628_rfb) to make use of the compressed image.
>>
>> Many thanks for working on this. Unfortunately I didn't receive these patches
>> directly (I remember that you had problems sending patches to me before).
>>
>> I did a quick test on the LinkIt board and have a few questions before
>> I start reviewing the patches in more detail:
>>
>> I don't see any serial output from the SPL. I assume that the muxing
>> for UART2 is missing here in the SPL image (see board_debug_uart_init)?
> 
> I'll do further test for this.

I also did some tests and here is a WIP patch that fixes this issue for
the LinkIt board. Please feel free to integrate it into your next patch
set version:


  
>>
>> Did you compare the boot speed with using SPL + main U-Boot vs U-Boot
>> only?
> 
> As far as I tested the difference of boot time should be less than 1s.

1 second is quite a lot of time. I prefer to stay with the non compressed
boot configuration then. Testing shows that the new SPL based version
also boots pretty fast, when the main U-Boot proper is not compressed.
  
>>
>> In the current combined image the non-compressed U-Boot proper is
>> included. Why did you not include the LZMA compressed image per default?
>> Is the decompression too slow?
> 
> I just kept these two boards untouched. I can switch them to compressed
> mode if you have no concern.

I did not test yet. If its really a difference in time of 1 second (or
similar), then let stay with uncompressed for these board please.

Thanks,
Stefan

Comments

Weijie Gao Jan. 10, 2020, 10:48 a.m. UTC | #1
On Thu, 2020-01-09 at 13:36 +0100, Stefan Roese wrote:
> Hi Weijie,
> 
> On 09.01.20 09:49, Weijie Gao wrote:
> > On Thu, 2020-01-09 at 08:52 +0100, Stefan Roese wrote:
> >> Hi Weijie,
> >>
> >> On 08.01.20 03:59, Weijie Gao wrote:
> >>> This patch series are divided into two parts:
> >>>
> >>> The main part is to rewrite the whole architecture code of mt7628:
> >>> 1. Lock parts of the d-cache for initial stack so the rest of the code can
> >>>      be reimplemented in C.
> >>> 2. Memory controller & DDR initialization have been fully written to support
> >>>      detecting DDR size automatically.
> >>> 3. DDR calibration has also been reimplemented with a clear logic.
> >>> 4. Implemented a new sysreset driver to take advantage of the reset
> >>>      controller so we can drop the use of syscon-based sysreset to reduce size.
> >>>
> >>> The second part is to add SPL support for mt7628:
> >>> 1. With SPL enabled we can build the ROM-bootable and RAM-bootable binary
> >>>      simultaneously, and we can drop RAM boot related configs and defconfig
> >>>      files.
> >>> 2. Generate compressed u-boot.bin image for SPL to reduce size of final
> >>>      combined binary.
> >>> 3. Add a demo board (mt7628_rfb) to make use of the compressed image.
> >>
> >> Many thanks for working on this. Unfortunately I didn't receive these patches
> >> directly (I remember that you had problems sending patches to me before).
> >>
> >> I did a quick test on the LinkIt board and have a few questions before
> >> I start reviewing the patches in more detail:
> >>
> >> I don't see any serial output from the SPL. I assume that the muxing
> >> for UART2 is missing here in the SPL image (see board_debug_uart_init)?
> > 
> > I'll do further test for this.
> 
> I also did some tests and here is a WIP patch that fixes this issue for
> the LinkIt board. Please feel free to integrate it into your next patch
> set version:
> 
> diff --git a/arch/mips/mach-mtmips/spl.c b/arch/mips/mach-mtmips/spl.c
> index 37172abadf..33b4485be2 100644
> --- a/arch/mips/mach-mtmips/spl.c
> +++ b/arch/mips/mach-mtmips/spl.c
> @@ -7,10 +7,25 @@
>   
>   #include <common.h>
>   #include <spl.h>
> +#include <asm/io.h>
> +
> +#define MT76XX_GPIO1_MODE      0x10000060
> +
> +static void uart2_mux_init(void)
> +{
> +       void __iomem *gpio_mode;
> +
> +       /* Select UART2 mode instead of GPIO mode (default) */
> +       gpio_mode = ioremap_nocache(MT76XX_GPIO1_MODE, 0x100);
> +       clrbits_le32(gpio_mode, GENMASK(27, 26));
> +}
>   
>   void __noreturn board_init_f(ulong dummy)
>   {
>   #ifdef CONFIG_SPL_SERIAL_SUPPORT
> +#if CONFIG_CONS_INDEX == 3
> +       uart2_mux_init();
> +#endif
>          preloader_console_init();
>   #endif
>   
> diff --git a/include/configs/linkit-smart-7688.h b/include/configs/linkit-smart-7688.h
> index 01705ee2a5..b4bba9dcc5 100644
> --- a/include/configs/linkit-smart-7688.h
> +++ b/include/configs/linkit-smart-7688.h
> @@ -32,8 +32,10 @@
>   #define CONFIG_SYS_NS16550_MEM32
>   #define CONFIG_SYS_NS16550_CLK         40000000
>   #define CONFIG_SYS_NS16550_REG_SIZE    -4
> -#define CONFIG_SYS_NS16550_COM1                0xb0000e00
> -#define CONFIG_CONS_INDEX              1
> +/* Console index starts with 1 -> UART2 = IDX 3 */
> +#define CONFIG_SYS_NS16550_COM3                0xb0000e00
> +#undef CONFIG_CONS_INDEX
> +#define CONFIG_CONS_INDEX              3
>   #endif
>   
>   /* UART */
> 
>   
> >>
> >> Did you compare the boot speed with using SPL + main U-Boot vs U-Boot
> >> only?
> > 
> > As far as I tested the difference of boot time should be less than 1s.
> 
> 1 second is quite a lot of time. I prefer to stay with the non compressed
> boot configuration then. Testing shows that the new SPL based version
> also boots pretty fast, when the main U-Boot proper is not compressed.
>   
> >>
> >> In the current combined image the non-compressed U-Boot proper is
> >> included. Why did you not include the LZMA compressed image per default?
> >> Is the decompression too slow?
> > 
> > I just kept these two boards untouched. I can switch them to compressed
> > mode if you have no concern.
> 
> I did not test yet. If its really a difference in time of 1 second (or
> similar), then let stay with uncompressed for these board please.
> 
> Thanks,
> Stefan


I did a precise measurement, and this is the result:

type       | spl load/decompression time | boot to shell (total time)
spl + lzma | 75ms                        | 563ms
spl + none | 139ms                       | 627ms
non-spl    | N/A                         | 626ms

Actually spl+lzma is the fastest. I think this is because it has the
smallest binary size.

It's OK to switch all of them to compressed mode.

Best Regards,
Stefan Roese Jan. 10, 2020, 1:13 p.m. UTC | #2
Hi Weijie,

On 10.01.20 11:48, Weijie Gao wrote:

<snip>

>>>> Did you compare the boot speed with using SPL + main U-Boot vs U-Boot
>>>> only?
>>>
>>> As far as I tested the difference of boot time should be less than 1s.
>>
>> 1 second is quite a lot of time. I prefer to stay with the non compressed
>> boot configuration then. Testing shows that the new SPL based version
>> also boots pretty fast, when the main U-Boot proper is not compressed.
>>    
>>>>
>>>> In the current combined image the non-compressed U-Boot proper is
>>>> included. Why did you not include the LZMA compressed image per default?
>>>> Is the decompression too slow?
>>>
>>> I just kept these two boards untouched. I can switch them to compressed
>>> mode if you have no concern.
>>
>> I did not test yet. If its really a difference in time of 1 second (or
>> similar), then let stay with uncompressed for these board please.
>>
>> Thanks,
>> Stefan
> 
> 
> I did a precise measurement, and this is the result:
> 
> type       | spl load/decompression time | boot to shell (total time)
> spl + lzma | 75ms                        | 563ms
> spl + none | 139ms                       | 627ms
> non-spl    | N/A                         | 626ms

Great. Thanks for doing these tests and reporting here.
  
> Actually spl+lzma is the fastest. I think this is because it has the
> smallest binary size.

Makes sense. SPI NOR load time savage is bigger than the added
decompression time.
  
> It's OK to switch all of them to compressed mode.

Yes, please do so. This might be a nice boot time increase for other
boards as well. I'll do some tests on i.MX6 soon. :)

Thanks,
Stefan
diff mbox

Patch

diff --git a/arch/mips/mach-mtmips/spl.c b/arch/mips/mach-mtmips/spl.c
index 37172abadf..33b4485be2 100644
--- a/arch/mips/mach-mtmips/spl.c
+++ b/arch/mips/mach-mtmips/spl.c
@@ -7,10 +7,25 @@ 
  
  #include <common.h>
  #include <spl.h>
+#include <asm/io.h>
+
+#define MT76XX_GPIO1_MODE      0x10000060
+
+static void uart2_mux_init(void)
+{
+       void __iomem *gpio_mode;
+
+       /* Select UART2 mode instead of GPIO mode (default) */
+       gpio_mode = ioremap_nocache(MT76XX_GPIO1_MODE, 0x100);
+       clrbits_le32(gpio_mode, GENMASK(27, 26));
+}
  
  void __noreturn board_init_f(ulong dummy)
  {
  #ifdef CONFIG_SPL_SERIAL_SUPPORT
+#if CONFIG_CONS_INDEX == 3
+       uart2_mux_init();
+#endif
         preloader_console_init();
  #endif
  
diff --git a/include/configs/linkit-smart-7688.h b/include/configs/linkit-smart-7688.h
index 01705ee2a5..b4bba9dcc5 100644
--- a/include/configs/linkit-smart-7688.h
+++ b/include/configs/linkit-smart-7688.h
@@ -32,8 +32,10 @@ 
  #define CONFIG_SYS_NS16550_MEM32
  #define CONFIG_SYS_NS16550_CLK         40000000
  #define CONFIG_SYS_NS16550_REG_SIZE    -4
-#define CONFIG_SYS_NS16550_COM1                0xb0000e00
-#define CONFIG_CONS_INDEX              1
+/* Console index starts with 1 -> UART2 = IDX 3 */
+#define CONFIG_SYS_NS16550_COM3                0xb0000e00
+#undef CONFIG_CONS_INDEX
+#define CONFIG_CONS_INDEX              3
  #endif
  
  /* UART */