[PATCH] dt-bindings: usb: atmel: fix a couple of copy-paste style typos

[Peter Rosin]

Signed-off-by: Peter Rosin 
---
 Documentation/devicetree/bindings/usb/atmel-usb.txt | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/atmel-usb.txt 
b/Documentation/devicetree/bindings/usb/atmel-usb.txt
index f4262ed60582..ad8ea56a9ed3 100644
--- a/Documentation/devicetree/bindings/usb/atmel-usb.txt
+++ b/Documentation/devicetree/bindings/usb/atmel-usb.txt
@@ -6,9 +6,9 @@ Required properties:
  - compatible: Should be "atmel,at91rm9200-ohci" for USB controllers
used in host mode.
  - reg: Address and length of the register set for the device
- - interrupts: Should contain ehci interrupt
+ - interrupts: Should contain ohci interrupt
  - clocks: Should reference the peripheral, host and system clocks
- - clock-names: Should contains two strings
+ - clock-names: Should contain three strings
"ohci_clk" for the peripheral clock
"hclk" for the host clock
"uhpck" for the system clock
@@ -35,7 +35,7 @@ Required properties:
  - reg: Address and length of the register set for the device
  - interrupts: Should contain ehci interrupt
  - clocks: Should reference the peripheral and the UTMI clocks
- - clock-names: Should contains two strings
+ - clock-names: Should contain two strings
"ehci_clk" for the peripheral clock
"usb_clk" for the UTMI clock
 
@@ -58,7 +58,7 @@ Required properties:
  - reg: Address and length of the register set for the device
  - interrupts: Should contain macb interrupt
  - clocks: Should reference the peripheral and the AHB clocks
- - clock-names: Should contains two strings
+ - clock-names: Should contain two strings
"pclk" for the peripheral clock
"hclk" for the AHB clock
 
@@ -85,7 +85,7 @@ Required properties:
  - reg: Address and length of the register set for the device
  - interrupts: Should contain usba interrupt
  - clocks: Should reference the peripheral and host clocks
- - clock-names: Should contains two strings
+ - clock-names: Should contain two strings
"pclk" for the peripheral clock
"hclk" for the host clock
  - ep childnode: To specify the number of endpoints and their properties.
-- 
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: usb_ep_{dis,en}able() calling context (was: Re: [PATCH 2/3] usb: dwc3: gadget: wait for End Transfer to complete)

[Baolin Wang]

Hi,

On 18 October 2016 at 16:21, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> Sure. The problem I met was, when we change the USB function with
>> configfs frequently, sometimes it will hang the system to crash. The
>> reason is,  we will start end transfer command when disable the
>> endpoint, but sometimes the end transfer command complete event comes
>> after we have freed the gadget irq (since the xHCI will share the same
>> interrupt number with gadget, thus when free the gadget irq, it will
>> not shutdown this gadget irq line), it will trigger the interrupt line
>> all the time.
>
> okay, thanks for describing it again. Another option would be to only
> wait_for_completion() before calling free_irq() is any endpoint has
> END_TRANSFER_PENDING flag set. Something like below:

I tested below patch, but it failed and I still met the kernel crash
with changing USB function.

>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 5fc437021ac7..0cb3b78d28b7 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -505,6 +506,7 @@ struct dwc3_event_buffer {
>   * @endpoint: usb endpoint
>   * @pending_list: list of pending requests for this endpoint
>   * @started_list: list of started requests on this endpoint
> + * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete
>   * @lock: spinlock for endpoint request queue traversal
>   * @regs: pointer to first endpoint register
>   * @trb_pool: array of transaction buffers
> @@ -530,6 +532,8 @@ struct dwc3_ep {
> struct list_headpending_list;
> struct list_headstarted_list;
>
> +   wait_queue_head_t   wait_end_transfer;
> +
> spinlock_t  lock;
> void __iomem*regs;
>
> @@ -546,6 +550,7 @@ struct dwc3_ep {
>  #define DWC3_EP_BUSY   (1 << 4)
>  #define DWC3_EP_PENDING_REQUEST(1 << 5)
>  #define DWC3_EP_MISSED_ISOC(1 << 6)
> +#define DWC3_EP_END_TRANSFER_PENDING (1 << 7)
>
> /* This last one is specific to EP0 */
>  #define DWC3_EP0_DIR_IN(1 << 31)
> @@ -1050,6 +1055,9 @@ struct dwc3_event_depevt {
>  #define DEPEVT_TRANSFER_BUS_EXPIRY 2
>
> u32 parameters:16;
> +
> +/* For Command Complete Events */
> +#define DEPEVT_PARAMETER_CMD(n) (((n) & (0xf << 8)) >> 8)
>  } __packed;
>
>  /**
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 3b53a5714df4..5929a75b3455 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -598,6 +598,8 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> reg |= DWC3_DALEPENA_EP(dep->number);
> dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
>
> +   init_waitqueue_head(>wait_end_transfer);
> +
> if (usb_endpoint_xfer_control(desc))
> return 0;
>
> @@ -1783,12 +1785,28 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>
>  static void __dwc3_gadget_stop(struct dwc3 *dwc)
>  {
> +   int epnum;
> +
> if (pm_runtime_suspended(dwc->dev))
> return;
>
> dwc3_gadget_disable_irq(dwc);
> __dwc3_gadget_ep_disable(dwc->eps[0]);
> __dwc3_gadget_ep_disable(dwc->eps[1]);
> +
> +   for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
> +   struct dwc3_ep  *dep = dwc->eps[epnum];
> +
> +   if (!(dep->flags & DWC3_EP_ENABLED))
> +   continue;

We should not check the DWC3_EP_ENABLED flag, since we will clear all
flags in __dwc3_gadget_ep_disable() after setting
DWC3_EP_END_TRANSFER_PENDING flags in dwc3_stop_active_transfer().

> +
> +   if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING))
> +   continue;

Ditto.

> +
> +   wait_event_lock_irq(dep->wait_end_transfer,
> +   !(dep->flags & DWC3_EP_END_TRANSFER_PENDING),
> +   dwc->lock);
> +   }
>  }
>
>  static int dwc3_gadget_stop(struct usb_gadget *g)
> @@ -2171,6 +2189,7 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
>  {
> struct dwc3_ep  *dep;
> u8  epnum = event->endpoint_number;
> +   u8  cmd;

Here we should add below modification, or the event can not be handled.

- if (!(dep->flags & DWC3_EP_ENABLED))
+ if (!(dep->flags & DWC3_EP_ENABLED) &&
+ !(dep->flags & DWC3_EP_END_TRANSFER_PENDING))
return;

>
> dep = dwc->eps[epnum];
>
> @@ -2215,8 +2234,15 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
> return;
> }
> break;
> -   case DWC3_DEPEVT_RXTXFIFOEVT:
> case DWC3_DEPEVT_EPCMDCMPLT:
> +   cmd = 

Re: [PATCH] dt-bindings: usb: atmel: fix a couple of copy-paste style typos

[Rob Herring]

On Tue, Oct 18, 2016 at 01:05:18PM +0200, Peter Rosin wrote:
> Signed-off-by: Peter Rosin 
> ---
>  Documentation/devicetree/bindings/usb/atmel-usb.txt | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

Acked-by: Rob Herring 
--
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 v18 2/4] usb: gadget: Support for the usb charger framework

[Baolin Wang]

For supporting the usb charger, it adds the usb_charger_init() and
usb_charger_exit() functions for usb charger initialization and exit.

It will report to the usb charger when the gadget state is changed,
then the usb charger can do the power things.

Signed-off-by: Baolin Wang 
Reviewed-by: Li Jun 
Tested-by: Li Jun 
---
 drivers/usb/gadget/udc/core.c |   19 ++-
 include/linux/usb/gadget.h|3 +++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 9483489..90df022 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -576,12 +577,17 @@ EXPORT_SYMBOL_GPL(usb_gadget_vbus_connect);
  * reporting how much power the device may consume.  For example, this
  * could affect how quickly batteries are recharged.
  *
+ * It will also notify the USB charger how much power the device may
+ * consume if there is a USB charger linking with the gadget.
+ *
  * Returns zero on success, else negative errno.
  */
 int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
 {
int ret = 0;
 
+   usb_charger_set_cur_limit_by_gadget(gadget, mA);
+
if (!gadget->ops->vbus_draw) {
ret = -EOPNOTSUPP;
goto out;
@@ -963,6 +969,9 @@ static void usb_gadget_state_work(struct work_struct *work)
struct usb_gadget *gadget = work_to_gadget(work);
struct usb_udc *udc = gadget->udc;
 
+   /* when the gadget state is changed, then report to USB charger */
+   usb_charger_plug_by_gadget(gadget, gadget->state);
+
if (udc)
sysfs_notify(>dev.kobj, NULL, "state");
 }
@@ -1132,6 +1141,10 @@ int usb_add_gadget_udc_release(struct device *parent, 
struct usb_gadget *gadget,
if (ret)
goto err4;
 
+   ret = usb_charger_init(gadget);
+   if (ret)
+   goto err5;
+
usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
udc->vbus = true;
 
@@ -1143,7 +1156,7 @@ int usb_add_gadget_udc_release(struct device *parent, 
struct usb_gadget *gadget,
if (ret != -EPROBE_DEFER)
list_del(>pending);
if (ret)
-   goto err5;
+   goto err6;
break;
}
}
@@ -1152,6 +1165,9 @@ int usb_add_gadget_udc_release(struct device *parent, 
struct usb_gadget *gadget,
 
return 0;
 
+err6:
+   usb_charger_exit(gadget);
+
 err5:
device_del(>dev);
 
@@ -1263,6 +1279,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
kobject_uevent(>dev.kobj, KOBJ_REMOVE);
flush_work(>work);
device_unregister(>dev);
+   usb_charger_exit(gadget);
device_unregister(>dev);
 }
 EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 8e81f9e..82a0d63 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define UDC_TRACE_STR_MAX  512
 
@@ -328,6 +329,7 @@ struct usb_gadget_ops {
  * @in_epnum: last used in ep number
  * @mA: last set mA value
  * @otg_caps: OTG capabilities of this gadget.
+ * @charger: Negotiate the power with the usb charger.
  * @sg_supported: true if we can handle scatter-gather
  * @is_otg: True if the USB device port uses a Mini-AB jack, so that the
  * gadget driver must provide a USB OTG descriptor.
@@ -387,6 +389,7 @@ struct usb_gadget {
unsignedin_epnum;
unsignedmA;
struct usb_otg_caps *otg_caps;
+   struct usb_charger  *charger;
 
unsignedsg_supported:1;
unsignedis_otg:1;
-- 
1.7.9.5

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

[PATCH v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

Currently the Linux kernel does not provide any standard integration of this
feature that integrates the USB subsystem with the system power regulation
provided by PMICs meaning that either vendors must add this in their kernels
or USB gadget devices based on Linux (such as mobile phones) may not behave
as they should. Thus provide a standard framework for doing this in kernel.

Now introduce one user with wm831x_power to support and test the usb charger,
which is pending testing. Moreover there may be other potential users will use
it in future.

Changes since v17:
 - Remove goto section in usb_charger_register() function.
 - Remove 'extern' in charger.h file.
 - Move the kfree() to usb_charger_exit() function.

Changes since v16:
 - Modify the charger current range with introducing the maximum and minimum
 current.
 - Remove the getting charger type method from power supply.
 - Add the getting charger type method from extcon system.
 - Introduce new usb_charger_get_current() API for users to get the maximum and
 minimum current.
 - Rename some APIs and other optimization.

Changes since v15:
 - Add charger state checking to avoid sending out duplicate notifies to users.
 - Add one work to notify power users the current has been changed.

Changes since v14:
 - Add kernel documentation for struct usb_cahrger.
 - Remove some redundant WARN() functions.

Changes since v13:
 - Remove the charger checking in usb_gadget_vbus_draw() function.
 - Rename some functions in charger.c file.
 - Rebase on git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
tags/usb-for-v4.8

Changes since v12:
 - Remove the class and device things.
 - Link usb charger to udc-core.ko.
 - Create one "charger" subdirectory which holds all charger-related attributes.

Changes since v11:
 - Reviewed and tested by Li Jun.

Changes since v10:
 - Introduce usb_charger_get_state() function to check charger state.
 - Remove the mutex lock in usb_charger_set_cur_limit_by_type() function
 in case will be issued in atomic context.

Baolin Wang (4):
  usb: gadget: Introduce the usb charger framework
  usb: gadget: Support for the usb charger framework
  usb: gadget: Integrate with the usb gadget supporting for usb charger
  power: wm831x_power: Support USB charger current limit management

 drivers/power/wm831x_power.c |   75 
 drivers/usb/gadget/Kconfig   |8 +
 drivers/usb/gadget/udc/Makefile  |1 +
 drivers/usb/gadget/udc/charger.c |  877 ++
 drivers/usb/gadget/udc/core.c|   19 +-
 include/linux/mfd/wm831x/pdata.h |3 +
 include/linux/usb/charger.h  |  185 
 include/linux/usb/gadget.h   |3 +
 include/uapi/linux/usb/charger.h |   31 ++
 9 files changed, 1201 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/gadget/udc/charger.c
 create mode 100644 include/linux/usb/charger.h
 create mode 100644 include/uapi/linux/usb/charger.h

-- 
1.7.9.5

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

[PATCH v18 4/4] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

Integrate with the newly added USB charger interface to limit the current
we draw from the USB input based on the input device configuration
identified by the USB stack, allowing us to charge more quickly from high
current inputs without drawing more current than specified from others.

Signed-off-by: Mark Brown 
Signed-off-by: Baolin Wang 
Acked-by: Lee Jones 
Acked-by: Charles Keepax 
Acked-by: Peter Chen 
Acked-by: Sebastian Reichel 
---
 drivers/power/wm831x_power.c |   75 ++
 include/linux/mfd/wm831x/pdata.h |3 ++
 2 files changed, 78 insertions(+)

diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
index 7082301..2bcd775 100644
--- a/drivers/power/wm831x_power.c
+++ b/drivers/power/wm831x_power.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -31,6 +32,8 @@ struct wm831x_power {
char usb_name[20];
char battery_name[20];
bool have_battery;
+   struct usb_charger *usb_charger;
+   struct notifier_block usb_notify;
 };
 
 static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
@@ -125,6 +128,49 @@ static enum power_supply_property wm831x_usb_props[] = {
POWER_SUPPLY_PROP_VOLTAGE_NOW,
 };
 
+/* In milliamps */
+static const unsigned int wm831x_usb_limits[] = {
+   0,
+   2,
+   100,
+   500,
+   900,
+   1500,
+   1800,
+   550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,
+  unsigned long state, void *data)
+{
+   struct wm831x_power *wm831x_power = container_of(nb,
+struct wm831x_power,
+usb_notify);
+   struct usb_charger *uchger = (struct usb_charger *)data;
+   unsigned int i, best, min, limit;
+
+   if (state == USB_CHARGER_PRESENT)
+   usb_charger_get_current(uchger, , );
+   else
+   limit = 0;
+
+   /* Find the highest supported limit */
+   best = 0;
+   for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+   if (limit >= wm831x_usb_limits[i] &&
+   wm831x_usb_limits[best] < wm831x_usb_limits[i])
+   best = i;
+   }
+
+   dev_dbg(wm831x_power->wm831x->dev,
+   "Limiting USB current to %umA", wm831x_usb_limits[best]);
+
+   wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
+   WM831X_USB_ILIM_MASK, best);
+
+   return 0;
+}
+
 /*
  * Battery properties
  */
@@ -607,8 +653,31 @@ static int wm831x_power_probe(struct platform_device *pdev)
}
}
 
+   if (wm831x_pdata && wm831x_pdata->usb_gadget) {
+   power->usb_charger =
+   usb_charger_find_by_name(wm831x_pdata->usb_gadget);
+   if (IS_ERR(power->usb_charger)) {
+   ret = PTR_ERR(power->usb_charger);
+   dev_err(>dev,
+   "Failed to find USB gadget: %d\n", ret);
+   goto err_bat_irq;
+   }
+
+   power->usb_notify.notifier_call = wm831x_usb_limit_change;
+
+   ret = usb_charger_register_notify(power->usb_charger,
+ >usb_notify);
+   if (ret != 0) {
+   dev_err(>dev,
+   "Failed to register notifier: %d\n", ret);
+   goto err_usb_charger;
+   }
+   }
+
return ret;
 
+err_usb_charger:
+   /* put_device on charger */
 err_bat_irq:
--i;
for (; i >= 0; i--) {
@@ -637,6 +706,12 @@ static int wm831x_power_remove(struct platform_device 
*pdev)
struct wm831x *wm831x = wm831x_power->wm831x;
int irq, i;
 
+   if (wm831x_power->usb_charger) {
+   usb_charger_unregister_notify(wm831x_power->usb_charger,
+ _power->usb_notify);
+   /* Free charger */
+   }
+
for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
irq = wm831x_irq(wm831x, 
 platform_get_irq_byname(pdev,
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index dcc9631..5af8399 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -126,6 +126,9 @@ struct wm831x_pdata {
/** The driver should initiate a power off sequence during shutdown */
bool soft_shutdown;
 
+   /** dev_name of USB charger gadget to 

[PATCH v18 3/4] usb: gadget: Integrate with the usb gadget supporting for usb charger

[Baolin Wang]

When the usb gadget supporting for usb charger is ready, the usb charger
can implement the usb_charger_plug_by_gadget() function, usb_charger_exit()
function and dev_to_uchger() function by getting 'struct usb_charger' from
'struct gadget'.

Signed-off-by: Baolin Wang 
Reviewed-by: Li Jun 
Tested-by: Li Jun 
---
 drivers/usb/gadget/udc/charger.c |   97 +-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/charger.c b/drivers/usb/gadget/udc/charger.c
index d885e86..56aee28 100644
--- a/drivers/usb/gadget/udc/charger.c
+++ b/drivers/usb/gadget/udc/charger.c
@@ -38,7 +38,9 @@ static DEFINE_MUTEX(charger_lock);
 
 static struct usb_charger *dev_to_uchger(struct device *dev)
 {
-   return NULL;
+   struct usb_gadget *gadget = container_of(dev, struct usb_gadget, dev);
+
+   return gadget->charger;
 }
 
 /*
@@ -308,6 +310,18 @@ static int __usb_charger_set_cur_limit_by_type(struct 
usb_charger *uchger,
 int usb_charger_set_cur_limit_by_gadget(struct usb_gadget *gadget,
unsigned int cur_limit)
 {
+   struct usb_charger *uchger = gadget->charger;
+   int ret;
+
+   if (!uchger)
+   return -EINVAL;
+
+   ret = __usb_charger_set_cur_limit_by_type(uchger, uchger->type,
+ cur_limit);
+   if (ret)
+   return ret;
+
+   schedule_work(>work);
return 0;
 }
 EXPORT_SYMBOL_GPL(usb_charger_set_cur_limit_by_gadget);
@@ -625,11 +639,66 @@ static int usb_charger_plug_by_extcon(struct 
notifier_block *nb,
 int usb_charger_plug_by_gadget(struct usb_gadget *gadget,
   unsigned long state)
 {
+   struct usb_charger *uchger = gadget->charger;
+   enum usb_charger_state uchger_state;
+
+   if (WARN(!uchger, "charger can not be NULL"))
+   return -EINVAL;
+
+   /*
+* Report event to power to setting the current limitation
+* for this usb charger when one usb charger state is changed
+* with detecting by usb gadget state.
+*/
+   if (uchger->old_gadget_state != state) {
+   uchger->old_gadget_state = state;
+
+   if (state >= USB_STATE_ATTACHED) {
+   uchger_state = USB_CHARGER_PRESENT;
+   } else if (state == USB_STATE_NOTATTACHED) {
+   mutex_lock(>lock);
+
+   /*
+* Need check the charger type to make sure the usb
+* cable is removed, in case it just changes the usb
+* function with configfs.
+*/
+   uchger->type = __usb_charger_get_type(uchger);
+   if (uchger->type != UNKNOWN_TYPE) {
+   mutex_unlock(>lock);
+   return 0;
+   }
+
+   mutex_unlock(>lock);
+   uchger_state = USB_CHARGER_REMOVE;
+   } else {
+   uchger_state = USB_CHARGER_DEFAULT;
+   }
+
+   usb_charger_notify_state(uchger, uchger_state);
+   }
+
return 0;
 }
 EXPORT_SYMBOL_GPL(usb_charger_plug_by_gadget);
 
 /*
+ * usb_charger_unregister() - Unregister a usb charger.
+ * @uchger - the usb charger to be unregistered.
+ */
+static int usb_charger_unregister(struct usb_charger *uchger)
+{
+   ida_simple_remove(_charger_ida, uchger->id);
+   sysfs_remove_groups(>gadget->dev.kobj, usb_charger_groups);
+
+   mutex_lock(_lock);
+   list_del(>list);
+   mutex_unlock(_lock);
+
+   return 0;
+}
+
+/*
  * usb_charger_register() - Register a new usb charger.
  * @uchger - the new usb charger instance.
  */
@@ -737,6 +806,7 @@ int usb_charger_init(struct usb_gadget *ugadget)
}
 
uchger->gadget = ugadget;
+   ugadget->charger = uchger;
uchger->old_gadget_state = USB_STATE_NOTATTACHED;
 
/* Register a new usb charger */
@@ -774,6 +844,31 @@ fail_extcon:
 
 int usb_charger_exit(struct usb_gadget *ugadget)
 {
+   struct usb_charger *uchger = ugadget->charger;
+
+   if (!uchger)
+   return -EINVAL;
+
+   usb_charger_unregister(uchger);
+   ugadget->charger = NULL;
+
+   extcon_unregister_notifier(uchger->extcon_dev,
+  EXTCON_USB,
+  >extcon_nb.nb);
+   extcon_unregister_notifier(uchger->extcon_dev,
+  EXTCON_CHG_USB_SDP,
+  >extcon_type_nb.nb);
+   extcon_unregister_notifier(uchger->extcon_dev,
+  EXTCON_CHG_USB_CDP,
+  >extcon_type_nb.nb);
+   extcon_unregister_notifier(uchger->extcon_dev,
+

[PATCH v18 1/4] usb: gadget: Introduce the usb charger framework

[Baolin Wang]

This patch introduces the usb charger driver based on usb gadget that
makes an enhancement to a power driver. It works well in practice but
that requires a system with suitable hardware.

The basic conception of the usb charger is that, when one usb charger
is added or removed by reporting from the usb gadget state change or
the extcon device state change, the usb charger will report to power
user to set the current limitation.

The usb charger will register notifiees on the usb gadget or the extcon
device to get notified the usb charger state. It also supplies the
notification mechanism to userspace When the usb charger state is changed.

Power user will register a notifiee on the usb charger to get notified
by status changes from the usb charger. It will report to power user
to set the current limitation when detecting the usb charger is added
or removed from extcon device state or usb gadget state.

This patch doesn't yet integrate with the gadget code, so some functions
which rely on the 'gadget' are not completed, that will be implemented
in the following patches.

Signed-off-by: Baolin Wang 
Reviewed-by: Li Jun 
Tested-by: Li Jun 
---
 drivers/usb/gadget/Kconfig   |8 +
 drivers/usb/gadget/udc/Makefile  |1 +
 drivers/usb/gadget/udc/charger.c |  782 ++
 include/linux/usb/charger.h  |  185 +
 include/uapi/linux/usb/charger.h |   31 ++
 5 files changed, 1007 insertions(+)
 create mode 100644 drivers/usb/gadget/udc/charger.c
 create mode 100644 include/linux/usb/charger.h
 create mode 100644 include/uapi/linux/usb/charger.h

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 2ea3fc3..7520677 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -134,6 +134,14 @@ config U_SERIAL_CONSOLE
help
   It supports the serial gadget can be used as a console.
 
+config USB_CHARGER
+   bool "USB charger support"
+   select EXTCON
+   help
+ The usb charger driver based on the usb gadget that makes an
+ enhancement to a power driver which can set the current limitation
+ when the usb charger is added or removed.
+
 source "drivers/usb/gadget/udc/Kconfig"
 
 #
diff --git a/drivers/usb/gadget/udc/Makefile b/drivers/usb/gadget/udc/Makefile
index 98e74ed..ede2351 100644
--- a/drivers/usb/gadget/udc/Makefile
+++ b/drivers/usb/gadget/udc/Makefile
@@ -2,6 +2,7 @@
 CFLAGS_trace.o := -I$(src)
 
 udc-core-y := core.o trace.o
+udc-core-$(CONFIG_USB_CHARGER) += charger.o
 
 #
 # USB peripheral controller drivers
diff --git a/drivers/usb/gadget/udc/charger.c b/drivers/usb/gadget/udc/charger.c
new file mode 100644
index 000..d885e86
--- /dev/null
+++ b/drivers/usb/gadget/udc/charger.c
@@ -0,0 +1,782 @@
+/*
+ * charger.c -- USB charger driver
+ *
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Default current range by charger type. */
+#define DEFAULT_SDP_CUR_MIN2
+#define DEFAULT_SDP_CUR_MAX500
+#define DEFAULT_SDP_CUR_MIN_SS 150
+#define DEFAULT_SDP_CUR_MAX_SS 900
+#define DEFAULT_DCP_CUR_MIN500
+#define DEFAULT_DCP_CUR_MAX5000
+#define DEFAULT_CDP_CUR_MIN1500
+#define DEFAULT_CDP_CUR_MAX5000
+#define DEFAULT_ACA_CUR_MIN1500
+#define DEFAULT_ACA_CUR_MAX5000
+
+static DEFINE_IDA(usb_charger_ida);
+static LIST_HEAD(charger_list);
+static DEFINE_MUTEX(charger_lock);
+
+static struct usb_charger *dev_to_uchger(struct device *dev)
+{
+   return NULL;
+}
+
+/*
+ * charger_current_show() - Show the charger current.
+ */
+static ssize_t charger_current_show(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct usb_charger *uchger = dev_to_uchger(dev);
+   unsigned int min, max;
+
+   usb_charger_get_current(uchger, , );
+   return sprintf(buf, "%u-%u\n", min, max);
+}
+static DEVICE_ATTR_RO(charger_current);
+
+/*
+ * charger_type_show() - Show the charger type.
+ *
+ * It can be SDP/DCP/CDP/ACA type, else for unknown type.
+ */
+static ssize_t charger_type_show(struct device *dev,
+struct device_attribute *attr,
+char *buf)
+{
+   struct usb_charger *uchger = dev_to_uchger(dev);
+   int cnt;
+
+   switch (uchger->type) {
+   case SDP_TYPE:
+   cnt = sprintf(buf, "%s\n", "SDP");
+   break;
+   case DCP_TYPE:
+   cnt = sprintf(buf, "%s\n", "DCP");
+   break;
+   case CDP_TYPE:
+   cnt = 

[PATCH v7, 4/8] usb: Add MediaTek USB3 DRD driver

[Chunfeng Yun]

This patch adds support for the MediaTek USB3 controller
integrated into MT8173. It currently supports High-Speed
Peripheral Only mode.

Super-Speed Peripheral, Dual-Role Device and Host Only (xHCI)
modes will be added in the next patchs.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/Kconfig|2 +
 drivers/usb/Makefile   |1 +
 drivers/usb/mtu3/Kconfig   |   32 ++
 drivers/usb/mtu3/Makefile  |2 +
 drivers/usb/mtu3/mtu3.h|  341 
 drivers/usb/mtu3/mtu3_core.c   |  675 ++
 drivers/usb/mtu3/mtu3_gadget.c |  709 
 drivers/usb/mtu3/mtu3_gadget_ep0.c |  791 
 drivers/usb/mtu3/mtu3_hw_regs.h|  440 
 drivers/usb/mtu3/mtu3_plat.c   |  251 
 drivers/usb/mtu3/mtu3_qmu.c|  573 ++
 drivers/usb/mtu3/mtu3_qmu.h|   43 ++
 12 files changed, 3860 insertions(+)
 create mode 100644 drivers/usb/mtu3/Kconfig
 create mode 100644 drivers/usb/mtu3/Makefile
 create mode 100644 drivers/usb/mtu3/mtu3.h
 create mode 100644 drivers/usb/mtu3/mtu3_core.c
 create mode 100644 drivers/usb/mtu3/mtu3_gadget.c
 create mode 100644 drivers/usb/mtu3/mtu3_gadget_ep0.c
 create mode 100644 drivers/usb/mtu3/mtu3_hw_regs.h
 create mode 100644 drivers/usb/mtu3/mtu3_plat.c
 create mode 100644 drivers/usb/mtu3/mtu3_qmu.c
 create mode 100644 drivers/usb/mtu3/mtu3_qmu.h

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 644e978..fbe493d 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -95,6 +95,8 @@ source "drivers/usb/usbip/Kconfig"
 
 endif
 
+source "drivers/usb/mtu3/Kconfig"
+
 source "drivers/usb/musb/Kconfig"
 
 source "drivers/usb/dwc3/Kconfig"
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index dca7856..7791af6 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_USB_DWC2)+= dwc2/
 obj-$(CONFIG_USB_ISP1760)  += isp1760/
 
 obj-$(CONFIG_USB_MON)  += mon/
+obj-$(CONFIG_USB_MTU3) += mtu3/
 
 obj-$(CONFIG_PCI)  += host/
 obj-$(CONFIG_USB_EHCI_HCD) += host/
diff --git a/drivers/usb/mtu3/Kconfig b/drivers/usb/mtu3/Kconfig
new file mode 100644
index 000..54dadee
--- /dev/null
+++ b/drivers/usb/mtu3/Kconfig
@@ -0,0 +1,32 @@
+# For MTK USB3.0 IP
+
+config USB_MTU3
+   tristate "MediaTek USB3 Dual Role controller"
+   depends on (USB || USB_GADGET) && HAS_DMA
+   depends on ARCH_MEDIATEK || COMPILE_TEST
+   help
+ Say Y or M here if your system runs on MediaTek SoCs with
+ Dual Role SuperSpeed USB controller. You can select usb
+ mode as peripheral role or host role, or both.
+
+ If you don't know what this is, please say N.
+
+ Choose M here to compile this driver as a module, and it
+ will be called mtu3.ko.
+
+
+if USB_MTU3
+choice
+   bool "MTU3 Mode Selection"
+   default USB_MTU3_GADGET if (!USB && USB_GADGET)
+
+config USB_MTU3_GADGET
+   bool "Gadget only mode"
+   depends on USB_GADGET=y || USB_GADGET=USB_MTU3
+   help
+ Select this when you want to use MTU3 in gadget mode only,
+ thereby the host feature will be regressed.
+
+endchoice
+
+endif
diff --git a/drivers/usb/mtu3/Makefile b/drivers/usb/mtu3/Makefile
new file mode 100644
index 000..532c257
--- /dev/null
+++ b/drivers/usb/mtu3/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_USB_MTU3) += mtu3.o
+mtu3-y := mtu3_plat.o mtu3_core.o mtu3_gadget_ep0.o mtu3_gadget.o mtu3_qmu.o
diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
new file mode 100644
index 000..ad1a133
--- /dev/null
+++ b/drivers/usb/mtu3/mtu3.h
@@ -0,0 +1,341 @@
+/*
+ * mtu3.h - MediaTek USB3 DRD header
+ *
+ * Copyright (C) 2016 MediaTek Inc.
+ *
+ * Author: Chunfeng Yun 
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __MTU3_H__
+#define __MTU3_H__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct mtu3;
+struct mtu3_ep;
+struct mtu3_request;
+
+#include "mtu3_hw_regs.h"
+#include "mtu3_qmu.h"
+
+#defineMU3D_EP_TXCR0(epnum)(U3D_TX1CSR0 + (((epnum) - 1) * 0x10))
+#defineMU3D_EP_TXCR1(epnum)(U3D_TX1CSR1 + (((epnum) - 1) * 0x10))
+#defineMU3D_EP_TXCR2(epnum)(U3D_TX1CSR2 + (((epnum) - 1) * 0x10))
+
+#defineMU3D_EP_RXCR0(epnum)

[PATCH v7, 3/8] usb: xhci-mtk: make IPPC register optional

[Chunfeng Yun]

Make IPPC register optional to support host side of dual-role mode,
due to it is moved into common glue layer for simplification.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/host/xhci-mtk.c |   38 +++---
 drivers/usb/host/xhci-mtk.h |1 +
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 79959f1..1094ebd 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -94,6 +94,9 @@ static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
int ret;
int i;
 
+   if (!mtk->has_ippc)
+   return 0;
+
/* power on host ip */
value = readl(>ip_pw_ctr1);
value &= ~CTRL1_IP_HOST_PDN;
@@ -139,6 +142,9 @@ static int xhci_mtk_host_disable(struct xhci_hcd_mtk *mtk)
int ret;
int i;
 
+   if (!mtk->has_ippc)
+   return 0;
+
/* power down all u3 ports */
for (i = 0; i < mtk->num_u3_ports; i++) {
value = readl(>u3_ctrl_p[i]);
@@ -173,6 +179,9 @@ static int xhci_mtk_ssusb_config(struct xhci_hcd_mtk *mtk)
struct mu3c_ippc_regs __iomem *ippc = mtk->ippc_regs;
u32 value;
 
+   if (!mtk->has_ippc)
+   return 0;
+
/* reset whole ip */
value = readl(>ip_pw_ctr0);
value |= CTRL0_IP_SW_RST;
@@ -475,6 +484,7 @@ static void xhci_mtk_quirks(struct device *dev, struct 
xhci_hcd *xhci)
 /* called during probe() after chip reset completes */
 static int xhci_mtk_setup(struct usb_hcd *hcd)
 {
+   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct xhci_hcd_mtk *mtk = hcd_to_mtk(hcd);
int ret;
 
@@ -482,12 +492,21 @@ static int xhci_mtk_setup(struct usb_hcd *hcd)
ret = xhci_mtk_ssusb_config(mtk);
if (ret)
return ret;
+   }
+
+   ret = xhci_gen_setup(hcd, xhci_mtk_quirks);
+   if (ret)
+   return ret;
+
+   if (usb_hcd_is_primary_hcd(hcd)) {
+   mtk->num_u3_ports = xhci->num_usb3_ports;
+   mtk->num_u2_ports = xhci->num_usb2_ports;
ret = xhci_mtk_sch_init(mtk);
if (ret)
return ret;
}
 
-   return xhci_gen_setup(hcd, xhci_mtk_quirks);
+   return ret;
 }
 
 static int xhci_mtk_probe(struct platform_device *pdev)
@@ -586,7 +605,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
mtk->hcd = platform_get_drvdata(pdev);
platform_set_drvdata(pdev, mtk);
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mac");
hcd->regs = devm_ioremap_resource(dev, res);
if (IS_ERR(hcd->regs)) {
ret = PTR_ERR(hcd->regs);
@@ -595,11 +614,16 @@ static int xhci_mtk_probe(struct platform_device *pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-   mtk->ippc_regs = devm_ioremap_resource(dev, res);
-   if (IS_ERR(mtk->ippc_regs)) {
-   ret = PTR_ERR(mtk->ippc_regs);
-   goto put_usb2_hcd;
+   res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ippc");
+   if (res) {  /* ippc register is optional */
+   mtk->ippc_regs = devm_ioremap_resource(dev, res);
+   if (IS_ERR(mtk->ippc_regs)) {
+   ret = PTR_ERR(mtk->ippc_regs);
+   goto put_usb2_hcd;
+   }
+   mtk->has_ippc = true;
+   } else {
+   mtk->has_ippc = false;
}
 
for (phy_num = 0; phy_num < mtk->num_phys; phy_num++) {
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index 7da677c..2845c49 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -118,6 +118,7 @@ struct xhci_hcd_mtk {
struct usb_hcd *hcd;
struct mu3h_sch_bw_info *sch_array;
struct mu3c_ippc_regs __iomem *ippc_regs;
+   bool has_ippc;
int num_u2_ports;
int num_u3_ports;
struct regulator *vusb33;
-- 
1.7.9.5

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

[PATCH v7, 5/8] usb: mtu3: Super-Speed Peripheral mode support

[Chunfeng Yun]

add super-speed funtion for peripheral mode

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/mtu3.h|   13 ++-
 drivers/usb/mtu3/mtu3_core.c   |  202 
 drivers/usb/mtu3/mtu3_gadget.c |   45 +---
 drivers/usb/mtu3/mtu3_gadget_ep0.c |   90 
 drivers/usb/mtu3/mtu3_hw_regs.h|   33 ++
 5 files changed, 346 insertions(+), 37 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
index ad1a133..41a0473 100644
--- a/drivers/usb/mtu3/mtu3.h
+++ b/drivers/usb/mtu3/mtu3.h
@@ -53,6 +53,7 @@
 #define USB_QMU_TQSAR(epnum)   (U3D_TXQSAR1 + (((epnum) - 1) * 0x10))
 #define USB_QMU_TQCPR(epnum)   (U3D_TXQCPR1 + (((epnum) - 1) * 0x10))
 
+#define SSUSB_U3_CTRL(p)   (U3D_SSUSB_U3_CTRL_0P + ((p) * 0x08))
 #define SSUSB_U2_CTRL(p)   (U3D_SSUSB_U2_CTRL_0P + ((p) * 0x08))
 
 #define MTU3_DRIVER_NAME   "mtu3"
@@ -63,6 +64,7 @@
 #define MTU3_EP_WEDGE  BIT(2)
 #define MTU3_EP_BUSY   BIT(3)
 
+#define MTU3_U3_IP_SLOT_DEFAULT 2
 #define MTU3_U2_IP_SLOT_DEFAULT 1
 
 /**
@@ -87,6 +89,7 @@ enum mtu3_speed {
MTU3_SPEED_INACTIVE = 0,
MTU3_SPEED_FULL = 1,
MTU3_SPEED_HIGH = 3,
+   MTU3_SPEED_SUPER = 4,
 };
 
 /**
@@ -190,6 +193,7 @@ struct mtu3_ep {
 
struct list_head req_list;
struct mtu3_gpd_ring gpd_ring;
+   const struct usb_ss_ep_comp_descriptor *comp_desc;
const struct usb_endpoint_descriptor *desc;
 
int flags;
@@ -208,7 +212,8 @@ struct mtu3_request {
 
 /**
  * struct mtu3 - device driver instance data.
- * @slot: MTU3_U2_IP_SLOT_DEFAULT for U2 IP
+ * @slot: MTU3_U2_IP_SLOT_DEFAULT for U2 IP only,
+ * MTU3_U3_IP_SLOT_DEFAULT for U3 IP
  * @may_wakeup: means device's remote wakeup is enabled
  * @is_self_powered: is reported in device status and the config descriptor
  * @ep0_req: dummy request used while handling standard USB requests
@@ -242,12 +247,16 @@ struct mtu3 {
struct usb_gadget_driver *gadget_driver;
struct mtu3_request ep0_req;
u8 setup_buf[EP0_RESPONSE_BUF];
+   u32 max_speed;
 
unsigned is_active:1;
unsigned may_wakeup:1;
unsigned is_self_powered:1;
unsigned test_mode:1;
unsigned softconnect:1;
+   unsigned u1_enable:1;
+   unsigned u2_enable:1;
+   unsigned is_u3_ip:1;
 
u8 address;
u8 test_mode_nr;
@@ -324,7 +333,7 @@ int mtu3_config_ep(struct mtu3 *mtu, struct mtu3_ep *mep,
 void mtu3_ep0_setup(struct mtu3 *mtu);
 void mtu3_start(struct mtu3 *mtu);
 void mtu3_stop(struct mtu3 *mtu);
-void mtu3_hs_softconn_set(struct mtu3 *mtu, bool enable);
+void mtu3_dev_on_off(struct mtu3 *mtu, int is_on);
 
 int mtu3_gadget_setup(struct mtu3 *mtu);
 void mtu3_gadget_cleanup(struct mtu3 *mtu);
diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c
index 33d21dd..f9817ad 100644
--- a/drivers/usb/mtu3/mtu3_core.c
+++ b/drivers/usb/mtu3/mtu3_core.c
@@ -72,8 +72,20 @@ static void ep_fifo_free(struct mtu3_ep *mep)
__func__, mep->fifo_seg_size, mep->fifo_size, start_bit);
 }
 
+/* enable/disable U3D SS function */
+static inline void mtu3_ss_func_set(struct mtu3 *mtu, bool enable)
+{
+   /* If usb3_en==0, LTSSM will go to SS.Disable state */
+   if (enable)
+   mtu3_setbits(mtu->mac_base, U3D_USB3_CONFIG, USB3_EN);
+   else
+   mtu3_clrbits(mtu->mac_base, U3D_USB3_CONFIG, USB3_EN);
+
+   dev_dbg(mtu->dev, "USB3_EN = %d\n", !!enable);
+}
+
 /* set/clear U3D HS device soft connect */
-void mtu3_hs_softconn_set(struct mtu3 *mtu, bool enable)
+static inline void mtu3_hs_softconn_set(struct mtu3 *mtu, bool enable)
 {
if (enable) {
mtu3_setbits(mtu->mac_base, U3D_POWER_MANAGEMENT,
@@ -92,6 +104,13 @@ static int mtu3_device_enable(struct mtu3 *mtu)
u32 check_clk = 0;
 
mtu3_clrbits(ibase, U3D_SSUSB_IP_PW_CTRL2, SSUSB_IP_DEV_PDN);
+
+   if (mtu->is_u3_ip) {
+   check_clk = SSUSB_U3_MAC_RST_B_STS;
+   mtu3_clrbits(ibase, SSUSB_U3_CTRL(0),
+   (SSUSB_U3_PORT_DIS | SSUSB_U3_PORT_PDN |
+   SSUSB_U3_PORT_HOST_SEL));
+   }
mtu3_clrbits(ibase, SSUSB_U2_CTRL(0),
(SSUSB_U2_PORT_DIS | SSUSB_U2_PORT_PDN |
SSUSB_U2_PORT_HOST_SEL));
@@ -104,6 +123,10 @@ static void mtu3_device_disable(struct mtu3 *mtu)
 {
void __iomem *ibase = mtu->ippc_base;
 
+   if (mtu->is_u3_ip)
+   mtu3_setbits(ibase, SSUSB_U3_CTRL(0),
+   (SSUSB_U3_PORT_DIS | SSUSB_U3_PORT_PDN));
+
mtu3_setbits(ibase, SSUSB_U2_CTRL(0),
SSUSB_U2_PORT_DIS | SSUSB_U2_PORT_PDN);
mtu3_clrbits(ibase, SSUSB_U2_CTRL(0), SSUSB_U2_PORT_OTG_SEL);
@@ -142,6 +165,9 @@ static void mtu3_intr_status_clear(struct mtu3 *mtu)
/* Clear U2 USB common interrupts status */

[PATCH v7, 2/8] dt-bindings: mt8173-mtu3: add devicetree bindings

[Chunfeng Yun]

add a DT binding doc for MediaTek USB3 DRD driver

Signed-off-by: Chunfeng Yun 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/usb/mt8173-mtu3.txt|   87 
 1 file changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/mt8173-mtu3.txt

diff --git a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt 
b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
new file mode 100644
index 000..e049d19
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
@@ -0,0 +1,87 @@
+The device node for Mediatek USB3.0 DRD controller
+
+Required properties:
+ - compatible : should be "mediatek,mt8173-mtu3"
+ - reg : specifies physical base address and size of the registers
+ - reg-names: should be "mac" for device IP and "ippc" for IP port control
+ - interrupts : interrupt used by the device IP
+ - power-domains : a phandle to USB power domain node to control USB's
+   mtcmos
+ - vusb33-supply : regulator of USB avdd3.3v
+ - clocks : a list of phandle + clock-specifier pairs, one for each
+   entry in clock-names
+ - clock-names : must contain "sys_ck" for clock of controller;
+   "wakeup_deb_p0" and "wakeup_deb_p1" are optional, they are
+   depends on "mediatek,enable-wakeup"
+ - phys : a list of phandle + phy specifier pairs
+ - dr_mode : should be one of "host", "peripheral" or "otg",
+   refer to usb/generic.txt
+
+Optional properties:
+ - #address-cells, #size-cells : should be '2' if the device has sub-nodes
+   with 'reg' property
+ - ranges : allows valid 1:1 translation between child's address space and
+   parent's address space
+ - extcon : external connector for vbus and idpin changes detection, needed
+   when supports dual-role mode.
+ - vbus-supply : reference to the VBUS regulator, needed when supports
+   dual-role mode.
+ - pinctl-names : a pinctrl state named "default" must be defined,
+   "id_float" and "id_ground" are optinal which depends on
+   "mediatek,enable-manual-drd"
+ - pinctrl-0 : pin control group
+   See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+
+ - maximum-speed : valid arguments are "super-speed", "high-speed" and
+   "full-speed"; refer to usb/generic.txt
+ - enable-manual-drd : supports manual dual-role switch via debugfs; usually
+   used when receptacle is TYPE-A and also wants to support dual-role
+   mode.
+ - mediatek,enable-wakeup : supports ip sleep wakeup used by host mode
+ - mediatek,syscon-wakeup : phandle to syscon used to access USB wakeup
+   control register, it depends on "mediatek,enable-wakeup".
+
+Sub-nodes:
+The xhci should be added as subnode to mtu3 as shown in the following example
+if host mode is enabled. The DT binding details of xhci can be found in:
+Documentation/devicetree/bindings/usb/mt8173-xhci.txt
+
+Example:
+ssusb: usb@11271000 {
+   compatible = "mediatek,mt8173-mtu3";
+   reg = <0 0x11271000 0 0x3000>,
+ <0 0x11280700 0 0x0100>;
+   reg-names = "mac", "ippc";
+   interrupts = ;
+   phys = <_port0 PHY_TYPE_USB3>,
+  <_port1 PHY_TYPE_USB2>;
+   power-domains = < MT8173_POWER_DOMAIN_USB>;
+   clocks = < CLK_TOP_USB30_SEL>,
+< CLK_PERI_USB0>,
+< CLK_PERI_USB1>;
+   clock-names = "sys_ck",
+ "wakeup_deb_p0",
+ "wakeup_deb_p1";
+   vusb33-supply = <_vusb_reg>;
+   vbus-supply = <_p0_vbus>;
+   extcon = <_usb>;
+   dr_mode = "otg";
+   mediatek,enable-wakeup;
+   mediatek,syscon-wakeup = <>;
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+   status = "disabled";
+
+   usb_host: xhci@1127 {
+   compatible = "mediatek,mt8173-xhci";
+   reg = <0 0x1127 0 0x1000>;
+   reg-names = "mac";
+   interrupts = ;
+   power-domains = < MT8173_POWER_DOMAIN_USB>;
+   clocks = < CLK_TOP_USB30_SEL>;
+   clock-names = "sys_ck";
+   vusb33-supply = <_vusb_reg>;
+   status = "disabled";
+   };
+};
-- 
1.7.9.5

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

[PATCH v7, 1/8] dt-bindings: mt8173-xhci: support host side of dual-role mode

[Chunfeng Yun]

Some resources, such as IPPC register etc, shared with device
driver are moved into common glue layer when xHCI driver is the
host side of dual-role mode and they should be changed as optional
properties if they are required ones before. For clarity, add
a new part of binding to support host side of dual-role mode.

Additionally add optional properties of pinctrl for host only mode

Signed-off-by: Chunfeng Yun 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/usb/mt8173-xhci.txt|   54 +++-
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt 
b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
index b3a7ffa..2a930bd 100644
--- a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
@@ -2,10 +2,18 @@ MT8173 xHCI
 
 The device node for Mediatek SOC USB3.0 host controller
 
+There are two scenarios: the first one only supports xHCI driver;
+the second one supports dual-role mode, and the host is based on xHCI
+driver. Take account of backward compatibility, we divide bindings
+into two parts.
+
+1st: only supports xHCI driver
+
+
 Required properties:
  - compatible : should contain "mediatek,mt8173-xhci"
- - reg : specifies physical base address and size of the registers,
-   the first one for MAC, the second for IPPC
+ - reg : specifies physical base address and size of the registers
+ - reg-names: should be "mac" for xHCI MAC and "ippc" for IP port control
  - interrupts : interrupt used by the controller
  - power-domains : a phandle to USB power domain node to control USB's
mtcmos
@@ -27,12 +35,16 @@ Optional properties:
control register, it depends on "mediatek,wakeup-src".
  - vbus-supply : reference to the VBUS regulator;
  - usb3-lpm-capable : supports USB3.0 LPM
+ - pinctrl-names : a pinctrl state named "default" must be defined
+ - pinctrl-0 : pin control group
+   See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
 
 Example:
 usb30: usb@1127 {
compatible = "mediatek,mt8173-xhci";
reg = <0 0x1127 0 0x1000>,
  <0 0x11280700 0 0x0100>;
+   reg-names = "mac", "ippc";
interrupts = ;
power-domains = < MT8173_POWER_DOMAIN_USB>;
clocks = < CLK_TOP_USB30_SEL>,
@@ -49,3 +61,41 @@ usb30: usb@1127 {
mediatek,syscon-wakeup = <>;
mediatek,wakeup-src = <1>;
 };
+
+2nd: dual-role mode with xHCI driver
+
+
+In the case, xhci is added as subnode to mtu3. An example and the DT binding
+details of mtu3 can be found in:
+Documentation/devicetree/bindings/usb/mtu3.txt
+
+Required properties:
+ - compatible : should contain "mediatek,mt8173-xhci"
+ - reg : specifies physical base address and size of the registers
+ - reg-names: should be "mac" for xHCI MAC
+ - interrupts : interrupt used by the host controller
+ - power-domains : a phandle to USB power domain node to control USB's
+   mtcmos
+ - vusb33-supply : regulator of USB avdd3.3v
+
+ - clocks : a list of phandle + clock-specifier pairs, one for each
+   entry in clock-names
+ - clock-names : must be
+   "sys_ck": for clock of xHCI MAC
+
+Optional properties:
+ - vbus-supply : reference to the VBUS regulator;
+ - usb3-lpm-capable : supports USB3.0 LPM
+
+Example:
+usb30: usb@1127 {
+   compatible = "mediatek,mt8173-xhci";
+   reg = <0 0x1127 0 0x1000>;
+   reg-names = "mac";
+   interrupts = ;
+   power-domains = < MT8173_POWER_DOMAIN_USB>;
+   clocks = < CLK_TOP_USB30_SEL>;
+   clock-names = "sys_ck";
+   vusb33-supply = <_vusb_reg>;
+   usb3-lpm-capable;
+};
-- 
1.7.9.5

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

[PATCH v7, 8/8] arm64: dts: mediatek: add USB3 DRD driver

[Chunfeng Yun]

USB3 DRD driver is added for MT8173-EVB, and xHCI driver
becomes its subnode

Signed-off-by: Chunfeng Yun 
---
 arch/arm64/boot/dts/mediatek/mt8173-evb.dts |   63 ++-
 arch/arm64/boot/dts/mediatek/mt8173.dtsi|   29 +---
 2 files changed, 74 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts 
b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
index 2a7f731..0ecaad4 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
@@ -34,15 +34,6 @@
 
chosen { };
 
-   usb_p1_vbus: regulator@0 {
-   compatible = "regulator-fixed";
-   regulator-name = "usb_vbus";
-   regulator-min-microvolt = <500>;
-   regulator-max-microvolt = <500>;
-   gpio = < 130 GPIO_ACTIVE_HIGH>;
-   enable-active-high;
-   };
-
connector {
compatible = "hdmi-connector";
label = "hdmi";
@@ -54,6 +45,29 @@
};
};
};
+
+   extcon_usb: extcon_iddig {
+   compatible = "linux,extcon-usb-gpio";
+   id-gpio = < 16 GPIO_ACTIVE_HIGH>;
+   };
+
+   usb_p1_vbus: regulator@0 {
+   compatible = "regulator-fixed";
+   regulator-name = "usb_vbus";
+   regulator-min-microvolt = <500>;
+   regulator-max-microvolt = <500>;
+   gpio = < 130 GPIO_ACTIVE_HIGH>;
+   enable-active-high;
+   };
+
+   usb_p0_vbus: regulator@1 {
+   compatible = "regulator-fixed";
+   regulator-name = "vbus";
+   regulator-min-microvolt = <500>;
+   regulator-max-microvolt = <500>;
+   gpio = < 9 GPIO_ACTIVE_HIGH>;
+   enable-active-high;
+   };
 };
 
  {
@@ -243,6 +257,20 @@
bias-pull-down = ;
};
};
+
+   usb_id_pins_float: usb_iddig_pull_up {
+   pins_iddig {
+   pinmux = ;
+   bias-pull-up;
+   };
+   };
+
+   usb_id_pins_ground: usb_iddig_pull_down {
+   pins_iddig {
+   pinmux = ;
+   bias-pull-down;
+   };
+   };
 };
 
  {
@@ -469,12 +497,25 @@
status = "okay";
 };
 
+ {
+   vusb33-supply = <_vusb_reg>;
+   vbus-supply = <_p0_vbus>;
+   extcon = <_usb>;
+   dr_mode = "otg";
+   mediatek,enable-wakeup;
+   pinctrl-names = "default", "id_float", "id_ground";
+   pinctrl-0 = <_id_pins_float>;
+   pinctrl-1 = <_id_pins_float>;
+   pinctrl-2 = <_id_pins_ground>;
+   status = "okay";
+};
+
  {
status = "okay";
 };
 
- {
+_host {
vusb33-supply = <_vusb_reg>;
vbus-supply = <_p1_vbus>;
-   mediatek,wakeup-src = <1>;
+   status = "okay";
 };
diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi 
b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 1c71e25..c2d588c 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -707,11 +707,14 @@
status = "disabled";
};
 
-   usb30: usb@1127 {
-   compatible = "mediatek,mt8173-xhci";
-   reg = <0 0x1127 0 0x1000>,
+   ssusb: usb@11271000 {
+   compatible = "mediatek,mt8173-mtu3";
+   reg = <0 0x11271000 0 0x3000>,
  <0 0x11280700 0 0x0100>;
-   interrupts = ;
+   reg-names = "mac", "ippc";
+   interrupts = ;
+   phys = <_port0 PHY_TYPE_USB3>,
+  <_port1 PHY_TYPE_USB2>;
power-domains = < MT8173_POWER_DOMAIN_USB>;
clocks = < CLK_TOP_USB30_SEL>,
 < CLK_PERI_USB0>,
@@ -719,10 +722,22 @@
clock-names = "sys_ck",
  "wakeup_deb_p0",
  "wakeup_deb_p1";
-   phys = <_port0 PHY_TYPE_USB3>,
-  <_port1 PHY_TYPE_USB2>;
mediatek,syscon-wakeup = <>;
-   status = "okay";
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+   status = "disabled";
+
+   usb_host: xhci@1127 {
+   compatible = "mediatek,mt8173-xhci";
+   reg = <0 0x1127 0 0x1000>;
+   reg-names = "mac";
+   interrupts = ;
+   power-domains = < 

[PATCH v7, 7/8] usb: mtu3: dual-role mode support

[Chunfeng Yun]

support dual-role mode; there are two ways to switch between
host and device modes, one is by idpin, another is by debugfs
which depends on user input.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/Kconfig   |   15 +-
 drivers/usb/mtu3/Makefile  |   11 +-
 drivers/usb/mtu3/mtu3.h|   34 +++-
 drivers/usb/mtu3/mtu3_core.c   |   14 +-
 drivers/usb/mtu3/mtu3_dr.c |  379 
 drivers/usb/mtu3/mtu3_dr.h |   27 ++-
 drivers/usb/mtu3/mtu3_gadget.c |6 +-
 drivers/usb/mtu3/mtu3_host.c   |6 +
 drivers/usb/mtu3/mtu3_plat.c   |   86 -
 9 files changed, 557 insertions(+), 21 deletions(-)
 create mode 100644 drivers/usb/mtu3/mtu3_dr.c

diff --git a/drivers/usb/mtu3/Kconfig b/drivers/usb/mtu3/Kconfig
index 59e3f6f..25cd619 100644
--- a/drivers/usb/mtu3/Kconfig
+++ b/drivers/usb/mtu3/Kconfig
@@ -2,7 +2,7 @@
 
 config USB_MTU3
tristate "MediaTek USB3 Dual Role controller"
-   depends on (USB || USB_GADGET) && HAS_DMA
+   depends on EXTCON && (USB || USB_GADGET) && HAS_DMA
depends on ARCH_MEDIATEK || COMPILE_TEST
select USB_XHCI_MTK if USB_SUPPORT && USB_XHCI_HCD
help
@@ -19,6 +19,7 @@ config USB_MTU3
 if USB_MTU3
 choice
bool "MTU3 Mode Selection"
+   default USB_MTU3_DUAL_ROLE if (USB && USB_GADGET)
default USB_MTU3_HOST if (USB && !USB_GADGET)
default USB_MTU3_GADGET if (!USB && USB_GADGET)
 
@@ -36,6 +37,18 @@ config USB_MTU3_GADGET
  Select this when you want to use MTU3 in gadget mode only,
  thereby the host feature will be regressed.
 
+config USB_MTU3_DUAL_ROLE
+   bool "Dual Role mode"
+   depends on ((USB=y || USB=USB_MTU3) && (USB_GADGET=y || 
USB_GADGET=USB_MTU3))
+   help
+ This is the default mode of working of MTU3 controller where
+ both host and gadget features are enabled.
+
 endchoice
 
+config USB_MTU3_DEBUG
+   bool "Enable Debugging Messages"
+   help
+ Say Y here to enable debugging messages in the MTU3 Driver.
+
 endif
diff --git a/drivers/usb/mtu3/Makefile b/drivers/usb/mtu3/Makefile
index 41e45e9..60e0fff 100644
--- a/drivers/usb/mtu3/Makefile
+++ b/drivers/usb/mtu3/Makefile
@@ -1,11 +1,18 @@
+
+ccflags-$(CONFIG_USB_MTU3_DEBUG)   += -DDEBUG
+
 obj-$(CONFIG_USB_MTU3) += mtu3.o
 
 mtu3-y := mtu3_plat.o
 
-ifneq ($(filter y,$(CONFIG_USB_MTU3_HOST)),)
+ifneq ($(filter y,$(CONFIG_USB_MTU3_HOST) $(CONFIG_USB_MTU3_DUAL_ROLE)),)
mtu3-y  += mtu3_host.o
 endif
 
-ifneq ($(filter y,$(CONFIG_USB_MTU3_GADGET)),)
+ifneq ($(filter y,$(CONFIG_USB_MTU3_GADGET) $(CONFIG_USB_MTU3_DUAL_ROLE)),)
mtu3-y  += mtu3_core.o mtu3_gadget_ep0.o mtu3_gadget.o mtu3_qmu.o
 endif
+
+ifneq ($(CONFIG_USB_MTU3_DUAL_ROLE),)
+   mtu3-y  += mtu3_dr.o
+endif
diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
index a7c0ce8..ba9df71 100644
--- a/drivers/usb/mtu3/mtu3.h
+++ b/drivers/usb/mtu3/mtu3.h
@@ -21,6 +21,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -172,15 +173,44 @@ struct mtu3_gpd_ring {
struct qmu_gpd *enqueue;
struct qmu_gpd *dequeue;
 };
+
+/**
+* @vbus: vbus 5V used by host mode
+* @edev: external connector used to detect vbus and iddig changes
+* @vbus_nb: notifier for vbus detection
+* @vbus_nb: notifier for iddig(idpin) detection
+* @extcon_reg_dwork: delay work for extcon notifier register, waiting for
+*  xHCI driver initialization, it's necessary for system bootup
+*  as device.
+* @is_u3_drd: whether port0 supports usb3.0 dual-role device or not
+* @id_*: used to maually switch between host and device modes by idpin
+* @manual_drd_enabled: it's true when supports dual-role device by debugfs
+*  to switch host/device modes depending on user input.
+*/
+struct otg_switch_mtk {
+   struct regulator *vbus;
+   struct extcon_dev *edev;
+   struct notifier_block vbus_nb;
+   struct notifier_block id_nb;
+   struct delayed_work extcon_reg_dwork;
+   bool is_u3_drd;
+   /* dual-role switch by debugfs */
+   struct pinctrl *id_pinctrl;
+   struct pinctrl_state *id_float;
+   struct pinctrl_state *id_ground;
+   bool manual_drd_enabled;
+};
+
 /**
  * @mac_base: register base address of device MAC, exclude xHCI's
- * @ippc_base: register base address of ip port controller interface (IPPC)
+ * @ippc_base: register base address of IP Power and Clock interface (IPPC)
  * @vusb33: usb3.3V shared by device/host IP
  * @sys_clk: system clock of mtu3, shared by device/host IP
  * @dr_mode: works in which mode:
  * host only, device only or dual-role mode
  * @u2_ports: number of usb2.0 host ports
  * @u3_ports: number of usb3.0 host ports
+ * @dbgfs_root: only used when supports manual dual-role switch via debugfs
  * @wakeup_en: it's true when supports remote wakeup in host mode
  * @wk_deb_p0: port0's wakeup debounce clock
  * 

[PATCH v7, 6/8] usb: mtu3: host only mode support

[Chunfeng Yun]

supports host only mode and the code is ported from
host/xhci-mtk.c
IPPC register shared between host and device is moved
into common glue layer.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/Kconfig |9 ++
 drivers/usb/mtu3/Makefile|   11 +-
 drivers/usb/mtu3/mtu3.h  |   47 ++-
 drivers/usb/mtu3/mtu3_core.c |   42 +-
 drivers/usb/mtu3/mtu3_dr.h   |   85 +
 drivers/usb/mtu3/mtu3_host.c |  288 +
 drivers/usb/mtu3/mtu3_plat.c |  289 --
 7 files changed, 691 insertions(+), 80 deletions(-)
 create mode 100644 drivers/usb/mtu3/mtu3_dr.h
 create mode 100644 drivers/usb/mtu3/mtu3_host.c

diff --git a/drivers/usb/mtu3/Kconfig b/drivers/usb/mtu3/Kconfig
index 54dadee..59e3f6f 100644
--- a/drivers/usb/mtu3/Kconfig
+++ b/drivers/usb/mtu3/Kconfig
@@ -4,6 +4,7 @@ config USB_MTU3
tristate "MediaTek USB3 Dual Role controller"
depends on (USB || USB_GADGET) && HAS_DMA
depends on ARCH_MEDIATEK || COMPILE_TEST
+   select USB_XHCI_MTK if USB_SUPPORT && USB_XHCI_HCD
help
  Say Y or M here if your system runs on MediaTek SoCs with
  Dual Role SuperSpeed USB controller. You can select usb
@@ -18,8 +19,16 @@ config USB_MTU3
 if USB_MTU3
 choice
bool "MTU3 Mode Selection"
+   default USB_MTU3_HOST if (USB && !USB_GADGET)
default USB_MTU3_GADGET if (!USB && USB_GADGET)
 
+config USB_MTU3_HOST
+   bool "Host only mode"
+   depends on USB=y || USB=USB_MTU3
+   help
+ Select this when you want to use MTU3 in host mode only,
+ thereby the gadget feature will be regressed.
+
 config USB_MTU3_GADGET
bool "Gadget only mode"
depends on USB_GADGET=y || USB_GADGET=USB_MTU3
diff --git a/drivers/usb/mtu3/Makefile b/drivers/usb/mtu3/Makefile
index 532c257..41e45e9 100644
--- a/drivers/usb/mtu3/Makefile
+++ b/drivers/usb/mtu3/Makefile
@@ -1,2 +1,11 @@
 obj-$(CONFIG_USB_MTU3) += mtu3.o
-mtu3-y := mtu3_plat.o mtu3_core.o mtu3_gadget_ep0.o mtu3_gadget.o mtu3_qmu.o
+
+mtu3-y := mtu3_plat.o
+
+ifneq ($(filter y,$(CONFIG_USB_MTU3_HOST)),)
+   mtu3-y  += mtu3_host.o
+endif
+
+ifneq ($(filter y,$(CONFIG_USB_MTU3_GADGET)),)
+   mtu3-y  += mtu3_core.o mtu3_gadget_ep0.o mtu3_gadget.o mtu3_qmu.o
+endif
diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
index 41a0473..a7c0ce8 100644
--- a/drivers/usb/mtu3/mtu3.h
+++ b/drivers/usb/mtu3/mtu3.h
@@ -172,6 +172,40 @@ struct mtu3_gpd_ring {
struct qmu_gpd *enqueue;
struct qmu_gpd *dequeue;
 };
+/**
+ * @mac_base: register base address of device MAC, exclude xHCI's
+ * @ippc_base: register base address of ip port controller interface (IPPC)
+ * @vusb33: usb3.3V shared by device/host IP
+ * @sys_clk: system clock of mtu3, shared by device/host IP
+ * @dr_mode: works in which mode:
+ * host only, device only or dual-role mode
+ * @u2_ports: number of usb2.0 host ports
+ * @u3_ports: number of usb3.0 host ports
+ * @wakeup_en: it's true when supports remote wakeup in host mode
+ * @wk_deb_p0: port0's wakeup debounce clock
+ * @wk_deb_p1: it's optional, and depends on port1 is supported or not
+ */
+struct ssusb_mtk {
+   struct device *dev;
+   struct mtu3 *u3d;
+   void __iomem *mac_base;
+   void __iomem *ippc_base;
+   struct phy **phys;
+   int num_phys;
+   /* common power & clock */
+   struct regulator *vusb33;
+   struct clk *sys_clk;
+   /* otg */
+   enum usb_dr_mode dr_mode;
+   bool is_host;
+   int u2_ports;
+   int u3_ports;
+   /* usb wakeup for host mode */
+   bool wakeup_en;
+   struct clk *wk_deb_p0;
+   struct clk *wk_deb_p1;
+   struct regmap *pericfg;
+};
 
 /**
  * @fifo_size: it is (@slot + 1) * @fifo_seg_size
@@ -210,6 +244,11 @@ struct mtu3_request {
int epnum;
 };
 
+static inline struct ssusb_mtk *dev_to_ssusb(struct device *dev)
+{
+   return dev_get_drvdata(dev);
+}
+
 /**
  * struct mtu3 - device driver instance data.
  * @slot: MTU3_U2_IP_SLOT_DEFAULT for U2 IP only,
@@ -222,12 +261,10 @@ struct mtu3_request {
  */
 struct mtu3 {
spinlock_t lock;
+   struct ssusb_mtk *ssusb;
struct device *dev;
void __iomem *mac_base;
void __iomem *ippc_base;
-   struct phy *phy;
-   struct regulator *vusb33;
-   struct clk *sys_clk;
int irq;
 
struct mtu3_fifo_info tx_fifo;
@@ -320,7 +357,7 @@ static inline void mtu3_clrbits(void __iomem *base, u32 
offset, u32 bits)
writel((tmp & ~(bits)), addr);
 }
 
-int ssusb_check_clocks(struct mtu3 *mtu, u32 ex_clks);
+int ssusb_check_clocks(struct ssusb_mtk *ssusb, u32 ex_clks);
 struct usb_request *mtu3_alloc_request(struct usb_ep *ep, gfp_t gfp_flags);
 void mtu3_free_request(struct usb_ep *ep, struct usb_request *req);
 void mtu3_req_complete(struct mtu3_ep *mep,
@@ -341,8 +378,6 @@ int 

[PATCH v7, 0/8] Add MediaTek USB3 DRD Driver

[Chunfeng Yun]

These patches introduce the MediaTek USB3 dual-role controller
driver.

The driver can be configured as Dual-Role Device (DRD),
Peripheral Only and Host Only (xHCI) modes. It works well
with Mass Storage, RNDIS and g_zero on FS/HS and SS. And it is
tested on MT8173 platform which only contains USB2.0 device IP,
and on MT6290 platform which contains USB3.0 device IP.

Change in v7:
1. split dual-role driver into four patchs
2. remove QMU done tasklet
3. add a bool in xhci_hcd_mtk to signal absence of IPPC

Change in v6:
1. handle endianness of GPD and SETUP data
2. remove dummy error log and return suitable error number
3. cancel delay work when deregiseter driver

Change in v5:
1. modify some comments
2. rename some unsuitable variables
3. add reg-names property for host node
4. add USB_MTU3_DEBUG to control debug messages

Change in v4:
1. fix build errors on non-mediatek platforms
2. provide manual dual-role switch via debugfs instead of sysfs

Change in v3:
1. fix some typo error
2. rename mtu3.txt to mt8173-mtu3.txt

Change in v2:
1. modify binding docs according to suggestions
2. modify some comments and remove some dummy blank lines
3. fix memory leakage


Chunfeng Yun (8):
  dt-bindings: mt8173-xhci: support host side of dual-role mode
  dt-bindings: mt8173-mtu3: add devicetree bindings
  usb: xhci-mtk: make IPPC register optional
  usb: Add MediaTek USB3 DRD driver
  usb: mtu3: Super-Speed Peripheral mode support
  usb: mtu3: host only mode support
  usb: mtu3: dual-role mode support
  arm64: dts: mediatek: add USB3 DRD driver

 .../devicetree/bindings/usb/mt8173-mtu3.txt|   87 ++
 .../devicetree/bindings/usb/mt8173-xhci.txt|   54 +-
 arch/arm64/boot/dts/mediatek/mt8173-evb.dts|   63 +-
 arch/arm64/boot/dts/mediatek/mt8173.dtsi   |   29 +-
 drivers/usb/Kconfig|2 +
 drivers/usb/Makefile   |1 +
 drivers/usb/host/xhci-mtk.c|   38 +-
 drivers/usb/host/xhci-mtk.h|1 +
 drivers/usb/mtu3/Kconfig   |   54 ++
 drivers/usb/mtu3/Makefile  |   18 +
 drivers/usb/mtu3/mtu3.h|  417 +
 drivers/usb/mtu3/mtu3_core.c   |  859 +++
 drivers/usb/mtu3/mtu3_dr.c |  379 +
 drivers/usb/mtu3/mtu3_dr.h |  108 +++
 drivers/usb/mtu3/mtu3_gadget.c |  730 
 drivers/usb/mtu3/mtu3_gadget_ep0.c |  881 
 drivers/usb/mtu3/mtu3_host.c   |  294 +++
 drivers/usb/mtu3/mtu3_hw_regs.h|  473 +++
 drivers/usb/mtu3/mtu3_plat.c   |  484 +++
 drivers/usb/mtu3/mtu3_qmu.c|  573 +
 drivers/usb/mtu3/mtu3_qmu.h|   43 +
 21 files changed, 5561 insertions(+), 27 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
 create mode 100644 drivers/usb/mtu3/Kconfig
 create mode 100644 drivers/usb/mtu3/Makefile
 create mode 100644 drivers/usb/mtu3/mtu3.h
 create mode 100644 drivers/usb/mtu3/mtu3_core.c
 create mode 100644 drivers/usb/mtu3/mtu3_dr.c
 create mode 100644 drivers/usb/mtu3/mtu3_dr.h
 create mode 100644 drivers/usb/mtu3/mtu3_gadget.c
 create mode 100644 drivers/usb/mtu3/mtu3_gadget_ep0.c
 create mode 100644 drivers/usb/mtu3/mtu3_host.c
 create mode 100644 drivers/usb/mtu3/mtu3_hw_regs.h
 create mode 100644 drivers/usb/mtu3/mtu3_plat.c
 create mode 100644 drivers/usb/mtu3/mtu3_qmu.c
 create mode 100644 drivers/usb/mtu3/mtu3_qmu.h

-- 
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 v5 11/23] usb: chipidea: Emulate OTGSC interrupt enable path

[Peter Chen]

On Mon, Oct 17, 2016 at 06:56:24PM -0700, Stephen Boyd wrote:
> In the case of an extcon-usb-gpio device being used with the
> chipidea driver we'll sometimes miss the BSVIS event in the OTGSC
> register. Consider the case where we don't have a cable attached
> and the id pin is indicating "host" mode. When we plug in the usb
> cable for "device" mode a gpio goes high and indicates that we
> should do the role switch and that vbus is high. When we're in
> "host" mode the OTGSC register doesn't have BSVIE set.

I have some questions for your description:

- Do you have any pending or related patches what this patch set
  is based on? Afaik, the extcon-usb-gpio has no vbus event supported.
- When the ID from 0->1, the chipidea driver will do role switch, and
  set BSVIE, why it does not occur for your case?

Peter
> 
> The following scenario can happen:
> 
> CPU0
> 
> 
>  ci_cable_notifier()
>   update id cable state
>   ci_irq()
>if (ci->is_otg && (otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) { // true
> ci->id_event = true;
> ci_otg_queue_work()
>  schedule()
> 
>  // same task as before
>  ci_cable_notifier()
>   update vbus cable state
>ci_irq()
> if (ci->is_otg && (otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) // false
>return IRQ_NONE
> 
> ci_otg_work() // switch task to the workqueue now
>  if (ci->id_event)
>   ci_handle_id_switch()
>ci_role_stop()
> host_stop()
>hw_wait_vbus_lower_bsv(ci); // this times out because vbus is already set
>ci_role_start()
> udc_id_switch_for_device()
>  hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE, OTGSC_BSVIS | OTGSC_BSVIE);
> 
> At this point, we don't replay the vbus connect event because the
> vbus event has already happened. This causes things like gadget
> instances to never see vbus appear, and thus the gadget is never
> started. Furthermore, we see timeout messages like:
> 
>   timeout waiting for 800 in OTGSC
> 
> Let's workaround this by skiping the wait for BSV when we're
> using an extcon for the vbus notification and let's properly
> emulate the BSVIS event that would happen when we enable the
> vbus interrupt while enabling "device" mode.
> 
> Cc: Peter Chen 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/usb/chipidea/ci.h   |  2 ++
>  drivers/usb/chipidea/core.c | 23 +--
>  drivers/usb/chipidea/otg.c  | 31 ---
>  3 files changed, 43 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 59e22389c10b..e099b8bc79e2 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -437,6 +437,8 @@ static inline void ci_ulpi_exit(struct ci_hdrc *ci) { }
>  static inline int ci_ulpi_resume(struct ci_hdrc *ci) { return 0; }
>  #endif
>  
> +irqreturn_t __ci_irq(int irq, struct ci_hdrc *ci);
> +
>  u32 hw_read_intr_enable(struct ci_hdrc *ci);
>  
>  u32 hw_read_intr_status(struct ci_hdrc *ci);
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 83bc2f2dd6a8..d1ae9a03e0fa 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -524,9 +524,8 @@ int hw_device_reset(struct ci_hdrc *ci)
>   return 0;
>  }
>  
> -static irqreturn_t ci_irq(int irq, void *data)
> +irqreturn_t __ci_irq(int irq, struct ci_hdrc *ci)
>  {
> - struct ci_hdrc *ci = data;
>   irqreturn_t ret = IRQ_NONE;
>   u32 otgsc = 0;
>  
> @@ -570,9 +569,20 @@ static irqreturn_t ci_irq(int irq, void *data)
>   return IRQ_HANDLED;
>   }
>  
> - /* Handle device/host interrupt */
> - if (ci->role != CI_ROLE_END)
> - ret = ci_role(ci)->irq(ci);
> + return ret;
> +}
> +
> +static irqreturn_t ci_irq(int irq, void *data)
> +{
> + irqreturn_t ret;
> + struct ci_hdrc *ci = data;
> +
> + ret = __ci_irq(irq, ci);
> + if (ret == IRQ_NONE) {
> + /* Handle device/host interrupt */
> + if (ci->role != CI_ROLE_END)
> + ret = ci_role(ci)->irq(ci);
> + }
>  
>   return ret;
>  }
> @@ -586,7 +596,8 @@ static int ci_cable_notifier(struct notifier_block *nb, 
> unsigned long event,
>   cbl->connected = event;
>   cbl->changed = true;
>  
> - ci_irq(ci->irq, ci);
> + __ci_irq(ci->irq, ci);
> +
>   return NOTIFY_DONE;
>  }
>  
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 695f3fe3ae21..f4a21ade1901 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -84,36 +84,44 @@ u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask)
>  void hw_write_otgsc(struct ci_hdrc *ci, u32 mask, u32 data)
>  {
>   struct ci_hdrc_cable *cable;
> + bool raise_irq = false;
>  
>   cable = >platdata->vbus_extcon;
>   if (!IS_ERR(cable->edev)) {
> - if (data & mask & OTGSC_BSVIS)
> -   

[PATCH 1/1] usb: xhci: clean up error_bitmask usage

[Lu Baolu]

In xhci_handle_event(), when errors are detected, driver always sets
a bit in error_bitmask (one member of the xhci private driver data).
That means users have to retrieve and decode the value of error_bitmask
in xhci private driver data if they want to know whether those erros
ever happened in xhci_handle_event(). Otherwise, those errors are just
ignored silently.

This patch cleans up this by replacing the setting of error_bitmask
with the kernel print functions, so that users can easily check and
report the errors happened in xhci_handle_event().

Signed-off-by: Lu Baolu 
---
 drivers/usb/host/xhci-ring.c | 42 +++---
 drivers/usb/host/xhci.h  |  2 --
 2 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 797137e..a460423 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1183,7 +1183,7 @@ static void xhci_handle_cmd_nec_get_fw(struct xhci_hcd 
*xhci,
struct xhci_event_cmd *event)
 {
if (!(xhci->quirks & XHCI_NEC_HOST)) {
-   xhci->error_bitmask |= 1 << 6;
+   xhci_warn(xhci, "WARN NEC_GET_FW command on non-NEC host\n");
return;
}
xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
@@ -1325,14 +1325,13 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
cmd_trb = xhci->cmd_ring->dequeue;
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
cmd_trb);
-   /* Is the command ring deq ptr out of sync with the deq seg ptr? */
-   if (cmd_dequeue_dma == 0) {
-   xhci->error_bitmask |= 1 << 4;
-   return;
-   }
-   /* Does the DMA address match our internal dequeue pointer address? */
-   if (cmd_dma != (u64) cmd_dequeue_dma) {
-   xhci->error_bitmask |= 1 << 5;
+   /*
+* Check whether the completion event is for our internal kept
+* command.
+*/
+   if (!cmd_dequeue_dma || cmd_dma != (u64)cmd_dequeue_dma) {
+   xhci_warn(xhci,
+ "ERROR mismatched command completion event\n");
return;
}
 
@@ -1418,7 +1417,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
break;
default:
/* Skip over unknown commands on the event ring */
-   xhci->error_bitmask |= 1 << 6;
+   xhci_info(xhci, "INFO unknown command type %d\n", cmd_type);
break;
}
 
@@ -1521,7 +1520,7 @@ static void handle_port_status(struct xhci_hcd *xhci,
/* Port status change events always have a successful completion code */
if (GET_COMP_CODE(le32_to_cpu(event->generic.field[2])) != 
COMP_SUCCESS) {
xhci_warn(xhci, "WARN: xHC returned failed port status 
event\n");
-   xhci->error_bitmask |= 1 << 8;
+   return;
}
port_id = GET_PORT_ID(le32_to_cpu(event->generic.field[0]));
xhci_dbg(xhci, "Port Status Change Event for port %d\n", port_id);
@@ -2642,20 +2641,18 @@ static int xhci_handle_event(struct xhci_hcd *xhci)
 {
union xhci_trb *event;
int update_ptrs = 1;
-   int ret;
 
+   /* Event ring hasn't been allocated yet. */
if (!xhci->event_ring || !xhci->event_ring->dequeue) {
-   xhci->error_bitmask |= 1 << 1;
-   return 0;
+   xhci_err(xhci, "ERROR event ring not ready\n");
+   return -ENOMEM;
}
 
event = xhci->event_ring->dequeue;
/* Does the HC or OS own the TRB? */
if ((le32_to_cpu(event->event_cmd.flags) & TRB_CYCLE) !=
-   xhci->event_ring->cycle_state) {
-   xhci->error_bitmask |= 1 << 2;
+   xhci->event_ring->cycle_state)
return 0;
-   }
 
/*
 * Barrier between reading the TRB_CYCLE (valid) flag above and any
@@ -2663,7 +2660,7 @@ static int xhci_handle_event(struct xhci_hcd *xhci)
 */
rmb();
/* FIXME: Handle more event types. */
-   switch ((le32_to_cpu(event->event_cmd.flags) & TRB_TYPE_BITMASK)) {
+   switch (le32_to_cpu(event->event_cmd.flags) & TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_COMPLETION):
handle_cmd_completion(xhci, >event_cmd);
break;
@@ -2672,10 +2669,7 @@ static int xhci_handle_event(struct xhci_hcd *xhci)
update_ptrs = 0;
break;
case TRB_TYPE(TRB_TRANSFER):
-   ret = handle_tx_event(xhci, >trans_event);
-   if (ret < 0)
-   xhci->error_bitmask |= 1 << 9;
-   else
+   if (!handle_tx_event(xhci, >trans_event))
update_ptrs = 0;
break;
case TRB_TYPE(TRB_DEV_NOTE):
@@ -2686,7 +2680,9 @@ static int xhci_handle_event(struct 

[RESEND PATCH v2 4/4] usb: doc: add document for USB3 debug port usage

[Lu Baolu]

Add Documentation/usb/usb3-debug-port.txt. This document includes
the user guide for USB3 debug port.

Cc: linux-...@vger.kernel.org
Signed-off-by: Lu Baolu 
---
 Documentation/usb/usb3-debug-port.txt | 87 +++
 1 file changed, 87 insertions(+)
 create mode 100644 Documentation/usb/usb3-debug-port.txt

diff --git a/Documentation/usb/usb3-debug-port.txt 
b/Documentation/usb/usb3-debug-port.txt
new file mode 100644
index 000..df5ce27
--- /dev/null
+++ b/Documentation/usb/usb3-debug-port.txt
@@ -0,0 +1,87 @@
+  USB3 debug port
+
+ Lu Baolu 
+
+  Last-updated: October 2016
+
+GENERAL
+===
+
+This is a HOWTO for using USB3 debug port on x86 systems.
+
+Before using any kernel debugging functionalities based on USB3
+debug port, you need to check 1) whether debug port is supported
+by the xHCI host, 2) which port is used for debugging purpose
+(normally the first USB3 root port). You must have a USB 3.0
+super-speed A-to-A debugging cable to connect the debug target
+with a debug host. In this document, a debug target stands for
+the system under debugging; while, a debug host stands for a
+stand-alone system that is able to talk to the debugging target
+through the USB3 debug port.
+
+EARLY PRINTK
+
+
+On debug target system, you need to customize a debugging kernel
+with CONFIG_EARLY_PRINTK_XDBC enabled. And add below kernel boot
+parameter.
+
+"earlyprintk=xdbc"
+
+If there are multiple xHCI controllers in the system, you can
+append a host contoller index to this kernel parameter. This
+index is started from 0.
+
+If you are going to leverage the keep option defined by the
+early printk framework to keep the boot console alive after
+early boot, you'd better add below kernel boot parameter.
+
+"usbcore.autosuspend=-1"
+
+On debug host side, you don't need to customize the kernel, but
+you need to disable usb subsystem runtime power management by
+adding below kernel boot parameter.
+
+"usbcore.autosuspend=-1"
+
+Before starting the debug target, you should connect the debug
+port on debug target with a root port or port of any external hub
+on the debug host. The cable used to connect these two ports
+should be a USB 3.0 super-speed A-to-A debugging cable.
+
+During early boot of debug target, DbC (the debug engine for USB3
+debug port) hardware gets initialized. Debug host should be able
+to enumerate the debug target as a debug device. Debug host will
+then bind the debug device with the usb_debug driver module and
+create the /dev/ttyUSB0 device.
+
+If device enumeration goes smoothly, you should be able to see
+below kernel messages on debug host.
+
+# tail -f /var/log/kern.log
+
+[ 1815.983374] usb 4-3: new SuperSpeed USB device number 4 using xhci_hcd
+[ 1815.999595] usb 4-3: LPM exit latency is zeroed, disabling LPM.
+[ 1815.999899] usb 4-3: New USB device found, idVendor=1d6b, idProduct=0004
+[ 1815.02] usb 4-3: New USB device strings: Mfr=1, Product=2, 
SerialNumber=3
+[ 1815.03] usb 4-3: Product: Remote GDB
+[ 1815.04] usb 4-3: Manufacturer: Linux
+[ 1815.05] usb 4-3: SerialNumber: 0001
+[ 1816.000240] usb_debug 4-3:1.0: xhci_dbc converter detected
+[ 1816.000360] usb 4-3: xhci_dbc converter now attached to ttyUSB0
+
+You can run below bash scripts on debug host to read the kernel
+log sent from debug target.
+
+= start of bash scripts =
+#!/bin/bash
+
+while true ; do
+   while [ ! -d /sys/class/tty/ttyUSB0 ] ; do
+   :
+   done
+   cat /dev/ttyUSB0 >> xdbc.log
+done
+= end of bash scripts ===
+
+You should be able to see the early boot message in xdbc.log.
-- 
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

[RESEND PATCH v2 0/4] usb: early: add support for early printk through USB3 debug port

[Lu Baolu]

xHCI debug capability (DbC) is an optional but standalone
functionality provided by an xHCI host controller. With DbC
hardware initialized, the system will present a debug device
through the USB3 debug port (normally the first USB3 port).
The debug device is fully compliant with the USB framework
and provides the equivalent of a very high performance (USB3)
full-duplex serial link between the debug host and target.
The DbC functionality is independent of xHCI host. There
isn't any precondition from xHCI host side for DbC to work.

This patch set adds support for early printk functionality
through a USB3 debug port by 1) initializing and enabling
the DbC hardware during early boot; 2) registering a boot
console to the system so that early printk messages can go
through the USB3 debug port. It also includes some lines
of changes in usb_debug driver so that it can be bound when
a USB3 debug device is enumerated.

This code is designed to be used only for kernel debugging
when machine crashes very early before the console code is
initialized. It makes the life of kernel debugging easier
when people work with a modern machine without any legacy
serial ports.

---
Change log:
v1->v2:
  - Refactor the duplicate code in xdbc_early_start() and
xdbc_handle_external_reset().
  - Free resources when hardware not used any more.
  - Refine the user guide document.

Lu Baolu (4):
  usb: dbc: early driver for xhci debug capability
  x86: add support for earlyprintk via USB3 debug port
  usb: serial: usb_debug: add support for dbc debug device
  usb: doc: add document for USB3 debug port usage

 Documentation/kernel-parameters.txt   |1 +
 Documentation/usb/usb3-debug-port.txt |   87 +++
 arch/x86/Kconfig.debug|   14 +
 arch/x86/kernel/early_printk.c|5 +
 arch/x86/kernel/setup.c   |7 +
 drivers/usb/Kconfig   |3 +
 drivers/usb/Makefile  |2 +-
 drivers/usb/early/Makefile|1 +
 drivers/usb/early/xhci-dbc.c  | 1097 +
 drivers/usb/early/xhci-dbc.h  |  206 +++
 drivers/usb/serial/usb_debug.c|   28 +-
 include/linux/usb/xhci-dbgp.h |   22 +
 12 files changed, 1469 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/usb/usb3-debug-port.txt
 create mode 100644 drivers/usb/early/xhci-dbc.c
 create mode 100644 drivers/usb/early/xhci-dbc.h
 create mode 100644 include/linux/usb/xhci-dbgp.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

[RESEND PATCH v2 3/4] usb: serial: usb_debug: add support for dbc debug device

[Lu Baolu]

This patch add dbc debug device support in usb_debug driver.

Signed-off-by: Lu Baolu 
Acked-by: Johan Hovold 
---
 drivers/usb/serial/usb_debug.c | 28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/usb_debug.c b/drivers/usb/serial/usb_debug.c
index ca2fa5b..92f7e5c 100644
--- a/drivers/usb/serial/usb_debug.c
+++ b/drivers/usb/serial/usb_debug.c
@@ -32,7 +32,18 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE(0x0525, 0x127a) },
{ },
 };
-MODULE_DEVICE_TABLE(usb, id_table);
+
+static const struct usb_device_id dbc_id_table[] = {
+   { USB_DEVICE(0x1d6b, 0x0004) },
+   { },
+};
+
+static const struct usb_device_id id_table_combined[] = {
+   { USB_DEVICE(0x0525, 0x127a) },
+   { USB_DEVICE(0x1d6b, 0x0004) },
+   { },
+};
+MODULE_DEVICE_TABLE(usb, id_table_combined);
 
 /* This HW really does not support a serial break, so one will be
  * emulated when ever the break state is set to true.
@@ -71,9 +82,20 @@ static struct usb_serial_driver debug_device = {
.process_read_urb = usb_debug_process_read_urb,
 };
 
+static struct usb_serial_driver dbc_device = {
+   .driver = {
+   .owner =THIS_MODULE,
+   .name = "xhci_dbc",
+   },
+   .id_table = dbc_id_table,
+   .num_ports =1,
+   .break_ctl =usb_debug_break_ctl,
+   .process_read_urb = usb_debug_process_read_urb,
+};
+
 static struct usb_serial_driver * const serial_drivers[] = {
-   _device, NULL
+   _device, _device, NULL
 };
 
-module_usb_serial_driver(serial_drivers, id_table);
+module_usb_serial_driver(serial_drivers, id_table_combined);
 MODULE_LICENSE("GPL");
-- 
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

[RESEND PATCH v2 2/4] x86: add support for earlyprintk via USB3 debug port

[Lu Baolu]

Add support for early printk by writing debug messages to the
USB3 debug port.   Users can use this type of early printk by
specifying kernel parameter of "earlyprintk=xdbc". This gives
users a chance of providing debug output.

The hardware for USB3 debug port requires DMA memory blocks.
This requires to delay setting up debugging hardware and
registering boot console until the memblocks are filled.

Cc: Ingo Molnar 
Cc: x...@kernel.org
Signed-off-by: Lu Baolu 
---
 Documentation/kernel-parameters.txt | 1 +
 arch/x86/kernel/early_printk.c  | 5 +
 arch/x86/kernel/setup.c | 7 +++
 3 files changed, 13 insertions(+)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 37babf9..99b64b3 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1178,6 +1178,7 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
earlyprintk=ttySn[,baudrate]
earlyprintk=dbgp[debugController#]
earlyprintk=pciserial,bus:device.function[,baudrate]
+   earlyprintk=xdbc[xhciController#]
 
earlyprintk is useful when the kernel crashes before
the normal console is initialized. It is not enabled by
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 8a12199..c4031b9 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -381,6 +382,10 @@ static int __init setup_early_printk(char *buf)
if (!strncmp(buf, "efi", 3))
early_console_register(_efi_console, keep);
 #endif
+#ifdef CONFIG_EARLY_PRINTK_XDBC
+   if (!strncmp(buf, "xdbc", 4))
+   early_xdbc_parse_parameter(buf + 4);
+#endif
 
buf++;
}
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index bbfbca5..3ceacc8 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -70,6 +70,8 @@
 #include 
 #include 
 
+#include 
+
 #include 
 
 #include 
@@ -1096,6 +1098,11 @@ void __init setup_arch(char **cmdline_p)
memblock_set_current_limit(ISA_END_ADDRESS);
memblock_x86_fill();
 
+#ifdef CONFIG_EARLY_PRINTK_XDBC
+   if (!early_xdbc_setup_hardware())
+   early_xdbc_register_console();
+#endif
+
reserve_bios_regions();
 
if (efi_enabled(EFI_MEMMAP)) {
-- 
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

[RESEND PATCH v2 1/4] usb: dbc: early driver for xhci debug capability

[Lu Baolu]

xHCI debug capability (DbC) is an optional but standalone
functionality provided by an xHCI host controller. Software
learns this capability by walking through the extended
capability list of the host. xHCI specification describes
DbC in section 7.6.

This patch introduces the code to probe and initialize the
debug capability hardware during early boot. With hardware
initialized, the debug target (system on which this code is
running) will present a debug device through the debug port
(normally the first USB3 port). The debug device is fully
compliant with the USB framework and provides the equivalent
of a very high performance (USB3) full-duplex serial link
between the debug host and target. The DbC functionality is
independent of xHCI host. There isn't any precondition from
xHCI host side for DbC to work.

This patch also includes bulk out and bulk in interfaces.
These interfaces could be used to implement early printk
bootconsole or hook to various system debuggers.

This code is designed to be only used for kernel debugging
when machine crashes very early before the console code is
initialized. For normal operation it is not recommended.

Cc: Mathias Nyman 
Signed-off-by: Lu Baolu 
---
 arch/x86/Kconfig.debug|   14 +
 drivers/usb/Kconfig   |3 +
 drivers/usb/Makefile  |2 +-
 drivers/usb/early/Makefile|1 +
 drivers/usb/early/xhci-dbc.c  | 1097 +
 drivers/usb/early/xhci-dbc.h  |  206 
 include/linux/usb/xhci-dbgp.h |   22 +
 7 files changed, 1344 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/early/xhci-dbc.c
 create mode 100644 drivers/usb/early/xhci-dbc.h
 create mode 100644 include/linux/usb/xhci-dbgp.h

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 67eec55..13e85b7 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -29,6 +29,7 @@ config EARLY_PRINTK
 config EARLY_PRINTK_DBGP
bool "Early printk via EHCI debug port"
depends on EARLY_PRINTK && PCI
+   select USB_EARLY_PRINTK
---help---
  Write kernel log output directly into the EHCI debug port.
 
@@ -48,6 +49,19 @@ config EARLY_PRINTK_EFI
  This is useful for kernel debugging when your machine crashes very
  early before the console code is initialized.
 
+config EARLY_PRINTK_XDBC
+   bool "Early printk via xHCI debug port"
+   depends on EARLY_PRINTK && PCI
+   select USB_EARLY_PRINTK
+   ---help---
+ Write kernel log output directly into the xHCI debug port.
+
+ This is useful for kernel debugging when your machine crashes very
+ early before the console code is initialized. For normal operation
+ it is not recommended because it looks ugly and doesn't cooperate
+ with klogd/syslogd or the X server. You should normally N here,
+ unless you want to debug such a crash.
+
 config X86_PTDUMP_CORE
def_bool n
 
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 644e978..860d81b1 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -19,6 +19,9 @@ config USB_EHCI_BIG_ENDIAN_MMIO
 config USB_EHCI_BIG_ENDIAN_DESC
bool
 
+config USB_EARLY_PRINTK
+   bool
+
 menuconfig USB_SUPPORT
bool "USB support"
depends on HAS_IOMEM
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index dca7856..dd91ca1 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -48,7 +48,7 @@ obj-$(CONFIG_USB_MICROTEK)+= image/
 obj-$(CONFIG_USB_SERIAL)   += serial/
 
 obj-$(CONFIG_USB)  += misc/
-obj-$(CONFIG_EARLY_PRINTK_DBGP)+= early/
+obj-$(CONFIG_USB_EARLY_PRINTK) += early/
 
 obj-$(CONFIG_USB_ATM)  += atm/
 obj-$(CONFIG_USB_SPEEDTOUCH)   += atm/
diff --git a/drivers/usb/early/Makefile b/drivers/usb/early/Makefile
index 24bbe51..2db5906 100644
--- a/drivers/usb/early/Makefile
+++ b/drivers/usb/early/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_EARLY_PRINTK_DBGP) += ehci-dbgp.o
+obj-$(CONFIG_EARLY_PRINTK_XDBC) += xhci-dbc.o
diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
new file mode 100644
index 000..939fff2
--- /dev/null
+++ b/drivers/usb/early/xhci-dbc.c
@@ -0,0 +1,1097 @@
+/**
+ * xhci-dbc.c - xHCI debug capability early driver
+ *
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Author: Lu Baolu 
+ *
+ * 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.
+ */
+
+#define pr_fmt(fmt)KBUILD_MODNAME ":%s: " fmt, __func__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "../host/xhci.h"
+#include "xhci-dbc.h"
+
+static struct xdbc_state xdbc;
+static int 

Re: [RFC][PATCH 2/2] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

[John Youn]

On 10/16/2016 7:42 PM, Chen Yu wrote:
> 
> 
> On 2016/10/15 3:37, John Youn wrote:
>> On 10/13/2016 4:36 PM, John Stultz wrote:
>>> From: Chen Yu 
>>>
>>> The Hi6220's usb controller is limited in that it does not
>>> automatically autonegotiate the usb speed. Thus it requires a
>>> quirk so that we can manually negotiate the best usb speed for
>>> the attached device.
>>
>> Hi,
>>
>> Could you expand more on this by explaining what exactly is the
>> limitation and the workaround?
>>
> 
> The USB host limitation of Hisilicon Hi6220 is full-speed and low-speed
> devices can not be enumerated when gets plugged behind a hub.
> 
>> [snip]
>>
>>> +/*
>>> + * HPRT0_SPD_HIGH_SPEED: high speed
>>> + * HPRT0_SPD_FULL_SPEED: full speed
>>> + */
>>> +static void dwc2_change_bus_speed(struct usb_hcd *hcd, int speed)
>>> +{
>>> +   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
>>> +
>>> +   if (hsotg->core_params->speed == speed)
>>> +   return;
>>> +
>>> +   hsotg->core_params->speed = speed;
>>> +   queue_work(hsotg->wq_otg, >wf_otg);
>>> +}
>>> +
>>> +static int dwc2_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
>>> +{
>>> +   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
>>> +
>>> +   if (!hsotg->change_speed_quirk)
>>> +   return 1;
>>> +
>>> +   hsotg->device_count++;
>>
>> Why do you need to track the device count?
>>
>>> +   dev_info(hsotg->dev, "Device count is %u after alloc dev\n",
>>> +   hsotg->device_count);
>>> +
>>> +   return 1;
>>> +}
>>> +
>>> +static void dwc2_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
>>> +{
>>> +   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
>>> +
>>> +   if (!hsotg->change_speed_quirk)
>>> +   return;
>>> +
>>> +   if (hsotg->device_count)
>>> +   hsotg->device_count--;
>>> +
>>> +   dev_info(hsotg->dev, "Device count is %u after free dev\n",
>>> +   hsotg->device_count);
>>> +
>>> +   if (hsotg->device_count == 1 && udev->parent &&
>>> +   udev->parent->speed > USB_SPEED_UNKNOWN &&
>>> +   udev->parent->speed < USB_SPEED_HIGH) {
>>> +   dev_info(hsotg->dev, "Set speed to default high-speed\n");
>>> +   dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
>>> +   }
>>> +}
>>> +
>>> +static int dwc2_reset_device(struct usb_hcd *hcd, struct usb_device *udev)
>>> +{
>>> +   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
>>> +
>>> +   if (!hsotg->change_speed_quirk)
>>> +   return 0;
>>> +
>>> +   if (udev->speed == USB_SPEED_HIGH) {
>>> +   dev_info(hsotg->dev, "Set speed to high-speed\n");
>>> +   dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
>>> +   } else if (udev->speed == USB_SPEED_FULL
>>> +   || udev->speed == USB_SPEED_LOW) {
>>> +   dev_info(hsotg->dev, "Set speed to full-speed\n");
>>> +   dwc2_change_bus_speed(hcd, HPRT0_SPD_FULL_SPEED);
>>> +   }
>>
>> It seems you are reinitializing the core every time a device is reset
>> and the udev->speed does not match the core_param speed. But how is
>> the udev->speed being set correctly if the hw cannot negotiate the
>> speed in the first place?
>>
> 
> The hardware can negotiate the speed, but communication with a full-speed or
> low-speed device behind a hub is the problem.
> 
>> Also should it be for every device? What about if a device gets
>> plugged in behind a hub? I don't think you want to execute this code
>> in that case.
>>
>> This should only affect devices plugged into the root hub, correct?
>> And the hsotg controller only has one root hub port. It seems things
>> could be simplified a bit.
>>
> 
> The patch is initially written for Hikey Hi6220 board, and there is a
> hub always connected to root hub, so the patch sets the configuration to
> HPRT0_SPD_HIGH_SPEED when there is only one device(the hub).

Ok, I see.

> 
> Thanks for your suggestions, the patch needs modified in these aspect:
> 1. Change the speed setting only when the device is behind a hub in 
> dwc2_reset_device.

I still think you will have issues with multiple devices. Since you
have a built-in hub after root hub, it will always be behind the
hub. So whenver you need to change speeds, it will always reset every
device in the tree. Have you tested with multiple devices and also
multiple levels of hubs?

> 2. Change the speed to HPRT0_SPD_HIGH_SPEED only when the last device is a 
> hub.
> 
> What do you think about the fix? Any suggestions will be appreciate!

I'm not sure if any fix can work for all cases. Has this problem
always been there?

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

Re: [PATCH] xhci: Fix dereference from empty xhci->cmd_list

[OGAWA Hirofumi]

Mathias Nyman  writes:

>> OGAWA Hirofumi  writes:
>>> If race with timeout or stop event on empty cmd_list was happened,
>>> handle_cmd_completion() deferences wrong pointer from xhci->cmd_list.
>>>
>>> So this makes sure we don't dereference wrong pointer from empty
>>> xhci->cmd_list.
>>>
>>> Signed-off-by: OGAWA Hirofumi 
>>> ---
>
> Thanks, nice catch.
>
> What kernel is this based on, There is a partial fix in 4.8:
>
> commit 33be126510974e2eb9679f1ca9bca4f67ee4c4c7
>  xhci: always handle "Command Ring Stopped" events
>
> Looking at the existing code the case you describe could be possible if
> handle_cmd_completion is racing with:
> -  command timed out twice and xhci_cleanup_command_queue() is called
> -  or command timed out and we fail to restart command ring (assume host is 
> dead)
>
> It would be interesting to know how this was triggered, were there any error 
> messages like:
> "Abort command ring failed"
> or with xhci debugging enabled something like:
> "command timed out twice, ring start fail?"

Ah, sorry for confusing. I wrote this for older kernel (3.8.x, and not
for vanilla). So 4.8 might not have race what I saw actually.
(33be126510974e2eb9679f1ca9bca4f67ee4c4c7 seems to be prevent it)

However, even if 4.8 fixed that case, if there is spurious event or
driver state confusing, fetch and dereference empty list is possible and
bad thing. So, IMO, it would be better to make sure to avoid it.

>>> +   if (list_empty(>cmd_list))
>>> +   cmd = NULL;
>>> +   else {
>>> +   cmd = list_first_entry(>cmd_list, struct xhci_command,
>>> +  cmd_list);
>>> +   }
>>
>> These few lines could become a nice helper. Something like:
>>
>> static inline struct xhci_command *next_command(struct xhci_hcd *xhci)
>> {
>>  if (list_empty(>cmd_list))
>>  return NULL;
>>  return list_first_entry(>cmd_list, struct xhci_command, cmd_list);
>> }
>>
>
> Both works for me, helper function, or code in this patch.

There are some opencode list_head usages too. It might be better to
change at once as list cleanup if need.

E.g.

/* restart timer if this wasn't the last command */
if (cmd->cmd_list.next != >cmd_list) {

is

if (!list_is_last(>cmd_list, >cmd_list)) {

Well, so, if next_command() cleanup is not required, I'd like to use as
is, in this patch.

Thanks.
-- 
OGAWA Hirofumi 
--
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 49/58] zr364xx: don't break long lines

[Mauro Carvalho Chehab]

Due to the 80-cols restrictions, and latter due to checkpatch
warnings, several strings were broken into multiple lines. This
is not considered a good practice anymore, as it makes harder
to grep for strings at the source code.

As we're right now fixing other drivers due to KERN_CONT, we need
to be able to identify what printk strings don't end with a "\n".
It is a way easier to detect those if we don't break long lines.

So, join those continuation lines.

The patch was generated via the script below, and manually
adjusted if needed.


use Text::Tabs;
while (<>) {
if ($next ne "") {
$c=$_;
if ($c =~ /^\s+\"(.*)/) {
$c2=$1;
$next =~ s/\"\n$//;
$n = expand($next);
$funpos = index($n, '(');
$pos = index($c2, '",');
if ($funpos && $pos > 0) {
$s1 = substr $c2, 0, $pos + 2;
$s2 = ' ' x ($funpos + 1) . substr $c2, $pos + 
2;
$s2 =~ s/^\s+//;

$s2 = ' ' x ($funpos + 1) . $s2 if ($s2 ne "");

print unexpand("$next$s1\n");
print unexpand("$s2\n") if ($s2 ne "");
} else {
print "$next$c2\n";
}
$next="";
next;
} else {
print $next;
}
$next="";
} else {
if (m/\"$/) {
if (!m/\\n\"$/) {
$next=$_;
next;
}
}
}
print $_;
}


Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/zr364xx/zr364xx.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/zr364xx/zr364xx.c 
b/drivers/media/usb/zr364xx/zr364xx.c
index cc128db85723..3950708cbb32 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -633,8 +633,7 @@ static int zr364xx_read_video_callback(struct 
zr364xx_camera *cam,
} else {
if (frm->cur_size + purb->actual_length > MAX_FRAME_SIZE) {
dev_info(>udev->dev,
-"%s: buffer (%d bytes) too small to hold "
-"frame data. Discarding frame data.\n",
+"%s: buffer (%d bytes) too small to hold frame 
data. Discarding frame data.\n",
 __func__, MAX_FRAME_SIZE);
} else {
pdest += frm->cur_size;
@@ -1373,8 +1372,7 @@ static int zr364xx_board_init(struct zr364xx_camera *cam)
>buffer.frame[i], i,
cam->buffer.frame[i].lpvbits);
if (cam->buffer.frame[i].lpvbits == NULL) {
-   printk(KERN_INFO KBUILD_MODNAME ": out of memory. "
-  "Using less frames\n");
+   printk(KERN_INFO KBUILD_MODNAME ": out of memory. Using 
less frames\n");
break;
}
}
-- 
2.7.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 net-next] r8152: add new products of Lenovo

[David Miller]

From: Hayes Wang 
Date: Tue, 18 Oct 2016 11:41:48 +0800

> Add the following four products of Lenovo and sort the order of the list.
> 
>   VID PID
>   0x17ef  0x3062
>   0x17ef  0x3069
>   0x17ef  0x720c
>   0x17ef  0x7214
> 
> Signed-off-by: Hayes Wang 

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

Re: [PATCH v5 03/23] usb: ulpi: Support device discovery via DT

[Rob Herring]

On Mon, Oct 17, 2016 at 06:56:16PM -0700, Stephen Boyd wrote:
> The qcom HSIC ULPI phy doesn't have any bits set in the vendor or
> product ID registers. This makes it impossible to make a ULPI
> driver match against the ID registers. Add support to discover
> the ULPI phys via DT help alleviate this problem. In the DT case,
> we'll look for a ULPI bus node underneath the device registering
> the ULPI viewport (or the parent of that device to support
> chipidea's device layout) and then match up the phy node
> underneath that with the ULPI device that's created.
> 
> The side benefit of this is that we can use standard properties
> in the phy node like clks, regulators, gpios, etc. because we
> don't have firmware like ACPI to turn these things on for us. And
> we can use the DT phy binding to point our phy consumer to the
> phy provider.
> 
> The ULPI bus code supports native enumeration by reading the
> vendor ID and product ID registers at device creation time, but
> we can't be certain that those register reads will succeed if the
> phy is not powered up. To avoid any problems with reading the ID
> registers before the phy is powered we fallback to DT matching
> when the ID reads fail.
> 
> If the ULPI spec had some generic power sequencing for these
> registers we could put that into the ULPI bus layer and power up
> the device before reading the ID registers. Unfortunately this
> doesn't exist and the power sequence is usually device specific.
> By having the device matched up with DT we can avoid this
> problem.
> 
> Cc: Greg Kroah-Hartman 
> Cc: Heikki Krogerus 
> Cc: 
> Cc: Rob Herring 
> Signed-off-by: Stephen Boyd 
> ---
>  Documentation/devicetree/bindings/usb/ulpi.txt | 20 +++
>  drivers/usb/common/ulpi.c  | 79 
> --
>  2 files changed, 93 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/ulpi.txt

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

Re: [PATCH v5 23/23] phy: Add support for Qualcomm's USB HS phy

[Rob Herring]

On Mon, Oct 17, 2016 at 06:56:36PM -0700, Stephen Boyd wrote:
> The high-speed phy on qcom SoCs is controlled via the ULPI
> viewport.
> 
> Cc: Kishon Vijay Abraham I 
> Cc: 
> Cc: Rob Herring 
> Signed-off-by: Stephen Boyd 
> ---
>  .../devicetree/bindings/phy/qcom,usb-hs-phy.txt|  86 +++

Acked-by: Rob Herring 

>  drivers/phy/Kconfig|   8 +
>  drivers/phy/Makefile   |   1 +
>  drivers/phy/phy-qcom-usb-hs.c  | 286 
> +
>  4 files changed, 381 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
>  create mode 100644 drivers/phy/phy-qcom-usb-hs.c
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 23/23] phy: Add support for Qualcomm's USB HS phy

[Rob Herring]

On Mon, Oct 17, 2016 at 06:56:36PM -0700, Stephen Boyd wrote:
> The high-speed phy on qcom SoCs is controlled via the ULPI
> viewport.
> 
> Cc: Kishon Vijay Abraham I 
> Cc: 
> Cc: Rob Herring 
> Signed-off-by: Stephen Boyd 
> ---
>  .../devicetree/bindings/phy/qcom,usb-hs-phy.txt|  86 +++

Acked-by: Rob Herring 

>  drivers/phy/Kconfig|   8 +
>  drivers/phy/Makefile   |   1 +
>  drivers/phy/phy-qcom-usb-hs.c  | 286 
> +
>  4 files changed, 381 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
>  create mode 100644 drivers/phy/phy-qcom-usb-hs.c
--
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: crash by cdc_acm driver in kernels 4.8-rc1/5

[Wim Osterholt]

On Tue, Oct 18, 2016 at 02:18:43PM +0200, Oliver Neukum wrote:
> Oct 18 14:05:07 linux-dtbq.site kernel: usb 1-9: Manufacturer: Conexant
> Oct 18 14:05:07 linux-dtbq.site kernel: usb 1-9: SerialNumber: 12345678

With that unique serial number it must be that very device. :-)

> It definitely does not crash and is probed and your .config is not
> extremely unusual.
> I am afraid unless you test the last patch I sent we will not make
> progress. Something odd is going on.

Whell, I DID test that patch and it already crashed before it could print
anything. That's why the output I sent you looked the same.

Once again, this time on 4.9-rc1.
Applied your patch 0001-CDC-ACM-more-paranoid-debugging to cdc_acm.c .

Did
> > dmesg -c
> > echo 9 > /proc/sysrq-trigger
> > modprobe cdc_acm
> > echo "module cdc_acm +mpf" > /sys/kernel/debug/dynamic_debug/control
> >
> > [plug your device in]
> >
> > and provide the full output of dmesg after that.

Got
[  765.409057] sysrq: SysRq : Changing Loglevel
[  765.416465] sysrq: Loglevel set to 9
[  778.299271] usbcore: registered new interface driver cdc_acm
[  778.301921] cdc_acm: USB Abstract Control Model driver for USB modems and 
ISDN adapters
[  833.204100] usb 6-1: new full-speed USB device number 2 using uhci_hcd
[  833.411088] usb 6-1: New USB device found, idVendor=0572, idProduct=1340
[  833.412127] usb 6-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[  833.416129] usb 6-1: Product: USB Modem
[  833.420123] usb 6-1: Manufacturer: Conexant
[  833.420126] usb 6-1: SerialNumber: 12345678
[  833.473854] cdc_acm:acm_probe: cdc_acm 6-1:1.0: interfaces are valid
[  833.473876] BUG: unable to handle kernel NULL pointer dereference at 0249
[  833.473882] IP: [] acm_probe+0x540/0xd00 [cdc_acm]
[  833.473885] *pde =  
[  833.473887] Oops:  [#1] SMP
[  833.473925] Modules linked in: cdc_acm nouveau video drm_kms_helper 
syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm agpgart i2c_algo_bit 
cfg80211 rfkill binfmt_misc svgalib_helper(O) snd_pcm_oss snd_mixer_oss fbcon 
bitblit softcursor font tileblit sr9700 dm9601 snd_hda_codec_generic usbnet 
usb_storage snd_hda_intel mii snd_hda_codec tg3 snd_hwdep snd_hda_core ptp 
pps_core snd_pcm libphy gpio_ich snd_timer firmware_class lpc_ich pcspkr ppdev 
snd ohci_pci mfd_core ohci_hcd floppy wmi uhci_hcd soundcore parport_pc 
acpi_cpufreq ehci_pci parport ehci_hcd processor button
[  833.473928] CPU: 0 PID: 4 Comm: kworker/0:0 Tainted: G   O
4.9.0-rc1 #1
[  833.473930] Hardware name: Hewlett-Packard HP xw4300 Workstation/0A00h, BIOS 
786D3 v01.08 03/10/2006
[  833.473935] Workqueue: usb_hub_wq hub_event
[  833.473937] task: df4e15c0 task.stack: df4f4000
[  833.473939] EIP: 0060:[] EFLAGS: 00010202 CPU: 0
[  833.473942] EIP is at acm_probe+0x540/0xd00 [cdc_acm]
[  833.473944] EAX: 0246 EBX: dc4b2800 ECX: e08fe594 EDX: 
[  833.473945] ESI: 0100 EDI:  EBP: df4f5c18 ESP: df4f5b80
[  833.473947]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[  833.473949] CR0: 80050033 CR2: 0249 CR3: 1c8c4000 CR4: 0690
[  833.473950] Stack:
[  833.473956]  3a20 3de7 000f df4a9d50   0010 
0040
[  833.473960]  0080 0246 dee5f000 d9614d80 d960e070 0001 d2aee100 
d960e000
[  833.473965]  d2aee138 dee5f400 dee5f000  c82931b0 0004 0246 
df4f5c00
[  833.473966] Call Trace:
[  833.473975]  [] ? __mutex_unlock_slowpath+0xf4/0xfc
[  833.473978]  [] ? usb_probe_interface+0x17b/0x1f6
[  833.473980]  [] ? usb_probe_interface+0x17b/0x1f6
[  833.473984]  [] ? driver_probe_device+0x17b/0x30e
[  833.473986]  [] ? driver_probe_device+0x17b/0x30e
[  833.473989]  [] ? bus_for_each_drv+0x59/0x68
[  833.473991]  [] ? bus_for_each_drv+0x59/0x68
[  833.473993]  [] ? __device_attach+0x91/0x105
[  833.473996]  [] ? driver_allows_async_probing+0x2f/0x2f
[  833.473998]  [] ? bus_probe_device+0x27/0x6b
[  833.474000]  [] ? bus_probe_device+0x27/0x6b
[  833.474002]  [] ? device_add+0x28d/0x4c0
[  833.474006]  [] ? usb_set_configuration+0x594/0x5d7
[  833.474008]  [] ? usb_set_configuration+0x594/0x5d7
[  833.474012]  [] ? generic_probe+0x3b/0x67
[  833.474014]  [] ? generic_probe+0x3b/0x67
[  833.474016]  [] ? usb_probe_device+0x49/0x62
[  833.474017]  [] ? usb_suspend+0xcd/0xcd
[  833.474020]  [] ? driver_probe_device+0x17b/0x30e
[  833.474022]  [] ? driver_probe_device+0x17b/0x30e
[  833.474024]  [] ? bus_for_each_drv+0x59/0x68
[  833.474026]  [] ? bus_for_each_drv+0x59/0x68
[  833.474028]  [] ? __device_attach+0x91/0x105
[  833.474031]  [] ? driver_allows_async_probing+0x2f/0x2f
[  833.474033]  [] ? bus_probe_device+0x27/0x6b
[  833.474035]  [] ? bus_probe_device+0x27/0x6b
[  833.474037]  [] ? device_add+0x28d/0x4c0
[  833.474041]  [] ? add_device_randomness+0x84/0x9c
[  833.474043]  [] ? usb_new_device+0x29d/0x3b5
[  833.474045]  [] ? usb_new_device+0x29d/0x3b5
[  833.474048]  [] ? hub_event+0xb32/0xed8
[  833.474050]  [] ? 

Re: [PATCH] xhci: Fix dereference from empty xhci->cmd_list

[Mathias Nyman]

Hi

On 18.10.2016 16:07, Felipe Balbi wrote:


Hi,

OGAWA Hirofumi  writes:

If race with timeout or stop event on empty cmd_list was happened,
handle_cmd_completion() deferences wrong pointer from xhci->cmd_list.

So this makes sure we don't dereference wrong pointer from empty
xhci->cmd_list.

Signed-off-by: OGAWA Hirofumi 
---


Thanks, nice catch.

What kernel is this based on, There is a partial fix in 4.8:

commit 33be126510974e2eb9679f1ca9bca4f67ee4c4c7
xhci: always handle "Command Ring Stopped" events


Looking at the existing code the case you describe could be possible if
handle_cmd_completion is racing with:
-  command timed out twice and xhci_cleanup_command_queue() is called
-  or command timed out and we fail to restart command ring (assume host is 
dead)

It would be interesting to know how this was triggered, were there any error 
messages like:
"Abort command ring failed"
or with xhci debugging enabled something like:
"command timed out twice, ring start fail?"



  drivers/usb/host/xhci-ring.c |   12 +---
  1 file changed, 9 insertions(+), 3 deletions(-)

diff -puN drivers/usb/host/xhci-ring.c~xhci-fix-empty-list 
drivers/usb/host/xhci-ring.c
--- linux/drivers/usb/host/xhci-ring.c~xhci-fix-empty-list  2016-10-18 
21:20:07.973055018 +0900
+++ linux-hirofumi/drivers/usb/host/xhci-ring.c 2016-10-18 21:21:35.949060365 
+0900
@@ -1336,7 +1336,12 @@ static void handle_cmd_completion(struct
return;
}

-   cmd = list_entry(xhci->cmd_list.next, struct xhci_command, cmd_list);




+   if (list_empty(>cmd_list))
+   cmd = NULL;
+   else {
+   cmd = list_first_entry(>cmd_list, struct xhci_command,
+  cmd_list);
+   }


These few lines could become a nice helper. Something like:

static inline struct xhci_command *next_command(struct xhci_hcd *xhci)
{
 if (list_empty(>cmd_list))
return NULL;
return list_first_entry(>cmd_list, struct xhci_command, cmd_list);
}



Both works for me, helper function, or code in this patch.


@@ -1350,9 +1355,10 @@ static void handle_cmd_completion(struct


Before the lines below, there's a call to del_timer() and a
tracepoint. I have a feeling we don't want those to happen if the list
is corrupt. Perhaps you should skip del_timer() and adding a log to
trace buffer, no?


I think those are fine, they shouldn't matter. With a empty command list
we don't wan't any command timeout timer to trigger, and the trancepoint
prints out info about the command trb on the command ring and event
trb on event ring. It doesn't touch the struct command  in the command list.

-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: [alsa-devel] Jack sensing in snd_usb_audio ?

[Takashi Iwai]

On Tue, 18 Oct 2016 15:33:42 +0200,
Felipe Ferreri Tonello wrote:
> 
> Hi Takashi
> 
> On 18/10/16 13:07, Takashi Iwai wrote:
> > On Wed, 12 Oct 2016 18:15:04 +0200,
> > Bastien Nocera wrote:
> >>
> >> On Wed, 2016-10-12 at 18:06 +0200, Clemens Ladisch wrote:
> >>> Bastien Nocera wrote:
>  On Wed, 2016-10-12 at 14:43 +0200, Clemens Ladisch wrote:
> > Bastien Nocera wrote:
> >> "
> >> A change of state in the audio function is most often caused by
> >> a
> >> certain event that takes place. An event can either be user-
> >> initiated
> >> or device-initiated. User-initiated jack insertion or removal
> >> is a
> >> typical example of a user-initiated event.
> >> "
> >
> >
> > There are not many USB Audio 2.0 devices, and I'm not aware of
> > any
> > that actually implements this.
> 
> 
>  I guess I would see whether there are events if I captured the USB
>  traffic even without special handling/turning on a feature in the
>  drivers, right?
> >>>
> >>>
> >>> Most devices do not even have the status endpoint (see "lsusb -v").
> >>> To check what events arrive, you can add logging to the
> >>> snd_usb_mixer_interrupt() function.
> >>
> >> I'm guessing it doesn't support it then (see attached log)
> > 
> > So this looks like a HID, not from the audio device class.
> > It's an oft-seen implementation.
> > 
> >> I also checked the input device output when plugging in something, with
> >> evtest, and no feedback either.
> > 
> > Then at first you need to hack a HID driver to support this device.
> > It'll create an input device, and then we'll need to find some way to
> > couple the given input device and the audio device.  We can parse the
> > sysfs device path to figure out, but I'm not sure what's the best way
> > to tell it to applications.
> > 
> 
> Why not use similar API as a normal ALSA card? This will enable jack
> detection by default in applications using kcontrol interface.

Are you suggesting to create another sound card object by a HID
driver?  This would be even less useful.  Then you'll have two
individual sound cards in the end that have no connection between
them.


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

Re: [PATCH 2/4] USB: ch341: reinitialize chip on reconfiguration

[Johan Hovold]

On Tue, Oct 18, 2016 at 12:39:23PM -, Karl Palsson wrote:
> 
> Johan Hovold  wrote:
> > On Sun, Oct 16, 2016 at 05:09:24PM +0100, Aidan Thornton wrote:
> > > On Mon, Oct 10, 2016 at 8:56 PM, Grigori Goronzy  
> > > wrote:
> > > > On 2016-10-08 23:07, Aidan Thornton wrote:
> > > >>
> > > >> On Fri, Oct 7, 2016 at 12:30 PM, Johan Hovold  wrote:
> > > >>>
> > > >>> Why is this change needed? I see no write to this register in the
> > > >>> vendor driver.
> > > >>
> > > >>
> > > >> In principle, it might not be because the value written to register
> > > >> 0x18 is probably overwritten by ch341_init_set_baudrate anyway. It's
> > > >> there because it's in Grigori's original patch, it looks superficially
> > > >> reasonable (corresponds to ENABLE_RX|ENABLE_TX|CS8, as opposed to the
> > > >> rather implausible ENABLE_TX|PAR_EVEN|CS5), and given that some
> > > >> hardware revisions are apparently a little temperamental I was
> > > >> reluctant to remove it without fully understanding why it was added in
> > > >> the first place. As the vendor driver does without it might make sense
> > > >> to delete the write altogether, but it does do quite a few things
> > > >> differently from this driver. Depends what Grigori says and the
> > > >> results of actual testing.
> > 
> > Ok, thanks for the explanation. I think we should try to get
> > rid of anything apparently redundant eventually, moving towards
> > how the vendor driver does things. It would also be good to
> > replace more of the magic constants in there to make the code
> > more self-describing.
> > 
> > As long this is done in steps that can be (more easily)
> > rolled-back in case it turns out we do actually break support
> > for some device in the process.
> 
> Perhaps it needs to be repeated more, but the current driver is
> so badly broken for so many devices that people simply don't use
> them. This insistence that any patch trying to fix this driver
> somehow preserves all the existing undocumented, unexplained and
> _non-functioning_ bizarre lines of code is exhausting.

No one has insisted on that as far as I'm aware. Please re-read my reply
above where I explicitly say we should be moving towards how the vendor
driver does things and try to get rid of this legacy, reverse-engineered
cruft. This still needs to be done using the normal kernel development
process, which is to do things incrementally in self-contained steps of
logical changes.

> There's been probably ~6 people now submit patches to this
> driver, all of which make marked useful improvements to a driver
> that _is_broken_ and they're all in limbo because "compatibility
> with unknown magic number XYZ" that cant' be explained anyway.

We still cannot completely disregard current user of this driver, and
the normal process of incremental changes still applies.

If we have reasonable test coverage for a change, we'll go ahead. And in
the unlikely event that this breaks some legacy device, we'll deal with
it then.

> Is there any chance of any improvements to this driver ever
> landing? The requirements have been so high, when the existing
> was s poor.

Certainly. This series is one line away from getting merged for example.

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

Re: [alsa-devel] Jack sensing in snd_usb_audio ?

[Felipe Ferreri Tonello]

Hi Takashi

On 18/10/16 13:07, Takashi Iwai wrote:
> On Wed, 12 Oct 2016 18:15:04 +0200,
> Bastien Nocera wrote:
>>
>> On Wed, 2016-10-12 at 18:06 +0200, Clemens Ladisch wrote:
>>> Bastien Nocera wrote:
 On Wed, 2016-10-12 at 14:43 +0200, Clemens Ladisch wrote:
> Bastien Nocera wrote:
>> "
>> A change of state in the audio function is most often caused by
>> a
>> certain event that takes place. An event can either be user-
>> initiated
>> or device-initiated. User-initiated jack insertion or removal
>> is a
>> typical example of a user-initiated event.
>> "
>
>
> There are not many USB Audio 2.0 devices, and I'm not aware of
> any
> that actually implements this.


 I guess I would see whether there are events if I captured the USB
 traffic even without special handling/turning on a feature in the
 drivers, right?
>>>
>>>
>>> Most devices do not even have the status endpoint (see "lsusb -v").
>>> To check what events arrive, you can add logging to the
>>> snd_usb_mixer_interrupt() function.
>>
>> I'm guessing it doesn't support it then (see attached log)
> 
> So this looks like a HID, not from the audio device class.
> It's an oft-seen implementation.
> 
>> I also checked the input device output when plugging in something, with
>> evtest, and no feedback either.
> 
> Then at first you need to hack a HID driver to support this device.
> It'll create an input device, and then we'll need to find some way to
> couple the given input device and the audio device.  We can parse the
> sysfs device path to figure out, but I'm not sure what's the best way
> to tell it to applications.
> 

Why not use similar API as a normal ALSA card? This will enable jack
detection by default in applications using kcontrol interface.

-- 
Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: [PATCH] xhci: Fix dereference from empty xhci->cmd_list

[Felipe Balbi]

Hi,

OGAWA Hirofumi  writes:
> If race with timeout or stop event on empty cmd_list was happened,
> handle_cmd_completion() deferences wrong pointer from xhci->cmd_list.
>
> So this makes sure we don't dereference wrong pointer from empty
> xhci->cmd_list.
>
> Signed-off-by: OGAWA Hirofumi 
> ---
>
>  drivers/usb/host/xhci-ring.c |   12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff -puN drivers/usb/host/xhci-ring.c~xhci-fix-empty-list 
> drivers/usb/host/xhci-ring.c
> --- linux/drivers/usb/host/xhci-ring.c~xhci-fix-empty-list2016-10-18 
> 21:20:07.973055018 +0900
> +++ linux-hirofumi/drivers/usb/host/xhci-ring.c   2016-10-18 
> 21:21:35.949060365 +0900
> @@ -1336,7 +1336,12 @@ static void handle_cmd_completion(struct
>   return;
>   }
>  
> - cmd = list_entry(xhci->cmd_list.next, struct xhci_command, cmd_list);


> + if (list_empty(>cmd_list))
> + cmd = NULL;
> + else {
> + cmd = list_first_entry(>cmd_list, struct xhci_command,
> +cmd_list);
> + }

These few lines could become a nice helper. Something like:

static inline struct xhci_command *next_command(struct xhci_hcd *xhci)
{
if (list_empty(>cmd_list))
return NULL;
return list_first_entry(>cmd_list, struct xhci_command, cmd_list);
}

> @@ -1350,9 +1355,10 @@ static void handle_cmd_completion(struct

Before the lines below, there's a call to del_timer() and a
tracepoint. I have a feeling we don't want those to happen if the list
is corrupt. Perhaps you should skip del_timer() and adding a log to
trace buffer, no?

>   return;
>   }
>  
> - if (cmd->command_trb != xhci->cmd_ring->dequeue) {
> + if (cmd == NULL || cmd->command_trb != xhci->cmd_ring->dequeue) {
>   xhci_err(xhci,
> -  "Command completion event does not match command\n");
> +  "Command completion event %s\n",
> +  cmd ? "does not match command" : "on empty list");
>   return;
>   }

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] xhci: Fix dereference from empty xhci->cmd_list

[OGAWA Hirofumi]

If race with timeout or stop event on empty cmd_list was happened,
handle_cmd_completion() deferences wrong pointer from xhci->cmd_list.

So this makes sure we don't dereference wrong pointer from empty
xhci->cmd_list.

Signed-off-by: OGAWA Hirofumi 
---

 drivers/usb/host/xhci-ring.c |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff -puN drivers/usb/host/xhci-ring.c~xhci-fix-empty-list 
drivers/usb/host/xhci-ring.c
--- linux/drivers/usb/host/xhci-ring.c~xhci-fix-empty-list  2016-10-18 
21:20:07.973055018 +0900
+++ linux-hirofumi/drivers/usb/host/xhci-ring.c 2016-10-18 21:21:35.949060365 
+0900
@@ -1336,7 +1336,12 @@ static void handle_cmd_completion(struct
return;
}
 
-   cmd = list_entry(xhci->cmd_list.next, struct xhci_command, cmd_list);
+   if (list_empty(>cmd_list))
+   cmd = NULL;
+   else {
+   cmd = list_first_entry(>cmd_list, struct xhci_command,
+  cmd_list);
+   }
 
del_timer(>cmd_timer);
 
@@ -1350,9 +1355,10 @@ static void handle_cmd_completion(struct
return;
}
 
-   if (cmd->command_trb != xhci->cmd_ring->dequeue) {
+   if (cmd == NULL || cmd->command_trb != xhci->cmd_ring->dequeue) {
xhci_err(xhci,
-"Command completion event does not match command\n");
+"Command completion event %s\n",
+cmd ? "does not match command" : "on empty list");
return;
}
 
_

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

Re: [PATCH 2/4] USB: ch341: reinitialize chip on reconfiguration

[Karl Palsson]

Johan Hovold  wrote:
> On Sun, Oct 16, 2016 at 05:09:24PM +0100, Aidan Thornton wrote:
> > On Mon, Oct 10, 2016 at 8:56 PM, Grigori Goronzy  wrote:
> > > On 2016-10-08 23:07, Aidan Thornton wrote:
> > >>
> > >> On Fri, Oct 7, 2016 at 12:30 PM, Johan Hovold  wrote:
> > >>>
> > >>> Why is this change needed? I see no write to this register in the
> > >>> vendor driver.
> > >>
> > >>
> > >> In principle, it might not be because the value written to register
> > >> 0x18 is probably overwritten by ch341_init_set_baudrate anyway. It's
> > >> there because it's in Grigori's original patch, it looks superficially
> > >> reasonable (corresponds to ENABLE_RX|ENABLE_TX|CS8, as opposed to the
> > >> rather implausible ENABLE_TX|PAR_EVEN|CS5), and given that some
> > >> hardware revisions are apparently a little temperamental I was
> > >> reluctant to remove it without fully understanding why it was added in
> > >> the first place. As the vendor driver does without it might make sense
> > >> to delete the write altogether, but it does do quite a few things
> > >> differently from this driver. Depends what Grigori says and the
> > >> results of actual testing.
> 
> Ok, thanks for the explanation. I think we should try to get
> rid of anything apparently redundant eventually, moving towards
> how the vendor driver does things. It would also be good to
> replace more of the magic constants in there to make the code
> more self-describing.
> 
> As long this is done in steps that can be (more easily)
> rolled-back in case it turns out we do actually break support
> for some device in the process.

Perhaps it needs to be repeated more, but the current driver is
so badly broken for so many devices that people simply don't use
them. This insistence that any patch trying to fix this driver
somehow preserves all the existing undocumented, unexplained and
_non-functioning_ bizarre lines of code is exhausting.
There's been probably ~6 people now submit patches to this
driver, all of which make marked useful improvements to a driver
that _is_broken_ and they're all in limbo because "compatibility
with unknown magic number XYZ" that cant' be explained anyway.

Is there any chance of any improvements to this driver ever
landing? The requirements have been so high, when the existing
was s poor.

Sincerely,
Karl Palsson

signature.asc
Description: OpenPGP Digital Signature

Re: crash by cdc_acm driver in kernels 4.8-rc1/5

[Oliver Neukum]

On Mon, 2016-10-17 at 17:20 +0200, Wim Osterholt wrote:
> On Mon, Oct 17, 2016 at 04:10:45PM +0200, Oliver Neukum wrote:
> > Hi,
> >
> > I got one of those devices. However, I don't get a crash.
> > Could you please give me instructions on how you trigger it?
>   
> That's not too hard, just plug it in. :-)
>   
> However you must have set cdc_acm in your kernel, or availabe as a module. 
> It happens on all my machines on kernels 4.8 and 4.9.
> Now, all my kernel configs will differ a bit, but must have something
> peculiar in common. Or you've received a totally different device.
>  
> Here's one config at http://webserver.djo.tudelft.nl/.config-4.8.1
>  
> Many options are inherited by 'make oldconfig' from version to version,
> without me knowing what it all means. So maybe it's just a weird combination
> of options then?

Oct 18 14:05:07 linux-dtbq.site kernel: usb 1-9: new full-speed USB device 
number 10 using xhci_hcd
Oct 18 14:05:07 linux-dtbq.site mtp-probe[2583]: checking bus 1, device 10: 
"/sys/devices/pci:00/:00:14.0/usb1/1-9"
Oct 18 14:05:07 linux-dtbq.site mtp-probe[2583]: bus: 1, device: 10 was not an 
MTP device
Oct 18 14:05:07 linux-dtbq.site kernel: usb 1-9: New USB device found, 
idVendor=0572, idProduct=1340
Oct 18 14:05:07 linux-dtbq.site kernel: usb 1-9: New USB device strings: Mfr=1, 
Product=2, SerialNumber=3
Oct 18 14:05:07 linux-dtbq.site kernel: usb 1-9: Product: USB Modem
Oct 18 14:05:07 linux-dtbq.site kernel: usb 1-9: Manufacturer: Conexant
Oct 18 14:05:07 linux-dtbq.site kernel: usb 1-9: SerialNumber: 12345678
Oct 18 14:05:07 linux-dtbq.site kernel: cdc_acm 1-9:1.0: ttyACM0: USB ACM device
Oct 18 14:05:07 linux-dtbq.site kernel: usbcore: registered new interface 
driver cdc_acm
Oct 18 14:05:07 linux-dtbq.site kernel: cdc_acm: USB Abstract Control Model 
driver for USB modems and ISDN adapters
Oct 18 14:05:11 linux-dtbq.site ModemManager[901]:   Creating modem with 
plugin 'Generic' and '1' ports
Oct 18 14:05:11 linux-dtbq.site ModemManager[901]:   Modem for device at 
'/sys/devices/pci:00/:00:14.0/usb1/1-9' successfully created
Oct 18 14:05:11 linux-dtbq.site ModemManager[901]:   Modem couldn't be 
initialized: couldn't load current capabilities: Failed to determine modem 
capabilities.
Oct 18 14:05:11 linux-dtbq.site NetworkManager[945]:  (ttyACM0): failed 
to look up interface index
Oct 18 14:05:11 linux-dtbq.site NetworkManager[945]:  (ttyACM0): device 
state change: unmanaged -> unavailable (reason 'none') [10 20 0]
Oct 18 14:05:11 linux-dtbq.site NetworkManager[945]:  (ttyACM0): 
deactivating device (reason 'none') [0] 
   
Oct 18 14:05:11 linux-dtbq.site NetworkManager[945]:  (ttyACM0): modem 
state 'unknown' 
 
Oct 18 14:05:11 linux-dtbq.site NetworkManager[945]:  (ttyACM0): new 
Broadband device (driver: 'cdc_acm' ifindex: 0) 
   
Oct 18 14:05:11 linux-dtbq.site NetworkManager[945]:  (ttyACM0): exported 
as /org/freedesktop/NetworkManager/Devices/3 

It definitely does not crash and is probed and your .config is not
extremely unusual.
I am afraid unless you test the last patch I sent we will not make
progress. Something odd is going on.

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: PROBLEM: DWC3 USB 3.0 not working on Odroid-XU4 with Exynos 5422

[Michael Niewöhner]

Hi,
On Mon, 2016-10-17 at 15:21 +0530, Vivek Gautam wrote:
> 
> On 10/17/2016 01:38 PM, Felipe Balbi wrote:
> > Hi,
> > 
> > Michael Niewöhner  writes:
> > > Hi Felipe,
> > > On Fri, 2016-10-07 at 22:26 +0200, Michael Niewöhner wrote:
> > > > Hi Felipe,
> > > > 
> > > > On Fr, 2016-10-07 at 10:42 +0300, Felipe Balbi wrote:
> > > > > Hi,
> > > > > 
> > > > > Michael Niewöhner  writes:
> > > > > > > The clocks are same across working/non-working.
> > > > > > > Is it possible to bisect the commit that's causing hang for 4.8x ?
> > > > > > 
> > > > > > [c499ff71ff2a281366c6ec7a904c547d806cbcd1] usb: dwc3: core: 
> > > > > > re-factor init and exit paths
> > > > > > This patch causes both the hang on reboot and the lsusb hang.
> > > > > 
> > > > > How to reproduce? Why don't we see this on x86 and TI boards? I'm
> > > > > guessing this is failed bisection, as I can't see anything in that
> > > > > commit that would cause reboot hang. Also, that code path is *NOT*
> > > > > executed when you run lsusb.
> > > > > 
> > > > 
> > > > I've tested this procedure multiple times to be sure:
> > > > 
> > > > - checkout c499ff71, compile, boot the odroid
> > > > - run lsusb -v => lsusb hangs, can't terminate with ctrl-c
> > > > - hard reset, after boot run poweroff or reboot => board does not 
> > > > completely power off / reboot (see log below)
> > > > - revert c499ff71, mrproper, compile, boot the odroid
> > > > - run lsusb -v => shows full output, not hanging
> > > > - run reboot or poweroff => board powers off / reboots just fine
> > > > 
> > > > 
> > > > dmesg poweroff not working:
> > > > ...
> > > > [  120.733519] systemd-journald[144]: systemd-journald stopped as pid 
> > > > 144
> > > > [  120.742663] systemd-shutdown[1]: Sending SIGKILL to remaining 
> > > > processes...
> > > > [  120.769212] systemd-shutdown[1]: Unmounting file systems.
> > > > [  120.773713] systemd-shutdown[1]: Unmounting /sys/kernel/debug.
> > > > [  120.827211] systemd-shutdown[1]: Unmounting /dev/mqueue.
> > > > [  121.081672] EXT4-fs (mmcblk1p2): re-mounted. Opts: (null)
> > > > [  121.091687] EXT4-fs (mmcblk1p2): re-mounted. Opts: (null)
> > > > [  121.095608] EXT4-fs (mmcblk1p2): re-mounted. Opts: (null)
> > > > [  121.101014] systemd-shutdown[1]: All filesystems unmounted.
> > > > [  121.106523] systemd-shutdown[1]: Deactivating swaps.
> > > > [  121.111585] systemd-shutdown[1]: All swaps deactivated.
> > > > [  121.116661] systemd-shutdown[1]: Detaching loop devices.
> > > > [  121.126395] systemd-shutdown[1]: All loop devices detached.
> > > > [  121.130525] systemd-shutdown[1]: Detaching DM devices.
> > > > [  121.135824] systemd-shutdown[1]: All DM devices detached.
> > > > [  121.166327] systemd-shutdown[1]: /lib/systemd/system-shutdown 
> > > > succeeded.
> > > > [  121.171739] systemd-shutdown[1]: Powering off.
> > > > 
> > > > => at this point removing the sd card would show a message
> > > > "removed mmc0" (not sure what the real message was...) so the board is 
> > > > not completely off.
> > > > 
> > > > 
> > > > dmesg poweroff working:
> > > > ...
> > > > [  120.733519] systemd-journald[144]: systemd-journald stopped as pid 
> > > > 144
> > > > [  120.742663] systemd-shutdown[1]: Sending SIGKILL to remaining 
> > > > processes...
> > > > [  120.769212] systemd-shutdown[1]: Unmounting file systems.
> > > > [  120.773713] systemd-shutdown[1]: Unmounting /sys/kernel/debug.
> > > > [  120.827211] systemd-shutdown[1]: Unmounting /dev/mqueue.
> > > > [  121.081672] EXT4-fs (mmcblk1p2): re-mounted. Opts: (null)
> > > > [  121.091687] EXT4-fs (mmcblk1p2): re-mounted. Opts: (null)
> > > > [  121.095608] EXT4-fs (mmcblk1p2): re-mounted. Opts: (null)
> > > > [  121.101014] systemd-shutdown[1]: All filesystems unmounted.
> > > > [  121.106523] systemd-shutdown[1]: Deactivating swaps.
> > > > [  121.111585] systemd-shutdown[1]: All swaps deactivated.
> > > > [  121.116661] systemd-shutdown[1]: Detaching loop devices.
> > > > [  121.126395] systemd-shutdown[1]: All loop devices detached.
> > > > [  121.130525] systemd-shutdown[1]: Detaching DM devices.
> > > > [  121.135824] systemd-shutdown[1]: All DM devices detached.
> > > > [  121.166327] systemd-shutdown[1]: /lib/systemd/system-shutdown 
> > > > succeeded.
> > > > [  121.171739] systemd-shutdown[1]: Powering off.
> > > > [  121.182331] rebo�
> > > > 
> > > > 
> > > > 
> > > > Best regards
> > > > Michael Niewöhner
> > > 
> > > I did some more tests with next-20161016. Reverting / commenting out
> > > one part of your patch "solves" the lsusb hang, the reboot problem
> > > and also the "debounce failed" message. [1]
> > > Another "solution" is to call phy_power_off before phy_power_on. [2]
> > > 
> > > Disclaimer: I have no idea what I was doing ;-) These were just some
> > > simple trial-and-error attempts that maybe help to find the real
> > > cause of the problems.
> > > 
> > > [1]
> > > diff --git a/drivers/usb/dwc3/core.c 

Re: [alsa-devel] Jack sensing in snd_usb_audio ?

[Takashi Iwai]

On Wed, 12 Oct 2016 18:15:04 +0200,
Bastien Nocera wrote:
> 
> On Wed, 2016-10-12 at 18:06 +0200, Clemens Ladisch wrote:
> > Bastien Nocera wrote:
> > > On Wed, 2016-10-12 at 14:43 +0200, Clemens Ladisch wrote:
> > > > Bastien Nocera wrote:
> > > > > "
> > > > > A change of state in the audio function is most often caused by
> > > > > a
> > > > > certain event that takes place. An event can either be user-
> > > > > initiated
> > > > > or device-initiated. User-initiated jack insertion or removal
> > > > > is a
> > > > > typical example of a user-initiated event.
> > > > > "
> > > > 
> > > > 
> > > > There are not many USB Audio 2.0 devices, and I'm not aware of
> > > > any
> > > > that actually implements this.
> > > 
> > > 
> > > I guess I would see whether there are events if I captured the USB
> > > traffic even without special handling/turning on a feature in the
> > > drivers, right?
> > 
> > 
> > Most devices do not even have the status endpoint (see "lsusb -v").
> > To check what events arrive, you can add logging to the
> > snd_usb_mixer_interrupt() function.
> 
> I'm guessing it doesn't support it then (see attached log)

So this looks like a HID, not from the audio device class.
It's an oft-seen implementation.

> I also checked the input device output when plugging in something, with
> evtest, and no feedback either.

Then at first you need to hack a HID driver to support this device.
It'll create an input device, and then we'll need to find some way to
couple the given input device and the audio device.  We can parse the
sysfs device path to figure out, but I'm not sure what's the best way
to tell it to applications.


Takashi

> Bus 003 Device 035: ID 1b3f:2008 Generalplus Technology Inc. 
> Device Descriptor:
>   bLength18
>   bDescriptorType 1
>   bcdUSB   1.10
>   bDeviceClass0 
>   bDeviceSubClass 0 
>   bDeviceProtocol 0 
>   bMaxPacketSize0 8
>   idVendor   0x1b3f Generalplus Technology Inc.
>   idProduct  0x2008 
>   bcdDevice1.00
>   iManufacturer   1 GeneralPlus
>   iProduct2 USB Audio Device
>   iSerial 0 
>   bNumConfigurations  1
>   Configuration Descriptor:
> bLength 9
> bDescriptorType 2
> wTotalLength  253
> bNumInterfaces  4
> bConfigurationValue 1
> iConfiguration  0 
> bmAttributes 0x80
>   (Bus Powered)
> MaxPower  100mA
> Interface Descriptor:
>   bLength 9
>   bDescriptorType 4
>   bInterfaceNumber0
>   bAlternateSetting   0
>   bNumEndpoints   0
>   bInterfaceClass 1 Audio
>   bInterfaceSubClass  1 Control Device
>   bInterfaceProtocol  0 
>   iInterface  0 
>   AudioControl Interface Descriptor:
> bLength10
> bDescriptorType36
> bDescriptorSubtype  1 (HEADER)
> bcdADC   1.00
> wTotalLength  100
> bInCollection   2
> baInterfaceNr( 0)   1
> baInterfaceNr( 1)   2
>   AudioControl Interface Descriptor:
> bLength12
> bDescriptorType36
> bDescriptorSubtype  2 (INPUT_TERMINAL)
> bTerminalID 1
> wTerminalType  0x0101 USB Streaming
> bAssocTerminal  0
> bNrChannels 2
> wChannelConfig 0x0003
>   Left Front (L)
>   Right Front (R)
> iChannelNames   0 
> iTerminal   0 
>   AudioControl Interface Descriptor:
> bLength12
> bDescriptorType36
> bDescriptorSubtype  2 (INPUT_TERMINAL)
> bTerminalID 4
> wTerminalType  0x0201 Microphone
> bAssocTerminal  0
> bNrChannels 1
> wChannelConfig 0x0001
>   Left Front (L)
> iChannelNames   0 
> iTerminal   0 
>   AudioControl Interface Descriptor:
> bLength 9
> bDescriptorType36
> bDescriptorSubtype  3 (OUTPUT_TERMINAL)
> bTerminalID 3
> wTerminalType  0x0301 Speaker
> bAssocTerminal  0
> bSourceID   6
> iTerminal   0 
>   AudioControl Interface Descriptor:
> bLength 9
> bDescriptorType36
> bDescriptorSubtype  3 (OUTPUT_TERMINAL)
> bTerminalID 2
> wTerminalType  0x0101 USB Streaming
> bAssocTerminal  0
> bSourceID   9
> iTerminal   0 
>   AudioControl Interface Descriptor:
> bLength 7
>  

Re: [PATCH 2/4] USB: ch341: reinitialize chip on reconfiguration

[Johan Hovold]

On Sun, Oct 16, 2016 at 05:09:24PM +0100, Aidan Thornton wrote:
> On Mon, Oct 10, 2016 at 8:56 PM, Grigori Goronzy  wrote:
> > On 2016-10-08 23:07, Aidan Thornton wrote:
> >>
> >> On Fri, Oct 7, 2016 at 12:30 PM, Johan Hovold  wrote:
> >>>
> >>> Why is this change needed? I see no write to this register in the
> >>> vendor driver.
> >>
> >>
> >> In principle, it might not be because the value written to register
> >> 0x18 is probably overwritten by ch341_init_set_baudrate anyway. It's
> >> there because it's in Grigori's original patch, it looks superficially
> >> reasonable (corresponds to ENABLE_RX|ENABLE_TX|CS8, as opposed to the
> >> rather implausible ENABLE_TX|PAR_EVEN|CS5), and given that some
> >> hardware revisions are apparently a little temperamental I was
> >> reluctant to remove it without fully understanding why it was added in
> >> the first place. As the vendor driver does without it might make sense
> >> to delete the write altogether, but it does do quite a few things
> >> differently from this driver. Depends what Grigori says and the
> >> results of actual testing.

Ok, thanks for the explanation. I think we should try to get rid of
anything apparently redundant eventually, moving towards how the vendor
driver does things. It would also be good to replace more of the magic
constants in there to make the code more self-describing.

As long this is done in steps that can be (more easily) rolled-back in
case it turns out we do actually break support for some device in the
process.

> > Yes, I think I introduced it for consistency. The whole init sequence is a
> > bit strange and doesn't make too much sense, but I tried to avoid modifying
> > it because I don't know how the different hardware revisions react to that.
> > In theory it shouldn't make a difference as the chip is reinitialized later
> > on, though.

Yeah, that init sequence looks a bit weird indeed.

But you say you introduced it for consistency; with what? Given that the
vendor driver lacks this write and all devices supposedly have been
working with the current sequence, I mean.

> > By the way, I tested the series against my CH341A board and it works just
> > fine, as expected.

Great, thanks.

> Ah, OK. Thanks for the help and sorry for the late reply, this got
> buried under a pile of other e-mails in my inbox and it took me a
> while to see it. Thoughts on omitting the write to 0x2518 altogether
> then? Doing so doesn't seem to cause any problems on the CH340G I'm
> testing on, but obviously I can't guarantee this will always be the
> case.

I suggest either keeping what's currently there or dropping it entirely
as part of this change, and then possibly clean up the rest of the init
sequence in a separate patch.

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

Re: [PATCH] dt-bindings: usb: atmel: fix a couple of copy-paste style typos

[Nicolas Ferre]

Le 18/10/2016 à 13:05, Peter Rosin a écrit :
> Signed-off-by: Peter Rosin 

Acked-by: Nicolas Ferre 

We may take it in a future at91-x.y-dt branch which will go through arm-soc.

Thanks

> ---
>  Documentation/devicetree/bindings/usb/atmel-usb.txt | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/atmel-usb.txt 
> b/Documentation/devicetree/bindings/usb/atmel-usb.txt
> index f4262ed60582..ad8ea56a9ed3 100644
> --- a/Documentation/devicetree/bindings/usb/atmel-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/atmel-usb.txt
> @@ -6,9 +6,9 @@ Required properties:
>   - compatible: Should be "atmel,at91rm9200-ohci" for USB controllers
> used in host mode.
>   - reg: Address and length of the register set for the device
> - - interrupts: Should contain ehci interrupt
> + - interrupts: Should contain ohci interrupt
>   - clocks: Should reference the peripheral, host and system clocks
> - - clock-names: Should contains two strings
> + - clock-names: Should contain three strings
>   "ohci_clk" for the peripheral clock
>   "hclk" for the host clock
>   "uhpck" for the system clock
> @@ -35,7 +35,7 @@ Required properties:
>   - reg: Address and length of the register set for the device
>   - interrupts: Should contain ehci interrupt
>   - clocks: Should reference the peripheral and the UTMI clocks
> - - clock-names: Should contains two strings
> + - clock-names: Should contain two strings
>   "ehci_clk" for the peripheral clock
>   "usb_clk" for the UTMI clock
>  
> @@ -58,7 +58,7 @@ Required properties:
>   - reg: Address and length of the register set for the device
>   - interrupts: Should contain macb interrupt
>   - clocks: Should reference the peripheral and the AHB clocks
> - - clock-names: Should contains two strings
> + - clock-names: Should contain two strings
>   "pclk" for the peripheral clock
>   "hclk" for the AHB clock
>  
> @@ -85,7 +85,7 @@ Required properties:
>   - reg: Address and length of the register set for the device
>   - interrupts: Should contain usba interrupt
>   - clocks: Should reference the peripheral and host clocks
> - - clock-names: Should contains two strings
> + - clock-names: Should contain two strings
>   "pclk" for the peripheral clock
>   "hclk" for the host clock
>   - ep childnode: To specify the number of endpoints and their properties.
> 


-- 
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: usb: dwc2: Revert incorrect FIFO changes

[Felipe Balbi]

Hi,

John Youn  writes:
> This series reverts patches that incorrectly removed some FIFO
> programming.
>
> Hi Felipe,
>
> This fixes a regression on 4.9-rc1, could you queue them in your fixes
> tree?

added to testing/fixes. I'll be sending a pull request tomorrow if I
manage to get my tests done by the end of tomorrow ;-)

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v5 00/23] Support qcom's HSIC USB and rewrite USB2 HS support

[Peter Chen]

On Mon, Oct 17, 2016 at 06:56:13PM -0700, Stephen Boyd wrote:
> The state of USB ChipIdea support on Qualcomm's platforms is not great.
> The DT description of these devices requires up to three different nodes
> for what amounts to be the same hardware block, when there should really
> only be one. Furthermore, the "phy" driver that is in mainline (phy-msm-usb.c)
> duplicates the OTG state machine and touches the ci controller wrapper
> registers when it should really be focused on the phy and the ULPI accesses
> needed to get the phy working. There's also a slimmed down phy driver for
> the msm8916 platform, but really the phy hardware is the same as other MSMs,
> so we have two drivers doing pretty much the same thing. This leads to a
> situtaion where we have the chipidea core driver, the "phy" driver, and
> sometimes the ehci-msm.c driver operating the same device all at the same
> time with very little coordination. This just isn't very safe and is
> confusing from a driver perspective when trying to figure out who does what.
> Finally, there isn't any HSIC support on platforms like apq8074 so we
> should add that.
> 
> This patch series updates the ChipIdea driver and the MSM wrapper
> (ci_hdrc_msm.c) to properly handle the PHY and wrapper bits at the right
> times in the right places. To get there, we update the ChipIdea core to
> have support for the ULPI phy bus introduced by Heikki. Along the way
> we fix bugs with the extcon handling for peripheral and OTG mode controllers
> and move the parts of phy-usb-msm.c that are touching the CI controller
> wrapper into the wrapper driver (ci_hdrc_msm.c). Finally we add support
> for the HSIC phy based on the ULPI bus and rewrite the HS phy driver
> (phy-usb-msm.c) as a standard ULPI phy driver.
> 
> Once this series is accepted, we should be able to delete the phy-usb-msm.c,
> phy-qcom-8x16-usb.c, and ehci-msm.c drivers from the tree and use the ULPI
> based phy driver (which also lives in drivers/phy/ instead of 
> drivers/usb/phy/)
> and the chipidea host core instead.
> 
> I've also sent separate patches for other minor pieces to make this
> all work. The full tree can be found here[2], hacks and all to get
> things working. I've tested this on the db410c, apq8074 dragonboard,
> and ifc6410 with configfs gadgets and otg cables.
> 
> Patches based on v4.8-rc1
> 
> Changes from v4:
>  * Picked up Acks from Rob
>  * Updated HS phy init sequence DT property to restrict it to offsets

I remembered that you got all my acks for chipidea patches, right? I did
not check for this series.

Besides, the patch "gpu: Remove depends on RESET_CONTROLLER when not a
provider" [1]  still not be accepted, I need this patch to be merged
first, then apply your chipidea part, otherwise, there is a building
warning.

[1] https://patchwork.kernel.org/patch/9322583/

Peter

> 
> Changes from v3:
>  * Picked up Acks from Peter
>  * Updated extcon consolidation patch per Peter's comments
>  * Folded in simplification from Heikki for ULPI DT matching
> 
> Changes from v2:
>  * Added SoC specific compatibles in phy bindings
>  * Dropped AVVIS patch for OTG statemachine
>  * New patch to consolidate extcon handlers
>  * Picked up Acks from Peter
>  * Rebased onto v4.8-rc1
>  * Reworked ULPI OF code to look at vid == 0 instead of pid == 0
>  * Dropped ULPI bindings for vid and pid overrides
> 
> Changes from v1:
>  * Reworked ULPI device probing to keep using vendor/product ids that
>come from DT if needed and falls back to OF style match when product id
>is 0
>  * PHY init later patch was rejected so that moved to a quirk flag and
>the msm wrapper started managing the phy on/off
>  * Updated clk requirements for HSIC phy in binding doc
>  * Added optional clk in wrapper for "housekeeping" found on older qcom
>platforms
>  * Bug fix to OTGSC polling function
>  * Changed runtime PM patch to set as active instead of get/put
> 
> TODO:
>  * DMA fails on arm64 so we need something like [1] or [3] to make it work.
>  * The db410c needs a driver to toggle the onboard switch to connect
>the usb hub instead of micro port when the usb cable is disconnected.
>I've sent a patch set for this[4], which needs some further
>discussion/development.
>  * apq8064 platforms need a vbus regulator to really use otg and I haven't
>tried out the RPM based regulators yet
>  * The HSIC phy on the apq8074 dragonboard is connected to a usb4604
>device which requires the i2c driver to probe and send an i2c
>sequence before the HSIC controller enumerates or HSIC doesn't work.
>Right now I have a hack to force the controller to probe defer
>once so that usb4604 probes first. This needs a more proper solution
>like having the DT describe a linkage between the controller and
>the usb device so we can enforce probe ordering.
>  * There are problems around the OTG switch still, due to how we handle
>extcon events that I'm working through
> 
> [1] 

Re: usb_ep_{dis,en}able() calling context (was: Re: [PATCH 2/3] usb: dwc3: gadget: wait for End Transfer to complete)

[Felipe Balbi]

Hi,

Baolin Wang  writes:
>> >> 8<
>> >> From f3fa94f3171709f787a30e3c5ce69a668960b66e Mon Sep 17 00:00:00 2001
>> >> From: Felipe Balbi 
>> >> Date: Thu, 13 Oct 2016 14:09:47 +0300
>> >> Subject: [PATCH v2] usb: dwc3: gadget: wait for End Transfer to 
>> >> complete
>> >>
>> >> Instead of just delaying for 100us, we should
>> >> actually wait for End Transfer Command Complete
>> >> interrupt before moving on. Note that this should
>> >> only be done if we're dealing with one of the core
>> >> revisions that actually require the interrupt before
>> >> moving on.
>> >>
>> >> Reported-by: Baolin Wang 
>> >> Signed-off-by: Felipe Balbi 
>> >
>> > From my testing, there are still some problems caused by the nested
>> > lock, at lease I found 2 functions will issue the usb_ep_disable()
>> > function with spinlock:
>>
>> Thanks for testing. Seems like I really need to revisit locking in the
>> entire gadget API. In either case, we need to have a larger discussion
>> about this situation.
>>
>> IMO, usb_ep_disable() and usb_ep_enable() should only be callable from
>> process context, but that has never been a requirement before. Still,
>> it's not far-fetched to assume that a controller (such as dwc3, but
>> probably others) might sleep while cancelling a transfer because they
>> need to wait for an Interrupt.
>>
>> In fact, we know of two controllers that need this: dwc3 and dwc3.1.
>
> It's not clear that this should be the deciding factor.  That is, does
> usb_ep_disable() need to wait until all the endpoint's outstanding
> transfers have been completed before it returns?  I don't think it
> does.

 not all, no. And, frankly, this is really only a problem if we're
 unregistering a gadget while there are still pending transfers.

 The original problem statement is that we unregister a gadget, issue End
 Transfer, but CmdCompletion for the EndTransfer comes way too
 late. Baolin, can you clarify again what happens when the IRQ comes
 late?
>>>
>>> Sure. The problem I met was, when we change the USB function with
>>> configfs frequently, sometimes it will hang the system to crash. The
>>> reason is,  we will start end transfer command when disable the
>>> endpoint, but sometimes the end transfer command complete event comes
>>> after we have freed the gadget irq (since the xHCI will share the same
>>> interrupt number with gadget, thus when free the gadget irq, it will
>>> not shutdown this gadget irq line), it will trigger the interrupt line
>>> all the time.
>>
>> okay, thanks for describing it again. Another option would be to only
>> wait_for_completion() before calling free_irq() is any endpoint has
>> END_TRANSFER_PENDING flag set. Something like below:
>
> Yeah, that is what my original patch did, but you did one better
> patch, I will test it too.

That's true. I'll give you proper credit in the commit log since you
originated the idea.

-- 
balbi


signature.asc
Description: PGP signature

Re: usb_ep_{dis,en}able() calling context (was: Re: [PATCH 2/3] usb: dwc3: gadget: wait for End Transfer to complete)

[Peter Chen]

On Tue, Oct 18, 2016 at 10:19:38AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Alan Stern  writes:
> >> Baolin Wang  writes:
> >> >> Felipe Balbi  writes:
> >> >>> Instead of just delaying for 100us, we should
> >> >>> actually wait for End Transfer Command Complete
> >> >>> interrupt before moving on. Note that this should
> >> >>> only be done if we're dealing with one of the core
> >> >>> revisions that actually require the interrupt before
> >> >>> moving on.
> >> >>>
> >> >>> Reported-by: Baolin Wang 
> >> >>> Signed-off-by: Felipe Balbi 
> >> >>
> >> >> I've updated this one in order to fix the comment we had about delaying
> >> >> 100us. No further changes were made, only the comment. Here it is:
> >> >>
> >> >> 8<
> >> >> From f3fa94f3171709f787a30e3c5ce69a668960b66e Mon Sep 17 00:00:00 2001
> >> >> From: Felipe Balbi 
> >> >> Date: Thu, 13 Oct 2016 14:09:47 +0300
> >> >> Subject: [PATCH v2] usb: dwc3: gadget: wait for End Transfer to complete
> >> >>
> >> >> Instead of just delaying for 100us, we should
> >> >> actually wait for End Transfer Command Complete
> >> >> interrupt before moving on. Note that this should
> >> >> only be done if we're dealing with one of the core
> >> >> revisions that actually require the interrupt before
> >> >> moving on.
> >> >>
> >> >> Reported-by: Baolin Wang 
> >> >> Signed-off-by: Felipe Balbi 
> >> >
> >> > From my testing, there are still some problems caused by the nested
> >> > lock, at lease I found 2 functions will issue the usb_ep_disable()
> >> > function with spinlock:
> >> 
> >> Thanks for testing. Seems like I really need to revisit locking in the
> >> entire gadget API. In either case, we need to have a larger discussion
> >> about this situation.
> >> 
> >> IMO, usb_ep_disable() and usb_ep_enable() should only be callable from
> >> process context, but that has never been a requirement before. Still,
> >> it's not far-fetched to assume that a controller (such as dwc3, but
> >> probably others) might sleep while cancelling a transfer because they
> >> need to wait for an Interrupt.
> >> 
> >> In fact, we know of two controllers that need this: dwc3 and dwc3.1.
> >
> > It's not clear that this should be the deciding factor.  That is, does 
> > usb_ep_disable() need to wait until all the endpoint's outstanding 
> > transfers have been completed before it returns?  I don't think it 
> > does.
> 
> not all, no. And, frankly, this is really only a problem if we're
> unregistering a gadget while there are still pending transfers.
> 
> The original problem statement is that we unregister a gadget, issue End
> Transfer, but CmdCompletion for the EndTransfer comes way too
> late. Baolin, can you clarify again what happens when the IRQ comes
> late?
> 

Per my understanding, if there are pending transfers, the gadget driver
needs to call usb_ep_dequeue to cancel transfers first, then call
usb_ep_disable.

-- 

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

Re: usb_ep_{dis,en}able() calling context (was: Re: [PATCH 2/3] usb: dwc3: gadget: wait for End Transfer to complete)

[Baolin Wang]

On 18 October 2016 at 16:21, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>>> Alan Stern  writes:
> Baolin Wang  writes:
> >> Felipe Balbi  writes:
> >>> Instead of just delaying for 100us, we should
> >>> actually wait for End Transfer Command Complete
> >>> interrupt before moving on. Note that this should
> >>> only be done if we're dealing with one of the core
> >>> revisions that actually require the interrupt before
> >>> moving on.
> >>>
> >>> Reported-by: Baolin Wang 
> >>> Signed-off-by: Felipe Balbi 
> >>
> >> I've updated this one in order to fix the comment we had about delaying
> >> 100us. No further changes were made, only the comment. Here it is:
> >>
> >> 8<
> >> From f3fa94f3171709f787a30e3c5ce69a668960b66e Mon Sep 17 00:00:00 2001
> >> From: Felipe Balbi 
> >> Date: Thu, 13 Oct 2016 14:09:47 +0300
> >> Subject: [PATCH v2] usb: dwc3: gadget: wait for End Transfer to 
> >> complete
> >>
> >> Instead of just delaying for 100us, we should
> >> actually wait for End Transfer Command Complete
> >> interrupt before moving on. Note that this should
> >> only be done if we're dealing with one of the core
> >> revisions that actually require the interrupt before
> >> moving on.
> >>
> >> Reported-by: Baolin Wang 
> >> Signed-off-by: Felipe Balbi 
> >
> > From my testing, there are still some problems caused by the nested
> > lock, at lease I found 2 functions will issue the usb_ep_disable()
> > function with spinlock:
>
> Thanks for testing. Seems like I really need to revisit locking in the
> entire gadget API. In either case, we need to have a larger discussion
> about this situation.
>
> IMO, usb_ep_disable() and usb_ep_enable() should only be callable from
> process context, but that has never been a requirement before. Still,
> it's not far-fetched to assume that a controller (such as dwc3, but
> probably others) might sleep while cancelling a transfer because they
> need to wait for an Interrupt.
>
> In fact, we know of two controllers that need this: dwc3 and dwc3.1.

 It's not clear that this should be the deciding factor.  That is, does
 usb_ep_disable() need to wait until all the endpoint's outstanding
 transfers have been completed before it returns?  I don't think it
 does.
>>>
>>> not all, no. And, frankly, this is really only a problem if we're
>>> unregistering a gadget while there are still pending transfers.
>>>
>>> The original problem statement is that we unregister a gadget, issue End
>>> Transfer, but CmdCompletion for the EndTransfer comes way too
>>> late. Baolin, can you clarify again what happens when the IRQ comes
>>> late?
>>
>> Sure. The problem I met was, when we change the USB function with
>> configfs frequently, sometimes it will hang the system to crash. The
>> reason is,  we will start end transfer command when disable the
>> endpoint, but sometimes the end transfer command complete event comes
>> after we have freed the gadget irq (since the xHCI will share the same
>> interrupt number with gadget, thus when free the gadget irq, it will
>> not shutdown this gadget irq line), it will trigger the interrupt line
>> all the time.
>
> okay, thanks for describing it again. Another option would be to only
> wait_for_completion() before calling free_irq() is any endpoint has
> END_TRANSFER_PENDING flag set. Something like below:

Yeah, that is what my original patch did, but you did one better
patch, I will test it too.

>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 5fc437021ac7..0cb3b78d28b7 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -505,6 +506,7 @@ struct dwc3_event_buffer {
>   * @endpoint: usb endpoint
>   * @pending_list: list of pending requests for this endpoint
>   * @started_list: list of started requests on this endpoint
> + * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete
>   * @lock: spinlock for endpoint request queue traversal
>   * @regs: pointer to first endpoint register
>   * @trb_pool: array of transaction buffers
> @@ -530,6 +532,8 @@ struct dwc3_ep {
> struct list_headpending_list;
> struct list_headstarted_list;
>
> +   wait_queue_head_t   wait_end_transfer;
> +
> spinlock_t  lock;
> void __iomem*regs;
>
> @@ -546,6 +550,7 @@ struct 

Re: usb_ep_{dis,en}able() calling context (was: Re: [PATCH 2/3] usb: dwc3: gadget: wait for End Transfer to complete)

[Felipe Balbi]

Hi,

Baolin Wang  writes:
>> Alan Stern  writes:
 Baolin Wang  writes:
 >> Felipe Balbi  writes:
 >>> Instead of just delaying for 100us, we should
 >>> actually wait for End Transfer Command Complete
 >>> interrupt before moving on. Note that this should
 >>> only be done if we're dealing with one of the core
 >>> revisions that actually require the interrupt before
 >>> moving on.
 >>>
 >>> Reported-by: Baolin Wang 
 >>> Signed-off-by: Felipe Balbi 
 >>
 >> I've updated this one in order to fix the comment we had about delaying
 >> 100us. No further changes were made, only the comment. Here it is:
 >>
 >> 8<
 >> From f3fa94f3171709f787a30e3c5ce69a668960b66e Mon Sep 17 00:00:00 2001
 >> From: Felipe Balbi 
 >> Date: Thu, 13 Oct 2016 14:09:47 +0300
 >> Subject: [PATCH v2] usb: dwc3: gadget: wait for End Transfer to complete
 >>
 >> Instead of just delaying for 100us, we should
 >> actually wait for End Transfer Command Complete
 >> interrupt before moving on. Note that this should
 >> only be done if we're dealing with one of the core
 >> revisions that actually require the interrupt before
 >> moving on.
 >>
 >> Reported-by: Baolin Wang 
 >> Signed-off-by: Felipe Balbi 
 >
 > From my testing, there are still some problems caused by the nested
 > lock, at lease I found 2 functions will issue the usb_ep_disable()
 > function with spinlock:

 Thanks for testing. Seems like I really need to revisit locking in the
 entire gadget API. In either case, we need to have a larger discussion
 about this situation.

 IMO, usb_ep_disable() and usb_ep_enable() should only be callable from
 process context, but that has never been a requirement before. Still,
 it's not far-fetched to assume that a controller (such as dwc3, but
 probably others) might sleep while cancelling a transfer because they
 need to wait for an Interrupt.

 In fact, we know of two controllers that need this: dwc3 and dwc3.1.
>>>
>>> It's not clear that this should be the deciding factor.  That is, does
>>> usb_ep_disable() need to wait until all the endpoint's outstanding
>>> transfers have been completed before it returns?  I don't think it
>>> does.
>>
>> not all, no. And, frankly, this is really only a problem if we're
>> unregistering a gadget while there are still pending transfers.
>>
>> The original problem statement is that we unregister a gadget, issue End
>> Transfer, but CmdCompletion for the EndTransfer comes way too
>> late. Baolin, can you clarify again what happens when the IRQ comes
>> late?
>
> Sure. The problem I met was, when we change the USB function with
> configfs frequently, sometimes it will hang the system to crash. The
> reason is,  we will start end transfer command when disable the
> endpoint, but sometimes the end transfer command complete event comes
> after we have freed the gadget irq (since the xHCI will share the same
> interrupt number with gadget, thus when free the gadget irq, it will
> not shutdown this gadget irq line), it will trigger the interrupt line
> all the time.

okay, thanks for describing it again. Another option would be to only
wait_for_completion() before calling free_irq() is any endpoint has
END_TRANSFER_PENDING flag set. Something like below:

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 5fc437021ac7..0cb3b78d28b7 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -505,6 +506,7 @@ struct dwc3_event_buffer {
  * @endpoint: usb endpoint
  * @pending_list: list of pending requests for this endpoint
  * @started_list: list of started requests on this endpoint
+ * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete
  * @lock: spinlock for endpoint request queue traversal
  * @regs: pointer to first endpoint register
  * @trb_pool: array of transaction buffers
@@ -530,6 +532,8 @@ struct dwc3_ep {
struct list_headpending_list;
struct list_headstarted_list;
 
+   wait_queue_head_t   wait_end_transfer;
+
spinlock_t  lock;
void __iomem*regs;
 
@@ -546,6 +550,7 @@ struct dwc3_ep {
 #define DWC3_EP_BUSY   (1 << 4)
 #define DWC3_EP_PENDING_REQUEST(1 << 5)
 #define DWC3_EP_MISSED_ISOC(1 << 6)
+#define DWC3_EP_END_TRANSFER_PENDING (1 << 7)
 
/* This last one is specific to EP0 */
 #define DWC3_EP0_DIR_IN(1 << 31)
@@ -1050,6 +1055,9 @@ struct 

Re: usb_ep_{dis,en}able() calling context (was: Re: [PATCH 2/3] usb: dwc3: gadget: wait for End Transfer to complete)

[Baolin Wang]

Hi,

On 18 October 2016 at 15:19, Felipe Balbi  wrote:
>
> Hi,
>
> Alan Stern  writes:
>>> Baolin Wang  writes:
>>> >> Felipe Balbi  writes:
>>> >>> Instead of just delaying for 100us, we should
>>> >>> actually wait for End Transfer Command Complete
>>> >>> interrupt before moving on. Note that this should
>>> >>> only be done if we're dealing with one of the core
>>> >>> revisions that actually require the interrupt before
>>> >>> moving on.
>>> >>>
>>> >>> Reported-by: Baolin Wang 
>>> >>> Signed-off-by: Felipe Balbi 
>>> >>
>>> >> I've updated this one in order to fix the comment we had about delaying
>>> >> 100us. No further changes were made, only the comment. Here it is:
>>> >>
>>> >> 8<
>>> >> From f3fa94f3171709f787a30e3c5ce69a668960b66e Mon Sep 17 00:00:00 2001
>>> >> From: Felipe Balbi 
>>> >> Date: Thu, 13 Oct 2016 14:09:47 +0300
>>> >> Subject: [PATCH v2] usb: dwc3: gadget: wait for End Transfer to complete
>>> >>
>>> >> Instead of just delaying for 100us, we should
>>> >> actually wait for End Transfer Command Complete
>>> >> interrupt before moving on. Note that this should
>>> >> only be done if we're dealing with one of the core
>>> >> revisions that actually require the interrupt before
>>> >> moving on.
>>> >>
>>> >> Reported-by: Baolin Wang 
>>> >> Signed-off-by: Felipe Balbi 
>>> >
>>> > From my testing, there are still some problems caused by the nested
>>> > lock, at lease I found 2 functions will issue the usb_ep_disable()
>>> > function with spinlock:
>>>
>>> Thanks for testing. Seems like I really need to revisit locking in the
>>> entire gadget API. In either case, we need to have a larger discussion
>>> about this situation.
>>>
>>> IMO, usb_ep_disable() and usb_ep_enable() should only be callable from
>>> process context, but that has never been a requirement before. Still,
>>> it's not far-fetched to assume that a controller (such as dwc3, but
>>> probably others) might sleep while cancelling a transfer because they
>>> need to wait for an Interrupt.
>>>
>>> In fact, we know of two controllers that need this: dwc3 and dwc3.1.
>>
>> It's not clear that this should be the deciding factor.  That is, does
>> usb_ep_disable() need to wait until all the endpoint's outstanding
>> transfers have been completed before it returns?  I don't think it
>> does.
>
> not all, no. And, frankly, this is really only a problem if we're
> unregistering a gadget while there are still pending transfers.
>
> The original problem statement is that we unregister a gadget, issue End
> Transfer, but CmdCompletion for the EndTransfer comes way too
> late. Baolin, can you clarify again what happens when the IRQ comes
> late?

Sure. The problem I met was, when we change the USB function with
configfs frequently, sometimes it will hang the system to crash. The
reason is,  we will start end transfer command when disable the
endpoint, but sometimes the end transfer command complete event comes
after we have freed the gadget irq (since the xHCI will share the same
interrupt number with gadget, thus when free the gadget irq, it will
not shutdown this gadget irq line), it will trigger the interrupt line
all the time.

>
>>> I wonder if there are any other controllers with this
>>> requirement. Either way, We should also consider if there are any strong
>>> reasons to call usb_ep_disable() with interrupts disabled and locks held
>>> considering that UDC _must_ call ->complete() for each and every
>>> request.
>>>
>>> If we go ahead with this, it'll mean a rather large series (again) to
>>> fix all the calling semantics in every single gadget driver for both
>>> usb_ep_enable() and usb_ep_disable(). Then, making sure all UDC drivers
>>> respect the requirement, then we update documentation about the
>>> functions and add might_sleep() to their implementation in udc/core.c
>>>
>>> Anybody objects to this new requirement?
>>
>> The usual times for calling usb_ep_disable() is when the gadget driver
>> processes an altsetting or configuration change, or a reset or
>> disconnect.  The notifications for these things happen in interrupt
>> context, so it doesn't seem reasonable to require that endpoints be
>> disabled in process context.
>
> Oh, that's true. I completely forgot about altsetting change.
>
>> f_mass_storage has its own worker thread.  Mainly for other reasons,
>> but it can also use the thread to handle these events.  But it doesn't
>> seem right to require all gadget drivers to do the same thing.
>>
>> There is still an open question about how a gadget driver can change
>> altsettings, though.  A particular endpoint might be bulk in one
>> altsetting and isochronous in another, for example. 

Re: usb_ep_{dis,en}able() calling context (was: Re: [PATCH 2/3] usb: dwc3: gadget: wait for End Transfer to complete)

[Felipe Balbi]

Hi,

Alan Stern  writes:
>> Baolin Wang  writes:
>> >> Felipe Balbi  writes:
>> >>> Instead of just delaying for 100us, we should
>> >>> actually wait for End Transfer Command Complete
>> >>> interrupt before moving on. Note that this should
>> >>> only be done if we're dealing with one of the core
>> >>> revisions that actually require the interrupt before
>> >>> moving on.
>> >>>
>> >>> Reported-by: Baolin Wang 
>> >>> Signed-off-by: Felipe Balbi 
>> >>
>> >> I've updated this one in order to fix the comment we had about delaying
>> >> 100us. No further changes were made, only the comment. Here it is:
>> >>
>> >> 8<
>> >> From f3fa94f3171709f787a30e3c5ce69a668960b66e Mon Sep 17 00:00:00 2001
>> >> From: Felipe Balbi 
>> >> Date: Thu, 13 Oct 2016 14:09:47 +0300
>> >> Subject: [PATCH v2] usb: dwc3: gadget: wait for End Transfer to complete
>> >>
>> >> Instead of just delaying for 100us, we should
>> >> actually wait for End Transfer Command Complete
>> >> interrupt before moving on. Note that this should
>> >> only be done if we're dealing with one of the core
>> >> revisions that actually require the interrupt before
>> >> moving on.
>> >>
>> >> Reported-by: Baolin Wang 
>> >> Signed-off-by: Felipe Balbi 
>> >
>> > From my testing, there are still some problems caused by the nested
>> > lock, at lease I found 2 functions will issue the usb_ep_disable()
>> > function with spinlock:
>> 
>> Thanks for testing. Seems like I really need to revisit locking in the
>> entire gadget API. In either case, we need to have a larger discussion
>> about this situation.
>> 
>> IMO, usb_ep_disable() and usb_ep_enable() should only be callable from
>> process context, but that has never been a requirement before. Still,
>> it's not far-fetched to assume that a controller (such as dwc3, but
>> probably others) might sleep while cancelling a transfer because they
>> need to wait for an Interrupt.
>> 
>> In fact, we know of two controllers that need this: dwc3 and dwc3.1.
>
> It's not clear that this should be the deciding factor.  That is, does 
> usb_ep_disable() need to wait until all the endpoint's outstanding 
> transfers have been completed before it returns?  I don't think it 
> does.

not all, no. And, frankly, this is really only a problem if we're
unregistering a gadget while there are still pending transfers.

The original problem statement is that we unregister a gadget, issue End
Transfer, but CmdCompletion for the EndTransfer comes way too
late. Baolin, can you clarify again what happens when the IRQ comes
late?

>> I wonder if there are any other controllers with this
>> requirement. Either way, We should also consider if there are any strong
>> reasons to call usb_ep_disable() with interrupts disabled and locks held
>> considering that UDC _must_ call ->complete() for each and every
>> request.
>> 
>> If we go ahead with this, it'll mean a rather large series (again) to
>> fix all the calling semantics in every single gadget driver for both
>> usb_ep_enable() and usb_ep_disable(). Then, making sure all UDC drivers
>> respect the requirement, then we update documentation about the
>> functions and add might_sleep() to their implementation in udc/core.c
>> 
>> Anybody objects to this new requirement?
>
> The usual times for calling usb_ep_disable() is when the gadget driver 
> processes an altsetting or configuration change, or a reset or 
> disconnect.  The notifications for these things happen in interrupt 
> context, so it doesn't seem reasonable to require that endpoints be 
> disabled in process context.

Oh, that's true. I completely forgot about altsetting change.

> f_mass_storage has its own worker thread.  Mainly for other reasons, 
> but it can also use the thread to handle these events.  But it doesn't 
> seem right to require all gadget drivers to do the same thing.
>
> There is still an open question about how a gadget driver can change 
> altsettings, though.  A particular endpoint might be bulk in one 
> altsetting and isochronous in another, for example.  I don't see how we 
> can change the endpoint's characteristics without being allowed to 
> sleep.

Heh, seems that I ended up touching a subject that hasn't been revisited
in few years :-)

Anyway, let's see what Baolin says about the IRQ again. Now that I think
of it, free_irq() should be calling synchronize IRQ, right? So by the
time free_irq() returns, we shouldn't get any further interrupts. Also,
when the endpoint is disabled it shouldn't trigger any further
interrupts.

-- 
balbi


signature.asc
Description: PGP signature

RE: USB GADGET: have a question about usb2eth

[Felipe Balbi]

Hi,

Lipengcheng  writes:
>> -Original Message-
>> From: Felipe Balbi [mailto:ba...@kernel.org]
>> Sent: Monday, October 17, 2016 5:29 PM
>> To: Lipengcheng; Peter Chen
>> Cc: gre...@linuxfoundation.org; linux-usb@vger.kernel.org; 
>> linux-ker...@vger.kernel.org
>> Subject: RE: USB GADGET: have a question about usb2eth
>> 
>> 
>> Hi,
>> 
>> (please, avoid top-posting: http://daringfireball.net/2007/07/on_top)
>> 
>> Lipengcheng  writes:
>> > Hi,
>> >thank you for your suggestion.
>> >
>> >I have a question about usb2eth. In the function gen_ndis_set_resp of
>> > the rndis.c, the value of *params->filter may be 0x20 from the pc set
>> > command; Howerver the value is used cdc_filter =
>> > dev->port_usb->cdc_filter in the function eth_start_xmit of the u_ether.c.
>> > If we do not judge the RNDIS_PACKET_TYPE_PROMISCUOUS and the value is
>> > 0x20, the broadcast packet can not send the pc win7; At the result,
>> > the linux ping the win7 is slow an the first. At the same time, Why
>> > are different value between RNDIS_PACKET_TYPE_PROMISCUOUS and
>> > USB_CDC_PACKET_TYPE_PROMISCUOUS? If the value of
>> > RNDIS_PACKET_TYPE_PROMISCUOUS
>> 
>> because they are defined by different specifications. You should read both 
>> CDC specification from USB.org and RNDIS spec from Microsoft to
>> understand the details of that.
> Ok. I will understand the different both CDC specification from USB.org and 
> RNDIS spec from Microsoft. They should have transformational rule in the 
> linux code
> between CDC spe and RNDIS spe. Through debugging, I found there has been no 
> conversion about the filter. I feel a little problem. I will find the rule.
>> 
>> > and USB_CDC_PACKET_TYPE_PROMISCUOUS is same, then the linux ping the
>> > win7 is normal speed.
>> 
>> I don't understand what's going on here. Care to describe which kernel 
>> you're using, which USB peripheral controller, what is the actual
>> problem you're trying to solve?
>
> VERSION = 3.18.20

why not v4.9-rc1? We can't help you with your kernel in this forum, I
reckon you realise that :-)

> USB peripheral controller: Synopsys HS OTG Linux Software Driver. It's
> not the standard linux code. But I think the controller do not have
> problem.

so you're not using drivers/usb/dwc2?

> The probem is that I am using one controller board with Linux running
> on it. I want to interface my device to the Host computer (Windows OS)
> through USB.

many have tried and succeeded before you :-)

> I have driver ready at device side (linux) and the bridge has been
> established. The device side ping the Host computer is very slow at
> the first time. It spends a few minutes. The problem is the first ping
> takes too long time. Now I have a way to quickly ping at the first ,
> but I do not know the risk about the change.I will continue debug the
> problem. If it is valuable, I will submit the kernel community.

Well, if you can reproduce the same problem with v4.8 or v4.9-rc1, then
we'll be happy to review your patch.

Thank you

-- 
balbi


signature.asc
Description: PGP signature