Re: [PATCH 4/6] media: qcom: camss: tpg: Support TPG hardware version 2.2.0

[sashiko-bot@kernel.org]

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

commit ed70791a72b90a115dbcf4eead6d852fc6c9f986
Author: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

media: qcom: camss: tpg: Support TPG hardware version 2.2.0

This commit lowers the TPG hardware version threshold from 2.4.0 to 2.2.0 in
tpg_stream_on() and tpg_reset() to support Qualcomm SM8650 CAMSS TPG blocks,
which start test pattern generation via TPG_CTRL_CMD rather than a test enable
bit in TPG_CTRL.

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

[ ... ]

> @@ -183,7 +184,7 @@ static int tpg_stream_on(struct tpg_device *tpg)
>  
>  static int tpg_reset(struct tpg_device *tpg)
>  {
> -	if (tpg->hw_version < TPG_HW_VER_2_4_0)
> +	if (tpg->hw_version < TPG_HW_VER_2_2_0)
>  		writel(0, tpg->base + TPG_CTRL);

[Severity: Medium]
When a TPG subdevice is powered on for the first time via tpg_set_power(), is
tpg->hw_version initialized before tpg_reset() checks it?

In drivers/media/platform/qcom/camss/camss-tpg.c, tpg_set_power() invokes the
reset callback before reading and storing the hardware version via the
hw_version callback (tpg_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);
	}
	...
}

Because camss->tpg is zero-allocated in camss_probe(), tpg->hw_version is
still 0 on the initial tpg_set_power() -> tpg_reset() call. As a result,
tpg->hw_version < TPG_HW_VER_2_2_0 evaluates to true on the first power-on
even on TPG v2.2.0 and newer hardware, causing tpg_reset() to execute
writel(0, tpg->base + TPG_CTRL).

Should tpg_set_power() call hw_ops->hw_version(tpg) before hw_ops->reset(tpg)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031112203.545319-1-vladimir.zapolskiy@linaro.org?part=4