| Message ID | 20220509163851.1513880-1-dimitri.ledkov@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [kernel-snaps-uc2*/main|master] trim-firmware: correctly support firmware wildcard patterns | expand |
Acked-by: Tim Gardner <tim.gardner@canonical.com> On 5/9/22 10:38, Dimitri John Ledkov wrote: > firmware stanzas in kernel modules can contain wildcard > expansions. Thus explicitely do not quote fw_file such that wildcard > expansion finds all of: > > brcm/brcmfmac*-sdio.*.bin > brcm/brcmfmac*-pcie.*.txt > brcm/brcmfmac*-sdio.*.txt > > Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com> > --- > trim-firmware | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/trim-firmware b/trim-firmware > index cc7c9fff6c..cc8b61c42d 100755 > --- a/trim-firmware > +++ b/trim-firmware > @@ -23,7 +23,8 @@ DESTDIR=${1} > > # Copy required firmware files to a new directory > while IFS= read -r fw_file ; do > - for src_file in "${DESTDIR}"/firmware/${fw_file} ; do > + # Note path expansion required, as fw_file can be a wildcard > + for src_file in $DESTDIR/firmware/$fw_file ; do > if ! [ -e "${src_file}" ] ; then > continue # Skip non-existing source files > fi > @@ -38,12 +39,6 @@ while IFS= read -r fw_file ; do > done > done < <(list_firmware "${DESTDIR}"/modules | sort -u) > > -# Copy all brcm files, since there might be config files that the kernel > -# doesn't expose via modinfo > -if [ -d "${DESTDIR}"/firmware.new/brcm ] ; then > - cp "${DESTDIR}"/firmware/brcm/* "${DESTDIR}"/firmware.new/brcm > -fi > - > # Copy the wifi regulatory database > if [ -e "${DESTDIR}"/firmware/regulatory.db ] ; then > cp "${DESTDIR}"/firmware/regulatory.* "${DESTDIR}"/firmware.new
Applied to uc22 only for now, once pi & pc kernels ship with that in place correctly, will apply to uc20 too.
On Mon, 9 May 2022 17:38:51 +0100 Dimitri John Ledkov <dimitri.ledkov@canonical.com> wrote: > firmware stanzas in kernel modules can contain wildcard > expansions. Thus explicitely do not quote fw_file such that wildcard > expansion finds all of: > > brcm/brcmfmac*-sdio.*.bin > brcm/brcmfmac*-pcie.*.txt > brcm/brcmfmac*-sdio.*.txt > > Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com> > --- > trim-firmware | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/trim-firmware b/trim-firmware > index cc7c9fff6c..cc8b61c42d 100755 > --- a/trim-firmware > +++ b/trim-firmware > @@ -23,7 +23,8 @@ DESTDIR=${1} > > # Copy required firmware files to a new directory > while IFS= read -r fw_file ; do > - for src_file in "${DESTDIR}"/firmware/${fw_file} ; do > + # Note path expansion required, as fw_file can be a wildcard > + for src_file in $DESTDIR/firmware/$fw_file ; do What does that do? fw_file wasn't quoted before, for that very reason. ...Juerg > if ! [ -e "${src_file}" ] ; then > continue # Skip non-existing source files > fi > @@ -38,12 +39,6 @@ while IFS= read -r fw_file ; do > done > done < <(list_firmware "${DESTDIR}"/modules | sort -u) > > -# Copy all brcm files, since there might be config files that the kernel > -# doesn't expose via modinfo > -if [ -d "${DESTDIR}"/firmware.new/brcm ] ; then > - cp "${DESTDIR}"/firmware/brcm/* "${DESTDIR}"/firmware.new/brcm > -fi > - > # Copy the wifi regulatory database > if [ -e "${DESTDIR}"/firmware/regulatory.db ] ; then > cp "${DESTDIR}"/firmware/regulatory.* "${DESTDIR}"/firmware.new
On Thu, 12 May 2022 at 12:17, Juerg Haefliger <juerg.haefliger@canonical.com> wrote: > > On Mon, 9 May 2022 17:38:51 +0100 > Dimitri John Ledkov <dimitri.ledkov@canonical.com> wrote: > > > firmware stanzas in kernel modules can contain wildcard > > expansions. Thus explicitely do not quote fw_file such that wildcard > > expansion finds all of: > > > > brcm/brcmfmac*-sdio.*.bin > > brcm/brcmfmac*-pcie.*.txt > > brcm/brcmfmac*-sdio.*.txt > > > > Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com> > > --- > > trim-firmware | 9 ++------- > > 1 file changed, 2 insertions(+), 7 deletions(-) > > > > diff --git a/trim-firmware b/trim-firmware > > index cc7c9fff6c..cc8b61c42d 100755 > > --- a/trim-firmware > > +++ b/trim-firmware > > @@ -23,7 +23,8 @@ DESTDIR=${1} > > > > # Copy required firmware files to a new directory > > while IFS= read -r fw_file ; do > > - for src_file in "${DESTDIR}"/firmware/${fw_file} ; do > > + # Note path expansion required, as fw_file can be a wildcard > > + for src_file in $DESTDIR/firmware/$fw_file ; do > > What does that do? fw_file wasn't quoted before, for that very reason. > I want to say that path expansion happened before variable expansion when quoted.... but I am now failing to reproduce this. And I am now questioning my sanity and maybe the quoting change is redundant / semi harmful (as it now prevents DESTDIR from having spaces". I'll go and do clean experiments, maybe my local shell is setup weirdly. > ...Juerg > > > if ! [ -e "${src_file}" ] ; then > > continue # Skip non-existing source files > > fi > > @@ -38,12 +39,6 @@ while IFS= read -r fw_file ; do > > done > > done < <(list_firmware "${DESTDIR}"/modules | sort -u) > > > > -# Copy all brcm files, since there might be config files that the kernel > > -# doesn't expose via modinfo > > -if [ -d "${DESTDIR}"/firmware.new/brcm ] ; then > > - cp "${DESTDIR}"/firmware/brcm/* "${DESTDIR}"/firmware.new/brcm > > -fi > > - > > # Copy the wifi regulatory database > > if [ -e "${DESTDIR}"/firmware/regulatory.db ] ; then > > cp "${DESTDIR}"/firmware/regulatory.* "${DESTDIR}"/firmware.new >
and further investigation resulted in me reverting this patch.... because some brcm firmware files are still underdeclared by some kernels, thus i don't want to risk regressing their support. Instead i will pursue fixing firmware declarations for those modules first.
diff --git a/trim-firmware b/trim-firmware index cc7c9fff6c..cc8b61c42d 100755 --- a/trim-firmware +++ b/trim-firmware @@ -23,7 +23,8 @@ DESTDIR=${1} # Copy required firmware files to a new directory while IFS= read -r fw_file ; do - for src_file in "${DESTDIR}"/firmware/${fw_file} ; do + # Note path expansion required, as fw_file can be a wildcard + for src_file in $DESTDIR/firmware/$fw_file ; do if ! [ -e "${src_file}" ] ; then continue # Skip non-existing source files fi @@ -38,12 +39,6 @@ while IFS= read -r fw_file ; do done done < <(list_firmware "${DESTDIR}"/modules | sort -u) -# Copy all brcm files, since there might be config files that the kernel -# doesn't expose via modinfo -if [ -d "${DESTDIR}"/firmware.new/brcm ] ; then - cp "${DESTDIR}"/firmware/brcm/* "${DESTDIR}"/firmware.new/brcm -fi - # Copy the wifi regulatory database if [ -e "${DESTDIR}"/firmware/regulatory.db ] ; then cp "${DESTDIR}"/firmware/regulatory.* "${DESTDIR}"/firmware.new
firmware stanzas in kernel modules can contain wildcard expansions. Thus explicitely do not quote fw_file such that wildcard expansion finds all of: brcm/brcmfmac*-sdio.*.bin brcm/brcmfmac*-pcie.*.txt brcm/brcmfmac*-sdio.*.txt Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com> --- trim-firmware | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)