| Message ID | 20180622151432.1566-9-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 22/06/18 16:14, Miquel Raynal wrote: > NSR (non-secure interrupts) are handled in the ICU driver like if there > was only this type of interrupt in the ICU. Change this behavior to > prepare the introduction of SEI (System Error Interrupts) support by > moving the NSR code in a separate function. This is done under the form > of a 'probe' function to ease future migration to NSR/SEI being platform > devices part of the ICU. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > Reviewed-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > --- > drivers/irqchip/irq-mvebu-icu.c | 58 +++++++++++++++++++++++------------------ > 1 file changed, 33 insertions(+), 25 deletions(-) > > diff --git a/drivers/irqchip/irq-mvebu-icu.c b/drivers/irqchip/irq-mvebu-icu.c > index 607948870a14..24d45186eb6b 100644 > --- a/drivers/irqchip/irq-mvebu-icu.c > +++ b/drivers/irqchip/irq-mvebu-icu.c > @@ -205,6 +205,37 @@ static const struct irq_domain_ops mvebu_icu_domain_ops = { > .free = mvebu_icu_irq_domain_free, > }; > > +static int mvebu_icu_subset_probe(struct platform_device *pdev) > +{ > + struct device_node *msi_parent_dn; > + struct irq_domain *irq_domain; > + struct mvebu_icu *icu; > + > + icu = dev_get_drvdata(&pdev->dev); > + if (!icu) > + return -ENODEV; > + > + pdev->dev.msi_domain = of_msi_get_domain(&pdev->dev, pdev->dev.of_node, > + DOMAIN_BUS_PLATFORM_MSI); > + if (!pdev->dev.msi_domain) > + return -EPROBE_DEFER; > + > + msi_parent_dn = irq_domain_get_of_node(pdev->dev.msi_domain); > + if (!msi_parent_dn) > + return -ENODEV; > + > + irq_domain = platform_msi_create_device_domain(&pdev->dev, ICU_MAX_IRQS, > + mvebu_icu_write_msg, > + &mvebu_icu_domain_ops, > + icu); > + if (!irq_domain) { > + dev_err(&pdev->dev, "Failed to create ICU MSI domain\n"); > + return -ENOMEM; > + } > + > + return 0; > +} > + > static struct regmap_config mvebu_icu_regmap_config = { > .reg_bits = 32, > .val_bits = 32, > @@ -215,9 +246,6 @@ static struct regmap_config mvebu_icu_regmap_config = { > static int mvebu_icu_probe(struct platform_device *pdev) > { > struct mvebu_icu *icu; > - struct device_node *node = pdev->dev.of_node; > - struct device_node *gicp_dn; > - struct irq_domain *irq_domain; > struct resource *res; > void __iomem *regs; > int i; > @@ -255,19 +283,6 @@ static int mvebu_icu_probe(struct platform_device *pdev) > icu->irq_chip.irq_set_affinity = irq_chip_set_affinity_parent; > #endif > > - /* > - * We're probed after MSI domains have been resolved, so force > - * resolution here. > - */ > - pdev->dev.msi_domain = of_msi_get_domain(&pdev->dev, node, > - DOMAIN_BUS_PLATFORM_MSI); > - if (!pdev->dev.msi_domain) > - return -EPROBE_DEFER; > - > - gicp_dn = irq_domain_get_of_node(pdev->dev.msi_domain); > - if (!gicp_dn) > - return -ENODEV; > - > /* > * Clean all ICU interrupts with type SPI_NSR, required to > * avoid unpredictable SPI assignments done by firmware. > @@ -282,16 +297,9 @@ static int mvebu_icu_probe(struct platform_device *pdev) > regmap_write(icu->regmap, ICU_INT_CFG(i), 0); > } > > - irq_domain = > - platform_msi_create_device_domain(&pdev->dev, ICU_MAX_IRQS, > - mvebu_icu_write_msg, > - &mvebu_icu_domain_ops, icu); > - if (!irq_domain) { > - dev_err(&pdev->dev, "Failed to create ICU domain\n"); > - return -ENOMEM; > - } > + platform_set_drvdata(pdev, icu); What is the upshot of passing this icu pointer as part of the platform device structure instead of simply passing it as a parameter? Is anything else using it? Thanks, M.
Hi Marc, [...] > > - /* > > - * We're probed after MSI domains have been resolved, so force > > - * resolution here. > > - */ > > - pdev->dev.msi_domain = of_msi_get_domain(&pdev->dev, node, > > - DOMAIN_BUS_PLATFORM_MSI); > > - if (!pdev->dev.msi_domain) > > - return -EPROBE_DEFER; > > - > > - gicp_dn = irq_domain_get_of_node(pdev->dev.msi_domain); > > - if (!gicp_dn) > > - return -ENODEV; > > - > > /* > > * Clean all ICU interrupts with type SPI_NSR, required to > > * avoid unpredictable SPI assignments done by firmware. > > @@ -282,16 +297,9 @@ static int mvebu_icu_probe(struct platform_device *pdev) > > regmap_write(icu->regmap, ICU_INT_CFG(i), 0); > > } > > > > - irq_domain = > > - platform_msi_create_device_domain(&pdev->dev, ICU_MAX_IRQS, > > - mvebu_icu_write_msg, > > - &mvebu_icu_domain_ops, icu); > > - if (!irq_domain) { > > - dev_err(&pdev->dev, "Failed to create ICU domain\n"); > > - return -ENOMEM; > > - } > > + platform_set_drvdata(pdev, icu); > > What is the upshot of passing this icu pointer as part of the platform > device structure instead of simply passing it as a parameter? Is > anything else using it? Not at this stage of the series, but right after the patch "irqchip/irq-mvebu-icu: support ICU subnodes" does the following: @@ -299,7 +356,10 @@ static int mvebu_icu_probe(struct platform_device *pdev) platform_set_drvdata(pdev, icu); - return mvebu_icu_subset_probe(pdev); + if (icu->legacy_bindings) + return mvebu_icu_subset_probe(pdev); + else + return devm_of_platform_populate(&pdev->dev); } Using driver data in both cases was, from my point of view, the easiest (meanwhile, I'll simplify it thanks to another comment you made). Thanks, Miquèl
diff --git a/drivers/irqchip/irq-mvebu-icu.c b/drivers/irqchip/irq-mvebu-icu.c index 607948870a14..24d45186eb6b 100644 --- a/drivers/irqchip/irq-mvebu-icu.c +++ b/drivers/irqchip/irq-mvebu-icu.c @@ -205,6 +205,37 @@ static const struct irq_domain_ops mvebu_icu_domain_ops = { .free = mvebu_icu_irq_domain_free, }; +static int mvebu_icu_subset_probe(struct platform_device *pdev) +{ + struct device_node *msi_parent_dn; + struct irq_domain *irq_domain; + struct mvebu_icu *icu; + + icu = dev_get_drvdata(&pdev->dev); + if (!icu) + return -ENODEV; + + pdev->dev.msi_domain = of_msi_get_domain(&pdev->dev, pdev->dev.of_node, + DOMAIN_BUS_PLATFORM_MSI); + if (!pdev->dev.msi_domain) + return -EPROBE_DEFER; + + msi_parent_dn = irq_domain_get_of_node(pdev->dev.msi_domain); + if (!msi_parent_dn) + return -ENODEV; + + irq_domain = platform_msi_create_device_domain(&pdev->dev, ICU_MAX_IRQS, + mvebu_icu_write_msg, + &mvebu_icu_domain_ops, + icu); + if (!irq_domain) { + dev_err(&pdev->dev, "Failed to create ICU MSI domain\n"); + return -ENOMEM; + } + + return 0; +} + static struct regmap_config mvebu_icu_regmap_config = { .reg_bits = 32, .val_bits = 32, @@ -215,9 +246,6 @@ static struct regmap_config mvebu_icu_regmap_config = { static int mvebu_icu_probe(struct platform_device *pdev) { struct mvebu_icu *icu; - struct device_node *node = pdev->dev.of_node; - struct device_node *gicp_dn; - struct irq_domain *irq_domain; struct resource *res; void __iomem *regs; int i; @@ -255,19 +283,6 @@ static int mvebu_icu_probe(struct platform_device *pdev) icu->irq_chip.irq_set_affinity = irq_chip_set_affinity_parent; #endif - /* - * We're probed after MSI domains have been resolved, so force - * resolution here. - */ - pdev->dev.msi_domain = of_msi_get_domain(&pdev->dev, node, - DOMAIN_BUS_PLATFORM_MSI); - if (!pdev->dev.msi_domain) - return -EPROBE_DEFER; - - gicp_dn = irq_domain_get_of_node(pdev->dev.msi_domain); - if (!gicp_dn) - return -ENODEV; - /* * Clean all ICU interrupts with type SPI_NSR, required to * avoid unpredictable SPI assignments done by firmware. @@ -282,16 +297,9 @@ static int mvebu_icu_probe(struct platform_device *pdev) regmap_write(icu->regmap, ICU_INT_CFG(i), 0); } - irq_domain = - platform_msi_create_device_domain(&pdev->dev, ICU_MAX_IRQS, - mvebu_icu_write_msg, - &mvebu_icu_domain_ops, icu); - if (!irq_domain) { - dev_err(&pdev->dev, "Failed to create ICU domain\n"); - return -ENOMEM; - } + platform_set_drvdata(pdev, icu); - return 0; + return mvebu_icu_subset_probe(pdev); } static const struct of_device_id mvebu_icu_of_match[] = {