diff mbox series

[v2] media: camss: Do not attach an already attached power domain on MSM8916 platform

Message ID 20220704220814.629130-1-vladimir.zapolskiy@linaro.org (mailing list archive)
State Not Applicable
Headers show
Series [v2] media: camss: Do not attach an already attached power domain on MSM8916 platform | expand

Commit Message

Vladimir Zapolskiy July 4, 2022, 10:08 p.m. UTC
The change to dynamically allocated power domains neglected a case of
CAMSS on MSM8916 platform, where a single VFE power domain is neither
attached, linked or managed in runtime in any way explicitly.

This is a special case and it shall be kept as is, because the power
domain management is done outside of the driver, and it's very different
in comparison to all other platforms supported by CAMSS.

Fixes: 6b1814e26989 ("media: camss: Allocate power domain resources dynamically")
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
Changes from v1 to v2:
* corrected the fixed commit id, which is found on media/master

 drivers/media/platform/qcom/camss/camss.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Robert Foss July 5, 2022, 4:19 p.m. UTC | #1
On Tue, 5 Jul 2022 at 00:08, Vladimir Zapolskiy
<vladimir.zapolskiy@linaro.org> wrote:
>
> The change to dynamically allocated power domains neglected a case of
> CAMSS on MSM8916 platform, where a single VFE power domain is neither
> attached, linked or managed in runtime in any way explicitly.
>
> This is a special case and it shall be kept as is, because the power
> domain management is done outside of the driver, and it's very different
> in comparison to all other platforms supported by CAMSS.
>
> Fixes: 6b1814e26989 ("media: camss: Allocate power domain resources dynamically")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> Changes from v1 to v2:
> * corrected the fixed commit id, which is found on media/master
>
>  drivers/media/platform/qcom/camss/camss.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 932968e5f1e5..7a929f19e79b 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -1465,6 +1465,14 @@ static int camss_configure_pd(struct camss *camss)
>                 return camss->genpd_num;
>         }
>
> +       /*
> +        * If a platform device has just one power domain, then it is attached
> +        * at platform_probe() level, thus there shall be no need and even no
> +        * option to attach it again, this is the case for CAMSS on MSM8916.
> +        */
> +       if (camss->genpd_num == 1)
> +               return 0;
> +
>         camss->genpd = devm_kmalloc_array(dev, camss->genpd_num,
>                                           sizeof(*camss->genpd), GFP_KERNEL);
>         if (!camss->genpd)
> @@ -1698,6 +1706,9 @@ void camss_delete(struct camss *camss)
>
>         pm_runtime_disable(camss->dev);
>
> +       if (camss->genpd_num == 1)
> +               return;
> +
>         for (i = 0; i < camss->genpd_num; i++) {
>                 device_link_del(camss->genpd_link[i]);
>                 dev_pm_domain_detach(camss->genpd[i], true);
> --
> 2.33.0
>

Reviewed-by: Robert Foss <robert.foss@linaro.org>
Vladimir Zapolskiy Sept. 9, 2022, 12:19 p.m. UTC | #2
On 7/5/22 01:08, Vladimir Zapolskiy wrote:
> The change to dynamically allocated power domains neglected a case of
> CAMSS on MSM8916 platform, where a single VFE power domain is neither
> attached, linked or managed in runtime in any way explicitly.
> 
> This is a special case and it shall be kept as is, because the power
> domain management is done outside of the driver, and it's very different
> in comparison to all other platforms supported by CAMSS.
> 
> Fixes: 6b1814e26989 ("media: camss: Allocate power domain resources dynamically")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---

Gentle ping, it would be great to see this fix on media/fixes for 6.0.

> Changes from v1 to v2:
> * corrected the fixed commit id, which is found on media/master
> 
>   drivers/media/platform/qcom/camss/camss.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 932968e5f1e5..7a929f19e79b 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -1465,6 +1465,14 @@ static int camss_configure_pd(struct camss *camss)
>   		return camss->genpd_num;
>   	}
>   
> +	/*
> +	 * If a platform device has just one power domain, then it is attached
> +	 * at platform_probe() level, thus there shall be no need and even no
> +	 * option to attach it again, this is the case for CAMSS on MSM8916.
> +	 */
> +	if (camss->genpd_num == 1)
> +		return 0;
> +
>   	camss->genpd = devm_kmalloc_array(dev, camss->genpd_num,
>   					  sizeof(*camss->genpd), GFP_KERNEL);
>   	if (!camss->genpd)
> @@ -1698,6 +1706,9 @@ void camss_delete(struct camss *camss)
>   
>   	pm_runtime_disable(camss->dev);
>   
> +	if (camss->genpd_num == 1)
> +		return;
> +
>   	for (i = 0; i < camss->genpd_num; i++) {
>   		device_link_del(camss->genpd_link[i]);
>   		dev_pm_domain_detach(camss->genpd[i], true);

--
Best wishes,
Vladimir
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 932968e5f1e5..7a929f19e79b 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -1465,6 +1465,14 @@  static int camss_configure_pd(struct camss *camss)
 		return camss->genpd_num;
 	}
 
+	/*
+	 * If a platform device has just one power domain, then it is attached
+	 * at platform_probe() level, thus there shall be no need and even no
+	 * option to attach it again, this is the case for CAMSS on MSM8916.
+	 */
+	if (camss->genpd_num == 1)
+		return 0;
+
 	camss->genpd = devm_kmalloc_array(dev, camss->genpd_num,
 					  sizeof(*camss->genpd), GFP_KERNEL);
 	if (!camss->genpd)
@@ -1698,6 +1706,9 @@  void camss_delete(struct camss *camss)
 
 	pm_runtime_disable(camss->dev);
 
+	if (camss->genpd_num == 1)
+		return;
+
 	for (i = 0; i < camss->genpd_num; i++) {
 		device_link_del(camss->genpd_link[i]);
 		dev_pm_domain_detach(camss->genpd[i], true);