Re: [PATCH 2/2] usb: dwc3: gadget: Various fixes to trbs_left calculation

[Felipe Balbi]

Hi,

John Youn  writes:
> On 5/19/2016 12:51 AM, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> John Youn  writes:
>>> This patch fixes up some issues related to the trb_left calculation.
>>>
>>> This calculation sometimes included the link trb slot in the trbs_left
>>> and sometimes didn't.
>> 
>> good catch. But this patch seems like it can be broken into smaller
>> pieces. See below
>
> Ok, I can split it up.

thanks :-)

>>> In the case where the dequeue < enqueue, this number does not include
>>> the link trb and should be used as-is. Otherwise it will include the
>>> link_trb and should be decremented to reflect the actual amount of free
>>> trb slots. The fixed calculation will be the proper amount of free TRB
>>> slots left, taking into account whether a link TRB takes up one of the
>>> slots.
>> 
>> this is one fix :-)
>> 
>>> When checking for full or empty where enqueue == dequeue, the pointer
>>> may be 0. In that case previous TRB is the one before the link TRB. Use
>>> that to check the HWO bit to see if we are full or empty.
>> 
>> if enqueue == dequeue, we could be anywhere in the ring. So previous TRB
>> might not be the one before the link. Care to further explain this case?
>
> The enqueue and dequeue can both be 0 and full and index 0 is treated
> just like any other index in the ring. That has to be the case as
> whenever we increment we don't ever point to a link TRB. We pretend it
> doesn't exist and that it is just a 255-TRB circular queue. How we end
> up full at index 0 is that they are both initially 0, then we enqueue
> 255 TRBs without the hardware getting a chance to process anything
> yet. The enqueue will wrap and point to 0 again, and then it will be
> full.

oh, you're right. I completely missed the case were gadget driver fills
up the ring in one go.

> We check for empty/full in the same way, except that at index 0, when
> we look at the previous TRB, we must wrap around backwards, skip the
> link trb, and look at the TRB prior to the link TRB.

right

>> It's also a separate fix, btw.
>> 
>>> In case DWC3_TRB_NUM is ever set lower than 256, mod trbs_left result by
>>> DWC3_TRB_NUM.
>> 
>> I don't get this. Where did we have % 256? I can only see % DWC3_TRB_NUM.
>
> Right. That was taken care of in the trb_enqueue/deqeuue increment
> functions. But it also needs to be accounted for in the trbs_left
> calculation which was just returning (trb->dequeue - trb->enqueue).
>
> For example if you queue one trb the calculation would
> 0x00 - 0x01 = 0xFF (255), minus 1 for link trb = 254 TRB slots
> free. But if DWC3_TRB_NUM = 8, then I have 254 % 8 = 6 slots free.

okay.

>>> In dwc3_prepare_trbs, if trbs_left is 0, do nothing and return early.
>> 
>> another fix.
>> 
>>> When an EP is enabled, initialized the TRB pool to clean up any stale
>>> data. When the previous TRB was not properly cleaned up on disconnect,
>>> the empty/full checking logic sometimes failed causing the EP to stop
>>> processing requests.
>> 
>> another fix.
>> 
>>> Signed-off-by: John Youn 
>>> ---
>>>  drivers/usb/dwc3/gadget.c | 37 ++---
>>>  1 file changed, 30 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 3eaef22..a76634b 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -561,6 +561,12 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
>>> if (usb_endpoint_xfer_control(desc))
>>> goto out;
>>>  
>>> +   /* Initialize the TRB pool */
>>> +   dep->trb_dequeue = 0;
>>> +   dep->trb_enqueue = 0;
>>> +   memset(dep->trb_pool, 0,
>>> +  sizeof(struct dwc3_trb) * DWC3_TRB_NUM);
>> 
>> this is a separate fix. A very good one, actually ;-)
>> 
>>> /* Link TRB. The HWO bit is never reset */
>>> trb_st_hw = &dep->trb_pool[0];
>>>  
>>> @@ -849,6 +855,8 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
>>>  static u32 dwc3_calc_trbs_left(struct dwc3_ep *dep)
>>>  {
>>> struct dwc3_trb *tmp;
>>> +   u8  tmp_index;
>>> +   u8  trbs_left;
>>>  
>>> /*
>>>  * If enqueue & dequeue are equal than it is either full or empty.
>>> @@ -858,17 +866,29 @@ static u32 dwc3_calc_trbs_left(struct dwc3_ep *dep)
>>>  * more transfers in our ring.
>>>  */
>>> if (dep->trb_enqueue == dep->trb_dequeue) {
>>> -   /* If we're full, enqueue/dequeue are > 0 */
>>> -   if (dep->trb_enqueue) {
>>> -   tmp = &dep->trb_pool[dep->trb_enqueue - 1];
>>> -   if (tmp->ctrl & DWC3_TRB_CTRL_HWO)
>>> -   return 0;
>>> -   }
>>> +   if (!dep->trb_enqueue)
>>> +   /*
>>> +* The TRB just before the zeroth one is the
>>> +* one just before the LINK TRB.
>>> +*/
>>> + 

Re: composite gadget with _real_ USB device

[Felipe Balbi]

Hi,

Shea Ako  writes:
> I’ve been learning about and playing with configfs and functionfs to
> create composite user space USB gadgets. My objective is to create a
> composite USB gadget that incorporates a custom functionfs function of
> my own creation along with some _real_ USB devices connected to my
> linux platform.
>
> Is there an easy existing way to bundle _real_ USB devices into a
> composite gadget like this or am I looking at writing my own user
> space functionfs functions which handle wrapping and forwarding the
> interfaces/endpoints/data of the connected _real_ USB devices?

heh, you're really on your own. Sounds pretty interesting but you're
gonna spend a lot of time with this :p

> I haven’t found any documented existing way to do this, but I thought
> I should ask before I go off an implement it myself as it seems that
> this might be a use case which isn’t entirely off the wall and there
> could already be support for this somewhere that I haven’t yet
> encountered.

I don't think anybody has done anything like this yet. The closest I got
was to attach some USB sticks to host port via HUB, setup RAID and use
that RAID as backing store for g_mass_storage, then connect
g_mass_storage back to same host which has the RAID of several USB
sticks (same machine has host and peripheral controllers).

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core

[Roger Quadros]

Peter,

On 20/05/16 04:39, Peter Chen wrote:
> On Wed, May 18, 2016 at 03:45:11PM +0300, Roger Quadros wrote:
>> On 18/05/16 06:18, Peter Chen wrote:
>>> On Mon, May 16, 2016 at 12:51:53PM +0300, Roger Quadros wrote:
 On 16/05/16 12:23, Peter Chen wrote:
> On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote:
>> Hi,
>>
>> On 16/05/16 10:02, Peter Chen wrote:
>>> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
 +
 +static int usb_gadget_connect_control(struct usb_gadget *gadget, bool 
 connect)
 +{
 +  struct usb_udc *udc;
 +
 +  mutex_lock(&udc_lock);
 +  udc = usb_gadget_to_udc(gadget);
 +  if (!udc) {
 +  dev_err(gadget->dev.parent, "%s: gadget not 
 registered.\n",
 +  __func__);
 +  mutex_unlock(&udc_lock);
 +  return -EINVAL;
 +  }
 +
 +  if (connect) {
 +  if (!gadget->connected)
 +  usb_gadget_connect(udc->gadget);
 +  } else {
 +  if (gadget->connected) {
 +  usb_gadget_disconnect(udc->gadget);
 +  udc->driver->disconnect(udc->gadget);
 +  }
 +  }
 +
 +  mutex_unlock(&udc_lock);
 +
 +  return 0;
 +}
 +
>>>
>>> Since this is called for vbus interrupt, why not using
>>> usb_udc_vbus_handler directly, and call udc->driver->disconnect
>>> at usb_gadget_stop.
>>
>> We can't assume that this is always called for vbus interrupt so
>> I decided not to call usb_udc_vbus_handler.
>>
>> udc->vbus is really pointless for us. We keep vbus states in our
>> state machine and leave udc->vbus as ture always.
>>
>> Why do you want to move udc->driver->disconnect() to stop?
>> If USB controller disconnected from bus then the gadget driver
>> must be notified about the disconnect immediately. The controller
>> may or may not be stopped by the core.
>>
>
> Then, would you give some comments when this API will be used?
> I was assumed it is only used for drd state machine.

 drd_state machine didn't even need this API in the first place :).
 You guys wanted me to separate out start/stop and connect/disconnect for 
 full OTG case.
 Won't full OTG state machine want to use this API? If not what would it 
 use?

>>>
>>> Oh, I meant only drd and fully otg state machine needs it. I am
>>> wondering if we need have a new API to do it. Two questions:
>>
>> OK.
>>>
>>> - Except for vbus interrupt, any chances this API will be used at
>>> current logic?
>>
>> I don't think so. But we can't assume caller behaviour for any API.
>>
>>> - When this API is called but without a coming gadget->stop?
>>>
>> Never for DRD case. But we want to catch wrong users.
>>
> 
> In future, otg_start_gadget will be used for both DRD and fully OTG FSM.
> There is no otg_loc_conn at current DRD FSM, but there is
> otg_loc_conn at current OTG FSM, see below.
> 
> DRD FSM:
>   case OTG_STATE_B_IDLE:
>   drd_set_protocol(fsm, PROTO_UNDEF);
>   otg_drv_vbus(otg, 0);
>   break;
>   case OTG_STATE_B_PERIPHERAL:
>   drd_set_protocol(fsm, PROTO_GADGET);
>   otg_drv_vbus(otg, 0);
>   break;
> 
> OTG FSM:
>   case OTG_STATE_B_IDLE:
>   otg_drv_vbus(otg, 0);
>   otg_chrg_vbus(otg, 0);
>   otg_loc_conn(otg, 0);
>   otg_loc_sof(otg, 0);
>   /*
>* Driver is responsible for starting ADP probing
>* if ADP sensing times out.
>*/
>   otg_start_adp_sns(otg);
>   otg_set_protocol(fsm, PROTO_UNDEF);
>   otg_add_timer(otg, B_SE0_SRP);
>   break;
>   case OTG_STATE_B_PERIPHERAL:
>   otg_chrg_vbus(otg, 0);
>   otg_loc_sof(otg, 0);
>   otg_set_protocol(fsm, PROTO_GADGET);
>   otg_loc_conn(otg, 1);
>   break;
> 
> My original suggestion is to have an API to do pull dp and this API
> will be used at both DRD and OTG FSM, and called at otg_loc_conn.

The API is usb_gadget_connect_control();

> The (de)initialize is the same for both two FSMs, it both includes
> init peripheral mode and pull up dp, and can be done by drd_set_protocol(fsm, 
> PROTO_GADGET)
> otg_loc_conn(otg, 1);
> 
> What do you think?
> 

I think loc_conn is a bit confusing to drd users. Another issue I see is that 
DRD controller drivers will need to explicitly pass .loc_conn ops via the 
otg_fsm_ops.
This is an additional step and totally unnecessary as it can be automatically 
done
via direct DRD -> UDC-core call.

cheers,
-roger

Re: [PATCH 1/1] net: pegasus: remove dead coding

[Petko Manolov]

On 16-05-19 11:35:42, David Miller wrote:
> From: Heinrich Schuchardt 
> Date: Wed, 18 May 2016 02:13:30 +0200
> 
> > (!count || count < 4) is always true.
> > So let's remove the coding which is dead at least since 2005.
> > 
> > Signed-off-by: Heinrich Schuchardt 
> 
> Applied.

David, the patch you applied is broken.  It seems that you didn't follow the 
discussion from the past couple of days.  Please revert it.


Petko
--
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: ffs-test fails with warning (-19) No such device

[Krzysztof Opasiak]

On 05/19/2016 03:43 PM, jan.hu...@k2l.de wrote:
> I'm trying to establish a high performance USB connection from device/gadget 
> to host. On the USB gadget side I'm running on a Atmel ATSAMA5D35-EK. The 
> Linux Kernel version is 4.1 (4.1.0-linux4sam_5.2-00045-g633e08a) and I'm 
> using configfs to set things up.
> 
> I strictly followed the steps according to the presentations from the 
> Embedded Linux Conference 2014 (by Alan Ott and Matt Porter)
> 
> Here is what I did on the USB device side so far:
> 
> # insmod atmel_usba_udc.ko
> # insmod usb_f_fs.ko
> 
> # mkdir -p /home/root/configfs
> # mount -t configfs none /home/root/configfs
> # mkdir -p /home/root/configfs/usb_gadget/g1
> 
> # echo "0x1a0a" > /home/root/configfs/usb_gadget/g1/idVendor
> # echo "0xbadd" > /home/root/configfs/usb_gadget/g1/idProduct
> 
> # mkdir -p /home/root/configfs/usb_gadget/g1/strings/0x409
> # echo "1234" > /home/root/configfs/usb_gadget/g1/strings/0x409/serialnumber
> # echo "manufacturer" > 
> /home/root/configfs/usb_gadget/g1/strings/0x409/manufacturer
> # echo "product" > /home/root/configfs/usb_gadget/g1/strings/0x409/product
> 
> # mkdir -p /home/root/configfs/usb_gadget/g1/configs/c.1
> # mkdir -p /home/root/configfs/usb_gadget/g1/configs/c.1/strings/0x409
> # echo "Config1" > 
> /home/root/configfs/usb_gadget/g1/configs/c.1/strings/0x409/configuration
> 
> # mkdir -p /home/root/configfs/usb_gadget/g1/functions/ffs.usb0
> # ln -s /home/root/configfs/usb_gadget/g1/functions/ffs.usb0 
> /home/root/configfs/usb_gadget/g1/configs/c.1
> 
> # mkdir -p /home/root/ffs
> # mount usb0 /home/root/ffs -t functionfs
> 
> # cd /home/root/ffs
> # ../ffs-test 64 &
> 
> Setting up the device and functionfs works perfectly fine, but when executing 
> the ffs-test I get the following log messages:
> 
> ffs-test: info: ep0: writing descriptors (in v2 format)
> ffs-test: info: ep0: writing strings
> ffs-test: dbg:  ep1: starting
> ffs-test: dbg:  ep2: starting
> ffs-test: info: ep0: starts
> ffs-test: info: ep2: starts
> ffs-test: info: ep1: starts
> ffs-test: warn: ep1: write: (-19) No such device
> ffs-test: warn: ep2: read: (-19) No such device
> ffs-test: info: ep1: ends
> ffs-test: info: ep2: ends
> Event BIND
> Event ENABLE
> 

You shouldn't use epX where X != 0 until you get enable event. You may
refer to ffs-aio examples which have a suitable flag to wait for that event.

In addition I'd recommend you using libusbgx[1] instead of direct
configfs manipulation. There is even an example which sets usb gadget
with two instances of functionfs[2]

Footnotes:
1 - https://github.com/libusbgx/libusbgx
2 - https://github.com/libusbgx/libusbgx/blob/master/examples/gadget-ffs.c

Best regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
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: ZTE 403ZT supported ?

[Oliver Neukum]

On Mon, 2016-05-16 at 02:43 +, YAMAMOTO Hiroyuki wrote:
> Hi USB serial experts,
> 
> I am trying to support ZTE 403ZT modem by linux serial driver.
> 
> Currently, option driver can not be used for the modem, because USB Class 
> Code of the modem does not match.
> Linux driver has 0xff as USB Class Code for the modem but the device actually 
> has 0x00.
> 
> By default, option driver has this device ID with the following configuration:
> { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x1282, 0xff, 0xff, 0xff) },
> 
> I have changed the configuration as follows, and it works fine.
> { USB_DEVICE(ZTE_VENDOR_ID, 0x1282) },
> 
> Is this a genuine bug? Or is there any other way to use the option driver for 
> the modem?

This is a bug. But you should not change the existing entry. It would
break other devices. You need to add an entry matching your device.

Regards
Oliver


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

[PATCH] net: pegasus: remove unused variables and labels

[Arnd Bergmann]

The latest dead-code removal was slightly incomplete and
left a few things behind that we now get compiler warnings
for:

drivers/net/usb/pegasus.c: In function 'read_bulk_callback':
drivers/net/usb/pegasus.c:475:1: error: label 'goon' defined but not used 
[-Werror=unused-label]
drivers/net/usb/pegasus.c:446:8: error: unused variable 'pkt_len' 
[-Werror=unused-variable]
drivers/net/usb/pegasus.c:445:6: error: unused variable 'buf' 
[-Werror=unused-variable]
drivers/net/usb/pegasus.c:443:17: error: unused variable 'count' 
[-Werror=unused-variable]

This removes them as well.

Signed-off-by: Arnd Bergmann 
Fixes: e00be9e4d0ff ("net: pegasus: remove dead coding")
---
 drivers/net/usb/pegasus.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 1903d2e2b62d..df6d90ab7ca7 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -440,10 +440,8 @@ static void read_bulk_callback(struct urb *urb)
 {
pegasus_t *pegasus = urb->context;
struct net_device *net;
-   int rx_status, count = urb->actual_length;
+   int rx_status;
int status = urb->status;
-   u8 *buf = urb->transfer_buffer;
-   __u16 pkt_len;
 
if (!pegasus)
return;
@@ -472,7 +470,6 @@ static void read_bulk_callback(struct urb *urb)
netif_dbg(pegasus, rx_err, net, "RX status %d\n", status);
}
 
-goon:
usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
  usb_rcvbulkpipe(pegasus->usb, 1),
  pegasus->rx_skb->data, PEGASUS_MTU,
-- 
2.7.0

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

Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core

[Roger Quadros]

On 18/05/16 15:59, Roger Quadros wrote:
> Hi Peter,
> 
> On 18/05/16 10:45, Peter Chen wrote:
>>
>>
>> On Mon, May 16, 2016 at 5:00 PM, Roger Quadros > > wrote:
>>
>> On 13/05/16 13:03, Roger Quadros wrote:
>> > It provides APIs for the following tasks
>> >
>> > - Registering an OTG/dual-role capable controller
>> > - Registering Host and Gadget controllers to OTG core
>> > - Providing inputs to and kicking the OTG state machine
>> >
>> > Provide a dual-role device (DRD) state machine.
>> > DRD mode is a reduced functionality OTG mode. In this mode
>> > we don't support SRP, HNP and dynamic role-swap.
>> >
>> > In DRD operation, the controller mode (Host or Peripheral)
>> > is decided based on the ID pin status. Once a cable plug (Type-A
>> > or Type-B) is attached the controller selects the state
>> > and doesn't change till the cable in unplugged and a different
>> > cable type is inserted.
>> >
>> > As we don't need most of the complex OTG states and OTG timers
>> > we implement a lean DRD state machine in usb-otg.c.
>> > The DRD state machine is only interested in 2 hardware inputs
>> > 'id' and 'b_sess_vld'.
>> >
>> > Signed-off-by: Roger Quadros mailto:rog...@ti.com>>
>> > ---
>> >  drivers/usb/common/Makefile  |2 +-
>> >  drivers/usb/common/usb-otg.c | 1042 
>> ++
>> >  drivers/usb/core/Kconfig |4 +-
>> >  include/linux/usb/gadget.h   |2 +
>> >  include/linux/usb/hcd.h  |1 +
>> >  include/linux/usb/otg-fsm.h  |7 +
>> >  include/linux/usb/otg.h  |  154 ++-
>> >  7 files changed, 1206 insertions(+), 6 deletions(-)
>> >  create mode 100644 drivers/usb/common/usb-otg.c
>>
>>
>> This patch causes the following build issues when CONFIG_USB_GADGET=m, 
>> CONFIG_USB=m,
>> CONFIG_USB_COMMON=m and CONFIG_USB_OTG=y
>>
>> ERROR: "usb_otg_register_gadget" [drivers/usb/gadget/udc/udc-core.ko] 
>> undefined!
>> ERROR: "usb_otg_unregister_gadget" [drivers/usb/gadget/udc/udc-core.ko] 
>> undefined!
>> ERROR: "usb_otg_register_hcd" [drivers/usb/core/usbcore.ko] undefined!
>> ERROR: "usb_otg_unregister_hcd" [drivers/usb/core/usbcore.ko] undefined!
>> ERROR: "otg_statemachine" [drivers/usb/chipidea/ci_hdrc.ko] undefined!
>> scripts/Makefile.modpost:91: recipe for target '__modpost' failed
>> make[1]: *** [__modpost] Error 1
>> Makefile:1141: recipe for target 'modules' failed
>> make: *** [modules] Error 2
>> make: *** Waiting for unfinished jobs
>>
>> drivers/built-in.o: In function `drd_set_state':
>> usb-otg.c:(.text+0x2b4242): undefined reference to `usb_otg_state_string'
>> drivers/built-in.o: In function `drd_statemachine':
>> (.text+0x2b4b4c): undefined reference to `usb_otg_state_string'
>> Makefile:937: recipe for target 'vmlinux' failed
>>
>> I'll fix it up with the following diff.
>>
>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>> index dca7856..16a5b55 100644
>> --- a/drivers/usb/Makefile
>> +++ b/drivers/usb/Makefile
>> @@ -59,5 +59,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS)   += renesas_usbhs/
>>  obj-$(CONFIG_USB_GADGET)   += gadget/
>>
>>  obj-$(CONFIG_USB_COMMON)   += common/
>> +obj-$(CONFIG_USB_OTG)  += common/
>>
>>  obj-$(CONFIG_USBIP_CORE)   += usbip/
>> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
>> index 77048aa..17e449e 100644
>> --- a/drivers/usb/common/usb-otg.c
>> +++ b/drivers/usb/common/usb-otg.c
>> @@ -56,6 +56,30 @@ static int usb_otg_hcd_is_primary_hcd(struct usb_hcd 
>> *hcd)
>> return hcd == hcd->primary_hcd;
>>  }
>>
>> +static const char *otg_state_string(enum usb_otg_state state)
>> +{
>> +   static const char *const names[] = {
>> +   [OTG_STATE_A_IDLE] = "a_idle",
>> +   [OTG_STATE_A_WAIT_VRISE] = "a_wait_vrise",
>> +   [OTG_STATE_A_WAIT_BCON] = "a_wait_bcon",
>> +   [OTG_STATE_A_HOST] = "a_host",
>> +   [OTG_STATE_A_SUSPEND] = "a_suspend",
>> +   [OTG_STATE_A_PERIPHERAL] = "a_peripheral",
>> +   [OTG_STATE_A_WAIT_VFALL] = "a_wait_vfall",
>> +   [OTG_STATE_A_VBUS_ERR] = "a_vbus_err",
>> +   [OTG_STATE_B_IDLE] = "b_idle",
>> +   [OTG_STATE_B_SRP_INIT] = "b_srp_init",
>> +   [OTG_STATE_B_PERIPHERAL] = "b_peripheral",
>> +   [OTG_STATE_B_WAIT_ACON] = "b_wait_acon",
>> +   [OTG_STATE_B_HOST] = "b_host",
>> +   };
>> +
>> +   if (state < 0 || state >= ARRAY_SIZE(names))
>> +   return "UNDEFINED";
>> +
>> +   return names[state];
>> +}
>> +
>>
>>
>>
>> From my point, make another

[PATCH v11 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 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):
  gadget: Introduce the usb charger framework
  gadget: Support for the usb charger framework
  gadget: Integrate with the usb gadget supporting for usb charger
  power: wm831x_power: Support USB charger current limit management

 drivers/power/wm831x_power.c  |   69 
 drivers/usb/gadget/Kconfig|7 +
 drivers/usb/gadget/udc/Makefile   |1 +
 drivers/usb/gadget/udc/charger.c  |  807 +
 drivers/usb/gadget/udc/udc-core.c |   11 +
 include/linux/mfd/wm831x/pdata.h  |3 +
 include/linux/usb/charger.h   |  191 +
 include/linux/usb/gadget.h|   11 +
 include/uapi/linux/usb/charger.h  |   31 ++
 9 files changed, 1131 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

-- 
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 v11 3/4] 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 and usb_charger_exit()
function by getting 'struct usb_charger' from 'struct gadget'.

Signed-off-by: Baolin Wang 
---
 drivers/usb/gadget/udc/charger.c |   39 +-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/charger.c b/drivers/usb/gadget/udc/charger.c
index 7be4c76..949396f 100644
--- a/drivers/usb/gadget/udc/charger.c
+++ b/drivers/usb/gadget/udc/charger.c
@@ -568,6 +568,30 @@ 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)
+   uchger_state = USB_CHARGER_REMOVE;
+   else
+   uchger_state = USB_CHARGER_DEFAULT;
+
+   usb_charger_notify_others(uchger, uchger_state);
+   }
+
return 0;
 }
 EXPORT_SYMBOL_GPL(usb_charger_plug_by_gadget);
@@ -724,6 +748,7 @@ int usb_charger_init(struct usb_gadget *ugadget)
 
/* register a notifier on a usb gadget device */
uchger->gadget = ugadget;
+   ugadget->charger = uchger;
uchger->old_gadget_state = USB_STATE_NOTATTACHED;
 
/* register a new usb charger */
@@ -744,7 +769,19 @@ fail:
 
 int usb_charger_exit(struct usb_gadget *ugadget)
 {
-   return 0;
+   struct usb_charger *uchger = ugadget->charger;
+
+   if (WARN(!uchger, "charger can not be NULL"))
+   return -EINVAL;
+
+   if (uchger->extcon_dev)
+   extcon_unregister_notifier(uchger->extcon_dev,
+  EXTCON_USB,
+  &uchger->extcon_nb.nb);
+
+   ida_simple_remove(&usb_charger_ida, uchger->id);
+
+   return usb_charger_unregister(uchger);
 }
 
 static int __init usb_charger_class_init(void)
-- 
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 v11 2/4] 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 
---
 drivers/usb/gadget/udc/udc-core.c |   11 +++
 include/linux/usb/gadget.h|   11 +++
 2 files changed, 22 insertions(+)

diff --git a/drivers/usb/gadget/udc/udc-core.c 
b/drivers/usb/gadget/udc/udc-core.c
index e4e70e1..58207d0 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * struct usb_udc - describes one usb device controller
@@ -230,6 +231,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(&udc->dev.kobj, NULL, "state");
 }
@@ -399,6 +403,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;
 
@@ -419,6 +427,8 @@ int usb_add_gadget_udc_release(struct device *parent, 
struct usb_gadget *gadget,
 
return 0;
 
+err5:
+   device_del(&udc->dev);
 err4:
list_del(&udc->list);
mutex_unlock(&udc_lock);
@@ -527,6 +537,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
flush_work(&gadget->work);
device_unregister(&udc->dev);
+   usb_charger_exit(gadget);
device_unregister(&gadget->dev);
 }
 EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 5d4e151..9735309 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct usb_ep;
 
@@ -639,6 +640,8 @@ struct usb_gadget {
unsignedout_epnum;
unsignedin_epnum;
struct usb_otg_caps *otg_caps;
+   /* negotiate the power with the usb charger */
+   struct usb_charger  *charger;
 
unsignedsg_supported:1;
unsignedis_otg:1;
@@ -855,10 +858,18 @@ static inline int usb_gadget_vbus_connect(struct 
usb_gadget *gadget)
  * 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.
  */
 static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
 {
+   if (gadget->charger)
+   usb_charger_set_cur_limit_by_type(gadget->charger,
+ gadget->charger->type,
+ mA);
+
if (!gadget->ops->vbus_draw)
return -EOPNOTSUPP;
return gadget->ops->vbus_draw(gadget, mA);
-- 
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 v11 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 |   69 ++
 include/linux/mfd/wm831x/pdata.h |3 ++
 2 files changed, 72 insertions(+)

diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
index 7082301..cef1812 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,43 @@ 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 limit, void *data)
+{
+   struct wm831x_power *wm831x_power = container_of(nb,
+struct wm831x_power,
+usb_notify);
+   unsigned int i, best;
+
+   /* 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 +647,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(&pdev->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,
+ &power->usb_notify);
+   if (ret != 0) {
+   dev_err(&pdev->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 +700,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,
+ &wm831x_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 integrate with */
+   const char *usb_gadget;
+
int irq_base;
int gpio_base;
int gpio_defaults[WM831X_GPIO_NUM];
-- 
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.h

[PATCH v11 1/4] 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 
---
 drivers/usb/gadget/Kconfig   |7 +
 drivers/usb/gadget/udc/Makefile  |1 +
 drivers/usb/gadget/udc/charger.c |  770 ++
 include/linux/usb/charger.h  |  191 ++
 include/uapi/linux/usb/charger.h |   31 ++
 5 files changed, 1000 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 af5d922..82a5b3c 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -133,6 +133,13 @@ config U_SERIAL_CONSOLE
help
   It supports the serial gadget can be used as a console.
 
+config USB_CHARGER
+   bool "USB charger support"
+   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 dfee534..0e9564c 100644
--- a/drivers/usb/gadget/udc/Makefile
+++ b/drivers/usb/gadget/udc/Makefile
@@ -2,6 +2,7 @@
 # USB peripheral controller drivers
 #
 obj-$(CONFIG_USB_GADGET)   += udc-core.o
+obj-$(CONFIG_USB_CHARGER)  += charger.o
 obj-$(CONFIG_USB_DUMMY_HCD)+= dummy_hcd.o
 obj-$(CONFIG_USB_NET2272)  += net2272.o
 obj-$(CONFIG_USB_NET2280)  += net2280.o
diff --git a/drivers/usb/gadget/udc/charger.c b/drivers/usb/gadget/udc/charger.c
new file mode 100644
index 000..7be4c76
--- /dev/null
+++ b/drivers/usb/gadget/udc/charger.c
@@ -0,0 +1,770 @@
+/*
+ * charger.c -- USB charger driver
+ *
+ * Copyright (C) 2015 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 
+#include 
+
+#define DEFAULT_SDP_CUR_LIMIT  500
+#define DEFAULT_SDP_CUR_LIMIT_SS   900
+#define DEFAULT_DCP_CUR_LIMIT  1500
+#define DEFAULT_CDP_CUR_LIMIT  1500
+#define DEFAULT_ACA_CUR_LIMIT  1500
+
+static DEFINE_IDA(usb_charger_ida);
+struct class *usb_charger_class;
+static unsigned int usb_charger_get_cur_limit(struct usb_charger *uchger);
+
+static struct usb_charger *dev_to_uchger(struct device *dev)
+{
+   return container_of(dev, struct usb_charger, dev);
+}
+
+/*
+ * charger_current_show() - Show the charger current limit.
+ */
+static ssize_t charger_current_show(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct usb_charger *uchger = dev_to_uchger(dev);
+
+   return sprintf(buf, "%u\n", usb_charger_get_cur_limit(uchger));
+}
+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 = sprintf(buf, "%s\n", "CDP");
+   break;
+   case ACA_TYPE:
+   cnt = sprintf(buf, "%s\n", "ACA");
+   break;
+   default:
+   cnt = sp

Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core

[Roger Quadros]

On 20/05/16 11:31, Roger Quadros wrote:
> On 18/05/16 15:59, Roger Quadros wrote:
>> Hi Peter,
>>
>> On 18/05/16 10:45, Peter Chen wrote:
>>>
>>>
>>> On Mon, May 16, 2016 at 5:00 PM, Roger Quadros >> > wrote:
>>>
>>> On 13/05/16 13:03, Roger Quadros wrote:
>>> > It provides APIs for the following tasks
>>> >
>>> > - Registering an OTG/dual-role capable controller
>>> > - Registering Host and Gadget controllers to OTG core
>>> > - Providing inputs to and kicking the OTG state machine
>>> >
>>> > Provide a dual-role device (DRD) state machine.
>>> > DRD mode is a reduced functionality OTG mode. In this mode
>>> > we don't support SRP, HNP and dynamic role-swap.
>>> >
>>> > In DRD operation, the controller mode (Host or Peripheral)
>>> > is decided based on the ID pin status. Once a cable plug (Type-A
>>> > or Type-B) is attached the controller selects the state
>>> > and doesn't change till the cable in unplugged and a different
>>> > cable type is inserted.
>>> >
>>> > As we don't need most of the complex OTG states and OTG timers
>>> > we implement a lean DRD state machine in usb-otg.c.
>>> > The DRD state machine is only interested in 2 hardware inputs
>>> > 'id' and 'b_sess_vld'.
>>> >
>>> > Signed-off-by: Roger Quadros mailto:rog...@ti.com>>
>>> > ---
>>> >  drivers/usb/common/Makefile  |2 +-
>>> >  drivers/usb/common/usb-otg.c | 1042 
>>> ++
>>> >  drivers/usb/core/Kconfig |4 +-
>>> >  include/linux/usb/gadget.h   |2 +
>>> >  include/linux/usb/hcd.h  |1 +
>>> >  include/linux/usb/otg-fsm.h  |7 +
>>> >  include/linux/usb/otg.h  |  154 ++-
>>> >  7 files changed, 1206 insertions(+), 6 deletions(-)
>>> >  create mode 100644 drivers/usb/common/usb-otg.c
>>>
>>>
>>> This patch causes the following build issues when CONFIG_USB_GADGET=m, 
>>> CONFIG_USB=m,
>>> CONFIG_USB_COMMON=m and CONFIG_USB_OTG=y
>>>
>>> ERROR: "usb_otg_register_gadget" [drivers/usb/gadget/udc/udc-core.ko] 
>>> undefined!
>>> ERROR: "usb_otg_unregister_gadget" [drivers/usb/gadget/udc/udc-core.ko] 
>>> undefined!
>>> ERROR: "usb_otg_register_hcd" [drivers/usb/core/usbcore.ko] undefined!
>>> ERROR: "usb_otg_unregister_hcd" [drivers/usb/core/usbcore.ko] undefined!
>>> ERROR: "otg_statemachine" [drivers/usb/chipidea/ci_hdrc.ko] undefined!
>>> scripts/Makefile.modpost:91: recipe for target '__modpost' failed
>>> make[1]: *** [__modpost] Error 1
>>> Makefile:1141: recipe for target 'modules' failed
>>> make: *** [modules] Error 2
>>> make: *** Waiting for unfinished jobs
>>>
>>> drivers/built-in.o: In function `drd_set_state':
>>> usb-otg.c:(.text+0x2b4242): undefined reference to 
>>> `usb_otg_state_string'
>>> drivers/built-in.o: In function `drd_statemachine':
>>> (.text+0x2b4b4c): undefined reference to `usb_otg_state_string'
>>> Makefile:937: recipe for target 'vmlinux' failed
>>>
>>> I'll fix it up with the following diff.
>>>
>>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>>> index dca7856..16a5b55 100644
>>> --- a/drivers/usb/Makefile
>>> +++ b/drivers/usb/Makefile
>>> @@ -59,5 +59,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS)   += 
>>> renesas_usbhs/
>>>  obj-$(CONFIG_USB_GADGET)   += gadget/
>>>
>>>  obj-$(CONFIG_USB_COMMON)   += common/
>>> +obj-$(CONFIG_USB_OTG)  += common/
>>>
>>>  obj-$(CONFIG_USBIP_CORE)   += usbip/
>>> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
>>> index 77048aa..17e449e 100644
>>> --- a/drivers/usb/common/usb-otg.c
>>> +++ b/drivers/usb/common/usb-otg.c
>>> @@ -56,6 +56,30 @@ static int usb_otg_hcd_is_primary_hcd(struct usb_hcd 
>>> *hcd)
>>> return hcd == hcd->primary_hcd;
>>>  }
>>>
>>> +static const char *otg_state_string(enum usb_otg_state state)
>>> +{
>>> +   static const char *const names[] = {
>>> +   [OTG_STATE_A_IDLE] = "a_idle",
>>> +   [OTG_STATE_A_WAIT_VRISE] = "a_wait_vrise",
>>> +   [OTG_STATE_A_WAIT_BCON] = "a_wait_bcon",
>>> +   [OTG_STATE_A_HOST] = "a_host",
>>> +   [OTG_STATE_A_SUSPEND] = "a_suspend",
>>> +   [OTG_STATE_A_PERIPHERAL] = "a_peripheral",
>>> +   [OTG_STATE_A_WAIT_VFALL] = "a_wait_vfall",
>>> +   [OTG_STATE_A_VBUS_ERR] = "a_vbus_err",
>>> +   [OTG_STATE_B_IDLE] = "b_idle",
>>> +   [OTG_STATE_B_SRP_INIT] = "b_srp_init",
>>> +   [OTG_STATE_B_PERIPHERAL] = "b_peripheral",
>>> +   [OTG_STATE_B_WAIT_ACON] = "b_wait_acon",
>>> +   [OTG_STATE_B_HOST] = "b_host",
>>> +   };
>>> +
>>> +   if (state < 0 || state >= ARRAY_

[PATCH v9 09/14] usb: of: add an API to get OTG device from USB controller node

[Roger Quadros]

The OTG controller and the USB controller can be linked via the
'otg-controller' property in the USB controller's device node.

of_usb_get_otg() can be used to get the OTG controller device
from the USB controller's device node.

Signed-off-by: Roger Quadros 
Acked-by: Peter Chen 
---
v9:
Clearly indicate which properties are for OTG controller and which ones
are for host/device controllers

 Documentation/devicetree/bindings/usb/generic.txt |  7 ++
 drivers/usb/common/common.c   | 27 +++
 include/linux/usb/of.h|  9 
 3 files changed, 43 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/generic.txt 
b/Documentation/devicetree/bindings/usb/generic.txt
index bba8257..0887d6e 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -11,6 +11,8 @@ Optional properties:
"peripheral" and "otg". In case this attribute isn't
passed via DT, USB DRD controllers should default to
OTG.
+
+Optional properties for OTG controllers:
  - otg-rev: tells usb driver the release number of the OTG and EH supplement
with which the device and its descriptors are compliant,
in binary-coded decimal (i.e. 2.0 is 0200H). This
@@ -25,6 +27,11 @@ Optional properties:
  - adp-disable: tells OTG controllers we want to disable OTG ADP, ADP is
optional for OTG device.
 
+Optional properties for host/device controllers:
+ - otg-controller: phandle to otg controller. Host or gadget controllers can
+   contain this property to link it to a particular OTG
+   controller.
+
 This is an attribute to a USB controller such as:
 
 dwc3@4a03 {
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index e3d0161..d7ec471 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -238,6 +238,33 @@ int of_usb_update_otg_caps(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(of_usb_update_otg_caps);
 
+#ifdef CONFIG_USB_OTG
+/**
+ * of_usb_get_otg - get the OTG controller linked to the USB controller
+ * @np: Pointer to the device_node of the USB controller
+ * @otg_caps: Pointer to the target usb_otg_caps to be set
+ *
+ * Returns the OTG controller device or NULL on error.
+ */
+struct device *of_usb_get_otg(struct device_node *np)
+{
+   struct device_node *otg_np;
+   struct platform_device *pdev;
+
+   otg_np = of_parse_phandle(np, "otg-controller", 0);
+   if (!otg_np)
+   return NULL;
+
+   pdev = of_find_device_by_node(otg_np);
+   of_node_put(otg_np);
+   if (!pdev)
+   return NULL;
+
+   return &pdev->dev;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_otg);
+#endif
+
 #endif
 
 MODULE_LICENSE("GPL");
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
index de3237f..499a4e8 100644
--- a/include/linux/usb/of.h
+++ b/include/linux/usb/of.h
@@ -40,6 +40,15 @@ static inline struct device_node *usb_of_get_child_node
 }
 #endif
 
+#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB_OTG)
+struct device *of_usb_get_otg(struct device_node *np);
+#else
+static inline struct device *of_usb_get_otg(struct device_node *np)
+{
+   return NULL;
+}
+#endif
+
 #if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB_SUPPORT)
 enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np);
 #else
-- 
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

[PATCH 1/1] usbip: don't call stub_device_reset() during stub_disconnect()

[Alexander Popov]

stub_disconnect() calls stub_device_reset() during usb_unbind_device() when
usb device is locked. So usb_lock_device_for_reset() in stub_device_reset()
in that case polls for one second and returns -EBUSY anyway.

Remove useless flag USBIP_EH_RESET from SDEV_EVENT_REMOVED.

Signed-off-by: Alexander Popov 
---
 drivers/usb/usbip/usbip_common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/usbip/usbip_common.h b/drivers/usb/usbip/usbip_common.h
index 86b0847..c4b4938 100644
--- a/drivers/usb/usbip/usbip_common.h
+++ b/drivers/usb/usbip/usbip_common.h
@@ -242,7 +242,7 @@ enum usbip_side {
 #define USBIP_EH_RESET (1 << 2)
 #define USBIP_EH_UNUSABLE  (1 << 3)
 
-#define SDEV_EVENT_REMOVED   (USBIP_EH_SHUTDOWN | USBIP_EH_RESET | 
USBIP_EH_BYE)
+#defineSDEV_EVENT_REMOVED  (USBIP_EH_SHUTDOWN | USBIP_EH_BYE)
 #defineSDEV_EVENT_DOWN (USBIP_EH_SHUTDOWN | USBIP_EH_RESET)
 #defineSDEV_EVENT_ERROR_TCP(USBIP_EH_SHUTDOWN | USBIP_EH_RESET)
 #defineSDEV_EVENT_ERROR_SUBMIT (USBIP_EH_SHUTDOWN | USBIP_EH_RESET)
-- 
1.9.1

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

Re: [PATCH] net: pegasus: remove unused variables and labels

[Petko Manolov]

Guys, come on.  This code is not dead.  This code is executed every time an 
ethernet packet is received.  It takes care of various error statistics. More 
importantly, it sends the actual (reported by the adapter) packet length to the 
network layer along with the packet.

This patch removes skb_put() and netif_rx() calls and effectively kills the RX 
path.  Not to mention that the driver was not even compiled before sending the 
patch upstream.

The only sensible, although cosmetic, change would be to replace:

if (!count || count < 4)

with

if (count < 4)

even though GCC takes care and it optimizes away "!count" condition.

Please revert this patch before Linus pulls from the network tree.


Petko



On 16-05-20 10:22:30, Arnd Bergmann wrote:
> The latest dead-code removal was slightly incomplete and
> left a few things behind that we now get compiler warnings
> for:
> 
> drivers/net/usb/pegasus.c: In function 'read_bulk_callback':
> drivers/net/usb/pegasus.c:475:1: error: label 'goon' defined but not used 
> [-Werror=unused-label]
> drivers/net/usb/pegasus.c:446:8: error: unused variable 'pkt_len' 
> [-Werror=unused-variable]
> drivers/net/usb/pegasus.c:445:6: error: unused variable 'buf' 
> [-Werror=unused-variable]
> drivers/net/usb/pegasus.c:443:17: error: unused variable 'count' 
> [-Werror=unused-variable]
> 
> This removes them as well.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: e00be9e4d0ff ("net: pegasus: remove dead coding")
> ---
>  drivers/net/usb/pegasus.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index 1903d2e2b62d..df6d90ab7ca7 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -440,10 +440,8 @@ static void read_bulk_callback(struct urb *urb)
>  {
>   pegasus_t *pegasus = urb->context;
>   struct net_device *net;
> - int rx_status, count = urb->actual_length;
> + int rx_status;
>   int status = urb->status;
> - u8 *buf = urb->transfer_buffer;
> - __u16 pkt_len;
>  
>   if (!pegasus)
>   return;
> @@ -472,7 +470,6 @@ static void read_bulk_callback(struct urb *urb)
>   netif_dbg(pegasus, rx_err, net, "RX status %d\n", status);
>   }
>  
> -goon:
>   usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
> usb_rcvbulkpipe(pegasus->usb, 1),
> pegasus->rx_skb->data, PEGASUS_MTU,
> -- 
> 2.7.0
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v9 10/14] usb: otg: add hcd companion support

[Roger Quadros]

Since some host controller (e.g. EHCI) needs a companion host controller
(e.g. OHCI), this patch adds such a configuration to use it in the OTG
core.

Signed-off-by: Yoshihiro Shimoda 
Signed-off-by: Roger Quadros 
Acked-by: Peter Chen 
---
v9:
- add DT property documentation under OTG controllers.

 Documentation/devicetree/bindings/usb/generic.txt |  3 +++
 drivers/usb/common/usb-otg.c  | 32 ---
 include/linux/usb/otg.h   |  7 -
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/generic.txt 
b/Documentation/devicetree/bindings/usb/generic.txt
index 0887d6e..6013296 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -26,6 +26,9 @@ Optional properties for OTG controllers:
optional for OTG device.
  - adp-disable: tells OTG controllers we want to disable OTG ADP, ADP is
optional for OTG device.
+ - hcd-needs-companion: must be present if otg controller is dealing with
+   EHCI host controller that needs a companion OHCI host
+   controller.
 
 Optional properties for host/device controllers:
  - otg-controller: phandle to otg controller. Host or gadget controllers can
diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
index 64be6df..8a2a0d4 100644
--- a/drivers/usb/common/usb-otg.c
+++ b/drivers/usb/common/usb-otg.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -584,6 +585,10 @@ struct usb_otg *usb_otg_register(struct device *dev,
else
INIT_WORK(&otg->work, usb_drd_work);
 
+   if (of_find_property(dev->of_node, "hcd-needs-companion", NULL) ||
+   config->hcd_needs_companion)/* needs companion ? */
+   otg->flags |= OTG_FLAG_HCD_NEEDS_COMPANION;
+
otg->wq = create_freezable_workqueue("usb_otg");
if (!otg->wq) {
dev_err(dev, "otg: %s: can't create workqueue\n",
@@ -807,15 +812,18 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned 
int irqnum,
/* HCD will be started by OTG fsm when needed */
mutex_lock(&otg->fsm.lock);
if (otg->primary_hcd.hcd) {
-   /* probably a shared HCD ? */
-   if (usb_otg_hcd_is_primary_hcd(hcd)) {
+   /* probably a shared HCD or a companion OHCI HCD ? */
+   if (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
+   usb_otg_hcd_is_primary_hcd(hcd)) {
dev_err(otg_dev, "otg: primary host already 
registered\n");
goto err;
}
 
-   if (hcd->shared_hcd == otg->primary_hcd.hcd) {
+   if (otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION ||
+   (hcd->shared_hcd == otg->primary_hcd.hcd)) {
if (otg->shared_hcd.hcd) {
-   dev_err(otg_dev, "otg: shared host already 
registered\n");
+   dev_err(otg_dev,
+   "otg: shared/companion host already 
registered\n");
goto err;
}
 
@@ -823,10 +831,12 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned 
int irqnum,
otg->shared_hcd.irqnum = irqnum;
otg->shared_hcd.irqflags = irqflags;
otg->shared_hcd.ops = ops;
-   dev_info(otg_dev, "otg: shared host %s registered\n",
+   dev_info(otg_dev,
+"otg: shared/companion host %s registered\n",
 dev_name(hcd->self.controller));
} else {
-   dev_err(otg_dev, "otg: invalid shared host %s\n",
+   dev_err(otg_dev,
+   "otg: invalid shared/companion host %s\n",
dev_name(hcd->self.controller));
goto err;
}
@@ -849,14 +859,17 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned 
int irqnum,
 * we're ready only if we have shared HCD
 * or we don't need shared HCD.
 */
-   if (otg->shared_hcd.hcd || !otg->primary_hcd.hcd->shared_hcd) {
+   if (otg->shared_hcd.hcd ||
+   (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
+!otg->primary_hcd.hcd->shared_hcd)) {
otg->host = hcd_to_bus(hcd);
/* FIXME: set bus->otg_port if this is true OTG port with HNP */
 
/* start FSM */
usb_otg_start_fsm(otg);
} else {
-   dev_dbg(otg_dev, "otg: can't start till shared host 
registers\n");
+   dev_dbg(otg_dev,
+   "otg: can't start till shared/companion host

Re: ZTE 403ZT supported ?

[Lars Melin]

On 2016-05-16 09:43, YAMAMOTO Hiroyuki wrote:

Hi USB serial experts,

I am trying to support ZTE 403ZT modem by linux serial driver.

Currently, option driver can not be used for the modem, because USB Class Code 
of the modem does not match.
Linux driver has 0xff as USB Class Code for the modem but the device actually 
has 0x00.

By default, option driver has this device ID with the following configuration:
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x1282, 0xff, 0xff, 0xff) },



A device class code of 0x00 means that the class code is defined at the 
interface level.
An interface class code of 0x00 on a ZTE device would be the first for 
me, please provide a lsusb -vd 19d2:1282 listing.



rgds
/Lars



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

Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core

[Peter Chen]

On Fri, May 20, 2016 at 12:19:07PM +0300, Roger Quadros wrote:
> On 20/05/16 11:31, Roger Quadros wrote:
> > On 18/05/16 15:59, Roger Quadros wrote:
> >> Hi Peter,
> >>
> >> On 18/05/16 10:45, Peter Chen wrote:
> >>>
> >>>
> >>> On Mon, May 16, 2016 at 5:00 PM, Roger Quadros  >>> > wrote:
> >>>
> >>> On 13/05/16 13:03, Roger Quadros wrote:
> >>> > It provides APIs for the following tasks
> >>> >
> >>> > - Registering an OTG/dual-role capable controller
> >>> > - Registering Host and Gadget controllers to OTG core
> >>> > - Providing inputs to and kicking the OTG state machine
> >>> >
> >>> > Provide a dual-role device (DRD) state machine.
> >>> > DRD mode is a reduced functionality OTG mode. In this mode
> >>> > we don't support SRP, HNP and dynamic role-swap.
> >>> >
> >>> > In DRD operation, the controller mode (Host or Peripheral)
> >>> > is decided based on the ID pin status. Once a cable plug (Type-A
> >>> > or Type-B) is attached the controller selects the state
> >>> > and doesn't change till the cable in unplugged and a different
> >>> > cable type is inserted.
> >>> >
> >>> > As we don't need most of the complex OTG states and OTG timers
> >>> > we implement a lean DRD state machine in usb-otg.c.
> >>> > The DRD state machine is only interested in 2 hardware inputs
> >>> > 'id' and 'b_sess_vld'.
> >>> >
> >>> > Signed-off-by: Roger Quadros mailto:rog...@ti.com>>
> >>> > ---
> >>> >  drivers/usb/common/Makefile  |2 +-
> >>> >  drivers/usb/common/usb-otg.c | 1042 
> >>> ++
> >>> >  drivers/usb/core/Kconfig |4 +-
> >>> >  include/linux/usb/gadget.h   |2 +
> >>> >  include/linux/usb/hcd.h  |1 +
> >>> >  include/linux/usb/otg-fsm.h  |7 +
> >>> >  include/linux/usb/otg.h  |  154 ++-
> >>> >  7 files changed, 1206 insertions(+), 6 deletions(-)
> >>> >  create mode 100644 drivers/usb/common/usb-otg.c
> >>>
> >>>
> >>> This patch causes the following build issues when 
> >>> CONFIG_USB_GADGET=m, CONFIG_USB=m,
> >>> CONFIG_USB_COMMON=m and CONFIG_USB_OTG=y
> >>>
> >>> ERROR: "usb_otg_register_gadget" [drivers/usb/gadget/udc/udc-core.ko] 
> >>> undefined!
> >>> ERROR: "usb_otg_unregister_gadget" 
> >>> [drivers/usb/gadget/udc/udc-core.ko] undefined!
> >>> ERROR: "usb_otg_register_hcd" [drivers/usb/core/usbcore.ko] undefined!
> >>> ERROR: "usb_otg_unregister_hcd" [drivers/usb/core/usbcore.ko] 
> >>> undefined!
> >>> ERROR: "otg_statemachine" [drivers/usb/chipidea/ci_hdrc.ko] undefined!
> >>> scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> >>> make[1]: *** [__modpost] Error 1
> >>> Makefile:1141: recipe for target 'modules' failed
> >>> make: *** [modules] Error 2
> >>> make: *** Waiting for unfinished jobs
> >>>
> >>> drivers/built-in.o: In function `drd_set_state':
> >>> usb-otg.c:(.text+0x2b4242): undefined reference to 
> >>> `usb_otg_state_string'
> >>> drivers/built-in.o: In function `drd_statemachine':
> >>> (.text+0x2b4b4c): undefined reference to `usb_otg_state_string'
> >>> Makefile:937: recipe for target 'vmlinux' failed
> >>>
> >>> I'll fix it up with the following diff.
> >>>
> >>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> >>> index dca7856..16a5b55 100644
> >>> --- a/drivers/usb/Makefile
> >>> +++ b/drivers/usb/Makefile
> >>> @@ -59,5 +59,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS)   += 
> >>> renesas_usbhs/
> >>>  obj-$(CONFIG_USB_GADGET)   += gadget/
> >>>
> >>>  obj-$(CONFIG_USB_COMMON)   += common/
> >>> +obj-$(CONFIG_USB_OTG)  += common/
> >>>
> >>>  obj-$(CONFIG_USBIP_CORE)   += usbip/
> >>> diff --git a/drivers/usb/common/usb-otg.c 
> >>> b/drivers/usb/common/usb-otg.c
> >>> index 77048aa..17e449e 100644
> >>> --- a/drivers/usb/common/usb-otg.c
> >>> +++ b/drivers/usb/common/usb-otg.c
> >>> @@ -56,6 +56,30 @@ static int usb_otg_hcd_is_primary_hcd(struct 
> >>> usb_hcd *hcd)
> >>> return hcd == hcd->primary_hcd;
> >>>  }
> >>>
> >>> +static const char *otg_state_string(enum usb_otg_state state)
> >>> +{
> >>> +   static const char *const names[] = {
> >>> +   [OTG_STATE_A_IDLE] = "a_idle",
> >>> +   [OTG_STATE_A_WAIT_VRISE] = "a_wait_vrise",
> >>> +   [OTG_STATE_A_WAIT_BCON] = "a_wait_bcon",
> >>> +   [OTG_STATE_A_HOST] = "a_host",
> >>> +   [OTG_STATE_A_SUSPEND] = "a_suspend",
> >>> +   [OTG_STATE_A_PERIPHERAL] = "a_peripheral",
> >>> +   [OTG_STATE_A_WAIT_VFALL] = "a_wait_vfall",
> >>> +   [OTG_STATE_A_VBUS_ERR] = "a_vbus_err",
> >>> +   [OTG_STATE_B_IDLE] = "b_idle",
> >>> +   [OTG_STATE_B_S

Re: [PATCH] net: pegasus: remove unused variables and labels

[Arnd Bergmann]

On Friday 20 May 2016 12:32:23 Petko Manolov wrote:
> Guys, come on.  This code is not dead.  This code is executed every time an 
> ethernet packet is received.  It takes care of various error statistics. More 
> importantly, it sends the actual (reported by the adapter) packet length to 
> the 
> network layer along with the packet.
> 
> This patch removes skb_put() and netif_rx() calls and effectively kills the 
> RX 
> path.  Not to mention that the driver was not even compiled before sending 
> the 
> patch upstream.
> 
> The only sensible, although cosmetic, change would be to replace:
> 
> if (!count || count < 4)
> 
> with
> 
> if (count < 4)
> 
> even though GCC takes care and it optimizes away "!count" condition.
> 
> Please revert this patch before Linus pulls from the network tree.
> 

Agreed. I failed to check the commit that introduced the warning for
the more serious problem.

Please revert e00be9e4d0ff, it just makes no sense.

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

[PATCH v2 1/1] usb: gadget: f_fs: Fix wrong check on reserved1 of OS_DESC_EXT_COMPAT

[Jim Lin]

Current __ffs_data_do_os_desc() of f_fs.c will check reserved1 field
of OS_DESC_EXT_COMPAT and return -EINVAL if it's 1.
But MS OS 1.0 Descriptors
http://msdn.microsoft.com/en-us/library/windows/hardware/gg463179.aspx
defines that field to be 1.

Signed-off-by: Jim Lin 
---
v2: Rewording on commit log. Original title is
 usb: gadget: f_fs: Fix kernel panic for SuperSpeed

 drivers/usb/gadget/function/f_fs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 15b648c..bd969ef 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -2051,7 +2051,7 @@ static int __ffs_data_do_os_desc(enum ffs_os_desc_type 
type,
 
if (len < sizeof(*d) ||
d->bFirstInterfaceNumber >= ffs->interfaces_count ||
-   d->Reserved1)
+   !d->Reserved1)
return -EINVAL;
for (i = 0; i < ARRAY_SIZE(d->Reserved2); ++i)
if (d->Reserved2[i])
-- 
1.9.1

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

Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Heikki Krogerus]

On Thu, May 19, 2016 at 10:53:04AM -0700, Guenter Roeck wrote:
> Hello Heikki,
> 
> On Thu, May 19, 2016 at 03:44:54PM +0300, Heikki Krogerus wrote:
> > The purpose of this class is to provide unified interface for user
> > space to get the status and basic information about USB Type-C
> > Connectors in the system, control data role swapping, and when USB PD
> > is available, also power role swapping and Alternate Modes.
> > 
> > Signed-off-by: Heikki Krogerus 
> > ---
> >  drivers/usb/Kconfig |   2 +
> >  drivers/usb/Makefile|   2 +
> >  drivers/usb/type-c/Kconfig  |   7 +
> >  drivers/usb/type-c/Makefile |   1 +
> >  drivers/usb/type-c/typec.c  | 957 
> > 
> >  include/linux/usb/typec.h   | 230 +++
> >  6 files changed, 1199 insertions(+)
> >  create mode 100644 drivers/usb/type-c/Kconfig
> >  create mode 100644 drivers/usb/type-c/Makefile
> >  create mode 100644 drivers/usb/type-c/typec.c
> >  create mode 100644 include/linux/usb/typec.h
> > 
> > Hi,
> > 
> > Like I've told some of you guys, I'm trying to implement a bus for
> > the Alternate Modes, but I'm still nowhere near finished with that
> > one, so let's just get the class ready now. The altmode bus should in
> > any case not affect the userspace interface proposed in this patch.
> > 
> > As you can see, the Alternate Modes are handled completely differently
> > compared to the original proposal. Every Alternate Mode will have
> > their own device instance (which will be then later bound to an
> > Alternate Mode specific driver once we have the bus), but also every
> > partner, cable and cable plug will have their own device instances
> > representing them.
> > 
> > An other change is that the data role is now handled in two ways.
> > The current_data_role file will represent static mode of the port, and
> > it will use the names for the roles as they are defined in the spec:
> > DFP, UFP and DRP. This file should be used if the port needs to be
> > fixed to one specific role with DRP ports. So this approach will
> > replace the suggestions for "preferred" data role we had. The
> > current_usb_data_role will use values "host" and "device" and it will
> > be used for data role swapping when already connected.
> > 
> 
> What I am missing completely is a means to handle role and alternate mode
> changes triggered by the partner. The need for those should be obvious,
> unless I am really missing something (just consider two devices supporting
> this code connected to each other).

We are missing the notifications that are needed in these cases. But I
don't see much more we can do about those cases. We can not put any
policies in place at this level, because we have to be able to support
also things like USB PD and Type-C controllers that take care of all
that, leaving us to not be able to do anything else but to pass the
information forward. So the framework at this level has to be
"stupid", and if more infrastructure is needed, it has to be
introduced in an other layer.

> Also, I am not sure where the policy engine is supposed to reside.
> I understand that some policy changes (eg unsolicited requests to switch 
> roles)
> can be triggered from user space. However, role change requests triggered from
> the partner need to be evaluated quickly (typically within 15 ms), so user
> space can not get involved. Maybe it would help to have some text describing
> where the policy engine is expected to reside and how it is involved
> in the decision making process. This includes the initial decision making
> process, when it needs to be decided if role changes should be requested
> or if one or multiple alternate modes should be entered after the initial
> connection has been established.

Well, yes we need to document these things, but you are now coupling
this framework with USB PD and we really should not do that.

The policy engine, and the whole USB PD stack, belongs inside the
kernel, and it will be completely separated from this framework. This
framework can not have any dependencies on the future USB PD stack.
This is not only because of the USB PD/Type-C controllers which handle
the policy engine on their own and only allow "unsolicited" requests
like "swap role" and "enter/exit mode", but also because this
framework must work smoothly on systems that don't want to use USB PD
and of course also with USB Type-C PHYs that simply don't include USB
PD transceiver.

The layer that joins these two parts together will be the port drivers
themselves, so the USB Type-C/PD PHYs and controllers, at least in the
beginning.

Any initial decisions about which role or which alternate mode to
select belongs to the stack. The userspace will need to be notified,
and the userspace can then attempt to request changes after that, but
if there is something that blocks the requests, the attempt has to
just fail. So we can't provide any knowledge for the userspace about
the requirements regarding the hig

Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Heikki Krogerus]

On Thu, May 19, 2016 at 08:43:27AM -0700, Greg KH wrote:
> On Thu, May 19, 2016 at 04:48:46PM +0200, Oliver Neukum wrote:
> > On Thu, 2016-05-19 at 15:44 +0300, Heikki Krogerus wrote:
> > > + ret = typec_register_altmodes(dev, partner->alt_modes);
> > > + if (ret) {
> > > + device_unregister(dev);
> > > + return ret;
> > > + }
> > > +
> > > + /* REVISIT: Creating symlink for the port device for now. */
> > > + ret = sysfs_create_link(&port->dev.kobj, &dev->kobj, "partner");
> > > + if (ret)
> > > + dev_WARN(&port->dev, "failed to create link to %s (%d)\n",
> > > +  dev_name(dev), ret);
> > 
> > The attributes should be present as soon as the device is announced.
> 
> Yes, this is wrong and racy (hint, if you have to drop down to a sysfs
> call within a driver, almost always something is wrong...)

I'll remove those links.

I'm hoping to get comments about the attributes in general. Do we
have everything needed to describe the state and capabilities of USB
Type-C ports with them, and are they in reasonable places. I'm playing
with these links in couple of places in the code, and I've put
questions in the comments on some of them. I'm really not planning on
leaving them as they are.

So if you guys could provide your opinions on those questions, I would
much appreciate.


Thanks a lot,

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

Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Heikki Krogerus]

On Thu, May 19, 2016 at 04:47:17PM +0200, Oliver Neukum wrote:
> On Thu, 2016-05-19 at 15:44 +0300, Heikki Krogerus wrote:
> > The purpose of this class is to provide unified interface for user
> > space to get the status and basic information about USB Type-C
> > Connectors in the system, control data role swapping, and when USB PD
> > is available, also power role swapping and Alternate Modes.
> > 
> > Signed-off-by: Heikki Krogerus 
> > ---
> >  drivers/usb/Kconfig |   2 +
> >  drivers/usb/Makefile|   2 +
> >  drivers/usb/type-c/Kconfig  |   7 +
> >  drivers/usb/type-c/Makefile |   1 +
> >  drivers/usb/type-c/typec.c  | 957 
> > 
> >  include/linux/usb/typec.h   | 230 +++
> >  6 files changed, 1199 insertions(+)
> >  create mode 100644 drivers/usb/type-c/Kconfig
> >  create mode 100644 drivers/usb/type-c/Makefile
> >  create mode 100644 drivers/usb/type-c/typec.c
> >  create mode 100644 include/linux/usb/typec.h
> > 
> > Hi,
> > 
> > Like I've told some of you guys, I'm trying to implement a bus for
> > the Alternate Modes, but I'm still nowhere near finished with that
> > one, so let's just get the class ready now. The altmode bus should in
> > any case not affect the userspace interface proposed in this patch.
> > 
> > As you can see, the Alternate Modes are handled completely differently
> > compared to the original proposal. Every Alternate Mode will have
> > their own device instance (which will be then later bound to an
> > Alternate Mode specific driver once we have the bus), but also every
> > partner, cable and cable plug will have their own device instances
> > representing them.
> 
> That raises a question. If I read the standard correctly, alternate
> modes could be combinable. So how do you represent that.
> 
> > An other change is that the data role is now handled in two ways.
> > The current_data_role file will represent static mode of the port, and
> > it will use the names for the roles as they are defined in the spec:
> > DFP, UFP and DRP. This file should be used if the port needs to be
> > fixed to one specific role with DRP ports. So this approach will
> > replace the suggestions for "preferred" data role we had. The
> > current_usb_data_role will use values "host" and "device" and it will
> > be used for data role swapping when already connected.
> 
> Please explain. How does that express DRP but prefered master?

Sorry but I'm not sure what you mean here. If the port is capable of
being used as dual role port (DRP in the supported_data_roles file),
that is the only case where you can select the role with this file. So
I would imagine that in your case you want to make the port act as
DFP only, right? But if the port is capable of acting only as UFP, you
are stuck with that role.

> > I Hope I remembered to CC everybody interested.
> 
> Alternate modes can be left involuntarily. So we need a method of
> notification.

Yes, that is missing indeed.


Thanks,

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

Re: USB 3.1 Gen 2 - 4.6 mainline broken

[Mathias Nyman]

Hi

I'm adding the mailing list to this reply
(removed your contact info)

USB 3.1 Gen 2 in 4.6 is not broken just because device is not seen as
SuperSpeedPlus 10Gbps device.

On 20.05.2016 02:04, Everett Wenzel wrote:

Hello,

I noticed that an FPGA was used to develop most of the USB3.1 Gen2 code.
This appears to be broken with actual devices. I have 2 startech docks,
and 2 startech hdd enclosures. I am also testing with an akitio gen2 ssd
tray.

When testing speeds with 3.0 and 3.1 ports I do notice a difference in
speeds. But dmesg and lsusb -t will never enumerate a device above
Gen1(5000m cap). It always states "new SuperSpeed device number *" in
dmesg.  I believe the code itself is broken so I was reaching out to you
for some advice, as this subject is still new and unexplored in depth.

Motherboard: https://www.asus.com/us/Motherboards/Z97AUSB_31/
Atikio: https://www.akitio.com/adapters/neutrino-bridge
Startech:
https://www.startech.com/HDD/Docking/2-bay-usb-3-1-gen-2-sata-dock~SDOCK2U
313



All parts in the chain need to support usb3.1 Gen2 capability before the speed 
of a
device is seen as SuperSpeedPlus.

1. First the xhci host controller SBRN register (Serial Bus Release Number) must
  be 3.1 (0x31). If this is true you will see this info message during boot:
  "Host supports USB 3.1 Enhanced SuperSpeed"

2. The supported protocols for ports listed in xhci extended capabilities need
   to have major and minor revisions set to 3 and 1. (3.1)
   If the xhci host has custom speed mappings (PSIC is set) then they must 
include
   the necessary settings for 3.1 10Gbps support (LP = 1 etc). If not then 
default
   speed mappings are set. Default mappings includ SSP 10Gbps support.
   The major and minor revisions can be seen with lsusb -v,
   (bcdUSB = 3.10 in device descriptor)
   If you compile the latest lsusb from sources it will also show the 
SuperSpeedPlus
device capability with the supported speeds.

3. The device itself needs to be 3.1 capable, bcdUSB = 3.1 and it needs to have 
the
   SuperSpeedPlus device capability with the correct speeds supported.

So far I've seen two 3.1 Gen2 host controllers with SBRN set to 0x30 instead of 
0x31
So that could be a likely issue.

If this is becoming a trend we might need to work around it, or ignore checking 
sbrn.

sample outputs:
# ./lsusb -t
/:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 1M
|__ Port 1: Dev 2, If 0, Class=, Driver=usb-storage, 1M
/:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 480M
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/6p, 5000M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/12p, 480M
|__ Port 7: Dev 2, If 0, Class=, Driver=, 12M
|__ Port 7: Dev 2, If 1, Class=, Driver=, 12M


** usb 3.1 root hub: **
#./lsusb -v
Bus 004 Device 001: ID 1d6b:0003
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   3.10
  bDeviceClass9
  bDeviceSubClass 0
  bDeviceProtocol 3
  ...
Hub Descriptor:
  ...
Binary Object Store Descriptor:
  ...
  SuperSpeed USB Device Capability:
  ...
  SuperSpeedPlus USB Device Capability:
bLength28
bDescriptorType16
bDevCapabilityType 10
bmAttributes 0x0023
  Sublink Speed Attribute count 3
  Sublink Speed ID count 1
wFunctionalitySupport   0x0001
bmSublinkSpeedAttr[0]   0x00050034
  Speed attr ID: 4 5Gb/s SuperSpeed
bmSublinkSpeedAttr[1]   0x000500b4
  Speed attr ID: 4 5Gb/s SuperSpeed
bmSublinkSpeedAttr[2]   0x000a4035
  Speed attr ID: 5 10Gb/s SuperSpeedPlus
bmSublinkSpeedAttr[3]   0x000a40b5
  Speed attr ID: 5 10Gb/s SuperSpeedPlus

** usb 3.1 mass storage device **
#./lsusb -v
Bus 004 Device 002: ID 174c:1351
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   3.10
  ...
  iManufacturer   2 Generic
  iProduct3 USB3.1 Device
Binary Object Store Descriptor:
  USB 2.0 Extension Device Capability:
  SuperSpeed USB Device Capability:
...
  SuperSpeedPlus USB Device Capability:
bLength20
bDescriptorType16
bDevCapabilityType 10
bmAttributes 0x0001
  Sublink Speed Attribute count 1
  Sublink Speed ID count 0
wFunctionalitySupport   0x1100
bmSublinkSpeedAttr[0]   0x000a4030
  Speed attr ID: 0 10Gb/s SuperSpeedPlus
bmSublinkSpeedAttr[1]   0x000a40b0
  Speed attr ID: 0 10Gb/s SuperSpeedPlus

-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: [RFC 4/8] usb: phy: move TCSR driver into new file

[Arnd Bergmann]

On Thursday 19 May 2016 14:08:43 Andy Gross wrote:
> > + * - Tim
> > + */
> > +int qcom_tcsr_phy_sel(u32 val)
> > +{
> > +   void __iomem *phy_select;
> > +   int ret;
> > +
> > +   phy_select = ioremap(USB2_PHY_SEL, 4);
> > +
> > +   if (!phy_select) {
> > +   ret = -ENOMEM;
> > +   goto out;
> > +   }
> > +   /* Enable second PHY with the OTG port */
> > +   writel(0x1, phy_select);
> > +   ret = 0;
> > +out:
> > +   iounmap(phy_select);
> > +   return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_tcsr_phy_sel);
> 
> I'd rather do something like what we did for the GSBI.  It needed to
> change some phy related bits in the TCSR as well.  We defined the TCSR
> as a syscon, with binding documentation under mfd.  If we add a syscon
> entry and use it if it is present, we can deal with that pretty
> easily.  The offsets change for each soc, and this would also fix that
> issue because we can change offset based on tcsr compat.

Works for me, but be aware that this will break the server chips,
as ACPI has no support for regmap devices.

I think that's fine, they should really handle this register access
in the firmware anyway rather than relying on a hardcoded MMIO
location.

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

Re: [RFC 4/8] usb: phy: move TCSR driver into new file

[Mark Brown]

On Fri, May 20, 2016 at 02:24:14PM +0200, Arnd Bergmann wrote:
> On Thursday 19 May 2016 14:08:43 Andy Gross wrote:

> > I'd rather do something like what we did for the GSBI.  It needed to
> > change some phy related bits in the TCSR as well.  We defined the TCSR
> > as a syscon, with binding documentation under mfd.  If we add a syscon
> > entry and use it if it is present, we can deal with that pretty
> > easily.  The offsets change for each soc, and this would also fix that
> > issue because we can change offset based on tcsr compat.

> Works for me, but be aware that this will break the server chips,
> as ACPI has no support for regmap devices.

Just to be clear there's nothing precluding the use of regmap on ACPI
devices, it's syscon it doesn't have anything for.


signature.asc
Description: PGP signature

[OOPS] cppi41_dma_channel_program: Unable to handle kernel NULL pointer dereference

[matwey]

Hello,

I am running 4.6-rc3 on BealgeBone Black and when I try to interract with pwc 
webcam attached to usb port the following kernel panic happening.

Please also find used kernel config here:
http://kernel.opensuse.org/cgit/kernel-source/plain/config/armv7hl/default?id=43f1ed415e2227af11856fc120821c791d51494d

[ 4587.746169] Unable to handle kernel NULL pointer dereference at virtual 
address 
[ 4587.754333] pgd = c0204000
[ 4587.757055] [] *pgd=
[ 4587.760667] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
[ 4587.766095] Modules linked in: af_packet bridge stp llc nls_iso8859_1 
nls_cp437 vfat fat snd_usb_audio snd_usbmidi_lib snd_hwdep snd_rawmidi 
snd_seq_device snd_pcm snd_timer snd soundcore pwc videobuf2_vmalloc 
videobuf2_memops videobuf2_v4l2 videobuf2_core videodev media smsc musb_dsps 
musb_hdrc udc_core davinci_mdio phy_am335x usbcore phy_am335x_control 
phy_generic cppi41 usb_common wkup_m3_rproc remoteproc virtio_ring virtio 
pps_gpio ti_cpsw pps_core cpsw_ale cpsw_common omap_aes davinci_cpdma omap_sham 
crypto_engine omap_rng omap_mailbox musb_am335x omap_wdt at24 nvmem_core 
leds_gpio cpufreq_dt mmc_block tps65217_regulator omap_hsmmc omap_dma mmc_core 
tps65217 sg
[ 4587.825879] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
4.6.0-rc3-2.gcfd5095-default #1
[ 4587.833745] Hardware name: Generic AM33XX (Flattened Device Tree)
[ 4587.839867] task: c15081c0 ti: c150 task.ti: c150
[ 4587.845453] PC is at cppi41_dma_channel_program+0x20/0x2f0 [musb_hdrc]
[ 4587.852063] LR is at musb_host_rx+0xc54/0xdd4 [musb_hdrc]
[ 4587.857490] pc : []lr : []psr: 000e0193
[ 4587.857490] sp : c1501c48  ip : c1501cb8  fp : c1501cb4
[ 4587.869021] r10: 9b8443c0  r9 : d92baf48  r8 : e0c72c10
[ 4587.874270] r7 :   r6 : 03c0  r5 : d9c8e900  r4 : d92ba010
[ 4587.880827] r3 : 9b8443c0  r2 :   r1 : 03c0  r0 : 
[ 4587.887385] Flags: nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment 
kernel
[ 4587.894815] Control: 10c5387d  Table: 9b4fc019  DAC: 0055
[ 4587.900587] Process swapper/0 (pid: 0, stack limit = 0xc1500220)
[ 4587.906621] Stack: (0xc1501c48 to 0xc1502000)
[ 4587.911002] 1c40:   0001  c1501ca4 c1501c60 
df926740 c1501c68
[ 4587.919221] 1c60: c03efed0 c0344ec8 8065dcc6  c153c490 df926740 
df926740 2193
[ 4587.927440] 1c80:  df926740 c1504920 d92ba010 d9c8e900 03c0 
 e0c72c10
[ 4587.935659] 1ca0: d92baf48 bf2a55cc c1501d2c c1501cb8 bf297570 bf29f1fc 
03c0 df926740
[ 4587.943877] 1cc0: c1501cf4 c1501cd0 c038ffac c038fef0  9b8443c0 
0c40 d92ba308
[ 4587.952096] 1ce0: d9dcc000 bf2a55c4 db50f800 000e  bf2a4838 
e0c72c10 db50f814
[ 4587.960315] 1d00: c1501d44 d9dcc3bc d92baf48 d92baf48 d92ba010 d92ba010 
e0c72c10 
[ 4587.968533] 1d20: c1501d3c c1501d30 bf28ffb4 bf296928 c1501d94 c1501d40 
bf29eb10 bf28ff28
[ 4587.976751] 1d40: c0392284 c0391db8 c1501d8c c1501d58 c03abc14 c0392274 
c03e77e8 
[ 4587.984970] 1d60: c03c73f0 db4f9bf0 db4f9bec d9dcc3bc  d92baf48 
bf2a5348 d92ba010
[ 4587.993188] 1d80: 200e0193  c1501de4 c1501d98 bf29ec1c bf29e990 
c1501dc4 c1501da8
[ 4588.001406] 1da0: c03ac91c c0c39b04 d9c8e900 0002 0002 03c0 
c03e4a18 
[ 4588.009625] 1dc0: db30ac10  0001 0080 0004 00a0 
c1501e1c c1501de8
[ 4588.017843] 1de0: bf16ff38 bf29eb68 c150d8c0 c03e4a18 c1501e3c d9d4fd80 
db40b2a0 db40b250
[ 4588.026062] 1e00: 0001 00b8  c1500020 c1501e6c c1501e20 
c03c7260 bf16fe48
[ 4588.034281] 1e20: 042c   c162b1a4 c162b1c0 db40b240 
c15035ac 
[ 4588.042499] 1e40: c1501e7c db40b240 db40b2a0 db40b250 0001 db02e000 
c150361c 
[ 4588.050718] 1e60: c1501e8c c1501e70 c03c74a0 c03c71a8 db40b240 db40b2a0 
db40b250 0001
[ 4588.058936] 1e80: c1501eac c1501e90 c03cabf0 c03c7464 c143b77c  
 0001
[ 4588.067154] 1ea0: c1501ebc c1501eb0 c03c676c c03cab40 c1501ee4 c1501ec0 
c03c6abc c03c6744
[ 4588.075373] 1ec0: c16f981c 600e0013  c1501f34 c162b9a0 c150361c 
c1501efc c1501ee8
[ 4588.083592] 1ee0: c0301770 c03c6a5c c0315568 600e0013 c1501f5c c1501f00 
c0c3a280 c0301738
[ 4588.091810] 1f00: 0001   c032c5c0 c150 c1503614 
 c15035ac
[ 4588.100029] 1f20: c162b9a0 c150361c  c1501f5c c1501f60 c1501f50 
c0315560 c0315568
[ 4588.108247] 1f40: 600e0013  c03d5acc c03acc04 c1501f6c c1501f60 
c03acc04 c0315524
[ 4588.116466] 1f60: c1501f94 c1501f70 c03acf24 c03acbe0 0002 c162ad8b 
c143d988 c1501f70
[ 4588.124685] 1f80: c0c35e64 c1503500 c1501fac c1501f98 c0c33f18 c03acc24 
c163f040 0001
[ 4588.132904] 1fa0: c1501ff4 c1501fb0 c1300db8 c0c33e90   
 c1300714
[ 4588.141122] 1fc0:  c13b4a28  c163f5d4 c1503588 c13b4a24 
c150973c 80204059
[ 4588.149340] 1fe0: 413fc082   c1501ff8 8020807c c13009c0 
 
[ 4588.157705

Re: [OOPS] cppi41_dma_channel_program: Unable to handle kernel NULL pointer dereference

[Matwey V. Kornilov]

2016-05-20 16:19 GMT+03:00  :
> Hello,
>
> I am running 4.6-rc3 on BealgeBone Black and when I try to interract with pwc 
> webcam attached to usb port the following kernel panic happening.

Please note, that the same is happening with 4.6.0 release.

>
> Please also find used kernel config here:
> http://kernel.opensuse.org/cgit/kernel-source/plain/config/armv7hl/default?id=43f1ed415e2227af11856fc120821c791d51494d
>
> [ 4587.746169] Unable to handle kernel NULL pointer dereference at virtual 
> address 
> [ 4587.754333] pgd = c0204000
> [ 4587.757055] [] *pgd=
> [ 4587.760667] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
> [ 4587.766095] Modules linked in: af_packet bridge stp llc nls_iso8859_1 
> nls_cp437 vfat fat snd_usb_audio snd_usbmidi_lib snd_hwdep snd_rawmidi 
> snd_seq_device snd_pcm snd_timer snd soundcore pwc videobuf2_vmalloc 
> videobuf2_memops videobuf2_v4l2 videobuf2_core videodev media smsc musb_dsps 
> musb_hdrc udc_core davinci_mdio phy_am335x usbcore phy_am335x_control 
> phy_generic cppi41 usb_common wkup_m3_rproc remoteproc virtio_ring virtio 
> pps_gpio ti_cpsw pps_core cpsw_ale cpsw_common omap_aes davinci_cpdma 
> omap_sham crypto_engine omap_rng omap_mailbox musb_am335x omap_wdt at24 
> nvmem_core leds_gpio cpufreq_dt mmc_block tps65217_regulator omap_hsmmc 
> omap_dma mmc_core tps65217 sg
> [ 4587.825879] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
> 4.6.0-rc3-2.gcfd5095-default #1
> [ 4587.833745] Hardware name: Generic AM33XX (Flattened Device Tree)
> [ 4587.839867] task: c15081c0 ti: c150 task.ti: c150
> [ 4587.845453] PC is at cppi41_dma_channel_program+0x20/0x2f0 [musb_hdrc]
> [ 4587.852063] LR is at musb_host_rx+0xc54/0xdd4 [musb_hdrc]
> [ 4587.857490] pc : []lr : []psr: 000e0193
> [ 4587.857490] sp : c1501c48  ip : c1501cb8  fp : c1501cb4
> [ 4587.869021] r10: 9b8443c0  r9 : d92baf48  r8 : e0c72c10
> [ 4587.874270] r7 :   r6 : 03c0  r5 : d9c8e900  r4 : d92ba010
> [ 4587.880827] r3 : 9b8443c0  r2 :   r1 : 03c0  r0 : 
> [ 4587.887385] Flags: nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment 
> kernel
> [ 4587.894815] Control: 10c5387d  Table: 9b4fc019  DAC: 0055
> [ 4587.900587] Process swapper/0 (pid: 0, stack limit = 0xc1500220)
> [ 4587.906621] Stack: (0xc1501c48 to 0xc1502000)
> [ 4587.911002] 1c40:   0001  c1501ca4 c1501c60 
> df926740 c1501c68
> [ 4587.919221] 1c60: c03efed0 c0344ec8 8065dcc6  c153c490 df926740 
> df926740 2193
> [ 4587.927440] 1c80:  df926740 c1504920 d92ba010 d9c8e900 03c0 
>  e0c72c10
> [ 4587.935659] 1ca0: d92baf48 bf2a55cc c1501d2c c1501cb8 bf297570 bf29f1fc 
> 03c0 df926740
> [ 4587.943877] 1cc0: c1501cf4 c1501cd0 c038ffac c038fef0  9b8443c0 
> 0c40 d92ba308
> [ 4587.952096] 1ce0: d9dcc000 bf2a55c4 db50f800 000e  bf2a4838 
> e0c72c10 db50f814
> [ 4587.960315] 1d00: c1501d44 d9dcc3bc d92baf48 d92baf48 d92ba010 d92ba010 
> e0c72c10 
> [ 4587.968533] 1d20: c1501d3c c1501d30 bf28ffb4 bf296928 c1501d94 c1501d40 
> bf29eb10 bf28ff28
> [ 4587.976751] 1d40: c0392284 c0391db8 c1501d8c c1501d58 c03abc14 c0392274 
> c03e77e8 
> [ 4587.984970] 1d60: c03c73f0 db4f9bf0 db4f9bec d9dcc3bc  d92baf48 
> bf2a5348 d92ba010
> [ 4587.993188] 1d80: 200e0193  c1501de4 c1501d98 bf29ec1c bf29e990 
> c1501dc4 c1501da8
> [ 4588.001406] 1da0: c03ac91c c0c39b04 d9c8e900 0002 0002 03c0 
> c03e4a18 
> [ 4588.009625] 1dc0: db30ac10  0001 0080 0004 00a0 
> c1501e1c c1501de8
> [ 4588.017843] 1de0: bf16ff38 bf29eb68 c150d8c0 c03e4a18 c1501e3c d9d4fd80 
> db40b2a0 db40b250
> [ 4588.026062] 1e00: 0001 00b8  c1500020 c1501e6c c1501e20 
> c03c7260 bf16fe48
> [ 4588.034281] 1e20: 042c   c162b1a4 c162b1c0 db40b240 
> c15035ac 
> [ 4588.042499] 1e40: c1501e7c db40b240 db40b2a0 db40b250 0001 db02e000 
> c150361c 
> [ 4588.050718] 1e60: c1501e8c c1501e70 c03c74a0 c03c71a8 db40b240 db40b2a0 
> db40b250 0001
> [ 4588.058936] 1e80: c1501eac c1501e90 c03cabf0 c03c7464 c143b77c  
>  0001
> [ 4588.067154] 1ea0: c1501ebc c1501eb0 c03c676c c03cab40 c1501ee4 c1501ec0 
> c03c6abc c03c6744
> [ 4588.075373] 1ec0: c16f981c 600e0013  c1501f34 c162b9a0 c150361c 
> c1501efc c1501ee8
> [ 4588.083592] 1ee0: c0301770 c03c6a5c c0315568 600e0013 c1501f5c c1501f00 
> c0c3a280 c0301738
> [ 4588.091810] 1f00: 0001   c032c5c0 c150 c1503614 
>  c15035ac
> [ 4588.100029] 1f20: c162b9a0 c150361c  c1501f5c c1501f60 c1501f50 
> c0315560 c0315568
> [ 4588.108247] 1f40: 600e0013  c03d5acc c03acc04 c1501f6c c1501f60 
> c03acc04 c0315524
> [ 4588.116466] 1f60: c1501f94 c1501f70 c03acf24 c03acbe0 0002 c162ad8b 
> c143d988 c1501f70
> [ 4588.124685] 1f80: c0c35e64 c1503500 c1501fac c1501f98 c0c33f18 c03acc24 
> c163f040 0001
> [ 4588.132904] 1fa

Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Oliver Neukum]

On Fri, 2016-05-20 at 14:24 +0300, Heikki Krogerus wrote:
> On Thu, May 19, 2016 at 04:47:17PM +0200, Oliver Neukum wrote:
> >
> > Please explain. How does that express DRP but prefered master?
> 
> Sorry but I'm not sure what you mean here. If the port is capable of
> being used as dual role port (DRP in the supported_data_roles file),
> that is the only case where you can select the role with this file. So
> I would imagine that in your case you want to make the port act as
> DFP only, right? But if the port is capable of acting only as UFP, you
> are stuck with that role.

How do I trigger that Try.SRC is to be used on a port?

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: xhci DWC3 flavor problem

[Mathias Nyman]

On 19.05.2016 18:42, Joao Pinto wrote:


After a few moments the schedule problem happen again:

# INFO: task kworker/0:1:349 blocked for more than 120 seconds.
   Not tainted 4.6.0-rc5 #9
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/0:1 D ff8008086c60 0   349  2 0x
Workqueue: usb_hub_wq hub_event
Call trace:
[] __switch_to+0xc8/0xd4
[] __schedule+0x18c/0x5c8
[] schedule+0x38/0x98
[] schedule_timeout+0x160/0x1ac
[] wait_for_common+0xac/0x150
[] wait_for_completion+0x14/0x1c
[] xhci_alloc_dev+0xf4/0x2a0
[] usb_alloc_dev+0x68/0x2cc
[] hub_event+0x784/0x11f4
[] process_one_work+0x130/0x2f4
[] worker_thread+0x54/0x434
[] kthread+0xd4/0xe8
[] ret_from_fork+0x10/0x40
INFO: task kworker/0:1:349 blocked for more than 120 seconds.
   Not tainted 4.6.0-rc5 #9
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/0:1 D ff8008086c60 0   349  2 0x
Workqueue: usb_hub_wq hub_event
Call trace:
[] __switch_to+0xc8/0xd4
[] __schedule+0x18c/0x5c8
[] schedule+0x38/0x98
[] schedule_timeout+0x160/0x1ac
[] wait_for_common+0xac/0x150
[] wait_for_completion+0x14/0x1c
[] xhci_alloc_dev+0xf4/0x2a0
[] usb_alloc_dev+0x68/0x2cc
[] hub_event+0x784/0x11f4
[] process_one_work+0x130/0x2f4
[] worker_thread+0x54/0x434
[] kthread+0xd4/0xe8
[] ret_from_fork+0x10/0x40

So Chris' patch did not solve this problem either.

Thanks.


Looks like there still are some issue in cleaning up
pending commands after host dies.

This shouldn't happen, when host dies we clean up the command
ring and call completion for all pending commands, we set the state
to DISABLED or HALTED to prevent new commands from being queued.

But this is all long after the first command times out, ring abortion
fails, and we kill the host, right? So the bigger concern for you is
why the command never complete in the first place

-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: [RFC 4/8] usb: phy: move TCSR driver into new file

[Arnd Bergmann]

On Friday 20 May 2016 13:44:14 Mark Brown wrote:
> On Fri, May 20, 2016 at 02:24:14PM +0200, Arnd Bergmann wrote:
> > On Thursday 19 May 2016 14:08:43 Andy Gross wrote:
> 
> > > I'd rather do something like what we did for the GSBI.  It needed to
> > > change some phy related bits in the TCSR as well.  We defined the TCSR
> > > as a syscon, with binding documentation under mfd.  If we add a syscon
> > > entry and use it if it is present, we can deal with that pretty
> > > easily.  The offsets change for each soc, and this would also fix that
> > > issue because we can change offset based on tcsr compat.
> 
> > Works for me, but be aware that this will break the server chips,
> > as ACPI has no support for regmap devices.
> 
> Just to be clear there's nothing precluding the use of regmap on ACPI
> devices, it's syscon it doesn't have anything for.
> 

Yes, that's what I meant.

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

Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Oliver Neukum]

On Thu, 2016-05-19 at 15:44 +0300, Heikki Krogerus wrote:
> Like I've told some of you guys, I'm trying to implement a bus for
> the Alternate Modes, but I'm still nowhere near finished with that
> one, so let's just get the class ready now. The altmode bus should in
> any case not affect the userspace interface proposed in this patch.

Is this strictly divorced from USB PD?
How do you trigger a cable reset or a USB PD reset?

Regards
Oliver


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

Re: [PATCH] usb: echi-hcd: Add ehci_setup check before echi_shutdown

[Alan Stern]

On Thu, 19 May 2016, Andy Gross wrote:

> On 19 May 2016 at 05:12, Srinivas Kandagatla
>  wrote:
> 
> 
> 
> > +++ b/drivers/usb/host/ehci-hcd.c
> > @@ -368,6 +368,15 @@ static void ehci_shutdown(struct usb_hcd *hcd)
> >  {
> > struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> >
> > +   /**
> > +* Protect the system from crashing at system shutdown in cases 
> > where
> > +* usb host is not added yet from OTG controller driver.
> > +* As ehci_setup() not done yet, so stop accessing registers or
> > +* variables initialized in ehci_setup()
> > +*/
> > +   if (!ehci->sbrn)
> > +   return;
> > +
> > spin_lock_irq(&ehci->lock);
> > ehci->shutdown = true;
> > ehci->rh_state = EHCI_RH_STOPPING;
> 
> 
> Should we also not check this in ehci_suspend and ehci_resume?  If I
> do suspend/resume, it crashes in a similar manner.  I know this goes
> beyond the problem scope of the patch.  Perhaps I should send in an
> additional patch with the code?
> 
> Are there any other places in the ehci-hcd where this needs to be checked?

Really, this is an indication that the problem lies not in ehci-hcd but 
rather in the platform glue code.  The platform code allows itself to 
be registered as the driver of the controller device but does not 
initialize the ehci-hcd parts.

Neither ehci_suspend nor ehci_resume is called directly by the driver
core; the calls all have to pass through the glue code.  So maybe the
glue layer needs to be careful about invoking these routines in
ehci-hcd before ehci-hcd has been initialized.

Alan Stern

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

Re: composite gadget with _real_ USB device

[Alan Stern]

On Fri, 20 May 2016, Felipe Balbi wrote:

> 
> Hi,
> 
> Shea Ako  writes:
> > I’ve been learning about and playing with configfs and functionfs to
> > create composite user space USB gadgets. My objective is to create a
> > composite USB gadget that incorporates a custom functionfs function of
> > my own creation along with some _real_ USB devices connected to my
> > linux platform.
> >
> > Is there an easy existing way to bundle _real_ USB devices into a
> > composite gadget like this or am I looking at writing my own user
> > space functionfs functions which handle wrapping and forwarding the
> > interfaces/endpoints/data of the connected _real_ USB devices?
> 
> heh, you're really on your own. Sounds pretty interesting but you're
> gonna spend a lot of time with this :p
> 
> > I haven’t found any documented existing way to do this, but I thought
> > I should ask before I go off an implement it myself as it seems that
> > this might be a use case which isn’t entirely off the wall and there
> > could already be support for this somewhere that I haven’t yet
> > encountered.
> 
> I don't think anybody has done anything like this yet. The closest I got
> was to attach some USB sticks to host port via HUB, setup RAID and use
> that RAID as backing store for g_mass_storage, then connect
> g_mass_storage back to same host which has the RAID of several USB
> sticks (same machine has host and peripheral controllers).

How about setting up USBIP, using the same machine as both the server
and the client?

Alan Stern

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

Re: [PATCH] - silence UBSAN complaint in ehci-hcd.

[Alan Stern]

On Thu, 19 May 2016 valdis.kletni...@vt.edu wrote:

> On Thu, 19 May 2016 17:50:31 -0700, Greg Kroah-Hartman said:
> > On Thu, May 19, 2016 at 05:19:00PM -0400, Valdis Kletnieks wrote:
> > > UBSAN throws a complaint:
> > >
> > > [2.418579] UBSAN: Undefined behaviour in 
> > > drivers/usb/host/ehci-hub.c:877:47
> > > [2.418582] index -1 is out of range for type 'u32 [1]'
> > >
> > > though it's only on the hostpc[] part, not  on the port_status[] on the
> > > previous line which has the same exact index calculation.  The root cause 
> > > is
> > > that the first declaration is port_status[0], which uses a GCC extension 
> > > and
> > > UBSAN is smart enough to realize the programmer is doing something
> > > intentionally odd.
> > >
> > > However, the problematic declaration is hostpc[1], which doesn't have
> > > the "I know what I'm doing" semantics of [0].  Change the declaration to 
> > > match.
> > >
> > > Signed-Off-By: Valdis Kletnieks 
> > >
> > > --- a/include/linux/usb/ehci_def.h2015-01-06 01:04:24.342436706 
> > > -0500
> > > +++ b/include/linux/usb/ehci_def.h2016-05-19 13:57:20.869304540 
> > > -0400
> > > @@ -180,11 +180,11 @@ struct ehci_regs {
> > >   * PORTSCx
> > >   */
> > >   /* HOSTPC: offset 0x84 */
> > > - u32 hostpc[1];  /* HOSTPC extension */
> > > + u32 hostpc[0];  /* HOSTPC extension */
> > >  #define HOSTPC_PHCD  (1<<22) /* Phy clock disable */
> > >  #define HOSTPC_PSPD  (3<<25) /* Port speed detection */
> >
> > Hm, this is odd, you really do want hostpc to be 1 u32 value, don't make
> > it 0 please.  If you walk off the end of hostpc, well, let's fix that
> > properly.
> 
> Well, UBSAN doesn't complain about the *other* use of the same exact index,
> apparently because 'u32 port_status[0]' tells it to shut up we know what we're
> doing.
> 
> And I'm pretty sure that if hostpc was supposed to be exactly one u32 rather
> than an array, it wouldn't have the [] semantics everyplace...

hostpc is supposed to have the same number of elements as the number of 
ports.  (On the other hand, I'm not sure if any of the platforms which 
implement the hostpc register have more than one port...)

Alan Stern

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

Re: [PATCH] - silence UBSAN complaint in ehci-hcd.

[Alan Stern]

On Thu, 19 May 2016, Valdis Kletnieks wrote:

> UBSAN throws a complaint:
> 
> [2.418579] UBSAN: Undefined behaviour in 
> drivers/usb/host/ehci-hub.c:877:47
> [2.418582] index -1 is out of range for type 'u32 [1]'
> 
> though it's only on the hostpc[] part, not  on the port_status[] on the
> previous line which has the same exact index calculation.  The root cause is
> that the first declaration is port_status[0], which uses a GCC extension and
> UBSAN is smart enough to realize the programmer is doing something
> intentionally odd.
> 
> However, the problematic declaration is hostpc[1], which doesn't have
> the "I know what I'm doing" semantics of [0].  Change the declaration to 
> match.
> 
> Signed-Off-By: Valdis Kletnieks 
> 
> --- a/include/linux/usb/ehci_def.h2015-01-06 01:04:24.342436706 -0500
> +++ b/include/linux/usb/ehci_def.h2016-05-19 13:57:20.869304540 -0400
> @@ -180,11 +180,11 @@ struct ehci_regs {
>   * PORTSCx
>   */
>   /* HOSTPC: offset 0x84 */
> - u32 hostpc[1];  /* HOSTPC extension */
> + u32 hostpc[0];  /* HOSTPC extension */
>  #define HOSTPC_PHCD  (1<<22) /* Phy clock disable */
>  #define HOSTPC_PSPD  (3<<25) /* Port speed detection */
>  
> - u32 reserved5[16];
> + u32 reserved5[17];
>  
>   /* USBMODE_EX: offset 0xc8 */
>   u32 usbmode_ex; /* USB Device mode extension */

Is this problem still present with my latest patch 
(http://marc.info/?l=linux-usb&m=146368979514228&w=2)?

I agree that this is a reasonable change to make in any case.

Alan Stern

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

[PATCH 2/2] usb: musb: Stop bulk endpoint while queue is rotated

[Andrew Goodbody]

Ensure that the endpoint is stopped by clearing REQPKT before
clearing DATAERR_NAKTIMEOUT before rotating the queue on the
dedicated bulk endpoint.
This addresses an issue where a race could result in the endpoint
receiving data before it was reprogrammed resulting in a warning
about such data from musb_rx_reinit before it was thrown away.
The data thrown away was a valid packet that had been correctly
ACKed which meant the host and device got out of sync.

Signed-off-by: Andrew Goodbody 
Cc: sta...@vger.kernel.org
---
 drivers/usb/musb/musb_host.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 30e0d65..777ff30 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -999,6 +999,8 @@ static void musb_bulk_nak_timeout(struct musb *musb, struct 
musb_hw_ep *ep,
/* clear nak timeout bit */
rx_csr = musb_readw(epio, MUSB_RXCSR);
rx_csr |= MUSB_RXCSR_H_WZC_BITS;
+   rx_csr &= ~MUSB_RXCSR_H_REQPKT;
+   musb_writew(epio, MUSB_RXCSR, rx_csr);
rx_csr &= ~MUSB_RXCSR_DATAERROR;
musb_writew(epio, MUSB_RXCSR, rx_csr);
 
-- 
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

[PATCH 1/2] usb: musb: Ensure rx reinit occurs for shared_fifo endpoints

[Andrew Goodbody]

shared_fifo endpoints would only get a previous tx state cleared
out, the rx state was only cleared for non shared_fifo endpoints
Change this so that the rx state is cleared for all endpoints.
This addresses an issue that resulted in rx packets being dropped
silently.

Signed-off-by: Andrew Goodbody 
Cc: sta...@vger.kernel.org
---
 drivers/usb/musb/musb_host.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 2f8ad7f..30e0d65 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -594,14 +594,15 @@ musb_rx_reinit(struct musb *musb, struct musb_qh *qh, u8 
epnum)
musb_writew(ep->regs, MUSB_TXCSR, 0);
 
/* scrub all previous state, clearing toggle */
-   } else {
-   csr = musb_readw(ep->regs, MUSB_RXCSR);
-   if (csr & MUSB_RXCSR_RXPKTRDY)
-   WARNING("rx%d, packet/%d ready?\n", ep->epnum,
-   musb_readw(ep->regs, MUSB_RXCOUNT));
-
-   musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG);
}
+   csr = musb_readw(ep->regs, MUSB_RXCSR);
+   if (csr & MUSB_RXCSR_RXPKTRDY) {
+   WARNING("rx%d, packet/%d ready?\n", ep->epnum,
+   musb_readw(ep->regs, MUSB_RXCOUNT));
+   urb_qh_dump(musb);
+   }
+
+   musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG);
 
/* target addr and (for multipoint) hub addr/port */
if (musb->is_multipoint) {
-- 
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

[PATCH 0/2] usb: musb: fix dropped packets

[Andrew Goodbody]

The musb driver can drop rx packets when heavily loaded. These two
patches address two issues that can cause this. Both issues arose
when an endpoint was reprogrammed. The first patch is a logic bug
that resulted in a shared_fifo in rx mode not having its state
cleared out. The second patch fixes a race condition caused by
not stopping the dedicated endpoint for bulk packets before
rotating its queue which allowed a packet to be recieved and then
thrown away.

Andrew Goodbody (2):
  usb: musb: Ensure rx reinit occurs for shared_fifo endpoints
  usb: musb: Stop bulk endpoint while queue is rotated

 drivers/usb/musb/musb_host.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

-- 
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 1/2] usb: musb: Ensure rx reinit occurs for shared_fifo endpoints

[Sergei Shtylyov]

Hello.

On 05/20/2016 05:51 PM, Andrew Goodbody wrote:


shared_fifo endpoints would only get a previous tx state cleared
out, the rx state was only cleared for non shared_fifo endpoints
Change this so that the rx state is cleared for all endpoints.
This addresses an issue that resulted in rx packets being dropped
silently.

Signed-off-by: Andrew Goodbody 
Cc: sta...@vger.kernel.org
---
 drivers/usb/musb/musb_host.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 2f8ad7f..30e0d65 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -594,14 +594,15 @@ musb_rx_reinit(struct musb *musb, struct musb_qh *qh, u8 
epnum)
musb_writew(ep->regs, MUSB_TXCSR, 0);

/* scrub all previous state, clearing toggle */
-   } else {
-   csr = musb_readw(ep->regs, MUSB_RXCSR);
-   if (csr & MUSB_RXCSR_RXPKTRDY)
-   WARNING("rx%d, packet/%d ready?\n", ep->epnum,
-   musb_readw(ep->regs, MUSB_RXCOUNT));
-
-   musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG);
}
+   csr = musb_readw(ep->regs, MUSB_RXCSR);
+   if (csr & MUSB_RXCSR_RXPKTRDY) {
+   WARNING("rx%d, packet/%d ready?\n", ep->epnum,
+   musb_readw(ep->regs, MUSB_RXCOUNT));
+   urb_qh_dump(musb);


   I'm not seeing this function anywhere... debugging leftover?


+   }
+
+   musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG);

/* target addr and (for multipoint) hub addr/port */
if (musb->is_multipoint) {


MBR, Sergei

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

Re: [PATCH] usb: echi-hcd: Add ehci_setup check before echi_shutdown

[Andy Gross]

On 20 May 2016 at 09:31, Alan Stern  wrote:
> On Thu, 19 May 2016, Andy Gross wrote:
>
>> On 19 May 2016 at 05:12, Srinivas Kandagatla
>>  wrote:
>>
>> 
>>
>> > +++ b/drivers/usb/host/ehci-hcd.c
>> > @@ -368,6 +368,15 @@ static void ehci_shutdown(struct usb_hcd *hcd)
>> >  {
>> > struct ehci_hcd *ehci = hcd_to_ehci(hcd);
>> >
>> > +   /**
>> > +* Protect the system from crashing at system shutdown in cases 
>> > where
>> > +* usb host is not added yet from OTG controller driver.
>> > +* As ehci_setup() not done yet, so stop accessing registers or
>> > +* variables initialized in ehci_setup()
>> > +*/
>> > +   if (!ehci->sbrn)
>> > +   return;
>> > +
>> > spin_lock_irq(&ehci->lock);
>> > ehci->shutdown = true;
>> > ehci->rh_state = EHCI_RH_STOPPING;
>>
>>
>> Should we also not check this in ehci_suspend and ehci_resume?  If I
>> do suspend/resume, it crashes in a similar manner.  I know this goes
>> beyond the problem scope of the patch.  Perhaps I should send in an
>> additional patch with the code?
>>
>> Are there any other places in the ehci-hcd where this needs to be checked?
>
> Really, this is an indication that the problem lies not in ehci-hcd but
> rather in the platform glue code.  The platform code allows itself to
> be registered as the driver of the controller device but does not
> initialize the ehci-hcd parts.
>
> Neither ehci_suspend nor ehci_resume is called directly by the driver
> core; the calls all have to pass through the glue code.  So maybe the
> glue layer needs to be careful about invoking these routines in
> ehci-hcd before ehci-hcd has been initialized.

That's a good point.  Perhaps all of this should be in the glue,
including the shutdown case.

Regards,

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

usb: customized USB Hub device doubt

[Muni Sekhar]

Hi,


We have a customized USB Hub device which has a inbuilt device attached to it.

By default Linux kernel’s USB Hub Class driver claims the customized Hub.

I need to Send Vendor Specific command to my USB Hub to enable the
inbuilt device. What is the best way to achieve it?



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

Re: [PATCH] usb: echi-hcd: Add ehci_setup check before echi_shutdown

[Alan Stern]

On Fri, 20 May 2016, Andy Gross wrote:

> On 20 May 2016 at 09:31, Alan Stern  wrote:
> > On Thu, 19 May 2016, Andy Gross wrote:
> >
> >> On 19 May 2016 at 05:12, Srinivas Kandagatla
> >>  wrote:
> >>
> >> 
> >>
> >> > +++ b/drivers/usb/host/ehci-hcd.c
> >> > @@ -368,6 +368,15 @@ static void ehci_shutdown(struct usb_hcd *hcd)
> >> >  {
> >> > struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> >> >
> >> > +   /**
> >> > +* Protect the system from crashing at system shutdown in cases 
> >> > where
> >> > +* usb host is not added yet from OTG controller driver.
> >> > +* As ehci_setup() not done yet, so stop accessing registers or
> >> > +* variables initialized in ehci_setup()
> >> > +*/
> >> > +   if (!ehci->sbrn)
> >> > +   return;
> >> > +
> >> > spin_lock_irq(&ehci->lock);
> >> > ehci->shutdown = true;
> >> > ehci->rh_state = EHCI_RH_STOPPING;
> >>
> >>
> >> Should we also not check this in ehci_suspend and ehci_resume?  If I
> >> do suspend/resume, it crashes in a similar manner.  I know this goes
> >> beyond the problem scope of the patch.  Perhaps I should send in an
> >> additional patch with the code?
> >>
> >> Are there any other places in the ehci-hcd where this needs to be checked?
> >
> > Really, this is an indication that the problem lies not in ehci-hcd but
> > rather in the platform glue code.  The platform code allows itself to
> > be registered as the driver of the controller device but does not
> > initialize the ehci-hcd parts.
> >
> > Neither ehci_suspend nor ehci_resume is called directly by the driver
> > core; the calls all have to pass through the glue code.  So maybe the
> > glue layer needs to be careful about invoking these routines in
> > ehci-hcd before ehci-hcd has been initialized.
> 
> That's a good point.  Perhaps all of this should be in the glue,
> including the shutdown case.

Many of the platform glue files set usb_hcd_platform_shutdown() as 
their shutdown callback, which makes it impossible for them to include 
a controller-specific test.

Alan Stern

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

[PATCH][v3.13.y-ckt][v3.19-ckt][v4.2.y-ckt]Revert "usb: hub: do not clear BOS field during reset device"

[Joseph Salisbury]

Hello,

Please consider including commit 
e5bdfd50d6f76077bf8441d130c606229e100d40 in the next v3.13.y-ckt,
v3.19-ckt and v4.2.y-ckt releases.  It was included upstream as of
v4.5-rc6.  This commit has been tested and resolves the following bug:
http://pad.lv/1582864.

According to the commit message, this patch has already been included in
the following -stable kernels:
4.5.0-rc4 (current git)
4.4.2
4.3.6 (currently in review)
4.1.18
3.18.27
3.14.61


commit e5bdfd50d6f76077bf8441d130c606229e100d40
Author: Greg Kroah-Hartman 
Date:   Sat Feb 20 14:19:34 2016 -0800

Revert "usb: hub: do not clear BOS field during reset device"




Sincerely,

Joseph Salisbury


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

Re: [PATCH] usb: echi-hcd: Add ehci_setup check before echi_shutdown

[Andy Gross]

On 20 May 2016 at 10:57, Alan Stern  wrote:
> On Fri, 20 May 2016, Andy Gross wrote:
>
>> On 20 May 2016 at 09:31, Alan Stern  wrote:
>> > On Thu, 19 May 2016, Andy Gross wrote:
>> >
>> >> On 19 May 2016 at 05:12, Srinivas Kandagatla
>> >>  wrote:
>> >>
>> >> 
>> >>
>> >> > +++ b/drivers/usb/host/ehci-hcd.c
>> >> > @@ -368,6 +368,15 @@ static void ehci_shutdown(struct usb_hcd *hcd)
>> >> >  {
>> >> > struct ehci_hcd *ehci = hcd_to_ehci(hcd);
>> >> >
>> >> > +   /**
>> >> > +* Protect the system from crashing at system shutdown in cases 
>> >> > where
>> >> > +* usb host is not added yet from OTG controller driver.
>> >> > +* As ehci_setup() not done yet, so stop accessing registers or
>> >> > +* variables initialized in ehci_setup()
>> >> > +*/
>> >> > +   if (!ehci->sbrn)
>> >> > +   return;
>> >> > +
>> >> > spin_lock_irq(&ehci->lock);
>> >> > ehci->shutdown = true;
>> >> > ehci->rh_state = EHCI_RH_STOPPING;
>> >>
>> >>
>> >> Should we also not check this in ehci_suspend and ehci_resume?  If I
>> >> do suspend/resume, it crashes in a similar manner.  I know this goes
>> >> beyond the problem scope of the patch.  Perhaps I should send in an
>> >> additional patch with the code?
>> >>
>> >> Are there any other places in the ehci-hcd where this needs to be checked?
>> >
>> > Really, this is an indication that the problem lies not in ehci-hcd but
>> > rather in the platform glue code.  The platform code allows itself to
>> > be registered as the driver of the controller device but does not
>> > initialize the ehci-hcd parts.
>> >
>> > Neither ehci_suspend nor ehci_resume is called directly by the driver
>> > core; the calls all have to pass through the glue code.  So maybe the
>> > glue layer needs to be careful about invoking these routines in
>> > ehci-hcd before ehci-hcd has been initialized.
>>
>> That's a good point.  Perhaps all of this should be in the glue,
>> including the shutdown case.
>
> Many of the platform glue files set usb_hcd_platform_shutdown() as
> their shutdown callback, which makes it impossible for them to include
> a controller-specific test.

Yes, I just found this out for myself.  So in that specific case, it
makes sense to have the check within the ehci-hcd.  Whereas in the
glue driver, the suspend/resume use direct calls to
ehci_suspend/resume.

I'll send along a ehci-msm patch to address the issues i see with
suspend/resume.

Thanks!

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

Re: [PATCH 2/2] usb: musb: Stop bulk endpoint while queue is rotated

[Sergei Shtylyov]

On 05/20/2016 05:51 PM, Andrew Goodbody wrote:


Ensure that the endpoint is stopped by clearing REQPKT before
clearing DATAERR_NAKTIMEOUT before rotating the queue on the
dedicated bulk endpoint.
This addresses an issue where a race could result in the endpoint
receiving data before it was reprogrammed resulting in a warning
about such data from musb_rx_reinit before it was thrown away.
The data thrown away was a valid packet that had been correctly
ACKed which meant the host and device got out of sync.

Signed-off-by: Andrew Goodbody 
Cc: sta...@vger.kernel.org
---
 drivers/usb/musb/musb_host.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 30e0d65..777ff30 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -999,6 +999,8 @@ static void musb_bulk_nak_timeout(struct musb *musb, struct 
musb_hw_ep *ep,
/* clear nak timeout bit */
rx_csr = musb_readw(epio, MUSB_RXCSR);
rx_csr |= MUSB_RXCSR_H_WZC_BITS;
+   rx_csr &= ~MUSB_RXCSR_H_REQPKT;
+   musb_writew(epio, MUSB_RXCSR, rx_csr);
rx_csr &= ~MUSB_RXCSR_DATAERROR;
musb_writew(epio, MUSB_RXCSR, rx_csr);


   Can we not clear both in one write?

[...]

MBR, Sergei

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

Re: [PATCH] net: pegasus: remove unused variables and labels

[David Miller]

From: Arnd Bergmann 
Date: Fri, 20 May 2016 10:22:30 +0200

> The latest dead-code removal was slightly incomplete and
> left a few things behind that we now get compiler warnings
> for:
> 
> drivers/net/usb/pegasus.c: In function 'read_bulk_callback':
> drivers/net/usb/pegasus.c:475:1: error: label 'goon' defined but not used 
> [-Werror=unused-label]
> drivers/net/usb/pegasus.c:446:8: error: unused variable 'pkt_len' 
> [-Werror=unused-variable]
> drivers/net/usb/pegasus.c:445:6: error: unused variable 'buf' 
> [-Werror=unused-variable]
> drivers/net/usb/pegasus.c:443:17: error: unused variable 'count' 
> [-Werror=unused-variable]
> 
> This removes them as well.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: e00be9e4d0ff ("net: pegasus: remove dead coding")

The patch in question was broken and has been reverted.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] net: pegasus: remove dead coding

[David Miller]

From: Petko Manolov 
Date: Fri, 20 May 2016 10:33:47 +0300

> On 16-05-19 11:35:42, David Miller wrote:
>> From: Heinrich Schuchardt 
>> Date: Wed, 18 May 2016 02:13:30 +0200
>> 
>> > (!count || count < 4) is always true.
>> > So let's remove the coding which is dead at least since 2005.
>> > 
>> > Signed-off-by: Heinrich Schuchardt 
>> 
>> Applied.
> 
> David, the patch you applied is broken.  It seems that you didn't follow the 
> discussion from the past couple of days.  Please revert it.

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

RE: [PATCH 1/2] usb: musb: Ensure rx reinit occurs for shared_fifo endpoints

[Andrew Goodbody]

> From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com]
> 
> Hello.
> 
> On 05/20/2016 05:51 PM, Andrew Goodbody wrote:
> 
> > shared_fifo endpoints would only get a previous tx state cleared out,
> > the rx state was only cleared for non shared_fifo endpoints Change
> > this so that the rx state is cleared for all endpoints.
> > This addresses an issue that resulted in rx packets being dropped
> > silently.
> >
> > Signed-off-by: Andrew Goodbody 
> > Cc: sta...@vger.kernel.org
> > ---
> >  drivers/usb/musb/musb_host.c | 15 ---
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/usb/musb/musb_host.c
> > b/drivers/usb/musb/musb_host.c index 2f8ad7f..30e0d65 100644
> > --- a/drivers/usb/musb/musb_host.c
> > +++ b/drivers/usb/musb/musb_host.c
> > @@ -594,14 +594,15 @@ musb_rx_reinit(struct musb *musb, struct
> musb_qh *qh, u8 epnum)
> > musb_writew(ep->regs, MUSB_TXCSR, 0);
> >
> > /* scrub all previous state, clearing toggle */
> > -   } else {
> > -   csr = musb_readw(ep->regs, MUSB_RXCSR);
> > -   if (csr & MUSB_RXCSR_RXPKTRDY)
> > -   WARNING("rx%d, packet/%d ready?\n", ep-
> >epnum,
> > -   musb_readw(ep->regs, MUSB_RXCOUNT));
> > -
> > -   musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG);
> > }
> > +   csr = musb_readw(ep->regs, MUSB_RXCSR);
> > +   if (csr & MUSB_RXCSR_RXPKTRDY) {
> > +   WARNING("rx%d, packet/%d ready?\n", ep->epnum,
> > +   musb_readw(ep->regs, MUSB_RXCOUNT));
> > +   urb_qh_dump(musb);
> 
> I'm not seeing this function anywhere... debugging leftover?

Sorry, yes, my bad. I'll remove it for v2.

Andrew

> > +   }
> > +
> > +   musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG);
> >
> > /* target addr and (for multipoint) hub addr/port */
> > if (musb->is_multipoint) {
> 
> MBR, Sergei

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

Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Guenter Roeck]

On Fri, May 20, 2016 at 01:47:03PM +0300, Heikki Krogerus wrote:
> On Thu, May 19, 2016 at 10:53:04AM -0700, Guenter Roeck wrote:
> > Hello Heikki,
> > 
> > On Thu, May 19, 2016 at 03:44:54PM +0300, Heikki Krogerus wrote:
> > > The purpose of this class is to provide unified interface for user
> > > space to get the status and basic information about USB Type-C
> > > Connectors in the system, control data role swapping, and when USB PD
> > > is available, also power role swapping and Alternate Modes.
> > > 
> > > Signed-off-by: Heikki Krogerus 
> > > ---
> > >  drivers/usb/Kconfig |   2 +
> > >  drivers/usb/Makefile|   2 +
> > >  drivers/usb/type-c/Kconfig  |   7 +
> > >  drivers/usb/type-c/Makefile |   1 +
> > >  drivers/usb/type-c/typec.c  | 957 
> > > 
> > >  include/linux/usb/typec.h   | 230 +++
> > >  6 files changed, 1199 insertions(+)
> > >  create mode 100644 drivers/usb/type-c/Kconfig
> > >  create mode 100644 drivers/usb/type-c/Makefile
> > >  create mode 100644 drivers/usb/type-c/typec.c
> > >  create mode 100644 include/linux/usb/typec.h
> > > 
> > > Hi,
> > > 
> > > Like I've told some of you guys, I'm trying to implement a bus for
> > > the Alternate Modes, but I'm still nowhere near finished with that
> > > one, so let's just get the class ready now. The altmode bus should in
> > > any case not affect the userspace interface proposed in this patch.
> > > 
> > > As you can see, the Alternate Modes are handled completely differently
> > > compared to the original proposal. Every Alternate Mode will have
> > > their own device instance (which will be then later bound to an
> > > Alternate Mode specific driver once we have the bus), but also every
> > > partner, cable and cable plug will have their own device instances
> > > representing them.
> > > 
> > > An other change is that the data role is now handled in two ways.
> > > The current_data_role file will represent static mode of the port, and
> > > it will use the names for the roles as they are defined in the spec:
> > > DFP, UFP and DRP. This file should be used if the port needs to be
> > > fixed to one specific role with DRP ports. So this approach will
> > > replace the suggestions for "preferred" data role we had. The
> > > current_usb_data_role will use values "host" and "device" and it will
> > > be used for data role swapping when already connected.
> > > 
> > 
> > What I am missing completely is a means to handle role and alternate mode
> > changes triggered by the partner. The need for those should be obvious,
> > unless I am really missing something (just consider two devices supporting
> > this code connected to each other).
> 
> We are missing the notifications that are needed in these cases. But I
> don't see much more we can do about those cases. We can not put any
> policies in place at this level, because we have to be able to support
> also things like USB PD and Type-C controllers that take care of all
> that, leaving us to not be able to do anything else but to pass the
> information forward. So the framework at this level has to be
> "stupid", and if more infrastructure is needed, it has to be
> introduced in an other layer.
> 
Ok.

> > Also, I am not sure where the policy engine is supposed to reside.
> > I understand that some policy changes (eg unsolicited requests to switch 
> > roles)
> > can be triggered from user space. However, role change requests triggered 
> > from
> > the partner need to be evaluated quickly (typically within 15 ms), so user
> > space can not get involved. Maybe it would help to have some text describing
> > where the policy engine is expected to reside and how it is involved
> > in the decision making process. This includes the initial decision making
> > process, when it needs to be decided if role changes should be requested
> > or if one or multiple alternate modes should be entered after the initial
> > connection has been established.
> 
> Well, yes we need to document these things, but you are now coupling
> this framework with USB PD and we really should not do that.
> 
Not really. I was trying to understand where you would expect the policy engine
to reside, which you answered above.

> The policy engine, and the whole USB PD stack, belongs inside the
> kernel, and it will be completely separated from this framework. This
> framework can not have any dependencies on the future USB PD stack.
> This is not only because of the USB PD/Type-C controllers which handle
> the policy engine on their own and only allow "unsolicited" requests
> like "swap role" and "enter/exit mode", but also because this
> framework must work smoothly on systems that don't want to use USB PD
> and of course also with USB Type-C PHYs that simply don't include USB
> PD transceiver.
> 
> The layer that joins these two parts together will be the port drivers
> themselves, so the USB Type-C/PD PHYs and controllers, at least in the
> beginning.
> 

RE: [PATCH 2/2] usb: musb: Stop bulk endpoint while queue is rotated

[Andrew Goodbody]

> From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com]
> On 05/20/2016 05:51 PM, Andrew Goodbody wrote:
> 
> > Ensure that the endpoint is stopped by clearing REQPKT before clearing
> > DATAERR_NAKTIMEOUT before rotating the queue on the dedicated bulk
> > endpoint.
> > This addresses an issue where a race could result in the endpoint
> > receiving data before it was reprogrammed resulting in a warning about
> > such data from musb_rx_reinit before it was thrown away.
> > The data thrown away was a valid packet that had been correctly ACKed
> > which meant the host and device got out of sync.
> >
> > Signed-off-by: Andrew Goodbody 
> > Cc: sta...@vger.kernel.org
> > ---
> >  drivers/usb/musb/musb_host.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/usb/musb/musb_host.c
> > b/drivers/usb/musb/musb_host.c index 30e0d65..777ff30 100644
> > --- a/drivers/usb/musb/musb_host.c
> > +++ b/drivers/usb/musb/musb_host.c
> > @@ -999,6 +999,8 @@ static void musb_bulk_nak_timeout(struct musb
> *musb, struct musb_hw_ep *ep,
> > /* clear nak timeout bit */
> > rx_csr = musb_readw(epio, MUSB_RXCSR);
> > rx_csr |= MUSB_RXCSR_H_WZC_BITS;
> > +   rx_csr &= ~MUSB_RXCSR_H_REQPKT;
> > +   musb_writew(epio, MUSB_RXCSR, rx_csr);
> > rx_csr &= ~MUSB_RXCSR_DATAERROR;
> > musb_writew(epio, MUSB_RXCSR, rx_csr);
> 
> Can we not clear both in one write?

Section 16.3.8.2.2.1.2 of the TRM says to clear REQPKT before 
DATAERR_NAKTIMEOUT.

Andrew

> [...]
> 
> MBR, Sergei

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

Re: [PATCH 2/2] usb: musb: Stop bulk endpoint while queue is rotated

[Sergei Shtylyov]

On 05/20/2016 08:06 PM, Andrew Goodbody wrote:


Ensure that the endpoint is stopped by clearing REQPKT before clearing
DATAERR_NAKTIMEOUT before rotating the queue on the dedicated bulk
endpoint.
This addresses an issue where a race could result in the endpoint
receiving data before it was reprogrammed resulting in a warning about
such data from musb_rx_reinit before it was thrown away.
The data thrown away was a valid packet that had been correctly ACKed
which meant the host and device got out of sync.

Signed-off-by: Andrew Goodbody 
Cc: sta...@vger.kernel.org
---
 drivers/usb/musb/musb_host.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/musb/musb_host.c
b/drivers/usb/musb/musb_host.c index 30e0d65..777ff30 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -999,6 +999,8 @@ static void musb_bulk_nak_timeout(struct musb

*musb, struct musb_hw_ep *ep,

/* clear nak timeout bit */
rx_csr = musb_readw(epio, MUSB_RXCSR);
rx_csr |= MUSB_RXCSR_H_WZC_BITS;
+   rx_csr &= ~MUSB_RXCSR_H_REQPKT;
+   musb_writew(epio, MUSB_RXCSR, rx_csr);
rx_csr &= ~MUSB_RXCSR_DATAERROR;
musb_writew(epio, MUSB_RXCSR, rx_csr);


Can we not clear both in one write?


Section 16.3.8.2.2.1.2 of the TRM says to clear REQPKT before 
DATAERR_NAKTIMEOUT.


   Right, the MUSB programmer's guide also says that. Then a comment wouldn't 
hurt here.



Andrew


MBR, Sergei

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

Re: usb: customized USB Hub device doubt

[Greg KH]

On Fri, May 20, 2016 at 09:20:15PM +0530, Muni Sekhar wrote:
> Hi,
> 
> 
> We have a customized USB Hub device which has a inbuilt device attached to it.
> 
> By default Linux kernel’s USB Hub Class driver claims the customized Hub.
> 
> I need to Send Vendor Specific command to my USB Hub to enable the
> inbuilt device. What is the best way to achieve it?

Send it from userspace using usbfs?

Or write a new hub driver or I'm sure the existing hub driver can be
extended to support "custom" hubs, it shouldn't be that tough.

good luck!

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

RE: [PATCH 2/2] usb: musb: Stop bulk endpoint while queue is rotated

[Andrew Goodbody]

> From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com]
> On 05/20/2016 08:06 PM, Andrew Goodbody wrote:
> 
> >>> Ensure that the endpoint is stopped by clearing REQPKT before
> >>> clearing DATAERR_NAKTIMEOUT before rotating the queue on the
> >>> dedicated bulk endpoint.
> >>> This addresses an issue where a race could result in the endpoint
> >>> receiving data before it was reprogrammed resulting in a warning
> >>> about such data from musb_rx_reinit before it was thrown away.
> >>> The data thrown away was a valid packet that had been correctly
> >>> ACKed which meant the host and device got out of sync.
> >>>
> >>> Signed-off-by: Andrew Goodbody
> 
> >>> Cc: sta...@vger.kernel.org
> >>> ---
> >>>  drivers/usb/musb/musb_host.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/usb/musb/musb_host.c
> >>> b/drivers/usb/musb/musb_host.c index 30e0d65..777ff30 100644
> >>> --- a/drivers/usb/musb/musb_host.c
> >>> +++ b/drivers/usb/musb/musb_host.c
> >>> @@ -999,6 +999,8 @@ static void musb_bulk_nak_timeout(struct musb
> >> *musb, struct musb_hw_ep *ep,
> >>>   /* clear nak timeout bit */
> >>>   rx_csr = musb_readw(epio, MUSB_RXCSR);
> >>>   rx_csr |= MUSB_RXCSR_H_WZC_BITS;
> >>> + rx_csr &= ~MUSB_RXCSR_H_REQPKT;
> >>> + musb_writew(epio, MUSB_RXCSR, rx_csr);
> >>>   rx_csr &= ~MUSB_RXCSR_DATAERROR;
> >>>   musb_writew(epio, MUSB_RXCSR, rx_csr);
> >>
> >> Can we not clear both in one write?
> >
> > Section 16.3.8.2.2.1.2 of the TRM says to clear REQPKT before
> DATAERR_NAKTIMEOUT.
> 
> Right, the MUSB programmer's guide also says that. Then a comment
> wouldn't hurt here.

I'll add that for v2, thanks.

Andrew

> > Andrew
> 
> MBR, Sergei

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

[PATCH v2] usb: musb: only restore devctl when session was set in backup

[Bin Liu]

If the session bit was not set in the backup of devctl register,
restoring devctl would clear the session bit. Therefor, only restore
devctl register when the session bit was set in the backup.

This solves the device enumeration failure in otg mode exposed by commit
56f487c (PM / Runtime: Update last_busy in rpm_resume).

Cc:  # v4.2+
Signed-off-by: Bin Liu 
---
v2: revise cc stable@ with version and drop Fixes tag since it is not accurate.

 drivers/usb/musb/musb_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 0286a39..f824336 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2418,7 +2418,8 @@ static void musb_restore_context(struct musb *musb)
musb_writew(musb_base, MUSB_INTRTXE, musb->intrtxe);
musb_writew(musb_base, MUSB_INTRRXE, musb->intrrxe);
musb_writeb(musb_base, MUSB_INTRUSBE, musb->context.intrusbe);
-   musb_writeb(musb_base, MUSB_DEVCTL, musb->context.devctl);
+   if (musb->context.devctl & MUSB_DEVCTL_SESSION)
+   musb_writeb(musb_base, MUSB_DEVCTL, musb->context.devctl);
 
for (i = 0; i < musb->config->num_eps; ++i) {
struct musb_hw_ep   *hw_ep;
-- 
1.9.1

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

Re: [OOPS] cppi41_dma_channel_program: Unable to handle kernel NULL pointer dereference

[Bin Liu]

Hi,

On Fri, May 20, 2016 at 04:32:06PM +0300, Matwey V. Kornilov wrote:
> 2016-05-20 16:19 GMT+03:00  :
> > Hello,
> >
> > I am running 4.6-rc3 on BealgeBone Black and when I try to interract
> > with pwc webcam attached to usb port the following kernel panic
> > happening.
> 
> Please note, that the same is happening with 4.6.0 release.

Please apply the following patch and reproduce the oops, I'd like to
check if the oops is caused by NULL of hw_ep->in_qh.

Regards,
-Bin.

---8<---
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index e499b86..3492c6e 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -489,8 +489,11 @@ static int cppi41_dma_channel_program(struct dma_channel 
*channel,
if (is_host_active(cppi41_channel->controller->musb)) {
if (cppi41_channel->is_tx)
hb_mult = cppi41_channel->hw_ep->out_qh->hb_mult;
-   else
+   else {
+   if (!cppi41_channel->hw_ep->in_qh)
+   dev_err(NULL, "->in_qh is NULL\n");
hb_mult = cppi41_channel->hw_ep->in_qh->hb_mult;
+   }
}
 
channel->status = MUSB_DMA_STATUS_BUSY;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[[PATCH v2] 2/2] usb: musb: host: don't start next rx urb if current one failed

[Bin Liu]

urb->status is set when endpoint csr RXSTALL, H_ERROR, DATAERROR or
INCOMPRX bit is set. Those bits mean a broken pipe, so don't start next
urb when any of these bits is set by checking urb->status.

To minimize the risk of regression, only do so for RX, until we have a
test case to understand the behavior of TX.

The patch fixes system freeze issue caused by repeatedly invoking RX ISR
while removing a usb uart device connected to a hub, in which case the
hub has no chance to report the disconnect event due to the kernel is
busy in processing the RX interrupt flooding.

Fix checkpatch complaint (qh != NULL) as while.

Reported-by: Max Uvarov 
Tested-by: Yegor Yefremov 
Signed-off-by: Bin Liu 
---
v2: fix checkpatch complaint (qh != NULL) as while.

 drivers/usb/musb/musb_host.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 05fb30b..f02361d 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -434,7 +434,13 @@ static void musb_advance_schedule(struct musb *musb, 
struct urb *urb,
}
}
 
-   if (qh != NULL && qh->is_ready) {
+   /*
+* The pipe must be broken if current urb->status is set, so don't
+* start next urb.
+* TODO: to minimize the risk of regression, only check urb->status
+* for RX, until we have a test case to understand the behavior of TX.
+*/
+   if ((!status || !is_in) && qh && qh->is_ready) {
dev_dbg(musb->controller, "... next ep%d %cX urb %p\n",
hw_ep->epnum, is_in ? 'R' : 'T', next_urb(qh));
musb_start_urb(musb, is_in, qh);
-- 
1.9.1

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

[[PATCH v2] 1/2] usb: musb: host: clear rxcsr error bit if set

[Bin Liu]

The MUSB Programming Guide states that the driver should clear RXCSR
bit2 when the controller sets the bit.

Signed-off-by: Bin Liu 
---
v2: no change.

 drivers/usb/musb/musb_host.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 9ad70e5..05fb30b 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1862,6 +1862,9 @@ void musb_host_rx(struct musb *musb, u8 epnum)
status = -EPROTO;
musb_writeb(epio, MUSB_RXINTERVAL, 0);
 
+   rx_csr &= ~MUSB_RXCSR_H_ERROR;
+   musb_writew(epio, MUSB_RXCSR, rx_csr);
+
} else if (rx_csr & MUSB_RXCSR_DATAERROR) {
 
if (USB_ENDPOINT_XFER_ISOC != qh->type) {
-- 
1.9.1

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

Re: [PATCH 0/2] musb_host: TX DMA cleanup after Tony's patches to the MUSB host driver

[Bin Liu]

Hi,

On Wed, May 04, 2016 at 01:49:06AM +0300, Sergei Shtylyov wrote:
> Hello.
> 
>Here's 2 patches against the 'next' branch of Felipe's 'usb.git' repo. Tony
> Lindgren's patches from the last year didn't seem complete, so trying to clean
> up the TX DMA patch...
> 
> [1/2] musb_host: move DMA engine check from musb_tx_dma_set_mode_cppi_tusb() 
> to its caller
> [2/2] musb_host: make musb_tx_dma_set_mode_*() *void*

I revised the subject prefix to "usb: musb: host: ..." for both patches.

Regards,
-Bin.

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

Re: [PATCH v2 1/2] musb: sunxi: Add set_mode platform function

[Bin Liu]

Hi,

On Mon, May 16, 2016 at 03:11:34PM -0500, Bin Liu wrote:
> On Sat, May 14, 2016 at 02:45:04PM +0200, Hans de Goede wrote:
> > Move the mode handling to the platform_set_mode callback.
> > 
> > Signed-off-by: Hans de Goede 
> 
> Signed-off-by: Bin Liu 

I revised the subject prefix to "usb: musb: sunxi: ..." for both
patches.

Regards,
-Bin.

> 
> Regards,
> -Bin.
--
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: [OOPS] cppi41_dma_channel_program: Unable to handle kernel NULL pointer dereference

[Matwey V. Kornilov]

I used kgdb. Is it ok?
channel appeared to be 0x0:

(gdb) continue
Continuing.
[Switching to Thread 315]

Breakpoint 1, cppi41_dma_channel_program (channel=0x0, packet_sz=960,
mode=0 '\000', dma_addr=2609136576, len=960)
at ../drivers/usb/musb/musb_cppi41.c:481
481 {
(gdb) bt
#0  cppi41_dma_channel_program (channel=0x0, packet_sz=960, mode=0
'\000', dma_addr=2609136576, len=960)
at ../drivers/usb/musb/musb_cppi41.c:481
#1  0xbf251570 in musb_rx_dma_iso_cppi41 (len=,
urb=, qh=, hw_ep=,
dma=) at ../drivers/usb/musb/musb_host.c:1569
#2  musb_rx_dma_inventra_cppi41 (len=, urb=, qh=, hw_ep=,
dma=) at ../drivers/usb/musb/musb_host.c:1652
#3  musb_host_rx (musb=0xdb3e0010, epnum=) at
../drivers/usb/musb/musb_host.c:1969
#4  0xbf249fb4 in musb_dma_completion (musb=,
epnum=, transmit=)
at ../drivers/usb/musb/musb_core.c:1693
#5  0xbf258b10 in cppi41_trans_done (cppi41_channel=0xd914c3bc) at
../drivers/usb/musb/musb_cppi41.c:148
#6  0xbf258c1c in cppi41_dma_callback (private_data=)
at ../drivers/usb/musb/musb_cppi41.c:286
#7  0xbf229f38 in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)


2016-05-20 23:10 GMT+03:00 Bin Liu :
> Hi,
>
> On Fri, May 20, 2016 at 04:32:06PM +0300, Matwey V. Kornilov wrote:
>> 2016-05-20 16:19 GMT+03:00  :
>> > Hello,
>> >
>> > I am running 4.6-rc3 on BealgeBone Black and when I try to interract
>> > with pwc webcam attached to usb port the following kernel panic
>> > happening.
>>
>> Please note, that the same is happening with 4.6.0 release.
>
> Please apply the following patch and reproduce the oops, I'd like to
> check if the oops is caused by NULL of hw_ep->in_qh.
>
> Regards,
> -Bin.
>
> ---8<---
> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
> index e499b86..3492c6e 100644
> --- a/drivers/usb/musb/musb_cppi41.c
> +++ b/drivers/usb/musb/musb_cppi41.c
> @@ -489,8 +489,11 @@ static int cppi41_dma_channel_program(struct dma_channel 
> *channel,
> if (is_host_active(cppi41_channel->controller->musb)) {
> if (cppi41_channel->is_tx)
> hb_mult = cppi41_channel->hw_ep->out_qh->hb_mult;
> -   else
> +   else {
> +   if (!cppi41_channel->hw_ep->in_qh)
> +   dev_err(NULL, "->in_qh is NULL\n");
> hb_mult = cppi41_channel->hw_ep->in_qh->hb_mult;
> +   }
> }
>
> channel->status = MUSB_DMA_STATUS_BUSY;
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
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: [OOPS] cppi41_dma_channel_program: Unable to handle kernel NULL pointer dereference

[Matwey V. Kornilov]

(gdb) frame 3
#3  musb_host_rx (musb=0xdb3e0010, epnum=) at
../drivers/usb/musb/musb_host.c:1969
1969done =
musb_rx_dma_inventra_cppi41(c, hw_ep, qh, urb, xfer_len);
(gdb) info locals
hw_ep = 0xdb3e0f48
c = 
epio = 
qh = 0xd9cb2000
xfer_len = 
mbase = 
iso_err = 
done = 
status = 
dma = 
__func__ = 
(gdb) print hw_ep
$21 = (struct musb_hw_ep *) 0xdb3e0f48
(gdb) print *hw_ep
$22 = {musb = 0xdb3e0010, fifo = 0xe0bf6c58, regs = 0xe0bf6c10, epnum
= 14 '\016', is_shared_fifo = true, tx_double_buffered = false,
  rx_double_buffered = false, max_packet_sz_tx = 1024,
max_packet_sz_rx = 1024, tx_channel = 0x0, rx_channel = 0xd914c3bc,
  in_qh = 0xd9cb2000, out_qh = 0xd9cb2000, rx_reinit = 0 '\000',
tx_reinit = 1 '\001', ep_in = {end_point = {driver_data = 0x0,
  name = 0x0, ops = 0x0, ep_list = {next = 0x0, prev = 0x0}, caps
= {type_control = 0, type_iso = 0, type_bulk = 0, type_int = 0,
dir_in = 0, dir_out = 0}, claimed = false, enabled = false,
maxpacket = 0, maxpacket_limit = 0, max_streams = 0, mult = 0,
  maxburst = 0, address = 0 '\000', desc = 0x0, comp_desc = 0x0},
name = '\000' , hw_ep = 0x0, musb = 0x0,
current_epnum = 0 '\000', type = 0 '\000', is_in = 0 '\000',
packet_sz = 0, desc = 0x0, dma = 0x0, req_list = {next = 0x0,
  prev = 0x0}, wedged = 0 '\000', busy = 0 '\000', hb_mult = 0
'\000'}, ep_out = {end_point = {driver_data = 0x0, name = 0x0,
  ops = 0x0, ep_list = {next = 0x0, prev = 0x0}, caps =
{type_control = 0, type_iso = 0, type_bulk = 0, type_int = 0, dir_in =
0,
dir_out = 0}, claimed = false, enabled = false, maxpacket = 0,
maxpacket_limit = 0, max_streams = 0, mult = 0, maxburst = 0,
  address = 0 '\000', desc = 0x0, comp_desc = 0x0}, name = '\000'
, hw_ep = 0x0, musb = 0x0,
current_epnum = 0 '\000', type = 0 '\000', is_in = 0 '\000',
packet_sz = 0, desc = 0x0, dma = 0x0, req_list = {next = 0x0,
  prev = 0x0}, wedged = 0 '\000', busy = 0 '\000', hb_mult = 0 '\000'}}

2016-05-20 23:55 GMT+03:00 Matwey V. Kornilov :
> I used kgdb. Is it ok?
> channel appeared to be 0x0:
>
> (gdb) continue
> Continuing.
> [Switching to Thread 315]
>
> Breakpoint 1, cppi41_dma_channel_program (channel=0x0, packet_sz=960,
> mode=0 '\000', dma_addr=2609136576, len=960)
> at ../drivers/usb/musb/musb_cppi41.c:481
> 481 {
> (gdb) bt
> #0  cppi41_dma_channel_program (channel=0x0, packet_sz=960, mode=0
> '\000', dma_addr=2609136576, len=960)
> at ../drivers/usb/musb/musb_cppi41.c:481
> #1  0xbf251570 in musb_rx_dma_iso_cppi41 (len=,
> urb=, qh=, hw_ep=,
> dma=) at ../drivers/usb/musb/musb_host.c:1569
> #2  musb_rx_dma_inventra_cppi41 (len=, urb= out>, qh=, hw_ep=,
> dma=) at ../drivers/usb/musb/musb_host.c:1652
> #3  musb_host_rx (musb=0xdb3e0010, epnum=) at
> ../drivers/usb/musb/musb_host.c:1969
> #4  0xbf249fb4 in musb_dma_completion (musb=,
> epnum=, transmit=)
> at ../drivers/usb/musb/musb_core.c:1693
> #5  0xbf258b10 in cppi41_trans_done (cppi41_channel=0xd914c3bc) at
> ../drivers/usb/musb/musb_cppi41.c:148
> #6  0xbf258c1c in cppi41_dma_callback (private_data=)
> at ../drivers/usb/musb/musb_cppi41.c:286
> #7  0xbf229f38 in ?? ()
> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
>
>
> 2016-05-20 23:10 GMT+03:00 Bin Liu :
>> Hi,
>>
>> On Fri, May 20, 2016 at 04:32:06PM +0300, Matwey V. Kornilov wrote:
>>> 2016-05-20 16:19 GMT+03:00  :
>>> > Hello,
>>> >
>>> > I am running 4.6-rc3 on BealgeBone Black and when I try to interract
>>> > with pwc webcam attached to usb port the following kernel panic
>>> > happening.
>>>
>>> Please note, that the same is happening with 4.6.0 release.
>>
>> Please apply the following patch and reproduce the oops, I'd like to
>> check if the oops is caused by NULL of hw_ep->in_qh.
>>
>> Regards,
>> -Bin.
>>
>> ---8<---
>> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
>> index e499b86..3492c6e 100644
>> --- a/drivers/usb/musb/musb_cppi41.c
>> +++ b/drivers/usb/musb/musb_cppi41.c
>> @@ -489,8 +489,11 @@ static int cppi41_dma_channel_program(struct 
>> dma_channel *channel,
>> if (is_host_active(cppi41_channel->controller->musb)) {
>> if (cppi41_channel->is_tx)
>> hb_mult = cppi41_channel->hw_ep->out_qh->hb_mult;
>> -   else
>> +   else {
>> +   if (!cppi41_channel->hw_ep->in_qh)
>> +   dev_err(NULL, "->in_qh is NULL\n");
>> hb_mult = cppi41_channel->hw_ep->in_qh->hb_mult;
>> +   }
>> }
>>
>> channel->status = MUSB_DMA_STATUS_BUSY;
>>
>
>
>
> --
> With best regards,
> Matwey V. Kornilov.
> Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
> 119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Un

Re: [OOPS] cppi41_dma_channel_program: Unable to handle kernel NULL pointer dereference

[Matwey V. Kornilov]

By the way, is it ok that function musb_rx_dma_iso_cppi41 uses
hw_ep->tx_channel? I would suppose that it should use rx_channel
instead.


2016-05-20 23:58 GMT+03:00 Matwey V. Kornilov :
> (gdb) frame 3
> #3  musb_host_rx (musb=0xdb3e0010, epnum=) at
> ../drivers/usb/musb/musb_host.c:1969
> 1969done =
> musb_rx_dma_inventra_cppi41(c, hw_ep, qh, urb, xfer_len);
> (gdb) info locals
> hw_ep = 0xdb3e0f48
> c = 
> epio = 
> qh = 0xd9cb2000
> xfer_len = 
> mbase = 
> iso_err = 
> done = 
> status = 
> dma = 
> __func__ =  address 0x3e8)>
> (gdb) print hw_ep
> $21 = (struct musb_hw_ep *) 0xdb3e0f48
> (gdb) print *hw_ep
> $22 = {musb = 0xdb3e0010, fifo = 0xe0bf6c58, regs = 0xe0bf6c10, epnum
> = 14 '\016', is_shared_fifo = true, tx_double_buffered = false,
>   rx_double_buffered = false, max_packet_sz_tx = 1024,
> max_packet_sz_rx = 1024, tx_channel = 0x0, rx_channel = 0xd914c3bc,
>   in_qh = 0xd9cb2000, out_qh = 0xd9cb2000, rx_reinit = 0 '\000',
> tx_reinit = 1 '\001', ep_in = {end_point = {driver_data = 0x0,
>   name = 0x0, ops = 0x0, ep_list = {next = 0x0, prev = 0x0}, caps
> = {type_control = 0, type_iso = 0, type_bulk = 0, type_int = 0,
> dir_in = 0, dir_out = 0}, claimed = false, enabled = false,
> maxpacket = 0, maxpacket_limit = 0, max_streams = 0, mult = 0,
>   maxburst = 0, address = 0 '\000', desc = 0x0, comp_desc = 0x0},
> name = '\000' , hw_ep = 0x0, musb = 0x0,
> current_epnum = 0 '\000', type = 0 '\000', is_in = 0 '\000',
> packet_sz = 0, desc = 0x0, dma = 0x0, req_list = {next = 0x0,
>   prev = 0x0}, wedged = 0 '\000', busy = 0 '\000', hb_mult = 0
> '\000'}, ep_out = {end_point = {driver_data = 0x0, name = 0x0,
>   ops = 0x0, ep_list = {next = 0x0, prev = 0x0}, caps =
> {type_control = 0, type_iso = 0, type_bulk = 0, type_int = 0, dir_in =
> 0,
> dir_out = 0}, claimed = false, enabled = false, maxpacket = 0,
> maxpacket_limit = 0, max_streams = 0, mult = 0, maxburst = 0,
>   address = 0 '\000', desc = 0x0, comp_desc = 0x0}, name = '\000'
> , hw_ep = 0x0, musb = 0x0,
> current_epnum = 0 '\000', type = 0 '\000', is_in = 0 '\000',
> packet_sz = 0, desc = 0x0, dma = 0x0, req_list = {next = 0x0,
>   prev = 0x0}, wedged = 0 '\000', busy = 0 '\000', hb_mult = 0 '\000'}}
>
> 2016-05-20 23:55 GMT+03:00 Matwey V. Kornilov :
>> I used kgdb. Is it ok?
>> channel appeared to be 0x0:
>>
>> (gdb) continue
>> Continuing.
>> [Switching to Thread 315]
>>
>> Breakpoint 1, cppi41_dma_channel_program (channel=0x0, packet_sz=960,
>> mode=0 '\000', dma_addr=2609136576, len=960)
>> at ../drivers/usb/musb/musb_cppi41.c:481
>> 481 {
>> (gdb) bt
>> #0  cppi41_dma_channel_program (channel=0x0, packet_sz=960, mode=0
>> '\000', dma_addr=2609136576, len=960)
>> at ../drivers/usb/musb/musb_cppi41.c:481
>> #1  0xbf251570 in musb_rx_dma_iso_cppi41 (len=,
>> urb=, qh=, hw_ep=,
>> dma=) at ../drivers/usb/musb/musb_host.c:1569
>> #2  musb_rx_dma_inventra_cppi41 (len=, urb=> out>, qh=, hw_ep=,
>> dma=) at ../drivers/usb/musb/musb_host.c:1652
>> #3  musb_host_rx (musb=0xdb3e0010, epnum=) at
>> ../drivers/usb/musb/musb_host.c:1969
>> #4  0xbf249fb4 in musb_dma_completion (musb=,
>> epnum=, transmit=)
>> at ../drivers/usb/musb/musb_core.c:1693
>> #5  0xbf258b10 in cppi41_trans_done (cppi41_channel=0xd914c3bc) at
>> ../drivers/usb/musb/musb_cppi41.c:148
>> #6  0xbf258c1c in cppi41_dma_callback (private_data=)
>> at ../drivers/usb/musb/musb_cppi41.c:286
>> #7  0xbf229f38 in ?? ()
>> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
>>
>>
>> 2016-05-20 23:10 GMT+03:00 Bin Liu :
>>> Hi,
>>>
>>> On Fri, May 20, 2016 at 04:32:06PM +0300, Matwey V. Kornilov wrote:
 2016-05-20 16:19 GMT+03:00  :
 > Hello,
 >
 > I am running 4.6-rc3 on BealgeBone Black and when I try to interract
 > with pwc webcam attached to usb port the following kernel panic
 > happening.

 Please note, that the same is happening with 4.6.0 release.
>>>
>>> Please apply the following patch and reproduce the oops, I'd like to
>>> check if the oops is caused by NULL of hw_ep->in_qh.
>>>
>>> Regards,
>>> -Bin.
>>>
>>> ---8<---
>>> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
>>> index e499b86..3492c6e 100644
>>> --- a/drivers/usb/musb/musb_cppi41.c
>>> +++ b/drivers/usb/musb/musb_cppi41.c
>>> @@ -489,8 +489,11 @@ static int cppi41_dma_channel_program(struct 
>>> dma_channel *channel,
>>> if (is_host_active(cppi41_channel->controller->musb)) {
>>> if (cppi41_channel->is_tx)
>>> hb_mult = cppi41_channel->hw_ep->out_qh->hb_mult;
>>> -   else
>>> +   else {
>>> +   if (!cppi41_channel->hw_ep->in_qh)
>>> +   dev_err(NULL, "->in_qh is NULL\n");
>>> hb_mult = cppi41_channel->hw_ep->in_qh->hb_mult;
>>> +   }
>>> }
>>>
>>> chan

Re: [OOPS] cppi41_dma_channel_program: Unable to handle kernel NULL pointer dereference

[Bin Liu]

Hi,

On Sat, May 21, 2016 at 12:05:06AM +0300, Matwey V. Kornilov wrote:
> By the way, is it ok that function musb_rx_dma_iso_cppi41 uses
> hw_ep->tx_channel? I would suppose that it should use rx_channel
> instead.

I just got here, and am wondering the same. But the question is why just
your case hit the problem. I will try to look at it more next week.

I had an impression the linux-usb@ has a discussion before about
rx/tx-channel messing up, will have to look it up.

Regards,
-Bin.
--
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: [OOPS] cppi41_dma_channel_program: Unable to handle kernel NULL pointer dereference

[Matwey V. Kornilov]

2016-05-21 0:12 GMT+03:00 Bin Liu :
> Hi,
>
> On Sat, May 21, 2016 at 12:05:06AM +0300, Matwey V. Kornilov wrote:
>> By the way, is it ok that function musb_rx_dma_iso_cppi41 uses
>> hw_ep->tx_channel? I would suppose that it should use rx_channel
>> instead.
>
> I just got here, and am wondering the same. But the question is why just
> your case hit the problem. I will try to look at it more next week.
>
> I had an impression the linux-usb@ has a discussion before about
> rx/tx-channel messing up, will have to look it up.
>

Thank you.
If you need additional info, I can use kgdb with this issue.

> Regards,
> -Bin.
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: host: ehci-msm: Conditionally call ehci suspend/resume

[Andy Gross]

This patch fixes a suspend/resume issue where the driver is blindly
calling ehci_suspend/resume functions when the ehci hasn't been setup.
This results in a crash during suspend/resume operations.

Signed-off-by: Andy Gross 
---
 drivers/usb/host/ehci-msm.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
index 3e226ef..9996a60 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -179,22 +179,32 @@ static int ehci_msm_remove(struct platform_device *pdev)
 static int ehci_msm_pm_suspend(struct device *dev)
 {
struct usb_hcd *hcd = dev_get_drvdata(dev);
+   struct ehci_hcd *ehci = hcd_to_ehci(hcd);
bool do_wakeup = device_may_wakeup(dev);
 
dev_dbg(dev, "ehci-msm PM suspend\n");
 
-   return ehci_suspend(hcd, do_wakeup);
+   /* Only call ehci_suspend if ehci_setup has been done */
+   if (ehci->sbrn)
+   return ehci_suspend(hcd, do_wakeup);
+
+   return 0;
 }
 
 static int ehci_msm_pm_resume(struct device *dev)
 {
struct usb_hcd *hcd = dev_get_drvdata(dev);
+   struct ehci_hcd *ehci = hcd_to_ehci(hcd);
 
dev_dbg(dev, "ehci-msm PM resume\n");
-   ehci_resume(hcd, false);
+
+   /* Only call ehci_resume if ehci_setup has been done */
+   if (ehci->sbrn)
+   ehci_resume(hcd, false);
 
return 0;
 }
+
 #else
 #define ehci_msm_pm_suspendNULL
 #define ehci_msm_pm_resume NULL
-- 
1.9.1

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

Re: [PATCH][v3.13.y-ckt][v3.19-ckt][v4.2.y-ckt]Revert "usb: hub: do not clear BOS field during reset device"

[Kamal Mostafa]

On Fri, May 20, 2016 at 11:56:39AM -0400, Joseph Salisbury wrote:
> Hello,
> 
> Please consider including commit 
> e5bdfd50d6f76077bf8441d130c606229e100d40 in the next v3.13.y-ckt,
> v3.19-ckt and v4.2.y-ckt releases.

Queued up for 3.19- and 4.2- ckt stable.  (3.13.y-ckt is no longer maintained).

Thanks Joseph!

 -Kamal


> It was included upstream as of
> v4.5-rc6.  This commit has been tested and resolves the following bug:
> http://pad.lv/1582864.
> 
> According to the commit message, this patch has already been included in
> the following -stable kernels:
> 4.5.0-rc4 (current git)
> 4.4.2
> 4.3.6 (currently in review)
> 4.1.18
> 3.18.27
> 3.14.61
> 
> 
> commit e5bdfd50d6f76077bf8441d130c606229e100d40
> Author: Greg Kroah-Hartman 
> Date:   Sat Feb 20 14:19:34 2016 -0800
> 
> Revert "usb: hub: do not clear BOS field during reset device"
> 
> 
> 
> 
> Sincerely,
> 
> Joseph Salisbury
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 00/13] usb: dwc2: Fix up gadget isochronous support

[John Youn]

The following patch series fixes up isochronous support for the dwc2
gadget. The existing isochronous support lacked a few features. Most
notably it did not properly sync up with the first packet and it
didn't handle the Incomplete ISO IN/OUT interrupts.

These patches have been sitting in our internal tree for a while and
tested mostly on a HAPS-based FPGA IP validation platform.

v2:
* Changed if/else to switch statement
* Changed frame_overrun to bool
* Downgraded a info message to dbg
* Added John Keeping's tested-by

Regards,
John


Vardan Mikayelyan (13):
  usb: dwc2: Add missing register field definitions
  usb: dwc2: gadget: Remove unnecessary line
  usb: dwc2: gadget: Remove unnecessary code
  usb: dwc2: gadget: Corrected field names
  usb: dwc2: gadget: Fix transfer stop programming for out endpoint
  usb: dwc2: gadget: Add dwc2_gadget_incr_frame_num()
  usb: dwc2: gadget: Corrected interval calculation
  usb: dwc2: gadget: Add dwc2_gadget_read_ep_interrupts function
  usb: dwc2: gadget: Add dwc2_gadget_start_next_request function
  usb: dwc2: gadget: Add OUTTKNEPDIS and NAKINTRPT handlers
  usb: dwc2: gadget: Add Incomplete ISO IN/OUT Interrupt handlers
  usb: dwc2: gadget: Add EP disabled interrupt handler
  usb: dwc2: gadget: Final fixes for BDMA ISOC

 drivers/usb/dwc2/core.h   |   8 +-
 drivers/usb/dwc2/gadget.c | 581 --
 drivers/usb/dwc2/hw.h |  14 ++
 3 files changed, 477 insertions(+), 126 deletions(-)

-- 
2.8.2

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

[PATCH v2 03/13] usb: dwc2: gadget: Remove unnecessary code

[John Youn]

From: Vardan Mikayelyan 

This chunk is not needed here. There is no functionality
depend on this, so if no-op, I think we do not need to have
this interrupt unmasked.

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 5521c70..e9595be 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -658,14 +658,6 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg,
}
 
/*
-* clear the INTknTXFEmpMsk when we start request, more as a aide
-* to debugging to see what is going on.
-*/
-   if (dir_in)
-   dwc2_writel(DIEPMSK_INTKNTXFEMPMSK,
-  hsotg->regs + DIEPINT(index));
-
-   /*
 * Note, trying to clear the NAK here causes problems with transmit
 * on the S3C6400 ending up with the TXFIFO becoming full.
 */
-- 
2.8.2

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

[PATCH v2 02/13] usb: dwc2: gadget: Remove unnecessary line

[John Youn]

From: Vardan Mikayelyan 

Removed "ctrl |= DXEPCTL_USBACTEP" from
dwc2_hsotg_start_req() function because this
step is done in dwc2_hsotg_ep_enable().

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 4c5e300..5521c70 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -632,7 +632,6 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg,
}
 
ctrl |= DXEPCTL_EPENA;  /* ensure ep enabled */
-   ctrl |= DXEPCTL_USBACTEP;
 
dev_dbg(hsotg->dev, "ep0 state:%d\n", hsotg->ep0_state);
 
-- 
2.8.2

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

[PATCH v2 04/13] usb: dwc2: gadget: Corrected field names

[John Youn]

From: Vardan Mikayelyan 

No-op change. Changed field names to prevent misunderstanding.

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e9595be..a62a2b1 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2037,20 +2037,20 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, 
unsigned int idx,
 
if (dir_in && !hs_ep->isochronous) {
/* not sure if this is important, but we'll clear it anyway */
-   if (ints & DIEPMSK_INTKNTXFEMPMSK) {
+   if (ints & DXEPINT_INTKNTXFEMP) {
dev_dbg(hsotg->dev, "%s: ep%d: INTknTXFEmpMsk\n",
__func__, idx);
}
 
/* this probably means something bad is happening */
-   if (ints & DIEPMSK_INTKNEPMISMSK) {
+   if (ints & DXEPINT_INTKNEPMIS) {
dev_warn(hsotg->dev, "%s: ep%d: INTknEP\n",
 __func__, idx);
}
 
/* FIFO has space or is empty (see GAHBCFG) */
if (hsotg->dedicated_fifos &&
-   ints & DIEPMSK_TXFIFOEMPTY) {
+   ints & DXEPINT_TXFEMP) {
dev_dbg(hsotg->dev, "%s: ep%d: TxFIFOEmpty\n",
__func__, idx);
if (!using_dma(hsotg))
-- 
2.8.2

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

[PATCH v2 01/13] usb: dwc2: Add missing register field definitions

[John Youn]

From: Vardan Mikayelyan 

Added register field definitions, register names are according
DWC-OTG databook.

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/hw.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index 281b57b..1126141 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -459,6 +459,9 @@
 #define DSTS_SUSPSTS   (1 << 0)
 
 #define DIEPMSKHSOTG_REG(0x810)
+#define DIEPMSK_NAKMSK (1 << 13)
+#define DIEPMSK_BNAININTRMSK   (1 << 9)
+#define DIEPMSK_TXFIFOUNDRNMSK (1 << 8)
 #define DIEPMSK_TXFIFOEMPTY(1 << 7)
 #define DIEPMSK_INEPNAKEFFMSK  (1 << 6)
 #define DIEPMSK_INTKNEPMISMSK  (1 << 5)
@@ -470,6 +473,7 @@
 
 #define DOEPMSKHSOTG_REG(0x814)
 #define DOEPMSK_BACK2BACKSETUP (1 << 6)
+#define DOEPMSK_STSPHSERCVDMSK (1 << 5)
 #define DOEPMSK_OUTTKNEPDISMSK (1 << 4)
 #define DOEPMSK_SETUPMSK   (1 << 3)
 #define DOEPMSK_AHBERRMSK  (1 << 2)
@@ -486,6 +490,7 @@
 #define DTKNQR2HSOTG_REG(0x824)
 #define DTKNQR3HSOTG_REG(0x830)
 #define DTKNQR4HSOTG_REG(0x834)
+#define DIEPEMPMSK HSOTG_REG(0x834)
 
 #define DVBUSDIS   HSOTG_REG(0x828)
 #define DVBUSPULSE HSOTG_REG(0x82C)
@@ -544,6 +549,14 @@
 #define DIEPINT(_a)HSOTG_REG(0x908 + ((_a) * 0x20))
 #define DOEPINT(_a)HSOTG_REG(0xB08 + ((_a) * 0x20))
 #define DXEPINT_SETUP_RCVD (1 << 15)
+#define DXEPINT_NYETINTRPT (1 << 14)
+#define DXEPINT_NAKINTRPT  (1 << 13)
+#define DXEPINT_BBLEERRINTRPT  (1 << 12)
+#define DXEPINT_PKTDRPSTS  (1 << 11)
+#define DXEPINT_BNAINTR(1 << 9)
+#define DXEPINT_TXFIFOUNDRN(1 << 8)
+#define DXEPINT_OUTPKTERR  (1 << 8)
+#define DXEPINT_TXFEMP (1 << 7)
 #define DXEPINT_INEPNAKEFF (1 << 6)
 #define DXEPINT_BACK2BACKSETUP (1 << 6)
 #define DXEPINT_INTKNEPMIS (1 << 5)
-- 
2.8.2

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

[PATCH v2 12/13] usb: dwc2: gadget: Add EP disabled interrupt handler

[John Youn]

From: Vardan Mikayelyan 

Reimplemented EP disabled interrupt handler and moved to
corresponding function.

This interrupt indicates that the endpoint has been disabled per
the application's request.

For IN endpoints flushes txfifo, in case of BULK clears DCTL_CGNPINNAK,
in case of ISOC completes current request.

For ISOC-OUT endpoints completes expired requests. If there is
remaining request starts it. This is the part of ISOC-OUT transfer
drop flow. When ISOC-OUT transfer expired we must disable ep to drop
ongoing transfer.

Tested-by: John Keeping 
Reviewed-by: Vahram Aharonyan 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 87 ++-
 1 file changed, 70 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index b9c0ab7..6264299 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2024,6 +2024,74 @@ static u32 dwc2_gadget_read_ep_interrupts(struct 
dwc2_hsotg *hsotg,
 }
 
 /**
+ * dwc2_gadget_handle_ep_disabled - handle DXEPINT_EPDISBLD
+ * @hs_ep: The endpoint on which interrupt is asserted.
+ *
+ * This interrupt indicates that the endpoint has been disabled per the
+ * application's request.
+ *
+ * For IN endpoints flushes txfifo, in case of BULK clears DCTL_CGNPINNAK,
+ * in case of ISOC completes current request.
+ *
+ * For ISOC-OUT endpoints completes expired requests. If there is remaining
+ * request starts it.
+ */
+static void dwc2_gadget_handle_ep_disabled(struct dwc2_hsotg_ep *hs_ep)
+{
+   struct dwc2_hsotg *hsotg = hs_ep->parent;
+   struct dwc2_hsotg_req *hs_req;
+   unsigned char idx = hs_ep->index;
+   int dir_in = hs_ep->dir_in;
+   u32 epctl_reg = dir_in ? DIEPCTL(idx) : DOEPCTL(idx);
+   int dctl = dwc2_readl(hsotg->regs + DCTL);
+
+   dev_dbg(hsotg->dev, "%s: EPDisbld\n", __func__);
+
+   if (dir_in) {
+   int epctl = dwc2_readl(hsotg->regs + epctl_reg);
+
+   dwc2_hsotg_txfifo_flush(hsotg, hs_ep->fifo_index);
+
+   if (hs_ep->isochronous) {
+   dwc2_hsotg_complete_in(hsotg, hs_ep);
+   return;
+   }
+
+   if ((epctl & DXEPCTL_STALL) && (epctl & DXEPCTL_EPTYPE_BULK)) {
+   int dctl = dwc2_readl(hsotg->regs + DCTL);
+
+   dctl |= DCTL_CGNPINNAK;
+   dwc2_writel(dctl, hsotg->regs + DCTL);
+   }
+   return;
+   }
+
+   if (dctl & DCTL_GOUTNAKSTS) {
+   dctl |= DCTL_CGOUTNAK;
+   dwc2_writel(dctl, hsotg->regs + DCTL);
+   }
+
+   if (!hs_ep->isochronous)
+   return;
+
+   if (list_empty(&hs_ep->queue)) {
+   dev_dbg(hsotg->dev, "%s: complete_ep 0x%p, ep->queue empty!\n",
+   __func__, hs_ep);
+   return;
+   }
+
+   do {
+   hs_req = get_ep_head(hs_ep);
+   if (hs_req)
+   dwc2_hsotg_complete_request(hsotg, hs_ep, hs_req,
+   -ENODATA);
+   dwc2_gadget_incr_frame_num(hs_ep);
+   } while (dwc2_gadget_target_frame_elapsed(hs_ep));
+
+   dwc2_gadget_start_next_request(hs_ep);
+}
+
+/**
  * dwc2_gadget_handle_out_token_ep_disabled - handle DXEPINT_OUTTKNEPDIS
  * @hs_ep: The endpoint on which interrupt is asserted.
  *
@@ -2177,23 +2245,8 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, 
unsigned int idx,
}
}
 
-   if (ints & DXEPINT_EPDISBLD) {
-   dev_dbg(hsotg->dev, "%s: EPDisbld\n", __func__);
-
-   if (dir_in) {
-   int epctl = dwc2_readl(hsotg->regs + epctl_reg);
-
-   dwc2_hsotg_txfifo_flush(hsotg, hs_ep->fifo_index);
-
-   if ((epctl & DXEPCTL_STALL) &&
-   (epctl & DXEPCTL_EPTYPE_BULK)) {
-   int dctl = dwc2_readl(hsotg->regs + DCTL);
-
-   dctl |= DCTL_CGNPINNAK;
-   dwc2_writel(dctl, hsotg->regs + DCTL);
-   }
-   }
-   }
+   if (ints & DXEPINT_EPDISBLD)
+   dwc2_gadget_handle_ep_disabled(hs_ep);
 
if (ints & DXEPINT_OUTTKNEPDIS)
dwc2_gadget_handle_out_token_ep_disabled(hs_ep);
-- 
2.8.2

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

[PATCH v2 13/13] usb: dwc2: gadget: Final fixes for BDMA ISOC

[John Youn]

From: Vardan Mikayelyan 

Done fixes and tested hsotg gadget's BDMA mode. Tested Control,
Bulk, Isoc, Inter transfers. Added code for isoc transfers,
removed unusable code, done minor fixes. Affected functions
and IRQ handlers:
- dwc2_hsotg_start_req(),
- dwc2_hsotg_ep_enable(),
- dwc2_hsotg_ep_queue(),
- dwc2_hsotg_handle_outdone(),
- GINTSTS_GOUTNAKEFF handler,

Removed 'has_correct_parity' flag from 'dwc2_hsotg_ep' struct.
Before this patch series, to set the data pid the DWC2 gadget
driver was toggling the even/odd until it match, then were
leaving it set. But now I have added mechanism to set pid and
excluded all code where this flag was set.

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   |  1 -
 drivers/usb/dwc2/gadget.c | 97 ++-
 drivers/usb/dwc2/hw.h |  1 +
 3 files changed, 71 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index f2e7de7..1902c54 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -188,7 +188,6 @@ struct dwc2_hsotg_ep {
unsigned intperiodic:1;
unsigned intisochronous:1;
unsigned intsend_zlp:1;
-   unsigned inthas_correct_parity:1;
unsigned inttarget_frame;
 #define TARGET_FRAME_INITIAL   0x
boolframe_overrun;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 6264299..0628649 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -667,6 +667,16 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg,
__func__, &ureq->dma, dma_reg);
}
 
+   if (hs_ep->isochronous && hs_ep->interval == 1) {
+   hs_ep->target_frame = dwc2_hsotg_read_frameno(hsotg);
+   dwc2_gadget_incr_frame_num(hs_ep);
+
+   if (hs_ep->target_frame & 0x1)
+   ctrl |= DXEPCTL_SETODDFR;
+   else
+   ctrl |= DXEPCTL_SETEVENFR;
+   }
+
ctrl |= DXEPCTL_EPENA;  /* ensure ep enabled */
 
dev_dbg(hsotg->dev, "ep0 state:%d\n", hsotg->ep0_state);
@@ -863,9 +873,18 @@ static int dwc2_hsotg_ep_queue(struct usb_ep *ep, struct 
usb_request *req,
first = list_empty(&hs_ep->queue);
list_add_tail(&hs_req->queue, &hs_ep->queue);
 
-   if (first)
-   dwc2_hsotg_start_req(hs, hs_ep, hs_req, false);
+   if (first) {
+   if (!hs_ep->isochronous) {
+   dwc2_hsotg_start_req(hs, hs_ep, hs_req, false);
+   return 0;
+   }
 
+   while (dwc2_gadget_target_frame_elapsed(hs_ep))
+   dwc2_gadget_incr_frame_num(hs_ep);
+
+   if (hs_ep->target_frame != TARGET_FRAME_INITIAL)
+   dwc2_hsotg_start_req(hs, hs_ep, hs_req, false);
+   }
return 0;
 }
 
@@ -1673,9 +1692,10 @@ static void dwc2_hsotg_handle_outdone(struct dwc2_hsotg 
*hsotg, int epnum)
 * adjust the ISOC parity here.
 */
if (!using_dma(hsotg)) {
-   hs_ep->has_correct_parity = 1;
if (hs_ep->isochronous && hs_ep->interval == 1)
dwc2_hsotg_change_ep_iso_parity(hsotg, DOEPCTL(epnum));
+   else if (hs_ep->isochronous && hs_ep->interval > 1)
+   dwc2_gadget_incr_frame_num(hs_ep);
}
 
dwc2_hsotg_complete_request(hsotg, hs_ep, hs_req, result);
@@ -2216,11 +2236,10 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, 
unsigned int idx,
if (idx == 0 && (ints & (DXEPINT_SETUP | DXEPINT_SETUP_RCVD)))
ints &= ~DXEPINT_XFERCOMPL;
 
-   if (ints & DXEPINT_XFERCOMPL) {
-   hs_ep->has_correct_parity = 1;
-   if (hs_ep->isochronous && hs_ep->interval == 1)
-   dwc2_hsotg_change_ep_iso_parity(hsotg, epctl_reg);
+   if (ints & DXEPINT_STSPHSERCVD)
+   dev_dbg(hsotg->dev, "%s: StsPhseRcvd asserted\n", __func__);
 
+   if (ints & DXEPINT_XFERCOMPL) {
dev_dbg(hsotg->dev,
"%s: XferCompl: DxEPCTL=0x%08x, DXEPTSIZ=%08x\n",
__func__, dwc2_readl(hsotg->regs + epctl_reg),
@@ -2231,7 +2250,12 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, 
unsigned int idx,
 * at completing IN requests here
 */
if (dir_in) {
+   if (hs_ep->isochronous && hs_ep->interval > 1)
+   dwc2_gadget_incr_frame_num(hs_ep);
+
dwc2_hsotg_complete_in(hsotg, hs_ep);
+   if (ints & DXEPINT_NAKINTRPT)
+   ints &= ~DXEPINT_NAKINTRPT;
 
if (idx == 0 && !hs_ep->req)
dwc2_hsotg_

[PATCH v2 11/13] usb: dwc2: gadget: Add Incomplete ISO IN/OUT Interrupt handlers

[John Youn]

From: Vardan Mikayelyan 

Incomplete ISO IN interrupt indicates one of the following conditions
occurred while transmitting an ISOC transaction.
- Corrupted IN Token for ISOC EP.
- Packet not complete in FIFO.

Incomplete ISO OUT indicates that there is at least one isochronous OUT
endpoint on which the transfer is not completed in the current
microframe.

The following actions will be taken:

In case of EP-IN
- Determine the EP
- Disable EP directly from this handler; when "Endpoint Disabled"
  interrupt is received flush FIFO

In case of EP-OUT
- Determine the EP
- If target frame elapsed set DCTL_SGOUTNAK, unmask GOUTNAKEFF and
  proceed as described in section 7.5.1 of DWC-HSOTG Programming Guide

Also added dwc2_gadget_target_frame_elapsed() helper function which
will be used in Incomplete ISO IN/OUT Interrupt handlers.

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 173 +-
 1 file changed, 124 insertions(+), 49 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e8d4c78..b9c0ab7 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -523,6 +523,23 @@ static unsigned get_ep_limit(struct dwc2_hsotg_ep *hs_ep)
 }
 
 /**
+* dwc2_hsotg_read_frameno - read current frame number
+* @hsotg: The device instance
+*
+* Return the current frame number
+*/
+static u32 dwc2_hsotg_read_frameno(struct dwc2_hsotg *hsotg)
+{
+   u32 dsts;
+
+   dsts = dwc2_readl(hsotg->regs + DSTS);
+   dsts &= DSTS_SOFFN_MASK;
+   dsts >>= DSTS_SOFFN_SHIFT;
+
+   return dsts;
+}
+
+/**
  * dwc2_hsotg_start_req - start a USB request from an endpoint's queue
  * @hsotg: The controller state.
  * @hs_ep: The endpoint to process a request for
@@ -783,6 +800,30 @@ static void 
dwc2_hsotg_handle_unaligned_buf_complete(struct dwc2_hsotg *hsotg,
hs_req->saved_req_buf = NULL;
 }
 
+/**
+ * dwc2_gadget_target_frame_elapsed - Checks target frame
+ * @hs_ep: The driver endpoint to check
+ *
+ * Returns 1 if targeted frame elapsed. If returned 1 then we need to drop
+ * corresponding transfer.
+ */
+static bool dwc2_gadget_target_frame_elapsed(struct dwc2_hsotg_ep *hs_ep)
+{
+   struct dwc2_hsotg *hsotg = hs_ep->parent;
+   u32 target_frame = hs_ep->target_frame;
+   u32 current_frame = dwc2_hsotg_read_frameno(hsotg);
+   bool frame_overrun = hs_ep->frame_overrun;
+
+   if (!frame_overrun && current_frame >= target_frame)
+   return true;
+
+   if (frame_overrun && current_frame >= target_frame &&
+   ((current_frame - target_frame) < DSTS_SOFFN_LIMIT / 2))
+   return true;
+
+   return false;
+}
+
 static int dwc2_hsotg_ep_queue(struct usb_ep *ep, struct usb_request *req,
  gfp_t gfp_flags)
 {
@@ -1641,23 +1682,6 @@ static void dwc2_hsotg_handle_outdone(struct dwc2_hsotg 
*hsotg, int epnum)
 }
 
 /**
- * dwc2_hsotg_read_frameno - read current frame number
- * @hsotg: The device instance
- *
- * Return the current frame number
- */
-static u32 dwc2_hsotg_read_frameno(struct dwc2_hsotg *hsotg)
-{
-   u32 dsts;
-
-   dsts = dwc2_readl(hsotg->regs + DSTS);
-   dsts &= DSTS_SOFFN_MASK;
-   dsts >>= DSTS_SOFFN_SHIFT;
-
-   return dsts;
-}
-
-/**
  * dwc2_hsotg_handle_rx - RX FIFO has data
  * @hsotg: The device instance
  *
@@ -2571,6 +2595,85 @@ void dwc2_hsotg_core_connect(struct dwc2_hsotg *hsotg)
 }
 
 /**
+ * dwc2_gadget_handle_incomplete_isoc_in - handle incomplete ISO IN Interrupt.
+ * @hsotg: The device state:
+ *
+ * This interrupt indicates one of the following conditions occurred while
+ * transmitting an ISOC transaction.
+ * - Corrupted IN Token for ISOC EP.
+ * - Packet not complete in FIFO.
+ *
+ * The following actions will be taken:
+ * - Determine the EP
+ * - Disable EP; when 'Endpoint Disabled' interrupt is received Flush FIFO
+ */
+static void dwc2_gadget_handle_incomplete_isoc_in(struct dwc2_hsotg *hsotg)
+{
+   struct dwc2_hsotg_ep *hs_ep;
+   u32 epctrl;
+   u32 idx;
+
+   dev_dbg(hsotg->dev, "Incomplete isoc in interrupt received:\n");
+
+   for (idx = 1; idx <= hsotg->num_of_eps; idx++) {
+   hs_ep = hsotg->eps_in[idx];
+   epctrl = dwc2_readl(hsotg->regs + DIEPCTL(idx));
+   if ((epctrl & DXEPCTL_EPENA) && hs_ep->isochronous &&
+   dwc2_gadget_target_frame_elapsed(hs_ep)) {
+   epctrl |= DXEPCTL_SNAK;
+   epctrl |= DXEPCTL_EPDIS;
+   dwc2_writel(epctrl, hsotg->regs + DIEPCTL(idx));
+   }
+   }
+
+   /* Clear interrupt */
+   dwc2_writel(GINTSTS_INCOMPL_SOIN, hsotg->regs + GINTSTS);
+}
+
+/**
+ * dwc2_gadget_handle_incomplete_isoc_out - handle incomplete ISO OUT Interrupt
+ * @hsotg: The device state:
+ *
+ * This interrupt indicates one of the following conditions occurred 

[PATCH v2 08/13] usb: dwc2: gadget: Add dwc2_gadget_read_ep_interrupts function

[John Youn]

From: Vardan Mikayelyan 

Reads and returns interrupts for given endpoint, by masking epint_reg
with corresponding mask.

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index dfcb64d..5817646 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1947,6 +1947,34 @@ static void dwc2_hsotg_complete_in(struct dwc2_hsotg 
*hsotg,
 }
 
 /**
+ * dwc2_gadget_read_ep_interrupts - reads interrupts for given ep
+ * @hsotg: The device state.
+ * @idx: Index of ep.
+ * @dir_in: Endpoint direction 1-in 0-out.
+ *
+ * Reads for endpoint with given index and direction, by masking
+ * epint_reg with coresponding mask.
+ */
+static u32 dwc2_gadget_read_ep_interrupts(struct dwc2_hsotg *hsotg,
+ unsigned int idx, int dir_in)
+{
+   u32 epmsk_reg = dir_in ? DIEPMSK : DOEPMSK;
+   u32 epint_reg = dir_in ? DIEPINT(idx) : DOEPINT(idx);
+   u32 ints;
+   u32 mask;
+   u32 diepempmsk;
+
+   mask = dwc2_readl(hsotg->regs + epmsk_reg);
+   diepempmsk = dwc2_readl(hsotg->regs + DIEPEMPMSK);
+   mask |= ((diepempmsk >> idx) & 0x1) ? DIEPMSK_TXFIFOEMPTY : 0;
+   mask |= DXEPINT_SETUP_RCVD;
+
+   ints = dwc2_readl(hsotg->regs + epint_reg);
+   ints &= mask;
+   return ints;
+}
+
+/**
  * dwc2_hsotg_epint - handle an in/out endpoint interrupt
  * @hsotg: The driver state
  * @idx: The index for the endpoint (0..15)
@@ -1964,7 +1992,7 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, 
unsigned int idx,
u32 ints;
u32 ctrl;
 
-   ints = dwc2_readl(hsotg->regs + epint_reg);
+   ints = dwc2_gadget_read_ep_interrupts(hsotg, idx, dir_in);
ctrl = dwc2_readl(hsotg->regs + epctl_reg);
 
/* Clear endpoint interrupts */
-- 
2.8.2

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

[PATCH v2 07/13] usb: dwc2: gadget: Corrected interval calculation

[John Youn]

From: Vardan Mikayelyan 

Calculate the interval according to the USB 2.0 specification section
9.6.6.

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   |  2 +-
 drivers/usb/dwc2/gadget.c | 15 +++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 22fa1f5..f2e7de7 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -139,7 +139,7 @@ struct dwc2_hsotg_req;
  *  means that it is sending data to the Host.
  * @index: The index for the endpoint registers.
  * @mc: Multi Count - number of transactions per microframe
- * @interval - Interval for periodic endpoints
+ * @interval - Interval for periodic endpoints, in frames or microframes.
  * @name: The name array passed to the USB core.
  * @halted: Set if the endpoint has been halted.
  * @periodic: Set if this is a periodic ep, such as Interrupt
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7f15bc0..dfcb64d 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2693,16 +2693,13 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
hs_ep->periodic = 0;
hs_ep->halted = 0;
hs_ep->interval = desc->bInterval;
-   hs_ep->has_correct_parity = 0;
-
-   if (hs_ep->interval > 1 && hs_ep->mc > 1)
-   dev_err(hsotg->dev, "MC > 1 when interval is not 1\n");
 
switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
case USB_ENDPOINT_XFER_ISOC:
epctrl |= DXEPCTL_EPTYPE_ISO;
epctrl |= DXEPCTL_SETEVENFR;
hs_ep->isochronous = 1;
+   hs_ep->interval = 1 << (desc->bInterval - 1);
if (dir_in)
hs_ep->periodic = 1;
break;
@@ -2715,6 +2712,16 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
if (dir_in)
hs_ep->periodic = 1;
 
+   switch (hsotg->gadget.speed) {
+   case USB_SPEED_LOW:
+   case USB_SPEED_FULL:
+   hs_ep->interval = desc->bInterval;
+   break;
+   case USB_SPEED_HIGH:
+   hs_ep->interval = 1 << (desc->bInterval - 1);
+   break;
+   }
+
epctrl |= DXEPCTL_EPTYPE_INTERRUPT;
break;
 
-- 
2.8.2

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

[PATCH v2 10/13] usb: dwc2: gadget: Add OUTTKNEPDIS and NAKINTRPT handlers

[John Youn]

From: Vardan Mikayelyan 

NAKINTRPT interrupt is starting point for isoc-in transfer,
synchronization done with first in token received from host,
core asserts this interrupt when responds with 0 length data
to in token, received from host.

The first IN token is asynchronous for device - device does not
know when first one token will arrive from host. On first token
arrival HW generates 2 interrupts: 'in token received while FIFO
empty' and 'NAK'. NAK interrupt for ISOC in means that token has
arrived and ZLP was sent in response to that as there was no data
in FIFO. SW is basing on this interrupt to obtain frame in which
token has come and then based on the interval calculates next
frame for transfer.

OUTTKNEPDIS interrupt is starting point for isoc-out transfer,
synchronization done with first out token received from host
while corresponding ep is disabled.

For OUTs the reason is same - device does not know initial frame
in which out token will come. For this HW generates OUTTKNEPDIS
- out token is received while EP is disabled. Upon getting this
interrupt SW starts calculation for next transfer frame.

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 94 +++
 1 file changed, 94 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 1286981..e8d4c78 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2000,6 +2000,94 @@ static u32 dwc2_gadget_read_ep_interrupts(struct 
dwc2_hsotg *hsotg,
 }
 
 /**
+ * dwc2_gadget_handle_out_token_ep_disabled - handle DXEPINT_OUTTKNEPDIS
+ * @hs_ep: The endpoint on which interrupt is asserted.
+ *
+ * This is starting point for ISOC-OUT transfer, synchronization done with
+ * first out token received from host while corresponding EP is disabled.
+ *
+ * Device does not know initial frame in which out token will come. For this
+ * HW generates OUTTKNEPDIS - out token is received while EP is disabled. Upon
+ * getting this interrupt SW starts calculation for next transfer frame.
+ */
+static void dwc2_gadget_handle_out_token_ep_disabled(struct dwc2_hsotg_ep *ep)
+{
+   struct dwc2_hsotg *hsotg = ep->parent;
+   int dir_in = ep->dir_in;
+   u32 doepmsk;
+
+   if (dir_in || !ep->isochronous)
+   return;
+
+   dwc2_hsotg_complete_request(hsotg, ep, get_ep_head(ep), -ENODATA);
+
+   if (ep->interval > 1 &&
+   ep->target_frame == TARGET_FRAME_INITIAL) {
+   u32 dsts;
+   u32 ctrl;
+
+   dsts = dwc2_readl(hsotg->regs + DSTS);
+   ep->target_frame = dwc2_hsotg_read_frameno(hsotg);
+   dwc2_gadget_incr_frame_num(ep);
+
+   ctrl = dwc2_readl(hsotg->regs + DOEPCTL(ep->index));
+   if (ep->target_frame & 0x1)
+   ctrl |= DXEPCTL_SETODDFR;
+   else
+   ctrl |= DXEPCTL_SETEVENFR;
+
+   dwc2_writel(ctrl, hsotg->regs + DOEPCTL(ep->index));
+   }
+
+   dwc2_gadget_start_next_request(ep);
+   doepmsk = dwc2_readl(hsotg->regs + DOEPMSK);
+   doepmsk &= ~DOEPMSK_OUTTKNEPDISMSK;
+   dwc2_writel(doepmsk, hsotg->regs + DOEPMSK);
+}
+
+/**
+* dwc2_gadget_handle_nak - handle NAK interrupt
+* @hs_ep: The endpoint on which interrupt is asserted.
+*
+* This is starting point for ISOC-IN transfer, synchronization done with
+* first IN token received from host while corresponding EP is disabled.
+*
+* Device does not know when first one token will arrive from host. On first
+* token arrival HW generates 2 interrupts: 'in token received while FIFO empty'
+* and 'NAK'. NAK interrupt for ISOC-IN means that token has arrived and ZLP was
+* sent in response to that as there was no data in FIFO. SW is basing on this
+* interrupt to obtain frame in which token has come and then based on the
+* interval calculates next frame for transfer.
+*/
+static void dwc2_gadget_handle_nak(struct dwc2_hsotg_ep *hs_ep)
+{
+   struct dwc2_hsotg *hsotg = hs_ep->parent;
+   int dir_in = hs_ep->dir_in;
+
+   if (!dir_in || !hs_ep->isochronous)
+   return;
+
+   if (hs_ep->target_frame == TARGET_FRAME_INITIAL) {
+   hs_ep->target_frame = dwc2_hsotg_read_frameno(hsotg);
+   if (hs_ep->interval > 1) {
+   u32 ctrl = dwc2_readl(hsotg->regs +
+ DIEPCTL(hs_ep->index));
+   if (hs_ep->target_frame & 0x1)
+   ctrl |= DXEPCTL_SETODDFR;
+   else
+   ctrl |= DXEPCTL_SETEVENFR;
+
+   dwc2_writel(ctrl, hsotg->regs + DIEPCTL(hs_ep->index));
+   }
+
+   dwc2_hsotg_complete_request(hsotg, hs_ep,
+   get_ep_head(hs_ep), 0);
+   }
+
+   dwc2_gadget_incr_frame_num(hs_ep);

[PATCH v2 05/13] usb: dwc2: gadget: Fix transfer stop programming for out endpoint

[John Youn]

From: Vardan Mikayelyan 

According DWC-OTG databook, "GOUTNakEff" is read only and can be
cleared only by "DCTL.CGOUTNak", but here we do not need to clear
it because DWC-OTG programming guide says that before disabling
any OUT endpoint, the application must enable Global OUT NAK mode,
so if this mode is enabled we can continue without this step.

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index a62a2b1..22d4f5b 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2866,10 +2866,8 @@ static void dwc2_hsotg_ep_stop_xfr(struct dwc2_hsotg 
*hsotg,
dev_warn(hsotg->dev,
"%s: timeout DIEPINT.NAKEFF\n", __func__);
} else {
-   /* Clear any pending nak effect interrupt */
-   dwc2_writel(GINTSTS_GOUTNAKEFF, hsotg->regs + GINTSTS);
-
-   __orr32(hsotg->regs + DCTL, DCTL_SGOUTNAK);
+   if (!(dwc2_readl(hsotg->regs + GINTSTS) & GINTSTS_GOUTNAKEFF))
+   __orr32(hsotg->regs + DCTL, DCTL_SGOUTNAK);
 
/* Wait for global nak to take effect */
if (dwc2_hsotg_wait_bit_set(hsotg, GINTSTS,
-- 
2.8.2

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

[PATCH v2 09/13] usb: dwc2: gadget: Add dwc2_gadget_start_next_request function

[John Youn]

From: Vardan Mikayelyan 

Replaced repeating code with function call.

Starts next request from ep queue.
If queue is empty and ep is isoc
-In case of OUT-EP unmasks OUTTKNEPDIS.

OUTTKNEPDIS is masked in it's handler, so we need to unmask it here
to be able to do resynchronization.

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 51 +++
 1 file changed, 38 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 5817646..1286981 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1045,6 +1045,42 @@ static struct dwc2_hsotg_req *get_ep_head(struct 
dwc2_hsotg_ep *hs_ep)
 }
 
 /**
+ * dwc2_gadget_start_next_request - Starts next request from ep queue
+ * @hs_ep: Endpoint structure
+ *
+ * If queue is empty and EP is ISOC-OUT - unmasks OUTTKNEPDIS which is masked
+ * in its handler. Hence we need to unmask it here to be able to do
+ * resynchronization.
+ */
+static void dwc2_gadget_start_next_request(struct dwc2_hsotg_ep *hs_ep)
+{
+   u32 mask;
+   struct dwc2_hsotg *hsotg = hs_ep->parent;
+   int dir_in = hs_ep->dir_in;
+   struct dwc2_hsotg_req *hs_req;
+   u32 epmsk_reg = dir_in ? DIEPMSK : DOEPMSK;
+
+   if (!list_empty(&hs_ep->queue)) {
+   hs_req = get_ep_head(hs_ep);
+   dwc2_hsotg_start_req(hsotg, hs_ep, hs_req, false);
+   return;
+   }
+   if (!hs_ep->isochronous)
+   return;
+
+   if (dir_in) {
+   dev_dbg(hsotg->dev, "%s: No more ISOC-IN requests\n",
+   __func__);
+   } else {
+   dev_dbg(hsotg->dev, "%s: No more ISOC-OUT requests\n",
+   __func__);
+   mask = dwc2_readl(hsotg->regs + epmsk_reg);
+   mask |= DOEPMSK_OUTTKNEPDISMSK;
+   dwc2_writel(mask, hsotg->regs + epmsk_reg);
+   }
+}
+
+/**
  * dwc2_hsotg_process_req_feature - process request {SET,CLEAR}_FEATURE
  * @hsotg: The device state
  * @ctrl: USB control request
@@ -1054,7 +1090,6 @@ static int dwc2_hsotg_process_req_feature(struct 
dwc2_hsotg *hsotg,
 {
struct dwc2_hsotg_ep *ep0 = hsotg->eps_out[0];
struct dwc2_hsotg_req *hs_req;
-   bool restart;
bool set = (ctrl->bRequest == USB_REQ_SET_FEATURE);
struct dwc2_hsotg_ep *ep;
int ret;
@@ -1137,12 +1172,7 @@ static int dwc2_hsotg_process_req_feature(struct 
dwc2_hsotg *hsotg,
 
/* If we have pending request, then start it */
if (!ep->req) {
-   restart = !list_empty(&ep->queue);
-   if (restart) {
-   hs_req = get_ep_head(ep);
-   dwc2_hsotg_start_req(hsotg, ep,
-   hs_req, false);
-   }
+   dwc2_gadget_start_next_request(ep);
}
}
 
@@ -1383,7 +1413,6 @@ static void dwc2_hsotg_complete_request(struct dwc2_hsotg 
*hsotg,
   struct dwc2_hsotg_req *hs_req,
   int result)
 {
-   bool restart;
 
if (!hs_req) {
dev_dbg(hsotg->dev, "%s: nothing to complete?\n", __func__);
@@ -1427,11 +1456,7 @@ static void dwc2_hsotg_complete_request(struct 
dwc2_hsotg *hsotg,
 */
 
if (!hs_ep->req && result >= 0) {
-   restart = !list_empty(&hs_ep->queue);
-   if (restart) {
-   hs_req = get_ep_head(hs_ep);
-   dwc2_hsotg_start_req(hsotg, hs_ep, hs_req, false);
-   }
+   dwc2_gadget_start_next_request(hs_ep);
}
 }
 
-- 
2.8.2

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

[PATCH v2 06/13] usb: dwc2: gadget: Add dwc2_gadget_incr_frame_num()

[John Youn]

From: Vardan Mikayelyan 

Increases and checks targeted frame number of current ep
if overrun happened, sets flag and masks with DSTS_SOFFN_LIMIT

Added following fields to struct dwc2_hsotg_ep
-target_frame: Targeted frame num to setup next ISOC transfer
-frame_overrun: Indicates SOF number overrun in DSTS

Tested-by: John Keeping 
Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   |  5 +
 drivers/usb/dwc2/gadget.c | 19 +++
 2 files changed, 24 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 3c58d63..22fa1f5 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -150,6 +150,8 @@ struct dwc2_hsotg_req;
  * @fifo_load: The amount of data loaded into the FIFO (periodic IN)
  * @last_load: The offset of data for the last start of request.
  * @size_loaded: The last loaded size for DxEPTSIZE for periodic IN
+ * @target_frame: Targeted frame num to setup next ISOC transfer
+ * @frame_overrun: Indicates SOF number overrun in DSTS
  *
  * This is the driver's state for each registered enpoint, allowing it
  * to keep track of transactions that need doing. Each endpoint has a
@@ -187,6 +189,9 @@ struct dwc2_hsotg_ep {
unsigned intisochronous:1;
unsigned intsend_zlp:1;
unsigned inthas_correct_parity:1;
+   unsigned inttarget_frame;
+#define TARGET_FRAME_INITIAL   0x
+   boolframe_overrun;
 
charname[10];
 };
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 22d4f5b..7f15bc0 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -97,6 +97,25 @@ static inline bool using_dma(struct dwc2_hsotg *hsotg)
 }
 
 /**
+ * dwc2_gadget_incr_frame_num - Increments the targeted frame number.
+ * @hs_ep: The endpoint
+ * @increment: The value to increment by
+ *
+ * This function will also check if the frame number overruns DSTS_SOFFN_LIMIT.
+ * If an overrun occurs it will wrap the value and set the frame_overrun flag.
+ */
+static inline void dwc2_gadget_incr_frame_num(struct dwc2_hsotg_ep *hs_ep)
+{
+   hs_ep->target_frame += hs_ep->interval;
+   if (hs_ep->target_frame > DSTS_SOFFN_LIMIT) {
+   hs_ep->frame_overrun = 1;
+   hs_ep->target_frame &= DSTS_SOFFN_LIMIT;
+   } else {
+   hs_ep->frame_overrun = 0;
+   }
+}
+
+/**
  * dwc2_hsotg_en_gsint - enable one or more of the general interrupt
  * @hsotg: The device state
  * @ints: A bitmask of the interrupts to enable
-- 
2.8.2

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

Re: [PATCH net,stable v2] net: cdc_ncm: update datagram size after changing mtu

[David Miller]

From: Robert Dobrowolski 
Date: Thu, 19 May 2016 11:56:09 +0200

> From: Rafal Redzimski 
> 
> Current implementation updates the mtu size and notify cdc_ncm
> device using USB_CDC_SET_MAX_DATAGRAM_SIZE request about datagram
> size change instead of changing rx_urb_size.
> 
> Whenever mtu is being changed, datagram size should also be
> updated. Also updating maxmtu formula so it takes max_datagram_size with
> use of cdc_ncm_max_dgram_size() and not ctx.
> 
> Signed-off-by: Robert Dobrowolski 
> Signed-off-by: Rafal Redzimski 

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

[PATCH 1/2] usb: dwc3: Fix DWC3_USB31_REVISION_110A definition

[John Youn]

The DWC3_USB31_REVISION_110A macro uses an invalid constant name in its
definition. This is currently not used.

Signed-off-by: John Youn 
---
 drivers/usb/dwc3/core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 186a886..428ecff 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -859,7 +859,7 @@ struct dwc3 {
  * just so dwc31 revisions are always larger than dwc3.
  */
 #define DWC3_REVISION_IS_DWC31 0x8000
-#define DWC3_USB31_REVISION_110A   (0x3131302a | DWC3_REVISION_IS_USB31)
+#define DWC3_USB31_REVISION_110A   (0x3131302a | DWC3_REVISION_IS_DWC31)
 
enum dwc3_ep0_next  ep0_next_event;
enum dwc3_ep0_state ep0state;
-- 
2.8.2

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

[PATCH 2/2] usb: dwc3: Use the correct speed macros for DSTS/DCFG

[John Youn]

Correct the use of the DWC3_DSTS_XXX_SPEED and DWC3_DCFG_XXX_SPEED
macros. The wrong set of macros were being used in a few places.

This is only a cosmetic change as the values for both sets are
identical.

Signed-off-by: John Youn 
---
 drivers/usb/dwc3/gadget.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c3b0d01..d09d209 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1647,16 +1647,16 @@ static int dwc3_gadget_start(struct usb_gadget *g,
} else {
switch (dwc->maximum_speed) {
case USB_SPEED_LOW:
-   reg |= DWC3_DSTS_LOWSPEED;
+   reg |= DWC3_DCFG_LOWSPEED;
break;
case USB_SPEED_FULL:
-   reg |= DWC3_DSTS_FULLSPEED1;
+   reg |= DWC3_DCFG_FULLSPEED1;
break;
case USB_SPEED_HIGH:
-   reg |= DWC3_DSTS_HIGHSPEED;
+   reg |= DWC3_DCFG_HIGHSPEED;
break;
case USB_SPEED_SUPER_PLUS:
-   reg |= DWC3_DSTS_SUPERSPEED_PLUS;
+   reg |= DWC3_DCFG_SUPERSPEED_PLUS;
break;
default:
dev_err(dwc->dev, "invalid dwc->maximum_speed (%d)\n",
@@ -2375,12 +2375,12 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 
*dwc)
dwc3_update_ram_clk_sel(dwc, speed);
 
switch (speed) {
-   case DWC3_DCFG_SUPERSPEED_PLUS:
+   case DWC3_DSTS_SUPERSPEED_PLUS:
dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
dwc->gadget.ep0->maxpacket = 512;
dwc->gadget.speed = USB_SPEED_SUPER_PLUS;
break;
-   case DWC3_DCFG_SUPERSPEED:
+   case DWC3_DSTS_SUPERSPEED:
/*
 * WORKAROUND: DWC3 revisions <1.90a have an issue which
 * would cause a missing USB3 Reset event.
@@ -2401,18 +2401,18 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 
*dwc)
dwc->gadget.ep0->maxpacket = 512;
dwc->gadget.speed = USB_SPEED_SUPER;
break;
-   case DWC3_DCFG_HIGHSPEED:
+   case DWC3_DSTS_HIGHSPEED:
dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(64);
dwc->gadget.ep0->maxpacket = 64;
dwc->gadget.speed = USB_SPEED_HIGH;
break;
-   case DWC3_DCFG_FULLSPEED2:
-   case DWC3_DCFG_FULLSPEED1:
+   case DWC3_DSTS_FULLSPEED2:
+   case DWC3_DSTS_FULLSPEED1:
dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(64);
dwc->gadget.ep0->maxpacket = 64;
dwc->gadget.speed = USB_SPEED_FULL;
break;
-   case DWC3_DCFG_LOWSPEED:
+   case DWC3_DSTS_LOWSPEED:
dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(8);
dwc->gadget.ep0->maxpacket = 8;
dwc->gadget.speed = USB_SPEED_LOW;
@@ -2422,8 +2422,8 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 
*dwc)
/* Enable USB2 LPM Capability */
 
if ((dwc->revision > DWC3_REVISION_194A) &&
-   (speed != DWC3_DCFG_SUPERSPEED) &&
-   (speed != DWC3_DCFG_SUPERSPEED_PLUS)) {
+   (speed != DWC3_DSTS_SUPERSPEED) &&
+   (speed != DWC3_DSTS_SUPERSPEED_PLUS)) {
reg = dwc3_readl(dwc->regs, DWC3_DCFG);
reg |= DWC3_DCFG_LPM_CAP;
dwc3_writel(dwc->regs, DWC3_DCFG, reg);
-- 
2.8.2

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

Re: [PATCH] net: usb: ch9200: use kmemdup

[David Miller]

From: Muhammad Falak R Wani 
Date: Thu, 19 May 2016 19:26:50 +0530

> Use kmemdup when some other buffer is immediately copied into allocated
> region. It replaces call to allocation followed by memcpy, by a single
> call to kmemdup.
> 
> Signed-off-by: Muhammad Falak R Wani 

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: NET2280: Adding PLX usb2380 support (issues encountered)

[Tim Harvey]

On Thu, May 12, 2016 at 3:48 PM, Justin DeFields
 wrote:
> On Thu, May 12, 2016 at 6:27 PM, Tim Harvey  wrote:
>> On Tue, Jan 26, 2016 at 6:40 AM, Justin DeFields
>>  wrote:
>>> I spoke with the Avago/PLX FAE, and they said that the PLX software
>>> team that worked on this driver is no longer with the company, so they
>>> cannot offer support :(
>>>
>>> I'm not sure there is going to be a good answer for this issue.
>>> Hopefully someone here can help me evaluate the "/* paranoia */" code
>>> to determine if/why/when it's necessary?
>>>
>>> On Fri, Jan 22, 2016 at 10:46 AM, Justin DeFields
>>>  wrote:

 It turns out that I had a filesystem issue on my target when testing
 my removal of the /* paranoia */ block.  I thought I was writing the
 file to NAND, when in fact it was being written over NFS to my Virtual
 Machine!!!  After resolving this issue, and with the removal of the
 paranoia logic, the transfer completes as expected very quickly and
 reliably (20/20 transfers completed successfully).

 Can anyone here help me determine if that paranoia loop is still
 necessary for the PCIE cards, or what the purpose of it was in the
 first place?

>>
>> Justin,
>>
>> Did you ever get anywhere with this? I have an ARM based board with a
>> USB2380 on-board (-AA rev by the way) and gave it a try with the Linux
>> 4.4 kernel today without any luck (I couldn't get g_serial,
>> g_mass_storage, or g_ether to work for me).
>>
>> The USB2380 product brief I have states that it can be used with
>> existing NET 2280/2282 software drivers with no change but of course
>> at least the PCI ID's are missing. What patch/patches do you have thus
>> far to add USB2380 support to net2280?
>>
>> Regards,
>>
>> Tim
>
> I've never received any answers for my questions unfortunately :(.  If
> I remember correctly, just to get the deivce working (though I'm not
> sure how stable) I added a new structure for the 2380, and gave it all
> the same options as the 3380, except that I split the PLX_SUPERSPEED
> into 2 flags (PLX_SUPERSPEED and PLX_PCIE_MSI) to avoid setting the
> super speed for the 2380 in this line:
>
> http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/net2280.c#L3553
>
> Everywhere except for that line, I replaced instances of
> PLX_SUPERSPEED with PLX_PCIE_MSI (a new flag I created).
>

Justin,

thanks for the info. I've done what you suggested and have tested with
g_serial, g_mass_storage, g_ncm, and g_ether against a Linux target
with a USB host controller. I don't see anything misbehaving as far as
I can tell and I don't fall into the paranoia block.

g_serial - no issues using serial
g_ether - no issues using usb0, iperf TCP performance test yields ~75mbps
g_ncm - no issues using usb0, ieprf TCP performance test yeilds ~98mbps
g_usb_mass_storage - I see 6 occurrences of net2280_set_halt_and_wedge
returning -EAGAIN at the beginning (which may be ok?) but my tests
pass fine
# insmod g_mass_storage file=/tmp/backing_file
[  770.18] Mass Storage Function, version: 2009/09/11
[  770.19] LUN: removable file: (no medium)
[  770.19] LUN: file: /tmp/backing_file
[  770.20] Number of LUNs=1
[  770.20] g_mass_storage gadget: Mass Storage Gadget, version: 2009/09/11
[  770.21] g_mass_storage gadget: userspace failed to provide iSerialNumber
[  770.22] g_mass_storage gadget: g_mass_storage ready
[  770.22] net2280 :01:00.0: Operate Defect 7374 workaround
soft this time
[  770.23] net2280 :01:00.0: It will operate on cold-reboot
and SS connect
[  770.83] g_mass_storage gadget: high-speed config #1: Linux
File-Backed Storage
[  771.88] net2280 :01:00.0: net2280_set_halt_and_wedge: error=-11
[  771.99] net2280 :01:00.0: net2280_set_halt_and_wedge: error=-11
[  772.12] net2280 :01:00.0: net2280_set_halt_and_wedge: error=-11
[  772.23] net2280 :01:00.0: net2280_set_halt_and_wedge: error=-11
[  772.34] net2280 :01:00.0: net2280_set_halt_and_wedge: error=-11
[  772.45] net2280 :01:00.0: net2280_set_halt_and_wedge: error=-11
 these are -EAGAIN returned because the ep->queue was empty

I'm assuming these messages are benign.

I'll post a patch to add USB3280 support to net2280 and see if there
is any additional feedback.

Regards,

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

Re: question on trust in chaoskey

[Steve Calfee]

Hi Keith,

On Thu, May 19, 2016 at 9:23 PM, Keith Packard  wrote:
> Dave Tian  writes:
>
>> I am personally in favor of a TPM-like solution, since we probably
>> couldn’t/shouldn’t disable the firmware update anyway,
>> and we really need a hardware root of trust (with a key embedded) in
>> the device, like the TPM in the host.
>
> I don't think we need a true TPM in the hardware; the device is
> read-only in normal operation with firmware upgrades requiring physical
> presence. So, supply the private key with the firmware and then erase it
> From the host once programmed. Once the programming jumper is removed,
> only physical access would be sufficient to extract the private key.
>
> Here's more information about the hardware:
>
> http://altusmetrum.org/ChaosKey/
>
> --

This is the first I have seen this gadget. The obvious use is with
encryption. The obvious problem is someone substituting a false USB
key for a genuine one. The classic "drop flash drives in a conference
lobby", and someone will use it and infect windows. A more
capitalistic attack for any system is someone offering a false USB
"key" for 10% less than anyone else. Any computer that allows any USB
device to be plugged in is at risk. That's why there is physical
protection for servers.

A clever attacker would provide a false USB key which is "almost"
random. This would allow them to decrypt messages based on the false
key, with nobody else knowing there was a vulnerability. An almost
random number simplifies cracking.

It is easy to exactly duplicate all the descriptors and functionality
in a false device. It could be easily done with a PIC, Arduino, or $9
CHIP. Who could tell a key is false or genuine? The false device could
do the same dance with public keys (or whatever secret handshake you
setup).

If a user cannot be sure a key is genuine, and a false key can leak
information, I don't see the point of anyone using such a USB device.

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

Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core

[Peter Chen]

On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote:
> On 18/05/16 17:46, Jun Li wrote:
> > 
> > 
> 
>  I didn't want to have complex Kconfig so decided to have otg as
>  built-in only.
>  What do you want me to change in existing code? and why?
> >>>
> >>> Remove those stuff which only for pass diff driver config Like every
> >>> controller driver need a duplicated
> >>>
> >>> static struct otg_hcd_ops ci_hcd_ops = {
> >>> ...
> >>> }
> >>
> >> This is an exception only. Every controller driver doesn't need to
> >> implement hcd_ops. It is implemented in the hcd core.
> >>
> >>>
> >>> And here is another example, for gadget connect, otg driver can
> >>> directly call to usb_udc_vbus_handler() in drd state machine, but you
> >>> create another interface:
> >>>
> >>> .connect_control = usb_gadget_connect_control,
> >>>
> >>> If the symbol is defined in one driver which is 'm', another driver
> >>> reference it should be 'm' as well, then there is no this kind of
> >>> problem as my understanding.
> >>
> >> That is fine as long as all are 'm'. but how do you solve the case when
> >> Gadget is built in and host is 'm'? OTG has to be built-in and you will
> >> need an hcd to gadget interface.
> > 
> > Hcd to gadget interface? Or you want to say otg to host interface?
> 
> Sorry, I meant to say host to otg interface.
> 
> > 
> > I think hcd and gadget are independent each other, now
> > 
> > Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)
> 
> It is actually a circular dependency for both.
>  hcd <--> otg and gadget <--> otg
> 
> hcd -> otg for usb_otg_register/unregister_hcd
> otg -> hcd for usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg, 
> usb_hub_find_child
> 
> gadget -> otg for usb_otg_register/unregister_gadget
> otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler
> 
> Now consider what will happen if I get rid of the otg_hcd and otg_gadget 
> interfaces.
> 'y' means built-in, 'm' means module.
> 
> 1) hcd 'y', gadget 'y'
> otg has to be 'y' for proper build.
> 
> 2) hcd 'm', gadget 'm'
> otg has to be 'm' for proper build.
> 
> 3) hcd 'y', gadget 'm'
> Build will fail always.
> If otg is 'y', otg build will fail due to dependency on gadget.
> If otg is 'm', hcd build will fail due to dependency on otg.
> 
> 4) hcd 'm', gadget 'y'
> Build will fail always.
> If otg is 'y', otg build will fail due to dependency on hcd.
> If otg is 'm', gadget build will fails due to dependency on otg.
> 
> So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops
> to remove otg->hcd and otg->gadget dependency.
> 
> Now we can address 3) and 4) like so
> 
> 3) hcd 'y', gadget 'm'
> otg has to be 'y' for proper build.
> 
> 4) hcd 'm', gadget 'y'
> otg has to be 'y' for proper build.
> 

How about this:
Moving usb_otg_register/unregister_hcd to host driver to remove
dependency hcd->otg. And moving usb_otg_get_data to common.c.

Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data
returns NULL, the host/device driver's probe return -EPROBE_DEFER.
When the otg driver is probed successfully, the host/device will be
re-probed again, and usb_otg_register_hcd will be called again.

And let OTG depends on HCD && GADGET, and delete otg_hcd_ops and
otg_gadget_ops. Below build dependency issues can be fixed.
What do you think?

> 1) hcd 'y', gadget 'y'
> otg has to be 'y' for proper build.
> 
> 2) hcd 'm', gadget 'm'
> otg has to be 'm' for proper build.
> 
> 3) hcd 'y', gadget 'm'
> Build will fail always.
> If otg is 'y', otg build will fail due to dependency on gadget.
> If otg is 'm', hcd build will fail due to dependency on otg.
> 
> 4) hcd 'm', gadget 'y'
> Build will fail always.
> If otg is 'y', otg build will fail due to dependency on hcd.
> If otg is 'm', gadget build will fails due to dependency on otg.
-- 

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

Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core

[Peter Chen]

On Fri, May 20, 2016 at 10:26:03AM +0300, Roger Quadros wrote:
> Peter,
> 
> On 20/05/16 04:39, Peter Chen wrote:
> > On Wed, May 18, 2016 at 03:45:11PM +0300, Roger Quadros wrote:
> >> On 18/05/16 06:18, Peter Chen wrote:
> >>> On Mon, May 16, 2016 at 12:51:53PM +0300, Roger Quadros wrote:
>  On 16/05/16 12:23, Peter Chen wrote:
> > On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote:
> >> Hi,
> >>
> >> On 16/05/16 10:02, Peter Chen wrote:
> >>> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
>  +
>  +static int usb_gadget_connect_control(struct usb_gadget *gadget, 
>  bool connect)
>  +{
>  +struct usb_udc *udc;
>  +
>  +mutex_lock(&udc_lock);
>  +udc = usb_gadget_to_udc(gadget);
>  +if (!udc) {
>  +dev_err(gadget->dev.parent, "%s: gadget not 
>  registered.\n",
>  +__func__);
>  +mutex_unlock(&udc_lock);
>  +return -EINVAL;
>  +}
>  +
>  +if (connect) {
>  +if (!gadget->connected)
>  +usb_gadget_connect(udc->gadget);
>  +} else {
>  +if (gadget->connected) {
>  +usb_gadget_disconnect(udc->gadget);
>  +udc->driver->disconnect(udc->gadget);
>  +}
>  +}
>  +
>  +mutex_unlock(&udc_lock);
>  +
>  +return 0;
>  +}
>  +
> >>>
> >>> Since this is called for vbus interrupt, why not using
> >>> usb_udc_vbus_handler directly, and call udc->driver->disconnect
> >>> at usb_gadget_stop.
> >>
> >> We can't assume that this is always called for vbus interrupt so
> >> I decided not to call usb_udc_vbus_handler.
> >>
> >> udc->vbus is really pointless for us. We keep vbus states in our
> >> state machine and leave udc->vbus as ture always.
> >>
> >> Why do you want to move udc->driver->disconnect() to stop?
> >> If USB controller disconnected from bus then the gadget driver
> >> must be notified about the disconnect immediately. The controller
> >> may or may not be stopped by the core.
> >>
> >
> > Then, would you give some comments when this API will be used?
> > I was assumed it is only used for drd state machine.
> 
>  drd_state machine didn't even need this API in the first place :).
>  You guys wanted me to separate out start/stop and connect/disconnect for 
>  full OTG case.
>  Won't full OTG state machine want to use this API? If not what would it 
>  use?
> 
> >>>
> >>> Oh, I meant only drd and fully otg state machine needs it. I am
> >>> wondering if we need have a new API to do it. Two questions:
> >>
> >> OK.
> >>>
> >>> - Except for vbus interrupt, any chances this API will be used at
> >>> current logic?
> >>
> >> I don't think so. But we can't assume caller behaviour for any API.
> >>
> >>> - When this API is called but without a coming gadget->stop?
> >>>
> >> Never for DRD case. But we want to catch wrong users.
> >>
> > 
> > In future, otg_start_gadget will be used for both DRD and fully OTG FSM.
> > There is no otg_loc_conn at current DRD FSM, but there is
> > otg_loc_conn at current OTG FSM, see below.
> > 
> > DRD FSM:
> > case OTG_STATE_B_IDLE:
> > drd_set_protocol(fsm, PROTO_UNDEF);
> > otg_drv_vbus(otg, 0);
> > break;
> > case OTG_STATE_B_PERIPHERAL:
> > drd_set_protocol(fsm, PROTO_GADGET);
> > otg_drv_vbus(otg, 0);
> > break;
> > 
> > OTG FSM:
> > case OTG_STATE_B_IDLE:
> > otg_drv_vbus(otg, 0);
> > otg_chrg_vbus(otg, 0);
> > otg_loc_conn(otg, 0);
> > otg_loc_sof(otg, 0);
> > /*
> >  * Driver is responsible for starting ADP probing
> >  * if ADP sensing times out.
> >  */
> > otg_start_adp_sns(otg);
> > otg_set_protocol(fsm, PROTO_UNDEF);
> > otg_add_timer(otg, B_SE0_SRP);
> > break;
> > case OTG_STATE_B_PERIPHERAL:
> > otg_chrg_vbus(otg, 0);
> > otg_loc_sof(otg, 0);
> > otg_set_protocol(fsm, PROTO_GADGET);
> > otg_loc_conn(otg, 1);
> > break;
> > 
> > My original suggestion is to have an API to do pull dp and this API
> > will be used at both DRD and OTG FSM, and called at otg_loc_conn.
> 
> The API is usb_gadget_connect_control();
> 
> > The (de)initialize is the same for both two FSMs, it both includes
> > init peripheral mode and pull up dp, and can be done by 
> > drd_set_protocol(fsm, PROTO_GADGET)
> > otg_loc_conn(otg, 1);
> > 
> > What do you think?
> > 
> 
> I think loc_conn is a bit confusing to drd users. Another issu

Re: [OOPS] cppi41_dma_channel_program: Unable to handle kernel NULL pointer dereference

[Bin Liu]

Hi,

On Fri, May 20, 2016 at 4:20 PM, Matwey V. Kornilov  wrote:
> 2016-05-21 0:12 GMT+03:00 Bin Liu :
>> Hi,
>>
>> On Sat, May 21, 2016 at 12:05:06AM +0300, Matwey V. Kornilov wrote:
>>> By the way, is it ok that function musb_rx_dma_iso_cppi41 uses
>>> hw_ep->tx_channel? I would suppose that it should use rx_channel
>>> instead.
>>
>> I just got here, and am wondering the same. But the question is why just
>> your case hit the problem. I will try to look at it more next week.
>>
>> I had an impression the linux-usb@ has a discussion before about
>> rx/tx-channel messing up, will have to look it up.
>>
>
> Thank you.
> If you need additional info, I can use kgdb with this issue.

After reviewed the code, it must be hw_ep->rx_channel instead. It is likely a
bug caused by commit 069a3fd (usb: musb: Remove ifdefs for musb_host_rx
in musb_host.c part1). Please test with the following patch.

Regards,
-Bin.

-8<-
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 2f8ad7f..9b2553c 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1551,7 +1551,7 @@ static int musb_rx_dma_iso_cppi41(struct
dma_controller *dma,
  struct urb *urb,
  size_t len)
 {
-   struct dma_channel *channel = hw_ep->tx_channel;
+   struct dma_channel *channel = hw_ep->rx_channel;
void __iomem *epio = hw_ep->regs;
dma_addr_t *buf;
u32 length, res;
--
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

[GIT PULL] USB driver update for 4.7-rc1

[Greg KH]

The following changes since commit 44549e8f5eea4e0a41b487b63e616cb089922b99:

  Linux 4.6-rc7 (2016-05-08 14:38:32 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-4.7-rc1

for you to fetch changes up to 60d5794fe5a50d02f4a0df84b45910a4dfa8b487:

  Merge tag 'usb-serial-4.7-rc1' of 
git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial into usb-next 
(2016-05-14 11:25:35 +0200)


USB patches for 4.7-rc1

Here's the big pull request for USB and PHY drivers for 4.7-rc1

Full details in the shortlog, but it's the normal major gadget driver
updates, phy updates, new usbip code, as well as a bit of lots of other
stuff.

All have been in linux-next with no reported issues.

Signed-off-by: Greg Kroah-Hartman 


Alan Stern (2):
  USB: EHCI: make all debugging depend on CONFIG_DYNAMIC_DEBUG
  USB: leave LPM alone if possible when binding/unbinding interface drivers

Alexander Popov (1):
  usbip: fix NULL pointer dereference on errors

Alexandr Ivanov (3):
  usb: xhci: merge xhci_queue_bulk_tx and queue_bulk_sg_tx functions
  usb: xhci: remove duplicate function xhci_urb_to_transfer_ring
  usb: xhci: remove duplicate code of interval checking

Alexey Khoroshilov (1):
  USB: whci-hcd: add more checks for dma mapping error

Andy Shevchenko (5):
  usb: gadged: pch_udc: PCI core handles power state for us
  usb: gadget: pch_udc: convert to devres API
  usb: gadget: pch_udc: enable MSI if hardware supports
  usb: gadged: pch_udc: get rid of redundant assignments
  usb: gadget: pch_udc: sort IDs

Anup Patel (3):
  phy: Rename phy-brcmstb-sata driver to phy-brcm-sata driver
  phy: Add support for NS2 SATA3 PHY in Broadcom SATA3 PHY driver
  dt-bindings: phy: bindings document for common Broadcom SATA3 PHY driver

Arnd Bergmann (2):
  usb: common: rework CONFIG_USB_COMMON logic
  usbip: vudc: fix Kconfig dependencies

Chris Bainbridge (1):
  usb: core: hub: hub_port_init lock controller instead of bus

Chunfeng Yun (4):
  usb: core: buffer: avoid NULL pointer dereferrence
  usb: misc: usbtest: fix error of urb allocation
  dt-bindings: phy-mt65xx-usb: add support for mt2701 platform
  phy: phy-mt65xx-usb3: add support for mt2701 platform

Colin Ian King (2):
  usb: hcd: do not call whc_clean_up on wch_init call failure
  usb/host/fotg210: remove dead code in create_sysfs_files

Dan Carpenter (1):
  usb: dwc3: gadget: fix mask and shift order in DWC3_DCFG_NUMP()

David Mosberger (2):
  drivers: usb: core: Don't disable irqs in usb_sg_wait() during URB submit.
  drivers: usb: core: Minimize irq disabling in usb_sg_cancel()

Denys Vlasenko (1):
  usb: gadget: r8a66597-udc: Deinline pipe_change, save 2176 bytes

Du, Changbin (1):
  usb: dwc3: make dwc3_debugfs_init return value be void

Fei Yang (1):
  usb: dwc3: ep0: sanity check test mode selector

Felipe Balbi (38):
  usb: dwc3: drop FIFO resizing logic
  usb: dwc3: gadget: always enable CSP
  usb: dwc3: increase maximum number of TRBs per endpoint
  usb: dwc3: better name for our request management lists
  usb: gadget: udc: at91: use PTR_ERR_OR_ZERO()
  usb: phy: qcom: use PTR_ERR_OR_ZERO()
  usb: dwc3: remove num_event_buffers
  usb: dwc3: drop ev_buffs array
  usb: dwc3: gadget: pass ev_buff as cookie to irq handler
  usb: dwc3: gadget: combine return points into a single one
  usb: dwc3: gadget: clear SUSPHY bit before ep cmds
  usb: dwc3: gadget: extract unlocked dwc3_gadget_wakeup()
  usb: dwc3: gadget: put link to U0 before Start Transfer
  usb: dwc3: gadget: rename busy/free_slot to trb_enqueue/dequeue
  usb: dwc3: core: document struct dwc3_request
  usb: dwc3: switch trb enqueue/dequeue and first_trb_index to u8
  usb: dwc3: get rid of DWC3_TRB_MASK
  usb: dwc3: gadget: add trb enqueue/dequeue helpers
  usb: dwc3: gadget: move % operation to increment helpers
  usb: dwc3: gadget: use link TRB for all endpoint types
  usb: dwc3: gadget: remove newline from trace
  usb: dwc3: gadget: don't interrupt when chained
  usb: gadget: pch_udc: don't free devm allocated memory
  usb: dwc3: core: add fifo space helper
  usb: dwc3: core: add helper to extract trb type
  usb: dwc3: debugfs: dump out endpoint details
  usb: storage: scsiglue: further describe our 240 sector limit
  usb: storage: scsiglue: limit USB3 devices to 2048 sectors
  usb: storage: fix multi-line comment style
  usb: host: xhci: rcar: retire use of xhci_plat_type_is()
  usb: xhci: plat: add ->plat_start() and ->init_quirk() methods
  usb: host: xhci: plat: move mvebu init_quirk() to xhci_plat_setup()
  usb: host: xhci: plat: change type of mvebu init_qu

Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Guenter Roeck]

On 05/20/2016 06:37 AM, Oliver Neukum wrote:

On Fri, 2016-05-20 at 14:24 +0300, Heikki Krogerus wrote:

On Thu, May 19, 2016 at 04:47:17PM +0200, Oliver Neukum wrote:


Please explain. How does that express DRP but prefered master?


Sorry but I'm not sure what you mean here. If the port is capable of
being used as dual role port (DRP in the supported_data_roles file),
that is the only case where you can select the role with this file. So
I would imagine that in your case you want to make the port act as
DFP only, right? But if the port is capable of acting only as UFP, you
are stuck with that role.


How do I trigger that Try.SRC is to be used on a port?



This would be part of the USB PD protocol, ie probably outside the scope
of the class code. In my implementation, I enable Try.SRC or Try.SNK based
on the platform's preferred role.

Guenter

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

Re: question on trust in chaoskey

[Oliver Neukum]

On Fri, 2016-05-20 at 17:13 -0700, Steve Calfee wrote:

> A clever attacker would provide a false USB key which is "almost"
> random. This would allow them to decrypt messages based on the false
> key, with nobody else knowing there was a vulnerability. An almost
> random number simplifies cracking.
> 
> It is easy to exactly duplicate all the descriptors and functionality
> in a false device. It could be easily done with a PIC, Arduino, or $9
> CHIP. Who could tell a key is false or genuine? The false device could
> do the same dance with public keys (or whatever secret handshake you
> setup).

To a point.There is no reason a key would ever have to go over the wire
unencrypted. You can get at it only by man-in-the-middle or if you get
at the hardware.
We can protect against sniffing and require authentification.

> If a user cannot be sure a key is genuine, and a false key can leak
> information, I don't see the point of anyone using such a USB device.

You will have to trust your hardware if you run a computer.
The questions of whether your hardware is indeed your hardware
and whether you can trust your hardware are distinct.
The former problem we can address.

Regards
Oliver


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

Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Oliver Neukum]

On Fri, 2016-05-20 at 22:51 -0700, Guenter Roeck wrote:
> On 05/20/2016 06:37 AM, Oliver Neukum wrote:
> > On Fri, 2016-05-20 at 14:24 +0300, Heikki Krogerus wrote:
> >> On Thu, May 19, 2016 at 04:47:17PM +0200, Oliver Neukum wrote:
> >>>
> >>> Please explain. How does that express DRP but prefered master?
> >>
> >> Sorry but I'm not sure what you mean here. If the port is capable of
> >> being used as dual role port (DRP in the supported_data_roles file),
> >> that is the only case where you can select the role with this file. So
> >> I would imagine that in your case you want to make the port act as
> >> DFP only, right? But if the port is capable of acting only as UFP, you
> >> are stuck with that role.
> >
> > How do I trigger that Try.SRC is to be used on a port?
> >
> 
> This would be part of the USB PD protocol, ie probably outside the scope
> of the class code. In my implementation, I enable Try.SRC or Try.SNK based
> on the platform's preferred role.

Hi,

from a purely formal point of view that makes sense.

>From a usability viewpoint I'd ideally want all controls
for role at one place. And possibly the controls for modes
at the same place.

I think part of the problem here is that we lack a statement
of mission. What are these controls for?
Merely for controlling modes and providing information
about modes? And to use neutral terms only for the "master"
side or both sides?

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