Dave Gerlach [off-list ref] writes:

On 08/08/2013 10:03 AM, Santosh Shilimkar wrote:

quoted

$subject and patch don't match.

On Thursday 08 August 2013 08:26 AM, Nishanth Menon wrote:

quoted

On 08/08/2013 03:45 AM, Russ Dill wrote:

quoted

   In reference to
the M3 handling it, the M3 wouldn't know which devices have a driver
bound and which don't.

Does it need to? M3 firmware can pretty much define "I will force
the device into low power state, and if the drivers dont handle
things properly, fix the darned driver". M3 behavior should be
considered as a "hardware" as far as Linux running on MPU is
concerned, and firmware helps change the behavior by accounting for
SoC quirks. *if* we have ability to handle this in the firmware,
there is no need to carry this in Linux.

I agree with Nishant. I don't like this patch and IIRC, I gave same
comment in the last version. Linux need not know about all such firmware
quirks. Also all these M3 specific stuff, should be done somewhere
else. Probably having a small M3 driver won't be a bad idea.

Regards,
Santosh

I am not opposed to doing it this way and letting the M3 firmware
handle idling these modules, however the one concern raised in the
last series is that an approach that does not acknowledge drivers will
hide driver PM bugs. I suppose as long as I make sure to document that
the devices are being idled by the M3 firmware this may not be an
issue. I will look into implementing this.

No, please don't start idling devices in firmware that are otherwise
managed by Linux.  Keep the firmware simple and dumb.  Linux is managing
these devices, it should manage their bugs too.

This is not just about idling devices.  This is about handling broken IP
blocks whose power-on reset state does not allow the the powerdomain to
reach its target state.  That's just bad hardware design.  

That being said, IMO, the kernel (specifically omap_device) should
handle this, and it should be rather easy to do in the omap_device layer
and keep the SoC suspend/resume core code simple and ignorant of these
"quirks."

AFAICT, there's no reason these quirks need to be dealt with immediatly
on suspend.  A slight delay should be fine, as long as it's before the
next suspend/idle attempt, right?  

Given that, what we need to do (and by we, I mean you) is to flag all
broken IP blocks, and let omap_device handle them in a suspend/resume
notifier (c.f. register_pm_notifier() and PM_POST_SUSPEND.)

That will keep things contained to the omap_device/hwmod level and allow
flexiblity for future broken SoCs where the list of broken IP blocks is
different.  Though surely this broken hardware doesn't exist in AM4xxx
because someone noticed this early on and pointed out that it should be
fixed in hardware, right?  ;)

Kevin