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:   Tue, 12 Jul 2022 18:14:45 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To:     Marc Zyngier <maz@kernel.org>
Cc:     Robert Marko <robimarko@gmail.com>, bjorn.andersson@linaro.org,
        agross@kernel.org, linus.walleij@linaro.org,
        linux-arm-msm@vger.kernel.org, linux-gpio@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] pinctrl: qcom: spmi-gpio: make the irqchip immutable
Message-ID: <20220712124445.GC21746@workstation>
References: <20220624195112.894916-1-robimarko@gmail.com>
 <87edyq1ujr.wl-maz@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87edyq1ujr.wl-maz@kernel.org>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Tue, Jul 12, 2022 at 11:42:32AM +0100, Marc Zyngier wrote:
> On Fri, 24 Jun 2022 20:51:12 +0100,
> Robert Marko <robimarko@gmail.com> wrote:
> > 
> > Commit 6c846d026d49 ("gpio: Don't fiddle with irqchips marked as
> > immutable") added a warning to indicate if the gpiolib is altering the
> > internals of irqchips.
> > 
> > Following this change the following warning is now observed for the SPMI
> > PMIC pinctrl driver:
> > gpio gpiochip1: (200f000.spmi:pmic@0:gpio@c000): not an immutable chip, please consider fixing it!
> > 
> > Fix this by making the irqchip in the SPMI PMIC pinctrl driver immutable.
> > 
> > Signed-off-by: Robert Marko <robimarko@gmail.com>
> > ---
> >  drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 22 ++++++++++++----------
> >  1 file changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> > index c3255b0bece4..406ee0933d0b 100644
> > --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> > +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> > @@ -171,7 +171,6 @@ struct pmic_gpio_state {
> >  	struct regmap	*map;
> >  	struct pinctrl_dev *ctrl;
> >  	struct gpio_chip chip;
> > -	struct irq_chip irq;
> >  	u8 usid;
> >  	u8 pid_base;
> >  };
> > @@ -988,6 +987,17 @@ static void *pmic_gpio_populate_parent_fwspec(struct gpio_chip *chip,
> >  	return fwspec;
> >  }
> >  
> > +static const struct irq_chip spmi_gpio_irq_chip = {
> > +	.name		= "spmi-gpio",
> > +	.irq_ack	= irq_chip_ack_parent,
> > +	.irq_mask	= irq_chip_mask_parent,
> > +	.irq_unmask	= irq_chip_unmask_parent,
> 
> No, this is wrong. Please look at the documentation to see how you
> must now directly call into the gpiolib helpers for these two
> callbacks.
> 

IIUC, you are referring to gpiochip_disable_irq() and
gpiochip_enable_irq() APIs. These APIs are supposed to let the gpiolib
know about that the IRQ usage of these GPIOs. But for the case of hierarchial
IRQ domain, isn't the parent is going to do that?

Please correct me if I'm wrong.

Thanks,
Mani

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