[PATCH v2 02/17] iommmu/of: Do not return struct iommu_ops from of_iommu_configure()

Jason Gunthorpe
2023-11-15T14:06:16Z

Nothing needs this pointer. Return a normal error code with the usual
IOMMU semantic that ENODEV means 'there is no IOMMU driver'.

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
drivers/iommu/of_iommu.c | 31 +++++++++++++++++++------------
drivers/of/device.c | 22 +++++++++++++++-------
include/linux/of_iommu.h | 13 ++++++-------
3 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 157b286e36bf3a..b47dcb66cde98d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -107,20 +107,26 @@ static int of_iommu_configure_device(struct device_node *master_np,
of_iommu_configure_dev(master_np, dev);
}

-const struct iommu_ops *of_iommu_configure(struct device *dev,
- struct device_node *master_np,
- const u32 *id)
+/*
+ * Returns:
+ * 0 on success, an iommu was configured
+ * -ENODEV if the device does not have any IOMMU
+ * -EPROBEDEFER if probing should be tried again
+ * -errno fatal errors
+ */
+int of_iommu_configure(struct device *dev, struct device_node *master_np,
+ const u32 *id)
{
const struct iommu_ops *ops = NULL;
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
int err = NO_IOMMU;

if (!master_np)
- return NULL;
+ return -ENODEV;

if (fwspec) {
if (fwspec->ops)
- return fwspec->ops;
+ return 0;

/* In the deferred case, start again from scratch */
iommu_fwspec_free(dev);
@@ -163,14 +169,15 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
err = iommu_probe_device(dev);

/* Ignore all other errors apart from EPROBE_DEFER */
- if (err == -EPROBE_DEFER) {
- ops = ERR_PTR(err);
- } else if (err < 0) {
- dev_dbg(dev, "Adding to IOMMU failed: %d\n", err);
- ops = NULL;
+ if (err < 0) {
+ if (err == -EPROBE_DEFER)
+ return err;
+ dev_dbg(dev, "Adding to IOMMU failed: %pe\n", ERR_PTR(err));
+ return err;
}
-
- return ops;
+ if (!ops)
+ return -ENODEV;
+ return 0;
}

static enum iommu_resv_type __maybe_unused
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 65c71be71a8d45..873d933e8e6d1d 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -93,12 +93,12 @@ of_dma_set_restricted_buffer(struct device *dev, struct device_node *np)
int of_dma_configure_id(struct device *dev, struct device_node *np,
bool force_dma, const u32 *id)
{
- const struct iommu_ops *iommu;
const struct bus_dma_region *map = NULL;
struct device_node *bus_np;
u64 dma_start = 0;
u64 mask, end, size = 0;
bool coherent;
+ int iommu_ret;
int ret;

if (np == dev->of_node)
@@ -181,21 +181,29 @@ int of_dma_configure_id(struct device *dev, struct device_node *np,
dev_dbg(dev, "device is%sdma coherent\n",
coherent ? " " : " not ");

- iommu = of_iommu_configure(dev, np, id);
- if (PTR_ERR(iommu) == -EPROBE_DEFER) {
+ iommu_ret = of_iommu_configure(dev, np, id);
+ if (iommu_ret == -EPROBE_DEFER) {
/* Don't touch range map if it wasn't set from a valid dma-ranges */
if (!ret)
dev->dma_range_map = NULL;
kfree(map);
return -EPROBE_DEFER;
- }
+ } else if (iommu_ret == -ENODEV) {
+ dev_dbg(dev, "device is not behind an iommu\n");
+ } else if (iommu_ret) {
+ dev_err(dev, "iommu configuration for device failed with %pe\n",
+ ERR_PTR(iommu_ret));

- dev_dbg(dev, "device is%sbehind an iommu\n",
- iommu ? " " : " not ");
+ /*
+ * Historically this routine doesn't fail driver probing
+ * due to errors in of_iommu_configure()
+ */
+ } else
+ dev_dbg(dev, "device is behind an iommu\n");

arch_setup_dma_ops(dev, dma_start, size, coherent);

- if (!iommu)
+ if (iommu_ret)
of_dma_set_restricted_buffer(dev, np);

return 0;
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 9a5e6b410dd2fb..e61cbbe12dac6f 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -8,20 +8,19 @@ struct iommu_ops;

#ifdef CONFIG_OF_IOMMU

-extern const struct iommu_ops *of_iommu_configure(struct device *dev,
- struct device_node *master_np,
- const u32 *id);
+extern int of_iommu_configure(struct device *dev, struct device_node *master_np,
+ const u32 *id);

extern void of_iommu_get_resv_regions(struct device *dev,
struct list_head *list);

#else

-static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
- struct device_node *master_np,
- const u32 *id)
+static inline int of_iommu_configure(struct device *dev,
+ struct device_node *master_np,
+ const u32 *id)
{
- return NULL;
+ return -ENODEV;
}

static inline void of_iommu_get_resv_regions(struct device *dev,
--
2.42.0