| Message ID | 20210705111250.1513634-1-fido_max@inbox.ru (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | soc: fsl: qe: convert QE interrupt controller to platform_device | expand |
| Related | show |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (858ea4d1f5c22aa2f8a6724c2a65458294f2ddc6) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 1 warnings, 0 checks, 83 lines checked |
| snowpatch_ozlabs/needsstable | warning | Please consider tagging this patch for stable! |
Hi Maxim,
url: https://github.com/0day-ci/linux/commits/Maxim-Kochetkov/soc-fsl-qe-convert-QE-interrupt-controller-to-platform_device/20210705-191227
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
config: openrisc-randconfig-m031-20210709 (attached as .config)
compiler: or1k-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/soc/fsl/qe/qe_ic.c:461 qe_ic_init() warn: 'qe_ic->regs' not released on lines: 442.
vim +461 drivers/soc/fsl/qe/qe_ic.c
43f09464f68dbb drivers/soc/fsl/qe/qe_ic.c Maxim Kochetkov 2021-07-05 408 static int qe_ic_init(struct platform_device *pdev)
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 409 {
4e0e161d3cc403 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 410 void (*low_handler)(struct irq_desc *desc);
4e0e161d3cc403 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 411 void (*high_handler)(struct irq_desc *desc);
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 412 struct qe_ic *qe_ic;
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 413 struct resource res;
43f09464f68dbb drivers/soc/fsl/qe/qe_ic.c Maxim Kochetkov 2021-07-05 414 struct device_node *node = pdev->dev.of_node;
882c626d1d4650 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 415 u32 ret;
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 416
2272a55f16c998 arch/powerpc/sysdev/qe_lib/qe_ic.c Michael Ellerman 2008-05-26 417 ret = of_address_to_resource(node, 0, &res);
2272a55f16c998 arch/powerpc/sysdev/qe_lib/qe_ic.c Michael Ellerman 2008-05-26 418 if (ret)
43f09464f68dbb drivers/soc/fsl/qe/qe_ic.c Maxim Kochetkov 2021-07-05 419 return -ENODEV;
2272a55f16c998 arch/powerpc/sysdev/qe_lib/qe_ic.c Michael Ellerman 2008-05-26 420
ea96025a26ab89 arch/powerpc/sysdev/qe_lib/qe_ic.c Anton Vorontsov 2009-07-01 421 qe_ic = kzalloc(sizeof(*qe_ic), GFP_KERNEL);
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 422 if (qe_ic == NULL)
43f09464f68dbb drivers/soc/fsl/qe/qe_ic.c Maxim Kochetkov 2021-07-05 423 return -ENOMEM;
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 424
a8db8cf0d894df arch/powerpc/sysdev/qe_lib/qe_ic.c Grant Likely 2012-02-14 425 qe_ic->irqhost = irq_domain_add_linear(node, NR_QE_IC_INTS,
a8db8cf0d894df arch/powerpc/sysdev/qe_lib/qe_ic.c Grant Likely 2012-02-14 426 &qe_ic_host_ops, qe_ic);
3475dd8a68a7c7 arch/powerpc/sysdev/qe_lib/qe_ic.c Julia Lawall 2009-08-01 427 if (qe_ic->irqhost == NULL) {
^^^^^^^^^^^^^^
Does this need to be cleaned up?
3475dd8a68a7c7 arch/powerpc/sysdev/qe_lib/qe_ic.c Julia Lawall 2009-08-01 428 kfree(qe_ic);
43f09464f68dbb drivers/soc/fsl/qe/qe_ic.c Maxim Kochetkov 2021-07-05 429 return -ENODEV;
3475dd8a68a7c7 arch/powerpc/sysdev/qe_lib/qe_ic.c Julia Lawall 2009-08-01 430 }
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 431
28f65c11f2ffb3 arch/powerpc/sysdev/qe_lib/qe_ic.c Joe Perches 2011-06-09 432 qe_ic->regs = ioremap(res.start, resource_size(&res));
^^^^^^^^^^^^^^^^^^^^^
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 433
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 434 qe_ic->hc_irq = qe_ic_irq_chip;
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 435
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 436 qe_ic->virq_high = irq_of_parse_and_map(node, 0);
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 437 qe_ic->virq_low = irq_of_parse_and_map(node, 1);
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 438
10d7930dbb51a8 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 439 if (!qe_ic->virq_low) {
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 440 printk(KERN_ERR "Failed to map QE_IC low IRQ\n");
3475dd8a68a7c7 arch/powerpc/sysdev/qe_lib/qe_ic.c Julia Lawall 2009-08-01 441 kfree(qe_ic);
43f09464f68dbb drivers/soc/fsl/qe/qe_ic.c Maxim Kochetkov 2021-07-05 442 return -ENODEV;
Call iounmap() before returning?
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 443 }
4e0e161d3cc403 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 444 if (qe_ic->virq_high != qe_ic->virq_low) {
523eef1d206a67 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 445 low_handler = qe_ic_cascade_low;
523eef1d206a67 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 446 high_handler = qe_ic_cascade_high;
4e0e161d3cc403 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 447 } else {
4e0e161d3cc403 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 448 low_handler = qe_ic_cascade_muxed_mpic;
4e0e161d3cc403 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 449 high_handler = NULL;
4e0e161d3cc403 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 450 }
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 451
882c626d1d4650 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 452 qe_ic_write(qe_ic->regs, QEIC_CICR, 0);
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 453
ec775d0e70eb6b arch/powerpc/sysdev/qe_lib/qe_ic.c Thomas Gleixner 2011-03-25 454 irq_set_handler_data(qe_ic->virq_low, qe_ic);
ec775d0e70eb6b arch/powerpc/sysdev/qe_lib/qe_ic.c Thomas Gleixner 2011-03-25 455 irq_set_chained_handler(qe_ic->virq_low, low_handler);
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 456
10d7930dbb51a8 drivers/soc/fsl/qe/qe_ic.c Rasmus Villemoes 2019-11-28 457 if (qe_ic->virq_high && qe_ic->virq_high != qe_ic->virq_low) {
ec775d0e70eb6b arch/powerpc/sysdev/qe_lib/qe_ic.c Thomas Gleixner 2011-03-25 458 irq_set_handler_data(qe_ic->virq_high, qe_ic);
ec775d0e70eb6b arch/powerpc/sysdev/qe_lib/qe_ic.c Thomas Gleixner 2011-03-25 459 irq_set_chained_handler(qe_ic->virq_high, high_handler);
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 460 }
43f09464f68dbb drivers/soc/fsl/qe/qe_ic.c Maxim Kochetkov 2021-07-05 @461 return 0;
9865853851313e arch/powerpc/sysdev/qe_lib/qe_ic.c Li Yang 2006-10-03 462 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Mon, Jul 5, 2021 at 6:12 AM Maxim Kochetkov <fido_max@inbox.ru> wrote: > > Since 5.13 QE's ucc nodes can't get interrupts from devicetree: > > ucc@2000 { > cell-index = <1>; > reg = <0x2000 0x200>; > interrupts = <32>; > interrupt-parent = <&qeic>; > }; > > Now fw_devlink expects driver to create and probe a struct device > for interrupt controller. > > So lets convert this driver to simple platform_device with probe(). > > [1] - https://lore.kernel.org/lkml/CAGETcx9PiX==mLxB9PO8Myyk6u2vhPVwTMsA5NkD-ywH5xhusw@mail.gmail.com > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > Fixes: ea718c699055 ("Revert "Revert "driver core: Set fw_devlink=on by default""") > Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru> > --- > drivers/soc/fsl/qe/qe_ic.c | 38 +++++++++++++++++++++++--------------- > 1 file changed, 23 insertions(+), 15 deletions(-) > > diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c > index 3f711c1a0996..03d291376895 100644 > --- a/drivers/soc/fsl/qe/qe_ic.c > +++ b/drivers/soc/fsl/qe/qe_ic.c > @@ -23,6 +23,7 @@ > #include <linux/signal.h> > #include <linux/device.h> > #include <linux/spinlock.h> > +#include <linux/platform_device.h> > #include <asm/irq.h> > #include <asm/io.h> > #include <soc/fsl/qe/qe.h> > @@ -404,27 +405,28 @@ static void qe_ic_cascade_muxed_mpic(struct irq_desc *desc) > chip->irq_eoi(&desc->irq_data); > } > > -static void __init qe_ic_init(struct device_node *node) > +static int qe_ic_init(struct platform_device *pdev) > { > void (*low_handler)(struct irq_desc *desc); > void (*high_handler)(struct irq_desc *desc); > struct qe_ic *qe_ic; > struct resource res; > + struct device_node *node = pdev->dev.of_node; > u32 ret; > > ret = of_address_to_resource(node, 0, &res); > if (ret) > - return; > + return -ENODEV; > > qe_ic = kzalloc(sizeof(*qe_ic), GFP_KERNEL); > if (qe_ic == NULL) > - return; > + return -ENOMEM; > > qe_ic->irqhost = irq_domain_add_linear(node, NR_QE_IC_INTS, > &qe_ic_host_ops, qe_ic); > if (qe_ic->irqhost == NULL) { > kfree(qe_ic); > - return; > + return -ENODEV; > } > > qe_ic->regs = ioremap(res.start, resource_size(&res)); > @@ -437,7 +439,7 @@ static void __init qe_ic_init(struct device_node *node) > if (!qe_ic->virq_low) { > printk(KERN_ERR "Failed to map QE_IC low IRQ\n"); > kfree(qe_ic); > - return; > + return -ENODEV; > } > if (qe_ic->virq_high != qe_ic->virq_low) { > low_handler = qe_ic_cascade_low; > @@ -456,20 +458,26 @@ static void __init qe_ic_init(struct device_node *node) > irq_set_handler_data(qe_ic->virq_high, qe_ic); > irq_set_chained_handler(qe_ic->virq_high, high_handler); > } > + return 0; > } > +static const struct of_device_id qe_ic_ids[] = { > + { .compatible = "fsl,qe-ic"}, > + { .compatible = "qeic"}, From the original code, this should be type = "qeic". It is not defined in current binding but probably needed for backward compatibility. It would be great if you can also deal with the comments from Dan too. Thanks. > + {}, > +}; > > -static int __init qe_ic_of_init(void) > +static struct platform_driver qe_ic_driver = > { > - struct device_node *np; > + .driver = { > + .name = "qe-ic", > + .of_match_table = qe_ic_ids, > + }, > + .probe = qe_ic_init, > +}; > > - np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic"); > - if (!np) { > - np = of_find_node_by_type(NULL, "qeic"); > - if (!np) > - return -ENODEV; > - } > - qe_ic_init(np); > - of_node_put(np); > +static int __init qe_ic_of_init(void) > +{ > + platform_driver_register(&qe_ic_driver); > return 0; > } > subsys_initcall(qe_ic_of_init); > -- > 2.31.1 >
15.07.2021 01:29, Li Yang wrote: > From the original code, this should be type = "qeic". It is not > defined in current binding but probably needed for backward > compatibility. I took these strings from this part: np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic"); if (!np) { np = of_find_node_by_type(NULL, "qeic"); if (!np) return -ENODEV; } However I can't find usage of "qeic" in any dts, so I will drop this in V2
On Mon, Jul 19, 2021 at 1:57 AM Maxim Kochetkov <fido_max@inbox.ru> wrote: > > 15.07.2021 01:29, Li Yang wrote: > > From the original code, this should be type = "qeic". It is not > > defined in current binding but probably needed for backward > > compatibility. > > I took these strings from this part: > > np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic"); > > if (!np) { > > np = of_find_node_by_type(NULL, "qeic"); > > if (!np) > > return -ENODEV; > > } > > However I can't find usage of "qeic" in any dts, so I will drop this in V2 It is really a bit hard to find as it is pretty old. But it was really used up until this commit below in 2008. So probably it will be better to keep it just for backward compatibility? commit a2dd70a11d4c9cb8a4e4bb41f53a9b430e08559b Author: Anton Vorontsov <avorontsov@ru.mvista.com> Date: Thu Jan 24 18:39:59 2008 +0300 [POWERPC] QE: get rid of most device_types and model Now we're searching for "fsl,qe", "fsl,qe-muram", "fsl,qe-muram-data" and "fsl,qe-ic". Unfortunately it's still impossible to remove device_type = "qe" from the existing device trees because older u-boots are looking for it. Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> Signed-off-by: Kumar Gala <galak@kernel.crashing.org> Regards, Leo
diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c index 3f711c1a0996..03d291376895 100644 --- a/drivers/soc/fsl/qe/qe_ic.c +++ b/drivers/soc/fsl/qe/qe_ic.c @@ -23,6 +23,7 @@ #include <linux/signal.h> #include <linux/device.h> #include <linux/spinlock.h> +#include <linux/platform_device.h> #include <asm/irq.h> #include <asm/io.h> #include <soc/fsl/qe/qe.h> @@ -404,27 +405,28 @@ static void qe_ic_cascade_muxed_mpic(struct irq_desc *desc) chip->irq_eoi(&desc->irq_data); } -static void __init qe_ic_init(struct device_node *node) +static int qe_ic_init(struct platform_device *pdev) { void (*low_handler)(struct irq_desc *desc); void (*high_handler)(struct irq_desc *desc); struct qe_ic *qe_ic; struct resource res; + struct device_node *node = pdev->dev.of_node; u32 ret; ret = of_address_to_resource(node, 0, &res); if (ret) - return; + return -ENODEV; qe_ic = kzalloc(sizeof(*qe_ic), GFP_KERNEL); if (qe_ic == NULL) - return; + return -ENOMEM; qe_ic->irqhost = irq_domain_add_linear(node, NR_QE_IC_INTS, &qe_ic_host_ops, qe_ic); if (qe_ic->irqhost == NULL) { kfree(qe_ic); - return; + return -ENODEV; } qe_ic->regs = ioremap(res.start, resource_size(&res)); @@ -437,7 +439,7 @@ static void __init qe_ic_init(struct device_node *node) if (!qe_ic->virq_low) { printk(KERN_ERR "Failed to map QE_IC low IRQ\n"); kfree(qe_ic); - return; + return -ENODEV; } if (qe_ic->virq_high != qe_ic->virq_low) { low_handler = qe_ic_cascade_low; @@ -456,20 +458,26 @@ static void __init qe_ic_init(struct device_node *node) irq_set_handler_data(qe_ic->virq_high, qe_ic); irq_set_chained_handler(qe_ic->virq_high, high_handler); } + return 0; } +static const struct of_device_id qe_ic_ids[] = { + { .compatible = "fsl,qe-ic"}, + { .compatible = "qeic"}, + {}, +}; -static int __init qe_ic_of_init(void) +static struct platform_driver qe_ic_driver = { - struct device_node *np; + .driver = { + .name = "qe-ic", + .of_match_table = qe_ic_ids, + }, + .probe = qe_ic_init, +}; - np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic"); - if (!np) { - np = of_find_node_by_type(NULL, "qeic"); - if (!np) - return -ENODEV; - } - qe_ic_init(np); - of_node_put(np); +static int __init qe_ic_of_init(void) +{ + platform_driver_register(&qe_ic_driver); return 0; } subsys_initcall(qe_ic_of_init);
Since 5.13 QE's ucc nodes can't get interrupts from devicetree: ucc@2000 { cell-index = <1>; reg = <0x2000 0x200>; interrupts = <32>; interrupt-parent = <&qeic>; }; Now fw_devlink expects driver to create and probe a struct device for interrupt controller. So lets convert this driver to simple platform_device with probe(). [1] - https://lore.kernel.org/lkml/CAGETcx9PiX==mLxB9PO8Myyk6u2vhPVwTMsA5NkD-ywH5xhusw@mail.gmail.com Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") Fixes: ea718c699055 ("Revert "Revert "driver core: Set fw_devlink=on by default""") Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru> --- drivers/soc/fsl/qe/qe_ic.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-)