lkml.org 
[lkml]   [2022]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 10/11] PCI: mvebu: Implement support for legacy INTx interrupts
    On Fri, 07 Jan 2022 11:50:53 +0000,
    Pali Rohár <pali@kernel.org> wrote:
    >
    > On Thursday 06 January 2022 17:31:36 Marc Zyngier wrote:
    > > On Thu, 06 Jan 2022 17:20:44 +0000,
    > > Marek Behún <kabel@kernel.org> wrote:
    > > >
    > > > On Thu, 06 Jan 2022 16:27:44 +0000
    > > > Marc Zyngier <maz@kernel.org> wrote:
    > > > > You are completely missing my point. I'm talking about data
    > > > > structures, you're talking about interrupts. You have this:
    > > > >
    > > > > struct mvebu_pcie_port {
    > > > > // Tons of stuff
    > > > > struct irq_chip intx_chip;
    > > > > };
    > > > >
    > > > > What I want you to do is:
    > > > >
    > > > > struct mvebu_pcie_port {
    > > > > // Tons of stuff
    > > > > };
    > > > >
    > > > > static struct irq_chip intx_chip = {
    > > > > .name = "INTx",
    > > > > .irq_mask = mvebu_pcie_intx_irq_mask,
    > > > > .irq_unmask = mvebu_pcie_intx_irq_unmask;
    > > > > };
    > > > >
    > > > > That's it. No more, no less.
    > > > >
    > > > > M.
    > > > >
    > > >
    > > > Hmm, but struct irq_chip contains a dynamic member,
    > > > struct device *parent_device;
    > > > Isn't that used? Or are you planning to kill it?
    > >
    > > Indeed, and I am definitely planning to kill it. This is the wrong
    > > place for this stuff, and I want it gone. There are thankfully very
    > > few users of this misfeature.
    >
    > Ok, so what about this change?
    >
    > @@ -1458,7 +1617,17 @@ static int mvebu_pcie_remove(struct platform_device *pdev)
    > mvebu_writel(port, cmd, PCIE_CMD_OFF);
    >
    > /* Mask all interrupt sources. */
    > - mvebu_writel(port, 0, PCIE_MASK_OFF);
    > + mvebu_writel(port, ~PCIE_INT_ALL_MASK, PCIE_INT_UNMASK_OFF);
    > +
    > + /* Clear all interrupt causes. */
    > + mvebu_writel(port, ~PCIE_INT_ALL_MASK, PCIE_INT_CAUSE_OFF);
    > +
    > + /* Remove IRQ domains. */
    > + if (port->intx_irq_domain)
    > + irq_domain_remove(port->intx_irq_domain);
    > +
    > + if (irq > 0)
    > + irq_set_chained_handler_and_data(irq, NULL, NULL);

    You really want this to be done *before* you remove the domain, as
    there still could be interrupts in flight at this point.

    M.

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

    \
     
     \ /
      Last update: 2022-01-07 19:55    [W:3.892 / U:0.632 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site