| Message ID | 20191120040725.32207-7-jerry.huang@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | new board ls1028ardb introduced | expand |
Hello, On Wed, 20 Nov 2019 12:07:22 +0800 Changming Huang <jerry.huang@nxp.com> wrote: > This package provides the firmware for LS1028ARDB DP (display port). > > Signed-off-by: Changming Huang <jerry.huang@nxp.com> > --- > package/freescale-qoriq/Config.in | 2 ++ > package/freescale-qoriq/cadence/Config.in | 9 +++++++++ > package/freescale-qoriq/cadence/cadence.mk | 19 +++++++++++++++++++ We need an entry in the DEVELOPERS file for this package. Also, "cadence" is not descriptive enough as a package name. freescale-qoriq-cadence-dp-firmware perhaps? It's a bit long, but at least it's descriptive. > 3 files changed, 30 insertions(+) > create mode 100644 package/freescale-qoriq/cadence/Config.in > create mode 100644 package/freescale-qoriq/cadence/cadence.mk The .hash file is missing. > diff --git a/package/freescale-qoriq/cadence/Config.in b/package/freescale-qoriq/cadence/Config.in > new file mode 100644 > index 0000000000..67292a799f > --- /dev/null > +++ b/package/freescale-qoriq/cadence/Config.in > @@ -0,0 +1,9 @@ > +config BR2_PACKAGE_CADENCE > + bool "display port firmware" This should be just the package name. > + help > + Display Port, a resident EL3 firmware. Indentation is not correct, we also need a link to some upstream web site that gives details about this firmware. Just curious: the DisplayPort really needs a firmware that runs on the main CPU, at EL3 ? This is pretty scary :-/ How does it get loaded ? > +if BR2_PACKAGE_CADENCE > +config BR2_PACKAGE_CADENCE_DP_BIN > + string "Custom DP Firmware BIN" This is not needed, there is a single firmware image in the archive, just hardcode it in the package. > +endif > diff --git a/package/freescale-qoriq/cadence/cadence.mk b/package/freescale-qoriq/cadence/cadence.mk > new file mode 100644 > index 0000000000..dd78411e15 > --- /dev/null > +++ b/package/freescale-qoriq/cadence/cadence.mk > @@ -0,0 +1,19 @@ > +################################################################################ > +# > +# DP firmware for NXP layerscape platforms Just the name of the package. > +# > +################################################################################ > +CADENCE_INSTALL_IMAGES = YES One empty line between the comment header and the first variable. Also, please specify _INSTALL_TARGET = NO > +DP_BIN = $(call qstrip,$(BR2_PACKAGE_CADENCE_DP_BIN)) As per the comment above, this is not needed. > + > +define CADENCE_BUILD_CMDS > + cd $(@D)/ && wget http://www.nxp.com/lgfiles/sdk/lsdk1909/firmware-cadence-lsdk1909.bin &&\ Urgh, this is horrible: you are completely bypassing Buildroot's download infrastructure. Please use it: <pkg>_VERSION = lsdk1909 <pkg>_SITE = http://www.nxp.com/lgfiles/sdk/lsdk1909/ <pkg>_SOURCE = firmware-cadence-$(<pkg>_VERSION).bin define <pkg>_EXTRACT_CMDS cd $(@D); \ sh $(<pkg>_DL_DIR)/$(<pkg>_SOURCE) --auto-accept endef > +define CADENCE_INSTALL_IMAGES_CMDS > + cp $(@D)/firmware-cadence-lsdk1909/dp/$(DP_BIN) $(BINARIES_DIR)/; Use: $(INSTALL) -D -m 0644 $(@D)/firmware-cadence-lsdk1909/dp/ls1028a-dp-fw.bin $(BINARIES_DIR)/ls1028a-dp-fw.bin Also, please add the appropriate licensing information, since there is a COPYING file in the archive. Thanks! Thomas
Thanks a lot for your detail comments. Mine in lines. Best Regards Jerry Huang > -----Original Message----- > From: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > Sent: Wednesday, November 20, 2019 5:13 PM > To: Jerry Huang <jerry.huang@nxp.com> > Cc: buildroot@busybox.net > Subject: [EXT] Re: [Buildroot] [PATCH 6/9] package/freescale-qoriq/cadence: > new package > > Caution: EXT Email > > Hello, > > On Wed, 20 Nov 2019 12:07:22 +0800 > Changming Huang <jerry.huang@nxp.com> wrote: > > > This package provides the firmware for LS1028ARDB DP (display port). > > > > Signed-off-by: Changming Huang <jerry.huang@nxp.com> > > --- > > package/freescale-qoriq/Config.in | 2 ++ > > package/freescale-qoriq/cadence/Config.in | 9 +++++++++ > > package/freescale-qoriq/cadence/cadence.mk | 19 +++++++++++++++++++ > > We need an entry in the DEVELOPERS file for this package. Ok, I will add the entry in DEVELOPERS > Also, "cadence" is not descriptive enough as a package name. > freescale-qoriq-cadence-dp-firmware perhaps? It's a bit long, but at least it's > descriptive. I think "cadence-dp-firmware" can be used for its name. Because it is under sub-directory " freescale-qoriq", we can remove the prefix " freescale-qoriq" from package name. > > 3 files changed, 30 insertions(+) > > create mode 100644 package/freescale-qoriq/cadence/Config.in > > create mode 100644 package/freescale-qoriq/cadence/cadence.mk > > The .hash file is missing. I will add the hash file. > > diff --git a/package/freescale-qoriq/cadence/Config.in > > b/package/freescale-qoriq/cadence/Config.in > > new file mode 100644 > > index 0000000000..67292a799f > > --- /dev/null > > +++ b/package/freescale-qoriq/cadence/Config.in > > @@ -0,0 +1,9 @@ > > +config BR2_PACKAGE_CADENCE > > + bool "display port firmware" > > This should be just the package name. Ok > > + help > > + Display Port, a resident EL3 firmware. > > Indentation is not correct, we also need a link to some upstream web site that > gives details about this firmware. Sure, I will adjust it. > Just curious: the DisplayPort really needs a firmware that runs on the main CPU, > at EL3 ? This is pretty scary :-/ How does it get loaded ? Yes, LS1028ARDB need this firmware for display port. We used the command "hdp load" in uboot before starting Linux kernel. > > +if BR2_PACKAGE_CADENCE > > +config BR2_PACKAGE_CADENCE_DP_BIN > > + string "Custom DP Firmware BIN" > > This is not needed, there is a single firmware image in the archive, just hardcode > it in the package. ok > > +endif > > diff --git a/package/freescale-qoriq/cadence/cadence.mk > > b/package/freescale-qoriq/cadence/cadence.mk > > new file mode 100644 > > index 0000000000..dd78411e15 > > --- /dev/null > > +++ b/package/freescale-qoriq/cadence/cadence.mk > > @@ -0,0 +1,19 @@ > > > +############################################################### > ###### > > +########### > > +# > > +# DP firmware for NXP layerscape platforms > > Just the name of the package. > > > +# > > > +############################################################### > ###### > > +########### > > +CADENCE_INSTALL_IMAGES = YES > > One empty line between the comment header and the first variable. > > Also, please specify _INSTALL_TARGET = NO > > > +DP_BIN = $(call qstrip,$(BR2_PACKAGE_CADENCE_DP_BIN)) > > As per the comment above, this is not needed. > > > + > > +define CADENCE_BUILD_CMDS > > + cd $(@D)/ && wget > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww. > > > +nxp.com%2Flgfiles%2Fsdk%2Flsdk1909%2Ffirmware-cadence-lsdk1909.bin&a > m > > > +p;data=02%7C01%7Cjerry.huang%40nxp.com%7C9c92f3028db94a45041a08d > 76d99 > > > +e558%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637098380050 > 336719& > > > +amp;sdata=1f65%2B7j1t%2FuLicJMICfrV%2BrZpjme4a4YUtK88gDJF8g%3D&a > mp;re > > +served=0 &&\ > > Urgh, this is horrible: you are completely bypassing Buildroot's download > infrastructure. Please use it: > > <pkg>_VERSION = lsdk1909 > <pkg>_SITE = > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.nxp. > com%2Flgfiles%2Fsdk%2Flsdk1909%2F&data=02%7C01%7Cjerry.huang%4 > 0nxp.com%7C9c92f3028db94a45041a08d76d99e558%7C686ea1d3bc2b4c6fa9 > 2cd99c5c301635%7C0%7C0%7C637098380050336719&sdata=NJTQlJNknn > I3Dmi62e%2B06a%2BoI%2BzD8nT2DtXTS27c3SU%3D&reserved=0 > <pkg>_SOURCE = firmware-cadence-$(<pkg>_VERSION).bin > > define <pkg>_EXTRACT_CMDS > cd $(@D); \ > sh $(<pkg>_DL_DIR)/$(<pkg>_SOURCE) --auto-accept endef > > > +define CADENCE_INSTALL_IMAGES_CMDS > > + cp $(@D)/firmware-cadence-lsdk1909/dp/$(DP_BIN) > > +$(BINARIES_DIR)/; > > Use: > > $(INSTALL) -D -m 0644 > $(@D)/firmware-cadence-lsdk1909/dp/ls1028a-dp-fw.bin > $(BINARIES_DIR)/ls1028a-dp-fw.bin ☹ I will following the download infrastructure. > Also, please add the appropriate licensing information, since there is a COPYING > file in the archive. Ok, will add the license information > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin.c > om&data=02%7C01%7Cjerry.huang%40nxp.com%7C9c92f3028db94a4504 > 1a08d76d99e558%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637 > 098380050336719&sdata=1KYF0jJSSz75f%2FthSq5U3BPIoYst%2BqnqpVAe > uYsMUD0%3D&reserved=0
Hello, On Wed, 20 Nov 2019 09:30:26 +0000 Jerry Huang <jerry.huang@nxp.com> wrote: > > Also, "cadence" is not descriptive enough as a package name. > > freescale-qoriq-cadence-dp-firmware perhaps? It's a bit long, but at least it's > > descriptive. > I think "cadence-dp-firmware" can be used for its name. > Because it is under sub-directory " freescale-qoriq", > we can remove the prefix " freescale-qoriq" from package name. Being inside the freescale-qoriq doesn't improve things in any way: the namespace of package names (and therefore variables) is global in Buildroot. > > Indentation is not correct, we also need a link to some upstream web site that > > gives details about this firmware. > Sure, I will adjust it. > > > Just curious: the DisplayPort really needs a firmware that runs on the main CPU, > > at EL3 ? This is pretty scary :-/ How does it get loaded ? > Yes, LS1028ARDB need this firmware for display port. > We used the command "hdp load" in uboot before starting Linux kernel. OK. Where is this firmware image stored? You just install it to $(BINARIES_DIR). Should it be installed in the rootfs instead, so that U-Boot can pick it up from there ? Thanks, Thomas
Best Regards Jerry Huang > -----Original Message----- > From: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > Sent: Wednesday, November 20, 2019 5:36 PM > To: Jerry Huang <jerry.huang@nxp.com> > Cc: buildroot@busybox.net > Subject: Re: [EXT] Re: [Buildroot] [PATCH 6/9] package/freescale-qoriq/cadence: > new package > > Caution: EXT Email > > Hello, > > On Wed, 20 Nov 2019 09:30:26 +0000 > Jerry Huang <jerry.huang@nxp.com> wrote: > > > > Also, "cadence" is not descriptive enough as a package name. > > > freescale-qoriq-cadence-dp-firmware perhaps? It's a bit long, but at > > > least it's descriptive. > > I think "cadence-dp-firmware" can be used for its name. > > Because it is under sub-directory " freescale-qoriq", we can remove > > the prefix " freescale-qoriq" from package name. > > Being inside the freescale-qoriq doesn't improve things in any way: the > namespace of package names (and therefore variables) is global in Buildroot. > > > > Indentation is not correct, we also need a link to some upstream web > > > site that gives details about this firmware. > > Sure, I will adjust it. > > > > > Just curious: the DisplayPort really needs a firmware that runs on > > > the main CPU, at EL3 ? This is pretty scary :-/ How does it get loaded ? > > Yes, LS1028ARDB need this firmware for display port. > > We used the command "hdp load" in uboot before starting Linux kernel. > > OK. Where is this firmware image stored? You just install it to $(BINARIES_DIR). > Should it be installed in the rootfs instead, so that U-Boot can pick it up from > there ? The post image scripts will create one full images including uboot, rcw, firmware, kernel ITB/rootfs. This firmware is stored NOR flash if booting the board from QSPI/FlexSPI boot, Or it is stored one partition on SD/eMMC when booting from SD card or eMMC chip. Then, U-boot can pick it up from flash, SD or eMMC. > Thanks, > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin.c > om&data=02%7C01%7Cjerry.huang%40nxp.com%7C7af319f85d8c4ad276 > 4008d76d9d1d2e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637 > 098393876225198&sdata=%2F8LtHwidlTszcsesa9IwePY%2FUJQt7iuJQi%2F > T%2BfSOu9w%3D&reserved=0
diff --git a/package/freescale-qoriq/Config.in b/package/freescale-qoriq/Config.in index 51497c588e..03fdf345c0 100644 --- a/package/freescale-qoriq/Config.in +++ b/package/freescale-qoriq/Config.in @@ -1,4 +1,6 @@ menu "Freescale QorIQ libraries" depends on BR2_aarch64 || BR2_arm || BR2_powerpc64 || BR2_powerpc +source "package/freescale-qoriq/cadence/Config.in" + endmenu diff --git a/package/freescale-qoriq/cadence/Config.in b/package/freescale-qoriq/cadence/Config.in new file mode 100644 index 0000000000..67292a799f --- /dev/null +++ b/package/freescale-qoriq/cadence/Config.in @@ -0,0 +1,9 @@ +config BR2_PACKAGE_CADENCE + bool "display port firmware" + help + Display Port, a resident EL3 firmware. + +if BR2_PACKAGE_CADENCE +config BR2_PACKAGE_CADENCE_DP_BIN + string "Custom DP Firmware BIN" +endif diff --git a/package/freescale-qoriq/cadence/cadence.mk b/package/freescale-qoriq/cadence/cadence.mk new file mode 100644 index 0000000000..dd78411e15 --- /dev/null +++ b/package/freescale-qoriq/cadence/cadence.mk @@ -0,0 +1,19 @@ +################################################################################ +# +# DP firmware for NXP layerscape platforms +# +################################################################################ +CADENCE_INSTALL_IMAGES = YES + +DP_BIN = $(call qstrip,$(BR2_PACKAGE_CADENCE_DP_BIN)) + +define CADENCE_BUILD_CMDS + cd $(@D)/ && wget http://www.nxp.com/lgfiles/sdk/lsdk1909/firmware-cadence-lsdk1909.bin &&\ + chmod +x firmware-cadence-lsdk1909.bin && ./firmware-cadence-lsdk1909.bin --auto-accept; +endef + +define CADENCE_INSTALL_IMAGES_CMDS + cp $(@D)/firmware-cadence-lsdk1909/dp/$(DP_BIN) $(BINARIES_DIR)/; +endef + +$(eval $(generic-package))
This package provides the firmware for LS1028ARDB DP (display port). Signed-off-by: Changming Huang <jerry.huang@nxp.com> --- package/freescale-qoriq/Config.in | 2 ++ package/freescale-qoriq/cadence/Config.in | 9 +++++++++ package/freescale-qoriq/cadence/cadence.mk | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+) create mode 100644 package/freescale-qoriq/cadence/Config.in create mode 100644 package/freescale-qoriq/cadence/cadence.mk