* [PATCH v4 00/13] iommu/rockchip: Use OF_IOMMU
@ 2018-01-18 11:52 Jeffy Chen
2018-01-18 11:52 ` [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes Jeffy Chen
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Jeffy Chen @ 2018-01-18 11:52 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Jeffy Chen,
Russell King, jcliang-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Heiko Stuebner
This series fixes some issues in rockchip iommu driver, and add of_iommu
support in it.
Changes in v4:
Rewrite commit message.
Changes in v3:
Also remove remove() and module_exit() as Tomasz suggested.
Loop platform_get_irq() as Robin suggested.
Add struct rk_iommudata.
Squash iommu/rockchip: Use iommu_group_get_for_dev() for add_device
Only call startup() and shutdown() when iommu attached.
Remove pm_mutex.
Check runtime PM disabled.
Check pm_runtime in rk_iommu_irq().
Remove rk_iommudata->domain.
Changes in v2:
Move irq request to probe(in patch[0])
Move bus_set_iommu() to rk_iommu_probe().
Jeffy Chen (9):
iommu/rockchip: Prohibit unbind and remove
iommu/rockchip: Fix error handling in probe
iommu/rockchip: Request irqs in rk_iommu_probe()
ARM: dts: rockchip: add clocks in vop iommu nodes
iommu/rockchip: Use IOMMU device for dma mapping operations
iommu/rockchip: Use OF_IOMMU to attach devices automatically
iommu/rockchip: Fix error handling in init
iommu/rockchip: Add runtime PM support
iommu/rockchip: Support sharing IOMMU between masters
Tomasz Figa (4):
iommu/rockchip: Fix error handling in attach
iommu/rockchip: Use iopoll helpers to wait for hardware
iommu/rockchip: Fix TLB flush of secondary IOMMUs
iommu/rockchip: Control clocks needed to access the IOMMU
.../devicetree/bindings/iommu/rockchip,iommu.txt | 8 +
arch/arm/boot/dts/rk3036.dtsi | 2 +
arch/arm/boot/dts/rk3288.dtsi | 4 +
drivers/iommu/rockchip-iommu.c | 653 ++++++++++++---------
4 files changed, 386 insertions(+), 281 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes
2018-01-18 11:52 [PATCH v4 00/13] iommu/rockchip: Use OF_IOMMU Jeffy Chen
@ 2018-01-18 11:52 ` Jeffy Chen
[not found] ` <20180118115251.5542-8-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
[not found] ` <20180118115251.5542-1-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-01-18 12:44 ` [PATCH v4 00/13] iommu/rockchip: Use OF_IOMMU Joerg Roedel
2 siblings, 1 reply; 17+ messages in thread
From: Jeffy Chen @ 2018-01-18 11:52 UTC (permalink / raw)
To: linux-kernel
Cc: jcliang, robin.murphy, xxm, tfiga, Jeffy Chen, devicetree,
Heiko Stuebner, linux-rockchip, Rob Herring, Mark Rutland,
Russell King, linux-arm-kernel
Add clocks in vop iommu nodes, since we are going to control clocks in
rockchip iommu driver.
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
arch/arm/boot/dts/rk3036.dtsi | 2 ++
arch/arm/boot/dts/rk3288.dtsi | 4 ++++
2 files changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index 3b704cfed69a..95b0ebc7a40f 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -197,6 +197,8 @@
reg = <0x10118300 0x100>;
interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "vop_mmu";
+ clocks = <&cru ACLK_LCDC>, <&cru SCLK_LCDC>, <&cru HCLK_LCDC>;
+ clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
#iommu-cells = <0>;
status = "disabled";
};
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 6102e4e7f35c..5132dd887297 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -1026,6 +1026,8 @@
reg = <0x0 0xff930300 0x0 0x100>;
interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "vopb_mmu";
+ clocks = <&cru ACLK_VOP0>, <&cru DCLK_VOP0>, <&cru HCLK_VOP0>;
+ clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
power-domains = <&power RK3288_PD_VIO>;
#iommu-cells = <0>;
status = "disabled";
@@ -1074,6 +1076,8 @@
reg = <0x0 0xff940300 0x0 0x100>;
interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "vopl_mmu";
+ clocks = <&cru ACLK_VOP1>, <&cru DCLK_VOP1>, <&cru HCLK_VOP1>;
+ clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
power-domains = <&power RK3288_PD_VIO>;
#iommu-cells = <0>;
status = "disabled";
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU
[not found] ` <20180118115251.5542-1-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2018-01-18 11:52 ` Jeffy Chen
[not found] ` <20180118115251.5542-9-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Jeffy Chen @ 2018-01-18 11:52 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Jeffy Chen,
jcliang-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Heiko Stuebner
From: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Current code relies on master driver enabling necessary clocks before
IOMMU is accessed, however there are cases when the IOMMU should be
accessed while the master is not running yet, for example allocating
V4L2 videobuf2 buffers, which is done by the VB2 framework using DMA
mapping API and doesn't engage the master driver at all.
This patch fixes the problem by letting clocks needed for IOMMU
operation to be listed in Device Tree and making the driver enable them
for the time of accessing the hardware.
Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Signed-off-by: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
.../devicetree/bindings/iommu/rockchip,iommu.txt | 8 ++
drivers/iommu/rockchip-iommu.c | 114 +++++++++++++++++++--
2 files changed, 116 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt b/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
index 2098f7732264..33dd853359fa 100644
--- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
+++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
@@ -14,6 +14,13 @@ Required properties:
"single-master" device, and needs no additional information
to associate with its master device. See:
Documentation/devicetree/bindings/iommu/iommu.txt
+Optional properties:
+- clocks : A list of master clocks requires for the IOMMU to be accessible
+ by the host CPU. The number of clocks depends on the master
+ block and might as well be zero. See [1] for generic clock
+ bindings description.
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
Optional properties:
- rockchip,disable-mmu-reset : Don't use the mmu reset operation.
@@ -27,5 +34,6 @@ Example:
reg = <0xff940300 0x100>;
interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "vopl_mmu";
+ clocks = <&cru ACLK_VOP1>, <&cru DCLK_VOP1>, <&cru HCLK_VOP1>;
#iommu-cells = <0>;
};
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 1914ac52042c..9b85a3050449 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -4,6 +4,7 @@
* published by the Free Software Foundation.
*/
+#include <linux/clk.h>
#include <linux/compiler.h>
#include <linux/delay.h>
#include <linux/device.h>
@@ -89,6 +90,8 @@ struct rk_iommu {
struct device *dev;
void __iomem **bases;
int num_mmu;
+ struct clk **clocks;
+ int num_clocks;
bool reset_disabled;
struct iommu_device iommu;
struct list_head node; /* entry in rk_iommu_domain.iommus */
@@ -443,6 +446,83 @@ static int rk_iommu_force_reset(struct rk_iommu *iommu)
return 0;
}
+static void rk_iommu_put_clocks(struct rk_iommu *iommu)
+{
+ int i;
+
+ for (i = 0; i < iommu->num_clocks; ++i) {
+ clk_unprepare(iommu->clocks[i]);
+ clk_put(iommu->clocks[i]);
+ }
+}
+
+static int rk_iommu_get_clocks(struct rk_iommu *iommu)
+{
+ struct device_node *np = iommu->dev->of_node;
+ int ret;
+ int i;
+
+ ret = of_count_phandle_with_args(np, "clocks", "#clock-cells");
+ if (ret == -ENOENT)
+ return 0;
+ else if (ret < 0)
+ return ret;
+
+ iommu->num_clocks = ret;
+ iommu->clocks = devm_kcalloc(iommu->dev, iommu->num_clocks,
+ sizeof(*iommu->clocks), GFP_KERNEL);
+ if (!iommu->clocks)
+ return -ENOMEM;
+
+ for (i = 0; i < iommu->num_clocks; ++i) {
+ iommu->clocks[i] = of_clk_get(np, i);
+ if (IS_ERR(iommu->clocks[i])) {
+ iommu->num_clocks = i;
+ goto err_clk_put;
+ }
+ ret = clk_prepare(iommu->clocks[i]);
+ if (ret) {
+ clk_put(iommu->clocks[i]);
+ iommu->num_clocks = i;
+ goto err_clk_put;
+ }
+ }
+
+ return 0;
+
+err_clk_put:
+ rk_iommu_put_clocks(iommu);
+
+ return ret;
+}
+
+static int rk_iommu_enable_clocks(struct rk_iommu *iommu)
+{
+ int i, ret;
+
+ for (i = 0; i < iommu->num_clocks; ++i) {
+ ret = clk_enable(iommu->clocks[i]);
+ if (ret)
+ goto err_disable;
+ }
+
+ return 0;
+
+err_disable:
+ for (--i; i >= 0; --i)
+ clk_disable(iommu->clocks[i]);
+
+ return ret;
+}
+
+static void rk_iommu_disable_clocks(struct rk_iommu *iommu)
+{
+ int i;
+
+ for (i = 0; i < iommu->num_clocks; ++i)
+ clk_disable(iommu->clocks[i]);
+}
+
static void log_iova(struct rk_iommu *iommu, int index, dma_addr_t iova)
{
void __iomem *base = iommu->bases[index];
@@ -499,6 +579,8 @@ static irqreturn_t rk_iommu_irq(int irq, void *dev_id)
irqreturn_t ret = IRQ_NONE;
int i;
+ WARN_ON(rk_iommu_enable_clocks(iommu));
+
for (i = 0; i < iommu->num_mmu; i++) {
int_status = rk_iommu_read(iommu->bases[i], RK_MMU_INT_STATUS);
if (int_status == 0)
@@ -545,6 +627,8 @@ static irqreturn_t rk_iommu_irq(int irq, void *dev_id)
rk_iommu_write(iommu->bases[i], RK_MMU_INT_CLEAR, int_status);
}
+ rk_iommu_disable_clocks(iommu);
+
return ret;
}
@@ -587,7 +671,9 @@ static void rk_iommu_zap_iova(struct rk_iommu_domain *rk_domain,
list_for_each(pos, &rk_domain->iommus) {
struct rk_iommu *iommu;
iommu = list_entry(pos, struct rk_iommu, node);
+ rk_iommu_enable_clocks(iommu);
rk_iommu_zap_lines(iommu, iova, size);
+ rk_iommu_disable_clocks(iommu);
}
spin_unlock_irqrestore(&rk_domain->iommus_lock, flags);
}
@@ -816,10 +902,14 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
if (!iommu)
return 0;
- ret = rk_iommu_enable_stall(iommu);
+ ret = rk_iommu_enable_clocks(iommu);
if (ret)
return ret;
+ ret = rk_iommu_enable_stall(iommu);
+ if (ret)
+ goto err_disable_clocks;
+
ret = rk_iommu_force_reset(iommu);
if (ret)
goto err_disable_stall;
@@ -844,11 +934,14 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
dev_dbg(dev, "Attached to iommu domain\n");
rk_iommu_disable_stall(iommu);
+ rk_iommu_disable_clocks(iommu);
return 0;
err_disable_stall:
rk_iommu_disable_stall(iommu);
+err_disable_clocks:
+ rk_iommu_disable_clocks(iommu);
return ret;
}
@@ -871,6 +964,7 @@ static void rk_iommu_detach_device(struct iommu_domain *domain,
spin_unlock_irqrestore(&rk_domain->iommus_lock, flags);
/* Ignore error while disabling, just keep going */
+ WARN_ON(rk_iommu_enable_clocks(iommu));
rk_iommu_enable_stall(iommu);
rk_iommu_disable_paging(iommu);
for (i = 0; i < iommu->num_mmu; i++) {
@@ -878,6 +972,7 @@ static void rk_iommu_detach_device(struct iommu_domain *domain,
rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR, 0);
}
rk_iommu_disable_stall(iommu);
+ rk_iommu_disable_clocks(iommu);
iommu->domain = NULL;
@@ -1167,21 +1262,28 @@ static int rk_iommu_probe(struct platform_device *pdev)
return err;
}
+ err = rk_iommu_get_clocks(iommu);
+ if (err)
+ return err;
+
iommu->reset_disabled = device_property_read_bool(dev,
"rockchip,disable-mmu-reset");
err = iommu_device_sysfs_add(&iommu->iommu, dev, NULL, dev_name(dev));
if (err)
- return err;
+ goto err_put_clocks;
iommu_device_set_ops(&iommu->iommu, &rk_iommu_ops);
err = iommu_device_register(&iommu->iommu);
- if (err) {
- iommu_device_sysfs_remove(&iommu->iommu);
- return err;
- }
+ if (err)
+ goto err_remove_sysfs;
return 0;
+err_remove_sysfs:
+ iommu_device_sysfs_remove(&iommu->iommu);
+err_put_clocks:
+ rk_iommu_put_clocks(iommu);
+ return err;
}
static const struct of_device_id rk_iommu_dt_ids[] = {
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU
[not found] ` <20180118115251.5542-9-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2018-01-18 12:27 ` Robin Murphy
2018-01-18 14:25 ` JeffyChen
0 siblings, 1 reply; 17+ messages in thread
From: Robin Murphy @ 2018-01-18 12:27 UTC (permalink / raw)
To: Jeffy Chen, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
tfiga-F7+t8E8rja9g9hUCZPvPmw
Cc: jcliang-F7+t8E8rja9g9hUCZPvPmw, xxm-TNX95d0MmH7DzftRWevZcw,
devicetree-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
Mark Rutland, Joerg Roedel,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 18/01/18 11:52, Jeffy Chen wrote:
> From: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>
> Current code relies on master driver enabling necessary clocks before
> IOMMU is accessed, however there are cases when the IOMMU should be
> accessed while the master is not running yet, for example allocating
> V4L2 videobuf2 buffers, which is done by the VB2 framework using DMA
> mapping API and doesn't engage the master driver at all.
>
> This patch fixes the problem by letting clocks needed for IOMMU
> operation to be listed in Device Tree and making the driver enable them
> for the time of accessing the hardware.
Is it worth using the clk_bulk_*() APIs for this? At a glance, most of
the code being added here appears to duplicate what those functions
already do (but I'm no clk API expert, for sure).
Robin.
> Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Signed-off-by: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
> .../devicetree/bindings/iommu/rockchip,iommu.txt | 8 ++
> drivers/iommu/rockchip-iommu.c | 114 +++++++++++++++++++--
> 2 files changed, 116 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt b/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
> index 2098f7732264..33dd853359fa 100644
> --- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
> +++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
> @@ -14,6 +14,13 @@ Required properties:
> "single-master" device, and needs no additional information
> to associate with its master device. See:
> Documentation/devicetree/bindings/iommu/iommu.txt
> +Optional properties:
> +- clocks : A list of master clocks requires for the IOMMU to be accessible
> + by the host CPU. The number of clocks depends on the master
> + block and might as well be zero. See [1] for generic clock
> + bindings description.
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>
> Optional properties:
> - rockchip,disable-mmu-reset : Don't use the mmu reset operation.
> @@ -27,5 +34,6 @@ Example:
> reg = <0xff940300 0x100>;
> interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-names = "vopl_mmu";
> + clocks = <&cru ACLK_VOP1>, <&cru DCLK_VOP1>, <&cru HCLK_VOP1>;
> #iommu-cells = <0>;
> };
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 1914ac52042c..9b85a3050449 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -4,6 +4,7 @@
> * published by the Free Software Foundation.
> */
>
> +#include <linux/clk.h>
> #include <linux/compiler.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> @@ -89,6 +90,8 @@ struct rk_iommu {
> struct device *dev;
> void __iomem **bases;
> int num_mmu;
> + struct clk **clocks;
> + int num_clocks;
> bool reset_disabled;
> struct iommu_device iommu;
> struct list_head node; /* entry in rk_iommu_domain.iommus */
> @@ -443,6 +446,83 @@ static int rk_iommu_force_reset(struct rk_iommu *iommu)
> return 0;
> }
>
> +static void rk_iommu_put_clocks(struct rk_iommu *iommu)
> +{
> + int i;
> +
> + for (i = 0; i < iommu->num_clocks; ++i) {
> + clk_unprepare(iommu->clocks[i]);
> + clk_put(iommu->clocks[i]);
> + }
> +}
> +
> +static int rk_iommu_get_clocks(struct rk_iommu *iommu)
> +{
> + struct device_node *np = iommu->dev->of_node;
> + int ret;
> + int i;
> +
> + ret = of_count_phandle_with_args(np, "clocks", "#clock-cells");
> + if (ret == -ENOENT)
> + return 0;
> + else if (ret < 0)
> + return ret;
> +
> + iommu->num_clocks = ret;
> + iommu->clocks = devm_kcalloc(iommu->dev, iommu->num_clocks,
> + sizeof(*iommu->clocks), GFP_KERNEL);
> + if (!iommu->clocks)
> + return -ENOMEM;
> +
> + for (i = 0; i < iommu->num_clocks; ++i) {
> + iommu->clocks[i] = of_clk_get(np, i);
> + if (IS_ERR(iommu->clocks[i])) {
> + iommu->num_clocks = i;
> + goto err_clk_put;
> + }
> + ret = clk_prepare(iommu->clocks[i]);
> + if (ret) {
> + clk_put(iommu->clocks[i]);
> + iommu->num_clocks = i;
> + goto err_clk_put;
> + }
> + }
> +
> + return 0;
> +
> +err_clk_put:
> + rk_iommu_put_clocks(iommu);
> +
> + return ret;
> +}
> +
> +static int rk_iommu_enable_clocks(struct rk_iommu *iommu)
> +{
> + int i, ret;
> +
> + for (i = 0; i < iommu->num_clocks; ++i) {
> + ret = clk_enable(iommu->clocks[i]);
> + if (ret)
> + goto err_disable;
> + }
> +
> + return 0;
> +
> +err_disable:
> + for (--i; i >= 0; --i)
> + clk_disable(iommu->clocks[i]);
> +
> + return ret;
> +}
> +
> +static void rk_iommu_disable_clocks(struct rk_iommu *iommu)
> +{
> + int i;
> +
> + for (i = 0; i < iommu->num_clocks; ++i)
> + clk_disable(iommu->clocks[i]);
> +}
> +
> static void log_iova(struct rk_iommu *iommu, int index, dma_addr_t iova)
> {
> void __iomem *base = iommu->bases[index];
> @@ -499,6 +579,8 @@ static irqreturn_t rk_iommu_irq(int irq, void *dev_id)
> irqreturn_t ret = IRQ_NONE;
> int i;
>
> + WARN_ON(rk_iommu_enable_clocks(iommu));
> +
> for (i = 0; i < iommu->num_mmu; i++) {
> int_status = rk_iommu_read(iommu->bases[i], RK_MMU_INT_STATUS);
> if (int_status == 0)
> @@ -545,6 +627,8 @@ static irqreturn_t rk_iommu_irq(int irq, void *dev_id)
> rk_iommu_write(iommu->bases[i], RK_MMU_INT_CLEAR, int_status);
> }
>
> + rk_iommu_disable_clocks(iommu);
> +
> return ret;
> }
>
> @@ -587,7 +671,9 @@ static void rk_iommu_zap_iova(struct rk_iommu_domain *rk_domain,
> list_for_each(pos, &rk_domain->iommus) {
> struct rk_iommu *iommu;
> iommu = list_entry(pos, struct rk_iommu, node);
> + rk_iommu_enable_clocks(iommu);
> rk_iommu_zap_lines(iommu, iova, size);
> + rk_iommu_disable_clocks(iommu);
> }
> spin_unlock_irqrestore(&rk_domain->iommus_lock, flags);
> }
> @@ -816,10 +902,14 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
> if (!iommu)
> return 0;
>
> - ret = rk_iommu_enable_stall(iommu);
> + ret = rk_iommu_enable_clocks(iommu);
> if (ret)
> return ret;
>
> + ret = rk_iommu_enable_stall(iommu);
> + if (ret)
> + goto err_disable_clocks;
> +
> ret = rk_iommu_force_reset(iommu);
> if (ret)
> goto err_disable_stall;
> @@ -844,11 +934,14 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
> dev_dbg(dev, "Attached to iommu domain\n");
>
> rk_iommu_disable_stall(iommu);
> + rk_iommu_disable_clocks(iommu);
>
> return 0;
>
> err_disable_stall:
> rk_iommu_disable_stall(iommu);
> +err_disable_clocks:
> + rk_iommu_disable_clocks(iommu);
>
> return ret;
> }
> @@ -871,6 +964,7 @@ static void rk_iommu_detach_device(struct iommu_domain *domain,
> spin_unlock_irqrestore(&rk_domain->iommus_lock, flags);
>
> /* Ignore error while disabling, just keep going */
> + WARN_ON(rk_iommu_enable_clocks(iommu));
> rk_iommu_enable_stall(iommu);
> rk_iommu_disable_paging(iommu);
> for (i = 0; i < iommu->num_mmu; i++) {
> @@ -878,6 +972,7 @@ static void rk_iommu_detach_device(struct iommu_domain *domain,
> rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR, 0);
> }
> rk_iommu_disable_stall(iommu);
> + rk_iommu_disable_clocks(iommu);
>
> iommu->domain = NULL;
>
> @@ -1167,21 +1262,28 @@ static int rk_iommu_probe(struct platform_device *pdev)
> return err;
> }
>
> + err = rk_iommu_get_clocks(iommu);
> + if (err)
> + return err;
> +
> iommu->reset_disabled = device_property_read_bool(dev,
> "rockchip,disable-mmu-reset");
>
> err = iommu_device_sysfs_add(&iommu->iommu, dev, NULL, dev_name(dev));
> if (err)
> - return err;
> + goto err_put_clocks;
>
> iommu_device_set_ops(&iommu->iommu, &rk_iommu_ops);
> err = iommu_device_register(&iommu->iommu);
> - if (err) {
> - iommu_device_sysfs_remove(&iommu->iommu);
> - return err;
> - }
> + if (err)
> + goto err_remove_sysfs;
>
> return 0;
> +err_remove_sysfs:
> + iommu_device_sysfs_remove(&iommu->iommu);
> +err_put_clocks:
> + rk_iommu_put_clocks(iommu);
> + return err;
> }
>
> static const struct of_device_id rk_iommu_dt_ids[] = {
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 00/13] iommu/rockchip: Use OF_IOMMU
2018-01-18 11:52 [PATCH v4 00/13] iommu/rockchip: Use OF_IOMMU Jeffy Chen
2018-01-18 11:52 ` [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes Jeffy Chen
[not found] ` <20180118115251.5542-1-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2018-01-18 12:44 ` Joerg Roedel
2018-01-18 14:07 ` JeffyChen
2 siblings, 1 reply; 17+ messages in thread
From: Joerg Roedel @ 2018-01-18 12:44 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel, jcliang, robin.murphy, xxm, tfiga, devicetree,
Heiko Stuebner, linux-rockchip, iommu, Rob Herring, Mark Rutland,
Russell King, linux-arm-kernel
On Thu, Jan 18, 2018 at 07:52:38PM +0800, Jeffy Chen wrote:
> Jeffy Chen (9):
> iommu/rockchip: Prohibit unbind and remove
> iommu/rockchip: Fix error handling in probe
> iommu/rockchip: Request irqs in rk_iommu_probe()
> ARM: dts: rockchip: add clocks in vop iommu nodes
> iommu/rockchip: Use IOMMU device for dma mapping operations
> iommu/rockchip: Use OF_IOMMU to attach devices automatically
> iommu/rockchip: Fix error handling in init
> iommu/rockchip: Add runtime PM support
> iommu/rockchip: Support sharing IOMMU between masters
>
> Tomasz Figa (4):
> iommu/rockchip: Fix error handling in attach
> iommu/rockchip: Use iopoll helpers to wait for hardware
> iommu/rockchip: Fix TLB flush of secondary IOMMUs
> iommu/rockchip: Control clocks needed to access the IOMMU
Please stop resending this every day with minimal changes. I am not
going to take it for v4.16, as we are late in the cycle already and there
are still review comments.
Just collect all feedback, update the patches, rebase them to v4.16-rc1
when its out, and send a new version.
Joerg
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 00/13] iommu/rockchip: Use OF_IOMMU
2018-01-18 12:44 ` [PATCH v4 00/13] iommu/rockchip: Use OF_IOMMU Joerg Roedel
@ 2018-01-18 14:07 ` JeffyChen
0 siblings, 0 replies; 17+ messages in thread
From: JeffyChen @ 2018-01-18 14:07 UTC (permalink / raw)
To: Joerg Roedel
Cc: linux-kernel, jcliang, robin.murphy, xxm, tfiga, devicetree,
Heiko Stuebner, linux-rockchip, iommu, Rob Herring, Mark Rutland,
Russell King, linux-arm-kernel
Hi Joerg,
Thanks for your reply.
On 01/18/2018 08:44 PM, Joerg Roedel wrote:
> On Thu, Jan 18, 2018 at 07:52:38PM +0800, Jeffy Chen wrote:
>> Jeffy Chen (9):
>> iommu/rockchip: Prohibit unbind and remove
>> iommu/rockchip: Fix error handling in probe
>> iommu/rockchip: Request irqs in rk_iommu_probe()
>> ARM: dts: rockchip: add clocks in vop iommu nodes
>> iommu/rockchip: Use IOMMU device for dma mapping operations
>> iommu/rockchip: Use OF_IOMMU to attach devices automatically
>> iommu/rockchip: Fix error handling in init
>> iommu/rockchip: Add runtime PM support
>> iommu/rockchip: Support sharing IOMMU between masters
>>
>> Tomasz Figa (4):
>> iommu/rockchip: Fix error handling in attach
>> iommu/rockchip: Use iopoll helpers to wait for hardware
>> iommu/rockchip: Fix TLB flush of secondary IOMMUs
>> iommu/rockchip: Control clocks needed to access the IOMMU
>
> Please stop resending this every day with minimal changes. I am not
> going to take it for v4.16, as we are late in the cycle already and there
> are still review comments.
>
> Just collect all feedback, update the patches, rebase them to v4.16-rc1
> when its out, and send a new version.
oops, sorry, i send v4 today is mainly because i found v3 would break
some of the rk platforms, which needs an extra patch [7] (ARM: dts:
rockchip: add clocks in vop iommu nodes), but forgot to mention it in
the change log...
will send new version after all the rest patches been reviewed :)
>
>
> Joerg
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU
2018-01-18 12:27 ` Robin Murphy
@ 2018-01-18 14:25 ` JeffyChen
[not found] ` <5A60AE41.2050101-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: JeffyChen @ 2018-01-18 14:25 UTC (permalink / raw)
To: Robin Murphy, linux-kernel, tfiga
Cc: jcliang, xxm, devicetree, Heiko Stuebner, linux-rockchip, iommu,
Rob Herring, Mark Rutland, Joerg Roedel, linux-arm-kernel
Hi Robin,
On 01/18/2018 08:27 PM, Robin Murphy wrote:
>>
>
> Is it worth using the clk_bulk_*() APIs for this? At a glance, most of
> the code being added here appears to duplicate what those functions
> already do (but I'm no clk API expert, for sure).
right, i think it's doable, the clk_bulk APIs are very helpful. i think
we didn't use that is because this patch were wrote for the chromeos 4.4
kernel, which doesn't have clk_bulk yet:)
will do it in the next version, thanks.
>
> Robin.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes
[not found] ` <20180118115251.5542-8-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2018-01-19 3:23 ` Tomasz Figa
[not found] ` <CAAFQd5ACbV6Q2Jwze=hoG=13jSJKkYyDDs2HzvvdSK-RGK+w2g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Tomasz Figa @ 2018-01-19 3:23 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ricky Liang, Robin Murphy,
simon xue, devicetree-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner,
open list:ARM/Rockchip SoC...,
Rob Herring, Mark Rutland, Russell King,
list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS
<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>,
On Thu, Jan 18, 2018 at 8:52 PM, Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> Add clocks in vop iommu nodes, since we are going to control clocks in
> rockchip iommu driver.
>
> Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
> arch/arm/boot/dts/rk3036.dtsi | 2 ++
> arch/arm/boot/dts/rk3288.dtsi | 4 ++++
> 2 files changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
> index 3b704cfed69a..95b0ebc7a40f 100644
> --- a/arch/arm/boot/dts/rk3036.dtsi
> +++ b/arch/arm/boot/dts/rk3036.dtsi
> @@ -197,6 +197,8 @@
> reg = <0x10118300 0x100>;
> interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-names = "vop_mmu";
> + clocks = <&cru ACLK_LCDC>, <&cru SCLK_LCDC>, <&cru HCLK_LCDC>;
> + clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
We should remove clock-names from IOMMU nodes. The Rockchip IOMMU
bindings don't define clock names and only the clocks property should
be given.
Not even saying that the names currently listed are not good examples,
they name SoC clock controller output, rather than device inputs.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes
[not found] ` <CAAFQd5ACbV6Q2Jwze=hoG=13jSJKkYyDDs2HzvvdSK-RGK+w2g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-01-19 4:55 ` JeffyChen
[not found] ` <5A617A5E.6070606-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: JeffyChen @ 2018-01-19 4:55 UTC (permalink / raw)
To: Tomasz Figa
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Russell King, Ricky Liang,
open list:ARM/Rockchip SoC...,
list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Hi Tomasz,
Thanks for your reply.
On 01/19/2018 11:23 AM, Tomasz Figa wrote:
> On Thu, Jan 18, 2018 at 8:52 PM, Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
>> Add clocks in vop iommu nodes, since we are going to control clocks in
>> rockchip iommu driver.
>>
>> Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> ---
>>
>> Changes in v4: None
>> Changes in v3: None
>> Changes in v2: None
>>
>> arch/arm/boot/dts/rk3036.dtsi | 2 ++
>> arch/arm/boot/dts/rk3288.dtsi | 4 ++++
>> 2 files changed, 6 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
>> index 3b704cfed69a..95b0ebc7a40f 100644
>> --- a/arch/arm/boot/dts/rk3036.dtsi
>> +++ b/arch/arm/boot/dts/rk3036.dtsi
>> @@ -197,6 +197,8 @@
>> reg = <0x10118300 0x100>;
>> interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
>> interrupt-names = "vop_mmu";
>> + clocks = <&cru ACLK_LCDC>, <&cru SCLK_LCDC>, <&cru HCLK_LCDC>;
>> + clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
>
> We should remove clock-names from IOMMU nodes. The Rockchip IOMMU
> bindings don't define clock names and only the clocks property should
> be given.
>
hmmm, i'm trying to switch to clk_bulk APIs, the get and put are name
based. or maybe i can use clk_get/put along with other clk_bulk APIs
> Not even saying that the names currently listed are not good examples,
> they name SoC clock controller output, rather than device inputs.
>
> Best regards,
> Tomasz
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes
[not found] ` <5A617A5E.6070606-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2018-01-19 5:12 ` Tomasz Figa
0 siblings, 0 replies; 17+ messages in thread
From: Tomasz Figa @ 2018-01-19 5:12 UTC (permalink / raw)
To: JeffyChen
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Russell King, Ricky Liang,
open list:ARM/Rockchip SoC...,
list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS, Rob Herring,
list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS
<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>,
On Fri, Jan 19, 2018 at 1:55 PM, JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> Hi Tomasz,
>
> Thanks for your reply.
>
>
> On 01/19/2018 11:23 AM, Tomasz Figa wrote:
>>
>> On Thu, Jan 18, 2018 at 8:52 PM, Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> wrote:
>>>
>>> Add clocks in vop iommu nodes, since we are going to control clocks in
>>> rockchip iommu driver.
>>>
>>> Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>> ---
>>>
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>> arch/arm/boot/dts/rk3036.dtsi | 2 ++
>>> arch/arm/boot/dts/rk3288.dtsi | 4 ++++
>>> 2 files changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/rk3036.dtsi
>>> b/arch/arm/boot/dts/rk3036.dtsi
>>> index 3b704cfed69a..95b0ebc7a40f 100644
>>> --- a/arch/arm/boot/dts/rk3036.dtsi
>>> +++ b/arch/arm/boot/dts/rk3036.dtsi
>>> @@ -197,6 +197,8 @@
>>> reg = <0x10118300 0x100>;
>>> interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
>>> interrupt-names = "vop_mmu";
>>> + clocks = <&cru ACLK_LCDC>, <&cru SCLK_LCDC>, <&cru
>>> HCLK_LCDC>;
>>> + clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
>>
>>
>> We should remove clock-names from IOMMU nodes. The Rockchip IOMMU
>> bindings don't define clock names and only the clocks property should
>> be given.
>>
> hmmm, i'm trying to switch to clk_bulk APIs, the get and put are name based.
> or maybe i can use clk_get/put along with other clk_bulk APIs
I think it should be possible to just put the clock pointers to the
clk_bulk_data struct manually. Otherwise, I'm not sure what names we
could use for clock-names, since the clocks depend on master.
(Something like "clock0, clock1, clock2, ..., clockN" could work, but
it doesn't add any value IMHO...).
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU
[not found] ` <5A60AE41.2050101-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2018-01-22 1:18 ` Randy Li
[not found] ` <1f4b3c0d-1414-0012-a072-f4a11f432c21-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Randy Li @ 2018-01-22 1:18 UTC (permalink / raw)
To: JeffyChen, Robin Murphy, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
tfiga-F7+t8E8rja9g9hUCZPvPmw
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
xxm-TNX95d0MmH7DzftRWevZcw, Joerg Roedel,
jcliang-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Heiko Stuebner
On 01/18/2018 10:25 PM, JeffyChen wrote:
> Hi Robin,
>
> On 01/18/2018 08:27 PM, Robin Murphy wrote:
>>>
>>
>> Is it worth using the clk_bulk_*() APIs for this? At a glance, most of
>> the code being added here appears to duplicate what those functions
>> already do (but I'm no clk API expert, for sure).
> right, i think it's doable, the clk_bulk APIs are very helpful. i think
> we didn't use that is because this patch were wrote for the chromeos 4.4
> kernel, which doesn't have clk_bulk yet:)
>
> will do it in the next version, thanks.
Also the power domain driver could manage the clocks as well, I would
suggest to use pm_runtime_*.
>>
>> Robin.
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
--
Randy Li
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU
[not found] ` <1f4b3c0d-1414-0012-a072-f4a11f432c21-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2018-01-22 2:15 ` JeffyChen
[not found] ` <5A654925.9020203-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: JeffyChen @ 2018-01-22 2:15 UTC (permalink / raw)
To: Randy Li, Robin Murphy, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
tfiga-F7+t8E8rja9g9hUCZPvPmw
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
xxm-TNX95d0MmH7DzftRWevZcw, Joerg Roedel,
jcliang-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Heiko Stuebner
Hi Randy,
On 01/22/2018 09:18 AM, Randy Li wrote:
>>
> Also the power domain driver could manage the clocks as well, I would
> suggest to use pm_runtime_*.
actually the clocks required by pm domain may not be the same as what we
want to control here, there might be some clocks only be needed when
accessing mmu registers.
but i'm not very sure about that, will confirm it with Simon Xue.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU
[not found] ` <5A654925.9020203-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2018-01-22 4:09 ` JeffyChen
[not found] ` <5A6563FA.8080602-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: JeffyChen @ 2018-01-22 4:09 UTC (permalink / raw)
To: Randy Li, Robin Murphy, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
tfiga-F7+t8E8rja9g9hUCZPvPmw
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
jcliang-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Heiko Stuebner
Hi Randy,
On 01/22/2018 10:15 AM, JeffyChen wrote:
> Hi Randy,
>
> On 01/22/2018 09:18 AM, Randy Li wrote:
>>>
>> Also the power domain driver could manage the clocks as well, I would
>> suggest to use pm_runtime_*.
>
> actually the clocks required by pm domain may not be the same as what we
> want to control here, there might be some clocks only be needed when
> accessing mmu registers.
>
> but i'm not very sure about that, will confirm it with Simon Xue.
confirmed with Simon, there might be some iommus don't have a pd, and
the CONFIG_PM could be disabled.
so it might be better to control clocks in iommu driver itself.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU
[not found] ` <5A6563FA.8080602-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2018-01-24 9:51 ` Tomasz Figa
2018-01-25 9:42 ` Randy Li
1 sibling, 0 replies; 17+ messages in thread
From: Tomasz Figa @ 2018-01-24 9:51 UTC (permalink / raw)
To: JeffyChen
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Randy Li,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ricky Liang,
open list:ARM/Rockchip SoC...,
list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS
<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>,
, Rob Herring,
list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS
<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>,
, Heiko Stuebner
On Mon, Jan 22, 2018 at 1:09 PM, JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> Hi Randy,
>
>
> On 01/22/2018 10:15 AM, JeffyChen wrote:
>>
>> Hi Randy,
>>
>> On 01/22/2018 09:18 AM, Randy Li wrote:
>>>>
>>>>
>>> Also the power domain driver could manage the clocks as well, I would
>>> suggest to use pm_runtime_*.
>>
>>
>> actually the clocks required by pm domain may not be the same as what we
>> want to control here, there might be some clocks only be needed when
>> accessing mmu registers.
>>
>> but i'm not very sure about that, will confirm it with Simon Xue.
>
>
> confirmed with Simon, there might be some iommus don't have a pd, and the
> CONFIG_PM could be disabled.
>
> so it might be better to control clocks in iommu driver itself.
>
Agreed with Jeffy.
I'd give Reviewed-by, but this is my own patch reposted by Jeffy
(thanks!), so it wouldn't have any value. :)
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU
[not found] ` <5A6563FA.8080602-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-01-24 9:51 ` Tomasz Figa
@ 2018-01-25 9:42 ` Randy Li
[not found] ` <c05d4794-b76d-cfaa-d7ed-c44108117ee2-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
1 sibling, 1 reply; 17+ messages in thread
From: Randy Li @ 2018-01-25 9:42 UTC (permalink / raw)
To: JeffyChen, tfiga-F7+t8E8rja9g9hUCZPvPmw
Cc: Robin Murphy, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA, xxm-TNX95d0MmH7DzftRWevZcw,
Joerg Roedel, jcliang-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Heiko Stuebner
On 01/22/2018 12:09 PM, JeffyChen wrote:
> Hi Randy,
>
> On 01/22/2018 10:15 AM, JeffyChen wrote:
>> Hi Randy,
>>
>> On 01/22/2018 09:18 AM, Randy Li wrote:
>>>>
>>> Also the power domain driver could manage the clocks as well, I would
>>> suggest to use pm_runtime_*.
>>
>> actually the clocks required by pm domain may not be the same as what we
>> want to control here, there might be some clocks only be needed when
>> accessing mmu registers.
>>
>> but i'm not very sure about that, will confirm it with Simon Xue.
>
> confirmed with Simon, there might be some iommus don't have a pd, and
We use the pd to control the NIU node(not on upstream), without a pd or
fake pd, none of the platform would work.
> the CONFIG_PM could be disabled.I am hard to believe a modern platform can work without that.
>
> so it might be better to control clocks in iommu driver itself.I won't insist how the version of the iommu patch on the upstream, I
just post an idea here.
The version for kernel 4.4 is under internal review, the implementation
has been modified many times.
I would suggest the managing clocks in pd is a more easy way and don't
need to spare those thing in two places.
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
--
Randy Li
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU
[not found] ` <c05d4794-b76d-cfaa-d7ed-c44108117ee2-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2018-01-25 10:24 ` JeffyChen
[not found] ` <5A69B065.30501-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: JeffyChen @ 2018-01-25 10:24 UTC (permalink / raw)
To: Randy Li, tfiga-F7+t8E8rja9g9hUCZPvPmw
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jcliang-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Heiko Stuebner
On 01/25/2018 05:42 PM, Randy Li wrote:
>>>
>>
>> confirmed with Simon, there might be some iommus don't have a pd, and
> We use the pd to control the NIU node(not on upstream), without a pd or
> fake pd, none of the platform would work.
after talked offline, it's possible to have iommu without pd in upstream
kernel(and chromeos kernel), but on our internal kernel, the drivers
would require pd(or fake pd) to reset modules when error happens.
anyway, i think that means we do need clock control here.
>> the CONFIG_PM could be disabled.I am hard to believe a modern platform
>> can work without that.
>>
>> so it might be better to control clocks in iommu driver itself.
>I won't
> insist how the version of the iommu patch on the upstream, I
> just post an idea here.
> The version for kernel 4.4 is under internal review, the implementation
> has been modified many times.
>
> I would suggest the managing clocks in pd is a more easy way and don't
> need to spare those thing in two places.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU
[not found] ` <5A69B065.30501-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2018-02-23 10:36 ` JeffyChen
0 siblings, 0 replies; 17+ messages in thread
From: JeffyChen @ 2018-02-23 10:36 UTC (permalink / raw)
To: Randy Li, tfiga-F7+t8E8rja9g9hUCZPvPmw
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jcliang-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Heiko Stuebner
Hi guys,
On 01/25/2018 06:24 PM, JeffyChen wrote:
> On 01/25/2018 05:42 PM, Randy Li wrote:
>>>>
>>>
>>> confirmed with Simon, there might be some iommus don't have a pd, and
>> We use the pd to control the NIU node(not on upstream), without a pd or
>> fake pd, none of the platform would work.
> after talked offline, it's possible to have iommu without pd in upstream
> kernel(and chromeos kernel), but on our internal kernel, the drivers
> would require pd(or fake pd) to reset modules when error happens.
>
> anyway, i think that means we do need clock control here.
found another reason to not depend on pd to control clocks.
currently we are using pd's pm_clk to keep clocks enabled during power
on. but in our pd binding doc, that is not needed:
- clocks (optional): phandles to clocks which need to be enabled while
power domain
switches state.
confirmed with Caesar, the pm_clk only required for some old
chips(rk3288 for example) due to hardware issue.
and i tested my chromebook kevin(rk3399), it works well after remove the
pm_clk:
+++ b/drivers/soc/rockchip/pm_domains.c
@@ -478,7 +478,6 @@ static int rockchip_pm_add_one_domain(struct
rockchip_pmu *pmu,
pd->genpd.power_on = rockchip_pd_power_on;
pd->genpd.attach_dev = rockchip_pd_attach_dev;
pd->genpd.detach_dev = rockchip_pd_detach_dev;
- pd->genpd.flags = GENPD_FLAG_PM_CLK;
will do more tests and send patch tomorrow.
>
>>> the CONFIG_PM could be disabled.I am hard to believe a modern platform
>>> can work without that.
>>>
>>> so it might be better to control clocks in iommu driver itself.
>> I won't
>> insist how the version of the iommu patch on the upstream, I
>> just post an idea here.
>> The version for kernel 4.4 is under internal review, the implementation
>> has been modified many times.
>>
>> I would suggest the managing clocks in pd is a more easy way and don't
>> need to spare those thing in two places.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-02-23 10:36 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-18 11:52 [PATCH v4 00/13] iommu/rockchip: Use OF_IOMMU Jeffy Chen
2018-01-18 11:52 ` [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes Jeffy Chen
[not found] ` <20180118115251.5542-8-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-01-19 3:23 ` Tomasz Figa
[not found] ` <CAAFQd5ACbV6Q2Jwze=hoG=13jSJKkYyDDs2HzvvdSK-RGK+w2g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-19 4:55 ` JeffyChen
[not found] ` <5A617A5E.6070606-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-01-19 5:12 ` Tomasz Figa
[not found] ` <20180118115251.5542-1-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-01-18 11:52 ` [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU Jeffy Chen
[not found] ` <20180118115251.5542-9-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-01-18 12:27 ` Robin Murphy
2018-01-18 14:25 ` JeffyChen
[not found] ` <5A60AE41.2050101-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-01-22 1:18 ` Randy Li
[not found] ` <1f4b3c0d-1414-0012-a072-f4a11f432c21-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-01-22 2:15 ` JeffyChen
[not found] ` <5A654925.9020203-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-01-22 4:09 ` JeffyChen
[not found] ` <5A6563FA.8080602-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-01-24 9:51 ` Tomasz Figa
2018-01-25 9:42 ` Randy Li
[not found] ` <c05d4794-b76d-cfaa-d7ed-c44108117ee2-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-01-25 10:24 ` JeffyChen
[not found] ` <5A69B065.30501-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2018-02-23 10:36 ` JeffyChen
2018-01-18 12:44 ` [PATCH v4 00/13] iommu/rockchip: Use OF_IOMMU Joerg Roedel
2018-01-18 14:07 ` JeffyChen
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).