Re: [PATCH v4 07/17] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

On Tue, Jan 07, 2025 at 11:02:15AM +0800, Damon Ding wrote:

Hi Dmitry,

On 2024/12/30 20:45, Dmitry Baryshkov wrote:

quoted

On Thu, Dec 26, 2024 at 02:33:03PM +0800, Damon Ding wrote:

quoted

Add basic support for RBR/HBR/HBR2 link rates, and the voltage swing and
pre-emphasis configurations of each link rate have been verified according
to the eDP 1.3 requirements.

Well... Please describe what's happening here. That the HDMI PHY on your
platform also provides support for DP / eDP. Please document any design
decisions that you had to make.

Yes, I will add more relevant descriptions in the next version.

quoted

quoted

Signed-off-by: Damon Ding <redacted>

---

Changes in v2:
- Add the module author

Changes in v3:
- Split this patch into two, one for correction and the other for
   extension

Changes in v4:
- Add link_rate and lanes parameters in struct rk_hdptx_phy to store the
   phy_configure() result for &phy_configure_opts.dp.link_rate and
   &phy_configure_opts.dp.lanes
---
  .../phy/rockchip/phy-rockchip-samsung-hdptx.c | 896 +++++++++++++++++-
  1 file changed, 889 insertions(+), 7 deletions(-)

@@ -933,9 +1484,339 @@ static int rk_hdptx_phy_power_off(struct phy *phy)
  	return rk_hdptx_phy_consumer_put(hdptx, false);
  }
+static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
+				 int submode)
+{
+	return 0;
+}

No need for the stub, please drop it. The host controller driver can
still call phy_set_mode() / _ext(), the call will return 0.

Without the &phy_ops.set_mode(), the phy driver can not get phy_mode to
distinguish between HDMI and DP mode via the phy_get_mode(), even if the
host driver calls phy_set_mode() / _ext(). Additionally, the previous
discussion [0] also mentioned future considerations for dynamic switching.
Indeed, I should add a related comment before the 'return 0;' to enhance
understandability.

Oh... I see. Without .set_mode() callback the attrs.mode won't change.
We should probably fix that, there should be no need to add dummy stubs.


-- 
With best wishes
Dmitry