linux-sunxi.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support
@ 2023-01-03  1:08 Samuel Holland
  2023-01-03  1:08 ` [PATCH v2 1/6] dt-bindings: iommu: sun50i: Add compatible for Allwinner D1 Samuel Holland
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Samuel Holland @ 2023-01-03  1:08 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon, Robin Murphy, Chen-Yu Tsai,
	Jernej Skrabec, Krzysztof Kozlowski, Rob Herring
  Cc: iommu, linux-arm-kernel, linux-sunxi, linux-kernel, devicetree,
	Maxime Ripard, Samuel Holland

D1 is a RISC-V SoC from Allwinner's sunxi family. This series adds IOMMU
binding and driver support. The RISC-V architecture code still needs
some small updates to use an IOMMU for DMA[1][2]. I will send those
separately.

[1]: https://lore.kernel.org/linux-riscv/20220428010401.11323-1-samuel@sholland.org/
[2]: https://lore.kernel.org/linux-riscv/7b09e989-0aa1-a557-485e-572f69caf881@arm.com/

Changes in v2:
 - Disallow the 'resets' property for the D1 variant
 - Set bypass based on attached devices instead of using a fixed value

Samuel Holland (6):
  dt-bindings: iommu: sun50i: Add compatible for Allwinner D1
  iommu/sun50i: Track masters attached to the domain
  iommu/sun50i: Keep the bypass register up to date
  iommu/sun50i: Support variants without an external reset
  iommu/sun50i: Add support for the D1 variant
  riscv: dts: allwinner: d1: Add the IOMMU node

 .../iommu/allwinner,sun50i-h6-iommu.yaml      | 20 +++++-
 .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    | 10 +++
 drivers/iommu/sun50i-iommu.c                  | 68 ++++++++++++++-----
 3 files changed, 79 insertions(+), 19 deletions(-)

-- 
2.37.4


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

* [PATCH v2 1/6] dt-bindings: iommu: sun50i: Add compatible for Allwinner D1
  2023-01-03  1:08 [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Samuel Holland
@ 2023-01-03  1:08 ` Samuel Holland
  2023-01-08 20:53   ` Rob Herring
  2023-01-03  1:08 ` [PATCH v2 2/6] iommu/sun50i: Track masters attached to the domain Samuel Holland
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Samuel Holland @ 2023-01-03  1:08 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon, Robin Murphy, Chen-Yu Tsai,
	Jernej Skrabec, Krzysztof Kozlowski, Rob Herring
  Cc: iommu, linux-arm-kernel, linux-sunxi, linux-kernel, devicetree,
	Maxime Ripard, Samuel Holland

D1 contains an IOMMU similar to the one in the H6 SoC, but the D1
variant has no external reset signal.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - Disallow the 'resets' property for the D1 variant

 .../iommu/allwinner,sun50i-h6-iommu.yaml      | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml b/Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml
index e20016f12017..5aeea10cf899 100644
--- a/Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml
@@ -17,7 +17,9 @@ properties:
       The content of the cell is the master ID.
 
   compatible:
-    const: allwinner,sun50i-h6-iommu
+    enum:
+      - allwinner,sun20i-d1-iommu
+      - allwinner,sun50i-h6-iommu
 
   reg:
     maxItems: 1
@@ -37,7 +39,21 @@ required:
   - reg
   - interrupts
   - clocks
-  - resets
+
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - allwinner,sun50i-h6-iommu
+
+then:
+  required:
+    - resets
+
+else:
+  properties:
+    resets: false
 
 additionalProperties: false
 
-- 
2.37.4


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

* [PATCH v2 2/6] iommu/sun50i: Track masters attached to the domain
  2023-01-03  1:08 [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Samuel Holland
  2023-01-03  1:08 ` [PATCH v2 1/6] dt-bindings: iommu: sun50i: Add compatible for Allwinner D1 Samuel Holland
@ 2023-01-03  1:08 ` Samuel Holland
  2023-01-04 22:04   ` Jernej Škrabec
  2023-01-03  1:09 ` [PATCH v2 3/6] iommu/sun50i: Keep the bypass register up to date Samuel Holland
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Samuel Holland @ 2023-01-03  1:08 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon, Robin Murphy, Chen-Yu Tsai,
	Jernej Skrabec, Krzysztof Kozlowski, Rob Herring
  Cc: iommu, linux-arm-kernel, linux-sunxi, linux-kernel, devicetree,
	Maxime Ripard, Samuel Holland

The refcount logic here is broken. The refcount is initialized to 1 with
no devices attached, so it will be decremented back to 1 when the last
device is detached. However, refcount_dec_and_test() returns true only
when the refcount gets decremented to zero, so the domain will never be
cleaned up, and the hardware will never be disabled.

Replace the refcount with a mask of masters attached to the domain. The
domain can be cleaned up when the last master detaches. This mask is
also necessary to correctly set the bypass register.

Fixes: 4100b8c229b3 ("iommu: Add Allwinner H6 IOMMU driver")
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - New patch for v2

 drivers/iommu/sun50i-iommu.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index 5b585eace3d4..3757d5a18318 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -114,8 +114,8 @@ struct sun50i_iommu {
 struct sun50i_iommu_domain {
 	struct iommu_domain domain;
 
-	/* Number of devices attached to the domain */
-	refcount_t refcnt;
+	/* Mask of masters attached to this domain */
+	atomic_t masters;
 
 	/* L1 Page Table */
 	u32 *dt;
@@ -684,8 +684,6 @@ static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type)
 	if (!sun50i_domain->dt)
 		goto err_free_domain;
 
-	refcount_set(&sun50i_domain->refcnt, 1);
-
 	sun50i_domain->domain.geometry.aperture_start = 0;
 	sun50i_domain->domain.geometry.aperture_end = DMA_BIT_MASK(32);
 	sun50i_domain->domain.geometry.force_aperture = true;
@@ -760,23 +758,27 @@ static void sun50i_iommu_detach_domain(struct sun50i_iommu *iommu,
 static void sun50i_iommu_detach_device(struct iommu_domain *domain,
 				       struct device *dev)
 {
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
 	struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
 	struct sun50i_iommu *iommu = dev_iommu_priv_get(dev);
+	int masters = 0;
 
 	dev_dbg(dev, "Detaching from IOMMU domain\n");
 
-	if (iommu->domain != domain)
-		return;
+	for (unsigned int i = 0; i < fwspec->num_ids; i++)
+		masters |= BIT(fwspec->ids[i]);
 
-	if (refcount_dec_and_test(&sun50i_domain->refcnt))
+	if (atomic_fetch_andnot(masters, &sun50i_domain->masters) == masters)
 		sun50i_iommu_detach_domain(iommu, sun50i_domain);
 }
 
 static int sun50i_iommu_attach_device(struct iommu_domain *domain,
 				      struct device *dev)
 {
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
 	struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
 	struct sun50i_iommu *iommu;
+	int masters = 0;
 
 	iommu = sun50i_iommu_from_dev(dev);
 	if (!iommu)
@@ -784,15 +786,11 @@ static int sun50i_iommu_attach_device(struct iommu_domain *domain,
 
 	dev_dbg(dev, "Attaching to IOMMU domain\n");
 
-	refcount_inc(&sun50i_domain->refcnt);
-
-	if (iommu->domain == domain)
-		return 0;
-
-	if (iommu->domain)
-		sun50i_iommu_detach_device(iommu->domain, dev);
+	for (unsigned int i = 0; i < fwspec->num_ids; i++)
+		masters |= BIT(fwspec->ids[i]);
 
-	sun50i_iommu_attach_domain(iommu, sun50i_domain);
+	if (atomic_fetch_or(masters, &sun50i_domain->masters) == 0)
+		sun50i_iommu_attach_domain(iommu, sun50i_domain);
 
 	return 0;
 }
-- 
2.37.4


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

* [PATCH v2 3/6] iommu/sun50i: Keep the bypass register up to date
  2023-01-03  1:08 [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Samuel Holland
  2023-01-03  1:08 ` [PATCH v2 1/6] dt-bindings: iommu: sun50i: Add compatible for Allwinner D1 Samuel Holland
  2023-01-03  1:08 ` [PATCH v2 2/6] iommu/sun50i: Track masters attached to the domain Samuel Holland
@ 2023-01-03  1:09 ` Samuel Holland
  2023-01-04 22:06   ` Jernej Škrabec
  2023-01-03  1:09 ` [PATCH v2 4/6] iommu/sun50i: Support variants without an external reset Samuel Holland
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Samuel Holland @ 2023-01-03  1:09 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon, Robin Murphy, Chen-Yu Tsai,
	Jernej Skrabec, Krzysztof Kozlowski, Rob Herring
  Cc: iommu, linux-arm-kernel, linux-sunxi, linux-kernel, devicetree,
	Maxime Ripard, Samuel Holland

Currently, the IOMMU driver leaves the bypass register at its default
value. The H6 variant of the hardware disables bypass by default. So
once the first device is attached to the IOMMU, translation is enabled
for all masters, even those not attached to an IOMMU group/domain.

On the other hand, the D1 hardware variant enables bypass by default, so
keeping the default value prevents the IOMMU from functioning entirely.

Fixes: 4100b8c229b3 ("iommu: Add Allwinner H6 IOMMU driver")
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - Set bypass based on attached devices instead of using a fixed value

 drivers/iommu/sun50i-iommu.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index 3757d5a18318..a3a462933c62 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -441,6 +441,9 @@ static int sun50i_iommu_enable(struct sun50i_iommu *iommu)
 
 	spin_lock_irqsave(&iommu->iommu_lock, flags);
 
+	iommu_write(iommu, IOMMU_BYPASS_REG,
+		    ~atomic_read(&sun50i_domain->masters));
+
 	iommu_write(iommu, IOMMU_TTB_REG, sun50i_domain->dt_dma);
 	iommu_write(iommu, IOMMU_TLB_PREFETCH_REG,
 		    IOMMU_TLB_PREFETCH_MASTER_ENABLE(0) |
@@ -755,6 +758,17 @@ static void sun50i_iommu_detach_domain(struct sun50i_iommu *iommu,
 	iommu->domain = NULL;
 }
 
+static void sun50i_iommu_update_masters(struct sun50i_iommu *iommu,
+					struct sun50i_iommu_domain *sun50i_domain)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&iommu->iommu_lock, flags);
+	iommu_write(iommu, IOMMU_BYPASS_REG,
+		    ~atomic_read(&sun50i_domain->masters));
+	spin_unlock_irqrestore(&iommu->iommu_lock, flags);
+}
+
 static void sun50i_iommu_detach_device(struct iommu_domain *domain,
 				       struct device *dev)
 {
@@ -770,6 +784,8 @@ static void sun50i_iommu_detach_device(struct iommu_domain *domain,
 
 	if (atomic_fetch_andnot(masters, &sun50i_domain->masters) == masters)
 		sun50i_iommu_detach_domain(iommu, sun50i_domain);
+	else
+		sun50i_iommu_update_masters(iommu, sun50i_domain);
 }
 
 static int sun50i_iommu_attach_device(struct iommu_domain *domain,
@@ -791,6 +807,8 @@ static int sun50i_iommu_attach_device(struct iommu_domain *domain,
 
 	if (atomic_fetch_or(masters, &sun50i_domain->masters) == 0)
 		sun50i_iommu_attach_domain(iommu, sun50i_domain);
+	else
+		sun50i_iommu_update_masters(iommu, sun50i_domain);
 
 	return 0;
 }
-- 
2.37.4


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

* [PATCH v2 4/6] iommu/sun50i: Support variants without an external reset
  2023-01-03  1:08 [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Samuel Holland
                   ` (2 preceding siblings ...)
  2023-01-03  1:09 ` [PATCH v2 3/6] iommu/sun50i: Keep the bypass register up to date Samuel Holland
@ 2023-01-03  1:09 ` Samuel Holland
  2023-01-03  1:09 ` [PATCH v2 5/6] iommu/sun50i: Add support for the D1 variant Samuel Holland
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Samuel Holland @ 2023-01-03  1:09 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon, Robin Murphy, Chen-Yu Tsai,
	Jernej Skrabec, Krzysztof Kozlowski, Rob Herring
  Cc: iommu, linux-arm-kernel, linux-sunxi, linux-kernel, devicetree,
	Maxime Ripard, Samuel Holland, Philipp Zabel

The IOMMU in the Allwinner D1 SoC does not have an external reset line.

Only attempt to get the reset on hardware variants which should have one
according to the binding. And switch from the deprecated function to the
explicit "exclusive" variant.

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

(no changes since v1)

 drivers/iommu/sun50i-iommu.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index a3a462933c62..d19f6ce25f76 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -95,6 +95,10 @@
 
 #define SPAGE_SIZE			4096
 
+struct sun50i_iommu_variant {
+	bool has_reset;
+};
+
 struct sun50i_iommu {
 	struct iommu_device iommu;
 
@@ -995,9 +999,14 @@ static irqreturn_t sun50i_iommu_irq(int irq, void *dev_id)
 
 static int sun50i_iommu_probe(struct platform_device *pdev)
 {
+	const struct sun50i_iommu_variant *variant;
 	struct sun50i_iommu *iommu;
 	int ret, irq;
 
+	variant = of_device_get_match_data(&pdev->dev);
+	if (!variant)
+		return -EINVAL;
+
 	iommu = devm_kzalloc(&pdev->dev, sizeof(*iommu), GFP_KERNEL);
 	if (!iommu)
 		return -ENOMEM;
@@ -1037,7 +1046,8 @@ static int sun50i_iommu_probe(struct platform_device *pdev)
 		goto err_free_group;
 	}
 
-	iommu->reset = devm_reset_control_get(&pdev->dev, NULL);
+	if (variant->has_reset)
+		iommu->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
 	if (IS_ERR(iommu->reset)) {
 		dev_err(&pdev->dev, "Couldn't get our reset line.\n");
 		ret = PTR_ERR(iommu->reset);
@@ -1075,8 +1085,12 @@ static int sun50i_iommu_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static const struct sun50i_iommu_variant sun50i_h6_iommu = {
+	.has_reset = true,
+};
+
 static const struct of_device_id sun50i_iommu_dt[] = {
-	{ .compatible = "allwinner,sun50i-h6-iommu", },
+	{ .compatible = "allwinner,sun50i-h6-iommu", .data = &sun50i_h6_iommu },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, sun50i_iommu_dt);
-- 
2.37.4


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

* [PATCH v2 5/6] iommu/sun50i: Add support for the D1 variant
  2023-01-03  1:08 [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Samuel Holland
                   ` (3 preceding siblings ...)
  2023-01-03  1:09 ` [PATCH v2 4/6] iommu/sun50i: Support variants without an external reset Samuel Holland
@ 2023-01-03  1:09 ` Samuel Holland
  2023-01-03  1:09 ` [PATCH v2 6/6] riscv: dts: allwinner: d1: Add the IOMMU node Samuel Holland
  2023-01-20 15:11 ` [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Joerg Roedel
  6 siblings, 0 replies; 17+ messages in thread
From: Samuel Holland @ 2023-01-03  1:09 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon, Robin Murphy, Chen-Yu Tsai,
	Jernej Skrabec, Krzysztof Kozlowski, Rob Herring
  Cc: iommu, linux-arm-kernel, linux-sunxi, linux-kernel, devicetree,
	Maxime Ripard, Samuel Holland

D1 contains an IOMMU similar to the one in the H6 SoC, but the D1
variant has no external reset signal. It also has some register
definition changes, but none that affect the current driver.

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

(no changes since v1)

 drivers/iommu/sun50i-iommu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index d19f6ce25f76..8652559a2ca3 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -1085,11 +1085,15 @@ static int sun50i_iommu_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static const struct sun50i_iommu_variant sun20i_d1_iommu = {
+};
+
 static const struct sun50i_iommu_variant sun50i_h6_iommu = {
 	.has_reset = true,
 };
 
 static const struct of_device_id sun50i_iommu_dt[] = {
+	{ .compatible = "allwinner,sun20i-d1-iommu", .data = &sun20i_d1_iommu },
 	{ .compatible = "allwinner,sun50i-h6-iommu", .data = &sun50i_h6_iommu },
 	{ /* sentinel */ },
 };
-- 
2.37.4


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

* [PATCH v2 6/6] riscv: dts: allwinner: d1: Add the IOMMU node
  2023-01-03  1:08 [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Samuel Holland
                   ` (4 preceding siblings ...)
  2023-01-03  1:09 ` [PATCH v2 5/6] iommu/sun50i: Add support for the D1 variant Samuel Holland
@ 2023-01-03  1:09 ` Samuel Holland
  2023-01-04 22:07   ` Jernej Škrabec
  2023-01-13 15:35   ` Joerg Roedel
  2023-01-20 15:11 ` [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Joerg Roedel
  6 siblings, 2 replies; 17+ messages in thread
From: Samuel Holland @ 2023-01-03  1:09 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon, Robin Murphy, Chen-Yu Tsai,
	Jernej Skrabec, Krzysztof Kozlowski, Rob Herring
  Cc: iommu, linux-arm-kernel, linux-sunxi, linux-kernel, devicetree,
	Maxime Ripard, Samuel Holland

D1 contains an IOMMU for its video-related hardware. Add the node, and
hook it up to the masters which are already described in the devicetree.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - New patch for v2

 arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
index dff363a3c934..ade50f1e01a4 100644
--- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
+++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
@@ -138,6 +138,14 @@ ccu: clock-controller@2001000 {
 			#reset-cells = <1>;
 		};
 
+		iommu: iommu@2010000 {
+			compatible = "allwinner,sun20i-d1-iommu";
+			reg = <0x2010000 0x10000>;
+			interrupts = <SOC_PERIPHERAL_IRQ(64) IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_IOMMU>;
+			#iommu-cells = <1>;
+		};
+
 		dmic: dmic@2031000 {
 			compatible = "allwinner,sun20i-d1-dmic",
 				     "allwinner,sun50i-h6-dmic";
@@ -574,6 +582,7 @@ mixer0: mixer@5100000 {
 				 <&display_clocks CLK_MIXER0>;
 			clock-names = "bus", "mod";
 			resets = <&display_clocks RST_MIXER0>;
+			iommus = <&iommu 2>;
 
 			ports {
 				#address-cells = <1>;
@@ -596,6 +605,7 @@ mixer1: mixer@5200000 {
 				 <&display_clocks CLK_MIXER1>;
 			clock-names = "bus", "mod";
 			resets = <&display_clocks RST_MIXER1>;
+			iommus = <&iommu 2>;
 
 			ports {
 				#address-cells = <1>;
-- 
2.37.4


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

* Re: [PATCH v2 2/6] iommu/sun50i: Track masters attached to the domain
  2023-01-03  1:08 ` [PATCH v2 2/6] iommu/sun50i: Track masters attached to the domain Samuel Holland
@ 2023-01-04 22:04   ` Jernej Škrabec
  0 siblings, 0 replies; 17+ messages in thread
From: Jernej Škrabec @ 2023-01-04 22:04 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon, Robin Murphy, Chen-Yu Tsai,
	Krzysztof Kozlowski, Rob Herring, Samuel Holland
  Cc: iommu, linux-arm-kernel, linux-sunxi, linux-kernel, devicetree,
	Maxime Ripard, Samuel Holland

Dne torek, 03. januar 2023 ob 02:08:59 CET je Samuel Holland napisal(a):
> The refcount logic here is broken. The refcount is initialized to 1 with
> no devices attached, so it will be decremented back to 1 when the last
> device is detached. However, refcount_dec_and_test() returns true only
> when the refcount gets decremented to zero, so the domain will never be
> cleaned up, and the hardware will never be disabled.
> 
> Replace the refcount with a mask of masters attached to the domain. The
> domain can be cleaned up when the last master detaches. This mask is
> also necessary to correctly set the bypass register.
> 
> Fixes: 4100b8c229b3 ("iommu: Add Allwinner H6 IOMMU driver")
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej



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

* Re: [PATCH v2 3/6] iommu/sun50i: Keep the bypass register up to date
  2023-01-03  1:09 ` [PATCH v2 3/6] iommu/sun50i: Keep the bypass register up to date Samuel Holland
@ 2023-01-04 22:06   ` Jernej Škrabec
  0 siblings, 0 replies; 17+ messages in thread
From: Jernej Škrabec @ 2023-01-04 22:06 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon, Robin Murphy, Chen-Yu Tsai,
	Krzysztof Kozlowski, Rob Herring, Samuel Holland
  Cc: iommu, linux-arm-kernel, linux-sunxi, linux-kernel, devicetree,
	Maxime Ripard, Samuel Holland

Dne torek, 03. januar 2023 ob 02:09:00 CET je Samuel Holland napisal(a):
> Currently, the IOMMU driver leaves the bypass register at its default
> value. The H6 variant of the hardware disables bypass by default. So
> once the first device is attached to the IOMMU, translation is enabled
> for all masters, even those not attached to an IOMMU group/domain.
> 
> On the other hand, the D1 hardware variant enables bypass by default, so
> keeping the default value prevents the IOMMU from functioning entirely.
> 
> Fixes: 4100b8c229b3 ("iommu: Add Allwinner H6 IOMMU driver")
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej



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

* Re: [PATCH v2 6/6] riscv: dts: allwinner: d1: Add the IOMMU node
  2023-01-03  1:09 ` [PATCH v2 6/6] riscv: dts: allwinner: d1: Add the IOMMU node Samuel Holland
@ 2023-01-04 22:07   ` Jernej Škrabec
  2023-01-13 15:35   ` Joerg Roedel
  1 sibling, 0 replies; 17+ messages in thread
From: Jernej Škrabec @ 2023-01-04 22:07 UTC (permalink / raw)
  To: Joerg Roedel, Will Deacon, Robin Murphy, Chen-Yu Tsai,
	Krzysztof Kozlowski, Rob Herring, Samuel Holland
  Cc: iommu, linux-arm-kernel, linux-sunxi, linux-kernel, devicetree,
	Maxime Ripard, Samuel Holland

Dne torek, 03. januar 2023 ob 02:09:03 CET je Samuel Holland napisal(a):
> D1 contains an IOMMU for its video-related hardware. Add the node, and
> hook it up to the masters which are already described in the devicetree.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej



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

* Re: [PATCH v2 1/6] dt-bindings: iommu: sun50i: Add compatible for Allwinner D1
  2023-01-03  1:08 ` [PATCH v2 1/6] dt-bindings: iommu: sun50i: Add compatible for Allwinner D1 Samuel Holland
@ 2023-01-08 20:53   ` Rob Herring
  0 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2023-01-08 20:53 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Chen-Yu Tsai, linux-sunxi, Robin Murphy, iommu, Jernej Skrabec,
	Maxime Ripard, Joerg Roedel, Krzysztof Kozlowski, linux-kernel,
	devicetree, linux-arm-kernel, Rob Herring, Will Deacon


On Mon, 02 Jan 2023 19:08:58 -0600, Samuel Holland wrote:
> D1 contains an IOMMU similar to the one in the H6 SoC, but the D1
> variant has no external reset signal.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> Changes in v2:
>  - Disallow the 'resets' property for the D1 variant
> 
>  .../iommu/allwinner,sun50i-h6-iommu.yaml      | 20 +++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v2 6/6] riscv: dts: allwinner: d1: Add the IOMMU node
  2023-01-03  1:09 ` [PATCH v2 6/6] riscv: dts: allwinner: d1: Add the IOMMU node Samuel Holland
  2023-01-04 22:07   ` Jernej Škrabec
@ 2023-01-13 15:35   ` Joerg Roedel
  2023-01-14 17:17     ` Samuel Holland
  1 sibling, 1 reply; 17+ messages in thread
From: Joerg Roedel @ 2023-01-13 15:35 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Will Deacon, Robin Murphy, Chen-Yu Tsai, Jernej Skrabec,
	Krzysztof Kozlowski, Rob Herring, iommu, linux-arm-kernel,
	linux-sunxi, linux-kernel, devicetree, Maxime Ripard

On Mon, Jan 02, 2023 at 07:09:03PM -0600, Samuel Holland wrote:
>  arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi | 10 ++++++++++

This file does not exist in v6.2-rc3, what tree ist this patch-set based
on?

Regards,

	Joerg


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

* Re: [PATCH v2 6/6] riscv: dts: allwinner: d1: Add the IOMMU node
  2023-01-13 15:35   ` Joerg Roedel
@ 2023-01-14 17:17     ` Samuel Holland
  2023-01-20  9:18       ` Joerg Roedel
  0 siblings, 1 reply; 17+ messages in thread
From: Samuel Holland @ 2023-01-14 17:17 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Will Deacon, Robin Murphy, Chen-Yu Tsai, Jernej Skrabec,
	Krzysztof Kozlowski, Rob Herring, iommu, linux-arm-kernel,
	linux-sunxi, linux-kernel, devicetree, Maxime Ripard

Hi Joerg,

On 1/13/23 09:35, Joerg Roedel wrote:
> On Mon, Jan 02, 2023 at 07:09:03PM -0600, Samuel Holland wrote:
>>  arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi | 10 ++++++++++
> 
> This file does not exist in v6.2-rc3, what tree ist this patch-set based
> on?

The D1/D1s/T113 devicetree is added by this series[1], which will be
merged through the sunxi -> soc tree. That patch is included to show how
the new compatible string is used, and that the driver changes have been
tested. You can ignore it when merging the binding/driver changes. The
rest of the series should apply cleanly to v6.2-rc3.

Regards,
Samuel

[1]:
https://lore.kernel.org/linux-sunxi/20221231233851.24923-1-samuel@sholland.org/


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

* Re: [PATCH v2 6/6] riscv: dts: allwinner: d1: Add the IOMMU node
  2023-01-14 17:17     ` Samuel Holland
@ 2023-01-20  9:18       ` Joerg Roedel
  0 siblings, 0 replies; 17+ messages in thread
From: Joerg Roedel @ 2023-01-20  9:18 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Will Deacon, Robin Murphy, Chen-Yu Tsai, Jernej Skrabec,
	Krzysztof Kozlowski, Rob Herring, iommu, linux-arm-kernel,
	linux-sunxi, linux-kernel, devicetree, Maxime Ripard

On Sat, Jan 14, 2023 at 11:17:06AM -0600, Samuel Holland wrote:
> The D1/D1s/T113 devicetree is added by this series[1], which will be
> merged through the sunxi -> soc tree. That patch is included to show how
> the new compatible string is used, and that the driver changes have been
> tested. You can ignore it when merging the binding/driver changes. The
> rest of the series should apply cleanly to v6.2-rc3.

Thanks for clarifying. Patches 1-5 are now applied.

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

* Re: [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support
  2023-01-03  1:08 [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Samuel Holland
                   ` (5 preceding siblings ...)
  2023-01-03  1:09 ` [PATCH v2 6/6] riscv: dts: allwinner: d1: Add the IOMMU node Samuel Holland
@ 2023-01-20 15:11 ` Joerg Roedel
  2023-02-03 10:21   ` Joerg Roedel
  6 siblings, 1 reply; 17+ messages in thread
From: Joerg Roedel @ 2023-01-20 15:11 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Will Deacon, Robin Murphy, Chen-Yu Tsai, Jernej Skrabec,
	Krzysztof Kozlowski, Rob Herring, iommu, linux-arm-kernel,
	linux-sunxi, linux-kernel, devicetree, Maxime Ripard

Hi Samuel,

On Mon, Jan 02, 2023 at 07:08:57PM -0600, Samuel Holland wrote:
> Samuel Holland (6):
>   dt-bindings: iommu: sun50i: Add compatible for Allwinner D1
>   iommu/sun50i: Track masters attached to the domain
>   iommu/sun50i: Keep the bypass register up to date
>   iommu/sun50i: Support variants without an external reset
>   iommu/sun50i: Add support for the D1 variant
>   riscv: dts: allwinner: d1: Add the IOMMU node

There is a conflict between these patches and changes in the IOMMU core
branch. With those merged in there is a compile warning about
sun50i_iommu_detach_domain() being unused. Fixing that requires removing
of 3-4 functions, which I am not sure is the right solution.

I pushed the arm/allwinner branch out to the iommu tree (with the core
branch merged in) and exluded it from next for now. Can you please have
a look and fix this up in a way that does not break the driver?

Once this is fixed I will include the arm/allwinner branch into my next
branch again.

Thanks,

	Joerg

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

* Re: [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support
  2023-01-20 15:11 ` [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Joerg Roedel
@ 2023-02-03 10:21   ` Joerg Roedel
  2023-02-04 14:49     ` Samuel Holland
  0 siblings, 1 reply; 17+ messages in thread
From: Joerg Roedel @ 2023-02-03 10:21 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Will Deacon, Robin Murphy, Chen-Yu Tsai, Jernej Skrabec,
	Krzysztof Kozlowski, Rob Herring, iommu, linux-arm-kernel,
	linux-sunxi, linux-kernel, devicetree, Maxime Ripard

Hi Samuel,

On Fri, Jan 20, 2023 at 04:11:30PM +0100, Joerg Roedel wrote:
> Once this is fixed I will include the arm/allwinner branch into my next
> branch again.

Since there was no reply to this I nuked the patches from the IOMMU
tree. If this is still relevant please resubmit them after the next
merge window.

Regards,

	Joerg

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

* Re: [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support
  2023-02-03 10:21   ` Joerg Roedel
@ 2023-02-04 14:49     ` Samuel Holland
  0 siblings, 0 replies; 17+ messages in thread
From: Samuel Holland @ 2023-02-04 14:49 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Will Deacon, Robin Murphy, Chen-Yu Tsai, Jernej Skrabec,
	Krzysztof Kozlowski, Rob Herring, iommu, linux-arm-kernel,
	linux-sunxi, linux-kernel, devicetree, Maxime Ripard

Hi Joerg,

On 2/3/23 04:21, Joerg Roedel wrote:
> On Fri, Jan 20, 2023 at 04:11:30PM +0100, Joerg Roedel wrote:
>> There is a conflict between these patches and changes in the IOMMU
>> core branch. With those merged in there is a compile warning about 
>> sun50i_iommu_detach_domain() being unused. Fixing that requires
>> removing of 3-4 functions, which I am not sure is the right
>> solution.
>>
>> Once this is fixed I will include the arm/allwinner branch into my
>> next branch again.
> 
> Since there was no reply to this I nuked the patches from the IOMMU 
> tree. If this is still relevant please resubmit them after the next 
> merge window.

I am not sure what the right solution is here either, and I have not had
the chance to look at it again. With my limited understanding of how the
default domain logic works, and the fact that the IOMMU driver only
supports one domain, it seems the driver should keep that domain enabled
permanently, regardless of which devices are attached. So my patch 2
would be wrong. I will investigate and send a v3 after the merge window.

Regards,
Samuel

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

end of thread, other threads:[~2023-02-04 14:49 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-03  1:08 [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Samuel Holland
2023-01-03  1:08 ` [PATCH v2 1/6] dt-bindings: iommu: sun50i: Add compatible for Allwinner D1 Samuel Holland
2023-01-08 20:53   ` Rob Herring
2023-01-03  1:08 ` [PATCH v2 2/6] iommu/sun50i: Track masters attached to the domain Samuel Holland
2023-01-04 22:04   ` Jernej Škrabec
2023-01-03  1:09 ` [PATCH v2 3/6] iommu/sun50i: Keep the bypass register up to date Samuel Holland
2023-01-04 22:06   ` Jernej Škrabec
2023-01-03  1:09 ` [PATCH v2 4/6] iommu/sun50i: Support variants without an external reset Samuel Holland
2023-01-03  1:09 ` [PATCH v2 5/6] iommu/sun50i: Add support for the D1 variant Samuel Holland
2023-01-03  1:09 ` [PATCH v2 6/6] riscv: dts: allwinner: d1: Add the IOMMU node Samuel Holland
2023-01-04 22:07   ` Jernej Škrabec
2023-01-13 15:35   ` Joerg Roedel
2023-01-14 17:17     ` Samuel Holland
2023-01-20  9:18       ` Joerg Roedel
2023-01-20 15:11 ` [PATCH v2 0/6] iommu/sun50i: Allwinner D1 support Joerg Roedel
2023-02-03 10:21   ` Joerg Roedel
2023-02-04 14:49     ` Samuel Holland

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