diff mbox

[PATCHv7,10/12] iommu/tegra: smmu: allow duplicate ASID wirte

Message ID 1386835033-4701-11-git-send-email-hdoyu@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hiroshi DOYU Dec. 12, 2013, 7:57 a.m. UTC 
The device, which belongs to the same ASID, can try to enable the same
ASID as the other swgroup devices. This should be allowed but just
skip the actual register write. If the write value is different, it
will return -EINVAL.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
v4:
This was the part of v3, which isn't used any more.
  [PATCHv3 10/19] iommu/tegra: smmu: Get "nvidia,swgroups" from DT

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

Comments

Stephen Warren Dec. 16, 2013, 7:19 p.m. UTC | #1 
On 12/12/2013 12:57 AM, Hiroshi Doyu wrote:
> The device, which belongs to the same ASID, can try to enable the same
> ASID as the other swgroup devices. This should be allowed but just
> skip the actual register write. If the write value is different, it
> will return -EINVAL.

I guess this simplifies the body of the code a lot. Given this, I would
revise the code I suggested in response to a previous patch, to
something more like:

		offs = HWGRP_ASID_REG(i);
		val = smmu_read(smmu, offs);
		if (on) {
			if (WARN_ON(val && val != new_val))
				return -ENODEV;
			memcpy(c->hwgrp, map, sizeof(u64));
		} else {
			WARN_ON(val);
		}
		smmu_write(smmu, val, offs);

> +			if (val) {
> +				if (WARN_ON(val != mask))
> +					return -EINVAL;
> +				goto skip;

That means that:

a) No error is returned when the ASIDs don't match. Surely you still
want to return an error?

b) "skip" skips setting up all HWGRP IDs for the device, whereas perhaps
only 1 was shared yet the rest still need to be set up. If you really do
want to ignore the error, then surely you want "continue;" here not
"goto skip;"?

> +			}
...
> +skip:
>  	return 0;
diff mbox

Patch

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index fd4479a..2b8a302 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -418,9 +418,13 @@  static int __smmu_client_set_hwgrp(struct smmu_client *c,
 		offs = HWGRP_ASID_REG(i);
 		val = smmu_read(smmu, offs);
 		if (on) {
-			if (WARN_ON(val & mask))
-				goto err_hw_busy;
-			val |= mask;
+			if (val) {
+				if (WARN_ON(val != mask))
+					return -EINVAL;
+				goto skip;
+			}
+
+			val = mask;
 			memcpy(c->hwgrp, map, sizeof(u64));
 		} else {
 			WARN_ON((val & mask) == mask);
@@ -430,16 +434,8 @@  static int __smmu_client_set_hwgrp(struct smmu_client *c,
 	}
 
 	FLUSH_SMMU_REGS(smmu);
+skip:
 	return 0;
-
-err_hw_busy:
-	for_each_set_bit(i, map, TEGRA_SWGROUP_MAX) {
-		offs = HWGRP_ASID_REG(i);
-		val = smmu_read(smmu, offs);
-		val &= ~mask;
-		smmu_write(smmu, val, offs);
-	}
-	return -EBUSY;
 }
 
 static int smmu_client_set_hwgrp(struct smmu_client *c,