[GIT PULL] USB fixes for v4.20-rc4
Hi Greg, here's my second pull request for the current -rc cycle. Looks like things are really calm this merge window. Let me know if you want anything to be changed. cheers The following changes since commit 2e6e902d185027f8e3cb8b7305238f7e35d6a436: Linux 4.20-rc4 (2018-11-25 14:19:31 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git tags/fixes-for-v4.20-rc4 for you to fetch changes up to c9287fa657b3328b4549c0ab39ea7f197a3d6a50: usb: gadget: u_ether: fix unsafe list iteration (2018-11-28 08:46:26 +0200) USB: fixes for v4.20-rc4 In this second set of fixes for the current -rc cycle, we have some regressions fixes for the old omap_udc driver done by Aaro Koskinen. We're also reverting an old patch on dwc3 which is, now, known to break USB certification in some cases. We have a fix on u_ether for an unsafe list iteration. Aaro Koskinen (5): USB: omap_udc: use devm_request_irq() USB: omap_udc: fix crashes on probe error and module removal USB: omap_udc: fix omap_udc_start() on 15xx machines USB: omap_udc: fix USB gadget functionality on Palm Tungsten E USB: omap_udc: fix rejection of out transfers when DMA is used Felipe Balbi (1): Revert "usb: dwc3: gadget: skip Set/Clear Halt when invalid" Marek Szyprowski (1): usb: gadget: u_ether: fix unsafe list iteration drivers/usb/dwc3/gadget.c | 5 -- drivers/usb/gadget/function/u_ether.c | 11 +++-- drivers/usb/gadget/udc/omap_udc.c | 88 --- 3 files changed, 37 insertions(+), 67 deletions(-) -- balbi signature.asc Description: PGP signature
Re: Support Mac address pass through on Dell DS1000 dock
Le 27/11/2018 à 21:27, mario.limoncie...@dell.com a écrit :
Le 27/11/2018 à 16:19, mario.limoncie...@dell.com a écrit :
Some news from today.
I realized that I was not always restarting the computer when changing
the Mac pass-through setting in the BIOS.
Sometimes I was just hibernating, and this could be the origin of some
failures.
OK. Great.
The second usbc dongle works again so there was no permanent change to
the device.
But Mac pass-through does not work for this device (even from a fresh
restart with no dock connected). This is what I get when connecting it:
Nov 27 11:06:53 latitude5285 kernel: [ 61.796105] usb 2-1: new
SuperSpeed Gen 1 USB device number 2 using xhci_hcd
Nov 27 11:06:53 latitude5285 kernel: [ 61.816686] usb 2-1: New USB
device found, idVendor=0bda, idProduct=8153, bcdDevice=30.00
Nov 27 11:06:53 latitude5285 kernel: [ 61.816688] usb 2-1: New USB
device strings: Mfr=1, Product=2, SerialNumber=6
Nov 27 11:06:53 latitude5285 kernel: [ 61.816688] usb 2-1: Product:
USB 10/100/1000 LAN
Nov 27 11:06:53 latitude5285 kernel: [ 61.816689] usb 2-1:
Manufacturer: Realtek
Nov 27 11:06:53 latitude5285 kernel: [ 61.816690] usb 2-1:
SerialNumber: 01
Nov 27 11:06:53 latitude5285 kernel: [ 61.835060] usbcore: registered
new interface driver r8152
Nov 27 11:06:53 latitude5285 kernel: [ 61.837654] usbcore: registered
new interface driver cdc_ether
Nov 27 11:06:53 latitude5285 kernel: [ 61.964264] usb 2-1: reset
SuperSpeed Gen 1 USB device number 2 using xhci_hcd
Nov 27 11:06:53 latitude5285 kernel: [ 62.016553] r8152 2-1:1.0 eth0:
v1.09.9
Nov 27 11:06:53 latitude5285 kernel: [ 62.021108] r8152 2-1:1.0
enx00e04c68035e: renamed from eth0
Nov 27 11:06:53 latitude5285 kernel: [ 62.041086] IPv6:
ADDRCONF(NETDEV_UP): enx00e04c68035e: link is not ready
Nov 27 11:06:53 latitude5285 kernel: [ 62.045852] IPv6:
ADDRCONF(NETDEV_UP): enx00e04c68035e: link is not ready
Did you have the debugging patch applied and the dynamic debugging properly
enabled during this time?
Yes
I don't see any lines related to the MAC pass through failures here.
And to be clear - was this particular dongle ever working for MAC pass through?
I don't know, I never tested it on windows.
If you have dynamic debugging turned on properly and this dongle meets the
criteria
then the only thing that I can see looking through the code that would explain
this is
tp->version == RTL_VER_01 on your particular dongle matching.
You can try this to see if it helps.
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f1b5201..29734ec 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1214,15 +1214,15 @@ static int set_ethernet_addr(struct r8152 *tp)
struct sockaddr sa;
int ret;
- if (tp->version == RTL_VER_01) {
- ret = pla_ocp_read(tp, PLA_IDR, 8, sa.sa_data);
- } else {
- /* if this is not an RTL8153-AD, no eFuse mac pass thru set,
-* or system doesn't provide valid _SB.AMAC this will be
-* be expected to non-zero
-*/
- ret = vendor_mac_passthru_addr_read(tp, &sa);
- if (ret < 0)
+ /* if this is not an RTL8153-AD, no eFuse mac pass thru set,
+* or system doesn't provide valid _SB.AMAC this will be
+* be expected to non-zero
+*/
+ ret = vendor_mac_passthru_addr_read(tp, &sa);
+ if (ret < 0) {
+ if (tp->version == RTL_VER_01)
+ ret = pla_ocp_read(tp, PLA_IDR, 8, sa.sa_data);
+ else
ret = pla_ocp_read(tp, PLA_BACKUP, 8, sa.sa_data);
}
I tried with your patch but the Mac address pass-through still does not
work with the usbc adaptor, so it probably does not support it.
So I will buy a Dell one, it should work.
Thanks anyway. Frédéric
Re: [GIT PULL] USB fixes for v4.20-rc4
On Thu, Nov 29, 2018 at 12:26:29PM +0200, Felipe Balbi wrote: > > Hi Greg, > > here's my second pull request for the current -rc cycle. Looks like > things are really calm this merge window. > > Let me know if you want anything to be changed. > > cheers > > The following changes since commit 2e6e902d185027f8e3cb8b7305238f7e35d6a436: > > Linux 4.20-rc4 (2018-11-25 14:19:31 -0800) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git > tags/fixes-for-v4.20-rc4 Pulled and pushed out, thanks. greg k-h
RE: Support Mac address pass through on Dell DS1000 dock
> I tried with your patch but the Mac address pass-through still does not > work with the usbc adaptor, so it probably does not support it. > > So I will buy a Dell one, it should work. > > Thanks anyway. Frédéric OK, sounds good. Oliver, That debugging patch that provided to Frédéric, would you consider to formally submit it? I do think it could be useful if a similar situation crops up again down the road to make debugging easier. Thanks,
RE: [PATCH v3 4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups
> > On Tue, Nov 27, 2018 at 7:31 AM PETER CHEN wrote: > > > > For USB HSIC, the data and strobe pin needs to be pulled down at > > default, we consider it as "idle" state. When the USB host is ready to > > be used, the strobe pin needs to be pulled up, we consider it as > > "active" state. > > > > Signed-off-by: Peter Chen > > --- > > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 8 +++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > index 529e51879fb2..1e5e7ddfb1a5 100644 > > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > @@ -80,7 +80,10 @@ Optional properties: > >controller. It's expected that a mux state of 0 indicates device mode > > and a > >mux state of 1 indicates host mode. > > - mux-control-names: Shall be "usb_switch" if mux-controls is specified. > > -- pinctrl-names: Names for optional pin modes in "default", "host", > > "device" > > +- pinctrl-names: Names for optional pin modes in "default", "host", > > "device". > > + In case of HSIC-mode, "idle" and "active" pin modes are mandatory. > > +In this > > + case, the "idle" state needs to pull down the data and strobe pin > > + and the "active" state needs to pull up the strobe pin. > > - pinctrl-n: alternate pin modes > > > > i.mx specific properties > > @@ -110,4 +113,7 @@ Example: > > phy-clkgate-delay-us = <400>; > > mux-controls = <&usb_switch>; > > mux-control-names = "usb_switch"; > > + pinctrl-names = "idle", "active"; > > + pinctrl-0 = <&pinctrl_usbh2_1>; > > + pinctrl-1 = <&pinctrl_usbh2_2>; > > I would put the "idle" and "active" entries inside a explicit section for > HSIC. > > Otherwise, it may confuse people using non-HSIC bindings. It is just an example; the user should check the meaning for optional properties before using it, like the property "phy-clkgate-delay-us", only imx7d/7s needs it. Peter
RE: [PATCH v3 3/4] usb: chipidea: host: override ehci->hub_control
> On 27.11.18 10:30, PETER CHEN wrote:
> > The chipidea controller has some special requirements during
> > suspend/resume, override common ehci->hub_control to implement it.
> >
> > Signed-off-by: Peter Chen
>
> Tested on i.MX6S with Microchip LAN9730 USB HSIC Ethernet Controller.
> I have some minor format fixes below. Apart from that:
>
> Reviewed-by: Frieder Schrempf
> Tested-by: Frieder Schrempf
>
> > ---
> > drivers/usb/chipidea/host.c | 76
> +
> > 1 file changed, 76 insertions(+)
> >
> > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > index 028a3574266a..aa4756dab487 100644
> > --- a/drivers/usb/chipidea/host.c
> > +++ b/drivers/usb/chipidea/host.c
> > @@ -220,6 +220,81 @@ void ci_hdrc_host_destroy(struct ci_hdrc *ci)
> > host_stop(ci);
> > }
> >
> > +/* The below code is based on tegra ehci driver */ static int
> > +ci_ehci_hub_control(
> > + struct usb_hcd *hcd,
> > + u16 typeReq,
> > + u16 wValue,
> > + u16 wIndex,
> > + char*buf,
> > + u16 wLength
> > +)
> > +{
> > + struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> > + u32 __iomem *status_reg;
> > + u32 temp;
> > + unsigned long flags;
> > + int retval = 0;
> > + struct device *dev = hcd->self.controller;
> > + struct ci_hdrc *ci = dev_get_drvdata(dev);
> > +
> > + status_reg = &ehci->regs->port_status[(wIndex & 0xff) - 1];
> > +
> > + spin_lock_irqsave(&ehci->lock, flags);
> > +
> > + if (typeReq == SetPortFeature && wValue == USB_PORT_FEAT_SUSPEND)
> {
> > + temp = ehci_readl(ehci, status_reg);
> > + if ((temp & PORT_PE) == 0 || (temp & PORT_RESET) != 0) {
> > + retval = -EPIPE;
> > + goto done;
> > + }
> > +
> > + temp &= ~(PORT_RWC_BITS | PORT_WKCONN_E);
> > + temp |= PORT_WKDISC_E | PORT_WKOC_E;
> > + ehci_writel(ehci, temp | PORT_SUSPEND, status_reg);
> > +
> > + /*
> > +* If a transaction is in progress, there may be a delay in
> > +* suspending the port. Poll until the port is suspended.
> > +*/
> > + if (ehci_handshake(ehci, status_reg, PORT_SUSPEND,
> > + PORT_SUSPEND, 5000))
>
> Indentation: ^^
>
> > + ehci_err(ehci, "timeout waiting for SUSPEND\n");
> > +
> > + if (ci->platdata->flags & CI_HDRC_IMX_IS_HSIC) {
> > + if (ci->platdata->notify_event)
> > + ci->platdata->notify_event
> > + (ci, CI_HDRC_IMX_HSIC_SUSPEND_EVENT);
>
> I think the open parentheses and the first parameter should be on the first
> line:
>
> ci->platdata->notify_event(ci,
> CI_HDRC_IMX_HSIC_SUSPEND_EVENT);
>
> > +
> > + temp = ehci_readl(ehci, status_reg);
> > + temp &= ~(PORT_WKDISC_E | PORT_WKCONN_E);
> > + ehci_writel(ehci, temp, status_reg);
> > + }
> > +
> > + set_bit((wIndex & 0xff) - 1, &ehci->suspended_ports);
> > + goto done;
> > + }
> > +
> > + /*
> > +* After resume has finished, it needs do some post resume
> > +* operation for some SoCs.
> > +*/
> > + else if (typeReq == ClearPortFeature &&
> > + wValue == USB_PORT_FEAT_C_SUSPEND)
> {
>
> Indentation: ^ ^
>
Thanks, I will change these code style comments for next revision.
Peter
RE: [PATCH] usb: chipidea: imx: Allow OC polarity active low
>
> The implementation for setting the over current polarity has always been the
> over-
> current-active-high property. The problem with this is there is no way to
> enable
> over current active low polarity since the default state of the register bit
> that controls
> the over current polarity is a 0 which means active high. This value never
> actually
> did anything since active high is already the default state. Also it is not
> used in any
> device tree source in the kernel. The default register bit state was
> verified by
> checking the i.MX6Q and i.MX7Dual reference manuals.
>
> The change replaces the over-current-active-high property with the
> over-current-
> active-low property. Without this property the over current polarity will be
> active
> high by default like it always has been.
> With the property the over current polarity will be active low.
>
Would you please check it? Why it is opposite for your patch and Matthew's?
Peter
> Signed-off-by: Matthew Starr
> ---
> Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 4 ++--
> drivers/usb/chipidea/ci_hdrc_imx.c | 2 +-
> drivers/usb/chipidea/usbmisc_imx.c | 10 ++
> 3 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index 529e51879fb2..3dee58037839 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -87,8 +87,8 @@ i.mx specific properties
> - fsl,usbmisc: phandler of non-core register device, with one
>argument that indicate usb controller index
> - disable-over-current: disable over current detect
> -- over-current-active-high: over current signal polarity is high active,
> - typically over current signal polarity is low active.
> +- over-current-active-low: over current signal polarity is low active,
> + the default signal polarity is high active.
> - external-vbus-divider: enables off-chip resistor divider for Vbus
>
> Example:
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 09b37c0d075d..f7069544fb42 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -135,7 +135,7 @@ static struct imx_usbmisc_data
> *usbmisc_get_init_data(struct device *dev)
> if (of_find_property(np, "disable-over-current", NULL))
> data->disable_oc = 1;
>
> - if (of_find_property(np, "over-current-active-high", NULL))
> + if (of_find_property(np, "over-current-active-low", NULL))
> data->oc_polarity = 1;
>
> if (of_find_property(np, "external-vbus-divider", NULL)) diff --git
> a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index def80ff547e4..39223708e859 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -341,8 +341,9 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data
> *data)
> if (data->disable_oc) {
> reg |= MX6_BM_OVER_CUR_DIS;
> } else if (data->oc_polarity == 1) {
> - /* High active */
> - reg &= ~(MX6_BM_OVER_CUR_DIS |
> MX6_BM_OVER_CUR_POLARITY);
> + /* Low active */
> + reg &= ~(MX6_BM_OVER_CUR_DIS);
> + reg |= MX6_BM_OVER_CUR_POLARITY;
> } else {
> reg &= ~(MX6_BM_OVER_CUR_DIS);
> }
> @@ -445,8 +446,9 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data
> *data)
> if (data->disable_oc) {
> reg |= MX6_BM_OVER_CUR_DIS;
> } else if (data->oc_polarity == 1) {
> - /* High active */
> - reg &= ~(MX6_BM_OVER_CUR_DIS |
> MX6_BM_OVER_CUR_POLARITY);
> + /* Low active */
> + reg &= ~(MX6_BM_OVER_CUR_DIS);
> + reg |= MX6_BM_OVER_CUR_POLARITY;
> }
> writel(reg, usbmisc->base);
>
> --
> 2.17.1
Re: [PATCH] [stable, netdev 4.4+] lan78xx: make sure RX_ADDRL & RX_ADDRH regs are always up to date
On Fri, Nov 09, 2018 at 12:31:59PM +0100, Paolo Pisati wrote: > On Thu, Nov 08, 2018 at 10:49:04AM -0500, Sasha Levin wrote: > > > > Can you confirm it actually works on 4.4? > > Yes, built and tested on 4.4.y: > > Tested-by: Paolo Pisati Now queued up, thanks. greg k-h
RE: [PATCH V6 01/10] usb: gadget: udc: Add timer for stream capable endpoints
Hi,
Anurag Kumar Vulisha writes:
>>> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
>>> index af88b48..41cc23b 100644
>>> --- a/drivers/usb/gadget/udc/core.c
>>> +++ b/drivers/usb/gadget/udc/core.c
>>> @@ -52,6 +52,24 @@ static int udc_bind_to_driver(struct usb_udc *udc,
>>> /*
>>> - */
>>>
>>> /**
>>> + * usb_ep_stream_timeout - callback function for endpoint stream timeout
>>> timer
>>> + * @arg: pointer to struct timer_list
>>> + *
>>> + * This function gets called only when bulk streams are enabled in the
>>> endpoint
>>> + * and only after ep->stream_timeout_timer has expired. The timer gets
>>> expired
>>> + * only when the gadget controller failed to find a valid stream event for
>>> this
>>> + * endpoint. On timer expiry, this function calls the endpoint-specific
>>> timeout
>>> + * handler registered to endpoint ops->stream_timeout API.
>>> + */
>>> +static void usb_ep_stream_timeout(struct timer_list *arg)
>>> +{
>>> + struct usb_ep *ep = from_timer(ep, arg, stream_timeout_timer);
>>> +
>>> + if (ep->stream_capable && ep->ops->stream_timeout)
>>
>>why is the timer only for stream endpoints? What if we want to run this
>>on non-stream capable eps?
>>
>
> I have seen this issue only with stream capable eps between PRIME &
> EPRDY. But this issue can potentially occur with non-stream endpoints
> also. Will remove this stream capable checks in next series of patch.
you're adding support for transfer timeouts, which some gadgets may want
regardless of endpoint type. One use is to correct cases of streams
going out of sync.
>>> + ep->ops->stream_timeout(ep);
>>
>>you don't ned an extra operation here, ep_dequeue should be enough.
>>
>
> I think issuing ep_dequeue() would be more trickier than calling
> ep->ops->stream_timeout().
> This is because, every gadget driver has their own way of handling
> ep_dequeue. Some
> drivers (like dwc3) may sleep for an event (wait_event_lock_irq) in the
> ep_dequeue routine.
not anymore. There's, now, a requirement that ->dequeue can be called
from any context.
> Since we are calling ep_dequeue from timer timeout callback which is in
> softirq context,
> sleeping or waiting for an event would hang the system. Also adding
> ep->ops->stream_timeout()
> would make the gadget drivers handle the issue in better way based on their
> implementation.
no problems
> Another advantage is the drivers which doesn't have this timeout issue
> doesn't have to register the
> timeout handler and can avoid the logic of deleting the timer for every
> request. So, I think
> ep->ops->stream_timeout() would be better than having a generic handler which
> does
> ep_dequeue() & ep_queue(). Please provide your suggestion on this
> implementation.
call ep_dequeue()
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH] [stable, netdev 4.4+] lan78xx: make sure RX_ADDRL & RX_ADDRH regs are always up to date
On Thu, Nov 08, 2018 at 10:49:04AM -0500, Sasha Levin wrote: > On Thu, Nov 08, 2018 at 12:01:27PM +0100, Paolo Pisati wrote: > > On Wed, Nov 07, 2018 at 07:17:51PM -0500, Sasha Levin wrote: > > > So why not just take 760db29bdc completely? It looks safer than taking a > > > partial backport, and will make applying future patches easier. > > > > > > I tried to do it and it doesn't look like there are any dependencies > > > that would cause an issue. > > > > Somehow i was convinced it didn't build on 4.4.x... can you pick it up? > > > > commit 760db29bdc97b73ff60b091315ad787b1deb5cf5 > > Author: Phil Elwell > > Date: Thu Apr 19 17:59:38 2018 +0100 > > > >lan78xx: Read MAC address from DT if present > > > >There is a standard mechanism for locating and using a MAC address from > >the Device Tree. Use this facility in the lan78xx driver to support > >applications without programmed EEPROM or OTP. At the same time, > >regularise the handling of the different address sources. > > > >Signed-off-by: Phil Elwell > >Signed-off-by: David S. Miller > > Can you confirm it actually works on 4.4? It does not build on 4.4, so I'm dropping it :( greg k-h
Re: [PATCH 1/2] USB: serial: mos7840: Adjust port settings for read and write registers
On Thu, Nov 29, 2018 at 02:51:36PM +0800, JackyChou wrote: > > From: JackyChou > > In the read/write function, set port 2 independently in the 2-port case. > > When setting the offset of port registers, the offset between port 1 and > other ports is different, so port 1 is set independently. > Then in the rest of ports, the port 2 between 2-ports case and 4-ports case > is different, so port 2 in 2-ports case is set independently. > > Signed-off-by: JackyChou > --- Thanks for the update. Unfortunately the patches are still white-space damaged. You may want to take a look at: Documentation/process/email-clients.rst and verify if its your mail client or server that corrupts the patches. Either way, try sending them to yourself first and run checkpatch on the result. Also, remember to include a changelog when you resend (here below the cut-off, ---, line, and increase the patch revision (e.g. this is really v4?). Thanks, Johan
RE: [PATCH V6 01/10] usb: gadget: udc: Add timer for stream capable endpoints
Hi Felipe,
>-Original Message-
>From: Felipe Balbi [mailto:ba...@kernel.org]
>Sent: Thursday, November 29, 2018 6:22 PM
>To: Anurag Kumar Vulisha ; Greg Kroah-Hartman
>; Alan Stern ; Johan
>Hovold ; Jaejoong Kim ; Benjamin
>Herrenschmidt ; Roger Quadros
>Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org;
>v.anuragku...@gmail.com; Thinh Nguyen ; Tejas Joglekar
>; Ajay Yugalkishore Pandey
>Subject: RE: [PATCH V6 01/10] usb: gadget: udc: Add timer for stream capable
>endpoints
>
>
>Hi,
>
>Anurag Kumar Vulisha writes:
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index af88b48..41cc23b 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -52,6 +52,24 @@ static int udc_bind_to_driver(struct usb_udc *udc,
/*
-
*/
/**
+ * usb_ep_stream_timeout - callback function for endpoint stream timeout
>timer
+ * @arg: pointer to struct timer_list
+ *
+ * This function gets called only when bulk streams are enabled in the
endpoint
+ * and only after ep->stream_timeout_timer has expired. The timer gets
expired
+ * only when the gadget controller failed to find a valid stream event
for this
+ * endpoint. On timer expiry, this function calls the endpoint-specific
timeout
+ * handler registered to endpoint ops->stream_timeout API.
+ */
+static void usb_ep_stream_timeout(struct timer_list *arg)
+{
+ struct usb_ep *ep = from_timer(ep, arg, stream_timeout_timer);
+
+ if (ep->stream_capable && ep->ops->stream_timeout)
>>>
>>>why is the timer only for stream endpoints? What if we want to run this
>>>on non-stream capable eps?
>>>
>>
>> I have seen this issue only with stream capable eps between PRIME &
>> EPRDY. But this issue can potentially occur with non-stream endpoints
>> also. Will remove this stream capable checks in next series of patch.
>
>you're adding support for transfer timeouts, which some gadgets may want
>regardless of endpoint type. One use is to correct cases of streams
>going out of sync.
>
Thanks for making me understand, will implement your suggestions and resend
the patches soon.
Best Regards,
Anurag Kumar Vulisha
+ ep->ops->stream_timeout(ep);
>>>
>>>you don't ned an extra operation here, ep_dequeue should be enough.
>>>
>>
>> I think issuing ep_dequeue() would be more trickier than calling ep->ops-
>>stream_timeout().
>> This is because, every gadget driver has their own way of handling
>> ep_dequeue.
>Some
>> drivers (like dwc3) may sleep for an event (wait_event_lock_irq) in the
>> ep_dequeue
>routine.
>
>not anymore. There's, now, a requirement that ->dequeue can be called
>from any context.
>
>> Since we are calling ep_dequeue from timer timeout callback which is in
>> softirq
>context,
>> sleeping or waiting for an event would hang the system. Also adding ep->ops-
>>stream_timeout()
>> would make the gadget drivers handle the issue in better way based on their
>implementation.
>
>no problems
>
>> Another advantage is the drivers which doesn't have this timeout issue
>> doesn't have
>to register the
>> timeout handler and can avoid the logic of deleting the timer for every
>> request. So, I
>think
>> ep->ops->stream_timeout() would be better than having a generic handler which
>does
>> ep_dequeue() & ep_queue(). Please provide your suggestion on this
>implementation.
>
>call ep_dequeue()
>
>--
>balbi
RE: [RFC PATCH v2 07/15] usb:cdns3: Adds Device mode support - initialization.
> Roger Quadros writes:
> >> +static void cdns3_gadget_config(struct cdns3_device *priv_dev) {
> >> + struct cdns3_usb_regs __iomem *regs = priv_dev->regs;
> >> +
> >> + cdns3_ep0_config(priv_dev);
> >> +
> >> + /* enable interrupts for endpoint 0 (in and out) */
> >> + writel(EP_IEN_EP_OUT0 | EP_IEN_EP_IN0, ®s->ep_ien);
> >> +
> >> + /* enable generic interrupt*/
> >> + writel(USB_IEN_INIT, ®s->usb_ien);
> >> + writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, ®s->usb_conf);
> >> + writel(USB_CONF_DMULT, ®s->usb_conf);
> >> + writel(USB_CONF_DEVEN, ®s->usb_conf);
> >
> > If you are enabling interrupts in this patch you should handle them in the
> > ISR.
>
> Frankly, I don't understand why this is a series. It's a single driver and
> splitting it into
> a series just makes it more difficult to review, actually.
>
> Sure, a single patch will be large, but there's no way to have a functional
> driver until
> all patches are applied, anyway.
>
Yes, I agree with Felipe. Pawel, you could remove the "RFC" prefix, and send
the whole
one as one patch. I will test it at my hardware.
Peter
RE: [RFC PATCH v2 07/15] usb:cdns3: Adds Device mode support - initialization.
Hi,
>> Roger Quadros writes:
>> >> +static void cdns3_gadget_config(struct cdns3_device *priv_dev) {
>> >> + struct cdns3_usb_regs __iomem *regs = priv_dev->regs;
>> >> +
>> >> + cdns3_ep0_config(priv_dev);
>> >> +
>> >> + /* enable interrupts for endpoint 0 (in and out) */
>> >> + writel(EP_IEN_EP_OUT0 | EP_IEN_EP_IN0, ®s->ep_ien);
>> >> +
>> >> + /* enable generic interrupt*/
>> >> + writel(USB_IEN_INIT, ®s->usb_ien);
>> >> + writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, ®s->usb_conf);
>> >> + writel(USB_CONF_DMULT, ®s->usb_conf);
>> >> + writel(USB_CONF_DEVEN, ®s->usb_conf);
>> >
>> > If you are enabling interrupts in this patch you should handle them in the
>> > ISR.
>>
>> Frankly, I don't understand why this is a series. It's a single driver and
>> splitting it into
>> a series just makes it more difficult to review, actually.
>>
>> Sure, a single patch will be large, but there's no way to have a functional
>> driver until
>> all patches are applied, anyway.
>>
>
>Yes, I agree with Felipe. Pawel, you could remove the "RFC" prefix, and send
>the whole
>one as one patch. I will test it at my hardware.
Ok, I will prepare such single patch.
Peter I know that you have little different platform.
Probably you should made some additional changes for your platform.
I assume that core.c should be common file.
Probably we should add also some platform specific file.
Thanks.
Pawel.
[PATCH 1/2 v4] USB: serial: mos7840: Adjust port settings for read and write registers
From: JackyChou
In the read/write function, set port 2 independently in the 2-port case.
When setting the offset of port registers, the offset between port 1 and
other ports is different, so port 1 is set independently.
Then in the rest of ports, the port 2 between 2-ports case and 4-ports case
is different, so port 2 in 2-ports case is set independently.
Signed-off-by: JackyChou
---
drivers/usb/serial/mos7840.c | 48
1 file changed, 16 insertions(+), 32 deletions(-)
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 88828b4b8c44..92ab68ef08ab 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -298,15 +298,10 @@ static int mos7840_set_uart_reg(struct usb_serial_port
*port, __u16 reg,
val = val & 0x00ff;
/* For the UART control registers, the application number need
to be Or'ed */
- if (port->serial->num_ports == 4) {
+ if (port->serial->num_ports == 2 && port->port_number != 0)
+ val |= ((__u16)port->port_number + 2) << 8;
+ else
val |= ((__u16)port->port_number + 1) << 8;
- } else {
- if (port->port_number == 0) {
- val |= ((__u16)port->port_number + 1) << 8;
- } else {
- val |= ((__u16)port->port_number + 2) << 8;
- }
- }
dev_dbg(&port->dev, "%s application number is %x\n", __func__, val);
return usb_control_msg(dev, usb_sndctrlpipe(dev, 0), MCS_WRREQ,
MCS_WR_RTYPE, val, reg, NULL, 0,
@@ -332,15 +327,10 @@ static int mos7840_get_uart_reg(struct usb_serial_port
*port, __u16 reg,
return -ENOMEM;
/* Wval is same as application number */
- if (port->serial->num_ports == 4) {
+ if (port->serial->num_ports == 2 && port->port_number != 0)
+ Wval = ((__u16)port->port_number + 2) << 8;
+ else
Wval = ((__u16)port->port_number + 1) << 8;
- } else {
- if (port->port_number == 0) {
- Wval = ((__u16)port->port_number + 1) << 8;
- } else {
- Wval = ((__u16)port->port_number + 2) << 8;
- }
- }
dev_dbg(&port->dev, "%s application number is %x\n", __func__, Wval);
ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), MCS_RDREQ,
MCS_RD_RTYPE, Wval, reg, buf, VENDOR_READ_LENGTH,
@@ -2132,22 +2122,16 @@ static int mos7840_port_probe(struct usb_serial_port
*port)
mos7840_port->SpRegOffset = 0x0;
mos7840_port->ControlRegOffset = 0x1;
mos7840_port->DcrRegOffset = 0x4;
- } else if ((mos7840_port->port_num == 2) && (serial->num_ports == 4)) {
- mos7840_port->SpRegOffset = 0x8;
- mos7840_port->ControlRegOffset = 0x9;
- mos7840_port->DcrRegOffset = 0x16;
- } else if ((mos7840_port->port_num == 2) && (serial->num_ports == 2)) {
- mos7840_port->SpRegOffset = 0xa;
- mos7840_port->ControlRegOffset = 0xb;
- mos7840_port->DcrRegOffset = 0x19;
- } else if ((mos7840_port->port_num == 3) && (serial->num_ports == 4)) {
- mos7840_port->SpRegOffset = 0xa;
- mos7840_port->ControlRegOffset = 0xb;
- mos7840_port->DcrRegOffset = 0x19;
- } else if ((mos7840_port->port_num == 4) && (serial->num_ports == 4)) {
- mos7840_port->SpRegOffset = 0xc;
- mos7840_port->ControlRegOffset = 0xd;
- mos7840_port->DcrRegOffset = 0x1c;
+ } else {
+ u8 port_offset;
+
+ if ((mos7840_port->port_num == 2) && (serial->num_ports == 2))
+ port_offset = 1;
+ else
+ port_offset = mos7840_port->port_num - 2;
+ mos7840_port->SpRegOffset = 0x8 + (2 * port_offset);
+ mos7840_port->ControlRegOffset = 0x9 + (2 * port_offset);
+ mos7840_port->DcrRegOffset = 0x16 + (3 * port_offset);
}
mos7840_dump_serial_port(port, mos7840_port);
mos7840_set_port_private(port, mos7840_port);
--
2.17.1
[PATCH 2/2 v4] USB: serial: mos7840: Add a product ID for the new product
From: JackyChou
Add a new PID 0x7843 to the driver.
Let the new products be able to set up 3 serial ports with the driver.
Signed-off-by: JackyChou
---
drivers/usb/serial/mos7840.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 92ab68ef08ab..b6e7b55fcb7c 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -94,6 +94,7 @@
/* The native mos7840/7820 component */
#define USB_VENDOR_ID_MOSCHIP 0x9710
#define MOSCHIP_DEVICE_ID_7840 0x7840
+#define MOSCHIP_DEVICE_ID_7843 0x7843
#define MOSCHIP_DEVICE_ID_7820 0x7820
#define MOSCHIP_DEVICE_ID_7810 0x7810
/* The native component can have its vendor/device id's overridden
@@ -176,6 +177,7 @@ enum mos7840_flag {
static const struct usb_device_id id_table[] = {
{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7840)},
+ {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7843)},
{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7820)},
{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7810)},
{USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_2)},
@@ -2033,7 +2035,8 @@ static int mos7840_probe(struct usb_serial *serial,
int device_type;
if (product == MOSCHIP_DEVICE_ID_7810 ||
- product == MOSCHIP_DEVICE_ID_7820) {
+ product == MOSCHIP_DEVICE_ID_7820 ||
+ product == MOSCHIP_DEVICE_ID_7843) {
device_type = product;
goto out;
}
@@ -2067,7 +2070,10 @@ static int mos7840_calc_num_ports(struct usb_serial
*serial,
int device_type = (unsigned long)usb_get_serial_data(serial);
int num_ports;
- num_ports = (device_type >> 4) & 0x000F;
+ if (device_type == MOSCHIP_DEVICE_ID_7843)
+ num_ports = 3;
+ else
+ num_ports = (device_type >> 4) & 0x000F;
/*
* num_ports is currently never zero as device_type is one of
--
2.17.1
RE: [RFC PATCH v2 02/15] usb:cdns3: Device side header file.
> +
> +/*
> + * USBSS-DEV register interface.
> + * This corresponds to the USBSS Device Controller Interface */
> +/**
> + * struct xhci_cap_regs - xHCI Host Controller Capability Registers.
struct cdns3_usb_regs - device controller registers
> +struct cdns3_device;
> +
> +struct cdns3_endpoint {
> + struct usb_ep endpoint;
> + struct list_headrequest_list;
> + struct list_headep_match_pending_list;
> +
> + struct cdns3_trb*trb_pool;
> + dma_addr_t trb_pool_dma;
> +
> + struct cdns3_device *cdns3_dev;
> + charname[20];
> +
> +#define EP_ENABLED BIT(0)
> +#define EP_STALL BIT(1)
> +#define EP_WEDGE BIT(2)
> +#define EP_TRANSFER_STARTED BIT(3)
> +#define EP_UPDATE_EP_TRBADDR BIT(4)
> +#define EP_PENDING_REQUEST BIT(5)
> +#define EP_USED BIT(5)
> + u32 flags;
> +
> + void*aligned_buff;
> + dma_addr_t aligned_dma_addr;
> + u8 dir;
> + u8 num;
> + u8 type;
> +
> + int free_trbs;
> + u8 pcs;
> + u8 ccs;
> + int enqueue;
> + int dequeue;
> +};
> +
Would you please add kernel doc for above structure?
> +struct cdns3_request {
> + struct usb_request request;
> + struct cdns3_endpoint *priv_ep;
> + struct cdns3_trb *trb;
> + int start_trb;
> + int end_trb;
> + int on_ring:1;
> +};
> +
> +#define to_cdns3_request(r) (container_of(r, struct cdns3_request,
> +request))
> +
> +struct cdns3_device {
> + struct device dev;
> + struct cdns3_usb_regs __iomem *regs;
> +
> + struct usb_gadget gadget;
> + struct usb_gadget_driver*gadget_driver;
> +
> + struct usb_ctrlrequest *setup;
> + dma_addr_t setup_dma;
> + dma_addr_t trb_ep0_dma;
> + struct cdns3_trb*trb_ep0;
> + void*zlp_buf;
> +
> + struct cdns3_endpoint *eps[USB_SS_ENDPOINTS_MAX_COUNT];
> + int ep_nums;
> + /*
> + * field used for improving performance. It holds the last
> + * selected endpoint number
> + */
> + u32 selected_ep;
> + struct usb_request *ep0_request;
> + int ep0_data_dir;
> + int hw_configured_flag;
> + int wake_up_flag;
> + u16 isoch_delay;
> + /* generic spin-lock for drivers */
> + spinlock_t lock;
> +
> + unsignedis_connected:1;
> + unsignedin_standby_mode:1;
> + unsignedstatus_completion_no_call:1;
> + unsignedu1_allowed:1;
> + unsignedu2_allowed:1;
> +
> + u32 usb_ien;
> + u32 ep_ien;
> + int setup_pending;
> + struct device *sysdev;
> + /* The device mode is enabled */
> + int start_gadget;
> + struct list_headep_match_list;
> + /* KB */
> + int onchip_mem_allocated_size;
> + /* Memory is allocated for OUT */
> + int out_mem_is_allocated:1;
> + struct work_struct pending_status_wq;
> + struct usb_request *pending_status_request;
> +};
> +
kernel-doc please
Peter
Re: [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code.
On Sun, Nov 18, 2018 at 6:16 PM Pawel Laszczak wrote:
>
> Patch adds core.c and core.h file that implements initialization
> of platform driver and adds function responsible for selecting,
> switching and running appropriate Device/Host mode.
>
> Signed-off-by: Pawel Laszczak
> ---
> drivers/usb/cdns3/Makefile | 2 +
> drivers/usb/cdns3/core.c | 413 +
> drivers/usb/cdns3/core.h | 100 +
> 3 files changed, 515 insertions(+)
> create mode 100644 drivers/usb/cdns3/core.c
> create mode 100644 drivers/usb/cdns3/core.h
>
> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
> index dcdd62003c6a..02d25b23c5d3 100644
> --- a/drivers/usb/cdns3/Makefile
> +++ b/drivers/usb/cdns3/Makefile
> @@ -1,3 +1,5 @@
> +obj-$(CONFIG_USB_CDNS3)+= cdns3.o
> obj-$(CONFIG_USB_CDNS3_PCI_WRAP) += cdns3-pci.o
>
> +cdns3-y:= core.o
> cdns3-pci-y:= cdns3-pci-wrap.o
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> new file mode 100644
> index ..f9055d4da67f
> --- /dev/null
> +++ b/drivers/usb/cdns3/core.c
> @@ -0,0 +1,413 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Cadence USBSS DRD Driver.
> + *
> + * Copyright (C) 2018 Cadence.
> + *
Please add NXP copyright too.
> + * Author: Peter Chen
> + * Pawel Laszczak
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "gadget.h"
> +#include "core.h"
> +
> +static inline struct cdns3_role_driver *cdns3_get_current_role_driver(struct
> cdns3 *cdns)
> +{
> + WARN_ON(cdns->role >= CDNS3_ROLE_END || !cdns->roles[cdns->role]);
> + return cdns->roles[cdns->role];
> +}
> +
Can we delete "current", and use cdns3_get_role_driver directly?
> +static inline int cdns3_role_start(struct cdns3 *cdns, enum cdns3_roles role)
> +{
> + int ret;
> +
> + if (role >= CDNS3_ROLE_END)
> + return 0;
> +
> + if (!cdns->roles[role])
> + return -ENXIO;
> +
> + mutex_lock(&cdns->mutex);
> + cdns->role = role;
> + ret = cdns->roles[role]->start(cdns);
> + mutex_unlock(&cdns->mutex);
> + return ret;
> +}
> +
> +static inline void cdns3_role_stop(struct cdns3 *cdns)
> +{
> + enum cdns3_roles role = cdns->role;
> +
> + if (role == CDNS3_ROLE_END)
> + return;
> +
> + mutex_lock(&cdns->mutex);
> + cdns->roles[role]->stop(cdns);
> + cdns->role = CDNS3_ROLE_END;
> + mutex_unlock(&cdns->mutex);
> +}
> +
> +static enum cdns3_roles cdns3_get_role(struct cdns3 *cdns)
> +{
> + if (cdns->roles[CDNS3_ROLE_HOST] && cdns->roles[CDNS3_ROLE_GADGET]) {
> + //TODO: implements selecting device/host mode
> + return CDNS3_ROLE_HOST;
> + }
> + return cdns->roles[CDNS3_ROLE_HOST]
> + ? CDNS3_ROLE_HOST
> + : CDNS3_ROLE_GADGET;
> +}
> +
> +/**
> + * cdns3_core_init_role - initialize role of operation
> + * @cdns: Pointer to cdns3 structure
> + *
> + * Returns 0 on success otherwise negative errno
> + */
> +static int cdns3_core_init_role(struct cdns3 *cdns)
> +{
> + struct device *dev = cdns->dev;
> + enum usb_dr_mode dr_mode;
> +
> + dr_mode = usb_get_dr_mode(dev);
> + cdns->role = CDNS3_ROLE_END;
> +
> + /*
> +* If driver can't read mode by means of usb_get_dr_mdoe function then
> +* chooses mode according with Kernel configuration. This setting
> +* can be restricted later depending on strap pin configuration.
> +*/
> + if (dr_mode == USB_DR_MODE_UNKNOWN) {
> + if (IS_ENABLED(CONFIG_USB_CDNS3_HOST) &&
> + IS_ENABLED(CONFIG_USB_CDNS3_GADGET))
> + dr_mode = USB_DR_MODE_OTG;
> + else if (IS_ENABLED(CONFIG_USB_CDNS3_HOST))
> + dr_mode = USB_DR_MODE_HOST;
> + else if (IS_ENABLED(CONFIG_USB_CDNS3_GADGET))
> + dr_mode = USB_DR_MODE_PERIPHERAL;
> + }
> +
> + if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
> + //TODO: implements host initialization
> + }
> +
> + if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
> + //TODO: implements device initialization
> + }
> +
> + if (!cdns->roles[CDNS3_ROLE_HOST] && !cdns->roles[CDNS3_ROLE_GADGET])
> {
> + dev_err(dev, "no supported roles\n");
> + return -ENODEV;
> + }
> +
> + cdns->dr_mode = dr_mode;
> + return 0;
> +}
> +
> +/**
> + * cdns3_irq - interrupt handler for cdns3 core device
> + *
> + * @irq: irq number for cdns3 core device
> + * @data: structure of cdns3
> + *
> + * Returns IRQ_HANDLED or IRQ_NONE
> + */
> +static irqreturn_t cdns3_irq(int irq, void *data)
> +{
> + struct cd
