[PATCH 00/11] Initial support for the Fairphone 5 smartphone

[PATCH 00/11] Initial support for the Fairphone 5 smartphone

[Luca Weiss]

Add support to boot up mainline kernel on the QCM6490-based Fairphone 5
smartphone.

These patches only cover a part of the functionality brought up on
mainline so far, with the rest needing larger dts and driver changes or
depend on patches that are not yet merged. I will work on sending those
once these base patches here have settled.

Since QCM6490, like SC7280 are 'yupik' in the vendor-provided kernel, we
can base the dts on it and leverage existing support. Though current
sc7280 support mostly assumes ChromeOS devices which have a different
TrustZone setup, so we need to move some ChromeOS-specific bits to the
sc7280-chrome-common.dtsi file to make it boot on a standard TZ board.

Depends on (just for the #include in sc7280.dtsi):
https://lore.kernel.org/linux-arm-msm/20230818-qcom-vmid-defines-v1-1-45b610c96b13@fairphone.com/

The pm7250b patch has been picked up from this series:
https://lore.kernel.org/linux-arm-msm/20230407-pm7250b-sid-v1-2-fc648478cc25@fairphone.com/

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
Luca Weiss (11):
      arm64: dts: qcom: sc7280: Mark some nodes as 'reserved'
      nvmem: qfprom: Mark core clk as optional
      arm64: dts: qcom: sc7280: Move qfprom clock to chrome-common
      arm64: dts: qcom: pm7250b: make SID configurable
      arm64: dts: qcom: pm8350c: Add flash led node
      dt-bindings: pinctrl: qcom,sc7280: Allow gpio-reserved-ranges
      dt-bindings: arm: qcom,ids: Add SoC ID for QCM6490
      soc: qcom: socinfo: Add SoC ID for QCM6490
      cpufreq: Add QCM6490 to cpufreq-dt-platdev blocklist
      dt-bindings: arm: qcom: Add QCM6490 Fairphone 5
      arm64: dts: qcom: qcm6490: Add device-tree for Fairphone 5

 Documentation/devicetree/bindings/arm/qcom.yaml    |   6 +
 .../bindings/pinctrl/qcom,sc7280-pinctrl.yaml      |   4 +
 arch/arm64/boot/dts/qcom/Makefile                  |   1 +
 arch/arm64/boot/dts/qcom/pm7250b.dtsi              |  23 +-
 arch/arm64/boot/dts/qcom/pm8350c.dtsi              |   6 +
 arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 659 +++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |  17 +
 arch/arm64/boot/dts/qcom/sc7280.dtsi               |   7 +-
 drivers/cpufreq/cpufreq-dt-platdev.c               |   1 +
 drivers/nvmem/qfprom.c                             |   2 +-
 drivers/soc/qcom/socinfo.c                         |   1 +
 include/dt-bindings/arm/qcom,ids.h                 |   1 +
 12 files changed, 717 insertions(+), 11 deletions(-)
---
base-commit: 0255bba921438ea1e45d3f0873c3e8c5a1e03876
change-id: 20230818-fp5-initial-b6c8210ba9c8

Best regards,
-- 
Luca Weiss <luca.weiss@fairphone.com>

[PATCH 01/11] arm64: dts: qcom: sc7280: Mark some nodes as 'reserved'

[Luca Weiss]

With the standard Qualcomm TrustZone setup, components such as lpasscc,
pdc_reset and watchdog shouldn't be touched by Linux. Mark them with
the status 'reserved' and reeable them in the chrome-common dtsi.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 12 ++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi               |  5 ++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
index 2e1cd219fc18..8eb30aa226a2 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -46,6 +46,14 @@ wpss_mem: memory@9ae00000 {
 	};
 };
 
+&lpasscc {
+	status = "okay";
+};
+
+&pdc_reset {
+	status = "okay";
+};
+
 /* The PMIC PON code isn't compatible w/ how Chrome EC/BIOS handle things. */
 &pmk8350_pon {
 	status = "disabled";
@@ -84,6 +92,10 @@ &scm {
 	dma-coherent;
 };
 
+&watchdog {
+	status = "okay";
+};
+
 &wifi {
 	status = "okay";
 
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 042908048d09..98a8d627a348 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2267,6 +2267,7 @@ lpasscc: lpasscc@3000000 {
 			clocks = <&gcc GCC_CFG_NOC_LPASS_CLK>;
 			clock-names = "iface";
 			#clock-cells = <1>;
+			status = "reserved";
 		};
 
 		lpass_rx_macro: codec@3200000 {
@@ -4216,6 +4217,7 @@ pdc_reset: reset-controller@b5e0000 {
 			compatible = "qcom,sc7280-pdc-global";
 			reg = <0 0x0b5e0000 0 0x20000>;
 			#reset-cells = <1>;
+			status = "reserved";
 		};
 
 		tsens0: thermal-sensor@c263000 {
@@ -5212,11 +5214,12 @@ msi-controller@17a40000 {
 			};
 		};
 
-		watchdog@17c10000 {
+		watchdog: watchdog@17c10000 {
 			compatible = "qcom,apss-wdt-sc7280", "qcom,kpss-wdt";
 			reg = <0 0x17c10000 0 0x1000>;
 			clocks = <&sleep_clk>;
 			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+			status = "reserved";
 		};
 
 		timer@17c20000 {

-- 
2.42.0

[PATCH 02/11] nvmem: qfprom: Mark core clk as optional

[Luca Weiss]

On some platforms like sc7280 on non-ChromeOS devices the core clock
cannot be touched by Linux so we cannot provide it. Mark it as optional
as accessing qfprom works without it.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/nvmem/qfprom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
index 14814cba2dd6..739dc502b39e 100644
--- a/drivers/nvmem/qfprom.c
+++ b/drivers/nvmem/qfprom.c
@@ -423,7 +423,7 @@ static int qfprom_probe(struct platform_device *pdev)
 		if (IS_ERR(priv->vcc))
 			return PTR_ERR(priv->vcc);
 
-		priv->secclk = devm_clk_get(dev, "core");
+		priv->secclk = devm_clk_get_optional(dev, "core");
 		if (IS_ERR(priv->secclk))
 			return dev_err_probe(dev, PTR_ERR(priv->secclk), "Error getting clock\n");
 

-- 
2.42.0

[PATCH 03/11] arm64: dts: qcom: sc7280: Move qfprom clock to chrome-common

[Luca Weiss]

On non-ChromeOS boards the clock cannot be touched, so move it in the
chrome-common dtsi which is the only place where it's needed.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 5 +++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi               | 2 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
index 8eb30aa226a2..6cfcec1eabd9 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -59,6 +59,11 @@ &pmk8350_pon {
 	status = "disabled";
 };
 
+&qfprom {
+	clocks = <&gcc GCC_SEC_CTRL_CLK_SRC>;
+	clock-names = "core";
+};
+
 /*
  * Chrome designs always boot from SPI flash hooked up to the qspi.
  *
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 98a8d627a348..5c78038369fd 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -897,8 +897,6 @@ qfprom: efuse@784000 {
 			      <0 0x00780000 0 0xa20>,
 			      <0 0x00782000 0 0x120>,
 			      <0 0x00786000 0 0x1fff>;
-			clocks = <&gcc GCC_SEC_CTRL_CLK_SRC>;
-			clock-names = "core";
 			power-domains = <&rpmhpd SC7280_MX>;
 			#address-cells = <1>;
 			#size-cells = <1>;

-- 
2.42.0

[PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Luca Weiss]

Like other Qualcomm PMICs the PM7250B can be used on different addresses
on the SPMI bus. Use similar defines like the PMK8350 to make this
possible.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
index e8540c36bd99..3514de536baa 100644
--- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
@@ -7,6 +7,15 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
+/* This PMIC can be configured to be at different SIDs */
+#ifndef PM7250B_SID
+	#define PM7250B_SID 2
+#endif
+
+#ifndef PM7250B_SID1
+	#define PM7250B_SID1 3
+#endif
+
 / {
 	thermal-zones {
 		pm7250b-thermal {
@@ -39,16 +48,16 @@ trip2 {
 };
 
 &spmi_bus {
-	pmic@2 {
+	pmic@PM7250B_SID {
 		compatible = "qcom,pm7250b", "qcom,spmi-pmic";
-		reg = <0x2 SPMI_USID>;
+		reg = <PM7250B_SID SPMI_USID>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 
 		pm7250b_temp: temp-alarm@2400 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0x2400>;
-			interrupts = <0x2 0x24 0x0 IRQ_TYPE_EDGE_BOTH>;
+			interrupts = <PM7250B_SID 0x24 0x0 IRQ_TYPE_EDGE_BOTH>;
 			io-channels = <&pm7250b_adc ADC5_DIE_TEMP>;
 			io-channel-names = "thermal";
 			#thermal-sensor-cells = <0>;
@@ -60,7 +69,7 @@ pm7250b_adc: adc@3100 {
 			#address-cells = <1>;
 			#size-cells = <0>;
 			#io-channel-cells = <1>;
-			interrupts = <0x2 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+			interrupts = <PM7250B_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
 
 			channel@0 {
 				reg = <ADC5_REF_GND>;
@@ -141,7 +150,7 @@ channel@99 {
 		pm7250b_adc_tm: adc-tm@3500 {
 			compatible = "qcom,spmi-adc-tm5";
 			reg = <0x3500>;
-			interrupts = <0x2 0x35 0x0 IRQ_TYPE_EDGE_RISING>;
+			interrupts = <PM7250B_SID 0x35 0x0 IRQ_TYPE_EDGE_RISING>;
 			#thermal-sensor-cells = <1>;
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -159,9 +168,9 @@ pm7250b_gpios: pinctrl@c000 {
 		};
 	};
 
-	pmic@3 {
+	pmic@PM7250B_SID1 {
 		compatible = "qcom,pm7250b", "qcom,spmi-pmic";
-		reg = <0x3 SPMI_USID>;
+		reg = <PM7250B_SID1 SPMI_USID>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 	};

-- 
2.42.0

[PATCH 05/11] arm64: dts: qcom: pm8350c: Add flash led node

[Luca Weiss]

Add a node for the led controller found on PM8350C, used for flash and
torch purposes.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/pm8350c.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
index f28e71487d5c..aa74e21fe0dc 100644
--- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
@@ -30,6 +30,12 @@ pm8350c_gpios: gpio@8800 {
 			#interrupt-cells = <2>;
 		};
 
+		pm8350c_flash: led-controller@ee00 {
+			compatible = "qcom,pm8350c-flash-led", "qcom,spmi-flash-led";
+			reg = <0xee00>;
+			status = "disabled";
+		};
+
 		pm8350c_pwm: pwm {
 			compatible = "qcom,pm8350c-pwm";
 			#pwm-cells = <2>;

-- 
2.42.0

[PATCH 06/11] dt-bindings: pinctrl: qcom,sc7280: Allow gpio-reserved-ranges

[Luca Weiss]

Allow the gpio-reserved-ranges property on SC7280 TLMM.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml
index 368d44ff5468..c8735ab97e40 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml
@@ -41,6 +41,10 @@ properties:
   gpio-ranges:
     maxItems: 1
 
+  gpio-reserved-ranges:
+    minItems: 1
+    maxItems: 88
+
   gpio-line-names:
     maxItems: 175
 

-- 
2.42.0

[PATCH 07/11] dt-bindings: arm: qcom,ids: Add SoC ID for QCM6490

[Luca Weiss]

Add the ID for the Qualcomm QCM6490 SoC.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 include/dt-bindings/arm/qcom,ids.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/arm/qcom,ids.h b/include/dt-bindings/arm/qcom,ids.h
index be12e1dd1f38..b6aafb988e08 100644
--- a/include/dt-bindings/arm/qcom,ids.h
+++ b/include/dt-bindings/arm/qcom,ids.h
@@ -233,6 +233,7 @@
 #define QCOM_ID_SM8450_3		482
 #define QCOM_ID_SC7280			487
 #define QCOM_ID_SC7180P			495
+#define QCOM_ID_QCM6490			497
 #define QCOM_ID_IPQ5000			503
 #define QCOM_ID_IPQ0509			504
 #define QCOM_ID_IPQ0518			505

-- 
2.42.0

[PATCH 08/11] soc: qcom: socinfo: Add SoC ID for QCM6490

[Luca Weiss]

Add SoC ID table entries for Qualcomm QCM6490.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/soc/qcom/socinfo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
index 497cfb720fcb..649732bf9f88 100644
--- a/drivers/soc/qcom/socinfo.c
+++ b/drivers/soc/qcom/socinfo.c
@@ -389,6 +389,7 @@ static const struct soc_id soc_id[] = {
 	{ qcom_board_id_named(SM8450_3, "SM8450") },
 	{ qcom_board_id(SC7280) },
 	{ qcom_board_id(SC7180P) },
+	{ qcom_board_id(QCM6490) },
 	{ qcom_board_id(IPQ5000) },
 	{ qcom_board_id(IPQ0509) },
 	{ qcom_board_id(IPQ0518) },

-- 
2.42.0

[PATCH 09/11] cpufreq: Add QCM6490 to cpufreq-dt-platdev blocklist

[Luca Weiss]

The Qualcomm QCM6490 platform uses the qcom-cpufreq-hw driver, so add it
to the cpufreq-dt-platdev driver's blocklist.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index fb2875ce1fdd..02ec58a8603b 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -145,6 +145,7 @@ static const struct of_device_id blocklist[] __initconst = {
 	{ .compatible = "qcom,msm8996", },
 	{ .compatible = "qcom,msm8998", },
 	{ .compatible = "qcom,qcm2290", },
+	{ .compatible = "qcom,qcm6490", },
 	{ .compatible = "qcom,qcs404", },
 	{ .compatible = "qcom,qdu1000", },
 	{ .compatible = "qcom,sa8155p" },

-- 
2.42.0

[PATCH 10/11] dt-bindings: arm: qcom: Add QCM6490 Fairphone 5

[Luca Weiss]

Fairphone 5 is a smartphone based on the QCM6490 SoC.

Also allow qcom,board-id and qcom,msm-id for QCM6490 since it's required
by the bootloader.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index adbfaea32343..b09a41812cf0 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -391,6 +391,11 @@ properties:
           - const: qcom,qrb2210
           - const: qcom,qcm2290
 
+      - items:
+          - enum:
+              - fairphone,fp5
+          - const: qcom,qcm6490
+
       - description: Qualcomm Technologies, Inc. Distributed Unit 1000 platform
         items:
           - enum:
@@ -1074,6 +1079,7 @@ allOf:
               - qcom,msm8994
               - qcom,msm8996
               - qcom,msm8998
+              - qcom,qcm6490
               - qcom,sdm450
               - qcom,sdm630
               - qcom,sdm632

-- 
2.42.0

[PATCH 11/11] arm64: dts: qcom: qcm6490: Add device-tree for Fairphone 5

[Luca Weiss]

Add device tree for the Fairphone 5 smartphone which is based on
the QCM6490 SoC.

Supported features are, as of now:
* Bluetooth
* Debug UART
* Display via simplefb
* Flash/torch LED
* Flip cover sensor
* Power & volume buttons
* RTC
* SD card
* USB
* Various plumbing like regulators, i2c, spi, etc

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/Makefile                  |   1 +
 arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 659 +++++++++++++++++++++
 2 files changed, 660 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 2cca20563a1d..73c3be0f8872 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -81,6 +81,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-sony-xperia-yoshino-lilac.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-sony-xperia-yoshino-maple.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-sony-xperia-yoshino-poplar.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-xiaomi-sagit.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= qcm6490-fairphone-fp5.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-1000.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-4000.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
new file mode 100644
index 000000000000..572b254d3af2
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -0,0 +1,659 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2023, Luca Weiss <luca.weiss@fairphone.com>
+ */
+
+/dts-v1/;
+
+/* PM7250B is configured to use SID8/9 instead of SID2/3 */
+#define PM7250B_SID 8
+#define PM7250B_SID1 9
+
+#include <dt-bindings/arm/qcom,ids.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+#include "sc7280.dtsi"
+#include "pm7250b.dtsi"
+#include "pm7325.dtsi"
+#include "pm8350c.dtsi" /* PM7350C */
+#include "pmk8350.dtsi" /* PMK7325 */
+
+/ {
+	model = "Fairphone 5";
+	compatible = "fairphone,fp5", "qcom,qcm6490";
+	chassis-type = "handset";
+
+	/* required for bootloader to select correct board */
+	qcom,msm-id = <QCOM_ID_QCM6490 0x10000>;
+	qcom,board-id = <34 0>;
+
+	aliases {
+		serial0 = &uart5;
+		serial1 = &uart7;
+	};
+
+	chosen {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		framebuffer0: framebuffer@a000000 {
+			compatible = "simple-framebuffer";
+			reg = <0 0xe1000000 0 (2700 * 1224 * 4)>;
+			width = <1224>;
+			height = <2700>;
+			stride = <(1224 * 4)>;
+			format = "a8r8g8b8";
+			panel = <&panel>;
+			clocks = <&gcc GCC_DISP_HF_AXI_CLK>;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		pinctrl-0 = <&volume_down_default>, <&hall_sensor_default>;
+		pinctrl-names = "default";
+
+		key-volume-up {
+			label = "Volume up";
+			gpios = <&pm7325_gpios 6 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_VOLUMEUP>;
+		};
+
+		/* Hall sensor uses vreg_l8c as VDD, it's being enabled using
+		 * always-on on the regulator
+		 */
+		event-hall-sensor {
+			label = "Hall Effect Sensor";
+			gpios = <&tlmm 155 GPIO_ACTIVE_LOW>;
+			linux,input-type = <EV_SW>;
+			linux,code = <SW_LID>;
+			linux,can-disable;
+			wakeup-source;
+		};
+	};
+
+	panel: panel {
+		compatible = "boe,rm692e5";
+
+		width-mm = <68>;
+		height-mm = <150>;
+	};
+
+	reserved-memory {
+		cont_splash_mem: cont-splash@e1000000 {
+			reg = <0 0xe1000000 0 0x2300000>;
+			no-map;
+		};
+
+		adsp_mem: adsp@86700000 {
+			reg = <0x0 0x86700000 0x0 0x2800000>;
+			no-map;
+		};
+
+		cdsp_mem: cdsp@88f00000 {
+			reg = <0x0 0x88f00000 0x0 0x1e00000>;
+			no-map;
+		};
+
+		mpss_mem: mpss@8b800000 {
+			reg = <0x0 0x8b800000 0x0 0xf600000>;
+			no-map;
+		};
+
+		wpss_mem: wpss@9ae00000 {
+			reg = <0x0 0x9ae00000 0x0 0x1900000>;
+			no-map;
+		};
+	};
+
+	ois_avdd0_1p8: regulator-ois-avdd0-1p8 {
+		compatible = "regulator-fixed";
+		regulator-name = "OIS_AVDD0_1P8";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		gpio = <&tlmm 157 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		vin-supply = <&vreg_bob>;
+	};
+
+	ois_dvdd_1p1: regulator-ois-dvdd-1p1 {
+		compatible = "regulator-fixed";
+		regulator-name = "OIS_DVDD_1P1";
+		regulator-min-microvolt = <1100000>;
+		regulator-max-microvolt = <1100000>;
+		gpio = <&tlmm 97 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		vin-supply = <&vreg_s8b>;
+	};
+
+	afvdd_2p8: regulator-afvdd-2p8 {
+		compatible = "regulator-fixed";
+		regulator-name = "AFVDD_2P8";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		gpio = <&tlmm 68 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		vin-supply = <&vreg_bob>;
+	};
+};
+
+&apps_rsc {
+	regulators-0 {
+		compatible = "qcom,pm7325-rpmh-regulators";
+		qcom,pmic-id = "b";
+
+		vreg_s1b: smps1 {
+			regulator-min-microvolt = <1840000>;
+			regulator-max-microvolt = <2040000>;
+		};
+
+		vreg_s7b: smps7 {
+			regulator-min-microvolt = <535000>;
+			regulator-max-microvolt = <1120000>;
+		};
+
+		vreg_s8b: smps8 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1500000>;
+		};
+
+		vreg_l1b: ldo1 {
+			regulator-min-microvolt = <825000>;
+			regulator-max-microvolt = <925000>;
+		};
+
+		vreg_l2b: ldo2 {
+			regulator-min-microvolt = <2700000>;
+			regulator-max-microvolt = <3544000>;
+		};
+
+		vreg_l3b: ldo3 {
+			regulator-min-microvolt = <312000>;
+			regulator-max-microvolt = <910000>;
+		};
+
+		vreg_l6b: ldo6 {
+			regulator-min-microvolt = <1140000>;
+			regulator-max-microvolt = <1260000>;
+		};
+
+		vreg_l7b: ldo7 {
+			regulator-min-microvolt = <2400000>;
+			regulator-max-microvolt = <3544000>;
+		};
+
+		vreg_l8b: ldo8 {
+			regulator-min-microvolt = <870000>;
+			regulator-max-microvolt = <970000>;
+		};
+
+		vreg_l9b: ldo9 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1304000>;
+		};
+
+		vreg_l11b: ldo11 {
+			regulator-min-microvolt = <1504000>;
+			regulator-max-microvolt = <2000000>;
+		};
+
+		vreg_l12b: ldo12 {
+			regulator-min-microvolt = <751000>;
+			regulator-max-microvolt = <824000>;
+		};
+
+		vreg_l13b: ldo13 {
+			regulator-min-microvolt = <530000>;
+			regulator-max-microvolt = <824000>;
+		};
+
+		vreg_l14b: ldo14 {
+			regulator-min-microvolt = <1080000>;
+			regulator-max-microvolt = <1304000>;
+		};
+
+		vreg_l15b: ldo15 {
+			regulator-min-microvolt = <765000>;
+			regulator-max-microvolt = <1020000>;
+		};
+
+		vreg_l16b: ldo16 {
+			regulator-min-microvolt = <1100000>;
+			regulator-max-microvolt = <1300000>;
+		};
+
+		vreg_l17b: ldo17 {
+			regulator-min-microvolt = <1700000>;
+			regulator-max-microvolt = <1900000>;
+		};
+
+		vreg_l18b: ldo18 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2000000>;
+		};
+
+		vreg_l19b: ldo19 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2000000>;
+		};
+	};
+
+	regulators-1 {
+		compatible = "qcom,pm8350c-rpmh-regulators";
+		qcom,pmic-id = "c";
+
+		vreg_s1c: smps1 {
+			regulator-min-microvolt = <2190000>;
+			regulator-max-microvolt = <2210000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_s9c: smps9 {
+			regulator-min-microvolt = <1010000>;
+			regulator-max-microvolt = <1170000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l1c: ldo1 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1980000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l2c: ldo2 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1950000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l3c: ldo3 {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3400000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l4c: ldo4 {
+			regulator-min-microvolt = <1620000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l5c: ldo5 {
+			regulator-min-microvolt = <1620000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l6c: ldo6 {
+			regulator-min-microvolt = <1650000>;
+			regulator-max-microvolt = <3544000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l7c: ldo7 {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3544000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l8c: ldo8 {
+			regulator-min-microvolt = <1620000>;
+			regulator-max-microvolt = <2000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			/* Hall sensor VDD */
+			regulator-always-on;
+		};
+
+		vreg_l9c: ldo9 {
+			regulator-min-microvolt = <2700000>;
+			regulator-max-microvolt = <3544000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l10c: ldo10 {
+			regulator-min-microvolt = <720000>;
+			regulator-max-microvolt = <1050000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l11c: ldo11 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <3544000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l12c: ldo12 {
+			regulator-min-microvolt = <1650000>;
+			regulator-max-microvolt = <2000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l13c: ldo13 {
+			regulator-min-microvolt = <2700000>;
+			regulator-max-microvolt = <3544000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_bob: bob {
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3960000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+		};
+	};
+};
+
+&dispcc {
+	/* Disable for now so simple-framebuffer continues working */
+	status = "disabled";
+};
+
+&gcc {
+	protected-clocks = <GCC_CFG_NOC_LPASS_CLK>,
+			   <GCC_EDP_CLKREF_EN>,
+			   <GCC_MSS_CFG_AHB_CLK>,
+			   <GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>,
+			   <GCC_MSS_OFFLINE_AXI_CLK>,
+			   <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
+			   <GCC_MSS_Q6_MEMNOC_AXI_CLK>,
+			   <GCC_MSS_SNOC_AXI_CLK>,
+			   <GCC_QSPI_CNOC_PERIPH_AHB_CLK>,
+			   <GCC_QSPI_CORE_CLK>,
+			   <GCC_QSPI_CORE_CLK_SRC>,
+			   <GCC_SEC_CTRL_CLK_SRC>,
+			   <GCC_WPSS_AHB_BDG_MST_CLK>,
+			   <GCC_WPSS_AHB_CLK>,
+			   <GCC_WPSS_RSCP_CLK>;
+};
+
+&gpi_dma0 {
+	status = "okay";
+};
+
+&gpi_dma1 {
+	status = "okay";
+};
+
+&i2c1 {
+	status = "okay";
+
+	/* PM8008 PMIC @ 8 and 9 */
+	/* Pixelworks @ 26 */
+	/* FSA4480 USB audio switch @ 42 */
+	/* AW86927FCR haptics @ 5a */
+};
+
+&i2c2 {
+	status = "okay";
+
+	/* AW88261FCR amplifier @ 34 */
+	/* AW88261FCR amplifier @ 35 */
+};
+
+&i2c4 {
+	status = "okay";
+
+	/* PTN36502 USB redriver @ 1a */
+};
+
+&i2c9 {
+	status = "okay";
+
+	/* ST21NFC NFC @ 28 */
+	/* VL53L3 ToF @ 29 */
+};
+
+&ipa {
+	qcom,gsi-loader = "self";
+	memory-region = <&ipa_fw_mem>;
+	firmware-name = "qcom/qcm6490/fairphone5/ipa_fws.mdt";
+	status = "okay";
+};
+
+&pm7325_gpios {
+	volume_down_default: volume-down-default-state {
+		pins = "gpio6";
+		function = PMIC_GPIO_FUNC_NORMAL;
+		power-source = <1>;
+		bias-pull-up;
+		input-enable;
+	};
+};
+
+&pm8350c_flash {
+	status = "okay";
+
+	led-0 {
+		function = LED_FUNCTION_FLASH;
+		color = <LED_COLOR_ID_WHITE>;
+		led-sources = <1>, <4>;
+		led-max-microamp = <500000>;
+		flash-max-microamp = <1500000>;
+		flash-max-timeout-us = <1280000>;
+	};
+};
+
+&pmk8350_rtc {
+	status = "okay";
+};
+
+&pon_pwrkey {
+	status = "okay";
+};
+
+&pon_resin {
+	status = "okay";
+	linux,code = <KEY_VOLUMEDOWN>;
+};
+
+&qup_spi13_cs {
+	drive-strength = <6>;
+	bias-disable;
+};
+
+&qup_spi13_data_clk {
+	drive-strength = <6>;
+	bias-disable;
+};
+
+&qup_uart5_rx {
+	drive-strength = <2>;
+	bias-disable;
+};
+
+&qup_uart5_tx {
+	drive-strength = <2>;
+	bias-disable;
+};
+
+&qupv3_id_0 {
+	status = "okay";
+};
+
+&qupv3_id_1 {
+	status = "okay";
+};
+
+&rmtfs_mem {
+	qcom,vmid = <QCOM_SCM_VMID_MSS_MSA>, <QCOM_SCM_VMID_NAV>;
+	reg = <0x0 0xf8500000 0x0 0x600000>;
+};
+
+&sdc2_clk {
+	drive-strength = <16>;
+	bias-disable;
+};
+
+&sdc2_cmd {
+	drive-strength = <10>;
+	bias-pull-up;
+};
+
+&sdc2_data {
+	drive-strength = <10>;
+	bias-pull-up;
+};
+
+&sdhc_2 {
+	vmmc-supply = <&vreg_l9c>;
+	vqmmc-supply = <&vreg_l6c>;
+
+	pinctrl-0 = <&sdc2_clk>, <&sdc2_cmd>, <&sdc2_data>;
+	pinctrl-1 = <&sdc2_clk_sleep>, <&sdc2_cmd_sleep>, <&sdc2_data_sleep>;
+
+	status = "okay";
+};
+
+&spi13 {
+	status = "okay";
+
+	/* Goodix touchscreen @ 0 */
+};
+
+&tlmm {
+	/*
+	 * 32-33: SMB1394 (SPMI)
+	 * 56-59: fingerprint reader (SPI)
+	 */
+	gpio-reserved-ranges = <32 2>, <56 4>;
+
+	bluetooth_enable_default: bluetooth-enable-default-state {
+		pins = "gpio85";
+		function = "gpio";
+		output-low;
+		bias-disable;
+	};
+
+	hall_sensor_default: hall-sensor-default-state {
+		pins = "gpio155";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
+	qup_uart7_sleep_cts: qup-uart7-sleep-cts-state {
+		pins = "gpio28";
+		function = "gpio";
+		/*
+		 * Configure a bias-bus-hold on CTS to lower power
+		 * usage when Bluetooth is turned off. Bus hold will
+		 * maintain a low power state regardless of whether
+		 * the Bluetooth module drives the pin in either
+		 * direction or leaves the pin fully unpowered.
+		 */
+		bias-bus-hold;
+	};
+
+	qup_uart7_sleep_rts: qup-uart7-sleep-rts-state {
+		pins = "gpio29";
+		function = "gpio";
+		/*
+		 * Configure pull-down on RTS. As RTS is active low
+		 * signal, pull it low to indicate the BT SoC that it
+		 * can wakeup the system anytime from suspend state by
+		 * pulling RX low (by sending wakeup bytes).
+		 */
+		bias-pull-down;
+	};
+
+	qup_uart7_sleep_tx: qup-uart7-sleep-tx-state {
+		pins = "gpio30";
+		function = "gpio";
+		/*
+		 * Configure pull-up on TX when it isn't actively driven
+		 * to prevent BT SoC from receiving garbage during sleep.
+		 */
+		bias-pull-up;
+	};
+
+	qup_uart7_sleep_rx: qup-uart7-sleep-rx-state {
+		pins = "gpio31";
+		function = "gpio";
+		/*
+		 * Configure a pull-up on RX. This is needed to avoid
+		 * garbage data when the TX pin of the Bluetooth module
+		 * is floating which may cause spurious wakeups.
+		 */
+		bias-pull-up;
+	};
+
+	sw_ctrl_default: sw-ctrl-default-state {
+		pins = "gpio86";
+		function = "gpio";
+		bias-pull-down;
+	};
+};
+
+&uart5 {
+	compatible = "qcom,geni-debug-uart";
+	status = "okay";
+};
+
+&uart7 {
+	/delete-property/interrupts;
+	interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>,
+			      <&tlmm 31 IRQ_TYPE_EDGE_FALLING>;
+
+	pinctrl-names = "default", "sleep";
+	pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>;
+
+	status = "okay";
+
+	bluetooth: bluetooth {
+		compatible = "qcom,wcn6750-bt";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&bluetooth_enable_default>, <&sw_ctrl_default>;
+
+		enable-gpios = <&tlmm 85 GPIO_ACTIVE_HIGH>;
+		swctrl-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>;
+
+		vddio-supply = <&vreg_l19b>;
+		vddaon-supply = <&vreg_s7b>;
+		vddbtcxmx-supply = <&vreg_s7b>;
+		vddrfacmn-supply = <&vreg_s7b>;
+		vddrfa0p8-supply = <&vreg_s7b>;
+		vddrfa1p7-supply = <&vreg_s1b>;
+		vddrfa1p2-supply = <&vreg_s8b>;
+		vddrfa2p2-supply = <&vreg_s1c>;
+		vddasd-supply = <&vreg_l11c>;
+
+		max-speed = <3200000>;
+	};
+};
+
+&usb_1 {
+	status = "okay";
+};
+
+&usb_1_dwc3 {
+	dr_mode = "peripheral";
+};
+
+&usb_1_hsphy {
+	vdda-pll-supply = <&vreg_l10c>;
+	vdda18-supply = <&vreg_l1c>;
+	vdda33-supply = <&vreg_l2b>;
+
+	qcom,hs-crossover-voltage-microvolt = <28000>;
+	qcom,hs-output-impedance-micro-ohms = <2600000>;
+	qcom,hs-rise-fall-time-bp = <5430>;
+	qcom,hs-disconnect-bp = <1743>;
+	qcom,hs-amplitude-bp = <2430>;
+
+	qcom,pre-emphasis-amplitude-bp = <20000>;
+	qcom,pre-emphasis-duration-bp = <20000>;
+
+	qcom,squelch-detector-bp = <(-2090)>;
+
+	status = "okay";
+};
+
+&usb_1_qmpphy {
+	vdda-phy-supply = <&vreg_l6b>;
+	vdda-pll-supply = <&vreg_l1b>;
+
+	status = "okay";
+};

-- 
2.42.0

Re: [PATCH 02/11] nvmem: qfprom: Mark core clk as optional

[Krzysztof Kozlowski]

On 30/08/2023 11:58, Luca Weiss wrote:
> On some platforms like sc7280 on non-ChromeOS devices the core clock
> cannot be touched by Linux so we cannot provide it. Mark it as optional
> as accessing qfprom works without it.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  drivers/nvmem/qfprom.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This should not be between DTS patches. DTS goes different tree than
other driver changes. Please split your patchset and do not mix drivers
and DTS.

Best regards,
Krzysztof

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Krzysztof Kozlowski]

On 30/08/2023 11:58, Luca Weiss wrote:
> Like other Qualcomm PMICs the PM7250B can be used on different addresses
> on the SPMI bus. Use similar defines like the PMK8350 to make this
> possible.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> index e8540c36bd99..3514de536baa 100644
> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> @@ -7,6 +7,15 @@
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/spmi/spmi.h>
>  
> +/* This PMIC can be configured to be at different SIDs */
> +#ifndef PM7250B_SID
> +	#define PM7250B_SID 2
> +#endif

Why do you send the same patch as v1, without any reference to previous
discussions?

You got here feedback already.

https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/

Best regards,
Krzysztof

Re: [PATCH 06/11] dt-bindings: pinctrl: qcom,sc7280: Allow gpio-reserved-ranges

[Krzysztof Kozlowski]

On 30/08/2023 11:58, Luca Weiss wrote:
> Allow the gpio-reserved-ranges property on SC7280 TLMM.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---

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

Best regards,
Krzysztof

Re: [PATCH 01/11] arm64: dts: qcom: sc7280: Mark some nodes as 'reserved'

[Konrad Dybcio]

On 30.08.2023 11:58, Luca Weiss wrote:
> With the standard Qualcomm TrustZone setup, components such as lpasscc,
> pdc_reset and watchdog shouldn't be touched by Linux. Mark them with
> the status 'reserved' and reeable them in the chrome-common dtsi.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
Could probably use /* Owned by ADSP firmware */ or /* Owned by Gunyah hyp */

the change lgtm though

Konrad

Re: [PATCH 07/11] dt-bindings: arm: qcom,ids: Add SoC ID for QCM6490

[Krzysztof Kozlowski]

On 30/08/2023 11:58, Luca Weiss wrote:
> Add the ID for the Qualcomm QCM6490 SoC.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---

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

Best regards,
Krzysztof

Re: [PATCH 08/11] soc: qcom: socinfo: Add SoC ID for QCM6490

[Krzysztof Kozlowski]

On 30/08/2023 11:58, Luca Weiss wrote:
> Add SoC ID table entries for Qualcomm QCM6490.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---

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

Best regards,
Krzysztof

Re: [PATCH 03/11] arm64: dts: qcom: sc7280: Move qfprom clock to chrome-common

[Konrad Dybcio]

On 30.08.2023 11:58, Luca Weiss wrote:
> On non-ChromeOS boards the clock cannot be touched, so move it in the
> chrome-common dtsi which is the only place where it's needed.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
If that clock is not registered (e.g. it's in protected-clocks = <>,
would the _optional handler not handle it just fine?

Konrad

Re: [PATCH 09/11] cpufreq: Add QCM6490 to cpufreq-dt-platdev blocklist

[Krzysztof Kozlowski]

On 30/08/2023 11:58, Luca Weiss wrote:
> The Qualcomm QCM6490 platform uses the qcom-cpufreq-hw driver, so add it
> to the cpufreq-dt-platdev driver's blocklist.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>

This patchset is collecting unnecessarily way too many subsystems. It
creates some feeling of dependencies, which is not true and not correct.
Putting DTS in the middle makes it more confusing - if driver depends on
DTS, then it is NAK.

Please split your patchset - independent changes touching separate
subsystems should go to their own set.

Best regards,
Krzysztof

Re: [PATCH 10/11] dt-bindings: arm: qcom: Add QCM6490 Fairphone 5

[Krzysztof Kozlowski]

On 30/08/2023 11:58, Luca Weiss wrote:
> Fairphone 5 is a smartphone based on the QCM6490 SoC.
> 
> Also allow qcom,board-id and qcom,msm-id for QCM6490 since it's required
> by the bootloader.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  Documentation/devicetree/bindings/arm/qcom.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index adbfaea32343..b09a41812cf0 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -391,6 +391,11 @@ properties:
>            - const: qcom,qrb2210
>            - const: qcom,qcm2290
>  
> +      - items:
> +          - enum:
> +              - fairphone,fp5
> +          - const: qcom,qcm6490

You miss update to first list of SoCs for bootloader.

> +
>        - description: Qualcomm Technologies, Inc. Distributed Unit 1000 platform
>          items:
>            - enum:
> @@ -1074,6 +1079,7 @@ allOf:
>                - qcom,msm8994
>                - qcom,msm8996
>                - qcom,msm8998
> +              - qcom,qcm6490

This is not a legacy SoC (released in 2022). Not allowed.

Best regards,
Krzysztof

Re: [PATCH 07/11] dt-bindings: arm: qcom,ids: Add SoC ID for QCM6490

[Konrad Dybcio]

On 30.08.2023 11:58, Luca Weiss wrote:
> Add the ID for the Qualcomm QCM6490 SoC.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: [PATCH 11/11] arm64: dts: qcom: qcm6490: Add device-tree for Fairphone 5

[Krzysztof Kozlowski]

On 30/08/2023 11:58, Luca Weiss wrote:
> Add device tree for the Fairphone 5 smartphone which is based on
> the QCM6490 SoC.
> 

...

> +
> +#include <dt-bindings/arm/qcom,ids.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> +#include "sc7280.dtsi"
> +#include "pm7250b.dtsi"
> +#include "pm7325.dtsi"
> +#include "pm8350c.dtsi" /* PM7350C */
> +#include "pmk8350.dtsi" /* PMK7325 */
> +
> +/ {
> +	model = "Fairphone 5";
> +	compatible = "fairphone,fp5", "qcom,qcm6490";
> +	chassis-type = "handset";
> +
> +	/* required for bootloader to select correct board */
> +	qcom,msm-id = <QCOM_ID_QCM6490 0x10000>;
> +	qcom,board-id = <34 0>;

Sorry, not allowed. This is not an old, legacy platform.

> +
> +	aliases {
> +		serial0 = &uart5;
> +		serial1 = &uart7;
> +	};
> +
> +	chosen {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		framebuffer0: framebuffer@a000000 {
> +			compatible = "simple-framebuffer";
> +			reg = <0 0xe1000000 0 (2700 * 1224 * 4)>;
> +			width = <1224>;
> +			height = <2700>;
> +			stride = <(1224 * 4)>;
> +			format = "a8r8g8b8";
> +			panel = <&panel>;
> +			clocks = <&gcc GCC_DISP_HF_AXI_CLK>;
> +		};
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		pinctrl-0 = <&volume_down_default>, <&hall_sensor_default>;
> +		pinctrl-names = "default";
> +
> +		key-volume-up {
> +			label = "Volume up";
> +			gpios = <&pm7325_gpios 6 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_VOLUMEUP>;
> +		};
> +
> +		/* Hall sensor uses vreg_l8c as VDD, it's being enabled using

Use Linux style comments:
/*
 * foo
 * bar...


> +		 * always-on on the regulator
> +		 */
> +		event-hall-sensor {
> +			label = "Hall Effect Sensor";
> +			gpios = <&tlmm 155 GPIO_ACTIVE_LOW>;
> +			linux,input-type = <EV_SW>;
> +			linux,code = <SW_LID>;
> +			linux,can-disable;
> +			wakeup-source;
> +		};
> +	};
> +
> +	panel: panel {
> +		compatible = "boe,rm692e5";

Undocumented. You cannot add nodes with undocumented compatibles. Drop.

> +
> +		width-mm = <68>;
> +		height-mm = <150>;
> +	};


Best regards,
Krzysztof

Re: [PATCH 01/11] arm64: dts: qcom: sc7280: Mark some nodes as 'reserved'

[Luca Weiss]

On Wed Aug 30, 2023 at 12:08 PM CEST, Konrad Dybcio wrote:
> On 30.08.2023 11:58, Luca Weiss wrote:
> > With the standard Qualcomm TrustZone setup, components such as lpasscc,
> > pdc_reset and watchdog shouldn't be touched by Linux. Mark them with
> > the status 'reserved' and reeable them in the chrome-common dtsi.
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> Could probably use /* Owned by ADSP firmware */ or /* Owned by Gunyah hyp */

Do you know which one is more fitting for these nodes? I don't really
have a reference to if the ADSP or Gunyah (is this even used here?) owns
this.

Regards
Luca

>
> the change lgtm though
>
> Konrad

Re: [PATCH 11/11] arm64: dts: qcom: qcm6490: Add device-tree for Fairphone 5

[Konrad Dybcio]

On 30.08.2023 11:58, Luca Weiss wrote:
> Add device tree for the Fairphone 5 smartphone which is based on
> the QCM6490 SoC.
> 
> Supported features are, as of now:
> * Bluetooth
> * Debug UART
> * Display via simplefb
> * Flash/torch LED
> * Flip cover sensor
> * Power & volume buttons
> * RTC
> * SD card
> * USB
> * Various plumbing like regulators, i2c, spi, etc
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  arch/arm64/boot/dts/qcom/Makefile                  |   1 +
>  arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 659 +++++++++++++++++++++
>  2 files changed, 660 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 2cca20563a1d..73c3be0f8872 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -81,6 +81,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-sony-xperia-yoshino-lilac.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-sony-xperia-yoshino-maple.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-sony-xperia-yoshino-poplar.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-xiaomi-sagit.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= qcm6490-fairphone-fp5.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-1000.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-4000.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> new file mode 100644
> index 000000000000..572b254d3af2
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> @@ -0,0 +1,659 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2023, Luca Weiss <luca.weiss@fairphone.com>
> + */
> +
> +/dts-v1/;
> +
> +/* PM7250B is configured to use SID8/9 instead of SID2/3 */
> +#define PM7250B_SID 8
> +#define PM7250B_SID1 9
> +
> +#include <dt-bindings/arm/qcom,ids.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> +#include "sc7280.dtsi"
> +#include "pm7250b.dtsi"
> +#include "pm7325.dtsi"
> +#include "pm8350c.dtsi" /* PM7350C */
> +#include "pmk8350.dtsi" /* PMK7325 */
> +
> +/ {
> +	model = "Fairphone 5";
> +	compatible = "fairphone,fp5", "qcom,qcm6490";
> +	chassis-type = "handset";
> +
> +	/* required for bootloader to select correct board */
> +	qcom,msm-id = <QCOM_ID_QCM6490 0x10000>;
> +	qcom,board-id = <34 0>;
Try [1], it worked for me on a picky device.

> +
> +	aliases {
> +		serial0 = &uart5;
> +		serial1 = &uart7;
> +	};
> +
> +	chosen {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		framebuffer0: framebuffer@a000000 {
> +			compatible = "simple-framebuffer";
> +			reg = <0 0xe1000000 0 (2700 * 1224 * 4)>;
0x0?

> +			width = <1224>;
> +			height = <2700>;
> +			stride = <(1224 * 4)>;
> +			format = "a8r8g8b8";
> +			panel = <&panel>;
> +			clocks = <&gcc GCC_DISP_HF_AXI_CLK>;
> +		};
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		pinctrl-0 = <&volume_down_default>, <&hall_sensor_default>;
> +		pinctrl-names = "default";
> +
> +		key-volume-up {
> +			label = "Volume up";
> +			gpios = <&pm7325_gpios 6 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_VOLUMEUP>;
> +		};
> +
> +		/* Hall sensor uses vreg_l8c as VDD, it's being enabled using
> +		 * always-on on the regulator
> +		 */
/* Powered by the always-on vreg_l8c */ - fits in a single line

> +		event-hall-sensor {
> +			label = "Hall Effect Sensor";
> +			gpios = <&tlmm 155 GPIO_ACTIVE_LOW>;
> +			linux,input-type = <EV_SW>;
> +			linux,code = <SW_LID>;
> +			linux,can-disable;
> +			wakeup-source;
> +		};
> +	};
> +
> +	panel: panel {
> +		compatible = "boe,rm692e5";
Undocumented compatbile

> +
> +		width-mm = <68>;
> +		height-mm = <150>;
> +	};
> +
> +	reserved-memory {
> +		cont_splash_mem: cont-splash@e1000000 {
> +			reg = <0 0xe1000000 0 0x2300000>;
0x0

[...]

> +		vreg_s1b: smps1 {
> +			regulator-min-microvolt = <1840000>;
> +			regulator-max-microvolt = <2040000>;
No regulator-initial-mode on this pmic?

[...]

> +
> +	/* PM8008 PMIC @ 8 and 9 */
> +	/* Pixelworks @ 26 */
> +	/* FSA4480 USB audio switch @ 42 */
There's a driver for the fsa4480, you may wanna include a node here

> +	/* AW86927FCR haptics @ 5a */
> +};
[...]

> +
> +	led-0 {
> +		function = LED_FUNCTION_FLASH;
No dual-tone stuff?

> +		color = <LED_COLOR_ID_WHITE>;
> +		led-sources = <1>, <4>;
> +		led-max-microamp = <500000>;
> +		flash-max-microamp = <1500000>;
> +		flash-max-timeout-us = <1280000>;
> +	};
> +};
> +
> +&pmk8350_rtc {
> +	status = "okay";
> +};
> +
> +&pon_pwrkey {
> +	status = "okay";
> +};
> +
> +&pon_resin {
> +	status = "okay";
> +	linux,code = <KEY_VOLUMEDOWN>;
status last

[...]

> +&rmtfs_mem {
> +	qcom,vmid = <QCOM_SCM_VMID_MSS_MSA>, <QCOM_SCM_VMID_NAV>;
> +	reg = <0x0 0xf8500000 0x0 0x600000>;
> +};
/delete-node/ + redefinition would probably be cleaner-looking

[...]

> +&uart7 {
> +	/delete-property/interrupts;
> +	interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>,
> +			      <&tlmm 31 IRQ_TYPE_EDGE_FALLING>;
> +
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>;
property-n
property-names

> +
> +	status = "okay";
> +
> +	bluetooth: bluetooth {
> +		compatible = "qcom,wcn6750-bt";
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&bluetooth_enable_default>, <&sw_ctrl_default>;
property-n
property-names

Konrad

[1] https://lore.kernel.org/linux-arm-msm/9db02015-2c41-40d6-bf35-69ef277e9ce4@linaro.org/

Re: [PATCH 01/11] arm64: dts: qcom: sc7280: Mark some nodes as 'reserved'

[Konrad Dybcio]

On 30.08.2023 12:35, Luca Weiss wrote:
> On Wed Aug 30, 2023 at 12:08 PM CEST, Konrad Dybcio wrote:
>> On 30.08.2023 11:58, Luca Weiss wrote:
>>> With the standard Qualcomm TrustZone setup, components such as lpasscc,
>>> pdc_reset and watchdog shouldn't be touched by Linux. Mark them with
>>> the status 'reserved' and reeable them in the chrome-common dtsi.
>>>
>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>> ---
>> Could probably use /* Owned by ADSP firmware */ or /* Owned by Gunyah hyp */
> 
> Do you know which one is more fitting for these nodes? I don't really
> have a reference to if the ADSP or Gunyah (is this even used here?) owns
> this.
ADSP owns the audio hw, Gunyah owns the wdog

Konrad

Re: [PATCH 06/11] dt-bindings: pinctrl: qcom,sc7280: Allow gpio-reserved-ranges

[Linus Walleij]

On Wed, Aug 30, 2023 at 11:58 AM Luca Weiss <luca.weiss@fairphone.com> wrote:

> Allow the gpio-reserved-ranges property on SC7280 TLMM.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

I assume this will be merged with the rest of the patches, poke me
after v6.6-rc1 if you want me to apply it to the pinctrl tree.

Yours,
Linus Walleij

Re: [PATCH 02/11] nvmem: qfprom: Mark core clk as optional

[Doug Anderson]

Hi,

On Wed, Aug 30, 2023 at 2:58 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
>
> On some platforms like sc7280 on non-ChromeOS devices the core clock
> cannot be touched by Linux so we cannot provide it. Mark it as optional
> as accessing qfprom works without it.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  drivers/nvmem/qfprom.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Are you actually testing burning fuses from the OS, or are you just
using the nvmem in "read-only" mode? From comments in the bindings, if
you're trying to burn the fuses then the clock is required. If things
are in read-only mode then the clock isn't required.

When I compare to the driver, it seems like the driver assumes that if
more than one memory region is provided then you must be supporting
burning fuses. The bindings agree that having 4 memory regions
specified means that the nvmem supports burning and 1 memory region
specified means read-only. The extra 3 memory regions in the nvmem are
all about fuse burning, I believe.

So maybe the right fix here is to just change your dts to specify one
memory region?

-Doug

Re: [PATCH 02/11] nvmem: qfprom: Mark core clk as optional

[Luca Weiss]

On Wed Aug 30, 2023 at 4:30 PM CEST, Doug Anderson wrote:
> Hi,
>
> On Wed, Aug 30, 2023 at 2:58 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> >
> > On some platforms like sc7280 on non-ChromeOS devices the core clock
> > cannot be touched by Linux so we cannot provide it. Mark it as optional
> > as accessing qfprom works without it.
> >
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> >  drivers/nvmem/qfprom.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Are you actually testing burning fuses from the OS, or are you just
> using the nvmem in "read-only" mode? From comments in the bindings, if
> you're trying to burn the fuses then the clock is required. If things
> are in read-only mode then the clock isn't required.

Hi Doug,

I definitely don't plan on burning any fuses on this phone. Not even
sure that's allowed by the TZ / boot stack.

>
> When I compare to the driver, it seems like the driver assumes that if
> more than one memory region is provided then you must be supporting
> burning fuses. The bindings agree that having 4 memory regions
> specified means that the nvmem supports burning and 1 memory region
> specified means read-only. The extra 3 memory regions in the nvmem are
> all about fuse burning, I believe.
>
> So maybe the right fix here is to just change your dts to specify one
> memory region?

I got feedback from Konrad that this here would be the preferred
approach compared to having a different dts for ChromeOS vs non-ChromeOS
devices. I don't feel strongly to either, for me it's also okay to
remove the extra memory regions and only have the main one used on
regular qcom devices.

Let me know what you think.

Regards
Luca

>
> -Doug

Re: [PATCH 02/11] nvmem: qfprom: Mark core clk as optional

[Doug Anderson]

Hi,

On Wed, Aug 30, 2023 at 7:43 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
>
> On Wed Aug 30, 2023 at 4:30 PM CEST, Doug Anderson wrote:
> > Hi,
> >
> > On Wed, Aug 30, 2023 at 2:58 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> > >
> > > On some platforms like sc7280 on non-ChromeOS devices the core clock
> > > cannot be touched by Linux so we cannot provide it. Mark it as optional
> > > as accessing qfprom works without it.
> > >
> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > ---
> > >  drivers/nvmem/qfprom.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Are you actually testing burning fuses from the OS, or are you just
> > using the nvmem in "read-only" mode? From comments in the bindings, if
> > you're trying to burn the fuses then the clock is required. If things
> > are in read-only mode then the clock isn't required.
>
> Hi Doug,
>
> I definitely don't plan on burning any fuses on this phone. Not even
> sure that's allowed by the TZ / boot stack.
>
> >
> > When I compare to the driver, it seems like the driver assumes that if
> > more than one memory region is provided then you must be supporting
> > burning fuses. The bindings agree that having 4 memory regions
> > specified means that the nvmem supports burning and 1 memory region
> > specified means read-only. The extra 3 memory regions in the nvmem are
> > all about fuse burning, I believe.
> >
> > So maybe the right fix here is to just change your dts to specify one
> > memory region?
>
> I got feedback from Konrad that this here would be the preferred
> approach compared to having a different dts for ChromeOS vs non-ChromeOS
> devices. I don't feel strongly to either, for me it's also okay to
> remove the extra memory regions and only have the main one used on
> regular qcom devices.
>
> Let me know what you think.

I don't hate the idea of leaving the extra memory regions in the dts.
They do describe the hardware, after all, even if the main OS can't
actually access those memory regions. ...though the same could also be
said about the clock you've removed. Said another way: if you want to
fully describe the hardware then the dts should have the extra memory
regions and the clock. If you are OK w/ just describing the hardware
in the way that the OS has access to then the dts should not have the
extra memory regions and not have the clock. Does that sound right?

If somehow you do end up with something like your patch, though,
you're still missing a bit. Specifically, you don't want to "enable
writing" a few lines below if you didn't get the clock, right?

-Doug

Re: [PATCH 09/11] cpufreq: Add QCM6490 to cpufreq-dt-platdev blocklist

[Viresh Kumar]

On 30-08-23, 11:58, Luca Weiss wrote:
> The Qualcomm QCM6490 platform uses the qcom-cpufreq-hw driver, so add it
> to the cpufreq-dt-platdev driver's blocklist.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> index fb2875ce1fdd..02ec58a8603b 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -145,6 +145,7 @@ static const struct of_device_id blocklist[] __initconst = {
>  	{ .compatible = "qcom,msm8996", },
>  	{ .compatible = "qcom,msm8998", },
>  	{ .compatible = "qcom,qcm2290", },
> +	{ .compatible = "qcom,qcm6490", },
>  	{ .compatible = "qcom,qcs404", },
>  	{ .compatible = "qcom,qdu1000", },
>  	{ .compatible = "qcom,sa8155p" },

Applied. Thanks.

-- 
viresh

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Luca Weiss]

On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> On 30/08/2023 11:58, Luca Weiss wrote:
> > Like other Qualcomm PMICs the PM7250B can be used on different addresses
> > on the SPMI bus. Use similar defines like the PMK8350 to make this
> > possible.
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> >  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> >  1 file changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > index e8540c36bd99..3514de536baa 100644
> > --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > @@ -7,6 +7,15 @@
> >  #include <dt-bindings/interrupt-controller/irq.h>
> >  #include <dt-bindings/spmi/spmi.h>
> >  
> > +/* This PMIC can be configured to be at different SIDs */
> > +#ifndef PM7250B_SID
> > +	#define PM7250B_SID 2
> > +#endif
>
> Why do you send the same patch as v1, without any reference to previous
> discussions?
>
> You got here feedback already.
>
> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/

Hi Krzysztof,

I did mention that original patch in the cover letter of this series.
I'm definitely aware of the discussion earlier this year there but also
tried to get an update lately if there's any update with no response.

If you want to block this patch, I'll have to remove pm7250b from the
device dts, so we'll lose some functionality. Not sure what other
approaches there could be.

Regards
Luca

>
> Best regards,
> Krzysztof

Re: [PATCH 03/11] arm64: dts: qcom: sc7280: Move qfprom clock to chrome-common

[Luca Weiss]

On Wed Aug 30, 2023 at 12:09 PM CEST, Konrad Dybcio wrote:
> On 30.08.2023 11:58, Luca Weiss wrote:
> > On non-ChromeOS boards the clock cannot be touched, so move it in the
> > chrome-common dtsi which is the only place where it's needed.
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> If that clock is not registered (e.g. it's in protected-clocks = <>,
> would the _optional handler not handle it just fine?

Right, that appears to work!

~ # ls -d /sys/bus/platform/drivers/qcom,qfprom/*.efuse
/sys/bus/platform/drivers/qcom,qfprom/784000.efuse
~ # cat /sys/firmware/devicetree/base/soc@0/efuse@784000/clock-names; echo
core
~ # hexdump -C /sys/firmware/devicetree/base/soc@0/efuse@784000/clocks
00000000  00 00 00 03 00 00 00 b8                           |........|
00000008

Never tested this case before, but since it appears to work with the
patched qfprom driver (other patch in this series) I think we can drop
this patch.
Will also have to adjust some other patches in my local tree then that
do similar things ;)

Regards
Luca

>
> Konrad

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Dmitry Baryshkov]

On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
>
> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> > On 30/08/2023 11:58, Luca Weiss wrote:
> > > Like other Qualcomm PMICs the PM7250B can be used on different addresses
> > > on the SPMI bus. Use similar defines like the PMK8350 to make this
> > > possible.
> > >
> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > ---
> > >  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> > >  1 file changed, 16 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > index e8540c36bd99..3514de536baa 100644
> > > --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > @@ -7,6 +7,15 @@
> > >  #include <dt-bindings/interrupt-controller/irq.h>
> > >  #include <dt-bindings/spmi/spmi.h>
> > >
> > > +/* This PMIC can be configured to be at different SIDs */
> > > +#ifndef PM7250B_SID
> > > +   #define PM7250B_SID 2
> > > +#endif
> >
> > Why do you send the same patch as v1, without any reference to previous
> > discussions?
> >
> > You got here feedback already.
> >
> > https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
>
> Hi Krzysztof,
>
> I did mention that original patch in the cover letter of this series.
> I'm definitely aware of the discussion earlier this year there but also
> tried to get an update lately if there's any update with no response.

I think the overall consensus was that my proposal is too complicated
for the DT files.

>
> If you want to block this patch, I'll have to remove pm7250b from the
> device dts, so we'll lose some functionality. Not sure what other
> approaches there could be.
>
> Regards
> Luca
>
> >
> > Best regards,
> > Krzysztof
>


-- 
With best wishes
Dmitry

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Krzysztof Kozlowski]

On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
>>
>> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
>>> On 30/08/2023 11:58, Luca Weiss wrote:
>>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
>>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
>>>> possible.
>>>>
>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>> ---
>>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
>>>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> index e8540c36bd99..3514de536baa 100644
>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> @@ -7,6 +7,15 @@
>>>>  #include <dt-bindings/interrupt-controller/irq.h>
>>>>  #include <dt-bindings/spmi/spmi.h>
>>>>
>>>> +/* This PMIC can be configured to be at different SIDs */
>>>> +#ifndef PM7250B_SID
>>>> +   #define PM7250B_SID 2
>>>> +#endif
>>>
>>> Why do you send the same patch as v1, without any reference to previous
>>> discussions?
>>>
>>> You got here feedback already.
>>>
>>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
>>
>> Hi Krzysztof,
>>
>> I did mention that original patch in the cover letter of this series.
>> I'm definitely aware of the discussion earlier this year there but also
>> tried to get an update lately if there's any update with no response.
> 
> I think the overall consensus was that my proposal is too complicated
> for the DT files.

I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
customize per address? No.

I definitely do not agree to these ifndef->define. Maybe using just
define would work (so drop ifndef->define), because this makes it
obvious and fail-safe if included in wrong place... except that it is
still not the define we expect. This is not the coding style present in
other DTSes.

The true problem how these SPMI bindings were created. Requiring SID
address in every child is clearly redundant and I think we do not follow
such approach anywhere else.

Best regards,
Krzysztof

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Dmitry Baryshkov]

On Thu, 31 Aug 2023 at 14:54, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> > On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
> >>
> >> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> >>> On 30/08/2023 11:58, Luca Weiss wrote:
> >>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
> >>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
> >>>> possible.
> >>>>
> >>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> >>>> ---
> >>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> >>>>  1 file changed, 16 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> index e8540c36bd99..3514de536baa 100644
> >>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> @@ -7,6 +7,15 @@
> >>>>  #include <dt-bindings/interrupt-controller/irq.h>
> >>>>  #include <dt-bindings/spmi/spmi.h>
> >>>>
> >>>> +/* This PMIC can be configured to be at different SIDs */
> >>>> +#ifndef PM7250B_SID
> >>>> +   #define PM7250B_SID 2
> >>>> +#endif
> >>>
> >>> Why do you send the same patch as v1, without any reference to previous
> >>> discussions?
> >>>
> >>> You got here feedback already.
> >>>
> >>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
> >>
> >> Hi Krzysztof,
> >>
> >> I did mention that original patch in the cover letter of this series.
> >> I'm definitely aware of the discussion earlier this year there but also
> >> tried to get an update lately if there's any update with no response.
> >
> > I think the overall consensus was that my proposal is too complicated
> > for the DT files.
>
> I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
> customize per address? No.

At the same time, we do keep SoC files separate from the board files.
Yes, I'm slightly exaggerating here.

I think that for PMIC files it makes sense to extract common parts if
that eases reuse of the common parts.

>
> I definitely do not agree to these ifndef->define. Maybe using just
> define would work (so drop ifndef->define), because this makes it
> obvious and fail-safe if included in wrong place... except that it is
> still not the define we expect. This is not the coding style present in
> other DTSes.
>
> The true problem how these SPMI bindings were created. Requiring SID
> address in every child is clearly redundant and I think we do not follow
> such approach anywhere else.
>
> Best regards,
> Krzysztof
>


-- 
With best wishes
Dmitry

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Luca Weiss]

On Thu Aug 31, 2023 at 1:54 PM CEST, Krzysztof Kozlowski wrote:
> On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> > On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
> >>
> >> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> >>> On 30/08/2023 11:58, Luca Weiss wrote:
> >>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
> >>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
> >>>> possible.
> >>>>
> >>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> >>>> ---
> >>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> >>>>  1 file changed, 16 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> index e8540c36bd99..3514de536baa 100644
> >>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> @@ -7,6 +7,15 @@
> >>>>  #include <dt-bindings/interrupt-controller/irq.h>
> >>>>  #include <dt-bindings/spmi/spmi.h>
> >>>>
> >>>> +/* This PMIC can be configured to be at different SIDs */
> >>>> +#ifndef PM7250B_SID
> >>>> +   #define PM7250B_SID 2
> >>>> +#endif
> >>>
> >>> Why do you send the same patch as v1, without any reference to previous
> >>> discussions?
> >>>
> >>> You got here feedback already.
> >>>
> >>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
> >>
> >> Hi Krzysztof,
> >>
> >> I did mention that original patch in the cover letter of this series.
> >> I'm definitely aware of the discussion earlier this year there but also
> >> tried to get an update lately if there's any update with no response.
> > 
> > I think the overall consensus was that my proposal is too complicated
> > for the DT files.
>
> I proposed to duplicate the entries.

If you mean creating a pm7250b-8.dtsi with pm7250b copy-pasted but the
SID changed from 2 & 3 to 8 & 9, I can do that if that's the way
forward.

If this was done, I'd also say then that pm7250b.dtsi should be renamed
to e.g. pm7250b-2.dtsi since it's currently sitting on SID 2 & 3.

> Do you keep QUP nodes in DTSI and customize per address? No.
>
> I definitely do not agree to these ifndef->define. Maybe using just
> define would work (so drop ifndef->define), because this makes it
> obvious and fail-safe if included in wrong place... except that it is
> still not the define we expect. This is not the coding style present in
> other DTSes.

I really don't mind either way, I'd just like to have some way for now.

>
> The true problem how these SPMI bindings were created. Requiring SID
> address in every child is clearly redundant and I think we do not follow
> such approach anywhere else.

Is this something that could be fixed long term? Especially since
Qualcomm is reconfiguring PMICs on different addresses nowadays maybe
there's more or a push to do this?

Regards
Luca

>
> Best regards,
> Krzysztof

Re: [PATCH 11/11] arm64: dts: qcom: qcm6490: Add device-tree for Fairphone 5

[Luca Weiss]

On Wed Aug 30, 2023 at 12:45 PM CEST, Konrad Dybcio wrote:
> On 30.08.2023 11:58, Luca Weiss wrote:
> > Add device tree for the Fairphone 5 smartphone which is based on
> > the QCM6490 SoC.
> > 
> > Supported features are, as of now:
> > * Bluetooth
> > * Debug UART
> > * Display via simplefb
> > * Flash/torch LED
> > * Flip cover sensor
> > * Power & volume buttons
> > * RTC
> > * SD card
> > * USB
> > * Various plumbing like regulators, i2c, spi, etc
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> >  arch/arm64/boot/dts/qcom/Makefile                  |   1 +
> >  arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 659 +++++++++++++++++++++
> >  2 files changed, 660 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> > index 2cca20563a1d..73c3be0f8872 100644
> > --- a/arch/arm64/boot/dts/qcom/Makefile
> > +++ b/arch/arm64/boot/dts/qcom/Makefile
> > @@ -81,6 +81,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-sony-xperia-yoshino-lilac.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-sony-xperia-yoshino-maple.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-sony-xperia-yoshino-poplar.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-xiaomi-sagit.dtb
> > +dtb-$(CONFIG_ARCH_QCOM)	+= qcm6490-fairphone-fp5.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-1000.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-4000.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
> > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > new file mode 100644
> > index 000000000000..572b254d3af2
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > @@ -0,0 +1,659 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) 2023, Luca Weiss <luca.weiss@fairphone.com>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +/* PM7250B is configured to use SID8/9 instead of SID2/3 */
> > +#define PM7250B_SID 8
> > +#define PM7250B_SID1 9
> > +
> > +#include <dt-bindings/arm/qcom,ids.h>
> > +#include <dt-bindings/leds/common.h>
> > +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> > +#include "sc7280.dtsi"
> > +#include "pm7250b.dtsi"
> > +#include "pm7325.dtsi"
> > +#include "pm8350c.dtsi" /* PM7350C */
> > +#include "pmk8350.dtsi" /* PMK7325 */
> > +
> > +/ {
> > +	model = "Fairphone 5";
> > +	compatible = "fairphone,fp5", "qcom,qcm6490";
> > +	chassis-type = "handset";
> > +
> > +	/* required for bootloader to select correct board */
> > +	qcom,msm-id = <QCOM_ID_QCM6490 0x10000>;
> > +	qcom,board-id = <34 0>;
> Try [1], it worked for me on a picky device.

Yes, that works! Thanks!

>
> > +
> > +	aliases {
> > +		serial0 = &uart5;
> > +		serial1 = &uart7;
> > +	};
> > +
> > +	chosen {
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		ranges;
> > +
> > +		framebuffer0: framebuffer@a000000 {
> > +			compatible = "simple-framebuffer";
> > +			reg = <0 0xe1000000 0 (2700 * 1224 * 4)>;
> 0x0?

Ack

>
> > +			width = <1224>;
> > +			height = <2700>;
> > +			stride = <(1224 * 4)>;
> > +			format = "a8r8g8b8";
> > +			panel = <&panel>;
> > +			clocks = <&gcc GCC_DISP_HF_AXI_CLK>;
> > +		};
> > +	};
> > +
> > +	gpio-keys {
> > +		compatible = "gpio-keys";
> > +
> > +		pinctrl-0 = <&volume_down_default>, <&hall_sensor_default>;
> > +		pinctrl-names = "default";
> > +
> > +		key-volume-up {
> > +			label = "Volume up";
> > +			gpios = <&pm7325_gpios 6 GPIO_ACTIVE_LOW>;
> > +			linux,code = <KEY_VOLUMEUP>;
> > +		};
> > +
> > +		/* Hall sensor uses vreg_l8c as VDD, it's being enabled using
> > +		 * always-on on the regulator
> > +		 */
> /* Powered by the always-on vreg_l8c */ - fits in a single line

Ack

>
> > +		event-hall-sensor {
> > +			label = "Hall Effect Sensor";
> > +			gpios = <&tlmm 155 GPIO_ACTIVE_LOW>;
> > +			linux,input-type = <EV_SW>;
> > +			linux,code = <SW_LID>;
> > +			linux,can-disable;
> > +			wakeup-source;
> > +		};
> > +	};
> > +
> > +	panel: panel {
> > +		compatible = "boe,rm692e5";
> Undocumented compatbile

Not quite sure how to document this properly. This dummy panel node is
used for the simplefb/simpledrm so that user space can get panel
dimensions already.

This approach of a separate panel node was apparently preferred when the
functionality was upstreamed but of course as can be seen this panel
node is very barebones where many properties are not present which would
be needed with the full panel driver (once it's working).

Also for example for the bindings I think width-mm and height-mm won't
be needed anymore since that information will be provided by the driver
then.

Please let me know how/where to document this.

>
> > +
> > +		width-mm = <68>;
> > +		height-mm = <150>;
> > +	};
> > +
> > +	reserved-memory {
> > +		cont_splash_mem: cont-splash@e1000000 {
> > +			reg = <0 0xe1000000 0 0x2300000>;
> 0x0

Ack

>
> [...]
>
> > +		vreg_s1b: smps1 {
> > +			regulator-min-microvolt = <1840000>;
> > +			regulator-max-microvolt = <2040000>;
> No regulator-initial-mode on this pmic?

This specific regulator doesn't have one downstream, but it seems other
pm7325 regulators do have this downstream so I'll add it in v2.

>
> [...]
>
> > +
> > +	/* PM8008 PMIC @ 8 and 9 */
> > +	/* Pixelworks @ 26 */
> > +	/* FSA4480 USB audio switch @ 42 */
> There's a driver for the fsa4480, you may wanna include a node here

I'm aware, but will work on this later since it's not useful without
USB-C role switching and working audio.

>
> > +	/* AW86927FCR haptics @ 5a */
> > +};
> [...]
>
> > +
> > +	led-0 {
> > +		function = LED_FUNCTION_FLASH;
> No dual-tone stuff?

Nope.

>
> > +		color = <LED_COLOR_ID_WHITE>;
> > +		led-sources = <1>, <4>;
> > +		led-max-microamp = <500000>;
> > +		flash-max-microamp = <1500000>;
> > +		flash-max-timeout-us = <1280000>;
> > +	};
> > +};
> > +
> > +&pmk8350_rtc {
> > +	status = "okay";
> > +};
> > +
> > +&pon_pwrkey {
> > +	status = "okay";
> > +};
> > +
> > +&pon_resin {
> > +	status = "okay";
> > +	linux,code = <KEY_VOLUMEDOWN>;
> status last

Ack

>
> [...]
>
> > +&rmtfs_mem {
> > +	qcom,vmid = <QCOM_SCM_VMID_MSS_MSA>, <QCOM_SCM_VMID_NAV>;
> > +	reg = <0x0 0xf8500000 0x0 0x600000>;
> > +};
> /delete-node/ + redefinition would probably be cleaner-looking

Ack, looks better.

>
> [...]
>
> > +&uart7 {
> > +	/delete-property/interrupts;
> > +	interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>,
> > +			      <&tlmm 31 IRQ_TYPE_EDGE_FALLING>;
> > +
> > +	pinctrl-names = "default", "sleep";
> > +	pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>;
> property-n
> property-names

Ack

>
> > +
> > +	status = "okay";
> > +
> > +	bluetooth: bluetooth {
> > +		compatible = "qcom,wcn6750-bt";
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&bluetooth_enable_default>, <&sw_ctrl_default>;
> property-n
> property-names

Ack

Regards
Luca

>
> Konrad
>
> [1] https://lore.kernel.org/linux-arm-msm/9db02015-2c41-40d6-bf35-69ef277e9ce4@linaro.org/

Re: [PATCH 02/11] nvmem: qfprom: Mark core clk as optional

[Luca Weiss]

On Wed Aug 30, 2023 at 4:57 PM CEST, Doug Anderson wrote:
> Hi,
>
> On Wed, Aug 30, 2023 at 7:43 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> >
> > On Wed Aug 30, 2023 at 4:30 PM CEST, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Wed, Aug 30, 2023 at 2:58 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> > > >
> > > > On some platforms like sc7280 on non-ChromeOS devices the core clock
> > > > cannot be touched by Linux so we cannot provide it. Mark it as optional
> > > > as accessing qfprom works without it.
> > > >
> > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > > ---
> > > >  drivers/nvmem/qfprom.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > Are you actually testing burning fuses from the OS, or are you just
> > > using the nvmem in "read-only" mode? From comments in the bindings, if
> > > you're trying to burn the fuses then the clock is required. If things
> > > are in read-only mode then the clock isn't required.
> >
> > Hi Doug,
> >
> > I definitely don't plan on burning any fuses on this phone. Not even
> > sure that's allowed by the TZ / boot stack.
> >
> > >
> > > When I compare to the driver, it seems like the driver assumes that if
> > > more than one memory region is provided then you must be supporting
> > > burning fuses. The bindings agree that having 4 memory regions
> > > specified means that the nvmem supports burning and 1 memory region
> > > specified means read-only. The extra 3 memory regions in the nvmem are
> > > all about fuse burning, I believe.
> > >
> > > So maybe the right fix here is to just change your dts to specify one
> > > memory region?
> >
> > I got feedback from Konrad that this here would be the preferred
> > approach compared to having a different dts for ChromeOS vs non-ChromeOS
> > devices. I don't feel strongly to either, for me it's also okay to
> > remove the extra memory regions and only have the main one used on
> > regular qcom devices.
> >
> > Let me know what you think.
>
> I don't hate the idea of leaving the extra memory regions in the dts.
> They do describe the hardware, after all, even if the main OS can't
> actually access those memory regions. ...though the same could also be
> said about the clock you've removed. Said another way: if you want to
> fully describe the hardware then the dts should have the extra memory
> regions and the clock. If you are OK w/ just describing the hardware
> in the way that the OS has access to then the dts should not have the
> extra memory regions and not have the clock. Does that sound right?

Not sure which of those memory regions are actually accessible on this
board, but honestly I don't even want to try accessing it. Blowing fuses
is not my wish there ;)

On downstream the node is just described like the following:

	qfprom: qfprom@780000 {
		compatible = "qcom,qfprom";
		reg = <0x780000 0x7000>;
		...
	};

So we have 0x780000 - 0x786fff here.

In sc7280.dtsi we have the following:

	qfprom: efuse@784000 {
		compatible = "qcom,sc7280-qfprom", "qcom,qfprom";
		reg = <0 0x00784000 0 0xa20>,
			  <0 0x00780000 0 0xa20>,
			  <0 0x00782000 0 0x120>,
			  <0 0x00786000 0 0x1fff>;
		...
	};

So I guess this:
* 0x780000 - 0x780a1f
* 0x782000 - 0x78211f
* 0x784000 - 0x784a1f
* 0x786000 - 0x787ffe

So at least the last memory region seems to be partially out of range
according to downstream.

So after reading all of this I tried running this commmand on the phone
and the phone reboots into 900e mode.

  $ cat /sys/devices/platform/soc@0/784000.efuse/qfprom0/nvmem

I guess normally this should work? So if I interpret this correctly, the
Linux driver thinks it can access more than it can/should. But also
should probably try this command on another chipset to see if it works
on any really?

Regards
Luca

>
> If somehow you do end up with something like your patch, though,
> you're still missing a bit. Specifically, you don't want to "enable
> writing" a few lines below if you didn't get the clock, right?
>
> -Doug

Re: [PATCH 02/11] nvmem: qfprom: Mark core clk as optional

[Doug Anderson]

Hi,

On Fri, Sep 1, 2023 at 7:54 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
>
> > > > So maybe the right fix here is to just change your dts to specify one
> > > > memory region?
> > >
> > > I got feedback from Konrad that this here would be the preferred
> > > approach compared to having a different dts for ChromeOS vs non-ChromeOS
> > > devices. I don't feel strongly to either, for me it's also okay to
> > > remove the extra memory regions and only have the main one used on
> > > regular qcom devices.
> > >
> > > Let me know what you think.
> >
> > I don't hate the idea of leaving the extra memory regions in the dts.
> > They do describe the hardware, after all, even if the main OS can't
> > actually access those memory regions. ...though the same could also be
> > said about the clock you've removed. Said another way: if you want to
> > fully describe the hardware then the dts should have the extra memory
> > regions and the clock. If you are OK w/ just describing the hardware
> > in the way that the OS has access to then the dts should not have the
> > extra memory regions and not have the clock. Does that sound right?
>
> Not sure which of those memory regions are actually accessible on this
> board, but honestly I don't even want to try accessing it. Blowing fuses
> is not my wish there ;)
>
> On downstream the node is just described like the following:
>
>         qfprom: qfprom@780000 {
>                 compatible = "qcom,qfprom";
>                 reg = <0x780000 0x7000>;
>                 ...
>         };
>
> So we have 0x780000 - 0x786fff here.
>
> In sc7280.dtsi we have the following:
>
>         qfprom: efuse@784000 {
>                 compatible = "qcom,sc7280-qfprom", "qcom,qfprom";
>                 reg = <0 0x00784000 0 0xa20>,
>                           <0 0x00780000 0 0xa20>,
>                           <0 0x00782000 0 0x120>,
>                           <0 0x00786000 0 0x1fff>;
>                 ...
>         };
>
> So I guess this:
> * 0x780000 - 0x780a1f
> * 0x782000 - 0x78211f
> * 0x784000 - 0x784a1f
> * 0x786000 - 0x787ffe
>
> So at least the last memory region seems to be partially out of range
> according to downstream.

From the other discussion, it sounds as if you _can_ leave the clock
in the device tree and then use "clk_get_optional" here. IMO then, the
right answer is to use "clk_get_optional" but then also modify the
check below so that instead of:

/* Only enable writing if we have SoC data. */
if (priv->soc_data)
  econfig.reg_write = qfprom_reg_write;

It is:

/* Only enable writing if we have SoC data and a valid clock */
if (priv->soc_data && priv->secclk)
  econfig.reg_write = qfprom_reg_write;


Does that work for you?


> So after reading all of this I tried running this commmand on the phone
> and the phone reboots into 900e mode.
>
>   $ cat /sys/devices/platform/soc@0/784000.efuse/qfprom0/nvmem
>
> I guess normally this should work? So if I interpret this correctly, the
> Linux driver thinks it can access more than it can/should. But also
> should probably try this command on another chipset to see if it works
> on any really?

Presumably your firmware needs a different "sc7280_qfprom_keepout". If
that's true then I guess you'll have to undergo negotiations with the
DT bindings folks and the nvmem maintainer to figure out how to
specify that your firmware protects different things than the ChromeOS
firmware?


-Doug

Re: [PATCH 02/11] nvmem: qfprom: Mark core clk as optional

[Trilok Soni]

On 8/30/2023 7:43 AM, Luca Weiss wrote:
> On Wed Aug 30, 2023 at 4:30 PM CEST, Doug Anderson wrote:
>> Hi,
>>
>> On Wed, Aug 30, 2023 at 2:58 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
>>>
>>> On some platforms like sc7280 on non-ChromeOS devices the core clock
>>> cannot be touched by Linux so we cannot provide it. Mark it as optional
>>> as accessing qfprom works without it.
>>>
>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>> ---
>>>  drivers/nvmem/qfprom.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Are you actually testing burning fuses from the OS, or are you just
>> using the nvmem in "read-only" mode? From comments in the bindings, if
>> you're trying to burn the fuses then the clock is required. If things
>> are in read-only mode then the clock isn't required.
> 
> Hi Doug,
> 
> I definitely don't plan on burning any fuses on this phone. Not even
> sure that's allowed by the TZ / boot stack.
> 
>>
>> When I compare to the driver, it seems like the driver assumes that if
>> more than one memory region is provided then you must be supporting
>> burning fuses. The bindings agree that having 4 memory regions
>> specified means that the nvmem supports burning and 1 memory region
>> specified means read-only. The extra 3 memory regions in the nvmem are
>> all about fuse burning, I believe.
>>
>> So maybe the right fix here is to just change your dts to specify one
>> memory region?
> 
> I got feedback from Konrad that this here would be the preferred
> approach compared to having a different dts for ChromeOS vs non-ChromeOS
> devices. I don't feel strongly to either, for me it's also okay to
> remove the extra memory regions and only have the main one used on
> regular qcom devices.
> 
> Let me know what you think.

I would prefer to re-use the sc7280 DT as well. Thank you for your patches. We plan to use your patches for platform on the same part. 

-- 
---Trilok Soni

Re: [PATCH 02/11] nvmem: qfprom: Mark core clk as optional

[Konrad Dybcio]

On 1.09.2023 17:08, Doug Anderson wrote:
> Hi,
> 
> On Fri, Sep 1, 2023 at 7:54 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
>>
>>>>> So maybe the right fix here is to just change your dts to specify one
>>>>> memory region?
>>>>
>>>> I got feedback from Konrad that this here would be the preferred
>>>> approach compared to having a different dts for ChromeOS vs non-ChromeOS
>>>> devices. I don't feel strongly to either, for me it's also okay to
>>>> remove the extra memory regions and only have the main one used on
>>>> regular qcom devices.
>>>>
>>>> Let me know what you think.
>>>
>>> I don't hate the idea of leaving the extra memory regions in the dts.
>>> They do describe the hardware, after all, even if the main OS can't
>>> actually access those memory regions. ...though the same could also be
>>> said about the clock you've removed. Said another way: if you want to
>>> fully describe the hardware then the dts should have the extra memory
>>> regions and the clock. If you are OK w/ just describing the hardware
>>> in the way that the OS has access to then the dts should not have the
>>> extra memory regions and not have the clock. Does that sound right?
>>
>> Not sure which of those memory regions are actually accessible on this
>> board, but honestly I don't even want to try accessing it. Blowing fuses
>> is not my wish there ;)
>>
>> On downstream the node is just described like the following:
>>
>>         qfprom: qfprom@780000 {
>>                 compatible = "qcom,qfprom";
>>                 reg = <0x780000 0x7000>;
>>                 ...
>>         };
>>
>> So we have 0x780000 - 0x786fff here.
>>
>> In sc7280.dtsi we have the following:
>>
>>         qfprom: efuse@784000 {
>>                 compatible = "qcom,sc7280-qfprom", "qcom,qfprom";
>>                 reg = <0 0x00784000 0 0xa20>,
>>                           <0 0x00780000 0 0xa20>,
>>                           <0 0x00782000 0 0x120>,
>>                           <0 0x00786000 0 0x1fff>;
>>                 ...
>>         };
>>
>> So I guess this:
>> * 0x780000 - 0x780a1f
>> * 0x782000 - 0x78211f
>> * 0x784000 - 0x784a1f
>> * 0x786000 - 0x787ffe
>>
>> So at least the last memory region seems to be partially out of range
>> according to downstream.
> 
> From the other discussion, it sounds as if you _can_ leave the clock
> in the device tree and then use "clk_get_optional" here. IMO then, the
> right answer is to use "clk_get_optional" but then also modify the
> check below so that instead of:
> 
> /* Only enable writing if we have SoC data. */
> if (priv->soc_data)
>   econfig.reg_write = qfprom_reg_write;
> 
> It is:
> 
> /* Only enable writing if we have SoC data and a valid clock */
> if (priv->soc_data && priv->secclk)
>   econfig.reg_write = qfprom_reg_write;
> 
> 
> Does that work for you?
> 
> 
>> So after reading all of this I tried running this commmand on the phone
>> and the phone reboots into 900e mode.
>>
>>   $ cat /sys/devices/platform/soc@0/784000.efuse/qfprom0/nvmem
>>
>> I guess normally this should work? So if I interpret this correctly, the
>> Linux driver thinks it can access more than it can/should. But also
>> should probably try this command on another chipset to see if it works
>> on any really?
> 
> Presumably your firmware needs a different "sc7280_qfprom_keepout". If
> that's true then I guess you'll have to undergo negotiations with the
> DT bindings folks and the nvmem maintainer to figure out how to
> specify that your firmware protects different things than the ChromeOS
> firmware?
Luca, if you feel like wasting some time, you can probably bruteforce
this.

I assume this keepout thing could be expanded in a generic way and
made into a dt property.

Other than that, I think it'd be fine to skip that for now, as it
sounds like it's functional so long as you don't intentionally access
forbidden regs.

Konrad

Re: [PATCH 11/11] arm64: dts: qcom: qcm6490: Add device-tree for Fairphone 5

[Konrad Dybcio]

On 1.09.2023 16:27, Luca Weiss wrote:
> On Wed Aug 30, 2023 at 12:45 PM CEST, Konrad Dybcio wrote:
>> On 30.08.2023 11:58, Luca Weiss wrote:
>>> Add device tree for the Fairphone 5 smartphone which is based on
>>> the QCM6490 SoC.
>>>
>>> Supported features are, as of now:
>>> * Bluetooth
>>> * Debug UART
>>> * Display via simplefb
>>> * Flash/torch LED
>>> * Flip cover sensor
>>> * Power & volume buttons
>>> * RTC
>>> * SD card
>>> * USB
>>> * Various plumbing like regulators, i2c, spi, etc
>>>
>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>> ---
[...]

>>> +	panel: panel {
>>> +		compatible = "boe,rm692e5";
>> Undocumented compatbile
> 
> Not quite sure how to document this properly. This dummy panel node is
> used for the simplefb/simpledrm so that user space can get panel
> dimensions already.
> 
> This approach of a separate panel node was apparently preferred when the
> functionality was upstreamed but of course as can be seen this panel
> node is very barebones where many properties are not present which would
> be needed with the full panel driver (once it's working).
> 
> Also for example for the bindings I think width-mm and height-mm won't
> be needed anymore since that information will be provided by the driver
> then.
> 
> Please let me know how/where to document this.
As discussed offline, the workflow here would be to oneOf: (wink)

- wait until there's a proper driver and create a binding based on
  what you know (because you have a working driver and can test it)
  is necessary for it to function

- create the binding for that display panel + driver ic combo in
  advance and pray that whatever you put there will be enough when
  you take upon yourself to write the driver

I'd suggest dropping these properties (or keeping them downstream or
something) for now, the display should not be terribly hard to bring
up properly, let's hope that can be done soon!

> I'm aware, but will work on this later since it's not useful without
> USB-C role switching and working audio.
You can still peek at it in like sysfs or so, but up to you.


Konrad

Re: [PATCH 02/11] nvmem: qfprom: Mark core clk as optional

[Luca Weiss]

On Fri Sep 1, 2023 at 5:08 PM CEST, Doug Anderson wrote:
> Hi,
>
> On Fri, Sep 1, 2023 at 7:54 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> >
> > > > > So maybe the right fix here is to just change your dts to specify one
> > > > > memory region?
> > > >
> > > > I got feedback from Konrad that this here would be the preferred
> > > > approach compared to having a different dts for ChromeOS vs non-ChromeOS
> > > > devices. I don't feel strongly to either, for me it's also okay to
> > > > remove the extra memory regions and only have the main one used on
> > > > regular qcom devices.
> > > >
> > > > Let me know what you think.
> > >
> > > I don't hate the idea of leaving the extra memory regions in the dts.
> > > They do describe the hardware, after all, even if the main OS can't
> > > actually access those memory regions. ...though the same could also be
> > > said about the clock you've removed. Said another way: if you want to
> > > fully describe the hardware then the dts should have the extra memory
> > > regions and the clock. If you are OK w/ just describing the hardware
> > > in the way that the OS has access to then the dts should not have the
> > > extra memory regions and not have the clock. Does that sound right?
> >
> > Not sure which of those memory regions are actually accessible on this
> > board, but honestly I don't even want to try accessing it. Blowing fuses
> > is not my wish there ;)
> >
> > On downstream the node is just described like the following:
> >
> >         qfprom: qfprom@780000 {
> >                 compatible = "qcom,qfprom";
> >                 reg = <0x780000 0x7000>;
> >                 ...
> >         };
> >
> > So we have 0x780000 - 0x786fff here.
> >
> > In sc7280.dtsi we have the following:
> >
> >         qfprom: efuse@784000 {
> >                 compatible = "qcom,sc7280-qfprom", "qcom,qfprom";
> >                 reg = <0 0x00784000 0 0xa20>,
> >                           <0 0x00780000 0 0xa20>,
> >                           <0 0x00782000 0 0x120>,
> >                           <0 0x00786000 0 0x1fff>;
> >                 ...
> >         };
> >
> > So I guess this:
> > * 0x780000 - 0x780a1f
> > * 0x782000 - 0x78211f
> > * 0x784000 - 0x784a1f
> > * 0x786000 - 0x787ffe
> >
> > So at least the last memory region seems to be partially out of range
> > according to downstream.
>
> From the other discussion, it sounds as if you _can_ leave the clock
> in the device tree and then use "clk_get_optional" here. IMO then, the
> right answer is to use "clk_get_optional" but then also modify the
> check below so that instead of:
>
> /* Only enable writing if we have SoC data. */
> if (priv->soc_data)
>   econfig.reg_write = qfprom_reg_write;
>
> It is:
>
> /* Only enable writing if we have SoC data and a valid clock */
> if (priv->soc_data && priv->secclk)
>   econfig.reg_write = qfprom_reg_write;
>
>
> Does that work for you?

Thanks Doug, this feels like a good solution. I'll update v2 with that.

>
>
> > So after reading all of this I tried running this commmand on the phone
> > and the phone reboots into 900e mode.
> >
> >   $ cat /sys/devices/platform/soc@0/784000.efuse/qfprom0/nvmem
> >
> > I guess normally this should work? So if I interpret this correctly, the
> > Linux driver thinks it can access more than it can/should. But also
> > should probably try this command on another chipset to see if it works
> > on any really?
>
> Presumably your firmware needs a different "sc7280_qfprom_keepout". If
> that's true then I guess you'll have to undergo negotiations with the
> DT bindings folks and the nvmem maintainer to figure out how to
> specify that your firmware protects different things than the ChromeOS
> firmware?

Right. But based on Konrad's reply here, I think I'll skip this for now
since generally qfprom seems to be fine with the current data, at least
for the purposes it's used for on my device.

Regards
Luca


>
>
> -Doug

Re: [PATCH 11/11] arm64: dts: qcom: qcm6490: Add device-tree for Fairphone 5

[Luca Weiss]

On Sat Sep 2, 2023 at 1:45 PM CEST, Konrad Dybcio wrote:
> On 1.09.2023 16:27, Luca Weiss wrote:
> > On Wed Aug 30, 2023 at 12:45 PM CEST, Konrad Dybcio wrote:
> >> On 30.08.2023 11:58, Luca Weiss wrote:
> >>> Add device tree for the Fairphone 5 smartphone which is based on
> >>> the QCM6490 SoC.
> >>>
> >>> Supported features are, as of now:
> >>> * Bluetooth
> >>> * Debug UART
> >>> * Display via simplefb
> >>> * Flash/torch LED
> >>> * Flip cover sensor
> >>> * Power & volume buttons
> >>> * RTC
> >>> * SD card
> >>> * USB
> >>> * Various plumbing like regulators, i2c, spi, etc
> >>>
> >>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> >>> ---
> [...]
>
> >>> +	panel: panel {
> >>> +		compatible = "boe,rm692e5";
> >> Undocumented compatbile
> > 
> > Not quite sure how to document this properly. This dummy panel node is
> > used for the simplefb/simpledrm so that user space can get panel
> > dimensions already.
> > 
> > This approach of a separate panel node was apparently preferred when the
> > functionality was upstreamed but of course as can be seen this panel
> > node is very barebones where many properties are not present which would
> > be needed with the full panel driver (once it's working).
> > 
> > Also for example for the bindings I think width-mm and height-mm won't
> > be needed anymore since that information will be provided by the driver
> > then.
> > 
> > Please let me know how/where to document this.
> As discussed offline, the workflow here would be to oneOf: (wink)
>
> - wait until there's a proper driver and create a binding based on
>   what you know (because you have a working driver and can test it)
>   is necessary for it to function
>
> - create the binding for that display panel + driver ic combo in
>   advance and pray that whatever you put there will be enough when
>   you take upon yourself to write the driver
>
> I'd suggest dropping these properties (or keeping them downstream or
> something) for now, the display should not be terribly hard to bring
> up properly, let's hope that can be done soon!

Yeah that sounds like a solution for now.

But for other devices I hope something can be figured out so that these
dummy panels can go into upstream dts. Otherwise the functionality added
in the simplefb driver is kinda useless for pure upstream if noone's
allowed to have incomplete bindings for providing the panel dimensions.

>
> > I'm aware, but will work on this later since it's not useful without
> > USB-C role switching and working audio.
> You can still peek at it in like sysfs or so, but up to you.

Yes, later ;)

Regards
Luca

>
>
> Konrad

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Luca Weiss]

On Thu Aug 31, 2023 at 2:27 PM CEST, Dmitry Baryshkov wrote:
> On Thu, 31 Aug 2023 at 14:54, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> > > On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
> > >>
> > >> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> > >>> On 30/08/2023 11:58, Luca Weiss wrote:
> > >>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
> > >>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
> > >>>> possible.
> > >>>>
> > >>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > >>>> ---
> > >>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> > >>>>  1 file changed, 16 insertions(+), 7 deletions(-)
> > >>>>
> > >>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > >>>> index e8540c36bd99..3514de536baa 100644
> > >>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > >>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > >>>> @@ -7,6 +7,15 @@
> > >>>>  #include <dt-bindings/interrupt-controller/irq.h>
> > >>>>  #include <dt-bindings/spmi/spmi.h>
> > >>>>
> > >>>> +/* This PMIC can be configured to be at different SIDs */
> > >>>> +#ifndef PM7250B_SID
> > >>>> +   #define PM7250B_SID 2
> > >>>> +#endif
> > >>>
> > >>> Why do you send the same patch as v1, without any reference to previous
> > >>> discussions?
> > >>>
> > >>> You got here feedback already.
> > >>>
> > >>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
> > >>
> > >> Hi Krzysztof,
> > >>
> > >> I did mention that original patch in the cover letter of this series.
> > >> I'm definitely aware of the discussion earlier this year there but also
> > >> tried to get an update lately if there's any update with no response.
> > >
> > > I think the overall consensus was that my proposal is too complicated
> > > for the DT files.
> >
> > I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
> > customize per address? No.
>
> At the same time, we do keep SoC files separate from the board files.
> Yes, I'm slightly exaggerating here.
>
> I think that for PMIC files it makes sense to extract common parts if
> that eases reuse of the common parts.

Hi all,

what can I do for v2 now?

1. Keep this patch as-is, and keep pm7250b in device dts.

2. Drop pm7250b patch and drop from device dts, until _someone_ figures
out a solution talking to the PMIC on different SID.

3. Something else like copy-pasting pm7250b.dtsi to pm7250-8.dtsi and
changing the SID there, and using that in device dts.

Please let me know what to do.

Regards
Luca

>
> >
> > I definitely do not agree to these ifndef->define. Maybe using just
> > define would work (so drop ifndef->define), because this makes it
> > obvious and fail-safe if included in wrong place... except that it is
> > still not the define we expect. This is not the coding style present in
> > other DTSes.
> >
> > The true problem how these SPMI bindings were created. Requiring SID
> > address in every child is clearly redundant and I think we do not follow
> > such approach anywhere else.
> >
> > Best regards,
> > Krzysztof
> >

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Luca Weiss]

On Tue Sep 5, 2023 at 10:30 AM CEST, Luca Weiss wrote:
> On Thu Aug 31, 2023 at 2:27 PM CEST, Dmitry Baryshkov wrote:
> > On Thu, 31 Aug 2023 at 14:54, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > >
> > > On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> > > > On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
> > > >>
> > > >> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> > > >>> On 30/08/2023 11:58, Luca Weiss wrote:
> > > >>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
> > > >>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
> > > >>>> possible.
> > > >>>>
> > > >>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > >>>> ---
> > > >>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> > > >>>>  1 file changed, 16 insertions(+), 7 deletions(-)
> > > >>>>
> > > >>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > >>>> index e8540c36bd99..3514de536baa 100644
> > > >>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > >>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > >>>> @@ -7,6 +7,15 @@
> > > >>>>  #include <dt-bindings/interrupt-controller/irq.h>
> > > >>>>  #include <dt-bindings/spmi/spmi.h>
> > > >>>>
> > > >>>> +/* This PMIC can be configured to be at different SIDs */
> > > >>>> +#ifndef PM7250B_SID
> > > >>>> +   #define PM7250B_SID 2
> > > >>>> +#endif
> > > >>>
> > > >>> Why do you send the same patch as v1, without any reference to previous
> > > >>> discussions?
> > > >>>
> > > >>> You got here feedback already.
> > > >>>
> > > >>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
> > > >>
> > > >> Hi Krzysztof,
> > > >>
> > > >> I did mention that original patch in the cover letter of this series.
> > > >> I'm definitely aware of the discussion earlier this year there but also
> > > >> tried to get an update lately if there's any update with no response.
> > > >
> > > > I think the overall consensus was that my proposal is too complicated
> > > > for the DT files.
> > >
> > > I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
> > > customize per address? No.
> >
> > At the same time, we do keep SoC files separate from the board files.
> > Yes, I'm slightly exaggerating here.
> >
> > I think that for PMIC files it makes sense to extract common parts if
> > that eases reuse of the common parts.
>
> Hi all,
>
> what can I do for v2 now?
>
> 1. Keep this patch as-is, and keep pm7250b in device dts.
>
> 2. Drop pm7250b patch and drop from device dts, until _someone_ figures
> out a solution talking to the PMIC on different SID.
>
> 3. Something else like copy-pasting pm7250b.dtsi to pm7250-8.dtsi and
> changing the SID there, and using that in device dts.
>
> Please let me know what to do.
>
> Regards
> Luca

Hi,

if there's no feedback I'll keep this patch in v2 of this series and we
can continue to discuss there (if necessary).

Regards
Luca

>
> >
> > >
> > > I definitely do not agree to these ifndef->define. Maybe using just
> > > define would work (so drop ifndef->define), because this makes it
> > > obvious and fail-safe if included in wrong place... except that it is
> > > still not the define we expect. This is not the coding style present in
> > > other DTSes.
> > >
> > > The true problem how these SPMI bindings were created. Requiring SID
> > > address in every child is clearly redundant and I think we do not follow
> > > such approach anywhere else.
> > >
> > > Best regards,
> > > Krzysztof
> > >

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Krzysztof Kozlowski]

On 11/09/2023 10:34, Luca Weiss wrote:
> On Tue Sep 5, 2023 at 10:30 AM CEST, Luca Weiss wrote:
>> On Thu Aug 31, 2023 at 2:27 PM CEST, Dmitry Baryshkov wrote:
>>> On Thu, 31 Aug 2023 at 14:54, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 31/08/2023 13:33, Dmitry Baryshkov wrote:
>>>>> On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
>>>>>>
>>>>>> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
>>>>>>> On 30/08/2023 11:58, Luca Weiss wrote:
>>>>>>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
>>>>>>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
>>>>>>>> possible.
>>>>>>>>
>>>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>>>>>> ---
>>>>>>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
>>>>>>>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>> index e8540c36bd99..3514de536baa 100644
>>>>>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>> @@ -7,6 +7,15 @@
>>>>>>>>  #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>>>  #include <dt-bindings/spmi/spmi.h>
>>>>>>>>
>>>>>>>> +/* This PMIC can be configured to be at different SIDs */
>>>>>>>> +#ifndef PM7250B_SID
>>>>>>>> +   #define PM7250B_SID 2
>>>>>>>> +#endif
>>>>>>>
>>>>>>> Why do you send the same patch as v1, without any reference to previous
>>>>>>> discussions?
>>>>>>>
>>>>>>> You got here feedback already.
>>>>>>>
>>>>>>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
>>>>>>
>>>>>> Hi Krzysztof,
>>>>>>
>>>>>> I did mention that original patch in the cover letter of this series.
>>>>>> I'm definitely aware of the discussion earlier this year there but also
>>>>>> tried to get an update lately if there's any update with no response.
>>>>>
>>>>> I think the overall consensus was that my proposal is too complicated
>>>>> for the DT files.
>>>>
>>>> I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
>>>> customize per address? No.
>>>
>>> At the same time, we do keep SoC files separate from the board files.
>>> Yes, I'm slightly exaggerating here.
>>>
>>> I think that for PMIC files it makes sense to extract common parts if
>>> that eases reuse of the common parts.
>>
>> Hi all,
>>
>> what can I do for v2 now?
>>
>> 1. Keep this patch as-is, and keep pm7250b in device dts.

This was NAKed by me. What Qualcomm SoC maintainers decide (or not
decide) about other options, should not cause the wrong solution to be
re-posted...

>>
>> 2. Drop pm7250b patch and drop from device dts, until _someone_ figures
>> out a solution talking to the PMIC on different SID.
>>
>> 3. Something else like copy-pasting pm7250b.dtsi to pm7250-8.dtsi and
>> changing the SID there, and using that in device dts.
>>
>> Please let me know what to do.
>>
>> Regards
>> Luca
> 
> Hi,
> 
> if there's no feedback I'll keep this patch in v2 of this series and we
> can continue to discuss there (if necessary).

Sorry, I still do not agree and there were no arguments convincing me to
change the mind.

I gave you the solution from my perspective. Why do you decided to
ignore it and send it as is?


Best regards,
Krzysztof

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Luca Weiss]

On Mon Sep 11, 2023 at 11:44 AM CEST, Krzysztof Kozlowski wrote:
> On 11/09/2023 10:34, Luca Weiss wrote:
> > On Tue Sep 5, 2023 at 10:30 AM CEST, Luca Weiss wrote:
> >> On Thu Aug 31, 2023 at 2:27 PM CEST, Dmitry Baryshkov wrote:
> >>> On Thu, 31 Aug 2023 at 14:54, Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> >>>>> On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
> >>>>>>
> >>>>>> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> >>>>>>> On 30/08/2023 11:58, Luca Weiss wrote:
> >>>>>>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
> >>>>>>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
> >>>>>>>> possible.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> >>>>>>>> ---
> >>>>>>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> >>>>>>>>  1 file changed, 16 insertions(+), 7 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>>>>>> index e8540c36bd99..3514de536baa 100644
> >>>>>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>>>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>>>>>> @@ -7,6 +7,15 @@
> >>>>>>>>  #include <dt-bindings/interrupt-controller/irq.h>
> >>>>>>>>  #include <dt-bindings/spmi/spmi.h>
> >>>>>>>>
> >>>>>>>> +/* This PMIC can be configured to be at different SIDs */
> >>>>>>>> +#ifndef PM7250B_SID
> >>>>>>>> +   #define PM7250B_SID 2
> >>>>>>>> +#endif
> >>>>>>>
> >>>>>>> Why do you send the same patch as v1, without any reference to previous
> >>>>>>> discussions?
> >>>>>>>
> >>>>>>> You got here feedback already.
> >>>>>>>
> >>>>>>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
> >>>>>>
> >>>>>> Hi Krzysztof,
> >>>>>>
> >>>>>> I did mention that original patch in the cover letter of this series.
> >>>>>> I'm definitely aware of the discussion earlier this year there but also
> >>>>>> tried to get an update lately if there's any update with no response.
> >>>>>
> >>>>> I think the overall consensus was that my proposal is too complicated
> >>>>> for the DT files.
> >>>>
> >>>> I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
> >>>> customize per address? No.
> >>>
> >>> At the same time, we do keep SoC files separate from the board files.
> >>> Yes, I'm slightly exaggerating here.
> >>>
> >>> I think that for PMIC files it makes sense to extract common parts if
> >>> that eases reuse of the common parts.
> >>
> >> Hi all,
> >>
> >> what can I do for v2 now?
> >>
> >> 1. Keep this patch as-is, and keep pm7250b in device dts.
>
> This was NAKed by me. What Qualcomm SoC maintainers decide (or not
> decide) about other options, should not cause the wrong solution to be
> re-posted...
>
> >>
> >> 2. Drop pm7250b patch and drop from device dts, until _someone_ figures
> >> out a solution talking to the PMIC on different SID.
> >>
> >> 3. Something else like copy-pasting pm7250b.dtsi to pm7250-8.dtsi and
> >> changing the SID there, and using that in device dts.

@Konrad, @Bjorn: Can you give any feedback here what's preferable?
Otherwise I'm just blocked on this series.

> >>
> >> Please let me know what to do.
> >>
> >> Regards
> >> Luca
> > 
> > Hi,
> > 
> > if there's no feedback I'll keep this patch in v2 of this series and we
> > can continue to discuss there (if necessary).
>
> Sorry, I still do not agree and there were no arguments convincing me to
> change the mind.
>
> I gave you the solution from my perspective. Why do you decided to
> ignore it and send it as is?

I get it that you are not final decider for qcom dts changes but it's
quite difficult for someone sending patches to not get any feedback what
other change to replace this is appropriate. I doubt it's a good idea to
just implement some random pm7250-8.dtsi or whatever to potentially
immediately get a response that that way is also bad.

That's why I'm trying to get some info before working on something and
sending it. Hopefully Bjorn or Konrad can add their thoughts above.

Also I don't recall me ever reading a "solution" from your side but
maybe I need to dig through the old emails again.

Regards
Luca

>
>
> Best regards,
> Krzysztof

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Krzysztof Kozlowski]

On 11/09/2023 11:59, Luca Weiss wrote:
> On Mon Sep 11, 2023 at 11:44 AM CEST, Krzysztof Kozlowski wrote:
>> On 11/09/2023 10:34, Luca Weiss wrote:
>>> On Tue Sep 5, 2023 at 10:30 AM CEST, Luca Weiss wrote:
>>>> On Thu Aug 31, 2023 at 2:27 PM CEST, Dmitry Baryshkov wrote:
>>>>> On Thu, 31 Aug 2023 at 14:54, Krzysztof Kozlowski
>>>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>>>
>>>>>> On 31/08/2023 13:33, Dmitry Baryshkov wrote:
>>>>>>> On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
>>>>>>>>
>>>>>>>> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
>>>>>>>>> On 30/08/2023 11:58, Luca Weiss wrote:
>>>>>>>>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
>>>>>>>>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
>>>>>>>>>> possible.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>>>>>>>> ---
>>>>>>>>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
>>>>>>>>>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>>>> index e8540c36bd99..3514de536baa 100644
>>>>>>>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>>>> @@ -7,6 +7,15 @@
>>>>>>>>>>  #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>>>>>  #include <dt-bindings/spmi/spmi.h>
>>>>>>>>>>
>>>>>>>>>> +/* This PMIC can be configured to be at different SIDs */
>>>>>>>>>> +#ifndef PM7250B_SID
>>>>>>>>>> +   #define PM7250B_SID 2
>>>>>>>>>> +#endif
>>>>>>>>>
>>>>>>>>> Why do you send the same patch as v1, without any reference to previous
>>>>>>>>> discussions?
>>>>>>>>>
>>>>>>>>> You got here feedback already.
>>>>>>>>>
>>>>>>>>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
>>>>>>>>
>>>>>>>> Hi Krzysztof,
>>>>>>>>
>>>>>>>> I did mention that original patch in the cover letter of this series.
>>>>>>>> I'm definitely aware of the discussion earlier this year there but also
>>>>>>>> tried to get an update lately if there's any update with no response.
>>>>>>>
>>>>>>> I think the overall consensus was that my proposal is too complicated
>>>>>>> for the DT files.
>>>>>>
>>>>>> I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
>>>>>> customize per address? No.
>>>>>
>>>>> At the same time, we do keep SoC files separate from the board files.
>>>>> Yes, I'm slightly exaggerating here.
>>>>>
>>>>> I think that for PMIC files it makes sense to extract common parts if
>>>>> that eases reuse of the common parts.
>>>>
>>>> Hi all,
>>>>
>>>> what can I do for v2 now?
>>>>
>>>> 1. Keep this patch as-is, and keep pm7250b in device dts.
>>
>> This was NAKed by me. What Qualcomm SoC maintainers decide (or not
>> decide) about other options, should not cause the wrong solution to be
>> re-posted...
>>
>>>>
>>>> 2. Drop pm7250b patch and drop from device dts, until _someone_ figures
>>>> out a solution talking to the PMIC on different SID.
>>>>
>>>> 3. Something else like copy-pasting pm7250b.dtsi to pm7250-8.dtsi and
>>>> changing the SID there, and using that in device dts.
> 
> @Konrad, @Bjorn: Can you give any feedback here what's preferable?
> Otherwise I'm just blocked on this series.
> 
>>>>
>>>> Please let me know what to do.
>>>>
>>>> Regards
>>>> Luca
>>>
>>> Hi,
>>>
>>> if there's no feedback I'll keep this patch in v2 of this series and we
>>> can continue to discuss there (if necessary).
>>
>> Sorry, I still do not agree and there were no arguments convincing me to
>> change the mind.
>>
>> I gave you the solution from my perspective. Why do you decided to
>> ignore it and send it as is?
> 
> I get it that you are not final decider for qcom dts changes but it's
> quite difficult for someone sending patches to not get any feedback what
> other change to replace this is appropriate. I doubt it's a good idea to
> just implement some random pm7250-8.dtsi or whatever to potentially
> immediately get a response that that way is also bad.
> 
> That's why I'm trying to get some info before working on something and
> sending it. Hopefully Bjorn or Konrad can add their thoughts above.

I understand, and it is frustrating. If such case happens the solution
in upstream is not sending the same NAKed version but send something else.

> 
> Also I don't recall me ever reading a "solution" from your side but
> maybe I need to dig through the old emails again.

Here:
"I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
customize per address? No."

Dmitry responded that having PMICs extracted help re-using. He is right.
But here you hit the limit of such re-usage.

Best regards,
Krzysztof

Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

[Konrad Dybcio]

On 11.09.2023 13:15, Krzysztof Kozlowski wrote:
> On 11/09/2023 11:59, Luca Weiss wrote:
>> On Mon Sep 11, 2023 at 11:44 AM CEST, Krzysztof Kozlowski wrote:
>>> On 11/09/2023 10:34, Luca Weiss wrote:
>>>> On Tue Sep 5, 2023 at 10:30 AM CEST, Luca Weiss wrote:
>>>>> On Thu Aug 31, 2023 at 2:27 PM CEST, Dmitry Baryshkov wrote:
>>>>>> On Thu, 31 Aug 2023 at 14:54, Krzysztof Kozlowski
>>>>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>>>>
>>>>>>> On 31/08/2023 13:33, Dmitry Baryshkov wrote:
>>>>>>>> On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
>>>>>>>>>
>>>>>>>>> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
>>>>>>>>>> On 30/08/2023 11:58, Luca Weiss wrote:
>>>>>>>>>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
>>>>>>>>>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
>>>>>>>>>>> possible.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>>>>>>>>> ---
>>>>>>>>>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
>>>>>>>>>>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>>>>> index e8540c36bd99..3514de536baa 100644
>>>>>>>>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>>>>> @@ -7,6 +7,15 @@
>>>>>>>>>>>  #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>>>>>>  #include <dt-bindings/spmi/spmi.h>
>>>>>>>>>>>
>>>>>>>>>>> +/* This PMIC can be configured to be at different SIDs */
>>>>>>>>>>> +#ifndef PM7250B_SID
>>>>>>>>>>> +   #define PM7250B_SID 2
>>>>>>>>>>> +#endif
>>>>>>>>>>
>>>>>>>>>> Why do you send the same patch as v1, without any reference to previous
>>>>>>>>>> discussions?
>>>>>>>>>>
>>>>>>>>>> You got here feedback already.
>>>>>>>>>>
>>>>>>>>>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
>>>>>>>>>
>>>>>>>>> Hi Krzysztof,
>>>>>>>>>
>>>>>>>>> I did mention that original patch in the cover letter of this series.
>>>>>>>>> I'm definitely aware of the discussion earlier this year there but also
>>>>>>>>> tried to get an update lately if there's any update with no response.
>>>>>>>>
>>>>>>>> I think the overall consensus was that my proposal is too complicated
>>>>>>>> for the DT files.
>>>>>>>
>>>>>>> I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
>>>>>>> customize per address? No.
>>>>>>
>>>>>> At the same time, we do keep SoC files separate from the board files.
>>>>>> Yes, I'm slightly exaggerating here.
>>>>>>
>>>>>> I think that for PMIC files it makes sense to extract common parts if
>>>>>> that eases reuse of the common parts.
>>>>>
>>>>> Hi all,
>>>>>
>>>>> what can I do for v2 now?
>>>>>
>>>>> 1. Keep this patch as-is, and keep pm7250b in device dts.
>>>
>>> This was NAKed by me. What Qualcomm SoC maintainers decide (or not
>>> decide) about other options, should not cause the wrong solution to be
>>> re-posted...
>>>
>>>>>
>>>>> 2. Drop pm7250b patch and drop from device dts, until _someone_ figures
>>>>> out a solution talking to the PMIC on different SID.
>>>>>
>>>>> 3. Something else like copy-pasting pm7250b.dtsi to pm7250-8.dtsi and
>>>>> changing the SID there, and using that in device dts.
>>
>> @Konrad, @Bjorn: Can you give any feedback here what's preferable?
>> Otherwise I'm just blocked on this series.
I'm sure Krzysztof will disagree, but all of the solutions (which are
either duplicate the dt, add ifdefs or skip adding this pmic) are
equally band-aid-class.. A bright future where this PMIC thing is
handled on the driver side that will hopefully come soon(tm) should
resolve such problems..

From my side, ifdef is the least burdensome, even if ugly..

Konrad

Re: (subset) [PATCH 00/11] Initial support for the Fairphone 5 smartphone

[Bjorn Andersson]

On Wed, 30 Aug 2023 11:58:25 +0200, Luca Weiss wrote:
> Add support to boot up mainline kernel on the QCM6490-based Fairphone 5
> smartphone.
> 
> These patches only cover a part of the functionality brought up on
> mainline so far, with the rest needing larger dts and driver changes or
> depend on patches that are not yet merged. I will work on sending those
> once these base patches here have settled.
> 
> [...]

Applied, thanks!

[07/11] dt-bindings: arm: qcom,ids: Add SoC ID for QCM6490
        commit: ccfb4d8b606302d857a03ea29039e21029311335
[08/11] soc: qcom: socinfo: Add SoC ID for QCM6490
        commit: 59872d59d164ec67f295d6f96fe818b92973ee40

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