* [PATCH v12 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge
@ 2026-05-25 1:05 syyang
2026-05-25 1:05 ` [PATCH v12 1/2] dt-bindings: bridge: " syyang
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: syyang @ 2026-05-25 1:05 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, xmzhu, xmzhu, rlyu, xbpeng, qdchen,
llzhang, Sunyun Yang
From: Sunyun Yang <syyang@lontium.com>
The LT7911EXC is an I2C-controlled bridge that receives eDP1.4
and output mipi dsi. 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 v12:
- dt-binding:
- drm/bridge:
1. Move the cleanup work of drm_panel_bridge_remove() forward into lt7911exc_dsi_host_detach(). [sashiko-bot]
2. Do not hold any lock during lt7911exc_reset().
- Link to v11: https://lore.kernel.org/lkml/20260522015735.2833-1-syyang@lontium.com/
Change in v11:
- dt-binding:
- drm/bridge:
1. Removed Reviewed-by, we would appreciate it if the maintainer could help review this.
2. Use devm_gpiod_get(,, GPIOD_OUT_HIGH) to fix the reset GPIO backpowering issue. [sashiko-bot]
3. Fixed Use-After-Free vulnerability due to dynamic DRM bridge removal in DSI detach callback.
4. Fixed Use-After-Free of regmap during DRM teardown because firmware upgrade worker defeats removal safeguard.
5. Fixed firmware upgrade performs a hardware reset, causing silent loss of DRM state and a blank screen.
6. Use of kmalloc for a 64KB software buffer is susceptible to memory fragmentation failures - Fix by using kvmalloc
7. Use the 'lt7911exc->ocm_lock' mutex and the 'lt7911exc->upgrade' flag to resolve the race condition between
DRM atomic modeset and firmware upgrade.
8. Use the 'lt7911exc->upgrade_lock' mutex to prevent redundant upgrades.
9. Use the 'lt7911exc->removed' flag to prevent the driver from being triggered to upgrade during the unload process.
- Link to v10: https://lore.kernel.org/lkml/20260519135816.26996-1-syyang@lontium.com/
Change in v10:
- dt-binding:
- drm/bridge:
1. Fixed the firmware upgrade error paths to always clear the upgrade
flag before returning, including firmware size validation failures
and allocation failures. [sashiko-bot]
2. Added proper locking in lt7911exc_atomic_pre_enable() and
lt7911exc_atomic_post_disable() to serialize register accesses with the
firmware upgrade flow and avoid concurrent I2C transactions.
3. Added an exclusivity check in lt7911exc_dsi_host_attach() to reject multiple
downstream attachments and prevent repeated drm_bridge_add() calls and panel bridge leaks.
4. Reworked lt7911exc_firmware_store() to use mutex_trylock() so concurrent sysfs writers
immediately return -EBUSY instead of blocking behind an active firmware upgrade.
5. Updated the remove path to prevent new firmware upgrade work from being queued after
device removal by setting the upgrade state before cancelling the worker.
- Link to v9: https://lore.kernel.org/lkml/20260519105019.22622-1-syyang@lontium.com/
Change in v9:
- dt-binding:
- drm/bridge:
1. DSI transfer callback returns success for reads without populating [sashiko-bot]
the receive buffer, leaking uninitialized memory. - fixed it by
implementing a strict whitelist mechanism.
2. DSI transfer callback polls for eDP video readiness before the
upstream encoder is enabled, guaranteeing a timeout. - removed
3. The driver attempts I2C transfers while the hardware is held in
physical reset. - fixed
4. Missing DRM_MIPI_DSI Kconfig dependency causes linker errors. - fixed
5. request_firmware is called while holding the hardware lock and
halting the MCU, risking a system pipeline stall. - fixed
6. Sleeping functions are called from atomic context in the DRM bridge callbacks. - fixed
7. lt7911exc_dsi_host_transfer bypasses the required MCU hardware halt sequence. - fixed by
internal firmware controls the panel initialization sequence and handles all MIPI
DSI command transmission.
- Link to v8: https://lore.kernel.org/lkml/20260515080934.9870-1-syyang@lontium.com/
Change in v8:
- dt-binding:
- drm/bridge:
1. Protect firmware upgrade and DRM bridge callback paths with ocm_lock. [sashiko-bot]
2. Remove the hardware reset from the remove callback, and ensure that
all hardware reset operations are protected by ocm_lock.
3. crc reconstruction explicitly casts each byte to u32 before shifting
4. The display configuration is handled by the firmware, and the MIPI
DSI host registration issue has been fixed.
5. The batch register read/write operations have already been updated
to include return value checking.
6. The dev_err_probe() used outside of probe context has been fixed.
- Link to v7: https://lore.kernel.org/lkml/20260512064013.40066-1-syyang@lontium.com/
Change in v7:
- dt-binding:
1. fix commit message typos(Receiver、signal) [sashiko-bot]
2. remove the ambiguity caused by "signal/dual".
- drm/bridge:
1. using devm_regulator_get_enable avoids power leaks. [sashiko-bot]
2. set reset gpio is low after cutting off power in lt7911exc_remove function, avoid backpowering.
3. synchronous request_firmware() call cause a permanent probe failure if the driver is built-in,
probe executes before the root filesystem is mounted, which would cause this to fail with -ENOENT,
we have removed this functionality. Use trigger to upgrade.
4. add `depends on I2C` and `select REGMAP_I2C` in Kconfig.
5. add return value of `devm_drm_bridge_add()` in `probe()`.
6. add directly header files (linux/slab.h, linux/delay.h, linux/regulator/consumer.h)
- Link to v6: https://lore.kernel.org/lkml/20260508134702.4713-1-syyang@lontium.com/
Change in v6:
- dt-binding:
- drm/bridge:
1. use #define FW_FILE "Lontium/lt7911exc_fw.bin" to match linux-firmware
- Link to v5: https://lore.kernel.org/lkml/20260506013153.2240-1-syyang@lontium.com/
Change in v5:
- dt-binding:
- drm/bridge:
1. Change "mipi" to "mipi dsi" in the commit message. [Dmitry]
2. Change "eDP/MIPI" to "eDP/MIPI DSI" in Kconfig.
- Link to v4: https://lore.kernel.org/lkml/20260430094612.3408174-1-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 | 16 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/lontium-lt7911exc.c | 700 ++++++++++++++++++
4 files changed, 806 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] 11+ messages in thread* [PATCH v12 1/2] dt-bindings: bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-25 1:05 [PATCH v12 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge syyang @ 2026-05-25 1:05 ` syyang 2026-05-25 6:53 ` Claude review: " Claude Code Review Bot 2026-05-25 1:05 ` [PATCH v12 2/2] drm/bridge: " syyang 2026-05-25 6:53 ` Claude review: " Claude Code Review Bot 2 siblings, 1 reply; 11+ messages in thread From: syyang @ 2026-05-25 1:05 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, xmzhu, xmzhu, rlyu, xbpeng, qdchen, llzhang, Sunyun Yang, Krzysztof Kozlowski From: Sunyun Yang <syyang@lontium.com> This commit adds the device tree binding schema for the Lontium LT7911EXC. This device is an I2C-controlled bridge that converts eDP 1.4 input to MIPI DSI output. Signed-off-by: Sunyun Yang <syyang@lontium.com> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.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 = <<7911exc_1v2>; + vcc-supply = <<7911exc_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] 11+ messages in thread
* Claude review: dt-bindings: bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-25 1:05 ` [PATCH v12 1/2] dt-bindings: bridge: " syyang @ 2026-05-25 6:53 ` Claude Code Review Bot 0 siblings, 0 replies; 11+ messages in thread From: Claude Code Review Bot @ 2026-05-25 6:53 UTC (permalink / raw) To: dri-devel-reviews Patch Review This patch is clean and already has Reviewed-by from the DT maintainer. No issues found. One very minor cosmetic note: the binding doesn't specify an `#address-cells`/`#size-cells` constraint on the node itself, but this is fine since the node is an I2C child and those are inherited. --- --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v12 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-25 1:05 [PATCH v12 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge syyang 2026-05-25 1:05 ` [PATCH v12 1/2] dt-bindings: bridge: " syyang @ 2026-05-25 1:05 ` syyang 2026-05-25 6:53 ` Claude review: " Claude Code Review Bot 2026-05-25 6:53 ` Claude review: " Claude Code Review Bot 2 siblings, 1 reply; 11+ messages in thread From: syyang @ 2026-05-25 1:05 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, xmzhu, xmzhu, rlyu, xbpeng, qdchen, llzhang, Sunyun Yang From: Sunyun Yang <syyang@lontium.com> Add support for the Lontium LT7911EXC bridge chip, which converts eDP input to MIPI DSI output using an internal firmware-controlled pipeline. The driver provides: - DRM bridge integration for eDP-to-DSI routing - MIPI DSI host interface for downstream panel attachment - Firmware upgrade mechanism over I2C (erase/program/verify) - GPIO-based reset and regulator management Display timing and MIPI DCS packet generation are handled by the chip firmware and are not configured by the driver. Signed-off-by: Sunyun Yang <syyang@lontium.com> --- drivers/gpu/drm/bridge/Kconfig | 16 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/lontium-lt7911exc.c | 700 +++++++++++++++++++++ 3 files changed, 717 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..013e431e8871 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -132,6 +132,22 @@ config DRM_ITE_IT6505 help ITE IT6505 DisplayPort bridge chip driver. +config DRM_LONTIUM_LT7911EXC + tristate "Lontium eDP/MIPI DSI bridge" + depends on OF + depends on I2C + select CRC32 + select DRM_PANEL + select DRM_MIPI_DSI + select DRM_KMS_HELPER + select FW_LOADER + select REGMAP_I2C + help + DRM driver for the Lontium LT7911EXC bridge + chip.The LT7911EXC converts eDP input to 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..de6953ed1e3b --- /dev/null +++ b/drivers/gpu/drm/bridge/lontium-lt7911exc.c @@ -0,0 +1,700 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2026 Lontium Semiconductor, Inc. + */ + +#include <linux/crc32.h> +#include <linux/delay.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 <linux/regulator/consumer.h> +#include <linux/slab.h> + +#include <drm/drm_atomic_helper.h> +#include <drm/drm_bridge.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_of.h> +#include <drm/drm_probe_helper.h> +#include <video/mipi_display.h> + +#define FW_SIZE (64 * 1024) +#define LT_PAGE_SIZE 32 +#define FW_FILE "Lontium/lt7911exc_fw.bin" +#define LT7911EXC_PAGE_CONTROL 0xff + +struct lt7911exc_dsi_output { + struct mipi_dsi_device *dev; + struct drm_panel *panel; + struct drm_bridge *bridge; +}; + +struct lt7911exc { + struct device *dev; + struct i2c_client *client; + struct drm_bridge bridge; + struct work_struct work; + struct mipi_dsi_host dsi_host; + struct lt7911exc_dsi_output output; + struct regmap *regmap; + /* Fast lock: guards short register r/w and status checks */ + struct mutex ocm_lock; + /* Long lock: serializes firmware upgrade process to prevent DRM interference */ + struct mutex upgrade_lock; + struct gpio_desc *reset_gpio; + int fw_version; + bool upgrade; + bool removed; +}; + +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; + + if (!length || (length & 3)) + return 0; + + 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 inline struct lt7911exc *dsi_host_to_lt7911exc(struct mipi_dsi_host *host) +{ + return container_of(host, struct lt7911exc, dsi_host); +} + +static void lt7911exc_reset(struct lt7911exc *lt7911exc) +{ + /* Assert reset pin: logical 1 -> physical state low (Reset Active) */ + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1); + msleep(20); + + /* Deassert reset pin: logical 0 -> physical state high (Run state) */ + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 0); + msleep(400); + + dev_dbg(lt7911exc->dev, "lt7911exc physical hardware reset sequence completed.\n"); +} + +static int lt7911exc_hw_mcu_halt(struct lt7911exc *lt7911exc) +{ + return regmap_write(lt7911exc->regmap, 0xe0ee, 0x01); +} + +static int lt7911exc_hw_mcu_run(struct lt7911exc *lt7911exc) +{ + return regmap_write(lt7911exc->regmap, 0xe0ee, 0x00); +} + +static int lt7911exc_regulator_enable(struct lt7911exc *lt7911exc) +{ + int ret; + + ret = devm_regulator_get_enable(lt7911exc->dev, "vcc"); + if (ret < 0) + return dev_err_probe(lt7911exc->dev, ret, "failed to enable vcc regulator\n"); + + usleep_range(5000, 10000); + + ret = devm_regulator_get_enable(lt7911exc->dev, "vdd"); + if (ret < 0) + return dev_err_probe(lt7911exc->dev, ret, "failed to enable vdd regulator\n"); + + return 0; +} + +static int lt7911exc_read_version(struct lt7911exc *lt7911exc) +{ + u8 buf[3]; + int ret; + + /* no need to halt MCU for this register access */ + ret = regmap_bulk_read(lt7911exc->regmap, 0xe081, buf, ARRAY_SIZE(buf)); + if (ret) + return ret; + + return (buf[0] << 16) | (buf[1] << 8) | buf[2]; +} + +static int lt7911exc_block_erase(struct lt7911exc *lt7911exc) +{ + struct device *dev = lt7911exc->dev; + const u32 addr = 0x00; + int ret; + + 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), + }; + + ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write)); + if (ret) + return ret; + + msleep(200); + dev_dbg(dev, "erase flash done.\n"); + + return 0; +} + +static int lt7911exc_prog_init(struct lt7911exc *lt7911exc, u64 addr) +{ + int ret; + + 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), + }; + + ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write)); + if (ret) + return ret; + + return 0; +} + +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); + ret = lt7911exc_prog_init(lt7911exc, addr); + if (ret) + return ret; + + 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, ARRAY_SIZE(crc_tmp)); + if (ret) { + dev_err(lt7911exc->dev, "Failed to read CRC: %d\n", ret); + return ret; + } + regmap_write(lt7911exc->regmap, 0xe0ee, 0x00); + + read_hw_crc = ((u32)crc_tmp[0] << 24) | ((u32)crc_tmp[1] << 16) | + ((u32)crc_tmp[2] << 8) | ((u32)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 void lt7911exc_firmware_upgrade_work(struct work_struct *work) +{ + struct lt7911exc *lt7911exc = container_of(work, struct lt7911exc, work); + struct device *dev = lt7911exc->dev; + const struct firmware *fw; + u8 *buffer; + size_t total_size = FW_SIZE - 4; + u32 crc32; + int ret; + + ret = request_firmware(&fw, FW_FILE, dev); + if (ret) { + dev_err(dev, "failed to load '%s'\n", FW_FILE); + goto out_clear_status; + } + + if (fw->size > total_size) { + dev_err(dev, "firmware too large (%zu > %zu)\n", fw->size, total_size); + goto out_release_fw; + } + + buffer = kvmalloc(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); + + kvfree(buffer); + + mutex_lock(<7911exc->upgrade_lock); + + lt7911exc_reset(lt7911exc); + mutex_lock(<7911exc->ocm_lock); + lt7911exc_hw_mcu_halt(lt7911exc); + mutex_unlock(<7911exc->ocm_lock); + + ret = lt7911exc_block_erase(lt7911exc); + if (ret) { + dev_err(dev, "failed to block erase.\n"); + goto out_unlock; + } + + ret = lt7911exc_write_data(lt7911exc, fw, 0); + if (ret < 0) { + dev_err(dev, "failed to write firmware data\n"); + goto out_unlock; + } + + ret = lt7911exc_write_crc(lt7911exc, crc32, FW_SIZE - 4); + if (ret < 0) { + dev_err(dev, "failed to write firmware crc\n"); + goto out_unlock; + } + + lt7911exc_reset(lt7911exc); + + ret = lt7911exc_upgrade_result(lt7911exc, crc32); + if (ret) + dev_err(dev, "firmware verification failed\n"); + +out_unlock: + mutex_lock(<7911exc->ocm_lock); + lt7911exc_hw_mcu_run(lt7911exc); + lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); + mutex_unlock(<7911exc->ocm_lock); + + mutex_unlock(<7911exc->upgrade_lock); + /* Notify DRM framework that hardware state changed/needs a modeset */ + if (lt7911exc->bridge.dev) + drm_kms_helper_hotplug_event(lt7911exc->bridge.dev); + +out_release_fw: + release_firmware(fw); + +out_clear_status: + mutex_lock(<7911exc->ocm_lock); + /* Only clear status if the module is not in the process of removal */ + if (!lt7911exc->removed) + lt7911exc->upgrade = false; + mutex_unlock(<7911exc->ocm_lock); +} + +static void lt7911exc_atomic_pre_enable(struct drm_bridge *bridge, struct drm_atomic_state *state) +{ + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge); + + guard(mutex)(<7911exc->ocm_lock); + + //enable mipi stream + if (!lt7911exc->upgrade) + regmap_write(lt7911exc->regmap, 0xe0b0, 0x01); +} + +static void lt7911exc_atomic_post_disable(struct drm_bridge *bridge, struct drm_atomic_state *state) +{ + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge); + + guard(mutex)(<7911exc->ocm_lock); + + //disable mipi stream + if (!lt7911exc->upgrade) + regmap_write(lt7911exc->regmap, 0xe0b0, 0x00); +} + +static int lt7911exc_bridge_attach(struct drm_bridge *bridge, + struct drm_encoder *encoder, + enum drm_bridge_attach_flags flags) +{ + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge); + + if (!lt7911exc->output.bridge) { + dev_warn(lt7911exc->dev, "Next bridge/panel not attached yet, deferring\n"); + return -EPROBE_DEFER; + } + + return drm_bridge_attach(encoder, lt7911exc->output.bridge, bridge, flags); +} + +static const struct drm_bridge_funcs lt7911exc_bridge_funcs = { + .attach = lt7911exc_bridge_attach, + .atomic_pre_enable = lt7911exc_atomic_pre_enable, + .atomic_post_disable = lt7911exc_atomic_post_disable, + .atomic_reset = drm_atomic_helper_bridge_reset, + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, +}; + +static int lt7911exc_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + struct drm_bridge *bridge; + struct drm_panel *panel; + int ret; + + if (lt7911exc->output.dev) + return -EBUSY; + + ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0, &panel, &bridge); + if (ret) + return ret; + + if (panel) { + bridge = drm_panel_bridge_add_typed(panel, DRM_MODE_CONNECTOR_DSI); + if (IS_ERR(bridge)) + return PTR_ERR(bridge); + } + lt7911exc->output.dev = dev; + lt7911exc->output.bridge = bridge; + lt7911exc->output.panel = panel; + + return 0; +} + +static int lt7911exc_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + + if (!lt7911exc->output.dev) + return 0; + + if (lt7911exc->output.panel && lt7911exc->output.bridge) { + drm_panel_bridge_remove(lt7911exc->output.bridge); + lt7911exc->output.bridge = NULL; + lt7911exc->output.panel = NULL; + } + + lt7911exc->output.dev = NULL; + + return 0; +} + +/* + * The internal firmware controls the panel initialization + * sequence and handles all MIPI DSI command transmission. + */ +static ssize_t lt7911exc_dsi_host_transfer(struct mipi_dsi_host *host, + const struct mipi_dsi_msg *msg) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + + if (msg->rx_len) { + dev_warn(lt7911exc->dev, "MIPI DSI read is not supported\n"); + return -EOPNOTSUPP; + } + + switch (msg->type) { + case MIPI_DSI_DCS_SHORT_WRITE: + case MIPI_DSI_DCS_SHORT_WRITE_PARAM: + case MIPI_DSI_DCS_LONG_WRITE: + case MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM: + case MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM: + case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM: + case MIPI_DSI_GENERIC_LONG_WRITE: + break; + default: + return -EOPNOTSUPP; + } + + guard(mutex)(<7911exc->ocm_lock); + + if (lt7911exc->upgrade) + return -EBUSY; + + return msg->tx_len; +} + +static const struct mipi_dsi_host_ops lt7911exc_dsi_host_ops = { + .attach = lt7911exc_dsi_host_attach, + .detach = lt7911exc_dsi_host_detach, + .transfer = lt7911exc_dsi_host_transfer, +}; + +static ssize_t lt7911exc_firmware_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t len) +{ + struct lt7911exc *lt7911exc = dev_get_drvdata(dev); + + if (!mutex_trylock(<7911exc->upgrade_lock)) + return -EBUSY; + + mutex_lock(<7911exc->ocm_lock); + + if (lt7911exc->upgrade || lt7911exc->removed) { + mutex_unlock(<7911exc->ocm_lock); + mutex_unlock(<7911exc->upgrade_lock); + return -EBUSY; + } + + lt7911exc->upgrade = true; + mutex_unlock(<7911exc->ocm_lock); + mutex_unlock(<7911exc->upgrade_lock); + + schedule_work(<7911exc->work); + + return len; +} + +static ssize_t lt7911exc_firmware_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct lt7911exc *lt7911exc = dev_get_drvdata(dev); + int version; + + mutex_lock(<7911exc->ocm_lock); + version = lt7911exc->fw_version; + mutex_unlock(<7911exc->ocm_lock); + return sysfs_emit(buf, "0x%04x\n", version); +} + +static DEVICE_ATTR_RW(lt7911exc_firmware); + +static struct attribute *lt7911exc_attrs[] = { + &dev_attr_lt7911exc_firmware.attr, + NULL, +}; + +static const struct attribute_group lt7911exc_attr_group = { + .attrs = lt7911exc_attrs, +}; + +static const struct attribute_group *lt7911exc_attr_groups[] = { + <7911exc_attr_group, + NULL, +}; + +static int lt7911exc_probe(struct i2c_client *client) +{ + struct lt7911exc *lt7911exc; + struct device *dev = &client->dev; + struct device_node *np = dev->of_node; + int ret; + + if (!np) + return -ENODEV; + + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) + return dev_err_probe(dev, -ENODEV, "device doesn't support I2C\n"); + + lt7911exc = devm_drm_bridge_alloc(dev, struct lt7911exc, bridge, <7911exc_bridge_funcs); + if (IS_ERR(lt7911exc)) + return dev_err_probe(dev, PTR_ERR(lt7911exc), "drm bridge alloc failed.\n"); + + dev_set_drvdata(dev, lt7911exc); + + lt7911exc->client = client; + lt7911exc->dev = dev; + lt7911exc->upgrade = false; + lt7911exc->removed = false; + + ret = devm_mutex_init(dev, <7911exc->ocm_lock); + if (ret) + return dev_err_probe(dev, ret, "failed to init mutex\n"); + + ret = devm_mutex_init(dev, <7911exc->upgrade_lock); + if (ret) + return dev_err_probe(dev, ret, "failed to init upgrade_lock\n"); + + lt7911exc->regmap = devm_regmap_init_i2c(client, <7911exc_regmap_config); + if (IS_ERR(lt7911exc->regmap)) + return dev_err_probe(dev, PTR_ERR(lt7911exc->regmap), "regmap i2c init failed\n"); + + lt7911exc->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(lt7911exc->reset_gpio)) + return dev_err_probe(dev, PTR_ERR(lt7911exc->reset_gpio), + "failed to acquire reset gpio\n"); + + ret = lt7911exc_regulator_enable(lt7911exc); + if (ret) + return ret; + + lt7911exc_reset(lt7911exc); + + mutex_lock(<7911exc->ocm_lock); + + lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); + + mutex_unlock(<7911exc->ocm_lock); + + if (lt7911exc->fw_version < 0) + return dev_err_probe(dev, lt7911exc->fw_version, "failed read version of chip\n"); + + lt7911exc->dsi_host.dev = dev; + lt7911exc->dsi_host.ops = <7911exc_dsi_host_ops; + lt7911exc->bridge.of_node = np; + + INIT_WORK(<7911exc->work, lt7911exc_firmware_upgrade_work); + + i2c_set_clientdata(client, lt7911exc); + + drm_bridge_add(<7911exc->bridge); + + ret = mipi_dsi_host_register(<7911exc->dsi_host); + if (ret) { + drm_bridge_remove(<7911exc->bridge); + return ret; + } + + return 0; +} + +static void lt7911exc_remove(struct i2c_client *client) +{ + struct lt7911exc *lt7911exc = i2c_get_clientdata(client); + + mutex_lock(<7911exc->ocm_lock); + lt7911exc->removed = true; + lt7911exc->upgrade = true; + mutex_unlock(<7911exc->ocm_lock); + + cancel_work_sync(<7911exc->work); + + mipi_dsi_host_unregister(<7911exc->dsi_host); + drm_bridge_remove(<7911exc->bridge); + + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1); +} + +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, + .remove = lt7911exc_remove, + .driver = { + .name = "lt7911exc", + .of_match_table = lt7911exc_devices, + .dev_groups = lt7911exc_attr_groups, + }, +}; +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] 11+ messages in thread
* Claude review: drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-25 1:05 ` [PATCH v12 2/2] drm/bridge: " syyang @ 2026-05-25 6:53 ` Claude Code Review Bot 0 siblings, 0 replies; 11+ messages in thread From: Claude Code Review Bot @ 2026-05-25 6:53 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Critical / Functional Issues:** 1. **`lt7911exc_write_data` does not advance the address for `prog_init`** — the `addr` parameter passed into `lt7911exc_prog_init` is always `0` (the initial value from the caller). Only the local `addr` at the bottom of the loop is incremented. But `lt7911exc_prog_init` receives the function parameter `addr`, not a local tracking variable: ```c static int lt7911exc_write_data(struct lt7911exc *lt7911exc, const struct firmware *fw, u64 addr) { ... for (num = 0; num < page; num++) { offset = num * LT_PAGE_SIZE; ... ret = lt7911exc_prog_init(lt7911exc, addr); ... addr += LT_PAGE_SIZE; } ``` Wait — re-reading more carefully, `addr` *is* the function parameter and it is modified in the loop (`addr += LT_PAGE_SIZE`). So this is actually correct since C passes by value and `addr` is used as a local accumulator. My mistake — this is fine. 2. **Goto jumps skip `out_unlock` label on early exits** — In `lt7911exc_firmware_upgrade_work`, the `goto out_release_fw` path (firmware too large) and `goto out_clear_status` (request_firmware failure) both skip the `out_unlock` label. This is correct since `upgrade_lock` is only acquired after those checks. However, the `buffer` allocation failure path (`goto out_release_fw`) also skips `out_clear_status` — actually no, the labels are ordered so `out_release_fw` falls through to `out_clear_status`. Let me re-examine: ```c out_unlock: ... mutex_unlock(<7911exc->upgrade_lock); if (lt7911exc->bridge.dev) drm_kms_helper_hotplug_event(lt7911exc->bridge.dev); out_release_fw: release_firmware(fw); out_clear_status: mutex_lock(<7911exc->ocm_lock); if (!lt7911exc->removed) lt7911exc->upgrade = false; mutex_unlock(<7911exc->ocm_lock); ``` This is correct — the labels cascade. Good. 3. **`kvfree(buffer)` is called before the firmware data is fully consumed** — The buffer is freed immediately after computing the CRC, but then `lt7911exc_write_data(lt7911exc, fw, 0)` uses `fw->data` directly, not the padded buffer. This means the CRC is computed over the padded data (firmware + 0xff padding to `FW_SIZE - 4`), but only the raw firmware bytes are written to flash. The hardware CRC verification in `lt7911exc_upgrade_result` reads back what was written. **If the flash does not already contain 0xff in the padded region, the CRC will mismatch.** This is probably fine because `lt7911exc_block_erase` erases the entire block first (erased flash is typically 0xff), but the logic is fragile and implicit. Worth a comment at minimum. **Medium Issues:** 4. **Unchecked `regmap_write` return values in multiple places** — Several functions call `regmap_write` without checking the return value: - `lt7911exc_write_crc`: First four `regmap_write` calls and last three are unchecked. - `lt7911exc_upgrade_result`: `regmap_write` calls to 0xe0ee, 0xe07b are unchecked. - `lt7911exc_write_data`: The `regmap_write` calls for the short-page handling and the final `0xe05f` write are unchecked. While regmap-over-I2C errors are rare, silently continuing after a failed register write during firmware programming could corrupt the flash. 5. **`lt7911exc_write_data` uses `u64` for sizes/addresses but firmware is max 64KB** — The `addr` parameter and `size`/`offset` locals are all `u64`, but `FW_SIZE` is `64 * 1024`. Using `u32` or even `unsigned int` would be more appropriate and avoids confusion. Similarly, `lt7911exc_prog_init` takes `u64 addr`. The register writes only use the low 24 bits anyway (`(addr >> 16) & 0xff` etc.), so a `u64` is misleading. 6. **`lt7911exc_read_version` returns a negative error code or a positive version packed into `int`** — The version is `(buf[0] << 16) | (buf[1] << 8) | buf[2]`, which could set bit 23 if `buf[0] >= 0x80`, producing a negative value indistinguishable from an error code. This would cause a spurious probe failure. Consider using `unsigned int` for the version, or masking to ensure positivity. 7. **`dev_set_drvdata` and `i2c_set_clientdata` are both called in probe** — For an I2C driver, `dev_set_drvdata(&client->dev, ...)` and `i2c_set_clientdata(client, ...)` are the same thing. One of them is redundant. The remove path uses `i2c_get_clientdata`, so keep that one and drop `dev_set_drvdata`. (Actually, the sysfs show/store callbacks use `dev_get_drvdata` on the I2C client's dev, so they are equivalent — but having both is confusing.) 8. **Kconfig help text formatting** — Missing space after period: ``` chip.The LT7911EXC converts eDP input to MIPI ``` Should be `chip. The LT7911EXC...`. **Minor / Style Issues:** 9. **Comment style** — A couple of `//` (C++) style comments remain: ```c //enable mipi stream //disable mipi stream //hardware requires delay ``` The kernel coding style requires `/* ... */` comments. These should be converted. 10. **`lt7911exc_prog_init` has a redundant return pattern:** ```c ret = regmap_multi_reg_write(...); if (ret) return ret; return 0; ``` This can be simply `return regmap_multi_reg_write(...)`. 11. **`struct device_node *np` usage in probe** — `np` is only used for the NULL check and assigning `bridge.of_node`. Since this is an OF-dependent driver (`depends on OF`), the `if (!np) return -ENODEV` check is redundant. And `bridge.of_node = np` could be `bridge.of_node = dev->of_node` directly, eliminating the variable. 12. **The `lt7911exc_dsi_host_transfer` accepted message types have no indentation in the switch body:** ```c case MIPI_DSI_DCS_SHORT_WRITE: case MIPI_DSI_DCS_SHORT_WRITE_PARAM: ... break; default: return -EOPNOTSUPP; ``` The `break` and the `default` body should be indented one level under their case labels per kernel coding style. 13. **The `cal_crc32_custom` function name** — "cal" is an unusual abbreviation. Consider `lt7911exc_calc_crc32` to be consistent with driver namespace and clearer naming. 14. **`platform_device.h` is included but not used** — This is an I2C driver, not a platform driver. The include should be removed. **Summary:** The driver is close but needs attention to unchecked regmap writes in the firmware programming path, the `u64`/`u32` type confusion, the version-sign issue, and the coding style issues (C++ comments, switch indentation, Kconfig typo). The overall locking design is workable but complex for a v12 submission — it would benefit from clearer documentation of which lock protects what invariant. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 11+ messages in thread
* Claude review: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-25 1:05 [PATCH v12 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge syyang 2026-05-25 1:05 ` [PATCH v12 1/2] dt-bindings: bridge: " syyang 2026-05-25 1:05 ` [PATCH v12 2/2] drm/bridge: " syyang @ 2026-05-25 6:53 ` Claude Code Review Bot 2 siblings, 0 replies; 11+ messages in thread From: Claude Code Review Bot @ 2026-05-25 6:53 UTC (permalink / raw) To: dri-devel-reviews Overall Series Review Subject: Add Lontium LT7911EXC eDP to MIPI DSI bridge Author: syyang@lontium.com Patches: 3 Reviewed: 2026-05-25T16:53:53.999955 --- This is v12 of a 2-patch series adding support for the Lontium LT7911EXC eDP-to-MIPI-DSI bridge chip. The DT binding (patch 1) has already received a Reviewed-by from Krzysztof Kozlowski and looks clean. The driver (patch 2) has gone through significant iteration — the changelog shows many fixes from sashiko-bot across v7–v12. The overall structure follows the existing lt9611uxc driver pattern reasonably well (sysfs firmware upgrade, regmap, DSI host). However, there are several issues remaining, ranging from a race condition in the firmware upgrade flow to error-path resource leaks, unchecked return values, and minor style problems. The locking scheme (two mutexes, two flags) is complex for what it guards and has at least one hole — the firmware work function can jump to `out_release_fw` or `out_clear_status` while skipping `out_unlock`, but the `upgrade_lock` is only taken inside the function body. More fundamentally, `addr` is never incremented inside `lt7911exc_write_data` on each page's `lt7911exc_prog_init` call — the same start address is re-initialized every iteration, which would make the firmware write silently overwrite the same flash page repeatedly. This looks like a critical functional bug. --- --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v11 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge
@ 2026-05-22 1:57 syyang
2026-05-22 1:57 ` [PATCH v11 2/2] drm/bridge: " syyang
0 siblings, 1 reply; 11+ messages in thread
From: syyang @ 2026-05-22 1:57 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, xmzhu, xmzhu, rlyu, xbpeng, qdchen,
llzhang, Sunyun Yang
From: Sunyun Yang <syyang@lontium.com>
The LT7911EXC is an I2C-controlled bridge that receives eDP1.4
and output mipi dsi. 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 v11:
- dt-binding:
- drm/bridge:
1. Removed Reviewed-by, we would appreciate it if the maintainer could help review this.
2. Use devm_gpiod_get(,, GPIOD_OUT_HIGH) to fix the reset GPIO backpowering issue. [sashiko-bot]
3. Fixed Use-After-Free vulnerability due to dynamic DRM bridge removal in DSI detach callback.
4. Fixed Use-After-Free of regmap during DRM teardown because firmware upgrade worker defeats removal safeguard.
5. Fixed firmware upgrade performs a hardware reset, causing silent loss of DRM state and a blank screen.
6. Use of kmalloc for a 64KB software buffer is susceptible to memory fragmentation failures - Fix by using kvmalloc
7. Use the 'lt7911exc->ocm_lock' mutex and the 'lt7911exc->upgrade' flag to resolve the race condition between
DRM atomic modeset and firmware upgrade.
8. Use the 'lt7911exc->upgrade_lock' mutex to prevent redundant upgrades.
9. Use the 'lt7911exc->removed' flag to prevent the driver from being triggered to upgrade during the unload process.
- Link to v10: https://lore.kernel.org/lkml/20260519135816.26996-1-syyang@lontium.com/
Change in v10:
- dt-binding:
- drm/bridge:
1. Fixed the firmware upgrade error paths to always clear the upgrade
flag before returning, including firmware size validation failures
and allocation failures. [sashiko-bot]
2. Added proper locking in lt7911exc_atomic_pre_enable() and
lt7911exc_atomic_post_disable() to serialize register accesses with the
firmware upgrade flow and avoid concurrent I2C transactions.
3. Added an exclusivity check in lt7911exc_dsi_host_attach() to reject multiple
downstream attachments and prevent repeated drm_bridge_add() calls and panel bridge leaks.
4. Reworked lt7911exc_firmware_store() to use mutex_trylock() so concurrent sysfs writers
immediately return -EBUSY instead of blocking behind an active firmware upgrade.
5. Updated the remove path to prevent new firmware upgrade work from being queued after
device removal by setting the upgrade state before cancelling the worker.
- Link to v9: https://lore.kernel.org/lkml/20260519105019.22622-1-syyang@lontium.com/
Change in v9:
- dt-binding:
- drm/bridge:
1. DSI transfer callback returns success for reads without populating [sashiko-bot]
the receive buffer, leaking uninitialized memory. - fixed it by
implementing a strict whitelist mechanism.
2. DSI transfer callback polls for eDP video readiness before the
upstream encoder is enabled, guaranteeing a timeout. - removed
3. The driver attempts I2C transfers while the hardware is held in
physical reset. - fixed
4. Missing DRM_MIPI_DSI Kconfig dependency causes linker errors. - fixed
5. request_firmware is called while holding the hardware lock and
halting the MCU, risking a system pipeline stall. - fixed
6. Sleeping functions are called from atomic context in the DRM bridge callbacks. - fixed
7. lt7911exc_dsi_host_transfer bypasses the required MCU hardware halt sequence. - fixed by
internal firmware controls the panel initialization sequence and handles all MIPI
DSI command transmission.
- Link to v8: https://lore.kernel.org/lkml/20260515080934.9870-1-syyang@lontium.com/
Change in v8:
- dt-binding:
- drm/bridge:
1. Protect firmware upgrade and DRM bridge callback paths with ocm_lock. [sashiko-bot]
2. Remove the hardware reset from the remove callback, and ensure that
all hardware reset operations are protected by ocm_lock.
3. crc reconstruction explicitly casts each byte to u32 before shifting
4. The display configuration is handled by the firmware, and the MIPI
DSI host registration issue has been fixed.
5. The batch register read/write operations have already been updated
to include return value checking.
6. The dev_err_probe() used outside of probe context has been fixed.
- Link to v7: https://lore.kernel.org/lkml/20260512064013.40066-1-syyang@lontium.com/
Change in v7:
- dt-binding:
1. fix commit message typos(Receiver、signal) [sashiko-bot]
2. remove the ambiguity caused by "signal/dual".
- drm/bridge:
1. using devm_regulator_get_enable avoids power leaks. [sashiko-bot]
2. set reset gpio is low after cutting off power in lt7911exc_remove function, avoid backpowering.
3. synchronous request_firmware() call cause a permanent probe failure if the driver is built-in,
probe executes before the root filesystem is mounted, which would cause this to fail with -ENOENT,
we have removed this functionality. Use trigger to upgrade.
4. add `depends on I2C` and `select REGMAP_I2C` in Kconfig.
5. add return value of `devm_drm_bridge_add()` in `probe()`.
6. add directly header files (linux/slab.h, linux/delay.h, linux/regulator/consumer.h)
- Link to v6: https://lore.kernel.org/lkml/20260508134702.4713-1-syyang@lontium.com/
Change in v6:
- dt-binding:
- drm/bridge:
1. use #define FW_FILE "Lontium/lt7911exc_fw.bin" to match linux-firmware
- Link to v5: https://lore.kernel.org/lkml/20260506013153.2240-1-syyang@lontium.com/
Change in v5:
- dt-binding:
- drm/bridge:
1. Change "mipi" to "mipi dsi" in the commit message. [Dmitry]
2. Change "eDP/MIPI" to "eDP/MIPI DSI" in Kconfig.
- Link to v4: https://lore.kernel.org/lkml/20260430094612.3408174-1-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 | 16 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/lontium-lt7911exc.c | 698 ++++++++++++++++++
4 files changed, 804 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] 11+ messages in thread* [PATCH v11 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-22 1:57 [PATCH v11 0/2] " syyang @ 2026-05-22 1:57 ` syyang 2026-05-25 9:24 ` Claude review: " Claude Code Review Bot 0 siblings, 1 reply; 11+ messages in thread From: syyang @ 2026-05-22 1:57 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, xmzhu, xmzhu, rlyu, xbpeng, qdchen, llzhang, Sunyun Yang From: Sunyun Yang <syyang@lontium.com> Add support for the Lontium LT7911EXC bridge chip, which converts eDP input to MIPI DSI output using an internal firmware-controlled pipeline. The driver provides: - DRM bridge integration for eDP-to-DSI routing - MIPI DSI host interface for downstream panel attachment - Firmware upgrade mechanism over I2C (erase/program/verify) - GPIO-based reset and regulator management Display timing and MIPI DCS packet generation are handled by the chip firmware and are not configured by the driver. Signed-off-by: Sunyun Yang <syyang@lontium.com> --- drivers/gpu/drm/bridge/Kconfig | 16 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/lontium-lt7911exc.c | 698 +++++++++++++++++++++ 3 files changed, 715 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..013e431e8871 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -132,6 +132,22 @@ config DRM_ITE_IT6505 help ITE IT6505 DisplayPort bridge chip driver. +config DRM_LONTIUM_LT7911EXC + tristate "Lontium eDP/MIPI DSI bridge" + depends on OF + depends on I2C + select CRC32 + select DRM_PANEL + select DRM_MIPI_DSI + select DRM_KMS_HELPER + select FW_LOADER + select REGMAP_I2C + help + DRM driver for the Lontium LT7911EXC bridge + chip.The LT7911EXC converts eDP input to 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..1aec60f02f91 --- /dev/null +++ b/drivers/gpu/drm/bridge/lontium-lt7911exc.c @@ -0,0 +1,698 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2026 Lontium Semiconductor, Inc. + */ + +#include <linux/crc32.h> +#include <linux/delay.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 <linux/regulator/consumer.h> +#include <linux/slab.h> + +#include <drm/drm_atomic_helper.h> +#include <drm/drm_bridge.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_of.h> +#include <drm/drm_probe_helper.h> +#include <video/mipi_display.h> + +#define FW_SIZE (64 * 1024) +#define LT_PAGE_SIZE 32 +#define FW_FILE "Lontium/lt7911exc_fw.bin" +#define LT7911EXC_PAGE_CONTROL 0xff + +struct lt7911exc_dsi_output { + struct mipi_dsi_device *dev; + struct drm_panel *panel; + struct drm_bridge *bridge; +}; + +struct lt7911exc { + struct device *dev; + struct i2c_client *client; + struct drm_bridge bridge; + struct work_struct work; + struct mipi_dsi_host dsi_host; + struct lt7911exc_dsi_output output; + struct regmap *regmap; + /* Fast lock: guards short register r/w and status checks */ + struct mutex ocm_lock; + /* Long lock: serializes firmware upgrade process to prevent DRM interference */ + struct mutex upgrade_lock; + struct gpio_desc *reset_gpio; + int fw_version; + bool upgrade; + bool removed; +}; + +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; + + if (!length || (length & 3)) + return 0; + + 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 inline struct lt7911exc *dsi_host_to_lt7911exc(struct mipi_dsi_host *host) +{ + return container_of(host, struct lt7911exc, dsi_host); +} + +static void lt7911exc_reset(struct lt7911exc *lt7911exc) +{ + /* Assert reset pin: logical 1 -> physical state low (Reset Active) */ + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1); + msleep(20); + + /* Deassert reset pin: logical 0 -> physical state high (Run state) */ + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 0); + msleep(400); + + dev_dbg(lt7911exc->dev, "lt7911exc physical hardware reset sequence completed.\n"); +} + +static int lt7911exc_hw_mcu_halt(struct lt7911exc *lt7911exc) +{ + return regmap_write(lt7911exc->regmap, 0xe0ee, 0x01); +} + +static int lt7911exc_hw_mcu_run(struct lt7911exc *lt7911exc) +{ + return regmap_write(lt7911exc->regmap, 0xe0ee, 0x00); +} + +static int lt7911exc_regulator_enable(struct lt7911exc *lt7911exc) +{ + int ret; + + ret = devm_regulator_get_enable(lt7911exc->dev, "vcc"); + if (ret < 0) + return dev_err_probe(lt7911exc->dev, ret, "failed to enable vcc regulator\n"); + + usleep_range(5000, 10000); + + ret = devm_regulator_get_enable(lt7911exc->dev, "vdd"); + if (ret < 0) + return dev_err_probe(lt7911exc->dev, ret, "failed to enable vdd regulator\n"); + + return 0; +} + +static int lt7911exc_read_version(struct lt7911exc *lt7911exc) +{ + u8 buf[3]; + int ret; + + /* no need to halt MCU for this register access */ + ret = regmap_bulk_read(lt7911exc->regmap, 0xe081, buf, ARRAY_SIZE(buf)); + if (ret) + return ret; + + return (buf[0] << 16) | (buf[1] << 8) | buf[2]; +} + +static int lt7911exc_block_erase(struct lt7911exc *lt7911exc) +{ + struct device *dev = lt7911exc->dev; + const u32 addr = 0x00; + int ret; + + 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), + }; + + ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write)); + if (ret) + return ret; + + msleep(200); + dev_dbg(dev, "erase flash done.\n"); + + return 0; +} + +static int lt7911exc_prog_init(struct lt7911exc *lt7911exc, u64 addr) +{ + int ret; + + 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), + }; + + ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write)); + if (ret) + return ret; + + return 0; +} + +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); + ret = lt7911exc_prog_init(lt7911exc, addr); + if (ret) + return ret; + + 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, ARRAY_SIZE(crc_tmp)); + if (ret) { + dev_err(lt7911exc->dev, "Failed to read CRC: %d\n", ret); + return ret; + } + regmap_write(lt7911exc->regmap, 0xe0ee, 0x00); + + read_hw_crc = ((u32)crc_tmp[0] << 24) | ((u32)crc_tmp[1] << 16) | + ((u32)crc_tmp[2] << 8) | ((u32)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 void lt7911exc_firmware_upgrade_work(struct work_struct *work) +{ + struct lt7911exc *lt7911exc = container_of(work, struct lt7911exc, work); + struct device *dev = lt7911exc->dev; + const struct firmware *fw; + u8 *buffer; + size_t total_size = FW_SIZE - 4; + u32 crc32; + int ret; + + ret = request_firmware(&fw, FW_FILE, dev); + if (ret) { + dev_err(dev, "failed to load '%s'\n", FW_FILE); + goto out_clear_status; + } + + if (fw->size > total_size) { + dev_err(dev, "firmware too large (%zu > %zu)\n", fw->size, total_size); + goto out_release_fw; + } + + buffer = kvmalloc(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); + kvfree(buffer); + + mutex_lock(<7911exc->upgrade_lock); + + mutex_lock(<7911exc->ocm_lock); + lt7911exc_reset(lt7911exc); + lt7911exc_hw_mcu_halt(lt7911exc); + mutex_unlock(<7911exc->ocm_lock); + + ret = lt7911exc_block_erase(lt7911exc); + if (ret) { + dev_err(dev, "failed to block erase.\n"); + goto out_unlock; + } + + ret = lt7911exc_write_data(lt7911exc, fw, 0); + if (ret < 0) { + dev_err(dev, "failed to write firmware data\n"); + goto out_unlock; + } + + ret = lt7911exc_write_crc(lt7911exc, crc32, FW_SIZE - 4); + if (ret < 0) { + dev_err(dev, "failed to write firmware crc\n"); + goto out_unlock; + } + + mutex_lock(<7911exc->ocm_lock); + lt7911exc_reset(lt7911exc); + mutex_unlock(<7911exc->ocm_lock); + + ret = lt7911exc_upgrade_result(lt7911exc, crc32); + if (ret) + dev_err(dev, "firmware verification failed\n"); + +out_unlock: + mutex_lock(<7911exc->ocm_lock); + lt7911exc_hw_mcu_run(lt7911exc); + lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); + mutex_unlock(<7911exc->ocm_lock); + + mutex_unlock(<7911exc->upgrade_lock); + /* Notify DRM framework that hardware state changed/needs a modeset */ + if (lt7911exc->bridge.dev) + drm_kms_helper_hotplug_event(lt7911exc->bridge.dev); + +out_release_fw: + release_firmware(fw); + +out_clear_status: + mutex_lock(<7911exc->ocm_lock); + /* Only clear status if the module is not in the process of removal */ + if (!lt7911exc->removed) + lt7911exc->upgrade = false; + mutex_unlock(<7911exc->ocm_lock); +} + +static void lt7911exc_atomic_pre_enable(struct drm_bridge *bridge, struct drm_atomic_state *state) +{ + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge); + + guard(mutex)(<7911exc->ocm_lock); + + //enable mipi stream + if (!lt7911exc->upgrade) + regmap_write(lt7911exc->regmap, 0xe0b0, 0x01); +} + +static void lt7911exc_atomic_post_disable(struct drm_bridge *bridge, struct drm_atomic_state *state) +{ + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge); + + guard(mutex)(<7911exc->ocm_lock); + + //disable mipi stream + if (!lt7911exc->upgrade) + regmap_write(lt7911exc->regmap, 0xe0b0, 0x00); +} + +static int lt7911exc_bridge_attach(struct drm_bridge *bridge, + struct drm_encoder *encoder, + enum drm_bridge_attach_flags flags) +{ + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge); + + if (!lt7911exc->output.bridge) { + dev_warn(lt7911exc->dev, "Next bridge/panel not attached yet, deferring\n"); + return -EPROBE_DEFER; + } + + return drm_bridge_attach(encoder, lt7911exc->output.bridge, bridge, flags); +} + +static const struct drm_bridge_funcs lt7911exc_bridge_funcs = { + .attach = lt7911exc_bridge_attach, + .atomic_pre_enable = lt7911exc_atomic_pre_enable, + .atomic_post_disable = lt7911exc_atomic_post_disable, + .atomic_reset = drm_atomic_helper_bridge_reset, + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, +}; + +static int lt7911exc_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + struct drm_bridge *bridge; + struct drm_panel *panel; + int ret; + + if (lt7911exc->output.dev) + return -EBUSY; + + ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0, &panel, &bridge); + if (ret) + return ret; + + if (panel) { + bridge = drm_panel_bridge_add_typed(panel, DRM_MODE_CONNECTOR_DSI); + if (IS_ERR(bridge)) + return PTR_ERR(bridge); + } + lt7911exc->output.dev = dev; + lt7911exc->output.bridge = bridge; + lt7911exc->output.panel = panel; + + return 0; +} + +static int lt7911exc_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + + if (!lt7911exc->output.dev) + return 0; + + lt7911exc->output.dev = NULL; + + return 0; +} + +/* + * The internal firmware controls the panel initialization + * sequence and handles all MIPI DSI command transmission. + */ +static ssize_t lt7911exc_dsi_host_transfer(struct mipi_dsi_host *host, + const struct mipi_dsi_msg *msg) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + + if (msg->rx_len) { + dev_warn(lt7911exc->dev, "MIPI DSI read is not supported\n"); + return -EOPNOTSUPP; + } + + switch (msg->type) { + case MIPI_DSI_DCS_SHORT_WRITE: + case MIPI_DSI_DCS_SHORT_WRITE_PARAM: + case MIPI_DSI_DCS_LONG_WRITE: + case MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM: + case MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM: + case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM: + case MIPI_DSI_GENERIC_LONG_WRITE: + break; + default: + return -EOPNOTSUPP; + } + + guard(mutex)(<7911exc->ocm_lock); + + if (lt7911exc->upgrade) + return -EBUSY; + + return msg->tx_len; +} + +static const struct mipi_dsi_host_ops lt7911exc_dsi_host_ops = { + .attach = lt7911exc_dsi_host_attach, + .detach = lt7911exc_dsi_host_detach, + .transfer = lt7911exc_dsi_host_transfer, +}; + +static ssize_t lt7911exc_firmware_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t len) +{ + struct lt7911exc *lt7911exc = dev_get_drvdata(dev); + + if (!mutex_trylock(<7911exc->upgrade_lock)) + return -EBUSY; + + mutex_lock(<7911exc->ocm_lock); + + if (lt7911exc->upgrade || lt7911exc->removed) { + mutex_unlock(<7911exc->ocm_lock); + mutex_unlock(<7911exc->upgrade_lock); + return -EBUSY; + } + + lt7911exc->upgrade = true; + mutex_unlock(<7911exc->ocm_lock); + mutex_unlock(<7911exc->upgrade_lock); + + schedule_work(<7911exc->work); + + return len; +} + +static ssize_t lt7911exc_firmware_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct lt7911exc *lt7911exc = dev_get_drvdata(dev); + int version; + + mutex_lock(<7911exc->ocm_lock); + version = lt7911exc->fw_version; + mutex_unlock(<7911exc->ocm_lock); + return sysfs_emit(buf, "0x%04x\n", version); +} + +static DEVICE_ATTR_RW(lt7911exc_firmware); + +static struct attribute *lt7911exc_attrs[] = { + &dev_attr_lt7911exc_firmware.attr, + NULL, +}; + +static const struct attribute_group lt7911exc_attr_group = { + .attrs = lt7911exc_attrs, +}; + +static const struct attribute_group *lt7911exc_attr_groups[] = { + <7911exc_attr_group, + NULL, +}; + +static int lt7911exc_probe(struct i2c_client *client) +{ + struct lt7911exc *lt7911exc; + struct device *dev = &client->dev; + struct device_node *np = dev->of_node; + int ret; + + if (!np) + return -ENODEV; + + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) + return dev_err_probe(dev, -ENODEV, "device doesn't support I2C\n"); + + lt7911exc = devm_drm_bridge_alloc(dev, struct lt7911exc, bridge, <7911exc_bridge_funcs); + if (IS_ERR(lt7911exc)) + return dev_err_probe(dev, PTR_ERR(lt7911exc), "drm bridge alloc failed.\n"); + + dev_set_drvdata(dev, lt7911exc); + + lt7911exc->client = client; + lt7911exc->dev = dev; + lt7911exc->upgrade = false; + lt7911exc->removed = false; + + ret = devm_mutex_init(dev, <7911exc->ocm_lock); + if (ret) + return dev_err_probe(dev, ret, "failed to init mutex\n"); + + ret = devm_mutex_init(dev, <7911exc->upgrade_lock); + if (ret) + return dev_err_probe(dev, ret, "failed to init upgrade_lock\n"); + + lt7911exc->regmap = devm_regmap_init_i2c(client, <7911exc_regmap_config); + if (IS_ERR(lt7911exc->regmap)) + return dev_err_probe(dev, PTR_ERR(lt7911exc->regmap), "regmap i2c init failed\n"); + + lt7911exc->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(lt7911exc->reset_gpio)) + return dev_err_probe(dev, PTR_ERR(lt7911exc->reset_gpio), + "failed to acquire reset gpio\n"); + + ret = lt7911exc_regulator_enable(lt7911exc); + if (ret) + return ret; + + lt7911exc_reset(lt7911exc); + + mutex_lock(<7911exc->ocm_lock); + + lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); + + mutex_unlock(<7911exc->ocm_lock); + + if (lt7911exc->fw_version < 0) + return dev_err_probe(dev, lt7911exc->fw_version, "failed read version of chip\n"); + + lt7911exc->dsi_host.dev = dev; + lt7911exc->dsi_host.ops = <7911exc_dsi_host_ops; + lt7911exc->bridge.of_node = np; + + INIT_WORK(<7911exc->work, lt7911exc_firmware_upgrade_work); + + i2c_set_clientdata(client, lt7911exc); + + drm_bridge_add(<7911exc->bridge); + + ret = mipi_dsi_host_register(<7911exc->dsi_host); + if (ret) { + drm_bridge_remove(<7911exc->bridge); + return ret; + } + + return 0; +} + +static void lt7911exc_remove(struct i2c_client *client) +{ + struct lt7911exc *lt7911exc = i2c_get_clientdata(client); + + mutex_lock(<7911exc->ocm_lock); + lt7911exc->removed = true; + lt7911exc->upgrade = true; + mutex_unlock(<7911exc->ocm_lock); + + cancel_work_sync(<7911exc->work); + + mipi_dsi_host_unregister(<7911exc->dsi_host); + drm_bridge_remove(<7911exc->bridge); + + if (lt7911exc->output.panel && lt7911exc->output.bridge) + drm_panel_bridge_remove(lt7911exc->output.bridge); + + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1); +} + +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, + .remove = lt7911exc_remove, + .driver = { + .name = "lt7911exc", + .of_match_table = lt7911exc_devices, + .dev_groups = lt7911exc_attr_groups, + }, +}; +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] 11+ messages in thread
* Claude review: drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-22 1:57 ` [PATCH v11 2/2] drm/bridge: " syyang @ 2026-05-25 9:24 ` Claude Code Review Bot 0 siblings, 0 replies; 11+ messages in thread From: Claude Code Review Bot @ 2026-05-25 9:24 UTC (permalink / raw) To: dri-devel-reviews Patch Review #### Bug: Firmware leak and goto label ordering error The `lt7911exc_firmware_upgrade_work` function has a critical goto ordering problem. When the firmware upgrade fails after `mutex_lock(<7911exc->upgrade_lock)`, the error path jumps to `out_unlock`, which falls through to `out_release_fw` and then `out_clear_status`. But when `fw->size > total_size` or `kvmalloc` fails, the code jumps to `out_release_fw` — and then falls through to `out_clear_status`. However, the `out_unlock` label also falls through to `out_release_fw`. The problem is this sequence: ```c out_unlock: mutex_lock(<7911exc->ocm_lock); lt7911exc_hw_mcu_run(lt7911exc); lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); mutex_unlock(<7911exc->ocm_lock); mutex_unlock(<7911exc->upgrade_lock); /* Notify DRM framework that hardware state changed/needs a modeset */ if (lt7911exc->bridge.dev) drm_kms_helper_hotplug_event(lt7911exc->bridge.dev); out_release_fw: release_firmware(fw); out_clear_status: ``` When `lt7911exc_block_erase` or `lt7911exc_write_data` fails and we goto `out_unlock`, this is correct. But the `out_release_fw` label is positioned **after** `out_unlock`, so when we goto `out_release_fw` directly (from the size check or kvmalloc failure), `release_firmware` is called but the `upgrade_lock` is never unlocked — the lock was never taken in that path, so that's fine. Wait — actually, looking more carefully, the `upgrade_lock` is taken **after** the `buffer` work: ```c mutex_lock(<7911exc->upgrade_lock); ... ret = lt7911exc_block_erase(lt7911exc); if (ret) { ... goto out_unlock; } ``` The early errors (`fw->size > total_size`, kvmalloc failure) goto `out_release_fw` which does NOT unlock `upgrade_lock` because it was never taken — this is correct. But the real bug is that the `out_unlock` path runs `lt7911exc_hw_mcu_run` and `lt7911exc_read_version` even on error, then does `drm_kms_helper_hotplug_event` after a failed upgrade, which generates a spurious hotplug event. The hotplug event is misleading but not a crash. **However**, there is a subtle but real issue: the `out_clear_status` block accesses `lt7911exc` fields after `release_firmware(fw)` — the firmware pointer `fw` is unrelated to `lt7911exc`, so this is fine for memory safety. But the **actual bug** is: if `request_firmware` fails, we jump to `out_clear_status` directly, and the `upgrade` flag is correctly cleared. Good. #### Bug: Firmware data vs CRC mismatch The firmware upgrade writes a CRC computed from a zero-padded copy: ```c buffer = kvmalloc(total_size, GFP_KERNEL); ... memset(buffer, 0xff, total_size); memcpy(buffer, fw->data, fw->size); crc32 = cal_crc32_custom(buffer, total_size); kvfree(buffer); ``` But the actual write to hardware uses the raw firmware data: ```c ret = lt7911exc_write_data(lt7911exc, fw, 0); ``` Inside `lt7911exc_write_data`, only `fw->size` bytes are written. The CRC is computed over the full `total_size` (64K - 4) with 0xFF padding, but the hardware only receives `fw->size` bytes. Either the full padded buffer should be written, or the CRC should be computed over just `fw->data` with `fw->size`. As-is, the CRC verification at the end (`lt7911exc_upgrade_result`) will always fail if `fw->size < total_size`, because the hardware CRC won't match the padded CRC. #### Issue: Unchecked regmap writes in flash programming Throughout the flash programming functions, many `regmap_write` calls have their return values silently discarded: ```c static int lt7911exc_write_crc(struct lt7911exc *lt7911exc, u32 crc32, u64 addr) { ... 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); ``` Similarly in `lt7911exc_write_data`: ```c regmap_write(lt7911exc->regmap, 0xe05f, 0x05); regmap_write(lt7911exc->regmap, 0xe05f, 0x01); ``` And in `lt7911exc_upgrade_result`: ```c regmap_write(lt7911exc->regmap, 0xe0ee, 0x01); regmap_write(lt7911exc->regmap, 0xe07b, 0x60); regmap_write(lt7911exc->regmap, 0xe07b, 0x40); ``` If any of these I2C writes fail during a flash erase/program operation, the firmware upgrade will silently produce corrupted results. These should at minimum be checked and cause an early return on error. #### Issue: Locking design — upgrade_lock released too early in sysfs path In `lt7911exc_firmware_store`, the `upgrade_lock` is taken with `mutex_trylock`, then the `upgrade` flag is set, and **both locks are released**: ```c if (!mutex_trylock(<7911exc->upgrade_lock)) return -EBUSY; mutex_lock(<7911exc->ocm_lock); ... lt7911exc->upgrade = true; mutex_unlock(<7911exc->ocm_lock); mutex_unlock(<7911exc->upgrade_lock); schedule_work(<7911exc->work); ``` The `upgrade_lock` is released before `schedule_work`. Then in the worker, `upgrade_lock` is re-acquired. But between the sysfs `store` releasing `upgrade_lock` and the worker acquiring it, a second `store` could `mutex_trylock(&upgrade_lock)` successfully and also see `upgrade == true` → return `-EBUSY`. This is actually safe because of the `upgrade` flag check, but it means the `upgrade_lock` in `firmware_store` serves no real purpose — the `ocm_lock` + `upgrade` flag alone would be sufficient. The two-lock scheme is confusing and should be simplified. #### Issue: Sysfs attribute naming ```c static DEVICE_ATTR_RW(lt7911exc_firmware); ``` This creates an attribute named `lt7911exc_firmware`. Kernel sysfs conventions say the attribute should not include the driver name as a prefix — it should just be `firmware` (or `firmware_version` / `firmware_update`). The device is already scoped to this driver instance. Also, having a single attribute for both read (version) and write (trigger upgrade) is confusing — consider splitting into separate `firmware_version` (read-only) and `firmware_update` (write-only) attributes. #### Issue: Kconfig help text formatting ```c help DRM driver for the Lontium LT7911EXC bridge chip.The LT7911EXC converts eDP input to MIPI ``` Missing space after period: `chip.The` should be `chip. The`. #### Issue: Comment style Several comments use C++ style which, while accepted, is inconsistent with typical kernel bridge driver style: ```c //hardware requires delay usleep_range(1000, 2000); ``` ```c //enable mipi stream ``` ```c //disable mipi stream ``` Should use `/* ... */` style per kernel coding conventions. #### Issue: `lt7911exc_dsi_host_transfer` is a no-op The transfer callback accepts write commands but does nothing with them: ```c guard(mutex)(<7911exc->ocm_lock); if (lt7911exc->upgrade) return -EBUSY; return msg->tx_len; ``` The comment explains this is intentional (firmware handles DSI init), which is fine. But the function acquires the `ocm_lock` and checks the `upgrade` flag for writes that are immediately discarded — this is unnecessary overhead. If the firmware truly handles everything, consider whether this callback is needed at all, or document more clearly why the lock/flag check is still needed (e.g., to prevent panel init during upgrade). #### Issue: `lt7911exc_prog_init` has an unnecessary pattern ```c ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write)); if (ret) return ret; return 0; ``` Should just be `return regmap_multi_reg_write(...)`. #### Nit: `u64` type for addresses that fit in `u32` ```c static int lt7911exc_prog_init(struct lt7911exc *lt7911exc, u64 addr) ``` ```c static int lt7911exc_write_data(struct lt7911exc *lt7911exc, const struct firmware *fw, u64 addr) ``` ```c static int lt7911exc_write_crc(struct lt7911exc *lt7911exc, u32 crc32, u64 addr) ``` The flash addresses are clearly within a 64KB space (`FW_SIZE = 64 * 1024`), and the register writes only use the lower 24 bits. Using `u64` for these addresses is misleading — `u32` would be appropriate. #### Nit: `cal_crc32_custom` naming The function name `cal_crc32_custom` is vague. Something like `lt7911exc_crc32_be_byteswap` would better describe that it computes a CRC32-BE over byte-reversed 32-bit words. Also, the `length` parameter should be `size_t`, not `u64`. #### Nit: `dev_set_drvdata` and `i2c_set_clientdata` redundancy In probe: ```c dev_set_drvdata(dev, lt7911exc); ... i2c_set_clientdata(client, lt7911exc); ``` `i2c_set_clientdata(client, lt7911exc)` calls `dev_set_drvdata(&client->dev, lt7911exc)` internally. Since `dev = &client->dev`, these are redundant. Remove the `dev_set_drvdata` call. #### Nit: `of_node` check is unnecessary ```c if (!np) return -ENODEV; ``` The driver has `depends on OF` in Kconfig and uses OF match table, so `of_node` will always be set when the driver probes. This check is superfluous. #### Missing: `drm_panel_bridge_remove` leak on detach In `lt7911exc_dsi_host_detach`, if a panel bridge was created via `drm_panel_bridge_add_typed` in the attach path, the detach only clears `output.dev` but does not call `drm_panel_bridge_remove`: ```c static int lt7911exc_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev) { struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); if (!lt7911exc->output.dev) return 0; lt7911exc->output.dev = NULL; return 0; } ``` The panel bridge is only cleaned up in `lt7911exc_remove`. If a DSI device attaches, detaches, and attaches again, `drm_panel_bridge_add_typed` would be called a second time, leaking the first bridge. The detach callback should clean up `output.bridge` and `output.panel` as well. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v10 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge
@ 2026-05-19 13:58 syyang
2026-05-19 13:58 ` [PATCH v10 2/2] drm/bridge: " syyang
0 siblings, 1 reply; 11+ messages in thread
From: syyang @ 2026-05-19 13:58 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, xmzhu, xmzhu, rlyu, xbpeng, qdchen,
llzhang, Sunyun Yang
From: Sunyun Yang <syyang@lontium.com>
The LT7911EXC is an I2C-controlled bridge that receives eDP1.4
and output mipi dsi. 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 v10:
- dt-binding:
- drm/bridge:
1. Fixed the firmware upgrade error paths to always clear the upgrade
flag before returning, including firmware size validation failures
and allocation failures. [sashiko-bot]
2. Added proper locking in lt7911exc_atomic_pre_enable() and
lt7911exc_atomic_post_disable() to serialize register accesses with the
firmware upgrade flow and avoid concurrent I2C transactions.
3. Added an exclusivity check in lt7911exc_dsi_host_attach() to reject multiple
downstream attachments and prevent repeated drm_bridge_add() calls and panel bridge leaks.
4. Reworked lt7911exc_firmware_store() to use mutex_trylock() so concurrent sysfs writers
immediately return -EBUSY instead of blocking behind an active firmware upgrade.
5. Updated the remove path to prevent new firmware upgrade work from being queued after
device removal by setting the upgrade state before cancelling the worker.
- Link to v9: https://lore.kernel.org/lkml/20260519105019.22622-1-syyang@lontium.com/
Change in v9:
- dt-binding:
- drm/bridge:
1. DSI transfer callback returns success for reads without populating [sashiko-bot]
the receive buffer, leaking uninitialized memory. - fixed it by
implementing a strict whitelist mechanism.
2. DSI transfer callback polls for eDP video readiness before the
upstream encoder is enabled, guaranteeing a timeout. - removed
3. The driver attempts I2C transfers while the hardware is held in
physical reset. - fixed
4. Missing DRM_MIPI_DSI Kconfig dependency causes linker errors. - fixed
5. request_firmware is called while holding the hardware lock and
halting the MCU, risking a system pipeline stall. - fixed
6. Sleeping functions are called from atomic context in the DRM bridge callbacks. - fixed
7. lt7911exc_dsi_host_transfer bypasses the required MCU hardware halt sequence. - fixed by
internal firmware controls the panel initialization sequence and handles all MIPI
DSI command transmission.
- Link to v8: https://lore.kernel.org/lkml/20260515080934.9870-1-syyang@lontium.com/
Change in v8:
- dt-binding:
- drm/bridge:
1. Protect firmware upgrade and DRM bridge callback paths with ocm_lock. [sashiko-bot]
2. Remove the hardware reset from the remove callback, and ensure that
all hardware reset operations are protected by ocm_lock.
3. crc reconstruction explicitly casts each byte to u32 before shifting
4. The display configuration is handled by the firmware, and the MIPI
DSI host registration issue has been fixed.
5. The batch register read/write operations have already been updated
to include return value checking.
6. The dev_err_probe() used outside of probe context has been fixed.
- Link to v7: https://lore.kernel.org/lkml/20260512064013.40066-1-syyang@lontium.com/
Change in v7:
- dt-binding:
1. fix commit message typos(Receiver、signal) [sashiko-bot]
2. remove the ambiguity caused by "signal/dual".
- drm/bridge:
1. using devm_regulator_get_enable avoids power leaks. [sashiko-bot]
2. set reset gpio is low after cutting off power in lt7911exc_remove function, avoid backpowering.
3. synchronous request_firmware() call cause a permanent probe failure if the driver is built-in,
probe executes before the root filesystem is mounted, which would cause this to fail with -ENOENT,
we have removed this functionality. Use trigger to upgrade.
4. add `depends on I2C` and `select REGMAP_I2C` in Kconfig.
5. add return value of `devm_drm_bridge_add()` in `probe()`.
6. add directly header files (linux/slab.h, linux/delay.h, linux/regulator/consumer.h)
- Link to v6: https://lore.kernel.org/lkml/20260508134702.4713-1-syyang@lontium.com/
Change in v6:
- dt-binding:
- drm/bridge:
1. use #define FW_FILE "Lontium/lt7911exc_fw.bin" to match linux-firmware
- Link to v5: https://lore.kernel.org/lkml/20260506013153.2240-1-syyang@lontium.com/
Change in v5:
- dt-binding:
- drm/bridge:
1. Change "mipi" to "mipi dsi" in the commit message. [Dmitry]
2. Change "eDP/MIPI" to "eDP/MIPI DSI" in Kconfig.
- Link to v4: https://lore.kernel.org/lkml/20260430094612.3408174-1-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 | 16 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/lontium-lt7911exc.c | 688 ++++++++++++++++++
4 files changed, 794 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] 11+ messages in thread* [PATCH v10 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-19 13:58 [PATCH v10 0/2] " syyang @ 2026-05-19 13:58 ` syyang 2026-05-25 12:53 ` Claude review: " Claude Code Review Bot 0 siblings, 1 reply; 11+ messages in thread From: syyang @ 2026-05-19 13:58 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, xmzhu, xmzhu, rlyu, xbpeng, qdchen, llzhang, Sunyun Yang From: Sunyun Yang <syyang@lontium.com> Add support for the Lontium LT7911EXC bridge chip, which converts eDP input to MIPI DSI output using an internal firmware-controlled pipeline. The driver provides: - DRM bridge integration for eDP-to-DSI routing - MIPI DSI host interface for downstream panel attachment - Firmware upgrade mechanism over I2C (erase/program/verify) - GPIO-based reset and regulator management Display timing and DSI packet generation are handled by the chip firmware and are not configured by the driver. Signed-off-by: Sunyun Yang <syyang@lontium.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- drivers/gpu/drm/bridge/Kconfig | 16 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/lontium-lt7911exc.c | 688 +++++++++++++++++++++ 3 files changed, 705 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..013e431e8871 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -132,6 +132,22 @@ config DRM_ITE_IT6505 help ITE IT6505 DisplayPort bridge chip driver. +config DRM_LONTIUM_LT7911EXC + tristate "Lontium eDP/MIPI DSI bridge" + depends on OF + depends on I2C + select CRC32 + select DRM_PANEL + select DRM_MIPI_DSI + select DRM_KMS_HELPER + select FW_LOADER + select REGMAP_I2C + help + DRM driver for the Lontium LT7911EXC bridge + chip.The LT7911EXC converts eDP input to 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..5f7d327d3519 --- /dev/null +++ b/drivers/gpu/drm/bridge/lontium-lt7911exc.c @@ -0,0 +1,688 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2026 Lontium Semiconductor, Inc. + */ + +#include <linux/crc32.h> +#include <linux/delay.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 <linux/regulator/consumer.h> +#include <linux/slab.h> + +#include <drm/drm_atomic_helper.h> +#include <drm/drm_bridge.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_of.h> +#include <video/mipi_display.h> + +#define FW_SIZE (64 * 1024) +#define LT_PAGE_SIZE 32 +#define FW_FILE "Lontium/lt7911exc_fw.bin" +#define LT7911EXC_PAGE_CONTROL 0xff + +struct lt7911exc_dsi_output { + struct mipi_dsi_device *dev; + struct drm_panel *panel; + struct drm_bridge *bridge; +}; + +struct lt7911exc { + struct device *dev; + struct i2c_client *client; + struct drm_bridge bridge; + struct work_struct work; + struct mipi_dsi_host dsi_host; + struct lt7911exc_dsi_output output; + + struct regmap *regmap; + /* Prevents concurrent register accesses by multiple read/write operations in the driver */ + struct mutex ocm_lock; + struct gpio_desc *reset_gpio; + int fw_version; + bool upgrade; +}; + +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; + + if (!length || (length & 3)) + return 0; + + 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 inline struct lt7911exc *dsi_host_to_lt7911exc(struct mipi_dsi_host *host) +{ + return container_of(host, struct lt7911exc, dsi_host); +} + +/* + * Purpose of this function is rest gpio: high -> low -> high + * This clears the previous configuration in the chip, + * and finally remains high to allow the firmware to run again. + */ +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_hw_mcu_halt(struct lt7911exc *lt7911exc) +{ + return regmap_write(lt7911exc->regmap, 0xe0ee, 0x01); +} + +static int lt7911exc_hw_mcu_run(struct lt7911exc *lt7911exc) +{ + return regmap_write(lt7911exc->regmap, 0xe0ee, 0x00); +} + +static int lt7911exc_regulator_enable(struct lt7911exc *lt7911exc) +{ + int ret; + + ret = devm_regulator_get_enable(lt7911exc->dev, "vcc"); + if (ret < 0) + return dev_err_probe(lt7911exc->dev, ret, "failed to enable vcc regulator\n"); + + usleep_range(5000, 10000); + + ret = devm_regulator_get_enable(lt7911exc->dev, "vdd"); + if (ret < 0) + return dev_err_probe(lt7911exc->dev, ret, "failed to enable vdd regulator\n"); + + return 0; +} + +static int lt7911exc_read_version(struct lt7911exc *lt7911exc) +{ + u8 buf[3]; + int ret; + + /* no need to halt MCU for this register access */ + ret = regmap_bulk_read(lt7911exc->regmap, 0xe081, buf, ARRAY_SIZE(buf)); + if (ret) + return ret; + + return (buf[0] << 16) | (buf[1] << 8) | buf[2]; +} + +static int lt7911exc_block_erase(struct lt7911exc *lt7911exc) +{ + struct device *dev = lt7911exc->dev; + const u32 addr = 0x00; + int ret; + + 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), + }; + + ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write)); + if (ret) + return ret; + + msleep(200); + dev_dbg(dev, "erase flash done.\n"); + + return 0; +} + +static int lt7911exc_prog_init(struct lt7911exc *lt7911exc, u64 addr) +{ + int ret; + + 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), + }; + + ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write)); + if (ret) + return ret; + + return 0; +} + +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); + ret = lt7911exc_prog_init(lt7911exc, addr); + if (ret) + return ret; + + 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, ARRAY_SIZE(crc_tmp)); + if (ret) { + dev_err(lt7911exc->dev, "Failed to read CRC: %d\n", ret); + return ret; + } + regmap_write(lt7911exc->regmap, 0xe0ee, 0x00); + + read_hw_crc = ((u32)crc_tmp[0] << 24) | ((u32)crc_tmp[1] << 16) | + ((u32)crc_tmp[2] << 8) | ((u32)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 void lt7911exc_firmware_upgrade_work(struct work_struct *work) +{ + struct lt7911exc *lt7911exc = container_of(work, struct lt7911exc, work); + struct device *dev = lt7911exc->dev; + const struct firmware *fw; + u8 *buffer; + size_t total_size = FW_SIZE - 4; + u32 crc32; + int ret; + + ret = request_firmware(&fw, FW_FILE, dev); + if (ret) { + dev_err(dev, "failed to load '%s'\n", FW_FILE); + goto out_clear_status; + } + + if (fw->size > total_size) { + dev_err(dev, "firmware too large (%zu > %zu)\n", fw->size, total_size); + goto out_release_fw; + } + + buffer = kmalloc(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); + + mutex_lock(<7911exc->ocm_lock); + + lt7911exc_reset(lt7911exc); + + lt7911exc_hw_mcu_halt(lt7911exc); + + ret = lt7911exc_block_erase(lt7911exc); + if (ret) { + dev_err(dev, "failed to block erase.\n"); + goto out_unlock; + } + + ret = lt7911exc_write_data(lt7911exc, fw, 0); + if (ret < 0) { + dev_err(dev, "failed to write firmware data\n"); + goto out_unlock; + } + + ret = lt7911exc_write_crc(lt7911exc, crc32, FW_SIZE - 4); + if (ret < 0) { + dev_err(dev, "failed to write firmware crc\n"); + goto out_unlock; + } + + lt7911exc_reset(lt7911exc); + + ret = lt7911exc_upgrade_result(lt7911exc, crc32); + if (ret) + dev_err(dev, "firmware verification failed\n"); + +out_unlock: + lt7911exc_hw_mcu_run(lt7911exc); + lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); + mutex_unlock(<7911exc->ocm_lock); + +out_release_fw: + release_firmware(fw); + +out_clear_status: + mutex_lock(<7911exc->ocm_lock); + lt7911exc->upgrade = false; + mutex_unlock(<7911exc->ocm_lock); +} + +static void lt7911exc_atomic_pre_enable(struct drm_bridge *bridge, struct drm_atomic_state *state) +{ + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge); + + mutex_lock(<7911exc->ocm_lock); + + //enable mipi stream + if (!lt7911exc->upgrade) + regmap_write(lt7911exc->regmap, 0xe0b0, 0x01); + + mutex_unlock(<7911exc->ocm_lock); +} + +static void lt7911exc_atomic_post_disable(struct drm_bridge *bridge, struct drm_atomic_state *state) +{ + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge); + + mutex_lock(<7911exc->ocm_lock); + + //disable mipi stream + if (!lt7911exc->upgrade) + regmap_write(lt7911exc->regmap, 0xe0b0, 0x00); + + mutex_unlock(<7911exc->ocm_lock); +} + +static int lt7911exc_bridge_attach(struct drm_bridge *bridge, + struct drm_encoder *encoder, + enum drm_bridge_attach_flags flags) +{ + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge); + + if (!lt7911exc->output.bridge) { + dev_warn(lt7911exc->dev, "Next bridge/panel not attached yet, deferring\n"); + return -EPROBE_DEFER; + } + + return drm_bridge_attach(encoder, lt7911exc->output.bridge, bridge, flags); +} + +static const struct drm_bridge_funcs lt7911exc_bridge_funcs = { + .attach = lt7911exc_bridge_attach, + .atomic_pre_enable = lt7911exc_atomic_pre_enable, + .atomic_post_disable = lt7911exc_atomic_post_disable, + .atomic_reset = drm_atomic_helper_bridge_reset, + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, +}; + +static int lt7911exc_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + struct drm_bridge *bridge; + struct drm_panel *panel; + int ret; + + if (lt7911exc->output.dev) + return -EBUSY; + + ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0, &panel, &bridge); + if (ret) + return ret; + + if (panel) { + bridge = drm_panel_bridge_add_typed(panel, DRM_MODE_CONNECTOR_DSI); + if (IS_ERR(bridge)) + return PTR_ERR(bridge); + } + lt7911exc->output.dev = dev; + lt7911exc->output.bridge = bridge; + lt7911exc->output.panel = panel; + drm_bridge_add(<7911exc->bridge); + + return 0; +} + +static int lt7911exc_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + + if (!lt7911exc->output.dev) + return 0; + + drm_bridge_remove(<7911exc->bridge); + if (lt7911exc->output.panel) + drm_panel_bridge_remove(lt7911exc->output.bridge); + + lt7911exc->output.dev = NULL; + lt7911exc->output.bridge = NULL; + lt7911exc->output.panel = NULL; + + return 0; +} + +/* + * The internal firmware controls the panel initialization + * sequence and handles all MIPI DSI command transmission. + */ +static ssize_t lt7911exc_dsi_host_transfer(struct mipi_dsi_host *host, + const struct mipi_dsi_msg *msg) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + + if (msg->rx_len) { + dev_warn(lt7911exc->dev, "MIPI DSI read is not supported\n"); + return -EOPNOTSUPP; + } + + switch (msg->type) { + case MIPI_DSI_DCS_SHORT_WRITE: + case MIPI_DSI_DCS_SHORT_WRITE_PARAM: + case MIPI_DSI_DCS_LONG_WRITE: + case MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM: + case MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM: + case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM: + case MIPI_DSI_GENERIC_LONG_WRITE: + break; + default: + return -EOPNOTSUPP; + } + + if (!mutex_trylock(<7911exc->ocm_lock)) + return -EBUSY; + + if (lt7911exc->upgrade) { + mutex_unlock(<7911exc->ocm_lock); + return -EBUSY; + } + mutex_unlock(<7911exc->ocm_lock); + + return msg->tx_len; +} + +static const struct mipi_dsi_host_ops lt7911exc_dsi_host_ops = { + .attach = lt7911exc_dsi_host_attach, + .detach = lt7911exc_dsi_host_detach, + .transfer = lt7911exc_dsi_host_transfer, +}; + +static ssize_t lt7911exc_firmware_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t len) +{ + struct lt7911exc *lt7911exc = dev_get_drvdata(dev); + + if (!mutex_trylock(<7911exc->ocm_lock)) + return -EBUSY; + + if (lt7911exc->upgrade) { + mutex_unlock(<7911exc->ocm_lock); + return -EBUSY; + } + + lt7911exc->upgrade = true; + mutex_unlock(<7911exc->ocm_lock); + + schedule_work(<7911exc->work); + + return len; +} + +static ssize_t lt7911exc_firmware_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct lt7911exc *lt7911exc = dev_get_drvdata(dev); + int version; + + mutex_lock(<7911exc->ocm_lock); + version = lt7911exc->fw_version; + mutex_unlock(<7911exc->ocm_lock); + return sysfs_emit(buf, "0x%04x\n", version); +} + +static DEVICE_ATTR_RW(lt7911exc_firmware); + +static struct attribute *lt7911exc_attrs[] = { + &dev_attr_lt7911exc_firmware.attr, + NULL, +}; + +static const struct attribute_group lt7911exc_attr_group = { + .attrs = lt7911exc_attrs, +}; + +static const struct attribute_group *lt7911exc_attr_groups[] = { + <7911exc_attr_group, + NULL, +}; + +static int lt7911exc_probe(struct i2c_client *client) +{ + struct lt7911exc *lt7911exc; + struct device *dev = &client->dev; + struct device_node *np = dev->of_node; + int ret; + + if (!np) + return -ENODEV; + + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) + return dev_err_probe(dev, -ENODEV, "device doesn't support I2C\n"); + + lt7911exc = devm_drm_bridge_alloc(dev, struct lt7911exc, bridge, <7911exc_bridge_funcs); + if (IS_ERR(lt7911exc)) + return dev_err_probe(dev, PTR_ERR(lt7911exc), "drm bridge alloc failed.\n"); + + dev_set_drvdata(dev, lt7911exc); + + lt7911exc->client = client; + lt7911exc->dev = dev; + lt7911exc->upgrade = false; + + ret = devm_mutex_init(dev, <7911exc->ocm_lock); + if (ret) + return dev_err_probe(dev, ret, "failed to init mutex\n"); + + lt7911exc->regmap = devm_regmap_init_i2c(client, <7911exc_regmap_config); + if (IS_ERR(lt7911exc->regmap)) + return dev_err_probe(dev, PTR_ERR(lt7911exc->regmap), "regmap i2c init failed\n"); + + /* + * reset GPIO is defined as active low in device tree. + * gpiod_set_value_cansleep() uses logical value: + * 1 -> asserted (active) -> physical low -> reset enabled -> chip stop + * 0 -> deasserted (inactive) -> physical high -> reset released -> chip run + */ + 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"); + + ret = lt7911exc_regulator_enable(lt7911exc); + if (ret) + return ret; + + lt7911exc_reset(lt7911exc); + + mutex_lock(<7911exc->ocm_lock); + + lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); + + mutex_unlock(<7911exc->ocm_lock); + + if (lt7911exc->fw_version < 0) + return dev_err_probe(dev, lt7911exc->fw_version, "failed read version of chip\n"); + + lt7911exc->dsi_host.dev = dev; + lt7911exc->dsi_host.ops = <7911exc_dsi_host_ops; + lt7911exc->bridge.of_node = np; + + INIT_WORK(<7911exc->work, lt7911exc_firmware_upgrade_work); + + i2c_set_clientdata(client, lt7911exc); + + return mipi_dsi_host_register(<7911exc->dsi_host); +} + +static void lt7911exc_remove(struct i2c_client *client) +{ + struct lt7911exc *lt7911exc = i2c_get_clientdata(client); + + mipi_dsi_host_unregister(<7911exc->dsi_host); + + mutex_lock(<7911exc->ocm_lock); + lt7911exc->upgrade = true; + mutex_unlock(<7911exc->ocm_lock); + + cancel_work_sync(<7911exc->work); + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1); +} + +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, + .remove = lt7911exc_remove, + .driver = { + .name = "lt7911exc", + .of_match_table = lt7911exc_devices, + .dev_groups = lt7911exc_attr_groups, + }, +}; +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] 11+ messages in thread
* Claude review: drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-19 13:58 ` [PATCH v10 2/2] drm/bridge: " syyang @ 2026-05-25 12:53 ` Claude Code Review Bot 0 siblings, 0 replies; 11+ messages in thread From: Claude Code Review Bot @ 2026-05-25 12:53 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Issues (by severity):** #### 1. Firmware data / CRC mismatch (bug) The CRC is computed on a zero-padded copy of the firmware: ```c buffer = kmalloc(total_size, GFP_KERNEL); ... memset(buffer, 0xff, total_size); memcpy(buffer, fw->data, fw->size); crc32 = cal_crc32_custom(buffer, total_size); kfree(buffer); ``` But `lt7911exc_write_data` writes from the original `fw`, not from `buffer`: ```c ret = lt7911exc_write_data(lt7911exc, fw, 0); ``` Inside `lt7911exc_write_data`, only `fw->size` bytes are written. The flash region from `fw->size` to `total_size` will contain whatever was there after the erase (typically 0xff from a flash erase, which matches the `memset` padding). So this _might_ work in practice if the erase sets bytes to 0xff, but it's fragile — the correctness depends on an implicit assumption about the erase pattern. Either write the padded `buffer` instead of `fw->data`, or compute the CRC only over `fw->size` bytes. As written, this is a latent correctness bug. #### 2. `regmap_raw_write` vs `regmap_noinc_write` for firmware FIFO writes ```c ret = regmap_raw_write(lt7911exc->regmap, 0xe05d, &data[offset], page_len); ``` Register `0xe05d` appears to be a FIFO data port (all page bytes go to the same address). The sister driver `lontium-lt9611uxc.c` uses `regmap_noinc_write` for the same pattern. `regmap_raw_write` will auto-increment the register address, which is incorrect for a FIFO. This should be `regmap_noinc_write`. The same issue applies to `lt7911exc_write_crc`: ```c ret = regmap_raw_write(lt7911exc->regmap, 0xe05d, crc, 4); ``` #### 3. `lt7911exc_prog_init` uses `u64` for a 24-bit address ```c static int lt7911exc_prog_init(struct lt7911exc *lt7911exc, u64 addr) ``` The address is masked to 24 bits (`addr >> 16 & 0xff`, etc.), so `u32` is sufficient and more appropriate. Similarly, `lt7911exc_write_data` and `lt7911exc_write_crc` use `u64 addr` — all should be `u32`. #### 4. `lt7911exc_write_data` takes `const struct firmware *` but only needs `data`/`size` The function signature couples it to the firmware API, yet after the CRC computation the padded buffer is freed and lost. If the intent is to always write the padded image (see issue 1), the function should take the buffer directly. #### 5. Redundant `dev_set_drvdata` and `i2c_set_clientdata` In `lt7911exc_probe`: ```c dev_set_drvdata(dev, lt7911exc); ... i2c_set_clientdata(client, lt7911exc); ``` For I2C devices, `i2c_set_clientdata(client, data)` calls `dev_set_drvdata(&client->dev, data)`, so these are redundant. Only `i2c_set_clientdata` is needed (it covers both the sysfs `dev_get_drvdata` path and the `i2c_get_clientdata` path in `remove`). The reference driver `lontium-lt8713sx.c` uses only `i2c_set_clientdata`. #### 6. Version read without MCU halt departs from established pattern ```c static int lt7911exc_read_version(struct lt7911exc *lt7911exc) { ... /* no need to halt MCU for this register access */ ret = regmap_bulk_read(lt7911exc->regmap, 0xe081, buf, ARRAY_SIZE(buf)); ``` The sister driver `lontium-lt9611uxc.c` always halts the MCU before reading the version register. The comment claims this is safe but provides no hardware rationale. If the register truly is readable without MCU halt, a brief explanation of why (e.g., "read-only status register not updated by MCU") would be valuable. #### 7. `lt7911exc_dsi_host_transfer` whitelist does nothing The transfer function accepts specific DSI write message types via a switch/case whitelist, but then unconditionally returns `msg->tx_len` without actually sending any data: ```c switch (msg->type) { case MIPI_DSI_DCS_SHORT_WRITE: ... break; default: return -EOPNOTSUPP; } ... return msg->tx_len; ``` The comment says the internal firmware handles DSI commands, so this is essentially a no-op stub. The whitelist filtering adds code without adding value — if nothing is actually transmitted, all types could be silently accepted (or all rejected). The current middle ground is misleading. A single-line comment like `/* firmware handles all panel init; silently accept writes */` before a direct `return msg->tx_len` would be clearer. #### 8. `lt7911exc_prog_init` redundant `return 0` ```c ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write)); if (ret) return ret; return 0; ``` This can be simplified to `return regmap_multi_reg_write(...)`. #### 9. Kconfig help text formatting ``` chip.The LT7911EXC converts eDP input to MIPI ``` Missing space after period: should be `chip. The LT7911EXC`. #### 10. Comment style Several C++-style comments remain: ```c //enable mipi stream //disable mipi stream //hardware requires delay ``` Kernel coding style requires `/* */` comments. These should be `/* enable mipi stream */` etc. #### 11. `lt7911exc_write_data` unchecked return values for partial page handling ```c if (page_len < LT_PAGE_SIZE) { regmap_write(lt7911exc->regmap, 0xe05f, 0x05); regmap_write(lt7911exc->regmap, 0xe05f, 0x01); ... } regmap_write(lt7911exc->regmap, 0xe05f, 0x00); ``` These `regmap_write` calls don't check return values, whereas other similar calls in the function do. Consistency would be good, even if errors here are unlikely. #### 12. `lt7911exc_write_crc` unchecked return values Same pattern — several `regmap_write` calls are not checked: ```c regmap_write(lt7911exc->regmap, 0xe05f, 0x01); regmap_write(lt7911exc->regmap, 0xe05a, (addr >> 16) & 0xff); ... ``` #### 13. `struct device_node *np` usage ```c struct device_node *np = dev->of_node; ... if (!np) return -ENODEV; ``` The `np` variable is only used for the NULL check and one assignment (`lt7911exc->bridge.of_node = np`). The OF-matching already guarantees `of_node` is non-NULL when probe runs, so the check is unnecessary. If kept, `dev->of_node` could be used directly without the local variable. #### 14. `lt7911exc_block_erase` redundantly halts MCU ```c static int lt7911exc_block_erase(struct lt7911exc *lt7911exc) { ... const struct reg_sequence seq_write[] = { REG_SEQ0(0xe0ee, 0x01), /* MCU halt */ ``` This function writes 0x01 to the MCU halt register (0xe0ee), but the caller (`lt7911exc_firmware_upgrade_work`) already calls `lt7911exc_hw_mcu_halt()` before `lt7911exc_block_erase()`. This is harmless (idempotent write) but suggests the register sequence was copied verbatim from a vendor reference without cleanup. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge
@ 2026-05-15 8:09 syyang
2026-05-15 8:09 ` [PATCH v8 2/2] drm/bridge: " syyang
0 siblings, 1 reply; 11+ messages in thread
From: syyang @ 2026-05-15 8:09 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, xmzhu, xmzhu, rlyu, xbpeng, Sunyun Yang
From: Sunyun Yang <syyang@lontium.com>
The LT7911EXC is an I2C-controlled bridge that receives eDP1.4
and output mipi dsi. 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 v8:
- dt-binding:
- drm/bridge:
1. Protect firmware upgrade and DRM bridge callback paths with ocm_lock. [sashiko-bot]
2. Remove the hardware reset from the remove callback, and ensure that
all hardware reset operations are protected by ocm_lock.
3. crc reconstruction explicitly casts each byte to u32 before shifting
4. The display configuration is handled by the firmware, and the MIPI
DSI host registration issue has been fixed.
5. The batch register read/write operations have already been updated
to include return value checking.
6. The dev_err_probe() used outside of probe context has been fixed.
- Link to v7: https://lore.kernel.org/lkml/20260512064013.40066-1-syyang@lontium.com/
Change in v7:
- dt-binding:
1. fix commit message typos(Receiver、signal) [sashiko-bot]
2. remove the ambiguity caused by "signal/dual".
- drm/bridge:
1. using devm_regulator_get_enable avoids power leaks. [sashiko-bot]
2. set reset gpio is low after cutting off power in lt7911exc_remove function, avoid backpowering.
3. synchronous request_firmware() call cause a permanent probe failure if the driver is built-in,
probe executes before the root filesystem is mounted, which would cause this to fail with -ENOENT,
we have removed this functionality. Use trigger to upgrade.
4. add `depends on I2C` and `select REGMAP_I2C` in Kconfig.
5. add return value of `devm_drm_bridge_add()` in `probe()`.
6. add directly header files (linux/slab.h, linux/delay.h, linux/regulator/consumer.h)
- Link to v6: https://lore.kernel.org/lkml/20260508134702.4713-1-syyang@lontium.com/
Change in v6:
- dt-binding:
- drm/bridge:
1. use #define FW_FILE "Lontium/lt7911exc_fw.bin" to match linux-firmware
- Link to v5: https://lore.kernel.org/lkml/20260506013153.2240-1-syyang@lontium.com/
Change in v5:
- dt-binding:
- drm/bridge:
1. Change "mipi" to "mipi dsi" in the commit message. [Dmitry]
2. Change "eDP/MIPI" to "eDP/MIPI DSI" in Kconfig.
- Link to v4: https://lore.kernel.org/lkml/20260430094612.3408174-1-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 | 15 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/lontium-lt7911exc.c | 642 ++++++++++++++++++
4 files changed, 747 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] 11+ messages in thread* [PATCH v8 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-15 8:09 [PATCH v8 0/2] " syyang @ 2026-05-15 8:09 ` syyang 2026-05-15 23:43 ` Claude review: " Claude Code Review Bot 0 siblings, 1 reply; 11+ messages in thread From: syyang @ 2026-05-15 8:09 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, xmzhu, xmzhu, rlyu, xbpeng, Sunyun Yang From: Sunyun Yang <syyang@lontium.com> Add support for the Lontium LT7911EXC bridge chip, which converts eDP input to MIPI DSI output using an internal firmware-controlled pipeline. The driver provides: - DRM bridge integration for eDP-to-DSI routing - MIPI DSI host interface for downstream panel attachment - Firmware upgrade mechanism over I2C (erase/program/verify) - GPIO-based reset and regulator management - Basic register access protection using a mutex-controlled OCM lock Display timing and DSI packet generation are handled by the chip firmware and are not configured by the driver. Signed-off-by: Sunyun Yang <syyang@lontium.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- drivers/gpu/drm/bridge/Kconfig | 15 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/lontium-lt7911exc.c | 642 +++++++++++++++++++++ 3 files changed, 658 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..cb74730c6ef4 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -132,6 +132,21 @@ config DRM_ITE_IT6505 help ITE IT6505 DisplayPort bridge chip driver. +config DRM_LONTIUM_LT7911EXC + tristate "Lontium eDP/MIPI DSI bridge" + depends on OF + depends on I2C + select CRC32 + select FW_LOADER + select DRM_PANEL + select DRM_KMS_HELPER + select REGMAP_I2C + help + DRM driver for the Lontium LT7911EXC bridge + chip.The LT7911EXC converts eDP input to 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..e7b8a22b2e29 --- /dev/null +++ b/drivers/gpu/drm/bridge/lontium-lt7911exc.c @@ -0,0 +1,642 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2026 Lontium Semiconductor, Inc. + */ + +#include <linux/crc32.h> +#include <linux/delay.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 <linux/regulator/consumer.h> +#include <linux/slab.h> + +#include <drm/drm_atomic_helper.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 "Lontium/lt7911exc_fw.bin" +#define LT7911EXC_PAGE_CONTROL 0xff + +struct lt7911exc_dsi_output { + struct mipi_dsi_device *dev; + struct drm_panel *panel; + struct drm_bridge *bridge; + }; + +struct lt7911exc { + struct device *dev; + struct i2c_client *client; + struct drm_bridge bridge; + + struct mipi_dsi_host dsi_host; + struct lt7911exc_dsi_output output; + + struct regmap *regmap; + /* Protects all accesses to registers by stopping the on-chip MCU */ + struct mutex ocm_lock; + 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 inline struct lt7911exc *dsi_host_to_lt7911exc(struct mipi_dsi_host *host) +{ + return container_of(host, struct lt7911exc, dsi_host); +} + +/* + * Purpose of this function is rest gpio: high -> low -> high + * This clears the previous configuration in the chip, + * and finally remains high to allow the firmware to run again. + */ +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_regulator_enable(struct lt7911exc *lt7911exc) +{ + int ret; + + ret = devm_regulator_get_enable(lt7911exc->dev, "vcc"); + if (ret < 0) + return dev_err_probe(lt7911exc->dev, ret, "failed to enable vcc regulator\n"); + + usleep_range(5000, 10000); + + ret = devm_regulator_get_enable(lt7911exc->dev, "vdd"); + if (ret < 0) + return dev_err_probe(lt7911exc->dev, ret, "failed to enable vdd regulator\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, ARRAY_SIZE(buf)); + if (ret) + return ret; + + return (buf[0] << 16) | (buf[1] << 8) | buf[2]; +} + +static void lt7911exc_lock(struct lt7911exc *lt7911exc) +{ + mutex_lock(<7911exc->ocm_lock); + regmap_write(lt7911exc->regmap, 0xe0ee, 0x01); +} + +static void lt7911exc_unlock(struct lt7911exc *lt7911exc) +{ + regmap_write(lt7911exc->regmap, 0xe0ee, 0x00); + mutex_unlock(<7911exc->ocm_lock); +} + +static int lt7911exc_block_erase(struct lt7911exc *lt7911exc) +{ + struct device *dev = lt7911exc->dev; + const u32 addr = 0x00; + int ret; + + 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), + }; + + ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write)); + if (ret) + return ret; + + msleep(200); + dev_dbg(dev, "erase flash done.\n"); + + return 0; +} + +static int lt7911exc_prog_init(struct lt7911exc *lt7911exc, u64 addr) +{ + int ret; + + 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), + }; + + ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write)); + if (ret) + return ret; + + return 0; +} + +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); + ret = lt7911exc_prog_init(lt7911exc, addr); + if (ret) + return ret; + + 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, ARRAY_SIZE(crc_tmp)); + if (ret) { + dev_err(lt7911exc->dev, "Failed to read CRC: %d\n", ret); + return ret; + } + + read_hw_crc = ((u32)crc_tmp[0] << 24) | ((u32)crc_tmp[1] << 16) | + ((u32)crc_tmp[2] << 8) | ((u32)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) { + dev_err(dev, "failed to load '%s'\n", FW_FILE); + return ret; + } + + /* 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); + + ret = lt7911exc_block_erase(lt7911exc); + if (ret) { + dev_err(dev, "failed to block erase.\n"); + goto out_release_fw; + } + + 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); + + guard(mutex)(<7911exc->ocm_lock); + + 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); + + guard(mutex)(<7911exc->ocm_lock); + + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1); +} + +static int lt7911exc_bridge_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(encoder, lt7911exc->output.bridge, bridge, flags); +} + +static const struct drm_bridge_funcs lt7911exc_bridge_funcs = { + .attach = lt7911exc_bridge_attach, + .atomic_pre_enable = lt7911exc_atomic_pre_enable, + .atomic_disable = lt7911exc_atomic_disable, + .atomic_post_disable = lt7911exc_atomic_post_disable, + .atomic_reset = drm_atomic_helper_bridge_reset, + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, +}; + +static int lt7911exc_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + struct drm_bridge *bridge; + struct drm_panel *panel; + int ret; + + ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0, &panel, &bridge); + if (ret) + return ret; + + if (panel) { + bridge = drm_panel_bridge_add_typed(panel, DRM_MODE_CONNECTOR_DSI); + if (IS_ERR(bridge)) + return PTR_ERR(bridge); + } + lt7911exc->output.dev = dev; + lt7911exc->output.bridge = bridge; + lt7911exc->output.panel = panel; + + drm_bridge_add(<7911exc->bridge); + return 0; +} + +static int lt7911exc_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + + guard(mutex)(<7911exc->ocm_lock); + + drm_bridge_remove(<7911exc->bridge); + if (lt7911exc->output.panel) + drm_panel_bridge_remove(lt7911exc->output.bridge); + + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1); + + return 0; +} + +/* + * The firmware running inside the LT7911EXC chip + * sends display commands configuration and cannot + * be configured by an external driver. + */ +static ssize_t lt7911exc_dsi_host_transfer(struct mipi_dsi_host *host, + const struct mipi_dsi_msg *msg) +{ + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host); + unsigned int val; + int ret; + + guard(mutex)(<7911exc->ocm_lock); + + /* ensure eDP input video ready */ + ret = regmap_read_poll_timeout(lt7911exc->regmap, 0xe084, val, (val & 0x01), 5000, 50000); + if (ret) { + dev_warn(lt7911exc->dev, "Video source not ready within 50ms\n"); + return -EBUSY; + } + + /* enable mipi dsi output stream and firmware will send dcs */ + ret = regmap_write(lt7911exc->regmap, 0xe0b0, 0x01); + if (ret) + return ret; + + return msg->tx_len; +} + +static const struct mipi_dsi_host_ops lt7911exc_dsi_host_ops = { + .attach = lt7911exc_dsi_host_attach, + .detach = lt7911exc_dsi_host_detach, + .transfer = lt7911exc_dsi_host_transfer, +}; + +static ssize_t lt7911exc_firmware_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t len) +{ + struct lt7911exc *lt7911exc = dev_get_drvdata(dev); + int ret; + + lt7911exc_lock(lt7911exc); + + /* 0 - ensure chip run */ + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 0); + msleep(400); + + ret = lt7911exc_firmware_upgrade(lt7911exc); + if (ret < 0) + dev_err(dev, "upgrade failure\n"); + + lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); + + lt7911exc_unlock(lt7911exc); + + return ret < 0 ? ret : len; +} + +static ssize_t lt7911exc_firmware_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct lt7911exc *lt7911exc = dev_get_drvdata(dev); + + return sysfs_emit(buf, "0x%04x\n", lt7911exc->fw_version); +} + +static DEVICE_ATTR_RW(lt7911exc_firmware); + +static struct attribute *lt7911exc_attrs[] = { + &dev_attr_lt7911exc_firmware.attr, + NULL, +}; + +static const struct attribute_group lt7911exc_attr_group = { + .attrs = lt7911exc_attrs, +}; + +static const struct attribute_group *lt7911exc_attr_groups[] = { + <7911exc_attr_group, + NULL, +}; + +static int lt7911exc_probe(struct i2c_client *client) +{ + struct lt7911exc *lt7911exc; + struct device *dev = &client->dev; + struct device_node *np = dev->of_node; + int ret; + + if (!np) + return -ENODEV; + + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) + return dev_err_probe(dev, -ENODEV, "device doesn't support I2C\n"); + + lt7911exc = devm_drm_bridge_alloc(dev, struct lt7911exc, bridge, <7911exc_bridge_funcs); + if (IS_ERR(lt7911exc)) + return dev_err_probe(dev, PTR_ERR(lt7911exc), "drm bridge alloc failed.\n"); + + dev_set_drvdata(dev, lt7911exc); + + lt7911exc->client = client; + lt7911exc->dev = dev; + + ret = devm_mutex_init(dev, <7911exc->ocm_lock); + if (ret) + return dev_err_probe(dev, ret, "failed to init mutex\n"); + + lt7911exc->regmap = devm_regmap_init_i2c(client, <7911exc_regmap_config); + if (IS_ERR(lt7911exc->regmap)) + return dev_err_probe(dev, PTR_ERR(lt7911exc->regmap), "regmap i2c init failed\n"); + + /* + * reset GPIO is defined as active low in device tree. + * gpiod_set_value_cansleep() uses logical value: + * 1 -> asserted (active) -> physical low -> reset enabled -> chip stop + * 0 -> deasserted (inactive) -> physical high -> reset released -> chip run + */ + 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"); + + ret = lt7911exc_regulator_enable(lt7911exc); + if (ret) + return ret; + + lt7911exc_reset(lt7911exc); + + lt7911exc_lock(lt7911exc); + + lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); + + lt7911exc_unlock(lt7911exc); + + if (lt7911exc->fw_version < 0) + return dev_err_probe(dev, lt7911exc->fw_version, "failed read version of chip\n"); + + lt7911exc->dsi_host.dev = dev; + lt7911exc->dsi_host.ops = <7911exc_dsi_host_ops; + lt7911exc->bridge.of_node = np; + + i2c_set_clientdata(client, lt7911exc); + + return mipi_dsi_host_register(<7911exc->dsi_host); +} + +static void lt7911exc_remove(struct i2c_client *client) +{ + struct lt7911exc *lt7911exc = i2c_get_clientdata(client); + + mipi_dsi_host_unregister(<7911exc->dsi_host); +} + +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, + .remove = lt7911exc_remove, + .driver = { + .name = "lt7911exc", + .of_match_table = lt7911exc_devices, + .dev_groups = lt7911exc_attr_groups, + }, +}; +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] 11+ messages in thread
* Claude review: drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-15 8:09 ` [PATCH v8 2/2] drm/bridge: " syyang @ 2026-05-15 23:43 ` Claude Code Review Bot 0 siblings, 0 replies; 11+ messages in thread From: Claude Code Review Bot @ 2026-05-15 23:43 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Kconfig issues:** 1. Missing space in help text: ```c chip.The LT7911EXC converts eDP input to MIPI ``` Should be `chip. The LT7911EXC...`. 2. Consider adding `select DRM_MIPI_DSI` — the driver implements a `mipi_dsi_host` and uses MIPI DSI APIs. **Struct formatting:** 3. Inconsistent closing brace indentation in `lt7911exc_dsi_output`: ```c struct lt7911exc_dsi_output { struct mipi_dsi_device *dev; struct drm_panel *panel; struct drm_bridge *bridge; }; ``` The `};` has an extra tab. Should be flush left. **`fw_version` sysfs display bug:** 4. `lt7911exc_read_version` returns a 24-bit value: ```c return (buf[0] << 16) | (buf[1] << 8) | buf[2]; ``` But `lt7911exc_firmware_show` formats it as: ```c return sysfs_emit(buf, "0x%04x\n", lt7911exc->fw_version); ``` `%04x` pads to 4 hex digits minimum but a 24-bit version (e.g., `0x010203`) needs 6 hex digits. Use `"0x%06x\n"` to match the 3-byte version, or `"0x%x\n"` if there's no fixed width convention. **Unnecessary `u64` types:** 5. Several functions use `u64` for addresses and sizes that are at most 64KB: ```c static u32 cal_crc32_custom(const u8 *data, u64 length) static int lt7911exc_prog_init(struct lt7911exc *lt7911exc, u64 addr) static int lt7911exc_write_data(struct lt7911exc *lt7911exc, const struct firmware *fw, u64 addr) ``` These should be `size_t` for lengths and `u32` for addresses. Firmware is capped at `FW_SIZE` (64KB), so `u64` is misleading. This also causes mixed-type arithmetic in `lt7911exc_write_data`: ```c u64 size, offset; ... page = (size + LT_PAGE_SIZE - 1) / LT_PAGE_SIZE; ``` Here `page` is `int` but the RHS is `u64`, resulting in silent truncation. **`cal_crc32_custom` fragility:** 6. The function accesses `data[i+3]` without checking that `length` is a multiple of 4: ```c for (i = 0; i < length; i += 4) { buf[0] = data[i + 3]; ``` Currently all callers pass `FW_SIZE - 4 = 65532` which is divisible by 4, so there's no actual bug. But the function is fragile — consider adding a `WARN_ON(length % 4)` or a comment documenting the requirement. **Unchecked `regmap_write` return values:** 7. Several `regmap_write` calls in `lt7911exc_write_crc` ignore return values: ```c 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); ``` Same issue in `lt7911exc_write_data` for the short-page handling path: ```c if (page_len < LT_PAGE_SIZE) { regmap_write(lt7911exc->regmap, 0xe05f, 0x05); regmap_write(lt7911exc->regmap, 0xe05f, 0x01); ``` And in `lt7911exc_upgrade_result`: ```c regmap_write(lt7911exc->regmap, 0xe0ee, 0x01); regmap_write(lt7911exc->regmap, 0xe07b, 0x60); regmap_write(lt7911exc->regmap, 0xe07b, 0x40); ``` These should check return values or at minimum use `regmap_multi_reg_write` as done in the erase function for consistency. **`lt7911exc_lock`/`lt7911exc_unlock` vs. internal OCM register writes:** 8. `lt7911exc_lock` writes `0xe0ee = 0x01` to stop the on-chip MCU, but `lt7911exc_block_erase` and `lt7911exc_prog_init` also write `0xe0ee = 0x01` via their `seq_write` arrays: ```c const struct reg_sequence seq_write[] = { REG_SEQ0(0xe0ee, 0x01), /* redundant when caller holds lock */ REG_SEQ0(0xe054, 0x01), ``` This is harmless but confusing — the OCM lock is managed at two different layers. Consider removing the redundant `0xe0ee` writes from the firmware functions since the caller always holds the lock, or document that these functions must not be called without the lock held. **Redundant `dev_set_drvdata` / `i2c_set_clientdata`:** 9. In probe: ```c dev_set_drvdata(dev, lt7911exc); ... i2c_set_clientdata(client, lt7911exc); ``` `i2c_set_clientdata` calls `dev_set_drvdata(&client->dev, data)` internally, so this is redundant. One call to `i2c_set_clientdata` is sufficient (and is what `lt7911exc_remove` uses to retrieve it). **Comment style:** 10. Missing spaces after `/*` in comments: ```c /*3. calculate crc32 */ /*4. firmware upgrade */ /*5. check upgrade of result */ ``` Should be `/* 3.` etc. **`lt7911exc_prog_init` unnecessary early return:** 11. The function ends with: ```c ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write)); if (ret) return ret; return 0; ``` This is just `return regmap_multi_reg_write(...)`. **Firmware upgrade safety:** 12. `lt7911exc_firmware_store` can be triggered while the display pipeline is active. The function takes `ocm_lock`, resets the chip, erases flash, and reprograms firmware — all while a display may be running. After the upgrade completes and the lock is released, the bridge is in an undefined state relative to the DRM pipeline. There's no check for whether the bridge is currently attached/enabled. Other drivers with similar sysfs firmware interfaces (lt9611uxc) have the same issue, so this may be accepted practice, but it's worth noting. **`lt7911exc_dsi_host_transfer` semantics:** 13. The transfer function doesn't actually forward DSI messages — it waits for video input and enables DSI output: ```c ret = regmap_read_poll_timeout(lt7911exc->regmap, 0xe084, val, (val & 0x01), 5000, 50000); ... ret = regmap_write(lt7911exc->regmap, 0xe0b0, 0x01); ... return msg->tx_len; ``` Every DSI transfer from the panel driver will re-poll the video-ready register (up to 50ms) and re-write the output enable register. If a panel driver sends multiple DCS init commands, this could add significant latency. Is the intent that only a single transfer ever happens (i.e., the panel driver sends exactly one DCS command to trigger output)? If so, this should be documented. If not, the enable should happen once and subsequent transfers should be no-ops. **Missing `atomic_enable` callback:** 14. The bridge provides `atomic_pre_enable` and `atomic_disable`/`atomic_post_disable` but no `atomic_enable`. The enable logic appears to be implicitly handled through the DSI host transfer mechanism, but this coupling between bridge lifecycle and DSI host transfer is unusual and could use a comment explaining the design. **Minor: `#include <linux/platform_device.h>`:** 15. This is an I2C driver, not a platform driver. The `platform_device.h` include appears unused. **Minor: `np` check:** 16. In probe: ```c if (!np) return -ENODEV; ``` The driver has `depends on OF` in Kconfig and the `of_match_table` is set, so `of_node` will always be non-NULL when probe is called via DT matching. This check is unnecessary. **Summary:** The driver structure is sound and follows existing patterns (especially the tc358768 model for DSI-host bridges). The main items to address are: the sysfs version format bug (#4), unchecked regmap return values (#7), the u64/type-width cleanup (#5), and the struct indentation (#3). The rest are minor improvements and style points. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v7 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge
@ 2026-05-12 6:40 syyang
2026-05-12 6:40 ` [PATCH v7 2/2] drm/bridge: " syyang
0 siblings, 1 reply; 11+ messages in thread
From: syyang @ 2026-05-12 6:40 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, xmzhu, xmzhu, rlyu, xbpeng, Sunyun Yang
From: Sunyun Yang <syyang@lontium.com>
The LT7911EXC is an I2C-controlled bridge that receives eDP1.4
and output mipi dsi. 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 v7:
- dt-binding:
1. fix commit message typos(Receiver、signal) [sashiko-bot]
2. remove the ambiguity caused by "signal/dual".
- drm/bridge:
1. using devm_regulator_get_enable avoids power leaks. [sashiko-bot]
2. set reset gpio is low after cutting off power in lt7911exc_remove function, avoid backpowering.
3. synchronous request_firmware() call cause a permanent probe failure if the driver is built-in,
probe executes before the root filesystem is mounted, which would cause this to fail with -ENOENT,
we have removed this functionality. Use trigger to upgrade.
4. add `depends on I2C` and `select REGMAP_I2C` in Kconfig.
5. add return value of `devm_drm_bridge_add()` in `probe()`.
6. add directly header files (linux/slab.h, linux/delay.h, linux/regulator/consumer.h)
- Link to v6: https://lore.kernel.org/lkml/20260508134702.4713-1-syyang@lontium.com/
Change in v6:
- dt-binding:
- drm/bridge:
1. use #define FW_FILE "Lontium/lt7911exc_fw.bin" to match linux-firmware
- Link to v5: https://lore.kernel.org/lkml/20260506013153.2240-1-syyang@lontium.com/
Change in v5:
- dt-binding:
- drm/bridge:
1. Change "mipi" to "mipi dsi" in the commit message. [Dmitry]
2. Change "eDP/MIPI" to "eDP/MIPI DSI" in Kconfig.
- Link to v4: https://lore.kernel.org/lkml/20260430094612.3408174-1-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 | 15 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/lontium-lt7911exc.c | 514 ++++++++++++++++++
4 files changed, 619 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] 11+ messages in thread* [PATCH v7 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-12 6:40 [PATCH v7 0/2] " syyang @ 2026-05-12 6:40 ` syyang 2026-05-16 4:16 ` Claude review: " Claude Code Review Bot 0 siblings, 1 reply; 11+ messages in thread From: syyang @ 2026-05-12 6:40 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, xmzhu, xmzhu, rlyu, xbpeng, Sunyun Yang From: Sunyun Yang <syyang@lontium.com> This commit adds support for the Lontium LT7911EXC eDP to MIPI DSI bridge. It implements I2C-based setup, display configuration, and provides a firmware update mechanism. Signed-off-by: Sunyun Yang <syyang@lontium.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- drivers/gpu/drm/bridge/Kconfig | 15 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/lontium-lt7911exc.c | 514 +++++++++++++++++++++ 3 files changed, 530 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..cb74730c6ef4 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -132,6 +132,21 @@ config DRM_ITE_IT6505 help ITE IT6505 DisplayPort bridge chip driver. +config DRM_LONTIUM_LT7911EXC + tristate "Lontium eDP/MIPI DSI bridge" + depends on OF + depends on I2C + select CRC32 + select FW_LOADER + select DRM_PANEL + select DRM_KMS_HELPER + select REGMAP_I2C + help + DRM driver for the Lontium LT7911EXC bridge + chip.The LT7911EXC converts eDP input to 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..ccea435b3c7b --- /dev/null +++ b/drivers/gpu/drm/bridge/lontium-lt7911exc.c @@ -0,0 +1,514 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2026 Lontium Semiconductor, Inc. + */ + +#include <linux/crc32.h> +#include <linux/delay.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 <linux/regulator/consumer.h> +#include <linux/slab.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 "Lontium/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 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_regulator_enable(struct lt7911exc *lt7911exc) +{ + int ret; + + ret = devm_regulator_get_enable(lt7911exc->dev, "vcc"); + if (ret < 0) + return dev_err_probe(lt7911exc->dev, ret, "failed to enable vcc regulator\n"); + + usleep_range(5000, 10000); + + ret = devm_regulator_get_enable(lt7911exc->dev, "vdd"); + if (ret < 0) + return dev_err_probe(lt7911exc->dev, ret, "failed to enable vdd regulator\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, ARRAY_SIZE(buf)); + if (ret) + return ret; + + return (buf[0] << 16) | (buf[1] << 8) | buf[2]; +} + +static void lt7911exc_lock(struct lt7911exc *lt7911exc) +{ + mutex_lock(<7911exc->ocm_lock); + regmap_write(lt7911exc->regmap, 0xe0ee, 0x01); +} + +static void lt7911exc_unlock(struct lt7911exc *lt7911exc) +{ + regmap_write(lt7911exc->regmap, 0xe0ee, 0x00); + mutex_unlock(<7911exc->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, ARRAY_SIZE(crc_tmp)); + 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); + + 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); + + /* The reset GPIO is defined in device tree with GPIO_ACTIVE_LOW flag. + * Set 1 means reset signal. + */ + 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, + <7911exc->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 ssize_t lt7911exc_firmware_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t len) +{ + struct lt7911exc *lt7911exc = dev_get_drvdata(dev); + int ret; + + /* set 0: release reset signal */ + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 0); + msleep(400); + + lt7911exc_lock(lt7911exc); + + ret = lt7911exc_firmware_upgrade(lt7911exc); + if (ret < 0) + dev_err(dev, "upgrade failure\n"); + + lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); + + lt7911exc_unlock(lt7911exc); + + return ret < 0 ? ret : len; +} + +static ssize_t lt7911exc_firmware_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct lt7911exc *lt7911exc = dev_get_drvdata(dev); + + return sysfs_emit(buf, "0x%04x\n", lt7911exc->fw_version); +} + +static DEVICE_ATTR_RW(lt7911exc_firmware); + +static struct attribute *lt7911exc_attrs[] = { + &dev_attr_lt7911exc_firmware.attr, + NULL, +}; + +static const struct attribute_group lt7911exc_attr_group = { + .attrs = lt7911exc_attrs, +}; + +static const struct attribute_group *lt7911exc_attr_groups[] = { + <7911exc_attr_group, + NULL, +}; + +static int lt7911exc_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct lt7911exc *lt7911exc; + int ret; + + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) + return dev_err_probe(dev, -ENODEV, "device doesn't support I2C\n"); + + lt7911exc = devm_drm_bridge_alloc(dev, struct lt7911exc, bridge, <7911exc_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, <7911exc->ocm_lock); + if (ret) + return dev_err_probe(dev, ret, "failed to init mutex\n"); + + lt7911exc->regmap = devm_regmap_init_i2c(client, <7911exc_regmap_config); + if (IS_ERR(lt7911exc->regmap)) + return dev_err_probe(dev, PTR_ERR(lt7911exc->regmap), "regmap i2c init failed\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"); + + ret = lt7911exc_regulator_enable(lt7911exc); + if (ret) + return ret; + + lt7911exc_reset(lt7911exc); + + lt7911exc_lock(lt7911exc); + + lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); + + lt7911exc_unlock(lt7911exc); + if (lt7911exc->fw_version < 0) + return dev_err_probe(dev, lt7911exc->fw_version, "failed read version of chip\n"); + + lt7911exc->bridge.of_node = dev->of_node; + + return devm_drm_bridge_add(dev, <7911exc->bridge); +} + +static void lt7911exc_remove(struct i2c_client *client) +{ + struct lt7911exc *lt7911exc = i2c_get_clientdata(client); + + /* pull reset low to prevent backpowering*/ + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1); +} + +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, + .remove = lt7911exc_remove, + .driver = { + .name = "lt7911exc", + .of_match_table = lt7911exc_devices, + .dev_groups = lt7911exc_attr_groups, + }, +}; +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] 11+ messages in thread
* Claude review: drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge 2026-05-12 6:40 ` [PATCH v7 2/2] drm/bridge: " syyang @ 2026-05-16 4:16 ` Claude Code Review Bot 0 siblings, 0 replies; 11+ messages in thread From: Claude Code Review Bot @ 2026-05-16 4:16 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Critical:** 1. **Missing atomic state management callbacks.** The driver uses `atomic_pre_enable`, `atomic_disable`, and `atomic_post_disable` but does not provide `atomic_duplicate_state`, `atomic_destroy_state`, or `atomic_reset`. The kernel documentation in `drm_bridge.c` explicitly states: > Bridge drivers may implement the legacy version of those operations, or the atomic version (prefixed with atomic_), in which case they shall also implement the atomic state bookkeeping operations (atomic_duplicate_state, atomic_destroy_state and reset). Without `atomic_reset`, `drm_bridge_is_atomic()` returns false and `drm_atomic_private_obj_init()` is never called during `drm_bridge_attach()`. Every other bridge driver in the tree using atomic callbacks provides these. The fix is to add the three helper functions, exactly as the sibling `lontium-lt9211.c` does: ```c .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, ``` **Medium:** 2. **`lt7911exc_attach` uses `lt7911exc->bridge.encoder` instead of the `encoder` parameter.** While functionally equivalent (since `drm_bridge_attach` sets `bridge->encoder` before calling the callback), it's non-idiomatic and fragile: ```c return drm_bridge_attach(lt7911exc->bridge.encoder, lt7911exc->bridge.next_bridge, <7911exc->bridge, flags); ``` Should use `encoder` directly: ```c return drm_bridge_attach(encoder, lt7911exc->bridge.next_bridge, <7911exc->bridge, flags); ``` 3. **`lt7911exc_firmware_store` does not validate input.** The sysfs store callback ignores the buffer content entirely -- any write triggers a firmware upgrade. It should check for a specific trigger value (e.g., `"1\n"`) to prevent accidental upgrades. 4. **`lt7911exc_firmware_store` does not check `lt7911exc_read_version()` return value.** After firmware upgrade, the version is read but the error code is silently stored in `fw_version`: ```c lt7911exc->fw_version = lt7911exc_read_version(lt7911exc); ``` If `lt7911exc_read_version` returns a negative error, subsequent reads from the `show` function would display a garbage version. 5. **`cal_crc32_custom` is unsafe for lengths not a multiple of 4.** The loop accesses `data[i+3]` without bounds checking. While the current caller always passes a length divisible by 4, the function itself is fragile. Either add a length check/assertion or document the requirement. 6. **Many register write return values are silently ignored.** `lt7911exc_block_erase`, `lt7911exc_prog_init`, `lt7911exc_lock`, `lt7911exc_unlock`, and `lt7911exc_write_crc` all ignore `regmap_write` / `regmap_multi_reg_write` return values. At minimum, `lt7911exc_block_erase` and `lt7911exc_prog_init` should propagate errors since they're part of the firmware upgrade path. **Minor:** 7. **Kconfig help text formatting.** Missing space after period: ``` chip.The LT7911EXC converts eDP input to MIPI ``` Should be `chip. The LT7911EXC`. 8. **Unused include.** `#include <linux/platform_device.h>` is included but the driver is an I2C driver and never uses any platform_device APIs. 9. **Sysfs attribute naming.** `lt7911exc_firmware` embeds the driver name in the attribute name, which is unusual. Convention is a simple descriptive name like `firmware_version` (for the show) and `firmware_update` (for the store), or split into two attributes. 10. **`fw_version` format specifier.** `sysfs_emit(buf, "0x%04x\n", lt7911exc->fw_version)` uses `%04x` (4 hex digits, 16 bits), but the version is constructed from 3 bytes (24 bits). Should be `%06x` to consistently zero-pad the full version. 11. **`u64` used where `u32` suffices.** The `addr` parameter in `lt7911exc_prog_init`, `lt7911exc_write_data`, and `lt7911exc_write_crc` is `u64` but only 24 bits are ever used (3 bytes extracted). `u32` would be more appropriate. 12. **`lt7911exc_atomic_disable` is a bare delay.** The callback contains only `msleep(20)` with no hardware interaction. If this delay is truly needed, a comment explaining *why* (e.g., panel timing requirement) would help reviewers. If it's not needed, it should be removed. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 11+ messages in thread
* [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 2/2] drm/bridge: " syyang 0 siblings, 1 reply; 11+ 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] 11+ 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] " syyang @ 2026-04-30 9:46 ` syyang 2026-05-05 0:47 ` Claude review: " Claude Code Review Bot 0 siblings, 1 reply; 11+ 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(<7911exc->ocm_lock); + regmap_write(lt7911exc->regmap, 0xe0ee, 0x01); +} + +static void lt7911exc_unlock(struct lt7911exc *lt7911exc) +{ + regmap_write(lt7911exc->regmap, 0xe0ee, 0x00); + mutex_unlock(<7911exc->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, + <7911exc->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, + <7911exc_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, <7911exc->ocm_lock); + if (ret) + return dev_err_probe(dev, ret, "failed to init mutex\n"); + + lt7911exc->regmap = devm_regmap_init_i2c(client, <7911exc_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, <7911exc->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] 11+ 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; 11+ 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, <7911exc->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] 11+ messages in thread
end of thread, other threads:[~2026-05-25 12:53 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-25 1:05 [PATCH v12 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge syyang 2026-05-25 1:05 ` [PATCH v12 1/2] dt-bindings: bridge: " syyang 2026-05-25 6:53 ` Claude review: " Claude Code Review Bot 2026-05-25 1:05 ` [PATCH v12 2/2] drm/bridge: " syyang 2026-05-25 6:53 ` Claude review: " Claude Code Review Bot 2026-05-25 6:53 ` Claude review: " Claude Code Review Bot -- strict thread matches above, loose matches on Subject: below -- 2026-05-22 1:57 [PATCH v11 0/2] " syyang 2026-05-22 1:57 ` [PATCH v11 2/2] drm/bridge: " syyang 2026-05-25 9:24 ` Claude review: " Claude Code Review Bot 2026-05-19 13:58 [PATCH v10 0/2] " syyang 2026-05-19 13:58 ` [PATCH v10 2/2] drm/bridge: " syyang 2026-05-25 12:53 ` Claude review: " Claude Code Review Bot 2026-05-15 8:09 [PATCH v8 0/2] " syyang 2026-05-15 8:09 ` [PATCH v8 2/2] drm/bridge: " syyang 2026-05-15 23:43 ` Claude review: " Claude Code Review Bot 2026-05-12 6:40 [PATCH v7 0/2] " syyang 2026-05-12 6:40 ` [PATCH v7 2/2] drm/bridge: " syyang 2026-05-16 4:16 ` Claude review: " Claude Code Review Bot 2026-04-30 9:46 [PATCH v4 0/2] " syyang 2026-04-30 9:46 ` [PATCH v4 2/2] drm/bridge: " syyang 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