Re: [PATCH net-next v6 2/2] net: wwan: qcom_bam_dmux: Assign restricted DMA pool to remote processor

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net: wwan: qcom_bam_dmux: lost PC edge in bam_dmux_probe()
  with IRQF_NO_AUTOEN
--

commit 4515119c8a0e425457d7366d017600d0d4e872df
Author: Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>

net: wwan: qcom_bam_dmux: Assign restricted DMA pool to remote processor

This commit assigns a restricted DMA pool to both the local and remote
execution environments via SCM when qcom,vmid is present, reclaims the
pool on teardown, and defers enabling pc_irq until the end of probe.

> diff --git a/drivers/net/wwan/qcom_bam_dmux.c b/drivers/net/wwan/qcom_bam_dmux.c
> index 1479f8362e04e..04f3d46d4c3f3 100644
> --- a/drivers/net/wwan/qcom_bam_dmux.c
> +++ b/drivers/net/wwan/qcom_bam_dmux.c
[ ... ]
> @@ -821,7 +913,7 @@ static int bam_dmux_probe(struct platform_device *pdev)
>  		goto err_disable_pm;
>  
>  	ret = devm_request_threaded_irq(dev, dmux->pc_irq, NULL, bam_dmux_pc_irq,
> -					IRQF_ONESHOT, NULL, dmux);
> +					IRQF_ONESHOT | IRQF_NO_AUTOEN, NULL, dmux);
>  	if (ret)
>  		goto err_disable_pm;
>  
> @@ -838,11 +930,14 @@ static int bam_dmux_probe(struct platform_device *pdev)
>  			bam_dmux_power_off(dmux);
>  	}
>  
> +	enable_irq(dmux->pc_irq);
> +
>  	return 0;

[Severity: Medium]
Can requesting dmux->pc_irq with IRQF_NO_AUTOEN drop a power-control edge
between irq_get_irqchip_state() and enable_irq(), permanently inverting
dmux->pc_state?

CPU0                              Remote modem
bam_dmux_probe()
  devm_request_threaded_irq(...,
    IRQF_NO_AUTOEN, ...)
  irq_get_irqchip_state(...,
    &dmux->pc_state) -> false
                                  toggles pc line (0 -> 1)
  enable_irq(dmux->pc_irq)
    smsm_unmask_irq() / smp2p
      caches remote_state = 1     <-- edge lost, pc_state stays false

With IRQF_NO_AUTOEN, dmux->pc_irq stays unstarted in the underlying
Qualcomm shared-memory irqchip (qcom,smsm or qcom,smp2p) while
bam_dmux_probe() samples the initial line level and handles initial power
on.  Neither irqchip latches transitions that happen before enable_irq():

drivers/soc/qcom/smsm.c:smsm_unmask_irq() {
	...
	/* Make sure our last cached state is up-to-date */
	assign_bit(irq, &entry->last_value,
		   readl(entry->remote_state) & BIT(irq));

	set_bit(irq, entry->irq_enabled);
	...
}

drivers/soc/qcom/smp2p.c:qcom_smp2p_notify_in() similarly updates
entry->last_value for all bits while only delivering interrupts for bits
already set in entry->irq_enabled.

Because bam_dmux_pc_irq() tracks state by toggling
new_state = !dmux->pc_state on each edge rather than reading the line
level, missing an edge before enable_irq() leaves dmux->pc_state inverted
for the lifetime of the driver, breaking power control and DMA pool
reclaim on remove.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-qcom-bam-dmux-vmid-ext-v6-0-7a16bd3dff18@oss.qualcomm.com?part=2