| Message ID | 20191017190710.29985-3-jagan@amarulasolutions.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rockchip: Add Binman support | expand |
Jagan, On 2019/10/18 上午3:07, Jagan Teki wrote: > idbloader.img name is specific to rockchip, This is specific for rockchip, like rksd, rkspi type in mkimage, since it's clear for what it stands for, I think it can also used in U-Boot. > where it usually > created using rockchip tools. No, idbloader stands for the image with idb header packaged with two stage loader, eg. TPL+SPL or ddr.bin+miniloader.bin, and maybe ddr.bin + SPL， TPL+miniloader.bin, not related to rockchip tools, it can also be created with U-Boot mkimage tool. > Since the image is created as > per U-Boot generic builds like SPL, better to follow the > generic U-Boot naming notation like other SoC's following. > > Enable idbloader.img with u-boot-spl-rockchip.bin and create > the same in spl directory since it is SPL file and Makefile.spl > will clean it by default. > > Cc: Kever Yang <kever.yang@rock-chips.com> > Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index ec55e0f6a4..918b5d53e0 100644 > --- a/Makefile > +++ b/Makefile > @@ -887,7 +887,7 @@ ALL-y += u-boot-with-dtb.bin > endif > > ifeq ($(CONFIG_ARCH_ROCKCHIP)$(CONFIG_SPL)$(CONFIG_TPL),yyy) > -ALL-y += idbloader.img > +ALL-y += spl/u-boot-spl-rockchip.bin This idbloader including IDB header, TPL, and SPL, I don't think name it as spl and put it in spl directory make any sense. I though the origin output file use postfix '.bin' and those image after pack and can be dd to storage directly better to use postfix '.img'. About the clean, it will be better to be clean like other '.img' file in the Makefile. Thanks, - Kever > endif > > LDFLAGS_u-boot += $(LDFLAGS_FINAL) > @@ -1344,7 +1344,7 @@ endif > MKIMAGEFLAGS_u-boot-tpl.img = -n $(CONFIG_SYS_SOC) -T $(ROCKCHIP_IMG_TYPE) > tpl/u-boot-tpl.img: tpl/u-boot-tpl.bin FORCE > $(call if_changed,mkimage) > -idbloader.img: tpl/u-boot-tpl.img spl/u-boot-spl.bin FORCE > +spl/u-boot-spl-rockchip.bin: tpl/u-boot-tpl.img spl/u-boot-spl.bin FORCE > $(call if_changed,cat) > endif >
Hi Kever, On Fri, Oct 18, 2019 at 4:26 PM Kever Yang <kever.yang@rock-chips.com> wrote: > > Jagan, > > > On 2019/10/18 上午3:07, Jagan Teki wrote: > > idbloader.img name is specific to rockchip, > > This is specific for rockchip, like rksd, rkspi type in mkimage, since > it's clear for what it stands for, > > I think it can also used in U-Boot. > > > where it usually > > created using rockchip tools. > > > No, idbloader stands for the image with idb header packaged with two > stage loader, eg. TPL+SPL > > or ddr.bin+miniloader.bin, and maybe ddr.bin + SPL， TPL+miniloader.bin, > > not related to rockchip tools, it can also be created with U-Boot > mkimage tool. I understand what idbloader contains, it is like rockchip SPL. but the main concern here is the naming convention used in U-Boot. It would be a standard way of using naming conventions where the final output naming conventions should have u-boot support stages(TPL, SPL, U-Boot) and platform specific name. This way it is less confused what exactly it contains. Moreover most of the platform specific bin ended up using this type of conventions. - u-boot-spl-mtk.bin - Mediatek SPL bin - u-boot-mtk.bin - Mediatek U-Boot bin (would be final image) - u-boot-sunxi-with-spl.bin - Allwinner U-Boot with SPL - u-boot-x86-start16-tpl.bin Similar naming conventions used for tegra, x86 etc. I'm completely aware of what you're trying to abbreviate idbloader.img, but having u-boot-spl-rockchip.bin is more readable and understand than idbloader. > > > Since the image is created as > > per U-Boot generic builds like SPL, better to follow the > > generic U-Boot naming notation like other SoC's following. > > > > Enable idbloader.img with u-boot-spl-rockchip.bin and create > > the same in spl directory since it is SPL file and Makefile.spl > > will clean it by default. > > > > Cc: Kever Yang <kever.yang@rock-chips.com> > > Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > --- > > Makefile | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index ec55e0f6a4..918b5d53e0 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -887,7 +887,7 @@ ALL-y += u-boot-with-dtb.bin > > endif > > > > ifeq ($(CONFIG_ARCH_ROCKCHIP)$(CONFIG_SPL)$(CONFIG_TPL),yyy) > > -ALL-y += idbloader.img > > +ALL-y += spl/u-boot-spl-rockchip.bin > > > This idbloader including IDB header, TPL, and SPL, I don't think name it > as spl and > > put it in spl directory make any sense. > > I though the origin output file use postfix '.bin' and those image after > pack and can be > > dd to storage directly better to use postfix '.img'. > > About the clean, it will be better to be clean like other '.img' file in > the Makefile. This make sense, I have next version set where it used handle these so carefully.
On Mon, Oct 21, 2019 at 1:43 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > From: Jagan Teki <jagan@amarulasolutions.com> > > Date: Mon, 21 Oct 2019 10:56:39 +0530 > > > > Hi Kever, > > > > On Fri, Oct 18, 2019 at 4:26 PM Kever Yang <kever.yang@rock-chips.com> wrote: > > > > > > Jagan, > > > > > > > > > On 2019/10/18 上午3:07, Jagan Teki wrote: > > > > idbloader.img name is specific to rockchip, > > > > > > This is specific for rockchip, like rksd, rkspi type in mkimage, since > > > it's clear for what it stands for, > > > > > > I think it can also used in U-Boot. > > > > > > > where it usually > > > > created using rockchip tools. > > > > > > > > > No, idbloader stands for the image with idb header packaged with two > > > stage loader, eg. TPL+SPL > > > > > > or ddr.bin+miniloader.bin, and maybe ddr.bin + SPL， TPL+miniloader.bin, > > > > > > not related to rockchip tools, it can also be created with U-Boot > > > mkimage tool. > > > > I understand what idbloader contains, it is like rockchip SPL. but the > > main concern here is the naming convention used in U-Boot. It would be > > a standard way of using naming conventions where the final output > > naming conventions should have u-boot support stages(TPL, SPL, U-Boot) > > and platform specific name. This way it is less confused what exactly > > it contains. > > > > Moreover most of the platform specific bin ended up using this type of > > conventions. > > > > - u-boot-spl-mtk.bin - Mediatek SPL bin > > - u-boot-mtk.bin - Mediatek U-Boot bin (would be final image) > > - u-boot-sunxi-with-spl.bin - Allwinner U-Boot with SPL > > - u-boot-x86-start16-tpl.bin > > No real consistency there though... Not sure what you pointed here, each bin listed above shows the platform and u-boot stage and prefix with u-boot- as well. of course all can't maintain the same but has mentioned details. > > > Similar naming conventions used for tegra, x86 etc. > > > > I'm completely aware of what you're trying to abbreviate > > idbloader.img, but having u-boot-spl-rockchip.bin is more readable and > > understand than idbloader. > > As was discussed before, idbloader.img is the name used in external > documentation. Yes, used in external documentation. when it comes to upstream it would follow as per existing platforms does atleast.
Hi Jagan, On 2019/10/22 下午6:10, Jagan Teki wrote: > On Mon, Oct 21, 2019 at 1:43 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: >>> From: Jagan Teki <jagan@amarulasolutions.com> >>> Date: Mon, 21 Oct 2019 10:56:39 +0530 >>> >>> Hi Kever, >>> >>> On Fri, Oct 18, 2019 at 4:26 PM Kever Yang <kever.yang@rock-chips.com> wrote: >>>> Jagan, >>>> >>>> >>>> On 2019/10/18 上午3:07, Jagan Teki wrote: >>>>> idbloader.img name is specific to rockchip, >>>> This is specific for rockchip, like rksd, rkspi type in mkimage, since >>>> it's clear for what it stands for, >>>> >>>> I think it can also used in U-Boot. >>>> >>>>> where it usually >>>>> created using rockchip tools. >>>> >>>> No, idbloader stands for the image with idb header packaged with two >>>> stage loader, eg. TPL+SPL >>>> >>>> or ddr.bin+miniloader.bin, and maybe ddr.bin + SPL， TPL+miniloader.bin, >>>> >>>> not related to rockchip tools, it can also be created with U-Boot >>>> mkimage tool. >>> I understand what idbloader contains, it is like rockchip SPL. but the >>> main concern here is the naming convention used in U-Boot. It would be >>> a standard way of using naming conventions where the final output >>> naming conventions should have u-boot support stages(TPL, SPL, U-Boot) >>> and platform specific name. This way it is less confused what exactly >>> it contains. >>> >>> Moreover most of the platform specific bin ended up using this type of >>> conventions. >>> >>> - u-boot-spl-mtk.bin - Mediatek SPL bin >>> - u-boot-mtk.bin - Mediatek U-Boot bin (would be final image) >>> - u-boot-sunxi-with-spl.bin - Allwinner U-Boot with SPL >>> - u-boot-x86-start16-tpl.bin >> No real consistency there though... > Not sure what you pointed here, each bin listed above shows the > platform and u-boot stage and prefix with u-boot- as well. of course > all can't maintain the same but has mentioned details. The image for Vendor's BootRom is always vendor specific, so I don't think invent a new name for idbloader.img make any sense. I know what's the u-boot.bin, u-boot.itb, u-boot-spl.bin, u-boot-tpl.bin, because all the boards use them in the same way, but I don't know what is u-boot-spl-mtk.bin or u-boot-sunxi-with-spl.bin, and also names like lpc32xx-***, because they always specific for the SoC vendor, people never know what it's before they work on this platform. For idbloader.img, it already used for a period of time, it's document in both Rockchip document and U-Boot document, developers know what it's when they see this name, I don't think add a new name for it and add a new document and discards the document with old name is a good idea, it only confuse developers. You can have a new name for binman output for that's new output, but I would like not to invent new name for idbloader until most of developers think that's really necessary. Thanks, - Kever > >>> Similar naming conventions used for tegra, x86 etc. >>> >>> I'm completely aware of what you're trying to abbreviate >>> idbloader.img, but having u-boot-spl-rockchip.bin is more readable and >>> understand than idbloader. >> As was discussed before, idbloader.img is the name used in external >> documentation. > Yes, used in external documentation. when it comes to upstream it > would follow as per existing platforms does atleast. >
diff --git a/Makefile b/Makefile index ec55e0f6a4..918b5d53e0 100644 --- a/Makefile +++ b/Makefile @@ -887,7 +887,7 @@ ALL-y += u-boot-with-dtb.bin endif ifeq ($(CONFIG_ARCH_ROCKCHIP)$(CONFIG_SPL)$(CONFIG_TPL),yyy) -ALL-y += idbloader.img +ALL-y += spl/u-boot-spl-rockchip.bin endif LDFLAGS_u-boot += $(LDFLAGS_FINAL) @@ -1344,7 +1344,7 @@ endif MKIMAGEFLAGS_u-boot-tpl.img = -n $(CONFIG_SYS_SOC) -T $(ROCKCHIP_IMG_TYPE) tpl/u-boot-tpl.img: tpl/u-boot-tpl.bin FORCE $(call if_changed,mkimage) -idbloader.img: tpl/u-boot-tpl.img spl/u-boot-spl.bin FORCE +spl/u-boot-spl-rockchip.bin: tpl/u-boot-tpl.img spl/u-boot-spl.bin FORCE $(call if_changed,cat) endif
idbloader.img name is specific to rockchip, where it usually created using rockchip tools. Since the image is created as per U-Boot generic builds like SPL, better to follow the generic U-Boot naming notation like other SoC's following. Enable idbloader.img with u-boot-spl-rockchip.bin and create the same in spl directory since it is SPL file and Makefile.spl will clean it by default. Cc: Kever Yang <kever.yang@rock-chips.com> Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)