diff mbox series

soc: fsl: qe: convert QE interrupt controller to platform_device

Message ID 20210705111250.1513634-1-fido_max@inbox.ru (mailing list archive)
State New, archived
Headers show
Series soc: fsl: qe: convert QE interrupt controller to platform_device | expand

Commit Message

Maxim Kochetkov July 5, 2021, 11:12 a.m. UTC
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(-)

Comments

Dan Carpenter July 10, 2021, 2:55 p.m. UTC | #1
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
Leo Li July 14, 2021, 10:29 p.m. UTC | #2
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
>
Maxim Kochetkov July 19, 2021, 6:58 a.m. UTC | #3
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
Leo Li July 22, 2021, 7:37 p.m. UTC | #4
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 mbox series

Patch

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);