IOMMU Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] iommu/qcom: add optional 'tbu' clock for TLB invalidate
@ 2020-05-18 14:16 Shawn Guo
  2020-05-19  1:41 ` Bjorn Andersson
  2020-05-19 12:24 ` Joerg Roedel
  0 siblings, 2 replies; 3+ messages in thread
From: Shawn Guo @ 2020-05-18 14:16 UTC (permalink / raw)
  To: Rob Clark, Joerg Roedel
  Cc: Shawn Guo, linux-arm-msm, Konrad Dybcio, Stanimir Varbanov,
	iommu, Andy Gross, Bjorn Andersson

On some SoCs like MSM8939 with A405 adreno, there is a gfx_tbu clock
needs to be on while doing TLB invalidate. Otherwise, TLBSYNC status
will not be correctly reflected, causing the system to go into a bad
state.  Add it as an optional clock, so that platforms that have this
clock can pass it over DT.

While adding the third clock, let's switch to bulk clk API to simplify
the enable/disable calls.  clk_bulk_get() cannot used because the
existing two clocks are required while the new one is optional.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
Changes for v2:
 - Use devm_clk_get_optional() to simplify code and improve readability.
 - Rename the new clock from 'tlb' to 'tbu'.
 - qcom_iommu: use bulk clk API to simplfy enable/disable.

 drivers/iommu/qcom_iommu.c | 62 ++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index 0e2a96467767..116d8188f87f 100644
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -37,14 +37,20 @@
 
 #define SMMU_INTR_SEL_NS     0x2000
 
+enum qcom_iommu_clk {
+	CLK_IFACE,
+	CLK_BUS,
+	CLK_TBU,
+	CLK_NUM,
+};
+
 struct qcom_iommu_ctx;
 
 struct qcom_iommu_dev {
 	/* IOMMU core code handle */
 	struct iommu_device	 iommu;
 	struct device		*dev;
-	struct clk		*iface_clk;
-	struct clk		*bus_clk;
+	struct clk_bulk_data clks[CLK_NUM];
 	void __iomem		*local_base;
 	u32			 sec_id;
 	u8			 num_ctxs;
@@ -626,32 +632,6 @@ static const struct iommu_ops qcom_iommu_ops = {
 	.pgsize_bitmap	= SZ_4K | SZ_64K | SZ_1M | SZ_16M,
 };
 
-static int qcom_iommu_enable_clocks(struct qcom_iommu_dev *qcom_iommu)
-{
-	int ret;
-
-	ret = clk_prepare_enable(qcom_iommu->iface_clk);
-	if (ret) {
-		dev_err(qcom_iommu->dev, "Couldn't enable iface_clk\n");
-		return ret;
-	}
-
-	ret = clk_prepare_enable(qcom_iommu->bus_clk);
-	if (ret) {
-		dev_err(qcom_iommu->dev, "Couldn't enable bus_clk\n");
-		clk_disable_unprepare(qcom_iommu->iface_clk);
-		return ret;
-	}
-
-	return 0;
-}
-
-static void qcom_iommu_disable_clocks(struct qcom_iommu_dev *qcom_iommu)
-{
-	clk_disable_unprepare(qcom_iommu->bus_clk);
-	clk_disable_unprepare(qcom_iommu->iface_clk);
-}
-
 static int qcom_iommu_sec_ptbl_init(struct device *dev)
 {
 	size_t psize = 0;
@@ -808,6 +788,7 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
 	struct qcom_iommu_dev *qcom_iommu;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
+	struct clk *clk;
 	int ret, max_asid = 0;
 
 	/* find the max asid (which is 1:1 to ctx bank idx), so we know how
@@ -827,17 +808,26 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
 	if (res)
 		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
 
-	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
-	if (IS_ERR(qcom_iommu->iface_clk)) {
+	clk = devm_clk_get(dev, "iface");
+	if (IS_ERR(clk)) {
 		dev_err(dev, "failed to get iface clock\n");
-		return PTR_ERR(qcom_iommu->iface_clk);
+		return PTR_ERR(clk);
 	}
+	qcom_iommu->clks[CLK_IFACE].clk = clk;
 
-	qcom_iommu->bus_clk = devm_clk_get(dev, "bus");
-	if (IS_ERR(qcom_iommu->bus_clk)) {
+	clk = devm_clk_get(dev, "bus");
+	if (IS_ERR(clk)) {
 		dev_err(dev, "failed to get bus clock\n");
-		return PTR_ERR(qcom_iommu->bus_clk);
+		return PTR_ERR(clk);
+	}
+	qcom_iommu->clks[CLK_BUS].clk = clk;
+
+	clk = devm_clk_get_optional(dev, "tbu");
+	if (IS_ERR(clk)) {
+		dev_err(dev, "failed to get tbu clock\n");
+		return PTR_ERR(clk);
 	}
+	qcom_iommu->clks[CLK_TBU].clk = clk;
 
 	if (of_property_read_u32(dev->of_node, "qcom,iommu-secure-id",
 				 &qcom_iommu->sec_id)) {
@@ -909,14 +899,14 @@ static int __maybe_unused qcom_iommu_resume(struct device *dev)
 {
 	struct qcom_iommu_dev *qcom_iommu = dev_get_drvdata(dev);
 
-	return qcom_iommu_enable_clocks(qcom_iommu);
+	return clk_bulk_prepare_enable(CLK_NUM, qcom_iommu->clks);
 }
 
 static int __maybe_unused qcom_iommu_suspend(struct device *dev)
 {
 	struct qcom_iommu_dev *qcom_iommu = dev_get_drvdata(dev);
 
-	qcom_iommu_disable_clocks(qcom_iommu);
+	clk_bulk_disable_unprepare(CLK_NUM, qcom_iommu->clks);
 
 	return 0;
 }
-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH v2] iommu/qcom: add optional 'tbu' clock for TLB invalidate
  2020-05-18 14:16 [PATCH v2] iommu/qcom: add optional 'tbu' clock for TLB invalidate Shawn Guo
@ 2020-05-19  1:41 ` Bjorn Andersson
  2020-05-19 12:24 ` Joerg Roedel
  1 sibling, 0 replies; 3+ messages in thread
From: Bjorn Andersson @ 2020-05-19  1:41 UTC (permalink / raw)
  To: Shawn Guo
  Cc: linux-arm-msm, Konrad Dybcio, iommu, Stanimir Varbanov, Andy Gross

On Mon 18 May 07:16 PDT 2020, Shawn Guo wrote:

> On some SoCs like MSM8939 with A405 adreno, there is a gfx_tbu clock
> needs to be on while doing TLB invalidate. Otherwise, TLBSYNC status
> will not be correctly reflected, causing the system to go into a bad
> state.  Add it as an optional clock, so that platforms that have this
> clock can pass it over DT.
> 
> While adding the third clock, let's switch to bulk clk API to simplify
> the enable/disable calls.  clk_bulk_get() cannot used because the
> existing two clocks are required while the new one is optional.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> Changes for v2:
>  - Use devm_clk_get_optional() to simplify code and improve readability.
>  - Rename the new clock from 'tlb' to 'tbu'.
>  - qcom_iommu: use bulk clk API to simplfy enable/disable.
> 
>  drivers/iommu/qcom_iommu.c | 62 ++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index 0e2a96467767..116d8188f87f 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -37,14 +37,20 @@
>  
>  #define SMMU_INTR_SEL_NS     0x2000
>  
> +enum qcom_iommu_clk {
> +	CLK_IFACE,
> +	CLK_BUS,
> +	CLK_TBU,
> +	CLK_NUM,
> +};
> +
>  struct qcom_iommu_ctx;
>  
>  struct qcom_iommu_dev {
>  	/* IOMMU core code handle */
>  	struct iommu_device	 iommu;
>  	struct device		*dev;
> -	struct clk		*iface_clk;
> -	struct clk		*bus_clk;
> +	struct clk_bulk_data clks[CLK_NUM];
>  	void __iomem		*local_base;
>  	u32			 sec_id;
>  	u8			 num_ctxs;
> @@ -626,32 +632,6 @@ static const struct iommu_ops qcom_iommu_ops = {
>  	.pgsize_bitmap	= SZ_4K | SZ_64K | SZ_1M | SZ_16M,
>  };
>  
> -static int qcom_iommu_enable_clocks(struct qcom_iommu_dev *qcom_iommu)
> -{
> -	int ret;
> -
> -	ret = clk_prepare_enable(qcom_iommu->iface_clk);
> -	if (ret) {
> -		dev_err(qcom_iommu->dev, "Couldn't enable iface_clk\n");
> -		return ret;
> -	}
> -
> -	ret = clk_prepare_enable(qcom_iommu->bus_clk);
> -	if (ret) {
> -		dev_err(qcom_iommu->dev, "Couldn't enable bus_clk\n");
> -		clk_disable_unprepare(qcom_iommu->iface_clk);
> -		return ret;
> -	}
> -
> -	return 0;
> -}
> -
> -static void qcom_iommu_disable_clocks(struct qcom_iommu_dev *qcom_iommu)
> -{
> -	clk_disable_unprepare(qcom_iommu->bus_clk);
> -	clk_disable_unprepare(qcom_iommu->iface_clk);
> -}
> -
>  static int qcom_iommu_sec_ptbl_init(struct device *dev)
>  {
>  	size_t psize = 0;
> @@ -808,6 +788,7 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
>  	struct qcom_iommu_dev *qcom_iommu;
>  	struct device *dev = &pdev->dev;
>  	struct resource *res;
> +	struct clk *clk;
>  	int ret, max_asid = 0;
>  
>  	/* find the max asid (which is 1:1 to ctx bank idx), so we know how
> @@ -827,17 +808,26 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
>  	if (res)
>  		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
>  
> -	qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
> -	if (IS_ERR(qcom_iommu->iface_clk)) {
> +	clk = devm_clk_get(dev, "iface");
> +	if (IS_ERR(clk)) {
>  		dev_err(dev, "failed to get iface clock\n");
> -		return PTR_ERR(qcom_iommu->iface_clk);
> +		return PTR_ERR(clk);
>  	}
> +	qcom_iommu->clks[CLK_IFACE].clk = clk;
>  
> -	qcom_iommu->bus_clk = devm_clk_get(dev, "bus");
> -	if (IS_ERR(qcom_iommu->bus_clk)) {
> +	clk = devm_clk_get(dev, "bus");
> +	if (IS_ERR(clk)) {
>  		dev_err(dev, "failed to get bus clock\n");
> -		return PTR_ERR(qcom_iommu->bus_clk);
> +		return PTR_ERR(clk);
> +	}
> +	qcom_iommu->clks[CLK_BUS].clk = clk;
> +
> +	clk = devm_clk_get_optional(dev, "tbu");
> +	if (IS_ERR(clk)) {
> +		dev_err(dev, "failed to get tbu clock\n");
> +		return PTR_ERR(clk);
>  	}
> +	qcom_iommu->clks[CLK_TBU].clk = clk;
>  
>  	if (of_property_read_u32(dev->of_node, "qcom,iommu-secure-id",
>  				 &qcom_iommu->sec_id)) {
> @@ -909,14 +899,14 @@ static int __maybe_unused qcom_iommu_resume(struct device *dev)
>  {
>  	struct qcom_iommu_dev *qcom_iommu = dev_get_drvdata(dev);
>  
> -	return qcom_iommu_enable_clocks(qcom_iommu);
> +	return clk_bulk_prepare_enable(CLK_NUM, qcom_iommu->clks);
>  }
>  
>  static int __maybe_unused qcom_iommu_suspend(struct device *dev)
>  {
>  	struct qcom_iommu_dev *qcom_iommu = dev_get_drvdata(dev);
>  
> -	qcom_iommu_disable_clocks(qcom_iommu);
> +	clk_bulk_disable_unprepare(CLK_NUM, qcom_iommu->clks);
>  
>  	return 0;
>  }
> -- 
> 2.17.1
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH v2] iommu/qcom: add optional 'tbu' clock for TLB invalidate
  2020-05-18 14:16 [PATCH v2] iommu/qcom: add optional 'tbu' clock for TLB invalidate Shawn Guo
  2020-05-19  1:41 ` Bjorn Andersson
@ 2020-05-19 12:24 ` Joerg Roedel
  1 sibling, 0 replies; 3+ messages in thread
From: Joerg Roedel @ 2020-05-19 12:24 UTC (permalink / raw)
  To: Shawn Guo
  Cc: linux-arm-msm, Konrad Dybcio, iommu, Stanimir Varbanov,
	Andy Gross, Bjorn Andersson

On Mon, May 18, 2020 at 10:16:56PM +0800, Shawn Guo wrote:
> On some SoCs like MSM8939 with A405 adreno, there is a gfx_tbu clock
> needs to be on while doing TLB invalidate. Otherwise, TLBSYNC status
> will not be correctly reflected, causing the system to go into a bad
> state.  Add it as an optional clock, so that platforms that have this
> clock can pass it over DT.
> 
> While adding the third clock, let's switch to bulk clk API to simplify
> the enable/disable calls.  clk_bulk_get() cannot used because the
> existing two clocks are required while the new one is optional.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> Changes for v2:
>  - Use devm_clk_get_optional() to simplify code and improve readability.
>  - Rename the new clock from 'tlb' to 'tbu'.
>  - qcom_iommu: use bulk clk API to simplfy enable/disable.
> 
>  drivers/iommu/qcom_iommu.c | 62 ++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 36 deletions(-)

Applied, thanks.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-18 14:16 [PATCH v2] iommu/qcom: add optional 'tbu' clock for TLB invalidate Shawn Guo
2020-05-19  1:41 ` Bjorn Andersson
2020-05-19 12:24 ` Joerg Roedel

IOMMU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iommu/0 linux-iommu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iommu linux-iommu/ https://lore.kernel.org/linux-iommu \
		iommu@lists.linux-foundation.org
	public-inbox-index linux-iommu

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linux-foundation.lists.iommu


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git