linux-kernel - RE: [PATCH v3] soc: fsl: qe: convert QE interrupt controller to platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <AS8PR04MB894679107E1AE2A899D46FA88FE89@AS8PR04MB8946.eurprd04.prod.outlook.com>
Date:   Mon, 26 Jul 2021 20:44:58 +0000
From:   Leo Li <leoyang.li@....com>
To:     Maxim Kochetkov <fido_max@...ox.ru>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
CC:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "saravanak@...gle.com" <saravanak@...gle.com>,
        Qiang Zhao <qiang.zhao@....com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        kernel test robot <lkp@...el.com>,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: RE: [PATCH v3] soc: fsl: qe: convert QE interrupt controller to
 platform_device



> -----Original Message-----
> From: Maxim Kochetkov <fido_max@...ox.ru>
> Sent: Monday, July 26, 2021 12:22 AM
> To: linuxppc-dev@...ts.ozlabs.org
> Cc: linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org;
> saravanak@...gle.com; Leo Li <leoyang.li@....com>; Qiang Zhao
> <qiang.zhao@....com>; gregkh@...uxfoundation.org; Maxim Kochetkov
> <fido_max@...ox.ru>; kernel test robot <lkp@...el.com>; Dan Carpenter
> <dan.carpenter@...cle.com>
> Subject: [PATCH v3] soc: fsl: qe: convert QE interrupt controller to
> platform_device
> 
> Since 5.13 QE's ucc nodes can't get interrupts from devicetree:
> 
> 	ucc@...0 {
> 		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().
> Also use platform_get_ and devm_ family function to get/allocate resources
> and drop unused .compatible = "qeic".
> 
> [1] -
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k
> ernel.org%2Flkml%2FCAGETcx9PiX%3D%3DmLxB9PO8Myyk6u2vhPVwTMsA
> 5NkD-
> ywH5xhusw%40mail.gmail.com&amp;data=04%7C01%7Cleoyang.li%40nxp.co
> m%7C6e64e4b86f2d4a89390808d94ff50bec%7C686ea1d3bc2b4c6fa92cd99c5c
> 301635%7C0%7C0%7C637628736153046082%7CUnknown%7CTWFpbGZsb3d8
> eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7C1000&amp;sdata=G7HhGFmRLvMyNMULSddWctD3HhtVWMfZAxPXjl8
> CBTY%3D&amp;reserved=0
> 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@...ox.ru>
> Reported-by: kernel test robot <lkp@...el.com>
> Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
> ---
> Changes in v3:
>  - use .compatible = "qeic" again (Li Yang <leoyang.li@....com> asks to keep
> it)
> 
> Changes in v2:
>  - use devm_ family functions to allocate mem/resources
>  - use platform_get_ family functions to get resources/irqs
>  - drop unused .compatible = "qeic"
> 
>  drivers/soc/fsl/qe/qe_ic.c | 75 ++++++++++++++++++++++----------------
>  1 file changed, 44 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c index
> 3f711c1a0996..54cabd2605dd 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,41 +405,40 @@ 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)
>  {
> +	struct device *dev = &pdev->dev;
>  	void (*low_handler)(struct irq_desc *desc);
>  	void (*high_handler)(struct irq_desc *desc);
>  	struct qe_ic *qe_ic;
> -	struct resource res;
> -	u32 ret;
> +	struct resource *res;
> +	struct device_node *node = pdev->dev.of_node;
> 
> -	ret = of_address_to_resource(node, 0, &res);
> -	if (ret)
> -		return;
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (res == NULL) {
> +		dev_err(dev, "no memory resource defined\n");
> +		return -ENODEV;
> +	}
> 
> -	qe_ic = kzalloc(sizeof(*qe_ic), GFP_KERNEL);
> +	qe_ic = devm_kzalloc(dev, 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;
> +	qe_ic->regs = devm_ioremap(dev, res->start, resource_size(res));
> +	if (qe_ic->regs == NULL) {
> +		dev_err(dev, "failed to ioremap() registers\n");
> +		return -ENODEV;
>  	}
> 
> -	qe_ic->regs = ioremap(res.start, resource_size(&res));
> -
>  	qe_ic->hc_irq = qe_ic_irq_chip;
> 
> -	qe_ic->virq_high = irq_of_parse_and_map(node, 0);
> -	qe_ic->virq_low = irq_of_parse_and_map(node, 1);
> +	qe_ic->virq_high = platform_get_irq(pdev, 0);
> +	qe_ic->virq_low = platform_get_irq(pdev, 1);
> 
> -	if (!qe_ic->virq_low) {
> -		printk(KERN_ERR "Failed to map QE_IC low IRQ\n");
> -		kfree(qe_ic);
> -		return;
> +	if (qe_ic->virq_low < 0) {
> +		return -ENODEV;
>  	}
> +
>  	if (qe_ic->virq_high != qe_ic->virq_low) {
>  		low_handler = qe_ic_cascade_low;
>  		high_handler = qe_ic_cascade_high;
> @@ -447,6 +447,13 @@ static void __init qe_ic_init(struct device_node
> *node)
>  		high_handler = NULL;
>  	}
> 
> +	qe_ic->irqhost = irq_domain_add_linear(node, NR_QE_IC_INTS,
> +					       &qe_ic_host_ops, qe_ic);
> +	if (qe_ic->irqhost == NULL) {
> +		dev_err(dev, "failed to add irq domain\n");
> +		return -ENODEV;
> +	}
> +
>  	qe_ic_write(qe_ic->regs, QEIC_CICR, 0);
> 
>  	irq_set_handler_data(qe_ic->virq_low, qe_ic); @@ -456,20 +463,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"},

Probably you missed the point from my first response.  The original code is match by device type "qeic" instead of compatible string "qeic".   So we should use { .type = "qeic"} here.

> +	{},
> +};
> 
> -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