On Tue, Nov 23, 2010 at 01:46:48AM +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 23-11-2010 0:47, Sarah Sharp wrote:
>
> >>>The AMD SB700 I'm testing with wakes up fine on connection after runtime
> >>>suspend, but doesn't appear to set the "Port change detect" bit in USBSTS.
> >>>The change does appear in the individual port register, so try checking
> >>>the port status as well. It's an infrequent enough operation that the
> >>>overhead shouldn't be an issue.
>
> >>>Signed-off-by: Matthew Garrett<mjg@xxxxxxxxxx>
> >>>---
> >>> drivers/usb/host/ehci-hub.c | 17 ++++++++++++++++-
> >>> 1 files changed, 16 insertions(+), 1 deletions(-)
>
> >>>diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> >>>index 796ea0c..7653117 100644
> >>>--- a/drivers/usb/host/ehci-hub.c
> >>>+++ b/drivers/usb/host/ehci-hub.c
> >>>@@ -106,12 +106,25 @@ static void ehci_handover_companion_ports(struct ehci_hcd *ehci)
> >>> ehci->owned_ports = 0;
> >>> }
> >>>
> >>>+static int ehci_port_change(struct ehci_hcd *ehci)
> >>>+{
> >>>+ int i = HCS_N_PORTS(ehci->hcs_params);
> >>>+
> >>>+ while (i--) {
> >>>+ if (ehci_readl(ehci,&ehci->regs->port_status[i]) & PORT_CSC)
> >>>+ return 1;
> >>>+ }
>
> >> {} not needed. scripts/checkpatch.pl doesn't complain?
>
> >No checkpatch.pl doesn't complain.
>
> That's not very consistent of it. It complains (or used to
> complain) in some situations. Maybe it has stopped complaining on
> this matter altogether, I'm not sure. And I couldn't check at the
> time of writing...
I think they changed the checkpatch.pl in the last couple of releases.
> >I also find the braces more readable
> >when there's multiple indentation levels, or when an if conditional
> >wraps around.
>
> OK, I don't.
And you're welcome to your opinion. But this is a gray area in coding
style, and lots of kernel hackers have differing opinions. So
checkpatch.pl doesn't complain about it. Let the maintainer of the
driver decide if they want to be picky.
> >Please stop giving this feedback about braces to
> >individuals when checkpatch.pl does not complain.
>
> I'll try to check in the future, although it's not as easy as it
> seems -- I usually don't review patches at work where I have the
> environment to run checkpatch.pl in; I don't have Linux at home
> currently.
Perl seems like something you could easily run in Windows. And if you
can't check it against checkpatch.pl, then why not give feedback on the
actual code instead? Feedback on style is always better accepted if
some functional feedback is included along with it. It makes people
think you actually read, understand, and appreciate what the code is
doing.
Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
