On 17/07/15 11:41, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Mon, Jul 13, 2015 at 01:39:48PM +0100, Jon Hunter wrote: >> Add support to the tegra dc driver for generic PM domains. However, >> to ensure backward compatibility with older device tree blobs ensure >> that the driver can work with or without generic PM domains. In order >> to migrate to generic PM domain infrastructure the necessary changes >> are: >> >> 1. If the "power-domains" property is present in the DT device node then >> generic PM domains is supported and pm_runtime_enable() should be >> called for the device. Furthermore, the enabling and disabling of the >> power-domain is handled via calling pm_runtime_get/put, respectively. > > The device could be PM runtime enabled even if no power-domains property > is set. Couldn't we check something more direct, like for example if the > dev->pm_domain is non-NULL? Yes could do that instead. > Perhaps it'd be worth converting the driver to use runtime PM first, and > move all the powergate and clock handling into suspend/resume functions, > and then we can conditionalize whether or not we call the legacy API iff > dev->pm_domain == NULL? May be that would be a cleaner transition than trying to do it all in one go. >> 2. To ensure that clocks are managed consistently when generic PM domains >> are used and are not used, drivers should be migrated to use the >> tegra_powergate_power_on_legacy() and tegra_powergate_power_off_legacy() >> functions instead of the current tegra_powergate_sequence_power_up() >> and tegra_powergate_power_off(). The purpose of the >> tegra_powergate_power_on_legacy() and tegra_powergate_power_off_legacy() >> APIs is to mimick the behaviour of the tegra generic power-domain code, >> such that if generic power domains are not supported the functionality >> is the same. >> >> 3. The main difference between the tegra_powergate_sequence_power_up() API >> and the tegra_powergate_power_on_legacy() is that the clock used to >> enable the powergate is not kept enabled when using the >> tegra_powergate_power_on_legacy() API. Therefore, drivers must enable >> the clocks they need after calling tegra_powergate_power_on_legacy() >> and disable these clocks before calling >> tegra_powergate_power_off_legacy(). > > This seems like it should go into the previous patch. I'm not sure I > understand why this is necessary. Wouldn't it be easier to update the > drivers to properly cope with this? We'll need to move them to runtime > PM at some point anyway, so if we do that first, we should be able to > hide all these details within the driver's suspend/resume functions > but without the need for the API churn here. I will take a look at that. I was trying to get the clock handling in the driver to be consistent when generic power domains are used and when they are not. However, may be that is not a big deal. Jon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html