On 03/13/2018 12:16 PM, Andy Shevchenko wrote:
On Fri, Mar 9, 2018 at 2:50 AM, Gary R Hook <gary.h...@amd.com> wrote:
+ default n
Keep in order?
What order would that be? These few needed files are listed in the same
order as which they appear in amd_iommu.c. I'm gonna need a preference
spelled out, please (and a rationale, so I may better understand).
+/* 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.
I used this technique in the CCP driver code (where it was accepted), in
an effort to do the opposite of what you claim: make the code more
readable. Given the 80 column limit, a large number of arguments, and
very long statements, IMO something needs to give. I don't find the use
of #defines to be obfuscating.
I'm not trying to argue, but rather simply state the perspective /
reasoning I used to create a source file I feel is manageable. I have 17
more iommu patches built upon this strategy, and this seems to be
advantageous for all of them.
+ for (i = start ; i <= end ; i++)
Wasn't sure about the M.O. given that the body of this loop is a single
if statement. And I don't see anywhere in
in section 3.1 where curly braces are called for in this situation. May
I ask for clarification on the style rule, please?
+ if ((amd_iommu_dev_table[i].data ^ 0x3)
+ || amd_iommu_dev_table[i].data)
+ 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.
I can tune these up.
+ if (!iommu)
+ return 0;
When this possible?
It was intended as a sanity check, but if this happens, much worse has
already gone wrong. I'll remove.
+ 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.
I'm sorry, I don't understand. The added blank line doesn't belong to
Extra unneeded line.
iommu mailing list