Hi All,
Building on top of the arch_setup_dma_ops() cleanup[1], the next step
down the chain is {acpi,of}_dma_configure()... There's plenty to do
here, but it may as well start with this fairly self-contained little
cleanup, pruning yet more redundancy and exposed API surface.
Thanks,
Robin.
[1] https://lore.kernel.org/linux-iommu/[email protected]
Robin Murphy (4):
iommu: Resolve fwspec ops automatically
ACPI: Retire acpi_iommu_fwspec_ops()
OF: Simplify of_iommu_configure()
iommu: Remove iommu_fwspec ops
drivers/acpi/arm64/iort.c | 19 +++-------
drivers/acpi/scan.c | 38 +++++---------------
drivers/acpi/viot.c | 11 ++----
drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 +-
drivers/iommu/iommu-priv.h | 7 ++++
drivers/iommu/iommu.c | 20 +++++------
drivers/iommu/mtk_iommu_v1.c | 2 +-
drivers/iommu/of_iommu.c | 50 ++++++++++-----------------
drivers/iommu/tegra-smmu.c | 2 +-
drivers/of/device.c | 30 ++++++----------
include/acpi/acpi_bus.h | 3 +-
include/linux/iommu.h | 15 ++------
12 files changed, 66 insertions(+), 134 deletions(-)
--
2.39.2.101.g768bb238c484.dirty
There's no real need for callers to resolve ops from a fwnode in order
to then pass both to iommu_fwspec_init() - it's simpler and more sensible
for that to resolve the ops itself. This in turn means we can centralise
the notion of checking for a present driver, and enforce that fwspecs
aren't allocated unless and until we know they will be usable.
Also we've grown a generic fwnode_handle_get() since this code was first
written, so may as well clear up that ugly mismatch while we're in here.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/acpi/arm64/iort.c | 19 +++++--------------
drivers/acpi/scan.c | 8 +++-----
drivers/acpi/viot.c | 11 ++---------
drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 +--
drivers/iommu/iommu-priv.h | 2 ++
drivers/iommu/iommu.c | 9 ++++++---
drivers/iommu/mtk_iommu_v1.c | 2 +-
drivers/iommu/of_iommu.c | 19 ++++++-------------
drivers/iommu/tegra-smmu.c | 2 +-
include/acpi/acpi_bus.h | 3 +--
include/linux/iommu.h | 13 ++-----------
11 files changed, 30 insertions(+), 61 deletions(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index c0b1c2c19444..1b39e9ae7ac1 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1221,10 +1221,10 @@ static bool iort_pci_rc_supports_ats(struct acpi_iort_node *node)
static int iort_iommu_xlate(struct device *dev, struct acpi_iort_node *node,
u32 streamid)
{
- const struct iommu_ops *ops;
struct fwnode_handle *iort_fwnode;
- if (!node)
+ /* If there's no SMMU driver at all, give up now */
+ if (!node || !iort_iommu_driver_enabled(node->type))
return -ENODEV;
iort_fwnode = iort_get_fwnode(node);
@@ -1232,19 +1232,10 @@ static int iort_iommu_xlate(struct device *dev, struct acpi_iort_node *node,
return -ENODEV;
/*
- * If the ops look-up fails, this means that either
- * the SMMU drivers have not been probed yet or that
- * the SMMU drivers are not built in the kernel;
- * Depending on whether the SMMU drivers are built-in
- * in the kernel or not, defer the IOMMU configuration
- * or just abort it.
+ * If the SMMU drivers are enabled but not loaded/probed
+ * yet, this will defer.
*/
- ops = iommu_ops_from_fwnode(iort_fwnode);
- if (!ops)
- return iort_iommu_driver_enabled(node->type) ?
- -EPROBE_DEFER : -ENODEV;
-
- return acpi_iommu_fwspec_init(dev, streamid, iort_fwnode, ops);
+ return acpi_iommu_fwspec_init(dev, streamid, iort_fwnode);
}
struct iort_pci_alias_info {
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index b1a88992c1a9..9d36fc3dc5ac 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1578,10 +1578,9 @@ int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map)
#ifdef CONFIG_IOMMU_API
int acpi_iommu_fwspec_init(struct device *dev, u32 id,
- struct fwnode_handle *fwnode,
- const struct iommu_ops *ops)
+ struct fwnode_handle *fwnode)
{
- int ret = iommu_fwspec_init(dev, fwnode, ops);
+ int ret = iommu_fwspec_init(dev, fwnode);
if (!ret)
ret = iommu_fwspec_add_ids(dev, &id, 1);
@@ -1640,8 +1639,7 @@ static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
#else /* !CONFIG_IOMMU_API */
int acpi_iommu_fwspec_init(struct device *dev, u32 id,
- struct fwnode_handle *fwnode,
- const struct iommu_ops *ops)
+ struct fwnode_handle *fwnode)
{
return -ENODEV;
}
diff --git a/drivers/acpi/viot.c b/drivers/acpi/viot.c
index c8025921c129..2aa69a2fba73 100644
--- a/drivers/acpi/viot.c
+++ b/drivers/acpi/viot.c
@@ -307,21 +307,14 @@ void __init acpi_viot_init(void)
static int viot_dev_iommu_init(struct device *dev, struct viot_iommu *viommu,
u32 epid)
{
- const struct iommu_ops *ops;
-
- if (!viommu)
+ if (!viommu || !IS_ENABLED(CONFIG_VIRTIO_IOMMU))
return -ENODEV;
/* We're not translating ourself */
if (device_match_fwnode(dev, viommu->fwnode))
return -EINVAL;
- ops = iommu_ops_from_fwnode(viommu->fwnode);
- if (!ops)
- return IS_ENABLED(CONFIG_VIRTIO_IOMMU) ?
- -EPROBE_DEFER : -ENODEV;
-
- return acpi_iommu_fwspec_init(dev, epid, viommu->fwnode, ops);
+ return acpi_iommu_fwspec_init(dev, epid, viommu->fwnode);
}
static int viot_pci_dev_iommu_init(struct pci_dev *pdev, u16 dev_id, void *data)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index c572d877b0e1..21c77dbb5ada 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -178,8 +178,7 @@ static int arm_smmu_register_legacy_master(struct device *dev,
it.cur_count = 1;
}
- err = iommu_fwspec_init(dev, &smmu_dev->of_node->fwnode,
- &arm_smmu_ops);
+ err = iommu_fwspec_init(dev, NULL);
if (err)
return err;
diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
index 5f731d994803..078cafcf49b4 100644
--- a/drivers/iommu/iommu-priv.h
+++ b/drivers/iommu/iommu-priv.h
@@ -17,6 +17,8 @@ static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
return dev->iommu->iommu_dev->ops;
}
+const struct iommu_ops *iommu_ops_from_fwnode(const struct fwnode_handle *fwnode);
+
int iommu_group_replace_domain(struct iommu_group *group,
struct iommu_domain *new_domain);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f01133b906e2..07a647e34c72 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2822,11 +2822,14 @@ const struct iommu_ops *iommu_ops_from_fwnode(const struct fwnode_handle *fwnode
return ops;
}
-int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
- const struct iommu_ops *ops)
+int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
{
+ const struct iommu_ops *ops = iommu_ops_from_fwnode(iommu_fwnode);
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+ if (!ops)
+ return -EPROBE_DEFER;
+
if (fwspec)
return ops == fwspec->ops ? 0 : -EINVAL;
@@ -2838,7 +2841,7 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
if (!fwspec)
return -ENOMEM;
- of_node_get(to_of_node(iommu_fwnode));
+ fwnode_handle_get(iommu_fwnode);
fwspec->iommu_fwnode = iommu_fwnode;
fwspec->ops = ops;
dev_iommu_fwspec_set(dev, fwspec);
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index a9fa2a54dc9b..59b1f8701e7d 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -414,7 +414,7 @@ static int mtk_iommu_v1_create_mapping(struct device *dev,
}
if (!fwspec) {
- ret = iommu_fwspec_init(dev, &args->np->fwnode, &mtk_iommu_v1_ops);
+ ret = iommu_fwspec_init(dev, &args->np->fwnode);
if (ret)
return ret;
fwspec = dev_iommu_fwspec_get(dev);
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 3afe0b48a48d..bbfe960cdc13 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -21,26 +21,19 @@ static int of_iommu_xlate(struct device *dev,
struct of_phandle_args *iommu_spec)
{
const struct iommu_ops *ops;
- struct fwnode_handle *fwnode = &iommu_spec->np->fwnode;
int ret;
- ops = iommu_ops_from_fwnode(fwnode);
- if ((ops && !ops->of_xlate) ||
- !of_device_is_available(iommu_spec->np))
+ if (!of_device_is_available(iommu_spec->np))
return -ENODEV;
- ret = iommu_fwspec_init(dev, fwnode, ops);
+ ret = iommu_fwspec_init(dev, &iommu_spec->np->fwnode);
+ if (ret == -EPROBE_DEFER)
+ return driver_deferred_probe_check_state(dev);
if (ret)
return ret;
- /*
- * The otherwise-empty fwspec handily serves to indicate the specific
- * IOMMU device we're waiting for, which will be useful if we ever get
- * a proper probe-ordering dependency mechanism in future.
- */
- if (!ops)
- return driver_deferred_probe_check_state(dev);
- if (!try_module_get(ops->owner))
+ ops = dev_iommu_fwspec_get(dev)->ops;
+ if (!ops->of_xlate || !try_module_get(ops->owner))
return -ENODEV;
ret = ops->of_xlate(dev, iommu_spec);
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 14e525bd0d9b..bd8143b82eac 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -835,7 +835,7 @@ static int tegra_smmu_configure(struct tegra_smmu *smmu, struct device *dev,
const struct iommu_ops *ops = smmu->iommu.ops;
int err;
- err = iommu_fwspec_init(dev, &dev->of_node->fwnode, ops);
+ err = iommu_fwspec_init(dev, &dev->of_node->fwnode);
if (err < 0) {
dev_err(dev, "failed to initialize fwspec: %d\n", err);
return err;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 5de954e2b18a..589cd697738f 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -724,8 +724,7 @@ struct iommu_ops;
bool acpi_dma_supported(const struct acpi_device *adev);
enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
int acpi_iommu_fwspec_init(struct device *dev, u32 id,
- struct fwnode_handle *fwnode,
- const struct iommu_ops *ops);
+ struct fwnode_handle *fwnode);
int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map);
int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
const u32 *input_id);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index ae6e5adebbd1..0614b2736d66 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1002,11 +1002,9 @@ struct iommu_mm_data {
struct list_head sva_handles;
};
-int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
- const struct iommu_ops *ops);
+int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode);
void iommu_fwspec_free(struct device *dev);
int iommu_fwspec_add_ids(struct device *dev, const u32 *ids, int num_ids);
-const struct iommu_ops *iommu_ops_from_fwnode(const struct fwnode_handle *fwnode);
static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
{
@@ -1312,8 +1310,7 @@ static inline void iommu_device_unlink(struct device *dev, struct device *link)
}
static inline int iommu_fwspec_init(struct device *dev,
- struct fwnode_handle *iommu_fwnode,
- const struct iommu_ops *ops)
+ struct fwnode_handle *iommu_fwnode)
{
return -ENODEV;
}
@@ -1328,12 +1325,6 @@ static inline int iommu_fwspec_add_ids(struct device *dev, u32 *ids,
return -ENODEV;
}
-static inline
-const struct iommu_ops *iommu_ops_from_fwnode(const struct fwnode_handle *fwnode)
-{
- return NULL;
-}
-
static inline int
iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features feat)
{
--
2.39.2.101.g768bb238c484.dirty
Now that iommu_fwspec_init() can signal for probe deferral directly,
acpi_iommu_fwspec_ops() is unneeded and can be cleaned up.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/acpi/scan.c | 30 ++++++------------------------
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 9d36fc3dc5ac..d6b64dcbf9a6 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1588,26 +1588,14 @@ int acpi_iommu_fwspec_init(struct device *dev, u32 id,
return ret;
}
-static inline const struct iommu_ops *acpi_iommu_fwspec_ops(struct device *dev)
-{
- struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-
- return fwspec ? fwspec->ops : NULL;
-}
-
static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
{
int err;
- const struct iommu_ops *ops;
/* Serialise to make dev->iommu stable under our potential fwspec */
mutex_lock(&iommu_probe_device_lock);
- /*
- * If we already translated the fwspec there is nothing left to do,
- * return the iommu_ops.
- */
- ops = acpi_iommu_fwspec_ops(dev);
- if (ops) {
+ /* If we already translated the fwspec there is nothing left to do */
+ if (dev_iommu_fwspec_get(dev)) {
mutex_unlock(&iommu_probe_device_lock);
return 0;
}
@@ -1624,16 +1612,7 @@ static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
if (!err && dev->bus)
err = iommu_probe_device(dev);
- /* Ignore all other errors apart from EPROBE_DEFER */
- if (err == -EPROBE_DEFER) {
- return err;
- } else if (err) {
- dev_dbg(dev, "Adding to IOMMU failed: %d\n", err);
- return -ENODEV;
- }
- if (!acpi_iommu_fwspec_ops(dev))
- return -ENODEV;
- return 0;
+ return err;
}
#else /* !CONFIG_IOMMU_API */
@@ -1672,6 +1651,9 @@ int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
ret = acpi_iommu_configure_id(dev, input_id);
if (ret == -EPROBE_DEFER)
return -EPROBE_DEFER;
+ /* Ignore all other errors apart from EPROBE_DEFER */
+ if (ret)
+ dev_dbg(dev, "Adding to IOMMU failed: %d\n", ret);
arch_setup_dma_ops(dev, attr == DEV_DMA_COHERENT);
--
2.39.2.101.g768bb238c484.dirty
The ops in iommu_fwspec are only needed for the early configuration and
probe process, and by now are easy enough to derive on-demand in those
couple of places which need them, so remove the redundant stored copy.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/iommu-priv.h | 5 +++++
drivers/iommu/iommu.c | 11 ++---------
drivers/iommu/of_iommu.c | 4 +++-
include/linux/iommu.h | 2 --
4 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
index 078cafcf49b4..a34efed2884b 100644
--- a/drivers/iommu/iommu-priv.h
+++ b/drivers/iommu/iommu-priv.h
@@ -19,6 +19,11 @@ static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
const struct iommu_ops *iommu_ops_from_fwnode(const struct fwnode_handle *fwnode);
+static inline const struct iommu_ops *iommu_fwspec_ops(struct iommu_fwspec *fwspec)
+{
+ return iommu_ops_from_fwnode(fwspec ? fwspec->iommu_fwnode : NULL);
+}
+
int iommu_group_replace_domain(struct iommu_group *group,
struct iommu_domain *new_domain);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 07a647e34c72..996e79dc582d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -510,7 +510,6 @@ DEFINE_MUTEX(iommu_probe_device_lock);
static int __iommu_probe_device(struct device *dev, struct list_head *group_list)
{
const struct iommu_ops *ops;
- struct iommu_fwspec *fwspec;
struct iommu_group *group;
struct group_device *gdev;
int ret;
@@ -523,12 +522,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
* be present, and that any of their registered instances has suitable
* ops for probing, and thus cheekily co-opt the same mechanism.
*/
- fwspec = dev_iommu_fwspec_get(dev);
- if (fwspec && fwspec->ops)
- ops = fwspec->ops;
- else
- ops = iommu_ops_from_fwnode(NULL);
-
+ ops = iommu_fwspec_ops(dev_iommu_fwspec_get(dev));
if (!ops)
return -ENODEV;
/*
@@ -2831,7 +2825,7 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
return -EPROBE_DEFER;
if (fwspec)
- return ops == fwspec->ops ? 0 : -EINVAL;
+ return ops == iommu_fwspec_ops(fwspec) ? 0 : -EINVAL;
if (!dev_iommu_get(dev))
return -ENOMEM;
@@ -2843,7 +2837,6 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
fwnode_handle_get(iommu_fwnode);
fwspec->iommu_fwnode = iommu_fwnode;
- fwspec->ops = ops;
dev_iommu_fwspec_set(dev, fwspec);
return 0;
}
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index a5d6ff8e4e72..fde3053706c0 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -17,6 +17,8 @@
#include <linux/slab.h>
#include <linux/fsl/mc.h>
+#include "iommu-priv.h"
+
static int of_iommu_xlate(struct device *dev,
struct of_phandle_args *iommu_spec)
{
@@ -32,7 +34,7 @@ static int of_iommu_xlate(struct device *dev,
if (ret)
return ret;
- ops = dev_iommu_fwspec_get(dev)->ops;
+ ops = iommu_ops_from_fwnode(&iommu_spec->np->fwnode);
if (!ops->of_xlate || !try_module_get(ops->owner))
return -ENODEV;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 0614b2736d66..ddfc2c9cdda4 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -965,7 +965,6 @@ extern struct iommu_group *generic_single_device_group(struct device *dev);
/**
* struct iommu_fwspec - per-device IOMMU instance data
- * @ops: ops for this device's IOMMU
* @iommu_fwnode: firmware handle for this device's IOMMU
* @flags: IOMMU_FWSPEC_* flags
* @num_ids: number of associated device IDs
@@ -976,7 +975,6 @@ extern struct iommu_group *generic_single_device_group(struct device *dev);
* consumers.
*/
struct iommu_fwspec {
- const struct iommu_ops *ops;
struct fwnode_handle *iommu_fwnode;
u32 flags;
unsigned int num_ids;
--
2.39.2.101.g768bb238c484.dirty
Fri, Apr 19, 2024 at 05:55:59PM +0100, Robin Murphy kirjoitti:
> There's no real need for callers to resolve ops from a fwnode in order
> to then pass both to iommu_fwspec_init() - it's simpler and more sensible
> for that to resolve the ops itself. This in turn means we can centralise
> the notion of checking for a present driver, and enforce that fwspecs
> aren't allocated unless and until we know they will be usable.
>
> Also we've grown a generic fwnode_handle_get() since this code was first
> written, so may as well clear up that ugly mismatch while we're in here.
..
> +++ b/drivers/iommu/mtk_iommu_v1.c
> if (!fwspec) {
> - ret = iommu_fwspec_init(dev, &args->np->fwnode, &mtk_iommu_v1_ops);
> + ret = iommu_fwspec_init(dev, &args->np->fwnode);
I'm wondering, while at it, if can avoid direct dereference of fwnode by using of_fwnode_handle().
> if (ret)
> return ret;
..
> +++ b/drivers/iommu/of_iommu.c
> - ret = iommu_fwspec_init(dev, fwnode, ops);
> + ret = iommu_fwspec_init(dev, &iommu_spec->np->fwnode);
Ditto.
> + if (ret == -EPROBE_DEFER)
> + return driver_deferred_probe_check_state(dev);
> if (ret)
> return ret;
..
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> - err = iommu_fwspec_init(dev, &dev->of_node->fwnode, ops);
> + err = iommu_fwspec_init(dev, &dev->of_node->fwnode);
Ditto.
> if (err < 0) {
> dev_err(dev, "failed to initialize fwspec: %d\n", err);
> return err;
--
With Best Regards,
Andy Shevchenko
On Fri, Apr 19, 2024 at 6:56 PM Robin Murphy <[email protected]> wrote:
>
> Now that iommu_fwspec_init() can signal for probe deferral directly,
> acpi_iommu_fwspec_ops() is unneeded and can be cleaned up.
>
> Signed-off-by: Robin Murphy <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/acpi/scan.c | 30 ++++++------------------------
> 1 file changed, 6 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 9d36fc3dc5ac..d6b64dcbf9a6 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1588,26 +1588,14 @@ int acpi_iommu_fwspec_init(struct device *dev, u32 id,
> return ret;
> }
>
> -static inline const struct iommu_ops *acpi_iommu_fwspec_ops(struct device *dev)
> -{
> - struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> -
> - return fwspec ? fwspec->ops : NULL;
> -}
> -
> static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
> {
> int err;
> - const struct iommu_ops *ops;
>
> /* Serialise to make dev->iommu stable under our potential fwspec */
> mutex_lock(&iommu_probe_device_lock);
> - /*
> - * If we already translated the fwspec there is nothing left to do,
> - * return the iommu_ops.
> - */
> - ops = acpi_iommu_fwspec_ops(dev);
> - if (ops) {
> + /* If we already translated the fwspec there is nothing left to do */
> + if (dev_iommu_fwspec_get(dev)) {
> mutex_unlock(&iommu_probe_device_lock);
> return 0;
> }
> @@ -1624,16 +1612,7 @@ static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
> if (!err && dev->bus)
> err = iommu_probe_device(dev);
>
> - /* Ignore all other errors apart from EPROBE_DEFER */
> - if (err == -EPROBE_DEFER) {
> - return err;
> - } else if (err) {
> - dev_dbg(dev, "Adding to IOMMU failed: %d\n", err);
> - return -ENODEV;
> - }
> - if (!acpi_iommu_fwspec_ops(dev))
> - return -ENODEV;
> - return 0;
> + return err;
> }
>
> #else /* !CONFIG_IOMMU_API */
> @@ -1672,6 +1651,9 @@ int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
> ret = acpi_iommu_configure_id(dev, input_id);
> if (ret == -EPROBE_DEFER)
> return -EPROBE_DEFER;
> + /* Ignore all other errors apart from EPROBE_DEFER */
> + if (ret)
> + dev_dbg(dev, "Adding to IOMMU failed: %d\n", ret);
>
> arch_setup_dma_ops(dev, attr == DEV_DMA_COHERENT);
>
> --
> 2.39.2.101.g768bb238c484.dirty
>
On Fri, Apr 19, 2024 at 05:55:58PM +0100, Robin Murphy wrote:
> Hi All,
>
> Building on top of the arch_setup_dma_ops() cleanup[1], the next step
> down the chain is {acpi,of}_dma_configure()... There's plenty to do
> here, but it may as well start with this fairly self-contained little
> cleanup, pruning yet more redundancy and exposed API surface.
Tested with QEMU: SMMUv3 DT/IORT, virtio-iommu builtin/module DT/VIOT arm64/x86
Tested-by: Jean-Philippe Brucker <[email protected]>
>
> Thanks,
> Robin.
>
> [1] https://lore.kernel.org/linux-iommu/[email protected]
>
>
> Robin Murphy (4):
> iommu: Resolve fwspec ops automatically
> ACPI: Retire acpi_iommu_fwspec_ops()
> OF: Simplify of_iommu_configure()
> iommu: Remove iommu_fwspec ops
>
> drivers/acpi/arm64/iort.c | 19 +++-------
> drivers/acpi/scan.c | 38 +++++---------------
> drivers/acpi/viot.c | 11 ++----
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 +-
> drivers/iommu/iommu-priv.h | 7 ++++
> drivers/iommu/iommu.c | 20 +++++------
> drivers/iommu/mtk_iommu_v1.c | 2 +-
> drivers/iommu/of_iommu.c | 50 ++++++++++-----------------
> drivers/iommu/tegra-smmu.c | 2 +-
> drivers/of/device.c | 30 ++++++----------
> include/acpi/acpi_bus.h | 3 +-
> include/linux/iommu.h | 15 ++------
> 12 files changed, 66 insertions(+), 134 deletions(-)
>
> --
> 2.39.2.101.g768bb238c484.dirty
>