| Message ID | 20200615152054.6819-6-ulf.hansson@linaro.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | cpuidle: psci: Various improvements for PSCI PM domains | expand |
On Mon, Jun 15, 2020 at 8:21 AM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > Depending on the SoC/platform, additional devices may be part of the PSCI > PM domain topology. This is the case with 'qcom,rpmh-rsc' device, for > example, even if this is not yet visible in the corresponding DTS-files. > > Without going into too much details, a device like the 'qcom,rpmh-rsc' may > have HW constraints that needs to be obeyed to, before a domain idlestate > can be picked. > > Therefore, let's implement the ->sync_state() callback to receive a > notification when all consumers of the PSCI PM domain providers have been > attached/probed to it. In this way, we can make sure all constraints from > all relevant devices, are taken into account before allowing a domain > idlestate to be picked. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > drivers/cpuidle/cpuidle-psci-domain.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c > index bf527d2bb4b6..b6e9649ab0da 100644 > --- a/drivers/cpuidle/cpuidle-psci-domain.c > +++ b/drivers/cpuidle/cpuidle-psci-domain.c > @@ -27,6 +27,7 @@ struct psci_pd_provider { > }; > > static LIST_HEAD(psci_pd_providers); > +static bool psci_pd_allow_domain_state; Is there ever only 1 device that's probed by this driver? If yes, this is okay. Otherwise, you'll need to handle this on a per device basis. -Saravana
On Mon, 15 Jun 2020 at 20:06, Saravana Kannan <saravanak@google.com> wrote: > > On Mon, Jun 15, 2020 at 8:21 AM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > Depending on the SoC/platform, additional devices may be part of the PSCI > > PM domain topology. This is the case with 'qcom,rpmh-rsc' device, for > > example, even if this is not yet visible in the corresponding DTS-files. > > > > Without going into too much details, a device like the 'qcom,rpmh-rsc' may > > have HW constraints that needs to be obeyed to, before a domain idlestate > > can be picked. > > > > Therefore, let's implement the ->sync_state() callback to receive a > > notification when all consumers of the PSCI PM domain providers have been > > attached/probed to it. In this way, we can make sure all constraints from > > all relevant devices, are taken into account before allowing a domain > > idlestate to be picked. > > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > --- > > drivers/cpuidle/cpuidle-psci-domain.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c > > index bf527d2bb4b6..b6e9649ab0da 100644 > > --- a/drivers/cpuidle/cpuidle-psci-domain.c > > +++ b/drivers/cpuidle/cpuidle-psci-domain.c > > @@ -27,6 +27,7 @@ struct psci_pd_provider { > > }; > > > > static LIST_HEAD(psci_pd_providers); > > +static bool psci_pd_allow_domain_state; > > Is there ever only 1 device that's probed by this driver? If yes, this > is okay. Otherwise, you'll need to handle this on a per device basis. There is only one device. Subnodes, may exist to describe a hierarchical description of the topology of the power-domains [1]. Kind regards Uffe [1] Documentation/devicetree/bindings/arm/psci.yaml
On Mon, Jun 15, 2020 at 11:50 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Mon, 15 Jun 2020 at 20:06, Saravana Kannan <saravanak@google.com> wrote: > > > > On Mon, Jun 15, 2020 at 8:21 AM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > > > Depending on the SoC/platform, additional devices may be part of the PSCI > > > PM domain topology. This is the case with 'qcom,rpmh-rsc' device, for > > > example, even if this is not yet visible in the corresponding DTS-files. > > > > > > Without going into too much details, a device like the 'qcom,rpmh-rsc' may > > > have HW constraints that needs to be obeyed to, before a domain idlestate > > > can be picked. > > > > > > Therefore, let's implement the ->sync_state() callback to receive a > > > notification when all consumers of the PSCI PM domain providers have been > > > attached/probed to it. In this way, we can make sure all constraints from > > > all relevant devices, are taken into account before allowing a domain > > > idlestate to be picked. > > > > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > > --- > > > drivers/cpuidle/cpuidle-psci-domain.c | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c > > > index bf527d2bb4b6..b6e9649ab0da 100644 > > > --- a/drivers/cpuidle/cpuidle-psci-domain.c > > > +++ b/drivers/cpuidle/cpuidle-psci-domain.c > > > @@ -27,6 +27,7 @@ struct psci_pd_provider { > > > }; > > > > > > static LIST_HEAD(psci_pd_providers); > > > +static bool psci_pd_allow_domain_state; > > > > Is there ever only 1 device that's probed by this driver? If yes, this > > is okay. Otherwise, you'll need to handle this on a per device basis. > > There is only one device. Subnodes, may exist to describe a > hierarchical description of the topology of the power-domains [1]. Thanks. In that case: Acked-by: Saravana Kannan <saravanak@google.com> -Saravana
diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c index bf527d2bb4b6..b6e9649ab0da 100644 --- a/drivers/cpuidle/cpuidle-psci-domain.c +++ b/drivers/cpuidle/cpuidle-psci-domain.c @@ -27,6 +27,7 @@ struct psci_pd_provider { }; static LIST_HEAD(psci_pd_providers); +static bool psci_pd_allow_domain_state; static int psci_pd_power_off(struct generic_pm_domain *pd) { @@ -36,6 +37,9 @@ static int psci_pd_power_off(struct generic_pm_domain *pd) if (!state->data) return 0; + if (!psci_pd_allow_domain_state) + return -EBUSY; + /* OSI mode is enabled, set the corresponding domain state. */ pd_state = state->data; psci_set_domain_state(*pd_state); @@ -222,6 +226,15 @@ static void psci_pd_remove_topology(struct device_node *np) psci_pd_init_topology(np, false); } +static void psci_cpuidle_domain_sync_state(struct device *dev) +{ + /* + * All devices have now been attached/probed to the PM domain topology, + * hence it's fine to allow domain states to be picked. + */ + psci_pd_allow_domain_state = true; +} + static const struct of_device_id psci_of_match[] = { { .compatible = "arm,psci-1.0" }, {} @@ -289,6 +302,7 @@ static struct platform_driver psci_cpuidle_domain_driver = { .driver = { .name = "psci-cpuidle-domain", .of_match_table = psci_of_match, + .sync_state = psci_cpuidle_domain_sync_state, }, };
Depending on the SoC/platform, additional devices may be part of the PSCI PM domain topology. This is the case with 'qcom,rpmh-rsc' device, for example, even if this is not yet visible in the corresponding DTS-files. Without going into too much details, a device like the 'qcom,rpmh-rsc' may have HW constraints that needs to be obeyed to, before a domain idlestate can be picked. Therefore, let's implement the ->sync_state() callback to receive a notification when all consumers of the PSCI PM domain providers have been attached/probed to it. In this way, we can make sure all constraints from all relevant devices, are taken into account before allowing a domain idlestate to be picked. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- drivers/cpuidle/cpuidle-psci-domain.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)