Re: Regression: arm64: dts: sdm845-db845c: make firmware filenames follow linux-firmware

From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: John Stultz <john.stultz@linaro.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Vinod Koul <vinod.koul@linaro.org>,
	Srini Kandagatla <srinivas.kandagatla@linaro.org>,
	Amit Pundir <amit.pundir@linaro.org>,
	YongQin Liu <yongqin.liu@linaro.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>
Subject: Re: Regression: arm64: dts: sdm845-db845c: make firmware filenames follow linux-firmware
Date: Wed, 5 May 2021 14:06:13 -0500	[thread overview]
Message-ID: <20210505190613.GG2484@yoga> (raw)
In-Reply-To: <CALAqxLXsLcA=r1x6hXxLx3aVA-8=RG5qd+yj+sKEFiaWPUzkSA@mail.gmail.com>

On Wed 05 May 01:20 CDT 2021, John Stultz wrote:

> Hey Dmitry, Bjorn,
>   I wanted to raise a regression I caught in the merge window on db845c.
> 
> I was seeing troubles with audio and while there are a few other
> pending fixes needed, they did not seem to work for me. So I spent
> some time bisecting things down and found the problematic commit was
> 7443ff06da45 ("arm64: dts: sdm845-db845c: make firmware filenames
> follow linux-firmware").
> 
> It seems for systems using the old firmware filenames, this will break
> dependent devices on adsp_pas and cdsp_pas nodes.
> 
> Now, obviously updating the firmware files in userland should resolve
> this, but it adds the complexity that we can't just replace the
> firmware files because older LTS kernels will look for the old names,
> while newer kernels will look for the new names. We can add both files
> to the system images, but then there is some confusion on which
> version of the firmware files are being used where.
> 
> So yes, we should align with linux-firmware file names, but I think
> more care is needed for this sort of thing as it has the potential to
> break folks, and this isn't the first time around we've had similar
> firmware name changes break us.
> 

Due to the workings of the userspace firmware loader fallback I
unfortunately don't see any reasonable way to deal with this.
Introducing a fallback mechanism would suffer from an unavoidable 60
second delay waiting for the first choice to timeout, or if we used the
non-userspace-assisted method we'd probably give up too early.

> So I'm working on fixing this by including both filenames in userland,

The kernel will detect in runtime if you pass it a squashed or split
firmware package, the suffix has no significance. So if you have the
need to go back and forth just make sure you have a symlink that points
the .mbn to the .mdt (or vice versa).

> so we probably don't need a revert here, but *please* maybe take more
> care on this sort of change.
> 

Yes, I should have paid more attention when we merged the original
firmware-name to avoid this issue. Sorry for not getting this right from
the beginning.

Regards,
Bjorn