[PATCH] usb: Retry port status check on resume to work around RH bugs

2015-01-23 Thread Julius Werner
The EHCI controller on the RK3288 SoC is violating basic parts of the
USB spec and thereby unable to properly resume a suspended port. It does
not start SOF generation within 3ms of finishing resume signaling, so
the attached device will drop off the bus again. This is a particular
problem with runtime PM, where accessing the device will trigger a
resume that immediately makes it unavailabe (and reenumerate with a new
handle).

Thankfully, the persist feature is generally able to work around stuff
like that. Unfortunately, it doesn't quite work in this particular case
because the controller will turn off the CurrentConnectStatus bit for an
instant while the device is reconnecting, which causes the kernel to
conclude that it permanently disappeared. This patch adds a tiny retry
mechanism to the core port resume code which will catch this case and
shouldn't have any notable impact on other controllers.

Signed-off-by: Julius Werner 
---
 drivers/usb/core/hub.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index aeb50bb..d1d0a66 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2896,10 +2896,12 @@ static int port_is_suspended(struct usb_hub *hub, 
unsigned portstatus)
  */
 static int check_port_resume_type(struct usb_device *udev,
struct usb_hub *hub, int port1,
-   int status, unsigned portchange, unsigned portstatus)
+   int status, u16 portchange, u16 portstatus)
 {
struct usb_port *port_dev = hub->ports[port1 - 1];
+   int retries = 3;
 
+retry:
/* Is a warm reset needed to recover the connection? */
if (status == 0 && udev->reset_resume
&& hub_port_warm_reset_required(hub, port1, portstatus)) {
@@ -2909,8 +2911,15 @@ static int check_port_resume_type(struct usb_device 
*udev,
else if (status || port_is_suspended(hub, portstatus) ||
!port_is_power_on(hub, portstatus) ||
!(portstatus & USB_PORT_STAT_CONNECTION)) {
-   if (status >= 0)
+   if (status >= 0) {
+   if (retries--) {
+   udelay(200);
+   status = hub_port_status(hub, port1,
+   &portstatus, &portchange);
+   goto retry;
+   }
status = -ENODEV;
+   }
}
 
/* Can't do a normal resume if the port isn't enabled,
-- 
2.1.2

--
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] usb: host: oxu210hp-hcd: Fix deadlock in oxu_qh_alloc()

2015-01-23 Thread Alexey Khoroshilov
oxu_qh_alloc() acquires oxu->mem_lock spinlock, finds free qh slot
and calls ehci_qtd_alloc() to initialize qh->dummy, but
ehci_qtd_alloc() acquires oxu->mem_lock as well.
That means an unavoidable deadlock in oxu_qh_alloc().

The patch fixes the issue by introduction of unlocked version
of ehci_qtd_alloc().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/host/oxu210hp-hcd.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index 036924e640f5..15d4cf2c35cd 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -573,13 +573,11 @@ static inline void oxu_qtd_free(struct oxu_hcd *oxu, 
struct ehci_qtd *qtd)
spin_unlock(&oxu->mem_lock);
 }
 
-static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
+static struct ehci_qtd *ehci_qtd_alloc_unlocked(struct oxu_hcd *oxu)
 {
int i;
struct ehci_qtd *qtd = NULL;
 
-   spin_lock(&oxu->mem_lock);
-
for (i = 0; i < QTD_NUM; i++)
if (!oxu->qtd_used[i])
break;
@@ -598,6 +596,17 @@ static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
oxu->qtd_used[i] = 1;
}
 
+   return qtd;
+}
+
+static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
+{
+   struct ehci_qtd *qtd;
+
+   spin_lock(&oxu->mem_lock);
+
+   qtd = ehci_qtd_alloc_unlocked(oxu);
+
spin_unlock(&oxu->mem_lock);
 
return qtd;
@@ -651,7 +660,7 @@ static struct ehci_qh *oxu_qh_alloc(struct oxu_hcd *oxu)
INIT_LIST_HEAD(&qh->qtd_list);
 
/* dummy td enables safe urb queuing */
-   qh->dummy = ehci_qtd_alloc(oxu);
+   qh->dummy = ehci_qtd_alloc_unlocked(oxu);
if (qh->dummy == NULL) {
oxu_dbg(oxu, "no dummy td\n");
oxu->qh_used[i] = 0;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Boot regression on Versatile Express TC2 after commit b2b49ccbdd

2015-01-23 Thread Nicolas Pitre
On Fri, 23 Jan 2015, Alan Stern wrote:

> On Fri, 23 Jan 2015, Nicolas Pitre wrote:
> 
> > On Fri, 23 Jan 2015, Alan Stern wrote:
> > 
> > > On Wed, 21 Jan 2015, Rafael J. Wysocki wrote:
> > > 
> > > > Well, OK, so there is the case when the controller is not suspended, 
> > > > but it
> > > > doesn't generate interrupts for wakeup signals from devices, which 
> > > > generally
> > > > is different from the case when the controller itself is suspended too 
> > > > that
> > > > requires it to be wakeup-capable for things to work.
> > > > 
> > > > An additional flag may still be better then.
> > > 
> > > We can do that if it turns out to be necessary, but let's try something
> > > simpler first.  Nicolas, does this patch fix the problem?
> > 
> > No, it doesn't.
> 
> All right, then how about this?

This worked.

Tested-by: Nicolas Pitre 

> 
> Alan Stern
> 
> 
> 
> Index: usb-3.19/include/linux/usb/hcd.h
> ===
> --- usb-3.19.orig/include/linux/usb/hcd.h
> +++ usb-3.19/include/linux/usb/hcd.h
> @@ -146,6 +146,8 @@ struct usb_hcd {
>   unsignedamd_resume_bug:1; /* AMD remote wakeup quirk */
>   unsignedcan_do_streams:1; /* HC supports streams */
>   unsignedtpl_support:1; /* OTG & EH TPL support */
> + unsignedcant_recv_wakeups:1;
> + /* wakeup requests from downstream aren't received */
>  
>   unsigned intirq;/* irq allocated */
>   void __iomem*regs;  /* device memory/io */
> Index: usb-3.19/drivers/usb/host/isp1760-hcd.c
> ===
> --- usb-3.19.orig/drivers/usb/host/isp1760-hcd.c
> +++ usb-3.19/drivers/usb/host/isp1760-hcd.c
> @@ -2247,6 +2247,9 @@ struct usb_hcd *isp1760_register(phys_ad
>   hcd->rsrc_start = res_start;
>   hcd->rsrc_len = res_len;
>  
> + /* This driver doesn't support wakeup requests */
> + hcd->cant_recv_wakeups = 1;
> +
>   ret = usb_add_hcd(hcd, irq, irqflags);
>   if (ret)
>   goto err_unmap;
> Index: usb-3.19/drivers/usb/core/driver.c
> ===
> --- usb-3.19.orig/drivers/usb/core/driver.c
> +++ usb-3.19/drivers/usb/core/driver.c
> @@ -1780,6 +1780,18 @@ static int autosuspend_check(struct usb_
>   dev_dbg(&udev->dev, "remote wakeup needed for autosuspend\n");
>   return -EOPNOTSUPP;
>   }
> +
> + /*
> +  * If the device is a direct child of the root hub and the HCD
> +  * doesn't handle wakeup requests, don't allow autosuspend when
> +  * wakeup is needed.
> +  */
> + if (w && udev->parent == udev->bus->root_hub &&
> + bus_to_hcd(udev->bus)->cant_recv_wakeups) {
> + dev_dbg(&udev->dev, "HCD doesn't handle wakeup requests\n");
> + return -EOPNOTSUPP;
> + }
> +
>   udev->do_remote_wakeup = w;
>   return 0;
>  }
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Boot regression on Versatile Express TC2 after commit b2b49ccbdd

2015-01-23 Thread Alan Stern
On Fri, 23 Jan 2015, Nicolas Pitre wrote:

> On Fri, 23 Jan 2015, Alan Stern wrote:
> 
> > On Wed, 21 Jan 2015, Rafael J. Wysocki wrote:
> > 
> > > Well, OK, so there is the case when the controller is not suspended, but 
> > > it
> > > doesn't generate interrupts for wakeup signals from devices, which 
> > > generally
> > > is different from the case when the controller itself is suspended too 
> > > that
> > > requires it to be wakeup-capable for things to work.
> > > 
> > > An additional flag may still be better then.
> > 
> > We can do that if it turns out to be necessary, but let's try something
> > simpler first.  Nicolas, does this patch fix the problem?
> 
> No, it doesn't.

All right, then how about this?

Alan Stern



Index: usb-3.19/include/linux/usb/hcd.h
===
--- usb-3.19.orig/include/linux/usb/hcd.h
+++ usb-3.19/include/linux/usb/hcd.h
@@ -146,6 +146,8 @@ struct usb_hcd {
unsignedamd_resume_bug:1; /* AMD remote wakeup quirk */
unsignedcan_do_streams:1; /* HC supports streams */
unsignedtpl_support:1; /* OTG & EH TPL support */
+   unsignedcant_recv_wakeups:1;
+   /* wakeup requests from downstream aren't received */
 
unsigned intirq;/* irq allocated */
void __iomem*regs;  /* device memory/io */
Index: usb-3.19/drivers/usb/host/isp1760-hcd.c
===
--- usb-3.19.orig/drivers/usb/host/isp1760-hcd.c
+++ usb-3.19/drivers/usb/host/isp1760-hcd.c
@@ -2247,6 +2247,9 @@ struct usb_hcd *isp1760_register(phys_ad
hcd->rsrc_start = res_start;
hcd->rsrc_len = res_len;
 
+   /* This driver doesn't support wakeup requests */
+   hcd->cant_recv_wakeups = 1;
+
ret = usb_add_hcd(hcd, irq, irqflags);
if (ret)
goto err_unmap;
Index: usb-3.19/drivers/usb/core/driver.c
===
--- usb-3.19.orig/drivers/usb/core/driver.c
+++ usb-3.19/drivers/usb/core/driver.c
@@ -1780,6 +1780,18 @@ static int autosuspend_check(struct usb_
dev_dbg(&udev->dev, "remote wakeup needed for autosuspend\n");
return -EOPNOTSUPP;
}
+
+   /*
+* If the device is a direct child of the root hub and the HCD
+* doesn't handle wakeup requests, don't allow autosuspend when
+* wakeup is needed.
+*/
+   if (w && udev->parent == udev->bus->root_hub &&
+   bus_to_hcd(udev->bus)->cant_recv_wakeups) {
+   dev_dbg(&udev->dev, "HCD doesn't handle wakeup requests\n");
+   return -EOPNOTSUPP;
+   }
+
udev->do_remote_wakeup = w;
return 0;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Boot regression on Versatile Express TC2 after commit b2b49ccbdd

2015-01-23 Thread Nicolas Pitre
On Fri, 23 Jan 2015, Alan Stern wrote:

> On Wed, 21 Jan 2015, Rafael J. Wysocki wrote:
> 
> > Well, OK, so there is the case when the controller is not suspended, but it
> > doesn't generate interrupts for wakeup signals from devices, which generally
> > is different from the case when the controller itself is suspended too that
> > requires it to be wakeup-capable for things to work.
> > 
> > An additional flag may still be better then.
> 
> We can do that if it turns out to be necessary, but let's try something
> simpler first.  Nicolas, does this patch fix the problem?

No, it doesn't.

> 
> Alan Stern
> 
> 
> 
> Index: usb-3.19/drivers/usb/host/isp1760-hcd.c
> ===
> --- usb-3.19.orig/drivers/usb/host/isp1760-hcd.c
> +++ usb-3.19/drivers/usb/host/isp1760-hcd.c
> @@ -1321,7 +1321,10 @@ static int isp1760_run(struct usb_hcd *h
>   u32 command;
>   u32 chipid;
>  
> + /* Use old-style polling to detect wakeup requests */
> +#if 0
>   hcd->uses_new_polling = 1;
> +#endif
>  
>   hcd->state = HC_STATE_RUNNING;
>  
> 
> 
--
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: Boot regression on Versatile Express TC2 after commit b2b49ccbdd

2015-01-23 Thread Alan Stern
On Wed, 21 Jan 2015, Rafael J. Wysocki wrote:

> Well, OK, so there is the case when the controller is not suspended, but it
> doesn't generate interrupts for wakeup signals from devices, which generally
> is different from the case when the controller itself is suspended too that
> requires it to be wakeup-capable for things to work.
> 
> An additional flag may still be better then.

We can do that if it turns out to be necessary, but let's try something
simpler first.  Nicolas, does this patch fix the problem?

Alan Stern



Index: usb-3.19/drivers/usb/host/isp1760-hcd.c
===
--- usb-3.19.orig/drivers/usb/host/isp1760-hcd.c
+++ usb-3.19/drivers/usb/host/isp1760-hcd.c
@@ -1321,7 +1321,10 @@ static int isp1760_run(struct usb_hcd *h
u32 command;
u32 chipid;
 
+   /* Use old-style polling to detect wakeup requests */
+#if 0
hcd->uses_new_polling = 1;
+#endif
 
hcd->state = HC_STATE_RUNNING;
 

--
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: ohci: sporadic crash/lockup in ohci-hcd io_watchdog_func()

2015-01-23 Thread Alan Stern
On Tue, 20 Jan 2015, Heiko Przybyl wrote:

> I'm not 100% sure, but then it's probably a race between urb 
> enqueuing (duplicates?) and the watchdog orphan cleanup.
> 
> The crash log already shows the double list add in ohci_urb_enqueue
> "
> ohci-hcd.c:238: list_add(&ed->in_use_list, &ohci->eds_in_use);
> "
> This is probably due to the ed returned by ed_get() being reused before the
> watchdog ran, thus the same in_use_list re-added to ohci.eds_in_use.

But this won't happen unless ed->state was ED_IDLE; and then
ed_schedule() changes ed->state to ED_OPER before ed is added to the
in_use_list.

> Entries seem to get removed in finish_unlinks()
> "
> ohci-q.c:1090: list_del(&ed->in_use_list);
> "
> with list_del() poisoning the next/prev pointers of the removed entry.

And finish_unlinks() is where ed->state gets set back to ED_IDLE.  
Furthermore, all these operations take place under the protection of 
ohci->lock, so they can't race with each other.

They have to happen in strict sequence: list_add, list_del, list_add.  
The second list_add can't occur until after the list_del, because 
ed->state won't be equal to ED_IDLE until then.

At least, that's how it's _supposed_ to work...

> Now with the watchdog starting cleanup it iterates over the ohci.eds_in_use 
> list
> that still has the second very same entry of in_use_list we double-added (but 
> now with 0xdead... pointers) and we fault on 
> "
> ohci-hcd.c:761: if (ed->pending_td) {
> "
> 
> I hope that makes any sense? I'll hook up the list checking tomorrow. Though 
> I 
> haven't hit the (double-add) problem again, since the bug report. Seems 
> pretty 
> specific the whole thing.

It's a puzzling problem.  I hope you can find the cause.

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 v2] usb: phy: generic: Do not fail when 'reset-gpios' is not used

2015-01-23 Thread Fabio Estevam
On Fri, Jan 23, 2015 at 5:13 PM, Robert Jarzmik  wrote:

> I think the right solution was proposed a week ago or so here :
>   https://lkml.org/lkml/2015/1/14/1016
>
> I added Paul to the discussion as he is the author of it.

This makes USB host to work again on imx53-qsb, but on imx51-babbage I
still get a hang:

[1.341002] ci_hdrc ci_hdrc.1: doesn't support gadget
[1.346177] ci_hdrc ci_hdrc.1: EHCI Host Controller
[1.351393] ci_hdrc ci_hdrc.1: new USB bus registered, assigned bus number 1
[1.373328] ci_hdrc ci_hdrc.1: USB 2.0 started, EHCI 1.00
[1.383907] hub 1-0:1.0: USB hub found
[1.388005] hub 1-0:1.0: 1 port detected
--
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: generic: Do not fail when 'reset-gpios' is not used

2015-01-23 Thread Robert Jarzmik
Fabio Estevam  writes:

> On Fri, Jan 23, 2015 at 2:12 AM, Fabio Estevam  wrote:
>> Will need to debug more.
>
> The problem here is that:
>
> nop->gpiod_reset = devm_gpiod_get(dev, "reset-gpios");
>
> Always return -2.
>
> On imx51-babbage.dts we pass 'reset-gpios' and it used to retrieve it
> fine prior to e9f2cefb0cdc2ae ("usb: phy: generic: migrate to
> gpio_desc") by using of_get_named_gpio_flags().
>
> Any ideas as to why devm_gpiod_get() always fail now?
I think the right solution was proposed a week ago or so here :
  https://lkml.org/lkml/2015/1/14/1016

I added Paul to the discussion as he is the author of it.

Cheers.

-- 
Robert
--
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] Fix force effect modifications for the Microsoft Sidewinder Force Feedback Pro 2 joystick

2015-01-23 Thread Benjamin Tissoires
I have no clue what this code is doing (don't know much about the ff system).
Quoting your initial 0001/0001, I thing the commit message should be
the following.
Jim, can you please validate it?

"
The FF2 driver (usbhid/hid-pidff.c) does not set the effect ID when
uploading an effect. The result is that the initial upload works but
subsequent uploads to modify effect parameters are all directed at the
last-created effect.

The targeted effect ID must be passed back to the device when effect
parameters are changed. This is done at the start of
"pidff_set_condition_report", "pidff_set_periodic_report" etc. based on
the value of "pidff->block_load[PID_EFFECT_
BLOCK_INDEX].value[0]".
However, this value is only ever set during pidff_request_effect_upload.
The result is stored in "pidff->pid_id[effect->id]" at the end of
pid_upload_effect, for later use. However, if an effect is modified and
re-sent then this identifier is not being copied back from
pidff->pid_id[effect->id] before sending the command to the device. The
fix is to do this at the start of pidff_upload_effect.
"

---

As said, I have no clue* of what this is supposed to do, so I can not
add my reviewed-by here. The patch seems good however so nothing
should prevent us to take it if it fixes your problem :)

Cheers,
Benjamin

* OK, I am pushing a little bit, I just don't want today to go deep in
the code :-)


On Fri, Jan 23, 2015 at 12:21 PM, Jim Keir  wrote:
> ---
> Hi,
>
> Changes from previous patches:
> - All changes removed from higher-level files
> - Extra debug line removed
>
> Signed-off-by: Jim Keir 
>
>  drivers/hid/usbhid/hid-pidff.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
> index 0b531c6..1b3fa70 100644
> --- a/drivers/hid/usbhid/hid-pidff.c
> +++ b/drivers/hid/usbhid/hid-pidff.c
> @@ -568,6 +568,12 @@ static int pidff_upload_effect(struct input_dev *dev, 
> struct ff_effect *effect,
> int type_id;
> int error;
>
> +   pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] = 0;
> +   if (old && effect) {
> +   pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] =
> +   pidff->pid_id[effect->id];
> +   }
> +
> switch (effect->type) {
> case FF_CONSTANT:
> if (!old) {
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] Fix initialisation for the Microsoft Sidewinder Force Feedback Pro 2 joystick

2015-01-23 Thread Benjamin Tissoires
You are missing here (before the first '---' the commit message which
should state why you want to have this patch merged.

To rephrase and continue what you wrote in your very first submission
of this series, this could be (including your Signed-of-by):

The FF2 driver (usbhid/hid-pidff.c) sends commands to the stick
during ff_init. However, this is called inside a block where
driver_input_lock is locked, so the results of these initial commands
are discarded. This behavior is the "killer", without this nothing else works.

ff_init issues commands using "hid_hw_request". This eventually goes to
hid_input_report, which returns -EBUSY because driver_input_lock is
locked. The change is to delay the ff_init call in hid-core.c until
after this lock has been released.

Calling hid_device_io_start() releases the lock so the device can be configured.
We also need to call hid_device_io_stop() on exit for the lock to
remain locked while ending the init of the drivers.

Signed-off-by: Jim Keir 

---

With the changes above (or if you fix my typos), the patch is
Reviewed-by: Benjamin.tissoires 

Jiri, could you amend the commit with the above so that Jim won't be
desperate by submitting a patch?

Cheers,
Benjamin


On Fri, Jan 23, 2015 at 12:21 PM, Jim Keir  wrote:
> ---
> Hi,
>
> Changes from previous patches:
> - All changes removed from higher-level files
> - Calls to hid_device_io_start/stop added here
>
> Signed-off-by: Jim Keir 
>
>  drivers/hid/usbhid/hid-pidff.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
> index 10b6167..0b531c6 100644
> --- a/drivers/hid/usbhid/hid-pidff.c
> +++ b/drivers/hid/usbhid/hid-pidff.c
> @@ -1252,6 +1252,8 @@ int hid_pidff_init(struct hid_device *hid)
>
> pidff->hid = hid;
>
> +   hid_device_io_start(hid);
> +
> pidff_find_reports(hid, HID_OUTPUT_REPORT, pidff);
> pidff_find_reports(hid, HID_FEATURE_REPORT, pidff);
>
> @@ -1315,9 +1317,13 @@ int hid_pidff_init(struct hid_device *hid)
>
> hid_info(dev, "Force feedback for USB HID PID devices by Anssi 
> Hannula \n");
>
> +   hid_device_io_stop(hid);
> +
> return 0;
>
>   fail:
> +   hid_device_io_stop(hid);
> +
> kfree(pidff);
> return error;
>  }
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] Fix initialisation for the Microsoft Sidewinder Force Feedback Pro 2 joystick

2015-01-23 Thread Jim Keir
---
Hi,

Changes from previous patches:
- All changes removed from higher-level files
- Calls to hid_device_io_start/stop added here

Signed-off-by: Jim Keir  

 drivers/hid/usbhid/hid-pidff.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 10b6167..0b531c6 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -1252,6 +1252,8 @@ int hid_pidff_init(struct hid_device *hid)
 
pidff->hid = hid;
 
+   hid_device_io_start(hid);
+
pidff_find_reports(hid, HID_OUTPUT_REPORT, pidff);
pidff_find_reports(hid, HID_FEATURE_REPORT, pidff);
 
@@ -1315,9 +1317,13 @@ int hid_pidff_init(struct hid_device *hid)
 
hid_info(dev, "Force feedback for USB HID PID devices by Anssi Hannula 
\n");
 
+   hid_device_io_stop(hid);
+
return 0;
 
  fail:
+   hid_device_io_stop(hid);
+
kfree(pidff);
return error;
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Boot regression on Versatile Express TC2 after commit b2b49ccbdd

2015-01-23 Thread Pawel Moll
On Fri, 2015-01-23 at 17:08 +, Nicolas Pitre wrote:
> On Fri, 23 Jan 2015, Pawel Moll wrote:
> 
> > On Thu, 2015-01-22 at 21:11 +, Rafael J. Wysocki wrote:
> > > > I'll work on it when I have some spare time.  Of course, it would be 
> > > > better if somebody could add the proper wakeup-detect code to the 
> > > > isp1760-hcd driver...
> > > 
> > > Yes, it would.
> > 
> > Whether it's a hack or no - I don't know, but it makes my board detect
> > USB devices again with v3.19-rc5, without any extra kernel parameters:
> > 
> > 8<---
> > diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c
> > index 395649f..92a8297 100644
> > --- a/drivers/usb/host/isp1760-hcd.c
> > +++ b/drivers/usb/host/isp1760-hcd.c
> > @@ -2250,7 +2250,6 @@ struct usb_hcd *isp1760_register(phys_addr_t 
> > res_start, resource_size_t res_len,
> > ret = usb_add_hcd(hcd, irq, irqflags);
> > if (ret)
> > goto err_unmap;
> > -   device_wakeup_enable(hcd->self.controller);
> >  
> > return hcd;
> > 8<---
> > 
> > Care to give it a go, Nico?
> 
> Doesn't work for me. I still need usbcore.autosuspend=-1 on the kernel 
> cmdline for USB to work.

Interesting. Tried again and this time didn't work either... Will have a
look again on Monday and see if it changes again :-) (and in the
meantime try to think what has changed).

Pawel

--
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] Fix force effect modifications for the Microsoft Sidewinder Force Feedback Pro 2 joystick

2015-01-23 Thread Jim Keir
---
Hi,

Changes from previous patches: 
- All changes removed from higher-level files
- Extra debug line removed

Signed-off-by: Jim Keir  

 drivers/hid/usbhid/hid-pidff.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 0b531c6..1b3fa70 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -568,6 +568,12 @@ static int pidff_upload_effect(struct input_dev *dev, 
struct ff_effect *effect,
int type_id;
int error;
 
+   pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] = 0;
+   if (old && effect) {
+   pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] =
+   pidff->pid_id[effect->id];
+   }
+
switch (effect->type) {
case FF_CONSTANT:
if (!old) {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Boot regression on Versatile Express TC2 after commit b2b49ccbdd

2015-01-23 Thread Nicolas Pitre
On Fri, 23 Jan 2015, Pawel Moll wrote:

> On Thu, 2015-01-22 at 21:11 +, Rafael J. Wysocki wrote:
> > > I'll work on it when I have some spare time.  Of course, it would be 
> > > better if somebody could add the proper wakeup-detect code to the 
> > > isp1760-hcd driver...
> > 
> > Yes, it would.
> 
> Whether it's a hack or no - I don't know, but it makes my board detect
> USB devices again with v3.19-rc5, without any extra kernel parameters:
> 
> 8<---
> diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c
> index 395649f..92a8297 100644
> --- a/drivers/usb/host/isp1760-hcd.c
> +++ b/drivers/usb/host/isp1760-hcd.c
> @@ -2250,7 +2250,6 @@ struct usb_hcd *isp1760_register(phys_addr_t res_start, 
> resource_size_t res_len,
>   ret = usb_add_hcd(hcd, irq, irqflags);
>   if (ret)
>   goto err_unmap;
> - device_wakeup_enable(hcd->self.controller);
>  
>   return hcd;
> 8<---
> 
> Care to give it a go, Nico?

Doesn't work for me. I still need usbcore.autosuspend=-1 on the kernel 
cmdline for USB to work.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: gadget: composite: Provide list of registered functions

2015-01-23 Thread Felipe Balbi
On Fri, Jan 23, 2015 at 05:56:22PM +0100, Krzysztof Opasiak wrote:
> 
> 
> > -Original Message-
> > From: Felipe Balbi [mailto:ba...@ti.com]
> > Sent: Friday, January 23, 2015 5:27 PM
> > To: Krzysztof Opasiak
> > Cc: ba...@ti.com; linux-usb@vger.kernel.org;
> > gre...@linuxfoundation.org; bige...@breakpoint.cc;
> > s.wa...@samsung.com; k.lewando...@samsung.com;
> > m.szyprow...@samsung.com; andrze...@samsung.com
> > Subject: Re: [PATCH] usb: gadget: composite: Provide list of
> > registered functions
> > 
> > On Fri, Jan 23, 2015 at 05:09:07PM +0100, Krzysztof Opasiak wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Felipe Balbi [mailto:ba...@ti.com]
> > > > Sent: Monday, January 19, 2015 7:59 PM
> > > > To: Krzysztof Opasiak
> > > > Cc: ba...@ti.com; linux-usb@vger.kernel.org;
> > > > gre...@linuxfoundation.org; bige...@breakpoint.cc;
> > > > s.wa...@samsung.com; k.lewando...@samsung.com;
> > > > m.szyprow...@samsung.com; andrze...@samsung.com
> > > > Subject: Re: [PATCH] usb: gadget: composite: Provide list of
> > > > registered functions
> > > >
> > > > On Mon, Jan 19, 2015 at 02:17:19PM +0100, Krzysztof Opasiak
> > wrote:
> > > > > Driver which provides implementation of some USB functions
> > > > registers
> > > > > usb_function_driver structure in composite framework.
> > > > > Function drivers are identifed using registered name.
> > > > >
> > > > > When gadget is composed using configfs user must know what
> > names
> > > > has
> > > > > been registered. If function is compiled as a module this
> > > > information
> > > > > can be found in modules.alias file. If function is compiled-
> > in,
> > > > there
> > > > > is no way to discover what usb functions are available in
> > > > currently
> > > > > running kernel.
> > > > >
> > > > > Such situation is nothing new for linux kernel.
> > > > > Similar situation is with file systems. While mounting user
> > can
> > > > > provide an fs type argument using -t option in mount.
> > > > > Those type names are registered by drivers. To make those
> > names
> > > > > discoverable there is a /proc/filesystems which exports the
> > list
> > > > of
> > > > > currently registered file systems.
> > > > >
> > > > > This patch adds /proc/usb-functions attribute which exports
> > the
> > > > list
> > > > > of currently registered function drivers.
> > > > > This allows user to discover list of both compiled-in
> > functions
> > > > and
> > > > > from loaded kernel modules.
> > > > >
> > > > > Signed-off-by: Krzysztof Opasiak 
> > > >
> > > > you need to document the new file under Documentation/ABI/
> > > >
> > >
> > > I have just sent v2 version with documentation.
> > >
> > > I have done some more research and it looks like /sys/kernel
> > could be
> > > a good alternative if you find that proc usage is not a good
> > idea.
> > > What do you thing? Should we continue with /proc/usb-functions or
> > > replace this patch with /sys/kernel/usb-functions or maybe
> > 
> > why don't we place the file at the same directory as our configfs
> > has been mounted ?
> 
> Default place for mounting configfs is /sys/kernel/config.
> It is created in init function of configfs module. For example
> systemd mounts there configfs on system startup.
> 
> So summing up, desired location should be /sys/kernel/usb-functions?

shouldn't this be /sys/kernel/config/usb-functions ?

in fact, thinking about absolute paths isn't really the best. I can
decide to mount configfs anywhere. What we want is for the file to be
available on the root directory of our usb-functions configfs interface
:-)

-- 
balbi


signature.asc
Description: Digital signature


RE: [PATCH] usb: gadget: composite: Provide list of registered functions

2015-01-23 Thread Krzysztof Opasiak


> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Friday, January 23, 2015 5:27 PM
> To: Krzysztof Opasiak
> Cc: ba...@ti.com; linux-usb@vger.kernel.org;
> gre...@linuxfoundation.org; bige...@breakpoint.cc;
> s.wa...@samsung.com; k.lewando...@samsung.com;
> m.szyprow...@samsung.com; andrze...@samsung.com
> Subject: Re: [PATCH] usb: gadget: composite: Provide list of
> registered functions
> 
> On Fri, Jan 23, 2015 at 05:09:07PM +0100, Krzysztof Opasiak wrote:
> >
> >
> > > -Original Message-
> > > From: Felipe Balbi [mailto:ba...@ti.com]
> > > Sent: Monday, January 19, 2015 7:59 PM
> > > To: Krzysztof Opasiak
> > > Cc: ba...@ti.com; linux-usb@vger.kernel.org;
> > > gre...@linuxfoundation.org; bige...@breakpoint.cc;
> > > s.wa...@samsung.com; k.lewando...@samsung.com;
> > > m.szyprow...@samsung.com; andrze...@samsung.com
> > > Subject: Re: [PATCH] usb: gadget: composite: Provide list of
> > > registered functions
> > >
> > > On Mon, Jan 19, 2015 at 02:17:19PM +0100, Krzysztof Opasiak
> wrote:
> > > > Driver which provides implementation of some USB functions
> > > registers
> > > > usb_function_driver structure in composite framework.
> > > > Function drivers are identifed using registered name.
> > > >
> > > > When gadget is composed using configfs user must know what
> names
> > > has
> > > > been registered. If function is compiled as a module this
> > > information
> > > > can be found in modules.alias file. If function is compiled-
> in,
> > > there
> > > > is no way to discover what usb functions are available in
> > > currently
> > > > running kernel.
> > > >
> > > > Such situation is nothing new for linux kernel.
> > > > Similar situation is with file systems. While mounting user
> can
> > > > provide an fs type argument using -t option in mount.
> > > > Those type names are registered by drivers. To make those
> names
> > > > discoverable there is a /proc/filesystems which exports the
> list
> > > of
> > > > currently registered file systems.
> > > >
> > > > This patch adds /proc/usb-functions attribute which exports
> the
> > > list
> > > > of currently registered function drivers.
> > > > This allows user to discover list of both compiled-in
> functions
> > > and
> > > > from loaded kernel modules.
> > > >
> > > > Signed-off-by: Krzysztof Opasiak 
> > >
> > > you need to document the new file under Documentation/ABI/
> > >
> >
> > I have just sent v2 version with documentation.
> >
> > I have done some more research and it looks like /sys/kernel
> could be
> > a good alternative if you find that proc usage is not a good
> idea.
> > What do you thing? Should we continue with /proc/usb-functions or
> > replace this patch with /sys/kernel/usb-functions or maybe
> 
> why don't we place the file at the same directory as our configfs
> has been mounted ?

Default place for mounting configfs is /sys/kernel/config.
It is created in init function of configfs module. For example
systemd mounts there configfs on system startup.

So summing up, desired location should be /sys/kernel/usb-functions?

Please correct me if I misunderstood something.
I'll send suitable patches shortly.

-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opas...@samsung.com




--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: net2280 on iMX6

2015-01-23 Thread Ricardo Ribalda Delgado
Hello Tod

If you cc the linux-usb mailing list, there will be more chances to
get a proper answer :)

>From the error message it seems that the device did not have an irq,
which is needed by the driver. Take a loop to lspci -vvv to see if it
is true.

Unfortunately I have not access to that board to test it.

Regards and good luck :) !



On Fri, Jan 23, 2015 at 5:32 PM, Tord Andersson
 wrote:
> Hi Ricardo,
>
> I am trying to get a PLX3380 PCIe/USB3 adapter up and running on a Nitrogen
> Max board using a backported net2280 driver from the latest kernel to
> Boundary's 3.10.17 kernel.
>
> I get the following message which indicate that I have not configured my
> interrupts as I should. I get it both when using msi and disabling msi:
>
> ---
>
> PCI: enabling device :01:00.0 ( -> 0002)
>
> net2280 :01:00.0: usb_reset_338x: Defect 7374 FsmValue 0xf000
>
> net2280 :01:00.0: usb_reinit_338x: Defect 7374 FsmValue f000
>
> net2280 :01:00.0: No IRQ.  Check PCI setup!
>
> (NULL device *): gadget not registered.
>
> net2280 :01:00.0: unbind
>
> ---
>
> Do you know if anyone has been able to test this driver/HW combo with an
> iMX6 target or other ARM target?
> It works OK on a PC running the latest kernel, so the hardware should be OK
> :)
> Other PCIe cards such as WLAN cards works OK in Nitrogen Max.
>
> Best regards,
>
> Tord
>
> --
> __
>
> mail: tord.o.anders...@gmail.com
> pgp: F934 1A9B 5E2D 0563 49FE B8EB 0F53 851B E1FA 3E8B



-- 
Ricardo Ribalda
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: gadget: composite: Provide list of registered functions

2015-01-23 Thread Felipe Balbi
On Fri, Jan 23, 2015 at 05:09:07PM +0100, Krzysztof Opasiak wrote:
> 
> 
> > -Original Message-
> > From: Felipe Balbi [mailto:ba...@ti.com]
> > Sent: Monday, January 19, 2015 7:59 PM
> > To: Krzysztof Opasiak
> > Cc: ba...@ti.com; linux-usb@vger.kernel.org;
> > gre...@linuxfoundation.org; bige...@breakpoint.cc;
> > s.wa...@samsung.com; k.lewando...@samsung.com;
> > m.szyprow...@samsung.com; andrze...@samsung.com
> > Subject: Re: [PATCH] usb: gadget: composite: Provide list of
> > registered functions
> > 
> > On Mon, Jan 19, 2015 at 02:17:19PM +0100, Krzysztof Opasiak wrote:
> > > Driver which provides implementation of some USB functions
> > registers
> > > usb_function_driver structure in composite framework.
> > > Function drivers are identifed using registered name.
> > >
> > > When gadget is composed using configfs user must know what names
> > has
> > > been registered. If function is compiled as a module this
> > information
> > > can be found in modules.alias file. If function is compiled-in,
> > there
> > > is no way to discover what usb functions are available in
> > currently
> > > running kernel.
> > >
> > > Such situation is nothing new for linux kernel.
> > > Similar situation is with file systems. While mounting user can
> > > provide an fs type argument using -t option in mount.
> > > Those type names are registered by drivers. To make those names
> > > discoverable there is a /proc/filesystems which exports the list
> > of
> > > currently registered file systems.
> > >
> > > This patch adds /proc/usb-functions attribute which exports the
> > list
> > > of currently registered function drivers.
> > > This allows user to discover list of both compiled-in functions
> > and
> > > from loaded kernel modules.
> > >
> > > Signed-off-by: Krzysztof Opasiak 
> > 
> > you need to document the new file under Documentation/ABI/
> > 
> 
> I have just sent v2 version with documentation.
> 
> I have done some more research and it looks like /sys/kernel
> could be a good alternative if you find that proc usage is not
> a good idea. What do you thing? Should we continue with
> /proc/usb-functions or replace this patch with
> /sys/kernel/usb-functions or maybe

why don't we place the file at the same directory as our configfs has
been mounted ?

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-23 Thread Felipe Balbi
Hi,

On Fri, Jan 23, 2015 at 05:12:56PM +0200, Heikki Krogerus wrote:
> +int dwc3_ulpi_init(struct dwc3 *dwc)
> +{
> + u32 reg;
> +
> + /* First check USB2 PHY interface type */
> + switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3)) {
> + case DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI:
> + /* Select ULPI Interface */
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> + /* FALLTHROUGH */
> + case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
> + break;
> + default:
> + return 0;
> + }
> +
> + /* Register the interface */
> + dwc->ulpi = ulpi_register_interface(dwc->dev, &dwc3_ulpi);
> + if (IS_ERR(dwc->ulpi)) {

so, this will only build and link if DWC3_ULPI is enabled, in case of
error you return early...

> + dev_err(dwc->dev, "failed to register ULPI interface");
> + return PTR_ERR(dwc->ulpi);
> + }
> +
> + return 0;
> +}
> +
> +void dwc3_ulpi_exit(struct dwc3 *dwc)
> +{
> + if (dwc->ulpi) {

... looks like this branch is unnecessary.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v2 3/5] usb: dwc3: Add quirk for Synopsis device disconnection errata

2015-01-23 Thread Felipe Balbi
Hi,

On Wed, Jan 21, 2015 at 05:02:57PM +0800, Sneeker Yeh wrote:
> Hi Felipe:
> 
> Thanks for reviewing these,
> 
> 2015-01-19 22:50 GMT+08:00 Felipe Balbi :
> > Hi,
> >
> > On Mon, Jan 19, 2015 at 03:56:47PM +0800, Sneeker Yeh wrote:
> >> Synopsis Designware USB3 IP earlier than v3.00a which is configured in 
> >> silicon
> >> with DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, would need a specific quirk to 
> >> prevent
> >> xhci host controller from dying when device is disconnected.
> >>
> >> Since DWC_USB3_SUSPEND_ON_DISCONNECT_EN is an IP configuration whose state
> >> cannot be checked from software in runtime, it has to be enabled via 
> >> platform
> >> data or device tree.
> >>
> >> Signed-off-by: Sneeker Yeh 
> >> ---
> >>  Documentation/devicetree/bindings/usb/dwc3.txt |   17 +
> >>  drivers/usb/dwc3/core.c|6 ++
> >>  drivers/usb/dwc3/core.h|1 +
> >>  drivers/usb/dwc3/host.c|4 
> >>  drivers/usb/dwc3/platform_data.h   |1 +
> >>  5 files changed, 29 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
> >> b/Documentation/devicetree/bindings/usb/dwc3.txt
> >> index cd7f045..1b78b29 100644
> >> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> >> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> >> @@ -37,6 +37,23 @@ Optional properties:
> >>   - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal
> >>   utmi_l1_suspend_n, false when asserts utmi_sleep_n
> >>   - snps,hird-threshold: HIRD threshold
> >> + - snps,has_suspend_on_disconnect: true when IP is configured in silicon 
> >> with
> >> + DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, it can inject a
> >> + specific quirk to prevent xhci host controller from
> >> + dying when usb device is disconnected from root hub.
> >> + Since DWC_USB3_SUSPEND_ON_DISCONNECT_EN is an IP
> >> + configuration whose state cannot be checked from
> >> + software in runtime, it has to be enabled via 
> >> platform
> >> + data or device tree.
> >> +
> >> + xhci host dying symptom here is caused by that
> >> + DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1
> >> + configuration makes IP auto-suspended after PORTCSC 
> >> is
> >> + cleared when usb device detached, then an 
> >> asynchronous
> >> + disconnection procedure might fail using endpoint
> >> + command that suspened IP won't have any response to.
> >> +
> >> + this issue is fixed when IP version >= 3.00a.
> >>
> >>  This is usually a subnode to DWC3 glue to which it is connected.
> >>
> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> >> index 25ddc39..fbceab1 100644
> >> --- a/drivers/usb/dwc3/core.c
> >> +++ b/drivers/usb/dwc3/core.c
> >> @@ -838,6 +838,9 @@ static int dwc3_probe(struct platform_device *pdev)
> >>   "snps,tx_de_emphasis_quirk");
> >>   of_property_read_u8(node, "snps,tx_de_emphasis",
> >>   &tx_de_emphasis);
> >> +
> >> + dwc->suspend_on_disconnect_quirk = 
> >> of_property_read_bool(node,
> >> + "snps,has_suspend_on_disconnect");
> >>   } else if (pdata) {
> >>   dwc->maximum_speed = pdata->maximum_speed;
> >>   dwc->has_lpm_erratum = pdata->has_lpm_erratum;
> >> @@ -864,6 +867,9 @@ static int dwc3_probe(struct platform_device *pdev)
> >>   dwc->tx_de_emphasis_quirk = pdata->tx_de_emphasis_quirk;
> >>   if (pdata->tx_de_emphasis)
> >>   tx_de_emphasis = pdata->tx_de_emphasis;
> >> +
> >> + dwc->suspend_on_disconnect_quirk =
> >> + pdata->has_suspend_on_disconnect;
> >>   }
> >>
> >>   /* default to superspeed if no maximum_speed passed */
> >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> >> index 8090249..d7458ff 100644
> >> --- a/drivers/usb/dwc3/core.h
> >> +++ b/drivers/usb/dwc3/core.h
> >> @@ -832,6 +832,7 @@ struct dwc3 {
> >>
> >>   unsignedtx_de_emphasis_quirk:1;
> >>   unsignedtx_de_emphasis:2;
> >> + unsignedsuspend_on_disconnect_quirk:1;
> >
> > you're missing the comment on the structure and these should be
> > alphabetically sorted.
> 
> okay, I see, I'll change it like this:
> 
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -692,6 +692,9 @@ struct dwc3_scratchpad_array {
>   * @resize_fifos: tells us it's ok to reconfigure our TxFIFO sizes.
>   * @setup_packet_pending: true when there's a Setup Packet in FIFO. 
> Workaround
>   * @start_config_issued: true when StartConfig command has been issued
> + 

Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210

2015-01-23 Thread Felipe Balbi
On Fri, Jan 23, 2015 at 10:23:48AM +0200, Heikki Krogerus wrote:
> On Thu, Jan 22, 2015 at 02:51:24PM -0600, Felipe Balbi wrote:
> > On Wed, Jan 21, 2015 at 01:39:58PM +0200, Heikki Krogerus wrote:
> > > Hi,
> > > 
> > > On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> > > > On Tue, Jan 20, 2015 at 09:45:39AM -0600, Felipe Balbi wrote:
> > > > > > diff --git a/drivers/phy/ulpi/tusb1210.c 
> > > > > > b/drivers/phy/ulpi/tusb1210.c
> > > > > > new file mode 100644
> > > > > > index 000..ac77f98
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/phy/ulpi/tusb1210.c
> > > > > 
> > > > > do you really need this extra ulpi directory ?
> > > > > 
> > > > > I wonder if phy-tusb1210.c as a name would be enough.
> > > > 
> > > > IMO grouping the ULPI PHY drivers and other ULPI bus code into
> > > > separate folder from the start is the right thing to do.
> > > 
> > > A correction to this comment. I probable don't need this folder. Like
> > > you said, phy-tusb1210.c should be enough..
> > > 
> > > 
> > > 
> > > > > In fact, we might decide to add an entire ULPI bus, eventually, though
> > > > > I'm still considering if there's any benefit to that.
> > > > 
> > > > I don't think I understand this comment? ULPI bus is what I'm
> > > > introducing in this set (the first patch in it)?
> > > 
> > > ..I talked with Alex about this :). So I think the bus belongs under
> > > drivers/usb/core/ instead of driver/phy/. It's not really tied to the
> > > Generic PHY framework in any way, but ULPI is of course USB specific.
> > 
> > right, maybe drivers/usb/ulpi or maybe drivers/ulpi, and have
> > phy-tusb1201 register under that ulpi_bus_type instead of
> > platform_bus_type, but still use drivers/phy to register itself a phy
> > provider ;-)
> 
> So just for the record: This driver does not register under
> platform_bus_type bus but instead already under ulpi_bus_type.

hah! :-)

> I'll prepare new version out of these today and try to figure out
> proper place for the code (maybe drivers/usb/ulpi?).

drivers/phy is fine, it is a phy driver anyway... I just should've read
it to see there was a ulpi_bus_type :-)

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210

2015-01-23 Thread Felipe Balbi
On Fri, Jan 23, 2015 at 10:12:41AM +0200, Heikki Krogerus wrote:
> Hi,
> 
> On Thu, Jan 22, 2015 at 02:49:25PM -0600, Felipe Balbi wrote:
> > On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> > > > > + /* Store initial eye diagram optimisation value */
> > > > > + ret = ulpi_read(ulpi, ULPI_EXT_VENDOR_SPECIFIC);
> > > > 
> > > > do they *all* use this register for eye diagram optimization or is this
> > > > something that Intel decided to do ?
> > > > 
> > > > (sorry, don't know much about tusb1210 other than it sucks like hell :-)
> > > 
> > > All I know that somebody needs to save the value. The ones using this
> > > PHY who don't need to save it can most likely live without the driver.
> > 
> > right, but what I mean is: is it mandatory that Eye diagram
> > configuration be stored in *this* register? Or is it more like a scratch
> > register which Intel just happens to be using for Eye diagram data ?
> 
> The eye diagram tuning is in that register. Here's the spec:
> http://www.ti.com/lit/ds/symlink/tusb1210.pdf
> 
> I'll add definition for the register (which is colourfully named
> "VENDOR_SPECIFIC2").

alright, thanks.

> > > > > + if (ret < 0)
> > > > > + return ret;
> > > > > +
> > > > > + tusb->ctx[0] = ret;
> > > > > +
> > > > > + tusb->phy = ulpi_phy_create(ulpi, &phy_ops);
> > > > > + if (IS_ERR(tusb->phy))
> > > > > + return PTR_ERR(tusb->phy);
> > > > > +
> > > > > + tusb->ulpi = ulpi;
> > > > > +
> > > > > + phy_set_drvdata(tusb->phy, tusb);
> > > > > + dev_set_drvdata(&ulpi->dev, tusb);
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static void tusb1210_remove(struct ulpi *ulpi)
> > > > > +{
> > > > > + struct tusb1210 *tusb = dev_get_drvdata(&ulpi->dev);
> > > > 
> > > > completely unrelated to $subject, but we might want to have a
> > > > ulpi_{set,get}_drvdata() at some point.
> > > 
> > > Makes sense.
> > > 
> > > > In fact, we might decide to add an entire ULPI bus, eventually, though
> > > > I'm still considering if there's any benefit to that.
> > > 
> > > I don't think I understand this comment? ULPI bus is what I'm
> > > introducing in this set (the first patch in it)?
> > 
> > I mean introducing a real struct bus ulpi_bus_type :-) With match,
> > probe, remove, etc.
> 
> I'm already doing that. Please check the first patch in this set:
> "phy: add bus for USB ULPI PHYs".

yeah, sorry about that.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 0/5] extcon: usb: Introduce USB GPIO extcon driver. Fix DRA7 USB.

2015-01-23 Thread Felipe Balbi
On Fri, Jan 23, 2015 at 09:26:56AM +0200, Roger Quadros wrote:
> Felipe,
> 
> On 22/01/15 22:29, Felipe Balbi wrote:
> > On Thu, Jan 22, 2015 at 04:57:56PM +0200, Roger Quadros wrote:
> >>
> >> On 22/01/15 15:32, Roger Quadros wrote:
> >>> Felipe,
> >>>
> >>> On 20/01/15 21:02, Felipe Balbi wrote:
>  On Mon, Jan 19, 2015 at 07:52:17PM +0200, Roger Quadros wrote:
> > Hi,
> >
> > On DRA7 EVMs the USB ID pin is connected to a GPIO line. The USB drivers
> > (dwc3 + dwc3-omap) depend on extcon framework to get the USB cable state
> > (USB or USB-Host) to put the controller in the right mode.
> >
> > There were earlier attempts [1] to get this working by trying to patch 
> > up
> > the existing GPIO extcon driver.
> >
> > This series attemts to take a different approach by introducing a new
> > USB specific extcon driver to handle the USB ID GPIO pin and
> > interpret a right USB cable state.
> >
> > The reasoning to introduce this new driver is:
> > 1) The existing GPIO extcon driver doesn't understand USB cable states
> > and it can't handle more than one cable per instance.
> >
> > For the USB case we need to handle at least 2 cable states.
> > a) USB (attach/detach)
> > b) USB-Host (attach/detach)
> > and could possible include more states like
> > c) Fast-charger (attach/detach)
> > d) Slow-charger (attach/detach)
> > 
> > 2) This USB specific driver can be easily updated in the future to
> > handle VBUS events, or charger detect events, in case it happens
> > to be available on GPIO for any platform.
> >
> > 3) The DT implementation is very easy. You just need one extcon node 
> > per USB
> > instead of one extcon node per cable state as in case of [1].
> >
> > 4) The cable state string doesn't need to be encoded in the device tree
> > as in case of [1].
> >
> > 5) With only ID event available, you can simulate a USB-peripheral 
> > attach
> > when USB-Host is detacted instead of hacking the USB driver to do the 
> > same.
> >
> > Tested on DRA7-evm and DRA72-evm.
> 
>  while at that, you might want to patch X15 too.
> 
> >>> USB2 port is meant for peripheral use only. ID pin from USB port is not 
> >>> connected to GPIO.
> >>>
> >>
> >> OK answering myself here :).
> >> Peripheral mode doesn't work on x15-bb as the USB driver (dwc3-omap)
> >> doesn't set the mailbox correctly even when dwc3 node is set as otg = 
> >> "peripheral".
> >>
> >> Looks like we need to implement usb-gpio-extcon for x15 even though ID is 
> >> hard coded.
> > 
> > right, another option is to have dwc3-omap read the child's DTS to check
> > dr_mode and hardcode things based on that.
> > 
> I think that option is better as it doesn't require a GPIO line to be
> reserved for ID when USB is not meant for dual-role use.

yeah, let's implement that for 3.21.

-- 
balbi


signature.asc
Description: Digital signature


RE: [PATCH] usb: gadget: composite: Provide list of registered functions

2015-01-23 Thread Krzysztof Opasiak


> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Monday, January 19, 2015 7:59 PM
> To: Krzysztof Opasiak
> Cc: ba...@ti.com; linux-usb@vger.kernel.org;
> gre...@linuxfoundation.org; bige...@breakpoint.cc;
> s.wa...@samsung.com; k.lewando...@samsung.com;
> m.szyprow...@samsung.com; andrze...@samsung.com
> Subject: Re: [PATCH] usb: gadget: composite: Provide list of
> registered functions
> 
> On Mon, Jan 19, 2015 at 02:17:19PM +0100, Krzysztof Opasiak wrote:
> > Driver which provides implementation of some USB functions
> registers
> > usb_function_driver structure in composite framework.
> > Function drivers are identifed using registered name.
> >
> > When gadget is composed using configfs user must know what names
> has
> > been registered. If function is compiled as a module this
> information
> > can be found in modules.alias file. If function is compiled-in,
> there
> > is no way to discover what usb functions are available in
> currently
> > running kernel.
> >
> > Such situation is nothing new for linux kernel.
> > Similar situation is with file systems. While mounting user can
> > provide an fs type argument using -t option in mount.
> > Those type names are registered by drivers. To make those names
> > discoverable there is a /proc/filesystems which exports the list
> of
> > currently registered file systems.
> >
> > This patch adds /proc/usb-functions attribute which exports the
> list
> > of currently registered function drivers.
> > This allows user to discover list of both compiled-in functions
> and
> > from loaded kernel modules.
> >
> > Signed-off-by: Krzysztof Opasiak 
> 
> you need to document the new file under Documentation/ABI/
> 

I have just sent v2 version with documentation.

I have done some more research and it looks like /sys/kernel
could be a good alternative if you find that proc usage is not
a good idea. What do you thing? Should we continue with
/proc/usb-functions or replace this patch with
/sys/kernel/usb-functions or maybe
/sys/kernel/usb-gadget/usb-functions to have a directory for some
future attributes related to usb gadget?

-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opas...@samsung.com




--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 22/23] usbip: vhci_hcd: use HUB_CHAR_*

2015-01-23 Thread Valentina Manea
On Mon, Jan 19, 2015 at 1:00 AM, Sergei Shtylyov
 wrote:
> Fix  using the  bare number  to set the 'wHubCharacteristics' field of the Hub
> Descriptor while the values are #define'd in .
>
> Signed-off-by: Sergei Shtylyov 
>
> ---
>  drivers/usb/usbip/vhci_hcd.c |3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: usb/drivers/usb/usbip/vhci_hcd.c
> ===
> --- usb.orig/drivers/usb/usbip/vhci_hcd.c
> +++ usb/drivers/usb/usbip/vhci_hcd.c
> @@ -218,7 +218,8 @@ static inline void hub_descriptor(struct
> memset(desc, 0, sizeof(*desc));
> desc->bDescriptorType = 0x29;
> desc->bDescLength = 9;
> -   desc->wHubCharacteristics = (__constant_cpu_to_le16(0x0001));
> +   desc->wHubCharacteristics = __constant_cpu_to_le16(
> +   HUB_CHAR_INDV_PORT_LPSM | HUB_CHAR_COMMON_OCPM);
> desc->bNbrPorts = VHCI_NPORTS;
> desc->u.hs.DeviceRemovable[0] = 0xff;
> desc->u.hs.DeviceRemovable[1] = 0xff;
>

Looks good.

Thanks,
Valentina
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] usb: gadget: composite: Provide list of registered functions

2015-01-23 Thread Krzysztof Opasiak
Driver which provides implementation of some USB functions
registers usb_function_driver structure in composite framework.
Function drivers are identifed using registered name.

When gadget is composed using configfs user must know what
names has been registered. If function is compiled as a module
this information can be found in modules.alias file. If
function is compiled-in, there is no way to discover what
usb functions are available in currently running kernel.

Such situation is nothing new for linux kernel.
Similar situation is with file systems. While mounting user
can provide an fs type argument using -t option in mount.
Those type names are registered by drivers. To make those names
discoverable there is a /proc/filesystems which
exports the list of currently registered file systems.

This patch adds /proc/usb-functions attribute which exports
the list of currently registered function drivers.
This allows user to discover list of both compiled-in functions
and from loaded kernel modules.

Signed-off-by: Krzysztof Opasiak 
---
 Documentation/ABI/testing/procfs-usb-functions |   17 
 drivers/usb/gadget/configfs.c  |   53 
 drivers/usb/gadget/configfs.h  |2 +
 drivers/usb/gadget/functions.c |   13 ++
 4 files changed, 85 insertions(+)
 create mode 100644 Documentation/ABI/testing/procfs-usb-functions

diff --git a/Documentation/ABI/testing/procfs-usb-functions 
b/Documentation/ABI/testing/procfs-usb-functions
new file mode 100644
index 000..ff66382
--- /dev/null
+++ b/Documentation/ABI/testing/procfs-usb-functions
@@ -0,0 +1,17 @@
+What:  /proc/usb-functions
+Date:  January 2014
+Contact:   Krzysztof Opasiak 
+   linux-usb@vger.kernel.org
+Description:
+   The /proc/usb-functions file displays the list
+   of currently available usb functions. File contains
+   one line per each function. First column is
+   a name of function which has been registered in
+   composite framework. This name should be used
+   while creating instance of this function using
+   configfs composite gadget. For more details
+   please refer to
+  Documentation/usb/gadget_configfs.txt
+   All other columns should be ignored by any kind
+   of parsing tool.
+
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 7564814..275d7d0 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -5,6 +5,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "configfs.h"
 #include "u_f.h"
 #include "u_os_desc.h"
@@ -1558,13 +1559,63 @@ void unregister_gadget_item(struct config_item *item)
 }
 EXPORT_SYMBOL_GPL(unregister_gadget_item);
 
+#ifdef CONFIG_PROC_FS
+#define PROC_ATTR_NAME "usb-functions"
+
+static int functions_proc_open(struct inode *inode, struct file *file)
+{
+   return single_open(file, functions_proc_show, NULL);
+}
+
+static const struct file_operations functions_proc_fops = {
+   .owner  = THIS_MODULE,
+   .open   = functions_proc_open,
+   .read   = seq_read,
+   .llseek = seq_lseek,
+   .release= single_release,
+};
+
+static inline int proc_attr_init(void)
+{
+   struct proc_dir_entry *entry;
+
+   entry = proc_create(PROC_ATTR_NAME, 0, NULL, &functions_proc_fops);
+   return entry != NULL ? 0 : -EINVAL;
+}
+
+static inline void proc_attr_cleanup(void)
+{
+   remove_proc_entry(PROC_ATTR_NAME, NULL);
+}
+
+#else
+
+static inline int proc_attr_init(void)
+{
+   return 0;
+}
+
+static inline void proc_attr_cleanup(void)
+{
+   return;
+}
+
+#endif
+
 static int __init gadget_cfs_init(void)
 {
int ret;
 
config_group_init(&gadget_subsys.su_group);
 
+   ret = proc_attr_init();
+   if (ret)
+   return ret;
+
ret = configfs_register_subsystem(&gadget_subsys);
+   if (ret)
+   proc_attr_cleanup();
+
return ret;
 }
 module_init(gadget_cfs_init);
@@ -1572,5 +1623,7 @@ module_init(gadget_cfs_init);
 static void __exit gadget_cfs_exit(void)
 {
configfs_unregister_subsystem(&gadget_subsys);
+   proc_attr_cleanup();
 }
 module_exit(gadget_cfs_exit);
+
diff --git a/drivers/usb/gadget/configfs.h b/drivers/usb/gadget/configfs.h
index 36c468c..2af1e39 100644
--- a/drivers/usb/gadget/configfs.h
+++ b/drivers/usb/gadget/configfs.h
@@ -16,4 +16,6 @@ static inline struct usb_os_desc *to_usb_os_desc(struct 
config_item *item)
return container_of(to_config_group(item), struct usb_os_desc, group);
 }
 
+int functions_proc_show(struct seq_file *m, void *v);
+
 #endif /*  USB__GADGET__CONFIGFS__H */
diff --git a/drivers/usb/gadget/functions.c b/drivers/usb/gadget/functions.c
index b13f839..95f08a4 100644
--- a/drivers/usb/gadget/functions.c
+++ b/driver

[PATCH v2] usb: gadget: composite: Provide list of registered functions

2015-01-23 Thread Krzysztof Opasiak
This is a respin of previously discussed RFC patch[1].

*Why?*

USB gadget can be composed from userspace using ConfigFS. During
gadget composition user has to create at least one instance of
function. Instance is being created using

cd /usb-gadget/
mkdir functions/.

Where:
instance - unique name for instance of function of given type
type - name registered by USB function driver during it's module
init

Instance is a string which allow to identify instance if we have for
example two acm functions in one gadget. Type is a USB function driver
identifier registered by driver during module init. This string is
also a part of USB function module alias. User has to know that
functionfs module is identifed using ffs and source sink is identified
by SourceSink.

When function is compiled as a module user may read module.alias file
and get those names form module alias. When function is compiled-in
kernel it doesn't provide entry in module.alias file and currently
there is no way to learn what functions are available.

*Does similar problem exist in other place?*

Yes, similar problem is with file systems. Each module which provides
file system implementation registers some file system name which can
be used during mounting (-t option). User may also check the
module.alias file to find fs types available as modules. To discover
what fs types has been compiled-in kernel there is a /proc/filesystems
file which exports names of all fs types currently registered in
framework.

*How to solve this?*

This patch solves our problem by adding /proc/usb-functions file which
exports to userspace list of all currently registered USB function
drivers. This is similar solution as one used by file systems.

I'm aware that adding new files to proc is not very well welcome.
I have looked for some other more suitable location for this list
but didn't find any, so suggestions are very welcome.

Footnotes:
1 - http://www.spinics.net/lists/linux-usb/msg115872.html

--
Best regards,
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics

---
Changes since v1:
- Add documentation for new file

Krzysztof Opasiak (1):
  usb: gadget: composite: Provide list of registered functions

 drivers/usb/gadget/configfs.c  |   53 
 drivers/usb/gadget/configfs.h  |2 ++
 drivers/usb/gadget/functions.c |   13 ++
 3 files changed, 68 insertions(+)

-- 
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] USB: cdc-acm: check for descriptors with invalid length

2015-01-23 Thread Oliver Neukum
On Fri, 2015-01-23 at 23:02 +0800, Adam Lee wrote:
> On Fri, Jan 23, 2015 at 09:44:38AM +0100, Oliver Neukum wrote:
> > On Fri, 2015-01-23 at 14:54 +0800, Adam Lee wrote:
> > > In my scenario(pull that device then plug into another usb port while
> > > booting), invalid descriptor accessing happens just like Simon reported.
> > > Checking length and ignoring the invalid descriptors works.
> > 
> > Hi,
> > 
> > interesting. Have a look at what I sent Greg last week.
> > 
> > Regards
> > Oliver
> > 
> > From 14f823958cb3e99646419f743f3176d1059c3282 Mon Sep 17 00:00:00 2001
> > From: Oliver Neukum 
> > Date: Tue, 13 Jan 2015 16:55:52 +0100
> > Subject: [PATCH 1/4] cdc-acm: add sanity checks
> > 
> > Check the special CDC headers for a plausible minimum length.
> > Another big operating systems ignores such garbage.
> > 
> > Signed-off-by: Oliver Neukum 
> > ---
> >
> > ...
> >
> > case USB_CDC_ACM_TYPE:
> > +   if (elength < 3)
> > +   break;
> > ac_management_function = buffer[3];
> > break;
> > case USB_CDC_CALL_MANAGEMENT_TYPE:
> > +   if (elength < 4)
> > +   break;
> >   call_management_function = buffer[3];
> >   call_interface_num = buffer[4];
> >   break;
> >
> > ...
> >
> >  next_desc:
> > -   buflen -= buffer[0];
> > -   buffer += buffer[0];
> > +   buflen -= elength;
> > +   buffer += elength;
> > }
> >  
> > if (!union_header) {
> > -- 
> > 1.8.4.5
> 
> Hi, Oliver
> 
> Shouldn't the length checks be "if (elength < 4)" and "if (elength <
> 5)"? Think the logic of "buflen -= elength" and "buffer += elength",
> elength must count itself(buffer[0]) in.
> 

Yes, you are right. I am making a version for you to test.

Regards
Oliver

-- 
Oliver Neukum 

--
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 0/8] usb: ulpi bus

2015-01-23 Thread Heikki Krogerus

Heikki Krogerus (8):
  usb: add bus type for USB ULPI
  usb: dwc3: USB2 PHY register access bits
  usb: dwc3: store driver data earlier
  usb: dwc3: cache hwparams earlier
  usb: dwc3: ULPI or UTMI+ select
  usb: dwc3: add ULPI interface support
  phy: helpers for USB ULPI PHY registering
  phy: add driver for TI TUSB1210 ULPI PHY

 MAINTAINERS   |   7 ++
 drivers/phy/Kconfig   |   6 +
 drivers/phy/Makefile  |   1 +
 drivers/phy/phy-tusb1210.c| 133 
 drivers/phy/ulpi_phy.h|  31 +
 drivers/usb/common/Makefile   |   3 +-
 drivers/usb/common/ulpi.c | 253 ++
 drivers/usb/core/Kconfig  |   8 ++
 drivers/usb/dwc3/Kconfig  |   7 ++
 drivers/usb/dwc3/Makefile |   4 +
 drivers/usb/dwc3/core.c   |  12 +-
 drivers/usb/dwc3/core.h   |  22 
 drivers/usb/dwc3/ulpi.c   | 112 +
 include/linux/mod_devicetable.h   |   6 +
 include/linux/ulpi/driver.h   |  62 ++
 include/linux/ulpi/interface.h|  23 
 include/linux/ulpi/regs.h | 130 
 include/linux/usb/ulpi.h  | 134 +---
 scripts/mod/devicetable-offsets.c |   4 +
 scripts/mod/file2alias.c  |  13 ++
 20 files changed, 835 insertions(+), 136 deletions(-)
 create mode 100644 drivers/phy/phy-tusb1210.c
 create mode 100644 drivers/phy/ulpi_phy.h
 create mode 100644 drivers/usb/common/ulpi.c
 create mode 100644 drivers/usb/dwc3/ulpi.c
 create mode 100644 include/linux/ulpi/driver.h
 create mode 100644 include/linux/ulpi/interface.h
 create mode 100644 include/linux/ulpi/regs.h

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/8] usb: add bus type for USB ULPI

2015-01-23 Thread Heikki Krogerus
UTMI+ Low Pin Interface (ULPI) is a commonly used PHY
interface for USB 2.0. The ULPI specification describes a
standard set of registers which the vendors can extend for
their specific needs. ULPI PHYs provide often functions
such as charger detection and ADP sensing and probing.

There are two major issues that the bus type is meant to
tackle:

Firstly, ULPI registers are accessed from the controller.
The bus provides convenient method for the controller
drivers to share that access with the actual PHY drivers.

Secondly, there are already platforms that assume ULPI PHYs
are runtime detected, such as many Intel Baytrail based
platforms. They do not provide any kind of hardware
description for the ULPI PHYs like separate ACPI device
object that could be used to enumerate a device from.

Signed-off-by: Heikki Krogerus 
---
 MAINTAINERS   |   7 ++
 drivers/usb/common/Makefile   |   3 +-
 drivers/usb/common/ulpi.c | 253 ++
 drivers/usb/core/Kconfig  |   8 ++
 include/linux/mod_devicetable.h   |   6 +
 include/linux/ulpi/driver.h   |  62 ++
 include/linux/ulpi/interface.h|  23 
 include/linux/ulpi/regs.h | 130 
 include/linux/usb/ulpi.h  | 134 +---
 scripts/mod/devicetable-offsets.c |   4 +
 scripts/mod/file2alias.c  |  13 ++
 11 files changed, 510 insertions(+), 133 deletions(-)
 create mode 100644 drivers/usb/common/ulpi.c
 create mode 100644 include/linux/ulpi/driver.h
 create mode 100644 include/linux/ulpi/interface.h
 create mode 100644 include/linux/ulpi/regs.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 761a4a1..7d0c58c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10234,6 +10234,13 @@ S: Maintained
 F: Documentation/video4linux/zr364xx.txt
 F: drivers/media/usb/zr364xx/
 
+ULPI BUS
+M: Heikki Krogerus 
+L: linux-usb@vger.kernel.org
+S: Maintained
+F: drivers/usb/common/ulpi.c
+F: include/linux/ulpi/
+
 USER-MODE LINUX (UML)
 M: Jeff Dike 
 M: Richard Weinberger 
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index ca2f8bd..5791d6c 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -6,4 +6,5 @@ obj-$(CONFIG_USB_COMMON)  += usb-common.o
 usb-common-y += common.o
 usb-common-$(CONFIG_USB_LED_TRIG) += led.o
 
-obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
+obj-$(CONFIG_USB_OTG_FSM)  += usb-otg-fsm.o
+obj-$(CONFIG_USB_ULPI_BUS) += ulpi.o
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
new file mode 100644
index 000..3ed66be
--- /dev/null
+++ b/drivers/usb/common/ulpi.c
@@ -0,0 +1,253 @@
+/**
+ * ulpi.c - USB ULPI PHY bus
+ *
+ * Copyright (C) 2015 Intel Corporation
+ *
+ * Author: Heikki Krogerus 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* -- 
*/
+
+int ulpi_read(struct ulpi *ulpi, u8 addr)
+{
+   return ulpi->ops->read(ulpi->ops, addr);
+}
+EXPORT_SYMBOL_GPL(ulpi_read);
+
+int ulpi_write(struct ulpi *ulpi, u8 addr, u8 val)
+{
+   return ulpi->ops->write(ulpi->ops, addr, val);
+}
+EXPORT_SYMBOL_GPL(ulpi_write);
+
+/* -- 
*/
+
+static int ulpi_match(struct device *dev, struct device_driver *driver)
+{
+   struct ulpi_driver *drv = to_ulpi_driver(driver);
+   struct ulpi *ulpi = to_ulpi_dev(dev);
+   const struct ulpi_device_id *id;
+
+   for (id = drv->id_table; id->vendor; id++)
+   if (id->vendor == ulpi->id.vendor &&
+   id->product == ulpi->id.product)
+   return 1;
+
+   return 0;
+}
+
+static int ulpi_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+   struct ulpi *ulpi = to_ulpi_dev(dev);
+
+   if (add_uevent_var(env, "MODALIAS=ulpi:v%04xp%04x",
+  ulpi->id.vendor, ulpi->id.product))
+   return -ENOMEM;
+   return 0;
+}
+
+static int ulpi_probe(struct device *dev)
+{
+   struct ulpi_driver *drv = to_ulpi_driver(dev->driver);
+
+   return drv->probe(to_ulpi_dev(dev));
+}
+
+static int ulpi_remove(struct device *dev)
+{
+   struct ulpi_driver *drv = to_ulpi_driver(dev->driver);
+
+   if (drv->remove)
+   drv->remove(to_ulpi_dev(dev));
+
+   return 0;
+}
+
+struct bus_type ulpi_bus = {
+   .name = "ulpi",
+   .match = ulpi_match,
+   .uevent = ulpi_uevent,
+   .probe = ulpi_probe,
+   .remove = ulpi_remove,
+};
+EXPORT_SYMBOL_GPL(ulpi_bus);
+
+/* -- 
*/
+
+static ssize_t modalias_show(struct d

[PATCH 5/8] usb: dwc3: ULPI or UTMI+ select

2015-01-23 Thread Heikki Krogerus
Make selection between ULPI and UTMI+ interfaces possible by
providing definition for the bit in Global USB2 PHY
Configuration Register that controls it.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/dwc3/core.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index be1c20e..9b6f4c7 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -173,6 +173,7 @@
 /* Global USB2 PHY Configuration Register */
 #define DWC3_GUSB2PHYCFG_PHYSOFTRST(1 << 31)
 #define DWC3_GUSB2PHYCFG_SUSPHY(1 << 6)
+#define DWC3_GUSB2PHYCFG_ULPI_UTMI (1 << 4)
 
 /* Global USB2 PHY Vendor Control Register */
 #define DWC3_GUSB2PHYACC_NEWREGREQ (1 << 25)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 7/8] phy: helpers for USB ULPI PHY registering

2015-01-23 Thread Heikki Krogerus
ULPI PHYs need to be bound to their controllers with a
lookup. This adds helpers that the ULPI drivers can use to
do both, the registration of the PHY and the lookup, at the
same time.

Signed-off-by: Heikki Krogerus 
Cc: Kishon Vijay Abraham I 
---
 drivers/phy/ulpi_phy.h | 31 +++
 1 file changed, 31 insertions(+)
 create mode 100644 drivers/phy/ulpi_phy.h

diff --git a/drivers/phy/ulpi_phy.h b/drivers/phy/ulpi_phy.h
new file mode 100644
index 000..ac49fb6
--- /dev/null
+++ b/drivers/phy/ulpi_phy.h
@@ -0,0 +1,31 @@
+#include 
+
+/**
+ * Helper that registers PHY for a ULPI device and adds a lookup for binding it
+ * and it's controller, which is always the parent.
+ */
+static inline struct phy
+*ulpi_phy_create(struct ulpi *ulpi, struct phy_ops *ops)
+{
+   struct phy *phy;
+   int ret;
+
+   phy = phy_create(&ulpi->dev, NULL, ops);
+   if (IS_ERR(phy))
+   return phy;
+
+   ret = phy_create_lookup(phy, "usb2-phy", dev_name(ulpi->dev.parent));
+   if (ret) {
+   phy_destroy(phy);
+   return ERR_PTR(ret);
+   }
+
+   return phy;
+}
+
+/* Remove a PHY that was created with ulpi_phy_create() and it's lookup. */
+static inline void ulpi_phy_destroy(struct ulpi *ulpi, struct phy *phy)
+{
+   phy_remove_lookup(phy, "usb2-phy", dev_name(ulpi->dev.parent));
+   phy_destroy(phy);
+}
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/8] usb: dwc3: store driver data earlier

2015-01-23 Thread Heikki Krogerus
We need to store it before phys are handled, so we can later
use it in ULPI interface support code.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/dwc3/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 25ddc39..b06e88b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -876,12 +876,13 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->hird_threshold = hird_threshold
| (dwc->is_utmi_l1_suspend << 4);
 
+   platform_set_drvdata(pdev, dwc);
+
ret = dwc3_core_get_phy(dwc);
if (ret)
return ret;
 
spin_lock_init(&dwc->lock);
-   platform_set_drvdata(pdev, dwc);
 
if (!dev->dma_mask) {
dev->dma_mask = dev->parent->dma_mask;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/8] usb: dwc3: cache hwparams earlier

2015-01-23 Thread Heikki Krogerus
So they are available when ULPI interface support is added.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/dwc3/core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index b06e88b..a8c9062 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -877,6 +877,7 @@ static int dwc3_probe(struct platform_device *pdev)
| (dwc->is_utmi_l1_suspend << 4);
 
platform_set_drvdata(pdev, dwc);
+   dwc3_cache_hwparams(dwc);
 
ret = dwc3_core_get_phy(dwc);
if (ret)
@@ -894,8 +895,6 @@ static int dwc3_probe(struct platform_device *pdev)
pm_runtime_get_sync(dev);
pm_runtime_forbid(dev);
 
-   dwc3_cache_hwparams(dwc);
-
ret = dwc3_alloc_event_buffers(dwc, DWC3_EVENT_BUFFERS_SIZE);
if (ret) {
dev_err(dwc->dev, "failed to allocate event buffers\n");
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-23 Thread Heikki Krogerus
Registers DWC3's ULPI interface with the ULPI bus when it's
available.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/dwc3/Kconfig  |   7 +++
 drivers/usb/dwc3/Makefile |   4 ++
 drivers/usb/dwc3/core.c   |   6 +++
 drivers/usb/dwc3/core.h   |  13 ++
 drivers/usb/dwc3/ulpi.c   | 112 ++
 5 files changed, 142 insertions(+)
 create mode 100644 drivers/usb/dwc3/ulpi.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 58b5b2c..cda82ad 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -11,6 +11,13 @@ config USB_DWC3
 
 if USB_DWC3
 
+config USB_DWC3_ULPI
+   bool "Register ULPI PHY Interface"
+   depends on USB_ULPI_BUS=y
+   help
+ Select this if you have ULPI type PHY attached to your DWC3
+ controller.
+
 choice
bool "DWC3 Mode Selection"
default USB_DWC3_DUAL_ROLE if (USB && USB_GADGET)
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index bb34fbc..2fc44e0 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -16,6 +16,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) 
$(CONFIG_USB_DWC3_DUAL_ROLE)),)
dwc3-y  += gadget.o ep0.o
 endif
 
+ifneq ($(CONFIG_USB_DWC3_ULPI),)
+   dwc3-y  += ulpi.o
+endif
+
 ifneq ($(CONFIG_DEBUG_FS),)
dwc3-y  += debugfs.o
 endif
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index a8c9062..66cbf38 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, dwc);
dwc3_cache_hwparams(dwc);
 
+   ret = dwc3_ulpi_init(dwc);
+   if (ret)
+   return ret;
+
ret = dwc3_core_get_phy(dwc);
if (ret)
return ret;
@@ -965,6 +969,7 @@ err1:
 
 err0:
dwc3_free_event_buffers(dwc);
+   dwc3_ulpi_exit(dwc);
 
return ret;
 }
@@ -984,6 +989,7 @@ static int dwc3_remove(struct platform_device *pdev)
phy_power_off(dwc->usb3_generic_phy);
 
dwc3_core_exit(dwc);
+   dwc3_ulpi_exit(dwc);
 
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 9b6f4c7..ad6b371 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -748,6 +749,8 @@ struct dwc3 {
struct phy  *usb2_generic_phy;
struct phy  *usb3_generic_phy;
 
+   struct ulpi *ulpi;
+
void __iomem*regs;
size_t  regs_size;
 
@@ -1044,4 +1047,14 @@ static inline int dwc3_gadget_resume(struct dwc3 *dwc)
 }
 #endif /* !IS_ENABLED(CONFIG_USB_DWC3_HOST) */
 
+#if IS_ENABLED(CONFIG_USB_DWC3_ULPI)
+int dwc3_ulpi_init(struct dwc3 *dwc);
+void dwc3_ulpi_exit(struct dwc3 *dwc);
+#else
+static inline int dwc3_ulpi_init(struct dwc3 *dwc)
+{ return 0; }
+static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
+{ }
+#endif
+
 #endif /* __DRIVERS_USB_DWC3_CORE_H */
diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
new file mode 100644
index 000..8806981
--- /dev/null
+++ b/drivers/usb/dwc3/ulpi.c
@@ -0,0 +1,112 @@
+/**
+ * ulpi.c - DesignWare USB3 Controller's ULPI PHY interface
+ *
+ * Copyright (C) 2015 Intel Corporation
+ *
+ * Author: Heikki Krogerus 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+
+#include "core.h"
+#include "io.h"
+
+#define DWC3_ULPI_ADDR(a) \
+   ((a >= ULPI_EXT_VENDOR_SPECIFIC) ? \
+   DWC3_GUSB2PHYACC_ADDR(ULPI_ACCESS_EXTENDED) | \
+   DWC3_GUSB2PHYACC_EXTEND_ADDR(a) : DWC3_GUSB2PHYACC_ADDR(a))
+
+static int dwc3_ulpi_busyloop(struct dwc3 *dwc)
+{
+   unsigned count = 1000;
+   u32 reg;
+
+   while (count--) {
+   reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYACC(0));
+   if (!(reg & DWC3_GUSB2PHYACC_BUSY))
+   return 0;
+   cpu_relax();
+   }
+
+   return -ETIMEDOUT;
+}
+
+static int dwc3_ulpi_read(struct ulpi_ops *ops, u8 addr)
+{
+   struct dwc3 *dwc = dev_get_drvdata(ops->dev);
+   u32 reg;
+   int ret;
+
+   reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
+   dwc3_writel(dwc->regs, DWC3_GUSB2PHYACC(0), reg);
+
+   ret = dwc3_ulpi_busyloop(dwc);
+   if (ret)
+   return ret;
+
+   reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYACC(0));
+
+   return DWC3_GUSB2PHYACC_DATA(reg);
+}
+
+static int dwc3_ulpi_write(struct ulpi_ops *ops, u8 addr, u8 val)
+{
+   struct dwc3 *dwc = dev_get_drvdata(ops->dev);
+   u32 reg;
+   int ret;
+
+ 

[PATCH 2/8] usb: dwc3: USB2 PHY register access bits

2015-01-23 Thread Heikki Krogerus
Definitions for Global USB2 PHY Vendor Control Register
bits. We will need them to access ULPI PHY registers later.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/dwc3/core.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 0842aa8..be1c20e 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -174,6 +174,14 @@
 #define DWC3_GUSB2PHYCFG_PHYSOFTRST(1 << 31)
 #define DWC3_GUSB2PHYCFG_SUSPHY(1 << 6)
 
+/* Global USB2 PHY Vendor Control Register */
+#define DWC3_GUSB2PHYACC_NEWREGREQ (1 << 25)
+#define DWC3_GUSB2PHYACC_BUSY  (1 << 23)
+#define DWC3_GUSB2PHYACC_WRITE (1 << 22)
+#define DWC3_GUSB2PHYACC_ADDR(n)   (n << 16)
+#define DWC3_GUSB2PHYACC_EXTEND_ADDR(n)(n << 8)
+#define DWC3_GUSB2PHYACC_DATA(n)   (n & 0xff)
+
 /* Global USB3 PIPE Control Register */
 #define DWC3_GUSB3PIPECTL_PHYSOFTRST   (1 << 31)
 #define DWC3_GUSB3PIPECTL_U2SSINP3OK   (1 << 29)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY

2015-01-23 Thread Heikki Krogerus
TUSB1210 ULPI PHY has vendor specific register for eye
diagram tuning. On some platforms the system firmware has
set optimized value to it. In order to not loose the
optimized value, the driver stores it during probe and
restores it every time the PHY is powered back on.

Signed-off-by: Heikki Krogerus 
Cc: Kishon Vijay Abraham I 
---
 drivers/phy/Kconfig|   6 ++
 drivers/phy/Makefile   |   1 +
 drivers/phy/phy-tusb1210.c | 133 +
 3 files changed, 140 insertions(+)
 create mode 100644 drivers/phy/phy-tusb1210.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 26a7623..52ee720 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -284,4 +284,10 @@ config PHY_QCOM_UFS
help
  Support for UFS PHY on QCOM chipsets.
 
+config PHY_TUSB1210
+   tristate "TI TUSB1210 ULPI PHY module"
+   depends on USB_ULPI_BUS
+   help
+ Support for TI TUSB1210 USB ULPI PHY.
+
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index cfbb720..03f3d85 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -37,3 +37,4 @@ obj-$(CONFIG_PHY_STIH41X_USB) += phy-stih41x-usb.o
 obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o
 obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o
 obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o
+obj-$(CONFIG_PHY_TUSB1210) += phy-tusb1210.o
diff --git a/drivers/phy/phy-tusb1210.c b/drivers/phy/phy-tusb1210.c
new file mode 100644
index 000..1551ae8
--- /dev/null
+++ b/drivers/phy/phy-tusb1210.c
@@ -0,0 +1,133 @@
+/**
+ * tusb1210.c - TUSB1210 USB ULPI PHY driver
+ *
+ * Copyright (C) 2015 Intel Corporation
+ *
+ * Author: Heikki Krogerus 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include 
+#include 
+#include 
+
+#include "ulpi_phy.h"
+
+#define TUSB1210_VENDOR_SPECIFIC2 0x80
+
+struct tusb1210 {
+   struct ulpi *ulpi;
+   struct phy *phy;
+   struct gpio_desc *gpio_reset;
+   struct gpio_desc *gpio_cs;
+   u8 eye_diagram_tuning;
+};
+
+static int tusb1210_power_on(struct phy *phy)
+{
+   struct tusb1210 *tusb = phy_get_drvdata(phy);
+
+   gpiod_set_value_cansleep(tusb->gpio_reset, 1);
+   gpiod_set_value_cansleep(tusb->gpio_cs, 1);
+
+   /* Restore eye diagram optimisation value */
+   ulpi_write(tusb->ulpi, TUSB1210_VENDOR_SPECIFIC2,
+  tusb->eye_diagram_tuning);
+
+   return 0;
+}
+
+static int tusb1210_power_off(struct phy *phy)
+{
+   struct tusb1210 *tusb = phy_get_drvdata(phy);
+
+   gpiod_set_value_cansleep(tusb->gpio_reset, 0);
+   gpiod_set_value_cansleep(tusb->gpio_cs, 0);
+
+   return 0;
+}
+
+static struct phy_ops phy_ops = {
+   .power_on = tusb1210_power_on,
+   .power_off = tusb1210_power_off,
+   .init = tusb1210_power_on,
+   .exit = tusb1210_power_off,
+   .owner = THIS_MODULE,
+};
+
+static int tusb1210_probe(struct ulpi *ulpi)
+{
+   struct gpio_desc *gpio;
+   struct tusb1210 *tusb;
+   int ret;
+
+   tusb = devm_kzalloc(&ulpi->dev, sizeof(*tusb), GFP_KERNEL);
+   if (!tusb)
+   return -ENOMEM;
+
+   gpio = devm_gpiod_get(&ulpi->dev, "reset");
+   if (!IS_ERR(gpio)) {
+   ret = gpiod_direction_output(gpio, 0);
+   if (ret)
+   return ret;
+   tusb->gpio_reset = gpio;
+   }
+
+   gpio = devm_gpiod_get(&ulpi->dev, "cs");
+   if (!IS_ERR(gpio)) {
+   ret = gpiod_direction_output(gpio, 0);
+   if (ret)
+   return ret;
+   tusb->gpio_cs = gpio;
+   }
+
+   /* Store initial eye diagram optimisation value */
+   ret = ulpi_read(ulpi, TUSB1210_VENDOR_SPECIFIC2);
+   if (ret < 0)
+   return ret;
+
+   tusb->eye_diagram_tuning = ret;
+
+   tusb->phy = ulpi_phy_create(ulpi, &phy_ops);
+   if (IS_ERR(tusb->phy))
+   return PTR_ERR(tusb->phy);
+
+   tusb->ulpi = ulpi;
+
+   phy_set_drvdata(tusb->phy, tusb);
+   ulpi_set_drvdata(ulpi, tusb);
+   return 0;
+}
+
+static void tusb1210_remove(struct ulpi *ulpi)
+{
+   struct tusb1210 *tusb = ulpi_get_drvdata(ulpi);
+
+   ulpi_phy_destroy(ulpi, tusb->phy);
+}
+
+#define TI_VENDOR_ID 0x0451
+
+static const struct ulpi_device_id tusb1210_ulpi_id[] = {
+   { TI_VENDOR_ID, 0x1508, },
+   { },
+};
+MODULE_DEVICE_TABLE(ulpi, tusb1210_ulpi_id);
+
+static struct ulpi_driver tusb1210_driver = {
+   .id_table = tusb1210_ulpi_id,
+   .probe = tusb1210_probe,
+   .remove = tusb1210_remove,
+   .driver = {
+   .name = "tusb1210",
+   .owner = THIS_MODULE,
+   },
+};
+
+module_ulpi_driver(tusb1210_driver);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE(

Re: [PATCH] USB: cdc-acm: check for descriptors with invalid length

2015-01-23 Thread Adam Lee
On Fri, Jan 23, 2015 at 11:02:19PM +0800, Adam Lee wrote:
> On Fri, Jan 23, 2015 at 09:44:38AM +0100, Oliver Neukum wrote:
> > On Fri, 2015-01-23 at 14:54 +0800, Adam Lee wrote:
> > > In my scenario(pull that device then plug into another usb port while
> > > booting), invalid descriptor accessing happens just like Simon reported.
> > > Checking length and ignoring the invalid descriptors works.
> > 
> > Hi,
> > 
> > interesting. Have a look at what I sent Greg last week.
> > 
> > Regards
> > Oliver
> > 
> > From 14f823958cb3e99646419f743f3176d1059c3282 Mon Sep 17 00:00:00 2001
> > From: Oliver Neukum 
> > Date: Tue, 13 Jan 2015 16:55:52 +0100
> > Subject: [PATCH 1/4] cdc-acm: add sanity checks
> > 
> > Check the special CDC headers for a plausible minimum length.
> > Another big operating systems ignores such garbage.
> > 
> > Signed-off-by: Oliver Neukum 
> > ---
> >
> > ...
> >
> > case USB_CDC_ACM_TYPE:
> > +   if (elength < 3)
> > +   break;
> > ac_management_function = buffer[3];
> > break;
> > case USB_CDC_CALL_MANAGEMENT_TYPE:
> > +   if (elength < 4)
> > +   break;
> >   call_management_function = buffer[3];
> >   call_interface_num = buffer[4];
> >   break;
> >
> > ...
> >
> >  next_desc:
> > -   buflen -= buffer[0];
> > -   buffer += buffer[0];
> > +   buflen -= elength;
> > +   buffer += elength;
> > }
> >  
> > if (!union_header) {
> > -- 
> > 1.8.4.5
> 
> Hi, Oliver
> 
> Shouldn't the length checks be "if (elength < 4)" and "if (elength <
> 5)"? Think the logic of "buflen -= elength" and "buffer += elength",
> elength must count itself(buffer[0]) in.

Also I think you should "goto next_desc" but not "break".

-- 
Adam Lee
http://adam8157.info
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: cdc-acm: check for descriptors with invalid length

2015-01-23 Thread Adam Lee
On Fri, Jan 23, 2015 at 09:44:38AM +0100, Oliver Neukum wrote:
> On Fri, 2015-01-23 at 14:54 +0800, Adam Lee wrote:
> > In my scenario(pull that device then plug into another usb port while
> > booting), invalid descriptor accessing happens just like Simon reported.
> > Checking length and ignoring the invalid descriptors works.
> 
> Hi,
> 
> interesting. Have a look at what I sent Greg last week.
> 
>   Regards
>   Oliver
> 
> From 14f823958cb3e99646419f743f3176d1059c3282 Mon Sep 17 00:00:00 2001
> From: Oliver Neukum 
> Date: Tue, 13 Jan 2015 16:55:52 +0100
> Subject: [PATCH 1/4] cdc-acm: add sanity checks
> 
> Check the special CDC headers for a plausible minimum length.
> Another big operating systems ignores such garbage.
> 
> Signed-off-by: Oliver Neukum 
> ---
>
> ...
>
>   case USB_CDC_ACM_TYPE:
> + if (elength < 3)
> + break;
>   ac_management_function = buffer[3];
>   break;
>   case USB_CDC_CALL_MANAGEMENT_TYPE:
> + if (elength < 4)
> + break;
>   call_management_function = buffer[3];
>   call_interface_num = buffer[4];
>   break;
>
> ...
>
>  next_desc:
> - buflen -= buffer[0];
> - buffer += buffer[0];
> + buflen -= elength;
> + buffer += elength;
>   }
>  
>   if (!union_header) {
> -- 
> 1.8.4.5

Hi, Oliver

Shouldn't the length checks be "if (elength < 4)" and "if (elength <
5)"? Think the logic of "buflen -= elength" and "buffer += elength",
elength must count itself(buffer[0]) in.

-- 
Adam Lee
http://adam8157.info
--
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 3/4] mfd: dln2: add support for ACPI

2015-01-23 Thread Rafael J. Wysocki
On Friday, January 23, 2015 08:47:58 AM Octavian Purdila wrote:
> On Fri, Jan 23, 2015 at 12:09 AM, Rafael J. Wysocki  
> wrote:
> > On Thursday, January 22, 2015 12:13:13 PM Octavian Purdila wrote:
> >> On Thu, Jan 22, 2015 at 4:00 AM, Rafael J. Wysocki  
> >> wrote:
> 
> >> The idea here is to set the companion for the MFD sub-devices. Mika's
> >> commit "mfd: Add ACPI support" propagates the parent's companion to
> >> the MFD sub-devices, so it is sufficient to set the ACPI companion to
> >> the USB device.
> >
> > For the USB device itself you'll then end up with an incomplete binding (you
> > can't get back to the USB device from the ACPI object), so no, it isn't
> > sufficient.
> >
> >> Then, the companion will be propagated to the sub-devices after which
> >> acpi_bind_one() will be called for the sub-devices from
> >> mfd_add_devices (via platform_device_add -> device_add ->
> >> platform_notify).
> >
> > In fact, your use case doesn't seem to cover any of the use cases that
> > the Mika's commit addressed.  Namely, your parent device doesn't have
> > an ACPI companion to start with and you want your MFD cells to be bound
> > to the "DLN2000" thing.  That's why you're setting the ACPI companion
> > for the USB device, isn't it?
> >
> >> It is true that the USB dev will have its ACPI companion set without
> >> having acpi_bind_one called but I do not see any harm in that. Even
> >> though acpi_unbind_one() is called it will not find the USB dev on the
> >> physical node list so no put_device() imbalance is caused.
> >
> > Well, there are places where it matters.  Some links in sysfs will be 
> > missing
> > for one example.  Also please see the changelog of commit 52870786ff5d 
> > (ACPI:
> > Use ACPI companion to match only the first physical device).
> >
> > Bottom line: You really should be using acpi_bind_one() here and
> > acpi_unbind_one() on disconnect if you have to.
> >
> 
> OK, I understand now.
> 
> >> > And no, "Let's come up with a patch that sort of works, throw it at the 
> >> > maintainers
> >> > and see what happens" is not an acceptable approach, sorry.
> >>
> >> This patch is based on your feedback of the previous RFC patch set:
> >
> > Oh, is it?  I can't recall advising you to use request_firmware() for
> > uploading ACPI tables or some other questionable things that the patch is
> > doing.
> >
> > And if it still was an RFC, that wouldn't be a problem, but if you just send
> > non-RFC patches out, that means you want people to merge them.  This is bad
> > if the patches in question are not in a good enough shape and this one 
> > isn't.
> >
> 
> Yes, this should have been tagged with RFC, sorry about that.
> 
> > Now, why is this a bad idea to load ACPI tables from a driver using
> > request_firmware()?  Because those tables are not device firmare.  They are
> > not firmware that you load into a device to make it work (which then works
> > with all instances of the given hardware), they are part of system 
> > configuration
> > information and have to match what's there in the system.  For instance, if 
> > you
> > ship your example SSDT with a general-purpose distro, it may just not match 
> > the
> > hardware configuration of systems that people will try to use it with.
> >
> > So, while it is sort of OK to look up "DLN2000" and bind the USB device to
> > that by hand (although this looks ugly to me), it is not OK to load a random
> > custom SSDT if it is missing.
> >
> > We need to add a generic mechanism for loading custom SSDTs not present in 
> > the
> > firmware image and maybe even to load them on demand, but that cannot 
> > happen in
> > an ad-hoc way.  So the entire table loading-unloading code in your patch 
> > can go
> > away for now and you need to fail probing if the "DLN2000" ACPI object is 
> > not
> > present.
> >
> 
> That sounds interesting, I like the idea of a generic mechanism for
> loading custom SSDTs. Do you have any initial thoughts/pointers for
> starting that?

Thoughts - yes, pointers - not so much.  I'll get back to you when I'm done
with the e-mail backlog from the last few weeks.

> Thanks for the review and feedback.

No problem.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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: Boot regression on Versatile Express TC2 after commit b2b49ccbdd

2015-01-23 Thread Rafael J. Wysocki
On Friday, January 23, 2015 01:40:46 PM Pawel Moll wrote:
> On Thu, 2015-01-22 at 21:11 +, Rafael J. Wysocki wrote:
> > > I'll work on it when I have some spare time.  Of course, it would be 
> > > better if somebody could add the proper wakeup-detect code to the 
> > > isp1760-hcd driver...
> > 
> > Yes, it would.
> 
> Whether it's a hack or no - I don't know, but it makes my board detect
> USB devices again with v3.19-rc5, without any extra kernel parameters:
> 
> 8<---
> diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c
> index 395649f..92a8297 100644
> --- a/drivers/usb/host/isp1760-hcd.c
> +++ b/drivers/usb/host/isp1760-hcd.c
> @@ -2250,7 +2250,6 @@ struct usb_hcd *isp1760_register(phys_addr_t res_start, 
> resource_size_t res_len,
>   ret = usb_add_hcd(hcd, irq, irqflags);
>   if (ret)
>   goto err_unmap;
> - device_wakeup_enable(hcd->self.controller);
>  
>   return hcd;
> 8<---

Makes sense to me.  It is not useful to enable features that are not working ...

Rafael

--
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: generic: Do not fail when 'reset-gpios' is not used

2015-01-23 Thread Fabio Estevam
On Fri, Jan 23, 2015 at 2:12 AM, Fabio Estevam  wrote:

> Ok, I managed to test this on a imx51_babbage that uses reset_gpios.
> Without this patch USB is broken due to e9f2cefb0cdc2ae, but if I
> apply my patch I get a hang:
>
> [1.392824] ci_hdrc ci_hdrc.1: doesn't support gadget
> [1.397975] ci_hdrc ci_hdrc.1: EHCI Host Controller
> [1.403205] ci_hdrc ci_hdrc.1: new USB bus registered, assigned bus number 
> 1
> [1.422335] ci_hdrc ci_hdrc.1: USB 2.0 started, EHCI 1.00
> [1.432962] hub 1-0:1.0: USB hub found
> [1.437119] hub 1-0:1.0: 1 port detected
>
> Will need to debug more.

The problem here is that:

nop->gpiod_reset = devm_gpiod_get(dev, "reset-gpios");

Always return -2.

On imx51-babbage.dts we pass 'reset-gpios' and it used to retrieve it
fine prior to e9f2cefb0cdc2ae ("usb: phy: generic: migrate to
gpio_desc") by using of_get_named_gpio_flags().

Any ideas as to why devm_gpiod_get() always fail now?

I am running linux-next 20150123.

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


Re: Boot regression on Versatile Express TC2 after commit b2b49ccbdd

2015-01-23 Thread Pawel Moll
On Thu, 2015-01-22 at 21:11 +, Rafael J. Wysocki wrote:
> > I'll work on it when I have some spare time.  Of course, it would be 
> > better if somebody could add the proper wakeup-detect code to the 
> > isp1760-hcd driver...
> 
> Yes, it would.

Whether it's a hack or no - I don't know, but it makes my board detect
USB devices again with v3.19-rc5, without any extra kernel parameters:

8<---
diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c
index 395649f..92a8297 100644
--- a/drivers/usb/host/isp1760-hcd.c
+++ b/drivers/usb/host/isp1760-hcd.c
@@ -2250,7 +2250,6 @@ struct usb_hcd *isp1760_register(phys_addr_t res_start, 
resource_size_t res_len,
ret = usb_add_hcd(hcd, irq, irqflags);
if (ret)
goto err_unmap;
-   device_wakeup_enable(hcd->self.controller);
 
return hcd;
8<---

Care to give it a go, Nico?

Paweł



--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] usb: gadget: ffs: add eventfd notification about ffs events

2015-01-23 Thread Robert Baldyga
Add eventfd which notifies userspace about ep0 events and AIO completion
events. It simplifies using of FunctionFS with event loop, because now
we need to poll on single file (instead of polling on ep0 and eventfd's
supplied to AIO layer).

FunctionFS eventfd is not triggered if another eventfd is supplied to
AIO layer (in AIO request). It can be useful, for example, when we want
to handle AIO transations for chosen endpoint in separate thread.

Signed-off-by: Robert Baldyga 
Acked-by: Michal Nazarewicz 
---

Changelog:

v2:
- Fixed comment from Michal Nazarewicz

v1: https://lkml.org/lkml/2015/1/23/142

 drivers/usb/gadget/function/f_fs.c  | 29 -
 drivers/usb/gadget/function/u_fs.h  |  1 +
 include/uapi/linux/usb/functionfs.h |  1 +
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index e78a2c6..0eb441c 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "u_fs.h"
 #include "u_f.h"
@@ -153,6 +154,8 @@ struct ffs_io_data {
 
struct usb_ep *ep;
struct usb_request *req;
+
+   struct ffs_data *ffs;
 };
 
 struct ffs_desc_helper {
@@ -674,6 +677,9 @@ static void ffs_user_copy_worker(struct work_struct *work)
 
aio_complete(io_data->kiocb, ret, ret);
 
+   if (io_data->ffs->ffs_eventfd && !io_data->kiocb->ki_eventfd)
+   eventfd_signal(io_data->ffs->ffs_eventfd, 1);
+
usb_ep_free_request(io_data->ep, io_data->req);
 
io_data->kiocb->private = NULL;
@@ -827,6 +833,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
io_data->buf = data;
io_data->ep = ep->ep;
io_data->req = req;
+   io_data->ffs = epfile->ffs;
 
req->context  = io_data;
req->complete = ffs_epfile_async_io_complete;
@@ -1510,6 +1517,9 @@ static void ffs_data_clear(struct ffs_data *ffs)
if (ffs->epfiles)
ffs_epfiles_destroy(ffs->epfiles, ffs->eps_count);
 
+   if (ffs->ffs_eventfd)
+   eventfd_ctx_put(ffs->ffs_eventfd);
+
kfree(ffs->raw_descs_data);
kfree(ffs->raw_strings);
kfree(ffs->stringtabs);
@@ -2169,7 +2179,8 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
  FUNCTIONFS_HAS_HS_DESC |
  FUNCTIONFS_HAS_SS_DESC |
  FUNCTIONFS_HAS_MS_OS_DESC |
- FUNCTIONFS_VIRTUAL_ADDR)) {
+ FUNCTIONFS_VIRTUAL_ADDR |
+ FUNCTIONFS_EVENTFD)) {
ret = -ENOSYS;
goto error;
}
@@ -2180,6 +2191,20 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
goto error;
}
 
+   if (flags & FUNCTIONFS_EVENTFD) {
+   if (len < 4)
+   goto error;
+   ffs->ffs_eventfd =
+   eventfd_ctx_fdget((int)get_unaligned_le32(data));
+   if (IS_ERR(ffs->ffs_eventfd)) {
+   ret = PTR_ERR(ffs->ffs_eventfd);
+   ffs->ffs_eventfd = NULL;
+   goto error;
+   }
+   data += 4;
+   len  -= 4;
+   }
+
/* Read fs_count, hs_count and ss_count (if present) */
for (i = 0; i < 3; ++i) {
if (!(flags & (1 << i))) {
@@ -2454,6 +2479,8 @@ static void __ffs_event_add(struct ffs_data *ffs,
pr_vdebug("adding event %d\n", type);
ffs->ev.types[ffs->ev.count++] = type;
wake_up_locked(&ffs->ev.waitq);
+   if (ffs->ffs_eventfd)
+   eventfd_signal(ffs->ffs_eventfd, 1);
 }
 
 static void ffs_event_add(struct ffs_data *ffs,
diff --git a/drivers/usb/gadget/function/u_fs.h 
b/drivers/usb/gadget/function/u_fs.h
index 284a1f0..6013985 100644
--- a/drivers/usb/gadget/function/u_fs.h
+++ b/drivers/usb/gadget/function/u_fs.h
@@ -272,6 +272,7 @@ struct ffs_data {
kgid_t  gid;
}   file_perms;
 
+   struct eventfd_ctx *ffs_eventfd;
bool no_disconnect;
struct work_struct reset_work;
 
diff --git a/include/uapi/linux/usb/functionfs.h 
b/include/uapi/linux/usb/functionfs.h
index 295ba29..108dd79 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -20,6 +20,7 @@ enum functionfs_flags {
FUNCTIONFS_HAS_SS_DESC = 4,
FUNCTIONFS_HAS_MS_OS_DESC = 8,
FUNCTIONFS_VIRTUAL_ADDR = 16,
+   FUNCTIONFS_EVENTFD = 32,
 };
 
 /* Descriptor of an non-audio endpoint */
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a messa

Re: [PATCH] usb: gadget: ffs: add eventfd notification about ffs events

2015-01-23 Thread Robert Baldyga
On 01/23/2015 01:21 PM, Michał Nazarewicz wrote:
> 23 sty 2015 10:28 "Robert Baldyga"  > napisał(a):
>> Add eventfd which notifies userspace about ep0 events and AIO completion
>> events. It simplifies using of FunctionFS with event loop, because now
>> we need to poll on single file (instead of polling on ep0 and eventfd's
>> supplied to AIO layer).
>>
>> FunctionFS eventfd is not triggered if another eventfd is supplied to
>> AIO layer (in AIO request). It can be useful, for example, when we want
>> to handle AIO transations for chosen endpoint in separate thread.
>>
>> Signed-off-by: Robert Baldyga
> 
> I don't know much about eventfd interface bit from ffs point of view:
> 
> Acked-by: Michal Nazarewicz mailto:min...@mina86.com>>
> 
>> @@ -2180,6 +2191,20 @@ static int __ffs_data_got_descs(struct ffs_data
> *ffs,
>> goto error;
>> }
>>
>> +   if (flags & FUNCTIONFS_EVENTFD) {
>> +   if (len < 4)
>> +   goto error;
>> +   ffs->ffs_eventfd =
>> +   eventfd_ctx_fdget((int)get_unaligned_le32(data));
>> +   if (IS_ERR(ffs->ffs_eventfd)) {
>> +   ffs->ffs_eventfd = NULL;
>> +   ret = PTR_ERR(ffs->ffs_eventfd);
> 
> Need to swap those lines.
> 
>> +   goto error;
>> +   }
>> +   data += 4;
>> +   len  -= 4;
>> +   }
>> +
>> /* Read fs_count, hs_count and ss_count (if present) */
>> for (i = 0; i < 3; ++i) {
>> if (!(flags & (1 << i))) {
> 

Good catch, thanks!

Best regards,
Robert Baldyga
--
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: EHCI/xHCI port switching issue

2015-01-23 Thread Mathias Nyman
Hi

On 23.01.2015 13:02, Adrian-Ken Rueegsegger wrote:
> Hello,
> 
> We run Linux as a VM on top of the Muen Separation Kernel [1] where we
> have implemented PCI device passthrough using VT-d.
> 
> I have come across an issue where USB ports of an assigned xHCI
> controller are no longer being enabled in recent kernels (tested with
> v3.18.3 and v3.19-rc5) while it is working in v3.11. I did a bisect and
> identified commit 26b7679 ("Intel xhci: refactor EHCI/xHCI port
> switching") as the cause. Upon closer inspection the relevant part of
> the change seems to be in drivers/usb/host/pci-quirks.c:
> 
> void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev)
> {
> ...
> /* make sure an intel EHCI controller exists */
> for_each_pci_dev(companion) {
> if (companion->class == PCI_CLASS_SERIAL_USB_EHCI &&
> companion->vendor == PCI_VENDOR_ID_INTEL) {
> ehci_found = true;
> break;
> }
> }
> 
> if (!ehci_found)
> return;
> ...
> }
> 
> In our test case we only grant Linux access to the xHCI but *not* the
> EHCI controller. Since ehci_found is never true in this case the xHCI
> ports will not be enabled. Commenting out the return statement results
> in working xHCI ports.
> 
> Is there a different way to decide whether xHCI ports should be enabled
> instead of looking for a companion EHCI controller or must the xHCI and
> EHCI controllers always be accessible at the same time?
> 
> I suspect that the issue is also relevant for other hypervisors that
> allow PCI passthrough in a similar fashion.
> 

We don't really need to access the ehci for doing the portswitching.
Checking that ehci exists is a way to check if switching ports from ehci to xhci
makes sense at all. 

But we didn't consider a case like this.

I don't know what will happend if we try to write to xhci portswitching 
registers
on platforms with xhci only, so just removing the ehci check is a bit risky 
considering future platforms.

I need to ask around internally and see if we can detect the portswitching 
feature from
inside xhci, (digging around its pci config space before loading the driver), 
or if there's
any harm in always trying to do the portswitch on intel xhci controllers.

Switching the ports means your host os might lose all the ehci usb2 ports

-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: [PATCHv6 2/5] USB: gadget: atmel_usba_udc: Request an auto disabled Vbus signal IRQ instead of an auto enabled IRQ request followed by IRQ disable

2015-01-23 Thread Jean-Christophe PLAGNIOL-VILLARD

> On Jan 23, 2015, at 12:56 AM, Sylvain Rochet  
> wrote:
> 
> Vbus IRQ handler needs a started UDC driver to work because it uses
> udc->driver, which is set by the UDC start handler. The previous way
> chosen was to return from interrupt if udc->driver is NULL using a
> spinlock around the check.
> 
> We now request an auto disabled (IRQ_NOAUTOEN) Vbus signal IRQ instead
> of an auto enabled IRQ followed by disable_irq(). This way we remove the
> very small timeslot of enabled IRQ which existed previously between
> request() and disable(). We don't need anymore to check if udc->driver
> is NULL in IRQ handler.
> 
> Signed-off-by: Sylvain Rochet 
> Suggested-by: Boris Brezillon 
> Acked-by: Boris Brezillon 
> ---
> drivers/usb/gadget/udc/atmel_usba_udc.c | 9 ++---
> 1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
> b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index 40e5fc7..f9f305f 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -1745,10 +1745,6 @@ static irqreturn_t usba_vbus_irq(int irq, void *devid)
> 
>   spin_lock(&udc->lock);
> 
> - /* May happen if Vbus pin toggles during probe() */
> - if (!udc->driver)
> - goto out;
> -
>   vbus = vbus_is_present(udc);
>   if (vbus != udc->vbus_prev) {
>   if (vbus) {
> @@ -1769,7 +1765,6 @@ static irqreturn_t usba_vbus_irq(int irq, void *devid)
>   udc->vbus_prev = vbus;
>   }
> 
> -out:
>   spin_unlock(&udc->lock);
> 
>   return IRQ_HANDLED;
> @@ -2109,6 +2104,8 @@ static int usba_udc_probe(struct platform_device *pdev)
> 
>   if (gpio_is_valid(udc->vbus_pin)) {
>   if (!devm_gpio_request(&pdev->dev, udc->vbus_pin, 
> "atmel_usba_udc")) {
> + irq_set_status_flags(gpio_to_irq(udc->vbus_pin),
> + IRQ_NOAUTOEN);

not happy about still using the broken gpio_to_irq API

Linus can pass the IRQ via board file?

and via DT we can use IRQ directly

Best Regards,
J.
>   ret = devm_request_irq(&pdev->dev,
>   gpio_to_irq(udc->vbus_pin),
>   usba_vbus_irq, 0,
> @@ -2118,8 +2115,6 @@ static int usba_udc_probe(struct platform_device *pdev)
>   dev_warn(&udc->pdev->dev,
>"failed to request vbus irq; "
>"assuming always on\n");
> - } else {
> - disable_irq(gpio_to_irq(udc->vbus_pin));
>   }
>   } else {
>   /* gpio_request fail so use -EINVAL for gpio_is_valid */
> -- 
> 2.1.4
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
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: XHCI, "brain-dead scanner", and microframe rounding

2015-01-23 Thread Mathias Nyman
On 22.01.2015 21:39, Gunter Königsmann wrote:
> Seems like this time we are nearly there:
>  - the scanner appears in lsusb
>Bus 001 Device 005: ID 04f2:b33c Chicony Electronics Co., Ltd
>Bus 001 Device 006: ID 0489:e056 Foxconn / Hon Hai
>Bus 001 Device 002: ID 8087:0024 Intel Corp. Integrated Rate
> Matching Hub
>Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>Bus 003 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
>Bus 002 Device 002: ID 04f3:0011 Elan Microelectronics Corp.
>Bus 002 Device 003: ID 04b8:0133 Seiko Epson Corp. GT-1500 [GT-D1000]
>Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>  - The scanner is detected by sane-find-scanner
>  - I can select it in xSane
>  - but then the xSane window no more reacts for about a minute and
> then I get the message: "Failed to open device
> 'epgowa:interpreter:002:003': Error during device I/O.
> 
> Kind regards,
> 

I think something went wrong when cloning the tree/branch, it shows old debug 
output like:
"Endpoint 0x81 not halted, refusing to reset."

Which shouldn't exists anymore.
Are you sure you tried the ep_reset_halt_test branch?

git log should look like this:
https://git.kernel.org/cgit/linux/kernel/git/mnyman/xhci.git/log/?h=ep_reset_halt_test

-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


EHCI/xHCI port switching issue

2015-01-23 Thread Adrian-Ken Rueegsegger
Hello,

We run Linux as a VM on top of the Muen Separation Kernel [1] where we
have implemented PCI device passthrough using VT-d.

I have come across an issue where USB ports of an assigned xHCI
controller are no longer being enabled in recent kernels (tested with
v3.18.3 and v3.19-rc5) while it is working in v3.11. I did a bisect and
identified commit 26b7679 ("Intel xhci: refactor EHCI/xHCI port
switching") as the cause. Upon closer inspection the relevant part of
the change seems to be in drivers/usb/host/pci-quirks.c:

void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev)
{
...
/* make sure an intel EHCI controller exists */
for_each_pci_dev(companion) {
if (companion->class == PCI_CLASS_SERIAL_USB_EHCI &&
companion->vendor == PCI_VENDOR_ID_INTEL) {
ehci_found = true;
break;
}
}

if (!ehci_found)
return;
...
}

In our test case we only grant Linux access to the xHCI but *not* the
EHCI controller. Since ehci_found is never true in this case the xHCI
ports will not be enabled. Commenting out the return statement results
in working xHCI ports.

Is there a different way to decide whether xHCI ports should be enabled
instead of looking for a companion EHCI controller or must the xHCI and
EHCI controllers always be accessible at the same time?

I suspect that the issue is also relevant for other hypervisors that
allow PCI passthrough in a similar fashion.

Note: I am not subscribed to the linux-usb mailing list, please keep me
as CC.

Regards,
Adrian

[1] - http://muen.codelabs.ch/
--
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: Add OTG PET device to TPL

2015-01-23 Thread Sergei Shtylyov

Hello.

On 1/23/2015 9:39 AM, Macpaul Lin wrote:


OTG device shall support this device for allowing compliance automated testing.
The modification is derived from Pavankumar and Vijayavardhans' previous work.



Signed-off-by: Macpaul Lin 
Cc: Pavankumar Kondeti 
Cc: Vijayavardhan Vennapusa 
---
  drivers/usb/core/otg_whitelist.h | 5 +
  drivers/usb/core/quirks.c| 4 
  2 files changed, 9 insertions(+)



diff --git a/drivers/usb/core/otg_whitelist.h b/drivers/usb/core/otg_whitelist.h
index de0c9c9..a6315ab 100644
--- a/drivers/usb/core/otg_whitelist.h
+++ b/drivers/usb/core/otg_whitelist.h
@@ -55,6 +55,11 @@ static int is_targeted(struct usb_device *dev)
 le16_to_cpu(dev->descriptor.idProduct) == 0xbadd))
return 0;

+   /* OTG PET device is always targeted (see OTG 2.0 ECN 6.4.2) */
+   if ((le16_to_cpu(dev->descriptor.idVendor) == 0x1a0a &&
+le16_to_cpu(dev->descriptor.idProduct) == 0x0200))


   Why double parens?

[...]

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: gadget: zero: fix format string warnings

2015-01-23 Thread Amit Virdi

On 1/21/2015 2:36 PM, Asaf Vertz wrote:

Fixed the following warnings (reported by cppcheck):
[drivers/usb/gadget/function/f_sourcesink.c:1217]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1261]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1305]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1349]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1393]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1437]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1476]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1520]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1564]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1608]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'

Signed-off-by: Asaf Vertz
---
  drivers/usb/gadget/function/f_sourcesink.c |   20 ++--
  1 files changed, 10 insertions(+), 10 deletions(-)


Reviewed-by: Amit Virdi 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v3 1/3] usb: udc: add usb_gadget_find_udc

2015-01-23 Thread Peter Chen

 
> >
> > > > > An idea just struck me...  Instead of looping through all the
> > > > > udc's to find the right one, why not simply store a pointer to
> > > > > the udc in struct usb_gadget?
> > > > >
> > >
> > > We still have code to find usb_gadget_driver to iterate udc_list, it
> > > is better to change all, we need to have solution to consolidate
> > > struct usb_udc, struct usb_gadget, and struct usb_gadget_drver.
> > > I have no good solution now.
> >
> > I think it's okay to iterate through the list to find a
> > usb_gadget_driver.  But it's silly to iterate to find the usb_udc
> > corresponding to a usb_gadget.
> 
> Felipe, what's your opinion?
> 

Hi Felipe, would you have comments for it?

- Follow Alan's suggestion
- Adding a new API like this patch
- Drop this patch, and still iterate udc_list to get usb_gadget

Peter
 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 00/13] usb: second series of updates for dwc2 driver

2015-01-23 Thread Robert Baldyga
Hi,

On 01/21/2015 03:37 PM, Mian Yousaf Kaukab wrote:
> Hi,
> This patchset consists of some bug fixes, feature enhancements and
> cosmetic changes for the dwc2 driver. All the patches are verified on
> dwc2 v3.0a with dedicated fifos. Main focus of testing was with dma
> enabled. Although basic testing without dma was also done.
> 
> This is based on testing/next branch in Felipe's git.
> 
> Removed Paul Zimmerman from CC as his Synopsys address is not valid
> any longer.
> 
> Thank you,
> 
> Best regards,
> Yousaf
> 

[for gadget part]
Tested-by: Robert Baldyga 

Thanks,
Robert Baldyga

> History:
> v1:
>  - Fixed comments from Sergei Shtylyov and Robert Baldyga
> 
> Gregory Herrero (8):
>   usb: dwc2: host: register hcd handle to the phy
>   usb: dwc2: host: resume root hub on remote wakeup
>   usb: dwc2: gadget: fix clear halt feature handling
>   usb: dwc2: gadget: add TEST_MODE feature support
>   usb: dwc2: gadget: fix a typo in comment
>   usb: dwc2: gadget: add reset flag in init function
>   usb: dwc2: gadget: don't modify pullup status during reset
>   usb: dwc2: gadget: initialize controller in pullup callback
> 
> Mian Yousaf Kaukab (5):
>   usb: dwc2: gadget: remove hardcoded if (0) and if (1) checks
>   usb: dwc2: gadget: add unaligned buffers support
>   usb: dwc2: gadget: fix debug message for zlp
>   usb: dwc2: gadget: fix phy interface configuration
>   usb: dwc2: gadget: replace constants with defines
> 
>  drivers/usb/dwc2/core.h   |  12 +-
>  drivers/usb/dwc2/gadget.c | 359 
> ++
>  drivers/usb/dwc2/hcd.c|  43 --
>  drivers/usb/dwc2/hw.h |   1 +
>  4 files changed, 349 insertions(+), 66 deletions(-)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv6 2/5] USB: gadget: atmel_usba_udc: Request an auto disabled Vbus signal IRQ instead of an auto enabled IRQ request followed by IRQ disable

2015-01-23 Thread Sylvain Rochet
Hello,

On Fri, Jan 23, 2015 at 09:59:58AM +0100, Nicolas Ferre wrote:
> Le 23/01/2015 08:43, Jean-Christophe PLAGNIOL-VILLARD a écrit :
> > > On Jan 23, 2015, at 12:56 AM, Sylvain Rochet 
> > >  wrote:
> > > + irq_set_status_flags(gpio_to_irq(udc->vbus_pin),
> > > + IRQ_NOAUTOEN);
> > 
> > not happy about still using the broken gpio_to_irq API
> > 
> > Linus can pass the IRQ via board file?
> > 
> > and via DT we can use IRQ directly

Although I agree on the general principle, this driver is also used by 
non-DT boards.

I have concerns at least for at32ap700x-based boards which use this 
driver and are non-DT. The suggested change requires changing platform 
data of at32ap700x in such a way we may break it in my opinion.


> Absolutely not the topic of this patch series.
> Maybe the subject of another patch later?

Indeed. This series is only about PM support, harmless rework, or 
necessary rework for PM. Changing the way we acquire the IRQ is not 
harmless, this should be checked carefully for DT and particularly 
non-DT boards.


Sylvain
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] usb: gadget: ffs: add eventfd notification about ffs events

2015-01-23 Thread Robert Baldyga
Add eventfd which notifies userspace about ep0 events and AIO completion
events. It simplifies using of FunctionFS with event loop, because now
we need to poll on single file (instead of polling on ep0 and eventfd's
supplied to AIO layer).

FunctionFS eventfd is not triggered if another eventfd is supplied to
AIO layer (in AIO request). It can be useful, for example, when we want
to handle AIO transations for chosen endpoint in separate thread.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_fs.c  | 29 -
 drivers/usb/gadget/function/u_fs.h  |  1 +
 include/uapi/linux/usb/functionfs.h |  1 +
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index e78a2c6..dc7f816 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "u_fs.h"
 #include "u_f.h"
@@ -153,6 +154,8 @@ struct ffs_io_data {
 
struct usb_ep *ep;
struct usb_request *req;
+
+   struct ffs_data *ffs;
 };
 
 struct ffs_desc_helper {
@@ -674,6 +677,9 @@ static void ffs_user_copy_worker(struct work_struct *work)
 
aio_complete(io_data->kiocb, ret, ret);
 
+   if (io_data->ffs->ffs_eventfd && !io_data->kiocb->ki_eventfd)
+   eventfd_signal(io_data->ffs->ffs_eventfd, 1);
+
usb_ep_free_request(io_data->ep, io_data->req);
 
io_data->kiocb->private = NULL;
@@ -827,6 +833,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
io_data->buf = data;
io_data->ep = ep->ep;
io_data->req = req;
+   io_data->ffs = epfile->ffs;
 
req->context  = io_data;
req->complete = ffs_epfile_async_io_complete;
@@ -1510,6 +1517,9 @@ static void ffs_data_clear(struct ffs_data *ffs)
if (ffs->epfiles)
ffs_epfiles_destroy(ffs->epfiles, ffs->eps_count);
 
+   if (ffs->ffs_eventfd)
+   eventfd_ctx_put(ffs->ffs_eventfd);
+
kfree(ffs->raw_descs_data);
kfree(ffs->raw_strings);
kfree(ffs->stringtabs);
@@ -2169,7 +2179,8 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
  FUNCTIONFS_HAS_HS_DESC |
  FUNCTIONFS_HAS_SS_DESC |
  FUNCTIONFS_HAS_MS_OS_DESC |
- FUNCTIONFS_VIRTUAL_ADDR)) {
+ FUNCTIONFS_VIRTUAL_ADDR |
+ FUNCTIONFS_EVENTFD)) {
ret = -ENOSYS;
goto error;
}
@@ -2180,6 +2191,20 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
goto error;
}
 
+   if (flags & FUNCTIONFS_EVENTFD) {
+   if (len < 4)
+   goto error;
+   ffs->ffs_eventfd =
+   eventfd_ctx_fdget((int)get_unaligned_le32(data));
+   if (IS_ERR(ffs->ffs_eventfd)) {
+   ffs->ffs_eventfd = NULL;
+   ret = PTR_ERR(ffs->ffs_eventfd);
+   goto error;
+   }
+   data += 4;
+   len  -= 4;
+   }
+
/* Read fs_count, hs_count and ss_count (if present) */
for (i = 0; i < 3; ++i) {
if (!(flags & (1 << i))) {
@@ -2454,6 +2479,8 @@ static void __ffs_event_add(struct ffs_data *ffs,
pr_vdebug("adding event %d\n", type);
ffs->ev.types[ffs->ev.count++] = type;
wake_up_locked(&ffs->ev.waitq);
+   if (ffs->ffs_eventfd)
+   eventfd_signal(ffs->ffs_eventfd, 1);
 }
 
 static void ffs_event_add(struct ffs_data *ffs,
diff --git a/drivers/usb/gadget/function/u_fs.h 
b/drivers/usb/gadget/function/u_fs.h
index 284a1f0..6013985 100644
--- a/drivers/usb/gadget/function/u_fs.h
+++ b/drivers/usb/gadget/function/u_fs.h
@@ -272,6 +272,7 @@ struct ffs_data {
kgid_t  gid;
}   file_perms;
 
+   struct eventfd_ctx *ffs_eventfd;
bool no_disconnect;
struct work_struct reset_work;
 
diff --git a/include/uapi/linux/usb/functionfs.h 
b/include/uapi/linux/usb/functionfs.h
index 295ba29..108dd79 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -20,6 +20,7 @@ enum functionfs_flags {
FUNCTIONFS_HAS_SS_DESC = 4,
FUNCTIONFS_HAS_MS_OS_DESC = 8,
FUNCTIONFS_VIRTUAL_ADDR = 16,
+   FUNCTIONFS_EVENTFD = 32,
 };
 
 /* Descriptor of an non-audio endpoint */
-- 
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


Seagate Expansion external drive, still problems

2015-01-23 Thread prasnik
Hi. The device in question is the external usb hdd Seagate Expansion 1TB.
It claims to be usb 3.0 and also to be usable on usb 2.0 systems.
Vendor and device ids are 0bc2 and 2312.

I have tested it on the fresh 3.18.3 from kernel.org, on a laptop with usb 2.0
(I guess).

According to [1] and [2], I simply plugged in the device without stating
any quirk or (as suggested in [3]) delay_use option for usb-storage under
/etc/modprobe.d/. The result is that the disk gets never recognized by the 
system,
as better showed in the (bottom part of the) attached dmesg output.

I also tried by stating 'quirks' and 'delay_use' as suggested in [2] and [3],
in every combination, also on older kernels, obtaining the same result.

Note: the disk (0bc2:2312) discussed in [1] is 500GB while mine is 1TB..
Verbose versions of lspci and lsusb are attached.

Cheers

[1] https://bugzilla.kernel.org/show_bug.cgi?id=79511
[2] http://permalink.gmane.org/gmane.linux.scsi/94584
[3] https://bbs.archlinux.org/viewtopic.php?id=183190

lspci -nn,

00:00.0 Host bridge [0600]: Intel Corporation Mobile PM965/GM965/GL960 Memory 
Controller Hub [8086:2a00] (rev 03)
00:02.0 VGA compatible controller [0300]: Intel Corporation Mobile GM965/GL960 
Integrated Graphics Controller (primary) [8086:2a02] (rev 03)
00:02.1 Display controller [0380]: Intel Corporation Mobile GM965/GL960 
Integrated Graphics Controller (secondary) [8086:2a03] (rev 03)
00:1a.0 USB controller [0c03]: Intel Corporation 82801H (ICH8 Family) USB UHCI 
Controller #4 [8086:2834] (rev 04)
00:1a.1 USB controller [0c03]: Intel Corporation 82801H (ICH8 Family) USB UHCI 
Controller #5 [8086:2835] (rev 04)
00:1a.7 USB controller [0c03]: Intel Corporation 82801H (ICH8 Family) USB2 EHCI 
Controller #2 [8086:283a] (rev 04)
00:1b.0 Audio device [0403]: Intel Corporation 82801H (ICH8 Family) HD Audio 
Controller [8086:284b] (rev 04)
00:1c.0 PCI bridge [0604]: Intel Corporation 82801H (ICH8 Family) PCI Express 
Port 1 [8086:283f] (rev 04)
00:1c.1 PCI bridge [0604]: Intel Corporation 82801H (ICH8 Family) PCI Express 
Port 2 [8086:2841] (rev 04)
00:1c.2 PCI bridge [0604]: Intel Corporation 82801H (ICH8 Family) PCI Express 
Port 3 [8086:2843] (rev 04)
00:1c.3 PCI bridge [0604]: Intel Corporation 82801H (ICH8 Family) PCI Express 
Port 4 [8086:2845] (rev 04)
00:1d.0 USB controller [0c03]: Intel Corporation 82801H (ICH8 Family) USB UHCI 
Controller #1 [8086:2830] (rev 04)
00:1d.1 USB controller [0c03]: Intel Corporation 82801H (ICH8 Family) USB UHCI 
Controller #2 [8086:2831] (rev 04)
00:1d.2 USB controller [0c03]: Intel Corporation 82801H (ICH8 Family) USB UHCI 
Controller #3 [8086:2832] (rev 04)
00:1d.7 USB controller [0c03]: Intel Corporation 82801H (ICH8 Family) USB2 EHCI 
Controller #1 [8086:2836] (rev 04)
00:1e.0 PCI bridge [0604]: Intel Corporation 82801 Mobile PCI Bridge 
[8086:2448] (rev f4)
00:1f.0 ISA bridge [0601]: Intel Corporation 82801HM (ICH8M) LPC Interface 
Controller [8086:2815] (rev 04)
00:1f.1 IDE interface [0101]: Intel Corporation 82801HM/HEM (ICH8M/ICH8M-E) IDE 
Controller [8086:2850] (rev 04)
00:1f.2 SATA controller [0106]: Intel Corporation 82801HM/HEM (ICH8M/ICH8M-E) 
SATA Controller [AHCI mode] [8086:2829] (rev 04)
01:01.0 CardBus bridge [0607]: Ricoh Co Ltd RL5c476 II [1180:0476] (rev b4)
01:01.1 SD Host controller [0805]: Ricoh Co Ltd R5C822 SD/SDIO/MMC/MS/MSPro 
Host Adapter [1180:0822] (rev 18)
01:01.2 System peripheral [0880]: Ricoh Co Ltd R5C592 Memory Stick Bus Host 
Adapter [1180:0592] (rev 09)
01:07.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. 
RTL-8139/8139C/8139C+ [10ec:8139] (rev 10)
04:00.0 Network controller [0280]: Atheros Communications Inc. AR928X Wireless 
Network Adapter (PCI-Express) [168c:002a] (rev 01)

lsusb,

Bus 004 Device 002: ID 064e:a116 Suyin Corp. 
Bus 001 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 004 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 006 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 007 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub

[0.00] Initializing cgroup subsys cpuset
[0.00] Initializing cgroup subsys cpu
[0.00] Initializing cgroup subsys cpuacct
[0.00] Linux version 3.18.3 (m@debian) (gcc version 4.7.2 (Debian 
4.7.2-5) ) #1 SMP Wed Jan 21 09:53:29 PST 2015
[0.00] e820: BIOS-provided physical RAM map:
[0.00] BIOS-e820: [mem 0x-0x0009fbff] usable
[0.00] BIOS-e820: [mem 0x0009fc00-0x0009] reserved
[0.00] BIOS-e820: [mem 0x000e4000-0x000f] reserved
[0.00] BIOS-e820: [mem 0x0010-0xbf79] usable
[0.00] BIOS-e820: [mem 0xbf7a-0xbf7adfff] ACPI data
[0.00] BIOS-e820: [mem 0xbf7

Re: [PATCHv6 2/5] USB: gadget: atmel_usba_udc: Request an auto disabled Vbus signal IRQ instead of an auto enabled IRQ request followed by IRQ disable

2015-01-23 Thread Nicolas Ferre
Le 23/01/2015 08:43, Jean-Christophe PLAGNIOL-VILLARD a écrit :
> 
>> On Jan 23, 2015, at 12:56 AM, Sylvain Rochet  
>> wrote:
>>
>> Vbus IRQ handler needs a started UDC driver to work because it uses
>> udc->driver, which is set by the UDC start handler. The previous way
>> chosen was to return from interrupt if udc->driver is NULL using a
>> spinlock around the check.
>>
>> We now request an auto disabled (IRQ_NOAUTOEN) Vbus signal IRQ instead
>> of an auto enabled IRQ followed by disable_irq(). This way we remove the
>> very small timeslot of enabled IRQ which existed previously between
>> request() and disable(). We don't need anymore to check if udc->driver
>> is NULL in IRQ handler.
>>
>> Signed-off-by: Sylvain Rochet 
>> Suggested-by: Boris Brezillon 
>> Acked-by: Boris Brezillon 
>> ---
>> drivers/usb/gadget/udc/atmel_usba_udc.c | 9 ++---
>> 1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
>> b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> index 40e5fc7..f9f305f 100644
>> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
>> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> @@ -1745,10 +1745,6 @@ static irqreturn_t usba_vbus_irq(int irq, void *devid)
>>
>>  spin_lock(&udc->lock);
>>
>> -/* May happen if Vbus pin toggles during probe() */
>> -if (!udc->driver)
>> -goto out;
>> -
>>  vbus = vbus_is_present(udc);
>>  if (vbus != udc->vbus_prev) {
>>  if (vbus) {
>> @@ -1769,7 +1765,6 @@ static irqreturn_t usba_vbus_irq(int irq, void *devid)
>>  udc->vbus_prev = vbus;
>>  }
>>
>> -out:
>>  spin_unlock(&udc->lock);
>>
>>  return IRQ_HANDLED;
>> @@ -2109,6 +2104,8 @@ static int usba_udc_probe(struct platform_device *pdev)
>>
>>  if (gpio_is_valid(udc->vbus_pin)) {
>>  if (!devm_gpio_request(&pdev->dev, udc->vbus_pin, 
>> "atmel_usba_udc")) {
>> +irq_set_status_flags(gpio_to_irq(udc->vbus_pin),
>> +IRQ_NOAUTOEN);
> 
> not happy about still using the broken gpio_to_irq API
> 
> Linus can pass the IRQ via board file?
> 
> and via DT we can use IRQ directly

Absolutely not the topic of this patch series.
Maybe the subject of another patch later?

Best regards,


>>  ret = devm_request_irq(&pdev->dev,
>>  gpio_to_irq(udc->vbus_pin),
>>  usba_vbus_irq, 0,
>> @@ -2118,8 +2115,6 @@ static int usba_udc_probe(struct platform_device *pdev)
>>  dev_warn(&udc->pdev->dev,
>>   "failed to request vbus irq; "
>>   "assuming always on\n");
>> -} else {
>> -disable_irq(gpio_to_irq(udc->vbus_pin));
>>  }
>>  } else {
>>  /* gpio_request fail so use -EINVAL for gpio_is_valid */
>> -- 
>> 2.1.4
>>
>>
>> ___
>> linux-arm-kernel mailing list
>> linux-arm-ker...@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 


-- 
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: cdc-acm: check for descriptors with invalid length

2015-01-23 Thread Oliver Neukum
On Fri, 2015-01-23 at 14:54 +0800, Adam Lee wrote:
> In my scenario(pull that device then plug into another usb port while
> booting), invalid descriptor accessing happens just like Simon reported.
> Checking length and ignoring the invalid descriptors works.

Hi,

interesting. Have a look at what I sent Greg last week.

Regards
Oliver

>From 14f823958cb3e99646419f743f3176d1059c3282 Mon Sep 17 00:00:00 2001
From: Oliver Neukum 
Date: Tue, 13 Jan 2015 16:55:52 +0100
Subject: [PATCH 1/4] cdc-acm: add sanity checks

Check the special CDC headers for a plausible minimum length.
Another big operating systems ignores such garbage.

Signed-off-by: Oliver Neukum 
---
 drivers/usb/class/cdc-acm.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 546a17e8..8eadb38 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1091,6 +1091,7 @@ static int acm_probe(struct usb_interface *intf,
unsigned long quirks;
int num_rx_buf;
int i;
+   unsigned int elength = 0;
int combined_interfaces = 0;
struct device *tty_dev;
int rv = -ENOMEM;
@@ -1136,9 +1137,12 @@ static int acm_probe(struct usb_interface *intf,
dev_err(&intf->dev, "skipping garbage\n");
goto next_desc;
}
+   elength = buffer[0];
 
switch (buffer[2]) {
case USB_CDC_UNION_TYPE: /* we've found it */
+   if (elength < sizeof(struct usb_cdc_union_desc))
+   break; 
if (union_header) {
dev_err(&intf->dev, "More than one "
"union descriptor, skipping ...\n");
@@ -1147,14 +1151,20 @@ static int acm_probe(struct usb_interface *intf,
union_header = (struct usb_cdc_union_desc *)buffer;
break;
case USB_CDC_COUNTRY_TYPE: /* export through sysfs*/
+   if (elength < sizeof(struct 
usb_cdc_country_functional_desc))
+   break;
cfd = (struct usb_cdc_country_functional_desc *)buffer;
break;
case USB_CDC_HEADER_TYPE: /* maybe check version */
break; /* for now we ignore it */
case USB_CDC_ACM_TYPE:
+   if (elength < 3)
+   break;
ac_management_function = buffer[3];
break;
case USB_CDC_CALL_MANAGEMENT_TYPE:
+   if (elength < 4)
+   break;
call_management_function = buffer[3];
call_interface_num = buffer[4];
break;
@@ -1163,13 +1173,13 @@ static int acm_probe(struct usb_interface *intf,
 * could legitimately be found here.
 */
dev_dbg(&intf->dev, "Ignoring descriptor: "
-   "type %02x, length %d\n",
-   buffer[2], buffer[0]);
+   "type %02x, length %ud\n",
+   buffer[2], elength);
break;
}
 next_desc:
-   buflen -= buffer[0];
-   buffer += buffer[0];
+   buflen -= elength;
+   buffer += elength;
}
 
if (!union_header) {
-- 
1.8.4.5


-- 
Oliver Neukum 

--
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 3/3] phy: ulpi: add driver for TI TUSB1210

2015-01-23 Thread Heikki Krogerus
On Thu, Jan 22, 2015 at 02:51:24PM -0600, Felipe Balbi wrote:
> On Wed, Jan 21, 2015 at 01:39:58PM +0200, Heikki Krogerus wrote:
> > Hi,
> > 
> > On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> > > On Tue, Jan 20, 2015 at 09:45:39AM -0600, Felipe Balbi wrote:
> > > > > diff --git a/drivers/phy/ulpi/tusb1210.c b/drivers/phy/ulpi/tusb1210.c
> > > > > new file mode 100644
> > > > > index 000..ac77f98
> > > > > --- /dev/null
> > > > > +++ b/drivers/phy/ulpi/tusb1210.c
> > > > 
> > > > do you really need this extra ulpi directory ?
> > > > 
> > > > I wonder if phy-tusb1210.c as a name would be enough.
> > > 
> > > IMO grouping the ULPI PHY drivers and other ULPI bus code into
> > > separate folder from the start is the right thing to do.
> > 
> > A correction to this comment. I probable don't need this folder. Like
> > you said, phy-tusb1210.c should be enough..
> > 
> > 
> > 
> > > > In fact, we might decide to add an entire ULPI bus, eventually, though
> > > > I'm still considering if there's any benefit to that.
> > > 
> > > I don't think I understand this comment? ULPI bus is what I'm
> > > introducing in this set (the first patch in it)?
> > 
> > ..I talked with Alex about this :). So I think the bus belongs under
> > drivers/usb/core/ instead of driver/phy/. It's not really tied to the
> > Generic PHY framework in any way, but ULPI is of course USB specific.
> 
> right, maybe drivers/usb/ulpi or maybe drivers/ulpi, and have
> phy-tusb1201 register under that ulpi_bus_type instead of
> platform_bus_type, but still use drivers/phy to register itself a phy
> provider ;-)

So just for the record: This driver does not register under
platform_bus_type bus but instead already under ulpi_bus_type.

I'll prepare new version out of these today and try to figure out
proper place for the code (maybe drivers/usb/ulpi?).


Cheers,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] usb: dwc2: Use platform endianness when accessing registers

2015-01-23 Thread Vincent Pelletier
On Fri, 23 Jan 2015 02:38:37 +, John Youn 
wrote:
> Having the __raw functions everywhere is not pretty and probably not a
> good idea either.
> 
> I would rather have a dwc2_writel/dwc2_readl (like in dwc3), and we
> can figure out what needs to happen in there to support your platform.
> 
> As for readl/writel or the __raw equivalents, I am not sure. I haven't
> run the in-kernel driver on a big endian system so I don't know what
> the issue is. But I suspect there may be consequences to other
> platforms if we simply change this to __raw_readl/__raw_writel.

(focussing on readl, but the following also applies to writel)

readl is defined as __le32_to_cpu(__raw_readl(...)), so using readl on
little endian is equivalent to __raw_readl, as __le32_to_cpu is a no-op.

On big endian, it causes problems because __le32_to_cpu swaps bytes
around, causing GSNPSID to be read as 0xnn2n544f instead of expected
0x4f542nnn - and likewise for all other registers.

An earlier version (sent to linux-usb a week ago with not enough CC,
but I cannot find it in archives now) defined dwc2_readl as 
  le32_to_cpu(readl(...))
which is equivalent to
  le32_to_cpu(le32_to_cpu(__raw_readl(...)))
and which, on big-endian, makes le32_to_cpu(le32_to_cpu()) a no-op and
(ignoring possible compiler optimisation) swaps bytes twice per call.
On little endian, it is still equivalent to __raw_readl(...).

As for the prettiness of calling double-underscore functions,
speaking for myself I'm not used to Linux development enough to tell.
My python developer reflex tell me it's not how it should be.
OTOH there is no "raw_readl" or so alias, and other USB drivers
reference __raw_readl (example as of v3.14.28):
linux/drivers/usb$ git grep -l __raw_readl
gadget/at91_udc.c
gadget/atmel_usba_udc.c
gadget/atmel_usba_udc.h
gadget/fsl_udc_core.c
gadget/pxa27x_udc.h
host/ehci-omap.c
host/ehci-orion.c
host/ehci-w90x900.c
host/ehci.h
host/isp1760-hcd.c
host/ohci-da8xx.c
host/ohci-nxp.c
host/ohci-pxa27x.c
musb/da8xx.c
musb/davinci.c
musb/musb_dsps.c
musb/musb_io.h
phy/phy-fsl-usb.c

Would it be better to wrap __raw_readl in a macro and call that
everywhere rather than calling __raw_readl itself ?

Regards,
-- 
Vincent Pelletier
--
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 3/3] phy: ulpi: add driver for TI TUSB1210

2015-01-23 Thread Heikki Krogerus
Hi,

On Thu, Jan 22, 2015 at 02:49:25PM -0600, Felipe Balbi wrote:
> On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> > > > +   /* Store initial eye diagram optimisation value */
> > > > +   ret = ulpi_read(ulpi, ULPI_EXT_VENDOR_SPECIFIC);
> > > 
> > > do they *all* use this register for eye diagram optimization or is this
> > > something that Intel decided to do ?
> > > 
> > > (sorry, don't know much about tusb1210 other than it sucks like hell :-)
> > 
> > All I know that somebody needs to save the value. The ones using this
> > PHY who don't need to save it can most likely live without the driver.
> 
> right, but what I mean is: is it mandatory that Eye diagram
> configuration be stored in *this* register? Or is it more like a scratch
> register which Intel just happens to be using for Eye diagram data ?

The eye diagram tuning is in that register. Here's the spec:
http://www.ti.com/lit/ds/symlink/tusb1210.pdf

I'll add definition for the register (which is colourfully named
"VENDOR_SPECIFIC2").

> > > > +   if (ret < 0)
> > > > +   return ret;
> > > > +
> > > > +   tusb->ctx[0] = ret;
> > > > +
> > > > +   tusb->phy = ulpi_phy_create(ulpi, &phy_ops);
> > > > +   if (IS_ERR(tusb->phy))
> > > > +   return PTR_ERR(tusb->phy);
> > > > +
> > > > +   tusb->ulpi = ulpi;
> > > > +
> > > > +   phy_set_drvdata(tusb->phy, tusb);
> > > > +   dev_set_drvdata(&ulpi->dev, tusb);
> > > > +   return 0;
> > > > +}
> > > > +
> > > > +static void tusb1210_remove(struct ulpi *ulpi)
> > > > +{
> > > > +   struct tusb1210 *tusb = dev_get_drvdata(&ulpi->dev);
> > > 
> > > completely unrelated to $subject, but we might want to have a
> > > ulpi_{set,get}_drvdata() at some point.
> > 
> > Makes sense.
> > 
> > > In fact, we might decide to add an entire ULPI bus, eventually, though
> > > I'm still considering if there's any benefit to that.
> > 
> > I don't think I understand this comment? ULPI bus is what I'm
> > introducing in this set (the first patch in it)?
> 
> I mean introducing a real struct bus ulpi_bus_type :-) With match,
> probe, remove, etc.

I'm already doing that. Please check the first patch in this set:
"phy: add bus for USB ULPI PHYs".


Cheers,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html