Re: [PATCH v5 3/8] phy: rockchip-samsung-dcphy: Move block-level setup to runtime resume

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

Hi Jason,

Thanks for the patch!

On 10/16/26 10:56, Jason Yang wrote:
> The APB reset and the BIAS references have to be treated as block-level
> [1]: both the transmitter and the receiver need them, and they only have
> to be programmed once. They are programmed from the transmitter's
> power-on path today. A later change registers one struct phy for each of
> them, and a second consumer coming up would then reset the block
> underneath a PHY that is already running.
> 
> Program them from runtime resume instead, where whichever of the two
> comes up first brings the block up and a second one no longer resets it.
> This puts the BIAS writes ahead of the per-PHY reset assert, where the
> TRM's worked example has them after [1]. That is safe: neither M_RESETN
> nor S_RESETN covers the BIAS registers, and the values written are those
> registers' reset defaults.
> 
> The output level select in BIAS_CON4 stays on the power-on path: it
> selects 400mV for D-PHY and 530mV for C-PHY, and the driver core resumes
> a runtime-PM supplier before the consumer's probe runs, so runtime resume
> cannot yet know which of the two applies.

This all is about to change. As discussed in the other thread, it is a
HW specific decision whether D-PHY or C-PHY is used, and it holds for
both PHYs at the same time. Thus, it is reflected in the device tree, e.g.,

&mipidcphy0 {
    phy-mode = <PHY_TYPE_CPHY>;
};

The value can be stored in samsung->type at provider probe time and
shall be available at PM resume.

> 
> [1] RK3588 TRM: sections 22.4.1 (address map), 22.4.2 (reset values),
>     22.6.1.2 (reset) and 22.6.4.1 (D-PHY transmitter start-up)
> 
> Signed-off-by: Jason Yang <jason98166@gmail.com>
> ---
>  drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c | 25 ++++++++++++-----------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> index 7382043e1f31..1471a1563d2a 100644
> --- a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> @@ -980,13 +980,6 @@ static void samsung_mipi_dcphy_bias_block_enable(struct samsung_mipi_dcphy *sams
>  						 I_LADDER_1_00V);
>  	regmap_write(samsung->regmap, BIAS_CON2, REG_325M_325MV | REG_LP_400M_400MV |
>  						 REG_400M_400MV | REG_645M_645MV);
> -
> -	/* default output voltage select:
> -	 * dphy: 400mv
> -	 * cphy: 530mv
> -	 */
> -	regmap_update_bits(samsung->regmap, BIAS_CON4,
> -			   I_MUX_SEL_MASK, I_MUX_400MV);

This can stay here but will need some

switch (samsung->type) {
case PHY_TYPE_CPHY:
	// write 530 mV
	break;
case PHY_TYPE_DPHY:
	// write 400 mV
	break;
default:
	//...
};

once there is support for CPHY.

>  }
>  
>  static void samsung_mipi_dphy_lane_enable(struct samsung_mipi_dcphy *samsung)
> @@ -1338,7 +1331,13 @@ static int samsung_mipi_dphy_power_on(struct samsung_mipi_dcphy *samsung)
>  
>  	reset_control_assert(samsung->m_phy_rst);
>  
> -	samsung_mipi_dcphy_bias_block_enable(samsung);
> +	/* default output voltage select:
> +	 * dphy: 400mv
> +	 * cphy: 530mv
> +	 */
> +	regmap_update_bits(samsung->regmap, BIAS_CON4,
> +			   I_MUX_SEL_MASK, I_MUX_400MV);
> +

No need for that change, then.

>  	samsung_mipi_dcphy_pll_configure(samsung);
>  	samsung_mipi_dphy_clk_lane_timing_init(samsung);
>  	samsung_mipi_dphy_data_lane_timing_init(samsung);
> @@ -1362,10 +1361,6 @@ static int samsung_mipi_dcphy_power_on(struct phy *phy)
>  {
>  	struct samsung_mipi_dcphy *samsung = phy_get_drvdata(phy);
>  
> -	reset_control_assert(samsung->apb_rst);
> -	udelay(1);
> -	reset_control_deassert(samsung->apb_rst);
> -
>  	switch (samsung->type) {
>  	case PHY_TYPE_DPHY:
>  		return samsung_mipi_dphy_power_on(samsung);
> @@ -1656,6 +1651,12 @@ static __maybe_unused int samsung_mipi_dcphy_runtime_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	reset_control_assert(samsung->apb_rst);
> +	udelay(1);
> +	reset_control_deassert(samsung->apb_rst);
> +
> +	samsung_mipi_dcphy_bias_block_enable(samsung);

Adjust this accordingly. I guess it makes sense to reset from the PM
resume method.

Best regards,
Michael

> +
>  	return 0;
>  }
>  
>