[PATCH 0/6] Add MSM8226 OCMEM support plus some extra OCMEM driver fixes

[PATCH 0/6] Add MSM8226 OCMEM support plus some extra OCMEM driver fixes

[Luca Weiss]

Like MSM8974 the MSM8226 SoC also contains some OCMEM but it has just
one region for graphics compared to 8974.

While adding support I found a bug in the existing driver that is being
fixed in this series also and the rest of the matches are mostly
preparations for MSM8226 support.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
Luca Weiss (6):
      soc: qcom: ocmem: Fix NUM_PORTS & NUM_MACROS macros
      soc: qcom: ocmem: Use dev_err_probe where appropriate
      soc: qcom: ocmem: make iface clock optional
      dt-bindings: sram: qcom,ocmem: Add msm8226 support
      soc: qcom: ocmem: Add support for msm8226
      ARM: dts: qcom: msm8226: Add ocmem

 .../devicetree/bindings/sram/qcom,ocmem.yaml       |  6 +-
 arch/arm/boot/dts/qcom-msm8226.dtsi                | 17 ++++++
 drivers/soc/qcom/ocmem.c                           | 67 ++++++++++++----------
 3 files changed, 58 insertions(+), 32 deletions(-)
---
base-commit: 2e210278b67c67e76aeefc1a16d18a692d15c847
change-id: 20230506-msm8226-ocmem-bee17571e8eb

Best regards,
-- 
Luca Weiss <luca@z3ntu.xyz>

[PATCH 1/6] soc: qcom: ocmem: Fix NUM_PORTS & NUM_MACROS macros

[Luca Weiss]

Since we're using these two macros to read a value from a register, we
need to use the FIELD_GET instead of the FIELD_PREP macro, otherwise
we're getting wrong values.

So instead of:

  [    3.111779] ocmem fdd00000.sram: 2 ports, 1 regions, 512 macros, not interleaved

we now get the correct value of:

  [    3.129672] ocmem fdd00000.sram: 2 ports, 1 regions, 2 macros, not interleaved

Fixes: 88c1e9404f1d ("soc: qcom: add OCMEM driver")
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 drivers/soc/qcom/ocmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
index 199fe9872035..c3e78411c637 100644
--- a/drivers/soc/qcom/ocmem.c
+++ b/drivers/soc/qcom/ocmem.c
@@ -76,8 +76,8 @@ struct ocmem {
 #define OCMEM_REG_GFX_MPU_START			0x00001004
 #define OCMEM_REG_GFX_MPU_END			0x00001008
 
-#define OCMEM_HW_PROFILE_NUM_PORTS(val)		FIELD_PREP(0x0000000f, (val))
-#define OCMEM_HW_PROFILE_NUM_MACROS(val)	FIELD_PREP(0x00003f00, (val))
+#define OCMEM_HW_PROFILE_NUM_PORTS(val)		FIELD_GET(0x0000000f, (val))
+#define OCMEM_HW_PROFILE_NUM_MACROS(val)	FIELD_GET(0x00003f00, (val))
 
 #define OCMEM_HW_PROFILE_LAST_REGN_HALFSIZE	0x00010000
 #define OCMEM_HW_PROFILE_INTERLEAVING		0x00020000

-- 
2.40.1

[PATCH 2/6] soc: qcom: ocmem: Use dev_err_probe where appropriate

[Luca Weiss]

Use dev_err_probe in the driver probe function where useful, to simplify
getting PTR_ERR and to ensure the underlying errors are included in the
error message.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 drivers/soc/qcom/ocmem.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
index c3e78411c637..a11a955a1327 100644
--- a/drivers/soc/qcom/ocmem.c
+++ b/drivers/soc/qcom/ocmem.c
@@ -317,18 +317,13 @@ static int ocmem_dev_probe(struct platform_device *pdev)
 	ocmem->config = device_get_match_data(dev);
 
 	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(ocmem_clks), ocmem_clks);
-	if (ret) {
-		if (ret != -EPROBE_DEFER)
-			dev_err(dev, "Unable to get clocks\n");
-
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Unable to get clocks\n");
 
 	ocmem->mmio = devm_platform_ioremap_resource_byname(pdev, "ctrl");
-	if (IS_ERR(ocmem->mmio)) {
-		dev_err(&pdev->dev, "Failed to ioremap ocmem_ctrl resource\n");
-		return PTR_ERR(ocmem->mmio);
-	}
+	if (IS_ERR(ocmem->mmio))
+		return dev_err_probe(&pdev->dev, PTR_ERR(ocmem->mmio),
+				     "Failed to ioremap ocmem_ctrl resource\n");
 
 	ocmem->memory = platform_get_resource_byname(pdev, IORESOURCE_MEM,
 						     "mem");
@@ -341,16 +336,14 @@ static int ocmem_dev_probe(struct platform_device *pdev)
 	WARN_ON(clk_set_rate(ocmem_clks[OCMEM_CLK_CORE_IDX].clk, 1000) < 0);
 
 	ret = clk_bulk_prepare_enable(ARRAY_SIZE(ocmem_clks), ocmem_clks);
-	if (ret) {
-		dev_info(ocmem->dev, "Failed to enable clocks\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(ocmem->dev, ret, "Failed to enable clocks\n");
 
 	if (qcom_scm_restore_sec_cfg_available()) {
 		dev_dbg(dev, "configuring scm\n");
 		ret = qcom_scm_restore_sec_cfg(QCOM_SCM_OCMEM_DEV_ID, 0);
 		if (ret) {
-			dev_err(dev, "Could not enable secure configuration\n");
+			dev_err_probe(dev, ret, "Could not enable secure configuration\n");
 			goto err_clk_disable;
 		}
 	}

-- 
2.40.1

[PATCH 3/6] soc: qcom: ocmem: make iface clock optional

[Luca Weiss]

Some platforms such as msm8226 do not have an iface clk. Since clk_bulk
APIs don't offer to a way to treat some clocks as optional simply add
core_clk and iface_clk members to our drvdata.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 drivers/soc/qcom/ocmem.c | 42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
index a11a955a1327..6235065d3bc9 100644
--- a/drivers/soc/qcom/ocmem.c
+++ b/drivers/soc/qcom/ocmem.c
@@ -54,6 +54,8 @@ struct ocmem {
 	const struct ocmem_config *config;
 	struct resource *memory;
 	void __iomem *mmio;
+	struct clk *core_clk;
+	struct clk *iface_clk;
 	unsigned int num_ports;
 	unsigned int num_macros;
 	bool interleaved;
@@ -91,16 +93,6 @@ struct ocmem {
 #define OCMEM_PSGSC_CTL_MACRO2_MODE(val)	FIELD_PREP(0x00000700, (val))
 #define OCMEM_PSGSC_CTL_MACRO3_MODE(val)	FIELD_PREP(0x00007000, (val))
 
-#define OCMEM_CLK_CORE_IDX			0
-static struct clk_bulk_data ocmem_clks[] = {
-	{
-		.id = "core",
-	},
-	{
-		.id = "iface",
-	},
-};
-
 static inline void ocmem_write(struct ocmem *ocmem, u32 reg, u32 data)
 {
 	writel(data, ocmem->mmio + reg);
@@ -316,9 +308,15 @@ static int ocmem_dev_probe(struct platform_device *pdev)
 	ocmem->dev = dev;
 	ocmem->config = device_get_match_data(dev);
 
-	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(ocmem_clks), ocmem_clks);
-	if (ret)
-		return dev_err_probe(dev, ret, "Unable to get clocks\n");
+	ocmem->core_clk = devm_clk_get(dev, "core");
+	if (IS_ERR(ocmem->core_clk))
+		return dev_err_probe(dev, PTR_ERR(ocmem->core_clk),
+				     "Unable to get core clock\n");
+
+	ocmem->iface_clk = devm_clk_get_optional(dev, "iface");
+	if (IS_ERR(ocmem->iface_clk))
+		return dev_err_probe(dev, PTR_ERR(ocmem->iface_clk),
+				     "Unable to get iface clock\n");
 
 	ocmem->mmio = devm_platform_ioremap_resource_byname(pdev, "ctrl");
 	if (IS_ERR(ocmem->mmio))
@@ -333,11 +331,15 @@ static int ocmem_dev_probe(struct platform_device *pdev)
 	}
 
 	/* The core clock is synchronous with graphics */
-	WARN_ON(clk_set_rate(ocmem_clks[OCMEM_CLK_CORE_IDX].clk, 1000) < 0);
+	WARN_ON(clk_set_rate(ocmem->core_clk, 1000) < 0);
+
+	ret = clk_prepare_enable(ocmem->core_clk);
+	if (ret)
+		return dev_err_probe(ocmem->dev, ret, "Failed to enable core clock\n");
 
-	ret = clk_bulk_prepare_enable(ARRAY_SIZE(ocmem_clks), ocmem_clks);
+	ret = clk_prepare_enable(ocmem->iface_clk);
 	if (ret)
-		return dev_err_probe(ocmem->dev, ret, "Failed to enable clocks\n");
+		return dev_err_probe(ocmem->dev, ret, "Failed to enable iface clock\n");
 
 	if (qcom_scm_restore_sec_cfg_available()) {
 		dev_dbg(dev, "configuring scm\n");
@@ -396,13 +398,17 @@ static int ocmem_dev_probe(struct platform_device *pdev)
 	return 0;
 
 err_clk_disable:
-	clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
+	clk_disable_unprepare(ocmem->core_clk);
+	clk_disable_unprepare(ocmem->iface_clk);
 	return ret;
 }
 
 static int ocmem_dev_remove(struct platform_device *pdev)
 {
-	clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
+	struct ocmem *ocmem = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(ocmem->core_clk);
+	clk_disable_unprepare(ocmem->iface_clk);
 
 	return 0;
 }

-- 
2.40.1

[PATCH 4/6] dt-bindings: sram: qcom,ocmem: Add msm8226 support

[Luca Weiss]

Add the compatible for the OCMEM found on msm8226 which compared to
msm8974 only has a core clock and no iface clock.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 Documentation/devicetree/bindings/sram/qcom,ocmem.yaml | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
index 4bbf6db0b6bd..515f0d8ec641 100644
--- a/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
+++ b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
@@ -15,7 +15,9 @@ description: |
 
 properties:
   compatible:
-    const: qcom,msm8974-ocmem
+    enum:
+      - qcom,msm8226-ocmem
+      - qcom,msm8974-ocmem
 
   reg:
     items:
@@ -28,11 +30,13 @@ properties:
       - const: mem
 
   clocks:
+    minItems: 1
     items:
       - description: Core clock
       - description: Interface clock
 
   clock-names:
+    minItems: 1
     items:
       - const: core
       - const: iface

-- 
2.40.1

[PATCH 5/6] soc: qcom: ocmem: Add support for msm8226

[Luca Weiss]

The msm8226 SoC also contains OCMEM but with one region only.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 drivers/soc/qcom/ocmem.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
index 6235065d3bc9..d5892ce999c9 100644
--- a/drivers/soc/qcom/ocmem.c
+++ b/drivers/soc/qcom/ocmem.c
@@ -413,12 +413,18 @@ static int ocmem_dev_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct ocmem_config ocmem_8226_config = {
+	.num_regions = 1,
+	.macro_size = SZ_128K,
+};
+
 static const struct ocmem_config ocmem_8974_config = {
 	.num_regions = 3,
 	.macro_size = SZ_128K,
 };
 
 static const struct of_device_id ocmem_of_match[] = {
+	{ .compatible = "qcom,msm8226-ocmem", .data = &ocmem_8226_config },
 	{ .compatible = "qcom,msm8974-ocmem", .data = &ocmem_8974_config },
 	{ }
 };

-- 
2.40.1

[PATCH 6/6] ARM: dts: qcom: msm8226: Add ocmem

[Luca Weiss]

Add a node for the ocmem found on msm8226. It contains one region, used
as gmu_ram.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 arch/arm/boot/dts/qcom-msm8226.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom-msm8226.dtsi
index 42acb9ddb8cc..7ad073eb85c8 100644
--- a/arch/arm/boot/dts/qcom-msm8226.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
@@ -636,6 +636,23 @@ smd-edge {
 				label = "lpass";
 			};
 		};
+
+		sram@fdd00000 {
+			compatible = "qcom,msm8226-ocmem";
+			reg = <0xfdd00000 0x2000>,
+			      <0xfec00000 0x20000>;
+			reg-names = "ctrl", "mem";
+			ranges = <0 0xfec00000 0x20000>;
+			clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>;
+			clock-names = "core";
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			gmu_sram: gmu-sram@0 {
+				reg = <0x0 0x20000>;
+			};
+		};
 	};
 
 	timer {

-- 
2.40.1

Re: [PATCH 1/6] soc: qcom: ocmem: Fix NUM_PORTS & NUM_MACROS macros

[Konrad Dybcio]

On 7.05.2023 11:12, Luca Weiss wrote:
> Since we're using these two macros to read a value from a register, we
> need to use the FIELD_GET instead of the FIELD_PREP macro, otherwise
> we're getting wrong values.
> 
> So instead of:
> 
>   [    3.111779] ocmem fdd00000.sram: 2 ports, 1 regions, 512 macros, not interleaved
> 
> we now get the correct value of:
> 
>   [    3.129672] ocmem fdd00000.sram: 2 ports, 1 regions, 2 macros, not interleaved
> 
> Fixes: 88c1e9404f1d ("soc: qcom: add OCMEM driver")
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  drivers/soc/qcom/ocmem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
> index 199fe9872035..c3e78411c637 100644
> --- a/drivers/soc/qcom/ocmem.c
> +++ b/drivers/soc/qcom/ocmem.c
> @@ -76,8 +76,8 @@ struct ocmem {
>  #define OCMEM_REG_GFX_MPU_START			0x00001004
>  #define OCMEM_REG_GFX_MPU_END			0x00001008
>  
> -#define OCMEM_HW_PROFILE_NUM_PORTS(val)		FIELD_PREP(0x0000000f, (val))
> -#define OCMEM_HW_PROFILE_NUM_MACROS(val)	FIELD_PREP(0x00003f00, (val))
> +#define OCMEM_HW_PROFILE_NUM_PORTS(val)		FIELD_GET(0x0000000f, (val))
> +#define OCMEM_HW_PROFILE_NUM_MACROS(val)	FIELD_GET(0x00003f00, (val))
>  
>  #define OCMEM_HW_PROFILE_LAST_REGN_HALFSIZE	0x00010000
>  #define OCMEM_HW_PROFILE_INTERLEAVING		0x00020000
> 

Re: [PATCH 2/6] soc: qcom: ocmem: Use dev_err_probe where appropriate

[Konrad Dybcio]

On 7.05.2023 11:12, Luca Weiss wrote:
> Use dev_err_probe in the driver probe function where useful, to simplify
> getting PTR_ERR and to ensure the underlying errors are included in the
> error message.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  drivers/soc/qcom/ocmem.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
> index c3e78411c637..a11a955a1327 100644
> --- a/drivers/soc/qcom/ocmem.c
> +++ b/drivers/soc/qcom/ocmem.c
> @@ -317,18 +317,13 @@ static int ocmem_dev_probe(struct platform_device *pdev)
>  	ocmem->config = device_get_match_data(dev);
>  
>  	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(ocmem_clks), ocmem_clks);
> -	if (ret) {
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(dev, "Unable to get clocks\n");
> -
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Unable to get clocks\n");
>  
>  	ocmem->mmio = devm_platform_ioremap_resource_byname(pdev, "ctrl");
> -	if (IS_ERR(ocmem->mmio)) {
> -		dev_err(&pdev->dev, "Failed to ioremap ocmem_ctrl resource\n");
> -		return PTR_ERR(ocmem->mmio);
> -	}
> +	if (IS_ERR(ocmem->mmio))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(ocmem->mmio),
> +				     "Failed to ioremap ocmem_ctrl resource\n");
>  
>  	ocmem->memory = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>  						     "mem");
> @@ -341,16 +336,14 @@ static int ocmem_dev_probe(struct platform_device *pdev)
>  	WARN_ON(clk_set_rate(ocmem_clks[OCMEM_CLK_CORE_IDX].clk, 1000) < 0);
>  
>  	ret = clk_bulk_prepare_enable(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> -	if (ret) {
> -		dev_info(ocmem->dev, "Failed to enable clocks\n");
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(ocmem->dev, ret, "Failed to enable clocks\n");
>  
>  	if (qcom_scm_restore_sec_cfg_available()) {
>  		dev_dbg(dev, "configuring scm\n");
>  		ret = qcom_scm_restore_sec_cfg(QCOM_SCM_OCMEM_DEV_ID, 0);
>  		if (ret) {
> -			dev_err(dev, "Could not enable secure configuration\n");
> +			dev_err_probe(dev, ret, "Could not enable secure configuration\n");
>  			goto err_clk_disable;
>  		}
>  	}
> 

Re: [PATCH 3/6] soc: qcom: ocmem: make iface clock optional

[Konrad Dybcio]

On 7.05.2023 11:12, Luca Weiss wrote:
> Some platforms such as msm8226 do not have an iface clk. Since clk_bulk
> APIs don't offer to a way to treat some clocks as optional simply add
> core_clk and iface_clk members to our drvdata.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
I think I don't see anything wrong in here!

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  drivers/soc/qcom/ocmem.c | 42 ++++++++++++++++++++++++------------------
>  1 file changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
> index a11a955a1327..6235065d3bc9 100644
> --- a/drivers/soc/qcom/ocmem.c
> +++ b/drivers/soc/qcom/ocmem.c
> @@ -54,6 +54,8 @@ struct ocmem {
>  	const struct ocmem_config *config;
>  	struct resource *memory;
>  	void __iomem *mmio;
> +	struct clk *core_clk;
> +	struct clk *iface_clk;
>  	unsigned int num_ports;
>  	unsigned int num_macros;
>  	bool interleaved;
> @@ -91,16 +93,6 @@ struct ocmem {
>  #define OCMEM_PSGSC_CTL_MACRO2_MODE(val)	FIELD_PREP(0x00000700, (val))
>  #define OCMEM_PSGSC_CTL_MACRO3_MODE(val)	FIELD_PREP(0x00007000, (val))
>  
> -#define OCMEM_CLK_CORE_IDX			0
> -static struct clk_bulk_data ocmem_clks[] = {
> -	{
> -		.id = "core",
> -	},
> -	{
> -		.id = "iface",
> -	},
> -};
> -
>  static inline void ocmem_write(struct ocmem *ocmem, u32 reg, u32 data)
>  {
>  	writel(data, ocmem->mmio + reg);
> @@ -316,9 +308,15 @@ static int ocmem_dev_probe(struct platform_device *pdev)
>  	ocmem->dev = dev;
>  	ocmem->config = device_get_match_data(dev);
>  
> -	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(ocmem_clks), ocmem_clks);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "Unable to get clocks\n");
> +	ocmem->core_clk = devm_clk_get(dev, "core");
> +	if (IS_ERR(ocmem->core_clk))
> +		return dev_err_probe(dev, PTR_ERR(ocmem->core_clk),
> +				     "Unable to get core clock\n");
> +
> +	ocmem->iface_clk = devm_clk_get_optional(dev, "iface");
> +	if (IS_ERR(ocmem->iface_clk))
> +		return dev_err_probe(dev, PTR_ERR(ocmem->iface_clk),
> +				     "Unable to get iface clock\n");
>  
>  	ocmem->mmio = devm_platform_ioremap_resource_byname(pdev, "ctrl");
>  	if (IS_ERR(ocmem->mmio))
> @@ -333,11 +331,15 @@ static int ocmem_dev_probe(struct platform_device *pdev)
>  	}
>  
>  	/* The core clock is synchronous with graphics */
> -	WARN_ON(clk_set_rate(ocmem_clks[OCMEM_CLK_CORE_IDX].clk, 1000) < 0);
> +	WARN_ON(clk_set_rate(ocmem->core_clk, 1000) < 0);
> +
> +	ret = clk_prepare_enable(ocmem->core_clk);
> +	if (ret)
> +		return dev_err_probe(ocmem->dev, ret, "Failed to enable core clock\n");
>  
> -	ret = clk_bulk_prepare_enable(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> +	ret = clk_prepare_enable(ocmem->iface_clk);
>  	if (ret)
> -		return dev_err_probe(ocmem->dev, ret, "Failed to enable clocks\n");
> +		return dev_err_probe(ocmem->dev, ret, "Failed to enable iface clock\n");
>  
>  	if (qcom_scm_restore_sec_cfg_available()) {
>  		dev_dbg(dev, "configuring scm\n");
> @@ -396,13 +398,17 @@ static int ocmem_dev_probe(struct platform_device *pdev)
>  	return 0;
>  
>  err_clk_disable:
> -	clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> +	clk_disable_unprepare(ocmem->core_clk);
> +	clk_disable_unprepare(ocmem->iface_clk);
>  	return ret;
>  }
>  
>  static int ocmem_dev_remove(struct platform_device *pdev)
>  {
> -	clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> +	struct ocmem *ocmem = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(ocmem->core_clk);
> +	clk_disable_unprepare(ocmem->iface_clk);
>  
>  	return 0;
>  }
> 

Re: [PATCH 4/6] dt-bindings: sram: qcom,ocmem: Add msm8226 support

[Konrad Dybcio]

On 7.05.2023 11:12, Luca Weiss wrote:
> Add the compatible for the OCMEM found on msm8226 which compared to
> msm8974 only has a core clock and no iface clock.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
>  Documentation/devicetree/bindings/sram/qcom,ocmem.yaml | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> index 4bbf6db0b6bd..515f0d8ec641 100644
> --- a/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> +++ b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> @@ -15,7 +15,9 @@ description: |
>  
>  properties:
>    compatible:
> -    const: qcom,msm8974-ocmem
> +    enum:
> +      - qcom,msm8226-ocmem
> +      - qcom,msm8974-ocmem
Any chance you could read the revision field on both and add comments
like:

- qcom,msm8974-ocmem # vX.Y

>  
>    reg:
>      items:
> @@ -28,11 +30,13 @@ properties:
>        - const: mem
>  
>    clocks:
> +    minItems: 1
>      items:
>        - description: Core clock
>        - description: Interface clock
allOf: if: properties: compatible: 8974 / then: clock(s|-names): minItems: 2

Konrad
>  
>    clock-names:
> +    minItems: 1
>      items:
>        - const: core
>        - const: iface
> 

Re: [PATCH 5/6] soc: qcom: ocmem: Add support for msm8226

[Konrad Dybcio]

On 7.05.2023 11:12, Luca Weiss wrote:
> The msm8226 SoC also contains OCMEM but with one region only.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  drivers/soc/qcom/ocmem.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
> index 6235065d3bc9..d5892ce999c9 100644
> --- a/drivers/soc/qcom/ocmem.c
> +++ b/drivers/soc/qcom/ocmem.c
> @@ -413,12 +413,18 @@ static int ocmem_dev_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct ocmem_config ocmem_8226_config = {
> +	.num_regions = 1,
> +	.macro_size = SZ_128K,
> +};
> +
>  static const struct ocmem_config ocmem_8974_config = {
>  	.num_regions = 3,
>  	.macro_size = SZ_128K,
>  };
>  
>  static const struct of_device_id ocmem_of_match[] = {
> +	{ .compatible = "qcom,msm8226-ocmem", .data = &ocmem_8226_config },
>  	{ .compatible = "qcom,msm8974-ocmem", .data = &ocmem_8974_config },
>  	{ }
>  };
> 

Re: [PATCH 3/6] soc: qcom: ocmem: make iface clock optional

[Dmitry Baryshkov]

On 07/05/2023 12:12, Luca Weiss wrote:
> Some platforms such as msm8226 do not have an iface clk. Since clk_bulk
> APIs don't offer to a way to treat some clocks as optional simply add
> core_clk and iface_clk members to our drvdata.

What about using devm_clk_bulk_get_optional()? I think it would be 
simpler this way.

> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
>   drivers/soc/qcom/ocmem.c | 42 ++++++++++++++++++++++++------------------
>   1 file changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
> index a11a955a1327..6235065d3bc9 100644
> --- a/drivers/soc/qcom/ocmem.c
> +++ b/drivers/soc/qcom/ocmem.c
> @@ -54,6 +54,8 @@ struct ocmem {
>   	const struct ocmem_config *config;
>   	struct resource *memory;
>   	void __iomem *mmio;
> +	struct clk *core_clk;
> +	struct clk *iface_clk;
>   	unsigned int num_ports;
>   	unsigned int num_macros;
>   	bool interleaved;
> @@ -91,16 +93,6 @@ struct ocmem {
>   #define OCMEM_PSGSC_CTL_MACRO2_MODE(val)	FIELD_PREP(0x00000700, (val))
>   #define OCMEM_PSGSC_CTL_MACRO3_MODE(val)	FIELD_PREP(0x00007000, (val))
>   
> -#define OCMEM_CLK_CORE_IDX			0
> -static struct clk_bulk_data ocmem_clks[] = {
> -	{
> -		.id = "core",
> -	},
> -	{
> -		.id = "iface",
> -	},
> -};
> -
>   static inline void ocmem_write(struct ocmem *ocmem, u32 reg, u32 data)
>   {
>   	writel(data, ocmem->mmio + reg);
> @@ -316,9 +308,15 @@ static int ocmem_dev_probe(struct platform_device *pdev)
>   	ocmem->dev = dev;
>   	ocmem->config = device_get_match_data(dev);
>   
> -	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(ocmem_clks), ocmem_clks);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "Unable to get clocks\n");
> +	ocmem->core_clk = devm_clk_get(dev, "core");
> +	if (IS_ERR(ocmem->core_clk))
> +		return dev_err_probe(dev, PTR_ERR(ocmem->core_clk),
> +				     "Unable to get core clock\n");
> +
> +	ocmem->iface_clk = devm_clk_get_optional(dev, "iface");
> +	if (IS_ERR(ocmem->iface_clk))
> +		return dev_err_probe(dev, PTR_ERR(ocmem->iface_clk),
> +				     "Unable to get iface clock\n");
>   
>   	ocmem->mmio = devm_platform_ioremap_resource_byname(pdev, "ctrl");
>   	if (IS_ERR(ocmem->mmio))
> @@ -333,11 +331,15 @@ static int ocmem_dev_probe(struct platform_device *pdev)
>   	}
>   
>   	/* The core clock is synchronous with graphics */
> -	WARN_ON(clk_set_rate(ocmem_clks[OCMEM_CLK_CORE_IDX].clk, 1000) < 0);
> +	WARN_ON(clk_set_rate(ocmem->core_clk, 1000) < 0);
> +
> +	ret = clk_prepare_enable(ocmem->core_clk);
> +	if (ret)
> +		return dev_err_probe(ocmem->dev, ret, "Failed to enable core clock\n");
>   
> -	ret = clk_bulk_prepare_enable(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> +	ret = clk_prepare_enable(ocmem->iface_clk);
>   	if (ret)
> -		return dev_err_probe(ocmem->dev, ret, "Failed to enable clocks\n");
> +		return dev_err_probe(ocmem->dev, ret, "Failed to enable iface clock\n");
>   
>   	if (qcom_scm_restore_sec_cfg_available()) {
>   		dev_dbg(dev, "configuring scm\n");
> @@ -396,13 +398,17 @@ static int ocmem_dev_probe(struct platform_device *pdev)
>   	return 0;
>   
>   err_clk_disable:
> -	clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> +	clk_disable_unprepare(ocmem->core_clk);
> +	clk_disable_unprepare(ocmem->iface_clk);
>   	return ret;
>   }
>   
>   static int ocmem_dev_remove(struct platform_device *pdev)
>   {
> -	clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> +	struct ocmem *ocmem = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(ocmem->core_clk);
> +	clk_disable_unprepare(ocmem->iface_clk);
>   
>   	return 0;
>   }
> 

-- 
With best wishes
Dmitry

Re: [PATCH 4/6] dt-bindings: sram: qcom,ocmem: Add msm8226 support

[Luca Weiss]

On Montag, 8. Mai 2023 09:39:22 CEST Konrad Dybcio wrote:
> On 7.05.2023 11:12, Luca Weiss wrote:
> > Add the compatible for the OCMEM found on msm8226 which compared to
> > msm8974 only has a core clock and no iface clock.
> > 
> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > ---
> > 
> >  Documentation/devicetree/bindings/sram/qcom,ocmem.yaml | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> > b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml index
> > 4bbf6db0b6bd..515f0d8ec641 100644
> > --- a/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> > +++ b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> > @@ -15,7 +15,9 @@ description: |
> > 
> >  properties:
> >    compatible:
> > -    const: qcom,msm8974-ocmem
> > +    enum:
> > +      - qcom,msm8226-ocmem
> > +      - qcom,msm8974-ocmem
> 
> Any chance you could read the revision field on both and add comments
> like:
> 
> - qcom,msm8974-ocmem # vX.Y

Do you mean the OCMEM_REG_HW_VERSION register? It's currently not read in the 
driver so no idea what the value is - without adding some code.

> 
> >    reg:
> >      items:
> > @@ -28,11 +30,13 @@ properties:
> >        - const: mem
> >    
> >    clocks:
> > +    minItems: 1
> > 
> >      items:
> >        - description: Core clock
> >        - description: Interface clock
> 
> allOf: if: properties: compatible: 8974 / then: clock(s|-names): minItems: 2

Sure, can update

> 
> Konrad
> 
> >    clock-names:
> > +    minItems: 1
> > 
> >      items:
> >        - const: core
> >        - const: iface

Re: [PATCH 3/6] soc: qcom: ocmem: make iface clock optional

[Luca Weiss]

On Montag, 8. Mai 2023 13:34:23 CEST Dmitry Baryshkov wrote:
> On 07/05/2023 12:12, Luca Weiss wrote:
> > Some platforms such as msm8226 do not have an iface clk. Since clk_bulk
> > APIs don't offer to a way to treat some clocks as optional simply add
> > core_clk and iface_clk members to our drvdata.
> 
> What about using devm_clk_bulk_get_optional()? I think it would be
> simpler this way.

Using that function both clocks would be optional which may or may not be a 
bad idea. Not sure how much binding yaml and/or driver should try and catch 
bad usages of the driver.

But honestly the current usage of the bulk API seems a bit clunky, we have a 
static array of clocks that we use (not in struct ocmem for some reason) and 
then we refer to the core clock by index? Feels better to just have the two 
clock references in the device struct and then we're good.

Let me know.

Regards
Luca

> 
> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > ---
> > 
> >   drivers/soc/qcom/ocmem.c | 42 ++++++++++++++++++++++++------------------
> >   1 file changed, 24 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
> > index a11a955a1327..6235065d3bc9 100644
> > --- a/drivers/soc/qcom/ocmem.c
> > +++ b/drivers/soc/qcom/ocmem.c
> > @@ -54,6 +54,8 @@ struct ocmem {
> > 
> >   	const struct ocmem_config *config;
> >   	struct resource *memory;
> >   	void __iomem *mmio;
> > 
> > +	struct clk *core_clk;
> > +	struct clk *iface_clk;
> > 
> >   	unsigned int num_ports;
> >   	unsigned int num_macros;
> >   	bool interleaved;
> > 
> > @@ -91,16 +93,6 @@ struct ocmem {
> > 
> >   #define OCMEM_PSGSC_CTL_MACRO2_MODE(val)	FIELD_PREP(0x00000700, 
(val))
> >   #define OCMEM_PSGSC_CTL_MACRO3_MODE(val)	FIELD_PREP(0x00007000, 
(val))
> > 
> > -#define OCMEM_CLK_CORE_IDX			0
> > -static struct clk_bulk_data ocmem_clks[] = {
> > -	{
> > -		.id = "core",
> > -	},
> > -	{
> > -		.id = "iface",
> > -	},
> > -};
> > -
> > 
> >   static inline void ocmem_write(struct ocmem *ocmem, u32 reg, u32 data)
> >   {
> >   
> >   	writel(data, ocmem->mmio + reg);
> > 
> > @@ -316,9 +308,15 @@ static int ocmem_dev_probe(struct platform_device
> > *pdev)> 
> >   	ocmem->dev = dev;
> >   	ocmem->config = device_get_match_data(dev);
> > 
> > -	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > -	if (ret)
> > -		return dev_err_probe(dev, ret, "Unable to get clocks\n");
> > +	ocmem->core_clk = devm_clk_get(dev, "core");
> > +	if (IS_ERR(ocmem->core_clk))
> > +		return dev_err_probe(dev, PTR_ERR(ocmem->core_clk),
> > +				     "Unable to get core clock\n");
> > +
> > +	ocmem->iface_clk = devm_clk_get_optional(dev, "iface");
> > +	if (IS_ERR(ocmem->iface_clk))
> > +		return dev_err_probe(dev, PTR_ERR(ocmem->iface_clk),
> > +				     "Unable to get iface clock\n");
> > 
> >   	ocmem->mmio = devm_platform_ioremap_resource_byname(pdev, "ctrl");
> >   	if (IS_ERR(ocmem->mmio))
> > 
> > @@ -333,11 +331,15 @@ static int ocmem_dev_probe(struct platform_device
> > *pdev)> 
> >   	}
> >   	
> >   	/* The core clock is synchronous with graphics */
> > 
> > -	WARN_ON(clk_set_rate(ocmem_clks[OCMEM_CLK_CORE_IDX].clk, 1000) < 0);
> > +	WARN_ON(clk_set_rate(ocmem->core_clk, 1000) < 0);
> > +
> > +	ret = clk_prepare_enable(ocmem->core_clk);
> > +	if (ret)
> > +		return dev_err_probe(ocmem->dev, ret, "Failed to enable 
core clock\n");
> > 
> > -	ret = clk_bulk_prepare_enable(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > +	ret = clk_prepare_enable(ocmem->iface_clk);
> > 
> >   	if (ret)
> > 
> > -		return dev_err_probe(ocmem->dev, ret, "Failed to enable 
clocks\n");
> > +		return dev_err_probe(ocmem->dev, ret, "Failed to enable 
iface
> > clock\n");
> > 
> >   	if (qcom_scm_restore_sec_cfg_available()) {
> >   	
> >   		dev_dbg(dev, "configuring scm\n");
> > 
> > @@ -396,13 +398,17 @@ static int ocmem_dev_probe(struct platform_device
> > *pdev)> 
> >   	return 0;
> >   
> >   err_clk_disable:
> > -	clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > +	clk_disable_unprepare(ocmem->core_clk);
> > +	clk_disable_unprepare(ocmem->iface_clk);
> > 
> >   	return ret;
> >   
> >   }
> >   
> >   static int ocmem_dev_remove(struct platform_device *pdev)
> >   {
> > 
> > -	clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > +	struct ocmem *ocmem = platform_get_drvdata(pdev);
> > +
> > +	clk_disable_unprepare(ocmem->core_clk);
> > +	clk_disable_unprepare(ocmem->iface_clk);
> > 
> >   	return 0;
> >   
> >   }

Re: [PATCH 3/6] soc: qcom: ocmem: make iface clock optional

[Dmitry Baryshkov]

On Tue, 9 May 2023 at 19:47, Luca Weiss <luca@z3ntu.xyz> wrote:
>
> On Montag, 8. Mai 2023 13:34:23 CEST Dmitry Baryshkov wrote:
> > On 07/05/2023 12:12, Luca Weiss wrote:
> > > Some platforms such as msm8226 do not have an iface clk. Since clk_bulk
> > > APIs don't offer to a way to treat some clocks as optional simply add
> > > core_clk and iface_clk members to our drvdata.
> >
> > What about using devm_clk_bulk_get_optional()? I think it would be
> > simpler this way.
>
> Using that function both clocks would be optional which may or may not be a
> bad idea. Not sure how much binding yaml and/or driver should try and catch
> bad usages of the driver.

The generic rule is that we should not validate the DT unless required
(e.g. because of the possibility of legacy DT which used other
bindings or contained less information).

> But honestly the current usage of the bulk API seems a bit clunky, we have a
> static array of clocks that we use (not in struct ocmem for some reason) and
> then we refer to the core clock by index? Feels better to just have the two
> clock references in the device struct and then we're good.
>
> Let me know.
>
> Regards
> Luca
>
> >
> > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > > ---
> > >
> > >   drivers/soc/qcom/ocmem.c | 42 ++++++++++++++++++++++++------------------
> > >   1 file changed, 24 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
> > > index a11a955a1327..6235065d3bc9 100644
> > > --- a/drivers/soc/qcom/ocmem.c
> > > +++ b/drivers/soc/qcom/ocmem.c
> > > @@ -54,6 +54,8 @@ struct ocmem {
> > >
> > >     const struct ocmem_config *config;
> > >     struct resource *memory;
> > >     void __iomem *mmio;
> > >
> > > +   struct clk *core_clk;
> > > +   struct clk *iface_clk;
> > >
> > >     unsigned int num_ports;
> > >     unsigned int num_macros;
> > >     bool interleaved;
> > >
> > > @@ -91,16 +93,6 @@ struct ocmem {
> > >
> > >   #define OCMEM_PSGSC_CTL_MACRO2_MODE(val)  FIELD_PREP(0x00000700,
> (val))
> > >   #define OCMEM_PSGSC_CTL_MACRO3_MODE(val)  FIELD_PREP(0x00007000,
> (val))
> > >
> > > -#define OCMEM_CLK_CORE_IDX                 0
> > > -static struct clk_bulk_data ocmem_clks[] = {
> > > -   {
> > > -           .id = "core",
> > > -   },
> > > -   {
> > > -           .id = "iface",
> > > -   },
> > > -};
> > > -
> > >
> > >   static inline void ocmem_write(struct ocmem *ocmem, u32 reg, u32 data)
> > >   {
> > >
> > >     writel(data, ocmem->mmio + reg);
> > >
> > > @@ -316,9 +308,15 @@ static int ocmem_dev_probe(struct platform_device
> > > *pdev)>
> > >     ocmem->dev = dev;
> > >     ocmem->config = device_get_match_data(dev);
> > >
> > > -   ret = devm_clk_bulk_get(dev, ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > > -   if (ret)
> > > -           return dev_err_probe(dev, ret, "Unable to get clocks\n");
> > > +   ocmem->core_clk = devm_clk_get(dev, "core");
> > > +   if (IS_ERR(ocmem->core_clk))
> > > +           return dev_err_probe(dev, PTR_ERR(ocmem->core_clk),
> > > +                                "Unable to get core clock\n");
> > > +
> > > +   ocmem->iface_clk = devm_clk_get_optional(dev, "iface");
> > > +   if (IS_ERR(ocmem->iface_clk))
> > > +           return dev_err_probe(dev, PTR_ERR(ocmem->iface_clk),
> > > +                                "Unable to get iface clock\n");
> > >
> > >     ocmem->mmio = devm_platform_ioremap_resource_byname(pdev, "ctrl");
> > >     if (IS_ERR(ocmem->mmio))
> > >
> > > @@ -333,11 +331,15 @@ static int ocmem_dev_probe(struct platform_device
> > > *pdev)>
> > >     }
> > >
> > >     /* The core clock is synchronous with graphics */
> > >
> > > -   WARN_ON(clk_set_rate(ocmem_clks[OCMEM_CLK_CORE_IDX].clk, 1000) < 0);
> > > +   WARN_ON(clk_set_rate(ocmem->core_clk, 1000) < 0);
> > > +
> > > +   ret = clk_prepare_enable(ocmem->core_clk);
> > > +   if (ret)
> > > +           return dev_err_probe(ocmem->dev, ret, "Failed to enable
> core clock\n");
> > >
> > > -   ret = clk_bulk_prepare_enable(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > > +   ret = clk_prepare_enable(ocmem->iface_clk);
> > >
> > >     if (ret)
> > >
> > > -           return dev_err_probe(ocmem->dev, ret, "Failed to enable
> clocks\n");
> > > +           return dev_err_probe(ocmem->dev, ret, "Failed to enable
> iface
> > > clock\n");
> > >
> > >     if (qcom_scm_restore_sec_cfg_available()) {
> > >
> > >             dev_dbg(dev, "configuring scm\n");
> > >
> > > @@ -396,13 +398,17 @@ static int ocmem_dev_probe(struct platform_device
> > > *pdev)>
> > >     return 0;
> > >
> > >   err_clk_disable:
> > > -   clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > > +   clk_disable_unprepare(ocmem->core_clk);
> > > +   clk_disable_unprepare(ocmem->iface_clk);
> > >
> > >     return ret;
> > >
> > >   }
> > >
> > >   static int ocmem_dev_remove(struct platform_device *pdev)
> > >   {
> > >
> > > -   clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > > +   struct ocmem *ocmem = platform_get_drvdata(pdev);
> > > +
> > > +   clk_disable_unprepare(ocmem->core_clk);
> > > +   clk_disable_unprepare(ocmem->iface_clk);
> > >
> > >     return 0;
> > >
> > >   }
>
>
>
>


-- 
With best wishes
Dmitry

Re: [PATCH 4/6] dt-bindings: sram: qcom,ocmem: Add msm8226 support

[Konrad Dybcio]

On 9.05.2023 18:44, Luca Weiss wrote:
> On Montag, 8. Mai 2023 09:39:22 CEST Konrad Dybcio wrote:
>> On 7.05.2023 11:12, Luca Weiss wrote:
>>> Add the compatible for the OCMEM found on msm8226 which compared to
>>> msm8974 only has a core clock and no iface clock.
>>>
>>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
>>> ---
>>>
>>>  Documentation/devicetree/bindings/sram/qcom,ocmem.yaml | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
>>> b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml index
>>> 4bbf6db0b6bd..515f0d8ec641 100644
>>> --- a/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
>>> +++ b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
>>> @@ -15,7 +15,9 @@ description: |
>>>
>>>  properties:
>>>    compatible:
>>> -    const: qcom,msm8974-ocmem
>>> +    enum:
>>> +      - qcom,msm8226-ocmem
>>> +      - qcom,msm8974-ocmem
>>
>> Any chance you could read the revision field on both and add comments
>> like:
>>
>> - qcom,msm8974-ocmem # vX.Y
> 
> Do you mean the OCMEM_REG_HW_VERSION register?
Yep

It's currently not read in the 
> driver so no idea what the value is - without adding some code.
Would be appreciated!

Konrad
> 
>>
>>>    reg:
>>>      items:
>>> @@ -28,11 +30,13 @@ properties:
>>>        - const: mem
>>>    
>>>    clocks:
>>> +    minItems: 1
>>>
>>>      items:
>>>        - description: Core clock
>>>        - description: Interface clock
>>
>> allOf: if: properties: compatible: 8974 / then: clock(s|-names): minItems: 2
> 
> Sure, can update
> 
>>
>> Konrad
>>
>>>    clock-names:
>>> +    minItems: 1
>>>
>>>      items:
>>>        - const: core
>>>        - const: iface
> 
> 
> 
> 

Re: [PATCH 4/6] dt-bindings: sram: qcom,ocmem: Add msm8226 support

[Luca Weiss]

On Dienstag, 9. Mai 2023 22:00:03 CEST Konrad Dybcio wrote:
> On 9.05.2023 18:44, Luca Weiss wrote:
> > On Montag, 8. Mai 2023 09:39:22 CEST Konrad Dybcio wrote:
> >> On 7.05.2023 11:12, Luca Weiss wrote:
> >>> Add the compatible for the OCMEM found on msm8226 which compared to
> >>> msm8974 only has a core clock and no iface clock.
> >>> 
> >>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> >>> ---
> >>> 
> >>>  Documentation/devicetree/bindings/sram/qcom,ocmem.yaml | 6 +++++-
> >>>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>> 
> >>> diff --git a/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> >>> b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml index
> >>> 4bbf6db0b6bd..515f0d8ec641 100644
> >>> --- a/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> >>> +++ b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> >>> @@ -15,7 +15,9 @@ description: |
> >>> 
> >>>  properties:
> >>>    compatible:
> >>> -    const: qcom,msm8974-ocmem
> >>> +    enum:
> >>> +      - qcom,msm8226-ocmem
> >>> +      - qcom,msm8974-ocmem
> >> 
> >> Any chance you could read the revision field on both and add comments
> >> like:
> >> 
> >> - qcom,msm8974-ocmem # vX.Y
> > 
> > Do you mean the OCMEM_REG_HW_VERSION register?
> 
> Yep
> 
> It's currently not read in the
> 
> > driver so no idea what the value is - without adding some code.
> 
> Would be appreciated!

Appears msm8974 has v1.4.0 and msm8226 has v1.1.0. Will include this in the
next revision.

Regards
Luca

> 
> Konrad
> 
> >>>    reg:
> >>>      items:
> >>> @@ -28,11 +30,13 @@ properties:
> >>>        - const: mem
> >>>    
> >>>    clocks:
> >>> +    minItems: 1
> >>> 
> >>>      items:
> >>>        - description: Core clock
> >>>        - description: Interface clock
> >> 
> >> allOf: if: properties: compatible: 8974 / then: clock(s|-names):
> >> minItems: 2> 
> > Sure, can update
> > 
> >> Konrad
> >> 
> >>>    clock-names:
> >>> +    minItems: 1
> >>> 
> >>>      items:
> >>>        - const: core
> >>>        - const: iface

Re: [PATCH 3/6] soc: qcom: ocmem: make iface clock optional

[Luca Weiss]

On Dienstag, 9. Mai 2023 19:08:50 CEST Dmitry Baryshkov wrote:
> On Tue, 9 May 2023 at 19:47, Luca Weiss <luca@z3ntu.xyz> wrote:
> > On Montag, 8. Mai 2023 13:34:23 CEST Dmitry Baryshkov wrote:
> > > On 07/05/2023 12:12, Luca Weiss wrote:
> > > > Some platforms such as msm8226 do not have an iface clk. Since
> > > > clk_bulk
> > > > APIs don't offer to a way to treat some clocks as optional simply add
> > > > core_clk and iface_clk members to our drvdata.
> > > 
> > > What about using devm_clk_bulk_get_optional()? I think it would be
> > > simpler this way.
> > 
> > Using that function both clocks would be optional which may or may not be
> > a
> > bad idea. Not sure how much binding yaml and/or driver should try and
> > catch
> > bad usages of the driver.
> 
> The generic rule is that we should not validate the DT unless required
> (e.g. because of the possibility of legacy DT which used other
> bindings or contained less information).

Got it.

But since in this driver we use one of the clocks for setting clock rate I'd
keep using the two separate struct clk as I've done in this patch if you don't
mind too much.

Regards
Luca

> 
> > But honestly the current usage of the bulk API seems a bit clunky, we have
> > a static array of clocks that we use (not in struct ocmem for some
> > reason) and then we refer to the core clock by index? Feels better to
> > just have the two clock references in the device struct and then we're
> > good.
> > 
> > Let me know.
> > 
> > Regards
> > Luca
> > 
> > > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > > > ---
> > > > 
> > > >   drivers/soc/qcom/ocmem.c | 42
> > > >   ++++++++++++++++++++++++------------------
> > > >   1 file changed, 24 insertions(+), 18 deletions(-)
> > > > 
> > > > diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
> > > > index a11a955a1327..6235065d3bc9 100644
> > > > --- a/drivers/soc/qcom/ocmem.c
> > > > +++ b/drivers/soc/qcom/ocmem.c
> > > > @@ -54,6 +54,8 @@ struct ocmem {
> > > > 
> > > >     const struct ocmem_config *config;
> > > >     struct resource *memory;
> > > >     void __iomem *mmio;
> > > > 
> > > > +   struct clk *core_clk;
> > > > +   struct clk *iface_clk;
> > > > 
> > > >     unsigned int num_ports;
> > > >     unsigned int num_macros;
> > > >     bool interleaved;
> > > > 
> > > > @@ -91,16 +93,6 @@ struct ocmem {
> > > > 
> > > >   #define OCMEM_PSGSC_CTL_MACRO2_MODE(val)  FIELD_PREP(0x00000700,
> > 
> > (val))
> > 
> > > >   #define OCMEM_PSGSC_CTL_MACRO3_MODE(val)  FIELD_PREP(0x00007000,
> > 
> > (val))
> > 
> > > > -#define OCMEM_CLK_CORE_IDX                 0
> > > > -static struct clk_bulk_data ocmem_clks[] = {
> > > > -   {
> > > > -           .id = "core",
> > > > -   },
> > > > -   {
> > > > -           .id = "iface",
> > > > -   },
> > > > -};
> > > > -
> > > > 
> > > >   static inline void ocmem_write(struct ocmem *ocmem, u32 reg, u32
> > > >   data)
> > > >   {
> > > >   
> > > >     writel(data, ocmem->mmio + reg);
> > > > 
> > > > @@ -316,9 +308,15 @@ static int ocmem_dev_probe(struct platform_device
> > > > *pdev)>
> > > > 
> > > >     ocmem->dev = dev;
> > > >     ocmem->config = device_get_match_data(dev);
> > > > 
> > > > -   ret = devm_clk_bulk_get(dev, ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > > > -   if (ret)
> > > > -           return dev_err_probe(dev, ret, "Unable to get clocks\n");
> > > > +   ocmem->core_clk = devm_clk_get(dev, "core");
> > > > +   if (IS_ERR(ocmem->core_clk))
> > > > +           return dev_err_probe(dev, PTR_ERR(ocmem->core_clk),
> > > > +                                "Unable to get core clock\n");
> > > > +
> > > > +   ocmem->iface_clk = devm_clk_get_optional(dev, "iface");
> > > > +   if (IS_ERR(ocmem->iface_clk))
> > > > +           return dev_err_probe(dev, PTR_ERR(ocmem->iface_clk),
> > > > +                                "Unable to get iface clock\n");
> > > > 
> > > >     ocmem->mmio = devm_platform_ioremap_resource_byname(pdev, "ctrl");
> > > >     if (IS_ERR(ocmem->mmio))
> > > > 
> > > > @@ -333,11 +331,15 @@ static int ocmem_dev_probe(struct
> > > > platform_device
> > > > *pdev)>
> > > > 
> > > >     }
> > > >     
> > > >     /* The core clock is synchronous with graphics */
> > > > 
> > > > -   WARN_ON(clk_set_rate(ocmem_clks[OCMEM_CLK_CORE_IDX].clk, 1000) <
> > > > 0);
> > > > +   WARN_ON(clk_set_rate(ocmem->core_clk, 1000) < 0);
> > > > +
> > > > +   ret = clk_prepare_enable(ocmem->core_clk);
> > > > +   if (ret)
> > > > +           return dev_err_probe(ocmem->dev, ret, "Failed to enable
> > 
> > core clock\n");
> > 
> > > > -   ret = clk_bulk_prepare_enable(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > > > +   ret = clk_prepare_enable(ocmem->iface_clk);
> > > > 
> > > >     if (ret)
> > > > 
> > > > -           return dev_err_probe(ocmem->dev, ret, "Failed to enable
> > 
> > clocks\n");
> > 
> > > > +           return dev_err_probe(ocmem->dev, ret, "Failed to enable
> > 
> > iface
> > 
> > > > clock\n");
> > > > 
> > > >     if (qcom_scm_restore_sec_cfg_available()) {
> > > >     
> > > >             dev_dbg(dev, "configuring scm\n");
> > > > 
> > > > @@ -396,13 +398,17 @@ static int ocmem_dev_probe(struct
> > > > platform_device
> > > > *pdev)>
> > > > 
> > > >     return 0;
> > > >   
> > > >   err_clk_disable:
> > > > -   clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > > > +   clk_disable_unprepare(ocmem->core_clk);
> > > > +   clk_disable_unprepare(ocmem->iface_clk);
> > > > 
> > > >     return ret;
> > > >   
> > > >   }
> > > >   
> > > >   static int ocmem_dev_remove(struct platform_device *pdev)
> > > >   {
> > > > 
> > > > -   clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> > > > +   struct ocmem *ocmem = platform_get_drvdata(pdev);
> > > > +
> > > > +   clk_disable_unprepare(ocmem->core_clk);
> > > > +   clk_disable_unprepare(ocmem->iface_clk);
> > > > 
> > > >     return 0;
> > > >   
> > > >   }

Re: [PATCH 1/6] soc: qcom: ocmem: Fix NUM_PORTS & NUM_MACROS macros

[Caleb Connolly]

On 07/05/2023 10:12, Luca Weiss wrote:
> Since we're using these two macros to read a value from a register, we
> need to use the FIELD_GET instead of the FIELD_PREP macro, otherwise
> we're getting wrong values.
> 
> So instead of:
> 
>   [    3.111779] ocmem fdd00000.sram: 2 ports, 1 regions, 512 macros, not interleaved
> 
> we now get the correct value of:
> 
>   [    3.129672] ocmem fdd00000.sram: 2 ports, 1 regions, 2 macros, not interleaved
> 
> Fixes: 88c1e9404f1d ("soc: qcom: add OCMEM driver")
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>

Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  drivers/soc/qcom/ocmem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
> index 199fe9872035..c3e78411c637 100644
> --- a/drivers/soc/qcom/ocmem.c
> +++ b/drivers/soc/qcom/ocmem.c
> @@ -76,8 +76,8 @@ struct ocmem {
>  #define OCMEM_REG_GFX_MPU_START			0x00001004
>  #define OCMEM_REG_GFX_MPU_END			0x00001008
>  
> -#define OCMEM_HW_PROFILE_NUM_PORTS(val)		FIELD_PREP(0x0000000f, (val))
> -#define OCMEM_HW_PROFILE_NUM_MACROS(val)	FIELD_PREP(0x00003f00, (val))
> +#define OCMEM_HW_PROFILE_NUM_PORTS(val)		FIELD_GET(0x0000000f, (val))
> +#define OCMEM_HW_PROFILE_NUM_MACROS(val)	FIELD_GET(0x00003f00, (val))
>  
>  #define OCMEM_HW_PROFILE_LAST_REGN_HALFSIZE	0x00010000
>  #define OCMEM_HW_PROFILE_INTERLEAVING		0x00020000
> 

-- 
Kind Regards,
Caleb (they/them)

Re: [PATCH 2/6] soc: qcom: ocmem: Use dev_err_probe where appropriate

[Caleb Connolly]

On 07/05/2023 10:12, Luca Weiss wrote:
> Use dev_err_probe in the driver probe function where useful, to simplify
> getting PTR_ERR and to ensure the underlying errors are included in the
> error message.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>

Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  drivers/soc/qcom/ocmem.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
> index c3e78411c637..a11a955a1327 100644
> --- a/drivers/soc/qcom/ocmem.c
> +++ b/drivers/soc/qcom/ocmem.c
> @@ -317,18 +317,13 @@ static int ocmem_dev_probe(struct platform_device *pdev)
>  	ocmem->config = device_get_match_data(dev);
>  
>  	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(ocmem_clks), ocmem_clks);
> -	if (ret) {
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(dev, "Unable to get clocks\n");
> -
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Unable to get clocks\n");
>  
>  	ocmem->mmio = devm_platform_ioremap_resource_byname(pdev, "ctrl");
> -	if (IS_ERR(ocmem->mmio)) {
> -		dev_err(&pdev->dev, "Failed to ioremap ocmem_ctrl resource\n");
> -		return PTR_ERR(ocmem->mmio);
> -	}
> +	if (IS_ERR(ocmem->mmio))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(ocmem->mmio),
> +				     "Failed to ioremap ocmem_ctrl resource\n");
>  
>  	ocmem->memory = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>  						     "mem");
> @@ -341,16 +336,14 @@ static int ocmem_dev_probe(struct platform_device *pdev)
>  	WARN_ON(clk_set_rate(ocmem_clks[OCMEM_CLK_CORE_IDX].clk, 1000) < 0);
>  
>  	ret = clk_bulk_prepare_enable(ARRAY_SIZE(ocmem_clks), ocmem_clks);
> -	if (ret) {
> -		dev_info(ocmem->dev, "Failed to enable clocks\n");
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(ocmem->dev, ret, "Failed to enable clocks\n");
>  
>  	if (qcom_scm_restore_sec_cfg_available()) {
>  		dev_dbg(dev, "configuring scm\n");
>  		ret = qcom_scm_restore_sec_cfg(QCOM_SCM_OCMEM_DEV_ID, 0);
>  		if (ret) {
> -			dev_err(dev, "Could not enable secure configuration\n");
> +			dev_err_probe(dev, ret, "Could not enable secure configuration\n");
>  			goto err_clk_disable;
>  		}
>  	}
> 

-- 
Kind Regards,
Caleb (they/them)

Re: [PATCH 3/6] soc: qcom: ocmem: make iface clock optional

[Konrad Dybcio]

On 9.05.2023 23:41, Luca Weiss wrote:
> On Dienstag, 9. Mai 2023 19:08:50 CEST Dmitry Baryshkov wrote:
>> On Tue, 9 May 2023 at 19:47, Luca Weiss <luca@z3ntu.xyz> wrote:
>>> On Montag, 8. Mai 2023 13:34:23 CEST Dmitry Baryshkov wrote:
>>>> On 07/05/2023 12:12, Luca Weiss wrote:
>>>>> Some platforms such as msm8226 do not have an iface clk. Since
>>>>> clk_bulk
>>>>> APIs don't offer to a way to treat some clocks as optional simply add
>>>>> core_clk and iface_clk members to our drvdata.
>>>>
>>>> What about using devm_clk_bulk_get_optional()? I think it would be
>>>> simpler this way.
>>>
>>> Using that function both clocks would be optional which may or may not be
>>> a
>>> bad idea. Not sure how much binding yaml and/or driver should try and
>>> catch
>>> bad usages of the driver.
>>
>> The generic rule is that we should not validate the DT unless required
>> (e.g. because of the possibility of legacy DT which used other
>> bindings or contained less information).
> 
> Got it.
> 
> But since in this driver we use one of the clocks for setting clock rate I'd
> keep using the two separate struct clk as I've done in this patch if you don't
> mind too much.
I'd also advocate for 2x struct clk, using bulk here is like trying to
spread butter with a fork, it works, but has holes..

Konrad
> 
> Regards
> Luca
> 
>>
>>> But honestly the current usage of the bulk API seems a bit clunky, we have
>>> a static array of clocks that we use (not in struct ocmem for some
>>> reason) and then we refer to the core clock by index? Feels better to
>>> just have the two clock references in the device struct and then we're
>>> good.
>>>
>>> Let me know.
>>>
>>> Regards
>>> Luca
>>>
>>>>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
>>>>> ---
>>>>>
>>>>>   drivers/soc/qcom/ocmem.c | 42
>>>>>   ++++++++++++++++++++++++------------------
>>>>>   1 file changed, 24 insertions(+), 18 deletions(-)
>>>>>
>>>>> diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
>>>>> index a11a955a1327..6235065d3bc9 100644
>>>>> --- a/drivers/soc/qcom/ocmem.c
>>>>> +++ b/drivers/soc/qcom/ocmem.c
>>>>> @@ -54,6 +54,8 @@ struct ocmem {
>>>>>
>>>>>     const struct ocmem_config *config;
>>>>>     struct resource *memory;
>>>>>     void __iomem *mmio;
>>>>>
>>>>> +   struct clk *core_clk;
>>>>> +   struct clk *iface_clk;
>>>>>
>>>>>     unsigned int num_ports;
>>>>>     unsigned int num_macros;
>>>>>     bool interleaved;
>>>>>
>>>>> @@ -91,16 +93,6 @@ struct ocmem {
>>>>>
>>>>>   #define OCMEM_PSGSC_CTL_MACRO2_MODE(val)  FIELD_PREP(0x00000700,
>>>
>>> (val))
>>>
>>>>>   #define OCMEM_PSGSC_CTL_MACRO3_MODE(val)  FIELD_PREP(0x00007000,
>>>
>>> (val))
>>>
>>>>> -#define OCMEM_CLK_CORE_IDX                 0
>>>>> -static struct clk_bulk_data ocmem_clks[] = {
>>>>> -   {
>>>>> -           .id = "core",
>>>>> -   },
>>>>> -   {
>>>>> -           .id = "iface",
>>>>> -   },
>>>>> -};
>>>>> -
>>>>>
>>>>>   static inline void ocmem_write(struct ocmem *ocmem, u32 reg, u32
>>>>>   data)
>>>>>   {
>>>>>   
>>>>>     writel(data, ocmem->mmio + reg);
>>>>>
>>>>> @@ -316,9 +308,15 @@ static int ocmem_dev_probe(struct platform_device
>>>>> *pdev)>
>>>>>
>>>>>     ocmem->dev = dev;
>>>>>     ocmem->config = device_get_match_data(dev);
>>>>>
>>>>> -   ret = devm_clk_bulk_get(dev, ARRAY_SIZE(ocmem_clks), ocmem_clks);
>>>>> -   if (ret)
>>>>> -           return dev_err_probe(dev, ret, "Unable to get clocks\n");
>>>>> +   ocmem->core_clk = devm_clk_get(dev, "core");
>>>>> +   if (IS_ERR(ocmem->core_clk))
>>>>> +           return dev_err_probe(dev, PTR_ERR(ocmem->core_clk),
>>>>> +                                "Unable to get core clock\n");
>>>>> +
>>>>> +   ocmem->iface_clk = devm_clk_get_optional(dev, "iface");
>>>>> +   if (IS_ERR(ocmem->iface_clk))
>>>>> +           return dev_err_probe(dev, PTR_ERR(ocmem->iface_clk),
>>>>> +                                "Unable to get iface clock\n");
>>>>>
>>>>>     ocmem->mmio = devm_platform_ioremap_resource_byname(pdev, "ctrl");
>>>>>     if (IS_ERR(ocmem->mmio))
>>>>>
>>>>> @@ -333,11 +331,15 @@ static int ocmem_dev_probe(struct
>>>>> platform_device
>>>>> *pdev)>
>>>>>
>>>>>     }
>>>>>     
>>>>>     /* The core clock is synchronous with graphics */
>>>>>
>>>>> -   WARN_ON(clk_set_rate(ocmem_clks[OCMEM_CLK_CORE_IDX].clk, 1000) <
>>>>> 0);
>>>>> +   WARN_ON(clk_set_rate(ocmem->core_clk, 1000) < 0);
>>>>> +
>>>>> +   ret = clk_prepare_enable(ocmem->core_clk);
>>>>> +   if (ret)
>>>>> +           return dev_err_probe(ocmem->dev, ret, "Failed to enable
>>>
>>> core clock\n");
>>>
>>>>> -   ret = clk_bulk_prepare_enable(ARRAY_SIZE(ocmem_clks), ocmem_clks);
>>>>> +   ret = clk_prepare_enable(ocmem->iface_clk);
>>>>>
>>>>>     if (ret)
>>>>>
>>>>> -           return dev_err_probe(ocmem->dev, ret, "Failed to enable
>>>
>>> clocks\n");
>>>
>>>>> +           return dev_err_probe(ocmem->dev, ret, "Failed to enable
>>>
>>> iface
>>>
>>>>> clock\n");
>>>>>
>>>>>     if (qcom_scm_restore_sec_cfg_available()) {
>>>>>     
>>>>>             dev_dbg(dev, "configuring scm\n");
>>>>>
>>>>> @@ -396,13 +398,17 @@ static int ocmem_dev_probe(struct
>>>>> platform_device
>>>>> *pdev)>
>>>>>
>>>>>     return 0;
>>>>>   
>>>>>   err_clk_disable:
>>>>> -   clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
>>>>> +   clk_disable_unprepare(ocmem->core_clk);
>>>>> +   clk_disable_unprepare(ocmem->iface_clk);
>>>>>
>>>>>     return ret;
>>>>>   
>>>>>   }
>>>>>   
>>>>>   static int ocmem_dev_remove(struct platform_device *pdev)
>>>>>   {
>>>>>
>>>>> -   clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
>>>>> +   struct ocmem *ocmem = platform_get_drvdata(pdev);
>>>>> +
>>>>> +   clk_disable_unprepare(ocmem->core_clk);
>>>>> +   clk_disable_unprepare(ocmem->iface_clk);
>>>>>
>>>>>     return 0;
>>>>>   
>>>>>   }
> 
> 
> 
> 

Re: [PATCH 6/6] ARM: dts: qcom: msm8226: Add ocmem

[Konrad Dybcio]

On 7.05.2023 11:12, Luca Weiss wrote:
> Add a node for the ocmem found on msm8226. It contains one region, used
> as gmu_ram.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm/boot/dts/qcom-msm8226.dtsi | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom-msm8226.dtsi
> index 42acb9ddb8cc..7ad073eb85c8 100644
> --- a/arch/arm/boot/dts/qcom-msm8226.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
> @@ -636,6 +636,23 @@ smd-edge {
>  				label = "lpass";
>  			};
>  		};
> +
> +		sram@fdd00000 {
> +			compatible = "qcom,msm8226-ocmem";
> +			reg = <0xfdd00000 0x2000>,
> +			      <0xfec00000 0x20000>;
> +			reg-names = "ctrl", "mem";
> +			ranges = <0 0xfec00000 0x20000>;
> +			clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>;
> +			clock-names = "core";
> +
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			gmu_sram: gmu-sram@0 {
> +				reg = <0x0 0x20000>;
> +			};
> +		};
>  	};
>  
>  	timer {
>