devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Cc: marc.zyngier@arm.com, Mark Rutland <mark.rutland@arm.com>,
	jason@lakedaemon.net, pawel.moll@arm.com,
	Catalin.Marinas@arm.com, Will.Deacon@arm.com,
	liviu.dudau@arm.com, Harish.Kasiviswanathan@amd.com,
	linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [V8 1/2] irqchip: gic: Add support for multiple MSI for ARM64
Date: Tue, 23 Sep 2014 23:33:44 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.10.1409232036200.4604@nanos> (raw)
In-Reply-To: <5421A825.70201@amd.com>

On Tue, 23 Sep 2014, Suravee Suthikulpanit wrote:
> > > This patch implelments the ARM64 version of arch_setup_msi_irqs(),
> > > which does not return 1 for when PCI_CAP_ID_MSI and nvec > 1.
> > 
> > I can see that myself. What your changelog is missing is the reason
> > WHY you think that copying that code from drivers/pci/msi.c and
> > removing the "PCI_CAP_ID_MSI and nvec > 1" has any value.
> 
> [Suravee] This is mainly be cause the weak version of arch_setup_msi_irqs() in
> the drivers/pci/msi.c doesn't support multi-MSI. Sorry for not being clear in
> the commit message.

Groan. I asked you: 

> > WHY you think that copying that code from drivers/pci/msi.c and
> > removing the "PCI_CAP_ID_MSI and nvec > 1" has any value.

And your answer is that the function in drivers/pci/msi.c does not
support Multi-MSI. Hell I know that myself. And there is a fricking
good reason why allocating multi-MSI via

    for_each_msi()
	alloc_msi_irq();

is wrong. And while it might work by chance, there is no guarantee
that it will work. It works for Multi-MSIX, but that has an additional
X at the end and is a different beast when it comes to interrupts.

I have no idea how crooked you are trying to work around that on the
GIC side, but its going to be wrong and convoluted.

Read and understand the MSI and MSI-X spec and the subtle differences
of interrupt delivery. And if you groked that come back with a proper
explanation why that patch makes sense or just go back to the drawing
board and do it proper.

Thanks,

	tglx

  reply	other threads:[~2014-09-23 21:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-20 16:31 [V8 0/2] irqchip: gic: Introduce ARM GICv2m MSI(-X) support suravee.suthikulpanit
2014-09-20 16:31 ` [V8 1/2] irqchip: gic: Add support for multiple MSI for ARM64 suravee.suthikulpanit
2014-09-22  9:15   ` Will Deacon
2014-09-22 23:08   ` Thomas Gleixner
2014-09-23 17:04     ` Suravee Suthikulpanit
2014-09-23 21:33       ` Thomas Gleixner [this message]
2014-09-20 16:31 ` [V8 2/2] irqchip: gicv2m: Add supports for ARM GICv2m MSI(-X) suravee.suthikulpanit
2014-09-22 17:37   ` Mark Rutland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.10.1409232036200.4604@nanos \
    --to=tglx@linutronix.de \
    --cc=Catalin.Marinas@arm.com \
    --cc=Harish.Kasiviswanathan@amd.com \
    --cc=Suravee.Suthikulpanit@amd.com \
    --cc=Will.Deacon@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jason@lakedaemon.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=liviu.dudau@arm.com \
    --cc=marc.zyngier@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).