[02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

Enable the AESS auto-gating control bit during AESS hwmod setup.  This
fixes the following boot warning on OMAP4:

omap_hwmod: aess: _wait_target_disable failed

Without this patch, the AESS IP block does not indicate to the PRCM
that it is idle after it is reset.  This prevents some types of SoC
power management until something sets the auto-gating control bit.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
Cc: Péter Ujfalusi <peter.ujfalusi@ti.com>
---
 arch/arm/mach-omap2/Makefile               |    2 +
 arch/arm/mach-omap2/aess.c                 |   55 ++++++++++++++++++++++++++++
 arch/arm/mach-omap2/common.h               |    2 +
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    2 +
 4 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-omap2/aess.c

* Paul Walmsley <paul@pwsan.com> [120606 23:26]:
> Enable the AESS auto-gating control bit during AESS hwmod setup.  This
> fixes the following boot warning on OMAP4:
> 
> omap_hwmod: aess: _wait_target_disable failed
> 
> Without this patch, the AESS IP block does not indicate to the PRCM
> that it is idle after it is reset.  This prevents some types of SoC
> power management until something sets the auto-gating control bit.

I don't like the idea of having custom platform init code for every driver
to reset it, let's see if there's some better way to deal with stuff like
this.

It seems that most/many IP blocks need their custom reset hacks, and it's
not limited to just few instances?

Maybe we can distribute custom hacks like this to the drivers? If there is
no generic way to reset some IP block, then the driver should pass it's
whatever workaround bits/methdod/ to the bus level (hwmod) code during the
init rather than piling up all the possible hacks to the bus level code.

This way the driver init can still do the bus level reset during it's init,
and bail out after that if the module is not in use for the board in question.
That is already being flagged in device tree with status = "disable" flag,
and can also passed in platform data if necessary.

AFAIK there's no need to reset the IP blocks before the driver init, it's
really needed for PM. So it's not needed early on, and it's OK to require
running  the driver init for driver modules that are not in use to reset
them properly. After all, the hardware is on the device, even if it's not
being used.

Regards,

Tony

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> It seems that most/many IP blocks need their custom reset hacks, and 
> it's not limited to just few instances?

Only four out of the fifty-seven omap_hwmod_classes defined in 
mach-omap2/omap_hwmod_44xx_data.c after this series have custom reset 
functions used:

$ fgrep 'struct omap_hwmod_class ' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
57
$ fgrep '.reset' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
4

That's 7% of the classes.  In terms of the total number of IP block 
instances that use custom reset functions viewed against the total number 
of instances on the chip, the percentage is even smaller.

> AFAIK there's no need to reset the IP blocks before the driver init, 
> it's really needed for PM. So it's not needed early on, and it's OK to 
> require running the driver init for driver modules that are not in use 
> to reset them properly. After all, the hardware is on the device, even 
> if it's not being used.

I don't think I'm following you.  It's not just PM; the problem is also 
with kexec or buggy bootloaders.  If an IP block isn't reset when the 
kernel boots, and is doing DMA or anything else that could affect the 
reset of the system, it could easily cause unpredictable behavior or 
crashes in unrelated kernel code.

It's also worth mentioning that many IP blocks, such as AESS, don't have 
Linux drivers.


- Paul

* Paul Walmsley <paul@pwsan.com> [120607 00:35]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > It seems that most/many IP blocks need their custom reset hacks, and 
> > it's not limited to just few instances?
> 
> Only four out of the fifty-seven omap_hwmod_classes defined in 
> mach-omap2/omap_hwmod_44xx_data.c after this series have custom reset 
> functions used:
> 
> $ fgrep 'struct omap_hwmod_class ' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
> 57
> $ fgrep '.reset' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
> 4
> 
> That's 7% of the classes.  In terms of the total number of IP block 
> instances that use custom reset functions viewed against the total number 
> of instances on the chip, the percentage is even smaller.

OK so that's not too bad then. But there's also the omap2_wd_timer_disable
pre_shutdown too. And there's also the sysconfig autoidle bit for each driver
that we're tweaking in the bus level code?

If we can remove the ioremapping and accessing driver registers in the bus
level code things get much simpler for the bus level code.

> > AFAIK there's no need to reset the IP blocks before the driver init, 
> > it's really needed for PM. So it's not needed early on, and it's OK to 
> > require running the driver init for driver modules that are not in use 
> > to reset them properly. After all, the hardware is on the device, even 
> > if it's not being used.
> 
> I don't think I'm following you.  It's not just PM; the problem is also 
> with kexec or buggy bootloaders.  If an IP block isn't reset when the 
> kernel boots, and is doing DMA or anything else that could affect the 
> reset of the system, it could easily cause unpredictable behavior or 
> crashes in unrelated kernel code.

Still sounds like the responsibility of the bootloaders and drivers to
set things up properly, that's the standard behaviour.
 
> It's also worth mentioning that many IP blocks, such as AESS, don't have 
> Linux drivers.

Sounds like it should have a minimal driver then, just to reset it if
nothing else.

Regards,

Tony

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> OK so that's not too bad then. But there's also the 
> omap2_wd_timer_disable pre_shutdown too. And there's also the sysconfig 
> autoidle bit for each driver that we're tweaking in the bus level code?

I think I lost your point here.  The ioremap() issue is separate from the 
reset functions, etc., in my view.  Moving the reset functions out to 
drivers/ seems potentially more reasonable than dropping the ioremap().

> If we can remove the ioremapping and accessing driver registers in the 
> bus level code things get much simpler for the bus level code.

That's like saying if PCI Configuration Header handling were to be moved 
into the driver code, then the PCI bus-level code would be much simpler 
:-)

The hwmod code ioremaps the device registers to handle the 
integration-level registers at the beginning of the device's address 
space.  These registers can be thought of as part of the PRCM, not part of 
the IP block.  It would have been better if TI had put these integration 
registers in a separate address space like PCI does.  But we are stuck 
with the existing hardware design.  The integration registers also differ 
from chip to chip even with the same underlying IP block, see for example 
the 32k sync timer.

The main reasons why these integration registers are handled now in common 
code are:

1. to avoid duplicating integration code between lots of different drivers 
   that is unrelated to the driver itself, such as bus-level reset

2. to ensure consistency of the OCP registers with the rest of the PM
   state

3. to avoid callbacks into drivers that might otherwise be needed for 
   bitfields like CLOCKACTIVITY 

4. to make it easier to debug integration problems with drivers

If we don't handle those registers in common code, the number of SoC 
integration workarounds that need to be placed into the drivers will 
increase.  For example, when OMAP4 added the smart-idle-with-wakeup and 
smart-standby-with-wakeup OCP idle modes, only a couple of files needed to 
be changed.  If those integration-level details were still in the drivers, 
a large number of files would need to be changed.  And $DEITY help us if 
the code sequence for dealing with those bits were to ever change in the 
future - we'd need to change a bunch of drivers, rather than just one or 
two files.  Also some people are going to need to audit the driver code 
from an integration level pretty carefully for PM to work consistently.

I suppose one option, if we were to have a real omap_device, would be to 
define callbacks for each driver to implement that would read and write 
the OCP header registers.  Then the omap_bus code could call those 
callbacks to handle the OCP register accesses, when called from the 
driver's PM runtime calls.  Adds another layer of indirection, but would 
localize IP block register accesses to the IP block's driver.

...

As far as the reset and preconfiguration aspects of the hwmod code go, 
they just happen to be possible since we're doing the ioremap anyway.  It 
can be ensured that no matter what drivers are present, or what the 
bootloader or previous OS did or didn't do, a minimal kernel should behave 
predictably.

It seems like it might be reasonable to move these to some built-in driver 
shim layer as you suggest in your other E-mail.  But that is assuming that 
it can be made to work without needless layers of indirection.  I don't 
know of any driver that does this now.  Maybe you know of one?


- Paul

* Paul Walmsley <paul@pwsan.com> [120607 03:49]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > OK so that's not too bad then. But there's also the 
> > omap2_wd_timer_disable pre_shutdown too. And there's also the sysconfig 
> > autoidle bit for each driver that we're tweaking in the bus level code?
> 
> I think I lost your point here.  The ioremap() issue is separate from the 
> reset functions, etc., in my view.  Moving the reset functions out to 
> drivers/ seems potentially more reasonable than dropping the ioremap().
> 
> > If we can remove the ioremapping and accessing driver registers in the 
> > bus level code things get much simpler for the bus level code.
> 
> That's like saying if PCI Configuration Header handling were to be moved 
> into the driver code, then the PCI bus-level code would be much simpler 
> :-)
> 
> The hwmod code ioremaps the device registers to handle the 
> integration-level registers at the beginning of the device's address 
> space.  These registers can be thought of as part of the PRCM, not part of 
> the IP block.  It would have been better if TI had put these integration 
> registers in a separate address space like PCI does.  But we are stuck 
> with the existing hardware design.  The integration registers also differ 
> from chip to chip even with the same underlying IP block, see for example 
> the 32k sync timer.

Yes having these registers in the device address space sucks.
 
> The main reasons why these integration registers are handled now in common 
> code are:
> 
> 1. to avoid duplicating integration code between lots of different drivers 
>    that is unrelated to the driver itself, such as bus-level reset
> 
> 2. to ensure consistency of the OCP registers with the rest of the PM
>    state
> 
> 3. to avoid callbacks into drivers that might otherwise be needed for 
>    bitfields like CLOCKACTIVITY 
> 
> 4. to make it easier to debug integration problems with drivers

Sure that all makes sense.
 
> If we don't handle those registers in common code, the number of SoC 
> integration workarounds that need to be placed into the drivers will 
> increase.  For example, when OMAP4 added the smart-idle-with-wakeup and 
> smart-standby-with-wakeup OCP idle modes, only a couple of files needed to 
> be changed.  If those integration-level details were still in the drivers, 
> a large number of files would need to be changed.  And $DEITY help us if 
> the code sequence for dealing with those bits were to ever change in the 
> future - we'd need to change a bunch of drivers, rather than just one or 
> two files.  Also some people are going to need to audit the driver code 
> from an integration level pretty carefully for PM to work consistently.
> 
> I suppose one option, if we were to have a real omap_device, would be to 
> define callbacks for each driver to implement that would read and write 
> the OCP header registers.  Then the omap_bus code could call those 
> callbacks to handle the OCP register accesses, when called from the 
> driver's PM runtime calls.  Adds another layer of indirection, but would 
> localize IP block register accesses to the IP block's driver.

Yes there may be some way to deal with this cleanly while keeping the
driver registers in the drivers. Maybe inline functions in the driver
headers that hwmod code could include would be enough here.
 
> ...
> 
> As far as the reset and preconfiguration aspects of the hwmod code go, 
> they just happen to be possible since we're doing the ioremap anyway.  It 
> can be ensured that no matter what drivers are present, or what the 
> bootloader or previous OS did or didn't do, a minimal kernel should behave 
> predictably.
> 
> It seems like it might be reasonable to move these to some built-in driver 
> shim layer as you suggest in your other E-mail.  But that is assuming that 
> it can be made to work without needless layers of indirection.  I don't 
> know of any driver that does this now.  Maybe you know of one?

Yes good point, see the suggestion on the driver header inline functions
in the other email I just sent.

Regards,

Tony