Re: [PATCH 5/7] arm64: dts: qcom: Add LeEco Le2

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

On 8/10/26 6:55 PM, Adam Skladowski wrote:
> Add a devicetree for the LeEco Le2 smartphone, which is based
> on the Eldarion/MSM8976 SoC.

[...]

> +	reserved-memory {
> +
> +		ramoops {

Stray \n above

> +			compatible = "ramoops";
> +			no-map;
> +			reg = <0 0x9ff00000 0 0x100000>;
> +			record-size = <0x10000>;
> +			console-size = <0x60000>;
> +			ftrace-size = <0x10000>;
> +			pmsg-size = <0x20000>;
> +			ecc-size = <16>;
> +		};
> +
> +		/delete-node/ memory@83000000;

Referring to nodes by their node names is fragile. In this case, you fell
victim to that since this node doesn't exist. The more modern approach is
to assign a label at the definition site and then do /delete-node/ &foo

[...]

> +&blsp1_i2c4 {
> +	status = "okay";
> +
> +	rmi4-i2c-dev@4b {

Node names must be generic, so touchscreen@

> +		reg = <0x4b>;
> +		compatible = "syna,rmi4-i2c";
> +		interrupts-extended = <&tlmm 65 0x2008>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		vdd-supply = <&pm8950_l17>;
> +		vio-supply = <&pm8950_l6>;
> +
> +		syna,reset-delay-ms = <100>;
> +		syna,startup-delay-ms = <500>;
> +		rmi4-f01@1 {

Let's keep a \n between properties and the following subnodes

[...]

> +&blsp2_i2c2 {
> +	status = "okay";
> +
> +	haptics: haptics@5a {
> +		compatible = "ti,drv2604";
> +		reg = <0x5a>;
> +		enable-gpios = <&tlmm 109 GPIO_ACTIVE_HIGH>;
> +		mode = <DRV260X_LRA_MODE>;
> +		library-sel = <DRV260X_LIB_LRA>;
> +	};
> +
> +	tiusb320: tusb320@47 {

I'm not super sure what this chip is, typec@ or extcon@?


> +		compatible = "ti,tusb320";
> +		reg = <0x47>;
> +		interrupts-extended = <&tlmm 21 IRQ_TYPE_EDGE_RISING>;
> +	};
> +};
> +
> +&blsp2_i2c4 {
> +	status = "okay";
> +	led-controller@45 {

Let's add a \n beforehand


> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		compatible = "awinic,aw2013";
> +		reg = <0x45>;
> +		vcc-supply = <&pm8950_l10>;
> +
> +		led@0 {
> +			reg = <0>;
> +			led-max-microamp = <5000>;
> +			function = LED_FUNCTION_INDICATOR;
> +			color = <LED_COLOR_ID_RED>;
> +		};
> +		led@1 {

And an \n between each subnode

[...]

> +&otg {
> +	status = "okay";

Let's keep 'status' last and with a \n before it
> +	extcon = <&tiusb320>,<0>;

The second extcon is optional, you can drop it

from the chipidea bindings:

    minItems: 1
    items:
      - description: vbus extcon
        maxItems: 1
      - description: id extcon
        maxItems: 1

Konrad