Re: [PATCH 2/4] iommu/virtio: Add probe request

[Jean-Philippe Brucker <jean-philippe.brucker@arm.com>]

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