From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753683AbdLMC2z (ORCPT ); Tue, 12 Dec 2017 21:28:55 -0500 Received: from mga06.intel.com ([134.134.136.31]:47543 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753260AbdLMC2t (ORCPT ); Tue, 12 Dec 2017 21:28:49 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,396,1508828400"; d="scan'208";a="2349682" Subject: Re: [PATCH v3 2/6] iommu/vt-d: Add Intel IOMMU debugfs to show context internals To: "Mehta, Sohil" , "joro@8bytes.org" , "alex.williamson@redhat.com" References: <1512531807-24268-1-git-send-email-sohil.mehta@intel.com> <1512531807-24268-3-git-send-email-sohil.mehta@intel.com> <5A27A74C.9070201@linux.intel.com> <1512678006.120652.17.camel@intel.com> Cc: "Yu, Fenghua" , "Shankar, Ravi V" , "linux-kernel@vger.kernel.org" , "Kammela, Gayatri" , "iommu@lists.linux-foundation.org" , "dwmw2@infradead.org" , "Shevchenko, Andriy" From: Lu Baolu Message-ID: <5A30905D.9000303@linux.intel.com> Date: Wed, 13 Dec 2017 10:28:45 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1512678006.120652.17.camel@intel.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Sorry for late reply. On 12/08/2017 04:19 AM, Mehta, Sohil wrote: > On Wed, 2017-12-06 at 16:16 +0800, Lu Baolu wrote: >> Hi, >> >> On 12/06/2017 11:43 AM, Sohil Mehta wrote: >>> From: Gayatri Kammela >>> >>> >>> + seq_printf(m, "%s Context table entries for Bus: %d\n", >>> + ext ? "Lower" : "", bus); >>> + seq_printf(m, "[entry]\tDID :B :D .F\tLow\t\tHigh\n"); >> WARNING: Prefer seq_puts to seq_printf >> #119: FILE: drivers/iommu/intel-iommu-debug.c:59: >> + seq_printf(m, "[entry]\tDID :B :D .F\tLow\t\tHigh\n"); >> >> (caught by checkpatch.pl) >> > Hi Lu, > > We'll fix this and the other checkpatch.pl warnings. > > >>> + >>> +static void root_tbl_entry_show(struct seq_file *m, void *unused, >> Why do you define the "unused" parameter which will never been used? >> The same questions to other show functions. >> > Some functions in our code that are registered with seq_file needed to > have an unused parameter since seq_file.h defines the show function as: > int (*show) (struct seq_file *m, void *v); > > But a lot of other functions including the one you pointed don't need > to have the unused parameter. We'll remove it from those. > > >>> +void __init intel_iommu_debugfs_init(void) >>> +{ >>> + struct dentry *iommu_debug_root; >>> + >>> + iommu_debug_root = debugfs_create_dir("intel_iommu", >>> NULL); >>> + >>> + if (!iommu_debug_root) { >>> + pr_err("can't create debugfs dir\n"); >> I don't think we need a pr_err() here. System works well even >> debugfs_create_dir() returns NULL. >> >> This is same to all pr_err() in this file. >> > Would the recommendation be to use pr_warn instead of pr_err or should > we entirely skip the message altogether? Greg ever educated me about the use of debugfs_ functions in this thread. https://spinics.net/lists/linux-usb/msg159384.html At least we should avoid the warning/error messages here. Best regards, Lu Baolu