[PATCH RESEND v2 00/16] Misc fixes, sanity checks and xz/zstd compression

[PATCH RESEND v2 00/16] Misc fixes, sanity checks and xz/zstd compression

[Emil Velikov via B4 Relay]

Hey everyone,

Here's an updated series which fixes some outstanding issues, enhances
the check_whence checks and adds xz/zstd firmware support.

v2:
 - Commend out duplicated MediaTek firmware
 - Tweak list_links_list() after Adam's approach
 - Skip compression of amd ucode

Link to v1: https://lore.kernel.org/linux-firmware/CACvgo50RhmRKnUoos2wGWOP-Pb8oGo1BMPunfTtBQO0eX1D2Dg@mail.gmail.com/T/

Josh, others, kindly let me know if you'd prefer a PR or if there is
anything I can do to move this forward. Be that as a whole, or chunks of
it.

Thanks
Emil

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
Emil Velikov (16):
      WHENCE: remove trailing white space
      WHENCE: remove unnecessary filename quotation
      check_whence, WHENCE, copy-firmware: escape filenames with spaces
      WHENCE: remove duplicate File entries
      WHENCE: comment out duplicate MediaTek firmware
      check_whence: error on duplicate file entries
      check_whence: error on directory listed as File
      copy-firmware: remove non-applicable file presence test
      check_whence: error if File: is actually a link
      check_whence: error if symlinks are in-tree
      copy-firmware: remove unreachable symlink workarounds
      copy-firmware: quote deskdir to prevent word splitting
      copy-firmware: tweak sed invocation
      copy-firmware: quote the output of dirname
      copy-firmware: silence the last shellcheck warnings
      Makefile, copy-firmware: support xz/zstd compressed firmware

 Makefile         |   8 +++++
 WHENCE           | 107 ++++++++++++++++++++++++-------------------------------
 check_whence.py  |  53 +++++++++++++++++++++++----
 copy-firmware.sh |  76 ++++++++++++++++++++-------------------
 4 files changed, 141 insertions(+), 103 deletions(-)
---
base-commit: 60971a6485ee604c47cc905ff77c583ae001f9df
change-id: 20230301-fixes-and-compression-7247407cab21

Best regards,
-- 
Emil Velikov <emil.l.velikov@gmail.com>

[PATCH RESEND v2 01/16] WHENCE: remove trailing white space

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 WHENCE | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/WHENCE b/WHENCE
index e957cc9..205162e 100644
--- a/WHENCE
+++ b/WHENCE
@@ -395,7 +395,7 @@ Version: 8.07.00 MIDQ
 
 Licence: Redistributable. See LICENCE.qla2xxx for details
 
-Available from http://ldriver.qlogic.com/firmware/ 
+Available from http://ldriver.qlogic.com/firmware/
 
 --------------------------------------------------------------------------
 
@@ -2563,29 +2563,29 @@ Licence: Redistributable. See LICENCE.ueagle-atm4-firmware for details
 Driver: ueagle-atm - Driver for USB ADSL Modems based on Eagle I,II,III
 
 File: ueagle-atm/930-fpga.bin
-File: ueagle-atm/CMVeiWO.bin    
-File: ueagle-atm/CMVepFR10.bin  
-File: ueagle-atm/DSP9p.bin	
+File: ueagle-atm/CMVeiWO.bin
+File: ueagle-atm/CMVepFR10.bin
+File: ueagle-atm/DSP9p.bin
 File: ueagle-atm/eagleIII.fw
-File: ueagle-atm/adi930.fw     
-File: ueagle-atm/CMVep.bin      
-File: ueagle-atm/CMVepFR.bin    
+File: ueagle-atm/adi930.fw
+File: ueagle-atm/CMVep.bin
+File: ueagle-atm/CMVepFR.bin
 File: ueagle-atm/DSPei.bin
-File: ueagle-atm/CMV9i.bin     
-File: ueagle-atm/CMVepES03.bin  
-File: ueagle-atm/CMVepIT.bin    
+File: ueagle-atm/CMV9i.bin
+File: ueagle-atm/CMVepES03.bin
+File: ueagle-atm/CMVepIT.bin
 File: ueagle-atm/DSPep.bin
-File: ueagle-atm/CMV9p.bin     
-File: ueagle-atm/CMVepES.bin    
-File: ueagle-atm/CMVepWO.bin    
+File: ueagle-atm/CMV9p.bin
+File: ueagle-atm/CMVepES.bin
+File: ueagle-atm/CMVepWO.bin
 File: ueagle-atm/eagleI.fw
-File: ueagle-atm/CMVei.bin     
-File: ueagle-atm/CMVepFR04.bin  
-File: ueagle-atm/DSP9i.bin	    
+File: ueagle-atm/CMVei.bin
+File: ueagle-atm/CMVepFR04.bin
+File: ueagle-atm/DSP9i.bin
 File: ueagle-atm/eagleII.fw
 Version: 1.1
 
-Licence: Redistributable. Based on 
+Licence: Redistributable. Based on
          https://mail.gna.org/public/eagleusb-dev/2004-11/msg00172.html
 
 --------------------------------------------------------------------------
@@ -3951,7 +3951,7 @@ License:
 
   This SDD ("Static Dynamic Data") file is licensed strictly for use with
   the Sagrad WiFi modules (such as the SG901-1091/1098) that utilize the
-  cw1200 driver. There is no warranty expressed or implied about its 
+  cw1200 driver. There is no warranty expressed or implied about its
   fitness for any purpose.
 
   Permission is hereby granted for the distribution of this SDD file as

-- 
2.39.2

Re: [PATCH RESEND v2 01/16] WHENCE: remove trailing white space

[Josh Boyer]

Applied and pushed out.

josh

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
>  WHENCE | 36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/WHENCE b/WHENCE
> index e957cc9..205162e 100644
> --- a/WHENCE
> +++ b/WHENCE
> @@ -395,7 +395,7 @@ Version: 8.07.00 MIDQ
>
>  Licence: Redistributable. See LICENCE.qla2xxx for details
>
> -Available from http://ldriver.qlogic.com/firmware/
> +Available from http://ldriver.qlogic.com/firmware/
>
>  --------------------------------------------------------------------------
>
> @@ -2563,29 +2563,29 @@ Licence: Redistributable. See LICENCE.ueagle-atm4-firmware for details
>  Driver: ueagle-atm - Driver for USB ADSL Modems based on Eagle I,II,III
>
>  File: ueagle-atm/930-fpga.bin
> -File: ueagle-atm/CMVeiWO.bin
> -File: ueagle-atm/CMVepFR10.bin
> -File: ueagle-atm/DSP9p.bin
> +File: ueagle-atm/CMVeiWO.bin
> +File: ueagle-atm/CMVepFR10.bin
> +File: ueagle-atm/DSP9p.bin
>  File: ueagle-atm/eagleIII.fw
> -File: ueagle-atm/adi930.fw
> -File: ueagle-atm/CMVep.bin
> -File: ueagle-atm/CMVepFR.bin
> +File: ueagle-atm/adi930.fw
> +File: ueagle-atm/CMVep.bin
> +File: ueagle-atm/CMVepFR.bin
>  File: ueagle-atm/DSPei.bin
> -File: ueagle-atm/CMV9i.bin
> -File: ueagle-atm/CMVepES03.bin
> -File: ueagle-atm/CMVepIT.bin
> +File: ueagle-atm/CMV9i.bin
> +File: ueagle-atm/CMVepES03.bin
> +File: ueagle-atm/CMVepIT.bin
>  File: ueagle-atm/DSPep.bin
> -File: ueagle-atm/CMV9p.bin
> -File: ueagle-atm/CMVepES.bin
> -File: ueagle-atm/CMVepWO.bin
> +File: ueagle-atm/CMV9p.bin
> +File: ueagle-atm/CMVepES.bin
> +File: ueagle-atm/CMVepWO.bin
>  File: ueagle-atm/eagleI.fw
> -File: ueagle-atm/CMVei.bin
> -File: ueagle-atm/CMVepFR04.bin
> -File: ueagle-atm/DSP9i.bin
> +File: ueagle-atm/CMVei.bin
> +File: ueagle-atm/CMVepFR04.bin
> +File: ueagle-atm/DSP9i.bin
>  File: ueagle-atm/eagleII.fw
>  Version: 1.1
>
> -Licence: Redistributable. Based on
> +Licence: Redistributable. Based on
>           https://mail.gna.org/public/eagleusb-dev/2004-11/msg00172.html
>
>  --------------------------------------------------------------------------
> @@ -3951,7 +3951,7 @@ License:
>
>    This SDD ("Static Dynamic Data") file is licensed strictly for use with
>    the Sagrad WiFi modules (such as the SG901-1091/1098) that utilize the
> -  cw1200 driver. There is no warranty expressed or implied about its
> +  cw1200 driver. There is no warranty expressed or implied about its
>    fitness for any purpose.
>
>    Permission is hereby granted for the distribution of this SDD file as
>
> --
> 2.39.2
>

[PATCH RESEND v2 02/16] WHENCE: remove unnecessary filename quotation

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 WHENCE | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/WHENCE b/WHENCE
index 205162e..d4654dd 100644
--- a/WHENCE
+++ b/WHENCE
@@ -2691,20 +2691,20 @@ Link: brcm/brcmfmac54591-pcie.clm_blob -> ../cypress/cyfmac54591-pcie.clm_blob
 Licence: Redistributable. See LICENCE.cypress for details.
 
 File: "brcm/brcmfmac43241b4-sdio.Intel Corp.-VALLEYVIEW C0 PLATFORM.txt"
-File: "brcm/brcmfmac4330-sdio.Prowise-PT301.txt"
+File: brcm/brcmfmac4330-sdio.Prowise-PT301.txt
 File: "brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-TF103CE.txt"
-File: "brcm/brcmfmac43340-sdio.meegopad-t08.txt"
-File: "brcm/brcmfmac43340-sdio.pov-tab-p1006w-data.txt"
-File: "brcm/brcmfmac43340-sdio.predia-basic.txt"
-File: "brcm/brcmfmac43362-sdio.WC121.txt"
-File: "brcm/brcmfmac43362-sdio.cubietech,cubietruck.txt"
+File: brcm/brcmfmac43340-sdio.meegopad-t08.txt
+File: brcm/brcmfmac43340-sdio.pov-tab-p1006w-data.txt
+File: brcm/brcmfmac43340-sdio.predia-basic.txt
+File: brcm/brcmfmac43362-sdio.WC121.txt
+File: brcm/brcmfmac43362-sdio.cubietech,cubietruck.txt
 Link: brcm/brcmfmac43362-sdio.kobo,aura.txt -> brcmfmac43362-sdio.WC121.txt
 Link: brcm/brcmfmac43362-sdio.kobo,tolino-shine2hd.txt -> brcmfmac43362-sdio.WC121.txt
 Link: brcm/brcmfmac43362-sdio.lemaker,bananapro.txt -> brcmfmac43362-sdio.cubietech,cubietruck.txt
-File: "brcm/brcmfmac43430a0-sdio.ilife-S806.txt"
-File: "brcm/brcmfmac43430a0-sdio.jumper-ezpad-mini3.txt"
+File: brcm/brcmfmac43430a0-sdio.ilife-S806.txt
+File: brcm/brcmfmac43430a0-sdio.jumper-ezpad-mini3.txt
 File: "brcm/brcmfmac43430a0-sdio.ONDA-V80 PLUS.txt"
-File: "brcm/brcmfmac43430-sdio.AP6212.txt"
+File: brcm/brcmfmac43430-sdio.AP6212.txt
 Link: brcm/brcmfmac43430-sdio.sinovoip,bpi-m2-plus.txt -> brcmfmac43430-sdio.AP6212.txt
 Link: brcm/brcmfmac43430-sdio.sinovoip,bpi-m2-zero.txt -> brcmfmac43430-sdio.AP6212.txt
 Link: brcm/brcmfmac43430-sdio.sinovoip,bpi-m2-ultra.txt -> brcmfmac43430-sdio.AP6212.txt
@@ -2713,19 +2713,19 @@ Link: brcm/brcmfmac43430-sdio.friendlyarm,nanopi-r1.txt -> brcmfmac43430-sdio.AP
 Link: brcm/brcmfmac43430-sdio.starfive,visionfive-v1.txt -> brcmfmac43430-sdio.AP6212.txt
 Link: brcm/brcmfmac43430-sdio.beagle,beaglev-starlight-jh7100-a1.txt -> brcmfmac43430-sdio.AP6212.txt
 Link: brcm/brcmfmac43430-sdio.beagle,beaglev-starlight-jh7100-r0.txt -> brcmfmac43430-sdio.AP6212.txt
-File: "brcm/brcmfmac43430-sdio.Hampoo-D2D3_Vi8A1.txt"
-File: "brcm/brcmfmac43430-sdio.MUR1DX.txt"
-File: "brcm/brcmfmac43430-sdio.raspberrypi,3-model-b.txt"
+File: brcm/brcmfmac43430-sdio.Hampoo-D2D3_Vi8A1.txt
+File: brcm/brcmfmac43430-sdio.MUR1DX.txt
+File: brcm/brcmfmac43430-sdio.raspberrypi,3-model-b.txt
 Link: brcm/brcmfmac43430-sdio.raspberrypi,model-zero-w.txt -> brcmfmac43430-sdio.raspberrypi,3-model-b.txt
 Link: brcm/brcmfmac43430-sdio.raspberrypi,model-zero-2-w.txt -> brcmfmac43430-sdio.raspberrypi,3-model-b.txt
-File: "brcm/brcmfmac43455-sdio.acepc-t8.txt"
-File: "brcm/brcmfmac43455-sdio.raspberrypi,3-model-b-plus.txt"
+File: brcm/brcmfmac43455-sdio.acepc-t8.txt
+File: brcm/brcmfmac43455-sdio.raspberrypi,3-model-b-plus.txt
 Link: brcm/brcmfmac43455-sdio.raspberrypi,3-model-a-plus.txt -> brcmfmac43455-sdio.raspberrypi,3-model-b-plus.txt
-File: "brcm/brcmfmac43455-sdio.raspberrypi,4-model-b.txt"
+File: brcm/brcmfmac43455-sdio.raspberrypi,4-model-b.txt
 Link: brcm/brcmfmac43455-sdio.Raspberry\ Pi\ Foundation-Raspberry\ Pi\ 4\ Model\ B.txt -> brcmfmac43455-sdio.raspberrypi,4-model-b.txt
 Link: brcm/brcmfmac43455-sdio.Raspberry\ Pi\ Foundation-Raspberry\ Pi\ Compute\ Module\ 4.txt -> brcmfmac43455-sdio.raspberrypi,4-model-b.txt
 File: "brcm/brcmfmac43455-sdio.MINIX-NEO Z83-4.txt"
-File: "brcm/brcmfmac4356-pcie.gpd-win-pocket.txt"
+File: brcm/brcmfmac4356-pcie.gpd-win-pocket.txt
 File: "brcm/brcmfmac4356-pcie.Xiaomi Inc-Mipad2.txt"
 File: brcm/brcmfmac4356-sdio.AP6356S.txt
 Link: brcm/brcmfmac4356-sdio.firefly,firefly-rk3399.txt -> brcmfmac4356-sdio.AP6356S.txt

-- 
2.39.2

Re: [PATCH RESEND v2 02/16] WHENCE: remove unnecessary filename quotation

[Josh Boyer]

This one no longer applies.  While the quotes might be unnecessary,
they also don't hurt.

josh

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
>  WHENCE | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/WHENCE b/WHENCE
> index 205162e..d4654dd 100644
> --- a/WHENCE
> +++ b/WHENCE
> @@ -2691,20 +2691,20 @@ Link: brcm/brcmfmac54591-pcie.clm_blob -> ../cypress/cyfmac54591-pcie.clm_blob
>  Licence: Redistributable. See LICENCE.cypress for details.
>
>  File: "brcm/brcmfmac43241b4-sdio.Intel Corp.-VALLEYVIEW C0 PLATFORM.txt"
> -File: "brcm/brcmfmac4330-sdio.Prowise-PT301.txt"
> +File: brcm/brcmfmac4330-sdio.Prowise-PT301.txt
>  File: "brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-TF103CE.txt"
> -File: "brcm/brcmfmac43340-sdio.meegopad-t08.txt"
> -File: "brcm/brcmfmac43340-sdio.pov-tab-p1006w-data.txt"
> -File: "brcm/brcmfmac43340-sdio.predia-basic.txt"
> -File: "brcm/brcmfmac43362-sdio.WC121.txt"
> -File: "brcm/brcmfmac43362-sdio.cubietech,cubietruck.txt"
> +File: brcm/brcmfmac43340-sdio.meegopad-t08.txt
> +File: brcm/brcmfmac43340-sdio.pov-tab-p1006w-data.txt
> +File: brcm/brcmfmac43340-sdio.predia-basic.txt
> +File: brcm/brcmfmac43362-sdio.WC121.txt
> +File: brcm/brcmfmac43362-sdio.cubietech,cubietruck.txt
>  Link: brcm/brcmfmac43362-sdio.kobo,aura.txt -> brcmfmac43362-sdio.WC121.txt
>  Link: brcm/brcmfmac43362-sdio.kobo,tolino-shine2hd.txt -> brcmfmac43362-sdio.WC121.txt
>  Link: brcm/brcmfmac43362-sdio.lemaker,bananapro.txt -> brcmfmac43362-sdio.cubietech,cubietruck.txt
> -File: "brcm/brcmfmac43430a0-sdio.ilife-S806.txt"
> -File: "brcm/brcmfmac43430a0-sdio.jumper-ezpad-mini3.txt"
> +File: brcm/brcmfmac43430a0-sdio.ilife-S806.txt
> +File: brcm/brcmfmac43430a0-sdio.jumper-ezpad-mini3.txt
>  File: "brcm/brcmfmac43430a0-sdio.ONDA-V80 PLUS.txt"
> -File: "brcm/brcmfmac43430-sdio.AP6212.txt"
> +File: brcm/brcmfmac43430-sdio.AP6212.txt
>  Link: brcm/brcmfmac43430-sdio.sinovoip,bpi-m2-plus.txt -> brcmfmac43430-sdio.AP6212.txt
>  Link: brcm/brcmfmac43430-sdio.sinovoip,bpi-m2-zero.txt -> brcmfmac43430-sdio.AP6212.txt
>  Link: brcm/brcmfmac43430-sdio.sinovoip,bpi-m2-ultra.txt -> brcmfmac43430-sdio.AP6212.txt
> @@ -2713,19 +2713,19 @@ Link: brcm/brcmfmac43430-sdio.friendlyarm,nanopi-r1.txt -> brcmfmac43430-sdio.AP
>  Link: brcm/brcmfmac43430-sdio.starfive,visionfive-v1.txt -> brcmfmac43430-sdio.AP6212.txt
>  Link: brcm/brcmfmac43430-sdio.beagle,beaglev-starlight-jh7100-a1.txt -> brcmfmac43430-sdio.AP6212.txt
>  Link: brcm/brcmfmac43430-sdio.beagle,beaglev-starlight-jh7100-r0.txt -> brcmfmac43430-sdio.AP6212.txt
> -File: "brcm/brcmfmac43430-sdio.Hampoo-D2D3_Vi8A1.txt"
> -File: "brcm/brcmfmac43430-sdio.MUR1DX.txt"
> -File: "brcm/brcmfmac43430-sdio.raspberrypi,3-model-b.txt"
> +File: brcm/brcmfmac43430-sdio.Hampoo-D2D3_Vi8A1.txt
> +File: brcm/brcmfmac43430-sdio.MUR1DX.txt
> +File: brcm/brcmfmac43430-sdio.raspberrypi,3-model-b.txt
>  Link: brcm/brcmfmac43430-sdio.raspberrypi,model-zero-w.txt -> brcmfmac43430-sdio.raspberrypi,3-model-b.txt
>  Link: brcm/brcmfmac43430-sdio.raspberrypi,model-zero-2-w.txt -> brcmfmac43430-sdio.raspberrypi,3-model-b.txt
> -File: "brcm/brcmfmac43455-sdio.acepc-t8.txt"
> -File: "brcm/brcmfmac43455-sdio.raspberrypi,3-model-b-plus.txt"
> +File: brcm/brcmfmac43455-sdio.acepc-t8.txt
> +File: brcm/brcmfmac43455-sdio.raspberrypi,3-model-b-plus.txt
>  Link: brcm/brcmfmac43455-sdio.raspberrypi,3-model-a-plus.txt -> brcmfmac43455-sdio.raspberrypi,3-model-b-plus.txt
> -File: "brcm/brcmfmac43455-sdio.raspberrypi,4-model-b.txt"
> +File: brcm/brcmfmac43455-sdio.raspberrypi,4-model-b.txt
>  Link: brcm/brcmfmac43455-sdio.Raspberry\ Pi\ Foundation-Raspberry\ Pi\ 4\ Model\ B.txt -> brcmfmac43455-sdio.raspberrypi,4-model-b.txt
>  Link: brcm/brcmfmac43455-sdio.Raspberry\ Pi\ Foundation-Raspberry\ Pi\ Compute\ Module\ 4.txt -> brcmfmac43455-sdio.raspberrypi,4-model-b.txt
>  File: "brcm/brcmfmac43455-sdio.MINIX-NEO Z83-4.txt"
> -File: "brcm/brcmfmac4356-pcie.gpd-win-pocket.txt"
> +File: brcm/brcmfmac4356-pcie.gpd-win-pocket.txt
>  File: "brcm/brcmfmac4356-pcie.Xiaomi Inc-Mipad2.txt"
>  File: brcm/brcmfmac4356-sdio.AP6356S.txt
>  Link: brcm/brcmfmac4356-sdio.firefly,firefly-rk3399.txt -> brcmfmac4356-sdio.AP6356S.txt
>
> --
> 2.39.2
>

Re: [PATCH RESEND v2 02/16] WHENCE: remove unnecessary filename quotation

[Emil Velikov]

On Tue, 7 Mar 2023 at 14:56, Josh Boyer <jwboyer@kernel.org> wrote:
>
> This one no longer applies.  While the quotes might be unnecessary,
> they also don't hurt.
>

Yup, it's understandable. I might have written/sent the series a
couple of weeks ago. Cough cough wink wink :-P

I've dropped the quotation marks for consistency's sake - it's hard to
argue for dozens of cases out of ~2500 instances in-tree.
It will also make the sanity checks (latter part of the series) a bit
more hairy but it's manageable.

Do you agree with my sentiment - be that here or in the other email?
Unfortunately, I'm not the one to get subtle hints over email.

If you have concrete preferences - please voice them clearly. I'm
happy to redo the series.
Speaking of which: did you run out of time, or these comments are
blocking the rest of it?

Thanks again,
Emil

Re: [PATCH RESEND v2 02/16] WHENCE: remove unnecessary filename quotation

[Josh Boyer]

On Tue, Mar 7, 2023 at 12:54 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Tue, 7 Mar 2023 at 14:56, Josh Boyer <jwboyer@kernel.org> wrote:
> >
> > This one no longer applies.  While the quotes might be unnecessary,
> > they also don't hurt.
> >
>
> Yup, it's understandable. I might have written/sent the series a
> couple of weeks ago. Cough cough wink wink :-P

Yes, I'm aware.

> I've dropped the quotation marks for consistency's sake - it's hard to
> argue for dozens of cases out of ~2500 instances in-tree.
> It will also make the sanity checks (latter part of the series) a bit
> more hairy but it's manageable.
>
> Do you agree with my sentiment - be that here or in the other email?
> Unfortunately, I'm not the one to get subtle hints over email.
>
> If you have concrete preferences - please voice them clearly. I'm
> happy to redo the series.
> Speaking of which: did you run out of time, or these comments are
> blocking the rest of it?

Still working through things.

josh

[PATCH RESEND v2 03/16] check_whence, WHENCE, copy-firmware: escape filenames with spaces

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

We have a handful of instances which have space in the filename - just
escape them (like we do for the symlinks) and drop the extra re pattern.

With this we can also drop the extra sed pattern, stripping out the
quotation marks.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 WHENCE           | 10 +++++-----
 check_whence.py  |  8 ++------
 copy-firmware.sh |  2 +-
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/WHENCE b/WHENCE
index d4654dd..214952e 100644
--- a/WHENCE
+++ b/WHENCE
@@ -2690,9 +2690,9 @@ Link: brcm/brcmfmac54591-pcie.clm_blob -> ../cypress/cyfmac54591-pcie.clm_blob
 
 Licence: Redistributable. See LICENCE.cypress for details.
 
-File: "brcm/brcmfmac43241b4-sdio.Intel Corp.-VALLEYVIEW C0 PLATFORM.txt"
+File: brcm/brcmfmac43241b4-sdio.Intel\ Corp.-VALLEYVIEW\ C0\ PLATFORM.txt
 File: brcm/brcmfmac4330-sdio.Prowise-PT301.txt
-File: "brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-TF103CE.txt"
+File: brcm/brcmfmac43340-sdio.ASUSTeK\ COMPUTER\ INC.-TF103CE.txt
 File: brcm/brcmfmac43340-sdio.meegopad-t08.txt
 File: brcm/brcmfmac43340-sdio.pov-tab-p1006w-data.txt
 File: brcm/brcmfmac43340-sdio.predia-basic.txt
@@ -2703,7 +2703,7 @@ Link: brcm/brcmfmac43362-sdio.kobo,tolino-shine2hd.txt -> brcmfmac43362-sdio.WC1
 Link: brcm/brcmfmac43362-sdio.lemaker,bananapro.txt -> brcmfmac43362-sdio.cubietech,cubietruck.txt
 File: brcm/brcmfmac43430a0-sdio.ilife-S806.txt
 File: brcm/brcmfmac43430a0-sdio.jumper-ezpad-mini3.txt
-File: "brcm/brcmfmac43430a0-sdio.ONDA-V80 PLUS.txt"
+File: brcm/brcmfmac43430a0-sdio.ONDA-V80\ PLUS.txt
 File: brcm/brcmfmac43430-sdio.AP6212.txt
 Link: brcm/brcmfmac43430-sdio.sinovoip,bpi-m2-plus.txt -> brcmfmac43430-sdio.AP6212.txt
 Link: brcm/brcmfmac43430-sdio.sinovoip,bpi-m2-zero.txt -> brcmfmac43430-sdio.AP6212.txt
@@ -2724,9 +2724,9 @@ Link: brcm/brcmfmac43455-sdio.raspberrypi,3-model-a-plus.txt -> brcmfmac43455-sd
 File: brcm/brcmfmac43455-sdio.raspberrypi,4-model-b.txt
 Link: brcm/brcmfmac43455-sdio.Raspberry\ Pi\ Foundation-Raspberry\ Pi\ 4\ Model\ B.txt -> brcmfmac43455-sdio.raspberrypi,4-model-b.txt
 Link: brcm/brcmfmac43455-sdio.Raspberry\ Pi\ Foundation-Raspberry\ Pi\ Compute\ Module\ 4.txt -> brcmfmac43455-sdio.raspberrypi,4-model-b.txt
-File: "brcm/brcmfmac43455-sdio.MINIX-NEO Z83-4.txt"
+File: brcm/brcmfmac43455-sdio.MINIX-NEO\ Z83-4.txt
 File: brcm/brcmfmac4356-pcie.gpd-win-pocket.txt
-File: "brcm/brcmfmac4356-pcie.Xiaomi Inc-Mipad2.txt"
+File: brcm/brcmfmac4356-pcie.Xiaomi\ Inc-Mipad2.txt
 File: brcm/brcmfmac4356-sdio.AP6356S.txt
 Link: brcm/brcmfmac4356-sdio.firefly,firefly-rk3399.txt -> brcmfmac4356-sdio.AP6356S.txt
 Link: brcm/brcmfmac4356-sdio.khadas,vim2.txt -> brcmfmac4356-sdio.AP6356S.txt
diff --git a/check_whence.py b/check_whence.py
index 8805e99..f347f0e 100755
--- a/check_whence.py
+++ b/check_whence.py
@@ -6,13 +6,9 @@ from io import open
 def list_whence():
     with open('WHENCE', encoding='utf-8') as whence:
         for line in whence:
-            match = re.match(r'(?:File|Source):\s*"(.*)"', line)
+            match = re.match(r'(?:File|Source):\s*(.*)', line)
             if match:
-                yield match.group(1)
-                continue
-            match = re.match(r'(?:File|Source):\s*(\S*)', line)
-            if match:
-                yield match.group(1)
+                yield match.group(1).replace("\ ", " ")
                 continue
             match = re.match(r'Licen[cs]e: (?:.*\bSee (.*) for details\.?|(\S*))\n',
                              line)
diff --git a/copy-firmware.sh b/copy-firmware.sh
index bbacb92..6bceef3 100755
--- a/copy-firmware.sh
+++ b/copy-firmware.sh
@@ -31,7 +31,7 @@ while test $# -gt 0; do
     esac
 done
 
-grep '^File:' WHENCE | sed -e's/^File: *//g' -e's/"//g' | while read f; do
+grep '^File:' WHENCE | sed -e's/^File: *//g' | while read f; do
     test -f "$f" || continue
     $verbose "copying file $f"
     install -d $destdir/$(dirname "$f")

-- 
2.39.2

Re: [PATCH RESEND v2 03/16] check_whence, WHENCE, copy-firmware: escape filenames with spaces

[Josh Boyer]

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> We have a handful of instances which have space in the filename - just
> escape them (like we do for the symlinks) and drop the extra re pattern.
>
> With this we can also drop the extra sed pattern, stripping out the
> quotation marks.

While that is all true, why is it necessary?  I suspect patch
submitters are far more likely to forget to escape the spaces than
they are to just include quotes.  I see this change as making it
slightly harder to submit correct patches and I'm not sure it's worth
it.

josh

> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
>  WHENCE           | 10 +++++-----
>  check_whence.py  |  8 ++------
>  copy-firmware.sh |  2 +-
>  3 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/WHENCE b/WHENCE
> index d4654dd..214952e 100644
> --- a/WHENCE
> +++ b/WHENCE
> @@ -2690,9 +2690,9 @@ Link: brcm/brcmfmac54591-pcie.clm_blob -> ../cypress/cyfmac54591-pcie.clm_blob
>
>  Licence: Redistributable. See LICENCE.cypress for details.
>
> -File: "brcm/brcmfmac43241b4-sdio.Intel Corp.-VALLEYVIEW C0 PLATFORM.txt"
> +File: brcm/brcmfmac43241b4-sdio.Intel\ Corp.-VALLEYVIEW\ C0\ PLATFORM.txt
>  File: brcm/brcmfmac4330-sdio.Prowise-PT301.txt
> -File: "brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-TF103CE.txt"
> +File: brcm/brcmfmac43340-sdio.ASUSTeK\ COMPUTER\ INC.-TF103CE.txt
>  File: brcm/brcmfmac43340-sdio.meegopad-t08.txt
>  File: brcm/brcmfmac43340-sdio.pov-tab-p1006w-data.txt
>  File: brcm/brcmfmac43340-sdio.predia-basic.txt
> @@ -2703,7 +2703,7 @@ Link: brcm/brcmfmac43362-sdio.kobo,tolino-shine2hd.txt -> brcmfmac43362-sdio.WC1
>  Link: brcm/brcmfmac43362-sdio.lemaker,bananapro.txt -> brcmfmac43362-sdio.cubietech,cubietruck.txt
>  File: brcm/brcmfmac43430a0-sdio.ilife-S806.txt
>  File: brcm/brcmfmac43430a0-sdio.jumper-ezpad-mini3.txt
> -File: "brcm/brcmfmac43430a0-sdio.ONDA-V80 PLUS.txt"
> +File: brcm/brcmfmac43430a0-sdio.ONDA-V80\ PLUS.txt
>  File: brcm/brcmfmac43430-sdio.AP6212.txt
>  Link: brcm/brcmfmac43430-sdio.sinovoip,bpi-m2-plus.txt -> brcmfmac43430-sdio.AP6212.txt
>  Link: brcm/brcmfmac43430-sdio.sinovoip,bpi-m2-zero.txt -> brcmfmac43430-sdio.AP6212.txt
> @@ -2724,9 +2724,9 @@ Link: brcm/brcmfmac43455-sdio.raspberrypi,3-model-a-plus.txt -> brcmfmac43455-sd
>  File: brcm/brcmfmac43455-sdio.raspberrypi,4-model-b.txt
>  Link: brcm/brcmfmac43455-sdio.Raspberry\ Pi\ Foundation-Raspberry\ Pi\ 4\ Model\ B.txt -> brcmfmac43455-sdio.raspberrypi,4-model-b.txt
>  Link: brcm/brcmfmac43455-sdio.Raspberry\ Pi\ Foundation-Raspberry\ Pi\ Compute\ Module\ 4.txt -> brcmfmac43455-sdio.raspberrypi,4-model-b.txt
> -File: "brcm/brcmfmac43455-sdio.MINIX-NEO Z83-4.txt"
> +File: brcm/brcmfmac43455-sdio.MINIX-NEO\ Z83-4.txt
>  File: brcm/brcmfmac4356-pcie.gpd-win-pocket.txt
> -File: "brcm/brcmfmac4356-pcie.Xiaomi Inc-Mipad2.txt"
> +File: brcm/brcmfmac4356-pcie.Xiaomi\ Inc-Mipad2.txt
>  File: brcm/brcmfmac4356-sdio.AP6356S.txt
>  Link: brcm/brcmfmac4356-sdio.firefly,firefly-rk3399.txt -> brcmfmac4356-sdio.AP6356S.txt
>  Link: brcm/brcmfmac4356-sdio.khadas,vim2.txt -> brcmfmac4356-sdio.AP6356S.txt
> diff --git a/check_whence.py b/check_whence.py
> index 8805e99..f347f0e 100755
> --- a/check_whence.py
> +++ b/check_whence.py
> @@ -6,13 +6,9 @@ from io import open
>  def list_whence():
>      with open('WHENCE', encoding='utf-8') as whence:
>          for line in whence:
> -            match = re.match(r'(?:File|Source):\s*"(.*)"', line)
> +            match = re.match(r'(?:File|Source):\s*(.*)', line)
>              if match:
> -                yield match.group(1)
> -                continue
> -            match = re.match(r'(?:File|Source):\s*(\S*)', line)
> -            if match:
> -                yield match.group(1)
> +                yield match.group(1).replace("\ ", " ")
>                  continue
>              match = re.match(r'Licen[cs]e: (?:.*\bSee (.*) for details\.?|(\S*))\n',
>                               line)
> diff --git a/copy-firmware.sh b/copy-firmware.sh
> index bbacb92..6bceef3 100755
> --- a/copy-firmware.sh
> +++ b/copy-firmware.sh
> @@ -31,7 +31,7 @@ while test $# -gt 0; do
>      esac
>  done
>
> -grep '^File:' WHENCE | sed -e's/^File: *//g' -e's/"//g' | while read f; do
> +grep '^File:' WHENCE | sed -e's/^File: *//g' | while read f; do
>      test -f "$f" || continue
>      $verbose "copying file $f"
>      install -d $destdir/$(dirname "$f")
>
> --
> 2.39.2
>

Re: [PATCH RESEND v2 03/16] check_whence, WHENCE, copy-firmware: escape filenames with spaces

[Emil Velikov]

On Tue, 7 Mar 2023 at 14:58, Josh Boyer <jwboyer@kernel.org> wrote:
>
> On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
> <devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
> >
> > From: Emil Velikov <emil.l.velikov@gmail.com>
> >
> > We have a handful of instances which have space in the filename - just
> > escape them (like we do for the symlinks) and drop the extra re pattern.
> >
> > With this we can also drop the extra sed pattern, stripping out the
> > quotation marks.
>
> While that is all true, why is it necessary?  I suspect patch
> submitters are far more likely to forget to escape the spaces than
> they are to just include quotes.  I see this change as making it
> slightly harder to submit correct patches and I'm not sure it's worth
> it.
>

We already have properly escaped paths in WHENCE, so it only makes
sense to fix the outliers.
Would you be in favour if check_whence.py checks for quoted files and
produces meaningful error message?

Thanks
Emil

Re: [PATCH RESEND v2 03/16] check_whence, WHENCE, copy-firmware: escape filenames with spaces

[Josh Boyer]

On Tue, Mar 7, 2023 at 12:30 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Tue, 7 Mar 2023 at 14:58, Josh Boyer <jwboyer@kernel.org> wrote:
> >
> > On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
> > <devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
> > >
> > > From: Emil Velikov <emil.l.velikov@gmail.com>
> > >
> > > We have a handful of instances which have space in the filename - just
> > > escape them (like we do for the symlinks) and drop the extra re pattern.
> > >
> > > With this we can also drop the extra sed pattern, stripping out the
> > > quotation marks.
> >
> > While that is all true, why is it necessary?  I suspect patch
> > submitters are far more likely to forget to escape the spaces than
> > they are to just include quotes.  I see this change as making it
> > slightly harder to submit correct patches and I'm not sure it's worth
> > it.
> >
>
> We already have properly escaped paths in WHENCE, so it only makes
> sense to fix the outliers.

That's true if we believe escaped paths are the suggested way to do
it.  I'm questioning if we really want that.

If you're a firmware submitter, which is more natural to you?

File: "foo bar with spaces.bin"

or

File: foo\ bar\ with\ spaces.bin

Based on the number of occurrences in WHENCE, quoting the filename
seems to be more prevalent.

> Would you be in favour if check_whence.py checks for quoted files and
> produces meaningful error message?

Not really.  I don't understand why we have to force only one format.
The code already supports both.  What is the benefit of dropping that?

josh

Re: [PATCH RESEND v2 03/16] check_whence, WHENCE, copy-firmware: escape filenames with spaces

[Emil Velikov]

On Tue, 7 Mar 2023 at 18:00, Josh Boyer <jwboyer@kernel.org> wrote:
>
> On Tue, Mar 7, 2023 at 12:30 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > On Tue, 7 Mar 2023 at 14:58, Josh Boyer <jwboyer@kernel.org> wrote:
> > >
> > > On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
> > > <devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
> > > >
> > > > From: Emil Velikov <emil.l.velikov@gmail.com>
> > > >
> > > > We have a handful of instances which have space in the filename - just
> > > > escape them (like we do for the symlinks) and drop the extra re pattern.
> > > >
> > > > With this we can also drop the extra sed pattern, stripping out the
> > > > quotation marks.
> > >
> > > While that is all true, why is it necessary?  I suspect patch
> > > submitters are far more likely to forget to escape the spaces than
> > > they are to just include quotes.  I see this change as making it
> > > slightly harder to submit correct patches and I'm not sure it's worth
> > > it.
> > >
> >
> > We already have properly escaped paths in WHENCE, so it only makes
> > sense to fix the outliers.
>
> That's true if we believe escaped paths are the suggested way to do
> it.  I'm questioning if we really want that.
>
> If you're a firmware submitter, which is more natural to you?
>
> File: "foo bar with spaces.bin"
>
> or
>
> File: foo\ bar\ with\ spaces.bin
>

If I'm distracted I would use the following, which would be flagged by
check_whence.py and rectified immediately by skimming through WHENCE.
File: foo bar with spaces.bin

Although if I'm careful I would first grep through WHENCE for examples.

> Based on the number of occurrences in WHENCE, quoting the filename
> seems to be more prevalent.
>
> > Would you be in favour if check_whence.py checks for quoted files and
> > produces meaningful error message?
>
> Not really.  I don't understand why we have to force only one format.
> The code already supports both.  What is the benefit of dropping that?
>

Yes, escaped files are more common. And no the code does not support both.

Look at how Link: is handled. It fails miserably if either target or
link are quoted, regardless if they have space in the name.
Fixing that is an option, although since less than 1% is "quirky" it's
not worth adding any extra code. But fixing them instead.

Plus a mismatched quotation will (currently) lead to greater chaos, or
confusion if one is so inclined, IMHO.

Does that make sense?

-Emil

[PATCH RESEND v2 04/16] WHENCE: remove duplicate File entries

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 WHENCE | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/WHENCE b/WHENCE
index 214952e..7de6d6b 100644
--- a/WHENCE
+++ b/WHENCE
@@ -1073,10 +1073,6 @@ Version: 75.27f1c37b.0
 File: iwlwifi-so-a0-jf-b0-77.ucode
 Version: 74.f92b5fed.0
 
-File: iwlwifi-so-a0-gf-a0.pnvm
-
-File: iwlwifi-so-a0-gf4-a0.pnvm
-
 File: iwlwifi-ty-a0-gf-a0.pnvm
 
 Licence: Redistributable. See LICENCE.iwlwifi_firmware for details
@@ -2980,19 +2976,6 @@ Licence: Redistributable. See LICENCE.rtlwifi_firmware.txt for details.
 
 --------------------------------------------------------------------------
 
-Driver: r8723au - Realtek 802.11n WLAN driver for RTL8723AU
-
-Info: Taken from Realtek driver rtl8723A_WiFi_linux_v4.1.3_6044.20121224
-      Firmware is embedded in the driver as data statements. This info
-      has been extracted into a binary file.
-File: rtlwifi/rtl8723aufw_A.bin
-File: rtlwifi/rtl8723aufw_B.bin
-File: rtlwifi/rtl8723aufw_B_NoBT.bin
-
-Licence: Redistributable. See LICENCE.rtlwifi_firmware.txt for details.
-
---------------------------------------------------------------------------
-
 Driver: rtl8188ee - Realtek 802.11n WLAN driver for RTL8188EE
 
 Info: Taken from Realtek version rtl_92ce_92se_92de_8723ae_88ee_linux_mac80211_0010.0109.2013
@@ -5942,8 +5925,6 @@ Link: cirrus/cs35l41-dsp1-spk-prot-103c8995.wmfw -> cs35l41/v6.61.1/halo_cspl_RA
 Link: cirrus/cs35l41-dsp1-spk-cali-103c8995.wmfw -> cs35l41/v6.61.1/halo_cspl_RAM_revB2_29.63.1.wmfw
 Link: cirrus/cs35l41-dsp1-spk-prot-103c89c6.wmfw -> cs35l41/v6.61.1/halo_cspl_RAM_revB2_29.63.1.wmfw
 Link: cirrus/cs35l41-dsp1-spk-cali-103c89c6.wmfw -> cs35l41/v6.61.1/halo_cspl_RAM_revB2_29.63.1.wmfw
-File: cirrus/cs35l41-dsp1-spk-cali-103c8971.bin
-File: cirrus/cs35l41-dsp1-spk-prot-103c8971.bin
 File: cirrus/cs35l41-dsp1-spk-prot-103c8971.bin
 File: cirrus/cs35l41-dsp1-spk-cali-103c8971.bin
 File: cirrus/cs35l41-dsp1-spk-prot-103c8972.bin

-- 
2.39.2

Re: [PATCH RESEND v2 04/16] WHENCE: remove duplicate File entries

[Josh Boyer]

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>

Applied and pushed out.

josh

> ---
>  WHENCE | 19 -------------------
>  1 file changed, 19 deletions(-)
>
> diff --git a/WHENCE b/WHENCE
> index 214952e..7de6d6b 100644
> --- a/WHENCE
> +++ b/WHENCE
> @@ -1073,10 +1073,6 @@ Version: 75.27f1c37b.0
>  File: iwlwifi-so-a0-jf-b0-77.ucode
>  Version: 74.f92b5fed.0
>
> -File: iwlwifi-so-a0-gf-a0.pnvm
> -
> -File: iwlwifi-so-a0-gf4-a0.pnvm
> -
>  File: iwlwifi-ty-a0-gf-a0.pnvm
>
>  Licence: Redistributable. See LICENCE.iwlwifi_firmware for details
> @@ -2980,19 +2976,6 @@ Licence: Redistributable. See LICENCE.rtlwifi_firmware.txt for details.
>
>  --------------------------------------------------------------------------
>
> -Driver: r8723au - Realtek 802.11n WLAN driver for RTL8723AU
> -
> -Info: Taken from Realtek driver rtl8723A_WiFi_linux_v4.1.3_6044.20121224
> -      Firmware is embedded in the driver as data statements. This info
> -      has been extracted into a binary file.
> -File: rtlwifi/rtl8723aufw_A.bin
> -File: rtlwifi/rtl8723aufw_B.bin
> -File: rtlwifi/rtl8723aufw_B_NoBT.bin
> -
> -Licence: Redistributable. See LICENCE.rtlwifi_firmware.txt for details.
> -
> ---------------------------------------------------------------------------
> -
>  Driver: rtl8188ee - Realtek 802.11n WLAN driver for RTL8188EE
>
>  Info: Taken from Realtek version rtl_92ce_92se_92de_8723ae_88ee_linux_mac80211_0010.0109.2013
> @@ -5942,8 +5925,6 @@ Link: cirrus/cs35l41-dsp1-spk-prot-103c8995.wmfw -> cs35l41/v6.61.1/halo_cspl_RA
>  Link: cirrus/cs35l41-dsp1-spk-cali-103c8995.wmfw -> cs35l41/v6.61.1/halo_cspl_RAM_revB2_29.63.1.wmfw
>  Link: cirrus/cs35l41-dsp1-spk-prot-103c89c6.wmfw -> cs35l41/v6.61.1/halo_cspl_RAM_revB2_29.63.1.wmfw
>  Link: cirrus/cs35l41-dsp1-spk-cali-103c89c6.wmfw -> cs35l41/v6.61.1/halo_cspl_RAM_revB2_29.63.1.wmfw
> -File: cirrus/cs35l41-dsp1-spk-cali-103c8971.bin
> -File: cirrus/cs35l41-dsp1-spk-prot-103c8971.bin
>  File: cirrus/cs35l41-dsp1-spk-prot-103c8971.bin
>  File: cirrus/cs35l41-dsp1-spk-cali-103c8971.bin
>  File: cirrus/cs35l41-dsp1-spk-prot-103c8972.bin
>
> --
> 2.39.2
>

[PATCH RESEND v2 05/16] WHENCE: comment out duplicate MediaTek firmware

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

As mentioned in the inline comment - file is already listed further
down. To prevent check_whence.py errors, comment it out.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 WHENCE | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/WHENCE b/WHENCE
index 7de6d6b..bce94c5 100644
--- a/WHENCE
+++ b/WHENCE
@@ -5183,8 +5183,9 @@ File: mediatek/mt7622pr2h.bin
 Version: 20180621204904
 File: mediatek/mt7668pr2h.bin
 Version: 20180517181834
-File: mediatek/mt7663pr2h.bin
-Version: 7663e2ccn04-2006030247
+# Note: explicitly commented out, since it's duplicated further down
+# File: mediatek/mt7663pr2h.bin
+# Version: 7663e2ccn04-2006030247
 
 Licence: Redistributable. See LICENCE.mediatek for details.
 

-- 
2.39.2

Re: [PATCH RESEND v2 05/16] WHENCE: comment out duplicate MediaTek firmware

[Josh Boyer]

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> As mentioned in the inline comment - file is already listed further
> down. To prevent check_whence.py errors, comment it out.

Why did you comment this one out instead of remove it as duplicate
like the previous patch?

josh

>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
>  WHENCE | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/WHENCE b/WHENCE
> index 7de6d6b..bce94c5 100644
> --- a/WHENCE
> +++ b/WHENCE
> @@ -5183,8 +5183,9 @@ File: mediatek/mt7622pr2h.bin
>  Version: 20180621204904
>  File: mediatek/mt7668pr2h.bin
>  Version: 20180517181834
> -File: mediatek/mt7663pr2h.bin
> -Version: 7663e2ccn04-2006030247
> +# Note: explicitly commented out, since it's duplicated further down
> +# File: mediatek/mt7663pr2h.bin
> +# Version: 7663e2ccn04-2006030247
>
>  Licence: Redistributable. See LICENCE.mediatek for details.
>
>
> --
> 2.39.2
>

Re: [PATCH RESEND v2 05/16] WHENCE: comment out duplicate MediaTek firmware

[Emil Velikov]

On Tue, 7 Mar 2023 at 15:02, Josh Boyer <jwboyer@kernel.org> wrote:
>
> On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
> <devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
> >
> > From: Emil Velikov <emil.l.velikov@gmail.com>
> >
> > As mentioned in the inline comment - file is already listed further
> > down. To prevent check_whence.py errors, comment it out.
>
> Why did you comment this one out instead of remove it as duplicate
> like the previous patch?
>

In the previous patch, we had either a) duplicated within the same
section or b) section that is no longer applicable. Aka the r8723au
driver referenced does not existing in kernel.org
Here we have two drivers which use different firmware sets, which
partially overlap.

Dropping one or the other would be misleading, but I don't have a
strong opinion. Let me know what you prefer and I'll respin the patch.

Thanks
Emil

Re: [PATCH RESEND v2 05/16] WHENCE: comment out duplicate MediaTek firmware

[Josh Boyer]

On Tue, Mar 7, 2023 at 12:19 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Tue, 7 Mar 2023 at 15:02, Josh Boyer <jwboyer@kernel.org> wrote:
> >
> > On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
> > <devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
> > >
> > > From: Emil Velikov <emil.l.velikov@gmail.com>
> > >
> > > As mentioned in the inline comment - file is already listed further
> > > down. To prevent check_whence.py errors, comment it out.
> >
> > Why did you comment this one out instead of remove it as duplicate
> > like the previous patch?
> >
>
> In the previous patch, we had either a) duplicated within the same
> section or b) section that is no longer applicable. Aka the r8723au
> driver referenced does not existing in kernel.org
> Here we have two drivers which use different firmware sets, which
> partially overlap.

Ok.  That makes sense.

> Dropping one or the other would be misleading, but I don't have a
> strong opinion. Let me know what you prefer and I'll respin the patch.

I think this is a case where both should be listed.  If you comment
one out and the other entry gets removed, perhaps because none of the
upstream kernels support the driver any longer, then the installation
will not copy the file to disk and the driver that is still in-tree
will now have missing firmware.  I don't want to leave people without
a functioning system just because we wanted to avoid some text in
WHENCE.

josh

Re: [PATCH RESEND v2 05/16] WHENCE: comment out duplicate MediaTek firmware

[Emil Velikov]

On Tue, 7 Mar 2023 at 18:08, Josh Boyer <jwboyer@kernel.org> wrote:
>

> I think this is a case where both should be listed.  If you comment
> one out and the other entry gets removed, perhaps because none of the
> upstream kernels support the driver any longer, then the installation
> will not copy the file to disk and the driver that is still in-tree
> will now have missing firmware.  I don't want to leave people without
> a functioning system just because we wanted to avoid some text in
> WHENCE.
>

That's a valid point, although I wonder if we have greater odds of
witnessing the sun turn into red giant :-P

Joking aside - I'll drop this patch and 6/16 while adding a "grep foo
| sort -u" patch for copy-firmware.sh. Barring any objections of
course.

Thanks
Emil

Re: [PATCH RESEND v2 05/16] WHENCE: comment out duplicate MediaTek firmware

[Josh Boyer]

On Tue, Mar 7, 2023 at 3:57 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Tue, 7 Mar 2023 at 18:08, Josh Boyer <jwboyer@kernel.org> wrote:
> >
>
> > I think this is a case where both should be listed.  If you comment
> > one out and the other entry gets removed, perhaps because none of the
> > upstream kernels support the driver any longer, then the installation
> > will not copy the file to disk and the driver that is still in-tree
> > will now have missing firmware.  I don't want to leave people without
> > a functioning system just because we wanted to avoid some text in
> > WHENCE.
> >
>
> That's a valid point, although I wonder if we have greater odds of
> witnessing the sun turn into red giant :-P
>
> Joking aside - I'll drop this patch and 6/16 while adding a "grep foo
> | sort -u" patch for copy-firmware.sh. Barring any objections of
> course.

No objections.

josh

[PATCH RESEND v2 06/16] check_whence: error on duplicate file entries

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

There's little point in copying (or compressing with later patches) the
same files multiple times. So let's error out when duplicate entries are
present.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 check_whence.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/check_whence.py b/check_whence.py
index f347f0e..7ff21f6 100755
--- a/check_whence.py
+++ b/check_whence.py
@@ -24,6 +24,14 @@ def list_whence():
                         yield match.group(2)
                         continue
 
+def list_whence_files():
+    with open('WHENCE', encoding='utf-8') as whence:
+        for line in whence:
+            match = re.match(r'File:\s*(.*)', line)
+            if match:
+                yield match.group(1).replace("\ ", " ")
+                continue
+
 def list_git():
     with os.popen('git ls-files') as git_files:
         for line in git_files:
@@ -32,12 +40,17 @@ def list_git():
 def main():
     ret = 0
     whence_list = list(list_whence())
+    whence_files = list(list_whence_files())
     known_files = set(name for name in whence_list if not name.endswith('/')) | \
                   set(['check_whence.py', 'configure', 'Makefile',
                        'README', 'copy-firmware.sh', 'WHENCE'])
     known_prefixes = set(name for name in whence_list if name.endswith('/'))
     git_files = set(list_git())
 
+    for name in set(fw for fw in whence_files if whence_files.count(fw) > 1):
+        sys.stderr.write('E: %s listed in WHENCE twice\n' % name)
+        ret = 1
+
     for name in sorted(list(known_files - git_files)):
         sys.stderr.write('E: %s listed in WHENCE does not exist\n' % name)
         ret = 1

-- 
2.39.2

Re: [PATCH RESEND v2 06/16] check_whence: error on duplicate file entries

[Josh Boyer]

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> There's little point in copying (or compressing with later patches) the
> same files multiple times. So let's error out when duplicate entries are
> present.

I like this idea, but see my reply to patch 5.  There are cases where
the same firmware file *should* be duplicated because the driver using
it is different.  That would imply this would need to build out a
dictionary that can map files to drivers.  That seems over-complicated
for the fundamental goal of this check.

> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
>  check_whence.py | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/check_whence.py b/check_whence.py
> index f347f0e..7ff21f6 100755
> --- a/check_whence.py
> +++ b/check_whence.py
> @@ -24,6 +24,14 @@ def list_whence():
>                          yield match.group(2)
>                          continue
>
> +def list_whence_files():
> +    with open('WHENCE', encoding='utf-8') as whence:
> +        for line in whence:
> +            match = re.match(r'File:\s*(.*)', line)
> +            if match:
> +                yield match.group(1).replace("\ ", " ")
> +                continue
> +
>  def list_git():
>      with os.popen('git ls-files') as git_files:
>          for line in git_files:
> @@ -32,12 +40,17 @@ def list_git():
>  def main():
>      ret = 0
>      whence_list = list(list_whence())
> +    whence_files = list(list_whence_files())
>      known_files = set(name for name in whence_list if not name.endswith('/')) | \
>                    set(['check_whence.py', 'configure', 'Makefile',
>                         'README', 'copy-firmware.sh', 'WHENCE'])
>      known_prefixes = set(name for name in whence_list if name.endswith('/'))
>      git_files = set(list_git())
>
> +    for name in set(fw for fw in whence_files if whence_files.count(fw) > 1):
> +        sys.stderr.write('E: %s listed in WHENCE twice\n' % name)
> +        ret = 1
> +

Perhaps we can add this check via a cmdline option that is run
occasionally to look for duplicates?  Then you could still keep
simpler code for anything that's operating on the files themselves.

josh

>      for name in sorted(list(known_files - git_files)):
>          sys.stderr.write('E: %s listed in WHENCE does not exist\n' % name)
>          ret = 1
>
> --
> 2.39.2
>

[PATCH RESEND v2 07/16] check_whence: error on directory listed as File

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

No occurrences in-tree, but it makes sense to sanity check that.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 check_whence.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/check_whence.py b/check_whence.py
index 7ff21f6..130966e 100755
--- a/check_whence.py
+++ b/check_whence.py
@@ -47,6 +47,11 @@ def main():
     known_prefixes = set(name for name in whence_list if name.endswith('/'))
     git_files = set(list_git())
 
+    for name in set(name for name in whence_files if name.endswith('/')):
+        sys.stderr.write('E: %s listed in WHENCE as File, but is directory\n' %
+                         name)
+        ret = 1
+
     for name in set(fw for fw in whence_files if whence_files.count(fw) > 1):
         sys.stderr.write('E: %s listed in WHENCE twice\n' % name)
         ret = 1

-- 
2.39.2

Re: [PATCH RESEND v2 07/16] check_whence: error on directory listed as File

[Emil Velikov]

On Wed, 1 Mar 2023 at 18:56, Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> No occurrences in-tree, but it makes sense to sanity check that.
>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
>  check_whence.py | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/check_whence.py b/check_whence.py
> index 7ff21f6..130966e 100755
> --- a/check_whence.py
> +++ b/check_whence.py
> @@ -47,6 +47,11 @@ def main():
>      known_prefixes = set(name for name in whence_list if name.endswith('/'))
>      git_files = set(list_git())
>
> +    for name in set(name for name in whence_files if name.endswith('/')):
> +        sys.stderr.write('E: %s listed in WHENCE as File, but is directory\n' %
> +                         name)
> +        ret = 1
> +
>      for name in set(fw for fw in whence_files if whence_files.count(fw) > 1):
>          sys.stderr.write('E: %s listed in WHENCE twice\n' % name)
>          ret = 1
>
> --
> 2.39.2
>

Josh, humble poke about the remainder of the series. Would you have
any comments or suggestions?
Note that many parts can be picked individually. Should there be any
merge conflicts, those will be dead trivial to resolve.

Thanks
Emil

Re: [PATCH RESEND v2 07/16] check_whence: error on directory listed as File

[Emil Velikov]

On Mon, 13 Mar 2023 at 20:24, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Wed, 1 Mar 2023 at 18:56, Emil Velikov via B4 Relay
> <devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
> >
> > From: Emil Velikov <emil.l.velikov@gmail.com>
> >
> > No occurrences in-tree, but it makes sense to sanity check that.
> >
> > Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> > ---
> >  check_whence.py | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/check_whence.py b/check_whence.py
> > index 7ff21f6..130966e 100755
> > --- a/check_whence.py
> > +++ b/check_whence.py
> > @@ -47,6 +47,11 @@ def main():
> >      known_prefixes = set(name for name in whence_list if name.endswith('/'))
> >      git_files = set(list_git())
> >
> > +    for name in set(name for name in whence_files if name.endswith('/')):
> > +        sys.stderr.write('E: %s listed in WHENCE as File, but is directory\n' %
> > +                         name)
> > +        ret = 1
> > +
> >      for name in set(fw for fw in whence_files if whence_files.count(fw) > 1):
> >          sys.stderr.write('E: %s listed in WHENCE twice\n' % name)
> >          ret = 1
> >
> > --
> > 2.39.2
> >
>
> Josh, humble poke about the remainder of the series. Would you have
> any comments or suggestions?
> Note that many parts can be picked individually. Should there be any
> merge conflicts, those will be dead trivial to resolve.
>

Humble weekly ping?

Emil

Re: [PATCH RESEND v2 07/16] check_whence: error on directory listed as File

[Emil Velikov]

On Tue, 21 Mar 2023 at 10:31, Emil Velikov <emil.l.velikov@gmail.com> wrote:

> > Josh, humble poke about the remainder of the series. Would you have
> > any comments or suggestions?
> > Note that many parts can be picked individually. Should there be any
> > merge conflicts, those will be dead trivial to resolve.
> >
>
> Humble weekly ping?
>

Another humble ping?

Josh, are you too busy these days to look at linux-firmware? If so I
can stop the weekly pings (for say a month), yet I would love to see
this move forward in one shape or another.

> Emil

[PATCH RESEND v2 08/16] copy-firmware: remove non-applicable file presence test

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

The check_whence.py script already ensures that the files listed in
WHENCE are present on-disk. Thus the extra check isn't needed.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 copy-firmware.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/copy-firmware.sh b/copy-firmware.sh
index 6bceef3..656b0ec 100755
--- a/copy-firmware.sh
+++ b/copy-firmware.sh
@@ -32,7 +32,6 @@ while test $# -gt 0; do
 done
 
 grep '^File:' WHENCE | sed -e's/^File: *//g' | while read f; do
-    test -f "$f" || continue
     $verbose "copying file $f"
     install -d $destdir/$(dirname "$f")
     cp -d "$f" $destdir/"$f"

-- 
2.39.2

Re: [PATCH RESEND v2 08/16] copy-firmware: remove non-applicable file presence test

[Josh Boyer]

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> The check_whence.py script already ensures that the files listed in
> WHENCE are present on-disk. Thus the extra check isn't needed.

check_whence.py isn't run during installation though.  It's really
only there as a repo validation script.  There's no harm in keeping
this test.

josh

> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
>  copy-firmware.sh | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/copy-firmware.sh b/copy-firmware.sh
> index 6bceef3..656b0ec 100755
> --- a/copy-firmware.sh
> +++ b/copy-firmware.sh
> @@ -32,7 +32,6 @@ while test $# -gt 0; do
>  done
>
>  grep '^File:' WHENCE | sed -e's/^File: *//g' | while read f; do
> -    test -f "$f" || continue
>      $verbose "copying file $f"
>      install -d $destdir/$(dirname "$f")
>      cp -d "$f" $destdir/"$f"
>
> --
> 2.39.2
>

[PATCH RESEND v2 09/16] check_whence: error if File: is actually a link

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

This allows us to remove the -d aka --no-dereference --preserve=links
from the copy command.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 check_whence.py  | 5 +++++
 copy-firmware.sh | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/check_whence.py b/check_whence.py
index 130966e..92b104e 100755
--- a/check_whence.py
+++ b/check_whence.py
@@ -56,6 +56,11 @@ def main():
         sys.stderr.write('E: %s listed in WHENCE twice\n' % name)
         ret = 1
 
+    for name in set(link for link in whence_files if os.path.islink(link)):
+        sys.stderr.write('E: %s listed in WHENCE as File, but is a symlink\n' %
+                         name)
+        ret = 1
+
     for name in sorted(list(known_files - git_files)):
         sys.stderr.write('E: %s listed in WHENCE does not exist\n' % name)
         ret = 1
diff --git a/copy-firmware.sh b/copy-firmware.sh
index 656b0ec..e3f3176 100755
--- a/copy-firmware.sh
+++ b/copy-firmware.sh
@@ -34,7 +34,7 @@ done
 grep '^File:' WHENCE | sed -e's/^File: *//g' | while read f; do
     $verbose "copying file $f"
     install -d $destdir/$(dirname "$f")
-    cp -d "$f" $destdir/"$f"
+    cp "$f" $destdir/"$f"
 done
 
 grep -E '^Link:' WHENCE | sed -e's/^Link: *//g' -e's/-> //g' | while read f d; do

-- 
2.39.2

Re: [PATCH RESEND v2 09/16] check_whence: error if File: is actually a link

[Josh Boyer]

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> This allows us to remove the -d aka --no-dereference --preserve=links
> from the copy command.

This again relies on check_whence.py doing something that the
installation script then assumes is done but there's nothing to
enforce that.  I'm inclined to be safe rather than assumptive.

josh

>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
>  check_whence.py  | 5 +++++
>  copy-firmware.sh | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/check_whence.py b/check_whence.py
> index 130966e..92b104e 100755
> --- a/check_whence.py
> +++ b/check_whence.py
> @@ -56,6 +56,11 @@ def main():
>          sys.stderr.write('E: %s listed in WHENCE twice\n' % name)
>          ret = 1
>
> +    for name in set(link for link in whence_files if os.path.islink(link)):
> +        sys.stderr.write('E: %s listed in WHENCE as File, but is a symlink\n' %
> +                         name)
> +        ret = 1
> +
>      for name in sorted(list(known_files - git_files)):
>          sys.stderr.write('E: %s listed in WHENCE does not exist\n' % name)
>          ret = 1
> diff --git a/copy-firmware.sh b/copy-firmware.sh
> index 656b0ec..e3f3176 100755
> --- a/copy-firmware.sh
> +++ b/copy-firmware.sh
> @@ -34,7 +34,7 @@ done
>  grep '^File:' WHENCE | sed -e's/^File: *//g' | while read f; do
>      $verbose "copying file $f"
>      install -d $destdir/$(dirname "$f")
> -    cp -d "$f" $destdir/"$f"
> +    cp "$f" $destdir/"$f"
>  done
>
>  grep -E '^Link:' WHENCE | sed -e's/^Link: *//g' -e's/-> //g' | while read f d; do
>
> --
> 2.39.2
>

[PATCH RESEND v2 10/16] check_whence: error if symlinks are in-tree

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

Currently we have no symlinks in-tree. Add a simple check, ensuring they
don't get added in the future.

This allows us to remove the clunky symlink checking code in
copy-firmware.sh

v2:
 - tweak helper to produce link and target (based off Adam's patch)

Cc: Adam Sampson <ats@offog.org>
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
Adam, you should be able to use this helper with your fixes.

Note that symlinks should not point to another symlink, so you might
want to tweak your patch to check and error out in such cases.

HTH o/
---
 check_whence.py | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/check_whence.py b/check_whence.py
index 92b104e..f94037d 100755
--- a/check_whence.py
+++ b/check_whence.py
@@ -32,6 +32,23 @@ def list_whence_files():
                 yield match.group(1).replace("\ ", " ")
                 continue
 
+def list_links_list():
+    with open('WHENCE', encoding='utf-8') as whence:
+        for line in whence:
+            match = re.match(r'Link:\s*(.*)', line)
+            if match:
+                linkname, target = match.group(1).split("->")
+
+                linkname = linkname.strip().replace("\ ", " ")
+                target = target.strip().replace("\ ", " ")
+
+                # Link target is relative to the link
+                target = os.path.join(os.path.dirname(linkname), target)
+                target = os.path.normpath(target)
+
+                yield (linkname, target)
+                continue
+
 def list_git():
     with os.popen('git ls-files') as git_files:
         for line in git_files:
@@ -41,6 +58,7 @@ def main():
     ret = 0
     whence_list = list(list_whence())
     whence_files = list(list_whence_files())
+    links_list = list(list_links_list())
     known_files = set(name for name in whence_list if not name.endswith('/')) | \
                   set(['check_whence.py', 'configure', 'Makefile',
                        'README', 'copy-firmware.sh', 'WHENCE'])
@@ -61,6 +79,10 @@ def main():
                          name)
         ret = 1
 
+    for name in set(link[0] for link in links_list if os.path.islink(link[0])):
+        sys.stderr.write('E: %s listed in WHENCE as Link, is in tree\n' % name)
+        ret = 1
+
     for name in sorted(list(known_files - git_files)):
         sys.stderr.write('E: %s listed in WHENCE does not exist\n' % name)
         ret = 1

-- 
2.39.2

Re: [PATCH RESEND v2 10/16] check_whence: error if symlinks are in-tree

[Josh Boyer]

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> Currently we have no symlinks in-tree. Add a simple check, ensuring they
> don't get added in the future.
>
> This allows us to remove the clunky symlink checking code in
> copy-firmware.sh

I like the addition to check_whence, but this no longer applies
cleanly and I don't think we should remove checks from
copy-firmware.sh

josh

>
> v2:
>  - tweak helper to produce link and target (based off Adam's patch)
>
> Cc: Adam Sampson <ats@offog.org>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
> Adam, you should be able to use this helper with your fixes.
>
> Note that symlinks should not point to another symlink, so you might
> want to tweak your patch to check and error out in such cases.
>
> HTH o/
> ---
>  check_whence.py | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/check_whence.py b/check_whence.py
> index 92b104e..f94037d 100755
> --- a/check_whence.py
> +++ b/check_whence.py
> @@ -32,6 +32,23 @@ def list_whence_files():
>                  yield match.group(1).replace("\ ", " ")
>                  continue
>
> +def list_links_list():
> +    with open('WHENCE', encoding='utf-8') as whence:
> +        for line in whence:
> +            match = re.match(r'Link:\s*(.*)', line)
> +            if match:
> +                linkname, target = match.group(1).split("->")
> +
> +                linkname = linkname.strip().replace("\ ", " ")
> +                target = target.strip().replace("\ ", " ")
> +
> +                # Link target is relative to the link
> +                target = os.path.join(os.path.dirname(linkname), target)
> +                target = os.path.normpath(target)
> +
> +                yield (linkname, target)
> +                continue
> +
>  def list_git():
>      with os.popen('git ls-files') as git_files:
>          for line in git_files:
> @@ -41,6 +58,7 @@ def main():
>      ret = 0
>      whence_list = list(list_whence())
>      whence_files = list(list_whence_files())
> +    links_list = list(list_links_list())
>      known_files = set(name for name in whence_list if not name.endswith('/')) | \
>                    set(['check_whence.py', 'configure', 'Makefile',
>                         'README', 'copy-firmware.sh', 'WHENCE'])
> @@ -61,6 +79,10 @@ def main():
>                           name)
>          ret = 1
>
> +    for name in set(link[0] for link in links_list if os.path.islink(link[0])):
> +        sys.stderr.write('E: %s listed in WHENCE as Link, is in tree\n' % name)
> +        ret = 1
> +
>      for name in sorted(list(known_files - git_files)):
>          sys.stderr.write('E: %s listed in WHENCE does not exist\n' % name)
>          ret = 1
>
> --
> 2.39.2
>

[PATCH RESEND v2 11/16] copy-firmware: remove unreachable symlink workarounds

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

There are no symlinks in-tree and the earlier commit to check_whence.py
ensures new ones don't get added. Thus we can remove the consistency
checks and pruning code.

With this done we can also drop:
 - the --force argument for ln
 - the prune argument and related handling

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 copy-firmware.sh | 36 +++---------------------------------
 1 file changed, 3 insertions(+), 33 deletions(-)

diff --git a/copy-firmware.sh b/copy-firmware.sh
index e3f3176..0132244 100755
--- a/copy-firmware.sh
+++ b/copy-firmware.sh
@@ -5,7 +5,6 @@
 #
 
 verbose=:
-prune=no
 
 while test $# -gt 0; do
     case $1 in
@@ -14,11 +13,6 @@ while test $# -gt 0; do
             shift
             ;;
 
-        -P | --prune)
-            prune=yes
-            shift
-            ;;
-
         *)
             if test "x$destdir" != "x"; then
                 echo "ERROR: unknown command-line options: $@"
@@ -38,33 +32,9 @@ grep '^File:' WHENCE | sed -e's/^File: *//g' | while read f; do
 done
 
 grep -E '^Link:' WHENCE | sed -e's/^Link: *//g' -e's/-> //g' | while read f d; do
-    if test -L "$f"; then
-        test -f "$destdir/$f" && continue
-        $verbose "copying link $f"
-        install -d $destdir/$(dirname "$f")
-        cp -d "$f" $destdir/"$f"
-
-        if test "x$d" != "x"; then
-            target=`readlink "$f"`
-
-            if test "x$target" != "x$d"; then
-                $verbose "WARNING: inconsistent symlink target: $target != $d"
-            else
-                if test "x$prune" != "xyes"; then
-                    $verbose "WARNING: unneeded symlink detected: $f"
-                else
-                    $verbose "WARNING: pruning unneeded symlink $f"
-                    rm -f "$f"
-                fi
-            fi
-        else
-            $verbose "WARNING: missing target for symlink $f"
-        fi
-    else
-        $verbose "creating link $f -> $d"
-        install -d $destdir/$(dirname "$f")
-        ln -sf "$d" "$destdir/$f"
-    fi
+    $verbose "creating link $f -> $d"
+    install -d $destdir/$(dirname "$f")
+    ln -s "$d" "$destdir/$f"
 done
 
 exit 0

-- 
2.39.2

[PATCH RESEND v2 12/16] copy-firmware: quote deskdir to prevent word splitting

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 copy-firmware.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/copy-firmware.sh b/copy-firmware.sh
index 0132244..c6a2fe9 100755
--- a/copy-firmware.sh
+++ b/copy-firmware.sh
@@ -27,13 +27,13 @@ done
 
 grep '^File:' WHENCE | sed -e's/^File: *//g' | while read f; do
     $verbose "copying file $f"
-    install -d $destdir/$(dirname "$f")
-    cp "$f" $destdir/"$f"
+    install -d "$destdir"/$(dirname "$f")
+    cp "$f" "$destdir/$f"
 done
 
 grep -E '^Link:' WHENCE | sed -e's/^Link: *//g' -e's/-> //g' | while read f d; do
     $verbose "creating link $f -> $d"
-    install -d $destdir/$(dirname "$f")
+    install -d "$destdir"/$(dirname "$f")
     ln -s "$d" "$destdir/$f"
 done
 

-- 
2.39.2

Re: [PATCH RESEND v2 12/16] copy-firmware: quote deskdir to prevent word splitting

[Josh Boyer]

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>

Can you send this separately?  It doesn't apply cleanly because of
some of the other changes made in the series.

josh

> ---
>  copy-firmware.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/copy-firmware.sh b/copy-firmware.sh
> index 0132244..c6a2fe9 100755
> --- a/copy-firmware.sh
> +++ b/copy-firmware.sh
> @@ -27,13 +27,13 @@ done
>
>  grep '^File:' WHENCE | sed -e's/^File: *//g' | while read f; do
>      $verbose "copying file $f"
> -    install -d $destdir/$(dirname "$f")
> -    cp "$f" $destdir/"$f"
> +    install -d "$destdir"/$(dirname "$f")
> +    cp "$f" "$destdir/$f"
>  done
>
>  grep -E '^Link:' WHENCE | sed -e's/^Link: *//g' -e's/-> //g' | while read f d; do
>      $verbose "creating link $f -> $d"
> -    install -d $destdir/$(dirname "$f")
> +    install -d "$destdir"/$(dirname "$f")
>      ln -s "$d" "$destdir/$f"
>  done
>
>
> --
> 2.39.2
>

[PATCH RESEND v2 13/16] copy-firmware: tweak sed invocation

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

Add space between the arguments and pattern and combine patterns where
possible.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 copy-firmware.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/copy-firmware.sh b/copy-firmware.sh
index c6a2fe9..722227f 100755
--- a/copy-firmware.sh
+++ b/copy-firmware.sh
@@ -25,13 +25,13 @@ while test $# -gt 0; do
     esac
 done
 
-grep '^File:' WHENCE | sed -e's/^File: *//g' | while read f; do
+grep '^File:' WHENCE | sed -e 's/^File: *//g' | while read f; do
     $verbose "copying file $f"
     install -d "$destdir"/$(dirname "$f")
     cp "$f" "$destdir/$f"
 done
 
-grep -E '^Link:' WHENCE | sed -e's/^Link: *//g' -e's/-> //g' | while read f d; do
+grep -E '^Link:' WHENCE | sed -e 's/^Link: *//g;s/-> //g' | while read f d; do
     $verbose "creating link $f -> $d"
     install -d "$destdir"/$(dirname "$f")
     ln -s "$d" "$destdir/$f"

-- 
2.39.2

Re: [PATCH RESEND v2 13/16] copy-firmware: tweak sed invocation

[Josh Boyer]

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> Add space between the arguments and pattern and combine patterns where
> possible.

Same request here for a separate patch not dependent on the series.

josh

>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
>  copy-firmware.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/copy-firmware.sh b/copy-firmware.sh
> index c6a2fe9..722227f 100755
> --- a/copy-firmware.sh
> +++ b/copy-firmware.sh
> @@ -25,13 +25,13 @@ while test $# -gt 0; do
>      esac
>  done
>
> -grep '^File:' WHENCE | sed -e's/^File: *//g' | while read f; do
> +grep '^File:' WHENCE | sed -e 's/^File: *//g' | while read f; do
>      $verbose "copying file $f"
>      install -d "$destdir"/$(dirname "$f")
>      cp "$f" "$destdir/$f"
>  done
>
> -grep -E '^Link:' WHENCE | sed -e's/^Link: *//g' -e's/-> //g' | while read f d; do
> +grep -E '^Link:' WHENCE | sed -e 's/^Link: *//g;s/-> //g' | while read f d; do
>      $verbose "creating link $f -> $d"
>      install -d "$destdir"/$(dirname "$f")
>      ln -s "$d" "$destdir/$f"
>
> --
> 2.39.2
>

[PATCH RESEND v2 14/16] copy-firmware: quote the output of dirname

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

As highlighted by shellcheck - as-is one could get word splitting. That
isn't an issue, yet, since none of the in-tree directories have space in
their name.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 copy-firmware.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/copy-firmware.sh b/copy-firmware.sh
index 722227f..5a5e1a8 100755
--- a/copy-firmware.sh
+++ b/copy-firmware.sh
@@ -27,13 +27,13 @@ done
 
 grep '^File:' WHENCE | sed -e 's/^File: *//g' | while read f; do
     $verbose "copying file $f"
-    install -d "$destdir"/$(dirname "$f")
+    install -d "$destdir/$(dirname "$f")"
     cp "$f" "$destdir/$f"
 done
 
 grep -E '^Link:' WHENCE | sed -e 's/^Link: *//g;s/-> //g' | while read f d; do
     $verbose "creating link $f -> $d"
-    install -d "$destdir"/$(dirname "$f")
+    install -d "$destdir/$(dirname "$f")"
     ln -s "$d" "$destdir/$f"
 done
 

-- 
2.39.2

[PATCH RESEND v2 15/16] copy-firmware: silence the last shellcheck warnings

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 copy-firmware.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/copy-firmware.sh b/copy-firmware.sh
index 5a5e1a8..a52b847 100755
--- a/copy-firmware.sh
+++ b/copy-firmware.sh
@@ -9,13 +9,14 @@ verbose=:
 while test $# -gt 0; do
     case $1 in
         -v | --verbose)
+            # shellcheck disable=SC2209
             verbose=echo
             shift
             ;;
 
         *)
             if test "x$destdir" != "x"; then
-                echo "ERROR: unknown command-line options: $@"
+                echo "ERROR: unknown command-line options: $*"
                 exit 1
             fi
 
@@ -25,12 +26,14 @@ while test $# -gt 0; do
     esac
 done
 
+# shellcheck disable=SC2162
 grep '^File:' WHENCE | sed -e 's/^File: *//g' | while read f; do
     $verbose "copying file $f"
     install -d "$destdir/$(dirname "$f")"
     cp "$f" "$destdir/$f"
 done
 
+# shellcheck disable=SC2162
 grep -E '^Link:' WHENCE | sed -e 's/^Link: *//g;s/-> //g' | while read f d; do
     $verbose "creating link $f -> $d"
     install -d "$destdir/$(dirname "$f")"

-- 
2.39.2

[PATCH RESEND v2 16/16] Makefile, copy-firmware: support xz/zstd compressed firmware

[Emil Velikov via B4 Relay]

From: Emil Velikov <emil.l.velikov@gmail.com>

The kernel has supported compressed firmware for quite some time. So
let's add a couple of targets to produce that. In practical terms this
means it we'll use ~5x times less space on disk.

Reportedly the amd ucode, needs to be uncompressed _within_ the
initrd in order to work. Using compressed ucode in late load just works.
Ideally this will be addressed by the initrd generators, but considering
the files are tiny in size let's skip the compression.

v2
 - commit message, skip compression for files annotated as Raw

Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 Makefile         |  8 ++++++++
 WHENCE           |  5 +++++
 copy-firmware.sh | 38 ++++++++++++++++++++++++++++++++++----
 3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 83a0ec6..50a4c29 100644
--- a/Makefile
+++ b/Makefile
@@ -11,3 +11,11 @@ check:
 install:
 	install -d $(DESTDIR)$(FIRMWAREDIR)
 	./copy-firmware.sh $(DESTDIR)$(FIRMWAREDIR)
+
+install-xz:
+	install -d $(DESTDIR)$(FIRMWAREDIR)
+	./copy-firmware.sh --xz $(DESTDIR)$(FIRMWAREDIR)
+
+install-zst:
+	install -d $(DESTDIR)$(FIRMWAREDIR)
+	./copy-firmware.sh --zstd $(DESTDIR)$(FIRMWAREDIR)
diff --git a/WHENCE b/WHENCE
index bce94c5..6e44afa 100644
--- a/WHENCE
+++ b/WHENCE
@@ -3878,14 +3878,19 @@ License: Redistributable. See LICENSE.amd-sev for details
 Driver: microcode_amd - AMD CPU Microcode Update Driver for Linux
 
 File: amd-ucode/microcode_amd.bin
+Raw: amd-ucode/microcode_amd.bin
 Version: 2013-07-10
 File: amd-ucode/microcode_amd_fam15h.bin
+Raw: amd-ucode/microcode_amd_fam15h.bin
 Version: 2018-05-24
 File: amd-ucode/microcode_amd_fam16h.bin
+Raw: amd-ucode/microcode_amd_fam16h.bin
 Version: 2014-10-28
 File: amd-ucode/microcode_amd_fam17h.bin
+Raw: amd-ucode/microcode_amd_fam17h.bin
 Version: 2022-04-08
 File: amd-ucode/microcode_amd_fam19h.bin
+Raw: amd-ucode/microcode_amd_fam19h.bin
 Version: 2023-01-31
 File: amd-ucode/README
 
diff --git a/copy-firmware.sh b/copy-firmware.sh
index a52b847..6da9111 100755
--- a/copy-firmware.sh
+++ b/copy-firmware.sh
@@ -5,6 +5,9 @@
 #
 
 verbose=:
+# shellcheck disable=SC2209
+compress=cat
+compext=
 
 while test $# -gt 0; do
     case $1 in
@@ -14,6 +17,28 @@ while test $# -gt 0; do
             shift
             ;;
 
+        --xz)
+            if test "$compext" == ".zst"; then
+                echo "ERROR: cannot mix XZ and ZSTD compression"
+                exit 1
+            fi
+            compress="xz --compress --quiet --stdout --check=crc32"
+            compext=".xz"
+            shift
+            ;;
+
+        --zstd)
+            if test "$compext" == ".xz"; then
+                echo "ERROR: cannot mix XZ and ZSTD compression"
+                exit 1
+            fi
+            # shellcheck disable=SC2209
+            verbose=echo
+            compress="zstd --compress --quiet --stdout"
+            compext=".zst"
+            shift
+            ;;
+
         *)
             if test "x$destdir" != "x"; then
                 echo "ERROR: unknown command-line options: $*"
@@ -28,16 +53,21 @@ done
 
 # shellcheck disable=SC2162
 grep '^File:' WHENCE | sed -e 's/^File: *//g' | while read f; do
-    $verbose "copying file $f"
     install -d "$destdir/$(dirname "$f")"
-    cp "$f" "$destdir/$f"
+    $verbose "copying/compressing file $f$compext"
+    if test "$compress" != "cat" && grep -q "^Raw: $f\$" WHENCE; then
+        $verbose "compression will be skipped for file $f"
+        cat "$f" > "$destdir/$f"
+    else
+        $compress "$f" > "$destdir/$f$compext"
+    fi
 done
 
 # shellcheck disable=SC2162
 grep -E '^Link:' WHENCE | sed -e 's/^Link: *//g;s/-> //g' | while read f d; do
-    $verbose "creating link $f -> $d"
     install -d "$destdir/$(dirname "$f")"
-    ln -s "$d" "$destdir/$f"
+    $verbose "creating link $f$compext -> $d$compext"
+    ln -s "$d$compext" "$destdir/$f$compext"
 done
 
 exit 0

-- 
2.39.2

Re: [PATCH RESEND v2 16/16] Makefile, copy-firmware: support xz/zstd compressed firmware

[Emil Velikov]

On Wed, 1 Mar 2023 at 18:56, Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> From: Emil Velikov <emil.l.velikov@gmail.com>
>
> The kernel has supported compressed firmware for quite some time. So
> let's add a couple of targets to produce that. In practical terms this
> means it we'll use ~5x times less space on disk.
>
> Reportedly the amd ucode, needs to be uncompressed _within_ the
> initrd in order to work. Using compressed ucode in late load just works.
> Ideally this will be addressed by the initrd generators, but considering
> the files are tiny in size let's skip the compression.
>
> v2
>  - commit message, skip compression for files annotated as Raw
>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---

David, does the updated patch (or the rest of the series for that
matter) seem reasonable to you?
Josh, is there anything I can do to move this forward?

Thanks
Emil

Re: [PATCH RESEND v2 00/16] Misc fixes, sanity checks and xz/zstd compression

[Josh Boyer]

I applied a few more of the patches, but generally speaking some of
the changes conflate check_whence.py and copy-firmware.sh.  I think
they should remain independent scripts and not make assumptions that
one is doing something on behalf of the other.

josh

On Wed, Mar 1, 2023 at 1:56 PM Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> Hey everyone,
>
> Here's an updated series which fixes some outstanding issues, enhances
> the check_whence checks and adds xz/zstd firmware support.
>
> v2:
>  - Commend out duplicated MediaTek firmware
>  - Tweak list_links_list() after Adam's approach
>  - Skip compression of amd ucode
>
> Link to v1: https://lore.kernel.org/linux-firmware/CACvgo50RhmRKnUoos2wGWOP-Pb8oGo1BMPunfTtBQO0eX1D2Dg@mail.gmail.com/T/
>
> Josh, others, kindly let me know if you'd prefer a PR or if there is
> anything I can do to move this forward. Be that as a whole, or chunks of
> it.
>
> Thanks
> Emil
>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
> Emil Velikov (16):
>       WHENCE: remove trailing white space
>       WHENCE: remove unnecessary filename quotation
>       check_whence, WHENCE, copy-firmware: escape filenames with spaces
>       WHENCE: remove duplicate File entries
>       WHENCE: comment out duplicate MediaTek firmware
>       check_whence: error on duplicate file entries
>       check_whence: error on directory listed as File
>       copy-firmware: remove non-applicable file presence test
>       check_whence: error if File: is actually a link
>       check_whence: error if symlinks are in-tree
>       copy-firmware: remove unreachable symlink workarounds
>       copy-firmware: quote deskdir to prevent word splitting
>       copy-firmware: tweak sed invocation
>       copy-firmware: quote the output of dirname
>       copy-firmware: silence the last shellcheck warnings
>       Makefile, copy-firmware: support xz/zstd compressed firmware
>
>  Makefile         |   8 +++++
>  WHENCE           | 107 ++++++++++++++++++++++++-------------------------------
>  check_whence.py  |  53 +++++++++++++++++++++++----
>  copy-firmware.sh |  76 ++++++++++++++++++++-------------------
>  4 files changed, 141 insertions(+), 103 deletions(-)
> ---
> base-commit: 60971a6485ee604c47cc905ff77c583ae001f9df
> change-id: 20230301-fixes-and-compression-7247407cab21
>
> Best regards,
> --
> Emil Velikov <emil.l.velikov@gmail.com>
>

Re: [PATCH RESEND v2 00/16] Misc fixes, sanity checks and xz/zstd compression

[Emil Velikov]

Welcome back Josh,

On Mon, 8 May 2023 at 13:51, Josh Boyer <jwboyer@kernel.org> wrote:
>
> I applied a few more of the patches, but generally speaking some of
> the changes conflate check_whence.py and copy-firmware.sh.  I think
> they should remain independent scripts and not make assumptions that
> one is doing something on behalf of the other.
>

Thank you. Must admit that I got worried there for a bit, seeing you
drop off for a few months.

Indeed It's quite likely that I'm conflating the two scripts.

While working on the series, I had a long hard look at the in-tree
README and ML to figure out the line there ... Although the docs are
quite barebone. Do you mind adding enhancing the README to cover this?

Alternatively I can come up with something, time permitting...
Speaking of which, next week I'll try to scrape some time to
split/respin the remaining patches.

Thanks again,
Emil