From: Bjorn Andersson <andersson@kernel.org>
To: Maulik Shah <maulik.shah@oss.qualcomm.com>
Cc: Linus Walleij <linusw@kernel.org>,
linux-arm-msm@vger.kernel.org, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] pinctrl: qcom: Replace open coded eoi call with irq_chip_eoi_parent()
Date: Mon, 18 May 2026 22:33:10 -0500 [thread overview]
Message-ID: <agvXluPCLIQlq267@baldur> (raw)
In-Reply-To: <agvW8hetKSsPAXgv@baldur>
On Mon, May 18, 2026 at 10:20:55PM -0500, Bjorn Andersson wrote:
> On Thu, May 14, 2026 at 02:08:25PM +0530, Maulik Shah wrote:
> > Replace open coded eoi call to parent irqchip with irq_chip_eoi_parent().
> >
> > No functional impact.
> >
>
> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>
On second though, I'm not sure I want to r-b this patch.
The commit message explains the action of the patch, not the reasons for
the patch. From the description I inferred that irq_chip_eoi_parent()
does implement what is open coded here, and a quick glance confirms
that.
I'm guessing that irq_chip_eoi_parent() didn't exist when
msm_gpio_irq_eoi() was written? Or was it not used for some reason?
> Regards,
> Bjorn
>
> > Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> > ---
> > drivers/pinctrl/qcom/pinctrl-msm.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> > index 45b3a2763eb8..6771f5eb29e4 100644
> > --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> > @@ -1012,10 +1012,8 @@ static void msm_gpio_irq_ack(struct irq_data *d)
> >
> > static void msm_gpio_irq_eoi(struct irq_data *d)
> > {
> > - d = d->parent_data;
> > -
> > - if (d)
> > - d->chip->irq_eoi(d);
> > + if (d->parent_data)
"I know that irq_chip_eoi_parent() will peak into d->parent_data, so
let's peek into the object first to avoid it dereferencing a NULL
pointer".
I see one other caller to irq_chip_eoi_parent() doing this, most
everyone else just register irq_chip_eoi_parent directly in the ops
struct.
Are we doing it right?
Regards,
Bjorn
> > + irq_chip_eoi_parent(d);
> > }
> >
> > static bool msm_gpio_needs_dual_edge_parent_workaround(struct irq_data *d,
> >
> > ---
> > base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
> > change-id: 20260514-pinctrl_msm_irq_eoi-ab736e16d411
> >
> > Best regards,
> > --
> > Maulik Shah <maulik.shah@oss.qualcomm.com>
> >
>
next prev parent reply other threads:[~2026-05-19 3:33 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-14 8:38 [PATCH] pinctrl: qcom: Replace open coded eoi call with irq_chip_eoi_parent() Maulik Shah
2026-05-19 3:20 ` Bjorn Andersson
2026-05-19 3:33 ` Bjorn Andersson [this message]
2026-05-19 4:10 ` Maulik Shah (mkshah)
2026-05-25 8:38 ` Linus Walleij
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=agvXluPCLIQlq267@baldur \
--to=andersson@kernel.org \
--cc=linusw@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maulik.shah@oss.qualcomm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.