Re: [PATCH 1/2] dt-bindings: display: panel: Add Samsung AMS662ZS01

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The binding introduces a custom `vout-gpios` property instead of using a standard property or a regulator node.
- [Low] The device tree example assigns the exact same active pinctrl state to both the default and sleep states.
--

commit 35f05d4d7ea0faa656ae9f5edd021117b1b534c6
Author: Vsevolod Nevorotov <sevanevorotov29@gmail.com>

dt-bindings: display: panel: Add Samsung AMS662ZS01

Add Device Tree binding documentation for the Samsung AMS662ZS01
FHD+ AMOLED DSI panel found in the OnePlus 9RT smartphone.

> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,ams662zs01.yaml b/Documentation/devicetree/bindings/display/panel/samsung,ams662zs01.yaml
> new file mode 100644
> index 0000000000000..b2697d140edef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,ams662zs01.yaml
[ ... ]
> +  reset-gpios:
> +    maxItems: 1
> +
> +  vout-gpios:
> +    maxItems: 1
> +    description: Panel VOUT enable GPIO line

[Severity: Low]
Is a custom vout-gpios property required here?

Using custom GPIO properties for standard panel functionality like power
enabling fragments the device tree ABI and prevents the use of generic
driver infrastructure.

Could this be modeled with standard paradigms instead, such as
enable-gpios or a regulator-fixed node?

> +
> +  vddio-supply:
> +    description: 1.8V I/O power supply
[ ... ]
> +            vddio-supply = <&vreg_l12c_1p8>;
> +            vdd-supply = <&vreg_l13c_3p0>;
> +
> +            pinctrl-names = "default", "sleep";
> +            pinctrl-0 = <&mdp_vsync_active>;
> +            pinctrl-1 = <&mdp_vsync_active>;

[Severity: Low]
Should the sleep state use a different pin configuration?

The example assigns the exact same active pinctrl state (&mdp_vsync_active) to
both the default and sleep states.

Since device tree examples are frequently copied verbatim into production
device trees by developers, using the active pin configuration for the
sleep state defeats power management and can lead to increased power
leakage during system suspend.

> +
> +            port {
> +                panel_in: endpoint {
> +                    remote-endpoint = <&mdss_dsi0_out>;
> +                };
> +            };
> +        };
> +    };

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