On Wed, Jun 13, 2012 at 12:30:38PM -0700, Greg KH wrote: > On Mon, Jun 11, 2012 at 10:24:33AM +0800, Lan Tianyu wrote: > > + > > + if (usb_acpi_power_manageable(hcd->self.root_hub, > > + wIndex + 1)) > > Why +1? If you have to do this everywhere, then do it only in the > function, so you can be 0 based properly. > > Also, minor coding style nit, please rewrite as: > if (usb_acpi_power_manageable(hcd->self.root_hub, > wIndex + 1)) This is an arbitrarily applied rule, and I don't follow it in the xHCI driver. The code indentation should be left at the default indentation in the xHCI driver. Let's keep the code style in the driver consistent, please. I don't understand why people want to move the trailing arguments to align with the function parenthesis. I can see that it might add to readability for some people, but it's just more work on both the developer's and maintainer's side. Checkpatch will complain about mixed tabs and spaces, so that's more work for me to edit my git pre-commit hook when the checkpatch error stops git-am. Plus the developer has to actually pause and think about indentation, rather than letting their editor take care of it. I'm not even sure when you would want the alignment rule applied. In all function calls with trailing arguments? That seems really excessive. > Or even better yet, use a temp variable for the value returned and then > check that, it's clearer and easier to read, right? A temp variable would be fine until that function can just be refactored. The case statements should really just be broken out into separate functions. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
