Re: [PATCH v3 2/2] drm/bridge: add support for lontium lt8713sx bridge driver
From: Vishnu Saini
Date: Fri Feb 20 2026 - 04:03:21 EST
On Sun, Dec 28, 2025 at 05:01:05PM +0200, Dmitry Baryshkov wrote:
> On Sun, Dec 28, 2025 at 05:10:40PM +0530, Vishnu Saini wrote:
> > The lt8713sx is a Type-C/DP1.4 to Type-C/DP1.4/HDMI2.0 converter,
> > with three configurable DP1.4/HDMI2.0/DP++ output interfaces and
> > audio output interface.
> >
> > Driver is required for firmware upgrade in the bridge chip.
>
> Or for enabling the bridge chip. Or for building the bridge chains
> inside Linux kernel...
>
> Also, do we need any special handling for DP++ / HDMI devices being
> attached to the bridge?
Needed to add minimal handling of drm_bridge in the driver when ports
and remote endpoints added in the dt.
The V1 patch was also working where drm_bridge was not added in driver
and ports remote endpoints not defined in the dt.
> >
> > Co-developed-by: Prahlad Valluru <vvalluru@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Prahlad Valluru <vvalluru@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Vishnu Saini <vishnu.saini@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/bridge/Kconfig | 10 +
> > drivers/gpu/drm/bridge/Makefile | 1 +
> > drivers/gpu/drm/bridge/lontium-lt8713sx.c | 682 ++++++++++++++++++++++++++++++
> > 3 files changed, 693 insertions(+)
> >
> > +
> > +#define FW_FILE "lt8713sx_fw.bin"
>
> I'm still looking forward to seeing this file in linux-firmware.
raised a patch in linux-firmware: https://lore.kernel.org/all/20260217-lt8713sx_firmware-v1-1-f78b85fceb63@xxxxxxxxxxxxxxxx/
> > +
> > +#define REG_PAGE_CONTROL 0xff
> > +
> > +#define MAX_OUTPUT_PORTS 3
> > +#define LT8713SX_PAGE_SIZE 256
> > +
> > +DECLARE_CRC8_TABLE(lt8713sx_crc_table);
> > +
> > +struct lt8713sx {
> > + struct device *dev;
> > + struct drm_bridge bridge;
> > + struct drm_bridge *next_bridge[MAX_OUTPUT_PORTS];
> > + int num_outputs;
> > +
> > + struct regmap *regmap;
> > + /* Protects all accesses to registers by stopping the on-chip MCU */
> > + struct mutex ocm_lock;
> > +
> > + struct gpio_desc *reset_gpio;
> > + struct gpio_desc *enable_gpio;
> > +
> > + struct i2c_client *client;
> > + const struct firmware *fw;
> > +
> > + u8 *fw_buffer;
> > +
> > + u32 main_crc_value;
> > + u32 bank_crc_value[17];
> > +
> > + int bank_num;
> > +};
> > +
> > +static void lt8713sx_reset(struct lt8713sx *lt8713sx);
> > +
> > +static const struct regmap_range lt8713sx_ranges[] = {
> > + {
> > + .range_min = 0x0000,
> > + .range_max = 0xffff
> > + },
> > +};
> > +
> > +static const struct regmap_access_table lt8713sx_table = {
> > + .yes_ranges = lt8713sx_ranges,
> > + .n_yes_ranges = ARRAY_SIZE(lt8713sx_ranges),
> > +};
> > +
> > +static const struct regmap_range_cfg lt8713sx_range_cfg = {
> > + .name = "lt8713sx",
> > + .range_min = 0x0000,
> > + .range_max = 0xffff,
> > + .selector_reg = REG_PAGE_CONTROL,
> > + .selector_mask = 0xff,
> > + .selector_shift = 0,
> > + .window_start = 0,
> > + .window_len = 0x100,
> > +};
> > +
> > +static const struct regmap_config lt8713sx_regmap_config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > + .volatile_table = <8713sx_table,
> > + .ranges = <8713sx_range_cfg,
> > + .num_ranges = 1,
> > + .cache_type = REGCACHE_NONE,
> > + .max_register = 0xffff,
> > +};
> > +
> > +static void lt8713sx_i2c_enable(struct lt8713sx *lt8713sx)
> > +{
> > + regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> > +}
> > +
> > +static void lt8713sx_i2c_disable(struct lt8713sx *lt8713sx)
> > +{
> > + regmap_write(lt8713sx->regmap, 0xe0ee, 0x00);
> > +}
> > +
> > +static u32 calculate_crc(const u8 *upgrade_data, u64 len, u64 crc_size)
> > +{
> > + u8 crc = 0x00;
> > + u8 pad = 0xff;
> > +
> > + crc = crc8(lt8713sx_crc_table, upgrade_data, len, crc);
> > +
> > + /* pad remaining bytes */
> > + crc_size -= len;
> > + while (crc_size--)
> > + crc = crc8(lt8713sx_crc_table, &pad, 1, crc);
>
> As I wrote before, you are basically calculating the CRC over the padded
> buffer. Please pad the buffer and calculate it over the whole buffer
> rather than doing this byte by byte.
yes, will fix this in the next patch.
> > +
> > + return crc;
> > +}
> > +
> > +static int lt8713sx_prepare_firmware_data(struct lt8713sx *lt8713sx)
> > +{
> > + int ret = 0;
> > + u64 sz_12k = 12 * SZ_1K;
> > +
> > + ret = request_firmware(<8713sx->fw, FW_FILE, lt8713sx->dev);
> > + if (ret < 0) {
> > + dev_err(lt8713sx->dev, "request firmware failed\n");
> > + return ret;
> > + }
> > +
> > + dev_dbg(lt8713sx->dev, "Firmware size: %zu bytes\n", lt8713sx->fw->size);
> > +
> > + if (lt8713sx->fw->size > SZ_256K - 1) {
> > + dev_err(lt8713sx->dev, "Firmware size exceeds 256KB limit\n");
> > + release_firmware(lt8713sx->fw);
> > + return -EINVAL;
> > + }
> > +
> > + lt8713sx->fw_buffer = kvmalloc(SZ_256K, GFP_KERNEL);
> > + if (!lt8713sx->fw_buffer) {
> > + release_firmware(lt8713sx->fw);
> > + return -ENOMEM;
> > + }
> > +
> > + memset(lt8713sx->fw_buffer, 0xff, SZ_256K);
> > +
> > + if (lt8713sx->fw->size < SZ_64K) {
> > + memcpy(lt8713sx->fw_buffer, lt8713sx->fw->data, lt8713sx->fw->size);
> > + lt8713sx->fw_buffer[SZ_64K - 1] =
> > + calculate_crc(lt8713sx->fw->data, lt8713sx->fw->size, SZ_64K - 1);
> > + lt8713sx->main_crc_value = lt8713sx->fw_buffer[SZ_64K - 1];
> > + dev_dbg(lt8713sx->dev,
> > + "Main Firmware Data Crc=0x%02X\n", lt8713sx->main_crc_value);
> > +
> > + } else {
> > + /* main firmware */
> > + memcpy(lt8713sx->fw_buffer, lt8713sx->fw->data, SZ_64K - 1);
> > + lt8713sx->fw_buffer[SZ_64K - 1] =
> > + calculate_crc(lt8713sx->fw_buffer, SZ_64K - 1, SZ_64K - 1);
> > + lt8713sx->main_crc_value = lt8713sx->fw_buffer[SZ_64K - 1];
> > + dev_dbg(lt8713sx->dev,
> > + "Main Firmware Data Crc=0x%02X\n", lt8713sx->main_crc_value);
> > +
> > + /* bank firmware */
> > + memcpy(lt8713sx->fw_buffer + SZ_64K,
> > + lt8713sx->fw->data + SZ_64K,
> > + lt8713sx->fw->size - SZ_64K);
> > +
> > + lt8713sx->bank_num = (lt8713sx->fw->size - SZ_64K + sz_12k - 1) / sz_12k;
> > + dev_dbg(lt8713sx->dev, "Bank Number Total is %d.\n", lt8713sx->bank_num);
> > +
> > + for (int i = 0; i < lt8713sx->bank_num; i++) {
> > + lt8713sx->bank_crc_value[i] =
> > + calculate_crc(lt8713sx->fw_buffer + SZ_64K + i * sz_12k,
> > + sz_12k, sz_12k);
> > + dev_dbg(lt8713sx->dev, "Bank number:%d; Firmware Data Crc:0x%02X\n",
> > + i, lt8713sx->bank_crc_value[i]);
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > +static void lt8713sx_config_parameters(struct lt8713sx *lt8713sx)
> > +{
> > + regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> > + regmap_write(lt8713sx->regmap, 0xe05e, 0xc1);
> > + regmap_write(lt8713sx->regmap, 0xe058, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe059, 0x50);
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x10);
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe058, 0x21);
> > +}
> > +
> > +static void lt8713sx_wren(struct lt8713sx *lt8713sx)
> > +{
> > + regmap_write(lt8713sx->regmap, 0xe103, 0xbf);
> > + regmap_write(lt8713sx->regmap, 0xe103, 0xff);
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x04);
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> > +}
> > +
> > +static void lt8713sx_wrdi(struct lt8713sx *lt8713sx)
> > +{
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x08);
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> > +}
> > +
> > +static void lt8713sx_fifo_reset(struct lt8713sx *lt8713sx)
> > +{
> > + regmap_write(lt8713sx->regmap, 0xe103, 0xbf);
> > + regmap_write(lt8713sx->regmap, 0xe103, 0xff);
> > +}
> > +
> > +static void lt8713sx_disable_sram_write(struct lt8713sx *lt8713sx)
> > +{
> > + regmap_write(lt8713sx->regmap, 0xe055, 0x00);
> > +}
> > +
> > +static void lt8713sx_sram_to_flash(struct lt8713sx *lt8713sx)
> > +{
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x30);
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> > +}
> > +
> > +static void lt8713sx_i2c_to_sram(struct lt8713sx *lt8713sx)
> > +{
> > + regmap_write(lt8713sx->regmap, 0xe055, 0x80);
> > + regmap_write(lt8713sx->regmap, 0xe05e, 0xc0);
> > + regmap_write(lt8713sx->regmap, 0xe058, 0x21);
> > +}
> > +
> > +static u8 lt8713sx_read_flash_status(struct lt8713sx *lt8713sx)
> > +{
> > + u32 flash_status = 0;
> > +
> > + regmap_write(lt8713sx->regmap, 0xe103, 0x3f);
> > + regmap_write(lt8713sx->regmap, 0xe103, 0xff);
> > +
> > + regmap_write(lt8713sx->regmap, 0xe05e, 0x40);
> > + regmap_write(lt8713sx->regmap, 0xe056, 0x05); /* opcode=read status register */
> > + regmap_write(lt8713sx->regmap, 0xe055, 0x25);
> > + regmap_write(lt8713sx->regmap, 0xe055, 0x01);
> > + regmap_write(lt8713sx->regmap, 0xe058, 0x21);
> > +
> > + regmap_read(lt8713sx->regmap, 0xe05f, &flash_status);
> > + dev_dbg(lt8713sx->dev, "flash_status:%x\n", flash_status);
> > +
> > + return flash_status;
> > +}
> > +
> > +static void lt8713sx_block_erase(struct lt8713sx *lt8713sx)
> > +{
> > + u32 i = 0;
> > + u8 flash_status = 0;
> > + u8 blocknum = 0x00;
> > + u32 flashaddr = 0x00;
> > +
> > + for (blocknum = 0; blocknum < 8; blocknum++) {
> > + flashaddr = blocknum * SZ_32K;
> > + regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
>
> lt8713sx_i2c_enable() ? Why is it set again?
Right, it is set again in lt8713sx_config_parameters(), lt8713sx_block_erase()
and couple of more places, could be redundant.
Will try removing it.
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x04);
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe05b, flashaddr >> 16);
> > + regmap_write(lt8713sx->regmap, 0xe05c, flashaddr >> 8);
> > + regmap_write(lt8713sx->regmap, 0xe05d, flashaddr);
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x01);
> > + regmap_write(lt8713sx->regmap, 0xe05a, 0x00);
> > + msleep(100);
> > + i = 0;
> > + while (1) {
> > + flash_status = lt8713sx_read_flash_status(lt8713sx);
> > + if ((flash_status & 0x01) == 0)
> > + break;
> > +
> > + if (i > 50)
> > + break;
> > +
> > + i++;
> > + msleep(50);
> > + }
> > + }
> > + dev_dbg(lt8713sx->dev, "erase flash done.\n");
> > +}
> > +
> > +static void lt8713sx_load_main_fw_to_sram(struct lt8713sx *lt8713sx)
> > +{
> > + regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> > + regmap_write(lt8713sx->regmap, 0xe068, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe069, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe06a, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe065, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe066, 0xff);
> > + regmap_write(lt8713sx->regmap, 0xe067, 0xff);
> > + regmap_write(lt8713sx->regmap, 0xe06b, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe06c, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe060, 0x01);
> > + msleep(200);
> > + regmap_write(lt8713sx->regmap, 0xe060, 0x00);
> > +}
> > +
> > +static void lt8713sx_load_bank_fw_to_sram(struct lt8713sx *lt8713sx, u64 addr)
> > +{
> > + regmap_write(lt8713sx->regmap, 0xe0ee, 0x01);
> > + regmap_write(lt8713sx->regmap, 0xe068, ((addr & 0xff0000) >> 16));
> > + regmap_write(lt8713sx->regmap, 0xe069, ((addr & 0x00ff00) >> 8));
> > + regmap_write(lt8713sx->regmap, 0xe06a, (addr & 0x0000ff));
> > + regmap_write(lt8713sx->regmap, 0xe065, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe066, 0x30);
> > + regmap_write(lt8713sx->regmap, 0xe067, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe06b, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe06c, 0x00);
> > + regmap_write(lt8713sx->regmap, 0xe060, 0x01);
> > + msleep(50);
> > + regmap_write(lt8713sx->regmap, 0xe060, 0x00);
> > +}
> > +
> > +static int lt8713sx_write_data(struct lt8713sx *lt8713sx, const u8 *data, u64 filesize)
> > +{
> > + int page = 0, num = 0, i = 0, val;
> > +
> > + page = (filesize % LT8713SX_PAGE_SIZE) ?
> > + ((filesize / LT8713SX_PAGE_SIZE) + 1) : (filesize / LT8713SX_PAGE_SIZE);
> > +
> > + dev_dbg(lt8713sx->dev,
> > + "Writing to Sram=%u pages, total size = %llu bytes\n", page, filesize);
> > +
> > + for (num = 0; num < page; num++) {
> > + dev_dbg(lt8713sx->dev, "page[%d]\n", num);
> > + lt8713sx_i2c_to_sram(lt8713sx);
> > +
> > + for (i = 0; i < LT8713SX_PAGE_SIZE; i++) {
> > + if ((num * LT8713SX_PAGE_SIZE + i) < filesize)
> > + val = *(data + (num * LT8713SX_PAGE_SIZE + i));
> > + else
> > + val = 0xff;
> > + regmap_write(lt8713sx->regmap, 0xe059, val);
> > + }
> > +
> > + lt8713sx_wren(lt8713sx);
> > + lt8713sx_sram_to_flash(lt8713sx);
> > + }
> > +
> > + lt8713sx_wrdi(lt8713sx);
> > + lt8713sx_disable_sram_write(lt8713sx);
> > +
> > + return 0;
> > +}
> > +
> > +static void lt8713sx_main_upgrade_result(struct lt8713sx *lt8713sx)
> > +{
> > + u32 main_crc_result;
> > +
> > + regmap_read(lt8713sx->regmap, 0xe023, &main_crc_result);
> > +
> > + dev_dbg(lt8713sx->dev, "Main CRC HW: 0x%02X\n", main_crc_result);
> > + dev_dbg(lt8713sx->dev, "Main CRC FW: 0x%02X\n", lt8713sx->main_crc_value);
> > +
> > + if (main_crc_result == lt8713sx->main_crc_value)
> > + dev_dbg(lt8713sx->dev, "Main Firmware Upgrade Success.\n");
> > + else
> > + dev_err(lt8713sx->dev, "Main Firmware Upgrade Failed.\n");
> > +}
> > +
> > +static void lt8713sx_bank_upgrade_result(struct lt8713sx *lt8713sx, u8 banknum)
> > +{
> > + u32 bank_crc_result;
> > +
> > + regmap_read(lt8713sx->regmap, 0xe023, &bank_crc_result);
> > +
> > + dev_dbg(lt8713sx->dev, "Bank %d CRC Result: 0x%02X\n", banknum, bank_crc_result);
> > +
> > + if (bank_crc_result == lt8713sx->bank_crc_value[banknum])
> > + dev_dbg(lt8713sx->dev, "Bank %d Firmware Upgrade Success.\n", banknum);
> > + else
> > + dev_err(lt8713sx->dev, "Bank %d Firmware Upgrade Failed.\n", banknum);
> > +}
> > +
> > +static void lt8713sx_bank_result_check(struct lt8713sx *lt8713sx)
> > +{
> > + int i;
> > + u64 addr = 0x010000;
> > +
> > + for (i = 0; i < lt8713sx->bank_num; i++) {
> > + lt8713sx_load_bank_fw_to_sram(lt8713sx, addr);
> > + lt8713sx_bank_upgrade_result(lt8713sx, i);
> > + addr += 0x3000;
> > + }
> > +}
> > +
> > +static int lt8713sx_firmware_upgrade(struct lt8713sx *lt8713sx)
> > +{
> > + int ret;
> > +
> > + lt8713sx_config_parameters(lt8713sx);
> > +
> > + lt8713sx_block_erase(lt8713sx);
> > +
> > + if (lt8713sx->fw->size < SZ_64K) {
> > + ret = lt8713sx_write_data(lt8713sx, lt8713sx->fw_buffer, SZ_64K);
> > + if (ret < 0) {
> > + dev_err(lt8713sx->dev, "Failed to write firmware data: %d\n", ret);
> > + return ret;
> > + }
> > + } else {
> > + ret = lt8713sx_write_data(lt8713sx, lt8713sx->fw_buffer, lt8713sx->fw->size);
> > + if (ret < 0) {
> > + dev_err(lt8713sx->dev, "Failed to write firmware data: %d\n", ret);
> > + return ret;
> > + }
> > + }
> > + dev_dbg(lt8713sx->dev, "Write Data done.\n");
> > +
> > + return 0;
> > +}
> > +
> > +static int lt8713sx_firmware_update(struct lt8713sx *lt8713sx)
> > +{
> > + int ret = 0;
> > +
> > + mutex_lock(<8713sx->ocm_lock);
> > + lt8713sx_i2c_enable(lt8713sx);
> > +
> > + ret = lt8713sx_prepare_firmware_data(lt8713sx);
> > + if (ret < 0) {
> > + dev_err(lt8713sx->dev, "Failed to prepare firmware data: %d\n", ret);
> > + goto error;
> > + }
> > +
> > + ret = lt8713sx_firmware_upgrade(lt8713sx);
> > + if (ret < 0) {
> > + dev_err(lt8713sx->dev, "Upgrade failure.\n");
> > + goto error;
> > + } else {
> > + /* Validate CRC */
> > + lt8713sx_load_main_fw_to_sram(lt8713sx);
> > + lt8713sx_main_upgrade_result(lt8713sx);
> > + lt8713sx_wrdi(lt8713sx);
> > + lt8713sx_fifo_reset(lt8713sx);
> > + lt8713sx_bank_result_check(lt8713sx);
> > + lt8713sx_wrdi(lt8713sx);
> > + }
> > +
> > +error:
> > + lt8713sx_i2c_disable(lt8713sx);
> > + if (!ret)
> > + lt8713sx_reset(lt8713sx);
> > +
> > + kvfree(lt8713sx->fw_buffer);
> > + lt8713sx->fw_buffer = NULL;
> > +
> > + if (lt8713sx->fw) {
> > + release_firmware(lt8713sx->fw);
> > + lt8713sx->fw = NULL;
> > + }
> > + mutex_unlock(<8713sx->ocm_lock);
> > +
> > + return ret;
> > +}
> > +
> > +static void lt8713sx_reset(struct lt8713sx *lt8713sx)
> > +{
> > + dev_dbg(lt8713sx->dev, "reset bridge.\n");
> > + gpiod_set_value_cansleep(lt8713sx->reset_gpio, 1);
> > + msleep(20);
> > +
> > + gpiod_set_value_cansleep(lt8713sx->reset_gpio, 0);
> > + msleep(20);
> > +
> > + dev_dbg(lt8713sx->dev, "reset done.\n");
> > +}
> > +
> > +static int lt8713sx_regulator_enable(struct lt8713sx *lt8713sx)
> > +{
> > + int ret;
> > +
> > + ret = devm_regulator_get_enable(lt8713sx->dev, "vdd");
> > + if (ret < 0)
> > + return dev_err_probe(lt8713sx->dev, ret, "failed to enable vdd regulator\n");
> > +
> > + usleep_range(1000, 10000);
> > +
> > + ret = devm_regulator_get_enable(lt8713sx->dev, "vcc");
> > + if (ret < 0)
> > + return dev_err_probe(lt8713sx->dev, ret, "failed to enable vcc regulator\n");
> > + return 0;
> > +}
> > +
> > +static int lt8713sx_bridge_attach(struct drm_bridge *bridge,
> > + struct drm_encoder *encoder,
> > + enum drm_bridge_attach_flags flags)
> > +{
> > + struct lt8713sx *lt8713sx = container_of(bridge, struct lt8713sx, bridge);
> > + int i, ret;
> > +
> > + for (i = 0; i < lt8713sx->num_outputs; i++) {
> > + if (!lt8713sx->next_bridge[i])
> > + continue;
> > +
> > + ret = drm_bridge_attach(encoder,
> > + lt8713sx->next_bridge[i],
> > + bridge, flags);
>
> This wasn't really tested. This code will not result in what you intend
> to do here. Each next bridge should have its own encoder / bridge chain.
This patch is working, might be because lt8713sx itself provides 3edp output from 1edp.
I am trying to attach all next bridges to the one previous in chain.
I will try to attach a single next_bridge in next patch.
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int lt8713sx_get_ports(struct lt8713sx *lt8713sx)
> > +{
> > + struct device *dev = lt8713sx->dev;
> > + struct device_node *port, *ports, *ep, *remote;
> > + int i = 0;
> > + u32 reg;
> > +
> > + ports = of_get_child_by_name(dev->of_node, "ports");
> > + if (!ports)
> > + return -ENODEV;
> > +
> > + for_each_child_of_node(ports, port) {
> > + if (of_property_read_u32(port, "reg", ®))
> > + continue;
> > +
> > + if (reg == 0)
> > + continue;
> > +
> > + if (i >= ARRAY_SIZE(lt8713sx->next_bridge)) {
> > + of_node_put(port);
> > + break;
> > + }
> > +
> > + ep = of_graph_get_next_endpoint(port, NULL);
> > + if (!ep)
> > + continue;
> > +
> > + remote = of_graph_get_remote_port_parent(ep);
> > + of_node_put(ep);
>
> And this misses the of_device_is_available() check. Please use
> drm_of_find_panel_or_bridge().
yes, Will fix in next patch.
> > +
> > + if (!remote)
> > + continue;
> > +
> > + lt8713sx->next_bridge[i] = of_drm_find_bridge(remote);
> > + of_node_put(remote);
> > + if (lt8713sx->next_bridge[i])
> > + i++;
>
> And if not, the driver should be returning -EPROBE_DEFER.
Will fix in next patch.
> > + }
> > + lt8713sx->num_outputs = i;
> > + dev_dbg(dev, "Enabled %d output ports", i);
> > +
> > + of_node_put(ports);
> > + return 0;
> > +};
> > +
> > +static int lt8713sx_gpio_init(struct lt8713sx *lt8713sx)
> > +{
> > + struct device *dev = lt8713sx->dev;
> > +
> > + lt8713sx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> > + if (IS_ERR(lt8713sx->reset_gpio))
> > + return dev_err_probe(dev, PTR_ERR(lt8713sx->reset_gpio),
> > + "failed to acquire reset gpio\n");
> > +
> > + /* power enable gpio */
> > + lt8713sx->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
> > + if (IS_ERR(lt8713sx->enable_gpio))
> > + return dev_err_probe(dev, PTR_ERR(lt8713sx->enable_gpio),
> > + "failed to acquire enable gpio\n");
> > + return 0;
> > +}
> > +
> > +static ssize_t lt8713sx_firmware_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t len)
> > +{
> > + struct lt8713sx *lt8713sx = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + ret = lt8713sx_firmware_update(lt8713sx);
> > + if (ret < 0)
> > + return ret;
> > + return len;
> > +}
> > +
> > +static DEVICE_ATTR_WO(lt8713sx_firmware);
> > +
> > +static struct attribute *lt8713sx_attrs[] = {
> > + &dev_attr_lt8713sx_firmware.attr,
> > + NULL,
> > +};
> > +
> > +static const struct attribute_group lt8713sx_attr_group = {
> > + .attrs = lt8713sx_attrs,
> > +};
> > +
> > +static const struct attribute_group *lt8713sx_attr_groups[] = {
> > + <8713sx_attr_group,
> > + NULL,
> > +};
> > +
> > +static const struct drm_bridge_funcs lt8713sx_bridge_funcs = {
> > + .attach = lt8713sx_bridge_attach,
> > +};
> > +
> > +static int lt8713sx_probe(struct i2c_client *client)
> > +{
> > + struct lt8713sx *lt8713sx;
> > + struct device *dev = &client->dev;
> > + int ret;
> > +
> > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> > + return dev_err_probe(dev, -ENODEV, "device doesn't support I2C\n");
> > +
> > + lt8713sx = devm_drm_bridge_alloc(dev, struct lt8713sx, bridge, <8713sx_bridge_funcs);
> > + if (IS_ERR(lt8713sx))
> > + return PTR_ERR(lt8713sx);
> > +
> > + lt8713sx->dev = dev;
> > + lt8713sx->client = client;
> > + i2c_set_clientdata(client, lt8713sx);
> > +
> > + ret = devm_mutex_init(lt8713sx->dev, <8713sx->ocm_lock);
> > + if (ret)
> > + return ret;
> > +
> > + lt8713sx->regmap = devm_regmap_init_i2c(client, <8713sx_regmap_config);
> > + if (IS_ERR(lt8713sx->regmap))
> > + return dev_err_probe(dev, PTR_ERR(lt8713sx->regmap), "regmap i2c init failed\n");
> > +
> > + ret = lt8713sx_get_ports(lt8713sx);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = lt8713sx_gpio_init(lt8713sx);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = lt8713sx_regulator_enable(lt8713sx);
> > + if (ret)
> > + return ret;
> > +
> > + lt8713sx_reset(lt8713sx);
> > +
> > + lt8713sx->bridge.funcs = <8713sx_bridge_funcs;
> > + lt8713sx->bridge.of_node = dev->of_node;
> > + lt8713sx->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
> > + drm_bridge_add(<8713sx->bridge);
> > +
> > + crc8_populate_msb(lt8713sx_crc_table, 0x31);
> > +
> > + return 0;
> > +}
> > +
> > +static void lt8713sx_remove(struct i2c_client *client)
> > +{
> > + struct lt8713sx *lt8713sx = i2c_get_clientdata(client);
> > +
> > + drm_bridge_remove(<8713sx->bridge);
> > +}
> > +
> > +static struct i2c_device_id lt8713sx_id[] = {
> > + { "lontium,lt8713sx", 0 },
> > + { /* sentinel */ }
> > +};
> > +
> > +static const struct of_device_id lt8713sx_match_table[] = {
> > + { .compatible = "lontium,lt8713sx" },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, lt8713sx_match_table);
> > +
> > +static struct i2c_driver lt8713sx_driver = {
> > + .driver = {
> > + .name = "lt8713sx",
> > + .of_match_table = lt8713sx_match_table,
> > + .dev_groups = lt8713sx_attr_groups,
> > + },
> > + .probe = lt8713sx_probe,
> > + .remove = lt8713sx_remove,
> > + .id_table = lt8713sx_id,
> > +};
> > +
> > +module_i2c_driver(lt8713sx_driver);
> > +MODULE_LICENSE("GPL");
> > +MODULE_DESCRIPTION("lt8713sx drm bridge driver");
> > +MODULE_AUTHOR("Tony <syyang@xxxxxxxxxxx>");
> > +MODULE_FIRMWARE(FW_FILE);
> >
> > --
> > 2.34.1
> >
Sorry maintainers for delay in response.
I was on travel and than went on leave for a couple of weeks. Will be dedicately work on this now.
> --
> With best wishes
> Dmitry