Re: [U-Boot] [PATCH 01/22] tools: imx8m_image: align spl bin image size

2019-08-14 Thread Peng Fan
Hi Frieder

> -Original Message-
> From: Schrempf Frieder 
> Sent: 2019年8月14日 15:22
> To: Peng Fan ; lu...@denx.de; sba...@denx.de;
> feste...@gmail.com
> Cc: dl-uboot-imx ; u-boot@lists.denx.de
> Subject: Re: [PATCH 01/22] tools: imx8m_image: align spl bin image size
> 
> Hi Peng,
> 
> On 09.08.19 06:14, Peng Fan wrote:
> > Align spl bin image size to 4 byte aligned, because we need to pad ddr
> > firmware in the end of spl bin. However when enable SPL OF, the spl
> > dtb will be padded to u-boot-nodtb.bin, then u-boot-spl.bin size might
> > not be 4 bytes aligned.
> >
> > ddr_load_train_firmware in drivers/ddr/imx/imx8m/helper.c use 4 bytes
> > aligned address to load ddr firmware, so we need make sure
> > u-boot-spl.bin is 4 bytes aligned, in this patch we use dd to create a
> > new file named u-boot-spl-pad.bin, then pad ddr firmware. > If SPL OF
> > not enabled, this patch not hurt, because `_end` already is 4 bytes
> > aligned.
> 
> Can you please try to make full English sentences at least? Like "subject verb
> object": If SPL *is* not enabled, this patch *does* not hurt, ...
> 

I'll try.

> Also your explanations are very difficult to understand for someone who will
> read the commit message. 

Sorry. My English is not that good.

I would have used something like this:
> 
> """
> The loader for the DDR firmware in drivers/ddr/imx/imx8m/helper.c uses a
> 4-byte-aligned address to load the firmware. In cases where OF is enabled in
> SPL the dtb will be appended to the SPL binary and can result in a binary that
> is not aligned correctly. If OF is not enabled in SPL, `_end` is already 
> aligned
> correctly, but this patch does not hurt.
> 
> To ensure the correct alignment we use dd to create a temporary file
> u-boot-spl-pad.bin with the correct padding.
> """

Thanks for help improve the commit message.

Thanks,
Peng.

> 
> >
> > Signed-off-by: Peng Fan 
> 
> With a better commit message:
> 
> Reviewed-by: Frieder Schrempf 
> Tested-by: Frieder Schrempf 
> 
> Thanks,
> Frieder
> 
> > ---
> >   tools/imx8m_image.sh | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/imx8m_image.sh b/tools/imx8m_image.sh index
> > ec0881a128..08a6a48180 100755
> > --- a/tools/imx8m_image.sh
> > +++ b/tools/imx8m_image.sh
> > @@ -35,8 +35,9 @@ if [ $post_process = 1 ]; then
> > objcopy -I binary -O binary --pad-to 0x8000 --gap-fill=0x0
> $srctree/lpddr4_pmu_train_2d_imem.bin
> lpddr4_pmu_train_2d_imem_pad.bin
> > cat lpddr4_pmu_train_1d_imem_pad.bin
> lpddr4_pmu_train_1d_dmem_pad.bin > lpddr4_pmu_train_1d_fw.bin
> > cat lpddr4_pmu_train_2d_imem_pad.bin
> $srctree/lpddr4_pmu_train_2d_dmem.bin > lpddr4_pmu_train_2d_fw.bin
> > -   cat spl/u-boot-spl.bin lpddr4_pmu_train_1d_fw.bin
> lpddr4_pmu_train_2d_fw.bin > spl/u-boot-spl-ddr.bin
> > -   rm -f lpddr4_pmu_train_1d_fw.bin lpddr4_pmu_train_2d_fw.bin
> lpddr4_pmu_train_1d_imem_pad.bin lpddr4_pmu_train_1d_dmem_pad.bin
> lpddr4_pmu_train_2d_imem_pad.bin
> > +   dd if=spl/u-boot-spl.bin of=spl/u-boot-spl-pad.bin bs=4 
> > conv=sync
> > +   cat spl/u-boot-spl-pad.bin lpddr4_pmu_train_1d_fw.bin
> lpddr4_pmu_train_2d_fw.bin > spl/u-boot-spl-ddr.bin
> > +   rm -f lpddr4_pmu_train_1d_fw.bin lpddr4_pmu_train_2d_fw.bin
> > +lpddr4_pmu_train_1d_imem_pad.bin
> lpddr4_pmu_train_1d_dmem_pad.bin
> > +lpddr4_pmu_train_2d_imem_pad.bin spl/u-boot-spl-pad.bin
> > fi
> >   fi
> >
> >
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 01/22] tools: imx8m_image: align spl bin image size

2019-08-14 Thread Schrempf Frieder
Hi Peng,

On 09.08.19 06:14, Peng Fan wrote:
> Align spl bin image size to 4 byte aligned, because we need
> to pad ddr firmware in the end of spl bin. However when enable
> SPL OF, the spl dtb will be padded to u-boot-nodtb.bin, then
> u-boot-spl.bin size might not be 4 bytes aligned.
> 
> ddr_load_train_firmware in drivers/ddr/imx/imx8m/helper.c use 4 bytes
> aligned address to load ddr firmware, so we need make sure
> u-boot-spl.bin is 4 bytes aligned, in this patch we use dd
> to create a new file named u-boot-spl-pad.bin, then pad ddr firmware. >
> If SPL OF not enabled, this patch not hurt, because `_end` already
> is 4 bytes aligned.

Can you please try to make full English sentences at least? Like 
"subject verb object": If SPL *is* not enabled, this patch *does* not 
hurt, ...

Also your explanations are very difficult to understand for someone who 
will read the commit message. I would have used something like this:

"""
The loader for the DDR firmware in drivers/ddr/imx/imx8m/helper.c uses a 
4-byte-aligned address to load the firmware. In cases where OF is 
enabled in SPL the dtb will be appended to the SPL binary and can result 
in a binary that is not aligned correctly. If OF is not enabled in SPL, 
`_end` is already aligned correctly, but this patch does not hurt.

To ensure the correct alignment we use dd to create a temporary file 
u-boot-spl-pad.bin with the correct padding.
"""

> 
> Signed-off-by: Peng Fan 

With a better commit message:

Reviewed-by: Frieder Schrempf 
Tested-by: Frieder Schrempf 

Thanks,
Frieder

> ---
>   tools/imx8m_image.sh | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/imx8m_image.sh b/tools/imx8m_image.sh
> index ec0881a128..08a6a48180 100755
> --- a/tools/imx8m_image.sh
> +++ b/tools/imx8m_image.sh
> @@ -35,8 +35,9 @@ if [ $post_process = 1 ]; then
>   objcopy -I binary -O binary --pad-to 0x8000 --gap-fill=0x0 
> $srctree/lpddr4_pmu_train_2d_imem.bin lpddr4_pmu_train_2d_imem_pad.bin
>   cat lpddr4_pmu_train_1d_imem_pad.bin 
> lpddr4_pmu_train_1d_dmem_pad.bin > lpddr4_pmu_train_1d_fw.bin
>   cat lpddr4_pmu_train_2d_imem_pad.bin 
> $srctree/lpddr4_pmu_train_2d_dmem.bin > lpddr4_pmu_train_2d_fw.bin
> - cat spl/u-boot-spl.bin lpddr4_pmu_train_1d_fw.bin 
> lpddr4_pmu_train_2d_fw.bin > spl/u-boot-spl-ddr.bin
> - rm -f lpddr4_pmu_train_1d_fw.bin lpddr4_pmu_train_2d_fw.bin 
> lpddr4_pmu_train_1d_imem_pad.bin lpddr4_pmu_train_1d_dmem_pad.bin 
> lpddr4_pmu_train_2d_imem_pad.bin
> + dd if=spl/u-boot-spl.bin of=spl/u-boot-spl-pad.bin bs=4 
> conv=sync
> + cat spl/u-boot-spl-pad.bin lpddr4_pmu_train_1d_fw.bin 
> lpddr4_pmu_train_2d_fw.bin > spl/u-boot-spl-ddr.bin
> + rm -f lpddr4_pmu_train_1d_fw.bin lpddr4_pmu_train_2d_fw.bin 
> lpddr4_pmu_train_1d_imem_pad.bin lpddr4_pmu_train_1d_dmem_pad.bin 
> lpddr4_pmu_train_2d_imem_pad.bin spl/u-boot-spl-pad.bin
>   fi
>   fi
>   
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 01/22] tools: imx8m_image: align spl bin image size

2019-08-13 Thread Peng Fan
Hi Lukasz,

> Subject: Re: [PATCH 01/22] tools: imx8m_image: align spl bin image size
> 
> Hi Peng,
> 
> > Align spl bin image size to 4 byte aligned, because we need to pad ddr
> > firmware in the end of spl bin. However when enable SPL OF, the spl
> > dtb will be padded to u-boot-nodtb.bin, then u-boot-spl.bin size might
> > not be 4 bytes aligned.
> >
> > ddr_load_train_firmware in drivers/ddr/imx/imx8m/helper.c
> 
> I've poked around and there is a U-Boot commit:
> e3963c0943042afcb38d99041a8dc3d55f092f5f
> 
> Which adds:
> "drivers: ddr: introduce DDR driver for i.MX8M"
> 
> Is there any documentation on how to use it (any ./doc/ entry)?
> 
> I guess that this code (from helper.c) helps with setting DDR4 controller
> properly.
> 
> However, patch 22/22 in this series - which adds EVK imx8m board
> - doesn't use this function, but it has hardcoded long table of magic numbers
> to setup DDR controller.
> 
> I guess that those magic numbers are created from running this code?

As we talked on IRC,
There is an external ddr phy firware padded to SPL, which will be loaded
by SPL. Those magic numbers are generated by an DDR TOOL released
by NXP, and SPL will use those arrays to init DDR.

Thanks,
Peng.

> 
> If yes - would it be possible to train/setup the DDR4 controller in SPL as it 
> is
> done on i.MX6Q? Or at least provide a detailed documentation on how to use
> it?
> 
> > use 4 bytes
> > aligned address to load ddr firmware, so we need make sure
> > u-boot-spl.bin is 4 bytes aligned, in this patch we use dd to create a
> > new file named u-boot-spl-pad.bin, then pad ddr firmware.
> >
> > If SPL OF not enabled, this patch not hurt, because `_end` already is
> > 4 bytes aligned.
> >
> > Signed-off-by: Peng Fan 
> > ---
> >  tools/imx8m_image.sh | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/imx8m_image.sh b/tools/imx8m_image.sh index
> > ec0881a128..08a6a48180 100755
> > --- a/tools/imx8m_image.sh
> > +++ b/tools/imx8m_image.sh
> > @@ -35,8 +35,9 @@ if [ $post_process = 1 ]; then
> > objcopy -I binary -O binary --pad-to 0x8000
> > --gap-fill=0x0 $srctree/lpddr4_pmu_train_2d_imem.bin
> > lpddr4_pmu_train_2d_imem_pad.bin cat
> lpddr4_pmu_train_1d_imem_pad.bin
> > lpddr4_pmu_train_1d_dmem_pad.bin > lpddr4_pmu_train_1d_fw.bin cat
> > lpddr4_pmu_train_2d_imem_pad.bin
> $srctree/lpddr4_pmu_train_2d_dmem.bin
> > > lpddr4_pmu_train_2d_fw.bin
> > -   cat spl/u-boot-spl.bin lpddr4_pmu_train_1d_fw.bin
> > lpddr4_pmu_train_2d_fw.bin > spl/u-boot-spl-ddr.bin
> > -   rm -f lpddr4_pmu_train_1d_fw.bin
> > lpddr4_pmu_train_2d_fw.bin lpddr4_pmu_train_1d_imem_pad.bin
> > lpddr4_pmu_train_1d_dmem_pad.bin
> lpddr4_pmu_train_2d_imem_pad.bin
> > +   dd if=spl/u-boot-spl.bin of=spl/u-boot-spl-pad.bin
> > bs=4 conv=sync
> > +   cat spl/u-boot-spl-pad.bin
> > lpddr4_pmu_train_1d_fw.bin lpddr4_pmu_train_2d_fw.bin >
> > spl/u-boot-spl-ddr.bin
> > +   rm -f lpddr4_pmu_train_1d_fw.bin
> > lpddr4_pmu_train_2d_fw.bin lpddr4_pmu_train_1d_imem_pad.bin
> > lpddr4_pmu_train_1d_dmem_pad.bin
> lpddr4_pmu_train_2d_imem_pad.bin
> > spl/u-boot-spl-pad.bin fi fi
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lu...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 01/22] tools: imx8m_image: align spl bin image size

2019-08-11 Thread Lukasz Majewski
Hi Peng,

> Align spl bin image size to 4 byte aligned, because we need
> to pad ddr firmware in the end of spl bin. However when enable
> SPL OF, the spl dtb will be padded to u-boot-nodtb.bin, then
> u-boot-spl.bin size might not be 4 bytes aligned.
> 
> ddr_load_train_firmware in drivers/ddr/imx/imx8m/helper.c 

I've poked around and there is a U-Boot commit:
e3963c0943042afcb38d99041a8dc3d55f092f5f

Which adds:
"drivers: ddr: introduce DDR driver for i.MX8M"

Is there any documentation on how to use it (any ./doc/ entry)?

I guess that this code (from helper.c) helps with setting DDR4
controller properly.

However, patch 22/22 in this series - which adds EVK imx8m board
- doesn't use this function, but it has hardcoded long table of magic
numbers to setup DDR controller.

I guess that those magic numbers are created from running this code?

If yes - would it be possible to train/setup the DDR4 controller in SPL
as it is done on i.MX6Q? Or at least provide a detailed documentation
on how to use it?

> use 4 bytes
> aligned address to load ddr firmware, so we need make sure
> u-boot-spl.bin is 4 bytes aligned, in this patch we use dd
> to create a new file named u-boot-spl-pad.bin, then pad ddr firmware.
> 
> If SPL OF not enabled, this patch not hurt, because `_end` already
> is 4 bytes aligned.
> 
> Signed-off-by: Peng Fan 
> ---
>  tools/imx8m_image.sh | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/imx8m_image.sh b/tools/imx8m_image.sh
> index ec0881a128..08a6a48180 100755
> --- a/tools/imx8m_image.sh
> +++ b/tools/imx8m_image.sh
> @@ -35,8 +35,9 @@ if [ $post_process = 1 ]; then
>   objcopy -I binary -O binary --pad-to 0x8000
> --gap-fill=0x0 $srctree/lpddr4_pmu_train_2d_imem.bin
> lpddr4_pmu_train_2d_imem_pad.bin cat lpddr4_pmu_train_1d_imem_pad.bin
> lpddr4_pmu_train_1d_dmem_pad.bin > lpddr4_pmu_train_1d_fw.bin cat
> lpddr4_pmu_train_2d_imem_pad.bin
> $srctree/lpddr4_pmu_train_2d_dmem.bin > lpddr4_pmu_train_2d_fw.bin
> - cat spl/u-boot-spl.bin lpddr4_pmu_train_1d_fw.bin
> lpddr4_pmu_train_2d_fw.bin > spl/u-boot-spl-ddr.bin
> - rm -f lpddr4_pmu_train_1d_fw.bin
> lpddr4_pmu_train_2d_fw.bin lpddr4_pmu_train_1d_imem_pad.bin
> lpddr4_pmu_train_1d_dmem_pad.bin lpddr4_pmu_train_2d_imem_pad.bin
> + dd if=spl/u-boot-spl.bin of=spl/u-boot-spl-pad.bin
> bs=4 conv=sync
> + cat spl/u-boot-spl-pad.bin
> lpddr4_pmu_train_1d_fw.bin lpddr4_pmu_train_2d_fw.bin >
> spl/u-boot-spl-ddr.bin
> + rm -f lpddr4_pmu_train_1d_fw.bin
> lpddr4_pmu_train_2d_fw.bin lpddr4_pmu_train_1d_imem_pad.bin
> lpddr4_pmu_train_1d_dmem_pad.bin lpddr4_pmu_train_2d_imem_pad.bin
> spl/u-boot-spl-pad.bin fi fi 



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de


pgp_zgUO2V9Og.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot