linux-kernel - Re: [PATCH 7/9] remoteproc: qcom_q6v5

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5116928.iIbC2pHGDl@nukework.gtech>
Date: Tue, 23 Dec 2025 14:35:14 -0600
From: "Alex G." <mr.nuke.me@...il.com>
To: andersson@...nel.org, Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: mathieu.poirier@...aro.org, krzk+dt@...nel.org,
 linux-arm-msm@...r.kernel.org, linux-remoteproc@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 7/9] remoteproc: qcom_q6v5_wcss: support m3 firmware

On Friday, December 19, 2025 7:29:07 AM CST Konrad Dybcio wrote:
> On 12/19/25 5:34 AM, Alexandru Gagniuc wrote:
> > IPQ8074, IPQ6018, and IPQ9574 support an m3 firmware image in addtion
> > to the q6 firmware. The firmware releases from qcom provide both q6
> > and m3 firmware for these SoCs. Support loading the m3 firmware image.
> > 
> > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@...il.com>
> > ---
> > 
> >  drivers/remoteproc/qcom_q6v5_wcss.c | 44 +++++++++++++++++++++++++----
> >  1 file changed, 39 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c
> > b/drivers/remoteproc/qcom_q6v5_wcss.c index b62e97c92d058..265010c5c82cb
> > 100644
> > --- a/drivers/remoteproc/qcom_q6v5_wcss.c
> > +++ b/drivers/remoteproc/qcom_q6v5_wcss.c
> > @@ -101,7 +101,8 @@ enum {
> > 
> >  };
> >  
> >  struct wcss_data {
> > 
> > -	const char *firmware_name;
> > +	const char *q6_firmware_name;
> > +	const char *m3_firmware_name;
> > 
> >  	unsigned int crash_reason_smem;
> >  	u32 version;
> >  	bool aon_reset_required;
> > 
> > @@ -161,6 +162,7 @@ struct q6v5_wcss {
> > 
> >  	unsigned int crash_reason_smem;
> >  	u32 version;
> >  	bool requires_force_stop;
> > 
> > +	const char *m3_firmware_name;
> > 
> >  	struct qcom_rproc_glink glink_subdev;
> >  	struct qcom_rproc_pdm pdm_subdev;
> > 
> > @@ -922,11 +924,40 @@ static void *q6v5_wcss_da_to_va(struct rproc *rproc,
> > u64 da, size_t len, bool *i> 
> >  	return wcss->mem_region + offset;
> >  
> >  }
> > 
> > +static int q6v5_wcss_load_aux(struct q6v5_wcss *wcss, const char
> > *fw_name)
> > +{
> > +	const struct firmware *extra_fw;
> > +	int ret;
> > +
> > +	dev_info(wcss->dev, "loading additional firmware image %s\n", 
fw_name);
> 
> I don't think this log line is useful beyond development

Remoteproc driver prints the main (q6) fimrware name, so I thought it would be 
prudent to print the names of any additional firmwares:

    remoteproc remoteproc0: Booting fw image IPQ9574/q6_fw.mdt, size 8140

> > +
> > +	ret = request_firmware(&extra_fw, fw_name, wcss->dev);
> > +	if (ret)
> > +		return 0;
> 
> return ret, perhaps? Unless you want to say that "it's fine if the M3 image
> is missing, particularly not to impose any new requirements on existing
> setups". But you haven't spelt that out explicitly.

I intended to not abort when aux firmware is missing. Maybe the better way to 
handle this is to check for "-ENOENT" in the caller instead of return 0 here.

> You also haven't provided an explanation as to why the firmware should be
> loaded. Is it necessary for some functionality? Is it that case on the
> newly-supported IPQ9574?

I don't have a good answer. I reasoned that since the qcom provides it [1], 
the M3 firmware would need to be loaded. I haven't done much testing without 
it.

Alex

[1] https://github.com/quic/upstream-wifi-fw.git


> Konrad