Re: [PATCH v18 08/11] media: qcom: camss: tpg: Add support for v2.4.0 TPG - sashiko-bot

From: sashiko-bot@kernel.org
To: "Hangxiang Ma" <hangxiang.ma@oss.qualcomm.com>
Cc: media-ci@linuxtv.org, vkoul@kernel.org,
	linux-phy@lists.infradead.org, olteanv@gmail.com,
	robh@kernel.org, neil.armstrong@linaro.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v18 08/11] media: qcom: camss: tpg: Add support for v2.4.0 TPG
Date: Sat, 03 Oct 2026 09:16:10 +0000	[thread overview]
Message-ID: <20261003091610.BC9351F0089B@smtp.kernel.org> (raw)
In-Reply-To: <20261003-kaanapali-camss-v18-8-87ba4ebe0db1@oss.qualcomm.com>

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

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

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

Add support for the TPG found on Kaanapali. This TPG uses hardware
version 2.4.0, which drives test-enable and reset through a separate
TPG_CTRL_CMD register instead of TPG_CTRL, and routes its output into
the CSID 1080 RX via the CSI2 TPG mux.

> 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)

[Severity: Medium]
Does this newly added check incorrectly evaluate to true on the first power
cycle due to tpg->hw_version being uninitialized?

Looking at tpg_set_power() in camss-tpg.c, the reset callback is invoked
before hw_version is populated:

tpg_set_power() {
    ...
    tpg->res->hw_ops->reset(tpg);
    tpg->res->hw_ops->hw_version(tpg);
    ...
}

Since tpg->hw_version is 0 during the first power cycle, this condition
will evaluate to true even for v2.4.0+ hardware. This executes the legacy
fallback reset writel(0, tpg->base + TPG_CTRL), which this patch explicitly
intended to avoid for newer hardware.

Can this unintended write prior to TPG_CTRL_CMD_HW_RESET cause issues on
v2.4.0 hardware?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003-kaanapali-camss-v18-0-87ba4ebe0db1@oss.qualcomm.com?part=8

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