Re: [PATCH v1 7/9 RESEND] ARM: tegra: tf600t: Configure panel

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Removal of `regulator-always-on` from a regulator named `vddio_ddr_1v2`.
- [Medium] Undocumented feature addition of camera and video input subsystems.
- [Low] Incomplete DT graph due to dangling camera endpoints.
--

commit 3e9ff009424216cb47835e1aa1d04e00f8c7e7e8
Author: Svyatoslav Ryhel <clamor95@gmail.com>

ARM: tegra: tf600t: Configure panel

This commit configures the DSI panel used in the ASUS VivoTab TF600T. It
adds the necessary display nodes, modifies the vdd1 regulator to serve as
vio_panel, and adds a new vdd_panel regulator for the display.

> diff --git a/arch/arm/boot/dts/nvidia/tegra30-asus-tf600t.dts b/arch/arm/boot/dts/nvidia/tegra30-asus-tf600t.dts
> index 9296e7970ce4e..8b68bfef8dee8 100644
> --- a/arch/arm/boot/dts/nvidia/tegra30-asus-tf600t.dts
> +++ b/arch/arm/boot/dts/nvidia/tegra30-asus-tf600t.dts

[ ... ]

> @@ -55,6 +56,37 @@ linux,cma@80000000 {
>  	};
>  
>  	host1x@50000000 {
> +		vi@54080000 {
> +			status = "okay";
> +
> +			csi@800 {
> +				status = "okay";
> +
> +				avdd-dsi-csi-supply = <&avdd_dsi_csi>;
> +			};
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +
> +					vi_ppa_input: endpoint {
> +						/* Link to the rear camera */
> +					};
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +
> +					vi_ppb_input: endpoint {
> +						/* Link to the front camera */
> +					};
> +				};
> +			};
> +		};

The commit message states the goal is to configure the DSI panel, but this
adds Video Input and CSI nodes for cameras. Should these camera subsystem
additions be split into a separate patch, or documented in the commit message?

Also, do these endpoints need remote-endpoint properties linking them to the
actual sensor nodes? Without them, the OF graph appears incomplete and might
trigger warnings during device tree schema validation.

[ ... ]

> @@ -1123,11 +1171,10 @@ pmic-sleep-hog {
>  			};
>  
>  			regulators {
> -				vdd_lcd: vdd1 {
> +				vio_panel: vdd1 {
>  					regulator-name = "vddio_ddr_1v2";
>  					regulator-min-microvolt = <1200000>;
>  					regulator-max-microvolt = <1200000>;
> -					regulator-always-on;
>  					regulator-boot-on;
>  					ti,regulator-ext-sleep-control = <8>;
>  				};

Does removing regulator-always-on from vddio_ddr_1v2 risk powering down system
memory when the display panel is disabled?

If this regulator is shared with DDR, turning off the panel might cause a system
crash. 

If it is only used for the panel on this specific board, could the
regulator-name be updated to reflect that and avoid confusion?

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