public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: tglx@linutronix.de (Thomas Gleixner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 1/3] initialize each mbigen device node as a interrupt controller.
Date: Sun, 11 Oct 2015 18:45:48 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.11.1510111335150.6097@nanos> (raw)
In-Reply-To: <20151011120331.09093ffc@arm.com>

On Sun, 11 Oct 2015, Marc Zyngier wrote:
> On Sun, 11 Oct 2015 11:54:49 +0200
> Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > On Sat, 10 Oct 2015, Marc Zyngier wrote:
> > > On Sat, 10 Oct 2015 17:01:32 +0800
> > > "majun (F)" <majun258@huawei.com> wrote:
> > > > But there is a problem If i make the structure like you said.
> > > > 
> > > > For example, my hardware structure likes below:
> > > > 
> > > > uart ------> mbigen --> ITS-pMSI --> ITS --> GIC
> > > >      virq1
> > > > 
> > > > virq1 means the virq number allocted by irq_of_parse_and_map() function
> > > > when system parse the uart dts node in initializing  stage.
> > > > 
> > > > To create a ITS device, I need to call msi_domain_alloc_irqs() function
> > > > in my mbigen alloc function.
> > > > 
> > > > In this function, a new virq number(named as virq2 ) which different from
> > > > virq1 is allocated.
> > > > So, this is a big problem.
> > > 
> > > I think I see what your problem is:
> > > - The wired interrupt (uart -> mbigen) is allocated through DT (and
> > >   must be available early, because of of_platform_populate),
> > > - The MSI (mgigen -> ITS) is dynamic (and allocated much later,
> > >   because the device model kicks in after irqchip init, and we cannot
> > >   allocate MSIs without a device).
> > 
> > Why do we need that wired interrupt at all? 
> > 
> > We can make mbigen the 'msi-parent' of the device and let the
> > msi_domain_ops::msi_prepare() callback figure out the actual wiring
> > through device->fwnode.
> 
> That's because the device behind the mbigen can't do any MSI at all.
> Think of a 8250 uart, for example.
> 
> If we make the mbigen the msi-parent of the uart, then we need to teach
> the 8250 driver to request MSIs.

I really do not understand why of_platform_populate cares about
interrupt allocations. That's outright stupid. We should care about
that at device probe time, i.e. at the point where the driver is
registered and probed if there is matching platform data. If we do it
in of_platform_populate then we allocate interrupts even for devices
which do not have a driver, i.e. we just waste memory.

So we better teach a couple of drivers to handle that instead of
inventing horrible workarounds.

> It also means that the DT doesn't represent the HW anymore (this
> wired interrupt actually exists).

I think the abstraction here is wrong. If it would be correct, then
PCI-MSI would be wrong. The MSI part of PCI is a MSI producer, mbigen
is as well. Technically MSI is not integral part of the PCI device, it
just happens to have it's configuration registers in the PCI
configuration space of the device:

    [PCI-BUS]------[Interrupt mode selector]
    	      	|        |
    	      	|        |
		------[Legacy irq gate]<-----
		|        |                  |
		|        |                  |---[Device interrupt]
		|        |                  |
		------[MSI unit]<------------

So you have a 'wire' from the device to the MSI unit, but we do not
care about that 'wire'. All we care about are the MSI configuration
registers. We find them via the PCI device which gives us the address
of the PCI configuration space.

So now in the mbigen case this looks like this:

    [MSI-BUS] ----- [MBIGEN]<-------------------[Device interrupt]

Again, you have a 'wire' from the device to the MSI unit (MBIGEN) and
we do not care about that 'wire' either. What we care about is how we
find the MSI (mbigen) configuration registers for a particular
device. So we need a DT/ACPI entry which describes those configuration
registers and whatever supplementary information is required. That
will make the mbigen driver extremly simple.

Thanks,

	tglx

  reply	other threads:[~2015-10-11 16:45 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-30  9:39 [PATCH v5 0/3] Support Mbigen interrupt controller MaJun
2015-09-30  9:39 ` [PATCH v5 1/3] initialize each mbigen device node as a " MaJun
2015-09-30 21:37   ` Thomas Gleixner
2015-10-04  7:22     ` majun (F)
2015-10-09 13:47       ` Thomas Gleixner
2015-10-10  9:01         ` majun (F)
2015-10-10 10:09           ` Marc Zyngier
2015-10-11  9:54             ` Thomas Gleixner
2015-10-11 11:03               ` Marc Zyngier
2015-10-11 16:45                 ` Thomas Gleixner [this message]
2015-10-13  6:32                   ` majun (F)
2015-10-13  6:55                     ` Thomas Gleixner
2015-10-14  8:16                       ` majun (F)
2015-10-14  8:20                         ` Thomas Gleixner
2015-10-14  8:54                           ` majun (F)
2015-10-14  8:55                   ` Marc Zyngier
2015-10-14  9:17                     ` Thomas Gleixner
2015-10-14  9:49                       ` Marc Zyngier
2015-09-30  9:39 ` [PATCH v5 2/3] Probe mbigen chip and initial mbigen device as platform device MaJun
2015-09-30  9:39 ` [PATCH v5 3/3] dt-binding:Documents of the mbigen bindings MaJun

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=alpine.DEB.2.11.1510111335150.6097@nanos \
    --to=tglx@linutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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