From: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Gustavo Pimentel <gustavo.pimentel@synopsys.com> Cc: Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Masahiro Yamada <yamada.masahiro@socionext.com>, "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Masami Hiramatsu <masami.hiramatsu@linaro.org>, Jassi Brar <jaswinder.singh@linaro.org> Subject: Re: [PATCH v2 2/2] PCI: controller: dwc: add UniPhier PCIe host controller support Date: Thu, 27 Sep 2018 16:44:26 +0900 [thread overview] Message-ID: <20180927164426.ECD6.4A936039@socionext.com> (raw) In-Reply-To: <20180926213135.A4B4.4A936039@socionext.com> Hi Lorenzo, Gustavo, On Wed, 26 Sep 2018 21:31:36 +0900 <hayashi.kunihiko@socionext.com> wrote: > Hi Lorenzo, Gustavo, > > Thank you for reviewing. > > On Tue, 25 Sep 2018 18:53:01 +0100 > Gustavo Pimentel <gustavo.pimentel@synopsys.com> wrote: > > > On 25/09/2018 17:14, Lorenzo Pieralisi wrote: > > > [+Gustavo, please have a look at INTX/MSI management] > > > > > > On Thu, Sep 06, 2018 at 06:40:32PM +0900, Kunihiko Hayashi wrote: > > >> This introduces specific glue layer for UniPhier platform to support > > >> PCIe host controller that is based on the DesignWare PCIe core, and > > >> this driver supports Root Complex (host) mode. > > > > > > Please read this thread and apply it to next versions: > > > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dpci-26m-3D150905742808166-26w-3D2&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=H8UNDDUGQnQnqfWr4CBios689dJcjxu4qeTTRGulLmU&s=CgcXc_2LThyOpW-4bCriJNo9H1lzROEdy_cG9p-Y5hU&e= > > I also found this thread in previous linux-pci, and I think it's helpful for me. > I'll check it carefully. [snip] > > >> + ret = devm_request_irq(dev, pp->irq, uniphier_pcie_irq_handler, > > >> + IRQF_SHARED, "pcie", priv); > > > > > > This is wrong, you should set-up a chained IRQ for INTX. > > > > > > I *think* that > > > > > > ks_pcie_setup_interrupts() > > > > > > is a good example to start with but I wonder whether it is worth > > > generalizing the INTX approach to designware as a whole as it was > > > done for MSIs. > > > > > > Thoughts ? > > > > From what I understood this is for legacy IRQ, right? > > Yes. For legacy IRQ. > > > Like you (Lorenzo) said there is 2 drivers (pci-keystone-dw.c and pci-dra7xx.c) > > that uses it and can be use as a template for handling this type of interrupts. > > > > We can try to pass some kind of generic INTX function to the DesignWare host > > library to handling this, but this will require some help from keystone and > > dra7xx maintainers, since my setup doesn't have legacy IRQ HW support. > > Now I think it's difficult to make a template for INTX function, > and at first, I'll try to re-write this part with reference to pci-keystone-dw.c. I understand that there are 2 types of interrupt and the drivers. One like pci-keystone-dw.c is: - there are 4 interrupts for legacy, - invoke handlers for each interrupt, and handle the interrupt, - call irq_set_chained_handler_and_data() to make a chain of the interrupts when initializing The other like pci-dra7xx.c is: - there is 1 IRQ for legacy as a parent, - check an interrupt factor register, and handle the interrupt correspond to the factor, - call request_irq() for the parent IRQ and irq_domain_add_linear() for the factor when initializing The pcie-uniphier.c is the same type as the latter (like pci-dra7xx.c). However, in pci-dra7xx.c, MSI and legacy IRQ share the same interrupt number, so the same handler is called and the handler divides these IRQs. (found in dra7xx_pcie_msi_irq_handler()) In pcie-uniphier.c, MSI and legacy IRQ are independent. Therefore it's necessary to prepare the handler for the legacy IRQ. I think that it's difficult to apply the way of pci-keystone-dw.c, and uniphier_pcie_irq_handler() and calling devm_request_irq() are still necessary to handle legacy IRQ. Thank you, --- Best Regards, Kunihiko Hayashi
WARNING: multiple messages have this Message-ID (diff)
From: hayashi.kunihiko@socionext.com (Kunihiko Hayashi) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 2/2] PCI: controller: dwc: add UniPhier PCIe host controller support Date: Thu, 27 Sep 2018 16:44:26 +0900 [thread overview] Message-ID: <20180927164426.ECD6.4A936039@socionext.com> (raw) In-Reply-To: <20180926213135.A4B4.4A936039@socionext.com> Hi Lorenzo, Gustavo, On Wed, 26 Sep 2018 21:31:36 +0900 <hayashi.kunihiko@socionext.com> wrote: > Hi Lorenzo, Gustavo, > > Thank you for reviewing. > > On Tue, 25 Sep 2018 18:53:01 +0100 > Gustavo Pimentel <gustavo.pimentel@synopsys.com> wrote: > > > On 25/09/2018 17:14, Lorenzo Pieralisi wrote: > > > [+Gustavo, please have a look at INTX/MSI management] > > > > > > On Thu, Sep 06, 2018 at 06:40:32PM +0900, Kunihiko Hayashi wrote: > > >> This introduces specific glue layer for UniPhier platform to support > > >> PCIe host controller that is based on the DesignWare PCIe core, and > > >> this driver supports Root Complex (host) mode. > > > > > > Please read this thread and apply it to next versions: > > > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dpci-26m-3D150905742808166-26w-3D2&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=H8UNDDUGQnQnqfWr4CBios689dJcjxu4qeTTRGulLmU&s=CgcXc_2LThyOpW-4bCriJNo9H1lzROEdy_cG9p-Y5hU&e= > > I also found this thread in previous linux-pci, and I think it's helpful for me. > I'll check it carefully. [snip] > > >> + ret = devm_request_irq(dev, pp->irq, uniphier_pcie_irq_handler, > > >> + IRQF_SHARED, "pcie", priv); > > > > > > This is wrong, you should set-up a chained IRQ for INTX. > > > > > > I *think* that > > > > > > ks_pcie_setup_interrupts() > > > > > > is a good example to start with but I wonder whether it is worth > > > generalizing the INTX approach to designware as a whole as it was > > > done for MSIs. > > > > > > Thoughts ? > > > > From what I understood this is for legacy IRQ, right? > > Yes. For legacy IRQ. > > > Like you (Lorenzo) said there is 2 drivers (pci-keystone-dw.c and pci-dra7xx.c) > > that uses it and can be use as a template for handling this type of interrupts. > > > > We can try to pass some kind of generic INTX function to the DesignWare host > > library to handling this, but this will require some help from keystone and > > dra7xx maintainers, since my setup doesn't have legacy IRQ HW support. > > Now I think it's difficult to make a template for INTX function, > and at first, I'll try to re-write this part with reference to pci-keystone-dw.c. I understand that there are 2 types of interrupt and the drivers. One like pci-keystone-dw.c is: - there are 4 interrupts for legacy, - invoke handlers for each interrupt, and handle the interrupt, - call irq_set_chained_handler_and_data() to make a chain of the interrupts when initializing The other like pci-dra7xx.c is: - there is 1 IRQ for legacy as a parent, - check an interrupt factor register, and handle the interrupt correspond to the factor, - call request_irq() for the parent IRQ and irq_domain_add_linear() for the factor when initializing The pcie-uniphier.c is the same type as the latter (like pci-dra7xx.c). However, in pci-dra7xx.c, MSI and legacy IRQ share the same interrupt number, so the same handler is called and the handler divides these IRQs. (found in dra7xx_pcie_msi_irq_handler()) In pcie-uniphier.c, MSI and legacy IRQ are independent. Therefore it's necessary to prepare the handler for the legacy IRQ. I think that it's difficult to apply the way of pci-keystone-dw.c, and uniphier_pcie_irq_handler() and calling devm_request_irq() are still necessary to handle legacy IRQ. Thank you, --- Best Regards, Kunihiko Hayashi
next prev parent reply other threads:[~2018-09-27 7:44 UTC|newest] Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-09-06 9:40 [PATCH v2 0/2] add new UniPhier PCIe host driver Kunihiko Hayashi 2018-09-06 9:40 ` Kunihiko Hayashi 2018-09-06 9:40 ` Kunihiko Hayashi 2018-09-06 9:40 ` [PATCH v2 1/2] dt-bindings: PCI: add UniPhier PCIe host controller description Kunihiko Hayashi 2018-09-06 9:40 ` Kunihiko Hayashi 2018-09-06 9:40 ` Kunihiko Hayashi 2018-09-25 20:55 ` Rob Herring 2018-09-25 20:55 ` Rob Herring 2018-09-26 12:33 ` Kunihiko Hayashi 2018-09-26 12:33 ` Kunihiko Hayashi 2018-09-26 12:33 ` Kunihiko Hayashi 2018-09-06 9:40 ` [PATCH v2 2/2] PCI: controller: dwc: add UniPhier PCIe host controller support Kunihiko Hayashi 2018-09-06 9:40 ` Kunihiko Hayashi 2018-09-06 9:40 ` Kunihiko Hayashi 2018-09-25 16:14 ` Lorenzo Pieralisi 2018-09-25 16:14 ` Lorenzo Pieralisi 2018-09-25 17:53 ` Gustavo Pimentel 2018-09-25 17:53 ` Gustavo Pimentel 2018-09-25 17:53 ` Gustavo Pimentel 2018-09-26 12:31 ` Kunihiko Hayashi 2018-09-26 12:31 ` Kunihiko Hayashi 2018-09-26 12:31 ` Kunihiko Hayashi 2018-09-27 7:44 ` Kunihiko Hayashi [this message] 2018-09-27 7:44 ` Kunihiko Hayashi 2018-09-27 7:44 ` Kunihiko Hayashi 2018-09-28 11:06 ` Lorenzo Pieralisi 2018-09-28 11:06 ` Lorenzo Pieralisi 2018-09-28 11:06 ` Lorenzo Pieralisi 2018-09-28 13:17 ` Marc Zyngier 2018-09-28 13:17 ` Marc Zyngier 2018-09-28 13:17 ` Marc Zyngier 2018-09-28 15:43 ` Lorenzo Pieralisi 2018-09-28 15:43 ` Lorenzo Pieralisi 2018-09-28 15:43 ` Lorenzo Pieralisi 2018-10-08 5:45 ` Kishon Vijay Abraham I 2018-10-08 5:45 ` Kishon Vijay Abraham I 2018-10-08 5:45 ` Kishon Vijay Abraham I 2018-10-08 14:32 ` Lorenzo Pieralisi 2018-10-08 14:32 ` Lorenzo Pieralisi 2018-10-08 14:32 ` Lorenzo Pieralisi 2018-10-12 10:50 ` Kunihiko Hayashi 2018-10-12 10:50 ` Kunihiko Hayashi 2018-10-12 10:50 ` Kunihiko Hayashi 2018-10-01 10:06 ` Lorenzo Pieralisi 2018-10-01 10:06 ` Lorenzo Pieralisi 2018-10-01 10:06 ` Lorenzo Pieralisi 2018-10-01 11:06 ` Kunihiko Hayashi 2018-10-01 11:06 ` Kunihiko Hayashi 2018-10-01 11:06 ` Kunihiko Hayashi
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=20180927164426.ECD6.4A936039@socionext.com \ --to=hayashi.kunihiko@socionext.com \ --cc=bhelgaas@google.com \ --cc=devicetree@vger.kernel.org \ --cc=gustavo.pimentel@synopsys.com \ --cc=jaswinder.singh@linaro.org \ --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=mark.rutland@arm.com \ --cc=masami.hiramatsu@linaro.org \ --cc=robh+dt@kernel.org \ --cc=yamada.masahiro@socionext.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.