[PATCH v2 4/4] phy: rockchip-samsung-dcphy: add MIPI D-PHY receiver support

[楊智成]

> Does this code poll for PHY_READY while the PHY is still held in reset?

Yes, and that is the sequence the TRM specifies: the RK3588 TRM
start-up sequence (figure 22-8) asserts S_RESETN, programs the settle
registers, enables the lanes, waits for PHY_READY to go high, and only
then deasserts S_RESETN. The poll does succeed on real hardware -
every capture listed in the cover's Testing section went through this
exact path.

> Without this in the RX path, could the shared APB bus interface and BIAS
> block remain in an uninitialized state from the bootloader?

This one came up on v1 as well; summarizing the disposition: the
vendor BSP's RX bring-up does not touch the APB reset either, the TRM
does not require it for the receiver, and the CSI-only configuration -
where the receiver is the only direction ever powered - captures
frames on real hardware without the APB reset ever being issued. The
BIAS block is programmed explicitly by whichever direction powers on
first, so nothing relies on bootloader state.

Happy to revisit if either assessment looks wrong.

Thanks,
Jason


<sashiko-bot at kernel.org> 於 2026年7月26日週日 下午11:07寫道：
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The RX power-on sequence fails to toggle the shared APB reset (`apb_rst`) when it is the first PHY direction to power on, potentially leaving shared PHY state uninitialized.
> - [High] The RX lane enable function polls for `PHY_READY` while the PHY is still held in reset, guaranteeing a timeout failure.
> --
>
> --- Patch [4]: [PATCH v2 4/4] phy: rockchip-samsung-dcphy: add MIPI D-PHY receiver support ---
> commit 8b1f5adbd06df56ad03e2b8d081a6587be5fe71d
> Author: Jason Yang <jason98166 at gmail.com>
>
> phy: rockchip-samsung-dcphy: add MIPI D-PHY receiver support
>
> This patch implements the receiver (CSI) direction of the D-PHY for the
> Rockchip Samsung driver. It adds the RX register layout, HS-frequency settle
> table, and hooks the receiver bring-up and teardown into the per-direction
> power paths. The PLL sequence is also skipped for the receive path as the
> sensor supplies the link clock.
>
> > diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> > index 95eb1200cab48..f6d112935c016 100644
> > --- a/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> > +++ b/drivers/phy/rockchip/phy-rockchip-samsung-dcphy.c
> [ ... ]
> > @@ -988,6 +1046,228 @@ struct samsung_mipi_dphy_timing samsung_mipi_dphy_timing_table[] = {
> [ ... ]
> > +static int samsung_mipi_dphy_rx_lane_enable(struct samsung_mipi_dcphy *samsung)
> > +{
> > +     struct samsung_mipi_dcphy_dir *pd = &samsung->phys[RK_DCPHY_DIR_RX];
> > +     u32 sts;
> > +     int ret, i;
> > +
> > +     regmap_update_bits(samsung->regmap, DPHY_SC_GNR_CON0, PHY_ENABLE, PHY_ENABLE);
> > +     for (i = 0; i < pd->lanes; i++)
> > +             regmap_update_bits(samsung->regmap, rx_data_lane_gnr_con0[i],
> > +                                PHY_ENABLE, PHY_ENABLE);
> > +
> > +     ret = regmap_read_poll_timeout(samsung->regmap, DPHY_SC_GNR_CON0,
> > +                                    sts, sts & PHY_READY, 200, 4000);
>
> [Severity: High]
> Does this code poll for PHY_READY while the PHY is still held in reset?
>
> Looking at samsung_mipi_dphy_rx_power_on(), samsung->s_phy_rst is asserted,
> then samsung_mipi_dphy_rx_lane_enable() is called, and only after that is
> s_phy_rst deasserted.
>
> Will this cause the PHY_READY poll to always timeout?
>
> > +     if (ret) {
> > +             dev_err(samsung->dev, "RX clock lane not ready\n");
> > +             goto err_lane_disable;
> > +     }
> [ ... ]
> > @@ -1004,6 +1284,44 @@ static void samsung_mipi_dcphy_bias_block_enable(struct samsung_mipi_dcphy *sams
> [ ... ]
> > +static int samsung_mipi_dphy_rx_power_on(struct samsung_mipi_dcphy *samsung)
> > +{
> > +     bool first = !samsung->phys[RK_DCPHY_DIR_TX].powered;
> > +     int ret;
> > +
> > +     if (!samsung->phys[RK_DCPHY_DIR_RX].hs_clk_rate)
> > +             return -EINVAL;
> > +
> > +     reset_control_assert(samsung->s_phy_rst);
> > +
> > +     if (first)
> > +             samsung_mipi_dcphy_bias_block_enable(samsung);
>
> [Severity: High]
> If the RX direction is powered on first, does it need to toggle the shared APB
> reset (apb_rst) before enabling the bias block?
>
> In samsung_mipi_dphy_tx_power_on(), when first is true, it does:
>
>         if (first) {
>                 reset_control_assert(samsung->apb_rst);
>                 udelay(1);
>                 reset_control_deassert(samsung->apb_rst);
>         }
>
> Without this in the RX path, could the shared APB bus interface and BIAS
> block remain in an uninitialized state from the bootloader?
>
> > +     samsung_mipi_dphy_rx_config_common(samsung);
> > +
> > +     ret = samsung_mipi_dphy_rx_config_settle(samsung);
> > +     if (ret)
> > +             goto out_deassert;
> > +
> > +     ret = samsung_mipi_dphy_rx_lane_enable(samsung);
> > +
> > +out_deassert:
> > +     reset_control_deassert(samsung->s_phy_rst);
> > +
> > +     return ret;
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260726-dcphy-rx-v1-v2-0-cf9cc34a412a@gmail.com?part=4