REVERTED: [kernel-snaps-uc2*/main|master][PATCH] trim-firmware: correctly support firmware wildcard patterns

Dimitri John Ledkov dimitri.ledkov at canonical.com
Fri May 13 12:02:21 UTC 2022


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.
-- 
okurrr,

Dimitri

On Thu, 12 May 2022 at 14:44, Dimitri John Ledkov
<dimitri.ledkov at canonical.com> wrote:
>
> On Thu, 12 May 2022 at 12:17, Juerg Haefliger
> <juerg.haefliger at canonical.com> wrote:
> >
> > On Mon,  9 May 2022 17:38:51 +0100
> > Dimitri John Ledkov <dimitri.ledkov at 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 at 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
> >



More information about the kernel-team mailing list