All of lore.kernel.org
 help / color / mirror / Atom feed
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 09/14] pinctrl/abx500: use direct IRQ defines
Date: Thu, 07 Feb 2013 10:59:39 -0700	[thread overview]
Message-ID: <5113EB8B.4060002@wwwdotorg.org> (raw)
In-Reply-To: <CAF2Aj3jHe+mP0dU_BHThBV-va_iJLcFQedmWQOTaYafAZM1Yxg@mail.gmail.com>

On 02/07/2013 02:01 AM, Lee Jones wrote:
> I don't see myself on cc. Was that intentional?

The original patch was that way; I assume git send-email only CC'd you
on patches written by you.

> I quite like the idea of this.
> 
> Stephen,
> 
> It doesn't mean the other patch was wrong, it just transfers the math.

Ah, I see. The issue is that the code below clearly calculates the hwirq
differently, and it wasn't immediately obvious that this part of the
patch for example:

>  struct abx500_gpio_irq_cluster ab8500_gpio_irq_cluster[] = {
> -	GPIO_IRQ_CLUSTER(6,  13, 34),
> -	GPIO_IRQ_CLUSTER(24, 25, 24),
> -	GPIO_IRQ_CLUSTER(36, 41, 14),
> +	GPIO_IRQ_CLUSTER(6,  13, AB8500_INT_GPIO6R),
> +	GPIO_IRQ_CLUSTER(24, 25, AB8500_INT_GPIO24R),
> +	GPIO_IRQ_CLUSTER(36, 41, AB8500_INT_GPIO36R),
>  };

... actually changes the values in the table (AB8500_INT_GPIO6R is 40,
so when using that value, you need to subtract of the value 6 for the
base to get the original 34).

> I wouldn't squash it into mine. I like the transition and the
> possibility to revert it if there's been some mistake.
> 
> (not to say there is one, but just in case.)
> 
> Sent from my mobile Linux device.
> 
> On Feb 7, 2013 12:14 AM, "Stephen Warren" <swarren@wwwdotorg.org
> <mailto:swarren@wwwdotorg.org>> wrote:
> 
>     On 02/05/2013 12:48 PM, Linus Walleij wrote:
>     > From: Linus Walleij <linus.walleij@linaro.org
>     <mailto:linus.walleij@linaro.org>>
>     >
>     > Make it harder to do mistakes by introducing the actual
>     > defined ABx500 IRQ number into the IRQ cluster definitions.
>     > Deduct cluster offset from the GPIO offset to make each
>     > cluster coherent.
> 
>     Shouldn't this patch be squashed into the previous patch to avoid churn?
> 
>     >  static struct abx500_pinctrl_soc_data ab9540_soc = {
> 
>     > @@ -273,8 +273,7 @@ static int abx500_gpio_to_irq(struct gpio_chip
>     *chip, unsigned offset)
> 
>     > -                     hwirq = gpio + cluster->to_irq;
>     > -
>     > +                     hwirq = gpio - cluster->start + cluster->to_irq;
>     >                       return
>     irq_create_mapping(pct->parent->domain, hwirq);
> 
>     In particular, this change implies that the previous patch was simply
>     incorrect, although I haven't really thought about it in detail.
>     --
>     To unsubscribe from this list: send the line "unsubscribe
>     linux-kernel" in
>     the body of a message to majordomo at vger.kernel.org
>     <mailto:majordomo@vger.kernel.org>
>     More majordomo info at  http://vger.kernel.org/majordomo-info.html
>     Please read the FAQ at  http://www.tux.org/lkml/
> 

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren@wwwdotorg.org>
To: Lee Jones <lee.jones@linaro.org>
Cc: Anmar Oueja <anmar.oueja@linaro.org>,
	linux-arm-kernel@lists.infradead.org,
	Linus Walleij <linus.walleij@linaro.org>,
	Stephen Warren <swarren@nvidia.com>,
	Linus Walleij <linus.walleij@stericsson.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 09/14] pinctrl/abx500: use direct IRQ defines
Date: Thu, 07 Feb 2013 10:59:39 -0700	[thread overview]
Message-ID: <5113EB8B.4060002@wwwdotorg.org> (raw)
In-Reply-To: <CAF2Aj3jHe+mP0dU_BHThBV-va_iJLcFQedmWQOTaYafAZM1Yxg@mail.gmail.com>

On 02/07/2013 02:01 AM, Lee Jones wrote:
> I don't see myself on cc. Was that intentional?

The original patch was that way; I assume git send-email only CC'd you
on patches written by you.

> I quite like the idea of this.
> 
> Stephen,
> 
> It doesn't mean the other patch was wrong, it just transfers the math.

Ah, I see. The issue is that the code below clearly calculates the hwirq
differently, and it wasn't immediately obvious that this part of the
patch for example:

>  struct abx500_gpio_irq_cluster ab8500_gpio_irq_cluster[] = {
> -	GPIO_IRQ_CLUSTER(6,  13, 34),
> -	GPIO_IRQ_CLUSTER(24, 25, 24),
> -	GPIO_IRQ_CLUSTER(36, 41, 14),
> +	GPIO_IRQ_CLUSTER(6,  13, AB8500_INT_GPIO6R),
> +	GPIO_IRQ_CLUSTER(24, 25, AB8500_INT_GPIO24R),
> +	GPIO_IRQ_CLUSTER(36, 41, AB8500_INT_GPIO36R),
>  };

... actually changes the values in the table (AB8500_INT_GPIO6R is 40,
so when using that value, you need to subtract of the value 6 for the
base to get the original 34).

> I wouldn't squash it into mine. I like the transition and the
> possibility to revert it if there's been some mistake.
> 
> (not to say there is one, but just in case.)
> 
> Sent from my mobile Linux device.
> 
> On Feb 7, 2013 12:14 AM, "Stephen Warren" <swarren@wwwdotorg.org
> <mailto:swarren@wwwdotorg.org>> wrote:
> 
>     On 02/05/2013 12:48 PM, Linus Walleij wrote:
>     > From: Linus Walleij <linus.walleij@linaro.org
>     <mailto:linus.walleij@linaro.org>>
>     >
>     > Make it harder to do mistakes by introducing the actual
>     > defined ABx500 IRQ number into the IRQ cluster definitions.
>     > Deduct cluster offset from the GPIO offset to make each
>     > cluster coherent.
> 
>     Shouldn't this patch be squashed into the previous patch to avoid churn?
> 
>     >  static struct abx500_pinctrl_soc_data ab9540_soc = {
> 
>     > @@ -273,8 +273,7 @@ static int abx500_gpio_to_irq(struct gpio_chip
>     *chip, unsigned offset)
> 
>     > -                     hwirq = gpio + cluster->to_irq;
>     > -
>     > +                     hwirq = gpio - cluster->start + cluster->to_irq;
>     >                       return
>     irq_create_mapping(pct->parent->domain, hwirq);
> 
>     In particular, this change implies that the previous patch was simply
>     incorrect, although I haven't really thought about it in detail.
>     --
>     To unsubscribe from this list: send the line "unsubscribe
>     linux-kernel" in
>     the body of a message to majordomo@vger.kernel.org
>     <mailto:majordomo@vger.kernel.org>
>     More majordomo info at  http://vger.kernel.org/majordomo-info.html
>     Please read the FAQ at  http://www.tux.org/lkml/
> 


  parent reply	other threads:[~2013-02-07 17:59 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-05 19:48 [PATCH 00/14] reform the ABx500 IRQ handling Linus Walleij
2013-02-05 19:48 ` Linus Walleij
2013-02-05 19:48 ` [PATCH 01/14] mfd: ab8500: prepare to handle AB8500 GPIO's IRQs correctly Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-05 19:48 ` [PATCH 02/14] mfd: ab8500: provide a irq_set_type() function Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-07  0:07   ` Stephen Warren
2013-02-07  0:07     ` Stephen Warren
     [not found]     ` <CAF2Aj3iO+6j+T5C0X2Hh6TsvT4yaJ2cai6cC9DaiRAwL9n-dVw@mail.gmail.com>
2013-02-07 10:01       ` Linus Walleij
2013-02-07 10:01         ` Linus Walleij
2013-02-05 19:48 ` [PATCH 03/14] mfd: ab8500: ensure new AB8500 pinctrl driver is probed correctly Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-07  0:08   ` Stephen Warren
2013-02-07  0:08     ` Stephen Warren
2013-02-05 19:48 ` [PATCH 04/14] mfd: ab8500: allow AB9540 based devices to use ABX500 pinctrl Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-05 19:48 ` [PATCH 05/14] pinctrl/abx500: prevent error path from corrupting returning error Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-05 19:48 ` [PATCH 06/14] pinctrl/abx500: align GPIO cluster boundaries Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-05 19:48 ` [PATCH 07/14] pinctrl/abx500: move IRQ handling to ab8500-core Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-05 19:48 ` [PATCH 08/14] pinctrl/abx500: replace IRQ offsets with table read-in values Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-05 19:48 ` [PATCH 09/14] pinctrl/abx500: use direct IRQ defines Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-07  0:13   ` Stephen Warren
2013-02-07  0:13     ` Stephen Warren
     [not found]     ` <CAF2Aj3jHe+mP0dU_BHThBV-va_iJLcFQedmWQOTaYafAZM1Yxg@mail.gmail.com>
2013-02-07 17:59       ` Stephen Warren [this message]
2013-02-07 17:59         ` Stephen Warren
2013-02-08  8:25         ` Lee Jones
2013-02-08  8:25           ` Lee Jones
2013-02-08 17:06           ` Stephen Warren
2013-02-08 17:06             ` Stephen Warren
2013-02-07 19:10       ` Linus Walleij
2013-02-07 19:10         ` Linus Walleij
2013-02-05 19:48 ` [PATCH 10/14] pinctrl/abx500: add Device Tree support Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-05 19:48 ` [PATCH 11/14] ARM: ux500: remove irq_base property from platform_data Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-10  2:42   ` Olof Johansson
2013-02-10  2:42     ` Olof Johansson
2013-02-10 15:01     ` Linus Walleij
2013-02-10 15:01       ` Linus Walleij
2013-02-05 19:48 ` [PATCH 12/14] ARM: ux500: use real AB8500 IRQ numbers instead of virtual ones Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-05 19:48 ` [PATCH 13/14] ARM: ux500: enable AB8500 GPIO for HREF Linus Walleij
2013-02-05 19:48   ` Linus Walleij
2013-02-07  0:17   ` Stephen Warren
2013-02-07  0:17     ` Stephen Warren
2013-02-07  8:30     ` Arnd Bergmann
2013-02-07  8:30       ` Arnd Bergmann
2013-02-05 19:48 ` [PATCH 14/14] ARM: ux500: allow Snowball access to the AB8500 GPIO pins Linus Walleij
2013-02-05 19:48   ` Linus Walleij

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=5113EB8B.4060002@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --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 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.