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 225F6C433FE for ; Thu, 12 May 2022 22:24:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240450AbiELWYd (ORCPT ); Thu, 12 May 2022 18:24:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39182 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236178AbiELWYd (ORCPT ); Thu, 12 May 2022 18:24:33 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A5040281347; Thu, 12 May 2022 15:24:31 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 37A5B61F33; Thu, 12 May 2022 22:24:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8351BC385B8; Thu, 12 May 2022 22:24:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1652394270; bh=Lc1EANfP1MQNRs942EGNTUmsGp9phgY1vwN9up7zOUg=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=HxCPOXn5RDIRhIhlPJoSYXFerF91jClgOchhrxJEx6Kbl9hrWzRz5BsaNkSy8gKvY QljXFwyWRICSG6VUm6ymyGO9L17dOZF9h4JTehA4q9AKLKhyY6xUi2lsff280lhl/z 836UWmqVTrzexW58AWWlP8ZpnknmG7Oi6JLISQxTXsbw5kJWQlxjeT0dTHbQqqr7+b 7g2hFBIJWFu9QcgCBmVkThpoWZ4+0gFsJ5MSXTwkuyPqzPFaT2WE9lBT3hwAoHpkG+ UbTTfkX9uVnKCBjfA7ZE7eh6DqnF+40doDQSmV24YJS/wLJ9E4B95m/ltFQUKA2BHf z8jpBQgux/cUw== Received: from ip-185-104-136-29.ptr.icomera.net ([185.104.136.29] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1npHER-00AzZF-VL; Thu, 12 May 2022 23:24:28 +0100 Date: Thu, 12 May 2022 23:24:25 +0100 Message-ID: <87sfpemlt2.wl-maz@kernel.org> From: Marc Zyngier To: "Lad, Prabhakar" Cc: Lad Prabhakar , Geert Uytterhoeven , Linus Walleij , Thomas Gleixner , Rob Herring , Krzysztof Kozlowski , Bartosz Golaszewski , Philipp Zabel , "open list:GPIO SUBSYSTEM" , LKML , Linux-Renesas , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Phil Edworthy , Biju Das Subject: Re: [PATCH v3 3/5] gpio: gpiolib: Allow free() callback to be overridden In-Reply-To: References: <20220511183210.5248-1-prabhakar.mahadev-lad.rj@bp.renesas.com> <20220511183210.5248-4-prabhakar.mahadev-lad.rj@bp.renesas.com> <87y1z75770.wl-maz@kernel.org> <87wneq6fz3.wl-maz@kernel.org> <87v8ua67kt.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.104.136.29 X-SA-Exim-Rcpt-To: prabhakar.csengg@gmail.com, prabhakar.mahadev-lad.rj@bp.renesas.com, geert+renesas@glider.be, linus.walleij@linaro.org, tglx@linutronix.de, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, brgl@bgdev.pl, p.zabel@pengutronix.de, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, devicetree@vger.kernel.org, phil.edworthy@renesas.com, biju.das.jz@bp.renesas.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org On Thu, 12 May 2022 18:55:38 +0100, "Lad, Prabhakar" wrote: > > Hi Marc, > > On Thu, May 12, 2022 at 5:26 PM Marc Zyngier wrote: > > > > On Thu, 12 May 2022 14:50:05 +0100, > > "Lad, Prabhakar" wrote: > > > > > > Hi Marc, > > > > > > On Thu, May 12, 2022 at 2:24 PM Marc Zyngier wrote: > > > > > > > > On Thu, 12 May 2022 13:48:53 +0100, > > > > "Lad, Prabhakar" wrote: > > > > > > > > > > Hi Marc, > > > > > > > > > > Thank you for the review. > > > > > > > > > > On Thu, May 12, 2022 at 12:19 PM Marc Zyngier wrote: > > > > > > > > > > > > On Wed, 11 May 2022 19:32:08 +0100, > > > > > > Lad Prabhakar wrote: > > > > > > > > > > > > > > Allow free() callback to be overridden from irq_domain_ops for > > > > > > > hierarchical chips. > > > > > > > > > > > > > > This allows drivers to free any resources which are allocated during > > > > > > > populate_parent_alloc_arg(). > > > > > > > > > > > > Do you mean more than the fwspec? I don't see this being used. > > > > > > > > > > > The free callback is used in patch 5/5 where free is overridden by > > > > > rzg2l_gpio_irq_domain_free. I just gave an example there as an > > > > > populate_parent_alloc_arg() In actual in the child_to_parent_hwirq > > > > > callback I am using a bitmap [0] to get a free tint slot, this bitmap > > > > > needs freeing up when the GPIO interrupt is released from the driver > > > > > that as when overridden free callback frees the allocated tint slot so > > > > > that its available for re-use. > > > > > > > > Right, so that's actually a different life-cycle, and the whole > > > > populate_parent_alloc_arg() is a red herring. What you want is to free > > > > resources that have been allocated via some other paths. It'd be good > > > Is there any other path which I have missed where I can free up resources? > > > > No, that's the only one. It is just that usually, the alloc() > > callback is where you are supposed to perform... allocations. > > > OK. > > > It'd be good if you could move your allocation there, as I would > > expect calls to child_to_parent_hwirq() to be idempotent. > > > For now I'll go with the current implementation, as currently a an > array is maintained which is tied with the tint slot and child (which > is obtained from child_to_parent_hwirq) > > > > > > > > if your commit message actually reflected this instead of using an > > > > example that doesn't actually exist. > > > > > > > My bad, I will update the commit message. > > > > > > > > > > > > > > There is also the question of why we need to have dynamic allocation > > > > > > for the fwspec itself. Why isn't that a simple stack allocation in the > > > > > > context of gpiochip_hierarchy_irq_domain_alloc()? > > > > > > > > > > > you mean gpio core itself should handle the fwspec > > > > > allocation/freeing? > > > > > > > > Yes. The only reason we resort to dynamic allocation is because > > > > ThunderX is using MSI-based GPIOs, and thus doesn't use a fwspec (no > > > > firmware is involved here). > > > > > > > I see.. > > > > > > > If we had a union of the two types, we could just have a stack > > > > variable, and pass that along, completely sidestepping the whole > > > > dynamic allocation/freeing business. > > > > > > > Right agreed. > > > > FWIW, I've just posted a PoC patch[1]. > > > I guess I'll have to rebase my changes on top of it now ;) Not yet. Let's see what people say about it. M. -- Without deviation from the norm, progress is not possible.