Hi Ulrich-san, Thank you for the review! > From: Ulrich Hecht, Sent: Monday, July 16, 2018 8:09 PM > > On Tue, Jul 3, 2018 at 12:46 PM, Yoshihiro Shimoda > <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: > > This patch adds a condition check about the PLL acvice of this > > controller. Otherwise, the controller might cause hang when > > any USB clocks are not supplied yet and accesses the xHCI registers. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> > > --- > > drivers/usb/host/xhci-rcar.c | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c > > index 4a22f25..a6e4637 100644 > > --- a/drivers/usb/host/xhci-rcar.c > > +++ b/drivers/usb/host/xhci-rcar.c > > @@ -29,6 +29,7 @@ > > MODULE_FIRMWARE(XHCI_RCAR_FIRMWARE_NAME_V3); > > > > /*** Register Offset ***/ > > +#define RCAR_USB3_AXH_STA 0x104 /* AXI Host Control Status */ > > #define RCAR_USB3_INT_ENA 0x224 /* Interrupt Enable */ > > #define RCAR_USB3_DL_CTRL 0x250 /* FW Download Control & Status */ > > #define RCAR_USB3_FW_DATA0 0x258 /* FW Data0 */ > > @@ -41,6 +42,12 @@ > > #define RCAR_USB3_TX_POL 0xab8 /* USB3.0 TX Polarity */ > > > > /*** Register Settings ***/ > > +/* AXI Host Control Status */ > > +#define RCAR_USB3_AXH_STA_B3_PLL_ACTIVE 0x00010000 > > +#define RCAR_USB3_AXH_STA_B2_PLL_ACTIVE 0x00000001 > > Maybe use BIT() here? I think so. # Since RCAR_USB3_INT_XHC_ENA doesn’t use BIT(), I don't use the macro to the new #define. # But, RCAR_USB3_RX_POL_VAL uses BIT(). However, this patch has already been merged into Greg's repo. So, I'd like to submit an incremental patch for cleaning up the code (almost all #define use BIT()) later. Best regards, Yoshihiro Shimoda > > +#define RCAR_USB3_AXH_STA_PLL_ACTIVE_MASK (RCAR_USB3_AXH_STA_B3_PLL_ACTIVE | \ > > + RCAR_USB3_AXH_STA_B2_PLL_ACTIVE) > > + > > /* Interrupt Enable */ > > #define RCAR_USB3_INT_XHC_ENA 0x00000001 > > #define RCAR_USB3_INT_PME_ENA 0x00000002 > > @@ -200,6 +207,22 @@ static int xhci_rcar_download_firmware(struct usb_hcd *hcd) > > return retval; > > } > > > > +static bool xhci_rcar_wait_for_pll_active(struct usb_hcd *hcd) > > +{ > > + int timeout = 1000; > > + u32 val, mask = RCAR_USB3_AXH_STA_PLL_ACTIVE_MASK; > > + > > + while (timeout > 0) { > > + val = readl(hcd->regs + RCAR_USB3_AXH_STA); > > + if ((val & mask) == mask) > > + return true; > > + udelay(1); > > + timeout--; > > + } > > + > > + return false; > > +} > > + > > /* This function needs to initialize a "phy" of usb before */ > > int xhci_rcar_init_quirk(struct usb_hcd *hcd) > > { > > @@ -220,6 +243,9 @@ int xhci_rcar_init_quirk(struct usb_hcd *hcd) > > xhci_rcar_is_gen3(hcd->self.controller)) > > xhci->quirks |= XHCI_NO_64BIT_SUPPORT; > > > > + if (!xhci_rcar_wait_for_pll_active(hcd)) > > + return -ETIMEDOUT; > > + > > return xhci_rcar_download_firmware(hcd); > > } > > > > -- > > 1.9.1 > > > > Reviewed-by: Ulrich Hecht <ulrich.hecht+renesas@xxxxxxxxx> > > CU > Uli ��.n��������+%������w��{.n�����{���)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥