diff mbox

ARM: dts: AM33XX: fix gpio node numbering to match hardware

Message ID 1347294020-22661-1-git-send-email-mporter@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matt Porter Sept. 10, 2012, 4:20 p.m. UTC
On AM33xx, the datasheet and TRM refer to four GPIO instances that
are 0-based, GPIO0-3.

Signed-off-by: Matt Porter <mporter@ti.com>
---
 arch/arm/boot/dts/am33xx.dtsi |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Benoit Cousson Sept. 10, 2012, 4:34 p.m. UTC | #1
+ Tony

Hi Matt,

30 minutes too late for my pull request :-(

There are a couple of am33xx patches under discussion, so I'll take them
and send a for_3.7/dts-part2 pull request if this is not too late for Tony.

On 09/10/2012 06:20 PM, Matt Porter wrote:
> On AM33xx, the datasheet and TRM refer to four GPIO instances that
> are 0-based, GPIO0-3.

Or maybe you should just update the spec to use a 1-based GPIO number
like OMAP :-)

Regards,
Benoit


> 
> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
>  arch/arm/boot/dts/am33xx.dtsi |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index bb31bff..1369bfc 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -62,7 +62,7 @@
>  			reg = <0x48200000 0x1000>;
>  		};
>  
> -		gpio1: gpio@44e07000 {
> +		gpio0: gpio@44e07000 {
>  			compatible = "ti,omap4-gpio";
>  			ti,hwmods = "gpio1";
>  			gpio-controller;
> @@ -74,7 +74,7 @@
>  			interrupts = <96>;
>  		};
>  
> -		gpio2: gpio@4804c000 {
> +		gpio1: gpio@4804c000 {
>  			compatible = "ti,omap4-gpio";
>  			ti,hwmods = "gpio2";
>  			gpio-controller;
> @@ -86,7 +86,7 @@
>  			interrupts = <98>;
>  		};
>  
> -		gpio3: gpio@481ac000 {
> +		gpio2: gpio@481ac000 {
>  			compatible = "ti,omap4-gpio";
>  			ti,hwmods = "gpio3";
>  			gpio-controller;
> @@ -98,7 +98,7 @@
>  			interrupts = <32>;
>  		};
>  
> -		gpio4: gpio@481ae000 {
> +		gpio3: gpio@481ae000 {
>  			compatible = "ti,omap4-gpio";
>  			ti,hwmods = "gpio4";
>  			gpio-controller;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matt Porter Sept. 10, 2012, 4:52 p.m. UTC | #2
On Mon, Sep 10, 2012 at 06:34:20PM +0200, Benoit Cousson wrote:
> + Tony
> 
> Hi Matt,
> 
> 30 minutes too late for my pull request :-(
> 
> There are a couple of am33xx patches under discussion, so I'll take them
> and send a for_3.7/dts-part2 pull request if this is not too late for Tony.

Yeah, believe me, I did a faceplant when I saw your pull request come by
at the same time I discovered this issue. ;) In particular, AnilKumar's
user leds patch would need to be adjusted for this change. I can
resubmit with the user leds dts changes adjusted as well if that
discussion comes to a conclusion and his patches accepted.
 
> On 09/10/2012 06:20 PM, Matt Porter wrote:
> > On AM33xx, the datasheet and TRM refer to four GPIO instances that
> > are 0-based, GPIO0-3.
> 
> Or maybe you should just update the spec to use a 1-based GPIO number
> like OMAP :-)

I am powerless here. :)

-Matt

> > Signed-off-by: Matt Porter <mporter@ti.com>
> > ---
> >  arch/arm/boot/dts/am33xx.dtsi |    8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > index bb31bff..1369bfc 100644
> > --- a/arch/arm/boot/dts/am33xx.dtsi
> > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > @@ -62,7 +62,7 @@
> >  			reg = <0x48200000 0x1000>;
> >  		};
> >  
> > -		gpio1: gpio@44e07000 {
> > +		gpio0: gpio@44e07000 {
> >  			compatible = "ti,omap4-gpio";
> >  			ti,hwmods = "gpio1";
> >  			gpio-controller;
> > @@ -74,7 +74,7 @@
> >  			interrupts = <96>;
> >  		};
> >  
> > -		gpio2: gpio@4804c000 {
> > +		gpio1: gpio@4804c000 {
> >  			compatible = "ti,omap4-gpio";
> >  			ti,hwmods = "gpio2";
> >  			gpio-controller;
> > @@ -86,7 +86,7 @@
> >  			interrupts = <98>;
> >  		};
> >  
> > -		gpio3: gpio@481ac000 {
> > +		gpio2: gpio@481ac000 {
> >  			compatible = "ti,omap4-gpio";
> >  			ti,hwmods = "gpio3";
> >  			gpio-controller;
> > @@ -98,7 +98,7 @@
> >  			interrupts = <32>;
> >  		};
> >  
> > -		gpio4: gpio@481ae000 {
> > +		gpio3: gpio@481ae000 {
> >  			compatible = "ti,omap4-gpio";
> >  			ti,hwmods = "gpio4";
> >  			gpio-controller;
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benoit Cousson Sept. 10, 2012, 4:59 p.m. UTC | #3
On 09/10/2012 06:52 PM, Matt Porter wrote:
> On Mon, Sep 10, 2012 at 06:34:20PM +0200, Benoit Cousson wrote:
>> + Tony
>>
>> Hi Matt,
>>
>> 30 minutes too late for my pull request :-(
>>
>> There are a couple of am33xx patches under discussion, so I'll take them
>> and send a for_3.7/dts-part2 pull request if this is not too late for Tony.
> 
> Yeah, believe me, I did a faceplant when I saw your pull request come by
> at the same time I discovered this issue. ;) In particular, AnilKumar's
> user leds patch would need to be adjusted for this change. I can
> resubmit with the user leds dts changes adjusted as well if that
> discussion comes to a conclusion and his patches accepted.

Yeah, I was wondering if the gpios label were already used somewhere.
I've just added this patch on top of my current series.
So you or Anil should just post the missing patches whenever they'll be
available and accepted.

>> On 09/10/2012 06:20 PM, Matt Porter wrote:
>>> On AM33xx, the datasheet and TRM refer to four GPIO instances that
>>> are 0-based, GPIO0-3.
>>
>> Or maybe you should just update the spec to use a 1-based GPIO number
>> like OMAP :-)
> 
> I am powerless here. :)

That's too bad :-(

Benoit

> 
> -Matt
> 
>>> Signed-off-by: Matt Porter <mporter@ti.com>
>>> ---
>>>  arch/arm/boot/dts/am33xx.dtsi |    8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
>>> index bb31bff..1369bfc 100644
>>> --- a/arch/arm/boot/dts/am33xx.dtsi
>>> +++ b/arch/arm/boot/dts/am33xx.dtsi
>>> @@ -62,7 +62,7 @@
>>>  			reg = <0x48200000 0x1000>;
>>>  		};
>>>  
>>> -		gpio1: gpio@44e07000 {
>>> +		gpio0: gpio@44e07000 {
>>>  			compatible = "ti,omap4-gpio";
>>>  			ti,hwmods = "gpio1";
>>>  			gpio-controller;
>>> @@ -74,7 +74,7 @@
>>>  			interrupts = <96>;
>>>  		};
>>>  
>>> -		gpio2: gpio@4804c000 {
>>> +		gpio1: gpio@4804c000 {
>>>  			compatible = "ti,omap4-gpio";
>>>  			ti,hwmods = "gpio2";
>>>  			gpio-controller;
>>> @@ -86,7 +86,7 @@
>>>  			interrupts = <98>;
>>>  		};
>>>  
>>> -		gpio3: gpio@481ac000 {
>>> +		gpio2: gpio@481ac000 {
>>>  			compatible = "ti,omap4-gpio";
>>>  			ti,hwmods = "gpio3";
>>>  			gpio-controller;
>>> @@ -98,7 +98,7 @@
>>>  			interrupts = <32>;
>>>  		};
>>>  
>>> -		gpio4: gpio@481ae000 {
>>> +		gpio3: gpio@481ae000 {
>>>  			compatible = "ti,omap4-gpio";
>>>  			ti,hwmods = "gpio4";
>>>  			gpio-controller;
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Sept. 10, 2012, 5:03 p.m. UTC | #4
* Benoit Cousson <b-cousson@ti.com> [120910 09:35]:
> + Tony
> 
> Hi Matt,
> 
> 30 minutes too late for my pull request :-(
> 
> There are a couple of am33xx patches under discussion, so I'll take them
> and send a for_3.7/dts-part2 pull request if this is not too late for Tony.

Yes please do, it' hard to say when exactly we need to stop merging,
but probably we still have a bit more time.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matt Porter Sept. 10, 2012, 7:51 p.m. UTC | #5
On Mon, Sep 10, 2012 at 06:59:21PM +0200, Benoit Cousson wrote:
> On 09/10/2012 06:52 PM, Matt Porter wrote:
> > On Mon, Sep 10, 2012 at 06:34:20PM +0200, Benoit Cousson wrote:
> >> + Tony
> >>
> >> Hi Matt,
> >>
> >> 30 minutes too late for my pull request :-(
> >>
> >> There are a couple of am33xx patches under discussion, so I'll take them
> >> and send a for_3.7/dts-part2 pull request if this is not too late for Tony.
> > 
> > Yeah, believe me, I did a faceplant when I saw your pull request come by
> > at the same time I discovered this issue. ;) In particular, AnilKumar's
> > user leds patch would need to be adjusted for this change. I can
> > resubmit with the user leds dts changes adjusted as well if that
> > discussion comes to a conclusion and his patches accepted.
> 
> Yeah, I was wondering if the gpios label were already used somewhere.
> I've just added this patch on top of my current series.
> So you or Anil should just post the missing patches whenever they'll be
> available and accepted.

Ok, I went ahead and posted a series that applies to your for_3.7/dts
branch and Anil's series. That's assuming his v7 series is taken "as
is" after the leds-gpio/pinctrl discussion comes to conclusion. I see
Tony acked it aside from that discussion.

Thanks,
Matt

> 
> >> On 09/10/2012 06:20 PM, Matt Porter wrote:
> >>> On AM33xx, the datasheet and TRM refer to four GPIO instances that
> >>> are 0-based, GPIO0-3.
> >>
> >> Or maybe you should just update the spec to use a 1-based GPIO number
> >> like OMAP :-)
> > 
> > I am powerless here. :)
> 
> That's too bad :-(
> 
> Benoit
> 
> > 
> > -Matt
> > 
> >>> Signed-off-by: Matt Porter <mporter@ti.com>
> >>> ---
> >>>  arch/arm/boot/dts/am33xx.dtsi |    8 ++++----
> >>>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> >>> index bb31bff..1369bfc 100644
> >>> --- a/arch/arm/boot/dts/am33xx.dtsi
> >>> +++ b/arch/arm/boot/dts/am33xx.dtsi
> >>> @@ -62,7 +62,7 @@
> >>>  			reg = <0x48200000 0x1000>;
> >>>  		};
> >>>  
> >>> -		gpio1: gpio@44e07000 {
> >>> +		gpio0: gpio@44e07000 {
> >>>  			compatible = "ti,omap4-gpio";
> >>>  			ti,hwmods = "gpio1";
> >>>  			gpio-controller;
> >>> @@ -74,7 +74,7 @@
> >>>  			interrupts = <96>;
> >>>  		};
> >>>  
> >>> -		gpio2: gpio@4804c000 {
> >>> +		gpio1: gpio@4804c000 {
> >>>  			compatible = "ti,omap4-gpio";
> >>>  			ti,hwmods = "gpio2";
> >>>  			gpio-controller;
> >>> @@ -86,7 +86,7 @@
> >>>  			interrupts = <98>;
> >>>  		};
> >>>  
> >>> -		gpio3: gpio@481ac000 {
> >>> +		gpio2: gpio@481ac000 {
> >>>  			compatible = "ti,omap4-gpio";
> >>>  			ti,hwmods = "gpio3";
> >>>  			gpio-controller;
> >>> @@ -98,7 +98,7 @@
> >>>  			interrupts = <32>;
> >>>  		};
> >>>  
> >>> -		gpio4: gpio@481ae000 {
> >>> +		gpio3: gpio@481ae000 {
> >>>  			compatible = "ti,omap4-gpio";
> >>>  			ti,hwmods = "gpio4";
> >>>  			gpio-controller;
> >>>
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Hiremath Sept. 11, 2012, 4:57 a.m. UTC | #6
On Mon, Sep 10, 2012 at 21:50:20, Porter, Matt wrote:
> On AM33xx, the datasheet and TRM refer to four GPIO instances that
> are 0-based, GPIO0-3.
> 

Thanks Matt,
I think Anil labeled it as gpio1-4 due to hwmod naming convention, as you 
can not have gpioo id = 0 (refer to arch/arm/mach-omap2/gpio.c).

But in case of DT we should simply follow TRM/Spec, as naming convention is 
based on base-addr and not id, so your patch looks good me.

Thanks,
Vaibhav

> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
>  arch/arm/boot/dts/am33xx.dtsi |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index bb31bff..1369bfc 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -62,7 +62,7 @@
>  			reg = <0x48200000 0x1000>;
>  		};
>  
> -		gpio1: gpio@44e07000 {
> +		gpio0: gpio@44e07000 {
>  			compatible = "ti,omap4-gpio";
>  			ti,hwmods = "gpio1";
>  			gpio-controller;
> @@ -74,7 +74,7 @@
>  			interrupts = <96>;
>  		};
>  
> -		gpio2: gpio@4804c000 {
> +		gpio1: gpio@4804c000 {
>  			compatible = "ti,omap4-gpio";
>  			ti,hwmods = "gpio2";
>  			gpio-controller;
> @@ -86,7 +86,7 @@
>  			interrupts = <98>;
>  		};
>  
> -		gpio3: gpio@481ac000 {
> +		gpio2: gpio@481ac000 {
>  			compatible = "ti,omap4-gpio";
>  			ti,hwmods = "gpio3";
>  			gpio-controller;
> @@ -98,7 +98,7 @@
>  			interrupts = <32>;
>  		};
>  
> -		gpio4: gpio@481ae000 {
> +		gpio3: gpio@481ae000 {
>  			compatible = "ti,omap4-gpio";
>  			ti,hwmods = "gpio4";
>  			gpio-controller;
> -- 
> 1.7.9.5
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Hiremath Sept. 11, 2012, 5 a.m. UTC | #7
On Mon, Sep 10, 2012 at 22:29:21, Cousson, Benoit wrote:
> On 09/10/2012 06:52 PM, Matt Porter wrote:
> > On Mon, Sep 10, 2012 at 06:34:20PM +0200, Benoit Cousson wrote:
> >> + Tony
> >>
> >> Hi Matt,
> >>
> >> 30 minutes too late for my pull request :-(
> >>
> >> There are a couple of am33xx patches under discussion, so I'll take them
> >> and send a for_3.7/dts-part2 pull request if this is not too late for Tony.
> > 
> > Yeah, believe me, I did a faceplant when I saw your pull request come by
> > at the same time I discovered this issue. ;) In particular, AnilKumar's
> > user leds patch would need to be adjusted for this change. I can
> > resubmit with the user leds dts changes adjusted as well if that
> > discussion comes to a conclusion and his patches accepted.
> 
> Yeah, I was wondering if the gpios label were already used somewhere.
> I've just added this patch on top of my current series.
> So you or Anil should just post the missing patches whenever they'll be
> available and accepted.
> 
> >> On 09/10/2012 06:20 PM, Matt Porter wrote:
> >>> On AM33xx, the datasheet and TRM refer to four GPIO instances that
> >>> are 0-based, GPIO0-3.
> >>
> >> Or maybe you should just update the spec to use a 1-based GPIO number
> >> like OMAP :-)
> > 
> > I am powerless here. :)
> 
> That's too bad :-(
> 

I agree with Matt here. Also at this point of time it is too much change to 
modify spec for this, and certainly it would be very confusing. 


Thanks,
Vaibhav

> Benoit
> 
> > 
> > -Matt
> > 
> >>> Signed-off-by: Matt Porter <mporter@ti.com>
> >>> ---
> >>>  arch/arm/boot/dts/am33xx.dtsi |    8 ++++----
> >>>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> >>> index bb31bff..1369bfc 100644
> >>> --- a/arch/arm/boot/dts/am33xx.dtsi
> >>> +++ b/arch/arm/boot/dts/am33xx.dtsi
> >>> @@ -62,7 +62,7 @@
> >>>  			reg = <0x48200000 0x1000>;
> >>>  		};
> >>>  
> >>> -		gpio1: gpio@44e07000 {
> >>> +		gpio0: gpio@44e07000 {
> >>>  			compatible = "ti,omap4-gpio";
> >>>  			ti,hwmods = "gpio1";
> >>>  			gpio-controller;
> >>> @@ -74,7 +74,7 @@
> >>>  			interrupts = <96>;
> >>>  		};
> >>>  
> >>> -		gpio2: gpio@4804c000 {
> >>> +		gpio1: gpio@4804c000 {
> >>>  			compatible = "ti,omap4-gpio";
> >>>  			ti,hwmods = "gpio2";
> >>>  			gpio-controller;
> >>> @@ -86,7 +86,7 @@
> >>>  			interrupts = <98>;
> >>>  		};
> >>>  
> >>> -		gpio3: gpio@481ac000 {
> >>> +		gpio2: gpio@481ac000 {
> >>>  			compatible = "ti,omap4-gpio";
> >>>  			ti,hwmods = "gpio3";
> >>>  			gpio-controller;
> >>> @@ -98,7 +98,7 @@
> >>>  			interrupts = <32>;
> >>>  		};
> >>>  
> >>> -		gpio4: gpio@481ae000 {
> >>> +		gpio3: gpio@481ae000 {
> >>>  			compatible = "ti,omap4-gpio";
> >>>  			ti,hwmods = "gpio4";
> >>>  			gpio-controller;
> >>>
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matt Porter Sept. 11, 2012, 11:29 a.m. UTC | #8
On Tue, Sep 11, 2012 at 04:57:08AM +0000, Hiremath, Vaibhav wrote:
> On Mon, Sep 10, 2012 at 21:50:20, Porter, Matt wrote:
> > On AM33xx, the datasheet and TRM refer to four GPIO instances that
> > are 0-based, GPIO0-3.
> > 
> 
> Thanks Matt,
> I think Anil labeled it as gpio1-4 due to hwmod naming convention, as you 
> can not have gpioo id = 0 (refer to arch/arm/mach-omap2/gpio.c).

Right, and that convention originally came from the assumption that
everything would be 1-based like OMAP3/4.
 
> But in case of DT we should simply follow TRM/Spec, as naming convention is 
> based on base-addr and not id, so your patch looks good me.

Yes, my biggest concern here was the coming frustration that the end
"user" or system integrator would have with these labels not matching
the docs that have been around for a year. It would be !sane to have
somebody look at a schematic and then write their dts with a value that
doesn't match the h/w. That would run counter to the fundamental
requirement that DT is a description of the hardware. The user led patch
was already the first example of that where the comments mentioned gpio1
in the pinmux data but the data referenced the gpio2 label.

-Matt

> > Signed-off-by: Matt Porter <mporter@ti.com>
> > ---
> >  arch/arm/boot/dts/am33xx.dtsi |    8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > index bb31bff..1369bfc 100644
> > --- a/arch/arm/boot/dts/am33xx.dtsi
> > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > @@ -62,7 +62,7 @@
> >  			reg = <0x48200000 0x1000>;
> >  		};
> >  
> > -		gpio1: gpio@44e07000 {
> > +		gpio0: gpio@44e07000 {
> >  			compatible = "ti,omap4-gpio";
> >  			ti,hwmods = "gpio1";
> >  			gpio-controller;
> > @@ -74,7 +74,7 @@
> >  			interrupts = <96>;
> >  		};
> >  
> > -		gpio2: gpio@4804c000 {
> > +		gpio1: gpio@4804c000 {
> >  			compatible = "ti,omap4-gpio";
> >  			ti,hwmods = "gpio2";
> >  			gpio-controller;
> > @@ -86,7 +86,7 @@
> >  			interrupts = <98>;
> >  		};
> >  
> > -		gpio3: gpio@481ac000 {
> > +		gpio2: gpio@481ac000 {
> >  			compatible = "ti,omap4-gpio";
> >  			ti,hwmods = "gpio3";
> >  			gpio-controller;
> > @@ -98,7 +98,7 @@
> >  			interrupts = <32>;
> >  		};
> >  
> > -		gpio4: gpio@481ae000 {
> > +		gpio3: gpio@481ae000 {
> >  			compatible = "ti,omap4-gpio";
> >  			ti,hwmods = "gpio4";
> >  			gpio-controller;
> > -- 
> > 1.7.9.5
> > 
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to 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/
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Hiremath Sept. 11, 2012, 11:30 a.m. UTC | #9
On Tue, Sep 11, 2012 at 16:59:05, Porter, Matt wrote:
> On Tue, Sep 11, 2012 at 04:57:08AM +0000, Hiremath, Vaibhav wrote:
> > On Mon, Sep 10, 2012 at 21:50:20, Porter, Matt wrote:
> > > On AM33xx, the datasheet and TRM refer to four GPIO instances that
> > > are 0-based, GPIO0-3.
> > > 
> > 
> > Thanks Matt,
> > I think Anil labeled it as gpio1-4 due to hwmod naming convention, as you 
> > can not have gpioo id = 0 (refer to arch/arm/mach-omap2/gpio.c).
> 
> Right, and that convention originally came from the assumption that
> everything would be 1-based like OMAP3/4.
>  
> > But in case of DT we should simply follow TRM/Spec, as naming convention is 
> > based on base-addr and not id, so your patch looks good me.
> 
> Yes, my biggest concern here was the coming frustration that the end
> "user" or system integrator would have with these labels not matching
> the docs that have been around for a year. It would be !sane to have
> somebody look at a schematic and then write their dts with a value that
> doesn't match the h/w. That would run counter to the fundamental
> requirement that DT is a description of the hardware. The user led patch
> was already the first example of that where the comments mentioned gpio1
> in the pinmux data but the data referenced the gpio2 label.
> 

I understand and your patch is already fixing the "biggest concern" here, 
right.

Thanks,
Vaibhav

> -Matt
> 
> > > Signed-off-by: Matt Porter <mporter@ti.com>
> > > ---
> > >  arch/arm/boot/dts/am33xx.dtsi |    8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > > index bb31bff..1369bfc 100644
> > > --- a/arch/arm/boot/dts/am33xx.dtsi
> > > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > > @@ -62,7 +62,7 @@
> > >  			reg = <0x48200000 0x1000>;
> > >  		};
> > >  
> > > -		gpio1: gpio@44e07000 {
> > > +		gpio0: gpio@44e07000 {
> > >  			compatible = "ti,omap4-gpio";
> > >  			ti,hwmods = "gpio1";
> > >  			gpio-controller;
> > > @@ -74,7 +74,7 @@
> > >  			interrupts = <96>;
> > >  		};
> > >  
> > > -		gpio2: gpio@4804c000 {
> > > +		gpio1: gpio@4804c000 {
> > >  			compatible = "ti,omap4-gpio";
> > >  			ti,hwmods = "gpio2";
> > >  			gpio-controller;
> > > @@ -86,7 +86,7 @@
> > >  			interrupts = <98>;
> > >  		};
> > >  
> > > -		gpio3: gpio@481ac000 {
> > > +		gpio2: gpio@481ac000 {
> > >  			compatible = "ti,omap4-gpio";
> > >  			ti,hwmods = "gpio3";
> > >  			gpio-controller;
> > > @@ -98,7 +98,7 @@
> > >  			interrupts = <32>;
> > >  		};
> > >  
> > > -		gpio4: gpio@481ae000 {
> > > +		gpio3: gpio@481ae000 {
> > >  			compatible = "ti,omap4-gpio";
> > >  			ti,hwmods = "gpio4";
> > >  			gpio-controller;
> > > -- 
> > > 1.7.9.5
> > > 
> > > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to 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/
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index bb31bff..1369bfc 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -62,7 +62,7 @@ 
 			reg = <0x48200000 0x1000>;
 		};
 
-		gpio1: gpio@44e07000 {
+		gpio0: gpio@44e07000 {
 			compatible = "ti,omap4-gpio";
 			ti,hwmods = "gpio1";
 			gpio-controller;
@@ -74,7 +74,7 @@ 
 			interrupts = <96>;
 		};
 
-		gpio2: gpio@4804c000 {
+		gpio1: gpio@4804c000 {
 			compatible = "ti,omap4-gpio";
 			ti,hwmods = "gpio2";
 			gpio-controller;
@@ -86,7 +86,7 @@ 
 			interrupts = <98>;
 		};
 
-		gpio3: gpio@481ac000 {
+		gpio2: gpio@481ac000 {
 			compatible = "ti,omap4-gpio";
 			ti,hwmods = "gpio3";
 			gpio-controller;
@@ -98,7 +98,7 @@ 
 			interrupts = <32>;
 		};
 
-		gpio4: gpio@481ae000 {
+		gpio3: gpio@481ae000 {
 			compatible = "ti,omap4-gpio";
 			ti,hwmods = "gpio4";
 			gpio-controller;