Skip to contentSkip to conversation

clk: qcom: add MSM8x60 LPASS Clock Controller

PATCH Superseded Herman van Hazendonk 10 participants 60 messages May 30 – Jun 12

Subsystems (12)
lore
Share & tools
lore
#0 lore raw

cover clk: qcom: add MSM8x60 LPASS Clock Controller

Hi all,

This series adds the LPASS (Low Power Audio SubSystem) Clock Controller driver for the MSM8x60 family of SoCs (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The register layout, parent muxing and divider topology of the LPASS PLL/clk fabric differ from MSM8960's LCC enough that a clean separate driver is simpler than parameterising lcc-msm8960.c. Both drivers can coexist (different Kconfig, match table and compatible).

Used on the HP TouchPad (Tenderloin) where the LPASS Q6 audio DSP needs functional MI2S / SLIMBus / PCM clocks before audio playback or capture works.

The new binding header is dual-licensed (GPL-2.0-only OR BSD-2-Clause) per current qcom-binding convention.

Companion to the MSM8x60 MMCC series.

Thanks,
Herman

Herman van Hazendonk (2):
dt-bindings: clock: qcom: add lcc-msm8660 LPASS clock IDs
clk: qcom: add MSM8x60 LCC (LPASS) driver

 drivers/clk/qcom/Kconfig                     |   9 +
 drivers/clk/qcom/Makefile                    |   1 +
 drivers/clk/qcom/lcc-msm8660.c               | 517 +++++++++++++++++++
 include/dt-bindings/clock/qcom,lcc-msm8660.h |  48 ++
 4 files changed, 575 insertions(+)
 create mode 100644 drivers/clk/qcom/lcc-msm8660.c
 create mode 100644 include/dt-bindings/clock/qcom,lcc-msm8660.h
Signature · 1 line
2.43.0
#14 lore raw

PATCH 2/2 interconnect: qcom: add MSM8x60 NoC driver

Add a Qualcomm interconnect driver for the MSM8x60 family (MSM8260/MSM8660/APQ8060), modelling the four fabrics that connect masters and slaves on these Scorpion-class SoCs:

- AFAB : Application/CPU fabric
- SFAB : System fabric (peripherals, USB, SDCC, etc.)
- MMFAB : Multimedia fabric (MDP, VFE, VIDC, GPU, JPEG, VPE, ROT)
- DFAB : Daytona fabric (low-bandwidth peripherals)

The driver implements the interconnect-provider API so that consumer drivers (display, camera, video, GPU, USB, MMC) can request bandwidth between specific masters and slaves via icc_set_bw(), letting the firmware-managed bus-scaling decide actual NoC clock rates.

Used on the HP TouchPad (Tenderloin) and on the Pre3 / Veer form-factors; without it, the multimedia and storage paths are starved of bandwidth and run at minimum NoC clocks.

---
 drivers/interconnect/qcom/Kconfig   |   10 +
 drivers/interconnect/qcom/Makefile  |    2 +
 drivers/interconnect/qcom/msm8660.c | 1008 +++++++++++++++++++++++++++
 3 files changed, 1020 insertions(+)
 create mode 100644 drivers/interconnect/qcom/msm8660.c
drivers/interconnect/qcom/Kconfig +10 -0
index 786b4eda44b4..17364be522c7 100644
@@ -80,6 +80,16 @@ config INTERCONNECT_QCOM_MSM8953
 	  This is a driver for the Qualcomm Network-on-Chip on msm8953-based
 	  platforms.
 
+config INTERCONNECT_QCOM_MSM8660
+	tristate "Qualcomm MSM8x60 interconnect driver"
+	depends on INTERCONNECT_QCOM
+	depends on MFD_QCOM_RPM
+	help
+	  This is a driver for the Qualcomm fabric-based bus interconnect
+	  on MSM8x60 family (MSM8260/MSM8660/APQ8060) platforms (e.g., HP TouchPad).
+	  The driver manages APPSS, System, and MMSS fabrics and sends
+	  per-port bandwidth arbitration requests to RPM firmware.
+
 config INTERCONNECT_QCOM_MSM8974
 	tristate "Qualcomm MSM8974 interconnect driver"
 	depends on INTERCONNECT_QCOM
drivers/interconnect/qcom/Makefile +2 -0
index cdf2c6c9fbf3..0c849c30a907 100644
@@ -13,6 +13,7 @@ qnoc-msm8916-objs			:= msm8916.o
 qnoc-msm8937-objs			:= msm8937.o
 qnoc-msm8939-objs			:= msm8939.o
 qnoc-msm8953-objs			:= msm8953.o
+qnoc-msm8660-objs			:= msm8660.o
 qnoc-msm8974-objs			:= msm8974.o
 qnoc-msm8976-objs			:= msm8976.o
 qnoc-msm8996-objs			:= msm8996.o
@@ -58,6 +59,7 @@ obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += qnoc-msm8916.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8937) += qnoc-msm8937.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8939) += qnoc-msm8939.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8953) += qnoc-msm8953.o
+obj-$(CONFIG_INTERCONNECT_QCOM_MSM8660) += qnoc-msm8660.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8974) += qnoc-msm8974.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8976) += qnoc-msm8976.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8996) += qnoc-msm8996.o
drivers/interconnect/qcom/msm8660.c new file +1008 -0

1,008 lines not on this page · raw message

Signature · 1 line
2.43.0
#1 lore raw

PATCH 1/2 dt-bindings: interrupt-controller: qcom: add msm8660-mpm

In reply to Herman van Hazendonk

Add the binding for the Modem Power Manager (MPM) interrupt controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The MPM is a small wake-capable interrupt aggregator that lets the SoC stay in low-power states while a small set of GIC SPIs continues to be monitored and can wake the system on an edge.

The binding describes its register region (in the RPM syscon), the GIC SPI it raises on a wake event, the GIC mailbox it shares with the RPM, and the per-pin wake-source mapping table.

---
 .../qcom,msm8660-mpm.yaml                     | 122 ++++++++++++++++++
 1 file changed, 122 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +122 -0
new file mode 100644
index 000000000000..72f9c2be03b2
@@ -0,0 +1,122 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/qcom,msm8660-mpm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm MSM8x60 MPM wakeup interrupt controller
+
+maintainers:
+  - Herman van Hazendonk <[email protected]>
+
+description: |
+  The MSM Power Manager (MPM) on the MSM8x60 family (MSM8260/MSM8660/
+  APQ8060) is an always-on hardware block that keeps a set of wake
+  interrupts alive while the application processor is powered down.
+  On MSM8x60 the vMPM registers live INSIDE the RPM control block
+  (request regs at RPM_BASE + 0x9d8, status at RPM_BASE + 0xdf8), with
+  notification of new wake-source configuration delivered via the
+  qcom-apcs-ipc mailbox (writing GCC + 0x008 bit 1).
+
+  This binding is MSM8x60-specific. Newer Qualcomm SoCs (sm6375, etc.)
+  use the qcom,mpm binding which assumes a dedicated MPM SRAM region
+  and an IPCC mailbox — neither holds on MSM8x60. The driver lives at
+  drivers/irqchip/irq-msm8660-mpm.c.
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    const: qcom,msm8660-mpm
+
+  interrupts:
+    maxItems: 1
+    description:
+      IPC IRQ raised by MPM when one of the enabled wake sources fires.
+      On MSM8x60 this is wired to GIC SPI 2.
+
+  qcom,rpm-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to the RPM control block exposed as a syscon. The vMPM
+      registers are accessed at offsets within this block.
+
+  qcom,mpm-offset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Offset of the vMPM register window within the RPM syscon. On
+      MSM8x60 this is 0x9d8. Status registers are at this offset + 0x420
+      (== 0xdf8 from RPM base).
+
+  mboxes:
+    maxItems: 1
+    description:
+      Mailbox channel used to notify MPM that the vMPM request
+      registers have been updated. On MSM8x60 this is the
+      qcom-apcs-ipc mailbox channel 1 (writes GCC + 0x008 bit 1).
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+    description:
+      First cell is the MPM pin number / parent GIC SPI; second cell
+      is the trigger type.
+
+  qcom,mpm-pin-count:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Total number of MPM pins exposed.
+
+  qcom,mpm-pin-map:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    items:
+      items:
+        - description: MPM pin number
+        - description: GIC SPI number this pin maps to
+    description: |
+      List of (MPM-pin, GIC-SPI) tuples for wake sources that have a
+      corresponding GIC IRQ. Consumers route their interrupts through
+      the MPM hierarchical irqdomain to take advantage of these
+      mappings.
+
+      Raw wake pins like SDC3/4 DATx (pins 21-24) are NOT listed
+      here — they have no GIC SPI mapping and are accessed through
+      the msm8660_mpm_set_pin_wake() / msm8660_mpm_enable_pin() C API
+      by consumer drivers.
+
+required:
+  - compatible
+  - interrupts
+  - qcom,rpm-syscon
+  - qcom,mpm-offset
+  - mboxes
+  - interrupt-controller
+  - '#interrupt-cells'
+  - qcom,mpm-pin-count
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        msm8660_mpm: interrupt-controller {
+            compatible = "qcom,msm8660-mpm";
+            qcom,rpm-syscon = <&rpm>;
+            qcom,mpm-offset = <0x9d8>;
+            interrupts = <GIC_SPI 2 IRQ_TYPE_EDGE_RISING>;
+            mboxes = <&gcc_ipc 1>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+            interrupt-parent = <&intc>;
+            qcom,mpm-pin-count = <64>;
+            qcom,mpm-pin-map = <25 100>,
+                               <26 50>,
+                               <27 79>;
+        };
+    };
Signature · 1 line
2.43.0
#3 lore raw

PATCH 1/2 dt-bindings: thermal: qcom: add pm8901-temp-alarm

In reply to Herman van Hazendonk

Add the binding for the temperature-alarm block inside the Qualcomm PM8901 PMIC (companion to the PM8058 on MSM8x60). The driver exposes the PM8901 die-temperature trip stages (105 / 125 / 145 C) to the thermal framework via a thermal-zone, with stage 3 wired as a critical trip so the kernel issues orderly_poweroff() when the part overheats.

The binding describes the SSBI sub-node address, the GIC interrupt the alarm raises on a stage transition, and the parent PMIC reference.

---
 .../thermal/qcom,pm8901-temp-alarm.yaml       | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml
Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml new file +79 -0
new file mode 100644
index 000000000000..569943b4aedc
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/qcom,pm8901-temp-alarm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm PM8901 PMIC Temperature Alarm
+
+maintainers:
+  - Herman van Hazendonk <[email protected]>
+
+description: |
+  PM8901 is a secondary PMIC paired with PM8058 on MSM8x60 family
+  (MSM8260/MSM8660/APQ8060) platforms. It exposes an over-temperature
+  alarm block at SSBI offset
+  0x23 (CTRL) / 0x24 (PWM) with four selectable thresholds and three
+  escalating stages. Unlike PM8058, there is no raw die-temperature
+  ADC channel — the driver decodes the stage + threshold pair into a
+  representative millicelsius value.
+
+  Two PMIC-internal interrupts are exposed: a stage-transition alarm
+  (TEMP_ALARM, PM8901 IRQ block 6 bit 4 == 52) and a hi-temp alarm
+  (TEMP_HI_ALARM, block 6 bit 5 == 53).
+
+  The driver registers a thermal-of sensor; board DTs declare trip
+  points and a critical-trip action against it.
+
+properties:
+  compatible:
+    const: qcom,pm8901-temp-alarm
+
+  reg:
+    description: SSBI offset of the temp-alarm CTRL register.
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: Stage-transition alarm interrupt (TEMP_ALARM).
+      - description: Hi-temperature alarm interrupt (TEMP_HI_ALARM).
+
+  interrupt-names:
+    items:
+      - const: alarm
+      - const: hi-alarm
+
+  "#thermal-sensor-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - "#thermal-sensor-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    ssbi {
+      pmic {
+        compatible = "qcom,pm8901";
+        #interrupt-cells = <2>;
+        interrupt-controller;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pm8901_temp: temp-alarm@23 {
+          compatible = "qcom,pm8901-temp-alarm";
+          reg = <0x23>;
+          interrupts = <52 IRQ_TYPE_EDGE_RISING>,
+                       <53 IRQ_TYPE_EDGE_RISING>;
+          interrupt-names = "alarm", "hi-alarm";
+          #thermal-sensor-cells = <0>;
+        };
+      };
+    };
Signature · 1 line
2.43.0
#24 lore raw

PATCH 2/2 thermal: qcom: add PM8901 PMIC temperature-alarm driver

In reply to Herman van Hazendonk

Add a thermal-of sensor driver for the temperature-alarm block inside the Qualcomm PM8901 PMIC. PM8901 is a secondary PMIC paired with PM8058 on the MSM8x60 family (MSM8260/MSM8660/APQ8060). It exposes an over-temperature alarm at SSBI offset 0x23/0x24 with three escalating stages (105/125/145 C); the driver decodes the stage + threshold pair into a millicelsius reading and registers two PMIC- internal interrupts (TEMP_ALARM at block 6 bit 4, TEMP_HI_ALARM at block 6 bit 5).

Used by board thermal-zones for the orderly_poweroff path on the HP TouchPad.

---
 drivers/thermal/qcom/Kconfig          |  12 +
 drivers/thermal/qcom/Makefile         |   1 +
 drivers/thermal/qcom/qcom-pm8901-tm.c | 341 ++++++++++++++++++++++++++
 3 files changed, 354 insertions(+)
 create mode 100644 drivers/thermal/qcom/qcom-pm8901-tm.c
drivers/thermal/qcom/Kconfig +12 -0
index a6bb01082ec6..af099032f1e6 100644
@@ -32,6 +32,18 @@ config QCOM_SPMI_TEMP_ALARM
 	  real time die temperature if an ADC is present or an estimate of the
 	  temperature based upon the over temperature stage value.
 
+config QCOM_PM8901_TEMP_ALARM
+	tristate "Qualcomm PM8901 PMIC Temperature Alarm"
+	depends on MFD_PM8XXX || COMPILE_TEST
+	depends on THERMAL_OF
+	help
+	  This enables the thermal driver for the PM8901 PMIC over-temperature
+	  alarm block. PM8901 exposes a stage-based alarm (no raw ADC) with
+	  four selectable thresholds and three escalating stages. The driver
+	  registers a thermal-of sensor so a board device tree can declare
+	  trip points and a critical-trip action (orderly_poweroff). Used on
+	  HP TouchPad (APQ8060) where PM8901 supplies the secondary PMIC die.
+
 config QCOM_LMH
 	tristate "Qualcomm Limits Management Hardware"
 	depends on ARCH_QCOM || COMPILE_TEST
drivers/thermal/qcom/Makefile +1 -0
index 0fa2512042e7..90dc05151e33 100644
@@ -5,4 +5,5 @@ qcom_tsens-y			+= tsens.o tsens-v2.o tsens-v1.o tsens-v0_1.o \
 				   tsens-8960.o
 obj-$(CONFIG_QCOM_SPMI_ADC_TM5)	+= qcom-spmi-adc-tm5.o
 obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
+obj-$(CONFIG_QCOM_PM8901_TEMP_ALARM)	+= qcom-pm8901-tm.o
 obj-$(CONFIG_QCOM_LMH)		+= lmh.o
drivers/thermal/qcom/qcom-pm8901-tm.c new file +341 -0

341 lines not on this page · raw message

Signature · 1 line
2.43.0
#11 lore raw

PATCH v2 1/3 dt-bindings: clock: qcom: add mmcc-msm8660 clock IDs

In reply to Herman van Hazendonk

Add the dt-binding clock-ID header for the MSM8x60 family (MSM8260/MSM8660/APQ8060) Multimedia Clock Controller (MMCC). The header enumerates the clocks and power-domains consumed by the multimedia subsystem (MDP4 display, Adreno A220 GPU, CAMSS image pipeline, VFE, Gemini JPEG, video codec, rotator, VPE and the GFX2D Z180 cores).

IDs intentionally match the numeric values used by the original shared mmcc-msm8960.h so the driver's clk array indexing is preserved; only the clocks actually implemented by mmcc-msm8660.c are defined.

---
 include/dt-bindings/clock/qcom,mmcc-msm8660.h | 126 ++++++++++++++++++
 1 file changed, 126 insertions(+)
 create mode 100644 include/dt-bindings/clock/qcom,mmcc-msm8660.h
include/dt-bindings/clock/qcom,mmcc-msm8660.h new file +126 -0
new file mode 100644
index 000000000000..00c3a75e8b71
@@ -0,0 +1,126 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * Clock and power-domain bindings for the MSM8x60 family (MSM8260/MSM8660/APQ8060)
+ * Multimedia Clock Controller (MMCC).
+ *
+ * MSM8260, MSM8660 and APQ8060 are the same Scorpion-class MSM8x60 SoC
+ * with different bin/feature labels. MSM8960 is a newer generation (Krait)
+ * — its bindings live in
+ * <dt-bindings/clock/qcom,mmcc-msm8960.h> and must not be reused here.
+ *
+ * IDs below intentionally match the numeric values used by the original
+ * shared mmcc-msm8960.h so the driver's clk array indexing is preserved;
+ * only the clocks actually implemented by mmcc-msm8660.c are defined.
+ */
+
+#ifndef _DT_BINDINGS_CLK_MSM_MMCC_8660_H
+#define _DT_BINDINGS_CLK_MSM_MMCC_8660_H
+
+#define TV_ENC_AHB_CLK					3
+#define AMP_AHB_CLK					4
+#define JPEGD_AHB_CLK					6
+#define GFX2D0_AHB_CLK					7
+#define DSI_S_AHB_CLK					8
+#define VPE_AHB_CLK					10
+#define SMMU_AHB_CLK					11
+#define HDMI_M_AHB_CLK					12
+#define VFE_AHB_CLK					13
+#define ROT_AHB_CLK					14
+#define VCODEC_AHB_CLK					15
+#define MDP_AHB_CLK					16
+#define DSI_M_AHB_CLK					17
+#define CSI0_AHB_CLK					18
+#define MMSS_IMEM_AHB_CLK				19
+#define IJPEG_AHB_CLK					20
+#define HDMI_S_AHB_CLK					21
+#define GFX3D_AHB_CLK					22
+#define GFX2D1_AHB_CLK					23
+#define JPEGD_AXI_CLK					28
+#define GMEM_AXI_CLK					29
+#define MDP_AXI_CLK					30
+#define MMSS_IMEM_AXI_CLK				31
+#define IJPEG_AXI_CLK					32
+#define GFX3D_AXI_CLK					33
+#define VCODEC_AXI_CLK					34
+#define VFE_AXI_CLK					35
+#define VPE_AXI_CLK					36
+#define ROT_AXI_CLK					37
+#define VCODEC_AXI_A_CLK				38
+#define VCODEC_AXI_B_CLK				39
+#define CSI0_SRC					47
+#define CSI0_CLK					48
+#define CSI0_PHY_CLK					49
+#define CSI1_SRC					50
+#define CSI1_CLK					51
+#define CSI1_PHY_CLK					52
+#define DSI_SRC						56
+#define DSI_CLK						57
+#define CSI_PIX_CLK					58
+#define CSI_RDI_CLK					59
+#define MDP_VSYNC_CLK					60
+#define HDMI_APP_CLK					62
+#define GFX2D0_SRC					66
+#define GFX2D0_CLK					67
+#define GFX2D1_SRC					68
+#define GFX2D1_CLK					69
+#define GFX3D_SRC					70
+#define GFX3D_CLK					71
+#define IJPEG_SRC					72
+#define IJPEG_CLK					73
+#define JPEGD_SRC					74
+#define JPEGD_CLK					75
+#define MDP_SRC						76
+#define MDP_CLK						77
+#define MDP_LUT_CLK					78
+#define DSI1_BYTE_SRC					83
+#define DSI1_BYTE_CLK					84
+#define DSI1_ESC_SRC					87
+#define DSI1_ESC_CLK					88
+#define ROT_SRC						91
+#define ROT_CLK						92
+#define TV_ENC_CLK					93
+#define TV_DAC_CLK					94
+#define HDMI_TV_CLK					95
+#define MDP_TV_CLK					96
+#define TV_SRC						97
+#define VCODEC_SRC					98
+#define VCODEC_CLK					99
+#define VFE_SRC						100
+#define VFE_CLK						101
+#define VFE_CSI0_CLK					102
+#define VPE_SRC						103
+#define VPE_CLK						104
+#define DSI_PIXEL_SRC					105
+#define DSI_PIXEL_CLK					106
+#define CAMCLK0_SRC					107
+#define CAMCLK0_CLK					108
+#define CAMCLK1_SRC					109
+#define CAMCLK1_CLK					110
+#define CSIPHYTIMER_SRC					113
+#define CSIPHY1_TIMER_CLK				115
+#define CSIPHY0_TIMER_CLK				116
+#define PLL2						118
+#define MDP_PIXEL_SRC					129
+#define MDP_PIXEL_CLK					130
+#define MDP_LCDC_CLK					131
+#define VFE_CSI1_CLK					132
+#define CSI1_AHB_CLK					133
+
+/*
+ * MSM8x60 legacy footswitch power domains.
+ * Used with the MMCC power-domain provider (#power-domain-cells = <1>).
+ * Numbering is independent of the clock ID space above.
+ */
+#define GFX2D0_GDSC					0
+#define GFX2D1_GDSC					1
+#define GFX3D_GDSC					2
+#define IJPEG_GDSC					3
+#define MDP_GDSC					4
+#define ROT_GDSC					5
+#define VED_GDSC					6
+#define VFE_GDSC					7
+#define VPE_GDSC					8
+
+#endif
Signature · 1 line
2.43.0
#16 lore raw

PATCH v2 3/3 clk: qcom: add MSM8x60 MMCC driver

In reply to Herman van Hazendonk

Add a clock driver for the Multimedia Clock Controller (MMCC) on the MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The MMCC layout on MSM8x60 differs from MSM8960 in several ways that make a separate driver cleaner than parameterising mmcc-msm8960.c:

- the pix_rdi mux requires a custom set_parent op that temporarily

    enables both parents during the glitch-free transition;
  - the IJPEG GDSC requires releasing AXI, AHB and CORE resets;
  - several rate-source pairs (MDP pixel, GFX2D/3D) only exist on
    8x60 (e.g. PLL2-derived 228571000/266667000 for graphics);

- the camera CSI / VFE / JPEG / VPE / ROT clock topology lacks the later 8960 reorganisation.

Used on the HP TouchPad (Tenderloin) for graphics (Adreno A220), display (MDP4), camera (CSI/VFE), JPEG (Gemini), VIDC, VPE and rotator. Reset IDs are exposed via a separate header so consumers can reset the GDSCs and individual blocks.

---
 drivers/clk/qcom/Kconfig        |   11 +
 drivers/clk/qcom/Makefile       |    1 +
 drivers/clk/qcom/mmcc-msm8660.c | 2998 +++++++++++++++++++++++++++++++
 3 files changed, 3010 insertions(+)
 create mode 100644 drivers/clk/qcom/mmcc-msm8660.c
drivers/clk/qcom/Kconfig +11 -0
index d9cff5b0281d..f64b0002c85a 100644
@@ -572,6 +572,17 @@ config MSM_MMCC_8960
 	  Say Y if you want to support multimedia devices such as display,
 	  graphics, video encode/decode, camera, etc.
 
+config MSM_MMCC_8660
+	tristate "MSM8x60 Multimedia Clock Controller"
+	depends on ARM || COMPILE_TEST
+	select MSM_GCC_8660
+	select QCOM_GDSC
+	help
+	  Support for the multimedia clock controller on the MSM8x60 family
+	  (MSM8260/MSM8660/APQ8060). Say Y if you want to support multimedia
+	  devices such as display, graphics, video encode/decode, camera, etc.
+	  on these Scorpion-class SoCs (e.g. HP TouchPad).
+
 config MSM_GCC_8953
 	tristate "MSM8953 Global Clock Controller"
 	select QCOM_GDSC
drivers/clk/qcom/Makefile +1 -0
index e100cfd6a52d..ff317fce086d 100644
@@ -81,6 +81,7 @@ obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o
 obj-$(CONFIG_MSM_GCC_8998) += gcc-msm8998.o
 obj-$(CONFIG_MSM_GPUCC_8998) += gpucc-msm8998.o
 obj-$(CONFIG_MSM_MMCC_8960) += mmcc-msm8960.o
+obj-$(CONFIG_MSM_MMCC_8660) += mmcc-msm8660.o
 obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o
 obj-$(CONFIG_MSM_MMCC_8994) += mmcc-msm8994.o
 obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o
drivers/clk/qcom/mmcc-msm8660.c new file +2998 -0

2,976 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 1 line
2.43.0

Part of the diff is not shown here. View the raw message.

#7 lore raw

PATCH 1/2 dt-bindings: clock: qcom: add lcc-msm8660 LPASS clock IDs

In reply to Herman van Hazendonk

Add the dt-binding clock-ID header for the MSM8x60 family (MSM8260/MSM8660/APQ8060) Low Power Audio SubSystem Clock Controller (LCC). The header enumerates the LPASS clocks consumed by the qcom,apq8060-lpaif sound card and the codec/AIF nodes downstream of it. It mirrors the format and ID range of the existing LCC headers for newer Qualcomm SoCs (lcc-msm8960, lcc-msm8974) so the drivers/clk/qcom/lcc-msm8660.c driver can be hooked up the same way once it lands.

---
 include/dt-bindings/clock/qcom,lcc-msm8660.h | 48 ++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 include/dt-bindings/clock/qcom,lcc-msm8660.h
include/dt-bindings/clock/qcom,lcc-msm8660.h new file +48 -0
new file mode 100644
index 000000000000..d5d9b0d71a78
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_LCC_MSM8660_H
+#define _DT_BINDINGS_CLK_LCC_MSM8660_H
+
+/*
+ * MSM8x60 family (MSM8260/MSM8660/APQ8060) LPASS Clock Controller (LCC)
+ * clock IDs. These are compatible with MSM8960 LCC as MSM8x60 and
+ * MSM8960 share the same audio subsystem clock architecture.
+ */
+
+#define PLL4				0
+#define MI2S_OSR_SRC			1
+#define MI2S_OSR_CLK			2
+#define MI2S_DIV_CLK			3
+#define MI2S_BIT_DIV_CLK		4
+#define MI2S_BIT_CLK			5
+#define PCM_SRC				6
+#define PCM_CLK_OUT			7
+#define PCM_CLK				8
+#define SLIMBUS_SRC			9
+#define AUDIO_SLIMBUS_CLK		10
+#define SPS_SLIMBUS_CLK			11
+#define CODEC_I2S_MIC_OSR_SRC		12
+#define CODEC_I2S_MIC_OSR_CLK		13
+#define CODEC_I2S_MIC_DIV_CLK		14
+#define CODEC_I2S_MIC_BIT_DIV_CLK	15
+#define CODEC_I2S_MIC_BIT_CLK		16
+#define SPARE_I2S_MIC_OSR_SRC		17
+#define SPARE_I2S_MIC_OSR_CLK		18
+#define SPARE_I2S_MIC_DIV_CLK		19
+#define SPARE_I2S_MIC_BIT_DIV_CLK	20
+#define SPARE_I2S_MIC_BIT_CLK		21
+#define CODEC_I2S_SPKR_OSR_SRC		22
+#define CODEC_I2S_SPKR_OSR_CLK		23
+#define CODEC_I2S_SPKR_DIV_CLK		24
+#define CODEC_I2S_SPKR_BIT_DIV_CLK	25
+#define CODEC_I2S_SPKR_BIT_CLK		26
+#define SPARE_I2S_SPKR_OSR_SRC		27
+#define SPARE_I2S_SPKR_OSR_CLK		28
+#define SPARE_I2S_SPKR_DIV_CLK		29
+#define SPARE_I2S_SPKR_BIT_DIV_CLK	30
+#define SPARE_I2S_SPKR_BIT_CLK		31
+
+#endif
Signature · 1 line
2.43.0
#13 lore raw

PATCH v2 2/3 dt-bindings: reset: qcom: add mmcc-msm8660 reset IDs

In reply to Herman van Hazendonk

Add the dt-binding reset-ID header for the MSM8x60 family (MSM8260/MSM8660/APQ8060) MMCC reset lines. The MMCC exposes per-IP software-reset bits used by MDP4 (display), CAMSS / VFE (camera), Gemini (JPEG), rotator, VPE and the GFX2D / GFX3D blocks; each gets a matching reset-controller ID here.

IDs match the numeric values used by the original shared mmcc-msm8960.h reset bindings; only the resets actually implemented by mmcc-msm8660.c are defined.

---
 include/dt-bindings/reset/qcom,mmcc-msm8660.h | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 include/dt-bindings/reset/qcom,mmcc-msm8660.h
include/dt-bindings/reset/qcom,mmcc-msm8660.h new file +88 -0
new file mode 100644
index 000000000000..c3ffd57834c9
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * Reset bindings for the MSM8x60 family (MSM8260/MSM8660/APQ8060) Multimedia Clock
+ * Controller (MMCC).
+ *
+ * MSM8260, MSM8660 and APQ8060 are the same Scorpion-class MSM8x60 SoC
+ * with different bin/feature labels. MSM8960 is a newer generation (Krait)
+ * — its reset bindings live in
+ * <dt-bindings/reset/qcom,mmcc-msm8960.h> and must not be reused here.
+ *
+ * IDs intentionally match the numeric values used by the original shared
+ * mmcc-msm8960.h so the driver's qcom_reset_map array indexing is preserved;
+ * only the resets actually implemented by mmcc-msm8660.c are defined.
+ */
+
+#ifndef _DT_BINDINGS_RESET_MSM_MMCC_8660_H
+#define _DT_BINDINGS_RESET_MSM_MMCC_8660_H
+
+#define VPE_AXI_RESET					0
+#define IJPEG_AXI_RESET					1
+#define MPD_AXI_RESET					2
+#define VFE_AXI_RESET					3
+#define SP_AXI_RESET					4
+#define VCODEC_AXI_RESET				5
+#define ROT_AXI_RESET					6
+#define VCODEC_AXI_A_RESET				7
+#define VCODEC_AXI_B_RESET				8
+#define FAB_S3_AXI_RESET				9
+#define FAB_S2_AXI_RESET				10
+#define FAB_S1_AXI_RESET				11
+#define FAB_S0_AXI_RESET				12
+#define SMMU_GFX3D_ABH_RESET				13
+#define SMMU_VPE_AHB_RESET				14
+#define SMMU_VFE_AHB_RESET				15
+#define SMMU_ROT_AHB_RESET				16
+#define SMMU_VCODEC_B_AHB_RESET				17
+#define SMMU_VCODEC_A_AHB_RESET				18
+#define SMMU_MDP1_AHB_RESET				19
+#define SMMU_MDP0_AHB_RESET				20
+#define SMMU_JPEGD_AHB_RESET				21
+#define SMMU_IJPEG_AHB_RESET				22
+#define APU_AHB_RESET					25
+#define CSI_AHB_RESET					26
+#define TV_ENC_AHB_RESET				27
+#define VPE_AHB_RESET					28
+#define FABRIC_AHB_RESET				29
+#define GFX2D0_AHB_RESET				30
+#define GFX2D1_AHB_RESET				31
+#define GFX3D_AHB_RESET					32
+#define HDMI_AHB_RESET					33
+#define MSSS_IMEM_AHB_RESET				34
+#define IJPEG_AHB_RESET					35
+#define DSI_M_AHB_RESET					36
+#define DSI_S_AHB_RESET					37
+#define JPEGD_AHB_RESET					38
+#define MDP_AHB_RESET					39
+#define ROT_AHB_RESET					40
+#define VCODEC_AHB_RESET				41
+#define VFE_AHB_RESET					42
+#define CSIPHY0_RESET					47
+#define CSIPHY1_RESET					48
+#define VFE_CSI_RESET					50
+#define MDP_RESET					51
+#define AMP_RESET					52
+#define JPEGD_RESET					53
+#define CSI1_RESET					54
+#define VPE_RESET					55
+#define MMSS_FABRIC_RESET				56
+#define VFE_RESET					57
+#define GFX3D_RESET					60
+#define HDMI_RESET					61
+#define MMSS_IMEM_RESET					62
+#define IJPEG_RESET					63
+#define CSI0_RESET					64
+#define DSI_RESET					65
+#define VCODEC_RESET					66
+#define MDP_TV_RESET					67
+#define MDP_VSYNC_RESET					68
+#define ROT_RESET					69
+#define TV_HDMI_RESET					70
+#define TV_ENC_RESET					71
+#define GFX3D_AXI_RESET					75
+#define CSI_RDI_RESET					79
+#define CSI_PIX_RESET					80
+
+#endif
Signature · 1 line
2.43.0
#10 lore raw

PATCH 1/2 dt-bindings: interconnect: qcom: add msm8660 fabric IDs

In reply to Herman van Hazendonk

Add the dt-binding interconnect master/slave ID header for the MSM8x60 family (MSM8260/MSM8660/APQ8060) fabric mesh. The chip's NoC fabric is split into multiple sub-fabrics that the qnoc-msm8660 driver models:

AFAB - Applications fabric (Scorpion CPU + L2) SFAB - System fabric (DMA, SPS, security)
MMFAB - Multimedia fabric (MDP, GPU, camera, video, rotator)

  DFAB  - Daytona fabric (SDC, ADM master/slave)
  SFPB  - System Fast Peripheral Bridge (RPM, MPM, PMIC SSBI)
  CFPB  - CPU Subsystem Fast Peripheral Bus (GSBI UART/QUP, USB FS,

TSIF, TSSC, PDM, PRNG)

IDs derived from the legacy webOS msm_bus_board_8660.c master/slave enums, normalised to the upstream interconnect-framework naming.

---
 .../dt-bindings/interconnect/qcom,msm8660.h   | 156 ++++++++++++++++++
 1 file changed, 156 insertions(+)
 create mode 100644 include/dt-bindings/interconnect/qcom,msm8660.h
include/dt-bindings/interconnect/qcom,msm8660.h new file +156 -0
new file mode 100644
index 000000000000..d2639f737a54
@@ -0,0 +1,156 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Qualcomm MSM8x60 family (MSM8260/MSM8660/APQ8060) interconnect IDs
+ *
+ * Copyright (c) 2026, Herman van Hazendonk <[email protected]>
+ *
+ * Based on webOS kernel msm_bus_board_8660.c
+ * Copyright (c) 2010-2011, Code Aurora Forum. All rights reserved.
+ */
+
+#ifndef __DT_BINDINGS_INTERCONNECT_QCOM_MSM8660_H
+#define __DT_BINDINGS_INTERCONNECT_QCOM_MSM8660_H
+
+/*
+ * MSM8x60 has a fabric-based bus architecture:
+ * - APPSS Fabric: CPU and memory interface
+ * - System Fabric: System peripherals and DMA
+ * - MMSS Fabric: Multimedia subsystem (display, camera, video)
+ * - Daytona Fabric: Peripheral bus (SDCC, ADM DMA)
+ * - System FPB: System Fast Peripheral Bus
+ * - CPSS FPB: CPU Subsystem Fast Peripheral Bus
+ */
+
+/* APPSS Fabric - Apps processor fabric */
+#define AFAB_MAS_AMPSS_M0		0
+#define AFAB_MAS_AMPSS_M1		1
+#define AFAB_SLV_EBI_CH0		2
+#define AFAB_SLV_AMPSS_L2		3
+#define AFAB_TO_MMSS			4
+#define AFAB_TO_SYSTEM			5
+
+/* System Fabric - System bus */
+#define SFAB_MAS_APPSS			0
+#define SFAB_MAS_SPS			1
+#define SFAB_MAS_ADM0_PORT0		2
+#define SFAB_MAS_ADM0_PORT1		3
+#define SFAB_MAS_ADM1_PORT0		4
+#define SFAB_MAS_ADM1_PORT1		5
+#define SFAB_MAS_LPASS_PROC		6
+#define SFAB_MAS_MSS_PROCI		7
+#define SFAB_MAS_MSS_PROCD		8
+#define SFAB_MAS_MSS_MDM_PORT0		9
+#define SFAB_MAS_LPASS			10
+#define SFAB_MAS_MMSS_FPB		11
+#define SFAB_MAS_ADM1_CI		12
+#define SFAB_MAS_ADM0_CI		13
+#define SFAB_MAS_MSS_MDM_PORT1		14
+#define SFAB_MAS_USB_HS			15
+#define SFAB_TO_APPSS			16
+#define SFAB_TO_SYSTEM_FPB		17
+#define SFAB_TO_CPSS_FPB		18
+#define SFAB_SLV_SPS			19
+#define SFAB_SLV_SYSTEM_IMEM		20
+#define SFAB_SLV_AMPSS			21
+#define SFAB_SLV_MSS			22
+#define SFAB_SLV_LPASS			23
+#define SFAB_SLV_MMSS_FPB		24
+#define SFAB_TO_DFAB			25
+
+/* MMSS Fabric - Multimedia subsystem */
+#define MMFAB_MAS_MDP_PORT0		0
+#define MMFAB_MAS_MDP_PORT1		1
+#define MMFAB_MAS_ADM1_PORT0		2
+#define MMFAB_MAS_ROTATOR		3
+#define MMFAB_MAS_GRAPHICS_3D		4
+#define MMFAB_MAS_JPEG_DEC		5
+#define MMFAB_MAS_GRAPHICS_2D_CORE0	6
+#define MMFAB_MAS_VFE			7
+#define MMFAB_MAS_VPE			8
+#define MMFAB_MAS_JPEG_ENC		9
+#define MMFAB_MAS_GRAPHICS_2D_CORE1	10
+#define MMFAB_MAS_HD_CODEC_PORT0	11
+#define MMFAB_MAS_HD_CODEC_PORT1	12
+#define MMFAB_TO_APPSS			13
+#define MMFAB_SLV_SMI			14
+#define MMFAB_SLV_MM_IMEM		15
+
+/*
+ * Daytona Fabric (DFAB) - Peripheral bus
+ *
+ * DFAB connects slower peripherals (SDCC, ADM DMA) to the system fabric.
+ * The webOS kernel managed DFAB bandwidth via voter clocks (dfab_sdc*_clk,
+ * dfab_usb_hs_clk). In mainline, this is handled by the interconnect framework.
+ *
+ * USB HS is included as a DFAB voter for compatibility with the legacy clock
+ * voting mechanism. The webOS kernel comment said: "if usb link is in sps
+ * there is no need for usb pclk as daytona fabric clock will be used instead".
+ * This keeps DFAB clock stable when USB is active.
+ */
+#define DFAB_MAS_SDC1			0
+#define DFAB_MAS_SDC2			1
+#define DFAB_MAS_SDC3			2
+#define DFAB_MAS_SDC4			3
+#define DFAB_MAS_SDC5			4
+#define DFAB_MAS_ADM0_MASTER		5
+#define DFAB_MAS_ADM1_MASTER		6
+#define DFAB_TO_SFAB			7
+#define DFAB_SLV_SDC1			8
+#define DFAB_SLV_SDC2			9
+#define DFAB_SLV_SDC3			10
+#define DFAB_SLV_SDC4			11
+#define DFAB_SLV_SDC5			12
+#define DFAB_MAS_USB_HS			13
+#define DFAB_MAS_DSPS			14
+
+/* System FPB - Slow peripheral bus for system */
+#define SFPB_MAS_SYSTEM			0
+#define SFPB_MAS_SPDM			1
+#define SFPB_MAS_RPM			2
+#define SFPB_SLV_SPDM			3
+#define SFPB_SLV_RPM			4
+#define SFPB_SLV_RPM_MSG_RAM		5
+#define SFPB_SLV_MPM			6
+#define SFPB_SLV_PMIC1_SSBI1_A		7
+#define SFPB_SLV_PMIC1_SSBI1_B		8
+#define SFPB_SLV_PMIC1_SSBI1_C		9
+#define SFPB_SLV_PMIC2_SSBI2_A		10
+#define SFPB_SLV_PMIC2_SSBI2_B		11
+
+/* CPSS FPB - CPU subsystem fast peripheral bus */
+#define CFPB_MAS_SYSTEM			0
+#define CFPB_SLV_GSBI1_UART		1
+#define CFPB_SLV_GSBI2_UART		2
+#define CFPB_SLV_GSBI3_UART		3
+#define CFPB_SLV_GSBI4_UART		4
+#define CFPB_SLV_GSBI5_UART		5
+#define CFPB_SLV_GSBI6_UART		6
+#define CFPB_SLV_GSBI7_UART		7
+#define CFPB_SLV_GSBI8_UART		8
+#define CFPB_SLV_GSBI9_UART		9
+#define CFPB_SLV_GSBI10_UART		10
+#define CFPB_SLV_GSBI11_UART		11
+#define CFPB_SLV_GSBI12_UART		12
+#define CFPB_SLV_GSBI1_QUP		13
+#define CFPB_SLV_GSBI2_QUP		14
+#define CFPB_SLV_GSBI3_QUP		15
+#define CFPB_SLV_GSBI4_QUP		16
+#define CFPB_SLV_GSBI5_QUP		17
+#define CFPB_SLV_GSBI6_QUP		18
+#define CFPB_SLV_GSBI7_QUP		19
+#define CFPB_SLV_GSBI8_QUP		20
+#define CFPB_SLV_GSBI9_QUP		21
+#define CFPB_SLV_GSBI10_QUP		22
+#define CFPB_SLV_GSBI11_QUP		23
+#define CFPB_SLV_GSBI12_QUP		24
+#define CFPB_SLV_EBI2_NAND		25
+#define CFPB_SLV_USB_FS1		26
+#define CFPB_SLV_USB_FS2		27
+#define CFPB_SLV_TSIF			28
+#define CFPB_SLV_MSM_TSSC		29
+#define CFPB_SLV_MSM_PDM		30
+#define CFPB_SLV_MSM_DIMEM		31
+#define CFPB_SLV_MSM_TCSR		32
+#define CFPB_SLV_MSM_PRNG		33
+
+#endif /* __DT_BINDINGS_INTERCONNECT_QCOM_MSM8660_H */
Signature · 1 line
2.43.0
#4 lore raw

Re: [PATCH 1/2] dt-bindings: thermal: qcom: add pm8901-temp-alarm

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Medium] The binding introduces a child node without updating the parent MFD schema to allow it, and the…

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The binding introduces a child node without updating the parent MFD schema to allow it, and the provided device tree example violates parent requirements.
- [Medium] Meaningful mismatch between the commit message and the actual binding implementation regarding interrupts and parent references.
- [Low] The binding defines a thermal sensor provider but fails to reference the core thermal-sensor.yaml schema.
--

commit 547082ca486fab0b441c0b967105187e700c4d9c
Author: Herman van Hazendonk <[email protected]>

dt-bindings: thermal: qcom: add pm8901-temp-alarm

This commit adds a device tree binding for the temperature-alarm block inside the Qualcomm PM8901 PMIC. The driver exposes the PM8901 die-temperature trip stages to the thermal framework via a thermal-zone.

Quoted 3 lines

The binding describes the SSBI sub-node address, the GIC interrupt the alarm raises on a stage transition, and the parent PMIC reference.

[Severity: Medium]
Does the commit message match the binding implementation? The message states it describes the GIC interrupt and parent PMIC reference, but the schema actually defines PMIC-internal interrupts (indices 52 and 53), and relies purely on the standard device tree parent-child hierarchy rather than a parent PMIC reference property.

Quoted 5 lines
Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml new file +0 -0
new file mode 100644
index 0000000000000..569943b4aedc5

[ ... ]

Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml line 30

Quoted 3 lines
Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml new file +3 -0
+properties:
+  compatible:
+    const: qcom,pm8901-temp-alarm

[Severity: Low]
Should this binding include an allOf block containing a $ref to /schemas/thermal/thermal-sensor.yaml#?

Standard device tree bindings for thermal sensors usually include this reference to inherit core schema definitions. By omitting it and defining
additionalProperties: false, the schema will reject future common thermal sensor properties.

[ ... ]

Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml line 70

Quoted 13 lines
Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml new file +13 -0
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    ssbi {
+      pmic {
+        compatible = "qcom,pm8901";
+        #interrupt-cells = <2>;
+        interrupt-controller;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pm8901_temp: temp-alarm@23 {

[Severity: Medium]
Will this pass dtbs_check and dt_binding_check?

The qcom,pm8901 parent schema in qcom-pm8xxx.yaml strictly prohibits undocumented child nodes via additionalProperties: false and requires an interrupts property on the pmic node.

It seems the qcom,pm8901-temp-alarm node was added without adding a corresponding temp-alarm@[0-9a-f]+$ pattern to the parent schema. Additionally, the pmic node in this example lacks the mandatory interrupts property.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#27 lore raw

PATCH 2/2 clk: qcom: add MSM8x60 LCC (LPASS) driver

In reply to Herman van Hazendonk

Add a clock driver for the LPASS (Low Power Audio SubSystem) Clock Controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The register layout, parent muxing and divider topology of the LPASS PLL/clk fabric differ from MSM8960's LCC enough that a clean separate driver is simpler than parameterising mmcc-msm8960.c. Both drivers can coexist in tree (different KConfig options, different match table, different device-tree compatible).

Used on the HP TouchPad (Tenderloin) where the LPASS Q6 audio DSP needs functional MI2S / SLIMBus / PCM clocks before audio playback or capture works.

---
 drivers/clk/qcom/Kconfig       |   9 +
 drivers/clk/qcom/Makefile      |   1 +
 drivers/clk/qcom/lcc-msm8660.c | 517 +++++++++++++++++++++++++++++++++
 3 files changed, 527 insertions(+)
 create mode 100644 drivers/clk/qcom/lcc-msm8660.c
drivers/clk/qcom/Kconfig +9 -0
index d9cff5b0281d..2ea95f69355e 100644
@@ -547,6 +547,15 @@ config MSM_LCC_8960
 	  Say Y if you want to use audio devices such as i2s, pcm,
 	  SLIMBus, etc.
 
+config MSM_LCC_8660
+	tristate "MSM8x60 LPASS Clock Controller"
+	depends on ARM || COMPILE_TEST
+	help
+	  Support for the LPASS clock controller on the MSM8x60 family
+	  (MSM8260/MSM8660/APQ8060). MSM8960 is the newer
+	  Krait-based generation handled separately by MSM_LCC_8960.
+	  Say Y if you want to use audio devices such as i2s, pcm, SLIMBus.
+
 config MDM_GCC_9607
 	tristate "MDM9607 Global Clock Controller"
 	depends on ARM || COMPILE_TEST
drivers/clk/qcom/Makefile +1 -0
index e100cfd6a52d..41c973d7db59 100644
@@ -77,6 +77,7 @@ obj-$(CONFIG_MSM_GCC_8974) += gcc-msm8974.o
 obj-$(CONFIG_MSM_GCC_8976) += gcc-msm8976.o
 obj-$(CONFIG_MSM_GCC_8994) += gcc-msm8994.o
 obj-$(CONFIG_MSM_GCC_8996) += gcc-msm8996.o
+obj-$(CONFIG_MSM_LCC_8660) += lcc-msm8660.o
 obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o
 obj-$(CONFIG_MSM_GCC_8998) += gcc-msm8998.o
 obj-$(CONFIG_MSM_GPUCC_8998) += gpucc-msm8998.o
drivers/clk/qcom/lcc-msm8660.c new file +517 -0

517 lines not on this page · raw message

Signature · 1 line
2.43.0
#30 lore raw

PATCH 2/2 irqchip: add MSM8x60 MPM wakeup interrupt controller driver

In reply to Herman van Hazendonk

Add a driver for the MSM Power Manager (MPM) on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The MPM is a small wake-source controller implemented in the always-on power domain that latches edge-triggered interrupts during APPS power collapse and signals the RPM to wake the APPS back up.

The driver:
- implements an irqchip that wraps a parent GIC SPI line and

    overrides ->irq_set_wake() to enable the MPM mirror of the same
    interrupt;

- exposes msm8660_mpm_set_pin_wake() etc. for consumers (e.g. mmci's SDC4 SDIO wake) that do not flow through irq_set_wake() directly;
- communicates with the RPM via SMSM/syscon to apply the wake enable/clear bits and request a power collapse vote.

Used on the HP TouchPad (Tenderloin) for SDIO-attached Wi-Fi and panel touch wake-from-suspend.

---
 drivers/irqchip/Kconfig           |  23 ++
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-msm8660-mpm.c | 627 ++++++++++++++++++++++++++++++
 include/soc/qcom/msm8660-mpm.h    |  80 ++++
 4 files changed, 731 insertions(+)
 create mode 100644 drivers/irqchip/irq-msm8660-mpm.c
 create mode 100644 include/soc/qcom/msm8660-mpm.h
drivers/irqchip/Kconfig +23 -0
index e755a2a05209..35598a56ac79 100644
@@ -525,6 +525,29 @@ config QCOM_MPM
 	  MSM Power Manager driver to manage and configure wakeup
 	  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
 
+config QCOM_MSM8660_MPM
+	bool "MSM8x60 MPM wakeup interrupt controller"
+	depends on ARCH_QCOM
+	depends on MFD_SYSCON
+	depends on MAILBOX
+	select IRQ_DOMAIN_HIERARCHY
+	default y
+	help
+	  Platform driver for the MSM Power Manager (MPM) wakeup interrupt
+	  controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The vMPM registers live inside
+	  the RPM control block, which makes the generic QCOM_MPM driver
+	  unusable here (it assumes a dedicated MPM SRAM region, an IPCC
+	  mailbox, and uses IRQCHIP_DECLARE early-init that races platform
+	  device creation).
+
+	  This driver replicates the legacy 2.6.35-palm arch/arm/mach-msm/
+	  mpm.c mechanism as a regular platform driver, accessing the vMPM
+	  registers via a syscon phandle to the RPM block and signaling the
+	  MPM via the qcom-apcs-ipc mailbox (writing to GCC + 0x008 bit 1).
+
+	  Required for cpuidle deep-sleep states (SPC/PC) and for
+	  WoWLAN-style wake-from-suspend on MSM8x60 SoCs.
+
 config CSKY_MPINTC
 	bool
 	depends on CSKY
drivers/irqchip/Makefile +1 -0
index 26aa3b6ec99f..3e64591f0f5b 100644
@@ -100,6 +100,7 @@ obj-$(CONFIG_MESON_IRQ_GPIO)		+= irq-meson-gpio.o
 obj-$(CONFIG_GOLDFISH_PIC) 		+= irq-goldfish-pic.o
 obj-$(CONFIG_QCOM_PDC)			+= qcom-pdc.o
 obj-$(CONFIG_QCOM_MPM)			+= irq-qcom-mpm.o
+obj-$(CONFIG_QCOM_MSM8660_MPM)		+= irq-msm8660-mpm.o
 obj-$(CONFIG_CSKY_MPINTC)		+= irq-csky-mpintc.o
 obj-$(CONFIG_CSKY_APB_INTC)		+= irq-csky-apb-intc.o
 obj-$(CONFIG_RISCV_INTC)		+= irq-riscv-intc.o
drivers/irqchip/irq-msm8660-mpm.c new file +627 -0

627 lines not on this page · raw message

include/soc/qcom/msm8660-mpm.h new file +80 -0

80 lines not on this page · raw message

Signature · 1 line
2.43.0
#15 lore raw

Re: [PATCH 2/2] interconnect: qcom: add MSM8x60 NoC driver

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider: - [Critical] Compilation failure due to undeclared, unprefixed identifiers in node array initializers.…

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] Compilation failure due to undeclared, unprefixed identifiers in node array initializers.
- [High] Clock rate calculation uses per-node buswidth and caches globally-shared clock rate into node-local state, causing incorrect clock frequencies.
- [High] Use-After-Free of RPM device data due to prematurely dropping device reference without establishing a device link.
- [High] Missing handling of -EPROBE_DEFER permanently cripples bandwidth scaling on the platform.
- [High] Arbitration collision due to mmfab_to_appss incorrectly claiming master port 2.
- [High] Bandwidth calculation divides bytes by 8, resulting in severe bus starvation.
--

commit dbe665a3a4a2b6e23a2d9c951ce890ab7021e52a
Author: Herman van Hazendonk <[email protected]>
Subject: interconnect: qcom: add MSM8x60 NoC driver

This commit adds a Qualcomm interconnect driver for the MSM8x60 family SoCs. It models the four fabrics and implements the interconnect-provider API to allow consumer drivers to request bandwidth via icc_set_bw(). The driver translates these requests to RPM firmware arbitration votes.

Quoted 5 lines
drivers/interconnect/qcom/msm8660.c new file +0 -0
new file mode 100644
@@ -0,0 +1,1008 @@

[ ... ]

drivers/interconnect/qcom/msm8660.c line 320

Quoted 2 lines
drivers/interconnect/qcom/msm8660.c new file +2 -0
+static struct msm8660_icc_node * const msm8660_afab_nodes[] = {
+	[AFAB_MAS_AMPSS_M0] = &mas_ampss_m0,

[Severity: Critical]
Does this cause a compilation failure?

The internal node ID enum defines values with the MSM8660_ prefix, such as MSM8660_AFAB_MAS_AMPSS_M0. Using the unprefixed version AFAB_MAS_AMPSS_M0 here in the array initializer appears to reference an undeclared identifier.

[ ... ]

Quoted 4 lines
drivers/interconnect/qcom/msm8660.c new file +4 -0
+/*
+ * Gateway from APPSS into MMSS: slave (port 1) for outbound traffic
+ * leaving MMSS, AND master (port 2) for inbound traffic arriving from
+ * APPSS (e.g. CPU memremap_wc accesses to SMI BOs). Without the master

drivers/interconnect/qcom/msm8660.c line 499

Quoted 10 lines
drivers/interconnect/qcom/msm8660.c new file +10 -0
+ * port and the forward links into MMFAB slaves (SMI / MM_IMEM), the
+ * ICC path-finder has no route from AMPSS_M0 to MMFAB_SLV_SMI; the
+ * cross-fabric gateway only worked for outbound traffic (MMSS masters
+ * reaching APPSS slaves like EBI).
+ *
+ * ARB_TIER1 keeps AMPSS->SMI traffic high-priority within MMFAB so
+ * CPU mmap reads/writes to SMI BOs don't get starved by MDP scanout.
+ */
+DEFINE_QNODE(mmfab_to_appss, MSM8660_MMFAB_TO_APPSS, 8, 2, 1, ARB_TIER1,
+	     MSM8660_AFAB_TO_MMSS,

[Severity: High]
Does mmfab_to_appss incorrectly claim master port 2 here?

The block comment for the MMSS Fabric nodes mentions that the APPS_FAB gateway should be on master port 11, while port 2 is already correctly assigned to ADM1.

When the arbitration arrays are packed, will this duplicate assignment overwrite the arbitration vote for port 2 and completely deprive ADM DMA of its bandwidth scaling on the MMSS fabric?

[ ... ]

Quoted 2 lines
drivers/interconnect/qcom/msm8660.c new file +2 -0
+static void msm8660_rpm_commit(struct msm8660_icc_provider *qp)
+{

[ ... ]

drivers/interconnect/qcom/msm8660.c line 676

Quoted 9 lines
drivers/interconnect/qcom/msm8660.c new file +9 -0
+	list_for_each_entry(n, &provider->nodes, node_list) {
+		struct msm8660_icc_node *qn = n->data;
+		u64 bw_bytes;
+		u16 bw_128k;
+
+		/* Use max of avg and peak bandwidth, convert to 128KB units */
+		bw_bytes = max(icc_units_to_bps(n->avg_bw),
+			       icc_units_to_bps(n->peak_bw));
+		do_div(bw_bytes, 8);	/* bits -> bytes */

[Severity: High]
Does icc_units_to_bps() return bandwidth in bytes per second rather than bits per second?

If bw_bytes is already in bytes, dividing it by 8 here would cause the driver to request only 1/8th of the necessary bandwidth from the firmware, which might lead to severe bus starvation.

[ ... ]

Quoted 3 lines
drivers/interconnect/qcom/msm8660.c new file +3 -0
+static int msm8660_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+	struct msm8660_icc_node *src_qn;

[ ... ]

drivers/interconnect/qcom/msm8660.c line 735

Quoted 12 lines
drivers/interconnect/qcom/msm8660.c new file +12 -0
+	sum_bw = icc_units_to_bps(agg_avg);
+	max_peak_bw = icc_units_to_bps(agg_peak);
+
+	rate = max(sum_bw, max_peak_bw);
+	do_div(rate, src_qn->buswidth);
+	/* Apply minimum floor to prevent bus starvation */
+	rate = max_t(u64, rate, MSM8660_FABRIC_MIN_RATE);
+	rate = min_t(u32, rate, INT_MAX);
+
+	if (src_qn->rate != rate) {
+		for (i = 0; i < qp->num_clks; i++) {
+			ret = clk_set_rate(qp->bus_clks[i].clk, rate);

[Severity: High]
Could this rate calculation and caching lead to incorrect hardware clock frequencies?

The code divides the globally aggregated fabric bandwidth by the local bus width of the specific node that triggered the update (src_qn->buswidth). Since nodes can have different bus widths, will this cause the calculated clock rate to oscillate depending on which node requests an update?

Additionally, the globally calculated rate is cached in the local node's state (src_qn->rate). If another node changes the shared hardware clock rate in the interim, a subsequent update from this node might skip the clk_set_rate() call entirely because its local cache matches the new rate, leaving the hardware stuck at the wrong frequency.

[ ... ]

Quoted 18 lines
drivers/interconnect/qcom/msm8660.c new file +18 -0
+static struct qcom_rpm *msm8660_get_rpm(struct device *dev)
+{
+	struct device_node *rpm_np;
+	struct platform_device *rpm_pdev;
+	struct qcom_rpm *rpm;
+
+	rpm_np = of_parse_phandle(dev->of_node, "qcom,rpm", 0);
+	if (!rpm_np) {
+		dev_dbg(dev, "no qcom,rpm phandle, RPM ARB disabled\n");
+		return NULL;
+	}
+
+	rpm_pdev = of_find_device_by_node(rpm_np);
+	of_node_put(rpm_np);
+	if (!rpm_pdev) {
+		dev_warn(dev, "RPM device not found, ARB disabled\n");
+		return NULL;
+	}

drivers/interconnect/qcom/msm8660.c line 787

Quoted 10 lines
drivers/interconnect/qcom/msm8660.c new file +10 -0
+
+	rpm = dev_get_drvdata(&rpm_pdev->dev);
+	put_device(&rpm_pdev->dev);
+	if (!rpm) {
+		dev_warn(dev, "RPM not ready, ARB disabled\n");
+		return NULL;
+	}
+
+	return rpm;
+}

[Severity: High]
Is it safe to drop the device reference with put_device() while keeping a pointer to the devres-managed qcom_rpm structure?

Without establishing a device link, the RPM device could be unbound at any time. If that happens, the qcom_rpm memory would be freed, leaving a dangling pointer in qp->rpm. A subsequent bandwidth update would then pass this freed pointer to qcom_rpm_write() in msm8660_rpm_commit(), causing a use-after-free.

Also, if the RPM device exists but hasn't probed yet, dev_get_drvdata() returns NULL. Should this function return -EPROBE_DEFER instead of NULL? Since the interconnect driver registers at core_initcall, returning NULL swallows the deferral and permanently disables RPM arbitration.

[ ... ]

Quoted 2 lines
drivers/interconnect/qcom/msm8660.c new file +2 -0
+static int msm8660_icc_probe(struct platform_device *pdev)
+{

[ ... ]

drivers/interconnect/qcom/msm8660.c line 834

Quoted 9 lines
drivers/interconnect/qcom/msm8660.c new file +9 -0
+	/*
+	 * MSM8660 fabric clocks are managed by RPM firmware and may not be
+	 * available in mainline Linux yet. Make them optional.
+	 */
+	ret = devm_clk_bulk_get_optional(dev, qp->num_clks, qp->bus_clks);
+	if (ret) {
+		dev_warn(dev, "Failed to get bus clocks: %d (continuing anyway)\n", ret);
+		qp->num_clks = 0;
+	}

[Severity: High]
Should this code propagate -EPROBE_DEFER when the clocks are not yet available?

Because this driver registers at core_initcall, it is very likely to probe before the clock provider. Catching all errors and continuing with qp->num_clks = 0 means the driver will silently discard -EPROBE_DEFER, leaving the platform permanently without clock scaling.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#8 lore raw

Re: [PATCH 1/2] dt-bindings: clock: qcom: add lcc-msm8660 LPASS clock IDs

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The commit introduces a new dt-bindings header for the MSM8x60 LPASS Clock Controller but entirely…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The commit introduces a new dt-bindings header for the MSM8x60 LPASS Clock Controller but entirely omits the required update to the corresponding YAML schema (Documentation/devicetree/bindings/clock/qcom,lcc.yaml). --

commit 2ab7502c050fb596d568ab63294755ed48d68189
Author: Herman van Hazendonk <[email protected]>

dt-bindings: clock: qcom: add lcc-msm8660 LPASS clock IDs

This commit introduces the device tree binding clock-ID header for the MSM8x60 family Low Power Audio SubSystem Clock Controller. The header enumerates the LPASS clocks to prepare for the new driver.

include/dt-bindings/clock/qcom,lcc-msm8660.h

Quoted 5 lines
include/dt-bindings/clock/qcom,lcc-msm8660.h new file +0 -0
new file mode 100644
index 0000000000000..d5d9b0d71a78f

[Severity: Low]
Is there a missing update to the corresponding YAML binding schema?

Documentation/devicetree/bindings/clock/qcom,lcc.yaml currently lists compatible strings like:

properties:
compatible:
enum:
- qcom,lcc-apq8064
- qcom,lcc-ipq8064
- qcom,lcc-mdm9615
- qcom,lcc-msm8960

Should qcom,lcc-msm8660 be added to this enum and any corresponding if-then conditional blocks to allow device tree nodes using this compatible string to pass validation?

Quoted 8 lines
include/dt-bindings/clock/qcom,lcc-msm8660.h new file +7 -0
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_LCC_MSM8660_H
+#define _DT_BINDINGS_CLK_LCC_MSM8660_H

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#25 lore raw

Re: [PATCH 2/2] thermal: qcom: add PM8901 PMIC temperature-alarm driver

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Hardware auto-shutdown override is left enabled if probe fails, permanently disabling PMIC thermal…

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Hardware auto-shutdown override is left enabled if probe fails, permanently disabling PMIC thermal protection.
- [Low] The calculation for the initial uninitialized temperature computes the lower bound of the stage, contradicting the comment that claims it picks the midpoint.
- [Low] Lockless read of shared state variables in dev_info during probe.
--

commit 4441da30420e5466a8731b177283d08a1edfc099
Author: Herman van Hazendonk <[email protected]>

thermal: qcom: add PM8901 PMIC temperature-alarm driver

This commit adds a thermal-of sensor driver for the temperature-alarm block inside the Qualcomm PM8901 PMIC. It registers interrupts and decodes stages into temperatures for board thermal zones.

Quoted 4 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +0 -0
new file mode 100644

[ ... ]

Quoted 13 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +13 -0
+/*
+ * Decode the (stage, threshold) pair into a single millicelsius value.
+ * Logic matches the legacy pmic8901-tm.c hysteresis selection:
+ *  - on a rising stage transition, use the lower bound of the new stage
+ *    plus +HYSTERESIS so we don't bounce
+ *  - on a falling stage transition, use the upper bound of the new stage
+ *    minus -HYSTERESIS
+ *  - on the first read after probe (initialised == false), pick a
+ *    representative point: midpoint of the stage range, or
+ *    PM8901_TEMP_NO_ALARM when stage == 0.
+ */
+static int pm8901_tm_update_temp_locked(struct pm8901_tm_chip *chip)
+{

[ ... ]

drivers/thermal/qcom/qcom-pm8901-tm.c line 117

Quoted 5 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +5 -0
+	if (!chip->initialised) {
+		if (new_stage)
+			chip->temp = PM8901_TEMP_THRESH_MIN +
+				     chip->thresh * PM8901_TEMP_THRESH_STEP +
+				     (new_stage - 1) * PM8901_TEMP_STAGE_STEP;

[Severity: Low]
This isn't a bug, but the comment above states this block picks the "midpoint of the stage range". Does this formula actually compute the exact lower bound of the stage instead?

Quoted 3 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +3 -0
+		else
+			chip->temp = PM8901_TEMP_NO_ALARM;
+		chip->initialised = true;

[ ... ]

Quoted 2 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +2 -0
+static int pm8901_tm_init_hw(struct pm8901_tm_chip *chip)
+{

[ ... ]

Quoted 10 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +10 -0
+	/*
+	 * Enable software override so PMIC does NOT auto-shut-down on stage 3.
+	 * Critical-trip orderly_poweroff is delivered by the kernel thermal
+	 * core via the DT thermal-zone trip with type = "critical".
+	 */
+	reg = (reg & ~(CTRL_OVRD_MASK | CTRL_STATUS_MASK | CTRL_THRESH_MASK)) |
+	      CTRL_OVRD_ST3 | CTRL_OVRD_ST2;
+	ret = pm8901_tm_write_ctrl(chip, reg);
+	if (ret)
+		goto out;

[ ... ]

Quoted 2 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +2 -0
+static int pm8901_tm_probe(struct platform_device *pdev)
+{

[ ... ]

drivers/thermal/qcom/qcom-pm8901-tm.c line 279

Quoted 9 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +9 -0
+	ret = pm8901_tm_init_hw(chip);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "hw init failed\n");
+
+	chip->tz_dev = devm_thermal_of_zone_register(&pdev->dev, 0, chip,
+						     &pm8901_tm_zone_ops);
+	if (IS_ERR(chip->tz_dev))
+		return dev_err_probe(&pdev->dev, PTR_ERR(chip->tz_dev),
+				     "thermal zone register failed\n");

[Severity: High]
Because pm8901_tm_init_hw() disables the PMIC's hardware auto-shutdown, if a subsequent initialization step like devm_thermal_of_zone_register() or IRQ registration fails, probe() exits with an error.

Since there is no devm action or cleanup path to restore the hardware state on failure, does this leave the system running with no hardware or software thermal protection?

Quoted 4 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +4 -0
+
+	ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL,
+					pm8901_tm_isr, IRQF_ONESHOT,
+					"pm8901-tm-alarm", chip);

[ ... ]

drivers/thermal/qcom/qcom-pm8901-tm.c line 300

Quoted 6 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +6 -0
+	platform_set_drvdata(pdev, chip);
+	thermal_zone_device_update(chip->tz_dev, THERMAL_EVENT_UNSPECIFIED);
+
+	dev_info(&pdev->dev,
+		 "PM8901 thermal alarm: base=0x%x stage=%u thresh=%u temp=%d\n",
+		 chip->base, chip->stage, chip->thresh, chip->temp);

[Severity: Low]
Since IRQs and the thermal core polling are fully enabled at this point, could an interrupt or thermal zone update execute concurrently and modify chip->stage, chip->thresh, or chip->temp?

Does reading these variables here without holding chip->lock risk printing torn or inconsistent values?

Quoted 3 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +3 -0
+
+	return 0;
+}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#31 lore raw

Re: [PATCH 2/2] irqchip: add MSM8x60 MPM wakeup interrupt controller driver

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider: - [High] The driver requests an IPC mailbox channel to notify the RPM of configuration changes but never…

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] The driver requests an IPC mailbox channel to notify the RPM of configuration changes but never sends any messages, rendering wake configurations ineffective.
- [High] Devres teardown sequence mismatch leaves the parent interrupt active after the IRQ domain is freed, leading to a Use-After-Free.
- [High] Calling regmap_read() and regmap_write() on a syscon regmap from within an irqchip's raw spinlock context causes a sleeping-while-atomic bug on PREEMPT_RT.
- [High] The irqchip fails to implement .irq_set_wake, breaking wakeup capabilities for the MPM and its parent GIC.
- [High] Clearing edge-triggered pending interrupts after executing the interrupt handler will drop subsequent edges.
- [Medium] Missing bounds validation on MPM pin indices from Device Tree allows arbitrary out-of-bounds register accesses.
- [Medium] Unsafe consumer API lacks reference counting or device links, leading to UAF if the MPM driver is unbound.
- [Medium] Failure to check the return value of regmap_read() results in uninitialized stack memory being used as register state.
--

commit 73a7572c3031147f818620520a565acba0e95ad1
Author: Herman van Hazendonk <[email protected]>

irqchip: add MSM8x60 MPM wakeup interrupt controller driver

This commit adds a driver for the MSM Power Manager (MPM) on the MSM8x60 family. It implements an irqchip wrapper to configure wake pins and exposes an API for consumer drivers to manage raw wake signals. The driver interacts with the RPM via a syscon regmap and IPC mailbox.

Quoted 6 lines
drivers/irqchip/irq-msm8660-mpm.c new file +0 -0
new file mode 100644
index 0000000000000..c579d1476e01b
@@ -0,0 +1,627 @@

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 105

Quoted 6 lines
drivers/irqchip/irq-msm8660-mpm.c new file +6 -0
+static u32 msm8660_mpm_read(struct msm8660_mpm *mpm, unsigned int reg)
+{
+	u32 val;
+	regmap_read(mpm->regmap, mpm->regmap_offset + reg, &val);
+	return val;
+}

[Severity: Medium]
Will this return uninitialized stack memory if the register read fails?

The variable val is uninitialized. If the regmap_read() function fails, it will not update val, and this function will return stack garbage.

This could result in corrupted state when the read value is treated as hardware pending status or used in read-modify-write operations.

[ ... ]

Quoted 21 lines
drivers/irqchip/irq-msm8660-mpm.c new file +21 -0
+static irqreturn_t msm8660_mpm_irq(int irq, void *data)
+{
+	struct msm8660_mpm *mpm = data;
+	unsigned long pending[MSM8660_MPM_REG_WIDTH];
+	unsigned long enable[MSM8660_MPM_REG_WIDTH];
+	int i, j;
+
+	for (i = 0; i < MSM8660_MPM_REG_WIDTH; i++) {
+		pending[i] = msm8660_mpm_read(mpm,
+			MSM8660_MPM_STATUS_OFFSET + i * 4);
+		enable[i] = msm8660_mpm_read(mpm,
+			MSM8660_MPM_REG_ENABLE + i * 4);
+	}
+
+	for (i = 0; i < MSM8660_MPM_REG_WIDTH; i++) {
+		unsigned long bits = pending[i] & enable[i];
+
+		for_each_set_bit(j, &bits, 32) {
+			int pin = i * 32 + j;
+			int hwirq = msm8660_mpm_pin_to_hwirq(mpm, pin);
+

drivers/irqchip/irq-msm8660-mpm.c line 160

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+			if (hwirq >= 0) {
+				dev_dbg(mpm->dev, "wake pin %d -> hwirq %d\n",
+					pin, hwirq);
+				generic_handle_domain_irq(mpm->domain, hwirq);
+			}
+		}
+
+		if (bits)
+			msm8660_mpm_write(mpm,
+				MSM8660_MPM_REG_CLEAR + i * 4, bits);

[Severity: High]
Does clearing the pending bits after executing the handler drop subsequent edges?

If a new edge interrupt arrives on any of these pins while the handler is executing in generic_handle_domain_irq(), the hardware will latch it again.

Because the write to the CLEAR register is delayed until after the handler finishes, it will wipe out the newly latched interrupt, permanently losing the new edge so it is not serviced.

Quoted 16 lines
drivers/irqchip/irq-msm8660-mpm.c new file +16 -0
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void msm8660_mpm_enable_hwirq(struct irq_data *d, bool enable)
+{
+	struct msm8660_mpm *mpm = irq_data_get_irq_chip_data(d);
+	unsigned long flags;
+	int i, pin = -1;
+	u32 val;
+
+	for (i = 0; i < mpm->pin_map_count; i++) {
+		if (mpm->pin_map[i].hwirq == d->hwirq) {
+			pin = mpm->pin_map[i].pin;
+			break;

drivers/irqchip/irq-msm8660-mpm.c line 186

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+		}
+	}
+
+	if (pin < 0)
+		return;
+
+	raw_spin_lock_irqsave(&mpm->lock, flags);
+
+	val = msm8660_mpm_read(mpm,
+		MSM8660_MPM_REG_ENABLE + (pin / 32) * 4);

[Severity: High]
Can this sleepable syscon regmap access while holding a raw spinlock cause a locking violation?

The core IRQ subsystem invokes irqchip callbacks like irq_mask() and irq_unmask() with the descriptor's raw_spinlock_t held and preemption disabled.

Inside these functions, the driver explicitly acquires another raw_spinlock_t and calls msm8660_mpm_read(), which invokes regmap_read() on the syscon regmap.

Syscon regmaps use a standard spinlock_t for synchronization, which on PREEMPT_RT is a sleepable rt_mutex. Attempting to acquire a sleepable lock while holding a raw_spinlock_t is a locking violation.

drivers/irqchip/irq-msm8660-mpm.c line 192

Quoted 6 lines
drivers/irqchip/irq-msm8660-mpm.c new file +6 -0
+	if (enable)
+		val |= BIT(pin % 32);
+	else
+		val &= ~BIT(pin % 32);
+	msm8660_mpm_write(mpm,
+		MSM8660_MPM_REG_ENABLE + (pin / 32) * 4, val);

[Severity: High]
Should this register update be followed by a mailbox notification to the RPM?

The driver requests an IPC mailbox channel in msm8660_mpm_probe() to signal configuration changes to the MPM. However, the driver never calls mbox_send_message() after updating the hardware state here.

Without the doorbell ping, the RPM hardware will not re-evaluate the wake configuration, which prevents the device from successfully waking the system from suspend.

Quoted 3 lines
drivers/irqchip/irq-msm8660-mpm.c new file +3 -0
+
+	raw_spin_unlock_irqrestore(&mpm->lock, flags);
+}

[ ... ]

Quoted 11 lines
drivers/irqchip/irq-msm8660-mpm.c new file +11 -0
+static int msm8660_mpm_set_type(struct irq_data *d, unsigned int type)
+{
+	struct msm8660_mpm *mpm = irq_data_get_irq_chip_data(d);
+	unsigned long flags;
+	int i, pin = -1;
+	u32 detect, polarity;
+
+	for (i = 0; i < mpm->pin_map_count; i++) {
+		if (mpm->pin_map[i].hwirq == d->hwirq) {
+			pin = mpm->pin_map[i].pin;
+			break;

drivers/irqchip/irq-msm8660-mpm.c line 229

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+		}
+	}
+
+	if (pin < 0)
+		return -ENOENT;
+
+	raw_spin_lock_irqsave(&mpm->lock, flags);
+
+	detect = msm8660_mpm_read(mpm,
+		MSM8660_MPM_REG_DETECT_CTL + (pin / 32) * 4);

[Severity: Medium]
Could this result in an out-of-bounds register access if the pin index is invalid?

The pin value is parsed from the Device Tree in msm8660_mpm_probe() without any bounds checking. If a malformed or malicious DT specifies a large pin index, calculating the register offset using (pin / 32) * 4 will result in an out-of-bounds read and write inside the RPM syscon block.

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 272

Quoted 8 lines
drivers/irqchip/irq-msm8660-mpm.c new file +8 -0
+static struct irq_chip msm8660_mpm_chip = {
+	.name			= "msm8660-mpm",
+	.irq_mask		= msm8660_mpm_mask_irq,
+	.irq_unmask		= msm8660_mpm_unmask_irq,
+	.irq_set_type		= msm8660_mpm_set_type,
+	.irq_eoi		= irq_chip_eoi_parent,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+	.flags			= IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SKIP_SET_WAKE,

[Severity: High]
Does this missing irq_set_wake callback break system wakeup functionality?

The commit message states the driver "overrides ->irq_set_wake() to enable the MPM mirror of the same interrupt".

However, the .irq_set_wake callback is omitted here and the flag IRQCHIP_SKIP_SET_WAKE is applied instead.

When a consumer driver calls enable_irq_wake(), the generic IRQ core will return 0 and stop propagating the wake request. Neither the MPM hardware nor the parent GIC will receive the wake configuration.

Quoted 1 line

+};

[ ... ]

Quoted 6 lines
drivers/irqchip/irq-msm8660-mpm.c new file +6 -0
+struct msm8660_mpm *msm8660_mpm_get(struct device_node *np,
+				    const char *propname)
+{
+	struct device_node *mpm_np;
+
+	if (!msm8660_mpm_global)

drivers/irqchip/irq-msm8660-mpm.c line 346

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+		return ERR_PTR(-EPROBE_DEFER);
+
+	if (np && propname) {
+		mpm_np = of_parse_phandle(np, propname, 0);
+		if (!mpm_np)
+			return ERR_PTR(-ENOENT);
+		of_node_put(mpm_np);
+	}
+
+	return msm8660_mpm_global;

[Severity: Medium]
Does returning the devm-managed pointer here create a use-after-free risk?

This consumer API returns a direct pointer to the devm_kzalloc allocated structure without establishing a device_link, incrementing module refcounts, or suppressing bind attributes.

If the MPM driver is unbound, devres will immediately free the structure. Consumer drivers like mmci will retain a dangling pointer, leading to a use-after-free when they later call msm8660_mpm_enable_pin().

Quoted 2 lines
drivers/irqchip/irq-msm8660-mpm.c new file +2 -0
+}
+EXPORT_SYMBOL_GPL(msm8660_mpm_get);

[ ... ]

Quoted 9 lines
drivers/irqchip/irq-msm8660-mpm.c new file +9 -0
+static int msm8660_mpm_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct msm8660_mpm *mpm;
+	struct irq_domain *parent_domain;
+	struct device_node *parent_np;
+	int ret, i;
+	u32 offset;

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 520

Quoted 9 lines
drivers/irqchip/irq-msm8660-mpm.c new file +9 -0
+	for (i = 0; i < mpm->pin_map_count; i++) {
+		u32 pin, hwirq;
+
+		of_property_read_u32_index(np, "qcom,mpm-pin-map",
+					   i * 2, &pin);
+		of_property_read_u32_index(np, "qcom,mpm-pin-map",
+					   i * 2 + 1, &hwirq);
+
+		mpm->pin_map[i].pin = pin;

[Severity: Medium]
Should the pin value be validated against MSM8660_MPM_PIN_COUNT before use?

The pin value is extracted from the Device Tree and stored directly. Missing bounds validation here allows arbitrary out-of-bounds register accesses later in functions like msm8660_mpm_set_type() and msm8660_mpm_enable_hwirq().

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 557

Quoted 3 lines
drivers/irqchip/irq-msm8660-mpm.c new file +3 -0
+	ret = devm_request_irq(dev, mpm->parent_irq, msm8660_mpm_irq,
+			       IRQF_TRIGGER_HIGH | IRQF_NO_SUSPEND,
+			       "msm8660-mpm", mpm);

[Severity: High]
Does registering the IRQ handler with devm create a teardown mismatch?

This registers the parent interrupt handler with devm_request_irq(), meaning it remains active until the devres cleanup phase executes. The devres cleanup happens after the remove() callback returns.

drivers/irqchip/irq-msm8660-mpm.c line 569

Quoted 12 lines
drivers/irqchip/irq-msm8660-mpm.c new file +12 -0
+	if (ret) {
+		dev_err(dev, "failed to request IRQ %d: %d\n",
+			mpm->parent_irq, ret);
+		goto err_remove_domain;
+	}
+
+	/* Mailbox channel for poking MPM to re-read its config (writes to
+	 * GCC + 0x008 bit 1 via the qcom-apcs-ipc mailbox).
+	 */
+	mpm->mbox_client.dev = dev;
+	mpm->mbox_client.knows_txdone = true;
+	mpm->mbox_chan = mbox_request_channel(&mpm->mbox_client, 0);

[Severity: High]
Is the mailbox channel requested here actually used to send messages?

The channel is requested for the purpose of signaling the MPM, but the driver never calls mbox_send_message() after updating the vMPM registers to trigger the IPC ping.

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 596

Quoted 8 lines
drivers/irqchip/irq-msm8660-mpm.c new file +8 -0
+static void msm8660_mpm_remove(struct platform_device *pdev)
+{
+	struct msm8660_mpm *mpm = platform_get_drvdata(pdev);
+
+	msm8660_mpm_global = NULL;
+	if (mpm->mbox_chan)
+		mbox_free_channel(mpm->mbox_chan);
+	irq_domain_remove(mpm->domain);

[Severity: High]
Can freeing the irq domain manually here cause a use-after-free crash?

The irq_domain_remove() function immediately frees the domain memory, while the devm-managed msm8660_mpm_irq() handler is still active.

If a hardware interrupt fires between the return of msm8660_mpm_remove() and devres teardown, the active handler will execute and access the freed domain pointer in generic_handle_domain_irq().

Quoted 1 line

+}

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#28 lore raw

Re: [PATCH 2/2] clk: qcom: add MSM8x60 LCC (LPASS) driver

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] Hardware register overlap in `CLK_AIF_OSR_DIV` macro corrupts clock enable states. - [Medium]…

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Hardware register overlap in CLK_AIF_OSR_DIV macro corrupts clock enable states.
- [Medium] Uninitialized variable val used if regmap_read() fails.
- [Low] Missing 27 MHz PXO entry in clk_tbl_pcm_393 fallback frequency table.
- [Low] Global state mutation in lcc_msm8660_probe() breaks state immutability. --

commit 757996c6435b09325bf482510e7802bb7053fdf8
Author: Herman van Hazendonk <[email protected]>

clk: qcom: add MSM8x60 LCC (LPASS) driver

This commit adds a clock driver for the LPASS Clock Controller on the MSM8x60 family. It introduces a separate driver due to register layout and parent muxing differences compared to the MSM8960.

Quoted 5 lines
drivers/clk/qcom/lcc-msm8660.c new file +0 -0
new file mode 100644
index 0000000000000..7f13279467f95

[ ... ]

Quoted 5 lines
drivers/clk/qcom/lcc-msm8660.c new file +5 -0
+#define CLK_AIF_OSR_DIV_CLK(prefix, _ns, _width)		\
+static struct clk_regmap_div prefix##_div_clk = {		\
+	.reg = _ns,						\
+	.shift = 10,						\
+	.width = _width,					\

[ ... ]

drivers/clk/qcom/lcc-msm8660.c line 231

Quoted 12 lines
drivers/clk/qcom/lcc-msm8660.c new file +12 -0
+/*
+ * CLK_AIF_OSR_DIV - Audio Interface with divider clocks.
+ * Enable bits per legacy MSM8660 kernel:
+ *   - OSR branch enable: BIT(17)
+ *   - BIT_DIV branch enable: BIT(15)
+ */
+#define CLK_AIF_OSR_DIV(prefix, _ns, _md, hr)			\
+	CLK_AIF_OSR_SRC(prefix, _ns, _md)			\
+	CLK_AIF_OSR_CLK(prefix, _ns, hr, 17)			\
+	CLK_AIF_OSR_DIV_CLK(prefix, _ns, 8)			\
+	CLK_AIF_OSR_BIT_DIV_CLK(prefix, _ns, hr, 15)		\
+	CLK_AIF_OSR_BIT_CLK(prefix, _ns, 18)

[Severity: High]
Does this configuration cause a hardware register overlap that corrupts clock enable states?

The macro configures a clock divider using CLK_AIF_OSR_DIV_CLK with shift 10 and width 8 on the _ns register, occupying bits 10 through 17.

However, the same macro configures branch clocks on the same _ns register with enable mask BIT(17) (via CLK_AIF_OSR_CLK) and enable mask BIT(15) (via CLK_AIF_OSR_BIT_DIV_CLK).

Since clock operations perform read-modify-writes, configuring the clock divider will overwrite the clock enable bits (15 and 17), and toggling the branch clocks will corrupt the divider value.

[ ... ]

Quoted 5 lines
drivers/clk/qcom/lcc-msm8660.c new file +5 -0
+static const struct freq_tbl clk_tbl_pcm_393[] = {
+	{   256000, P_PLL4, 4, 1, 384 },
+	{   512000, P_PLL4, 4, 1, 192 },
+	{   768000, P_PLL4, 4, 1, 128 },
+	{  1024000, P_PLL4, 4, 1,  96 },

drivers/clk/qcom/lcc-msm8660.c line 270

Quoted 10 lines
drivers/clk/qcom/lcc-msm8660.c new file +10 -0
+	{  1536000, P_PLL4, 4, 1,  64 },
+	{  2048000, P_PLL4, 4, 1,  48 },
+	{  3072000, P_PLL4, 4, 1,  32 },
+	{  4096000, P_PLL4, 4, 1,  24 },
+	{  6144000, P_PLL4, 4, 1,  16 },
+	{  8192000, P_PLL4, 4, 1,  12 },
+	{ 12288000, P_PLL4, 4, 1,   8 },
+	{ 24576000, P_PLL4, 4, 1,   4 },
+	{ }
+};

[Severity: Low]
Is the 27 MHz PXO entry missing from this fallback frequency table?

Other frequency tables like clk_tbl_pcm_540 include the
{ 27000000, P_PXO, 1, 0, 0 } entry right before the empty sentinel. Its omission here might prevent the PCM clock from falling back to the 27 MHz PXO source when the 393 MHz frequency plan is active.

[ ... ]

Quoted 6 lines
drivers/clk/qcom/lcc-msm8660.c new file +6 -0
+static int lcc_msm8660_probe(struct platform_device *pdev)
+{
+	struct regmap *regmap;
+	u32 val;
+
+	regmap = qcom_cc_map(pdev, &lcc_msm8660_desc);

drivers/clk/qcom/lcc-msm8660.c line 484

Quoted 10 lines
drivers/clk/qcom/lcc-msm8660.c new file +10 -0
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	/*
+	 * MSM8x60 should always boot with PLL4 L=22 (540.672 MHz).
+	 * Detect anyway so a board with a non-standard L value still gets a
+	 * coherent frequency plan instead of silently producing wrong rates.
+	 */
+	regmap_read(regmap, 0x4, &val);
+	if (val == 0x16) {

[Severity: Medium]
Can an uninitialized variable be used here if regmap_read() fails?

The return value of regmap_read() is ignored. If it fails and returns a negative error code, val will remain uninitialized stack memory. It is then evaluated in this condition and logged, which could cause undefined behavior.

drivers/clk/qcom/lcc-msm8660.c line 493

Quoted 9 lines
drivers/clk/qcom/lcc-msm8660.c new file +9 -0
+		dev_info(&pdev->dev,
+			 "PLL4 L=0x%x, using 540MHz frequency plan\n", val);
+		slimbus_src.freq_tbl = clk_tbl_aif_osr_540;
+		mi2s_osr_src.freq_tbl = clk_tbl_aif_osr_540;
+		codec_i2s_mic_osr_src.freq_tbl = clk_tbl_aif_osr_540;
+		spare_i2s_mic_osr_src.freq_tbl = clk_tbl_aif_osr_540;
+		codec_i2s_spkr_osr_src.freq_tbl = clk_tbl_aif_osr_540;
+		spare_i2s_spkr_osr_src.freq_tbl = clk_tbl_aif_osr_540;
+		pcm_src.freq_tbl = clk_tbl_pcm_540;

[Severity: Low]
Could mutating these static global clock structures break state immutability?

This probe dynamically modifies the freq_tbl pointers of static global clock structures (like slimbus_src and mi2s_osr_src) based on a hardware register read.

If a subsequent probe executes where val is not 0x16, the else branch does not restore these pointers to their original 393 tables, causing the device to incorrectly inherit the mutated 540MHz state. While unlikely to manifest in a singleton driver with constant hardware values, it violates the principle of keeping global clock structures immutable.

Quoted 5 lines
drivers/clk/qcom/lcc-msm8660.c new file +5 -0
+	} else {
+		dev_info(&pdev->dev,
+			 "PLL4 L=0x%x, using fallback 393MHz frequency plan\n",
+			 val);
+	}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#5 lore raw

Re: [PATCH 1/2] dt-bindings: thermal: qcom: add pm8901-temp-alarm

In reply to Herman van Hazendonk
Quoted 11 lines · Herman van Hazendonk

Add the binding for the temperature-alarm block inside the Qualcomm PM8901 PMIC (companion to the PM8058 on MSM8x60). The driver exposes the PM8901 die-temperature trip stages (105 / 125 / 145 C) to the thermal framework via a thermal-zone, with stage 3 wired as a critical trip so the kernel issues orderly_poweroff() when the part overheats.

The binding describes the SSBI sub-node address, the GIC interrupt the alarm raises on a stage transition, and the parent PMIC reference.

Quoted 6 lines
---
 .../thermal/qcom,pm8901-temp-alarm.yaml       | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.example.dtb: pmic (qcom,pm8901): 'temp-alarm@23' does not match any of the regexes: '^pinctrl-[0-9]+$', 'gpio@[0-9a-f]+$', 'keypad@[0-9a-f]+$', 'led@[0-9a-f]+$', 'mpps@[0-9a-f]+$', 'pwrkey@[0-9a-f]+$', 'rtc@[0-9a-f]+$', 'vibrator@[0-9a-f]+$', 'xoadc@[0-9a-f]+$'
from schema $id: http://devicetree.org/schemas/mfd/qcom-pm8xxx.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.example.dtb: pmic (qcom,pm8901): 'oneOf' conditional failed, one must be fixed:
'interrupts' is a required property
'interrupts-extended' is a required property
from schema $id: http://devicetree.org/schemas/mfd/qcom-pm8xxx.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/386f3cef9d50d61f588f99706d9f979c56f8bab8.1780148149.git.github.com@herrie.org

The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

#32 lore raw

v2 cover clk: qcom: add MSM8x60 LPASS Clock Controller

In reply to Herman van Hazendonk

Hi all,

Self-review (with Sashiko AI assist) caught five issues in v1 before maintainer review reached them. v1:

https://lore.kernel.org/linux-arm-msm/[email protected]/

v2 changes:

- NEW patch 1/3: extend Documentation/devicetree/bindings/clock/ qcom,lcc.yaml with the three MSM8x60 family compatibles (qcom,lcc-msm8260, qcom,lcc-msm8660, qcom,lcc-apq8060) and add them to the existing apq8064/msm8960 conditional block. Without this, board DTs using the new compatibles fail dt_binding_check.

- patch 2/3: dt-bindings clock-ID header for lcc-msm8660, unchanged from v1.

- patch 3/3: LCC driver. Five fixes folded in:

* CLK_AIF_OSR_DIV macro: divider width was 8, which made the bit-divider field span bits 10 through 17 on the _ns register. On MSM8x60 BIT(15) (BIT_DIV branch enable) and BIT(17) (OSR branch enable) sit inside that range, so any clk_regmap_div read-modify-write would clobber both branch gates. Width 4 (bits 10 through 13) matches the standalone mi2s div clock and the legacy downstream Samsung MSM8660 / webOS clock-8x60.c register layout. Added a block comment documenting the verified bit assignments and the rationale for the width change.

* Match table: add qcom,lcc-msm8260 alongside the existing qcom,lcc-msm8660 and qcom,lcc-apq8060. MSM8x60 covers all three variants.

* clk_tbl_pcm_393: add the 27 MHz PXO fallback entry that clk_tbl_pcm_540 already carries, so the 393 plan can also fall back to the board PXO source.

* lcc_msm8660_probe(): check the return value of regmap_read() on PLL4 L. v1 ignored it, so a regmap failure would have left val as uninitialised stack memory and steered the rest of probe down a random branch. Use dev_err_probe() on failure.

* lcc_msm8660_probe(): assign the freq_tbl pointers on the static clk_rcg structs unconditionally on both PLL4 plans instead of mutating-on-540 / leaving-mutated-on-393. The driver is effectively singleton today, but explicit assignment also restores the defaults if the driver is ever rebound on a system whose PLL4 has been reprogrammed.

Thanks,
Herman

Herman van Hazendonk (3):
dt-bindings: clock: qcom,lcc: add MSM8x60 family compatibles
dt-bindings: clock: qcom: add lcc-msm8660 LPASS clock IDs
clk: qcom: add MSM8x60 LCC (LPASS) driver

 .../devicetree/bindings/clock/qcom,lcc.yaml   |   6 +
 drivers/clk/qcom/Kconfig                      |   9 +
 drivers/clk/qcom/Makefile                     |   1 +
 drivers/clk/qcom/lcc-msm8660.c                | 551 ++++++++++++++++++
 include/dt-bindings/clock/qcom,lcc-msm8660.h  |  48 ++
 5 files changed, 615 insertions(+)
 create mode 100644 drivers/clk/qcom/lcc-msm8660.c
 create mode 100644 include/dt-bindings/clock/qcom,lcc-msm8660.h
Signature · 1 line
2.43.0
#33 lore raw

PATCH v2 1/3 dt-bindings: clock: qcom,lcc: add MSM8x60 family compatibles

Add "qcom,lcc-msm8260", "qcom,lcc-msm8660" and "qcom,lcc-apq8060" to the qcom,lcc enum and to the existing parent-clock-name conditional block that already covers apq8064/msm8960.

The MSM8x60 family (MSM8260/MSM8660/APQ8060) is the Scorpion-class generation preceding MSM8960; its LPASS Clock Controller is fed by the same pxo board source and pll4_vote parents as apq8064/msm8960, and the codec-fed slave-mode I2S/PCM clocks have the same names, so the existing apq8064/msm8960 conditional block applies unchanged once the new compatibles are listed.

This allows board DTs using the new compatibles to pass dt_binding_check; the actual driver support is added by a later patch in this series.

---
 Documentation/devicetree/bindings/clock/qcom,lcc.yaml | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/clock/qcom,lcc.yaml +6 -0
index 55985e562a34..bbdad157f03f 100644
@@ -12,9 +12,12 @@ maintainers:
 properties:
   compatible:
     enum:
+      - qcom,lcc-apq8060
       - qcom,lcc-apq8064
       - qcom,lcc-ipq8064
       - qcom,lcc-mdm9615
+      - qcom,lcc-msm8260
+      - qcom,lcc-msm8660
       - qcom,lcc-msm8960
 
   clocks:
@@ -46,7 +49,10 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,lcc-apq8060
               - qcom,lcc-apq8064
+              - qcom,lcc-msm8260
+              - qcom,lcc-msm8660
               - qcom,lcc-msm8960
     then:
       properties:
Signature · 1 line
2.43.0
#36 lore raw

PATCH v2 2/3 dt-bindings: clock: qcom: add lcc-msm8660 LPASS clock IDs

In reply to Herman van Hazendonk

Add the dt-binding clock-ID header for the MSM8x60 family (MSM8260/MSM8660/APQ8060) Low Power Audio SubSystem Clock Controller (LCC). The header enumerates the LPASS clocks consumed by the qcom,apq8060-lpaif sound card and the codec/AIF nodes downstream of it. It mirrors the format and ID range of the existing LCC headers for newer Qualcomm SoCs (lcc-msm8960, lcc-msm8974) so the drivers/clk/qcom/lcc-msm8660.c driver can be hooked up the same way once it lands.

---
 include/dt-bindings/clock/qcom,lcc-msm8660.h | 48 ++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 include/dt-bindings/clock/qcom,lcc-msm8660.h
include/dt-bindings/clock/qcom,lcc-msm8660.h new file +48 -0
new file mode 100644
index 000000000000..d5d9b0d71a78
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_LCC_MSM8660_H
+#define _DT_BINDINGS_CLK_LCC_MSM8660_H
+
+/*
+ * MSM8x60 family (MSM8260/MSM8660/APQ8060) LPASS Clock Controller (LCC)
+ * clock IDs. These are compatible with MSM8960 LCC as MSM8x60 and
+ * MSM8960 share the same audio subsystem clock architecture.
+ */
+
+#define PLL4				0
+#define MI2S_OSR_SRC			1
+#define MI2S_OSR_CLK			2
+#define MI2S_DIV_CLK			3
+#define MI2S_BIT_DIV_CLK		4
+#define MI2S_BIT_CLK			5
+#define PCM_SRC				6
+#define PCM_CLK_OUT			7
+#define PCM_CLK				8
+#define SLIMBUS_SRC			9
+#define AUDIO_SLIMBUS_CLK		10
+#define SPS_SLIMBUS_CLK			11
+#define CODEC_I2S_MIC_OSR_SRC		12
+#define CODEC_I2S_MIC_OSR_CLK		13
+#define CODEC_I2S_MIC_DIV_CLK		14
+#define CODEC_I2S_MIC_BIT_DIV_CLK	15
+#define CODEC_I2S_MIC_BIT_CLK		16
+#define SPARE_I2S_MIC_OSR_SRC		17
+#define SPARE_I2S_MIC_OSR_CLK		18
+#define SPARE_I2S_MIC_DIV_CLK		19
+#define SPARE_I2S_MIC_BIT_DIV_CLK	20
+#define SPARE_I2S_MIC_BIT_CLK		21
+#define CODEC_I2S_SPKR_OSR_SRC		22
+#define CODEC_I2S_SPKR_OSR_CLK		23
+#define CODEC_I2S_SPKR_DIV_CLK		24
+#define CODEC_I2S_SPKR_BIT_DIV_CLK	25
+#define CODEC_I2S_SPKR_BIT_CLK		26
+#define SPARE_I2S_SPKR_OSR_SRC		27
+#define SPARE_I2S_SPKR_OSR_CLK		28
+#define SPARE_I2S_SPKR_DIV_CLK		29
+#define SPARE_I2S_SPKR_BIT_DIV_CLK	30
+#define SPARE_I2S_SPKR_BIT_CLK		31
+
+#endif
Signature · 1 line
2.43.0
#38 lore raw

PATCH v2 3/3 clk: qcom: add MSM8x60 LCC (LPASS) driver

In reply to Herman van Hazendonk

Add a clock driver for the LPASS (Low Power Audio SubSystem) Clock Controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The register layout, parent muxing and divider topology of the LPASS PLL/clk fabric differ from MSM8960's LCC enough that a clean separate driver is simpler than parameterising mmcc-msm8960.c. Both drivers can coexist in tree (different KConfig options, different match table, different device-tree compatible).

Used on the HP TouchPad (Tenderloin) where the LPASS Q6 audio DSP needs functional MI2S / SLIMBus / PCM clocks before audio playback or capture works.

---
 drivers/clk/qcom/Kconfig       |   9 +
 drivers/clk/qcom/Makefile      |   1 +
 drivers/clk/qcom/lcc-msm8660.c | 551 +++++++++++++++++++++++++++++++++
 3 files changed, 561 insertions(+)
 create mode 100644 drivers/clk/qcom/lcc-msm8660.c
drivers/clk/qcom/Kconfig +9 -0
index d9cff5b0281d..2ea95f69355e 100644
@@ -547,6 +547,15 @@ config MSM_LCC_8960
 	  Say Y if you want to use audio devices such as i2s, pcm,
 	  SLIMBus, etc.
 
+config MSM_LCC_8660
+	tristate "MSM8x60 LPASS Clock Controller"
+	depends on ARM || COMPILE_TEST
+	help
+	  Support for the LPASS clock controller on the MSM8x60 family
+	  (MSM8260/MSM8660/APQ8060). MSM8960 is the newer
+	  Krait-based generation handled separately by MSM_LCC_8960.
+	  Say Y if you want to use audio devices such as i2s, pcm, SLIMBus.
+
 config MDM_GCC_9607
 	tristate "MDM9607 Global Clock Controller"
 	depends on ARM || COMPILE_TEST
drivers/clk/qcom/Makefile +1 -0
index e100cfd6a52d..41c973d7db59 100644
@@ -77,6 +77,7 @@ obj-$(CONFIG_MSM_GCC_8974) += gcc-msm8974.o
 obj-$(CONFIG_MSM_GCC_8976) += gcc-msm8976.o
 obj-$(CONFIG_MSM_GCC_8994) += gcc-msm8994.o
 obj-$(CONFIG_MSM_GCC_8996) += gcc-msm8996.o
+obj-$(CONFIG_MSM_LCC_8660) += lcc-msm8660.o
 obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o
 obj-$(CONFIG_MSM_GCC_8998) += gcc-msm8998.o
 obj-$(CONFIG_MSM_GPUCC_8998) += gpucc-msm8998.o
drivers/clk/qcom/lcc-msm8660.c new file +551 -0

551 lines not on this page · raw message

Signature · 1 line
2.43.0
#41 lore raw

v2 cover interconnect: qcom: add MSM8x60 NoC driver

In reply to Herman van Hazendonk

Hi all,

Self-review (with Sashiko AI assist) caught five real issues in v1's NoC driver before the maintainer review cycle reached them, so re-rolling promptly. v1:

https://lore.kernel.org/linux-arm-msm/[email protected]/

v2 changes (driver patch only; the binding header is unchanged from v1):

- MMFAB master port collision: mmfab_to_appss claimed port 2, but that port was already owned by mmfab_mas_adm1_port0. Per the fabric's port table the APPS_FAB gateway lives at port 11; fix the DEFINE_QNODE so ADM1 DMA's arbitration vote on MMFAB is no longer overwritten by the gateway.

- msm8660_rpm_commit() double-converted bandwidth units: it called icc_units_to_bps() (which already returns bytes/s) and then immediately divided by 8 "bits -> bytes", asking RPM for 1/8 of the bandwidth that consumers had requested. Drop the spurious divide.

- Fabric rate cache hoisted to provider scope and divided by the fabric's own bus width rather than the triggering node's local buswidth. A fabric has one shared hardware clock; using each master's local buswidth makes the requested clk rate oscillate depending on which master called icc_set_bw() last, and caching the result on the node lets a subsequent update skip clk_set_rate even though a different node has already moved the hardware. The new layout adds desc->bus_width per fabric (AFAB/SFAB/DFAB=8, MMFAB=16) and qp->rate as the single source of truth.

- msm8660_get_rpm() rewritten:
* returns ERR_PTR(-EPROBE_DEFER) when the RPM phandle resolves but the device hasn't probed yet, instead of returning NULL and silently disabling ARB forever;
* adds a device_link to the RPM device so the devres-managed qcom_rpm struct can't be freed out from under us if the RPM driver is unbound at runtime (previously a use-after-free risk because we kept a pointer past put_device()).
Probe propagates IS_ERR(qp->rpm) up so the framework retries.

- devm_clk_bulk_get_optional() now distinguishes -EPROBE_DEFER (propagated) from real "no clocks" (continue without scaling). Previously every error was swallowed, which permanently disabled clock scaling if the icc driver happened to probe before the clock provider - which it usually does, given the core_initcall ordering.

No functional change intended on the working paths; this is purely correctness work.

Thanks,
Herman

Herman van Hazendonk (2):
dt-bindings: interconnect: qcom: add msm8660 fabric IDs
interconnect: qcom: add MSM8x60 NoC driver

 drivers/interconnect/qcom/Kconfig             |   10 +
 drivers/interconnect/qcom/Makefile            |    2 +
 drivers/interconnect/qcom/msm8660.c           | 1069 +++++++++++++++++
 .../dt-bindings/interconnect/qcom,msm8660.h   |  156 +++
 4 files changed, 1237 insertions(+)
 create mode 100644 drivers/interconnect/qcom/msm8660.c
 create mode 100644 include/dt-bindings/interconnect/qcom,msm8660.h
Signature · 1 line
2.43.0
#42 lore raw

PATCH v2 1/2 dt-bindings: interconnect: qcom: add msm8660 fabric IDs

Add the dt-binding interconnect master/slave ID header for the MSM8x60 family (MSM8260/MSM8660/APQ8060) fabric mesh. The chip's NoC fabric is split into multiple sub-fabrics that the qnoc-msm8660 driver models:

AFAB - Applications fabric (Scorpion CPU + L2) SFAB - System fabric (DMA, SPS, security)
MMFAB - Multimedia fabric (MDP, GPU, camera, video, rotator)

  DFAB  - Daytona fabric (SDC, ADM master/slave)
  SFPB  - System Fast Peripheral Bridge (RPM, MPM, PMIC SSBI)
  CFPB  - CPU Subsystem Fast Peripheral Bus (GSBI UART/QUP, USB FS,

TSIF, TSSC, PDM, PRNG)

IDs derived from the legacy webOS msm_bus_board_8660.c master/slave enums, normalised to the upstream interconnect-framework naming.

---
 .../dt-bindings/interconnect/qcom,msm8660.h   | 156 ++++++++++++++++++
 1 file changed, 156 insertions(+)
 create mode 100644 include/dt-bindings/interconnect/qcom,msm8660.h
include/dt-bindings/interconnect/qcom,msm8660.h new file +156 -0
new file mode 100644
index 000000000000..c9ce3f5a5276
@@ -0,0 +1,156 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Qualcomm MSM8x60 family (MSM8260/MSM8660/APQ8060) interconnect IDs
+ *
+ * Copyright (c) 2026 Herman van Hazendonk <[email protected]>
+ *
+ * Based on webOS kernel msm_bus_board_8660.c
+ * Copyright (c) 2010-2011, Code Aurora Forum. All rights reserved.
+ */
+
+#ifndef __DT_BINDINGS_INTERCONNECT_QCOM_MSM8660_H
+#define __DT_BINDINGS_INTERCONNECT_QCOM_MSM8660_H
+
+/*
+ * MSM8x60 has a fabric-based bus architecture:
+ * - APPSS Fabric: CPU and memory interface
+ * - System Fabric: System peripherals and DMA
+ * - MMSS Fabric: Multimedia subsystem (display, camera, video)
+ * - Daytona Fabric: Peripheral bus (SDCC, ADM DMA)
+ * - System FPB: System Fast Peripheral Bus
+ * - CPSS FPB: CPU Subsystem Fast Peripheral Bus
+ */
+
+/* APPSS Fabric - Apps processor fabric */
+#define AFAB_MAS_AMPSS_M0		0
+#define AFAB_MAS_AMPSS_M1		1
+#define AFAB_SLV_EBI_CH0		2
+#define AFAB_SLV_AMPSS_L2		3
+#define AFAB_TO_MMSS			4
+#define AFAB_TO_SYSTEM			5
+
+/* System Fabric - System bus */
+#define SFAB_MAS_APPSS			0
+#define SFAB_MAS_SPS			1
+#define SFAB_MAS_ADM0_PORT0		2
+#define SFAB_MAS_ADM0_PORT1		3
+#define SFAB_MAS_ADM1_PORT0		4
+#define SFAB_MAS_ADM1_PORT1		5
+#define SFAB_MAS_LPASS_PROC		6
+#define SFAB_MAS_MSS_PROCI		7
+#define SFAB_MAS_MSS_PROCD		8
+#define SFAB_MAS_MSS_MDM_PORT0		9
+#define SFAB_MAS_LPASS			10
+#define SFAB_MAS_MMSS_FPB		11
+#define SFAB_MAS_ADM1_CI		12
+#define SFAB_MAS_ADM0_CI		13
+#define SFAB_MAS_MSS_MDM_PORT1		14
+#define SFAB_MAS_USB_HS			15
+#define SFAB_TO_APPSS			16
+#define SFAB_TO_SYSTEM_FPB		17
+#define SFAB_TO_CPSS_FPB		18
+#define SFAB_SLV_SPS			19
+#define SFAB_SLV_SYSTEM_IMEM		20
+#define SFAB_SLV_AMPSS			21
+#define SFAB_SLV_MSS			22
+#define SFAB_SLV_LPASS			23
+#define SFAB_SLV_MMSS_FPB		24
+#define SFAB_TO_DFAB			25
+
+/* MMSS Fabric - Multimedia subsystem */
+#define MMFAB_MAS_MDP_PORT0		0
+#define MMFAB_MAS_MDP_PORT1		1
+#define MMFAB_MAS_ADM1_PORT0		2
+#define MMFAB_MAS_ROTATOR		3
+#define MMFAB_MAS_GRAPHICS_3D		4
+#define MMFAB_MAS_JPEG_DEC		5
+#define MMFAB_MAS_GRAPHICS_2D_CORE0	6
+#define MMFAB_MAS_VFE			7
+#define MMFAB_MAS_VPE			8
+#define MMFAB_MAS_JPEG_ENC		9
+#define MMFAB_MAS_GRAPHICS_2D_CORE1	10
+#define MMFAB_MAS_HD_CODEC_PORT0	11
+#define MMFAB_MAS_HD_CODEC_PORT1	12
+#define MMFAB_TO_APPSS			13
+#define MMFAB_SLV_SMI			14
+#define MMFAB_SLV_MM_IMEM		15
+
+/*
+ * Daytona Fabric (DFAB) - Peripheral bus
+ *
+ * DFAB connects slower peripherals (SDCC, ADM DMA) to the system fabric.
+ * The webOS kernel managed DFAB bandwidth via voter clocks (dfab_sdc*_clk,
+ * dfab_usb_hs_clk). In mainline, this is handled by the interconnect framework.
+ *
+ * USB HS is included as a DFAB voter for compatibility with the legacy clock
+ * voting mechanism. The webOS kernel comment said: "if usb link is in sps
+ * there is no need for usb pclk as daytona fabric clock will be used instead".
+ * This keeps DFAB clock stable when USB is active.
+ */
+#define DFAB_MAS_SDC1			0
+#define DFAB_MAS_SDC2			1
+#define DFAB_MAS_SDC3			2
+#define DFAB_MAS_SDC4			3
+#define DFAB_MAS_SDC5			4
+#define DFAB_MAS_ADM0_MASTER		5
+#define DFAB_MAS_ADM1_MASTER		6
+#define DFAB_TO_SFAB			7
+#define DFAB_SLV_SDC1			8
+#define DFAB_SLV_SDC2			9
+#define DFAB_SLV_SDC3			10
+#define DFAB_SLV_SDC4			11
+#define DFAB_SLV_SDC5			12
+#define DFAB_MAS_USB_HS			13
+#define DFAB_MAS_DSPS			14
+
+/* System FPB - Slow peripheral bus for system */
+#define SFPB_MAS_SYSTEM			0
+#define SFPB_MAS_SPDM			1
+#define SFPB_MAS_RPM			2
+#define SFPB_SLV_SPDM			3
+#define SFPB_SLV_RPM			4
+#define SFPB_SLV_RPM_MSG_RAM		5
+#define SFPB_SLV_MPM			6
+#define SFPB_SLV_PMIC1_SSBI1_A		7
+#define SFPB_SLV_PMIC1_SSBI1_B		8
+#define SFPB_SLV_PMIC1_SSBI1_C		9
+#define SFPB_SLV_PMIC2_SSBI2_A		10
+#define SFPB_SLV_PMIC2_SSBI2_B		11
+
+/* CPSS FPB - CPU subsystem fast peripheral bus */
+#define CFPB_MAS_SYSTEM			0
+#define CFPB_SLV_GSBI1_UART		1
+#define CFPB_SLV_GSBI2_UART		2
+#define CFPB_SLV_GSBI3_UART		3
+#define CFPB_SLV_GSBI4_UART		4
+#define CFPB_SLV_GSBI5_UART		5
+#define CFPB_SLV_GSBI6_UART		6
+#define CFPB_SLV_GSBI7_UART		7
+#define CFPB_SLV_GSBI8_UART		8
+#define CFPB_SLV_GSBI9_UART		9
+#define CFPB_SLV_GSBI10_UART		10
+#define CFPB_SLV_GSBI11_UART		11
+#define CFPB_SLV_GSBI12_UART		12
+#define CFPB_SLV_GSBI1_QUP		13
+#define CFPB_SLV_GSBI2_QUP		14
+#define CFPB_SLV_GSBI3_QUP		15
+#define CFPB_SLV_GSBI4_QUP		16
+#define CFPB_SLV_GSBI5_QUP		17
+#define CFPB_SLV_GSBI6_QUP		18
+#define CFPB_SLV_GSBI7_QUP		19
+#define CFPB_SLV_GSBI8_QUP		20
+#define CFPB_SLV_GSBI9_QUP		21
+#define CFPB_SLV_GSBI10_QUP		22
+#define CFPB_SLV_GSBI11_QUP		23
+#define CFPB_SLV_GSBI12_QUP		24
+#define CFPB_SLV_EBI2_NAND		25
+#define CFPB_SLV_USB_FS1		26
+#define CFPB_SLV_USB_FS2		27
+#define CFPB_SLV_TSIF			28
+#define CFPB_SLV_MSM_TSSC		29
+#define CFPB_SLV_MSM_PDM		30
+#define CFPB_SLV_MSM_DIMEM		31
+#define CFPB_SLV_MSM_TCSR		32
+#define CFPB_SLV_MSM_PRNG		33
+
+#endif /* __DT_BINDINGS_INTERCONNECT_QCOM_MSM8660_H */
Signature · 1 line
2.43.0
#44 lore raw

PATCH v2 2/2 interconnect: qcom: add MSM8x60 NoC driver

In reply to Herman van Hazendonk

Add a Qualcomm interconnect driver for the MSM8x60 family (MSM8260/MSM8660/APQ8060), modelling the four fabrics that connect masters and slaves on these Scorpion-class SoCs:

- AFAB : Application/CPU fabric
- SFAB : System fabric (peripherals, USB, SDCC, etc.)
- MMFAB : Multimedia fabric (MDP, VFE, VIDC, GPU, JPEG, VPE, ROT)
- DFAB : Daytona fabric (low-bandwidth peripherals)

The driver implements the interconnect-provider API so that consumer drivers (display, camera, video, GPU, USB, MMC) can request bandwidth between specific masters and slaves via icc_set_bw(), letting the firmware-managed bus-scaling decide actual NoC clock rates.

Used on the HP TouchPad (Tenderloin) and on the Pre3 / Veer form-factors; without it, the multimedia and storage paths are starved of bandwidth and run at minimum NoC clocks.

---
 drivers/interconnect/qcom/Kconfig   |   10 +
 drivers/interconnect/qcom/Makefile  |    2 +
 drivers/interconnect/qcom/msm8660.c | 1069 +++++++++++++++++++++++++++
 3 files changed, 1081 insertions(+)
 create mode 100644 drivers/interconnect/qcom/msm8660.c
drivers/interconnect/qcom/Kconfig +10 -0
index 786b4eda44b4..17364be522c7 100644
@@ -80,6 +80,16 @@ config INTERCONNECT_QCOM_MSM8953
 	  This is a driver for the Qualcomm Network-on-Chip on msm8953-based
 	  platforms.
 
+config INTERCONNECT_QCOM_MSM8660
+	tristate "Qualcomm MSM8x60 interconnect driver"
+	depends on INTERCONNECT_QCOM
+	depends on MFD_QCOM_RPM
+	help
+	  This is a driver for the Qualcomm fabric-based bus interconnect
+	  on MSM8x60 family (MSM8260/MSM8660/APQ8060) platforms (e.g., HP TouchPad).
+	  The driver manages APPSS, System, and MMSS fabrics and sends
+	  per-port bandwidth arbitration requests to RPM firmware.
+
 config INTERCONNECT_QCOM_MSM8974
 	tristate "Qualcomm MSM8974 interconnect driver"
 	depends on INTERCONNECT_QCOM
drivers/interconnect/qcom/Makefile +2 -0
index cdf2c6c9fbf3..0c849c30a907 100644
@@ -13,6 +13,7 @@ qnoc-msm8916-objs			:= msm8916.o
 qnoc-msm8937-objs			:= msm8937.o
 qnoc-msm8939-objs			:= msm8939.o
 qnoc-msm8953-objs			:= msm8953.o
+qnoc-msm8660-objs			:= msm8660.o
 qnoc-msm8974-objs			:= msm8974.o
 qnoc-msm8976-objs			:= msm8976.o
 qnoc-msm8996-objs			:= msm8996.o
@@ -58,6 +59,7 @@ obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += qnoc-msm8916.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8937) += qnoc-msm8937.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8939) += qnoc-msm8939.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8953) += qnoc-msm8953.o
+obj-$(CONFIG_INTERCONNECT_QCOM_MSM8660) += qnoc-msm8660.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8974) += qnoc-msm8974.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8976) += qnoc-msm8976.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8996) += qnoc-msm8996.o
drivers/interconnect/qcom/msm8660.c new file +1069 -0

1,069 lines not on this page · raw message

Signature · 1 line
2.43.0
#51 lore raw

v2 cover irqchip: add MSM8x60 MPM wakeup interrupt controller

In reply to Herman van Hazendonk

Hi all,

Self-review (with Sashiko AI assist) caught eight real issues in v1 before maintainer review reached them; re-rolling promptly. v1:

https://lore.kernel.org/linux-arm-msm/[email protected]/

v2 changes:

- PREEMPT_RT: drop the syscon regmap and ioremap the vMPM sub-region of the RPM control block directly. The IRQ core invokes the irqchip mask/unmask/set_type/set_wake callbacks with the irq_desc raw_spinlock_t held, and syscon regmaps use a sleepable spinlock_t that becomes an rt_mutex on PREEMPT_RT. Direct MMIO (readl_relaxed/writel_relaxed) is what every other SoC irqchip in tree does for the same reason (qcom-pdc, gic-v3, ...).

- RPM doorbell: after every vMPM register write, send a message through the qcom-apcs-ipc mailbox. v1 acquired the mailbox channel but never used it, so the RPM kept running with its cached enable / detect / polarity state and our configuration changes had no effect.

- Teardown order: v1 used devm_request_irq() and then called irq_domain_remove() manually in ->remove(); the devres-managed handler outlives the domain, so an IRQ arriving in the removal window would dereference a freed domain. Switch to plain request_irq() / free_irq() and remove in strict reverse-init order (free_irq, free mailbox, irq_domain_remove).

- .irq_set_wake: implement it (toggle the MPM enable bit + delegate to irq_chip_set_wake_parent for the GIC) and drop IRQCHIP_SKIP_SET_WAKE. v1 declared SKIP and provided no callback, so enable_irq_wake() was a silent no-op for every consumer.

- Clear-before-dispatch: in the IPC IRQ handler, write CLEAR for the pending bits before calling generic_handle_domain_irq() for each pin. v1 cleared after the handler returned, racing against a fresh edge latched during dispatch and silently dropping it.

- DT pin-map bounds check: validate each qcom,mpm-pin-map pin entry against MSM8660_MPM_PIN_COUNT before storing it, so a malformed DT cannot induce out-of-bounds register accesses in later set_type/enable paths.

- Consumer API lifetime: msm8660_mpm_get() now takes a consumer device argument and establishes a DL_FLAG_AUTOREMOVE_CONSUMER device link to the MPM device. Without this, an unbind of the MPM driver would free the devres-managed struct while consumers still hold pointers, causing UAF on the next call.

- regmap_read return check: moot now that we use readl_relaxed, but the original Sashiko finding (uninitialised val on regmap error) is closed.

DT binding: drop qcom,rpm-syscon and qcom,mpm-offset; replace with a single reg property pointing at the vMPM window inside the RPM control block. dt_binding_check passes on the example.

Driver passes checkpatch with zero warnings or errors.

Thanks,
Herman

Herman van Hazendonk (2):
dt-bindings: interrupt-controller: qcom: add msm8660-mpm
irqchip: add MSM8x60 MPM wakeup interrupt controller driver

 .../qcom,msm8660-mpm.yaml                     | 122 +++
 drivers/irqchip/Kconfig                       |  23 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-msm8660-mpm.c             | 740 ++++++++++++++++++
 include/soc/qcom/msm8660-mpm.h                |  83 ++
 5 files changed, 969 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml
 create mode 100644 drivers/irqchip/irq-msm8660-mpm.c
 create mode 100644 include/soc/qcom/msm8660-mpm.h
Signature · 1 line
2.43.0
#52 lore raw

PATCH v2 1/2 dt-bindings: interrupt-controller: qcom: add msm8660-mpm

Add the binding for the Modem Power Manager (MPM) interrupt controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The MPM is a small wake-capable interrupt aggregator that lets the SoC stay in low-power states while a small set of GIC SPIs continues to be monitored and can wake the system on an edge.

The binding describes its register region (in the RPM syscon), the GIC SPI it raises on a wake event, the GIC mailbox it shares with the RPM, and the per-pin wake-source mapping table.

---
 .../qcom,msm8660-mpm.yaml                     | 122 ++++++++++++++++++
 1 file changed, 122 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +122 -0
new file mode 100644
index 000000000000..1288e83d75d6
@@ -0,0 +1,122 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/qcom,msm8660-mpm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm MSM8x60 MPM wakeup interrupt controller
+
+maintainers:
+  - Herman van Hazendonk <[email protected]>
+
+description: |
+  The MSM Power Manager (MPM) on the MSM8x60 family (MSM8260/MSM8660/
+  APQ8060) is an always-on hardware block that keeps a set of wake
+  interrupts alive while the application processor is powered down.
+
+  The vMPM (virtual MPM) registers live INSIDE the RPM control block:
+  the request window (ENABLE, DETECT_CTL, POLARITY, CLEAR) starts at
+  RPM_BASE + 0x9d8 and the status window starts 0x420 above the
+  request window. The reg property points at this vMPM sub-region;
+  the qcom,rpm-msm8660 driver maps the surrounding RPM control area
+  for its own use, so the two mappings overlap but neither claims
+  exclusive ownership of the other's range.
+
+  Notification of new wake-source configuration is delivered via the
+  qcom-apcs-ipc mailbox (writing GCC + 0x008 bit 1).
+
+  This binding is MSM8x60-specific. Newer Qualcomm SoCs (sm6375, etc.)
+  use the qcom,mpm binding which assumes a dedicated MPM SRAM region
+  and an IPCC mailbox - neither holds on MSM8x60. The driver lives at
+  drivers/irqchip/irq-msm8660-mpm.c.
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    const: qcom,msm8660-mpm
+
+  reg:
+    maxItems: 1
+    description: |
+      vMPM register window inside the RPM control block. Must cover
+      the request registers at offset 0 of the window and the status
+      registers at offset 0x420 of the window. A length of 0x440 is
+      sufficient for the four-word request banks and the status bank.
+
+  interrupts:
+    maxItems: 1
+    description:
+      IPC IRQ raised by MPM when one of the enabled wake sources fires.
+      On MSM8x60 this is wired to GIC SPI 2.
+
+  mboxes:
+    maxItems: 1
+    description:
+      Mailbox channel used to notify MPM that the vMPM request
+      registers have been updated. On MSM8x60 this is the
+      qcom-apcs-ipc mailbox channel 1 (writes GCC + 0x008 bit 1).
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+    description:
+      First cell is the MPM pin number / parent GIC SPI; second cell
+      is the trigger type.
+
+  qcom,mpm-pin-count:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Total number of MPM pins exposed.
+
+  qcom,mpm-pin-map:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    items:
+      items:
+        - description: MPM pin number
+        - description: GIC SPI number this pin maps to
+    description: |
+      List of (MPM-pin, GIC-SPI) tuples for wake sources that have a
+      corresponding GIC IRQ. Consumers route their interrupts through
+      the MPM hierarchical irqdomain to take advantage of these
+      mappings.
+
+      Raw wake pins like SDC3/4 DATx (pins 21-24) are NOT listed
+      here - they have no GIC SPI mapping and are accessed through
+      the msm8660_mpm_set_pin_wake() / msm8660_mpm_enable_pin() C API
+      by consumer drivers.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - mboxes
+  - interrupt-controller
+  - '#interrupt-cells'
+  - qcom,mpm-pin-count
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        msm8660_mpm: interrupt-controller@1049d8 {
+            compatible = "qcom,msm8660-mpm";
+            reg = <0x001049d8 0x440>;
+            interrupts = <GIC_SPI 2 IRQ_TYPE_EDGE_RISING>;
+            mboxes = <&gcc_ipc 1>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+            interrupt-parent = <&intc>;
+            qcom,mpm-pin-count = <64>;
+            qcom,mpm-pin-map = <25 100>,
+                               <26 50>,
+                               <27 79>;
+        };
+    };
Signature · 1 line
2.43.0
#55 lore raw

PATCH v2 2/2 irqchip: add MSM8x60 MPM wakeup interrupt controller driver

In reply to Herman van Hazendonk

Add a driver for the MSM Power Manager (MPM) on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The MPM is a small wake-source controller implemented in the always-on power domain that latches edge-triggered interrupts during APPS power collapse and signals the RPM to wake the APPS back up.

The driver:
- implements an irqchip that wraps a parent GIC SPI line and

    overrides ->irq_set_wake() to enable the MPM mirror of the same
    interrupt;

- exposes msm8660_mpm_set_pin_wake() etc. for consumers (e.g. mmci's SDC4 SDIO wake) that do not flow through irq_set_wake() directly;
- communicates with the RPM via SMSM/syscon to apply the wake enable/clear bits and request a power collapse vote.

Used on the HP TouchPad (Tenderloin) for SDIO-attached Wi-Fi and panel touch wake-from-suspend.

---
 drivers/irqchip/Kconfig           |  23 +
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-msm8660-mpm.c | 740 ++++++++++++++++++++++++++++++
 include/soc/qcom/msm8660-mpm.h    |  83 ++++
 4 files changed, 847 insertions(+)
 create mode 100644 drivers/irqchip/irq-msm8660-mpm.c
 create mode 100644 include/soc/qcom/msm8660-mpm.h
drivers/irqchip/Kconfig +23 -0
index e755a2a05209..35598a56ac79 100644
@@ -525,6 +525,29 @@ config QCOM_MPM
 	  MSM Power Manager driver to manage and configure wakeup
 	  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
 
+config QCOM_MSM8660_MPM
+	bool "MSM8x60 MPM wakeup interrupt controller"
+	depends on ARCH_QCOM
+	depends on MFD_SYSCON
+	depends on MAILBOX
+	select IRQ_DOMAIN_HIERARCHY
+	default y
+	help
+	  Platform driver for the MSM Power Manager (MPM) wakeup interrupt
+	  controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The vMPM registers live inside
+	  the RPM control block, which makes the generic QCOM_MPM driver
+	  unusable here (it assumes a dedicated MPM SRAM region, an IPCC
+	  mailbox, and uses IRQCHIP_DECLARE early-init that races platform
+	  device creation).
+
+	  This driver replicates the legacy 2.6.35-palm arch/arm/mach-msm/
+	  mpm.c mechanism as a regular platform driver, accessing the vMPM
+	  registers via a syscon phandle to the RPM block and signaling the
+	  MPM via the qcom-apcs-ipc mailbox (writing to GCC + 0x008 bit 1).
+
+	  Required for cpuidle deep-sleep states (SPC/PC) and for
+	  WoWLAN-style wake-from-suspend on MSM8x60 SoCs.
+
 config CSKY_MPINTC
 	bool
 	depends on CSKY
drivers/irqchip/Makefile +1 -0
index 26aa3b6ec99f..3e64591f0f5b 100644
@@ -100,6 +100,7 @@ obj-$(CONFIG_MESON_IRQ_GPIO)		+= irq-meson-gpio.o
 obj-$(CONFIG_GOLDFISH_PIC) 		+= irq-goldfish-pic.o
 obj-$(CONFIG_QCOM_PDC)			+= qcom-pdc.o
 obj-$(CONFIG_QCOM_MPM)			+= irq-qcom-mpm.o
+obj-$(CONFIG_QCOM_MSM8660_MPM)		+= irq-msm8660-mpm.o
 obj-$(CONFIG_CSKY_MPINTC)		+= irq-csky-mpintc.o
 obj-$(CONFIG_CSKY_APB_INTC)		+= irq-csky-apb-intc.o
 obj-$(CONFIG_RISCV_INTC)		+= irq-riscv-intc.o
drivers/irqchip/irq-msm8660-mpm.c new file +740 -0

740 lines not on this page · raw message

include/soc/qcom/msm8660-mpm.h new file +83 -0

83 lines not on this page · raw message

Signature · 1 line
2.43.0
#46 lore raw

PATCH v2 1/3 dt-bindings: mfd: qcom-pm8xxx: allow temp-alarm subnode

In reply to Herman van Hazendonk

The qcom,pm8xxx parent schema closes its child set via additionalProperties: false and an explicit list of patternProperties. PM8901 (and prospectively other parts in the family) exposes an over-temperature alarm block as an SSBI sub-node; without a matching pattern here, any board DT that instantiates temp-alarm@<offset> under a PM8xxx parent fails dtbs_check.

Add the temp-alarm@[0-9a-f]+$ pattern, referencing the new qcom,pm8901-temp-alarm schema, so the temperature-alarm sub-node validates as a recognised child of the PMIC.

---
 Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml | 4 ++++
 1 file changed, 4 insertions(+)
Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml +4 -0
index 63e18d6a9c21..dde290569b03 100644
@@ -68,6 +68,10 @@ patternProperties:
     type: object
     $ref: /schemas/rtc/qcom-pm8xxx-rtc.yaml#
 
+  "temp-alarm@[0-9a-f]+$":
+    type: object
+    $ref: /schemas/thermal/qcom,pm8901-temp-alarm.yaml#
+
   "vibrator@[0-9a-f]+$":
     type: object
     $ref: /schemas/input/qcom,pm8xxx-vib.yaml#
Signature · 1 line
2.43.0
#48 lore raw

PATCH v2 2/3 dt-bindings: thermal: qcom: add pm8901-temp-alarm

In reply to Herman van Hazendonk

Add the binding for the temperature-alarm block inside the Qualcomm PM8901 PMIC (companion to the PM8058 on MSM8x60). The block has four selectable thresholds and three escalating stages; the driver maps these to representative millicelsius readings exposed to the thermal-of framework, and a board DT can wire stage 3 as a critical trip so the kernel issues orderly_poweroff() when the part overheats.

The binding describes the SSBI sub-node address (CTRL register offset) and the two PMIC-internal interrupts the alarm raises:

- TEMP_ALARM (PM8901 IRQ block 6 bit 4 == hwirq 52),

                  asserted on every stage transition;

- TEMP_HI_ALARM (PM8901 IRQ block 6 bit 5 == hwirq 53), asserted when the high-temperature stage is reached.

The interrupts are sourced from the parent qcom,pm8901 PMIC's own interrupt-controller (not the SoC GIC); the node references the core /schemas/thermal/thermal-sensor.yaml so that the standard #thermal-sensor-cells handling and other shared thermal-sensor constraints are inherited automatically.

---
 .../thermal/qcom,pm8901-temp-alarm.yaml       | 90 +++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml
Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml new file +90 -0
new file mode 100644
index 000000000000..5d9eaeab8326
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/qcom,pm8901-temp-alarm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm PM8901 PMIC Temperature Alarm
+
+maintainers:
+  - Herman van Hazendonk <[email protected]>
+
+description: |
+  PM8901 is a secondary PMIC paired with PM8058 on MSM8x60 family
+  (MSM8260/MSM8660/APQ8060) platforms. It exposes an over-temperature
+  alarm block at SSBI offset 0x23 (CTRL) with four selectable
+  thresholds and three escalating stages.
+
+  Unlike PM8058, there is no raw die-temperature ADC channel - the
+  driver decodes the stage + threshold pair into a representative
+  millicelsius value reported via the thermal-of framework.
+
+  Two PMIC-internal interrupts are exposed:
+
+    - TEMP_ALARM    (PM8901 IRQ block 6 bit 4 == hwirq 52): asserted
+                    on every stage transition;
+    - TEMP_HI_ALARM (PM8901 IRQ block 6 bit 5 == hwirq 53): asserted
+                    when the part enters the high-temperature stage.
+
+  Both line up on the parent qcom,pm8901 interrupt-controller.
+
+allOf:
+  - $ref: /schemas/thermal/thermal-sensor.yaml#
+
+properties:
+  compatible:
+    const: qcom,pm8901-temp-alarm
+
+  reg:
+    description: SSBI offset of the temp-alarm CTRL register.
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: Stage-transition alarm interrupt (TEMP_ALARM).
+      - description: Hi-temperature alarm interrupt (TEMP_HI_ALARM).
+
+  interrupt-names:
+    items:
+      - const: alarm
+      - const: hi-alarm
+
+  "#thermal-sensor-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    ssbi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      pmic@0 {
+        compatible = "qcom,pm8901";
+        reg = <0>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        #interrupt-cells = <2>;
+        interrupt-controller;
+        interrupt-parent = <&tlmm>;
+        interrupts = <31 IRQ_TYPE_EDGE_RISING>;
+
+        temp-alarm@23 {
+          compatible = "qcom,pm8901-temp-alarm";
+          reg = <0x23>;
+          interrupts = <52 IRQ_TYPE_EDGE_RISING>,
+                       <53 IRQ_TYPE_EDGE_RISING>;
+          interrupt-names = "alarm", "hi-alarm";
+          #thermal-sensor-cells = <0>;
+        };
+      };
+    };
Signature · 1 line
2.43.0
#49 lore raw

PATCH v2 3/3 thermal: qcom: add PM8901 PMIC temperature-alarm driver

In reply to Herman van Hazendonk

Add a thermal-of sensor driver for the temperature-alarm block inside the Qualcomm PM8901 PMIC. PM8901 is a secondary PMIC paired with PM8058 on the MSM8x60 family (MSM8260/MSM8660/APQ8060). It exposes an over-temperature alarm at SSBI offset 0x23/0x24 with three escalating stages (105/125/145 C); the driver decodes the stage + threshold pair into a millicelsius reading and registers two PMIC- internal interrupts (TEMP_ALARM at block 6 bit 4, TEMP_HI_ALARM at block 6 bit 5).

Used by board thermal-zones for the orderly_poweroff path on the HP TouchPad.

---
 drivers/thermal/qcom/Kconfig          |  12 +
 drivers/thermal/qcom/Makefile         |   1 +
 drivers/thermal/qcom/qcom-pm8901-tm.c | 408 ++++++++++++++++++++++++++
 3 files changed, 421 insertions(+)
 create mode 100644 drivers/thermal/qcom/qcom-pm8901-tm.c
drivers/thermal/qcom/Kconfig +12 -0
index a6bb01082ec6..af099032f1e6 100644
@@ -32,6 +32,18 @@ config QCOM_SPMI_TEMP_ALARM
 	  real time die temperature if an ADC is present or an estimate of the
 	  temperature based upon the over temperature stage value.
 
+config QCOM_PM8901_TEMP_ALARM
+	tristate "Qualcomm PM8901 PMIC Temperature Alarm"
+	depends on MFD_PM8XXX || COMPILE_TEST
+	depends on THERMAL_OF
+	help
+	  This enables the thermal driver for the PM8901 PMIC over-temperature
+	  alarm block. PM8901 exposes a stage-based alarm (no raw ADC) with
+	  four selectable thresholds and three escalating stages. The driver
+	  registers a thermal-of sensor so a board device tree can declare
+	  trip points and a critical-trip action (orderly_poweroff). Used on
+	  HP TouchPad (APQ8060) where PM8901 supplies the secondary PMIC die.
+
 config QCOM_LMH
 	tristate "Qualcomm Limits Management Hardware"
 	depends on ARCH_QCOM || COMPILE_TEST
drivers/thermal/qcom/Makefile +1 -0
index 0fa2512042e7..90dc05151e33 100644
@@ -5,4 +5,5 @@ qcom_tsens-y			+= tsens.o tsens-v2.o tsens-v1.o tsens-v0_1.o \
 				   tsens-8960.o
 obj-$(CONFIG_QCOM_SPMI_ADC_TM5)	+= qcom-spmi-adc-tm5.o
 obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
+obj-$(CONFIG_QCOM_PM8901_TEMP_ALARM)	+= qcom-pm8901-tm.o
 obj-$(CONFIG_QCOM_LMH)		+= lmh.o
drivers/thermal/qcom/qcom-pm8901-tm.c new file +408 -0

408 lines not on this page · raw message

Signature · 1 line
2.43.0
#34 lore raw

Re: [PATCH v2 1/3] dt-bindings: clock: qcom,lcc: add MSM8x60 family compatibles

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Missing overarching schema updates in `qcom-soc.yaml` for legacy-formatted compatible strings. --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing overarching schema updates in qcom-soc.yaml for legacy-formatted compatible strings. --

commit 5610427121060301089bb1b2b018a18be452795c
Author: Herman van Hazendonk <[email protected]>

dt-bindings: clock: qcom,lcc: add MSM8x60 family compatibles

This patch adds MSM8x60 family compatibles to the qcom,lcc dt-bindings schema. It includes them in the main enum and the existing parent-clock-name conditional block that already covers apq8064 and msm8960.

Quoted 5 lines
Documentation/devicetree/bindings/clock/qcom,lcc.yaml +0 -0
index 55985e562a34f..bbdad157f03fe 100644

Documentation/devicetree/bindings/clock/qcom,lcc.yaml maintainers: · line 21

Quoted 10 lines
Documentation/devicetree/bindings/clock/qcom,lcc.yaml +3 -0
@@ -12,9 +12,12 @@ maintainers:
 properties:
   compatible:
     enum:
+      - qcom,lcc-apq8060
       - qcom,lcc-apq8064
       - qcom,lcc-ipq8064
       - qcom,lcc-mdm9615
+      - qcom,lcc-msm8260
+      - qcom,lcc-msm8660
       - qcom,lcc-msm8960

[Severity: Low]
Does Documentation/devicetree/bindings/arm/qcom-soc.yaml also need to be updated to include these new compatibles?

Because these new compatibles use the legacy qcom,<ip>-<soc> format, they fail the preferred pattern validation in qcom-soc.yaml. Usually, legacy formatted strings are whitelisted in the overarching qcom-soc.yaml schema under properties/compatible/oneOf/enum, where qcom,lcc-msm8960 and others currently reside:

Documentation/devicetree/bindings/arm/qcom-soc.yaml:
- enum:
- qcom,dsi-ctrl-6g-qcm2290
- qcom,gpucc-sdm630
- qcom,gpucc-sdm660
- qcom,lcc-apq8064
- qcom,lcc-ipq8064
- qcom,lcc-mdm9615
- qcom,lcc-msm8960

Will omitting them from this whitelist cause dtbs_check validation to fail when evaluating board device trees?

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#53 lore raw

Re: [PATCH v2 1/2] dt-bindings: interrupt-controller: qcom: add msm8660-mpm

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] Missing memory barrier and doorbell notification to RPM after clearing pending interrupts.…

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing memory barrier and doorbell notification to RPM after clearing pending interrupts.
- [Medium] The qcom,mpm-pin-map property is required by the driver but missing from the YAML schema's required list, while qcom,mpm-pin-count is strictly required by the schema but ignored by the driver.
- [Medium] The device tree binding defines a reg property that explicitly overlaps with the RPM control block, an MFD/syscon architectural anti-pattern.
- [Medium] The binding and driver omit hardware wake pins from the DT mapping, relying instead on a custom driver C API.
--

commit ff9b33b822acb8b5cceef507bbe942c3ff3618b7
Author: Herman van Hazendonk <[email protected]>

dt-bindings: interrupt-controller: qcom: add msm8660-mpm

Add the binding for the Modem Power Manager (MPM) interrupt controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The MPM is a small wake-capable interrupt aggregator that lets the SoC stay in low-power states while a small set of GIC SPIs continues to be monitored and can wake the system on an edge.

Quoted 5 lines
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +0 -0
new file mode 100644
index 0000000000000..1288e83d75d6b

[ ... ]

Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml line 46

Quoted 11 lines
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +11 -0
+properties:
+  compatible:
+    const: qcom,msm8660-mpm
+
+  reg:
+    maxItems: 1
+    description: |
+      vMPM register window inside the RPM control block. Must cover
+      the request registers at offset 0 of the window and the status
+      registers at offset 0x420 of the window. A length of 0x440 is
+      sufficient for the four-word request banks and the status bank.

[Severity: Medium]
Does this explicit overlap violate standard Device Tree architecture?

Overlapping hardware resources should typically be modeled as child nodes of an MFD sharing a regmap, or accessed via a syscon phandle. By directly using devm_ioremap() in the driver to bypass standard resource reservation, this introduces an architectural anti-pattern:

drivers/irqchip/irq-msm8660-mpm.c:msm8660_mpm_probe() {
/*
* Use a non-exclusive mapping: the qcom,rpm driver maps the
* surrounding RPM control block via its own platform resource,
* and our vMPM sub-region overlaps that mapping. devm_ioremap()
* does not call request_mem_region() so there is no conflict. */
mpm->base = devm_ioremap(dev, res->start, resource_size(res));
}

[ ... ]

Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml line 52

Quoted 5 lines
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +5 -0
+  interrupts:
+    maxItems: 1
+    description:
+      IPC IRQ raised by MPM when one of the enabled wake sources fires.
+      On MSM8x60 this is wired to GIC SPI 2.

[Severity: High]
Does the interrupt handler in the driver lack a memory barrier and doorbell notification after clearing pending interrupts?

Looking at drivers/irqchip/irq-msm8660-mpm.c:msm8660_mpm_irq(), the CLEAR register is written using a relaxed IO accessor:

drivers/irqchip/irq-msm8660-mpm.c:msm8660_mpm_irq() {
...

		if (pending[i])
			msm8660_mpm_write(mpm,
				MSM8660_MPM_REG_CLEAR + i * 4, pending[i]);

...
}

Since the RPM caches the vMPM request registers, won't it miss the CLEAR command without a doorbell notification, causing it to continue asserting the IPC wake interrupt?

Also, without a memory barrier, this might reorder against the subsequent parent GIC EOI. Could this cause an immediate spurious interrupt storm since the parent IPC interrupt is level-triggered?

[ ... ]

Quoted 6 lines
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +6 -0
+  qcom,mpm-pin-map:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    items:
+      items:
+        - description: MPM pin number
+        - description: GIC SPI number this pin maps to

Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml line 88

Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +10 -0
+    description: |
+      List of (MPM-pin, GIC-SPI) tuples for wake sources that have a
+      corresponding GIC IRQ. Consumers route their interrupts through
+      the MPM hierarchical irqdomain to take advantage of these
+      mappings.
+
+      Raw wake pins like SDC3/4 DATx (pins 21-24) are NOT listed
+      here - they have no GIC SPI mapping and are accessed through
+      the msm8660_mpm_set_pin_wake() / msm8660_mpm_enable_pin() C API
+      by consumer drivers.

[Severity: Medium]
Is it correct to omit hardware wake pins from the device tree mapping and rely on a custom C API?

Device tree bindings should describe hardware resources completely and independently of OS implementation. Omitting pins just because they don't map to a GIC SPI forces consumers to be tightly coupled to this specific driver implementation via C symbol exports:

drivers/irqchip/irq-msm8660-mpm.c:msm8660_mpm_enable_pin()

Would it be better to list these pins and avoid bypassing standard kernel subsystems like dummy IRQ domains or pinctrl frameworks?

[ ... ]

Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml line 97

Quoted 8 lines
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +8 -0
+required:
+  - compatible
+  - reg
+  - interrupts
+  - mboxes
+  - interrupt-controller
+  - '#interrupt-cells'
+  - qcom,mpm-pin-count

[Severity: Medium]
Are the required properties synchronized with the driver's expectations?

The schema enforces qcom,mpm-pin-count, but the driver ignores this property and hardcodes the pin count to 64:

drivers/irqchip/irq-msm8660-mpm.c:
#define MSM8660_MPM_PIN_COUNT 64

Conversely, the driver's probe unconditionally fails if qcom,mpm-pin-map is missing, returning -EINVAL:

drivers/irqchip/irq-msm8660-mpm.c:msm8660_mpm_probe() {
...

	ret = of_property_count_u32_elems(np, "qcom,mpm-pin-map");
	if (ret < 0 || ret % 2)
		return dev_err_probe(dev, -EINVAL,
				     "invalid qcom,mpm-pin-map\n");

...
}

Should qcom,mpm-pin-map be added to the required list to prevent invalid device trees from silently passing schema validation?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#37 lore raw

Re: [PATCH v2 2/3] dt-bindings: clock: qcom: add lcc-msm8660 LPASS clock IDs

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The new `qcom,lcc-msm8660`, `qcom,lcc-msm8260`, and `qcom,lcc-apq8060` compatibles were not added…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The new qcom,lcc-msm8660, qcom,lcc-msm8260, and qcom,lcc-apq8060 compatibles were not added to the legacy naming enumerations in qcom-soc.yaml. --

commit f40b14d3bc25d211dba1f4b6ea323fcbe8e84acb
Author: Herman van Hazendonk <[email protected]>

dt-bindings: clock: qcom: add lcc-msm8660 LPASS clock IDs

Add the dt-binding clock-ID header for the MSM8x60 family (MSM8260/MSM8660/APQ8060) Low Power Audio SubSystem Clock Controller (LCC). It mirrors the format and ID range of the existing LCC headers for newer Qualcomm SoCs.

Quoted 9 lines
include/dt-bindings/clock/qcom,lcc-msm8660.h new file +3 -0
new file mode 100644
index 0000000000000..d5d9b0d71a78f
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.

include/dt-bindings/clock/qcom,lcc-msm8660.h line 13

Quoted 10 lines
include/dt-bindings/clock/qcom,lcc-msm8660.h new file +10 -0
+ */
+
+#ifndef _DT_BINDINGS_CLK_LCC_MSM8660_H
+#define _DT_BINDINGS_CLK_LCC_MSM8660_H
+
+/*
+ * MSM8x60 family (MSM8260/MSM8660/APQ8060) LPASS Clock Controller (LCC)
+ * clock IDs. These are compatible with MSM8960 LCC as MSM8x60 and
+ * MSM8960 share the same audio subsystem clock architecture.
+ */

[Severity: Low]
Since this series introduces new compatibles for the MSM8x60 family (qcom,lcc-msm8660, qcom,lcc-msm8260, and qcom,lcc-apq8060), do these legacy format strings also need to be added to the generic SoC naming constraints allowlist?

Without adding them to Documentation/devicetree/bindings/arm/qcom-soc.yaml, will device tree schema validation fail for boards using them? They match the generic regex but aren't in the legacy permitted list:

Documentation/devicetree/bindings/arm/qcom-soc.yaml:
- enum:
- qcom,dsi-ctrl-6g-qcm2290
- qcom,gpucc-sdm630
- qcom,gpucc-sdm660
- qcom,lcc-apq8064
- qcom,lcc-ipq8064
- qcom,lcc-mdm9615
- qcom,lcc-msm8960

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#56 lore raw

Re: [PATCH v2 2/2] irqchip: add MSM8x60 MPM wakeup interrupt controller driver

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider: - [High] Unprotected read-modify-write operations on shared vMPM registers lead to lost configuration…

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Unprotected read-modify-write operations on shared vMPM registers lead to lost configuration updates.
- [High] Calling mbox_send_message() from irqchip callbacks causes a sleeping-while-atomic deadlock on PREEMPT_RT.
- [High] Identity confusion between MPM pin numbers and parent GIC SPI numbers leads to allocation failures or dropped interrupts.
- [High] Toggling the hardware ENABLE bit inside irq_set_wake inadvertently disables unmasked interrupts upon resume.
- [High] Missing suppress_bind_attrs allows sysfs unbind, causing Use-After-Free via irq_domain_remove and concurrent probes.
- [Medium] Missing read-back flush after clearing MPM interrupt status using writel_relaxed.
- [Low] Phandle resolution ignores the actual target node, leading to incorrect device linking.
--

commit c09bf824269ef596633ac65838f83c2f22429764
Author: Herman van Hazendonk <[email protected]>

irqchip: add MSM8x60 MPM wakeup interrupt controller driver

Add a driver for the MSM Power Manager (MPM) on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The MPM latches edge-triggered interrupts during APPS power collapse and signals the RPM to wake the APPS back up.

Quoted 6 lines
drivers/irqchip/irq-msm8660-mpm.c new file +0 -0
new file mode 100644
index 0000000000000..b8394d9c076d2
@@ -0,0 +1,740 @@

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 141

Quoted 8 lines
drivers/irqchip/irq-msm8660-mpm.c new file +8 -0
+static void msm8660_mpm_doorbell(struct msm8660_mpm *mpm)
+{
+	int ret;
+
+	if (!mpm->mbox_chan)
+		return;
+
+	ret = mbox_send_message(mpm->mbox_chan, NULL);

[Severity: High]
Does calling mbox_send_message() from within an irqchip callback cause a sleeping-while-atomic deadlock on PREEMPT_RT?

The irq_mask, irq_unmask, and irq_set_type callbacks run with the raw_spinlock_t irq_desc->lock held.

The mailbox core conditionally waits and takes chan->lock, which is a spinlock_t that gets converted to a sleepable RT-mutex on PREEMPT_RT.

Deferring the mailbox communication using irq_bus_lock and irq_bus_sync_unlock could avoid this deadlock.

[ ... ]

Quoted 2 lines
drivers/irqchip/irq-msm8660-mpm.c new file +2 -0
+static irqreturn_t msm8660_mpm_irq(int irq, void *data)
+{

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 188

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+		/*
+		 * Clear before dispatching: a new edge latched on this pin
+		 * after this point will set the pending bit again and we
+		 * will service it on the next IPC IRQ. Clearing AFTER the
+		 * handler would race with that new latch and silently lose
+		 * the new edge.
+		 */
+		if (pending[i])
+			msm8660_mpm_write(mpm,
+				MSM8660_MPM_REG_CLEAR + i * 4, pending[i]);

[Severity: Medium]
Can a delayed write in the CPU store buffer overwrite newly latched edges and drop interrupts?

Using writel_relaxed does not flush the write over the interconnect. If a child driver triggers a new edge on a peripheral during the handler, the MPM hardware will latch it.

If the delayed REG_CLEAR write arrives afterward, it will incorrectly wipe out the newly latched edge. Adding a dummy read from the MPM register space ensures the write completes before executing the handler.

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 219

Quoted 6 lines
drivers/irqchip/irq-msm8660-mpm.c new file +6 -0
+static int msm8660_mpm_hwirq_to_pin(struct msm8660_mpm *mpm, unsigned int hwirq)
+{
+	int i;
+
+	for (i = 0; i < mpm->pin_map_count; i++) {
+		if (mpm->pin_map[i].hwirq == hwirq)

[Severity: High]
Is the GIC SPI number being confused with the MPM pin number here?

When msm8660_mpm_set_type calls msm8660_mpm_hwirq_to_pin(mpm, d->hwirq), it searches for mpm->pin_map[i].hwirq == hwirq.

Because d->hwirq is the MPM pin and pin_map[i].hwirq is the GIC SPI, this lookup will fail and return -1, causing set_type to return -ENOENT.

Quoted 8 lines
drivers/irqchip/irq-msm8660-mpm.c new file +8 -0
+			return mpm->pin_map[i].pin;
+	}
+	return -1;
+}
+
+static void msm8660_mpm_enable_hwirq(struct irq_data *d, bool enable)
+{
+	struct msm8660_mpm *mpm = irq_data_get_irq_chip_data(d);

drivers/irqchip/irq-msm8660-mpm.c line 237

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+	int pin;
+	u32 val, mask;
+
+	pin = msm8660_mpm_hwirq_to_pin(mpm, d->hwirq);
+	if (pin < 0)
+		return;
+
+	mask = BIT(pin % 32);
+	val = msm8660_mpm_read(mpm,
+		MSM8660_MPM_REG_ENABLE + (pin / 32) * 4);

[Severity: High]
Could this concurrent read-modify-write corrupt the shared register state?

The driver manipulates shared 32-bit registers (ENABLE, DETECT_CTL, POLARITY) to configure individual pins.

Because the IRQ subsystem calls irq_mask and irq_unmask with the per-interrupt irq_desc->lock held, concurrent mask/unmask operations on different interrupts within the same bank will execute without mutual exclusion.

A driver-local raw_spinlock_t is needed to serialize these accesses.

drivers/irqchip/irq-msm8660-mpm.c line 245

Quoted 8 lines
drivers/irqchip/irq-msm8660-mpm.c new file +8 -0
+	if (enable)
+		val |= mask;
+	else
+		val &= ~mask;
+	msm8660_mpm_write(mpm,
+		MSM8660_MPM_REG_ENABLE + (pin / 32) * 4, val);
+
+	msm8660_mpm_doorbell(mpm);

[Severity: High]
Is it safe to invoke the doorbell from a context where a raw_spinlock_t is held?

The irq_mask and irq_unmask callbacks execute with irq_desc->lock held.

Calling msm8660_mpm_doorbell() here will invoke mbox_send_message(), which sleeps on PREEMPT_RT, causing a scheduling-while-atomic deadlock.

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 272

Quoted 13 lines
drivers/irqchip/irq-msm8660-mpm.c new file +13 -0
+static int msm8660_mpm_set_type(struct irq_data *d, unsigned int type)
+{
+	struct msm8660_mpm *mpm = irq_data_get_irq_chip_data(d);
+	u32 detect, polarity, mask;
+	int pin;
+
+	pin = msm8660_mpm_hwirq_to_pin(mpm, d->hwirq);
+	if (pin < 0)
+		return -ENOENT;
+
+	mask = BIT(pin % 32);
+	detect = msm8660_mpm_read(mpm,
+		MSM8660_MPM_REG_DETECT_CTL + (pin / 32) * 4);

[Severity: High]
Does this unlocked read-modify-write of the DETECT_CTL and POLARITY registers race with other set_type calls?

Similar to the ENABLE register, these are shared across multiple pins and need mutual exclusion to prevent lost configuration updates.

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 324

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+static int msm8660_mpm_set_wake(struct irq_data *d, unsigned int on)
+{
+	struct msm8660_mpm *mpm = irq_data_get_irq_chip_data(d);
+	int pin;
+
+	pin = msm8660_mpm_hwirq_to_pin(mpm, d->hwirq);
+	if (pin < 0)
+		return -ENOENT;
+
+	msm8660_mpm_enable_hwirq(d, !!on);

[Severity: High]
Will toggling the hardware ENABLE bit here cause the interrupt to remain permanently disabled after the first system resume?

When the system suspends, IRQCHIP_MASK_ON_SUSPEND ensures non-wake interrupts are logically masked. During resume, device drivers call irq_set_wake(..., 0) to disable wakeup.

Since this writes 0 to the hardware ENABLE bit, but the IRQ core assumes the interrupt is still logically unmasked, it will not call irq_unmask() and the hardware state will remain disabled.

[ ... ]

Quoted 4 lines
drivers/irqchip/irq-msm8660-mpm.c new file +4 -0
+static int msm8660_mpm_domain_alloc(struct irq_domain *domain,
+				    unsigned int virq, unsigned int nr_irqs,
+				    void *data)
+{

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 362

Quoted 4 lines
drivers/irqchip/irq-msm8660-mpm.c new file +4 -0
+	parent_fwspec.fwnode = domain->parent->fwnode;
+	parent_fwspec.param_count = 3;
+	parent_fwspec.param[0] = 0;
+	parent_fwspec.param[1] = hwirq;

[Severity: High]
Does passing the MPM pin number directly to the parent GIC domain map the wrong interrupt?

The parent GIC domain expects a GIC SPI number in param[1], not the MPM pin number extracted from fwspec->param[0]. The correct mapped SPI should be looked up and passed here instead.

[ ... ]

Quoted 5 lines
drivers/irqchip/irq-msm8660-mpm.c new file +5 -0
+struct msm8660_mpm *msm8660_mpm_get(struct device *consumer,
+				    struct device_node *np,
+				    const char *propname)
+{
+	struct device_node *mpm_np;

drivers/irqchip/irq-msm8660-mpm.c line 427

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+	struct device_link *link;
+
+	if (!msm8660_mpm_global)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	if (np && propname) {
+		mpm_np = of_parse_phandle(np, propname, 0);
+		if (!mpm_np)
+			return ERR_PTR(-ENOENT);
+		of_node_put(mpm_np);

[Severity: Low]
Should this function verify that the resolved mpm_np actually matches msm8660_mpm_global->dev->of_node?

If a consumer's DT property points to a completely different node by mistake, it appears this code will still drop the reference and successfully return the global MPM handle anyway.

drivers/irqchip/irq-msm8660-mpm.c line 434

Quoted 7 lines
drivers/irqchip/irq-msm8660-mpm.c new file +7 -0
+	}
+
+	if (!consumer)
+		return msm8660_mpm_global;
+
+	link = device_link_add(consumer, msm8660_mpm_global->dev,
+			       DL_FLAG_AUTOREMOVE_CONSUMER);

[Severity: High]
Can msm8660_mpm_global->dev be dereferenced safely here without a lock?

If this races with a sysfs unbind of the MPM driver, msm8660_mpm_global could be set to NULL or freed, leading to a use-after-free.

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 464

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+int msm8660_mpm_enable_pin(struct msm8660_mpm *mpm, unsigned int pin,
+			   bool enable)
+{
+	u32 val;
+
+	if (!mpm || pin >= MSM8660_MPM_PIN_COUNT)
+		return -EINVAL;
+
+	val = msm8660_mpm_read(mpm,
+		MSM8660_MPM_REG_ENABLE + (pin / 32) * 4);

[Severity: High]
Does this raw-pin consumer API modify the ENABLE register without locking?

Concurrent calls from different consumer drivers could overwrite each other's enable states without mutual exclusion.

[ ... ]

Quoted 7 lines
drivers/irqchip/irq-msm8660-mpm.c new file +7 -0
+static void msm8660_mpm_remove(struct platform_device *pdev)
+{
+	struct msm8660_mpm *mpm = platform_get_drvdata(pdev);
+
+	/*
+	 * Tear down in strict reverse order: drop the singleton so new
+	 * consumers cannot grab a handle, free the IRQ so the handler

drivers/irqchip/irq-msm8660-mpm.c line 709

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+	 * cannot fire again, free the mailbox channel, then remove the
+	 * domain. Consumer device_links established in msm8660_mpm_get()
+	 * prevent the parent device from being unbound while a consumer
+	 * still holds a handle.
+	 */
+	msm8660_mpm_global = NULL;
+	free_irq(mpm->parent_irq, mpm);
+	if (mpm->mbox_chan)
+		mbox_free_channel(mpm->mbox_chan);
+	irq_domain_remove(mpm->domain);

[Severity: High]
Does allowing sysfs unbind lead to a use-after-free in the core IRQ subsystem?

While consumer drivers are unbound due to the device_link, their virtual IRQ mappings persist in the core IRQ subsystem because they do not automatically call irq_dispose_mapping().

Removing the domain here frees the domain memory, but the core IRQ subsystem retains dangling pointers to it in irq_data structures.

Quoted 1 line

+}

[ ... ]

drivers/irqchip/irq-msm8660-mpm.c line 722

Quoted 5 lines
drivers/irqchip/irq-msm8660-mpm.c new file +5 -0
+static struct platform_driver msm8660_mpm_driver = {
+	.probe		= msm8660_mpm_probe,
+	.remove		= msm8660_mpm_remove,
+	.driver		= {
+		.name	= "msm8660-mpm",

[Severity: High]
Should this driver set .suppress_bind_attrs = true?

Without it, the driver can be unbound via sysfs, which triggers the irq_domain_remove() use-after-free issue described above.

Quoted 3 lines
drivers/irqchip/irq-msm8660-mpm.c new file +3 -0
+		.of_match_table = msm8660_mpm_of_match,
+	},
+};
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#39 lore raw

Re: [PATCH v2 3/3] clk: qcom: add MSM8x60 LCC (LPASS) driver

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing Clock Parent pll4_vote leading to orphaned clocks.

Pre-existing issues:
- [High] Race condition between RCG clock set_rate and Branch clock enable/disable sharing the same register. --

commit 0c6b3f0cf3eb1cc15e3add96b0433ff823210d70
Author: Herman van Hazendonk <[email protected]>

clk: qcom: add MSM8x60 LCC (LPASS) driver

This commit adds a clock driver for the LPASS Clock Controller on the MSM8x60 family. It handles the specific register layout, muxing, and divider topology for this generation, ensuring LPASS audio DSP clocks work correctly before audio playback or capture.

Quoted 4 lines
drivers/clk/qcom/lcc-msm8660.c new file +0 -0
new file mode 100644

[ ... ]

drivers/clk/qcom/lcc-msm8660.c line 64

Quoted 4 lines
drivers/clk/qcom/lcc-msm8660.c new file +4 -0
+static const struct clk_parent_data lcc_pxo_pll4[] = {
+	{ .fw_name = "pxo", .name = "pxo_board" },
+	{ .fw_name = "pll4_vote", .name = "pll4_vote" },
+};

[Severity: High]
Does this correctly reference the local PLL4?

The driver registers the local PLL simply as "pll4" (via clk_pll pll4), and unlike MSM8960, the MSM8660 platform doesn't appear to export a "pll4_vote" clock from the global clock controller.

Because the clock framework cannot find "pll4_vote", will parent resolution fail and leave the essential LPASS root clocks orphaned?

Should this refer to the local PLL using .hw = &pll4.clkr.hw or .name = "pll4" instead?

[ ... ]

Quoted 3 lines
drivers/clk/qcom/lcc-msm8660.c new file +3 -0
+#define CLK_AIF_OSR_SRC(prefix, _ns, _md)			\
+static struct clk_rcg prefix##_osr_src = {			\
+	.ns_reg = _ns,						\

[ ... ]

Quoted 7 lines
drivers/clk/qcom/lcc-msm8660.c new file +7 -0
+#define CLK_AIF_OSR_CLK(prefix, _ns, hr, en_bit)		\
+static struct clk_branch prefix##_osr_clk = {			\
+	.halt_reg = hr,						\
+	.halt_bit = 1,						\
+	.halt_check = BRANCH_HALT_ENABLE,			\
+	.clkr = {						\
+		.enable_reg = _ns,				\

[ ... ]

drivers/clk/qcom/lcc-msm8660.c line 215

Quoted 2 lines
drivers/clk/qcom/lcc-msm8660.c new file +2 -0
+CLK_AIF_OSR_SRC(mi2s, 0x48, 0x4c)
+CLK_AIF_OSR_CLK(mi2s, 0x48, 0x50, 17)

[Severity: High]
This is a pre-existing issue, but does sharing the ns_reg (e.g., 0x48) between the RCG clock and Branch clock expose a race condition?

The RCG clock's set_rate callback (__clk_rcg_set_rate) performs a non-atomic read-modify-write on this register protected only by the global CCF prepare_lock mutex.

Concurrently, a branch clock's enable callback (clk_enable_regmap) performs an atomic regmap_update_bits on the same register protected by the global CCF enable_lock spinlock.

Since these two locks don't serialize against each other, can an interrupt-driven clk_enable preempt the clk_set_rate thread between its read and write steps?

If clk_set_rate writes its stale value back, it could revert the branch clock's hardware enable state, potentially triggering a fatal external abort on unclocked register access.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
#45 lore raw

Re: [PATCH v2 2/2] interconnect: qcom: add MSM8x60 NoC driver

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] Clock prepare/enable reference leak on probe deferral when RPM device lookup fails. - [High]…

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Clock prepare/enable reference leak on probe deferral when RPM device lookup fails.
- [High] Time-of-check to time-of-use (TOCTOU) race condition in msm8660_get_rpm leading to Use-After-Free.
- [High] Integer truncation in the fabric clock rate calculation allows unprivileged processes to freeze the interconnect bus.
- [Low] COUNT_ARGS(__VA_ARGS__) evaluates to 1 when no arguments are passed, creating bogus links to node ID 0. --

commit 3b79422423710904c503ef7d19429d69c2f02d18
Author: Herman van Hazendonk <[email protected]>

interconnect: qcom: add MSM8x60 NoC driver

Add a Qualcomm interconnect driver for the MSM8x60 family, modelling the four fabrics that connect masters and slaves on these Scorpion-class SoCs. The driver implements the interconnect-provider API to manage bandwidth between specific masters and slaves.

Quoted 6 lines
drivers/interconnect/qcom/msm8660.c new file +0 -0
new file mode 100644
index 0000000000000..174bc59da74f7
@@ -0,0 +1,1069 @@

[ ... ]

drivers/interconnect/qcom/msm8660.c line 287

Quoted 6 lines
drivers/interconnect/qcom/msm8660.c new file +6 -0
+#define DEFINE_QNODE(_name, _id, _buswidth, _mas, _slv, _tier, ...)	\
+	static struct msm8660_icc_node _name = {			\
+		.name = #_name,						\
+		.id = _id,						\
+		.buswidth = _buswidth,					\
+		.num_links = COUNT_ARGS(__VA_ARGS__),			\

[Severity: Low]
Does COUNT_ARGS(__VA_ARGS__) evaluate to 1 when invoked with no links?

When DEFINE_QNODE is invoked with an empty variadic argument list, this might receive a single empty token and initialize .links to { 0 }, causing a phantom node with ID 0 to be created.

[ ... ]

Quoted 2 lines
drivers/interconnect/qcom/msm8660.c new file +2 -0
+static int msm8660_icc_set(struct icc_node *src, struct icc_node *dst)
+{

[ ... ]

drivers/interconnect/qcom/msm8660.c line 750

Quoted 3 lines
drivers/interconnect/qcom/msm8660.c new file +3 -0
+	/* Apply minimum floor to prevent bus starvation */
+	rate = max_t(u64, rate, MSM8660_FABRIC_MIN_RATE);
+	rate = min_t(u32, rate, INT_MAX);

[Severity: High]
Can this truncation bypass the minimum floor limit?

If rate exceeds the 32-bit maximum (for example, during very high bandwidth requests), the cast to u32 in min_t() will truncate the upper bits. This could result in a near-zero clock rate instead of capping it, potentially freezing the interconnect bus.

[ ... ]

Quoted 2 lines
drivers/interconnect/qcom/msm8660.c new file +2 -0
+static struct qcom_rpm *msm8660_get_rpm(struct device *dev)
+{

[ ... ]

drivers/interconnect/qcom/msm8660.c line 816

Quoted 8 lines
drivers/interconnect/qcom/msm8660.c new file +8 -0
+	rpm_pdev = of_find_device_by_node(rpm_np);
+	of_node_put(rpm_np);
+	if (!rpm_pdev) {
+		dev_dbg(dev, "RPM device not found yet, deferring probe\n");
+		return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	rpm = dev_get_drvdata(&rpm_pdev->dev);

[Severity: High]
Could this lead to a race condition between retrieving the pointer and pinning the device?

Because dev_get_drvdata() retrieves the pointer before device_link_add() pins the supplier, a concurrent unbind and rebind of the supplier in this window could cause the original pointer to be freed, even though the link might succeed on the rebound instance, leading to a use-after-free.

[ ... ]

Quoted 2 lines
drivers/interconnect/qcom/msm8660.c new file +2 -0
+static int msm8660_icc_probe(struct platform_device *pdev)
+{

[ ... ]

Quoted 4 lines
drivers/interconnect/qcom/msm8660.c new file +4 -0
+	if (qp->num_clks) {
+		ret = clk_bulk_prepare_enable(qp->num_clks, qp->bus_clks);
+		if (ret) {
+			dev_warn(dev, "Failed to enable bus clocks: %d\n", ret);

drivers/interconnect/qcom/msm8660.c line 908

Quoted 10 lines
drivers/interconnect/qcom/msm8660.c new file +10 -0
+			qp->num_clks = 0;
+		}
+	}
+
+	/* Set up RPM fabric arbitration */
+	qp->desc = desc;
+	if (desc->rpm_resource >= 0) {
+		qp->rpm = msm8660_get_rpm(dev);
+		if (IS_ERR(qp->rpm))
+			return PTR_ERR(qp->rpm);

[Severity: High]
Does returning directly here leak the clock prepare/enable reference?

When msm8660_get_rpm() returns -EPROBE_DEFER, returning directly skips the rollback for the clk_bulk_prepare_enable() call made earlier. This will leak the clock reference on every probe retry.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#50 lore raw

Re: [PATCH v2 3/3] thermal: qcom: add PM8901 PMIC temperature-alarm driver

In reply to Herman van Hazendonk
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The driver disables the PMIC's hardware thermal auto-shutdown during probe but does not implement…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The driver disables the PMIC's hardware thermal auto-shutdown during probe but does not implement a .shutdown callback to restore it, leaving the device physically unprotected across warm reboots.
- [High] The driver overrides the PMIC's hardware thermal shutdown but fails to configure the thermal interrupts as system wakeup sources or restore the hardware shutdown during suspend, leaving the device unprotected while suspended. --

commit ea0822f7dd85ca701edd835f63cf382da86b80eb
Author: Herman van Hazendonk <[email protected]>

thermal: qcom: add PM8901 PMIC temperature-alarm driver

Add a thermal-of sensor driver for the temperature-alarm block inside the Qualcomm PM8901 PMIC. It exposes an over-temperature alarm at SSBI offset 0x23/0x24 with three escalating stages.

Quoted 5 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +0 -0
new file mode 100644
index 0000000000000..b159e9917c2c9

[ ... ]

drivers/thermal/qcom/qcom-pm8901-tm.c line 334

Quoted 9 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +9 -0
+	chip->tz_dev = devm_thermal_of_zone_register(&pdev->dev, 0, chip,
+						     &pm8901_tm_zone_ops);
+	if (IS_ERR(chip->tz_dev))
+		return dev_err_probe(&pdev->dev, PTR_ERR(chip->tz_dev),
+				     "thermal zone register failed\n");
+
+	ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL,
+					pm8901_tm_isr, IRQF_ONESHOT,
+					"pm8901-tm-alarm", chip);

[Severity: High]
Since the PMIC's hardware thermal shutdown is overridden by the software driver during probe, should these thermal interrupts be configured as system wakeup sources?

During system suspend, non-wakeup interrupts are disabled and the software thermal core sleeps. Without wakeup enabled, could the device overheat while suspended without the hardware or OS acting on it to prevent damage?

[ ... ]

drivers/thermal/qcom/qcom-pm8901-tm.c line 403

Quoted 13 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +13 -0
+static const struct of_device_id pm8901_tm_match_table[] = {
+	{ .compatible = "qcom,pm8901-temp-alarm" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pm8901_tm_match_table);
+
+static struct platform_driver pm8901_tm_driver = {
+	.driver = {
+		.name		= "pm8901-temp-alarm",
+		.of_match_table	= pm8901_tm_match_table,
+	},
+	.probe	= pm8901_tm_probe,
+};

[Severity: High]
Is a .shutdown callback needed in pm8901_tm_driver to restore the PMIC's hardware thermal auto-shutdown?

While pm8901_tm_enable_sw_override() registers a devm action to restore it on unbind, devm actions are not executed during the system shutdown or reboot path in device_shutdown().

Could this leave the device in software-override mode across a warm reboot, where it would be physically unprotected if it overheats during the bootloader phase before the OS boots?

Additionally, does this driver need suspend and resume PM callbacks in dev_pm_ops to temporarily re-enable the hardware shutdown while the software thermal core is asleep?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
#35 lore raw

Re: [PATCH v2 1/3] dt-bindings: clock: qcom,lcc: add MSM8x60 family compatibles

In reply to Herman van Hazendonk
Quoted 11 lines · Herman van Hazendonk

Add "qcom,lcc-msm8260", "qcom,lcc-msm8660" and "qcom,lcc-apq8060" to the qcom,lcc enum and to the existing parent-clock-name conditional block that already covers apq8064/msm8960.

The MSM8x60 family (MSM8260/MSM8660/APQ8060) is the Scorpion-class generation preceding MSM8960; its LPASS Clock Controller is fed by the same pxo board source and pll4_vote parents as apq8064/msm8960, and the codec-fed slave-mode I2S/PCM clocks have the same names, so the existing apq8064/msm8960 conditional block applies unchanged once the new compatibles are listed.

Quoted 8 lines

This allows board DTs using the new compatibles to pass dt_binding_check; the actual driver support is added by a later patch in this series.

---
 Documentation/devicetree/bindings/clock/qcom,lcc.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

This is a complete mess - one thread has like 10 different patchsets and versions.

I have no clue what to do here. I am dropping everything from DT patchwork.

Please read carefully submitting patches.

Best regards,
Krzysztof

#47 lore raw

Re: [PATCH v2 1/3] dt-bindings: mfd: qcom-pm8xxx: allow temp-alarm subnode

In reply to Herman van Hazendonk
Quoted 7 lines · Herman van Hazendonk

The qcom,pm8xxx parent schema closes its child set via additionalProperties: false and an explicit list of patternProperties. PM8901 (and prospectively other parts in the family) exposes an over-temperature alarm block as an SSBI sub-node; without a matching pattern here, any board DT that instantiates temp-alarm@<offset> under a PM8xxx parent fails dtbs_check.

Quoted 9 lines

Add the temp-alarm@[0-9a-f]+$ pattern, referencing the new qcom,pm8901-temp-alarm schema, so the temperature-alarm sub-node validates as a recognised child of the PMIC.

---
 Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml | 4 ++++
 1 file changed, 4 insertions(+)

No clue why MFD patch is in interconnect thread...

Best regards,
Krzysztof

#43 lore raw

Re: [PATCH v2 1/2] dt-bindings: interconnect: qcom: add msm8660 fabric IDs

In reply to Herman van Hazendonk
Quoted 11 lines · Herman van Hazendonk

Add the dt-binding interconnect master/slave ID header for the MSM8x60 family (MSM8260/MSM8660/APQ8060) fabric mesh. The chip's NoC fabric is split into multiple sub-fabrics that the qnoc-msm8660 driver models:

AFAB - Applications fabric (Scorpion CPU + L2) SFAB - System fabric (DMA, SPS, security)
MMFAB - Multimedia fabric (MDP, GPU, camera, video, rotator)

  DFAB  - Daytona fabric (SDC, ADM master/slave)
  SFPB  - System Fast Peripheral Bridge (RPM, MPM, PMIC SSBI)
  CFPB  - CPU Subsystem Fast Peripheral Bus (GSBI UART/QUP, USB FS,
Quoted 6 lines

TSIF, TSSC, PDM, PRNG)

IDs derived from the legacy webOS msm_bus_board_8660.c master/slave enums, normalised to the upstream interconnect-framework naming.

And why interconnects are part of thermal...

Do not attach (thread) your patchsets to some other threads (unrelated or older versions). This buries them deep in the mailbox and might interfere with applying entire sets. See also:
https://elixir.bootlin.com/linux/v6.16-rc2/source/Documentation/process/submitting-patches.rst#L830

Best regards,
Krzysztof

#54 lore raw

Re: [PATCH v2 1/2] dt-bindings: interrupt-controller: qcom: add msm8660-mpm

In reply to Herman van Hazendonk
Quoted 11 lines · Herman van Hazendonk

Add the binding for the Modem Power Manager (MPM) interrupt controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The MPM is a small wake-capable interrupt aggregator that lets the SoC stay in low-power states while a small set of GIC SPIs continues to be monitored and can wake the system on an edge.

The binding describes its register region (in the RPM syscon), the GIC SPI it raises on a wake event, the GIC mailbox it shares with the RPM, and the per-pin wake-source mapping table.

Dropping for the same reason as in other patches - mixed up with everything else.

Best regards,
Krzysztof

#59 lore raw
In reply to Herman van Hazendonk
Quoted 18 lines · Herman van Hazendonk

Hi all,

This series adds the LPASS (Low Power Audio SubSystem) Clock Controller driver for the MSM8x60 family of SoCs (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The register layout, parent muxing and divider topology of the LPASS PLL/clk fabric differ from MSM8960's LCC enough that a clean separate driver is simpler than parameterising lcc-msm8960.c. Both drivers can coexist (different Kconfig, match table and compatible).

Used on the HP TouchPad (Tenderloin) where the LPASS Q6 audio DSP needs functional MI2S / SLIMBus / PCM clocks before audio playback or capture works.

The new binding header is dual-licensed (GPL-2.0-only OR BSD-2-Clause) per current qcom-binding convention.

Quoted 8 lines

Companion to the MSM8x60 MMCC series.

Thanks,
Herman

Herman van Hazendonk (2):
dt-bindings: clock: qcom: add lcc-msm8660 LPASS clock IDs
clk: qcom: add MSM8x60 LCC (LPASS) driver

ALl of your series got entangled and linked to the same thread. Please make sure to send each patch series separately. Never send two series or two versions as reply to each other (yes, new version should also start a new thread).

Quoted 12 lines
 drivers/clk/qcom/Kconfig                     |   9 +
 drivers/clk/qcom/Makefile                    |   1 +
 drivers/clk/qcom/lcc-msm8660.c               | 517 +++++++++++++++++++
 include/dt-bindings/clock/qcom,lcc-msm8660.h |  48 ++
 4 files changed, 575 insertions(+)
 create mode 100644 drivers/clk/qcom/lcc-msm8660.c
 create mode 100644 include/dt-bindings/clock/qcom,lcc-msm8660.h
Signature · 1 line
2.43.0
Signature · 2 lines
With best wishes
Dmitry
#12 lore raw

Re: [PATCH 1/3] dt-bindings: clock: qcom: add mmcc-msm8660 clock IDs

In reply to Herman van Hazendonk
Quoted 9 lines · Herman van Hazendonk

Add the dt-binding clock-ID header for the MSM8x60 family (MSM8260/MSM8660/APQ8060) Multimedia Clock Controller (MMCC). The header enumerates the clocks and power-domains consumed by the multimedia subsystem (MDP4 display, Adreno A220 GPU, CAMSS image pipeline, VFE, Gemini JPEG, video codec, rotator, VPE and the GFX2D Z180 cores).

IDs intentionally match the numeric values used by the original shared mmcc-msm8960.h so the driver's clk array indexing is preserved;

Why?

Quoted 1 line

only the clocks actually implemented by mmcc-msm8660.c are defined.

Please also extend the qcom,mmcc.yaml to define pecularities of the device.

Quoted 7 lines
---
 include/dt-bindings/clock/qcom,mmcc-msm8660.h | 126 ++++++++++++++++++
 1 file changed, 126 insertions(+)
 create mode 100644 include/dt-bindings/clock/qcom,mmcc-msm8660.h
Signature · 2 lines
With best wishes
Dmitry
#29 lore raw

Re: [PATCH 2/2] clk: qcom: add MSM8x60 LCC (LPASS) driver

In reply to Herman van Hazendonk
Quoted 10 lines · Herman van Hazendonk

Add a clock driver for the LPASS (Low Power Audio SubSystem) Clock Controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The register layout, parent muxing and divider topology of the LPASS PLL/clk fabric differ from MSM8960's LCC enough that a clean separate driver is simpler than parameterising mmcc-msm8960.c. Both drivers can coexist in tree (different KConfig options, different match table, different device-tree compatible).

Quoted 3 lines

Used on the HP TouchPad (Tenderloin) where the LPASS Q6 audio DSP needs functional MI2S / SLIMBus / PCM clocks before audio playback or capture works.

Could you please merge these changes into lcc-msm8960.c? There are minimal differences, so you can just patch the definitions if the device is compatible with msm8660.

Quoted 9 lines
---
 drivers/clk/qcom/Kconfig       |   9 +
 drivers/clk/qcom/Makefile      |   1 +
 drivers/clk/qcom/lcc-msm8660.c | 517 +++++++++++++++++++++++++++++++++
 3 files changed, 527 insertions(+)
 create mode 100644 drivers/clk/qcom/lcc-msm8660.c
Signature · 2 lines
With best wishes
Dmitry
#57 lore raw

Re: [PATCH v2 2/2] irqchip: add MSM8x60 MPM wakeup interrupt controller driver

In reply to Herman van Hazendonk

drivers/irqchip/Kconfig config QCOM_MPM · line 534

Quoted 13 lines · Herman van Hazendonk
drivers/irqchip/Kconfig +7 -0
@@ -525,6 +525,29 @@ config QCOM_MPM
 	  MSM Power Manager driver to manage and configure wakeup
 	  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
 
+config QCOM_MSM8660_MPM
+	bool "MSM8x60 MPM wakeup interrupt controller"
+	depends on ARCH_QCOM
+	depends on MFD_SYSCON
+	depends on MAILBOX
+	select IRQ_DOMAIN_HIERARCHY
+	default y

This shouldn't by y by default.

drivers/irqchip/Kconfig config QCOM_MPM · line 546

Quoted 12 lines
drivers/irqchip/Kconfig +12 -0
+	help
+	  Platform driver for the MSM Power Manager (MPM) wakeup interrupt
+	  controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The vMPM registers live inside
+	  the RPM control block, which makes the generic QCOM_MPM driver
+	  unusable here (it assumes a dedicated MPM SRAM region, an IPCC
+	  mailbox, and uses IRQCHIP_DECLARE early-init that races platform
+	  device creation).
+
+	  This driver replicates the legacy 2.6.35-palm arch/arm/mach-msm/
+	  mpm.c mechanism as a regular platform driver, accessing the vMPM
+	  registers via a syscon phandle to the RPM block and signaling the
+	  MPM via the qcom-apcs-ipc mailbox (writing to GCC + 0x008 bit 1).

This is a reference to something that does not exist.

Quoted 43 lines
drivers/irqchip/Kconfig +3 -0
+	  Required for cpuidle deep-sleep states (SPC/PC) and for
+	  WoWLAN-style wake-from-suspend on MSM8x60 SoCs.
+
 config CSKY_MPINTC
 	bool
 	depends on CSKY
drivers/irqchip/Makefile +1 -0
index 26aa3b6ec99f..3e64591f0f5b 100644
@@ -100,6 +100,7 @@ obj-$(CONFIG_MESON_IRQ_GPIO)		+= irq-meson-gpio.o
 obj-$(CONFIG_GOLDFISH_PIC) 		+= irq-goldfish-pic.o
 obj-$(CONFIG_QCOM_PDC)			+= qcom-pdc.o
 obj-$(CONFIG_QCOM_MPM)			+= irq-qcom-mpm.o
+obj-$(CONFIG_QCOM_MSM8660_MPM)		+= irq-msm8660-mpm.o
 obj-$(CONFIG_CSKY_MPINTC)		+= irq-csky-mpintc.o
 obj-$(CONFIG_CSKY_APB_INTC)		+= irq-csky-apb-intc.o
 obj-$(CONFIG_RISCV_INTC)		+= irq-riscv-intc.o
drivers/irqchip/irq-msm8660-mpm.c new file +19 -0
new file mode 100644
index 000000000000..b8394d9c076d
@@ -0,0 +1,740 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MSM8x60 family (MSM8260/MSM8660/APQ8060) MPM (MSM Power Manager) wakeup interrupt controller
+ *
+ * The MPM is an always-on hardware block that keeps a small set of wake
+ * sources alive while the application processor is powered down for
+ * cpuidle Power Collapse or suspend-to-RAM. On MSM8x60 the
+ * vMPM (virtual MPM) registers live INSIDE the RPM's 4 KB control block
+ * at:
+ *
+ *   request (control) regs: RPM_BASE + 0x9d8  (ENABLE, DETECT_CTL,
+ *                                              POLARITY, CLEAR)
+ *   status (pending) regs:  RPM_BASE + 0xdf8  (== 0x9d8 + 0x420)
+ *
+ * The mainline qcom-mpm driver (drivers/irqchip/irq-qcom-mpm.c) is
+ * fundamentally incompatible with this layout:
+ *   - it assumes a dedicated MPM SRAM region separate from RPM;
+ *   - it assumes a mailbox controller (IPCC) for wake notification;
+ *   - it uses IRQCHIP_DECLARE which runs before platform devices exist,

drivers/irqchip/irq-msm8660-mpm.c line 29

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+ *     so of_find_device_by_node() returns NULL and the init silently
+ *     hangs.
+ *
+ * This driver replicates the 2.6.35-palm `arch/arm/mach-msm/mpm.c`
+ * mechanism as a regular platform driver: probes after platform
+ * infrastructure is ready, ioremaps the vMPM sub-region of the RPM
+ * control block (the qcom,rpm driver maps the surrounding area for
+ * its own use; the two mappings overlap and neither claims exclusive
+ * ownership), and uses the qcom-apcs-ipc mailbox for wake notification
+ * (writing to GCC + 0x008 bit 1).

Again, a file reference to something that does not exist.

drivers/irqchip/irq-msm8660-mpm.c line 36

Quoted 6 lines
drivers/irqchip/irq-msm8660-mpm.c new file +6 -0
+ * Register access is done with readl_relaxed/writel_relaxed rather than
+ * via the RPM syscon regmap. The IRQ core invokes our mask/unmask/
+ * set_type/set_wake callbacks with the irq_desc's raw_spinlock_t held,
+ * and syscon regmaps use a sleepable spinlock_t which on PREEMPT_RT
+ * would deadlock under that raw lock. Direct MMIO is also what every
+ * other SoC irqchip (qcom-pdc, gic-v3, ...) does.

Why is this kind of description here and not in the commit message.

Quoted 86 lines
drivers/irqchip/irq-msm8660-mpm.c new file +86 -0
+ * Two consumer interfaces:
+ *
+ *   1. Hierarchical irqdomain: for MPM pins that map to GIC SPIs (USB,
+ *      HDMI, ...). Consumers wire their interrupts through this
+ *      controller via interrupts-extended and the kernel manages
+ *      enable / mask / set_type / set_wake via the IRQ subsystem.
+ *
+ *   2. Raw-pin API: for MPM pins that do NOT correspond to a GIC IRQ
+ *      (SDC3_DAT1=21, SDC3_DAT3=22, SDC4_DAT1=23, SDC4_DAT3=24).
+ *      These are physical wake-signal lines monitored by MPM
+ *      directly. Consumers (mmci for SDC4 wake) call
+ *      msm8660_mpm_set_pin_wake() etc. The consumer API establishes
+ *      a device_link from consumer to producer so the MPM device
+ *      cannot disappear while a consumer holds a handle.
+ *
+ * Copyright (c) 2026 Herman van Hazendonk <[email protected]>
+ * Copyright (c) 2010-2012, The Linux Foundation (legacy mpm.c reference)
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include <soc/qcom/msm8660-mpm.h>
+
+/*
+ * vMPM register offsets (relative to the start of the ioremap'd window
+ * = RPM base + 0x9d8). Each register is two 32-bit slots because MPM
+ * exposes 64 wake pins.
+ */
+#define MSM8660_MPM_REG_ENABLE		0x00
+#define MSM8660_MPM_REG_DETECT_CTL	0x08
+#define MSM8660_MPM_REG_POLARITY	0x10
+#define MSM8660_MPM_REG_CLEAR		0x18
+
+/* Status registers at +0x420 from vMPM base (== RPM + 0xdf8). */
+#define MSM8660_MPM_STATUS_OFFSET	0x420
+
+#define MSM8660_MPM_PIN_COUNT		64
+#define MSM8660_MPM_REG_WIDTH		2
+
+struct msm8660_mpm_pin {
+	int pin;
+	int hwirq;
+};
+
+struct msm8660_mpm {
+	struct device *dev;
+	void __iomem *base;
+	struct irq_domain *domain;
+	struct msm8660_mpm_pin *pin_map;
+	unsigned int pin_map_count;
+	int parent_irq;
+	struct mbox_client mbox_client;
+	struct mbox_chan *mbox_chan;
+};
+
+/*
+ * Singleton - there is only one MPM instance per SoC. msm8660_mpm_get()
+ * returns this. Updates are serialised through the binding lifecycle so
+ * a plain pointer is sufficient.
+ */
+static struct msm8660_mpm *msm8660_mpm_global;
+
+static u32 msm8660_mpm_read(struct msm8660_mpm *mpm, unsigned int reg)
+{
+	return readl_relaxed(mpm->base + reg);
+}
+
+static void msm8660_mpm_write(struct msm8660_mpm *mpm, unsigned int reg,
+			      u32 val)
+{
+	writel_relaxed(val, mpm->base + reg);
+}
+

drivers/irqchip/irq-msm8660-mpm.c line 133

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+/*
+ * Doorbell the RPM after touching the vMPM request registers. Without
+ * this the RPM keeps using its last cached copy of the enable/detect/
+ * polarity state and our configuration changes have no effect.
+ *
+ * Called from raw_spinlock_t-held contexts (irq_chip mask/unmask/
+ * set_type/set_wake), so the mailbox driver must accept that. The
+ * qcom-apcs-ipc mailbox just does a writel into the IPC trigger
+ * register; it is safe under a raw lock.
+ */

Where are these comments coming from?

drivers/irqchip/irq-msm8660-mpm.c line 134

Quoted 1 line
drivers/irqchip/irq-msm8660-mpm.c new file +1 -0
+static void msm8660_mpm_doorbell(struct msm8660_mpm *mpm)

…

Sebastian

maintainer
#58 lore raw

Re: [PATCH v2 2/2] irqchip: add MSM8x60 MPM wakeup interrupt controller driver

In reply to Herman van Hazendonk
Quoted 13 lines · Herman van Hazendonk

+ *
+ * 1. Hierarchical irqdomain: for MPM pins that map to GIC SPIs (USB,

+ *      HDMI, ...). Consumers wire their interrupts through this
+ *      controller via interrupts-extended and the kernel manages
+ *      enable / mask / set_type / set_wake via the IRQ subsystem.
+ *
+ *   2. Raw-pin API: for MPM pins that do NOT correspond to a GIC IRQ
+ *      (SDC3_DAT1=21, SDC3_DAT3=22, SDC4_DAT1=23, SDC4_DAT3=24).
+ *      These are physical wake-signal lines monitored by MPM
+ *      directly. Consumers (mmci for SDC4 wake) call
+ *      msm8660_mpm_set_pin_wake() etc. The consumer API establishes
+ *      a device_link from consumer to producer so the MPM device
+ *      cannot disappear while a consumer holds a handle.

Why can't this be described in the device tree?

drivers/irqchip/irq-msm8660-mpm.c line 104

Quoted 11 lines
drivers/irqchip/irq-msm8660-mpm.c new file +11 -0
+
+struct msm8660_mpm {
+	struct device *dev;
+	void __iomem *base;
+	struct irq_domain *domain;
+	struct msm8660_mpm_pin *pin_map;
+	unsigned int pin_map_count;
+	int parent_irq;
+	struct mbox_client mbox_client;
+	struct mbox_chan *mbox_chan;
+};

https://docs.kernel.org/process/maintainer-tip.html#struct-declarations-and-initializers

Please read the rest of this document too.

Quoted 5 lines
drivers/irqchip/irq-msm8660-mpm.c new file +5 -0
+
+/*
+ * Singleton - there is only one MPM instance per SoC. msm8660_mpm_get()
+ * returns this. Updates are serialised through the binding lifecycle so
+ * a plain pointer is sufficient.

drivers/irqchip/irq-msm8660-mpm.c line 119

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+ */
+static struct msm8660_mpm *msm8660_mpm_global;
+
+static u32 msm8660_mpm_read(struct msm8660_mpm *mpm, unsigned int reg)
+{
+	return readl_relaxed(mpm->base + reg);
+}
+
+static void msm8660_mpm_write(struct msm8660_mpm *mpm, unsigned int reg,
+			      u32 val)

No line break required. You have 100 characters. All over the place.

drivers/irqchip/irq-msm8660-mpm.c line 151

Quoted 5 lines
drivers/irqchip/irq-msm8660-mpm.c new file +5 -0
+static int msm8660_mpm_pin_to_hwirq(struct msm8660_mpm *mpm, int pin)
+{
+	int i;
+
+	for (i = 0; i < mpm->pin_map_count; i++) {

for (int i = 0; ....

Quoted 9 lines
drivers/irqchip/irq-msm8660-mpm.c new file +9 -0
+		if (mpm->pin_map[i].pin == pin)
+			return mpm->pin_map[i].hwirq;
+	}
+	return -ENOENT;
+}
+
+/*
+ * IPC handler: MPM fires this IRQ when one or more enabled wake pins
+ * have pending activity. Read pending status, CLEAR the pending bits

drivers/irqchip/irq-msm8660-mpm.c line 170

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+ * BEFORE dispatching the per-pin handlers so a fresh edge that arrives
+ * during dispatch cannot be wiped out by a later CLEAR write, then
+ * replay each pending pin through the irqdomain.
+ */
+static irqreturn_t msm8660_mpm_irq(int irq, void *data)
+{
+	struct msm8660_mpm *mpm = data;
+	unsigned long pending[MSM8660_MPM_REG_WIDTH];
+	unsigned long enable[MSM8660_MPM_REG_WIDTH];
+	int i, j;

See documented variable ordering and put the iterator variables into context.

drivers/irqchip/irq-msm8660-mpm.c line 229

Quoted 5 lines
drivers/irqchip/irq-msm8660-mpm.c new file +5 -0
+static void msm8660_mpm_enable_hwirq(struct irq_data *d, bool enable)
+{
+	struct msm8660_mpm *mpm = irq_data_get_irq_chip_data(d);
+	int pin;
+	u32 val, mask;

See docs

Quoted 8 lines
drivers/irqchip/irq-msm8660-mpm.c new file +8 -0
+static int msm8660_mpm_domain_alloc(struct irq_domain *domain,
+				    unsigned int virq, unsigned int nr_irqs,
+				    void *data)
+{
+	struct msm8660_mpm *mpm = domain->host_data;
+	struct irq_fwspec *fwspec = data;
+	struct irq_fwspec parent_fwspec;
+	irq_hw_number_t hwirq;

drivers/irqchip/irq-msm8660-mpm.c line 357

Quoted 10 lines
drivers/irqchip/irq-msm8660-mpm.c new file +10 -0
+	int i, ret;
+
+	if (fwspec->param_count != 2)
+		return -EINVAL;
+
+	hwirq = fwspec->param[0];
+
+	for (i = 0; i < nr_irqs; i++)
+		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+					      &msm8660_mpm_chip, mpm);

See bracket rules.

drivers/irqchip/irq-msm8660-mpm.c line 699

Quoted 7 lines
drivers/irqchip/irq-msm8660-mpm.c new file +7 -0
+static void msm8660_mpm_remove(struct platform_device *pdev)
+{
+	struct msm8660_mpm *mpm = platform_get_drvdata(pdev);
+
+	/*
+	 * Tear down in strict reverse order: drop the singleton so new
+	 * consumers cannot grab a handle, free the IRQ so the handler

How is that serialized against a concurrent consumer request?

Also please look at:

https://sashiko.dev/#/message/20260531043213.D18801F00893%40smtp.kernel.org

Thanks,

tglx

#17 lore raw

Re: [PATCH 3/3] clk: qcom: add MSM8x60 MMCC driver

In reply to Herman van Hazendonk
Quoted 9 lines · Herman van Hazendonk

Add a clock driver for the Multimedia Clock Controller (MMCC) on the MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The MMCC layout on MSM8x60 differs from MSM8960 in several ways that make a separate driver cleaner than parameterising mmcc-msm8960.c:

- the pix_rdi mux requires a custom set_parent op that temporarily

    enables both parents during the glitch-free transition;

Can it be handled by CLK_OPS_PARENT_ENABLE ?

Quoted 24 lines

- the IJPEG GDSC requires releasing AXI, AHB and CORE resets;
- several rate-source pairs (MDP pixel, GFX2D/3D) only exist on

    8x60 (e.g. PLL2-derived 228571000/266667000 for graphics);

- the camera CSI / VFE / JPEG / VPE / ROT clock topology lacks the later 8960 reorganisation.

Used on the HP TouchPad (Tenderloin) for graphics (Adreno A220), display (MDP4), camera (CSI/VFE), JPEG (Gemini), VIDC, VPE and rotator. Reset IDs are exposed via a separate header so consumers can reset the GDSCs and individual blocks.

---
 drivers/clk/qcom/Kconfig        |   11 +
 drivers/clk/qcom/Makefile       |    1 +
 drivers/clk/qcom/mmcc-msm8660.c | 2998 +++++++++++++++++++++++++++++++
 3 files changed, 3010 insertions(+)
 create mode 100644 drivers/clk/qcom/mmcc-msm8660.c
drivers/clk/qcom/Kconfig +0 -0
index d9cff5b0281d..f64b0002c85a 100644

drivers/clk/qcom/Kconfig config MSM_MMCC_8960 · line 575

Quoted 4 lines
drivers/clk/qcom/Kconfig +1 -0
@@ -572,6 +572,17 @@ config MSM_MMCC_8960
 	  Say Y if you want to support multimedia devices such as display,
 	  graphics, video encode/decode, camera, etc.
 
+config MSM_MMCC_8660

Please put it before MSM_MMCC_8960

Quoted 18 lines
drivers/clk/qcom/Kconfig +10 -0
+	tristate "MSM8x60 Multimedia Clock Controller"
+	depends on ARM || COMPILE_TEST
+	select MSM_GCC_8660
+	select QCOM_GDSC
+	help
+	  Support for the multimedia clock controller on the MSM8x60 family
+	  (MSM8260/MSM8660/APQ8060). Say Y if you want to support multimedia
+	  devices such as display, graphics, video encode/decode, camera, etc.
+	  on these Scorpion-class SoCs (e.g. HP TouchPad).
+
 config MSM_GCC_8953
 	tristate "MSM8953 Global Clock Controller"
 	select QCOM_GDSC
drivers/clk/qcom/Makefile +0 -0
index e100cfd6a52d..ff317fce086d 100644

drivers/clk/qcom/Makefile obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o · line 84

Quoted 4 lines
drivers/clk/qcom/Makefile +1 -0
@@ -81,6 +81,7 @@ obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o
 obj-$(CONFIG_MSM_GCC_8998) += gcc-msm8998.o
 obj-$(CONFIG_MSM_GPUCC_8998) += gpucc-msm8998.o
 obj-$(CONFIG_MSM_MMCC_8960) += mmcc-msm8960.o
+obj-$(CONFIG_MSM_MMCC_8660) += mmcc-msm8660.o

Also before CONFIG_MSM_MMCC_8960

Quoted 49 lines
drivers/clk/qcom/Makefile +0 -0
 obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o
 obj-$(CONFIG_MSM_MMCC_8994) += mmcc-msm8994.o
 obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o
drivers/clk/qcom/mmcc-msm8660.c new file +40 -0
new file mode 100644
index 000000000000..5e6943430d29
@@ -0,0 +1,2998 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ Herman van Hazendonk <[email protected]>
+ *
+ * MSM8x60 family (MSM8260/MSM8660/APQ8060) Multimedia Clock Controller driver
+ *
+ * Split from mmcc-msm8960.c to properly handle MSM8x60-specific clock
+ * configurations, particularly the GFX3D reset bits which differ from MSM8960.
+ */
+
+#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/mfd/qcom_rpm.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/qcom,mmcc-msm8660.h>
+#include <dt-bindings/mfd/qcom-rpm.h>
+#include <dt-bindings/reset/qcom,mmcc-msm8660.h>
+
+#include "common.h"
+#include "clk-regmap.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-branch.h"
+#include "reset.h"
+#include "gdsc.h"
+
+enum {
+	P_PXO,
+	P_PLL8,

drivers/clk/qcom/mmcc-msm8660.c line 50

Quoted 10 lines
drivers/clk/qcom/mmcc-msm8660.c new file +10 -0
+	P_PLL2,
+	P_PLL3,
+	P_HDMI_PLL,
+	P_DSI1_PLL_DSICLK,
+	P_DSI2_PLL_DSICLK,
+	P_DSI1_PLL_BYTECLK,
+	P_DSI2_PLL_BYTECLK,
+};
+
+#define F_MN(f, s, _m, _n) { .freq = f, .src = s, .pre_div = 1, .m = _m, .n = _n }

I think you should have gotten a warning here. It should be .freq = (f), etc.

drivers/clk/qcom/mmcc-msm8660.c line 52

Quoted 2 lines
drivers/clk/qcom/mmcc-msm8660.c new file +2 -0
+/* Pure divider+source RCG (no MND), e.g. VPE on MSM8660: NS_DIVSRC(15,12,d,2,0,s) */
+#define F_DIV(f, s, d) { .freq = f, .src = s, .pre_div = d }

The same.

drivers/clk/qcom/mmcc-msm8660.c line 65

Quoted 13 lines
drivers/clk/qcom/mmcc-msm8660.c new file +13 -0
+
+static struct clk_pll pll2 = {
+	.l_reg = 0x320,
+	.m_reg = 0x324,
+	.n_reg = 0x328,
+	.config_reg = 0x32c,
+	.mode_reg = 0x31c,
+	.status_reg = 0x334,
+	.status_bit = 16,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "pll2",
+		.parent_data = (const struct clk_parent_data[]){
+			{ .fw_name = "pxo", .name = "pxo_board" },

There was no MMCC node beforehand. You can drop all the .name entries, they are mostly required for the legacy cases where we had an incomplete DT.

Also, current approach is to use indices instead of clock-names to identify parents (see any modern clock driver under drivers/clk/qcom).

Quoted 34 lines
drivers/clk/qcom/mmcc-msm8660.c new file +34 -0
+		},
+		.num_parents = 1,
+		.ops = &clk_pll_ops,
+	},
+};
+
+
+/*
+ * CSI PIX/RDI clock mux support
+ *
+ * On MSM8660, the csi_pix_clk and csi_rdi_clk can source from either
+ * CSI0 or CSI1. The selection is controlled by bits in CSC_CC_REG (0x0058):
+ *   - BIT(25): csi_pix_clk source (0=CSI0, 1=CSI1)
+ *   - BIT(12): csi_rdi_clk source (0=CSI0, 1=CSI1)
+ *
+ * This mux is critical for HP TouchPad which uses CSI1 for the front camera.
+ */
+struct clk_pix_rdi {
+	u32 s_reg;
+	u32 s_mask;
+	struct clk_regmap clkr;
+};
+
+#define to_clk_pix_rdi(_hw) \
+	container_of(to_clk_regmap(_hw), struct clk_pix_rdi, clkr)
+
+static int pix_rdi_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct clk_pix_rdi *rdi = to_clk_pix_rdi(hw);
+	u32 val;
+	int i, ret = 0;
+	int num_parents = clk_hw_get_num_parents(hw);
+
+	/*

drivers/clk/qcom/mmcc-msm8660.c line 436

Quoted 10 lines
drivers/clk/qcom/mmcc-msm8660.c new file +10 -0
+	 * Turn on all parent sources during mux switch to ensure
+	 * glitch-free transition.
+	 */
+	for (i = 0; i < num_parents; i++) {
+		struct clk_hw *p = clk_hw_get_parent_by_index(hw, i);
+
+		ret = clk_prepare_enable(p->clk);
+		if (ret)
+			goto err;
+	}

As I wrote, I think CLK_OPS_PARENT_ENABLE should handle this for you.

Quoted 55 lines
drivers/clk/qcom/mmcc-msm8660.c new file +55 -0
+
+	/* Select parent: 0=CSI0, 1=CSI1 */
+	val = (index == 1) ? rdi->s_mask : 0;
+	regmap_update_bits(rdi->clkr.regmap, rdi->s_reg, rdi->s_mask, val);
+
+	/*
+	 * Wait at least 6 cycles of slowest clock for the
+	 * glitch-free MUX to fully switch sources.
+	 */
+	udelay(1);
+
+	/*
+	 * Now disable all parents that were temporarily enabled.
+	 * The clock framework will keep the selected parent enabled
+	 * as long as the child (csi_pix_clk/csi_rdi_clk) is enabled.
+	 */
+	for (i = num_parents - 1; i >= 0; i--) {
+		struct clk_hw *p = clk_hw_get_parent_by_index(hw, i);
+
+		clk_disable_unprepare(p->clk);
+	}
+
+	return 0;
+
+err:
+	/* On error, disable only the parents we successfully enabled */
+	for (i--; i >= 0; i--) {
+		struct clk_hw *p = clk_hw_get_parent_by_index(hw, i);
+
+		clk_disable_unprepare(p->clk);
+	}
+
+	return ret;
+}
+
+static u8 pix_rdi_get_parent(struct clk_hw *hw)
+{
+	struct clk_pix_rdi *rdi = to_clk_pix_rdi(hw);
+	u32 val;
+
+	regmap_read(rdi->clkr.regmap, rdi->s_reg, &val);
+	return (val & rdi->s_mask) ? 1 : 0;
+}
+
+static const struct clk_ops clk_ops_pix_rdi = {
+	.enable = clk_enable_regmap,
+	.disable = clk_disable_regmap,
+	.is_enabled = clk_is_enabled_regmap,
+	.set_parent = pix_rdi_set_parent,
+	.get_parent = pix_rdi_get_parent,
+	.determine_rate = __clk_mux_determine_rate,
+};
+
+static const struct clk_hw *pix_rdi_parents[] = {
+	&csi0_clk.clkr.hw,

drivers/clk/qcom/mmcc-msm8660.c line 501

Quoted 10 lines
drivers/clk/qcom/mmcc-msm8660.c new file +10 -0
+	&csi1_clk.clkr.hw,
+};
+
+static struct clk_pix_rdi csi_pix_clk = {
+	.s_reg = 0x0058,
+	.s_mask = BIT(25),
+	.clkr = {
+		.enable_reg = 0x0058,
+		.enable_mask = BIT(26),
+		.hw.init = &(struct clk_init_data){

WHile I noticed, const struct clk_init_data.

Quoted 42 lines
drivers/clk/qcom/mmcc-msm8660.c new file +42 -0
+			.name = "csi_pix_clk",
+			.parent_hws = pix_rdi_parents,
+			.num_parents = ARRAY_SIZE(pix_rdi_parents),
+			.ops = &clk_ops_pix_rdi,
+		},
+	},
+};
+
+
+/*
+ * MSM8x60 legacy footswitches.
+ * These use a different register layout than modern GDSCs:
+ * - Bit 8: ENABLE (set to enable power)
+ * - Bit 5: CLAMP (set to clamp I/O)
+ * - No status bit, requires timed delays
+ */
+static struct gdsc gfx2d0_gdsc = {
+	.gdscr = 0x0180,
+	.resets = (unsigned int []){ GFX2D0_AHB_RESET },
+	.reset_count = 1,
+	.pd = {
+		.name = "gfx2d0",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = LEGACY_FOOTSWITCH | SW_RESET,
+};
+
+static struct gdsc gfx2d1_gdsc = {
+	.gdscr = 0x0184,
+	.resets = (unsigned int []){ GFX2D1_AHB_RESET },
+	.reset_count = 1,
+	.pd = {
+		.name = "gfx2d1",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = LEGACY_FOOTSWITCH | SW_RESET,
+};
+
+static struct gdsc gfx3d_gdsc = {
+	.gdscr = 0x0188,
+	/*
+	 * GFX3D (Adreno 220) requires the core reset to be toggled on every

drivers/clk/qcom/mmcc-msm8660.c line 2502

Quoted 10 lines
drivers/clk/qcom/mmcc-msm8660.c new file +10 -0
+	 * power-on, in addition to the AHB reset. The legacy MSM8x60
+	 * footswitch driver (mach-msm/footswitch-8x60.c) does exactly this:
+	 * after powering the GFX3D rail it issues an extra
+	 * clk_reset(core_clk, ASSERT/DEASSERT). Without it the Adreno 220
+	 * core (parameter cache) comes up free-running, producing the
+	 * deterministic period-8 render cycle on the HP TouchPad. Listing
+	 * GFX3D_RESET here lets the GDSC framework assert/deassert it around
+	 * the rail charge (see gdsc_enable LEGACY_FOOTSWITCH path), which is
+	 * the framework-correct equivalent of the legacy toggle.
+	 */

If only all upstream commits had such a nice comments.

Quoted 18 lines
drivers/clk/qcom/mmcc-msm8660.c new file +18 -0
+	.resets = (unsigned int []){ GFX3D_AHB_RESET, GFX3D_RESET },
+	.reset_count = 2,
+	.pd = {
+		.name = "gfx3d",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	/*
+	 * RPM_ALWAYS_ON: keep the GFX3D footswitch powered across runtime PM
+	 * idle (clocks still gate), collapsing it only on system suspend. The
+	 * Adreno 220 shares the MMSS AXI fabric with the MDP4 display; cold-
+	 * cycling this footswitch on every GPU idle forces an a2xx_hw_init
+	 * microcode reload whose MMIO burst can stall the shared bus when it
+	 * lands during an MDP client-switch underrun, hard-hanging the SoC.
+	 * Mirrors legacy KGSL, which parked the GPU in SLEEP (rail up, clocks
+	 * gated) during use and only power-collapsed (SLUMBER) on suspend.
+	 */
+	.flags = LEGACY_FOOTSWITCH | SW_RESET | RPM_ALWAYS_ON,
+};

drivers/clk/qcom/mmcc-msm8660.c

Quoted 10 lines
drivers/clk/qcom/mmcc-msm8660.c new file +10 -0
+
+
+static const struct qcom_cc_desc mmcc_msm8660_desc = {
+	.config = &mmcc_msm8660_regmap_config,
+	.clks = mmcc_msm8660_clks,
+	.num_clks = ARRAY_SIZE(mmcc_msm8660_clks),
+	.resets = mmcc_msm8660_resets,
+	.num_resets = ARRAY_SIZE(mmcc_msm8660_resets),
+	.gdscs = mmcc_msm8660_gdscs,
+	.num_gdscs = ARRAY_SIZE(mmcc_msm8660_gdscs),

I commented on the other patchset. I think handling footswitches separately from GDSCs would be better.

drivers/clk/qcom/mmcc-msm8660.c line 2673

Quoted 5 lines
drivers/clk/qcom/mmcc-msm8660.c new file +5 -0
+};
+
+static const struct of_device_id mmcc_msm8660_match_table[] = {
+	{ .compatible = "qcom,mmcc-msm8660", .data = &mmcc_msm8660_desc },
+	{ .compatible = "qcom,mmcc-apq8060", .data = &mmcc_msm8660_desc },

Unless you fancy any quirks, just leave qcom,mmcc-msm8660 here.

Quoted 93 lines
drivers/clk/qcom/mmcc-msm8660.c new file +93 -0
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mmcc_msm8660_match_table);
+
+/*
+ * MSM8660 MMCC register offsets for initialization.
+ * Based on webOS kernel arch/arm/mach-msm/clock-8x60.c
+ */
+
+/* Reset registers - safe to deassert */
+#define SW_RESET_ALL_REG	0x0204
+#define SW_RESET_AHB_REG	0x020c
+#define SW_RESET_AXI_REG	0x0208
+#define SW_RESET_CORE_REG	0x0210
+
+/* AHB enable registers - contain both control bits and clock enables */
+#define AHB_EN_REG		0x0008
+#define AHB_EN2_REG		0x0038
+
+/* AXI enable registers - control AXI bus paths for memory access */
+#define MAXI_EN_REG		0x0018
+#define MAXI_EN3_REG		0x002c
+#define SAXI_EN_REG		0x01d8
+
+/* Misc CC registers */
+#define CSI_CC_REG		0x0040
+#define MISC_CC_REG		0x0058
+#define MISC_CC2_REG		0x005c
+
+/* CC registers - FORCE_CORE_ON in upper bits, enable in lower bits */
+#define GFX2D0_CC_REG		0x0060
+#define GFX2D1_CC_REG		0x0074
+#define GFX3D_CC_REG		0x0080
+#define IJPEG_CC_REG		0x0098
+#define JPEGD_CC_REG		0x00a4
+#define MDP_CC_REG		0x00c0
+#define PIXEL_CC_REG		0x00d4
+#define PIXEL_CC2_REG		0x0120  /* Not used as enable_reg */
+#define ROT_CC_REG		0x00e0
+#define TV_CC_REG		0x00ec
+#define TV_CC2_REG		0x0124  /* Not used as enable_reg */
+#define VCODEC_CC_REG		0x00f8
+#define VFE_CC_REG		0x0104
+#define VPE_CC_REG		0x0110
+
+/*
+ * Mask for FORCE_CORE_ON and sleep/wakeup delay bits in CC registers.
+ * Only touches upper bits to avoid conflicting with clock enable bits
+ * in the lower portion of the register.
+ */
+#define CC_FORCE_CORE_ON_MASK	0xe0ff0000
+#define CC_FORCE_CORE_ON_VAL	0x80ff0000  /* FORCE_CORE_ON + 0xFF delays */
+#define VCODEC_FORCE_CORE_ON	0xc0ff0000  /* VCODEC uses different bits */
+
+static void mmcc_msm8660_init_hw(struct regmap *regmap)
+{
+	u32 val;
+
+	/*
+	 * Configure PLL2 (MM_PLL1) to 800 MHz for VFE and other MM clocks.
+	 *
+	 * PLL rate = PXO * (L + M/N) = 27 MHz * (29 + 17/27) = 800 MHz
+	 *
+	 * The bootloader (moboot) enables PLL2 but does not configure L/M/N,
+	 * leaving it at whatever values are in the hardware. We must set it
+	 * properly for VFE to reach 228 MHz. Without this, VFE gets ~66 MHz.
+	 */
+	regmap_read(regmap, 0x320, &val);  /* PLL2 L value */
+	if (val != 29) {
+		u32 m_val, n_val;
+		/* PLL2 is not configured for 800 MHz, configure it */
+		regmap_read(regmap, 0x324, &m_val);
+		regmap_read(regmap, 0x328, &n_val);
+		pr_info("mmcc-msm8660: Configuring PLL2 for 800 MHz (was L=%u M=%u N=%u)\n",
+			val, m_val, n_val);
+
+		/* Disable PLL output first */
+		regmap_update_bits(regmap, 0x31c, BIT(0), 0);
+		udelay(10);
+
+		/* Set L=29, M=17, N=27 for 800 MHz from 27 MHz PXO */
+		regmap_write(regmap, 0x320, 29);   /* L value */
+		regmap_write(regmap, 0x324, 17);   /* M value */
+		regmap_write(regmap, 0x328, 27);   /* N value */
+
+		/* Configure PLL: enable main output, set VCO */
+		regmap_write(regmap, 0x32c, 0x00800000);
+
+		/* Enable PLL: bypass off, reset deassert, output enable */
+		regmap_update_bits(regmap, 0x31c, BIT(1), BIT(1));  /* Bypass off */
+		udelay(10);
+		regmap_update_bits(regmap, 0x31c, BIT(2), BIT(2));  /* Reset deassert */
+		udelay(50);

drivers/clk/qcom/mmcc-msm8660.c line 2776

Quoted 10 lines
drivers/clk/qcom/mmcc-msm8660.c new file +10 -0
+		regmap_update_bits(regmap, 0x31c, BIT(0), BIT(0));  /* Output enable */
+		udelay(50);
+
+		/* Verify PLL locked (status register at 0x334, bit 16) */
+		regmap_read(regmap, 0x334, &val);
+		if (val & BIT(16))
+			pr_info("mmcc-msm8660: PLL2 locked at 800 MHz\n");
+		else
+			pr_warn("mmcc-msm8660: PLL2 may not be locked (status=0x%x)\n", val);
+	}

Extracgt to a separate function, please. Pass pll2 as an arg and use registers from it instead of raw values.

Quoted 125 lines
drivers/clk/qcom/mmcc-msm8660.c new file +125 -0
+
+	/*
+	 * MSM8660 MMCC hardware initialization based on webOS kernel.
+	 *
+	 * WebOS sets specific control bits in AHB_EN_REG:
+	 *   rmwreg(0x00000003, AHB_EN_REG, 0x0F7FFFFF);
+	 * BIT(0) and BIT(1) are control bits (FPB enable, HW gating disable),
+	 * NOT clock enables. Clock enables start at BIT(2) and above.
+	 *
+	 * We initialize these control bits but leave clock enable bits
+	 * for the clock framework to manage.
+	 */
+
+	/*
+	 * Set FPB enable and disable HW gating in AHB_EN_REG.
+	 * BIT(0) = FPB clock enable
+	 * BIT(1) = Disable HW gating for all AHB clocks
+	 * These are required for CSI register writes to work.
+	 */
+	regmap_update_bits(regmap, AHB_EN_REG, 0x3, 0x3);
+
+	/*
+	 * AHB_EN2_REG contains additional control bits including
+	 * VFE_AHB FORCE_CORE_ON to prevent memory collapse.
+	 * WebOS: rmwreg(0x000007F9, AHB_EN2_REG, 0x7FFFBFFF);
+	 */
+	regmap_update_bits(regmap, AHB_EN2_REG, 0x7fffbfff, 0x000007f9);
+
+	/*
+	 * Initialize AXI bus registers for memory access paths.
+	 * These enable HW gating and set FORCE_CORE_ON bits for AXI clocks.
+	 * WebOS: rmwreg(0x000307F9, MAXI_EN_REG, 0x0FFFFFFF);
+	 *        writel(0x3FE7FCFF, MAXI_EN3_REG);
+	 *        writel(0x000001D8, SAXI_EN_REG);
+	 * Note: MAXI_EN2_REG is owned by RPM, don't touch it.
+	 */
+	regmap_update_bits(regmap, MAXI_EN_REG, 0x0fffffff, 0x000307f9);
+	regmap_write(regmap, MAXI_EN3_REG, 0x3fe7fcff);
+	regmap_write(regmap, SAXI_EN_REG, 0x000001d8);
+
+	/* Deassert all MM resets */
+	regmap_write(regmap, SW_RESET_ALL_REG, 0);
+	regmap_write(regmap, SW_RESET_AHB_REG, 0);
+	regmap_write(regmap, SW_RESET_AXI_REG, 0);
+	regmap_write(regmap, SW_RESET_CORE_REG, 0);
+
+	/*
+	 * Initialize CSI and MISC CC registers.
+	 *
+	 * CSI_CC_REG (0x040): webOS camera dump shows 0x85
+	 *   - Bit 0: CSI digital wrapper 0 enable
+	 *   - Bit 2: CSI digital wrapper 1 enable
+	 *   - Bit 7: Unknown (possibly global CSI enable)
+	 * Without these bits set, CSIPHY data never reaches VFE.
+	 *
+	 * MISC_CC_REG (0x058): webOS camera dump shows 0x0400
+	 *   - Bit 10: CSI1-to-VFE async bridge enable
+	 * MSM8660 has direct CSIPHY->VFE path, NOT the csi_pix/csi_rdi mux
+	 * architecture found in VFE3.2+. The csi_pix_clk/csi_rdi_clk (bits
+	 * 25-26 and 12-13) don't exist on this SoC - only bit 10 matters.
+	 *
+	 * MISC_CC2_REG: webOS shows 0x004007fd - additional enables.
+	 */
+	regmap_write(regmap, CSI_CC_REG, 0x00000085);
+	regmap_update_bits(regmap, MISC_CC_REG, 0xfefff7ff, 0x00000400);
+	regmap_update_bits(regmap, MISC_CC2_REG, 0xffff7fff, 0x000007fd);
+	/* Set dsi_byte_clk src to DSI PHY PLL, hdmi_app_clk src to PXO */
+	regmap_update_bits(regmap, MISC_CC2_REG, 0x00424003, 0x00400001);
+
+	/*
+	 * Set FORCE_CORE_ON bits in all multimedia CC registers to prevent
+	 * core memories from being collapsed when clocks are halted.
+	 *
+	 * Value 0x80ff0000 sets:
+	 *   - Bit 31: FORCE_CORE_ON
+	 *   - Bits 16-23: Safe sleep/wakeup delay values (0xFF)
+	 *
+	 * We use regmap_update_bits to only touch upper bits, avoiding
+	 * conflict with clock enable bits in the lower portion.
+	 */
+
+	/* Graphics */
+	regmap_update_bits(regmap, GFX2D0_CC_REG, CC_FORCE_CORE_ON_MASK, CC_FORCE_CORE_ON_VAL);
+	regmap_update_bits(regmap, GFX2D1_CC_REG, CC_FORCE_CORE_ON_MASK, CC_FORCE_CORE_ON_VAL);
+	regmap_update_bits(regmap, GFX3D_CC_REG, CC_FORCE_CORE_ON_MASK, CC_FORCE_CORE_ON_VAL);
+
+	/* Display */
+	regmap_update_bits(regmap, MDP_CC_REG, CC_FORCE_CORE_ON_MASK, CC_FORCE_CORE_ON_VAL);
+	regmap_update_bits(regmap, PIXEL_CC_REG, CC_FORCE_CORE_ON_MASK, CC_FORCE_CORE_ON_VAL);
+	regmap_write(regmap, PIXEL_CC2_REG, 0x000004ff);
+	regmap_update_bits(regmap, TV_CC_REG, CC_FORCE_CORE_ON_MASK, CC_FORCE_CORE_ON_VAL);
+	regmap_write(regmap, TV_CC2_REG, 0x000004ff);
+	regmap_update_bits(regmap, ROT_CC_REG, CC_FORCE_CORE_ON_MASK, CC_FORCE_CORE_ON_VAL);
+
+	/* Video */
+	regmap_update_bits(regmap, VCODEC_CC_REG, CC_FORCE_CORE_ON_MASK, VCODEC_FORCE_CORE_ON);
+	regmap_update_bits(regmap, VPE_CC_REG, CC_FORCE_CORE_ON_MASK, CC_FORCE_CORE_ON_VAL);
+
+	/* Camera */
+	regmap_update_bits(regmap, VFE_CC_REG, CC_FORCE_CORE_ON_MASK, CC_FORCE_CORE_ON_VAL);
+
+	/* JPEG */
+	regmap_update_bits(regmap, IJPEG_CC_REG, CC_FORCE_CORE_ON_MASK, CC_FORCE_CORE_ON_VAL);
+	regmap_update_bits(regmap, JPEGD_CC_REG, CC_FORCE_CORE_ON_MASK, CC_FORCE_CORE_ON_VAL);
+}
+
+/*
+ * Unhalt all RPM fabric AXI master ports.
+ *
+ * webOS downstream kernels (msm_bus_board_8660.c) program halt registers
+ * on three fabrics: APPS (4 masters), SYSTEM (17 masters), MMSS
+ * (14 masters). The downstream GDSC driver calls msm_bus_axi_portunhalt()
+ * in footswitch_enable() when each power domain comes up; mainline GDSC
+ * does not, leaving fabric master ports in their default (potentially
+ * halted) state. This can cause DMA stalls and arbitration hangs.
+ *
+ * MMSS fabric master ports (port:name from webOS enum):
+ *   0:MDP_PORT0   1:MDP_PORT1   2:ADM1_PORT0  3:ROTATOR
+ *   4:GFX3D       5:JPEG_DEC    6:GFX2D0      7:VFE
+ *   8:VPE         9:JPEG_ENC   10:GFX2D1     11:APPS_FAB
+ *  12:HDCODEC0   13:HDCODEC1
+ *
+ * SYSTEM fabric master ports:
+ *   0:APPSS_FAB    1:SPS         2:ADM0_PORT0  3:ADM0_PORT1
+ *   4:ADM1_PORT0   5:ADM1_PORT1  6:LPASS_PROC  7:MSS_PROCI

drivers/clk/qcom/mmcc-msm8660.c line 2911

Quoted 10 lines
drivers/clk/qcom/mmcc-msm8660.c new file +10 -0
+ *   8:MSS_PROCD    9:MDM_PORT0  10:LPASS      11:CPSS_FPB
+ *  12:SYSTEM_FPB  13:MMSS_FPB   14:ADM1_AHB   15:ADM0_AHB
+ *  16:MSS_MDM_PORT1
+ *
+ * APPS fabric has 4 masters; unhalting all is safe.
+ *
+ * Driven from mmcc probe because it runs after the qcom_rpm platform
+ * driver registers (mmcc is module_platform_driver, RPM is platform_init).
+ * Doing the same in gcc probe (core_initcall) is too early -- RPM is
+ * not yet bound and qcom_rpm_write() has no target.

A very strange idea, but... Make clk-rpm driver export several resets for you, asserting / deasserting a reset would halt / unhalt the clocks. Then you can solve the problem for both GCC and MMCC drivers. Also, if you have a separate footswitch_enable() (instead of reusing gdsc_enable()), you can freely toggle necessary resets.

Quoted 23 lines
drivers/clk/qcom/mmcc-msm8660.c new file +23 -0
+ */
+static void mmcc_msm8660_unhalt_fabric_ports(struct device *dev)
+{
+	struct device_node *rpm_node;
+	struct platform_device *rpm_pdev;
+	struct qcom_rpm *rpm;
+	/* halt_data[0]=0 = CLK_UNHALT for all bits; halt_data[1] = port mask */
+	u32 mmss_halt[2] = {0, GENMASK(13, 0)};
+	u32 sys_halt[2]  = {0, GENMASK(16, 0)};
+	u32 apps_halt[2] = {0, GENMASK(3, 0)};
+	int rc;
+
+	rpm_node = of_find_compatible_node(NULL, NULL, "qcom,rpm-msm8660");
+	if (!rpm_node)
+		return;
+
+	rpm_pdev = of_find_device_by_node(rpm_node);
+	of_node_put(rpm_node);
+	if (!rpm_pdev)
+		return;
+
+	rpm = dev_get_drvdata(&rpm_pdev->dev);
+	if (!rpm) {

drivers/clk/qcom/mmcc-msm8660.c line 2944

Quoted 10 lines
drivers/clk/qcom/mmcc-msm8660.c new file +10 -0
+		put_device(&rpm_pdev->dev);
+		return;
+	}
+
+	rc = qcom_rpm_write(rpm, QCOM_RPM_ACTIVE_STATE,
+			    QCOM_RPM_MM_FABRIC_HALT, mmss_halt, 2);
+	if (rc)
+		dev_warn(dev, "MMSS fabric unhalt failed: %d\n", rc);
+	else
+		dev_info(dev, "MMSS fabric: unhalted all master ports (0-13)\n");

General comment: if nothing happened, keep it silent. Use dev_dbg() here, if unhalting succeeded.

Quoted 57 lines
drivers/clk/qcom/mmcc-msm8660.c new file +54 -0
+
+	rc = qcom_rpm_write(rpm, QCOM_RPM_ACTIVE_STATE,
+			    QCOM_RPM_SYS_FABRIC_HALT, sys_halt, 2);
+	if (rc)
+		dev_warn(dev, "system fabric unhalt failed: %d\n", rc);
+	else
+		dev_info(dev, "system fabric: unhalted all master ports (0-16)\n");
+
+	rc = qcom_rpm_write(rpm, QCOM_RPM_ACTIVE_STATE,
+			    QCOM_RPM_APPS_FABRIC_HALT, apps_halt, 2);
+	if (rc)
+		dev_warn(dev, "apps fabric unhalt failed: %d\n", rc);
+	else
+		dev_info(dev, "apps fabric: unhalted all master ports (0-3)\n");
+
+	put_device(&rpm_pdev->dev);
+}
+
+static int mmcc_msm8660_probe(struct platform_device *pdev)
+{
+	struct regmap *regmap;
+
+	regmap = qcom_cc_map(pdev, &mmcc_msm8660_desc);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	/* Initialize MMCC hardware before registering clocks */
+	mmcc_msm8660_init_hw(regmap);
+
+	/*
+	 * Unhalt all MMSS / SYSTEM / APPS fabric AXI master ports.  Must
+	 * happen before any peripheral (MMSS block for MMSS fabric;
+	 * CE2/ADM/SDC/USB for SYSTEM fabric) performs DMA.  Driven from
+	 * mmcc probe because it runs after the qcom_rpm platform driver
+	 * has bound (gcc core_initcall is too early).
+	 */
+	mmcc_msm8660_unhalt_fabric_ports(&pdev->dev);
+
+	return qcom_cc_really_probe(&pdev->dev, &mmcc_msm8660_desc, regmap);
+}
+
+static struct platform_driver mmcc_msm8660_driver = {
+	.probe		= mmcc_msm8660_probe,
+	.driver		= {
+		.name	= "mmcc-msm8660",
+		.of_match_table = mmcc_msm8660_match_table,
+	},
+};
+
+module_platform_driver(mmcc_msm8660_driver);
+
+MODULE_DESCRIPTION("Qualcomm MSM8x60 Multimedia Clock Controller driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:mmcc-msm8660");
Signature · 1 line
2.43.0
Signature · 2 lines
With best wishes
Dmitry
#9 lore raw

Re: [PATCH 1/2] dt-bindings: clock: qcom: add lcc-msm8660 LPASS clock IDs

In reply to Herman van Hazendonk
Quoted 9 lines · Herman van Hazendonk

Add the dt-binding clock-ID header for the MSM8x60 family (MSM8260/MSM8660/APQ8060) Low Power Audio SubSystem Clock Controller (LCC). The header enumerates the LPASS clocks consumed by the qcom,apq8060-lpaif sound card and the codec/AIF nodes downstream of it. It mirrors the format and ID range of the existing LCC headers for newer Qualcomm SoCs (lcc-msm8960, lcc-msm8974) so the drivers/clk/qcom/lcc-msm8660.c driver can be hooked up the same way once it lands.

Quoted 5 lines
---
 include/dt-bindings/clock/qcom,lcc-msm8660.h | 48 ++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 include/dt-bindings/clock/qcom,lcc-msm8660.h

Full bindings, please. You can extend the existing qcom,lcc.yaml, but the compatible should be defined and constrained.

Signature · 2 lines
With best wishes
Dmitry
#2 lore raw

Re: [PATCH 1/2] dt-bindings: interrupt-controller: qcom: add msm8660-mpm

In reply to Herman van Hazendonk
Quoted 118 lines · Herman van Hazendonk

Add the binding for the Modem Power Manager (MPM) interrupt controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The MPM is a small wake-capable interrupt aggregator that lets the SoC stay in low-power states while a small set of GIC SPIs continues to be monitored and can wake the system on an edge.

The binding describes its register region (in the RPM syscon), the GIC SPI it raises on a wake event, the GIC mailbox it shares with the RPM, and the per-pin wake-source mapping table.

---
 .../qcom,msm8660-mpm.yaml                     | 122 ++++++++++++++++++
 1 file changed, 122 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +96 -0
new file mode 100644
index 000000000000..72f9c2be03b2
@@ -0,0 +1,122 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/qcom,msm8660-mpm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm MSM8x60 MPM wakeup interrupt controller
+
+maintainers:
+  - Herman van Hazendonk <[email protected]>
+
+description: |
+  The MSM Power Manager (MPM) on the MSM8x60 family (MSM8260/MSM8660/
+  APQ8060) is an always-on hardware block that keeps a set of wake
+  interrupts alive while the application processor is powered down.
+  On MSM8x60 the vMPM registers live INSIDE the RPM control block
+  (request regs at RPM_BASE + 0x9d8, status at RPM_BASE + 0xdf8), with
+  notification of new wake-source configuration delivered via the
+  qcom-apcs-ipc mailbox (writing GCC + 0x008 bit 1).
+
+  This binding is MSM8x60-specific. Newer Qualcomm SoCs (sm6375, etc.)
+  use the qcom,mpm binding which assumes a dedicated MPM SRAM region
+  and an IPCC mailbox — neither holds on MSM8x60. The driver lives at
+  drivers/irqchip/irq-msm8660-mpm.c.
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    const: qcom,msm8660-mpm
+
+  interrupts:
+    maxItems: 1
+    description:
+      IPC IRQ raised by MPM when one of the enabled wake sources fires.
+      On MSM8x60 this is wired to GIC SPI 2.
+
+  qcom,rpm-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to the RPM control block exposed as a syscon. The vMPM
+      registers are accessed at offsets within this block.
+
+  qcom,mpm-offset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Offset of the vMPM register window within the RPM syscon. On
+      MSM8x60 this is 0x9d8. Status registers are at this offset + 0x420
+      (== 0xdf8 from RPM base).
+
+  mboxes:
+    maxItems: 1
+    description:
+      Mailbox channel used to notify MPM that the vMPM request
+      registers have been updated. On MSM8x60 this is the
+      qcom-apcs-ipc mailbox channel 1 (writes GCC + 0x008 bit 1).
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+    description:
+      First cell is the MPM pin number / parent GIC SPI; second cell
+      is the trigger type.
+
+  qcom,mpm-pin-count:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Total number of MPM pins exposed.
+
+  qcom,mpm-pin-map:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    items:
+      items:
+        - description: MPM pin number
+        - description: GIC SPI number this pin maps to
+    description: |
+      List of (MPM-pin, GIC-SPI) tuples for wake sources that have a
+      corresponding GIC IRQ. Consumers route their interrupts through
+      the MPM hierarchical irqdomain to take advantage of these
+      mappings.
+
+      Raw wake pins like SDC3/4 DATx (pins 21-24) are NOT listed
+      here — they have no GIC SPI mapping and are accessed through
+      the msm8660_mpm_set_pin_wake() / msm8660_mpm_enable_pin() C API
+      by consumer drivers.
+
+required:
+  - compatible
+  - interrupts
+  - qcom,rpm-syscon
+  - qcom,mpm-offset
+  - mboxes
+  - interrupt-controller
+  - '#interrupt-cells'
+  - qcom,mpm-pin-count

Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml line 106

Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +10 -0
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;

No need to, drop the soc and cells.

Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml line 108

Quoted 2 lines
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +2 -0
+
+        msm8660_mpm: interrupt-controller {

Drop the label.

Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml line 110

Quoted 2 lines
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +2 -0
+            compatible = "qcom,msm8660-mpm";
+            qcom,rpm-syscon = <&rpm>;

Can we define it under the RPM device node? Then you won't need to pass rpm like this.

Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml line 111

Quoted 1 line
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +1 -0
+            qcom,mpm-offset = <0x9d8>;

I assume it doesn't change on a chip. Use compatible to identify the offset.

But then it becomes compatible with the qcom,mpm.yaml schema. Just add qcom,msm8660-mpm to that file.

Quoted 14 lines
Documentation/devicetree/bindings/interrupt-controller/qcom,msm8660-mpm.yaml new file +11 -0
+            interrupts = <GIC_SPI 2 IRQ_TYPE_EDGE_RISING>;
+            mboxes = <&gcc_ipc 1>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+            interrupt-parent = <&intc>;
+            qcom,mpm-pin-count = <64>;
+            qcom,mpm-pin-map = <25 100>,
+                               <26 50>,
+                               <27 79>;
+        };
+    };
Signature · 1 line
2.43.0
Signature · 2 lines
With best wishes
Dmitry
#6 lore raw

Re: [PATCH 1/2] dt-bindings: thermal: qcom: add pm8901-temp-alarm

In reply to Herman van Hazendonk
Quoted 32 lines · Herman van Hazendonk

Add the binding for the temperature-alarm block inside the Qualcomm PM8901 PMIC (companion to the PM8058 on MSM8x60). The driver exposes the PM8901 die-temperature trip stages (105 / 125 / 145 C) to the thermal framework via a thermal-zone, with stage 3 wired as a critical trip so the kernel issues orderly_poweroff() when the part overheats.

The binding describes the SSBI sub-node address, the GIC interrupt the alarm raises on a stage transition, and the parent PMIC reference.

---
 .../thermal/qcom,pm8901-temp-alarm.yaml       | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml
Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml new file +9 -0
new file mode 100644
index 000000000000..569943b4aedc
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/qcom,pm8901-temp-alarm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm PM8901 PMIC Temperature Alarm
+
+maintainers:

Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml line 19

Quoted 10 lines
Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml new file +10 -0
+  - Herman van Hazendonk <[email protected]>
+
+description: |
+  PM8901 is a secondary PMIC paired with PM8058 on MSM8x60 family
+  (MSM8260/MSM8660/APQ8060) platforms. It exposes an over-temperature
+  alarm block at SSBI offset
+  0x23 (CTRL) / 0x24 (PWM) with four selectable thresholds and three
+  escalating stages. Unlike PM8058, there is no raw die-temperature
+  ADC channel — the driver decodes the stage + threshold pair into a
+  representative millicelsius value.

Don't describe the driver in the bindings.

Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml line 23

Quoted 4 lines
Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml new file +4 -0
+
+  Two PMIC-internal interrupts are exposed: a stage-transition alarm
+  (TEMP_ALARM, PM8901 IRQ block 6 bit 4 == 52) and a hi-temp alarm
+  (TEMP_HI_ALARM, block 6 bit 5 == 53).

I think, register definitions are too verbose for the bindings.

Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml line 26

Quoted 3 lines
Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml new file +3 -0
+
+  The driver registers a thermal-of sensor; board DTs declare trip
+  points and a critical-trip action against it.

The same.

Quoted 56 lines
Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml new file +53 -0
+
+properties:
+  compatible:
+    const: qcom,pm8901-temp-alarm
+
+  reg:
+    description: SSBI offset of the temp-alarm CTRL register.
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: Stage-transition alarm interrupt (TEMP_ALARM).
+      - description: Hi-temperature alarm interrupt (TEMP_HI_ALARM).
+
+  interrupt-names:
+    items:
+      - const: alarm
+      - const: hi-alarm
+
+  "#thermal-sensor-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - "#thermal-sensor-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    ssbi {
+      pmic {
+        compatible = "qcom,pm8901";
+        #interrupt-cells = <2>;
+        interrupt-controller;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pm8901_temp: temp-alarm@23 {
+          compatible = "qcom,pm8901-temp-alarm";
+          reg = <0x23>;
+          interrupts = <52 IRQ_TYPE_EDGE_RISING>,
+                       <53 IRQ_TYPE_EDGE_RISING>;
+          interrupt-names = "alarm", "hi-alarm";
+          #thermal-sensor-cells = <0>;
+        };
+      };
+    };
Signature · 1 line
2.43.0
Signature · 2 lines
With best wishes
Dmitry
#26 lore raw

Re: [PATCH 2/2] thermal: qcom: add PM8901 PMIC temperature-alarm driver

In reply to Herman van Hazendonk
Quoted 136 lines · Herman van Hazendonk

Add a thermal-of sensor driver for the temperature-alarm block inside the Qualcomm PM8901 PMIC. PM8901 is a secondary PMIC paired with PM8058 on the MSM8x60 family (MSM8260/MSM8660/APQ8060). It exposes an over-temperature alarm at SSBI offset 0x23/0x24 with three escalating stages (105/125/145 C); the driver decodes the stage + threshold pair into a millicelsius reading and registers two PMIC- internal interrupts (TEMP_ALARM at block 6 bit 4, TEMP_HI_ALARM at block 6 bit 5).

Used by board thermal-zones for the orderly_poweroff path on the HP TouchPad.

---
 drivers/thermal/qcom/Kconfig          |  12 +
 drivers/thermal/qcom/Makefile         |   1 +
 drivers/thermal/qcom/qcom-pm8901-tm.c | 341 ++++++++++++++++++++++++++
 3 files changed, 354 insertions(+)
 create mode 100644 drivers/thermal/qcom/qcom-pm8901-tm.c
drivers/thermal/qcom/Kconfig +12 -0
index a6bb01082ec6..af099032f1e6 100644
@@ -32,6 +32,18 @@ config QCOM_SPMI_TEMP_ALARM
 	  real time die temperature if an ADC is present or an estimate of the
 	  temperature based upon the over temperature stage value.
 
+config QCOM_PM8901_TEMP_ALARM
+	tristate "Qualcomm PM8901 PMIC Temperature Alarm"
+	depends on MFD_PM8XXX || COMPILE_TEST
+	depends on THERMAL_OF
+	help
+	  This enables the thermal driver for the PM8901 PMIC over-temperature
+	  alarm block. PM8901 exposes a stage-based alarm (no raw ADC) with
+	  four selectable thresholds and three escalating stages. The driver
+	  registers a thermal-of sensor so a board device tree can declare
+	  trip points and a critical-trip action (orderly_poweroff). Used on
+	  HP TouchPad (APQ8060) where PM8901 supplies the secondary PMIC die.
+
 config QCOM_LMH
 	tristate "Qualcomm Limits Management Hardware"
 	depends on ARCH_QCOM || COMPILE_TEST
drivers/thermal/qcom/Makefile +1 -0
index 0fa2512042e7..90dc05151e33 100644
@@ -5,4 +5,5 @@ qcom_tsens-y			+= tsens.o tsens-v2.o tsens-v1.o tsens-v0_1.o \
 				   tsens-8960.o
 obj-$(CONFIG_QCOM_SPMI_ADC_TM5)	+= qcom-spmi-adc-tm5.o
 obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
+obj-$(CONFIG_QCOM_PM8901_TEMP_ALARM)	+= qcom-pm8901-tm.o
 obj-$(CONFIG_QCOM_LMH)		+= lmh.o
drivers/thermal/qcom/qcom-pm8901-tm.c new file +77 -0
new file mode 100644
index 000000000000..d174d6897921
@@ -0,0 +1,341 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm PM8901 PMIC Thermal-Alarm Driver
+ *
+ * Mainline port of the legacy 2.6.35-palm drivers/thermal/pmic8901-tm.c.
+ * PM8901 exposes a stage-based over-temperature alarm (no raw ADC) with
+ * four selectable thresholds and three escalating stages. This driver
+ * mirrors the legacy programming exactly (threshold-set 0, software
+ * override enabled, PWM gating at 8 Hz) and registers a thermal-of
+ * sensor so a board DT can declare trip points and a critical action.
+ *
+ * Copyright (c) 2010-2011, Code Aurora Forum.
+ * Copyright (c) 2026, HP TouchPad mainline port.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/thermal.h>
+
+/* SSBI registers (offsets from the per-instance reg base) */
+#define PM8901_TM_REG_CTRL		0x00	/* CTRL/STATUS  (base + 0) */
+#define PM8901_TM_REG_PWM		0x01	/* PWM gating  (base + 1) */
+
+/* CTRL register fields */
+#define CTRL_ST3_SD			BIT(7)
+#define CTRL_ST2_SD			BIT(6)
+#define CTRL_STATUS_MASK		GENMASK(5, 4)
+#define CTRL_THRESH_MASK		GENMASK(3, 2)
+#define CTRL_OVRD_ST3			BIT(1)
+#define CTRL_OVRD_ST2			BIT(0)
+#define CTRL_OVRD_MASK			GENMASK(1, 0)
+
+/* PWM register fields */
+#define PWM_EN				BIT(7)
+#define PWM_PER_PRE_MASK		GENMASK(5, 3)
+#define PWM_PER_DIV_MASK		GENMASK(2, 0)
+
+/* Temperature math (from legacy pmic8901-tm.c) */
+#define PM8901_TEMP_STAGE_STEP		20000	/* 20 deg C between stages */
+#define PM8901_TEMP_STAGE_HYSTERESIS	2000	/*  2 deg C transition guard */
+#define PM8901_TEMP_THRESH_MIN		105000	/* threshold 0 base = 105 C */
+#define PM8901_TEMP_THRESH_STEP		5000	/*  5 deg C per threshold step */
+
+/*
+ * PM8901 has no real die ADC; when stage == 0 ("below threshold") we
+ * report a plausible idle estimate matching the legacy DEFAULT_NO_ADC_TEMP.
+ */
+#define PM8901_TEMP_NO_ALARM		37000
+
+struct pm8901_tm_chip {
+	struct device			*dev;
+	struct regmap			*map;
+	struct thermal_zone_device	*tz_dev;
+	struct mutex			lock;
+	unsigned int			base;	/* SSBI offset, from DT reg */
+	unsigned int			stage;
+	unsigned int			thresh;
+	int				temp;
+	bool				initialised;
+};
+
+static int pm8901_tm_read_ctrl(struct pm8901_tm_chip *chip, u8 *val)
+{
+	unsigned int v;
+	int ret;
+
+	ret = regmap_read(chip->map, chip->base + PM8901_TM_REG_CTRL, &v);
+	if (!ret)
+		*val = v;
+	return ret;
+}

drivers/thermal/qcom/qcom-pm8901-tm.c line 87

Quoted 10 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +10 -0
+
+static int pm8901_tm_write_ctrl(struct pm8901_tm_chip *chip, u8 val)
+{
+	return regmap_write(chip->map, chip->base + PM8901_TM_REG_CTRL, val);
+}
+
+static int pm8901_tm_write_pwm(struct pm8901_tm_chip *chip, u8 val)
+{
+	return regmap_write(chip->map, chip->base + PM8901_TM_REG_PWM, val);
+}

I'm personally not a fan of one-line wrappers (all three), but it's up to you and the maintainers...

Quoted 58 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +58 -0
+
+/*
+ * Decode the (stage, threshold) pair into a single millicelsius value.
+ * Logic matches the legacy pmic8901-tm.c hysteresis selection:
+ *  - on a rising stage transition, use the lower bound of the new stage
+ *    plus +HYSTERESIS so we don't bounce
+ *  - on a falling stage transition, use the upper bound of the new stage
+ *    minus -HYSTERESIS
+ *  - on the first read after probe (initialised == false), pick a
+ *    representative point: midpoint of the stage range, or
+ *    PM8901_TEMP_NO_ALARM when stage == 0.
+ */
+static int pm8901_tm_update_temp_locked(struct pm8901_tm_chip *chip)
+{
+	unsigned int new_stage;
+	u8 reg;
+	int ret;
+
+	ret = pm8901_tm_read_ctrl(chip, &reg);
+	if (ret)
+		return ret;
+
+	new_stage = FIELD_GET(CTRL_STATUS_MASK, reg);
+	chip->thresh = FIELD_GET(CTRL_THRESH_MASK, reg);
+
+	if (!chip->initialised) {
+		if (new_stage)
+			chip->temp = PM8901_TEMP_THRESH_MIN +
+				     chip->thresh * PM8901_TEMP_THRESH_STEP +
+				     (new_stage - 1) * PM8901_TEMP_STAGE_STEP;
+		else
+			chip->temp = PM8901_TEMP_NO_ALARM;
+		chip->initialised = true;
+	} else if (new_stage > chip->stage) {
+		chip->temp = PM8901_TEMP_THRESH_MIN +
+			     chip->thresh * PM8901_TEMP_THRESH_STEP +
+			     (new_stage - 1) * PM8901_TEMP_STAGE_STEP +
+			     PM8901_TEMP_STAGE_HYSTERESIS;
+	} else if (new_stage < chip->stage) {
+		chip->temp = PM8901_TEMP_THRESH_MIN +
+			     chip->thresh * PM8901_TEMP_THRESH_STEP +
+			     new_stage * PM8901_TEMP_STAGE_STEP -
+			     PM8901_TEMP_STAGE_HYSTERESIS;
+	}
+
+	chip->stage = new_stage;
+	return 0;
+}
+
+static int pm8901_tm_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+	struct pm8901_tm_chip *chip = thermal_zone_device_priv(tz);
+	int ret;
+
+	if (!temp)
+		return -EINVAL;
+
+	mutex_lock(&chip->lock);

drivers/thermal/qcom/qcom-pm8901-tm.c line 155

Quoted 10 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +10 -0
+	ret = pm8901_tm_update_temp_locked(chip);
+	if (!ret)
+		*temp = chip->temp;
+	mutex_unlock(&chip->lock);
+
+	return ret;
+}
+
+static const struct thermal_zone_device_ops pm8901_tm_zone_ops = {
+	.get_temp = pm8901_tm_get_temp,

You don't have to implement it, but please add a comment. Are those hardcoded?

drivers/thermal/qcom/qcom-pm8901-tm.c line 196

Quoted 4 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +4 -0
+};
+
+/*
+ * Program PM8901 to the legacy default: threshold-set 0 (105 / 125 / 145 C),

Any other options?

Quoted 92 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +92 -0
+ * software override enabled (kernel handles shutdown, PMIC does not auto-cut),
+ * PWM at 8 Hz (legacy "cut down on unnecessary interrupts" rate).
+ */
+static int pm8901_tm_init_hw(struct pm8901_tm_chip *chip)
+{
+	int ret;
+	u8 reg;
+
+	mutex_lock(&chip->lock);
+
+	ret = pm8901_tm_read_ctrl(chip, &reg);
+	if (ret)
+		goto out;
+
+	/*
+	 * Enable software override so PMIC does NOT auto-shut-down on stage 3.
+	 * Critical-trip orderly_poweroff is delivered by the kernel thermal
+	 * core via the DT thermal-zone trip with type = "critical".
+	 */
+	reg = (reg & ~(CTRL_OVRD_MASK | CTRL_STATUS_MASK | CTRL_THRESH_MASK)) |
+	      CTRL_OVRD_ST3 | CTRL_OVRD_ST2;
+	ret = pm8901_tm_write_ctrl(chip, reg);
+	if (ret)
+		goto out;
+
+	chip->thresh = 0;
+
+	/* PWM @ 8 Hz: PWM_EN | PRE=3 | DIV=3 — verbatim from legacy. */
+	reg = PWM_EN | FIELD_PREP(PWM_PER_PRE_MASK, 3) |
+	      FIELD_PREP(PWM_PER_DIV_MASK, 3);
+	ret = pm8901_tm_write_pwm(chip, reg);
+	if (ret)
+		goto out;
+
+	/* Prime the cached temperature from current hardware state. */
+	chip->initialised = false;
+	ret = pm8901_tm_update_temp_locked(chip);
+
+out:
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+static int pm8901_tm_probe(struct platform_device *pdev)
+{
+	struct pm8901_tm_chip *chip;
+	int ret, irq_alarm, irq_hi_alarm;
+	u32 res;
+
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->dev = &pdev->dev;
+	mutex_init(&chip->lock);
+
+	chip->map = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!chip->map)
+		return dev_err_probe(&pdev->dev, -ENXIO,
+				     "no regmap on PM8901 parent\n");
+
+	ret = of_property_read_u32(pdev->dev.of_node, "reg", &res);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "missing reg property\n");
+	chip->base = res;
+
+	irq_alarm = platform_get_irq_byname(pdev, "alarm");
+	if (irq_alarm < 0)
+		return irq_alarm;
+	irq_hi_alarm = platform_get_irq_byname(pdev, "hi-alarm");
+	if (irq_hi_alarm < 0)
+		return irq_hi_alarm;
+
+	ret = pm8901_tm_init_hw(chip);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "hw init failed\n");
+
+	chip->tz_dev = devm_thermal_of_zone_register(&pdev->dev, 0, chip,
+						     &pm8901_tm_zone_ops);
+	if (IS_ERR(chip->tz_dev))
+		return dev_err_probe(&pdev->dev, PTR_ERR(chip->tz_dev),
+				     "thermal zone register failed\n");
+
+	ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL,
+					pm8901_tm_isr, IRQF_ONESHOT,
+					"pm8901-tm-alarm", chip);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "alarm IRQ request failed\n");
+
+	ret = devm_request_threaded_irq(&pdev->dev, irq_hi_alarm, NULL,

drivers/thermal/qcom/qcom-pm8901-tm.c line 298

Quoted 10 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +10 -0
+					pm8901_tm_isr, IRQF_ONESHOT,
+					"pm8901-tm-hi-alarm", chip);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "hi-alarm IRQ request failed\n");
+
+	platform_set_drvdata(pdev, chip);
+	thermal_zone_device_update(chip->tz_dev, THERMAL_EVENT_UNSPECIFIED);
+
+	dev_info(&pdev->dev,

dev_dbg() or drop it completely.

Quoted 31 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +31 -0
+		 "PM8901 thermal alarm: base=0x%x stage=%u thresh=%u temp=%d\n",
+		 chip->base, chip->stage, chip->thresh, chip->temp);
+
+	return 0;
+}
+
+static void pm8901_tm_remove(struct platform_device *pdev)
+{
+	struct pm8901_tm_chip *chip = platform_get_drvdata(pdev);
+	u8 reg;
+
+	/*
+	 * Disable software override on the way out so the PMIC reverts to
+	 * its hardware auto-cut behaviour if the kernel is no longer the
+	 * shutdown agent. Best-effort: ignore errors.
+	 */
+	mutex_lock(&chip->lock);
+	if (!pm8901_tm_read_ctrl(chip, &reg)) {
+		reg &= ~CTRL_OVRD_MASK;
+		pm8901_tm_write_ctrl(chip, reg);
+	}
+	mutex_unlock(&chip->lock);
+}
+
+static const struct of_device_id pm8901_tm_match_table[] = {
+	{ .compatible = "qcom,pm8901-temp-alarm" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pm8901_tm_match_table);
+
+static struct platform_driver pm8901_tm_driver = {

drivers/thermal/qcom/qcom-pm8901-tm.c line 339

Quoted 10 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +10 -0
+	.driver = {
+		.name		= "pm8901-temp-alarm",
+		.of_match_table	= pm8901_tm_match_table,
+	},
+	.probe	= pm8901_tm_probe,
+	.remove	= pm8901_tm_remove,
+};
+module_platform_driver(pm8901_tm_driver);
+
+MODULE_ALIAS("platform:pm8901-temp-alarm");

No need for platform aliases (neither here nor in any other driver which has MODULE_DEVICE_TABLE(of).

Quoted 5 lines
drivers/thermal/qcom/qcom-pm8901-tm.c new file +2 -0
+MODULE_DESCRIPTION("Qualcomm PM8901 PMIC Thermal Alarm driver");
+MODULE_LICENSE("GPL v2");
Signature · 1 line
2.43.0
Signature · 2 lines
With best wishes
Dmitry
#18 lore raw

Re: [PATCH 3/3] clk: qcom: add MSM8x60 MMCC driver

In reply to Herman van Hazendonk
Quoted 22 lines · Herman van Hazendonk

Add a clock driver for the Multimedia Clock Controller (MMCC) on the MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The MMCC layout on MSM8x60 differs from MSM8960 in several ways that make a separate driver cleaner than parameterising mmcc-msm8960.c:

- the pix_rdi mux requires a custom set_parent op that temporarily

    enables both parents during the glitch-free transition;
  - the IJPEG GDSC requires releasing AXI, AHB and CORE resets;
  - several rate-source pairs (MDP pixel, GFX2D/3D) only exist on
    8x60 (e.g. PLL2-derived 228571000/266667000 for graphics);

- the camera CSI / VFE / JPEG / VPE / ROT clock topology lacks the later 8960 reorganisation.

Used on the HP TouchPad (Tenderloin) for graphics (Adreno A220), display (MDP4), camera (CSI/VFE), JPEG (Gemini), VIDC, VPE and rotator. Reset IDs are exposed via a separate header so consumers can reset the GDSCs and individual blocks.

---

[...]

drivers/clk/qcom/mmcc-msm8660.c line 65

Quoted 4 lines
drivers/clk/qcom/mmcc-msm8660.c new file +4 -0
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "pll2",
+		.parent_data = (const struct clk_parent_data[]){
+			{ .fw_name = "pxo", .name = "pxo_board" },

Please drop .name (the kernel-global clock lookup), this is only a backwards compatiblity measure on existing drivers. For new entries, .index is the best (because well, it's the fastest)

[...]

drivers/clk/qcom/mmcc-msm8660.c line 177

Quoted 8 lines
drivers/clk/qcom/mmcc-msm8660.c new file +8 -0
+static struct clk_branch camclk0_clk = {
+	.halt_reg = 0x01e8,
+	.halt_bit = 15,
+	/*
+	 * The legacy webOS kernel used halt_reg = NULL for this clock,
+	 * meaning it never checked the halt status. The hardware doesn't
+	 * properly report the clock state via the halt register. Use
+	 * BRANCH_HALT_SKIP to avoid the "status stuck at 'off'" warning.

It may be that some piece of hw is holding this clock online behind the scenes. Is there perhaps a Qualcomm-authored commit that mentions the hw bug, or is it a guess? Due to the age of this chip I would imagine I won't be find an answer if you don't have one..

[...]

I see you have a lot of inline note-to-self comments, please strip some of them.

Konrad

#19 lore raw

Re: [PATCH 3/3] clk: qcom: add MSM8x60 MMCC driver

Quoted 35 lines · Konrad Dybcio
Quoted 22 lines · Herman van Hazendonk

Add a clock driver for the Multimedia Clock Controller (MMCC) on the MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The MMCC layout on MSM8x60 differs from MSM8960 in several ways that make a separate driver cleaner than parameterising mmcc-msm8960.c:

- the pix_rdi mux requires a custom set_parent op that temporarily

    enables both parents during the glitch-free transition;
  - the IJPEG GDSC requires releasing AXI, AHB and CORE resets;
  - several rate-source pairs (MDP pixel, GFX2D/3D) only exist on
    8x60 (e.g. PLL2-derived 228571000/266667000 for graphics);

- the camera CSI / VFE / JPEG / VPE / ROT clock topology lacks the later 8960 reorganisation.

Used on the HP TouchPad (Tenderloin) for graphics (Adreno A220), display (MDP4), camera (CSI/VFE), JPEG (Gemini), VIDC, VPE and rotator. Reset IDs are exposed via a separate header so consumers can reset the GDSCs and individual blocks.

---

[...]

drivers/clk/qcom/mmcc-msm8660.c line 65

Quoted 4 lines
drivers/clk/qcom/mmcc-msm8660.c new file +4 -0
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "pll2",
+		.parent_data = (const struct clk_parent_data[]){
+			{ .fw_name = "pxo", .name = "pxo_board" },

Please drop .name (the kernel-global clock lookup), this is only a backwards compatiblity measure on existing drivers. For new entries, .index is the best (because well, it's the fastest)

Quoted 1 line

[...]

Ok will rework in the new version.

Quoted 14 lines

drivers/clk/qcom/mmcc-msm8660.c line 177

Quoted 9 lines
drivers/clk/qcom/mmcc-msm8660.c new file +8 -0
+static struct clk_branch camclk0_clk = {
+	.halt_reg = 0x01e8,
+	.halt_bit = 15,
+	/*
+	 * The legacy webOS kernel used halt_reg = NULL for this clock,
+	 * meaning it never checked the halt status. The hardware doesn't
+	 * properly report the clock state via the halt register. Use
+	 * BRANCH_HALT_SKIP to avoid the "status stuck at 'off'" warning.

It may be that some piece of hw is holding this clock online behind the scenes. Is there perhaps a Qualcomm-authored commit that mentions the hw bug, or is it a guess? Due to the age of this chip I would imagine I won't be find an answer if you don't have one..

Re: the halt-status decision, it's not a guess. Downstream Qualcomm-authored BSP arch/arm/mach-msm/clock-8x60.c configures CAMCLK_CC_REG with halt_check = DELAY, indicating Qualcomm engineering deliberately avoided polling the halt register on this clock. The right mainline mapping
is BRANCH_HALT_DELAY rather than the BRANCH_HALT_SKIP I used (which works, but seemingly incorrect); I'll switch it in the new version and apply the same DELAY-vs-NOCHECK mapping to the other status-stuck clocks I flagged.

Quoted 7 lines

[...]

I see you have a lot of inline note-to-self comments, please strip some of them.

Konrad

I'll also strip the inline note-to-self comments fair point, they belonged in the commit message rather than in the code.

Thanks,
Herman

Reviewed-by on v2 3/3
#23 lore raw

Re: [PATCH 3/3] clk: qcom: add MSM8x60 MMCC driver

In reply to Herman van Hazendonk
Quoted 15 lines · Herman van Hazendonk

Add a clock driver for the Multimedia Clock Controller (MMCC) on the MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The MMCC layout on MSM8x60 differs from MSM8960 in several ways that make a separate driver cleaner than parameterising mmcc-msm8960.c:

- the pix_rdi mux requires a custom set_parent op that temporarily

    enables both parents during the glitch-free transition;
  - the IJPEG GDSC requires releasing AXI, AHB and CORE resets;
  - several rate-source pairs (MDP pixel, GFX2D/3D) only exist on
    8x60 (e.g. PLL2-derived 228571000/266667000 for graphics);

- the camera CSI / VFE / JPEG / VPE / ROT clock topology lacks the later 8960 reorganisation.

Quoted 6 lines

Used on the HP TouchPad (Tenderloin) for graphics (Adreno A220), display (MDP4), camera (CSI/VFE), JPEG (Gemini), VIDC, VPE and rotator. Reset IDs are exposed via a separate header so consumers can reset the GDSCs and individual blocks.

I spent some time looking at this driver and comparing it to my earlier attempt at the same, it's basically a better and more complete version of what I was hacking on so naturally I'm a big fan of this driver.

I can't find anything more to add, except perhaps a top-level comment that in some documentation PLL2 is called called MM_PLL1 and PLL3 is called MM_PLL2 which can be confusing, so it's good to mention this somewhere.

Yours,
Linus Walleij

#20 lore raw

Re: [PATCH 3/3] clk: qcom: add MSM8x60 MMCC driver

In reply to Konrad Dybcio
Quoted 35 lines · Konrad Dybcio
Quoted 22 lines · Herman van Hazendonk

Add a clock driver for the Multimedia Clock Controller (MMCC) on the MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The MMCC layout on MSM8x60 differs from MSM8960 in several ways that make a separate driver cleaner than parameterising mmcc-msm8960.c:

- the pix_rdi mux requires a custom set_parent op that temporarily

    enables both parents during the glitch-free transition;
  - the IJPEG GDSC requires releasing AXI, AHB and CORE resets;
  - several rate-source pairs (MDP pixel, GFX2D/3D) only exist on
    8x60 (e.g. PLL2-derived 228571000/266667000 for graphics);

- the camera CSI / VFE / JPEG / VPE / ROT clock topology lacks the later 8960 reorganisation.

Used on the HP TouchPad (Tenderloin) for graphics (Adreno A220), display (MDP4), camera (CSI/VFE), JPEG (Gemini), VIDC, VPE and rotator. Reset IDs are exposed via a separate header so consumers can reset the GDSCs and individual blocks.

---

[...]

drivers/clk/qcom/mmcc-msm8660.c line 65

Quoted 4 lines
drivers/clk/qcom/mmcc-msm8660.c new file +4 -0
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "pll2",
+		.parent_data = (const struct clk_parent_data[]){
+			{ .fw_name = "pxo", .name = "pxo_board" },

Please drop .name (the kernel-global clock lookup), this is only a backwards compatiblity measure on existing drivers. For new entries, .index is the best (because well, it's the fastest)

Quoted 4 lines

[...]

Review

Quoted 2 lines
(quoted diff) +2 -0
+static struct clk_branch camclk0_clk = {
+	.halt_reg = 0x01e8,

From msm-3.0:

+#define DBG_BUS_VEC_I_REG REG_MM(0x01E8)

So, it seems, it is a debug reg, not an actual halt reg (but I might be mistaken here, I haven't looked into the details).

Quoted 18 lines

drivers/clk/qcom/mmcc-msm8660.c line 177

Quoted 6 lines
drivers/clk/qcom/mmcc-msm8660.c new file +6 -0
+	.halt_bit = 15,
+	/*
+	 * The legacy webOS kernel used halt_reg = NULL for this clock,
+	 * meaning it never checked the halt status. The hardware doesn't
+	 * properly report the clock state via the halt register. Use
+	 * BRANCH_HALT_SKIP to avoid the "status stuck at 'off'" warning.

It may be that some piece of hw is holding this clock online behind the scenes. Is there perhaps a Qualcomm-authored commit that mentions the hw bug, or is it a guess? Due to the age of this chip I would imagine I won't be find an answer if you don't have one..

[...]

I see you have a lot of inline note-to-self comments, please strip some of them.

Konrad

Signature · 2 lines
With best wishes
Dmitry
#21 lore raw

Re: [PATCH 3/3] clk: qcom: add MSM8x60 MMCC driver

Quoted 41 lines · Dmitry Baryshkov
Quoted 31 lines · Konrad Dybcio
Quoted 22 lines · Herman van Hazendonk

Add a clock driver for the Multimedia Clock Controller (MMCC) on the MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The MMCC layout on MSM8x60 differs from MSM8960 in several ways that make a separate driver cleaner than parameterising mmcc-msm8960.c:

- the pix_rdi mux requires a custom set_parent op that temporarily

    enables both parents during the glitch-free transition;
  - the IJPEG GDSC requires releasing AXI, AHB and CORE resets;
  - several rate-source pairs (MDP pixel, GFX2D/3D) only exist on
    8x60 (e.g. PLL2-derived 228571000/266667000 for graphics);

- the camera CSI / VFE / JPEG / VPE / ROT clock topology lacks the later 8960 reorganisation.

Used on the HP TouchPad (Tenderloin) for graphics (Adreno A220), display (MDP4), camera (CSI/VFE), JPEG (Gemini), VIDC, VPE and rotator. Reset IDs are exposed via a separate header so consumers can reset the GDSCs and individual blocks.

---

[...]

drivers/clk/qcom/mmcc-msm8660.c line 65

Quoted 4 lines
drivers/clk/qcom/mmcc-msm8660.c new file +4 -0
+   .clkr.hw.init = &(struct clk_init_data){
+           .name = "pll2",
+           .parent_data = (const struct clk_parent_data[]){
+                   { .fw_name = "pxo", .name = "pxo_board" },
Quoted 8 lines

Please drop .name (the kernel-global clock lookup), this is only a backwards compatiblity measure on existing drivers. For new entries, .index is the best (because well, it's the fastest)

[...]

Quoted 2 lines
(quoted diff) +2 -0
+static struct clk_branch camclk0_clk = {
+   .halt_reg = 0x01e8,
Quoted 7 lines

From msm-3.0:

+#define DBG_BUS_VEC_I_REG REG_MM(0x01E8)

So, it seems, it is a debug reg, not an actual halt reg (but I might be mistaken here, I haven't looked into the details).

I think the define just has a bad name. You have to look at how it's used.

This define is in msm-3.4 for msm8960, clock-msm8960:
#define DBG_BUS_VEC_I_REG REG_MM(0x01E8)
... and then it is used in 10 different places as a halt reg:

$ git grep DBG_BUS_VEC_I_REG *
clock-8960.c:#define DBG_BUS_VEC_I_REG                  REG_MM(0x01E8)
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:                   .halt_reg = DBG_BUS_VEC_I_REG, \
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,

and in the mainline msm8960 driver 01e8 is used as a halt reg in the as well, see drivers/clk/qcom/mmcc-msm8960.c.

Some qualcomm clock registers being named DBG_* has no semantic meaning, one has to look at the usage. Maybe at some point some engineer thought it's a debug feature to be able to halt a clock.

Yours,
Linus Walleij

#22 lore raw

Re: [PATCH 3/3] clk: qcom: add MSM8x60 MMCC driver

Quoted 71 lines · Linus Walleij
Quoted 41 lines · Dmitry Baryshkov
Quoted 31 lines · Konrad Dybcio
Quoted 22 lines · Herman van Hazendonk

Add a clock driver for the Multimedia Clock Controller (MMCC) on the MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class generation that preceded MSM8960's Krait CPUs.

The MMCC layout on MSM8x60 differs from MSM8960 in several ways that make a separate driver cleaner than parameterising mmcc-msm8960.c:

- the pix_rdi mux requires a custom set_parent op that temporarily

    enables both parents during the glitch-free transition;
  - the IJPEG GDSC requires releasing AXI, AHB and CORE resets;
  - several rate-source pairs (MDP pixel, GFX2D/3D) only exist on
    8x60 (e.g. PLL2-derived 228571000/266667000 for graphics);

- the camera CSI / VFE / JPEG / VPE / ROT clock topology lacks the later 8960 reorganisation.

Used on the HP TouchPad (Tenderloin) for graphics (Adreno A220), display (MDP4), camera (CSI/VFE), JPEG (Gemini), VIDC, VPE and rotator. Reset IDs are exposed via a separate header so consumers can reset the GDSCs and individual blocks.

---

[...]

drivers/clk/qcom/mmcc-msm8660.c line 65

Quoted 4 lines
drivers/clk/qcom/mmcc-msm8660.c new file +4 -0
+   .clkr.hw.init = &(struct clk_init_data){
+           .name = "pll2",
+           .parent_data = (const struct clk_parent_data[]){
+                   { .fw_name = "pxo", .name = "pxo_board" },
Quoted 8 lines

Please drop .name (the kernel-global clock lookup), this is only a backwards compatiblity measure on existing drivers. For new entries, .index is the best (because well, it's the fastest)

[...]

Quoted 2 lines
(quoted diff) +2 -0
+static struct clk_branch camclk0_clk = {
+   .halt_reg = 0x01e8,
Quoted 7 lines

From msm-3.0:

+#define DBG_BUS_VEC_I_REG REG_MM(0x01E8)

So, it seems, it is a debug reg, not an actual halt reg (but I might be mistaken here, I haven't looked into the details).

I think the define just has a bad name. You have to look at how it's used.

This define is in msm-3.4 for msm8960, clock-msm8960:
#define DBG_BUS_VEC_I_REG REG_MM(0x01E8)
... and then it is used in 10 different places as a halt reg:

$ git grep DBG_BUS_VEC_I_REG *
clock-8960.c:#define DBG_BUS_VEC_I_REG                  REG_MM(0x01E8)
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:                   .halt_reg = DBG_BUS_VEC_I_REG, \
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
clock-8960.c:           .halt_reg = DBG_BUS_VEC_I_REG,
Quoted 2 lines

and in the mainline msm8960 driver 01e8 is used as a halt reg in the as well, see drivers/clk/qcom/mmcc-msm8960.c.

Ack. It would be nice if this was mentioned in the commit message.

With the commit message updated, it makes sense to me now.

Quoted 8 lines

Some qualcomm clock registers being named DBG_* has no semantic meaning, one has to look at the usage. Maybe at some point some engineer thought it's a debug feature to be able to halt a clock.

Yours,
Linus Walleij

Signature · 2 lines
With best wishes
Dmitry