From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4C95C43381 for ; Mon, 25 Mar 2019 09:24:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 918BA2087E for ; Mon, 25 Mar 2019 09:24:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730332AbfCYJYM (ORCPT ); Mon, 25 Mar 2019 05:24:12 -0400 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:43118 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730096AbfCYJYL (ORCPT ); Mon, 25 Mar 2019 05:24:11 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1071A15AB; Mon, 25 Mar 2019 02:24:11 -0700 (PDT) Received: from big-swifty.misterjones.org (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2A01B3F614; Mon, 25 Mar 2019 02:24:08 -0700 (PDT) Date: Mon, 25 Mar 2019 09:24:06 +0000 Message-ID: <864l7rs3wp.wl-marc.zyngier@arm.com> From: Marc Zyngier To: Qiang Zhao Cc: "tglx@linutronix.de" , "jason@lakedaemon.net" , "linux-kernel@vger.kernel.org" Subject: Re: [patch v14 3/4] irqchip/qeic: merge qeic_of_init into qe_ic_init In-Reply-To: <20190325035418.19065-4-qiang.zhao@nxp.com> References: <20190325035418.19065-1-qiang.zhao@nxp.com> <20190325035418.19065-4-qiang.zhao@nxp.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 EasyPG/1.0.0 Emacs/26 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Organization: ARM Ltd MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 25 Mar 2019 03:53:41 +0000, Qiang Zhao wrote: > > qeic_of_init just get device_node of qeic from dtb and call qe_ic_init, > pass the device_node to qe_ic_init. > So merge qeic_of_init into qe_ic_init to get the qeic node in > qe_ic_init. > > Signed-off-by: Zhao Qiang > --- > drivers/irqchip/irq-qeic.c | 110 ++++++++++++-------------------------------- > include/soc/fsl/qe/qe_ic.h | 7 --- > 2 files changed, 30 insertions(+), 87 deletions(-) > > diff --git a/drivers/irqchip/irq-qeic.c b/drivers/irqchip/irq-qeic.c > index 673afb4..a6ccbfb 100644 > --- a/drivers/irqchip/irq-qeic.c > +++ b/drivers/irqchip/irq-qeic.c > @@ -407,27 +407,30 @@ unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic) > return irq_linear_revmap(qe_ic->irqhost, irq); > } > > -void __init qe_ic_init(struct device_node *node, unsigned int flags, > - void (*low_handler)(struct irq_desc *desc), > - void (*high_handler)(struct irq_desc *desc)) > +static int __init qe_ic_init(struct device_node *node, > + struct device_node *parent) > { > struct qe_ic *qe_ic; > struct resource res; > - u32 temp = 0, ret, high_active = 0; > + int ret = 0; > > ret = of_address_to_resource(node, 0, &res); > - if (ret) > - return; > + if (ret) { > + ret = -ENODEV; > + goto err_put_node; > + } > > qe_ic = kzalloc(sizeof(*qe_ic), GFP_KERNEL); > - if (qe_ic == NULL) > - return; > + if (qe_ic == NULL) { > + ret = -ENOMEM; > + goto err_put_node; > + } > > 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; > + ret = -ENOMEM; > + goto err_free_qe_ic; > } > > qe_ic->regs = ioremap(res.start, resource_size(&res)); > @@ -438,42 +441,30 @@ void __init qe_ic_init(struct device_node *node, unsigned int flags, > qe_ic->virq_low = irq_of_parse_and_map(node, 1); > > if (qe_ic->virq_low == NO_IRQ) { > - printk(KERN_ERR "Failed to map QE_IC low IRQ\n"); > - kfree(qe_ic); > - return; > - } > - > - /* default priority scheme is grouped. If spread mode is */ > - /* required, configure cicr accordingly. */ > - if (flags & QE_IC_SPREADMODE_GRP_W) > - temp |= CICR_GWCC; > - if (flags & QE_IC_SPREADMODE_GRP_X) > - temp |= CICR_GXCC; > - if (flags & QE_IC_SPREADMODE_GRP_Y) > - temp |= CICR_GYCC; > - if (flags & QE_IC_SPREADMODE_GRP_Z) > - temp |= CICR_GZCC; > - if (flags & QE_IC_SPREADMODE_GRP_RISCA) > - temp |= CICR_GRTA; > - if (flags & QE_IC_SPREADMODE_GRP_RISCB) > - temp |= CICR_GRTB; > - > - /* choose destination signal for highest priority interrupt */ > - if (flags & QE_IC_HIGH_SIGNAL) { > - temp |= (SIGNAL_HIGH << CICR_HPIT_SHIFT); > - high_active = 1; > + pr_err("Failed to map QE_IC low IRQ\n"); > + ret = -ENOMEM; > + goto err_domain_remove; None of this change is documented in the commit log. Why? How can you afford to get rid of it without breaking anything? > } > > - qe_ic_write(qe_ic->regs, QEIC_CICR, temp); > - > irq_set_handler_data(qe_ic->virq_low, qe_ic); > - irq_set_chained_handler(qe_ic->virq_low, low_handler); > + irq_set_chained_handler(qe_ic->virq_low, qe_ic_cascade_low_mpic); > > if (qe_ic->virq_high != NO_IRQ && > qe_ic->virq_high != qe_ic->virq_low) { > irq_set_handler_data(qe_ic->virq_high, qe_ic); > - irq_set_chained_handler(qe_ic->virq_high, high_handler); > + irq_set_chained_handler(qe_ic->virq_high, > + qe_ic_cascade_high_mpic); > } > + of_node_put(node); > + return 0; > + > +err_domain_remove: > + irq_domain_remove(qe_ic->irqhost); > +err_free_qe_ic: > + kfree(qe_ic); > +err_put_node: > + of_node_put(node); > + return ret; > } > > void qe_ic_set_highest_priority(unsigned int virq, int high) > @@ -570,45 +561,4 @@ int qe_ic_set_high_priority(unsigned int virq, unsigned int priority, int high) > return 0; > } > > -static struct bus_type qe_ic_subsys = { > - .name = "qe_ic", > - .dev_name = "qe_ic", > -}; > - > -static struct device device_qe_ic = { > - .id = 0, > - .bus = &qe_ic_subsys, > -}; > - > -static int __init init_qe_ic_sysfs(void) > -{ > - int rc; > - > - printk(KERN_DEBUG "Registering qe_ic with sysfs...\n"); > - > - rc = subsys_system_register(&qe_ic_subsys, NULL); > - if (rc) { > - printk(KERN_ERR "Failed registering qe_ic sys class\n"); > - return -ENODEV; > - } > - rc = device_register(&device_qe_ic); > - if (rc) { > - printk(KERN_ERR "Failed registering qe_ic sys device\n"); > - return -ENODEV; > - } > - return 0; > -} You're removing this sysfs stuff. Granted, it is completely pointless. Are you sure no userspace ever relied on this thing? In any case, this needs documenting. > - > -static int __init qeic_of_init(struct device_node *node, > - struct device_node *parent) > -{ > - if (!node) > - return -ENODEV; > - qe_ic_init(node, 0, qe_ic_cascade_low_mpic, > - qe_ic_cascade_high_mpic); > - of_node_put(node); > - return 0; > -} > - > -IRQCHIP_DECLARE(qeic, "fsl,qe-ic", qeic_of_init); > -subsys_initcall(init_qe_ic_sysfs); > +IRQCHIP_DECLARE(qeic, "fsl,qe-ic", qe_ic_init); > diff --git a/include/soc/fsl/qe/qe_ic.h b/include/soc/fsl/qe/qe_ic.h > index 1e155ca..6113699 100644 > --- a/include/soc/fsl/qe/qe_ic.h > +++ b/include/soc/fsl/qe/qe_ic.h > @@ -58,16 +58,9 @@ enum qe_ic_grp_id { > }; > > #ifdef CONFIG_QUICC_ENGINE > -void qe_ic_init(struct device_node *node, unsigned int flags, > - void (*low_handler)(struct irq_desc *desc), > - void (*high_handler)(struct irq_desc *desc)); > unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic); > unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic); > #else > -static inline void qe_ic_init(struct device_node *node, unsigned int flags, > - void (*low_handler)(struct irq_desc *desc), > - void (*high_handler)(struct irq_desc *desc)) > -{} > static inline unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic) > { return 0; } > static inline unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic) > -- > 1.7.1 > Thanks, M. -- Jazz is not dead, it just smell funny.