[v3,2/2] package/murata-cyw-fw: download NVRAMand BT_PATCH only when needed
diff mbox series

Message ID 20190510083505.1341-3-m.niestroj@grinn-global.com
State Rejected
Headers show
Series
  • package/murata-cyw-fw: new package
Related show

Commit Message

Marcin Niestroj May 10, 2019, 8:35 a.m. UTC
Do not download NVRAM and BT_PATCH repositories when selected modules do
not require files from them. One of the example module is CYW4359, which
has neither NVRAM nor BT_PATCH files.

There is also drawback of this approach: NVRAM and BT_PATCH are not
downloaded when package is being rebuilt with changed set selected
modules, so the whole build fails because of missing files.

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
---
Changes v2 -> v3: add this patch

 package/murata-cyw-fw/murata-cyw-fw.mk | 29 ++++++++++++++++----------
 1 file changed, 18 insertions(+), 11 deletions(-)

Comments

Thomas Petazzoni May 20, 2019, 8:21 p.m. UTC | #1
Hello Marcin,

On Fri, 10 May 2019 10:35:05 +0200
Marcin Niestroj <m.niestroj@grinn-global.com> wrote:

> Do not download NVRAM and BT_PATCH repositories when selected modules do
> not require files from them. One of the example module is CYW4359, which
> has neither NVRAM nor BT_PATCH files.
> 
> There is also drawback of this approach: NVRAM and BT_PATCH are not
> downloaded when package is being rebuilt with changed set selected
> modules, so the whole build fails because of missing files.
> 
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> ---
> Changes v2 -> v3: add this patch

I looked at the size of the different tarballs:

-rw-r--r-- 1 thomas thomas 193K 20 mai   21:53 cyw-bt-patch-748462f0b02ec4aeb500bedd60780ac51c37be31.tar.gz
-rw-r--r-- 1 thomas thomas  12K 20 mai   21:53 cyw-fmac-nvram-d27f1bf105fa1e5b828e355793b88d4b66188411.tar.gz
-rw-r--r-- 1 thomas thomas 2,7M 20 mai   21:53 murata-cyw-fw-8d87950bfad28c65926695b7357bd8995b60016a.tar.gz

So the main tarball is 2.7 MB, and it gets always downloaded. The other
ones are 193 KB and 12 KB. I don't think this really warrants some
additional complexity: we can simply always download all tarballs, as
you did in PATCH 1/2.

So I marked PATCH 2/2 as Rejected.

Best regards,

Thomas
Yann E. MORIN May 20, 2019, 8:25 p.m. UTC | #2
Thomas, All,

On 2019-05-20 22:21 +0200, Thomas Petazzoni spake thusly:
> On Fri, 10 May 2019 10:35:05 +0200
> Marcin Niestroj <m.niestroj@grinn-global.com> wrote:
> > Do not download NVRAM and BT_PATCH repositories when selected modules do
> > not require files from them. One of the example module is CYW4359, which
> > has neither NVRAM nor BT_PATCH files.
> > 
> > There is also drawback of this approach: NVRAM and BT_PATCH are not
> > downloaded when package is being rebuilt with changed set selected
> > modules, so the whole build fails because of missing files.
> > 
> > Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> > ---
> > Changes v2 -> v3: add this patch
> 
> I looked at the size of the different tarballs:
> 
> -rw-r--r-- 1 thomas thomas 193K 20 mai   21:53 cyw-bt-patch-748462f0b02ec4aeb500bedd60780ac51c37be31.tar.gz
> -rw-r--r-- 1 thomas thomas  12K 20 mai   21:53 cyw-fmac-nvram-d27f1bf105fa1e5b828e355793b88d4b66188411.tar.gz
> -rw-r--r-- 1 thomas thomas 2,7M 20 mai   21:53 murata-cyw-fw-8d87950bfad28c65926695b7357bd8995b60016a.tar.gz
> 
> So the main tarball is 2.7 MB, and it gets always downloaded. The other
> ones are 193 KB and 12 KB. I don't think this really warrants some
> additional complexity: we can simply always download all tarballs, as
> you did in PATCH 1/2.

It was I who suggested Marcin could do this conditional download, after
they explained the extra downloads where not always required.

However, I did not know the relative sizes. Now that I do, I do agree
that this does not warrant the extra complexity.

> So I marked PATCH 2/2 as Rejected.

Agreed.

Regards,
Yann E. MORIN.

> Best regards,
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Patch
diff mbox series

diff --git a/package/murata-cyw-fw/murata-cyw-fw.mk b/package/murata-cyw-fw/murata-cyw-fw.mk
index 0196b47821..c0ada217af 100644
--- a/package/murata-cyw-fw/murata-cyw-fw.mk
+++ b/package/murata-cyw-fw/murata-cyw-fw.mk
@@ -8,21 +8,10 @@  MURATA_CYW_FW_VERSION = 8d87950bfad28c65926695b7357bd8995b60016a
 MURATA_CYW_FW_VERSION_NVRAM = d27f1bf105fa1e5b828e355793b88d4b66188411
 MURATA_CYW_FW_VERSION_BT_PATCH = 748462f0b02ec4aeb500bedd60780ac51c37be31
 MURATA_CYW_FW_SITE = $(call github,murata-wireless,cyw-fmac-fw,$(MURATA_CYW_FW_VERSION))
-MURATA_CYW_FW_EXTRA_DOWNLOADS = \
-	$(call github,murata-wireless,cyw-fmac-nvram,$(MURATA_CYW_FW_VERSION_NVRAM))/cyw-fmac-nvram-$(MURATA_CYW_FW_VERSION_NVRAM).tar.gz \
-	$(call github,murata-wireless,cyw-bt-patch,$(MURATA_CYW_FW_VERSION_BT_PATCH))/cyw-bt-patch-$(MURATA_CYW_FW_VERSION_BT_PATCH).tar.gz
 MURATA_CYW_FW_LICENSE = PROPRIETARY
 MURATA_CYW_FW_LICENSE_FILES = LICENCE.cypress
 MURATA_CYW_FW_REDISTRIBUTE = NO
 
-define MURATA_CYW_FW_EXTRACT_NVRAM_PATCH
-	$(foreach tar, $(notdir $(MURATA_CYW_FW_EXTRA_DOWNLOADS)), \
-		$(call suitable-extractor,$(tar)) $(MURATA_CYW_FW_DL_DIR)/$(tar) | \
-		$(TAR) --strip-components=1 -C $(@D) $(TAR_OPTIONS) -$(sep) \
-	)
-endef
-MURATA_CYW_FW_POST_EXTRACT_HOOKS += MURATA_CYW_FW_EXTRACT_NVRAM_PATCH
-
 MURATA_CYW_FW_FILES_$(BR2_PACKAGE_MURATA_CYW_FW_CYW43012) += \
 	brcmfmac43012-sdio.bin \
 	brcmfmac43012-sdio.1LV.clm_blob \
@@ -80,6 +69,24 @@  MURATA_CYW_FW_FILES_$(BR2_PACKAGE_MURATA_CYW_FW_CYW4359) += \
 	brcmfmac4359-pcie.bin \
 	brcmfmac4359-pcie.1FD.clm_blob
 
+ifneq ($(findstring .txt,$(MURATA_CYW_FW_FILES_y)),)
+MURATA_CYW_FW_EXTRA_DOWNLOADS += \
+	$(call github,murata-wireless,cyw-fmac-nvram,$(MURATA_CYW_FW_VERSION_NVRAM))/cyw-fmac-nvram-$(MURATA_CYW_FW_VERSION_NVRAM).tar.gz
+endif
+
+ifneq ($(findstring .hcd,$(MURATA_CYW_FW_FILES_y)),)
+MURATA_CYW_FW_EXTRA_DOWNLOADS += \
+	$(call github,murata-wireless,cyw-bt-patch,$(MURATA_CYW_FW_VERSION_BT_PATCH))/cyw-bt-patch-$(MURATA_CYW_FW_VERSION_BT_PATCH).tar.gz
+endif
+
+define MURATA_CYW_FW_EXTRACT_NVRAM_PATCH
+	$(foreach tar, $(notdir $(MURATA_CYW_FW_EXTRA_DOWNLOADS)), \
+		$(call suitable-extractor,$(tar)) $(MURATA_CYW_FW_DL_DIR)/$(tar) | \
+		$(TAR) --strip-components=1 -C $(@D) $(TAR_OPTIONS) -$(sep) \
+	)
+endef
+MURATA_CYW_FW_POST_EXTRACT_HOOKS += MURATA_CYW_FW_EXTRACT_NVRAM_PATCH
+
 # Helper that assumes filename with model has two dots (CHIP.MODEL.EXT),
 # but filename without model has only single dot (CHIP.EXT).
 murata-cyw-fw-strip-model = $(shell echo -n $(1) | sed 's/\..*\./\./')