From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Subject: Re: [PATCH v2 04/10] usb: dwc3: Add two quirks for Hisilicon Kirin Soc Platform Date: Mon, 18 Feb 2019 14:21:34 +0200 Message-ID: References: <20190218112310.17860-1-chenyu56@huawei.com> <20190218112310.17860-5-chenyu56@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <20190218112310.17860-5-chenyu56@huawei.com> Sender: linux-kernel-owner@vger.kernel.org To: Yu Chen Cc: USB , devicetree , Linux Kernel Mailing List , John Stultz , Suzhuangluan , Kongfei , liuyu712@hisilicon.com, wanghu17@hisilicon.com, butao@hisilicon.com, Yao Chen , fangshengzhou@hisilicon.com, lipengcheng8@huawei.com, songxiaowei , xu yiping , xuyoujun4@huawei.com, yudongbin@hisilicon.com, zangleigang@hisilicon.com, Felipe Balbi , Greg Kroah-Hartman , Binghui Wang List-Id: devicetree@vger.kernel.org On Mon, Feb 18, 2019 at 1:23 PM Yu Chen wrote: > > There are tow quirks for DesignWare USB3 DRD Core of Hisilicon Kirin Soc. > 1)SPLIT_BOUNDARY_DISABLE should be set for Host mode > 2)A GCTL soft reset should be executed when switch mode Thus, it must be two patches, no? > +static void dwc3_gctl_core_soft_reset(struct dwc3 *dwc) > +{ > + u32 reg; > + > + reg = dwc3_readl(dwc->regs, DWC3_GCTL); > + reg |= (DWC3_GCTL_CORESOFTRESET); Redundant parens. > + dwc3_writel(dwc->regs, DWC3_GCTL, reg); > + > + reg = dwc3_readl(dwc->regs, DWC3_GCTL); > + reg &= ~(DWC3_GCTL_CORESOFTRESET); Ditto. > + dwc3_writel(dwc->regs, DWC3_GCTL, reg); > +} > +static void dwc3_complete(struct device *dev) > +{ > + struct dwc3 *dwc = dev_get_drvdata(dev); > + u32 reg; Indentation style is different to the other functions in the same patch. Use simple space here. > + if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST && > + dwc->dis_split_quirk) { > + dev_dbg(dwc->dev, "set DWC3_GUCTL3_SPLITDISABLE\n"); > + reg = dwc3_readl(dwc->regs, DWC3_GUCTL3); > + reg |= DWC3_GUCTL3_SPLITDISABLE; > + dwc3_writel(dwc->regs, DWC3_GUCTL3, reg); > + } > +} > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -269,7 +269,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned cmd, > { > const struct usb_endpoint_descriptor *desc = dep->endpoint.desc; > struct dwc3 *dwc = dep->dwc; > - u32 timeout = 1000; > + u32 timeout = 5000; I don't see anything about this change in commit message. Moreoever, it looks like it must be a separate logical change with its own description. > u32 saved_config = 0; > u32 reg; -- With Best Regards, Andy Shevchenko