Re: [PATCH v22 10/14] media: qcom: camss: tpg: Add support for v2.4.0 TPG

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: qcom: camss: uninitialized hw_version check in tpg_reset()
--

commit 0810eafd7af8a3c90956a80a5132187e634531e5
Author: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>

media: qcom: camss: tpg: Add support for v2.4.0 TPG

This patch adds support for TPG hardware version 2.4.0 on Kaanapali, using
the TPG_CTRL_CMD register for test-enable and reset, routing TPG output
through the CSID 1080 CSI2 TPG mux, and wiring up the TPG resources for
Kaanapali.

> diff --git a/drivers/media/platform/qcom/camss/camss-tpg-gen1.c b/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
> index d29de5f93c18e..cabbb2638cf38 100644
> --- a/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
> +++ b/drivers/media/platform/qcom/camss/camss-tpg-gen1.c

[ ... ]

> @@ -164,18 +167,26 @@ static int tpg_stream_on(struct tpg_device *tpg)

[ ... ]

>  static int tpg_reset(struct tpg_device *tpg)
>  {
> -	writel(0, tpg->base + TPG_CTRL);
> -	writel(1, tpg->base + TPG_CLEAR);
> +	if (tpg->hw_version < TPG_HW_VER_2_4_0)
> +		writel(0, tpg->base + TPG_CTRL);
> +
> +	writel(TPG_CTRL_CMD_HW_RESET, tpg->base + TPG_CTRL_CMD);
>  
>  	return 0;
>  }

[Severity: Medium]
Is tpg->hw_version initialized when tpg_reset() is called for the first
time?

In drivers/media/platform/qcom/camss/camss-tpg.c, tpg_set_power() calls
reset() before calling hw_version():

drivers/media/platform/qcom/camss/camss-tpg.c:tpg_set_power() {
	if (on) {
		...
		tpg->res->hw_ops->reset(tpg);

		tpg->res->hw_ops->hw_version(tpg);
	}
}

Since tpg->hw_version is only populated in tpg_hw_version() and starts at 0
from the initial devm_kcalloc() in camss_probe(), won't the first power-on
of a v2.4.0 TPG evaluate 0 < TPG_HW_VER_2_4_0 as true and still write 0 to
TPG_CTRL?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024-kaanapali-camss-v22-0-7da3ea49c8a1@oss.qualcomm.com?part=10