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
