Re: [RFC PATCH 1/4] usb: dwc3: add AMD NL support
Hi,
On Fri, Sep 26, 2014 at 12:53:21PM +0800, Huang Rui wrote:
> On Thu, Sep 25, 2014 at 09:53:55AM -0500, Felipe Balbi wrote:
> > On Thu, Sep 25, 2014 at 03:21:44PM +0800, Huang Rui wrote:
> > > Add PCI device ID of AMD NL.
> > >
> > > Signed-off-by: Huang Rui
> > > ---
> > > drivers/usb/dwc3/dwc3-pci.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> > > index 436fb08..cebbd01 100644
> > > --- a/drivers/usb/dwc3/dwc3-pci.c
> > > +++ b/drivers/usb/dwc3/dwc3-pci.c
> > > @@ -30,6 +30,7 @@
> > > #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3 0xabcd
> > > #define PCI_DEVICE_ID_INTEL_BYT 0x0f37
> > > #define PCI_DEVICE_ID_INTEL_MRFLD0x119e
> > > +#define PCI_DEVICE_ID_AMD_NL 0x7912
> > >
> > > struct dwc3_pci {
> > > struct device *dev;
> > > @@ -183,6 +184,7 @@ static const struct pci_device_id dwc3_pci_id_table[]
> > > = {
> > > },
> > > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT), },
> > > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRFLD), },
> > > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_NL), },
> >
> > this patch causes a build breakage.
> >
>
> That's strange, I tested it once again in my side based on your "next"
> branch and didn't find any build error. If I misunderstood your
> meaning, please correct me. :)
no, I just misread your patch. I'm sorry. I thought the PCI Device ID
was set to the quirk which was not defined here. I apologize, this patch
is 100% correct.
--
balbi
signature.asc
Description: Digital signature
Re: [RFC PATCH 1/4] usb: dwc3: add AMD NL support
Hi Felipe,
On Thu, Sep 25, 2014 at 09:53:55AM -0500, Felipe Balbi wrote:
> On Thu, Sep 25, 2014 at 03:21:44PM +0800, Huang Rui wrote:
> > Add PCI device ID of AMD NL.
> >
> > Signed-off-by: Huang Rui
> > ---
> > drivers/usb/dwc3/dwc3-pci.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> > index 436fb08..cebbd01 100644
> > --- a/drivers/usb/dwc3/dwc3-pci.c
> > +++ b/drivers/usb/dwc3/dwc3-pci.c
> > @@ -30,6 +30,7 @@
> > #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB30xabcd
> > #define PCI_DEVICE_ID_INTEL_BYT0x0f37
> > #define PCI_DEVICE_ID_INTEL_MRFLD 0x119e
> > +#define PCI_DEVICE_ID_AMD_NL 0x7912
> >
> > struct dwc3_pci {
> > struct device *dev;
> > @@ -183,6 +184,7 @@ static const struct pci_device_id dwc3_pci_id_table[] =
> > {
> > },
> > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT), },
> > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRFLD), },
> > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_NL), },
>
> this patch causes a build breakage.
>
That's strange, I tested it once again in my side based on your "next"
branch and didn't find any build error. If I misunderstood your
meaning, please correct me. :)
Thanks,
Rui
--
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 v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
Hi again,
On Thu, Sep 25, 2014 at 07:37:50PM -0500, Felipe Balbi wrote:
> On Fri, Sep 26, 2014 at 07:39:34AM +0800, Peter Chen wrote:
> > On Thu, Sep 25, 2014 at 09:15:53AM -0500, Felipe Balbi wrote:
> > > On Wed, Sep 24, 2014 at 02:23:38PM +0200, Arnd Bergmann wrote:
> > > > On Wednesday 24 September 2014 19:29:05 Peter Chen wrote:
> > > > >
> > > > > So, it is IP CORE LIB (you suggest) vs IP CORE Platform Driver
> > > > > (dwc3, musb, chipidea) you are talking about, right? Except for
> > > > > creating another platform driver as well as related DT node
> > > > > (optional),
> > > > > are there any advantages compared to current IP core driver structure?
> > > >
> > > > Having a library module usually requires less code, and is more
> > > > consistent with other drivers, which helps new developers understand
> > > > what the driver is doing.
> > >
> > > I beg to differ. You end-up having to pass function pointers through
> > > platform_data to handle differences which could be handled by the core
> > > IP.
> > >
> > > > The most important aspect though is the DT binding: once the structure
> > > > with separate kernel drivers leaks out into the DT format, you can't
> > > > easily change the driver any more, e.g. to make a property that was
> > > > introduced for one glue driver more general so it can get handled by
> > > > the common part. Having a single node lets us convert to the library
> > > > model later, so that would be a strong reason to keep the DT binding
> > > > simple, without child nodes.
> > > >
> > > > Following that logic, it turns into another reason for using the library
> > > > model to start with, because otherwise the child device does not have
> > > > any DT properties itself and has to rely on the parent device
> > > > properties,
> > > > which somewhat complicates the driver structure.
> > >
> > > this is bullcrap. Take a look at
> > > Documentation/devicetree/bindings/usb/dwc3.txt:
> > >
> > > synopsys DWC3 CORE
> > >
> > > DWC3- USB3 CONTROLLER
> > >
> > > Required properties:
> > > - compatible: must be "snps,dwc3"
> > > - reg : Address and length of the register set for the device
> > > - interrupts: Interrupts used by the dwc3 controller.
> > >
> > > Optional properties:
> > > - usb-phy : array of phandle for the PHY device. The first element
> > >in the array is expected to be a handle to the USB2/HS PHY and
> > >the second element is expected to be a handle to the USB3/SS PHY
> > > - phys: from the *Generic PHY* bindings
> > > - phy-names: from the *Generic PHY* bindings
> > > - tx-fifo-resize: determines if the FIFO *has* to be reallocated.
> > >
> > > This is usually a subnode to DWC3 glue to which it is connected.
> > >
> > > dwc3@4a03 {
> > > compatible = "snps,dwc3";
> > > reg = <0x4a03 0xcfff>;
> > > interrupts = <0 92 4>
> > > usb-phy = <&usb2_phy>, <&usb3,phy>;
> > > tx-fifo-resize;
> > > };
> > >
> > > This contains all the attributes it needs to work. In fact, this could
> > > be used without any glue.
> > >
> >
> > If you want to add "vbus-supply" core node to support ID switch, what's
> > the plan for that?
>
> send a patch ? Just make sure it's optional because not everybody needs
> a vbus-supply. Also, DRD will still take a few merge windows to become a
> reality. I don't want to merge something before considering it carefuly,
> otherwise we will having which is broken and doesn't work for everybody
> ;-)
>
> > Is it possible that the glue layer needs to access core register
> > (eg, portsc.phcd during suspend)? The wrapper IP may wait some signals
> > at core IP to change.
>
> why would a glue layer need to access registers from the core ? That
> sounds very odd. I haven't seen that and will, definitely, NACK such a
> patch :-)
>
> can you further describe why you think a glue layer might need to access
> core IP's registers ?
I just realised we're talking about chipidea here... in any case, it's
still valid to ask why would glue need to fiddle with core IP's
registers.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
HI,
On Fri, Sep 26, 2014 at 07:39:34AM +0800, Peter Chen wrote:
> On Thu, Sep 25, 2014 at 09:15:53AM -0500, Felipe Balbi wrote:
> > On Wed, Sep 24, 2014 at 02:23:38PM +0200, Arnd Bergmann wrote:
> > > On Wednesday 24 September 2014 19:29:05 Peter Chen wrote:
> > > >
> > > > So, it is IP CORE LIB (you suggest) vs IP CORE Platform Driver
> > > > (dwc3, musb, chipidea) you are talking about, right? Except for
> > > > creating another platform driver as well as related DT node (optional),
> > > > are there any advantages compared to current IP core driver structure?
> > >
> > > Having a library module usually requires less code, and is more
> > > consistent with other drivers, which helps new developers understand
> > > what the driver is doing.
> >
> > I beg to differ. You end-up having to pass function pointers through
> > platform_data to handle differences which could be handled by the core
> > IP.
> >
> > > The most important aspect though is the DT binding: once the structure
> > > with separate kernel drivers leaks out into the DT format, you can't
> > > easily change the driver any more, e.g. to make a property that was
> > > introduced for one glue driver more general so it can get handled by
> > > the common part. Having a single node lets us convert to the library
> > > model later, so that would be a strong reason to keep the DT binding
> > > simple, without child nodes.
> > >
> > > Following that logic, it turns into another reason for using the library
> > > model to start with, because otherwise the child device does not have
> > > any DT properties itself and has to rely on the parent device properties,
> > > which somewhat complicates the driver structure.
> >
> > this is bullcrap. Take a look at
> > Documentation/devicetree/bindings/usb/dwc3.txt:
> >
> > synopsys DWC3 CORE
> >
> > DWC3- USB3 CONTROLLER
> >
> > Required properties:
> > - compatible: must be "snps,dwc3"
> > - reg : Address and length of the register set for the device
> > - interrupts: Interrupts used by the dwc3 controller.
> >
> > Optional properties:
> > - usb-phy : array of phandle for the PHY device. The first element
> >in the array is expected to be a handle to the USB2/HS PHY and
> >the second element is expected to be a handle to the USB3/SS PHY
> > - phys: from the *Generic PHY* bindings
> > - phy-names: from the *Generic PHY* bindings
> > - tx-fifo-resize: determines if the FIFO *has* to be reallocated.
> >
> > This is usually a subnode to DWC3 glue to which it is connected.
> >
> > dwc3@4a03 {
> > compatible = "snps,dwc3";
> > reg = <0x4a03 0xcfff>;
> > interrupts = <0 92 4>
> > usb-phy = <&usb2_phy>, <&usb3,phy>;
> > tx-fifo-resize;
> > };
> >
> > This contains all the attributes it needs to work. In fact, this could
> > be used without any glue.
> >
>
> If you want to add "vbus-supply" core node to support ID switch, what's
> the plan for that?
send a patch ? Just make sure it's optional because not everybody needs
a vbus-supply. Also, DRD will still take a few merge windows to become a
reality. I don't want to merge something before considering it carefuly,
otherwise we will having which is broken and doesn't work for everybody
;-)
> Is it possible that the glue layer needs to access core register
> (eg, portsc.phcd during suspend)? The wrapper IP may wait some signals
> at core IP to change.
why would a glue layer need to access registers from the core ? That
sounds very odd. I haven't seen that and will, definitely, NACK such a
patch :-)
can you further describe why you think a glue layer might need to access
core IP's registers ?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
On Thu, Sep 25, 2014 at 09:11:35AM +0200, Arnd Bergmann wrote:
> On Thursday 25 September 2014 09:16:48 Peter Chen wrote:
> > > + }
> > > +
> > > + if (dev->of_node) {
> > > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata);
> > > + if (ret)
> > > + goto clk_err;
> > > + } else {
> > > + ret = dma_set_mask_and_coherent(&pdev->dev,
> > > DMA_BIT_MASK(32));
> > > + if (ret)
> > > + goto clk_err;
> > > + }
> >
> > Hi Antoine, the above code may not be needed, since get phy and set dma
> > mask are common operation, we can do it at core code, the only thing you
> > need to do is something like: dev->dma_mask = DMA_BIT_MASK(32).
> >
>
> Certainly not, doing that would be broken for a number of reasons:
>
> - dev->dma_mask is a pointer, a driver should not reassign that
> - the device may have been set up for a bus that requires a smaller mask
> that is already set, changing the mask would cause data corruption
> - setting just the dma mask but not coherent mask is wrong
> - setting the dma mask can fail, e.g. if the mask is smaller than the
> smallest memory zone, so you have to check the return value.
>
In current chipidea structure, the parent (glue layer) driver will not be
used for dma, udc/host driver uses dma mask from child (core layer), at core
layer we will do:
pdev->dev.dma_mask = dev->dma_mask; /* this device is parent */
dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask);
Would you suggestion us a suitable way? Or it is ok we use just Antoine's way
that
call dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) at parent
driver no matter dt or non-dt? Thanks.
--
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 v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
On Thu, Sep 25, 2014 at 09:15:53AM -0500, Felipe Balbi wrote:
> On Wed, Sep 24, 2014 at 02:23:38PM +0200, Arnd Bergmann wrote:
> > On Wednesday 24 September 2014 19:29:05 Peter Chen wrote:
> > >
> > > So, it is IP CORE LIB (you suggest) vs IP CORE Platform Driver
> > > (dwc3, musb, chipidea) you are talking about, right? Except for
> > > creating another platform driver as well as related DT node (optional),
> > > are there any advantages compared to current IP core driver structure?
> >
> > Having a library module usually requires less code, and is more
> > consistent with other drivers, which helps new developers understand
> > what the driver is doing.
>
> I beg to differ. You end-up having to pass function pointers through
> platform_data to handle differences which could be handled by the core
> IP.
>
> > The most important aspect though is the DT binding: once the structure
> > with separate kernel drivers leaks out into the DT format, you can't
> > easily change the driver any more, e.g. to make a property that was
> > introduced for one glue driver more general so it can get handled by
> > the common part. Having a single node lets us convert to the library
> > model later, so that would be a strong reason to keep the DT binding
> > simple, without child nodes.
> >
> > Following that logic, it turns into another reason for using the library
> > model to start with, because otherwise the child device does not have
> > any DT properties itself and has to rely on the parent device properties,
> > which somewhat complicates the driver structure.
>
> this is bullcrap. Take a look at
> Documentation/devicetree/bindings/usb/dwc3.txt:
>
> synopsys DWC3 CORE
>
> DWC3- USB3 CONTROLLER
>
> Required properties:
> - compatible: must be "snps,dwc3"
> - reg : Address and length of the register set for the device
> - interrupts: Interrupts used by the dwc3 controller.
>
> Optional properties:
> - usb-phy : array of phandle for the PHY device. The first element
>in the array is expected to be a handle to the USB2/HS PHY and
>the second element is expected to be a handle to the USB3/SS PHY
> - phys: from the *Generic PHY* bindings
> - phy-names: from the *Generic PHY* bindings
> - tx-fifo-resize: determines if the FIFO *has* to be reallocated.
>
> This is usually a subnode to DWC3 glue to which it is connected.
>
> dwc3@4a03 {
> compatible = "snps,dwc3";
> reg = <0x4a03 0xcfff>;
> interrupts = <0 92 4>
> usb-phy = <&usb2_phy>, <&usb3,phy>;
> tx-fifo-resize;
> };
>
> This contains all the attributes it needs to work. In fact, this could
> be used without any glue.
>
If you want to add "vbus-supply" core node to support ID switch, what's
the plan for that?
Is it possible that the glue layer needs to access core register
(eg, portsc.phcd during suspend)? The wrapper IP may wait some signals
at core IP to change.
--
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: g_mass_storage bug ?
Hi, On Thu, Sep 25, 2014 at 09:57:59PM +, Paul Zimmerman wrote: > > From: Felipe Balbi [mailto:ba...@ti.com] > > > > On Thu, Sep 25, 2014 at 07:08:12PM +, Paul Zimmerman wrote: > > > > From: Alan Stern [mailto:st...@rowland.harvard.edu] > > > > Sent: Thursday, September 25, 2014 11:08 AM > > > > > > > > On Thu, 25 Sep 2014, Paul Zimmerman wrote: > > > > > > > > > That's why I don't understand how this can happen for IN. AFAIK, a > > > > > STALL > > > > > is only sent in response to something the host sent in the CBW. At > > > > > that > > > > > point, there shouldn't be any IN transfers active. > > > > > > > > The gadget may send a partial response to the CBW. The end of the > > > > response is marked with a STALL. The mass-storage gadget driver > > > > submits the partial response and then calls usb_ep_set_halt() without > > > > waiting for the IN data to be delivered. It relies on the UDC driver > > > > returning -EAGAIN if any data is still pending. > > > > > > I guess you're referring to the code under the DATA_DIR_TO_HOST case > > > in finish_reply(). > > > > > > Felipe, in our vendor driver, the ep_set_halt() and ep_set_wedge() > > > functions check the request queue for the endpoint, and if it is not > > > empty, they return -EAGAIN. > > > > is that really enough ? The request is deleted (rather, moved to another > > list) as soon as StartTransfer is issued. I can certainly check both > > lists (and already do) but I fear that we might still race with the HW > > because there's no documentation to guarantee that I won't :-) > > > > > I see your patch for the dwc3 driver does add that, in addition to the > > > FIFO empty check. Does it still work OK if you remove the FIFO empty > > > check portion? > > > > It probably does, but I can't be sure there won't be a corner case where > > the list is empty (Xfercomplete was issued and request given back) but > > host hasn't moved all the data. Synopsys databook doesn't guarantee that > > will never be the case. > > > > Can you confirm, without a shadow of a doubt, that XferComplete will > > only be issued after host has moved every single bit of data ? If that > > can be updated to the databook, then sure, I can remove the FIFO space > > check; otherwise we might leave a corner case that will take us a few > > days to debug again because it will be very difficult to trigger :-) > > From the 2.70a databook, section 8.2.2: > > "While processing TRBs, two conditions may cause the core to write out an > event and raise an interrupt line: > - TRB Complete: > - For OUT endpoints, a packet is received which reduces the > remaining byte count in the TRB buffer to zero. > - For IN endpoints, an acknowledgement is received for a > transmitted packet which reduces the remaining byte count > in the TRB buffer to zero. > - Short Packet Received: > - For OUT endpoints only. While writing to a TRB buffer, > the endpoint receives a packet that is smaller than the > endpoint's MaxPacketSize. > " > > So for an IN, the completion event doesn't come until the TRB has been > ACKed, which means all the data has been sent. a very good point indeed. I completely missed that. Well, the patch can become a lot smaller, which makes my life easier when backporting :-) I'll test that out tomorrow, for sure :-) > But, I'm not saying you *have* to remove the FIFO space check. I think > we now know the problem was caused by the missing -EAGAIN. So having sure, that's certainly what was missing. > the FIFO check there shouldn't hurt anything, it's not masking some it's also completely useless. A few register accesses for no reason :-) > other problem. And you may need it in the future for one of those > other cases the databook talks about. I'll keep that in the back of my head. Perhaps I can repurpose these FIFO space accessors to implement usb_ep_fifo_status(). I'll consider that. cheers -- balbi signature.asc Description: Digital signature
RE: g_mass_storage bug ?
> From the 2.70a databook, section 8.2.2: Sorry, it's databook section 8.2.3, not 8.2.2. -- Paul > From: Paul Zimmerman > Sent: Thursday, September 25, 2014 2:58 PM > > > From: Felipe Balbi [mailto:ba...@ti.com] > > > > On Thu, Sep 25, 2014 at 07:08:12PM +, Paul Zimmerman wrote: > > > > From: Alan Stern [mailto:st...@rowland.harvard.edu] > > > > Sent: Thursday, September 25, 2014 11:08 AM > > > > > > > > On Thu, 25 Sep 2014, Paul Zimmerman wrote: > > > > > > > > > That's why I don't understand how this can happen for IN. AFAIK, a > > > > > STALL > > > > > is only sent in response to something the host sent in the CBW. At > > > > > that > > > > > point, there shouldn't be any IN transfers active. > > > > > > > > The gadget may send a partial response to the CBW. The end of the > > > > response is marked with a STALL. The mass-storage gadget driver > > > > submits the partial response and then calls usb_ep_set_halt() without > > > > waiting for the IN data to be delivered. It relies on the UDC driver > > > > returning -EAGAIN if any data is still pending. > > > > > > I guess you're referring to the code under the DATA_DIR_TO_HOST case > > > in finish_reply(). > > > > > > Felipe, in our vendor driver, the ep_set_halt() and ep_set_wedge() > > > functions check the request queue for the endpoint, and if it is not > > > empty, they return -EAGAIN. > > > > is that really enough ? The request is deleted (rather, moved to another > > list) as soon as StartTransfer is issued. I can certainly check both > > lists (and already do) but I fear that we might still race with the HW > > because there's no documentation to guarantee that I won't :-) > > > > > I see your patch for the dwc3 driver does add that, in addition to the > > > FIFO empty check. Does it still work OK if you remove the FIFO empty > > > check portion? > > > > It probably does, but I can't be sure there won't be a corner case where > > the list is empty (Xfercomplete was issued and request given back) but > > host hasn't moved all the data. Synopsys databook doesn't guarantee that > > will never be the case. > > > > Can you confirm, without a shadow of a doubt, that XferComplete will > > only be issued after host has moved every single bit of data ? If that > > can be updated to the databook, then sure, I can remove the FIFO space > > check; otherwise we might leave a corner case that will take us a few > > days to debug again because it will be very difficult to trigger :-) > > From the 2.70a databook, section 8.2.2: > > "While processing TRBs, two conditions may cause the core to write out an > event and raise an interrupt line: > - TRB Complete: > - For OUT endpoints, a packet is received which reduces the > remaining byte count in the TRB buffer to zero. > - For IN endpoints, an acknowledgement is received for a > transmitted packet which reduces the remaining byte count > in the TRB buffer to zero. > - Short Packet Received: > - For OUT endpoints only. While writing to a TRB buffer, > the endpoint receives a packet that is smaller than the > endpoint's MaxPacketSize. > " > > So for an IN, the completion event doesn't come until the TRB has been > ACKed, which means all the data has been sent. > > But, I'm not saying you *have* to remove the FIFO space check. I think > we now know the problem was caused by the missing -EAGAIN. So having > the FIFO check there shouldn't hurt anything, it's not masking some > other problem. And you may need it in the future for one of those > other cases the databook talks about. > > -- > Paul -- 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: g_mass_storage bug ?
> From: Felipe Balbi [mailto:ba...@ti.com] > > On Thu, Sep 25, 2014 at 07:08:12PM +, Paul Zimmerman wrote: > > > From: Alan Stern [mailto:st...@rowland.harvard.edu] > > > Sent: Thursday, September 25, 2014 11:08 AM > > > > > > On Thu, 25 Sep 2014, Paul Zimmerman wrote: > > > > > > > That's why I don't understand how this can happen for IN. AFAIK, a STALL > > > > is only sent in response to something the host sent in the CBW. At that > > > > point, there shouldn't be any IN transfers active. > > > > > > The gadget may send a partial response to the CBW. The end of the > > > response is marked with a STALL. The mass-storage gadget driver > > > submits the partial response and then calls usb_ep_set_halt() without > > > waiting for the IN data to be delivered. It relies on the UDC driver > > > returning -EAGAIN if any data is still pending. > > > > I guess you're referring to the code under the DATA_DIR_TO_HOST case > > in finish_reply(). > > > > Felipe, in our vendor driver, the ep_set_halt() and ep_set_wedge() > > functions check the request queue for the endpoint, and if it is not > > empty, they return -EAGAIN. > > is that really enough ? The request is deleted (rather, moved to another > list) as soon as StartTransfer is issued. I can certainly check both > lists (and already do) but I fear that we might still race with the HW > because there's no documentation to guarantee that I won't :-) > > > I see your patch for the dwc3 driver does add that, in addition to the > > FIFO empty check. Does it still work OK if you remove the FIFO empty > > check portion? > > It probably does, but I can't be sure there won't be a corner case where > the list is empty (Xfercomplete was issued and request given back) but > host hasn't moved all the data. Synopsys databook doesn't guarantee that > will never be the case. > > Can you confirm, without a shadow of a doubt, that XferComplete will > only be issued after host has moved every single bit of data ? If that > can be updated to the databook, then sure, I can remove the FIFO space > check; otherwise we might leave a corner case that will take us a few > days to debug again because it will be very difficult to trigger :-) >From the 2.70a databook, section 8.2.2: "While processing TRBs, two conditions may cause the core to write out an event and raise an interrupt line: - TRB Complete: - For OUT endpoints, a packet is received which reduces the remaining byte count in the TRB buffer to zero. - For IN endpoints, an acknowledgement is received for a transmitted packet which reduces the remaining byte count in the TRB buffer to zero. - Short Packet Received: - For OUT endpoints only. While writing to a TRB buffer, the endpoint receives a packet that is smaller than the endpoint's MaxPacketSize. " So for an IN, the completion event doesn't come until the TRB has been ACKed, which means all the data has been sent. But, I'm not saying you *have* to remove the FIFO space check. I think we now know the problem was caused by the missing -EAGAIN. So having the FIFO check there shouldn't hurt anything, it's not masking some other problem. And you may need it in the future for one of those other cases the databook talks about. -- Paul -- 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 v6 partial v2 2/3] usb: gadget: pxa27x_udc: transfer mach_info into pxa_udc
On Wed, Sep 24, 2014 at 11:06:00PM +0200, Robert Jarzmik wrote: > Convert the mach info, and store the udc_command in the pxa_udc control > structure. > > It is to be noticed that the udc_is_connected() in mach info is not > transfered. This was not used, as mioa701 machine doesn't need it, > balloon3 doesn't really use it, and most importantly the current driver > never uses it. > > Signed-off-by: Robert Jarzmik this looks good to me. -- balbi signature.asc Description: Digital signature
Re: g_mass_storage bug ?
Hi, On Thu, Sep 25, 2014 at 07:08:12PM +, Paul Zimmerman wrote: > > From: Alan Stern [mailto:st...@rowland.harvard.edu] > > Sent: Thursday, September 25, 2014 11:08 AM > > > > On Thu, 25 Sep 2014, Paul Zimmerman wrote: > > > > > That's why I don't understand how this can happen for IN. AFAIK, a STALL > > > is only sent in response to something the host sent in the CBW. At that > > > point, there shouldn't be any IN transfers active. > > > > The gadget may send a partial response to the CBW. The end of the > > response is marked with a STALL. The mass-storage gadget driver > > submits the partial response and then calls usb_ep_set_halt() without > > waiting for the IN data to be delivered. It relies on the UDC driver > > returning -EAGAIN if any data is still pending. > > I guess you're referring to the code under the DATA_DIR_TO_HOST case > in finish_reply(). > > Felipe, in our vendor driver, the ep_set_halt() and ep_set_wedge() > functions check the request queue for the endpoint, and if it is not > empty, they return -EAGAIN. is that really enough ? The request is deleted (rather, moved to another list) as soon as StartTransfer is issued. I can certainly check both lists (and already do) but I fear that we might still race with the HW because there's no documentation to guarantee that I won't :-) > I see your patch for the dwc3 driver does add that, in addition to the > FIFO empty check. Does it still work OK if you remove the FIFO empty > check portion? It probably does, but I can't be sure there won't be a corner case where the list is empty (Xfercomplete was issued and request given back) but host hasn't moved all the data. Synopsys databook doesn't guarantee that will never be the case. Can you confirm, without a shadow of a doubt, that XferComplete will only be issued after host has moved every single bit of data ? If that can be updated to the databook, then sure, I can remove the FIFO space check; otherwise we might leave a corner case that will take us a few days to debug again because it will be very difficult to trigger :-) cheers -- balbi signature.asc Description: Digital signature
Re: g_mass_storage bug ?
Hi, On Thu, Sep 25, 2014 at 05:50:59PM +, Paul Zimmerman wrote: > > From: Felipe Balbi [mailto:ba...@ti.com] > > Sent: Wednesday, September 24, 2014 7:41 PM > > > > On Thu, Sep 25, 2014 at 01:37:05AM +, Paul Zimmerman wrote: > > > > > > Then, after the host cleared the halt, the gadget apparently sent > > > > > > the > > > > > > data that _should_ have been sent previously. The host was > > > > > > expecting > > > > > > to receive the CSW at this point, so there was an overflow error. > > > > > > That's what caused the host to perform a reset. > > > > > > > > > > > > Evidently this UDC implements the set_halt method incorrectly. > > > > > > According to the kerneldoc for usb_ep_set_halt: > > > > > > > > > > > > * Attempts to halt IN endpoints will fail (returning -EAGAIN) if > > > > > > any > > > > > > * transfer requests are still queued, or if the controller hardware > > > > > > * (usually a FIFO) still holds bytes that the host hasn't > > > > > > collected. > > > > > > > > > > damn old bugs :-) I'll fix that up and Cc stable. > > > > > > > > alright fixed. Below you can see a combined diff which I'll still split > > > > into patches so they can be applied properly. > > > > > > [ snipped patch which grubs around in the GDBGFIFOSPACE registers ] > > > > > > Ick. That shouldn't be necessary. The Synopsys vendor driver works > > > > you need to make sure that both there are no pending transfers and FIFO > > is empty in case of IN endpoints. Databook itself says (sorry, forgot > > the section and no access to the docs right now) that to figure out if > > the FIFO is empty, application can use GDBGFIFOSPACE. Do you have any > > better ideas ? > > I guess I'm just afraid this may just be papering over the real cause. Well, the API documentation, as pointed out by Alan, calls for: 380 * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any 381 * transfer requests are still queued, or if the controller hardware 382 * (usually a FIFO) still holds bytes that the host hasn't collected. There's no other way to ensuring FIFO is empty with this core. > > > fine with the mass-storage gadget (with stall=1) without doing anything > > > like that. > > > > > > When did the dwc3 driver start showing this problem? Wouldn't it be > > > better to find the change which caused this? Or has dwc3 always had > > > > it's probably been like that since ever :-) I just figured that my > > scripts always had stall=0 and ended up never testing the other way. > > > > > this problem, but you never tested with stall=1 before so didn't see > > > it? > > > > yup. Note that it's not enough to checked for TRB completion because > > there could still be data in the FIFO which the host hasn't moved yet, > > unless it's 100% guaranteed that the core won't fire XferComplete until > > every single bit of data has been moved by the host. > > That is supposed to be 100% guaranteed. The IN XferInProgress and > XferComplete events are only delivered after the host has ACKed the > packet. > > > But the way things are, I'd expect core to be firing transfer complete > > as soon as all data has reached the FIFO (in case of TX). > > Nope. > > That's why I don't understand how this can happen for IN. AFAIK, a STALL > is only sent in response to something the host sent in the CBW. At that > point, there shouldn't be any IN transfers active. you could race with the HW, right ? You just issued Start Transfer for that IN transfer and control returns to the gadget driver which can, at that very moment, issue a Set Stall. SW *must* make sure that there's nothing pending yet. We haven't received XferComplete for that IN we just started. > BTW, about a year ago, I fixed a similar issue in our vendor driver. > But I don't remember what the cause was, I will search the Git log to > see if I can find the commit that fixed it. ok, thanks. -- balbi signature.asc Description: Digital signature
Re: g_mass_storage bug ?
On Thu, Sep 25, 2014 at 07:43:35PM +, Paul Zimmerman wrote: > > From: Paul Zimmerman > > > > > From: Felipe Balbi [mailto:ba...@ti.com] > > > > > > On Wed, Sep 24, 2014 at 09:40:37PM -0500, Felipe Balbi wrote: > > > > On Thu, Sep 25, 2014 at 01:37:05AM +, Paul Zimmerman wrote: > > > > > > > > Then, after the host cleared the halt, the gadget apparently > > > > > > > > sent the > > > > > > > > data that _should_ have been sent previously. The host was > > > > > > > > expecting > > > > > > > > to receive the CSW at this point, so there was an overflow > > > > > > > > error. > > > > > > > > That's what caused the host to perform a reset. > > > > > > > > > > > > > > > > Evidently this UDC implements the set_halt method incorrectly. > > > > > > > > According to the kerneldoc for usb_ep_set_halt: > > > > > > > > > > > > > > > > * Attempts to halt IN endpoints will fail (returning -EAGAIN) > > > > > > > > if any > > > > > > > > * transfer requests are still queued, or if the controller > > > > > > > > hardware > > > > > > > > * (usually a FIFO) still holds bytes that the host hasn't > > > > > > > > collected. > > > > > > > > > > > > > > damn old bugs :-) I'll fix that up and Cc stable. > > > > > > > > > > > > alright fixed. Below you can see a combined diff which I'll still > > > > > > split > > > > > > into patches so they can be applied properly. > > > > > > > > > > [ snipped patch which grubs around in the GDBGFIFOSPACE registers ] > > > > > > > > > > Ick. That shouldn't be necessary. The Synopsys vendor driver works > > > > > > > > you need to make sure that both there are no pending transfers and FIFO > > > > is empty in case of IN endpoints. Databook itself says (sorry, forgot > > > > the section and no access to the docs right now) that to figure out if > > > > the FIFO is empty, application can use GDBGFIFOSPACE. Do you have any > > > > better ideas ? > > > > > > oh, and btw... I only noticed the failure with Linux. I mentioned that > > > it, though flakey during start, worked very stably against Mac OS X. > > > Perhaps Windows is also more forgiving ? > > > > Yeah, I see you said it only fails with 3.17-rc on an embedded platform > > you have. Can you say which platform that is? Do you know whose xHCI > > controller it uses? > > > > > Can you share a little more details on what you guys did to get it to > > > work without making sure FIFO is empty ? I can't really understand how > > > you'd cover all cases otherwise... > > > > See my other email, I will try to find the fix for the similar issue > > that we saw. > > OK, it looks like that issue is not related to this one. We were > accidentally setting the Stream Capable bit in the endpoint > configuration for all bulk EPs, not just stream-enabled ones. That > caused several strange behaviors, including the Set Stall command > not working. ok, this it not about Set Stall not working, however, it's about making sure we don't stall at the wrong time :-) -- balbi signature.asc Description: Digital signature
Re: g_mass_storage bug ?
On Thu, Sep 25, 2014 at 05:54:10PM +, Paul Zimmerman wrote: > > From: Felipe Balbi [mailto:ba...@ti.com] > > > > On Wed, Sep 24, 2014 at 09:40:37PM -0500, Felipe Balbi wrote: > > > On Thu, Sep 25, 2014 at 01:37:05AM +, Paul Zimmerman wrote: > > > > > > > Then, after the host cleared the halt, the gadget apparently sent > > > > > > > the > > > > > > > data that _should_ have been sent previously. The host was > > > > > > > expecting > > > > > > > to receive the CSW at this point, so there was an overflow error. > > > > > > > That's what caused the host to perform a reset. > > > > > > > > > > > > > > Evidently this UDC implements the set_halt method incorrectly. > > > > > > > According to the kerneldoc for usb_ep_set_halt: > > > > > > > > > > > > > > * Attempts to halt IN endpoints will fail (returning -EAGAIN) if > > > > > > > any > > > > > > > * transfer requests are still queued, or if the controller > > > > > > > hardware > > > > > > > * (usually a FIFO) still holds bytes that the host hasn't > > > > > > > collected. > > > > > > > > > > > > damn old bugs :-) I'll fix that up and Cc stable. > > > > > > > > > > alright fixed. Below you can see a combined diff which I'll still > > > > > split > > > > > into patches so they can be applied properly. > > > > > > > > [ snipped patch which grubs around in the GDBGFIFOSPACE registers ] > > > > > > > > Ick. That shouldn't be necessary. The Synopsys vendor driver works > > > > > > you need to make sure that both there are no pending transfers and FIFO > > > is empty in case of IN endpoints. Databook itself says (sorry, forgot > > > the section and no access to the docs right now) that to figure out if > > > the FIFO is empty, application can use GDBGFIFOSPACE. Do you have any > > > better ideas ? > > > > oh, and btw... I only noticed the failure with Linux. I mentioned that > > it, though flakey during start, worked very stably against Mac OS X. > > Perhaps Windows is also more forgiving ? > > Yeah, I see you said it only fails with 3.17-rc on an embedded platform > you have. Can you say which platform that is? Do you know whose xHCI AM473x with DWC3 2.40a (2 instances of it). > controller it uses? yours :-) > > Can you share a little more details on what you guys did to get it to > > work without making sure FIFO is empty ? I can't really understand how > > you'd cover all cases otherwise... > > See my other email, I will try to find the fix for the similar issue > that we saw. alright, tks -- balbi signature.asc Description: Digital signature
Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
Arnd,
On Thu, Sep 25, 2014 at 09:12:07PM +0200, Arnd Bergmann wrote:
> On Tuesday 23 September 2014, Antoine Tenart wrote:
> > +static int ci_hdrc_usb2_dt_probe(struct device *dev,
> > +struct ci_hdrc_platform_data *ci_pdata)
> > +{
> > + ci_pdata->phy = of_phy_get(dev->of_node, 0);
>
> FWIW, I accidentally built a kernel with this driver enabled and got a warning
> for this code. The problem is that ci_pdata->phy is a 'struct usb_phy'
> pointer,
> while of_phy_get() returns a generic 'struct phy'. While the two have similar
> behavior, they are not the same thing and this can't work.
That's because this series applies on top of:
https://lkml.org/lkml/2014/9/15/141
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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: g_mass_storage bug ?
> From: Paul Zimmerman > > > From: Felipe Balbi [mailto:ba...@ti.com] > > > > On Wed, Sep 24, 2014 at 09:40:37PM -0500, Felipe Balbi wrote: > > > On Thu, Sep 25, 2014 at 01:37:05AM +, Paul Zimmerman wrote: > > > > > > > Then, after the host cleared the halt, the gadget apparently sent > > > > > > > the > > > > > > > data that _should_ have been sent previously. The host was > > > > > > > expecting > > > > > > > to receive the CSW at this point, so there was an overflow error. > > > > > > > That's what caused the host to perform a reset. > > > > > > > > > > > > > > Evidently this UDC implements the set_halt method incorrectly. > > > > > > > According to the kerneldoc for usb_ep_set_halt: > > > > > > > > > > > > > > * Attempts to halt IN endpoints will fail (returning -EAGAIN) if > > > > > > > any > > > > > > > * transfer requests are still queued, or if the controller > > > > > > > hardware > > > > > > > * (usually a FIFO) still holds bytes that the host hasn't > > > > > > > collected. > > > > > > > > > > > > damn old bugs :-) I'll fix that up and Cc stable. > > > > > > > > > > alright fixed. Below you can see a combined diff which I'll still > > > > > split > > > > > into patches so they can be applied properly. > > > > > > > > [ snipped patch which grubs around in the GDBGFIFOSPACE registers ] > > > > > > > > Ick. That shouldn't be necessary. The Synopsys vendor driver works > > > > > > you need to make sure that both there are no pending transfers and FIFO > > > is empty in case of IN endpoints. Databook itself says (sorry, forgot > > > the section and no access to the docs right now) that to figure out if > > > the FIFO is empty, application can use GDBGFIFOSPACE. Do you have any > > > better ideas ? > > > > oh, and btw... I only noticed the failure with Linux. I mentioned that > > it, though flakey during start, worked very stably against Mac OS X. > > Perhaps Windows is also more forgiving ? > > Yeah, I see you said it only fails with 3.17-rc on an embedded platform > you have. Can you say which platform that is? Do you know whose xHCI > controller it uses? > > > Can you share a little more details on what you guys did to get it to > > work without making sure FIFO is empty ? I can't really understand how > > you'd cover all cases otherwise... > > See my other email, I will try to find the fix for the similar issue > that we saw. OK, it looks like that issue is not related to this one. We were accidentally setting the Stream Capable bit in the endpoint configuration for all bulk EPs, not just stream-enabled ones. That caused several strange behaviors, including the Set Stall command not working. -- Paul -- 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 00/27] add pm_runtime_last_busy_and_autosuspend() helper
On Thursday, September 25, 2014 04:27:58 PM Wolfram Sang wrote: > > --Bn2rw/3z4jIqBvZU > Content-Type: text/plain; charset=us-ascii > Content-Disposition: inline > Content-Transfer-Encoding: quoted-printable > > On Thu, Sep 25, 2014 at 09:22:01AM -0500, Felipe Balbi wrote: > > On Thu, Sep 25, 2014 at 01:27:18PM +0530, Vinod Koul wrote: > > > On Wed, Sep 24, 2014 at 03:32:19PM -0500, Felipe Balbi wrote: > > > > > > > OK, I guess this is as good as it gets. > > > > > > >=20 > > > > > > > What tree would you like it go through? > > > > > >=20 > > > > > > Do we really need this new helper ? I mean, the very moment when = > we > > > > > > decide to implement ->runtime_idle() we will need to get rid of t= > his > > > > > > change. I wonder if it's really valid... > > > > >=20 > > > > > I'm not sure I'm following? This seems to simply implement what dr= > ivers > > > > > have been doing already as one function. Why would it be invalid t= > o reduce > > > > > code duplication? > > > >=20 > > > > For two reasons: > > > >=20 > > > > 1) the helper has no inteligence whatsoever. It just calls the same > > > > functions. > > > >=20 > > > > 2) the duplication will vanish whenever someone implements > > > > ->runtime_idle() and have that call pm_runtime_autosuspend() (like PCI > > > > and USB buses are doing today). This will just be yet another line th= > at > > > > needs to change. > > > >=20 > > > > Frankly though, no strong feelings, I just think it's a commit that > > > > doesn't bring that any benefits other than looking like one line was > > > > removed. > > > and yes that is what it tries to do nothing more nothing less. If in fu= > ture > > > there are no users (today we have quite a few), then we can remove the = > dead > > > macro, no harm. But that is not the situation today. > >=20 > > as I said, a commit that's bound to be useless. It's not like you're > > saving 10 lines of code, it's only one. Replacing two simple lines with > > a function which takes almost as many characters to type . > >=20 > > IMO, this is pretty useless and I'd rather not see them in the drivers I > > maintain, sorry. > > It is not a NACK from me; yet from a high-level perspective I agree with > Felipe. OK I'd rather not merge something that driver people don't want to use. Vinod? -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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 v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
On Tuesday 23 September 2014, Antoine Tenart wrote:
> +static int ci_hdrc_usb2_dt_probe(struct device *dev,
> +struct ci_hdrc_platform_data *ci_pdata)
> +{
> + ci_pdata->phy = of_phy_get(dev->of_node, 0);
FWIW, I accidentally built a kernel with this driver enabled and got a warning
for this code. The problem is that ci_pdata->phy is a 'struct usb_phy' pointer,
while of_phy_get() returns a generic 'struct phy'. While the two have similar
behavior, they are not the same thing and this can't work.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: g_mass_storage bug ?
> From: Alan Stern [mailto:st...@rowland.harvard.edu] > Sent: Thursday, September 25, 2014 11:08 AM > > On Thu, 25 Sep 2014, Paul Zimmerman wrote: > > > That's why I don't understand how this can happen for IN. AFAIK, a STALL > > is only sent in response to something the host sent in the CBW. At that > > point, there shouldn't be any IN transfers active. > > The gadget may send a partial response to the CBW. The end of the > response is marked with a STALL. The mass-storage gadget driver > submits the partial response and then calls usb_ep_set_halt() without > waiting for the IN data to be delivered. It relies on the UDC driver > returning -EAGAIN if any data is still pending. I guess you're referring to the code under the DATA_DIR_TO_HOST case in finish_reply(). Felipe, in our vendor driver, the ep_set_halt() and ep_set_wedge() functions check the request queue for the endpoint, and if it is not empty, they return -EAGAIN. I see your patch for the dwc3 driver does add that, in addition to the FIFO empty check. Does it still work OK if you remove the FIFO empty check portion? -- Paul -- 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: project wide: git config entry for [diff] renames=true
Jeff King writes: > There is no such mechanism within git. We've resisted adding one because > of the danger of something like: > > [diff] > external = rm -rf / > > diff.renames is probably safe, but any config-sharing mechanism would > have to deal with either whitelisting, or providing some mechanism for > the puller to review changes before blindly following them. It might be useful to add a "safe include" feature, perhaps? We ship a small set of hardcoded default whitelist (diff.renames may be included in there), and allow the user who do not want to be affected to override it with [include] safe = !diff.renames or even [config] safe = !* at the same time allow them to add what we do not hardcode to it using the same mechanism, e.g. [config] safe = merge.* Then [include] safe path = ../project.gitconfig [include] path = $HOME/.gitconfig-variant1 would only allow the variables include.safe deems safe to affect us from the in-tree file, and use everything from my personal set in my home directory. -- 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: project wide: git config entry for [diff] renames=true
Joe Perches writes: > On Thu, 2014-09-25 at 14:00 -0400, Jeff King wrote: > ... >> diff.renames is probably safe, but any config-sharing mechanism would >> have to deal with either whitelisting, or providing some mechanism for >> the puller to review changes before blindly following them. > > Another mechanism might be to add a repository > top level .gitconfig and add whatever to that. That could be smaller half of an implementation detail of one of the two possibilities Jeff mentioned i.e. "mechanism for the puller to review changes before blindly following". It gives the transfer part. You still need a new mechanism to make that file that is tracked in the repository to be used as part of your configuration variable set after letting the puller to review and approve. A puller who blindly trust the project could use the "include" mechanism from your .git/config to include a file with a well-known name that is tracked by the project _without_ review or approval. I doubt we would recommend that in an open source setting, 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: g_mass_storage bug ?
On Thu, 25 Sep 2014, Paul Zimmerman wrote: > That's why I don't understand how this can happen for IN. AFAIK, a STALL > is only sent in response to something the host sent in the CBW. At that > point, there shouldn't be any IN transfers active. The gadget may send a partial response to the CBW. The end of the response is marked with a STALL. The mass-storage gadget driver submits the partial response and then calls usb_ep_set_halt() without waiting for the IN data to be delivered. It relies on the UDC driver returning -EAGAIN if any data is still pending. 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: project wide: git config entry for [diff] renames=true
On Thu, 2014-09-25 at 14:00 -0400, Jeff King wrote: > On Thu, Sep 25, 2014 at 08:48:31AM -0700, Joe Perches wrote: > > > On Thu, 2014-09-25 at 17:03 +0200, Greg Kroah-Hartman wrote: > > > > > In the future, please generate a git "move" diff, which makes it easier > > > to review, and prove that nothing really changed. It also helps if the > > > file is a bit different from what you diffed against, which in my case, > > > was true. > > > > Maybe it'd be possible to add > > > > [diff] > > renames = true > > > > to the .git/config file. > > > > but I don't find a mechanism to add anything to the > > .git/config and have it be pulled. > > There is no such mechanism within git. We've resisted adding one because > of the danger of something like: > > [diff] > external = rm -rf / > > diff.renames is probably safe, but any config-sharing mechanism would > have to deal with either whitelisting, or providing some mechanism for > the puller to review changes before blindly following them. Another mechanism might be to add a repository top level .gitconfig and add whatever to that. -- 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 v6 partial v2 1/3] usb: gadget: pxa27x_udc: prepare device-tree support
Linus Walleij writes: > On Thu, Sep 25, 2014 at 3:44 PM, wrote: > >> Could you also review patch 1/3 while at it ;) ? > > This is patch 1/3... > > I guess you mean the others. Ah yeah, I meant patch 3/3, slipped on the keyboard. A full review is always welcome, but patches 1/3 and 3/3 touch gpios and comments or ack would be good. Patch 2/3 touches only udc mach info, so you could very well pass this one. Cheers. -- Robert -- 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: project wide: git config entry for [diff] renames=true
On Thu, Sep 25, 2014 at 08:48:31AM -0700, Joe Perches wrote: > On Thu, 2014-09-25 at 17:03 +0200, Greg Kroah-Hartman wrote: > > > In the future, please generate a git "move" diff, which makes it easier > > to review, and prove that nothing really changed. It also helps if the > > file is a bit different from what you diffed against, which in my case, > > was true. > > Maybe it'd be possible to add > > [diff] > renames = true > > to the .git/config file. > > but I don't find a mechanism to add anything to the > .git/config and have it be pulled. There is no such mechanism within git. We've resisted adding one because of the danger of something like: [diff] external = rm -rf / diff.renames is probably safe, but any config-sharing mechanism would have to deal with either whitelisting, or providing some mechanism for the puller to review changes before blindly following them. -Peff -- 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: g_mass_storage bug ?
> From: Felipe Balbi [mailto:ba...@ti.com] > > On Wed, Sep 24, 2014 at 09:40:37PM -0500, Felipe Balbi wrote: > > On Thu, Sep 25, 2014 at 01:37:05AM +, Paul Zimmerman wrote: > > > > > > Then, after the host cleared the halt, the gadget apparently sent > > > > > > the > > > > > > data that _should_ have been sent previously. The host was > > > > > > expecting > > > > > > to receive the CSW at this point, so there was an overflow error. > > > > > > That's what caused the host to perform a reset. > > > > > > > > > > > > Evidently this UDC implements the set_halt method incorrectly. > > > > > > According to the kerneldoc for usb_ep_set_halt: > > > > > > > > > > > > * Attempts to halt IN endpoints will fail (returning -EAGAIN) if > > > > > > any > > > > > > * transfer requests are still queued, or if the controller hardware > > > > > > * (usually a FIFO) still holds bytes that the host hasn't > > > > > > collected. > > > > > > > > > > damn old bugs :-) I'll fix that up and Cc stable. > > > > > > > > alright fixed. Below you can see a combined diff which I'll still split > > > > into patches so they can be applied properly. > > > > > > [ snipped patch which grubs around in the GDBGFIFOSPACE registers ] > > > > > > Ick. That shouldn't be necessary. The Synopsys vendor driver works > > > > you need to make sure that both there are no pending transfers and FIFO > > is empty in case of IN endpoints. Databook itself says (sorry, forgot > > the section and no access to the docs right now) that to figure out if > > the FIFO is empty, application can use GDBGFIFOSPACE. Do you have any > > better ideas ? > > oh, and btw... I only noticed the failure with Linux. I mentioned that > it, though flakey during start, worked very stably against Mac OS X. > Perhaps Windows is also more forgiving ? Yeah, I see you said it only fails with 3.17-rc on an embedded platform you have. Can you say which platform that is? Do you know whose xHCI controller it uses? > Can you share a little more details on what you guys did to get it to > work without making sure FIFO is empty ? I can't really understand how > you'd cover all cases otherwise... See my other email, I will try to find the fix for the similar issue that we saw. -- Paul -- 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: g_mass_storage bug ?
> From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Wednesday, September 24, 2014 7:41 PM > > On Thu, Sep 25, 2014 at 01:37:05AM +, Paul Zimmerman wrote: > > > > > Then, after the host cleared the halt, the gadget apparently sent the > > > > > data that _should_ have been sent previously. The host was expecting > > > > > to receive the CSW at this point, so there was an overflow error. > > > > > That's what caused the host to perform a reset. > > > > > > > > > > Evidently this UDC implements the set_halt method incorrectly. > > > > > According to the kerneldoc for usb_ep_set_halt: > > > > > > > > > > * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any > > > > > * transfer requests are still queued, or if the controller hardware > > > > > * (usually a FIFO) still holds bytes that the host hasn't collected. > > > > > > > > damn old bugs :-) I'll fix that up and Cc stable. > > > > > > alright fixed. Below you can see a combined diff which I'll still split > > > into patches so they can be applied properly. > > > > [ snipped patch which grubs around in the GDBGFIFOSPACE registers ] > > > > Ick. That shouldn't be necessary. The Synopsys vendor driver works > > you need to make sure that both there are no pending transfers and FIFO > is empty in case of IN endpoints. Databook itself says (sorry, forgot > the section and no access to the docs right now) that to figure out if > the FIFO is empty, application can use GDBGFIFOSPACE. Do you have any > better ideas ? I guess I'm just afraid this may just be papering over the real cause. > > fine with the mass-storage gadget (with stall=1) without doing anything > > like that. > > > > When did the dwc3 driver start showing this problem? Wouldn't it be > > better to find the change which caused this? Or has dwc3 always had > > it's probably been like that since ever :-) I just figured that my > scripts always had stall=0 and ended up never testing the other way. > > > this problem, but you never tested with stall=1 before so didn't see > > it? > > yup. Note that it's not enough to checked for TRB completion because > there could still be data in the FIFO which the host hasn't moved yet, > unless it's 100% guaranteed that the core won't fire XferComplete until > every single bit of data has been moved by the host. That is supposed to be 100% guaranteed. The IN XferInProgress and XferComplete events are only delivered after the host has ACKed the packet. > But the way things are, I'd expect core to be firing transfer complete > as soon as all data has reached the FIFO (in case of TX). Nope. That's why I don't understand how this can happen for IN. AFAIK, a STALL is only sent in response to something the host sent in the CBW. At that point, there shouldn't be any IN transfers active. BTW, about a year ago, I fixed a similar issue in our vendor driver. But I don't remember what the cause was, I will search the Git log to see if I can find the commit that fixed it. -- Paul -- 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 v6 2/4] i2c: add support for Diolan DLN-2 USB-I2C adapter
From: Laurentiu Palcu
This patch adds support for the Diolan DLN-2 I2C master module. Due
to hardware limitations it does not support SMBUS quick commands.
Information about the USB protocol interface can be found in the
Programmer's Reference Manual [1], see section 6.2.2 for the I2C
master module commands and responses.
[1] https://www.diolan.com/downloads/dln-api-manual.pdf
Signed-off-by: Laurentiu Palcu
Signed-off-by: Octavian Purdila
---
.../ABI/testing/sysfs-bus-i2c-busses-dln2 | 5 +
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile| 1 +
drivers/i2c/busses/i2c-dln2.c | 378 +
4 files changed, 394 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-busses-dln2
create mode 100644 drivers/i2c/busses/i2c-dln2.c
diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-busses-dln2
b/Documentation/ABI/testing/sysfs-bus-i2c-busses-dln2
new file mode 100644
index 000..ad55af6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-i2c-busses-dln2
@@ -0,0 +1,5 @@
+What: /sys/bus/i2c/devices/.../dln2_i2c_freq
+Date: September 2014
+Contact: Octavian Purdila
+Description:
+ This attribute shows/sets the frequency (in Hz) of the I2C bus.
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 2ac87fa..6afc17e 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1021,4 +1021,14 @@ config SCx200_ACB
This support is also available as a module. If so, the module
will be called scx200_acb.
+config I2C_DLN2
+ tristate "Diolan DLN-2 USB I2C adapter"
+ depends on MFD_DLN2
+ help
+ If you say yes to this option, support will be included for Diolan
+ DLN2, a USB to I2C interface.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-dln2.
+
endmenu
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 49bf07e..3118fea 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -100,5 +100,6 @@ obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o
obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o
obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
+obj-$(CONFIG_I2C_DLN2) += i2c-dln2.o
ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
diff --git a/drivers/i2c/busses/i2c-dln2.c b/drivers/i2c/busses/i2c-dln2.c
new file mode 100644
index 000..15b7f35b
--- /dev/null
+++ b/drivers/i2c/busses/i2c-dln2.c
@@ -0,0 +1,378 @@
+/*
+ * Driver for the Diolan DLN-2 USB-I2C adapter
+ *
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * Derived from:
+ * i2c-diolan-u2c.c
+ * Copyright (c) 2010-2011 Ericsson AB
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DLN2_I2C_MODULE_ID 0x03
+#define DLN2_I2C_CMD(cmd) DLN2_CMD(cmd, DLN2_I2C_MODULE_ID)
+
+/* I2C commands */
+#define DLN2_I2C_GET_PORT_COUNTDLN2_I2C_CMD(0x00)
+#define DLN2_I2C_ENABLEDLN2_I2C_CMD(0x01)
+#define DLN2_I2C_DISABLE DLN2_I2C_CMD(0x02)
+#define DLN2_I2C_IS_ENABLEDDLN2_I2C_CMD(0x03)
+#define DLN2_I2C_SET_FREQUENCY DLN2_I2C_CMD(0x04)
+#define DLN2_I2C_GET_FREQUENCY DLN2_I2C_CMD(0x05)
+#define DLN2_I2C_WRITE DLN2_I2C_CMD(0x06)
+#define DLN2_I2C_READ DLN2_I2C_CMD(0x07)
+#define DLN2_I2C_SCAN_DEVICES DLN2_I2C_CMD(0x08)
+#define DLN2_I2C_PULLUP_ENABLE DLN2_I2C_CMD(0x09)
+#define DLN2_I2C_PULLUP_DISABLEDLN2_I2C_CMD(0x0A)
+#define DLN2_I2C_PULLUP_IS_ENABLED DLN2_I2C_CMD(0x0B)
+#define DLN2_I2C_TRANSFER DLN2_I2C_CMD(0x0C)
+#define DLN2_I2C_SET_MAX_REPLY_COUNT DLN2_I2C_CMD(0x0D)
+#define DLN2_I2C_GET_MAX_REPLY_COUNT DLN2_I2C_CMD(0x0E)
+#define DLN2_I2C_GET_MIN_FREQUENCY DLN2_I2C_CMD(0x40)
+#define DLN2_I2C_GET_MAX_FREQUENCY DLN2_I2C_CMD(0x41)
+
+#define DLN2_I2C_FREQ_STD 10
+
+#define DLN2_I2C_MAX_XFER_SIZE 256
+#define DLN2_I2C_BUF_SIZE (DLN2_I2C_MAX_XFER_SIZE + 16)
+
+struct dln2_i2c {
+ struct platform_device *pdev;
+ struct i2c_adapter adapter;
+ u32 freq;
+ u32 min_freq;
+ u32 max_freq;
+ u8 port;
+ /*
+* Buffer to hold the packet for read or write transfers. One
+* is enough since we can't have multiple transfers in
+* parallel on the i2c adapter.
+*/
+ void *buf;
+};
+
+static int dln2_i2c_enable(struct dln2_i2c *dln2, bool enable)
+{
+ int ret;
+ u16 cmd;
+
+ if (enable)
+ cmd = DLN2
[PATCH v6 0/4] mfd: add support for Diolan DLN-2
This patch series adds support for Diolan USB-I2C/GPIO Master Adapter DLN-2. Details about device can be found here: https://www.diolan.com/i2c/i2c_interface.html. Changes since v5: * MFD: use enum for handles, fix a couple of miss spells, rename a few fields for clarity, rework the disconnect mechanism, fix a couple of missing newlines in printks, use straight list_for_each instead of RCU versions in the update sides, used busnum << 8 | devnum as id in mfd_add_devices() * GPIO: renamed pin_dir to output_enabled, disable GPIO if we failed getting the direction, don't set platform's driver owner field * I2C: update Documentation/ABI for the added sysfs attributed, stored the port number in the dln2_i2c structure, used a temporary pointer to the tx/rx struct for readability, don't set platform's driver owner field Daniel Baluta (1): gpio: add support for the Diolan DLN-2 USB GPIO driver Laurentiu Palcu (1): i2c: add support for Diolan DLN-2 USB-I2C adapter Octavian Purdila (2): mfd: add support for Diolan DLN-2 devices gpiolib: add irq_not_threaded flag to gpio_chip .../ABI/testing/sysfs-bus-i2c-busses-dln2 | 5 + drivers/gpio/Kconfig | 12 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-dln2.c | 556 +++ drivers/gpio/gpiolib.c | 2 +- drivers/i2c/busses/Kconfig | 10 + drivers/i2c/busses/Makefile| 1 + drivers/i2c/busses/i2c-dln2.c | 378 +++ drivers/mfd/Kconfig| 9 + drivers/mfd/Makefile | 1 + drivers/mfd/dln2.c | 752 + include/linux/gpio/driver.h| 3 + include/linux/mfd/dln2.h | 67 ++ 13 files changed, 1796 insertions(+), 1 deletion(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-busses-dln2 create mode 100644 drivers/gpio/gpio-dln2.c create mode 100644 drivers/i2c/busses/i2c-dln2.c create mode 100644 drivers/mfd/dln2.c create mode 100644 include/linux/mfd/dln2.h -- 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 v6 1/4] mfd: add support for Diolan DLN-2 devices
This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
Master Adapter DLN-2. Details about the device can be found here:
https://www.diolan.com/i2c/i2c_interface.html.
Information about the USB protocol can be found in the Programmer's
Reference Manual [1], see section 1.7.
Because the hardware has a single transmit endpoint and a single
receive endpoint the communication between the various DLN2 drivers
and the hardware will be muxed/demuxed by this driver.
Each DLN2 module will be identified by the handle field within the DLN2
message header. If a DLN2 module issues multiple commands in parallel
they will be identified by the echo counter field in the message header.
The DLN2 modules can use the dln2_transfer() function to issue a
command and wait for its response. They can also register a callback
that is going to be called when a specific event id is generated by
the device (e.g. GPIO interrupts). The device uses handle 0 for
sending events.
[1] https://www.diolan.com/downloads/dln-api-manual.pdf
Signed-off-by: Octavian Purdila
---
drivers/mfd/Kconfig | 9 +
drivers/mfd/Makefile | 1 +
drivers/mfd/dln2.c | 751 +++
include/linux/mfd/dln2.h | 67 +
4 files changed, 828 insertions(+)
create mode 100644 drivers/mfd/dln2.c
create mode 100644 include/linux/mfd/dln2.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..7bcf895 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -183,6 +183,15 @@ config MFD_DA9063
Additional drivers must be enabled in order to use the functionality
of the device.
+config MFD_DLN2
+ tristate "Diolan DLN2 support"
+ select MFD_CORE
+ depends on USB
+ help
+ This adds support for Diolan USB-I2C/SPI/GPIO Master Adapter DLN-2.
+ Additional drivers must be enabled in order to use the functionality
+ of the device.
+
config MFD_MC13XXX
tristate
depends on (SPI_MASTER || I2C)
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..591988d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -169,6 +169,7 @@ obj-$(CONFIG_MFD_AS3711)+= as3711.o
obj-$(CONFIG_MFD_AS3722) += as3722.o
obj-$(CONFIG_MFD_STW481X) += stw481x.o
obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o
+obj-$(CONFIG_MFD_DLN2) += dln2.o
intel-soc-pmic-objs:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
new file mode 100644
index 000..3ff8c6d
--- /dev/null
+++ b/drivers/mfd/dln2.c
@@ -0,0 +1,751 @@
+/*
+ * Driver for the Diolan DLN-2 USB adapter
+ *
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * Derived from:
+ * i2c-diolan-u2c.c
+ * Copyright (c) 2010-2011 Ericsson AB
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct dln2_header {
+ __le16 size;
+ __le16 id;
+ __le16 echo;
+ __le16 handle;
+} __packed;
+
+struct dln2_response {
+ struct dln2_header hdr;
+ __le16 result;
+} __packed;
+
+#define DLN2_GENERIC_MODULE_ID 0x00
+#define DLN2_GENERIC_CMD(cmd) DLN2_CMD(cmd, DLN2_GENERIC_MODULE_ID)
+#define CMD_GET_DEVICE_VER DLN2_GENERIC_CMD(0x30)
+#define CMD_GET_DEVICE_SN DLN2_GENERIC_CMD(0x31)
+
+#define DLN2_HW_ID 0x200
+#define DLN2_USB_TIMEOUT 200 /* in ms */
+#define DLN2_MAX_RX_SLOTS 16
+#define DLN2_MAX_URBS 16
+#define DLN2_RX_BUF_SIZE 512
+
+enum dln2_handle {
+ DLN2_HANDLE_EVENT,
+ DLN2_HANDLE_CTRL,
+ DLN2_HANDLE_GPIO,
+ DLN2_HANDLE_I2C,
+ DLN2_HANDLES
+};
+
+/*
+ * Receive context used between the receive demultiplexer and the
+ * transfer routine. While sending a request the transfer routine
+ * will look for a free receive context and use it to wait for a
+ * response and to receive the URB and thus the response data.
+ */
+struct dln2_rx_context {
+ /* completion used to wait for a response */
+ struct completion done;
+
+ /* if non-NULL the URB contains the response */
+ struct urb *urb;
+
+ /* if true then this context is used to wait for a response */
+ bool in_use;
+};
+
+/*
+ * Receive contexts for a particular DLN2 module (i2c, gpio, etc.). We
+ * use the handle header field to identify the module in
+ * dln2_dev.mod_rx_slots and then the echo header field to index the
+ * slots field and find the receive context for a particular request.
+ */
+struct dln2_mod_rx_slots {
+ /* RX slots bitmap */
+ unsigned long bmap;
+
+
[PATCH v6 4/4] gpio: add support for the Diolan DLN-2 USB GPIO driver
From: Daniel Baluta
This patch adds GPIO and IRQ support for the Diolan DLN-2 GPIO module.
Information about the USB protocol interface can be found in the
Programmer's Reference Manual [1], see section 2.9 for the GPIO
module commands and responses.
[1] https://www.diolan.com/downloads/dln-api-manual.pdf
Signed-off-by: Daniel Baluta
Signed-off-by: Octavian Purdila
---
drivers/gpio/Kconfig | 12 +
drivers/gpio/Makefile| 1 +
drivers/gpio/gpio-dln2.c | 555 +++
3 files changed, 568 insertions(+)
create mode 100644 drivers/gpio/gpio-dln2.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 9de1515..44ec206 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -897,4 +897,16 @@ config GPIO_VIPERBOARD
River Tech's viperboard.h for detailed meaning
of the module parameters.
+config GPIO_DLN2
+ tristate "Diolan DLN2 GPIO support"
+ depends on MFD_DLN2
+ select GPIOLIB_IRQCHIP
+
+ help
+ Select this option to enable GPIO driver for the Diolan DLN2
+ board.
+
+ This driver can also be built as a module. If so, the module
+ will be called gpio-dln2.
+
endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 5d024e3..eaa97a0 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_GPIO_CRYSTAL_COVE) += gpio-crystalcove.o
obj-$(CONFIG_GPIO_DA9052) += gpio-da9052.o
obj-$(CONFIG_GPIO_DA9055) += gpio-da9055.o
obj-$(CONFIG_GPIO_DAVINCI) += gpio-davinci.o
+obj-$(CONFIG_GPIO_DLN2)+= gpio-dln2.o
obj-$(CONFIG_GPIO_DWAPB) += gpio-dwapb.o
obj-$(CONFIG_GPIO_EM) += gpio-em.o
obj-$(CONFIG_GPIO_EP93XX) += gpio-ep93xx.o
diff --git a/drivers/gpio/gpio-dln2.c b/drivers/gpio/gpio-dln2.c
new file mode 100644
index 000..c047f3e
--- /dev/null
+++ b/drivers/gpio/gpio-dln2.c
@@ -0,0 +1,555 @@
+/*
+ * Driver for the Diolan DLN-2 USB-GPIO adapter
+ *
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DLN2_GPIO_ID 0x01
+
+#define DLN2_GPIO_GET_PIN_COUNTDLN2_CMD(0x01, DLN2_GPIO_ID)
+#define DLN2_GPIO_SET_DEBOUNCE DLN2_CMD(0x04, DLN2_GPIO_ID)
+#define DLN2_GPIO_GET_DEBOUNCE DLN2_CMD(0x05, DLN2_GPIO_ID)
+#define DLN2_GPIO_PORT_GET_VAL DLN2_CMD(0x06, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_GET_VAL DLN2_CMD(0x0B, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_SET_OUT_VAL DLN2_CMD(0x0C, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_GET_OUT_VAL DLN2_CMD(0x0D, DLN2_GPIO_ID)
+#define DLN2_GPIO_CONDITION_MET_EV DLN2_CMD(0x0F, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_ENABLE DLN2_CMD(0x10, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_DISABLE DLN2_CMD(0x11, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_SET_DIRECTIONDLN2_CMD(0x13, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_GET_DIRECTIONDLN2_CMD(0x14, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_SET_EVENT_CFGDLN2_CMD(0x1E, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_GET_EVENT_CFGDLN2_CMD(0x1F, DLN2_GPIO_ID)
+
+#define DLN2_GPIO_EVENT_NONE 0
+#define DLN2_GPIO_EVENT_CHANGE 1
+#define DLN2_GPIO_EVENT_LVL_HIGH 2
+#define DLN2_GPIO_EVENT_LVL_LOW3
+#define DLN2_GPIO_EVENT_CHANGE_RISING 0x11
+#define DLN2_GPIO_EVENT_CHANGE_FALLING 0x21
+#define DLN2_GPIO_EVENT_MASK 0x0F
+
+#define DLN2_GPIO_MAX_PINS 32
+
+struct dln2_irq_work {
+ struct work_struct work;
+ struct dln2_gpio *dln2;
+ int pin;
+ int type;
+};
+
+struct dln2_gpio {
+ struct platform_device *pdev;
+ struct gpio_chip gpio;
+
+ /*
+* Cache pin direction to save us one transfer, since the
+* hardware has separate commands to read the in and out
+* values.
+*/
+ DECLARE_BITMAP(output_enabled, DLN2_GPIO_MAX_PINS);
+
+ DECLARE_BITMAP(irqs_masked, DLN2_GPIO_MAX_PINS);
+ DECLARE_BITMAP(irqs_enabled, DLN2_GPIO_MAX_PINS);
+ DECLARE_BITMAP(irqs_pending, DLN2_GPIO_MAX_PINS);
+ struct dln2_irq_work *irq_work;
+};
+
+struct dln2_gpio_pin {
+ __le16 pin;
+};
+
+struct dln2_gpio_pin_val {
+ __le16 pin __packed;
+ u8 value;
+};
+
+static int dln2_gpio_get_pin_count(struct platform_device *pdev)
+{
+ int ret;
+ __le16 count;
+ int len = sizeof(count);
+
+ ret = dln2_transfer(pdev, DLN2_GPIO_GET_PIN_COUNT, NULL, 0, &count,
+ &len);
+ if (ret < 0)
+ return ret;
+ if (len < sizeof(count))
+ return -EPROTO;
+
+ return le16_to_cpu(count);
+}
+
+static int dln2_
[PATCH v6 3/4] gpiolib: add irq_not_threaded flag to gpio_chip
Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set
operation but do not need a threaded irq handler.
Signed-off-by: Octavian Purdila
---
drivers/gpio/gpiolib.c | 2 +-
include/linux/gpio/driver.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 15cc0bb..3fa7e73 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -447,7 +447,7 @@ static int gpiochip_irq_map(struct irq_domain *d, unsigned
int irq,
irq_set_lockdep_class(irq, &gpiochip_irq_lock_class);
irq_set_chip_and_handler(irq, chip->irqchip, chip->irq_handler);
/* Chips that can sleep need nested thread handlers */
- if (chip->can_sleep)
+ if (chip->can_sleep && !chip->irq_not_threaded)
irq_set_nested_thread(irq, 1);
#ifdef CONFIG_ARM
set_irq_flags(irq, IRQF_VALID);
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index e78a237..44161ac 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -56,6 +56,8 @@ struct seq_file;
* as the chip access may sleep when e.g. reading out the IRQ status
* registers.
* @exported: flags if the gpiochip is exported for use from sysfs. Private.
+ * @irq_not_threaded: flag must be set if @can_sleep is set but the
+ * IRQs don't need to be threaded
*
* A gpio_chip can help platforms abstract various sources of GPIOs so
* they can all be accessed through a common programing interface.
@@ -101,6 +103,7 @@ struct gpio_chip {
struct gpio_desc*desc;
const char *const *names;
boolcan_sleep;
+ boolirq_not_threaded;
boolexported;
#ifdef CONFIG_GPIOLIB_IRQCHIP
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/3] mfd: viperboard: remove unused platform_device
On Thu, Sep 25, 2014 at 06:41:37PM +0300, Octavian Purdila wrote:
> Remove pdev field from struct vpbrd as it is not used in the MFD
> driver or in any of the sub-drivers.
>
> Signed-off-by: Octavian Purdila
Reviewed-by: Johan Hovold
> ---
> drivers/mfd/viperboard.c | 1 -
> include/linux/mfd/viperboard.h | 1 -
> 2 files changed, 2 deletions(-)
>
> diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
> index 0d46d05..f4dcba6 100644
> --- a/drivers/mfd/viperboard.c
> +++ b/drivers/mfd/viperboard.c
> @@ -73,7 +73,6 @@ static int vprbrd_probe(struct usb_interface *interface,
>
> /* save our data pointer in this interface device */
> usb_set_intfdata(interface, vb);
> - dev_set_drvdata(&vb->pdev.dev, vb);
>
> /* get version information, major first, minor then */
> pipe = usb_rcvctrlpipe(vb->usb_dev, 0);
> diff --git a/include/linux/mfd/viperboard.h b/include/linux/mfd/viperboard.h
> index af928d0..afc14ed 100644
> --- a/include/linux/mfd/viperboard.h
> +++ b/include/linux/mfd/viperboard.h
> @@ -104,7 +104,6 @@ struct vprbrd {
> struct usb_device *usb_dev; /* the usb device for this device */
> struct mutex lock;
> u8 *buf;
> - struct platform_device pdev;
> };
>
> #endif /* __MFD_VIPERBOARD_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 v4 2/3] mfd: viperboard: switch to devm_ allocation
On Thu, Sep 25, 2014 at 06:41:36PM +0300, Octavian Purdila wrote:
> Switch to using devm_ allocation to simplify the error path. Also
> remove a redundant OOM error message.
>
> Signed-off-by: Octavian Purdila
Reviewed-by: Johan Hovold
> ---
> drivers/mfd/viperboard.c | 23 +++
> 1 file changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
> index 5f62f4e..0d46d05 100644
> --- a/drivers/mfd/viperboard.c
> +++ b/drivers/mfd/viperboard.c
> @@ -58,17 +58,14 @@ static int vprbrd_probe(struct usb_interface *interface,
> int pipe, ret;
>
> /* allocate memory for our device state and initialize it */
> - vb = kzalloc(sizeof(*vb), GFP_KERNEL);
> - if (vb == NULL) {
> - dev_err(&interface->dev, "Out of memory\n");
> + vb = devm_kzalloc(&interface->dev, sizeof(*vb), GFP_KERNEL);
> + if (vb == NULL)
> return -ENOMEM;
> - }
>
> - vb->buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
> - if (vb->buf == NULL) {
> - ret = -ENOMEM;
> - goto error;
> - }
> + vb->buf = devm_kzalloc(&interface->dev,
> +sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
> + if (vb->buf == NULL)
> + return -ENOMEM;
>
> mutex_init(&vb->lock);
>
> @@ -109,11 +106,7 @@ static int vprbrd_probe(struct usb_interface *interface,
> return 0;
>
> error:
> - if (vb) {
> - usb_put_dev(vb->usb_dev);
> - kfree(vb->buf);
> - kfree(vb);
> - }
> + usb_put_dev(vb->usb_dev);
>
> return ret;
> }
> @@ -125,8 +118,6 @@ static void vprbrd_disconnect(struct usb_interface
> *interface)
> mfd_remove_devices(&interface->dev);
> usb_set_intfdata(interface, NULL);
> usb_put_dev(vb->usb_dev);
> - kfree(vb->buf);
> - kfree(vb);
>
> dev_dbg(&interface->dev, "disconnected\n");
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/3] mfd: viperboard: allocate I/O buffer separately
On Thu, Sep 25, 2014 at 06:41:35PM +0300, Octavian Purdila wrote:
> Currently the I/O buffer is allocated part of the device status
> structure, potentially sharing the same cache line with other members
> in this structure.
>
> Allocate the buffer separately, to avoid the I/O operations corrupting
> the device status structure due to cache line sharing.
>
> Compiled tested only as I don't have access to hardware.
>
> Signed-off-by: Octavian Purdila
Reviewed-by: Johan Hovold
> ---
> drivers/mfd/viperboard.c | 8
> include/linux/mfd/viperboard.h | 2 +-
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
> index e00f534..5f62f4e 100644
> --- a/drivers/mfd/viperboard.c
> +++ b/drivers/mfd/viperboard.c
> @@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface *interface,
> return -ENOMEM;
> }
>
> + vb->buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
> + if (vb->buf == NULL) {
> + ret = -ENOMEM;
> + goto error;
> + }
> +
> mutex_init(&vb->lock);
>
> vb->usb_dev = usb_get_dev(interface_to_usbdev(interface));
> @@ -105,6 +111,7 @@ static int vprbrd_probe(struct usb_interface *interface,
> error:
> if (vb) {
> usb_put_dev(vb->usb_dev);
> + kfree(vb->buf);
> kfree(vb);
> }
>
> @@ -118,6 +125,7 @@ static void vprbrd_disconnect(struct usb_interface
> *interface)
> mfd_remove_devices(&interface->dev);
> usb_set_intfdata(interface, NULL);
> usb_put_dev(vb->usb_dev);
> + kfree(vb->buf);
> kfree(vb);
>
> dev_dbg(&interface->dev, "disconnected\n");
> diff --git a/include/linux/mfd/viperboard.h b/include/linux/mfd/viperboard.h
> index 1934528..af928d0 100644
> --- a/include/linux/mfd/viperboard.h
> +++ b/include/linux/mfd/viperboard.h
> @@ -103,7 +103,7 @@ struct vprbrd_i2c_addr_msg {
> struct vprbrd {
> struct usb_device *usb_dev; /* the usb device for this device */
> struct mutex lock;
> - u8 buf[sizeof(struct vprbrd_i2c_write_msg)];
> + u8 *buf;
> struct platform_device pdev;
> };
--
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
project wide: git config entry for [diff] renames=true
On Thu, 2014-09-25 at 17:03 +0200, Greg Kroah-Hartman wrote: > In the future, please generate a git "move" diff, which makes it easier > to review, and prove that nothing really changed. It also helps if the > file is a bit different from what you diffed against, which in my case, > was true. Maybe it'd be possible to add [diff] renames = true to the .git/config file. but I don't find a mechanism to add anything to the .git/config and have it be pulled. -- 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 v4 0/3] mfd: viperboard: fix cache line sharing and cleanups
Changes since v3: * undo a whitespace change * one more error path cleanup * add commit body to the last patch Changes since v2: * switch to using devm_ allocation * remove unused platform_device Changes since v1: * split cache sharing fix and cleanups into separate patches Octavian Purdila (3): mfd: viperboard: allocate I/O buffer separately mfd: viperboard: switch to devm_ allocation mfd: viperboard: remove unused platform_device drivers/mfd/viperboard.c | 18 -- include/linux/mfd/viperboard.h | 3 +-- 2 files changed, 9 insertions(+), 12 deletions(-) -- 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 v4 1/3] mfd: viperboard: allocate I/O buffer separately
Currently the I/O buffer is allocated part of the device status
structure, potentially sharing the same cache line with other members
in this structure.
Allocate the buffer separately, to avoid the I/O operations corrupting
the device status structure due to cache line sharing.
Compiled tested only as I don't have access to hardware.
Signed-off-by: Octavian Purdila
---
drivers/mfd/viperboard.c | 8
include/linux/mfd/viperboard.h | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index e00f534..5f62f4e 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface *interface,
return -ENOMEM;
}
+ vb->buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
+ if (vb->buf == NULL) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
mutex_init(&vb->lock);
vb->usb_dev = usb_get_dev(interface_to_usbdev(interface));
@@ -105,6 +111,7 @@ static int vprbrd_probe(struct usb_interface *interface,
error:
if (vb) {
usb_put_dev(vb->usb_dev);
+ kfree(vb->buf);
kfree(vb);
}
@@ -118,6 +125,7 @@ static void vprbrd_disconnect(struct usb_interface
*interface)
mfd_remove_devices(&interface->dev);
usb_set_intfdata(interface, NULL);
usb_put_dev(vb->usb_dev);
+ kfree(vb->buf);
kfree(vb);
dev_dbg(&interface->dev, "disconnected\n");
diff --git a/include/linux/mfd/viperboard.h b/include/linux/mfd/viperboard.h
index 1934528..af928d0 100644
--- a/include/linux/mfd/viperboard.h
+++ b/include/linux/mfd/viperboard.h
@@ -103,7 +103,7 @@ struct vprbrd_i2c_addr_msg {
struct vprbrd {
struct usb_device *usb_dev; /* the usb device for this device */
struct mutex lock;
- u8 buf[sizeof(struct vprbrd_i2c_write_msg)];
+ u8 *buf;
struct platform_device pdev;
};
--
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 v4 3/3] mfd: viperboard: remove unused platform_device
Remove pdev field from struct vpbrd as it is not used in the MFD
driver or in any of the sub-drivers.
Signed-off-by: Octavian Purdila
---
drivers/mfd/viperboard.c | 1 -
include/linux/mfd/viperboard.h | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index 0d46d05..f4dcba6 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -73,7 +73,6 @@ static int vprbrd_probe(struct usb_interface *interface,
/* save our data pointer in this interface device */
usb_set_intfdata(interface, vb);
- dev_set_drvdata(&vb->pdev.dev, vb);
/* get version information, major first, minor then */
pipe = usb_rcvctrlpipe(vb->usb_dev, 0);
diff --git a/include/linux/mfd/viperboard.h b/include/linux/mfd/viperboard.h
index af928d0..afc14ed 100644
--- a/include/linux/mfd/viperboard.h
+++ b/include/linux/mfd/viperboard.h
@@ -104,7 +104,6 @@ struct vprbrd {
struct usb_device *usb_dev; /* the usb device for this device */
struct mutex lock;
u8 *buf;
- struct platform_device pdev;
};
#endif /* __MFD_VIPERBOARD_H__ */
--
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 v4 2/3] mfd: viperboard: switch to devm_ allocation
Switch to using devm_ allocation to simplify the error path. Also
remove a redundant OOM error message.
Signed-off-by: Octavian Purdila
---
drivers/mfd/viperboard.c | 23 +++
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index 5f62f4e..0d46d05 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -58,17 +58,14 @@ static int vprbrd_probe(struct usb_interface *interface,
int pipe, ret;
/* allocate memory for our device state and initialize it */
- vb = kzalloc(sizeof(*vb), GFP_KERNEL);
- if (vb == NULL) {
- dev_err(&interface->dev, "Out of memory\n");
+ vb = devm_kzalloc(&interface->dev, sizeof(*vb), GFP_KERNEL);
+ if (vb == NULL)
return -ENOMEM;
- }
- vb->buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
- if (vb->buf == NULL) {
- ret = -ENOMEM;
- goto error;
- }
+ vb->buf = devm_kzalloc(&interface->dev,
+ sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
+ if (vb->buf == NULL)
+ return -ENOMEM;
mutex_init(&vb->lock);
@@ -109,11 +106,7 @@ static int vprbrd_probe(struct usb_interface *interface,
return 0;
error:
- if (vb) {
- usb_put_dev(vb->usb_dev);
- kfree(vb->buf);
- kfree(vb);
- }
+ usb_put_dev(vb->usb_dev);
return ret;
}
@@ -125,8 +118,6 @@ static void vprbrd_disconnect(struct usb_interface
*interface)
mfd_remove_devices(&interface->dev);
usb_set_intfdata(interface, NULL);
usb_put_dev(vb->usb_dev);
- kfree(vb->buf);
- kfree(vb);
dev_dbg(&interface->dev, "disconnected\n");
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] mfd: viperboard: remove unused platform_device
On Thu, Sep 25, 2014 at 06:29:48PM +0300, Octavian Purdila wrote: > On Thu, Sep 25, 2014 at 6:07 PM, Johan Hovold wrote: > > Still feels like the kind of clean up that should have a Tested-by. > > Unfortunately I do not have access to hardware to test. On second thought, compile-testing should be enough. But please add a proper commit message. 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 v3 3/3] mfd: viperboard: remove unused platform_device
On Thu, Sep 25, 2014 at 6:07 PM, Johan Hovold wrote:
> On Thu, Sep 25, 2014 at 05:43:17PM +0300, Octavian Purdila wrote:
>
> Where's the commit message body?
>
>> Signed-off-by: Octavian Purdila
>> ---
>> drivers/mfd/viperboard.c | 1 -
>> include/linux/mfd/viperboard.h | 1 -
>> 2 files changed, 2 deletions(-)
>>
>> diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
>> index 57fac1d..1e7c316 100644
>> --- a/drivers/mfd/viperboard.c
>> +++ b/drivers/mfd/viperboard.c
>> @@ -74,7 +74,6 @@ static int vprbrd_probe(struct usb_interface *interface,
>>
>> /* save our data pointer in this interface device */
>> usb_set_intfdata(interface, vb);
>> - dev_set_drvdata(&vb->pdev.dev, vb);
>
> This looks ok, as the subdrivers are accessing the driver data via the
> usb-interface (their parent) device.
>
> You verified that too, right?
Yes, I checked that the pdev field of struct vpbrd is not used in the
rest of the drivers.
>
>>
>> /* get version information, major first, minor then */
>> pipe = usb_rcvctrlpipe(vb->usb_dev, 0);
>> diff --git a/include/linux/mfd/viperboard.h b/include/linux/mfd/viperboard.h
>> index af928d0..afc14ed 100644
>> --- a/include/linux/mfd/viperboard.h
>> +++ b/include/linux/mfd/viperboard.h
>> @@ -104,7 +104,6 @@ struct vprbrd {
>> struct usb_device *usb_dev; /* the usb device for this device */
>> struct mutex lock;
>> u8 *buf;
>> - struct platform_device pdev;
>> };
>>
>> #endif /* __MFD_VIPERBOARD_H__ */
>
> Still feels like the kind of clean up that should have a Tested-by.
>
Unfortunately I do not have access to hardware to test.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: core: log higher level message on malformed LANGID descriptor
Commit 0cce2eda19923e5e5ccc8b042dec5af87b3ffad0 USB: fix LANGID=0 regression defaults to a langid of 0x0409 if it's not properly implemented by the device. Explain with a higher level error message what this means. Signed-off-by: Scot Doyle --- drivers/usb/core/message.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 0c8a7fc..f7b7713 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -770,9 +770,7 @@ static int usb_get_langid(struct usb_device *dev, unsigned char *tbuf) dev->string_langid = 0x0409; dev->have_langid = 1; dev_err(&dev->dev, - "string descriptor 0 malformed (err = %d), " - "defaulting to 0x%04x\n", - err, dev->string_langid); + "language id specifier not provided by device, defaulting to English\n"); return 0; } -- 2.1.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: Poor performance with USB 1.1 drive connected to USB 3.0 port
On Wed, 24 Sep 2014, Mark Knibbs wrote: > On Wed, 24 Sep 2014 11:37:11 -0400 (EDT) > Alan Stern wrote: > > > On Wed, 24 Sep 2014, Mark Knibbs wrote: > > > > > I did some benchmarks to check the maximum transfer rate of a USB-to-SCSI > > > converter. The converter is USB 1.1, so limited to the 12Mbps full speed > > > rate. The performance when connected to a USB 3.0 port is significantly > > > worse than when connected to a USB 2.0 port, about 26.5% slower > > > (0.63MB/sec > > > vs 0.86MB/sec). > > > > > > The USB 3.0 port is provided by an ExpressCard which has a Renesas > > > controller. It doesn't seem to be defective because I can get 138MB/sec > > > from a USB 3.0 hard disk. lspci shows > > > 05:00.0 USB Controller: Renesas Technology Corp. Device 0015 (rev 02) > > > > > > I didn't test with an unmodified mainline kernel, but the issue is present > > > with both Ubuntu kernel 3.0 and Debian 3.14 (booted from Kali Linux live > > > DVD). > > > > > > I used sg_rbuf from the sg3-utils package. That issues READ BUFFER > > > commands > > > instead of reading data from disk, so the result should be closer to the > > > maximum achievable. The USB-SCSI converter was a Newer Technology uSCSI > > > connected to an HP 2600fx MO drive. > > > ... > > > Any ideas what the reason for the discrepancy might be? Can anyone else > > > reproduce it? I guess USB 1.1-only devices aren't that common nowadays. > > > > You might be able to get more detailed timings if you collect usbmon > > traces of the two tests. They won't answer your question but they may > > help point in a particular direction. > > > > The xhci-hcd driver has been under active development. For the best > > results, you really should use the most up-to-date version of the > > kernel. > > Thanks, I booted a Knoppix live DVD (kernel 3.16.2) and ran the tests > again. The USB 3.0 port rate is now about 0.75MB/sec; some improvement but > still significantly lower than the USB 2.0 port figure. > > Connected to USB 2.0: > # sg_rbuf --buffer=524288 -q -t -v /dev/sg2 > Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 > READ BUFFER reports: buffer capacity=983040, offset boundary=0 > time to read data from buffer was 242.800446 secs, 0.86 MB/sec > Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes) > > Connected to USB 3.0: > # sg_rbuf --buffer=524288 -q -t -v /dev/sg2 > Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 > READ BUFFER reports: buffer capacity=983040, offset boundary=0 > time to read data from buffer was 280.701167 secs, 0.75 MB/sec > Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes) > > > I captured usbmon logs for this command: > # sg_rbuf --buffer=524288 -q -t -v --size=16MiB /dev/sg2 > > Each is a little under 16KB. Hopefully it's not too annoying to paste them > here... Well, the traces are a little informative but not very. The transfers are broken up into 524288-byte segments (not surprising, since that's what you told sg_rbuf to do). With the USB-2 controller, each transfer took a uniform 606 ms. With the USB-3 controller, the transfers varied between roughly 650 ms and 770 ms. You can calculate these values for yourself, if you are interested. Here's how. The submission and completion of each 512-KB transfer are indicated by lines looking like these: > USB 2.0 log: ... > 8800ab56ee80 2411529751 S Bi:5:008:2 -115 524288 < > 8800ab56ee80 2412135377 C Bi:5:008:2 0 524288 = > with 524288 in the sixth column. The second column is a timestamp, in microseconds. Thus the duration of this transfer was 2412135377 - 2411529751 = 605626 us, or about 606 ms. By comparison, > USB 3.0 log: ... > 880135223040 2116432829 S Bi:8:002:2 -115 524288 < > 880135223040 2117202040 C Bi:8:002:2 0 524288 = > This took 2117202040 - 2116432829 = 769211 us, or about 769 ms. There are other differences, due to the overhead in starting and finishing each transfer, but this is the largest component. There's no telling the reason for this difference. It's got to be a hardware issue, though, not a software problem. Maybe your xHCI controller just isn't optimized for carrying out full-speed transfers. 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 v3 3/3] mfd: viperboard: remove unused platform_device
On Thu, Sep 25, 2014 at 05:43:17PM +0300, Octavian Purdila wrote:
Where's the commit message body?
> Signed-off-by: Octavian Purdila
> ---
> drivers/mfd/viperboard.c | 1 -
> include/linux/mfd/viperboard.h | 1 -
> 2 files changed, 2 deletions(-)
>
> diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
> index 57fac1d..1e7c316 100644
> --- a/drivers/mfd/viperboard.c
> +++ b/drivers/mfd/viperboard.c
> @@ -74,7 +74,6 @@ static int vprbrd_probe(struct usb_interface *interface,
>
> /* save our data pointer in this interface device */
> usb_set_intfdata(interface, vb);
> - dev_set_drvdata(&vb->pdev.dev, vb);
This looks ok, as the subdrivers are accessing the driver data via the
usb-interface (their parent) device.
You verified that too, right?
>
> /* get version information, major first, minor then */
> pipe = usb_rcvctrlpipe(vb->usb_dev, 0);
> diff --git a/include/linux/mfd/viperboard.h b/include/linux/mfd/viperboard.h
> index af928d0..afc14ed 100644
> --- a/include/linux/mfd/viperboard.h
> +++ b/include/linux/mfd/viperboard.h
> @@ -104,7 +104,6 @@ struct vprbrd {
> struct usb_device *usb_dev; /* the usb device for this device */
> struct mutex lock;
> u8 *buf;
> - struct platform_device pdev;
> };
>
> #endif /* __MFD_VIPERBOARD_H__ */
Still feels like the kind of clean up that should have a Tested-by.
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: MUSB: extra cppi irq?
Hi Bin,
On Wed, Sep 24, 2014 at 01:55:14PM -0500, Bin Liu wrote:
> Hi Felipe and all,
>
> The musb driver musb_host_tx() has the following:
>
> 1244 /* with CPPI, DMA sometimes triggers "extra" irqs */
> 1245 if (!urb) {
> 1246 dev_dbg(musb->controller, "extra TX%d ready, csr
> %04x\n", epnum, tx_csr);
> 1247 return;
> 1248 }
>
> and
>
> 1321 /* second cppi case */
> 1322 if (dma_channel_status(dma) == MUSB_DMA_STATUS_BUSY) {
> 1323 dev_dbg(musb->controller, "extra TX%d ready, csr
> %04x\n", epnum, tx_csr);
> 1324 return;
> 1325 }
>
> which come with the very first commit of musb driver and never got
> changed. Is there any more information about the 'extra' irqs? and
> what is the 'second cppi case'?
I'm afraid the details of that were long after Dave B left us,
unfortunately. He was the only one dealing with MUSB on those early
phases.
> I ran into this problem and musb stops working after hits here.
>
> [13542.933563] musb-hdrc musb-hdrc.1: qh c261ad80 urb eec92bc0 dev4
> ep7in-bulk, hw_ep 2, c2599000/4096
> [13542.953552] musb-hdrc musb-hdrc.1: usbintr (0) epintr(4)
> [13542.953582] musb-hdrc musb-hdrc.1: extra TX2 ready, csr 0004
>
> I bet this 'extra TX2 ready' log is printed by the first case, because
> only RX2 is used in this test, and TX2 is never got used, so there
> should not be any urb in TX2 queue. I am still waiting for another
> test failure to confirm this, but I am trying to understand why TX2
> interrupt happened...which seems to be related RX2 request.
>
> Thanks for any help.
unfortunately no clue on what's going on :-(
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v6 3/4] usb: Rename usb-common.c
On Wed, Sep 24, 2014 at 10:43:20PM +0200, Michal Sojka wrote: > In the next commit, we will want the usb-common module to be composed of > two object files. Since Kbuild cannot "append" another object to an > existing one, we need to rename usb-common.c to something > else (common.c) and create usb-common.o by linking the wanted objects > together. Currently, usb-common.o comprises only common.o. > > Signed-off-by: Michal Sojka > Acked-by: Felipe Balbi > Tested-by: Felipe Balbi > --- > drivers/usb/common/Makefile | 4 +- > drivers/usb/common/common.c | 144 > > drivers/usb/common/usb-common.c | 144 > > 3 files changed, 147 insertions(+), 145 deletions(-) > create mode 100644 drivers/usb/common/common.c > delete mode 100644 drivers/usb/common/usb-common.c In the future, please generate a git "move" diff, which makes it easier to review, and prove that nothing really changed. It also helps if the file is a bit different from what you diffed against, which in my case, was true. 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 v3 2/3] mfd: viperboard: switch to devm_ allocation
On Thu, Sep 25, 2014 at 05:43:16PM +0300, Octavian Purdila wrote:
> Switch to using devm_ allocation to simplify the error path. Also
> remove a redundant OOM error message.
>
> Signed-off-by: Octavian Purdila
> ---
> drivers/mfd/viperboard.c | 18 +-
> 1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
> index 5f62f4e..57fac1d 100644
> --- a/drivers/mfd/viperboard.c
> +++ b/drivers/mfd/viperboard.c
> @@ -53,18 +53,16 @@ static int vprbrd_probe(struct usb_interface *interface,
> const struct usb_device_id *id)
> {
> struct vprbrd *vb;
> -
Don't include random whitespace changes in your patches.
> u16 version = 0;
> int pipe, ret;
>
> /* allocate memory for our device state and initialize it */
> - vb = kzalloc(sizeof(*vb), GFP_KERNEL);
> - if (vb == NULL) {
> - dev_err(&interface->dev, "Out of memory\n");
> + vb = devm_kzalloc(&interface->dev, sizeof(*vb), GFP_KERNEL);
> + if (vb == NULL)
> return -ENOMEM;
> - }
>
> - vb->buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
> + vb->buf = devm_kzalloc(&interface->dev,
> +sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
> if (vb->buf == NULL) {
> ret = -ENOMEM;
> goto error;
You should just return -ENOMEM here.
> @@ -109,11 +107,7 @@ static int vprbrd_probe(struct usb_interface *interface,
> return 0;
>
> error:
> - if (vb) {
> - usb_put_dev(vb->usb_dev);
> - kfree(vb->buf);
> - kfree(vb);
> - }
> + usb_put_dev(vb->usb_dev);
>
> return ret;
> }
> @@ -125,8 +119,6 @@ static void vprbrd_disconnect(struct usb_interface
> *interface)
> mfd_remove_devices(&interface->dev);
> usb_set_intfdata(interface, NULL);
> usb_put_dev(vb->usb_dev);
> - kfree(vb->buf);
> - kfree(vb);
>
> dev_dbg(&interface->dev, "disconnected\n");
> }
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 v6 0/4] LED triggers for USB host and device
On Thu, Sep 25, 2014 at 08:56:32AM -0500, Felipe Balbi wrote: > Hi, > > On Thu, Sep 25, 2014 at 12:36:23PM +0200, Greg Kroah-Hartman wrote: > > On Wed, Sep 24, 2014 at 06:15:50PM -0500, Felipe Balbi wrote: > > > On Wed, Sep 24, 2014 at 05:18:30PM -0500, Felipe Balbi wrote: > > > > On Wed, Sep 24, 2014 at 11:41:55PM +0200, Greg Kroah-Hartman wrote: > > > > > On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote: > > > > > > On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote: > > > > > > > This adds LED triggers for USB host and device. First two patches > > > > > > > refactor UDC drivers as requested by Felipe Balbi, the next > > > > > > > renames a > > > > > > > file and the last is the actual implementation of the LED > > > > > > > triggers. > > > > > > > > > > > > > > Changes from v5: > > > > > > > - Refactoring of USB gadget completion split into two patches > > > > > > > (Filipe > > > > > > > Balbi) > > > > > > > - Removed check for non-NULL req->completion (Filipe Balbi) > > > > > > > > > > > > > > Changes from v4: > > > > > > > - Added performance numbers to the commit message of the last > > > > > > > patch > > > > > > > (greg k-h). > > > > > > > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h). > > > > > > > - Used proper coding style for switch statement (greg k-h). > > > > > > > - Added comment about NULL argument (greg k-h). > > > > > > > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h). > > > > > > > - Both triggers are now registerd even if host or gagdet subsystem > > > > > > > is not enabled (Bryan Wu, greg k-h). > > > > > > > > > > > > > > Changes from v3: > > > > > > > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA > > > > > > > conditioned block. > > > > > > > - Added kernel-doc for usb_gadget_giveback_request() (Felipe > > > > > > > Balbi). > > > > > > > - Removed outdated comment (Alan Stern). > > > > > > > - req->complete == NULL is now a bug. Previously, this was ignored > > > > > > > (Alan Stern). > > > > > > > - File rename moved to a separate commit (greg k-h). > > > > > > > > > > > > > > Changes from v2: > > > > > > > - Host/gadget triggers merged to a single file in usb/common/ > > > > > > > (Felipe > > > > > > > Balbi). > > > > > > > - UDC drivers refactored so that LED trigger works for all of > > > > > > > them. > > > > > > > > > > > > > > Changes from v1: > > > > > > > - Moved from drivers/leds/ to drivers/usb/. > > > > > > > - Improved Kconfig help. > > > > > > > - Linked with other modules rather than being standalone modules. > > > > > > > > > > > > > > Michal Sojka (4): > > > > > > > usb: gadget: Introduce usb_gadget_giveback_request() > > > > > > > usb: gadget: Refactor request completion > > > > > > > usb: Rename usb-common.c > > > > > > > usb: Add LED triggers for USB activity > > > > > > > > > > > > Greg, how do you want to handle this one as it touched both host and > > > > > > gadget ? I could wait until you apply this series to your usb-next > > > > > > and > > > > > > based my pull request on that. That'll work as long as it doesn't > > > > > > take > > > > > > too long for this to be merged. > > > > > > > > > > I can just take this now, for 3.18-rc1, you've already sent me your > > > > > patches for that release, so there shouldn't be any conflicts, right? > > > > > > > > that's better. Can you wait 1 day though ? I can run these through my > > > > build tests. > > > > > > running my build tests on top of v3.17-rc6 + these 4 patches. I'll > > > report back tomorrow. From a code perspective, though, they look > > > alright. > > > > Yes, will wait, thanks. > > nothing interesting on my build tests. I have also tested the series > with my AM437x SK and I can see LED blinking with usb-gadget and > usb-host triggers. > > For the whole series: > > Tested-by: Felipe Balbi Thanks for testing and letting me know, will queue it up now. 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 v6 partial v2 1/3] usb: gadget: pxa27x_udc: prepare device-tree support
Hi,
On Thu, Sep 25, 2014 at 03:44:34PM +0200, robert.jarz...@free.fr wrote:
> - Mail original -
> De: "Linus Walleij"
> À: "Robert Jarzmik"
> Cc: "Felipe Balbi" , linux-usb@vger.kernel.org
> Envoyé: Jeudi 25 Septembre 2014 15:10:16
> Objet: Re: [PATCH v6 partial v2 1/3] usb: gadget: pxa27x_udc: prepare
> device-tree support
>
> In that case, that whole clause is actually using the old API without
> explicit request, so you can just do this:
>
> if (mach) {
> /* This clause will go away when we have transitioned to
> just using descriptors */
> if (devm_gpio_request_one(&pdev->dev, mach->gpio_pullup,
> mach->gpio_pullup_inverted ?
> GPIOF_ACTIVE_LOW : 0,
> "my pin"))
> goto err;
> udc->gpiod = gpio_to_desc(mach->gpio_pullup);
> udc->mach = mach;
> } else {
> ...
> }
>
> Excellent, sold. I'll put that in partial v3 then.
hey, sounds good to me too :-)
> Could you also review patch 1/3 while at it ;) ?
>
> Felipe, I'll wait for a couple of days to release partial v3 to give
> you time to review the patches, let's say next wednesday ?
sure, but I'm liking where this is going. Looks like a nice transition
path.
--
balbi
signature.asc
Description: Digital signature
Re: [RFC PATCH 1/4] usb: dwc3: add AMD NL support
On Thu, Sep 25, 2014 at 03:21:44PM +0800, Huang Rui wrote:
> Add PCI device ID of AMD NL.
>
> Signed-off-by: Huang Rui
> ---
> drivers/usb/dwc3/dwc3-pci.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> index 436fb08..cebbd01 100644
> --- a/drivers/usb/dwc3/dwc3-pci.c
> +++ b/drivers/usb/dwc3/dwc3-pci.c
> @@ -30,6 +30,7 @@
> #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3 0xabcd
> #define PCI_DEVICE_ID_INTEL_BYT 0x0f37
> #define PCI_DEVICE_ID_INTEL_MRFLD0x119e
> +#define PCI_DEVICE_ID_AMD_NL 0x7912
>
> struct dwc3_pci {
> struct device *dev;
> @@ -183,6 +184,7 @@ static const struct pci_device_id dwc3_pci_id_table[] = {
> },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT), },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRFLD), },
> + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_NL), },
this patch causes a build breakage.
--
balbi
signature.asc
Description: Digital signature
Re: [RFC PATCH 4/4] usb: dwc3: introduce dual role switch function with debugfs
Hi,
On Thu, Sep 25, 2014 at 03:21:47PM +0800, Huang Rui wrote:
> This patch implemented a feature to dynamic switch to host or device
> role under debugfs for some physical limitation that unable to
> leverage connector A/B cables (ID pin) to change roles.
>
> The default role should be set as OTG mode. Then use below commands:
>
> [1] switch to host:
> echo host > /sys/kernel/debug/dwc3.0.auto/mode
>
> [2] switch to device:
> echo device > /sys/kernel/debug/dwc3.0.auto/mode
>
> [3] switch to otg (default mode):
> echo otg > /sys/kernel/debug/dwc3.0.auto/mode
thanks, but not thanks. This is not how things should be done.
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 6138c5d..7b50584 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -61,7 +61,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
> * dwc3_core_soft_reset - Issues core soft reset and PHY reset
> * @dwc: pointer to our context structure
> */
> -static int dwc3_core_soft_reset(struct dwc3 *dwc)
> +int dwc3_core_soft_reset(struct dwc3 *dwc)
this should not be exposed.
> @@ -228,7 +228,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc,
> unsigned length)
> *
> * Returns 0 on success otherwise negative errno.
> */
> -static int dwc3_event_buffers_setup(struct dwc3 *dwc)
> +int dwc3_event_buffers_setup(struct dwc3 *dwc)
this should not be exposed.
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index a1c7dc5..3a30f33 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -643,6 +643,8 @@ struct dwc3_scratchpad_array {
> * @maximum_speed: maximum speed requested (mainly for testing purposes)
> * @revision: revision register contents
> * @quirks: represents different SOCs hardware work-arounds and quirks
> + * @has_gadget: true when gadget is initialized
> + * @has_xhci: true when xhci is initialized
you shouldn't need these.
> diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> index 9ac37fe..76384df 100644
> --- a/drivers/usb/dwc3/debugfs.c
> +++ b/drivers/usb/dwc3/debugfs.c
> @@ -32,6 +32,7 @@
> #include "gadget.h"
> #include "io.h"
> #include "debug.h"
> +#include "drd.h"
and debugfs is definitely *not* the way to get this going.
What you need here is something to talk to usbcore and udc-core and
orchestrate the mode change through usb_add_hcd()/usb_add_gadget_udc()
and their counterparts.
George Cherian is already working on a version of that.
--
balbi
signature.asc
Description: Digital signature
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi,
On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index b0f4d52..6138c5d 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> }
>
> /**
> + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> + * @dwc: Pointer to our controller context structure
> + */
> +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> +{
> + u32 reg = 0;
> +
> + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK | DWC3_GUSB3PIPECTL_UX_EXITINPX
> + | DWC3_GUSB3PIPECTL_UX_EXITINPX | DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> + | DWC3_GUSB3PIPECTL_DEPOCHANGE | DWC3_GUSB3PIPECTL_RX_DETOPOLL;
UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
erratum before I can accept it. You have also duplicated the bit in this
initialization.
U1U2EXITFAIL -> also a workaround bit and I need to see an erratum.
RX_DETOPOLL -> it seems like it's safe to leave this one out as it can
only be proven to work with this bit after going through full USB
certification.
> + reg |= DWC3_GUSB3PIPECTL_DEP1P2P3(1) | DWC3_GUSB3PIPECTL_TX_DEEPH(1);
DEP1P2P3 and DEP0CHANGE seem to be required only for faulty PHYs too, I
need to see an erratum.
> + dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> +
> + mdelay(100);
> +}
> +
> +/**
> * dwc3_free_one_event_buffer - Frees one event buffer
> * @dwc: Pointer to our controller context structure
> * @evt: Pointer to event buffer to be freed
> @@ -412,9 +431,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
> if (ret)
> goto err0;
>
> + if (dwc->quirks & DWC3_AMD_NL_PLAT)
> + dwc3_core_nl_set_pipe3(dwc);
> +
> reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> +
> reg &= ~DWC3_GCTL_SCALEDOWN_MASK;
> - reg &= ~DWC3_GCTL_DISSCRAMBLE;
> +
> + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> + reg |= DWC3_GCTL_DISSCRAMBLE;
you're disabling scrambling ? What about EMI ? Why doesn't your device
work with scrambling ? I need to see an erratum before I can accept
this.
> + reg |= DWC3_GCTL_U2EXIT_LFPS;
hmm, seems like this bit was added due to a faulty host which was found.
I need to see an erratum before I can accept this.
> + reg |= DWC3_GCTL_GBLHIBERNATIONEN;
looks like this should always be set for all versions of the core
configured with hibernation.
> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> index cebbd01..7f471ff 100644
> --- a/drivers/usb/dwc3/dwc3-pci.c
> +++ b/drivers/usb/dwc3/dwc3-pci.c
> @@ -25,6 +25,9 @@
> #include
> #include
>
> +#include "platform_data.h"
> +#include "core.h"
you should never need to include "core.h", why are you including
"core.h" ???
> @@ -102,6 +105,9 @@ static int dwc3_pci_probe(struct pci_dev *pci,
> struct dwc3_pci *glue;
> int ret;
> struct device *dev = &pci->dev;
> + struct dwc3_platform_data dwc3_pdata;
> +
> + memset(&dwc3_pdata, 0x00, sizeof(dwc3_pdata));
>
> glue = devm_kzalloc(dev, sizeof(*glue), GFP_KERNEL);
> if (!glue)
> @@ -148,6 +154,14 @@ static int dwc3_pci_probe(struct pci_dev *pci,
>
> pci_set_drvdata(pci, glue);
>
> + if (pci->vendor == PCI_VENDOR_ID_AMD &&
> + pci->device == PCI_DEVICE_ID_AMD_NL)
> + dwc3_pdata.quirks |= DWC3_AMD_NL_PLAT;
this looks wrong. It looks like you need several quirk bits for each of
the workarounds you need. Having a single bit for "my device" is wrong
and if your next device fixes one or two of these errata, you'd still
have to introduce a new "my new device" bit, instead of just clearing
the ones which were fixed.
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 0fcc0a3..8277065 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void *_dwc)
> */
> int dwc3_gadget_init(struct dwc3 *dwc)
> {
> + u32 reg;
> int ret;
>
> dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req),
> @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
> if (ret)
> goto err4;
>
> + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
weird, why would Synopsys put this here ? It seems like this is only
useful when LPM Errata is enabled and that's, apparently, a host-side
thing.
Paul, can you comment ?
--
balbi
signature.asc
Description: Digital signature
[PATCH v3 1/3] mfd: viperboard: allocate I/O buffer separately
Currently the I/O buffer is allocated part of the device status
structure, potentially sharing the same cache line with other members
in this structure.
Allocate the buffer separately, to avoid the I/O operations corrupting
the device status structure due to cache line sharing.
Compiled tested only as I don't have access to hardware.
Signed-off-by: Octavian Purdila
---
drivers/mfd/viperboard.c | 8
include/linux/mfd/viperboard.h | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index e00f534..5f62f4e 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface *interface,
return -ENOMEM;
}
+ vb->buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
+ if (vb->buf == NULL) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
mutex_init(&vb->lock);
vb->usb_dev = usb_get_dev(interface_to_usbdev(interface));
@@ -105,6 +111,7 @@ static int vprbrd_probe(struct usb_interface *interface,
error:
if (vb) {
usb_put_dev(vb->usb_dev);
+ kfree(vb->buf);
kfree(vb);
}
@@ -118,6 +125,7 @@ static void vprbrd_disconnect(struct usb_interface
*interface)
mfd_remove_devices(&interface->dev);
usb_set_intfdata(interface, NULL);
usb_put_dev(vb->usb_dev);
+ kfree(vb->buf);
kfree(vb);
dev_dbg(&interface->dev, "disconnected\n");
diff --git a/include/linux/mfd/viperboard.h b/include/linux/mfd/viperboard.h
index 1934528..af928d0 100644
--- a/include/linux/mfd/viperboard.h
+++ b/include/linux/mfd/viperboard.h
@@ -103,7 +103,7 @@ struct vprbrd_i2c_addr_msg {
struct vprbrd {
struct usb_device *usb_dev; /* the usb device for this device */
struct mutex lock;
- u8 buf[sizeof(struct vprbrd_i2c_write_msg)];
+ u8 *buf;
struct platform_device pdev;
};
--
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 2/3] mfd: viperboard: switch to devm_ allocation
Switch to using devm_ allocation to simplify the error path. Also
remove a redundant OOM error message.
Signed-off-by: Octavian Purdila
---
drivers/mfd/viperboard.c | 18 +-
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index 5f62f4e..57fac1d 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -53,18 +53,16 @@ static int vprbrd_probe(struct usb_interface *interface,
const struct usb_device_id *id)
{
struct vprbrd *vb;
-
u16 version = 0;
int pipe, ret;
/* allocate memory for our device state and initialize it */
- vb = kzalloc(sizeof(*vb), GFP_KERNEL);
- if (vb == NULL) {
- dev_err(&interface->dev, "Out of memory\n");
+ vb = devm_kzalloc(&interface->dev, sizeof(*vb), GFP_KERNEL);
+ if (vb == NULL)
return -ENOMEM;
- }
- vb->buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
+ vb->buf = devm_kzalloc(&interface->dev,
+ sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL);
if (vb->buf == NULL) {
ret = -ENOMEM;
goto error;
@@ -109,11 +107,7 @@ static int vprbrd_probe(struct usb_interface *interface,
return 0;
error:
- if (vb) {
- usb_put_dev(vb->usb_dev);
- kfree(vb->buf);
- kfree(vb);
- }
+ usb_put_dev(vb->usb_dev);
return ret;
}
@@ -125,8 +119,6 @@ static void vprbrd_disconnect(struct usb_interface
*interface)
mfd_remove_devices(&interface->dev);
usb_set_intfdata(interface, NULL);
usb_put_dev(vb->usb_dev);
- kfree(vb->buf);
- kfree(vb);
dev_dbg(&interface->dev, "disconnected\n");
}
--
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/3] mfd: viperboard: fix cache line sharing and cleanups
Changes since v2: * switch to using devm_ allocation * remove unused platform_device Changes since v1: * split cache sharing fix and cleanups into separate patches Octavian Purdila (3): mfd: viperboard: allocate I/O buffer separately mfd: viperboard: switch to devm_ allocation mfd: viperboard: remove unused platform_device drivers/mfd/viperboard.c | 19 +-- include/linux/mfd/viperboard.h | 3 +-- 2 files changed, 10 insertions(+), 12 deletions(-) -- 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 3/3] mfd: viperboard: remove unused platform_device
Signed-off-by: Octavian Purdila
---
drivers/mfd/viperboard.c | 1 -
include/linux/mfd/viperboard.h | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index 57fac1d..1e7c316 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -74,7 +74,6 @@ static int vprbrd_probe(struct usb_interface *interface,
/* save our data pointer in this interface device */
usb_set_intfdata(interface, vb);
- dev_set_drvdata(&vb->pdev.dev, vb);
/* get version information, major first, minor then */
pipe = usb_rcvctrlpipe(vb->usb_dev, 0);
diff --git a/include/linux/mfd/viperboard.h b/include/linux/mfd/viperboard.h
index af928d0..afc14ed 100644
--- a/include/linux/mfd/viperboard.h
+++ b/include/linux/mfd/viperboard.h
@@ -104,7 +104,6 @@ struct vprbrd {
struct usb_device *usb_dev; /* the usb device for this device */
struct mutex lock;
u8 *buf;
- struct platform_device pdev;
};
#endif /* __MFD_VIPERBOARD_H__ */
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: core: log higher level message on malformed LANGID descriptor
On Thu, 2014-09-25 at 13:56 +, Scot Doyle wrote: > Commit 0cce2eda19923e5e5ccc8b042dec5af87b3ffad0 > USB: fix LANGID=0 regression trivia: > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c [] > @@ -770,9 +770,7 @@ static int usb_get_langid(struct usb_device *dev, > unsigned char *tbuf) > dev->string_langid = 0x0409; > dev->have_langid = 1; > dev_err(&dev->dev, > - "string descriptor 0 malformed (err = %d), " > - "defaulting to 0x%04x\n", > - err, dev->string_langid); > + "language id specifier not provided by device, > defaulting to English"); missing '\n' format termination. -- 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 v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
On Wed, Sep 24, 2014 at 02:23:38PM +0200, Arnd Bergmann wrote:
> On Wednesday 24 September 2014 19:29:05 Peter Chen wrote:
> >
> > So, it is IP CORE LIB (you suggest) vs IP CORE Platform Driver
> > (dwc3, musb, chipidea) you are talking about, right? Except for
> > creating another platform driver as well as related DT node (optional),
> > are there any advantages compared to current IP core driver structure?
>
> Having a library module usually requires less code, and is more
> consistent with other drivers, which helps new developers understand
> what the driver is doing.
I beg to differ. You end-up having to pass function pointers through
platform_data to handle differences which could be handled by the core
IP.
> The most important aspect though is the DT binding: once the structure
> with separate kernel drivers leaks out into the DT format, you can't
> easily change the driver any more, e.g. to make a property that was
> introduced for one glue driver more general so it can get handled by
> the common part. Having a single node lets us convert to the library
> model later, so that would be a strong reason to keep the DT binding
> simple, without child nodes.
>
> Following that logic, it turns into another reason for using the library
> model to start with, because otherwise the child device does not have
> any DT properties itself and has to rely on the parent device properties,
> which somewhat complicates the driver structure.
this is bullcrap. Take a look at
Documentation/devicetree/bindings/usb/dwc3.txt:
synopsys DWC3 CORE
DWC3- USB3 CONTROLLER
Required properties:
- compatible: must be "snps,dwc3"
- reg : Address and length of the register set for the device
- interrupts: Interrupts used by the dwc3 controller.
Optional properties:
- usb-phy : array of phandle for the PHY device. The first element
in the array is expected to be a handle to the USB2/HS PHY and
the second element is expected to be a handle to the USB3/SS PHY
- phys: from the *Generic PHY* bindings
- phy-names: from the *Generic PHY* bindings
- tx-fifo-resize: determines if the FIFO *has* to be reallocated.
This is usually a subnode to DWC3 glue to which it is connected.
dwc3@4a03 {
compatible = "snps,dwc3";
reg = <0x4a03 0xcfff>;
interrupts = <0 92 4>
usb-phy = <&usb2_phy>, <&usb3,phy>;
tx-fifo-resize;
};
This contains all the attributes it needs to work. In fact, this could
be used without any glue.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
Hi, On Wed, Sep 24, 2014 at 09:44:19AM +0200, Arnd Bergmann wrote: > > It is a good suggestion for adding DT support for core driver, Since we did > > not do it at the first, it is a little embarrass at current situation. > > > > - For the new chipidea glue drivers, it is ok we can have a child node > > for core device at glue device node, and some common entries can be there > > like: phy, vbus, dr_mode, etc. We need to add support for getting > > these common things for both through device tree and platform data > > (parent is DT support and parent is non-DT support) at core driver. > > I don't really want to see the child devices show up in DT, as this is > really an implementation detail of the way that the driver currently works, > and IMHO we should change that. > > I know that Felipe disagrees with me on this, but almost every other > driver that has soc-specific specializations does not use parent/child > nodes, neither in DT nor in Linux. Instead you have a single device > node that gets distinguished by "compatible" string. I have two answers for this: 1) We need to let different buses use the same device. The same IP can be used on PCI, platform, OMAP (yes, OMAP is pretty much a bus, even though we're using platform-bus with a bunch of code to match things), AMBA, etc. 2) I like to mode the HW in code and if you look into RTL you'll see that PCIe, OMAP, QCOM, Exynos, etc, take a semi-bus-agnostic core IP and write a wrapper IP to make it fit into the SoC. That Wrapper is also an IP of its own and has its own clocks, sometimes its own IRQs. Not to mention that PM requirements for different architectures might be (and actually are) different, while PM requirements for the core IP is "generic" because it comes from IP provider. By using a parent device, we can hide all platform-/bus-specific details on the parent driver and keep core driver "pristine". I see many, many benefits of doing things this way and many of the benefits are already cooked into the driver model itself, so why not use it ? > > - For the existing glue drivers, since we can't change existed dts, we can > > only do it from future SoC support. Then, in this kinds of glue drivers, > > we need to support for both create core driver by node and by current > > calling platform_device_add way. > > We should be able to easily change the ci_hdrc_add_device call into > a different exported function that calls a version of the probe() > code directly, as we do in all sorts of other drivers (ehci, ohci, > dwc2, ..., but not dwc3 and musb as they are maintained by Felipe). Go back a few commits and you'll see EHCI couldn't even be built with different glues (say ehci-omap and ehci-pci). > We can also gradually move in some of the other glue drivers into > the main driver if the differences are small enough. you'll end up with a bunch of conflicting requirements very soon. In fact that's one reason for MUSB being a mess. It was, originally, a single driver with platform-specific glue chosen in build-time. To this day, we still can't have different DMA glues on the kernel and TUSB6010-glue doesn't work with anybody else. When you have a single driver like that, you end up accepting platform-specific details into the core IP just because the changes are small enough. > Moving the PCI driver over to this model requires a little more > work but is absolutely doable (again, see ehci, ahci, sdhci examples) > by moving the platform_device handling out of core.c and changing > it just operate on the plain 'struct device', with an exported > probe function that gets called on either the platform device > or the pci device. sounds like a disaster waiting to happen for me. -- balbi signature.asc Description: Digital signature
Re: [PATCH v6 partial v2 1/3] usb: gadget: pxa27x_udc: prepare device-tree support
On Thu, Sep 25, 2014 at 3:44 PM, wrote: > Could you also review patch 1/3 while at it ;) ? This is patch 1/3... I guess you mean the others. Yours, Linus Walleij -- 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: core: log higher level message on malformed LANGID descriptor
Commit 0cce2eda19923e5e5ccc8b042dec5af87b3ffad0 USB: fix LANGID=0 regression defaults to a langid of 0x0409 if it's not properly implemented by the device. Explain with a higher level error message what this means. Signed-off-by: Scot Doyle --- drivers/usb/core/message.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 0c8a7fc..5317081 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -770,9 +770,7 @@ static int usb_get_langid(struct usb_device *dev, unsigned char *tbuf) dev->string_langid = 0x0409; dev->have_langid = 1; dev_err(&dev->dev, - "string descriptor 0 malformed (err = %d), " - "defaulting to 0x%04x\n", - err, dev->string_langid); + "language id specifier not provided by device, defaulting to English"); return 0; } -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/4] LED triggers for USB host and device
Hi, On Thu, Sep 25, 2014 at 12:36:23PM +0200, Greg Kroah-Hartman wrote: > On Wed, Sep 24, 2014 at 06:15:50PM -0500, Felipe Balbi wrote: > > On Wed, Sep 24, 2014 at 05:18:30PM -0500, Felipe Balbi wrote: > > > On Wed, Sep 24, 2014 at 11:41:55PM +0200, Greg Kroah-Hartman wrote: > > > > On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote: > > > > > On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote: > > > > > > This adds LED triggers for USB host and device. First two patches > > > > > > refactor UDC drivers as requested by Felipe Balbi, the next renames > > > > > > a > > > > > > file and the last is the actual implementation of the LED triggers. > > > > > > > > > > > > Changes from v5: > > > > > > - Refactoring of USB gadget completion split into two patches > > > > > > (Filipe > > > > > > Balbi) > > > > > > - Removed check for non-NULL req->completion (Filipe Balbi) > > > > > > > > > > > > Changes from v4: > > > > > > - Added performance numbers to the commit message of the last patch > > > > > > (greg k-h). > > > > > > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h). > > > > > > - Used proper coding style for switch statement (greg k-h). > > > > > > - Added comment about NULL argument (greg k-h). > > > > > > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h). > > > > > > - Both triggers are now registerd even if host or gagdet subsystem > > > > > > is not enabled (Bryan Wu, greg k-h). > > > > > > > > > > > > Changes from v3: > > > > > > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA > > > > > > conditioned block. > > > > > > - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi). > > > > > > - Removed outdated comment (Alan Stern). > > > > > > - req->complete == NULL is now a bug. Previously, this was ignored > > > > > > (Alan Stern). > > > > > > - File rename moved to a separate commit (greg k-h). > > > > > > > > > > > > Changes from v2: > > > > > > - Host/gadget triggers merged to a single file in usb/common/ > > > > > > (Felipe > > > > > > Balbi). > > > > > > - UDC drivers refactored so that LED trigger works for all of them. > > > > > > > > > > > > Changes from v1: > > > > > > - Moved from drivers/leds/ to drivers/usb/. > > > > > > - Improved Kconfig help. > > > > > > - Linked with other modules rather than being standalone modules. > > > > > > > > > > > > Michal Sojka (4): > > > > > > usb: gadget: Introduce usb_gadget_giveback_request() > > > > > > usb: gadget: Refactor request completion > > > > > > usb: Rename usb-common.c > > > > > > usb: Add LED triggers for USB activity > > > > > > > > > > Greg, how do you want to handle this one as it touched both host and > > > > > gadget ? I could wait until you apply this series to your usb-next and > > > > > based my pull request on that. That'll work as long as it doesn't take > > > > > too long for this to be merged. > > > > > > > > I can just take this now, for 3.18-rc1, you've already sent me your > > > > patches for that release, so there shouldn't be any conflicts, right? > > > > > > that's better. Can you wait 1 day though ? I can run these through my > > > build tests. > > > > running my build tests on top of v3.17-rc6 + these 4 patches. I'll > > report back tomorrow. From a code perspective, though, they look > > alright. > > Yes, will wait, thanks. nothing interesting on my build tests. I have also tested the series with my AM437x SK and I can see LED blinking with usb-gadget and usb-host triggers. For the whole series: Tested-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH v6 partial v2 1/3] usb: gadget: pxa27x_udc: prepare device-tree support
- Mail original -
De: "Linus Walleij"
À: "Robert Jarzmik"
Cc: "Felipe Balbi" , linux-usb@vger.kernel.org
Envoyé: Jeudi 25 Septembre 2014 15:10:16
Objet: Re: [PATCH v6 partial v2 1/3] usb: gadget: pxa27x_udc: prepare
device-tree support
In that case, that whole clause is actually using the old API without
explicit request, so you can just do this:
if (mach) {
/* This clause will go away when we have transitioned to
just using descriptors */
if (devm_gpio_request_one(&pdev->dev, mach->gpio_pullup,
mach->gpio_pullup_inverted ?
GPIOF_ACTIVE_LOW : 0,
"my pin"))
goto err;
udc->gpiod = gpio_to_desc(mach->gpio_pullup);
udc->mach = mach;
} else {
...
}
Excellent, sold. I'll put that in partial v3 then.
Could you also review patch 1/3 while at it ;) ?
Felipe, I'll wait for a couple of days to release partial v3 to give
you time to review the patches, let's say next wednesday ?
Cheers.
--
Robert
--
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 v6 partial v2 1/3] usb: gadget: pxa27x_udc: prepare device-tree support
On Thu, Sep 25, 2014 at 11:47 AM, wrote:
>> } else {
>> udv->gpiod = devm_gpiod_get(&pdev->dev, ...);
>> }
>> Here you can also use the flags.
>
> Yeah, but my problem is in the if statement, not in the else. The else is for
> the DT/ACPI cases, in which there is no problem.
Damned mailer sent of the mail before I was finished.
Well you got the solution anyway, check it out.
Yours,
Linus Walleij
--
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 v6 partial v2 1/3] usb: gadget: pxa27x_udc: prepare device-tree support
On Thu, Sep 25, 2014 at 11:47 AM, wrote:
> De: "Linus Walleij"
>> @@ -2415,7 +2411,13 @@ static int pxa_udc_probe(struct platform_device *pdev)
>> {
>> struct resource *regs;
>> struct pxa_udc *udc = &memory;
>> - int retval = 0, gpio;
>> + struct pxa2xx_udc_mach_info *mach = dev_get_platdata(&pdev->dev);
>> + int retval = 0;
>> +
>> + if (mach) {
>> + udc->gpiod = gpio_to_desc(mach->gpio_pullup);
>> + udc->mach = mach;
>> + }
>
>> } else {
>> udv->gpiod = devm_gpiod_get(&pdev->dev, ...);
>> }
>> Here you can also use the flags.
>
> Yeah, but my problem is in the if statement, not in the else. The else is for
> the DT/ACPI cases, in which there is no problem.
In that case, that whole clause is actually using the old API without
explicit request, so you can just do this:
if (mach) {
/* This clause will go away when we have transitioned to
just using descriptors */
if (devm_gpio_request_one(&pdev->dev, mach->gpio_pullup,
mach->gpio_pullup_inverted ?
GPIOF_ACTIVE_LOW : 0,
"my pin"))
goto err;
udc->gpiod = gpio_to_desc(mach->gpio_pullup);
udc->mach = mach;
} else {
...
}
> On the other hand, the "if" is for the legacy platform_data cases which
> bother me.
> The mach info contains :
> - gpio_pullup : number of the gpio
> - gpio_pullup_inverted : the active low state
>
> What I would have needed would be something like :
> if (mach) {
> udc->gpiod = gpio_to_desc(mach->gpio_pullup);
> gpiod_MAGIC_FUNC_set_flags(udc->gpiod, mach->gpio_pullup_inverted ?
> ACTIVE_LOW : 0);
> udc->mach = mach;
> } else {
> udv->gpiod = devm_gpiod_get(&pdev->dev, ...);
> }
>
> The "MAGIC_FUNC" is what I'm missing here. And I agree this is something that
> is bound in
> electronics, and eventually will have to be shifted to platform code. The
> porting issue I
> have is that I don't want the dependency of changing *all* pxa27x machines as
> a dependency
> for a patch to pxa27x_udc to support device-tree.
> Or alternatively have something like :
> udc->gpiod = gpio_to_desc_with_flags(mach->gpio_pullup, flags).
> Or another way, to enable a smooth transition without the dependency.
>
> Even I have not found any machine code using this inverted flag, I might have
> overlooked
> something, and I'm not feeling confortable killing a platform because I made
> a wrong
> decision.
>
> Cheers.
>
> --
> Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 1/3] r8152: change the EEE definition
Replace the EEE definitions with the ones which is declared in "mdio.h". Chage some definitions to make them readable. Signed-off-by: Hayes Wang --- drivers/net/usb/r8152.c | 35 ++- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 3d5c39a..d225aa1 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -22,6 +22,8 @@ #include #include #include +#include +#include /* Version Information */ #define DRIVER_VERSION "v1.06.0 (2014/03/03)" @@ -129,7 +131,7 @@ #define OCP_SRAM_ADDR 0xa436 #define OCP_SRAM_DATA 0xa438 #define OCP_DOWN_SPEED 0xa442 -#define OCP_EEE_CFG2 0xa5d0 +#define OCP_EEE_ADV0xa5d0 #define OCP_ADC_CFG0xbc06 /* SRAM Register */ @@ -361,7 +363,7 @@ #define EEE_NWAY_EN0x1000 #define TX_QUIET_EN0x0200 #define RX_QUIET_EN0x0100 -#define SDRISETIME 0x0010 /* bit 4 ~ 6 */ +#define sd_rise_time(x)(min(x, 7) << 4)/* bit 4 ~ 6 */ #define RG_RXLPI_MSK_HFDUP 0x0008 #define SDFALLTIME 0x0007 /* bit 0 ~ 2 */ @@ -373,7 +375,7 @@ #define RG_EEEPRG_EN 0x0010 /* OCP_EEE_CONFIG3 */ -#define FST_SNR_EYE_R 0x1500 /* bit 7 ~ 15 */ +#define fast_snr(x)(min(x, 0x1ff) << 7)/* bit 7 ~ 15 */ #define RG_LFS_SEL 0x0060 /* bit 6 ~ 5 */ #define MSK_PH 0x0006 /* bit 0 ~ 3 */ @@ -382,11 +384,6 @@ #define FUN_ADDR 0x #define FUN_DATA 0x4000 /* bit[4:0] device addr */ -#define DEVICE_ADDR0x0007 - -/* OCP_EEE_DATA */ -#define EEE_ADDR 0x003C -#define EEE_DATA 0x0002 /* OCP_EEE_CFG */ #define CTAP_SHORT_EN 0x0040 @@ -395,10 +392,6 @@ /* OCP_DOWN_SPEED */ #define EN_10M_BGOFF 0x0080 -/* OCP_EEE_CFG2 */ -#define MY1000_EEE 0x0004 -#define MY100_EEE 0x0002 - /* OCP_ADC_CFG */ #define CKADSEL_L 0x0100 #define ADC_EN 0x0080 @@ -2967,16 +2960,16 @@ static void r8152b_enable_eee(struct r8152 *tp) ocp_reg_write(tp, OCP_EEE_CONFIG1, RG_TXLPI_MSK_HFDUP | RG_MATCLR_EN | EEE_10_CAP | EEE_NWAY_EN | TX_QUIET_EN | RX_QUIET_EN | - SDRISETIME | RG_RXLPI_MSK_HFDUP | + sd_rise_time(1) | RG_RXLPI_MSK_HFDUP | SDFALLTIME); ocp_reg_write(tp, OCP_EEE_CONFIG2, RG_LPIHYS_NUM | RG_DACQUIET_EN | RG_LDVQUIET_EN | RG_CKRSEL | RG_EEEPRG_EN); - ocp_reg_write(tp, OCP_EEE_CONFIG3, FST_SNR_EYE_R | RG_LFS_SEL | MSK_PH); - ocp_reg_write(tp, OCP_EEE_AR, FUN_ADDR | DEVICE_ADDR); - ocp_reg_write(tp, OCP_EEE_DATA, EEE_ADDR); - ocp_reg_write(tp, OCP_EEE_AR, FUN_DATA | DEVICE_ADDR); - ocp_reg_write(tp, OCP_EEE_DATA, EEE_DATA); + ocp_reg_write(tp, OCP_EEE_CONFIG3, fast_snr(42) | RG_LFS_SEL | MSK_PH); + ocp_reg_write(tp, OCP_EEE_AR, FUN_ADDR | MDIO_MMD_AN); + ocp_reg_write(tp, OCP_EEE_DATA, MDIO_AN_EEE_ADV); + ocp_reg_write(tp, OCP_EEE_AR, FUN_DATA | MDIO_MMD_AN); + ocp_reg_write(tp, OCP_EEE_DATA, MDIO_EEE_100TX); ocp_reg_write(tp, OCP_EEE_AR, 0x); } @@ -2991,9 +2984,9 @@ static void r8153_enable_eee(struct r8152 *tp) data = ocp_reg_read(tp, OCP_EEE_CFG); data |= EEE10_EN; ocp_reg_write(tp, OCP_EEE_CFG, data); - data = ocp_reg_read(tp, OCP_EEE_CFG2); - data |= MY1000_EEE | MY100_EEE; - ocp_reg_write(tp, OCP_EEE_CFG2, data); + data = ocp_reg_read(tp, OCP_EEE_ADV); + data |= MDIO_EEE_1000T | MDIO_EEE_100TX; + ocp_reg_write(tp, OCP_EEE_ADV, data); } static void r8152b_enable_fc(struct r8152 *tp) -- 1.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
[PATCH net-next 0/3] r8152: support setting eee by ethtool
Modify some definitions about EEE, and add the support of setting the EEE through ethtool. Hayes Wang (3): r8152: change the EEE definition r8152: add functions to set EEE r8152: support ethtool eee drivers/net/usb/r8152.c | 246 1 file changed, 209 insertions(+), 37 deletions(-) -- 1.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
[PATCH net-next 2/3] r8152: add functions to set EEE
Add functions to enable EEE and set EEE advertisement.
Signed-off-by: Hayes Wang
---
drivers/net/usb/r8152.c | 101
1 file changed, 76 insertions(+), 25 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index d225aa1..887b6a2 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -363,6 +363,7 @@
#define EEE_NWAY_EN0x1000
#define TX_QUIET_EN0x0200
#define RX_QUIET_EN0x0100
+#define sd_rise_time_mask 0x0070
#define sd_rise_time(x)(min(x, 7) << 4)/* bit 4 ~ 6 */
#define RG_RXLPI_MSK_HFDUP 0x0008
#define SDFALLTIME 0x0007 /* bit 0 ~ 2 */
@@ -375,6 +376,7 @@
#define RG_EEEPRG_EN 0x0010
/* OCP_EEE_CONFIG3 */
+#define fast_snr_mask 0xff80
#define fast_snr(x)(min(x, 0x1ff) << 7)/* bit 7 ~ 15 */
#define RG_LFS_SEL 0x0060 /* bit 6 ~ 5 */
#define MSK_PH 0x0006 /* bit 0 ~ 3 */
@@ -2950,43 +2952,92 @@ static int rtl8152_close(struct net_device *netdev)
return res;
}
-static void r8152b_enable_eee(struct r8152 *tp)
+static inline void r8152_mmd_indirect(struct r8152 *tp, u16 dev, u16 reg)
+{
+ ocp_reg_write(tp, OCP_EEE_AR, FUN_ADDR | dev);
+ ocp_reg_write(tp, OCP_EEE_DATA, reg);
+ ocp_reg_write(tp, OCP_EEE_AR, FUN_DATA | dev);
+}
+
+static u16 r8152_mmd_read(struct r8152 *tp, u16 dev, u16 reg)
+{
+ u16 data;
+
+ r8152_mmd_indirect(tp, dev, reg);
+ data = ocp_reg_read(tp, OCP_EEE_DATA);
+ ocp_reg_write(tp, OCP_EEE_AR, 0x);
+
+ return data;
+}
+
+static void r8152_mmd_write(struct r8152 *tp, u16 dev, u16 reg, u16 data)
{
+ r8152_mmd_indirect(tp, dev, reg);
+ ocp_reg_write(tp, OCP_EEE_DATA, data);
+ ocp_reg_write(tp, OCP_EEE_AR, 0x);
+}
+
+static void r8152_eee_en(struct r8152 *tp, bool enable)
+{
+ u16 config1, config2, config3;
u32 ocp_data;
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
- ocp_data |= EEE_RX_EN | EEE_TX_EN;
+ config1 = ocp_reg_read(tp, OCP_EEE_CONFIG1) & ~sd_rise_time_mask;
+ config2 = ocp_reg_read(tp, OCP_EEE_CONFIG2);
+ config3 = ocp_reg_read(tp, OCP_EEE_CONFIG3) & ~fast_snr_mask;
+
+ if (enable) {
+ ocp_data |= EEE_RX_EN | EEE_TX_EN;
+ config1 |= EEE_10_CAP | EEE_NWAY_EN | TX_QUIET_EN | RX_QUIET_EN;
+ config1 |= sd_rise_time(1);
+ config2 |= RG_DACQUIET_EN | RG_LDVQUIET_EN;
+ config3 |= fast_snr(42);
+ } else {
+ ocp_data &= ~(EEE_RX_EN | EEE_TX_EN);
+ config1 &= ~(EEE_10_CAP | EEE_NWAY_EN | TX_QUIET_EN |
+RX_QUIET_EN);
+ config1 |= sd_rise_time(7);
+ config2 &= ~(RG_DACQUIET_EN | RG_LDVQUIET_EN);
+ config3 |= fast_snr(511);
+ }
+
ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEE_CR, ocp_data);
- ocp_reg_write(tp, OCP_EEE_CONFIG1, RG_TXLPI_MSK_HFDUP | RG_MATCLR_EN |
- EEE_10_CAP | EEE_NWAY_EN |
- TX_QUIET_EN | RX_QUIET_EN |
- sd_rise_time(1) | RG_RXLPI_MSK_HFDUP
|
- SDFALLTIME);
- ocp_reg_write(tp, OCP_EEE_CONFIG2, RG_LPIHYS_NUM | RG_DACQUIET_EN |
- RG_LDVQUIET_EN | RG_CKRSEL |
- RG_EEEPRG_EN);
- ocp_reg_write(tp, OCP_EEE_CONFIG3, fast_snr(42) | RG_LFS_SEL | MSK_PH);
- ocp_reg_write(tp, OCP_EEE_AR, FUN_ADDR | MDIO_MMD_AN);
- ocp_reg_write(tp, OCP_EEE_DATA, MDIO_AN_EEE_ADV);
- ocp_reg_write(tp, OCP_EEE_AR, FUN_DATA | MDIO_MMD_AN);
- ocp_reg_write(tp, OCP_EEE_DATA, MDIO_EEE_100TX);
- ocp_reg_write(tp, OCP_EEE_AR, 0x);
+ ocp_reg_write(tp, OCP_EEE_CONFIG1, config1);
+ ocp_reg_write(tp, OCP_EEE_CONFIG2, config2);
+ ocp_reg_write(tp, OCP_EEE_CONFIG3, config3);
}
-static void r8153_enable_eee(struct r8152 *tp)
+static void r8152b_enable_eee(struct r8152 *tp)
+{
+ r8152_eee_en(tp, true);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, MDIO_EEE_100TX);
+}
+
+static void r8153_eee_en(struct r8152 *tp, bool enable)
{
u32 ocp_data;
- u16 data;
+ u16 config;
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
- ocp_data |= EEE_RX_EN | EEE_TX_EN;
+ config = ocp_reg_read(tp, OCP_EEE_CFG);
+
+ if (enable) {
+ ocp_data |= EEE_RX_EN | EEE_TX_EN;
+ config |= EEE10_EN;
+ } else {
+ ocp_data &= ~(EEE_RX_EN | EEE_TX_EN);
+ config &= ~EEE10_EN;
+ }
+
ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEE_CR, ocp_data);
- data = ocp_reg_read(tp, OCP_EEE_CFG);
- data |= EEE10_EN;
- ocp_reg_writ
[PATCH net-next 3/3] r8152: support ethtool eee
Support get_eee() and set_eee() of ethtool_ops.
Signed-off-by: Hayes Wang
---
drivers/net/usb/r8152.c | 128
1 file changed, 128 insertions(+)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 887b6a2..a4d4c4a 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -131,7 +131,9 @@
#define OCP_SRAM_ADDR 0xa436
#define OCP_SRAM_DATA 0xa438
#define OCP_DOWN_SPEED 0xa442
+#define OCP_EEE_ABLE 0xa5c4
#define OCP_EEE_ADV0xa5d0
+#define OCP_EEE_LPABLE 0xa5d2
#define OCP_ADC_CFG0xbc06
/* SRAM Register */
@@ -572,6 +574,8 @@ struct r8152 {
void (*up)(struct r8152 *);
void (*down)(struct r8152 *);
void (*unload)(struct r8152 *);
+ int (*eee_get)(struct r8152 *, struct ethtool_eee *);
+ int (*eee_set)(struct r8152 *, struct ethtool_eee *);
} rtl_ops;
int intr_interval;
@@ -3366,6 +3370,122 @@ static void rtl8152_get_strings(struct net_device *dev,
u32 stringset, u8 *data)
}
}
+static int r8152_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
+{
+ u32 ocp_data, lp, adv, supported = 0;
+ u16 val;
+
+ val = r8152_mmd_read(tp, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
+ supported = mmd_eee_cap_to_ethtool_sup_t(val);
+
+ val = r8152_mmd_read(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV);
+ adv = mmd_eee_adv_to_ethtool_adv_t(val);
+
+ val = r8152_mmd_read(tp, MDIO_MMD_AN, MDIO_AN_EEE_LPABLE);
+ lp = mmd_eee_adv_to_ethtool_adv_t(val);
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
+ ocp_data &= EEE_RX_EN | EEE_TX_EN;
+
+ eee->eee_enabled = !!ocp_data;
+ eee->eee_active = !!(supported & adv & lp);
+ eee->supported = supported;
+ eee->advertised = adv;
+ eee->lp_advertised = lp;
+
+ return 0;
+}
+
+static int r8152_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
+{
+ u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
+
+ r8152_eee_en(tp, eee->eee_enabled);
+
+ if (!eee->eee_enabled)
+ val = 0;
+
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val);
+
+ return 0;
+}
+
+static int r8153_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
+{
+ u32 ocp_data, lp, adv, supported = 0;
+ u16 val;
+
+ val = ocp_reg_read(tp, OCP_EEE_ABLE);
+ supported = mmd_eee_cap_to_ethtool_sup_t(val);
+
+ val = ocp_reg_read(tp, OCP_EEE_ADV);
+ adv = mmd_eee_adv_to_ethtool_adv_t(val);
+
+ val = ocp_reg_read(tp, OCP_EEE_LPABLE);
+ lp = mmd_eee_adv_to_ethtool_adv_t(val);
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
+ ocp_data &= EEE_RX_EN | EEE_TX_EN;
+
+ eee->eee_enabled = !!ocp_data;
+ eee->eee_active = !!(supported & adv & lp);
+ eee->supported = supported;
+ eee->advertised = adv;
+ eee->lp_advertised = lp;
+
+ return 0;
+}
+
+static int r8153_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
+{
+ u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
+
+ r8153_eee_en(tp, eee->eee_enabled);
+
+ if (!eee->eee_enabled)
+ val = 0;
+
+ ocp_reg_write(tp, OCP_EEE_ADV, val);
+
+ return 0;
+}
+
+static int
+rtl_ethtool_get_eee(struct net_device *net, struct ethtool_eee *edata)
+{
+ struct r8152 *tp = netdev_priv(net);
+ int ret;
+
+ ret = usb_autopm_get_interface(tp->intf);
+ if (ret < 0)
+ goto out;
+
+ ret = tp->rtl_ops.eee_get(tp, edata);
+
+ usb_autopm_put_interface(tp->intf);
+
+out:
+ return ret;
+}
+
+static int
+rtl_ethtool_set_eee(struct net_device *net, struct ethtool_eee *edata)
+{
+ struct r8152 *tp = netdev_priv(net);
+ int ret;
+
+ ret = usb_autopm_get_interface(tp->intf);
+ if (ret < 0)
+ goto out;
+
+ ret = tp->rtl_ops.eee_set(tp, edata);
+
+ usb_autopm_put_interface(tp->intf);
+
+out:
+ return ret;
+}
+
static struct ethtool_ops ops = {
.get_drvinfo = rtl8152_get_drvinfo,
.get_settings = rtl8152_get_settings,
@@ -3378,6 +3498,8 @@ static struct ethtool_ops ops = {
.get_strings = rtl8152_get_strings,
.get_sset_count = rtl8152_get_sset_count,
.get_ethtool_stats = rtl8152_get_ethtool_stats,
+ .get_eee = rtl_ethtool_get_eee,
+ .set_eee = rtl_ethtool_set_eee,
};
static int rtl8152_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
@@ -3519,6 +3641,8 @@ static int rtl_ops_init(struct r8152 *tp, const struct
usb_device_id *id)
ops->up = rtl8152_up;
ops->down = rtl8152_down;
ops->unload = rtl8152_unload;
+ ops->eee_get= r8152_get_eee;
+ ops->
Re: USB3 WD drive failure on ASUS M5A 78L-M USB3
Thanks. I tried with the kernel from backports & it seems to be fixed there. Linux foobar 3.14-0.bpo.2-amd64 #1 SMP Debian 3.14.15-2~bpo70+1 (2014-08-21) x86_64 GNU/Linux --Len Berman Chief Entertainment Officer Out to Pasture Enterprises Blog On Thu, Sep 25, 2014 at 4:34 AM, Lu, Baolu wrote: > Can you try with the latest kernel? > > > On 9/24/2014 6:13 AM, Len Berman wrote: >> >> I'm running debian 7. >> >> Linux foobar 3.2.0-4-amd64 #1 SMP Debian 3.2.60-1+deb7u3 x86_64 GNU/Linux. >> >> I have a Passport drive which fails on the USB3 connector. It works >> fine on USB2. It also works fine as USB3 on my Lenovo laptop. >> >> The output of lspci -v & lsusb -v is at >> http://outtopastureenterprises.com/device.txt >> >> >> When I plug in the device to the USB3 port I get the following in >> /var/log/messages. If I wait long enough, eventually it seems to >> connect. This can take tens of minutes. >> >> I don't know what else to include. Any ideas greatly appreciated. >> >> Thanks. >> --Len >> >> >> Sep 23 17:49:22 foobar kernel: [23209.332493] usb 4-1: new SuperSpeed >> USB device number 4 using xhci_hcd >> Sep 23 17:49:22 foobar kernel: [23209.352862] usb 4-1: New USB device >> found, idVendor=1058, idProduct=0820 >> Sep 23 17:49:22 foobar kernel: [23209.352875] usb 4-1: New USB device >> strings: Mfr=1, Product=2, SerialNumber=5 >> Sep 23 17:49:22 foobar kernel: [23209.352884] usb 4-1: Product: My >> Passport 0820 >> Sep 23 17:49:22 foobar kernel: [23209.352890] usb 4-1: Manufacturer: >> Western Digital >> Sep 23 17:49:22 foobar kernel: [23209.352897] usb 4-1: SerialNumber: >> 575835314142334434373334 >> Sep 23 17:49:22 foobar kernel: [23209.356231] scsi9 : usb-storage 4-1:1.0 >> Sep 23 17:49:22 foobar mtp-probe: checking bus 4, device 4: >> "/sys/devices/pci:00/:00:07.0/:02:00.0/usb4/4-1" >> Sep 23 17:49:22 foobar mtp-probe: bus: 4, device: 4 was not an MTP device >> Sep 23 17:49:23 foobar kernel: [23210.356893] scsi 9:0:0:0: >> Direct-Access WD My Passport 0820 1007 PQ: 0 ANSI: 6 >> Sep 23 17:49:23 foobar kernel: [23210.357131] scsi 9:0:0:1: Enclosure >> WD SES Device 1007 PQ: 0 ANSI: 6 >> Sep 23 17:49:23 foobar kernel: [23210.358448] sd 9:0:0:0: Attached >> scsi generic sg5 type 0 >> Sep 23 17:49:23 foobar kernel: [23210.358724] ses 9:0:0:1: Attached >> Enclosure device >> Sep 23 17:49:23 foobar kernel: [23210.358922] ses 9:0:0:1: Attached >> scsi generic sg6 type 13 >> Sep 23 17:49:54 foobar kernel: [23240.945357] usb 4-1: reset >> SuperSpeed USB device number 4 using xhci_hcd >> Sep 23 17:49:54 foobar kernel: [23240.962978] xhci_hcd :02:00.0: >> xHCI xhci_drop_endpoint called with disabled ep 8801f234f2c0 >> Sep 23 17:49:54 foobar kernel: [23240.962992] xhci_hcd :02:00.0: >> xHCI xhci_drop_endpoint called with disabled ep 8801f234f300 >> Sep 23 17:50:25 foobar kernel: [23271.953310] usb 4-1: reset >> SuperSpeed USB device number 4 using xhci_hcd >> Sep 23 17:50:25 foobar kernel: [23271.971070] xhci_hcd :02:00.0: >> xHCI xhci_drop_endpoint called with disabled ep 8801f234f2c0 >> Sep 23 17:50:25 foobar kernel: [23271.971085] xhci_hcd :02:00.0: >> xHCI xhci_drop_endpoint called with disabled ep 8801f234f300 >> Sep 23 17:50:25 foobar kernel: [23271.973226] sd 9:0:0:0: [sde] >> 1953458176 512-byte logical blocks: (1.00 TB/931 GiB) >> Sep 23 17:50:56 foobar kernel: [23302.993335] usb 4-1: reset >> SuperSpeed USB device number 4 using xhci_hcd >> Sep 23 17:50:56 foobar kernel: [23303.010894] xhci_hcd :02:00.0: >> xHCI xhci_drop_endpoint called with disabled ep 8801f234f2c0 >> Sep 23 17:50:56 foobar kernel: [23303.010909] xhci_hcd :02:00.0: >> xHCI xhci_drop_endpoint called with disabled ep 8801f234f300 >> Sep 23 17:51:27 foobar kernel: [2.973362] usb 4-1: reset >> SuperSpeed USB device number 4 using xhci_hcd >> Sep 23 17:51:27 foobar kernel: [2.990995] xhci_hcd :02:00.0: >> xHCI xhci_drop_endpoint called with disabled ep 8801f234f2c0 >> Sep 23 17:51:27 foobar kernel: [2.991009] xhci_hcd :02:00.0: >> xHCI xhci_drop_endpoint called with disabled ep 8801f234f300 >> -- >> 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 > > -- 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/6] usb: dwc2/gadget: move phy bus legth initialization
On Thu, Sep 25, 2014 at 02:26:58PM +0200, Marek Szyprowski wrote: > Hi Greg and Paul, > > On 2014-09-09 10:44, Robert Baldyga wrote: > >From: Kamil Debski > > > >This patch moves the part of code that initializes the PHY bus width. > >This results in simpler code and removes the need to check whether > >the Generic PHY Framework is used. > > I've noticed that patches from this patchset have been finally merged to > usb-next tree, but I cannot find this patch there. Is there any reason for > dropping it? Ugh, did I miss it somehow? Yes I did, sorry about that, now applied... thanks for picking this out. 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 1/6] usb: dwc2/gadget: move phy bus legth initialization
Hi Greg and Paul,
On 2014-09-09 10:44, Robert Baldyga wrote:
From: Kamil Debski
This patch moves the part of code that initializes the PHY bus width.
This results in simpler code and removes the need to check whether
the Generic PHY Framework is used.
I've noticed that patches from this patchset have been finally merged to
usb-next tree, but I cannot find this patch there. Is there any reason for
dropping it?
Signed-off-by: Kamil Debski
Signed-off-by: Marek Szyprowski
Signed-off-by: Robert Baldyga
---
drivers/usb/dwc2/gadget.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index ce6071d..9cbe136 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3392,6 +3392,9 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
if (!hsotg)
return -ENOMEM;
+ /* Set default UTMI width */
+ hsotg->phyif = GUSBCFG_PHYIF16;
+
/*
* Attempt to find a generic PHY, then look for an old style
* USB PHY, finally fall back to pdata
@@ -3410,8 +3413,15 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
hsotg->plat = plat;
} else
hsotg->uphy = uphy;
- } else
+ } else {
hsotg->phy = phy;
+ /*
+* If using the generic PHY framework, check if the PHY bus
+* width is 8-bit and set the phyif appropriately.
+*/
+ if (phy_get_bus_width(phy) == 8)
+ hsotg->phyif = GUSBCFG_PHYIF8;
+ }
hsotg->dev = dev;
@@ -3471,16 +3481,6 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
goto err_supplies;
}
- /* Set default UTMI width */
- hsotg->phyif = GUSBCFG_PHYIF16;
-
- /*
-* If using the generic PHY framework, check if the PHY bus
-* width is 8-bit and set the phyif appropriately.
-*/
- if (hsotg->phy && (phy_get_bus_width(phy) == 8))
- hsotg->phyif = GUSBCFG_PHYIF8;
-
/* usb phy enable */
s3c_hsotg_phy_enable(hsotg);
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
--
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: Remove .owner field for driver
On Wed, 2014-09-24 at 16:48 +0530, Kiran Padwal wrote: > There is no need to init .owner field. > > Based on the patch from Peter Griffin > "mmc: remove .owner field for drivers using module_platform_driver" > > This patch removes the superflous .owner field for drivers which > use the module_platform_driver API, as this is overriden in > platform_driver_register anyway." > > Signed-off-by: Kiran Padwal Thank you Kiran. Reviewed-by: Ivan T. Ivanov -- 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
XHCI host dies with LPM + webcam gadget
Hello Mathias, I'm trying to connect a webcam gadget on USB3.0 interface with xHCI host when LPM is enabled. My xHCI host is TI's TUSB7340EVM. xHCI host stops responding as soon as the webcam gadget module is inserted on the device side (The device is also running linux) Host machine kernel version is 3.16. I have enabled quirk for LPM enable for my XHCI card. The detailed description and a host kernel logs can be found from bugzila: https://bugzilla.kernel.org/show_bug.cgi?id=85141 Could you please help in rectifying this bug? Please let me know if there's any more information required. Thanks Amit Virdi -- 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 v5 1/4] mfd: add support for Diolan DLN-2 devices
On Thu, Sep 25, 2014 at 01:41:16PM +0300, Octavian Purdila wrote: > On Thu, Sep 25, 2014 at 1:30 PM, Johan Hovold wrote: > > On Thu, Sep 25, 2014 at 01:25:24PM +0300, Octavian Purdila wrote: > > > >> Johan, I think we don't really need the spinlock, the disconnect flag > >> and an atomic counter should work. Do you see any issues with that? > > > > No, you need to test and increment atomically so the lock is needed. > > > > Consider what could happen if you get a disconnect after testing but > > before incrementing. > > I am still not seeing the problem. We would continue with the > increment, we will try to send which will fail and go on the error > path where we will decrement and wake_up. What am I missing? The whole point of the counter and flag is to make sure that no transfers are started after the flag is set. If you remove the lock you cannot guarantee that. Disconnect sets the flag (after you test it in transfer() but before incrementing the counter), checks the counter which is 0 and proceeds with deregistration and deallocation. Then transfer() gets to run... 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 v5 1/4] mfd: add support for Diolan DLN-2 devices
On Thu, Sep 25, 2014 at 1:30 PM, Johan Hovold wrote: > On Thu, Sep 25, 2014 at 01:25:24PM +0300, Octavian Purdila wrote: > >> Johan, I think we don't really need the spinlock, the disconnect flag >> and an atomic counter should work. Do you see any issues with that? > > No, you need to test and increment atomically so the lock is needed. > > Consider what could happen if you get a disconnect after testing but > before incrementing. > I am still not seeing the problem. We would continue with the increment, we will try to send which will fail and go on the error path where we will decrement and wake_up. What am I missing? -- 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 v6 0/4] LED triggers for USB host and device
On Wed, Sep 24, 2014 at 06:15:50PM -0500, Felipe Balbi wrote: > On Wed, Sep 24, 2014 at 05:18:30PM -0500, Felipe Balbi wrote: > > On Wed, Sep 24, 2014 at 11:41:55PM +0200, Greg Kroah-Hartman wrote: > > > On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote: > > > > On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote: > > > > > This adds LED triggers for USB host and device. First two patches > > > > > refactor UDC drivers as requested by Felipe Balbi, the next renames a > > > > > file and the last is the actual implementation of the LED triggers. > > > > > > > > > > Changes from v5: > > > > > - Refactoring of USB gadget completion split into two patches (Filipe > > > > > Balbi) > > > > > - Removed check for non-NULL req->completion (Filipe Balbi) > > > > > > > > > > Changes from v4: > > > > > - Added performance numbers to the commit message of the last patch > > > > > (greg k-h). > > > > > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h). > > > > > - Used proper coding style for switch statement (greg k-h). > > > > > - Added comment about NULL argument (greg k-h). > > > > > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h). > > > > > - Both triggers are now registerd even if host or gagdet subsystem > > > > > is not enabled (Bryan Wu, greg k-h). > > > > > > > > > > Changes from v3: > > > > > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA > > > > > conditioned block. > > > > > - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi). > > > > > - Removed outdated comment (Alan Stern). > > > > > - req->complete == NULL is now a bug. Previously, this was ignored > > > > > (Alan Stern). > > > > > - File rename moved to a separate commit (greg k-h). > > > > > > > > > > Changes from v2: > > > > > - Host/gadget triggers merged to a single file in usb/common/ (Felipe > > > > > Balbi). > > > > > - UDC drivers refactored so that LED trigger works for all of them. > > > > > > > > > > Changes from v1: > > > > > - Moved from drivers/leds/ to drivers/usb/. > > > > > - Improved Kconfig help. > > > > > - Linked with other modules rather than being standalone modules. > > > > > > > > > > Michal Sojka (4): > > > > > usb: gadget: Introduce usb_gadget_giveback_request() > > > > > usb: gadget: Refactor request completion > > > > > usb: Rename usb-common.c > > > > > usb: Add LED triggers for USB activity > > > > > > > > Greg, how do you want to handle this one as it touched both host and > > > > gadget ? I could wait until you apply this series to your usb-next and > > > > based my pull request on that. That'll work as long as it doesn't take > > > > too long for this to be merged. > > > > > > I can just take this now, for 3.18-rc1, you've already sent me your > > > patches for that release, so there shouldn't be any conflicts, right? > > > > that's better. Can you wait 1 day though ? I can run these through my > > build tests. > > running my build tests on top of v3.17-rc6 + these 4 patches. I'll > report back tomorrow. From a code perspective, though, they look > alright. Yes, will wait, 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 v5 1/4] mfd: add support for Diolan DLN-2 devices
On Thu, Sep 25, 2014 at 01:25:24PM +0300, Octavian Purdila wrote: > Johan, I think we don't really need the spinlock, the disconnect flag > and an atomic counter should work. Do you see any issues with that? No, you need to test and increment atomically so the lock is needed. Consider what could happen if you get a disconnect after testing but before incrementing. 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 v5 1/4] mfd: add support for Diolan DLN-2 devices
On Wed, Sep 24, 2014 at 6:22 PM, Octavian Purdila
wrote:
> On Wed, Sep 24, 2014 at 6:07 PM, Johan Hovold wrote:
>> On Wed, Sep 24, 2014 at 05:54:15PM +0300, Octavian Purdila wrote:
>>> On Wed, Sep 24, 2014 at 4:54 PM, Johan Hovold wrote:
>>> > On Wed, Sep 24, 2014 at 04:36:22PM +0300, Octavian Purdila wrote:
>>> >> On Wed, Sep 24, 2014 at 1:48 PM, Johan Hovold wrote:
>>> >> > On Fri, Sep 19, 2014 at 11:22:42PM +0300, Octavian Purdila wrote:
>>> >>
>>> >>
>>> >>
>>> >> >> + * dln2_dev.mod_rx_slots and then the echo header field to index the
>>> >> >> + * slots field and find the receive context for a particular
>>> >> >> + * request.
>>> >> >> + */
>>> >> >> +struct dln2_mod_rx_slots {
>>> >> >> + /* RX slots bitmap */
>>> >> >> + unsigned long bmap;
>>> >> >> +
>>> >> >> + /* used to wait for a free RX slot */
>>> >> >> + wait_queue_head_t wq;
>>> >> >> +
>>> >> >> + /* used to wait for an RX operation to complete */
>>> >> >> + struct dln2_rx_context slots[DLN2_MAX_RX_SLOTS];
>>> >> >> +
>>> >> >> + /* device has been disconnected */
>>> >> >> + bool disconnected;
>>> >> >
>>> >> > This belongs in the dln2_dev struct.
>>> >> >
>>> >> > I think you're overcomplicating the disconnect handling by intertwining
>>> >> > it with your slots.
>>> >> >
>>> >> > Add a lock, an active-transfer counter, a disconnected flag, and a wait
>>> >> > queue to struct dln2_dev.
>>> >> >
>>> >>
>>> >> I agree that disconnected is better suited in dln2_dev.
>>> >>
>>> >> However, I don't think that we need the active-transfer counter and a
>>> >> new wait queue. We can simply use the existing waiting queues and the
>>> >> implicit alloc_rx_slot()/free_rx_slot() calls to see if we are still
>>> >> waiting for I/O.
>>> >
>>> > Just because you can reuse them doesn't mean it's a good idea. By
>>> > separating a generic disconnect solution from your custom slot
>>> > implementation we get something that is way easier to verify for
>>> > correctness and that could also be reused in other drivers.
>>>
>>> Maybe I miss-understood what you are proposing, let me try to
>>> summarize it to see if I got it right.
>>>
>>> You are suggesting to add a counter, increment it in alloc_rx_slot(),
>>> decrement it in free_rx_slot().
>>
>> No increment it at the start of _dln2_transfer, and decrement it before
>> returning from that function.
>>
>>> Then add a new waitqueue in dln2_dev
>>> and in free_rx_slot() wake it up while in disconnect do a wait_event()
>>> on it and check for the counter.
>>
>> Where you also wake the disconnect (or wait-until-sent) wait queue.
>>
>>> Also, alloc_rx_slot() should fail if
>>> the disconnect flag is set.
>>
>> That is not required, but you can bail out early after alloc_rx_slot if
>> the disconnect flag is set (no locking).
>>
>>> In this case we are still coupled to the slots implementation, in the
>>> sense that you would need to understand the slots implementation to
>>> understand how the disconnect works. We are also doing two wake-up
>>> operations which I find redundant and which does not add much value in
>>> clarity (since we still need to wake-up all completions for each
>>> handle).
>>>
>>> I do agree that using a counter instead of checking the bitmaps is
>>> cleaner though.
>>
>> You only need to the wake up if disconnected is set when returning from
>> _dln2_transfer.
>>
>> Sure, the optimisation bit -- to abort any ongoing transfer -- still
>> requires some insight into the slot implementation.
>>
>> But this way everything disconnect related (correctness-wise) is
>> isolated to _dln2_transfer and dln2_disconnect.
>>
>
> OK, I see what you mean now. I'll give it a try and will follow up
> with a new patch set.
>
Johan, I think we don't really need the spinlock, the disconnect flag
and an atomic counter should work. Do you see any issues with that?
--
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: [GIT PULL] USB-serial fixes for v3.17-final
On Wed, Sep 24, 2014 at 10:29:22AM +0200, Johan Hovold wrote: > Hi Greg, > > Here are two more device ids for v3.17-final. As these are "just" new device ids, I've pulled these into my -next tree for 3.18-rc1 and they will get backported to 3.17-stable. 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 v6 partial v2 1/3] usb: gadget: pxa27x_udc: prepare device-tree support
- Mail original -
De: "Linus Walleij"
> @@ -2415,7 +2411,13 @@ static int pxa_udc_probe(struct platform_device *pdev)
> {
> struct resource *regs;
> struct pxa_udc *udc = &memory;
> - int retval = 0, gpio;
> + struct pxa2xx_udc_mach_info *mach = dev_get_platdata(&pdev->dev);
> + int retval = 0;
> +
> + if (mach) {
> + udc->gpiod = gpio_to_desc(mach->gpio_pullup);
> + udc->mach = mach;
> + }
> } else {
> udv->gpiod = devm_gpiod_get(&pdev->dev, ...);
> }
> Here you can also use the flags.
Yeah, but my problem is in the if statement, not in the else. The else is for
the DT/ACPI cases, in which there is no problem.
On the other hand, the "if" is for the legacy platform_data cases which bother
me.
The mach info contains :
- gpio_pullup : number of the gpio
- gpio_pullup_inverted : the active low state
What I would have needed would be something like :
if (mach) {
udc->gpiod = gpio_to_desc(mach->gpio_pullup);
gpiod_MAGIC_FUNC_set_flags(udc->gpiod, mach->gpio_pullup_inverted ?
ACTIVE_LOW : 0);
udc->mach = mach;
} else {
udv->gpiod = devm_gpiod_get(&pdev->dev, ...);
}
The "MAGIC_FUNC" is what I'm missing here. And I agree this is something that
is bound in
electronics, and eventually will have to be shifted to platform code. The
porting issue I
have is that I don't want the dependency of changing *all* pxa27x machines as a
dependency
for a patch to pxa27x_udc to support device-tree.
Or alternatively have something like :
udc->gpiod = gpio_to_desc_with_flags(mach->gpio_pullup, flags).
Or another way, to enable a smooth transition without the dependency.
Even I have not found any machine code using this inverted flag, I might have
overlooked
something, and I'm not feeling confortable killing a platform because I made a
wrong
decision.
Cheers.
--
Robert
--
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 v6 2/2] usb: gadget: pxa27x_udc: add devicetree support
On Wed, Sep 24, 2014 at 9:41 PM, Robert Jarzmik wrote:
> Add support for device-tree device discovery. If devicetree is not
> provided, fallback to legacy platform data "discovery".
>
> Signed-off-by: Robert Jarzmik
> Cc: devicet...@vger.kernel.org
(...)
> @@ -2417,6 +2425,8 @@ static int pxa_udc_probe(struct platform_device *pdev)
> if (mach) {
> udc->gpiod = gpio_to_desc(mach->gpio_pullup);
> udc->udc_command = mach->udc_command;
> + } else {
> + udc->gpiod = devm_gpiod_get(&pdev->dev, NULL);
> }
Aha OK there it is.
devm_gpiod_get() actually uses three arguments now, you're using
a deprecated version of the interface:
struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
const char *con_id,
enum gpiod_flags flags);
Yours,
Linus Walleij
--
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 v6 partial v2 1/3] usb: gadget: pxa27x_udc: prepare device-tree support
On Wed, Sep 24, 2014 at 11:05 PM, Robert Jarzmik wrote:
> For this preparation, a preliminary cleanup is done :
> - convert the probing of pxa27x_udc to gpio_desc.
> The conversion is partial because :
> - the platform data still provides a gpio number, not a gpio desc
> - the "invert" attribute is lost, hence a loss in the translation
>
> The drawback with the gpio_desc conversion is that the "inverted" gpio
> attribute is lost, as no gpiod_*() function exists to set the
> "active_low" state of a gpio, and that attribute was at driver's
> creation forecast to be set up by the driver and not the machine
> specific code.
This can be fixed as of kernel v3.17.
> #include
> +#include
You should only need the lower include, remove
.
> @@ -2415,7 +2411,13 @@ static int pxa_udc_probe(struct platform_device *pdev)
> {
> struct resource *regs;
> struct pxa_udc *udc = &memory;
> - int retval = 0, gpio;
> + struct pxa2xx_udc_mach_info *mach = dev_get_platdata(&pdev->dev);
> + int retval = 0;
> +
> + if (mach) {
> + udc->gpiod = gpio_to_desc(mach->gpio_pullup);
> + udc->mach = mach;
> + }
} else {
udv->gpiod = devm_gpiod_get(&pdev->dev, ...);
}
Here you can also use the flags.
So the idea is to use device-bound GPIOs for the future.
Please consult Documentation/gpio/consumer.txt
Yours,
Linus Walleij
--
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 v6 partial 0/2] pxa27x_udc port to device-tree and gpio_desc
On Thu, Sep 25, 2014 at 10:26 AM, Linus Walleij wrote: > On Wed, Sep 24, 2014 at 9:41 PM, Robert Jarzmik > wrote: > >> I have not found in the gpiolib anything for a driver to set that >> active_low value, only for machine code. The legacy behaviour was that >> that information was handed over to the driver. > > That's correct, active low is a property of the electronics, and > not something the driver should mess with going forward. However you can still do it if you have to. In the final v3.17 we have added a "flags" argument to devm_gpiod_get() and friends so you can set GPIOD_OUT_LOW / GPIOD_OUT_HIGH The old style without specifying flags also works, but we have moved to always adding the flags. Yours, Linus Walleij -- 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: USB3 WD drive failure on ASUS M5A 78L-M USB3
Can you try with the latest kernel? On 9/24/2014 6:13 AM, Len Berman wrote: I'm running debian 7. Linux foobar 3.2.0-4-amd64 #1 SMP Debian 3.2.60-1+deb7u3 x86_64 GNU/Linux. I have a Passport drive which fails on the USB3 connector. It works fine on USB2. It also works fine as USB3 on my Lenovo laptop. The output of lspci -v & lsusb -v is at http://outtopastureenterprises.com/device.txt When I plug in the device to the USB3 port I get the following in /var/log/messages. If I wait long enough, eventually it seems to connect. This can take tens of minutes. I don't know what else to include. Any ideas greatly appreciated. Thanks. --Len Sep 23 17:49:22 foobar kernel: [23209.332493] usb 4-1: new SuperSpeed USB device number 4 using xhci_hcd Sep 23 17:49:22 foobar kernel: [23209.352862] usb 4-1: New USB device found, idVendor=1058, idProduct=0820 Sep 23 17:49:22 foobar kernel: [23209.352875] usb 4-1: New USB device strings: Mfr=1, Product=2, SerialNumber=5 Sep 23 17:49:22 foobar kernel: [23209.352884] usb 4-1: Product: My Passport 0820 Sep 23 17:49:22 foobar kernel: [23209.352890] usb 4-1: Manufacturer: Western Digital Sep 23 17:49:22 foobar kernel: [23209.352897] usb 4-1: SerialNumber: 575835314142334434373334 Sep 23 17:49:22 foobar kernel: [23209.356231] scsi9 : usb-storage 4-1:1.0 Sep 23 17:49:22 foobar mtp-probe: checking bus 4, device 4: "/sys/devices/pci:00/:00:07.0/:02:00.0/usb4/4-1" Sep 23 17:49:22 foobar mtp-probe: bus: 4, device: 4 was not an MTP device Sep 23 17:49:23 foobar kernel: [23210.356893] scsi 9:0:0:0: Direct-Access WD My Passport 0820 1007 PQ: 0 ANSI: 6 Sep 23 17:49:23 foobar kernel: [23210.357131] scsi 9:0:0:1: Enclosure WD SES Device 1007 PQ: 0 ANSI: 6 Sep 23 17:49:23 foobar kernel: [23210.358448] sd 9:0:0:0: Attached scsi generic sg5 type 0 Sep 23 17:49:23 foobar kernel: [23210.358724] ses 9:0:0:1: Attached Enclosure device Sep 23 17:49:23 foobar kernel: [23210.358922] ses 9:0:0:1: Attached scsi generic sg6 type 13 Sep 23 17:49:54 foobar kernel: [23240.945357] usb 4-1: reset SuperSpeed USB device number 4 using xhci_hcd Sep 23 17:49:54 foobar kernel: [23240.962978] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with disabled ep 8801f234f2c0 Sep 23 17:49:54 foobar kernel: [23240.962992] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with disabled ep 8801f234f300 Sep 23 17:50:25 foobar kernel: [23271.953310] usb 4-1: reset SuperSpeed USB device number 4 using xhci_hcd Sep 23 17:50:25 foobar kernel: [23271.971070] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with disabled ep 8801f234f2c0 Sep 23 17:50:25 foobar kernel: [23271.971085] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with disabled ep 8801f234f300 Sep 23 17:50:25 foobar kernel: [23271.973226] sd 9:0:0:0: [sde] 1953458176 512-byte logical blocks: (1.00 TB/931 GiB) Sep 23 17:50:56 foobar kernel: [23302.993335] usb 4-1: reset SuperSpeed USB device number 4 using xhci_hcd Sep 23 17:50:56 foobar kernel: [23303.010894] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with disabled ep 8801f234f2c0 Sep 23 17:50:56 foobar kernel: [23303.010909] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with disabled ep 8801f234f300 Sep 23 17:51:27 foobar kernel: [2.973362] usb 4-1: reset SuperSpeed USB device number 4 using xhci_hcd Sep 23 17:51:27 foobar kernel: [2.990995] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with disabled ep 8801f234f2c0 Sep 23 17:51:27 foobar kernel: [2.991009] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with disabled ep 8801f234f300 -- 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 -- 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 v6 partial 0/2] pxa27x_udc port to device-tree and gpio_desc
On Wed, Sep 24, 2014 at 9:41 PM, Robert Jarzmik wrote: > I have not found in the gpiolib anything for a driver to set that > active_low value, only for machine code. The legacy behaviour was that > that information was handed over to the driver. That's correct, active low is a property of the electronics, and not something the driver should mess with going forward. Active low is now defined in either the descriptor table in the machine, in the device tree or in ACPI nodes. The old way was convoluted and something like: board file defines custom platform data -> driver gets this -> tells GPIOlib to use active low Or in worst case the polarity is hard-coded in the driver (!) That was not a clean separation, the driver should not need to care about polarity, the core should handle this. Yours, Linus Walleij -- 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 00/27] add pm_runtime_last_busy_and_autosuspend() helper
On Wed, Sep 24, 2014 at 10:28:07PM +0200, Rafael J. Wysocki wrote: > > OK, I guess this is as good as it gets. > > What tree would you like it go through? Since rest of the patches are dependent upon 1st patch which should go thru your tree, we should merge this thru your tree Thanks -- ~Vinod -- 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 3/3] usb: dwc3: add ACPI support
Adding ACPI ID used on newer Intel SoCs.
Signed-off-by: Heikki Krogerus
---
drivers/usb/dwc3/core.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index d08cac5..88e29f5 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include
#include
@@ -960,12 +961,24 @@ static const struct of_device_id of_dwc3_match[] = {
MODULE_DEVICE_TABLE(of, of_dwc3_match);
#endif
+#ifdef CONFIG_ACPI
+
+#define ACPI_ID_INTEL_BSW "808622B7"
+
+static const struct acpi_device_id dwc3_acpi_match[] = {
+ { ACPI_ID_INTEL_BSW, 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match);
+#endif
+
static struct platform_driver dwc3_driver = {
.probe = dwc3_probe,
.remove = dwc3_remove,
.driver = {
.name = "dwc3",
.of_match_table = of_match_ptr(of_dwc3_match),
+ .acpi_match_table = ACPI_PTR(dwc3_acpi_match),
.pm = DWC3_PM_OPS,
},
};
--
2.1.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
[RFC PATCH 1/4] usb: dwc3: add AMD NL support
Add PCI device ID of AMD NL.
Signed-off-by: Huang Rui
---
drivers/usb/dwc3/dwc3-pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 436fb08..cebbd01 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -30,6 +30,7 @@
#define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB30xabcd
#define PCI_DEVICE_ID_INTEL_BYT0x0f37
#define PCI_DEVICE_ID_INTEL_MRFLD 0x119e
+#define PCI_DEVICE_ID_AMD_NL 0x7912
struct dwc3_pci {
struct device *dev;
@@ -183,6 +184,7 @@ static const struct pci_device_id dwc3_pci_id_table[] = {
},
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT), },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRFLD), },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_NL), },
{ }/* Terminating Entry */
};
MODULE_DEVICE_TABLE(pci, dwc3_pci_id_table);
--
1.8.1.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
[RFC PATCH 4/4] usb: dwc3: introduce dual role switch function with debugfs
This patch implemented a feature to dynamic switch to host or device role under
debugfs for some physical limitation that unable to leverage connector A/B
cables (ID pin) to change roles.
The default role should be set as OTG mode. Then use below commands:
[1] switch to host:
echo host > /sys/kernel/debug/dwc3.0.auto/mode
[2] switch to device:
echo device > /sys/kernel/debug/dwc3.0.auto/mode
[3] switch to otg (default mode):
echo otg > /sys/kernel/debug/dwc3.0.auto/mode
Signed-off-by: Huang Rui
---
drivers/usb/dwc3/Makefile | 2 +-
drivers/usb/dwc3/core.c| 4 +-
drivers/usb/dwc3/core.h| 6 ++
drivers/usb/dwc3/debugfs.c | 19 --
drivers/usb/dwc3/drd.c | 163 +
drivers/usb/dwc3/drd.h | 30 +
drivers/usb/dwc3/gadget.c | 92 -
drivers/usb/dwc3/gadget.h | 3 +
drivers/usb/dwc3/host.c| 2 +
9 files changed, 280 insertions(+), 41 deletions(-)
create mode 100644 drivers/usb/dwc3/drd.c
create mode 100644 drivers/usb/dwc3/drd.h
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index bb34fbc..115bbc7 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -13,7 +13,7 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_HOST)
$(CONFIG_USB_DWC3_DUAL_ROLE)),)
endif
ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) $(CONFIG_USB_DWC3_DUAL_ROLE)),)
- dwc3-y += gadget.o ep0.o
+ dwc3-y += gadget.o ep0.o drd.o
endif
ifneq ($(CONFIG_DEBUG_FS),)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 6138c5d..7b50584 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -61,7 +61,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
* dwc3_core_soft_reset - Issues core soft reset and PHY reset
* @dwc: pointer to our context structure
*/
-static int dwc3_core_soft_reset(struct dwc3 *dwc)
+int dwc3_core_soft_reset(struct dwc3 *dwc)
{
u32 reg;
int ret;
@@ -228,7 +228,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc,
unsigned length)
*
* Returns 0 on success otherwise negative errno.
*/
-static int dwc3_event_buffers_setup(struct dwc3 *dwc)
+int dwc3_event_buffers_setup(struct dwc3 *dwc)
{
struct dwc3_event_buffer*evt;
int n;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index a1c7dc5..3a30f33 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -643,6 +643,8 @@ struct dwc3_scratchpad_array {
* @maximum_speed: maximum speed requested (mainly for testing purposes)
* @revision: revision register contents
* @quirks: represents different SOCs hardware work-arounds and quirks
+ * @has_gadget: true when gadget is initialized
+ * @has_xhci: true when xhci is initialized
* @dr_mode: requested mode of operation
* @usb2_phy: pointer to USB2 PHY
* @usb3_phy: pointer to USB3 PHY
@@ -750,6 +752,8 @@ struct dwc3 {
#define DWC3_AMD_NL_PLAT (1 << 0)
+ boolhas_gadget;
+ boolhas_xhci;
enum dwc3_ep0_next ep0_next_event;
enum dwc3_ep0_state ep0state;
enum dwc3_link_statelink_state;
@@ -935,6 +939,8 @@ struct dwc3_gadget_ep_cmd_params {
/* prototypes */
void dwc3_set_mode(struct dwc3 *dwc, u32 mode);
int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
+int dwc3_core_soft_reset(struct dwc3 *dwc);
+int dwc3_event_buffers_setup(struct dwc3 *dwc);
#if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
int dwc3_host_init(struct dwc3 *dwc);
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 9ac37fe..76384df 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -32,6 +32,7 @@
#include "gadget.h"
#include "io.h"
#include "debug.h"
+#include "drd.h"
#define dump_register(nm) \
{ \
@@ -394,7 +395,6 @@ static ssize_t dwc3_mode_write(struct file *file,
{
struct seq_file *s = file->private_data;
struct dwc3 *dwc = s->private;
- unsigned long flags;
u32 mode = 0;
charbuf[32];
@@ -410,10 +410,19 @@ static ssize_t dwc3_mode_write(struct file *file,
if (!strncmp(buf, "otg", 3))
mode |= DWC3_GCTL_PRTCAP_OTG;
- if (mode) {
- spin_lock_irqsave(&dwc->lock, flags);
- dwc3_set_mode(dwc, mode);
- spin_unlock_irqrestore(&dwc->lock, flags);
+ switch (mode) {
+ case DWC3_GCTL_PRTCAP_DEVICE:
+ dwc3_drd_to_device(dwc);
+ break;
+ case DWC3_GCTL_PRTCAP_HOST:
+ dwc3_drd_to_host(dwc);
+ break;
+ case DWC3_GCTL_PRTCAP_OTG:
+ dwc3_drd_to_otg(dwc);
+
[RFC PATCH 0/4] usb: dwc3: add support for AMD NL SoC
Hi, The series of patches add AMD NL SoC support for DesignWare USB3 OTG IP with PCI bus glue layer. And add a role switch feature with debugfs in order that create an entry to alternative roles between host and device. Because of the limitation for PCI-based temporary platform currently, it's unable to switch roles by connector A/B cables physically. These patches are generated on balbi/next Patch 1: - Add PCI device id into pci bus glue. Patch 2: - Add some register macros for patch 3 use. Patch 3: - Add a quirk to programe GCTL and PIPE3 to configure controller and PHY interface specifically for AMD NL SoCs. Patch 4: - This patch add a feature to dynamic switch roles on user space like below, switch to host: echo host > /sys/kernel/debug/dwc3.0.auto/mode switch to device: echo device > /sys/kernel/debug/dwc3.0.auto/mode switch to otg (default): echo otg > /sys/kernel/debug/dwc3.0.auto/mode Thanks, Rui Huang Rui (4): usb: dwc3: add AMD NL support usb: dwc3: add some register macros for future use usb: dwc3: add quirk to be compatible for AMD NL usb: dwc3: introduce dual role switch function with debugfs drivers/usb/dwc3/Makefile| 2 +- drivers/usb/dwc3/core.c | 37 - drivers/usb/dwc3/core.h | 20 + drivers/usb/dwc3/debugfs.c | 19 +++-- drivers/usb/dwc3/drd.c | 163 +++ drivers/usb/dwc3/drd.h | 30 +++ drivers/usb/dwc3/dwc3-pci.c | 16 drivers/usb/dwc3/gadget.c| 100 drivers/usb/dwc3/gadget.h| 3 + drivers/usb/dwc3/host.c | 2 + drivers/usb/dwc3/platform_data.h | 1 + 11 files changed, 351 insertions(+), 42 deletions(-) create mode 100644 drivers/usb/dwc3/drd.c create mode 100644 drivers/usb/dwc3/drd.h -- 1.8.1.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
[RFC PATCH 2/4] usb: dwc3: add some register macros for future use
This patch adds some undefined register macros on DWC3 spec 2.80a, these macros will be used on AMD NL specific configuration. Signed-off-by: Huang Rui --- drivers/usb/dwc3/core.h | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 66f6256..ccde369 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -166,6 +166,7 @@ #define DWC3_GCTL_SCALEDOWN(n) ((n) << 4) #define DWC3_GCTL_SCALEDOWN_MASK DWC3_GCTL_SCALEDOWN(3) #define DWC3_GCTL_DISSCRAMBLE (1 << 3) +#define DWC3_GCTL_U2EXIT_LFPS (1 << 2) #define DWC3_GCTL_GBLHIBERNATIONEN (1 << 1) #define DWC3_GCTL_DSBLCLKGTNG (1 << 0) @@ -175,7 +176,14 @@ /* Global USB3 PIPE Control Register */ #define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31) +#define DWC3_GUSB3PIPECTL_U2SSINP3OK (1 << 29) +#define DWC3_GUSB3PIPECTL_UX_EXITINPX (1 << 27) +#define DWC3_GUSB3PIPECTL_U1U2EXITFAIL (1 << 25) +#define DWC3_GUSB3PIPECTL_DEP1P2P3(n) ((n) << 19) +#define DWC3_GUSB3PIPECTL_DEPOCHANGE (1 << 18) #define DWC3_GUSB3PIPECTL_SUSPHY (1 << 17) +#define DWC3_GUSB3PIPECTL_RX_DETOPOLL (1 << 8) +#define DWC3_GUSB3PIPECTL_TX_DEEPH(n) ((n) << 1) /* Global TX Fifo Size Register */ #define DWC3_GTXFIFOSIZ_TXFDEF(n) ((n) & 0x) @@ -243,6 +251,7 @@ #define DWC3_DCTL_TRGTULST_SS_INACT(DWC3_DCTL_TRGTULST(6)) /* These apply for core versions 1.94a and later */ +#define DWC3_DCTL_LPM_ERRATA(n)((n) << 20) #define DWC3_DCTL_KEEP_CONNECT (1 << 19) #define DWC3_DCTL_L1_HIBER_EN (1 << 18) #define DWC3_DCTL_CRS (1 << 17) -- 1.8.1.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
[RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
This patch add a quirk to be compatible for AMD NL SoC
Some specific behaviors on NL:
- configure USB3 PIPE registers
- enable GCTL disscramble
- enable U2EXIT_LFPS
- enable hibernation at the global level
- set LPM errata dissabled
Signed-off-by: Huang Rui
---
drivers/usb/dwc3/core.c | 33 -
drivers/usb/dwc3/core.h | 5 +
drivers/usb/dwc3/dwc3-pci.c | 14 ++
drivers/usb/dwc3/gadget.c| 8
drivers/usb/dwc3/platform_data.h | 1 +
5 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index b0f4d52..6138c5d 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
}
/**
+ * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
+ * @dwc: Pointer to our controller context structure
+ */
+static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
+{
+ u32 reg = 0;
+
+ reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK | DWC3_GUSB3PIPECTL_UX_EXITINPX
+ | DWC3_GUSB3PIPECTL_UX_EXITINPX | DWC3_GUSB3PIPECTL_U1U2EXITFAIL
+ | DWC3_GUSB3PIPECTL_DEPOCHANGE | DWC3_GUSB3PIPECTL_RX_DETOPOLL;
+
+ reg |= DWC3_GUSB3PIPECTL_DEP1P2P3(1) | DWC3_GUSB3PIPECTL_TX_DEEPH(1);
+
+ dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
+
+ mdelay(100);
+}
+
+/**
* dwc3_free_one_event_buffer - Frees one event buffer
* @dwc: Pointer to our controller context structure
* @evt: Pointer to event buffer to be freed
@@ -412,9 +431,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
if (ret)
goto err0;
+ if (dwc->quirks & DWC3_AMD_NL_PLAT)
+ dwc3_core_nl_set_pipe3(dwc);
+
reg = dwc3_readl(dwc->regs, DWC3_GCTL);
+
reg &= ~DWC3_GCTL_SCALEDOWN_MASK;
- reg &= ~DWC3_GCTL_DISSCRAMBLE;
+
+ if (dwc->quirks & DWC3_AMD_NL_PLAT) {
+ reg |= DWC3_GCTL_DISSCRAMBLE;
+ reg |= DWC3_GCTL_U2EXIT_LFPS;
+ reg |= DWC3_GCTL_GBLHIBERNATIONEN;
+ } else
+ reg &= ~DWC3_GCTL_DISSCRAMBLE;
switch (DWC3_GHWPARAMS1_EN_PWROPT(dwc->hwparams.hwparams1)) {
case DWC3_GHWPARAMS1_EN_PWROPT_CLK:
@@ -695,6 +724,8 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->needs_fifo_resize = pdata->tx_fifo_resize;
dwc->dr_mode = pdata->dr_mode;
+
+ dwc->quirks = pdata->quirks;
}
/* default to superspeed if no maximum_speed passed */
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index ccde369..a1c7dc5 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -642,6 +642,7 @@ struct dwc3_scratchpad_array {
* @u1u2: only used on revisions <1.83a for workaround
* @maximum_speed: maximum speed requested (mainly for testing purposes)
* @revision: revision register contents
+ * @quirks: represents different SOCs hardware work-arounds and quirks
* @dr_mode: requested mode of operation
* @usb2_phy: pointer to USB2 PHY
* @usb3_phy: pointer to USB3 PHY
@@ -745,6 +746,10 @@ struct dwc3 {
#define DWC3_REVISION_270A 0x5533270a
#define DWC3_REVISION_280A 0x5533280a
+ u32 quirks;
+
+#define DWC3_AMD_NL_PLAT (1 << 0)
+
enum dwc3_ep0_next ep0_next_event;
enum dwc3_ep0_state ep0state;
enum dwc3_link_statelink_state;
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index cebbd01..7f471ff 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -25,6 +25,9 @@
#include
#include
+#include "platform_data.h"
+#include "core.h"
+
/* FIXME define these in */
#define PCI_VENDOR_ID_SYNOPSYS 0x16c3
#define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB30xabcd
@@ -102,6 +105,9 @@ static int dwc3_pci_probe(struct pci_dev *pci,
struct dwc3_pci *glue;
int ret;
struct device *dev = &pci->dev;
+ struct dwc3_platform_data dwc3_pdata;
+
+ memset(&dwc3_pdata, 0x00, sizeof(dwc3_pdata));
glue = devm_kzalloc(dev, sizeof(*glue), GFP_KERNEL);
if (!glue)
@@ -148,6 +154,14 @@ static int dwc3_pci_probe(struct pci_dev *pci,
pci_set_drvdata(pci, glue);
+ if (pci->vendor == PCI_VENDOR_ID_AMD &&
+ pci->device == PCI_DEVICE_ID_AMD_NL)
+ dwc3_pdata.quirks |= DWC3_AMD_NL_PLAT;
+
+ ret = platform_device_add_data(dwc3, &dwc3_pdata, sizeof(dwc3_pdata));
+ if (ret)
+ goto err3;
+
dma_set_coherent_mask(&dwc3->dev, dev->coherent_dma_mask);
dwc3->dev.dma_mask = dev->dma_mask;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0fcc0a3..8277065 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq
Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
On Thursday 25 September 2014 09:16:48 Peter Chen wrote:
> > + }
> > +
> > + if (dev->of_node) {
> > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata);
> > + if (ret)
> > + goto clk_err;
> > + } else {
> > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> > + if (ret)
> > + goto clk_err;
> > + }
>
> Hi Antoine, the above code may not be needed, since get phy and set dma
> mask are common operation, we can do it at core code, the only thing you
> need to do is something like: dev->dma_mask = DMA_BIT_MASK(32).
>
Certainly not, doing that would be broken for a number of reasons:
- dev->dma_mask is a pointer, a driver should not reassign that
- the device may have been set up for a bus that requires a smaller mask
that is already set, changing the mask would cause data corruption
- setting just the dma mask but not coherent mask is wrong
- setting the dma mask can fail, e.g. if the mask is smaller than the
smallest memory zone, so you have to check the return value.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] phy: improved lookup method
> >>> Assume you have 2 phys in your system..
> >>> static struct phy_lookup usb_lookup = {
> >>> .phy_name = "phy-usb.0",
> >>> .dev_id = "usb.0",
> >>> .con_id = "usb",
> >>> };
> >>>
> >>> static struct phy_lookup sata_lookup = {
> >>> .phy_name = "sata-usb.1",
> >>> .dev_id = "sata.0",
> >>> .con_id = "sata",
> >>> };
> >>>
> >>> First you do modprobe phy-usb, the probe of USB PHY driver gets
> >>> invoked and it
> >>> creates the PHY. The phy-core will find a free id (now it will be 0)
> >>> and then
> >>> name the phy as phy-usb.0.
> >>> Then with modprobe phy-sata, the phy-core will create phy-sata.1.
> >>>
> >>> This is an ideal case where the .phy_name in phy_lookup matches.
> >>>
> >>> Consider if the order is flipped and the user does modprobe phy-sata
> >>> first. The
> >>> phy_names won't match anymore (the sata phy device name would be
> >>> "sata-usb.0").
> >
> > Actually, I don't think there would be this problem if we used the
> > name of the actual device which is the parent of phy devices, right?
>
> hmm.. but if the parent is a multi-phy phy provider (like pipe3 PHY
> driver), we
> might end up with the same problem.
> >>>
> >>> I'm not completely sure what you mean? If you are talking about
> >>> platforms with multiple instances of a single phy, I don't see how
> >>> there could ever be a scenario where we did not know the order in
> >>> which they were enumerated. Can you give an example again?
> >>
> >> If a single IP implements multiple PHYs (phy-miphy365x.c in linux-phy
> >> next),
> >> the parent for all the phy devices would be the same.
Hold on...
Let's take a step back here. Where could we actually have a scenario
where the phy device, the dev_id (consumer) and the con_id would all
be the same? There can't be such a case.
It's not like you could ever have a driver requesting multiple phys
with the same con_id. You would just get the same phy handle even if
you used dt.
phy1 = phy_get(dev, "phy");
...
phy2 = phy_get(dev, "phy");
And if the drivers requesting those phys are different, your consumers
are different.
> Isn't making the PHY to be aware of it's user much simpler?
No it's not. I'm not going into this again. We have already gone
through this in the past.
Cheers,
--
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
