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

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

[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.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 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

diff --git a/include/dt-bindings/clock/qcom,mmcc-msm8660.h b/include/dt-bindings/clock/qcom,mmcc-msm8660.h
new file mode 100644
index 000000000000..00c3a75e8b71
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,mmcc-msm8660.h
@@ -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
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Silent failure to unhalt MMSS fabric if qcom_rpm is not bound yet.
--

commit 7a52c01ee4faffcac27f221b8547899fcd741b7b
Author: Herman van Hazendonk <github.com@herrie.org>

clk: qcom: add MSM8x60 MMCC driver

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.

> diff --git a/drivers/clk/qcom/mmcc-msm8660.c b/drivers/clk/qcom/mmcc-msm8660.c
> new file mode 100644
> index 0000000000000..50781a63e65fe
> --- /dev/null
> +++ b/drivers/clk/qcom/mmcc-msm8660.c
[ ... ]
> +static void mmcc_msm8660_unhalt_fabric_ports(struct device *dev)
> +{
> +	struct device_node *rpm_node;
> +	struct platform_device *rpm_pdev;
> +	struct device_link *link;
> +	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)};
> +	int rc;
[ ... ]
> +	rpm = dev_get_drvdata(&rpm_pdev->dev);
> +	if (!rpm) {
> +		/*
> +		 * Supplier exists but has not bound yet -- skip the
> +		 * unhalt rather than blocking mmcc probe; downstream
> +		 * MMSS clients will be enabled on demand.
> +		 */
> +		put_device(&rpm_pdev->dev);
> +		return;
> +	}

[Severity: High]
Does this code silently fail to unhalt the MMSS fabric if the qcom_rpm driver
has not yet probed?

If dev_get_drvdata() returns NULL because the RPM driver hasn't bound yet,
this function returns without an error. Since mmcc_msm8660_probe() calls this
and ignores any failure, it will complete probe successfully but leave the
MMSS fabric permanently halted.

The comment here states "downstream MMSS clients will be enabled on demand",
but this contradicts the block comment above indicating that mainline GDSC
does not implement on-demand unhalting. When multimedia hardware later powers
on and attempts DMA, the hardware queues will freeze and stall the system.

Should mmcc_msm8660_unhalt_fabric_ports() return an int so we can pass
-EPROBE_DEFER back to the probe function to wait for the RPM driver?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602043623.285901-1-github.com@herrie.org?part=1

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

[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.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 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

diff --git a/include/dt-bindings/reset/qcom,mmcc-msm8660.h b/include/dt-bindings/reset/qcom,mmcc-msm8660.h
new file mode 100644
index 000000000000..c3ffd57834c9
--- /dev/null
+++ b/include/dt-bindings/reset/qcom,mmcc-msm8660.h
@@ -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
-- 
2.43.0

Re: [PATCH 2/3] dt-bindings: reset: qcom: add mmcc-msm8660 reset IDs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unmapped GDSC reset IDs (GFX2D0_AHB_RESET, GFX2D1_AHB_RESET) cause memory corruption at register 0x0.
- [Low] Typographical errors in newly defined reset ID macro names.
--

commit 318183bccf1215fa02d8a5a344be5c0173d40944
Author: Herman van Hazendonk <github.com@herrie.org>

dt-bindings: reset: qcom: add mmcc-msm8660 reset IDs

This commit adds the dt-binding reset-ID header for the MSM8x60 family
(MSM8260/MSM8660/APQ8060) MMCC reset lines. It exposes per-IP software-reset
bits with IDs matching the numeric values from the mmcc-msm8960.h bindings.

> diff --git a/include/dt-bindings/reset/qcom,mmcc-msm8660.h b/include/dt-bindings/reset/qcom,mmcc-msm8660.h
> new file mode 100644
> index 0000000000000..c3ffd57834c97
> --- /dev/null
> +++ b/include/dt-bindings/reset/qcom,mmcc-msm8660.h
> @@ -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

[Severity: Low]
This isn't a bug, but should this be MDP_AXI_RESET instead of MPD? Propagating
misspelled macro names from the older platform might create confusion without
providing any ABI benefits.

> +#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

[Severity: Low]
This isn't a bug, but should this be SMMU_GFX3D_AHB_RESET instead of ABH?

> +#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

[Severity: High]
Are these reset IDs missing from the mmcc_msm8660_resets array in the driver?

Looking at drivers/clk/qcom/mmcc-msm8660.c, gfx2d0_gdsc and gfx2d1_gdsc specify
.resets using GFX2D0_AHB_RESET (30) and GFX2D1_AHB_RESET (31):

drivers/clk/qcom/mmcc-msm8660.c:gfx2d0_gdsc
    .resets = (unsigned int []){ GFX2D0_AHB_RESET },

However, indices 30 and 31 appear to be skipped in the mmcc_msm8660_resets array:

drivers/clk/qcom/mmcc-msm8660.c:mmcc_msm8660_resets() {
    ...
    [FABRIC_AHB_RESET] = { 0x020c, 13 },
    [GFX3D_AHB_RESET] = { 0x020c, 10 },
    ...
}

Because omitted indices in a static array are implicitly zero-initialized,
accesses to rst->reset_map[30] and rst->reset_map[31] would yield { .reg = 0,
.bit = 0 }.

During GDSC power-on/off, qcom_reset_set_assert() uses these zeroed entries to
perform a read-modify-write that erroneously toggles bit 0 of register 0x0000.
Could this regression silently corrupt clock controller state instead of
resetting the intended block?

> +#define GFX3D_AHB_RESET					32
> +#define HDMI_AHB_RESET					33
> +#define MSSS_IMEM_AHB_RESET				34

[Severity: Low]
This isn't a bug, but would it make sense to name this MMSS_IMEM_AHB_RESET
instead of MSSS?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602043623.285901-1-github.com@herrie.org?part=2

Re: [PATCH 0/3] clk: qcom: add MSM8x60 Multimedia Clock Controller (MMCC) driver

[Herman van Hazendonk]

Hi,

Two things I need to flag against this v1 before further review:

1. Missed prerequisite (build break on a clean tree).

   mmcc-msm8660.c references LEGACY_FOOTSWITCH (4 GDSCs: ROT, IJPEG,
   VFE, VPE) and RPM_ALWAYS_ON (GFX3D), both of which are not in
   mainline gdsc.{c,h} today. I was supposed to send the GDSC
   framework groundwork series first; I sent mmcc by mistake before
   it. The prereq is incoming on linux-clk in a separate thread:

       Subject: [PATCH 0/2] clk: qcom: gdsc: groundwork for
                MSM8x60 power domains

   v2 of this MMCC series will declare the dependency explicitly in
   the cover letter and will not be sent before the gdsc series has
   review traction.

2. mmcc_msm8660_unhalt_fabric_ports() silent-fail bug.

   The function returns silently on every "RPM supplier not ready"
   path (no qcom,rpm-msm8660 node, of_find_device_by_node() returns
   NULL, device_link_add() fails, dev_get_drvdata() returns NULL).
   probe() ignores it. The "downstream clients will be enabled on
   demand" comment is wrong: mainline GDSC does not re-issue the
   unhalt on power-domain enable, so a system where qcom_rpm has
   not yet bound when mmcc probes ends up with the MMSS AXI fabric
   permanently halted. First MMSS DMA (MDP / CAMSS / GFX / JPEG /
   VPE / HDCODEC) silently freezes.

   v2 will:
     - change unhalt_fabric_ports() to return int,
     - return -EPROBE_DEFER on every "supplier not ready" path,
     - return -ENODEV via dev_err_probe() when the DT node is
       absent entirely,
     - propagate the result through probe() so the driver core
       retries when qcom_rpm finally binds.

   The patch is in my local tree and on-device validated; I will
   roll it into v2 once initial review feedback on the rest of the
   series has had a chance to settle (and once the gdsc prereq is
   on-list).

Apologies for the noise. v2 is coming, just not immediately.

Thanks,
Herman