[v2,02/14] crypto: omap-aes: Add useful debug macros

Commit Message

Joel Fernandes Aug. 18, 2013, 2:42 a.m. UTC

When DEBUG is enabled, these macros can be used to print variables in integer
and hex format, and clearly display which registers, offsets and values are
being read/written , including printing the names of the offsets and their values.

Note:
This patch results in a checkpatch error that cannot be fixed.
ERROR: Macros with multiple statements should be enclosed in a do - while loop
+#define omap_aes_read(dd, offset)                                      \
+       __raw_readl(dd->io_base + offset);                              \
+       pr_debug("omap_aes_read(" #offset ")\n");

Using do-while loop will break a lot of code such as:
ret = omap_aes_read(..);

On the other hand, not using a do-while loop will only result in a spurious
debug print message when DEBUG is enabled, all other issues would be caught at
compile time if any. As such, there is no code in the driver as of now that
requires a do-while loop, but there is code that will break if a do-while loop
is used in the macro so we ignore the checkpatch error in this case.

Signed-off-by: Joel Fernandes <[e-mail]>
---
 drivers/crypto/omap-aes.c |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Comments

Joe Perches Aug. 18, 2013, 4:22 a.m. UTC | #1

On Sat, 2013-08-17 at 21:42 -0500, Joel Fernandes wrote:
> When DEBUG is enabled, these macros can be used to print variables in integer
> and hex format, and clearly display which registers, offsets and values are
> being read/written , including printing the names of the offsets and their values.
> 
> Note:
> This patch results in a checkpatch error that cannot be fixed.
> ERROR: Macros with multiple statements should be enclosed in a do - while loop
> +#define omap_aes_read(dd, offset)                                      \
> +       __raw_readl(dd->io_base + offset);                              \
> +       pr_debug("omap_aes_read(" #offset ")\n");
> 
> Using do-while loop will break a lot of code such as:
> ret = omap_aes_read(..);

That's where you use a statement expression macro

#define omap_aes_read(dd, offset)					\
({									\
	pr_debug("omap_aes_read("omap_aes_read(" #offset ")\n");	\
	__raw_readl((dd)->iobase + offset);				\
})

Patch

diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
index ee15b0f..26b802b 100644
--- a/drivers/crypto/omap-aes.c
+++ b/drivers/crypto/omap-aes.c
@@ -13,7 +13,8 @@ 
  *
  */
 
-#define pr_fmt(fmt) "%s: " fmt, __func__
+#define prn(num) pr_debug(#num "=%d\n", num)
+#define prx(num) pr_debug(#num "=%x\n", num)
 
 #include <linux/err.h>
 #include <linux/module.h>
@@ -172,16 +173,35 @@  struct omap_aes_dev {
 static LIST_HEAD(dev_list);
 static DEFINE_SPINLOCK(list_lock);
 
+#ifdef DEBUG
+/*
+ * Note: In DEBUG mode, when using conditionals, omap_aes_read _must_
+ * be surrounded by braces otherwise you may see spurious prints.
+ */
+#define omap_aes_read(dd, offset)					\
+	__raw_readl(dd->io_base + offset);				\
+	pr_debug("omap_aes_read(" #offset ")\n");
+#else
 static inline u32 omap_aes_read(struct omap_aes_dev *dd, u32 offset)
 {
 	return __raw_readl(dd->io_base + offset);
 }
+#endif
 
+#ifdef DEBUG
+#define omap_aes_write(dd, offset, value)				\
+	do {								\
+		pr_debug("omap_aes_write(" #offset "=%x) value=%d\n",	\
+			 offset, value);				\
+		__raw_writel(value, dd->io_base + offset);		\
+	} while (0)
+#else
 static inline void omap_aes_write(struct omap_aes_dev *dd, u32 offset,
 				  u32 value)
 {
 	__raw_writel(value, dd->io_base + offset);
 }
+#endif
 
 static inline void omap_aes_write_mask(struct omap_aes_dev *dd, u32 offset,
 					u32 value, u32 mask)