Messages in this thread Patch in this message | | | From | Andy Shevchenko <> | Date | Tue, 13 Mar 2018 19:16:44 +0200 | Subject | Re: [PATCH v2 1/5] iommu/amd - Add debugfs support |
| |
On Fri, Mar 9, 2018 at 2:50 AM, Gary R Hook <gary.hook@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
| |