From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1755862AbdAJOnq (ORCPT <rfc822;w@1wt.eu>);
        Tue, 10 Jan 2017 09:43:46 -0500
Received: from 8bytes.org ([81.169.241.247]:50694 "EHLO theia.8bytes.org"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1756799AbdAJOnm (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 10 Jan 2017 09:43:42 -0500
Date: Tue, 10 Jan 2017 15:43:40 +0100
From: Joerg Roedel <joro@8bytes.org>
To: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Cc: linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
        bp@alien8.de, peterz@infradead.org, mingo@redhat.com
Subject: Re: [PATCH v7 2/7] perf/amd/iommu: Modify functions to query max
 banks and counters
Message-ID: <20170110144340.GT17255@8bytes.org>
References: <1484019227-11473-1-git-send-email-Suravee.Suthikulpanit@amd.com>
 <1484019227-11473-3-git-send-email-Suravee.Suthikulpanit@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1484019227-11473-3-git-send-email-Suravee.Suthikulpanit@amd.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Jan 09, 2017 at 09:33:42PM -0600, Suthikulpanit, Suravee wrote:
> +static struct amd_iommu *get_amd_iommu(uint idx)
> +{
> +	uint i = 0;
> +	struct amd_iommu *iommu = NULL;
> +
> +	for_each_iommu(iommu) {
> +		if (i == idx)
> +			break;
> +		i++;
> +	}
> +	return iommu;
> +}

Sorry for missing that in the last review, but this function returns
just the last iommu in the list when there are less iommus than the
requested index.

Is that intentional? The following code checks for !iommu, so I guess
not. It should look more like this then:

	static struct amd_iommu *get_amd_iommu(uint idx)
	{
		uint i = 0;
		struct amd_iommu *iommu, *ret = NULL;

		for_each_iommu(iommu)
			if (i++ == idx) {
				ret = iommu;
				break;
			}
		return ret;
	}


Regards,

	Joerg