linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali@kernel.org>
To: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Bjorn Helgaas <helgaas@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Rob Herring <robh@kernel.org>, Jingoo Han <jingoohan1@gmail.com>,
	Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
	Marc Zyngier <maz@kernel.org>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jassi Brar <jaswinder.singh@linaro.org>,
	Masami Hiramatsu <masami.hiramatsu@linaro.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v8 3/3] PCI: uniphier: Add misc interrupt handler to invoke PME and AER
Date: Thu, 29 Jul 2021 00:35:08 +0200	[thread overview]
Message-ID: <20210728223508.iffjpmk6ipjpvddh@pali> (raw)
In-Reply-To: <d96880c4-75ab-50b5-3ecf-0dfd2aa3b8f3@socionext.com>

On Wednesday 28 July 2021 14:29:15 Kunihiko Hayashi wrote:
> Hi Lorenzo, Pali,
> 
> On 2021/07/23 18:36, Kunihiko Hayashi wrote:
> > Hi Pali,
> 
> [snip]
> 
> > > Just you need to specify that new/private IRQ domain into
> > > irq_find_mapping() call.
> > 
> > I'll try to replace the events with new IRQ domain.
> According to Pali's suggestion, the bridge handles INTX and it isn't difficult
> to change IRQ's map for Root Port like the example.
> It seems that it can't be applied to MSI.

Hm... And it is hard to change mapping also for MSI via custom/new
callback?

> On the other hand, according to Lorenzo's suggestion,
> 
> >>>>>>> IMO this should be modelled with a separate IRQ domain and chip for
> >>>>>>> the root port (yes this implies describing the root port in the dts
> >>>>>>> file with a separate msi-parent).
> 
> Interrupts for PME/AER event is assigned to number 0 of MSI IRQ domain.

Yes. This is because Root Port of your PCIe controller provides this
information to OS.

> (pcie_port_enable_irq_vec() in portdrv_core.c)
> This expects MSI status bit 0 to be set when the event occurs.

Obviously (according to PCIe spec).

> However, in the uniphier PCIe controller, MSI status bit 0 is not set, but
> the PME/AER status bit in the glue logic is set.

And this "violates" PCIe spec and your controller needs custom handling
in driver to work...

> I think that it's hard to associate the new domain and "MSI-IRQ 0" event
> if the new IRQ domain and chip is modelled.

No. It was mean to assign all MSI IRQs (not only MSI number 0) which
comes from Root Port to new domain. Assigning just one MSI number to
separate domain is I guess impossible (and also does not make sense).

This is required to difference between MSI number 0 which comes from
real MSI path and "fake MSI number 0" which is just specific for Root
Port and does not share anything with real MSI interrupts. As MSI
interrupts are not shared it is required to prevent "mixing" interrupt
sources. And kernel can do it via different MSI domains.

So in the end you would have two different MSI numbers 0.

> So, I have no idea to handle both new IRQ domain and cascaded MSI event.

It was mean to define Root Port PCIe device in DTS. Then define a new
IRQ chip / domain in DTS. And specify in DTS that this Root Port PCIe
device should use this new IRQ chip / domain instead of default one.
And then you need to implement "driver" for this "virtual" IRQ chip /
domain to handle specific glue logic in this controller driver.

I was hoping that it is possible to set this mapping directly in
controller driver. But if you checked that only legacy IRQs can be done
in this way, and not MSI then it is either needed to go via this DTS
path OR try to figure out how to define this mapping in PCI subsystem
(maybe needs some changes?) also for MSI.

> Is there any example for that?

I do not know. I think you are the first one who have such buggy PCIe
controller which needs this specific kind of configuration and domains.

In my case in pci-aardvark.c, emulated kernel Root Port does not support
MSI interrupts (yet) so these "fake" interrupts are routed as legacy
INTA. And because legacy INTx are shared interrupts they can be mixed
together with real (as it is done prior my patch). My patch (link sent
in previous email) just separates "fake" INTA from "real" INTA via
separate domains for performance reasons.

But you use MSI interrupts, which means that it is required to have
logic to separate them into different domains to prevent mixing.

> Thank you,
> 
> ---
> Best Regards
> Kunihiko Hayashi

      reply	other threads:[~2021-07-28 22:35 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-28  1:31 [PATCH v8 0/3] PCI: uniphier: Add PME/AER support for UniPhier PCIe host controller Kunihiko Hayashi
2020-10-28  1:31 ` [PATCH v8 1/3] PCI: portdrv: Add pcie_port_service_get_irq() function Kunihiko Hayashi
2020-11-24 23:25   ` Bjorn Helgaas
2020-10-28  1:31 ` [PATCH v8 2/3] PCI: dwc: Add msi_host_isr() callback Kunihiko Hayashi
2020-10-28  1:31 ` [PATCH v8 3/3] PCI: uniphier: Add misc interrupt handler to invoke PME and AER Kunihiko Hayashi
2020-11-24 23:20   ` Bjorn Helgaas
2020-11-25 10:23     ` Lorenzo Pieralisi
2020-11-27 12:02       ` Kunihiko Hayashi
2021-07-18  0:51         ` Pali Rohár
2021-07-22 16:54           ` Kunihiko Hayashi
2021-07-22 17:26             ` Pali Rohár
2021-07-23  6:59               ` Kunihiko Hayashi
2021-07-23  8:37                 ` Pali Rohár
2021-07-23  9:36                   ` Kunihiko Hayashi
2021-07-28  5:29                     ` Kunihiko Hayashi
2021-07-28 22:35                       ` Pali Rohár [this message]

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=20210728223508.iffjpmk6ipjpvddh@pali \
    --to=pali@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=gustavo.pimentel@synopsys.com \
    --cc=hayashi.kunihiko@socionext.com \
    --cc=helgaas@kernel.org \
    --cc=jaswinder.singh@linaro.org \
    --cc=jingoohan1@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=masami.hiramatsu@linaro.org \
    --cc=maz@kernel.org \
    --cc=robh@kernel.org \
    --subject='Re: [PATCH v8 3/3] PCI: uniphier: Add misc interrupt handler to invoke PME and AER' \
    /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

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).