Re: [PATCH v2 3/3] media: camss: Link CAMSS power domain

Bryan O'Donoghue
2023-05-26T20:57:08Z

On 26/05/2023 19:07, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> The CAMSS power domain was previously enabled implicitly when the VFE
> power domains were enabled.
> Commit 46cc03175498 ("media: camss: Split power domain management")
> delayed enabling VFE power domains which in turn delayed enabling the
> CAMSS power domain. This made CSIPHY fail to enable camss_top_ahb_clk
> which requires the CAMSS power domain to be on:
>
> [ 199.097810] ------------[ cut here ]------------
> [ 199.097893] camss_top_ahb_clk status stuck at 'off'
> [ 199.097913] WARNING: CPU: 3 PID: 728 at drivers/clk/qcom/clk-branch.c:91 clk_branch_wait+0x140/0x160
> ...
> [ 199.100064] clk_branch_wait+0x140/0x160
> [ 199.100112] clk_branch2_enable+0x30/0x40
> [ 199.100159] clk_core_enable+0x6c/0xb0
> [ 199.100211] clk_enable+0x2c/0x50
> [ 199.100257] camss_enable_clocks+0x94/0xe0 [qcom_camss]
> [ 199.100342] csiphy_set_power+0x154/0x2a0 [qcom_camss]
> ...
> [ 199.101594] ---[ end trace 0000000000000000 ]---
>
> Link the CAMSS power domain in camss_configure_pd to make sure it gets
> enabled before CSIPHY tries to enable clocks.
>
> Fixes: 02afa816dbbf ("media: camss: Add basic runtime PM support")
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> drivers/media/platform/qcom/camss/camss.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 1ef26aea3eae..9aea8220d923 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -1453,6 +1453,7 @@ static const struct media_device_ops camss_media_ops = {
> static int camss_configure_pd(struct camss *camss)
> {
> struct device *dev = camss->dev;
> + int camss_pd_index;
> int i;
> int ret;
>
> @@ -1496,7 +1497,13 @@ static int camss_configure_pd(struct camss *camss)
> }
> }
>
> - if (i > camss->vfe_num) {
> + /* Link CAMSS power domain if available */
> + camss_pd_index = device_property_match_string(camss->dev, "power-domain-names", "camss");
> + if (camss_pd_index >= 0)

Surely if you get this far you already know that

camss->genpd_num = of_count_phandle_with_args(dev->of_node,
"power-domains",
"#power-domain-cells");
if (camss->genpd_num < 0) {
dev_err(dev, "Power domains are not defined for camss\n");
return camss->genpd_num;
}

So you don't need to add this additional dependency ?

---
bod