Message-ID: <ud4eneuxamylrbiaqr6d6ntoh52kk6aqr6rc2v7lu24lhulidg@sdruua7h4yxh>
Date: Sun, 28 Dec 2025 17:01:05 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Vishnu Saini <vishnu.saini@....qualcomm.com>
Cc: Andrzej Hajda <andrzej.hajda@...el.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Robert Foss <rfoss@...nel.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, Tony <syyang@...tium.com>,
        dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, prahlad.valluru@....qualcomm.com,
        Prahlad Valluru <vvalluru@....qualcomm.com>
Subject: Re: [PATCH v3 2/2] drm/bridge: add support for lontium lt8713sx
 bridge driver

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

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

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

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

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

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

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

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

lt8713sx_i2c_enable() ? Why is it set again?

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

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

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

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

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

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

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

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists