Re: [PATCH v17 08/11] 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: camss: uninitialized tpg->hw_version used in tpg_reset()
--

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

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

This commit adds support for the TPG hardware version 2.4.0 found on
Kaanapali. It updates the configuration and reset logic to use the new
TPG_CTRL_CMD register instead of 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 d29de5f93c18e..770a9e5d5ba5f 100644
> --- a/drivers/media/platform/qcom/camss/camss-tpg-gen1.c
> +++ b/drivers/media/platform/qcom/camss/camss-tpg-gen1.c

[ ... ]

> @@ -164,18 +167,30 @@ 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);
> +	/*
> +	 * On TPG older than v2.4.0 test-enable lives in TPG_CTRL, so clear it
> +	 * first; v2.4.0+ drives both test-enable and reset through TPG_CTRL_CMD.
> +	 */
> +	if (tpg->hw_version < TPG_HW_VER_2_4_0)
> +		writel(0, tpg->base + TPG_CTRL);

[Severity: Medium]
Will this execute the legacy reset logic on v2.4.0+ hardware during the
first power-on sequence?

When the driver powers up the device, tpg_set_power() calls the reset
function before calling hw_version():

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

Because the tpg object is zero-initialized and tpg->hw_version is not
populated until after the reset function is called, tpg->hw_version
is strictly 0 during the first execution of tpg_reset().

Does this mean the condition here evaluates to true for all hardware
versions on the initial power-up, inadvertently causing an unintended
write to TPG_CTRL?

> +
> +	writel(TPG_CTRL_CMD_HW_RESET, tpg->base + TPG_CTRL_CMD);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928-kaanapali-camss-v17-0-dcf3fd37f76c@oss.qualcomm.com?part=8

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy