Re: [PATCH] usbnet: Fix a race between usbnet_stop() and the BH
01.09.2015 17:05, Eugene Shatokhin пишет:
The race may happen when a device (e.g. YOTA 4G LTE Modem) is
unplugged while the system is downloading a large file from the Net.
Hardware breakpoints and Kprobes with delays were used to confirm that
the race does actually happen.
The race is on skb_queue ('next' pointer) between usbnet_stop()
and rx_complete(), which, in turn, calls usbnet_bh().
Here is a part of the call stack with the code where the changes to the
queue happen. The line numbers are for the kernel 4.1.0:
*0 __skb_unlink (skbuff.h:1517)
prev->next = next;
*1 defer_bh (usbnet.c:430)
spin_lock_irqsave(&list->lock, flags);
old_state = entry->state;
entry->state = state;
__skb_unlink(skb, list);
spin_unlock(&list->lock);
spin_lock(&dev->done.lock);
__skb_queue_tail(&dev->done, skb);
if (dev->done.qlen == 1)
tasklet_schedule(&dev->bh);
spin_unlock_irqrestore(&dev->done.lock, flags);
*2 rx_complete (usbnet.c:640)
state = defer_bh(dev, skb, &dev->rxq, state);
At the same time, the following code repeatedly checks if the queue is
empty and reads these values concurrently with the above changes:
*0 usbnet_terminate_urbs (usbnet.c:765)
/* maybe wait for deletions to finish. */
while (!skb_queue_empty(&dev->rxq)
&& !skb_queue_empty(&dev->txq)
&& !skb_queue_empty(&dev->done)) {
schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
set_current_state(TASK_UNINTERRUPTIBLE);
netif_dbg(dev, ifdown, dev->net,
"waited for %d urb completions\n", temp);
}
*1 usbnet_stop (usbnet.c:806)
if (!(info->flags & FLAG_AVOID_UNLINK_URBS))
usbnet_terminate_urbs(dev);
As a result, it is possible, for example, that the skb is removed from
dev->rxq by __skb_unlink() before the check
"!skb_queue_empty(&dev->rxq)" in usbnet_terminate_urbs() is made. It is
also possible in this case that the skb is added to dev->done queue
after "!skb_queue_empty(&dev->done)" is checked. So
usbnet_terminate_urbs() may stop waiting and return while dev->done
queue still has an item.
Locking in defer_bh() and usbnet_terminate_urbs() was revisited to avoid
this race.
Signed-off-by: Eugene Shatokhin
---
drivers/net/usb/usbnet.c | 39 ---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index e049857..b4cf107 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -428,12 +428,18 @@ static enum skb_state defer_bh(struct usbnet *dev, struct
sk_buff *skb,
old_state = entry->state;
entry->state = state;
__skb_unlink(skb, list);
- spin_unlock(&list->lock);
- spin_lock(&dev->done.lock);
+
+ /* defer_bh() is never called with list == &dev->done.
+* spin_lock_nested() tells lockdep that it is OK to take
+* dev->done.lock here with list->lock held.
+*/
+ spin_lock_nested(&dev->done.lock, SINGLE_DEPTH_NESTING);
+
__skb_queue_tail(&dev->done, skb);
if (dev->done.qlen == 1)
tasklet_schedule(&dev->bh);
- spin_unlock_irqrestore(&dev->done.lock, flags);
+ spin_unlock(&dev->done.lock);
+ spin_unlock_irqrestore(&list->lock, flags);
return old_state;
}
@@ -749,6 +755,20 @@ EXPORT_SYMBOL_GPL(usbnet_unlink_rx_urbs);
/*-*/
+static void wait_skb_queue_empty(struct sk_buff_head *q)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&q->lock, flags);
+ while (!skb_queue_empty(q)) {
+ spin_unlock_irqrestore(&q->lock, flags);
+ schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ spin_lock_irqsave(&q->lock, flags);
+ }
+ spin_unlock_irqrestore(&q->lock, flags);
+}
+
// precondition: never called in_interrupt
static void usbnet_terminate_urbs(struct usbnet *dev)
{
@@ -762,14 +782,11 @@ static void usbnet_terminate_urbs(struct usbnet *dev)
unlink_urbs(dev, &dev->rxq);
/* maybe wait for deletions to finish. */
- while (!skb_queue_empty(&dev->rxq)
- && !skb_queue_empty(&dev->txq)
- && !skb_queue_empty(&dev->done)) {
- schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
- set_current_state(TASK_UNINTERRUPTIBLE);
- netif_dbg(dev, ifdown, dev->net,
- "waited for %d urb completions\n", temp);
- }
+ wait_skb_queue_empty(&dev->rxq);
+ wait_skb_queue_empty(&dev->txq);
+ wait_skb_queue_empty(&dev->done);
+ netif_dbg(dev, ifdown, dev->net,
+ "waited for %d urb completions\n", temp);
set_current_state(TASK_RUNNING);
remove_wait_queue(&
Re: [PATCH v4 0/22] On-demand device probing
On 7 September 2015 at 22:50, Rob Herring wrote: > On Mon, Sep 7, 2015 at 7:23 AM, Tomeu Vizoso > wrote: >> Hello, >> >> I have a problem with the panel on my Tegra Chromebook taking longer >> than expected to be ready during boot (Stéphane Marchesin reported what >> is basically the same issue in [0]), and have looked into ordered >> probing as a better way of solving this than moving nodes around in the >> DT or playing with initcall levels and linking order. >> >> While reading the thread [1] that Alexander Holler started with his >> series to make probing order deterministic, it occurred to me that it >> should be possible to achieve the same by probing devices as they are >> referenced by other devices. >> >> This basically reuses the information that is already implicit in the >> probe() implementations, saving us from refactoring existing drivers or >> adding information to DTBs. >> >> During review of v1 of this series Linus Walleij suggested that it >> should be the device driver core to make sure that dependencies are >> ready before probing a device. I gave this idea a try [2] but Mark Brown >> pointed out to the logic duplication between the resource acquisition >> and dependency discovery code paths (though I think it's fairly minor). >> >> To address that code duplication I experimented with Arnd's devm_probe >> [3] concept of having drivers declare their dependencies instead of >> acquiring them during probe, and while it worked [4], I don't think we >> end up winning anything when compared to just probing devices on-demand >> from resource getters. >> >> One remaining objection is to the "sprinkling" of calls to >> of_device_probe() in the resource getters of each subsystem, but I think >> it's the right thing to do given that the storage of resources is >> currently subsystem-specific. >> >> We could avoid the above by moving resource storage into the core, but I >> don't think there's a compelling case for that. >> >> I have tested this on boards with Tegra, iMX.6, Exynos, Rockchip and >> OMAP SoCs, and these patches were enough to eliminate all the deferred >> probes (except one in PandaBoard because omap_dma_system doesn't have a >> firmware node as of yet). >> >> Have submitted a branch [5] with only these patches on top of thursday's >> linux-next to kernelci.org and I don't see any issues that could be >> caused by them. For some reason it currently has more passes than the >> version of -next it's based on! >> >> With this series I get the kernel to output to the panel in 0.5s, >> instead of 2.8s. >> >> Regards, >> >> Tomeu >> >> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html >> >> [1] https://lkml.org/lkml/2014/5/12/452 >> >> [2] https://lkml.org/lkml/2015/6/17/305 >> >> [3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689 >> >> [4] https://lkml.org/lkml/2015/7/21/441a >> >> [5] >> https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v6 >> >> [6] >> http://kernelci.org/boot/all/job/collabora/kernel/v4.2-11902-g25d80c927f8b/ >> >> [7] http://kernelci.org/boot/all/job/next/kernel/next-20150903/ >> >> Changes in v4: >> - Added bus.pre_probe callback so the probes of Primecell devices can be >> deferred if their device IDs cannot be yet read because of the clock >> driver not having probed when they are registered. Maybe this goes >> overboard and the matching information should be in the DT if there is >> one. > > Seems overboard to me or at least a separate problem. It's a separate problem but this was preventing the series from working on a few boards. > Most clocks have > to be setup before the driver model simply because timers depend on > clocks usually. Yes, but in this case the apb clocks for the primecell devices are implemented in a normal platform driver (vexpress_osc_driver), instead of using CLK_OF_DECLARE. Regards, Tomeu > Rob > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- 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] usbnet: Fix a race between usbnet_stop() and the BH
Eugene Shatokhin writes: > I resent the patch to make it separate. What is the status of this now? One of the many nice features of patchwork is that you don't need to ask those questions :) See http://patchwork.ozlabs.org/patch/512856/ I really don't think it's appropriate for me to ack this, but I can add my Reviewed-by: Bjørn Mork if that helps. Bjørn -- 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] usbnet: Fix a race between usbnet_stop() and the BH
On Tue, 2015-09-08 at 09:37 +0200, Bjørn Mork wrote: > Eugene Shatokhin writes: > > > I resent the patch to make it separate. What is the status of this now? > > One of the many nice features of patchwork is that you don't need to ask > those questions :) > > See http://patchwork.ozlabs.org/patch/512856/ > > I really don't think it's appropriate for me to ack this, but I can add > my Well, in case this helps: > Reviewed-by: Bjørn Mork Acked-by: Oliver Neukum Regards Oliver -- 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 v4 04/13] otg-fsm: move usb_bus_start_enum into otg-fsm->ops
On Mon, Sep 07, 2015 at 12:57:21PM +0300, Roger Quadros wrote:
> On 07/09/15 04:24, Peter Chen wrote:
> > On Mon, Aug 24, 2015 at 04:21:15PM +0300, Roger Quadros wrote:
> >> This is to prevent missing symbol build error if OTG is
> >> enabled (built-in) and HCD core (CONFIG_USB) is module.
> >>
> >> Signed-off-by: Roger Quadros
> >> Acked-by: Peter Chen
> >> ---
> >> drivers/usb/common/usb-otg-fsm.c | 6 --
> >> drivers/usb/phy/phy-fsl-usb.c| 2 ++
> >> include/linux/usb/otg-fsm.h | 1 +
> >> 3 files changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/common/usb-otg-fsm.c
> >> b/drivers/usb/common/usb-otg-fsm.c
> >> index a46f29a..6e56c8c 100644
> >> --- a/drivers/usb/common/usb-otg-fsm.c
> >> +++ b/drivers/usb/common/usb-otg-fsm.c
> >> @@ -165,8 +165,10 @@ static int otg_set_state(struct otg_fsm *fsm, enum
> >> usb_otg_state new_state)
> >>otg_loc_conn(fsm, 0);
> >>otg_loc_sof(fsm, 1);
> >>otg_set_protocol(fsm, PROTO_HOST);
> >> - usb_bus_start_enum(fsm->otg->host,
> >> - fsm->otg->host->otg_port);usb_bus_start_enum
> >> + if (fsm->ops->start_enum) {
> >> + fsm->ops->start_enum(fsm->otg->host,
> >> + fsm->otg->host->otg_port);
> >> + }
> >>break;
> >>case OTG_STATE_A_IDLE:
> >>otg_drv_vbus(fsm, 0);
> >> diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
> >> index ee3f2c2..19541ed 100644
> >> --- a/drivers/usb/phy/phy-fsl-usb.c
> >> +++ b/drivers/usb/phy/phy-fsl-usb.c
> >> @@ -783,6 +783,8 @@ static struct otg_fsm_ops fsl_otg_ops = {
> >>
> >>.start_host = fsl_otg_start_host,
> >>.start_gadget = fsl_otg_start_gadget,
> >> +
> >> + .start_enum = usb_bus_start_enum,
> >> };
> >>
> >> /* Initialize the global variable fsl_otg_dev and request IRQ for OTG */
> >> diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
> >> index 672551c..75e82cc 100644
> >> --- a/include/linux/usb/otg-fsm.h
> >> +++ b/include/linux/usb/otg-fsm.h
> >> @@ -199,6 +199,7 @@ struct otg_fsm_ops {
> >>void(*del_timer)(struct otg_fsm *fsm, enum otg_fsm_timer timer);
> >>int (*start_host)(struct otg_fsm *fsm, int on);
> >>int (*start_gadget)(struct otg_fsm *fsm, int on);
> >> + int (*start_enum)(struct usb_bus *bus, unsigned port_num);
> >> };
> >>
> >>
> >
> > Get one build warning:
> >
> > In file included from
> > /u/home/b29397/work/projects/usb/drivers/usb/chipidea/udc.c:23:0:
> > /u/home/b29397/work/projects/usb/include/linux/usb/otg-fsm.h:207:27:
> > warning: 'struct usb_bus' declared inside parameter list
> > int (*start_enum)(struct usb_bus *bus, unsigned port_num);
> > ^
> > /u/home/b29397/work/projects/usb/include/linux/usb/otg-fsm.h:207:27:
> > warning: its scope is only this definition or declaration, which is
> > probably not what you want
> >
> > It probably dues to we should not have struct usb_bus* at udc driver
> >
> How about changing it to struct otg_fsm instead like the other APIs?
> And do we leave usb_bus_start_enum() as it is?
>
You have defined struct otg_hcd_ops to let otg visit hcd stuff, how
about move this to otg_hcd_ops?
> cheers,
> -roger
--
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 v4 04/13] otg-fsm: move usb_bus_start_enum into otg-fsm->ops
On 08/09/15 09:54, Peter Chen wrote:
> On Mon, Sep 07, 2015 at 12:57:21PM +0300, Roger Quadros wrote:
>> On 07/09/15 04:24, Peter Chen wrote:
>>> On Mon, Aug 24, 2015 at 04:21:15PM +0300, Roger Quadros wrote:
This is to prevent missing symbol build error if OTG is
enabled (built-in) and HCD core (CONFIG_USB) is module.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
drivers/usb/common/usb-otg-fsm.c | 6 --
drivers/usb/phy/phy-fsl-usb.c| 2 ++
include/linux/usb/otg-fsm.h | 1 +
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/common/usb-otg-fsm.c
b/drivers/usb/common/usb-otg-fsm.c
index a46f29a..6e56c8c 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -165,8 +165,10 @@ static int otg_set_state(struct otg_fsm *fsm, enum
usb_otg_state new_state)
otg_loc_conn(fsm, 0);
otg_loc_sof(fsm, 1);
otg_set_protocol(fsm, PROTO_HOST);
- usb_bus_start_enum(fsm->otg->host,
- fsm->otg->host->otg_port);usb_bus_start_enum
+ if (fsm->ops->start_enum) {
+ fsm->ops->start_enum(fsm->otg->host,
+ fsm->otg->host->otg_port);
+ }
break;
case OTG_STATE_A_IDLE:
otg_drv_vbus(fsm, 0);
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index ee3f2c2..19541ed 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -783,6 +783,8 @@ static struct otg_fsm_ops fsl_otg_ops = {
.start_host = fsl_otg_start_host,
.start_gadget = fsl_otg_start_gadget,
+
+ .start_enum = usb_bus_start_enum,
};
/* Initialize the global variable fsl_otg_dev and request IRQ for OTG */
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 672551c..75e82cc 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -199,6 +199,7 @@ struct otg_fsm_ops {
void(*del_timer)(struct otg_fsm *fsm, enum otg_fsm_timer timer);
int (*start_host)(struct otg_fsm *fsm, int on);
int (*start_gadget)(struct otg_fsm *fsm, int on);
+ int (*start_enum)(struct usb_bus *bus, unsigned port_num);
};
>>>
>>> Get one build warning:
>>>
>>> In file included from
>>> /u/home/b29397/work/projects/usb/drivers/usb/chipidea/udc.c:23:0:
>>> /u/home/b29397/work/projects/usb/include/linux/usb/otg-fsm.h:207:27:
>>> warning: 'struct usb_bus' declared inside parameter list
>>> int (*start_enum)(struct usb_bus *bus, unsigned port_num);
>>> ^
>>> /u/home/b29397/work/projects/usb/include/linux/usb/otg-fsm.h:207:27:
>>> warning: its scope is only this definition or declaration, which is
>>> probably not what you want
>>>
>>> It probably dues to we should not have struct usb_bus* at udc driver
>>>
>> How about changing it to struct otg_fsm instead like the other APIs?
>> And do we leave usb_bus_start_enum() as it is?
>>
>
> You have defined struct otg_hcd_ops to let otg visit hcd stuff, how
> about move this to otg_hcd_ops?
Yes, this is a better idea. Thanks.
cheers,
-roger
--
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 v1] media: uvcvideo: handle urb completion in a work queue
On Mon, 2015-09-07 at 18:23 +0200, Mian Yousaf Kaukab wrote: > urb completion callback is executed in host controllers interrupt > context. To keep preempt disable time short, add urbs to a list on > completion and schedule work to process the list. > > Moreover, save timestamp and sof number in the urb completion callback > to avoid any delays. > > Signed-off-by: Mian Yousaf Kaukab > --- > History: > v1: > - Use global work queue instead of creating ordered queue. 1. using a common queue for real-time work is probably not nice for picture quality 2. it will deadlock under some conditions The explanation is a bit long Suppose we have a device with a camera and a storage device, like an ordinary camera you can use as a video device which also exports its memory card. Now we assume that the storage part is suspended. CPU A CPU B work item scheduled entering uvc_uninit_video() work item executed work item allocates memory write to storage interface storage interface being resumed flush_work() - waiting for CPU B DEADLOCK If you want to use flush_work() you must use a dedicated queue. Regards Oliver -- 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 v4 07/13] usb: otg: add OTG core
On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
> On 07/09/15 04:23, Peter Chen wrote:
> > On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
> >> + * This is used by the USB Host stack to register the Host controller
> >> + * to the OTG core. Host controller must not be started by the
> >> + * caller as it is left upto the OTG state machine to do so.
> >> + *
> >> + * Returns: 0 on success, error value otherwise.
> >> + */
> >> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> >> + unsigned long irqflags, struct otg_hcd_ops *ops)
> >> +{
> >> + struct usb_otg *otgd;
> >> + struct device *hcd_dev = hcd->self.controller;
> >> + struct device *otg_dev = usb_otg_get_device(hcd_dev);
> >> +
> >
> > One big problem here is: there are two designs for current (IP) driver
> > code, one creates dedicated hcd device as roothub's parent, like dwc3.
> > Another one doesn't do this, roothub's parent is core device (or otg device
> > in your patch), like chipidea and dwc2.
> >
> > Then, otg_dev will be glue layer device for chipidea after that.
>
> OK. Let's add a way for the otg controller driver to provide the host and
> gadget
> information to the otg core for such devices like chipidea and dwc2.
>
Roger, not only chipidea and dwc2, I think the musb uses the same
hierarchy. If the host, device, and otg share the same register
region, host part can't be a platform driver since we don't want
to remap the same register region again.
So, in the design, we may need to consider both situations, one
is otg/host/device has its own register region, and host is a
separate platform device (A), the other is three parts share the
same register region, there is only one platform driver (B).
A:
IP core device
|
|
|-|-|
gadget host platform device
|
roothub
B:
IP core device
|
|
|-|-|
gadget roothub
> This API must be called before the hcd/gadget-driver is added so that the otg
> core knows it's linked to an OTG controller.
>
> Any better idea?
>
A flag stands for this hcd controller is the same with otg controller
can be used, this flag can be stored at struct usb_otg_config.
Peter
P.S: I still read your code, I find not all APIs in this file are used
in your dwc3 example.
> >
> >> + if (!otg_dev)
> >> + return -EINVAL; /* we're definitely not OTG */
> >> +
> >> + /* we're otg but otg controller might not yet be registered */
> >> + mutex_lock(&otg_list_mutex);
> >> + otgd = usb_otg_get_data(otg_dev);
> >> + mutex_unlock(&otg_list_mutex);
> >> + if (!otgd) {
> >> + dev_dbg(hcd_dev,
> >> + "otg: controller not yet registered. waiting..\n");
> >> + /*
> >> + * otg controller might register later. Put the hcd in
> >> + * wait list and call us back when ready
> >> + */
> >> + if (usb_otg_hcd_wait_add(otg_dev, hcd, irqnum, irqflags, ops)) {
> >> + dev_dbg(hcd_dev, "otg: failed to add to wait list\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + return 0;
> >> + }
> >> +
> >> + /* HCD will be started by OTG fsm when needed */
> >> + mutex_lock(&otgd->fsm.lock);
> >> + if (otgd->primary_hcd.hcd) {
> >> + /* probably a shared HCD ? */
> >> + if (usb_otg_hcd_is_primary_hcd(hcd)) {
> >> + dev_err(otg_dev, "otg: primary host already
> >> registered\n");
> >> + goto err;
> >> + }
> >> +
> >> + if (hcd->shared_hcd == otgd->primary_hcd.hcd) {
> >> + if (otgd->shared_hcd.hcd) {
> >> + dev_err(otg_dev, "otg: shared host already
> >> registered\n");
> >> + goto err;
> >> + }
> >> +
> >> + otgd->shared_hcd.hcd = hcd;
> >> + otgd->shared_hcd.irqnum = irqnum;
> >> + otgd->shared_hcd.irqflags = irqflags;
> >> + otgd->shared_hcd.ops = ops;
> >> + dev_info(otg_dev, "otg: shared host %s registered\n",
> >> + dev_name(hcd->self.controller));
> >> + } else {
> >> + dev_err(otg_dev, "otg: invalid shared host %s\n",
> >> + dev_name(hcd->self.controller));
> >> + goto err;
> >> + }
> >> + } else {
> >> + if (!usb_otg_hcd_is_primary_hcd(hcd)) {
> >> + dev_err(otg_dev, "otg: primary host must be registered
> >> first\n");
> >> + goto err;
> >> + }
> >> +
> >> + otgd->
Re: "Tell linux-usb@vger.kernel.org to add your device to a proper driver"
2015-09-04 17:53 GMT-03:00 Greg KH : > On Fri, Sep 04, 2015 at 08:47:40PM +, Katarína Križáni wrote: >> So how should I install it correctly so that it "just runs" on Ubuntu? I did >> it first time: I am learning. > > Install what? > > The driver should "just work", no need to install anything, just plug > the device in and the driver will be loaded. Yes. I don't click "install", should loaded automatic. -- Albino B Neto www.bino.us -- 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 v7 4/5] xhci: mediatek: support MTK xHCI host controller
Hi On 08.09.2015 09:18, Chunfeng Yun wrote: There some vendor quirks for MTK xhci host controller: 1. It defines some extra SW scheduling parameters for HW to minimize the scheduling effort for synchronous and interrupt endpoints. The parameters are put into reseved DWs of slot context and endpoint context. 2. Its IMODI unit for Interrupter Moderation register is 8 times as much as that defined in xHCI spec. 3. Its TDS in Normal TRB defines a number of packets that remains to be transferred for a TD after processing all Max packets in all previous TRBs. I just rewrote and unified the the xhci TD remainder functions, now we got only one xhci_td_remainder() function which should be more suitable for quirks. You can add the quirk directly to it, no need to duplicate the whole function. -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] usb: chipidea: Use extcon framework for VBUS and ID detect
On Mon, Sep 07, 2015 at 02:45:25PM +0300, Ivan T. Ivanov wrote:
> On recent Qualcomm platforms VBUS and ID lines are not routed to
> USB PHY LINK controller. Use extcon framework to receive connect
> and disconnect ID and VBUS notification.
>
> Signed-off-by: Ivan T. Ivanov
I will queue it for next rc1, thanks.
Peter
> ---
>
> Changes sice v3 [1]:
>
> * Migrate to new extcon framework API
> * Address comments from Peter Chen.
>
> Tested DRD role with "qcom,ci-hdrc" and "qcom,usb-8x16-phy" on DB410c
>
> [1] https://lkml.org/lkml/2015/6/2/333
>
> .../devicetree/bindings/usb/ci-hdrc-usb2.txt | 6 +
> drivers/usb/chipidea/Kconfig | 1 +
> drivers/usb/chipidea/core.c| 125
> +
> drivers/usb/chipidea/otg.c | 39 ++-
> include/linux/usb/chipidea.h | 23
> 5 files changed, 193 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index d71ef07bca5d..aea7d0af5fd6 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -45,6 +45,11 @@ Optional properties:
>(4 bytes), This register represents the maximum length of a the burst
>in 32-bit words while moving data from the USB bus to system memory,
>changing this value takes effect only the SBUSCFG.AHBBRST is 0.
> +- extcon: phandles to external connector devices. First phandle should point
> to
> + external connector, which provide "USB" cable events, the second should
> point
> + to external connector device, which provide "USB-HOST" cable events. If one
> + of the external connector devices is not required, empty <0> phandle should
> + be specified.
>
> Example:
>
> @@ -61,4 +66,5 @@ Example:
> ahb-burst-config = <0x0>;
> tx-burst-size-dword = <0x10>; /* 64 bytes */
> rx-burst-size-dword = <0x10>;
> + extcon = <0>, <&usb_id>;
> };
> diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
> index 5ce3f1d6a6ed..5619b8ca3bf3 100644
> --- a/drivers/usb/chipidea/Kconfig
> +++ b/drivers/usb/chipidea/Kconfig
> @@ -1,6 +1,7 @@
> config USB_CHIPIDEA
> tristate "ChipIdea Highspeed Dual Role Controller"
> depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD &&
> !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA
> + select EXTCON
> help
> Say Y here if your system has a dual role high speed USB
> controller based on ChipIdea silicon IP. Currently, only the
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 3feebf7f31f0..573c2876b263 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -47,6 +47,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -602,9 +603,45 @@ static irqreturn_t ci_irq(int irq, void *data)
> return ret;
> }
>
> +static int ci_vbus_notifier(struct notifier_block *nb, unsigned long event,
> + void *ptr)
> +{
> + struct ci_hdrc_cable *vbus = container_of(nb, struct ci_hdrc_cable, nb);
> + struct ci_hdrc *ci = vbus->ci;
> +
> + if (event)
> + vbus->state = true;
> + else
> + vbus->state = false;
> +
> + vbus->changed = true;
> +
> + ci_irq(ci->irq, ci);
> + return NOTIFY_DONE;
> +}
> +
> +static int ci_id_notifier(struct notifier_block *nb, unsigned long event,
> + void *ptr)
> +{
> + struct ci_hdrc_cable *id = container_of(nb, struct ci_hdrc_cable, nb);
> + struct ci_hdrc *ci = id->ci;
> +
> + if (event)
> + id->state = false;
> + else
> + id->state = true;
> +
> + id->changed = true;
> +
> + ci_irq(ci->irq, ci);
> + return NOTIFY_DONE;
> +}
> +
> static int ci_get_platdata(struct device *dev,
> struct ci_hdrc_platform_data *platdata)
> {
> + struct extcon_dev *ext_vbus, *ext_id;
> + struct ci_hdrc_cable *cable;
> int ret;
>
> if (!platdata->phy_mode)
> @@ -695,9 +732,91 @@ static int ci_get_platdata(struct device *dev,
> platdata->flags |= CI_HDRC_OVERRIDE_RX_BURST;
> }
>
> + ext_id = ERR_PTR(-ENODEV);
> + ext_vbus = ERR_PTR(-ENODEV);
> + if (of_property_read_bool(dev->of_node, "extcon")) {
> + /* Each one of them is not mandatory */
> + ext_vbus = extcon_get_edev_by_phandle(dev, 0);
> + if (IS_ERR(ext_vbus) && PTR_ERR(ext_vbus) != -ENODEV)
> + return PTR_ERR(ext_vbus);
> +
> + ext_id = extcon_get_edev_by_phandle(dev, 1);
> + if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
> + return PTR_ERR(ext_id);
> + }
> +
> + cable = &platdata->vbus_extcon;
[PATCH] xhci: create one unified function to calculate TRB TD remainder.
xhci versions 1.0 and later report the untransferred data remaining in a
TD a bit differently than older hosts.
We used to have separate functions for these, and needed to check host
version before calling the right function.
Now Mediatek host has an additional quirk on how it uses the TD Size
field for remaining data. To prevent yet another function for calculating
remainder we instead want to make one quirk friendly unified function.
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-ring.c | 106 ++-
drivers/usb/host/xhci.h | 2 +
2 files changed, 46 insertions(+), 62 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a47a1e8..57f40a1 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3020,21 +3020,6 @@ int xhci_queue_intr_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
}
/*
- * The TD size is the number of bytes remaining in the TD (including this TRB),
- * right shifted by 10.
- * It must fit in bits 21:17, so it can't be bigger than 31.
- */
-static u32 xhci_td_remainder(unsigned int remainder)
-{
- u32 max = (1 << (21 - 17 + 1)) - 1;
-
- if ((remainder >> 10) >= max)
- return max << 17;
- else
- return (remainder >> 10) << 17;
-}
-
-/*
* For xHCI 1.0 host controllers, TD size is the number of max packet sized
* packets remaining in the TD (*not* including this TRB).
*
@@ -3046,30 +3031,36 @@ static u32 xhci_td_remainder(unsigned int remainder)
*
* TD size = total_packet_count - packets_transferred
*
- * It must fit in bits 21:17, so it can't be bigger than 31.
+ * For xHCI 0.96 and older, TD size field should be the remaining bytes
+ * including this TRB, right shifted by 10
+ *
+ * For all hosts it must fit in bits 21:17, so it can't be bigger than 31.
+ * This is taken care of in the TRB_TD_SIZE() macro
+ *
* The last TRB in a TD must have the TD size set to zero.
*/
-static u32 xhci_v1_0_td_remainder(int running_total, int trb_buff_len,
- unsigned int total_packet_count, struct urb *urb,
- unsigned int num_trbs_left)
+static u32 xhci_td_remainder(struct xhci_hcd *xhci, int transferred,
+ int trb_buff_len, unsigned int td_total_len,
+ struct urb *urb, unsigned int num_trbs_left)
{
- int packets_transferred;
+ u32 maxp, total_packet_count;
+
+ if (xhci->hci_version < 0x100)
+ return ((td_total_len - transferred) >> 10);
+
+ maxp = GET_MAX_PACKET(usb_endpoint_maxp(&urb->ep->desc));
+ total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
/* One TRB with a zero-length data packet. */
- if (num_trbs_left == 0 || (running_total == 0 && trb_buff_len == 0))
+ if (num_trbs_left == 0 || (transferred == 0 && trb_buff_len == 0) ||
+ trb_buff_len == td_total_len)
return 0;
- /* All the TRB queueing functions don't count the current TRB in
-* running_total.
-*/
- packets_transferred = (running_total + trb_buff_len) /
- GET_MAX_PACKET(usb_endpoint_maxp(&urb->ep->desc));
-
- if ((total_packet_count - packets_transferred) > 31)
- return 31 << 17;
- return (total_packet_count - packets_transferred) << 17;
+ /* Queueing functions don't count the current TRB into transferred */
+ return (total_packet_count - ((transferred + trb_buff_len) / maxp));
}
+
static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
struct urb *urb, int slot_id, unsigned int ep_index)
{
@@ -3191,17 +3182,12 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
}
/* Set the TRB length, TD size, and interrupter fields. */
- if (xhci->hci_version < 0x100) {
- remainder = xhci_td_remainder(
- urb->transfer_buffer_length -
- running_total);
- } else {
- remainder = xhci_v1_0_td_remainder(running_total,
- trb_buff_len, total_packet_count, urb,
- num_trbs - 1);
- }
+ remainder = xhci_td_remainder(xhci, running_total, trb_buff_len,
+ urb->transfer_buffer_length,
+ urb, num_trbs - 1);
+
length_field = TRB_LEN(trb_buff_len) |
- remainder |
+ TRB_TD_SIZE(remainder) |
TRB_INTR_TARGET(0);
if (num_trbs > 1)
@@ -3364,17 +3350,12 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
field |= TRB_ISP;
/* Set the TRB length, TD size, and interrupter fiel
Re: [PATCH] xhci: create one unified function to calculate TRB TD remainder.
On Tue, 2015-09-08 at 14:09 +0300, Mathias Nyman wrote: > Now Mediatek host has an additional quirk on how it uses the TD Size > field for remaining data. To prevent yet another function for > calculating > remainder we instead want to make one quirk friendly unified function. Could you clarify whether this replaces an existing quirk or renders unnecessary the introduction of a new quirk, because that decides whether this patch must go into stable. Regards Oliver -- 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] xhci: create one unified function to calculate TRB TD remainder.
On 08.09.2015 14:46, Oliver Neukum wrote: On Tue, 2015-09-08 at 14:09 +0300, Mathias Nyman wrote: Now Mediatek host has an additional quirk on how it uses the TD Size field for remaining data. To prevent yet another function for calculating remainder we instead want to make one quirk friendly unified function. Could you clarify whether this replaces an existing quirk or renders unnecessary the introduction of a new quirk, because that decides whether this patch must go into stable. Neither :) This patch will simplify the quirk Mediatek wants to include in their patchseries. After this patch the TD size part of the mediatek quirk can be reduced to maybe a couple lines in total. Yet another version of the Mediatek patcheries will be needed after this, but it will be 50 - 100 lines shorter. This patch is basically just code refactoring, enabling easier quirking. It seems I need to do some rewording of the commit message of this patch as well -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 07/13] usb: otg: add OTG core
On 08/09/15 11:31, Peter Chen wrote:
> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
>> On 07/09/15 04:23, Peter Chen wrote:
>>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
+ * This is used by the USB Host stack to register the Host controller
+ * to the OTG core. Host controller must not be started by the
+ * caller as it is left upto the OTG state machine to do so.
+ *
+ * Returns: 0 on success, error value otherwise.
+ */
+int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
+ unsigned long irqflags, struct otg_hcd_ops *ops)
+{
+ struct usb_otg *otgd;
+ struct device *hcd_dev = hcd->self.controller;
+ struct device *otg_dev = usb_otg_get_device(hcd_dev);
+
>>>
>>> One big problem here is: there are two designs for current (IP) driver
>>> code, one creates dedicated hcd device as roothub's parent, like dwc3.
>>> Another one doesn't do this, roothub's parent is core device (or otg device
>>> in your patch), like chipidea and dwc2.
>>>
>>> Then, otg_dev will be glue layer device for chipidea after that.
>>
>> OK. Let's add a way for the otg controller driver to provide the host and
>> gadget
>> information to the otg core for such devices like chipidea and dwc2.
>>
>
> Roger, not only chipidea and dwc2, I think the musb uses the same
> hierarchy. If the host, device, and otg share the same register
> region, host part can't be a platform driver since we don't want
> to remap the same register region again.
>
> So, in the design, we may need to consider both situations, one
> is otg/host/device has its own register region, and host is a
> separate platform device (A), the other is three parts share the
> same register region, there is only one platform driver (B).
>
> A:
>
> IP core device
> |
> |
> |-|-|
> gadget host platform device
> |
> roothub
>
> B:
>
> IP core device
> |
> |
> |-|-|
> gadget roothub
>
>
>> This API must be called before the hcd/gadget-driver is added so that the otg
>> core knows it's linked to an OTG controller.
>>
>> Any better idea?
>>
>
> A flag stands for this hcd controller is the same with otg controller
> can be used, this flag can be stored at struct usb_otg_config.
What if there is another architecture like so?
C:
[Parent]
|
|
|--|--|
[OTG core] [gadget][host]
We need a more flexible mechanism to link the gadget and
host device to the otg core for non DT case.
How about adding struct usb_otg parameter to usb_otg_register_hcd()?
e.g.
int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
If otg is NULL it will try DT otg-controller property or parent to
get the otg controller.
>
> Peter
>
> P.S: I still read your code, I find not all APIs in this file are used
> in your dwc3 example.
Which ones? The ones for registering/unregistered host/gadget are used
by hcd/udc core as part of usb_add/remove_hcd() and
udc_bind_to_driver()/usb_gadget_remove_driver()
cheers,
-roger
--
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 v1] media: uvcvideo: handle urb completion in a work queue
Hi,
On 09/07/2015 06:23 PM, Mian Yousaf Kaukab wrote:
urb completion callback is executed in host controllers interrupt
context. To keep preempt disable time short, add urbs to a list on
completion and schedule work to process the list.
Moreover, save timestamp and sof number in the urb completion callback
to avoid any delays.
Erm, I thought that we had already moved to using threaded interrupt
handling for the urb completion a while (1-2 years ?) back. Is this then
still needed ?
Regards,
Hans
Signed-off-by: Mian Yousaf Kaukab
---
History:
v1:
- Use global work queue instead of creating ordered queue.
- Add completed urbs to a list and process all on the list when work
is scheduled
- Save timestamp and sof number in urb completion callback and use
in uvc_video_clock_decode() and uvc_video_decode_start()
- Fix race between usb_submit_urb() from uvc_video_complete() and
usb_kill_urb() from uvc_uninit_video()
drivers/media/usb/uvc/uvc_isight.c | 3 +-
drivers/media/usb/uvc/uvc_video.c | 132 +++--
drivers/media/usb/uvc/uvcvideo.h | 17 -
3 files changed, 113 insertions(+), 39 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_isight.c
b/drivers/media/usb/uvc/uvc_isight.c
index 8510e725..7f93d09 100644
--- a/drivers/media/usb/uvc/uvc_isight.c
+++ b/drivers/media/usb/uvc/uvc_isight.c
@@ -99,9 +99,10 @@ static int isight_decode(struct uvc_video_queue *queue,
struct uvc_buffer *buf,
return 0;
}
-void uvc_video_decode_isight(struct urb *urb, struct uvc_streaming *stream,
+void uvc_video_decode_isight(struct uvc_urb *uu, struct uvc_streaming *stream,
struct uvc_buffer *buf)
{
+ struct urb *urb = uu->urb;
int ret, i;
for (i = 0; i < urb->number_of_packets; ++i) {
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index f839654..71354ec 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -379,15 +379,14 @@ static inline void uvc_video_get_ts(struct timespec *ts)
static void
uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
- const __u8 *data, int len)
+ const __u8 *data, int len, u16 host_sof,
+ struct timespec *ts)
{
struct uvc_clock_sample *sample;
unsigned int header_size;
bool has_pts = false;
bool has_scr = false;
unsigned long flags;
- struct timespec ts;
- u16 host_sof;
u16 dev_sof;
switch (data[1] & (UVC_STREAM_PTS | UVC_STREAM_SCR)) {
@@ -435,9 +434,6 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct
uvc_buffer *buf,
stream->clock.last_sof = dev_sof;
- host_sof = usb_get_current_frame_number(stream->dev->udev);
- uvc_video_get_ts(&ts);
-
/* The UVC specification allows device implementations that can't obtain
* the USB frame number to keep their own frame counters as long as they
* match the size and frequency of the frame number associated with USB
@@ -473,7 +469,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct
uvc_buffer *buf,
sample->dev_stc = get_unaligned_le32(&data[header_size - 6]);
sample->dev_sof = dev_sof;
sample->host_sof = host_sof;
- sample->host_ts = ts;
+ sample->host_ts = *ts;
/* Update the sliding window head and count. */
stream->clock.head = (stream->clock.head + 1) % stream->clock.size;
@@ -964,7 +960,8 @@ static void uvc_video_stats_stop(struct uvc_streaming
*stream)
* uvc_video_decode_end will never be called with a NULL buffer.
*/
static int uvc_video_decode_start(struct uvc_streaming *stream,
- struct uvc_buffer *buf, const __u8 *data, int len)
+ struct uvc_buffer *buf, const __u8 *data, int len,
+ u16 host_sof, struct timespec *ts)
{
__u8 fid;
@@ -989,7 +986,7 @@ static int uvc_video_decode_start(struct uvc_streaming
*stream,
uvc_video_stats_update(stream);
}
- uvc_video_clock_decode(stream, buf, data, len);
+ uvc_video_clock_decode(stream, buf, data, len, host_sof, ts);
uvc_video_stats_decode(stream, data, len);
/* Store the payload FID bit and return immediately when the buffer is
@@ -1016,8 +1013,6 @@ static int uvc_video_decode_start(struct uvc_streaming
*stream,
* when the EOF bit is set to force synchronisation on the next packet.
*/
if (buf->state != UVC_BUF_STATE_ACTIVE) {
- struct timespec ts;
-
if (fid == stream->last_fid) {
uvc_trace(UVC_TRACE_FRAME, "Dropping payload (out of "
"sync).\n");
@@ -1027,13 +1022,11 @@ static int uvc_video_decode_start(struct uvc_streaming
*stream,
return -ENODATA;
}
-
Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M
On Mon, 7 Sep 2015, Roland Weber wrote: > Hi Alan, > > just a quick heads-up before I go to sleep, > more detailed info to follow later this week. > The freeze is triggered from hcd.c, function usb_remove_hcd: > > /* Prevent any more root-hub status calls from the timer. > * The HCD might still restart the timer (if a port status change > * interrupt occurs), but usb_hcd_poll_rh_status() won't invoke > * the hub_status_data() callback. > */ > hcd->rh_pollable = 0; > clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > del_timer_sync(&hcd->rh_timer); > > // I see output from here > printk(KERN_INFO "usb_remove_hcd five\n"); > > hcd->driver->stop(hcd); > hcd->state = HC_STATE_HALT; > > /* In case the HCD restarted the timer, stop it again. */ > clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); > del_timer_sync(&hcd->rh_timer); > > // this point is not reached > printk(KERN_INFO "usb_remove_hcd six\n"); In theory the freeze could happen during the second del_timer_sync. But I think it is much more likely to occur during hcd->driver->stop; that would be consistent with the probing-order bug you observed. > There was a lot of noise on the screen, but I saw output > from release_devnum with index 2, then with index 1. > Then follows the freeze. If you could give me a hint where > hcd->driver->stop(hcd); > might point to, I can track it further down. It points to ehci_stop() in host/ehci-hcd.c. > > Hmmm, I just took a look at the code. Something I had forgotten about > > was added recently; a table of devices that ehci-pci should ignore. If > > you add your device to that list, ehci-pci won't bind to it. See > > bypass_pci_id_table in drivers/usb/host/ehci-pci.c. Of course, that > > also will involve rebuilding part of the kernel... > > Thanks, that's good to know. At the moment, I'm still tracking the freeze > in the codebase of the first "bad" commit. Would it be better to switch > to a more recent kernel already? If so, which one would you suggest? I suggest using the 4.2 kernel, just because it is the most recent. The ehci-hcd driver hasn't changed very much recently, however, so it probably doesn't matter a lot. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 07/13] usb: otg: add OTG core
On Tue, 8 Sep 2015, Roger Quadros wrote:
> On 08/09/15 11:31, Peter Chen wrote:
> > On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
> >> On 07/09/15 04:23, Peter Chen wrote:
> >>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
> + * This is used by the USB Host stack to register the Host controller
> + * to the OTG core. Host controller must not be started by the
> + * caller as it is left upto the OTG state machine to do so.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> + unsigned long irqflags, struct otg_hcd_ops
> *ops)
> +{
> +struct usb_otg *otgd;
> +struct device *hcd_dev = hcd->self.controller;
> +struct device *otg_dev = usb_otg_get_device(hcd_dev);
> +
> >>>
> >>> One big problem here is: there are two designs for current (IP) driver
> >>> code, one creates dedicated hcd device as roothub's parent, like dwc3.
> >>> Another one doesn't do this, roothub's parent is core device (or otg
> >>> device
> >>> in your patch), like chipidea and dwc2.
> >>>
> >>> Then, otg_dev will be glue layer device for chipidea after that.
> >>
> >> OK. Let's add a way for the otg controller driver to provide the host and
> >> gadget
> >> information to the otg core for such devices like chipidea and dwc2.
> >>
> >
> > Roger, not only chipidea and dwc2, I think the musb uses the same
> > hierarchy. If the host, device, and otg share the same register
> > region, host part can't be a platform driver since we don't want
> > to remap the same register region again.
> >
> > So, in the design, we may need to consider both situations, one
> > is otg/host/device has its own register region, and host is a
> > separate platform device (A), the other is three parts share the
> > same register region, there is only one platform driver (B).
> >
> > A:
> >
> > IP core device
> > |
> > |
> > |-|-|
> > gadget host platform device
> > |
> > roothub
> >
> > B:
> >
> > IP core device
> > |
> > |
> > |-|-|
> > gadget roothub
> >
> >
> >> This API must be called before the hcd/gadget-driver is added so that the
> >> otg
> >> core knows it's linked to an OTG controller.
> >>
> >> Any better idea?
> >>
> >
> > A flag stands for this hcd controller is the same with otg controller
> > can be used, this flag can be stored at struct usb_otg_config.
>
> What if there is another architecture like so?
>
> C:
> [Parent]
> |
> |
> |--|--|
> [OTG core] [gadget][host]
>
> We need a more flexible mechanism to link the gadget and
> host device to the otg core for non DT case.
>
> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
>
> e.g.
> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
>
> If otg is NULL it will try DT otg-controller property or parent to
> get the otg controller.
This seems a lot like something Peter and I discussed recently. See
http://marc.info/?l=linux-usb&m=143977568021328&w=2
and the following messages in that thread.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] media: uvcvideo: handle urb completion in a work queue
On Tue, 8 Sep 2015, Hans de Goede wrote: > Hi, > > On 09/07/2015 06:23 PM, Mian Yousaf Kaukab wrote: > > urb completion callback is executed in host controllers interrupt > > context. To keep preempt disable time short, add urbs to a list on > > completion and schedule work to process the list. > > > > Moreover, save timestamp and sof number in the urb completion callback > > to avoid any delays. > > Erm, I thought that we had already moved to using threaded interrupt > handling for the urb completion a while (1-2 years ?) back. Is this then > still needed ? We moved to handling URB completions in a tasklet, not a threaded handler. (Similar idea, though.) And the change was made in only one or two HCDs, not in all of them. 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: similar files: fusbh200-hcd.c and fotg210-hcd.c
On Mon, Sep 07, 2015 at 04:47:45PM +0200, Peter Senna Tschudin wrote: > I executed a clone detection tool* on drivers source code and I found > that the files > > drivers/usb/host/fusbh200-hcd.c > > and > > drivers/usb/host/fotg210-hcd.c > > are very similar. The main difference between the two files are > replacing the string 'USBH20' by 'OTG21' and some white space fixes. > Some changes are being applied to only one of the files, such as the > commit f848a88d223cafa43cb318839a1171b498cf5ec8 that changes > fotg210-hcd.c but not fusbh200-hcd.c. > > Should these files be consolidated? And if so how? if you can find an easy way, that would be a very, very welcome patch. -- balbi signature.asc Description: Digital signature
Re: [PATCH v4 07/13] usb: otg: add OTG core
Alan,
On 08/09/15 17:34, Alan Stern wrote:
> On Tue, 8 Sep 2015, Roger Quadros wrote:
>
>> On 08/09/15 11:31, Peter Chen wrote:
>>> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
On 07/09/15 04:23, Peter Chen wrote:
> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
>> + * This is used by the USB Host stack to register the Host controller
>> + * to the OTG core. Host controller must not be started by the
>> + * caller as it is left upto the OTG state machine to do so.
>> + *
>> + * Returns: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>> + unsigned long irqflags, struct otg_hcd_ops
>> *ops)
>> +{
>> +struct usb_otg *otgd;
>> +struct device *hcd_dev = hcd->self.controller;
>> +struct device *otg_dev = usb_otg_get_device(hcd_dev);
>> +
>
> One big problem here is: there are two designs for current (IP) driver
> code, one creates dedicated hcd device as roothub's parent, like dwc3.
> Another one doesn't do this, roothub's parent is core device (or otg
> device
> in your patch), like chipidea and dwc2.
>
> Then, otg_dev will be glue layer device for chipidea after that.
OK. Let's add a way for the otg controller driver to provide the host and
gadget
information to the otg core for such devices like chipidea and dwc2.
>>>
>>> Roger, not only chipidea and dwc2, I think the musb uses the same
>>> hierarchy. If the host, device, and otg share the same register
>>> region, host part can't be a platform driver since we don't want
>>> to remap the same register region again.
>>>
>>> So, in the design, we may need to consider both situations, one
>>> is otg/host/device has its own register region, and host is a
>>> separate platform device (A), the other is three parts share the
>>> same register region, there is only one platform driver (B).
>>>
>>> A:
>>>
>>> IP core device
>>> |
>>> |
>>> |-|-|
>>> gadget host platform device
>>> |
>>> roothub
>>>
>>> B:
>>>
>>> IP core device
>>> |
>>> |
>>> |-|-|
>>> gadget roothub
>>>
>>>
This API must be called before the hcd/gadget-driver is added so that the
otg
core knows it's linked to an OTG controller.
Any better idea?
>>>
>>> A flag stands for this hcd controller is the same with otg controller
>>> can be used, this flag can be stored at struct usb_otg_config.
>>
>> What if there is another architecture like so?
>>
>> C:
>> [Parent]
>> |
>> |
>> |--|--|
>> [OTG core] [gadget][host]
>>
>> We need a more flexible mechanism to link the gadget and
>> host device to the otg core for non DT case.
>>
>> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
>>
>> e.g.
>> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
>>
>> If otg is NULL it will try DT otg-controller property or parent to
>> get the otg controller.
>
> This seems a lot like something Peter and I discussed recently. See
>
> http://marc.info/?l=linux-usb&m=143977568021328&w=2
>
> and the following messages in that thread.
>
If I understood right, your proposal was to add a usb_pointers data
struct to the device's drvdata?
This is fine only if the otg/gadget/host share the same device.
It does not solve the problem where each have different platform devices.
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: phy: msm: Unregister VBUS and ID events notifiers
On 09/07/2015 01:07 AM, Ivan T. Ivanov wrote:
> Right now even if driver failed to probe extcon framework will
> still deliver its VBUS and ID events, which will lead to random
> exception codes.
>
> Fix this by removing VBUS and ID events notifiers when probe fail.
>
> Fixes: 591fc116f330 ("usb: phy: msm: Use extcon framework for VBUS and ID
> detection")
>
> Reported-by: Tim Bird
> Signed-off-by: Ivan T. Ivanov
> ---
>
> Patch reworked to use new extcon API.
>
> drivers/usb/phy/phy-msm-usb.c | 25 -
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
> index c58c3c0dbe35..0fd21c147c7e 100644
> --- a/drivers/usb/phy/phy-msm-usb.c
> +++ b/drivers/usb/phy/phy-msm-usb.c
> @@ -1598,6 +1598,8 @@ static int msm_otg_read_dt(struct platform_device
> *pdev, struct msm_otg *motg)
> ret = extcon_register_notifier(ext_id, EXTCON_USB_HOST,
> &motg->id.nb);
> if (ret < 0) {
> + extcon_unregister_notifier(motg->vbus.extcon,
> +EXTCON_USB, &motg->vbus.nb);
> dev_err(&pdev->dev, "register ID notifier failed\n");
> return ret;
> }
> @@ -1660,15 +1662,6 @@ static int msm_otg_probe(struct platform_device *pdev)
> if (!motg)
> return -ENOMEM;
>
> - pdata = dev_get_platdata(&pdev->dev);
> - if (!pdata) {
> - if (!np)
> - return -ENXIO;
> - ret = msm_otg_read_dt(pdev, motg);
> - if (ret)
> - return ret;
> - }
> -
> motg->phy.otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg),
>GFP_KERNEL);
> if (!motg->phy.otg)
> @@ -1731,6 +1724,15 @@ static int msm_otg_probe(struct platform_device *pdev)
> return motg->irq;
> }
>
> + pdata = dev_get_platdata(&pdev->dev);
> + if (!pdata) {
> + if (!np)
> + return -ENXIO;
> + ret = msm_otg_read_dt(pdev, motg);
> + if (ret)
> + return ret;
> + }
> +
The following code depends on msm_otg_read_dt(), but the
movement of the call puts the phy_number test before msm_otg_read_dt().
/*
* NOTE: The PHYs can be multiplexed between the chipidea controller
* and the dwc3 controller, using a single bit. It is important that
* the dwc3 driver does not set this bit in an incompatible way.
*/
if (motg->phy_number) {
phy_select = devm_ioremap_nocache(&pdev->dev, USB2_PHY_SEL, 4);
if (!phy_select)
return -ENOMEM;
/* Enable second PHY with the OTG port */
writel(0x1, phy_select);
}
Can you please move the motg->phy_number code after the msm_otg_read_dt() call?
(right here, in the patch, would be good :-)
> regs[0].supply = "vddcx";
> regs[1].supply = "v3p3";
> regs[2].supply = "v1p8";
> @@ -1834,6 +1836,11 @@ disable_clks:
> clk_disable_unprepare(motg->clk);
> if (!IS_ERR(motg->core_clk))
> clk_disable_unprepare(motg->core_clk);
> +
> + extcon_unregister_notifier(motg->id.extcon,
> +EXTCON_USB_HOST, &motg->id.nb);
> + extcon_unregister_notifier(motg->vbus.extcon,
> +EXTCON_USB, &motg->vbus.nb);
> return ret;
> }
Other than that - it looks good.
-- Tim
--
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] usbnet: Fix a race between usbnet_stop() and the BH
From: Eugene Shatokhin
Date: Tue, 1 Sep 2015 17:05:33 +0300
> The race may happen when a device (e.g. YOTA 4G LTE Modem) is
> unplugged while the system is downloading a large file from the Net.
>
> Hardware breakpoints and Kprobes with delays were used to confirm that
> the race does actually happen.
>
> The race is on skb_queue ('next' pointer) between usbnet_stop()
> and rx_complete(), which, in turn, calls usbnet_bh().
>
> Here is a part of the call stack with the code where the changes to the
> queue happen. The line numbers are for the kernel 4.1.0:
...
> As a result, it is possible, for example, that the skb is removed from
> dev->rxq by __skb_unlink() before the check
> "!skb_queue_empty(&dev->rxq)" in usbnet_terminate_urbs() is made. It is
> also possible in this case that the skb is added to dev->done queue
> after "!skb_queue_empty(&dev->done)" is checked. So
> usbnet_terminate_urbs() may stop waiting and return while dev->done
> queue still has an item.
>
> Locking in defer_bh() and usbnet_terminate_urbs() was revisited to avoid
> this race.
>
> Signed-off-by: Eugene Shatokhin
Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: musb: set the controller speed based on the config setting
Set the Power register HSENAB bit based on musb->config->maximum_speed,
so that the glue layer can control MUSB to work in high- or full-speed.
Signed-off-by: Bin Liu
---
drivers/usb/musb/musb_core.c | 10 +-
include/linux/usb/musb.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 4e50650..80ce59d 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -936,6 +936,7 @@ void musb_start(struct musb *musb)
{
void __iomem*regs = musb->mregs;
u8 devctl = musb_readb(regs, MUSB_DEVCTL);
+ u8 power;
dev_dbg(musb->controller, "<== devctl %02x\n", devctl);
@@ -949,11 +950,10 @@ void musb_start(struct musb *musb)
musb_writeb(regs, MUSB_TESTMODE, 0);
/* put into basic highspeed mode and start session */
- musb_writeb(regs, MUSB_POWER, MUSB_POWER_ISOUPDATE
- | MUSB_POWER_HSENAB
- /* ENSUSPEND wedges tusb */
- /* | MUSB_POWER_ENSUSPEND */
- );
+ power = MUSB_POWER_ISOUPDATE;
+ if (musb->config->maximum_speed == USB_SPEED_HIGH)
+ power |= MUSB_POWER_HSENAB;
+ musb_writeb(regs, MUSB_POWER, power);
musb->is_active = 0;
devctl = musb_readb(regs, MUSB_DEVCTL);
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index a4ee1b5..fa6dc13 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -95,7 +95,7 @@ struct musb_hdrc_config {
/* musb CLKIN in Blackfin in MHZ */
unsigned char clkin;
#endif
-
+ u32 maximum_speed;
};
struct musb_hdrc_platform_data {
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: musb: dsps: control musb speed based on dts setting
Set musb config->maximum_speed based on the dts setting to control musb
speed.
By default musb works in high-speed mode. Adding
maximum-speed = "full-speed";
to dts usb node will force musb to full-speed mode.
Signed-off-by: Bin Liu
---
drivers/usb/musb/musb_dsps.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 0ca6011..268e2b5 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -736,6 +736,11 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
pdata.power = get_int_prop(dn, "mentor,power") / 2;
config->multipoint = of_property_read_bool(dn, "mentor,multipoint");
+ config->maximum_speed = of_usb_get_maximum_speed(musb->dev.of_node);
+ if (config->maximum_speed == USB_SPEED_UNKNOWN ||
+ config->maximum_speed > USB_SPEED_HIGH)
+ config->maximum_speed = USB_SPEED_HIGH;
+
ret = platform_device_add_data(musb, &pdata, sizeof(pdata));
if (ret) {
dev_err(dev, "failed to add platform_data\n");
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: musb: set the controller speed based on the config setting
Hi,
On 09/08/2015 04:10 PM, Bin Liu wrote:
Set the Power register HSENAB bit based on musb->config->maximum_speed,
so that the glue layer can control MUSB to work in high- or full-speed.
Signed-off-by: Bin Liu
Sorry, please discard this patch. I forgot to rebase it to 4.1.x.
Thanks,
-Bin.
---
drivers/usb/musb/musb_core.c | 10 +-
include/linux/usb/musb.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 4e50650..80ce59d 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -936,6 +936,7 @@ void musb_start(struct musb *musb)
{
void __iomem*regs = musb->mregs;
u8 devctl = musb_readb(regs, MUSB_DEVCTL);
+ u8 power;
dev_dbg(musb->controller, "<== devctl %02x\n", devctl);
@@ -949,11 +950,10 @@ void musb_start(struct musb *musb)
musb_writeb(regs, MUSB_TESTMODE, 0);
/* put into basic highspeed mode and start session */
- musb_writeb(regs, MUSB_POWER, MUSB_POWER_ISOUPDATE
- | MUSB_POWER_HSENAB
- /* ENSUSPEND wedges tusb */
- /* | MUSB_POWER_ENSUSPEND */
- );
+ power = MUSB_POWER_ISOUPDATE;
+ if (musb->config->maximum_speed == USB_SPEED_HIGH)
+ power |= MUSB_POWER_HSENAB;
+ musb_writeb(regs, MUSB_POWER, power);
musb->is_active = 0;
devctl = musb_readb(regs, MUSB_DEVCTL);
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index a4ee1b5..fa6dc13 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -95,7 +95,7 @@ struct musb_hdrc_config {
/* musb CLKIN in Blackfin in MHZ */
unsigned char clkin;
#endif
-
+ u32 maximum_speed;
};
struct musb_hdrc_platform_data {
--
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: musb: dsps: control musb speed based on dts setting
Hi,
On 09/08/2015 04:10 PM, Bin Liu wrote:
Set musb config->maximum_speed based on the dts setting to control musb
speed.
By default musb works in high-speed mode. Adding
maximum-speed = "full-speed";
to dts usb node will force musb to full-speed mode.
Signed-off-by: Bin Liu
Sorry, please discard this patch. I forgot to rebase it to 4.1.x.
Thanks,
-Bin.
---
drivers/usb/musb/musb_dsps.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 0ca6011..268e2b5 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -736,6 +736,11 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
pdata.power = get_int_prop(dn, "mentor,power") / 2;
config->multipoint = of_property_read_bool(dn, "mentor,multipoint");
+ config->maximum_speed = of_usb_get_maximum_speed(musb->dev.of_node);
+ if (config->maximum_speed == USB_SPEED_UNKNOWN ||
+ config->maximum_speed > USB_SPEED_HIGH)
+ config->maximum_speed = USB_SPEED_HIGH;
+
ret = platform_device_add_data(musb, &pdata, sizeof(pdata));
if (ret) {
dev_err(dev, "failed to add platform_data\n");
--
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: musb: dsps: control musb speed based on dts setting
On Tue, Sep 08, 2015 at 04:10:12PM -0500, Bin Liu wrote: > Set musb config->maximum_speed based on the dts setting to control musb > speed. > > By default musb works in high-speed mode. Adding > > maximum-speed = "full-speed"; > > to dts usb node will force musb to full-speed mode. > > Signed-off-by: Bin Liu > --- > drivers/usb/musb/musb_dsps.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c > index 0ca6011..268e2b5 100644 > --- a/drivers/usb/musb/musb_dsps.c > +++ b/drivers/usb/musb/musb_dsps.c > @@ -736,6 +736,11 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, > pdata.power = get_int_prop(dn, "mentor,power") / 2; > config->multipoint = of_property_read_bool(dn, "mentor,multipoint"); > > + config->maximum_speed = of_usb_get_maximum_speed(musb->dev.of_node); > + if (config->maximum_speed == USB_SPEED_UNKNOWN || > + config->maximum_speed > USB_SPEED_HIGH) I'd add a log message for > HIGH. That would mean somebody thinks musb can work in super speed. -- balbi signature.asc Description: Digital signature
Re: qcserial: AT unsolicited response codes missing with Dell Wireless 5808e
On Mon, Aug 31, 2015 at 07:18:05PM +, Ward, David - 0665 - MITLL wrote:
> On 01/06/2015 03:58 AM, Bjørn Mork wrote:
> > Johan Hovold writes:
> >
> >> Ok, let's move the PID to option and if it turns out that more of these
> >> devices require the modem-control signals (e.g. with more recent
> >> firmware) we can consider moving it back after adding such support to
> >> qcserial.
> >>
> >> Bjørn, do you see any problems with this? Are there more interface
> >> layouts for these devices out there perhaps (making options blacklisting
> >> a bad fit)?
> > No, I don't see any problem. It seems like the reasonable thing to do.
> > I would have added these devices to the option driver in the first place
> > had I known about the setup requirements. Sorry for not catching that
> > earlier. And thanks to Reinhard for tracking this down.
> >
> > There are plenty of different interface layouts for these devices, but
> > Sierra Wireless use consistent interface numbering for different
> > functions, so the blacklist_info should work fine regardless.
> >
> > Note however that these devices also have multiple PID identities (and
> > possibly also different VIDs?). The other identities may or may not have
> > similar issues with the qcserial driver. I don't know. But some of these
> > identities are shared with other devices, which complicates matters a
> > bit.
> >
>
> This also affects the Dell Wireless 5808e, a branded version of the
> Sierra Wireless EM7355. Again, moving it from the qcserial driver to the
> option driver, which calls send_setup, allows URCs to appear.
>
> Is it possible to test if the MC7710 also works with the option driver
> (does send_setup cause problems with it)? If that still works, are there
> any reasons not to move all of the devices using the Sierra Wireless
> layout (DEVICE_SWI) from the qcserial driver to the option driver?
>
> David
Hi David,
due to lack of time I could not test your patch
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index 876423b..035335c 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -1098,4 +1098,6 @@ static const struct usb_device_id option_ids[] = {
> { USB_DEVICE(QUALCOMM_VENDOR_ID, 0x0023)}, /* ONYX 3G device */
> { USB_DEVICE(QUALCOMM_VENDOR_ID, 0x9000)}, /* SIMCom SIM5218 */
> + { USB_DEVICE_INTERFACE_CLASS(SIERRA_VENDOR_ID, 0x68a2, 0xff),
> + .driver_info = (kernel_ulong_t)&sierra_mc73xx_blacklist }, /* MC7710
> */
> { USB_DEVICE_INTERFACE_CLASS(SIERRA_VENDOR_ID, 0x68c0, 0xff),
> .driver_info = (kernel_ulong_t)&sierra_mc73xx_blacklist }, /* MC73xx
> */
> diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c
> index ebcec8c..7b0de41 100644
> --- a/drivers/usb/serial/qcserial.c
> +++ b/drivers/usb/serial/qcserial.c
> @@ -143,5 +143,4 @@ static const struct usb_device_id id_table[] = {
> {DEVICE_SWI(0x0f3d, 0x68a2)}, /* Sierra Wireless MC7700 */
> {DEVICE_SWI(0x114f, 0x68a2)}, /* Sierra Wireless MC7750 */
> - {DEVICE_SWI(0x1199, 0x68a2)}, /* Sierra Wireless MC7710 */
> {DEVICE_SWI(0x1199, 0x901c)}, /* Sierra Wireless EM7700 */
> {DEVICE_SWI(0x1199, 0x901f)}, /* Sierra Wireless EM7355 */
> --
> 1.7.1
you sent me yesterday. I checked however using
# rmmod qcserial
# modprobe option
# echo 1199 68a2 > /sys/bus/usb-serial/drivers/option1/new_id
that AT URCs with a MC7710 still work when option_send_setup() is used.
Since AT URCs already work with the qcserial driver for the MC7710
"out of the box"
at+gmr
SWI9200X_03.05.29.03ap r6485 CNSHZ-ED-XP0031 2014/12/02 17:53:15
OK
at+cscs="GSM"
OK
at+cusd=1,"*101#",15
OK
+CUSD: 0,"Ihr Guthaben betr{gt: 42,00. Jetzt auch Ihr Guthaben aufladen:
einfach *103*Aufladenummer# und die H|rertaste eingeben.",15
my suggestion would be to keep the MC77xx in the qcserial driver to
avoid having to add additional blacklist entries. The current
version of your patch incorrectly uses the MC73xx blacklist also for
the MC77xx which would cause the option driver to bind to the MC77xx
QMI interfaces on USB IF 19 and 20 which it should not do.
Instead of moving the MC77xx to the option driver it might be more
worthwhile to check whether other EM7305/EM7355-based devices like the
recently added Dell Wireless 5809e device are also affected by the
missing AT URCs and should be moved from the qcserial to the option driver.
Regards,
Reinhard
--
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: musb: dsps: control musb speed based on dts setting
Hi, On 09/08/2015 04:16 PM, Felipe Balbi wrote: On Tue, Sep 08, 2015 at 04:10:12PM -0500, Bin Liu wrote: Set musb config->maximum_speed based on the dts setting to control musb speed. By default musb works in high-speed mode. Adding maximum-speed = "full-speed"; to dts usb node will force musb to full-speed mode. Signed-off-by: Bin Liu --- drivers/usb/musb/musb_dsps.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 0ca6011..268e2b5 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -736,6 +736,11 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, pdata.power = get_int_prop(dn, "mentor,power") / 2; config->multipoint = of_property_read_bool(dn, "mentor,multipoint"); + config->maximum_speed = of_usb_get_maximum_speed(musb->dev.of_node); + if (config->maximum_speed == USB_SPEED_UNKNOWN || + config->maximum_speed > USB_SPEED_HIGH) I'd add a log message for > HIGH. That would mean somebody thinks musb can work in super speed. Do you mean a comment, not runtime log? -- 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: musb: dsps: control musb speed based on dts setting
On Tue, Sep 08, 2015 at 04:18:14PM -0500, Bin Liu wrote: > Hi, > > On 09/08/2015 04:16 PM, Felipe Balbi wrote: > >On Tue, Sep 08, 2015 at 04:10:12PM -0500, Bin Liu wrote: > >>Set musb config->maximum_speed based on the dts setting to control musb > >>speed. > >> > >>By default musb works in high-speed mode. Adding > >> > >>maximum-speed = "full-speed"; > >> > >>to dts usb node will force musb to full-speed mode. > >> > >>Signed-off-by: Bin Liu > >>--- > >> drivers/usb/musb/musb_dsps.c | 5 + > >> 1 file changed, 5 insertions(+) > >> > >>diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c > >>index 0ca6011..268e2b5 100644 > >>--- a/drivers/usb/musb/musb_dsps.c > >>+++ b/drivers/usb/musb/musb_dsps.c > >>@@ -736,6 +736,11 @@ static int dsps_create_musb_pdev(struct dsps_glue > >>*glue, > >>pdata.power = get_int_prop(dn, "mentor,power") / 2; > >>config->multipoint = of_property_read_bool(dn, "mentor,multipoint"); > >> > >>+ config->maximum_speed = of_usb_get_maximum_speed(musb->dev.of_node); > >>+ if (config->maximum_speed == USB_SPEED_UNKNOWN || > >>+ config->maximum_speed > USB_SPEED_HIGH) > > > >I'd add a log message for > HIGH. That would mean somebody thinks musb > >can work in super speed. > > Do you mean a comment, not runtime log? runtime :-) dev_info() or something -- balbi signature.asc Description: Digital signature
[PATCH 1/2] usb: chipidea: Add support for 'phy-clkgate-delay-us' property
From: Fabio Estevam
Add support for the optional 'phy-clkgate-delay-us' property that is
used to describe the delay time between putting PHY into low power mode
and turning off the PHY clock.
Signed-off-by: Li Jun
Signed-off-by: Fabio Estevam
---
drivers/usb/chipidea/core.c | 7 +++
include/linux/usb/chipidea.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 3feebf7..9469675 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -651,6 +651,10 @@ static int ci_get_platdata(struct device *dev,
if (of_usb_get_maximum_speed(dev->of_node) == USB_SPEED_FULL)
platdata->flags |= CI_HDRC_FORCE_FULLSPEED;
+ if (of_find_property(dev->of_node, "phy-clkgate-delay-us", NULL))
+ of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
+&platdata->phy_clkgate_delay_us);
+
platdata->itc_setting = 1;
if (of_find_property(dev->of_node, "itc-setting", NULL)) {
ret = of_property_read_u32(dev->of_node, "itc-setting",
@@ -996,6 +1000,9 @@ static void ci_controller_suspend(struct ci_hdrc *ci)
{
disable_irq(ci->irq);
ci_hdrc_enter_lpm(ci, true);
+ if (ci->platdata->phy_clkgate_delay_us)
+ usleep_range(ci->platdata->phy_clkgate_delay_us,
+ci->platdata->phy_clkgate_delay_us + 50);
usb_phy_set_suspend(ci->usb_phy, 1);
ci->in_lpm = true;
enable_irq(ci->irq);
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index a41833c..32e8c9b 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -48,6 +48,7 @@ struct ci_hdrc_platform_data {
u32 ahb_burst_config;
u32 tx_burst_size;
u32 rx_burst_size;
+ u32 phy_clkgate_delay_us;
};
/* Default offset of capability registers */
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] Doc: usb: ci-hdrc-usb2: Add phy-clkgate-delay-us entry
From: Fabio Estevam Add an entry for the optional 'phy-clkgate-delay-us' property that is used to describe the delay time between putting PHY into low power mode and turning off the PHY clock. Signed-off-by: Li Jun Signed-off-by: Fabio Estevam --- Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt index d71ef07..48709dd 100644 --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt @@ -45,6 +45,8 @@ Optional properties: (4 bytes), This register represents the maximum length of a the burst in 32-bit words while moving data from the USB bus to system memory, changing this value takes effect only the SBUSCFG.AHBBRST is 0. +- phy-clkgate-delay-us: the delay time (us) between putting the PHY into + low power mode and gating the PHY clock. Example: @@ -61,4 +63,5 @@ Example: ahb-burst-config = <0x0>; tx-burst-size-dword = <0x10>; /* 64 bytes */ rx-burst-size-dword = <0x10>; + phy-clkgate-delay-us = <400>; }; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: chipidea: imx: fix a typo for imx6sx
Use imx6sx instead of imx6sl's platform flags for imx6sx.
Signed-off-by: Li Jun
---
drivers/usb/chipidea/ci_hdrc_imx.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
b/drivers/usb/chipidea/ci_hdrc_imx.c
index f9deea5..1becd33 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -67,7 +67,7 @@ static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
{ .compatible = "fsl,imx27-usb", .data = &imx27_usb_data},
{ .compatible = "fsl,imx6q-usb", .data = &imx6q_usb_data},
{ .compatible = "fsl,imx6sl-usb", .data = &imx6sl_usb_data},
- { .compatible = "fsl,imx6sx-usb", .data = &imx6sl_usb_data},
+ { .compatible = "fsl,imx6sx-usb", .data = &imx6sx_usb_data},
{ .compatible = "fsl,imx7d-usb", .data = &imx7d_usb_data},
{ /* sentinel */ }
};
--
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 2/2] usb: musb: dsps: control musb speed based on dts setting
Hi, > -Original Message- > From: Balbi, Felipe > Sent: Tuesday, September 08, 2015 4:25 PM > To: Liu, Bin > Cc: Balbi, Felipe; linux-usb@vger.kernel.org > Subject: Re: [PATCH 2/2] usb: musb: dsps: control musb speed based on dts > setting > > On Tue, Sep 08, 2015 at 04:18:14PM -0500, Bin Liu wrote: > > Hi, > > > > On 09/08/2015 04:16 PM, Felipe Balbi wrote: > > >On Tue, Sep 08, 2015 at 04:10:12PM -0500, Bin Liu wrote: > > >>Set musb config->maximum_speed based on the dts setting to control > > >>musb speed. > > >> > > >>By default musb works in high-speed mode. Adding > > >> > > >> maximum-speed = "full-speed"; > > >> > > >>to dts usb node will force musb to full-speed mode. > > >> > > >>Signed-off-by: Bin Liu > > >>--- > > >> drivers/usb/musb/musb_dsps.c | 5 + > > >> 1 file changed, 5 insertions(+) > > >> > > >>diff --git a/drivers/usb/musb/musb_dsps.c > > >>b/drivers/usb/musb/musb_dsps.c index 0ca6011..268e2b5 100644 > > >>--- a/drivers/usb/musb/musb_dsps.c > > >>+++ b/drivers/usb/musb/musb_dsps.c > > >>@@ -736,6 +736,11 @@ static int dsps_create_musb_pdev(struct dsps_glue > *glue, > > >> pdata.power = get_int_prop(dn, "mentor,power") / 2; > > >> config->multipoint = of_property_read_bool(dn, > > >>"mentor,multipoint"); > > >> > > >>+ config->maximum_speed = of_usb_get_maximum_speed(musb- > >dev.of_node); > > >>+ if (config->maximum_speed == USB_SPEED_UNKNOWN || > > >>+ config->maximum_speed > USB_SPEED_HIGH) > > > > > >I'd add a log message for > HIGH. That would mean somebody thinks > > >musb can work in super speed. > > > > Do you mean a comment, not runtime log? > > runtime :-) dev_info() or something Ok, will do. Regards, -Bin. > > -- > 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
[PATCH 1/2] usb: chipidea: imx: add usb support for imx7d
Add imx7d usb support.
Signed-off-by: Li Jun
---
drivers/usb/chipidea/ci_hdrc_imx.c |7
drivers/usb/chipidea/usbmisc_imx.c | 62
2 files changed, 69 insertions(+)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 867e9f3..f9deea5 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -56,12 +56,19 @@ static const struct ci_hdrc_imx_platform_flag
imx6sx_usb_data = {
CI_HDRC_DISABLE_HOST_STREAMING,
};
+static const struct ci_hdrc_imx_platform_flag imx7d_usb_data = {
+ .flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
+ CI_HDRC_TURN_VBUS_EARLY_ON |
+ CI_HDRC_DISABLE_HOST_STREAMING,
+};
+
static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
{ .compatible = "fsl,imx28-usb", .data = &imx28_usb_data},
{ .compatible = "fsl,imx27-usb", .data = &imx27_usb_data},
{ .compatible = "fsl,imx6q-usb", .data = &imx6q_usb_data},
{ .compatible = "fsl,imx6sl-usb", .data = &imx6sl_usb_data},
{ .compatible = "fsl,imx6sx-usb", .data = &imx6sl_usb_data},
+ { .compatible = "fsl,imx7d-usb", .data = &imx7d_usb_data},
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, ci_hdrc_imx_dt_ids);
diff --git a/drivers/usb/chipidea/usbmisc_imx.c
b/drivers/usb/chipidea/usbmisc_imx.c
index 5ddab30..f3a4753 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -72,6 +72,14 @@
#define VF610_OVER_CUR_DIS BIT(7)
+#define MX7D_USBNC_USB_CTRL2 0x4
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_MASK 0x3
+#define MX7D_USB_VBUS_WAKEUP_SOURCE(v) (v << 0)
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_VBUS MX7D_USB_VBUS_WAKEUP_SOURCE(0)
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_AVALID MX7D_USB_VBUS_WAKEUP_SOURCE(1)
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID MX7D_USB_VBUS_WAKEUP_SOURCE(2)
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_SESS_END MX7D_USB_VBUS_WAKEUP_SOURCE(3)
+
struct usbmisc_ops {
/* It's called once when probe a usb device */
int (*init)(struct imx_usbmisc_data *data);
@@ -324,6 +332,55 @@ static int usbmisc_vf610_init(struct imx_usbmisc_data
*data)
return 0;
}
+static int usbmisc_imx7d_set_wakeup
+ (struct imx_usbmisc_data *data, bool enabled)
+{
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+ unsigned long flags;
+ u32 val;
+ u32 wakeup_setting = (MX6_BM_WAKEUP_ENABLE |
+ MX6_BM_VBUS_WAKEUP | MX6_BM_ID_WAKEUP);
+
+ spin_lock_irqsave(&usbmisc->lock, flags);
+ val = readl(usbmisc->base);
+ if (enabled) {
+ writel(val | wakeup_setting, usbmisc->base);
+ } else {
+ if (val & MX6_BM_WAKEUP_INTR)
+ dev_dbg(data->dev, "wakeup int\n");
+ writel(val & ~wakeup_setting, usbmisc->base);
+ }
+ spin_unlock_irqrestore(&usbmisc->lock, flags);
+
+ return 0;
+}
+
+static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
+{
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+ unsigned long flags;
+ u32 reg;
+
+ if (data->index >= 1)
+ return -EINVAL;
+
+ spin_lock_irqsave(&usbmisc->lock, flags);
+ if (data->disable_oc) {
+ reg = readl(usbmisc->base);
+ writel(reg | MX6_BM_OVER_CUR_DIS, usbmisc->base);
+ }
+
+ reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
+ reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK;
+ writel(reg | MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID,
+usbmisc->base + MX7D_USBNC_USB_CTRL2);
+ spin_unlock_irqrestore(&usbmisc->lock, flags);
+
+ usbmisc_imx7d_set_wakeup(data, false);
+
+ return 0;
+}
+
static const struct usbmisc_ops imx25_usbmisc_ops = {
.init = usbmisc_imx25_init,
.post = usbmisc_imx25_post,
@@ -351,6 +408,11 @@ static const struct usbmisc_ops imx6sx_usbmisc_ops = {
.init = usbmisc_imx6sx_init,
};
+static const struct usbmisc_ops imx7d_usbmisc_ops = {
+ .init = usbmisc_imx7d_init,
+ .set_wakeup = usbmisc_imx7d_set_wakeup,
+};
+
int imx_usbmisc_init(struct imx_usbmisc_data *data)
{
struct imx_usbmisc *usbmisc;
--
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
[PATCH v2 1/2] usb: chipidea: imx: add usb support for imx7d
From: Peter Chen
Add imx7d usb support.
Signed-off-by: Peter Chen
Signed-off-by: Li Jun
---
change for v2:
- update author and signed-off
drivers/usb/chipidea/ci_hdrc_imx.c |7
drivers/usb/chipidea/usbmisc_imx.c | 62
2 files changed, 69 insertions(+)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 867e9f3..f9deea5 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -56,12 +56,19 @@ static const struct ci_hdrc_imx_platform_flag
imx6sx_usb_data = {
CI_HDRC_DISABLE_HOST_STREAMING,
};
+static const struct ci_hdrc_imx_platform_flag imx7d_usb_data = {
+ .flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
+ CI_HDRC_TURN_VBUS_EARLY_ON |
+ CI_HDRC_DISABLE_HOST_STREAMING,
+};
+
static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
{ .compatible = "fsl,imx28-usb", .data = &imx28_usb_data},
{ .compatible = "fsl,imx27-usb", .data = &imx27_usb_data},
{ .compatible = "fsl,imx6q-usb", .data = &imx6q_usb_data},
{ .compatible = "fsl,imx6sl-usb", .data = &imx6sl_usb_data},
{ .compatible = "fsl,imx6sx-usb", .data = &imx6sl_usb_data},
+ { .compatible = "fsl,imx7d-usb", .data = &imx7d_usb_data},
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, ci_hdrc_imx_dt_ids);
diff --git a/drivers/usb/chipidea/usbmisc_imx.c
b/drivers/usb/chipidea/usbmisc_imx.c
index 5ddab30..f3a4753 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -72,6 +72,14 @@
#define VF610_OVER_CUR_DIS BIT(7)
+#define MX7D_USBNC_USB_CTRL2 0x4
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_MASK 0x3
+#define MX7D_USB_VBUS_WAKEUP_SOURCE(v) (v << 0)
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_VBUS MX7D_USB_VBUS_WAKEUP_SOURCE(0)
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_AVALID MX7D_USB_VBUS_WAKEUP_SOURCE(1)
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID MX7D_USB_VBUS_WAKEUP_SOURCE(2)
+#define MX7D_USB_VBUS_WAKEUP_SOURCE_SESS_END MX7D_USB_VBUS_WAKEUP_SOURCE(3)
+
struct usbmisc_ops {
/* It's called once when probe a usb device */
int (*init)(struct imx_usbmisc_data *data);
@@ -324,6 +332,55 @@ static int usbmisc_vf610_init(struct imx_usbmisc_data
*data)
return 0;
}
+static int usbmisc_imx7d_set_wakeup
+ (struct imx_usbmisc_data *data, bool enabled)
+{
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+ unsigned long flags;
+ u32 val;
+ u32 wakeup_setting = (MX6_BM_WAKEUP_ENABLE |
+ MX6_BM_VBUS_WAKEUP | MX6_BM_ID_WAKEUP);
+
+ spin_lock_irqsave(&usbmisc->lock, flags);
+ val = readl(usbmisc->base);
+ if (enabled) {
+ writel(val | wakeup_setting, usbmisc->base);
+ } else {
+ if (val & MX6_BM_WAKEUP_INTR)
+ dev_dbg(data->dev, "wakeup int\n");
+ writel(val & ~wakeup_setting, usbmisc->base);
+ }
+ spin_unlock_irqrestore(&usbmisc->lock, flags);
+
+ return 0;
+}
+
+static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
+{
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+ unsigned long flags;
+ u32 reg;
+
+ if (data->index >= 1)
+ return -EINVAL;
+
+ spin_lock_irqsave(&usbmisc->lock, flags);
+ if (data->disable_oc) {
+ reg = readl(usbmisc->base);
+ writel(reg | MX6_BM_OVER_CUR_DIS, usbmisc->base);
+ }
+
+ reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
+ reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK;
+ writel(reg | MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID,
+usbmisc->base + MX7D_USBNC_USB_CTRL2);
+ spin_unlock_irqrestore(&usbmisc->lock, flags);
+
+ usbmisc_imx7d_set_wakeup(data, false);
+
+ return 0;
+}
+
static const struct usbmisc_ops imx25_usbmisc_ops = {
.init = usbmisc_imx25_init,
.post = usbmisc_imx25_post,
@@ -351,6 +408,11 @@ static const struct usbmisc_ops imx6sx_usbmisc_ops = {
.init = usbmisc_imx6sx_init,
};
+static const struct usbmisc_ops imx7d_usbmisc_ops = {
+ .init = usbmisc_imx7d_init,
+ .set_wakeup = usbmisc_imx7d_set_wakeup,
+};
+
int imx_usbmisc_init(struct imx_usbmisc_data *data)
{
struct imx_usbmisc *usbmisc;
--
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
[PATCH v2 2/2] usb: chipidea: imx: fix a typo for imx6sx
Use imx6sx instead of imx6sl's platform flags for imx6sx.
Signed-off-by: Li Jun
---
drivers/usb/chipidea/ci_hdrc_imx.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
b/drivers/usb/chipidea/ci_hdrc_imx.c
index f9deea5..1becd33 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -67,7 +67,7 @@ static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
{ .compatible = "fsl,imx27-usb", .data = &imx27_usb_data},
{ .compatible = "fsl,imx6q-usb", .data = &imx6q_usb_data},
{ .compatible = "fsl,imx6sl-usb", .data = &imx6sl_usb_data},
- { .compatible = "fsl,imx6sx-usb", .data = &imx6sl_usb_data},
+ { .compatible = "fsl,imx6sx-usb", .data = &imx6sx_usb_data},
{ .compatible = "fsl,imx7d-usb", .data = &imx7d_usb_data},
{ /* sentinel */ }
};
--
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 v4 07/13] usb: otg: add OTG core
On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:
>
>
> On 08/09/15 11:31, Peter Chen wrote:
> > On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
> >> On 07/09/15 04:23, Peter Chen wrote:
> >>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
> + * This is used by the USB Host stack to register the Host controller
> + * to the OTG core. Host controller must not be started by the
> + * caller as it is left upto the OTG state machine to do so.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> + unsigned long irqflags, struct otg_hcd_ops
> *ops)
> +{
> +struct usb_otg *otgd;
> +struct device *hcd_dev = hcd->self.controller;
> +struct device *otg_dev = usb_otg_get_device(hcd_dev);
> +
> >>>
> >>> One big problem here is: there are two designs for current (IP) driver
> >>> code, one creates dedicated hcd device as roothub's parent, like dwc3.
> >>> Another one doesn't do this, roothub's parent is core device (or otg
> >>> device
> >>> in your patch), like chipidea and dwc2.
> >>>
> >>> Then, otg_dev will be glue layer device for chipidea after that.
> >>
> >> OK. Let's add a way for the otg controller driver to provide the host and
> >> gadget
> >> information to the otg core for such devices like chipidea and dwc2.
> >>
> >
> > Roger, not only chipidea and dwc2, I think the musb uses the same
> > hierarchy. If the host, device, and otg share the same register
> > region, host part can't be a platform driver since we don't want
> > to remap the same register region again.
> >
> > So, in the design, we may need to consider both situations, one
> > is otg/host/device has its own register region, and host is a
> > separate platform device (A), the other is three parts share the
> > same register region, there is only one platform driver (B).
> >
> > A:
> >
> > IP core device
> > |
> > |
> > |-|-|
> > gadget host platform device
> > |
> > roothub
> >
> > B:
> >
> > IP core device
> > |
> > |
> > |-|-|
> > gadget roothub
> >
> >
> >> This API must be called before the hcd/gadget-driver is added so that the
> >> otg
> >> core knows it's linked to an OTG controller.
> >>
> >> Any better idea?
> >>
> >
> > A flag stands for this hcd controller is the same with otg controller
> > can be used, this flag can be stored at struct usb_otg_config.
>
> What if there is another architecture like so?
>
> C:
> [Parent]
> |
> |
> |--|--|
> [OTG core] [gadget][host]
>
> We need a more flexible mechanism to link the gadget and
> host device to the otg core for non DT case.
>
> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
>
> e.g.
> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
>
> If otg is NULL it will try DT otg-controller property or parent to
> get the otg controller.
How usb_otg_register_hcd get struct usb_otg, from where?
>
> >
> > Peter
> >
> > P.S: I still read your code, I find not all APIs in this file are used
> > in your dwc3 example.
>
> Which ones? The ones for registering/unregistered host/gadget are used
> by hcd/udc core as part of usb_add/remove_hcd() and
> udc_bind_to_driver()/usb_gadget_remove_driver()
>
Ok, now I understand your design, usb_create_hcd must be called before
the fsm kicks off. The call flow like below:
usb_otg_register->usb_create_hcd->usb_add_hcd->usb_otg_register_hcd->
usb_otg_start_host->usb_otg_add_hcd
We need to make some changes to let chipidea work since usb_create_hcd
is included at host->start.
--
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 v4 10/13] usb: hcd: Adapt to OTG core
On Mon, Aug 24, 2015 at 04:21:21PM +0300, Roger Quadros wrote:
> The existing usb_add/remove_hcd() functionality
> remains unchanged for non-OTG devices. For OTG
> devices they only register the HCD with the OTG core.
>
> Introduce usb_otg_add/remove_hcd() for use by OTG core.
> These functions actually add/remove the HCD.
>
> Signed-off-by: Roger Quadros
> ---
> drivers/usb/core/hcd.c | 55
> +-
> 1 file changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index c0fd1f6..4851682 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -46,6 +46,7 @@
> #include
> #include
> #include
> +#include
>
> #include "usb.h"
>
> @@ -2625,8 +2626,8 @@ static void usb_put_invalidate_rhdev(struct usb_hcd
> *hcd)
> * buffers of consistent memory, register the bus, request the IRQ line,
> * and call the driver's reset() and start() routines.
> */
> -int usb_add_hcd(struct usb_hcd *hcd,
> - unsigned int irqnum, unsigned long irqflags)
> +static int usb_otg_add_hcd(struct usb_hcd *hcd,
> +unsigned int irqnum, unsigned long irqflags)
You may change the kernel doc to this name too.
> {
> int retval;
> struct usb_device *rhdev;
> @@ -2839,17 +2840,16 @@ err_phy:
> }
> return retval;
> }
> -EXPORT_SYMBOL_GPL(usb_add_hcd);
>
> /**
> - * usb_remove_hcd - shutdown processing for generic HCDs
> + * usb_otg_remove_hcd - shutdown processing for generic HCDs
> * @hcd: the usb_hcd structure to remove
> * Context: !in_interrupt()
> *
> * Disconnects the root hub, then reverses the effects of usb_add_hcd(),
> * invoking the HCD's stop() method.
> */
> -void usb_remove_hcd(struct usb_hcd *hcd)
> +static void usb_otg_remove_hcd(struct usb_hcd *hcd)
> {
> struct usb_device *rhdev = hcd->self.root_hub;
>
> @@ -2923,6 +2923,51 @@ void usb_remove_hcd(struct usb_hcd *hcd)
>
> usb_put_invalidate_rhdev(hcd);
> }
> +
> +static struct otg_hcd_ops otg_hcd_intf = {
> + .add = usb_otg_add_hcd,
> + .remove = usb_otg_remove_hcd,
> +};
> +
> +/**
> + * usb_add_hcd - finish generic HCD structure initialization and register
> + * @hcd: the usb_hcd structure to initialize
> + * @irqnum: Interrupt line to allocate
> + * @irqflags: Interrupt type flags
> + *
> + * Finish the remaining parts of generic HCD initialization: allocate the
> + * buffers of consistent memory, register the bus, request the IRQ line,
> + * and call the driver's reset() and start() routines.
> + * If it is an OTG device then it only registers the HCD with OTG core.
> + *
> + */
> +int usb_add_hcd(struct usb_hcd *hcd,
> + unsigned int irqnum, unsigned long irqflags)
> +{
> + /* If OTG device, OTG core takes care of adding HCD */
> + if (usb_otg_register_hcd(hcd, irqnum, irqflags, &otg_hcd_intf))
> + return usb_otg_add_hcd(hcd, irqnum, irqflags);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_add_hcd);
> +
> +/**
> + * usb_remove_hcd - shutdown processing for generic HCDs
> + * @hcd: the usb_hcd structure to remove
> + * Context: !in_interrupt()
> + *
> + * Disconnects the root hub, then reverses the effects of usb_add_hcd(),
> + * invoking the HCD's stop() method.
> + * If it is an OTG device then it unregisters the HCD from OTG core
> + * as well.
> + */
> +void usb_remove_hcd(struct usb_hcd *hcd)
> +{
> + /* If OTG device, OTG core takes care of stopping HCD */
> + if (usb_otg_unregister_hcd(hcd))
> + usb_otg_remove_hcd(hcd);
> +}
> EXPORT_SYMBOL_GPL(usb_remove_hcd);
>
> void
> --
> 2.1.4
>
--
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 2/2] Doc: usb: ci-hdrc-usb2: Add phy-clkgate-delay-us entry
On Tue, Sep 08, 2015 at 10:18:15PM -0300, Fabio Estevam wrote: > From: Fabio Estevam > > Add an entry for the optional 'phy-clkgate-delay-us' property that is > used to describe the delay time between putting PHY into low power > mode and turning off the PHY clock. > > Signed-off-by: Li Jun > Signed-off-by: Fabio Estevam > --- > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > index d71ef07..48709dd 100644 > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > @@ -45,6 +45,8 @@ Optional properties: >(4 bytes), This register represents the maximum length of a the burst >in 32-bit words while moving data from the USB bus to system memory, >changing this value takes effect only the SBUSCFG.AHBBRST is 0. > +- phy-clkgate-delay-us: the delay time (us) between putting the PHY into > + low power mode and gating the PHY clock. > > Example: > > @@ -61,4 +63,5 @@ Example: > ahb-burst-config = <0x0>; > tx-burst-size-dword = <0x10>; /* 64 bytes */ > rx-burst-size-dword = <0x10>; > + phy-clkgate-delay-us = <400>; > }; > -- > 1.9.1 > I will queue both for next rc, thanks. -- 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 1/2] usb: chipidea: imx: add usb support for imx7d
On Wed, Sep 09, 2015 at 10:18:39AM +0800, Li Jun wrote:
> Add imx7d usb support.
>
> Signed-off-by: Li Jun
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c |7
> drivers/usb/chipidea/usbmisc_imx.c | 62
>
> 2 files changed, 69 insertions(+)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 867e9f3..f9deea5 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -56,12 +56,19 @@ static const struct ci_hdrc_imx_platform_flag
> imx6sx_usb_data = {
> CI_HDRC_DISABLE_HOST_STREAMING,
> };
>
> +static const struct ci_hdrc_imx_platform_flag imx7d_usb_data = {
> + .flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
> + CI_HDRC_TURN_VBUS_EARLY_ON |
> + CI_HDRC_DISABLE_HOST_STREAMING,
> +};
> +
For imx7, it uses v2.5 core, we don't need to disable stream mode
for host.
The reason for i.mx to disable host stream mode is below issue
(see 8022d3d51c6fb14736e26fd13caca91a38e07580)
TAR 9000378958
Title: Non-Double Word Aligned Buffer Address Sometimes Causes Host
to Hang on OUT Retry
At v2.5 core, this bug has fixed by IP vendor.
> static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
> { .compatible = "fsl,imx28-usb", .data = &imx28_usb_data},
> { .compatible = "fsl,imx27-usb", .data = &imx27_usb_data},
> { .compatible = "fsl,imx6q-usb", .data = &imx6q_usb_data},
> { .compatible = "fsl,imx6sl-usb", .data = &imx6sl_usb_data},
> { .compatible = "fsl,imx6sx-usb", .data = &imx6sl_usb_data},
> + { .compatible = "fsl,imx7d-usb", .data = &imx7d_usb_data},
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, ci_hdrc_imx_dt_ids);
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c
> b/drivers/usb/chipidea/usbmisc_imx.c
> index 5ddab30..f3a4753 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -72,6 +72,14 @@
>
> #define VF610_OVER_CUR_DIS BIT(7)
>
> +#define MX7D_USBNC_USB_CTRL2 0x4
> +#define MX7D_USB_VBUS_WAKEUP_SOURCE_MASK 0x3
> +#define MX7D_USB_VBUS_WAKEUP_SOURCE(v) (v << 0)
> +#define MX7D_USB_VBUS_WAKEUP_SOURCE_VBUS MX7D_USB_VBUS_WAKEUP_SOURCE(0)
> +#define MX7D_USB_VBUS_WAKEUP_SOURCE_AVALID MX7D_USB_VBUS_WAKEUP_SOURCE(1)
> +#define MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID MX7D_USB_VBUS_WAKEUP_SOURCE(2)
> +#define MX7D_USB_VBUS_WAKEUP_SOURCE_SESS_END MX7D_USB_VBUS_WAKEUP_SOURCE(3)
> +
> struct usbmisc_ops {
> /* It's called once when probe a usb device */
> int (*init)(struct imx_usbmisc_data *data);
> @@ -324,6 +332,55 @@ static int usbmisc_vf610_init(struct imx_usbmisc_data
> *data)
> return 0;
> }
>
> +static int usbmisc_imx7d_set_wakeup
> + (struct imx_usbmisc_data *data, bool enabled)
> +{
> + struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
> + unsigned long flags;
> + u32 val;
> + u32 wakeup_setting = (MX6_BM_WAKEUP_ENABLE |
> + MX6_BM_VBUS_WAKEUP | MX6_BM_ID_WAKEUP);
> +
> + spin_lock_irqsave(&usbmisc->lock, flags);
> + val = readl(usbmisc->base);
> + if (enabled) {
> + writel(val | wakeup_setting, usbmisc->base);
> + } else {
> + if (val & MX6_BM_WAKEUP_INTR)
> + dev_dbg(data->dev, "wakeup int\n");
> + writel(val & ~wakeup_setting, usbmisc->base);
> + }
> + spin_unlock_irqrestore(&usbmisc->lock, flags);
> +
> + return 0;
> +}
> +
> +static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
> +{
> + struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
> + unsigned long flags;
> + u32 reg;
> +
> + if (data->index >= 1)
> + return -EINVAL;
> +
> + spin_lock_irqsave(&usbmisc->lock, flags);
> + if (data->disable_oc) {
> + reg = readl(usbmisc->base);
> + writel(reg | MX6_BM_OVER_CUR_DIS, usbmisc->base);
> + }
> +
> + reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
> + reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK;
> + writel(reg | MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID,
> + usbmisc->base + MX7D_USBNC_USB_CTRL2);
> + spin_unlock_irqrestore(&usbmisc->lock, flags);
> +
> + usbmisc_imx7d_set_wakeup(data, false);
> +
> + return 0;
> +}
> +
> static const struct usbmisc_ops imx25_usbmisc_ops = {
> .init = usbmisc_imx25_init,
> .post = usbmisc_imx25_post,
> @@ -351,6 +408,11 @@ static const struct usbmisc_ops imx6sx_usbmisc_ops = {
> .init = usbmisc_imx6sx_init,
> };
>
> +static const struct usbmisc_ops imx7d_usbmisc_ops = {
> + .init = usbmisc_imx7d_init,
> + .set_wakeup = usbmisc_imx7d_set_wakeup,
> +};
> +
> int imx_usbmisc_init(struct imx_usbmisc_data *data)
> {
> struct imx_usbmisc *usbmisc;
> --
> 1.7.9.5
>
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body
Re: [PATCH 2/2] usb: chipidea: imx: fix a typo for imx6sx
On Wed, Sep 09, 2015 at 10:18:40AM +0800, Li Jun wrote:
> Use imx6sx instead of imx6sl's platform flags for imx6sx.
>
> Signed-off-by: Li Jun
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index f9deea5..1becd33 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -67,7 +67,7 @@ static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
> { .compatible = "fsl,imx27-usb", .data = &imx27_usb_data},
> { .compatible = "fsl,imx6q-usb", .data = &imx6q_usb_data},
> { .compatible = "fsl,imx6sl-usb", .data = &imx6sl_usb_data},
> - { .compatible = "fsl,imx6sx-usb", .data = &imx6sl_usb_data},
> + { .compatible = "fsl,imx6sx-usb", .data = &imx6sx_usb_data},
> { .compatible = "fsl,imx7d-usb", .data = &imx7d_usb_data},
> { /* sentinel */ }
> };
> --
> 1.7.9.5
>
Good catch
--
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 v7 2/5] dt-bindings: Add a binding for Mediatek xHCI host controller
On 09/08/2015 01:18 AM, Chunfeng Yun wrote:
> add a DT binding documentation of xHCI host controller for the
> MT8173 SoC from Mediatek.
>
> Signed-off-by: Chunfeng Yun
> ---
> .../devicetree/bindings/usb/mt8173-xhci.txt| 52
> ++
> 1 file changed, 52 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/mt8173-xhci.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
> b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
> new file mode 100644
> index 000..d851bf1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
> @@ -0,0 +1,52 @@
> +MT8173 xHCI
> +
> +The device node for Mediatek SOC USB3.0 host controller
> +
> +Required properties:
> + - compatible : should contain "mediatek,mt8173-xhci"
> + - reg : specifies physical base address and size of the registers,
> + the first one for MAC, the second for IPPC
> + - interrupts : interrupt used by the controller
> + - power-domains : a phandle to USB power domain node to control USB's
> + mtcmos
> + - vusb33-supply : regulator of USB avdd3.3v
> +
> + - clocks : a list of phandle + clock-specifier pairs, one for each
> + entry in clock-names
> + - clock-names : must contain
> + "sys_ck": for clock of xHCI MAC
> + "wakeup_deb_p0": for USB wakeup debounce clock of port0
> + "wakeup_deb_p0": for USB wakeup debounce clock of port1
> +
> + - phys : a list of phandle + phy specifier pairs
> + - usb3-lpm-capable : supports USB3.0 LPM
> + - mediatek,syscon-wakeup : phandle to syscon used to access USB wakeup
> + control register
Wake-up should probably be done in some generic way. That said, I don't
have a specific suggestion other than make it optional.
> + - mediatek,wakeup-src : 1: ip sleep wakeup mode; 2: line state wakeup
> + mode; Others means don't enable wakeup source of USB
This should be optional rather than any other value meaning disabled.
> +
> +Optional properties:
> + - vbus-supply : reference to the VBUS regulator;
> +
> +Example:
> +usb30: usb@1127 {
> + compatible = "mediatek,mt8173-xhci";
> + reg = <0 0x1127 0 0x1000>,
> + <0 0x11280700 0 0x0100>;
> + interrupts = ;
> + power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
> + clocks = <&topckgen CLK_TOP_USB30_SEL>,
> + <&pericfg CLK_PERI_USB0>,
> + <&pericfg CLK_PERI_USB1>;
> + clock-names = "sys_ck",
> + "wakeup_deb_p0",
> + "wakeup_deb_p1";
> + phys = <&phy_port0 PHY_TYPE_USB3>,
> +<&phy_port1 PHY_TYPE_USB2>;
> + vusb33-supply = <&mt6397_vusb_reg>;
> + vbus-supply = <&usb_p1_vbus>;
> + usb3-lpm-capable;
> + mediatek,syscon-wakeup = <&pericfg>;
> + mediatek,wakeup-src = <1>;
> + status = "okay";
> +};
>
--
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 v4 0/22] On-demand device probing
On 09/08/2015 02:30 AM, Tomeu Vizoso wrote: > On 7 September 2015 at 22:50, Rob Herring wrote: >> On Mon, Sep 7, 2015 at 7:23 AM, Tomeu Vizoso >> wrote: >>> Hello, >>> >>> I have a problem with the panel on my Tegra Chromebook taking longer >>> than expected to be ready during boot (Stéphane Marchesin reported what >>> is basically the same issue in [0]), and have looked into ordered >>> probing as a better way of solving this than moving nodes around in the >>> DT or playing with initcall levels and linking order. >>> >>> While reading the thread [1] that Alexander Holler started with his >>> series to make probing order deterministic, it occurred to me that it >>> should be possible to achieve the same by probing devices as they are >>> referenced by other devices. >>> >>> This basically reuses the information that is already implicit in the >>> probe() implementations, saving us from refactoring existing drivers or >>> adding information to DTBs. >>> >>> During review of v1 of this series Linus Walleij suggested that it >>> should be the device driver core to make sure that dependencies are >>> ready before probing a device. I gave this idea a try [2] but Mark Brown >>> pointed out to the logic duplication between the resource acquisition >>> and dependency discovery code paths (though I think it's fairly minor). >>> >>> To address that code duplication I experimented with Arnd's devm_probe >>> [3] concept of having drivers declare their dependencies instead of >>> acquiring them during probe, and while it worked [4], I don't think we >>> end up winning anything when compared to just probing devices on-demand >>> from resource getters. >>> >>> One remaining objection is to the "sprinkling" of calls to >>> of_device_probe() in the resource getters of each subsystem, but I think >>> it's the right thing to do given that the storage of resources is >>> currently subsystem-specific. >>> >>> We could avoid the above by moving resource storage into the core, but I >>> don't think there's a compelling case for that. >>> >>> I have tested this on boards with Tegra, iMX.6, Exynos, Rockchip and >>> OMAP SoCs, and these patches were enough to eliminate all the deferred >>> probes (except one in PandaBoard because omap_dma_system doesn't have a >>> firmware node as of yet). >>> >>> Have submitted a branch [5] with only these patches on top of thursday's >>> linux-next to kernelci.org and I don't see any issues that could be >>> caused by them. For some reason it currently has more passes than the >>> version of -next it's based on! >>> >>> With this series I get the kernel to output to the panel in 0.5s, >>> instead of 2.8s. >>> >>> Regards, >>> >>> Tomeu >>> >>> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html >>> >>> [1] https://lkml.org/lkml/2014/5/12/452 >>> >>> [2] https://lkml.org/lkml/2015/6/17/305 >>> >>> [3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689 >>> >>> [4] https://lkml.org/lkml/2015/7/21/441a >>> >>> [5] >>> https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v6 >>> >>> [6] >>> http://kernelci.org/boot/all/job/collabora/kernel/v4.2-11902-g25d80c927f8b/ >>> >>> [7] http://kernelci.org/boot/all/job/next/kernel/next-20150903/ >>> >>> Changes in v4: >>> - Added bus.pre_probe callback so the probes of Primecell devices can be >>> deferred if their device IDs cannot be yet read because of the clock >>> driver not having probed when they are registered. Maybe this goes >>> overboard and the matching information should be in the DT if there is >>> one. >> >> Seems overboard to me or at least a separate problem. > > It's a separate problem but this was preventing the series from > working on a few boards. What is the failure? Not booting? Fixing not working would certainly not be overboard. > >> Most clocks have >> to be setup before the driver model simply because timers depend on >> clocks usually. > > Yes, but in this case the apb clocks for the primecell devices are > implemented in a normal platform driver (vexpress_osc_driver), instead > of using CLK_OF_DECLARE. Okay. Rob -- 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 v7 1/5] dt-bindings: Add usb3.0 phy binding for MT65xx SoCs
On 09/08/2015 01:17 AM, Chunfeng Yun wrote:
> add a DT binding documentation of usb3.0 phy for MT65xx
> SoCs from Mediatek.
>
> Signed-off-by: Chunfeng Yun
One comment, otherwise:
Acked-by: Rob Herring
> ---
> .../devicetree/bindings/phy/phy-mt65xx-usb.txt | 69
> ++
> 1 file changed, 69 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
> b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
> new file mode 100644
> index 000..5812d20
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
> @@ -0,0 +1,69 @@
> +mt65xx USB3.0 PHY binding
> +--
> +
> +This binding describes a usb3.0 phy for mt65xx platforms of Medaitek SoC.
> +
> +Required properties (controller (parent) node):
> + - compatible: should be "mediatek,mt8173-u3phy"
> + - reg : offset and length of register for phy, exclude port's
> + register.
> + - clocks: a list of phandle + clock-specifier pairs, one for each
> + entry in clock-names
> + - clock-names : must contain
> + "u3phya_ref": for reference clock of usb3.0 analog phy.
> +
> +Required nodes : a sub-node is required for each port the controller
> + provides. Address range information including the usual
> + 'reg' property is used inside these nodes to describe
> + the controller's topology. These nodes are translated
> + by the driver's .xlate() function.
The last sentence is Linux specific. Please remove.
> +
> +Required properties (port (child) node):
> +- reg: address and length of the register set for the port.
> +- #phy-cells : should be 1 (See second example)
> + cell after port phandle is phy type from:
> + - PHY_TYPE_USB2
> + - PHY_TYPE_USB3
> +
> +Example:
> +
> +u3phy: usb-phy@1129 {
> + compatible = "mediatek,mt8173-u3phy";
> + reg = <0 0x1129 0 0x800>;
> + clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
> + clock-names = "u3phya_ref";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + status = "okay";
> +
> + phy_port0: port@11290800 {
> + reg = <0 0x11290800 0 0x800>;
> + #phy-cells = <1>;
> + status = "okay";
> + };
> +
> + phy_port1: port@11291000 {
> + reg = <0 0x11291000 0 0x800>;
> + #phy-cells = <1>;
> + status = "okay";
> + };
> +};
> +
> +Specifying phy control of devices
> +-
> +
> +Device nodes should specify the configuration required in their "phys"
> +property, containing a phandle to the phy port node and a device type;
> +phy-names for each port are optional.
> +
> +Example:
> +
> +#include
> +
> +usb30: usb@1127 {
> + ...
> + phys = <&phy_port0 PHY_TYPE_USB3>;
> + phy-names = "usb3-0";
> + ...
> +};
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: chipidea: imx: add usb support for imx7d
On Wed, Sep 09, 2015 at 10:35:59AM +0800, Peter Chen wrote:
... ...
> >
> > +static const struct ci_hdrc_imx_platform_flag imx7d_usb_data = {
> > + .flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
> > + CI_HDRC_TURN_VBUS_EARLY_ON |
> > + CI_HDRC_DISABLE_HOST_STREAMING,
> > +};
> > +
>
> For imx7, it uses v2.5 core, we don't need to disable stream mode
> for host.
>
> The reason for i.mx to disable host stream mode is below issue
> (see 8022d3d51c6fb14736e26fd13caca91a38e07580)
>
> TAR 9000378958
> Title: Non-Double Word Aligned Buffer Address Sometimes Causes Host
> to Hang on OUT Retry
>
> At v2.5 core, this bug has fixed by IP vendor.
>
ok, I will remove this flag.
Li Jun
--
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
