Re: [PATCH ath-next 03/16] wifi: ath11k: Register root PD rproc notifier

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ath11k: firmware reset and recovery broken for non-MPD targets
- [High] ath11k: NULL pointer dereference in root PD notifier debug log
--

commit cebde61194c39a9cdbfb9178d0b280c99923a9c0
Author: George Moussalem <george.moussalem@outlook.com>

wifi: ath11k: Register root PD rproc notifier

This commit prepares for multi-PD support by registering the rproc notifier
for the Q6 rootPD. It handles booting the root PD and waiting for power-up
notification before proceeding, adding driver-level locking to avoid races.

> diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/wireless/ath/ath11k/ahb.c
> index 27d01411b6e9c..86723a2eb3d26 100644
> --- a/drivers/net/wireless/ath/ath11k/ahb.c
> +++ b/drivers/net/wireless/ath/ath11k/ahb.c

[ ... ]

> @@ -408,21 +415,12 @@ static void ath11k_ahb_stop(struct ath11k_base *ab)
>  
>  static int ath11k_ahb_power_up(struct ath11k_base *ab)
>  {
> -	struct ath11k_ahb *ab_ahb = ath11k_ahb_priv(ab);
> -	int ret;
> -
> -	ret = rproc_boot(ab_ahb->tgt_rproc);
> -	if (ret)
> -		ath11k_err(ab, "failed to boot the remote processor Q6\n");
> -
> -	return ret;
> +	return 0;
>  }
>  
>  static void ath11k_ahb_power_down(struct ath11k_base *ab, bool is_suspend)
>  {
> -	struct ath11k_ahb *ab_ahb = ath11k_ahb_priv(ab);
> -
> -	rproc_shutdown(ab_ahb->tgt_rproc);
> +	return;
>  }

[Severity: High]
Does making ath11k_ahb_power_up() and ath11k_ahb_power_down() empty stubs
break the firmware recovery mechanisms for non-MPD targets like IPQ8074?

When ath11k_qmi_fwreset_from_cold_boot() or ath11k_core_reset() are called,
they invoke ath11k_hif_power_down() and ath11k_hif_power_up(). If these
hardware operations do nothing, wouldn't the hardware fail to restart
during firmware crash recovery or cold boot calibration, leaving the WiFi
subsystem broken?

[ ... ]

> @@ -832,24 +830,200 @@ static const struct ath11k_hif_ops ath11k_ahb_hif_ops_wcn6750 = {
>  	.ce_irq_disable = ath11k_pci_disable_ce_irqs_except_wake_irq,
>  };
>  
> +static int ath11k_ahb_root_pd_state_notifier(struct notifier_block *nb,
> +					     const unsigned long event, void *data)
> +{
> +	struct ath11k_ahb_rproc_info *rproc_info =
> +		container_of(nb, struct ath11k_ahb_rproc_info, root_pd_nb);
> +
> +	if (event == ATH11K_RPROC_AFTER_POWERUP) {
> +		ath11k_dbg(NULL, ATH11K_DBG_AHB, "Root PD is UP\n");

[Severity: High]
Will passing a NULL pointer as the struct ath11k_base *ab argument to
ath11k_dbg() cause a kernel panic?

If debug logging or tracepoints are enabled, it looks like __ath11k_dbg()
will unconditionally dereference ab->dev, which would be triggered
automatically when the root PD powers up.

> +		complete(&rproc_info->rootpd_ready);
> +	}
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-ipq5018-qcn6122-mpd-v1-0-86729521a75c@outlook.com?part=3