All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Vaussard <florian.vaussard@epfl.ch>
To: Tony Lindgren <tony@atomide.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>,
	Benoit Cousson <b-cousson@ti.com>,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/5] ARM: dts: OMAP2+: create a DT header for GPIO
Date: Wed, 22 May 2013 18:00:32 +0200	[thread overview]
Message-ID: <519CEBA0.1080103@epfl.ch> (raw)
In-Reply-To: <20130522153452.GZ10378@atomide.com>

Hello Stephan, Tony,

Thank you for your reviews.

On 05/22/2013 05:34 PM, Tony Lindgren wrote:
> * Stephen Warren <swarren@wwwdotorg.org> [130522 08:32]:
>> On 05/22/2013 08:27 AM, Florian Vaussard wrote:
>>> Define the OMAP_GPIO macro to conveniently use GPIO inside OMAP DT.
>>> For example:
>>>
>>> 	gpios = <&gpio6 3 0>;  /* GPIO 163 */
>>>
>>> can be replaced by
>>>
>>> 	gpios = OMAP_GPIO(163, 0);
>>
>>> diff --git a/include/dt-bindings/gpio/omap-gpio.h b/include/dt-bindings/gpio/omap-gpio.h
>>
>>> +#define OMAP_GPIO_0_BANK        gpio1
>>> +#define OMAP_GPIO_1_BANK        gpio1
>>> +#define OMAP_GPIO_2_BANK        gpio1
>>> +#define OMAP_GPIO_3_BANK        gpio1
>>
>> There are a /lot/ of those. Is this really worth it?
>>
>> If the OMAP GPIO HW is already represented as a bunch of separate DT
>> nodes which represent separate GPIO blocks, then I would have thought
>> the syntax <&gpioN M 0> more directly represents what would be found in
>> the HW manual? If not, surely the DT should have a single node to
>> represent a single GPIO controller, which just happens to internally
>> support a bunch of register arrays.
>
> Yes I agree, let's not go back to numbering anything except within the
> a single instance. If anything, we can put the gpio number into comments.
>

 From a board point a view, I consider this macro as being easier to use,
than having to perform the necessary arithmetic to get the bank + offset
for each GPIO when converting existing boards or developing new ones.

But I also agree with you, and I was sad not to find a more elegant
way. Maybe someone with better preprocessor skills could come up with
a better solution?

Regards,

Florian

WARNING: multiple messages have this Message-ID (diff)
From: florian.vaussard@epfl.ch (Florian Vaussard)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/5] ARM: dts: OMAP2+: create a DT header for GPIO
Date: Wed, 22 May 2013 18:00:32 +0200	[thread overview]
Message-ID: <519CEBA0.1080103@epfl.ch> (raw)
In-Reply-To: <20130522153452.GZ10378@atomide.com>

Hello Stephan, Tony,

Thank you for your reviews.

On 05/22/2013 05:34 PM, Tony Lindgren wrote:
> * Stephen Warren <swarren@wwwdotorg.org> [130522 08:32]:
>> On 05/22/2013 08:27 AM, Florian Vaussard wrote:
>>> Define the OMAP_GPIO macro to conveniently use GPIO inside OMAP DT.
>>> For example:
>>>
>>> 	gpios = <&gpio6 3 0>;  /* GPIO 163 */
>>>
>>> can be replaced by
>>>
>>> 	gpios = OMAP_GPIO(163, 0);
>>
>>> diff --git a/include/dt-bindings/gpio/omap-gpio.h b/include/dt-bindings/gpio/omap-gpio.h
>>
>>> +#define OMAP_GPIO_0_BANK        gpio1
>>> +#define OMAP_GPIO_1_BANK        gpio1
>>> +#define OMAP_GPIO_2_BANK        gpio1
>>> +#define OMAP_GPIO_3_BANK        gpio1
>>
>> There are a /lot/ of those. Is this really worth it?
>>
>> If the OMAP GPIO HW is already represented as a bunch of separate DT
>> nodes which represent separate GPIO blocks, then I would have thought
>> the syntax <&gpioN M 0> more directly represents what would be found in
>> the HW manual? If not, surely the DT should have a single node to
>> represent a single GPIO controller, which just happens to internally
>> support a bunch of register arrays.
>
> Yes I agree, let's not go back to numbering anything except within the
> a single instance. If anything, we can put the gpio number into comments.
>

 From a board point a view, I consider this macro as being easier to use,
than having to perform the necessary arithmetic to get the bank + offset
for each GPIO when converting existing boards or developing new ones.

But I also agree with you, and I was sad not to find a more elegant
way. Maybe someone with better preprocessor skills could come up with
a better solution?

Regards,

Florian

  reply	other threads:[~2013-05-22 16:00 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-22 14:27 [PATCH 0/5] ARM: dts: OMAP2+: use preprocessor for device trees Florian Vaussard
2013-05-22 14:27 ` Florian Vaussard
2013-05-22 14:27 ` [PATCH 1/5] ARM: dts: OMAP2+: use #include for all " Florian Vaussard
2013-05-22 14:27   ` Florian Vaussard
2013-05-22 15:40   ` Tony Lindgren
2013-05-22 15:40     ` Tony Lindgren
2013-05-22 16:05     ` Florian Vaussard
2013-05-22 16:05       ` Florian Vaussard
2013-05-22 14:27 ` [PATCH 2/5] ARM: dts: OMAP2+: create a DT header for GPIO Florian Vaussard
2013-05-22 14:27   ` Florian Vaussard
2013-05-22 15:26   ` Stephen Warren
2013-05-22 15:26     ` Stephen Warren
2013-05-22 15:34     ` Tony Lindgren
2013-05-22 15:34       ` Tony Lindgren
2013-05-22 16:00       ` Florian Vaussard [this message]
2013-05-22 16:00         ` Florian Vaussard
2013-05-22 16:03         ` Stephen Warren
2013-05-22 16:03           ` Stephen Warren
2013-05-22 14:27 ` [PATCH 3/5] ARM: dts: OMAP2+: convert DT files to use the new OMAP_GPIO macro Florian Vaussard
2013-05-22 14:27   ` Florian Vaussard
2013-05-22 15:28   ` Stephen Warren
2013-05-22 15:28     ` Stephen Warren
2013-05-22 16:02     ` Florian Vaussard
2013-05-22 16:02       ` Florian Vaussard
2013-05-22 14:27 ` [PATCH 4/5] ARM: dts: OMAP3: fix incorrect notation for musb-hdrc interrupt Florian Vaussard
2013-05-22 14:27   ` Florian Vaussard
2013-05-22 15:35   ` Tony Lindgren
2013-05-22 15:35     ` Tony Lindgren
2013-05-22 14:27 ` [PATCH 5/5] ARM: dts: OMAP4/5: use existing constants for IRQs Florian Vaussard
2013-05-22 14:27   ` Florian Vaussard

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=519CEBA0.1080103@epfl.ch \
    --to=florian.vaussard@epfl.ch \
    --cc=b-cousson@ti.com \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=swarren@wwwdotorg.org \
    --cc=tony@atomide.com \
    /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.