Re: [PATCH 10/10] arm64: dts: qcom: Add AYN Odin 2

Krzysztof Kozlowski
2024-04-25T06:28:11Z

On 24/04/2024 17:29, Xilin Wu via B4 Relay wrote:
> From: Xilin Wu <wuxilin123@gmail.com>
>
> AYN Odin 2 is a gaming handheld based on QCS8550, which is derived
> from SM8550 but without modem RF system.
>

> +
> +/ {
> + model = "AYN Odin 2";
> + compatible = "ayn,odin2", "qcom,qcs8550", "qcom,sm8550";
> + chassis-type = "handset";
> +
> + qcom,msm-id = <QCOM_ID_QCS8550 0x20000>;
> + qcom,board-id = <0x1001f 0>;

No, these are not allowed. You did not test your dts.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

> +
> + aliases {
> + serial0 = &uart7;
> + serial1 = &uart14;
> + serial2 = &uart15;
> + };
> +
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + pwms = <&pmk8550_pwm 0 860000>;
> + brightness-levels = <1023 0>;
> + num-interpolated-steps = <1023>;
> + default-brightness-level = <600>;
> + power-supply = <&vph_pwr>;
> + enable-gpios = <&pmk8550_gpios 5 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pwm_backlight_default>;
> + status = "okay";

Drop, why do you need it? Do you see it anywhere else in the backlight
nodes in DTS?

> + };
> +
> + fan_pwr: fan-pwr-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "fan_pwr";
> +
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> +
> + gpio = <&tlmm 109 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&fan_pwr_en>;
> +
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> +
> + pinctrl-0 = <&volume_up_n>, <&m1_m2_keys_default>;
> + pinctrl-names = "default";
> +
> + key-volume-up {
> + label = "Volume Up";
> + linux,code = <KEY_VOLUMEUP>;
> + gpios = <&pm8550_gpios 6 GPIO_ACTIVE_LOW>;
> + debounce-interval = <15>;
> + linux,can-disable;
> + wakeup-source;
> + };
> +
> + m1-button {
> + label = "M1";
> + linux,code = <BTN_TRIGGER_HAPPY1>;
> + gpios = <&tlmm 57 GPIO_ACTIVE_LOW>;
> + };
> +
> + m2-button {
> + label = "M2";
> + linux,code = <BTN_TRIGGER_HAPPY2>;
> + gpios = <&tlmm 58 GPIO_ACTIVE_LOW>;
> + };
> + };
> +
> + hdmi-out {
> + compatible = "hdmi-connector";
> + type = "d";
> + hpd-gpios = <&tlmm 9 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + hdmi_con: endpoint {
> + remote-endpoint = <&lt8912_out>;
> + };
> + };
> + };
> +
> + hdmi_pwr: hdmi-pwr-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "hdmi_pwr";
> +
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> +
> + gpio = <&tlmm 10 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + vdd_lcm_2p8: vdd-lcm-2p8-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "vdd_lcm_2p8";
> +
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> +
> + gpio = <&tlmm 142 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + led_left_side: led-controller-1 {
> + compatible = "pwm-leds-multicolor";
> +
> + multi-led {
> + label = "left-side";
> + color = <LED_COLOR_ID_RGB>;
> + max-brightness = <255>;
> +
> + led-red {
> + color = <LED_COLOR_ID_RED>;
> + pwms = <&pwm_rgb_left 0>;
> + };
> +
> + led-green {
> + color = <LED_COLOR_ID_GREEN>;
> + pwms = <&pwm_rgb_left 1>;
> + };
> +
> + led-blue {
> + color = <LED_COLOR_ID_BLUE>;
> + pwms = <&pwm_rgb_left 2>;
> + };
> + };
> + };
> +
> + led_left_joystick: led-controller-2 {
> + compatible = "pwm-leds-multicolor";
> +
> + multi-led {
> + label = "left-joystick";
> + color = <LED_COLOR_ID_RGB>;
> + max-brightness = <255>;
> +
> + led-red {
> + color = <LED_COLOR_ID_RED>;
> + pwms = <&pwm_rgb_left 6>;
> + };
> +
> + led-green {
> + color = <LED_COLOR_ID_GREEN>;
> + pwms = <&pwm_rgb_left 7>;
> + };
> +
> + led-blue {
> + color = <LED_COLOR_ID_BLUE>;
> + pwms = <&pwm_rgb_left 8>;
> + };
> + };
> + };
> +
> + led_right_side: led-controller-3 {
> + compatible = "pwm-leds-multicolor";
> +
> + multi-led {
> + label = "right-side";
> + color = <LED_COLOR_ID_RGB>;
> + max-brightness = <255>;
> +
> + led-red {
> + color = <LED_COLOR_ID_RED>;
> + pwms = <&pwm_rgb_right 0>;
> + };
> +
> + led-green {
> + color = <LED_COLOR_ID_GREEN>;
> + pwms = <&pwm_rgb_right 1>;
> + };
> +
> + led-blue {
> + color = <LED_COLOR_ID_BLUE>;
> + pwms = <&pwm_rgb_right 2>;
> + };
> + };
> + };
> +
> + led_right_joystick: led-controller-4 {
> + compatible = "pwm-leds-multicolor";
> +
> + multi-led {
> + label = "right-joystick";
> + color = <LED_COLOR_ID_RGB>;
> + max-brightness = <255>;
> +
> + led-red {
> + color = <LED_COLOR_ID_RED>;
> + pwms = <&pwm_rgb_right 6>;
> + };
> +
> + led-green {
> + color = <LED_COLOR_ID_GREEN>;
> + pwms = <&pwm_rgb_right 7>;
> + };
> +
> + led-blue {
> + color = <LED_COLOR_ID_BLUE>;
> + pwms = <&pwm_rgb_right 8>;
> + };
> + };
> + };
> +
> + mcu_3v3: mcu-3v3-regulator {

Name all regulators regulator-n, where n is decimal number. Then order
the nodes by name.

...

> +
> +&i2c4 {
> + clock-frequency = <400000>;
> + status = "okay";
> +
> + touchscreen@20 {
> + compatible = "syna,rmi4-i2c";
> + reg = <0x20>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupts-extended = <&tlmm 25 0x2008>;
> +
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&ts_int_default>;
> + pinctrl-1 = <&ts_int_sleep>;
> +
> + vio-supply = <&vreg_l12b_1p8>;
> +
> + syna,startup-delay-ms = <200>;
> + syna,reset-delay-ms = <200>;
> +
> + rmi4-f01@1 {
> + syna,nosleep-mode = <0x1>;
> + reg = <0x1>;
> + };
> +
> + rmi4-f12@12 {
> + reg = <0x12>;
> + syna,rezero-wait-ms = <20>;
> + syna,clip-x-low = <0>;
> + syna,clip-y-low = <0>;
> + syna,clip-x-high = <1080>;
> + syna,clip-y-high = <1920>;
> + syna,sensor-type = <1>;
> + touchscreen-inverted-x;
> + };
> + };

Please confirm the status of dtbs_check for your board. I am pretty sure
it fails.

Best regards,
Krzysztof