Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752019AbbBROCh (ORCPT ); Wed, 18 Feb 2015 09:02:37 -0500 Received: from mail-qg0-f41.google.com ([209.85.192.41]:33459 "EHLO mail-qg0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751871AbbBROCd (ORCPT ); Wed, 18 Feb 2015 09:02:33 -0500 MIME-Version: 1.0 In-Reply-To: <54E451B1.8020401@intel.com> References: <1424151697-2084-1-git-send-email-Sneeker.Yeh@tw.fujitsu.com> <1424151697-2084-2-git-send-email-Sneeker.Yeh@tw.fujitsu.com> <54E451B1.8020401@intel.com> Date: Wed, 18 Feb 2015 22:02:32 +0800 Message-ID: Subject: Re: [PATCH v4 1/5] xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core From: Sneeker Yeh To: Mathias Nyman Cc: Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Felipe Balbi , Greg Kroah-Hartman , Grant Likely , Huang Rui , Kishon Vijay Abraham I , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-usb@vger.kernel.org" , "linux-omap@vger.kernel.org" , Andy Green , Jassi Brar , Sneeker Yeh Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2315 Lines: 78 hi Mathias, thanks for reviewing these, 2015-02-18 16:47 GMT+08:00 Mathias Nyman : > Hi > > This looks correct, but if you are still going to make a new series fixing > Felipe's comments then the following tiny nitpicks could be fixed as well. > > Otherwise > > Acked-by: Mathias Nyman > > Felipe, Do you want to take this series through your tree? > > On 17.02.2015 07:41, Sneeker Yeh wrote: >> >> +static void xhci_try_to_clear_csc(struct usb_hcd *hcd, int dev_port_num) > > Either add a function description explaining something like "Late clearing of connect status. > Some quirky hardware will suspend the controller when CSC bit is cleared and leave URBs unhandled" > > Or change the function name to something like xhci_late_csc_clear_quirk() or xhci_deferred_csc_clear_quirk() > > Maybe the name should be changed anyways. > The "try to" makes it look like some non-blocking version of a csc clear function. hm...thanks...it totally make sense, i'd like to use xhci_deferred_csc_clear_quirk(), and will take it to my next patches. > >> +{ >> + int max_ports; >> + struct xhci_hcd *xhci = hcd_to_xhci(hcd); >> + __le32 __iomem **port_array; >> + u32 status; >> + >> + /* print debug info */ > > Remove this comment okay, i see. > >> + if (hcd->speed == HCD_USB3) { >> + max_ports = xhci->num_usb3_ports; >> + port_array = xhci->usb3_ports; >> + } else { >> + max_ports = xhci->num_usb2_ports; >> + port_array = xhci->usb2_ports; >> + } >> + >> + if (dev_port_num > max_ports) { >> + xhci_err(xhci, "%s() port number invalid", __func__); >> + return; >> + } >> + status = readl(port_array[dev_port_num - 1]); >> + >> + /* write 1 to clear */ > > Not really a helpful comment either, either remove or change to something like > "clearing the connect status bit will now immediately suspend these quirky controllers" okay, i got it , thanks. Much appreciate, Sneeker > > -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/