Re: [PATCH v5 4/8] phy: rockchip-samsung-dcphy: Name the transmitter helpers and ops - sashiko-bot

From: sashiko-bot@kernel.org
To: "Jason Yang" <jason98166@gmail.com>
Cc: linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org, vkoul@kernel.org,
	neil.armstrong@linaro.org, olteanv@gmail.com
Subject: Re: [PATCH v5 4/8] phy: rockchip-samsung-dcphy: Name the transmitter helpers and ops
Date: Fri, 16 Oct 2026 09:07:07 +0000	[thread overview]
Message-ID: <sashiko-outbox-174053@kernel.org> (raw)
In-Reply-To: <20261016-dcphy-rx-v1-v5-4-e63b49eabe2d@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] phy: missing PLL disable and reset deassert on tx_power_on error
--

commit 779b5f96ef2afd3f131c3123d0f3f45979f568d8
Author: Jason Yang <jason98166@gmail.com>

phy: rockchip-samsung-dcphy: Name the transmitter helpers and ops

The block is a combo PHY: it exposes a transmitter and a receiver, and a
later change registers one struct phy for each. Prepare for that. Give
every helper that programs the master lanes a tx_ prefix, and move the
phy pointer and the lane count, which only the transmitter uses, into a
struct samsung_mipi_phy that the receiver can have one of too.

> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> index 1471a1563d2a1..e4293c8d2b292 100644
> --- a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c

[ ... ]

> @@ -1339,13 +1343,13 @@ static int samsung_mipi_dphy_power_on(struct samsung_mipi_dcphy *samsung)
>  			   I_MUX_SEL_MASK, I_MUX_400MV);
>  
>  	samsung_mipi_dcphy_pll_configure(samsung);
> -	samsung_mipi_dphy_clk_lane_timing_init(samsung);
> -	samsung_mipi_dphy_data_lane_timing_init(samsung);
> +	samsung_mipi_dphy_tx_clk_lane_timing_init(samsung);
> +	samsung_mipi_dphy_tx_data_lane_timing_init(samsung);
>  	ret = samsung_mipi_dcphy_pll_enable(samsung);
>  	if (ret < 0)
>  		return ret;

[Severity: Medium]
If samsung_mipi_dcphy_pll_enable() times out, does this error path return
early without clearing the PLL_EN bit or deasserting the m_phy_rst reset line?

When samsung_mipi_dcphy_pll_enable() fails, it returns an error but the PHY is
left with the PLL_EN bit set and m_phy_rst asserted. The PHY framework does not
call phy_power_off() if phy_power_on() fails, so this hardware state could leak,
potentially preventing successful lock on future relock attempts.

>  
> -	samsung_mipi_dphy_lane_enable(samsung);
> +	samsung_mipi_dphy_tx_lane_enable(samsung);
>  
>  	reset_control_deassert(samsung->m_phy_rst);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-dcphy-rx-v1-v5-0-e63b49eabe2d@gmail.com?part=4

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