Re: [PATCH v2 1/4] PCI: X-Gene: Add the APM X-Gene v1 PCIe MSI/MSIX termination driver

[Marc Zyngier <marc.zyngier@arm.com>]

On Tue, 7 Apr 2015 20:56:48 +0100
Duc Dang <dhdang@apm.com> wrote:

Hi Duc,

> On Wed, Mar 18, 2015 at 11:52 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > On 18/03/15 18:29, Duc Dang wrote:
> >> On Wed, Mar 18, 2015 at 11:05 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> >>> On 04/03/15 19:39, Duc Dang wrote:
> >>>> X-Gene v1 SOC supports total 2688 MSI/MSIX vectors coalesced into
> >>>> 16 HW IRQ lines.
> >>>>
> >>>> Signed-off-by: Duc Dang <dhdang@apm.com>
> >>>> Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
> >>>
> >>> I just had a quick look at this, and this seems to be going in the exact
> >>> opposite direction compared to what we now have on arm64, where we move
> >>> away from using struct msi_controller for most thing, and implement PCI
> >>> MSI/MSIX in a generic way, using MSI domains.
> >>>
> >>> I suggest you have a look at how GICv2m and GICv3 ITS implement the MSI
> >>> support. You can also have a look at what I did for the Tegra MSI
> >>> controller in this patch:
> >>>
> >>> https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/kill-msi-controller&id=83b3602fcee7972b9d549ed729b56ec28de16081
> >>>
> >>> Eventually, the plan is to kill msi_controller entirely, so introducing
> >>> new drivers that rely on it is not something I'm eager to see.
> >>
> >> Thanks, Marc.
> >>
> >>  X-Gene 1 MSI is handled by separate MSI controller block, so its
> >> driver implementation is different from GICv2m and GICv3. I will refer
> >
> > It will certainly be different in the sense that you won't use a stacked
> > domain on top of the GIC. But what I want to see is the use of a generic
> > pci_msi_domain on top of an irq_domain, just like we have on v2m and v3.
> > Thomas has also been vocal enough about that in the past, and x86 is
> > going down that road as well.
> >
> >> to what you did for Tegra MSI, but I don't see your latest changes in
> >> 4.0-rc4. Is the change you made for Tegra MSI going to mainline soon?
> >
> > Not yet. As you can see in this branch, this relies on some other
> > cleanups. But you can already convert most of your driver and put it in
> > the shape that matches what we have for v2m and v3. Once the required
> > cleanups are in, I'll remove the last traces of msi_controller myself if
> > necessary.
> >
> 
> Hi Marc, Bjorn,
> 
> I follow Marc's suggestion and convert my X-Gene 1 MSI driver to
> remove msi_controller struct and use generic pci_msi_domain on top of
> an irq_domain. The code requires Marc's changes in
> irq/kill-msi-controller branch to be compiled and function correctly,
> so I plan to post the patch on top of Marc's tree. Please let me know
> if you think I should have different approach to post this patch.

I don't think you should rely on this branch just yet (this is why I
suggested matching what we currently have for GICv2m and v3). Keeping
msi_controller is fine at the moment, even if we only use to store the
MSI domain. If and when this series makes it to mainline, I'll go over
the individual drivers to convert them to the new scheme, just like I
did for GIC and Tegra.

> Another question I have is when do you plan to roll out this hierarchy
> irq domain implementation for MSI, as I see some of Marc's changes to
> kill msi_controller structure and implement MSI irq domain still does
> not get into Bjorn's tree.

I'm still tinkering with it (I have some long standing comments from
Bjorn to address), and I'm working on non-PCI MSI support at the moment.

But the core MSI irq domain stuff is already in (this is what GICv2m
and GICv3 ITS are using), and this should be an easy thing to convert
your code.

Please let me know if I can be of any help.

Thanks,

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