[PATCH] usb: xhci: Remove unuseful 'return' statement
Since these 'return' statements are not generally useful in void function, remove them. Signed-off-by: Baolin Wang --- drivers/usb/host/xhci-hub.c |2 -- drivers/usb/host/xhci-mem.c |1 - drivers/usb/host/xhci-ring.c |4 drivers/usb/host/xhci.c |3 --- 4 files changed, 10 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 0ef1690..470ad66 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -444,8 +444,6 @@ void xhci_ring_device(struct xhci_hcd *xhci, int slot_id) xhci_ring_ep_doorbell(xhci, slot_id, i, 0); } } - - return; } static void xhci_disable_port(struct usb_hcd *hcd, struct xhci_hcd *xhci, diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 48a26d378..d6f59a3 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1146,7 +1146,6 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud case USB_SPEED_WIRELESS: xhci_dbg(xhci, "FIXME xHCI doesn't support wireless speeds\n"); return -EINVAL; - break; default: /* Speed was set earlier, this shouldn't happen. */ return -EINVAL; diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index d415911..2057d08 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -157,7 +157,6 @@ static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring) ring->deq_seg = ring->deq_seg->next; ring->dequeue = ring->deq_seg->trbs; } - return; } /* @@ -1167,7 +1166,6 @@ static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, int slot_id, ring_doorbell_for_active_rings(xhci, slot_id, ep_index); return; } - return; } static void xhci_handle_cmd_reset_dev(struct xhci_hcd *xhci, int slot_id, @@ -1258,7 +1256,6 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci, mod_timer(&xhci->cmd_timer, jiffies + XHCI_CMD_DEFAULT_TIMEOUT); xhci_ring_cmd_db(xhci); } - return; } @@ -1307,7 +1304,6 @@ void xhci_handle_command_timeout(unsigned long data) xhci_dbg(xhci, "Command timeout on stopped ring\n"); xhci_handle_stopped_cmd_ring(xhci, xhci->current_cmd); spin_unlock_irqrestore(&xhci->lock, flags); - return; } static void handle_cmd_completion(struct xhci_hcd *xhci, diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index cf30cb6..dc337b3 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -269,8 +269,6 @@ static void xhci_free_irq(struct xhci_hcd *xhci) return; if (pdev->irq > 0) free_irq(pdev->irq, xhci_to_hcd(xhci)); - - return; } /* @@ -351,7 +349,6 @@ static void xhci_cleanup_msix(struct xhci_hcd *xhci) } hcd->msix_enabled = 0; - return; } static void __maybe_unused xhci_msix_sync_irqs(struct xhci_hcd *xhci) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issue with Telit LE922 and cdc_mbim
2016-11-23 19:38 GMT+01:00 Bjørn Mork : > Daniele Palmas writes: >> 2016-11-23 15:49 GMT+01:00 Bjørn Mork : >>> Daniele Palmas writes: 2016-11-21 10:49 GMT+01:00 Bjørn Mork : > Daniele Palmas writes: > >> it turned out that resetting the interface in cdc_ncm_init after >> getting the NTB parameters removes the need for the sleep, making the >> modem to work fine. > > Sounds very good, although I must admit that it isn't perfectly clear to > me what kind of reset we're talking about here. But no worries, the > patch will make that clear :) > Sorry for the confusion, I meant resetting the MBIM function with RESET_FUNCTION request code. >>> >>> Yes, the patch did make that clear, as expected :) >>> >> I wonder if this is an acceptable solution and can be applied also for >> MC7455. > > Quite possible. I will definitely test it. If we can avoid an > arbitrary and pointless delay, then that's great. But I guess this also > requires testing with a wide range of other MBIM devices to find out if > we can apply it unconditionally without breaking anything. Avoiding > device-specific or vendor-specific code is important in a class driver, > if possible. > Ok, unfortunately I can test only with Telit modems, so I'm not sure if the change I did is harmless for all the other modems: RESET_FUNCTION should not cause issues, >>> >>> Agreed. Unfortunately, we cannot depend on sane firmware behaviour ;) >>> >>> I did a semi-quick test on a Sierra Wireless EM7455 and can confirm that >>> your patch makes it work without the delay. Very nice. >>> >> >> Cool! >> >>> Do you happen to know if the Windows MBIM driver use RESET_FUNCTION >>> during initialisation? That would make this feel much safer. >>> >> >> Yes, Windows driver seems to send RESET_FUNCTION two times: after >> getting NTB parameters and after setting NTB input size. But it seems >> that only the first one is mandatory. >> >> Windows USB traces taken with TotalPhase Datacenter are available at >> >> https://drive.google.com/drive/folders/0B1kPnH2g8ISIZWJiV05qeWN5dVE >> >> file LE922win10_2.tdc > > This is comforting. It means that the risk introducing the > RESET_FUNCTION call is close to zero. The only remaining issue is > whether we can safely remove the altsetting toggle. > >>> I see that your testing also included Intel based modems. That's good. >>> It would still be nice to have test results from a few more MBIM >>> implementations, in particular the more "difficult" ones. But I don't >> >> I agree, especially Huawei ones for which the alternate setting >> toggling was introduced. > > I'll see what I can do about that. Looks like I have to dig through my > mailbox. It seems I didn't add any Reported-by tags on the relevant > commits. Wonder why... Well, there you have the reason why you always > should. Now I got to do the work instead of just loading it on you :) > >>> know how much help I can promise here. At the moment I don't even know >>> which box my modems are in. Moved a month ago and am still living in a >>> box. Or more like a hundred boxes ;) >>> >>> The easiest way to get this thoroughly tested is of course to push it >>> now, and try to respond quickly in case it breaks something. Don't know >>> what others think about that? >>> but I also removed altsetting toggling for all MBIM modems and maybe this is not acceptable. >>> >>> I wonder about that. It was added specifically for modems which seemed >>> to need it. Don't remember if we ever tried RESET_FUNCTION as an >>> alternative. Removing workaorunds which have proven to be necessary at >>> some point in time is a bit risky. >>> >> >> I totally agree, but the other way would be to add another quirk at >> the cdc_mbim driver level, isn't it? > > Yes, so I'm definitely with you here - let's try without the quirk > first. I'm ready to say "commit it", if noone else objects. > >> If this is a preferred solution I can try to modify the patch >> according to this path. > > No, the patch is fine as is. Just one question: If keeping the > altsetting toggle proves necessary, will that break the Qualcomm > devices? Yes, I could test that myself, but I'm lazy and I guess you > already have tested it? Unfortunately, yes: for Telit LE922A6 altsetting toggle seems to be part of the problem. Regards, Daniele > > > Bjørn -- 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: [RFT PATCH 1/1] xhci: free xhci virtual devices with leaf nodes first
Hi, Mathias Nyman writes: > the tt_info provided by a HS hub might be in use to by a child device > Make sure we free the devices in the correct order. > > This is needed in special cases such as when xhci controller is > reset when resuming from hibernate, and all virt_devices are freed. > > Also free the virt_devices starting from max slot_id as children > more commonly have higher slot_id than parent. > > CC: > Signed-off-by: Mathias Nyman > > --- > > Guenter Roeck, does this work for you? > > A rework of how tt_info is stored and used might be needed, > but that will take some time and won't go to stable as easily. > --- > drivers/usb/host/xhci-mem.c | 38 -- > 1 file changed, 36 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > index 6afe323..b3a5cd8 100644 > --- a/drivers/usb/host/xhci-mem.c > +++ b/drivers/usb/host/xhci-mem.c > @@ -979,6 +979,40 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int > slot_id) > xhci->devs[slot_id] = NULL; > } > > +/* > + * Free a virt_device structure. > + * If the virt_device added a tt_info (a hub) and has children pointing to > + * 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) > +{ > + struct xhci_virt_device *vdev; > + struct list_head *tt_list_head; > + struct xhci_tt_bw_info *tt_info, *next; > + int i; > + > + vdev = xhci->devs[slot_id]; > + if (!vdev) > + return; > + > + tt_list_head = &(xhci->rh_bw[vdev->real_port - 1].tts); > + list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) { > + /* is this a hub device that added a tt_info to the tts list */ > + if (tt_info->slot_id == slot_id) { if (tt_info->slot_id != slot_id) continue; > + /* are any devices using this tt_info? */ > + for (i = 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) { off-by-one here ? Why is i starting from 1? > + vdev = xhci->devs[i]; > + if (vdev && (vdev->tt_info == tt_info)) if (!vdev || vdev->tt_info != tt_info) continue; > + xhci_free_virt_devices_depth_first( > + xhci, i); > + } > + } > + } > + /* we are now at a leaf device */ > + xhci_free_virt_device(xhci, slot_id); > +} > + > int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id, > struct usb_device *udev, gfp_t flags) > { > @@ -1829,8 +1863,8 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) > } > } > > - for (i = 1; i < MAX_HC_SLOTS; ++i) > - xhci_free_virt_device(xhci, i); > + for (i = HCS_MAX_SLOTS(xhci->hcs_params1); i > 0; i--) converting MAX_HC_SLOTS to HCS_MAX_SLOTS(xhci->hcs_params1) seems unrelated to $subject. Perhaps just mention on commit log? -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: xhci: Remove unuseful 'return' statement
Hello. On 11/24/2016 11:13 AM, Baolin Wang wrote: Since these 'return' statements are not generally useful in void function, remove them. Signed-off-by: Baolin Wang [...] diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 48a26d378..d6f59a3 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1146,7 +1146,6 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud case USB_SPEED_WIRELESS: xhci_dbg(xhci, "FIXME xHCI doesn't support wireless speeds\n"); return -EINVAL; - break; Here you're removing *break* instead but not mentioning this in the patch description... [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fsl/usb: Workarourd for USB erratum-A005697
As per USB specification, in the Suspend state, the status bit does not change until the port is suspended. However, there may be a delay in suspending a port if there is a transaction currently in progress on the bus. In the USBDR controller, the PORTSCx[SUSP] bit changes immediately when the application sets it and not when the port is actually suspended Workaround for this issue involves waiting for a minimum of 10ms to allow the controller to go into SUSPEND state before proceeding ahead Signed-off-by: Changming Huang Signed-off-by: Ramneek Mehresh --- drivers/usb/host/ehci-fsl.c |3 +++ drivers/usb/host/ehci-hub.c |2 ++ drivers/usb/host/ehci.h |6 ++ drivers/usb/host/fsl-mph-dr-of.c |2 ++ include/linux/fsl_devices.h |1 + 5 files changed, 14 insertions(+) diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c index 9f5ffb6..91701cc 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/drivers/usb/host/ehci-fsl.c @@ -286,6 +286,9 @@ static int ehci_fsl_usb_setup(struct ehci_hcd *ehci) if (pdata->has_fsl_erratum_a005275 == 1) ehci->has_fsl_hs_errata = 1; + if (pdata->has_fsl_erratum_a005697 == 1) + ehci->has_fsl_susp_errata = 1; + if ((pdata->operating_mode == FSL_USB2_DR_HOST) || (pdata->operating_mode == FSL_USB2_DR_OTG)) if (ehci_fsl_setup_phy(hcd, pdata->phy_mode, 0)) diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index 74f62d6..86d154e 100644 --- a/drivers/usb/host/ehci-hub.c +++ b/drivers/usb/host/ehci-hub.c @@ -305,6 +305,8 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) USB_PORT_STAT_HIGH_SPEED) fs_idle_delay = true; ehci_writel(ehci, t2, reg); + if (ehci_has_fsl_susp_errata(ehci)) + mdelay(10); changed = 1; } } diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h index 3f3b74a..7706e4a 100644 --- a/drivers/usb/host/ehci.h +++ b/drivers/usb/host/ehci.h @@ -219,6 +219,7 @@ struct ehci_hcd { /* one per controller */ unsignedno_selective_suspend:1; unsignedhas_fsl_port_bug:1; /* FreeScale */ unsignedhas_fsl_hs_errata:1;/* Freescale HS quirk */ + unsignedhas_fsl_susp_errata:1; /*Freescale SUSP quirk*/ unsignedbig_endian_mmio:1; unsignedbig_endian_desc:1; unsignedbig_endian_capbase:1; @@ -703,10 +704,15 @@ struct ehci_tt { #if defined(CONFIG_PPC_85xx) /* Some Freescale processors have an erratum (USB A-005275) in which * incoming packets get corrupted in HS mode + * Some Freescale processors have an erratum (USB A-005697) in which + * we need to wait for 10ms for bus to fo into suspend mode after + * setting SUSP bit */ #define ehci_has_fsl_hs_errata(e) ((e)->has_fsl_hs_errata) +#define ehci_has_fsl_susp_errata(e)((e)->has_fsl_susp_errata) #else #define ehci_has_fsl_hs_errata(e) (0) +#define ehci_has_fsl_susp_errata(e)(0) #endif /* diff --git a/drivers/usb/host/fsl-mph-dr-of.c b/drivers/usb/host/fsl-mph-dr-of.c index f07ccb2..e90ddb5 100644 --- a/drivers/usb/host/fsl-mph-dr-of.c +++ b/drivers/usb/host/fsl-mph-dr-of.c @@ -226,6 +226,8 @@ static int fsl_usb2_mph_dr_of_probe(struct platform_device *ofdev) of_property_read_bool(np, "fsl,usb-erratum-a007792"); pdata->has_fsl_erratum_a005275 = of_property_read_bool(np, "fsl,usb-erratum-a005275"); + pdata->has_fsl_erratum_a005697 = + of_property_read_bool(np, "fsl,usb_erratum-a005697"); /* * Determine whether phy_clk_valid needs to be checked diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h index f291291..60cef82 100644 --- a/include/linux/fsl_devices.h +++ b/include/linux/fsl_devices.h @@ -100,6 +100,7 @@ struct fsl_usb2_platform_data { unsignedalready_suspended:1; unsignedhas_fsl_erratum_a007792:1; unsignedhas_fsl_erratum_a005275:1; + unsignedhas_fsl_erratum_a005697:1; unsignedcheck_phy_clk_valid:1; /* register save area for suspend/resume */ -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT PATCH 1/1] xhci: free xhci virtual devices with leaf nodes first
On 24.11.2016 11:02, Felipe Balbi wrote: Hi, Mathias Nyman writes: the tt_info provided by a HS hub might be in use to by a child device Make sure we free the devices in the correct order. This is needed in special cases such as when xhci controller is reset when resuming from hibernate, and all virt_devices are freed. Also free the virt_devices starting from max slot_id as children more commonly have higher slot_id than parent. CC: Signed-off-by: Mathias Nyman --- Guenter Roeck, does this work for you? A rework of how tt_info is stored and used might be needed, but that will take some time and won't go to stable as easily. --- drivers/usb/host/xhci-mem.c | 38 -- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 6afe323..b3a5cd8 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -979,6 +979,40 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int slot_id) xhci->devs[slot_id] = NULL; } +/* + * Free a virt_device structure. + * If the virt_device added a tt_info (a hub) and has children pointing to + * 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) +{ + struct xhci_virt_device *vdev; + struct list_head *tt_list_head; + struct xhci_tt_bw_info *tt_info, *next; + int i; + + vdev = xhci->devs[slot_id]; + if (!vdev) + return; + + tt_list_head = &(xhci->rh_bw[vdev->real_port - 1].tts); + list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) { + /* is this a hub device that added a tt_info to the tts list */ + if (tt_info->slot_id == slot_id) { if (tt_info->slot_id != slot_id) continue; + /* are any devices using this tt_info? */ + for (i = 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) { off-by-one here ? Why is i starting from 1? + vdev = xhci->devs[i]; slit_id 0 is reserved and xhci->devs[0] is not used, so ne need to check it. All other places that check xhci->devs[0] are avtually buggy -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv12 2/3] usb: USB Type-C connector class
On Wed, Nov 23, 2016 at 09:12:04PM -0800, Guenter Roeck wrote: > Hello Heikki, > > On 11/22/2016 06:11 AM, Heikki Krogerus wrote: > [ ... ] > > + > > +struct typec_port *typec_register_port(struct device *dev, > > + const struct typec_capability *cap) > > +{ > > + struct typec_port *port; > > + int ret; > > + int id; > > + > > + port = kzalloc(sizeof(*port), GFP_KERNEL); > > + if (!port) > > + return ERR_PTR(-ENOMEM); > > + > > + id = ida_simple_get(&typec_index_ida, 0, 0, GFP_KERNEL); > > + if (id < 0) { > > + kfree(port); > > + return ERR_PTR(id); > > + } > > + > > + port->prefer_role = TYPEC_NO_PREFERRED_ROLE; > > + > > Following up on this: > > In our implementation, the default preferred role is determined by the > low level driver (as, in my understanding, is suggested by the standard). > This means that the ABI will report "no preferred role", unless user space > overwrites it, even though there _is_ in fact a preferred role, and the > low level driver will execute try.src or try.snk based on that role. I'm not sure which standard are you referring? Try.SNK and Try.SRC are optional mechanisms for *policy-based* role preference according to the USB Type-C spec. The policy really should always come from the user space in our case, but I don't think that rules out for example initial role preferences coming from the lower level drivers. We will need a way the OS can set the initial preference for every port. Note that once we can support that, what ever the lower level drivers request will be overridden by it. So if for example the platform has preference for an initial role, we will simply ignore it if the policy says otherwise. > It might make sense to add the preferred role to struct typec_capability > and get the initial value from there. If a low level driver does not want > to specify it, it can easily set its value to TYPEC_NO_PREFERRED_ROLE. Well, ideally the port drivers would not need to do anything if there is no preference, but I don't think it's a problem. Since this is API, I guess we can even change this later if we come up with a better way of doing this. I'll add it. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3 v3] xhci: Fix race related to abort operation
Vincent Pelletier writes: > On Wed, 23 Nov 2016 22:48:35 +0900, OGAWA Hirofumi > wrote: >> ping? > > FWIW, I intend to run this patch on the hardware which caused the > issue (thanks Mathias for the CC !). > > So far, in the very short attempt I made, I failed to even build the > kernel (some stack protection feature error when being probed in > gcc...). I should have time to try again this week-end, but please do > not wait for me. If you need my help for something, let me know either publicly or privately what way you want. Well, I can't see why he is waiting something. My patchset is fix to race, and your stuff is to remove unnecessary code, i.e. patchset has no dependency actually. Thanks. -- OGAWA Hirofumi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] USB: ohci: da8xx: Resume the entire host controller
The da8xx ohci controller is not working after suspend and resume. This is because only the root hub is being resumed. Balance the ohci_suspend of the suspend path with an ohci_resume in the resume path so that we resume the entire controller, and not just the root hub. Also, while we are here, remove setting device power_state, as this is no longer needed and scheduled for removal Acked-by: Alan Stern Signed-off-by: Axel Haslam --- Changes v1->v2 * reword commit message (Alan Stern) drivers/usb/host/ohci-da8xx.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c index 9e336f4..05da2cb 100644 --- a/drivers/usb/host/ohci-da8xx.c +++ b/drivers/usb/host/ohci-da8xx.c @@ -528,8 +528,7 @@ static int ohci_da8xx_resume(struct platform_device *dev) if (ret) return ret; - dev->dev.power.power_state = PMSG_ON; - usb_hcd_resume_root_hub(hcd); + ohci_resume(hcd, false); return 0; } -- 2.9.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
Re: [PATCH] usb: xhci: Remove unuseful 'return' statement
Hi, On 24 November 2016 at 17:30, Sergei Shtylyov wrote: > Hello. > > On 11/24/2016 11:13 AM, Baolin Wang wrote: > >> Since these 'return' statements are not generally useful in void function, >> remove them. >> >> Signed-off-by: Baolin Wang > > [...] >> >> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c >> index 48a26d378..d6f59a3 100644 >> --- a/drivers/usb/host/xhci-mem.c >> +++ b/drivers/usb/host/xhci-mem.c >> @@ -1146,7 +1146,6 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd >> *xhci, struct usb_device *ud >> case USB_SPEED_WIRELESS: >> xhci_dbg(xhci, "FIXME xHCI doesn't support wireless >> speeds\n"); >> return -EINVAL; >> - break; > > >Here you're removing *break* instead but not mentioning this in the patch > description... OK. I will add description to explain it in new patch. Thanks. -- Baolin.wang 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: [RFT PATCH 1/1] xhci: free xhci virtual devices with leaf nodes first
Hi, Mathias Nyman writes: >>> + /* are any devices using this tt_info? */ >>> + for (i = 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) { >> >> off-by-one here ? Why is i starting from 1? >> >>> + vdev = xhci->devs[i]; > > slit_id 0 is reserved and xhci->devs[0] is not used, so ne need to > check it. hmm... it's reserved for the HW, sure. Do you need to over allocate the array by 1 just to keep this first member unused? Couldn't you handle the +1/-1 (depending on the case) in xhci driver itself? Saves a bit of memory there. > All other places that check xhci->devs[0] are avtually buggy fair enough, sounds like an accessor guaranteeing the 'rules of engagement' for this would be useful. -- balbi signature.asc Description: PGP signature
[PATCH v2] usb: xhci: Remove unuseful 'return' and 'break' statement
Since these 'return' statements are not generally useful in void function, remove them. Also remove one unuseful 'break' statement in xhci_setup_addressable_virt_dev() function. Signed-off-by: Baolin Wang --- Changes since v1: - Add description of removing 'break' statement in commitlog. --- drivers/usb/host/xhci-hub.c |2 -- drivers/usb/host/xhci-mem.c |1 - drivers/usb/host/xhci-ring.c |4 drivers/usb/host/xhci.c |3 --- 4 files changed, 10 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 0ef1690..470ad66 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -444,8 +444,6 @@ void xhci_ring_device(struct xhci_hcd *xhci, int slot_id) xhci_ring_ep_doorbell(xhci, slot_id, i, 0); } } - - return; } static void xhci_disable_port(struct usb_hcd *hcd, struct xhci_hcd *xhci, diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 48a26d378..d6f59a3 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1146,7 +1146,6 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud case USB_SPEED_WIRELESS: xhci_dbg(xhci, "FIXME xHCI doesn't support wireless speeds\n"); return -EINVAL; - break; default: /* Speed was set earlier, this shouldn't happen. */ return -EINVAL; diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index d415911..2057d08 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -157,7 +157,6 @@ static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring) ring->deq_seg = ring->deq_seg->next; ring->dequeue = ring->deq_seg->trbs; } - return; } /* @@ -1167,7 +1166,6 @@ static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, int slot_id, ring_doorbell_for_active_rings(xhci, slot_id, ep_index); return; } - return; } static void xhci_handle_cmd_reset_dev(struct xhci_hcd *xhci, int slot_id, @@ -1258,7 +1256,6 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci, mod_timer(&xhci->cmd_timer, jiffies + XHCI_CMD_DEFAULT_TIMEOUT); xhci_ring_cmd_db(xhci); } - return; } @@ -1307,7 +1304,6 @@ void xhci_handle_command_timeout(unsigned long data) xhci_dbg(xhci, "Command timeout on stopped ring\n"); xhci_handle_stopped_cmd_ring(xhci, xhci->current_cmd); spin_unlock_irqrestore(&xhci->lock, flags); - return; } static void handle_cmd_completion(struct xhci_hcd *xhci, diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index cf30cb6..dc337b3 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -269,8 +269,6 @@ static void xhci_free_irq(struct xhci_hcd *xhci) return; if (pdev->irq > 0) free_irq(pdev->irq, xhci_to_hcd(xhci)); - - return; } /* @@ -351,7 +349,6 @@ static void xhci_cleanup_msix(struct xhci_hcd *xhci) } hcd->msix_enabled = 0; - return; } static void __maybe_unused xhci_msix_sync_irqs(struct xhci_hcd *xhci) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697
>From: Changming Huang [mailto:jerry.hu...@nxp.com] >As per USB specification, in the Suspend state, the status bit does not change >until >the port is suspended. However, there may be a delay in suspending a port if >there >is a transaction currently in progress on the bus. > >In the USBDR controller, the PORTSCx[SUSP] bit changes immediately when the >application sets it and not when the port is actually suspended > >Workaround for this issue involves waiting for a minimum of 10ms to allow the >controller to go into SUSPEND state before proceeding ahead > >Signed-off-by: Changming Huang >Signed-off-by: Ramneek Mehresh >--- > drivers/usb/host/ehci-fsl.c |3 +++ > drivers/usb/host/ehci-hub.c |2 ++ > drivers/usb/host/ehci.h |6 ++ > drivers/usb/host/fsl-mph-dr-of.c |2 ++ > include/linux/fsl_devices.h |1 + > 5 files changed, 14 insertions(+) > >diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c index >9f5ffb6..91701cc 100644 >--- a/drivers/usb/host/ehci-fsl.c >+++ b/drivers/usb/host/ehci-fsl.c >@@ -286,6 +286,9 @@ static int ehci_fsl_usb_setup(struct ehci_hcd *ehci) > if (pdata->has_fsl_erratum_a005275 == 1) > ehci->has_fsl_hs_errata = 1; > >+ if (pdata->has_fsl_erratum_a005697 == 1) >+ ehci->has_fsl_susp_errata = 1; >+ > if ((pdata->operating_mode == FSL_USB2_DR_HOST) || > (pdata->operating_mode == FSL_USB2_DR_OTG)) > if (ehci_fsl_setup_phy(hcd, pdata->phy_mode, 0)) diff --git >a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index >74f62d6..86d154e 100644 >--- a/drivers/usb/host/ehci-hub.c >+++ b/drivers/usb/host/ehci-hub.c >@@ -305,6 +305,8 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) > USB_PORT_STAT_HIGH_SPEED) > fs_idle_delay = true; > ehci_writel(ehci, t2, reg); >+ if (ehci_has_fsl_susp_errata(ehci)) >+ mdelay(10); Hi Jerry, Move the delay out of the spin lock. Other than that, it looks fine to me. > changed = 1; > } > } >diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h index >3f3b74a..7706e4a 100644 >--- a/drivers/usb/host/ehci.h >+++ b/drivers/usb/host/ehci.h >@@ -219,6 +219,7 @@ struct ehci_hcd { /* one per controller */ > unsignedno_selective_suspend:1; > unsignedhas_fsl_port_bug:1; /* FreeScale */ > unsignedhas_fsl_hs_errata:1;/* Freescale HS quirk */ >+ unsignedhas_fsl_susp_errata:1; /*Freescale SUSP quirk*/ > unsignedbig_endian_mmio:1; > unsignedbig_endian_desc:1; > unsignedbig_endian_capbase:1; >@@ -703,10 +704,15 @@ struct ehci_tt { > #if defined(CONFIG_PPC_85xx) > /* Some Freescale processors have an erratum (USB A-005275) in which > * incoming packets get corrupted in HS mode >+ * Some Freescale processors have an erratum (USB A-005697) in which >+ * we need to wait for 10ms for bus to fo into suspend mode after >+ * setting SUSP bit > */ > #define ehci_has_fsl_hs_errata(e) ((e)->has_fsl_hs_errata) >+#define ehci_has_fsl_susp_errata(e) ((e)->has_fsl_susp_errata) > #else > #define ehci_has_fsl_hs_errata(e) (0) >+#define ehci_has_fsl_susp_errata(e) (0) > #endif > > /* >diff --git a/drivers/usb/host/fsl-mph-dr-of.c >b/drivers/usb/host/fsl-mph-dr-of.c >index f07ccb2..e90ddb5 100644 >--- a/drivers/usb/host/fsl-mph-dr-of.c >+++ b/drivers/usb/host/fsl-mph-dr-of.c >@@ -226,6 +226,8 @@ static int fsl_usb2_mph_dr_of_probe(struct >platform_device *ofdev) > of_property_read_bool(np, "fsl,usb-erratum-a007792"); > pdata->has_fsl_erratum_a005275 = > of_property_read_bool(np, "fsl,usb-erratum-a005275"); >+ pdata->has_fsl_erratum_a005697 = >+ of_property_read_bool(np, "fsl,usb_erratum-a005697"); > > /* >* Determine whether phy_clk_valid needs to be checked diff --git >a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h index >f291291..60cef82 >100644 >--- a/include/linux/fsl_devices.h >+++ b/include/linux/fsl_devices.h >@@ -100,6 +100,7 @@ struct fsl_usb2_platform_data { > unsignedalready_suspended:1; > unsignedhas_fsl_erratum_a007792:1; > unsignedhas_fsl_erratum_a005275:1; >+ unsignedhas_fsl_erratum_a005697:1; > unsignedcheck_phy_clk_valid:1; > > /* register save area for suspend/resume */ >-- >1.7.9.5 Regards, Sriram -- 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: [RFT PATCH 1/1] xhci: free xhci virtual devices with leaf nodes first
On 24.11.2016 13:03, Felipe Balbi wrote: Hi, Mathias Nyman writes: + /* are any devices using this tt_info? */ + for (i = 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) { off-by-one here ? Why is i starting from 1? + vdev = xhci->devs[i]; slit_id 0 is reserved and xhci->devs[0] is not used, so ne need to check it. hmm... it's reserved for the HW, sure. Do you need to over allocate the array by 1 just to keep this first member unused? Couldn't you handle the +1/-1 (depending on the case) in xhci driver itself? Saves a bit of memory there. There are many things that needs fixing in this area, but not in this patch -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
[PATCHv13 0/3] USB Type-C Connector class
The USB Type-C class is meant to provide unified interface to the userspace to present the USB Type-C ports in a system. Changes since v12: - Added prefer_role member to typec_capability structure as requested by Guenter Changes since v11: - The port drivers are responsible of removing the alternate modes (just like the documentation already said). Changes since v10: - Using ATTRIBUTE_GROUPS and DEVICE_ATTR marcos everywhere - Moved sysfs_match_string to lib/string.c - Rationalized uevents - Calling ida_destroy Changes since v9: - Minor typec_wcove.c cleanup as proposed by Guenter Roeck. No function affect. Changes since v8: - checking sysfs_streq() result correctly in sysfs_strmatch - fixed accessory check in supported_accessory_mode - using "none" as the only string that can clear the preferred role Changes since v7: - Removed "type" attribute from partners - Added supports_usb_power_delivery attribute for partner and cable Changes since v6: - current_vconn_role attr renamed to vconn_source (no API changes) - Small documentation improvements proposed by Vincent Palatin Changes since v5: - Only updating the roles based on driver notifications - Added MODULE_ALIAS for the WhiskeyCove module - Including the patch that creates the actual platform device for the WhiskeyCove Type-C PHY in this series. Changes since v4: - Remove the port lock completely Changes since v3: - Documentation cleanup as proposed by Roger Quadros - Setting partner altmodes member to NULL on removal and fixing a warning, as proposed by Guenter Roeck - Added the following attributes for partners and cables: * supports_usb_power_delivery * id_header_vdo - "id_header_vdo" is visible only when the partner or cable supports USB Power Delivery communication. - Partner attribute "accessory" is hidden when the partner type is not "Accessory". Changes since v2: - Notification on role and alternate mode changes - cleanups Changes since v1: - Completely rewrote alternate mode support - Patners, cables and cable plugs presented as devices. Heikki Krogerus (3): lib/string: add sysfs_match_string helper usb: USB Type-C connector class usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY Documentation/ABI/testing/sysfs-class-typec | 220 ++ Documentation/usb/typec.txt | 110 +++ MAINTAINERS |9 + drivers/usb/Kconfig |2 + drivers/usb/Makefile|2 + drivers/usb/typec/Kconfig | 21 + drivers/usb/typec/Makefile |2 + drivers/usb/typec/typec.c | 1013 +++ drivers/usb/typec/typec_wcove.c | 373 ++ include/linux/string.h | 10 + include/linux/usb/typec.h | 254 +++ lib/string.c| 26 + 12 files changed, 2042 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-typec create mode 100644 Documentation/usb/typec.txt create mode 100644 drivers/usb/typec/Kconfig create mode 100644 drivers/usb/typec/Makefile create mode 100644 drivers/usb/typec/typec.c create mode 100644 drivers/usb/typec/typec_wcove.c create mode 100644 include/linux/usb/typec.h -- 2.10.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv13 2/3] usb: USB Type-C connector class
The purpose of USB Type-C connector class is to provide unified interface for the user space to get the status and basic information about USB Type-C connectors on a system, control over data role swapping, and when the port supports USB Power Delivery, also control over power role swapping and Alternate Modes. Signed-off-by: Heikki Krogerus Reviewed-by: Guenter Roeck Tested-by: Guenter Roeck --- Documentation/ABI/testing/sysfs-class-typec | 220 ++ Documentation/usb/typec.txt | 110 +++ MAINTAINERS |9 + drivers/usb/Kconfig |2 + drivers/usb/Makefile|2 + drivers/usb/typec/Kconfig |7 + drivers/usb/typec/Makefile |1 + drivers/usb/typec/typec.c | 1013 +++ include/linux/usb/typec.h | 254 +++ 9 files changed, 1618 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-typec create mode 100644 Documentation/usb/typec.txt create mode 100644 drivers/usb/typec/Kconfig create mode 100644 drivers/usb/typec/Makefile create mode 100644 drivers/usb/typec/typec.c create mode 100644 include/linux/usb/typec.h diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec new file mode 100644 index 000..853fe2d --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-typec @@ -0,0 +1,220 @@ +USB Type-C port devices (eg. /sys/class/typec/usbc0/) + +What: /sys/class/typec//current_data_role +Date: December 2016 +Contact: Heikki Krogerus +Description: + The current USB data role the port is operating in. This + attribute can be used for requesting data role swapping on the + port. Swapping is supported as synchronous operation, so + write(2) to the attribute will not return until the operation + has finished. The attribute is notified about role changes so + that poll(2) on the attribute wakes up. Change on the role will + also generate uevent KOBJ_CHANGE on the port. + + Valid values: + - host + - device + +What: /sys/class/typec//current_power_role +Date: December 2016 +Contact: Heikki Krogerus +Description: + The current power role of the port. This attribute can be used + to request power role swap on the port when the port supports + USB Power Delivery. Swapping is supported as synchronous + operation, so write(2) to the attribute will not return until + the operation has finished. The attribute is notified about role + changes so that poll(2) on the attribute wakes up. Change on the + role will also generate uevent KOBJ_CHANGE. + + Valid values: + - source + - sink + +What: /sys/class/typec//vconn_source +Date: December 2016 +Contact: Heikki Krogerus +Description: + Shows is the port VCONN Source. This attribute can be used to + request VCONN swap to change the VCONN Source during connection + when both the port and the partner support USB Power Delivery. + Swapping is supported as synchronous operation, so write(2) to + the attribute will not return until the operation has finished. + The attribute is notified about VCONN source changes so that + poll(2) on the attribute wakes up. Change on VCONN source also + generates uevent KOBJ_CHANGE. + + Valid values are: + - 0 when the port is not the VCONN Source + - 1 when the port is the VCONN Source + +What: /sys/class/typec//power_operation_mode +Date: December 2016 +Contact: Heikki Krogerus +Description: + Shows the current power operational mode the port is in. + + Valid values: + - USB - Normal power levels defined in USB specifications + - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C + specification. + - USB Type-C 3.0A - Higher 3A current defined in USB Type-C + specification. +- USB Power Delivery - The voltages and currents defined in USB + Power Delivery specification + +What: /sys/class/typec//preferred_role +Date: December 2016 +Contact: Heikki Krogerus +Description: + The user space can notify the driver about the preferred role. + It should be handled as enabling of Try.SRC or Try.SNK, as + defined in USB Type-C specification, in the port drivers. By + default t
[PATCHv13 1/3] lib/string: add sysfs_match_string helper
Make a simple helper for matching strings with sysfs attribute files. In most parts the same as match_string(), except sysfs_match_string() uses sysfs_streq() instead of strcmp() for matching. This is more convenient when used with sysfs attributes. Signed-off-by: Heikki Krogerus Reviewed-by: Guenter Roeck Tested-by: Guenter Roeck --- include/linux/string.h | 10 ++ lib/string.c | 26 ++ 2 files changed, 36 insertions(+) diff --git a/include/linux/string.h b/include/linux/string.h index 26b6f6a..c4011b2 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -135,6 +135,16 @@ static inline int strtobool(const char *s, bool *res) } int match_string(const char * const *array, size_t n, const char *string); +int __sysfs_match_string(const char * const *array, size_t n, const char *s); + +/** + * sysfs_match_string - matches given string in an array + * @_a: array of strings + * @_s: string to match with + * + * Helper for __sysfs_match_string(). Calculates the size of @a automatically. + */ +#define sysfs_match_string(_a, _s) __sysfs_match_string(_a, ARRAY_SIZE(_a), _s) #ifdef CONFIG_BINARY_PRINTF int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args); diff --git a/lib/string.c b/lib/string.c index ed83562..c7a20cb 100644 --- a/lib/string.c +++ b/lib/string.c @@ -656,6 +656,32 @@ int match_string(const char * const *array, size_t n, const char *string) } EXPORT_SYMBOL(match_string); +/** + * __sysfs_match_string - matches given string in an array + * @array: array of strings + * @n: number of strings in the array or -1 for NULL terminated arrays + * @str: string to match with + * + * Returns index of @str in the @array or -EINVAL, just like match_string(). + * Uses sysfs_streq instead of strcmp for matching. + */ +int __sysfs_match_string(const char * const *array, size_t n, const char *str) +{ + const char *item; + int index; + + for (index = 0; index < n; index++) { + item = array[index]; + if (!item) + break; + if (!sysfs_streq(item, str)) + return index; + } + + return -EINVAL; +} +EXPORT_SYMBOL(__sysfs_match_string); + #ifndef __HAVE_ARCH_MEMSET /** * memset - Fill a region of memory with the given value -- 2.10.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv13 3/3] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY
This adds driver for the USB Type-C PHY on Intel WhiskeyCove PMIC which is available on some of the Intel Broxton SoC based platforms. Signed-off-by: Heikki Krogerus Reviewed-by: Guenter Roeck --- drivers/usb/typec/Kconfig | 14 ++ drivers/usb/typec/Makefile | 1 + drivers/usb/typec/typec_wcove.c | 373 3 files changed, 388 insertions(+) create mode 100644 drivers/usb/typec/typec_wcove.c diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig index 17792f9..2abbcb0 100644 --- a/drivers/usb/typec/Kconfig +++ b/drivers/usb/typec/Kconfig @@ -4,4 +4,18 @@ menu "USB Power Delivery and Type-C drivers" config TYPEC tristate +config TYPEC_WCOVE + tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver" + depends on ACPI + depends on INTEL_SOC_PMIC + depends on INTEL_PMC_IPC + select TYPEC + help + This driver adds support for USB Type-C detection on Intel Broxton + platforms that have Intel Whiskey Cove PMIC. The driver can detect the + role and cable orientation. + + To compile this driver as module, choose M here: the module will be + called typec_wcove + endmenu diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile index 1012a8b..b9cb862 100644 --- a/drivers/usb/typec/Makefile +++ b/drivers/usb/typec/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_TYPEC)+= typec.o +obj-$(CONFIG_TYPEC_WCOVE) += typec_wcove.o diff --git a/drivers/usb/typec/typec_wcove.c b/drivers/usb/typec/typec_wcove.c new file mode 100644 index 000..9065d7b --- /dev/null +++ b/drivers/usb/typec/typec_wcove.c @@ -0,0 +1,373 @@ +/** + * typec_wcove.c - WhiskeyCove PMIC USB Type-C PHY driver + * + * Copyright (C) 2016 Intel Corporation + * Author: Heikki Krogerus + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include + +/* Register offsets */ +#define WCOVE_CHGRIRQ0 0x4e09 +#define WCOVE_PHYCTRL 0x5e07 + +#define USBC_CONTROL1 0x7001 +#define USBC_CONTROL2 0x7002 +#define USBC_CONTROL3 0x7003 +#define USBC_CC1_CTRL 0x7004 +#define USBC_CC2_CTRL 0x7005 +#define USBC_STATUS1 0x7007 +#define USBC_STATUS2 0x7008 +#define USBC_STATUS3 0x7009 +#define USBC_IRQ1 0x7015 +#define USBC_IRQ2 0x7016 +#define USBC_IRQMASK1 0x7017 +#define USBC_IRQMASK2 0x7018 + +/* Register bits */ + +#define USBC_CONTROL1_MODE_DRP(r) (((r) & ~0x7) | 4) + +#define USBC_CONTROL2_UNATT_SNKBIT(0) +#define USBC_CONTROL2_UNATT_SRCBIT(1) +#define USBC_CONTROL2_DIS_ST BIT(2) + +#define USBC_CONTROL3_PD_DIS BIT(1) + +#define USBC_CC_CTRL_VCONN_EN BIT(1) + +#define USBC_STATUS1_DET_ONGOING BIT(6) +#define USBC_STATUS1_RSLT(r) ((r) & 0xf) +#define USBC_RSLT_NOTHING 0 +#define USBC_RSLT_SRC_DEFAULT 1 +#define USBC_RSLT_SRC_1_5A 2 +#define USBC_RSLT_SRC_3_0A 3 +#define USBC_RSLT_SNK 4 +#define USBC_RSLT_DEBUG_ACC5 +#define USBC_RSLT_AUDIO_ACC6 +#define USBC_RSLT_UNDEF15 +#define USBC_STATUS1_ORIENT(r) (((r) >> 4) & 0x3) +#define USBC_ORIENT_NORMAL 1 +#define USBC_ORIENT_REVERSE2 + +#define USBC_STATUS2_VBUS_REQ BIT(5) + +#define USBC_IRQ1_ADCDONE1 BIT(2) +#define USBC_IRQ1_OVERTEMP BIT(1) +#define USBC_IRQ1_SHORTBIT(0) + +#define USBC_IRQ2_CC_CHANGEBIT(7) +#define USBC_IRQ2_RX_PDBIT(6) +#define USBC_IRQ2_RX_HRBIT(5) +#define USBC_IRQ2_RX_CRBIT(4) +#define USBC_IRQ2_TX_SUCCESS BIT(3) +#define USBC_IRQ2_TX_FAIL BIT(2) + +#define USBC_IRQMASK1_ALL (USBC_IRQ1_ADCDONE1 | USBC_IRQ1_OVERTEMP | \ +USBC_IRQ1_SHORT) + +#define USBC_IRQMASK2_ALL (USBC_IRQ2_CC_CHANGE | USBC_IRQ2_RX_PD | \ +USBC_IRQ2_RX_HR | USBC_IRQ2_RX_CR | \ +USBC_IRQ2_TX_SUCCESS | USBC_IRQ2_TX_FAIL) + +struct wcove_typec { + struct mutex lock; /* device lock */ + struct device *dev; + struct regmap *regmap; + struct typec_port *port; + struct typec_capability cap; + struct typec_connection con; + struct typec_partner partner; +}; + +enum wcove_typec_func { + WCOVE_FUNC_DRIVE_VBUS = 1, + WCOVE_FUNC_ORIENTATION, + WCOVE_FUNC_ROLE, + WCOVE_FUNC_DRIVE_VCONN, +}; + +enum wcove_typec_orientation { + WCOVE_ORIENTATION_NORMAL, + WCOVE_ORIENTATION_REVERSE
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-23 02:29 PM, Mark Lord wrote: On 16-11-23 10:12 AM, Hayes Wang wrote: Mark Lord [ml...@pobox.com] [...] What does this code do: static void r8153_set_rx_early_size(struct r8152 *tp) { u32 mtu = tp->netdev->mtu; u32 ocp_data = (agg_buf_sz - mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4; ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data); } This only works for RTL8153. However, what you use is RTL8152. It is like delay completion. It is used to reduce the loading of CPU by letting a transfer contain more data to reduce the number of transfers. How is ocp_data used by the hardware? Shouldn't the calculation also include sizeof(rx_desc) in there somewhere? The algorithm is from our hw engineers, and it should be (agg_buf_sz - packet size) / 8 You could refer to commit a59e6d815226 ("r8152: correct the rx early size"). Thanks. Right now I am working quite hard trying to narrow things down exactly. You are correct that the driver does appear to be careful about accesses beyond the filled portion of a URB buffer -- for some reason I thought the original driver had issues there, but looking again it does not seem to. One idea that is now looking more likely: Things could be suffering from speculative CPU accesses to RAM (the system here has non-coherent d-cache/RAM). This could incorrectly pre-load data from adjacent URB buffers into the d-cache, creating coherency issues. I am testing now with cacheline-sized guard zones between the buffers to see if that is the issue or not. Nope. Guard zones did not fix it, so it's probably not a prefetch issue. Oddly, adding a couple of memory barriers to specific places in the driver does help, A LOT. Still not 100%, but it did pass 1800 reboot tests over night with only three bad rx_desc's reported. That's a new record here for the driver using kmalloc'd buffers, and put reliability on par with using non-cacheable buffers. Any way we look at it though, the chip/driver are simply unreliable, and relying upon hardware checksums (which fail due to the driver looking at garbage rather than the checksum bits) leads to data corruption. Cheers -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] usb: host: plat: Enable xhci plat runtime PM
On 24 November 2016 at 01:39, kbuild test robot wrote: > Hi Baolin, > > [auto build test ERROR on v4.9-rc6] > [also build test ERROR on next-20161123] > [cannot apply to balbi-usb/next usb/usb-testing] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Baolin-Wang/usb-host-plat-Enable-xhci-plat-runtime-PM/20161124-012323 > config: x86_64-randconfig-x008-201647 (attached as .config) > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 > reproduce: > # save the attached .config to linux build tree > make ARCH=x86_64 > > All errors (new ones prefixed by >>): > >drivers/usb/host/xhci-plat.c: In function 'xhci_plat_remove': >>> drivers/usb/host/xhci-plat.c:281:28: error: 'pdev' undeclared (first use in >>> this function) > pm_runtime_set_suspended(&pdev->dev); Oops, will fix my mistake in new patch. Please ignore this patch. Sorry. -- Baolin.wang 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: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
Mark Lord [mailto:ml...@pobox.com] > Sent: Wednesday, November 23, 2016 9:41 PM [...] > >static void r8153_set_rx_early_size(struct r8152 *tp) > >{ > >u32 mtu = tp->netdev->mtu; > >u32 ocp_data = (agg_buf_sz - mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4; > > > >ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data); > >} > > How is ocp_data used by the hardware? > Shouldn't the calculation also include sizeof(rx_desc) in there somewhere? I check your question with our hw engineers, and you are right. The size of rx descriptor should be calculated, too. Best Regards, Hayes N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
Re: [PATCH 3/3] xhci: Remove busy loop from xhci_abort_cmd_ring()
On 16.11.2016 07:01, OGAWA Hirofumi wrote: Now, xhci_abort_cmd_ring() is sleepable. So no reason to use busy loop anymore. - Convert udelay(1000) => msleep(1). Sounds good. - Add xhci_handshake_sleep(), and use it. This seems a but overkill, I'd rather don't have xhci_handshake(), xhci_handshake_sleep() and __xhci_handshake() to maintain. As we now can sleep in xhci_abort_cmd_ring() I would rather just first wait for the completion and then maybe handshake check the register. At that point it shouldn't need to busyloop anymore, one read should do it. But this is all just optimizing an error path, I'm actually fine with taking just patch 1/3 and 2/3 v3, and skipping 3/3. the udelay(1000) to msleep(1) I can add myself As related change, current xhci_handshake() is strange behavior, E.g. xhci_handshake(ptr, mask, done, 1) does result = readl(ptr); /* check result */ udelay(1); <= meaningless delay return -ETIMEDOUT; Only in the timeout case, so if we after 3 or 5 million register reads + udelays(1) still don't get the value we want then there is an unnecessary udelay(1). Not worth putting much effort on it now. Sorry about the review delay, I got my hands quite full at the moment -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
Mark Lord [mailto:ml...@pobox.com] > Sent: Thursday, November 24, 2016 8:31 PM [...] > Nope. Guard zones did not fix it, so it's probably not a prefetch issue. > Oddly, adding a couple of memory barriers to specific places in the driver > does help, A LOT. Still not 100%, but it did pass 1800 reboot tests over > night > with only three bad rx_desc's reported. > > That's a new record here for the driver using kmalloc'd buffers, > and put reliability on par with using non-cacheable buffers. > > Any way we look at it though, the chip/driver are simply unreliable, > and relying upon hardware checksums (which fail due to the driver > looking at garbage rather than the checksum bits) leads to data corruption. I don't think the garbage results from our driver or device. If it is the issue about memory, I think the host driver ought to deal with it, because it handles the DMA. Besides, it doesn't seem to occur for all platforms. I have tested the iperf more than 26 hours, and it still works fine. I think I would get the same result on x86 or x86_64 platform. Best Regards, Hayes -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] usb: host: xhci: two small fixes
Hi Mathias, these two patches help with two minor details in XHCI. Patch 1 reduces the amount of memory used by our virt_device array by discovering from the HW how many slots we can support. Patch 2 makes COMP_STOP work when a Stop Endpoint command is issued while we're still in SETUP stage. Note how I have also converted on_data_stage and all the pointer mathing to more robust TRB type matching. Tested (albeit lightly with USB storage and keyboard + rootfs on another USB storage) with a SKL box. Patches on top of today's next/master Felipe Balbi (2): usb: host: xhci: dynamically allocate devs array usb: host: xhci: handle COMP_STOP from SETUP phase too drivers/usb/host/xhci-hub.c | 2 +- drivers/usb/host/xhci-mem.c | 4 ++-- drivers/usb/host/xhci-ring.c | 32 +++- drivers/usb/host/xhci.c | 19 +++ drivers/usb/host/xhci.h | 2 +- 5 files changed, 38 insertions(+), 21 deletions(-) -- 2.10.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 1/2] usb: host: xhci: dynamically allocate devs array
Instead of always defaulting to a 256-entry array, we can dynamically allocate devs based on what HW tells us it supports. Note that we can't, yet, purge MAX_HC_SLOTS completely because of struct xhci_device_context_array reliance on it. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-hub.c | 2 +- drivers/usb/host/xhci-mem.c | 4 ++-- drivers/usb/host/xhci-ring.c | 2 +- drivers/usb/host/xhci.c | 19 +++ drivers/usb/host/xhci.h | 2 +- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 0ef16900efed..7b1b58ad6aac 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -356,7 +356,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci, enum usb_device_speed speed; slot_id = 0; - for (i = 0; i < MAX_HC_SLOTS; i++) { + for (i = 0; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) { if (!xhci->devs[i]) continue; speed = xhci->devs[i]->udev->speed; diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 321de2e0161b..05c8132f77c8 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1828,7 +1828,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) } } - for (i = 1; i < MAX_HC_SLOTS; ++i) + for (i = 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); ++i) xhci_free_virt_device(xhci, i); dma_pool_destroy(xhci->segment_pool); @@ -2535,7 +2535,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) * something other than the default (~1ms minimum between interrupts). * See section 5.5.1.2. */ - for (i = 0; i < MAX_HC_SLOTS; ++i) + for (i = 0; i < HCS_MAX_SLOTS(xhci->hcs_params1); ++i) xhci->devs[i] = NULL; for (i = 0; i < USB_MAXCHILDREN; ++i) { xhci->bus_state[0].resume_done[i] = 0; diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index bdf6b13d9b67..44cade15792b 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -895,7 +895,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) * doesn't touch the memory. */ } - for (i = 0; i < MAX_HC_SLOTS; i++) { + for (i = 0; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) { if (!xhci->devs[i]) continue; for (j = 0; j < 31; j++) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 1cd56417cbec..c8bbff4e57db 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -766,6 +766,8 @@ void xhci_shutdown(struct usb_hcd *hcd) /* Yet another workaround for spurious wakeups at shutdown with HSW */ if (xhci->quirks & XHCI_SPURIOUS_WAKEUP) pci_set_power_state(to_pci_dev(hcd->self.controller), PCI_D3hot); + + kfree(xhci->devs); } #ifdef CONFIG_PM @@ -4896,6 +4898,11 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) xhci->quirks |= quirks; + xhci->devs = kcalloc(HCS_MAX_SLOTS(xhci->hcs_params1), + sizeof(struct xhci_virt_device *), GFP_KERNEL); + if (!xhci->devs) + return -ENOMEM; + get_quirks(dev, xhci); /* In xhci controllers which follow xhci 1.0 spec gives a spurious @@ -4908,13 +4915,13 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) /* Make sure the HC is halted. */ retval = xhci_halt(xhci); if (retval) - return retval; + goto err; xhci_dbg(xhci, "Resetting HCD\n"); /* Reset the internal HC memory state and registers. */ retval = xhci_reset(xhci); if (retval) - return retval; + goto err; xhci_dbg(xhci, "Reset complete\n"); /* @@ -4940,7 +4947,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) */ retval = dma_set_mask(dev, DMA_BIT_MASK(32)); if (retval) - return retval; + goto err; xhci_dbg(xhci, "Enabling 32-bit DMA addresses.\n"); dma_set_coherent_mask(dev, DMA_BIT_MASK(32)); } @@ -4949,13 +4956,17 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) /* Initialize HCD and host controller data structures. */ retval = xhci_init(hcd); if (retval) - return retval; + goto err; xhci_dbg(xhci, "Called HCD init\n"); xhci_info(xhci, "hcc params 0x%08x hci version 0x%x quirks 0x%08x\n", xhci->hcc_params, xhci->hci_version, xhci->quirks); return 0; + +err: + kfree(xhci->devs); + return retval; } EXPORT_S
[PATCH 2/2] usb: host: xhci: handle COMP_STOP from SETUP phase too
Stop Endpoint command can come at any point and we have no control of that. We should make sure to handle COMP_STOP on SETUP phase as well, otherwise urb->actual_lenght might be set to negative values in some occasions such as below: urb->length = 4; build_control_transfer_td_for(urb, ep); stop_endpoint(ep); COMP_STOP: [...] urb->actual_length = urb->length - trb->length; trb->length is 8 for SETUP stage (8 control request bytes), so actual_length would be set to -4 in this case. While doing that, also make sure to use TRB_TYPE field of the actual TRB instead of matching pointers to figure out in which stage of the control transfer we got our completion event. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 30 ++ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 44cade15792b..40be843a1b2a 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1939,8 +1939,9 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_ep_ctx *ep_ctx; u32 trb_comp_code; u32 remaining, requested; - bool on_data_stage; + u32 trb_type; + trb_type = TRB_FIELD_TO_TYPE(le32_to_cpu(ep_trb->generic.field[3])); slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags)); xdev = xhci->devs[slot_id]; ep_index = TRB_TO_EP_ID(le32_to_cpu(event->flags)) - 1; @@ -1950,14 +1951,11 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, requested = td->urb->transfer_buffer_length; remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); - /* not setup (dequeue), or status stage means we are at data stage */ - on_data_stage = (ep_trb != ep_ring->dequeue && ep_trb != td->last_trb); - switch (trb_comp_code) { case COMP_SUCCESS: - if (ep_trb != td->last_trb) { + if (trb_type != TRB_STATUS) { xhci_warn(xhci, "WARN: Success on ctrl %s TRB without IOC set?\n", - on_data_stage ? "data" : "setup"); + (trb_type == TRB_DATA) ? "data" : "setup"); *status = -ESHUTDOWN; break; } @@ -1967,15 +1965,23 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, *status = 0; break; case COMP_STOP_SHORT: - if (on_data_stage) + if (trb_type == TRB_DATA) td->urb->actual_length = remaining; else xhci_warn(xhci, "WARN: Stopped Short Packet on ctrl setup or status TRB\n"); goto finish_td; case COMP_STOP: - if (on_data_stage) + switch (trb_type) { + case TRB_SETUP: + td->urb->actual_length = 0; + goto finish_td; + case TRB_DATA: td->urb->actual_length = requested - remaining; - goto finish_td; + goto finish_td; + default: + xhci_warn(xhci, "WARN: unexpected TRB Type %d\n", trb_type); + goto finish_td; + } case COMP_STOP_INVAL: goto finish_td; default: @@ -1987,7 +1993,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, /* else fall through */ case COMP_STALL: /* Did we transfer part of the data (middle) phase? */ - if (on_data_stage) + if (trb_type == TRB_DATA) td->urb->actual_length = requested - remaining; else if (!td->urb_length_set) td->urb->actual_length = 0; @@ -1995,14 +2001,14 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, } /* stopped at setup stage, no data transferred */ - if (ep_trb == ep_ring->dequeue) + if (trb_type == TRB_SETUP) goto finish_td; /* * if on data stage then update the actual_length of the URB and flag it * as set, so it won't be overwritten in the event for the last TRB. */ - if (on_data_stage) { + if (trb_type == TRB_DATA) { td->urb_length_set = true; td->urb->actual_length = requested - remaining; xhci_dbg(xhci, "Waiting for status stage event\n"); -- 2.10.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 3/3] xhci: Remove busy loop from xhci_abort_cmd_ring()
Mathias Nyman writes: >> - Add xhci_handshake_sleep(), and use it. > > This seems a but overkill, I'd rather don't have xhci_handshake(), > xhci_handshake_sleep() and __xhci_handshake() to maintain. I agree about it. However, on other hand, I thought about the possibility/effort to decreasing use-cases of xhci_handshake() busyloop. (there are several places to use more than e.g. 500ms timeout.) Because long busyloop (especially with interrupt-off) has really bad effect to whole system of non-usb. > As we now can sleep in xhci_abort_cmd_ring() I would rather just first > wait for the completion and then maybe handshake check the register. > At that point it shouldn't need to busyloop anymore, one read should > do it. I worried about in the case of real internal confusing, and consider about chip that doesn't have no stop/abort event. To handle both case, timeout choice would not be straight-forward. > But this is all just optimizing an error path, I'm actually fine with > taking just patch 1/3 and 2/3 v3, and skipping 3/3. the udelay(1000) > to msleep(1) I can add myself Right. The main point of patchset is 1/3 and 2/3. >> As related change, current xhci_handshake() is strange behavior, >> E.g. xhci_handshake(ptr, mask, done, 1) does >> >> result = readl(ptr); >> /* check result */ >> udelay(1); <= meaningless delay >> return -ETIMEDOUT; > > Only in the timeout case, so if we after 3 or 5 million register reads > + udelays(1) still don't get the value we want then there is an > unnecessary udelay(1). > > Not worth putting much effort on it now. True. But actual effort for it was very small. @@ -65,16 +65,18 @@ int xhci_handshake(void __iomem *ptr, u3 { u32 result; - do { + while (1) { result = readl(ptr); if (result == ~(u32)0) /* card removed */ return -ENODEV; result &= mask; if (result == done) return 0; + if (usec <= 0) + break; udelay(1); usec--; - } while (usec > 0); + } return -ETIMEDOUT; } Thanks. -- OGAWA Hirofumi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL] USB driver fixes for 4.9-rc7
The following changes since commit a25f0944ba9b1d8a6813fd6f1a86f1bd59ac25a6: Linux 4.9-rc5 (2016-11-13 10:32:32 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-4.9-rc7 for you to fetch changes up to c0da038d7afed2892346fdb9601e4fefee13a800: Merge tag 'usb-serial-4.9-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial into usb-linus (2016-11-18 15:49:31 +0100) USB fixes for 4.9-rc7 Here are a few small USB fixes and new device ids for 4.9-rc7. The majority of these fixes are in the musb driver, fixing a number of regressions that have been reported but took a while to resolve. The other fixes are all small ones, to resolve other reported minor issues. All have been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman Doug Brown (1): USB: serial: ftdi_sio: add support for TI CC3200 LaunchPad Felix Hädicke (1): usb: gadget: f_fs: fix wrong parenthesis in ffs_func_req_match() Greg Kroah-Hartman (2): Merge tag 'fixes-for-v4.9-rc6' of git://git.kernel.org/.../balbi/usb into usb-linus Merge tag 'usb-serial-4.9-rc6' of git://git.kernel.org/.../johan/usb-serial into usb-linus Paul Jakma (1): USB: serial: cp210x: add ID for the Zone DPMX Peter Chen (1): usb: chipidea: move the lock initialization to core file Petr Vandrovec (1): Fix USB CB/CBI storage devices with CONFIG_VMAP_STACK=y Tony Lindgren (6): usb: musb: Fix broken use of static variable for multiple instances usb: musb: Fix sleeping function called from invalid context for hdrc glue usb: musb: Fix PM for hub disconnect usb: musb: Add missing pm_runtime_disable and drop 2430 PM timeout usb: musb: Drop pointless PM runtime code for dsps glue phy: twl4030-usb: Fix for musb session bit based PM drivers/phy/phy-twl4030-usb.c | 4 +- drivers/usb/chipidea/core.c| 1 + drivers/usb/chipidea/udc.c | 2 - drivers/usb/gadget/function/f_fs.c | 8 +- drivers/usb/musb/musb_core.c | 147 - drivers/usb/musb/musb_core.h | 13 +++- drivers/usb/musb/musb_dsps.c | 58 +++ drivers/usb/musb/musb_gadget.c | 39 -- drivers/usb/musb/omap2430.c| 10 +-- drivers/usb/musb/tusb6010.c| 6 +- drivers/usb/serial/cp210x.c| 1 + drivers/usb/serial/ftdi_sio.c | 2 + drivers/usb/serial/ftdi_sio_ids.h | 6 ++ drivers/usb/storage/transport.c| 7 +- 14 files changed, 229 insertions(+), 75 deletions(-) -- 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 V12 1/1] usb:serial: Add Fintek F81532/534 driver
On Mon, Nov 14, 2016 at 01:37:59PM +0800, Ji-Ze Hong (Peter Hong) wrote: > This driver is for Fintek F81532/F81534 USB to Serial Ports IC. > > F81532 spec: > https://drive.google.com/file/d/0B8vRwwYO7aMFOTRRMmhWQVNvajQ/view?usp= > sharing > > F81534 spec: > https://drive.google.com/file/d/0B8vRwwYO7aMFV29pQWJqbVBNc00/view?usp= > sharing > > Features: > 1. F81532 is 1-to-2 & F81534 is 1-to-4 serial ports IC > 2. Support Baudrate from B50 to B115200. > > Signed-off-by: Ji-Ze Hong (Peter Hong) > --- > Changelog: > V12 > 1. Max TX change from 100 to 124 bytes. > 2. Add probe() to verify endpoints & packet size. > 3. Rename function names. > set/get_normal_register() -> set/get_register() > set/get_register() -> set/get_port_register() > set/get_register_delay() -> set/get_spi_register() > read_data() -> read_flash() > command_delay() -> wait_for_spi() > +static int f81534_probe(struct usb_serial *serial, > + const struct usb_device_id *id) > +{ > + struct usb_endpoint_descriptor *endpoint; > + struct usb_host_interface *iface_desc; > + struct device *dev; > + int num_bulk_in = 0; > + int num_bulk_out = 0; > + int size_bulk_in = 0; > + int size_bulk_out = 0; > + int i; > + > + dev = &serial->interface->dev; > + iface_desc = serial->interface->cur_altsetting; > + > + for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { > + endpoint = &iface_desc->endpoint[i].desc; > + > + if (usb_endpoint_is_bulk_in(endpoint)) { > + ++num_bulk_in; > + size_bulk_in = usb_endpoint_maxp(endpoint); > + } > + > + if (usb_endpoint_is_bulk_out(endpoint)) { > + ++num_bulk_out; > + size_bulk_out = usb_endpoint_maxp(endpoint); > + } > + } > + > + if (num_bulk_in != 1 || num_bulk_out != 1) { > + dev_err(dev, "%s: endpoints not matched\n", __func__); Better to spell out: "expected endpoints not found\n". > + return -EINVAL; And this should be -ENODEV; > + } > + > + if (size_bulk_out != F81534_WRITE_BUFFER_SIZE || > + size_bulk_in != F81534_MAX_RECEIVE_BLOCK_SIZE) { > + dev_err(dev, "%s: endpoints packet size not matched\n", > + __func__); Similarly: "unsupported endpoint max packet size\n". But just to be clear: You do want to bail out if connected at full speed? You could also ask usb-serial core to allocate large enough buffers (e.g. by setting the bulk_out_size driver field) and the host controller will handle partitioning. > + return -EINVAL; And -ENODEV. > + } > + > + return 0; > +} Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-24 08:26 AM, Hayes Wang wrote: .. Besides, it doesn't seem to occur for all platforms. I have tested the iperf more than 26 hours, and it still works fine. I think I would get the same result on x86 or x86_64 platform. .. x86 has near fully-coherent memory, so it is the "easy" platform to get things working on. But Linux supports a very diverse number of platforms, with varying degrees of cache/memory coherency, and it can be tricky for things to work correctly on all of them. If you are testing with the driver as currently in 4.4.34, then you won't even notice when things are screwing up, because the driver just silently drops packets. Or it passes them on without noticing that they have bad data. Here (attached) is the instrumented driver I am using here now. I suggest you use it or something similar when testing, and not the stock driver. This one has also been converted to use non-cacheable RAM for the receive buffers -- something that is probably a Good Thing for it to do regardless of this investigation. It also never drops a packet without logging the event, so we can see just how often there's an issue. This version behaves almost perfectly here, but I am still experimenting to see what is actually necessary, and what is not. In particular, there are some mb() calls I had put in there that shouldn't be required, so I have yet to try removing them again and see what changes. It takes at least an overnight run to pop up one or two errors, so do expect to hear back again until after the weekend at this point. Also, unrelated, but inside r8152_submit_rx() there is this code: /* The rx would be stopped, so skip submitting */ if (test_bit(RTL8152_UNPLUG, &tp->flags) || !test_bit(WORK_ENABLE, &tp->flags) || !netif_carrier_ok(tp->netdev)) return 0; If that "return 0" statement is ever executed, doesn't it result in the loss/leak of a buffer? Thanks /* * Copyright (c) 2014 Realtek Semiconductor Corp. All rights reserved. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License * version 2 as published by the Free Software Foundation. * */ #include #include #include #include #include #include #include #include #include #include #include #include #include #include #include #include #include #include /* Information for net-next */ #define NETNEXT_VERSION "08" /* Information for net */ #define NET_VERSION "2" #define DRIVER_VERSION "v1." NETNEXT_VERSION "." NET_VERSION #define DRIVER_AUTHOR "Realtek linux nic maintainers " #define DRIVER_DESC "Realtek RTL8152/RTL8153 Based USB Ethernet Adapters" #define MODULENAME "r8152" #define R8152_PHY_ID 32 #define PLA_IDR 0xc000 #define PLA_RCR 0xc010 #define PLA_RMS 0xc016 #define PLA_RXFIFO_CTRL0 0xc0a0 #define PLA_RXFIFO_CTRL1 0xc0a4 #define PLA_RXFIFO_CTRL2 0xc0a8 #define PLA_DMY_REG0 0xc0b0 #define PLA_FMC 0xc0b4 #define PLA_CFG_WOL 0xc0b6 #define PLA_TEREDO_CFG 0xc0bc #define PLA_MAR 0xcd00 #define PLA_BACKUP 0xd000 #define PAL_BDC_CR 0xd1a0 #define PLA_TEREDO_TIMER 0xd2cc #define PLA_REALWOW_TIMER 0xd2e8 #define PLA_LEDSEL 0xdd90 #define PLA_LED_FEATURE 0xdd92 #define PLA_PHYAR 0xde00 #define PLA_BOOT_CTRL 0xe004 #define PLA_GPHY_INTR_IMR 0xe022 #define PLA_EEE_CR 0xe040 #define PLA_EEEP_CR 0xe080 #define PLA_MAC_PWR_CTRL 0xe0c0 #define PLA_MAC_PWR_CTRL2 0xe0ca #define PLA_MAC_PWR_CTRL3 0xe0cc #define PLA_MAC_PWR_CTRL4 0xe0ce #define PLA_WDT6_CTRL 0xe428 #define PLA_TCR0 0xe610 #define PLA_TCR1 0xe612 #define PLA_MTPS 0xe615 #define PLA_TXFIFO_CTRL 0xe618 #define PLA_RSTTALLY 0xe800 #define PLA_CR 0xe813 #define PLA_CRWECR 0xe81c #define PLA_CONFIG12 0xe81e /* CONFIG1, CONFIG2 */ #define PLA_CONFIG34 0xe820 /* CONFIG3, CONFIG4 */ #define PLA_CONFIG5 0xe822 #define PLA_PHY_PWR 0xe84c #define PLA_OOB_CTRL 0xe84f #define PLA_CPCR 0xe854 #define PLA_MISC_0 0xe858 #define PLA_MISC_1 0xe85a #define PLA_OCP_GPHY_BASE 0xe86c #define PLA_TALLYCNT 0xe890 #define PLA_SFF_STS_7 0xe8de #define PLA_PHYSTATUS 0xe908 #define PLA_BP_BA 0xfc26 #define PLA_BP_0 0xfc28 #define PLA_BP_1 0xfc2a #define PLA_BP_2 0xfc2c #define PLA_BP_3 0xfc2e #define PLA_BP_4 0xfc30 #define PLA_BP_5 0xfc32 #define PLA_BP_6 0xfc34 #define PLA_BP_7 0xfc36 #define PLA_BP_EN 0xfc38 #define USB_USB2PHY 0xb41e #define USB_SSPHYLINK2 0xb428 #define USB_U2P3_CTRL 0xb460 #define USB_CSR_DUMMY1 0xb464 #define USB_CSR_DUMMY2 0xb466 #define USB_DEV_STAT 0xb808 #define USB_CONNECT_TIMER 0xcbf8 #define USB_BURST_SIZE 0xcfc0 #define USB_USB_CTRL 0xd406 #define USB_PHY_CTRL 0xd408 #define USB_TX_AGG 0xd40a #define USB_RX_BUF_TH 0xd40c #define USB_USB_TIMER 0xd428 #define USB_RX_EARLY_TIMEOUT 0xd42c #define USB_RX_EARLY_SIZE 0xd42e #define USB_PM_CTRL_STATUS 0xd432 #define USB_TX_DMA 0xd434 #define USB_TOLERANCE 0xd490 #define USB_LPM_CTRL 0xd41a #define USB_UPS_CTRL 0xd800 #defin
Re: [PATCH 1/2] usb: host: xhci: dynamically allocate devs array
On 24.11.2016 15:33, Felipe Balbi wrote: Instead of always defaulting to a 256-entry array, we can dynamically allocate devs based on what HW tells us it supports. Note that we can't, yet, purge MAX_HC_SLOTS completely because of struct xhci_device_context_array reliance on it. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-hub.c | 2 +- drivers/usb/host/xhci-mem.c | 4 ++-- drivers/usb/host/xhci-ring.c | 2 +- drivers/usb/host/xhci.c | 19 +++ drivers/usb/host/xhci.h | 2 +- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 0ef16900efed..7b1b58ad6aac 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -356,7 +356,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci, enum usb_device_speed speed; slot_id = 0; - for (i = 0; i < MAX_HC_SLOTS; i++) { + for (i = 0; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) { Normally that is what it should look like, but as we in xhci driver don't use xhci->devs[0], and xhci->devs[HCS_MAX_SLOTS(xhci->hcs_params1)] can be a valid virt_device it needs a +1. Same goes for everywhere else (also when allocating) This is probably originally due to that xhci hw returns a slot_id 0 for failure, and 1 to, including HCD_MAX_SLOTS[hcs_params1) for successful enable device command. the virt_dev is then straight allocated to xhci->devs[slot_id] = kzalloc(..) -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: host: xhci: handle COMP_STOP from SETUP phase too
On 24.11.2016 15:33, Felipe Balbi wrote: Stop Endpoint command can come at any point and we have no control of that. We should make sure to handle COMP_STOP on SETUP phase as well, otherwise urb->actual_lenght might be set to negative values in some occasions such as below: urb->length = 4; build_control_transfer_td_for(urb, ep); stop_endpoint(ep); COMP_STOP: [...] urb->actual_length = urb->length - trb->length; trb->length is 8 for SETUP stage (8 control request bytes), so actual_length would be set to -4 in this case. While doing that, also make sure to use TRB_TYPE field of the actual TRB instead of matching pointers to figure out in which stage of the control transfer we got our completion event. Signed-off-by: Felipe Balbi --- This is awsome. This probably fixes tons of issues related to failing getting port status and other control transfers. Looking at it now it turns out that in COMP_STOP case we can't rely on endpoint ring dequeue pointer to be correct, and still we used it to determine the control transfer stage. (setup, data, status). No wonder there has been issues related to control transfers. Using the TRB_TYPE of the actual TRB on the endpoint ring is definitely the way to go Thanks -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Hayes Wang Date: Thu, 24 Nov 2016 13:26:55 + > I don't think the garbage results from our driver or device. This is my impression with what has been presented so far as well. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord Date: Thu, 24 Nov 2016 07:31:17 -0500 > Any way we look at it though, the chip/driver are simply unreliable, > and relying upon hardware checksums (which fail due to the driver > looking at garbage rather than the checksum bits) leads to data > corruption. If the cpu/DMA implementation is the problem, then turning off checksums is not an appropriate fix at all. In fact, we have no idea what the cause is yet. That makes turning off random features no more than grasping at straws and makes no sense at all upstream. It may make sense for you to do such a change locally in _your_ tree to fix your situation temporarily. But upstream we shouldn't be doing it. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-24 11:21 AM, David Miller wrote: From: Hayes Wang Date: Thu, 24 Nov 2016 13:26:55 + I don't think the garbage results from our driver or device. This is my impression with what has been presented so far as well. It's not garbage. The latest run with the debug code I posted here earlier just spat out this below. Using coherent (guarded, non-cacheable) RX buffers, with mb() calls: [ 15.199157] r8152_check_rx_desc: rx_desc looks bad. [ 15.204270] r8152_rx_bottom: offset=0/3376 bad rx_desc [ 15.209584] r8152_dump_rx_desc: 3d435253 3034336d 202f3a30 47524154 2f3d5445 3034336d rx_len=21075 The bad data in this case is ASCII: "SRC=m3400:/ TARGET=/m340" This data is what is seen in /run/mount/utab, a file that is read/written over NFS on each boot. "SRC=m3400:/ TARGET=/m3400 ROOT=/ ATTRS=nolock,addr=192.168.8.1\n" But how does this ASCII data end up at offset zero of the rx buffer?? Not possible -- this isn't even stale data, because only an rx_desc could be at that offset in that buffer. So even if this were a platform memory coherency issue, one should still never see ASCII data at the beginning of an rx buffer. The driver NEVER writes anything to the rx buffers. Only the USB hardware ever does. And only the r8152 dongle/driver exhibits this issue. Other USB dongles do not. They *might* still have such issues, but because they use software checksums, the bad packets are caught/rejected. The r8152 driver, without the debug/error-checking additions, would have tried to interpret that ASCII data as an "rx_desc", and would have interpreted the "checksum bits" therein as "valid checksum", and the packet would have passed through the network stack, corrupting data. This driver worked without noticeable issues in 3.12.xx. It hasn't worked since. Because it now trusts the hardware checksums, without first checking to see if noise-on-the-line or something else has corrupted the data before receipt in the rx buffer. Based on the above capture, I suspect a bug in the chip itself, which perhaps is only manifest on a very slow CPU. Nobody here tests with slow CPUs, but they are very prevalent in embedded space. And very few people use USB network dongles nowadays either, as nearly all "computers" have built-in networking. The market for USB network dongles is mostly embedded space. Ergo. Cheers -- 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] fsl/usb: Workarourd for USB erratum-A005697
On Thu, 24 Nov 2016, Sriram Dash wrote: > >From: Changming Huang [mailto:jerry.hu...@nxp.com] > >As per USB specification, in the Suspend state, the status bit does not > >change until > >the port is suspended. However, there may be a delay in suspending a port if > >there > >is a transaction currently in progress on the bus. > > > >In the USBDR controller, the PORTSCx[SUSP] bit changes immediately when the > >application sets it and not when the port is actually suspended > > > >Workaround for this issue involves waiting for a minimum of 10ms to allow the > >controller to go into SUSPEND state before proceeding ahead The USB core guarantees that there won't be any data transactions in progress when a root hub is suspended. There might possibly be an SOF transaction, but that doesn't take more than a few microseconds at most. Certainly nowhere near 10 ms! Given that we already perform a 150-us delay, is this change really needed? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-24 11:43 AM, Mark Lord wrote: .. But how does this ASCII data end up at offset zero of the rx buffer?? Not possible -- this isn't even stale data, because only an rx_desc could be at that offset in that buffer. Answering my own question here, I suspect it ends up there as a result of overrunning the previous URB. So I have updated the test copy of the driver here now to check for that exact situation. It's running now, but could take hours or a day for the bug to occur again. It seems I am being overly helpful here. Perhaps I should have just stopped with the original regression report (driver works in 3.12.xx, fails on all newer kernels, as a result of enabling hardware checksums). Had I left it there, one might reasonably expect the onus to be on the driver developer to sort it out, with me providing retests of supplied patches as need be. But I've gone WAY BEYOND that, even questioning the sanity of the platform on which it is being used, just to avoid blaming a buggy USB dongle for some other issue. And this is leading people to suspect that I really think the platform is buggy. It isn't. It's been running for years, with a variety of USB hardware attached, and nary a problem. Except with this r8152 dongle on kernels > 3.12. So, yeah, the driver is fixed in our local tree, and has been for some time now. I just was hoping that perhaps others might be interested in it too, since the bug (whatever it is) corrupts data on the NFS server. Cheers -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord Date: Thu, 24 Nov 2016 11:43:53 -0500 > So even if this were a platform memory coherency issue, one should > still never see ASCII data at the beginning of an rx buffer. I'm not so convinced, since this is the kind of random corruption one would expect to see when dealing with virtual caches that have aliasing or similar issues. Writes to address X that show up at address Y or not at all are precisely the signature of virtual cache aliasing problems. Is it a case of the chip writing to X but the cpu is still seeing stale data from a previous CPU store? For NFS the cpu is writing into the page cache, so we know that cpu side stores are where the ASCII text is coming from. Now is the r8152 buffer one that the USB host controller is DMA'ing into directly, or is it one that SWIOMMU or similar bounce buffering is copying into? In the latter case we are doing cpu stores into the area and the writes aren't coming from the device. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord Date: Thu, 24 Nov 2016 12:00:15 -0500 > It seems I am being overly helpful here. Either you want to cry or you want to keep helping us track down this problem. It is your choice, and your choice alone. Please do not pretend otherwise, everyone else in this thread is operating with the best intentions and wants to see this through to a full analysis and a proper solution for the corruptions. Thank you. -- 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
Amlogic Meson GXL/GXM USB support (dwc2 and dwc3)
Hello, currently I am trying to get the USB controllers on the Amlogic Meson GXL SoCs working: there is one dwc2 and dwc3 controller each. The SoC itself provides 3x USB2 PHYs and 1x USB3 PHY. I wrote drivers for both of them based on the vendor kernel, see [0] The PHY registers of the USB2 PHYs seem to be identical, regardless of whether they are connected to dwc2 or dwc3. The USB3 PHY also takes care of the OTG interrupts (to switch between host and device) and seems to inform the USB2 PHY about mode-changes. USB3 seems to be disabled in the dwc3 configuration, meaning it provides only high-speed support. The dwc3 core fails to initialize currently due to some DMA issues which will be fixed in Linux v4.10 - the corresponding patchset can be found here: [1] With these patches applied we get the dwc3 controller to initialize: xhci-hcd xhci-hcd.0.auto: xHCI Host Controller xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus number 1 xhci-hcd xhci-hcd.0.auto: hcc params 0x0228f664 hci version 0x100 quirks 0x00010010 xhci-hcd xhci-hcd.0.auto: irq 20, io mem 0xc900 hub 1-0:1.0: USB hub found hub 1-0:1.0: 3 ports detected xhci-hcd xhci-hcd.0.auto: xHCI Host Controller xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus number 2 usb usb2: We don't know the algorithms for LPM for this host, disabling LPM. hub 2-0:1.0: USB hub found hub 2-0:1.0: config failed, hub doesn't have any ports! (err -19) (the last message seems fine, there are probably no USB3 ports enabled in the dwc3 hardware configuration) strange fact #1: there are 3 USB2 PHYs enabled in the dwc3 core (regdump from the vendor kernel - a full version is attached): GUSB2PHYCFG(0) = 0x40102500 GUSB2PHYCFG(1) = 0x40102540 GUSB2PHYCFG(2) = 0x40102540 (this explains the "hub 1-0:1.0: 3 ports detected" message on my GXM board - other SoCs seem to have a different number of ports available based on the vendor sources, GXL seem to have 2 ports, while "TXL" seems to have 4 ports). I tried enabling all available PHYs in the SoC and giving GUSB2PHYCFG(1 and 2) the same tickle that is currently done in dwc3_phy_setup() for GUSB2PHYCFG(0). The LED on my thumb drive flashes when I plug it into the dwc3 port, but I don't get any interrupts and the kernel does not recognize any new USB device. For the dwc2 controller I am probably missing a clock somewhere, because it's reporting: dwc2 c910.usb: Configuration mismatch. dr_mode forced to device dwc2 c910.usb: dwc2_core_reset() HANG! Soft Reset GRSTCTL=8001 dwc2 c910.usb: Specified GNPTXFDEP=1024 > 768 dwc2 c910.usb: EPs: 7, dedicated fifos, 712 entries in SPRAM I must admit that I have been focusing on the dwc3 controller so far, so I don't have any more information here. if any of these problems sound familiar to you or if you have any advice on what I could experiment with: please don't hesitate and let me know! I'll also keep continue looking into this, because it's not unlikely that I'm running into a platform specific issue. Regards, Martin [0] https://github.com/xdarklight/linux/commits/meson-gx-integration-4.10-20161123 [1] http://marc.info/?l=linux-usb&m=147938307209685&w=2 sys_kernel_debug_dwc3_regdump_vendor_kernel Description: Binary data
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-24 12:11 PM, David Miller wrote: > From: Mark Lord > Date: Thu, 24 Nov 2016 11:43:53 -0500 > >> So even if this were a platform memory coherency issue, one should >> still never see ASCII data at the beginning of an rx buffer. > > I'm not so convinced, since this is the kind of random corruption one > would expect to see when dealing with virtual caches that have > aliasing or similar issues. > > Writes to address X that show up at address Y or not at all are > precisely the signature of virtual cache aliasing problems. > > Is it a case of the chip writing to X but the cpu is still seeing > stale data from a previous CPU store? > > For NFS the cpu is writing into the page cache, so we know that > cpu side stores are where the ASCII text is coming from. > > Now is the r8152 buffer one that the USB host controller is DMA'ing > into directly, or is it one that SWIOMMU or similar bounce buffering > is copying into? In the latter case we are doing cpu stores into > the area and the writes aren't coming from the device. >From tracing through the powerpc arch code, this is the buffer that is being directly DMA'd into. And the USB layer does an invalidate_dcache on that entire buffer before initiating the DMA (confirmed via printk). The driver itself NEVER writes anything to that buffer, and nobody else has a pointer to it other than the USB host controller, so there's nothing else that can write to it either. According to the driver writer, the chip should only ever write a fresh rx_desc struct at the beginning of a buffer, never ASCII data. So how does that buffer end up containing ASCII data from the NFS transfers? The only explanation I can see, is if the URB itself contains the data that we see in the URB buffer. Which is what one would expect. So for that to happen, the ethernet chip must be transferring that data. The thing that is special about the situation here, is that the processor is very slow (800Mhz 32-bit powerpc), and very busy elsewhere. So it can easily fall way behind in servicing the ethernet dongle, something that never happens with most modern faster machines. So perhaps this results in a FIFO overflow somewhere in the chip. We can boot/run this same machine from a USB memory stick, and nary a problem. Ditto for other types of ethernet dongles. But boot/run from that specific ethernet dongle, and we get regular random segfaults from corrupted page fetches over NFS. The only end-to-end data integrity available here is the rx checksum, when verified by software rather than trusting it to the chip/driver. One thought: bulk data streams are byte streams, not packets. Scheduling on the USB bus can break up larger transfers across multiple in-kernel buffers. A "real" URB buffer on USB2 is max 512 bytes. The driver is providing 16384-byte buffers, and assumes that data will never spill over from one such buffer to the next. Yet the observations here consistently show otherwise. Cheers -- Mark Lord -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On Thu, Nov 24, 2016 at 11:43:53AM -0500, Mark Lord wrote: > On 16-11-24 11:21 AM, David Miller wrote: > > From: Hayes Wang > > Date: Thu, 24 Nov 2016 13:26:55 + > > > > > I don't think the garbage results from our driver or device. > > This is my impression with what has been presented so far as well. > > It's not garbage. > > The latest run with the debug code I posted here earlier just spat out this > below. > Using coherent (guarded, non-cacheable) RX buffers, with mb() calls: > > [ 15.199157] r8152_check_rx_desc: rx_desc looks bad. > [ 15.204270] r8152_rx_bottom: offset=0/3376 bad rx_desc > [ 15.209584] r8152_dump_rx_desc: 3d435253 3034336d 202f3a30 47524154 > 2f3d5445 3034336d rx_len=21075 > > The bad data in this case is ASCII: > > "SRC=m3400:/ TARGET=/m340" Have you tried using usbmon? Details for how to use it is in Documentation/usbmon.txt and it might help you rule out the driver vs. the USB host controller issues as it sees the raw data the USB host controller sees before it sends it to the driver. 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
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-24 01:34 PM, Mark Lord wrote: >From tracing through the powerpc arch code, this is the buffer that > is being directly DMA'd into. And the USB layer does an invalidate_dcache > on that entire buffer before initiating the DMA (confirmed via printk). Slight correction: the invalidate_dcache_range() is only done when using kmalloc'd buffers. I have converted the driver here to use usb_alloc_coherent() instead, so that now gets skipped since the memory is never cached. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-24 01:42 PM, Greg KH wrote: > > Have you tried using usbmon? This system is running rootfs over NFS, so usbmon isn't realistically going to be usable in that scenario without a lot of reconfiguration of the setup (which in itself might obscure the original problem). There is a hardware USB analyzer in the building though. But it requires a MS-Windows machine (very scarce here, I don't have one) for the incredibly user-unfriendly software. I'm not sure if it can be setup to stop the trace somehow at the right point either, as it takes overnight runs usually to catch an occurrence of the issue. I also seem to recall that it only exports data captures in a proprietary format that only that brand of software/device can read, but perhaps that might not be true. Would still need to find a MS-Windows machine/license to even check it out though. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On Thu, Nov 24, 2016 at 01:34:08PM -0500, Mark Lord wrote: > One thought: bulk data streams are byte streams, not packets. > Scheduling on the USB bus can break up larger transfers across > multiple in-kernel buffers. A "real" URB buffer on USB2 is max 512 bytes. > The driver is providing 16384-byte buffers, and assumes that data will > never spill over from one such buffer to the next. > Yet the observations here consistently show otherwise. Wait, how do you know that data will not spill over? What is making that guarantee? Will the USB device send a "zero packet" in order to show that all of the "logical" data is now sent for this specific endpoint? Is there some sort of "framing" that the device does with the USB data so that the driver "knows" where the end of packet is? Check the zero-packet stuff for this device, that's tripped up many a USB driver writer over the years, myself included. 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
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-24 02:00 PM, Greg KH wrote: > On Thu, Nov 24, 2016 at 01:34:08PM -0500, Mark Lord wrote: >> One thought: bulk data streams are byte streams, not packets. >> Scheduling on the USB bus can break up larger transfers across >> multiple in-kernel buffers. A "real" URB buffer on USB2 is max 512 bytes. >> The driver is providing 16384-byte buffers, and assumes that data will >> never spill over from one such buffer to the next. >> Yet the observations here consistently show otherwise. > > Wait, how do you know that data will not spill over? What is making > that guarantee? Will the USB device send a "zero packet" in order to > show that all of the "logical" data is now sent for this specific > endpoint? Is there some sort of "framing" that the device does with the > USB data so that the driver "knows" where the end of packet is? Exactly my point. > Check the zero-packet stuff for this device, that's tripped up many a > USB driver writer over the years, myself included. I haven't tripped over it myself, but only because we were careful to allow for such in the USB drivers I have worked on. The r8152 driver just assumes it never happens. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On Thu, Nov 24, 2016 at 02:10:36PM -0500, Mark Lord wrote: > On 16-11-24 02:00 PM, Greg KH wrote: > > On Thu, Nov 24, 2016 at 01:34:08PM -0500, Mark Lord wrote: > >> One thought: bulk data streams are byte streams, not packets. > >> Scheduling on the USB bus can break up larger transfers across > >> multiple in-kernel buffers. A "real" URB buffer on USB2 is max 512 bytes. > >> The driver is providing 16384-byte buffers, and assumes that data will > >> never spill over from one such buffer to the next. > >> Yet the observations here consistently show otherwise. > > > > Wait, how do you know that data will not spill over? What is making > > that guarantee? Will the USB device send a "zero packet" in order to > > show that all of the "logical" data is now sent for this specific > > endpoint? Is there some sort of "framing" that the device does with the > > USB data so that the driver "knows" where the end of packet is? > > Exactly my point. > > > Check the zero-packet stuff for this device, that's tripped up many a > > USB driver writer over the years, myself included. > > I haven't tripped over it myself, but only because we were careful > to allow for such in the USB drivers I have worked on. > > The r8152 driver just assumes it never happens. Assumes what? That the host will always consume data faster than the device can create it? If so, that sounds like your real problem there... good luck! greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT PATCH 1/1] xhci: free xhci virtual devices with leaf nodes first
On 11/23/2016 04:24 AM, Mathias Nyman wrote: the tt_info provided by a HS hub might be in use to by a child device Make sure we free the devices in the correct order. This is needed in special cases such as when xhci controller is reset when resuming from hibernate, and all virt_devices are freed. Also free the virt_devices starting from max slot_id as children more commonly have higher slot_id than parent. CC: Signed-off-by: Mathias Nyman --- Guenter Roeck, does this work for you? Sorry, I didn't have time this week. I'll test it first thing next week. Guenter A rework of how tt_info is stored and used might be needed, but that will take some time and won't go to stable as easily. --- drivers/usb/host/xhci-mem.c | 38 -- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 6afe323..b3a5cd8 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -979,6 +979,40 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int slot_id) xhci->devs[slot_id] = NULL; } +/* + * Free a virt_device structure. + * If the virt_device added a tt_info (a hub) and has children pointing to + * 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) +{ + struct xhci_virt_device *vdev; + struct list_head *tt_list_head; + struct xhci_tt_bw_info *tt_info, *next; + int i; + + vdev = xhci->devs[slot_id]; + if (!vdev) + return; + + tt_list_head = &(xhci->rh_bw[vdev->real_port - 1].tts); + list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) { + /* is this a hub device that added a tt_info to the tts list */ + if (tt_info->slot_id == slot_id) { + /* are any devices using this tt_info? */ + for (i = 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) { + vdev = xhci->devs[i]; + if (vdev && (vdev->tt_info == tt_info)) + xhci_free_virt_devices_depth_first( + xhci, i); + } + } + } + /* we are now at a leaf device */ + xhci_free_virt_device(xhci, slot_id); +} + int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id, struct usb_device *udev, gfp_t flags) { @@ -1829,8 +1863,8 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) } } - for (i = 1; i < MAX_HC_SLOTS; ++i) - xhci_free_virt_device(xhci, i); + for (i = HCS_MAX_SLOTS(xhci->hcs_params1); i > 0; i--) + xhci_free_virt_devices_depth_first(xhci, i); dma_pool_destroy(xhci->segment_pool); xhci->segment_pool = NULL; -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] Revert "usb: dwc2: gadget: fix TX FIFO size and address initialization"
Hi John, > John Youn hat am 18. Oktober 2016 um 02:36 > geschrieben: > > > This reverts commit aa381a7259c3 ("usb: dwc2: gadget: fix TX FIFO size > and address initialization"). > > The original commit removed the FIFO size programming per endpoint. The > DPTXFSIZn register is also used for DIEPTXFn and the SIZE field is r/w > in dedicated fifo mode. So it isn't appropriate to simply remove this > initialization as it might break existing behavior. > > Also, some cores might not have enough fifo space to handle the > programming method used in the reverted patch, resulting in fifo > initialization failure. since the original patch is still necessary for bcm2835 gadget support i want to know if there is an alternative solution? Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
Mark Lord : [...] > >From tracing through the powerpc arch code, this is the buffer that > is being directly DMA'd into. And the USB layer does an invalidate_dcache > on that entire buffer before initiating the DMA (confirmed via printk). > > The driver itself NEVER writes anything to that buffer, > and nobody else has a pointer to it other than the USB host controller, > so there's nothing else that can write to it either. > > According to the driver writer, the chip should only ever write a fresh > rx_desc struct at the beginning of a buffer, never ASCII data. > > So how does that buffer end up containing ASCII data from the NFS transfers? Through aliasing the URB was given a page that contains said (previously) received file. The ethernet chip/usb host does not write anything in it. There could be a device or a driver problem but it may not be the real problem. So far the analysis focused on "how was this corrupted content written into this receive buffer page ?". If I read David correctly (?) the "nobody else has a pointer to it other than the USB host controller" point may be replaced with "the pointer to it aliases some already used page". -- Ueimor -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-24 07:27 PM, Francois Romieu wrote: > > Through aliasing the URB was given a page that contains said (previously) > received file. The ethernet chip/usb host does not write anything in it. I don't see how that could be possible. Please elaborate. The URB buffers are statically allocated by the driver at probe time, ten of them in all, allocated with usb_alloc_coherent() in the copy of the driver I am testing with. There is no possibility for them to be used for anything other than USB receive buffers, for this driver only. Nothing in the driver or kernel ever writes to those buffers after initial allocation, and only the driver and USB host controller ever have pointers to the buffers. -- Mark Lord -- 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] fsl/usb: Workarourd for USB erratum-A005697
The EHCI specification states the following in the SUSP bit description: In the Suspend state, the port is senstive to resume detection. Note that the bit status does not change untile the port is suspended and that there may be a delay in susupending a port if there is a transaction currently in progress on the USB. However, in NXP USBDR controller, the PORTSCx[SUSP] bit changes immediately when the application sets it and not when the port is actually suspended. So the application must wait for at least 10 milliseconds after a port indicates that it is suspended, to make sure this port has entered suspended state before initiating this port resume using the Force Port Resume bit. This bit is for NXP controller, not EHCI compatible. Signed-off-by: Changming Huang Signed-off-by: Ramneek Mehresh --- Change in v2: - move sleep out of spin-lock and add more comment for this workaround drivers/usb/host/ehci-fsl.c |3 +++ drivers/usb/host/ehci-hub.c |7 +++ drivers/usb/host/ehci.h |6 ++ drivers/usb/host/fsl-mph-dr-of.c |2 ++ include/linux/fsl_devices.h |1 + 5 files changed, 19 insertions(+) diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c index 9f5ffb6..91701cc 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/drivers/usb/host/ehci-fsl.c @@ -286,6 +286,9 @@ static int ehci_fsl_usb_setup(struct ehci_hcd *ehci) if (pdata->has_fsl_erratum_a005275 == 1) ehci->has_fsl_hs_errata = 1; + if (pdata->has_fsl_erratum_a005697 == 1) + ehci->has_fsl_susp_errata = 1; + if ((pdata->operating_mode == FSL_USB2_DR_HOST) || (pdata->operating_mode == FSL_USB2_DR_OTG)) if (ehci_fsl_setup_phy(hcd, pdata->phy_mode, 0)) diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index 74f62d6..81e2310 100644 --- a/drivers/usb/host/ehci-hub.c +++ b/drivers/usb/host/ehci-hub.c @@ -310,6 +310,13 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) } spin_unlock_irq(&ehci->lock); + if (changed && ehci_has_fsl_susp_errata(ehci)) + /* Wait for at least 10 millisecondes to ensure the controller +* enter the suspend status before initiating a port resume +* using the Fore Port Resume bit (Not-EHCI compatible). +*/ + usleep_range(1, 2); + if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) { /* * Wait for HCD to enter low-power mode or for the bus diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h index 3f3b74a..7706e4a 100644 --- a/drivers/usb/host/ehci.h +++ b/drivers/usb/host/ehci.h @@ -219,6 +219,7 @@ struct ehci_hcd { /* one per controller */ unsignedno_selective_suspend:1; unsignedhas_fsl_port_bug:1; /* FreeScale */ unsignedhas_fsl_hs_errata:1;/* Freescale HS quirk */ + unsignedhas_fsl_susp_errata:1; /*Freescale SUSP quirk*/ unsignedbig_endian_mmio:1; unsignedbig_endian_desc:1; unsignedbig_endian_capbase:1; @@ -703,10 +704,15 @@ struct ehci_tt { #if defined(CONFIG_PPC_85xx) /* Some Freescale processors have an erratum (USB A-005275) in which * incoming packets get corrupted in HS mode + * Some Freescale processors have an erratum (USB A-005697) in which + * we need to wait for 10ms for bus to fo into suspend mode after + * setting SUSP bit */ #define ehci_has_fsl_hs_errata(e) ((e)->has_fsl_hs_errata) +#define ehci_has_fsl_susp_errata(e)((e)->has_fsl_susp_errata) #else #define ehci_has_fsl_hs_errata(e) (0) +#define ehci_has_fsl_susp_errata(e)(0) #endif /* diff --git a/drivers/usb/host/fsl-mph-dr-of.c b/drivers/usb/host/fsl-mph-dr-of.c index f07ccb2..e90ddb5 100644 --- a/drivers/usb/host/fsl-mph-dr-of.c +++ b/drivers/usb/host/fsl-mph-dr-of.c @@ -226,6 +226,8 @@ static int fsl_usb2_mph_dr_of_probe(struct platform_device *ofdev) of_property_read_bool(np, "fsl,usb-erratum-a007792"); pdata->has_fsl_erratum_a005275 = of_property_read_bool(np, "fsl,usb-erratum-a005275"); + pdata->has_fsl_erratum_a005697 = + of_property_read_bool(np, "fsl,usb_erratum-a005697"); /* * Determine whether phy_clk_valid needs to be checked diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h index f291291..60cef82 100644 --- a/include/linux/fsl_devices.h +++ b/include/linux/fsl_devices.h @@ -100,6 +100,7 @@ struct fsl_usb2_platform_data { unsignedalready_suspended:1; unsignedhas_fsl_erratum_a007792:1; unsignedhas_fsl_erratum_a005275:1; + unsignedhas_fsl_erratum_a005697:1; unsignedcheck_phy_clk_valid:1; /* register save area f
RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
Mark Lord [mailto:ml...@pobox.com] > Sent: Thursday, November 24, 2016 11:25 PM [...] > x86 has near fully-coherent memory, so it is the "easy" platform > to get things working on. But Linux supports a very diverse number > of platforms, with varying degrees of cache/memory coherency, > and it can be tricky for things to work correctly on all of them. However, I have test iperf on raspberry pi v1 which you suggest for more than one day. I still couldn't reproduce your issue. > If you are testing with the driver as currently in 4.4.34, > then you won't even notice when things are screwing up, > because the driver just silently drops packets. > Or it passes them on without noticing that they have bad data. I only drop the packet silently when the rx descriptor outside the urb buffer. Then, I check the rx descriptor before checking the length of the packet. > Here (attached) is the instrumented driver I am using here now. > I suggest you use it or something similar when testing, > and not the stock driver. I would test it again with your driver. [...] > Also, unrelated, but inside r8152_submit_rx() there is this code: > > /* The rx would be stopped, so skip submitting */ > if (test_bit(RTL8152_UNPLUG, &tp->flags) || > !test_bit(WORK_ENABLE, &tp->flags) > || !netif_carrier_ok(tp->netdev)) > return 0; > > If that "return 0" statement is ever executed, doesn't it result > in the loss/leak of a buffer? They would be found back by calling rtl_start_rx(), when the rx is restarted. Best Regards, Hayes -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
Mark Lord [mailto:ml...@pobox.com] > Sent: Friday, November 25, 2016 12:44 AM [...] > The bad data in this case is ASCII: > > "SRC=m3400:/ TARGET=/m340" > > This data is what is seen in /run/mount/utab, a file that is read/written > over NFS on > each boot. > > "SRC=m3400:/ TARGET=/m3400 ROOT=/ > ATTRS=nolock,addr=192.168.8.1\n" > > But how does this ASCII data end up at offset zero of the rx buffer?? > Not possible -- this isn't even stale data, because only an rx_desc could > be at that offset in that buffer. > > So even if this were a platform memory coherency issue, one should still > never see ASCII data at the beginning of an rx buffer. The driver NEVER > writes anything to the rx buffers. Only the USB hardware ever does. > > And only the r8152 dongle/driver exhibits this issue. > Other USB dongles do not. They *might* still have such issues, > but because they use software checksums, the bad packets are caught/rejected. Do you test it by rebooting? Maybe you could try a patch commit 93fe9b183840 ("r8152: reset the bmu"). However, it should only occur for the first urb buffer after rx is reset. I don't think you would reset the rx frequently, so the situation seems to be different. Best Regards, Hayes -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
> Mark Lord [mailto:ml...@pobox.com] > > Sent: Friday, November 25, 2016 12:44 AM > [...] > > The bad data in this case is ASCII: > > > > "SRC=m3400:/ TARGET=/m340" > > > > This data is what is seen in /run/mount/utab, a file that is read/written > > over NFS > on > > each boot. > > > > "SRC=m3400:/ TARGET=/m3400 ROOT=/ > > ATTRS=nolock,addr=192.168.8.1\n" > > > > But how does this ASCII data end up at offset zero of the rx buffer?? > > Not possible -- this isn't even stale data, because only an rx_desc could > > be at that offset in that buffer. > > > > So even if this were a platform memory coherency issue, one should still > > never see ASCII data at the beginning of an rx buffer. The driver NEVER > > writes anything to the rx buffers. Only the USB hardware ever does. > > > > And only the r8152 dongle/driver exhibits this issue. > > Other USB dongles do not. They *might* still have such issues, > > but because they use software checksums, the bad packets are > > caught/rejected. > > Do you test it by rebooting? Maybe you could try a patch > commit 93fe9b183840 ("r8152: reset the bmu"). However, it should > only occur for the first urb buffer after rx is reset. I don't > think you would reset the rx frequently, so the situation seems > to be different. Forgive me. I provide wrong information. This is about RTL8153, not RTL8152. Best Regards, Hayes -- 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