RE: [PATCH 1/4] ARM: dts: mxs-phy: Change mxs phy clock usage
> On Thu, Jan 10, 2013 at 04:35:51PM +0800, Peter Chen wrote: > > For mxs-phy user i.mx6q, the PHY's clock is controlled by > > hardware automatically, the software only needs to enable it > > at probe, this clock should be used like below: > > > > - Enable at mxs-phy's probe, and disable at mxs-phy's remove, so > > The clk framework doesn't need to know it. But other mxs-phy user > > mx28/mx23 may need it, so we give mxs-phy a dummy clock for imx6q. > > - During the runtime, we don't need to control it. > > > Turning it into a dummy clock, you will have no way to maintain the > use count. It could possibly cause parent clock be turned off while > usbphy is in use. > > Let's try to find some other way. > I will keep the phyclk unchanged, but just let it control a reserved bit In that case, the clk framework will know USB is using PLL. Meanwhile, the real USB PHY clk gate will only be open one time at phy driver's probe. > Shawn -- 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: [BUILD BREAK] usb: gadget: fsl_mxc_udc can't compile on current v3.8-rc3
> > > > > > Some recent patch has caused fsl_mxc_udc.c driver to fail compilation > > > because it can't find anymore. > > > > > > I would like this to be fixed still during this -rc cycle. > > > > Me too, who's sending a patch? :) > > Hi Peter, > > Who is currently working on the i.mx USB? > I am working on it, but there are two versions, this one and chipidea's. Anyway, I will send a patch to fix this problem. > Regards, > Leo -- 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: [BUILD BREAK] usb: gadget: fsl_mxc_udc can't compile on current v3.8-rc3
> > > > > > > I am working on it, but there are two versions, this one and chipidea's. > > > > Anyway, I will send a patch to fix this problem. > > if you're already using chipidea, then send me a patch removing this > driver and focus your effort on chipidea. > Added Sascha Now, not all of FSL i.mx USB can move to use chipidea due to some platform and USB PHY problem. > -- > balbi -- 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: chipidea: imx: Add system suspend/resume API
>
> On Fri, Jan 18, 2013 at 10:50:28AM +0800, Peter Chen wrote:
> > +#ifdef CONFIG_PM
> > +static int ci13xxx_imx_suspend(struct device *dev)
> > +{
> > + struct ci13xxx_imx_data *data =
> > + platform_get_drvdata(to_platform_device(dev));
>
> Is there a reason not to use dev_get_drvdata() here? Hint:
>
It is my careless, I will change. Thanks.
> #define to_platform_device(x) container_of((x), struct platform_device,
> dev)
>
> static inline void *platform_get_drvdata(const struct platform_device
> *pdev)
> {
> return dev_get_drvdata(&pdev->dev);
> }
>
> So, you're going from a dev => platform device => the same dev again.
>
> It's perfectly valid to use dev_get_drvdata() here because we're not
> going
> to separate these two (it makes absolutely no sense to.)
--
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: Root hub autosusend delay
> Hi Ming, I also find this problem at my platform. > (At chipidea controller, the resume signal will be ended about 21ms > automatically) > My test is echo auto to keyboard (hub in int), then press key to wakeup it, the system doesn't enter suspend. > Neither delay 30ms at hub_events, nor revert your patch > (596d789a211d134dc5f94d1e5957248c204ef850) can work. > > This problem may exist after my patch > (6273f1810f95f4deeb2f0d6810f301726ad32308) > > The first log is for this problem, the second one without problem > due to console output occupy much times and resume bit is cleared. > > ---First Log > mx_usb 2184000.usb: Wakeup interrupt occurs imx_controller_resume > mxs_phy 20c9000.usbphy: Connect line between phy and controller > ci_hdrc ci_hdrc.0: port 1 remote wakeup > imx_usb 2184000.usb: at the end of imx_controller_resume, ret = 0 > usb usb1: usb wakeup-resume > usb usb1: usb auto-resume > ci_hdrc ci_hdrc.0: resume root hub > hub 1-0:1.0: hub_resume > ci_hdrc ci_hdrc.0: GetStatus port:1 status 100014c5 8 ACK POWER sig=k > SUSPEND RESUME PE CONNECT > hub 1-0:1.0: port 1: status 0107 change > hub 1-0:1.0: state 7 ports 1 chg evt > hub 1-0:1.0: state 7 ports 1 chg evt > hub 1-0:1.0: hub_suspend This hub_suspend is expected or not? If it is expected, below error may not be avoided, since usb_port_resume still doesn't be called, and Get_PORT_STATUS is either to be called, then the ehci->resuming_ports will not be cleared. > usb usb1: bus auto-suspend, wakeup 1 > ci_hdrc ci_hdrc.0: suspend root hub > ci_hdrc ci_hdrc.0: suspend failed because a port is resuming > usb usb1: bus suspend fail, err -16 -- 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: Root hub autosusend delay
Add rpm trace log for log 1, the coming behavior is expected, the usb subsystem will enters low power mode again. root@freescale ~$ cat /sys/kernel/debug/tracing/trace # tracer: nop # # entries-in-buffer/entries-written: 156/156 #P:4 # # _-=> irqs-off # / _=> need-resched #| / _---=> hardirq/softirq #|| / _--=> preempt-depth #||| / delay # TASK-PID CPU# TIMESTAMP FUNCTION # | | | | | -0 [000] d.h.48.691373: rpm_resume: ci_hdrc.0 flags-5 cnt-1 dep-0 auto-1 p-0 irq-0 child-0 -0 [000] dNh.48.691389: rpm_return_int: rpm_resume+0x94/0x750:ci_hdrc.0 ret=0 kworker/0:1-53[000] d...48.691415: rpm_resume: ci_hdrc.0 flags-2 cnt-1 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.691418: rpm_resume: 2184000.usb flags-0 cnt-1 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.703000: rpm_idle: ci_hdrc.0 flags-5 cnt-0 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.703004: rpm_return_int: rpm_idle+0x4c/0x228:ci_hdrc.0 ret=-11 kworker/0:1-53[000] d...48.708896: rpm_idle: 2184000.usb flags-1 cnt-1 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.708899: rpm_return_int: rpm_idle+0x4c/0x228:2184000.usb ret=-11 kworker/0:1-53[000] d...48.708902: rpm_idle: 210.aips-bus flags-5 cnt-0 dep-1 auto-1 p-0 irq-0 child-1 kworker/0:1-53[000] d...48.708903: rpm_return_int: rpm_idle+0x4c/0x228:210.aips-bus ret=-13 kworker/0:1-53[000] d...48.708905: rpm_return_int: rpm_resume+0x94/0x750:2184000.usb ret=0 kworker/0:1-53[000] d...48.708908: rpm_idle: ci_hdrc.0 flags-1 cnt-0 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.708911: rpm_suspend: ci_hdrc.0 flags-1 cnt-0 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.708913: rpm_idle: 2184000.usb flags-1 cnt-1 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.708915: rpm_return_int: rpm_idle+0x4c/0x228:2184000.usb ret=-11 kworker/0:1-53[000] d...48.708917: rpm_return_int: rpm_suspend+0x370/0x6a0:ci_hdrc.0 ret=0 kworker/0:1-53[000] d...48.708918: rpm_return_int: rpm_idle+0x4c/0x228:ci_hdrc.0 ret=0 kworker/0:1-53[000] d...48.708920: rpm_idle: 2184000.usb flags-5 cnt-0 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.708923: rpm_return_int: rpm_idle+0x4c/0x228:2184000.usb ret=0 kworker/0:1-53[000] d...48.708925: rpm_return_int: rpm_resume+0x94/0x750:ci_hdrc.0 ret=0 kworker/0:1-53[000] d...48.708944: rpm_resume: usb1 flags-4 cnt-1 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.708947: rpm_resume: ci_hdrc.0 flags-0 cnt-1 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.708949: rpm_idle: ci_hdrc.0 flags-1 cnt-1 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.708951: rpm_return_int: rpm_idle+0x4c/0x228:ci_hdrc.0 ret=-11 kworker/0:1-53[000] d...48.708952: rpm_return_int: rpm_resume+0x94/0x750:ci_hdrc.0 ret=1 kworker/0:1-53[000] dN..48.710126: rpm_idle: usb1 flags-1 cnt-1 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] dN..48.710130: rpm_return_int: rpm_idle+0x4c/0x228:usb1 ret=-11 kworker/0:1-53[000] dN..48.710132: rpm_idle: ci_hdrc.0 flags-5 cnt-0 dep-0 auto-1 p-0 irq-0 child-1 kworker/0:1-53[000] dN..48.710133: rpm_return_int: rpm_idle+0x4c/0x228:ci_hdrc.0 ret=-16 kworker/0:1-53[000] dN..48.710135: rpm_return_int: rpm_resume+0x94/0x750:usb1 ret=0 kworker/0:1-53[000] d...48.710168: rpm_suspend: usb1 flags-c cnt-0 dep-0 auto-1 p-0 irq-0 child-0 kworker/0:1-53[000] d...48.710172: rpm_return_int: rpm_suspend+0x370/0x6a0:usb1 ret=0 kworker/0:1-53[000] dN..48.710181: rpm_idle: 2184000.usb flags-2 cnt-0 dep-0 auto-1 p-0 irq-0 child-1 kworker/0:1-53[000] dN..48.710183: rpm_return_int: rpm_idle+0x4c/0x228:2184000.usb ret=-16 khubd-27[000] d...48.710192: rpm_resume: 1-0:1.0 flags-4 cnt-2 dep-0 auto-1 p-0 irq-0 child-0 khubd-27[000] d...48.710195: rpm_idle: 1-0:1.0 flags-1 cnt-2 dep-0 auto-1 p-0 irq-0 child-0 khubd-27[000] d...48.710197: rpm_return_int: rpm_idle+0x4c/0x228:1-0:1.0 ret=-11 khubd-27[000] d...48.710199: rpm_return_int: rpm_resume+0x94/0x750:1-0:1.0 ret=1 khubd-27[000] d...48.710204: rpm_idle: 1-0:1.0 flags-4 cnt-0 dep-0 auto-1 p-0 irq-0 child-0 khubd-27[000] d...48.710205: rpm_suspend: 1-0:1.0 flags-4 cnt-0 dep-0 auto-1 p-0 irq-0 child-
RE: USB subsystem stops working
>
> > I have post one patch which may remove the message, see link[2].
>
> I don't think sleeping is the right answer. For example, at the same
> time as the resume there could be a new device plugged in.
>
> What we really want to do is prevent the root hub from autosuspending
> while the resume signal is active. One possibility is to have the HCD
> call pm_runtime_get_noresume. Can you think of anything better?
>
I also agree with no auto suspend occur during the resume signal is active.
But one think I still can't understand that you add
usb_autopm_get_interface_no_resume(
to_usb_interface(hub->intfdev));
the purpose should be avoid hub's auto-suspend.
But why the hub's suspend still occurs after calling it, see below log?
---LOG
usb usb1: usb wakeup-resume
usb usb1: usb auto-resume
ci_hdrc ci_hdrc.0: resume root hub
hub 1-0:1.0: hub_resume
usb usb1: wait for ports resuming over
hub 1-0:1.0: port 1: status 0107 change
process
hub 1-0:1.0: state 7 ports 1 chg evt
hub 1-0:1.0: hub_suspend
usb usb1: bus auto-suspend, wakeup 1
ci_hdrc ci_hdrc.0: suspend root hub
ci_hdrc ci_hdrc.0: suspend failed because a port is resuming
usb usb1: bus suspend fail, err -16
hub 1-0:1.0: hub_resume
usb usb1: wait for ports resuming over
ci_hdrc ci_hdrc.0: GetStatus port:1 status 10001805 8 ACK POWER sig=j PE
CONNECT
hub 1-0:1.0: port 1: status 0103 change 0004
process
hub 1-0:1.0: state 7 ports 1 chg 0002 evt
ci_hdrc ci_hdrc.0: GetStatus port:1 status 10001805 8 ACK POWER sig=j PE
CONNECT
usb 1-1: usb wakeup-resume
usb 1-1: finish resume
hub 1-1:1.0: hub_resume
-diff of
kick_khubd---
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -635,6 +635,12 @@ static void kick_khubd(struct usb_hub *hub)
if (!hub->disconnected && list_empty(&hub->event_list)) {
list_add_tail(&hub->event_list, &hub_event_list);
+ if (in_interrupt())
+ printk("in_int\n");
+ else if (in_irq())
+ printk("in_irq\n");
+ else
+ printk("process\n");
/* Suppress autosuspend until khubd runs */
usb_autopm_get_interface_no_resume(
to_usb_interface(hub->intfdev));
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
RE: Need review for chipidea driver
Sorry, forget to cc the list > > Hi Greg, > > Alex has no response for chipidea driver review for long time > (more than 1 month), below are some links about patchset. > Does anyone else can help do it? > > http://marc.info/?t=13540330436&r=1&w=2 > http://marc.info/?l=linux-usb&m=135873754825186&w=2 > http://marc.info/?t=13587354211&r=1&w=2 > http://marc.info/?l=linux-usb&m=135691704907220&w=2 > > -- > > Best Regards, > Peter Chen -- 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 v8 8/8] usb: chipidea: udc: fix the oops when plugs in usb cable after rmmod gadget
> > > > } else {
> > > > + if (ci->driver)
> > > > + ci->driver->disconnect(&ci->gadget);
> > >
> > > This looks wrong. Why do you need to call ->disconnect() here ?
> >
> > When we disconnect usb cable from the host, we need to notify gadget
> module
> > disconnection occurs, the gadget module may not be unloaded in future.
>
Sorry, can you point that?
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
RE: About USBADRA bit at DEVICEADDR for chipidea driver
> > > > For USB 3.0 host CV test, the host sends GET_DESCRIPTOR very quick > (<500us) > > once it accept the status of SET_ADDRESS > > The USB 2.0 spec says the recovery period after the status phase of > SetAddress is 2ms. That should be enough to process the transfer > completion interrupt, shouldn't it? > Why is USB 3 CV violating this and why should we care? I guess, if we > really want to, we can make USBADRA usage conditional, BUT something > tells me that there will be more arbitrary timing expectations then that > we won't necessarily be able to meet. > I will investigate it more after finishing several chipidea related patches. > Regards, > -- > Alex -- 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] USB: enable "power/wakeup" to control remote wakeup in the runtime suspend
> 2. If an internal USB port is suspended with remote wakeup disabled, > power off the port. Add code to the USB core to ignore the device > disconnect in this special case, so the driver thinks the device is > still suspended. Issue a reset-resume when the driver wants to resume > the device. > The problem is we can't switch remote wakeup enable <--> disable at runtime now. An /sys/../power/ entry to change intf->needs_remote_wakeup may be needed. Or a remote wakeup featured device plugs into that port will cause it will be not remote wakeup featured as SET_FEATURE for remote wakeup will not be called. Best regards, 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
RE: [PATCH v3 1/3] USB: chipidea: add imx usbmisc support
> > > > <&usbmisc 0> would then mean port 0 of the usbmisc device. > I didn't add the restriction that a usbmisc driver must have a usbmisc > device. I'm not sure whether all SoC and future SoC can be look as > a device. > > Peter, do you have any idea? > I have not followed this usbmisc design, I just list some facts at i.mx USB controller: Sigmatel-derived SoCs (i.mx23, i.mx28) have no this register region, all phy controls are through PHY register. Other freescale SoCs have this usbmisc register region to control phy and tune some signal quality. This register region is from another 0x800 or (last controller base address + 0x200) > Thanks > Richard > > > > If the usbmisc property exists, you can return -EPROBE_DEFER until it > is > > available. If it doesn't exist, you just continue without usbmisc > > support (i.MX28) > > > > Sascha > > > > > > -- > > 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: Does larger than 32 bits report size is supported at HID?
> > Which one? The MX28 p-o-s? Skimming through it's sources, I already used > two > puking bags, more to be filled tho. > mx23,mx28, maybe mx6x is the same. -- 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 1/1] usb: phy: move usb_otg_state from struct usb_phy to struct usb_otg
> > looks correct. Now that struct usb_otg should handle all otg details, it > makes sense to put the state there. The problem is that a few of the > states aren't really OTG specific. I think the only two otg-specific > states are A_DEVICE and B_HOST... > According to USB_OTG_and_EH_2-0-final_plus_errata_and_ecn_20110714 - final.pdf Chapter7.1, 7,2. The states at enum usb_otg_state are all OTG state. If you think others (except for A_DEVICE and B_HOST) are not otg-specific states, then what are they? Thanks Peter > So either move this, or rename he enumeration accordingly, dunno. I > guess it would work either way. -- 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 1/1] usb: phy: move usb_otg_state from struct usb_phy to struct usb_otg
> > > > > According to USB_OTG_and_EH_2-0-final_plus_errata_and_ecn_20110714 - > final.pdf > > Chapter7.1, 7,2. The states at enum usb_otg_state are all OTG state. > > > > If you think others (except for A_DEVICE and B_HOST) are not otg- > specific states, > > then what are they? > > granted, they were defined on the OTG spec, but if you think about them, > every peripheral usb controller starts on B_IDLE state. > > The OTG spec also says that any device can start SRP and any host can > reply to that, not only OTG-capable devices; which means that B_SRP_INIT > also applies to any device. > > B_PERIPHERAL applies to any device which gets enumerated. > > B_WAIT_ACON also applies to any device; it's that small timeframe where > your device is ready and just waits for host. > > A_IDLE it's when your host is fully initialized but has no > devices attached to it. > > A_WAIT_VRISE and A_WAIT_VFALL is applicable to any hosts which can > control their charge pumps. > > A_WAIT_BCON is applicable to any host > > A_HOST is the normal operation of a host when it enumerated a device > > A_SUSPEND applies to any host. It's when it drives bus suspend > signalling on the bus > > A_VBUS_ERR is that state when you just caught overcurrent. > So, the others are usb_state not usb_otg_state. Then, when we generalize otg driver, what kinds of **states** do you plan to use to stand for otg state machine? Besides, if we name something different with spec, does it will confuse the users? > -- > balbi -- 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 1/1] usb: phy: move usb_otg_state from struct usb_phy to struct usb_otg
> > So, the others are usb_state not usb_otg_state. Then, when we
> generalize
> > otg driver, what kinds of **states** do you plan to use to stand for
> otg
> > state machine? Besides, if we name something different with spec, does
> it
> > will confuse the users?
>
> We could have just a single enum usb_state defined something like:
>
> enum usb_otg_state {
> USB_STATE_UNDEFINED = 0,
>
> /* single-role peripheral, and dual-role default-b */
> USB_STATE_B_IDLE,
> USB_STATE_B_SRP_INIT,
> USB_STATE_B_PERIPHERAL,
>
> /* single-role host, and dual-role default-a */
> USB_STATE_A_IDLE,
> USB_STATE_A_WAIT_VRISE,
> USB_STATE_A_WAIT_BCON,
> USB_STATE_A_HOST,
> USB_STATE_A_SUSPEND,
> USB_STATE_A_WAIT_VFALL,
> USB_STATE_A_VBUS_ERR,
>
> #ifdef CONFIG_USB_OTG
> /* extra dual-role default-b states */
> USB_STATE_B_WAIT_ACON,
> USB_STATE_B_HOST,
> USB_STATE_A_PERIPHERAL,
> #endif /* CONFIG_USB_OTG
> };
>
> or something similar. Then we shouldn't allow anyone to access
> phy->state directly, so we would need:
>
> static inline void usb_phy_set_state(struct usb_phy *phy,
> enum usb_state state)
> {
> phy->state = state;
> }
>
> static inline enum usb_state usb_phy_get_state(struct usb_phy *phy)
> {
> return phy->state;
> }
>
Then, at later general OTG driver, we need to know struct usb_phy *phy,
but how can we get it at OTG driver?
I am a little confused by using phy->state to stand for usb_state that I think
there is no relationship between usb_state with USB PHY.
> --
> balbi
--
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 1/1] usb: phy: move usb_otg_state from struct usb_phy to struct usb_otg
> > > I am a little confused by using phy->state to stand for usb_state that > I think > > there is no relationship between usb_state with USB PHY. > > well, there's no relationship between usb_state and OTG. The state isn't > OTG-specific, it's USB specific. This is a difficult detail to find the > proper owner, but I don't think we should tie the state to OTG, because > systems without OTG wouldn't be able to track their states too. > I mean USB PHY. I mean these USB specific states is no relationship with USB PHY. In my mind, the system without OTG but using struct usb_phy can still track their states. One thing I am always puzzled of current code is the OTG should be no relationship with USB PHY. The system without OTG but has USB device or host only function should still own USB PHY. > -- > balbi -- 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 1/1] usb: phy: move usb_otg_state from struct usb_phy to struct usb_otg
> > > > In my mind, the system without OTG but using struct usb_phy can still > > track their states. > > > > One thing I am always puzzled of current code is the OTG should be no > > relationship with USB PHY. > > The system without OTG but has USB device or host only function should > > still own USB PHY. > > you don't need to tell me that. We're in the process of redesigning the > PHY and OTG apis exactly for that. This is basically legacy from the > original PHY patch (back from 2.6.18 or something) which added PHY > support considering only OTG systems. > > At that time we only had these plug&play PHYs and likely no standard > host controller on an embedded system. > > Anyway, it will take some time to finish converting everything, help's > welcome though. > OK, glad to hear that. My RFC and questions are all for eliminating the relationship between USB PHY and USB OTG, and find some good ways to convert everything. > -- > balbi -- 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: mxs-phy on i.MX233 not enumerating
> On 8/13/12, Felipe Balbi wrote: > > > Will there be another version for this or is this final ? Won't this > > cause any regressions to other boards ? > > The proposed patch is not meant to be a final one. > > We still need to figure out the proper way to handle HOSTDISCONDETECT > for all i.mx SoCs, including mx23, and would like to get some feedback > from the FSL guys in Cc. > According to IC guys, the logic of handling HOSTDISCONDETECT is the same between i.mx28 and i.mx23. > Thanks, > > Fabio Estevam
RE: mxs-phy on i.MX233 not enumerating
Add IC guy
Best regards,
Peter Chen
> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Tuesday, August 14, 2012 10:01 AM
> To: Chen Peter-B29397
> Cc: ba...@ti.com; Sean Cross; linux-usb@vger.kernel.org; Estevam Fabio-
> R49496; Zhao Richard-B20223; Shawn Guo; Marek Vasut; Li Frank-B20596
> Subject: Re: mxs-phy on i.MX233 not enumerating
>
> Peter,
>
> On 8/13/12, Chen Peter-B29397 wrote:
>
> > According to IC guys, the logic of handling HOSTDISCONDETECT is the
> same
> > between i.mx28 and i.mx23.
>
> As pointed out by Sean, on mx23 reference manual we have the following
> text describing HOSTDISCONDETECT:
>
> "Due to a on chip issue (Errata #2791), software must
> pay attention to when to assert the HOSTDISCONDETECT bit description
> has the following:
>
> ENHOSTDISCONDETECT bit in HW_USBPHY_CTRL register:
> 1. Only set HW_USBPHY_CTRL.ENHOSTDISCONDETECT
> during high speed host mode.
> 2. Do not set HW_USBPHY_CTRL.ENHOSTDISCONDETECT
> during the reset and speed negotiation period.
> 3. Do not set HW_USBPHY_CTRL.ENHOSTDISCONDETECT
> during host suspend/resume sequence."
>
> and the same is not present on mx28 reference manual.
>
> How can we make sure we are not violating the point 2 above?
>
> Thanks,
>
> Fabio Estevam
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
RE: mxs-phy on i.MX233 not enumerating
>
> > According to IC guys, the logic of handling HOSTDISCONDETECT is the
> same
> > between i.mx28 and i.mx23.
>
> As pointed out by Sean, on mx23 reference manual we have the following
> text describing HOSTDISCONDETECT:
>
> "Due to a on chip issue (Errata #2791), software must
> pay attention to when to assert the HOSTDISCONDETECT bit description
> has the following:
>
> ENHOSTDISCONDETECT bit in HW_USBPHY_CTRL register:
> 1. Only set HW_USBPHY_CTRL.ENHOSTDISCONDETECT
> during high speed host mode.
> 2. Do not set HW_USBPHY_CTRL.ENHOSTDISCONDETECT
> during the reset and speed negotiation period.
> 3. Do not set HW_USBPHY_CTRL.ENHOSTDISCONDETECT
> during host suspend/resume sequence."
>
> and the same is not present on mx28 reference manual.
>
> How can we make sure we are not violating the point 2 above?
>
It is the same with i.mx28. I think Richard's patch should not
violate point 2. Richard, can you confirm it?
> Thanks,
>
> Fabio Estevam
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
RE: mxs-phy on i.MX233 not enumerating
> > > > > It is the same with i.mx28. > If they're same, imx23 should be ok. The mxs_phy driver pass test > on mx28. > > I think Richard's patch should not > > violate point 2. Richard, can you confirm it? > You mean I should not set the bit in mxs_phy_on_connect? > or mxs_phy_on_connect is called in wrong place? > HW_USBPHY_CTRL.ENHOSTDISCONDETECT should be set after bus reset is finished. > Thanks > Richard > > > > > Thanks, > > > > > > Fabio Estevam > > -- 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/1] usb: hcd: mark controller's suspend routine as async
> > > Host controller's suspend should be executed later than root hub's, > > or the hang may occur when root hub try to visit some registers > > but host controller's suspend close the related clocks. > > Have you ever seen this happen? It should not be possible, because the > PM core is careful not to suspend a parent before its child devices. > > > Mark controller's suspend as async can make sure it is executed > > later than root hub's as host controller is the parent of root hub. > > We want to be careful about this sort of thing. Most of the async > suspend notations are for devices on a particular kind of bus, but the > notation you want to add would apply to all host controllers on any > bus. > > If you really do see a host controller suspending before its root hub, > this indicates there is a bug in the PM core. The bug should be fixed; > you shouldn't ignore it by marking all host controllers for async > suspend. > I am apologized that it does not occur after my experiments, I check this problem because one of my colleagues assumes its possibilities, and I thought dpm_wait_for_children at __device_suspend will not wait children if the dev is not async_suspend. In fact, at dpm_wait_for_children, the parent will wait if one of its child is async_suspend and this child's suspend is not finished. > 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
Potential fsg->state problem at file_storage.c
Hi Alan, One of my colleagues reports a problem that the enumeration will fail if the storage has some problems. I can re-produce this problem if I add a big delay (like 500ms) at handle_exception, at condition FSG_STATE_DISCONNECT, please see below code: 3050 case FSG_STATE_DISCONNECT: 3051 for (i = 0; i < fsg->nluns; ++i) 3052 fsg_lun_fsync_sub(fsg->luns + i); 3053 mdelay(500); 3054 do_set_config(fsg, 0); // Unconfigured state 3055 break; When the problem occurs, the call sequence like below: reset interrupt-> fsg->disconnect-> handle_exception reset interrupt-> fsg->disconnect ... udc setup package -> USB_REQ_SET_CONFIGURATION -> raise_exception(fsg, FSG_STATE_CONFIG_CHANGE); The reason why this problem occurs is that the first fsg_lun_fsync_sub(fsg->luns + i) consumes too much times, the fsg->state is changed by raise_exception (2nd reset interrupt), but the handle_exception (2nd reset interrupt) is not called after raise_exception(fsg, FSG_STATE_CONFIG_CHANGE) is called. The fsg->state is still FSG_STATE_DISCONNECT when raise_exception(fsg, FSG_STATE_CONFIG_CHANGE) is called, Since FSG_STATE_DISCONNECT > FSG_STATE_CONFIG_CHANGE, the SET_CONFIGURATION will never be handled, the enumeration will be failed. A workaround for this problem is: 3050 case FSG_STATE_DISCONNECT: 3051 if (fsg->config != 0) 3052 for (i = 0; i < fsg->nluns; ++i) 3053 fsg_lun_fsync_sub(fsg->luns + i); 3054 do_set_config(fsg, 0); // Unconfigured state 3055 break; But it can't handle the problem that the reset occurs during the transfer due to some timeout. (bus reset -> bus reset -> SET_CONFIGURATION) Best regards, Peter Chen MAD Linux BSP Team Freescale Semiconductor Ltd. -- 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: Potential fsg->state problem at file_storage.c
> > that sounds like a plan :-) Tomorrow I can send a series of patches > starting off the split, then people can just send in more patches for > other controllers as we go. cheers > I agree with this change > -- > balbi -- 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/4] g_file_storage: implement ->reset method
>
> drivers/usb/gadget/f_mass_storage.c |5 +++--
> drivers/usb/gadget/file_storage.c | 20
> drivers/usb/gadget/storage_common.c |3 ++-
> 3 files changed, 21 insertions(+), 7 deletions(-)
>
> Index: usb-3.5/drivers/usb/gadget/storage_common.c
> ===
> --- usb-3.5.orig/drivers/usb/gadget/storage_common.c
> +++ usb-3.5/drivers/usb/gadget/storage_common.c
> @@ -279,9 +279,10 @@ enum fsg_state {
>
> FSG_STATE_IDLE = 0,
> FSG_STATE_ABORT_BULK_OUT,
> - FSG_STATE_RESET,
> + FSG_STATE_CLASS_RESET,
> FSG_STATE_INTERFACE_CHANGE,
> FSG_STATE_CONFIG_CHANGE,
> + FSG_STATE_RESET,
> FSG_STATE_DISCONNECT,
> FSG_STATE_EXIT,
> FSG_STATE_TERMINATED
> Index: usb-3.5/drivers/usb/gadget/file_storage.c
> ===
> --- usb-3.5.orig/drivers/usb/gadget/file_storage.c
> +++ usb-3.5/drivers/usb/gadget/file_storage.c
> @@ -676,10 +676,18 @@ static void fsg_disconnect(struct usb_ga
> {
> struct fsg_dev *fsg = get_gadget_data(gadget);
>
> - DBG(fsg, "disconnect or port reset\n");
> + DBG(fsg, "disconnect\n");
> raise_exception(fsg, FSG_STATE_DISCONNECT);
> }
>
> +static void fsg_reset(struct usb_gadget *gadget)
> +{
> + struct fsg_dev *fsg = get_gadget_data(gadget);
> +
> + DBG(fsg, "port reset\n");
> + raise_exception(fsg, FSG_STATE_RESET);
> +}
> +
>
> static int ep0_queue(struct fsg_dev *fsg)
> {
> @@ -816,7 +824,7 @@ static void received_cbi_adsc(struct fsg
> /* Raise an exception to stop the current operation
>* and reinitialize our state. */
> DBG(fsg, "cbi reset request\n");
> - raise_exception(fsg, FSG_STATE_RESET);
> + raise_exception(fsg, FSG_STATE_CLASS_RESET);
> return;
> }
>
> @@ -867,7 +875,7 @@ static int class_setup_req(struct fsg_de
> /* Raise an exception to stop the current operation
>* and reinitialize our state. */
> DBG(fsg, "bulk reset request\n");
> - raise_exception(fsg, FSG_STATE_RESET);
> + raise_exception(fsg, FSG_STATE_CLASS_RESET);
> value = DELAYED_STATUS;
> break;
>
> @@ -3006,7 +3014,7 @@ static void handle_exception(struct fsg_
> spin_unlock_irq(&fsg->lock);
> break;
>
> - case FSG_STATE_RESET:
> + case FSG_STATE_CLASS_RESET:
> /* In case we were forced against our will to halt a
>* bulk endpoint, clear the halt now. (The SuperH UDC
>* requires this.) */
> @@ -3050,6 +3058,9 @@ static void handle_exception(struct fsg_
> case FSG_STATE_DISCONNECT:
> for (i = 0; i < fsg->nluns; ++i)
> fsg_lun_fsync_sub(fsg->luns + i);
> + /* FALL THROUGH */
> +
> + case FSG_STATE_RESET:
> do_set_config(fsg, 0); // Unconfigured state
> break;
>
> @@ -3608,6 +3619,7 @@ static struct usb_gadget_driver fsg_dri
> .function = (char *) fsg_string_product,
> .unbind = fsg_unbind,
> .disconnect = fsg_disconnect,
> + .reset = fsg_reset,
> .setup = fsg_setup,
> .suspend= fsg_suspend,
> .resume = fsg_resume,
> Index: usb-3.5/drivers/usb/gadget/f_mass_storage.c
> ===
> --- usb-3.5.orig/drivers/usb/gadget/f_mass_storage.c
> +++ usb-3.5/drivers/usb/gadget/f_mass_storage.c
> @@ -554,7 +554,7 @@ static int fsg_setup(struct usb_function
>* and reinitialize our state.
>*/
> DBG(fsg, "bulk reset request\n");
> - raise_exception(fsg->common, FSG_STATE_RESET);
> + raise_exception(fsg->common, FSG_STATE_CLASS_RESET);
> return DELAYED_STATUS;
>
> case US_BULK_GET_MAX_LUN:
> @@ -2470,7 +2470,7 @@ static void handle_exception(struct fsg_
> spin_unlock_irq(&common->lock);
> break;
>
> - case FSG_STATE_RESET:
> + case FSG_STATE_CLASS_RESET:
> /*
> * In case we were forced against our will to halt a
>* bulk endpoint, clear the halt now. (The SuperH UDC
> @@ -2511,6 +2511,7 @@ static void handle_ex
RE: [PATCH 1/4] g_file_storage: implement ->reset method
> > > Unfortunately the driver already contains an FSG_STATE_RESET symbol. > > > The patch renames it to FSG_STATE_CLASS_RESET, because it refers to a > > > class-specific reset event rather than a general USB port reset, and > > > uses FSG_STATE_RESET for port resets. The g_mass_storage driver > > > shares a source file with g_file_storage; therefore it had to be > > > modified accordingly. > > > > > > Signed-off-by: Alan Stern > > > Reported-by: Chen Peter-B29397 > > > > Looks good to me. > > > > Even though it makes me wonder whether f_mass_storage isn't missing > > something in its original code. > > I wondered about that too. The difference is that f_mass_storage uses > the composite framework. How does composite handle disconnects and > port resets? The framework might need to be modified to pass these > events down to the function drivers in separate ways. > f_mass_storage does not have disconnect callback, and do noop at FSG_STATE_DISCONNECT case. I can't find where f_mass_storage calls fsg_lun_fsync_sub when disconnect occurs. > 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: Potential fsg->state problem at file_storage.c
> > On Wed, Aug 15, 2012 at 09:49:55AM +, Chen Peter-B29397 wrote: > > Hi Alan, > Hi Peter, > > > One of my colleagues reports a problem that the enumeration will fail > if > > the storage has some problems. > > A silghtly different question: do you think you can check if this kind of > problem is also in the tcm_usb_gadget storage driver? > I have checked, but I can't understand tcm code well, so I don't know this problem exists or not. > Sebastian -- 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: gadget: fsl_udc_core: do not immediatly prime STATUS for IN xfer
> > Because the fsl_udc_core driver shares one 'status_req' object for the > complete ep0 control transfer, it is not possible to prime the final > STATUS phase immediately after the IN transaction. E.g. ch9getstatus() > executed: > > | req = udc->status_req; > | ... > | list_add_tail(&req->queue, &ep->queue); > | if (ep0_prime_status(udc, EP_DIR_OUT)) > | > | struct fsl_req *req = udc->status_req; > | list_add_tail(&req->queue, &ep->queue); > > which corrupts the ep->queue list by inserting 'status_req' twice. This > causes a kernel oops e.g. when 'lsusb -v' is executed on the host. > > Patch delays the final 'ep0_prime_status(udc, EP_DIR_OUT))' by moving it > into the ep0 completion handler. > Enrico, thanks for pointing this problem. As "prime STATUS phase immediately after the IN transaction" is followed USB 2.0 spec, to fix this problem, it is better to add data_req for ep0. In fact, it is already at FSL i.mx internal code, just still not mainlined. 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
RE: [PATCH] usb: gadget: fsl_udc_core: remove mapped flag
> @@ -195,14 +195,13 @@ static void done(struct fsl_ep *ep, struct fsl_req
> *req, int status)
> dma_pool_free(udc->td_pool, curr_td, curr_td->td_dma);
> }
>
> - if (req->mapped) {
> + if (req->req.dma != DMA_ADDR_INVALID) {
> dma_unmap_single(ep->udc->gadget.dev.parent,
> req->req.dma, req->req.length,
> ep_is_in(ep)
> ? DMA_TO_DEVICE
> : DMA_FROM_DEVICE);
> req->req.dma = DMA_ADDR_INVALID;
> - req->mapped = 0;
> } else
> dma_sync_single_for_cpu(ep->udc->gadget.dev.parent,
If the class driver has already mapped this address, the req->req.dma is not
DMA_ADDR_INVALID either, in this case, the dma_sync_single_for_cpu is enough.
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
RE: [PATCH v2 06/11] USB: mxs-phy: add basic otg support
> diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
> index c1a67cb..6a03e97 100644
> --- a/drivers/usb/otg/mxs-phy.c
> +++ b/drivers/usb/otg/mxs-phy.c
> @@ -97,12 +97,24 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy,
> int port)
> return 0;
> }
>
> +static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
> +{
> + return 0;
> +}
> +
> +static int mxs_phy_set_peripheral(struct usb_otg *otg,
> + struct usb_gadget *gadget)
> +{
> + return 0;
> +}
> +
> static int mxs_phy_probe(struct platform_device *pdev)
> {
> struct resource *res;
> void __iomem *base;
> struct clk *clk;
> struct mxs_phy *mxs_phy;
> + struct usb_otg *otg;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!res) {
> @@ -139,6 +151,15 @@ static int mxs_phy_probe(struct platform_device
> *pdev)
>
> mxs_phy->clk = clk;
>
> + otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);
> + if (!otg)
> + return -ENOMEM;
> + otg->phy = &mxs_phy->phy;
> + otg->set_host = mxs_phy_set_host;
> + otg->set_peripheral = mxs_phy_set_peripheral;
> +
> + mxs_phy->phy.otg = otg;
> +
Put otg struct at PHY driver is not a good practice.
Only OTG driver who handles ID interrupt cases about host and gadget struct.
PHY is a transceiver, it not cares otg, host and device at all.
> platform_set_drvdata(pdev, &mxs_phy->phy);
>
> return 0;
> --
> 1.7.9.5
--
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] drivers: phy: add generic PHY framework
> > The PHY framework provides a set of API's for the PHY drivers to > create/remove a PHY and the PHY users to obtain a reference to the PHY > using or without using phandle. If the PHY users has to obtain a > reference to > the PHY without using phandle, the platform specfic intialization code > (say > from board file) should have already called phy_bind with the binding > information. The binding information consists of phy's device name, phy > user device name and an index. The index is used when the same phy user > binds to mulitple phys. > What's an example of "the same phy user binds to multiple phys"? I only remembered that Felipe said there are two phy users for one single phy at omap5 that is both usb3 and sata uses the same phy. -- 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: Chipidea usb otg support for IMX/MXS (device functionality)
> > Peter, I dunno if you are already aware of it, but the USB peripheral > mode hangs > on MX233. It's easy to replicate for example if you try to run CDC > ethernet over > the USB peripheral mode, then telnet into the board and run "dmesg" . > This will > trigger a "larger" data transfer which will make the UDC driver hang. > This > doesn't happen on MX28 so it must be some MX233-specific thing. > Marek, Have you tried below? http://marc.info/?l=linux-usb&m=136537318510847&w=2 It may not the USB itself problem, the mx23's usb is almost the same with mx28's. Best regards, 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
RE: Chipidea usb otg support for IMX/MXS (device functionality)
> Hi Peter, > > > > Peter, I dunno if you are already aware of it, but the USB peripheral > > > mode hangs > > > on MX233. It's easy to replicate for example if you try to run CDC > > > ethernet over > > > the USB peripheral mode, then telnet into the board and run "dmesg" . > > > This will > > > trigger a "larger" data transfer which will make the UDC driver hang. > > > This > > > doesn't happen on MX28 so it must be some MX233-specific thing. > > > > Marek, Have you tried below? > > > > http://marc.info/?l=linux-usb&m=136537318510847&w=2 > > > > It may not the USB itself problem, the mx23's usb is almost > > the same with mx28's. > > This also happens on a different MX23-based board for me. This other > board I > tried has mDDR DRAM and the DRAM is operating correctly. The 96MHz thing > people > complained about in the Olimex forum is resolved now. > Add shawn. Marek, have you tried mx23 evk? Shawn, marek reported the udc function at mx23 works abnormal, but it works good at mx28. Have you tried mx23 udc recently? Best regards, 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
RE: Chipidea usb otg support for IMX/MXS (device functionality)
> > No, I don't have this one available right now. I tried both MX23 > Olinuxino maxi > (I just mutilated the board so that I cut traces to the USB devices on > the board > and routed out USB gadget connector) and a custom MX23 board. > > Note that both have working USB peripheral mode in U-Boot too, but the > driver in > U-Boot is very minimalistic. You mean, the larger data transfer with USB peripheral mode is ok at u-boot? but it can't work at Linux Kernel? Best regards, 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
RE: Chipidea usb otg support for IMX/MXS (device functionality)
> - I use the port in HOST mode by plugging in a USB pendrive over the OTG- > capable > reduction ; this works OK > - I disconnect the reduction and connect it to the computer ; here I get > "timeout waiting for 0800 in 11". > > This means the BSV bit in OTGSC register wasn't unset. > > Interestingly enough, transition the other way around (first use in > gadget mode > then in host mode) works fine, but if I do host mode -> gadget mode, I > get the > timeout. Do you have any hint for me? > It means your OTG VBUS does not lower than BSV (B SESSION VALID, 0.8v) after plugging out Micro-B-TO-A cable. Two possible reasons: 1. You have not gpio control for vbus toggle when role switches. 2. Your hardware has some problems that the vbus can't lower than 0.8v. > btw. what is the plan about cleaning up and upstreaming all these patches > we > have here? Is anyone working on it? I'd hate to stomp on anyones' > efforts,but > I'd also like to see this mess sorted out. We had something un-decided before, now, things almost are cleared. But I am a little busy recently, I hope I can begin to work on it from next week. Best regards, 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
RE: Chipidea usb otg support for IMX/MXS (device functionality)
> > Where can I measure this? > > > Two possible reasons: > > > > 1. You have not gpio control for vbus toggle when role switches. > > This works well. > > > 2. Your hardware has some problems that the vbus can't lower than 0.8v. > > How can I check that? > Just measure the voltage of your otg vbus pin. Best regards, 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
RE: Chipidea usb otg support for IMX/MXS (device functionality)
> > > > Just measure the voltage of your otg vbus pin. > > Ok, I see it's 2.25 volts after I disconnect the microB-A thing . I > should see > 0.8V constantly with nothing connected on the port? > You should see the voltage of vbus pin is less than 0.8v when nothing is connected, we use B_SESSION_VALID (0.8v) to judge it is connected or not. You get 2.25 all the time or it is discharged very slowly? Best regards, 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
RE: Chipidea usb otg support for IMX/MXS (device functionality)
> >
> > You should see the voltage of vbus pin is less than 0.8v when
> > nothing is connected, we use B_SESSION_VALID (0.8v) to judge
> > it is connected or not.
> >
> > You get 2.25 all the time or it is discharged very slowly?
>
> On my mx28evk it discharges very very slowly. Any hints about it?
>
Due to some hardware limitations, the dual-role functions at OTG port
can't be used with host 1 port at the same time at mx28evk. Please follow
below to use USB ports at mx28evk.
When testing usb OTG port, please switch VDD 5V SOURCE SELECT to USB 5V
When testing usb host1 port, please switch VDD 5V SOURCE SELECT to WALL 5V
Best regards,
Peter
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
RE: One USB mouse problem with runtime power management enabled
> > > > Have you opened the mouse by apps (like evtest)?, for USB mouse, > > the usbhid->intf->needs_remote_wakeup is only set at .open/.close > > API. That means if you have not opened mouse, the runtime pm status > > for mouse will changed to RPM_ACTIVE (choose_wakeup -> > pm_runtime_resume) > > If you have opened the mouse, the runtime pm status is RPM_SUSPENDED. > > Ah, okay, I will try that. > > > > That's right. The PM core should realize that the usbhid interface > > > is resumed, so it shouldn't try to suspend the root hub. > > > > You mean runtime pm core? I find the runtime pm core doesn't know > > the device has already resumed by system resume. For this case, > > the pm_runtime_set_active(dev) has returned -EBUSY at usb_resume, > > (I am still checking why, should be related to parent- > >power.disable_depth), > > so the usbhid is still RPM_SUSPENDED. > > It sounds like this is the real problem. Under normal conditions, the > runtime PM core won't allow pm_runtime_set_active() to succeed if the > device's parent is suspended. > > In your case, the parent was the root hub, right? So the root hub goes > back into autosuspend too quickly. > > > > The runtime PM status should be correct -- it should indicate that > the > > > device is active after a system resume. > > > > Then, who should take the responsibility to put the device to > autosuspend > > if the device has auto-suspended before the system suspend, we can > > see the ../power/control is still "auto". > > This happens automatically, when the PM core calls pm_runtime_put() > from device_complete() in drivers/base/power/main.c. This has changed > since 3.5 -- in fact, that's probably the explanation for your problem. > Back in 3.5, the pm_runtime_put() call was in device_resume() rather > than device_complete(). It was moved in 3.7. > It works after cherry-pick yours [1] and Ulf Hansson's patch [2] (only your patch should also work), besides, it needs to set controller active after system resume, or the child (roothub) can't set itself as active. ( [1]PM: Prevent runtime suspend during system resume [2] PM / Runtime: Asyncronous idle|suspend devices at system resume) Thanks, 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
RE: Chipidea usb otg support for IMX/MXS (device functionality)
> -Original Message- > From: Marek Vasut [mailto:ma...@denx.de] > Sent: Sunday, July 07, 2013 9:10 AM > To: Chen Peter-B29397 > Cc: shawn@linaro.org; Michael Grzeschik; maxime.ripard@free- > electrons.com; Hector Palacios; linux-usb@vger.kernel.org; > s.ha...@pengutronix.de; alexander.shish...@linux.intel.com; Estevam > Fabio-R49496; Marc Kleine-Budde; m.grzesc...@pengutronix.de; linux-arm- > ker...@lists.infradead.org > Subject: Re: Chipidea usb otg support for IMX/MXS (device functionality) > > Hi Peter, > > > > Where can I measure this? > > > > > > > Two possible reasons: > > > > > > > > 1. You have not gpio control for vbus toggle when role switches. > > > > > > This works well. > > > > > > > 2. Your hardware has some problems that the vbus can't lower than > 0.8v. > > > > > > How can I check that? > > > > Just measure the voltage of your otg vbus pin. > > It's 2.5 volt for me. What am I supposed to observe happening there? > http://marc.info/?l=linux-usb&m=137242465920952&w=2 Best regards, 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
RE: Chipidea usb otg support for IMX/MXS (device functionality)
> > > > -Original Message- > > > From: Marek Vasut [mailto:ma...@denx.de] > > > Sent: Sunday, July 07, 2013 9:10 AM > > > To: Chen Peter-B29397 > > > Cc: shawn@linaro.org; Michael Grzeschik; maxime.ripard@free- > > > electrons.com; Hector Palacios; linux-usb@vger.kernel.org; > > > s.ha...@pengutronix.de; alexander.shish...@linux.intel.com; Estevam > > > Fabio-R49496; Marc Kleine-Budde; m.grzesc...@pengutronix.de; linux- > arm- > > > ker...@lists.infradead.org > > > Subject: Re: Chipidea usb otg support for IMX/MXS (device > functionality) > > > > > > Hi Peter, > > > > > > > > Where can I measure this? > > > > > > > > > > > Two possible reasons: > > > > > > > > > > > > 1. You have not gpio control for vbus toggle when role switches. > > > > > > > > > > This works well. > > > > > > > > > > > 2. Your hardware has some problems that the vbus can't lower > than > > > > > > 0.8v. > > > > > > > > How can I check that? > > > > > > > > Just measure the voltage of your otg vbus pin. > > > > > > It's 2.5 volt for me. What am I supposed to observe happening there? > > > > http://marc.info/?l=linux-usb&m=137242465920952&w=2 > > Dang, must have missed that one, sorry. > > The VBUS is slowly discharging from around 5V to 2.5V , what does that > tell me? > It needs to check your hardware, it is probably there is a high capacitance at vbus pin. Besides, the vbus should be lower than 0.8v if it is disconnected from pc. Best regards, 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
RE: [PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea
> > Yes, ID interrupt (IDIE) is set. > > I noticed this backtrace in the kernel bootlog, but this only happens if > the > dr_mode="otg" , it comes from the host-mode irq handler : > > [2.757563] irq 238: nobody cared (try booting with the "irqpoll" > option) > [2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.0- > next-20130711-00013-g011c4b3-dirty #703 > [2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from [<80011644>] > (show_stack+0x10/0x14) > [2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>] > (__report_bad_irq.isra.6+0x20/0xe0) > [2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from > [<80065c98>] (note_interrupt+0x16c/0x230) > [2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from [<80064000>] > (handle_irq_event_percpu+0x10c/0x1a4) > [2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4) from > [<800640e8>] (handle_irq_event+0x50/0x78) > [2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from [<8006652c>] > (handle_level_irq+0x88/0x10c) > [2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from > [<800638d0>] > (generic_handle_irq+0x28/0x3c) > [2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from > [<8000f84c>] > (handle_IRQ+0x30/0x84) > [2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>] > (__irq_svc+0x40/0x6c) > [2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>] > (__do_softirq+0x90/0x1d8) > [2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from [<80022edc>] > (irq_exit+0x98/0xd4) > [2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>] > (handle_IRQ+0x34/0x84) > [2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>] > (__irq_svc+0x40/0x6c) > [2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>] > (vprintk_emit+0x1bc/0x524) > [2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from [<804da5a4>] > (printk+0x30/0x40) > [2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>] > (mousedev_init+0x4c/0x60) > [2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from [<806178fc>] > (do_one_initcall+0x94/0x14c) > [2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from [<80617b20>] > (kernel_init_freeable+0x16c/0x22c) > [2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c) from > [<804d8cbc>] > (kernel_init+0x8/0x150) > [2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from [<8000ea70>] > (ret_from_fork+0x14/0x24) > [2.949932] handlers: > [2.952227] [<8033fc58>] ci_irq > [2.955388] Disabling IRQ #238 > See this " Disabling IRQ #238", that is the reason you can't get ID interrupt. I have no other idea before I try, you can try below things: 1. please switch VDD 5V SOURCE SELECT to USB 5V 2. try not plug in mouse during the boots up > btw. do you have any kind of other CI13xxx documentation than what's in > the CPU > datasheets ? > Mx28 RM does not include many controller detail, please try to read it from mx6/mx5 RM. Best regards, 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
RE: [PATCH] usb: udc: add gadget state kobject uevent
> > On Wed, Jul 17, 2013 at 10:36 AM, Peter Chen > wrote: > > On Mon, Jul 15, 2013 at 11:31:17PM -0700, Greg KH wrote: > >> On Tue, Jul 16, 2013 at 11:49:07AM +0800, Rong Wang wrote: > >> > Hi Greg, > >> > > >> > The USB on our platform can change roles between HOST and GADGET, > but > >> > it is not capable of OTG. > >> > >> That kind of sounds like the definition of OTG :) > >> > >> > When the USB changes between roles the udev will run some scripts > >> > automatically according to the udev rules. > >> > >> What exactly does udev/userspace see when the roles change? > >> > >> And what can trigger the change in roles? > >> > >> > The default role is GADGET, and we bind the g_mass_storage to the > USB > >> > GADGET role. > >> > > >> > We should secure the back end file storage between the device and > the > >> > host PC connecting to our device. > >> > We need to know when the GADGET is really connect to a host PC, then > >> > we can umount the file on the device > >> > and export it to the g_mass_storage. > >> > >> I thought you already get an event for this, otherwise no one would be > >> able to properly deal with this. > >> > >> > The question is since we default GADGET, so the g_mass_storage.ko is > >> > installed early but connecting to a host PC > >> > is randomly, But the udev has no idea when a host PC connects our > device. > >> > > >> > So we consider it's reasonable to let the udev know the GADGET > device state. > >> > Is there any alternative to our question? > >> > >> I thought we already export events for gadget device states, have you > >> looked for them? I can't dig through the code at the moment, but this > >> seems like a pretty common issue... > >> > > > > If I understand correctly, what Rong wants is udev can be notified the > > udc state changes, like connect/disconnect event. Currently, we only > > export it to /sys. > > OK. Thanks for your share. > > And you develop a new utility rather than udev to monitor that file? > And you probably create a work queue in your udc driver to do this work? > Not yet, no one complains it, so I haven't added it. But, it is a useful user interface, Android has implemented similar functions at its gadget driver. Best regards, 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
RE: Build regressions/improvements in v3.11-rc3
My patches "usb: chipidea: fix the build error with randconfig" fixes chipidea
problem.
It has already at USB fixes for 3.11-rc4.
On Tue, 30 Jul 2013, Geert Uytterhoeven wrote:
> JFYI, when comparing v3.11-rc3 to v3.11-rc2[3], the summaries are:
> - build errors: +38/-14
+ arch/powerpc/kvm/book3s_emulate.c: error: 'bat' may be used uninitialized
in this function [-Werror=uninitialized]: => 349:2
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_ANDCOND' undeclared (first use
in this function): => 161:17, 86:16
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_AVPN' undeclared (first use in
this function): => 160:17, 85:16, 192:16
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_BULK_REMOVE' undeclared (first
use in this function): => 246:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_CEDE' undeclared (first use in
this function): => 250:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_CPPR' undeclared (first use in
this function): => 257:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_ENTER' undeclared (first use
in this function): => 240:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_EOI' undeclared (first use in
this function): => 258:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_EXACT' undeclared (first use
in this function): => 50:6
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_IPI' undeclared (first use in
this function): => 259:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_IPOLL' undeclared (first use
in this function): => 260:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_NOT_FOUND' undeclared (first
use in this function): => 193:27, 87:27
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_PARAMETER' undeclared (first
use in this function): => 138:10
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_PROTECT' undeclared (first use
in this function): => 244:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_PTEG_FULL' undeclared (first
use in this function): => 54:12
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_PUT_TCE' undeclared (first use
in this function): => 248:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_REMOVE' undeclared (first use
in this function): => 242:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_RTAS' undeclared (first use in
this function): => 265:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_SUCCESS' undeclared (first use
in this function): => 67:26, 96:26, 211:26, 125:12
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_TOO_HARD' undeclared (first
use in this function): => 224:12
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_XIRR' undeclared (first use in
this function): => 256:7
+ arch/powerpc/kvm/book3s_pr_papr.c: error: 'H_XIRR_X' undeclared (first use
in this function): => 261:7
+ arch/powerpc/platforms/pseries/hotplug-memory.c: error: 'SECTION_SIZE_BITS'
undeclared (first use in this function): => 24:31
+ mm/memory_hotplug.c: error: 'PAGES_PER_SECTION' undeclared (first use in
this function): => 1630:46
+ mm/memory_hotplug.c: error: implicit declaration of function
'__nr_to_section' [-Werror=implicit-function-declaration]: => 1635:3
+ mm/memory_hotplug.c: error: implicit declaration of function
'find_memory_block_hinted' [-Werror=implicit-function-declaration]: => 1642:3
+ mm/memory_hotplug.c: error: implicit declaration of function
'pfn_to_section_nr' [-Werror=implicit-function-declaration]: => 1631:3
+ mm/memory_hotplug.c: error: implicit declaration of function
'present_section_nr' [-Werror=implicit-function-declaration]: => 1632:3
powerpc-randconfig
+ drivers/md/dm-cache-policy-mq.c: error: conflicting types for
'remove_mapping': => 962:13
sparc-allmodconfig, not a regression (was hidden due to a sparc64/sparc32
mixup on kissb), patch submitted
+ error: "usb_add_gadget_udc" [drivers/usb/chipidea/ci_hdrc.ko] undefined!:
=> N/A
+ error: "usb_del_gadget_udc" [drivers/usb/chipidea/ci_hdrc.ko] undefined!:
=> N/A
+ error: "usb_gadget_map_request" [drivers/usb/chipidea/ci_hdrc.ko]
undefined!: => N/A
+ error: "usb_gadget_unmap_request" [drivers/usb/chipidea/ci_hdrc.ko]
undefined!: => N/A
x86_64-randconfig
> [1] http://kisskb.ellerman.id.au/kisskb/head/6490/ (all 120 configs)
> [3] http://kisskb.ellerman.id.au/kisskb/head/6461/ (all 120 configs)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vge
RE: [PATCH v13 00/14] Add tested id switch and vbus connect detect support for Chipidea
Hi Alex, any comments, now it finished 3.11-rc4, I don't want this patchset missed at 3.11. Please tell me which one is OK, and which one needs to be refined, thanks. Best regards, Peter From: linux-usb-ow...@vger.kernel.org [linux-usb-ow...@vger.kernel.org] on behalf of Peter Chen [peter.c...@freescale.com] Sent: Friday, July 26, 2013 5:18 PM To: alexander.shish...@linux.intel.com Cc: linux-usb@vger.kernel.org; linux-arm-ker...@lists.infradead.org; feste...@gmail.com; ma...@denx.de; maxime.rip...@free-electrons.com; shawn@linaro.org; ker...@pengutronix.de; m...@pengutronix.de; m.grzesc...@pengutronix.de; gre...@linuxfoundation.org; Li Frank-B20596 Subject: [PATCH v13 00/14] Add tested id switch and vbus connect detect support for Chipidea This patchset adds tested otg id switch function and vbus connect and disconnect detection for chipidea driver. And fix kinds of bugs found at chipidea drivers after enabling id and vbus detection. This patch are fully tested at imx6 sabresd and imx28evk platform by me. Besides, marek tested it on two STMP3780-based boards (not yet mainline) and two MX28-based boards. My chipidea repo: https://github.com/hzpeterchen/linux-usb.git Chagnes for v13: - Add Tested-by: Marek Vasut - [Sascha's comments]: Add return value check for devm_regulator_get. [3/14] - [Marc's comments]: Change timeout usage at hw_wait_reg. [11/14] - [Alex's comments]: Using platdata flag to indicate dual role but not OTG controller. [7/14] Changes for v12: - Rebased greg's usb-next tree (3.10.0-rc7+) - Split more small patches for single function and fix. Changes for v11: - mark ci_handle_vbus_change as static as it is only used at core.c [3/9] - Move the vbus operation for platform code to host code, as vbus operation is common operation, and host is the only user for vbus. When it is host mode, we need to open vbus, when it is out of host mode, we need to close vbus. [6/9] [8/9] - Delete the delayed work at core.c as it is not needed. [7/9] Changes for v10: - Delete [8/9] at v9, ci core's drvdata must be set for further operation. [8/8] Changes for v9: - Some small comments from Alex like: variable comment for otg event additional newline. [3/9] - Import function tell show if the controller has otg capable, if the controller supports both host and device, we think it is otg capable, and can read OTGSC. [3/9] - Merge two otg patches [v8 3/8] and [v8 4/8] to one [v9 3/9]. [3/9] - Add inline to ci_hdrc_gadget_destroy if CONFIG_USB_CHIPIDEA_UDC is not defined, it can fix one build warning "defined but not used" [3/9] - One comment from Felipe about changing calling gadget disconnect API at chipidea's udc driver. I move calling ci->driver->disconnect from _gadget_stop_activity to which calls _gadget_stop_activity except ci13xxx_stop, as udc core will call disconnect when do rmmod gadget. [7/9] - Add ci core probe's return value to ci's platform_data, we do this for getting core's probe's result at platform layer, and quit it if the core's probe fails. [8/9] [9/9] Changes for v8: - Add ci_supports_gadget helper to know if the controller supports gadget, if the controller supports gadget, it needs to read otgsc to know the vbus value, basically, if the controller supports gadget, it will support host as well [3/8] - At ci_hdrc_probe, it needs to add free memory at error path [3/8] - Cosolidate ci->driver = NULL at ci13xxx_stop [8/8] Changes for v7: For Patch 8/8, we only need to set ci->driver to NULL when usb cable is not connected, for other changes, it will case some runtime pm unmatch and un-align with udc-core & composite driver problems. Changes for v6: - Add Alex comments for init/destroy function [3/8] [4/8] - Remove memset(&ci->gadget, 0, sizeof(ci->gadget)) at destory function [4/8] - Add Kishon's comment: Change the format of struct usb_otg otg at drivers/usb/chipidea/ci.h [1/8] - Add comments for CI_VBUS_STABLE_TIMEOUT [3/8] - Change the otg_set_peripheral return value check as the fully chipidea driver users don't need it. [4/8] - Fix one bug that the oops when re-plug in usb cable after rmmod gadget [8/8] Peter Chen (14): usb: chipidea: add vbus regulator control usb: chipidea: imx: remove vbus regulator operation usb: chipidea: imx: add return value check for devm_regulator_get usb: chipidea: udc: otg_set_peripheral is useless for some chipidea users usb: chipidea: otg: Add otg file used to access otgsc usb: chipidea: Add role init and destory APIs usb: chipidea: add flag CI_HDRC_DUAL_ROLE_NOT_OTG usb: chipidea: disable all interrupts and clear all interrupts status usb: chipidea: move otg relate things to otg file usb: chipidea: add vbus interrupt handler usb: chipidea: add wait vbus lower than OTGSC_BSV before role starts usb: chipidea: udc: misuse flag CI_HDRC_REGS_SHARED and CI_HDRC_PULLUP_ON_VBUS usb: chipidea: udc: .pullup is valid when vbus is on at CI_HDRC_PULLUP_ON_VBUS usb: chi
RE: Testing USB connectors on iMX28EVK
> > Dear Peter Chen, > > > On Thu, Sep 12, 2013 at 12:55:54PM +0200, gianluca wrote: > > > Hello, > > > I just compiled the kernel from git repo of peter chen > > > (git://github.com/hzpeterchen/linux-usb) > > > > Please use current linux-next tree, it has already supported > > mx28 evk for vbus and id detection. > > I am afraid I have not updated my tree with mx28 updates. > > Just curious, how do you boot the linux kernel on your EVK? Do you use > imx > bootlets or uboot (which one, mainline?) or even some other setup ? > v2013.04 u-boot (FSL internal tree, but should be the same with mainline's for mx28) and the readme at u-boot folder for mx28. Best regards, 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
RE: Mx28 USB workaround for errata
> >> The reason was an mx28 errata: "ENGR119653 USB: ARM to USB register > >> error issue" > > Peter, does this erratum only affect mx28? Yes > >> > >> Freescale issued a patch for 2.6.35 to workaround this problem last > >> year. I ported this patch. However, it is not totally "device tree > >> compatible". I mean, this patch is only needed for mx28, not for the > >> other SOCs using chipidea, and I used ifdefs. So you can't compile a > >> kernel both for mx28 and e.g. for mx23. If you check the patch, you > see, > >> that it is modifying the lowest layer, the writel() function; it is > >> changing it to an SWP instruction. According the errata, I need to use > >> SWP instructions to write USB registers. > > > > Presumably you _could_ use the SWP instructions for other SOCs, but > > they would slow down the driver. Right? > > > >> So I'm curious, what other possibilities do I have to make this patch > >> device tree compatible? I mean, I can't check the machine's type on > each > >> USB register writing. > > > > Actually you can. See the definition of ehci_writel for an example. > > > >> Then I could use function pointers for register > >> writing function, or weak aliases somehow. But that would causes an > >> overhead because of the additional function calling, instead of simple > >> inline assembly codes. > >> > >> What's the standard way in this case? Or should I just leave the patch > >> as it is? > > > > Let's see what the Freescale and ChipIdea maintainers suggest. I will try to submit patches for both ehci and chipidea driver. Best regards, Peter
RE: [PATCH 1/1] usb: gadget: mark init as late_initcall
> > Since we introduce -EPROBE_DEFER for udc driver, it will be > > probed at late_initcall if it is defered. When the gadget > > is built in, it will return "couldn't find an available UDC" > > at such case. That's the problem we met at below link: > > > > http://marc.info/?l=linux-usb&m=137706435611447&w=2 > > > > We have no driver's probe at gadget driver, so we can't return > > -EPROBE_DEFER. And it is also not suitable to defer udc_bind_to_driver > > if the udc is not found temporarily, since it is hard to decide the > > return value for usb_gadget_probe_driver. > > > > Due to above reasons, mark gadget's init as late_initcall may be a > > moderate solution. > > > > Signed-off-by: Peter Chen > > Seems this tries to paper over an issue with module dependencies , no? > In fact, there are module dependencies between udc and composite gadget. The udc must be created before composite gadget. If the creation of udc (or its controller driver) is deferred, the composite gadget driver has no way to know it, unless we change gadget framework a lot, eg add probe API for composite driver, create a thread to check udc creation if there is no udc, etc. -- 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: When USB PHY framework should be used?
> > > I think you should have a wrapper driver to EHCI/OHCI to handle this > reset. > > Thank you Kishon and Peter for the quick replies. Is there any good > example of such a wrapper driver in the kernel already? > chipidea, dwc3, etc. -- 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: phy: rename otg.c to phy.c
> > On Mon, Sep 17, 2012 at 12:18 AM, Peter Chen > wrote: > > The operations in current otg.c are actually for USB PHY's > > Move it from drivers/usb/otg/ to drivers/usb/phy/ > > > > Signed-off-by: Peter Chen > > --- > > drivers/usb/otg/Makefile |3 - > > drivers/usb/otg/otg.c| 239 -- > > > drivers/usb/phy/Makefile |3 + > > drivers/usb/phy/phy.c| 239 > ++ > > 4 files changed, 242 insertions(+), 242 deletions(-) > > You would better use git mv / git format -M to do the renames. > Thanks, I will send v2. > Regards, > > Fabio Estevam > -- > 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: chipidea: use OTGSC_BSV to detect vbus valid
>
> OTGSC_AVVIS cannot be set when vbus voltage goes valid or invalid on
> imx, so convert to use OTGSC_BSV.
>
Not just on imx, from the IC guys, it is chipidea's feature.
the AVVIS will not go to 1 when the vbus goes to high if ID = 1.
> OTGSC_BSVIE and OTGSC_BSVIS is not cleared when hw_device_reset, so we
> don't need to call hw_enable_vbus_intr after hw_device_reset.
>
> Signed-off-by: Richard Zhao
> ---
> drivers/usb/chipidea/udc.c | 17 -
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 2f45bba..b158a04 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -307,14 +307,13 @@ static u32 hw_test_and_clear_intr_active(struct
> ci13xxx *ci)
>
> static void hw_enable_vbus_intr(struct ci13xxx *ci)
> {
> - hw_write(ci, OP_OTGSC, OTGSC_AVVIS, OTGSC_AVVIS);
> - hw_write(ci, OP_OTGSC, OTGSC_AVVIE, OTGSC_AVVIE);
> - queue_work(ci->wq, &ci->vbus_work);
> + hw_write(ci, OP_OTGSC, OTGSC_BSVIS, OTGSC_BSVIS);
> + hw_write(ci, OP_OTGSC, OTGSC_BSVIE, OTGSC_BSVIE);
> }
>
> static void hw_disable_vbus_intr(struct ci13xxx *ci)
> {
> - hw_write(ci, OP_OTGSC, OTGSC_AVVIE, 0);
> + hw_write(ci, OP_OTGSC, OTGSC_BSVIE, 0);
> }
>
> /**
> @@ -387,7 +386,7 @@ static void vbus_work(struct work_struct *work)
> {
> struct ci13xxx *ci = container_of(work, struct ci13xxx, vbus_work);
>
> - if (hw_read(ci, OP_OTGSC, OTGSC_AVV))
> + if (hw_read(ci, OP_OTGSC, OTGSC_BSV))
> usb_gadget_vbus_connect(&ci->gadget);
> else
> usb_gadget_vbus_disconnect(&ci->gadget);
> @@ -1392,7 +1391,6 @@ static int ci13xxx_vbus_session(struct usb_gadget
> *_gadget, int is_active)
> if (is_active) {
> pm_runtime_get_sync(&_gadget->dev);
> hw_device_reset(ci, USBMODE_CM_DC);
> - hw_enable_vbus_intr(ci);
> hw_device_state(ci, ci->ep0out->qh.dma);
> } else {
> hw_device_state(ci, 0);
> @@ -1569,7 +1567,6 @@ static int ci13xxx_start(struct usb_gadget *gadget,
> if (ci->vbus_active) {
> if (ci->platdata->flags & CI13XXX_REGS_SHARED) {
> hw_device_reset(ci, USBMODE_CM_DC);
> - hw_enable_vbus_intr(ci);
> }
> } else {
> pm_runtime_put_sync(&ci->gadget.dev);
> @@ -1680,7 +1677,7 @@ static irqreturn_t udc_irq(struct ci13xxx *ci)
> intr = hw_read(ci, OP_OTGSC, ~0);
> hw_write(ci, OP_OTGSC, ~0, intr);
>
> - if (intr & (OTGSC_AVVIE & OTGSC_AVVIS))
> + if ((intr & OTGSC_BSVIE) && (intr & OTGSC_BSVIS))
> queue_work(ci->wq, &ci->vbus_work);
>
> spin_unlock(&ci->lock);
> @@ -1758,7 +1755,6 @@ static int udc_start(struct ci13xxx *ci)
> retval = hw_device_reset(ci, USBMODE_CM_DC);
> if (retval)
> goto put_transceiver;
> - hw_enable_vbus_intr(ci);
> }
>
> retval = device_register(&ci->gadget.dev);
> @@ -1782,6 +1778,9 @@ static int udc_start(struct ci13xxx *ci)
> if (retval)
> goto remove_trans;
>
> + queue_work(ci->wq, &ci->vbus_work);
> + hw_enable_vbus_intr(ci);
> +
> pm_runtime_no_callbacks(&ci->gadget.dev);
> pm_runtime_enable(&ci->gadget.dev);
>
> --
> 1.7.9.5
--
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
About OTG operation for chipidea driver
Hi Alex, At current chipidea driver, we have not implemented struct usb_otg, so when udc calls otg_set_peripheral, it will fail. (udc_start at drivers/usb/chipidea/udc.c). In fact, if both host and device use chipidea driver, we do not need to call otg_set_peripheral at all at current chipidea framework. To fix it, a better solution is to add otg.c to implement struct usb_otg, but we also need to consider qualcomm (msm)'s driver, which also uses chipidea, but has its own host and otg driver. What's your opinion? Best regards, Peter Chen MAD Linux BSP Team Freescale Semiconductor Ltd. -- 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] USB: mxs-phy: add basic otg support
The purpose for this patch is to have set_peripheral implementation,
but in current chipidea code, we don't need to know gadget at all,
as when id switch occurs, the core code know its role (device or host)
very well, and will call related stop/start function.
We have already many code bind struct otg with struct phy, we'd better
split them at later code.
A better approach to fix this problem is to either not call set_peripheral
if both device and host use chipidea driver, or implement otg struct
at chipidea driver.
> Changes from v2:
> - assign host/gadget in mxs_phy_set_host/mxs_phy_set_peripheral
>
> drivers/usb/otg/mxs-phy.c | 25 +
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
> index 88db976..3255112 100644
> --- a/drivers/usb/otg/mxs-phy.c
> +++ b/drivers/usb/otg/mxs-phy.c
> @@ -129,12 +129,28 @@ static int mxs_phy_on_disconnect(struct usb_phy
> *phy, int port)
> return 0;
> }
>
> +static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
> +{
> + otg->host = host;
> +
> + return 0;
> +}
> +
> +static int mxs_phy_set_peripheral(struct usb_otg *otg,
> + struct usb_gadget *gadget)
> +{
> + otg->gadget = gadget;
> +
> + return 0;
> +}
> +
> static int mxs_phy_probe(struct platform_device *pdev)
> {
> struct resource *res;
> void __iomem *base;
> struct clk *clk;
> struct mxs_phy *mxs_phy;
> + struct usb_otg *otg;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!res) {
> @@ -171,6 +187,15 @@ static int mxs_phy_probe(struct platform_device
> *pdev)
>
> mxs_phy->clk = clk;
>
> + otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);
> + if (!otg)
> + return -ENOMEM;
> + otg->phy = &mxs_phy->phy;
> + otg->set_host = mxs_phy_set_host;
> + otg->set_peripheral = mxs_phy_set_peripheral;
> +
> + mxs_phy->phy.otg = otg;
> +
> platform_set_drvdata(pdev, &mxs_phy->phy);
>
> return 0;
> --
> 1.7.9.5
--
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] USB: mxs-phy: add basic otg support
> > > The purpose for this patch is to have set_peripheral implementation, > > but in current chipidea code, we don't need to know gadget at all, > > as when id switch occurs, the core code know its role (device or host) > > very well, and will call related stop/start function. > > > > We have already many code bind struct otg with struct phy, we'd better > > split them at later code. > > > > A better approach to fix this problem is to either not call > set_peripheral > > if both device and host use chipidea driver, or implement otg struct > > at chipidea driver. > > Yes, I think the otg driver should be part of the chipidea driver, the > only concern is the msm, although it can probably be converted once we > have an otg driver. I'll have a look at it soon unless someone beats me > to it. Currently, we still have no auto-suspend and wakeup support, but msm supports them, it may need much effort to move msm to current whole chipidea framework. As it affects we go on implement id-switch function and other usb functions at chipidea, I hope we can have it soon, thanks. I also would like to help it if you are busy on other things. > > The bigger plan was to implement a generic otg framework and base the > chipidea's otg driver on that, instead of dragging in one more state > machine and whatnot. > Yes, we can transfer it to use generic otg framework once it is ready, but first, it is better has own otg driver at chipidea. > Regards, > -- > Alex -- 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 0/4] Add PHY nofity for some i.mx SoCs
> -Original Message- > From: Mike Thompson [mailto:mpthomp...@gmail.com] > Sent: Thursday, September 20, 2012 3:58 AM > To: Chen Peter-B29397 > Cc: st...@rowland.harvard.edu; ba...@ti.com; > alexander.shish...@linux.intel.com; Zhao Richard-B20223; snijsure@grid- > net.com; Estevam Fabio-R49496; ma...@denx.de; gre...@linuxfoundation.org; > linux-usb@vger.kernel.org; linux-arm-ker...@lists.infradead.org > Subject: Re: [PATCH 0/4] Add PHY nofity for some i.mx SoCs > > Tested on OLinuXino-Micro imx233 based hardware with patches applied > against 'linux-next' branch. > > Tested-by: Mike Thompson > Hi Felipe, how about the first and second patches in this patchset? Alex, how about the third patch in this patchset? Alan, how about the fourth patch in this patset? Thanks, Peter > Mike Thompson > > On Thu, Sep 13, 2012 at 8:18 PM, Peter Chen > wrote: > > At some i.mx SoCs, when controller works at host mode, the PHY > > register needs to be changed at device connect, disconnect, bus > > suspend and resume due to the SoC limitations. > > > > The phy notification should be added according to below rules: > > > > 1. Only set HW_USBPHY_CTRL.ENHOSTDISCONDETECT > > during high speed host mode. > > 2. Do not set HW_USBPHY_CTRL.ENHOSTDISCONDETECT > > during the reset and speed negotiation period. > > 3. Do not set HW_USBPHY_CTRL.ENHOSTDISCONDETECT > > during host suspend/resume sequence. > > > > Please refer: i.mx23RM(page 413) for detail. > > http://www.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf > > > > Freescale i.MX SoC, i.mx23, i.mx28 and i.mx6(i.mx6SL does not > > need to follow the 3rd rule) need to follow above rules. > > > > The correct notification setting method should be: > > 1. Set connect notify after the second bus reset. > > 2. Set disconnect notify after disconnection. > > 3. Set suspend nofity after bus goes to suspend (portsc.suspendM=1). > > 4. Set resume notify after resume (portsc.fpr=0). > > > > Peter Chen (4): > > usb: phy: add notify_suspend/notify_resume callback > > usb: mxs-phy: implement notify_suspend/notify_resume callback > > usb: chipidea: add phy notify at suspend/resume procedure > > usb: refine phy notify operation during connection and disconnection > > > > drivers/usb/chipidea/host.c | 75 > +- > > drivers/usb/core/hub.c | 14 > > drivers/usb/otg/mxs-phy.c | 56 +--- > > include/linux/usb/phy.h | 44 + > > 4 files changed, 161 insertions(+), 28 deletions(-) > > -- 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 0/2] Some tiny changes for split phy from otg
> Below are two patches to split some phy related things from
> otg.
>
> Changes for v2:
> - Use git mv / git format -M to create patch for rename otg.c to phy.c
>
Hi Felipe, how about these two patches?
> Peter Chen (2):
> usb: phy: rename enum usb_otg_state to usb_state
> usb: phy: rename otg.c to phy.c
>
> drivers/usb/otg/Makefile |3 ---
> drivers/usb/otg/isp1301_omap.c |4 ++--
> drivers/usb/otg/otg_fsm.c|6 +++---
> drivers/usb/phy/Makefile |3 +++
> drivers/usb/{otg/otg.c => phy/phy.c} | 14 --
> include/linux/usb/msm_hsusb.h|2 +-
> include/linux/usb/otg.h |4 ++--
> include/linux/usb/phy.h | 18 ++
> 8 files changed, 29 insertions(+), 25 deletions(-)
> rename drivers/usb/{otg/otg.c => phy/phy.c} (97%)
>
>
> --
> 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
--
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 02/12] usb: chipidea: udc: add pullup fuction, needed by the uvc gadget
> > > hw_write(ci, OP_USBINTR, ~0,
> > >
> USBi_UI|USBi_UEI|USBi_PCI|USBi_URI|USBi_SLI);
> > > - hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
> > > } else {
> > > - hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
> > > hw_write(ci, OP_USBINTR, ~0, 0);
> > > }
> >
> > Hi Marc, your above change break the function that load gadget before
> > plug usb cable.
>
> What do you mean with that? When switching into device role, the
> otg can load every gadget-module without having the hardware pluged-in.
>
Are you sure? In current chipidea otg design, the gadget will be freed
when device->host, but the gadget will not be re-created when host->device.
So, when the device connects to pc again, there will be an null pointer error.
(I use g_serial.ko)
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
RE: [PATCH 02/12] usb: chipidea: udc: add pullup fuction, needed by the uvc gadget
> > Hmm, gadget gets zeroed out on every gadget->whatever switch, but it > also gets reinitialized on every whatever->gadget transition. And it > really shouldn't cause a null dereference. > When gadget->host, the call sequence like below: udc_stop -> usb_del_gadget_udc -> usb_gadget_remove_driver ->composite_unbind -> kfree(cdev); When host->gadget (just plug out MicroB-A cable), the call sequence like below: udc_start-> usb_add_gadget_udc It will only create device udc, the struct usb_composite_dev *cdev does not be reallocated again. When the device connects to host, the functions at composite.c will be called, as cdev is freed, the null dereference will occur. My oops like below (the oops occurs when bus reset occurs): Unable to handle kernel NULL pointer dereference at virtual address 003c pgd = 80004000 [003c] *pgd= Internal error: Oops: 17 [#1] SMP ARM Modules linked in: g_serial libcomposite CPU: 0Not tainted (3.6.0-rc3+ #42) PC is at _raw_spin_lock_irqsave+0x18/0x58 LR is at composite_disconnect+0x24/0x64 [libcomposite] pc : [<804cd920>]lr : [<7f001000>]psr: a193 sp : 80671da0 ip : 80671db0 fp : 80671dac r10: bf8086e4 r9 : bf808680 r8 : 004b r7 : bf833074 r6 : bf833078 r5 : 003c r4 : r3 : bf3abd80 r2 : 003c r1 : a193 r0 : a193 Flags: NzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel Control: 10c53c7d Table: 4ec1004a DAC: 0017 Process swapper/0 (pid: 0, stack limit = 0x806702f0) Stack: (0x80671da0 to 0x80672000) 1da0: 80671dcc 80671db0 7f001000 804cd914 bf833080 bf833010 bf833078 bf833074 1dc0: 80671dec 80671dd0 80366aa4 7f000fe8 bf833010 09202f20 bf833010 1de0: 80671e4c 80671df0 80367928 803669f0 daffc000 0001 8488 0040 1e00: b7eec000 bf833014 0007 981d 80671e3c 80671e20 8004694c 80046460 1e20: bf833010 09202f20 004b bf808680 bf8086e4 1e40: 80671e64 80671e50 80366050 80367638 bf01e100 bf8086d0 80671e9c 80671e68 1e60: 80073d48 80365ff8 2193 80679148 81257838 bf808680 bf8086d0 8066ddbc 1e80: 004b 412fc09a 80671eb4 80671ea0 80073ef4 80073d00 1ea0: bf808680 bf8086d0 80671ecc 80671eb8 80076c14 80073ea0 004b 80679148 1ec0: 80671ee4 80671ed0 80073cd8 80076b80 8067 80679148 80671f0c 80671ee8 1ee0: 8000f68c 80073cb4 f400010c 80678970 80671f30 f4000110 8067c978 412fc09a 1f00: 80671f2c 80671f10 800084bc 8000f644 8000f824 6013 80671f64 1f20: 80671f84 80671f30 8000e7c0 80008494 000f 80019720 1f40: 8067 806ae0c8 804cfd84 8067c978 412fc09a 80671f84 1f60: 80671f88 80671f78 8000f820 8000f824 6013 80671fac 80671f88 1f80: 8000fd38 8000f7f8 80678fb0 806ae000 8066063c 8fff 1000406a 412fc09a 1fa0: 80671fbc 80671fb0 804bad04 8000fc90 80671ff4 80671fc0 80624948 804bacac 1fc0: 806244c4 8066063c 10c53c7d 80678948 1fe0: 8066060c 8067c96c 80671ff8 10008044 806246b8 Backtrace: [<804cd908>] (_raw_spin_lock_irqsave+0x0/0x58) from [<7f001000>] (composite_disconnect+0x24/0x64 [libcomposite]) [<7f000fdc>] (composite_disconnect+0x0/0x64 [libcomposite]) from [<80366aa4>] (_gadget_stop_activity+0xc0/0x120) r7:bf833074 r6:bf833078 r5:bf833010 r4:bf833080 [<803669e4>] (_gadget_stop_activity+0x0/0x120) from [<80367928>] (udc_irq+0x2fc/0xf90) r7: r6:bf833010 r5:09202f20 r4:bf833010 [<8036762c>] (udc_irq+0x0/0xf90) from [<80366050>] (ci_irq+0x64/0xdc) [<80365fec>] (ci_irq+0x0/0xdc) from [<80073d48>] (handle_irq_event_percpu+0x54/0x1a0) r5:bf8086d0 r4:bf01e100 [<80073cf4>] (handle_irq_event_percpu+0x0/0x1a0) from [<80073ef4>] (handle_irq_event+0x60/0x80) [<80073e94>] (handle_irq_event+0x0/0x80) from [<80076c14>] (handle_fasteoi_irq+0xa0/0x170) r5:bf8086d0 r4:bf808680 [<80076b74>] (handle_fasteoi_irq+0x0/0x170) from [<80073cd8>] (generic_handle_irq+0x30/0x38) r5:80679148 r4:004b [<80073ca8>] (generic_handle_irq+0x0/0x38) from [<8000f68c>] (handle_IRQ+0x54/0xb4) r5:80679148 r4:8067 [<8000f638>] (handle_IRQ+0x0/0xb4) from [<800084bc>] (gic_handle_irq+0x34/0x68) r9:412fc09a r8:8067c978 r7:f4000110 r6:80671f30 r5:80678970 r4:f400010c [<80008488>] (gic_handle_irq+0x0/0x68) from [<8000e7c0>] (__irq_svc+0x40/0x54) Exception stack(0x80671f30 to 0x80671f78) 1f20: 000f 80019720 1f40: 8067 806ae0c8 804cfd84 8067c978 412fc09a 80671f84 1f60: 80671f88 80671f78 8000f820 8000f824 6013 r7:80671f64 r6: r5:6013 r4:8000f824 [<8000f7ec>] (default_idle+0x0/0x44) from [<8000fd38>] (cpu_idle+0xb4/0xf0) [<8000fc84>] (cpu_idle+0x0/0xf0) from [<804bad04>] (rest_init+0x64/0x7c) r9:412fc09a r8:1000406a r7:8fff r6:8066063c r5:806ae000 r4:80678fb0 [<804baca0>] (rest_init+0x0/0x7c) from [<80624948>] (start_kernel+0x29c/0x2ec) [<806246ac>] (
RE: [PATCH v2 4/4] usb: refine phy notify operation during connection and disconnection
>
> Simply reading this patch shows that it contains at least two errors.
Thanks for review.
>
> > @@ -4187,6 +4190,10 @@ static void hub_port_connect_change(struct
> usb_hub *hub, int port1,
> > }
> > }
> >
> > + if (hcd->phy && !hdev->parent &&
> > + !(portstatus & USB_PORT_STAT_CONNECTION))
> > + usb_phy_notify_disconnect(hcd->phy, udev->speed);
>
> What happens if udev is NULL (see the test in the next statement)?
>
I will add the condition of (udev), the thing I want to do is:
when the device is disconnected, the pcd interrupt will occur, and
the code will be there, I need to tell the phy driver, the disconnection
occurs, and the speed of the disconnected usb device.
> > +
> > /* Disconnect any existing devices under this port */
> > if (udev)
> > usb_disconnect(&hub->ports[port1 - 1]->child);
> > @@ -4212,13 +4219,6 @@ static void hub_port_connect_change(struct
> usb_hub *hub, int port1,
> > }
> > }
> >
> > - if (hcd->phy && !hdev->parent) {
> > - if (portstatus & USB_PORT_STAT_CONNECTION)
> > - usb_phy_notify_connect(hcd->phy, port1);
> > - else
> > - usb_phy_notify_disconnect(hcd->phy, port1);
>
> Is the second argument supposed to be a port number, like here, or a
> speed value, like above? Clearly something is wrong, either in the old
> code or in the new code.
>
The first patch at this patchset changes the API of usb_phy_notify_disconnect:
- /* notify phy connect status change */
- int (*notify_connect)(struct usb_phy *x, int port);
- int (*notify_disconnect)(struct usb_phy *x, int port);
+ /*
+* Notify phy that
+* - The controller's connect status change.
+* - The controller's suspend/resume occurs, and the device
+* is on the port.
+*/
+ int (*notify_connect)(struct usb_phy *x,
+ enum usb_device_speed speed);
+ int (*notify_disconnect)(struct usb_phy *x,
+ enum usb_device_speed speed);
> 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
linux-usb@vger.kernel.org
> > This if is a no-op, as OTG_ are defined as: > > #define OTGSC_AVVIS BIT(17) > #define OTGSC_AVVIE BIT(25) > > Resulting in queue_work() never called from here. > > > + > > spin_unlock(&ci->lock); > > > > I'm not that deep into the OTG stuff to fix it properly. > Yes, we have many problems at our OTG switch at chipidea driver. Alex, we have talked about it several weeks ago, do you have any thoughts about it? I will post some for your suggestion. Peter > regards, Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Industrial Linux Solutions| Phone: +49-231-2826-924 | > Vertretung West/Dortmund | Fax: +49-5121-206917- | > Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | -- 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 v5 1/4] Revert "usb: otg: mxs-phy: Fix mx23 operation"
> > On Mon, Oct 22, 2012 at 03:13:09PM +0800, Peter Chen wrote: > > The real reason causes mx23 fail are: > > > > - Calling mxs_phy_hw_init(mxs_phy) again at connection > > - Error connect/disconnect nodity at hub.c > > > > The coming patch will fix above two problems, Mike Thompson > > tested his hardware works OK after commented out this delay > > setting. > > > > This reverts commit 363366cf61c544ea476f3d220f43a95cb03014f5. > > > > Signed-off-by: Peter Chen > > I can't really take this series because it touches usbcore. So I rather > Greg took care of the entire series: > > Acked-by: Felipe Balbi Oh, I have found my mutt has broken these days, all my email have not sent out. Hi Alan, Can you help add Ack of usb core related (3/4, 4/4) of this patches series, you have reivewed v4 patchset, if you can't find them, I can send again. Thanks, 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
RE: [PATCH v5 1/4] Revert "usb: otg: mxs-phy: Fix mx23 operation"
> > On Mon, Oct 22, 2012 at 5:13 AM, Peter Chen > wrote: > > The real reason causes mx23 fail are: > > > > - Calling mxs_phy_hw_init(mxs_phy) again at connection > > - Error connect/disconnect nodity at hub.c > > > > The coming patch will fix above two problems, Mike Thompson > > So shouldn't this patch be the last one of the series? No, the later patches of this patchset is baesd on reverted version.
RE: [PATCH 4/4] usb: chipidea: imx: add internal vbus regulator control
> > @@ -89,14 +89,34 @@ static struct ci13xxx_platform_data
> > ci13xxx_imx_platdata __devinitdata = { .name =
> "ci13xxx_imx",
> > .flags = CI13XXX_REQUIRE_TRANSCEIVER |
> > CI13XXX_PULLUP_ON_VBUS |
> > - CI13XXX_DISABLE_STREAMING,
> > + CI13XXX_DISABLE_STREAMING |
> > + CI13XXX_REGS_SHARED,
>
> Why is this REGS_SHARED change needed?
>
After adding this id switch/vbus detect support, this chipidea
driver's behavior is much like msm's. The udc code may run at
non-device mode, the device/gadget code can't be freed even at
non-device mode.
> >
> > +static int ci13xxx_otg_set_vbus(struct usb_otg *otg, bool enabled)
> > +{
> > +
> > + struct ci13xxx *ci = container_of(otg, struct ci13xxx, otg);
> > + struct regulator *reg_vbus = ci->reg_vbus;
> > +
> > + WARN_ON(!reg_vbus);
> > +
> > + if (reg_vbus) {
>
> if (!reg_vbus) {
> WARN and return;
>
> }
>
Even reg_vbus is null, there will be no oops, besides, WARN_ON will
print dump.
>
> if (enabled)
> ...
>
> You'll cut down on the indent and will make it more readable I believe.
>
> [...]
Sorry, I can't understand you, can you give me an example?
Thanks,
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
RE: [PATCH 2/4] usb: chipidea: add otg id switch and vbus connect/disconnect detect
>
> Just a nitpick really, but what about parametrizing this code so we won't
> have
> two copies of the same loop?
>
> Say:
>
> uint32_t bit = low ? OTGSC_BSV : OTGSC_AWW;
> const char *name = low ? "B_SESS_VALID" : "A_VBUS_VALID";
>
> while (!(otgsc & bit)) {
> if (time_after(jiffies, timeout)) {
> dev_err(ci->dev, "wait vbus higher than\
> %s timeout!\n", name);
> return;
> }
> msleep(20);
> otgsc = hw_read(ci, OP_OTGSC, ~0);
> }
>
Good idea, I will change
> And shall we not be more careful about endless loops?
Yes, we should be careful of buggy hardware with vbus lower
very slow, so if timeout, return is needed.
>
> [...]
--
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: USB: chipidea: add vbus detect for udc
> > On Mon, Nov 05, 2012 at 08:27:40PM +0300, Dan Carpenter wrote: > > Hello Richard Zhao, > > > > The patch 8c4fc031954b: "USB: chipidea: add vbus detect for udc" from > > Sep 12, 2012, leads to the following Smatch warning: > > drivers/usb/chipidea/udc.c:1683 udc_irq() > > warn: odd binop '0x200 & 0x2' > > > > 1680 intr = hw_read(ci, OP_OTGSC, ~0); > > 1681 hw_write(ci, OP_OTGSC, ~0, intr); > > 1682 > > 1683 if (intr & (OTGSC_AVVIE & OTGSC_AVVIS)) > > ^ > > This is never true. I think it should either be: > > if (intr & (OTGSC_AVVIE | OTGSC_AVVIS)) > > or > > if ((intr & OTGSC_AVVIE) && (intr & OTGSC_AVVIE)) > > > > 1684 queue_work(ci->wq, &ci->vbus_work); > > 1685 > > > Hi Dan, Thanks for pointing this problem, I have already submitted a patchset which adds otg/vbus detect support for chipidea driver, in this patchset this problem is fixed. http://www.spinics.net/lists/linux-usb/msg73697.html Thanks, 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
RE: [PATCH 0/4] Add fully tested id switch and vbus connect detect support for Chipidea
> > This patchset adds fully tested otg id switch function and > vbus connect/disconnect detection for chipidea driver. > The mainly design of id/vbus handling follows msm otg driver. > I hope the msm usb maintainer can have a look of this patchset, > and give some comments, and move the whole msm usb driver to > chipidea framework if possible in the future. > > This patchset is fully tested at i.mx6Q saberlite board after adding > one pinctrl workaround(https://github.com/hzpeterchen/linux- > usb/commit/777425d693885a5f921a4b0bca45653a06e360dc) > > There is github repo for my chipdiea branch: > https://github.com/hzpeterchen/linux-usb.git > Hi Alexander, would you give some comments about this serial? Thanks. Peter > Peter Chen (4): > Revert "USB: chipidea: add vbus detect for udc" > usb: chipidea: add otg id switch and vbus connect/disconnect detect > usb: chipidea: udc: add pullup/pulldown dp at hw_device_state > usb: chipidea: imx: add internal vbus regulator control > > drivers/usb/chipidea/Makefile |2 +- > drivers/usb/chipidea/bits.h| 10 ++ > drivers/usb/chipidea/ci.h |9 ++- > drivers/usb/chipidea/ci13xxx_imx.c | 67 --- > drivers/usb/chipidea/core.c| 217 > +--- > drivers/usb/chipidea/otg.c | 60 ++ > drivers/usb/chipidea/otg.h |6 + > drivers/usb/chipidea/udc.c | 47 ++-- > 8 files changed, 342 insertions(+), 76 deletions(-) > create mode 100644 drivers/usb/chipidea/otg.c > create mode 100644 drivers/usb/chipidea/otg.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 -- 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: chipidea: ci13xxx_imx: Use dev_dbg for pinctrl message
Hi Fabio, I remebered devm_pinctrl_get_select_default will be returned successfully. Otherwise, how the patch included this code was tested? Best regards, Peter Chen From: Fabio Estevam [feste...@gmail.com] Sent: Sunday, April 28, 2013 3:28 AM To: Chen Peter-B29397 Cc: Uwe Kleine-König; Alexander Shishkin; gre...@linuxfoundation.org; ker...@pengutronix.de; linux-usb@vger.kernel.org; Estevam Fabio-R49496; m...@pengutronix.de; m.grzesc...@pengutronix.de; s.ha...@pengutronix.de Subject: Re: [PATCH] usb: chipidea: ci13xxx_imx: Use dev_dbg for pinctrl message On Fri, Apr 26, 2013 at 10:22 PM, Peter Chen wrote: > This pin is ID pin for OTG controller, some platforms use dedicated ID pin > which doesn't need to config pinctrl.(mx53, etc), some platforms can > choose ID pin from IOMUXC using pinctrl interface. Actually it is not only OTG ID pin, it can be any USB pin. Currently the i.MX device tree does not pass pinctrl node, so that's why devm_pinctrl_get_select_default fails. > I suggest that it uses of_match_table to differentiate platforms, and > import new function ci_imx_init to cover it. It may not the only > different things among i.mx SoC USB controllers. I don't understand what you explained here, but anyway it seems like a different issue. -- 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: chipidea: udc: configure iso endpoints
>
> This patch adds iso endpoint support to the device controller.
> It makes use of the multiplication bits in the maxpacket field
> of the endpoint and calculates the multiplier bits for each
> transfer description on every request.
>
> Signed-off-by: Michael Grzeschik
> ---
> drivers/usb/chipidea/core.c | 2 +-
> drivers/usb/chipidea/udc.c | 19 ++-
> drivers/usb/chipidea/udc.h | 1 +
> 3 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 450107e..3cdb889 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -43,7 +43,7 @@
> *
> * TODO List
> * - OTG
> - * - Isochronous & Interrupt Traffic
> + * - Interrupt Traffic
> * - Handle requests which spawns into several TDs
> * - GET_STATUS(device) - always reports 0
> * - Gadget API (majority of optional features)
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 3d90e61..84f5ec0 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -466,6 +466,13 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp,
> struct ci13xxx_req *mReq)
> mEp->qh.ptr->td.token &=
> cpu_to_le32(~(TD_STATUS_HALTED|TD_STATUS_ACTIVE));
>
> + if (mEp->type == USB_ENDPOINT_XFER_ISOC) {
> + u32 mul = mReq->req.length >> __ffs(mEp->ep.maxpacket);
> + if (mReq->req.length & ~(mul << __ffs(mEp->ep.maxpacket)))
> + mul++;
> + mEp->qh.ptr->cap |= mul << __ffs(QH_MULT);
> + }
> +
I think it is a little hard to read, why not
if (mEp->type == USB_ENDPOINT_XFER_ISOC) {
u32 mul = mReq->req.length / mEp->ep.maxpacket;
if (mReq->req.length % mEp->ep.maxpacket)
mul++;
mEp->qh.ptr->cap |= mul << __ffs(QH_MULT);
}
> wmb(); /* synchronize before ep prime */
>
> ret = hw_ep_prime(ci, mEp->num, mEp->dir,
> @@ -678,6 +685,12 @@ static int _ep_queue(struct usb_ep *ep, struct
> usb_request *req,
> }
> }
>
> + if (usb_endpoint_xfer_isoc(mEp->ep.desc)
> + && mReq->req.length > (1 + mEp->ep.mult) * mEp->ep.maxpacket) {
> + dev_err(mEp->ci->dev, "request length to big for
> isochronous\n");
> + return -EMSGSIZE;
> + }
> +
typo, "too big"
the request length should be less than 3*1024.
> /* first nuke then test link, e.g. previous status has not sent */
> if (!list_empty(&mReq->queue)) {
> dev_err(mEp->ci->dev, "request already in queue\n");
> @@ -1060,7 +1073,8 @@ static int ep_enable(struct usb_ep *ep,
> mEp->num = usb_endpoint_num(desc);
> mEp->type = usb_endpoint_type(desc);
>
> - mEp->ep.maxpacket = usb_endpoint_maxp(desc);
> + mEp->ep.maxpacket = usb_endpoint_maxp(desc) & 0x07ff;
> + mEp->ep.mult = QH_ISO_MULT(usb_endpoint_maxp(desc));
>
The above change doesn't need.
maxpacket <= 1024 for high speed
maxpacket <= 1023 for full speed
For high speed high bandwidth, it just has two or three transactions per
microframe
> if (mEp->type == USB_ENDPOINT_XFER_CONTROL)
> cap |= QH_IOS;
> @@ -1246,6 +1260,9 @@ static int ep_set_halt(struct usb_ep *ep, int value)
> if (ep == NULL || mEp->ep.desc == NULL)
> return -EINVAL;
>
> + if (usb_endpoint_xfer_isoc(mEp->ep.desc))
> + return -EOPNOTSUPP;
> +
> spin_lock_irqsave(mEp->lock, flags);
>
> #ifndef STALL_IN
> diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
> index d12e8b5..a75724a 100644
> --- a/drivers/usb/chipidea/udc.h
> +++ b/drivers/usb/chipidea/udc.h
> @@ -50,6 +50,7 @@ struct ci13xxx_qh {
> #define QH_MAX_PKT(0x07FFUL << 16)
> #define QH_ZLTBIT(29)
> #define QH_MULT (0x0003UL << 30)
> +#define QH_ISO_MULT(x) ((x >> 11) & 0x03)
> /* 1 */
> u32 curr;
> /* 2 - 8 */
> --
> 1.8.2.rc2
>
--
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: Chipidea usb otg support for IMX/MXS (device functionality)
> Hello, > > Am I right in assuming that the MXS USB on-the-go port does not currently > support the > device (gadget) functionality? > Anybody out there working on that? > As far as I know, Maxime Ripard may already let the chipidea durl-role function work ok at mx28. It may need my chipidea otg patch https://github.com/hzpeterchen/linux-usb.git 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
RE: Chipidea usb otg support for IMX/MXS (device functionality)
> > Indeed, I've been using the patchset "Add tested id switch and vbus > connect detect support for Chipidea" from Peter for quite some time on > top of 3.9 and it works like a charm for the gadget mode on an MX28 > platform. > > BTW, Peter, I've seen that these patches are still not merged in 3.10, > is there a reason for that? do you plan on sending a version rebased on > top of 3.10 some time in the future? I tried to do the rebasing myself, > but the chipidea driver seems to have changed quite heavily, which makes > the process quite difficult when you don't know what you're doing :) > I can spend few bandwidth on upstream work recently, I may have more bandwidth after June 15th. Currently, we still have no conclusion for chipidea core driver's coming work, like Device tree support, how to identify if the controller is OTG supported. Best regards, 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
RE: [PATCH 1/1] usb: chipidea: Improve kconfig
> > I did actually have this one in my tree, but since that time this > > commit 7c8bfed7aaeba690de30835fe89882e1047a55fd > Author: David Rientjes > Date: Fri Apr 26 13:25:01 2013 -0700 > > usb, chipidea: fix link error when USB_EHCI_HCD is a module > > has turned up in usb-next, and there's a conflict. So if you have time > to have a look at this, I'd appreciate that. > > My tree is at the same address on github. > David's fix just make chipidea host disappear when ehci hcd as a module. I will send a patch after merging david's. > Regards, > -- > Alex -- 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
