Re: [PATCH v3] PCI: vmd: Assign a number to each VMD controller

["Krzysztof Wilczyński" <kw@linux.com>]

Hi Xu,

Thank you for sending the patch over!

A small nitpick below, so feel free to ignore it.

[...] 
> @@ -769,28 +773,48 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  {
>  	unsigned long features = (unsigned long) id->driver_data;
>  	struct vmd_dev *vmd;
> -	int err;
> +	int err = 0;
>  
> -	if (resource_size(&dev->resource[VMD_CFGBAR]) < (1 << 20))
> -		return -ENOMEM;
> +	if (resource_size(&dev->resource[VMD_CFGBAR]) < (1 << 20)) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
>  
>  	vmd = devm_kzalloc(&dev->dev, sizeof(*vmd), GFP_KERNEL);
> -	if (!vmd)
> -		return -ENOMEM;
> +	if (!vmd) {
> +		err = -ENOMEM;
> +		goto out;
> +	}

I assume that you changed the above to use the newly added "out" label to
be consistent given that you also have the other label, but since there is
no clean-up to be done here, do we need this additional label?

>  	vmd->dev = dev;
> +	vmd->instance = ida_simple_get(&vmd_instance_ida, 0, 0, GFP_KERNEL);
> +	if (vmd->instance < 0) {
> +		err = vmd->instance;
> +		goto out;
> +	}

Similarly to here to the above, no clean-up to be done, and you could just
return immediately here.

What do you think?

Also, I think we might have lost a "Reviewed-by" from Jon Derrick somewhere
along the way.  Given that you only updated the commit log and the subject
like, it probably still applies (unless Jon would like to give his seal of
approval again).

	Krzysztof