Re: [PATCH v5 02/13] net: ethernet: aquantia: Common functions and definitions

From: Alexander Loktionov <redacted>
Date: Thu, 12 Jan 2017 21:02:18 -0800

+#define AQ_OBJ_HEADER spinlock_t lock; atomic_t flags; atomic_t busy_count
+
+struct aq_obj_s {
+	AQ_OBJ_HEADER;
+};

Please don't hide multiple declarations and types inside of a macro,
that makes the code harder to understand.

Use a sub-structure or similar, and pass that sub-structure to the
handlers.

+#define AQ_OBJ_TST(_OBJ_, _FLAG_)  ((_FLAG_) & atomic_read(&(_OBJ_)->flags))
+
+#define AQ_OBJ_SET(_OBJ_, _F_) \

 ...

+#define AQ_OBJ_CLR(_OBJ_, _F_) \

Please don't reinvent the wheel.

Use test_bit, set_bit, clear_bit, test_and_set_bit, and
test_and_clear_bit.  Using an atomic_t for flag bits is completely
inappropriate, that type is primarily meant for atomic counters.

The appropriate type for *_bit() operations is "unsigned long".