| Message ID | 1434278413-21157-2-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Hans de Goede |
| Headers | show |
On Sun, 2015-06-14 at 12:40 +0200, Hans de Goede wrote: > Fully resetting the controller is a too big hammer, and the musb_core will > then afterwards fail to communicate with any endpoints other then 0 as > too much state was cleared. > > Instead report vbus low for 200ms which will effectively end the current > session without needing to do a full reset. > > This fixes usb mass-storage devices no longer working after a "usb reset" > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Ian Campbell <ijc@hellion.org.uk> One question (which turned into two) (both more for the musb maint than you): > +/* musb_core does not call enable / disable in a balanced manner <sigh> */ > +static bool enabled = false; Is this sufficient, or should we be reference counting? Or should the core be fixed? Ian.
Hi, On 06/14/2015 01:46 PM, Ian Campbell wrote: > On Sun, 2015-06-14 at 12:40 +0200, Hans de Goede wrote: >> Fully resetting the controller is a too big hammer, and the musb_core will >> then afterwards fail to communicate with any endpoints other then 0 as >> too much state was cleared. >> >> Instead report vbus low for 200ms which will effectively end the current >> session without needing to do a full reset. >> >> This fixes usb mass-storage devices no longer working after a "usb reset" >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Acked-by: Ian Campbell <ijc@hellion.org.uk> > > One question (which turned into two) (both more for the musb maint than > you): > >> +/* musb_core does not call enable / disable in a balanced manner <sigh> */ >> +static bool enabled = false; > > Is this sufficient, or should we be reference counting? Or should the > core be fixed? The u-boot musb-new code is a copy of the kernel code, and both are of, ah, interesting quality. One problem is that the core calls musb_platform_disable as part of musb_init_controller and thus before musb_platform_enable is ever called. Ideally the core would not do this, but in u-boot it is used on 4 different SoCs and in the kernel even more, and chances are that that call was added to put the hw in a clean state on some SoC... Basically my approach for these kind of problems in the musb core so far has been to work around them were possible, so as to not cause any regressions on other SoCs. Regards, Hans
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index e8a3a23..42c6725 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -157,6 +157,17 @@ static void USBC_ForceIdToHigh(__iomem void *base) musb_writel(base, USBC_REG_o_ISCR, reg_val); } +static void USBC_ForceVbusValidToLow(__iomem void *base) +{ + u32 reg_val; + + reg_val = musb_readl(base, USBC_REG_o_ISCR); + reg_val &= ~(0x03 << USBC_BP_ISCR_FORCE_VBUS_VALID); + reg_val |= (0x02 << USBC_BP_ISCR_FORCE_VBUS_VALID); + reg_val = USBC_WakeUp_ClearChangeDetect(reg_val); + musb_writel(base, USBC_REG_o_ISCR, reg_val); +} + static void USBC_ForceVbusValidToHigh(__iomem void *base) { u32 reg_val; @@ -205,42 +216,41 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci) return retval; } +/* musb_core does not call enable / disable in a balanced manner <sigh> */ +static bool enabled = false; + static void sunxi_musb_enable(struct musb *musb) { pr_debug("%s():\n", __func__); + if (enabled) + return; + /* select PIO mode */ musb_writeb(musb->mregs, USBC_REG_o_VEND0, 0); - if (is_host_enabled(musb)) { - /* port power on */ - sunxi_usb_phy_power_on(0); - } + if (is_host_enabled(musb)) + sunxi_usb_phy_power_on(0); /* port power on */ + + USBC_ForceVbusValidToHigh(musb->mregs); + + enabled = true; } static void sunxi_musb_disable(struct musb *musb) { - struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; - pr_debug("%s():\n", __func__); - /* Put the controller back in a pristane state for "usb reset" */ - if (musb->is_active) { - sunxi_usb_phy_exit(0); -#ifdef CONFIG_SUNXI_GEN_SUN6I - clrbits_le32(&ccm->ahb_reset0_cfg, 1 << AHB_GATE_OFFSET_USB0); -#endif - clrbits_le32(&ccm->ahb_gate0, 1 << AHB_GATE_OFFSET_USB0); + if (!enabled) + return; - mdelay(10); + if (is_host_enabled(musb)) + sunxi_usb_phy_power_off(0); /* port power off */ - setbits_le32(&ccm->ahb_gate0, 1 << AHB_GATE_OFFSET_USB0); -#ifdef CONFIG_SUNXI_GEN_SUN6I - setbits_le32(&ccm->ahb_reset0_cfg, 1 << AHB_GATE_OFFSET_USB0); -#endif - sunxi_usb_phy_init(0); - musb->is_active = 0; - } + USBC_ForceVbusValidToLow(musb->mregs); + mdelay(200); /* Wait for the current session to timeout */ + + enabled = false; } static int sunxi_musb_init(struct musb *musb)
Fully resetting the controller is a too big hammer, and the musb_core will then afterwards fail to communicate with any endpoints other then 0 as too much state was cleared. Instead report vbus low for 200ms which will effectively end the current session without needing to do a full reset. This fixes usb mass-storage devices no longer working after a "usb reset" Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/usb/musb-new/sunxi.c | 52 ++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 21 deletions(-)