[PATCH] PM / Domains: Allow holes in genpd_data.domains array

[PATCH] PM / Domains: Allow holes in genpd_data.domains array

[Tomeu Vizoso]

In platforms such as Rockchip's, the array of domains isn't always
filled without holes, as which domains are present depend on the
particular SoC revision.

By allowing holes to be in the array, such SoCs can still use a single
set of constants to index the array of power domains.

Fixes: 0159ec670763 ("PM / Domains: Verify the PM domain is present when adding a provider")
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: Heiko Stuebner <heiko@sntech.de>
---
 drivers/base/power/domain.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index b0cf46dcae73..ce3f483ec67b 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1581,6 +1581,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 	mutex_lock(&gpd_list_lock);
 
 	for (i = 0; i < data->num_domains; i++) {
+		if (!data->domains[i])
+			continue;
 		if (!pm_genpd_present(data->domains[i]))
 			goto error;
 
-- 
2.7.4

Re: [PATCH] PM / Domains: Allow holes in genpd_data.domains array

[Jon Hunter]

On 15/09/16 11:39, Tomeu Vizoso wrote:
> In platforms such as Rockchip's, the array of domains isn't always
> filled without holes, as which domains are present depend on the
> particular SoC revision.
> 
> By allowing holes to be in the array, such SoCs can still use a single
> set of constants to index the array of power domains.
> 
> Fixes: 0159ec670763 ("PM / Domains: Verify the PM domain is present when adding a provider")
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/base/power/domain.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index b0cf46dcae73..ce3f483ec67b 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1581,6 +1581,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
>  	mutex_lock(&gpd_list_lock);
>  
>  	for (i = 0; i < data->num_domains; i++) {
> +		if (!data->domains[i])
> +			continue;
>  		if (!pm_genpd_present(data->domains[i]))
>  			goto error;
>  

Sounds reasonable and sorry I had not thought of this. Can you also make
sure we check that !data->domains[i] in the error path as well or
potentially we could have a NULL pointer dereference in the case of an
actual error.

Cheers
Jon

-- 
nvpublic

Re: [PATCH] PM / Domains: Allow holes in genpd_data.domains array

[Jon Hunter]

On 15/09/16 11:39, Tomeu Vizoso wrote:
> In platforms such as Rockchip's, the array of domains isn't always
> filled without holes, as which domains are present depend on the
> particular SoC revision.
> 
> By allowing holes to be in the array, such SoCs can still use a single
> set of constants to index the array of power domains.
> 
> Fixes: 0159ec670763 ("PM / Domains: Verify the PM domain is present when adding a provider")
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/base/power/domain.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index b0cf46dcae73..ce3f483ec67b 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1581,6 +1581,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
>  	mutex_lock(&gpd_list_lock);
>  
>  	for (i = 0; i < data->num_domains; i++) {
> +		if (!data->domains[i])
> +			continue;
>  		if (!pm_genpd_present(data->domains[i]))
>  			goto error;
>  

Sounds reasonable and sorry I had not thought of this. Can you also make
sure we check that !data->domains[i] in the error path as well or
potentially we could have a NULL pointer dereference in the case of an
actual error.

Cheers
Jon

-- 
nvpublic

[PATCH v2] PM / Domains: Allow holes in genpd_data.domains array

[Tomeu Vizoso]

In platforms such as Rockchip's, the array of domains isn't always
filled without holes, as which domains are present depend on the
particular SoC revision.

By allowing holes to be in the array, such SoCs can still use a single
set of constants to index the array of power domains.

Fixes: 0159ec670763 ("PM / Domains: Verify the PM domain is present when adding a provider")
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: Heiko Stuebner <heiko@sntech.de>

---

v2: Also skip holes in the error path.
---
 drivers/base/power/domain.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index b0cf46dcae73..83ae3d7d3fdd 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1581,6 +1581,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 	mutex_lock(&gpd_list_lock);
 
 	for (i = 0; i < data->num_domains; i++) {
+		if (!data->domains[i])
+			continue;
 		if (!pm_genpd_present(data->domains[i]))
 			goto error;
 
@@ -1598,6 +1600,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 
 error:
 	while (i--) {
+		if (!data->domains[i])
+			continue;
 		data->domains[i]->provider = NULL;
 		data->domains[i]->has_provider = false;
 	}
-- 
2.7.4

Re: [PATCH v2] PM / Domains: Allow holes in genpd_data.domains array

[Jon Hunter]

On 15/09/16 13:05, Tomeu Vizoso wrote:
> In platforms such as Rockchip's, the array of domains isn't always
> filled without holes, as which domains are present depend on the
> particular SoC revision.
> 
> By allowing holes to be in the array, such SoCs can still use a single
> set of constants to index the array of power domains.
> 
> Fixes: 0159ec670763 ("PM / Domains: Verify the PM domain is present when adding a provider")
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> 
> ---
> 
> v2: Also skip holes in the error path.
> ---
>  drivers/base/power/domain.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index b0cf46dcae73..83ae3d7d3fdd 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1581,6 +1581,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
>  	mutex_lock(&gpd_list_lock);
>  
>  	for (i = 0; i < data->num_domains; i++) {
> +		if (!data->domains[i])
> +			continue;
>  		if (!pm_genpd_present(data->domains[i]))
>  			goto error;
>  
> @@ -1598,6 +1600,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
>  
>  error:
>  	while (i--) {
> +		if (!data->domains[i])
> +			continue;
>  		data->domains[i]->provider = NULL;
>  		data->domains[i]->has_provider = false;
>  	}
> 

Thanks!

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2] PM / Domains: Allow holes in genpd_data.domains array

[Jon Hunter]

On 15/09/16 13:05, Tomeu Vizoso wrote:
> In platforms such as Rockchip's, the array of domains isn't always
> filled without holes, as which domains are present depend on the
> particular SoC revision.
> 
> By allowing holes to be in the array, such SoCs can still use a single
> set of constants to index the array of power domains.
> 
> Fixes: 0159ec670763 ("PM / Domains: Verify the PM domain is present when adding a provider")
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> 
> ---
> 
> v2: Also skip holes in the error path.
> ---
>  drivers/base/power/domain.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index b0cf46dcae73..83ae3d7d3fdd 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1581,6 +1581,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
>  	mutex_lock(&gpd_list_lock);
>  
>  	for (i = 0; i < data->num_domains; i++) {
> +		if (!data->domains[i])
> +			continue;
>  		if (!pm_genpd_present(data->domains[i]))
>  			goto error;
>  
> @@ -1598,6 +1600,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
>  
>  error:
>  	while (i--) {
> +		if (!data->domains[i])
> +			continue;
>  		data->domains[i]->provider = NULL;
>  		data->domains[i]->has_provider = false;
>  	}
> 

Thanks!

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2] PM / Domains: Allow holes in genpd_data.domains array

[Ulf Hansson]

On 15 September 2016 at 14:05, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> In platforms such as Rockchip's, the array of domains isn't always
> filled without holes, as which domains are present depend on the
> particular SoC revision.
>
> By allowing holes to be in the array, such SoCs can still use a single
> set of constants to index the array of power domains.
>
> Fixes: 0159ec670763 ("PM / Domains: Verify the PM domain is present when adding a provider")
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Heiko Stuebner <heiko@sntech.de>

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

>
> ---
>
> v2: Also skip holes in the error path.
> ---
>  drivers/base/power/domain.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index b0cf46dcae73..83ae3d7d3fdd 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1581,6 +1581,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
>         mutex_lock(&gpd_list_lock);
>
>         for (i = 0; i < data->num_domains; i++) {
> +               if (!data->domains[i])
> +                       continue;
>                 if (!pm_genpd_present(data->domains[i]))
>                         goto error;
>
> @@ -1598,6 +1600,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
>
>  error:
>         while (i--) {
> +               if (!data->domains[i])
> +                       continue;
>                 data->domains[i]->provider = NULL;
>                 data->domains[i]->has_provider = false;
>         }
> --
> 2.7.4
>

Re: [PATCH v2] PM / Domains: Allow holes in genpd_data.domains array

[Heiko Stübner]

Am Donnerstag, 15. September 2016, 14:05:23 schrieb Tomeu Vizoso:
> In platforms such as Rockchip's, the array of domains isn't always
> filled without holes, as which domains are present depend on the
> particular SoC revision.
> 
> By allowing holes to be in the array, such SoCs can still use a single
> set of constants to index the array of power domains.
> 
> Fixes: 0159ec670763 ("PM / Domains: Verify the PM domain is present when
> adding a provider") Signed-off-by: Tomeu Vizoso
> <tomeu.vizoso@collabora.com>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Heiko Stuebner <heiko@sntech.de>

We talked about this today and the change looks good for handling the
holes in Rockchip power-domain lists, so

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

also, on a rk3288-veyron Chromebook the display comes up again
with this patch (by not defering because of the missing domains), so

Tested-by: Heiko Stuebner <heiko@sntech.de>


Thanks
Heiko

Re: [PATCH v2] PM / Domains: Allow holes in genpd_data.domains array

[Kevin Hilman]

Tomeu Vizoso <tomeu.vizoso@collabora.com> writes:

> In platforms such as Rockchip's, the array of domains isn't always
> filled without holes, as which domains are present depend on the
> particular SoC revision.
>
> By allowing holes to be in the array, such SoCs can still use a single
> set of constants to index the array of power domains.
>
> Fixes: 0159ec670763 ("PM / Domains: Verify the PM domain is present when adding a provider")
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
>
> ---
>
> v2: Also skip holes in the error path.

Acked-by: Kevin Hilman <khilman@baylibre.com>