Skip to contentSkip to conversation

LLCC: Support for Broadcast_AND region

PATCH Applied Unnathi Chalicheemala 3 participants 9 messages May 31, 2024 – Jul 17, 2024

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

v6 cover LLCC: Support for Broadcast_AND region

This series adds:
1. Device tree register mapping for Broadcast_AND region in SM8450, SM8550, SM8650.
2. LLCC driver updates to reflect addition of Broadcast_AND regmap.

To support CSR programming, a broadcast interface is used to program all channels in a single command. Until SM8450 there was only one broadcast region (Broadcast_OR) used to broadcast write and check for status bit
0. From SM8450 onwards another broadcast region (Broadcast_AND) has been added which checks for status bit 1.

This series updates the device trees from SM8450 onwards to have a mapping to this Broadcast_AND region. It also updates the llcc_drv_data structure with a regmap for Broadcast_AND region and corrects the broadcast region used to check for status bit 1.

Changes in v6:
- Update commit message of driver code patch to include problem statement.

Changes in v5:
- Add additional check to remove warning from devres.c on older chipsets.
- Carried over Bjorn's and Krzysztof's R-b tags from v4.

Changes in v4:
- Updated Devicetree patches' commit messages to make problem statement clearer
- Resolved Konrad's comments on driver code patch
- Updated v3 changelog to include dropped R-b tag

Changes in v3:
- Removed new example in dt-bindings patch and ran 'make DT_CHECKER_FLAGS=-m dt_binding_check'
- Dropped Krzysztof's R-b tag on dt-bindings patch
- Use of ternary operator in llcc_update_act_ctrl()
- Add comment before initialization of Broadcast_AND regmap in probe
- Move DeviceTree patches to the end

Changes in v2:
- Added an additional check in the case old DT files are used for above mentioned chipsets for backwards compatibility
- Moved addition of if check in llcc_update_act_ctrl() to a separate "Fixes" patch; not part of this series

Link to v5: https://lore.kernel.org/all/[email protected]/
Link to v4: https://lore.kernel.org/all/[email protected]/
Link to v3: https://lore.kernel.org/all/[email protected]/
Link to v2: https://lore.kernel.org/all/[email protected]/
Link to v1: https://lore.kernel.org/all/[email protected]/

Unnathi Chalicheemala (5):
dt-bindings: arm: msm: Add llcc Broadcast_AND register
soc: qcom: llcc: Add regmap for Broadcast_AND region
arm64: dts: qcom: sm8450: Add Broadcast_AND register in LLCC block
arm64: dts: qcom: sm8550: Add Broadcast_AND register in LLCC block
arm64: dts: qcom: sm8650: Add Broadcast_AND register in LLCC block

 .../devicetree/bindings/cache/qcom,llcc.yaml  | 27 ++++++++++++++++++-
 arch/arm64/boot/dts/qcom/sm8450.dtsi          |  5 ++--
 arch/arm64/boot/dts/qcom/sm8550.dtsi          |  6 +++--
 arch/arm64/boot/dts/qcom/sm8650.dtsi          |  6 +++--
 drivers/soc/qcom/llcc-qcom.c                  | 16 ++++++++++-
 include/linux/soc/qcom/llcc-qcom.h            |  4 ++-
 6 files changed, 55 insertions(+), 9 deletions(-)
Signature · 1 line
2.34.1
#1 lore raw

PATCH v6 1/5 dt-bindings: arm: msm: Add llcc Broadcast_AND register

The LLCC block in SM8450, SM8550 and SM8650 have a new register space for Broadcast_AND region. This is used to check that all channels have bit set to "1", mainly in SCID activation/deactivation.

Previously we were mapping only the Broadcast_OR region assuming there was only one broadcast register region. Now we also map Broadcast_AND region.

---
 .../devicetree/bindings/cache/qcom,llcc.yaml  | 27 ++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/cache/qcom,llcc.yaml +26 -1
index 07ccbda4a0ab..a6237028957f 100644
@@ -141,8 +141,31 @@ allOf:
               - qcom,sm8150-llcc
               - qcom,sm8250-llcc
               - qcom,sm8350-llcc
+    then:
+      properties:
+        reg:
+          items:
+            - description: LLCC0 base register region
+            - description: LLCC1 base register region
+            - description: LLCC2 base register region
+            - description: LLCC3 base register region
+            - description: LLCC broadcast base register region
+        reg-names:
+          items:
+            - const: llcc0_base
+            - const: llcc1_base
+            - const: llcc2_base
+            - const: llcc3_base
+            - const: llcc_broadcast_base
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
               - qcom,sm8450-llcc
               - qcom,sm8550-llcc
+              - qcom,sm8650-llcc
     then:
       properties:
         reg:
@@ -151,7 +174,8 @@ allOf:
             - description: LLCC1 base register region
             - description: LLCC2 base register region
             - description: LLCC3 base register region
-            - description: LLCC broadcast base register region
+            - description: LLCC broadcast OR register region
+            - description: LLCC broadcast AND register region
         reg-names:
           items:
             - const: llcc0_base
@@ -159,6 +183,7 @@ allOf:
             - const: llcc2_base
             - const: llcc3_base
             - const: llcc_broadcast_base
+            - const: llcc_broadcast_and_base
 
 additionalProperties: false
 
Signature · 1 line
2.34.1
#2 lore raw

PATCH v6 2/5 soc: qcom: llcc: Add regmap for Broadcast_AND region

In reply to Unnathi Chalicheemala

Until SM8450, there was only one broadcast region (Broadcast_OR) used to broadcast write and check for status bit 0.
From SM8450 onwards another broadcast region (Broadcast_AND) has been added which checks for status bit 1. This hasn't been updated and Broadcast_OR region was wrongly being used to check for status bit 1 all along.

Hence define new regmap structure for Broadcast_AND region and initialize this regmap when HW block version is greater than 4.1, otherwise initialize as a NULL pointer for backwards compatibility. Switch from broadcast_OR to broadcast_AND region (when defined in DT) for checking status bit 1 as Broadcast_OR region checks only for bit 0.

---
 drivers/soc/qcom/llcc-qcom.c       | 16 +++++++++++++++-
 include/linux/soc/qcom/llcc-qcom.h |  4 +++-
 2 files changed, 18 insertions(+), 2 deletions(-)
drivers/soc/qcom/llcc-qcom.c +15 -1
index cbef0dea1d5d..668e0cb6a925 100644
@@ -821,6 +821,7 @@ EXPORT_SYMBOL_GPL(llcc_slice_putd);
 static int llcc_update_act_ctrl(u32 sid,
 				u32 act_ctrl_reg_val, u32 status)
 {
+	struct regmap *regmap;
 	u32 act_ctrl_reg;
 	u32 act_clear_reg;
 	u32 status_reg;
@@ -849,7 +850,8 @@ static int llcc_update_act_ctrl(u32 sid,
 		return ret;
 
 	if (drv_data->version >= LLCC_VERSION_4_1_0_0) {
-		ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg,
+		regmap = drv_data->bcast_and_regmap ?: drv_data->bcast_regmap;
+		ret = regmap_read_poll_timeout(regmap, status_reg,
 				      slice_status, (slice_status & ACT_COMPLETE),
 				      0, LLCC_STATUS_READ_DELAY);
 		if (ret)
@@ -1284,6 +1286,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
 
 	drv_data->version = version;
 
+	/* Applicable only when drv_data->version >= 4.1 */
+	if (drv_data->version >= LLCC_VERSION_4_1_0_0) {
+		drv_data->bcast_and_regmap = qcom_llcc_init_mmio(pdev, i + 1, "llcc_broadcast_and_base");
+		if (IS_ERR(drv_data->bcast_and_regmap)) {
+			ret = PTR_ERR(drv_data->bcast_and_regmap);
+			if (ret == -EINVAL)
+				drv_data->bcast_and_regmap = NULL;
+			else
+				goto err;
+		}
+	}
+
 	llcc_cfg = cfg->sct_data;
 	sz = cfg->size;
 
include/linux/soc/qcom/llcc-qcom.h +3 -1
index 1a886666bbb6..9e9f528b1370 100644
@@ -115,7 +115,8 @@ struct llcc_edac_reg_offset {
 /**
  * struct llcc_drv_data - Data associated with the llcc driver
  * @regmaps: regmaps associated with the llcc device
- * @bcast_regmap: regmap associated with llcc broadcast offset
+ * @bcast_regmap: regmap associated with llcc broadcast OR offset
+ * @bcast_and_regmap: regmap associated with llcc broadcast AND offset
  * @cfg: pointer to the data structure for slice configuration
  * @edac_reg_offset: Offset of the LLCC EDAC registers
  * @lock: mutex associated with each slice
@@ -129,6 +130,7 @@ struct llcc_edac_reg_offset {
 struct llcc_drv_data {
 	struct regmap **regmaps;
 	struct regmap *bcast_regmap;
+	struct regmap *bcast_and_regmap;
 	const struct llcc_slice_config *cfg;
 	const struct llcc_edac_reg_offset *edac_reg_offset;
 	struct mutex lock;
Signature · 1 line
2.34.1
#3 lore raw

Re: [PATCH v6 2/5] soc: qcom: llcc: Add regmap for Broadcast_AND region

Quoted 33 lines · Unnathi Chalicheemala

Until SM8450, there was only one broadcast region (Broadcast_OR) used to broadcast write and check for status bit 0.

Quoted 1 line

From SM8450 onwards another broadcast region (Broadcast_AND) has been

added which checks for status bit 1. This hasn't been updated and Broadcast_OR region was wrongly being used to check for status bit 1 all along.

Hence define new regmap structure for Broadcast_AND region and initialize this regmap when HW block version is greater than 4.1, otherwise initialize as a NULL pointer for backwards compatibility. Switch from broadcast_OR to broadcast_AND region (when defined in DT) for checking status bit 1 as Broadcast_OR region checks only for bit 0.

---
 drivers/soc/qcom/llcc-qcom.c       | 16 +++++++++++++++-
 include/linux/soc/qcom/llcc-qcom.h |  4 +++-
 2 files changed, 18 insertions(+), 2 deletions(-)
drivers/soc/qcom/llcc-qcom.c +1 -0
index cbef0dea1d5d..668e0cb6a925 100644
@@ -821,6 +821,7 @@ EXPORT_SYMBOL_GPL(llcc_slice_putd);
 static int llcc_update_act_ctrl(u32 sid,
 				u32 act_ctrl_reg_val, u32 status)
 {
+	struct regmap *regmap;
 	u32 act_ctrl_reg;
 	u32 act_clear_reg;
 	u32 status_reg;

drivers/soc/qcom/llcc-qcom.c static int llcc_update_act_ctrl(u32 sid, · line 852

Quoted 3 lines
drivers/soc/qcom/llcc-qcom.c +0 -0
@@ -849,7 +850,8 @@ static int llcc_update_act_ctrl(u32 sid,
 		return ret;
 
 	if (drv_data->version >= LLCC_VERSION_4_1_0_0) {

This makes me wonder, why is the broadcast_AND region being used only since LLCC 4.1, if it is existed since SM8450 (and SM8450 predates 4.1).

Quoted 50 lines
drivers/soc/qcom/llcc-qcom.c +14 -1
-		ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg,
+		regmap = drv_data->bcast_and_regmap ?: drv_data->bcast_regmap;
+		ret = regmap_read_poll_timeout(regmap, status_reg,
 				      slice_status, (slice_status & ACT_COMPLETE),
 				      0, LLCC_STATUS_READ_DELAY);
 		if (ret)
@@ -1284,6 +1286,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
 
 	drv_data->version = version;
 
+	/* Applicable only when drv_data->version >= 4.1 */
+	if (drv_data->version >= LLCC_VERSION_4_1_0_0) {
+		drv_data->bcast_and_regmap = qcom_llcc_init_mmio(pdev, i + 1, "llcc_broadcast_and_base");
+		if (IS_ERR(drv_data->bcast_and_regmap)) {
+			ret = PTR_ERR(drv_data->bcast_and_regmap);
+			if (ret == -EINVAL)
+				drv_data->bcast_and_regmap = NULL;
+			else
+				goto err;
+		}
+	}
+
 	llcc_cfg = cfg->sct_data;
 	sz = cfg->size;
 
include/linux/soc/qcom/llcc-qcom.h +3 -1
index 1a886666bbb6..9e9f528b1370 100644
@@ -115,7 +115,8 @@ struct llcc_edac_reg_offset {
 /**
  * struct llcc_drv_data - Data associated with the llcc driver
  * @regmaps: regmaps associated with the llcc device
- * @bcast_regmap: regmap associated with llcc broadcast offset
+ * @bcast_regmap: regmap associated with llcc broadcast OR offset
+ * @bcast_and_regmap: regmap associated with llcc broadcast AND offset
  * @cfg: pointer to the data structure for slice configuration
  * @edac_reg_offset: Offset of the LLCC EDAC registers
  * @lock: mutex associated with each slice
@@ -129,6 +130,7 @@ struct llcc_edac_reg_offset {
 struct llcc_drv_data {
 	struct regmap **regmaps;
 	struct regmap *bcast_regmap;
+	struct regmap *bcast_and_regmap;
 	const struct llcc_slice_config *cfg;
 	const struct llcc_edac_reg_offset *edac_reg_offset;
 	struct mutex lock;
Signature · 1 line
2.34.1
Signature · 2 lines
With best wishes
Dmitry
#4 lore raw

PATCH v6 3/5 arm64: dts: qcom: sm8450: Add Broadcast_AND register in LLCC block

In reply to Unnathi Chalicheemala

Chipsets before SM8450 have only one broadcast register (Broadcast_OR) which is used to broadcast writes and check for status bit 0 only in all channels.
From SM8450 onwards, a new Broadcast_AND region was added which checks for status bit 1. This hasn't been updated and Broadcast_OR region was wrongly being used to check for status bit 1 all along. Hence mapping Broadcast_AND region's address space to LLCC in SM8450.

---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/qcom/sm8450.dtsi +3 -2
index 616461fcbab9..f947cacc81b3 100644
@@ -4362,9 +4362,10 @@ system-cache-controller@19200000 {
 			compatible = "qcom,sm8450-llcc";
 			reg = <0 0x19200000 0 0x80000>, <0 0x19600000 0 0x80000>,
 			      <0 0x19300000 0 0x80000>, <0 0x19700000 0 0x80000>,
-			      <0 0x19a00000 0 0x80000>;
+			      <0 0x19a00000 0 0x80000>, <0 0x19c00000 0 0x80000>;
 			reg-names = "llcc0_base", "llcc1_base", "llcc2_base",
-				    "llcc3_base", "llcc_broadcast_base";
+				    "llcc3_base", "llcc_broadcast_base",
+				    "llcc_broadcast_and_base";
 			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
Signature · 1 line
2.34.1
#5 lore raw

PATCH v6 4/5 arm64: dts: qcom: sm8550: Add Broadcast_AND register in LLCC block

In reply to Unnathi Chalicheemala

Chipsets before SM8450 have only one broadcast register (Broadcast_OR) which is used to broadcast writes and check for status bit 0 only in all channels.
From SM8450 onwards, a new Broadcast_AND region was added which checks for status bit 1. This hasn't been updated and Broadcast_OR region was wrongly being used to check for status bit 1 all along. Hence mapping Broadcast_AND region's address space to LLCC in SM8550.

---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/qcom/sm8550.dtsi +4 -2
index bc5aeb05ffc3..d0ba6cf6c27e 100644
@@ -4295,12 +4295,14 @@ system-cache-controller@25000000 {
 			      <0 0x25200000 0 0x200000>,
 			      <0 0x25400000 0 0x200000>,
 			      <0 0x25600000 0 0x200000>,
-			      <0 0x25800000 0 0x200000>;
+			      <0 0x25800000 0 0x200000>,
+			      <0 0x25a00000 0 0x200000>;
 			reg-names = "llcc0_base",
 				    "llcc1_base",
 				    "llcc2_base",
 				    "llcc3_base",
-				    "llcc_broadcast_base";
+				    "llcc_broadcast_base",
+				    "llcc_broadcast_and_base";
 			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
Signature · 1 line
2.34.1
#6 lore raw

PATCH v6 5/5 arm64: dts: qcom: sm8650: Add Broadcast_AND register in LLCC block

In reply to Unnathi Chalicheemala

Chipsets before SM8450 have only one broadcast register (Broadcast_OR) which is used to broadcast writes and check for status bit 0 only in all channels.
From SM8450 onwards, a new Broadcast_AND region was added which checks for status bit 1. This hasn't been updated and Broadcast_OR region was wrongly being used to check for status bit 1 all along. Hence mapping Broadcast_AND region's address space to LLCC in SM8650.

---
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/qcom/sm8650.dtsi +4 -2
index 62a6e77730bc..0d71aa24a7ba 100644
@@ -4982,12 +4982,14 @@ system-cache-controller@25000000 {
 			      <0 0x25400000 0 0x200000>,
 			      <0 0x25200000 0 0x200000>,
 			      <0 0x25600000 0 0x200000>,
-			      <0 0x25800000 0 0x200000>;
+			      <0 0x25800000 0 0x200000>,
+			      <0 0x25a00000 0 0x200000>;
 			reg-names = "llcc0_base",
 				    "llcc1_base",
 				    "llcc2_base",
 				    "llcc3_base",
-				    "llcc_broadcast_base";
+				    "llcc_broadcast_base",
+				    "llcc_broadcast_and_base";
 
 			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
 		};
Signature · 1 line
2.34.1
maintainer Applied
#7 lore raw

Re: (subset) [PATCH v6 0/5] LLCC: Support for Broadcast_AND region

In reply to Unnathi Chalicheemala
Quoted 5 lines · Unnathi Chalicheemala

This series adds:
1. Device tree register mapping for Broadcast_AND region in SM8450, SM8550, SM8650.
2. LLCC driver updates to reflect addition of Broadcast_AND regmap.

Quoted 7 lines

To support CSR programming, a broadcast interface is used to program all channels in a single command. Until SM8450 there was only one broadcast region (Broadcast_OR) used to broadcast write and check for status bit
0. From SM8450 onwards another broadcast region (Broadcast_AND) has been added which checks for status bit 1.

[...]

Applied, thanks!

[3/5] arm64: dts: qcom: sm8450: Add Broadcast_AND register in LLCC block

      commit: c566143137aaacfed1af09d8710edab1971c312d
[4/5] arm64: dts: qcom: sm8550: Add Broadcast_AND register in LLCC block
      commit: 2a71a2eb1f5ec438f0ac1c7e294cd7ed32119af3
[5/5] arm64: dts: qcom: sm8650: Add Broadcast_AND register in LLCC block
      commit: a7823576f7f7b1cb0a595332ab6b0b38e15f45a7

Best regards,

Signature · 1 line
Bjorn Andersson <[email protected]>
#8 lore raw

Re: (subset) [PATCH v6 0/5] LLCC: Support for Broadcast_AND region

Quoted 13 lines · Bjorn Andersson
Quoted 11 lines · Unnathi Chalicheemala

This series adds:
1. Device tree register mapping for Broadcast_AND region in SM8450, SM8550, SM8650.
2. LLCC driver updates to reflect addition of Broadcast_AND regmap.

To support CSR programming, a broadcast interface is used to program all channels in a single command. Until SM8450 there was only one broadcast region (Broadcast_OR) used to broadcast write and check for status bit
0. From SM8450 onwards another broadcast region (Broadcast_AND) has been added which checks for status bit 1.

Quoted 4 lines
Quoted 1 line

[...]

Applied, thanks!

Thanks Bjorn, Krzystof and Konrad for the reviews!

Quoted 8 lines

[3/5] arm64: dts: qcom: sm8450: Add Broadcast_AND register in LLCC block

      commit: c566143137aaacfed1af09d8710edab1971c312d
[4/5] arm64: dts: qcom: sm8550: Add Broadcast_AND register in LLCC block
      commit: 2a71a2eb1f5ec438f0ac1c7e294cd7ed32119af3
[5/5] arm64: dts: qcom: sm8650: Add Broadcast_AND register in LLCC block
      commit: a7823576f7f7b1cb0a595332ab6b0b38e15f45a7

Best regards,