On 02/11/2016 01:27 PM, Paul Walmsley wrote:

Hi Kishon, Suman,

On Wed, 10 Feb 2016, Kishon Vijay Abraham I wrote:

quoted

On Wednesday 10 February 2016 07:12 AM, Suman Anna wrote:

quoted

On 02/09/2016 01:36 PM, Paul Walmsley wrote:

quoted

On Tue, 9 Feb 2016, Suman Anna wrote:

quoted

On 02/09/2016 02:49 AM, Paul Walmsley wrote:

quoted

On Mon, 8 Feb 2016, Suman Anna wrote:

quoted

On 02/07/2016 08:48 PM, Paul Walmsley wrote:

quoted

On Tue, 2 Feb 2016, Kishon Vijay Abraham I wrote:

quoted

Paul, what do you think is the best way forward to perform reset?

Many of the IP blocks with PRM hardreset lines are processor IP blocks. 
Those often need special reset handling to ensure that WFI/HLT-like 
instructions are executed after reset.  This special handling ensures that 
the IP blocks' bus initiator interfaces indicate that they are in standby 
to the PRCM - thus allowing power management for the rest of the chip to 
work correctly.

But that doesn't seem to be the case with PCIe - and maybe others - 
possibly some of the MMUs?  

Yeah, the sequencing between clocks and resets would still be the same
for MMUs, so, adding the custom flags for MMUs is fine.

I'm curious as to whether HWMOD_CUSTOM_HARDRESET is needed for the MMUs.  
We've stated that the main point of the custom hardreset code is to handle 
processors that need to be placed into WFI/HLT, but it doesn't seem like 
there would be an equivalent for MMUs.  Thoughts?

The current OMAP IOMMU code already leverages the pdata ops for
performing the resets, so not adding the flags would also require
additional changes in the driver.

Also, the reset lines controlling the MMUs actually also manage the
reset for all the other sub-modules other than the processor cores
within the sub-systems. We have currently different issues (see [1] for
eg. around the IPU sub-system entering RET in between), so from a PM
point of view, we do prefer to place the MMUs also in reset when we are
runtime suspended.

Should we reassert hardreset in _idle() for IP blocks that don't have 
HWMOD_CUSTOM_HARDRESET set on them?  Would that allow us to use this 
mechanism for the uncore hardreset lines, or are there other quirks?

Also - would that address the potential issue that you mentioned with the 
PCIe block, or is that a different issue?

Yeah, I think that would address the PCIe block issue in terms of reset
state balancing between pm_runtime_get_sync() and pm_runtime_put()
calls. Right now, they are unbalanced. The PCIe block is using these
only in probe and remove, so it should work for that IP.

As I mentioned before this would result in undesired behavior during
suspend/resume cycle in PCIe. (This should be okay for the current mainline
code but would break once we add suspend/resume support for PCIe).

I'd like to understand where we're currently at here.  It looks like we're 
waiting for testing from Suman, and we're waiting for Kishon to try using 
the bind/unbind driver model hook to see if that wedges PCIe?  Does this 
match your collective understanding of the status here?

Matches mine :)

For MMUs and (out of tree) OMAP remoteprocs, my current sequence is
omap_device_deassert_hardreset() followed by pm_runtime_get_sync() or
omap_device_enable() during booting, and pm_runtime_put_sync() or
omap_device_idle() followed by omap_device_assert_hardreset(). Atleast
they are bunched together.

So, the current code does _deassert_hardreset twice when invoking the
pm_runtime_get_sync() in my driver since the check for
_are_all_hardreset_lines_asserted(oh) would fail.

Thinking about the question of what to do about hardreset assertion in 
idle, if we need it, we could add a hwmod flag to control that mode.  I 
would consider it a temporary workaround until we have the hwmod code 
moved into a bus driver and the bus driver/hwmod code can hook into the 
LDM .remove operation (and connect it to .shutdown, etc.)  Suman/Kishon: 
is it your understanding that we could remove the existing hardreset 
control in the IOMMU drivers and the PCIe driver if we had these options 
in the hwmod code?

For MMUs/processors, the position where we deassert the reset becomes
important. It has to be after the clocks are enabled (which is why half
of the _deassert_hardreset code looks like the code sequence in _enable()).

regards
Suman

Dave, any further comments here?