Re: [PATCH V6 4/6] drm/panel: anbernic-td4310: Add RG Vita Pro panel

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Requesting `reset_gpio` as `GPIOD_OUT_LOW` in probe backpowers the panel, and missing reset assertion in the `prepare` error path leaves the panel backpowered.
- [Low] The driver calls a non-existent function `devm_drm_panel_add()`, causing a compilation failure.
--

commit 0a4500b372d995ebf665c9896430451a3e287a41
Author: Chris Morgan <macromorgan@hotmail.com>

drm/panel: anbernic-td4310: Add RG Vita Pro panel

The panel used by Anbernic in the RG Vita-Pro is a DSI panel based
on the TD4310 controller IC.

> diff --git a/drivers/gpu/drm/panel/panel-anbernic-td4310.c b/drivers/gpu/drm/panel/panel-anbernic-td4310.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-anbernic-td4310.c
[ ... ]
> +static int panel_anbernic_td4310_prepare(struct drm_panel *panel)
> +{
[ ... ]
> +	if (dsi_ctx.accum_err) {
> +		ret = dsi_ctx.accum_err;
> +		goto err_reset;
> +	}
> +
> +	return 0;
> +
> +err_reset:
> +	gpiod_set_value_cansleep(ctx->enable_gpio, 0);
> +err_enable:
> +	regulator_disable(ctx->vdd);

[Severity: High]
The err_reset path disables the VDD regulator without re-asserting the reset
GPIO (setting it to logical 1 / physical LOW). Could this leave the unpowered
panel backpowered through its IO lines if the prepare sequence fails?

> +	return ret;
> +}
[ ... ]
> +static int panel_anbernic_td4310_probe(struct mipi_dsi_device *dsi)
> +{
[ ... ]
> +	ret = of_drm_get_panel_orientation(dev->of_node, &ctx->orientation);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to get panel orientation\n");
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);

[Severity: High]
Assuming the reset GPIO is configured as active-low in the device tree,
requesting it with GPIOD_OUT_LOW sets its logical value to 0, which drives
the physical pin HIGH.

Because the panel's VDD supply is off at probe time, will driving the
reset pin HIGH backpower the panel via its IO protection diodes?

> +	if (IS_ERR(ctx->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
> +				     "Cannot get reset gpio\n");
[ ... ]
> +	dsi->mode_flags = ctx->panel_info->mode_flags;
> +
> +	ret = drm_panel_of_backlight(&ctx->panel);
> +	if (ret)
> +		return ret;
> +
> +	devm_drm_panel_add(dev, &ctx->panel);

[Severity: Low]
Is devm_drm_panel_add() defined in the DRM panel API? It appears only
drm_panel_add() and devm_drm_panel_add_follower() are available, which
might cause an implicit declaration build error here.

> +
> +	ret = devm_mipi_dsi_attach(dev, dsi);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to attach to DSI host\n");
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608175521.67449-1-macroalpha82@gmail.com?part=4