2013-05-15 15:00:01

by Eduardo Valentin

[permalink] [raw]
Subject: [PATCHv2 3/3] arm: dts: add bandgap entry for OMAP4460 devices

Include bandgap devices for OMAP4460 devices.

Cc: "Benoît Cousson" <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Eduardo Valentin <[email protected]>
---
arch/arm/boot/dts/omap4460.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/omap4460.dtsi b/arch/arm/boot/dts/omap4460.dtsi
index 2cf227c..e5bfbfe 100644
--- a/arch/arm/boot/dts/omap4460.dtsi
+++ b/arch/arm/boot/dts/omap4460.dtsi
@@ -29,4 +29,13 @@
<0 55 0x4>;
ti,hwmods = "debugss";
};
+
+ bandgap {
+ reg = <0x4a002260 0x4
+ 0x4a00232C 0x4
+ 0x4a002378 0x18>;
+ compatible = "ti,omap4460-bandgap";
+ interrupts = <0 126 4>; /* talert */
+ ti,tshut-gpio = <86>;
+ };
};
--
1.8.2.1.342.gfa7285d


2013-05-15 15:24:23

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCHv2 3/3] arm: dts: add bandgap entry for OMAP4460 devices

Hi Eduardo,

On 05/15/2013 04:58 PM, Eduardo Valentin wrote:
> Include bandgap devices for OMAP4460 devices.
>
> Cc: "Benoît Cousson" <[email protected]>
> Cc: Tony Lindgren <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Eduardo Valentin <[email protected]>
> ---
> arch/arm/boot/dts/omap4460.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/boot/dts/omap4460.dtsi b/arch/arm/boot/dts/omap4460.dtsi
> index 2cf227c..e5bfbfe 100644
> --- a/arch/arm/boot/dts/omap4460.dtsi
> +++ b/arch/arm/boot/dts/omap4460.dtsi
> @@ -29,4 +29,13 @@
> <0 55 0x4>;
> ti,hwmods = "debugss";
> };
> +
> + bandgap {
> + reg = <0x4a002260 0x4
> + 0x4a00232C 0x4
> + 0x4a002378 0x18>;
> + compatible = "ti,omap4460-bandgap";
> + interrupts = <0 126 4>; /* talert */
> + ti,tshut-gpio = <86>;

Why do you need a custom attribute for GPIO? Cannot you use the standard
one?

Where is the gpio controller phandle?

Usually it looks like this:

gpios = <&gpio1 8 0>;


Regards,
Benoit

2013-05-15 16:37:49

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCHv2 3/3] arm: dts: add bandgap entry for OMAP4460 devices

On 15-05-2013 11:23, Benoit Cousson wrote:
> Hi Eduardo,
>
> On 05/15/2013 04:58 PM, Eduardo Valentin wrote:
>> Include bandgap devices for OMAP4460 devices.
>>
>> Cc: "Benoît Cousson" <[email protected]>
>> Cc: Tony Lindgren <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Eduardo Valentin <[email protected]>
>> ---
>> arch/arm/boot/dts/omap4460.dtsi | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/omap4460.dtsi b/arch/arm/boot/dts/omap4460.dtsi
>> index 2cf227c..e5bfbfe 100644
>> --- a/arch/arm/boot/dts/omap4460.dtsi
>> +++ b/arch/arm/boot/dts/omap4460.dtsi
>> @@ -29,4 +29,13 @@
>> <0 55 0x4>;
>> ti,hwmods = "debugss";
>> };
>> +
>> + bandgap {
>> + reg = <0x4a002260 0x4
>> + 0x4a00232C 0x4
>> + 0x4a002378 0x18>;
>> + compatible = "ti,omap4460-bandgap";
>> + interrupts = <0 126 4>; /* talert */
>> + ti,tshut-gpio = <86>;



>
> Why do you need a custom attribute for GPIO? Cannot you use the standard
> one?

I believe it was by your suggestion :-), during the first attempts to
send this driver. But could not find the thread link :-( sorry.


I guess the reasoning to mark it as a ti specific is because it will be
used as IRQ line to treat thermal shutdown (in SW).

>
> Where is the gpio controller phandle?
>
> Usually it looks like this:
>
> gpios = <&gpio1 8 0>;
>
>
> Regards,
> Benoit
>
>
>



Attachments:
signature.asc (295.00 B)
OpenPGP digital signature
Subject: Re: [PATCHv2 3/3] arm: dts: add bandgap entry for OMAP4460 devices

On 12:36 Wed 15 May , Eduardo Valentin wrote:
> On 15-05-2013 11:23, Benoit Cousson wrote:
> > Hi Eduardo,
> >
> > On 05/15/2013 04:58 PM, Eduardo Valentin wrote:
> >> Include bandgap devices for OMAP4460 devices.
> >>
> >> Cc: "Beno?t Cousson" <[email protected]>
> >> Cc: Tony Lindgren <[email protected]>
> >> Cc: Russell King <[email protected]>
> >> Cc: [email protected]
> >> Cc: [email protected]
> >> Cc: [email protected]
> >> Cc: [email protected]
> >> Signed-off-by: Eduardo Valentin <[email protected]>
> >> ---
> >> arch/arm/boot/dts/omap4460.dtsi | 9 +++++++++
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/omap4460.dtsi b/arch/arm/boot/dts/omap4460.dtsi
> >> index 2cf227c..e5bfbfe 100644
> >> --- a/arch/arm/boot/dts/omap4460.dtsi
> >> +++ b/arch/arm/boot/dts/omap4460.dtsi
> >> @@ -29,4 +29,13 @@
> >> <0 55 0x4>;
> >> ti,hwmods = "debugss";
> >> };
> >> +
> >> + bandgap {
> >> + reg = <0x4a002260 0x4
> >> + 0x4a00232C 0x4
> >> + 0x4a002378 0x18>;
> >> + compatible = "ti,omap4460-bandgap";
> >> + interrupts = <0 126 4>; /* talert */
> >> + ti,tshut-gpio = <86>;
>
>
>
> >
> > Why do you need a custom attribute for GPIO? Cannot you use the standard
> > one?
>
> I believe it was by your suggestion :-), during the first attempts to
> send this driver. But could not find the thread link :-( sorry.
>
>
> I guess the reasoning to mark it as a ti specific is because it will be
> used as IRQ line to treat thermal shutdown (in SW).
so use interrup-parent
>
> >
> > Where is the gpio controller phandle?
> >
> > Usually it looks like this:
> >
> > gpios = <&gpio1 8 0>;
> >
> >
> > Regards,
> > Benoit
> >
> >
> >
>
>



> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2013-05-16 07:21:46

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCHv2 3/3] arm: dts: add bandgap entry for OMAP4460 devices

Hi Eduardo,

On 05/15/2013 06:36 PM, Eduardo Valentin wrote:
> On 15-05-2013 11:23, Benoit Cousson wrote:
>> Hi Eduardo,
>>
>> On 05/15/2013 04:58 PM, Eduardo Valentin wrote:
>>> Include bandgap devices for OMAP4460 devices.
>>>
>>> Cc: "Benoît Cousson" <[email protected]>
>>> Cc: Tony Lindgren <[email protected]>
>>> Cc: Russell King <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Signed-off-by: Eduardo Valentin <[email protected]>
>>> ---
>>> arch/arm/boot/dts/omap4460.dtsi | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/omap4460.dtsi b/arch/arm/boot/dts/omap4460.dtsi
>>> index 2cf227c..e5bfbfe 100644
>>> --- a/arch/arm/boot/dts/omap4460.dtsi
>>> +++ b/arch/arm/boot/dts/omap4460.dtsi
>>> @@ -29,4 +29,13 @@
>>> <0 55 0x4>;
>>> ti,hwmods = "debugss";
>>> };
>>> +
>>> + bandgap {
>>> + reg = <0x4a002260 0x4
>>> + 0x4a00232C 0x4
>>> + 0x4a002378 0x18>;
>>> + compatible = "ti,omap4460-bandgap";
>>> + interrupts = <0 126 4>; /* talert */
>>> + ti,tshut-gpio = <86>;
>
>
>
>>
>> Why do you need a custom attribute for GPIO? Cannot you use the standard
>> one?
>
> I believe it was by your suggestion :-), during the first attempts to
> send this driver. But could not find the thread link :-( sorry.

Ooops :-) I do not remember that... maybe it was long time ago, before
we had any decent binding available for GPIO and IRQ...

> I guess the reasoning to mark it as a ti specific is because it will be
> used as IRQ line to treat thermal shutdown (in SW).

Mmm, ok, so in that case, it is not even a gpio, but an interrupt entry
that is needed like that:

interrupt-parent = <&gpio3>;
interrupts = <22>; /* gpio line 86 */

Except that we already have an IRQ line connected to GIC for the
Talert... I'm not sure we can have 2 different IRQ controllers for one
device :-(

We need to check.

Regards,
Benoit


>> Where is the gpio controller phandle?
>>
>> Usually it looks like this:
>>
>> gpios = <&gpio1 8 0>;
>>
>>
>> Regards,
>> Benoit
>>
>>
>>
>
>

2013-05-16 12:28:27

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCHv2 3/3] arm: dts: add bandgap entry for OMAP4460 devices

On 16-05-2013 03:20, Benoit Cousson wrote:
> Hi Eduardo,
>
> On 05/15/2013 06:36 PM, Eduardo Valentin wrote:
>> On 15-05-2013 11:23, Benoit Cousson wrote:
>>> Hi Eduardo,
>>>
>>> On 05/15/2013 04:58 PM, Eduardo Valentin wrote:
>>>> Include bandgap devices for OMAP4460 devices.
>>>>
>>>> Cc: "Benoît Cousson" <[email protected]>
>>>> Cc: Tony Lindgren <[email protected]>
>>>> Cc: Russell King <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> Signed-off-by: Eduardo Valentin <[email protected]>
>>>> ---
>>>> arch/arm/boot/dts/omap4460.dtsi | 9 +++++++++
>>>> 1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/omap4460.dtsi b/arch/arm/boot/dts/omap4460.dtsi
>>>> index 2cf227c..e5bfbfe 100644
>>>> --- a/arch/arm/boot/dts/omap4460.dtsi
>>>> +++ b/arch/arm/boot/dts/omap4460.dtsi
>>>> @@ -29,4 +29,13 @@
>>>> <0 55 0x4>;
>>>> ti,hwmods = "debugss";
>>>> };
>>>> +
>>>> + bandgap {
>>>> + reg = <0x4a002260 0x4
>>>> + 0x4a00232C 0x4
>>>> + 0x4a002378 0x18>;
>>>> + compatible = "ti,omap4460-bandgap";
>>>> + interrupts = <0 126 4>; /* talert */
>>>> + ti,tshut-gpio = <86>;
>>
>>
>>
>>>
>>> Why do you need a custom attribute for GPIO? Cannot you use the standard
>>> one?
>>
>> I believe it was by your suggestion :-), during the first attempts to
>> send this driver. But could not find the thread link :-( sorry.
>
> Ooops :-) I do not remember that... maybe it was long time ago, before
> we had any decent binding available for GPIO and IRQ...
>


Probably it was because by that time we didnt have GPIO binding ready, yeah.

>> I guess the reasoning to mark it as a ti specific is because it will be
>> used as IRQ line to treat thermal shutdown (in SW).
>
> Mmm, ok, so in that case, it is not even a gpio, but an interrupt entry
> that is needed like that:
>
> interrupt-parent = <&gpio3>;
> interrupts = <22>; /* gpio line 86 */
>
> Except that we already have an IRQ line connected to GIC for the
> Talert... I'm not sure we can have 2 different IRQ controllers for one
> device :-(
>
> We need to check.
>


Yeah, I also dont think this will work, because we will reparent the
interrupt, setting to a different controller. That will break the TALERT
signal already defined at GIC (check original patch).

I propose keeping the way I sent. Unless there is a way to set two
different controllers to same device.

> Regards,
> Benoit
>
>
>>> Where is the gpio controller phandle?
>>>
>>> Usually it looks like this:
>>>
>>> gpios = <&gpio1 8 0>;
>>>
>>>
>>> Regards,
>>> Benoit
>>>
>>>
>>>
>>
>>
>
>
>



Attachments:
signature.asc (295.00 B)
OpenPGP digital signature

2013-05-16 12:30:21

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCHv2 3/3] arm: dts: add bandgap entry for OMAP4460 devices

On 15-05-2013 12:57, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 12:36 Wed 15 May , Eduardo Valentin wrote:
>> On 15-05-2013 11:23, Benoit Cousson wrote:
>>> Hi Eduardo,
>>>
>>> On 05/15/2013 04:58 PM, Eduardo Valentin wrote:
>>>> Include bandgap devices for OMAP4460 devices.
>>>>
>>>> Cc: "Beno?t Cousson" <[email protected]>
>>>> Cc: Tony Lindgren <[email protected]>
>>>> Cc: Russell King <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> Signed-off-by: Eduardo Valentin <[email protected]>
>>>> ---
>>>> arch/arm/boot/dts/omap4460.dtsi | 9 +++++++++
>>>> 1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/omap4460.dtsi b/arch/arm/boot/dts/omap4460.dtsi
>>>> index 2cf227c..e5bfbfe 100644
>>>> --- a/arch/arm/boot/dts/omap4460.dtsi
>>>> +++ b/arch/arm/boot/dts/omap4460.dtsi
>>>> @@ -29,4 +29,13 @@
>>>> <0 55 0x4>;
>>>> ti,hwmods = "debugss";
>>>> };
>>>> +
>>>> + bandgap {
>>>> + reg = <0x4a002260 0x4
>>>> + 0x4a00232C 0x4
>>>> + 0x4a002378 0x18>;
>>>> + compatible = "ti,omap4460-bandgap";
>>>> + interrupts = <0 126 4>; /* talert */
>>>> + ti,tshut-gpio = <86>;
>>
>>
>>
>>>
>>> Why do you need a custom attribute for GPIO? Cannot you use the standard
>>> one?
>>
>> I believe it was by your suggestion :-), during the first attempts to
>> send this driver. But could not find the thread link :-( sorry.
>>
>>
>> I guess the reasoning to mark it as a ti specific is because it will be
>> used as IRQ line to treat thermal shutdown (in SW).
> so use interrup-parent


Jean-Christophe,

That will change the controller and break the configuration for the
other IRQ line, coming from GIC.

>>
>>>
>>> Where is the gpio controller phandle?
>>>
>>> Usually it looks like this:
>>>
>>> gpios = <&gpio1 8 0>;
>>>
>>>
>>> Regards,
>>> Benoit
>>>
>>>
>>>
>>
>>
>
>
>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
>



Attachments:
signature.asc (295.00 B)
OpenPGP digital signature

2013-05-29 14:51:19

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCHv2 3/3] arm: dts: add bandgap entry for OMAP4460 devices

Salut Monsieur Benoit,

On 16-05-2013 08:27, Eduardo Valentin wrote:
> On 16-05-2013 03:20, Benoit Cousson wrote:
>> Hi Eduardo,
>>

<cut>

>> We need to check.
>>
>
>
> Yeah, I also dont think this will work, because we will reparent the
> interrupt, setting to a different controller. That will break the TALERT
> signal already defined at GIC (check original patch).
>
> I propose keeping the way I sent. Unless there is a way to set two
> different controllers to same device.
>

Any idea on this patch? Shall we keep the way it is?


>> Regards,
>> Benoit
>>
>>
>>>> Where is the gpio controller phandle?
>>>>
>>>> Usually it looks like this:
>>>>
>>>> gpios = <&gpio1 8 0>;
>>>>
>>>>
>>>> Regards,
>>>> Benoit
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>>
>
>


--
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


Attachments:
signature.asc (295.00 B)
OpenPGP digital signature

2013-05-29 14:53:55

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCHv2 3/3] arm: dts: add bandgap entry for OMAP4460 devices

Hi Eduardo,

On 5/29/2013 4:11 PM, Eduardo Valentin wrote:
> Salut Monsieur Benoit,
>
> On 16-05-2013 08:27, Eduardo Valentin wrote:
>> On 16-05-2013 03:20, Benoit Cousson wrote:
>>> Hi Eduardo,
>>>
>
> <cut>
>
>>> We need to check.
>>
>> Yeah, I also dont think this will work, because we will reparent the
>> interrupt, setting to a different controller. That will break the TALERT
>> signal already defined at GIC (check original patch).
>>
>> I propose keeping the way I sent. Unless there is a way to set two
>> different controllers to same device.
>>
>
> Any idea on this patch? Shall we keep the way it is?

Well since we cannot use directly interrupt, I think we need to use at
least the proper gpio binding.

Thanks,
Benoit

2013-05-29 15:43:14

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCHv2 3/3] arm: dts: add bandgap entry for OMAP4460 devices

On 29-05-2013 10:19, Cousson, Benoit wrote:
> Hi Eduardo,
>
> On 5/29/2013 4:11 PM, Eduardo Valentin wrote:
>> Salut Monsieur Benoit,
>>
>> On 16-05-2013 08:27, Eduardo Valentin wrote:
>>> On 16-05-2013 03:20, Benoit Cousson wrote:
>>>> Hi Eduardo,
>>>>
>>
>> <cut>
>>
>>>> We need to check.
>>>
>>> Yeah, I also dont think this will work, because we will reparent the
>>> interrupt, setting to a different controller. That will break the TALERT
>>> signal already defined at GIC (check original patch).
>>>
>>> I propose keeping the way I sent. Unless there is a way to set two
>>> different controllers to same device.
>>>
>>
>> Any idea on this patch? Shall we keep the way it is?
>
> Well since we cannot use directly interrupt, I think we need to use at
> least the proper gpio binding.
>

hmm... OK. sounds reasonable. I will change the driver and resend this
one in one single series.

> Thanks,
> Benoit
>
>
>


--
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


Attachments:
signature.asc (295.00 B)
OpenPGP digital signature
Subject: Re: [PATCHv2 3/3] arm: dts: add bandgap entry for OMAP4460 devices

On 08:29 Thu 16 May , Eduardo Valentin wrote:
> On 15-05-2013 12:57, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 12:36 Wed 15 May , Eduardo Valentin wrote:
> >> On 15-05-2013 11:23, Benoit Cousson wrote:
> >>> Hi Eduardo,
> >>>
> >>> On 05/15/2013 04:58 PM, Eduardo Valentin wrote:
> >>>> Include bandgap devices for OMAP4460 devices.
> >>>>
> >>>> Cc: "Beno?t Cousson" <[email protected]>
> >>>> Cc: Tony Lindgren <[email protected]>
> >>>> Cc: Russell King <[email protected]>
> >>>> Cc: [email protected]
> >>>> Cc: [email protected]
> >>>> Cc: [email protected]
> >>>> Cc: [email protected]
> >>>> Signed-off-by: Eduardo Valentin <[email protected]>
> >>>> ---
> >>>> arch/arm/boot/dts/omap4460.dtsi | 9 +++++++++
> >>>> 1 file changed, 9 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/omap4460.dtsi b/arch/arm/boot/dts/omap4460.dtsi
> >>>> index 2cf227c..e5bfbfe 100644
> >>>> --- a/arch/arm/boot/dts/omap4460.dtsi
> >>>> +++ b/arch/arm/boot/dts/omap4460.dtsi
> >>>> @@ -29,4 +29,13 @@
> >>>> <0 55 0x4>;
> >>>> ti,hwmods = "debugss";
> >>>> };
> >>>> +
> >>>> + bandgap {
> >>>> + reg = <0x4a002260 0x4
> >>>> + 0x4a00232C 0x4
> >>>> + 0x4a002378 0x18>;
> >>>> + compatible = "ti,omap4460-bandgap";
> >>>> + interrupts = <0 126 4>; /* talert */
> >>>> + ti,tshut-gpio = <86>;
> >>
> >>
> >>
> >>>
> >>> Why do you need a custom attribute for GPIO? Cannot you use the standard
> >>> one?
> >>
> >> I believe it was by your suggestion :-), during the first attempts to
> >> send this driver. But could not find the thread link :-( sorry.
> >>
> >>
> >> I guess the reasoning to mark it as a ti specific is because it will be
> >> used as IRQ line to treat thermal shutdown (in SW).
> > so use interrup-parent
>
>
> Jean-Christophe,
>
> That will change the controller and break the configuration for the
> other IRQ line, coming from GIC.

put this on hold I'm preparing a fix for this
I've discuss with Grant already.

This way for me is buggy

Best Regards,
J.
>
> >>
> >>>
> >>> Where is the gpio controller phandle?
> >>>
> >>> Usually it looks like this:
> >>>
> >>> gpios = <&gpio1 8 0>;
> >>>
> >>>
> >>> Regards,
> >>> Benoit
> >>>
> >>>
> >>>
> >>
> >>
> >
> >
> >
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> [email protected]
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
> >
> >
>
>