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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C5EC4C433F5 for ; Thu, 12 May 2022 07:06:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1350690AbiELHG2 (ORCPT ); Thu, 12 May 2022 03:06:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50740 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1350774AbiELHGI (ORCPT ); Thu, 12 May 2022 03:06:08 -0400 Received: from mail-qk1-f179.google.com (mail-qk1-f179.google.com [209.85.222.179]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5BDD5CD3; Thu, 12 May 2022 00:06:06 -0700 (PDT) Received: by mail-qk1-f179.google.com with SMTP id a22so3997804qkl.5; Thu, 12 May 2022 00:06:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uZq5KQ7PbYNEAlZ3QkxMc8y4Xac8EM18MZB7pmG8rGs=; b=RFH58T1tiKFNLyf3PNjwmqmRMNNDbE29N9RsKv+FdUpQ8FI3FrnOpL54mLNufp3RnL 2LtQGFk3TxfNngX+6HdQgvK48s4Jc7+cIywUFsWjQ3us/31C5rPAckmGAoRBXzq4EPj8 hZjPnWHZtxxVwOALDBWZCpSweyWOc9BM8N/j+M10hmzFPBaEv87xkNojrQY+JEf4pGK/ nrJ0PFFw3e8hxsIEx0eJSODsp/L7LHmEmClUdZHSfran9APtfFP0srEnrF67AVc3FBwQ 9yJSuyWdND/pICOafB3x5/rCSR9tIUzmgfgkz4PvjTPbIJZxbIOV3dQiBusQZTK7l0MG F80w== X-Gm-Message-State: AOAM530SrcF7JDabLi4P4mOuFGi6VSiQPTF9tfUMh9KACWqRmB8bKRz4 DA3VEmaC+WP5eXIQ7oyaWYC/ghsrkinSCA== X-Google-Smtp-Source: ABdhPJxHdA9T+GeoSG62uIbqlG++dUzTUGAvmmXXRTValPzVDa67XHHYCGDnhfNw9L6rUMkY/5Chkw== X-Received: by 2002:a37:bbc4:0:b0:69b:db2c:c962 with SMTP id l187-20020a37bbc4000000b0069bdb2cc962mr21881103qkf.565.1652339165226; Thu, 12 May 2022 00:06:05 -0700 (PDT) Received: from mail-yw1-f170.google.com (mail-yw1-f170.google.com. [209.85.128.170]) by smtp.gmail.com with ESMTPSA id x4-20020a05620a0b4400b0069fcebf9c0asm2453284qkg.37.2022.05.12.00.06.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 12 May 2022 00:06:04 -0700 (PDT) Received: by mail-yw1-f170.google.com with SMTP id 00721157ae682-2f7bb893309so45452707b3.12; Thu, 12 May 2022 00:06:03 -0700 (PDT) X-Received: by 2002:a81:2143:0:b0:2fb:1274:247e with SMTP id h64-20020a812143000000b002fb1274247emr29285779ywh.384.1652339163593; Thu, 12 May 2022 00:06:03 -0700 (PDT) MIME-Version: 1.0 References: <20220511183210.5248-1-prabhakar.mahadev-lad.rj@bp.renesas.com> <20220511183210.5248-3-prabhakar.mahadev-lad.rj@bp.renesas.com> In-Reply-To: <20220511183210.5248-3-prabhakar.mahadev-lad.rj@bp.renesas.com> From: Geert Uytterhoeven Date: Thu, 12 May 2022 09:05:51 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver To: Lad Prabhakar Cc: Linus Walleij , Thomas Gleixner , Marc Zyngier , Rob Herring , Krzysztof Kozlowski , Bartosz Golaszewski , Philipp Zabel , "open list:GPIO SUBSYSTEM" , Linux Kernel Mailing List , Linux-Renesas , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Phil Edworthy , Biju Das , Prabhakar Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org Hi Prabhakar, On Wed, May 11, 2022 at 8:32 PM Lad Prabhakar wrote: > Add a driver for the Renesas RZ/G2L Interrupt Controller. > > This supports external pins being used as interrupts. It supports > one line for NMI, 8 external pins and 32 GPIO pins (out of 123) > to be used as IRQ lines. > > Signed-off-by: Lad Prabhakar LGTM ("I'm no irqchip expert"), so Reviewed-by: Geert Uytterhoeven One suggestion for improvement below, which needs changes to the irqchip core code, though. > --- /dev/null > +++ b/drivers/irqchip/irq-renesas-rzg2l.c > +static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent) > +{ > + struct irq_domain *irq_domain, *parent_domain; > + struct reset_control *resetn; > + struct rzg2l_irqc_priv *priv; > + struct clk *clk; > + struct clk *pclk; > + int ret; > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->base = of_iomap(node, 0); > + if (!priv->base) { > + ret = -ENXIO; > + goto free_priv; > + } > + > + clk = of_clk_get_by_name(node, "clk"); > + if (IS_ERR(clk)) { > + ret = IS_ERR(clk); > + goto iounmap_base; > + } > + > + pclk = of_clk_get_by_name(node, "pclk"); > + if (IS_ERR(pclk)) { > + ret = IS_ERR(pclk); > + goto iounmap_base; > + } > + > + resetn = of_reset_control_get_exclusive_by_index(node, 0); > + if (IS_ERR(resetn)) { > + ret = IS_ERR(resetn); > + goto iounmap_base; > + } > + > + parent_domain = irq_find_host(parent); > + if (!parent_domain) { > + pr_err("%pOF: cannot find parent domain\n", node); > + ret = -ENODEV; > + goto iounmap_base; > + } > + > + ret = rzg2l_irqc_parse_map(priv, node); > + if (ret) { > + pr_err("%pOF: cannot parse interrupts: %d\n", node, ret); > + goto iounmap_base; > + } > + > + ret = reset_control_deassert(resetn); > + if (ret) { > + pr_err("%pOF: failed to deassert resetn pin, %d\n", node, ret); > + goto iounmap_base; > + } > + > + raw_spin_lock_init(&priv->lock); > + > + ret = clk_prepare_enable(clk); > + if (ret) > + goto assert_reset; > + > + ret = clk_prepare_enable(pclk); > + if (ret) > + goto disable_clk; As the driver doesn't care about the properties of the clock, but just needs to enable them, I think it would be worthwhile to replace the explicit clock handling by Runtime PM. But that needs access to the struct device pointer. Of course that can be done later (see below). > + > + irq_domain = irq_domain_add_hierarchy(parent_domain, 0, IRQC_NUM_IRQ, > + node, &rzg2l_irqc_domain_ops, > + priv); > + if (!irq_domain) { > + pr_err("%pOF: cannot initialize irq domain\n", node); > + ret = -ENOMEM; > + goto fail_irq_domain; > + } > + > + return 0; > + > +fail_irq_domain: > + clk_disable_unprepare(pclk); > +disable_clk: > + clk_disable_unprepare(clk); > +assert_reset: > + reset_control_assert(resetn); > +iounmap_base: > + iounmap(priv->base); > +free_priv: > + kfree(priv); > + return ret; > +} > + > +IRQCHIP_PLATFORM_DRIVER_BEGIN(rzg2l_irqc) > +IRQCHIP_MATCH("renesas,rzg2l-irqc", rzg2l_irqc_init) > +IRQCHIP_PLATFORM_DRIVER_END(rzg2l_irqc) While platform_irqchip_probe() has access to the platform_device, it does not pass that to the driver's initialization function, so that function does not have access to the device framework. Hence it can not be used for interrupt controllers that are part of a Power Domain, as, unlike clocks, Power Domains can only be controlled from Runtime PM, and thus require access to a struct device pointer. I think it would be good to fix this sooner rather than later; there are only a handful of users of this interface yet. > +MODULE_AUTHOR("Lad Prabhakar "); > +MODULE_DESCRIPTION("Renesas RZ/G2L IRQC Driver"); > +MODULE_LICENSE("GPL"); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds