devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/5] LLCC: Support for Broadcast_AND region
@ 2024-05-20 21:00 Unnathi Chalicheemala
  2024-05-20 21:00 ` [PATCH v5 1/5] dt-bindings: arm: msm: Add llcc Broadcast_AND register Unnathi Chalicheemala
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Unnathi Chalicheemala @ 2024-05-20 21:00 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Unnathi Chalicheemala, linux-arm-msm, devicetree, linux-kernel,
	kernel

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 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 v4: https://lore.kernel.org/all/20240329-llcc-broadcast-and-v4-0-107c76fd8ceb@quicinc.com/
Link to v3: https://lore.kernel.org/all/cover.1708551850.git.quic_uchalich@quicinc.com/
Link to v2: https://lore.kernel.org/all/cover.1707202761.git.quic_uchalich@quicinc.com/
Link to v1: https://lore.kernel.org/all/cover.1706296015.git.quic_uchalich@quicinc.com/

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(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v5 1/5] dt-bindings: arm: msm: Add llcc Broadcast_AND register
  2024-05-20 21:00 [PATCH v5 0/5] LLCC: Support for Broadcast_AND region Unnathi Chalicheemala
@ 2024-05-20 21:00 ` Unnathi Chalicheemala
  2024-05-20 21:00 ` [PATCH v5 2/5] soc: qcom: llcc: Add regmap for Broadcast_AND region Unnathi Chalicheemala
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Unnathi Chalicheemala @ 2024-05-20 21:00 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Unnathi Chalicheemala, linux-arm-msm, devicetree, linux-kernel,
	kernel, Krzysztof Kozlowski

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.

Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/cache/qcom,llcc.yaml  | 27 ++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/cache/qcom,llcc.yaml b/Documentation/devicetree/bindings/cache/qcom,llcc.yaml
index 07ccbda4a0ab..a6237028957f 100644
--- a/Documentation/devicetree/bindings/cache/qcom,llcc.yaml
+++ b/Documentation/devicetree/bindings/cache/qcom,llcc.yaml
@@ -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
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v5 2/5] soc: qcom: llcc: Add regmap for Broadcast_AND region
  2024-05-20 21:00 [PATCH v5 0/5] LLCC: Support for Broadcast_AND region Unnathi Chalicheemala
  2024-05-20 21:00 ` [PATCH v5 1/5] dt-bindings: arm: msm: Add llcc Broadcast_AND register Unnathi Chalicheemala
@ 2024-05-20 21:00 ` Unnathi Chalicheemala
  2024-05-27  3:11   ` Bjorn Andersson
  2024-05-20 21:00 ` [PATCH v5 3/5] arm64: dts: qcom: sm8450: Add Broadcast_AND register in LLCC block Unnathi Chalicheemala
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Unnathi Chalicheemala @ 2024-05-20 21:00 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Unnathi Chalicheemala, linux-arm-msm, devicetree, linux-kernel,
	kernel, Bjorn Andersson

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.

Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/soc/qcom/llcc-qcom.c       | 16 +++++++++++++++-
 include/linux/soc/qcom/llcc-qcom.h |  4 +++-
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index cbef0dea1d5d..5eac6aa567e7 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -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;
 
diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
index 1a886666bbb6..9e9f528b1370 100644
--- a/include/linux/soc/qcom/llcc-qcom.h
+++ b/include/linux/soc/qcom/llcc-qcom.h
@@ -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;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v5 3/5] arm64: dts: qcom: sm8450: Add Broadcast_AND register in LLCC block
  2024-05-20 21:00 [PATCH v5 0/5] LLCC: Support for Broadcast_AND region Unnathi Chalicheemala
  2024-05-20 21:00 ` [PATCH v5 1/5] dt-bindings: arm: msm: Add llcc Broadcast_AND register Unnathi Chalicheemala
  2024-05-20 21:00 ` [PATCH v5 2/5] soc: qcom: llcc: Add regmap for Broadcast_AND region Unnathi Chalicheemala
@ 2024-05-20 21:00 ` Unnathi Chalicheemala
  2024-05-20 21:00 ` [PATCH v5 4/5] arm64: dts: qcom: sm8550: " Unnathi Chalicheemala
  2024-05-20 21:00 ` [PATCH v5 5/5] arm64: dts: qcom: sm8650: " Unnathi Chalicheemala
  4 siblings, 0 replies; 8+ messages in thread
From: Unnathi Chalicheemala @ 2024-05-20 21:00 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Unnathi Chalicheemala, linux-arm-msm, devicetree, linux-kernel,
	kernel

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.

Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index b86be34a912b..72587b9c7bba 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -4347,9 +4347,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>;
 		};
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v5 4/5] arm64: dts: qcom: sm8550: Add Broadcast_AND register in LLCC block
  2024-05-20 21:00 [PATCH v5 0/5] LLCC: Support for Broadcast_AND region Unnathi Chalicheemala
                   ` (2 preceding siblings ...)
  2024-05-20 21:00 ` [PATCH v5 3/5] arm64: dts: qcom: sm8450: Add Broadcast_AND register in LLCC block Unnathi Chalicheemala
@ 2024-05-20 21:00 ` Unnathi Chalicheemala
  2024-05-20 21:00 ` [PATCH v5 5/5] arm64: dts: qcom: sm8650: " Unnathi Chalicheemala
  4 siblings, 0 replies; 8+ messages in thread
From: Unnathi Chalicheemala @ 2024-05-20 21:00 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Unnathi Chalicheemala, linux-arm-msm, devicetree, linux-kernel,
	kernel

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.

Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 3904348075f6..ee387e6f9832 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -4263,12 +4263,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>;
 		};
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v5 5/5] arm64: dts: qcom: sm8650: Add Broadcast_AND register in LLCC block
  2024-05-20 21:00 [PATCH v5 0/5] LLCC: Support for Broadcast_AND region Unnathi Chalicheemala
                   ` (3 preceding siblings ...)
  2024-05-20 21:00 ` [PATCH v5 4/5] arm64: dts: qcom: sm8550: " Unnathi Chalicheemala
@ 2024-05-20 21:00 ` Unnathi Chalicheemala
  4 siblings, 0 replies; 8+ messages in thread
From: Unnathi Chalicheemala @ 2024-05-20 21:00 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Unnathi Chalicheemala, linux-arm-msm, devicetree, linux-kernel,
	kernel

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.

Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index ba72d8f38420..8db052810357 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -4781,12 +4781,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>;
 		};
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 2/5] soc: qcom: llcc: Add regmap for Broadcast_AND region
  2024-05-20 21:00 ` [PATCH v5 2/5] soc: qcom: llcc: Add regmap for Broadcast_AND region Unnathi Chalicheemala
@ 2024-05-27  3:11   ` Bjorn Andersson
  2024-05-29 17:09     ` Unnathi Chalicheemala
  0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Andersson @ 2024-05-27  3:11 UTC (permalink / raw)
  To: Unnathi Chalicheemala
  Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, devicetree, linux-kernel, kernel, Bjorn Andersson

On Mon, May 20, 2024 at 02:00:14PM GMT, Unnathi Chalicheemala wrote:
> 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.
> 

This is a good technical description of the change you're making. But
it's been long enough since we discussed this that I've forgotten which
problem it solves, and the commit message doesn't tell me.

Please add a paragraph on the top describing the actual problem this
solves?

Regards,
Bjorn

> Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/soc/qcom/llcc-qcom.c       | 16 +++++++++++++++-
>  include/linux/soc/qcom/llcc-qcom.h |  4 +++-
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index cbef0dea1d5d..5eac6aa567e7 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -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;
>  
> diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
> index 1a886666bbb6..9e9f528b1370 100644
> --- a/include/linux/soc/qcom/llcc-qcom.h
> +++ b/include/linux/soc/qcom/llcc-qcom.h
> @@ -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;
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 2/5] soc: qcom: llcc: Add regmap for Broadcast_AND region
  2024-05-27  3:11   ` Bjorn Andersson
@ 2024-05-29 17:09     ` Unnathi Chalicheemala
  0 siblings, 0 replies; 8+ messages in thread
From: Unnathi Chalicheemala @ 2024-05-29 17:09 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, devicetree, linux-kernel, kernel, Bjorn Andersson

On 5/26/2024 8:11 PM, Bjorn Andersson wrote:
> On Mon, May 20, 2024 at 02:00:14PM GMT, Unnathi Chalicheemala wrote:
>> 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.
>>
> 
> This is a good technical description of the change you're making. But
> it's been long enough since we discussed this that I've forgotten which
> problem it solves, and the commit message doesn't tell me.
> 
> Please add a paragraph on the top describing the actual problem this
> solves?
>
Yes understood, I'll append this to the commit message:

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 hasn't been updated and
Broadcast_OR region was wrongly being used to check for status bit 1 all
along.

Thank you Bjorn.
> Regards,
> Bjorn
> 
>> Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
>> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>
>> ---
>>  drivers/soc/qcom/llcc-qcom.c       | 16 +++++++++++++++-
>>  include/linux/soc/qcom/llcc-qcom.h |  4 +++-
>>  2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
>> index cbef0dea1d5d..5eac6aa567e7 100644
>> --- a/drivers/soc/qcom/llcc-qcom.c
>> +++ b/drivers/soc/qcom/llcc-qcom.c
>> @@ -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;
>>  
>> diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
>> index 1a886666bbb6..9e9f528b1370 100644
>> --- a/include/linux/soc/qcom/llcc-qcom.h
>> +++ b/include/linux/soc/qcom/llcc-qcom.h
>> @@ -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;
>> -- 
>> 2.34.1
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-05-29 17:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-20 21:00 [PATCH v5 0/5] LLCC: Support for Broadcast_AND region Unnathi Chalicheemala
2024-05-20 21:00 ` [PATCH v5 1/5] dt-bindings: arm: msm: Add llcc Broadcast_AND register Unnathi Chalicheemala
2024-05-20 21:00 ` [PATCH v5 2/5] soc: qcom: llcc: Add regmap for Broadcast_AND region Unnathi Chalicheemala
2024-05-27  3:11   ` Bjorn Andersson
2024-05-29 17:09     ` Unnathi Chalicheemala
2024-05-20 21:00 ` [PATCH v5 3/5] arm64: dts: qcom: sm8450: Add Broadcast_AND register in LLCC block Unnathi Chalicheemala
2024-05-20 21:00 ` [PATCH v5 4/5] arm64: dts: qcom: sm8550: " Unnathi Chalicheemala
2024-05-20 21:00 ` [PATCH v5 5/5] arm64: dts: qcom: sm8650: " Unnathi Chalicheemala

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).