public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge
@ 2026-04-30  9:46 syyang
  2026-04-30  9:46 ` [PATCH v4 1/2] dt-bindings: bridge: " syyang
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: syyang @ 2026-04-30  9:46 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, andrzej.hajda, neil.armstrong,
	dmitry.baryshkov, maarten.lankhorst, rfoss, mripard
  Cc: Laurent.pinchart, tzimmermann, jonas, jernej.skrabec, devicetree,
	dri-devel, linux-kernel, yangsunyun1993, xmzhu, xmzhu, rlyu,
	xbpeng, Sunyun Yang

From: Sunyun Yang <syyang@lontium.com>

The LT7911EXC is an I2C-controlled bridge that Receiver eDP1.4
and output signal/dual port mipi. This series introduces:

- A device tree binding YAML file describing the hardware
- A new DRM bridge driver implementing the basic functionality

Signed-off-by: Sunyun Yang<syyang@lontium.com>
---
Change in v4:
- dt-binding:
 1. Fix the missing spaces on the "subject".             [Krzysztof]
 2. Fix the error descriptions for port@0 and port@1.
- drm/bridge:
- Link to v3: https://lore.kernel.org/lkml/20260429040541.3404116-1-syyang@lontium.com/

Change in v3:
- dt-binding:
- drm/bridge:
 1. already submit lt7911exc_fw.bin to linux-firmware.  [Dmitry]
 2. remove lt7911exc_remove function.
 3. drop  the "lontium, "  in lt7911exc_i2c_table.
- Link to v2: https://lore.kernel.org/lkml/20260428063224.3316655-1-syyang@lontium.com/

Change in v2:
- dt-binding:
 1. reset pins use active low.                        [Dmitry]
- drm/bridge:
 1. use atomic_* callbacks.                           [Quentin]
 2. fix the incorrect formatting and spaces.
 3. add the required header files.                    [Dmitry]
 4. remove "enabled" flag.
 5. remove *fw from the lt7911exc struct.
 6. .max_register and .range_max use actual range.
 7. regulator use bulk interface.
 8. use dev_err_probe, devm_mutex_init and devm_drm_bridge_add.
 9. Replace GPL v2 with GPL.
- Link to v1: https://lore.kernel.org/lkml/20260420023354.1192642-1-syyang@lontium.com/
---
Sunyun Yang (2):
  dt-bindings: bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
  drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge

 .../display/bridge/lontium,lt7911exc.yaml     |  89 ++++
 drivers/gpu/drm/bridge/Kconfig                |  13 +
 drivers/gpu/drm/bridge/Makefile               |   1 +
 drivers/gpu/drm/bridge/lontium-lt7911exc.c    | 493 ++++++++++++++++++
 4 files changed, 596 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt7911exc.yaml
 create mode 100644 drivers/gpu/drm/bridge/lontium-lt7911exc.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 1/2] dt-bindings: bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
  2026-04-30  9:46 [PATCH v4 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge syyang
@ 2026-04-30  9:46 ` syyang
  2026-05-03 12:07   ` Krzysztof Kozlowski
  2026-05-05  0:47   ` Claude review: " Claude Code Review Bot
  2026-04-30  9:46 ` [PATCH v4 2/2] drm/bridge: " syyang
  2026-05-05  0:47 ` Claude review: " Claude Code Review Bot
  2 siblings, 2 replies; 7+ messages in thread
From: syyang @ 2026-04-30  9:46 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, andrzej.hajda, neil.armstrong,
	dmitry.baryshkov, maarten.lankhorst, rfoss, mripard
  Cc: Laurent.pinchart, tzimmermann, jonas, jernej.skrabec, devicetree,
	dri-devel, linux-kernel, yangsunyun1993, xmzhu, xmzhu, rlyu,
	xbpeng, Sunyun Yang

From: Sunyun Yang <syyang@lontium.com>

The LT7911EXC is an I2C-controlled bridge that Receiver eDP1.4
and output signal/dual port mipi.

Signed-off-by: Sunyun Yang <syyang@lontium.com>
---
 .../display/bridge/lontium,lt7911exc.yaml     | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt7911exc.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt7911exc.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt7911exc.yaml
new file mode 100644
index 000000000000..3290b10ce883
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt7911exc.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/lontium,lt7911exc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lontium LT7911EXC eDP to MIPI DSI Bridge
+
+maintainers:
+  - Sunyun Yang <syyang@lontium.com>
+
+properties:
+  compatible:
+    enum:
+      - lontium,lt7911exc
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+    description: GPIO connected to RST_ pin.
+
+  vdd-supply:
+    description: Regulator for 1.2V MIPI phy power.
+
+  vcc-supply:
+    description: Regulator for 3.3V IO power.
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Video port for eDP input.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Video port for MIPI DSI output.
+
+    required:
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - reset-gpios
+  - vdd-supply
+  - vcc-supply
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        mipi-bridge@41 {
+            compatible = "lontium,lt7911exc";
+            reg = <0x41>;
+            reset-gpios = <&gpy8 8 GPIO_ACTIVE_LOW>;
+            vdd-supply = <&lt7911exc_1v2>;
+            vcc-supply = <&lt7911exc_3v3>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    bridge_in: endpoint {
+                        remote-endpoint = <&edp_out>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    bridge_out: endpoint {
+                        remote-endpoint = <&panel_in>;
+                    };
+                };
+            };
+        };
+    };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
  2026-04-30  9:46 [PATCH v4 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge syyang
  2026-04-30  9:46 ` [PATCH v4 1/2] dt-bindings: bridge: " syyang
@ 2026-04-30  9:46 ` syyang
  2026-05-05  0:47   ` Claude review: " Claude Code Review Bot
  2026-05-05  0:47 ` Claude review: " Claude Code Review Bot
  2 siblings, 1 reply; 7+ messages in thread
From: syyang @ 2026-04-30  9:46 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, andrzej.hajda, neil.armstrong,
	dmitry.baryshkov, maarten.lankhorst, rfoss, mripard
  Cc: Laurent.pinchart, tzimmermann, jonas, jernej.skrabec, devicetree,
	dri-devel, linux-kernel, yangsunyun1993, xmzhu, xmzhu, rlyu,
	xbpeng, Sunyun Yang

From: Sunyun Yang <syyang@lontium.com>

The LT7911EXC is an I2C-controlled bridge that Receiver eDP1.4
and output signal/dual port mipi.

Signed-off-by: Sunyun Yang <syyang@lontium.com>
---
 drivers/gpu/drm/bridge/Kconfig             |  13 +
 drivers/gpu/drm/bridge/Makefile            |   1 +
 drivers/gpu/drm/bridge/lontium-lt7911exc.c | 493 +++++++++++++++++++++
 3 files changed, 507 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/lontium-lt7911exc.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index c3209b0f4678..8cff2bf15b09 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -132,6 +132,19 @@ config DRM_ITE_IT6505
 	help
 	  ITE IT6505 DisplayPort bridge chip driver.
 
+config DRM_LONTIUM_LT7911EXC
+	tristate "Lontium eDP/MIPI bridge"
+	depends on OF
+	select CRC32
+	select FW_LOADER
+	select DRM_PANEL
+	select DRM_KMS_HELPER
+	help
+	  DRM driver for the Lontium LT7911EXC bridge chip.
+	  The LT7911EXC converts eDP input to single/dual port
+	  MIPI DSI output.
+	  Please say Y if you have such hardware.
+
 config DRM_LONTIUM_LT8912B
 	tristate "Lontium LT8912B DSI/HDMI bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index beab5b695a6e..70ddca75dd3a 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o
 obj-$(CONFIG_DRM_INNO_HDMI) += inno-hdmi.o
 obj-$(CONFIG_DRM_ITE_IT6263) += ite-it6263.o
 obj-$(CONFIG_DRM_ITE_IT6505) += ite-it6505.o
+obj-$(CONFIG_DRM_LONTIUM_LT7911EXC) += lontium-lt7911exc.o
 obj-$(CONFIG_DRM_LONTIUM_LT8912B) += lontium-lt8912b.o
 obj-$(CONFIG_DRM_LONTIUM_LT9211) += lontium-lt9211.o
 obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o
diff --git a/drivers/gpu/drm/bridge/lontium-lt7911exc.c b/drivers/gpu/drm/bridge/lontium-lt7911exc.c
new file mode 100644
index 000000000000..02648b396343
--- /dev/null
+++ b/drivers/gpu/drm/bridge/lontium-lt7911exc.c
@@ -0,0 +1,493 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2026 Lontium Semiconductor, Inc.
+ */
+
+#include <linux/crc32.h>
+#include <linux/firmware.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_bridge.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_of.h>
+
+#define FW_SIZE (64 * 1024)
+#define LT_PAGE_SIZE 32
+#define FW_FILE  "lt7911exc_fw.bin"
+#define LT7911EXC_PAGE_CONTROL 0xff
+
+struct lt7911exc {
+	struct device *dev;
+	struct i2c_client *client;
+	struct drm_bridge bridge;
+	struct regmap *regmap;
+	/* Protects all accesses to registers by stopping the on-chip MCU */
+	struct mutex ocm_lock;
+	struct regulator_bulk_data supplies[2];
+	struct gpio_desc *reset_gpio;
+	int fw_version;
+};
+
+static const struct regmap_range_cfg lt7911exc_ranges[] = {
+	{
+		.name = "register_range",
+		.range_min =  0,
+		.range_max = 0xe8ff,
+		.selector_reg = LT7911EXC_PAGE_CONTROL,
+		.selector_mask = 0xff,
+		.selector_shift = 0,
+		.window_start = 0,
+		.window_len = 0x100,
+	},
+};
+
+static const struct regmap_config lt7911exc_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0xe8ff,
+	.ranges = lt7911exc_ranges,
+	.num_ranges = ARRAY_SIZE(lt7911exc_ranges),
+};
+
+static u32 cal_crc32_custom(const u8 *data, u64 length)
+{
+	u32 crc = 0xffffffff;
+	u8 buf[4];
+	u64 i;
+
+	for (i = 0; i < length; i += 4) {
+		buf[0] = data[i + 3];
+		buf[1] = data[i + 2];
+		buf[2] = data[i + 1];
+		buf[3] = data[i + 0];
+		crc = crc32_be(crc, buf, 4);
+	}
+
+	return crc;
+}
+
+static inline struct lt7911exc *bridge_to_lt7911exc(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct lt7911exc, bridge);
+}
+
+static void lt7911exc_reset(struct lt7911exc *lt7911exc)
+{
+	gpiod_set_value_cansleep(lt7911exc->reset_gpio, 0);
+	msleep(20);
+
+	gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1);
+	msleep(20);
+
+	gpiod_set_value_cansleep(lt7911exc->reset_gpio, 0);
+	msleep(400);
+
+	dev_dbg(lt7911exc->dev, "lt7911exc reset.\n");
+}
+
+static int lt7911exc_parse_dt(struct lt7911exc *lt7911exc)
+{
+	struct device *dev = lt7911exc->dev;
+	int ret;
+
+	lt7911exc->supplies[0].supply = "vcc";
+	lt7911exc->supplies[1].supply = "vdd";
+
+	ret = devm_regulator_bulk_get(dev, 2, lt7911exc->supplies);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed get regulator\n");
+
+	lt7911exc->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(lt7911exc->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(lt7911exc->reset_gpio),
+				     "failed to acquire reset gpio\n");
+
+	return 0;
+}
+
+static int lt7911exc_read_version(struct lt7911exc *lt7911exc)
+{
+	u8 buf[3];
+	int ret;
+
+	ret = regmap_write(lt7911exc->regmap, 0xe0ee, 0x01);
+	if (ret)
+		return ret;
+	ret = regmap_bulk_read(lt7911exc->regmap, 0xe081, buf, sizeof(buf));
+	if (ret)
+		return ret;
+
+	return (buf[0] << 16) | (buf[1] << 8) | buf[2];
+}
+
+static void lt7911exc_lock(struct lt7911exc *lt7911exc)
+{
+	mutex_lock(&lt7911exc->ocm_lock);
+	regmap_write(lt7911exc->regmap, 0xe0ee, 0x01);
+}
+
+static void lt7911exc_unlock(struct lt7911exc *lt7911exc)
+{
+	regmap_write(lt7911exc->regmap, 0xe0ee, 0x00);
+	mutex_unlock(&lt7911exc->ocm_lock);
+}
+
+static void lt7911exc_block_erase(struct lt7911exc *lt7911exc)
+{
+	struct device *dev = lt7911exc->dev;
+	const u32 addr = 0x00;
+
+	const struct reg_sequence seq_write[] = {
+		REG_SEQ0(0xe0ee, 0x01),
+		REG_SEQ0(0xe054, 0x01),
+		REG_SEQ0(0xe055, 0x06),
+		REG_SEQ0(0xe051, 0x01),
+		REG_SEQ0(0xe051, 0x00),
+		REG_SEQ0(0xe054, 0x05),
+		REG_SEQ0(0xe055, 0xd8),
+		REG_SEQ0(0xe05a, (addr >> 16) & 0xff),
+		REG_SEQ0(0xe05b, (addr >> 8) & 0xff),
+		REG_SEQ0(0xe05c, addr & 0xff),
+		REG_SEQ0(0xe051, 0x01),
+		REG_SEQ0(0xe050, 0x00),
+	};
+
+	regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write));
+
+	msleep(200);
+	dev_dbg(dev, "erase flash done.\n");
+}
+
+static void lt7911exc_prog_init(struct lt7911exc *lt7911exc, u64 addr)
+{
+	const struct reg_sequence seq_write[] = {
+		REG_SEQ0(0xe0ee, 0x01),
+		REG_SEQ0(0xe05f, 0x01),
+		REG_SEQ0(0xe05a, (addr >> 16) & 0xff),
+		REG_SEQ0(0xe05b, (addr >> 8) & 0xff),
+		REG_SEQ0(0xe05c, addr & 0xff),
+	};
+
+	regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write));
+}
+
+static int lt7911exc_write_data(struct lt7911exc *lt7911exc, const struct firmware *fw, u64 addr)
+{
+	struct device *dev = lt7911exc->dev;
+	int ret;
+	int page = 0, num = 0, page_len = 0;
+	u64 size, offset;
+	const u8 *data;
+
+	data = fw->data;
+	size = fw->size;
+	page = (size + LT_PAGE_SIZE - 1) / LT_PAGE_SIZE;
+	if (page * LT_PAGE_SIZE > FW_SIZE) {
+		dev_err(dev, "firmware size out of range\n");
+		return -EINVAL;
+	}
+
+	dev_dbg(dev, "%u pages, total size %llu byte\n", page, size);
+
+	for (num = 0; num < page; num++) {
+		offset = num * LT_PAGE_SIZE;
+		page_len = (offset + LT_PAGE_SIZE <= size) ? LT_PAGE_SIZE : (size - offset);
+		lt7911exc_prog_init(lt7911exc, addr);
+
+		ret = regmap_raw_write(lt7911exc->regmap, 0xe05d, &data[offset], page_len);
+		if (ret) {
+			dev_err(dev, "write error at page %d\n", num);
+			return ret;
+		}
+
+		if (page_len < LT_PAGE_SIZE) {
+			regmap_write(lt7911exc->regmap, 0xe05f, 0x05);
+			regmap_write(lt7911exc->regmap, 0xe05f, 0x01);
+			//hardware requires delay
+			usleep_range(1000, 2000);
+		}
+
+		regmap_write(lt7911exc->regmap, 0xe05f, 0x00);
+		addr += LT_PAGE_SIZE;
+	}
+
+	return 0;
+}
+
+static int lt7911exc_write_crc(struct lt7911exc *lt7911exc, u32 crc32, u64 addr)
+{
+	u8 crc[4];
+	int ret;
+
+	crc[0] = crc32 & 0xff;
+	crc[1] = (crc32 >> 8) & 0xff;
+	crc[2] = (crc32 >> 16) & 0xff;
+	crc[3] = (crc32 >> 24) & 0xff;
+
+	regmap_write(lt7911exc->regmap, 0xe05f, 0x01);
+	regmap_write(lt7911exc->regmap, 0xe05a, (addr >> 16) & 0xff);
+	regmap_write(lt7911exc->regmap, 0xe05b, (addr >> 8) & 0xff);
+	regmap_write(lt7911exc->regmap, 0xe05c, addr & 0xff);
+
+	ret = regmap_raw_write(lt7911exc->regmap, 0xe05d, crc, 4);
+	if (ret)
+		return ret;
+
+	regmap_write(lt7911exc->regmap, 0xe05f, 0x05);
+	regmap_write(lt7911exc->regmap, 0xe05f, 0x01);
+	usleep_range(1000, 2000);
+	regmap_write(lt7911exc->regmap, 0xe05f, 0x00);
+
+	return 0;
+}
+
+static int lt7911exc_upgrade_result(struct lt7911exc *lt7911exc, u32 crc32)
+{
+	struct device *dev = lt7911exc->dev;
+	u32 read_hw_crc = 0;
+	u8 crc_tmp[4];
+	int ret;
+
+	regmap_write(lt7911exc->regmap, 0xe0ee, 0x01);
+	regmap_write(lt7911exc->regmap, 0xe07b, 0x60);
+	regmap_write(lt7911exc->regmap, 0xe07b, 0x40);
+	msleep(150);
+	ret = regmap_bulk_read(lt7911exc->regmap, 0x22, crc_tmp, 4);
+	if (ret) {
+		dev_err(lt7911exc->dev, "Failed to read CRC: %d\n", ret);
+		return ret;
+	}
+
+	read_hw_crc = crc_tmp[0] << 24 | crc_tmp[1] << 16 |
+				crc_tmp[2] << 8 | crc_tmp[3];
+
+	if (read_hw_crc != crc32) {
+		dev_err(dev, "lt7911exc firmware upgrade failed, expected CRC=0x%08x, read CRC=0x%08x\n",
+			crc32, read_hw_crc);
+		return -EIO;
+	}
+
+	dev_dbg(dev, "lt7911exc firmware upgrade success, CRC=0x%08x\n", read_hw_crc);
+	return 0;
+}
+
+static int lt7911exc_firmware_upgrade(struct lt7911exc *lt7911exc)
+{
+	struct device *dev = lt7911exc->dev;
+	const struct firmware *fw;
+	u8 *buffer;
+	size_t total_size = FW_SIZE - 4;
+	u32 crc32;
+	int ret;
+
+	/*1. load firmware*/
+	ret = request_firmware(&fw, FW_FILE, dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to load '%s'\n", FW_FILE);
+
+	/*2. check size*/
+	if (fw->size > total_size) {
+		dev_err(dev, "firmware too large (%zu > %zu)\n", fw->size, total_size);
+		ret = -EINVAL;
+		goto out_release_fw;
+	}
+
+	/*3. calculate crc32 */
+	buffer = kzalloc(total_size, GFP_KERNEL);
+	if (!buffer) {
+		ret = -ENOMEM;
+		goto out_release_fw;
+	}
+	memset(buffer, 0xff, total_size);
+	memcpy(buffer, fw->data, fw->size);
+
+	crc32 = cal_crc32_custom(buffer, total_size);
+	kfree(buffer);
+
+	/*4. firmware upgrade */
+	dev_dbg(dev, "starting firmware upgrade, size: %zu bytes\n", fw->size);
+
+	lt7911exc_block_erase(lt7911exc);
+
+	ret = lt7911exc_write_data(lt7911exc, fw, 0);
+	if (ret < 0) {
+		dev_err(dev, "failed to write firmware data\n");
+		goto out_release_fw;
+	}
+
+	ret = lt7911exc_write_crc(lt7911exc, crc32, FW_SIZE - 4);
+	if (ret < 0) {
+		dev_err(dev, "failed to write firmware crc\n");
+		goto out_release_fw;
+	}
+
+	/*5. check upgrade of result*/
+	lt7911exc_reset(lt7911exc);
+
+	ret = lt7911exc_upgrade_result(lt7911exc, crc32);
+
+out_release_fw:
+	release_firmware(fw);
+	return ret;
+}
+
+static void lt7911exc_atomic_pre_enable(struct drm_bridge *bridge, struct drm_atomic_state *state)
+{
+	struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(lt7911exc->supplies), lt7911exc->supplies);
+	if (ret)
+		return;
+
+	lt7911exc_reset(lt7911exc);
+}
+
+static void lt7911exc_atomic_disable(struct drm_bridge *bridge, struct drm_atomic_state *state)
+{
+	/* Delay after panel is disabled */
+	msleep(20);
+}
+
+static void lt7911exc_atomic_post_disable(struct drm_bridge *bridge, struct drm_atomic_state *state)
+{
+	struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge);
+	int ret;
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(lt7911exc->supplies), lt7911exc->supplies);
+	if (ret)
+		return;
+
+	gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1);
+}
+
+static int lt7911exc_attach(struct drm_bridge *bridge,
+			    struct drm_encoder *encoder,
+			    enum drm_bridge_attach_flags flags)
+{
+	struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge);
+
+	return drm_bridge_attach(lt7911exc->bridge.encoder, lt7911exc->bridge.next_bridge,
+				 &lt7911exc->bridge, flags);
+}
+
+static const struct drm_bridge_funcs lt7911exc_bridge_funcs = {
+	.attach = lt7911exc_attach,
+	.atomic_pre_enable = lt7911exc_atomic_pre_enable,
+	.atomic_disable = lt7911exc_atomic_disable,
+	.atomic_post_disable = lt7911exc_atomic_post_disable,
+};
+
+static int lt7911exc_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct lt7911exc *lt7911exc;
+	bool fw_updated = false;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		dev_err(dev, "device doesn't support I2C\n");
+		return -ENODEV;
+	}
+
+	lt7911exc = devm_drm_bridge_alloc(dev, struct lt7911exc, bridge,
+					  &lt7911exc_bridge_funcs);
+	if (IS_ERR(lt7911exc))
+		return dev_err_probe(dev, PTR_ERR(lt7911exc), "drm bridge alloc failed.\n");
+
+	lt7911exc->bridge.next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+	if (IS_ERR(lt7911exc->bridge.next_bridge))
+		return PTR_ERR(lt7911exc->bridge.next_bridge);
+
+	lt7911exc->client = client;
+	lt7911exc->dev = dev;
+	i2c_set_clientdata(client, lt7911exc);
+
+	ret = devm_mutex_init(dev, &lt7911exc->ocm_lock);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to init mutex\n");
+
+	lt7911exc->regmap = devm_regmap_init_i2c(client, &lt7911exc_regmap_config);
+	if (IS_ERR(lt7911exc->regmap))
+		return dev_err_probe(dev, PTR_ERR(lt7911exc->regmap), "regmap i2c init failed\n");
+
+	ret = lt7911exc_parse_dt(lt7911exc);
+	if (ret)
+		return ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(lt7911exc->supplies), lt7911exc->supplies);
+	if (ret)
+		return ret;
+
+	lt7911exc_reset(lt7911exc);
+	lt7911exc_lock(lt7911exc);
+
+retry:
+	lt7911exc->fw_version = lt7911exc_read_version(lt7911exc);
+	if (lt7911exc->fw_version < 0) {
+		dev_err(dev, "failed to read chip fw version\n");
+		ret = -EOPNOTSUPP;
+		goto err_disable_regulators;
+
+	} else if (lt7911exc->fw_version == 0) {
+		if (!fw_updated) {
+			fw_updated = true;
+			ret = lt7911exc_firmware_upgrade(lt7911exc);
+			if (ret < 0)
+				goto err_disable_regulators;
+
+			goto retry;
+
+		} else {
+			dev_err(dev, "fw version 0x%04x, update failed\n", lt7911exc->fw_version);
+			ret = -EOPNOTSUPP;
+			goto err_disable_regulators;
+		}
+	}
+
+	lt7911exc_unlock(lt7911exc);
+
+	lt7911exc->bridge.of_node = dev->of_node;
+	devm_drm_bridge_add(dev, &lt7911exc->bridge);
+
+	return 0;
+
+err_disable_regulators:
+	lt7911exc_unlock(lt7911exc);
+	regulator_bulk_disable(ARRAY_SIZE(lt7911exc->supplies), lt7911exc->supplies);
+	return ret;
+}
+
+static const struct i2c_device_id lt7911exc_i2c_table[] = {
+	{"lt7911exc"},
+	{/* sentinel */}
+};
+
+MODULE_DEVICE_TABLE(i2c, lt7911exc_i2c_table);
+
+static const struct of_device_id lt7911exc_devices[] = {
+	{.compatible = "lontium,lt7911exc"},
+	{/* sentinel */}
+};
+MODULE_DEVICE_TABLE(of, lt7911exc_devices);
+
+static struct i2c_driver lt7911exc_driver = {
+	.id_table	= lt7911exc_i2c_table,
+	.probe		= lt7911exc_probe,
+	.driver		= {
+		.name	= "lt7911exc",
+		.of_match_table = lt7911exc_devices,
+	},
+};
+module_i2c_driver(lt7911exc_driver);
+
+MODULE_AUTHOR("SunYun Yang <syyang@lontium.com>");
+MODULE_DESCRIPTION("Lontium LT7911EXC EDP to MIPI DSI bridge driver");
+MODULE_LICENSE("GPL");
+MODULE_FIRMWARE(FW_FILE);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] dt-bindings: bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
  2026-04-30  9:46 ` [PATCH v4 1/2] dt-bindings: bridge: " syyang
@ 2026-05-03 12:07   ` Krzysztof Kozlowski
  2026-05-05  0:47   ` Claude review: " Claude Code Review Bot
  1 sibling, 0 replies; 7+ messages in thread
From: Krzysztof Kozlowski @ 2026-05-03 12:07 UTC (permalink / raw)
  To: syyang
  Cc: robh, krzk+dt, conor+dt, andrzej.hajda, neil.armstrong,
	dmitry.baryshkov, maarten.lankhorst, rfoss, mripard,
	Laurent.pinchart, tzimmermann, jonas, jernej.skrabec, devicetree,
	dri-devel, linux-kernel, yangsunyun1993, xmzhu, xmzhu, rlyu,
	xbpeng

On Thu, Apr 30, 2026 at 05:46:11PM +0800, syyang@lontium.com wrote:
> From: Sunyun Yang <syyang@lontium.com>
> 
> The LT7911EXC is an I2C-controlled bridge that Receiver eDP1.4
> and output signal/dual port mipi.
> 
> Signed-off-by: Sunyun Yang <syyang@lontium.com>
> ---
>  .../display/bridge/lontium,lt7911exc.yaml     | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt7911exc.yaml

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

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Claude review: Add Lontium LT7911EXC eDP to MIPI DSI bridge
  2026-04-30  9:46 [PATCH v4 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge syyang
  2026-04-30  9:46 ` [PATCH v4 1/2] dt-bindings: bridge: " syyang
  2026-04-30  9:46 ` [PATCH v4 2/2] drm/bridge: " syyang
@ 2026-05-05  0:47 ` Claude Code Review Bot
  2 siblings, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-05-05  0:47 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: Add Lontium LT7911EXC eDP to MIPI DSI bridge
Author: syyang@lontium.com
Patches: 4
Reviewed: 2026-05-05T10:47:37.467462

---

This is a v4 patch series adding a new DRM bridge driver for the Lontium LT7911EXC eDP-to-MIPI-DSI bridge chip. The binding patch (1/2) is reasonable but the driver patch (2/2) has several significant issues that need to be addressed before it can be merged:

1. **Missing mandatory atomic state helpers** — the driver uses atomic callbacks but does not provide `atomic_duplicate_state`, `atomic_destroy_state`, or `atomic_reset`, which are mandatory per the DRM bridge API contract.
2. **No MIPI DSI host attachment** — this is a bridge that outputs MIPI DSI, yet the driver never registers with or attaches to a DSI host. Every other Lontium bridge with DSI output (lt9611, lt9611uxc, lt8912b) does this. Without it, the bridge cannot actually drive a DSI panel/pipeline.
3. **Firmware upgrade runs synchronously in probe** — this blocks boot and could take a long time.
4. **Regulators left enabled on success path** after probe, wasting power until the display pipeline starts.
5. **Incomplete error handling** throughout the firmware flashing code.

The driver feels like an early skeleton that implements firmware flashing but is missing the core bridge functionality (mode detection, mode validation, DSI configuration).

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Claude review: dt-bindings: bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
  2026-04-30  9:46 ` [PATCH v4 1/2] dt-bindings: bridge: " syyang
  2026-05-03 12:07   ` Krzysztof Kozlowski
@ 2026-05-05  0:47   ` Claude Code Review Bot
  1 sibling, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-05-05  0:47 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

This patch is in decent shape. Minor observations:

- **Missing `description` property at the top level**: The schema has `title` but no `description`. DT binding convention recommends a `description` block explaining the chip's capabilities (eDP 1.4 receiver, single/dual MIPI DSI output lanes, supported resolutions, etc.).

- **No `data-lanes` property for DSI output**: Since this chip supports "single/dual port MIPI" output, the binding should describe how the number of DSI lanes and ports is configured. Other MIPI DSI bridge bindings (e.g., `lontium,lt9611`) include `data-lanes` on the DSI port endpoints.

- **Commit message grammar**: "Receiver eDP1.4 and output signal/dual port mipi" — should be "receives eDP 1.4 input and outputs single/dual port MIPI DSI."

- **Example looks correct** — uses `GPIO_ACTIVE_LOW` for reset (matching the active-low convention), proper port definitions, correct I2C address.

Overall this patch is close but should add DSI lane configuration properties.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Claude review: drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
  2026-04-30  9:46 ` [PATCH v4 2/2] drm/bridge: " syyang
@ 2026-05-05  0:47   ` Claude Code Review Bot
  0 siblings, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-05-05  0:47 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

This patch has significant issues:

**Critical: Missing atomic state helpers**

The `lt7911exc_bridge_funcs` provides `atomic_pre_enable`, `atomic_disable`, and `atomic_post_disable` but does **not** provide the mandatory `atomic_duplicate_state`, `atomic_destroy_state`, and `atomic_reset` callbacks. Per `include/drm/drm_bridge.h`, these are "mandatory if the bridge implements any of the atomic hooks." Compare with the sibling driver `lontium-lt9611.c` which correctly provides all three using the standard helpers:

```c
static const struct drm_bridge_funcs lt7911exc_bridge_funcs = {
	.attach = lt7911exc_attach,
	.atomic_pre_enable = lt7911exc_atomic_pre_enable,
	.atomic_disable = lt7911exc_atomic_disable,
	.atomic_post_disable = lt7911exc_atomic_post_disable,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset = drm_atomic_helper_bridge_reset,
};
```

This will need `#include <drm/drm_atomic_helper.h>`.

**Critical: No MIPI DSI host registration**

The driver claims to output MIPI DSI but never calls `of_find_mipi_dsi_host_by_node()`, `devm_mipi_dsi_device_register_full()`, or `devm_mipi_dsi_attach()`. Without registering as a DSI device with the downstream DSI host, the MIPI output will not function. Every other Lontium bridge driver with DSI output implements this — see `lt9611uxc_attach_dsi()` in `lontium-lt9611uxc.c:248-279` for the pattern. This is fundamental missing functionality.

**Critical: No mode detection or mode validation**

The driver has no `mode_valid`, `mode_fixup`, `edid_read`, `detect`, or `atomic_get_input_bus_fmts` callbacks. This means the bridge cannot:
- Report what modes the connected eDP source provides
- Validate or constrain the display mode
- Report the input bus format

Without these, the display pipeline has no way to negotiate a valid mode through this bridge.

**Kconfig issues**

```c
select DRM_PANEL
select DRM_KMS_HELPER
```

`DRM_PANEL` is selected but never used anywhere in the driver — there is no panel reference or panel bridge. `DRM_KMS_HELPER` is questionable; the driver doesn't directly use KMS helpers. On the other hand, `depends on I2C` is missing (the driver is an I2C driver), and `select DRM_MIPI_DSI` should be added since the driver includes `<drm/drm_mipi_dsi.h>` (and should be using DSI APIs).

**Missing `#include <linux/delay.h>`**

The driver uses `msleep()` and `usleep_range()` but does not include `<linux/delay.h>`. It may compile by accident via transitive includes, but the include should be explicit.

**Regulators left enabled after probe success**

```c
ret = regulator_bulk_enable(ARRAY_SIZE(lt7911exc->supplies), lt7911exc->supplies);
// ... firmware check ...
lt7911exc_unlock(lt7911exc);
lt7911exc->bridge.of_node = dev->of_node;
devm_drm_bridge_add(dev, &lt7911exc->bridge);
return 0;  // regulators still enabled!
```

On the success path, regulators are enabled for the firmware version check but never disabled. They remain on until `atomic_post_disable` is eventually called. Regulators should be disabled after the probe-time version check and only re-enabled in `atomic_pre_enable`.

**Firmware upgrade runs synchronously in probe**

`lt7911exc_firmware_upgrade()` erases flash and writes 64KB of firmware with 32-byte pages, each requiring I2C transactions. This is slow and blocks the boot process. Consider deferring this to a workqueue or using `request_firmware_nowait()`.

**Error handling gaps in register write sequences**

Multiple functions ignore `regmap_write` / `regmap_multi_reg_write` return values:

- `lt7911exc_block_erase()` — ignores `regmap_multi_reg_write` return
- `lt7911exc_lock()` — ignores `regmap_write` return
- `lt7911exc_unlock()` — ignores `regmap_write` return  
- `lt7911exc_upgrade_result()` — ignores several `regmap_write` returns
- `lt7911exc_write_crc()` — ignores most `regmap_write` returns

If the I2C bus has errors, the firmware flash will silently corrupt. These should check and propagate errors.

**`kzalloc` + `memset` redundancy**

```c
buffer = kzalloc(total_size, GFP_KERNEL);
if (!buffer) { ... }
memset(buffer, 0xff, total_size);
```

`kzalloc` zeroes the buffer, then `memset` immediately overwrites with `0xff`. Use `kmalloc` instead to avoid the pointless zeroing.

**Firmware version format string mismatch**

```c
dev_err(dev, "fw version 0x%04x, update failed\n", lt7911exc->fw_version);
```

`fw_version` is constructed from 3 bytes (`(buf[0] << 16) | (buf[1] << 8) | buf[2]`), making it a 24-bit value. The format `0x%04x` only prints 4 hex digits (16 bits), truncating the high byte. Should be `0x%06x`.

**`u64` types used unnecessarily**

`lt7911exc_prog_init()` takes `u64 addr` and `lt7911exc_write_data()` uses `u64 size, offset`, but the maximum address is `FW_SIZE` (65536) and the maximum register is `0xe8ff`. These should all be `u32` to match the actual data widths and avoid implying 64-bit addressing.

**`cal_crc32_custom` is unsafe for non-multiple-of-4 lengths**

```c
for (i = 0; i < length; i += 4) {
    buf[0] = data[i + 3];  // reads data[length-1+3] on last iteration
```

If `length` is not a multiple of 4, this reads past the end of the buffer. The caller currently pads to `FW_SIZE - 4 = 65532` which is divisible by 4, so this works in practice, but the function should either validate or document the alignment requirement.

**Comment style violations**

```c
/*1. load firmware*/
/*2. check size*/
```

Kernel style requires spaces: `/* 1. load firmware */`. Also the `//hardware requires delay` should use C-style `/* ... */` comments per kernel convention (though `//` is technically allowed now, consistency matters).

**`lt7911exc_attach` uses `bridge->encoder` instead of parameter**

```c
static int lt7911exc_attach(struct drm_bridge *bridge,
                            struct drm_encoder *encoder,
                            enum drm_bridge_attach_flags flags)
{
    ...
    return drm_bridge_attach(lt7911exc->bridge.encoder, ...);
}
```

The `encoder` parameter should be used directly rather than reading `lt7911exc->bridge.encoder`. Compare with `lontium-lt9611uxc.c:287` which correctly uses the `encoder` parameter. While `bridge->encoder` may already be set by the framework, using the parameter is the correct pattern.

**`atomic_disable` has only a sleep**

```c
static void lt7911exc_atomic_disable(struct drm_bridge *bridge, struct drm_atomic_state *state)
{
    /* Delay after panel is disabled */
    msleep(20);
}
```

This does nothing except sleep. If the bridge has nothing to do at disable time, remove this callback. If the delay is truly needed for panel sequencing, this should be documented with a reference to the datasheet timing requirement, and it's questionable whether `atomic_disable` is the right place (vs. `atomic_post_disable`).

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-05-05  0:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-30  9:46 [PATCH v4 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge syyang
2026-04-30  9:46 ` [PATCH v4 1/2] dt-bindings: bridge: " syyang
2026-05-03 12:07   ` Krzysztof Kozlowski
2026-05-05  0:47   ` Claude review: " Claude Code Review Bot
2026-04-30  9:46 ` [PATCH v4 2/2] drm/bridge: " syyang
2026-05-05  0:47   ` Claude review: " Claude Code Review Bot
2026-05-05  0:47 ` Claude review: " Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox