Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[Nishanth Menon]

On 01/06/2016 02:13 AM, Laxman Dewangan wrote:
> 
> On Wednesday 06 January 2016 01:12 PM, H. Nikolaus Schaller wrote:
>> Hi,
>>
>> Am 06.01.2016 um 00:40 schrieb Nishanth Menon :
>>
>>> On 01/05/2016 06:01 AM, H. Nikolaus Schaller wrote:
 +rtc {
 +compatible = "ti,palmas-rtc";
 +interrupt-parent = <>;
 +interrupts = <8 IRQ_TYPE_NONE>;
>>> IRQ_TYPE_NONE is not correct here -> it should have some polarity - if
>>> it had none, there'd be no interrupt, right?
>> Well, it just translates IRQ_TYPE_NONE through
>>
>> Linux/include/dt-bindings/interrupt-controller/irq.h
>>
>> to
>>
>> interrupts = <8 0>;
>>
>> which is given as an example in
>>
>> Documentation//devicetree/bindings/rtc/rtc-palmas.txt
>>
>> Since I don't know anything about the rtc driver beyond the bindings
>> documentation I assume it is correct...
>> I have added Laxman Dewangan because he introduced this interrupts =
>> <8 0>;
>>
> 
> As this is for palmas interrupt controller, it does not use the second
> field for interrupt from RTC.
> So there is no really any polarity. It can be set to 0.
> 
> The second argument will be used for GPIOs mainly. However, support need
> to be added on GPIO driver for rising/falling configuration.


Device tree represents the hardware - not to reflect how the driver
works. if the driver is wrong, fix it. the interrupt polarity needs to
be described in DT. based on palmas like designs, you should probably
use IRQ_TYPE_EDGE_FALLING because that is the default signaling inside
the SoC as it reaches Secondary interrupt handlers(SIH) registers.

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

Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[Rob Herring]

On Wed, Jan 6, 2016 at 8:36 AM, Nishanth Menon  wrote:
> On 01/06/2016 02:13 AM, Laxman Dewangan wrote:
>>
>> On Wednesday 06 January 2016 01:12 PM, H. Nikolaus Schaller wrote:
>>> Hi,
>>>
>>> Am 06.01.2016 um 00:40 schrieb Nishanth Menon :
>>>
 On 01/05/2016 06:01 AM, H. Nikolaus Schaller wrote:
> +rtc {
> +compatible = "ti,palmas-rtc";
> +interrupt-parent = <>;
> +interrupts = <8 IRQ_TYPE_NONE>;
 IRQ_TYPE_NONE is not correct here -> it should have some polarity - if
 it had none, there'd be no interrupt, right?
>>> Well, it just translates IRQ_TYPE_NONE through
>>>
>>> Linux/include/dt-bindings/interrupt-controller/irq.h
>>>
>>> to
>>>
>>> interrupts = <8 0>;
>>>
>>> which is given as an example in
>>>
>>> Documentation//devicetree/bindings/rtc/rtc-palmas.txt
>>>
>>> Since I don't know anything about the rtc driver beyond the bindings
>>> documentation I assume it is correct...
>>> I have added Laxman Dewangan because he introduced this interrupts =
>>> <8 0>;
>>>
>>
>> As this is for palmas interrupt controller, it does not use the second
>> field for interrupt from RTC.
>> So there is no really any polarity. It can be set to 0.
>>
>> The second argument will be used for GPIOs mainly. However, support need
>> to be added on GPIO driver for rising/falling configuration.
>
>
> Device tree represents the hardware - not to reflect how the driver
> works. if the driver is wrong, fix it. the interrupt polarity needs to
> be described in DT. based on palmas like designs, you should probably
> use IRQ_TYPE_EDGE_FALLING because that is the default signaling inside
> the SoC as it reaches Secondary interrupt handlers(SIH) registers.

If the trigger type is not programmable, then not setting the trigger
type in the DT is fine. Internal connections are often not documented.

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

Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[Tony Lindgren]

* H. Nikolaus Schaller  [160106 08:48]:
> Hi Tony,
> 
> Am 06.01.2016 um 17:41 schrieb Tony Lindgren :
> 
> > Hi,
> > 
> > * H. Nikolaus Schaller  [160106 00:12]:
> >> Am 06.01.2016 um 02:00 schrieb Tony Lindgren :
> >>> 
> >>> Also I'm not seeing just zeroes coming from RTC after typing hwclock
> >>> on omap5-uevm. It's working on x15 though.
> >>> 
> >>> Nikolaus, is hwclock command working for you on omap5-uevm?
> >> 
> >> Well, yes and no. It appears it *was* working when tested last time
> >> (we sometimes have months of delay for submitting patches upstream).
> >> 
> >> I have found an SD image with 4.3-rc6 with this patch in the dtb and
> >> there it works. With 4.4-rc8 it does not work. hwclock command hangs for
> >> 10 seconds (I guess some timeout).
> >> 
> >> I have checked the dtb and in both cases it is interrupts = <8 0>;
> >> 
> >> xxd /sys/firmware/devicetree/base/ocp/i2c@4807/palmas@48/rtc/interrupts
> >> 000:  0008  
> >> 
> >> So I think something has changed in the rtc driver or somewhere else.
> > 
> > I just gave it a try on v4.3-rc6 with omap5-uevm.dts patched for
> > RTC, and I still don't have hwclock working with RTC.
> > 
> > It seems you have some additional patches there that make it work?
> 
> Hm. Not that I am aware of. We just did add the rtc nodes but did not
> touch palmas drivers (except adding the gpadc of this patch series).

OK

> > I guess it could also be a bootloader change if it's a different
> > SD image that works for you.
> 
> Yes, it is using a 2 years old U-Boot instead 2015.10 compiled from
> source. I will try to find out if it makes a difference.

OK. It could be also some .config change with something built-in?

Regards,

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

Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[Tony Lindgren]

Hi,

* H. Nikolaus Schaller  [160106 00:12]:
> Am 06.01.2016 um 02:00 schrieb Tony Lindgren :
> > 
> > Also I'm not seeing just zeroes coming from RTC after typing hwclock
> > on omap5-uevm. It's working on x15 though.
> > 
> > Nikolaus, is hwclock command working for you on omap5-uevm?
> 
> Well, yes and no. It appears it *was* working when tested last time
> (we sometimes have months of delay for submitting patches upstream).
> 
> I have found an SD image with 4.3-rc6 with this patch in the dtb and
> there it works. With 4.4-rc8 it does not work. hwclock command hangs for
> 10 seconds (I guess some timeout).
> 
> I have checked the dtb and in both cases it is interrupts = <8 0>;
> 
> xxd /sys/firmware/devicetree/base/ocp/i2c@4807/palmas@48/rtc/interrupts
> 000:  0008  
> 
> So I think something has changed in the rtc driver or somewhere else.

I just gave it a try on v4.3-rc6 with omap5-uevm.dts patched for
RTC, and I still don't have hwclock working with RTC.

It seems you have some additional patches there that make it work?

I guess it could also be a bootloader change if it's a different
SD image that works for you.

Regards,

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

Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[H. Nikolaus Schaller]

Hi Tony,

Am 06.01.2016 um 17:41 schrieb Tony Lindgren :

> Hi,
> 
> * H. Nikolaus Schaller  [160106 00:12]:
>> Am 06.01.2016 um 02:00 schrieb Tony Lindgren :
>>> 
>>> Also I'm not seeing just zeroes coming from RTC after typing hwclock
>>> on omap5-uevm. It's working on x15 though.
>>> 
>>> Nikolaus, is hwclock command working for you on omap5-uevm?
>> 
>> Well, yes and no. It appears it *was* working when tested last time
>> (we sometimes have months of delay for submitting patches upstream).
>> 
>> I have found an SD image with 4.3-rc6 with this patch in the dtb and
>> there it works. With 4.4-rc8 it does not work. hwclock command hangs for
>> 10 seconds (I guess some timeout).
>> 
>> I have checked the dtb and in both cases it is interrupts = <8 0>;
>> 
>> xxd /sys/firmware/devicetree/base/ocp/i2c@4807/palmas@48/rtc/interrupts
>> 000:  0008  
>> 
>> So I think something has changed in the rtc driver or somewhere else.
> 
> I just gave it a try on v4.3-rc6 with omap5-uevm.dts patched for
> RTC, and I still don't have hwclock working with RTC.
> 
> It seems you have some additional patches there that make it work?

Hm. Not that I am aware of. We just did add the rtc nodes but did not
touch palmas drivers (except adding the gpadc of this patch series).

> 
> I guess it could also be a bootloader change if it's a different
> SD image that works for you.

Yes, it is using a 2 years old U-Boot instead 2015.10 compiled from
source. I will try to find out if it makes a difference.

BR,
Nikolaus

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

Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[H. Nikolaus Schaller]

Hi Tony,

Am 06.01.2016 um 02:00 schrieb Tony Lindgren :

> * Nishanth Menon  [160105 15:40]:
>> On 01/05/2016 06:01 AM, H. Nikolaus Schaller wrote:
>>> tested on OMP5432 EVM
>>> 
>>> Signed-off-by: H. Nikolaus Schaller 
>>> ---
>>> arch/arm/boot/dts/omap5-board-common.dtsi | 8 
>>> 1 file changed, 8 insertions(+)
>>> 
>>> diff --git a/arch/arm/boot/dts/omap5-board-common.dtsi 
>>> b/arch/arm/boot/dts/omap5-board-common.dtsi
>>> index 5cf76a1..30c0d3b 100644
>>> --- a/arch/arm/boot/dts/omap5-board-common.dtsi
>>> +++ b/arch/arm/boot/dts/omap5-board-common.dtsi
>>> @@ -358,6 +358,14 @@
>>> #clock-cells = <0>;
>>> };
>>> 
>>> +   rtc {
>>> +   compatible = "ti,palmas-rtc";
>>> +   interrupt-parent = <>;
>>> +   interrupts = <8 IRQ_TYPE_NONE>;
>> 
>> IRQ_TYPE_NONE is not correct here -> it should have some polarity - if
>> it had none, there'd be no interrupt, right?
> 
> Also I'm not seeing just zeroes coming from RTC after typing hwclock
> on omap5-uevm. It's working on x15 though.
> 
> Nikolaus, is hwclock command working for you on omap5-uevm?

Well, yes and no. It appears it *was* working when tested last time
(we sometimes have months of delay for submitting patches upstream).

I have found an SD image with 4.3-rc6 with this patch in the dtb and
there it works. With 4.4-rc8 it does not work. hwclock command hangs for
10 seconds (I guess some timeout).

I have checked the dtb and in both cases it is interrupts = <8 0>;

xxd /sys/firmware/devicetree/base/ocp/i2c@4807/palmas@48/rtc/interrupts
000:  0008  

So I think something has changed in the rtc driver or somewhere else.

BR,
Nikolaus

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

Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[Laxman Dewangan]

On Wednesday 06 January 2016 01:12 PM, H. Nikolaus Schaller wrote:

Hi,

Am 06.01.2016 um 00:40 schrieb Nishanth Menon :


On 01/05/2016 06:01 AM, H. Nikolaus Schaller wrote:

+   rtc {
+   compatible = "ti,palmas-rtc";
+   interrupt-parent = <>;
+   interrupts = <8 IRQ_TYPE_NONE>;

IRQ_TYPE_NONE is not correct here -> it should have some polarity - if
it had none, there'd be no interrupt, right?

Well, it just translates IRQ_TYPE_NONE through

Linux/include/dt-bindings/interrupt-controller/irq.h

to

interrupts = <8 0>;

which is given as an example in

Documentation//devicetree/bindings/rtc/rtc-palmas.txt

Since I don't know anything about the rtc driver beyond the bindings 
documentation I assume it is correct...
I have added Laxman Dewangan because he introduced this interrupts = <8 0>;



As this is for palmas interrupt controller, it does not use the second 
field for interrupt from RTC.

So there is no really any polarity. It can be set to 0.

The second argument will be used for GPIOs mainly. However, support need 
to be added on GPIO driver for rising/falling configuration.





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

Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[Nishanth Menon]

On 01/06/2016 01:34 PM, Rob Herring wrote:
> On Wed, Jan 6, 2016 at 8:36 AM, Nishanth Menon  wrote:
>> On 01/06/2016 02:13 AM, Laxman Dewangan wrote:
>>>
>>> On Wednesday 06 January 2016 01:12 PM, H. Nikolaus Schaller wrote:
 Hi,

 Am 06.01.2016 um 00:40 schrieb Nishanth Menon :

> On 01/05/2016 06:01 AM, H. Nikolaus Schaller wrote:
>> +rtc {
>> +compatible = "ti,palmas-rtc";
>> +interrupt-parent = <>;
>> +interrupts = <8 IRQ_TYPE_NONE>;
> IRQ_TYPE_NONE is not correct here -> it should have some polarity - if
> it had none, there'd be no interrupt, right?
 Well, it just translates IRQ_TYPE_NONE through

 Linux/include/dt-bindings/interrupt-controller/irq.h

 to

 interrupts = <8 0>;

 which is given as an example in

 Documentation//devicetree/bindings/rtc/rtc-palmas.txt

 Since I don't know anything about the rtc driver beyond the bindings
 documentation I assume it is correct...
 I have added Laxman Dewangan because he introduced this interrupts =
 <8 0>;

>>>
>>> As this is for palmas interrupt controller, it does not use the second
>>> field for interrupt from RTC.
>>> So there is no really any polarity. It can be set to 0.
>>>
>>> The second argument will be used for GPIOs mainly. However, support need
>>> to be added on GPIO driver for rising/falling configuration.
>>
>>
>> Device tree represents the hardware - not to reflect how the driver
>> works. if the driver is wrong, fix it. the interrupt polarity needs to
>> be described in DT. based on palmas like designs, you should probably
>> use IRQ_TYPE_EDGE_FALLING because that is the default signaling inside
>> the SoC as it reaches Secondary interrupt handlers(SIH) registers.
> 
> If the trigger type is not programmable, then not setting the trigger
> type in the DT is fine. Internal connections are often not documented.
> 

Weird, that is not what I got feedback when I send
https://patchwork.ozlabs.org/patch/381125/

If this is the new norm, I retract my objection.


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

Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[H. Nikolaus Schaller]

Hi,

Am 06.01.2016 um 00:40 schrieb Nishanth Menon :

> On 01/05/2016 06:01 AM, H. Nikolaus Schaller wrote:
>> tested on OMP5432 EVM
>> 
>> Signed-off-by: H. Nikolaus Schaller 
>> ---
>> arch/arm/boot/dts/omap5-board-common.dtsi | 8 
>> 1 file changed, 8 insertions(+)
>> 
>> diff --git a/arch/arm/boot/dts/omap5-board-common.dtsi 
>> b/arch/arm/boot/dts/omap5-board-common.dtsi
>> index 5cf76a1..30c0d3b 100644
>> --- a/arch/arm/boot/dts/omap5-board-common.dtsi
>> +++ b/arch/arm/boot/dts/omap5-board-common.dtsi
>> @@ -358,6 +358,14 @@
>>  #clock-cells = <0>;
>>  };
>> 
>> +rtc {
>> +compatible = "ti,palmas-rtc";
>> +interrupt-parent = <>;
>> +interrupts = <8 IRQ_TYPE_NONE>;
> 
> IRQ_TYPE_NONE is not correct here -> it should have some polarity - if
> it had none, there'd be no interrupt, right?

Well, it just translates IRQ_TYPE_NONE through

Linux/include/dt-bindings/interrupt-controller/irq.h

to 

interrupts = <8 0>;

which is given as an example in

Documentation//devicetree/bindings/rtc/rtc-palmas.txt

Since I don't know anything about the rtc driver beyond the bindings 
documentation I assume it is correct...
I have added Laxman Dewangan because he introduced this interrupts = <8 0>;

> 
>> +ti,backup-battery-chargeable;
>> +ti,backup-battery-charge-high-current;
>> +};
>> +
>>  palmas_pmic {
>>  compatible = "ti,palmas-pmic";
>>  interrupt-parent = <>;
>> 
> 
> 
> -- 
> Regards,
> Nishanth Menon


BR,
Nikolaus

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

Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[Tony Lindgren]

* Nishanth Menon  [160105 15:40]:
> On 01/05/2016 06:01 AM, H. Nikolaus Schaller wrote:
> > tested on OMP5432 EVM
> > 
> > Signed-off-by: H. Nikolaus Schaller 
> > ---
> >  arch/arm/boot/dts/omap5-board-common.dtsi | 8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/omap5-board-common.dtsi 
> > b/arch/arm/boot/dts/omap5-board-common.dtsi
> > index 5cf76a1..30c0d3b 100644
> > --- a/arch/arm/boot/dts/omap5-board-common.dtsi
> > +++ b/arch/arm/boot/dts/omap5-board-common.dtsi
> > @@ -358,6 +358,14 @@
> > #clock-cells = <0>;
> > };
> >  
> > +   rtc {
> > +   compatible = "ti,palmas-rtc";
> > +   interrupt-parent = <>;
> > +   interrupts = <8 IRQ_TYPE_NONE>;
> 
> IRQ_TYPE_NONE is not correct here -> it should have some polarity - if
> it had none, there'd be no interrupt, right?

Also I'm not seeing just zeroes coming from RTC after typing hwclock
on omap5-uevm. It's working on x15 though.

Nikolaus, is hwclock command working for you on omap5-uevm?

Regards,

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

Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[Nishanth Menon]

On 01/05/2016 06:01 AM, H. Nikolaus Schaller wrote:
> tested on OMP5432 EVM
> 
> Signed-off-by: H. Nikolaus Schaller 
> ---
>  arch/arm/boot/dts/omap5-board-common.dtsi | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/omap5-board-common.dtsi 
> b/arch/arm/boot/dts/omap5-board-common.dtsi
> index 5cf76a1..30c0d3b 100644
> --- a/arch/arm/boot/dts/omap5-board-common.dtsi
> +++ b/arch/arm/boot/dts/omap5-board-common.dtsi
> @@ -358,6 +358,14 @@
>   #clock-cells = <0>;
>   };
>  
> + rtc {
> + compatible = "ti,palmas-rtc";
> + interrupt-parent = <>;
> + interrupts = <8 IRQ_TYPE_NONE>;

IRQ_TYPE_NONE is not correct here -> it should have some polarity - if
it had none, there'd be no interrupt, right?

> + ti,backup-battery-chargeable;
> + ti,backup-battery-charge-high-current;
> + };
> +
>   palmas_pmic {
>   compatible = "ti,palmas-pmic";
>   interrupt-parent = <>;
> 


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

[PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery

[H. Nikolaus Schaller]

tested on OMP5432 EVM

Signed-off-by: H. Nikolaus Schaller 
---
 arch/arm/boot/dts/omap5-board-common.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/omap5-board-common.dtsi 
b/arch/arm/boot/dts/omap5-board-common.dtsi
index 5cf76a1..30c0d3b 100644
--- a/arch/arm/boot/dts/omap5-board-common.dtsi
+++ b/arch/arm/boot/dts/omap5-board-common.dtsi
@@ -358,6 +358,14 @@
#clock-cells = <0>;
};
 
+   rtc {
+   compatible = "ti,palmas-rtc";
+   interrupt-parent = <>;
+   interrupts = <8 IRQ_TYPE_NONE>;
+   ti,backup-battery-chargeable;
+   ti,backup-battery-charge-high-current;
+   };
+
palmas_pmic {
compatible = "ti,palmas-pmic";
interrupt-parent = <>;
-- 
2.5.1

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