On Fri, Mar 9, 2018 at 2:50 AM, Gary R Hook <gary.h...@amd.com> wrote:
> + default n Redundant > +#include <linux/pci.h> > +#include <linux/iommu.h> > +#include <linux/debugfs.h> Keep in order? > +#include "amd_iommu_proto.h" > +#include "amd_iommu_types.h" > +/* DebugFS helpers */ > +#define OBUFP (obuf + oboff) > +#define OBUFLEN obuflen > +#define OBUFSPC (OBUFLEN - oboff) > +#define OSCNPRINTF(fmt, ...) \ > + scnprintf(OBUFP, OBUFSPC, fmt, ## __VA_ARGS__) I don't see any advantages of this. Other way around, they will simple makes things hard to read an understand in place. > + for (i = start ; i <= end ; i++) Missed {} > + if ((amd_iommu_dev_table[i].data[0] ^ 0x3) > + || amd_iommu_dev_table[i].data[1]) > + n++; > + return n; > +} > + > +static ssize_t amd_iommu_debugfs_dtecount_read(struct file *filp, > + char __user *ubuf, > + size_t count, loff_t *offp) > +{ > + struct amd_iommu *iommu = filp->private_data; > + unsigned int obuflen = 512; Sounds like way too much. > + if (!iommu) > + return 0; When this possible? > + obuf = kmalloc(OBUFLEN, GFP_KERNEL); > + if (!obuf) > + return -ENOMEM; > + > + n = amd_iommu_count_valid_dtes(0, 0xFFFF); > + oboff += OSCNPRINTF("%d\n", n); > + return ret; > +} > @@ -89,6 +89,7 @@ > #define ACPI_DEVFLAG_ATSDIS 0x10000000 > > #define LOOP_TIMEOUT 100000 > + > /* > * ACPI table definitions > * Doesn't belong to the patch. > +#endif > + > + Extra unneeded line. -- With Best Regards, Andy Shevchenko