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: [PATCHv5 05/11] of: pci: add registry of MSI chips
Date: Sun, 28 Jul 2013 16:27:11 +0200	[thread overview]
Message-ID: <20130728162711.32a9a21e@skate> (raw)
In-Reply-To: <20130728043310.6CF1D3E08FE@localhost>

Dear Grant Likely,

Thanks for your feedback! Some comments below.

On Sat, 27 Jul 2013 22:33:10 -0600, Grant Likely wrote:

> > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > 
> > Acked-by: Rob Herring <rob.herring@calxeda.com>
> 
> 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.

The problem that this patch tries to solve is how can the PCIe driver
work get a pointer to the msi_chip structure from the DT device node
pointed to by the 'msi-parent' property. I.e, we have:

	interrupt-parent = <&mpic>;

	mpic {
		reg = <...>;
		compatible = "...";
		interrupt-controller;
		msi-controller;
	};

	pcie-controller {
		msi-parent = <&mpic>;
	};

The 'mpic' driver registers two irq_domains, one for the "normal"
interrupts, and one for the MSI interrupts. Both irq_domain cannot be
associated to the same &mpic node, or the irq_domain lookup for
interrupt-parent and msi-parent is going to be confused.

In the very first version of this patch set, I was using two separate
DT device nodes to avoid this problem:

	interrupt-parent = <&mpic>;

	interrupt-controller {
		reg = <...>;
		compatible = "...";

		mpic {
			interrupt-controller;
		};

		msi {
			msi-controller;
		};
	};

	pcie-controller {
		msi-parent = <&msi>;
	};

This way, each of the two irq_domain was associated to a distinct DT
device node, and everything was working fine. But during the review, I
was pointed by Arnd that it wasn't the proper way of describing the
interrupt controller, and that there should be only one DT device node
having both the interrupt-controller and msi-controller roles.

So what is your suggestion to allow the PCIe controller to retrieve the
correct irq_domain if we have only one DT node for the IRQ controller
that registers two irq_domains ?

See:

   [RFCv1 00/11] MSI support for Marvell EBU PCIe driver
   https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-March/030578.html

and particularly:

   [RFCv1 07/11] irqchip: armada-370-xp: add MSI support to interrupt
   controller driver
   https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-March/030584.html

Thanks,

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

  reply	other threads:[~2013-07-28 14:27 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-15 11:52 [PATCHv5 00/11] MSI support for Marvell EBU PCIe driver Thomas Petazzoni
2013-07-15 11:52 ` [PATCHv5 01/11] irqdomain: add irq_alloc_mapping() function Thomas Petazzoni
2013-07-16  8:31   ` Thomas Petazzoni
2013-07-28  4:11   ` Grant Likely
2013-07-15 11:52 ` [PATCHv5 02/11] PCI: use weak functions for MSI arch-specific functions Thomas Petazzoni
2013-07-25 18:37   ` Thierry Reding
2013-07-25 20:27   ` Bjorn Helgaas
2013-07-15 11:52 ` [PATCHv5 03/11] PCI: remove ARCH_SUPPORTS_MSI kconfig option Thomas Petazzoni
2013-07-15 11:52 ` [PATCHv5 04/11] PCI: Introduce new MSI chip infrastructure Thomas Petazzoni
2013-07-25 18:40   ` Thierry Reding
2013-07-25 20:26   ` Bjorn Helgaas
2013-07-15 11:52 ` [PATCHv5 05/11] of: pci: add registry of MSI chips Thomas Petazzoni
2013-07-15 16:12   ` Rob Herring
2013-07-28  4:33     ` Grant Likely
2013-07-28 14:27       ` Thomas Petazzoni [this message]
2013-07-29  6:54         ` Thierry Reding
2013-07-29 12:26           ` Thomas Petazzoni
2013-07-29 12:58             ` Thierry Reding
2013-07-29 13:04               ` Thomas Petazzoni
2013-08-01  9:17               ` Thomas Petazzoni
2013-07-31 15:14             ` Thomas Petazzoni
2013-07-15 11:52 ` [PATCHv5 06/11] irqchip: armada-370-xp: properly request resources Thomas Petazzoni
2013-07-15 11:52 ` [PATCHv5 07/11] irqchip: armada-370-xp: implement MSI support Thomas Petazzoni
2013-07-15 11:52 ` [PATCHv5 08/11] ARM: pci: add ->add_bus() and ->remove_bus() hooks to hw_pci Thomas Petazzoni
2013-07-16  8:29   ` Thomas Petazzoni
2013-07-25 16:53   ` Thierry Reding
2013-08-07  9:13     ` Thomas Petazzoni
2013-08-07 13:37       ` Thierry Reding
2013-08-07 15:06         ` Thomas Petazzoni
2013-07-25 16:57   ` Thierry Reding
2013-07-26  8:13     ` Thomas Petazzoni
2013-07-26 11:49       ` Jason Cooper
2013-07-26 22:46     ` Russell King - ARM Linux
2013-07-29 12:31       ` Thierry Reding
2013-07-15 11:52 ` [PATCHv5 09/11] ARM: mvebu: the MPIC now provides MSI controller features Thomas Petazzoni
2013-07-15 11:52 ` [PATCHv5 10/11] PCI: mvebu: add support for MSI Thomas Petazzoni
2013-07-15 11:52 ` [PATCHv5 11/11] ARM: mvebu: link PCIe controllers to the MSI controller Thomas Petazzoni
2013-07-15 15:34 ` [PATCHv5 00/11] MSI support for Marvell EBU PCIe driver Stephen Warren
2013-07-15 16:27   ` Thierry Reding
2013-07-16  8:33     ` Thomas Petazzoni
2013-07-16 21:05       ` Thierry Reding
2013-07-16 21:43         ` Thomas Petazzoni
2013-07-23  0:30           ` Bjorn Helgaas
2013-07-15 17:33   ` Jason Cooper
2013-07-16  8:43   ` Thomas Petazzoni
2013-07-16 13:15     ` Jason Cooper
2013-07-16 13:39       ` Thomas Petazzoni
2013-07-25 18:49         ` Thierry Reding
2013-07-25 19:02           ` 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=20130728162711.32a9a21e@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).