Messages in this thread | | | From | "Mehta, Sohil" <> | Subject | Re: [PATCH v3 2/6] iommu/vt-d: Add Intel IOMMU debugfs to show context internals | Date | Thu, 7 Dec 2017 20:19:42 +0000 |
| |
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 <gayatri.kammela@intel.com> > > > > > > + 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?
Thanks, Sohil | |