| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c53b4149-fc8c-4357-a881-de07d6ebf7ac@cherry.de>
Date: Wed, 6 Nov 2024 14:18:49 +0100
From: Quentin Schulz <quentin.schulz@...rry.de>
To: Heiko Stuebner <heiko@...ech.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 Heiko,
On 11/6/24 1:37 PM, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@...rry.de>
>
> This adds support for the video-demo-adapter for the Haikou devkit with
> Tiger RK3588 SoM.
>
> The Video Demo adapter is an adapter connected to the fake PCIe slot
> labeled "Video Connector" on the Haikou devkit.
>
> It's main feature is a Leadtek DSI-display with touchscreen. To drive these
Well that and a camera :)
Maybe we can provide the "official" name of that adapter so people can
easily find it on our product page?
DEVKIT ADDON CAM-TS-A01
https://embedded.cherry.de/product/development-kit/
> components a number of additional regulators are grouped on the adapter as
> well as a PCA9670 gpio-expander to provide the needed additional gpio-lines.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@...rry.de>
> ---
> arch/arm64/boot/dts/rockchip/Makefile | 1 +
> .../rk3588-tiger-haikou-video-demo.dtso | 153 ++++++++++++++++++
> 2 files changed, 154 insertions(+)
> create mode 100644 arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou-video-demo.dtso
>
> 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
> @@ -143,6 +143,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-ep.dtbo
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-srns.dtbo
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-tiger-haikou.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-tiger-haikou-video-demo.dtbo
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-toybrick-x0.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-turing-rk1.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-coolpi-4b.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou-video-demo.dtso b/arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou-video-demo.dtso
> new file mode 100644
> index 000000000000..c7416349e7d5
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou-video-demo.dtso
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * DT-overlay for the camera / DSI demo appliance for Haikou boards.
> + * In the flavour for use with a Tiger system-on-module.
Add the official name in the comment here (maybe a link too?).
Aren't we missing a copyright notice here as well (I personally don't
care :) )?
If so, that would be
Copyright (C) 2024 Cherry Embedded Solutions GmbH
for us.
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/soc/rockchip,vop2.h>
> +
> +&{/} {
> + video-adapter-leds {
Please order the register-less-direct-children-nodes-of-root alphabetically.
> + compatible = "gpio-leds";
> + status = "okay";
Isn't the default if omitted already "okay"?
> +
> + video-adapter-led {
> + color = <LED_COLOR_ID_BLUE>;
> + gpios = <&pca9670 7 GPIO_ACTIVE_HIGH>;
> + label = "video-adapter-led";
> + linux,default-trigger = "none";
> + };
> + };
> +
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + power-supply = <&dc_12v>;
> + pwms = <&pwm0 0 25000 0>;
> + };
> +
> + hdmi-con {
> + compatible = "hdmi-connector";
> + type = "a";
> +
> + port {
> + hdmi_con_in: endpoint {
> + remote-endpoint = <&hdmi0_out_con>;
> + };
> + };
> + };
> +
Wrong copy-paste here I guess? No HDMI connector on that adapter :)
> + vcc1v8_video: regulator-vcc1v8-video {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc1v8-video";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + vin-supply = <&vcc3v3_baseboard>;
> + };
> +
> + vcc2v8_video: regulator-vcc2v8-video {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc2v8-video";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> + vin-supply = <&vcc3v3_baseboard>;
> + };
> +};
> +
> +&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?
> + 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?
> + touchscreen@14 {
> + compatible = "goodix,gt911";
> + reg = <0x14>;
> + interrupt-parent = <&gpio3>;
> + interrupts = <RK_PC3 IRQ_TYPE_LEVEL_LOW>;
> + irq-gpios = <&gpio3 RK_PC3 GPIO_ACTIVE_HIGH>;
> + pinctrl-0 = <&touch_int>;
> + pinctrl-names = "default";
> + reset-gpios = <&pca9670 1 GPIO_ACTIVE_HIGH>;
So this is correct because of a cancelling mistake. The driver inverts
the signal and here we invert it as well. c.f.
https://lore.kernel.org/linux-rockchip/20221103-upstream-goodix-reset-v3-0-0975809eb183@theobroma-systems.com/
I haven't send a new version yet, I don't really know what should be
done there. The main pain point being that this is probably a big patch
that spans multiple maintainer trees at once?
> + AVDD28-supply = <&vcc2v8_video>;
> + VDDIO-supply = <&vcc3v3_baseboard>;
> + };
> +
> + pca9670: gpio@27 {
> + compatible = "nxp,pca9670";
> + reg = <0x27>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +};
> +
> +&mipidcphy0 {
> + status = "okay";
> +};
> +
> +&pinctrl {
> + touch {
> + touch_int: touch-int {
> + rockchip,pins = <3 RK_PC3 RK_FUNC_GPIO &pcfg_pull_none>;
> + };
> + };
> +};
> +
> +&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.
> + status = "okay";
> +};
> +
> +&vp3 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
Shouldn't those be in the SoC dtsi?
Cheers,
Quentin
Powered by blists - more mailing lists