Re: [PATCH v13 2/2] drm/bridge: Add Lontium LT9611C(EX/UXD) MIPI DSI to HDMI driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hardware reset causes regmap's page selector cache to become desynchronized, leading to register accesses on the wrong page.
- [Medium] Memory leak of device tree node references on probe error paths.
--

commit 8472ac6fb646e6b178a64a965210c4a862f133a2
Author: Sunyun Yang <syyang@lontium.com>

drm/bridge: Add Lontium LT9611C(EX/UXD) MIPI DSI to HDMI driver

This commit introduces a new DRM bridge driver for the Lontium 
LT9611C(EX/UXD) DSI to HDMI chip. It handles chip initialization, EDID 
reading, firmware upgrading, and audio infoframe setup.

> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611c.c b/drivers/gpu/drm/bridge/lontium-lt9611c.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611c.c
[ ... ]
> +static int lt9611c_probe(struct i2c_client *client)
> +{
[ ... ]
> +	ret = lt9611c_parse_dt(dev, lt9611c, &dsi0_node, &dsi1_node);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to parse device tree\n");
> +
> +	lt9611c->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(lt9611c->reset_gpio)) {
> +		ret = PTR_ERR(lt9611c->reset_gpio);
> +		return ret;
> +	}

[Severity: Medium]
Does this error path leak the device tree node references? The call to
lt9611c_parse_dt() acquires remote endpoint nodes for dsi0_node and
dsi1_node, but returning directly here skips dropping those references with
of_node_put(). Should this jump to an error cleanup label instead?

> +
> +	ret = lt9611c_regulator_init(lt9611c);
> +	if (ret < 0)
> +		return ret;

[Severity: Medium]
Similarly, could this return statement leak the same device tree node
references from lt9611c_parse_dt() if regulator initialization fails?

[ ... ]
> +static int lt9611c_firmware_upgrade(struct lt9611c *lt9611c)
> +{
[ ... ]
> +	ret = lt9611c_write_crc(lt9611c, fw_crc, FW_SIZE - 1);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to write firmware crc\n");
> +		goto out_unlock;
> +	}
> +
> +	lt9611c_reset(lt9611c);
> +	ret = lt9611c_upgrade_result(lt9611c, fw_crc);

[Severity: High]
Can this cause regmap's page selector cache to become desynchronized?

The lt9611c_firmware_upgrade() function writes to page 0xe0 earlier, then
lt9611c_reset() toggles the reset GPIO. This reverts the chip's internal
page register to its default (0x00), but the regmap core's internal state
caches the window as still being at 0xe0.

When lt9611c_upgrade_result() immediately attempts to read from 0xe021,
will regmap skip writing the page selector and accidentally read from
page 0x00 instead, causing the upgrade verification to fail?

[ ... ]
> +static int lt9611c_bridge_resume(struct device *dev)
> +{
> +	struct lt9611c *lt9611c = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(lt9611c->supplies), lt9611c->supplies);
> +	if (ret) {
> +		dev_err(lt9611c->dev, "regulator bulk enable failed.\n");
> +		return ret;
> +	}
> +	lt9611c_reset(lt9611c);
> +	enable_irq(lt9611c->client->irq);
> +
> +	return ret;
> +}

[Severity: High]
Does this reset during resume also desynchronize the regmap cache for any
subsequent display setup or HPD detect reads?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-lt9611c-v7-v13-0-aec234483725@oss.qualcomm.com?part=2