Skip to contentSkip to conversation

Staging: octeon-usb: fixed a macro coding style issue

PATCH In review Nicolas Del Piano 3 participants 3 messages Apr 25, 2014 Staging

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Staging: octeon-usb: fixed a macro coding style issue

Fixed a coding style error, macros with complex values should be enclosed in parentheses.

---
 drivers/staging/octeon-usb/octeon-hcd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/octeon-usb/octeon-hcd.c +1 -1
index 8b8ce72..ef3a8ce 100644
@@ -246,7 +246,7 @@ enum cvmx_usb_pipe_flags {
 };
 
 /* Normal prefetch that use the pref instruction. */
-#define CVMX_PREFETCH(address, offset) asm volatile ("pref %[type], %[off](%[rbase])" : : [rbase] "d" (address), [off] "I" (offset), [type] "n" (0))
+#define CVMX_PREFETCH(address, offset) (asm volatile ("pref %[type], %[off](%[rbase])" : : [rbase] "d" (address), [off] "I" (offset), [type] "n" (0)))
 
 /* Maximum number of times to retry failed transactions */
 #define MAX_RETRIES		3

--
1.7.9.5

maintainer
#1 lore raw
Quoted 12 lines · Nicolas Del Piano

Fixed a coding style error, macros with complex values should be enclosed in parentheses.

---
 drivers/staging/octeon-usb/octeon-hcd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
index 8b8ce72..ef3a8ce 100644

drivers/staging/octeon-usb/octeon-hcd.c enum cvmx_usb_pipe_flags { · line 252

Quoted 8 lines
drivers/staging/octeon-usb/octeon-hcd.c +1 -1
@@ -246,7 +246,7 @@ enum cvmx_usb_pipe_flags {
 };
 
 /* Normal prefetch that use the pref instruction. */
-#define CVMX_PREFETCH(address, offset) asm volatile ("pref %[type], %[off](%[rbase])" : : [rbase] "d" (address), [off] "I" (offset), [type] "n" (0))
+#define CVMX_PREFETCH(address, offset) (asm volatile ("pref %[type], %[off](%[rbase])" : : [rbase] "d" (address), [off] "I" (offset), [type] "n" (0)))
 
 /* Maximum number of times to retry failed transactions */
 #define MAX_RETRIES		3

Will that actually compile?

Why the heck are we using asm in a define in the first place? Shouldn't this be an inline function or something?

thanks,

greg k-h

#2 lore raw
Quoted 15 lines · Greg KH
Quoted 14 lines · Nicolas Del Piano

Fixed a coding style error, macros with complex values should be enclosed in parentheses.

---
  drivers/staging/octeon-usb/octeon-hcd.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
index 8b8ce72..ef3a8ce 100644
@@ -246,7 +246,7 @@ enum cvmx_usb_pipe_flags {
  };
 
Quoted 8 lines

drivers/staging/octeon-usb/octeon-hcd.c enum cvmx_usb_pipe_flags { · line 252

Quoted 6 lines
drivers/staging/octeon-usb/octeon-hcd.c +1 -1
  /* Normal prefetch that use the pref instruction. */
-#define CVMX_PREFETCH(address, offset) asm volatile ("pref %[type], %[off](%[rbase])" : : [rbase] "d" (address), [off] "I" (offset), [type] "n" (0))
+#define CVMX_PREFETCH(address, offset) (asm volatile ("pref %[type], %[off](%[rbase])" : : [rbase] "d" (address), [off] "I" (offset), [type] "n" (0)))
 
  /* Maximum number of times to retry failed transactions */
  #define MAX_RETRIES		3

Will that actually compile?

No it does not. Really when changing things like this, you should first understand what it is doing. In the case of this patch, I don't think that understanding was present.

It appears that in the case of this patch, Documentation/SubmitChecklist was not consulted. In particular item #2(all sections) was omitted.

Q: Why did I have to waste 15 minutes of my day on this? Is the patch subimtter's time so valuable that standard patch submittal procedures cannot be followed?

Quoted 3 lines

Why the heck are we using asm in a define in the first place? Shouldn't this be an inline function or something?

You are correct. It turns out that <linux/prefetch.h> already has such a definition.

Quoted 5 lines

thanks,

greg k-h