On 23/03/18 15:00, Robin Murphy wrote:
[...]
>> + /*
>> + * Treat unknown subtype as RESERVED, but urge users to update their
>> + * driver.
>> + */
>> + if (mem->subtype != VIRTIO_IOMMU_RESV_MEM_T_RESERVED &&
>> + mem->subtype != VIRTIO_IOMMU_RESV_MEM_T_MSI)
>> + pr_warn("unknown resv mem subtype 0x%x\n", mem->subtype);
>
> Might as well avoid the extra comparisons by incorporating this into the
> switch statement, i.e.:
>
> default:
> dev_warn(vdev->viommu_dev->dev, ...);
> /* Fallthrough */
> case VIRTIO_IOMMU_RESV_MEM_T_RESERVED:
> ...
>
> (dev_warn is generally preferable to pr_warn when feasible)
Alright, that's nicer
[...]
>> /*
>> * Last step creates a default domain and attaches to it. Everything
>> * must be ready.
>> @@ -735,7 +849,19 @@ static int viommu_add_device(struct device *dev)
>>
>> static void viommu_remove_device(struct device *dev)
>> {
>> - kfree(dev->iommu_fwspec->iommu_priv);
>> + struct viommu_endpoint *vdev;
>> + struct iommu_resv_region *entry, *next;
>> + struct iommu_fwspec *fwspec = dev->iommu_fwspec;
>> +
>> + if (!fwspec || fwspec->ops != &viommu_ops)
>> + return;
>
> Oh good :) I guess that was just a patch-splitting issue. The group
> thing still applies, though.
Ok
Thanks,
Jean
