Re: [PATCH 7/7] arm64: dts: qcom: shikra-iqs-evk: Add Waveshare 8-inch DSI panel overlay

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] arm64: dts: qcom: missing panel_vcc regulator on wsgpio 8 in Waveshare overlay
--

commit 8859b55c691118c8af80fdf98b5e594862815afd
Author: Shashank Maurya <shashank.maurya@oss.qualcomm.com>

arm64: dts: qcom: shikra-iqs-evk: Add Waveshare 8-inch DSI panel overlay

This commit adds a devicetree overlay for the Waveshare 8-inch DSI panel on
the Shikra IQS EVK carrier board, configuring the jadard,jd9365da-h3 panel,
the waveshare,dsi-touch-gpio expander, and the goodix,gt9271 touchscreen
while disabling the default hdmi output.

> diff --git a/arch/arm64/boot/dts/qcom/shikra-iqs-evk-waveshare-dsi-8inch.dtso b/arch/arm64/boot/dts/qcom/shikra-iqs-evk-waveshare-dsi-8inch.dtso
> new file mode 100644
> index 0000000000000..d0aa115681ffd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/shikra-iqs-evk-waveshare-dsi-8inch.dtso
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +
> +&{/} {
> +	model = "Qualcomm Technologies, Inc. Shikra IQS EVK Waveshare 8inch DSI Panel";
> +
> +	panel_avdd: regulator-panel-avdd {
> +		compatible = "regulator-fixed";
> +		regulator-name = "panel-avdd";
> +		gpios = <&wsgpio 0 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	panel_iovcc: regulator-panel-iovcc {
> +		compatible = "regulator-fixed";
> +		regulator-name = "panel-iovcc";
> +		gpios = <&wsgpio 4 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +};
> +
> +&hdmi_bridge {
> +	status = "disabled";
> +};
> +
> +&hdmi_connector {
> +	status = "disabled";
> +};
> +
> +&i2c4 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	wsgpio: gpio@45 {
> +		compatible = "waveshare,dsi-touch-gpio";
> +		reg = <0x45>;
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +	};
> +
> +	touchscreen@5d {
> +		compatible = "goodix,gt9271";
> +		reg = <0x5d>;
> +		reset-gpios = <&wsgpio 9 GPIO_ACTIVE_HIGH>;
> +	};
> +};

[Severity: High]
Does this leave the Waveshare vcc rail unpowered?

In drivers/gpio/gpio-waveshare-dsi.c, the expander maps offset 8 to the vcc
rail and offset 9 to touchscreen reset:

enum {
	GPIO_AVDD = 0,
	GPIO_PANEL_RESET = 1,
	GPIO_BL_ENABLE = 2,
	GPIO_IOVCC = 4,
	GPIO_VCC = 8,
	GPIO_TS_RESET = 9,
};

During waveshare_gpio_probe()->waveshare_gpio_set(), state->poweron_state
starts at 0 and asserting offset 9 writes the upper byte of
state->poweron_state to the touch register:

	last_val = state->poweron_state;
	if (value)
		last_val |= BIT(offset);
	else
		last_val &= ~BIT(offset);

	state->poweron_state = last_val;

	err = regmap_write(state->regmap, REG_TP, last_val >> 8);
	if (!err)
		err = regmap_write(state->regmap, REG_LCD, last_val & 0xff);

Because the overlay only defines panel_avdd (offset 0) and panel_iovcc
(offset 4) under &{/} and does not configure a regulator for wsgpio
offset 8, bit 0 of the touch register is driven low on every
waveshare_gpio_set() call and never enabled before goodix_ts_probe()
accesses touchscreen@5d.

Should &{/} also define a panel_vcc regulator for wsgpio offset 8 (as in
Documentation/devicetree/bindings/gpio/waveshare,dsi-touch-gpio.yaml, or
wired to touchscreen@5d) so the vcc rail is enabled?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021-shikra_display_overlays-v1-0-adaea735999f@oss.qualcomm.com?part=7