[PATCH 1/1] USB: serial: option: add Telit ME910 ECM composition

[Daniele Palmas]

This patch adds Telit ME910 family ECM composition 0x1102.

Signed-off-by: Daniele Palmas 
---
lsusb verbose output:

Bus 003 Device 005: ID 1bc7:1102 Telit Wireless Solutions 
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass  239 Miscellaneous Device
  bDeviceSubClass 2 ?
  bDeviceProtocol 1 Interface Association
  bMaxPacketSize064
  idVendor   0x1bc7 Telit Wireless Solutions
  idProduct  0x1102 
  bcdDevice0.00
  iManufacturer   4 Telit
  iProduct3 Telit ME910
  iSerial 5 26fd75b1
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength  171
bNumInterfaces  5
bConfigurationValue 1
iConfiguration  2 Telit Configuration
bmAttributes 0xe0
  Self Powered
  Remote Wakeup
MaxPower  500mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   2
  bInterfaceClass   255 Vendor Specific Class
  bInterfaceSubClass255 Vendor Specific Subclass
  bInterfaceProtocol255 Vendor Specific Protocol
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01  EP 1 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber1
  bAlternateSetting   0
  bNumEndpoints   3
  bInterfaceClass   255 Vendor Specific Class
  bInterfaceSubClass255 Vendor Specific Subclass
  bInterfaceProtocol255 Vendor Specific Protocol
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82  EP 2 IN
bmAttributes3
  Transfer TypeInterrupt
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0040  1x 64 bytes
bInterval   5
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83  EP 3 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02  EP 2 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber2
  bAlternateSetting   0
  bNumEndpoints   3
  bInterfaceClass   255 Vendor Specific Class
  bInterfaceSubClass254 
  bInterfaceProtocol255 
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x84  EP 4 IN
bmAttributes3
  Transfer TypeInterrupt
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0040  1x 64 bytes
bInterval   5
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x85  EP 5 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03  EP 3 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type  

Re: [PATCH v2] USB: serial: cp210x: Fix GPIO in autosuspend

[Johan Hovold]

On Sun, Feb 17, 2019 at 06:59:01PM +0100, Karoly Pados wrote:
> Current GPIO code in cp210x fails to take USB autosuspend into account,
> making it practically impossible to use GPIOs with autosuspend enabled
> without user configuration. Fix this like for ftdi_sio in a previous patch.
> Tested on a CP2102N.
> 
> Signed-off-by: Karoly Pados 
> ---
> Changelog:
> - v2: Restrict new autopm calls to GPIO paths.
>   Always check result of usb_autopm_get.
>   Rebased to current usb_serial upstream.

Thanks for the v2. Now applied after removing the unmotivated white
space change.

I also added a Fixes and CC-stable tag as this seems annoying enough to
warrant a backport even if autosuspend was never actually supported.

Johan

Re: 4.20.7: pl2303 not working (post-4.19 regression) (limited info so far, not yet bisected)

[Johan Hovold]

On Mon, Feb 18, 2019 at 10:32:57AM +, Nix wrote:
> On 18 Feb 2019, Johan Hovold stated:
> 
> > On Sun, Feb 17, 2019 at 07:13:52PM +, Nix wrote:
> >> I'm still fairly sure this is a regression -- my machines are often up
> >> for a lot longer than that and I've never seen this before I upgraded to
> >> 4.20.x -- but I don't think I'm going to identify it by mindless
> >> bisection. I might have to actually *think* about it.
> >
> > I doubt it's a regression in usb-serial as essentially nothing changed
> > with respect to pl2303 or core since 4.19.
> 
> Yeah, I came to that conclusion as well.
> 
> > The -ENOSPC you're seeing is returned by the host controller to
> > indicate:
> >
> > This request would overcommit the usb bandwidth reserved for
> > periodic transfers (interrupt, isochronous).
> 
> Side note: probably not related to *this* -ENOSPC, which I've been
> seeing for a few releases now and which appears to break Chromium's U2F
> negotiation when the USB bus has sufficiently weird devices on it (like,
> uh, my wireless mouse):
> 
> 
> 
> (I say "probably not related" because it's much older and long predates
> the pl2303 trouble.)

Yeah, hard to tell from a quick look.

> > but if you're saying you can reproduce this on "every box" it may not be
> > related to any particular host-controller driver (or USB topology).
> 
> They are all xhci, at least. The pl2303 is USB 2. One machine, a
> two-year-old Broadwell server, says:

> So the quirks are all totally different, and the controllers are quite
> different as well...

Yeah, but they are all xhci as you point out so theoretically it could
be an xhci driver regression.

Johan

Re: [PATCH] usb: xhci: add Immediate Data Transfer support

[Felipe Balbi]

Nicolas Saenz Julienne  writes:

> Immediate data transfers (IDT) allow the HCD to copy small chunks of
> data (up to 8bytes) directly into its output transfer TRBs. This avoids
> the somewhat expensive DMA mappings that are performed by default on
> most URBs submissions.
>
> In the case an URB was suitable for IDT. The data is directly copied
> into the "Data Buffer Pointer" region of the TRB and the IDT flag is
> set. Instead of triggering memory accesses the HC will use the data
> directly.
>
> The implementation could cover all kind of output endpoints. Yet
> Isochronous endpoints are bypassed as I was unable to find one that
> matched IDT's constraints. As we try to bypass the default DMA mappings
> on URB buffers we'd need to find a Isochronous device with an
> urb->transfer_buffer_length <= 8 bytes.
>
> The implementation takes into account that the 8 byte buffers provided
> by the URB will never cross a 64KB boundary.
>
> Signed-off-by: Nicolas Saenz Julienne 

This looks good to my eyes.

Reviewed-by: Felipe Balbi 

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 1/1] USB: serial: option: add Telit ME910 ECM composition

[Johan Hovold]

On Wed, Feb 20, 2019 at 10:13:09AM +0100, Daniele Palmas wrote:
> This patch adds Telit ME910 family ECM composition 0x1102.
> 
> Signed-off-by: Daniele Palmas 
> ---
> lsusb verbose output:
> 
> Bus 003 Device 005: ID 1bc7:1102 Telit Wireless Solutions 
> Device Descriptor:
>   bLength18
>   bDescriptorType 1
>   bcdUSB   2.00
>   bDeviceClass  239 Miscellaneous Device
>   bDeviceSubClass 2 ?
>   bDeviceProtocol 1 Interface Association
>   bMaxPacketSize064
>   idVendor   0x1bc7 Telit Wireless Solutions
>   idProduct  0x1102 
>   bcdDevice0.00
>   iManufacturer   4 Telit
>   iProduct3 Telit ME910
>   iSerial 5 26fd75b1
>   bNumConfigurations  1
>   Configuration Descriptor:
> bLength 9
> bDescriptorType 2
> wTotalLength  171
> bNumInterfaces  5
> bConfigurationValue 1
> iConfiguration  2 Telit Configuration
> bmAttributes 0xe0
>   Self Powered
>   Remote Wakeup
> MaxPower  500mA
> Interface Descriptor:
>   bLength 9
>   bDescriptorType 4
>   bInterfaceNumber0
>   bAlternateSetting   0
>   bNumEndpoints   2
>   bInterfaceClass   255 Vendor Specific Class
>   bInterfaceSubClass255 Vendor Specific Subclass
>   bInterfaceProtocol255 Vendor Specific Protocol
>   iInterface  0 
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x81  EP 1 IN
> bmAttributes2
>   Transfer TypeBulk
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0200  1x 512 bytes
> bInterval   0
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x01  EP 1 OUT
> bmAttributes2
>   Transfer TypeBulk
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0200  1x 512 bytes
> bInterval   0
> Interface Descriptor:
>   bLength 9
>   bDescriptorType 4
>   bInterfaceNumber1
>   bAlternateSetting   0
>   bNumEndpoints   3
>   bInterfaceClass   255 Vendor Specific Class
>   bInterfaceSubClass255 Vendor Specific Subclass
>   bInterfaceProtocol255 Vendor Specific Protocol
>   iInterface  0 
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x82  EP 2 IN
> bmAttributes3
>   Transfer TypeInterrupt
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0040  1x 64 bytes
> bInterval   5
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x83  EP 3 IN
> bmAttributes2
>   Transfer TypeBulk
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0200  1x 512 bytes
> bInterval   0
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x02  EP 2 OUT
> bmAttributes2
>   Transfer TypeBulk
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0200  1x 512 bytes
> bInterval   0
> Interface Descriptor:
>   bLength 9
>   bDescriptorType 4
>   bInterfaceNumber2
>   bAlternateSetting   0
>   bNumEndpoints   3
>   bInterfaceClass   255 Vendor Specific Class
>   bInterfaceSubClass254 
>   bInterfaceProtocol255 
>   iInterface  0 
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x84  EP 4 IN
> bmAttributes3
>   Transfer TypeInterrupt
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0040  1x 64 bytes
> bInterval   5
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x85  EP 5 IN
> bmAttributes2
>   Transfer TypeBulk
>   Synch Type   None
>   Usage Type   Data

Re: [PATCH 1/1] USB: serial: option: add Telit ME910 ECM composition

[Daniele Palmas]

Hi Johan,

Il giorno mer 20 feb 2019 alle ore 10:39 Johan Hovold
 ha scritto:
>
> On Wed, Feb 20, 2019 at 10:13:09AM +0100, Daniele Palmas wrote:
> > This patch adds Telit ME910 family ECM composition 0x1102.
> >
> > Signed-off-by: Daniele Palmas 
> > ---
> > lsusb verbose output:
> >
> > Bus 003 Device 005: ID 1bc7:1102 Telit Wireless Solutions
> > Device Descriptor:
> >   bLength18
> >   bDescriptorType 1
> >   bcdUSB   2.00
> >   bDeviceClass  239 Miscellaneous Device
> >   bDeviceSubClass 2 ?
> >   bDeviceProtocol 1 Interface Association
> >   bMaxPacketSize064
> >   idVendor   0x1bc7 Telit Wireless Solutions
> >   idProduct  0x1102
> >   bcdDevice0.00
> >   iManufacturer   4 Telit
> >   iProduct3 Telit ME910
> >   iSerial 5 26fd75b1
> >   bNumConfigurations  1
> >   Configuration Descriptor:
> > bLength 9
> > bDescriptorType 2
> > wTotalLength  171
> > bNumInterfaces  5
> > bConfigurationValue 1
> > iConfiguration  2 Telit Configuration
> > bmAttributes 0xe0
> >   Self Powered
> >   Remote Wakeup
> > MaxPower  500mA
> > Interface Descriptor:
> >   bLength 9
> >   bDescriptorType 4
> >   bInterfaceNumber0
> >   bAlternateSetting   0
> >   bNumEndpoints   2
> >   bInterfaceClass   255 Vendor Specific Class
> >   bInterfaceSubClass255 Vendor Specific Subclass
> >   bInterfaceProtocol255 Vendor Specific Protocol
> >   iInterface  0
> >   Endpoint Descriptor:
> > bLength 7
> > bDescriptorType 5
> > bEndpointAddress 0x81  EP 1 IN
> > bmAttributes2
> >   Transfer TypeBulk
> >   Synch Type   None
> >   Usage Type   Data
> > wMaxPacketSize 0x0200  1x 512 bytes
> > bInterval   0
> >   Endpoint Descriptor:
> > bLength 7
> > bDescriptorType 5
> > bEndpointAddress 0x01  EP 1 OUT
> > bmAttributes2
> >   Transfer TypeBulk
> >   Synch Type   None
> >   Usage Type   Data
> > wMaxPacketSize 0x0200  1x 512 bytes
> > bInterval   0
> > Interface Descriptor:
> >   bLength 9
> >   bDescriptorType 4
> >   bInterfaceNumber1
> >   bAlternateSetting   0
> >   bNumEndpoints   3
> >   bInterfaceClass   255 Vendor Specific Class
> >   bInterfaceSubClass255 Vendor Specific Subclass
> >   bInterfaceProtocol255 Vendor Specific Protocol
> >   iInterface  0
> >   Endpoint Descriptor:
> > bLength 7
> > bDescriptorType 5
> > bEndpointAddress 0x82  EP 2 IN
> > bmAttributes3
> >   Transfer TypeInterrupt
> >   Synch Type   None
> >   Usage Type   Data
> > wMaxPacketSize 0x0040  1x 64 bytes
> > bInterval   5
> >   Endpoint Descriptor:
> > bLength 7
> > bDescriptorType 5
> > bEndpointAddress 0x83  EP 3 IN
> > bmAttributes2
> >   Transfer TypeBulk
> >   Synch Type   None
> >   Usage Type   Data
> > wMaxPacketSize 0x0200  1x 512 bytes
> > bInterval   0
> >   Endpoint Descriptor:
> > bLength 7
> > bDescriptorType 5
> > bEndpointAddress 0x02  EP 2 OUT
> > bmAttributes2
> >   Transfer TypeBulk
> >   Synch Type   None
> >   Usage Type   Data
> > wMaxPacketSize 0x0200  1x 512 bytes
> > bInterval   0
> > Interface Descriptor:
> >   bLength 9
> >   bDescriptorType 4
> >   bInterfaceNumber2
> >   bAlternateSetting   0
> >   bNumEndpoints   3
> >   bInterfaceClass   255 Vendor Specific Class
> >   bInterfaceSubClass254
> >   bInterfaceProtocol255
> >   iInterface  0
> >   Endpoint Descriptor:
> > bLength 7
> > bDescriptorType 5
> > bEndpointAddress 0x84  EP 4 IN
> > bmAttributes3
> >   Transfer TypeInterrupt
> >   Synch Type   None
> >   Usage Type   Data
> > wMaxPacketSize 0x0040  1x 64 bytes
> > bInterval 

Re: [PATCH 1/1] USB: serial: option: add Telit ME910 ECM composition

[Johan Hovold]

On Wed, Feb 20, 2019 at 10:47:59AM +0100, Daniele Palmas wrote:
> Hi Johan,
> 
> Il giorno mer 20 feb 2019 alle ore 10:39 Johan Hovold
>  ha scritto:
> >
> > On Wed, Feb 20, 2019 at 10:13:09AM +0100, Daniele Palmas wrote:
> > > This patch adds Telit ME910 family ECM composition 0x1102.

> > > diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> > > index aef15497ff31..0cf943281065 100644
> > > --- a/drivers/usb/serial/option.c
> > > +++ b/drivers/usb/serial/option.c
> > > @@ -1148,6 +1148,8 @@ static const struct usb_device_id option_ids[] = {
> > > .driver_info = NCTRL(0) | RSVD(1) | RSVD(3) },
> > >   { USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_ME910_DUAL_MODEM),
> > > .driver_info = NCTRL(0) | RSVD(3) },
> > > + { USB_DEVICE(TELIT_VENDOR_ID, 0x1102), /* TELIT ME910 (ECM) */
> > > +   .driver_info = NCTRL(0) | RSVD(3) | RSVD(4) },
> >
> > I realise this probably just reuses a pattern from the earlier Telit
> > entries, but why not match on the interface class instead of
> > blacklisting interface 3 and 4?
> >
> 
> Yes, it was just for keeping the entry coherent with previous ME910
> related ones.
> 
> I can send a V2 fixing this.

Please do, thanks. Let's try to keep the explicit blacklisting to a
minimum.

Johan

Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+

[Stanislaw Gruszka]

On Tue, Feb 19, 2019 at 10:40:47AM -0500, Alan Stern wrote:
> On Tue, 19 Feb 2019, Stanislaw Gruszka wrote:
> 
> > It would be interesting why urb->num_sgs = 0 & urb->sg cause
> > the troubles. This is how usb_sg_init() submit urbs for sg_tablesize = 0
> > controllers. So either are there are some requirement on urb->sg
> > mapped via dma_map_page() (which mt76usb does not meet) not needed
> > for urb->transfer_buffer mapped via dma_map_single() or there
> > is something wrong in dwc2 with sg and this driver will not
> > work with urb_sg_init() as well. I don't have hardware to investigate
> > this and don't want to bother you with more patches.
> 
> urb->sg != NULL and urb->num_sgs == 0 means that the transfer buffer 
> must fit into a single page, which is pointed to by the first element 
> of the scatterlist.

I asked about that in other thread 
https://lore.kernel.org/linux-wireless/2cc5674a-a3a0-d8fe-65f5-4357da9b8...@arm.com/

the answer was it is weird but valid. However I think do dma_map_page()
on buffer not fit in the single page is asking for troubles. I just posted
patch that should fix this for mt76usb.

> But now that I look at the code in usb_sg_init(), it seems odd that the
> !use_sg case doesn't increment sg during each loop iteration.  I don't
> see how that could possibly work -- it looks like a bug!

Looks for me that this is done via for_each_sg(sg, sg, io->entries, i) loop.

Stanislaw

Re: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

[Roger Quadros]

Pawel,

On 14/02/2019 21:45, Pawel Laszczak wrote:
> Controller for OUT endpoints has shared on-chip buffers for all incoming
> packets, including ep0out. It's FIFO buffer, so packets must be handle
> by DMA in correct order. If the first packet in the buffer will not be
> handled, then the following packets directed for other endpoints and
> functions will be blocked.
> 
> Additionally the packets directed to one endpoint can block entire on-chip
> buffers. In this case transfer to other endpoints also will blocked.
> 
> To resolve this issue after raising the descriptor missing interrupt
> driver prepares internal usb_request object and use it to arm DMA
> transfer.
> 
> The problematic situation was observed in case when endpoint has
> been enabled but no usb_request were queued. Driver try detects
> such endpoints and will use this workaround only for these endpoint.
> 
> Driver use limited number of buffer. This number can be set by macro
> CDNS_WA2_NUM_BUFFERS.
> 
> Such blocking situation was observed on ACM gadget. For this function
> host send OUT data packet but ACM function is not prepared for
> this packet. It's cause that buffer placed in on chip memory block
> transfer to other endpoints.
> 
> It's limitation of controller but maybe this issues should be fixed in
> function driver.
> 
> This work around can be disabled/enabled by means of quirk_internal_buffer
> module parameter. By default feature is enabled. It can has impact to
> transfer performance and in most case this feature can be disabled.

How much is the performance impact?

You mentioned that the issue was observed only in the ACM case and
could be fixed in the function driver?
It seems pointless to enable an endpoint and not have any requests queued for 
it.

Isn't fixing the ACM driver (if there is a real issue) a better approach than 
having
a workaround that affects performance of all other use cases?

cheers,
-roger

> 
> Signed-off-by: Pawel Laszczak 
> ---
>  drivers/usb/cdns3/gadget.c | 273 -
>  drivers/usb/cdns3/gadget.h |   7 +
>  2 files changed, 278 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> index 7f7f24ee3c4b..5dfbe6e1421c 100644
> --- a/drivers/usb/cdns3/gadget.c
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -27,6 +27,37 @@
>   * If (((Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr-1) or
>   *   (Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr))
>   *   and (DRBL==1 and (not EP0)))
> + *
> + * Work around 2:
> + * Controller for OUT endpoints has shared on-chip buffers for all incoming
> + * packets, including ep0out. It's FIFO buffer, so packets must be handle by 
> DMA
> + * in correct order. If the first packet in the buffer will not be handled,
> + * then the following packets directed for other endpoints and  functions
> + * will be blocked.
> + * Additionally the packets directed to one endpoint can block entire on-chip
> + * buffers. In this case transfer to other endpoints also will blocked.
> + *
> + * To resolve this issue after raising the descriptor missing interrupt
> + * driver prepares internal usb_request object and use it to arm DMA 
> transfer.
> + *
> + * The problematic situation was observed in case when endpoint has been 
> enabled
> + * but no usb_request were queued. Driver try detects such endpoints and will
> + * use this workaround only for these endpoint.
> + *
> + * Driver use limited number of buffer. This number can be set by macro
> + * CDNS_WA2_NUM_BUFFERS.
> + *
> + * Such blocking situation was observed on ACM gadget. For this function
> + * host send OUT data packet but ACM function is not prepared for this 
> packet.
> + * It's cause that buffer placed in on chip memory block transfer to other
> + * endpoints.
> + *
> + * It's limitation of controller but maybe this issues should be fixed in
> + * function driver.
> + *
> + * This work around can be disabled/enabled by means of quirk_internal_buffer
> + * module parameter. By default feature is enabled. It can has impact to
> + * transfer performance and in most case this feature can be disabled.
>   */
>  
>  #include 
> @@ -42,6 +73,14 @@ static int __cdns3_gadget_ep_queue(struct usb_ep *ep,
>  struct usb_request *request,
>  gfp_t gfp_flags);
>  
> +/*
> + * Parameter allows to disable/enable handling of work around 2 feature.
> + * By default this value is enabled.
> + */
> +static bool quirk_internal_buffer = 1;
> +module_param(quirk_internal_buffer, bool, 0644);
> +MODULE_PARM_DESC(quirk_internal_buffer, "Disable/enable WA2 algorithm");
> +
>  /**
>   * cdns3_handshake - spin reading  until handshake completes or fails
>   * @ptr: address of device controller register to be read
> @@ -105,6 +144,17 @@ struct usb_request *cdns3_next_request(struct list_head 
> *list)
>   return list_first_entry_or_null(list, struct usb_request, list);
>  }
>  
> +/**
> + * cdns3_nex

[PATCH v2 1/1] USB: serial: option: add Telit ME910 ECM composition

[Daniele Palmas]

This patch adds Telit ME910 family ECM composition 0x1102.

Signed-off-by: Daniele Palmas 
---
v2: add matching on interface class to remove blacklisting

lsusb verbose output:

Bus 003 Device 005: ID 1bc7:1102 Telit Wireless Solutions
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass  239 Miscellaneous Device
  bDeviceSubClass 2 ?
  bDeviceProtocol 1 Interface Association
  bMaxPacketSize064
  idVendor   0x1bc7 Telit Wireless Solutions
  idProduct  0x1102 
  bcdDevice0.00
  iManufacturer   4 Telit
  iProduct3 Telit ME910
  iSerial 5 26fd75b1
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength  171
bNumInterfaces  5
bConfigurationValue 1
iConfiguration  2 Telit Configuration
bmAttributes 0xe0
  Self Powered
  Remote Wakeup
MaxPower  500mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   2
  bInterfaceClass   255 Vendor Specific Class
  bInterfaceSubClass255 Vendor Specific Subclass
  bInterfaceProtocol255 Vendor Specific Protocol
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01  EP 1 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber1
  bAlternateSetting   0
  bNumEndpoints   3
  bInterfaceClass   255 Vendor Specific Class
  bInterfaceSubClass255 Vendor Specific Subclass
  bInterfaceProtocol255 Vendor Specific Protocol
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82  EP 2 IN
bmAttributes3
  Transfer TypeInterrupt
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0040  1x 64 bytes
bInterval   5
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83  EP 3 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02  EP 2 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber2
  bAlternateSetting   0
  bNumEndpoints   3
  bInterfaceClass   255 Vendor Specific Class
  bInterfaceSubClass254 
  bInterfaceProtocol255 
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x84  EP 4 IN
bmAttributes3
  Transfer TypeInterrupt
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0040  1x 64 bytes
bInterval   5
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x85  EP 5 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03  EP 3 OUT
bmAttributes2
  T

Re: usb: over-current condition

[Subhashini Rao Beerisetty]

On Tue, Feb 19, 2019 at 8:30 PM Roger Quadros  wrote:
>
> Please keep linux-usb in cc.
>
> On 19/02/2019 16:50, Subhashini Rao Beerisetty wrote:
> > On Tue, Feb 19, 2019 at 8:06 PM Roger Quadros  wrote:
> >>
> >> Hi Subhashini,
> >>
> >> On 19/02/2019 16:30, Subhashini Rao Beerisetty wrote:
> >>> Hi All,
> >>>
> >>> [ Please keep me in CC as I'm not subscribed to the list]
> >>>
> >>>
> >>> I’m using an Ubuntu 16.04.4 LTS PC and when I connect an USB device to
> >>> my Ubuntu PC, I see the below mentioned messages in dmesg.
> >>>
> >>>
> >>>
> >>> [ 9012.287432] usb usb3-port1: over-current condition
> >>>
> >>> [ 9012.287739] usb usb2-port1: over-current condition
> >>>
> >>> [ 9012.495919] usb 2-4: USB disconnect, device number 3
> >>>
> >>> [ 9012.495940] usb 2-4.1: USB disconnect, device number 5
> >>>
> >>> [ 9012.615609] usb usb3-port1: over-current condition
> >>>
> >>> [ 9012.628513] usb 2-4.2: USB disconnect, device number 7
> >>>
> >>> [ 9012.824274] usb usb3-port1: over-current condition
> >>>
> >>> [ 9015.304232] hub_port_connect: 12 callbacks suppressed
> >>>
> >>> [ 9015.304254] usb usb2-port4: connect-debounce failed
> >>>
> >>> [ 9015.512437] usb usb2-port1: over-current condition
> >>>
> >>> [ 9015.720767] usb usb2-port2: over-current condition
> >>>
> >>>
> >>>
> >>> I’d like to What these messages indicates? Is it a hardware problem
> >>> from host side(ubuntu PC) or device side?
> >>
> >> Can you try other USB devices?
> > Yes, on the same port other devices works fine. I tried connecting
> > Keyboard and it works fine. Basically I'm looking for how to debug
> > further for this type of issues.
>
> The device is drawing too much current which is triggering
> the overcurrent detection/protection circuit on the USB hub.
>
> What device is it? If it is an off the shelf device it might indicate
> a hardware fault and so time to buy a new one?
It is an embedded board. From Host PC, we are communicating to PIC on
embedded board via USB port.
>
> >
> >> If it happens with just that one USB device, it is a device problem.
> >>
>
> --
> cheers,
> -roger
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH v2 1/1] USB: serial: option: add Telit ME910 ECM composition

[Johan Hovold]

On Wed, Feb 20, 2019 at 11:43:17AM +0100, Daniele Palmas wrote:
> This patch adds Telit ME910 family ECM composition 0x1102.
> 
> Signed-off-by: Daniele Palmas 
> ---
> v2: add matching on interface class to remove blacklisting
> 
> lsusb verbose output:

> ---
>  drivers/usb/serial/option.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index aef15497ff31..f6674fb7269a 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -1148,6 +1148,8 @@ static const struct usb_device_id option_ids[] = {
> .driver_info = NCTRL(0) | RSVD(1) | RSVD(3) },
>   { USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_ME910_DUAL_MODEM),
> .driver_info = NCTRL(0) | RSVD(3) },
> + { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1102, 0xff),
> +   .driver_info = NCTRL(0) }, /* TELIT ME910 (ECM) */

I moved the comment to the USB_DEVICE line for consistency with the
other entries (it's ok to break the 80 col rule here).

>   { USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_LE910),
> .driver_info = NCTRL(0) | RSVD(1) | RSVD(2) },
>   { USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_LE910_USBCFG4),

Now applied.

Thanks,
Johan

[PATCH] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with 
plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
controllers")
Signed-off-by: Nikolaus Voss 
---
 drivers/usb/typec/tps6598x.c | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c84c8c189e90..57a3e6c5c175 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
*val, size_t len)
return 0;
 }
 
+static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
+   void *val, size_t len)
+{
+   u8 data[len + 1];
+
+   if (!tps->i2c_protocol)
+   return regmap_raw_write(tps->regmap, reg, val, len);
+
+   data[0] = len;
+   memcpy(&data[1], val, len);
+
+   return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
+}
+
 static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
 {
return tps6598x_block_read(tps, reg, val, sizeof(u16));
@@ -127,23 +141,23 @@ static inline int tps6598x_read64(struct tps6598x *tps, 
u8 reg, u64 *val)
 
 static inline int tps6598x_write16(struct tps6598x *tps, u8 reg, u16 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u16));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u16));
 }
 
 static inline int tps6598x_write32(struct tps6598x *tps, u8 reg, u32 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
 }
 
 static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u64));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u64));
 }
 
 static inline int
 tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
 }
 
 static int tps6598x_read_partner_identity(struct tps6598x *tps)
-- 
2.17.1

RE: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

[Pawel Laszczak]

Hi Roger.
>
>On 14/02/2019 21:45, Pawel Laszczak wrote:
>> Controller for OUT endpoints has shared on-chip buffers for all incoming
>> packets, including ep0out. It's FIFO buffer, so packets must be handle
>> by DMA in correct order. If the first packet in the buffer will not be
>> handled, then the following packets directed for other endpoints and
>> functions will be blocked.
>>
>> Additionally the packets directed to one endpoint can block entire on-chip
>> buffers. In this case transfer to other endpoints also will blocked.
>>
>> To resolve this issue after raising the descriptor missing interrupt
>> driver prepares internal usb_request object and use it to arm DMA
>> transfer.
>>
>> The problematic situation was observed in case when endpoint has
>> been enabled but no usb_request were queued. Driver try detects
>> such endpoints and will use this workaround only for these endpoint.
>>
>> Driver use limited number of buffer. This number can be set by macro
>> CDNS_WA2_NUM_BUFFERS.
>>
>> Such blocking situation was observed on ACM gadget. For this function
>> host send OUT data packet but ACM function is not prepared for
>> this packet. It's cause that buffer placed in on chip memory block
>> transfer to other endpoints.
>>
>> It's limitation of controller but maybe this issues should be fixed in
>> function driver.
>>
>> This work around can be disabled/enabled by means of quirk_internal_buffer
>> module parameter. By default feature is enabled. It can has impact to
>> transfer performance and in most case this feature can be disabled.
>
>How much is the performance impact?

I didn't check this, but performance will be decreased because driver has to 
copy data from internally allocated buffer to usb_request->buff.

>You mentioned that the issue was observed only in the ACM case and
>could be fixed in the function driver?
>It seems pointless to enable an endpoint and not have any requests queued for 
>it.

Yes, it’s true. The request in ACM class is send to Controller Driver when user 
read file associated 
with ACM gadget. Problem exist because host send data to controller but USB 
controller 
has not prepared buffer for this data by ACM class.

>Isn't fixing the ACM driver (if there is a real issue) a better approach than 
>having
>a workaround that affects performance of all other use cases?

Yes it should be better. But what ACM driver should do with unexpected data. 
I'm not sure if we 
can simply delete them. 

The best solution would be to make modification in controller. In such case 
Controller shouldn't accept 
packet but should send NAK. 
 
One more thing. Workaround has implemented algorithm that decide for which 
endpoint it should be enabled.
e.g for composite device MSC+NCM+ACM it should work only for ACM OUT endpoint.

Cheers,
Pawel
>
>>
>> Signed-off-by: Pawel Laszczak 
>> ---
>>  drivers/usb/cdns3/gadget.c | 273 -
>>  drivers/usb/cdns3/gadget.h |   7 +
>>  2 files changed, 278 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>> index 7f7f24ee3c4b..5dfbe6e1421c 100644
>> --- a/drivers/usb/cdns3/gadget.c
>> +++ b/drivers/usb/cdns3/gadget.c
>> @@ -27,6 +27,37 @@
>>   * If (((Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr-1) or
>>   *  (Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr))
>>   *  and (DRBL==1 and (not EP0)))
>> + *
>> + * Work around 2:
>> + * Controller for OUT endpoints has shared on-chip buffers for all incoming
>> + * packets, including ep0out. It's FIFO buffer, so packets must be handle 
>> by DMA
>> + * in correct order. If the first packet in the buffer will not be handled,
>> + * then the following packets directed for other endpoints and  functions
>> + * will be blocked.
>> + * Additionally the packets directed to one endpoint can block entire 
>> on-chip
>> + * buffers. In this case transfer to other endpoints also will blocked.
>> + *
>> + * To resolve this issue after raising the descriptor missing interrupt
>> + * driver prepares internal usb_request object and use it to arm DMA 
>> transfer.
>> + *
>> + * The problematic situation was observed in case when endpoint has been 
>> enabled
>> + * but no usb_request were queued. Driver try detects such endpoints and 
>> will
>> + * use this workaround only for these endpoint.
>> + *
>> + * Driver use limited number of buffer. This number can be set by macro
>> + * CDNS_WA2_NUM_BUFFERS.
>> + *
>> + * Such blocking situation was observed on ACM gadget. For this function
>> + * host send OUT data packet but ACM function is not prepared for this 
>> packet.
>> + * It's cause that buffer placed in on chip memory block transfer to other
>> + * endpoints.
>> + *
>> + * It's limitation of controller but maybe this issues should be fixed in
>> + * function driver.
>> + *
>> + * This work around can be disabled/enabled by means of 
>> quirk_internal_buffer
>> + * module parameter. By default feature is enabled. It can has impact to

Re: [PATCH] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Heikki Krogerus]

Hi,

On Mon, Sep 10, 2018 at 07:05:01AM +0200, Nikolaus Voss wrote:
> Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
> adapters, but the problem described with regmap-i2c not handling
> SMBus block transfers (i.e. read and writes) correctly also exists
> with writes.
> 
> As workaround, this patch adds a block write function the same way
> 1a2f474d328f adds a block read function.
> 
> Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately 
> with plain-I2C adapters")
> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
> controllers")
> Signed-off-by: Nikolaus Voss 
> ---
>  drivers/usb/typec/tps6598x.c | 22 ++
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index c84c8c189e90..57a3e6c5c175 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
> *val, size_t len)
>   return 0;
>  }
>  
> +static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> + void *val, size_t len)
> +{
> + u8 data[len + 1];
> +
> + if (!tps->i2c_protocol)
> + return regmap_raw_write(tps->regmap, reg, val, len);
> +
> + data[0] = len;
> + memcpy(&data[1], val, len);
> +
> + return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
> +}
> +
>  static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
>  {
>   return tps6598x_block_read(tps, reg, val, sizeof(u16));
> @@ -127,23 +141,23 @@ static inline int tps6598x_read64(struct tps6598x *tps, 
> u8 reg, u64 *val)
>  
>  static inline int tps6598x_write16(struct tps6598x *tps, u8 reg, u16 val)
>  {
> - return regmap_raw_write(tps->regmap, reg, &val, sizeof(u16));
> + return tps6598x_block_write(tps, reg, &val, sizeof(u16));
>  }
>  
>  static inline int tps6598x_write32(struct tps6598x *tps, u8 reg, u32 val)
>  {
> - return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
> + return tps6598x_block_write(tps, reg, &val, sizeof(u32));
>  }
>  
>  static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
>  {
> - return regmap_raw_write(tps->regmap, reg, &val, sizeof(u64));
> + return tps6598x_block_write(tps, reg, &val, sizeof(u64));
>  }
>  
>  static inline int
>  tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
>  {
> - return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
> + return tps6598x_block_write(tps, reg, &val, sizeof(u32));
>  }
>  
>  static int tps6598x_read_partner_identity(struct tps6598x *tps)

You need to fix tps6598x_exec_cmd() as well.

Did you really send this last September? If you did, then the mail has
been stuck somewhere for a long time.


thanks,

-- 
heikki

[PATCH 2/3] usb: gadget: atmel: support USB suspend

[Jonas Bonn]

This patch adds support for USB suspend to the Atmel UDC.

When suspended, the UDC clock can be stopped, resulting in some power
savings.  The "wake up" interrupt will fire irregardless of whether the
clock is running or not, allowing the UDC clock to be restarted when the
USB master wants to wake the device again.

The IRQ state of this device is somewhat fiddly.  The "wake up" IRQ
seems to actually be a "bus activity" indicator; the IRQ is almost
continuously asserted so enabling this IRQ should only be done after a
suspend when the wake IRQ becomes relevant.  Similarly, the "suspend"
IRQ detects "bus inactivity" and may therefore fire together with a
"wake" if the two types of activity coincide during the period between
two IRQ handler invocations; therefore, it's important to ignore the
"suspend" IRQ while waiting for a wake-up.

This has been tested on a SAMA5D2 board.

Signed-off-by: Jonas Bonn 
CC: Cristian Birsan 
CC: Felipe Balbi 
CC: Greg Kroah-Hartman 
CC: Nicolas Ferre 
CC: Alexandre Belloni 
CC: Ludovic Desroches 
CC: linux-arm-ker...@lists.infradead.org
CC: linux-usb@vger.kernel.org
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 55 ++---
 drivers/usb/gadget/udc/atmel_usba_udc.h |  1 +
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 9d18f9b2..740cb9308a86 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1703,6 +1703,9 @@ static void usba_dma_irq(struct usba_udc *udc, struct 
usba_ep *ep)
}
 }
 
+static int start_clock(struct usba_udc *udc);
+static void stop_clock(struct usba_udc *udc);
+
 static irqreturn_t usba_udc_irq(int irq, void *devid)
 {
struct usba_udc *udc = devid;
@@ -1720,10 +1723,13 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
DBG(DBG_INT, "irq, status=%#08x\n", status);
 
if (status & USBA_DET_SUSPEND) {
-   toggle_bias(udc, 0);
-   usba_writel(udc, INT_CLR, USBA_DET_SUSPEND);
+   usba_writel(udc, INT_CLR, USBA_DET_SUSPEND|USBA_WAKE_UP);
usba_int_enb_set(udc, USBA_WAKE_UP);
+   usba_int_enb_clear(udc, USBA_DET_SUSPEND);
+   udc->suspended = true;
+   toggle_bias(udc, 0);
udc->bias_pulse_needed = true;
+   stop_clock(udc);
DBG(DBG_BUS, "Suspend detected\n");
if (udc->gadget.speed != USB_SPEED_UNKNOWN
&& udc->driver && udc->driver->suspend) {
@@ -1734,14 +1740,17 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
}
 
if (status & USBA_WAKE_UP) {
+   start_clock(udc);
toggle_bias(udc, 1);
usba_writel(udc, INT_CLR, USBA_WAKE_UP);
-   usba_int_enb_clear(udc, USBA_WAKE_UP);
DBG(DBG_BUS, "Wake Up CPU detected\n");
}
 
if (status & USBA_END_OF_RESUME) {
+   udc->suspended = false;
usba_writel(udc, INT_CLR, USBA_END_OF_RESUME);
+   usba_int_enb_clear(udc, USBA_WAKE_UP);
+   usba_int_enb_set(udc, USBA_DET_SUSPEND);
generate_bias_pulse(udc);
DBG(DBG_BUS, "Resume detected\n");
if (udc->gadget.speed != USB_SPEED_UNKNOWN
@@ -1756,6 +1765,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (dma_status) {
int i;
 
+   usba_int_enb_set(udc, USBA_DET_SUSPEND);
+
for (i = 1; i <= USBA_NR_DMAS; i++)
if (dma_status & (1 << i))
usba_dma_irq(udc, &udc->usba_ep[i]);
@@ -1765,6 +1776,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (ep_status) {
int i;
 
+   usba_int_enb_set(udc, USBA_DET_SUSPEND);
+
for (i = 0; i < udc->num_ep; i++)
if (ep_status & (1 << i)) {
if (ep_is_control(&udc->usba_ep[i]))
@@ -1778,7 +1791,9 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
struct usba_ep *ep0, *ep;
int i, n;
 
-   usba_writel(udc, INT_CLR, USBA_END_OF_RESET);
+   usba_writel(udc, INT_CLR,
+   USBA_END_OF_RESET|USBA_END_OF_RESUME
+   |USBA_DET_SUSPEND|USBA_WAKE_UP);
generate_bias_pulse(udc);
reset_all_endpoints(udc);
 
@@ -1805,6 +1820,11 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
| USBA_BF(BK_NUMBER, USBA_BK_NUMBER_ONE)));
usba_ep_writel(ep0, CTL_ENB,
USBA_EPT_ENABLE | USBA_RX_SETUP);
+
+   /* If we get reset while suspended... */
+   udc->suspended = false;
+   usba_int_enb_clear(udc, USBA_WAKE_UP);
+
 

[PATCH 3/3] usb: gadget: atmel: tie wake lock to running clock

[Jonas Bonn]

If the USB device is connected to a host, the CPU cannot be suspended or
else the USB device appears to be disconnected from the host's point of
view.  Only after a "USB suspend" state has been entered (as set by the
host) or the host is disconnected can the system safely be suspended: in
both these states, the clock is stopped.  As such, this patch associates
a "wake lock" with the running clock of the UDC to keep the system awake
as long as the host maintains the USB connection active.

Signed-off-by: Jonas Bonn 
CC: Cristian Birsan 
CC: Felipe Balbi 
CC: Greg Kroah-Hartman 
CC: Nicolas Ferre 
CC: Alexandre Belloni 
CC: Ludovic Desroches 
CC: linux-arm-ker...@lists.infradead.org
CC: linux-usb@vger.kernel.org
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 740cb9308a86..864d03c3c9db 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1859,6 +1859,8 @@ static int start_clock(struct usba_udc *udc)
if (udc->clocked)
return 0;
 
+   pm_stay_awake(&udc->pdev->dev);
+
ret = clk_prepare_enable(udc->pclk);
if (ret)
return ret;
@@ -1881,6 +1883,8 @@ static void stop_clock(struct usba_udc *udc)
clk_disable_unprepare(udc->pclk);
 
udc->clocked = false;
+
+   pm_relax(&udc->pdev->dev);
 }
 
 static int usba_start(struct usba_udc *udc)
-- 
2.19.1

[PATCH 1/3] usb: gadget: atmel_usba_udc: simplify setting of interrupt-enabled mask

[Jonas Bonn]

This patch adds set and clear functions for enabling/disabling
interrupts.  This simplifies the implementation a bit as the masking of
previously set bits doesn't need to be so explicit.

Signed-off-by: Jonas Bonn 
CC: Cristian Birsan 
CC: Felipe Balbi 
CC: Greg Kroah-Hartman 
CC: Nicolas Ferre 
CC: Alexandre Belloni 
CC: Ludovic Desroches 
CC: linux-arm-ker...@lists.infradead.org
CC: linux-usb@vger.kernel.org
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 29 -
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 51a2b9232baa..9d18f9b2 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -358,8 +358,20 @@ static inline u32 usba_int_enb_get(struct usba_udc *udc)
return udc->int_enb_cache;
 }
 
-static inline void usba_int_enb_set(struct usba_udc *udc, u32 val)
+static inline void usba_int_enb_set(struct usba_udc *udc, u32 mask)
 {
+   u32 val;
+
+   val = udc->int_enb_cache | mask;
+   usba_writel(udc, INT_ENB, val);
+   udc->int_enb_cache = val;
+}
+
+static inline void usba_int_enb_clear(struct usba_udc *udc, u32 mask)
+{
+   u32 val;
+
+   val = udc->int_enb_cache & ~mask;
usba_writel(udc, INT_ENB, val);
udc->int_enb_cache = val;
 }
@@ -629,14 +641,12 @@ usba_ep_enable(struct usb_ep *_ep, const struct 
usb_endpoint_descriptor *desc)
if (ep->can_dma) {
u32 ctrl;
 
-   usba_int_enb_set(udc, usba_int_enb_get(udc) |
- USBA_BF(EPT_INT, 1 << ep->index) |
+   usba_int_enb_set(udc, USBA_BF(EPT_INT, 1 << ep->index) |
  USBA_BF(DMA_INT, 1 << ep->index));
ctrl = USBA_AUTO_VALID | USBA_INTDIS_DMA;
usba_ep_writel(ep, CTL_ENB, ctrl);
} else {
-   usba_int_enb_set(udc, usba_int_enb_get(udc) |
- USBA_BF(EPT_INT, 1 << ep->index));
+   usba_int_enb_set(udc, USBA_BF(EPT_INT, 1 << ep->index));
}
 
spin_unlock_irqrestore(&udc->lock, flags);
@@ -680,8 +690,7 @@ static int usba_ep_disable(struct usb_ep *_ep)
usba_dma_readl(ep, STATUS);
}
usba_ep_writel(ep, CTL_DIS, USBA_EPT_ENABLE);
-   usba_int_enb_set(udc, usba_int_enb_get(udc) &
- ~USBA_BF(EPT_INT, 1 << ep->index));
+   usba_int_enb_clear(udc, USBA_BF(EPT_INT, 1 << ep->index));
 
request_complete_list(ep, &req_list, -ESHUTDOWN);
 
@@ -1713,7 +1722,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (status & USBA_DET_SUSPEND) {
toggle_bias(udc, 0);
usba_writel(udc, INT_CLR, USBA_DET_SUSPEND);
-   usba_int_enb_set(udc, int_enb | USBA_WAKE_UP);
+   usba_int_enb_set(udc, USBA_WAKE_UP);
udc->bias_pulse_needed = true;
DBG(DBG_BUS, "Suspend detected\n");
if (udc->gadget.speed != USB_SPEED_UNKNOWN
@@ -1727,7 +1736,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (status & USBA_WAKE_UP) {
toggle_bias(udc, 1);
usba_writel(udc, INT_CLR, USBA_WAKE_UP);
-   usba_int_enb_set(udc, int_enb & ~USBA_WAKE_UP);
+   usba_int_enb_clear(udc, USBA_WAKE_UP);
DBG(DBG_BUS, "Wake Up CPU detected\n");
}
 
@@ -1796,7 +1805,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
| USBA_BF(BK_NUMBER, USBA_BK_NUMBER_ONE)));
usba_ep_writel(ep0, CTL_ENB,
USBA_EPT_ENABLE | USBA_RX_SETUP);
-   usba_int_enb_set(udc, int_enb | USBA_BF(EPT_INT, 1) |
+   usba_int_enb_set(udc, USBA_BF(EPT_INT, 1) |
  USBA_DET_SUSPEND | USBA_END_OF_RESUME);
 
/*
-- 
2.19.1

[PATCH 0/3] usb: gadget: atmel: support USB suspend/resume

[Jonas Bonn]

This patch series hooks up proper support for USB suspend and resume to the
Atmel UDC.

Jonas Bonn (3):
  usb: gadget: atmel_usba_udc: simplify setting of interrupt-enabled
mask
  usb: gadget: atmel: support USB suspend
  usb: gadget: atmel: tie wake lock to running clock

 drivers/usb/gadget/udc/atmel_usba_udc.c | 84 -
 drivers/usb/gadget/udc/atmel_usba_udc.h |  1 +
 2 files changed, 71 insertions(+), 14 deletions(-)

-- 
2.19.1

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with 
plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
controllers")
Signed-off-by: Nikolaus Voss 
---
v2: fix tps6598x_exec_cmd also
---
 drivers/usb/typec/tps6598x.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c84c8c189e90..c54b73fb2a2f 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
*val, size_t len)
return 0;
 }
 
+static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
+   void *val, size_t len)
+{
+   u8 data[len + 1];
+
+   if (!tps->i2c_protocol)
+   return regmap_raw_write(tps->regmap, reg, val, len);
+
+   data[0] = len;
+   memcpy(&data[1], val, len);
+
+   return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
+}
+
 static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
 {
return tps6598x_block_read(tps, reg, val, sizeof(u16));
@@ -127,23 +141,23 @@ static inline int tps6598x_read64(struct tps6598x *tps, 
u8 reg, u64 *val)
 
 static inline int tps6598x_write16(struct tps6598x *tps, u8 reg, u16 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u16));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u16));
 }
 
 static inline int tps6598x_write32(struct tps6598x *tps, u8 reg, u32 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
 }
 
 static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u64));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u64));
 }
 
 static inline int
 tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
 }
 
 static int tps6598x_read_partner_identity(struct tps6598x *tps)
@@ -229,8 +243,8 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const 
char *cmd,
return -EBUSY;
 
if (in_len) {
-   ret = regmap_raw_write(tps->regmap, TPS_REG_DATA1,
-  in_data, in_len);
+   ret = tps6598x_block_write(tps, TPS_REG_DATA1,
+  in_data, in_len);
if (ret)
return ret;
}
-- 
2.17.1

Re: [PATCH] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

Hi,

On Wed, 20 Feb 2019, Heikki Krogerus wrote:

Hi,

On Mon, Sep 10, 2018 at 07:05:01AM +0200, Nikolaus Voss wrote:

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with 
plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
controllers")
Signed-off-by: Nikolaus Voss 
---
 drivers/usb/typec/tps6598x.c | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c84c8c189e90..57a3e6c5c175 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
*val, size_t len)
return 0;
 }

+static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
+   void *val, size_t len)
+{
+   u8 data[len + 1];
+
+   if (!tps->i2c_protocol)
+   return regmap_raw_write(tps->regmap, reg, val, len);
+
+   data[0] = len;
+   memcpy(&data[1], val, len);
+
+   return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
+}
+
 static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
 {
return tps6598x_block_read(tps, reg, val, sizeof(u16));
@@ -127,23 +141,23 @@ static inline int tps6598x_read64(struct tps6598x *tps, 
u8 reg, u64 *val)

 static inline int tps6598x_write16(struct tps6598x *tps, u8 reg, u16 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u16));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u16));
 }

 static inline int tps6598x_write32(struct tps6598x *tps, u8 reg, u32 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
 }

 static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u64));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u64));
 }

 static inline int
 tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
 }

 static int tps6598x_read_partner_identity(struct tps6598x *tps)


You need to fix tps6598x_exec_cmd() as well.


Right, thanks. I will fix that.



Did you really send this last September? If you did, then the mail has
been stuck somewhere for a long time.


It's been stuck with me, I forgot to send it ;-).

Nikolaus

Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+

[Stanislaw Gruszka]

On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> >> >> The way I see it, we have two choices.
> >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> >> > 
> >> > I agree, if this is only needed for dwc2. Though I would investigate
> >> > if this is not a bug on other platforms as well.
> >> >From what I can see, using Lorenzo's patches seems to be the better
> >> solution, since they avoid these corner cases in dwc2 (and maybe other
> >> drivers as well). I will apply them and then we'll see if we need to do
> >> any further improvements later on.
> > 
> > They work on rpi dwc2, but they do not address root of the problem.
> > There is clearly something wrong how mt76usb handle SG, what is not
> > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > SG should be red flag.
> I think we're simply dealing with multiple issues here, only some of
> which are fixed by Lorenzo's patches.
> I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> since the Linux USB API supports non-aligned transfer buffers and it
> should be up to the controller driver to deal with that.

Agree.

> dwc2 tries to do that, but that has limitations which I already pointed
> out and which are properly dealt with by Lorenzo's patches.

I planed to just accept current solution, but I started to work on patch
that remove len, sglen arguments from mt76u_buf_alloc() and use
q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
related code is now tangled.

Would be ok to send this patch with proper changelog as fix for RPI
against wireless-drivers and cc:stable (assuming it works and really
fix things on RPI) and revert Lorenzo's patches in -next ?

Stanislaw

>From 4f8d7d3f4031b0a97b3bb147cb7e52533886e7cc Mon Sep 17 00:00:00 2001
From: Stanislaw Gruszka 
Date: Wed, 20 Feb 2019 13:29:42 +0100
Subject: [PATCH] mt76usb: use urb transfer_buffer for one segment

Signed-off-by: Stanislaw Gruszka 
---
 drivers/net/wireless/mediatek/mt76/mt76.h |  2 +
 .../wireless/mediatek/mt76/mt76x02_usb_mcu.c  |  4 +-
 drivers/net/wireless/mediatek/mt76/usb.c  | 75 +++
 3 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h 
b/drivers/net/wireless/mediatek/mt76/mt76.h
index 2e5bcb3fdff7..7e0680daeee6 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -86,6 +86,8 @@ struct mt76_queue_buf {
 struct mt76u_buf {
struct mt76_dev *dev;
struct urb *urb;
+   struct scatterlist *sg;
+   int num_sgs;
size_t len;
bool done;
 };
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c 
b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
index da299b8a1334..75561910d630 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
@@ -90,7 +90,7 @@ static int mt76x02u_mcu_wait_resp(struct mt76_dev *dev, u8 
seq)
if (urb->status)
return -EIO;
 
-   data = sg_virt(&urb->sg[0]);
+   data = sg_virt(&buf->sg[0]);
if (usb->mcu.rp)
mt76x02u_multiple_mcu_reads(dev, data + 4,
urb->actual_length - 8);
@@ -266,7 +266,7 @@ static int
 __mt76x02u_mcu_fw_send_data(struct mt76x02_dev *dev, struct mt76u_buf *buf,
const void *fw_data, int len, u32 dst_addr)
 {
-   u8 *data = sg_virt(&buf->urb->sg[0]);
+   u8 *data = sg_virt(&buf->sg[0]);
DECLARE_COMPLETION_ONSTACK(cmpl);
__le32 info;
u32 val;
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c 
b/drivers/net/wireless/mediatek/mt76/usb.c
index a1811c39415e..57bb16eaff06 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -277,7 +277,6 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf 
*buf,
 int nsgs, int len, int sglen)
 {
struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
-   struct urb *urb = buf->urb;
int i;
 
spin_lock_bh(&q->rx_page_lock);
@@ -292,21 +291,21 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf 
*buf,
 
page = virt_to_head_page(data);
offset = data - page_address(page);
-   sg_set_page(&urb->sg[i], page, sglen, offset);
+   sg_set_page(&buf->sg[i], page, sglen, offset);
}
spin_unlock_bh(&q->rx_page_lock);
 
if (i < nsgs) {
int j;
 
-   for (j = nsgs; j < urb->num_sgs; j++)
-   skb_free_frag(sg_virt(&urb->sg[j]));
-   urb->num_sgs = i;
+   for (j = nsgs; j < buf->num_sgs; j++)
+   skb_free_frag(sg_virt(&buf->sg[j]));
+   b

Re: [PATCH v4 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

[Roger Quadros]

Pawel,

On 14/02/2019 21:45, Pawel Laszczak wrote:
> This patch introduce new Cadence USBSS DRD driver to linux kernel.
> 
> The Cadence USBSS DRD Driver is a highly configurable IP Core whichi
> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
> Host Only (XHCI)configurations.
> 
> The current driver has been validated with FPGA burned. We have support
> for PCIe bus, which is used on FPGA prototyping.
> 
> The host side of USBSS-DRD controller is compliance with XHCI
> specification, so it works with standard XHCI linux driver.
> 
> Signed-off-by: Pawel Laszczak 
> ---
>  drivers/usb/Kconfig|2 +
>  drivers/usb/Makefile   |2 +
>  drivers/usb/cdns3/Kconfig  |   44 +
>  drivers/usb/cdns3/Makefile |   14 +
>  drivers/usb/cdns3/cdns3-pci-wrap.c |  155 +++
>  drivers/usb/cdns3/core.c   |  403 ++
>  drivers/usb/cdns3/core.h   |  116 ++
>  drivers/usb/cdns3/debug.h  |  168 +++
>  drivers/usb/cdns3/debugfs.c|  164 +++
>  drivers/usb/cdns3/drd.c|  365 +
>  drivers/usb/cdns3/drd.h|  162 +++
>  drivers/usb/cdns3/ep0.c|  907 +
>  drivers/usb/cdns3/gadget-export.h  |   28 +
>  drivers/usb/cdns3/gadget.c | 2003 
>  drivers/usb/cdns3/gadget.h | 1207 +
>  drivers/usb/cdns3/host-export.h|   28 +
>  drivers/usb/cdns3/host.c   |   72 +
>  drivers/usb/cdns3/trace.c  |   23 +
>  drivers/usb/cdns3/trace.h  |  404 ++
>  19 files changed, 6267 insertions(+)
>  create mode 100644 drivers/usb/cdns3/Kconfig
>  create mode 100644 drivers/usb/cdns3/Makefile
>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>  create mode 100644 drivers/usb/cdns3/core.c
>  create mode 100644 drivers/usb/cdns3/core.h
>  create mode 100644 drivers/usb/cdns3/debug.h
>  create mode 100644 drivers/usb/cdns3/debugfs.c
>  create mode 100644 drivers/usb/cdns3/drd.c
>  create mode 100644 drivers/usb/cdns3/drd.h
>  create mode 100644 drivers/usb/cdns3/ep0.c
>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>  create mode 100644 drivers/usb/cdns3/gadget.c
>  create mode 100644 drivers/usb/cdns3/gadget.h
>  create mode 100644 drivers/usb/cdns3/host-export.h
>  create mode 100644 drivers/usb/cdns3/host.c
>  create mode 100644 drivers/usb/cdns3/trace.c
>  create mode 100644 drivers/usb/cdns3/trace.h
> 
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 987fc5ba6321..5f9334019d04 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -112,6 +112,8 @@ source "drivers/usb/usbip/Kconfig"
>  
>  endif
>  
> +source "drivers/usb/cdns3/Kconfig"
> +
>  source "drivers/usb/mtu3/Kconfig"
>  
>  source "drivers/usb/musb/Kconfig"
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 7d1b8c82b208..ab125b966cac 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -12,6 +12,8 @@ obj-$(CONFIG_USB_DWC3)  += dwc3/
>  obj-$(CONFIG_USB_DWC2)   += dwc2/
>  obj-$(CONFIG_USB_ISP1760)+= isp1760/
>  
> +obj-$(CONFIG_USB_CDNS3)  += cdns3/
> +
>  obj-$(CONFIG_USB_MON)+= mon/
>  obj-$(CONFIG_USB_MTU3)   += mtu3/
>  
> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
> new file mode 100644
> index ..27cb3d8dbe3d
> --- /dev/null
> +++ b/drivers/usb/cdns3/Kconfig
> @@ -0,0 +1,44 @@
> +config USB_CDNS3
> + tristate "Cadence USB3 Dual-Role Controller"
> + depends on USB_SUPPORT && (USB || USB_GADGET) && HAS_DMA
> + help
> +   Say Y here if your system has a cadence USB3 dual-role controller.
> +   It supports: dual-role switch, Host-only, and Peripheral-only.
> +
> +   If you choose to build this driver is a dynamically linked
> +   as module, the module will be called cdns3.ko.
> +
> +if USB_CDNS3
> +
> +config USB_CDNS3_GADGET
> +bool "Cadence USB3 device controller"
> +depends on USB_GADGET
> +help
> +  Say Y here to enable device controller functionality of the
> +  cadence USBSS-DEV driver.

"Cadence" ?

> +
> +  This controller supports FF, HS and SS mode. It doesn't support
> +  LS and SSP mode.
> +
> +config USB_CDNS3_HOST
> +bool "Cadence USB3 host controller"
> +depends on USB_XHCI_HCD
> +help
> +  Say Y here to enable host controller functionality of the
> +  cadence driver.
> +
> +  Host controller is compliant with XHCI so it will use
> +  standard XHCI driver.
> +
> +config USB_CDNS3_PCI_WRAP
> + tristate "Cadence USB3 support on PCIe-based platforms"
> + depends on USB_PCI && ACPI
> + default USB_CDNS3
> + help
> +   If you're using the USBSS Core IP with a PCIe, please say
> +   'Y' or 'M' here.
> +
> +   If you choose to build this driver as module it will
> +   be dynamically linked and module will be

Re: usb: over-current condition

[Roger Quadros]

On 20/02/2019 13:00, Subhashini Rao Beerisetty wrote:
> On Tue, Feb 19, 2019 at 8:30 PM Roger Quadros  wrote:
>>
>> Please keep linux-usb in cc.
>>
>> On 19/02/2019 16:50, Subhashini Rao Beerisetty wrote:
>>> On Tue, Feb 19, 2019 at 8:06 PM Roger Quadros  wrote:

 Hi Subhashini,

 On 19/02/2019 16:30, Subhashini Rao Beerisetty wrote:
> Hi All,
>
> [ Please keep me in CC as I'm not subscribed to the list]
>
>
> I’m using an Ubuntu 16.04.4 LTS PC and when I connect an USB device to
> my Ubuntu PC, I see the below mentioned messages in dmesg.
>
>
>
> [ 9012.287432] usb usb3-port1: over-current condition
>
> [ 9012.287739] usb usb2-port1: over-current condition
>
> [ 9012.495919] usb 2-4: USB disconnect, device number 3
>
> [ 9012.495940] usb 2-4.1: USB disconnect, device number 5
>
> [ 9012.615609] usb usb3-port1: over-current condition
>
> [ 9012.628513] usb 2-4.2: USB disconnect, device number 7
>
> [ 9012.824274] usb usb3-port1: over-current condition
>
> [ 9015.304232] hub_port_connect: 12 callbacks suppressed
>
> [ 9015.304254] usb usb2-port4: connect-debounce failed
>
> [ 9015.512437] usb usb2-port1: over-current condition
>
> [ 9015.720767] usb usb2-port2: over-current condition
>
>
>
> I’d like to What these messages indicates? Is it a hardware problem
> from host side(ubuntu PC) or device side?

 Can you try other USB devices?
>>> Yes, on the same port other devices works fine. I tried connecting
>>> Keyboard and it works fine. Basically I'm looking for how to debug
>>> further for this type of issues.
>>
>> The device is drawing too much current which is triggering
>> the overcurrent detection/protection circuit on the USB hub.
>>
>> What device is it? If it is an off the shelf device it might indicate
>> a hardware fault and so time to buy a new one?
> It is an embedded board. From Host PC, we are communicating to PIC on
> embedded board via USB port.

OK. You might want to check for support from the embedded board manufacturer.

-- 
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

[Roger Quadros]

Pawel,

On 20/02/2019 13:18, Pawel Laszczak wrote:
> Hi Roger.
>>
>> On 14/02/2019 21:45, Pawel Laszczak wrote:
>>> Controller for OUT endpoints has shared on-chip buffers for all incoming
>>> packets, including ep0out. It's FIFO buffer, so packets must be handle
>>> by DMA in correct order. If the first packet in the buffer will not be
>>> handled, then the following packets directed for other endpoints and
>>> functions will be blocked.
>>>
>>> Additionally the packets directed to one endpoint can block entire on-chip
>>> buffers. In this case transfer to other endpoints also will blocked.
>>>
>>> To resolve this issue after raising the descriptor missing interrupt
>>> driver prepares internal usb_request object and use it to arm DMA
>>> transfer.
>>>
>>> The problematic situation was observed in case when endpoint has
>>> been enabled but no usb_request were queued. Driver try detects
>>> such endpoints and will use this workaround only for these endpoint.
>>>
>>> Driver use limited number of buffer. This number can be set by macro
>>> CDNS_WA2_NUM_BUFFERS.
>>>
>>> Such blocking situation was observed on ACM gadget. For this function
>>> host send OUT data packet but ACM function is not prepared for
>>> this packet. It's cause that buffer placed in on chip memory block
>>> transfer to other endpoints.
>>>
>>> It's limitation of controller but maybe this issues should be fixed in
>>> function driver.
>>>
>>> This work around can be disabled/enabled by means of quirk_internal_buffer
>>> module parameter. By default feature is enabled. It can has impact to
>>> transfer performance and in most case this feature can be disabled.
>>
>> How much is the performance impact?
> 
> I didn't check this, but performance will be decreased because driver has to 
> copy data from internally allocated buffer to usb_request->buff.
> 
>> You mentioned that the issue was observed only in the ACM case and
>> could be fixed in the function driver?
>> It seems pointless to enable an endpoint and not have any requests queued 
>> for it.
> 
> Yes, it’s true. The request in ACM class is send to Controller Driver when 
> user read file associated 
> with ACM gadget. Problem exist because host send data to controller but USB 
> controller 
> has not prepared buffer for this data by ACM class.
> 
>> Isn't fixing the ACM driver (if there is a real issue) a better approach 
>> than having
>> a workaround that affects performance of all other use cases?
> 
> Yes it should be better. But what ACM driver should do with unexpected data. 
> I'm not sure if we 
> can simply delete them. 
> 
> The best solution would be to make modification in controller. In such case 
> Controller shouldn't accept 
> packet but should send NAK. 

Yes, that should be standard behaviour. No?

>  
> One more thing. Workaround has implemented algorithm that decide for which 
> endpoint it should be enabled.
> e.g for composite device MSC+NCM+ACM it should work only for ACM OUT endpoint.
> 

If ACM driver didn't queue the request for ACM OUT endpoint, why does the 
controller accept the data at all?
I didn't understand why we need a workaround for this. It should be standard 
behaviour to NAK data if
function driver didn't request for all endpoints.

Also I didn't understand why performance should be impacted to just NAK data.

cheers,
-roger

> Cheers,
> Pawel
>>
>>>
>>> Signed-off-by: Pawel Laszczak 
>>> ---
>>>  drivers/usb/cdns3/gadget.c | 273 -
>>>  drivers/usb/cdns3/gadget.h |   7 +
>>>  2 files changed, 278 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>>> index 7f7f24ee3c4b..5dfbe6e1421c 100644
>>> --- a/drivers/usb/cdns3/gadget.c
>>> +++ b/drivers/usb/cdns3/gadget.c
>>> @@ -27,6 +27,37 @@
>>>   * If (((Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr-1) or
>>>   * (Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr))
>>>   * and (DRBL==1 and (not EP0)))
>>> + *
>>> + * Work around 2:
>>> + * Controller for OUT endpoints has shared on-chip buffers for all incoming
>>> + * packets, including ep0out. It's FIFO buffer, so packets must be handle 
>>> by DMA
>>> + * in correct order. If the first packet in the buffer will not be handled,
>>> + * then the following packets directed for other endpoints and  functions
>>> + * will be blocked.
>>> + * Additionally the packets directed to one endpoint can block entire 
>>> on-chip
>>> + * buffers. In this case transfer to other endpoints also will blocked.
>>> + *
>>> + * To resolve this issue after raising the descriptor missing interrupt
>>> + * driver prepares internal usb_request object and use it to arm DMA 
>>> transfer.
>>> + *
>>> + * The problematic situation was observed in case when endpoint has been 
>>> enabled
>>> + * but no usb_request were queued. Driver try detects such endpoints and 
>>> will
>>> + * use this workaround only for these endpoint.
>>> + *
>>> + * Driver use limited number of buffer. This num

Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+

[Lorenzo Bianconi]

> On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > >> >> The way I see it, we have two choices.
> > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > >> > 
> > >> > I agree, if this is only needed for dwc2. Though I would investigate
> > >> > if this is not a bug on other platforms as well.
> > >> >From what I can see, using Lorenzo's patches seems to be the better
> > >> solution, since they avoid these corner cases in dwc2 (and maybe other
> > >> drivers as well). I will apply them and then we'll see if we need to do
> > >> any further improvements later on.
> > > 
> > > They work on rpi dwc2, but they do not address root of the problem.
> > > There is clearly something wrong how mt76usb handle SG, what is not
> > > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > > SG should be red flag.
> > I think we're simply dealing with multiple issues here, only some of
> > which are fixed by Lorenzo's patches.
> > I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> > since the Linux USB API supports non-aligned transfer buffers and it
> > should be up to the controller driver to deal with that.
> 
> Agree.
> 
> > dwc2 tries to do that, but that has limitations which I already pointed
> > out and which are properly dealt with by Lorenzo's patches.
> 
> I planed to just accept current solution, but I started to work on patch
> that remove len, sglen arguments from mt76u_buf_alloc() and use
> q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
> related code is now tangled.
> 
> Would be ok to send this patch with proper changelog as fix for RPI
> against wireless-drivers and cc:stable (assuming it works and really
> fix things on RPI) and revert Lorenzo's patches in -next ?

Hi Stanislaw,

what is the advantage of doing so? You have duplicated most of the fields that 
are
already in the urb data structure and you use transfer_buffer (no SG I/O).
Moreover I have ready a series that removes 99% of the dual allocation code and
maintain it in the control path (instead of the datapath one).
I need just to rebase it ontop of your series. I will post it soon.

Regards,
Lorenzo

> 
> Stanislaw
> 
> From 4f8d7d3f4031b0a97b3bb147cb7e52533886e7cc Mon Sep 17 00:00:00 2001
> From: Stanislaw Gruszka 
> Date: Wed, 20 Feb 2019 13:29:42 +0100
> Subject: [PATCH] mt76usb: use urb transfer_buffer for one segment
> 
> Signed-off-by: Stanislaw Gruszka 
> ---
>  drivers/net/wireless/mediatek/mt76/mt76.h |  2 +
>  .../wireless/mediatek/mt76/mt76x02_usb_mcu.c  |  4 +-
>  drivers/net/wireless/mediatek/mt76/usb.c  | 75 +++
>  3 files changed, 46 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h 
> b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 2e5bcb3fdff7..7e0680daeee6 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -86,6 +86,8 @@ struct mt76_queue_buf {
>  struct mt76u_buf {
>   struct mt76_dev *dev;
>   struct urb *urb;
> + struct scatterlist *sg;
> + int num_sgs;
>   size_t len;
>   bool done;
>  };
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c 
> b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
> index da299b8a1334..75561910d630 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
> @@ -90,7 +90,7 @@ static int mt76x02u_mcu_wait_resp(struct mt76_dev *dev, u8 
> seq)
>   if (urb->status)
>   return -EIO;
>  
> - data = sg_virt(&urb->sg[0]);
> + data = sg_virt(&buf->sg[0]);
>   if (usb->mcu.rp)
>   mt76x02u_multiple_mcu_reads(dev, data + 4,
>   urb->actual_length - 8);
> @@ -266,7 +266,7 @@ static int
>  __mt76x02u_mcu_fw_send_data(struct mt76x02_dev *dev, struct mt76u_buf *buf,
>   const void *fw_data, int len, u32 dst_addr)
>  {
> - u8 *data = sg_virt(&buf->urb->sg[0]);
> + u8 *data = sg_virt(&buf->sg[0]);
>   DECLARE_COMPLETION_ONSTACK(cmpl);
>   __le32 info;
>   u32 val;
> diff --git a/drivers/net/wireless/mediatek/mt76/usb.c 
> b/drivers/net/wireless/mediatek/mt76/usb.c
> index a1811c39415e..57bb16eaff06 100644
> --- a/drivers/net/wireless/mediatek/mt76/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> @@ -277,7 +277,6 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf 
> *buf,
>int nsgs, int len, int sglen)
>  {
>   struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
> - struct urb *urb = buf->urb;
>   int i;
>  
>   spin_lock_bh(&q->rx_page_lock);
> @@ -292,21 +291,21 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf 
> *buf,
>  
>   page = virt_to_head_page(dat

Re: [PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Heikki Krogerus]

On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:
> Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
> adapters, but the problem described with regmap-i2c not handling
> SMBus block transfers (i.e. read and writes) correctly also exists
> with writes.
> 
> As workaround, this patch adds a block write function the same way
> 1a2f474d328f adds a block read function.
> 
> Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately 
> with plain-I2C adapters")
> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
> controllers")
> Signed-off-by: Nikolaus Voss 

You are missing a "from" line with address that matches your SoB
address.


thanks,

-- 
heikki

Re: [PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

On Wed, 20 Feb 2019, Heikki Krogerus wrote:

On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with 
plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
controllers")
Signed-off-by: Nikolaus Voss 


You are missing a "from" line with address that matches your SoB
address.


That's because I currently cannot send patch mails from my company 
account as our MTA breaks diffs. You could add


Signed-off-by: Nikolaus Voss 

Nikolaus

[PATCH] xhci: tegra: Prevent error pointer dereference

[Thierry Reding]

From: Thierry Reding 

During initialization, the host and super-speed power domains will
contain an ERR_PTR() encoded error code rather than being NULL. To
avoid a crash, use a !IS_ERR_OR_NULL() condition during cleanup.

Signed-off-by: Thierry Reding 
---
 drivers/usb/host/xhci-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 938ff06c0349..efb0cad8710e 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct device 
*dev,
device_link_del(tegra->genpd_dl_ss);
if (tegra->genpd_dl_host)
device_link_del(tegra->genpd_dl_host);
-   if (tegra->genpd_dev_ss)
+   if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss))
dev_pm_domain_detach(tegra->genpd_dev_ss, true);
-   if (tegra->genpd_dev_host)
+   if (!IS_ERR_OR_NULL(tegra->genpd_dev_host))
dev_pm_domain_detach(tegra->genpd_dev_host, true);
 }
 
-- 
2.20.1

Re: [PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Heikki Krogerus]

On Wed, Feb 20, 2019 at 02:38:47PM +0100, Nikolaus Voss wrote:
> On Wed, 20 Feb 2019, Heikki Krogerus wrote:
> > On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:
> > > Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
> > > adapters, but the problem described with regmap-i2c not handling
> > > SMBus block transfers (i.e. read and writes) correctly also exists
> > > with writes.
> > > 
> > > As workaround, this patch adds a block write function the same way
> > > 1a2f474d328f adds a block read function.
> > > 
> > > Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately 
> > > with plain-I2C adapters")
> > > Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power 
> > > Delivery controllers")
> > > Signed-off-by: Nikolaus Voss 
> > 
> > You are missing a "from" line with address that matches your SoB
> > address.
> 
> That's because I currently cannot send patch mails from my company account
> as our MTA breaks diffs.

I understand, but you can have a separate "From line" in your patch,
i.e. you send the patch using one address, and have an extra "From
line" (outside of the mail header) with another address.

That other From line will be interpreted as the author address, and
it should match your SoB address.


Try something like this in a branch where this patch is the HEAD:

% export MY_COMMIT=$(git show -s --pretty=%h HEAD)
% git reset HEAD^
% GIT_COMMITTER_IDENT='Nikolaus Voss 
' \
  GIT_AUTHOR_IDENT='Nikolaus Voss 
' \
  git commit -a -C $MY_COMMIT

Then:

% git format-patch HEAD^
% git send-email ...

thanks,

-- 
heikki

Re: [PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Heikki Krogerus]

On Wed, Feb 20, 2019 at 04:14:23PM +0200, Heikki Krogerus wrote:
> On Wed, Feb 20, 2019 at 02:38:47PM +0100, Nikolaus Voss wrote:
> > On Wed, 20 Feb 2019, Heikki Krogerus wrote:
> > > On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:
> > > > Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
> > > > adapters, but the problem described with regmap-i2c not handling
> > > > SMBus block transfers (i.e. read and writes) correctly also exists
> > > > with writes.
> > > > 
> > > > As workaround, this patch adds a block write function the same way
> > > > 1a2f474d328f adds a block read function.
> > > > 
> > > > Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads 
> > > > separately with plain-I2C adapters")
> > > > Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power 
> > > > Delivery controllers")
> > > > Signed-off-by: Nikolaus Voss 
> > > 
> > > You are missing a "from" line with address that matches your SoB
> > > address.
> > 
> > That's because I currently cannot send patch mails from my company account
> > as our MTA breaks diffs.
> 
> I understand, but you can have a separate "From line" in your patch,
> i.e. you send the patch using one address, and have an extra "From
> line" (outside of the mail header) with another address.
> 
> That other From line will be interpreted as the author address, and
> it should match your SoB address.
> 
> 
> Try something like this in a branch where this patch is the HEAD:
> 
> % export MY_COMMIT=$(git show -s --pretty=%h HEAD)
> % git reset HEAD^
> % GIT_COMMITTER_IDENT='Nikolaus Voss 
> ' \
>   GIT_AUTHOR_IDENT='Nikolaus Voss 
> ' \
>   git commit -a -C $MY_COMMIT

Correction here:

% GIT_COMMITTER_IDENT='Nikolaus Voss 
' \
  GIT_AUTHOR_IDENT='Nikolaus Voss 
' \
  git commit -a -C $MY_COMMIT --reset-author

That "--reset-author" was missing. Sorry for that.


thanks,

-- 
heikki

Re: [PATCH] xhci: tegra: Prevent error pointer dereference

[Jon Hunter]

On 20/02/2019 13:48, Thierry Reding wrote:
> From: Thierry Reding 
> 
> During initialization, the host and super-speed power domains will
> contain an ERR_PTR() encoded error code rather than being NULL. To
> avoid a crash, use a !IS_ERR_OR_NULL() condition during cleanup.
> 
> Signed-off-by: Thierry Reding 
> ---
>  drivers/usb/host/xhci-tegra.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 938ff06c0349..efb0cad8710e 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct device 
> *dev,
>   device_link_del(tegra->genpd_dl_ss);
>   if (tegra->genpd_dl_host)
>   device_link_del(tegra->genpd_dl_host);
> - if (tegra->genpd_dev_ss)
> + if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss))
>   dev_pm_domain_detach(tegra->genpd_dev_ss, true);
> - if (tegra->genpd_dev_host)
> + if (!IS_ERR_OR_NULL(tegra->genpd_dev_host))
>   dev_pm_domain_detach(tegra->genpd_dev_host, true);
>  }

Reviewed-by: Jon Hunter 

Thanks for fixing!
Jon

-- 
nvpublic

Re: [PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Guenter Roeck]

On 2/20/19 4:57 AM, Nikolaus Voss wrote:

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with 
plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
controllers")
Signed-off-by: Nikolaus Voss 
---
v2: fix tps6598x_exec_cmd also
---
  drivers/usb/typec/tps6598x.c | 26 --
  1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c84c8c189e90..c54b73fb2a2f 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
*val, size_t len)
return 0;
  }
  
+static int tps6598x_block_write(struct tps6598x *tps, u8 reg,

+   void *val, size_t len)
+{
+   u8 data[len + 1];
+


You should use TPS_MAX_LEN + 1 here to avoid the variable length array.
See upstream commit 0bb95f80a38f8 ("Makefile: Globally enable VLA warning")
and 8d361fa2c29dc ("usb: typec: tps6598x: Remove VLA usage"). Not sure if
the WARN_ON introduced by 8d361fa2c29dc is really needed; I dislike
unnecessary runtime checks.

Guenter


+   if (!tps->i2c_protocol)
+   return regmap_raw_write(tps->regmap, reg, val, len);
+
+   data[0] = len;
+   memcpy(&data[1], val, len);
+
+   return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
+}
+
  static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
  {
return tps6598x_block_read(tps, reg, val, sizeof(u16));
@@ -127,23 +141,23 @@ static inline int tps6598x_read64(struct tps6598x *tps, 
u8 reg, u64 *val)
  
  static inline int tps6598x_write16(struct tps6598x *tps, u8 reg, u16 val)

  {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u16));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u16));
  }
  
  static inline int tps6598x_write32(struct tps6598x *tps, u8 reg, u32 val)

  {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
  }
  
  static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)

  {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u64));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u64));
  }
  
  static inline int

  tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
  {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
  }
  
  static int tps6598x_read_partner_identity(struct tps6598x *tps)

@@ -229,8 +243,8 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const 
char *cmd,
return -EBUSY;
  
  	if (in_len) {

-   ret = regmap_raw_write(tps->regmap, TPS_REG_DATA1,
-  in_data, in_len);
+   ret = tps6598x_block_write(tps, TPS_REG_DATA1,
+  in_data, in_len);
if (ret)
return ret;
}

Re: [PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Greg Kroah-Hartman]

On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:
> Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
> adapters, but the problem described with regmap-i2c not handling
> SMBus block transfers (i.e. read and writes) correctly also exists
> with writes.
> 
> As workaround, this patch adds a block write function the same way
> 1a2f474d328f adds a block read function.
> 
> Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately 
> with plain-I2C adapters")
> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
> controllers")
> Signed-off-by: Nikolaus Voss 
> ---

As was pointed out, you have to have a From: that matches a
signed-off-by somewhere here.  If your company email systems is horrid
and can not handle patches, then put the correct from: line as the first
line of the commit message as the documentation says and all will be
good.



> v2: fix tps6598x_exec_cmd also
> ---
>  drivers/usb/typec/tps6598x.c | 26 --
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index c84c8c189e90..c54b73fb2a2f 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
> *val, size_t len)
>   return 0;
>  }
>  
> +static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> + void *val, size_t len)
> +{
> + u8 data[len + 1];

I thought the build system now warned when you did this :(

thanks,

greg k-h

Re: [PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

On Wed, 20 Feb 2019, Heikki Krogerus wrote:

On Wed, Feb 20, 2019 at 04:14:23PM +0200, Heikki Krogerus wrote:

On Wed, Feb 20, 2019 at 02:38:47PM +0100, Nikolaus Voss wrote:

On Wed, 20 Feb 2019, Heikki Krogerus wrote:

On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with 
plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
controllers")
Signed-off-by: Nikolaus Voss 


You are missing a "from" line with address that matches your SoB
address.


That's because I currently cannot send patch mails from my company account
as our MTA breaks diffs.


I understand, but you can have a separate "From line" in your patch,
i.e. you send the patch using one address, and have an extra "From
line" (outside of the mail header) with another address.

That other From line will be interpreted as the author address, and
it should match your SoB address.


Try something like this in a branch where this patch is the HEAD:

% export MY_COMMIT=$(git show -s --pretty=%h HEAD)
% git reset HEAD^
% GIT_COMMITTER_IDENT='Nikolaus Voss 
' \
  GIT_AUTHOR_IDENT='Nikolaus Voss 
' \
  git commit -a -C $MY_COMMIT


Correction here:

   % GIT_COMMITTER_IDENT='Nikolaus Voss 
' \
 GIT_AUTHOR_IDENT='Nikolaus Voss ' 
\
 git commit -a -C $MY_COMMIT --reset-author

That "--reset-author" was missing. Sorry for that.


Thanks, Heikki, I'll give it a try...

Nikolaus

[PATCHv3] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

From: Nikolaus Voss 

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with 
plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
controllers")
Signed-off-by: Nikolaus Voss 
---
v2: fix tps6598x_exec_cmd also
v3: use fixed length for stack buffer
---
 drivers/usb/typec/tps6598x.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c84c8c189e90..eb8046f87a54 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
*val, size_t len)
return 0;
 }
 
+static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
+   void *val, size_t len)
+{
+   u8 data[TPS_MAX_LEN + 1];
+
+   if (!tps->i2c_protocol)
+   return regmap_raw_write(tps->regmap, reg, val, len);
+
+   data[0] = len;
+   memcpy(&data[1], val, len);
+
+   return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
+}
+
 static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
 {
return tps6598x_block_read(tps, reg, val, sizeof(u16));
@@ -127,23 +141,23 @@ static inline int tps6598x_read64(struct tps6598x *tps, 
u8 reg, u64 *val)
 
 static inline int tps6598x_write16(struct tps6598x *tps, u8 reg, u16 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u16));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u16));
 }
 
 static inline int tps6598x_write32(struct tps6598x *tps, u8 reg, u32 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
 }
 
 static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u64));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u64));
 }
 
 static inline int
 tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
 {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
 }
 
 static int tps6598x_read_partner_identity(struct tps6598x *tps)
@@ -229,8 +243,8 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const 
char *cmd,
return -EBUSY;
 
if (in_len) {
-   ret = regmap_raw_write(tps->regmap, TPS_REG_DATA1,
-  in_data, in_len);
+   ret = tps6598x_block_write(tps, TPS_REG_DATA1,
+  in_data, in_len);
if (ret)
return ret;
}
-- 
2.17.1

Re: [PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

On Wed, 20 Feb 2019, Guenter Roeck wrote:

On 2/20/19 4:57 AM, Nikolaus Voss wrote:

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately 
with plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
controllers")

Signed-off-by: Nikolaus Voss 
---
v2: fix tps6598x_exec_cmd also
---
  drivers/usb/typec/tps6598x.c | 26 --
  1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c84c8c189e90..c54b73fb2a2f 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
*val, size_t len)

return 0;
  }
  +static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
+   void *val, size_t len)
+{
+   u8 data[len + 1];
+


You should use TPS_MAX_LEN + 1 here to avoid the variable length array.
See upstream commit 0bb95f80a38f8 ("Makefile: Globally enable VLA warning")
and 8d361fa2c29dc ("usb: typec: tps6598x: Remove VLA usage"). Not sure if
the WARN_ON introduced by 8d361fa2c29dc is really needed; I dislike
unnecessary runtime checks.


Thanks for the pointer, I fixed this...

Nikolaus

Re: [PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

On Wed, 20 Feb 2019, Greg Kroah-Hartman wrote:

On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with 
plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
controllers")
Signed-off-by: Nikolaus Voss 
---


As was pointed out, you have to have a From: that matches a
signed-off-by somewhere here.  If your company email systems is horrid
and can not handle patches, then put the correct from: line as the first
line of the commit message as the documentation says and all will be
good.




v2: fix tps6598x_exec_cmd also
---
 drivers/usb/typec/tps6598x.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c84c8c189e90..c54b73fb2a2f 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
*val, size_t len)
return 0;
 }

+static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
+   void *val, size_t len)
+{
+   u8 data[len + 1];


I thought the build system now warned when you did this :(


I must admit I'm developing on 4.19 stable series, so no warnings...

Nikolaus

Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+

[Alan Stern]

On Wed, 20 Feb 2019, Stanislaw Gruszka wrote:

> On Tue, Feb 19, 2019 at 10:40:47AM -0500, Alan Stern wrote:
> > On Tue, 19 Feb 2019, Stanislaw Gruszka wrote:
> > 
> > > It would be interesting why urb->num_sgs = 0 & urb->sg cause
> > > the troubles. This is how usb_sg_init() submit urbs for sg_tablesize = 0
> > > controllers. So either are there are some requirement on urb->sg
> > > mapped via dma_map_page() (which mt76usb does not meet) not needed
> > > for urb->transfer_buffer mapped via dma_map_single() or there
> > > is something wrong in dwc2 with sg and this driver will not
> > > work with urb_sg_init() as well. I don't have hardware to investigate
> > > this and don't want to bother you with more patches.
> > 
> > urb->sg != NULL and urb->num_sgs == 0 means that the transfer buffer 
> > must fit into a single page, which is pointed to by the first element 
> > of the scatterlist.
> 
> I asked about that in other thread 
> https://lore.kernel.org/linux-wireless/2cc5674a-a3a0-d8fe-65f5-4357da9b8...@arm.com/
> 
> the answer was it is weird but valid. However I think do dma_map_page()
> on buffer not fit in the single page is asking for troubles. I just posted
> patch that should fix this for mt76usb.
> 
> > But now that I look at the code in usb_sg_init(), it seems odd that the
> > !use_sg case doesn't increment sg during each loop iteration.  I don't
> > see how that could possibly work -- it looks like a bug!
> 
> Looks for me that this is done via for_each_sg(sg, sg, io->entries, i) loop.

Ah, of course.  Thanks for straightening me out; it's surprising what 
blind spots one's brain can develop.

Alan Stern

Re: [PATCHv3] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Guenter Roeck]

On 2/20/19 7:11 AM, Nikolaus Voss wrote:

From: Nikolaus Voss 

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with 
plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
controllers")
Signed-off-by: Nikolaus Voss 


Reviewed-by: Guenter Roeck 

Note that tps6598x_exec_cmd() is only called with in_len == out_len == 0
and NULL data pointers. It might make sense to simplify the function and
drop the unused parameters as well as the associated code.

Guenter


---
v2: fix tps6598x_exec_cmd also
v3: use fixed length for stack buffer
---
  drivers/usb/typec/tps6598x.c | 26 --
  1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c84c8c189e90..eb8046f87a54 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
*val, size_t len)
return 0;
  }
  
+static int tps6598x_block_write(struct tps6598x *tps, u8 reg,

+   void *val, size_t len)
+{
+   u8 data[TPS_MAX_LEN + 1];
+
+   if (!tps->i2c_protocol)
+   return regmap_raw_write(tps->regmap, reg, val, len);
+
+   data[0] = len;
+   memcpy(&data[1], val, len);
+
+   return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
+}
+
  static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
  {
return tps6598x_block_read(tps, reg, val, sizeof(u16));
@@ -127,23 +141,23 @@ static inline int tps6598x_read64(struct tps6598x *tps, 
u8 reg, u64 *val)
  
  static inline int tps6598x_write16(struct tps6598x *tps, u8 reg, u16 val)

  {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u16));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u16));
  }
  
  static inline int tps6598x_write32(struct tps6598x *tps, u8 reg, u32 val)

  {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
  }
  
  static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)

  {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u64));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u64));
  }
  
  static inline int

  tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
  {
-   return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+   return tps6598x_block_write(tps, reg, &val, sizeof(u32));
  }
  
  static int tps6598x_read_partner_identity(struct tps6598x *tps)

@@ -229,8 +243,8 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const 
char *cmd,
return -EBUSY;
  
  	if (in_len) {

-   ret = regmap_raw_write(tps->regmap, TPS_REG_DATA1,
-  in_data, in_len);
+   ret = tps6598x_block_write(tps, TPS_REG_DATA1,
+  in_data, in_len);
if (ret)
return ret;
}

Re: [PATCHv3] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Guenter Roeck]

On 2/20/19 7:35 AM, Guenter Roeck wrote:

On 2/20/19 7:11 AM, Nikolaus Voss wrote:

From: Nikolaus Voss 

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with 
plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
controllers")
Signed-off-by: Nikolaus Voss 


Reviewed-by: Guenter Roeck 

Note that tps6598x_exec_cmd() is only called with in_len == out_len == 0
and NULL data pointers. It might make sense to simplify the function and
drop the unused parameters as well as the associated code.


Clarification: That would be a separate patch.

Guenter


Guenter


---
v2: fix tps6598x_exec_cmd also
v3: use fixed length for stack buffer
---
  drivers/usb/typec/tps6598x.c | 26 --
  1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c84c8c189e90..eb8046f87a54 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
*val, size_t len)
  return 0;
  }
+static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
+    void *val, size_t len)
+{
+    u8 data[TPS_MAX_LEN + 1];
+
+    if (!tps->i2c_protocol)
+    return regmap_raw_write(tps->regmap, reg, val, len);
+
+    data[0] = len;
+    memcpy(&data[1], val, len);
+
+    return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
+}
+
  static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
  {
  return tps6598x_block_read(tps, reg, val, sizeof(u16));
@@ -127,23 +141,23 @@ static inline int tps6598x_read64(struct tps6598x *tps, 
u8 reg, u64 *val)
  static inline int tps6598x_write16(struct tps6598x *tps, u8 reg, u16 val)
  {
-    return regmap_raw_write(tps->regmap, reg, &val, sizeof(u16));
+    return tps6598x_block_write(tps, reg, &val, sizeof(u16));
  }
  static inline int tps6598x_write32(struct tps6598x *tps, u8 reg, u32 val)
  {
-    return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+    return tps6598x_block_write(tps, reg, &val, sizeof(u32));
  }
  static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
  {
-    return regmap_raw_write(tps->regmap, reg, &val, sizeof(u64));
+    return tps6598x_block_write(tps, reg, &val, sizeof(u64));
  }
  static inline int
  tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
  {
-    return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+    return tps6598x_block_write(tps, reg, &val, sizeof(u32));
  }
  static int tps6598x_read_partner_identity(struct tps6598x *tps)
@@ -229,8 +243,8 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const 
char *cmd,
  return -EBUSY;
  if (in_len) {
-    ret = regmap_raw_write(tps->regmap, TPS_REG_DATA1,
-   in_data, in_len);
+    ret = tps6598x_block_write(tps, TPS_REG_DATA1,
+   in_data, in_len);
  if (ret)
  return ret;
  }

RE: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

[Pawel Laszczak]

>
>On 20/02/2019 13:18, Pawel Laszczak wrote:
>> Hi Roger.
>>>
>>> On 14/02/2019 21:45, Pawel Laszczak wrote:
 Controller for OUT endpoints has shared on-chip buffers for all incoming
 packets, including ep0out. It's FIFO buffer, so packets must be handle
 by DMA in correct order. If the first packet in the buffer will not be
 handled, then the following packets directed for other endpoints and
 functions will be blocked.

 Additionally the packets directed to one endpoint can block entire on-chip
 buffers. In this case transfer to other endpoints also will blocked.

 To resolve this issue after raising the descriptor missing interrupt
 driver prepares internal usb_request object and use it to arm DMA
 transfer.

 The problematic situation was observed in case when endpoint has
 been enabled but no usb_request were queued. Driver try detects
 such endpoints and will use this workaround only for these endpoint.

 Driver use limited number of buffer. This number can be set by macro
 CDNS_WA2_NUM_BUFFERS.

 Such blocking situation was observed on ACM gadget. For this function
 host send OUT data packet but ACM function is not prepared for
 this packet. It's cause that buffer placed in on chip memory block
 transfer to other endpoints.

 It's limitation of controller but maybe this issues should be fixed in
 function driver.

 This work around can be disabled/enabled by means of quirk_internal_buffer
 module parameter. By default feature is enabled. It can has impact to
 transfer performance and in most case this feature can be disabled.
>>>
>>> How much is the performance impact?
>>
>> I didn't check this, but performance will be decreased because driver has to
>> copy data from internally allocated buffer to usb_request->buff.
>>
>>> You mentioned that the issue was observed only in the ACM case and
>>> could be fixed in the function driver?
>>> It seems pointless to enable an endpoint and not have any requests queued 
>>> for it.
>>
>> Yes, it’s true. The request in ACM class is send to Controller Driver when 
>> user read file associated
>> with ACM gadget. Problem exist because host send data to controller but USB 
>> controller
>> has not prepared buffer for this data by ACM class.
>>
>>> Isn't fixing the ACM driver (if there is a real issue) a better approach 
>>> than having
>>> a workaround that affects performance of all other use cases?
>>
>> Yes it should be better. But what ACM driver should do with unexpected data. 
>> I'm not sure if we
>> can simply delete them.
>>
>> The best solution would be to make modification in controller. In such case 
>> Controller shouldn't accept
>> packet but should send NAK.
>
>Yes, that should be standard behaviour. No?
>
>>
>> One more thing. Workaround has implemented algorithm that decide for which 
>> endpoint it should be enabled.
>> e.g for composite device MSC+NCM+ACM it should work only for ACM OUT 
>> endpoint.
>>
>
>If ACM driver didn't queue the request for ACM OUT endpoint, why does the 
>controller accept the data at all?
>I didn't understand why we need a workaround for this. It should be standard 
>behaviour to NAK data if
>function driver didn't request for all endpoints.

Yes, I agree with you. Controller shouldn’t accept such packet. As I know this 
behavior will be fixed in RTL. 
But I assume that some older version of this controller are one the market, and 
driver should work correct with them.
In the feature this workaround can be limited only to selected controllers. 
Even now I assume that it can be enabled/disabled by module parameter. 

>
>Also I didn't understand why performance should be impacted to just NAK data.

Data waiting in on-chip buffers can be very fast copied to system memory when 
DMA will be armed.
In same case this feature can little increase performance, but it makes many 
problems under OS.

Cheers,
Pawel


>cheers,
>-roger
>
>> Cheers,
>> Pawel
>>>

 Signed-off-by: Pawel Laszczak 
 ---
  drivers/usb/cdns3/gadget.c | 273 -
  drivers/usb/cdns3/gadget.h |   7 +
  2 files changed, 278 insertions(+), 2 deletions(-)

 diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
 index 7f7f24ee3c4b..5dfbe6e1421c 100644
 --- a/drivers/usb/cdns3/gadget.c
 +++ b/drivers/usb/cdns3/gadget.c
 @@ -27,6 +27,37 @@
   * If (((Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr-1) or
   *(Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr))
   *and (DRBL==1 and (not EP0)))
 + *
 + * Work around 2:
 + * Controller for OUT endpoints has shared on-chip buffers for all 
 incoming
 + * packets, including ep0out. It's FIFO buffer, so packets must be handle 
 by DMA
 + * in correct order. If the first packet in the buffer will not be 
 handled,
 + * then the following packets 

Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+

[Stanislaw Gruszka]

On Wed, Feb 20, 2019 at 02:22:08PM +0100, Lorenzo Bianconi wrote:
> > On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > > >> >> The way I see it, we have two choices.
> > > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> > > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > > >> > 
> > > >> > I agree, if this is only needed for dwc2. Though I would investigate
> > > >> > if this is not a bug on other platforms as well.
> > > >> >From what I can see, using Lorenzo's patches seems to be the better
> > > >> solution, since they avoid these corner cases in dwc2 (and maybe other
> > > >> drivers as well). I will apply them and then we'll see if we need to do
> > > >> any further improvements later on.
> > > > 
> > > > They work on rpi dwc2, but they do not address root of the problem.
> > > > There is clearly something wrong how mt76usb handle SG, what is not
> > > > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > > > SG should be red flag.
> > > I think we're simply dealing with multiple issues here, only some of
> > > which are fixed by Lorenzo's patches.
> > > I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> > > since the Linux USB API supports non-aligned transfer buffers and it
> > > should be up to the controller driver to deal with that.
> > 
> > Agree.
> > 
> > > dwc2 tries to do that, but that has limitations which I already pointed
> > > out and which are properly dealt with by Lorenzo's patches.
> > 
> > I planed to just accept current solution, but I started to work on patch
> > that remove len, sglen arguments from mt76u_buf_alloc() and use
> > q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
> > related code is now tangled.
> > 
> > Would be ok to send this patch with proper changelog as fix for RPI
> > against wireless-drivers and cc:stable (assuming it works and really
> > fix things on RPI) and revert Lorenzo's patches in -next ?
> 
> Hi Stanislaw,
> 
> what is the advantage of doing so?
To have small fix proper for -stable to fix the problem in 4.20 and 4.19,
and have simpler code.

> You have duplicated most of the fields that are
> already in the urb data structure and you use transfer_buffer (no SG I/O).
URB has plenty of fields, I duplicated 2. If size of mt76u_buf is a concern
this can be optimized by packing num_sgs, len, done into fields variable.

> Moreover I have ready a series that removes 99% of the dual allocation code 
> and
> maintain it in the control path (instead of the datapath one).
> I need just to rebase it ontop of your series. I will post it soon.
So I would ask what is the point to adding bunch of code and remove it in very
next patch?

Stanislaw 

Re: [PATCH] xhci: tegra: Prevent error pointer dereference

[Greg Kroah-Hartman]

On Wed, Feb 20, 2019 at 02:48:41PM +0100, Thierry Reding wrote:
> From: Thierry Reding 
> 
> During initialization, the host and super-speed power domains will
> contain an ERR_PTR() encoded error code rather than being NULL. To
> avoid a crash, use a !IS_ERR_OR_NULL() condition during cleanup.
> 
> Signed-off-by: Thierry Reding 
> ---
>  drivers/usb/host/xhci-tegra.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 938ff06c0349..efb0cad8710e 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct device 
> *dev,
>   device_link_del(tegra->genpd_dl_ss);
>   if (tegra->genpd_dl_host)
>   device_link_del(tegra->genpd_dl_host);
> - if (tegra->genpd_dev_ss)
> + if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss))
>   dev_pm_domain_detach(tegra->genpd_dev_ss, true);
> - if (tegra->genpd_dev_host)
> + if (!IS_ERR_OR_NULL(tegra->genpd_dev_host))
>   dev_pm_domain_detach(tegra->genpd_dev_host, true);
>  }

Should this go to older kernels?  If so, any hint as to what commit this
"fixes:"?

thanks,

greg k-h

Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+

[Lorenzo Bianconi]

> On Wed, Feb 20, 2019 at 02:22:08PM +0100, Lorenzo Bianconi wrote:
> > > On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > > > >> >> The way I see it, we have two choices.
> > > > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> > > > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > > > >> > 
> > > > >> > I agree, if this is only needed for dwc2. Though I would 
> > > > >> > investigate
> > > > >> > if this is not a bug on other platforms as well.
> > > > >> >From what I can see, using Lorenzo's patches seems to be the better
> > > > >> solution, since they avoid these corner cases in dwc2 (and maybe 
> > > > >> other
> > > > >> drivers as well). I will apply them and then we'll see if we need to 
> > > > >> do
> > > > >> any further improvements later on.
> > > > > 
> > > > > They work on rpi dwc2, but they do not address root of the problem.
> > > > > There is clearly something wrong how mt76usb handle SG, what is not
> > > > > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > > > > SG should be red flag.
> > > > I think we're simply dealing with multiple issues here, only some of
> > > > which are fixed by Lorenzo's patches.
> > > > I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> > > > since the Linux USB API supports non-aligned transfer buffers and it
> > > > should be up to the controller driver to deal with that.
> > > 
> > > Agree.
> > > 
> > > > dwc2 tries to do that, but that has limitations which I already pointed
> > > > out and which are properly dealt with by Lorenzo's patches.
> > > 
> > > I planed to just accept current solution, but I started to work on patch
> > > that remove len, sglen arguments from mt76u_buf_alloc() and use
> > > q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
> > > related code is now tangled.
> > > 
> > > Would be ok to send this patch with proper changelog as fix for RPI
> > > against wireless-drivers and cc:stable (assuming it works and really
> > > fix things on RPI) and revert Lorenzo's patches in -next ?
> > 
> > Hi Stanislaw,
> > 
> > what is the advantage of doing so?
> To have small fix proper for -stable to fix the problem in 4.20 and 4.19,
> and have simpler code.

merging the series I sent we will have a pretty simple approach, just a
single routine that allocates the rx buffers in the control path according to
the operating mode.

> 
> > You have duplicated most of the fields that are
> > already in the urb data structure and you use transfer_buffer (no SG I/O).
> URB has plenty of fields, I duplicated 2. If size of mt76u_buf is a concern
> this can be optimized by packing num_sgs, len, done into fields variable.
> 
> > Moreover I have ready a series that removes 99% of the dual allocation code 
> > and
> > maintain it in the control path (instead of the datapath one).
> > I need just to rebase it ontop of your series. I will post it soon.
> So I would ask what is the point to adding bunch of code and remove it in very
> next patch?

In the first series I fixed the issue, in this one I improved the code, I have
no added any new feature

Regards,
Lorenzo

> 
> Stanislaw 

Re: [PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Greg Kroah-Hartman]

On Wed, Feb 20, 2019 at 04:22:00PM +0100, Nikolaus Voss wrote:
> > > v2: fix tps6598x_exec_cmd also
> > > ---
> > >  drivers/usb/typec/tps6598x.c | 26 --
> > >  1 file changed, 20 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> > > index c84c8c189e90..c54b73fb2a2f 100644
> > > --- a/drivers/usb/typec/tps6598x.c
> > > +++ b/drivers/usb/typec/tps6598x.c
> > > @@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, 
> > > void *val, size_t len)
> > >   return 0;
> > >  }
> > > 
> > > +static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> > > + void *val, size_t len)
> > > +{
> > > + u8 data[len + 1];
> > 
> > I thought the build system now warned when you did this :(
> 
> I must admit I'm developing on 4.19 stable series, so no warnings...

Ick, no, you are 6 months behind where the rest of us are :(

Always, at the very least, work off of Linus's tree.  For best results,
work off of linux-next.

thanks,

greg k-h

Re: [PATCH] xhci: tegra: Prevent error pointer dereference

[Thierry Reding]

On Wed, Feb 20, 2019 at 05:16:12PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Feb 20, 2019 at 02:48:41PM +0100, Thierry Reding wrote:
> > From: Thierry Reding 
> > 
> > During initialization, the host and super-speed power domains will
> > contain an ERR_PTR() encoded error code rather than being NULL. To
> > avoid a crash, use a !IS_ERR_OR_NULL() condition during cleanup.
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> >  drivers/usb/host/xhci-tegra.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> > index 938ff06c0349..efb0cad8710e 100644
> > --- a/drivers/usb/host/xhci-tegra.c
> > +++ b/drivers/usb/host/xhci-tegra.c
> > @@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct device 
> > *dev,
> > device_link_del(tegra->genpd_dl_ss);
> > if (tegra->genpd_dl_host)
> > device_link_del(tegra->genpd_dl_host);
> > -   if (tegra->genpd_dev_ss)
> > +   if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss))
> > dev_pm_domain_detach(tegra->genpd_dev_ss, true);
> > -   if (tegra->genpd_dev_host)
> > +   if (!IS_ERR_OR_NULL(tegra->genpd_dev_host))
> > dev_pm_domain_detach(tegra->genpd_dev_host, true);
> >  }
> 
> Should this go to older kernels?  If so, any hint as to what commit this
> "fixes:"?

Technically this:

Fixes: 6494a9ad86de ("usb: xhci: tegra: Add genpd support")

which was merged into v4.20.

However, I have never seen this actually crash on current kernels and
only came across it while working on a change to reset controls which
can cause dev_pm_domain_attach_by_name() to fail. That function can't
fail in current kernels, so I don't think we need to fix this in old
kernels. So if we can get this into v5.0, that'd be good enough for
me.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH] xhci: tegra: Prevent error pointer dereference

[Greg Kroah-Hartman]

On Wed, Feb 20, 2019 at 05:27:02PM +0100, Thierry Reding wrote:
> On Wed, Feb 20, 2019 at 05:16:12PM +0100, Greg Kroah-Hartman wrote:
> > On Wed, Feb 20, 2019 at 02:48:41PM +0100, Thierry Reding wrote:
> > > From: Thierry Reding 
> > > 
> > > During initialization, the host and super-speed power domains will
> > > contain an ERR_PTR() encoded error code rather than being NULL. To
> > > avoid a crash, use a !IS_ERR_OR_NULL() condition during cleanup.
> > > 
> > > Signed-off-by: Thierry Reding 
> > > ---
> > >  drivers/usb/host/xhci-tegra.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> > > index 938ff06c0349..efb0cad8710e 100644
> > > --- a/drivers/usb/host/xhci-tegra.c
> > > +++ b/drivers/usb/host/xhci-tegra.c
> > > @@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct 
> > > device *dev,
> > >   device_link_del(tegra->genpd_dl_ss);
> > >   if (tegra->genpd_dl_host)
> > >   device_link_del(tegra->genpd_dl_host);
> > > - if (tegra->genpd_dev_ss)
> > > + if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss))
> > >   dev_pm_domain_detach(tegra->genpd_dev_ss, true);
> > > - if (tegra->genpd_dev_host)
> > > + if (!IS_ERR_OR_NULL(tegra->genpd_dev_host))
> > >   dev_pm_domain_detach(tegra->genpd_dev_host, true);
> > >  }
> > 
> > Should this go to older kernels?  If so, any hint as to what commit this
> > "fixes:"?
> 
> Technically this:
> 
> Fixes: 6494a9ad86de ("usb: xhci: tegra: Add genpd support")
> 
> which was merged into v4.20.
> 
> However, I have never seen this actually crash on current kernels and
> only came across it while working on a change to reset controls which
> can cause dev_pm_domain_attach_by_name() to fail. That function can't
> fail in current kernels, so I don't think we need to fix this in old
> kernels. So if we can get this into v5.0, that'd be good enough for
> me.

5.0-final is too late, but 5.0.y is doable.

Mathias, want me to take this directly?

thanks,

greg k-h

Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+

[Stanislaw Gruszka]

On Wed, Feb 20, 2019 at 05:22:18PM +0100, Lorenzo Bianconi wrote:
> > On Wed, Feb 20, 2019 at 02:22:08PM +0100, Lorenzo Bianconi wrote:
> > > > On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > > > > >> >> The way I see it, we have two choices.
> > > > > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL 
> > > > > >> >> case
> > > > > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > > > > >> > 
> > > > > >> > I agree, if this is only needed for dwc2. Though I would 
> > > > > >> > investigate
> > > > > >> > if this is not a bug on other platforms as well.
> > > > > >> >From what I can see, using Lorenzo's patches seems to be the 
> > > > > >> >better
> > > > > >> solution, since they avoid these corner cases in dwc2 (and maybe 
> > > > > >> other
> > > > > >> drivers as well). I will apply them and then we'll see if we need 
> > > > > >> to do
> > > > > >> any further improvements later on.
> > > > > > 
> > > > > > They work on rpi dwc2, but they do not address root of the problem.
> > > > > > There is clearly something wrong how mt76usb handle SG, what is not
> > > > > > fixed. And adding disable_usb_sg module parameter for hcd's 
> > > > > > supporting
> > > > > > SG should be red flag.
> > > > > I think we're simply dealing with multiple issues here, only some of
> > > > > which are fixed by Lorenzo's patches.
> > > > > I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> > > > > since the Linux USB API supports non-aligned transfer buffers and it
> > > > > should be up to the controller driver to deal with that.
> > > > 
> > > > Agree.
> > > > 
> > > > > dwc2 tries to do that, but that has limitations which I already 
> > > > > pointed
> > > > > out and which are properly dealt with by Lorenzo's patches.
> > > > 
> > > > I planed to just accept current solution, but I started to work on patch
> > > > that remove len, sglen arguments from mt76u_buf_alloc() and use
> > > > q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
> > > > related code is now tangled.
> > > > 
> > > > Would be ok to send this patch with proper changelog as fix for RPI
> > > > against wireless-drivers and cc:stable (assuming it works and really
> > > > fix things on RPI) and revert Lorenzo's patches in -next ?
> > > 
> > > Hi Stanislaw,
> > > 
> > > what is the advantage of doing so?
> > To have small fix proper for -stable to fix the problem in 4.20 and 4.19,
> > and have simpler code.
> 
> merging the series I sent we will have a pretty simple approach, just a
> single routine that allocates the rx buffers in the control path according to
> the operating mode.

So will you do the backport of your patches and post them to -stable ?
Do you think greg-kh will accept those ?

Stanislaw 

Re: [PATCH] xhci: tegra: Prevent error pointer dereference

[Mathias Nyman]

On 20.2.2019 18.31, Greg Kroah-Hartman wrote:

On Wed, Feb 20, 2019 at 05:27:02PM +0100, Thierry Reding wrote:

On Wed, Feb 20, 2019 at 05:16:12PM +0100, Greg Kroah-Hartman wrote:

On Wed, Feb 20, 2019 at 02:48:41PM +0100, Thierry Reding wrote:

From: Thierry Reding 

During initialization, the host and super-speed power domains will
contain an ERR_PTR() encoded error code rather than being NULL. To
avoid a crash, use a !IS_ERR_OR_NULL() condition during cleanup.

Signed-off-by: Thierry Reding 
---
  drivers/usb/host/xhci-tegra.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 938ff06c0349..efb0cad8710e 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct device 
*dev,
device_link_del(tegra->genpd_dl_ss);
if (tegra->genpd_dl_host)
device_link_del(tegra->genpd_dl_host);
-   if (tegra->genpd_dev_ss)
+   if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss))
dev_pm_domain_detach(tegra->genpd_dev_ss, true);
-   if (tegra->genpd_dev_host)
+   if (!IS_ERR_OR_NULL(tegra->genpd_dev_host))
dev_pm_domain_detach(tegra->genpd_dev_host, true);
  }


Should this go to older kernels?  If so, any hint as to what commit this
"fixes:"?


Technically this:

Fixes: 6494a9ad86de ("usb: xhci: tegra: Add genpd support")

which was merged into v4.20.

However, I have never seen this actually crash on current kernels and
only came across it while working on a change to reset controls which
can cause dev_pm_domain_attach_by_name() to fail. That function can't
fail in current kernels, so I don't think we need to fix this in old
kernels. So if we can get this into v5.0, that'd be good enough for
me.


5.0-final is too late, but 5.0.y is doable.

Mathias, want me to take this directly?



Yes please

Acked-by: Mathias Nyman 

Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+

[Lorenzo Bianconi]

> On Wed, Feb 20, 2019 at 05:22:18PM +0100, Lorenzo Bianconi wrote:
> > > On Wed, Feb 20, 2019 at 02:22:08PM +0100, Lorenzo Bianconi wrote:
> > > > > On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > > > > > >> >> The way I see it, we have two choices.
> > > > > > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL 
> > > > > > >> >> case
> > > > > > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > > > > > >> > 
> > > > > > >> > I agree, if this is only needed for dwc2. Though I would 
> > > > > > >> > investigate
> > > > > > >> > if this is not a bug on other platforms as well.
> > > > > > >> >From what I can see, using Lorenzo's patches seems to be the 
> > > > > > >> >better
> > > > > > >> solution, since they avoid these corner cases in dwc2 (and maybe 
> > > > > > >> other
> > > > > > >> drivers as well). I will apply them and then we'll see if we 
> > > > > > >> need to do
> > > > > > >> any further improvements later on.
> > > > > > > 
> > > > > > > They work on rpi dwc2, but they do not address root of the 
> > > > > > > problem.
> > > > > > > There is clearly something wrong how mt76usb handle SG, what is 
> > > > > > > not
> > > > > > > fixed. And adding disable_usb_sg module parameter for hcd's 
> > > > > > > supporting
> > > > > > > SG should be red flag.
> > > > > > I think we're simply dealing with multiple issues here, only some of
> > > > > > which are fixed by Lorenzo's patches.
> > > > > > I'm pretty sure it's still wrong for mt76 to try to align its 
> > > > > > buffers,
> > > > > > since the Linux USB API supports non-aligned transfer buffers and it
> > > > > > should be up to the controller driver to deal with that.
> > > > > 
> > > > > Agree.
> > > > > 
> > > > > > dwc2 tries to do that, but that has limitations which I already 
> > > > > > pointed
> > > > > > out and which are properly dealt with by Lorenzo's patches.
> > > > > 
> > > > > I planed to just accept current solution, but I started to work on 
> > > > > patch
> > > > > that remove len, sglen arguments from mt76u_buf_alloc() and use
> > > > > q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized 
> > > > > how
> > > > > related code is now tangled.
> > > > > 
> > > > > Would be ok to send this patch with proper changelog as fix for RPI
> > > > > against wireless-drivers and cc:stable (assuming it works and really
> > > > > fix things on RPI) and revert Lorenzo's patches in -next ?
> > > > 
> > > > Hi Stanislaw,
> > > > 
> > > > what is the advantage of doing so?
> > > To have small fix proper for -stable to fix the problem in 4.20 and 4.19,
> > > and have simpler code.
> > 
> > merging the series I sent we will have a pretty simple approach, just a
> > single routine that allocates the rx buffers in the control path according 
> > to
> > the operating mode.
> 
> So will you do the backport of your patches and post them to -stable ?
> Do you think greg-kh will accept those ?

I will take care of it

Regards,
Lorenzo

> 
> Stanislaw 

Re: [regression] USB power management failure to suspend / high CPU usage

[Mathias Nyman]

On 14.2.2019 20.04, Eric Blau wrote:

On Thu, Feb 14, 2019 at 9:56 AM Mathias Nyman
 wrote:



Thanks for looking into this, Mathias. Now that you point this out, on
older kernels where suspend and resume works, I noticed the following
log messages emitted when resuming from suspend:

Feb 06 18:58:05 eric-macbookpro kernel: usb usb2-port3: Cannot enable.
Maybe the USB cable is bad?


Attached a testpatch that should react to ports stuck in polling state,
and warm reset them.

It doesn't limit the numbers of reset tries, or allow system suspend with ports
stuck in polling state, but I'd like to know how the MacBook reacts to it

Can you test it with debugging enabled?


Hi Mathias,

Thanks for the patch. I tested it against 4.20.8 and got a couple
successful suspends but on the third attempt I got the same failure
again. I noticed after the first suspend/resume, the card reader
showed as "Link:Compliance" but on later attempts it showed stuck in
Polling again.

I've attached the logs with debugging enabled.



Thanks, logs show that several resets won't recover the card reader.

I'm taking a different approach, USB3 ports in polling state should
automatically move to connected/enabled. So instead of preventing
suspend if a port is found in polling I'll let it try to finish link
training for some time, and then just continue with suspend if it fails.

Patch attached.

This won't fix the broken card reader, but should allow your MacBook to suspend.
After this we can start looking at fixing the card reader, Ivan Miranov sent 
some
proposal for this.

-Mathias
>From 444cab4f41c79c5a04d1fc8939b450c89dc64768 Mon Sep 17 00:00:00 2001
From: Mathias Nyman 
Date: Wed, 20 Feb 2019 16:10:54 +0200
Subject: [PATCH 1/2] xhci: Don't let USB3 ports stuck in polling state prevent
 suspend

Commit 2f31a67f01a8 ("usb: xhci: Prevent bus suspend if a port connect
change or polling state is detected") was intended to prevent ports that
were still link training from being forced to U3 suspend state mid
enumeration.
This solved enumeration issues for devices with slow link training.

Turns out some devices are stuck in the link training/polling state,
and thus that patch will prevent suspend completely for these devices.
This is seen with USB3 card readers in some MacBooks.

Instead of preventing suspend, give some time to complete the link
training. On successful training the port will end up as connected
and enabled.
If port instead is stuck in link training the bus suspend will continue
suspending after 360ms (10 * 36ms) timeout (tPollingLFPSTimeout).

Original patch was sent to stable, this one should go there as well

Fixes: 2f31a67f01a8 ("usb: xhci: Prevent bus suspend if a port connect change or polling state is detected")
Cc: sta...@vger.kernel.org
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c | 19 ---
 drivers/usb/host/xhci.h |  8 
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index e2eece6..96a7405 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1545,20 +1545,25 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
 	port_index = max_ports;
 	while (port_index--) {
 		u32 t1, t2;
-
+		int retries = 10;
+retry:
 		t1 = readl(ports[port_index]->addr);
 		t2 = xhci_port_state_to_neutral(t1);
 		portsc_buf[port_index] = 0;
 
-		/* Bail out if a USB3 port has a new device in link training */
-		if ((hcd->speed >= HCD_USB3) &&
+		/*
+		 * Give a USB3 port in link training time to finish, but don't
+		 * prevent suspend as port might be stuck
+		 */
+		if ((hcd->speed >= HCD_USB3) && retries-- &&
 		(t1 & PORT_PLS_MASK) == XDEV_POLLING) {
-			bus_state->bus_suspended = 0;
 			spin_unlock_irqrestore(&xhci->lock, flags);
-			xhci_dbg(xhci, "Bus suspend bailout, port in polling\n");
-			return -EBUSY;
+			msleep(XHCI_PORT_POLLING_LFPS_TIME);
+			spin_lock_irqsave(&xhci->lock, flags);
+			xhci_dbg(xhci, "port %d polling in bus suspend, waiting\n",
+ port_index);
+			goto retry;
 		}
-
 		/* suspend ports in U0, or bail out for new connect changes */
 		if ((t1 & PORT_PE) && (t1 & PORT_PLS_MASK) == XDEV_U0) {
 			if ((t1 & PORT_CSC) && wake_enabled) {
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 652dc36..9334cde 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -452,6 +452,14 @@ struct xhci_op_regs {
  */
 #define XHCI_DEFAULT_BESL	4
 
+/*
+ * USB3 specification define a 360ms tPollingLFPSTiemout for USB3 ports
+ * to complete link training. usually link trainig completes much faster
+ * so check status 10 times with 36ms sleep in places we need to wait for
+ * polling to complete.
+ */
+#define XHCI_PORT_POLLING_LFPS_TIME  36
+
 /**
  * struct xhci_intr_reg - Interrupt Register Set
  * @irq_pending:	IMAN - Interrupt Management Register.  Used to enable
-- 
2.7.4

[PATCH 1/4] usb: xhci: fix build warning - missing prototype

[Mathias Nyman]

From: Jean-Philippe Menil 

Fix build warning when building drivers/usb/host/xhci-mem.o due to missing
prototype for xhci_free_virt_devices_depth_first.

This function is only used in xhci-mem.c so just make it static.

Signed-off-by: Jean-Philippe Menil 
---
 drivers/usb/host/xhci-mem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 8067f17..cf5e179 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -933,7 +933,7 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int 
slot_id)
  * that tt_info, then free the child first. Recursive.
  * We can't rely on udev at this point to find child-parent relationships.
  */
-void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_id)
+static void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int 
slot_id)
 {
struct xhci_virt_device *vdev;
struct list_head *tt_list_head;
-- 
2.7.4

[PATCH 0/4] xhci features for usb-next

[Mathias Nyman]

Hi Greg

A few minor xhci features for usb-next.
A bit late in the cycle, there is nothing urgent with these
but nice if they make 5.1

-Mathias

Balaji Manoharan (1):
  usb: xhci: Fix for Enabling USB ROLE SWITCH QUIRK on
INTEL_SUNRISEPOINT_LP_XHCI

Chunfeng Yun (1):
  usb: xhci: remove unused member 'parent' in xhci_regset struct

Jean-Philippe Menil (1):
  usb: xhci: fix build warning - missing prototype

Prabhat Chand Pandey (1):
  usb: xhci: dbc: Fixing typo error.

 drivers/usb/host/xhci-dbgcap.c  | 6 +++---
 drivers/usb/host/xhci-debugfs.h | 1 -
 drivers/usb/host/xhci-mem.c | 2 +-
 drivers/usb/host/xhci-pci.c | 1 +
 4 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH 3/4] usb: xhci: remove unused member 'parent' in xhci_regset struct

[Mathias Nyman]

From: Chunfeng Yun 

The member @parent of xhci_regset struct is not used in fact,
so remove it

Signed-off-by: Chunfeng Yun 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-debugfs.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/xhci-debugfs.h b/drivers/usb/host/xhci-debugfs.h
index ac5bc40..f7a4e24 100644
--- a/drivers/usb/host/xhci-debugfs.h
+++ b/drivers/usb/host/xhci-debugfs.h
@@ -80,7 +80,6 @@ struct xhci_regset {
charname[DEBUGFS_NAMELEN];
struct debugfs_regset32 regset;
size_t  nregs;
-   struct dentry   *parent;
struct list_headlist;
 };
 
-- 
2.7.4

[PATCH 2/4] usb: xhci: Fix for Enabling USB ROLE SWITCH QUIRK on INTEL_SUNRISEPOINT_LP_XHCI

[Mathias Nyman]

From: Balaji Manoharan 

This fix enables USB role feature on intel commercial nuc
platform which is based on Kabylake chipset.

Signed-off-by: Balaji Manoharan 
Reviewed-by: Hans de Goede 
Reviewed-by: Heikki Krogerus 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index a9ec705..c2fe218 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -194,6 +194,7 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
xhci->quirks |= XHCI_SSIC_PORT_UNUSED;
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
(pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_XHCI ||
 pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI))
xhci->quirks |= XHCI_INTEL_USB_ROLE_SW;
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
-- 
2.7.4

[PATCH 4/4] usb: xhci: dbc: Fixing typo error.

[Mathias Nyman]

From: Prabhat Chand Pandey 

Replaced "xhci_dbc_flush_reqests" with xhci_dbc_flush_requests".

Signed-off-by: Prabhat Chand Pandey 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-dbgcap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
index 86cff5c..c78be57 100644
--- a/drivers/usb/host/xhci-dbgcap.c
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -181,7 +181,7 @@ static void xhci_dbc_flush_endpoint_requests(struct dbc_ep 
*dep)
xhci_dbc_flush_single_request(req);
 }
 
-static void xhci_dbc_flush_reqests(struct xhci_dbc *dbc)
+static void xhci_dbc_flush_requests(struct xhci_dbc *dbc)
 {
xhci_dbc_flush_endpoint_requests(&dbc->eps[BULK_OUT]);
xhci_dbc_flush_endpoint_requests(&dbc->eps[BULK_IN]);
@@ -687,7 +687,7 @@ static enum evtreturn xhci_dbc_do_handle_events(struct 
xhci_dbc *dbc)
!(portsc & DBC_PORTSC_CONN_STATUS)) {
xhci_info(xhci, "DbC cable unplugged\n");
dbc->state = DS_ENABLED;
-   xhci_dbc_flush_reqests(dbc);
+   xhci_dbc_flush_requests(dbc);
 
return EVT_DISC;
}
@@ -697,7 +697,7 @@ static enum evtreturn xhci_dbc_do_handle_events(struct 
xhci_dbc *dbc)
xhci_info(xhci, "DbC port reset\n");
writel(portsc, &dbc->regs->portsc);
dbc->state = DS_ENABLED;
-   xhci_dbc_flush_reqests(dbc);
+   xhci_dbc_flush_requests(dbc);
 
return EVT_DISC;
}
-- 
2.7.4

Re: [PATCH 1/4] usb: xhci: fix build warning - missing prototype

[Mathias Nyman]

On 20.2.2019 19.50, Mathias Nyman wrote:

From: Jean-Philippe Menil 

Fix build warning when building drivers/usb/host/xhci-mem.o due to missing
prototype for xhci_free_virt_devices_depth_first.

This function is only used in xhci-mem.c so just make it static.

Signed-off-by: Jean-Philippe Menil 


My Signed-off-by was apparently lost after I did some patch juggling

Signed-off-by: Mathias Nyman 

Want me to resend?

-Mathias

Re: [PATCH 1/4] usb: xhci: fix build warning - missing prototype

[Greg KH]

On Wed, Feb 20, 2019 at 08:01:08PM +0200, Mathias Nyman wrote:
> On 20.2.2019 19.50, Mathias Nyman wrote:
> > From: Jean-Philippe Menil 
> > 
> > Fix build warning when building drivers/usb/host/xhci-mem.o due to missing
> > prototype for xhci_free_virt_devices_depth_first.
> > 
> > This function is only used in xhci-mem.c so just make it static.
> > 
> > Signed-off-by: Jean-Philippe Menil 
> 
> My Signed-off-by was apparently lost after I did some patch juggling
> 
> Signed-off-by: Mathias Nyman 
> 
> Want me to resend?

Nah, I can fix it up when I apply it in the morning.

thanks,

greg k-h

Re: [PATCH 2/4] usb: xhci: Fix for Enabling USB ROLE SWITCH QUIRK on INTEL_SUNRISEPOINT_LP_XHCI

[Greg KH]

On Wed, Feb 20, 2019 at 07:50:53PM +0200, Mathias Nyman wrote:
> From: Balaji Manoharan 
> 
> This fix enables USB role feature on intel commercial nuc
> platform which is based on Kabylake chipset.
> 
> Signed-off-by: Balaji Manoharan 
> Reviewed-by: Hans de Goede 
> Reviewed-by: Heikki Krogerus 
> Signed-off-by: Mathias Nyman 
> ---
>  drivers/usb/host/xhci-pci.c | 1 +
>  1 file changed, 1 insertion(+)

Shouldn't this be cc: stable?

Re: [PATCH 1/4] usb: xhci: fix build warning - missing prototype

[Greg KH]

On Wed, Feb 20, 2019 at 07:50:52PM +0200, Mathias Nyman wrote:
> From: Jean-Philippe Menil 
> 
> Fix build warning when building drivers/usb/host/xhci-mem.o due to missing
> prototype for xhci_free_virt_devices_depth_first.

There is no normal "build warning", this is a sparse fixup, right?

Re: [PATCH 1/4] usb: xhci: fix build warning - missing prototype

[Jean-Philippe Menil]

On Wed, 20 Feb 2019, Greg KH wrote:

> On Wed, Feb 20, 2019 at 07:50:52PM +0200, Mathias Nyman wrote:
> > From: Jean-Philippe Menil 
> > 
> > Fix build warning when building drivers/usb/host/xhci-mem.o due to missing
> > prototype for xhci_free_virt_devices_depth_first.
> 
> There is no normal "build warning", this is a sparse fixup, right?
> 
> 

Hi Greg,

this is a build warning, at least with W=1 :

drivers/usb/host/xhci-mem.c:936:6: warning: no previous prototype for 
‘xhci_free_virt_devices_depth_first’ [-Wmissing-prototypes]
 void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int 
slot_id)

Regards,

Jean-Philippe

USB mouse doesn't work after laptop entered in suspending mode

[Jorge Augusto]

Dear developers,
I have an ACER Aspire E1-522 but with this motherboard EG50-KB MB
12253-3M and the USB mouse stops working after resume and I need to
unbind and bin OCHI-PCI to mouse starts working again

This is the error:

dmesg --level=err
[ 836.062810] ohci-pci :00:12.0: frame counter not updating; disabled
[ 836.062813] ohci-pci :00:12.0: HC died; cleaning up
[ 840.894748] dpm_run_callback(): usb_dev_resume+0x0/0x20 returns -22
[ 840.894767] PM: Device 3-1 failed to resume async: error -22

I already tested with kernel 4.15.0-45-generic and kernel
4.18.0-15-generic and this fault still remains

I'm noob with regard to the kernel but I can test everything you need
about this subject, I just need you give me instructions what I need
to do

Thanks ind advanced for your help to solve this matter

My laptop specifications:
Product: ACER Aspire E1-522 ( Aspire E1-522_076B_2.09)
motherboard: EG50-KB MB 12253-3M
cpu: AMD E1-2500 APU with Radeon (TM) HD Graphics
width: 64 bits
memory: 4GiB


Best regards,
Jorge Augusto

RE: [PATCH v3] usb: chipidea: Grab the (legacy) USB PHY by phandle first

[Peter Chen]

 
> 
> On Mon, 2019-02-18 at 03:04 +, Peter Chen wrote:
> > > According to the chipidea driver bindings, the USB PHY is specified via 
> > > the
> "phys"
> > > phandle node. However, this only takes effect for USB PHYs that use
> > > the common PHY framework. For legacy USB PHYs, a simple lookup based
> > > on the USB PHY type is done instead.
> > >
> > > This does not play out well when more than one USB PHY is
> > > registered, since the first registered PHY matching the type will
> > > always be returned regardless of what the driver was bound to.
> > >
> > > Fix this by looking up the PHY based on the "phys" phandle node.
> > > Although generic PHYs are rather matched by their "phys-name" and not the
> "phys"
> > > phandle directly, there is no helper for similar lookup on legacy
> > > PHYs and it's probably not worth the effort to add it.
> > >
> > > When no legacy USB PHY is found by phandle, fallback to grabbing any
> > > registered
> > > USB2 PHY. This ensures backward compatibility if some users were
> > > actually relying on this mechanism.
> > >
> > > Signed-off-by: Paul Kocialkowski 
> > > ---
> > > Changes since v2:
> > > * Fixed typos in commit message.
> > >
> > > Changes since v1:
> > > * Only consider legacy USB PHY error for fallback as suggested;
> > > * Checked against EPROBE_DEFER before entering fallback.
> > >
> > >  drivers/usb/chipidea/core.c | 9 -
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/chipidea/core.c
> > > b/drivers/usb/chipidea/core.c index 7bfcbb23c2a4..016e4004fe9d
> > > 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -954,8 +954,15 @@ static int ci_hdrc_probe(struct platform_device 
> > > *pdev)
> > >   } else if (ci->platdata->usb_phy) {
> > >   ci->usb_phy = ci->platdata->usb_phy;
> > >   } else {
> > > + ci->usb_phy = devm_usb_get_phy_by_phandle(dev->parent, "phys",
> > > +   0);
> > >   ci->phy = devm_phy_get(dev->parent, "usb-phy");
> > > - ci->usb_phy = devm_usb_get_phy(dev->parent,
> > > USB_PHY_TYPE_USB2);
> > > +
> > > + /* Fallback to grabbing any registered USB2 PHY */
> > > + if (IS_ERR(ci->usb_phy) &&
> > > + PTR_ERR(ci->usb_phy) != -EPROBE_DEFER)
> > > + ci->usb_phy = devm_usb_get_phy(dev->parent,
> > > +USB_PHY_TYPE_USB2);
> > >
> >
> > I think you may need to do above if ci->phy is error, and not the error is -
> EPROBE_DEFER.
> 
> As Thomas pointed out during the review of v1, the initial flow is to try and 
> get both
> ci->usb_phy and ci->phy and let the code use ci->phy in priority later.
> 

If there is a generic PHY node under USB controller, and there is a USB PHY
at other sides, both ci->phy and ci->usb_phy are valid, I original thought it is
the problem you met.

Since you are trying to fix the legacy USB PHY grab issue, I hope you could 
consider
the generic PHY as well.

Peter
 

> This change attempts to keep this flow intact. The EPROBE_DEFER check is in
> case the initial ci->usb_phy is valid but deferred: we don't want to 
> overwrite it by
> calling devm_usb_get_phy which might return an actual error and result in 
> losing
> the EPROBE_DEFER information.
> 
> Does that make sense to you?
> 
> Cheers,
> 
> Paul
> 
> > Peter
> >
> > >   /* if both generic PHY and USB PHY layers aren't enabled */
> > >   if (PTR_ERR(ci->phy) == -ENOSYS &&
> > > --
> > > 2.20.1
> --
> Paul Kocialkowski, Bootlin
> Embedded Linux and kernel engineering
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin.com
> &data=02%7C01%7Cpeter.chen%40nxp.com%7Cadca65357daa4fe678dc08d
> 6957b9c49%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6368607554
> 66805805&sdata=c%2FdLqUhFxe1yz7lrqUoXZCvINiq1rdKJmMAuaH6Fr1k%3
> D&reserved=0

RE: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

[Felipe Balbi]

Hi,

(please break your emails at 80-columns)

Pawel Laszczak  writes:
>>> One more thing. Workaround has implemented algorithm that decide for which
>>> endpoint it should be enabled.  e.g for composite device MSC+NCM+ACM it
>>> should work only for ACM OUT endpoint.
>>>
>>
>>If ACM driver didn't queue the request for ACM OUT endpoint, why does the
>>controller accept the data at all?
>>
>>I didn't understand why we need a workaround for this. It should be standard
>>behaviour to NAK data if function driver didn't request for all endpoints.
>
> Yes, I agree with you. Controller shouldn’t accept such packet. As I know this
> behavior will be fixed in RTL.
>
> But I assume that some older version of this controller are one the market,
> and driver should work correct with them.
>
> In the feature this workaround can be limited only to selected controllers.
>
> Even now I assume that it can be enabled/disabled by module parameter.

no module parameters, please. Use revision detection in runtime.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: dwc2: suppress confusing warnings on BCM2835

[Minas Harutyunyan]

Hi Stefan,

On 2/19/2019 10:23 PM, Stefan Wahren wrote:
> According to the BCM2835 datasheet the used Synopsys IP isn't a LPM-capable
> core. So disable these features and suppress these confusing warnings:
> 
> dwc2 3f98.usb: dwc2_check_params: Invalid parameter lpm=1
> dwc2 3f98.usb: dwc2_check_params: Invalid parameter lpm_clock_gating=1
> dwc2 3f98.usb: dwc2_check_params: Invalid parameter besl=1
> dwc2 3f98.usb: dwc2_check_params: Invalid parameter hird_threshold_en=1
> 
> Signed-off-by: Stefan Wahren 
> ---
>   drivers/usb/dwc2/params.c | 4 
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index 24ff5f2..a158abb 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -47,6 +47,10 @@ static void dwc2_set_bcm_params(struct dwc2_hsotg *hsotg)
>   p->max_transfer_size = 65535;
>   p->max_packet_count = 511;
>   p->ahbcfg = 0x10;
> + p->lpm = false;
> + p->lpm_clock_gating = false;
> + p->besl = false;
> + p->hird_threshold_en = false;
>   }
>   
>   static void dwc2_set_his_params(struct dwc2_hsotg *hsotg)
> 

What about to apply below patch:


diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index c1912627a032..6b86aa42f003 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -303,11 +303,17 @@ static void dwc2_set_default_params(struct 
dwc2_hsotg *hsotg)
 p->reload_ctl = (hw->snpsid >= DWC2_CORE_REV_2_92a);
 p->uframe_sched = true;
 p->external_id_pin_ctl = false;
-   p->lpm = true;
-   p->lpm_clock_gating = true;
-   p->besl = true;
-   p->hird_threshold_en = true;
-   p->hird_threshold = 4;
+   p->lpm = hw->lpm_mode;
+   if (p->lpm) {
+   p->lpm_clock_gating = true;
+   p->besl = true;
+   p->hird_threshold_en = true;
+   p->hird_threshold = 4;
+   } else {
+   p->lpm_clock_gating = false;
+   p->besl = false;
+   p->hird_threshold_en = false;
+   }
 p->ipg_isoc_en = false;
 p->service_interval = false;
 p->max_packet_count = hw->max_packet_count;



Thanks,
Minas