* [PATCH v3 1/4] ARM: dts: qcom: msm8960: expressatt: Sort node references and includes
2026-04-01 20:32 [PATCH v3 0/4] Reorganize DTS and add fuel-gauge to samsung-expressatt Rudraksha Gupta via B4 Relay
@ 2026-04-01 20:32 ` Rudraksha Gupta via B4 Relay
2026-04-02 8:53 ` Konrad Dybcio
2026-04-07 19:59 ` Krzysztof Kozlowski
2026-04-01 20:32 ` [PATCH v3 2/4] ARM: dts: qcom: msm8960: Flatten I2C pinctrl state subnodes Rudraksha Gupta via B4 Relay
` (2 subsequent siblings)
3 siblings, 2 replies; 12+ messages in thread
From: Rudraksha Gupta via B4 Relay @ 2026-04-01 20:32 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Rudraksha Gupta
From: Rudraksha Gupta <guptarud@gmail.com>
Reorganize the DTS file for consistency with other msm8960 board files.
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
---
.../dts/qcom/qcom-msm8960-samsung-expressatt.dts | 408 +++++++++++----------
1 file changed, 207 insertions(+), 201 deletions(-)
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts b/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
index 35514fd53e3d..0e6959f7af6f 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
+++ b/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
@@ -1,13 +1,13 @@
// SPDX-License-Identifier: GPL-2.0
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/gpio-keys.h>
#include <dt-bindings/input/input.h>
#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
#include <dt-bindings/reset/qcom,gcc-msm8960.h>
#include "qcom-msm8960.dtsi"
#include "pm8921.dtsi"
-#include <dt-bindings/gpio/gpio.h>
-#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
-#include <dt-bindings/input/gpio-keys.h>
/ {
model = "Samsung Galaxy Express SGH-I437";
@@ -27,8 +27,8 @@ chosen {
gpio-keys {
compatible = "gpio-keys";
- pinctrl-names = "default";
pinctrl-0 = <&gpio_keys_pin_a>;
+ pinctrl-names = "default";
key-home {
label = "Home";
@@ -54,58 +54,25 @@ key-volume-down {
};
};
- touchkey_enable: touchkey-enable {
- compatible = "regulator-fixed";
- regulator-name = "touchkey_enable";
- gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
- enable-active-high;
- regulator-boot-on;
- };
-
- vreg_flash: regulator-flash {
- compatible = "regulator-fixed";
- regulator-name = "VREG_FLASH_3P3";
- regulator-min-microvolt = <3300000>;
- regulator-max-microvolt = <3300000>;
- gpio = <&pm8921_mpps 4 GPIO_ACTIVE_HIGH>;
- enable-active-high;
- pinctrl-0 = <&flash_led_unlock>;
- pinctrl-names = "default";
- };
-
- led-controller {
- compatible = "richtek,rt8515";
- enf-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
- vin-supply = <&vreg_flash>;
- richtek,rfs-ohms = <16000>;
- pinctrl-0 = <&cam_flash_en>;
- pinctrl-names = "default";
-
- led {
- function = LED_FUNCTION_FLASH;
- color = <LED_COLOR_ID_WHITE>;
- flash-max-timeout-us = <250000>;
- };
- };
-
i2c-gpio-touchkey {
compatible = "i2c-gpio";
#address-cells = <1>;
#size-cells = <0>;
sda-gpios = <&tlmm 71 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
scl-gpios = <&tlmm 72 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
- pinctrl-names = "default";
pinctrl-0 = <&touchkey_i2c_pins>;
- status = "okay";
+ pinctrl-names = "default";
i2c-gpio,delay-us = <2>;
+ status = "okay";
+
touchkey@20 {
compatible = "coreriver,tc360-touchkey";
reg = <0x20>;
interrupts-extended = <&tlmm 52 IRQ_TYPE_EDGE_FALLING>;
- pinctrl-names = "default";
pinctrl-0 = <&touchkey_irq_pin>;
+ pinctrl-names = "default";
vddio-supply = <&touchkey_enable>;
vdd-supply = <&pm8921_l29>;
@@ -114,6 +81,52 @@ touchkey@20 {
linux,keycodes = <KEY_MENU KEY_BACK>;
};
};
+
+ led-controller {
+ compatible = "richtek,rt8515";
+ enf-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
+ vin-supply = <&vreg_flash>;
+ richtek,rfs-ohms = <16000>;
+ pinctrl-0 = <&cam_flash_en>;
+ pinctrl-names = "default";
+
+ led {
+ function = LED_FUNCTION_FLASH;
+ color = <LED_COLOR_ID_WHITE>;
+ flash-max-timeout-us = <250000>;
+ };
+ };
+
+ vreg_flash: regulator-flash {
+ compatible = "regulator-fixed";
+ regulator-name = "VREG_FLASH_3P3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&pm8921_mpps 4 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ pinctrl-0 = <&flash_led_unlock>;
+ pinctrl-names = "default";
+ };
+
+ touchkey_enable: touchkey-enable {
+ compatible = "regulator-fixed";
+ regulator-name = "touchkey_enable";
+ gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-boot-on;
+ };
+};
+
+&gsbi1 {
+ qcom,mode = <GSBI_PROT_SPI>;
+ pinctrl-0 = <&spi1_default>;
+ pinctrl-names = "default";
+
+ status = "okay";
+};
+
+&gsbi1_spi {
+ status = "okay";
};
&gsbi2 {
@@ -141,51 +154,16 @@ light-sensor@39 {
};
};
-&gsbi5 {
- qcom,mode = <GSBI_PROT_I2C_UART>;
- status = "okay";
-};
-
-&gsbi5_serial {
- status = "okay";
-};
-
-&sdcc1 {
- vmmc-supply = <&pm8921_l5>;
- status = "okay";
-};
-
-&sdcc3 {
- vmmc-supply = <&pm8921_l6>;
- vqmmc-supply = <&pm8921_l7>;
-
- pinctrl-0 = <&sdcc3_default_state>;
- pinctrl-1 = <&sdcc3_sleep_state>;
- pinctrl-names = "default", "sleep";
-
- status = "okay";
-};
-
-&gsbi1 {
- qcom,mode = <GSBI_PROT_SPI>;
- pinctrl-0 = <&spi1_default>;
- pinctrl-names = "default";
- status = "okay";
-};
-
-&gsbi1_spi {
- status = "okay";
-};
-
&gsbi3 {
qcom,mode = <GSBI_PROT_I2C>;
+
status = "okay";
};
&gsbi3_i2c {
status = "okay";
- // Atmel mXT224S touchscreen
+ /* Atmel mXT224S touchscreen */
touchscreen@4a {
compatible = "atmel,maxtouch";
reg = <0x4a>;
@@ -193,93 +171,69 @@ touchscreen@4a {
interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
vdda-supply = <&pm8921_lvs6>;
vdd-supply = <&pm8921_l17>;
- pinctrl-names = "default";
pinctrl-0 = <&touchscreen>;
+ pinctrl-names = "default";
};
};
-&tlmm {
- cam_flash_en: cam-flash-en-state {
- pins = "gpio3";
- function = "gpio";
- drive-strength = <16>;
- bias-pull-down;
- };
+&gsbi5 {
+ qcom,mode = <GSBI_PROT_I2C_UART>;
- spi1_default: spi1-default-state {
- mosi-pins {
- pins = "gpio6";
- function = "gsbi1";
- drive-strength = <12>;
- bias-disable;
- };
+ status = "okay";
+};
- miso-pins {
- pins = "gpio7";
- function = "gsbi1";
- drive-strength = <12>;
- bias-disable;
- };
+&gsbi5_serial {
+ status = "okay";
+};
- cs-pins {
- pins = "gpio8";
- function = "gsbi1";
- drive-strength = <12>;
- bias-disable;
- output-low;
- };
+&gsbi7 {
+ qcom,mode = <GSBI_PROT_I2C>;
- clk-pins {
- pins = "gpio9";
- function = "gsbi1";
- drive-strength = <12>;
- bias-disable;
- };
- };
+ status = "okay";
+};
- gpio_keys_pin_a: gpio-keys-active-state {
- pins = "gpio40", "gpio50", "gpio81";
- function = "gpio";
- drive-strength = <8>;
- bias-disable;
- };
+&gsbi7_i2c {
+ status = "okay";
- touchscreen: touchscreen-int-state {
- pins = "gpio11";
- function = "gpio";
- output-enable;
- bias-disable;
- drive-strength = <2>;
+ nfc@2b {
+ compatible = "nxp,pn544-i2c";
+ reg = <0x2b>;
+ interrupts-extended = <&tlmm 106 IRQ_TYPE_EDGE_RISING>;
+ enable-gpios = <&pm8921_gpio 21 GPIO_ACTIVE_HIGH>;
+ firmware-gpios = <&tlmm 92 GPIO_ACTIVE_HIGH>;
+
+ pinctrl-0 = <&nfc_default &nfc_enable>;
+ pinctrl-names = "default";
};
+};
- nfc_default: nfc-default-state {
- irq-pins {
- pins = "gpio106";
- function = "gpio";
- drive-strength = <2>;
- bias-pull-down;
- };
+&gsbi12 {
+ qcom,mode = <GSBI_PROT_I2C>;
- firmware-pins {
- pins = "gpio92";
- function = "gpio";
- drive-strength = <2>;
- bias-disable;
- };
- };
+ status = "okay";
+};
- touchkey_i2c_pins: touchkey-i2c-state {
- pins = "gpio71", "gpio72";
- function = "gpio";
- drive-strength = <2>;
- bias-disable;
+&gsbi12_i2c {
+ status = "okay";
+
+ accelerometer@18 {
+ compatible = "bosch,bma254";
+ reg = <0x18>;
+ vdd-supply = <&pm8921_l9>;
+ vddio-supply = <&pm8921_lvs4>;
+
+ mount-matrix = "-1", "0", "0",
+ "0", "-1", "0",
+ "0", "0", "1";
};
- touchkey_irq_pin: touchkey-irq-state {
- pins = "gpio52";
- function = "gpio";
- drive-strength = <2>;
- bias-disable;
+ magnetometer@2e {
+ compatible = "yamaha,yas532";
+ reg = <0x2e>;
+ vdd-supply = <&pm8921_l9>;
+ iovdd-supply = <&pm8921_lvs4>;
+
+ /* TODO: Figure out Mount Matrix */
};
};
@@ -288,18 +242,27 @@ &pm8921 {
};
&pm8921_gpio {
+ nfc_enable: nfc-enable-state {
+ pins = "gpio21";
+ function = "normal";
+ bias-disable;
+ qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
+ power-source = <PM8921_GPIO_S4>;
+ };
+
prox_sensor_int: prox-sensor-int-state {
pins = "gpio6";
function = "normal";
input-enable;
bias-disable;
};
+};
- nfc_enable: nfc-enable-state {
- pins = "gpio21";
- function = "normal";
- bias-disable;
- qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
+&pm8921_mpps {
+ flash_led_unlock: flash-led-unlock-state {
+ pins = "mpp4";
+ function = "digital";
+ output-low;
power-source = <PM8921_GPIO_S4>;
};
};
@@ -546,72 +509,115 @@ pm8921_ncp: ncp {
};
};
-&usb_hs1_phy {
- v3p3-supply = <&pm8921_l3>;
- v1p8-supply = <&pm8921_l4>;
-};
+&sdcc1 {
+ vmmc-supply = <&pm8921_l5>;
-&usb1 {
- dr_mode = "otg";
status = "okay";
};
-&gsbi7 {
- qcom,mode = <GSBI_PROT_I2C>;
+&sdcc3 {
+ vmmc-supply = <&pm8921_l6>;
+ vqmmc-supply = <&pm8921_l7>;
+
+ pinctrl-0 = <&sdcc3_default_state>;
+ pinctrl-1 = <&sdcc3_sleep_state>;
+ pinctrl-names = "default", "sleep";
status = "okay";
};
-&gsbi7_i2c {
- status = "okay";
+&tlmm {
+ cam_flash_en: cam-flash-en-state {
+ pins = "gpio3";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-pull-down;
+ };
- nfc@2b {
- compatible = "nxp,pn544-i2c";
- reg = <0x2b>;
- interrupts-extended = <&tlmm 106 IRQ_TYPE_EDGE_RISING>;
- enable-gpios = <&pm8921_gpio 21 GPIO_ACTIVE_HIGH>;
- firmware-gpios = <&tlmm 92 GPIO_ACTIVE_HIGH>;
+ spi1_default: spi1-default-state {
+ mosi-pins {
+ pins = "gpio6";
+ function = "gsbi1";
+ drive-strength = <12>;
+ bias-disable;
+ };
- pinctrl-0 = <&nfc_default &nfc_enable>;
- pinctrl-names = "default";
- };
-};
+ miso-pins {
+ pins = "gpio7";
+ function = "gsbi1";
+ drive-strength = <12>;
+ bias-disable;
+ };
-&gsbi12 {
- qcom,mode = <GSBI_PROT_I2C>;
+ cs-pins {
+ pins = "gpio8";
+ function = "gsbi1";
+ drive-strength = <12>;
+ bias-disable;
+ output-low;
+ };
- status = "okay";
-};
+ clk-pins {
+ pins = "gpio9";
+ function = "gsbi1";
+ drive-strength = <12>;
+ bias-disable;
+ };
+ };
-&gsbi12_i2c {
- status = "okay";
+ touchscreen: touchscreen-int-state {
+ pins = "gpio11";
+ function = "gpio";
+ output-enable;
+ bias-disable;
+ drive-strength = <2>;
+ };
- accelerometer@18 {
- compatible = "bosch,bma254";
- reg = <0x18>;
- vdd-supply = <&pm8921_l9>;
- vddio-supply = <&pm8921_lvs4>;
+ gpio_keys_pin_a: gpio-keys-active-state {
+ pins = "gpio40", "gpio50", "gpio81";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-disable;
+ };
- mount-matrix = "-1", "0", "0",
- "0", "-1", "0",
- "0", "0", "1";
+ touchkey_irq_pin: touchkey-irq-state {
+ pins = "gpio52";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
};
- magnetometer@2e {
- compatible = "yamaha,yas532";
- reg = <0x2e>;
- vdd-supply = <&pm8921_l9>;
- iovdd-supply = <&pm8921_lvs4>;
+ touchkey_i2c_pins: touchkey-i2c-state {
+ pins = "gpio71", "gpio72";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
- /* TODO: Figure out Mount Matrix */
+ nfc_default: nfc-default-state {
+ firmware-pins {
+ pins = "gpio92";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ irq-pins {
+ pins = "gpio106";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
};
};
-&pm8921_mpps {
- flash_led_unlock: flash-led-unlock-state {
- pins = "mpp4";
- function = "digital";
- output-low;
- power-source = <PM8921_GPIO_S4>;
- };
+&usb_hs1_phy {
+ v3p3-supply = <&pm8921_l3>;
+ v1p8-supply = <&pm8921_l4>;
+};
+
+&usb1 {
+ dr_mode = "otg";
+
+ status = "okay";
};
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 1/4] ARM: dts: qcom: msm8960: expressatt: Sort node references and includes
2026-04-01 20:32 ` [PATCH v3 1/4] ARM: dts: qcom: msm8960: expressatt: Sort node references and includes Rudraksha Gupta via B4 Relay
@ 2026-04-02 8:53 ` Konrad Dybcio
2026-04-07 19:59 ` Krzysztof Kozlowski
1 sibling, 0 replies; 12+ messages in thread
From: Konrad Dybcio @ 2026-04-02 8:53 UTC (permalink / raw)
To: guptarud, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
On 4/1/26 10:32 PM, Rudraksha Gupta via B4 Relay wrote:
> From: Rudraksha Gupta <guptarud@gmail.com>
>
> Reorganize the DTS file for consistency with other msm8960 board files.
>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/4] ARM: dts: qcom: msm8960: expressatt: Sort node references and includes
2026-04-01 20:32 ` [PATCH v3 1/4] ARM: dts: qcom: msm8960: expressatt: Sort node references and includes Rudraksha Gupta via B4 Relay
2026-04-02 8:53 ` Konrad Dybcio
@ 2026-04-07 19:59 ` Krzysztof Kozlowski
2026-04-07 20:39 ` Rudraksha Gupta
1 sibling, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-04-07 19:59 UTC (permalink / raw)
To: guptarud, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
On 01/04/2026 22:32, Rudraksha Gupta via B4 Relay wrote:
> From: Rudraksha Gupta <guptarud@gmail.com>
>
> Reorganize the DTS file for consistency with other msm8960 board files.
>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
> ---
> .../dts/qcom/qcom-msm8960-samsung-expressatt.dts | 408 +++++++++++----------
> 1 file changed, 207 insertions(+), 201 deletions(-)
>
Sorry, but no. We are not taking Claude as one determining coding style.
Are we going to do the work again the moment we come with proper tool?
Same comment as usually - this is highly invasive change which should be
done with dts linter, not LLM, not manually.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/4] ARM: dts: qcom: msm8960: expressatt: Sort node references and includes
2026-04-07 19:59 ` Krzysztof Kozlowski
@ 2026-04-07 20:39 ` Rudraksha Gupta
2026-04-07 21:04 ` Krzysztof Kozlowski
0 siblings, 1 reply; 12+ messages in thread
From: Rudraksha Gupta @ 2026-04-07 20:39 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
On 4/7/26 12:59, Krzysztof Kozlowski wrote:
> On 01/04/2026 22:32, Rudraksha Gupta via B4 Relay wrote:
>> From: Rudraksha Gupta <guptarud@gmail.com>
>>
>> Reorganize the DTS file for consistency with other msm8960 board files.
>>
>> Assisted-by: Claude:claude-opus-4.6
>> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
>> ---
>> .../dts/qcom/qcom-msm8960-samsung-expressatt.dts | 408 +++++++++++----------
>> 1 file changed, 207 insertions(+), 201 deletions(-)
>>
> Sorry, but no. We are not taking Claude as one determining coding style.
> Are we going to do the work again the moment we come with proper tool?
There is no tool currently to auto format DTS, and doesn't seem to be
coming for a while:
https://www.youtube.com/watch?v=cvoIbTL_ZQA
Claude didn't determine the coding style. I did based on sony-huashan,
which is already upstream:
https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/qcom/qcom-msm8960-sony-huashan.dts
I just used Claude to do the manual work for me. In v2, I made sure the
diff before and after the change was nill. v3 included additional
changes requested by Konrad and some comments that I remembered during
prior attempts mainlining patch series for this device.
>
> Same comment as usually - this is highly invasive change which should be
> done with dts linter, not LLM, not manually.
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/4] ARM: dts: qcom: msm8960: expressatt: Sort node references and includes
2026-04-07 20:39 ` Rudraksha Gupta
@ 2026-04-07 21:04 ` Krzysztof Kozlowski
2026-04-07 21:29 ` Rudraksha Gupta
2026-04-07 21:46 ` David Heidelberg
0 siblings, 2 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-04-07 21:04 UTC (permalink / raw)
To: Rudraksha Gupta, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
On 07/04/2026 22:39, Rudraksha Gupta wrote:
>
> On 4/7/26 12:59, Krzysztof Kozlowski wrote:
>> On 01/04/2026 22:32, Rudraksha Gupta via B4 Relay wrote:
>>> From: Rudraksha Gupta <guptarud@gmail.com>
>>>
>>> Reorganize the DTS file for consistency with other msm8960 board files.
>>>
>>> Assisted-by: Claude:claude-opus-4.6
>>> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
>>> ---
>>> .../dts/qcom/qcom-msm8960-samsung-expressatt.dts | 408 +++++++++++----------
>>> 1 file changed, 207 insertions(+), 201 deletions(-)
>>>
>> Sorry, but no. We are not taking Claude as one determining coding style.
>> Are we going to do the work again the moment we come with proper tool?
>
> There is no tool currently to auto format DTS, and doesn't seem to be
> coming for a while:
>
> https://www.youtube.com/watch?v=cvoIbTL_ZQA
>
>
> Claude didn't determine the coding style. I did based on sony-huashan,
> which is already upstream:
>
> https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/qcom/qcom-msm8960-sony-huashan.dts
>
>
> I just used Claude to do the manual work for me. In v2, I made sure the
> diff before and after the change was nill. v3 included additional
> changes requested by Konrad and some comments that I remembered during
> prior attempts mainlining patch series for this device.
IMO, it is just too risky to let Claude reorganize the nodes, but I
assume reviewers of your code did run dtx_diff.
If they did not, then it is on them :(
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/4] ARM: dts: qcom: msm8960: expressatt: Sort node references and includes
2026-04-07 21:04 ` Krzysztof Kozlowski
@ 2026-04-07 21:29 ` Rudraksha Gupta
2026-04-07 21:46 ` David Heidelberg
1 sibling, 0 replies; 12+ messages in thread
From: Rudraksha Gupta @ 2026-04-07 21:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
On 4/7/26 14:04, Krzysztof Kozlowski wrote:
> On 07/04/2026 22:39, Rudraksha Gupta wrote:
>> On 4/7/26 12:59, Krzysztof Kozlowski wrote:
>>> On 01/04/2026 22:32, Rudraksha Gupta via B4 Relay wrote:
>>>> From: Rudraksha Gupta <guptarud@gmail.com>
>>>>
>>>> Reorganize the DTS file for consistency with other msm8960 board files.
>>>>
>>>> Assisted-by: Claude:claude-opus-4.6
>>>> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
>>>> ---
>>>> .../dts/qcom/qcom-msm8960-samsung-expressatt.dts | 408 +++++++++++----------
>>>> 1 file changed, 207 insertions(+), 201 deletions(-)
>>>>
>>> Sorry, but no. We are not taking Claude as one determining coding style.
>>> Are we going to do the work again the moment we come with proper tool?
>> There is no tool currently to auto format DTS, and doesn't seem to be
>> coming for a while:
>>
>> https://www.youtube.com/watch?v=cvoIbTL_ZQA
>>
>>
>> Claude didn't determine the coding style. I did based on sony-huashan,
>> which is already upstream:
>>
>> https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/qcom/qcom-msm8960-sony-huashan.dts
>>
>>
>> I just used Claude to do the manual work for me. In v2, I made sure the
>> diff before and after the change was nill. v3 included additional
>> changes requested by Konrad and some comments that I remembered during
>> prior attempts mainlining patch series for this device.
> IMO, it is just too risky to let Claude reorganize the nodes, but I
> assume reviewers of your code did run dtx_diff.
>
> If they did not, then it is on them :(
That's fair. I also re-ran my tests after DTS reorganization and they
all work:
https://wiki.postmarketos.org/wiki/Samsung_Galaxy_Express_SGH-I437_(samsung-expressatt)#Testing
Also, I think I'm the only oddball using this device too, so minimal
impact. Someone listed the I8730 model in the wiki page, but I believe
that's a MSM8930 device.
Hopefully that reassures you. :)
Thanks,
Rudraksha
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/4] ARM: dts: qcom: msm8960: expressatt: Sort node references and includes
2026-04-07 21:04 ` Krzysztof Kozlowski
2026-04-07 21:29 ` Rudraksha Gupta
@ 2026-04-07 21:46 ` David Heidelberg
1 sibling, 0 replies; 12+ messages in thread
From: David Heidelberg @ 2026-04-07 21:46 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rudraksha Gupta, Bjorn Andersson,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
On 07/04/2026 23:04, Krzysztof Kozlowski wrote:
> On 07/04/2026 22:39, Rudraksha Gupta wrote:
>>
>> On 4/7/26 12:59, Krzysztof Kozlowski wrote:
>>> On 01/04/2026 22:32, Rudraksha Gupta via B4 Relay wrote:
>>>> From: Rudraksha Gupta <guptarud@gmail.com>
>>>>
>>>> Reorganize the DTS file for consistency with other msm8960 board files.
>>>>
>>>> Assisted-by: Claude:claude-opus-4.6
>>>> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
>>>> ---
>>>> .../dts/qcom/qcom-msm8960-samsung-expressatt.dts | 408 +++++++++++----------
>>>> 1 file changed, 207 insertions(+), 201 deletions(-)
>>>>
>>> Sorry, but no. We are not taking Claude as one determining coding style.
>>> Are we going to do the work again the moment we come with proper tool?
>>
>> There is no tool currently to auto format DTS, and doesn't seem to be
>> coming for a while:
>>
>> https://www.youtube.com/watch?v=cvoIbTL_ZQA
>>
>>
>> Claude didn't determine the coding style. I did based on sony-huashan,
>> which is already upstream:
>>
>> https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/qcom/qcom-msm8960-sony-huashan.dts
>>
>>
>> I just used Claude to do the manual work for me. In v2, I made sure the
>> diff before and after the change was nill. v3 included additional
>> changes requested by Konrad and some comments that I remembered during
>> prior attempts mainlining patch series for this device.
>
> IMO, it is just too risky to let Claude reorganize the nodes, but I
> assume reviewers of your code did run dtx_diff.
I think it depends on the prompt. Since I’m performing many of the same tasks
repeatedly across multiple sdm845 devices, asking an LLM to do node-by-node
reorganization can be reasonably reviewable (at least when reviewing incremental
progress, not just the final diff).
I would prefer to do more of the sorting myself, but I find it quite tedious.
The diff tool struggles when similar or identical lines appear in different
nodes, which often results in a messy final diff (I noticed this in Sajattack’s
sdm845 LG patchset).
This leads me to an idea:
For these sorting cleanups, perhaps we could introduce a “squash mode”?
Contributors could submit commits per node, making the reorganization clearly
visible (and ensuring nothing is accidentally lost), and then the maintainer
could squash them into a single commit to avoid cluttering the git log.
What do you think?
>
> If they did not, then it is on them :(
>
> Best regards,
> Krzysztof
--
David Heidelberg
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/4] ARM: dts: qcom: msm8960: Flatten I2C pinctrl state subnodes
2026-04-01 20:32 [PATCH v3 0/4] Reorganize DTS and add fuel-gauge to samsung-expressatt Rudraksha Gupta via B4 Relay
2026-04-01 20:32 ` [PATCH v3 1/4] ARM: dts: qcom: msm8960: expressatt: Sort node references and includes Rudraksha Gupta via B4 Relay
@ 2026-04-01 20:32 ` Rudraksha Gupta via B4 Relay
2026-04-02 8:53 ` Konrad Dybcio
2026-04-01 20:32 ` [PATCH v3 3/4] ARM: dts: qcom: msm8960: Add GSBI5 I2C controller Rudraksha Gupta via B4 Relay
2026-04-01 20:32 ` [PATCH v3 4/4] ARM: dts: qcom: msm8960: expressatt: Add MAX17048 fuel gauge Rudraksha Gupta via B4 Relay
3 siblings, 1 reply; 12+ messages in thread
From: Rudraksha Gupta via B4 Relay @ 2026-04-01 20:32 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Rudraksha Gupta
From: Rudraksha Gupta <guptarud@gmail.com>
Remove unnecessary inner i2c*-pins {} wrapper nodes from the I2C
pinctrl state definitions. Properties are moved directly under the
-state {} node, consistent with modern DT style.
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
---
arch/arm/boot/dts/qcom/qcom-msm8960.dtsi | 140 +++++++++++++------------------
1 file changed, 56 insertions(+), 84 deletions(-)
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
index fd28401cebb5..6069fb925672 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
@@ -132,129 +132,101 @@ tlmm: pinctrl@800000 {
#interrupt-cells = <2>;
i2c1_default_state: i2c1-default-state {
- i2c1-pins {
- pins = "gpio8", "gpio9";
- function = "gsbi1";
- drive-strength = <8>;
- bias-disable;
- };
+ pins = "gpio8", "gpio9";
+ function = "gsbi1";
+ drive-strength = <8>;
+ bias-disable;
};
i2c1_sleep_state: i2c1-sleep-state {
- i2c1-pins {
- pins = "gpio8", "gpio9";
- function = "gpio";
- drive-strength = <2>;
- bias-bus-hold;
- };
+ pins = "gpio8", "gpio9";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-bus-hold;
};
i2c2_default_state: i2c2-default-state {
- i2c2-pins {
- pins = "gpio12", "gpio13";
- function = "gsbi2";
- drive-strength = <8>;
- bias-disable;
- };
+ pins = "gpio12", "gpio13";
+ function = "gsbi2";
+ drive-strength = <8>;
+ bias-disable;
};
i2c2_sleep_state: i2c2-sleep-state {
- i2c2-pins {
- pins = "gpio12", "gpio13";
- function = "gpio";
- drive-strength = <2>;
- bias-bus-hold;
- };
+ pins = "gpio12", "gpio13";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-bus-hold;
};
i2c3_default_state: i2c3-default-state {
- i2c3-pins {
- pins = "gpio16", "gpio17";
- function = "gsbi3";
- drive-strength = <8>;
- bias-disable;
- };
+ pins = "gpio16", "gpio17";
+ function = "gsbi3";
+ drive-strength = <8>;
+ bias-disable;
};
i2c3_sleep_state: i2c3-sleep-state {
- i2c3-pins {
- pins = "gpio16", "gpio17";
- function = "gpio";
- drive-strength = <2>;
- bias-bus-hold;
- };
+ pins = "gpio16", "gpio17";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-bus-hold;
};
i2c7_default_state: i2c7-default-state {
- i2c7-pins {
- pins = "gpio32", "gpio33";
- function = "gsbi7";
- drive-strength = <8>;
- bias-disable;
- };
+ pins = "gpio32", "gpio33";
+ function = "gsbi7";
+ drive-strength = <8>;
+ bias-disable;
};
i2c7_sleep_state: i2c7-sleep-state {
- i2c7-pins {
- pins = "gpio32", "gpio33";
- function = "gpio";
- drive-strength = <2>;
- bias-bus-hold;
- };
+ pins = "gpio32", "gpio33";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-bus-hold;
};
i2c8_default_state: i2c8-default-state {
- i2c8-pins {
- pins = "gpio36", "gpio37";
- function = "gsbi8";
- drive-strength = <8>;
- bias-disable;
- };
+ pins = "gpio36", "gpio37";
+ function = "gsbi8";
+ drive-strength = <8>;
+ bias-disable;
};
i2c8_sleep_state: i2c8-sleep-state {
- i2c8-pins {
- pins = "gpio36", "gpio37";
- function = "gpio";
- drive-strength = <2>;
- bias-bus-hold;
- };
+ pins = "gpio36", "gpio37";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-bus-hold;
};
i2c10_default_state: i2c10-default-state {
- i2c10-pins {
- pins = "gpio73", "gpio74";
- function = "gsbi10";
- drive-strength = <8>;
- bias-disable;
- };
+ pins = "gpio73", "gpio74";
+ function = "gsbi10";
+ drive-strength = <8>;
+ bias-disable;
};
i2c10_sleep_state: i2c10-sleep-state {
- i2c10-pins {
- pins = "gpio73", "gpio74";
- function = "gpio";
- drive-strength = <2>;
- bias-bus-hold;
- };
+ pins = "gpio73", "gpio74";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-bus-hold;
};
i2c12_default_state: i2c12-default-state {
- i2c12-pins {
- pins = "gpio44", "gpio45";
- function = "gsbi12";
- drive-strength = <8>;
- bias-disable;
- };
+ pins = "gpio44", "gpio45";
+ function = "gsbi12";
+ drive-strength = <8>;
+ bias-disable;
};
i2c12_sleep_state: i2c12-sleep-state {
- i2c12-pins {
- pins = "gpio44", "gpio45";
- function = "gpio";
- drive-strength = <2>;
- bias-bus-hold;
- };
+ pins = "gpio44", "gpio45";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-bus-hold;
};
sdcc3_default_state: sdcc3-default-state {
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 2/4] ARM: dts: qcom: msm8960: Flatten I2C pinctrl state subnodes
2026-04-01 20:32 ` [PATCH v3 2/4] ARM: dts: qcom: msm8960: Flatten I2C pinctrl state subnodes Rudraksha Gupta via B4 Relay
@ 2026-04-02 8:53 ` Konrad Dybcio
0 siblings, 0 replies; 12+ messages in thread
From: Konrad Dybcio @ 2026-04-02 8:53 UTC (permalink / raw)
To: guptarud, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
On 4/1/26 10:32 PM, Rudraksha Gupta via B4 Relay wrote:
> From: Rudraksha Gupta <guptarud@gmail.com>
>
> Remove unnecessary inner i2c*-pins {} wrapper nodes from the I2C
> pinctrl state definitions. Properties are moved directly under the
> -state {} node, consistent with modern DT style.
>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 3/4] ARM: dts: qcom: msm8960: Add GSBI5 I2C controller
2026-04-01 20:32 [PATCH v3 0/4] Reorganize DTS and add fuel-gauge to samsung-expressatt Rudraksha Gupta via B4 Relay
2026-04-01 20:32 ` [PATCH v3 1/4] ARM: dts: qcom: msm8960: expressatt: Sort node references and includes Rudraksha Gupta via B4 Relay
2026-04-01 20:32 ` [PATCH v3 2/4] ARM: dts: qcom: msm8960: Flatten I2C pinctrl state subnodes Rudraksha Gupta via B4 Relay
@ 2026-04-01 20:32 ` Rudraksha Gupta via B4 Relay
2026-04-01 20:32 ` [PATCH v3 4/4] ARM: dts: qcom: msm8960: expressatt: Add MAX17048 fuel gauge Rudraksha Gupta via B4 Relay
3 siblings, 0 replies; 12+ messages in thread
From: Rudraksha Gupta via B4 Relay @ 2026-04-01 20:32 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Rudraksha Gupta,
Konrad Dybcio
From: Rudraksha Gupta <guptarud@gmail.com>
Add the I2C controller node for GSBI5 (gpio24/gpio25) alongside
its pinctrl default and sleep states.
Assisted-by: Claude:claude-opus-4.6
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
---
arch/arm/boot/dts/qcom/qcom-msm8960.dtsi | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
index 6069fb925672..a427f0f41cd1 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
@@ -173,6 +173,20 @@ i2c3_sleep_state: i2c3-sleep-state {
bias-bus-hold;
};
+ i2c5_default_state: i2c5-default-state {
+ pins = "gpio24", "gpio25";
+ function = "gsbi5";
+ drive-strength = <8>;
+ bias-disable;
+ };
+
+ i2c5_sleep_state: i2c5-sleep-state {
+ pins = "gpio24", "gpio25";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-bus-hold;
+ };
+
i2c7_default_state: i2c7-default-state {
pins = "gpio32", "gpio33";
function = "gsbi7";
@@ -636,6 +650,23 @@ gsbi5_serial: serial@16440000 {
status = "disabled";
};
+
+ gsbi5_i2c: i2c@16480000 {
+ compatible = "qcom,i2c-qup-v1.1.1";
+ reg = <0x16480000 0x1000>;
+ pinctrl-0 = <&i2c5_default_state>;
+ pinctrl-1 = <&i2c5_sleep_state>;
+ pinctrl-names = "default", "sleep";
+ interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&gcc GSBI5_QUP_CLK>,
+ <&gcc GSBI5_H_CLK>;
+ clock-names = "core",
+ "iface";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "disabled";
+ };
};
gsbi7: gsbi@16600000 {
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 4/4] ARM: dts: qcom: msm8960: expressatt: Add MAX17048 fuel gauge
2026-04-01 20:32 [PATCH v3 0/4] Reorganize DTS and add fuel-gauge to samsung-expressatt Rudraksha Gupta via B4 Relay
` (2 preceding siblings ...)
2026-04-01 20:32 ` [PATCH v3 3/4] ARM: dts: qcom: msm8960: Add GSBI5 I2C controller Rudraksha Gupta via B4 Relay
@ 2026-04-01 20:32 ` Rudraksha Gupta via B4 Relay
3 siblings, 0 replies; 12+ messages in thread
From: Rudraksha Gupta via B4 Relay @ 2026-04-01 20:32 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Rudraksha Gupta,
Dmitry Baryshkov, Konrad Dybcio
From: Rudraksha Gupta <guptarud@gmail.com>
Add MAX17048 fuel gauge support.
Tested by comparing battery capacity readings between upstream (mainline
max17040 driver) and downstream (Samsung max17048_fuelgauge driver)
across a full discharge cycle. Upstream reads ~3% lower throughout. Both
track the discharge curve correctly:
Upstream: 95 92 88 87 86 87 83 82 80 68 60 55 50 45 40 35 30 20 16 10 10 5 5 1
Downstream: 95 94 92 91 91 89 87 86 84 73 64 59 51 48 43 38 33 23 17 14 12 8 6 3
Each pair of readings was collected by checking the upstream capacity
first, then moving the battery to a second expressatt running downstream
Android to check its capacity. The battery was then moved back to the
upstream device for the next reading. This swap occasionally caused the
upstream capacity to read slightly higher than the previous value
(e.g. 86 -> 87). When this happened, the reading was retaken after the
value settled.
Link: https://github.com/LineageOS/android_kernel_samsung_d2/blob/stable/cm-11.0-XNG3C/arch/arm/mach-msm/board-apexq-battery.c
Link: https://github.com/LineageOS/android_kernel_samsung_d2/blob/stable/cm-11.0-XNG3C/drivers/battery/Makefile#L5
Link: https://github.com/LineageOS/android_kernel_samsung_d2/blob/stable/cm-11.0-XNG3C/arch/arm/mach-msm/Makefile#L308
Assisted-by: Claude:claude-opus-4.6
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
---
.../dts/qcom/qcom-msm8960-samsung-expressatt.dts | 23 ++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts b/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
index 0e6959f7af6f..e4a5e49a43ec 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
+++ b/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
@@ -186,6 +186,22 @@ &gsbi5_serial {
status = "okay";
};
+&gsbi5_i2c {
+ status = "okay";
+
+ fuel-gauge@36 {
+ compatible = "maxim,max17048";
+ reg = <0x36>;
+ maxim,double-soc;
+ maxim,rcomp = /bits/ 8 <0x62>;
+ maxim,alert-low-soc-level = <2>;
+ interrupts-extended = <&tlmm 67 IRQ_TYPE_EDGE_FALLING>;
+ pinctrl-0 = <&fuelgauge_alert_pin>;
+ pinctrl-names = "default";
+ wakeup-source;
+ };
+};
+
&gsbi7 {
qcom,mode = <GSBI_PROT_I2C>;
@@ -587,6 +603,13 @@ touchkey_irq_pin: touchkey-irq-state {
bias-disable;
};
+ fuelgauge_alert_pin: fuelgauge-alert-state {
+ pins = "gpio67";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
touchkey_i2c_pins: touchkey-i2c-state {
pins = "gpio71", "gpio72";
function = "gpio";
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread