Re: [v8,4/7] PCI: mediatek-gen3: Add INTx support

[Marc Zyngier <maz@kernel.org>]

On Wed, 10 Mar 2021 03:05:15 +0000,
Jianjun Wang <jianjun.wang@mediatek.com> wrote:
> 
> On Tue, 2021-03-09 at 11:10 +0000, Marc Zyngier wrote:
> > On Wed, 24 Feb 2021 06:11:29 +0000,
> > Jianjun Wang <jianjun.wang@mediatek.com> wrote:
> > > 
> > > +static struct irq_chip mtk_intx_irq_chip = {
> > > +	.irq_enable		= mtk_intx_unmask,
> > > +	.irq_disable		= mtk_intx_mask,
> > 
> > Please get rid of enable/disable. Given that you already have
> > mask/unmask with the *same* implementation, this offers zero benefit.
> 
> Hi Marc,
> 
> Thanks for your review.
> 
> We need to support suspend/resume feature, the HW will be powered off
> when the system is suspended, and its register value will be cleared. If
> the enable/disable callback is not implemented, the unmask function will
> not be called when the system resume, so INTx will remain disabled.
> 
> Can I keep the enable/disable callback? Or do we have any solutions to
> restore the register value when the system resume?

Isn't that the job of the suspend/resume support to save and restore
the full state of the device? You shouldn't rely on the implicit
behaviours of the irq subsystem across suspend/resume.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek