Re: [PATCH 3/3] arm64: dts: rockchip: add overlay for tiger-haikou video-demo adapter

["Heiko Stübner" <heiko@sntech.de>]

Hi Quentin,

Am Mittwoch, 6. November 2024, 14:18:49 CET schrieb Quentin Schulz:
> On 11/6/24 1:37 PM, Heiko Stuebner wrote:
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > index 09423070c992..0c4ee6a767b8 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile

[...]

> > +&dsi0 {
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> 
> Shouldn't those be in the SoC dtsi? Is there any world where this would 
> be different per board?

It's more of a we need that here to _compile_ the dtbo, so one gets
to duplicate them, as the dtbo is not compiled _against_ the dtb, hence
dtc does not know about the parent #address-cells value.


> > +	status = "okay";
> > +
> > +	panel@0 {
> > +		compatible = "leadtek,ltk050h3148w";
> > +		reg = <0>;
> > +		backlight = <&backlight>;
> > +		iovcc-supply = <&vcc1v8_video>;
> > +		reset-gpios = <&pca9670 0 GPIO_ACTIVE_LOW>;
> > +		vci-supply = <&vcc2v8_video>;
> > +
> > +		port {
> > +			mipi_panel_in: endpoint {
> > +				remote-endpoint = <&dsi0_out_panel>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&dsi0_in {
> > +	dsi0_in_vp3: endpoint {
> > +		remote-endpoint = <&vp3_out_dsi0>;
> > +	};
> > +};
> > +
> > +&dsi0_out {
> > +	dsi0_out_panel: endpoint {
> > +		remote-endpoint = <&mipi_panel_in>;
> > +	};
> > +};
> > +
> > +&i2c6 {
> > +	/* OV5675, GT911, DW9714 are limited to 400KHz */
> > +	clock-frequency = <400000>;
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> 
> Mmmm why the address and size cells properties here? They should already 
> be part of the SoC dtsi no?

same as above

> > +&pwm0 {
> > +	pinctrl-0 = <&pwm0m1_pins>;
> > +	pinctrl-names = "default";
> 
> The other two pin muxes for PWM0 are either:
> - the pin used for CAN
> - the pin routed to an internal component (unexposed to Q7) and used as 
> a GPIO
> 
> so please move the pinctrl to the Tiger SoM DTSI.

the pwm0-pinctrl is actually already set in the tiger.dtsi, so
I'll just remove it from here.

> > +	status = "okay";
> > +};
> > +
> > +&vp3 {
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> 
> Shouldn't those be in the SoC dtsi?

same as dsi and i2c #adress-cells


Heiko