[PATCH v2] usb: load usb phy earlier
USB PHY works proper is the base for the coming USB controller operation. With this patch, it can avoid the controller drivers which are linked earlier than USB PHY always being probed deferral. Look at drivers/Makefile, it links phy first with the similar method. Signed-off-by: Zhangfei Gao Acked-by: Peter Chen --- drivers/usb/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile index 2f1e2aa..d8926c6 100644 --- a/drivers/usb/Makefile +++ b/drivers/usb/Makefile @@ -5,6 +5,7 @@ # Object files in subdirectories obj-$(CONFIG_USB) += core/ +obj-$(CONFIG_USB_SUPPORT) += phy/ obj-$(CONFIG_USB_DWC3) += dwc3/ obj-$(CONFIG_USB_DWC2) += dwc2/ @@ -48,7 +49,6 @@ obj-$(CONFIG_USB_MICROTEK)+= image/ obj-$(CONFIG_USB_SERIAL) += serial/ obj-$(CONFIG_USB) += misc/ -obj-$(CONFIG_USB_SUPPORT) += phy/ obj-$(CONFIG_EARLY_PRINTK_DBGP)+= early/ obj-$(CONFIG_USB_ATM) += atm/ -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net] r8152: restore hw settings
There is a capability which let the hw could change the settings
automatically when the power change to ON. However, the USB reset
would reset the settings to the hw default, so the driver has to
restore the relative settings. Otherwise, it would influence the
functions of the hw, and the compatibility for the USB hub and
USB host controller.
The relative settings are as following.
- set the power down scale to 96.
- enable the power saving function of USB 2.0.
- disable the ALDPS of ECM mode.
- set burst mode depending on the burst size.
- enable the flow control of endpoint full.
- set fifo empty boundary to 32448 bytes.
- enable the function of exiting LPM when Rx OK occurs.
- set the connect timer to 1.
Signed-off-by: Hayes Wang
---
drivers/net/usb/r8152.c | 59 +++--
1 file changed, 57 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 5980ac6..438fc6b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -40,6 +40,7 @@
#define PLA_RXFIFO_CTRL0 0xc0a0
#define PLA_RXFIFO_CTRL1 0xc0a4
#define PLA_RXFIFO_CTRL2 0xc0a8
+#define PLA_DMY_REG0 0xc0b0
#define PLA_FMC0xc0b4
#define PLA_CFG_WOL0xc0b6
#define PLA_TEREDO_CFG 0xc0bc
@@ -90,8 +91,14 @@
#define PLA_BP_7 0xfc36
#define PLA_BP_EN 0xfc38
+#define USB_USB2PHY0xb41e
+#define USB_SSPHYLINK2 0xb428
#define USB_U2P3_CTRL 0xb460
+#define USB_CSR_DUMMY1 0xb464
+#define USB_CSR_DUMMY2 0xb466
#define USB_DEV_STAT 0xb808
+#define USB_CONNECT_TIMER 0xcbf8
+#define USB_BURST_SIZE 0xcfc0
#define USB_USB_CTRL 0xd406
#define USB_PHY_CTRL 0xd408
#define USB_TX_AGG 0xd40a
@@ -170,6 +177,9 @@
#define TXFIFO_THR_NORMAL 0x0048
#define TXFIFO_THR_NORMAL2 0x0108
+/* PLA_DMY_REG0 */
+#define ECM_ALDPS 0x0002
+
/* PLA_FMC */
#define FMC_FCR_MCU_EN 0x0001
@@ -289,6 +299,20 @@
/* PLA_BOOT_CTRL */
#define AUTOLOAD_DONE 0x0002
+/* USB_USB2PHY */
+#define USB2PHY_SUSPEND0x0001
+#define USB2PHY_L1 0x0002
+
+/* USB_SSPHYLINK2 */
+#define pwd_dn_scale_mask 0x3ffe
+#define pwd_dn_scale(x)((x) << 1)
+
+/* USB_CSR_DUMMY1 */
+#define DYNAMIC_BURST 0x0001
+
+/* USB_CSR_DUMMY2 */
+#define EP4_FULL_FC0x0001
+
/* USB_DEV_STAT */
#define STAT_SPEED_MASK0x0006
#define STAT_SPEED_HIGH0x
@@ -334,9 +358,13 @@
#define TIMER11_EN 0x0001
/* USB_LPM_CTRL */
+/* bit 4 ~ 5: fifo empty boundary */
+#define FIFO_EMPTY_1FB 0x30/* 0x1fb * 64 = 32448 bytes */
+/* bit 2 ~ 3: LMP timer */
#define LPM_TIMER_MASK 0x0c
#define LPM_TIMER_500MS0x04/* 500 ms */
#define LPM_TIMER_500US0x0c/* 500 us */
+#define ROK_EXIT_LPM 0x02
/* USB_AFE_CTRL2 */
#define SEN_VAL_MASK 0xf800
@@ -3230,6 +3258,32 @@ static void r8153_init(struct r8152 *tp)
r8153_u2p3en(tp, false);
+ if (tp->version == RTL_VER_04) {
+ ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_SSPHYLINK2);
+ ocp_data &= ~pwd_dn_scale_mask;
+ ocp_data |= pwd_dn_scale(96);
+ ocp_write_word(tp, MCU_TYPE_USB, USB_SSPHYLINK2, ocp_data);
+
+ ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, USB_USB2PHY);
+ ocp_data |= USB2PHY_L1 | USB2PHY_SUSPEND;
+ ocp_write_byte(tp, MCU_TYPE_USB, USB_USB2PHY, ocp_data);
+ } else if (tp->version == RTL_VER_05) {
+ ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, PLA_DMY_REG0);
+ ocp_data &= ~ECM_ALDPS;
+ ocp_write_byte(tp, MCU_TYPE_PLA, PLA_DMY_REG0, ocp_data);
+
+ ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, USB_CSR_DUMMY1);
+ if (ocp_read_word(tp, MCU_TYPE_USB, USB_BURST_SIZE) == 0)
+ ocp_data &= ~DYNAMIC_BURST;
+ else
+ ocp_data |= DYNAMIC_BURST;
+ ocp_write_byte(tp, MCU_TYPE_USB, USB_CSR_DUMMY1, ocp_data);
+ }
+
+ ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, USB_CSR_DUMMY2);
+ ocp_data |= EP4_FULL_FC;
+ ocp_write_byte(tp, MCU_TYPE_USB, USB_CSR_DUMMY2, ocp_data);
+
ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_WDT11_CTRL);
ocp_data &= ~TIMER11_EN;
ocp_write_word(tp, MCU_TYPE_USB, USB_WDT11_CTRL, ocp_data);
@@ -3238,8 +3292,7 @@ static void r8153_init(struct r8152 *tp)
ocp_data &= ~LED_MODE_MASK;
ocp_write_word(tp, MCU_TYPE_PLA, PLA_LED_FEATURE, ocp_data);
- ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, USB_LPM_CTRL);
- ocp_data &= ~LPM_TIMER_MASK;
+ ocp_data = FIFO_EMPTY_1FB | ROK_EXIT_LPM;
if (tp->version ==
RE: [PATCH 2/2] usb: dwc2: gadget reuse ahbcfg assigned from platform
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: Wednesday, February 11, 2015 11:00 PM
> To: Zhangfei Gao; Kaukab, Yousaf
> Cc: ba...@ti.com; john.y...@synopsys.com; linux-usb@vger.kernel.org
> Subject: Re: [PATCH 2/2] usb: dwc2: gadget reuse ahbcfg assigned from
> platform
>
> On 2/6/2015 2:23 PM, John Youn wrote:
> > On 2/6/2015 6:02 AM, Zhangfei Gao wrote:
> >> On 6 February 2015 at 16:07, Kaukab, Yousaf
> wrote:
> >> GAHBCFG_HBSTLEN_INCR4 << diff --git a/drivers/usb/dwc2/gadget.c
> >> b/drivers/usb/dwc2/gadget.c index 15aa578..20085de 100644
> >> --- a/drivers/usb/dwc2/gadget.c
> >> +++ b/drivers/usb/dwc2/gadget.c
> >> @@ -2314,9 +2314,13 @@ void
> >> s3c_hsotg_core_init_disconnected(struct
> >> dwc2_hsotg *hsotg,
> >> GINTSTS_USBSUSP | GINTSTS_WKUPINT,
> >> hsotg->regs + GINTMSK);
> >>
> >> + if ((hsotg->core_params) && (hsotg->core_params->ahbcfg !=
> >> + -
> >> 1))
> >> + val = hsotg->core_params->ahbcfg &
> >> ~GAHBCFG_CTRL_MASK;
> >> + else
> >> + val = GAHBCFG_HBSTLEN_INCR4 <<
> >> GAHBCFG_HBSTLEN_SHIFT;
> >> +
> >> if (using_dma(hsotg))
> >> - writel(GAHBCFG_GLBL_INTR_EN |
> >> GAHBCFG_DMA_EN |
> >> -(GAHBCFG_HBSTLEN_INCR4 <<
> >> GAHBCFG_HBSTLEN_SHIFT),
> >> + writel(GAHBCFG_GLBL_INTR_EN |
> >> GAHBCFG_DMA_EN | val,
> >> hsotg->regs + GAHBCFG);
> >> else
> >> writel(((hsotg->dedicated_fifos) ?
> >> (GAHBCFG_NP_TXF_EMP_LVL |
> >
> > There are other bits in GAHBCFG that can be set from platform.
> > They will be
> preserved by your patch, as they are not part of GAHBCFG_CTRL_MASK,
> but only in case dma is enabled. Perhaps preserve them in non-dma case
> as well.
>
> Here may have issue if also set hsotg->core_params->ahbcfg for
> non-dma case, since GAHBCFG[4:1] may be set.
> >>>
> >>> You can mask off HBstLen in that case. However, I don't think setting
> >>> burst
> length will be an issue in non DMA case as DWC2 will not act as a bus master.
> John, can you please confirm if setting burst length will be an issue in
> non-dma
> case?
> >
> > I don't think it hurts to preserve those bits, but I will check.
> >
>
> I can confirm that AHB_SINGLE, NOTI_ALL_DMA_WRIT, REM_MEM_SUPP,
> HBSTLEN, and INV_DESC_ENDIANNESS only apply in DMA mode and are
> ignored in slave mode operation.
John, thank you! So, there is no harm in setting these in non-dma mode.
>
> John
>
>
BR,
Yousaf
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
RE: [PATCH] Added Pids for Actisense Usb Devices
Sorry, no difference just ignorance on my part. I had the impression having just missed one release I needed to resubmit the patch. Best regards Mark Glover Software Engineer Actisense®/Active Research Ltd Tel:+44 (0)1202 746682 http://www.actisense.com Active Research Limited is registered in England, Company number: 3295909. The information contained in this e-mail is confidential and is intended for the named recipient only. If you are not the named recipient, please notify us by telephone on +44 1202 746682 immediately and destroy the message by deleting it from your computer. Active Research Limited has taken every reasonable precaution to ensure that any attachment to this e-mail has been checked for viruses. However, we cannot accept liability for any damage sustained as a result of any such software viruses and advise you to carry out your own virus check before opening any attachment(s). Furthermore, we do not accept responsibility for any change made to this message after it was sent by the sender. -Original Message- From: Johan Hovold [mailto:jhov...@gmail.com] On Behalf Of Johan Hovold Sent: 12 February 2015 02:24 To: Mark Glover Cc: jo...@kernel.org; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH] Added Pids for Actisense Usb Devices On Wed, Feb 11, 2015 at 10:49:47AM +, Mark Glover wrote: > From: Mark Glover > > Signed-off-by: Mark Glover What is different from your previous version? Please address my comments to that one before resending. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added Pids for Actisense Usb Devices
On Thu, Feb 12, 2015 at 08:46:51AM -, Mark Glover wrote: > Sorry, no difference just ignorance on my part. > I had the impression having just missed one release I needed to resubmit the > patch. No, I'm still waiting for you to answer my comments on your patch. Mostly some clarifications needed. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Unable to reap urb after receiving signal using usbfs.
From: Dave Mielke > Okay. Found it. Yes, I was indeed making an assumption. When a signalfd file > descriptor is closed, it doesn't remove any of the signals that were in its > queue. The next time a signalfd file descriptor is opened for the same signal, > therefore, the signal from the closing of the urb on the previous attempt is > delivered. That sounds like a bug of its own. > Flushing the signalfd file descriptor before closing it resolves the > problem. Might it be better to flush after open() ? David -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: unable to execute hotplug script
On Wed, Feb 11, 2015 at 09:37:18PM +0530, temp sha wrote: > Hi All, > > I am trying to enable hotplugging for usb on my h/w based on 2.6.16 kernel. You are on your own with such an obsolete and out-of-date kernel version, sorry. Please work with whatever vendor is forcing you to use that release, as you are paying them for that support. good luck! greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next 1/3] r8152: separate USB_RX_EARLY_AGG
Hello.
On 2/12/2015 5:36 AM, Hayes Wang wrote:
[...]
+ ocp_data = tp->coalesce / 8;
Why not do it in the initializer?
This is for patch #3. The patch #3 would use this function.
The new function is already called in this patch.
The unit of the relative setting from the ethtool is 1 us.
However, the unit for the hw is 8 us. Therefore, I save the
value with the unit of 1 us, and transfer it to the unit of
the hw when setting.
You're replying to the question I didn't ask. I was just suggesting:
u32 ocp_data = tp->coalesce / 8;
+ ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_TIMEOUT, ocp_data);
... if you don't want to pass 'tp->coalesce / 8' directly here.
+}
+
+static void r8153_set_rx_early_size(struct r8152 *tp)
+{
+ struct net_device *dev = tp->netdev;
Not sure you actually need this variable.
If I replace dev->mtu with tp->netdev->mtu, the line would
more than 80 characters. This is used to avoid it. Should
I remove it?
OK, you can keep it.
+ u32 ocp_data;
+
+ ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;
Why not in initializer?
This is for patch #2. The patch #2 would use this function.
The new function is again used in this patch already.
It has to be re-calculated when the mtu is changed, or the
function is called when the linking status changes to ON.
You're again replying to the question I didn't ask. I was just suggesting:
u32 ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;
+ ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);
... if you don't want to pass that expression directly here.
[...]
@@ -3911,6 +3907,13 @@ static int rtl8152_probe(struct
usb_interface *intf,
tp->mii.reg_num_mask = 0x1f;
tp->mii.phy_id = R8152_PHY_ID;
+ if (udev->speed == USB_SPEED_SUPER)
+ tp->coalesce = COALESCE_SUPER;
+ else if (udev->speed == USB_SPEED_HIGH)
+ tp->coalesce = COALESCE_HIGH;
+ else
+ tp->coalesce = COALESCE_SLOW;
This is asking to be a *switch* statement.
Excuse me. I don't understand what you mean.
switch (udev->speed) {
case USB_SPEED_SUPER:
tp->coalesce = COALESCE_SUPER;
break;
case USB_SPEED_HIGH:
tp->coalesce = COALESCE_HIGH;
break;
default:
tp->coalesce = COALESCE_SLOW;
}
The usb speed is determined when the device is plugged on
the usb host controller or usb hub. The usb speed wouldn't
chage unless you unplug the device and plug it to another
port with different usb speed. Therefore, I provide different
default values for different usb speed.
I didn't ask for explanations here either. :-)
Best Regards,
Hayes
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] usb: dwc3: add ULPI interface support
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > index a8c9062..66cbf38 100644 > > --- a/drivers/usb/dwc3/core.c > > +++ b/drivers/usb/dwc3/core.c > > @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, dwc); > > dwc3_cache_hwparams(dwc); > > > > + ret = dwc3_ulpi_init(dwc); > > If I understood correctly, this call will result in enumerating the phy > via ULPI bus, then registering the correct ULPI device. > Can you guarantee ULPI will be always accessible at this point if we > remove dwc3 module and load it again? OK, got it. So yes, I can guarantee that ULPI will be acessible at this point. If we are in a state where we could soft reset dwc3, we know we can access ULPI. The fact that dwc3 itself expects to be able to write to the ULPI registers at that point guarantees it for us. So in practice when ever dwc3 is powered we will be able to access ULPI for as long as the USB2 PHY interface is not suspended separately with GUSB2PHYCFG SusPHY bit. And even then we would only need to resume it with the same bit and ULPI is accessible again. Cheers, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: dwc2: Register interrupt handler only once gadget is correctly initialized
ping 2015-02-06 17:50 GMT+01:00 Romain Perier : > Don't register interrupt handler before usb gadget is correctly initialized. > For some embedded platforms which don't have a usb-phy, it crashes the driver > because an interrupt is emitted with non-initialized hardware. > According to devm_request_irq documentation, an interrupt can be emitted > at any time once the interrupt is registered, so we have to care about driver > and hardware initialization. > > Signed-off-by: Romain Perier > --- > > Changes for v2: fix typos in commit log > > drivers/usb/dwc2/platform.c | 17 + > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > index ae095f0..b26cf8c 100644 > --- a/drivers/usb/dwc2/platform.c > +++ b/drivers/usb/dwc2/platform.c > @@ -196,14 +196,6 @@ static int dwc2_driver_probe(struct platform_device *dev) > return irq; > } > > - dev_dbg(hsotg->dev, "registering common handler for irq%d\n", > - irq); > - retval = devm_request_irq(hsotg->dev, irq, > - dwc2_handle_common_intr, IRQF_SHARED, > - dev_name(hsotg->dev), hsotg); > - if (retval) > - return retval; > - > res = platform_get_resource(dev, IORESOURCE_MEM, 0); > hsotg->regs = devm_ioremap_resource(&dev->dev, res); > if (IS_ERR(hsotg->regs)) > @@ -237,6 +229,15 @@ static int dwc2_driver_probe(struct platform_device *dev) > retval = dwc2_gadget_init(hsotg, irq); > if (retval) > return retval; > + > +dev_dbg(hsotg->dev, "registering common handler for irq%d\n", > +irq); > +retval = devm_request_irq(hsotg->dev, irq, > +dwc2_handle_common_intr, IRQF_SHARED, > +dev_name(hsotg->dev), hsotg); > +if (retval) > +return retval; > + > retval = dwc2_hcd_init(hsotg, irq, params); > if (retval) > return retval; > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH v1] usb: dwc2: reduce dwc2 driver probe time
> -Original Message- > From: John Youn [mailto:john.y...@synopsys.com] > Sent: Thursday, February 12, 2015 4:33 AM > To: Roy; john.y...@synopsys.com; Felipe Balbi > Cc: Yunzhi Li; jwer...@chromium.org; Herrero, Gregory; Kaukab, Yousaf; > r.bald...@samsung.com; Dinh Nguyen; Eddie Cai; Lin Huang; wulf; 杨凯; Tao > Huang; walkr...@126.com; Douglas Anderson; Greg Kroah-Hartman; linux- > u...@vger.kernel.org; LKML > Subject: Re: [RFC PATCH v1] usb: dwc2: reduce dwc2 driver probe time > > On 2/11/2015 3:42 AM, Roy wrote: > > Hi John Youn: > > > > Could you please give some suggestions from your point of view, > > about this probe time issue ? > > > > Thanks a lot. > > > > at 2015/2/11 2:23, Julius Werner wrote: > >>> @@ -2703,7 +2703,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg > *hsotg) > >>> gusbcfg = readl(hsotg->regs + GUSBCFG); > >>> gusbcfg &= ~GUSBCFG_FORCEHOSTMODE; > >>> writel(gusbcfg, hsotg->regs + GUSBCFG); > >>> - usleep_range(10, 15); > >>> + usleep_range(25000, 5); > >> The point of usleep_range() is to coalesce multiple timer interrupts > >> in idle systems for power efficiency. It's pretty pointless/harmful > >> during probe anyway and there's almost never a reason to make the > >> span larger than a few milliseconds. You should reduce this to > >> something reasonable (e.g. usleep_range(25000, 26000) or even > >> usleep_range(25000, 25000)) to save another chunk of time. Same > >> applies to other delays above. > > Databook does say 25ms. From what I could gather this has to do with the > debounce filter time on the IDDIG pin after the ForceHstMode/ForceDevMode > is programmed. There is no way to poll this. I think the change is acceptable, > even to lower the range as Julius suggested. > > >> > >>> do you know what's the upper boundary for AHB clock ? How fast can > >>> it be? It's not wise to change timers because "it works on my RK3288 > >>> board", you need to guarantee that this won't break anybody else. > >> But this code is already a loop that spins on the AHBIdle bit, right? > >> It should work correctly regardless of the delay. The only question > >> is whether the code could be more efficient with a longer sleep... > >> but since the general recommendation is to delay for ranges less than > >> 10us, and the AHB clock would need to be lower than 100KHz (the ones > >> I see are usually in the range of tens or hundreds of MHz) to take > >> longer than that, this seems reasonable to me. > > Agree with this. It shouldn't take nearly that long and you are polling > anyways. > > > As for the other change: > > > It seems that usleep_range() at boot time will pick the longest value > > in the range. In dwc2_core_reset() there is a very long delay takes > > 200ms, and this function run twice when probe, could any one tell me > > is this delay time resonable ? > > I'm not sure about this value or the reasoning/history behind it. It is not > in our > internal code. It looks like it is taking into account the delay for the > ForceHstMode/ForceDevMode programming. However, I think your change is > conservative and should be ok. Maybe Samsung engineers know about this? If the delay is due to ForceHstMode/ForceDevMode then it should be reduce to 25ms range. As done in dwc2_get_hwparams for example. > > John > > > BR, Yousaf
Re: [PATCH v3 1/5] xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core
On 25.01.2015 10:13, Sneeker Yeh wrote:
> This issue is defined by a three-way race at disconnect, between
> 1) Class driver interrupt endpoint resheduling attempts if the ISR gave an ep
>error event due to device detach (it would try 3 times)
> 2) Disconnect interrupt on PORTSC_CSC, which is cleared by hub thread
>asynchronously
> 3) The hardware IP was configured in silicon with
>- DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1
>- Synopsys IP version is < 3.00a
> The IP will auto-suspend itself on device detach with some phy-specific
> interval
> after CSC is cleared by 2)
>
> If 2) and 3) complete before 1), the interrupts it expects will not be
> generated
> by the autosuspended IP, leading to a deadlock. Even later disconnection
> procedure would detect that corresponding urb is still in-progress and issue a
> ep stop command, auto-suspended IP still won't respond to that command.
>
So did I understand correctly that the class driver submits a new urb which
is enqueued by xhci_urb_enqueue() before the hub thread notices the device is
disconnected.
Then hub thread clears CSC bit, controller suspends and the new urb is never
given back?
Doesn't the CSC bit and PORT_CONNECT bit show the device is disconnected when
we enter
xhci_enqueue_urb(), even it the hub thread doesn't know this yet?
Would it make sense to check those bits in xhci_enqueue_urb, and just return
error
in the xhci_urb_enqueue() if device is not connected? Then there wouldn't be a
need for any quirk
at all.
If that doesn't work then this patch looks good in general. See comments below
> this defect would result in this when device detached:
> ---
> [ 99.603544] usb 4-1: USB disconnect, device number 2
> [ 104.615254] xhci-hcd xhci-hcd.0.auto: xHCI host not responding to stop
> endpoint command.
> [ 104.623362] xhci-hcd xhci-hcd.0.auto: Assuming host is dying, halting host.
> [ 104.653261] xhci-hcd xhci-hcd.0.auto: Host not halted after 16000
> microseconds.
> [ 104.660584] xhci-hcd xhci-hcd.0.auto: Non-responsive xHCI host is not
> halting.
> [ 104.667817] xhci-hcd xhci-hcd.0.auto: Completing active URBs anyway.
> [ 104.674198] xhci-hcd xhci-hcd.0.auto: HC died; cleaning up
> --
> As a result, when device detached, we desired to postpone "PORTCSC clear"
> behind
> "disable slot". it's found that all executed ep command related to
> disconnetion,
> are executed before "disable slot".
>
> Signed-off-by: Sneeker Yeh
> ---
> drivers/usb/host/xhci-hub.c |4
> drivers/usb/host/xhci.c | 29 +
> drivers/usb/host/xhci.h | 24
> 3 files changed, 57 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index a7865c4..3b8f7fc 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -368,6 +368,10 @@ static void xhci_clear_port_change_bit(struct xhci_hcd
> *xhci, u16 wValue,
> port_change_bit = "warm(BH) reset";
> break;
> case USB_PORT_FEAT_C_CONNECTION:
> + if ((xhci->quirks & XHCI_DISCONNECT_QUIRK) &&
> + !(readl(addr) & PORT_CONNECT))
> + return;
> +
New port status event are prevented until CSC is cleared, not sure if there's
any harm in that?
> status = PORT_CSC;
> port_change_bit = "connect";
> break;
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index c50d8d2..aa8e02a 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -3584,6 +3584,33 @@ command_cleanup:
> return ret;
> }
>
> +static void xhci_try_to_clear_csc(struct usb_hcd *hcd, int dev_port_num)
> +{
> + int max_ports;
> + struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + __le32 __iomem **port_array;
> + u32 status;
> +
> + /* print debug info */
> + if (hcd->speed == HCD_USB3) {
> + max_ports = xhci->num_usb3_ports;
> + port_array = xhci->usb3_ports;
> + } else {
> + max_ports = xhci->num_usb2_ports;
> + port_array = xhci->usb2_ports;
> + }
> +
> + if (dev_port_num > max_ports) {
> + xhci_err(xhci, "%s() port number invalid", __func__);
> + return;
> + }
> + status = readl(port_array[dev_port_num - 1]);
> +
> + /* write 1 to clear */
> + if (!(status & PORT_CONNECT) && (status & PORT_CSC))
> + writel(status & PORT_CSC, port_array[0]);
Shouldn't this be writel(...,port_array[dev_port_num - 1]) ?
-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
ci_hdrc ci_hdrc.1: fatal error
Hi Peter, we currently see rare ocasions with the chipidea core code on an mx53 where the controller dies after a long period of usage. ci_hdrc ci_hdrc.1: fatal error ci_hdrc ci_hdrc.1: HC died; cleaning up This is the result of the 'SEI' (System Error) bit in register 'USBSTS' changing to '1' in the host controller. The Synopsys Datasheet says the following regarding this bit: "In the AMBA implementation, this bit will be set to '1b' when an Error response is seen by the master interface (HRESP[1:0]=ERROR)." I found some code in the freescale tree where some of your patches adjust the BURSTSIZE register and other respective things on some mxc platforms. [1] Do you think this could be relatet? Are there some known limitations for the mx5x? Thanks in advance, Michael [1] MLK-9785-5 usb: chipidea: usbmisc_imx: add unburst setting for imx6 MLK-9785-3 usb: chipidea: imx: change ahb burst setting MLK-9785-2 usb: chipidea: add AHB configuration interface MLK-9770-2 usb: chipidea: define stream mode disable for both roles -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Unable to reap urb after receiving signal using usbfs.
On Thu, 12 Feb 2015, David Laight wrote: > From: Dave Mielke > > Okay. Found it. Yes, I was indeed making an assumption. When a signalfd file > > descriptor is closed, it doesn't remove any of the signals that were in its > > queue. The next time a signalfd file descriptor is opened for the same > > signal, > > therefore, the signal from the closing of the urb on the previous attempt is > > delivered. > > That sounds like a bug of its own. Not to me. The purpose of signalfd is to report pending signals. Closing or opening a file descriptor shouldn't change the set of pending signals. > > Flushing the signalfd file descriptor before closing it resolves the > > problem. > > Might it be better to flush after open() ? Why? Besides, if any signals have been raised since the time the file descriptor was closed, flushing after open would lose those signals. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core
On Thu, 12 Feb 2015, Mathias Nyman wrote: > On 25.01.2015 10:13, Sneeker Yeh wrote: > > This issue is defined by a three-way race at disconnect, between > > 1) Class driver interrupt endpoint resheduling attempts if the ISR gave an > > ep > >error event due to device detach (it would try 3 times) > > 2) Disconnect interrupt on PORTSC_CSC, which is cleared by hub thread > >asynchronously > > 3) The hardware IP was configured in silicon with > >- DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1 > >- Synopsys IP version is < 3.00a > > The IP will auto-suspend itself on device detach with some phy-specific > > interval > > after CSC is cleared by 2) > > > > If 2) and 3) complete before 1), the interrupts it expects will not be > > generated > > by the autosuspended IP, leading to a deadlock. Even later disconnection > > procedure would detect that corresponding urb is still in-progress and > > issue a > > ep stop command, auto-suspended IP still won't respond to that command. If the Synopsys IP provides a way to do it, it would be better to turn off the autosuspend feature entirely. Doesn't autosuspend violate the xHCI specification? > So did I understand correctly that the class driver submits a new urb which > is enqueued by xhci_urb_enqueue() before the hub thread notices the device is > disconnected. > Then hub thread clears CSC bit, controller suspends and the new urb is never > given back? > > Doesn't the CSC bit and PORT_CONNECT bit show the device is disconnected when > we enter > xhci_enqueue_urb(), even it the hub thread doesn't know this yet? What if the device disconnects _after_ the new URB is enqueued? > Would it make sense to check those bits in xhci_enqueue_urb, and just return > error > in the xhci_urb_enqueue() if device is not connected? Then there wouldn't be > a need for any quirk > at all. That wouldn't help URBs that were already enqueued when the disconnect occurred. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to reap urb after receiving signal using usbfs.
[quoted lines by Alan Stern on 2015/02/12 at 10:10 -0500] >Not to me. The purpose of signalfd is to report pending signals. >Closing or opening a file descriptor shouldn't change the set of >pending signals. Once the last signalfd file descriptor for a given signal is closed - in our case, that's only one of them - shouldn't the handling revert to the default for that signal? Isn't it wrong that, when there are no signalfd file descriptors open for a given signal, the signal still seems to stay pending within the signalfd mechanism? -- Dave Mielke | 2213 Fox Crescent | The Bible is the very Word of God. Phone: 1-613-726-0014 | Ottawa, Ontario | http://Mielke.cc/bible/ EMail: d...@mielke.cc | Canada K2A 1H7 | http://FamilyRadio.com/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to reap urb after receiving signal using usbfs.
On Thu, 12 Feb 2015, Dave Mielke wrote: > [quoted lines by Alan Stern on 2015/02/12 at 10:10 -0500] > > >Not to me. The purpose of signalfd is to report pending signals. > >Closing or opening a file descriptor shouldn't change the set of > >pending signals. > > Once the last signalfd file descriptor for a given signal is closed - in our > case, that's only one of them - shouldn't the handling revert to the default > for that signal? Isn't it wrong that, when there are no signalfd file > descriptors open for a given signal, the signal still seems to stay pending > within the signalfd mechanism? Doesn't your program mask those signals before submitting URBs? It must; otherwise the completion signals would be delivered to the program in the normal way via a signal handler instead of being reported via signalfd. The signals remain masked even after the file descriptor is closed. Opening or closing a signalfd descriptor doesn't change the set of masked signals -- not according to the man page entry, anyway. Signals don't remain pending within the signalfd mechanism; they just remain pending. They operate more or less independently of signalfd. The only purpose of signalfd is to _report_ pending signals. It doesn't store them or anything like that. The only interaction is this: When the program reads from signalfd that a signal is pending, the signal then becomes unpending (just as if the program's signal handler had run). Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to reap urb after receiving signal using usbfs.
[quoted lines by Alan Stern on 2015/02/12 at 10:44 -0500] >Doesn't your program mask those signals before submitting URBs? It >must; otherwise the completion signals would be delivered to the >program in the normal way via a signal handler instead of being >reported via signalfd. Once the last monitor for a given signal is removed, the former signal blocking state for that signal is restored. That being said, yes, I'd forgotten that way back at program startup (when there's only one thread running), to prevent those signals from going to the wrong thread, all of them are arbitrarily blocked. That lets each thread safely unblock each signal it cares about. So, I aghree, there's no bug - just a bit of memory loss on my part. -- Dave Mielke | 2213 Fox Crescent | The Bible is the very Word of God. Phone: 1-613-726-0014 | Ottawa, Ontario | http://Mielke.cc/bible/ EMail: d...@mielke.cc | Canada K2A 1H7 | http://FamilyRadio.com/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv7 2/4] USB: gadget: atmel_usba_udc: Request an auto disabled Vbus signal IRQ instead of an auto enabled IRQ request followed by IRQ disable
Vbus IRQ handler needs a started UDC driver to work because it uses
udc->driver, which is set by the UDC start handler. The previous way
chosen was to return from interrupt if udc->driver is NULL using a
spinlock around the check.
We now request an auto disabled (IRQ_NOAUTOEN) Vbus signal IRQ instead
of an auto enabled IRQ followed by disable_irq(). This way we remove the
very small timeslot of enabled IRQ which existed previously between
request() and disable(). We don't need anymore to check if udc->driver
is NULL in IRQ handler.
Signed-off-by: Sylvain Rochet
Suggested-by: Boris Brezillon
Acked-by: Boris Brezillon
Acked-by: Nicolas Ferre
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index e63c6fc..bbbd5f1 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1749,10 +1749,6 @@ static irqreturn_t usba_vbus_irq(int irq, void *devid)
spin_lock(&udc->lock);
- /* May happen if Vbus pin toggles during probe() */
- if (!udc->driver)
- goto out;
-
vbus = vbus_is_present(udc);
if (vbus != udc->vbus_prev) {
if (vbus) {
@@ -1773,7 +1769,6 @@ static irqreturn_t usba_vbus_irq(int irq, void *devid)
udc->vbus_prev = vbus;
}
-out:
spin_unlock(&udc->lock);
return IRQ_HANDLED;
@@ -2113,6 +2108,8 @@ static int usba_udc_probe(struct platform_device *pdev)
if (gpio_is_valid(udc->vbus_pin)) {
if (!devm_gpio_request(&pdev->dev, udc->vbus_pin,
"atmel_usba_udc")) {
+ irq_set_status_flags(gpio_to_irq(udc->vbus_pin),
+ IRQ_NOAUTOEN);
ret = devm_request_irq(&pdev->dev,
gpio_to_irq(udc->vbus_pin),
usba_vbus_irq, 0,
@@ -2122,8 +2119,6 @@ static int usba_udc_probe(struct platform_device *pdev)
dev_warn(&udc->pdev->dev,
"failed to request vbus irq; "
"assuming always on\n");
- } else {
- disable_irq(gpio_to_irq(udc->vbus_pin));
}
} else {
/* gpio_request fail so use -EINVAL for gpio_is_valid */
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv7 4/4] USB: gadget: atmel_usba_udc: Add suspend/resume with wakeup support
This patch add suspend/resume with wakeup support for Atmel USBA.
On suspend: We stay continuously clocked if Vbus signal is not
available. If Vbus signal is available we set the Vbus signal as a wake
up source then we stop the USBA itself and all clocks used by USBA.
On resume: We recover clocks and USBA if we stopped them. If a device is
currently connected at resume time we enable the controller.
Signed-off-by: Sylvain Rochet
Acked-by: Boris Brezillon
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 57 +
1 file changed, 57 insertions(+)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 999e2f2..d019b6c 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -2177,6 +2177,7 @@ static int usba_udc_probe(struct platform_device *pdev)
ret = usb_add_gadget_udc(&pdev->dev, &udc->gadget);
if (ret)
return ret;
+ device_init_wakeup(&pdev->dev, 1);
usba_init_debugfs(udc);
for (i = 1; i < udc->num_ep; i++)
@@ -2192,6 +2193,7 @@ static int __exit usba_udc_remove(struct platform_device
*pdev)
udc = platform_get_drvdata(pdev);
+ device_init_wakeup(&pdev->dev, 0);
usb_del_gadget_udc(&udc->gadget);
for (i = 1; i < udc->num_ep; i++)
@@ -2201,10 +2203,65 @@ static int __exit usba_udc_remove(struct
platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM
+static int usba_udc_suspend(struct device *dev)
+{
+ struct usba_udc *udc = dev_get_drvdata(dev);
+
+ /* Not started */
+ if (!udc->driver)
+ return 0;
+
+ mutex_lock(&udc->vbus_mutex);
+
+ if (!device_may_wakeup(dev)) {
+ usba_stop(udc);
+ goto out;
+ }
+
+ /*
+* Device may wake up. We stay clocked if we failed
+* to request vbus irq, assuming always on.
+*/
+ if (gpio_is_valid(udc->vbus_pin)) {
+ usba_stop(udc);
+ enable_irq_wake(gpio_to_irq(udc->vbus_pin));
+ }
+
+out:
+ mutex_unlock(&udc->vbus_mutex);
+ return 0;
+}
+
+static int usba_udc_resume(struct device *dev)
+{
+ struct usba_udc *udc = dev_get_drvdata(dev);
+
+ /* Not started */
+ if (!udc->driver)
+ return 0;
+
+ if (device_may_wakeup(dev) && gpio_is_valid(udc->vbus_pin))
+ disable_irq_wake(gpio_to_irq(udc->vbus_pin));
+
+ /* If Vbus is present, enable the controller and wait for reset */
+ mutex_lock(&udc->vbus_mutex);
+ udc->vbus_prev = vbus_is_present(udc);
+ if (udc->vbus_prev)
+ usba_start(udc);
+ mutex_unlock(&udc->vbus_mutex);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(usba_udc_pm_ops, usba_udc_suspend, usba_udc_resume);
+
static struct platform_driver udc_driver = {
.remove = __exit_p(usba_udc_remove),
.driver = {
.name = "atmel_usba_udc",
+ .pm = &usba_udc_pm_ops,
.of_match_table = of_match_ptr(atmel_udc_dt_ids),
},
};
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv7 3/4] USB: gadget: atmel_usba_udc: Start clocks on rising edge of the Vbus signal, stop clocks on falling edge of the Vbus signal
If USB PLL is not necessary for other USB drivers (e.g. OHCI and EHCI)
we will reduce power consumption by switching off the USB PLL if no USB
Host is currently connected to this USB Device.
We are using Vbus GPIO signal to detect Host presence. If Vbus signal is
not available then the device stays continuously clocked.
Signed-off-by: Sylvain Rochet
Acked-by: Nicolas Ferre
Acked-by: Boris Brezillon
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 144 +---
drivers/usb/gadget/udc/atmel_usba_udc.h | 4 +
2 files changed, 100 insertions(+), 48 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index bbbd5f1..999e2f2 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1739,7 +1739,72 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
return IRQ_HANDLED;
}
-static irqreturn_t usba_vbus_irq(int irq, void *devid)
+static int start_clock(struct usba_udc *udc)
+{
+ int ret;
+
+ if (udc->clocked)
+ return 0;
+
+ ret = clk_prepare_enable(udc->pclk);
+ if (ret)
+ return ret;
+ ret = clk_prepare_enable(udc->hclk);
+ if (ret) {
+ clk_disable_unprepare(udc->pclk);
+ return ret;
+ }
+
+ udc->clocked = true;
+ return 0;
+}
+
+static void stop_clock(struct usba_udc *udc)
+{
+ if (!udc->clocked)
+ return;
+
+ clk_disable_unprepare(udc->hclk);
+ clk_disable_unprepare(udc->pclk);
+
+ udc->clocked = false;
+}
+
+static int usba_start(struct usba_udc *udc)
+{
+ unsigned long flags;
+ int ret;
+
+ ret = start_clock(udc);
+ if (ret)
+ return ret;
+
+ spin_lock_irqsave(&udc->lock, flags);
+ toggle_bias(udc, 1);
+ usba_writel(udc, CTRL, USBA_ENABLE_MASK);
+ usba_int_enb_set(udc, USBA_END_OF_RESET);
+ spin_unlock_irqrestore(&udc->lock, flags);
+
+ return 0;
+}
+
+static void usba_stop(struct usba_udc *udc)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&udc->lock, flags);
+ udc->gadget.speed = USB_SPEED_UNKNOWN;
+ reset_all_endpoints(udc);
+
+ /* This will also disable the DP pullup */
+ toggle_bias(udc, 0);
+ usba_writel(udc, CTRL, USBA_DISABLE_MASK);
+ spin_unlock_irqrestore(&udc->lock, flags);
+
+ stop_clock(udc);
+}
+
+static irqreturn_t usba_vbus_irq_thread(int irq, void *devid)
{
struct usba_udc *udc = devid;
int vbus;
@@ -1747,30 +1812,22 @@ static irqreturn_t usba_vbus_irq(int irq, void *devid)
/* debounce */
udelay(10);
- spin_lock(&udc->lock);
+ mutex_lock(&udc->vbus_mutex);
vbus = vbus_is_present(udc);
if (vbus != udc->vbus_prev) {
if (vbus) {
- toggle_bias(udc, 1);
- usba_writel(udc, CTRL, USBA_ENABLE_MASK);
- usba_int_enb_set(udc, USBA_END_OF_RESET);
+ usba_start(udc);
} else {
- udc->gadget.speed = USB_SPEED_UNKNOWN;
- reset_all_endpoints(udc);
- toggle_bias(udc, 0);
- usba_writel(udc, CTRL, USBA_DISABLE_MASK);
- if (udc->driver->disconnect) {
- spin_unlock(&udc->lock);
+ usba_stop(udc);
+
+ if (udc->driver->disconnect)
udc->driver->disconnect(&udc->gadget);
- spin_lock(&udc->lock);
- }
}
udc->vbus_prev = vbus;
}
- spin_unlock(&udc->lock);
-
+ mutex_unlock(&udc->vbus_mutex);
return IRQ_HANDLED;
}
@@ -1782,57 +1839,47 @@ static int atmel_usba_start(struct usb_gadget *gadget,
unsigned long flags;
spin_lock_irqsave(&udc->lock, flags);
-
udc->devstatus = 1 << USB_DEVICE_SELF_POWERED;
udc->driver = driver;
spin_unlock_irqrestore(&udc->lock, flags);
- ret = clk_prepare_enable(udc->pclk);
- if (ret)
- return ret;
- ret = clk_prepare_enable(udc->hclk);
- if (ret) {
- clk_disable_unprepare(udc->pclk);
- return ret;
- }
+ mutex_lock(&udc->vbus_mutex);
- udc->vbus_prev = 0;
if (gpio_is_valid(udc->vbus_pin))
enable_irq(gpio_to_irq(udc->vbus_pin));
/* If Vbus is present, enable the controller and wait for reset */
- spin_lock_irqsave(&udc->lock, flags);
- if (vbus_is_present(udc) && udc->vbus_prev == 0) {
- toggle_bias(udc, 1);
- usba_writel(udc, CTRL, USBA_ENABLE_MASK);
- usba_int_enb_set(udc, USBA_END_OF_RESET);
-
- udc->vbus_prev = 1;
+ udc->vbus_prev = vbus_i
[PATCHv7 1/4] USB: gadget: atmel_usba_udc: Fixed vbus_prev initial state
If vbus gpio is high at init, we should set vbus_prev to true accordingly to the current vbus state. Without that, we skip the first vbus interrupt because the saved vbus state is not consistent. Signed-off-by: Sylvain Rochet Acked-by: Nicolas Ferre Acked-by: Boris Brezillon --- drivers/usb/gadget/udc/atmel_usba_udc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c index d79cb35..e63c6fc 100644 --- a/drivers/usb/gadget/udc/atmel_usba_udc.c +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c @@ -1811,6 +1811,8 @@ static int atmel_usba_start(struct usb_gadget *gadget, toggle_bias(udc, 1); usba_writel(udc, CTRL, USBA_ENABLE_MASK); usba_int_enb_set(udc, USBA_END_OF_RESET); + + udc->vbus_prev = 1; } spin_unlock_irqrestore(&udc->lock, flags); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv7 0/4] USB: gadget: atmel_usba_udc: PM driver improvements
Start clocks on rising edge of the Vbus signal, stop clocks on falling edge of the Vbus signal. Add suspend/resume with wakeup support. Changes since v6: * Removed single IRQ edge support, which was used to wake up only on device connection. We don't have yet a clean solution to handle IRQ controller with (at91sam9x5) and without (at91rm9200) single edge support. Rework PATCH v6 4/5 removed, we don't need this one anymore. Changes since v5: * Some boards does not support configuring a GPIO interrupt on a specific edge (rising only or falling only). As suggested added a config boolean to distinguish between boards with and without support * Added a missing mutex_lock()/unlock() in usba_udc_suspend(), we need to protect usba_stop() because usba_start() and usba_stop() can still be called by Vbus IRQ handler. * Rebased the full series on linux-next * Patch cleaning with the help of checkpatch.pl Changes since v4: * Now using IRQ_NOAUTOEN flag to remove the unused check for udc->driver is not NULL in the Vbus IRQ. * Reworked the start/stop of clocks on Vbus edges to prepare for suspend/resume support, factorised start and stop procedures * New patch, suspend/resume for USBA with wakeup support Changes since v3: * Added stable tag for the first patch * As suggested, removed the unused check for udc->driver is NULL in Vbus IRQ by requesting IRQ after udc->driver is set and by releasing IRQ before udc->driver is cleared * Rebased the core patch of this series against the just explained changes Changes since v2: * Use spin_lock_irqsave/unlock_irqrestore instead of spin_lock/unlock in threaded interrupt because we are not in irq context anymore * Removed useless and probably harmful IRQF_NO_SUSPEND from devm_request_threaded_irq() flags Changes since v1: * Using a threaded irq and mutex instead of spinclock as suggested * Moved a silently fixed bug in a separate patch (1/2) Sylvain Rochet (4): USB: gadget: atmel_usba_udc: Fixed vbus_prev initial state USB: gadget: atmel_usba_udc: Request an auto disabled Vbus signal IRQ instead of an auto enabled IRQ request followed by IRQ disable USB: gadget: atmel_usba_udc: Start clocks on rising edge of the Vbus signal, stop clocks on falling edge of the Vbus signal USB: gadget: atmel_usba_udc: Add suspend/resume with wakeup support drivers/usb/gadget/udc/atmel_usba_udc.c | 208 drivers/usb/gadget/udc/atmel_usba_udc.h | 4 + 2 files changed, 159 insertions(+), 53 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv6 4/5] USB: gadget: atmel_usba_udc: Prepare for IRQ single edge support
Hello Boris, On Sun, Feb 08, 2015 at 10:24:39AM +0100, Boris Brezillon wrote: > On Sat, 7 Feb 2015 20:37:23 +0100 > Sylvain Rochet wrote: > > > If not, is udc->caps->irq_single_edge_support boolean acceptable ? > > Do you mean keeping the current approach ? Yes! > If you do, then maybe you can rework a bit the way you detect the GPIO > controller you depends on: instead of linking this information to the > usba compatible string you could link it to the gpio controller > compatible string. > > You can find the gpio controller node thanks to your "vbus-gpio" > property: use the phandle defined in this property to find the gpio > controller node, and once you have the device_node referencing the gpio > controller you can match it with your internal device_id table > (containing 2 entries: one for the at91rm9200 IP and the other for the > at91sam9x5 IP). I have a working PoC for that if this is the chosen solution. > Another solution would be to add an irq_try_set_irq_type function that > would not complain when it fails to set the requested trigger. > > Thomas, I know you did not follow the whole thread, but would you mind > adding this irq_try_set_irq_type function (here is a reference > implementation [1]), to prevent this error trace from happening when > we're just trying a configuration ? This would be great :-) > > If not, I am ok to drop the feature, this is only a bonus. > > That could be a short term solution, to get this series accepted. We > could then find a proper way to support that optimization. I agree, I have the feeling your proposed core change may takes a long time, I just sent a v7 without IRQ single edge support. Sylvain -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc3: dwc3-omap: Fix disable IRQ
On Thu, Feb 12, 2015 at 11:13:16AM +0530, George Cherian wrote:
> In the wrapper the IRQ disable should be done by writing 1's to the
> IRQ*_CLR register. Existing code is broken because it instead writes
> zeros to IRQ*_SET register.
>
> Fix this by adding functions dwc3_omap_write_irqmisc_clr() and
> dwc3_omap_write_irq0_clr() which do the right thing.
>
> Signed-off-by: George Cherian
please resend with:
Fixes: 72246da40f37 (usb: Introduce DesignWare USB3 DRD Driver)
Cc: # v3.2+
> ---
> drivers/usb/dwc3/dwc3-omap.c | 30 --
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
> index 172d64e..52e0c4e 100644
> --- a/drivers/usb/dwc3/dwc3-omap.c
> +++ b/drivers/usb/dwc3/dwc3-omap.c
> @@ -205,6 +205,18 @@ static void dwc3_omap_write_irq0_set(struct dwc3_omap
> *omap, u32 value)
> omap->irq0_offset, value);
> }
>
> +static void dwc3_omap_write_irqmisc_clr(struct dwc3_omap *omap, u32 value)
> +{
> + dwc3_omap_writel(omap->base, USBOTGSS_IRQENABLE_CLR_MISC +
> + omap->irqmisc_offset, value);
> +}
> +
> +static void dwc3_omap_write_irq0_clr(struct dwc3_omap *omap, u32 value)
> +{
> + dwc3_omap_writel(omap->base, USBOTGSS_IRQENABLE_CLR_0 -
> + omap->irq0_offset, value);
> +}
> +
> static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
> enum omap_dwc3_vbus_id_status status)
> {
> @@ -345,9 +357,23 @@ static void dwc3_omap_enable_irqs(struct dwc3_omap *omap)
>
> static void dwc3_omap_disable_irqs(struct dwc3_omap *omap)
> {
> + u32 reg;
> +
> /* disable all IRQs */
> - dwc3_omap_write_irqmisc_set(omap, 0x00);
> - dwc3_omap_write_irq0_set(omap, 0x00);
> + reg = USBOTGSS_IRQO_COREIRQ_ST;
> + dwc3_omap_write_irq0_clr(omap, reg);
> +
> + reg = (USBOTGSS_IRQMISC_OEVT |
> + USBOTGSS_IRQMISC_DRVVBUS_RISE |
> + USBOTGSS_IRQMISC_CHRGVBUS_RISE |
> + USBOTGSS_IRQMISC_DISCHRGVBUS_RISE |
> + USBOTGSS_IRQMISC_IDPULLUP_RISE |
> + USBOTGSS_IRQMISC_DRVVBUS_FALL |
> + USBOTGSS_IRQMISC_CHRGVBUS_FALL |
> + USBOTGSS_IRQMISC_DISCHRGVBUS_FALL |
> + USBOTGSS_IRQMISC_IDPULLUP_FALL);
> +
> + dwc3_omap_write_irqmisc_clr(omap, reg);
> }
>
> static u64 dwc3_omap_dma_mask = DMA_BIT_MASK(32);
> --
> 1.8.3.1
>
--
balbi
signature.asc
Description: Digital signature
[RFC] USB phy type C
Hi Felipe, et al, Is there any on going discussion regarding to USB phy for type C connectors? We'd like to know the community's preferable direction for handling (still unsupported) cases like USB3.1 PD, Accessories and Alternate modes as a new layer or an extension of current USB phy layer. Comments are welcome :) Br, David -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] USB phy type C
Hi,
On Thu, Feb 12, 2015 at 10:41:37AM -0800, David Cohen wrote:
> Hi Felipe, et al,
>
> Is there any on going discussion regarding to USB phy for type C connectors?
>
> We'd like to know the community's preferable direction for handling
> (still unsupported) cases like USB3.1 PD, Accessories and Alternate
> modes as a new layer or an extension of current USB phy layer.
>
> Comments are welcome :)
Not the USB phy layer, but the generic phy layer. All you need to do is
add a new phy provider and add support for it from within dwc3.
Something like below:
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9f0e209b8f6c..55a85b40e43e 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -653,6 +653,19 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
}
}
+ dwc->usb3_typec_phy = devm_phy_get(dev, "usb3-typec-phy");
+ if (IS_ERR(dwc->usb3_typec_phy)) {
+ ret = PTR_ERR(dwc->usb3_typec_phy);
+ if (ret == -ENOSYS || ret == -ENODEV) {
+ dwc->usb3_typec_phy = NULL;
+ } else if (ret == -EPROBE_DEFER) {
+ return ret;
+ } else {
+ dev_err(dev, "no usb3 typeC phy, continuing without\n");
+ dwc->usb3_typec_phy = NULL;
+ }
+ }
+
return 0;
}
The only interesting part will be dealing with the other modes of
operation, I haven't wrapped my head around it yet.
--
balbi
signature.asc
Description: Digital signature
Re: [RFC] USB phy type C
On Thu, Feb 12, 2015 at 12:45:53PM -0600, Felipe Balbi wrote:
> Hi,
>
> On Thu, Feb 12, 2015 at 10:41:37AM -0800, David Cohen wrote:
> > Hi Felipe, et al,
> >
> > Is there any on going discussion regarding to USB phy for type C connectors?
> >
> > We'd like to know the community's preferable direction for handling
> > (still unsupported) cases like USB3.1 PD, Accessories and Alternate
> > modes as a new layer or an extension of current USB phy layer.
> >
> > Comments are welcome :)
>
> Not the USB phy layer, but the generic phy layer. All you need to do is
> add a new phy provider and add support for it from within dwc3.
> Something like below:
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 9f0e209b8f6c..55a85b40e43e 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -653,6 +653,19 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
> }
> }
>
> + dwc->usb3_typec_phy = devm_phy_get(dev, "usb3-typec-phy");
> + if (IS_ERR(dwc->usb3_typec_phy)) {
> + ret = PTR_ERR(dwc->usb3_typec_phy);
> + if (ret == -ENOSYS || ret == -ENODEV) {
> + dwc->usb3_typec_phy = NULL;
> + } else if (ret == -EPROBE_DEFER) {
> + return ret;
> + } else {
> + dev_err(dev, "no usb3 typeC phy, continuing without\n");
> + dwc->usb3_typec_phy = NULL;
> + }
> + }
> +
> return 0;
> }
Thanks. That's a nice direction.
>
>
> The only interesting part will be dealing with the other modes of
> operation, I haven't wrapped my head around it yet.
We'll probably get back to here with more questions/proposals about
that.
Br, David
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] USB phy type C
On Thu, Feb 12, 2015 at 11:27:28AM -0800, David Cohen wrote:
> On Thu, Feb 12, 2015 at 12:45:53PM -0600, Felipe Balbi wrote:
> > Hi,
> >
> > On Thu, Feb 12, 2015 at 10:41:37AM -0800, David Cohen wrote:
> > > Hi Felipe, et al,
> > >
> > > Is there any on going discussion regarding to USB phy for type C
> > > connectors?
> > >
> > > We'd like to know the community's preferable direction for handling
> > > (still unsupported) cases like USB3.1 PD, Accessories and Alternate
> > > modes as a new layer or an extension of current USB phy layer.
> > >
> > > Comments are welcome :)
> >
> > Not the USB phy layer, but the generic phy layer. All you need to do is
> > add a new phy provider and add support for it from within dwc3.
> > Something like below:
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 9f0e209b8f6c..55a85b40e43e 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -653,6 +653,19 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
> > }
> > }
> >
> > + dwc->usb3_typec_phy = devm_phy_get(dev, "usb3-typec-phy");
> > + if (IS_ERR(dwc->usb3_typec_phy)) {
> > + ret = PTR_ERR(dwc->usb3_typec_phy);
> > + if (ret == -ENOSYS || ret == -ENODEV) {
> > + dwc->usb3_typec_phy = NULL;
> > + } else if (ret == -EPROBE_DEFER) {
> > + return ret;
> > + } else {
> > + dev_err(dev, "no usb3 typeC phy, continuing without\n");
> > + dwc->usb3_typec_phy = NULL;
> > + }
> > + }
> > +
> > return 0;
> > }
>
> Thanks. That's a nice direction.
btw, for typec, let's support only the new phy framework, that ought to
get people to move :-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 6/8] usb: dwc3: add ULPI interface support
On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote: > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > index a8c9062..66cbf38 100644 > > > --- a/drivers/usb/dwc3/core.c > > > +++ b/drivers/usb/dwc3/core.c > > > @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev) > > > platform_set_drvdata(pdev, dwc); > > > dwc3_cache_hwparams(dwc); > > > > > > + ret = dwc3_ulpi_init(dwc); > > > > If I understood correctly, this call will result in enumerating the phy > > via ULPI bus, then registering the correct ULPI device. > > Can you guarantee ULPI will be always accessible at this point if we > > remove dwc3 module and load it again? > > OK, got it. So yes, I can guarantee that ULPI will be acessible at > this point. If we are in a state where we could soft reset dwc3, we > know we can access ULPI. The fact that dwc3 itself expects to be able > to write to the ULPI registers at that point guarantees it for us. I just double checked DWC3 TRM. You are correct, by the time we're executing dwc3_core_soft_reset() ULPI bus is already accessible. But the TRM also specifies an ULPI phy would be reset by DCTL's core soft reset, which is executed by dwc3_core_init() before calling dwc3_core_soft_reset(). It does mention DWC3 writes data to an ULPI phy register during reset, but it also mentions the clock sync happens during that time. That makes no even OK, but more correct IMO to power on phy before core's soft reset (i.e. by ACPI methods). But it lets us in a grey area whether ULPI is reliably accessible before core's soft reset. I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after core's soft reset we've got no more grey area. Br, David > > So in practice when ever dwc3 is powered we will be able to access > ULPI for as long as the USB2 PHY interface is not suspended separately > with GUSB2PHYCFG SusPHY bit. And even then we would only need to > resume it with the same bit and ULPI is accessible again. > > > Cheers, > > -- > heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] usb: add bus type for USB ULPI
On 01/23/15 07:12, Heikki Krogerus wrote:
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index e614ef6..753cb08 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -1176,6 +1176,19 @@ static int do_rio_entry(const char *filename,
> }
> ADD_TO_DEVTABLE("rapidio", rio_device_id, do_rio_entry);
>
> +/* Looks like: mei:S */
This comment doesn't look right.
> +static int do_ulpi_entry(const char *filename, void *symval,
> + char *alias)
> +{
> + DEF_FIELD(symval, ulpi_device_id, vendor);
> + DEF_FIELD(symval, ulpi_device_id, product);
> +
> + sprintf(alias, "ulpi:v%04xp%04x", vendor, product);
> +
> + return 1;
> +}
> +ADD_TO_DEVTABLE("ulpi", ulpi_device_id, do_ulpi_entry);
> +
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY
Hi Heikki, Sorry I am starting a new branch on this thread. I need to go back to another topic on this same patch. On Fri, Jan 23, 2015 at 05:12:58PM +0200, Heikki Krogerus wrote: > TUSB1210 ULPI PHY has vendor specific register for eye > diagram tuning. On some platforms the system firmware has > set optimized value to it. In order to not loose the > optimized value, the driver stores it during probe and > restores it every time the PHY is powered back on. > > Signed-off-by: Heikki Krogerus > Cc: Kishon Vijay Abraham I > --- [snip] > + /* Store initial eye diagram optimisation value */ > + ret = ulpi_read(ulpi, TUSB1210_VENDOR_SPECIFIC2); We can't rely on eye diagram optimization value here. It's possible the phy went through reset (e.g. module unloading/loading). We need a more consistent method. Could we use _DSD instead? That would be more compatible with DT too. Br, David > + if (ret < 0) > + return ret; > + > + tusb->eye_diagram_tuning = ret; > + > + tusb->phy = ulpi_phy_create(ulpi, &phy_ops); > + if (IS_ERR(tusb->phy)) > + return PTR_ERR(tusb->phy); > + > + tusb->ulpi = ulpi; > + > + phy_set_drvdata(tusb->phy, tusb); > + ulpi_set_drvdata(ulpi, tusb); > + return 0; > +} > + -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] usb: dwc3: add ULPI interface support
On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote: > On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote: > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > > index a8c9062..66cbf38 100644 > > > > --- a/drivers/usb/dwc3/core.c > > > > +++ b/drivers/usb/dwc3/core.c > > > > @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev) > > > > platform_set_drvdata(pdev, dwc); > > > > dwc3_cache_hwparams(dwc); > > > > > > > > + ret = dwc3_ulpi_init(dwc); > > > > > > If I understood correctly, this call will result in enumerating the phy > > > via ULPI bus, then registering the correct ULPI device. > > > Can you guarantee ULPI will be always accessible at this point if we > > > remove dwc3 module and load it again? > > > > OK, got it. So yes, I can guarantee that ULPI will be acessible at > > this point. If we are in a state where we could soft reset dwc3, we > > know we can access ULPI. The fact that dwc3 itself expects to be able > > to write to the ULPI registers at that point guarantees it for us. > > I just double checked DWC3 TRM. > You are correct, by the time we're executing dwc3_core_soft_reset() ULPI > bus is already accessible. But the TRM also specifies an ULPI phy would > be reset by DCTL's core soft reset, which is executed by dwc3_core_init() > before calling dwc3_core_soft_reset(). It does mention DWC3 writes data > to an ULPI phy register during reset, but it also mentions the clock > sync happens during that time. > > That makes no even OK, but more correct IMO to power on phy before Sorry for the typo. I meant: That makes not only OK, but more correct... > core's soft reset (i.e. by ACPI methods). But it lets us in a grey area > whether ULPI is reliably accessible before core's soft reset. > > I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after > core's soft reset we've got no more grey area. > > Br, David > > > > > So in practice when ever dwc3 is powered we will be able to access > > ULPI for as long as the USB2 PHY interface is not suspended separately > > with GUSB2PHYCFG SusPHY bit. And even then we would only need to > > resume it with the same bit and ULPI is accessible again. > > > > > > Cheers, > > > > -- > > heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: dwc2: gadget reuse ahbcfg assigned from platform
On 2/12/2015 1:04 AM, Kaukab, Yousaf wrote: >> -Original Message- >> From: John Youn [mailto:john.y...@synopsys.com] >> Sent: Wednesday, February 11, 2015 11:00 PM >> To: Zhangfei Gao; Kaukab, Yousaf >> Cc: ba...@ti.com; john.y...@synopsys.com; linux-usb@vger.kernel.org >> Subject: Re: [PATCH 2/2] usb: dwc2: gadget reuse ahbcfg assigned from >> platform >> >> On 2/6/2015 2:23 PM, John Youn wrote: >>> On 2/6/2015 6:02 AM, Zhangfei Gao wrote: On 6 February 2015 at 16:07, Kaukab, Yousaf >> wrote: GAHBCFG_HBSTLEN_INCR4 << diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 15aa578..20085de 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -2314,9 +2314,13 @@ void s3c_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg, GINTSTS_USBSUSP | GINTSTS_WKUPINT, hsotg->regs + GINTMSK); + if ((hsotg->core_params) && (hsotg->core_params->ahbcfg != + - 1)) + val = hsotg->core_params->ahbcfg & ~GAHBCFG_CTRL_MASK; + else + val = GAHBCFG_HBSTLEN_INCR4 << GAHBCFG_HBSTLEN_SHIFT; + if (using_dma(hsotg)) - writel(GAHBCFG_GLBL_INTR_EN | GAHBCFG_DMA_EN | -(GAHBCFG_HBSTLEN_INCR4 << GAHBCFG_HBSTLEN_SHIFT), + writel(GAHBCFG_GLBL_INTR_EN | GAHBCFG_DMA_EN | val, hsotg->regs + GAHBCFG); else writel(((hsotg->dedicated_fifos) ? (GAHBCFG_NP_TXF_EMP_LVL | >>> >>> There are other bits in GAHBCFG that can be set from platform. >>> They will be >> preserved by your patch, as they are not part of GAHBCFG_CTRL_MASK, >> but only in case dma is enabled. Perhaps preserve them in non-dma case >> as well. >> >> Here may have issue if also set hsotg->core_params->ahbcfg for >> non-dma case, since GAHBCFG[4:1] may be set. > > You can mask off HBstLen in that case. However, I don't think setting > burst >> length will be an issue in non DMA case as DWC2 will not act as a bus master. >> John, can you please confirm if setting burst length will be an issue in >> non-dma >> case? >>> >>> I don't think it hurts to preserve those bits, but I will check. >>> >> >> I can confirm that AHB_SINGLE, NOTI_ALL_DMA_WRIT, REM_MEM_SUPP, >> HBSTLEN, and INV_DESC_ENDIANNESS only apply in DMA mode and are >> ignored in slave mode operation. > > John, thank you! So, there is no harm in setting these in non-dma mode. > That's correct. John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: dwc2: Register interrupt handler only once gadget is correctly initialized
On 2/12/2015 4:42 AM, Romain Perier wrote: > ping > > 2015-02-06 17:50 GMT+01:00 Romain Perier : >> Don't register interrupt handler before usb gadget is correctly initialized. >> For some embedded platforms which don't have a usb-phy, it crashes the driver >> because an interrupt is emitted with non-initialized hardware. >> According to devm_request_irq documentation, an interrupt can be emitted >> at any time once the interrupt is registered, so we have to care about driver >> and hardware initialization. >> >> Signed-off-by: Romain Perier >> --- >> >> Changes for v2: fix typos in commit log >> >> drivers/usb/dwc2/platform.c | 17 + >> 1 file changed, 9 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c >> index ae095f0..b26cf8c 100644 >> --- a/drivers/usb/dwc2/platform.c >> +++ b/drivers/usb/dwc2/platform.c >> @@ -196,14 +196,6 @@ static int dwc2_driver_probe(struct platform_device >> *dev) >> return irq; >> } >> >> - dev_dbg(hsotg->dev, "registering common handler for irq%d\n", >> - irq); >> - retval = devm_request_irq(hsotg->dev, irq, >> - dwc2_handle_common_intr, IRQF_SHARED, >> - dev_name(hsotg->dev), hsotg); >> - if (retval) >> - return retval; >> - >> res = platform_get_resource(dev, IORESOURCE_MEM, 0); >> hsotg->regs = devm_ioremap_resource(&dev->dev, res); >> if (IS_ERR(hsotg->regs)) >> @@ -237,6 +229,15 @@ static int dwc2_driver_probe(struct platform_device >> *dev) >> retval = dwc2_gadget_init(hsotg, irq); >> if (retval) >> return retval; >> + >> +dev_dbg(hsotg->dev, "registering common handler for irq%d\n", >> +irq); >> +retval = devm_request_irq(hsotg->dev, irq, >> +dwc2_handle_common_intr, IRQF_SHARED, >> +dev_name(hsotg->dev), hsotg); >> +if (retval) >> +return retval; >> + >> retval = dwc2_hcd_init(hsotg, irq, params); >> if (retval) >> return retval; Hi, I'm going to be away until Wednesday, Feb 18. I'll take a look at this and other pending dwc2 patches at that time. Regards, John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: ci_hdrc ci_hdrc.1: fatal error
> Hi Peter, > > we currently see rare ocasions with the chipidea core code on an mx53 > where the controller dies after a long period of usage. > > ci_hdrc ci_hdrc.1: fatal error > ci_hdrc ci_hdrc.1: HC died; cleaning up > > This is the result of the 'SEI' (System Error) bit in register 'USBSTS' > changing to '1' in the host controller. > > The Synopsys Datasheet says the following regarding this bit: > > "In the AMBA implementation, this bit will be set to '1b' when an > Error response is seen by the master interface (HRESP[1:0]=ERROR)." > When the controller visits invalidate memory address, the core will trigger system error interrupt. It usually occurs when the invalidate memory address at qtd/qHD. > > I found some code in the freescale tree where some of your patches > adjust the BURSTSIZE register and other respective things on some mxc > platforms. [1] > > Do you think this could be relatet? Not related, it is performance related. > Are there some known limitations for the mx5x? > AFAIK, no customer reports similar problem at imx5x, you may have a check for errate. > Thanks in Advance, > Michael > > [1] > > MLK-9785-5 usb: chipidea: usbmisc_imx: add unburst setting for imx6 > MLK-9785-3 usb: chipidea: imx: change ahb burst setting > MLK-9785-2 usb: chipidea: add AHB configuration interface > MLK-9770-2 usb: chipidea: define stream mode disable for both roles > Peter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: dwc3: dwc3-omap: Fix disable IRQ
In the wrapper the IRQ disable should be done by writing 1's to the
IRQ*_CLR register. Existing code is broken because it instead writes
zeros to IRQ*_SET register.
Fix this by adding functions dwc3_omap_write_irqmisc_clr() and
dwc3_omap_write_irq0_clr() which do the right thing.
Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
Cc: # v3.2+
Signed-off-by: George Cherian
---
drivers/usb/dwc3/dwc3-omap.c | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 172d64e..52e0c4e 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -205,6 +205,18 @@ static void dwc3_omap_write_irq0_set(struct dwc3_omap
*omap, u32 value)
omap->irq0_offset, value);
}
+static void dwc3_omap_write_irqmisc_clr(struct dwc3_omap *omap, u32 value)
+{
+ dwc3_omap_writel(omap->base, USBOTGSS_IRQENABLE_CLR_MISC +
+ omap->irqmisc_offset, value);
+}
+
+static void dwc3_omap_write_irq0_clr(struct dwc3_omap *omap, u32 value)
+{
+ dwc3_omap_writel(omap->base, USBOTGSS_IRQENABLE_CLR_0 -
+ omap->irq0_offset, value);
+}
+
static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
enum omap_dwc3_vbus_id_status status)
{
@@ -345,9 +357,23 @@ static void dwc3_omap_enable_irqs(struct dwc3_omap *omap)
static void dwc3_omap_disable_irqs(struct dwc3_omap *omap)
{
+ u32 reg;
+
/* disable all IRQs */
- dwc3_omap_write_irqmisc_set(omap, 0x00);
- dwc3_omap_write_irq0_set(omap, 0x00);
+ reg = USBOTGSS_IRQO_COREIRQ_ST;
+ dwc3_omap_write_irq0_clr(omap, reg);
+
+ reg = (USBOTGSS_IRQMISC_OEVT |
+ USBOTGSS_IRQMISC_DRVVBUS_RISE |
+ USBOTGSS_IRQMISC_CHRGVBUS_RISE |
+ USBOTGSS_IRQMISC_DISCHRGVBUS_RISE |
+ USBOTGSS_IRQMISC_IDPULLUP_RISE |
+ USBOTGSS_IRQMISC_DRVVBUS_FALL |
+ USBOTGSS_IRQMISC_CHRGVBUS_FALL |
+ USBOTGSS_IRQMISC_DISCHRGVBUS_FALL |
+ USBOTGSS_IRQMISC_IDPULLUP_FALL);
+
+ dwc3_omap_write_irqmisc_clr(omap, reg);
}
static u64 dwc3_omap_dma_mask = DMA_BIT_MASK(32);
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc3: dwc3-omap: Fix disable IRQ
On 02/12/2015 11:52 PM, Felipe Balbi wrote: On Thu, Feb 12, 2015 at 11:13:16AM +0530, George Cherian wrote: >In the wrapper the IRQ disable should be done by writing 1's to the >IRQ*_CLR register. Existing code is broken because it instead writes >zeros to IRQ*_SET register. > >Fix this by adding functions dwc3_omap_write_irqmisc_clr() and >dwc3_omap_write_irq0_clr() which do the right thing. > >Signed-off-by: George Cherian please resend with: Fixes: 72246da40f37 (usb: Introduce DesignWare USB3 DRD Driver) Cc: # v3.2+ Done!! -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: dwc2: Register interrupt handler only once gadget is correctly initialized
No problem Regards, Romain 2015-02-13 3:47 GMT+01:00 John Youn : > On 2/12/2015 4:42 AM, Romain Perier wrote: >> ping >> >> 2015-02-06 17:50 GMT+01:00 Romain Perier : >>> Don't register interrupt handler before usb gadget is correctly initialized. >>> For some embedded platforms which don't have a usb-phy, it crashes the >>> driver >>> because an interrupt is emitted with non-initialized hardware. >>> According to devm_request_irq documentation, an interrupt can be emitted >>> at any time once the interrupt is registered, so we have to care about >>> driver >>> and hardware initialization. >>> >>> Signed-off-by: Romain Perier >>> --- >>> >>> Changes for v2: fix typos in commit log >>> >>> drivers/usb/dwc2/platform.c | 17 + >>> 1 file changed, 9 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c >>> index ae095f0..b26cf8c 100644 >>> --- a/drivers/usb/dwc2/platform.c >>> +++ b/drivers/usb/dwc2/platform.c >>> @@ -196,14 +196,6 @@ static int dwc2_driver_probe(struct platform_device >>> *dev) >>> return irq; >>> } >>> >>> - dev_dbg(hsotg->dev, "registering common handler for irq%d\n", >>> - irq); >>> - retval = devm_request_irq(hsotg->dev, irq, >>> - dwc2_handle_common_intr, IRQF_SHARED, >>> - dev_name(hsotg->dev), hsotg); >>> - if (retval) >>> - return retval; >>> - >>> res = platform_get_resource(dev, IORESOURCE_MEM, 0); >>> hsotg->regs = devm_ioremap_resource(&dev->dev, res); >>> if (IS_ERR(hsotg->regs)) >>> @@ -237,6 +229,15 @@ static int dwc2_driver_probe(struct platform_device >>> *dev) >>> retval = dwc2_gadget_init(hsotg, irq); >>> if (retval) >>> return retval; >>> + >>> +dev_dbg(hsotg->dev, "registering common handler for irq%d\n", >>> +irq); >>> +retval = devm_request_irq(hsotg->dev, irq, >>> +dwc2_handle_common_intr, IRQF_SHARED, >>> +dev_name(hsotg->dev), hsotg); >>> +if (retval) >>> +return retval; >>> + >>> retval = dwc2_hcd_init(hsotg, irq, params); >>> if (retval) >>> return retval; > > Hi, > > I'm going to be away until Wednesday, Feb 18. I'll take a look at > this and other pending dwc2 patches at that time. > > Regards, > John > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
