On Sat, Oct 27, 2012 at 11:16:54PM +0100, Ceri James wrote:
> >>@@ -985,11 +992,13 @@ cntrlEnd:
> >>            if (uiData) {
> >>                    /* Allow All Packets */
> >>-                   BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, 
> >>DBG_LVL_ALL, "IOCTL_BCM_SWITCH_TRANSFER_MODE: ETH_PACKET_TUNNELING_MODE\n");
> >>-                           Adapter->TransferMode = 
> >>ETH_PACKET_TUNNELING_MODE;
> >>+                   BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, 
> >>DBG_LVL_ALL,
> >>+                                   "IOCTL_BCM_SWITCH_TRANSFER_MODE: 
> >>ETH_PACKET_TUNNELING_MODE\n");
> >>+                                   Adapter->TransferMode = 
> >>ETH_PACKET_TUNNELING_MODE;
> >                                         
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >This is indented too far now.  It's better to wait over night and
> >re-review your changes the next morning before resending.
> This is not a hasty mistake, it is not knowing the correct/expected
> indentation rules :)
> So the correct indentation is as below?
> >+                    BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, 
> >DBG_LVL_ALL,
> >+                            "IOCTL_BCM_SWITCH_TRANSFER_MODE: 
> >ETH_PACKET_TUNNELING_MODE\n");
> >+                            Adapter->TransferMode = 
> >ETH_PACKET_TUNNELING_MODE;

No.  It's a new statement.  It should be:
                        BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, 
DBG_LVL_ALL,
                                "IOCTL_BCM_SWITCH_TRANSFER_MODE: 
ETH_PACKET_TUNNELING_MODE\n");
                        Adapter->TransferMode = ETH_PACKET_TUNNELING_MODE;
> >>@@ -1397,7 +1423,9 @@ cntrlEnd:
> >>            }
> >>            do_gettimeofday(&tv1);
> >>-           BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, 
> >>DBG_LVL_ALL, " timetaken by Write/read :%ld msec\n", (tv1.tv_sec - 
> >>tv0.tv_sec)*1000 + (tv1.tv_usec - tv0.tv_usec)/1000);
> >>+           BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL,
> >>+                           " timetaken by Write/read :%ld msec\n",
> >>+                           (tv1.tv_sec - tv0.tv_sec)*1000 + (tv1.tv_usec - 
> >>tv0.tv_usec)/1000);
> >We would normally put spaces around the multiply and divide as well.
> >
> >                             (tv1.tv_sec - tv0.tv_sec) * 1000 + (tv1.tv_usec 
> > - tv0.tv_usec) / 1000);
> Fair enough, I may have missed this in the checkpatch output. I will
> double check.

Checkpatch.pl doesn't warn about these.  Normally I would ignore
them if you didn't introduce it, but since I was already commenting
on stuff I figured I may as well mention it.

regards,
dan carpenter

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

Reply via email to