[PATCH v4 00/11] arm: qcom: mdm9615: first round of bindings and DT fixes

[PATCH v4 00/11] arm: qcom: mdm9615: first round of bindings and DT fixes

[Neil Armstrong]

This is a first round of trivial bindings & DT fixes for the MDM9615 platform.

This first round focuses on trivial changes, the remaining work will
mainly be .txt to .yaml transition of old qcom pmic & co device bindings.

To: Andy Gross <agross@kernel.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Konrad Dybcio <konrad.dybcio@somainline.org>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Lee Jones <lee@kernel.org>
To: Satya Priya <quic_c_skakit@quicinc.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Alessandro Zummo <a.zummo@towertech.it>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: linux-input@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Dependencies: None
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v4:
- patch 1: None
- patch 2: None
- patch 3: None
- patch 4: None
- patch 5: Added reviewed-by tag
- patch 6: Fix descriptions, Added reviewed-by tags
- patch 7: None
- patch 8: None
- patch 9: Added acked-by tag
- patch 10: None
- patch 11: None
- Link to v3: https://lore.kernel.org/r/20220928-mdm9615-dt-schema-fixes-v3-0-531da552c354@linaro.org

Changes in v3:
- Rebased on v6.1-rc1
- patch 1: Added reviewed-by tag
- patch 2: Fixes typo in commit msg and added precision about why MIT was selected
- patch 3: Added reviewed-by tag
- patch 4: None
- patch 5: Drop second example node
- patch 6: Drop Andy, fix interrupts desc and fix example indentation
- patch 7: Fix commit msg wrap & add reviewed-by tag
- patch 8: Reword commit msg & add reviewed-by tag
- patch 9: Reword commit msg & add reviewed-by tag
- patch 10: None
- patch 11: Added reviewed-by tag
- Link to v2: https://lore.kernel.org/r/20220928-mdm9615-dt-schema-fixes-v2-0-87fbeb4ae053@linaro.org

Changes in v2:
- patch 1: switch to move from swir.txt to qcom.yaml
- patch 2: use MIT licence instead of X11 licence
- patch 3: move reg after compatible
- patch 4: added Krzysztof's review
- patch 5: split into 5 changes:
  - document qcom,pm8921 as fallback of qcom,pm8018
  - convert qcom,pm8921-pwrkey to dt-schema
  - document qcom,pm8921-rtc as fallback of qcom,pm8018-rtc
  - drop unused PM8018 compatible
  - drop unused pm8018 RTC compatible
- patch 6: None
- patch 7: Reworded commit log based on Dmitry's wording on similar patches
- Link to v1: https://lore.kernel.org/r/20220928-mdm9615-dt-schema-fixes-v1-0-b6e63a7df1e8@linaro.org

---
Neil Armstrong (11):
      dt-bindings: arm: qcom: move swir,mangoh-green-wp8548 board documentation to qcom.yaml
      arm: dts: qcom: mdm9615*: add SPDX-License-Identifier
      arm: dts: qcom: mdm9615: add missing reg in cpu@0 node
      arm: dts: qcom: mdm9615: remove invalid spi-max-frequency gsbi3_spi node
      dt-bindings: mfd: qcom-pm8xxx: document qcom,pm8921 as fallback of qcom,pm8018
      dt-bindings: input: qcom,pm8921-pwrkey: convert to dt-schema
      dt-bindings: rtc: qcom-pm8xxx: document qcom,pm8921-rtc as fallback of qcom,pm8018-rtc
      mfd: qcom-pm8xxx: drop unused PM8018 compatible
      rtc: pm8xxx: drop unused pm8018 compatible
      arm: dts: qcom: mdm9615: remove invalid interrupt-names from pl18x mmc nodes
      arm: dts: qcom: mdm9615: remove useless amba subnode

 Documentation/devicetree/bindings/arm/qcom.yaml    |   6 +
 Documentation/devicetree/bindings/arm/swir.txt     |  12 --
 .../bindings/input/qcom,pm8921-pwrkey.yaml         |  75 +++++++++++++
 .../bindings/input/qcom,pm8xxx-pwrkey.txt          |  46 --------
 .../devicetree/bindings/mfd/qcom-pm8xxx.yaml       |  33 +++++-
 .../devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml   |  16 ++-
 .../boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts  |  39 +------
 arch/arm/boot/dts/qcom-mdm9615-wp8548.dtsi         |  39 +------
 arch/arm/boot/dts/qcom-mdm9615.dtsi                | 121 +++++++--------------
 drivers/mfd/qcom-pm8xxx.c                          |   1 -
 drivers/rtc/rtc-pm8xxx.c                           |   1 -
 11 files changed, 159 insertions(+), 230 deletions(-)
---
base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
change-id: 20220928-mdm9615-dt-schema-fixes-66d4d0ccb7c7

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>

[PATCH v4 01/11] dt-bindings: arm: qcom: move swir,mangoh-green-wp8548 board documentation to qcom.yaml

[Neil Armstrong]

Document into the main qcom.yaml as conversion from swir.txt to dt-schema.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 Documentation/devicetree/bindings/arm/qcom.yaml |  6 ++++++
 Documentation/devicetree/bindings/arm/swir.txt  | 12 ------------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 1b5ac6b02bc5..0a9a1dc68c1b 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -265,6 +265,12 @@ properties:
               - qcom,ipq8074-hk10-c2
           - const: qcom,ipq8074
 
+      - description: Sierra Wireless MangOH Green with WP8548 Module
+        items:
+          - const: swir,mangoh-green-wp8548
+          - const: swir,wp8548
+          - const: qcom,mdm9615
+
       - description: Qualcomm Technologies, Inc. SC7180 IDP
         items:
           - enum:
diff --git a/Documentation/devicetree/bindings/arm/swir.txt b/Documentation/devicetree/bindings/arm/swir.txt
deleted file mode 100644
index 042be73a95d3..000000000000
--- a/Documentation/devicetree/bindings/arm/swir.txt
+++ /dev/null
@@ -1,12 +0,0 @@
-Sierra Wireless Modules device tree bindings
---------------------------------------------
-
-Supported Modules :
- - WP8548 : Includes MDM9615 and PM8018 in a module
-
-Sierra Wireless modules shall have the following properties :
-  Required root node property
-   - compatible: "swir,wp8548" for the WP8548 CF3 Module
-
-Board compatible values:
-  - "swir,mangoh-green-wp8548" for the mangOH green board with the WP8548 module

-- 
b4 0.10.1

[PATCH v4 02/11] arm: dts: qcom: mdm9615*: add SPDX-License-Identifier

[Neil Armstrong]

Replace the license blob by a clean SPDX-License-Identifier with GPL2+
or MIT even if X11 is specified in the original blob since the actual
license text corresponds to a MIT license.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts  | 39 +---------------------
 arch/arm/boot/dts/qcom-mdm9615-wp8548.dtsi         | 39 +---------------------
 arch/arm/boot/dts/qcom-mdm9615.dtsi                | 39 +---------------------
 3 files changed, 3 insertions(+), 114 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts b/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts
index 0827de5426c1..4e53b3d70195 100644
--- a/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts
+++ b/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts
@@ -1,46 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0+ OR MIT
 /*
  * Device Tree Source for mangOH Green Board with WP8548 Module
  *
  * Copyright (C) 2016 BayLibre, SAS.
  * Author : Neil Armstrong <narmstrong@baylibre.com>
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPL or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This file is free software; you can redistribute it and/or
- *     modify it under the terms of the GNU General Public License as
- *     published by the Free Software Foundation; either version 2 of the
- *     License, or (at your option) any later version.
- *
- *     This file is distributed in the hope that it will be useful,
- *     but WITHOUT ANY WARRANTY; without even the implied warranty of
- *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *     GNU General Public License for more details.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 #include <dt-bindings/input/input.h>
diff --git a/arch/arm/boot/dts/qcom-mdm9615-wp8548.dtsi b/arch/arm/boot/dts/qcom-mdm9615-wp8548.dtsi
index 49de1821ac3a..2fe8693dc3cd 100644
--- a/arch/arm/boot/dts/qcom-mdm9615-wp8548.dtsi
+++ b/arch/arm/boot/dts/qcom-mdm9615-wp8548.dtsi
@@ -1,46 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0+ OR MIT
 /*
  * Device Tree Source for Sierra Wireless WP8548 Module
  *
  * Copyright (C) 2016 BayLibre, SAS.
  * Author : Neil Armstrong <narmstrong@baylibre.com>
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPL or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This file is free software; you can redistribute it and/or
- *     modify it under the terms of the GNU General Public License as
- *     published by the Free Software Foundation; either version 2 of the
- *     License, or (at your option) any later version.
- *
- *     This file is distributed in the hope that it will be useful,
- *     but WITHOUT ANY WARRANTY; without even the implied warranty of
- *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *     GNU General Public License for more details.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 #include "qcom-mdm9615.dtsi"
diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi
index b47c86412de2..de36e4545e75 100644
--- a/arch/arm/boot/dts/qcom-mdm9615.dtsi
+++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi
@@ -1,46 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0+ OR MIT
 /*
  * Device Tree Source for Qualcomm MDM9615 SoC
  *
  * Copyright (C) 2016 BayLibre, SAS.
  * Author : Neil Armstrong <narmstrong@baylibre.com>
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPL or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This file is free software; you can redistribute it and/or
- *     modify it under the terms of the GNU General Public License as
- *     published by the Free Software Foundation; either version 2 of the
- *     License, or (at your option) any later version.
- *
- *     This file is distributed in the hope that it will be useful,
- *     but WITHOUT ANY WARRANTY; without even the implied warranty of
- *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *     GNU General Public License for more details.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 /dts-v1/;

-- 
b4 0.10.1

[PATCH v4 03/11] arm: dts: qcom: mdm9615: add missing reg in cpu@0 node

[Neil Armstrong]

Fixes cpu@0: 'reg' is a required property from dtbs check.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm/boot/dts/qcom-mdm9615.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi
index de36e4545e75..eaa3236f62db 100644
--- a/arch/arm/boot/dts/qcom-mdm9615.dtsi
+++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi
@@ -27,6 +27,7 @@ cpus {
 
 		cpu0: cpu@0 {
 			compatible = "arm,cortex-a5";
+			reg = <0>;
 			device_type = "cpu";
 			next-level-cache = <&L2>;
 		};

-- 
b4 0.10.1

[PATCH v4 04/11] arm: dts: qcom: mdm9615: remove invalid spi-max-frequency gsbi3_spi node

[Neil Armstrong]

The spi-max-frequency property has nothing to do in the controller's node,
remove it and fix the 'spi-max-frequency' was unexpected dtbs check error.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 arch/arm/boot/dts/qcom-mdm9615.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi
index eaa3236f62db..366241dee522 100644
--- a/arch/arm/boot/dts/qcom-mdm9615.dtsi
+++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi
@@ -170,7 +170,6 @@ gsbi3_spi: spi@16280000 {
 				#size-cells = <0>;
 				reg = <0x16280000 0x1000>;
 				interrupts = <GIC_SPI 151 IRQ_TYPE_LEVEL_HIGH>;
-				spi-max-frequency = <24000000>;
 
 				clocks = <&gcc GSBI3_QUP_CLK>, <&gcc GSBI3_H_CLK>;
 				clock-names = "core", "iface";

-- 
b4 0.10.1

[PATCH v4 05/11] dt-bindings: mfd: qcom-pm8xxx: document qcom,pm8921 as fallback of qcom,pm8018

[Neil Armstrong]

The PM8018 is used as compatible with PM8921 on the MDM9615, document this situation,
and an example section to validate this change.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../devicetree/bindings/mfd/qcom-pm8xxx.yaml       | 33 ++++++++++++++++++----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml b/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml
index 61bd0b3ce02f..84b87f01e029 100644
--- a/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml
@@ -15,11 +15,15 @@ description: |
 
 properties:
   compatible:
-    enum:
-      - qcom,pm8018
-      - qcom,pm8058
-      - qcom,pm8821
-      - qcom,pm8921
+    oneOf:
+      - enum:
+          - qcom,pm8058
+          - qcom,pm8821
+          - qcom,pm8921
+      - items:
+          - enum:
+              - qcom,pm8018
+          - const: qcom,pm8921
 
   reg:
     maxItems: 1
@@ -52,4 +56,23 @@ required:
   - interrupt-controller
 
 additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    ssbi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@0 {
+        compatible = "qcom,pm8921";
+        reg = <0>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        interrupt-controller;
+        #interrupt-cells = <2>;
+
+        interrupt-parent = <&tlmm>;
+        interrupts = <32 IRQ_TYPE_EDGE_RISING>;
+      };
+    };
 ...

-- 
b4 0.10.1

[PATCH v4 06/11] dt-bindings: input: qcom,pm8921-pwrkey: convert to dt-schema

[Neil Armstrong]

Convert input/qcom,pm8xxx-pwrkey.txt to YAML, and take in account that
the PM8921 pwrkey compatible is used as fallback for the PM8018 pwrkey.

Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../bindings/input/qcom,pm8921-pwrkey.yaml         | 75 ++++++++++++++++++++++
 .../bindings/input/qcom,pm8xxx-pwrkey.txt          | 46 -------------
 2 files changed, 75 insertions(+), 46 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/qcom,pm8921-pwrkey.yaml b/Documentation/devicetree/bindings/input/qcom,pm8921-pwrkey.yaml
new file mode 100644
index 000000000000..12c74c083258
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/qcom,pm8921-pwrkey.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/qcom,pm8921-pwrkey.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm PM8921 PMIC Power Key
+
+maintainers:
+  - Bjorn Andersson <andersson@kernel.org>
+
+allOf:
+  - $ref: input.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - qcom,pm8921-pwrkey
+          - qcom,pm8058-pwrkey
+      - items:
+          - enum:
+              - qcom,pm8018-pwrkey
+          - const: qcom,pm8921-pwrkey
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: key release
+      - description: key press
+
+  debounce:
+    description:
+      Time in microseconds that key must be pressed or
+      released for state change interrupt to trigger.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  pull-up:
+    description:
+      Presence of this property indicates that the KPDPWR_N
+      pin should be configured for pull up.
+    $ref: /schemas/types.yaml#/definitions/flag
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+   #include <dt-bindings/interrupt-controller/irq.h>
+   ssbi {
+     #address-cells = <1>;
+     #size-cells = <0>;
+
+     pmic@0 {
+       reg = <0x0>;
+       #address-cells = <1>;
+       #size-cells = <0>;
+
+       pwrkey@1c {
+         compatible = "qcom,pm8921-pwrkey";
+         reg = <0x1c>;
+         interrupt-parent = <&pmicint>;
+         interrupts = <50 IRQ_TYPE_EDGE_RISING>, <51 IRQ_TYPE_EDGE_RISING>;
+         debounce = <15625>;
+         pull-up;
+       };
+     };
+   };
+...
diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt
deleted file mode 100644
index 588536cc96ed..000000000000
--- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt
+++ /dev/null
@@ -1,46 +0,0 @@
-Qualcomm PM8xxx PMIC Power Key
-
-PROPERTIES
-
-- compatible:
-	Usage: required
-	Value type: <string>
-	Definition: must be one of:
-		    "qcom,pm8058-pwrkey"
-		    "qcom,pm8921-pwrkey"
-
-- reg:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: address of power key control register
-
-- interrupts:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: the first interrupt specifies the key release interrupt
-		    and the second interrupt specifies the key press interrupt.
-		    The format of the specifier is defined by the binding
-		    document describing the node's interrupt parent.
-
-- debounce:
-	Usage: optional
-	Value type: <u32>
-	Definition: time in microseconds that key must be pressed or release
-		    for state change interrupt to trigger.
-
-- pull-up:
-	Usage: optional
-	Value type: <empty>
-	Definition: presence of this property indicates that the KPDPWR_N pin
-		    should be configured for pull up.
-
-EXAMPLE
-
-	pwrkey@1c {
-		compatible = "qcom,pm8921-pwrkey";
-		reg = <0x1c>;
-		interrupt-parent = <&pmicintc>;
-		interrupts = <50 1>, <51 1>;
-		debounce = <15625>;
-		pull-up;
-	};

-- 
b4 0.10.1

[PATCH v4 07/11] dt-bindings: rtc: qcom-pm8xxx: document qcom,pm8921-rtc as fallback of qcom,pm8018-rtc

[Neil Armstrong]

The PM8018 RTC is used as compatible with PM8921 RTC on the MDM9615,
document this situation.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml         | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml b/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
index 23ab5bb4f395..0a7aa29563c1 100644
--- a/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
@@ -11,12 +11,16 @@ maintainers:
 
 properties:
   compatible:
-    enum:
-      - qcom,pm8058-rtc
-      - qcom,pm8921-rtc
-      - qcom,pm8941-rtc
-      - qcom,pm8018-rtc
-      - qcom,pmk8350-rtc
+    oneOf:
+      - enum:
+          - qcom,pm8058-rtc
+          - qcom,pm8921-rtc
+          - qcom,pm8941-rtc
+          - qcom,pmk8350-rtc
+      - items:
+          - enum:
+              - qcom,pm8018-rtc
+          - const: qcom,pm8921-rtc
 
   reg:
     minItems: 1

-- 
b4 0.10.1

[PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible

[Neil Armstrong]

The PM8018 compatible is always used with PM8921 fallback, so PM8018
compatible can be safely removed from device ID table

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/mfd/qcom-pm8xxx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mfd/qcom-pm8xxx.c b/drivers/mfd/qcom-pm8xxx.c
index 2f2734ba5273..601106580e2e 100644
--- a/drivers/mfd/qcom-pm8xxx.c
+++ b/drivers/mfd/qcom-pm8xxx.c
@@ -497,7 +497,6 @@ static const struct pm_irq_data pm8821_data = {
 };
 
 static const struct of_device_id pm8xxx_id_table[] = {
-	{ .compatible = "qcom,pm8018", .data = &pm8xxx_data},
 	{ .compatible = "qcom,pm8058", .data = &pm8xxx_data},
 	{ .compatible = "qcom,pm8821", .data = &pm8821_data},
 	{ .compatible = "qcom,pm8921", .data = &pm8xxx_data},

-- 
b4 0.10.1

[PATCH v4 09/11] rtc: pm8xxx: drop unused pm8018 compatible

[Neil Armstrong]

The PM8018 compatible is always used with PM8921 fallback, so PM8018
compatible can be safely removed from device ID table

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/rtc/rtc-pm8xxx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index dc6d1476baa5..716e5d9ad74d 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -461,7 +461,6 @@ static const struct pm8xxx_rtc_regs pmk8350_regs = {
  */
 static const struct of_device_id pm8xxx_id_table[] = {
 	{ .compatible = "qcom,pm8921-rtc", .data = &pm8921_regs },
-	{ .compatible = "qcom,pm8018-rtc", .data = &pm8921_regs },
 	{ .compatible = "qcom,pm8058-rtc", .data = &pm8058_regs },
 	{ .compatible = "qcom,pm8941-rtc", .data = &pm8941_regs },
 	{ .compatible = "qcom,pmk8350-rtc", .data = &pmk8350_regs },

-- 
b4 0.10.1

[PATCH v4 10/11] arm: dts: qcom: mdm9615: remove invalid interrupt-names from pl18x mmc nodes

[Neil Armstrong]

This solves the 'interrupt-names' was unexpected dtbs check error.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm/boot/dts/qcom-mdm9615.dtsi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi
index 366241dee522..9d950f96280d 100644
--- a/arch/arm/boot/dts/qcom-mdm9615.dtsi
+++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi
@@ -325,7 +325,6 @@ sdcc1: mmc@12180000 {
 				arm,primecell-periphid = <0x00051180>;
 				reg = <0x12180000 0x2000>;
 				interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
-				interrupt-names = "cmd_irq";
 				clocks = <&gcc SDC1_CLK>, <&gcc SDC1_H_CLK>;
 				clock-names = "mclk", "apb_pclk";
 				bus-width = <8>;
@@ -345,7 +344,6 @@ sdcc2: mmc@12140000 {
 				status = "disabled";
 				reg = <0x12140000 0x2000>;
 				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
-				interrupt-names = "cmd_irq";
 				clocks = <&gcc SDC2_CLK>, <&gcc SDC2_H_CLK>;
 				clock-names = "mclk", "apb_pclk";
 				bus-width = <4>;

-- 
b4 0.10.1

[PATCH v4 11/11] arm: dts: qcom: mdm9615: remove useless amba subnode

[Neil Armstrong]

The separate amba device node doesn't add anything significant to the
DT. The OF parsing code already creates amba_device or platform_device
depending on the compatibility lists.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm/boot/dts/qcom-mdm9615.dtsi | 78 +++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 42 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi
index 9d950f96280d..482fd246321c 100644
--- a/arch/arm/boot/dts/qcom-mdm9615.dtsi
+++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi
@@ -314,49 +314,43 @@ sdcc2bam: dma-controller@12142000{
 			qcom,ee = <0>;
 		};
 
-		amba {
-			compatible = "simple-bus";
-			#address-cells = <1>;
-			#size-cells = <1>;
-			ranges;
-			sdcc1: mmc@12180000 {
-				status = "disabled";
-				compatible = "arm,pl18x", "arm,primecell";
-				arm,primecell-periphid = <0x00051180>;
-				reg = <0x12180000 0x2000>;
-				interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
-				clocks = <&gcc SDC1_CLK>, <&gcc SDC1_H_CLK>;
-				clock-names = "mclk", "apb_pclk";
-				bus-width = <8>;
-				max-frequency = <48000000>;
-				cap-sd-highspeed;
-				cap-mmc-highspeed;
-				vmmc-supply = <&vsdcc_fixed>;
-				dmas = <&sdcc1bam 2>, <&sdcc1bam 1>;
-				dma-names = "tx", "rx";
-				assigned-clocks = <&gcc SDC1_CLK>;
-				assigned-clock-rates = <400000>;
-			};
+		sdcc1: mmc@12180000 {
+			status = "disabled";
+			compatible = "arm,pl18x", "arm,primecell";
+			arm,primecell-periphid = <0x00051180>;
+			reg = <0x12180000 0x2000>;
+			interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&gcc SDC1_CLK>, <&gcc SDC1_H_CLK>;
+			clock-names = "mclk", "apb_pclk";
+			bus-width = <8>;
+			max-frequency = <48000000>;
+			cap-sd-highspeed;
+			cap-mmc-highspeed;
+			vmmc-supply = <&vsdcc_fixed>;
+			dmas = <&sdcc1bam 2>, <&sdcc1bam 1>;
+			dma-names = "tx", "rx";
+			assigned-clocks = <&gcc SDC1_CLK>;
+			assigned-clock-rates = <400000>;
+		};
 
-			sdcc2: mmc@12140000 {
-				compatible = "arm,pl18x", "arm,primecell";
-				arm,primecell-periphid = <0x00051180>;
-				status = "disabled";
-				reg = <0x12140000 0x2000>;
-				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
-				clocks = <&gcc SDC2_CLK>, <&gcc SDC2_H_CLK>;
-				clock-names = "mclk", "apb_pclk";
-				bus-width = <4>;
-				cap-sd-highspeed;
-				cap-mmc-highspeed;
-				max-frequency = <48000000>;
-				no-1-8-v;
-				vmmc-supply = <&vsdcc_fixed>;
-				dmas = <&sdcc2bam 2>, <&sdcc2bam 1>;
-				dma-names = "tx", "rx";
-				assigned-clocks = <&gcc SDC2_CLK>;
-				assigned-clock-rates = <400000>;
-			};
+		sdcc2: mmc@12140000 {
+			compatible = "arm,pl18x", "arm,primecell";
+			arm,primecell-periphid = <0x00051180>;
+			status = "disabled";
+			reg = <0x12140000 0x2000>;
+			interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&gcc SDC2_CLK>, <&gcc SDC2_H_CLK>;
+			clock-names = "mclk", "apb_pclk";
+			bus-width = <4>;
+			cap-sd-highspeed;
+			cap-mmc-highspeed;
+			max-frequency = <48000000>;
+			no-1-8-v;
+			vmmc-supply = <&vsdcc_fixed>;
+			dmas = <&sdcc2bam 2>, <&sdcc2bam 1>;
+			dma-names = "tx", "rx";
+			assigned-clocks = <&gcc SDC2_CLK>;
+			assigned-clock-rates = <400000>;
 		};
 
 		tcsr: syscon@1a400000 {

-- 
b4 0.10.1

Re: [PATCH v4 03/11] arm: dts: qcom: mdm9615: add missing reg in cpu@0 node

[Konrad Dybcio]

On 21.10.2022 11:06, Neil Armstrong wrote:
> Fixes cpu@0: 'reg' is a required property from dtbs check.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>

Konrad
>  arch/arm/boot/dts/qcom-mdm9615.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi
> index de36e4545e75..eaa3236f62db 100644
> --- a/arch/arm/boot/dts/qcom-mdm9615.dtsi
> +++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi
> @@ -27,6 +27,7 @@ cpus {
>  
>  		cpu0: cpu@0 {
>  			compatible = "arm,cortex-a5";
> +			reg = <0>;
>  			device_type = "cpu";
>  			next-level-cache = <&L2>;
>  		};
> 

Re: [PATCH v4 04/11] arm: dts: qcom: mdm9615: remove invalid spi-max-frequency gsbi3_spi node

[Konrad Dybcio]

On 21.10.2022 11:06, Neil Armstrong wrote:
> The spi-max-frequency property has nothing to do in the controller's node,
> remove it and fix the 'spi-max-frequency' was unexpected dtbs check error.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>

Konrad
>  arch/arm/boot/dts/qcom-mdm9615.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi
> index eaa3236f62db..366241dee522 100644
> --- a/arch/arm/boot/dts/qcom-mdm9615.dtsi
> +++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi
> @@ -170,7 +170,6 @@ gsbi3_spi: spi@16280000 {
>  				#size-cells = <0>;
>  				reg = <0x16280000 0x1000>;
>  				interrupts = <GIC_SPI 151 IRQ_TYPE_LEVEL_HIGH>;
> -				spi-max-frequency = <24000000>;
>  
>  				clocks = <&gcc GSBI3_QUP_CLK>, <&gcc GSBI3_H_CLK>;
>  				clock-names = "core", "iface";
> 

Re: [PATCH v4 11/11] arm: dts: qcom: mdm9615: remove useless amba subnode

[Konrad Dybcio]

On 21.10.2022 11:06, Neil Armstrong wrote:
> The separate amba device node doesn't add anything significant to the
> DT. The OF parsing code already creates amba_device or platform_device
> depending on the compatibility lists.
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
If we ever wanted to do split buses, per-NoC nodes would make sense, but
as you mentioned, this does not, really.

Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>

Konrad
>  arch/arm/boot/dts/qcom-mdm9615.dtsi | 78 +++++++++++++++++--------------------
>  1 file changed, 36 insertions(+), 42 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi
> index 9d950f96280d..482fd246321c 100644
> --- a/arch/arm/boot/dts/qcom-mdm9615.dtsi
> +++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi
> @@ -314,49 +314,43 @@ sdcc2bam: dma-controller@12142000{
>  			qcom,ee = <0>;
>  		};
>  
> -		amba {
> -			compatible = "simple-bus";
> -			#address-cells = <1>;
> -			#size-cells = <1>;
> -			ranges;
> -			sdcc1: mmc@12180000 {
> -				status = "disabled";
> -				compatible = "arm,pl18x", "arm,primecell";
> -				arm,primecell-periphid = <0x00051180>;
> -				reg = <0x12180000 0x2000>;
> -				interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> -				clocks = <&gcc SDC1_CLK>, <&gcc SDC1_H_CLK>;
> -				clock-names = "mclk", "apb_pclk";
> -				bus-width = <8>;
> -				max-frequency = <48000000>;
> -				cap-sd-highspeed;
> -				cap-mmc-highspeed;
> -				vmmc-supply = <&vsdcc_fixed>;
> -				dmas = <&sdcc1bam 2>, <&sdcc1bam 1>;
> -				dma-names = "tx", "rx";
> -				assigned-clocks = <&gcc SDC1_CLK>;
> -				assigned-clock-rates = <400000>;
> -			};
> +		sdcc1: mmc@12180000 {
> +			status = "disabled";
> +			compatible = "arm,pl18x", "arm,primecell";
> +			arm,primecell-periphid = <0x00051180>;
> +			reg = <0x12180000 0x2000>;
> +			interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&gcc SDC1_CLK>, <&gcc SDC1_H_CLK>;
> +			clock-names = "mclk", "apb_pclk";
> +			bus-width = <8>;
> +			max-frequency = <48000000>;
> +			cap-sd-highspeed;
> +			cap-mmc-highspeed;
> +			vmmc-supply = <&vsdcc_fixed>;
> +			dmas = <&sdcc1bam 2>, <&sdcc1bam 1>;
> +			dma-names = "tx", "rx";
> +			assigned-clocks = <&gcc SDC1_CLK>;
> +			assigned-clock-rates = <400000>;
> +		};
>  
> -			sdcc2: mmc@12140000 {
> -				compatible = "arm,pl18x", "arm,primecell";
> -				arm,primecell-periphid = <0x00051180>;
> -				status = "disabled";
> -				reg = <0x12140000 0x2000>;
> -				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> -				clocks = <&gcc SDC2_CLK>, <&gcc SDC2_H_CLK>;
> -				clock-names = "mclk", "apb_pclk";
> -				bus-width = <4>;
> -				cap-sd-highspeed;
> -				cap-mmc-highspeed;
> -				max-frequency = <48000000>;
> -				no-1-8-v;
> -				vmmc-supply = <&vsdcc_fixed>;
> -				dmas = <&sdcc2bam 2>, <&sdcc2bam 1>;
> -				dma-names = "tx", "rx";
> -				assigned-clocks = <&gcc SDC2_CLK>;
> -				assigned-clock-rates = <400000>;
> -			};
> +		sdcc2: mmc@12140000 {
> +			compatible = "arm,pl18x", "arm,primecell";
> +			arm,primecell-periphid = <0x00051180>;
> +			status = "disabled";
> +			reg = <0x12140000 0x2000>;
> +			interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&gcc SDC2_CLK>, <&gcc SDC2_H_CLK>;
> +			clock-names = "mclk", "apb_pclk";
> +			bus-width = <4>;
> +			cap-sd-highspeed;
> +			cap-mmc-highspeed;
> +			max-frequency = <48000000>;
> +			no-1-8-v;
> +			vmmc-supply = <&vsdcc_fixed>;
> +			dmas = <&sdcc2bam 2>, <&sdcc2bam 1>;
> +			dma-names = "tx", "rx";
> +			assigned-clocks = <&gcc SDC2_CLK>;
> +			assigned-clock-rates = <400000>;
>  		};
>  
>  		tcsr: syscon@1a400000 {
> 

Re: [PATCH v4 09/11] rtc: pm8xxx: drop unused pm8018 compatible

[Konrad Dybcio]

On 21.10.2022 11:06, Neil Armstrong wrote:
> The PM8018 compatible is always used with PM8921 fallback, so PM8018
> compatible can be safely removed from device ID table
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>

Konrad
>  drivers/rtc/rtc-pm8xxx.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
> index dc6d1476baa5..716e5d9ad74d 100644
> --- a/drivers/rtc/rtc-pm8xxx.c
> +++ b/drivers/rtc/rtc-pm8xxx.c
> @@ -461,7 +461,6 @@ static const struct pm8xxx_rtc_regs pmk8350_regs = {
>   */
>  static const struct of_device_id pm8xxx_id_table[] = {
>  	{ .compatible = "qcom,pm8921-rtc", .data = &pm8921_regs },
> -	{ .compatible = "qcom,pm8018-rtc", .data = &pm8921_regs },
>  	{ .compatible = "qcom,pm8058-rtc", .data = &pm8058_regs },
>  	{ .compatible = "qcom,pm8941-rtc", .data = &pm8941_regs },
>  	{ .compatible = "qcom,pmk8350-rtc", .data = &pmk8350_regs },
> 

Re: [PATCH v4 02/11] arm: dts: qcom: mdm9615*: add SPDX-License-Identifier

[Krzysztof Kozlowski]

On 21/10/2022 05:06, Neil Armstrong wrote:
> Replace the license blob by a clean SPDX-License-Identifier with GPL2+
> or MIT even if X11 is specified in the original blob since the actual
> license text corresponds to a MIT license.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v4 02/11] arm: dts: qcom: mdm9615*: add SPDX-License-Identifier

[Krzysztof Kozlowski]

On 21/10/2022 09:42, Krzysztof Kozlowski wrote:
> On 21/10/2022 05:06, Neil Armstrong wrote:
>> Replace the license blob by a clean SPDX-License-Identifier with GPL2+
>> or MIT even if X11 is specified in the original blob since the actual
>> license text corresponds to a MIT license.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

But just to clarify: I was not involved in the DTS, so I hold no
copyrights over it. The review is only confirming the code looks good to me.


Best regards,
Krzysztof

Re: [PATCH v4 06/11] dt-bindings: input: qcom,pm8921-pwrkey: convert to dt-schema

[Dmitry Torokhov]

On Fri, Oct 21, 2022 at 11:06:42AM +0200, Neil Armstrong wrote:
> Convert input/qcom,pm8xxx-pwrkey.txt to YAML, and take in account that
> the PM8921 pwrkey compatible is used as fallback for the PM8018 pwrkey.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

Should I merge this through my tree or you want all these changes to go
together through some particular tree?

Thanks.

-- 
Dmitry

Re: [PATCH v4 06/11] dt-bindings: input: qcom,pm8921-pwrkey: convert to dt-schema

[Neil Armstrong]

Hi,

On 27/10/2022 09:55, Dmitry Torokhov wrote:
> On Fri, Oct 21, 2022 at 11:06:42AM +0200, Neil Armstrong wrote:
>> Convert input/qcom,pm8xxx-pwrkey.txt to YAML, and take in account that
>> the PM8921 pwrkey compatible is used as fallback for the PM8018 pwrkey.
>>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> Should I merge this through my tree or you want all these changes to go
> together through some particular tree?

I have no preference,
Krzysztof will you take it and prepare a branch to Bjorn ?

> 
> Thanks.
> 

Neil

Re: [PATCH v4 06/11] dt-bindings: input: qcom,pm8921-pwrkey: convert to dt-schema

[Krzysztof Kozlowski]

On 27/10/2022 08:39, Neil Armstrong wrote:
> Hi,
> 
> On 27/10/2022 09:55, Dmitry Torokhov wrote:
>> On Fri, Oct 21, 2022 at 11:06:42AM +0200, Neil Armstrong wrote:
>>> Convert input/qcom,pm8xxx-pwrkey.txt to YAML, and take in account that
>>> the PM8921 pwrkey compatible is used as fallback for the PM8018 pwrkey.
>>>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>
>> Should I merge this through my tree or you want all these changes to go
>> together through some particular tree?
> 
> I have no preference,
> Krzysztof will you take it and prepare a branch to Bjorn ?

No, I did not plan on doing it.

Dmitry,
Please grab it (assuming there are no dependencies).

Best regards,
Krzysztof

Re: [PATCH v4 06/11] dt-bindings: input: qcom,pm8921-pwrkey: convert to dt-schema

[Dmitry Torokhov]

On Thu, Oct 27, 2022 at 08:43:20AM -0400, Krzysztof Kozlowski wrote:
> On 27/10/2022 08:39, Neil Armstrong wrote:
> > Hi,
> > 
> > On 27/10/2022 09:55, Dmitry Torokhov wrote:
> >> On Fri, Oct 21, 2022 at 11:06:42AM +0200, Neil Armstrong wrote:
> >>> Convert input/qcom,pm8xxx-pwrkey.txt to YAML, and take in account that
> >>> the PM8921 pwrkey compatible is used as fallback for the PM8018 pwrkey.
> >>>
> >>> Reviewed-by: Rob Herring <robh@kernel.org>
> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> >>
> >> Should I merge this through my tree or you want all these changes to go
> >> together through some particular tree?
> > 
> > I have no preference,
> > Krzysztof will you take it and prepare a branch to Bjorn ?
> 
> No, I did not plan on doing it.
> 
> Dmitry,
> Please grab it (assuming there are no dependencies).

Applied, thank you.

-- 
Dmitry

Re: [PATCH v4 05/11] dt-bindings: mfd: qcom-pm8xxx: document qcom,pm8921 as fallback of qcom,pm8018

[Lee Jones]

On Fri, 21 Oct 2022, Neil Armstrong wrote:

> The PM8018 is used as compatible with PM8921 on the MDM9615, document this situation,
> and an example section to validate this change.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../devicetree/bindings/mfd/qcom-pm8xxx.yaml       | 33 ++++++++++++++++++----
>  1 file changed, 28 insertions(+), 5 deletions(-)

Applied, thanks.

-- 
Lee Jones [李琼斯]

Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible

[Lee Jones]

On Fri, 21 Oct 2022, Neil Armstrong wrote:

> The PM8018 compatible is always used with PM8921 fallback, so PM8018
> compatible can be safely removed from device ID table
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

Tags should appear chronologically.

I've fixed this up and applied the patch, thanks.

> ---
>  drivers/mfd/qcom-pm8xxx.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/mfd/qcom-pm8xxx.c b/drivers/mfd/qcom-pm8xxx.c
> index 2f2734ba5273..601106580e2e 100644
> --- a/drivers/mfd/qcom-pm8xxx.c
> +++ b/drivers/mfd/qcom-pm8xxx.c
> @@ -497,7 +497,6 @@ static const struct pm_irq_data pm8821_data = {
>  };
>  
>  static const struct of_device_id pm8xxx_id_table[] = {
> -	{ .compatible = "qcom,pm8018", .data = &pm8xxx_data},
>  	{ .compatible = "qcom,pm8058", .data = &pm8xxx_data},
>  	{ .compatible = "qcom,pm8821", .data = &pm8821_data},
>  	{ .compatible = "qcom,pm8921", .data = &pm8xxx_data},
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible

[Neil Armstrong]

Hi,

On 31/10/2022 16:32, Lee Jones wrote:
> On Fri, 21 Oct 2022, Neil Armstrong wrote:
> 
>> The PM8018 compatible is always used with PM8921 fallback, so PM8018
>> compatible can be safely removed from device ID table
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> Tags should appear chronologically.

Indeed, they were added by b4, I'll report this.

Thanks,
Neil

> 
> I've fixed this up and applied the patch, thanks.
> 
>> ---
>>   drivers/mfd/qcom-pm8xxx.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/mfd/qcom-pm8xxx.c b/drivers/mfd/qcom-pm8xxx.c
>> index 2f2734ba5273..601106580e2e 100644
>> --- a/drivers/mfd/qcom-pm8xxx.c
>> +++ b/drivers/mfd/qcom-pm8xxx.c
>> @@ -497,7 +497,6 @@ static const struct pm_irq_data pm8821_data = {
>>   };
>>   
>>   static const struct of_device_id pm8xxx_id_table[] = {
>> -	{ .compatible = "qcom,pm8018", .data = &pm8xxx_data},
>>   	{ .compatible = "qcom,pm8058", .data = &pm8xxx_data},
>>   	{ .compatible = "qcom,pm8821", .data = &pm8821_data},
>>   	{ .compatible = "qcom,pm8921", .data = &pm8xxx_data},
>>
> 

DCO chain of custody revisited (was Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible)

[Konstantin Ryabitsev]

Hijacking this thread for greater good.

On Mon, Oct 31, 2022 at 04:35:38PM +0100, Neil Armstrong wrote:
> Hi,
> 
> On 31/10/2022 16:32, Lee Jones wrote:
> > On Fri, 21 Oct 2022, Neil Armstrong wrote:
> > 
> > > The PM8018 compatible is always used with PM8921 fallback, so PM8018
> > > compatible can be safely removed from device ID table
> > > 
> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > 
> > Tags should appear chronologically.
> 
> Indeed, they were added by b4, I'll report this.

My trouble is that there are seemingly as many opinions about the order of
trailers as there are subsystem maintainers. The last time we had a long
discussion about this on the users list I got a strong message that what
matters most is the chain of custody, and the Signed-off-by trailer indicates
the chain of custody boundary.

In the scenario below, the chain consists of 3 people:

| Suggested-by: Reporter 1 <...>
| Signed-off-by: Developer 1 <...> -- initial DCO boundary
| Reviewed-by: Reviewer 1 <...>
| Tested-by: Tester 1 <...>
| Signed-off-by: Submaintainer 1 <...> -- intermediate DCO boundary
| Acked-by: Submaintainer 2 <...>
| Signed-off-by: Maintainer 1 <...> -- final DCO boundary

In terms of DCO, this makes the following claims:

Developer 1:
 - I am responsible for this change
 - It was suggested by Reporter 1

Submaintainer 1:
 - I am signing off on this change
 - I have collected the trailers from Reviewer 1 and Tester 1

Maintainer 1:
 - I am signing off on this change
 - I have collected the trailer from Submaintainer 2

In the current case, and using this principle, the following order is correct:

| Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
| Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

Because the Reviewerd-by trailer was sent to the v2 of the series and was
collected by Neil, so Neil is the person who is the DCO signatory of that
chain of custody in the v4 of the series.

I assume that in the final commit Lee rearranged the tags in the following
order:

| Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
| Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
| Signed-off-by: Lee Jones <lee@kernel.org>

This would indicate that it's *Lee* who is claiming responsibility for
collecting the Reviewed-by tag from Krzysztof, because it is in his chain of
custody. However, this is not the case -- it was Neil who collected the tag,
and therefore the "more correct" order should be:

| Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
| Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
| Signed-off-by: Lee Jones <lee@kernel.org>

If my reasoning is incorrect, then I need to go back to the drawing board.

-K

Re: DCO chain of custody revisited (was Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible)

[James Bottomley]

On Mon, 2022-10-31 at 12:58 -0400, Konstantin Ryabitsev wrote:
[...]
> I assume that in the final commit Lee rearranged the tags in the
> following order:
> 
> > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Signed-off-by: Lee Jones <lee@kernel.org>
> 
> This would indicate that it's *Lee* who is claiming responsibility
> for collecting the Reviewed-by tag from Krzysztof, because it is in
> his chain of custody. However, this is not the case -- it was Neil
> who collected the tag, and therefore the "more correct" order should
> be:
> 
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > Signed-off-by: Lee Jones <lee@kernel.org>
> 
> If my reasoning is incorrect, then I need to go back to the drawing
> board.

You're way over thinking this.  The only tag that matters from the DCO
point of view is Signed-off-by.  That's the ordering we care about for
the chain of custody.  All other tags are irrelevant.  Of course, it's
nice to think that reviews happen *after* the code was modified, which
is why most of us like to see the Reviewed-by after the initial author
signoff, but who added the tag has no DCO significance because it
doesn't affect the representations on the contribution.

Regards,

James

Re: DCO chain of custody revisited (was Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible)

[Konstantin Ryabitsev]

On Mon, Oct 31, 2022 at 01:10:58PM -0400, James Bottomley wrote:
> > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > Signed-off-by: Lee Jones <lee@kernel.org>
> > 
> > This would indicate that it's *Lee* who is claiming responsibility
> > for collecting the Reviewed-by tag from Krzysztof, because it is in
> > his chain of custody. However, this is not the case -- it was Neil
> > who collected the tag, and therefore the "more correct" order should
> > be:
> > 
> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > Signed-off-by: Lee Jones <lee@kernel.org>
> > 
> > If my reasoning is incorrect, then I need to go back to the drawing
> > board.
> 
> You're way over thinking this.

Yes, but it's my job to overthink this, so nobody else has to. :)

> The only tag that matters from the DCO point of view is Signed-off-by.
> That's the ordering we care about for the chain of custody.  All other tags
> are irrelevant.  Of course, it's nice to think that reviews happen *after*
> the code was modified, which is why most of us like to see the Reviewed-by
> after the initial author signoff,

But this is where it becomes complicated. The Reviewed-By trailer was sent to
the v2 of the series, and incorporated into v3(via b4 trailers). If we stick
it below Signed-off-by, then it may suggest that Krzysztof reviewed the v4 of
the patch.

By placing it above the Signed-off-by line, we at least clearly indicate that
it's Neil who put it there.

-K

Re: DCO chain of custody revisited (was Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible)

[James Bottomley]

On Mon, 2022-10-31 at 13:23 -0400, Konstantin Ryabitsev wrote:
> On Mon, Oct 31, 2022 at 01:10:58PM -0400, James Bottomley wrote:
> > > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > > Reviewed-by: Krzysztof Kozlowski <
> > > > krzysztof.kozlowski@linaro.org>
> > > > Signed-off-by: Lee Jones <lee@kernel.org>
> > > 
> > > This would indicate that it's *Lee* who is claiming
> > > responsibility
> > > for collecting the Reviewed-by tag from Krzysztof, because it is
> > > in
> > > his chain of custody. However, this is not the case -- it was
> > > Neil
> > > who collected the tag, and therefore the "more correct" order
> > > should
> > > be:
> > > 
> > > > Reviewed-by: Krzysztof Kozlowski <
> > > > krzysztof.kozlowski@linaro.org>
> > > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > > Signed-off-by: Lee Jones <lee@kernel.org>
> > > 
> > > If my reasoning is incorrect, then I need to go back to the
> > > drawing
> > > board.
> > 
> > You're way over thinking this.
> 
> Yes, but it's my job to overthink this, so nobody else has to. :)
> 
> > The only tag that matters from the DCO point of view is Signed-off-
> > by. That's the ordering we care about for the chain of
> > custody.  All other tags are irrelevant.  Of course, it's nice to
> > think that reviews happen *after* the code was modified, which is
> > why most of us like to see the Reviewed-by after the initial author
> > signoff,
> 
> But this is where it becomes complicated. The Reviewed-By trailer was
> sent to the v2 of the series, and incorporated into v3(via b4
> trailers). If we stick it below Signed-off-by, then it may suggest
> that Krzysztof reviewed the v4 of the patch.

If the changes from v2 to v4 were material enough for that question to
be relevant then the Reviewed-by tag shouldn't have been kept because
the patch needed reviewing again.

> By placing it above the Signed-off-by line, we at least clearly
> indicate that it's Neil who put it there.

Who put it there does not matter, so it's not really a problem that
needs solving.

However, there is a problem if b4 is preserving Reviewed-by tags for
patches with material changes ... the review has to be redone and thus
the tag should be lost.  You can only keep Reviewed-by tags for
cosmetic changes (or obviously patches which don't change from version
to version).

James

Re: DCO chain of custody revisited (was Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible)

[Alex Elder]

On 10/31/22 12:23 PM, Konstantin Ryabitsev wrote:
> On Mon, Oct 31, 2022 at 01:10:58PM -0400, James Bottomley wrote:
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> Signed-off-by: Lee Jones <lee@kernel.org>
>>>
>>> This would indicate that it's *Lee* who is claiming responsibility
>>> for collecting the Reviewed-by tag from Krzysztof, because it is in
>>> his chain of custody. However, this is not the case -- it was Neil
>>> who collected the tag, and therefore the "more correct" order should
>>> be:
>>>
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> Signed-off-by: Lee Jones <lee@kernel.org>
>>>
>>> If my reasoning is incorrect, then I need to go back to the drawing
>>> board.
>>
>> You're way over thinking this.
> 
> Yes, but it's my job to overthink this, so nobody else has to. :)
> 
>> The only tag that matters from the DCO point of view is Signed-off-by.
>> That's the ordering we care about for the chain of custody.  All other tags
>> are irrelevant.  Of course, it's nice to think that reviews happen *after*
>> the code was modified, which is why most of us like to see the Reviewed-by
>> after the initial author signoff,
> 
> But this is where it becomes complicated. The Reviewed-By trailer was sent to
> the v2 of the series, and incorporated into v3(via b4 trailers). If we stick
> it below Signed-off-by, then it may suggest that Krzysztof reviewed the v4 of
> the patch.
> 
> By placing it above the Signed-off-by line, we at least clearly indicate that
> it's Neil who put it there.

I prefer this interpretation.  That is, the "outer" (last) sign-off
is essentially indicating that everything within (above) it is what
their sign-off applies to.

Most (all?) of the other tags (like Suggested-by, etc.) don't 
necessarily have a strong requirement or precise meaning, so in
a way it doesn't matter that much.  But I think this convention
adds a small amount of value.

					-Alex

> 
> -K
> 

Re: DCO chain of custody revisited (was Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible)

[Michael S. Tsirkin]

On Mon, Oct 31, 2022 at 12:58:42PM -0400, Konstantin Ryabitsev wrote:
> Hijacking this thread for greater good.
> 
> On Mon, Oct 31, 2022 at 04:35:38PM +0100, Neil Armstrong wrote:
> > Hi,
> > 
> > On 31/10/2022 16:32, Lee Jones wrote:
> > > On Fri, 21 Oct 2022, Neil Armstrong wrote:
> > > 
> > > > The PM8018 compatible is always used with PM8921 fallback, so PM8018
> > > > compatible can be safely removed from device ID table
> > > > 
> > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > 
> > > Tags should appear chronologically.
> > 
> > Indeed, they were added by b4, I'll report this.
> 
> My trouble is that there are seemingly as many opinions about the order of
> trailers as there are subsystem maintainers. The last time we had a long
> discussion about this on the users list I got a strong message that what
> matters most is the chain of custody, and the Signed-off-by trailer indicates
> the chain of custody boundary.
> 
> In the scenario below, the chain consists of 3 people:
> 
> | Suggested-by: Reporter 1 <...>
> | Signed-off-by: Developer 1 <...> -- initial DCO boundary
> | Reviewed-by: Reviewer 1 <...>
> | Tested-by: Tester 1 <...>
> | Signed-off-by: Submaintainer 1 <...> -- intermediate DCO boundary
> | Acked-by: Submaintainer 2 <...>
> | Signed-off-by: Maintainer 1 <...> -- final DCO boundary
> 
> In terms of DCO, this makes the following claims:
> 
> Developer 1:
>  - I am responsible for this change
>  - It was suggested by Reporter 1
> 
> Submaintainer 1:
>  - I am signing off on this change
>  - I have collected the trailers from Reviewer 1 and Tester 1
> 
> Maintainer 1:
>  - I am signing off on this change
>  - I have collected the trailer from Submaintainer 2
> 
> In the current case, and using this principle, the following order is correct:
> 
> | Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> | Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> Because the Reviewerd-by trailer was sent to the v2 of the series and was
> collected by Neil, so Neil is the person who is the DCO signatory of that
> chain of custody in the v4 of the series.
> 
> I assume that in the final commit Lee rearranged the tags in the following
> order:
> 
> | Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> | Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> | Signed-off-by: Lee Jones <lee@kernel.org>
> 
> This would indicate that it's *Lee* who is claiming responsibility for
> collecting the Reviewed-by tag from Krzysztof, because it is in his chain of
> custody. However, this is not the case -- it was Neil who collected the tag,
> and therefore the "more correct" order should be:


I think this is often incorrect:
I often pick a patch, then amend it adding tags from reviewers,
typically I add tags at the end just because it's easier,
and tools like git-interpret-trailers exist.

> | Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> | Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> | Signed-off-by: Lee Jones <lee@kernel.org>
> 
> If my reasoning is incorrect, then I need to go back to the drawing board.
> 
> -K

Re: DCO chain of custody revisited (was Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible)

[Konstantin Ryabitsev]

On Mon, Oct 31, 2022 at 01:33:27PM -0400, James Bottomley wrote:
> > But this is where it becomes complicated. The Reviewed-By trailer was
> > sent to the v2 of the series, and incorporated into v3(via b4
> > trailers). If we stick it below Signed-off-by, then it may suggest
> > that Krzysztof reviewed the v4 of the patch.
> 
> If the changes from v2 to v4 were material enough for that question to
> be relevant then the Reviewed-by tag shouldn't have been kept because
> the patch needed reviewing again.
> 
> > By placing it above the Signed-off-by line, we at least clearly
> > indicate that it's Neil who put it there.
> 
> Who put it there does not matter, so it's not really a problem that
> needs solving.
> 
> However, there is a problem if b4 is preserving Reviewed-by tags for
> patches with material changes ... the review has to be redone and thus
> the tag should be lost.  You can only keep Reviewed-by tags for
> cosmetic changes (or obviously patches which don't change from version
> to version).

I am not disagreeing, but as far as b4 is concerned, there is no way to
recognize this reliably. A rebase to a newer tree may have resulted in the
patch-id changing, with the actual logic remaining exactly the same. It's the
responsibility of the person submitting the series to make sure that the
trailers are still relevant -- the best I can do is to clearly indicate who
was the guilty party behind the decision.

-K

Re: DCO chain of custody revisited (was Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible)

[Willy Tarreau]

On Mon, Oct 31, 2022 at 12:33:30PM -0500, Alex Elder wrote:
> On 10/31/22 12:23 PM, Konstantin Ryabitsev wrote:
> > On Mon, Oct 31, 2022 at 01:10:58PM -0400, James Bottomley wrote:
> > > > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > > Signed-off-by: Lee Jones <lee@kernel.org>
> > > > 
> > > > This would indicate that it's *Lee* who is claiming responsibility
> > > > for collecting the Reviewed-by tag from Krzysztof, because it is in
> > > > his chain of custody. However, this is not the case -- it was Neil
> > > > who collected the tag, and therefore the "more correct" order should
> > > > be:
> > > > 
> > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > > > Signed-off-by: Lee Jones <lee@kernel.org>
> > > > 
> > > > If my reasoning is incorrect, then I need to go back to the drawing
> > > > board.
> > > 
> > > You're way over thinking this.
> > 
> > Yes, but it's my job to overthink this, so nobody else has to. :)
> > 
> > > The only tag that matters from the DCO point of view is Signed-off-by.
> > > That's the ordering we care about for the chain of custody.  All other tags
> > > are irrelevant.  Of course, it's nice to think that reviews happen *after*
> > > the code was modified, which is why most of us like to see the Reviewed-by
> > > after the initial author signoff,
> > 
> > But this is where it becomes complicated. The Reviewed-By trailer was sent to
> > the v2 of the series, and incorporated into v3(via b4 trailers). If we stick
> > it below Signed-off-by, then it may suggest that Krzysztof reviewed the v4 of
> > the patch.
> > 
> > By placing it above the Signed-off-by line, we at least clearly indicate that
> > it's Neil who put it there.
> 
> I prefer this interpretation.  That is, the "outer" (last) sign-off
> is essentially indicating that everything within (above) it is what
> their sign-off applies to.
> 
> Most (all?) of the other tags (like Suggested-by, etc.) don't necessarily
> have a strong requirement or precise meaning, so in
> a way it doesn't matter that much.  But I think this convention
> adds a small amount of value.

At least for stable backports we've been using this a lot because we
often have to adapt patches that get backported, and it's often
convenient to add a small comment summarizing the changes after the
last s-o-b and before the backporter's. But I agree that most tags
probably do not care much about ordering if the contents are not
changed.

Sometimes a maintainer might reword a commit message and in this case
maybe the reviewed-by order may be relevant.

Willy

Re: DCO chain of custody revisited (was Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible)

[Theodore Ts'o]

On Mon, Oct 31, 2022 at 01:33:27PM -0400, James Bottomley wrote:
> Who put it there does not matter, so it's not really a problem that
> needs solving.
> 
> However, there is a problem if b4 is preserving Reviewed-by tags for
> patches with material changes ... the review has to be redone and thus
> the tag should be lost.  You can only keep Reviewed-by tags for
> cosmetic changes (or obviously patches which don't change from version
> to version).

What I've seen some reviewers do is to request some additional changes
(please add more details in the comments, or please drop the test if
the poitner is non-NULL before calling kfree(), and then they add the
Reviewed-by tag.  In fact, in some cases they explicitly say, "please
fix these things and then you may add "Reviewed-by: Jon Q. Random
<random@example.com>"

(For example, IIRC, Jan Kara does this a fair amount.)

I understand that b4 can't possibly parse english, and if it does
start gaining Skynet abilities, we've got bigger problems, but suffice
it to say, there *are* times the Reviewed-By tag should be kept even
though non-commit-description and/or non-commit changes were made.

       			      	     			     - Ted

Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible

[Krzysztof Kozlowski]

On 31/10/2022 11:32, Lee Jones wrote:
> On Fri, 21 Oct 2022, Neil Armstrong wrote:
> 
>> The PM8018 compatible is always used with PM8921 fallback, so PM8018
>> compatible can be safely removed from device ID table
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> Tags should appear chronologically.
> 

I would assume that as well, but `b4 trailers` disagrees. It documents
even this behavior (the chain of custody) here:
https://b4.docs.kernel.org/en/latest/config.html

So while I agree with you, I also prefer the tools to make the decision
instead of humans (to follow the process, assuming the tool implements
the process). Either the tool should be fixed or the tool's decision is
correct.

Best regards,
Krzysztof

Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible

[Lee Jones]

On Wed, 02 Nov 2022, Krzysztof Kozlowski wrote:

> On 31/10/2022 11:32, Lee Jones wrote:
> > On Fri, 21 Oct 2022, Neil Armstrong wrote:
> > 
> >> The PM8018 compatible is always used with PM8921 fallback, so PM8018
> >> compatible can be safely removed from device ID table
> >>
> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > 
> > Tags should appear chronologically.
> > 
> 
> I would assume that as well, but `b4 trailers` disagrees. It documents
> even this behavior (the chain of custody) here:
> https://b4.docs.kernel.org/en/latest/config.html
> 
> So while I agree with you, I also prefer the tools to make the decision
> instead of humans (to follow the process, assuming the tool implements
> the process). Either the tool should be fixed or the tool's decision is
> correct.

Take a look further down the thread, since this has already been
heavily debated.  Not sure if there has been a clear winner or any
kind of agreement yet (if there ever will be), but until there is or I
until I am convinced otherwise, I'm going to continue on with the
chronological method.

Also, as far as I'm aware, tools like `b4` and the-like haven't been
mandated (yet?), so using its rules to dictate our actions probably
isn't the correct approach.

-- 
Lee Jones [李琼斯]

Re: DCO chain of custody revisited (was Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible)

[Krzysztof Kozlowski]

On 31/10/2022 13:23, Konstantin Ryabitsev wrote:
> On Mon, Oct 31, 2022 at 01:10:58PM -0400, James Bottomley wrote:
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> Signed-off-by: Lee Jones <lee@kernel.org>
>>>
>>> This would indicate that it's *Lee* who is claiming responsibility
>>> for collecting the Reviewed-by tag from Krzysztof, because it is in
>>> his chain of custody. However, this is not the case -- it was Neil
>>> who collected the tag, and therefore the "more correct" order should
>>> be:
>>>
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> Signed-off-by: Lee Jones <lee@kernel.org>
>>>
>>> If my reasoning is incorrect, then I need to go back to the drawing
>>> board.
>>
>> You're way over thinking this.
> 
> Yes, but it's my job to overthink this, so nobody else has to. :)

I appreciate overthinking, because it makes the life of patch submitters
easier - they can rely on a tool (b4) as a source of the process instead
of being corrected by maintainers with conflicting points of view.

> 
>> The only tag that matters from the DCO point of view is Signed-off-by.
>> That's the ordering we care about for the chain of custody.  All other tags
>> are irrelevant.  Of course, it's nice to think that reviews happen *after*
>> the code was modified, which is why most of us like to see the Reviewed-by
>> after the initial author signoff,

I guess it depends then what is the meaning of SoB. In my understanding,
DCO (and SoB) only state the contents of the contribution, so:
1. I write patch (or collect it from somewhere etc).
2. I signed it with SoB.
3. There was a review tag given, so I add a review tag.

My SoB does not certify order of review tags or any other pieces than
what is explained in DCO: I created the contribution and I am allowed to
send it.

Therefore Rb/Ack/tags go after my SoB.

https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#:~:text=The%20Signed%2Doff%2Dby%3A,in%20the%20patch's%20delivery%20path.
is even saying:
"Some people also put extra tags at the end. "
so some tags are *after*.

> 
> But this is where it becomes complicated. The Reviewed-By trailer was sent to
> the v2 of the series, and incorporated into v3(via b4 trailers). If we stick
> it below Signed-off-by, then it may suggest that Krzysztof reviewed the v4 of
> the patch.

No, you cannot imply the order from that. That's only one possibility.

> By placing it above the Signed-off-by line, we at least clearly indicate that
> it's Neil who put it there.

True, but that's not the point of SoB... I can also argue that this
implies that review happened before contribution was signed off.

Best regards,
Krzysztof

Re: (subset) [PATCH v4 00/11] arm: qcom: mdm9615: first round of bindings and DT fixes

[Bjorn Andersson]

On Fri, 21 Oct 2022 11:06:36 +0200, Neil Armstrong wrote:
> This is a first round of trivial bindings & DT fixes for the MDM9615 platform.
> 
> This first round focuses on trivial changes, the remaining work will
> mainly be .txt to .yaml transition of old qcom pmic & co device bindings.
> 
> 

Applied, thanks!

[01/11] dt-bindings: arm: qcom: move swir,mangoh-green-wp8548 board documentation to qcom.yaml
        commit: f4ec5f28af13e2b8e62ae173cb6827e137cdd8cc
[02/11] arm: dts: qcom: mdm9615*: add SPDX-License-Identifier
        commit: c69af934db18ad165b1dc84f5450fa55afb34acb
[03/11] arm: dts: qcom: mdm9615: add missing reg in cpu@0 node
        commit: e58bdf93db08c16dd06bc1967e978708b44d9c83
[04/11] arm: dts: qcom: mdm9615: remove invalid spi-max-frequency gsbi3_spi node
        commit: 75353420d0d0abe3a57cedf4a6cfa00ea05842a3
[10/11] arm: dts: qcom: mdm9615: remove invalid interrupt-names from pl18x mmc nodes
        commit: 3627dd180c67d3e589c38a10b4be29a0352a70b6
[11/11] arm: dts: qcom: mdm9615: remove useless amba subnode
        commit: 10de96ba6d4287220962cdd82826b6a14af90e2e

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>