linux-kernel - Re: [PATCH 2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial device tree

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <44a00fc6-d700-4713-34f0-36b172d8cb90@linaro.org>
Date:   Wed, 20 Sep 2023 16:47:30 +0200
From:   Konrad Dybcio <konrad.dybcio@...aro.org>
To:     Lukas Walter <lukas.walter@...art.de>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     ~postmarketos/upstreaming@...ts.sr.ht, phone-devel@...r.kernel.org,
        Raymond Hackley <raymondhackley@...tonmail.com>
Subject: Re: [PATCH 2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial
 device tree



On 9/16/23 15:41, Lukas Walter wrote:
> This dts adds support for Huawei Honor 5X / GR5 (2016) smartphone
> released in 2015.
> 
> Add device tree with initial support for:
> 
> - GPIO keys
> - Hall sensor
> - SDHCI (internal and external storage)
> - WCNSS (BT/WIFI)
> - Sensors (accelerometer, proximity and gyroscope)
> - Vibrator
> - Touchscreen
> 
> Signed-off-by: Lukas Walter <lukas.walter@...art.de>
> Signed-off-by: Raymond Hackley <raymondhackley@...tonmail.com>
> ---
Beyond the signoff question from Krzysztof, this looks really good.
Some comments below.

[...]

> +
> +	reserved-memory {
> +		reserved@...00000 {
> +			reg = <0x0 0x84a00000 0x0 0x1600000>;
> +			no-map;
> +		};
Do we know what this is for?


> +	};
> +
> +	gpio-hall-sensor {
> +		compatible = "gpio-keys";
> +
> +		pinctrl-0 = <&gpio_hall_sensor_default>;
> +		pinctrl-names = "default";
> +
> +		label = "GPIO Hall Effect Sensor";
I think we can have both hall sensor and V+ under gpio-keys

And then I am not sure how useful the label is for the container
node, maybe you or somebody else can tell me whether it's used
anywhere
> +
> +		event-hall-sensor {
> +			label = "Hall Effect Sensor";
> +			gpios = <&tlmm 69 GPIO_ACTIVE_LOW>;
> +			linux,input-type = <EV_SW>;
> +			linux,code = <SW_LID>;
> +			linux,can-disable;
Should this not be a wakeup-source btw?

> +		};
> +	};
> +
[...]

> +		/*
> +		 * NOTE: vdd is not directly supplied by pm8916_l16, it seems to be a
> +		 * fixed regulator that is automatically enabled by pm8916_l16.
That sounds reasonable, many boards have such circuits

Konrad