Re: [PATCH ath-next] wifi: ath12k: add support to load shared firmware on multiPD

[Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>]

On 8/14/2026 11:54 PM, Jeff Johnson wrote:
> On 8/10/2026 10:43 PM, Aaradhana Sahu wrote:
>> +static int ath12k_ahb_load_auth_shared_fw(struct ath12k_base *ab,
>> +					  struct ath12k_ahb_rproc_info *rproc_info,
>> +					  const char *fw_name, u32 pasid)
>> +{
>> +	int ret;
>> +
>> +	const struct firmware *fw __free(firmware) = NULL;
>> +	ret = request_firmware(&fw, fw_name, ab->dev);
>> +	if (ret) {
>> +		ath12k_err(ab, "failed to request shared firmware %s: %d\n",
>> +			   fw_name, ret);
>> +		return ret;
>> +	}
>> +
>> +	if (!fw->size) {
>> +		ath12k_err(ab, "Invalid firmware size\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ath12k_dbg(ab, ATH12K_DBG_AHB, "loading firmware %s, size %zd\n", fw_name,
>> +		   fw->size);
>> +
>> +	ret = qcom_mdt_load(ab->dev, fw, fw_name, pasid, rproc_info->mem_region,
>> +			    rproc_info->mem_phys, rproc_info->mem_size, NULL);
>> +	if (ret) {
>> +		ath12k_err(ab, "failed to load RO firmware %s: %d\n", fw_name, ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = qcom_pas_auth_and_reset(pasid);
>> +	if (ret)
>> +		ath12k_err(ab, "failed to authenticate and boot shared firmware: %d\n",
>> +			   ret);
> 
> my review agent says:
> qcom_mdt_load() calls qcom_scm_pas_init_image() and qcom_scm_pas_mem_setup()
> internally, which register the PASID with TrustZone. If that succeeds but the
> subsequent qcom_pas_auth_and_reset() fails, the PASID remains registered in
> SCM with no corresponding shutdown. The caller
> (ath12k_ahb_load_shared_firmware()) calls ath12k_ahb_put_fw_load_region() on
> this error path, but does not call qcom_pas_shutdown(). On the next probe or
> reset cycle, qcom_mdt_load() calls qcom_scm_pas_init_image() again on an
> already-registered PASID; SCM returns an error and firmware loading is
> permanently broken until a full platform reset.
> 
> and it suggest calling qcom_pas_shutdown(pasid)
> 
>         ret = qcom_pas_auth_and_reset(pasid);
>         if (ret) {
>                 ath12k_err(...);
>                 qcom_pas_shutdown(pasid);
>         }
>         return ret;
> 
> 

qcom_pas_mem_setup() is called by qcom_mdt_load() only when the firmware contains a QCOM_MDT_RELOCATABLE segment.
The firmware loaded through this path is non-relocatable, and reloc_base is passed as NULL, so qcom_pas_mem_setup()
is not invoked.

I have also verified the SCM behavior for our AHB firmware and confirmed that qcom_pas_shutdown() is not required
when qcom_pas_auth_and_reset() fails.

>> +	return ret;
>> +}
>> +