| Message ID | 20200629021350.21262-1-peng.fan@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/7] usb: ehci-mx6: Add powerup_fixup implementation | expand |
On 6/29/20 4:13 AM, Peng Fan wrote: [...] > +static void ehci_mx6_powerup_fixup(struct ehci_ctrl *ctrl, uint32_t *status_reg, > + uint32_t *reg) > +{ > + u32 result; > + int usec = 2000; > + > + mdelay(50); > + > + do { > + result = ehci_readl(status_reg); > + udelay(5); > + if (!(result & EHCI_PS_PR)) > + break; > + usec--; > + } while (usec > 0); > + > + *reg = ehci_readl(status_reg); > +} Please use wait_for_bit*() . Also, is the 50 mS delay upfront required or can this be wait_for_bit with 60000 uS timeout ?
Hi Peng, Marek > From: Ye Li <ye.li@nxp.com> > > When doing port reset, the PR bit of PORTSC1 will be automatically > cleared by our IP, but standard EHCI needs explicit clear by > software. The EHCI-HCD driver follow the EHCI specification, so after > 50ms wait, it clear the PR bit by writting to the PORTSC1 register > with value loaded before setting PR. > > This sequence is ok for our IP when the delay time is exact. But when > the timer is slower, some bits like PE, PSPD have been set by > controller automatically after the PR is automatically cleared. So > the writing to the PORTSC1 will overwrite these bits set by > controller. And eventually the driver gets wrong status. > > We implement the powerup_fixup operation which delays 50ms and will > check the PR until it is cleared by controller. And will update the > reg value which is written to PORTSC register by EHCI-HCD driver. > This is much safer than depending on the delay time to be accurate > and aligining with controller's behaiver. Is there any plan for a follow up for this patch set? On my imx6q - kp_tpc70 board I can observe that there are some issues when trying to read data from USB [*]: Booting update from usb ... starting USB... Bus usb@2184200: USB EHCI 1.00 scanning bus usb@2184200 for devices... 2 USB Device(s) found scanning usb for storage devices... 1 Storage Device(s) found EHCI timed out on TD - token=0x1f8c80 EHCI timed out on TD - token=0x1f8c80 Peng do you see similar issue when you test it on your iMX8? The test is very simple - just put on a pendrive a fitImage and initramfs and try to boot with it. In my case it will hang at least once per ten attempts. Note: [*] - this is a similar issue to one, which I had on the iMX53 controller. However, Marek's patch fixed it on imx53: "usb: Keep async schedule running only across mass storage xfers" SHA1: 31232de07ef2bd97ff67625976eecd97eeb1b > > Signed-off-by: Ye Li <ye.li@nxp.com> > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/usb/host/ehci-mx6.c | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > index 5f84c7b91d..e654595683 100644 > --- a/drivers/usb/host/ehci-mx6.c > +++ b/drivers/usb/host/ehci-mx6.c > @@ -267,6 +267,25 @@ int usb_phy_mode(int port) > } > #endif > > +static void ehci_mx6_powerup_fixup(struct ehci_ctrl *ctrl, uint32_t > *status_reg, > + uint32_t *reg) > +{ > + u32 result; > + int usec = 2000; > + > + mdelay(50); > + > + do { > + result = ehci_readl(status_reg); > + udelay(5); > + if (!(result & EHCI_PS_PR)) > + break; > + usec--; > + } while (usec > 0); > + > + *reg = ehci_readl(status_reg); > +} > + > static void usb_oc_config(int index) > { > #if defined(CONFIG_MX6) > @@ -366,6 +385,10 @@ int ehci_mx6_common_init(struct usb_ehci *ehci, > int index) } > > #if !CONFIG_IS_ENABLED(DM_USB) > +static const struct ehci_ops mx6_ehci_ops = { > + .powerup_fixup = ehci_mx6_powerup_fixup, > +}; > + > int ehci_hcd_init(int index, enum usb_init_type init, > struct ehci_hccr **hccr, struct ehci_hcor **hcor) > { > @@ -394,6 +417,8 @@ int ehci_hcd_init(int index, enum usb_init_type > init, if (ret) > return ret; > > + ehci_set_controller_priv(index, NULL, &mx6_ehci_ops); > + > type = board_usb_phy_mode(index); > > if (hccr && hcor) { > @@ -467,7 +492,8 @@ static int mx6_init_after_reset(struct ehci_ctrl > *dev) } > > static const struct ehci_ops mx6_ehci_ops = { > - .init_after_reset = mx6_init_after_reset > + .powerup_fixup = ehci_mx6_powerup_fixup, > + .init_after_reset = mx6_init_after_reset > }; > > static int ehci_usb_phy_mode(struct udevice *dev) Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index 5f84c7b91d..e654595683 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -267,6 +267,25 @@ int usb_phy_mode(int port) } #endif +static void ehci_mx6_powerup_fixup(struct ehci_ctrl *ctrl, uint32_t *status_reg, + uint32_t *reg) +{ + u32 result; + int usec = 2000; + + mdelay(50); + + do { + result = ehci_readl(status_reg); + udelay(5); + if (!(result & EHCI_PS_PR)) + break; + usec--; + } while (usec > 0); + + *reg = ehci_readl(status_reg); +} + static void usb_oc_config(int index) { #if defined(CONFIG_MX6) @@ -366,6 +385,10 @@ int ehci_mx6_common_init(struct usb_ehci *ehci, int index) } #if !CONFIG_IS_ENABLED(DM_USB) +static const struct ehci_ops mx6_ehci_ops = { + .powerup_fixup = ehci_mx6_powerup_fixup, +}; + int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr, struct ehci_hcor **hcor) { @@ -394,6 +417,8 @@ int ehci_hcd_init(int index, enum usb_init_type init, if (ret) return ret; + ehci_set_controller_priv(index, NULL, &mx6_ehci_ops); + type = board_usb_phy_mode(index); if (hccr && hcor) { @@ -467,7 +492,8 @@ static int mx6_init_after_reset(struct ehci_ctrl *dev) } static const struct ehci_ops mx6_ehci_ops = { - .init_after_reset = mx6_init_after_reset + .powerup_fixup = ehci_mx6_powerup_fixup, + .init_after_reset = mx6_init_after_reset }; static int ehci_usb_phy_mode(struct udevice *dev)