Thread (20 messages) 20 messages, 5 authors, 2021-12-01

Re: [PATCH v2 2/2] irqchip: Add Qualcomm MPM controller driver

From: Shawn Guo <hidden>
Date: 2021-12-01 07:36:54
Also in: linux-arm-msm, lkml

On Tue, Nov 30, 2021 at 10:44:15AM +0000, Marc Zyngier wrote:
On Tue, 30 Nov 2021 09:17:08 +0000,
Shawn Guo [off-list ref] wrote:
quoted
On Tue, Nov 30, 2021 at 08:42:53AM +0000, Marc Zyngier wrote:
quoted
On Tue, 30 Nov 2021 02:31:52 +0000,
Shawn Guo [off-list ref] wrote:
quoted
+ Maulik

On Mon, Nov 29, 2021 at 03:24:39PM +0000, Marc Zyngier wrote:
[...]
quoted
quoted
quoted
quoted
@@ -430,6 +430,14 @@ config QCOM_PDC
 	  Power Domain Controller driver to manage and configure wakeup
 	  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
 
+config QCOM_MPM
+	bool "QCOM MPM"
Can't be built as a module?
The driver is implemented as a builtin_platform_driver().
This, on its own, shouldn't preclude the driver from being built as a
module. However, the config option only allows it to be built in. Why?
I just tried to build it as a module, and it seems that "irq_to_desc" is
only available for built-in build.
Yet another thing that you should not be using. The irqdomain code
gives you everything you need without having to resort to the
internals of the core IRQ infrastructure.
I see.  I should use irq_get_irq_data() rather than &desc->irq_data.
Even better:

	desc = irq_resolve_mapping(domain, hwirq);

Job done. No extra tracking, no dubious hack in the unmask callback,
works with modules.
quoted
quoted
quoted
quoted
Furthermore, why would you look up anywhere other than the wake-up
domain? My impression was that only these interrupts would require
being re-triggered.
Both domains have MPM pins that could wake up system.
Then why do you need two domains?
This is basically the same situation as qcom-pdc, and I have some
description about that in the commit log:

- For given SoC, a fixed number of MPM pins are supported, e.g. 96 pins
  on QCM2290.  Each of these MPM pins can be either a MPM_GIC pin or
  a MPM_GPIO pin. The mapping between MPM_GIC pin and GIC interrupt
  is defined by SoC, as well as the mapping between MPM_GPIO pin and
  GPIO number.  The former mapping can be found as the SoC data in this
  MPM driver, while the latter can be found as the msm_gpio_wakeirq_map[]
  in TLMM driver.

- Two irq domains are created for a single irq_chip to handle MPM_GIC
  and MPM_GPIO pins respectively, i.e. MPM_GIC domain and MPM_GPIO domain.
  The former is a child domain of GIC irq domain, while the latter is
  a parent domain of TLMM/GPIO irq domain.
That doesn't answer my question.

It doesn't matter what the pins are used for as long as you can
identify which ones are routed to the GIC and which are not. You are
obviously are able to do so, since you are able to disconnect part of
the hierarchy (why is qcom_mpm_gic_alloc() named as such, since most
of the interrupts it deals with are *never* routed to the GIC).

All the interrupts have the same irqchip callbacks and act on the same
'priv' data, so they it is obvious they don't overlap in the hwirq
space.

Ergo: you can implement the whole thing with a single domain. All you
need to make sure is that you identify the pins that are routed to the
GIC, and you already have that information.
You are right!  A single domain works.  Nice and clean!  Thanks for the
comment, Marc!

Shawn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help