Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 12 May 2022 14:24:48 +0100
Message-ID: <87wneq6fz3.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc:     Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Linus Walleij <linus.walleij@linaro.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        Phil Edworthy <phil.edworthy@renesas.com>,
        Biju Das <biju.das.jz@bp.renesas.com>
Subject: Re: [PATCH v3 3/5] gpio: gpiolib: Allow free() callback to be overridden
In-Reply-To: <CA+V-a8tf1RmT-cX5y807rTAPES2NXLJHp=u1WUG11fLrtt-5Mg@mail.gmail.com>
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>
        <CA+V-a8tf1RmT-cX5y807rTAPES2NXLJHp=u1WUG11fLrtt-5Mg@mail.gmail.com>
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
Precedence: bulk

On Thu, 12 May 2022 13:48:53 +0100,
"Lad, Prabhakar" <prabhakar.csengg@gmail.com> wrote:
> 
> Hi Marc,
> 
> Thank you for the review.
> 
> On Thu, May 12, 2022 at 12:19 PM Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Wed, 11 May 2022 19:32:08 +0100,
> > Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> 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
if your commit message actually reflected this instead of using an
example that doesn't actually exist.

> 
> > 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).

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.

	M.

-- 
Without deviation from the norm, progress is not possible.