Re: [PATCH 1/2] usb: gadget: udc: core: update usb_ep_queue() documentation

2018-03-27 Thread Felipe Balbi 

Hi,

Alan Stern  writes:
>> I agree with Alan that the spinlock must be dropped before calling
>> usb_ep_queue. An example can be found in the ep0_queue function of
>> the f_mass_storage driver.
>
> Thanks for the note of support, but you must be looking at a different 
> version of the f_mass_storage code than I am -- in my copy, ep0_queue 
> does not acquire or release any locks.
>
> I forgot to mention in my earlier email...  Felipe said that the 
> problem case involved calling the completion handler for a failed 
> submission.  That definitely is not the right approach; if a submission 
> fails then usb_ep_queue() should return an error code and the 
> completion routine should not be called.
>
> In fact, there is a general principal here which should be documented
> but doesn't seem to be (as far as I can tell).  Namely, a request's
> completion routine will be called if and only if usb_ep_queue() returns
> 0.  That would make a good addition to the kerneldoc for 
> usb_ep_queue().

agreed. Do you want to send a patch or should I write it?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v7] usb/gadget: Add driver for Aspeed SoC virtual hub

2018-03-27 Thread Felipe Balbi 

Hi,

Benjamin Herrenschmidt  writes:



> +static int ast_vhub_rep_desc(struct ast_vhub_ep *ep,
> +  u8 desc_type, u16 len)
> +{
> + const void *desc;
> + size_t dsize;
> +
> + EPDBG(ep, "GET_DESCRIPTOR(type:%d)\n", desc_type);
> + switch(desc_type) {
> + case USB_DT_DEVICE:
> + desc = &ast_vhub_dev_desc;
> + dsize = USB_DT_DEVICE_SIZE;
> + break;
> + case USB_DT_CONFIG:
> + desc = &ast_vhub_conf_desc;
> + dsize = AST_VHUB_CONF_DESC_SIZE;
> + break;
> + case USB_DT_HUB:
> + desc = &ast_vhub_hub_desc;
> + dsize = AST_VHUB_HUB_DESC_SIZE;
> + break;
> + default:
> + return std_req_stall;
> + }
> + if (dsize < len)
> + len = dsize;
> + /*
> +  * This is our limit for hub replies in our current
> +  * implementation, keeps things simpler.
> +  */
> + if (WARN_ON(len >= AST_VHUB_EP0_MAX_PACKET))
> + len = AST_VHUB_EP0_MAX_PACKET - 1;
> +
> + /*
> +  * Copy first to EP buffer and send from there, so
> +  * we can do some in-place patching if needed
> +  */
> + memcpy(ep->buf, desc, len);

still fails to compile here the same way.

$ gcc --version
gcc (Debian 7.3.0-12) 7.3.0

> +
> + /* Patch it if forcing USB1 */
> + if (desc_type == USB_DT_DEVICE && ep->vhub->force_usb1)
> + ast_vhub_patch_dev_desc_usb1(ep->buf);
> +
> + /* Shoot it from the EP buffer */
> + return ast_vhub_reply(ep, NULL, len);
> +}
> +
> +/* copied from hdc.c */
> +static unsigned
> +ascii2desc(char const *s, u8 *buf, unsigned len)

seems it would be best to move this helper to usb common, or something
like that. Copying is not a good idea.

> +static int ast_vhub_rep_string(struct ast_vhub_ep *ep,
> +u8 string_id, u16 lang_id,
> +u16 len)

spaces for indentation?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v7] usb/gadget: Add driver for Aspeed SoC virtual hub

2018-03-27 Thread Benjamin Herrenschmidt 
On Tue, 2018-03-27 at 10:30 +0300, Felipe Balbi wrote:
> Hi,
> 
> Benjamin Herrenschmidt  writes:
> 
> 
> 
> > +static int ast_vhub_rep_desc(struct ast_vhub_ep *ep,
> > +u8 desc_type, u16 len)
> > +{
> > +   const void *desc;
> > +   size_t dsize;
> > +
> > +   EPDBG(ep, "GET_DESCRIPTOR(type:%d)\n", desc_type);
> > +   switch(desc_type) {
> > +   case USB_DT_DEVICE:
> > +   desc = &ast_vhub_dev_desc;
> > +   dsize = USB_DT_DEVICE_SIZE;
> > +   break;
> > +   case USB_DT_CONFIG:
> > +   desc = &ast_vhub_conf_desc;
> > +   dsize = AST_VHUB_CONF_DESC_SIZE;
> > +   break;
> > +   case USB_DT_HUB:
> > +   desc = &ast_vhub_hub_desc;
> > +   dsize = AST_VHUB_HUB_DESC_SIZE;
> > +   break;
> > +   default:
> > +   return std_req_stall;
> > +   }
> > +   if (dsize < len)
> > +   len = dsize;
> > +   /*
> > +* This is our limit for hub replies in our current
> > +* implementation, keeps things simpler.
> > +*/
> > +   if (WARN_ON(len >= AST_VHUB_EP0_MAX_PACKET))
> > +   len = AST_VHUB_EP0_MAX_PACKET - 1;
> > +
> > +   /*
> > +* Copy first to EP buffer and send from there, so
> > +* we can do some in-place patching if needed
> > +*/
> > +   memcpy(ep->buf, desc, len);
> 
> still fails to compile here the same way.
> 
> $ gcc --version
> gcc (Debian 7.3.0-12) 7.3.0

Hrm, no idea what to do about it short of installing myself a debian on
some machine here. Is that an x86 compiler or a cross compiler ?

It really looks like a compiler bug to me...

> > +
> > +   /* Patch it if forcing USB1 */
> > +   if (desc_type == USB_DT_DEVICE && ep->vhub->force_usb1)
> > +   ast_vhub_patch_dev_desc_usb1(ep->buf);
> > +
> > +   /* Shoot it from the EP buffer */
> > +   return ast_vhub_reply(ep, NULL, len);
> > +}
> > +
> > +/* copied from hdc.c */
> > +static unsigned
> > +ascii2desc(char const *s, u8 *buf, unsigned len)
> 
> seems it would be best to move this helper to usb common, or something
> like that. Copying is not a good idea.

Yeah I can do that, would have to pick a better name though ;-)

> > +static int ast_vhub_rep_string(struct ast_vhub_ep *ep,
> > +  u8 string_id, u16 lang_id,
> > +  u16 len)
> 
> spaces for indentation?

An accident, not sure how those snuck in, I'll fix.

Ben.

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

Re: [RFC usb-next v3 1/2] usb: core: split usb_phy_roothub_{init,alloc}

2018-03-27 Thread Roger Quadros 
On 26/03/18 23:38, Martin Blumenstingl wrote:
> Before this patch usb_phy_roothub_init served two purposes (from a
> caller's point of view - like hcd.c):
> - parsing the PHYs and allocating the list entries
> - calling phy_init on each list entry
> 
> While this worked so far it has one disadvantage: if we need to call
> phy_init for each PHY instance then the existing code cannot be re-used.
> Solve this by splitting off usb_phy_roothub_alloc which only parses the
> PHYs and allocates the list entries.
> usb_phy_roothub_init then gets a struct usb_phy_roothub and only calls
> phy_init on each PHY instance (along with the corresponding cleanup if
> that failed somewhere).
> 
> This is a preparation step for adding proper suspend support for some
> hardware that requires phy_exit to be called during suspend and phy_init
> to be called during resume.
> 
> Signed-off-by: Martin Blumenstingl 

I don't think we need RFC in subject.

Reviewed-by: Roger Quadros 

> ---
>  drivers/usb/core/hcd.c | 10 +++---
>  drivers/usb/core/phy.c | 53 
> +-
>  drivers/usb/core/phy.h |  4 +++-
>  3 files changed, 37 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 777036ae6367..15b0418e3b6a 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2758,12 +2758,16 @@ int usb_add_hcd(struct usb_hcd *hcd,
>   }
>  
>   if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) {
> - hcd->phy_roothub = usb_phy_roothub_init(hcd->self.sysdev);
> + hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev);
>   if (IS_ERR(hcd->phy_roothub)) {
>   retval = PTR_ERR(hcd->phy_roothub);
> - goto err_phy_roothub_init;
> + goto err_phy_roothub_alloc;
>   }
>  
> + retval = usb_phy_roothub_init(hcd->phy_roothub);
> + if (retval)
> + goto err_phy_roothub_alloc;
> +
>   retval = usb_phy_roothub_power_on(hcd->phy_roothub);
>   if (retval)
>   goto err_usb_phy_roothub_power_on;
> @@ -2936,7 +2940,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>   usb_phy_roothub_power_off(hcd->phy_roothub);
>  err_usb_phy_roothub_power_on:
>   usb_phy_roothub_exit(hcd->phy_roothub);
> -err_phy_roothub_init:
> +err_phy_roothub_alloc:
>   if (hcd->remove_phy && hcd->usb_phy) {
>   usb_phy_shutdown(hcd->usb_phy);
>   usb_put_phy(hcd->usb_phy);
> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
> index f19aaa3c899c..44f008cda7a8 100644
> --- a/drivers/usb/core/phy.c
> +++ b/drivers/usb/core/phy.c
> @@ -19,19 +19,6 @@ struct usb_phy_roothub {
>   struct list_headlist;
>  };
>  
> -static struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
> -{
> - struct usb_phy_roothub *roothub_entry;
> -
> - roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
> - if (!roothub_entry)
> - return ERR_PTR(-ENOMEM);
> -
> - INIT_LIST_HEAD(&roothub_entry->list);
> -
> - return roothub_entry;
> -}
> -
>  static int usb_phy_roothub_add_phy(struct device *dev, int index,
>  struct list_head *list)
>  {
> @@ -45,9 +32,11 @@ static int usb_phy_roothub_add_phy(struct device *dev, int 
> index,
>   return PTR_ERR(phy);
>   }
>  
> - roothub_entry = usb_phy_roothub_alloc(dev);
> - if (IS_ERR(roothub_entry))
> - return PTR_ERR(roothub_entry);
> + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
> + if (!roothub_entry)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&roothub_entry->list);
>  
>   roothub_entry->phy = phy;
>  
> @@ -56,11 +45,9 @@ static int usb_phy_roothub_add_phy(struct device *dev, int 
> index,
>   return 0;
>  }
>  
> -struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev)
> +struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
>  {
>   struct usb_phy_roothub *phy_roothub;
> - struct usb_phy_roothub *roothub_entry;
> - struct list_head *head;
>   int i, num_phys, err;
>  
>   num_phys = of_count_phandle_with_args(dev->of_node, "phys",
> @@ -68,16 +55,31 @@ struct usb_phy_roothub *usb_phy_roothub_init(struct 
> device *dev)
>   if (num_phys <= 0)
>   return NULL;
>  
> - phy_roothub = usb_phy_roothub_alloc(dev);
> - if (IS_ERR(phy_roothub))
> - return phy_roothub;
> + phy_roothub = devm_kzalloc(dev, sizeof(*phy_roothub), GFP_KERNEL);
> + if (!phy_roothub)
> + return ERR_PTR(-ENOMEM);
> +
> + INIT_LIST_HEAD(&phy_roothub->list);
>  
>   for (i = 0; i < num_phys; i++) {
>   err = usb_phy_roothub_add_phy(dev, i, &phy_roothub->list);
>   if (err)
> - goto err_out;

Re: [RFC usb-next v3 2/2] usb: core: use phy_exit during suspend if wake up is not supported

2018-03-27 Thread Roger Quadros 
On 26/03/18 23:38, Martin Blumenstingl wrote:
> If the USB controller can wake up the system (which is the case for
> example with the Mediatek USB3 IP) then we must not call phy_exit during
> suspend to ensure that the USB controller doesn't have to re-enumerate
> the devices during resume.
> However, if the USB controller cannot wake up the system (which is the
> case for example on various TI platforms using a dwc3 controller) then
> we must call phy_exit during suspend. Otherwise the PHY driver keeps the
> clocks enabled, which prevents the system from reaching the lowest power
> levels in the suspend state.
> 
> Solve this by introducing two new functions in the PHY wrapper which are
> dedicated to the suspend and resume handling.
> If the controller can wake up the system the new usb_phy_roothub_suspend
> function will simply call usb_phy_roothub_power_off. However, if wake up
> is not supported by the controller it will also call
> usb_phy_roothub_exit.
> The also new usb_phy_roothub_resume function takes care of calling
> usb_phy_roothub_init (if the controller can't wake up the system) in
> addition to usb_phy_roothub_power_on.
> 
> Fixes: 07dbff0ddbd86c ("usb: core: add a wrapper for the USB PHYs on the HCD")
> Fixes: 178a0bce05cbc1 ("usb: core: hcd: integrate the PHY wrapper into the 
> HCD core")
> Reported-by: Roger Quadros 
> Suggested-by: Roger Quadros 
> Suggested-by: Chunfeng Yun 
> Signed-off-by: Martin Blumenstingl 

Reviewed-by: Roger Quadros 

> ---
>  drivers/usb/core/hcd.c |  8 +---
>  drivers/usb/core/phy.c | 35 +++
>  drivers/usb/core/phy.h |  5 +
>  3 files changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 15b0418e3b6a..78bae4ecd68b 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2262,7 +2262,8 @@ int hcd_bus_suspend(struct usb_device *rhdev, 
> pm_message_t msg)
>   hcd->state = HC_STATE_SUSPENDED;
>  
>   if (!PMSG_IS_AUTO(msg))
> - usb_phy_roothub_power_off(hcd->phy_roothub);
> + usb_phy_roothub_suspend(hcd->self.sysdev,
> + hcd->phy_roothub);
>  
>   /* Did we race with a root-hub wakeup event? */
>   if (rhdev->do_remote_wakeup) {
> @@ -2302,7 +2303,8 @@ int hcd_bus_resume(struct usb_device *rhdev, 
> pm_message_t msg)
>   }
>  
>   if (!PMSG_IS_AUTO(msg)) {
> - status = usb_phy_roothub_power_on(hcd->phy_roothub);
> + status = usb_phy_roothub_resume(hcd->self.sysdev,
> + hcd->phy_roothub);
>   if (status)
>   return status;
>   }
> @@ -2344,7 +2346,7 @@ int hcd_bus_resume(struct usb_device *rhdev, 
> pm_message_t msg)
>   }
>   } else {
>   hcd->state = old_state;
> - usb_phy_roothub_power_off(hcd->phy_roothub);
> + usb_phy_roothub_suspend(hcd->self.sysdev, hcd->phy_roothub);
>   dev_dbg(&rhdev->dev, "bus %s fail, err %d\n",
>   "resume", status);
>   if (status != -ESHUTDOWN)
> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
> index 44f008cda7a8..a39d9bb26a4f 100644
> --- a/drivers/usb/core/phy.c
> +++ b/drivers/usb/core/phy.c
> @@ -157,3 +157,38 @@ void usb_phy_roothub_power_off(struct usb_phy_roothub 
> *phy_roothub)
>   phy_power_off(roothub_entry->phy);
>  }
>  EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off);
> +
> +int usb_phy_roothub_suspend(struct device *controller_dev,
> + struct usb_phy_roothub *phy_roothub)
> +{
> + usb_phy_roothub_power_off(phy_roothub);
> +
> + /* keep the PHYs initialized so the device can wake up the system */
> + if (device_may_wakeup(controller_dev))
> + return 0;
> +
> + return usb_phy_roothub_exit(phy_roothub);
> +}
> +EXPORT_SYMBOL_GPL(usb_phy_roothub_suspend);
> +
> +int usb_phy_roothub_resume(struct device *controller_dev,
> +struct usb_phy_roothub *phy_roothub)
> +{
> + int err;
> +
> + /* if the device can't wake up the system _exit was called */
> + if (!device_may_wakeup(controller_dev)) {
> + err = usb_phy_roothub_init(phy_roothub);
> + if (err)
> + return err;
> + }
> +
> + err = usb_phy_roothub_power_on(phy_roothub);
> +
> + /* undo _init if _power_on failed */
> + if (err && !device_may_wakeup(controller_dev))
> + usb_phy_roothub_exit(phy_roothub);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(usb_phy_roothub_resume);
> diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h
> index eb31253201ad..60901d44 100644
> --- a/drivers/usb/core/phy.h
> +++ b/drivers/usb/core/phy.h
> @@ -7,3 +7,8 @@ int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub);
>  
>  int us

Re: [RFC usb-next v3 1/2] usb: core: split usb_phy_roothub_{init,alloc}

2018-03-27 Thread Greg KH 
On Tue, Mar 27, 2018 at 11:24:08AM +0300, Roger Quadros wrote:
> On 26/03/18 23:38, Martin Blumenstingl wrote:
> > Before this patch usb_phy_roothub_init served two purposes (from a
> > caller's point of view - like hcd.c):
> > - parsing the PHYs and allocating the list entries
> > - calling phy_init on each list entry
> > 
> > While this worked so far it has one disadvantage: if we need to call
> > phy_init for each PHY instance then the existing code cannot be re-used.
> > Solve this by splitting off usb_phy_roothub_alloc which only parses the
> > PHYs and allocates the list entries.
> > usb_phy_roothub_init then gets a struct usb_phy_roothub and only calls
> > phy_init on each PHY instance (along with the corresponding cleanup if
> > that failed somewhere).
> > 
> > This is a preparation step for adding proper suspend support for some
> > hardware that requires phy_exit to be called during suspend and phy_init
> > to be called during resume.
> > 
> > Signed-off-by: Martin Blumenstingl 
> 
> I don't think we need RFC in subject.

As I don't apply series with RFC in the subject, that might be a good
idea :)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 4.14.13: Kernel panic, NULL pointer dereference in xhci_hcd

2018-03-27 Thread Mathias Nyman 

On 25.03.2018 14:31, Juho Tykkälä wrote:

Hello,

Similar panic occurs on manually compiled 4.14.0.

However, same issue occurs like bellow on 4.9.82 (debian compiled with package 
linux-image-4.9.0-6-amd64)

One failure behavior:

[70684.574901] xhci_hcd :00:14.0: ERROR Transfer event for disabled 
endpoint slot 12 ep 17
[70684.574914] xhci_hcd :00:14.0: @000449d69a30 49fbd350 0044 
0100 0cd28001
[70684.575860] xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part 
of current TD ep_index 11 comp_code 1
[70684.575875] xhci_hcd :00:14.0: Looking for event-dma 000449fbd360 
trb-start 000449fbd350 trb-end 000449fbd350 seg-start 000449fbd000 
seg-end 000449fbdff0

Another failure behavior:

[ 6880.362440] do_IRQ: 2.192 No irq handler for vector
[ 6898.818312] xhci_hcd :00:14.0: xHCI host not responding to stop endpoint 
command.
[ 6898.818334] xhci_hcd :00:14.0: xHCI host controller not responding, 
assume dead
[ 6898.818377] xhci_hcd :00:14.0: HC died; cleaning up
[ 6898.821178] usb 1-1: USB disconnect, device number 4
[ 6898.821279] usb 2-3: USB disconnect, device number 2
[ 6898.886250] usb 1-3: USB disconnect, device number 2
[ 6898.886377] usb 1-4: USB disconnect, device number 3

I believe the origin of the issue in hand is the same as in earlier panicing. 
Machine is the same and interval of the occurence is similar.

The good thing is no panic with this kernel. The bad thing is no thunderbolt 
access control with this kernel.



Could you enable xhci traces with 4.14 or later, and send me the output:

mount -t debugfs none /sys/kernel/debug
echo 81920 > /sys/kernel/debug/tracing/buffer_size_kb
echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable

Then send me the /sys/kernel/debug/tracing/trace file after issue is triggered

If it has been tracing for hours without triggering, the trace could be
cleared every now and then with:
echo 0 >  /sys/kernel/debug/tracing/trace

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

[PATCH v2 1/2] usb: dwc3: gadget: Correct handling of scattergather lists

2018-03-27 Thread v.anuragkumar 
From: Anurag Kumar Vulisha 

The code logic in dwc3_prepare_one_trb() incorrectly uses the address
and length fields present in req packet for mapping TRB's instead of
using the address and length fields of scattergather lists. This patch
correct's the code to use sg->address and sg->length when scattergather
lists are present.

Signed-off-by: Anurag Kumar Vulisha 
---
 Changes in v2:
  1. Split the single patch into 2 patches as suggested by Felipe Balbi
  2. Renamed sg_to_start variable to start_sg 
---
 drivers/usb/dwc3/core.h   |  2 ++
 drivers/usb/dwc3/gadget.c | 25 ++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 860d2bc..1406c4f 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -718,6 +718,7 @@ struct dwc3_hwparams {
  * @list: a list_head used for request queueing
  * @dep: struct dwc3_ep owning this request
  * @sg: pointer to first incomplete sg
+ * @start_sg: pointer to the sg which should be queued next
  * @num_pending_sgs: counter to pending sgs
  * @remaining: amount of data remaining
  * @epnum: endpoint number to which this request refers
@@ -734,6 +735,7 @@ struct dwc3_request {
struct list_headlist;
struct dwc3_ep  *dep;
struct scatterlist  *sg;
+   struct scatterlist  *start_sg;
 
unsignednum_pending_sgs;
unsignedremaining;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 2bda4eb..330a4de 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -978,11 +978,19 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
struct dwc3_request *req, unsigned chain, unsigned node)
 {
struct dwc3_trb *trb;
-   unsignedlength = req->request.length;
+   unsigned intlength;
+   dma_addr_t  dma;
unsignedstream_id = req->request.stream_id;
unsignedshort_not_ok = req->request.short_not_ok;
unsignedno_interrupt = req->request.no_interrupt;
-   dma_addr_t  dma = req->request.dma;
+
+   if (req->request.num_sgs > 0) {
+   length = sg_dma_len(req->start_sg);
+   dma = sg_dma_address(req->start_sg);
+   } else {
+   length = req->request.length;
+   dma = req->request.dma;
+   }
 
trb = &dep->trb_pool[dep->trb_enqueue];
 
@@ -1048,7 +1056,7 @@ static u32 dwc3_calc_trbs_left(struct dwc3_ep *dep)
 static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
struct dwc3_request *req)
 {
-   struct scatterlist *sg = req->sg;
+   struct scatterlist *sg = req->start_sg;
struct scatterlist *s;
int i;
 
@@ -1081,6 +1089,16 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
dwc3_prepare_one_trb(dep, req, chain, i);
}
 
+   /*
+* There can be a situation where all sgs in sglist are not
+* queued because of insufficient trb number. To handle this
+* case, update start_sg to next sg to be queued, so that
+* we have free trbs we can continue queuing from where we
+* previously stopped
+*/
+   if (chain)
+   req->start_sg = sg_next(s);
+
if (!dwc3_calc_trbs_left(dep))
break;
}
@@ -1171,6 +1189,7 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep)
return;
 
req->sg = req->request.sg;
+   req->start_sg   = req->sg;
req->num_pending_sgs= req->request.num_mapped_sgs;
 
if (req->num_pending_sgs > 0)
-- 
2.1.1

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

[PATCH v2 2/2] usb: dwc3: gadget: Correct the logic for queuing sgs

2018-03-27 Thread v.anuragkumar 
From: Anurag Kumar Vulisha 

The present code correctly fetches the req which were previously not
queued from the started_list but fails to continue queuing from the sg
where it previously stopped queuing (because of the unavailable TRB's).
This patch correct's the code to continue queuing from the correct sg
present in the sglist.

For example, consider 5 sgs in req. Because of limited TRB's among the
5 sgs only 3 got queued. This patch corrects the code to start queuing
from correct sg i.e 4th sg when the TRBs are available.

Signed-off-by: Anurag Kumar Vulisha 
---
 Changes in v2:
1. Split the single patch into 2 patches as suggested by Felipe Balbi
2. Added dev_WARN_ONCE message when ((req->request.actual == length) &&
   (req->num_pending_sgs > 0))
---
 drivers/usb/dwc3/core.h   |  2 ++
 drivers/usb/dwc3/gadget.c | 23 ---
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 1406c4f..6bce913 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -720,6 +720,7 @@ struct dwc3_hwparams {
  * @sg: pointer to first incomplete sg
  * @start_sg: pointer to the sg which should be queued next
  * @num_pending_sgs: counter to pending sgs
+ * @num_queued_sgs: counter to the number of sgs which already got queued
  * @remaining: amount of data remaining
  * @epnum: endpoint number to which this request refers
  * @trb: pointer to struct dwc3_trb
@@ -738,6 +739,7 @@ struct dwc3_request {
struct scatterlist  *start_sg;
 
unsignednum_pending_sgs;
+   unsigned intnum_queued_sgs;
unsignedremaining;
u8  epnum;
struct dwc3_trb *trb;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 330a4de..ff62e13 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1060,7 +1060,10 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
struct scatterlist *s;
int i;
 
-   for_each_sg(sg, s, req->num_pending_sgs, i) {
+   unsigned int remaining = req->request.num_mapped_sgs
+   - req->num_queued_sgs;
+
+   for_each_sg(sg, s, remaining, i) {
unsigned int length = req->request.length;
unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
unsigned int rem = length % maxp;
@@ -1099,6 +1102,8 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
if (chain)
req->start_sg = sg_next(s);
 
+   req->num_queued_sgs++;
+
if (!dwc3_calc_trbs_left(dep))
break;
}
@@ -1190,6 +1195,7 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep)
 
req->sg = req->request.sg;
req->start_sg   = req->sg;
+   req->num_queued_sgs = 0;
req->num_pending_sgs= req->request.num_mapped_sgs;
 
if (req->num_pending_sgs > 0)
@@ -2346,8 +2352,19 @@ static int dwc3_cleanup_done_reqs(struct dwc3 *dwc, 
struct dwc3_ep *dep,
 
req->request.actual = length - req->remaining;
 
-   if ((req->request.actual < length) && req->num_pending_sgs)
-   return __dwc3_gadget_kick_transfer(dep);
+   if (req->request.actual < length || req->num_pending_sgs) {
+   /*
+* There could be a scenario where the whole req can't
+* be mapped into available TRB's. In that case, we need
+* to kick transfer again if (req->num_pending_sgs > 0)
+*/
+   if (req->num_pending_sgs) {
+   dev_WARN_ONCE(dwc->dev,
+ (req->request.actual == length),
+ "There are some pending sg's that 
needs to be queued again\n");
+   return __dwc3_gadget_kick_transfer(dep);
+   }
+   }
 
dwc3_gadget_giveback(dep, req, status);
 
-- 
2.1.1

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

Re: [PATCH 1/2] usb: gadget: udc: core: update usb_ep_queue() documentation

2018-03-27 Thread Alan Stern 
On Tue, 27 Mar 2018, Felipe Balbi wrote:

> 
> Hi,
> 
> Alan Stern  writes:
> > On Mon, 26 Mar 2018, Felipe Balbi wrote:
> >
> >> Mention that ->complete() should never be called from within
> >> usb_ep_queue().
> >> 
> >> Signed-off-by: Felipe Balbi 
> >> ---
> >>  drivers/usb/gadget/udc/core.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >> 
> >> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> >> index 50988b21a21b..842814bc0e4f 100644
> >> --- a/drivers/usb/gadget/udc/core.c
> >> +++ b/drivers/usb/gadget/udc/core.c
> >> @@ -238,6 +238,9 @@ EXPORT_SYMBOL_GPL(usb_ep_free_request);
> >>   * arranges to poll once per interval, and the gadget driver usually will
> >>   * have queued some data to transfer at that time.
> >>   *
> >> + * Note that @req's ->complete() callback must never be called from
> >> + * within usb_ep_queue() as that can create deadlock situations.
> >> + *
> >
> > I think this is highly questionable.  Certainly it was not David 
> > Brownell's original intention; his dummy-hcd driver will sometimes 
> > give back a request from within usb_ep_queue() -- and I believe he 
> > wrote it that way in order to emulate a feature of his net2280 driver.
> >
> > In this particular case, the problem is that a driver acquires a 
> > spinlock in its complete() routine, but then it holds that same 
> > spinlock while submitting a request.  This is a bug; it should be fixed 
> > in the driver.  The spinlock should be dropped while the request is 
> > submitted.  I'm sure there are examples whether other drivers do this.
> 
> usb_ep_queue() can be called from atomic, there's no explicit
> requirement that locks should be released. Either one case or the other
> should be made explicit.

Agreed.  The requirement should be that a routine calling
usb_ep_queue() should not hold any locks which can be acquired by the
request's completion handler.  This is independent of whether the call
is made in process context or interrupt/atomic context.

Alan Stern

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

[PATCH] usb: gadget: udc: core: Document the relation between usb_ep_queue() and completion callback

2018-03-27 Thread Alan Stern 
Improve the kerneldoc for usb_ep_queue() to note explicitly that the
request's completion routine will be called if and only if the return
value is 0.  The corresponding fact about usb_submit_urb() for the
host-side API has long been documented, and this has always been the
intention for the gadget API.  But until now, documentation seems to
have been lacking.

Signed-off-by: Alan Stern 

---


[as1861]


 drivers/usb/gadget/udc/core.c |6 ++
 1 file changed, 6 insertions(+)

Index: usb-4.x/drivers/usb/gadget/udc/core.c
===
--- usb-4.x.orig/drivers/usb/gadget/udc/core.c
+++ usb-4.x/drivers/usb/gadget/udc/core.c
@@ -241,6 +241,12 @@ EXPORT_SYMBOL_GPL(usb_ep_free_request);
  * Returns zero, or a negative error code.  Endpoints that are not enabled
  * report errors; errors will also be
  * reported when the usb peripheral is disconnected.
+ *
+ * If and only if @req is successfully queued (the return value is zero),
+ * @req->complete() will be called exactly once, when the Gadget core and
+ * UDC are finished with the request.  When the completion function is called,
+ * control of the request is returned to the device driver which submitted it.
+ * The completion handler may then immediately free or reuse @req.
  */
 int usb_ep_queue(struct usb_ep *ep,
   struct usb_request *req, gfp_t gfp_flags)

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

Re: [PATCH v4 2/2] dt-bindings: usb: rt1711h device tree binding document

2018-03-27 Thread 李書帆 
Hi Rob,

2018-03-27 6:23 GMT+08:00 Rob Herring :
> On Tue, Mar 20, 2018 at 05:15:04PM +0800, ShuFan Lee wrote:
>> From: ShuFan Lee 
>>
>> Add device tree binding document for Richtek RT1711H Type-C chip driver
>>
>> Signed-off-by: ShuFan Lee 
>> ---
>>  .../devicetree/bindings/usb/richtek,rt1711h.txt| 30 
>> ++
>>  1 file changed, 30 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/usb/richtek,rt1711h.txt
>>
>>  changelogs between v2 & v3
>>  - add dt-bindings for rt1711h typec driver
>>
>> diff --git a/Documentation/devicetree/bindings/usb/richtek,rt1711h.txt 
>> b/Documentation/devicetree/bindings/usb/richtek,rt1711h.txt
>> new file mode 100644
>> index ..7da4dac78ea7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/richtek,rt1711h.txt
>> @@ -0,0 +1,30 @@
>> +Richtek RT1711H TypeC PD Controller.
>> +
>> +Required properties:
>> + - compatible : Must be "richtek,rt1711h".
>> + - reg : Must be 0x4e, it's slave address of RT1711H.
>> +
>> +Recommended properties :
>> + - interrupt-parent : the phandle for the interrupt controller that
>> +   provides interrupts for this device.
>> + - interrupts :  where a is the interrupt number and b represents an
>> +   encoding of the sense and level information for the interrupt.
>> +
>> +Optional properties :
>> + - rt,intr-gpios : IRQ GPIO pin that's connected to RT1711H interrupt.
>> +   if interrupt-parent & interrupts are not defined, use this property 
>> instead.
>
> Drop this. You should simply always have interrupts property.
Does this also imply that we could always assume client->irq is ready
for request?
Therefore, there's no need to check client->irq and get gpio through
rt,intr-gpios.
>
> Rob



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

Re: 4.14.13: Kernel panic, NULL pointer dereference in xhci_hcd

2018-03-27 Thread Juho Tykkälä 

On 2018-03-27 13:20, Mathias Nyman wrote:


Could you enable xhci traces with 4.14 or later, and send me the 
output:


mount -t debugfs none /sys/kernel/debug
echo 81920 > /sys/kernel/debug/tracing/buffer_size_kb
echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable

Then send me the /sys/kernel/debug/tracing/trace file after issue is 
triggered


If it has been tracing for hours without triggering, the trace could be
cleared every now and then with:
echo 0 >  /sys/kernel/debug/tracing/trace

-Mathias


I would guess it requires at least ftrace_dump_on_oops=1 as one is 
unable

to obtain /sys/kernel/debug/tracing/trace after panic (and I'm using
netconsole as nothing gets written to my lvm luks ssd disk after panic).

Maybe also hung_task_panic=0 is required but trying first without.

I will come back to this issue asap.

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

Re: [v6] usb: ohci: Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks()

2018-03-27 Thread Alan Stern 
On Mon, 26 Mar 2018, Jonathan Liu wrote:

> > I am experiencing a USB function call hang from userspace with OCHI
> > (full speed USB device) after updating from Linux 4.14.15 to 4.14.24
> > and noticed this commit.
> >
> > Here is the Linux 4.14.24 kernel stack trace (extracted from SysRq+w
> > and amended with addr2line):
> > [] (__schedule) from [] (schedule+0x50/0xb4)
> > kernel/sched/core.c:2792
> > [] (schedule) from []
> > (usb_kill_urb.part.3+0x78/0xa8) include/asm-generic/preempt.h:59
> > [] (usb_kill_urb.part.3) from []
> > (usbdev_ioctl+0x1288/0x1cf0) drivers/usb/core/urb.c:690
> > [] (usbdev_ioctl) from []
> > (do_vfs_ioctl+0x9c/0x8ec) drivers/usb/core/devio.c:1835
> > [] (do_vfs_ioctl) from [] (SyS_ioctl+0x34/0x5c)
> > fs/ioctl.c:47
> > [] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x54)
> > include/linux/file.h:39
> >
> > Afterwards the kernel is unresponsive to disconnect/connect of the
> > full speed USB device but I can connect/disconnect a high speed USB
> > device to the same port and communicate to it without problem since it
> > uses EHCI (OHCI is companion controller). If I try to connect the full
> > speed USB device again it is still unresponsive. The userspace
> > application is still hanging after all this.
> >
> > Could this commit be causing the issue?

It sounds like the URB gets cancelled but is never given back.  But 
although the commit you mentioned does affect this logic, I don't see 
the problem could arise.

> There does seem to be something wrong with this commit. I did some
> more testing with Linux 4.16.0-rc6 on sun7i-a20-olinuxino-lime.
> I connected a sun7i-a20-pcduino3b running Linux 4.15.12 that has OTG
> port with gadget zero kernel module (g_zero) loaded to the USB port
> and did some testing with usbtest from tools/usb/testusb.c in kernel
> repository.
> 
> I had to do the following change to avoid getting flooded with
> "usbtest 1-1:3.0: unlink retry" kernel messages after test 11 of
> testusb starts:
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -1473,7 +1473,7 @@ static int unlink1(struct usbtest_dev *dev, int
> pipe, int
> size, int async)
>  * resubmission, but since we're testing 
> unlink
>  * paths, we can't.
>  */
> -   ERROR(dev, "unlink retry\n");
> + /*ERROR(dev, "unlink retry\n");*/

That's a bizarre message in any case.  The condition under which it
gets printed is _not_ an error, just a race.  I would expect it to show
up every now and then just based on random chance.  But if it gets
printed over and over again then something is definitely wrong.

> If I revert the commit, I can run "./testusb -a" to completion without
> any issues with Linux 4.16.0-rc6 on sun7i-a20-olinuxino-lime.
> I don't get a kernel stall detected when testing on x86_64 but I do
> still get at least one "unlink retry" error message from kernel in
> dmesg which can be reproduced using "./testusb -a" or more frequently
> with "./testusb -a -t 11 -c 5000".

A few of those "unlink retry" messages during a test with 5000 
iterations does not mean anything is wrong; don't worry about it.

It would be great if you could debug this farther.  I'm not sure what
to suggest, however.  Clearly we need to know what's going on in
ohci-q.c's finish_unlinks() routine; unfortunately this routine
probably gets called thousands of times before the problem shows up.

Anyway, here's a patch you can try out on the sun7i-a20-olinuxino-lime.  
I have no idea whether it will help pinpoint the source of the problem.

Alan Stern



Index: usb-4.x/drivers/usb/host/ohci-hcd.c
===
--- usb-4.x.orig/drivers/usb/host/ohci-hcd.c
+++ usb-4.x/drivers/usb/host/ohci-hcd.c
@@ -327,14 +327,21 @@ static int ohci_urb_dequeue(struct usb_h
 * some upcoming INTR_SF to call finish_unlinks()
 */
urb_priv = urb->hcpriv;
-   if (urb_priv->ed->state == ED_OPER)
+   if (urb_priv->ed->state == ED_OPER) {
+   ohci_info(ohci, "start dequeue URB %p ed %p\n", urb,
+   urb_priv->ed);
start_ed_unlink(ohci, urb_priv->ed);
-
+   } else
+   ohci_info(ohci, "idle dequeue URB %p ed %p\n", urb,
+   urb_priv->ed);
if (ohci->rh_state != OHCI_RH_RUNNING) {
/* With HC dead, we can clean up right away */
ohci_work(ohci);
}
}
+   else
+   ohci_info(ohci, "failed dequeue URB %p\n", urb);
+
spin_unlock_irqrestore (&ohci->lock, flags);
return rc;
 }
Index: usb-4.x/drivers/usb/host/ohci-q.c
===
--- usb-4

usb: usbtmc: Proposal for new ioctl functions

2018-03-27 Thread Guido . Kiener 
Hi all,

This is a follow up mail from the discussion of thread
 https://marc.info/?l=linux-usb&m=149485247607564&w=2

Our working group "VISA for Linux" (IVI Foundation, www.ivifoundation.org) 

wants to extend the Linux USBTMC driver (linux/drivers/usb/class/usbtmc.c)
that can communicate with the T&M instruments.

To meet our requirements we need some additional ioctl functions that 
can send control, bulk in/out messages in a generic way.

Kindly supported by Dave Penkler there is now a github project at 
https://github.com/dpenkler/linux-usbtmc
that already supports new functions to set/get timeout, EOM flag (End 
of Message), and termchar (detection of termination character).

This project and forked repos are our playground and still under 
development.

Our experimental and generic functions are currently developed at the 
fork:
https://github.com/GuidoKiener/linux-usbtmc

For a fast read/write we want to implement new generic ioctl functions:
#define USBTMC_IOCTL_WRITE  _IOWR(USBTMC_IOC_NR, 13, struct 
usbtmc_message)
#define USBTMC_IOCTL_READ_IOWR(USBTMC_IOC_NR, 14, struct 
usbtmc_message)
#define USBTMC_IOCTL_WRITE_RESULT _IOWR(USBTMC_IOC_NR, 15, __u64)
#define USBTMC488_IOCTL_WAIT_SRQ  _IOW(USBTMC_IOC_NR, 23, unsigned int)
#define USBTMC_IOCTL_CANCEL_IO_IO(USBTMC_IOC_NR, 35)
#define USBTMC_IOCTL_CLEANUP_IO   _IO(USBTMC_IOC_NR, 36)
/* For test purpose only */
#define USBTMC_IOCTL_SET_OUT_HALT _IO(USBTMC_IOC_NR, 30)
#define USBTMC_IOCTL_SET_IN_HALT  _IO(USBTMC_IOC_NR, 31)

For further description please refer to the readme.md file at
https://github.com/GuidoKiener/linux-usbtmc/blob/master/README.md

Open questions and comments can be just added to the wiki:
https://github.com/GuidoKiener/linux-usbtmc/wiki

When our working group is happy with the proposed extensions and the
driver tested by several T&M companies then we would like to submit 
these patches to the Linux kernel.

We are looking forward to your comments. Please let us know if 
some of you would like to participate in more discussions. 
Or let us know if you do not want to get more emails about this topic.

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

[PATCH usb-next v4 0/2] fix HCD PHY suspend handling

2018-03-27 Thread Martin Blumenstingl 
This is a follow-up to my previous series "initialize (multiple) PHYs
for a HCD": [0].

Roger Quadros reported [1] that it "is breaking low power cases on TI
SoCs when USB is in host mode". He further explains that "Not doing the
phy_exit() here [when entering suspend] leaves the clocks enabled on
our SoC and we're no longer able to reach low power states on system
suspend."
Chunfeng Yun from Mediatek noted [2] that we cannot unconditionally call
phy_exit while entering system suspend, because this would "disconnect
plugged devices on MTK platforms, due to re-initialize u2 phys when
resume"

In the discussion (which followed Roger's bug report: [1]) Roger,
Chunfeng and me came to the conclusion that we can fix suspend on the
TI SoCs without breaking it on the Mediatek SoCs by extending the
suspend and resume code in usb/core/phy.c by checking whether the USB
controller can wake up the system (which is the case for the Mediatek
MTU3 controller, but now for the dwc3 controller used on the TI SoCs):
- if the controller can wake up the system (Mediatek MTU3 use-case) we
  only call usb_phy_roothub_power_off (which calls phy_power_off) when
  entering system suspend
- if the controller however cannot wake up the system (dwc3 on TI SoCs)
  we additionally call usb_phy_roothub_exit (which calls phy_exit) when
  entering system suspend
- (we undo the previous steps during system resume)

The goal of this series is to fix the issue reported by Roger without
breaking suspend/resume on the Mediatek SoCs.
Since I neither have a TI nor a Mediatek device I am sending this as
RFC. I have tested it on an Amlogic Meson GXM board (Khadas VIM2) which
does NOT support suspend/resume yet.

this should be applied on top of [3] "usb: core: phy: fix return value
of usb_phy_roothub_exit()" (even though there's no strict dependency,
this is the order I wrote the patches in).

changes since RFC v3 at [6]:
- added Chunfeng Yun's Tested-by and Roger Quadros' Reviewed-by (thank
  you!)
- dropped RFC prefix

changes since RFC v2 at [5]:
- add missing INIT_LIST_HEAD call in usb_phy_roothub_add_phy (affects
  patch #1 - spotted by Roger Quadros, thank you!)
- fixed swapped conditions using device_may_wakeup() in
  usb_phy_roothub_resume because we need to call usb_phy_roothub_init
  if the controller cannot wake up the device (affects patch #2, spotted
  by Chunfeng Yun, thank you!)
- simplified the error condition to "undo" usb_phy_roothub_init if
  usb_phy_roothub_power_on failed in usb_phy_roothub_resume (suggested
  by Chunfeng Yun)
- updated the commit message (using Roger's wording) because (quote from
  Roger "it doesn't prevent the system from entering suspend but just
  prevents the system from reaching lowest power levels in the suspend
  state."

Changes since RFC v1 (blob attachments) at [4]:
- use device_may_wakeup instead of device_can_wakeup as suggested by
  Roger Quadros
- use the controller device from hcd->self.controller as suggested by
  Chunfeng Yun
- compile time fixes thanks to Roger Quadros
- if usb_phy_roothub_power_on in usb_phy_roothub_resume failes then
  we now call usb_phy_roothub_exit to keep the PHYs in the correct
  state if usb_phy_roothub_resume partially failed


[0] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006599.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006737.html
[2] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006758.html
[3] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006819.html
[4] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006794.html
[5] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006820.html
[6] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006847.html

Martin Blumenstingl (2):
  usb: core: split usb_phy_roothub_{init,alloc}
  usb: core: use phy_exit during suspend if wake up is not supported

 drivers/usb/core/hcd.c | 18 +++
 drivers/usb/core/phy.c | 88 +++---
 drivers/usb/core/phy.h |  9 +-
 3 files changed, 82 insertions(+), 33 deletions(-)

-- 
2.16.3

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

[PATCH usb-next v4 1/2] usb: core: split usb_phy_roothub_{init,alloc}

2018-03-27 Thread Martin Blumenstingl 
Before this patch usb_phy_roothub_init served two purposes (from a
caller's point of view - like hcd.c):
- parsing the PHYs and allocating the list entries
- calling phy_init on each list entry

While this worked so far it has one disadvantage: if we need to call
phy_init for each PHY instance then the existing code cannot be re-used.
Solve this by splitting off usb_phy_roothub_alloc which only parses the
PHYs and allocates the list entries.
usb_phy_roothub_init then gets a struct usb_phy_roothub and only calls
phy_init on each PHY instance (along with the corresponding cleanup if
that failed somewhere).

This is a preparation step for adding proper suspend support for some
hardware that requires phy_exit to be called during suspend and phy_init
to be called during resume.

Signed-off-by: Martin Blumenstingl 
Tested-by: Chunfeng Yun 
Reviewed-by: Roger Quadros 
---
 drivers/usb/core/hcd.c | 10 +++---
 drivers/usb/core/phy.c | 53 +-
 drivers/usb/core/phy.h |  4 +++-
 3 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae6367..15b0418e3b6a 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2758,12 +2758,16 @@ int usb_add_hcd(struct usb_hcd *hcd,
}
 
if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) {
-   hcd->phy_roothub = usb_phy_roothub_init(hcd->self.sysdev);
+   hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev);
if (IS_ERR(hcd->phy_roothub)) {
retval = PTR_ERR(hcd->phy_roothub);
-   goto err_phy_roothub_init;
+   goto err_phy_roothub_alloc;
}
 
+   retval = usb_phy_roothub_init(hcd->phy_roothub);
+   if (retval)
+   goto err_phy_roothub_alloc;
+
retval = usb_phy_roothub_power_on(hcd->phy_roothub);
if (retval)
goto err_usb_phy_roothub_power_on;
@@ -2936,7 +2940,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
usb_phy_roothub_power_off(hcd->phy_roothub);
 err_usb_phy_roothub_power_on:
usb_phy_roothub_exit(hcd->phy_roothub);
-err_phy_roothub_init:
+err_phy_roothub_alloc:
if (hcd->remove_phy && hcd->usb_phy) {
usb_phy_shutdown(hcd->usb_phy);
usb_put_phy(hcd->usb_phy);
diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
index f19aaa3c899c..44f008cda7a8 100644
--- a/drivers/usb/core/phy.c
+++ b/drivers/usb/core/phy.c
@@ -19,19 +19,6 @@ struct usb_phy_roothub {
struct list_headlist;
 };
 
-static struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
-{
-   struct usb_phy_roothub *roothub_entry;
-
-   roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
-   if (!roothub_entry)
-   return ERR_PTR(-ENOMEM);
-
-   INIT_LIST_HEAD(&roothub_entry->list);
-
-   return roothub_entry;
-}
-
 static int usb_phy_roothub_add_phy(struct device *dev, int index,
   struct list_head *list)
 {
@@ -45,9 +32,11 @@ static int usb_phy_roothub_add_phy(struct device *dev, int 
index,
return PTR_ERR(phy);
}
 
-   roothub_entry = usb_phy_roothub_alloc(dev);
-   if (IS_ERR(roothub_entry))
-   return PTR_ERR(roothub_entry);
+   roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
+   if (!roothub_entry)
+   return -ENOMEM;
+
+   INIT_LIST_HEAD(&roothub_entry->list);
 
roothub_entry->phy = phy;
 
@@ -56,11 +45,9 @@ static int usb_phy_roothub_add_phy(struct device *dev, int 
index,
return 0;
 }
 
-struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev)
+struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
 {
struct usb_phy_roothub *phy_roothub;
-   struct usb_phy_roothub *roothub_entry;
-   struct list_head *head;
int i, num_phys, err;
 
num_phys = of_count_phandle_with_args(dev->of_node, "phys",
@@ -68,16 +55,31 @@ struct usb_phy_roothub *usb_phy_roothub_init(struct device 
*dev)
if (num_phys <= 0)
return NULL;
 
-   phy_roothub = usb_phy_roothub_alloc(dev);
-   if (IS_ERR(phy_roothub))
-   return phy_roothub;
+   phy_roothub = devm_kzalloc(dev, sizeof(*phy_roothub), GFP_KERNEL);
+   if (!phy_roothub)
+   return ERR_PTR(-ENOMEM);
+
+   INIT_LIST_HEAD(&phy_roothub->list);
 
for (i = 0; i < num_phys; i++) {
err = usb_phy_roothub_add_phy(dev, i, &phy_roothub->list);
if (err)
-   goto err_out;
+   return ERR_PTR(err);
}
 
+   return phy_roothub;
+}
+EXPORT_SYMBOL_GPL(usb_phy_roothub_alloc);
+
+int usb_phy_roothub_init(struct usb_phy_roothub *phy_roothub)
+{
+

[PATCH usb-next v4 2/2] usb: core: use phy_exit during suspend if wake up is not supported

2018-03-27 Thread Martin Blumenstingl 
If the USB controller can wake up the system (which is the case for
example with the Mediatek USB3 IP) then we must not call phy_exit during
suspend to ensure that the USB controller doesn't have to re-enumerate
the devices during resume.
However, if the USB controller cannot wake up the system (which is the
case for example on various TI platforms using a dwc3 controller) then
we must call phy_exit during suspend. Otherwise the PHY driver keeps the
clocks enabled, which prevents the system from reaching the lowest power
levels in the suspend state.

Solve this by introducing two new functions in the PHY wrapper which are
dedicated to the suspend and resume handling.
If the controller can wake up the system the new usb_phy_roothub_suspend
function will simply call usb_phy_roothub_power_off. However, if wake up
is not supported by the controller it will also call
usb_phy_roothub_exit.
The also new usb_phy_roothub_resume function takes care of calling
usb_phy_roothub_init (if the controller can't wake up the system) in
addition to usb_phy_roothub_power_on.

Fixes: 07dbff0ddbd86c ("usb: core: add a wrapper for the USB PHYs on the HCD")
Fixes: 178a0bce05cbc1 ("usb: core: hcd: integrate the PHY wrapper into the HCD 
core")
Reported-by: Roger Quadros 
Suggested-by: Roger Quadros 
Suggested-by: Chunfeng Yun 
Signed-off-by: Martin Blumenstingl 
Tested-by: Chunfeng Yun 
Reviewed-by: Roger Quadros 
---
 drivers/usb/core/hcd.c |  8 +---
 drivers/usb/core/phy.c | 35 +++
 drivers/usb/core/phy.h |  5 +
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 15b0418e3b6a..78bae4ecd68b 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2262,7 +2262,8 @@ int hcd_bus_suspend(struct usb_device *rhdev, 
pm_message_t msg)
hcd->state = HC_STATE_SUSPENDED;
 
if (!PMSG_IS_AUTO(msg))
-   usb_phy_roothub_power_off(hcd->phy_roothub);
+   usb_phy_roothub_suspend(hcd->self.sysdev,
+   hcd->phy_roothub);
 
/* Did we race with a root-hub wakeup event? */
if (rhdev->do_remote_wakeup) {
@@ -2302,7 +2303,8 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t 
msg)
}
 
if (!PMSG_IS_AUTO(msg)) {
-   status = usb_phy_roothub_power_on(hcd->phy_roothub);
+   status = usb_phy_roothub_resume(hcd->self.sysdev,
+   hcd->phy_roothub);
if (status)
return status;
}
@@ -2344,7 +2346,7 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t 
msg)
}
} else {
hcd->state = old_state;
-   usb_phy_roothub_power_off(hcd->phy_roothub);
+   usb_phy_roothub_suspend(hcd->self.sysdev, hcd->phy_roothub);
dev_dbg(&rhdev->dev, "bus %s fail, err %d\n",
"resume", status);
if (status != -ESHUTDOWN)
diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
index 44f008cda7a8..a39d9bb26a4f 100644
--- a/drivers/usb/core/phy.c
+++ b/drivers/usb/core/phy.c
@@ -157,3 +157,38 @@ void usb_phy_roothub_power_off(struct usb_phy_roothub 
*phy_roothub)
phy_power_off(roothub_entry->phy);
 }
 EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off);
+
+int usb_phy_roothub_suspend(struct device *controller_dev,
+   struct usb_phy_roothub *phy_roothub)
+{
+   usb_phy_roothub_power_off(phy_roothub);
+
+   /* keep the PHYs initialized so the device can wake up the system */
+   if (device_may_wakeup(controller_dev))
+   return 0;
+
+   return usb_phy_roothub_exit(phy_roothub);
+}
+EXPORT_SYMBOL_GPL(usb_phy_roothub_suspend);
+
+int usb_phy_roothub_resume(struct device *controller_dev,
+  struct usb_phy_roothub *phy_roothub)
+{
+   int err;
+
+   /* if the device can't wake up the system _exit was called */
+   if (!device_may_wakeup(controller_dev)) {
+   err = usb_phy_roothub_init(phy_roothub);
+   if (err)
+   return err;
+   }
+
+   err = usb_phy_roothub_power_on(phy_roothub);
+
+   /* undo _init if _power_on failed */
+   if (err && !device_may_wakeup(controller_dev))
+   usb_phy_roothub_exit(phy_roothub);
+
+   return err;
+}
+EXPORT_SYMBOL_GPL(usb_phy_roothub_resume);
diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h
index eb31253201ad..60901d44 100644
--- a/drivers/usb/core/phy.h
+++ b/drivers/usb/core/phy.h
@@ -7,3 +7,8 @@ int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub);
 
 int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub);
 void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub);
+
+int usb_phy_roothub_suspend(struct device *

Re: usb: usbtmc: Proposal for new ioctl functions

2018-03-27 Thread Greg KH 
On Tue, Mar 27, 2018 at 08:57:27PM +0200, guido.kie...@rohde-schwarz.com wrote:
> Hi all,
> 
> This is a follow up mail from the discussion of thread
>  https://marc.info/?l=linux-usb&m=149485247607564&w=2
> 
> Our working group "VISA for Linux" (IVI Foundation, www.ivifoundation.org) 
> 
> wants to extend the Linux USBTMC driver (linux/drivers/usb/class/usbtmc.c)
> that can communicate with the T&M instruments.
> 
> To meet our requirements we need some additional ioctl functions that 
> can send control, bulk in/out messages in a generic way.
> 
> Kindly supported by Dave Penkler there is now a github project at 
> https://github.com/dpenkler/linux-usbtmc
> that already supports new functions to set/get timeout, EOM flag (End 
> of Message), and termchar (detection of termination character).
> 
> This project and forked repos are our playground and still under 
> development.
> 
> Our experimental and generic functions are currently developed at the 
> fork:
> https://github.com/GuidoKiener/linux-usbtmc
> 
> For a fast read/write we want to implement new generic ioctl functions:
> #define USBTMC_IOCTL_WRITE  _IOWR(USBTMC_IOC_NR, 13, struct 
> usbtmc_message)
> #define USBTMC_IOCTL_READ_IOWR(USBTMC_IOC_NR, 14, struct 
> usbtmc_message)
> #define USBTMC_IOCTL_WRITE_RESULT _IOWR(USBTMC_IOC_NR, 15, __u64)
> #define USBTMC488_IOCTL_WAIT_SRQ  _IOW(USBTMC_IOC_NR, 23, unsigned int)
> #define USBTMC_IOCTL_CANCEL_IO_IO(USBTMC_IOC_NR, 35)
> #define USBTMC_IOCTL_CLEANUP_IO   _IO(USBTMC_IOC_NR, 36)
> /* For test purpose only */
> #define USBTMC_IOCTL_SET_OUT_HALT _IO(USBTMC_IOC_NR, 30)
> #define USBTMC_IOCTL_SET_IN_HALT  _IO(USBTMC_IOC_NR, 31)
> 
> For further description please refer to the readme.md file at
> https://github.com/GuidoKiener/linux-usbtmc/blob/master/README.md
> 
> Open questions and comments can be just added to the wiki:
> https://github.com/GuidoKiener/linux-usbtmc/wiki
> 
> When our working group is happy with the proposed extensions and the
> driver tested by several T&M companies then we would like to submit 
> these patches to the Linux kernel.

Great, submit patches like any other developer when you have them ready.
You don't need to do anything special here, that's how Linux is
developed. :)

Note, please do not depend on these new apis until _after_ we have
merged the patches, as there might be changes required due to the review
process finding problems, so please submit changes as soon as possible.

thanks,

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