* [PATCH v2 0/2] Add MSI support @ 2018-05-09 8:20 Yao Chen 2018-05-09 8:20 ` [PATCH v2 1/2] PCI: kirin: " Yao Chen 2018-05-09 8:20 ` [PATCH v2 2/2] arm64: dts: hi3660: Add pcie msi interrupt attribute Yao Chen 0 siblings, 2 replies; 5+ messages in thread From: Yao Chen @ 2018-05-09 8:20 UTC (permalink / raw) To: songxiaowei, wangbinghui, lorenzo.pieralisi, bhelgaas, xuwei5, robh+dt, mark.rutland, catalin.marinas, will.deacon, linux-pci, linux-kernel, linux-arm-kernel, devicetree Cc: dimitrysh, guodong.xu, chenyao11, suzhuangluan, kongfei Changes between V2 and V1 ========================= 1. seperate DT binding patch. 2. fix issues according to review comments (1)from Bjorn Helgaas: Update the style of subject and changelog. (2)from Bjorn Helgaas: Add msi irq number in the message. (3)from Bjorn Helgaas: Delete unnecessary code 'pci->pp.root_bus_nr = -1'. (4)from Dmitry Shmidt: Fix typing error. Replace 'interrupts-names' of 'interrupt-names'. Yao Chen (2): PCI: kirin: Add MSI support arm64: dts: hi3660: Add pcie msi interrupt attribute arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 ++ drivers/pci/dwc/pcie-kirin.c | 39 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) -- 1.9.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] PCI: kirin: Add MSI support 2018-05-09 8:20 [PATCH v2 0/2] Add MSI support Yao Chen @ 2018-05-09 8:20 ` Yao Chen 2018-05-10 14:04 ` Bjorn Helgaas 2018-05-09 8:20 ` [PATCH v2 2/2] arm64: dts: hi3660: Add pcie msi interrupt attribute Yao Chen 1 sibling, 1 reply; 5+ messages in thread From: Yao Chen @ 2018-05-09 8:20 UTC (permalink / raw) To: songxiaowei, wangbinghui, lorenzo.pieralisi, bhelgaas, xuwei5, robh+dt, mark.rutland, catalin.marinas, will.deacon, linux-pci, linux-kernel, linux-arm-kernel, devicetree Cc: dimitrysh, guodong.xu, chenyao11, suzhuangluan, kongfei Add support for MSI. Signed-off-by: Yao Chen <chenyao11@huawei.com> --- drivers/pci/dwc/pcie-kirin.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/drivers/pci/dwc/pcie-kirin.c b/drivers/pci/dwc/pcie-kirin.c index d2970a0..4328efe 100644 --- a/drivers/pci/dwc/pcie-kirin.c +++ b/drivers/pci/dwc/pcie-kirin.c @@ -426,9 +426,28 @@ static int kirin_pcie_establish_link(struct pcie_port *pp) return 0; } +static irqreturn_t kirin_pcie_msi_irq_handler(int irq, void *arg) +{ + struct pcie_port *pp = arg; + + return dw_handle_msi_irq(pp); +} + +static void kirin_pcie_msi_init(struct pcie_port *pp) +{ + dw_pcie_msi_init(pp); +} + +static void kirin_pcie_enable_interrupts(struct pcie_port *pp) +{ + if (IS_ENABLED(CONFIG_PCI_MSI)) + kirin_pcie_msi_init(pp); +} + static int kirin_pcie_host_init(struct pcie_port *pp) { kirin_pcie_establish_link(pp); + kirin_pcie_enable_interrupts(pp); return 0; } @@ -448,6 +467,26 @@ static int kirin_pcie_host_init(struct pcie_port *pp) static int __init kirin_add_pcie_port(struct dw_pcie *pci, struct platform_device *pdev) { + int ret; + + if (IS_ENABLED(CONFIG_PCI_MSI)) { + pci->pp.msi_irq = platform_get_irq(pdev, 0); + if (!pci->pp.msi_irq) { + dev_err(&pdev->dev, "failed to get msi irq[%d]\n", + pci->pp.msi_irq); + return -ENODEV; + } + ret = devm_request_irq(&pdev->dev, pci->pp.msi_irq, + kirin_pcie_msi_irq_handler, + IRQF_SHARED | IRQF_NO_THREAD, + "kirin_pcie_msi", &pci->pp); + if (ret) { + dev_err(&pdev->dev, "failed to request msi irq[%d]\n", + pci->pp.msi_irq); + return ret; + } + } + pci->pp.ops = &kirin_pcie_host_ops; return dw_pcie_host_init(&pci->pp); -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] PCI: kirin: Add MSI support 2018-05-09 8:20 ` [PATCH v2 1/2] PCI: kirin: " Yao Chen @ 2018-05-10 14:04 ` Bjorn Helgaas 2018-05-11 8:30 ` chenyao (F) 0 siblings, 1 reply; 5+ messages in thread From: Bjorn Helgaas @ 2018-05-10 14:04 UTC (permalink / raw) To: Yao Chen Cc: songxiaowei, wangbinghui, lorenzo.pieralisi, bhelgaas, xuwei5, robh+dt, mark.rutland, catalin.marinas, will.deacon, linux-pci, linux-kernel, linux-arm-kernel, devicetree, suzhuangluan, kongfei, dimitrysh, guodong.xu, Wolfram Sang, Tejun Heo [+cc Tejun, Wolfram] On Wed, May 09, 2018 at 04:20:11PM +0800, Yao Chen wrote: > Add support for MSI. > ... > @@ -448,6 +467,26 @@ static int kirin_pcie_host_init(struct pcie_port *pp) > static int __init kirin_add_pcie_port(struct dw_pcie *pci, > struct platform_device *pdev) > { > + int ret; > + > + if (IS_ENABLED(CONFIG_PCI_MSI)) { > + pci->pp.msi_irq = platform_get_irq(pdev, 0); > + if (!pci->pp.msi_irq) { I think this test is incorrect. platform_get_irq() returns a negative errno value when it fails. Most calls test "irq < 0" to check for failure. There's a lot of duplicated code like this, so maybe we should consider putting that check into devm_request_irq(), similar to what devm_ioremap_resource() does, so the driver code could look like this: pci->pp.msi_irq = platform_get_irq(pdev, 0); ret = devm_request_irq(&pdev->dev, pci->pp.msi_irq, ...); if (ret) { dev_err(&pdev->dev, "failed to request MSI IRQ\n"); return ret; } The basic devm_ioremap_resource() motivation is here: 72f8c0bfa0de ("lib: devres: add convenience function to remap a resource") and the same considerations seem to apply here. But that's more than you need to do for *this* series. So for now, I would simply fix the test to check for "irq < 0" and update the messages as I mention below. > + dev_err(&pdev->dev, "failed to get msi irq[%d]\n", > + pci->pp.msi_irq); > + return -ENODEV; > + } > + ret = devm_request_irq(&pdev->dev, pci->pp.msi_irq, > + kirin_pcie_msi_irq_handler, > + IRQF_SHARED | IRQF_NO_THREAD, > + "kirin_pcie_msi", &pci->pp); > + if (ret) { > + dev_err(&pdev->dev, "failed to request msi irq[%d]\n", s/msi irq/MSI IRQ/ in both dev_err() messages above. This is because the message is English text (not code), and the convention is that non-words like these initialisms written in all caps. I would style the first one as "failed to get MSI IRQ (%d)" because the %d there is a return code, probably -ENXIO. The second one should be "failed to request MSI IRQ %d" because here the %d is the actual IRQ. > + pci->pp.msi_irq); > + return ret; > + } > + } > + > pci->pp.ops = &kirin_pcie_host_ops; > > return dw_pcie_host_init(&pci->pp); > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2 1/2] PCI: kirin: Add MSI support 2018-05-10 14:04 ` Bjorn Helgaas @ 2018-05-11 8:30 ` chenyao (F) 0 siblings, 0 replies; 5+ messages in thread From: chenyao (F) @ 2018-05-11 8:30 UTC (permalink / raw) To: Bjorn Helgaas Cc: songxiaowei, Wangbinghui, lorenzo.pieralisi, bhelgaas, xuwei (O), robh+dt, mark.rutland, catalin.marinas, will.deacon, linux-pci, linux-kernel, linux-arm-kernel, devicetree, Suzhuangluan, Kongfei, dimitrysh, guodong.xu, Wolfram Sang, Tejun Heo Hi Bjorn, > > [+cc Tejun, Wolfram] > > On Wed, May 09, 2018 at 04:20:11PM +0800, Yao Chen wrote: > > Add support for MSI. > > ... > > > @@ -448,6 +467,26 @@ static int kirin_pcie_host_init(struct pcie_port > > *pp) static int __init kirin_add_pcie_port(struct dw_pcie *pci, > > struct platform_device *pdev) { > > + int ret; > > + > > + if (IS_ENABLED(CONFIG_PCI_MSI)) { > > + pci->pp.msi_irq = platform_get_irq(pdev, 0); > > + if (!pci->pp.msi_irq) { > > I think this test is incorrect. platform_get_irq() returns a negative errno > value when it fails. Most calls test "irq < 0" to check for failure. > > There's a lot of duplicated code like this, so maybe we should consider > putting that check into devm_request_irq(), similar to what > devm_ioremap_resource() does, so the driver code could look like this: > > pci->pp.msi_irq = platform_get_irq(pdev, 0); > ret = devm_request_irq(&pdev->dev, pci->pp.msi_irq, ...); > if (ret) { > dev_err(&pdev->dev, "failed to request MSI IRQ\n"); > return ret; > } > > The basic devm_ioremap_resource() motivation is here: 72f8c0bfa0de ("lib: > devres: add convenience function to remap a resource") and the same > considerations seem to apply here. > > But that's more than you need to do for *this* series. So for now, I would > simply fix the test to check for "irq < 0" and update the messages as I > mention below. Thank you for pointing out my mistake. I'll fix it. > > > + dev_err(&pdev->dev, "failed to get msi irq[%d]\n", > > + pci->pp.msi_irq); > > + return -ENODEV; > > + } > > + ret = devm_request_irq(&pdev->dev, pci->pp.msi_irq, > > + kirin_pcie_msi_irq_handler, > > + IRQF_SHARED | IRQF_NO_THREAD, > > + "kirin_pcie_msi", &pci->pp); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to request msi > irq[%d]\n", > > s/msi irq/MSI IRQ/ in both dev_err() messages above. This is because the > message is English text (not code), and the convention is that non-words like > these initialisms written in all caps. > > I would style the first one as "failed to get MSI IRQ (%d)" because the %d > there is a return code, probably -ENXIO. > > The second one should be "failed to request MSI IRQ %d" because here > the %d is the actual IRQ. > I'll fix it. Thanks again. > > + pci->pp.msi_irq); > > + return ret; > > + } > > + } > > + > > pci->pp.ops = &kirin_pcie_host_ops; > > > > return dw_pcie_host_init(&pci->pp); > > -- > > 1.9.1 > > Best regards, Yao ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] arm64: dts: hi3660: Add pcie msi interrupt attribute 2018-05-09 8:20 [PATCH v2 0/2] Add MSI support Yao Chen 2018-05-09 8:20 ` [PATCH v2 1/2] PCI: kirin: " Yao Chen @ 2018-05-09 8:20 ` Yao Chen 1 sibling, 0 replies; 5+ messages in thread From: Yao Chen @ 2018-05-09 8:20 UTC (permalink / raw) To: songxiaowei, wangbinghui, lorenzo.pieralisi, bhelgaas, xuwei5, robh+dt, mark.rutland, catalin.marinas, will.deacon, linux-pci, linux-kernel, linux-arm-kernel, devicetree Cc: dimitrysh, guodong.xu, chenyao11, suzhuangluan, kongfei Add pcie msi interrupt attribute for hi3660 SOC. Signed-off-by: Yao Chen <chenyao11@huawei.com> --- arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi index ec3eb8e..2cef8f4 100644 --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi @@ -872,6 +872,8 @@ 0x0 0x02000000>; num-lanes = <1>; #interrupt-cells = <1>; + interrupts = <0 283 4>; + interrupt-names = "msi"; interrupt-map-mask = <0xf800 0 0 7>; interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>, -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-11 8:31 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-09 8:20 [PATCH v2 0/2] Add MSI support Yao Chen 2018-05-09 8:20 ` [PATCH v2 1/2] PCI: kirin: " Yao Chen 2018-05-10 14:04 ` Bjorn Helgaas 2018-05-11 8:30 ` chenyao (F) 2018-05-09 8:20 ` [PATCH v2 2/2] arm64: dts: hi3660: Add pcie msi interrupt attribute Yao Chen
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).