Re: [PATCH 1/5 v10] dt/bindings: Add binding for the DA8xx MUSB driver
Hi, Petr Kulhavy writes: > + - ti,usb2-phy-refclock-hz : Integer. Frequency in Hz of the USB 2.0 PHY > reference clock, > + either provided by the internal PLL or an external source. > + The supported values are: 12MHz, 13MHz, 19.2MHz, 20MHz, 24MHz, 26MHz, > 38.4MHz, 40MHz, 48MHz. why isn't this an actual clock phandle ? Driver could, then, use clk_get_rate() to figure this one out. You could just use a fixed clock here to satisfy the clock API. -- balbi signature.asc Description: PGP signature
Re: Freezable workqueue blocks non-freezable workqueue during the system resume process
On Fri 11-03-16 12:56:10, Tejun Heo wrote: > Hello, Jan. > > On Thu, Mar 03, 2016 at 10:33:10AM +0100, Jan Kara wrote: > > > Ugh... that's nasty. I wonder whether the right thing to do is making > > > writeback workers non-freezable. IOs are supposed to be blocked from > > > lower layer anyway. Jan, what do you think? > > > > Well no, at least currently IO is not blocked in lower layers AFAIK - for > > that you'd need to freeze block devices & filesystems and there are issues > > At least libata does and I think SCSI does too, but yeah, there > probably are drivers which depend on block layer blocking IOs, which > btw is a pretty fragile way to go about as upper layers might not be > the only source of activities. > > > with that (Jiri Kosina was the last one which was trying to make this work > > AFAIR). And I think you need to stop writeback (and generally any IO) to be > > generated so that it doesn't interact in a strange way with device drivers > > being frozen. So IMO until suspend freezes filesystems & devices properly > > you have to freeze writeback workqueue. > > I still think the right thing to do is plugging that block layer or > low level drivers. It's like we're trying to plug multiple sources > when we can plug the point where they come together anyway. I agree that freezing writeback workers is a workaround for real issues at best and ideally we shouldn't have to do that. But at least for now I had the impression that it is needed for suspend to work reasonably reliably. Honza -- Jan Kara SUSE Labs, CR -- 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 3/7] usb: mux: add common code for Intel dual role port mux
On 03/14/2016 11:27 AM, Greg Kroah-Hartman wrote:
> On Mon, Mar 14, 2016 at 09:09:22AM +0800, Lu Baolu wrote:
>> On 03/11/2016 08:06 AM, Greg Kroah-Hartman wrote:
>>> On Tue, Mar 08, 2016 at 03:53:44PM +0800, Lu Baolu wrote:
+struct intel_mux_dev {
+ struct device *dev;
+ char*extcon_name;
+ char*cable_name;
+ int (*cable_set_cb)(struct intel_mux_dev *mux);
+ int (*cable_unset_cb)(struct intel_mux_dev *mux);
+};
>>> This is a device, why not make it one? Don't just hold a reference.
>>> And do you really even hold that reference?
>> It's not a device. It's just an encapsulation for parameters passed into
>> intel_usb_mux_register().
> But you called it a device, so you can understand my confusion.
>
> And why not make it a device? Why isn't this one? Hint, I really think
> it should be...
I am sorry for the confusion.
The mux device has already been created. It could be a child of a mfd device,
or be created explicitly as a platform device. The mux driver (for example,
intel-mux-gpio.ko) will bind to the device. intel-mux-gpio.ko will then call
into this
shared code. This shared code is actually part of the mux driver, except that it
could also be used in other mux driver (for example, intel-mux-drcfg.ko).
I can't always expect the mux device to be created with intel_usb_mux_register()
since it could possibly be a cell of a mfd, or just an ACPI device. :-)
>
+#if IS_ENABLED(CONFIG_INTEL_USB_MUX)
+extern int intel_usb_mux_register(struct intel_mux_dev *mux);
+extern int intel_usb_mux_unregister(struct device *dev);
>>> It's obvious you didn't run this through checkpatch.pl, please do so...
>> I did, but didn't hit any errors or warnings.
> Odd, don't put extern in .h files for functions, I thought checkpatch
> catches that...
>
> Try it with --strict, as you should with all new code you submit.
Yes, if I add --strict, I hit some warnings.
I will fix these warnings and always run checkpatch.pl with --strict
before I submit patches.
Thank you!
>
>>> And your api is horrid, think about what you want the "core" to do here,
>>> it should be the one creating the device and returning it to the caller,
>>> not forcing the caller to somehow create it first and then pass it in.
>> This isn't a layer or core. It doesn't create any new devices. It's actually
>> some shared code which can be used by all Intel dual role port drivers.
> It should be a device, as you are treating it like one :)
I have answered above.
>
>> I put it in a separated file because 1) this can avoid duplication; 2) this
>> code
>> could be used for any architectures as long as a USB port is shared by
>> two components and it needs an OS response when event triggers.
> It's a bit hard for other arches to be using something called "intel_"
> :(
Do you mind if I change the symbols to something like "usb_mux_"?
Or, keep it Intel specific now and change it when there is real other
consumers later?
>> I guess intel_usb_mux_register/unregister() is a bit misleading. How about
>> changing them to intel_usb_mux_probe/remove()?
> You are going to probe/remove something that isn't a device? Come on
> now...
>
>>> And why is it not symmetrical, you are passing one thing into register
>>> and another into unregister.
>> struct intel_mux_dev is an encapsulation for parameters passed into
>> intel_usb_mux_register().
> Which is a device.
>
>> It's not a new device structure though the name
>> is a bit misleading.
> Yes it is, hint, you want it to be a device.
>
>> How about remove this structure and put these in function parameters?
> How about making it a real device? :)
The mux is a real device. :-) As my understanding (please correct me if I
misunderstood it), the question is that should 1) the device be created before
passing it to intel_usb_mux_register(), or 2) let intel_usb_mux_register()
create
the device.
IMHO, option 2) is not possible for Intel platform. The port mux in Intel device
could be an ACPI device or part of an ACPI device. Hence, the mux device could
be created implicitly by ACPI or mfd framework.
ps. this function name is really confusing. It isn't designed to register a new
mux device, but an interface for a shared common code. Sorry about it and
I will look for a meaningful one for the next version.
Best Regards,
Baolu
>
> 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] usb: dwc3: add disable receiver detection in P3 quirk
> -Original Message-
> From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
> Sent: Monday, March 14, 2016 12:26 PM
> To: Rajesh Bhagat
> Cc: gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-o...@vger.kernel.org; Sriram Dash
> ; Rajesh Bhagat
> Subject: Re: [PATCH] usb: dwc3: add disable receiver detection in P3 quirk
>
>
> Hi,
>
Hello Felipe,
Thanks for the comments.
> Rajesh Bhagat writes:
> > [ text/plain ]
> > Some freescale QorIQ platforms require to disable receiver detection
> > in P3 for correct detection of USB devices. If
> > GUSB3PIPECTL(DISRXDETINP3) is set, Core will change PHY power state to
> > P2 and then perform receiver detection. After receiver detection, Core
> > will change PHY power state to P3. Same quirk would be added in dts file in
> > future
> patches.
> >
> > Signed-off-by: Sriram Dash
> > Signed-off-by: Rajesh Bhagat
> > ---
> > drivers/usb/dwc3/core.c |6 ++
> > drivers/usb/dwc3/core.h |2 ++
> > drivers/usb/dwc3/platform_data.h |1 +
> > 3 files changed, 9 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
> > de5e01f..b2f2b08 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -446,6 +446,9 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
> > if (dwc->u2ss_inp3_quirk)
> > reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK;
> >
> > + if (dwc->dis_rxdet_inp3_quirk)
> > + reg |= DWC3_GUSB3PIPECTL_DISRXDETINP3;
> > +
> > if (dwc->req_p1p2p3_quirk)
> > reg |= DWC3_GUSB3PIPECTL_REQP1P2P3;
> >
> > @@ -903,6 +906,8 @@ static int dwc3_probe(struct platform_device *pdev)
> > "snps,u2exit_lfps_quirk");
> > dwc->u2ss_inp3_quirk = device_property_read_bool(dev,
> > "snps,u2ss_inp3_quirk");
> > + dwc->dis_rxdet_inp3_quirk = device_property_read_bool(dev,
> > + "snps,dis_rxdet_inp3_quirk");
>
> not documented under Documentation/devicetree/bindings/usb/dwc3.txt.
>
Will take care in v2.
> > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index
> > e4f8b90..41cc22c 100644
> > --- a/drivers/usb/dwc3/core.h
> > +++ b/drivers/usb/dwc3/core.h
> > @@ -193,6 +193,7 @@
> > /* Global USB3 PIPE Control Register */
> > #define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31)
> > #define DWC3_GUSB3PIPECTL_U2SSINP3OK (1 << 29)
> > +#define DWC3_GUSB3PIPECTL_DISRXDETINP3 (1 << 28)
> > #define DWC3_GUSB3PIPECTL_REQP1P2P3(1 << 24)
> > #define DWC3_GUSB3PIPECTL_DEP1P2P3(n) ((n) << 19)
> > #define DWC3_GUSB3PIPECTL_DEP1P2P3_MASK
> DWC3_GUSB3PIPECTL_DEP1P2P3(7)
> > @@ -873,6 +874,7 @@ struct dwc3 {
> >
> > unsignedtx_de_emphasis_quirk:1;
> > unsignedtx_de_emphasis:2;
> > + unsigneddis_rxdet_inp3_quirk:1;
>
> _must_ be sorted alphabetically and you _must_ update the kernel doc above
> this
> structure.
>
Will take care in v2.
> > };
> >
> > /*
> > --
> > */ diff --git a/drivers/usb/dwc3/platform_data.h
> > b/drivers/usb/dwc3/platform_data.h
> > index 2bb4d3a..9df1dfb 100644
> > --- a/drivers/usb/dwc3/platform_data.h
> > +++ b/drivers/usb/dwc3/platform_data.h
> > @@ -46,6 +46,7 @@ struct dwc3_platform_data {
> >
> > unsigned tx_de_emphasis_quirk:1;
> > unsigned tx_de_emphasis:2;
> > + unsigned dis_rxdet_inp3_quirk:1;
>
> likewise.
>
Will take care in v2.
> --
> balbi
--
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: NEC uPD720200 xHCI Controller dies when Runtime PM enabled
On 13.03.2016 11:16, Mike Murdoch wrote: On 2016-03-01 16:32, Mathias Nyman wrote: On 18.02.2016 18:34, Mike Murdoch wrote: On 2016-02-18 16:12, Mathias Nyman wrote: On 16.02.2016 23:58, main.ha...@googlemail.com wrote: On 2016-02-08 15:31, Mathias Nyman wrote: Hi On 06.02.2016 19:08, Mike Murdoch wrote: Bug ID: 111251 Hello, I have a NEC uPD720200 USB3.0 controller in a Thinkpad W520 laptop on kernel 4.4.1-gentoo. 0e:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host Controller (rev 04) (prog-if 30 [XHCI]) Subsystem: Lenovo uPD720200 USB 3.0 Host Controller When runtime power control for this controller is disabled (/sys/bus/pci/devices/:0e:00.0/power/control = on), the controller works fine and reaches over 120MB/s transfer rates. When runtime power control for this controller is enabled (/sys/bus/pci/devices/:0e:00.0/power/control = auto), two effects can be observed: - Transfer rates are much lower at around 30MB/s - During transfers, the controller dies after a couple of seconds: At this point, a reboot is required to reactivate the controller, unloading and reloading the xhci_* modules does not work. ... I did some more digging, there are a few things that need to be addressed: 1. We should resume USB3 bus before USB2 bus to let devices enumerate as USB3 better, this gives them more time to finish the link training. 2. After resuming xhci we don't see any port changes immediately, hub thinks nothing happended and stops polling the ports, hub will suspend again -> xhci will try to suspend. 3. Roothubs will autosuspend immediately after autoresume, (autosuspend timeout = 0) This could be a reason why we see the "xhci_suspend" entry in the log. We either need to increase the autosuspend timeout, or prevent suspend if we can see the pending event in a xhci status register. inserting usb3 storage device Feb 16 20:03:33 xhci_hcd :0e:00.0: // Setting command ring address to 0xe001 Feb 16 20:03:33 xhci_hcd :0e:00.0: xhci_resume: starting port polling. Feb 16 20:03:33 xhci_hcd :0e:00.0: xhci_hub_status_data: stopping port polling. Feb 16 20:03:33 xhci_hcd :0e:00.0: xhci_suspend: stopping port polling. I got a few patches, attached. They both partially try to fix the issue, and add more logging. Same changes can be found in a topic branch from in: git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git bug_usb3_enum_rtresume Any chance to try them out? -Mathias Hello, I've come around to testing these patches. I applied them all at once (did you want me to test them individually?) and they appear to fix this issue completely! Full speed and no dead controllers.Do you need any further logs? That's good news. Can I add your "Tested-by:" tag to two of the patches? I'll send them as fixes after rc1 is out. No more logs needed as it works, I'll send the third additional debug info patch to usb-next later. It will be useful for future debugging Thanks Mathias for further debugging this case The third patch is just additional debug info and useful for future debugging (or if those -- 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 0/2] usb: dwc3: add disable receiver detection in P3 quirk
Adds disable receiver detection in P3 quirk in DWC3 driver. Rajesh Bhagat (2): usb: dwc3: add disable receiver detection in P3 quirk Documentation: dt: dwc3: Add snps,dis_rxdet_inp3_quirk property Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++ drivers/usb/dwc3/core.c| 6 ++ drivers/usb/dwc3/core.h| 2 ++ drivers/usb/dwc3/platform_data.h | 1 + 4 files changed, 11 insertions(+) -- 2.6.2.198.g614a2ac -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] usb: dwc3: add disable receiver detection in P3 quirk
Some freescale QorIQ platforms require to disable receiver detection
in P3 for correct detection of USB devices. If GUSB3PIPECTL(DISRXDETINP3)
is set, Core will change PHY power state to P2 and then perform receiver
detection. After receiver detection, Core will change PHY power state to
P3. Same quirk would be added in dts file in future patches.
Signed-off-by: Sriram Dash
Signed-off-by: Rajesh Bhagat
---
Changes for v2 :
- added the property code in alphabetical order.
drivers/usb/dwc3/core.c | 6 ++
drivers/usb/dwc3/core.h | 2 ++
drivers/usb/dwc3/platform_data.h | 1 +
3 files changed, 9 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index de5e01f..c840df0 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -446,6 +446,9 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->u2ss_inp3_quirk)
reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK;
+ if (dwc->dis_rxdet_inp3_quirk)
+ reg |= DWC3_GUSB3PIPECTL_DISRXDETINP3;
+
if (dwc->req_p1p2p3_quirk)
reg |= DWC3_GUSB3PIPECTL_REQP1P2P3;
@@ -919,6 +922,8 @@ static int dwc3_probe(struct platform_device *pdev)
"snps,dis_u2_susphy_quirk");
dwc->dis_enblslpm_quirk = device_property_read_bool(dev,
"snps,dis_enblslpm_quirk");
+ dwc->dis_rxdet_inp3_quirk = device_property_read_bool(dev,
+ "snps,dis_rxdet_inp3_quirk");
dwc->tx_de_emphasis_quirk = device_property_read_bool(dev,
"snps,tx_de_emphasis_quirk");
@@ -953,6 +958,7 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->dis_u3_susphy_quirk = pdata->dis_u3_susphy_quirk;
dwc->dis_u2_susphy_quirk = pdata->dis_u2_susphy_quirk;
dwc->dis_enblslpm_quirk = pdata->dis_enblslpm_quirk;
+ dwc->dis_rxdet_inp3_quirk = pdata->dis_rxdet_inp3_quirk;
dwc->tx_de_emphasis_quirk = pdata->tx_de_emphasis_quirk;
if (pdata->tx_de_emphasis)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index e4f8b90..494119c 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -193,6 +193,7 @@
/* Global USB3 PIPE Control Register */
#define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31)
#define DWC3_GUSB3PIPECTL_U2SSINP3OK (1 << 29)
+#define DWC3_GUSB3PIPECTL_DISRXDETINP3 (1 << 28)
#define DWC3_GUSB3PIPECTL_REQP1P2P3(1 << 24)
#define DWC3_GUSB3PIPECTL_DEP1P2P3(n) ((n) << 19)
#define DWC3_GUSB3PIPECTL_DEP1P2P3_MASKDWC3_GUSB3PIPECTL_DEP1P2P3(7)
@@ -870,6 +871,7 @@ struct dwc3 {
unsigneddis_u3_susphy_quirk:1;
unsigneddis_u2_susphy_quirk:1;
unsigneddis_enblslpm_quirk:1;
+ unsigneddis_rxdet_inp3_quirk:1;
unsignedtx_de_emphasis_quirk:1;
unsignedtx_de_emphasis:2;
diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h
index 2bb4d3a..aaa44db 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -43,6 +43,7 @@ struct dwc3_platform_data {
unsigned dis_u3_susphy_quirk:1;
unsigned dis_u2_susphy_quirk:1;
unsigned dis_enblslpm_quirk:1;
+ unsigned dis_rxdet_inp3_quirk:1;
unsigned tx_de_emphasis_quirk:1;
unsigned tx_de_emphasis:2;
--
2.6.2.198.g614a2ac
--
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: merge xhci_queue_bulk_tx and queue_bulk_sg_tx functions
On 10.03.2016 12:54, Alexandr Ivanov wrote: In drivers/usb/host/xhci-ring.c there are two functions (xhci_queue_bulk_tx and queue_bulk_sg_tx) that are very similar, so a lot of code duplication. This patch merges these functions into to one xhci_queue_bulk_tx. Also counting the needed TRBs is merged and refactored. Signed-off-by: Alexandr Ivanov Very nice refactoring and cleanup. Have you tested that bulk transfers still work as they should? If everything is ok I'll send this forward to usb-next after the 4.6-rc1 merge window 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
[PATCH v2 2/2] Documentation: dt: dwc3: Add snps,dis_rxdet_inp3_quirk property
Add snps,dis_rxdet_inp3_quirk property which disables receiver detection in PHY P3 power state. Signed-off-by: Sriram Dash Signed-off-by: Rajesh Bhagat --- Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt index fb2ad0a..9e546c4 100644 --- a/Documentation/devicetree/bindings/usb/dwc3.txt +++ b/Documentation/devicetree/bindings/usb/dwc3.txt @@ -38,6 +38,8 @@ Optional properties: - snps,dis_u2_susphy_quirk: when set core will disable USB2 suspend phy. - snps,dis_enblslpm_quirk: when set clears the enblslpm in GUSB2PHYCFG, disabling the suspend signal to the PHY. + - snps,dis_rxdet_inp3_quirk: when set core will disable receiver detection + in PHY P3 power state. - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal utmi_l1_suspend_n, false when asserts utmi_sleep_n - snps,hird-threshold: HIRD threshold -- 2.6.2.198.g614a2ac -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 1/2] usb: host: xhci: add a new quirk XHCI_CLEAR_AC64
Hi,
> Hi,
>
> Yoshihiro Shimoda writes:
> > [ text/plain ]
> > On some xHCI controllers (e.g. R-Car SoCs), the AC64 bit (bit 0) of
> > HCCPARAMS1 is set to 1. However, the xHCs don't support 64-bit
> > address memory pointers actually. So, in this case, this driver should
> > call dma_set_coherent_mask(dev, DMA_BIT_MASK(32)) in xhci_gen_setup().
> > Otherwise, the xHCI controller will be died after a usb device is
> > connected if it runs on above 4GB physical memory environment.
> >
> > So, this patch adds a new quirk XHCI_CLEAR_AC64 to resolve such an issue.
> >
> > Signed-off-by: Yoshihiro Shimoda
> > ---
> > drivers/usb/host/xhci.c | 10 ++
> > drivers/usb/host/xhci.h | 1 +
> > 2 files changed, 11 insertions(+)
> >
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index d51ee0c..4b5e979 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -4948,6 +4948,16 @@ int xhci_gen_setup(struct usb_hcd *hcd,
> > xhci_get_quirks_t get_quirks)
> > return retval;
> > xhci_dbg(xhci, "Reset complete\n");
> >
> > + /*
> > +* On some xHCI controllers (e.g. R-Car SoCs), the AC64 bit (bit 0)
> > +* of HCCPARAMS1 is set to 1. However, the xHCs don't support 64-bit
> > +* address memory pointers actually. So, this driver clears the AC64
> > +* bit of xhci->hcc_params to call dma_set_coherent_mask(dev,
> > +* DMA_BIT_MASK(32)) in this xhci_gen_setup().
> > +*/
> > + if (xhci->quirks & XHCI_CLEAR_AC64)
> > + xhci->hcc_params &= ~BIT(0);
> > +
> > /* Set dma_mask and coherent_dma_mask to 64-bits,
> > * if xHC supports 64-bit addressing */
> > if (HCC_64BIT_ADDR(xhci->hcc_params) &&
> > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> > index e293e09..1963148 100644
> > --- a/drivers/usb/host/xhci.h
> > +++ b/drivers/usb/host/xhci.h
> > @@ -1641,6 +1641,7 @@ struct xhci_hcd {
> > #define XHCI_PME_STUCK_QUIRK (1 << 20)
> > #define XHCI_MTK_HOST (1 << 21)
> > #define XHCI_SSIC_PORT_UNUSED (1 << 22)
> > +#define XHCI_CLEAR_AC64(1 << 23)
>
> hmm, the quirk isn't really CLEAR_AC64, that's the workaround. How about
> calling this XHCI_NO_64BIT_SUPPORT ? or something along those lines.
Thank you for the suggestion. I agree with you.
So, I will change the name to XHCI_NO_64BIT_SUPPORT.
Best regards,
Yoshihiro Shimoda
> --
> balbi
--
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/2] usb: host: xhci-plat: fix cannot work if R-Car Gen2/3 run on above 4GB phys
Hi,
> Yoshihiro Shimoda writes:
>
> > [ text/plain ]
> > This patch fixes an issue that cannot work if R-Car Gen2/3 run on
> > above 4GB physical memory environment to use a quirk XHCI_CLEAR_AC64.
> >
> > Signed-off-by: Yoshihiro Shimoda
> > ---
> > drivers/usb/host/xhci-plat.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> > index 5c15e9b..4dbd56f 100644
> > --- a/drivers/usb/host/xhci-plat.c
> > +++ b/drivers/usb/host/xhci-plat.c
> > @@ -39,12 +39,19 @@ static const struct xhci_driver_overrides
> > xhci_plat_overrides __initconst = {
> >
> > static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
> > {
> > + struct usb_hcd *hcd = xhci_to_hcd(xhci);
> > +
> > /*
> > * As of now platform drivers don't provide MSI support so we ensure
> > * here that the generic code does not try to make a pci_dev from our
> > * dev struct in order to setup MSI
> > */
> > xhci->quirks |= XHCI_PLAT;
> > +
> > + /* Please refer to the xhci.c about the detail of this quirk */
>
> this is an odd comment. It should be okay to state here that RCAR
> GEN2/GEN3 lack proper 64BIT support even though register says otherwise.
Thank you for the comment. I will add comments like a patch v1.
Best regards,
Yoshihiro Shimoda
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] usb: host: xhci-plat: fix cannot work if R-Car Gen2/3 run on above 4GB phys
This patch fixes an issue that cannot work if R-Car Gen2/3 run on
above 4GB physical memory environment to use a quirk XHCI_NO_64BIT_SUPPORT.
Signed-off-by: Yoshihiro Shimoda
---
drivers/usb/host/xhci-plat.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 5c15e9b..474b5fa 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -39,12 +39,25 @@ static const struct xhci_driver_overrides
xhci_plat_overrides __initconst = {
static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
{
+ struct usb_hcd *hcd = xhci_to_hcd(xhci);
+
/*
* As of now platform drivers don't provide MSI support so we ensure
* here that the generic code does not try to make a pci_dev from our
* dev struct in order to setup MSI
*/
xhci->quirks |= XHCI_PLAT;
+
+ /*
+* On R-Car Gen2 and Gen3, the AC64 bit (bit 0) of HCCPARAMS1 is set
+* to 1. However, these SoCs don't support 64-bit address memory
+* pointers. So, this driver clears the AC64 bit of xhci->hcc_params
+* to call dma_set_coherent_mask(dev, DMA_BIT_MASK(32)) in
+* xhci_gen_setup().
+*/
+ if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
+ xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
+ xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
}
/* called during probe() after chip reset completes */
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/2] usb: host: xhci: add a new quirk and fix an issue for R-Car Gen2/3
This patch set is based on the latest usb.git / usb-next branch. (commit id = ce53bfc4374cada8b645765e2b4ad5831e760932) Changes from v2: - Change the quirk name to "XHCI_NO_64BIT_SUPPORT". - Add comments for R-Car Gen2/3 (like a patch v1) in xhci-plat.c. Changes from v1: - Add a new quirk "XHCI_CLEAR_AC64" for using it on some PCIe card. http://thread.gmane.org/gmane.linux.kernel.renesas-soc/858/focus=1694 Yoshihiro Shimoda (2): usb: host: xhci: add a new quirk XHCI_NO_64BIT_SUPPORT usb: host: xhci-plat: fix cannot work if R-Car Gen2/3 run on above 4GB phys drivers/usb/host/xhci-plat.c | 13 + drivers/usb/host/xhci.c | 10 ++ drivers/usb/host/xhci.h | 1 + 3 files changed, 24 insertions(+) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] usb: host: xhci: add a new quirk XHCI_NO_64BIT_SUPPORT
On some xHCI controllers (e.g. R-Car SoCs), the AC64 bit (bit 0) of
HCCPARAMS1 is set to 1. However, the xHCs don't support 64-bit
address memory pointers actually. So, in this case, this driver should
call dma_set_coherent_mask(dev, DMA_BIT_MASK(32)) in xhci_gen_setup().
Otherwise, the xHCI controller will be died after a usb device is
connected if it runs on above 4GB physical memory environment.
So, this patch adds a new quirk XHCI_NO_64BIT_SUPPORT to resolve
such an issue.
Signed-off-by: Yoshihiro Shimoda
---
drivers/usb/host/xhci.c | 10 ++
drivers/usb/host/xhci.h | 1 +
2 files changed, 11 insertions(+)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d51ee0c..489706f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4948,6 +4948,16 @@ int xhci_gen_setup(struct usb_hcd *hcd,
xhci_get_quirks_t get_quirks)
return retval;
xhci_dbg(xhci, "Reset complete\n");
+ /*
+* On some xHCI controllers (e.g. R-Car SoCs), the AC64 bit (bit 0)
+* of HCCPARAMS1 is set to 1. However, the xHCs don't support 64-bit
+* address memory pointers actually. So, this driver clears the AC64
+* bit of xhci->hcc_params to call dma_set_coherent_mask(dev,
+* DMA_BIT_MASK(32)) in this xhci_gen_setup().
+*/
+ if (xhci->quirks & XHCI_NO_64BIT_SUPPORT)
+ xhci->hcc_params &= ~BIT(0);
+
/* Set dma_mask and coherent_dma_mask to 64-bits,
* if xHC supports 64-bit addressing */
if (HCC_64BIT_ADDR(xhci->hcc_params) &&
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e293e09..70f215c 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1641,6 +1641,7 @@ struct xhci_hcd {
#define XHCI_PME_STUCK_QUIRK (1 << 20)
#define XHCI_MTK_HOST (1 << 21)
#define XHCI_SSIC_PORT_UNUSED (1 << 22)
+#define XHCI_NO_64BIT_SUPPORT (1 << 23)
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
/* There are two roothubs to keep track of bus suspend info for */
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: system awareness of endpoints
On Sat, 2016-03-12 at 20:30 -0800, Greg KH wrote: > On Sat, Mar 12, 2016 at 07:56:03PM -0800, bruce m beach wrote: > > I believe I can change the Device descriptors in firmware, > > upload the new firmware and reboot the stick but wouldn't > > the usb subsystem have the old device descriptor from the > > previous hotplug/system boot? > > Nope, the kernel reads the descriptor from the device when it is > connected, you can check this yourself by doing 'lsusb -v' when it is > plugged in. > > > If so then somehow I need to to tell the usb subsystem to re-request > > the device descriptor. kernel calls are okay. Any thoughts? > > No need to do anything in the kernel, all should be fine as long as your > firmware is correct. The firmware uploader may need to do a reset though. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: core: add power sequence for USB devices
On Sat, Mar 05, 2016 at 03:10:11PM +0100, Andrew Lunn wrote: > > So, would you like to accept the generic solution like below: > > > > - Create a generic power sequence driver, and it will be probed > > according to compatible string at device tree. At its probe, > > we can create a power sequence structure, and let this structure > > as the private data for this power sequence device. > > I'm not sure a separate driver is required. Why not consider it more > like pinctrl properties? They are listed in the devices node. Have the > bus enumerate code first walk all children and run their on sequence. > Bus shutdown would again walk the children and run the off sequence. > The device which needs power sequence may not at platform bus, it may be at USB bus, MMC bus, etc. >From what I see, A generic pwrseq driver can cover gpio-en, gpio-rst, clock, clock-freq, etc, but I find the pwrseq_emmc does something special, like request a restart handler. Add Ulf, who is the power sequence author for MMC. Hi Ulf, Rob suggested that if we can create some generic things (driver or library) for power sequence for all devices, what do you think? -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: chipidea: Configure DMA properties and ops from DT
On Wed, Mar 09, 2016 at 05:16:50PM -0600, Li Yang wrote: > On Tue, Mar 8, 2016 at 9:40 PM, Bjorn Andersson > wrote: > > On Tue, Mar 8, 2016 at 11:52 AM, Li Yang wrote: > >> On Wed, Mar 2, 2016 at 4:59 PM, Li Yang wrote: > >>> On Mon, Feb 22, 2016 at 4:07 PM, Bjorn Andersson > >>> wrote: > On Mon 22 Feb 02:03 PST 2016, Srinivas Kandagatla wrote: > > > > > > > On 22/02/16 05:32, Bjorn Andersson wrote: > > >On certain platforms (e.g. ARM64) the dma_ops needs to be explicitly > > >set > > >to be able to do DMA allocations, so use the of_dma_configure() helper > > >to populate the dma properties and assign an appropriate dma_ops. > > [..] > > None of the drivers call of_dma_configure() explicitly, which makes me > > feel > > that we are doing something wrong. TBH, this should be handled in more > > generic way rather than driver like this having an explicit call to > > of_dma_configure(). > > > > I agree, trying to figure out if it should be inherited or something. > >>> > >>> I also agree. We need address it in a more generic way. I did a > >>> search for platform_device_add()/platform_device_register() in the > >>> kernel source code. I found a lot of them and many could be also > >>> doing DMA. Looks like it is still too early to assume every device is > >>> already getting dma_ops set through bus probe. Otherwise, many > >>> drivers are potentially broken by this assumption. > >> > >> Any further comment on this topic? I added the linux-arm mailing list > >> which was missing from previous discussion. > >> > > > > I had the chance to go through this with Arnd and the verdict is that > > devices not described in DT should not do DMA (or allocate buffers for > > doing DMA). > > > > So I believe the solution is to fall back on Peter's description; the > > chipidea driver is the core driver and the Qualcomm code should just > > be a platform layer. > > > > My suggestion is that we turn the chipidea core into a set of APIs > > that can called by the platform specific pieces. That way we will have > > the chipidea core be the device described in the DT. > > But like I said, this problem is not just existing for chipidea > driver. We already found that the dwc3 driver is also suffering from > the same issue. I don't know how many other drivers are impacted by > this change, but I suspect there will be some. A grep of > platform_device_add() in driver/ directory returns many possible > drivers to be impacted. As far as I know, the > drivers/net/ethernet/freescale/fman/mac.c is registering child > ethernet devices that definitely will do dma. If you want to do this > kind of rework to all these drivers, it will be a really big effort. > +1 Yes, I think this DMA things should be covered by driver core too. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/2] usb: host: xhci: add a new quirk and fix an issue for R-Car Gen2/3
Yoshihiro Shimoda writes: > [ text/plain ] > This patch set is based on the latest usb.git / usb-next branch. > (commit id = ce53bfc4374cada8b645765e2b4ad5831e760932) > > Changes from v2: > - Change the quirk name to "XHCI_NO_64BIT_SUPPORT". > - Add comments for R-Car Gen2/3 (like a patch v1) in xhci-plat.c. > > Changes from v1: > - Add a new quirk "XHCI_CLEAR_AC64" for using it on some PCIe card. >http://thread.gmane.org/gmane.linux.kernel.renesas-soc/858/focus=1694 > > Yoshihiro Shimoda (2): > usb: host: xhci: add a new quirk XHCI_NO_64BIT_SUPPORT > usb: host: xhci-plat: fix cannot work if R-Car Gen2/3 run on above 4GB > phys for your entire series: Reviewed-by: Felipe Balbi -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: xhci: merge xhci_queue_bulk_tx and queue_bulk_sg_tx functions
Have you tested that bulk transfers still work as they should? Yes, I have tested it with EJ168 USB 3.0 Host Controller and USB 3.0 storage device and didn't find any problems. -- 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: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537
On Fri, 2016-03-11 at 11:54 -0500, Alan Stern wrote: > Perhaps I didn't do a good job of addressing the real underlying > problem. That's why I asked if this was the right approach. > > In Daniel's case, usbhid_start() never got called. Perhaps that's the > real problem? Anyway, this meant that usbhid->urbin never got set, and > consequently hid_start_in() returned an error when it was called by > hid_post_reset(). > > Does it make sense for usbhid_start() not to be called? If it depends on other modules, we can hardly guarantee it being called. > It stands out that hid_resume() checks the HID_STARTED bit before > calling hid_start_in() but hid_post_reset() doesn't. That is I think the assumption is that resets are caused by the operation of the HID interface. In practice this is almost invariably true, but strictly speaking both methods must check the flag. > inconsistent; the check should be done in both places or in neither, > but I don't know which. Is it possible for one of usbhid's devices to > be suspended or reset before usbhid_start() or after usbhid_stop()? If > it is then both places need to check -- my patch adds the missing > check. > > Finally, isn't it possible for an HID device not to have an > interrupt-IN endpoint? In which case usbhid->urbin will always be That would violate chapter 4.4 of the HID spec. > NULL, so hid_start_in() will always return an error unless it > specifically checks -- and my patch adds such a check. > > As you can see, it's a complicated and confusing situation. Maybe > you'd like to explore it in greater depth with Daniel to understand it > better. It seems to me that hid_resume_common() needs to be split into three parts a) doing the stuff for pending resets b) conditionally restarting IO c) reset the flag Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: input: powermate: fix oops with malicious USB descriptors
The powermate driver expects at least one valid USB endpoint in its probe function. If given malicious descriptors that specify 0 for the number of endpoints, it will crash. Validate the number of endpoints on the interface before using them. The full report for this issue can be found here: http://seclists.org/bugtraq/2016/Mar/85 Reported-by: Ralf Spenneberg Cc: stable Signed-off-by: Josh Boyer --- drivers/input/misc/powermate.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/input/misc/powermate.c b/drivers/input/misc/powermate.c index 63b539d3daba..84909a12ff36 100644 --- a/drivers/input/misc/powermate.c +++ b/drivers/input/misc/powermate.c @@ -307,6 +307,9 @@ static int powermate_probe(struct usb_interface *intf, const struct usb_device_i int error = -ENOMEM; interface = intf->cur_altsetting; + if (interface->desc.bNumEndpoints < 1) + return -EINVAL; + endpoint = &interface->endpoint[0].desc; if (!usb_endpoint_is_int_in(endpoint)) return -EIO; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Freezable workqueue blocks non-freezable workqueue during the system resume process
On Mon, 14 Mar 2016, Jan Kara wrote: > On Fri 11-03-16 12:56:10, Tejun Heo wrote: > > Hello, Jan. > > > > On Thu, Mar 03, 2016 at 10:33:10AM +0100, Jan Kara wrote: > > > > Ugh... that's nasty. I wonder whether the right thing to do is making > > > > writeback workers non-freezable. IOs are supposed to be blocked from > > > > lower layer anyway. Jan, what do you think? > > > > > > Well no, at least currently IO is not blocked in lower layers AFAIK - for > > > that you'd need to freeze block devices & filesystems and there are issues > > > > At least libata does and I think SCSI does too, but yeah, there > > probably are drivers which depend on block layer blocking IOs, which > > btw is a pretty fragile way to go about as upper layers might not be > > the only source of activities. > > > > > with that (Jiri Kosina was the last one which was trying to make this work > > > AFAIR). And I think you need to stop writeback (and generally any IO) to > > > be > > > generated so that it doesn't interact in a strange way with device drivers > > > being frozen. So IMO until suspend freezes filesystems & devices properly > > > you have to freeze writeback workqueue. What do you mean by "freezes ... devices"? Only a piece of code can be frozen -- not a device. The kernel does suspend device drivers; that is, it invokes their suspend callbacks. But it doesn't "freeze" them in any sense. Once a driver has been suspended, it assumes it won't receive any I/O requests until it has been resumed. Therefore the kernel first has to prevent all the upper layers from generating such requests and/or sending them to the low-level drivers. > > I still think the right thing to do is plugging that block layer or > > low level drivers. It's like we're trying to plug multiple sources > > when we can plug the point where they come together anyway. > > I agree that freezing writeback workers is a workaround for real issues at > best and ideally we shouldn't have to do that. But at least for now I had > the impression that it is needed for suspend to work reasonably reliably. The design is not to plug low-level drivers, but instead to prevent them from receiving any requests by plugging or freezing high-level code. It's pretty clear that we don't want to have ongoing I/O during a system suspend, right? And that means the I/O has to be prevented (or "plugged", if you prefer) somewhere -- either at an upper layer or at a lower layer. There was a choice to be made, and the decision was to do it at an upper layer. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: iowarrior: fix oops with malicious USB descriptors
The iowarrior driver expects at least one valid endpoint. If given
malicious descriptors that specify 0 for the number of endpoints,
it will crash in the probe function. Ensure there is at least
one endpoint on the interface before using it.
The full report of this issue can be found here:
http://seclists.org/bugtraq/2016/Mar/87
Reported-by: Ralf Spenneberg
Cc: stable
Signed-off-by: Josh Boyer
---
drivers/usb/misc/iowarrior.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c
index c6bfd13f6c92..1950e87b4219 100644
--- a/drivers/usb/misc/iowarrior.c
+++ b/drivers/usb/misc/iowarrior.c
@@ -787,6 +787,12 @@ static int iowarrior_probe(struct usb_interface *interface,
iface_desc = interface->cur_altsetting;
dev->product_id = le16_to_cpu(udev->descriptor.idProduct);
+ if (iface_desc->desc.bNumEndpoints < 1) {
+ dev_err(&interface->dev, "Invalid number of endpoints\n");
+ retval = -EINVAL;
+ goto error;
+ }
+
/* set up the endpoint information */
for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
endpoint = &iface_desc->endpoint[i].desc;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFT] digi_acceleport: do sanity checking for the number of ports
The driver can be crashed with devices that expose crafted
descriptors with too few endpoints.
See:
http://seclists.org/bugtraq/2016/Mar/61
Signed-off-by: Oliver Neukum
---
drivers/usb/serial/digi_acceleport.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/serial/digi_acceleport.c
b/drivers/usb/serial/digi_acceleport.c
index 12b0e67..c4d4d45 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -1260,6 +1260,11 @@ static int digi_startup(struct usb_serial *serial)
spin_lock_init(&serial_priv->ds_serial_lock);
serial_priv->ds_oob_port_num = serial->type->num_ports;
+ if (!(serial_priv->ds_oob_port_num == 2 && serial->type ==
&digi_acceleport_2_device)
+ && !(serial_priv->ds_oob_port_num == 4 && serial->type ==
&digi_acceleport_4_device)) {
+ kfree(serial_priv);
+ return -EINVAL;
+ }
serial_priv->ds_oob_port = serial->port[serial_priv->ds_oob_port_num];
ret = digi_port_init(serial_priv->ds_oob_port,
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFT] sanity checking in digiport driver
This addresses the DoS issue with respect to checking the number of ports Ralf reported here: http://seclists.org/bugtraq/2016/Mar/61 Could someone with the requisite hardware please test? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537
On Mon, 14 Mar 2016, Oliver Neukum wrote: > On Fri, 2016-03-11 at 11:54 -0500, Alan Stern wrote: > > > Perhaps I didn't do a good job of addressing the real underlying > > problem. That's why I asked if this was the right approach. > > > > In Daniel's case, usbhid_start() never got called. Perhaps that's the > > real problem? Anyway, this meant that usbhid->urbin never got set, and > > consequently hid_start_in() returned an error when it was called by > > hid_post_reset(). > > > > Does it make sense for usbhid_start() not to be called? > > If it depends on other modules, we can hardly guarantee it > being called. Okay. > > It stands out that hid_resume() checks the HID_STARTED bit before > > calling hid_start_in() but hid_post_reset() doesn't. That is > > I think the assumption is that resets are caused by the operation > of the HID interface. In practice this is almost invariably true, Not at all. For one thing, a reset may be started by a driver for another interface on the same device. For another, a reset-resume can occur at any time, if the device is attached through a host controller that loses power during suspend (which is what happened to Daniel). > but strictly speaking both methods must check the flag. So that does need to be added. > It seems to me that hid_resume_common() needs to be split into three > parts > > a) doing the stuff for pending resets > b) conditionally restarting IO > c) reset the flag Daniel's problem involved hid_post_reset(), not hid_resume_common(). Is that what you meant? Maybe I'm wrong, but it looks like hid_post_reset() just needs to check whether I/O should be started before trying to restart it. (It also looks like hid_resume() needs to clear the HID_SUSPENDED flag even when HID_STARTED isn't set.) 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: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537
On Mon, 2016-03-14 at 11:03 -0400, Alan Stern wrote: > On Mon, 14 Mar 2016, Oliver Neukum wrote: > > It seems to me that hid_resume_common() needs to be split into three > > parts > > > > a) doing the stuff for pending resets > > b) conditionally restarting IO > > c) reset the flag > > Daniel's problem involved hid_post_reset(), not hid_resume_common(). > Is that what you meant? Maybe I'm wrong, but it looks like > hid_post_reset() just needs to check whether I/O should be started > before trying to restart it. Yes. That means that all that hid_post_reset() does before calling hid_start_in() must be left unchanged and we can put the change into a common code path. > (It also looks like hid_resume() needs to clear the HID_SUSPENDED flag > even when HID_STARTED isn't set.) Exactly. Thus hid_resume_common() needs to be split. The code paths for reset_resume(), post_reset() and resume() diverge too much. You will see that they can be easily be reused if you split them into those three parts. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Chipidea USB driver on Linux 3.14?
Hi, We're using Linux 3.14.60 and we'd like to use the Chipidea USB driver that comes with it. We enabled USB support as "built-in" (ie: not as kernel module): CONFIG_USB=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_OHCI_HCD=y CONFIG_USB_STORAGE=y CONFIG_USB_CHIPIDEA=y CONFIG_USB_CHIPIDEA_HOST=y CONFIG_PHY_TANGO_USB=y However, after putting logs on the 'probe' functions in drivers/usb/chipidea/*.c they are not being called. NOTE: we are not using DeviceTree. Questions: 1) I would have thought that a "built-in" module would always be 'loaded' (sort of 'insmod -f'), is that right? 2) It appears that previously driver code was #included into drivers/usb/host/ehci-hcd.c is that still the recommended practice on Kernel 3.14? 3) What would be the recommended way of making this driver load and work? (without using DeviceTree) 4) Could somebody confirm the status of OTG support on 3.14? (there's a TODO about it on drivers/usb/chipidea/core.c) Thanks in advance, Sebastian -- 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: NEC uPD720200 xHCI Controller dies when Runtime PM enabled
On 2016-03-14 10:06, Mathias Nyman wrote: > On 13.03.2016 11:16, Mike Murdoch wrote: >> >> >> On 2016-03-01 16:32, Mathias Nyman wrote: >>> On 18.02.2016 18:34, Mike Murdoch wrote: On 2016-02-18 16:12, Mathias Nyman wrote: > On 16.02.2016 23:58, main.ha...@googlemail.com wrote: >> >> >> On 2016-02-08 15:31, Mathias Nyman wrote: >>> Hi >>> >>> On 06.02.2016 19:08, Mike Murdoch wrote: Bug ID: 111251 Hello, I have a NEC uPD720200 USB3.0 controller in a Thinkpad W520 laptop on kernel 4.4.1-gentoo. 0e:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host Controller (rev 04) (prog-if 30 [XHCI]) Subsystem: Lenovo uPD720200 USB 3.0 Host Controller When runtime power control for this controller is disabled (/sys/bus/pci/devices/:0e:00.0/power/control = on), the controller works fine and reaches over 120MB/s transfer rates. When runtime power control for this controller is enabled (/sys/bus/pci/devices/:0e:00.0/power/control = auto), two effects can be observed: - Transfer rates are much lower at around 30MB/s - During transfers, the controller dies after a couple of seconds: At this point, a reboot is required to reactivate the controller, unloading and reloading the xhci_* modules does not work. >>> >>> >>> ... >>> >>> I did some more digging, there are a few things that need to be >>> addressed: >>> 1. We should resume USB3 bus before USB2 bus to let devices enumerate >>> as USB3 better, >>> this gives them more time to finish the link training. >>> >>> 2. After resuming xhci we don't see any port changes immediately, hub >>> thinks nothing >>> happended and stops polling the ports, hub will suspend again -> >>> xhci will try to >>> suspend. >>> 3. Roothubs will autosuspend immediately after autoresume, >>> (autosuspend timeout = 0) >>> This could be a reason why we see the "xhci_suspend" entry in the >>> log. We either >>> need to increase the autosuspend timeout, or prevent suspend if we >>> can see the pending >>> event in a xhci status register. >>> >>> inserting usb3 storage device >>> Feb 16 20:03:33 xhci_hcd :0e:00.0: // Setting command ring address >>> to 0xe001 >>> Feb 16 20:03:33 xhci_hcd :0e:00.0: xhci_resume: starting port >>> polling. >>> Feb 16 20:03:33 xhci_hcd :0e:00.0: xhci_hub_status_data: stopping >>> port polling. >>> Feb 16 20:03:33 xhci_hcd :0e:00.0: xhci_suspend: stopping port >>> polling. >>> >>> I got a few patches, attached. They both partially try to fix the >>> issue, and add more logging. >>> Same changes can be found in a topic branch from in: >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git >>> bug_usb3_enum_rtresume >>> >>> Any chance to try them out? >>> >>> -Mathias >> >> Hello, >> >> I've come around to testing these patches. I applied them all at once >> (did you want me to test them individually?) and they appear to fix this >> issue completely! Full speed and no dead controllers.Do you need any >> further logs? >> > > That's good news. > > Can I add your "Tested-by:" tag to two of the patches? > I'll send them as fixes after rc1 is out. > > No more logs needed as it works, I'll send the third additional debug > info > patch to usb-next later. It will be useful for future debugging > > Thanks > Mathias > > > > for further debugging this case > The third patch is just additional debug info and useful for future > debugging (or if those > > Hello, yes, feel free to add the tag. Thanks for everything! - Mike -- 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: gadget: f_midi: Fixed a bug when buflen was smaller than wMaxPacketSize
Hi Michal,
On 11/03/16 23:07, Michal Nazarewicz wrote:
> On Wed, Mar 09 2016, Felipe F. Tonello wrote:
>> buflen by default (256) is smaller than wMaxPacketSize (512) in high-speed
>> devices.
>>
>> That caused the OUT endpoint to freeze if the host send any data packet of
>> length greater than 256 bytes.
>>
>> This is an example dump of what happended on that enpoint:
>> HOST: [DATA][Length=260][...]
>> DEVICE: [NAK]
>> HOST: [PING]
>> DEVICE: [NAK]
>> HOST: [PING]
>> DEVICE: [NAK]
>> ...
>> HOST: [PING]
>> DEVICE: [NAK]
>>
>> This patch fixes this problem by setting the minimum usb_request's buffer
>> size
>> for the OUT endpoint as its wMaxPacketSize.
>>
>> Signed-off-by: Felipe F. Tonello
>
> Acked-by: Michal Nazarewicz
>
> But see comment below:
>
>> ---
>> drivers/usb/gadget/function/f_midi.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_midi.c
>> b/drivers/usb/gadget/function/f_midi.c
>> index 8475e3dc82d4..826ba641f29d 100644
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -366,7 +366,9 @@ static int f_midi_set_alt(struct usb_function *f,
>> unsigned intf, unsigned alt)
>> /* allocate a bunch of read buffers and queue them all at once. */
>> for (i = 0; i < midi->qlen && err == 0; i++) {
>> struct usb_request *req =
>> -midi_alloc_ep_req(midi->out_ep, midi->buflen);
>> +midi_alloc_ep_req(midi->out_ep,
>> +max_t(unsigned, midi->buflen,
>> +bulk_out_desc.wMaxPacketSize));
>
> Or, just:
>
> + midi_alloc_ep_req(midi->out_ep,
> + bulk_out_desc.wMaxPacketSize);
>
> Packet cannot be greater than wMaxPacketSize so there is no need to
> allocate more (if buflen > wMaxPacketSize).
I didn't know that was a constraint. If so, I agree with you.
>
>> if (req == NULL)
>> return -ENOMEM;
>>
>
> I’m also wondering whether it would be beneficial to get rid of buflen
> all together and use wMaxPacketSie for in endpoints as well? Is that
> feasible?
Yes, we could just remove the buflen parameter.
The only scenario where I can see buflen been "useful" is if the user
wants to have a smaller buffer size for the OUT endpoint. Should we
support this case or not?
I can rework this patch for any case we agree on.
--
Felipe
0x92698E6A.asc
Description: application/pgp-keys
Re: [PATCH] USB: input: powermate: fix oops with malicious USB descriptors
On Mon, Mar 14, 2016 at 10:12:53AM -0400, Josh Boyer wrote: > The powermate driver expects at least one valid USB endpoint in its > probe function. If given malicious descriptors that specify 0 for > the number of endpoints, it will crash. Validate the number of > endpoints on the interface before using them. > > The full report for this issue can be found here: > http://seclists.org/bugtraq/2016/Mar/85 > > Reported-by: Ralf Spenneberg > Cc: stable > Signed-off-by: Josh Boyer > --- > drivers/input/misc/powermate.c | 3 +++ > 1 file changed, 3 insertions(+) I'll queue these up after 4.6-rc1 is out as the merge window is closed right now, but we might want to think about a better way to handle this type of thing in the USB core. A way to keep from having to add checks like this for every single driver, when the driver shouldn't even really have their probe function called unless their expected endpoints are going to be there. I'll think about that over the next few weeks... 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] USB: input: powermate: fix oops with malicious USB descriptors
On Mon, Mar 14, 2016 at 09:15:48AM -0700, Greg Kroah-Hartman wrote: > On Mon, Mar 14, 2016 at 10:12:53AM -0400, Josh Boyer wrote: > > The powermate driver expects at least one valid USB endpoint in its > > probe function. If given malicious descriptors that specify 0 for > > the number of endpoints, it will crash. Validate the number of > > endpoints on the interface before using them. > > > > The full report for this issue can be found here: > > http://seclists.org/bugtraq/2016/Mar/85 > > > > Reported-by: Ralf Spenneberg > > Cc: stable > > Signed-off-by: Josh Boyer > > --- > > drivers/input/misc/powermate.c | 3 +++ > > 1 file changed, 3 insertions(+) > > I'll queue these up after 4.6-rc1 is out as the merge window is closed > right now, but we might want to think about a better way to handle this > type of thing in the USB core. A way to keep from having to add checks I do not see any reason in holding it until after rc1, applied. > like this for every single driver, when the driver shouldn't even really > have their probe function called unless their expected endpoints are > going to be there. I had a patch where driver could declare minimal amount of endpoints it expects to find, but you mentioned we need something more flexible... Thanks. -- Dmitry -- 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: input: powermate: fix oops with malicious USB descriptors
On Mon, Mar 14, 2016 at 12:15 PM, Greg Kroah-Hartman wrote: > On Mon, Mar 14, 2016 at 10:12:53AM -0400, Josh Boyer wrote: >> The powermate driver expects at least one valid USB endpoint in its >> probe function. If given malicious descriptors that specify 0 for >> the number of endpoints, it will crash. Validate the number of >> endpoints on the interface before using them. >> >> The full report for this issue can be found here: >> http://seclists.org/bugtraq/2016/Mar/85 >> >> Reported-by: Ralf Spenneberg >> Cc: stable >> Signed-off-by: Josh Boyer >> --- >> drivers/input/misc/powermate.c | 3 +++ >> 1 file changed, 3 insertions(+) > > I'll queue these up after 4.6-rc1 is out as the merge window is closed > right now, but we might want to think about a better way to handle this > type of thing in the USB core. A way to keep from having to add checks > like this for every single driver, when the driver shouldn't even really > have their probe function called unless their expected endpoints are > going to be there. I thought this discussion came up a while ago, when something very similar was fixed in the whiteheat driver (commit cbb4be652d374). I can't find the thread at the moment, but I thought someone said this had to be per-driver for some reason. I'm more than happy to have a core subsystem fix if it's possible. > I'll think about that over the next few weeks... I have something around 8 drivers with issues like this. I think Oliver (now CC'd) is working from the same set of bugs. Should we hold off on submitting individual fixes until later, or should we go ahead and submit them? josh -- 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: input: powermate: fix oops with malicious USB descriptors
On Mon, Mar 14, 2016 at 9:46 AM, Josh Boyer wrote: > On Mon, Mar 14, 2016 at 12:15 PM, Greg Kroah-Hartman > wrote: >> On Mon, Mar 14, 2016 at 10:12:53AM -0400, Josh Boyer wrote: >>> The powermate driver expects at least one valid USB endpoint in its >>> probe function. If given malicious descriptors that specify 0 for >>> the number of endpoints, it will crash. Validate the number of >>> endpoints on the interface before using them. >>> >>> The full report for this issue can be found here: >>> http://seclists.org/bugtraq/2016/Mar/85 >>> >>> Reported-by: Ralf Spenneberg >>> Cc: stable >>> Signed-off-by: Josh Boyer >>> --- >>> drivers/input/misc/powermate.c | 3 +++ >>> 1 file changed, 3 insertions(+) >> >> I'll queue these up after 4.6-rc1 is out as the merge window is closed >> right now, but we might want to think about a better way to handle this >> type of thing in the USB core. A way to keep from having to add checks >> like this for every single driver, when the driver shouldn't even really >> have their probe function called unless their expected endpoints are >> going to be there. > > I thought this discussion came up a while ago, when something very > similar was fixed in the whiteheat driver (commit cbb4be652d374). I > can't find the thread at the moment, but I thought someone said this > had to be per-driver for some reason. I'm more than happy to have a > core subsystem fix if it's possible. > >> I'll think about that over the next few weeks... > > I have something around 8 drivers with issues like this. I think > Oliver (now CC'd) is working from the same set of bugs. Should we > hold off on submitting individual fixes until later, or should we go > ahead and submit them? I'll take input bits, there is no need to keep kernel oopsing while we are working on a more general solution. Thanks. -- Dmitry -- 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: input: powermate: fix oops with malicious USB descriptors
On Mon, Mar 14, 2016 at 12:46:26PM -0400, Josh Boyer wrote: > On Mon, Mar 14, 2016 at 12:15 PM, Greg Kroah-Hartman > wrote: > > On Mon, Mar 14, 2016 at 10:12:53AM -0400, Josh Boyer wrote: > >> The powermate driver expects at least one valid USB endpoint in its > >> probe function. If given malicious descriptors that specify 0 for > >> the number of endpoints, it will crash. Validate the number of > >> endpoints on the interface before using them. > >> > >> The full report for this issue can be found here: > >> http://seclists.org/bugtraq/2016/Mar/85 > >> > >> Reported-by: Ralf Spenneberg > >> Cc: stable > >> Signed-off-by: Josh Boyer > >> --- > >> drivers/input/misc/powermate.c | 3 +++ > >> 1 file changed, 3 insertions(+) > > > > I'll queue these up after 4.6-rc1 is out as the merge window is closed > > right now, but we might want to think about a better way to handle this > > type of thing in the USB core. A way to keep from having to add checks > > like this for every single driver, when the driver shouldn't even really > > have their probe function called unless their expected endpoints are > > going to be there. > > I thought this discussion came up a while ago, when something very > similar was fixed in the whiteheat driver (commit cbb4be652d374). I > can't find the thread at the moment, but I thought someone said this > had to be per-driver for some reason. I'm more than happy to have a > core subsystem fix if it's possible. > > > I'll think about that over the next few weeks... > > I have something around 8 drivers with issues like this. I think > Oliver (now CC'd) is working from the same set of bugs. Should we > hold off on submitting individual fixes until later, or should we go > ahead and submit them? Please submit them, that will give us a framework to be able to figure out the specifics of what needs to be changed. 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
[PATCH] usb: dwc2: gadget: avoid null dereference on incomplete transfer
Setting up a gadget with the uac2 function results in:
Unable to handle kernel NULL pointer dereference at virtual address 0058
...
PC is at dwc2_hsotg_irq+0x7f0/0x908
LR is at dwc2_hsotg_irq+0x4c/0x908
Backtrace:
[] (dwc2_hsotg_irq) from []
(handle_irq_event_percpu+0x130/0x3ec)
[] (handle_irq_event_percpu) from []
(handle_irq_event+0x48/0x6c)
In all other loops we already skip endpoints that are null, so do so
here as well.
Signed-off-by: John Keeping
---
drivers/usb/dwc2/gadget.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0abf73c..df43ec0 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2606,7 +2606,9 @@ irq_retry:
for (idx = 1; idx < hsotg->num_of_eps; idx++) {
hs_ep = hsotg->eps_in[idx];
- if (!hs_ep->isochronous || hs_ep->has_correct_parity)
+ if (!hs_ep ||
+ !hs_ep->isochronous ||
+ hs_ep->has_correct_parity)
continue;
epctl_reg = DIEPCTL(idx);
@@ -2623,7 +2625,9 @@ irq_retry:
for (idx = 1; idx < hsotg->num_of_eps; idx++) {
hs_ep = hsotg->eps_out[idx];
- if (!hs_ep->isochronous || hs_ep->has_correct_parity)
+ if (!hs_ep ||
+ !hs_ep->isochronous ||
+ hs_ep->has_correct_parity)
continue;
epctl_reg = DOEPCTL(idx);
--
2.7.0.226.gfe986fe
--
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
The mac80211 softmac driver subsystem and handling of monitor interfaces
Hi, For various reasons I have been looking into the handling of virtual monitor interfaces in drivers that link with the mac80211 module. It is my understanding that monitor interfaces per se. are not "passed down" to the driver module, and that driver modules should handle the IEEE80211_CONF_CHANGE_MONITOR flag passed down by the config callback whenever the set of virtual interfaces changes between having zero and one monitor member. Firstly is this assumption correct? Looking at the drivers in the current mainline kernel that link the mac80211 modules only a small minority of them hook this event. They are :- ath/ath10k ath/ath9k (both ath9k and ath9k_htc) cw1200 brcm80211/brcmsmac All the others seem to be doing things with NL80211_IFTYPE_MONITOR in the vif structure. Is this something to do with the somewhat opaque comment in mac80211.h? * @IEEE80211_CONF_MONITOR: there's a monitor interface present -- use this *to determine for example whether to calculate timestamps for packets *or not, do not use instead of filter flags! If my assumptions are true I would expect drivers to do whatever private interaction with the hardware to enable monitoring at this point. For example setting up the right frame headers. I also assume that hw filters are handled separately so that different monitor vifs can have different flags. Please feel free to tear this to shreds! I am on a steep learning curve on this. I have read most of the docs I could find and studied the code, but please let me have any suggestions you have for further research. Thanks Roger -- 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: The mac80211 softmac driver subsystem and handling of monitor interfaces
On Mon, Mar 14, 2016 at 08:47:34PM +, Roger James wrote: > Hi, > > For various reasons I have been looking into the handling of virtual monitor > interfaces in drivers that link with the mac80211 module. Wonderful, but try sending this to the netdev@ mailing list, not linux-usb :) good luck! greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: cdc-acm: add support for Sagem Monetel ELC930
On Sat, Mar 12, 2016 at 04:27:29PM -0800, Greg KH wrote:
> On Sat, Mar 12, 2016 at 11:44:51PM +, Nicolas Saenz Julienne wrote:
> > Signed-off-by: Nicolas Saenz Julienne
> > ---
> > drivers/usb/class/cdc-acm.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> > index fa4e239..9831607 100644
> > --- a/drivers/usb/class/cdc-acm.c
> > +++ b/drivers/usb/class/cdc-acm.c
> > @@ -1681,6 +1681,9 @@ static const struct usb_device_id acm_ids[] = {
> > { USB_DEVICE(0x079b, 0x000f), /* BT On-Air USB MODEM */
> > .driver_info = NO_UNION_NORMAL, /* has no union descriptor */
> > },
> > + { USB_DEVICE(0x079b, 0x0088), /* SAGEM Monetel ELC930 */
> > + .driver_info = NO_UNION_NORMAL, /* has no union descriptor */
> > + },
>
> Why is this needed? Does the descriptors not properly set the class
> device?
>
> thanks,
>
> greg k-h
Hi,
the device is missing all the ACM specific "extra" info on it's interface
descriptor. Which seems be triggering the "Zero length descriptor references"
error during the probe function. Adding the NO_UNION_NORMAL quirk seems to solve
the issue.
As for the class device, it seems to be ok, CDC (0x2).
Nicolas
--
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] watchdog: add driver for StreamLabs USB watchdog device
Hi Guenter,
On Thu, Mar 10, 2016 at 3:54 AM, Guenter Roeck wrote:
> On 03/09/2016 06:29 PM, Alexey Klimov wrote:
>>
>> This patch creates new driver that supports StreamLabs usb watchdog
>> device. This device plugs into 9-pin usb header and connects to
>> reset pin and reset button on common PC.
>>
>> USB commands used to communicate with device were reverse
>> engineered using usbmon.
>>
>> Signed-off-by: Alexey Klimov
>> ---
>> drivers/watchdog/Kconfig | 15 ++
>> drivers/watchdog/Makefile | 1 +
>> drivers/watchdog/streamlabs_wdt.c | 370
>> ++
>> 3 files changed, 386 insertions(+)
>> create mode 100644 drivers/watchdog/streamlabs_wdt.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 80825a7..95d8f72 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -1705,4 +1705,19 @@ config USBPCWATCHDOG
>>
>> Most people will say N.
>>
>> +config USB_STREAMLABS_WATCHDOG
>> + tristate "StreamLabs USB watchdog driver"
>> + depends on USB
>> + ---help---
>> + This is the driver for the USB Watchdog dongle from StreamLabs.
>> + If you correctly connect reset pins to motherboard Reset pin and
>> + to Reset button then this device will simply watch your kernel
>> to make
>> + sure it doesn't freeze, and if it does, it reboots your computer
>> + after a certain amount of time.
>> +
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called streamlabs_wdt.
>> +
>> + Most people will say N. Say yes or M if you want to use such usb
>> device.
>> endif # WATCHDOG
>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> index f6a6a38..d54fd31 100644
>> --- a/drivers/watchdog/Makefile
>> +++ b/drivers/watchdog/Makefile
>> @@ -25,6 +25,7 @@ obj-$(CONFIG_WDTPCI) += wdt_pci.o
>>
>> # USB-based Watchdog Cards
>> obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
>> +obj-$(CONFIG_USB_STREAMLABS_WATCHDOG) += streamlabs_wdt.o
>>
>> # ALPHA Architecture
>>
>> diff --git a/drivers/watchdog/streamlabs_wdt.c
>> b/drivers/watchdog/streamlabs_wdt.c
>> new file mode 100644
>> index 000..031dbc35
>> --- /dev/null
>> +++ b/drivers/watchdog/streamlabs_wdt.c
>> @@ -0,0 +1,370 @@
>> +/*
>> + * StreamLabs USB Watchdog driver
>> + *
>> + * Copyright (c) 2016 Alexey Klimov
>> + *
>> + * This program is free software; you may redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +/*
>> + * USB Watchdog device from Streamlabs
>> + * http://www.stream-labs.com/products/devices/watchdog/
>> + *
>> + * USB commands have been reverse engineered using usbmon.
>> + */
>> +
>> +#define DRIVER_AUTHOR "Alexey Klimov "
>> +#define DRIVER_DESC "StreamLabs USB watchdog driver"
>> +#define DRIVER_NAME "usb_streamlabs_wdt"
>> +
>> +MODULE_AUTHOR(DRIVER_AUTHOR);
>> +MODULE_DESCRIPTION(DRIVER_DESC);
>> +MODULE_LICENSE("GPL");
>> +
>> +#define USB_STREAMLABS_WATCHDOG_VENDOR 0x13c0
>> +#define USB_STREAMLABS_WATCHDOG_PRODUCT0x0011
>> +
>> +/* one buffer is used for communication, however transmitted message is
>> only
>> + * 32 bytes long */
>
>
> /*
> * Please use proper multi-line comments throughout.
>
> */
Ok, will fix them all.
>> +#define BUFFER_TRANSFER_LENGTH 32
>> +#define BUFFER_LENGTH 64
>> +#define USB_TIMEOUT350
>> +
>> +#define STREAMLABS_CMD_START 0
>> +#define STREAMLABS_CMD_STOP1
>> +
>> +#define STREAMLABS_WDT_MIN_TIMEOUT 1
>> +#define STREAMLABS_WDT_MAX_TIMEOUT 46
>> +
>> +struct streamlabs_wdt {
>> + struct watchdog_device wdt_dev;
>> + struct usb_device *usbdev;
>> + struct usb_interface *intf;
>> +
>> + struct kref kref;
>> + struct mutex lock;
>> + u8 *buffer;
>> +};
>> +
>> +static bool nowayout = WATCHDOG_NOWAYOUT;
>> +
>> +static int usb_streamlabs_wdt_validate_response(u8 *buf)
>> +{
>> + /* If watchdog device understood the command it will acknowledge
>> +* with values 1,2,3,4 at indexes 10, 11, 12, 13 in response
>> message.
>> +*/
>> + if (buf[10] != 1 || buf[11] != 2 || buf[12] != 3 || buf[13] != 4)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>> +static int usb_streamlabs_wdt_command(struct watchdog_device *wdt_dev,
>> int cmd)
>> +{
>> + struct streamlabs_wdt *streamlabs_wdt =
>> w
Re: [PATCH] watchdog: add driver for StreamLabs USB watchdog device
Hi Alexey,
On 03/14/2016 06:02 PM, Alexey Klimov wrote:
Hi Guenter,
On Thu, Mar 10, 2016 at 3:54 AM, Guenter Roeck wrote:
On 03/09/2016 06:29 PM, Alexey Klimov wrote:
This patch creates new driver that supports StreamLabs usb watchdog
device. This device plugs into 9-pin usb header and connects to
reset pin and reset button on common PC.
USB commands used to communicate with device were reverse
engineered using usbmon.
Signed-off-by: Alexey Klimov
---
drivers/watchdog/Kconfig | 15 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/streamlabs_wdt.c | 370
++
3 files changed, 386 insertions(+)
create mode 100644 drivers/watchdog/streamlabs_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 80825a7..95d8f72 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1705,4 +1705,19 @@ config USBPCWATCHDOG
Most people will say N.
+config USB_STREAMLABS_WATCHDOG
+ tristate "StreamLabs USB watchdog driver"
+ depends on USB
+ ---help---
+ This is the driver for the USB Watchdog dongle from StreamLabs.
+ If you correctly connect reset pins to motherboard Reset pin and
+ to Reset button then this device will simply watch your kernel
to make
+ sure it doesn't freeze, and if it does, it reboots your computer
+ after a certain amount of time.
+
+
+ To compile this driver as a module, choose M here: the
+ module will be called streamlabs_wdt.
+
+ Most people will say N. Say yes or M if you want to use such usb
device.
endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index f6a6a38..d54fd31 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_WDTPCI) += wdt_pci.o
# USB-based Watchdog Cards
obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
+obj-$(CONFIG_USB_STREAMLABS_WATCHDOG) += streamlabs_wdt.o
# ALPHA Architecture
diff --git a/drivers/watchdog/streamlabs_wdt.c
b/drivers/watchdog/streamlabs_wdt.c
new file mode 100644
index 000..031dbc35
--- /dev/null
+++ b/drivers/watchdog/streamlabs_wdt.c
@@ -0,0 +1,370 @@
+/*
+ * StreamLabs USB Watchdog driver
+ *
+ * Copyright (c) 2016 Alexey Klimov
+ *
+ * This program is free software; you may redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/*
+ * USB Watchdog device from Streamlabs
+ * http://www.stream-labs.com/products/devices/watchdog/
+ *
+ * USB commands have been reverse engineered using usbmon.
+ */
+
+#define DRIVER_AUTHOR "Alexey Klimov "
+#define DRIVER_DESC "StreamLabs USB watchdog driver"
+#define DRIVER_NAME "usb_streamlabs_wdt"
+
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
+
+#define USB_STREAMLABS_WATCHDOG_VENDOR 0x13c0
+#define USB_STREAMLABS_WATCHDOG_PRODUCT0x0011
+
+/* one buffer is used for communication, however transmitted message is
only
+ * 32 bytes long */
/*
* Please use proper multi-line comments throughout.
*/
Ok, will fix them all.
+#define BUFFER_TRANSFER_LENGTH 32
+#define BUFFER_LENGTH 64
+#define USB_TIMEOUT350
+
+#define STREAMLABS_CMD_START 0
+#define STREAMLABS_CMD_STOP1
+
+#define STREAMLABS_WDT_MIN_TIMEOUT 1
+#define STREAMLABS_WDT_MAX_TIMEOUT 46
+
+struct streamlabs_wdt {
+ struct watchdog_device wdt_dev;
+ struct usb_device *usbdev;
+ struct usb_interface *intf;
+
+ struct kref kref;
+ struct mutex lock;
+ u8 *buffer;
+};
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+
+static int usb_streamlabs_wdt_validate_response(u8 *buf)
+{
+ /* If watchdog device understood the command it will acknowledge
+* with values 1,2,3,4 at indexes 10, 11, 12, 13 in response
message.
+*/
+ if (buf[10] != 1 || buf[11] != 2 || buf[12] != 3 || buf[13] != 4)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int usb_streamlabs_wdt_command(struct watchdog_device *wdt_dev,
int cmd)
+{
+ struct streamlabs_wdt *streamlabs_wdt =
watchdog_get_drvdata(wdt_dev);
+ int retval;
+ int size;
+ unsigned long timeout_msec;
+ int retry_counter = 10; /* how many times to re-send stop
cmd */
+
+ mutex_lock(&streamlabs_wdt->lock);
+
+ timeout_msec = wdt_dev->timeout * MSEC_PER_SEC;
+
+ /* Prepare message that will be sent to device.
+* This bu
Re: [PATCH 1/5 v10] dt/bindings: Add binding for the DA8xx MUSB driver
On 03/14/2016 02:00 AM, Felipe Balbi wrote: Hi, Petr Kulhavy writes: + - ti,usb2-phy-refclock-hz : Integer. Frequency in Hz of the USB 2.0 PHY reference clock, + either provided by the internal PLL or an external source. + The supported values are: 12MHz, 13MHz, 19.2MHz, 20MHz, 24MHz, 26MHz, 38.4MHz, 40MHz, 48MHz. why isn't this an actual clock phandle ? Driver could, then, use clk_get_rate() to figure this one out. You could just use a fixed clock here to satisfy the clock API. I've actually been working on getting the da8xx ohci driver working with device tree. It has similar clock issues (in fact, the ohci clock is a child of the musb clock). Petr, give me a day or two and will have post some patches. It will have a clock that can be used here for clk_get_rate(). -- 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
system awareness of endpoints
Greg wrote: > lsusb is looking at the descriptor that the device originally gave the > operating system and parsing that. You can't dynamically change your > descriptors without resetting your device and having the OS restart it. Oliver wrote: > The firmware uploader may need to do a reset though. Lars wrote > Your device should do a reset after firmware is uploaded and it should > also change its usb id to reflect that it does not have the same > function(s) as before. The kernel will see the disappearance and > reappearance of the device on the same host port and will rescan the > device descriptors. Okay. This is what I was looking for. The question being: Does the system forever look at the original device descriptors or are there standard stock ways for the system to update these descriptors. The answer being: Yes, there are standard stock ways for the system to update these descriptors. The problem being: Changing the descriptors will be a real bother. On the other hand there are all sorts of nice things that could be done with 16 enpoints to play around with. Bruce -- 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
