Re: [PATCH v2 2/4] drm/bridge: Add THine THC63LVD827 LVDS encoder

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/bridge: back-powering via GPIOs on unpowered chip
--

commit b3195cf0e052ea83d4890e15599f3b4469d3b2e4
Author: Francesco Dolcini <francesco.dolcini@toradex.com>

drm/bridge: Add THine THC63LVD827 LVDS encoder

This commit introduces a new DRM bridge driver for the THine THC63LVD827 DPI
to LVDS bridge. The driver provides support for configuring the LVDS data
mapping, GPIO-based control pins, and regulator power supplies.

> diff --git a/drivers/gpu/drm/bridge/thc63lvd827.c b/drivers/gpu/drm/bridge/thc63lvd827.c
> new file mode 100644
> index 0000000000000..02b97d1cc71ea
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/thc63lvd827.c
[ ... ]
> +static void thc63lvd827_atomic_post_disable(struct drm_bridge *bridge,
> +					    struct drm_atomic_commit *state)
> +{
> +	struct thc63lvd827 *thc63lvd827 = bridge_to_thc63lvd827(bridge);
> +	int ret;
> +
> +	gpiod_set_value_cansleep(thc63lvd827->pd_gpio, 1);
> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(thc63lvd827_supplies),
> +				     thc63lvd827->supplies);

[Severity: Medium]
Does this sequence leave the chip exposed to back-powering?

The regulators are disabled here, cutting power to the chip, but the driver
does not reset the b6b8, map, or mode GPIOs to a safe physical low state first.
These pins remain driven in their previously asserted physical HIGH states,
causing continuous back-powering while the display is disabled.

Hardware back-powering causes leakage current through ESD diodes into the
unpowered chip's VDD net, which can lead to latch-up, increased power
consumption during suspend, or improper power-on reset sequencing.

[ ... ]
> +static int thc63lvd827_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	thc63lvd827->map_gpio = devm_gpiod_get_optional(dev, "map", GPIOD_OUT_LOW);
> +	if (IS_ERR(thc63lvd827->map_gpio))
> +		return dev_err_probe(dev, PTR_ERR(thc63lvd827->map_gpio),
> +				     "failed to get MAP GPIO\n");
> +
> +	thc63lvd827->mode_gpio = devm_gpiod_get_optional(dev, "mode", GPIOD_OUT_HIGH);

[Severity: Medium]
Will this initialization also cause back-powering via GPIOs on the unpowered
chip?

This configures the mode GPIO to be driven logically HIGH in
thc63lvd827_probe(). Since the chip's regulators are not enabled until later in
thc63lvd827_atomic_pre_enable(), this immediately sources voltage into the
unpowered IC.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005095006.20290-1-francesco@dolcini.it?part=2