Re: [PATCH v4 5/7] phy: rockchip-samsung-dcphy: Factor the transmitter teardown into a helper

[Michael Riesch <michael.riesch@collabora.com>]

Hi Jason,

Thank you for your work!

On 8/21/26 14:02, Jason Yang via B4 Relay wrote:
> From: Jason Yang <jason98166@gmail.com>
> 
> The transmitter's power-on path is in samsung_mipi_dphy_tx_power_on(),
> but the power_off callback still does the teardown itself.
> 
> Move the teardown into samsung_mipi_dphy_tx_power_off(), so that
> power-on and power-off are a matching pair. Both callbacks then have a
> type switch whose arms are plain returns. Turn each into an early guard
> for the not yet supported C-PHY and drop the unreachable trailing
> return.
> 
> No functional change intended.
> 
> Signed-off-by: Jason Yang <jason98166@gmail.com>
> ---
>  drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c | 31 ++++++++++-------------
>  1 file changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> index f58907dec733..5d9d44a1d6a2 100644
> --- a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> @@ -1361,37 +1361,34 @@ static int samsung_mipi_dphy_tx_power_on(struct samsung_mipi_dcphy *samsung)
>  	return 0;
>  }
>  
> +static int samsung_mipi_dphy_tx_power_off(struct samsung_mipi_dcphy *samsung)
> +{
> +	samsung_mipi_dphy_tx_lane_disable(samsung);
> +	samsung_mipi_dcphy_pll_disable(samsung);
> +
> +	return 0;
> +}
> +
>  static int samsung_mipi_dcphy_tx_power_on(struct phy *phy)
>  {
>  	struct samsung_mipi_dcphy *samsung = phy_get_drvdata(phy);
>  
> -	switch (samsung->type) {
> -	case PHY_TYPE_DPHY:
> -		return samsung_mipi_dphy_tx_power_on(samsung);
> -	default:
> -		/* CPHY part to be implemented later */
> +	/* CPHY part to be implemented later */
> +	if (samsung->type != PHY_TYPE_DPHY)
>  		return -EOPNOTSUPP;
> -	}

What is the value of doing that? Personally I'd leave the switch.

Best regards,
Michael

>  
> -	return 0;
> +	return samsung_mipi_dphy_tx_power_on(samsung);
>  }
>  
>  static int samsung_mipi_dcphy_tx_power_off(struct phy *phy)
>  {
>  	struct samsung_mipi_dcphy *samsung = phy_get_drvdata(phy);
>  
> -	switch (samsung->type) {
> -	case PHY_TYPE_DPHY:
> -		samsung_mipi_dphy_tx_lane_disable(samsung);
> -		break;
> -	default:
> -		/* CPHY part to be implemented later */
> +	/* CPHY part to be implemented later */
> +	if (samsung->type != PHY_TYPE_DPHY)
>  		return -EOPNOTSUPP;
> -	}
>  
> -	samsung_mipi_dcphy_pll_disable(samsung);
> -
> -	return 0;
> +	return samsung_mipi_dphy_tx_power_off(samsung);
>  }
>  
>  static int
>