On 06/17/2014 11:36 AM, Mikko Perttunen wrote:

On 06/17/2014 08:04 PM, Bartlomiej Zolnierkiewicz wrote:

quoted

Hi,

On Tuesday, June 17, 2014 10:10:23 AM Stephen Warren wrote:

quoted

On 06/17/2014 06:14 AM, Bartlomiej Zolnierkiewicz wrote:

[...]

quoted

quoted

quoted

+static struct platform_driver tegra_ahci_driver = {
+    .probe = tegra_ahci_probe,
+    .remove = ata_platform_remove_one,
+    .driver = {
+        .name = "tegra-ahci",
+        .owner = THIS_MODULE,
+        .of_match_table = tegra_ahci_of_match,
+    },

This driver lacks PM suspend/resume support.  I assume that
the Tegra124 platform also doesn't support suspend/resume yet
(if so a comment in the driver code about this would be useful),
otherwise the driver should be fixed.

We do have basic system suspend/resume support. However, I don't think
missing suspend/resume in an individual driver should stop it from being
merged. It can certainly be added later.

There should be at least FIXME in the driver explaining the situation and
I would really prefer to have PM support added when the driver is still
"hot" (meaning there are people actively working on it) instead of
possibly
having to chase people months/years later when they have already moved on
and are working on something else.  Please also note that adding PM
support
should be quite simple if the driver is designed correctly.

AFAIK, the deepest level of suspend currently supported on upstream is
LP1, for which the driver doesn't need to do anything. Only when we go
to LP0 the driver will need to save/reload registers and stuff.

Yes, it's generally true that no SoC register state is lost during
suspend, only CPU state.