All of lore.kernel.org
 help / color / mirror / Atom feed
From: Grant Likely <grant.likely@secretlab.ca>
To: Rob Herring <robherring2@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org,
	devicetree-discuss@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, marc.zyngier@arm.com,
	thomas.abraham@linaro.org, jamie@jamieiles.com, b-cousson@ti.com,
	shawn.guo@linaro.org, Rob Herring <rob.herring@calxeda.com>
Subject: Re: [PATCH 1/2] ARM: gic: add irq_domain support
Date: Thu, 29 Sep 2011 12:15:40 -0500	[thread overview]
Message-ID: <20110929171540.GC6800@ponder.secretlab.ca> (raw)
In-Reply-To: <1317268436-1613-2-git-send-email-robherring2@gmail.com>

On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Convert the gic interrupt controller to use irq domains in preparation
> for device-tree binding and MULTI_IRQ. This allows for translation between
> GIC interrupt IDs and Linux irq numbers.
> 
> The meaning of irq_offset has changed. It now is just the number of skipped
> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
> for secondary GICs.

[...]

>  /*
> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
>   */
>  static void gic_mask_irq(struct irq_data *d)
>  {
> -	u32 mask = 1 << (d->irq % 32);
> +	u32 mask = 1 << (gic_irq(d) % 32);

This can probably change simply to d->hwirq if irq_offset is
eliminated as I describe below.

>  void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
>  	void __iomem *dist_base, void __iomem *cpu_base)
>  {
>  	struct gic_chip_data *gic;
> +	struct irq_domain *domain;
> +	int gic_irqs;
>  
>  	BUG_ON(gic_nr >= MAX_GIC_NR);
>  
>  	gic = &gic_data[gic_nr];
> +	domain = &gic->domain;
>  	gic->dist_base = dist_base;
>  	gic->cpu_base = cpu_base;
> -	gic->irq_offset = (irq_start - 1) & ~31;
>  
> -	if (gic_nr == 0)
> +	/*
> +	 * For primary GICs, skip over SGIs.
> +	 * For secondary GICs, skip over PPIs, too.
> +	 */
> +	if (gic_nr == 0) {
>  		gic_cpu_base_addr = cpu_base;
> +		gic->irq_offset = 16;
> +		irq_start = (irq_start & ~31) + 16;

With the switch to irq_domain, there should no longer be any need for
a ~31 mask on the irq_start number.  Yes, you'll want to make sure
that it doesn't allocate below irq 16, but the driver should
completely use the irq_domain to manage the mapping from linux-irq
number to hwirq number.  The ~31 mask appears to have been an
optimization to quickly calculate hwirq number from the linux one, but
that value is now found in irq_data->hwirq.

irq_offset could almost be dropped entirely outside of probe because
it no longer matters.  Instead, populate the to_irq() hook so that the
hwirq value gets set correctly from the outset.

It would be better in general to have a consistent view of what the
hwirq number means regardless of if it is a root GIC.

g.

WARNING: multiple messages have this Message-ID (diff)
From: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
To: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 1/2] ARM: gic: add irq_domain support
Date: Thu, 29 Sep 2011 12:15:40 -0500	[thread overview]
Message-ID: <20110929171540.GC6800@ponder.secretlab.ca> (raw)
In-Reply-To: <1317268436-1613-2-git-send-email-robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
> Convert the gic interrupt controller to use irq domains in preparation
> for device-tree binding and MULTI_IRQ. This allows for translation between
> GIC interrupt IDs and Linux irq numbers.
> 
> The meaning of irq_offset has changed. It now is just the number of skipped
> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
> for secondary GICs.

[...]

>  /*
> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
>   */
>  static void gic_mask_irq(struct irq_data *d)
>  {
> -	u32 mask = 1 << (d->irq % 32);
> +	u32 mask = 1 << (gic_irq(d) % 32);

This can probably change simply to d->hwirq if irq_offset is
eliminated as I describe below.

>  void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
>  	void __iomem *dist_base, void __iomem *cpu_base)
>  {
>  	struct gic_chip_data *gic;
> +	struct irq_domain *domain;
> +	int gic_irqs;
>  
>  	BUG_ON(gic_nr >= MAX_GIC_NR);
>  
>  	gic = &gic_data[gic_nr];
> +	domain = &gic->domain;
>  	gic->dist_base = dist_base;
>  	gic->cpu_base = cpu_base;
> -	gic->irq_offset = (irq_start - 1) & ~31;
>  
> -	if (gic_nr == 0)
> +	/*
> +	 * For primary GICs, skip over SGIs.
> +	 * For secondary GICs, skip over PPIs, too.
> +	 */
> +	if (gic_nr == 0) {
>  		gic_cpu_base_addr = cpu_base;
> +		gic->irq_offset = 16;
> +		irq_start = (irq_start & ~31) + 16;

With the switch to irq_domain, there should no longer be any need for
a ~31 mask on the irq_start number.  Yes, you'll want to make sure
that it doesn't allocate below irq 16, but the driver should
completely use the irq_domain to manage the mapping from linux-irq
number to hwirq number.  The ~31 mask appears to have been an
optimization to quickly calculate hwirq number from the linux one, but
that value is now found in irq_data->hwirq.

irq_offset could almost be dropped entirely outside of probe because
it no longer matters.  Instead, populate the to_irq() hook so that the
hwirq value gets set correctly from the outset.

It would be better in general to have a consistent view of what the
hwirq number means regardless of if it is a root GIC.

g.

WARNING: multiple messages have this Message-ID (diff)
From: grant.likely@secretlab.ca (Grant Likely)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] ARM: gic: add irq_domain support
Date: Thu, 29 Sep 2011 12:15:40 -0500	[thread overview]
Message-ID: <20110929171540.GC6800@ponder.secretlab.ca> (raw)
In-Reply-To: <1317268436-1613-2-git-send-email-robherring2@gmail.com>

On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Convert the gic interrupt controller to use irq domains in preparation
> for device-tree binding and MULTI_IRQ. This allows for translation between
> GIC interrupt IDs and Linux irq numbers.
> 
> The meaning of irq_offset has changed. It now is just the number of skipped
> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
> for secondary GICs.

[...]

>  /*
> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
>   */
>  static void gic_mask_irq(struct irq_data *d)
>  {
> -	u32 mask = 1 << (d->irq % 32);
> +	u32 mask = 1 << (gic_irq(d) % 32);

This can probably change simply to d->hwirq if irq_offset is
eliminated as I describe below.

>  void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
>  	void __iomem *dist_base, void __iomem *cpu_base)
>  {
>  	struct gic_chip_data *gic;
> +	struct irq_domain *domain;
> +	int gic_irqs;
>  
>  	BUG_ON(gic_nr >= MAX_GIC_NR);
>  
>  	gic = &gic_data[gic_nr];
> +	domain = &gic->domain;
>  	gic->dist_base = dist_base;
>  	gic->cpu_base = cpu_base;
> -	gic->irq_offset = (irq_start - 1) & ~31;
>  
> -	if (gic_nr == 0)
> +	/*
> +	 * For primary GICs, skip over SGIs.
> +	 * For secondary GICs, skip over PPIs, too.
> +	 */
> +	if (gic_nr == 0) {
>  		gic_cpu_base_addr = cpu_base;
> +		gic->irq_offset = 16;
> +		irq_start = (irq_start & ~31) + 16;

With the switch to irq_domain, there should no longer be any need for
a ~31 mask on the irq_start number.  Yes, you'll want to make sure
that it doesn't allocate below irq 16, but the driver should
completely use the irq_domain to manage the mapping from linux-irq
number to hwirq number.  The ~31 mask appears to have been an
optimization to quickly calculate hwirq number from the linux one, but
that value is now found in irq_data->hwirq.

irq_offset could almost be dropped entirely outside of probe because
it no longer matters.  Instead, populate the to_irq() hook so that the
hwirq value gets set correctly from the outset.

It would be better in general to have a consistent view of what the
hwirq number means regardless of if it is a root GIC.

g.

  reply	other threads:[~2011-09-29 20:16 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-29  3:53 [PATCH 0/2] GIC OF bindings Rob Herring
2011-09-29  3:53 ` Rob Herring
2011-09-29  3:53 ` [PATCH 1/2] ARM: gic: add irq_domain support Rob Herring
2011-09-29  3:53   ` Rob Herring
2011-09-29  3:53   ` Rob Herring
2011-09-29 17:15   ` Grant Likely [this message]
2011-09-29 17:15     ` Grant Likely
2011-09-29 17:15     ` Grant Likely
2011-09-29 21:02     ` Rob Herring
2011-09-29 21:02       ` Rob Herring
2011-09-29 21:02       ` Rob Herring
2011-09-30  0:27       ` Grant Likely
2011-09-30  0:27         ` Grant Likely
2011-09-29  3:53 ` [PATCH 2/2] ARM: gic: add OF based initialization Rob Herring
2011-09-29  3:53   ` Rob Herring
2011-09-29  3:53   ` Rob Herring
2011-09-29 17:19   ` Grant Likely
2011-09-29 17:19     ` Grant Likely
2011-09-29  9:21 ` [PATCH 0/2] GIC OF bindings Jamie Iles
2011-09-29  9:21   ` Jamie Iles

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=20110929171540.GC6800@ponder.secretlab.ca \
    --to=grant.likely@secretlab.ca \
    --cc=b-cousson@ti.com \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=jamie@jamieiles.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=rob.herring@calxeda.com \
    --cc=robherring2@gmail.com \
    --cc=shawn.guo@linaro.org \
    --cc=thomas.abraham@linaro.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 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.