Re: [PATCH v2 10/15] PCI: rcar-gen4: Take over the iMSI-RX interrupt

[Marek Vasut <marek.vasut@mailbox.org>]

On 9/28/26 6:52 PM, Koichiro Den wrote:

[...]

> @@ -724,6 +773,9 @@ static void rcar_gen4_pcie_quiesce_irqs(struct rcar_gen4_pcie *rcar)
>   		rcar->reinit_pending = true;
>   		rcar_gen4_pcie_app_irq_sync_locked(rcar);
>   	}
> +
> +	/* The MSI status lives in DBI; keep the handler away during the reset. */

Would it make sense to add lockdep_assert_held(&rcar->reset_lock) here, 
to make it clear that this IRQ disable is protected by the reset lock ?

> +	disable_irq(rcar->msi_irq);
>   }
>   
>   static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar,
> @@ -733,6 +785,8 @@ static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar,
>   		rcar->reinit_pending = !recovered;
>   		rcar_gen4_pcie_app_irq_sync_locked(rcar);
>   	}
> +

Would it make sense to add lockdep_assert_held(&rcar->reset_lock) here, 
to make it clear that this IRQ enable is protected by the reset lock ?

> +	enable_irq(rcar->msi_irq);
>   }
>   
>   /*
> @@ -804,11 +858,17 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
>   
>   	ret = rcar_gen4_pcie_host_setup(pp);
>   	if (ret)
> -		goto err;
> +		goto err_deinit;
> +
> +	ret = rcar_gen4_pcie_msi_irq_init(rcar);

Would it make sense to acquire the IRQ a bit earlier, so you could avoid 
rcar_gen4_pcie_host_perst_assert(pp, true); in the fail path ?

I think if the MSI acquisition fails, perst signal would pulse (rapid 
sequence of deassert and assert), and that could be avoided.

> +	if (ret)
> +		goto err_assert_perst;
>   
>   	return 0;
>   
> -err:
> +err_assert_perst:
> +	rcar_gen4_pcie_host_perst_assert(pp, true);
> +err_deinit:
>   	rcar->drvdata->deinit(rcar);
>   	return ret;
>   }
> @@ -818,6 +878,9 @@ static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp)
>   	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
>   	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
>   
> +	/* Stop the handler before asserting reset and disabling the clocks. */
> +	free_irq(rcar->msi_irq, rcar);
> +
>   	rcar_gen4_pcie_host_perst_assert(pp, true);
>   	rcar->drvdata->deinit(rcar);
Shouldn't the IRQ be released only after reset is asserted and clock are 
stopped , otherwise it might accidentally fire and cause unhandled IRQ 
event ?