linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: thomas.petazzoni@free-electrons.com (Thomas Petazzoni)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv7 07/13] irqdomain: add function to find a MSI irq_domain
Date: Thu, 8 Aug 2013 10:16:27 +0200	[thread overview]
Message-ID: <20130808101627.065ec95a@skate> (raw)
In-Reply-To: <1375914665.12551.5.camel@pasglop>

Dear Benjamin Herrenschmidt,

On Thu, 08 Aug 2013 08:31:05 +1000, Benjamin Herrenschmidt wrote:

>  - I still don't like it. I find that it's looking more and more like
> over engineering. I don't like having any kind of infrastructure
> relationship between MSI stuff and irqdomain, ie, a PCI/PCIe specific
> construct and a generic interrupt remapper.

This is *exactly* the opposite of what Grant Likely said in:

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/187083.html
[PATCHv5 05/11] of: pci: add registry of MSI chips

Grant said:

"""
Actually, I'm going to disagree on this one and say NAK. I don't think
it is a good idea to create a completely separate registry of msi_chips
for binding to dt nodes. I think it would be better to include the
msi_chip pointer directly into the irq_domain which has to be there
anyway. It then becomes another feature for irq controllers if it can
support doing MSI.
"""

So Grant is completely in favor of a strong relationship between MSI
stuff and irqdomain.

> Trying to use irqdomain for HW number allocation seems to be pushing it
> where it wasn't designed to go. Are those interrupts really different
> domains ? Do they have separate number spaces, separate DT encodings and
> overall characteristics ?

This is also *exactly* the opoosite of what Grant Likely said in:

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/175430.html
[PATCH v2 4/8] irqchip: armada-370-xp: implement MSI support

He was replying to the patch that did a bitmap based allocation scheme,
within the IRQ controller driver, for MSI interrupts. And Grant said:

"""
This looks like something that the irq_domain should handle for you.
It would be good to have a variant of irq_create_mapping() that keeps
track of available hwirqs, allocates a free one, and maps it all with
one function call. I can see that being useful by other MSI interrupt
controllers and would eliminate needing to open-code it above.
"""

> What's wrong with the bitmap allocator in the PIC driver ? It's simple,
> and does the job just fine. If anything, take it from powerpc and sparc
> and move it to generic. It's already a "generic" (ie shared)
> infrastructure in powerpc.

See above. Grant said to *NOT* implement a bitmap allocator. He even
said it would be useful for other MSI interrupt controllers, and that
ultimately they should be migrated to not use the bitmap allocator that
you're talking about, but instead rely on irqdomain based allocations.

> That series makes me feel nervous, it feels like a hack. I really don't
> like creating that relationship between msi_chip and irqdomain. In fact,
> I think it makes it harder to understand what's happening in the code
> and following things.

Please see above, you're going completely backwards to what Grant
Likely was saying.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

  parent reply	other threads:[~2013-08-08  8:16 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-07  9:32 [PATCHv7 00/13] MSI support for Marvell EBU PCIe driver Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 01/13] PCI: use weak functions for MSI arch-specific functions Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 02/13] PCI: remove ARCH_SUPPORTS_MSI kconfig option Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 03/13] PCI: Introduce new MSI chip infrastructure Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 04/13] irqdomain: add irq_alloc_mapping() function Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 05/13] irqdomain: refactor __irq_domain_add() Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 06/13] irqdomain: add support to associate an irq_domain with a msi_chip Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 07/13] irqdomain: add function to find a MSI irq_domain Thomas Petazzoni
2013-08-07 20:50   ` Benjamin Herrenschmidt
2013-08-07 22:04     ` Thomas Petazzoni
2013-08-07 22:31       ` Benjamin Herrenschmidt
2013-08-07 22:42         ` Benjamin Herrenschmidt
2013-08-07 22:45           ` Benjamin Herrenschmidt
2013-08-08  8:22             ` Thomas Petazzoni
2013-08-08  8:41               ` Benjamin Herrenschmidt
2013-08-08  8:16         ` Thomas Petazzoni [this message]
2013-08-08  8:38           ` Benjamin Herrenschmidt
2013-08-08  8:54             ` Benjamin Herrenschmidt
2013-08-07  9:32 ` [PATCHv7 08/13] irqchip: armada-370-xp: properly request resources Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 09/13] irqchip: armada-370-xp: implement MSI support Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 10/13] ARM: pci: add ->add_bus() and ->remove_bus() hooks to hw_pci Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 11/13] ARM: mvebu: the MPIC now provides MSI controller features Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 12/13] PCI: mvebu: add support for MSI Thomas Petazzoni
2013-08-07  9:32 ` [PATCHv7 13/13] ARM: mvebu: link PCIe controllers to the MSI controller Thomas Petazzoni
2013-08-07 20:23 ` [PATCHv7 00/13] MSI support for Marvell EBU PCIe driver Jason Cooper

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=20130808101627.065ec95a@skate \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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).