From: "Pali Rohár" <email@example.com> To: Kunihiko Hayashi <firstname.lastname@example.org> Cc: Lorenzo Pieralisi <email@example.com>, Bjorn Helgaas <firstname.lastname@example.org>, Bjorn Helgaas <email@example.com>, Rob Herring <firstname.lastname@example.org>, Jingoo Han <email@example.com>, Gustavo Pimentel <firstname.lastname@example.org>, Marc Zyngier <email@example.com>, firstname.lastname@example.org, email@example.com, Jassi Brar <firstname.lastname@example.org>, Masami Hiramatsu <email@example.com>, firstname.lastname@example.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: <email@example.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
prev parent 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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --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).