Re: [RFC PATCH v0.2] PCI: Add support for tango PCIe host bridge

From: Marc Zyngier <marc.zyngier@arm.com>
To: Mason <slash.tmp@free.fr>, Thomas Gleixner <tglx@linutronix.de>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Liviu Dudau <liviu.dudau@arm.com>,
	David Laight <david.laight@aculab.com>,
	linux-pci <linux-pci@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Thibaud Cornic <thibaud_cornic@sigmadesigns.com>,
	Phuong Nguyen <phuong_nguyen@sigmadesigns.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH v0.2] PCI: Add support for tango PCIe host bridge
Date: Tue, 11 Apr 2017 16:49:58 +0100	[thread overview]
Message-ID: <5f81730d-fbe3-1f4c-de34-09bbfb893ee1@arm.com> (raw)
In-Reply-To: <30f662a6-5dab-515b-e35a-a312f3c7b509@free.fr>

On 11/04/17 16:13, Mason wrote:
> On 27/03/2017 19:09, Marc Zyngier wrote:
> 
>> Here's what your system looks like:
>>
>> PCI-EP -------> MSI Controller ------> INTC
>>          MSI                    IRQ
>>
>> A PCI MSI is always edge. No ifs, no buts. That's what it is, and nothing
>> else. Now, your MSI controller signals its output using a level interrupt,
>> since you need to whack it on the head so that it lowers its line.
>>
>> There is not a single trigger, because there is not a single interrupt.
> 
> Hello Marc,
> 
> I was hoping you or Thomas might help clear some confusion
> in my mind around IRQ domains (struct irq_domain).
> 
> I have read https://www.kernel.org/doc/Documentation/IRQ-domain.txt
> 
> IIUC, there should be one IRQ domain per IRQ controller.
> 
> I have this MSI controller handling 256 interrupts, so I should
> have *one* domain for all possible MSIs. Yet the Altera driver
> registers *two* domains (msi_domain and inner_domain).
> 
> Could I make everything work with a single IRQ domain?

No, because you have two irqchips. One that deals with the HW, and the
other that deals with the MSIs how they are presented to the kernel,
depending on the bus (PCI or something else). The fact that it doesn't
really drive any HW doesn't make it irrelevant.

> I'm confused by the sequence:
> 
>   irq_dom = irq_domain_create_linear(fwnode, MSI_COUNT, &msi_dom_ops, pcie);
>   msi_dom = pci_msi_create_irq_domain(fwnode, &msi_domain_info, irq_dom);
> 
> It seems I should be able to call only pci_msi_create_irq_domain...
> And the parent of the MSI controller is given in the DT.

No, see above.

> I'm not quite sure how I tell pci_msi_create_irq_domain how many
> MSIs it's supposed to manage. Nor how I pass my private struct.

You don't need to tell it anything about the number of interrupts you
manage. As for your private structure, you've already given it to your
low level domain, and there is no need to propagate it any further.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...