Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1301472 > unrolled thread

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

Started by"H. Nikolaus Schaller" <hns@goldelico.com>
First post2016-01-05 13:10 +0100
Last post2016-01-06 21:00 +0100
Articles 20 on this page of 44 — 8 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-05 13:10 +0100
    Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-06 00:50 +0100
      Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-06 02:10 +0100
        Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-06 09:20 +0100
          Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-06 17:50 +0100
            Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-06 18:10 +0100
              Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-08 19:00 +0100
                Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-08 19:20 +0100
                  Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-08 19:40 +0100
                    Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-08 20:10 +0100
                      Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-08 20:40 +0100
                        Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-11 21:30 +0100
                          Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-12 01:10 +0100
                            Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-12 14:40 +0100
                              Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-12 22:30 +0100
                                Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-12 22:40 +0100
                                  Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-12 23:20 +0100
                                Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-13 11:30 +0100
                                  Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-13 16:00 +0100
                                    Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Grygorii Strashko <grygorii.strashko@ti.com> - 2016-01-13 16:20 +0100
                                      Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-13 17:50 +0100
                                        Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Grygorii Strashko <grygorii.strashko@ti.com> - 2016-01-13 18:20 +0100
                                          Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-13 18:40 +0100
                                            Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-13 19:10 +0100
                                              Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-13 19:30 +0100
                                        Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-13 18:30 +0100
                                          Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-13 19:10 +0100
                                            Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-13 19:40 +0100
                                              Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-13 19:50 +0100
                                                Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-13 20:10 +0100
                                                  Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Grygorii Strashko <grygorii.strashko@ti.com> - 2016-01-13 20:30 +0100
                                                  Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-13 20:50 +0100
                                                    Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <menon.nishanth@gmail.com> - 2016-01-13 23:40 +0100
                                                      Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Keerthy <a0393675@ti.com> - 2016-01-14 11:10 +0100
                                                        Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-14 18:50 +0100
                                                          Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-14 19:40 +0100
                                          Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-13 19:10 +0100
                                            Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-13 19:20 +0100
          Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and  charging of backup battery Tony Lindgren <tony@atomide.com> - 2016-01-06 17:50 +0100
      Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-06 08:50 +0100
        Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Laxman Dewangan <ldewangan@nvidia.com> - 2016-01-06 09:30 +0100
          Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-06 15:40 +0100
            Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Rob Herring <robh+dt@kernel.org> - 2016-01-06 20:40 +0100
              Re: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging  of backup battery Nishanth Menon <nm@ti.com> - 2016-01-06 21:00 +0100

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →


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

FromTony Lindgren <tony@atomide.com>
Date2016-01-13 17:50 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQwMs-2sp-51@gated-at.bofh.it>
In reply to#1308495
* Grygorii Strashko <grygorii.strashko@ti.com> [160113 07:15]:
> On 01/13/2016 04:55 PM, Nishanth Menon wrote:
> > On 01/13/2016 04:25 AM, H. Nikolaus Schaller wrote:
> >>
> >> I wonder now what MODE1 is.
> >>
> >> In my OMAP5 TRM (Version "Y" - may be too old) the MODE1 is tagged as "reserved".
> >>
> >> Maybe "reserved" happens to output a "1" on OMAP5 and a "0" on the X15?

The 5430 data manual I listed in the commit states mode 1 is for
msecure. It is unlikely it got changed for 5432 as the mux registers
tend to stay the same for most part across a SoC generation with just
devices being enabled or disabled.

For beagle-x15, the msecure is now called "powerhold" and seems to
have some additional or different functionality in the PMIC. So
that's a separate issue from this one.

> >> And as far as I am aware there is no "driver" for some MSECURE module (but I don't know the details of MSECURE control by software).
> > 
> > Good catch. This one is interesting. If my memory serves me right,
> > MSECURE signal from SoC is triggered in secure mode (trustzone) - the
> > requirement was that certain PMIC modifications should only be done in
> > secure mode for certain product applications. What this means is that
> > certain functions of the PMIC will be unavailable when the SoC is
> > running in "untrusted" mode.
> > 
> > Instead, the usual mode of operation is to set it up as GPIO (as Nikolas
> > pointed below) and either use GPIO HOG or default weak pull to keep it
> > in the required state.
> > 
> > I think it is better to set it as GPIO than as DRM_MSECURE.

Well we do have the data manual saying it's the msecure pin, and
we are muxing it to msecure for omap4 in twl6030_omap4.dtsi. And a
TI commit has used msecure mode for GP omap5 evm at least here:

https://gitlab.com/ubuntu-omap/u-boot-omap5/commit/dcc5279ffe880e874abb4d7f95302a34ab4968ca

I've added Keerthy to Cc, maybe he knows how this should be handled
in the long run?

So if we start changing things to GPIO mode, we really need some
further explanations and neeed to handle the GPIO pin properly in
the TWL driver. And it should be done in a separate patch for all
of the TWL SoCs.

> > This is probably also the reason why this mode is NOT in public TRM -
> > all security related topics are probably in the NDA only secure TRM
> > addendum.

Right, probably the msecure pin has been set reserved in the public TRM
because of whatever NDA reasons there might be to not allow writes to RTC.

> > I'd suggest setting up a GPIO hog and a mux to GPIO for board-common (we
> > are not doing any HS OMAP5 at least in public domain :) ).
> 
> Yeah. As I remember the same issue was with OMAP4 (twl6030_omap4.dtsi)
> and, again if i remember correctly, someone reported that sys_drm_msecure might have different values
> on different SoCs. Also I'd like to note that on Old non-DT kernel such functionality
> was always modeled using GPIO.

Care to dig up some more information on that?

I don't have anything against adding GPIO handling to the TWL driver
so it can be optionally specified. But that's clearly a separate patch
and should be done by somebody who knows more about the issue and has
a test case needing the GPIO logic for this pin.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]


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

FromGrygorii Strashko <grygorii.strashko@ti.com>
Date2016-01-13 18:20 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQxfs-2SP-3@gated-at.bofh.it>
In reply to#1308604
On 01/13/2016 06:48 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [160113 07:15]:
>> On 01/13/2016 04:55 PM, Nishanth Menon wrote:
>>> On 01/13/2016 04:25 AM, H. Nikolaus Schaller wrote:
>>>>
>>>> I wonder now what MODE1 is.
>>>>
>>>> In my OMAP5 TRM (Version "Y" - may be too old) the MODE1 is tagged as "reserved".
>>>>
>>>> Maybe "reserved" happens to output a "1" on OMAP5 and a "0" on the X15?
> 
> The 5430 data manual I listed in the commit states mode 1 is for
> msecure. It is unlikely it got changed for 5432 as the mux registers
> tend to stay the same for most part across a SoC generation with just
> devices being enabled or disabled.
> 
> For beagle-x15, the msecure is now called "powerhold" and seems to
> have some additional or different functionality in the PMIC. So
> that's a separate issue from this one.
> 
>>>> And as far as I am aware there is no "driver" for some MSECURE module (but I don't know the details of MSECURE control by software).
>>>http://omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=a7a516be9338eabc9a7682e7433fa34d86c1f208
>>> Good catch. This one is interesting. If my memory serves me right,
>>> MSECURE signal from SoC is triggered in secure mode (trustzone) - the
>>> requirement was that certain PMIC modifications should only be done in
>>> secure mode for certain product applications. What this means is that
>>> certain functions of the PMIC will be unavailable when the SoC is
>>> running in "untrusted" mode.
>>>
>>> Instead, the usual mode of operation is to set it up as GPIO (as Nikolas
>>> pointed below) and either use GPIO HOG or default weak pull to keep it
>>> in the required state.
>>>
>>> I think it is better to set it as GPIO than as DRM_MSECURE.
> 
> Well we do have the data manual saying it's the msecure pin, and
> we are muxing it to msecure for omap4 in twl6030_omap4.dtsi. And a
> TI commit has used msecure mode for GP omap5 evm at least here:
> 
> https://gitlab.com/ubuntu-omap/u-boot-omap5/commit/dcc5279ffe880e874abb4d7f95302a34ab4968ca
> 
> I've added Keerthy to Cc, maybe he knows how this should be handled
> in the long run?
> 
> So if we start changing things to GPIO mode, we really need some
> further explanations and neeed to handle the GPIO pin properly in
> the TWL driver. And it should be done in a separate patch for all
> of the TWL SoCs.
> 
>>> This is probably also the reason why this mode is NOT in public TRM -
>>> all security related topics are probably in the NDA only secure TRM
>>> addendum.
> 
> Right, probably the msecure pin has been set reserved in the public TRM
> because of whatever NDA reasons there might be to not allow writes to RTC.
> 
>>> I'd suggest setting up a GPIO hog and a mux to GPIO for board-common (we
>>> are not doing any HS OMAP5 at least in public domain :) ).
>>
>> Yeah. As I remember the same issue was with OMAP4 (twl6030_omap4.dtsi)
>> and, again if i remember correctly, someone reported that sys_drm_msecure might have different values
>> on different SoCs. Also I'd like to note that on Old non-DT kernel such functionality
>> was always modeled using GPIO.
> 
> Care to dig up some more information on that?

i can't find this report, sry - as i remember there was difference
between some OMAP4 HS and GP SoCs. 

But links on commits for old 3.4 kernel below:
http://omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=a7a516be9338eabc9a7682e7433fa34d86c1f208
http://omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=262669aebf4af4044a25e8292f0e27986e18445a

> 
> I don't have anything against adding GPIO handling to the TWL driver
> so it can be optionally specified. But that's clearly a separate patch
> and should be done by somebody who knows more about the issue and has
> a test case needing the GPIO logic for this pin.
> 


-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]


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

FromNishanth Menon <nm@ti.com>
Date2016-01-13 18:40 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQxyO-300-25@gated-at.bofh.it>
In reply to#1308640
On 01/13/2016 11:12 AM, Grygorii Strashko wrote:
> On 01/13/2016 06:48 PM, Tony Lindgren wrote:

[...]

>> Care to dig up some more information on that?
> 
> i can't find this report, sry - as i remember there was difference
> between some OMAP4 HS and GP SoCs. 
> 
> But links on commits for old 3.4 kernel below:
> http://omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=a7a516be9338eabc9a7682e7433fa34d86c1f208
> http://omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=262669aebf4af4044a25e8292f0e27986e18445a
> 
>>
>> I don't have anything against adding GPIO handling to the TWL driver
>> so it can be optionally specified. But that's clearly a separate patch
>> and should be done by somebody who knows more about the issue and has
>> a test case needing the GPIO logic for this pin.
>>
> 
> 
if it helps in anyways
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/170707.html


-- 
Regards,
Nishanth Menon

[toc] | [prev] | [next] | [standalone]


#1308698

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2016-01-13 19:10 +0100
Message-ID<qQy1R-3qh-25@gated-at.bofh.it>
In reply to#1308669
Am 13.01.2016 um 18:38 schrieb Nishanth Menon <nm@ti.com>:

> On 01/13/2016 11:12 AM, Grygorii Strashko wrote:
>> On 01/13/2016 06:48 PM, Tony Lindgren wrote:
> 
> [...]
> 
>>> Care to dig up some more information on that?
>> 
>> i can't find this report, sry - as i remember there was difference
>> between some OMAP4 HS and GP SoCs. 
>> 
>> But links on commits for old 3.4 kernel below:
>> http://omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=a7a516be9338eabc9a7682e7433fa34d86c1f208
>> http://omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=262669aebf4af4044a25e8292f0e27986e18445a
>> 
>>> 
>>> I don't have anything against adding GPIO handling to the TWL driver
>>> so it can be optionally specified. But that's clearly a separate patch
>>> and should be done by somebody who knows more about the issue and has
>>> a test case needing the GPIO logic for this pin.
>>> 
>> 
>> 
> if it helps in anyways
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/170707.html

>> Yes, the TRM has some mode bits marked as reserved, but that doesn't
>> mean they don't work.  It just means the documentation is squirreled
>> away in the secure TRM addendum.

Ok, now I understand why the "reserved" MUX_MODE1 could still be correct
for OMAP5. And that I just have a "squirreled away" version of the TRM which
made me wonder what is going on.

From this discussion I read that for X15 there is a different PMIC
(Palmas derived, but not a twl6037) so that it needs something different.

So my proposal would be to keep the MUX_MODE1 (because it works
on OMAP5+TWL6037) as proposed by Tony. Maybe after adding a comment
that MUX_MODE1 is a weakly documented feature.

And the X15 board can "patch" it after using the omap5-board-common.dtsi
to whatever it needs to fix it.

BR,
Nikolaus

[toc] | [prev] | [next] | [standalone]


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

FromNishanth Menon <nm@ti.com>
Date2016-01-13 19:30 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQylc-3yp-7@gated-at.bofh.it>
In reply to#1308698
On 01/13/2016 12:00 PM, H. Nikolaus Schaller wrote:
[...]
> And the X15 board can "patch" it after using the omap5-board-common.dtsi
> to whatever it needs to fix it.
> 

There is nothing to patch on X15[1]. there is no MSECURE pin on X15.
there are three RTCs on X15: MCP, TPS and DRA7-RTC. TPS has no use for
RTC, in fact has no capability of having a backup battery - which pretty
much (at least IMHO) removes real world use of TPS as a RTC in a
non-networked world.. but anyways, lets keep X15 and uevm seperate -
they dont share much in this context.

[1]
https://github.com/beagleboard/beagleboard-x15/blob/master/BeagleBoard-X15_RevA2.pdf

-- 
Regards,
Nishanth Menon

[toc] | [prev] | [next] | [standalone]


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

FromNishanth Menon <nm@ti.com>
Date2016-01-13 18:30 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQxp9-2W5-29@gated-at.bofh.it>
In reply to#1308604
On 01/13/2016 10:48 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [160113 07:15]:
>> On 01/13/2016 04:55 PM, Nishanth Menon wrote:
>>> On 01/13/2016 04:25 AM, H. Nikolaus Schaller wrote:
>>>>
>>>> I wonder now what MODE1 is.
>>>>
>>>> In my OMAP5 TRM (Version "Y" - may be too old) the MODE1 is tagged as "reserved".
>>>>
>>>> Maybe "reserved" happens to output a "1" on OMAP5 and a "0" on the X15?
> 
> The 5430 data manual I listed in the commit states mode 1 is for
> msecure. It is unlikely it got changed for 5432 as the mux registers
> tend to stay the same for most part across a SoC generation with just
> devices being enabled or disabled.

Again - it depends on NDA or non-NDA version of the TRM being refered to.

> 
> For beagle-x15, the msecure is now called "powerhold" and seems to
> have some additional or different functionality in the PMIC. So
> that's a separate issue from this one.

powerhold is NOT the same as msecure. the PMICs for X15 and O5, though
they share the same pedigree, are NOT the same. there are distinct
changes done in both PMIC definition, functionality and markets being
targeted by the PMIC.

> 
>>>> And as far as I am aware there is no "driver" for some MSECURE module (but I don't know the details of MSECURE control by software).
>>>
>>> Good catch. This one is interesting. If my memory serves me right,
>>> MSECURE signal from SoC is triggered in secure mode (trustzone) - the
>>> requirement was that certain PMIC modifications should only be done in
>>> secure mode for certain product applications. What this means is that
>>> certain functions of the PMIC will be unavailable when the SoC is
>>> running in "untrusted" mode.
>>>
>>> Instead, the usual mode of operation is to set it up as GPIO (as Nikolas
>>> pointed below) and either use GPIO HOG or default weak pull to keep it
>>> in the required state.
>>>
>>> I think it is better to set it as GPIO than as DRM_MSECURE.
> 
> Well we do have the data manual saying it's the msecure pin, and
> we are muxing it to msecure for omap4 in twl6030_omap4.dtsi. And a
> TI commit has used msecure mode for GP omap5 evm at least here:
> 
> https://gitlab.com/ubuntu-omap/u-boot-omap5/commit/dcc5279ffe880e874abb4d7f95302a34ab4968ca

We used to have High security devices previously (before those got
scrapped).

> 
> I've added Keerthy to Cc, maybe he knows how this should be handled
> in the long run?
> 
> So if we start changing things to GPIO mode, we really need some
> further explanations and neeed to handle the GPIO pin properly in
> the TWL driver. And it should be done in a separate patch for all
> of the TWL SoCs.

That does not make sense to me. The original intent of MSECURE is to use
PMIC control (in specific certain usecases - which are no longer
relevant) in trustzone or equivalent secure processor modes. when such a
mode is not planned on being used, you just tell PMIC that it is always
in secure mode. In fact, there was discussion internally that MSECURE
should never even have been connected to SoC if the SoC was GP SoC - but
ofcourse, the want to have a consistent reference schematics for evms
(since EVMs have HS/Non-HS parts) trumped such talk.

trying to split this up into further steps adds 0 additional
functionality - what is the pmic driver supposed to do with the GPIO even?

in *real* HS product devices, in fact, the register space is really
firewalled out


> 
>>> This is probably also the reason why this mode is NOT in public TRM -
>>> all security related topics are probably in the NDA only secure TRM
>>> addendum.
> 
> Right, probably the msecure pin has been set reserved in the public TRM
> because of whatever NDA reasons there might be to not allow writes to RTC.
> 

Unfortunately, the norm inside TI, anything that remotely sounds
"secure" gets wrapped up in NDA and triple signed blah blah.. I cant
explain the rationale for why such a definition came on RTC.


>>> I'd suggest setting up a GPIO hog and a mux to GPIO for board-common (we
>>> are not doing any HS OMAP5 at least in public domain :) ).
>>
>> Yeah. As I remember the same issue was with OMAP4 (twl6030_omap4.dtsi)
>> and, again if i remember correctly, someone reported that sys_drm_msecure might have different values
>> on different SoCs. Also I'd like to note that on Old non-DT kernel such functionality
>> was always modeled using GPIO.
> 
> Care to dig up some more information on that?


The last TI product kernel tree that seriously focussed on OMAP5/OMAP4
was
http://git.omapzoom.org/?p=kernel/omap.git;a=shortlog;h=refs/heads/p-linux-omap-3.4
things changed definitions (in terms of descope) since then.. but
anyways.. thought I'd just pitch it out here.

sevm: - this board got scrapped
http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-omap5evm.c;h=bd8d71d75cc3da921856bb2004230e4cd6505328;hb=refs/heads/p-linux-omap-3.4#l1097

omap5-panda is the omap5uevm/evm now:
http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-omap5panda.c;h=6113bc0e04625a1bd794b3f169581c67ad3b42ff;hb=refs/heads/p-linux-omap-3.4#l816

> 
> I don't have anything against adding GPIO handling to the TWL driver
> so it can be optionally specified. But that's clearly a separate patch

TWL/TPS driver will need no change in the proposal I made with "gpio
hog" mechanism (Documentation/devicetree/bindings/gpio/gpio.txt -
gpio-hog property) - just a dt change for the right configuration.


> and should be done by somebody who knows more about the issue and has
> a test case needing the GPIO logic for this pin.
> 

Since my explanation does not seem to suffice, alright - we can wait for
the right person, then.


-- 
Regards,
Nishanth Menon

[toc] | [prev] | [next] | [standalone]


#1308693

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2016-01-13 19:10 +0100
Message-ID<qQy1Q-3qh-5@gated-at.bofh.it>
In reply to#1308658
Am 13.01.2016 um 19:00 schrieb Tony Lindgren <tony@atomide.com>:

> * Nishanth Menon <nm@ti.com> [160113 09:30]:
>> On 01/13/2016 10:48 AM, Tony Lindgren wrote:
>>> 
>>> So if we start changing things to GPIO mode, we really need some
>>> further explanations and neeed to handle the GPIO pin properly in
>>> the TWL driver. And it should be done in a separate patch for all
>>> of the TWL SoCs.
>> 
>> That does not make sense to me. The original intent of MSECURE is to use
>> PMIC control (in specific certain usecases - which are no longer
>> relevant) in trustzone or equivalent secure processor modes. when such a
>> mode is not planned on being used, you just tell PMIC that it is always
>> in secure mode. In fact, there was discussion internally that MSECURE
>> should never even have been connected to SoC if the SoC was GP SoC - but
>> ofcourse, the want to have a consistent reference schematics for evms
>> (since EVMs have HS/Non-HS parts) trumped such talk.
>> 
>> trying to split this up into further steps adds 0 additional
>> functionality - what is the pmic driver supposed to do with the GPIO even?
>> 
>> in *real* HS product devices, in fact, the register space is really
>> firewalled out
> 
> Right, OK here we are finally getting some answers to the "why" part :)
> 
> And I also have few more "why" question in mind. If this change from
> msecure to GPIO muxing is so important.
> 
> Why it was never fixed in the mainline kernel for omap4 and omap5 and
> it was just sitting in various TI trees?
> 
> And it sounds like any kind of muxing on HS devices here for this
> pin will oops the device?
> 
>> The last TI product kernel tree that seriously focussed on OMAP5/OMAP4
>> was
>> http://git.omapzoom.org/?p=kernel/omap.git;a=shortlog;h=refs/heads/p-linux-omap-3.4
>> things changed definitions (in terms of descope) since then.. but
>> anyways.. thought I'd just pitch it out here.
>> 
>> sevm: - this board got scrapped
>> http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-omap5evm.c;h=bd8d71d75cc3da921856bb2004230e4cd6505328;hb=refs/heads/p-linux-omap-3.4#l1097
>> 
>> omap5-panda is the omap5uevm/evm now:
>> http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-omap5panda.c;h=6113bc0e04625a1bd794b3f169581c67ad3b42ff;hb=refs/heads/p-linux-omap-3.4#l816
> 
> OK
> 
>>> I don't have anything against adding GPIO handling to the TWL driver
>>> so it can be optionally specified. But that's clearly a separate patch
>> 
>> TWL/TPS driver will need no change in the proposal I made with "gpio
>> hog" mechanism (Documentation/devicetree/bindings/gpio/gpio.txt -
>> gpio-hog property) - just a dt change for the right configuration.
> 
> OK. So are we sure the TWL driver will never have to toggle this pin?

After studying the Palmas TRM it appears that this pin just should be "high"
to be able to write to RTC and some scratchpad register. If the Palmas OTP
is programmed to use gpio7 as msecure input.

Since the scratchpad is not used we can permanently enable msecure. Which
means that we must somehow get the driving output to be "1".

This can be either done by
* a gpio with pull-up - switched to input mode as I proposed, or
* a real gpio output which is actively set to value 1 in u-boot or kernel driver, or
* the not well documented feature of MUX_MODE1 (OMAP5), other MUX_MODEs for OMAP3/4.

For my application we do not need to change the msecure state. We just need
to be able to write the rtc. And since the gpio7 of the Palmas is connected to
gpio8_234 of the OMAP5 we just have to initialize it properly once.

BR,
Nikolaus

[toc] | [prev] | [next] | [standalone]


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

FromNishanth Menon <nm@ti.com>
Date2016-01-13 19:40 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQyuR-3Ce-5@gated-at.bofh.it>
In reply to#1308693
On 01/13/2016 12:08 PM, H. Nikolaus Schaller wrote:
[...]

>> OK. So are we sure the TWL driver will never have to toggle this pin?
> 
> After studying the Palmas TRM it appears that this pin just should be "high"
> to be able to write to RTC and some scratchpad register. If the Palmas OTP
> is programmed to use gpio7 as msecure input.

Thanks for digging it up. we dont use the scratchpad, but in some cases
where SoC cold reset is involved, those registers may store additional
information.

> 
> Since the scratchpad is not used we can permanently enable msecure. Which
> means that we must somehow get the driving output to be "1".
> 
> This can be either done by
> * a gpio with pull-up - switched to input mode as I proposed, or

I think you intended to suggest to do a mux to gpio with just pinmux
pull? The internal pull on padconf is very weak - for typical needs like
these, it is rather suggested to stick with real GPIO drive to prevent
conditions like noise interference(for example).


-- 
Regards,
Nishanth Menon

[toc] | [prev] | [next] | [standalone]


#1308727

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2016-01-13 19:50 +0100
Message-ID<qQyEy-3Fw-9@gated-at.bofh.it>
In reply to#1308718
Am 13.01.2016 um 19:31 schrieb Nishanth Menon <nm@ti.com>:

> On 01/13/2016 12:08 PM, H. Nikolaus Schaller wrote:
> [...]
> 
>>> OK. So are we sure the TWL driver will never have to toggle this pin?
>> 
>> After studying the Palmas TRM it appears that this pin just should be "high"
>> to be able to write to RTC and some scratchpad register. If the Palmas OTP
>> is programmed to use gpio7 as msecure input.
> 
> Thanks for digging it up. we dont use the scratchpad, but in some cases
> where SoC cold reset is involved, those registers may store additional
> information.

I remember a similar thing from omap3-twl4030 where the boot source is stored
so that a warm reboot searches there. But I don#t know if the OMPAP5 Boot ROM
is using that.

> 
>> 
>> Since the scratchpad is not used we can permanently enable msecure. Which
>> means that we must somehow get the driving output to be "1".
>> 
>> This can be either done by
>> * a gpio with pull-up - switched to input mode as I proposed, or
> 
> I think you intended to suggest to do a mux to gpio with just pinmux
> pull?

Yes.

> The internal pull on padconf is very weak
> - for typical needs like
> these, it is rather suggested to stick with real GPIO drive to prevent
> conditions like noise interference(for example).


well, on OMAP5 pull up/down are astonishingly strong :)
100-250µA. Which translated roughly to 7 .. 18 kOhm @ 1.8V logic.
So a noise source must be coupled by an impedance in the 1 kOhm range.
This is quite rare. So I would not worry about that.

But if there is MUX_MODE1 for this purpose that works equally well, we
should use it instead of a gpin+pullup.

BR,
Nikolaus

[toc] | [prev] | [next] | [standalone]


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

FromNishanth Menon <nm@ti.com>
Date2016-01-13 20:10 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQyXU-41l-15@gated-at.bofh.it>
In reply to#1308727
On 01/13/2016 12:44 PM, H. Nikolaus Schaller wrote:
> 
> Am 13.01.2016 um 19:31 schrieb Nishanth Menon <nm@ti.com>:
> 
>> On 01/13/2016 12:08 PM, H. Nikolaus Schaller wrote:
>> [...]
>>
>>>> OK. So are we sure the TWL driver will never have to toggle this pin?
>>>
>>> After studying the Palmas TRM it appears that this pin just should be "high"
>>> to be able to write to RTC and some scratchpad register. If the Palmas OTP
>>> is programmed to use gpio7 as msecure input.
>>
>> Thanks for digging it up. we dont use the scratchpad, but in some cases
>> where SoC cold reset is involved, those registers may store additional
>> information.
> 
> I remember a similar thing from omap3-twl4030 where the boot source is stored
> so that a warm reboot searches there. But I don#t know if the OMPAP5 Boot ROM
> is using that.
> 

I believe that nonsense of OMAP ROM accessing PMIC stopped with OMAP3. I
dont believe OMAP4 or any later generation processors does any PMIC
access anymore - they instead make assumptions about specific voltage
levels they will work on (So called OPP_BOOT) instead of assuming
specific PMIC they will work with..

>>> Since the scratchpad is not used we can permanently enable msecure. Which
>>> means that we must somehow get the driving output to be "1".
>>>
>>> This can be either done by
>>> * a gpio with pull-up - switched to input mode as I proposed, or
>>
>> I think you intended to suggest to do a mux to gpio with just pinmux
>> pull?
> 
> Yes.
> 
>> The internal pull on padconf is very weak
>> - for typical needs like
>> these, it is rather suggested to stick with real GPIO drive to prevent
>> conditions like noise interference(for example).
> 
> 
> well, on OMAP5 pull up/down are astonishingly strong :)
> 100-250µA. Which translated roughly to 7 .. 18 kOhm @ 1.8V logic.
> So a noise source must be coupled by an impedance in the 1 kOhm range.
> This is quite rare. So I would not worry about that.
> 

Interesting. I did not know that, and have'nt dug at people to confirm
that either :).

An internal feedback I got some time back on AM57 (not OMAP5) - context
was that we were discussing if an external pull up resistor was needed
for a GPIO button:
"Internal pull-ups are relatively weak (ranging to 100kOhm or higher)
and are good for avoiding higher leakage due to floating input level,
and may not be sufficient for valid logic 1/0 depending on what else is
connected on the board.  If a signal must absolutely be pulled to a
valid logic 1 or 0 for system functionality, then an external pull
should be used."

Anyways... will let Tony decide where he wants to go on this..

-- 
Regards,
Nishanth Menon

[toc] | [prev] | [next] | [standalone]


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

FromGrygorii Strashko <grygorii.strashko@ti.com>
Date2016-01-13 20:30 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQzhg-48x-11@gated-at.bofh.it>
In reply to#1308739
On 01/13/2016 09:05 PM, Nishanth Menon wrote:
> On 01/13/2016 12:44 PM, H. Nikolaus Schaller wrote:
>>
>> Am 13.01.2016 um 19:31 schrieb Nishanth Menon <nm@ti.com>:
>>
>>> On 01/13/2016 12:08 PM, H. Nikolaus Schaller wrote:
>>> [...]
>>>
>>>>> OK. So are we sure the TWL driver will never have to toggle this pin?
>>>>
>>>> After studying the Palmas TRM it appears that this pin just should be "high"
>>>> to be able to write to RTC and some scratchpad register. If the Palmas OTP
>>>> is programmed to use gpio7 as msecure input.
>>>
>>> Thanks for digging it up. we dont use the scratchpad, but in some cases
>>> where SoC cold reset is involved, those registers may store additional
>>> information.
>>
>> I remember a similar thing from omap3-twl4030 where the boot source is stored
>> so that a warm reboot searches there. But I don#t know if the OMPAP5 Boot ROM
>> is using that.
>>
> 
> I believe that nonsense of OMAP ROM accessing PMIC stopped with OMAP3. I
> dont believe OMAP4 or any later generation processors does any PMIC
> access anymore - they instead make assumptions about specific voltage
> levels they will work on (So called OPP_BOOT) instead of assuming
> specific PMIC they will work with..
> 
>>>> Since the scratchpad is not used we can permanently enable msecure. Which
>>>> means that we must somehow get the driving output to be "1".
>>>>
>>>> This can be either done by
>>>> * a gpio with pull-up - switched to input mode as I proposed, or
>>>
>>> I think you intended to suggest to do a mux to gpio with just pinmux
>>> pull?
>>
>> Yes.
>>
>>> The internal pull on padconf is very weak
>>> - for typical needs like
>>> these, it is rather suggested to stick with real GPIO drive to prevent
>>> conditions like noise interference(for example).
>>
>>
>> well, on OMAP5 pull up/down are astonishingly strong :)
>> 100-250µA. Which translated roughly to 7 .. 18 kOhm @ 1.8V logic.
>> So a noise source must be coupled by an impedance in the 1 kOhm range.
>> This is quite rare. So I would not worry about that.
>>
> 
> Interesting. I did not know that, and have'nt dug at people to confirm
> that either :).
> 
> An internal feedback I got some time back on AM57 (not OMAP5) - context
> was that we were discussing if an external pull up resistor was needed
> for a GPIO button:
> "Internal pull-ups are relatively weak (ranging to 100kOhm or higher)
> and are good for avoiding higher leakage due to floating input level,
> and may not be sufficient for valid logic 1/0 depending on what else is
> connected on the board.  If a signal must absolutely be pulled to a
> valid logic 1 or 0 for system functionality, then an external pull
> should be used."

MUX_MODE1(secure modes) is not working well as i mentioned. 

Here I agree with Nishanth -  "gpio hog" mechanism looks like
the best solution now, because of:
- it exist now :P. At the moment when omap4/5 were fixed the pinmux
solution was the simplest and fastest one, with not too many alternatives.
- explicit gpio hog definition in DT will help other developer (and
what is more important HW designers) to better understand that this gpio
can be used as generic GPIO (at minimum without HW modification).  

-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]


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

FromTony Lindgren <tony@atomide.com>
Date2016-01-13 20:50 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQzAC-4i0-11@gated-at.bofh.it>
In reply to#1308739
* Nishanth Menon <nm@ti.com> [160113 11:06]:
> On 01/13/2016 12:44 PM, H. Nikolaus Schaller wrote:
> > 
> > Am 13.01.2016 um 19:31 schrieb Nishanth Menon <nm@ti.com>:
> > 
> >> On 01/13/2016 12:08 PM, H. Nikolaus Schaller wrote:
> >>> Since the scratchpad is not used we can permanently enable msecure. Which
> >>> means that we must somehow get the driving output to be "1".
> >>>
> >>> This can be either done by
> >>> * a gpio with pull-up - switched to input mode as I proposed, or
> >>
> >> I think you intended to suggest to do a mux to gpio with just pinmux
> >> pull?
> > 
> > Yes.
> > 
> >> The internal pull on padconf is very weak
> >> - for typical needs like
> >> these, it is rather suggested to stick with real GPIO drive to prevent
> >> conditions like noise interference(for example).
> > 
> > 
> > well, on OMAP5 pull up/down are astonishingly strong :)
> > 100-250µA. Which translated roughly to 7 .. 18 kOhm @ 1.8V logic.
> > So a noise source must be coupled by an impedance in the 1 kOhm range.
> > This is quite rare. So I would not worry about that.
> > 
> 
> Interesting. I did not know that, and have'nt dug at people to confirm
> that either :).
> 
> An internal feedback I got some time back on AM57 (not OMAP5) - context
> was that we were discussing if an external pull up resistor was needed
> for a GPIO button:
> "Internal pull-ups are relatively weak (ranging to 100kOhm or higher)
> and are good for avoiding higher leakage due to floating input level,
> and may not be sufficient for valid logic 1/0 depending on what else is
> connected on the board.  If a signal must absolutely be pulled to a
> valid logic 1 or 0 for system functionality, then an external pull
> should be used."
> 
> Anyways... will let Tony decide where he wants to go on this..

Eek now we have at least three options! Time for online vote:

1. Use msecure pinmux and let whatever mystery software control
   the pin completely out of our control like we've been doing with
   the mainline kernel for years.

2. Set up the msecure pin as GPIO output high unconditionally.
   This is what the TI android kernel tree seems to be doing.

3. New suggestion to use the SoC internal pull to keep the msecure
   pin high. This might be a little bit more power friendly than
   option #2 or #3.

Maybe option #3 would save a little bit more power compared to
options 1 and 2?

Anyways, considering what's been discussed, after the minimal RTC fix
we could also add code to allow the TWL driver optionally configure the
GPIO. This way the TWL driver could also check the GPIO state in case
some out-of-our-control mystery software goes tweak the msecure pin
state. Or the RTC driver could just check that the bits really change
after= writing them. Then we would at least know things are not working
right for the TWL related RTC drivers.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]


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

FromNishanth Menon <menon.nishanth@gmail.com>
Date2016-01-13 23:40 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQCf8-6cl-9@gated-at.bofh.it>
In reply to#1308772
On 01/13/2016 01:40 PM, Tony Lindgren wrote:

> Anyways, considering what's been discussed, after the minimal RTC fix
> we could also add code to allow the TWL driver optionally configure the
> GPIO. This way the TWL driver could also check the GPIO state in case
> some out-of-our-control mystery software goes tweak the msecure pin
> state.

I dont even know how that will work:
If you are using MSECURE as it is intended to be, then you'd mux it to
msecure, which means that GPIO read is just a waste of time - you dont
even mux it to external world. Now, some SoCs like DRA7 has input lines
always connected. even assuming this is for such a case:
a) when you are running linux, you are already in nonsecure - it needs
no read of MSECURE GPIO to figure that out.
b)  when you are in secure world, Linux wont be running either.

Reading from GPIO is just misguided in my opinion. firewalls are not
reconfigured, and muxes are usually done a single time.

 Or the RTC driver could just check that the bits really change
> after= writing them. Then we would at least know things are not working
> right for the TWL related RTC drivers.

that is reasonable to check, but just a overhead - anyways, just
isolated to palmas-rtc.. fail reason maynot always be issues with
MSECURE mux.. it could be very well be 32k clk fail etc.. but yeah -
that might give a hint that there is an issue..

-- 
Regards,
Nishanth Menon

[toc] | [prev] | [next] | [standalone]


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

FromKeerthy <a0393675@ti.com>
Date2016-01-14 11:10 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQN0S-5vL-11@gated-at.bofh.it>
In reply to#1308878

On Thursday 14 January 2016 04:02 AM, Nishanth Menon wrote:
> On 01/13/2016 01:40 PM, Tony Lindgren wrote:
>
>> Anyways, considering what's been discussed, after the minimal RTC fix
>> we could also add code to allow the TWL driver optionally configure the
>> GPIO. This way the TWL driver could also check the GPIO state in case
>> some out-of-our-control mystery software goes tweak the msecure pin
>> state.
>
> I dont even know how that will work:
> If you are using MSECURE as it is intended to be, then you'd mux it to
> msecure, which means that GPIO read is just a waste of time - you dont
> even mux it to external world. Now, some SoCs like DRA7 has input lines
> always connected. even assuming this is for such a case:
> a) when you are running linux, you are already in nonsecure - it needs
> no read of MSECURE GPIO to figure that out.
> b)  when you are in secure world, Linux wont be running either.
>
> Reading from GPIO is just misguided in my opinion. firewalls are not
> reconfigured, and muxes are usually done a single time.
>
>   Or the RTC driver could just check that the bits really change
>> after= writing them. Then we would at least know things are not working
>> right for the TWL related RTC drivers.
>
> that is reasonable to check, but just a overhead - anyways, just
> isolated to palmas-rtc.. fail reason maynot always be issues with
> MSECURE mux.. it could be very well be 32k clk fail etc.. but yeah -
> that might give a hint that there is an issue..
>

IIRC without configuring the mux mode of gpio234 to msecure mode we were 
unable to write to the rtc registers. Hence configured it one time at boot.

[toc] | [prev] | [next] | [standalone]


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

FromNishanth Menon <nm@ti.com>
Date2016-01-14 18:50 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQUc2-1S2-15@gated-at.bofh.it>
In reply to#1309141
On 01/14/2016 04:01 AM, Keerthy wrote:
> 
> 
> On Thursday 14 January 2016 04:02 AM, Nishanth Menon wrote:
>> On 01/13/2016 01:40 PM, Tony Lindgren wrote:
>>
>>> Anyways, considering what's been discussed, after the minimal RTC fix
>>> we could also add code to allow the TWL driver optionally configure the
>>> GPIO. This way the TWL driver could also check the GPIO state in case
>>> some out-of-our-control mystery software goes tweak the msecure pin
>>> state.
>>
>> I dont even know how that will work:
>> If you are using MSECURE as it is intended to be, then you'd mux it to
>> msecure, which means that GPIO read is just a waste of time - you dont
>> even mux it to external world. Now, some SoCs like DRA7 has input lines
>> always connected. even assuming this is for such a case:
>> a) when you are running linux, you are already in nonsecure - it needs
>> no read of MSECURE GPIO to figure that out.
>> b)  when you are in secure world, Linux wont be running either.
>>
>> Reading from GPIO is just misguided in my opinion. firewalls are not
>> reconfigured, and muxes are usually done a single time.
>>
>>   Or the RTC driver could just check that the bits really change
>>> after= writing them. Then we would at least know things are not working
>>> right for the TWL related RTC drivers.
>>
>> that is reasonable to check, but just a overhead - anyways, just
>> isolated to palmas-rtc.. fail reason maynot always be issues with
>> MSECURE mux.. it could be very well be 32k clk fail etc.. but yeah -
>> that might give a hint that there is an issue..
>>
> 
> IIRC without configuring the mux mode of gpio234 to msecure mode we were 
> unable to write to the rtc registers. Hence configured it one time at boot.
> 
Looks like you missed the code section that shows that the u-boot
configuration was overridden by kernel as GPIO for the very same reason.!

http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-omap5panda.c;h=6113bc0e04625a1bd794b3f169581c67ad3b42ff;hb=refs/heads/p-linux-omap-3.4#l816

-- 
Regards,
Nishanth Menon

[toc] | [prev] | [next] | [standalone]


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

FromTony Lindgren <tony@atomide.com>
Date2016-01-14 19:40 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQUYq-2y6-9@gated-at.bofh.it>
In reply to#1309539
* Nishanth Menon <nm@ti.com> [160114 09:40]:
> On 01/14/2016 04:01 AM, Keerthy wrote:
> > 
> > IIRC without configuring the mux mode of gpio234 to msecure mode we were 
> > unable to write to the rtc registers. Hence configured it one time at boot.
> > 
> Looks like you missed the code section that shows that the u-boot
> configuration was overridden by kernel as GPIO for the very same reason.!
> 
> http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-omap5panda.c;h=6113bc0e04625a1bd794b3f169581c67ad3b42ff;hb=refs/heads/p-linux-omap-3.4#l816

OK so let's use the GPIO hog for the msecure pin then. Here's an
updated patch, please retest that hwclock -w works properly with
the RTC patch in this thread.

Regards,

Tony

8<--------------------
From: Tony Lindgren <tony@atomide.com>
Date: Mon, 11 Jan 2016 14:35:24 -0800
Subject: [PATCH] ARM: dts: Fix omap5 PMIC control lines for RTC writes

The palmas PMIC has two control lines that need to be muxed properly
for things to work. The sys_nirq pin is used for interrupts, and msecure
pin is used for enabling writes to some PMIC registers.

Without these pins configured properly things can fail in mysterious
ways. For example, we can't update the RTC registers on palmas PMIC
unless the msecure pin is configured. And this is probably the reason
why we had RTC missing from the omap5 dts file.

According to "OMAP5430 ES2.0 Data Manual [Public] VErsion A (Rev. F)"
swps052f.pdf, mux mode 1 is for sys_drm_msecure so there's no need to
configure it as a GPIO pin.

However, it seems there are some reliability issues using the msecure
mux mode. The TI trees configure the msecure pin as GPIO out high
instead.

As the PMIC only cares that the msecure line is high to allow access
to the RTC registers, let's use a GPIO hog as suggested by Nishanth
Menon <nm@ti.com>. Also the use of the internal pull was considered
but supposedly that may not be capable of driving the line in a noisy
environment.

If we ever see high security omap5 products in the mainline tree,
those need to skip the msecure pin muxing and ignore setting the GPIO
hog. Chances are the related pin mux registers are locked in that case
and the msecure pin is managed by whatever software may be running in
the ARM TrustZone.

Who knows what the original intention of the msecure pin was. Maybe
it was supposed to prevent the system time to be set back for some
game demo modes to time out? Anyways, it seems that later PMICs like
tps659037 have recycled this pin for "powerhold" and devices like
beagle-x15 do not need changes to the msecure pin configuration.

To avoid further confusion with TWL variant PMICs, beagle-x15 does
not have a back-up battery for RTC palmas. Instead the mcp79410 RTC
is used with rtc-ds1307 driver. There is a "powerhold" jumper j5
holes near the palmas PMIC, and shorting it seems to power up
beagle-x15 automatically. It is unknown if it also has other side
effects to the beagle-x15 power up sequence.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/boot/dts/omap5-board-common.dtsi
+++ b/arch/arm/boot/dts/omap5-board-common.dtsi
@@ -130,6 +130,16 @@
 	};
 };
 
+&gpio8 {
+	/* TI trees use GPIO instead of msecure, see also muxing */
+	p234 {
+		gpio-hog;
+		gpios = <10 GPIO_ACTIVE_HIGH>;
+		output-high;
+		line-name = "gpio8_234/msecure";
+	};
+};
+
 &omap5_pmx_core {
 	pinctrl-names = "default";
 	pinctrl-0 = <
@@ -213,6 +223,13 @@
 		>;
 	};
 
+	/* TI trees use GPIO mode; msecure mode does not work reliably? */
+	palmas_msecure_pins: palmas_msecure_pins {
+		pinctrl-single,pins = <
+			OMAP5_IOPAD(0x180, PIN_OUTPUT | MUX_MODE7) /* gpio8_234 */
+		>;
+	};
+
 	usbhost_pins: pinmux_usbhost_pins {
 		pinctrl-single,pins = <
 			0x84 (PIN_INPUT | MUX_MODE0) /* usbb2_hsic_strobe */
@@ -278,6 +295,12 @@
 			&usbhost_wkup_pins
 	>;
 
+	palmas_sys_nirq_pins: pinmux_palmas_sys_nirq_pins {
+		pinctrl-single,pins = <
+			OMAP5_IOPAD(0x068, PIN_INPUT_PULLUP | MUX_MODE0) /* sys_nirq1 */
+		>;
+	};
+
 	usbhost_wkup_pins: pinmux_usbhost_wkup_pins {
 		pinctrl-single,pins = <
 			0x1A (PIN_OUTPUT | MUX_MODE0) /* fref_clk1_out, USB hub clk */
@@ -345,6 +368,8 @@
 		interrupt-controller;
 		#interrupt-cells = <2>;
 		ti,system-power-controller;
+		pinctrl-names = "default";
+		pinctrl-0 = <&palmas_sys_nirq_pins &palmas_msecure_pins>;
 
 		extcon_usb3: palmas_usb {
 			compatible = "ti,palmas-usb-vid";

[toc] | [prev] | [next] | [standalone]


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

FromTony Lindgren <tony@atomide.com>
Date2016-01-13 19:10 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQy1Q-3qh-7@gated-at.bofh.it>
In reply to#1308658
* Nishanth Menon <nm@ti.com> [160113 09:30]:
> On 01/13/2016 10:48 AM, Tony Lindgren wrote:
> > 
> > So if we start changing things to GPIO mode, we really need some
> > further explanations and neeed to handle the GPIO pin properly in
> > the TWL driver. And it should be done in a separate patch for all
> > of the TWL SoCs.
> 
> That does not make sense to me. The original intent of MSECURE is to use
> PMIC control (in specific certain usecases - which are no longer
> relevant) in trustzone or equivalent secure processor modes. when such a
> mode is not planned on being used, you just tell PMIC that it is always
> in secure mode. In fact, there was discussion internally that MSECURE
> should never even have been connected to SoC if the SoC was GP SoC - but
> ofcourse, the want to have a consistent reference schematics for evms
> (since EVMs have HS/Non-HS parts) trumped such talk.
> 
> trying to split this up into further steps adds 0 additional
> functionality - what is the pmic driver supposed to do with the GPIO even?
> 
> in *real* HS product devices, in fact, the register space is really
> firewalled out

Right, OK here we are finally getting some answers to the "why" part :)

And I also have few more "why" question in mind. If this change from
msecure to GPIO muxing is so important.

Why it was never fixed in the mainline kernel for omap4 and omap5 and
it was just sitting in various TI trees?

And it sounds like any kind of muxing on HS devices here for this
pin will oops the device?

> The last TI product kernel tree that seriously focussed on OMAP5/OMAP4
> was
> http://git.omapzoom.org/?p=kernel/omap.git;a=shortlog;h=refs/heads/p-linux-omap-3.4
> things changed definitions (in terms of descope) since then.. but
> anyways.. thought I'd just pitch it out here.
> 
> sevm: - this board got scrapped
> http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-omap5evm.c;h=bd8d71d75cc3da921856bb2004230e4cd6505328;hb=refs/heads/p-linux-omap-3.4#l1097
> 
> omap5-panda is the omap5uevm/evm now:
> http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-omap5panda.c;h=6113bc0e04625a1bd794b3f169581c67ad3b42ff;hb=refs/heads/p-linux-omap-3.4#l816

OK

> > I don't have anything against adding GPIO handling to the TWL driver
> > so it can be optionally specified. But that's clearly a separate patch
> 
> TWL/TPS driver will need no change in the proposal I made with "gpio
> hog" mechanism (Documentation/devicetree/bindings/gpio/gpio.txt -
> gpio-hog property) - just a dt change for the right configuration.

OK. So are we sure the TWL driver will never have to toggle this pin?

Again, that's another "why" that I have no clue about and that is not
documented anywhere.

> > and should be done by somebody who knows more about the issue and has
> > a test case needing the GPIO logic for this pin.
> 
> Since my explanation does not seem to suffice, alright - we can wait for
> the right person, then.

Sorry don't take it personally. I'm just trying to make sense of the
"why do we need to do this change?" part. Especially if I need to make
the change and write the commit log.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]


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

FromNishanth Menon <nm@ti.com>
Date2016-01-13 19:20 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qQybw-3tX-13@gated-at.bofh.it>
In reply to#1308699
On 01/13/2016 12:00 PM, Tony Lindgren wrote:
> * Nishanth Menon <nm@ti.com> [160113 09:30]:
>> On 01/13/2016 10:48 AM, Tony Lindgren wrote:
>>>
>>> So if we start changing things to GPIO mode, we really need some
>>> further explanations and neeed to handle the GPIO pin properly in
>>> the TWL driver. And it should be done in a separate patch for all
>>> of the TWL SoCs.
>>
>> That does not make sense to me. The original intent of MSECURE is to use
>> PMIC control (in specific certain usecases - which are no longer
>> relevant) in trustzone or equivalent secure processor modes. when such a
>> mode is not planned on being used, you just tell PMIC that it is always
>> in secure mode. In fact, there was discussion internally that MSECURE
>> should never even have been connected to SoC if the SoC was GP SoC - but
>> ofcourse, the want to have a consistent reference schematics for evms
>> (since EVMs have HS/Non-HS parts) trumped such talk.
>>
>> trying to split this up into further steps adds 0 additional
>> functionality - what is the pmic driver supposed to do with the GPIO even?
>>
>> in *real* HS product devices, in fact, the register space is really
>> firewalled out
> 
> Right, OK here we are finally getting some answers to the "why" part :)
> 
> And I also have few more "why" question in mind. If this change from
> msecure to GPIO muxing is so important.
> 
> Why it was never fixed in the mainline kernel for omap4 and omap5 and
> it was just sitting in various TI trees?
> 
> And it sounds like any kind of muxing on HS devices here for this
> pin will oops the device?
> 

It depends on what the secure firewall does - unfortunately HS devices
are basically lego blocks that way. In the original usecase where
specific function like MSECURE was desired, the 4k chunk for padconf
registers would be firewalled away and only setup for access from
trustzone or a specific "trusted" processor like IPU M3/DSP.

>> The last TI product kernel tree that seriously focussed on OMAP5/OMAP4
>> was
>> http://git.omapzoom.org/?p=kernel/omap.git;a=shortlog;h=refs/heads/p-linux-omap-3.4
>> things changed definitions (in terms of descope) since then.. but
>> anyways.. thought I'd just pitch it out here.
>>
>> sevm: - this board got scrapped
>> http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-omap5evm.c;h=bd8d71d75cc3da921856bb2004230e4cd6505328;hb=refs/heads/p-linux-omap-3.4#l1097
>>
>> omap5-panda is the omap5uevm/evm now:
>> http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-omap5panda.c;h=6113bc0e04625a1bd794b3f169581c67ad3b42ff;hb=refs/heads/p-linux-omap-3.4#l816
> 
> OK
> 
>>> I don't have anything against adding GPIO handling to the TWL driver
>>> so it can be optionally specified. But that's clearly a separate patch
>>
>> TWL/TPS driver will need no change in the proposal I made with "gpio
>> hog" mechanism (Documentation/devicetree/bindings/gpio/gpio.txt -
>> gpio-hog property) - just a dt change for the right configuration.
> 
> OK. So are we sure the TWL driver will never have to toggle this pin?
> 
> Again, that's another "why" that I have no clue about and that is not
> documented anywhere.
> 

It is not necessary for the functions we just described -MSECURE
function by itself is not something to be controlled by "non secure"
software (which is probably weird to us, but that is what security team
folks call HLOS like Linux). I dont recollect any recent product that
actually uses MSECURE the way we originally defined it. For the sake of
debate, Lets take a theoretical case where such a function might be
desired: in such a case, "non-secure" software should generate an SMC
service call into secure world for "setting RTC time"; When ARM enters
trustzone mode, MSECURE will be auto asserted by SoC. the secure
firmware will then have an I2C driver that will send a RTC set time
register access for the RTC time to be set.


In the above definition, we should not even have an TWL RTC driver,
instead a custom TWL-SMC-RTC driver will need to be written that will
access RTC on TWL/TPS over SMC calls to secure service. infact, if DSP
was desired for the "secure access", then a DSP-RPROC-RTC driver will
have to be written. The "generic definition" then became "MSECURE" and
was envisaged to protect further stuff eventually beyond RTC(I dont
recollect more than RTC unfortunately). In all such cases, you'd not use
MSECURE in GPIO mode - that will just defeat it's original purpose.
Instead you'd set it up as MSECURE in  secure boot software(even before
HLOS starts), then firewall the region off from access by non-secure
software, finally write a shim driver to send non-secure requests to the
secure world - which will determine who of the actors can actually do
which actions..

As you already see it is ridiculously round about way of protecting RTC
time.. but anyways, for what ever reason, that was mandatory function to
support on certain product lines.


I hope this helps.

>>> and should be done by somebody who knows more about the issue and has
>>> a test case needing the GPIO logic for this pin.
>>
>> Since my explanation does not seem to suffice, alright - we can wait for
>> the right person, then.
> 
> Sorry don't take it personally. I'm just trying to make sense of the
> "why do we need to do this change?" part. Especially if I need to make
> the change and write the commit log.

Not a problem, just trying to share what I can given that not all
thought process and background work that takes place inside TI is either
"logical" or in many cases fails to reach public documentation :( .

-- 
Regards,
Nishanth Menon

[toc] | [prev] | [next] | [standalone]


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

FromTony Lindgren <tony@atomide.com>
Date2016-01-06 17:50 +0100
SubjectRe: [PATCH 1/3] ARM: dts: omap5-board-common: enable rtc and charging of backup battery
Message-ID<qNZrB-3H4-9@gated-at.bofh.it>
In reply to#1302452
Hi,

* H. Nikolaus Schaller <hns@goldelico.com> [160106 00:12]:
> Am 06.01.2016 um 02:00 schrieb Tony Lindgren <tony@atomide.com>:
> > 
> > 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@48070000/palmas@48/rtc/interrupts
> 0000000: 0000 0008 0000 0000
> 
> 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-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/

[toc] | [prev] | [next] | [standalone]


#1302442

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2016-01-06 08:50 +0100
Message-ID<qNR10-6zh-9@gated-at.bofh.it>
In reply to#1302275
Hi,

Am 06.01.2016 um 00:40 schrieb Nishanth Menon <nm@ti.com>:

> On 01/05/2016 06:01 AM, H. Nikolaus Schaller wrote:
>> tested on OMP5432 EVM
>> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> 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 = <&palmas>;
>> +			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 = <&palmas>;
>> 
> 
> 
> -- 
> Regards,
> Nishanth Menon


BR,
Nikolaus

--
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/

[toc] | [prev] | [next] | [standalone]


Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel


csiph-web