Re: [PATCH RFC 04/17] acpi: Do not return struct iommu_ops from acpi_iommu_configure_id()
On Fri, Nov 03, 2023 at 05:48:01PM -0700, Jerry Snitselaar wrote: > > @@ -1632,10 +1633,15 @@ int acpi_dma_configure_id(struct device *dev, enum > > dev_dma_attr attr, > > > > acpi_arch_dma_setup(dev); > > > > - iommu = acpi_iommu_configure_id(dev, input_id); > > - if (PTR_ERR(iommu) == -EPROBE_DEFER) > > + ret = acpi_iommu_configure_id(dev, input_id); > > + if (ret == -EPROBE_DEFER) > > return -EPROBE_DEFER; > > > return ret; ? Maybe? Like this seemed to be a pattern in this code so I left it > > + /* > > +* Historically this routine doesn't fail driver probing due to errors > > +* in acpi_iommu_configure() > > acpi_iommu_configure_id() Thanks Jason ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH RFC 03/17] of: Use -ENODEV consistently in of_iommu_configure()
On Fri, Nov 03, 2023 at 03:03:53PM -0700, Jerry Snitselaar wrote:
> With this the following can be simplified in of_iommu_configure_dev_id:
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 4f77495a2543..b9b995712029 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -61,7 +61,7 @@ static int of_iommu_configure_dev_id(struct device_node
> *master_np,
>"iommu-map-mask", &iommu_spec.np,
>iommu_spec.args);
> if (err)
> - return err == -ENODEV ? NO_IOMMU : err;
> + return err;
Yeah, at this point it just makes sense to erase the whole thing:
-#define NO_IOMMU -ENODEV
-
static int of_iommu_xlate(struct device *dev,
struct of_phandle_args *iommu_spec)
{
@@ -29,7 +27,7 @@ static int of_iommu_xlate(struct device *dev,
ops = iommu_ops_from_fwnode(fwnode);
if ((ops && !ops->of_xlate) ||
!of_device_is_available(iommu_spec->np))
- return NO_IOMMU;
+ return -ENODEV;
ret = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
if (ret)
@@ -61,7 +59,7 @@ static int of_iommu_configure_dev_id(struct device_node
*master_np,
"iommu-map-mask", &iommu_spec.np,
iommu_spec.args);
if (err)
- return err == -ENODEV ? NO_IOMMU : err;
+ return err;
err = of_iommu_xlate(dev, &iommu_spec);
of_node_put(iommu_spec.np);
@@ -72,7 +70,7 @@ static int of_iommu_configure_dev(struct device_node
*master_np,
struct device *dev)
{
struct of_phandle_args iommu_spec;
- int err = NO_IOMMU, idx = 0;
+ int err = -ENODEV, idx = 0;
while (!of_parse_phandle_with_args(master_np, "iommus",
"#iommu-cells",
Thanks,
Jason
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH RFC 02/17] of: Do not return struct iommu_ops from of_iommu_configure()
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
> > ---
> > 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
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH RFC 04/17] acpi: Do not return struct iommu_ops from acpi_iommu_configure_id()
On Sun, Nov 05, 2023 at 09:24:09AM -0400, Jason Gunthorpe wrote:
> On Fri, Nov 03, 2023 at 05:48:01PM -0700, Jerry Snitselaar wrote:
> > > @@ -1632,10 +1633,15 @@ int acpi_dma_configure_id(struct device *dev,
> > > enum dev_dma_attr attr,
> > >
> > > acpi_arch_dma_setup(dev);
> > >
> > > - iommu = acpi_iommu_configure_id(dev, input_id);
> > > - if (PTR_ERR(iommu) == -EPROBE_DEFER)
> > > + ret = acpi_iommu_configure_id(dev, input_id);
> > > + if (ret == -EPROBE_DEFER)
> > > return -EPROBE_DEFER;
> > >
> > return ret; ?
>
> Maybe? Like this seemed to be a pattern in this code so I left it
Yeah, it is fine. I think it just caught my eye, because of this earlier
bit in the patch:
if (err == -EPROBE_DEFER) {
- return ERR_PTR(err);
+ return err;
which needed to get rid of the ERR_PTR.
Regards,
Jerry
>
> > > + /*
> > > + * Historically this routine doesn't fail driver probing due to errors
> > > + * in acpi_iommu_configure()
> >
> > acpi_iommu_configure_id()
>
> Thanks
>
> Jason
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH RFC 01/17] iommu: Remove struct iommu_ops *iommu from arch_setup_dma_ops()
On Fri, Nov 03, 2023 at 01:44:46PM -0300, Jason Gunthorpe wrote: > This is not being used to pass ops, it is just a way to tell if an > iommu driver was probed. These days this can be detected directly via > device_iommu_mapped(). Call device_iommu_mapped() in the two places that > need to check it and remove the iommu parameter everywhere. Yes, that's much better than exposing the iommu ops to a place that should not care about them: Acked-by: Christoph Hellwig ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
