linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 1/3] initialize each mbigen device node as a interrupt controller.
Date: Sat, 10 Oct 2015 11:09:42 +0100	[thread overview]
Message-ID: <20151010110942.52d34a4b@arm.com> (raw)
In-Reply-To: <5618D3EC.6050501@huawei.com>

On Sat, 10 Oct 2015 17:01:32 +0800
"majun (F)" <majun258@huawei.com> wrote:

> 
> 
> ? 2015/10/9 21:47, Thomas Gleixner ??:
> > On Sun, 4 Oct 2015, majun (F) wrote:
> >>>> +	info->reg_offset = get_mbigen_vec_reg_addr(info->nid, info->local_pin_offset);
> >>>
> >>> So you fill in a structure with 5 fields and the only information
> [...]
> >> On the other hand, it's a interrupt controller for the devices
> >> connected to it.(handled in current patch).
> >>
> >> To bind these two different irqs, I made a data sutruce named
> >> mbigen_irq_data which contains some information of this irq,
> >> including private index, pin_offset, nid, and local_pin_offset.
> >>
> >> All these information can help us to find the corresponding reg addr
> >> and msi_irq quickly.
> > 
> > This is completely wrong. Why would you need two linux virq numbers
> > for one interrupt?
> > 
> > This needs to be expressed in one hierarchy. mbigen is just a
> > translator between wired interrupts and MSI, nothing else.
> > 
> > So the hierarchy is:
> > 
> >   mbigen -> ITS-MSI -> ITS -> GIC
> 
> I think maybe you mean: mbigen -> ITS-pMSI -> ITS- > GIC
> 
> 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).

So we end-up with two virqs that need to be reconciled one way or
another. Is that an accurate description of your problem?

This is a silly situation, because there is no multiplexing at all
(there is exactly one wired interrupt per MSI). It is just that we
don't have the infrastructure to connect the two virtual interrupts.

Thomas, what would you think of extending the MSI layer a bit so that
instead of allocating the virqs dynamically, we could take an
optional array of virqs that would be used? It should be pretty easy to
hack this into msi_domain_alloc_irqs(), and modify the existing callers
(or have a separate entry point altogether). I'm still a bit fuzzy on
how we stitch the domains themselves (we need to set ->parent on
allocating the MSIs, which leaves it being a NULL pointer until then),
but I think there is a way. Something like:

int msi_domain_alloc_stitch_irqs(struct irq_domain *domain,
				 struct irq_domain *parent,
				 struct device *dev,
				 int *virqs, int nvecs);

Naming is crap, but you'll get the idea. Thoughts?

> If we want to use the hierarchy structure, I think
> 
> mbigen -> ITS -> GIC
> 
> maybe is a possible way .
> 
> The only problem is I need to do some change in ITS driver.
> 
> I mean move its_create_device() and its_find_device() into
>  its_irq_domain_alloc()
> 
> But this solution is similar to my v3 patch.

And that was really bad. Let's not go back there.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

  reply	other threads:[~2015-10-10 10:09 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 [this message]
2015-10-11  9:54             ` Thomas Gleixner
2015-10-11 11:03               ` Marc Zyngier
2015-10-11 16:45                 ` Thomas Gleixner
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=20151010110942.52d34a4b@arm.com \
    --to=marc.zyngier@arm.com \
    --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;
as well as URLs for NNTP newsgroup(s).