From: Marc Zyngier <maz@kernel.org>
To: Shawn Guo <shawn.guo@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Maulik Shah <quic_mkshah@quicinc.com>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Sudeep Holla <sudeep.holla@arm.com>,
Rob Herring <robh+dt@kernel.org>,
devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 2/2] irqchip: Add Qualcomm MPM controller driver
Date: Fri, 04 Mar 2022 15:24:43 +0000 [thread overview]
Message-ID: <87lexp211g.wl-maz@kernel.org> (raw)
In-Reply-To: <20220304082342.GO269879@dragon>
On Fri, 04 Mar 2022 08:23:42 +0000,
Shawn Guo <shawn.guo@linaro.org> wrote:
>
> On Fri, Mar 04, 2022 at 07:59:15AM +0000, Marc Zyngier wrote:
> > On Thu, 03 Mar 2022 04:02:29 +0000,
> > Shawn Guo <shawn.guo@linaro.org> wrote:
> > >
> > > On Wed, Mar 02, 2022 at 01:57:27PM +0000, Marc Zyngier wrote:
> > > > This code actually makes me ask more questions. Why is it programming
> > > > 2 'pins' for each IRQ?
> > >
> > > The mapping between MPM pin and GIC IRQ is not strictly 1-1. There are
> > > some rare case that up to 2 MPM pins map to a single GIC IRQ, for
> > > example the last two in QC2290 'qcom,mpm-pin-map' below.
> > >
> > > qcom,mpm-pin-map = <2 275>, /* tsens0_tsens_upper_lower_int */
> > > <5 296>, /* lpass_irq_out_sdc */
> > > <12 422>, /* b3_lfps_rxterm_irq */
> > > <24 79>, /* bi_px_lpi_1_aoss_mx */
> > > <86 183>, /* mpm_wake,spmi_m */
> > > <90 260>, /* eud_p0_dpse_int_mx */
> > > <91 260>; /* eud_p0_dmse_int_mx */
> > >
> > >
> > > The downstream uses a DT bindings that specifies GIC hwirq number in
> > > client device nodes. In that case, d->hwirq in the driver is GIC IRQ
> > > number, and the driver will need to query mapping table, find out the
> > > possible 2 MPM pins, and set them up.
> > >
> > > The patches I'm posting here use a different bindings that specifies MPM
> > > pin instead in client device nodes. Thus the driver can simply get the
> > > MPM pin from d->hwirq, so that the whole look-up procedure can be saved.
> >
> > It still remains that there is no 1:1 mapping between input and
> > output, which is the rule #1 to be able to use a hierarchical setup.
>
> For direction of MPM pin -> GIC interrupt, it's a 1:1 mapping, i.e. for
> given MPM pin, there is only one GIC interrupt. And that's the
> mapping MPM driver relies on. For GIC interrupt -> MPM pin, it's not
> a strict 1:1 mapping.
Then this isn't a 1:1 mapping *AT ALL*. The hierarchical setup
mandates that the mapping is a bijective function, and that's exactly
what 1:1 means. There is no such thing a 1:1 in a single
direction. When you take an interrupt, all you see is the GIC
interrupt. How do you know which of the *two* pins interrupted you? Oh
wait, you *can't* know. You end-up never servicing one of the two
interrupts (and I suspect this results in memory corruption if you
tear a hierarchy down).
This HW deals with 2:1 mappings, i.e. it is a mux. So all the time
spent on this driver is totally lost, because you have the wrong
abstraction. And the QC driver is just as bad.
M.
--
Without deviation from the norm, progress is not possible.
next prev parent reply other threads:[~2022-03-04 15:24 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-01 6:24 [PATCH v7 0/2] Add Qualcomm MPM irqchip driver support Shawn Guo
2022-03-01 6:24 ` [PATCH v7 1/2] dt-bindings: interrupt-controller: Add Qualcomm MPM support Shawn Guo
2022-03-01 6:24 ` [PATCH v7 2/2] irqchip: Add Qualcomm MPM controller driver Shawn Guo
2022-03-01 11:13 ` Marc Zyngier
2022-03-02 8:40 ` Shawn Guo
2022-03-02 10:25 ` Marc Zyngier
2022-03-02 13:34 ` Shawn Guo
2022-03-02 13:57 ` Marc Zyngier
2022-03-03 4:02 ` Shawn Guo
2022-03-04 7:59 ` Marc Zyngier
2022-03-04 8:23 ` Shawn Guo
2022-03-04 15:24 ` Marc Zyngier [this message]
2022-03-05 9:24 ` Shawn Guo
2022-03-05 11:05 ` Marc Zyngier
2022-03-06 12:57 ` Shawn Guo
2022-03-07 11:45 ` Marc Zyngier
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=87lexp211g.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=quic_mkshah@quicinc.com \
--cc=robh+dt@kernel.org \
--cc=shawn.guo@linaro.org \
--cc=sudeep.holla@arm.com \
--cc=tglx@linutronix.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).