Re: [PATCH v4 1/3] arm,smmu: switch to using iommu_fwspec functions

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 22 Jul 2021, Julien Grall wrote:
> Hi Stefano,
> 
> On 17/07/2021 00:59, Stefano Stabellini wrote:
> > diff --git a/xen/drivers/passthrough/device_tree.c
> > b/xen/drivers/passthrough/device_tree.c
> > index 999b831d90..911f82a561 100644
> > --- a/xen/drivers/passthrough/device_tree.c
> > +++ b/xen/drivers/passthrough/device_tree.c
> > @@ -140,8 +140,12 @@ int iommu_add_dt_device(struct dt_device_node *np)
> >       if ( !ops )
> >           return -EINVAL;
> >   +    /*
> > +     * Device already added. It can happen when both iommus and
> > +     * mmu-masters are present.
> > +     */
> 
> This is common code. So I would suggest to write a generic comment to avoid
> any misunderstanding. The one added just after the call in iommu_do_domctl()
> would seem more suitable.

OK, I went with this:

            * Some Device Trees may expose both legacy SMMU and generic
            * IOMMU bindings together. If both are present, the device
            * can be already added.



> >       if ( dev_iommu_fwspec_get(dev) )
> > -        return -EEXIST;
> > +        return 0;
> There are a few things to mention here:
> 
>  1) The change is not explained in the commit message

I will add


>  2) One of the caller was checking -EEXIST. As you dropped the only place
> where -EEXIST should be returned, can you drop the check in the caller?

Good point, yes I think it can be dropped.


> Ideally this should be in a separate patch because this change is not entirely
> related to this patch.

I can do that, I'll send out v5 shortly.