Re: [PATCH 2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial device tree
On 10/21/23 16:30, 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 and proximity) - Vibrator - Touchscreen Signed-off-by: Raymond Hackley Signed-off-by: Lukas Walter --- Reviewed-by: Konrad Dybcio Konrad
Re: [PATCH 2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial device tree
>> Is this fine? >> Should I send a V2 with the signoff and reserved-memory changes? >I don't quite get it, what signoff? Krzysztof noticed that the Signed-off-by are in the wrong order. But I was told this alone is not worth a V2.
Re: [PATCH 2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial device tree
On 25.09.2023 16:28, lukas walter wrote:
> Date: Wed, 20 Sep 2023 16:47:30 +0200
>
>>> +
>>> + reserved-memory {
>>> + reserved@84a0 {
>>> + reg = <0x0 0x84a0 0x0 0x160>;
>>> + no-map;
>>> + };
>> Do we know what this is for?
>
> This seems to be some QSEE/TrustZone memory required to boot.
> I would name it `qseecom_mem: qseecom@84a0` like other phones
> currently have it.
>
> `[1.162115] QSEECOM: qseecom_probe: secure app region
> addr=0x84a0 size=0x190`
Sounds good!
>
>>> + };
>>> +
>>> + gpio-hall-sensor {
>>> + compatible = "gpio-keys";
>>> +
>>> + pinctrl-0 = <_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 = < 69 GPIO_ACTIVE_LOW>;
>>> + linux,input-type = ;
>>> + linux,code = ;
>>> + linux,can-disable;
>> Should this not be a wakeup-source btw?
>
> I am not sure how to change this. I would like to leave this as many
> other hall sensors seem to be configured identically.
Krzysztof, opinions?
>
> Is this fine?
> Should I send a V2 with the signoff and reserved-memory changes?
I don't quite get it, what signoff?
Konrad
Re: [PATCH 2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial device tree
Date: Wed, 20 Sep 2023 16:47:30 +0200
>> +
>> +reserved-memory {
>> +reserved@84a0 {
>> +reg = <0x0 0x84a0 0x0 0x160>;
>> +no-map;
>> +};
>Do we know what this is for?
This seems to be some QSEE/TrustZone memory required to boot.
I would name it `qseecom_mem: qseecom@84a0` like other phones
currently have it.
`[1.162115] QSEECOM: qseecom_probe: secure app region
addr=0x84a0 size=0x190`
>> +};
>> +
>> +gpio-hall-sensor {
>> +compatible = "gpio-keys";
>> +
>> +pinctrl-0 = <_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 = < 69 GPIO_ACTIVE_LOW>;
>> +linux,input-type = ;
>> +linux,code = ;
>> +linux,can-disable;
>Should this not be a wakeup-source btw?
I am not sure how to change this. I would like to leave this as many
other hall sensors seem to be configured identically.
Is this fine?
Should I send a V2 with the signoff and reserved-memory changes?
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
Signed-off-by: Raymond Hackley
---
Beyond the signoff question from Krzysztof, this looks really good.
Some comments below.
[...]
+
+ reserved-memory {
+ reserved@84a0 {
+ reg = <0x0 0x84a0 0x0 0x160>;
+ no-map;
+ };
Do we know what this is for?
+ };
+
+ gpio-hall-sensor {
+ compatible = "gpio-keys";
+
+ pinctrl-0 = <_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 = < 69 GPIO_ACTIVE_LOW>;
+ linux,input-type = ;
+ linux,code = ;
+ 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
Re: [PATCH 2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial device tree
>Are you sure this is 3620, have you tried wcn3660 and/or wcn3680 ? I am sure. Downstream source [1] and downstream dmesg (wcnss: IRIS Reg: 51120004 which should equal [2]) indicate 3620 (3620A does not exist) [1]: https://github.com/CyanogenMod/android_kernel_huawei_kiwi/blob/cm-14.1/arch/arm/boot/dts/qcom/huawei_msm8939_kiw_al20_vb/huawei-bt.dtsi#L5 [2]: https://github.com/msm8916-mainline/linux-downstream/blob/b20608408caff817ec874f325127b07609fbaeb8/drivers/net/wireless/wcnss/wcnss_vreg.c#L51
Re: [PATCH 2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial device tree
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 >Order of SoB is unusual. Who did what here? I created the dts and they update the model name. So it should be the other way around -BEGIN PGP SIGNATURE- iQIzBAEBCAAdFiEEi1ngOOsyNO1iyMXiY16HCsLx2zUFAmUG/FQACgkQY16HCsLx 2zXbug//cFZNScSBr6k1I4tHuthR2+CFhTqOO0x0EqMa6W+l96s7ZimJA8UGq2jJ F+4BZHujtBlpd1p4bE/GMDERy6MMHmAW7/N8kl3dEfwErwvjwML4nvDLRSC2Movd 93MNJ+4tncTndMLUEyARUUg5sP6Idgc/83pMRzKn4toFxWMHfWPFHy5XlNLDjJT5 /BuvYlbAFpme9ZavTLMGtalOp6ovsS1qNRrNk/CZecV+I5tCVFs4ZbzMHf8vKi3/ B7yObHRsZEdy80KE77kedgyBlGWu0/Tvu7xrK2qZxawkbraLBn+qE76wCmgk91qg 7EheFFr30v1LuudZbA+5fOE3qC13u3LbmCaRwwKAC8S7C6OgMM8Vcg08rPdaaV10 m1MDSPgxddA/TitQNz3epvFahpm1WWuGhR2xihDP/r345kogUYcRiLw4eqIkYKjb 64Wgalclq8XByymGLndYuoBaR31wauOjCtShiHByRTp86XTWh9C+KJ2wVGdE2UWL ok+qs1hrao328+FtgtZn7FxnmO2yLA9I6xLeEPgTSt5VSZ9NWpRNQ5h61AYDEaZV YPy3cTJV5VN5Mrp6dul7bT/Ti5rp8tCH5zk6D62WTSq2xEzkDtjnE1DKzr2+0KNt c2VZQGmquTiTotnQF2c4E2M9ONd+TxN9sn0dYqwwZa0BLEI1JBk= =F2cY -END PGP SIGNATURE-
Re: [PATCH 2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial device tree
On 16/09/2023 14:41, Lukas Walter wrote:
+ +_iris { + compatible = "qcom,wcn3620"; +}; +
Are you sure this is 3620, have you tried wcn3660 and/or wcn3680 ?
---
bod
Re: [PATCH 2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial device tree
On 16/09/2023 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 > Signed-off-by: Raymond Hackley Order of SoB is unusual. Who did what here? Best regards, Krzysztof
