On Wed, May 08, 2019 at 03:26:09AM +0000, Yinbo Zhu wrote: > > > > -----Original Message----- > > From: Alan Stern [mailto:stern@xxxxxxxxxxxxxxxxxxx] > > Sent: 2019年1月28日 23:37 > > To: Yinbo Zhu <yinbo.zhu@xxxxxxx> > > Cc: Xiaobo Xie <xiaobo.xie@xxxxxxx>; Jerry Huang <jerry.huang@xxxxxxx>; > > Ran Wang <ran.wang_1@xxxxxxx>; Greg Kroah-Hartman > > <gregkh@xxxxxxxxxxxxxxxxxxx>; Ramneek Mehresh > > <ramneek.mehresh@xxxxxxxxxxxxx>; Nikhil Badola > > <nikhil.badola@xxxxxxxxxxxxx>; Suresh Gupta <suresh.gupta@xxxxxxxxxxxxx>; > > linux-usb@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Suresh Gupta > > <B42813@xxxxxxxxxxxxx> > > Subject: Re: [PATCH v4 2/5] usb: phy: Workaround for USB erratum-A005728 > > > > On Fri, 25 Jan 2019, Yinbo Zhu wrote: > > > > > From: Suresh Gupta <B42813@xxxxxxxxxxxxx> > > > > > > PHY_CLK_VALID bit for UTMI PHY in USBDR does not set even if PHY is > > > providing valid clock. Workaround for this involves resetting of PHY > > > and check PHY_CLK_VALID bit multiple times. If PHY_CLK_VALID bit is > > > still not set even after 5 retries, it would be safe to deaclare that > > > PHY clock is not available. > > > This erratum is applicable for USBDR less then ver 2.4. > > > > > > Signed-off-by: Suresh Gupta <B42813@xxxxxxxxxxxxx> > > > Signed-off-by: Yinbo Zhu <yinbo.zhu@xxxxxxx> > > > --- > > > Change in v4: > > > Incorrect indentation of the continuation line. > > > replace pr_err with dev_err. > > > > > > drivers/usb/host/ehci-fsl.c | 38 > > +++++++++++++++++++++++++++----------- > > > drivers/usb/host/ehci-fsl.h | 3 +++ > > > 2 files changed, 30 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c > > > index 38674b7..373a816 100644 > > > --- a/drivers/usb/host/ehci-fsl.c > > > +++ b/drivers/usb/host/ehci-fsl.c > > > @@ -183,6 +183,17 @@ static int fsl_ehci_drv_probe(struct platform_device > > *pdev) > > > return retval; > > > } > > > > > > +static bool usb_phy_clk_valid(struct usb_hcd *hcd) { > > > + void __iomem *non_ehci = hcd->regs; > > > + bool ret = true; > > > + > > > + if (!(ioread32be(non_ehci + FSL_SOC_USB_CTRL) & PHY_CLK_VALID)) > > > + ret = false; > > > + > > > + return ret; > > > +} > > > + > > > static int ehci_fsl_setup_phy(struct usb_hcd *hcd, > > > enum fsl_usb2_phy_modes phy_mode, > > > unsigned int port_offset) > > > @@ -226,6 +237,17 @@ static int ehci_fsl_setup_phy(struct usb_hcd *hcd, > > > /* fall through */ > > > case FSL_USB2_PHY_UTMI: > > > case FSL_USB2_PHY_UTMI_DUAL: > > > + /* PHY_CLK_VALID bit is de-featured from all controller > > > + * versions below 2.4 and is to be checked only for > > > + * internal UTMI phy > > > + */ > > > + if (pdata->controller_ver > FSL_USB_VER_2_4 && > > > + pdata->have_sysif_regs && !usb_phy_clk_valid(hcd)) { > > > + dev_err(dev, > > > + "%s: USB PHY clock invalid\n", dev_name(dev)); > > > > This looks silly; it prints the device name twice (once because that's what > > dev_err() does, and then again because you explicitly told it to print the device > > name). > > > > Look at how dev_err() is used in other parts of the driver and do the same thing. > > > > > + return -EINVAL; > > > + } > > > + > > > if (pdata->have_sysif_regs && pdata->controller_ver) { > > > /* controller version 1.6 or above */ > > > tmp = ioread32be(non_ehci + FSL_SOC_USB_CTRL); @@ -249,17 > > +271,11 > > > @@ static int ehci_fsl_setup_phy(struct usb_hcd *hcd, > > > break; > > > } > > > > > > - /* > > > - * check PHY_CLK_VALID to determine phy clock presence before writing > > > - * to portsc > > > - */ > > > - if (pdata->check_phy_clk_valid) { > > > - if (!(ioread32be(non_ehci + FSL_SOC_USB_CTRL) & > > > - PHY_CLK_VALID)) { > > > - dev_warn(hcd->self.controller, > > > - "USB PHY clock invalid\n"); > > > - return -EINVAL; > > > - } > > > + if (pdata->have_sysif_regs && > > > + pdata->controller_ver > FSL_USB_VER_1_6 && > > > + !usb_phy_clk_valid(hcd)) { > > > + dev_warn(hcd->self.controller, "USB PHY clock invalid\n"); > > > > Once again, you have a continuation line that is indented by the same amount as > > the code in the inner block. Please fix this properly. > Hi Alan stern, > > Above code indented is in oder to ensure that every line less than 80 characters, > Otherwise, check-patch tool to check patch that will has error. No, that's is not what Alan is asking you to do here. Please fix this up to match other multi-line if statements. greg k-h
