Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 17 Sep 2021 00:57:55 +0200
From:   Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>
To:     brookxu <brookxu.cn@gmail.com>
Cc:     jonathan.derrick@intel.com, lorenzo.pieralisi@arm.com,
        robh@kernel.org, bhelgaas@google.com, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] PCI: vmd: Assign a number to each VMD controller
Message-ID: <20210916225755.GA1511623@rocinante>
References: <1631675273-1934-1-git-send-email-brookxu.cn@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1631675273-1934-1-git-send-email-brookxu.cn@gmail.com>
Precedence: bulk

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