[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4636112.cEBGB3zze1@diego>
Date: Wed, 27 Nov 2024 10:46:00 +0100
From: Heiko Stübner <heiko@...ech.de>
To: Quentin Schulz <quentin.schulz@...rry.de>
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
Heiko Stuebner <heiko.stuebner@...rry.de>
Subject:
Re: [PATCH 3/3] arm64: dts: rockchip: add overlay for tiger-haikou video-demo
adapter
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
Powered by blists - more mailing lists