ARM: OMAP2+: Add voltagedomain, powerdomain & PRM support for AM33XX device

From V3:

From V1 & V2:

Currently dummy voltage domain data is being created in order to succeed boot process, nothing has been done w.r.t actual hardware (voltage control).

Also, hook up the AM33XX voltage domain to OMAP framework.

Signed-off-by Vaibhav Hiremath <[email protected]>

Signed-off-by Afzal Mohammed <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Rajendra Nayak <[email protected]>

Hi Vaibhav, Kevin,

Quoted 6 lines · Vaibhav Hiremath

Currently dummy voltage domain data is being created in order to succeed boot process, nothing has been done w.r.t actual hardware (voltage control).

Also, hook up the AM33XX voltage domain to OMAP framework.

Quoted 7 lines

Signed-off-by Vaibhav Hiremath <[email protected]>

Signed-off-by Afzal Mohammed <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Rajendra Nayak <[email protected]>

Looks okay to me. Kevin, you want to ack this one and I will queue it? Either that, or if you want to take it, I will ack it.

In case of AM33XX device, XXX_RSTST register offset is not consistent across PRM modules/instances,

PRM_XXX RSTST
=========================

PRM_PER_MOD:       0x04
PRM_WKUP_MOD:      0x0C
PRM_MPU_MOD:       NA
PRM_DEVICE_MOD:    0x08

This means, we need to pass on XXX_RSTST register offset information through omap_hwmod data, similar to XXX_RSTCTRL.

Currently, this field is only applicable and used for AM33XX devices.

Signed-off-by Vaibhav Hiremath <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Rajendra Nayak <[email protected]>

---
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

arch/arm/plat-omap/include/plat/omap_hwmod.h +0 -0

index 9ae87eb..50f42ec 100644
@@ -374,11 +374,13 @@ struct omap_hwmod_omap2_prcm {
  * struct omap_hwmod_omap4_prcm - OMAP4-specific PRCM data
  * @clkctrl_reg: PRCM address of the clock control register
  * @rstctrl_reg: address of the XXX_RSTCTRL register located in the PRM

As far as PRM/CM/PRCM modules are concerned, AM33XX device is different than OMAP3 and OMAP4 architectures; so we need to handle it separately.
This patch adds support for, Powerdomain, Powerdomain data, PRM api's required for AM33XX device.

And also, hooks up AM33XX powerdomain to existing OMAP framework.

Quoted 7 lines · Vaibhav Hiremath

As far as PRM/CM/PRCM modules are concerned, AM33XX device is different than OMAP3 and OMAP4 architectures; so we need to handle it separately.
This patch adds support for, Powerdomain, Powerdomain data, PRM api's required for AM33XX device.

And also, hooks up AM33XX powerdomain to existing OMAP framework.

[...]

arch/arm/mach-omap2/omap_hwmod.c static int _assert_hardreset(struct omap_hwmod *oh, const char *name) · line 1299

Quoted 13 lines

arch/arm/mach-omap2/omap_hwmod.c +8 -1

@@ -1288,7 +1289,15 @@ static int _assert_hardreset(struct omap_hwmod *oh, const char *name)
 	if (IS_ERR_VALUE(ret))
 		return ret;
 
-	if (cpu_is_omap24xx() || cpu_is_omap34xx())
+	/*
+	 * cpu_is_omap34xx() is true for am33xx device as well, so
+	 * fist check for cpu_is_am33xx().
+	 */
+	if (cpu_is_am33xx())
+		return am33xx_prm_assert_hardreset(ohri.rst_shift,
+				oh->clkdm->pwrdm.ptr->prcm_offs,
+				oh->prcm.omap4.rstctrl_offs);

This still troubles me. I *really* don't like that we have a dependence on cpu_is* call ordering. This is very fragile and error prone.

I also don't like all the cpu_is* checking currently in omap_hwmod.c (which is here before you added this) and have an idea on how to clean it up, I should have a patch by tomorrow for this. That should help adding am33xx support here without needing all the cpu_is* checking.

That being said, I just did a simple experiment[1] to see why cpu_is_omap34xx() needs to be true for AM33xx in the first place. Based on my quick experiment, it does not appear to be needed. I think our lives will be much simpler if cpu_is_omap34xx() is not true for the AM335x family.

Can you have a look at my test branch[1] and see what you think? I changed the omap_revision for AM335x so that cpu_is_omap34xx() is no longer true on this platform. Then, I only needed to fixup the SRAM init, and it boots just fine on my BeagleBone.

I really think we need to go this route, because having cpu_is_omap34xx() true on AM335x is causing more headaches than it is solving problems. This will require you to rework a little bit these clock/power/voltage domain patches, but I belive it will greatly simplify the maintainability of the end result.

Kevin

[1] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git tmp/am335x-cpu-is-hacking

Quoted 45 lines · Kevin

Quoted 7 lines · Vaibhav Hiremath

As far as PRM/CM/PRCM modules are concerned, AM33XX device is different than OMAP3 and OMAP4 architectures; so we need to handle it separately.
This patch adds support for, Powerdomain, Powerdomain data, PRM api's required for AM33XX device.

And also, hooks up AM33XX powerdomain to existing OMAP framework.

[...]

arch/arm/mach-omap2/omap_hwmod.c static int _assert_hardreset(struct omap_hwmod *oh, const char *name) · line 1299

Quoted 13 lines

arch/arm/mach-omap2/omap_hwmod.c +8 -1

@@ -1288,7 +1289,15 @@ static int _assert_hardreset(struct omap_hwmod *oh, const char *name)
 	if (IS_ERR_VALUE(ret))
 		return ret;
 
-	if (cpu_is_omap24xx() || cpu_is_omap34xx())
+	/*
+	 * cpu_is_omap34xx() is true for am33xx device as well, so
+	 * fist check for cpu_is_am33xx().
+	 */
+	if (cpu_is_am33xx())
+		return am33xx_prm_assert_hardreset(ohri.rst_shift,
+				oh->clkdm->pwrdm.ptr->prcm_offs,
+				oh->prcm.omap4.rstctrl_offs);

This still troubles me. I *really* don't like that we have a dependence on cpu_is* call ordering. This is very fragile and error prone.

I also don't like all the cpu_is* checking currently in omap_hwmod.c (which is here before you added this) and have an idea on how to clean it up, I should have a patch by tomorrow for this. That should help adding am33xx support here without needing all the cpu_is* checking.

That being said, I just did a simple experiment[1] to see why cpu_is_omap34xx() needs to be true for AM33xx in the first place. Based on my quick experiment, it does not appear to be needed. I think our lives will be much simpler if cpu_is_omap34xx() is not true for the AM335x family.

Can you have a look at my test branch[1] and see what you think? I changed the omap_revision for AM335x so that cpu_is_omap34xx() is no longer true on this platform. Then, I only needed to fixup the SRAM init, and it boots just fine on my BeagleBone.

Quoted 6 lines

I really think we need to go this route, because having cpu_is_omap34xx() true on AM335x is causing more headaches than it is solving problems. This will require you to rework a little bit these clock/power/voltage domain patches, but I belive it will greatly simplify the maintainability of the end result.

Let me spend some time, and explore your changes; I think getting system to boot should not be only criteria here. But honestly, I fully agree with you that, we are creating more issue,
adding more cpu_is_xxx() checks, with cpu_is_34xx() true for AM33xx.

As I have done in the past initially, I recommend and vote for,

1. Creating separate family cpu_is_am33xx() for AM33xx device.
OR
2. Bring it to omap44xx family, since prcm block is closer to omap4 and not with omap3. Also,

Tony,
I will let tony make a decision on this. By the time, Tony makes his decision, I will spend time to explore your (Kevin's below) branch.

I also don't like all the cpu_is* checking currently in omap_hwmod.c (which is here before you added this) and have an idea on how to clean it up, I should have a patch by tomorrow for this. That should help adding am33xx support here without needing all the cpu_is* checking.

After some healthy discussion, now we have come to the conclusion and decided to handle AM33XX PRM/CM part separately; as AM33XX-PRCM module is different than OMAP3 and OMAP4 architecture.

The difference becomes very interesting/weird when it comes to the consistency for register offsets in PRM address space and bit-field offsets inside PRM registers,
So along with Powerdomain data and PRM api's required for AM33XX device, this patch series adds,

- XXX_RSTST register offset to "struct omap_hwmod_omap4_prcm"
- PWRSTCTRL & PWRSTST register offsets to "struct powerdomain"
- Logicretstate and mem_on/ret/pwrst/retst mask to "struct powerdomain"

Testing: This patch series has been boot tested on AM37xEVM and AM335x
based BeagleBone community board.

Thanks to Paul here...for helping and concluding on this, shortly I will submit similar patch for CM, clockdomain and clock-tree support for AM33xx.

This patch-series is created on top of "linux-omap/cleanup" branch, and also gets applied to "linux-omap/master" branch.
The patches are also available at -
https://github.com/hvaibhav/am335x-linux/tree/am335x-prm-cm

After some healthy discussion, now we have come to the conclusion and decided to handle AM33XX PRM/CM part separately; as AM33XX-PRCM module is different than OMAP3 and OMAP4 architecture.

Hello Vaibhav,

Quoted 3 lines · Vaibhav Hiremath

After some healthy discussion, now we have come to the conclusion and decided to handle AM33XX PRM/CM part separately; as AM33XX-PRCM module is different than OMAP3 and OMAP4 architecture.

Have been reviewing these patch sets here. Are these associated with any board file? I had to hand-apply several patch sets due to some differences in mach-omap2/io.c. Upon looking more closely, and looking at your GitHub branch:

https://github.com/hvaibhav/am335x-linux/tree/am335x-prm-cm

it seems that internally you've been modifying the AM3517 EVM board file to test the BeagleBone? But due to the init_early changes, this is presumably not going to work if we want to keep the AM3517EVM working -- is this your understanding as well?

I'm kind of wondering how we're to test these...

...

BTW, the branch that I'm experimenting with is at
git://git.pwsan.com/linux-2.6 -- branch name 'am33xx_support_3.5' -- I am still reviewing the patches there.

- Paul

Yes, we are using am3517evm file for AM33XX MACHINE_INIT definition here. These patches have been reviewed and accepted in the list, only thing is they couldn't make it to upstream during v3.4 merge window.

I believe Tony is going to push them for v3.5 now.

Hi Vaibhav,

Quoted 4 lines · Vaibhav

Yes, we are using am3517evm file for AM33XX MACHINE_INIT definition here. These patches have been reviewed and accepted in the list, only thing is they couldn't make it to upstream during v3.4 merge window.

Quoted 1 line

I believe Tony is going to push them for v3.5 now.

Hmm, but if you modify the AM3517EVM board file to call am33xx_init_early(), won't that break the AM3517EVM board? Wouldn't a separate board file be needed for the AM33xx?

- Paul

Hi Vaibhav,

Quoted 4 lines · Vaibhav

Yes, we are using am3517evm file for AM33XX MACHINE_INIT definition here. These patches have been reviewed and accepted in the list, only thing is they couldn't make it to upstream during v3.4 merge window.

Quoted 1 line

I believe Tony is going to push them for v3.5 now.

Hmm, but if you modify the AM3517EVM board file to call am33xx_init_early(), won't that break the AM3517EVM board? Wouldn't a separate board file be needed for the AM33xx?

On Thu, Apr 12, 2012 at 13:56:06, Paul Walmsley wrote:

Yes, we are using am3517evm file for AM33XX MACHINE_INIT definition here. These patches have been reviewed and accepted in the list, only thing is they couldn't make it to upstream during v3.4 merge window.

I believe Tony is going to push them for v3.5 now.

Quoted 7 lines · Hiremath, Vaibhav

On Thu, Apr 12, 2012 at 13:56:06, Paul Walmsley wrote:

Yes, we are using am3517evm file for AM33XX MACHINE_INIT definition here. These patches have been reviewed and accepted in the list, only thing is they couldn't make it to upstream during v3.4 merge window.

I believe Tony is going to push them for v3.5 now.

Hmm care to clarify which patches are you talking about here?

Tony

Quoted 11 lines · Tony Lindgren

Quoted 7 lines · Hiremath, Vaibhav

On Thu, Apr 12, 2012 at 13:56:06, Paul Walmsley wrote:

Yes, we are using am3517evm file for AM33XX MACHINE_INIT definition here. These patches have been reviewed and accepted in the list, only thing is they couldn't make it to upstream during v3.4 merge window.

I believe Tony is going to push them for v3.5 now.

Hmm care to clarify which patches are you talking about here?

Tony,
I was referring to initial 2 baseport patches which we have dropped due dependency on mach-types.

arm:omap:am33xx: Add AM335XEVM machine support
arm:omap:am33xx: Add low level debugging support

One patch (which was independent) from this series, made its way to upstream,
ARM: OMAP2+: id: Add am33xx SoC type detection

Can you please make sure that, it gets included into your next pull request?

Quoted 13 lines · Hiremath, Vaibhav

Quoted 11 lines · Tony Lindgren

Quoted 7 lines · Hiremath, Vaibhav

On Thu, Apr 12, 2012 at 13:56:06, Paul Walmsley wrote:

Yes, we are using am3517evm file for AM33XX MACHINE_INIT definition here. These patches have been reviewed and accepted in the list, only thing is they couldn't make it to upstream during v3.4 merge window.

I believe Tony is going to push them for v3.5 now.

Hmm care to clarify which patches are you talking about here?

Quoted 6 lines

Tony,
I was referring to initial 2 baseport patches which we have dropped due dependency on mach-types.

arm:omap:am33xx: Add AM335XEVM machine support
arm:omap:am33xx: Add low level debugging support

Can you please repost these against current v3.4-rc4?

Quoted 2 lines

One patch (which was independent) from this series, made its way to upstream,
ARM: OMAP2+: id: Add am33xx SoC type detection

OK

Quoted 1 line

Can you please make sure that, it gets included into your next pull request?

As far as I remember they looked OK for minimal board changes.

Yes, they were accepted and were also available under linux-omap/soc2 branch for quite some time.

Quoted 3 lines · Hiremath, Vaibhav

Yes, they were accepted and were also available under linux-omap/soc2 branch for quite some time.

Hmm sorry if I've dropped them accidentally. I tend to drop the branches after each merge window and purge my linux-omap mailbox too.

I will be waiting for your comment and conformation on, which family AM33xx device should fall in? Please refer to the mail-chain -

http://www.mail-archive.com/linux-omap at vger.kernel.org/msg67275.html

Quoted 4 lines · Vaibhav

I will be waiting for your comment and conformation on, which family AM33xx device should fall in? Please refer to the mail-chain -

http://www.mail-archive.com/linux-omap at vger.kernel.org/msg67275.html

This decision turns out to be pretty important; it certainly affects the AM33xx PRM/CM/clockdomain/powerdomain patches that I've been looking at.

Here is my suggestion, based on our previous practice. I am not so sure that it is the best way, but it seems pretty reasonable"

Using CONFIG_ARCH_OMAP3 for CONFIG_ARCH_OMAPAM33XX doesn't make sense, as far as I can tell. The main area of similarity between the other CONFIG_ARCH_OMAP3 and AM33xx is the Cortex-A8. And even that MPU subsystem is quite different between, say, the 3430/3630 and the AM33xx. Most of the AM33xx is quite different from the 3430/3630: OMAP4-style PRCM interface, OMAP4-style interconnect, etc. Plus, most of the CONFIG_ARCH_OMAP3 code in our codebase is very 3430/3630-specific, like the PM code.

This would seem to suggest that we should use CONFIG_ARCH_OMAP4. But that option currently assumes an OMAP4-style MPU subsystem: SMP Cortex-A9, PL310, etc. None of that is true for AM335x.

So first we have to decide whether the CONFIG_ARCH_OMAP* options should have a strong dependency on the MPU type, as they currently do; or whether they should focus on the way the SoC is integrated.

Hi Paul,

Quoted 25 lines · Paul Walmsley

Quoted 5 lines · Vaibhav

I will be waiting for your comment and conformation on, which family AM33xx
device should fall in? Please refer to the mail-chain -

http://www.mail-archive.com/linux-omap at vger.kernel.org/msg67275.html

This decision turns out to be pretty important; it certainly affects the AM33xx PRM/CM/clockdomain/powerdomain patches that I've been looking at.

Here is my suggestion, based on our previous practice. ?I am not so sure that it is the best way, but it seems pretty reasonable"

Using CONFIG_ARCH_OMAP3 for CONFIG_ARCH_OMAPAM33XX doesn't make sense, as far as I can tell. ?The main area of similarity between the other CONFIG_ARCH_OMAP3 and AM33xx is the Cortex-A8. ?And even that MPU subsystem is quite different between, say, the 3430/3630 and the AM33xx. Most of the AM33xx is quite different from the 3430/3630: OMAP4-style PRCM interface, OMAP4-style interconnect, etc. ?Plus, most of the CONFIG_ARCH_OMAP3 code in our codebase is very 3430/3630-specific, like the PM code.

Quoted 7 lines

This would seem to suggest that we should use CONFIG_ARCH_OMAP4. ?But that option currently assumes an OMAP4-style MPU subsystem: SMP Cortex-A9, PL310, etc. ?None of that is true for AM335x.

So first we have to decide whether the CONFIG_ARCH_OMAP* options should have a strong dependency on the MPU type, as they currently do; or whether they should focus on the way the SoC is integrated.

I think as well that these devices should be considered as part of the OMAP4 family.
The CPU type should not be the parameter that decide the OMAP family, it is just an IP, that can change on some variant, whereas the whole PRCM infrastructure / interconnect is mostly the same.

Quoted 7 lines · Benoit

Hi Paul,

Quoted 3 lines · Paul Walmsley

On Fri, 27 Apr 2012, Hiremath, Vaibhav wrote:

Quoted 11 lines

Quoted 3 lines

So first we have to decide whether the CONFIG_ARCH_OMAP* options should have a strong dependency on the MPU type, as they currently do; or whether they should focus on the way the SoC is integrated.

I think as well that these devices should be considered as part of the OMAP4 family.
The CPU type should not be the parameter that decide the OMAP family, it is just an IP, that can change on some variant, whereas the whole PRCM infrastructure / interconnect is mostly the same.

I agree. In fact more and more I think of this problem, looks like we should get rid
off ARCH_OMAP* and just is SOC_* going forward.

Common ARM code already takes care of different CPU version and as Benoit mentioned it is just one of the IP in the entire SOC. I saw tony commenting in similarly on of the patch set.

Hello Vaibhav,

I've reworked these patches somewhat to separate the PRM and CM changes into their own patches. Also, the omap_hwmod changes have been separated and moved into a different series, as have the clock tree changes.

So now, pending for 3.5, there are the SCM, voltagedomain, PRM, CM, powerdomain, and clockdomain changes. Under the (probably incorrect) assumption that Tony will opt to create a CONFIG_ARCH_OMAPAM33XX, you might wish to take a look at the 'am33xx_prcm_devel_3.5' branch of
git://git.pwsan.com/linux-2.6. It is based on four patches, not yet upstream, which create the appropriate board file, Kconfig options, etc.

Once we're able to reach a conclusion as to which of the CONFIG_ARCH_OMAP* options the AM33xx belongs under, this branch will be updated and reposted, and if everyone is happy with it, proposed for pulling.

Hello Vaibhav,

I've reworked these patches somewhat to separate the PRM and CM changes into their own patches. Also, the omap_hwmod changes have been separated and moved into a different series, as have the clock tree changes.

So now, pending for 3.5, there are the SCM, voltagedomain, PRM, CM, powerdomain, and clockdomain changes. Under the (probably incorrect) assumption that Tony will opt to create a CONFIG_ARCH_OMAPAM33XX, you might wish to take a look at the 'am33xx_prcm_devel_3.5' branch of
git://git.pwsan.com/linux-2.6. It is based on four patches, not yet upstream, which create the appropriate board file, Kconfig options, etc.

Once we're able to reach a conclusion as to which of the CONFIG_ARCH_OMAP* options the AM33xx belongs under, this branch will be updated and reposted, and if everyone is happy with it, proposed for pulling.

After some healthy discussion, now we have come to the conclusion and decided to handle AM33XX PRM/CM part separately; as AM33XX-PRCM module is different than OMAP3 and OMAP4 architecture.

The difference becomes very interesting/weird when it comes to the consistency for register offsets in PRM address space and bit-field offsets inside PRM registers,
So along with Powerdomain data and PRM api's required for AM33XX device, this patch series adds,

- XXX_RSTST register offset to "struct omap_hwmod_omap4_prcm"
- PWRSTCTRL & PWRSTST register offsets to "struct powerdomain"
- Logicretstate and mem_on/ret/pwrst/retst mask to "struct powerdomain"

Testing: This patch series has been boot tested on AM37xEVM and AM335x
based BeagleBone community board.

Thanks to Paul here...for helping and concluding on this, shortly I will submit similar patch for CM, clockdomain and clock-tree support for AM33xx.

This patch-series is created on top of "linux-omap/cleanup" branch, and also gets applied to "linux-omap/master" branch.
The patches are also available at -
https://github.com/hvaibhav/am335x-linux/tree/am335x-prm-cm

Changes from previous versions:
===============================
From V3:
- No code change, only added Voltagedomain patch (from V2 series) to this series.

From V1 & V2:
- Rolled back to my original approach, where AM33xx device was handled separately (RFC version).
- As per Paul's comments, added Register offsets & bit-fields masks.

Vaibhav Hiremath (4):
ARM: OMAP3+: am33xx: Add voltage domain data
ARM: OMAP3/4: omap_hwmod: Add rstst_off field to struct
omap_hwmod_omap4_prcm
ARM: OMAP2+: powerdomain: Add offset & mask fields to struct
powerdomain
ARM: OMAP3+: am33xx: Add powerdomain & PRM support

 arch/arm/mach-omap2/Makefile                  |    6 +
 arch/arm/mach-omap2/io.c                      |    2 +
 arch/arm/mach-omap2/omap_hwmod.c              |   32 ++-
 arch/arm/mach-omap2/powerdomain.h             |   23 ++-
 arch/arm/mach-omap2/powerdomain33xx.c         |  230 ++++++++++++++++
 arch/arm/mach-omap2/powerdomains33xx_data.c   |  185 +++++++++++++
 arch/arm/mach-omap2/prm-regbits-33xx.h        |  357 +++++++++++++++++++++++++
 arch/arm/mach-omap2/prm33xx.c                 |  134 +++++++++
 arch/arm/mach-omap2/prm33xx.h                 |  129 +++++++++
 arch/arm/mach-omap2/voltage.h                 |    1 +
 arch/arm/mach-omap2/voltagedomains33xx_data.c |   43 +++

 arch/arm/plat-omap/include/plat/omap_hwmod.h  |    2 +
 12 files changed, 1139 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/mach-omap2/powerdomain33xx.c
 create mode 100644 arch/arm/mach-omap2/powerdomains33xx_data.c
 create mode 100644 arch/arm/mach-omap2/prm-regbits-33xx.h
 create mode 100644 arch/arm/mach-omap2/prm33xx.c
 create mode 100644 arch/arm/mach-omap2/prm33xx.h
 create mode 100644 arch/arm/mach-omap2/voltagedomains33xx_data.c