diff mbox series

[v5,4/4] riscv/barrier: Resolve checkpatch.pl error

Message ID 20240213224000.2597959-1-ericchancf@google.com
State New
Headers
Series riscv/barrier: tidying up barrier-related macro |

Commit Message

Eric Chan Feb. 13, 2024, 10:40 p.m. UTC 
  The past form of RISCV_FENCE would cause checkpatch.pl to issue
error messages, the example is as follows:
ERROR: space required after that ',' (ctx:VxV)
+#define __atomic_acquire_fence()       RISCV_FENCE(r,rw)
                                                     ^
fix the remaining of RISCV_FENCE.

Signed-off-by: Eric Chan <ericchancf@google.com>
---
 arch/riscv/include/asm/barrier.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Andrea Parri Feb. 16, 2024, 9:05 p.m. UTC | #1 
On Tue, Feb 13, 2024 at 10:40:00PM +0000, Eric Chan wrote:
> The past form of RISCV_FENCE would cause checkpatch.pl to issue
> error messages, the example is as follows:
> ERROR: space required after that ',' (ctx:VxV)
> +#define __atomic_acquire_fence()       RISCV_FENCE(r,rw)

Not the "best" example, according to Samuel's feedback and the diff
below.  How about something like:

ERROR: space required after that ',' (ctx:VxV)
#10: FILE: arch/riscv/include/asm/barrier.h:28:
+#define __smp_mb()	RISCV_FENCE(rw,rw)


> -#define __mb()		RISCV_FENCE(iorw,iorw)
> -#define __rmb()		RISCV_FENCE(ir,ir)
> -#define __wmb()		RISCV_FENCE(ow,ow)
> +#define __mb()		RISCV_FENCE(iorw, iorw)
> +#define __rmb()		RISCV_FENCE(ir, ir)
> +#define __wmb()		RISCV_FENCE(ow, ow)

This would go away per my comment to 1/4.


From a less technical viewpoint and FYI, an unwritten rule of "working
with the Linux kernel community" is:

  Don't forget to keep people who spent time reviewing, or providing
  feedback to, your patch in the loop/in Cc: in your next iterations;
  they might be the only people who will be able to provide feedback
  /suggestions/help in your future life.

;-)

  Andrea
Eric Chan Feb. 17, 2024, 1:07 p.m. UTC | #2 
Hi Andrea,

Thank you very much for patiently reviewing and pointing out areas for improvement.
I also appreciate your kindness.
This is actually my second time submitting an upstream patch,
and the first time submitting multiple patches.
There are still many shortcomings, even though I carefully read the documentation
and refer other submissions before submitting my changes.

Once again, thank you for reviewing and providing such friendly suggestions.

Sincerely,
  Eric Chan
diff mbox series

Patch

diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
index 4f4743d7440d..880b56d8480d 100644
--- a/arch/riscv/include/asm/barrier.h
+++ b/arch/riscv/include/asm/barrier.h
@@ -19,19 +19,19 @@ 
 
 
 /* These barriers need to enforce ordering on both devices or memory. */
-#define __mb()		RISCV_FENCE(iorw,iorw)
-#define __rmb()		RISCV_FENCE(ir,ir)
-#define __wmb()		RISCV_FENCE(ow,ow)
+#define __mb()		RISCV_FENCE(iorw, iorw)
+#define __rmb()		RISCV_FENCE(ir, ir)
+#define __wmb()		RISCV_FENCE(ow, ow)
 
 /* These barriers do not need to enforce ordering on devices, just memory. */
-#define __smp_mb()	RISCV_FENCE(rw,rw)
-#define __smp_rmb()	RISCV_FENCE(r,r)
-#define __smp_wmb()	RISCV_FENCE(w,w)
+#define __smp_mb()	RISCV_FENCE(rw, rw)
+#define __smp_rmb()	RISCV_FENCE(r, r)
+#define __smp_wmb()	RISCV_FENCE(w, w)
 
 #define __smp_store_release(p, v)					\
 do {									\
 	compiletime_assert_atomic_type(*p);				\
-	RISCV_FENCE(rw,w);						\
+	RISCV_FENCE(rw, w);						\
 	WRITE_ONCE(*p, v);						\
 } while (0)
 
@@ -39,7 +39,7 @@  do {									\
 ({									\
 	typeof(*p) ___p1 = READ_ONCE(*p);				\
 	compiletime_assert_atomic_type(*p);				\
-	RISCV_FENCE(r,rw);						\
+	RISCV_FENCE(r, rw);						\
 	___p1;								\
 })
 
@@ -68,7 +68,7 @@  do {									\
  * instances the scheduler pairs this with an mb(), so nothing is necessary on
  * the new hart.
  */
-#define smp_mb__after_spinlock()	RISCV_FENCE(iorw,iorw)
+#define smp_mb__after_spinlock()	RISCV_FENCE(iorw, iorw)
 
 #include <asm-generic/barrier.h>