Re: [PATCH] usb: storage: fix runtime pm issue in usb_stor_probe2

[Heiner Kallweit]

Am 03.08.2016 um 23:25 schrieb Alan Stern:
> On Wed, 3 Aug 2016, Heiner Kallweit wrote:
> 
>> Since commit 71723f95463d "PM / runtime: print error when activating a
>> child to unactive parent" I see the following error message:
>>
>> scsi host2: usb-storage 1-3:1.0
>> scsi host2: runtime PM trying to activate child device host2 but parent
>>  (1-3:1.0) is not active
>>
>> Digging into it it seems to be related to the problem described in the
>> commit message for cd998ded5c12 "i2c: designware: Prevent runtime
>> suspend during adapter registration" as scsi_add_host also calls
>> device_add and after the call to device_add the parent device is
>> suspended.
>>
>> Fix this by using the approach from the mentioned commit and getting
>> the runtime pm reference before calling scsi_add_host.
> 
> Acked-by: Alan Stern 
> 
> There's nothing wrong with this patch; it's a worthwhile thing to do 
> because it can prevent an unnecessary runtime-suspend/resume cycle.
> 
> Still, it looks like the real problem here may lie in
> drivers/scsi/hosts.c.  Commit bc4f24014de5 ("[SCSI] implement runtime
> Power Management") added a call to pm_runtime_set_active() in
> scsi_add_host_with_dma() _after_ device_add() instead of _before_.
> 
> If that were changed, this problem would not have occurred.
> 
In parallel to this patch I sent another one to address exactly the
ordering issue in scsi_add_host_with_dma you mention.
The other patch went to Martin and the SCSI mailing list only, sorry.
I'll forward it to you for review.

Heiner

> 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: usbtmc: vendor specific i/o

[Greg Kroah-Hartman]

On Wed, Aug 03, 2016 at 09:06:25AM +0200, Ladislav Michl wrote:
> On Wed, Aug 03, 2016 at 05:59:59AM +0200, Greg Kroah-Hartman wrote:
> > But your patch was "one-way", once you switched to the other mode, the
> > old one could not be used :(
> 
> Yes, also was lacking proper description and signoff. So, I'm considering
> ioctls based approach okay, although that question (the only one I really
> had) was never answered.
> 
> After re-reading specifications [*] I decided to allow arbitrary MsgID
> selection, as USB488 adds MsgID=TRIGGER (128) and other subclass
> specifications may add other values.
> 
> [*] http://www.usb.org/developers/docs/devclass_docs/USBTMC_1_006a.zip
> 
> After sorting out all eventual objections, patch bellow will be turned
> into proper one.

Looks reasonable to me.

thanks,

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

[PATCH v5] usb: ohci-at91: Forcibly suspend ports while USB suspend

[Wenyou Yang]

The usb controller does not managed correctly the suspend mode for
the ehci. In echi mode, there is no way to have utmi_suspend_o_n[1]
suspend without any device connected to it. This is why this specific
control is added to fix this issue. The suspend mode works in ohci
mode.

This specific control is by setting the SUSPEND_A/B/C fields of
SFR_OHCIICR(OHCI Interrupt Configuration Register) in the SFR
while OHCI USB suspend.

This setting operation must be done before the USB clock disabled,
clear them after the USB clock enabled.

Signed-off-by: Wenyou Yang 
Reviewed-by: Alexandre Belloni 
Acked-by: Nicolas Ferre 
---

Changes in v5:
 - Use the USB_PORT_FEAT_SUSPEND subcase of the SetPortFeature case
   to take care it.
 - Update the commit log.

Changes in v4:
 - To check whether the SFR node with "atmel,sama5d2-sfr" compatible
   is present or not to decide if this feature is applied or not
   when USB OHCI suspend/resume, instead of new compatible.
 - Drop the compatible "atmel,sama5d2-ohci".
 - Drop [PATCH 2/2] ARM: at91/dt: sama5d2: Use new compatible for
   ohci node.
 - Drop include/soc/at91/at91_sfr.h, move the macro definitions to
   atmel-sfr.h which already exists.
 - Change the defines to align the exists.

Changes in v3:
 - Change the compatible description for more precise.

Changes in v2:
 - Add compatible to support forcibly suspend the ports.
 - Add soc/at91/at91_sfr.h to accommodate the defines.
 - Add error checking for .sfr_regmap.
 - Remove unnecessary regmap_read() statement.

 drivers/usb/host/ohci-at91.c | 72 ++--
 include/soc/at91/atmel-sfr.h |  3 ++
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index d177372..4d7d7a0 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -21,8 +21,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
+#include 
 
 #include "ohci.h"
 
@@ -51,6 +54,7 @@ struct ohci_at91_priv {
struct clk *hclk;
bool clocked;
bool wakeup;/* Saved wake-up state for resume */
+   struct regmap *sfr_regmap;
 };
 /* interface and function clocks; sometimes also an AHB clock */
 
@@ -134,6 +138,17 @@ static void at91_stop_hc(struct platform_device *pdev)
 
 static void usb_hcd_at91_remove (struct usb_hcd *, struct platform_device *);
 
+struct regmap *at91_dt_syscon_sfr(void)
+{
+   struct regmap *regmap;
+
+   regmap = syscon_regmap_lookup_by_compatible("atmel,sama5d2-sfr");
+   if (IS_ERR(regmap))
+   regmap = NULL;
+
+   return regmap;
+}
+
 /* configure so an HC device and id are always provided */
 /* always called with process context; sleeping is OK */
 
@@ -197,6 +212,10 @@ static int usb_hcd_at91_probe(const struct hc_driver 
*driver,
goto err;
}
 
+   ohci_at91->sfr_regmap = at91_dt_syscon_sfr();
+   if (!ohci_at91->sfr_regmap)
+   dev_warn(dev, "failed to find sfr node\n");
+
board = hcd->self.controller->platform_data;
ohci = hcd_to_ohci(hcd);
ohci->num_ports = board->ports;
@@ -282,6 +301,28 @@ static int ohci_at91_hub_status_data(struct usb_hcd *hcd, 
char *buf)
return length;
 }
 
+static int ohci_at91_port_ctrl(struct regmap *regmap, u16 port, u8 set)
+{
+   u32 regval;
+   int ret;
+
+   if (!regmap)
+   return 0;
+
+   ret = regmap_read(regmap, AT91_SFR_OHCIICR, );
+   if (ret)
+   return ret;
+
+   if (set)
+   regval |= AT91_OHCIICR_SUSPEND(port);
+   else
+   regval &= ~AT91_OHCIICR_SUSPEND(port);
+
+   regmap_write(regmap, AT91_SFR_OHCIICR, regval);
+
+   return 0;
+}
+
 /*
  * Look at the control requests to the root hub and see if we need to override.
  */
@@ -289,6 +330,7 @@ static int ohci_at91_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
 u16 wIndex, char *buf, u16 wLength)
 {
struct at91_usbh_data *pdata = dev_get_platdata(hcd->self.controller);
+   struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
struct usb_hub_descriptor *desc;
int ret = -EINVAL;
u32 *data = (u32 *)buf;
@@ -301,7 +343,8 @@ static int ohci_at91_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
 
switch (typeReq) {
case SetPortFeature:
-   if (wValue == USB_PORT_FEAT_POWER) {
+   switch (wValue) {
+   case USB_PORT_FEAT_POWER:
dev_dbg(hcd->self.controller, "SetPortFeat: POWER\n");
if (valid_port(wIndex)) {
ohci_at91_usb_set_power(pdata, wIndex, 1);
@@ -309,6 +352,11 @@ static int ohci_at91_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
   

Re: [PATCH] USB: fix invalid memory access in hub_activate()

[mgautam]

On 2015-12-17 00:02, Alan Stern wrote:

Commit 8520f38099cc ("USB: change hub initialization sleeps to
delayed_work") changed the hub_activate() routine to make part of it
run in a workqueue.  However, the commit failed to take a reference to
the usb_hub structure or to lock the hub interface while doing so.  As
a result, if a hub is plugged in and quickly unplugged before the work
routine can run, the routine will try to access memory that has been
deallocated.  Or, if the hub is unplugged while the routine is
running, the memory may be deallocated while it is in active use.

This patch fixes the problem by taking a reference to the usb_hub at
the start of hub_activate() and releasing it at the end (when the work
is finished), and by locking the hub interface while the work routine
is running.  It also adds a check at the start of the routine to see
if the hub has already been disconnected, in which nothing should be
done.


Could this change result in below mutex lock that I am seeing:
"kworker/3:2" call stack:
{code}
-000|__switch_to(prev = 0xFFC032EAAA00, next = 0xFFC075D59C00)
-001|context_switch(inline)
-001|__schedule()
-002|schedule()
-003|schedule_preempt_disabled()
-004|__mutex_lock_common(inline)
-004|__mutex_lock_slowpath(lock_count = 0xFFC07BE94580)
-005|current_thread_info(inline)
-005|mutex_set_owner(inline)
-005|mutex_lock(
|lock = 0xFFC00255EE90 -> (
|  count = (counter = -1),
|  wait_lock = (rlock = (raw_lock = (owner = 4, next = 4), magic 
= 3735899821, owner_cpu = 4294967295, owner = 0x)),
|  wait_list = (next = 0xFFC0457DBC78, prev = 
0xFFC0457DBC78),

|  owner = 0xFFC075CD5400,
|  name = 0x0,
|  magic = 0xFFC00255EE90))
-006|hub_activate(hub = 0xFFC00253A680, type = 472914991)
-007|hub_init_func2(?)
-008|static_key_count(inline)
-008|static_key_false(inline)
-008|trace_workqueue_execute_end(inline)
-008|process_one_work(worker = 0xFFC017C8CA00, work = 
0xFFC00253A848)

-009|worker_thread(__worker = 0xFFC017C8CA00)



"kworker/u16:0" call stack:
{code}
-000|__switch_to(prev = 0xFFC075CD5400, next = 0xFFC02451)
-001|context_switch(inline)
-001|__schedule()
-002|schedule()
-003|schedule_timeout(timeout = -272798837376)
-004|do_wait_for_common(inline)
-004|__wait_for_common(inline)
-004|wait_for_common(x = 0xFFC075D3F928, timeout = 
9223372036854775807, ?)

-005|wait_for_completion(?)
-006|flush_work(<-- wq is 
already blocked for the mutex above

|work = 0xFFC00253A848 -> (
|  data = (counter = 417),
|  entry = (next = 0xFFC00253A850, prev = 
0xFFC00253A850),

|  func = 0xFFC000659B20 -> ))
|  barr = (work = (data = (counter = -272799003163), entry = (next = 
0xFFC017C8CA30, prev = 0xFFC017C8CA30), func =

|  pool = 0xFFC0703784C0
|  pwq = 0xFFC07BE98C00
|  linked = 26644480
|  __key = ()
-007|clear_work_data(inline)
-007|__cancel_work_timer(work = 0xFFC00253A848, is_dwork = FALSE)
-008|cancel_delayed_work_sync(?)
-009|hub_quiesce(hub = 0xFFC00253A680, ?)
-010|hub_disconnect(intf = 0xFFC00255EE00)
-011|usb_unbind_interface(dev = 0xFFC00255EE30)
-012|__device_release_driver(dev = 0xCB88537FDC8CAE50)
-013|device_unlock(inline)
-013|device_release_driver(dev = 0xFFC00255EE30)
-014|bus_remove_device(dev = 0xFFC00255EE30)
-015|device_del(dev = 0xFFC00255EE30)
-016|usb_disable_device(dev = 0xFFC032C42600, skip_ep0 = 0)
-017|usb_disconnect(pdev = 0xFFC075D3FCA0)
-018|usb_remove_hcd(hcd = 0xFFC0700AE880)



Should we rather have device_trylock instead of device_lock



Signed-off-by: Alan Stern 
Reported-by: Alexandru Cornea 
Tested-by: Alexandru Cornea 
Fixes: 8520f38099cc ("USB: change hub initialization sleeps to 
delayed_work")

CC: 


[as1791]


 drivers/usb/core/hub.c |   22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

Index: usb-4.3/drivers/usb/core/hub.c
===
--- usb-4.3.orig/drivers/usb/core/hub.c
+++ usb-4.3/drivers/usb/core/hub.c
@@ -1031,10 +1031,20 @@ static void hub_activate(struct usb_hub
unsigned delay;

/* Continue a partial initialization */
-   if (type == HUB_INIT2)
-   goto init2;
-   if (type == HUB_INIT3)
+   if (type == HUB_INIT2 || type == HUB_INIT3) {
+   device_lock(hub->intfdev);


device_trylock?? If failed to acquire then treat that as disconnect?


+
+   /* Was the hub disconnected while we were waiting? */
+   if (hub->disconnected) {
+   device_unlock(hub->intfdev);
+   kref_put(>kref, hub_release);
+   return;
+   }
+ 

RE: [PATCH] usb: ohci-at91: Suspend the ports while USB suspending

[Yang, Wenyou]

Hi Alan,

From: Alan Stern [st...@rowland.harvard.edu]
Sent: Friday, May 13, 2016 2:11
To: Yang, Wenyou
Cc: Greg Kroah-Hartman; Ferre, Nicolas; linux-usb@vger.kernel.org; 
linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
Subject: Re: [PATCH] usb: ohci-at91: Suspend the ports while USB suspending

On Thu, 12 May 2016, Wenyou Yang wrote:

> In order to get lower consumption, as a workaround, suspend
> the USB PORTA/B/C via set the SUSPEND_A/B/C bits of OHCI Interrupt
> Configuration Register while OHCI USB suspending.

What does this mean?  What does suspending a port do?  Is it the same
as a normal USB port suspend?

If it is the same, why doesn't the USB_PORT_FEAT_SUSPEND subcase of the
SetPortFeature case in ohci_hub_control() already take care of this?

Yes, you are right. 

We can use  the USB_PORT_FEAT_SUSPEND subcase of the
SetPortFeature case to take care this.

I will send a new patch, please help review it. Thanks a lot.

> This suspend operation must be done before stopping the USB clock,
> resume after the USB clock enabled.
>
> Signed-off-by: Wenyou Yang 
> ---

> @@ -132,6 +135,17 @@ static void at91_stop_hc(struct platform_device *pdev)
>
>  /*-*/
>
> +struct regmap *at91_dt_syscon_sfr(void)
> +{
> + struct regmap *regmap;
> +
> + regmap = syscon_regmap_lookup_by_compatible("atmel,sama5d2-sfr");
> + if (IS_ERR(regmap))
> + return NULL;

If you get an error, the regmap pointer is set to NULL...

> @@ -197,6 +211,8 @@ static int usb_hcd_at91_probe(const struct hc_driver 
> *driver,
>   goto err;
>   }
>
> + ohci_at91->sfr_regmap = at91_dt_syscon_sfr();

With no other error checking...

> +
>   board = hcd->self.controller->platform_data;
>   ohci = hcd_to_ohci(hcd);
>   ohci->num_ports = board->ports;

> +static int ohci_at91_port_ctrl(struct regmap *regmap, bool enable)
> +{
> + u32 regval;
> + int ret;
> +
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + ret = regmap_read(regmap, SFR_OHCIICR, );

And now what happens if regmap is NULL?  Hint: It won't be pretty...

Alan Stern


Best Regards,
Wenyou Yang

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

Re: [PATCH v4 2/6] power: add power sequence library

[Matthias Kaehlcke]

El Tue, Aug 02, 2016 at 11:30:48AM +0800 Peter Chen ha dit:

> diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
>
> ...
>
> +static DEFINE_MUTEX(pwrseq_list_mutex);
> +static LIST_HEAD(pwrseq_list);
> +
> +int pwrseq_get(struct device_node *np, struct pwrseq *p)
> +{
> + if (p && p->get)
> + return p->get(np, p);
> +
> + return -ENOTSUPP;
> +}
> +
> +int pwrseq_on(struct device_node *np, struct pwrseq *p)
> +{
> + if (p && p->on)
> + return p->on(np, p);
> +
> + return -ENOTSUPP;
> +}
> +
> +void pwrseq_off(struct pwrseq *p)
> +{
> + if (p && p->off)
> + p->off(p);
> +}
> +
> +void pwrseq_put(struct pwrseq *p)
> +{
> + if (p && p->put)
> + p->put(p);
> +}
> +
> +void pwrseq_free(struct pwrseq *p)
> +{
> + if (p && p->free)
> + p->free(p);
> +}
> +
> +void pwrseq_register(struct pwrseq *pwrseq)
> +{
> + mutex_lock(_list_mutex);
> + list_add(>node, _list);
> + mutex_unlock(_list_mutex);
> +}
> +
> +void pwrseq_unregister(struct pwrseq *pwrseq)
> +{
> + mutex_lock(_list_mutex);
> + list_del(>node);
> + mutex_unlock(_list_mutex);
> +}

What is the purpose of pwrseq_register/unregister()? The pwrseq
structs are added and removed from pwrseq_list, but besides that
pwrseq_list is not used.

Looks like this is a remainder from the ancestor of this code
(drivers/mmc/core/pwrseq.c) which uses the list to avoid having
multiple pwrseq instances for the same device.

Matthias
--
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: storage: fix runtime pm issue in usb_stor_probe2

[Alan Stern]

On Wed, 3 Aug 2016, Heiner Kallweit wrote:

> Since commit 71723f95463d "PM / runtime: print error when activating a
> child to unactive parent" I see the following error message:
> 
> scsi host2: usb-storage 1-3:1.0
> scsi host2: runtime PM trying to activate child device host2 but parent
>   (1-3:1.0) is not active
> 
> Digging into it it seems to be related to the problem described in the
> commit message for cd998ded5c12 "i2c: designware: Prevent runtime
> suspend during adapter registration" as scsi_add_host also calls
> device_add and after the call to device_add the parent device is
> suspended.
> 
> Fix this by using the approach from the mentioned commit and getting
> the runtime pm reference before calling scsi_add_host.

Acked-by: Alan Stern 

There's nothing wrong with this patch; it's a worthwhile thing to do 
because it can prevent an unnecessary runtime-suspend/resume cycle.

Still, it looks like the real problem here may lie in
drivers/scsi/hosts.c.  Commit bc4f24014de5 ("[SCSI] implement runtime
Power Management") added a call to pm_runtime_set_active() in
scsi_add_host_with_dma() _after_ device_add() instead of _before_.

If that were changed, this problem would not have occurred.

Alan Stern

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

[PATCH] usb: storage: fix runtime pm issue in usb_stor_probe2

[Heiner Kallweit]

Since commit 71723f95463d "PM / runtime: print error when activating a
child to unactive parent" I see the following error message:

scsi host2: usb-storage 1-3:1.0
scsi host2: runtime PM trying to activate child device host2 but parent
(1-3:1.0) is not active

Digging into it it seems to be related to the problem described in the
commit message for cd998ded5c12 "i2c: designware: Prevent runtime
suspend during adapter registration" as scsi_add_host also calls
device_add and after the call to device_add the parent device is
suspended.

Fix this by using the approach from the mentioned commit and getting
the runtime pm reference before calling scsi_add_host.

Signed-off-by: Heiner Kallweit 
---
 drivers/usb/storage/usb.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index ef2d8cd..8c5f011 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -1070,17 +1070,17 @@ int usb_stor_probe2(struct us_data *us)
result = usb_stor_acquire_resources(us);
if (result)
goto BadDevice;
+   usb_autopm_get_interface_no_resume(us->pusb_intf);
snprintf(us->scsi_name, sizeof(us->scsi_name), "usb-storage %s",
dev_name(>pusb_intf->dev));
result = scsi_add_host(us_to_host(us), dev);
if (result) {
dev_warn(dev,
"Unable to add the scsi host\n");
-   goto BadDevice;
+   goto HostAddErr;
}
 
/* Submit the delayed_work for SCSI-device scanning */
-   usb_autopm_get_interface_no_resume(us->pusb_intf);
set_bit(US_FLIDX_SCAN_PENDING, >dflags);
 
if (delay_use > 0)
@@ -1090,6 +1090,8 @@ int usb_stor_probe2(struct us_data *us)
return 0;
 
/* We come here if there are any problems */
+HostAddErr:
+   usb_autopm_put_interface_no_suspend(us->pusb_intf);
 BadDevice:
usb_stor_dbg(us, "storage_probe() failed\n");
release_everything(us);
-- 
2.9.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

Re: [PATCH v9 3/4] arm64: dts: rockchip: add usb2-phy support for rk3399

[Heiko Stübner]

Am Freitag, 22. Juli 2016, 15:00:45 schrieb Frank Wang:
> Add usb2-phy nodes and specify phys phandle for ehci.
> 
> Signed-off-by: Frank Wang 

looks good to me. Of course we need Kishon to be happy with the driver itself 
first and the merge-window to end :-) .

I see that this won't apply cleanly, as the grf changed under your feet, but 
no need to resend for this alone, as I can fix that up myself.


Heiko

> ---
> 
> Changes in v9:
>  - Move the usb gpio config to rk3399-evb.dts
>  - Fix ehci phy-names property.
> 
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi |   42
> +- 1 file changed, 41 insertions(+), 1
> deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index d7f8e06..843d51c 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -221,6 +221,8 @@
>   interrupts = ;
>   clocks = < HCLK_HOST0>, < HCLK_HOST0_ARB>;
>   clock-names = "hclk_host0", "hclk_host0_arb";
> + phys = <_host>;
> + phy-names = "usb";
>   status = "disabled";
>   };
> 
> @@ -239,6 +241,8 @@
>   interrupts = ;
>   clocks = < HCLK_HOST1>, < HCLK_HOST1_ARB>;
>   clock-names = "hclk_host1", "hclk_host1_arb";
> + phys = <_host>;
> + phy-names = "usb";
>   status = "disabled";
>   };
> 
> @@ -481,8 +485,44 @@
>   };
> 
>   grf: syscon@ff77 {
> - compatible = "rockchip,rk3399-grf", "syscon";
> + compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
>   reg = <0x0 0xff77 0x0 0x1>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + u2phy0: usb2-phy@e450 {
> + compatible = "rockchip,rk3399-usb2phy";
> + reg = <0xe450 0x10>;
> + clocks = < SCLK_USB2PHY0_REF>;
> + clock-names = "phyclk";
> + #clock-cells = <0>;
> + clock-output-names = "clk_usbphy0_480m";
> + status = "disabled";
> +
> + u2phy0_host: host-port {
> + #phy-cells = <0>;
> + interrupts = ;
> + interrupt-names = "linestate";
> + status = "disabled";
> + };
> + };
> +
> + u2phy1: usb2-phy@e460 {
> + compatible = "rockchip,rk3399-usb2phy";
> + reg = <0xe460 0x10>;
> + clocks = < SCLK_USB2PHY1_REF>;
> + clock-names = "phyclk";
> + #clock-cells = <0>;
> + clock-output-names = "clk_usbphy1_480m";
> + status = "disabled";
> +
> + u2phy1_host: host-port {
> + #phy-cells = <0>;
> + interrupts = ;
> + interrupt-names = "linestate";
> + status = "disabled";
> + };
> + };
>   };
> 
>   watchdog@ff84 {

--
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 3/4] usb: typec: USB Type-C Port Manager (tcpm)

[Guenter Roeck]

On Wed, Aug 3, 2016 at 2:28 AM, Oliver Neukum  wrote:
> On Tue, 2016-08-02 at 13:32 -0700, Guenter Roeck wrote:
>> +static bool svdm_consume_svids(struct tcpm_port *port, const u32
>> *payload,
>> +  int cnt)
>> +{
>> +   struct pd_mode_data *pmdata = >mode_data;
>> +   int i;
>> +
>> +   for (i = 1; i < cnt; i++) {
>> +   u16 svid;
>> +
>> +   svid = (payload[i] >> 16) & 0x;
>> +   if (!svid)
>> +   return false;
>
> Hi,
>
> this looks like an endianness bug.
>

Yes, you are right, and there are lots of those in my code. Thanks for
bringing it up.

Guenter
--
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 3/4] usb: typec: USB Type-C Port Manager (tcpm)

[Guenter Roeck]

On Wed, Aug 3, 2016 at 2:18 AM, Oliver Neukum  wrote:
> On Tue, 2016-08-02 at 13:32 -0700, Guenter Roeck wrote:
>> +static int tcpm_set_polarity(struct tcpm_port *port,
>> +enum typec_cc_polarity polarity)
>> +{
>> +   tcpm_log(port, "polarity %d", polarity);
>> +
>> +   port->polarity = polarity;
>> +
>> +   return port->tcpc->set_polarity(port->tcpc, port->polarity);
>> +}
>
> Here you don't care about the result.

Changed to only set port->polarity if the set_polarity callback was successful.

Thanks,
Guenter
--
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: [Letux-kernel] [PATCH v2] musb: omap2430: do not assume balanced enable()/disable()

[H. Nikolaus Schaller]

> Am 03.08.2016 um 17:38 schrieb Andreas Kemnade :
> 
> The code assumes that omap2430_musb_enable() and
> omap2430_musb_disable() are called in a balanced way.
> That fact is broken by the fact that musb_init_controller() calls
> musb_platform_disable() to switch from unknown state to off state
> on initialisation.
> 
> That means that phy_power_off() is called first so that
> phy->power_count gets -1 and the phy is not enabled on phy_power_on().
> So when usb gadget is started the phy is not powered on.
> Depending on the phy used that caused various problems.
> Besides of causing usb problems, that can also have side effects.
> 
> In the case of using the phy_twl4030, that prevents also charging
> the battery via usb (using twl4030_charger) and so makes further
> kernel debugging hard.
> The problem was seen with 4.7 on an openphoenux gta04. It has a DM3730
> SoC and a TPS65950 companion.  phy->power never became 1

s/TPS65950/TPS65950 (twl4030)/

> and so the usb did get powered on.

s/did get/did not get/

maybe add:

All this prevents detection of cable plugin-events and VBUS measurement
and setting OTG_EN before charging is attempted.

> 
> The patch prevents phy_power_off() from being called when
> it is already off.
> 
> Signed-off-by: Andreas Kemnade 
> ---
> changes in v2:
> improved commit message
> 
> drivers/usb/musb/omap2430.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index 0b4cec9..c1a2b7b 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -413,9 +413,10 @@ static void omap2430_musb_disable(struct musb *musb)
>   struct device *dev = musb->controller;
>   struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
> 
> - if (!WARN_ON(!musb->phy))
> - phy_power_off(musb->phy);
> -
> + if (glue->enabled) {
> + if (!WARN_ON(!musb->phy))
> + phy_power_off(musb->phy);
> + }
>   if (glue->status != MUSB_UNKNOWN)
>   omap_control_usb_set_mode(glue->control_otghs,
>   USB_MODE_DISCONNECT);
> -- 
> 2.1.4
> 
> ___
> http://projects.goldelico.com/p/gta04-kernel/
> Letux-kernel mailing list
> letux-ker...@openphoenux.org
> http://lists.goldelico.com/mailman/listinfo.cgi/letux-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

[PATCH v2] musb: omap2430: do not assume balanced enable()/disable()

[Andreas Kemnade]

The code assumes that omap2430_musb_enable() and
omap2430_musb_disable() are called in a balanced way.
That fact is broken by the fact that musb_init_controller() calls
musb_platform_disable() to switch from unknown state to off state
on initialisation.

That means that phy_power_off() is called first so that
phy->power_count gets -1 and the phy is not enabled on phy_power_on().
So when usb gadget is started the phy is not powered on.
Depending on the phy used that caused various problems.
Besides of causing usb problems, that can also have side effects.

In the case of using the phy_twl4030, that prevents also charging
the battery via usb (using twl4030_charger) and so makes further
kernel debugging hard.
The problem was seen with 4.7 on an openphoenux gta04. It has a DM3730
SoC and a TPS65950 companion.  phy->power never became 1
and so the usb did get powered on.

The patch prevents phy_power_off() from being called when
it is already off.

Signed-off-by: Andreas Kemnade 
---
changes in v2:
improved commit message

 drivers/usb/musb/omap2430.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 0b4cec9..c1a2b7b 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -413,9 +413,10 @@ static void omap2430_musb_disable(struct musb *musb)
struct device *dev = musb->controller;
struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
 
-   if (!WARN_ON(!musb->phy))
-   phy_power_off(musb->phy);
-
+   if (glue->enabled) {
+   if (!WARN_ON(!musb->phy))
+   phy_power_off(musb->phy);
+   }
if (glue->status != MUSB_UNKNOWN)
omap_control_usb_set_mode(glue->control_otghs,
USB_MODE_DISCONNECT);
-- 
2.1.4

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

Re: [PATCH] musb: omap2430: do not assume balanced enable()/disable()

[Andreas Kemnade]

On Tue, 2 Aug 2016 23:30:16 -0700
Tony Lindgren  wrote:

> * Andreas Kemnade  [160802 08:14]:
> > On Tue, 2 Aug 2016 03:33:34 -0700
> > Tony Lindgren  wrote:
> > 
> > > * Andreas Kemnade  [160729 11:14]:
> > > > The code assumes that omap2430_musb_enable() and
> > > > omap2430_musb_disable() is called in a balanced way. The
> > > > That fact is broken by the fact that musb_init_controller()
> > > > calls musb_platform_disable() to switch from unknown state to
> > > > off state.
> > > 
> > > OK, some spelling issues with the above paragraph though :)
> > > 
> > > > That means that phy_power_off() is called first so that
> > > > phy->power_count gets -1 and the phy is not enabled on
> > > > phy_power_on(). In the probably common case of using the
> > > > phy_twl4030, that prevents also charging the battery and so
> > > > makes further kernel debugging hard.
> > > 
> > > Is this with v4.7 kernel? Also, care to describe how you hit this
> > > and on which hardware? Just wondering..
> > 
> > I got this error on the Openphoenux GTA04 phone. It has a DM3730
> > SoC and a TPS65950 companion. Severe charging problems were already
> > observed with the 4.4rc1. I do not know if that already was exactly
> > *this* problem. I have debugged and patched the v4.7 kernel.
> 
> OK thanks for the info.
> 
> > How I hit the problem: Just boot an that device and try to charge
> > via usb.
> 
> OK so it's the twl4030 charger then I guess.
> 
yes, besides from causing usb gadget problems, it has side effect
towards the twl4030 charger.

> > Should I resubmit the patch with an extended commit message?
> 
> Well yeah it might be worth describing that it's the twl4030
> charger that otherwise does not work properly.
> 
Ok, will resend a better commit message.

Regards,
Andreas Kemnade


pgp37vW0A_rBI.pgp
Description: OpenPGP digital signature

Re: [PATCH 2/2] usb: dwc3: core: allow device to runtime_suspend several times

[Alan Stern]

On Wed, 3 Aug 2016, Felipe Balbi wrote:

> >> --- a/drivers/usb/dwc3/core.c
> >> +++ b/drivers/usb/dwc3/core.c
> >> @@ -1192,6 +1192,7 @@ static int dwc3_runtime_resume(struct device *dev)
> >>}
> >>  
> >>pm_runtime_mark_last_busy(dev);
> >> +  pm_runtime_put(dev);
> >>  
> >>return 0;
> >>  }
> >
> > This may be correct, but it certainly looks odd.  For example, it 
> > wouldn't work right if you ever called pm_runtime_resume() instead of 
> > pm_runtime_get().
> 
> well, we don't. But is there an alternative for this?

What are you trying to accomplish?  Is the problem that wakeup signals
only cause the platform device to be runtime-resumed, but you also need
the HCD to wake up?  And conversely, whenever the HCD gets
runtime-suspended you also want the platform device to go into runtime
suspend?

If that's so, the proper solution is for the platform device's
runtime_resume routine to call pm_runtime_resume() for the HCD, and
never to do pm_runtime_get_* or pm_runtime_put_* on the platform
device.  The HCD's callback routines would then be responsible for
doing runtime-PM gets and puts on the HCD as required.

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] usb: xhci-plat: Add generic PHY support

[Srinath Mannam]

Hi Mathias Nyman,

Please review and provide your comments to the changes in this patch.

Regards,
srinath.

On Thu, Jul 14, 2016 at 3:29 PM, Felipe Balbi
 wrote:
>
> Hi,
>
> Srinath Mannam  writes:
>> Generic phy support added in xhci platform driver.
>> In the case of multiple phys to the xhci controller, this approach
>> is helpful to pass multiple phandles to xhci platform driver from
>> xhci device node.
>>
>> Signed-off-by: Srinath Mannam 
>> Reviewed-by: Ray Jui 
>> Reviewed-by: Scott Branden 
>
> I have no issues with this, but Mathias is off for a few weeks.
>
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0984/1285] Replace numeric parameter like 0444 with macro

[Oliver Neukum]

On Tue, 2016-08-02 at 16:54 +0300, Felipe Balbi wrote:
> Baole Ni  writes:
> 
> > I find that the developers often just specified the numeric value
> > when calling a macro which is defined with a parameter for access 
> > permission.
> > As we know, these numeric value for access permission have had the 
> > corresponding macro,
> > and that using macro can improve the robustness and readability of the code,
> > thus, I suggest replacing the numeric parameter with the macro.
> >
> > Signed-off-by: Chuansheng Liu 
> > Signed-off-by: Baole Ni 
> > ---
> >  drivers/usb/misc/usbtest.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> > index 6b978f0..5e81dc3 100644
> > --- a/drivers/usb/misc/usbtest.c
> > +++ b/drivers/usb/misc/usbtest.c
> > @@ -15,7 +15,7 @@
> >  
> > /*-*/
> >  
> >  static int override_alt = -1;
> > -module_param_named(alt, override_alt, int, 0644);
> > +module_param_named(alt, override_alt, int, S_IRUSR | S_IWUSR | S_IRGRP | 
> > S_IROTH);
> 
> line too long. You need to run this series through scripts/checkpatch.pl
> 

Before we think about that, the basic question whether

S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH

is clearer and easier to read than

0644

must be decided. I would saz no, it is not.

Regards
Oliver


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

[PATCH v8 4/5] usb: dwc3: add dis_del_phy_power_chg_quirk

[William Wu]

Add a quirk to clear the GUSB3PIPECTL.DELAYP1TRANS bit,
which specifies whether disable delay PHY power change
from P0 to P1/P2/P3 when link state changing from U0
to U1/U2/U3 respectively.

Signed-off-by: William Wu 
Acked-by: Rob Herring 
---
Changes in v8:
- add Acked-by (Rob Herring)

Changes in v7:
- None

Changes in v6:
- use '-' instead of '_' in dts (Rob Herring)

Changes in v5:
- None

Changes in v4:
- rebase on top of balbi testing/next, remove pdata (balbi)

Changes in v3:
- None

Changes in v2:
- None

 Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
 drivers/usb/dwc3/core.c| 5 +
 drivers/usb/dwc3/core.h| 3 +++
 3 files changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
b/Documentation/devicetree/bindings/usb/dwc3.txt
index 020b0e9..e96bfc2 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -42,6 +42,8 @@ Optional properties:
  - snps,dis-u2-freeclk-exists-quirk: when set, clear the u2_freeclk_exists
in GUSB2PHYCFG, specify that USB2 PHY doesn't provide
a free-running PHY clock.
+ - snps,dis-del-phy-power-chg-quirk: when set core will change PHY power
+   from P0 to P1/P2/P3 without delay.
  - 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
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index edbca03..b5e0ccc 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -448,6 +448,9 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->dis_u3_susphy_quirk)
reg &= ~DWC3_GUSB3PIPECTL_SUSPHY;
 
+   if (dwc->dis_del_phy_power_chg_quirk)
+   reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
+
dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
 
reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
@@ -947,6 +950,8 @@ static int dwc3_probe(struct platform_device *pdev)
"snps,dis_rxdet_inp3_quirk");
dwc->dis_u2_freeclk_exists_quirk = device_property_read_bool(dev,
"snps,dis-u2-freeclk-exists-quirk");
+   dwc->dis_del_phy_power_chg_quirk = device_property_read_bool(dev,
+   "snps,dis-del-phy-power-chg-quirk");
 
dwc->tx_de_emphasis_quirk = device_property_read_bool(dev,
"snps,tx_de_emphasis_quirk");
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index ff5a83a..e57e4e2 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -814,6 +814,8 @@ struct dwc3_scratchpad_array {
  * @dis_u2_freeclk_exists_quirk : set if we clear u2_freeclk_exists
  * in GUSB2PHYCFG, specify that USB2 PHY doesn't
  * provide a free-running PHY clock.
+ * @dis_del_phy_power_chg_quirk: set if we disable delay phy power
+ * change quirk.
  * @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk
  * @tx_de_emphasis: Tx de-emphasis value
  * 0   - -6dB de-emphasis
@@ -959,6 +961,7 @@ struct dwc3 {
unsigneddis_enblslpm_quirk:1;
unsigneddis_rxdet_inp3_quirk:1;
unsigneddis_u2_freeclk_exists_quirk:1;
+   unsigneddis_del_phy_power_chg_quirk:1;
 
unsignedtx_de_emphasis_quirk:1;
unsignedtx_de_emphasis:2;
-- 
1.9.1


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

[PATCH v8 2/5] usb: dwc3: add dis_u2_freeclk_exists_quirk

[William Wu]

Add a quirk to clear the GUSB2PHYCFG.U2_FREECLK_EXISTS bit,
which specifies whether the USB2.0 PHY provides a free-running
PHY clock, which is active when the clock control input is active.

Signed-off-by: William Wu 
Acked-by: Rob Herring 
---
Changes in v8:
- add Acked-by (Rob Herring)

Changes in v7:
- None

Changes in v6:
- use '-' instead of '_' in dts (Rob Herring)

Changes in v5:
- None

Changes in v4:
- rebase on top of balbi testing/next, remove pdata (balbi)

Changes in v3:
- None

Changes in v2:
- None

 Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++
 drivers/usb/dwc3/core.c| 5 +
 drivers/usb/dwc3/core.h| 5 +
 3 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
b/Documentation/devicetree/bindings/usb/dwc3.txt
index 7d7ce08..020b0e9 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -39,6 +39,9 @@ Optional properties:
disabling the suspend signal to the PHY.
  - snps,dis_rxdet_inp3_quirk: when set core will disable receiver detection
in PHY P3 power state.
+ - snps,dis-u2-freeclk-exists-quirk: when set, clear the u2_freeclk_exists
+   in GUSB2PHYCFG, specify that USB2 PHY doesn't provide
+   a free-running PHY clock.
  - 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
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9466431..0b7bfd2 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -500,6 +500,9 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->dis_enblslpm_quirk)
reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
 
+   if (dwc->dis_u2_freeclk_exists_quirk)
+   reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
+
dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
 
return 0;
@@ -924,6 +927,8 @@ static int dwc3_probe(struct platform_device *pdev)
"snps,dis_enblslpm_quirk");
dwc->dis_rxdet_inp3_quirk = device_property_read_bool(dev,
"snps,dis_rxdet_inp3_quirk");
+   dwc->dis_u2_freeclk_exists_quirk = device_property_read_bool(dev,
+   "snps,dis-u2-freeclk-exists-quirk");
 
dwc->tx_de_emphasis_quirk = device_property_read_bool(dev,
"snps,tx_de_emphasis_quirk");
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 45d6de5..f321a5c 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -199,6 +199,7 @@
 
 /* Global USB2 PHY Configuration Register */
 #define DWC3_GUSB2PHYCFG_PHYSOFTRST(1 << 31)
+#define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS (1 << 30)
 #define DWC3_GUSB2PHYCFG_SUSPHY(1 << 6)
 #define DWC3_GUSB2PHYCFG_ULPI_UTMI (1 << 4)
 #define DWC3_GUSB2PHYCFG_ENBLSLPM  (1 << 8)
@@ -799,6 +800,9 @@ struct dwc3_scratchpad_array {
  * @dis_u2_susphy_quirk: set if we disable usb2 suspend phy
  * @dis_enblslpm_quirk: set if we clear enblslpm in GUSB2PHYCFG,
  *  disabling the suspend signal to the PHY.
+ * @dis_u2_freeclk_exists_quirk : set if we clear u2_freeclk_exists
+ * in GUSB2PHYCFG, specify that USB2 PHY doesn't
+ * provide a free-running PHY clock.
  * @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk
  * @tx_de_emphasis: Tx de-emphasis value
  * 0   - -6dB de-emphasis
@@ -942,6 +946,7 @@ struct dwc3 {
unsigneddis_u2_susphy_quirk:1;
unsigneddis_enblslpm_quirk:1;
unsigneddis_rxdet_inp3_quirk:1;
+   unsigneddis_u2_freeclk_exists_quirk:1;
 
unsignedtx_de_emphasis_quirk:1;
unsignedtx_de_emphasis:2;
-- 
1.9.1


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

[PATCH v8 3/5] usb: dwc3: make usb2 phy utmi interface configurable

[William Wu]

Support to configure the UTMI+ PHY with an 8- or 16-bit
interface via DT. The UTMI+ PHY interface is a hardware
capability, and it's platform dependent. Normally, the
PHYIF can be configured during coreconsultant.

But for some specific USB cores(e.g. rk3399 SoC DWC3),
the default PHYIF configuration value is false, so we
need to reconfigure it by software.

Signed-off-by: William Wu 
Acked-by: Rob Herring 
---
Changes in v8:
- configure utmi interface via phy_type property in DT (Heiko, Rob Herring)
- add Acked-by (Rob Herring)
- modify commit message (Rob Herring)

Changes in v7:
- remove quirk and use only one property to configure utmi (Heiko, Rob Herring)

Changes in v6:
- use '-' instead of '_' in dts (Rob Herring)

Changes in v5:
- None

Changes in v4:
- rebase on top of balbi testing/next, remove pdata (balbi)

Changes in v3:
- None

Changes in v2:
- add a quirk for phyif_utmi (balbi)

 Documentation/devicetree/bindings/usb/generic.txt |  6 ++
 drivers/usb/dwc3/core.c   | 18 ++
 drivers/usb/dwc3/core.h   | 12 
 3 files changed, 36 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/generic.txt 
b/Documentation/devicetree/bindings/usb/generic.txt
index bba8257..bfadeb1 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -11,6 +11,11 @@ Optional properties:
"peripheral" and "otg". In case this attribute isn't
passed via DT, USB DRD controllers should default to
OTG.
+ - phy_type: tells USB controllers that we want to configure the core to 
support
+   a UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is
+   selected. Valid arguments are "utmi" and "utmi_wide".
+   In case this isn't passed via DT, USB controllers should
+   default to HW capability.
  - otg-rev: tells usb driver the release number of the OTG and EH supplement
with which the device and its descriptors are compliant,
in binary-coded decimal (i.e. 2.0 is 0200H). This
@@ -34,6 +39,7 @@ dwc3@4a03 {
usb-phy = <_phy>, <,phy>;
maximum-speed = "super-speed";
dr_mode = "otg";
+   phy_type = "utmi_wide";
otg-rev = <0x0200>;
adp-disable;
 };
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 0b7bfd2..edbca03 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -485,6 +485,23 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
break;
}
 
+   switch (dwc->hsphy_mode) {
+   case USBPHY_INTERFACE_MODE_UTMI:
+   reg &= ~(DWC3_GUSB2PHYCFG_PHYIF_MASK |
+  DWC3_GUSB2PHYCFG_USBTRDTIM_MASK);
+   reg |= DWC3_GUSB2PHYCFG_PHYIF(UTMI_PHYIF_8_BIT) |
+  DWC3_GUSB2PHYCFG_USBTRDTIM(USBTRDTIM_UTMI_8_BIT);
+   break;
+   case USBPHY_INTERFACE_MODE_UTMIW:
+   reg &= ~(DWC3_GUSB2PHYCFG_PHYIF_MASK |
+  DWC3_GUSB2PHYCFG_USBTRDTIM_MASK);
+   reg |= DWC3_GUSB2PHYCFG_PHYIF(UTMI_PHYIF_16_BIT) |
+  DWC3_GUSB2PHYCFG_USBTRDTIM(USBTRDTIM_UTMI_16_BIT);
+   break;
+   default:
+   break;
+   }
+
/*
 * Above 1.94a, it is recommended to set DWC3_GUSB2PHYCFG_SUSPHY to
 * '0' during coreConsultant configuration. So default value will
@@ -891,6 +908,7 @@ static int dwc3_probe(struct platform_device *pdev)
 
dwc->maximum_speed = usb_get_maximum_speed(dev);
dwc->dr_mode = usb_get_dr_mode(dev);
+   dwc->hsphy_mode = of_usb_get_phy_mode(dev->of_node);
 
dwc->has_lpm_erratum = device_property_read_bool(dev,
"snps,has-lpm-erratum");
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index f321a5c..ff5a83a 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -203,6 +203,14 @@
 #define DWC3_GUSB2PHYCFG_SUSPHY(1 << 6)
 #define DWC3_GUSB2PHYCFG_ULPI_UTMI (1 << 4)
 #define DWC3_GUSB2PHYCFG_ENBLSLPM  (1 << 8)
+#define DWC3_GUSB2PHYCFG_PHYIF(n)  (n << 3)
+#define DWC3_GUSB2PHYCFG_PHYIF_MASKDWC3_GUSB2PHYCFG_PHYIF(1)
+#define DWC3_GUSB2PHYCFG_USBTRDTIM(n)  (n << 10)
+#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASKDWC3_GUSB2PHYCFG_USBTRDTIM(0xf)
+#define USBTRDTIM_UTMI_8_BIT   9
+#define USBTRDTIM_UTMI_16_BIT  5
+#define UTMI_PHYIF_16_BIT  1
+#define UTMI_PHYIF_8_BIT   0
 
 /* Global USB2 PHY Vendor Control Register */
 #define DWC3_GUSB2PHYACC_NEWREGREQ (1 << 25)
@@ -744,6 +752,9 @@ struct dwc3_scratchpad_array {
  * @maximum_speed: maximum speed requested (mainly for testing purposes)
  * @revision: revision register contents
  

[PATCH v8 0/5] support rockchip dwc3 driver

[William Wu]

This series add support for rockchip dwc3 driver,
and add additional optional properties for specific
platforms (e.g., rockchip rk3399 platform).

William Wu (5):
  usb: dwc3: of-simple: add compatible for rockchip rk3399
  usb: dwc3: add dis_u2_freeclk_exists_quirk
  usb: dwc3: make usb2 phy utmi interface configurable
  usb: dwc3: add dis_del_phy_power_chg_quirk
  usb: dwc3: rockchip: add devicetree bindings documentation

 Documentation/devicetree/bindings/usb/dwc3.txt |  5 ++
 Documentation/devicetree/bindings/usb/generic.txt  |  6 +++
 .../devicetree/bindings/usb/rockchip,dwc3.txt  | 59 ++
 drivers/usb/dwc3/core.c| 28 ++
 drivers/usb/dwc3/core.h| 20 
 drivers/usb/dwc3/dwc3-of-simple.c  |  1 +
 6 files changed, 119 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt

-- 
1.9.1


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

[PATCH v8 1/5] usb: dwc3: of-simple: add compatible for rockchip rk3399

[William Wu]

Rockchip platform merely enable usb3 clocks and
populate its children. So we can use this generic
glue layer to support Rockchip dwc3.

Signed-off-by: William Wu 
---
Changes in v8:
- None

Changes in v7:
- None

Changes in v6:
- None

Changes in v5:
- change compatible from "rockchip,dwc3" to "rockchip,rk3399-dwc3" (Heiko)

Changes in v4:
- None

Changes in v3:
- None

Changes in v2:
- sort the list of_dwc3_simple_match (Doug)

 drivers/usb/dwc3/dwc3-of-simple.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index 9743353..05c9349 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -161,6 +161,7 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops = {
 
 static const struct of_device_id of_dwc3_simple_match[] = {
{ .compatible = "qcom,dwc3" },
+   { .compatible = "rockchip,rk3399-dwc3" },
{ .compatible = "xlnx,zynqmp-dwc3" },
{ /* Sentinel */ }
 };
-- 
1.9.1


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

[PATCH v8 5/5] usb: dwc3: rockchip: add devicetree bindings documentation

[William Wu]

This patch adds the devicetree documentation required for Rockchip
USB3.0 core wrapper consisting of USB3.0 IP from Synopsys.

It supports DRD mode, and could operate in device mode (SS, HS, FS)
and host mode (SS, HS, FS, LS).

Signed-off-by: William Wu 
Acked-by: Rob Herring 
---
Changes in v8:
- None

Changes in v7:
- add Acked-by (Rob Herring)

Changes in v6:
- rename bus_clk, and add usbdrd3_1 node as an example (Heiko)

Changes in v5:
- rename clock-names, and remove unnecessary clocks (Heiko)

Changes in v4:
- modify commit log, and add phy documentation location (Sergei)

Changes in v3:
- add dwc3 address (balbi)

Changes in v2:
- add rockchip,dwc3.txt to Documentation/devicetree/bindings/ (balbi, Brian)

 .../devicetree/bindings/usb/rockchip,dwc3.txt  | 59 ++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt

diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt 
b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
new file mode 100644
index 000..0536a93
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
@@ -0,0 +1,59 @@
+Rockchip SuperSpeed DWC3 USB SoC controller
+
+Required properties:
+- compatible:  should contain "rockchip,rk3399-dwc3" for rk3399 SoC
+- clocks:  A list of phandle + clock-specifier pairs for the
+   clocks listed in clock-names
+- clock-names: Should contain the following:
+  "ref_clk"Controller reference clk, have to be 24 MHz
+  "suspend_clk"Controller suspend clk, have to be 24 MHz or 32 KHz
+  "bus_clk"Master/Core clock, have to be >= 62.5 MHz for SS
+   operation and >= 30MHz for HS operation
+  "grf_clk"Controller grf clk
+
+Required child node:
+A child node must exist to represent the core DWC3 IP block. The name of
+the node is not important. The content of the node is defined in dwc3.txt.
+
+Phy documentation is provided in the following places:
+Documentation/devicetree/bindings/phy/rockchip,dwc3-usb-phy.txt
+
+Example device nodes:
+
+   usbdrd3_0: usb@fe80 {
+   compatible = "rockchip,rk3399-dwc3";
+   clocks = < SCLK_USB3OTG0_REF>, < SCLK_USB3OTG0_SUSPEND>,
+< ACLK_USB3OTG0>, < ACLK_USB3_GRF>;
+   clock-names = "ref_clk", "suspend_clk",
+ "bus_clk", "grf_clk";
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+   status = "disabled";
+   usbdrd_dwc3_0: dwc3@fe80 {
+   compatible = "snps,dwc3";
+   reg = <0x0 0xfe80 0x0 0x10>;
+   interrupts = ;
+   dr_mode = "otg";
+   status = "disabled";
+   };
+   };
+
+   usbdrd3_1: usb@fe90 {
+   compatible = "rockchip,rk3399-dwc3";
+   clocks = < SCLK_USB3OTG1_REF>, < SCLK_USB3OTG1_SUSPEND>,
+< ACLK_USB3OTG1>, < ACLK_USB3_GRF>;
+   clock-names = "ref_clk", "suspend_clk",
+ "bus_clk", "grf_clk";
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+   status = "disabled";
+   usbdrd_dwc3_1: dwc3@fe90 {
+   compatible = "snps,dwc3";
+   reg = <0x0 0xfe90 0x0 0x10>;
+   interrupts = ;
+   dr_mode = "otg";
+   status = "disabled";
+   };
+   };
-- 
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: [PATCH 1/2] usb: dwc3: pci: runtime_resume child device

[Felipe Balbi]

Hi,

Sergei Shtylyov  writes:
>> During runtime_resume of dwc3-pci.c, we need to
>> runtime our child device (which is dwc3 proper)
>
> Since when "runtime" is a verb? :-)


seems like you need a new webster


I'll add the missing "suspend" there.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 0984/1285] Replace numeric parameter like 0444 with macro

[Michal Nazarewicz]

On Wed, Aug 03 2016, Oliver Neukum wrote:
> Before we think about that, the basic question whether
>
> S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH
>
> is clearer and easier to read than
>
> 0644
>
> must be decided. I would saz no, it is not.

I was about to write the same thing.

I dislike magic numbers just like the next guy, but this replaces
a compact representation of the permissions with a long string of hard
to read, awkwardly abbreviated strings.

On personal note, I can never remember whether ‘u’ means user and ‘o’
means other or ‘u’ means users and ‘o’ means ‘owner’.  In cited case
this is somehow averted because both USR and OTH are present, but what
does ‘S_IRWXU’ mean is a mystery to me.

To my mind, the macros make sense only when testing for particular bit
being set.  Something like:

if (mode & S_IRUSR && check_if_user_can_read())
success;

could be argued as better than ‘mode & 0400’ but even than the awkward
abbreviation doesn’t help.  Again, ‘PERM_USER_READABLE’ would be much
better (also for the reason mentioned above).

-- 
Best regards
ミハウ “퓶퓲퓷퓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: dwc3: pci: runtime_resume child device

[Sergei Shtylyov]

Hello.

On 8/2/2016 2:03 PM, Felipe Balbi wrote:


During runtime_resume of dwc3-pci.c, we need to
runtime our child device (which is dwc3 proper)


   Since when "runtime" is a verb? :-)


otherwise nothing will happen.

Signed-off-by: Felipe Balbi 

[...]

MBR, 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: [PACTH v1] cdc-wdm: Clear read pipeline in case of error

[Oliver Neukum]

On Tue, 2016-08-02 at 10:37 -0400, Robert Foss wrote:
> 
> On 2016-08-02 09:59 AM, Oliver Neukum wrote:
> > On Tue, 2016-08-02 at 09:54 -0400, Robert Foss wrote:
> >>
> >> On 2016-08-02 08:23 AM, Oliver Neukum wrote:
> >>> On Thu, 2016-07-28 at 14:19 -0400, robert.f...@collabora.com wrote:
>  From: Prathmesh Prabhu 
> 
>  Implemented queued response handling. This queue is processed every
>  time the
>  WDM_READ flag is cleared.
> 
>  In case of a read error, userspace may not actually read the data,
>  since the
>  driver returns an error through wdm_poll. After this, the underlying
>  device may
>  attempt to send us more data, but the queue is not processed. While
>  userspace is
>  also blocked, because the read error is never cleared.
> >>>
> >>> Could you explain why user space cannot just read more data?
> >>> That will clear the error.
> >>
> >> Userspace certainly could read more data, but for the case when
> >> userspace doesn't read and clear a potential an error, we still would
> >> like to not be stuck if the device sends more data. space
> >>
> >> I hope that answers your question, if not I'll try to be more elaborate.
> >
> > Clear, but why does that require the suppression of an error condition?
> > errors should always be delivered.
> 
> The goal is not to clear the error condition, but that is required to 
> not stay stuck.

How can that depend on what we return to user space?
In the driver we can continue just ignoring errors.
Now, if user space stops reading because we reported an error,
that is the decision user space has made. We cannot ignore errors
in the kernel because we don't like what user space does when it
sees the error.

Regards
Oliver


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

Re: [PATCH 2/2] usb: dwc3: core: allow device to runtime_suspend several times

[Felipe Balbi]

Hi,

Alan Stern  writes:
>> After going through runtime_suspend/runtime_resume
>> cycle once we would be left with an unbalanced
>> pm_runtime_get() call. Fix that by making sure that
>> we try to suspend right after resuming so things are
>> balanced and device can runtime_suspend again.
>> 
>> Signed-off-by: Felipe Balbi 
>> ---
>>  drivers/usb/dwc3/core.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 946643157b78..35d092456bec 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1192,6 +1192,7 @@ static int dwc3_runtime_resume(struct device *dev)
>>  }
>>  
>>  pm_runtime_mark_last_busy(dev);
>> +pm_runtime_put(dev);
>>  
>>  return 0;
>>  }
>
> This may be correct, but it certainly looks odd.  For example, it 
> wouldn't work right if you ever called pm_runtime_resume() instead of 
> pm_runtime_get().

well, we don't. But is there an alternative for this?

-- 
balbi


signature.asc
Description: PGP signature

[PATCH v2] usb: ehci: change order of register cleanup during shutdown

[Marc Ohlf]

In ehci_turn_off_all_ports() all EHCI port registers are cleared to zero.
On some hardware, this can lead to an system hang,
when ehci_port_power() accesses the already cleared registers.

This patch changes the order of cleanup.
First call ehci_port_power() which respects the current bits in
port status registers
and afterwards cleanup the hard way by setting everything to zero.

Signed-off-by: Marc Ohlf 
Acked-by: Alan Stern 
CC: 
---

v2: Corrected description, marked for the -stable kernels.

 drivers/usb/host/ehci-hcd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index a962b89..1e5f529 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -332,11 +332,11 @@ static void ehci_turn_off_all_ports(struct ehci_hcd *ehci)
int port = HCS_N_PORTS(ehci->hcs_params);
 
while (port--) {
-   ehci_writel(ehci, PORT_RWC_BITS,
-   >regs->port_status[port]);
spin_unlock_irq(>lock);
ehci_port_power(ehci, port, false);
spin_lock_irq(>lock);
+   ehci_writel(ehci, PORT_RWC_BITS,
+   >regs->port_status[port]);
}
 }
 
-- 
1.9.1

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

Re: [RFC PATCH 3/4] usb: typec: USB Type-C Port Manager (tcpm)

[Oliver Neukum]

On Tue, 2016-08-02 at 13:32 -0700, Guenter Roeck wrote:
> +static bool svdm_consume_svids(struct tcpm_port *port, const u32
> *payload,
> +  int cnt)
> +{
> +   struct pd_mode_data *pmdata = >mode_data;
> +   int i;
> +
> +   for (i = 1; i < cnt; i++) {
> +   u16 svid;
> +
> +   svid = (payload[i] >> 16) & 0x;
> +   if (!svid)
> +   return false;

Hi,

this looks like an endianness bug.

Regards
Oliver


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

Re: [RFC PATCH 3/4] usb: typec: USB Type-C Port Manager (tcpm)

[Oliver Neukum]

On Tue, 2016-08-02 at 13:32 -0700, Guenter Roeck wrote:
> +static int tcpm_set_polarity(struct tcpm_port *port,
> +enum typec_cc_polarity polarity)
> +{
> +   tcpm_log(port, "polarity %d", polarity);
> +
> +   port->polarity = polarity;
> +
> +   return port->tcpc->set_polarity(port->tcpc, port->polarity);
> +}

Here you don't care about the result.

> +
> +static int tcpm_set_vconn(struct tcpm_port *port, bool enable)
> +{
> +   int ret = 0;
> +
> +   tcpm_log(port, "vconn:=%d", enable);
> +
> +   ret = port->tcpc->set_vconn(port->tcpc, enable);
> +   if (!ret) {
> +   port->con.vconn_role = enable ? TYPEC_SOURCE
> + : TYPEC_SINK;
> +   typec_set_vconn_role(port->typec_port,
> port->con.vconn_role);
> +   }
> +
> +   return ret;
> +}

But here you do. Which is right?

Regards
Oliver


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

Re: usbtmc: vendor specific i/o

[Ladislav Michl]

On Wed, Aug 03, 2016 at 05:59:59AM +0200, Greg Kroah-Hartman wrote:
> But your patch was "one-way", once you switched to the other mode, the
> old one could not be used :(

Yes, also was lacking proper description and signoff. So, I'm considering
ioctls based approach okay, although that question (the only one I really
had) was never answered.

After re-reading specifications [*] I decided to allow arbitrary MsgID
selection, as USB488 adds MsgID=TRIGGER (128) and other subclass
specifications may add other values.

[*] http://www.usb.org/developers/docs/devclass_docs/USBTMC_1_006a.zip

After sorting out all eventual objections, patch bellow will be turned
into proper one.

Thank you,
ladis

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 917a55c..c6d3ca3 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -84,6 +84,9 @@ struct usbtmc_device_data {
unsigned int bulk_in;
unsigned int bulk_out;
 
+   u8 msgid_in;
+   u8 msgid_out;
+
u8 bTag;
u8 bTag_last_write; /* needed for abort */
u8 bTag_last_read;  /* needed for abort */
@@ -393,6 +396,20 @@ exit:
return rv;
 }
 
+static int usbtmc_ioctl_set_msgids(struct usbtmc_device_data *data,
+   void __user *arg)
+{
+   struct usbtmc_msg_ids val;
+
+   if (copy_from_user(, arg, sizeof(val)))
+   return -EFAULT;
+
+   data->msgid_in = val.in;
+   data->msgid_out = val.out;
+
+   return 0;
+}
+
 static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
void __user *arg)
 {
@@ -549,7 +566,7 @@ static int send_request_dev_dep_msg_in(struct 
usbtmc_device_data *data, size_t t
/* Setup IO buffer for REQUEST_DEV_DEP_MSG_IN message
 * Refer to class specs for details
 */
-   buffer[0] = 2;
+   buffer[0] = data->msgid_in;
buffer[1] = data->bTag;
buffer[2] = ~data->bTag;
buffer[3] = 0; /* Reserved */
@@ -677,8 +694,8 @@ static ssize_t usbtmc_read(struct file *filp, char __user 
*buf,
goto exit;
}
 
-   if (buffer[0] != 2) {
-   dev_err(dev, "Device sent reply with wrong 
MsgID: %u != 2\n", buffer[0]);
+   if (buffer[0] != data->msgid_in) {
+   dev_err(dev, "Device sent reply with wrong 
MsgID: %u != %u\n", buffer[0], data->msgid_in);
if (data->auto_abort)
usbtmc_ioctl_abort_bulk_in(data);
goto exit;
@@ -807,7 +824,7 @@ static ssize_t usbtmc_write(struct file *filp, const char 
__user *buf,
}
 
/* Setup IO buffer for DEV_DEP_MSG_OUT message */
-   buffer[0] = 1;
+   buffer[0] = data->msgid_out;
buffer[1] = data->bTag;
buffer[2] = ~data->bTag;
buffer[3] = 0; /* Reserved */
@@ -1227,6 +1244,10 @@ static long usbtmc_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
retval = usbtmc_ioctl_abort_bulk_in(data);
break;
 
+   case USBTMC_IOCTL_SET_MSGIDS:
+   retval = usbtmc_ioctl_set_msgids(data, (void __user *)arg);
+   break;
+
case USBTMC488_IOCTL_GET_CAPS:
retval = copy_to_user((void __user *)arg,
>usb488_caps,
@@ -1409,6 +1430,8 @@ static int usbtmc_probe(struct usb_interface *intf,
}
 
/* Initialize USBTMC bTag and other fields */
+   data->msgid_in  = 2;
+   data->msgid_out = 1;
data->bTag  = 1;
data->TermCharEnabled = 0;
data->TermChar = '\n';
diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h
index 2e59d9c..46f21cc 100644
--- a/include/uapi/linux/usb/tmc.h
+++ b/include/uapi/linux/usb/tmc.h
@@ -37,6 +37,12 @@
 #define USBTMC488_REQUEST_GOTO_LOCAL   161
 #define USBTMC488_REQUEST_LOCAL_LOCKOUT162
 
+/* USB TMC command messages ids */
+struct usbtmc_msg_ids {
+   __u8 in;
+   __u8 out;
+};
+
 /* Request values for USBTMC driver's ioctl entry point */
 #define USBTMC_IOC_NR  91
 #define USBTMC_IOCTL_INDICATOR_PULSE   _IO(USBTMC_IOC_NR, 1)
@@ -45,6 +51,7 @@
 #define USBTMC_IOCTL_ABORT_BULK_IN _IO(USBTMC_IOC_NR, 4)
 #define USBTMC_IOCTL_CLEAR_OUT_HALT_IO(USBTMC_IOC_NR, 6)
 #define USBTMC_IOCTL_CLEAR_IN_HALT _IO(USBTMC_IOC_NR, 7)
+#define USBTMC_IOCTL_SET_MSGIDS_IOW(USBTMC_IOC_NR, 8, struct 
usbtmc_msg_ids)
 #define USBTMC488_IOCTL_GET_CAPS   _IOR(USBTMC_IOC_NR, 17, unsigned char)
 #define USBTMC488_IOCTL_READ_STB   _IOR(USBTMC_IOC_NR, 18, unsigned char)
 #define USBTMC488_IOCTL_REN_CONTROL_IOW(USBTMC_IOC_NR, 19, unsigned char)
--
To unsubscribe 

Re: [PATCH] musb: omap2430: do not assume balanced enable()/disable()

[Tony Lindgren]

* Andreas Kemnade  [160802 08:14]:
> On Tue, 2 Aug 2016 03:33:34 -0700
> Tony Lindgren  wrote:
> 
> > * Andreas Kemnade  [160729 11:14]:
> > > The code assumes that omap2430_musb_enable() and
> > > omap2430_musb_disable() is called in a balanced way. The
> > > That fact is broken by the fact that musb_init_controller() calls
> > > musb_platform_disable() to switch from unknown state to off state.
> > 
> > OK, some spelling issues with the above paragraph though :)
> > 
> > > That means that phy_power_off() is called first so that
> > > phy->power_count gets -1 and the phy is not enabled on
> > > phy_power_on(). In the probably common case of using the
> > > phy_twl4030, that prevents also charging the battery and so makes
> > > further kernel debugging hard.
> > 
> > Is this with v4.7 kernel? Also, care to describe how you hit this
> > and on which hardware? Just wondering..
> 
> I got this error on the Openphoenux GTA04 phone. It has a DM3730
> SoC and a TPS65950 companion. Severe charging problems were already
> observed with the 4.4rc1. I do not know if that already was exactly
> *this* problem. I have debugged and patched the v4.7 kernel.

OK thanks for the info.

> How I hit the problem: Just boot an that device and try to charge
> via usb.

OK so it's the twl4030 charger then I guess.

> Should I resubmit the patch with an extended commit message?

Well yeah it might be worth describing that it's the twl4030
charger that otherwise does not work properly.

Regards,

Tony
--
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