On Fri, Nov 03, 2023 at 02:42:01PM -0700, Jerry Snitselaar wrote: > On Fri, Nov 03, 2023 at 01:44:47PM -0300, Jason Gunthorpe wrote: > > Nothing needs this pointer. Return a normal error code with the usual > > IOMMU semantic that ENODEV means 'there is no IOMMU driver'. > > > > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx> > > --- > > drivers/iommu/of_iommu.c | 29 ++++++++++++++++++----------- > > drivers/of/device.c | 22 +++++++++++++++------- > > include/linux/of_iommu.h | 13 ++++++------- > > 3 files changed, 39 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c > > index 157b286e36bf3a..e2fa29c16dd758 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 > > It looks to me like it will only return 0, -ENODEV, or -EPROBEDEFER > with other -errno getting boiled down to -ENODEV. Yeah, that next patch sorts it out, it is sort of a typo here: @@ -173,7 +173,7 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np, if (err == -EPROBE_DEFER) return err; dev_dbg(dev, "Adding to IOMMU failed: %d\n", err); - return -ENODEV; + return err; } if (!ops) return -ENODEV; > > @@ -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) { > > + if (err < 0) { > > + if (err == -EPROBE_DEFER) > > + return err; > > dev_dbg(dev, "Adding to IOMMU failed: %d\n", err); > > minor thing, but should this use %pe and ERR_PTR(err) like is done > in of_dma_configure_id? Sure Thanks, Jason