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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D4EC2C27C52 for ; Wed, 5 Jun 2024 20:14:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:Date:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=y5bmEmqMKVrl+nCu8T8sCopUkHKVwjtzkQaKLwrJzA8=; b=IuOoN8oz+46amj vYTv/6yru56KnLfROi68NFgja7KpR9j7cLz86SOvn7Hy4Qen3WLr4C3/4urhMvnt1oWKFdW6kku/Y +nH+l86nvOnQ3HZ6SX7dUDHYP98lUPdOQaMLxE9O6huh+dD+6iRZxm8q2CQ8DphtlNury0s9dAiqX slAotcFaKHoowuIRac01y/khnH9ve3TIq6m/WvYL4nS7S5q1kvlCr+iC77yh+jMOiWb3ArGH90sei S8uE292Mj/4//3DIJYoSm76J9vBfo37fsUfWLbEdDo6fWG8NfQp//6U8HRhiV8UVGBwv9bRFbEGDs g5pi1WUxvXbqbnX0cb6A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sEx1f-00000007QHT-1zUY; Wed, 05 Jun 2024 20:14:27 +0000 Received: from fgw20-7.mail.saunalahti.fi ([62.142.5.81]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sEx1c-00000007QGZ-1Rke for linux-arm-kernel@lists.infradead.org; Wed, 05 Jun 2024 20:14:25 +0000 Received: from localhost (88-113-26-230.elisa-laajakaista.fi [88.113.26.230]) by fgw23.mail.saunalahti.fi (Halon) with ESMTP id 31968b91-2378-11ef-80de-005056bdfda7; Wed, 05 Jun 2024 23:14:20 +0300 (EEST) From: Andy Shevchenko Date: Wed, 5 Jun 2024 23:14:20 +0300 To: Thomas Gleixner Cc: Herve Codina , Simon Horman , Sai Krishna Gajula , Rob Herring , Krzysztof Kozlowski , Conor Dooley , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Lee Jones , Arnd Bergmann , Horatiu Vultur , UNGLinuxDriver@microchip.com, Andrew Lunn , Heiner Kallweit , Russell King , Saravana Kannan , Bjorn Helgaas , Philipp Zabel , Lars Povlsen , Steen Hegelund , Daniel Machon , Alexandre Belloni , linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, netdev@vger.kernel.org, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Allan Nielsen , Luca Ceresoli , Thomas Petazzoni Subject: Re: [PATCH v2 11/19] irqchip: Add support for LAN966x OIC Message-ID: References: <20240527161450.326615-1-herve.codina@bootlin.com> <20240527161450.326615-12-herve.codina@bootlin.com> <87frtr4goe.ffs@tglx> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87frtr4goe.ffs@tglx> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240605_131424_560439_D3D5FE64 X-CRM114-Status: GOOD ( 19.82 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Wed, Jun 05, 2024 at 04:17:53PM +0200, Thomas Gleixner kirjoitti: > On Mon, May 27 2024 at 18:14, Herve Codina wrote: ... > > + irq_reg_writel(gc, ~0, gc->chip_types[0].regs.disable); > > ~0U > > > + irq_reg_writel(gc, ~0, gc->chip_types[0].regs.ack); ... Below just to annoy people a bit :) (Yes, I understand that this is a prototype, it's just a pre-review in case one want to blindly copy'n'paste it). Other than that, I like the result! > I just did a quick conversion to the template approach. Unsurprisingly > it removes 30 lines of boiler plate code: > > +static void lan966x_oic_chip_init(struct irq_chip_generic *gc) > +{ > + struct lan966x_oic_data *lan966x_oic = gc->domain->host_data; > + struct lan966x_oic_chip_regs *chip_regs; > + > + gc->reg_base = lan966x_oic->regs; > + > + chip_regs = lan966x_oic_chip_regs + gc->irq_base / 32; > + gc->chip_types[0].regs.enable = chip_regs->reg_off_ena_set; > + gc->chip_types[0].regs.disable = chip_regs->reg_off_ena_clr; > + gc->chip_types[0].regs.ack = chip_regs->reg_off_sticky; > + > + gc->chip_types[0].chip.irq_startup = lan966x_oic_irq_startup; > + gc->chip_types[0].chip.irq_shutdown = lan966x_oic_irq_shutdown; > + gc->chip_types[0].chip.irq_set_type = lan966x_oic_irq_set_type; > + gc->chip_types[0].chip.irq_mask = irq_gc_mask_disable_reg; > + gc->chip_types[0].chip.irq_unmask = irq_gc_unmask_enable_reg; > + gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit; > + gc->private = chip_regs; > + > + /* Disable all interrupts handled by this chip */ > + irq_reg_writel(gc, ~0, chip_regs->reg_off_ena_clr); > +} > + > +static void lan966x_oic_chip_exit(struct irq_chip_generic *gc) > +{ > + /* Disable and ack all interrupts handled by this chip */ > + irq_reg_writel(gc, ~0, gc->chip_types[0].regs.disable); > + irq_reg_writel(gc, ~0, gc->chip_types[0].regs.ack); ~0U :-) But I, for example, think that GENMASK() even better as it shows exactly what bits we set for the HW writes. > +} > + > +static void lan966x_oic_domain_init(struct irq_domain *d) > +{ > + struct lan966x_oic_data *lan966x_oic = d->host_data; > + > + irq_set_chained_handler_and_data(lan966x_oic->irq, lan966x_oic_irq_handler, d); > +} > + > +static int lan966x_oic_probe(struct platform_device *pdev) > +{ > + struct irq_domain_chip_generic_info gc_info = { > + .irqs_per_chip = 32, > + .num_chips = 1, > + .name = "lan966x-oic" > + .handler = handle_level_irq, > + .init = lan966x_oic_chip_init, > + .destroy = lan966x_oic_chip_exit, > + }; > + > + struct irq_domain_info info = { > + .fwnode = of_node_to_fwnode(pdev->dev.of_node), It's as simple as dev_fwnode() > + .size = LAN966X_OIC_NR_IRQ, > + .hwirq_max = LAN966X_OIC_NR_IRQ, > + .ops = &irq_generic_chip_ops, > + .gc_info = &gc_info, > + .init = lan966x_oic_domain_init, > + }; > + struct lan966x_oic_data *lan966x_oic; > + struct device *dev = &pdev->dev; > + > + lan966x_oic = devm_kmalloc(dev, sizeof(*lan966x_oic), GFP_KERNEL); > + if (!lan966x_oic) > + return -ENOMEM; > + > + lan966x_oic->regs = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(lan966x_oic->regs)) > + return dev_err_probe(dev, PTR_ERR(lan966x_oic->regs), "failed to map resource\n"); > + > + lan966x_oic->irq = platform_get_irq(pdev, 0); > + if (lan966x_oic->irq < 0) > + return dev_err_probe(dev, lan966x_oic->irq, "failed to get the IRQ\n"); > + > + lan966x_oic->domain = irq_domain_instantiate(&info); > + if (!lan966x_oic->domain) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, lan966x_oic); > + return 0; > +} > + > +static void lan966x_oic_remove(struct platform_device *pdev) > +{ > + struct lan966x_oic_data *lan966x_oic = platform_get_drvdata(pdev); > + > + irq_set_chained_handler_and_data(lan966x_oic->irq, NULL, NULL); > + irq_domain_remove(lan966x_oic->domain); > +} -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel