Re: [PATCH 3/3] arm64: dts: qcom: msm8916-wiko-chuppito: add initial devicetree

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 2/15/26 9:02 PM, Paul Adam via B4 Relay wrote:
> From: Paul Adam <adamp@posteo.de>
> 
> Add an initial device tree for Wiko PULP 4G.
> Includes support for:
> - UART
> - USB (no OTG)
> - Internal storage
> - MicroSD
> - Volume keys + Power button
> - Touchscreen
> - Backlight
> - Accelerometer: Invensense MPU6880
> - Magnetometer: Asahi Kasei AK09911
> - Hall sensor: Rohm BU52021HFV
> - Proximity sensor
> - Vibrator
> - Earpiece
> - Microphone 1
> - Headphones
> - Wifi
> - Bluetooth
> - GPU
> 
> Signed-off-by: Paul Adam <adamp@posteo.de>
> ---

Hello, just a couple comments below

[...]

> +	chosen {
> +		stdout-path = "serial0";
> +	};
> +	backlight: backlight {

Please add a \n above to separate these blocks

> +		compatible = "pwm-backlight";
> +		pwms = <&pm8916_pwm 0 100000>;
> +		brightness-levels = <0 255>;
> +		num-interpolated-steps = <255>;
> +		default-brightness-level = <255>;
> +		enable-gpios = <&tlmm 119 GPIO_ACTIVE_HIGH>;
> +		pinctrl-0 = <&button_backlight_default>;
> +		pinctrl-1 = <&button_backlight_off>;
> +		pinctrl-names = "default", "off";
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +		pinctrl-0 = <&gpio_keys_default>;
> +		pinctrl-names = "default";
> +		label = "GPIO Buttons";

I think the GPIO prefix is unnecesary here and below
> +
> +		button-volume-up {
> +			label = "Volume up";
> +			gpios = <&tlmm 107 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_VOLUMEUP>;
> +		};
> +	};
> +
> +	gpio-hall-sensor {
> +		compatible = "gpio-keys";
> +		pinctrl-0 = <&gpio_hall_sensor_default>;
> +		pinctrl-names = "default";
> +		label = "GPIO Hall Effect Sensor";

[...]

> +	proximity@48 {
> +		// downstream devicetree only specifies "stk,stk3x1x"
> +		compatible = "sensortek,stk3310";

Looks like the driver has a number of CHIP_IDs - you can be sure which chip
specifically this is by printing it out

[...]

> +// make sure gcc is probed with display disabled.
> +&gcc {
> +	clocks = <&xo_board>, <&sleep_clk>, <0>, <0>, <0>, <0>, <0>;
> +};

Do things break if you remove this line?

> +
> +&gpu {
> +	status = "okay";
> +};
> +
> +&mdss {
> +	status = "okay";
> +};
> +
> +&mpss_mem {
> +	reg = <0x0 0x86800000 0x0 0x5600000>;
> +};
> +
> +&pm8916_l6 {
> +	regulator-always-on;

Is there a reason for this?

[...]

> +&sound {
> +	audio-routing =
> +		"AMIC1", "MIC BIAS Internal1",

Please unbreak this line after '='

Konrad