Message ID | 1444923568-17413-8-git-send-email-marc.zyngier@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Marc: ? 2015/10/15 23:39, Marc Zyngier ??: > In order to demonstrate how to put together a wire/MSI bridge, > add a dummy driver that doesn't do anything at all, except > for allocating interrupts. > > It comes together with an even more stupid client driver that > allocates an interrupt and dump the hierarchy of that interrupt. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- [...] > + > +static int msichip_domain_alloc(struct irq_domain *domain, unsigned int virq, > + unsigned int nr_irqs, void *arg) > +{ > + int i, err; > + irq_hw_number_t hwirq; > + unsigned int type; > + struct irq_fwspec *fwspec = arg; > + void *data; > + > + err = msichip_domain_translate(domain, fwspec, &hwirq, &type); > + if (err) > + return err; > + .translate function already called once in irq_domain_translate(), I think we don't need call this fucntion one more time here. > + err = platform_msi_domain_alloc(domain, virq, nr_irqs); > + if (err) > + return err; > + > + data = platform_msi_get_host_data(domain); > + for (i = 0; i < nr_irqs; i++) > + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i, > + &msichip_chip, data); > + > + return 0; > +} > + [...] > + > +static struct platform_driver msichip_driver = { > + .driver = { > + .name = "msichip", > + .of_match_table = msichip_of_match, > + }, > + .probe = msichip_probe, > +}; > +/* Do not define this as an irqchip */ > +module_platform_driver(msichip_driver); > + > + I think,for a interrupt controller, msichip driver initialization maybe is too late for some devices which connect to this irqchip if we use module_platform_driver. So, how about use the arch_initcall to register the msichip driver? Thanks! Ma Jun
On 04/11/15 08:00, majun (F) wrote: > Hi Marc: > > ? 2015/10/15 23:39, Marc Zyngier ??: >> In order to demonstrate how to put together a wire/MSI bridge, >> add a dummy driver that doesn't do anything at all, except >> for allocating interrupts. >> >> It comes together with an even more stupid client driver that >> allocates an interrupt and dump the hierarchy of that interrupt. >> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >> --- > [...] >> + >> +static int msichip_domain_alloc(struct irq_domain *domain, unsigned int virq, >> + unsigned int nr_irqs, void *arg) >> +{ >> + int i, err; >> + irq_hw_number_t hwirq; >> + unsigned int type; >> + struct irq_fwspec *fwspec = arg; >> + void *data; >> + >> + err = msichip_domain_translate(domain, fwspec, &hwirq, &type); >> + if (err) >> + return err; >> + > > .translate function already called once in irq_domain_translate(), > I think we don't need call this fucntion one more time here. if you don't translate it here, how do you obtain the hwirq that you have to pass to irq_domain_set_hwirq_and_chip just below? >> + err = platform_msi_domain_alloc(domain, virq, nr_irqs); >> + if (err) >> + return err; >> + >> + data = platform_msi_get_host_data(domain); >> + for (i = 0; i < nr_irqs; i++) >> + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i, >> + &msichip_chip, data); >> + >> + return 0; >> +} >> + > [...] >> + >> +static struct platform_driver msichip_driver = { >> + .driver = { >> + .name = "msichip", >> + .of_match_table = msichip_of_match, >> + }, >> + .probe = msichip_probe, >> +}; >> +/* Do not define this as an irqchip */ >> +module_platform_driver(msichip_driver); >> + >> + > > I think,for a interrupt controller, msichip driver initialization maybe is too late > for some devices which connect to this irqchip if we use module_platform_driver. That's a consequence of this design. This is why I insisted on the fact that this is currently avoided by using deferred probe in drivers, and that it should be solved by having a probe order. Either way, this is not something that we can solve at that level (see the multiple proposal for this on the various lists). > So, how about use the arch_initcall to register the msichip driver? You're only pushing the problem one level up. And you'll realize that this is not enough for some random driver. This is not sustainable, and must be addressed properly. Thanks, M.
Hi Marc > -----Original Message----- > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org] > On Behalf Of Marc Zyngier > Sent: 04 November 2015 09:04 > To: majun (F); Thomas Gleixner; Jiang Liu; Jason Cooper > Cc: linux-arm-kernel@lists.infradead.org; linux-pci@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH RFC 7/7] irqchip: [Example] dummy wired interrupt/MSI > bridge driver > > On 04/11/15 08:00, majun (F) wrote: > > Hi Marc: > > > > ? 2015/10/15 23:39, Marc Zyngier ??: > >> In order to demonstrate how to put together a wire/MSI bridge, > >> add a dummy driver that doesn't do anything at all, except > >> for allocating interrupts. > >> > >> It comes together with an even more stupid client driver that > >> allocates an interrupt and dump the hierarchy of that interrupt. > >> > >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > >> --- > > [...] > >> + > >> +static int msichip_domain_alloc(struct irq_domain *domain, unsigned int > virq, > >> + unsigned int nr_irqs, void *arg) > >> +{ > >> + int i, err; > >> + irq_hw_number_t hwirq; > >> + unsigned int type; > >> + struct irq_fwspec *fwspec = arg; > >> + void *data; > >> + > >> + err = msichip_domain_translate(domain, fwspec, &hwirq, &type); > >> + if (err) > >> + return err; > >> + > > > > .translate function already called once in irq_domain_translate(), > > I think we don't need call this fucntion one more time here. > > if you don't translate it here, how do you obtain the hwirq that you > have to pass to irq_domain_set_hwirq_and_chip just below? > > >> + err = platform_msi_domain_alloc(domain, virq, nr_irqs); > >> + if (err) > >> + return err; > >> + > >> + data = platform_msi_get_host_data(domain); > >> + for (i = 0; i < nr_irqs; i++) > >> + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i, > >> + &msichip_chip, data); > >> + > >> + return 0; > >> +} > >> + > > [...] > >> + > >> +static struct platform_driver msichip_driver = { > >> + .driver = { > >> + .name = "msichip", > >> + .of_match_table = msichip_of_match, > >> + }, > >> + .probe = msichip_probe, > >> +}; > >> +/* Do not define this as an irqchip */ > >> +module_platform_driver(msichip_driver); > >> + > >> + > > > > I think,for a interrupt controller, msichip driver initialization maybe is > too late > > for some devices which connect to this irqchip if we use > module_platform_driver. > > That's a consequence of this design. This is why I insisted on the fact > that this is currently avoided by using deferred probe in drivers, and Mmm using te deferred probe would mean to rework all the drivers of the potential devices connected to mbi-gen...would that be sustainable/acceptable? > that it should be solved by having a probe order. Either way, this is > not something that we can solve at that level (see the multiple proposal > for this on the various lists). Could you point me to the relevant discussions for this...? Thanks Gab > > > So, how about use the arch_initcall to register the msichip driver? > > You're only pushing the problem one level up. And you'll realize that > this is not enough for some random driver. This is not sustainable, and > must be addressed properly. > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny... > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/11/15 08:25, Gabriele Paoloni wrote: > Hi Marc > >> -----Original Message----- >> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org] >> On Behalf Of Marc Zyngier >> Sent: 04 November 2015 09:04 >> To: majun (F); Thomas Gleixner; Jiang Liu; Jason Cooper >> Cc: linux-arm-kernel@lists.infradead.org; linux-pci@vger.kernel.org; linux- >> kernel@vger.kernel.org >> Subject: Re: [PATCH RFC 7/7] irqchip: [Example] dummy wired interrupt/MSI >> bridge driver >> >> On 04/11/15 08:00, majun (F) wrote: [...] >>> I think,for a interrupt controller, msichip driver initialization maybe is >> too late >>> for some devices which connect to this irqchip if we use >> module_platform_driver. >> >> That's a consequence of this design. This is why I insisted on the fact >> that this is currently avoided by using deferred probe in drivers, and > > Mmm using te deferred probe would mean to rework all the drivers of the > potential devices connected to mbi-gen...would that be sustainable/acceptable? I'm tempted to reply "Not my problem". Or rather, not a problem I'm trying to solve right now (or any time soon). I'm pretty sure that sprinkling -EPROBE_DEFER on all possible drivers will result in a resounding NAK, which is is why I suggested that someone with a vested interest dedicates some quality time helping those who are trying to solve this issue for good. >> that it should be solved by having a probe order. Either way, this is >> not something that we can solve at that level (see the multiple proposal >> for this on the various lists). > > Could you point me to the relevant discussions for this...? Google is, as always, your dearest friend. But here you go: - LWN has some quality coverage of the KS discussions (assuming you're a subscriber, otherwise you'll have to wait for another week): http://lwn.net/Articles/662820/ - There is also Tomeu Vizoso's series, which itself builds upon other previous attempts at solving this: https://lwn.net/Articles/658690/ Thanks, M.
> -----Original Message----- > From: Marc Zyngier [mailto:marc.zyngier@arm.com] > Sent: Thursday, November 05, 2015 9:36 AM > To: Gabriele Paoloni; majun (F); Thomas Gleixner; Jiang Liu; Jason > Cooper > Cc: linux-arm-kernel@lists.infradead.org; linux-pci@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH RFC 7/7] irqchip: [Example] dummy wired > interrupt/MSI bridge driver > > On 05/11/15 08:25, Gabriele Paoloni wrote: > > Hi Marc > > > >> -----Original Message----- > >> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci- > owner@vger.kernel.org] > >> On Behalf Of Marc Zyngier > >> Sent: 04 November 2015 09:04 > >> To: majun (F); Thomas Gleixner; Jiang Liu; Jason Cooper > >> Cc: linux-arm-kernel@lists.infradead.org; linux-pci@vger.kernel.org; > linux- > >> kernel@vger.kernel.org > >> Subject: Re: [PATCH RFC 7/7] irqchip: [Example] dummy wired > interrupt/MSI > >> bridge driver > >> > >> On 04/11/15 08:00, majun (F) wrote: > > [...] > > >>> I think,for a interrupt controller, msichip driver initialization > maybe is > >> too late > >>> for some devices which connect to this irqchip if we use > >> module_platform_driver. > >> > >> That's a consequence of this design. This is why I insisted on the > fact > >> that this is currently avoided by using deferred probe in drivers, > and > > > > Mmm using te deferred probe would mean to rework all the drivers of > the > > potential devices connected to mbi-gen...would that be > sustainable/acceptable? > > I'm tempted to reply "Not my problem". Or rather, not a problem I'm > trying to solve right now (or any time soon). > > I'm pretty sure that sprinkling -EPROBE_DEFER on all possible drivers > will result in a resounding NAK, which is is why I suggested that > someone with a vested interest dedicates some quality time helping > those > who are trying to solve this issue for good. Yes you're right, makes perfect sense > > >> that it should be solved by having a probe order. Either way, this > is > >> not something that we can solve at that level (see the multiple > proposal > >> for this on the various lists). > > > > Could you point me to the relevant discussions for this...? > > Google is, as always, your dearest friend. But here you go: > > - LWN has some quality coverage of the KS discussions (assuming you're > a > subscriber, otherwise you'll have to wait for another week): > http://lwn.net/Articles/662820/ > > - There is also Tomeu Vizoso's series, which itself builds upon other > previous attempts at solving this: https://lwn.net/Articles/658690/ > Great, many thanks for pointing them out. I'll look into these. Thanks again Gab > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 4d7294e..73e38e6 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -193,3 +193,10 @@ config IRQ_MXS def_bool y if MACH_ASM9260 || ARCH_MXS select IRQ_DOMAIN select STMP_DEVICE + +config DUMMY_MSICHIP + bool "Dummy irqchip on MSI" + depends on GENERIC_MSI_IRQ_DOMAIN + help + Dummy stacked irqchip on top of platform MSI + diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index 177f78f..682e7dc 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -55,3 +55,4 @@ obj-$(CONFIG_RENESAS_H8S_INTC) += irq-renesas-h8s.o obj-$(CONFIG_ARCH_SA1100) += irq-sa11x0.o obj-$(CONFIG_INGENIC_IRQ) += irq-ingenic.o obj-$(CONFIG_IMX_GPCV2) += irq-imx-gpcv2.o +obj-$(CONFIG_DUMMY_MSICHIP) += irq-msichip.o diff --git a/drivers/irqchip/irq-msichip.c b/drivers/irqchip/irq-msichip.c new file mode 100644 index 0000000..82b283b --- /dev/null +++ b/drivers/irqchip/irq-msichip.c @@ -0,0 +1,271 @@ +/* + * Non-functionnal example for a wired irq <-> MSI bridge + * + * Copyright (C) 2015 ARM Limited, All Rights Reserved. + * Author: Marc Zyngier <marc.zyngier@arm.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +/* + * DT fragment to represent the MSI bridge: + + intc: msichip { + compatible = "dummy,msichip"; + num-msis = 32; + interrupt-controller; + interrupt-parent = <&gic>; + #interrupt-cells = <0x2>; + msi-parent = <&its 1234>; + }; + + * DT fragment to represent the device connected to the bridge: + + dummy-dev { + compatible = "dummy,device"; + interrupt-parent = <intc>; + interrupts = <0x5 0x1>; + }; + + * When "dummy,device" gets probed, it dumps the hierarchy for the + * interrupt it has allocated: + + dummydev dummy-dev: Allocated IRQ35 + dummydev dummy-dev: Probing OK + dummydev dummy-dev: IRQ35 hwirq 5 domain msichip_domain_ops + dummydev dummy-dev: IRQ35 hwirq 0 domain msi_domain_ops + dummydev dummy-dev: IRQ35 hwirq 8192 domain its_domain_ops + dummydev dummy-dev: IRQ35 hwirq 8192 domain gic_irq_domain_ops + */ + +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/irq.h> +#include <linux/irqchip.h> +#include <linux/irqdomain.h> +#include <linux/module.h> +#include <linux/msi.h> +#include <linux/platform_device.h> +#include <linux/of.h> +#include <linux/of_irq.h> +#include <linux/of_address.h> +#include <linux/slab.h> +#include <linux/spinlock.h> + +static void msichip_mask(struct irq_data *data) +{ + /* Do something */ +} + +static void msichip_unmask(struct irq_data *data) +{ + /* Do something */ +} + +static void msichip_eoi(struct irq_data *data) +{ + /* Do something */ +} + +static int msichip_set_type(struct irq_data *data, unsigned int type) +{ + /* Do something */ + return 0; +} + +static int msichip_retrigger(struct irq_data *data) +{ + /* Do something */ + return 0; +} + +static int msichip_set_affinity(struct irq_data *data, + const struct cpumask *dest, bool force) +{ + /* Do something */ + return 0; +} + +static struct irq_chip msichip_chip = { + .name = "MSICHIP", + .irq_mask = msichip_mask, + .irq_unmask = msichip_unmask, + .irq_eoi = msichip_eoi, + .irq_set_type = msichip_set_type, + .irq_retrigger = msichip_retrigger, + .irq_set_affinity = msichip_set_affinity, +}; + +static int msichip_domain_translate(struct irq_domain *d, + struct irq_fwspec *fwspec, + unsigned long *hwirq, + unsigned int *type) +{ + if (is_of_node(fwspec->fwnode)) { + if (fwspec->param_count != 2) + return -EINVAL; + + *hwirq = fwspec->param[0]; + *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK; + return 0; + } + + return -EINVAL; +} + +static int msichip_domain_alloc(struct irq_domain *domain, unsigned int virq, + unsigned int nr_irqs, void *arg) +{ + int i, err; + irq_hw_number_t hwirq; + unsigned int type; + struct irq_fwspec *fwspec = arg; + void *data; + + err = msichip_domain_translate(domain, fwspec, &hwirq, &type); + if (err) + return err; + + err = platform_msi_domain_alloc(domain, virq, nr_irqs); + if (err) + return err; + + data = platform_msi_get_host_data(domain); + for (i = 0; i < nr_irqs; i++) + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i, + &msichip_chip, data); + + return 0; +} + +static const struct irq_domain_ops msichip_domain_ops = { + .translate = msichip_domain_translate, + .alloc = msichip_domain_alloc, + .free = irq_domain_free_irqs_common, +}; + +struct msichip_data { + /* Add whatever you fancy here */ + struct platform_device *pdev; +}; + +static void msichip_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg) +{ + /* Do the right thing */ +} + +static int msichip_probe(struct platform_device *pdev) +{ + struct irq_domain *domain; + struct msichip_data *msichip_data; + u32 num_msis; + + dev_info(&pdev->dev, "Probing\n"); + msichip_data = kzalloc(sizeof(*msichip_data), GFP_KERNEL); + if (!msichip_data) + return -ENOMEM; + + msichip_data->pdev = pdev; + + /* If there is no "num-msi" property, assume 64... */ + if (of_property_read_u32(pdev->dev.of_node, "num-msis", &num_msis) < 0) + num_msis = 64; + + dev_info(&pdev->dev, "allocating %d MSIs\n", num_msis); + + domain = platform_msi_create_device_domain(&pdev->dev, num_msis, + msichip_write_msi_msg, + &msichip_domain_ops, + msichip_data); + + if (!domain){ + kfree(msichip_data); + return -ENOMEM; + } + + dev_info(&pdev->dev, "Probing OK\n"); + return 0; +} + +static const struct of_device_id msichip_of_match[] = { + { .compatible = "dummy,msichip", }, + { }, +}; +MODULE_DEVICE_TABLE(of, msichip_of_match); + +static struct platform_driver msichip_driver = { + .driver = { + .name = "msichip", + .of_match_table = msichip_of_match, + }, + .probe = msichip_probe, +}; +/* Do not define this as an irqchip */ +module_platform_driver(msichip_driver); + + + +/* Driver for a dummy device connected to the MSI bridge */ +static irqreturn_t dummydev_handler(int irq, void *dummy) +{ + return IRQ_HANDLED; +} + +static void dummydev_dump_hierarchy(struct device *dev, int irq) +{ + struct irq_data *data = irq_get_irq_data(irq); + + while(data) { + dev_info(dev, "IRQ%d hwirq %d domain %ps\n", + data->irq, (int)data->hwirq, data->domain->ops); + data = data->parent_data; + } +} + +static int dummydev_probe(struct platform_device *pdev) +{ + int irq; + + dev_info(&pdev->dev, "Probing\n"); + irq = irq_of_parse_and_map(pdev->dev.of_node, 0); + if (!irq) { + dev_err(&pdev->dev, "irq allocation failed, defering\n"); + return -EPROBE_DEFER; + } + + dev_info(&pdev->dev, "Allocated IRQ%d\n", irq); + + if (request_irq(irq, dummydev_handler, 0, "dummydev", pdev)) + return -EINVAL; + + dev_info(&pdev->dev, "Probing OK\n"); + + dummydev_dump_hierarchy(&pdev->dev, irq); + return 0; +} + +static const struct of_device_id dummydev_of_match[] = { + { .compatible = "dummy,device", }, + { }, +}; +MODULE_DEVICE_TABLE(of, dummydev_of_match); + +static struct platform_driver dummydev_driver = { + .driver = { + .name = "dummydev", + .of_match_table = dummydev_of_match, + }, + .probe = dummydev_probe, +}; + +module_platform_driver(dummydev_driver);
In order to demonstrate how to put together a wire/MSI bridge, add a dummy driver that doesn't do anything at all, except for allocating interrupts. It comes together with an even more stupid client driver that allocates an interrupt and dump the hierarchy of that interrupt. Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- drivers/irqchip/Kconfig | 7 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-msichip.c | 271 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 279 insertions(+) create mode 100644 drivers/irqchip/irq-msichip.c