[PATCH v1 0/6] Add eDP support for RK3568

[PATCH v1 0/6] Add eDP support for RK3568

[Damon Ding]

Rebased after:
https://lore.kernel.org/all/20260928081418.3605775-1-damon.ding@rock-chips.com/

PATCH 1 is to document the "rockchip,rk3568-edp" compatible in the
        Rockchip Analogix DP binding.
PATCH 2 is to add the binding for the RK3568 eDP PHY with NaNeng IP block.
PATCH 3 is to add the RK3568 support for the Analogix DP bridge driver.
PATCH 4 is to add the driver for the Rockchip eDP Transmitter PHY with
        NaNeng IP block.
PATCH 5 is to add the RK3568 support for the Rockchip Analogix DP driver.
PATCH 6 is to add the eDP controller and eDP PHY nodes for the RK3568
        SoCs.

Tested on RK3568 EVB1 baord with LG eDP panel LP079QX1-SP0V.

&edp {
  avdd-0v9-supply = <&vdda0v9_image>;
  avdd-1v8-supply = <&vcca1v8_image>;
  force-hpd;
  status = "okay";

  aux-bus {
    panel {
      compatible = "edp-panel";
      backlight = <&backlight>;
      power-supply = <&vcc3v3_lcd0_n>;
      no-hpd;

      port {
        panel_in_edp: endpoint {
          remote-endpoint = <&edp_out_panel>;
        };
      };
    };
  };
};

&edp_in {
  edp_in_vp0: endpoint {
    remote-endpoint = <&vp0_out_edp>;
  };
};

&edp_out {
  edp_out_panel: endpoint {
    remote-endpoint = <&panel_in_edp>;
  };
};

&edp_phy {
  status = "okay";
};

&vp0 {
  vp0_out_edp: endpoint@ROCKCHIP_VOP2_EP_EDP0 {
    reg = <ROCKCHIP_VOP2_EP_EDP0>;
    remote-endpoint = <&edp_in_vp0>;
  };
};

Damon Ding (6):
  dt-bindings: display: rockchip: analogix-dp: Add support for RK3568
  dt-bindings: phy: Add rockchip,rk3568-edp-phy
  drm/bridge: analogix_dp: Add support for RK3568
  phy: rockchip: Add support for Naneng eDP phy
  drm/rockchip: analogix_dp: Add support for RK3568
  arm64: dts: rockchip: rk356x: Add eDP controller and phy nodes

 .../rockchip/rockchip,analogix-dp.yaml        |  25 +-
 .../bindings/phy/rockchip,rk3568-edp-phy.yaml |  59 +++
 .../devicetree/bindings/soc/rockchip/grf.yaml |  15 +
 arch/arm64/boot/dts/rockchip/rk356x-base.dtsi |  41 ++
 .../drm/bridge/analogix/analogix_dp_core.c    |   1 +
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  20 +-
 drivers/phy/rockchip/Kconfig                  |  10 +
 drivers/phy/rockchip/Makefile                 |   1 +
 .../phy/rockchip/phy-rockchip-naneng-edp.c    | 435 ++++++++++++++++++
 include/drm/bridge/analogix_dp.h              |   2 +
 10 files changed, 607 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/rockchip,rk3568-edp-phy.yaml
 create mode 100644 drivers/phy/rockchip/phy-rockchip-naneng-edp.c

-- 
2.34.1

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

[PATCH v1 1/6] dt-bindings: display: rockchip: analogix-dp: Add support for RK3568

[Damon Ding]

Document the "rockchip,rk3568-edp" compatible.  Like RK3576/RK3588,
the RK3568 eDP controller has the dp/pclk/hclk clocks and dp/apb
resets, but unlike them it does not use GRF registers, so make the
"rockchip,grf" property conditional on the SoC.

Also document the optional avdd-0v9 and avdd-1v8 supplies, which are
needed by the RK3568 eDP controller.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../rockchip/rockchip,analogix-dp.yaml        | 25 ++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index 39dafe75a680..f8670a4e0430 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -15,6 +15,7 @@ properties:
     enum:
       - rockchip,rk3288-dp
       - rockchip,rk3399-edp
+      - rockchip,rk3568-edp
       - rockchip,rk3576-edp
       - rockchip,rk3588-edp
 
@@ -47,6 +48,16 @@ properties:
       - const: dp
       - const: apb
 
+  avdd-0v9-supply:
+    description:
+      A 0.9V supply that powers up the SoC internal circuitry. The actual
+      supply source varies between different boards.
+
+  avdd-1v8-supply:
+    description:
+      A 1.8V supply that powers up the SoC internal circuitry. The actual
+      supply source varies between different boards.
+
   rockchip,grf:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
@@ -61,11 +72,22 @@ required:
   - clock-names
   - resets
   - reset-names
-  - rockchip,grf
 
 allOf:
   - $ref: /schemas/display/bridge/analogix,dp.yaml#
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3568-edp
+    then:
+      properties:
+        rockchip,grf: false
+    else:
+      required:
+        - rockchip,grf
+
   - if:
       properties:
         compatible:
@@ -100,6 +122,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - rockchip,rk3568-edp
               - rockchip,rk3576-edp
               - rockchip,rk3588-edp
     then:
-- 
2.34.1


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

[PATCH v1 2/6] dt-bindings: phy: Add rockchip,rk3568-edp-phy

[Damon Ding]

Document the eDP transmitter PHY with NaNeng IP block found on the
RK3568 SoC.  The PHY is controlled through the eDP PHY GRF registers,
so its node is modeled as a child of the eDP PHY GRF syscon.  Also
add the "rockchip,rk3568-edp-phy-grf" compatible and the "edp-phy"
child node reference to the Rockchip GRF schema.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../bindings/phy/rockchip,rk3568-edp-phy.yaml | 59 +++++++++++++++++++
 .../devicetree/bindings/soc/rockchip/grf.yaml | 15 +++++
 2 files changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/rockchip,rk3568-edp-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3568-edp-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3568-edp-phy.yaml
new file mode 100644
index 000000000000..afe7bc3d839e
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/rockchip,rk3568-edp-phy.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/rockchip,rk3568-edp-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip eDP Transmitter PHY with NaNeng IP block
+
+maintainers:
+  - Damon Ding <damon.ding@rock-chips.com>
+  - Heiko Stuebner <heiko@sntech.de>
+
+description:
+  On the RK3568 SoC the eDP PHY is controlled through the eDP PHY GRF
+  registers, therefore the PHY node is expected to be a child of the
+  eDP PHY GRF syscon node.
+
+properties:
+  compatible:
+    const: rockchip,rk3568-edp-phy
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: refclk
+
+  "#phy-cells":
+    const: 0
+
+required:
+  - compatible
+  - clocks
+  - clock-names
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rk3568-cru.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      edp_phy_grf: syscon@fdcb0000 {
+        compatible = "rockchip,rk3568-edp-phy-grf", "syscon", "simple-mfd";
+        reg = <0x0 0xfdcb0000 0x0 0x100>;
+        clocks = <&cru PCLK_EDPPHY_GRF>;
+
+        edp_phy: edp-phy {
+          compatible = "rockchip,rk3568-edp-phy";
+          clocks = <&pmucru XIN_OSC0_EDPPHY_G>;
+          clock-names = "refclk";
+          #phy-cells = <0>;
+        };
+      };
+    };
diff --git a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
index 7bcb4e2f47ec..8d4ced0145b8 100644
--- a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
+++ b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
@@ -93,6 +93,7 @@ properties:
               - rockchip,rk3399-grf
               - rockchip,rk3399-pmugrf
               - rockchip,rk3562-pmu-grf
+              - rockchip,rk3568-edp-phy-grf
               - rockchip,rk3568-grf
               - rockchip,rk3568-pmugrf
               - rockchip,rk3576-ioc-grf
@@ -156,6 +157,19 @@ allOf:
           $ref: /schemas/phy/rockchip,rk3288-dp-phy.yaml#
           unevaluatedProperties: false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3568-edp-phy-grf
+
+    then:
+      properties:
+        edp-phy:
+          type: object
+          $ref: /schemas/phy/rockchip,rk3568-edp-phy.yaml#
+          unevaluatedProperties: false
+
   - if:
       properties:
         compatible:
@@ -309,6 +323,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - rockchip,rk3568-edp-phy-grf
               - rockchip,rk3576-dcphy-grf
               - rockchip,rk3576-vo1-grf
               - rockchip,rk3588-vo-grf
-- 
2.34.1


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

[PATCH v1 3/6] drm/bridge: analogix_dp: Add support for RK3568

[Damon Ding]

The RK3568 SoC integrates an eDP transmitter based on the Analogix
DisplayPort core.  Like RK3288 and RK3399, its main link consists of
4 physical lanes running at 1.62/2.7 Gbps per lane.

Add a new RK3568_EDP device type and let it share the link rate and
lane count limits of the RK3288/RK3399 variants.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 +
 include/drm/bridge/analogix_dp.h                   | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 90245d55d1d2..2aefb4615a4f 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1375,6 +1375,7 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 	switch (dp->plat_data->dev_type) {
 	case RK3288_DP:
 	case RK3399_EDP:
+	case RK3568_EDP:
 		/*
 		 * Like Rk3288 DisplayPort TRM indicate that "Main link
 		 * containing 4 physical lanes of 2.7/1.62 Gbps/lane".
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index f152ca8c3218..7b5615ae0dc1 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -16,6 +16,7 @@ enum analogix_dp_devtype {
 	EXYNOS_DP,
 	RK3288_DP,
 	RK3399_EDP,
+	RK3568_EDP,
 	RK3576_EDP,
 	RK3588_EDP,
 };
@@ -25,6 +26,7 @@ static inline bool analogix_dp_is_rockchip(enum analogix_dp_devtype type)
 	switch (type) {
 	case RK3288_DP:
 	case RK3399_EDP:
+	case RK3568_EDP:
 	case RK3576_EDP:
 	case RK3588_EDP:
 		return true;
-- 
2.34.1


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

[PATCH v1 4/6] phy: rockchip: Add support for Naneng eDP phy

[Damon Ding]

Add a driver for the Rockchip eDP transmitter PHY with NaNeng IP
block, as found on the RK3568 SoC.

The PHY registers live in the eDP PHY GRF, of which the PHY is a
child node, so accesses are performed through the parent syscon
regmap. The PHY supports 1/2/4 lanes at 1.62 Gbps and 2.7 Gbps,
optional spread spectrum clocking, as well as per-lane voltage swing
and pre-emphasis tuning, all exposed through the generic PHY
DisplayPort configuration interface.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 drivers/phy/rockchip/Kconfig                  |  10 +
 drivers/phy/rockchip/Makefile                 |   1 +
 .../phy/rockchip/phy-rockchip-naneng-edp.c    | 435 ++++++++++++++++++
 3 files changed, 446 insertions(+)
 create mode 100644 drivers/phy/rockchip/phy-rockchip-naneng-edp.c

diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
index 14698571b607..202a6973e20b 100644
--- a/drivers/phy/rockchip/Kconfig
+++ b/drivers/phy/rockchip/Kconfig
@@ -74,6 +74,16 @@ config PHY_ROCKCHIP_NANENG_COMBO_PHY
 	  Enable this to support the Rockchip PCIe/USB3.0/SATA/QSGMII
 	  combo PHY with NaNeng IP block.
 
+config PHY_ROCKCHIP_NANENG_EDP
+	tristate "Rockchip Naneng eDP Transmitter PHY driver"
+	depends on ARCH_ROCKCHIP && OF
+	select GENERIC_PHY
+	help
+	  Enable this to support the Rockchip eDP Transmitter PHY with
+	  NaNeng IP block. The PHY, which is controlled through the eDP
+	  PHY GRF, provides 1/2/4 lanes running at 1.62/2.7 Gbps per lane
+	  for the eDP controller found on the RK3568 SoC.
+
 config PHY_ROCKCHIP_PCIE
 	tristate "Rockchip PCIe PHY Driver"
 	depends on (ARCH_ROCKCHIP && OF) || COMPILE_TEST
diff --git a/drivers/phy/rockchip/Makefile b/drivers/phy/rockchip/Makefile
index 117aaffd037d..260a10157461 100644
--- a/drivers/phy/rockchip/Makefile
+++ b/drivers/phy/rockchip/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_PHY_ROCKCHIP_INNO_DSIDPHY)	+= phy-rockchip-inno-dsidphy.o
 obj-$(CONFIG_PHY_ROCKCHIP_INNO_HDMI)	+= phy-rockchip-inno-hdmi.o
 obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)	+= phy-rockchip-inno-usb2.o
 obj-$(CONFIG_PHY_ROCKCHIP_NANENG_COMBO_PHY)	+= phy-rockchip-naneng-combphy.o
+obj-$(CONFIG_PHY_ROCKCHIP_NANENG_EDP)	+= phy-rockchip-naneng-edp.o
 obj-$(CONFIG_PHY_ROCKCHIP_PCIE)		+= phy-rockchip-pcie.o
 obj-$(CONFIG_PHY_ROCKCHIP_SAMSUNG_DCPHY)	+= phy-rockchip-samsung-dcphy.o
 obj-$(CONFIG_PHY_ROCKCHIP_SAMSUNG_HDPTX)	+= phy-rockchip-samsung-hdptx.o
diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-edp.c b/drivers/phy/rockchip/phy-rockchip-naneng-edp.c
new file mode 100644
index 000000000000..a407714f639a
--- /dev/null
+++ b/drivers/phy/rockchip/phy-rockchip-naneng-edp.c
@@ -0,0 +1,435 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2026 Rockchip Electronics Co., Ltd.
+ *
+ * Author: Damon Ding <damon.ding@rock-chips.com>
+ */
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/hw_bitfield.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define EDP_PHY_GRF_CON0		0x0000
+#define EDP_PHY_TX_IDLE_MASK		GENMASK(11, 8)
+#define EDP_PHY_TX_PD_MASK		GENMASK(7, 4)
+#define EDP_PHY_IDDQ_EN_MASK		BIT(1)
+#define EDP_PHY_PD_PLL_MASK		BIT(0)
+
+#define EDP_PHY_GRF_CON1		0x0004
+#define EDP_PHY_PLL_DIV_MASK		GENMASK(14, 0)
+
+#define EDP_PHY_GRF_CON2		0x0008
+#define EDP_PHY_TX_RTERM_MASK		GENMASK(10, 8)
+#define EDP_PHY_RATE_MASK		GENMASK(5, 4)
+#define EDP_PHY_REF_DIV_MASK		GENMASK(3, 0)
+
+#define EDP_PHY_GRF_CON3		0x000c
+#define EDP_PHY_TX3_EMP_MASK		GENMASK(15, 12)
+#define EDP_PHY_TX2_EMP_MASK		GENMASK(11, 8)
+#define EDP_PHY_TX1_EMP_MASK		GENMASK(7, 4)
+#define EDP_PHY_TX0_EMP_MASK		GENMASK(3, 0)
+
+#define EDP_PHY_GRF_CON4		0x0010
+#define EDP_PHY_TX3_AMP_MASK		GENMASK(14, 12)
+#define EDP_PHY_TX2_AMP_MASK		GENMASK(10, 8)
+#define EDP_PHY_TX1_AMP_MASK		GENMASK(6, 4)
+#define EDP_PHY_TX0_AMP_MASK		GENMASK(2, 0)
+
+#define EDP_PHY_GRF_CON5		0x0014
+#define EDP_PHY_TX_MODE_MASK		GENMASK(9, 8)
+#define EDP_PHY_TX3_AMP_SCALE_MASK	GENMASK(7, 6)
+#define EDP_PHY_TX2_AMP_SCALE_MASK	GENMASK(5, 4)
+#define EDP_PHY_TX1_AMP_SCALE_MASK	GENMASK(3, 2)
+#define EDP_PHY_TX0_AMP_SCALE_MASK	GENMASK(1, 0)
+
+#define EDP_PHY_GRF_CON6		0x0018
+#define EDP_PHY_SSC_DEPTH_MASK		GENMASK(15, 12)
+#define EDP_PHY_SSC_EN_MASK		BIT(11)
+#define EDP_PHY_SSC_CNT_MASK		GENMASK(9, 0)
+
+#define EDP_PHY_GRF_CON7		0x001c
+
+#define EDP_PHY_GRF_CON8		0x0020
+#define EDP_PHY_PLL_CTL_H_MASK		GENMASK(15, 0)
+
+#define EDP_PHY_GRF_CON9		0x0024
+#define EDP_PHY_TX_CTL_MASK		GENMASK(15, 0)
+
+#define EDP_PHY_GRF_CON10		0x0028
+#define EDP_PHY_AUX_RCV_PD_SEL_MASK	BIT(5)
+#define EDP_PHY_AUX_DRV_PD_SEL_MASK	BIT(4)
+#define EDP_PHY_AUX_IDLE_MASK		BIT(2)
+#define EDP_PHY_AUX_RCV_PD_MASK		BIT(1)
+#define EDP_PHY_AUX_DRV_PD_MASK		BIT(0)
+
+#define EDP_PHY_GRF_CON11		0x002c
+#define EDP_PHY_AUX_RCV_VCM_MASK	GENMASK(14, 12)
+#define EDP_PHY_AUX_MODE_MASK		GENMASK(11, 10)
+#define EDP_PHY_AUX_AMP_SCALE_MASK	GENMASK(9, 8)
+#define EDP_PHY_AUX_AMP_MASK		GENMASK(6, 4)
+#define EDP_PHY_AUX_RTERM_MASK		GENMASK(2, 0)
+
+#define EDP_PHY_GRF_STATUS0		0x0030
+#define PLL_RDY				BIT(0)
+
+#define EDP_PHY_GRF_STATUS1		0x0034
+
+struct rockchip_edp_phy {
+	struct regmap *grf;
+	struct device *dev;
+	struct clk *refclk;
+};
+
+static struct {
+	int amp;
+	int amp_scale;
+	int emp;
+} swing_pre_emph_table[4][4] = {
+	{ {0x1, 0x1, 0x0}, {0x2, 0x1, 0x4}, {0x3, 0x1, 0x8}, {0x4, 0x1, 0xd} },
+	{ {0x3, 0x1, 0x0}, {0x5, 0x1, 0x7}, {0x6, 0x1, 0x6}, { -1,  -1,  -1} },
+	{ {0x5, 0x1, 0x0}, {0x7, 0x1, 0x4}, { -1,  -1,  -1}, { -1,  -1,  -1} },
+	{ {0x7, 0x1, 0x0}, { -1,  -1,  -1}, { -1,  -1,  -1}, { -1,  -1,  -1} },
+};
+
+static void rockchip_edp_phy_set_voltage(struct rockchip_edp_phy *edpphy,
+					 struct phy_configure_opts_dp *dp,
+					 u8 lane)
+{
+	u32 amp, amp_scale, emp;
+
+	amp = swing_pre_emph_table[dp->voltage[lane]][dp->pre[lane]].amp;
+	amp_scale = swing_pre_emph_table[dp->voltage[lane]][dp->pre[lane]].amp_scale;
+	emp = swing_pre_emph_table[dp->voltage[lane]][dp->pre[lane]].emp;
+
+	switch (lane) {
+	case 0:
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON3,
+			     FIELD_PREP_WM16(EDP_PHY_TX0_EMP_MASK, emp));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON4,
+			     FIELD_PREP_WM16(EDP_PHY_TX0_AMP_MASK, amp));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON5,
+			     FIELD_PREP_WM16(EDP_PHY_TX0_AMP_SCALE_MASK, amp_scale));
+		break;
+	case 1:
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON3,
+			     FIELD_PREP_WM16(EDP_PHY_TX1_EMP_MASK, emp));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON4,
+			     FIELD_PREP_WM16(EDP_PHY_TX1_AMP_MASK, amp));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON5,
+			     FIELD_PREP_WM16(EDP_PHY_TX1_AMP_SCALE_MASK, amp_scale));
+		break;
+	case 2:
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON3,
+			     FIELD_PREP_WM16(EDP_PHY_TX2_EMP_MASK, emp));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON4,
+			     FIELD_PREP_WM16(EDP_PHY_TX2_AMP_MASK, amp));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON5,
+			     FIELD_PREP_WM16(EDP_PHY_TX2_AMP_SCALE_MASK, amp_scale));
+		break;
+	case 3:
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON3,
+			     FIELD_PREP_WM16(EDP_PHY_TX3_EMP_MASK, emp));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON4,
+			     FIELD_PREP_WM16(EDP_PHY_TX3_AMP_MASK, amp));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON5,
+			     FIELD_PREP_WM16(EDP_PHY_TX3_AMP_SCALE_MASK, amp_scale));
+		break;
+	}
+}
+
+static int rockchip_edp_phy_set_voltages(struct rockchip_edp_phy *edpphy,
+					 struct phy_configure_opts_dp *dp)
+{
+	u8 lane;
+
+	for (lane = 0; lane < dp->lanes; lane++)
+		rockchip_edp_phy_set_voltage(edpphy, dp, lane);
+
+	return 0;
+}
+
+static int rockchip_edp_phy_set_rate(struct rockchip_edp_phy *edpphy,
+				     struct phy_configure_opts_dp *dp)
+{
+	u32 value;
+	int ret;
+
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON0,
+		     FIELD_PREP_WM16(EDP_PHY_TX_IDLE_MASK, 0xf) |
+		     FIELD_PREP_WM16(EDP_PHY_TX_PD_MASK, 0xf));
+	usleep_range(100, 101);
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON5, FIELD_PREP_WM16(EDP_PHY_TX_MODE_MASK, 0x3));
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON0, FIELD_PREP_WM16(EDP_PHY_PD_PLL_MASK, 0x1));
+
+	switch (dp->link_rate) {
+	case 1620:
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON1,
+			     FIELD_PREP_WM16(EDP_PHY_PLL_DIV_MASK, 0x4380));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON2,
+			     FIELD_PREP_WM16(EDP_PHY_TX_RTERM_MASK, 0x1) |
+			     FIELD_PREP_WM16(EDP_PHY_RATE_MASK, 0x1) |
+			     FIELD_PREP_WM16(EDP_PHY_REF_DIV_MASK, 0x0));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON8,
+			     FIELD_PREP_WM16(EDP_PHY_PLL_CTL_H_MASK, 0x0800));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON9,
+			     FIELD_PREP_WM16(EDP_PHY_TX_CTL_MASK, 0x0000));
+		break;
+	case 2700:
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON1,
+			     FIELD_PREP_WM16(EDP_PHY_PLL_DIV_MASK, 0x3840));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON2,
+			     FIELD_PREP_WM16(EDP_PHY_TX_RTERM_MASK, 0x1) |
+			     FIELD_PREP_WM16(EDP_PHY_RATE_MASK, 0x0) |
+			     FIELD_PREP_WM16(EDP_PHY_REF_DIV_MASK, 0x0));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON8,
+			     FIELD_PREP_WM16(EDP_PHY_PLL_CTL_H_MASK, 0x0800));
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON9,
+			     FIELD_PREP_WM16(EDP_PHY_TX_CTL_MASK, 0x0000));
+		break;
+	}
+
+	if (dp->ssc)
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON6,
+			     FIELD_PREP_WM16(EDP_PHY_SSC_DEPTH_MASK, 0x9) |
+			     FIELD_PREP_WM16(EDP_PHY_SSC_EN_MASK, 0x1) |
+			     FIELD_PREP_WM16(EDP_PHY_SSC_CNT_MASK, 0x17d));
+	else
+		regmap_write(edpphy->grf, EDP_PHY_GRF_CON6,
+			     FIELD_PREP_WM16(EDP_PHY_SSC_EN_MASK, 0x0));
+
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON0, FIELD_PREP_WM16(EDP_PHY_PD_PLL_MASK, 0));
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON0,
+		     FIELD_PREP_WM16(EDP_PHY_TX_PD_MASK, ~GENMASK(dp->lanes - 1, 0)));
+	ret = regmap_read_poll_timeout(edpphy->grf, EDP_PHY_GRF_STATUS0,
+				       value, value & PLL_RDY, 100, 1000);
+	if (ret) {
+		dev_err(edpphy->dev, "Failed to wait for PHY PLL ready: %d\n", ret);
+		return ret;
+	}
+
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON5, FIELD_PREP_WM16(EDP_PHY_TX_MODE_MASK, 0x0));
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON0,
+		     FIELD_PREP_WM16(EDP_PHY_TX_IDLE_MASK, ~GENMASK(dp->lanes - 1, 0)));
+
+	return 0;
+}
+
+static int rockchip_edp_phy_verify_config(struct rockchip_edp_phy *edpphy,
+					  struct phy_configure_opts_dp *dp)
+{
+	int i;
+
+	/* If changing link rate was required, verify it's supported. */
+	if (dp->set_rate) {
+		switch (dp->link_rate) {
+		case 1620:
+		case 2700:
+			/* valid bit rate */
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	if (dp->set_lanes) {
+		/* Verify lane count. */
+		switch (dp->lanes) {
+		case 1:
+		case 2:
+		case 4:
+			/* valid lane count. */
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	/*
+	 * If changing voltages is required, check swing and pre-emphasis
+	 * levels, per-lane.
+	 */
+	if (dp->set_voltages) {
+		/* Lane count verified previously. */
+		for (i = 0; i < dp->lanes; i++) {
+			if (dp->voltage[i] > 3 || dp->pre[i] > 3)
+				return -EINVAL;
+
+			/*
+			 * Sum of voltage swing and pre-emphasis levels cannot
+			 * exceed 3.
+			 */
+			if (dp->voltage[i] + dp->pre[i] > 3)
+				return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int rockchip_edp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+	struct rockchip_edp_phy *edpphy = phy_get_drvdata(phy);
+	int ret;
+
+	ret = rockchip_edp_phy_verify_config(edpphy, &opts->dp);
+	if (ret) {
+		dev_err(edpphy->dev, "Invalid params for phy configure\n");
+		return ret;
+	}
+
+	if (opts->dp.set_rate) {
+		ret = rockchip_edp_phy_set_rate(edpphy, &opts->dp);
+		if (ret) {
+			dev_err(edpphy->dev, "Failed to set rate\n");
+			return ret;
+		}
+	}
+
+	if (opts->dp.set_voltages) {
+		ret = rockchip_edp_phy_set_voltages(edpphy, &opts->dp);
+		if (ret) {
+			dev_err(edpphy->dev, "Failed to set voltages\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static bool rockchip_edp_phy_enabled(struct rockchip_edp_phy *edpphy)
+{
+	u32 val;
+
+	regmap_read(edpphy->grf, EDP_PHY_GRF_STATUS0, &val);
+
+	return FIELD_GET(PLL_RDY, val);
+}
+
+static int rockchip_edp_phy_power_on(struct phy *phy)
+{
+	struct rockchip_edp_phy *edpphy = phy_get_drvdata(phy);
+	int ret;
+
+	ret = clk_prepare_enable(edpphy->refclk);
+	if (ret < 0) {
+		dev_err(edpphy->dev, "Failed to prepare_enable refclk [%d]\n", ret);
+		return ret;
+	}
+
+	if (rockchip_edp_phy_enabled(edpphy))
+		return 0;
+
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON10,
+		     FIELD_PREP_WM16(EDP_PHY_AUX_RCV_PD_MASK, 0x1) |
+		     FIELD_PREP_WM16(EDP_PHY_AUX_DRV_PD_MASK, 0x1) |
+		     FIELD_PREP_WM16(EDP_PHY_AUX_IDLE_MASK, 0x1));
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON0,
+		     FIELD_PREP_WM16(EDP_PHY_TX_IDLE_MASK, 0xf) |
+		     FIELD_PREP_WM16(EDP_PHY_TX_PD_MASK, 0xf) |
+		     FIELD_PREP_WM16(EDP_PHY_PD_PLL_MASK, 0x1));
+	usleep_range(100, 101);
+
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON11,
+		     FIELD_PREP_WM16(EDP_PHY_AUX_RCV_VCM_MASK, 0x4) |
+		     FIELD_PREP_WM16(EDP_PHY_AUX_MODE_MASK, 0x1) |
+		     FIELD_PREP_WM16(EDP_PHY_AUX_AMP_SCALE_MASK, 0x1) |
+		     FIELD_PREP_WM16(EDP_PHY_AUX_AMP_MASK, 0x3) |
+		     FIELD_PREP_WM16(EDP_PHY_AUX_RTERM_MASK, 0x1));
+
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON10,
+		     FIELD_PREP_WM16(EDP_PHY_AUX_RCV_PD_MASK, 0x0) |
+		     FIELD_PREP_WM16(EDP_PHY_AUX_DRV_PD_MASK, 0x0));
+	usleep_range(100, 101);
+
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON10, FIELD_PREP_WM16(EDP_PHY_AUX_IDLE_MASK, 0x0));
+	usleep_range(10000, 11000);
+
+	return 0;
+}
+
+static int rockchip_edp_phy_power_off(struct phy *phy)
+{
+	struct rockchip_edp_phy *edpphy = phy_get_drvdata(phy);
+
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON0,
+		     FIELD_PREP_WM16(EDP_PHY_TX_IDLE_MASK, 0xf) |
+		     FIELD_PREP_WM16(EDP_PHY_TX_PD_MASK, 0xf));
+	usleep_range(100, 101);
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON5, FIELD_PREP_WM16(EDP_PHY_TX_MODE_MASK, 0x3));
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON0, FIELD_PREP_WM16(EDP_PHY_PD_PLL_MASK, 0x1));
+	regmap_write(edpphy->grf, EDP_PHY_GRF_CON10,
+		     FIELD_PREP_WM16(EDP_PHY_AUX_RCV_PD_MASK, 0x1) |
+		     FIELD_PREP_WM16(EDP_PHY_AUX_DRV_PD_MASK, 0x1) |
+		     FIELD_PREP_WM16(EDP_PHY_AUX_IDLE_MASK, 0x1));
+
+	clk_disable_unprepare(edpphy->refclk);
+
+	return 0;
+}
+
+static const struct phy_ops rockchip_edp_phy_ops = {
+	.power_on = rockchip_edp_phy_power_on,
+	.power_off = rockchip_edp_phy_power_off,
+	.configure = rockchip_edp_phy_configure,
+	.owner = THIS_MODULE,
+};
+
+static int rockchip_edp_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rockchip_edp_phy *edpphy;
+	struct phy *phy;
+	struct phy_provider *phy_provider;
+
+	edpphy = devm_kzalloc(dev, sizeof(*edpphy), GFP_KERNEL);
+	if (!edpphy)
+		return -ENOMEM;
+
+	edpphy->dev = dev;
+
+	edpphy->grf = syscon_node_to_regmap(dev->parent->of_node);
+	if (IS_ERR(edpphy->grf))
+		return dev_err_probe(dev, PTR_ERR(edpphy->grf), "Failed to get GRF syscon\n");
+
+	edpphy->refclk = devm_clk_get(dev, "refclk");
+	if (IS_ERR(edpphy->refclk))
+		return dev_err_probe(dev, PTR_ERR(edpphy->refclk), "Failed to get refclk\n");
+
+	phy = devm_phy_create(dev, NULL, &rockchip_edp_phy_ops);
+	if (IS_ERR(phy))
+		return dev_err_probe(dev, PTR_ERR(phy), "Failed to create PHY\n");
+
+	phy_set_drvdata(phy, edpphy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return dev_err_probe(dev, PTR_ERR(phy_provider),
+				     "Failed to register PHY provider\n");
+
+	return 0;
+}
+
+static const struct of_device_id rockchip_edp_phy_of_match[] = {
+	{ .compatible = "rockchip,rk3568-edp-phy", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rockchip_edp_phy_of_match);
+
+static struct platform_driver rockchip_edp_phy_driver = {
+	.driver = {
+		.name = "rockchip-edpphy-naneng",
+		.of_match_table	= of_match_ptr(rockchip_edp_phy_of_match),
+	},
+	.probe = rockchip_edp_phy_probe,
+};
+module_platform_driver(rockchip_edp_phy_driver);
+
+MODULE_AUTHOR("Damon Ding <damon.ding@rock-chips.com>");
+MODULE_DESCRIPTION("Rockchip Naneng eDP Transmitter PHY driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1


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

[PATCH v1 5/6] drm/rockchip: analogix_dp: Add support for RK3568

[Damon Ding]

The RK3568 eDP controller does not require any GRF configurations,
so make the "rockchip,grf" property optional.  Also enable the
optional avdd-0v9 and avdd-1v8 supplies which power the eDP PHY.

Add the "rockchip,rk3568-edp" compatible along with its chip data;
the register base in the chip data is used to match the platform
device memory resource.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 20 ++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index ccdd4b9ecdb8..5bd59e69ae96 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -320,8 +320,9 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
 	struct device *dev = dp->dev;
 	struct device_node *np = dev->of_node;
 	struct clk *clk;
+	int ret;
 
-	dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
+	dp->grf = syscon_regmap_lookup_by_phandle_optional(np, "rockchip,grf");
 	if (IS_ERR(dp->grf))
 		return dev_err_probe(dev, PTR_ERR(dp->grf),
 				     "failed to get rockchip,grf property\n");
@@ -351,6 +352,14 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
 		return dev_err_probe(dev, PTR_ERR(dp->apbrst),
 				     "failed to get apb reset control\n");
 
+	ret = devm_regulator_get_enable_optional(dev, "avdd-0v9");
+	if (ret && ret != -ENODEV)
+		return dev_err_probe(dev, ret, "failed to enable avdd-0v9\n");
+
+	ret = devm_regulator_get_enable_optional(dev, "avdd-1v8");
+	if (ret && ret != -ENODEV)
+		return dev_err_probe(dev, ret, "failed to enable avdd-1v8\n");
+
 	return 0;
 }
 
@@ -530,6 +539,14 @@ static const struct rockchip_dp_chip_data rk3288_dp[] = {
 	{ /* sentinel */ }
 };
 
+static const struct rockchip_dp_chip_data rk3568_edp[] = {
+	{
+		.chip_type = RK3568_EDP,
+		.reg = 0xfe0c0000,
+	},
+	{ /* sentinel */ }
+};
+
 static const struct rockchip_dp_chip_data rk3576_edp[] = {
 	{
 		.chip_type = RK3576_EDP,
@@ -555,6 +572,7 @@ static const struct rockchip_dp_chip_data rk3588_edp[] = {
 static const struct of_device_id rockchip_dp_dt_ids[] = {
 	{.compatible = "rockchip,rk3288-dp", .data = &rk3288_dp },
 	{.compatible = "rockchip,rk3399-edp", .data = &rk3399_edp },
+	{.compatible = "rockchip,rk3568-edp", .data = &rk3568_edp },
 	{.compatible = "rockchip,rk3576-edp", .data = &rk3576_edp },
 	{.compatible = "rockchip,rk3588-edp", .data = &rk3588_edp },
 	{}
-- 
2.34.1


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

[PATCH v1 6/6] arm64: dts: rockchip: rk356x: Add eDP controller and phy nodes

[Damon Ding]

Add the eDP PHY GRF (with the eDP PHY child node) and the eDP
controller nodes for the RK3568/RK3566 SoCs.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 arch/arm64/boot/dts/rockchip/rk356x-base.dtsi | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
index a5832895bd39..3b60e4355ad8 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
@@ -403,6 +403,20 @@ usb2phy1_grf: syscon@fdca8000 {
 		reg = <0x0 0xfdca8000 0x0 0x8000>;
 	};
 
+	edp_phy_grf: syscon@fdcb0000 {
+		compatible = "rockchip,rk3568-edp-phy-grf", "syscon", "simple-mfd";
+		reg = <0x0 0xfdcb0000 0x0 0x100>;
+		clocks = <&cru PCLK_EDPPHY_GRF>;
+
+		edp_phy: edp-phy {
+			compatible = "rockchip,rk3568-edp-phy";
+			clocks = <&pmucru XIN_OSC0_EDPPHY_G>;
+			clock-names = "refclk";
+			#phy-cells = <0>;
+			status = "disabled";
+		};
+	};
+
 	pmucru: clock-controller@fdd00000 {
 		compatible = "rockchip,rk3568-pmucru";
 		reg = <0x0 0xfdd00000 0x0 0x1000>;
@@ -879,6 +893,33 @@ hdmi_out: port@1 {
 		};
 	};
 
+	edp: edp@fe0c0000 {
+		compatible = "rockchip,rk3568-edp";
+		reg = <0x0 0xfe0c0000 0x0 0x10000>;
+		clocks = <&pmucru XIN_OSC0_EDPPHY_G>, <&cru PCLK_EDP_CTRL>, <&cru HCLK_VO>;
+		clock-names = "dp", "pclk", "hclk";
+		interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
+		phys = <&edp_phy>;
+		phy-names = "dp";
+		power-domains = <&power RK3568_PD_VO>;
+		resets = <&cru SRST_EDP_24M>, <&cru SRST_P_EDP_CTRL>;
+		reset-names = "dp", "apb";
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			edp_in: port@0 {
+				reg = <0>;
+			};
+
+			edp_out: port@1 {
+				reg = <1>;
+			};
+		};
+	};
+
 	qos_gpu: qos@fe128000 {
 		compatible = "rockchip,rk3568-qos", "syscon";
 		reg = <0x0 0xfe128000 0x0 0x20>;
-- 
2.34.1


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

Re: [PATCH v1 5/6] drm/rockchip: analogix_dp: Add support for RK3568

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rockchip/dp: unchecked NULL pointer dereference on missing GRF property
--

commit 9a6e6f45c854c22f24d403cf8bdab7a0504199b6
Author: Damon Ding <damon.ding@rock-chips.com>

drm/rockchip: analogix_dp: Add support for RK3568

The RK3568 eDP controller does not require any GRF configurations,
so make the "rockchip,grf" property optional.  Also enable the
optional avdd-0v9 and avdd-1v8 supplies which power the eDP PHY.

> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -320,8 +320,9 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
>  	struct device *dev = dp->dev;
>  	struct device_node *np = dev->of_node;
>  	struct clk *clk;
> +	int ret;
>  
> -	dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> +	dp->grf = syscon_regmap_lookup_by_phandle_optional(np, "rockchip,grf");
>  	if (IS_ERR(dp->grf))
>  		return dev_err_probe(dev, PTR_ERR(dp->grf),
>  				     "failed to get rockchip,grf property\n");

[Severity: High]
Since this changes the lookup to syscon_regmap_lookup_by_phandle_optional(),
dp->grf will be NULL when the property is absent, and the IS_ERR() check will
pass.

Can this cause a NULL pointer dereference for legacy SoCs (like RK3288, RK3399,
or RK3588) if the device tree is missing the property?

For those older SoCs, the chip_data specifies valid GRF fields. During display
initialization, the driver calls rockchip_grf_field_write():

drivers/gpu/drm/rockchip/analogix_dp-rockchip.c:rockchip_grf_field_write() {
	if (!field->valid)
		return 0;

	mask = GENMASK(field->msb, field->lsb);
	val <<= field->lsb;

	return rockchip_grf_write(grf, field->reg, mask, val);
}

This forwards the potentially NULL grf pointer to rockchip_grf_write() and
eventually to regmap_write(), which dereferences it:

drivers/base/regmap/regmap.c:regmap_write() {
	...
	if (!IS_ALIGNED(reg, map->reg_stride))
		return -EINVAL;
	...
}

Should probe validate that dp->grf != NULL if the chip type requires it,
rather than allowing probe to succeed and causing a kernel oops later?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013124946.1807886-1-damon.ding@rock-chips.com?part=5

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

Re: [PATCH v1 4/6] phy: rockchip: Add support for Naneng eDP phy

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] phy: rockchip: Missing set_lanes configuration and UB on unverified dp->lanes
- [Medium] Uninitialized variable val used if regmap_read fails
--

commit be568cbd60b1a73a69ea7b9233f6bd9cf9a8b067
Author: Damon Ding <damon.ding@rock-chips.com>

    phy: rockchip: Add support for Naneng eDP phy
    
    Add a driver for the Rockchip eDP transmitter PHY with NaNeng IP
    block, as found on the RK3568 SoC.

> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-edp.c b/drivers/phy/rockchip/phy-rockchip-naneng-edp.c
> new file mode 100644
> index 0000000000000..a407714f639a7
> --- /dev/null
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-edp.c
[ ... ]
> +static int rockchip_edp_phy_set_rate(struct rockchip_edp_phy *edpphy,
> +				     struct phy_configure_opts_dp *dp)
> +{
[ ... ]
> +	regmap_write(edpphy->grf, EDP_PHY_GRF_CON0, FIELD_PREP_WM16(EDP_PHY_PD_PLL_MASK, 0));
> +	regmap_write(edpphy->grf, EDP_PHY_GRF_CON0,
> +		     FIELD_PREP_WM16(EDP_PHY_TX_PD_MASK, ~GENMASK(dp->lanes - 1, 0)));

[Severity: High]
If a caller passes a struct where dp->lanes is 0, will this trigger undefined
behavior?

If dp->lanes is 0, ~GENMASK(dp->lanes - 1, 0) evaluates to
GENMASK(UINT_MAX, 0), triggering a negative right-shift count in C. This can
occur if a caller invokes phy_configure() with opts->dp.set_rate = 1 without
having first configured the lane count.

[ ... ]
> +static int rockchip_edp_phy_verify_config(struct rockchip_edp_phy *edpphy,
> +					  struct phy_configure_opts_dp *dp)
> +{
[ ... ]
> +	if (dp->set_lanes) {
> +		/* Verify lane count. */
> +		switch (dp->lanes) {
> +		case 1:
> +		case 2:
> +		case 4:
> +			/* valid lane count. */
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	/*
> +	 * If changing voltages is required, check swing and pre-emphasis
> +	 * levels, per-lane.
> +	 */
> +	if (dp->set_voltages) {
> +		/* Lane count verified previously. */
> +		for (i = 0; i < dp->lanes; i++) {
> +			if (dp->voltage[i] > 3 || dp->pre[i] > 3)
> +				return -EINVAL;

[Severity: High]
Is it possible for dp->lanes to be used unvalidated here?

The comment says "Lane count verified previously", but the validation of
dp->lanes is bypassed if dp->set_lanes is false. If a driver calls
phy_configure() with opts->dp.set_voltages = 1 and opts->dp.set_lanes = 0,
an uninitialized or invalid dp->lanes value could lead to out-of-bounds array
reads.

[ ... ]
> +static int rockchip_edp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
> +{
> +	struct rockchip_edp_phy *edpphy = phy_get_drvdata(phy);
> +	int ret;
> +
> +	ret = rockchip_edp_phy_verify_config(edpphy, &opts->dp);
> +	if (ret) {
> +		dev_err(edpphy->dev, "Invalid params for phy configure\n");
> +		return ret;
> +	}
> +
> +	if (opts->dp.set_rate) {
> +		ret = rockchip_edp_phy_set_rate(edpphy, &opts->dp);
> +		if (ret) {
> +			dev_err(edpphy->dev, "Failed to set rate\n");
> +			return ret;
> +		}
> +	}
> +
> +	if (opts->dp.set_voltages) {
> +		ret = rockchip_edp_phy_set_voltages(edpphy, &opts->dp);
> +		if (ret) {
> +			dev_err(edpphy->dev, "Failed to set voltages\n");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}

[Severity: High]
Is opts->dp.set_lanes intentionally ignored when configuring the PHY?

The opts->dp.set_lanes flag isn't handled here, meaning the hardware lane
count is never configured. This missing state also causes the uninitialized
dp->lanes issues in rockchip_edp_phy_set_rate() and
rockchip_edp_phy_set_voltages().

[ ... ]
> +static bool rockchip_edp_phy_enabled(struct rockchip_edp_phy *edpphy)
> +{
> +	u32 val;
> +
> +	regmap_read(edpphy->grf, EDP_PHY_GRF_STATUS0, &val);
> +
> +	return FIELD_GET(PLL_RDY, val);

[Severity: Medium]
If regmap_read() fails, does this return stack garbage?

A failure in regmap_read() leaves val uninitialized. Using the uninitialized
stack variable could cause the driver to incorrectly assume the PHY is already
enabled.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013124946.1807886-1-damon.ding@rock-chips.com?part=4

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