[PATCH v3 0/2] Add lontium lt8713sx bridge driver

[PATCH v3 0/2] Add lontium lt8713sx bridge driver

[Vishnu Saini]

The lt8713sx is a Type-C/DP1.4 to Type-C/DP1.4/HDMI2.0 converter,
with three configurable DP1.4/HDMI2.0/DP++ output interfaces and
audio output interface.

This series provides bridge driver and dt bindings for lt8713sx.
The driver is required for firmware upgrade in the bridge chip.

Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
---
Changes in v3:
- Used linux/sizes.h header for size definations.
- Used linux/crc8.h for CRC calculation
- Added Basic drm_bridge changes to support corresponding ports handeling in dt
- Ran coccinelle, smatch and sparse checkpatch.pl tools to improve code quality.
- Link to v2: https://lore.kernel.org/r/20251118-lt8713sx-bridge-driver-v2-0-25ad49280a11@oss.qualcomm.com

Changes in v2:
- Addressed review comments from V1, majorly:
- Fixed DCO chain.
- Added supply in bindings.
- Handeled deferred probe in lt8713sx driver probe.
- Link to v1: https://lore.kernel.org/r/20251115-lt8713sx-bridge-driver-v1-0-bd5a1c1c730a@oss.qualcomm.com

---
Vishnu Saini (2):
      dt-bindings: bridge: lt8713sx: Add bindings
      drm/bridge: add support for lontium lt8713sx bridge driver

 .../bindings/display/bridge/lontium,lt8713sx.yaml  | 101 +++
 drivers/gpu/drm/bridge/Kconfig                     |  10 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/lontium-lt8713sx.c          | 682 +++++++++++++++++++++
 4 files changed, 794 insertions(+)
---
base-commit: de0d6e19d2ef33ba34be2467ffdf3595da5f5326
change-id: 20251115-lt8713sx-bridge-driver-32704455bc9a

Best regards,
-- 
Vishnu Saini <vishnu.saini@oss.qualcomm.com>

[PATCH v3 1/2] dt-bindings: bridge: lt8713sx: Add bindings

[Vishnu Saini]

Add bindings for lt8713sx.

Co-developed-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
Signed-off-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
---
 .../bindings/display/bridge/lontium,lt8713sx.yaml  | 101 +++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt8713sx.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt8713sx.yaml
new file mode 100644
index 000000000000..0a6dc56e337c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt8713sx.yaml
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/lontium,lt8713sx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lontium LT8713SX Type-C/DP1.4 to Type-C/DP1.4/HDMI2.0/DP++ bridge-hub
+
+maintainers:
+  - Tony <syyang@lontium.com>
+
+description:
+  The Lontium LT8713SX is a Type-C/DP1.4 to Type-C/DP1.4/HDMI2.0 converter
+  that integrates one DP input and up to three configurable output interfaces
+  (DP1.4 / HDMI2.0 / DP++), with SST/MST functionality and audio support.
+
+properties:
+  compatible:
+    enum:
+      - lontium,lt8713sx
+
+  reg:
+    maxItems: 1
+
+  vcc-supply:
+    description: Regulator for 3.3V vcc.
+
+  vdd-supply:
+    description: Regulator for 1.1V vdd.
+
+  reset-gpios:
+    description: GPIO connected to active low RESET pin.
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          DP port for DP input from soc to bridge chip
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          DP port for DP output from bridge
+
+      port@2:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          Additional DP port for DP output from bridge
+
+    required:
+      - port@0
+
+required:
+  - compatible
+  - reg
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        bridge@4f {
+            compatible = "lontium,lt8713sx";
+            reg = <0x4f>;
+            reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    lt8713sx_dp_in: endpoint {
+                        remote-endpoint = <&mdss_dp0_out>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    lt8713sx_dp0_out: endpoint {
+                        remote-endpoint = <&dp0_connector_in>;
+                    };
+                };
+
+                port@2 {
+                    reg = <2>;
+                    lt8713sx_dp1_out: endpoint {
+                        remote-endpoint = <&dp1_connector_in>;
+                    };
+                };
+            };
+        };
+    };

-- 
2.34.1

[PATCH v3 2/2] drm/bridge: add support for lontium lt8713sx bridge driver

[Vishnu Saini]

The lt8713sx is a Type-C/DP1.4 to Type-C/DP1.4/HDMI2.0 converter,
with three configurable DP1.4/HDMI2.0/DP++ output interfaces and
audio output interface.

Driver is required for firmware upgrade in the bridge chip.

Co-developed-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
Signed-off-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
---
 drivers/gpu/drm/bridge/Kconfig            |  10 +
 drivers/gpu/drm/bridge/Makefile           |   1 +
 drivers/gpu/drm/bridge/lontium-lt8713sx.c | 682 ++++++++++++++++++++++++++++++
 3 files changed, 693 insertions(+)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index a250afd8d662..7fef383ed7cb 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -184,6 +184,16 @@ config DRM_LONTIUM_LT9611UXC
 	  HDMI signals
 	  Please say Y if you have such hardware.
 
+config DRM_LONTIUM_LT8713SX
+	tristate "Lontium LT8713SX DP MST bridge"
+	depends on OF
+	select REGMAP_I2C
+	help
+	  Driver for Lontium LT8713SX DP MST bridge
+	  chip firmware upgrade, which converts Type-C/DP1.4
+	  to 3 configurable Type-C/DP1.4/HDMI2.0 outputs
+	  Please say Y if you have such hardware.
+
 config DRM_ITE_IT66121
 	tristate "ITE IT66121 HDMI bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index c7dc03182e59..07eeb13fa497 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_DRM_LONTIUM_LT8912B) += lontium-lt8912b.o
 obj-$(CONFIG_DRM_LONTIUM_LT9211) += lontium-lt9211.o
 obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o
 obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o
+obj-$(CONFIG_DRM_LONTIUM_LT8713SX) += lontium-lt8713sx.o
 obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o
 obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o
 obj-$(CONFIG_DRM_MICROCHIP_LVDS_SERIALIZER) += microchip-lvds.o
diff --git a/drivers/gpu/drm/bridge/lontium-lt8713sx.c b/drivers/gpu/drm/bridge/lontium-lt8713sx.c
new file mode 100644
index 000000000000..6ea54ff3733d
--- /dev/null
+++ b/drivers/gpu/drm/bridge/lontium-lt8713sx.c
@@ -0,0 +1,682 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/crc8.h>
+#include <linux/firmware.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/sizes.h>
+#include <linux/wait.h>
+#include <linux/workqueue.h>
+
+#include <drm/drm_bridge.h>
+
+#define FW_FILE "lt8713sx_fw.bin"
+
+#define REG_PAGE_CONTROL	0xff
+
+#define MAX_OUTPUT_PORTS	3
+#define LT8713SX_PAGE_SIZE	256
+
+DECLARE_CRC8_TABLE(lt8713sx_crc_table);
+
+struct lt8713sx {
+	struct device *dev;
+	struct drm_bridge bridge;
+	struct drm_bridge *next_bridge[MAX_OUTPUT_PORTS];
+	int num_outputs;
+
+	struct regmap *regmap;
+	/* Protects all accesses to registers by stopping the on-chip MCU */
+	struct mutex ocm_lock;
+
+	struct gpio_desc *reset_gpio;
+	struct gpio_desc *enable_gpio;
+
+	struct i2c_client *client;
+	const struct firmware *fw;
+
+	u8 *fw_buffer;
+
+	u32 main_crc_value;
+	u32 bank_crc_value[17];
+
+	int bank_num;
+};
+
+static void lt8713sx_reset(struct lt8713sx *lt8713sx);
+
+static const struct regmap_range lt8713sx_ranges[] = {
+	{
+		.range_min = 0x0000,
+		.range_max = 0xffff
+	},
+};
+
+static const struct regmap_access_table lt8713sx_table = {
+	.yes_ranges = lt8713sx_ranges,
+	.n_yes_ranges = ARRAY_SIZE(lt8713sx_ranges),
+};
+
+static const struct regmap_range_cfg lt8713sx_range_cfg = {
+	.name = "lt8713sx",
+	.range_min = 0x0000,
+	.range_max = 0xffff,
+	.selector_reg = REG_PAGE_CONTROL,
+	.selector_mask = 0xff,
+	.selector_shift = 0,
+	.window_start = 0,
+	.window_len = 0x100,
+};
+
+static const struct regmap_config lt8713sx_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.volatile_table = &lt8713sx_table,
+	.ranges = &lt8713sx_range_cfg,
+	.num_ranges = 1,
+	.cache_type = REGCACHE_NONE,
+	.max_register = 0xffff,
+};
+
+static void lt8713sx_i2c_enable(struct lt8713sx *lt8713sx)
+{
+	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
+}
+
+static void lt8713sx_i2c_disable(struct lt8713sx *lt8713sx)
+{
+	regmap_write(lt8713sx->regmap, 0xe0ee, 0x00);
+}
+
+static u32 calculate_crc(const u8 *upgrade_data, u64 len, u64 crc_size)
+{
+	u8 crc = 0x00;
+	u8 pad = 0xff;
+
+	crc = crc8(lt8713sx_crc_table, upgrade_data, len, crc);
+
+	/* pad remaining bytes */
+	crc_size -= len;
+	while (crc_size--)
+		crc = crc8(lt8713sx_crc_table, &pad, 1, crc);
+
+	return crc;
+}
+
+static int lt8713sx_prepare_firmware_data(struct lt8713sx *lt8713sx)
+{
+	int ret = 0;
+	u64 sz_12k = 12 * SZ_1K;
+
+	ret = request_firmware(&lt8713sx->fw, FW_FILE, lt8713sx->dev);
+	if (ret < 0) {
+		dev_err(lt8713sx->dev, "request firmware failed\n");
+		return ret;
+	}
+
+	dev_dbg(lt8713sx->dev, "Firmware size: %zu bytes\n", lt8713sx->fw->size);
+
+	if (lt8713sx->fw->size > SZ_256K - 1) {
+		dev_err(lt8713sx->dev, "Firmware size exceeds 256KB limit\n");
+		release_firmware(lt8713sx->fw);
+		return -EINVAL;
+	}
+
+	lt8713sx->fw_buffer = kvmalloc(SZ_256K, GFP_KERNEL);
+	if (!lt8713sx->fw_buffer) {
+		release_firmware(lt8713sx->fw);
+		return -ENOMEM;
+	}
+
+	memset(lt8713sx->fw_buffer, 0xff, SZ_256K);
+
+	if (lt8713sx->fw->size < SZ_64K) {
+		memcpy(lt8713sx->fw_buffer, lt8713sx->fw->data, lt8713sx->fw->size);
+		lt8713sx->fw_buffer[SZ_64K - 1] =
+				calculate_crc(lt8713sx->fw->data, lt8713sx->fw->size, SZ_64K - 1);
+		lt8713sx->main_crc_value = lt8713sx->fw_buffer[SZ_64K - 1];
+		dev_dbg(lt8713sx->dev,
+			"Main Firmware Data  Crc=0x%02X\n", lt8713sx->main_crc_value);
+
+	} else {
+		/* main firmware */
+		memcpy(lt8713sx->fw_buffer, lt8713sx->fw->data, SZ_64K - 1);
+		lt8713sx->fw_buffer[SZ_64K - 1] =
+				calculate_crc(lt8713sx->fw_buffer, SZ_64K - 1, SZ_64K - 1);
+		lt8713sx->main_crc_value = lt8713sx->fw_buffer[SZ_64K - 1];
+		dev_dbg(lt8713sx->dev,
+			"Main Firmware Data  Crc=0x%02X\n", lt8713sx->main_crc_value);
+
+		/* bank firmware */
+		memcpy(lt8713sx->fw_buffer + SZ_64K,
+		       lt8713sx->fw->data + SZ_64K,
+		       lt8713sx->fw->size - SZ_64K);
+
+		lt8713sx->bank_num = (lt8713sx->fw->size - SZ_64K + sz_12k - 1) / sz_12k;
+		dev_dbg(lt8713sx->dev, "Bank Number Total is %d.\n", lt8713sx->bank_num);
+
+		for (int i = 0; i < lt8713sx->bank_num; i++) {
+			lt8713sx->bank_crc_value[i] =
+				calculate_crc(lt8713sx->fw_buffer + SZ_64K + i * sz_12k,
+					      sz_12k, sz_12k);
+			dev_dbg(lt8713sx->dev, "Bank number:%d; Firmware Data  Crc:0x%02X\n",
+				i, lt8713sx->bank_crc_value[i]);
+		}
+	}
+	return 0;
+}
+
+static void lt8713sx_config_parameters(struct lt8713sx *lt8713sx)
+{
+	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
+	regmap_write(lt8713sx->regmap, 0xe05e, 0xc1);
+	regmap_write(lt8713sx->regmap, 0xe058, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe059, 0x50);
+	regmap_write(lt8713sx->regmap, 0xe05a, 0x10);
+	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe058, 0x21);
+}
+
+static void lt8713sx_wren(struct lt8713sx *lt8713sx)
+{
+	regmap_write(lt8713sx->regmap, 0xe103, 0xbf);
+	regmap_write(lt8713sx->regmap, 0xe103, 0xff);
+	regmap_write(lt8713sx->regmap, 0xe05a, 0x04);
+	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
+}
+
+static void lt8713sx_wrdi(struct lt8713sx *lt8713sx)
+{
+	regmap_write(lt8713sx->regmap, 0xe05a, 0x08);
+	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
+}
+
+static void lt8713sx_fifo_reset(struct lt8713sx *lt8713sx)
+{
+	regmap_write(lt8713sx->regmap, 0xe103, 0xbf);
+	regmap_write(lt8713sx->regmap, 0xe103, 0xff);
+}
+
+static void lt8713sx_disable_sram_write(struct lt8713sx *lt8713sx)
+{
+	regmap_write(lt8713sx->regmap, 0xe055, 0x00);
+}
+
+static void lt8713sx_sram_to_flash(struct lt8713sx *lt8713sx)
+{
+	regmap_write(lt8713sx->regmap, 0xe05a, 0x30);
+	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
+}
+
+static void lt8713sx_i2c_to_sram(struct lt8713sx *lt8713sx)
+{
+	regmap_write(lt8713sx->regmap, 0xe055, 0x80);
+	regmap_write(lt8713sx->regmap, 0xe05e, 0xc0);
+	regmap_write(lt8713sx->regmap, 0xe058, 0x21);
+}
+
+static u8 lt8713sx_read_flash_status(struct lt8713sx *lt8713sx)
+{
+	u32 flash_status = 0;
+
+	regmap_write(lt8713sx->regmap,  0xe103, 0x3f);
+	regmap_write(lt8713sx->regmap,  0xe103, 0xff);
+
+	regmap_write(lt8713sx->regmap,  0xe05e, 0x40);
+	regmap_write(lt8713sx->regmap,  0xe056, 0x05); /* opcode=read status register */
+	regmap_write(lt8713sx->regmap,  0xe055, 0x25);
+	regmap_write(lt8713sx->regmap,  0xe055, 0x01);
+	regmap_write(lt8713sx->regmap,  0xe058, 0x21);
+
+	regmap_read(lt8713sx->regmap, 0xe05f, &flash_status);
+	dev_dbg(lt8713sx->dev, "flash_status:%x\n", flash_status);
+
+	return flash_status;
+}
+
+static void lt8713sx_block_erase(struct lt8713sx *lt8713sx)
+{
+	u32 i = 0;
+	u8 flash_status = 0;
+	u8 blocknum = 0x00;
+	u32 flashaddr = 0x00;
+
+	for (blocknum = 0; blocknum < 8; blocknum++) {
+		flashaddr = blocknum * SZ_32K;
+		regmap_write(lt8713sx->regmap,  0xe0ee, 0x01);
+		regmap_write(lt8713sx->regmap,  0xe05a, 0x04);
+		regmap_write(lt8713sx->regmap,  0xe05a, 0x00);
+		regmap_write(lt8713sx->regmap,  0xe05b, flashaddr >> 16);
+		regmap_write(lt8713sx->regmap,  0xe05c, flashaddr >> 8);
+		regmap_write(lt8713sx->regmap,  0xe05d, flashaddr);
+		regmap_write(lt8713sx->regmap,  0xe05a, 0x01);
+		regmap_write(lt8713sx->regmap,  0xe05a, 0x00);
+		msleep(100);
+		i = 0;
+		while (1) {
+			flash_status = lt8713sx_read_flash_status(lt8713sx);
+			if ((flash_status & 0x01) == 0)
+				break;
+
+			if (i > 50)
+				break;
+
+			i++;
+			msleep(50);
+		}
+	}
+	dev_dbg(lt8713sx->dev, "erase flash done.\n");
+}
+
+static void lt8713sx_load_main_fw_to_sram(struct lt8713sx *lt8713sx)
+{
+	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
+	regmap_write(lt8713sx->regmap, 0xe068, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe069, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe06a, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe065, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe066, 0xff);
+	regmap_write(lt8713sx->regmap, 0xe067, 0xff);
+	regmap_write(lt8713sx->regmap, 0xe06b, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe06c, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe060, 0x01);
+	msleep(200);
+	regmap_write(lt8713sx->regmap, 0xe060, 0x00);
+}
+
+static void lt8713sx_load_bank_fw_to_sram(struct lt8713sx *lt8713sx, u64 addr)
+{
+	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
+	regmap_write(lt8713sx->regmap, 0xe068, ((addr & 0xff0000) >> 16));
+	regmap_write(lt8713sx->regmap, 0xe069, ((addr & 0x00ff00) >> 8));
+	regmap_write(lt8713sx->regmap, 0xe06a, (addr & 0x0000ff));
+	regmap_write(lt8713sx->regmap, 0xe065, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe066, 0x30);
+	regmap_write(lt8713sx->regmap, 0xe067, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe06b, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe06c, 0x00);
+	regmap_write(lt8713sx->regmap, 0xe060, 0x01);
+	msleep(50);
+	regmap_write(lt8713sx->regmap, 0xe060, 0x00);
+}
+
+static int lt8713sx_write_data(struct lt8713sx *lt8713sx, const u8 *data, u64 filesize)
+{
+	int page = 0, num = 0, i = 0, val;
+
+	page = (filesize % LT8713SX_PAGE_SIZE) ?
+			((filesize / LT8713SX_PAGE_SIZE) + 1) : (filesize / LT8713SX_PAGE_SIZE);
+
+	dev_dbg(lt8713sx->dev,
+		"Writing to Sram=%u pages, total size = %llu bytes\n", page, filesize);
+
+	for (num = 0; num < page; num++) {
+		dev_dbg(lt8713sx->dev, "page[%d]\n", num);
+		lt8713sx_i2c_to_sram(lt8713sx);
+
+		for (i = 0; i < LT8713SX_PAGE_SIZE; i++) {
+			if ((num * LT8713SX_PAGE_SIZE + i) < filesize)
+				val = *(data + (num * LT8713SX_PAGE_SIZE + i));
+			else
+				val = 0xff;
+			regmap_write(lt8713sx->regmap, 0xe059, val);
+		}
+
+		lt8713sx_wren(lt8713sx);
+		lt8713sx_sram_to_flash(lt8713sx);
+	}
+
+	lt8713sx_wrdi(lt8713sx);
+	lt8713sx_disable_sram_write(lt8713sx);
+
+	return 0;
+}
+
+static void lt8713sx_main_upgrade_result(struct lt8713sx *lt8713sx)
+{
+	u32 main_crc_result;
+
+	regmap_read(lt8713sx->regmap, 0xe023, &main_crc_result);
+
+	dev_dbg(lt8713sx->dev, "Main CRC HW: 0x%02X\n", main_crc_result);
+	dev_dbg(lt8713sx->dev, "Main CRC FW: 0x%02X\n", lt8713sx->main_crc_value);
+
+	if (main_crc_result == lt8713sx->main_crc_value)
+		dev_dbg(lt8713sx->dev, "Main Firmware Upgrade Success.\n");
+	else
+		dev_err(lt8713sx->dev, "Main Firmware Upgrade Failed.\n");
+}
+
+static void lt8713sx_bank_upgrade_result(struct lt8713sx *lt8713sx, u8 banknum)
+{
+	u32 bank_crc_result;
+
+	regmap_read(lt8713sx->regmap, 0xe023, &bank_crc_result);
+
+	dev_dbg(lt8713sx->dev, "Bank %d CRC Result: 0x%02X\n", banknum, bank_crc_result);
+
+	if (bank_crc_result == lt8713sx->bank_crc_value[banknum])
+		dev_dbg(lt8713sx->dev, "Bank %d Firmware Upgrade Success.\n", banknum);
+	else
+		dev_err(lt8713sx->dev, "Bank %d Firmware Upgrade Failed.\n", banknum);
+}
+
+static void lt8713sx_bank_result_check(struct lt8713sx *lt8713sx)
+{
+	int i;
+	u64 addr = 0x010000;
+
+	for (i = 0; i < lt8713sx->bank_num; i++) {
+		lt8713sx_load_bank_fw_to_sram(lt8713sx, addr);
+		lt8713sx_bank_upgrade_result(lt8713sx, i);
+		addr += 0x3000;
+	}
+}
+
+static int lt8713sx_firmware_upgrade(struct lt8713sx *lt8713sx)
+{
+	int ret;
+
+	lt8713sx_config_parameters(lt8713sx);
+
+	lt8713sx_block_erase(lt8713sx);
+
+	if (lt8713sx->fw->size < SZ_64K) {
+		ret = lt8713sx_write_data(lt8713sx, lt8713sx->fw_buffer, SZ_64K);
+		if (ret < 0) {
+			dev_err(lt8713sx->dev, "Failed to write firmware data: %d\n", ret);
+			return ret;
+		}
+	} else {
+		ret = lt8713sx_write_data(lt8713sx, lt8713sx->fw_buffer, lt8713sx->fw->size);
+		if (ret < 0) {
+			dev_err(lt8713sx->dev, "Failed to write firmware data: %d\n", ret);
+			return ret;
+		}
+	}
+	dev_dbg(lt8713sx->dev, "Write Data done.\n");
+
+	return 0;
+}
+
+static int lt8713sx_firmware_update(struct lt8713sx *lt8713sx)
+{
+	int ret = 0;
+
+	mutex_lock(&lt8713sx->ocm_lock);
+	lt8713sx_i2c_enable(lt8713sx);
+
+	ret = lt8713sx_prepare_firmware_data(lt8713sx);
+	if (ret < 0) {
+		dev_err(lt8713sx->dev, "Failed to prepare firmware data: %d\n", ret);
+		goto error;
+	}
+
+	ret = lt8713sx_firmware_upgrade(lt8713sx);
+	if (ret < 0) {
+		dev_err(lt8713sx->dev, "Upgrade failure.\n");
+		goto error;
+	} else {
+		/* Validate CRC */
+		lt8713sx_load_main_fw_to_sram(lt8713sx);
+		lt8713sx_main_upgrade_result(lt8713sx);
+		lt8713sx_wrdi(lt8713sx);
+		lt8713sx_fifo_reset(lt8713sx);
+		lt8713sx_bank_result_check(lt8713sx);
+		lt8713sx_wrdi(lt8713sx);
+	}
+
+error:
+	lt8713sx_i2c_disable(lt8713sx);
+	if (!ret)
+		lt8713sx_reset(lt8713sx);
+
+	kvfree(lt8713sx->fw_buffer);
+	lt8713sx->fw_buffer = NULL;
+
+	if (lt8713sx->fw) {
+		release_firmware(lt8713sx->fw);
+		lt8713sx->fw = NULL;
+	}
+	mutex_unlock(&lt8713sx->ocm_lock);
+
+	return ret;
+}
+
+static void lt8713sx_reset(struct lt8713sx *lt8713sx)
+{
+	dev_dbg(lt8713sx->dev, "reset bridge.\n");
+	gpiod_set_value_cansleep(lt8713sx->reset_gpio, 1);
+	msleep(20);
+
+	gpiod_set_value_cansleep(lt8713sx->reset_gpio, 0);
+	msleep(20);
+
+	dev_dbg(lt8713sx->dev, "reset done.\n");
+}
+
+static int lt8713sx_regulator_enable(struct lt8713sx *lt8713sx)
+{
+	int ret;
+
+	ret = devm_regulator_get_enable(lt8713sx->dev, "vdd");
+	if (ret < 0)
+		return dev_err_probe(lt8713sx->dev, ret, "failed to enable vdd regulator\n");
+
+	usleep_range(1000, 10000);
+
+	ret = devm_regulator_get_enable(lt8713sx->dev, "vcc");
+	if (ret < 0)
+		return dev_err_probe(lt8713sx->dev, ret, "failed to enable vcc regulator\n");
+	return 0;
+}
+
+static int lt8713sx_bridge_attach(struct drm_bridge *bridge,
+				  struct drm_encoder *encoder,
+				  enum drm_bridge_attach_flags flags)
+{
+	struct lt8713sx *lt8713sx = container_of(bridge, struct lt8713sx, bridge);
+	int i, ret;
+
+	for (i = 0; i < lt8713sx->num_outputs; i++) {
+		if (!lt8713sx->next_bridge[i])
+			continue;
+
+		ret = drm_bridge_attach(encoder,
+					lt8713sx->next_bridge[i],
+					bridge, flags);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int lt8713sx_get_ports(struct lt8713sx *lt8713sx)
+{
+	struct device *dev = lt8713sx->dev;
+	struct device_node *port, *ports, *ep, *remote;
+	int i = 0;
+	u32 reg;
+
+	ports = of_get_child_by_name(dev->of_node, "ports");
+	if (!ports)
+		return -ENODEV;
+
+	for_each_child_of_node(ports, port) {
+		if (of_property_read_u32(port, "reg", &reg))
+			continue;
+
+		if (reg == 0)
+			continue;
+
+		if (i >= ARRAY_SIZE(lt8713sx->next_bridge)) {
+			of_node_put(port);
+			break;
+		}
+
+		ep = of_graph_get_next_endpoint(port, NULL);
+		if (!ep)
+			continue;
+
+		remote = of_graph_get_remote_port_parent(ep);
+		of_node_put(ep);
+
+		if (!remote)
+			continue;
+
+		lt8713sx->next_bridge[i] = of_drm_find_bridge(remote);
+		of_node_put(remote);
+		if (lt8713sx->next_bridge[i])
+			i++;
+	}
+	lt8713sx->num_outputs = i;
+	dev_dbg(dev, "Enabled %d output ports", i);
+
+	of_node_put(ports);
+	return 0;
+};
+
+static int lt8713sx_gpio_init(struct lt8713sx *lt8713sx)
+{
+	struct device *dev = lt8713sx->dev;
+
+	lt8713sx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(lt8713sx->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(lt8713sx->reset_gpio),
+				     "failed to acquire reset gpio\n");
+
+	/* power enable gpio */
+	lt8713sx->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(lt8713sx->enable_gpio))
+		return dev_err_probe(dev, PTR_ERR(lt8713sx->enable_gpio),
+				     "failed to acquire enable gpio\n");
+	return 0;
+}
+
+static ssize_t lt8713sx_firmware_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t len)
+{
+	struct lt8713sx *lt8713sx = dev_get_drvdata(dev);
+	int ret;
+
+	ret = lt8713sx_firmware_update(lt8713sx);
+	if (ret < 0)
+		return ret;
+	return len;
+}
+
+static DEVICE_ATTR_WO(lt8713sx_firmware);
+
+static struct attribute *lt8713sx_attrs[] = {
+	&dev_attr_lt8713sx_firmware.attr,
+	NULL,
+};
+
+static const struct attribute_group lt8713sx_attr_group = {
+	.attrs = lt8713sx_attrs,
+};
+
+static const struct attribute_group *lt8713sx_attr_groups[] = {
+	&lt8713sx_attr_group,
+	NULL,
+};
+
+static const struct drm_bridge_funcs lt8713sx_bridge_funcs = {
+	.attach = lt8713sx_bridge_attach,
+};
+
+static int lt8713sx_probe(struct i2c_client *client)
+{
+	struct lt8713sx *lt8713sx;
+	struct device *dev = &client->dev;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return dev_err_probe(dev, -ENODEV, "device doesn't support I2C\n");
+
+	lt8713sx = devm_drm_bridge_alloc(dev, struct lt8713sx, bridge, &lt8713sx_bridge_funcs);
+	if (IS_ERR(lt8713sx))
+		return PTR_ERR(lt8713sx);
+
+	lt8713sx->dev = dev;
+	lt8713sx->client = client;
+	i2c_set_clientdata(client, lt8713sx);
+
+	ret = devm_mutex_init(lt8713sx->dev, &lt8713sx->ocm_lock);
+	if (ret)
+		return ret;
+
+	lt8713sx->regmap = devm_regmap_init_i2c(client, &lt8713sx_regmap_config);
+	if (IS_ERR(lt8713sx->regmap))
+		return dev_err_probe(dev, PTR_ERR(lt8713sx->regmap), "regmap i2c init failed\n");
+
+	ret = lt8713sx_get_ports(lt8713sx);
+	if (ret < 0)
+		return ret;
+
+	ret = lt8713sx_gpio_init(lt8713sx);
+	if (ret < 0)
+		return ret;
+
+	ret = lt8713sx_regulator_enable(lt8713sx);
+	if (ret)
+		return ret;
+
+	lt8713sx_reset(lt8713sx);
+
+	lt8713sx->bridge.funcs = &lt8713sx_bridge_funcs;
+	lt8713sx->bridge.of_node = dev->of_node;
+	lt8713sx->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
+	drm_bridge_add(&lt8713sx->bridge);
+
+	crc8_populate_msb(lt8713sx_crc_table, 0x31);
+
+	return 0;
+}
+
+static void lt8713sx_remove(struct i2c_client *client)
+{
+	struct lt8713sx *lt8713sx = i2c_get_clientdata(client);
+
+	drm_bridge_remove(&lt8713sx->bridge);
+}
+
+static struct i2c_device_id lt8713sx_id[] = {
+	{ "lontium,lt8713sx", 0 },
+	{ /* sentinel */ }
+};
+
+static const struct of_device_id lt8713sx_match_table[] = {
+	{ .compatible = "lontium,lt8713sx" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, lt8713sx_match_table);
+
+static struct i2c_driver lt8713sx_driver = {
+	.driver = {
+		.name = "lt8713sx",
+		.of_match_table = lt8713sx_match_table,
+		.dev_groups = lt8713sx_attr_groups,
+	},
+	.probe = lt8713sx_probe,
+	.remove = lt8713sx_remove,
+	.id_table = lt8713sx_id,
+};
+
+module_i2c_driver(lt8713sx_driver);
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("lt8713sx drm bridge driver");
+MODULE_AUTHOR("Tony <syyang@lontium.com>");
+MODULE_FIRMWARE(FW_FILE);

-- 
2.34.1

Re: [PATCH v3 1/2] dt-bindings: bridge: lt8713sx: Add bindings

[Krzysztof Kozlowski]

On 28/12/2025 12:40, Vishnu Saini wrote:
> Add bindings for lt8713sx.
> 
> Co-developed-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> Signed-off-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> ---



Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Re: [PATCH v3 1/2] dt-bindings: bridge: lt8713sx: Add bindings

[Dmitry Baryshkov]

On Sun, Dec 28, 2025 at 05:10:39PM +0530, Vishnu Saini wrote:
> Add bindings for lt8713sx.
> 
> Co-developed-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> Signed-off-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> ---
>  .../bindings/display/bridge/lontium,lt8713sx.yaml  | 101 +++++++++++++++++++++
>  1 file changed, 101 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt8713sx.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt8713sx.yaml
> new file mode 100644
> index 000000000000..0a6dc56e337c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt8713sx.yaml
> @@ -0,0 +1,101 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/lontium,lt8713sx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Lontium LT8713SX Type-C/DP1.4 to Type-C/DP1.4/HDMI2.0/DP++ bridge-hub
> +
> +maintainers:
> +  - Tony <syyang@lontium.com>

Is he/she aware of this?

> +
> +description:
> +  The Lontium LT8713SX is a Type-C/DP1.4 to Type-C/DP1.4/HDMI2.0 converter
> +  that integrates one DP input and up to three configurable output interfaces
> +  (DP1.4 / HDMI2.0 / DP++), with SST/MST functionality and audio support.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - lontium,lt8713sx
> +
> +  reg:
> +    maxItems: 1
> +
> +  vcc-supply:
> +    description: Regulator for 3.3V vcc.
> +
> +  vdd-supply:
> +    description: Regulator for 1.1V vdd.
> +
> +  reset-gpios:
> +    description: GPIO connected to active low RESET pin.
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          DP port for DP input from soc to bridge chip
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          DP port for DP output from bridge
> +
> +      port@2:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          Additional DP port for DP output from bridge

Why do you have only two output ports here? The datasheet documents
three ports.

> +
> +    required:
> +      - port@0
> +
> +required:
> +  - compatible
> +  - reg
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        bridge@4f {
> +            compatible = "lontium,lt8713sx";
> +            reg = <0x4f>;
> +            reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
> +
> +            ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                port@0 {
> +                    reg = <0>;
> +                    lt8713sx_dp_in: endpoint {
> +                        remote-endpoint = <&mdss_dp0_out>;
> +                    };
> +                };
> +
> +                port@1 {
> +                    reg = <1>;
> +                    lt8713sx_dp0_out: endpoint {
> +                        remote-endpoint = <&dp0_connector_in>;
> +                    };
> +                };
> +
> +                port@2 {
> +                    reg = <2>;
> +                    lt8713sx_dp1_out: endpoint {
> +                        remote-endpoint = <&dp1_connector_in>;
> +                    };
> +                };
> +            };
> +        };
> +    };
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3 2/2] drm/bridge: add support for lontium lt8713sx bridge driver

[Dmitry Baryshkov]

On Sun, Dec 28, 2025 at 05:10:40PM +0530, Vishnu Saini wrote:
> The lt8713sx is a Type-C/DP1.4 to Type-C/DP1.4/HDMI2.0 converter,
> with three configurable DP1.4/HDMI2.0/DP++ output interfaces and
> audio output interface.
> 
> Driver is required for firmware upgrade in the bridge chip.

Or for enabling the bridge chip. Or for building the bridge chains
inside Linux kernel...

Also, do we need any special handling for DP++ / HDMI devices being
attached to the bridge?

> 
> Co-developed-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> Signed-off-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/bridge/Kconfig            |  10 +
>  drivers/gpu/drm/bridge/Makefile           |   1 +
>  drivers/gpu/drm/bridge/lontium-lt8713sx.c | 682 ++++++++++++++++++++++++++++++
>  3 files changed, 693 insertions(+)
> 
> +
> +#define FW_FILE "lt8713sx_fw.bin"

I'm still looking forward to seeing this file in linux-firmware.

> +
> +#define REG_PAGE_CONTROL	0xff
> +
> +#define MAX_OUTPUT_PORTS	3
> +#define LT8713SX_PAGE_SIZE	256
> +
> +DECLARE_CRC8_TABLE(lt8713sx_crc_table);
> +
> +struct lt8713sx {
> +	struct device *dev;
> +	struct drm_bridge bridge;
> +	struct drm_bridge *next_bridge[MAX_OUTPUT_PORTS];
> +	int num_outputs;
> +
> +	struct regmap *regmap;
> +	/* Protects all accesses to registers by stopping the on-chip MCU */
> +	struct mutex ocm_lock;
> +
> +	struct gpio_desc *reset_gpio;
> +	struct gpio_desc *enable_gpio;
> +
> +	struct i2c_client *client;
> +	const struct firmware *fw;
> +
> +	u8 *fw_buffer;
> +
> +	u32 main_crc_value;
> +	u32 bank_crc_value[17];
> +
> +	int bank_num;
> +};
> +
> +static void lt8713sx_reset(struct lt8713sx *lt8713sx);
> +
> +static const struct regmap_range lt8713sx_ranges[] = {
> +	{
> +		.range_min = 0x0000,
> +		.range_max = 0xffff
> +	},
> +};
> +
> +static const struct regmap_access_table lt8713sx_table = {
> +	.yes_ranges = lt8713sx_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(lt8713sx_ranges),
> +};
> +
> +static const struct regmap_range_cfg lt8713sx_range_cfg = {
> +	.name = "lt8713sx",
> +	.range_min = 0x0000,
> +	.range_max = 0xffff,
> +	.selector_reg = REG_PAGE_CONTROL,
> +	.selector_mask = 0xff,
> +	.selector_shift = 0,
> +	.window_start = 0,
> +	.window_len = 0x100,
> +};
> +
> +static const struct regmap_config lt8713sx_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.volatile_table = &lt8713sx_table,
> +	.ranges = &lt8713sx_range_cfg,
> +	.num_ranges = 1,
> +	.cache_type = REGCACHE_NONE,
> +	.max_register = 0xffff,
> +};
> +
> +static void lt8713sx_i2c_enable(struct lt8713sx *lt8713sx)
> +{
> +	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> +}
> +
> +static void lt8713sx_i2c_disable(struct lt8713sx *lt8713sx)
> +{
> +	regmap_write(lt8713sx->regmap, 0xe0ee, 0x00);
> +}
> +
> +static u32 calculate_crc(const u8 *upgrade_data, u64 len, u64 crc_size)
> +{
> +	u8 crc = 0x00;
> +	u8 pad = 0xff;
> +
> +	crc = crc8(lt8713sx_crc_table, upgrade_data, len, crc);
> +
> +	/* pad remaining bytes */
> +	crc_size -= len;
> +	while (crc_size--)
> +		crc = crc8(lt8713sx_crc_table, &pad, 1, crc);

As I wrote before, you are basically calculating the CRC over the padded
buffer. Please pad the buffer and calculate it over the whole buffer
rather than doing this byte by byte.

> +
> +	return crc;
> +}
> +
> +static int lt8713sx_prepare_firmware_data(struct lt8713sx *lt8713sx)
> +{
> +	int ret = 0;
> +	u64 sz_12k = 12 * SZ_1K;
> +
> +	ret = request_firmware(&lt8713sx->fw, FW_FILE, lt8713sx->dev);
> +	if (ret < 0) {
> +		dev_err(lt8713sx->dev, "request firmware failed\n");
> +		return ret;
> +	}
> +
> +	dev_dbg(lt8713sx->dev, "Firmware size: %zu bytes\n", lt8713sx->fw->size);
> +
> +	if (lt8713sx->fw->size > SZ_256K - 1) {
> +		dev_err(lt8713sx->dev, "Firmware size exceeds 256KB limit\n");
> +		release_firmware(lt8713sx->fw);
> +		return -EINVAL;
> +	}
> +
> +	lt8713sx->fw_buffer = kvmalloc(SZ_256K, GFP_KERNEL);
> +	if (!lt8713sx->fw_buffer) {
> +		release_firmware(lt8713sx->fw);
> +		return -ENOMEM;
> +	}
> +
> +	memset(lt8713sx->fw_buffer, 0xff, SZ_256K);
> +
> +	if (lt8713sx->fw->size < SZ_64K) {
> +		memcpy(lt8713sx->fw_buffer, lt8713sx->fw->data, lt8713sx->fw->size);
> +		lt8713sx->fw_buffer[SZ_64K - 1] =
> +				calculate_crc(lt8713sx->fw->data, lt8713sx->fw->size, SZ_64K - 1);
> +		lt8713sx->main_crc_value = lt8713sx->fw_buffer[SZ_64K - 1];
> +		dev_dbg(lt8713sx->dev,
> +			"Main Firmware Data  Crc=0x%02X\n", lt8713sx->main_crc_value);
> +
> +	} else {
> +		/* main firmware */
> +		memcpy(lt8713sx->fw_buffer, lt8713sx->fw->data, SZ_64K - 1);
> +		lt8713sx->fw_buffer[SZ_64K - 1] =
> +				calculate_crc(lt8713sx->fw_buffer, SZ_64K - 1, SZ_64K - 1);
> +		lt8713sx->main_crc_value = lt8713sx->fw_buffer[SZ_64K - 1];
> +		dev_dbg(lt8713sx->dev,
> +			"Main Firmware Data  Crc=0x%02X\n", lt8713sx->main_crc_value);
> +
> +		/* bank firmware */
> +		memcpy(lt8713sx->fw_buffer + SZ_64K,
> +		       lt8713sx->fw->data + SZ_64K,
> +		       lt8713sx->fw->size - SZ_64K);
> +
> +		lt8713sx->bank_num = (lt8713sx->fw->size - SZ_64K + sz_12k - 1) / sz_12k;
> +		dev_dbg(lt8713sx->dev, "Bank Number Total is %d.\n", lt8713sx->bank_num);
> +
> +		for (int i = 0; i < lt8713sx->bank_num; i++) {
> +			lt8713sx->bank_crc_value[i] =
> +				calculate_crc(lt8713sx->fw_buffer + SZ_64K + i * sz_12k,
> +					      sz_12k, sz_12k);
> +			dev_dbg(lt8713sx->dev, "Bank number:%d; Firmware Data  Crc:0x%02X\n",
> +				i, lt8713sx->bank_crc_value[i]);
> +		}
> +	}
> +	return 0;
> +}
> +
> +static void lt8713sx_config_parameters(struct lt8713sx *lt8713sx)
> +{
> +	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> +	regmap_write(lt8713sx->regmap, 0xe05e, 0xc1);
> +	regmap_write(lt8713sx->regmap, 0xe058, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe059, 0x50);
> +	regmap_write(lt8713sx->regmap, 0xe05a, 0x10);
> +	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe058, 0x21);
> +}
> +
> +static void lt8713sx_wren(struct lt8713sx *lt8713sx)
> +{
> +	regmap_write(lt8713sx->regmap, 0xe103, 0xbf);
> +	regmap_write(lt8713sx->regmap, 0xe103, 0xff);
> +	regmap_write(lt8713sx->regmap, 0xe05a, 0x04);
> +	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> +}
> +
> +static void lt8713sx_wrdi(struct lt8713sx *lt8713sx)
> +{
> +	regmap_write(lt8713sx->regmap, 0xe05a, 0x08);
> +	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> +}
> +
> +static void lt8713sx_fifo_reset(struct lt8713sx *lt8713sx)
> +{
> +	regmap_write(lt8713sx->regmap, 0xe103, 0xbf);
> +	regmap_write(lt8713sx->regmap, 0xe103, 0xff);
> +}
> +
> +static void lt8713sx_disable_sram_write(struct lt8713sx *lt8713sx)
> +{
> +	regmap_write(lt8713sx->regmap, 0xe055, 0x00);
> +}
> +
> +static void lt8713sx_sram_to_flash(struct lt8713sx *lt8713sx)
> +{
> +	regmap_write(lt8713sx->regmap, 0xe05a, 0x30);
> +	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> +}
> +
> +static void lt8713sx_i2c_to_sram(struct lt8713sx *lt8713sx)
> +{
> +	regmap_write(lt8713sx->regmap, 0xe055, 0x80);
> +	regmap_write(lt8713sx->regmap, 0xe05e, 0xc0);
> +	regmap_write(lt8713sx->regmap, 0xe058, 0x21);
> +}
> +
> +static u8 lt8713sx_read_flash_status(struct lt8713sx *lt8713sx)
> +{
> +	u32 flash_status = 0;
> +
> +	regmap_write(lt8713sx->regmap,  0xe103, 0x3f);
> +	regmap_write(lt8713sx->regmap,  0xe103, 0xff);
> +
> +	regmap_write(lt8713sx->regmap,  0xe05e, 0x40);
> +	regmap_write(lt8713sx->regmap,  0xe056, 0x05); /* opcode=read status register */
> +	regmap_write(lt8713sx->regmap,  0xe055, 0x25);
> +	regmap_write(lt8713sx->regmap,  0xe055, 0x01);
> +	regmap_write(lt8713sx->regmap,  0xe058, 0x21);
> +
> +	regmap_read(lt8713sx->regmap, 0xe05f, &flash_status);
> +	dev_dbg(lt8713sx->dev, "flash_status:%x\n", flash_status);
> +
> +	return flash_status;
> +}
> +
> +static void lt8713sx_block_erase(struct lt8713sx *lt8713sx)
> +{
> +	u32 i = 0;
> +	u8 flash_status = 0;
> +	u8 blocknum = 0x00;
> +	u32 flashaddr = 0x00;
> +
> +	for (blocknum = 0; blocknum < 8; blocknum++) {
> +		flashaddr = blocknum * SZ_32K;
> +		regmap_write(lt8713sx->regmap,  0xe0ee, 0x01);

lt8713sx_i2c_enable() ? Why is it set again?

> +		regmap_write(lt8713sx->regmap,  0xe05a, 0x04);
> +		regmap_write(lt8713sx->regmap,  0xe05a, 0x00);
> +		regmap_write(lt8713sx->regmap,  0xe05b, flashaddr >> 16);
> +		regmap_write(lt8713sx->regmap,  0xe05c, flashaddr >> 8);
> +		regmap_write(lt8713sx->regmap,  0xe05d, flashaddr);
> +		regmap_write(lt8713sx->regmap,  0xe05a, 0x01);
> +		regmap_write(lt8713sx->regmap,  0xe05a, 0x00);
> +		msleep(100);
> +		i = 0;
> +		while (1) {
> +			flash_status = lt8713sx_read_flash_status(lt8713sx);
> +			if ((flash_status & 0x01) == 0)
> +				break;
> +
> +			if (i > 50)
> +				break;
> +
> +			i++;
> +			msleep(50);
> +		}
> +	}
> +	dev_dbg(lt8713sx->dev, "erase flash done.\n");
> +}
> +
> +static void lt8713sx_load_main_fw_to_sram(struct lt8713sx *lt8713sx)
> +{
> +	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> +	regmap_write(lt8713sx->regmap, 0xe068, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe069, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe06a, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe065, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe066, 0xff);
> +	regmap_write(lt8713sx->regmap, 0xe067, 0xff);
> +	regmap_write(lt8713sx->regmap, 0xe06b, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe06c, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe060, 0x01);
> +	msleep(200);
> +	regmap_write(lt8713sx->regmap, 0xe060, 0x00);
> +}
> +
> +static void lt8713sx_load_bank_fw_to_sram(struct lt8713sx *lt8713sx, u64 addr)
> +{
> +	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> +	regmap_write(lt8713sx->regmap, 0xe068, ((addr & 0xff0000) >> 16));
> +	regmap_write(lt8713sx->regmap, 0xe069, ((addr & 0x00ff00) >> 8));
> +	regmap_write(lt8713sx->regmap, 0xe06a, (addr & 0x0000ff));
> +	regmap_write(lt8713sx->regmap, 0xe065, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe066, 0x30);
> +	regmap_write(lt8713sx->regmap, 0xe067, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe06b, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe06c, 0x00);
> +	regmap_write(lt8713sx->regmap, 0xe060, 0x01);
> +	msleep(50);
> +	regmap_write(lt8713sx->regmap, 0xe060, 0x00);
> +}
> +
> +static int lt8713sx_write_data(struct lt8713sx *lt8713sx, const u8 *data, u64 filesize)
> +{
> +	int page = 0, num = 0, i = 0, val;
> +
> +	page = (filesize % LT8713SX_PAGE_SIZE) ?
> +			((filesize / LT8713SX_PAGE_SIZE) + 1) : (filesize / LT8713SX_PAGE_SIZE);
> +
> +	dev_dbg(lt8713sx->dev,
> +		"Writing to Sram=%u pages, total size = %llu bytes\n", page, filesize);
> +
> +	for (num = 0; num < page; num++) {
> +		dev_dbg(lt8713sx->dev, "page[%d]\n", num);
> +		lt8713sx_i2c_to_sram(lt8713sx);
> +
> +		for (i = 0; i < LT8713SX_PAGE_SIZE; i++) {
> +			if ((num * LT8713SX_PAGE_SIZE + i) < filesize)
> +				val = *(data + (num * LT8713SX_PAGE_SIZE + i));
> +			else
> +				val = 0xff;
> +			regmap_write(lt8713sx->regmap, 0xe059, val);
> +		}
> +
> +		lt8713sx_wren(lt8713sx);
> +		lt8713sx_sram_to_flash(lt8713sx);
> +	}
> +
> +	lt8713sx_wrdi(lt8713sx);
> +	lt8713sx_disable_sram_write(lt8713sx);
> +
> +	return 0;
> +}
> +
> +static void lt8713sx_main_upgrade_result(struct lt8713sx *lt8713sx)
> +{
> +	u32 main_crc_result;
> +
> +	regmap_read(lt8713sx->regmap, 0xe023, &main_crc_result);
> +
> +	dev_dbg(lt8713sx->dev, "Main CRC HW: 0x%02X\n", main_crc_result);
> +	dev_dbg(lt8713sx->dev, "Main CRC FW: 0x%02X\n", lt8713sx->main_crc_value);
> +
> +	if (main_crc_result == lt8713sx->main_crc_value)
> +		dev_dbg(lt8713sx->dev, "Main Firmware Upgrade Success.\n");
> +	else
> +		dev_err(lt8713sx->dev, "Main Firmware Upgrade Failed.\n");
> +}
> +
> +static void lt8713sx_bank_upgrade_result(struct lt8713sx *lt8713sx, u8 banknum)
> +{
> +	u32 bank_crc_result;
> +
> +	regmap_read(lt8713sx->regmap, 0xe023, &bank_crc_result);
> +
> +	dev_dbg(lt8713sx->dev, "Bank %d CRC Result: 0x%02X\n", banknum, bank_crc_result);
> +
> +	if (bank_crc_result == lt8713sx->bank_crc_value[banknum])
> +		dev_dbg(lt8713sx->dev, "Bank %d Firmware Upgrade Success.\n", banknum);
> +	else
> +		dev_err(lt8713sx->dev, "Bank %d Firmware Upgrade Failed.\n", banknum);
> +}
> +
> +static void lt8713sx_bank_result_check(struct lt8713sx *lt8713sx)
> +{
> +	int i;
> +	u64 addr = 0x010000;
> +
> +	for (i = 0; i < lt8713sx->bank_num; i++) {
> +		lt8713sx_load_bank_fw_to_sram(lt8713sx, addr);
> +		lt8713sx_bank_upgrade_result(lt8713sx, i);
> +		addr += 0x3000;
> +	}
> +}
> +
> +static int lt8713sx_firmware_upgrade(struct lt8713sx *lt8713sx)
> +{
> +	int ret;
> +
> +	lt8713sx_config_parameters(lt8713sx);
> +
> +	lt8713sx_block_erase(lt8713sx);
> +
> +	if (lt8713sx->fw->size < SZ_64K) {
> +		ret = lt8713sx_write_data(lt8713sx, lt8713sx->fw_buffer, SZ_64K);
> +		if (ret < 0) {
> +			dev_err(lt8713sx->dev, "Failed to write firmware data: %d\n", ret);
> +			return ret;
> +		}
> +	} else {
> +		ret = lt8713sx_write_data(lt8713sx, lt8713sx->fw_buffer, lt8713sx->fw->size);
> +		if (ret < 0) {
> +			dev_err(lt8713sx->dev, "Failed to write firmware data: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +	dev_dbg(lt8713sx->dev, "Write Data done.\n");
> +
> +	return 0;
> +}
> +
> +static int lt8713sx_firmware_update(struct lt8713sx *lt8713sx)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&lt8713sx->ocm_lock);
> +	lt8713sx_i2c_enable(lt8713sx);
> +
> +	ret = lt8713sx_prepare_firmware_data(lt8713sx);
> +	if (ret < 0) {
> +		dev_err(lt8713sx->dev, "Failed to prepare firmware data: %d\n", ret);
> +		goto error;
> +	}
> +
> +	ret = lt8713sx_firmware_upgrade(lt8713sx);
> +	if (ret < 0) {
> +		dev_err(lt8713sx->dev, "Upgrade failure.\n");
> +		goto error;
> +	} else {
> +		/* Validate CRC */
> +		lt8713sx_load_main_fw_to_sram(lt8713sx);
> +		lt8713sx_main_upgrade_result(lt8713sx);
> +		lt8713sx_wrdi(lt8713sx);
> +		lt8713sx_fifo_reset(lt8713sx);
> +		lt8713sx_bank_result_check(lt8713sx);
> +		lt8713sx_wrdi(lt8713sx);
> +	}
> +
> +error:
> +	lt8713sx_i2c_disable(lt8713sx);
> +	if (!ret)
> +		lt8713sx_reset(lt8713sx);
> +
> +	kvfree(lt8713sx->fw_buffer);
> +	lt8713sx->fw_buffer = NULL;
> +
> +	if (lt8713sx->fw) {
> +		release_firmware(lt8713sx->fw);
> +		lt8713sx->fw = NULL;
> +	}
> +	mutex_unlock(&lt8713sx->ocm_lock);
> +
> +	return ret;
> +}
> +
> +static void lt8713sx_reset(struct lt8713sx *lt8713sx)
> +{
> +	dev_dbg(lt8713sx->dev, "reset bridge.\n");
> +	gpiod_set_value_cansleep(lt8713sx->reset_gpio, 1);
> +	msleep(20);
> +
> +	gpiod_set_value_cansleep(lt8713sx->reset_gpio, 0);
> +	msleep(20);
> +
> +	dev_dbg(lt8713sx->dev, "reset done.\n");
> +}
> +
> +static int lt8713sx_regulator_enable(struct lt8713sx *lt8713sx)
> +{
> +	int ret;
> +
> +	ret = devm_regulator_get_enable(lt8713sx->dev, "vdd");
> +	if (ret < 0)
> +		return dev_err_probe(lt8713sx->dev, ret, "failed to enable vdd regulator\n");
> +
> +	usleep_range(1000, 10000);
> +
> +	ret = devm_regulator_get_enable(lt8713sx->dev, "vcc");
> +	if (ret < 0)
> +		return dev_err_probe(lt8713sx->dev, ret, "failed to enable vcc regulator\n");
> +	return 0;
> +}
> +
> +static int lt8713sx_bridge_attach(struct drm_bridge *bridge,
> +				  struct drm_encoder *encoder,
> +				  enum drm_bridge_attach_flags flags)
> +{
> +	struct lt8713sx *lt8713sx = container_of(bridge, struct lt8713sx, bridge);
> +	int i, ret;
> +
> +	for (i = 0; i < lt8713sx->num_outputs; i++) {
> +		if (!lt8713sx->next_bridge[i])
> +			continue;
> +
> +		ret = drm_bridge_attach(encoder,
> +					lt8713sx->next_bridge[i],
> +					bridge, flags);

This wasn't really tested. This code will not result in what you intend
to do here. Each next bridge should have its own encoder / bridge chain.

> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int lt8713sx_get_ports(struct lt8713sx *lt8713sx)
> +{
> +	struct device *dev = lt8713sx->dev;
> +	struct device_node *port, *ports, *ep, *remote;
> +	int i = 0;
> +	u32 reg;
> +
> +	ports = of_get_child_by_name(dev->of_node, "ports");
> +	if (!ports)
> +		return -ENODEV;
> +
> +	for_each_child_of_node(ports, port) {
> +		if (of_property_read_u32(port, "reg", &reg))
> +			continue;
> +
> +		if (reg == 0)
> +			continue;
> +
> +		if (i >= ARRAY_SIZE(lt8713sx->next_bridge)) {
> +			of_node_put(port);
> +			break;
> +		}
> +
> +		ep = of_graph_get_next_endpoint(port, NULL);
> +		if (!ep)
> +			continue;
> +
> +		remote = of_graph_get_remote_port_parent(ep);
> +		of_node_put(ep);

And this misses the of_device_is_available() check. Please use
drm_of_find_panel_or_bridge().

> +
> +		if (!remote)
> +			continue;
> +
> +		lt8713sx->next_bridge[i] = of_drm_find_bridge(remote);
> +		of_node_put(remote);
> +		if (lt8713sx->next_bridge[i])
> +			i++;

And if not, the driver should be returning -EPROBE_DEFER.

> +	}
> +	lt8713sx->num_outputs = i;
> +	dev_dbg(dev, "Enabled %d output ports", i);
> +
> +	of_node_put(ports);
> +	return 0;
> +};
> +
> +static int lt8713sx_gpio_init(struct lt8713sx *lt8713sx)
> +{
> +	struct device *dev = lt8713sx->dev;
> +
> +	lt8713sx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(lt8713sx->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(lt8713sx->reset_gpio),
> +				     "failed to acquire reset gpio\n");
> +
> +	/* power enable gpio */
> +	lt8713sx->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(lt8713sx->enable_gpio))
> +		return dev_err_probe(dev, PTR_ERR(lt8713sx->enable_gpio),
> +				     "failed to acquire enable gpio\n");
> +	return 0;
> +}
> +
> +static ssize_t lt8713sx_firmware_store(struct device *dev,
> +				       struct device_attribute *attr,
> +				       const char *buf, size_t len)
> +{
> +	struct lt8713sx *lt8713sx = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = lt8713sx_firmware_update(lt8713sx);
> +	if (ret < 0)
> +		return ret;
> +	return len;
> +}
> +
> +static DEVICE_ATTR_WO(lt8713sx_firmware);
> +
> +static struct attribute *lt8713sx_attrs[] = {
> +	&dev_attr_lt8713sx_firmware.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group lt8713sx_attr_group = {
> +	.attrs = lt8713sx_attrs,
> +};
> +
> +static const struct attribute_group *lt8713sx_attr_groups[] = {
> +	&lt8713sx_attr_group,
> +	NULL,
> +};
> +
> +static const struct drm_bridge_funcs lt8713sx_bridge_funcs = {
> +	.attach = lt8713sx_bridge_attach,
> +};
> +
> +static int lt8713sx_probe(struct i2c_client *client)
> +{
> +	struct lt8713sx *lt8713sx;
> +	struct device *dev = &client->dev;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> +		return dev_err_probe(dev, -ENODEV, "device doesn't support I2C\n");
> +
> +	lt8713sx = devm_drm_bridge_alloc(dev, struct lt8713sx, bridge, &lt8713sx_bridge_funcs);
> +	if (IS_ERR(lt8713sx))
> +		return PTR_ERR(lt8713sx);
> +
> +	lt8713sx->dev = dev;
> +	lt8713sx->client = client;
> +	i2c_set_clientdata(client, lt8713sx);
> +
> +	ret = devm_mutex_init(lt8713sx->dev, &lt8713sx->ocm_lock);
> +	if (ret)
> +		return ret;
> +
> +	lt8713sx->regmap = devm_regmap_init_i2c(client, &lt8713sx_regmap_config);
> +	if (IS_ERR(lt8713sx->regmap))
> +		return dev_err_probe(dev, PTR_ERR(lt8713sx->regmap), "regmap i2c init failed\n");
> +
> +	ret = lt8713sx_get_ports(lt8713sx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = lt8713sx_gpio_init(lt8713sx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = lt8713sx_regulator_enable(lt8713sx);
> +	if (ret)
> +		return ret;
> +
> +	lt8713sx_reset(lt8713sx);
> +
> +	lt8713sx->bridge.funcs = &lt8713sx_bridge_funcs;
> +	lt8713sx->bridge.of_node = dev->of_node;
> +	lt8713sx->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
> +	drm_bridge_add(&lt8713sx->bridge);
> +
> +	crc8_populate_msb(lt8713sx_crc_table, 0x31);
> +
> +	return 0;
> +}
> +
> +static void lt8713sx_remove(struct i2c_client *client)
> +{
> +	struct lt8713sx *lt8713sx = i2c_get_clientdata(client);
> +
> +	drm_bridge_remove(&lt8713sx->bridge);
> +}
> +
> +static struct i2c_device_id lt8713sx_id[] = {
> +	{ "lontium,lt8713sx", 0 },
> +	{ /* sentinel */ }
> +};
> +
> +static const struct of_device_id lt8713sx_match_table[] = {
> +	{ .compatible = "lontium,lt8713sx" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, lt8713sx_match_table);
> +
> +static struct i2c_driver lt8713sx_driver = {
> +	.driver = {
> +		.name = "lt8713sx",
> +		.of_match_table = lt8713sx_match_table,
> +		.dev_groups = lt8713sx_attr_groups,
> +	},
> +	.probe = lt8713sx_probe,
> +	.remove = lt8713sx_remove,
> +	.id_table = lt8713sx_id,
> +};
> +
> +module_i2c_driver(lt8713sx_driver);
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("lt8713sx drm bridge driver");
> +MODULE_AUTHOR("Tony <syyang@lontium.com>");
> +MODULE_FIRMWARE(FW_FILE);
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3 2/2] drm/bridge: add support for lontium lt8713sx bridge driver

[Markus Elfring]

…
> +++ b/drivers/gpu/drm/bridge/lontium-lt8713sx.c
> @@ -0,0 +1,682 @@
…
> +static int lt8713sx_firmware_update(struct lt8713sx *lt8713sx)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&lt8713sx->ocm_lock);
> +	lt8713sx_i2c_enable(lt8713sx);
…
> +	mutex_unlock(&lt8713sx->ocm_lock);
> +
> +	return ret;
> +}
…

Under which circumstances would you become interested to apply a statement
like “guard(mutex)(&lt8713sx->ocm_lock);”?
https://elixir.bootlin.com/linux/v6.19-rc2/source/include/linux/mutex.h#L253

Regards,
Markus

Re: [PATCH v3 1/2] dt-bindings: bridge: lt8713sx: Add bindings

[Vishnu Saini]

On Sun, Dec 28, 2025 at 04:20:46PM +0200, Dmitry Baryshkov wrote:
> On Sun, Dec 28, 2025 at 05:10:39PM +0530, Vishnu Saini wrote:
> > Add bindings for lt8713sx.
> > 
> > Co-developed-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> > Signed-off-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> > Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> > ---
> >  .../bindings/display/bridge/lontium,lt8713sx.yaml  | 101 +++++++++++++++++++++
> >  1 file changed, 101 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt8713sx.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt8713sx.yaml
> > new file mode 100644
> > index 000000000000..0a6dc56e337c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt8713sx.yaml
> > @@ -0,0 +1,101 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bridge/lontium,lt8713sx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Lontium LT8713SX Type-C/DP1.4 to Type-C/DP1.4/HDMI2.0/DP++ bridge-hub
> > +
> > +maintainers:
> > +  - Tony <syyang@lontium.com>
> 
> Is he/she aware of this?
>
yes, Got the confirmation in email for upstreaming. I will add him in CC in next patch. 
> > +
> > +description:
> > +  The Lontium LT8713SX is a Type-C/DP1.4 to Type-C/DP1.4/HDMI2.0 converter
> > +  that integrates one DP input and up to three configurable output interfaces
> > +  (DP1.4 / HDMI2.0 / DP++), with SST/MST functionality and audio support.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - lontium,lt8713sx
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  vcc-supply:
> > +    description: Regulator for 3.3V vcc.
> > +
> > +  vdd-supply:
> > +    description: Regulator for 1.1V vdd.
> > +
> > +  reset-gpios:
> > +    description: GPIO connected to active low RESET pin.
> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    properties:
> > +      port@0:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          DP port for DP input from soc to bridge chip
> > +
> > +      port@1:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          DP port for DP output from bridge
> > +
> > +      port@2:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          Additional DP port for DP output from bridge
> 
> Why do you have only two output ports here? The datasheet documents
> three ports.
Currently using only 2 ports in rb4 main board, 3rd edp port is in mezz board.
Will add one more port in next patch.
> > +
> > +    required:
> > +      - port@0
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        bridge@4f {
> > +            compatible = "lontium,lt8713sx";
> > +            reg = <0x4f>;
> > +            reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
> > +
> > +            ports {
> > +                #address-cells = <1>;
> > +                #size-cells = <0>;
> > +
> > +                port@0 {
> > +                    reg = <0>;
> > +                    lt8713sx_dp_in: endpoint {
> > +                        remote-endpoint = <&mdss_dp0_out>;
> > +                    };
> > +                };
> > +
> > +                port@1 {
> > +                    reg = <1>;
> > +                    lt8713sx_dp0_out: endpoint {
> > +                        remote-endpoint = <&dp0_connector_in>;
> > +                    };
> > +                };
> > +
> > +                port@2 {
> > +                    reg = <2>;
> > +                    lt8713sx_dp1_out: endpoint {
> > +                        remote-endpoint = <&dp1_connector_in>;
> > +                    };
> > +                };
> > +            };
> > +        };
> > +    };
> > 
> > -- 
> > 2.34.1
> > 
Sorry maintainers for delay in update on lt8713sx driver and dt series.
I was on travel and than went on leave for a couple of weeks. Will be dedicately work on this now. 
> -- 
> With best wishes
> Dmitry

Re: [PATCH v3 2/2] drm/bridge: add support for lontium lt8713sx bridge driver

[Vishnu Saini]

On Sun, Dec 28, 2025 at 05:01:05PM +0200, Dmitry Baryshkov wrote:
> On Sun, Dec 28, 2025 at 05:10:40PM +0530, Vishnu Saini wrote:
> > The lt8713sx is a Type-C/DP1.4 to Type-C/DP1.4/HDMI2.0 converter,
> > with three configurable DP1.4/HDMI2.0/DP++ output interfaces and
> > audio output interface.
> > 
> > Driver is required for firmware upgrade in the bridge chip.
> 
> Or for enabling the bridge chip. Or for building the bridge chains
> inside Linux kernel...
> 
> Also, do we need any special handling for DP++ / HDMI devices being
> attached to the bridge?
Needed to add minimal handling of drm_bridge in the driver when ports
and remote endpoints added in the dt.
The V1 patch was also working where drm_bridge was not added in driver
and ports remote endpoints not defined in the dt.
 
> > 
> > Co-developed-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> > Signed-off-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> > Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> > ---
> >  drivers/gpu/drm/bridge/Kconfig            |  10 +
> >  drivers/gpu/drm/bridge/Makefile           |   1 +
> >  drivers/gpu/drm/bridge/lontium-lt8713sx.c | 682 ++++++++++++++++++++++++++++++
> >  3 files changed, 693 insertions(+)
> > 
> > +
> > +#define FW_FILE "lt8713sx_fw.bin"
> 
> I'm still looking forward to seeing this file in linux-firmware.
raised a patch in linux-firmware: https://lore.kernel.org/all/20260217-lt8713sx_firmware-v1-1-f78b85fceb63@oss.qualcomm.com/

> > +
> > +#define REG_PAGE_CONTROL	0xff
> > +
> > +#define MAX_OUTPUT_PORTS	3
> > +#define LT8713SX_PAGE_SIZE	256
> > +
> > +DECLARE_CRC8_TABLE(lt8713sx_crc_table);
> > +
> > +struct lt8713sx {
> > +	struct device *dev;
> > +	struct drm_bridge bridge;
> > +	struct drm_bridge *next_bridge[MAX_OUTPUT_PORTS];
> > +	int num_outputs;
> > +
> > +	struct regmap *regmap;
> > +	/* Protects all accesses to registers by stopping the on-chip MCU */
> > +	struct mutex ocm_lock;
> > +
> > +	struct gpio_desc *reset_gpio;
> > +	struct gpio_desc *enable_gpio;
> > +
> > +	struct i2c_client *client;
> > +	const struct firmware *fw;
> > +
> > +	u8 *fw_buffer;
> > +
> > +	u32 main_crc_value;
> > +	u32 bank_crc_value[17];
> > +
> > +	int bank_num;
> > +};
> > +
> > +static void lt8713sx_reset(struct lt8713sx *lt8713sx);
> > +
> > +static const struct regmap_range lt8713sx_ranges[] = {
> > +	{
> > +		.range_min = 0x0000,
> > +		.range_max = 0xffff
> > +	},
> > +};
> > +
> > +static const struct regmap_access_table lt8713sx_table = {
> > +	.yes_ranges = lt8713sx_ranges,
> > +	.n_yes_ranges = ARRAY_SIZE(lt8713sx_ranges),
> > +};
> > +
> > +static const struct regmap_range_cfg lt8713sx_range_cfg = {
> > +	.name = "lt8713sx",
> > +	.range_min = 0x0000,
> > +	.range_max = 0xffff,
> > +	.selector_reg = REG_PAGE_CONTROL,
> > +	.selector_mask = 0xff,
> > +	.selector_shift = 0,
> > +	.window_start = 0,
> > +	.window_len = 0x100,
> > +};
> > +
> > +static const struct regmap_config lt8713sx_regmap_config = {
> > +	.reg_bits = 8,
> > +	.val_bits = 8,
> > +	.volatile_table = &lt8713sx_table,
> > +	.ranges = &lt8713sx_range_cfg,
> > +	.num_ranges = 1,
> > +	.cache_type = REGCACHE_NONE,
> > +	.max_register = 0xffff,
> > +};
> > +
> > +static void lt8713sx_i2c_enable(struct lt8713sx *lt8713sx)
> > +{
> > +	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> > +}
> > +
> > +static void lt8713sx_i2c_disable(struct lt8713sx *lt8713sx)
> > +{
> > +	regmap_write(lt8713sx->regmap, 0xe0ee, 0x00);
> > +}
> > +
> > +static u32 calculate_crc(const u8 *upgrade_data, u64 len, u64 crc_size)
> > +{
> > +	u8 crc = 0x00;
> > +	u8 pad = 0xff;
> > +
> > +	crc = crc8(lt8713sx_crc_table, upgrade_data, len, crc);
> > +
> > +	/* pad remaining bytes */
> > +	crc_size -= len;
> > +	while (crc_size--)
> > +		crc = crc8(lt8713sx_crc_table, &pad, 1, crc);
> 
> As I wrote before, you are basically calculating the CRC over the padded
> buffer. Please pad the buffer and calculate it over the whole buffer
> rather than doing this byte by byte.
yes, will fix this in the next patch.
 
> > +
> > +	return crc;
> > +}
> > +
> > +static int lt8713sx_prepare_firmware_data(struct lt8713sx *lt8713sx)
> > +{
> > +	int ret = 0;
> > +	u64 sz_12k = 12 * SZ_1K;
> > +
> > +	ret = request_firmware(&lt8713sx->fw, FW_FILE, lt8713sx->dev);
> > +	if (ret < 0) {
> > +		dev_err(lt8713sx->dev, "request firmware failed\n");
> > +		return ret;
> > +	}
> > +
> > +	dev_dbg(lt8713sx->dev, "Firmware size: %zu bytes\n", lt8713sx->fw->size);
> > +
> > +	if (lt8713sx->fw->size > SZ_256K - 1) {
> > +		dev_err(lt8713sx->dev, "Firmware size exceeds 256KB limit\n");
> > +		release_firmware(lt8713sx->fw);
> > +		return -EINVAL;
> > +	}
> > +
> > +	lt8713sx->fw_buffer = kvmalloc(SZ_256K, GFP_KERNEL);
> > +	if (!lt8713sx->fw_buffer) {
> > +		release_firmware(lt8713sx->fw);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	memset(lt8713sx->fw_buffer, 0xff, SZ_256K);
> > +
> > +	if (lt8713sx->fw->size < SZ_64K) {
> > +		memcpy(lt8713sx->fw_buffer, lt8713sx->fw->data, lt8713sx->fw->size);
> > +		lt8713sx->fw_buffer[SZ_64K - 1] =
> > +				calculate_crc(lt8713sx->fw->data, lt8713sx->fw->size, SZ_64K - 1);
> > +		lt8713sx->main_crc_value = lt8713sx->fw_buffer[SZ_64K - 1];
> > +		dev_dbg(lt8713sx->dev,
> > +			"Main Firmware Data  Crc=0x%02X\n", lt8713sx->main_crc_value);
> > +
> > +	} else {
> > +		/* main firmware */
> > +		memcpy(lt8713sx->fw_buffer, lt8713sx->fw->data, SZ_64K - 1);
> > +		lt8713sx->fw_buffer[SZ_64K - 1] =
> > +				calculate_crc(lt8713sx->fw_buffer, SZ_64K - 1, SZ_64K - 1);
> > +		lt8713sx->main_crc_value = lt8713sx->fw_buffer[SZ_64K - 1];
> > +		dev_dbg(lt8713sx->dev,
> > +			"Main Firmware Data  Crc=0x%02X\n", lt8713sx->main_crc_value);
> > +
> > +		/* bank firmware */
> > +		memcpy(lt8713sx->fw_buffer + SZ_64K,
> > +		       lt8713sx->fw->data + SZ_64K,
> > +		       lt8713sx->fw->size - SZ_64K);
> > +
> > +		lt8713sx->bank_num = (lt8713sx->fw->size - SZ_64K + sz_12k - 1) / sz_12k;
> > +		dev_dbg(lt8713sx->dev, "Bank Number Total is %d.\n", lt8713sx->bank_num);
> > +
> > +		for (int i = 0; i < lt8713sx->bank_num; i++) {
> > +			lt8713sx->bank_crc_value[i] =
> > +				calculate_crc(lt8713sx->fw_buffer + SZ_64K + i * sz_12k,
> > +					      sz_12k, sz_12k);
> > +			dev_dbg(lt8713sx->dev, "Bank number:%d; Firmware Data  Crc:0x%02X\n",
> > +				i, lt8713sx->bank_crc_value[i]);
> > +		}
> > +	}
> > +	return 0;
> > +}
> > +
> > +static void lt8713sx_config_parameters(struct lt8713sx *lt8713sx)
> > +{
> > +	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> > +	regmap_write(lt8713sx->regmap, 0xe05e, 0xc1);
> > +	regmap_write(lt8713sx->regmap, 0xe058, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe059, 0x50);
> > +	regmap_write(lt8713sx->regmap, 0xe05a, 0x10);
> > +	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe058, 0x21);
> > +}
> > +
> > +static void lt8713sx_wren(struct lt8713sx *lt8713sx)
> > +{
> > +	regmap_write(lt8713sx->regmap, 0xe103, 0xbf);
> > +	regmap_write(lt8713sx->regmap, 0xe103, 0xff);
> > +	regmap_write(lt8713sx->regmap, 0xe05a, 0x04);
> > +	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> > +}
> > +
> > +static void lt8713sx_wrdi(struct lt8713sx *lt8713sx)
> > +{
> > +	regmap_write(lt8713sx->regmap, 0xe05a, 0x08);
> > +	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> > +}
> > +
> > +static void lt8713sx_fifo_reset(struct lt8713sx *lt8713sx)
> > +{
> > +	regmap_write(lt8713sx->regmap, 0xe103, 0xbf);
> > +	regmap_write(lt8713sx->regmap, 0xe103, 0xff);
> > +}
> > +
> > +static void lt8713sx_disable_sram_write(struct lt8713sx *lt8713sx)
> > +{
> > +	regmap_write(lt8713sx->regmap, 0xe055, 0x00);
> > +}
> > +
> > +static void lt8713sx_sram_to_flash(struct lt8713sx *lt8713sx)
> > +{
> > +	regmap_write(lt8713sx->regmap, 0xe05a, 0x30);
> > +	regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> > +}
> > +
> > +static void lt8713sx_i2c_to_sram(struct lt8713sx *lt8713sx)
> > +{
> > +	regmap_write(lt8713sx->regmap, 0xe055, 0x80);
> > +	regmap_write(lt8713sx->regmap, 0xe05e, 0xc0);
> > +	regmap_write(lt8713sx->regmap, 0xe058, 0x21);
> > +}
> > +
> > +static u8 lt8713sx_read_flash_status(struct lt8713sx *lt8713sx)
> > +{
> > +	u32 flash_status = 0;
> > +
> > +	regmap_write(lt8713sx->regmap,  0xe103, 0x3f);
> > +	regmap_write(lt8713sx->regmap,  0xe103, 0xff);
> > +
> > +	regmap_write(lt8713sx->regmap,  0xe05e, 0x40);
> > +	regmap_write(lt8713sx->regmap,  0xe056, 0x05); /* opcode=read status register */
> > +	regmap_write(lt8713sx->regmap,  0xe055, 0x25);
> > +	regmap_write(lt8713sx->regmap,  0xe055, 0x01);
> > +	regmap_write(lt8713sx->regmap,  0xe058, 0x21);
> > +
> > +	regmap_read(lt8713sx->regmap, 0xe05f, &flash_status);
> > +	dev_dbg(lt8713sx->dev, "flash_status:%x\n", flash_status);
> > +
> > +	return flash_status;
> > +}
> > +
> > +static void lt8713sx_block_erase(struct lt8713sx *lt8713sx)
> > +{
> > +	u32 i = 0;
> > +	u8 flash_status = 0;
> > +	u8 blocknum = 0x00;
> > +	u32 flashaddr = 0x00;
> > +
> > +	for (blocknum = 0; blocknum < 8; blocknum++) {
> > +		flashaddr = blocknum * SZ_32K;
> > +		regmap_write(lt8713sx->regmap,  0xe0ee, 0x01);
> 
> lt8713sx_i2c_enable() ? Why is it set again?
Right, it is set again in lt8713sx_config_parameters(), lt8713sx_block_erase()
and couple of more places, could be redundant.
Will try removing it.
 
> > +		regmap_write(lt8713sx->regmap,  0xe05a, 0x04);
> > +		regmap_write(lt8713sx->regmap,  0xe05a, 0x00);
> > +		regmap_write(lt8713sx->regmap,  0xe05b, flashaddr >> 16);
> > +		regmap_write(lt8713sx->regmap,  0xe05c, flashaddr >> 8);
> > +		regmap_write(lt8713sx->regmap,  0xe05d, flashaddr);
> > +		regmap_write(lt8713sx->regmap,  0xe05a, 0x01);
> > +		regmap_write(lt8713sx->regmap,  0xe05a, 0x00);
> > +		msleep(100);
> > +		i = 0;
> > +		while (1) {
> > +			flash_status = lt8713sx_read_flash_status(lt8713sx);
> > +			if ((flash_status & 0x01) == 0)
> > +				break;
> > +
> > +			if (i > 50)
> > +				break;
> > +
> > +			i++;
> > +			msleep(50);
> > +		}
> > +	}
> > +	dev_dbg(lt8713sx->dev, "erase flash done.\n");
> > +}
> > +
> > +static void lt8713sx_load_main_fw_to_sram(struct lt8713sx *lt8713sx)
> > +{
> > +	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> > +	regmap_write(lt8713sx->regmap, 0xe068, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe069, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe06a, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe065, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe066, 0xff);
> > +	regmap_write(lt8713sx->regmap, 0xe067, 0xff);
> > +	regmap_write(lt8713sx->regmap, 0xe06b, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe06c, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe060, 0x01);
> > +	msleep(200);
> > +	regmap_write(lt8713sx->regmap, 0xe060, 0x00);
> > +}
> > +
> > +static void lt8713sx_load_bank_fw_to_sram(struct lt8713sx *lt8713sx, u64 addr)
> > +{
> > +	regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> > +	regmap_write(lt8713sx->regmap, 0xe068, ((addr & 0xff0000) >> 16));
> > +	regmap_write(lt8713sx->regmap, 0xe069, ((addr & 0x00ff00) >> 8));
> > +	regmap_write(lt8713sx->regmap, 0xe06a, (addr & 0x0000ff));
> > +	regmap_write(lt8713sx->regmap, 0xe065, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe066, 0x30);
> > +	regmap_write(lt8713sx->regmap, 0xe067, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe06b, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe06c, 0x00);
> > +	regmap_write(lt8713sx->regmap, 0xe060, 0x01);
> > +	msleep(50);
> > +	regmap_write(lt8713sx->regmap, 0xe060, 0x00);
> > +}
> > +
> > +static int lt8713sx_write_data(struct lt8713sx *lt8713sx, const u8 *data, u64 filesize)
> > +{
> > +	int page = 0, num = 0, i = 0, val;
> > +
> > +	page = (filesize % LT8713SX_PAGE_SIZE) ?
> > +			((filesize / LT8713SX_PAGE_SIZE) + 1) : (filesize / LT8713SX_PAGE_SIZE);
> > +
> > +	dev_dbg(lt8713sx->dev,
> > +		"Writing to Sram=%u pages, total size = %llu bytes\n", page, filesize);
> > +
> > +	for (num = 0; num < page; num++) {
> > +		dev_dbg(lt8713sx->dev, "page[%d]\n", num);
> > +		lt8713sx_i2c_to_sram(lt8713sx);
> > +
> > +		for (i = 0; i < LT8713SX_PAGE_SIZE; i++) {
> > +			if ((num * LT8713SX_PAGE_SIZE + i) < filesize)
> > +				val = *(data + (num * LT8713SX_PAGE_SIZE + i));
> > +			else
> > +				val = 0xff;
> > +			regmap_write(lt8713sx->regmap, 0xe059, val);
> > +		}
> > +
> > +		lt8713sx_wren(lt8713sx);
> > +		lt8713sx_sram_to_flash(lt8713sx);
> > +	}
> > +
> > +	lt8713sx_wrdi(lt8713sx);
> > +	lt8713sx_disable_sram_write(lt8713sx);
> > +
> > +	return 0;
> > +}
> > +
> > +static void lt8713sx_main_upgrade_result(struct lt8713sx *lt8713sx)
> > +{
> > +	u32 main_crc_result;
> > +
> > +	regmap_read(lt8713sx->regmap, 0xe023, &main_crc_result);
> > +
> > +	dev_dbg(lt8713sx->dev, "Main CRC HW: 0x%02X\n", main_crc_result);
> > +	dev_dbg(lt8713sx->dev, "Main CRC FW: 0x%02X\n", lt8713sx->main_crc_value);
> > +
> > +	if (main_crc_result == lt8713sx->main_crc_value)
> > +		dev_dbg(lt8713sx->dev, "Main Firmware Upgrade Success.\n");
> > +	else
> > +		dev_err(lt8713sx->dev, "Main Firmware Upgrade Failed.\n");
> > +}
> > +
> > +static void lt8713sx_bank_upgrade_result(struct lt8713sx *lt8713sx, u8 banknum)
> > +{
> > +	u32 bank_crc_result;
> > +
> > +	regmap_read(lt8713sx->regmap, 0xe023, &bank_crc_result);
> > +
> > +	dev_dbg(lt8713sx->dev, "Bank %d CRC Result: 0x%02X\n", banknum, bank_crc_result);
> > +
> > +	if (bank_crc_result == lt8713sx->bank_crc_value[banknum])
> > +		dev_dbg(lt8713sx->dev, "Bank %d Firmware Upgrade Success.\n", banknum);
> > +	else
> > +		dev_err(lt8713sx->dev, "Bank %d Firmware Upgrade Failed.\n", banknum);
> > +}
> > +
> > +static void lt8713sx_bank_result_check(struct lt8713sx *lt8713sx)
> > +{
> > +	int i;
> > +	u64 addr = 0x010000;
> > +
> > +	for (i = 0; i < lt8713sx->bank_num; i++) {
> > +		lt8713sx_load_bank_fw_to_sram(lt8713sx, addr);
> > +		lt8713sx_bank_upgrade_result(lt8713sx, i);
> > +		addr += 0x3000;
> > +	}
> > +}
> > +
> > +static int lt8713sx_firmware_upgrade(struct lt8713sx *lt8713sx)
> > +{
> > +	int ret;
> > +
> > +	lt8713sx_config_parameters(lt8713sx);
> > +
> > +	lt8713sx_block_erase(lt8713sx);
> > +
> > +	if (lt8713sx->fw->size < SZ_64K) {
> > +		ret = lt8713sx_write_data(lt8713sx, lt8713sx->fw_buffer, SZ_64K);
> > +		if (ret < 0) {
> > +			dev_err(lt8713sx->dev, "Failed to write firmware data: %d\n", ret);
> > +			return ret;
> > +		}
> > +	} else {
> > +		ret = lt8713sx_write_data(lt8713sx, lt8713sx->fw_buffer, lt8713sx->fw->size);
> > +		if (ret < 0) {
> > +			dev_err(lt8713sx->dev, "Failed to write firmware data: %d\n", ret);
> > +			return ret;
> > +		}
> > +	}
> > +	dev_dbg(lt8713sx->dev, "Write Data done.\n");
> > +
> > +	return 0;
> > +}
> > +
> > +static int lt8713sx_firmware_update(struct lt8713sx *lt8713sx)
> > +{
> > +	int ret = 0;
> > +
> > +	mutex_lock(&lt8713sx->ocm_lock);
> > +	lt8713sx_i2c_enable(lt8713sx);
> > +
> > +	ret = lt8713sx_prepare_firmware_data(lt8713sx);
> > +	if (ret < 0) {
> > +		dev_err(lt8713sx->dev, "Failed to prepare firmware data: %d\n", ret);
> > +		goto error;
> > +	}
> > +
> > +	ret = lt8713sx_firmware_upgrade(lt8713sx);
> > +	if (ret < 0) {
> > +		dev_err(lt8713sx->dev, "Upgrade failure.\n");
> > +		goto error;
> > +	} else {
> > +		/* Validate CRC */
> > +		lt8713sx_load_main_fw_to_sram(lt8713sx);
> > +		lt8713sx_main_upgrade_result(lt8713sx);
> > +		lt8713sx_wrdi(lt8713sx);
> > +		lt8713sx_fifo_reset(lt8713sx);
> > +		lt8713sx_bank_result_check(lt8713sx);
> > +		lt8713sx_wrdi(lt8713sx);
> > +	}
> > +
> > +error:
> > +	lt8713sx_i2c_disable(lt8713sx);
> > +	if (!ret)
> > +		lt8713sx_reset(lt8713sx);
> > +
> > +	kvfree(lt8713sx->fw_buffer);
> > +	lt8713sx->fw_buffer = NULL;
> > +
> > +	if (lt8713sx->fw) {
> > +		release_firmware(lt8713sx->fw);
> > +		lt8713sx->fw = NULL;
> > +	}
> > +	mutex_unlock(&lt8713sx->ocm_lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static void lt8713sx_reset(struct lt8713sx *lt8713sx)
> > +{
> > +	dev_dbg(lt8713sx->dev, "reset bridge.\n");
> > +	gpiod_set_value_cansleep(lt8713sx->reset_gpio, 1);
> > +	msleep(20);
> > +
> > +	gpiod_set_value_cansleep(lt8713sx->reset_gpio, 0);
> > +	msleep(20);
> > +
> > +	dev_dbg(lt8713sx->dev, "reset done.\n");
> > +}
> > +
> > +static int lt8713sx_regulator_enable(struct lt8713sx *lt8713sx)
> > +{
> > +	int ret;
> > +
> > +	ret = devm_regulator_get_enable(lt8713sx->dev, "vdd");
> > +	if (ret < 0)
> > +		return dev_err_probe(lt8713sx->dev, ret, "failed to enable vdd regulator\n");
> > +
> > +	usleep_range(1000, 10000);
> > +
> > +	ret = devm_regulator_get_enable(lt8713sx->dev, "vcc");
> > +	if (ret < 0)
> > +		return dev_err_probe(lt8713sx->dev, ret, "failed to enable vcc regulator\n");
> > +	return 0;
> > +}
> > +
> > +static int lt8713sx_bridge_attach(struct drm_bridge *bridge,
> > +				  struct drm_encoder *encoder,
> > +				  enum drm_bridge_attach_flags flags)
> > +{
> > +	struct lt8713sx *lt8713sx = container_of(bridge, struct lt8713sx, bridge);
> > +	int i, ret;
> > +
> > +	for (i = 0; i < lt8713sx->num_outputs; i++) {
> > +		if (!lt8713sx->next_bridge[i])
> > +			continue;
> > +
> > +		ret = drm_bridge_attach(encoder,
> > +					lt8713sx->next_bridge[i],
> > +					bridge, flags);
> 
> This wasn't really tested. This code will not result in what you intend
> to do here. Each next bridge should have its own encoder / bridge chain.
This patch is working, might be because lt8713sx itself provides 3edp output from 1edp.
I am trying to attach all next bridges to the one previous in chain.
I will try to attach a single next_bridge in next patch.

> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int lt8713sx_get_ports(struct lt8713sx *lt8713sx)
> > +{
> > +	struct device *dev = lt8713sx->dev;
> > +	struct device_node *port, *ports, *ep, *remote;
> > +	int i = 0;
> > +	u32 reg;
> > +
> > +	ports = of_get_child_by_name(dev->of_node, "ports");
> > +	if (!ports)
> > +		return -ENODEV;
> > +
> > +	for_each_child_of_node(ports, port) {
> > +		if (of_property_read_u32(port, "reg", &reg))
> > +			continue;
> > +
> > +		if (reg == 0)
> > +			continue;
> > +
> > +		if (i >= ARRAY_SIZE(lt8713sx->next_bridge)) {
> > +			of_node_put(port);
> > +			break;
> > +		}
> > +
> > +		ep = of_graph_get_next_endpoint(port, NULL);
> > +		if (!ep)
> > +			continue;
> > +
> > +		remote = of_graph_get_remote_port_parent(ep);
> > +		of_node_put(ep);
> 
> And this misses the of_device_is_available() check. Please use
> drm_of_find_panel_or_bridge().
yes, Will fix in next patch.
 
> > +
> > +		if (!remote)
> > +			continue;
> > +
> > +		lt8713sx->next_bridge[i] = of_drm_find_bridge(remote);
> > +		of_node_put(remote);
> > +		if (lt8713sx->next_bridge[i])
> > +			i++;
> 
> And if not, the driver should be returning -EPROBE_DEFER.
Will fix in next patch.
 
> > +	}
> > +	lt8713sx->num_outputs = i;
> > +	dev_dbg(dev, "Enabled %d output ports", i);
> > +
> > +	of_node_put(ports);
> > +	return 0;
> > +};
> > +
> > +static int lt8713sx_gpio_init(struct lt8713sx *lt8713sx)
> > +{
> > +	struct device *dev = lt8713sx->dev;
> > +
> > +	lt8713sx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> > +	if (IS_ERR(lt8713sx->reset_gpio))
> > +		return dev_err_probe(dev, PTR_ERR(lt8713sx->reset_gpio),
> > +				     "failed to acquire reset gpio\n");
> > +
> > +	/* power enable gpio */
> > +	lt8713sx->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
> > +	if (IS_ERR(lt8713sx->enable_gpio))
> > +		return dev_err_probe(dev, PTR_ERR(lt8713sx->enable_gpio),
> > +				     "failed to acquire enable gpio\n");
> > +	return 0;
> > +}
> > +
> > +static ssize_t lt8713sx_firmware_store(struct device *dev,
> > +				       struct device_attribute *attr,
> > +				       const char *buf, size_t len)
> > +{
> > +	struct lt8713sx *lt8713sx = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = lt8713sx_firmware_update(lt8713sx);
> > +	if (ret < 0)
> > +		return ret;
> > +	return len;
> > +}
> > +
> > +static DEVICE_ATTR_WO(lt8713sx_firmware);
> > +
> > +static struct attribute *lt8713sx_attrs[] = {
> > +	&dev_attr_lt8713sx_firmware.attr,
> > +	NULL,
> > +};
> > +
> > +static const struct attribute_group lt8713sx_attr_group = {
> > +	.attrs = lt8713sx_attrs,
> > +};
> > +
> > +static const struct attribute_group *lt8713sx_attr_groups[] = {
> > +	&lt8713sx_attr_group,
> > +	NULL,
> > +};
> > +
> > +static const struct drm_bridge_funcs lt8713sx_bridge_funcs = {
> > +	.attach = lt8713sx_bridge_attach,
> > +};
> > +
> > +static int lt8713sx_probe(struct i2c_client *client)
> > +{
> > +	struct lt8713sx *lt8713sx;
> > +	struct device *dev = &client->dev;
> > +	int ret;
> > +
> > +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> > +		return dev_err_probe(dev, -ENODEV, "device doesn't support I2C\n");
> > +
> > +	lt8713sx = devm_drm_bridge_alloc(dev, struct lt8713sx, bridge, &lt8713sx_bridge_funcs);
> > +	if (IS_ERR(lt8713sx))
> > +		return PTR_ERR(lt8713sx);
> > +
> > +	lt8713sx->dev = dev;
> > +	lt8713sx->client = client;
> > +	i2c_set_clientdata(client, lt8713sx);
> > +
> > +	ret = devm_mutex_init(lt8713sx->dev, &lt8713sx->ocm_lock);
> > +	if (ret)
> > +		return ret;
> > +
> > +	lt8713sx->regmap = devm_regmap_init_i2c(client, &lt8713sx_regmap_config);
> > +	if (IS_ERR(lt8713sx->regmap))
> > +		return dev_err_probe(dev, PTR_ERR(lt8713sx->regmap), "regmap i2c init failed\n");
> > +
> > +	ret = lt8713sx_get_ports(lt8713sx);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = lt8713sx_gpio_init(lt8713sx);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = lt8713sx_regulator_enable(lt8713sx);
> > +	if (ret)
> > +		return ret;
> > +
> > +	lt8713sx_reset(lt8713sx);
> > +
> > +	lt8713sx->bridge.funcs = &lt8713sx_bridge_funcs;
> > +	lt8713sx->bridge.of_node = dev->of_node;
> > +	lt8713sx->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
> > +	drm_bridge_add(&lt8713sx->bridge);
> > +
> > +	crc8_populate_msb(lt8713sx_crc_table, 0x31);
> > +
> > +	return 0;
> > +}
> > +
> > +static void lt8713sx_remove(struct i2c_client *client)
> > +{
> > +	struct lt8713sx *lt8713sx = i2c_get_clientdata(client);
> > +
> > +	drm_bridge_remove(&lt8713sx->bridge);
> > +}
> > +
> > +static struct i2c_device_id lt8713sx_id[] = {
> > +	{ "lontium,lt8713sx", 0 },
> > +	{ /* sentinel */ }
> > +};
> > +
> > +static const struct of_device_id lt8713sx_match_table[] = {
> > +	{ .compatible = "lontium,lt8713sx" },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, lt8713sx_match_table);
> > +
> > +static struct i2c_driver lt8713sx_driver = {
> > +	.driver = {
> > +		.name = "lt8713sx",
> > +		.of_match_table = lt8713sx_match_table,
> > +		.dev_groups = lt8713sx_attr_groups,
> > +	},
> > +	.probe = lt8713sx_probe,
> > +	.remove = lt8713sx_remove,
> > +	.id_table = lt8713sx_id,
> > +};
> > +
> > +module_i2c_driver(lt8713sx_driver);
> > +MODULE_LICENSE("GPL");
> > +MODULE_DESCRIPTION("lt8713sx drm bridge driver");
> > +MODULE_AUTHOR("Tony <syyang@lontium.com>");
> > +MODULE_FIRMWARE(FW_FILE);
> > 
> > -- 
> > 2.34.1
> > 
Sorry maintainers for delay in response.
I was on travel and than went on leave for a couple of weeks. Will be dedicately work on this now.
 
> -- 
> With best wishes
> Dmitry

Re: [PATCH v3 2/2] drm/bridge: add support for lontium lt8713sx bridge driver

[Vishnu Saini]

On Mon, Dec 29, 2025 at 01:40:10PM +0100, Markus Elfring wrote:
> …
> > +++ b/drivers/gpu/drm/bridge/lontium-lt8713sx.c
> > @@ -0,0 +1,682 @@
> …
> > +static int lt8713sx_firmware_update(struct lt8713sx *lt8713sx)
> > +{
> > +	int ret = 0;
> > +
> > +	mutex_lock(&lt8713sx->ocm_lock);
> > +	lt8713sx_i2c_enable(lt8713sx);
> …
> > +	mutex_unlock(&lt8713sx->ocm_lock);
> > +
> > +	return ret;
> > +}
> …
> 
> Under which circumstances would you become interested to apply a statement
> like “guard(mutex)(&lt8713sx->ocm_lock);”?
> https://elixir.bootlin.com/linux/v6.19-rc2/source/include/linux/mutex.h#L253

This function holds ocm_lock across the entire firmware update sequence and has multiple error exits. It is therefore a good candidate for guard(mutex) to simplify error handling and guarantee unlock on all paths. I will convert it to scoped locking using guard(mutex).
 
> Regards,
> Markus