Great to known. Sorry about not reading the code, will check more carefully before I fix it. Regards Nick On 14-10-11 11:25 AM, karthik nayak wrote: > Hey Nick, > Nice try to fix a checkpatch warning. But do read what you're changing. > Yes your format is right. If you haven't already, take a look at "git > send-email" . > Have fun hacking :D > Regards, > Karthik Nayak > > > On Sat, Oct 11, 2014 at 7:47 PM, nick <xerofoify@xxxxxxxxx> wrote: >> Thanks Hugo, >> Sorry about that. On the other hand was the patch good in terms of format? >> Cheers Nick >> >> On 14-10-11 09:52 AM, Hugo Mills wrote: >>> On Sat, Oct 11, 2014 at 09:44:05AM -0400, nick wrote: >>>> Thank you for your help, I'll study the code and see what I can do >>>> about it. Do you have any suggestions of how to fix this checkpatch >>>> warning? >>> >>> Ignore it. The checker has clearly triggered on a false positive -- >>> this is not a function call, and should not be held to that standard. >>> (Take a look at where the macro is actually used, to see what's going >>> on here). Move on to find something more interesting to fix. >>> >>> Hugo. >>> >>>> Nick >>>> >>>> On 14-10-11 05:53 AM, Kristofer Hallin wrote: >>>>> Even if you use checkpath you _should_ understand what you are changing. >>>>> The output of checkpatch merely there to help. >>>>> >>>>> In this case you can see that this is a macro just a few lines up in the >>>>> code. >>>>> On 11 Oct 2014 11:46, "Sudip Mukherjee" <sudipm.mukherjee@xxxxxxxxx> wrote: >>>>> >>>>>> I agree. But in my opinion checkpatch is here to help us fix style >>>>>> problems , but we should not blindly act on checkpatch warnings. >>>>>> >>>>>> thanks >>>>>> sudip >>>>>> >>>>>> On Sat, Oct 11, 2014 at 1:57 PM, Peter Senna Tschudin >>>>>> <peter.senna@xxxxxxxxx> wrote: >>>>>>> I think that, in this case, checkpatch.pl contributed: >>>>>>> >>>>>>> $ ./scripts/checkpatch.pl -f drivers/staging/octeon-usb/octeon-hcd.c >>>>>>> WARNING: space prohibited between function name and open parenthesis '(' >>>>>>> #415: FILE: drivers/staging/octeon-usb/octeon-hcd.c:415: >>>>>>> + if (c.s.field op (value)) { >>>>>> \ >>>>>>> >>>>>>> >>>>>>> On Sat, Oct 11, 2014 at 8:11 AM, Dave Tian <dave.jing.tian@xxxxxxxxx> >>>>>> wrote: >>>>>>>> Agreed - that is why I mentioned the patch is neither right nor useful:) >>>>>>>> >>>>>>>> -daveti >>>>>>>> >>>>>>>> >>>>>>>> On Oct 11, 2014, at 2:08 PM, Sudip Mukherjee < >>>>>> sudipm.mukherjee@xxxxxxxxx> wrote: >>>>>>>> >>>>>>>>> Hi Dave, >>>>>>>>> It will work. But my point of saying that was c.s.field ==(value) is >>>>>>>>> again not according to the style. >>>>>>>>> >>>>>>>>> thanks >>>>>>>>> sudip >>>>>>>>> >>>>>>>>> On Sat, Oct 11, 2014 at 10:53 AM, Dave Tian <dave.jing.tian@xxxxxxxxx> >>>>>> wrote: >>>>>>>>>> It also works as value is surrounded by (), though I do not think the >>>>>> patch itself is right or useful. >>>>>>>>>> >>>>>>>>>> Dave Tian >>>>>>>>>> dave.jing.tian@xxxxxxxxx >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Oct 11, 2014, at 12:58 PM, Sudip Mukherjee < >>>>>> sudipm.mukherjee@xxxxxxxxx> wrote: >>>>>>>>>> >>>>>>>>>>> On Fri, Oct 10, 2014 at 09:55:48PM -0400, Nicholas Krause wrote: >>>>>>>>>>>> Fixes checkpatch coding style warning about unneeded space >>>>>>>>>>>> between function name an parentheses. >>>>>>>>>>>> >>>>>>>>>>>> Signed-off-by: Nicholas Krause <xerofoify@xxxxxxxxx> >>>>>>>>>>>> --- >>>>>>>>>>>> Untested >>>>>>>>>>>> drivers/staging/octeon-usb/octeon-hcd.c | 2 +- >>>>>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>>>>>> >>>>>>>>>>>> diff --git a/drivers/staging/octeon-usb/octeon-hcd.c >>>>>> b/drivers/staging/octeon-usb/octeon-hcd.c >>>>>>>>>>>> index 5f9db4c..bbeb0cc 100644 >>>>>>>>>>>> --- a/drivers/staging/octeon-usb/octeon-hcd.c >>>>>>>>>>>> +++ b/drivers/staging/octeon-usb/octeon-hcd.c >>>>>>>>>>>> @@ -412,7 +412,7 @@ struct octeon_hcd { >>>>>>>>>>>> type c; >>>>>> \ >>>>>>>>>>>> while (1) { >>>>>> \ >>>>>>>>>>>> c.u32 = __cvmx_usb_read_csr32(usb, address); >>>>>> \ >>>>>>>>>>>> - if (c.s.field op (value)) { >>>>>> \ >>>>>>>>>>>> + if (c.s.field op(value)) { >>>>>> \ >>>>>>>>>>> >>>>>>>>>>> have you read the code before modifying it? >>>>>>>>>>> this is not a function. >>>>>>>>>>> have you seen how CVMX_WAIT_FOR_FIELD32 is being called? >>>>>>>>>>> on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "==" >>>>>>>>>>> so when called the macro will be c.s.field == (value). >>>>>>>>>>> if your patch is applied then it will become c.s.field ==(value) .. >>>>>> will that be correct ? >>>>>>>>>>> >>>>>>>>>>> thanks >>>>>>>>>>> sudip >>>>>>>>>>> >>>>>>>>>>>> result = 0; >>>>>> \ >>>>>>>>>>>> break; >>>>>> \ >>>>>>>>>>>> } else if (cvmx_get_cycle() > done) { >>>>>> \ >>> >> >> _______________________________________________ >> Kernelnewbies mailing list >> Kernelnewbies@xxxxxxxxxxxxxxxxx >> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies _______________________________________________ Kernelnewbies mailing list Kernelnewbies@xxxxxxxxxxxxxxxxx http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies