Re: USB client crash on Vybrid with USB gadget RNDIS connection
On Mon, Sep 14, 2015 at 12:01:04PM +0530, maitysancha...@gmail.com wrote: > On 15-09-14 13:11:16, Peter Chen wrote: > > On Fri, Sep 11, 2015 at 04:51:22PM +0530, maitysancha...@gmail.com wrote: > > > On 15-09-11 15:56:17, maitysancha...@gmail.com wrote: > > > > Hello Peter, > > > > > > > > On 15-09-11 16:58:52, Peter Chen wrote: > > > > > On Fri, Sep 11, 2015 at 02:36:58PM +0530, maitysancha...@gmail.com > > > > > wrote: > > > > > > Hello, > > > > > > > > > > > > We are using the 4.1.5 kernel on Freescale Vybrid SoC which has a > > > > > > Chipidea > > > > > > IP. One of our customer's reported a kernel crash while using USB > > > > > > client > > > > > > with the USB gadget RNDIS functionality while being connected to a > > > > > > host > > > > > > running Windows 7 SP1 Pro and I was also able to reproduce the > > > > > > issue here. > > > > > > > > > > > > The issue seems reproducible and occurs while doing bidirectional > > > > > > communication > > > > > > over socket after an hour or so. Strangely it did not happen while > > > > > > doing one > > > > > > way transfers from the Vybrid to PC side which I tested by running > > > > > > for almost > > > > > > 16 hours. For testing birectional communication I had a simple > > > > > > Python echo server > > > > > > running on PC and client on Vybrid side while for one way test I > > > > > > had Python > > > > > > client on Vybrid and Hercules application on Windows side. > > > > > > > > > > > > Both the Python client and server do a continous send/recv in a > > > > > > while loop. > > > > > > > > > > > > I could not reproduce it while doing bidirectional iperf tests for > > > > > > 5-6 hours > > > > > > with a Linux machine. > > > > > > > > > > > > The same issue is also seen with 4.0.5. Is this a known issue or > > > > > > reported > > > > > > earlier? > > > > > > > > > > > > The stack trace is below on 4.1.5 kernel. > > > > > > > > > > > > [69253.557550] Unable to handle kernel NULL pointer dereference at > > > > > > virtual address > > > > > > [69253.565681] pgd = 80004000 > > > > > > [69253.568396] [] *pgd= > > > > > > [69253.572004] Internal error: Oops: 817 [#1] ARM > > > > > > [69253.576457] Modules linked in: mcp251x can_dev > > > > > > [69253.580963] CPU: 0 PID: 0 Comm: swapper Not tainted > > > > > > 4.1.4-v2.5b1+gdc92514 #1 > > > > > > [69253.588016] Hardware name: Freescale Vybrid VF5xx/VF6xx (Device > > > > > > Tree) > > > > > > [69253.594469] task: 807d04b0 ti: 807ca000 task.ti: 807ca000 > > > > > > [69253.599896] PC is at add_td_to_list+0x118/0x1a0 > > > > > > [69253.604441] LR is at add_td_to_list+0x58/0x1a0 > > > > > > [69253.608895] pc : [<803b9fd4>]lr : [<803b9f14>]psr: > > > > > > 30010193 > > > > > > [69253.608895] sp : 807cbcf0 ip : 0006 fp : 807cbd14 > > > > > > [69253.620379] r10: 0008 r9 : 4000 r8 : 8da82db4 > > > > > > [69253.625614] r7 : 8e02f6e8 r6 : 0008 r5 : 8da82d80 r4 : > > > > > > 8da321c0 > > > > > > [69253.632148] r3 : r2 : 8e403580 r1 : 8da82dbc r0 : > > > > > > > > > > > > [69253.638687] Flags: nzCV IRQs off FIQs on Mode SVC_32 ISA ARM > > > > > > Segment kernel > > > > > > [69253.646087] Control: 10c5387d Table: 8cb50059 DAC: 0015 > > > > > > [69253.651839] Process swapper (pid: 0, stack limit = 0x807ca208) > > > > > > [69253.657681] Stack: (0x807cbcf0 to 0x807cc000) > > > > > > [69253.662053] bce0: 8da82d80 > > > > > > 8e02f6e8 0008 8e02f010 > > > > > > [69253.670248] bd00: 8da82db4 4000 807cbd54 807cbd18 803ba9dc > > > > > > 803b9ec8 > > > > > > [69253.678439] bd20: a0010193 8d94d5f4 > > > > > > 8e23f3d4 0024 > > > > > > [69253.686633] bd40: 8e02f408 8db1773c 807cbd6c 807cbd58 803bad8c > > > > > > 803ba8a4 8d94d540 8d94d5f4 > > > > > > [69253.694825] bd60: 807cbd84 807cbd70 803ca360 803bad68 8081f3f8 > > > > > > 8d8ce000 807cbda4 807cbd88 > > > > > > [69253.703017] bd80: 803cbd78 803ca310 8db17700 8db1773c 0024 > > > > > > 8db17700 807cbdbc 807cbda8 > > > > > > [69253.711209] bda0: 803ca624 803cbaec 0024 8db1773c 807cbdcc > > > > > > 807cbdc0 803c30e4 803ca610 > > > > > > [69253.719402] bdc0: 807cbe3c 807cbdd0 803bb264 803c30dc 807cbdec > > > > > > 80815d74 803ba500 8e02f408 > > > > > > [69253.727594] bde0: 807cc044 0001 8e02f440 8e02f630 8e02f010 > > > > > > 8e02f010 8db17734 > > > > > > [69253.735787] be00: 8e02f40c 8e02f460 0021 0024 807cbe5c > > > > > > 8e02f010 807dd644 > > > > > > [69253.743979] be20: 0027 8e0b8480 807fdc3d 807cbe54 > > > > > > 807cbe40 803b81ac 803bada8 > > > > > > [69253.752172] be40: 8e2c2580 807dd644 807cbe8c 807cbe58 8004ced0 > > > > > > 803b8160 800644b8 80040464 > > > > > > [69253.760363] be60: 3efc 8e0b8480 807dd644 0001 > > > > > > 8e006000 0001 807f2430 > > > > > > [69253.768556] be80: 807
Re: MIDI gadget allocating too much from coherent pool
I wrote: > Felipe Tonello wrote: >> DSP (read sensors and read/send MIDI) <- UART -> SOC (imx6) <- USB >> MIDI GADGET -> HOST >> >> When the throughput from the DSP is high, thus causing the throughput >> on the USB to be high as well, I get a Kernel Panic saying to increase >> the coherent_pool. I've used some crazy sizes like 1M, 4M and even 8M. >> Obviously when I increase, it takes longer to crash but it still >> crashes. > > This sounds like a memory leak in your USB host controller's driver > (whatever it is). > > The USB MIDI driver uses URB_NO_TRANSFER_DMA_MAP with bulk transfers, Sorry, wrong driver. There's no URB_NO_TRANSFER_DMA_MAP in the gadget API. The f_midi driver does not use pre-mapped buffers, and the URB buffers would use streaming DMA mappings anyway. The coherent pool is used by host controller drivers for their internal structures, such as DMA descriptors. The "fsl_usb2_udc" driver (if that is what you're using) uses a coherent DMA pool for "TD management". I see no obvious problem with how it calls dma_pool_alloc()/dma_pool_free(). Are there any messages in the system log? You might want to modify the kernel to check how often it calls these functions. Regards, Clemens -- 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: USB client crash on Vybrid with USB gadget RNDIS connection
On 15-09-14 13:50:22, Peter Chen wrote: > On Mon, Sep 14, 2015 at 12:01:04PM +0530, maitysancha...@gmail.com wrote: > > On 15-09-14 13:11:16, Peter Chen wrote: > > > On Fri, Sep 11, 2015 at 04:51:22PM +0530, maitysancha...@gmail.com wrote: > > > > On 15-09-11 15:56:17, maitysancha...@gmail.com wrote: > > > > > Hello Peter, > > > > > > > > > > On 15-09-11 16:58:52, Peter Chen wrote: > > > > > > On Fri, Sep 11, 2015 at 02:36:58PM +0530, maitysancha...@gmail.com > > > > > > wrote: > > > > > > > Hello, > > > > > > > > > > > > > > We are using the 4.1.5 kernel on Freescale Vybrid SoC which has a > > > > > > > Chipidea > > > > > > > IP. One of our customer's reported a kernel crash while using USB > > > > > > > client > > > > > > > with the USB gadget RNDIS functionality while being connected to > > > > > > > a host > > > > > > > running Windows 7 SP1 Pro and I was also able to reproduce the > > > > > > > issue here. > > > > > > > > > > > > > > The issue seems reproducible and occurs while doing bidirectional > > > > > > > communication > > > > > > > over socket after an hour or so. Strangely it did not happen > > > > > > > while doing one > > > > > > > way transfers from the Vybrid to PC side which I tested by > > > > > > > running for almost > > > > > > > 16 hours. For testing birectional communication I had a simple > > > > > > > Python echo server > > > > > > > running on PC and client on Vybrid side while for one way test I > > > > > > > had Python > > > > > > > client on Vybrid and Hercules application on Windows side. > > > > > > > > > > > > > > Both the Python client and server do a continous send/recv in a > > > > > > > while loop. > > > > > > > > > > > > > > I could not reproduce it while doing bidirectional iperf tests > > > > > > > for 5-6 hours > > > > > > > with a Linux machine. > > > > > > > > > > > > > > The same issue is also seen with 4.0.5. Is this a known issue or > > > > > > > reported > > > > > > > earlier? > > > > > > > > > > > > > > The stack trace is below on 4.1.5 kernel. > > > > > > > > > > > > > > [69253.557550] Unable to handle kernel NULL pointer dereference > > > > > > > at virtual address > > > > > > > [69253.565681] pgd = 80004000 > > > > > > > [69253.568396] [] *pgd= > > > > > > > [69253.572004] Internal error: Oops: 817 [#1] ARM > > > > > > > [69253.576457] Modules linked in: mcp251x can_dev > > > > > > > [69253.580963] CPU: 0 PID: 0 Comm: swapper Not tainted > > > > > > > 4.1.4-v2.5b1+gdc92514 #1 > > > > > > > [69253.588016] Hardware name: Freescale Vybrid VF5xx/VF6xx > > > > > > > (Device Tree) > > > > > > > [69253.594469] task: 807d04b0 ti: 807ca000 task.ti: 807ca000 > > > > > > > [69253.599896] PC is at add_td_to_list+0x118/0x1a0 > > > > > > > [69253.604441] LR is at add_td_to_list+0x58/0x1a0 > > > > > > > [69253.608895] pc : [<803b9fd4>]lr : [<803b9f14>]psr: > > > > > > > 30010193 > > > > > > > [69253.608895] sp : 807cbcf0 ip : 0006 fp : 807cbd14 > > > > > > > [69253.620379] r10: 0008 r9 : 4000 r8 : 8da82db4 > > > > > > > [69253.625614] r7 : 8e02f6e8 r6 : 0008 r5 : 8da82d80 r4 : > > > > > > > 8da321c0 > > > > > > > [69253.632148] r3 : r2 : 8e403580 r1 : 8da82dbc r0 : > > > > > > > > > > > > > > [69253.638687] Flags: nzCV IRQs off FIQs on Mode SVC_32 ISA > > > > > > > ARM Segment kernel > > > > > > > [69253.646087] Control: 10c5387d Table: 8cb50059 DAC: 0015 > > > > > > > [69253.651839] Process swapper (pid: 0, stack limit = 0x807ca208) > > > > > > > [69253.657681] Stack: (0x807cbcf0 to 0x807cc000) > > > > > > > [69253.662053] bce0: 8da82d80 > > > > > > > 8e02f6e8 0008 8e02f010 > > > > > > > [69253.670248] bd00: 8da82db4 4000 807cbd54 807cbd18 803ba9dc > > > > > > > 803b9ec8 > > > > > > > [69253.678439] bd20: a0010193 8d94d5f4 > > > > > > > 8e23f3d4 0024 > > > > > > > [69253.686633] bd40: 8e02f408 8db1773c 807cbd6c 807cbd58 803bad8c > > > > > > > 803ba8a4 8d94d540 8d94d5f4 > > > > > > > [69253.694825] bd60: 807cbd84 807cbd70 803ca360 803bad68 8081f3f8 > > > > > > > 8d8ce000 807cbda4 807cbd88 > > > > > > > [69253.703017] bd80: 803cbd78 803ca310 8db17700 8db1773c 0024 > > > > > > > 8db17700 807cbdbc 807cbda8 > > > > > > > [69253.711209] bda0: 803ca624 803cbaec 0024 8db1773c 807cbdcc > > > > > > > 807cbdc0 803c30e4 803ca610 > > > > > > > [69253.719402] bdc0: 807cbe3c 807cbdd0 803bb264 803c30dc 807cbdec > > > > > > > 80815d74 803ba500 8e02f408 > > > > > > > [69253.727594] bde0: 807cc044 0001 8e02f440 8e02f630 8e02f010 > > > > > > > 8e02f010 8db17734 > > > > > > > [69253.735787] be00: 8e02f40c 8e02f460 0021 0024 807cbe5c > > > > > > > 8e02f010 807dd644 > > > > > > > [69253.743979] be20: 0027 8e0b8480 807fdc3d 807cbe54 > > > > > > > 807cbe40 803b81ac 803bada8 > > > > > > > [69253.752172] be40: 8e2c2580 80
Re: USB bug
Andrew Gillis wrote: > I get an error when trying to play music through USB DACs under Linux. > This only happens with a few high end DACs and only when using a NEC > uPD72020x chipset USB 3.0 port. > > The people at RedHat think it may be the NEC uPD72020x chipset is > reporting it's capabilities incorrectly. > > https://bugzilla.redhat.com/show_bug.cgi?id=1240798 USB streams are not related to audio (isochronous) transfers. All those paste.fedoraproject.org links are broken. Regards, Clemens -- 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: qcserial: AT unsolicited response codes missing with Dell Wireless 5808e
On Sat, Sep 12, 2015 at 04:28:33PM +0200, Bjørn Mork wrote:
> David Ward writes:
>
> > An earlier e-mail from Reinhard contained a patch that did this for the
> > MC7304. That patch could be modified as shown below so that the control
> > request is enabled or disabled from qcprobe() instead. This way, it is
> > straightforward to enable it for specific interface(s) on a particular
> > device layout (here, it is only enabled on the AT port in the Sierra
> > Wireless layout). I tested this with the Dell Wireless 5808e and it was
> > able to connect to the mobile network as usual and also send AT URCs.
>
> I think this is a good idea.
>
> But I believe it would be nicer to consolidate the send_setup handling
> for all usb-wwan based drivers instead of making multiple copies of it.
> The code here seems to be modelled after the current option
> implementation. But there isn't really much driver-specific stuff in
> that at all. The only thing is the option_private/qcserial_private
> thingy, which I don't understand the need for...
>
> Johan, apologies for not commenting on this when you added it, but I
> fail to understand why you added the option_private struct in commit
> e463c6dda8f5 ("USB: option: handle send_setup blacklisting at probe")
> Care to explain?
>
> Moving the blacklisting logic from send_setup to probe made sense, but
> the ifNum change seems completely unrelated (and unnecessary?):
You're right, it's not necessary. I think I may have wanted to show how
to pass usb-wwan-subdriver private data to send_setup as it was
submitted together with some patches cleaning that up.
> If we go back to using
> serial->interface->cur_altsetting->desc.bInterfaceNumber for the control
> message instead of priv->bInterfaceNumber, then we can completely drop
> 'struct option_private' and make option_send_setup() into a generic
> usb_wwan_send_setup() for use in both option and qcserial.
Sounds like a good idea. Looks like you can just replace the callback
pointer with a send_setup flag in usb_wwan_intf_private too.
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug in Linux USB stack
On 12.09.2015 00:56, Andrew Gillis wrote: I get an error when trying to play music through USB DACs under Linux. This only happens with a few high end DACs and only when using a NEC uPD72020x chipset USB 3.0 port. The people at RedHat think it may be the NEC uPD72020x chipset is reporting it's capabilities incorrectly. I'm not sure why some only some DACs are effected. it may depend on their chipsets. There is a detailed description of the bug with traces here https://bugzilla.redhat.com/show_bug.cgi?id=1240798 Any idea what I can do to fix this? Can you re-send the logs. Both dmesg with xhci debugging enabled and lsusb -v. Clicking the links to the logs in redhat bugzilla justs says "The paste you are looking for does not exist" The command completion code 0x11 (17 dec) is a parameter error, indicating that xhci thinks that some value we used in the input context is wrong. -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/1] USB:option:add ZTE PIDs
On Mon, Aug 24, 2015 at 08:36:12AM -0700, Liu.Zhao wrote: > This is intended to add ZTE device PIDs on kernel. > > Signed-off-by: Liu.Zhao Now applied, thanks. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: option: Sierra Wireless MC73xx -> Sierra Wireless MC7304/MC7354
On Mon, Aug 31, 2015 at 02:16:10PM -0400, David Ward wrote:
> Other Sierra Wireless MC73xx devices exist, with different USB IDs.
You should mention that you're just fixing a comment somewhere. And the
patch summary (Subject) should mention that too rather than rely on
"->".
> Cc: Bjørn Mork
> Signed-off-by: David Ward
> ---
> drivers/usb/serial/option.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index 876423b..cc3e1c7 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -1098,7 +1098,7 @@ static const struct usb_device_id option_ids[] = {
> { USB_DEVICE(QUALCOMM_VENDOR_ID, 0x0023)}, /* ONYX 3G device */
> { USB_DEVICE(QUALCOMM_VENDOR_ID, 0x9000)}, /* SIMCom SIM5218 */
> { USB_DEVICE_INTERFACE_CLASS(SIERRA_VENDOR_ID, 0x68c0, 0xff),
> - .driver_info = (kernel_ulong_t)&sierra_mc73xx_blacklist }, /* MC73xx
> */
> + .driver_info = (kernel_ulong_t)&sierra_mc73xx_blacklist }, /*
> MC7304/MC7354 */
> { USB_DEVICE_INTERFACE_CLASS(SIERRA_VENDOR_ID, 0x9041, 0xff),
> .driver_info = (kernel_ulong_t)&sierra_mc73xx_blacklist }, /*
> MC7305/MC7355 */
> { USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_6001) },
This may cause some pain for people backporting PIDs due to the changed
context, but I guess that's ok.
Care to resend with an updated description?
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: USB client crash on Vybrid with USB gadget RNDIS connection
> > > > root@imx6sxsabresd:~# opkg > > opkg-buildopkg-compare-versions.sh opkg-make-index > > opkg-buildpackage opkg-diff opkg-show-deps > > opkg-compare-indexes opkg-extract-file opkg-unbuild > > opkg-compare-versions opkg-list-fields opkg.py > > What are you running for your root filesystem/user space? Yocto build image or > some other distribution? > yocto Peter
Re: USB client crash on Vybrid with USB gadget RNDIS connection
On 15-09-14 08:05:26, Peter Chen wrote: > > > > > > > root@imx6sxsabresd:~# opkg > > > opkg-buildopkg-compare-versions.sh opkg-make-index > > > opkg-buildpackage opkg-diff opkg-show-deps > > > opkg-compare-indexes opkg-extract-file opkg-unbuild > > > opkg-compare-versions opkg-list-fields opkg.py > > > > What are you running for your root filesystem/user space? Yocto build image > > or > > some other distribution? Ok. I am running the tests with the CONFIG_DEBUG_LIST enabled in kernel. Will get back to you with the results. - Sanchayan. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: chipidea: reuse the platform_data to store the ci info
On Tue, Aug 11, 2015 at 09:43:13AM +, Barry Song wrote:
> From: Rong Wang
>
> Chipidea puts ci information to drvdata, but this overwrites the drvdata
> placed by EHCI core. EHCI core thinks drvdata is ehci_hcd. We can find this
> from codes like ehci-sysfs.c:
> static ssize_t show_companion(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> struct ehci_hcd *ehci;
>
> ehci = hcd_to_ehci(bus_to_hcd(dev_get_drvdata(dev)));
> ...
> }
>
> So overwritting drvdata from chipidea driver actually breaks a part of
> functionalities of EHCI core.
>
> Since the platform_data would not be accessed after the device is added
> to system after the probe process, so it's safe to move to platform_data
> here. This fix is not elegant but currently it is the quickest fix.
Hi Rong,
Since we have no better solution at current stage, I accept this
solution, rebase the latest usb-next tree and make more changes please,
then send again, thanks.
Peter
>
> Signed-off-by: Rong Wang
> Signed-off-by: Barry Song
> ---
> drivers/usb/chipidea/core.c| 4 ++--
> drivers/usb/chipidea/host.c| 1 -
> drivers/usb/chipidea/otg_fsm.c | 14 +++---
> 3 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 3ad48e1..94e8d15 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -811,7 +811,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> }
> }
>
> - platform_set_drvdata(pdev, ci);
> + dev->platform_data = ci;
> ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED,
> ci->platdata->name, ci);
> if (ret)
> @@ -844,7 +844,7 @@ deinit_phy:
>
> static int ci_hdrc_remove(struct platform_device *pdev)
> {
> - struct ci_hdrc *ci = platform_get_drvdata(pdev);
> + struct ci_hdrc *ci = dev_get_platdata(&pdev->dev);
>
> if (ci->supports_runtime_pm) {
> pm_runtime_get_sync(&pdev->dev);
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index 7161439..57d0a8b 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -102,7 +102,6 @@ static int host_start(struct ci_hdrc *ci)
> if (!hcd)
> return -ENOMEM;
>
> - dev_set_drvdata(ci->dev, ci);
> hcd->rsrc_start = ci->hw_bank.phys;
> hcd->rsrc_len = ci->hw_bank.size;
> hcd->regs = ci->hw_bank.abs;
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index 19d655a..3dc146d 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -36,7 +36,7 @@ get_a_bus_req(struct device *dev, struct device_attribute
> *attr, char *buf)
> {
> char*next;
> unsignedsize, t;
> - struct ci_hdrc *ci = dev_get_drvdata(dev);
> + struct ci_hdrc *ci = dev_get_platdata(dev);
>
> next = buf;
> size = PAGE_SIZE;
> @@ -51,7 +51,7 @@ static ssize_t
> set_a_bus_req(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - struct ci_hdrc *ci = dev_get_drvdata(dev);
> + struct ci_hdrc *ci = dev_get_platdata(dev);
>
> if (count > 2)
> return -1;
> @@ -80,7 +80,7 @@ get_a_bus_drop(struct device *dev, struct device_attribute
> *attr, char *buf)
> {
> char*next;
> unsignedsize, t;
> - struct ci_hdrc *ci = dev_get_drvdata(dev);
> + struct ci_hdrc *ci = dev_get_platdata(dev);
>
> next = buf;
> size = PAGE_SIZE;
> @@ -95,7 +95,7 @@ static ssize_t
> set_a_bus_drop(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - struct ci_hdrc *ci = dev_get_drvdata(dev);
> + struct ci_hdrc *ci = dev_get_platdata(dev);
>
> if (count > 2)
> return -1;
> @@ -121,7 +121,7 @@ get_b_bus_req(struct device *dev, struct device_attribute
> *attr, char *buf)
> {
> char*next;
> unsignedsize, t;
> - struct ci_hdrc *ci = dev_get_drvdata(dev);
> + struct ci_hdrc *ci = dev_get_platdata(dev);
>
> next = buf;
> size = PAGE_SIZE;
> @@ -136,7 +136,7 @@ static ssize_t
> set_b_bus_req(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - struct ci_hdrc *ci = dev_get_drvdata(dev);
> + struct ci_hdrc *ci = dev_get_platdata(dev);
>
> if (count > 2)
> return -1;
> @@ -158,7 +158,7 @@ static ssize_t
> set_a_clr_err(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - struct ci_hdrc *ci = dev_get_drvdata(dev);
> +
Re: [PATCH] usb: gadget: dummy_hcd: emulate sending zlp in packet logic
On 09/11/2015 07:20 PM, Alan Stern wrote:
> On Fri, 11 Sep 2015, Igor Kotrasinski wrote:
>
>> currently, when a zlp flag is set and an urb/usb_request
>> buffer is filled without a short packet, transfer() leaves
>> its status at -EINPROGRESS and does not rescan for short
>> packet.
>>
>> In a scenario where ep.maxpacket bytes are copied,
>> URB_ZERO_PACKET is set, urb buffer is filled and usb_request
>> buffer is not, transfer() returns with an urb with
>> -EINPROGRESS status, which dummy_hcd treats as incomplete
>> transfer.
>>
>> Check for zlp and rescan appropriately.
>>
>> Signed-off-by: Igor Kotrasinski
> Acked-by: Alan Stern
>
> I can't believe we missed this for so long... This should be marked
> for -stable.
>
> While you're at it, you might like to clean up a few other things. For
> example, just before the code changed in this patch we have:
>
> ...
> } else if (to_host) {
> ...
> } else if (!to_host) {
> ...
>
> Obviously the "if (!to_host)" test is unnecessary.
>
> Also, a little above that we have:
>
> is_short = (len % ep->ep.maxpacket) != 0;
>
> But at this point we know that len != 0, and we also know that if len
>> = ep->ep.maxpacket then in fact len is a multiple of ep->ep.maxpacket.
> Hence the computation can be simplified to:
>
> is_short = (len < ep->ep.maxpacket);
>
> Alan Stern
>
>
I found and fixed a handful of issues, sending in a v2 patchset.
Igor Kotrasiński
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/5] usb: gadget: dummy_hcd: fix unneeded else-if condition
Signed-off-by: Igor Kotrasinski
---
drivers/usb/gadget/udc/dummy_hcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index 59be03e..183e368 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1356,7 +1356,7 @@ top:
*status = -EOVERFLOW;
else
*status = 0;
- } else if (!to_host) {
+ } else {
*status = 0;
if (host_len > dev_len)
req->req.status = -EOVERFLOW;
--
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 v2 4/5] usb: gadget: dummy_hcd: fix rescan logic for transfer
transfer() schedules a rescan for transfers larger than
maxpacket, which is wrong for transfers that are multiples
of maxpacket.
Rewrite to fix and clarify packet multiple / remainder
transfer logic.
Signed-off-by: Igor Kotrasinski
---
drivers/usb/gadget/udc/dummy_hcd.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index 552c533..0ba0ab3 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1288,6 +1288,7 @@ top:
/* if there's no request queued, the device is NAKing; return */
list_for_each_entry(req, &ep->queue, queue) {
unsignedhost_len, dev_len, len;
+ unsignedquot, rem;
int is_short, to_host;
int rescan = 0;
@@ -1324,12 +1325,18 @@ top:
if (len == 0)
break;
- /* use an extra pass for the final short packet */
- if (len > ep->ep.maxpacket) {
- rescan = 1;
- len -= (len % ep->ep.maxpacket);
+ /* send multiple of maxpacket first, then remainder */
+ rem = (len % ep->ep.maxpacket);
+ quot = len - rem;
+ if (quot > 0) {
+ is_short = 0;
+ len = quot;
+ if (rem > 0)
+ rescan = 1;
+ } else { /* rem > 0 */
+ is_short = 1;
+ len = rem;
}
- is_short = (len % ep->ep.maxpacket) != 0;
len = dummy_perform_transfer(urb, req, len);
--
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 v2 5/5] usb: gadget: dummy_hcd: in transfer(), return data sent, not limit
dummy_timer uses transfer() to update transfer limit. However,
limit passed to dummy_timer changes depending on transfer type,
so the actual limit is overwritten.
This can cause unpredictably slow / fast bulk transfers when
coupled with control / interrupt transfers.
Fix by returning actual amount of data sent in transfer() and
substracting from total.
Signed-off-by: Igor Kotrasinski
---
drivers/usb/gadget/udc/dummy_hcd.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index 0ba0ab3..38f51ff 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1283,6 +1283,7 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb
*urb,
{
struct dummy*dum = dum_hcd->dum;
struct dummy_request*req;
+ int sent = 0;
top:
/* if there's no request queued, the device is NAKing; return */
@@ -1345,6 +1346,7 @@ top:
req->req.status = len;
} else {
limit -= len;
+ sent += len;
urb->actual_length += len;
req->req.actual += len;
}
@@ -1414,7 +1416,7 @@ top:
if (rescan)
goto top;
}
- return limit;
+ return sent;
}
static int periodic_bytes(struct dummy *dum, struct dummy_ep *ep)
@@ -1844,7 +1846,7 @@ restart:
default:
treat_control_like_bulk:
ep->last_io = jiffies;
- total = transfer(dum_hcd, urb, ep, limit, &status);
+ total -= transfer(dum_hcd, urb, ep, limit, &status);
break;
}
--
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 v2 1/5] usb: gadget: dummy_hcd: emulate sending zlp in packet logic
currently, when a zlp flag is set and an urb/usb_request
buffer is filled without a short packet, transfer() leaves
its status at -EINPROGRESS and does not rescan for short
packet.
In a scenario where ep.maxpacket bytes are copied,
URB_ZERO_PACKET is set, urb buffer is filled and usb_request
buffer is not, transfer() returns with an urb with
-EINPROGRESS status, which dummy_hcd treats as incomplete
transfer.
Check for zlp and rescan appropriately.
Signed-off-by: Igor Kotrasinski
---
drivers/usb/gadget/udc/dummy_hcd.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index 181112c..59be03e 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1364,15 +1364,23 @@ top:
req->req.status = 0;
}
- /* many requests terminate without a short packet */
+ /* many requests terminate without a short packet.
+* send a zlp if demanded by flags.
+*/
} else {
- if (req->req.length == req->req.actual
- && !req->req.zero)
- req->req.status = 0;
- if (urb->transfer_buffer_length == urb->actual_length
- && !(urb->transfer_flags
- & URB_ZERO_PACKET))
- *status = 0;
+ if (req->req.length == req->req.actual) {
+ if (req->req.zero && to_host)
+ rescan = 1;
+ else
+ req->req.status = 0;
+ }
+ if (urb->transfer_buffer_length == urb->actual_length) {
+ if (urb->transfer_flags & URB_ZERO_PACKET &&
+ !to_host)
+ rescan = 1;
+ else
+ *status = 0;
+ }
}
/* device side completion --> continuable */
--
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 v2 3/5] usb: gadget: dummy_hcd: round down transfer when over frame limit
When close to transfer limit for a frame, transfer() decreases
data length to send to limit. This can cause an erroneous short
packet transfer to be simulated.
Rework transfer logic to decrease data to tranfer to multiple of
maxpacket.
Signed-off-by: Igor Kotrasinski
---
drivers/usb/gadget/udc/dummy_hcd.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index 183e368..552c533 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1313,10 +1313,14 @@ top:
if (unlikely(len == 0))
is_short = 1;
else {
- /* not enough bandwidth left? */
- if (limit < ep->ep.maxpacket && limit < len)
- break;
- len = min_t(unsigned, len, limit);
+ /* not enough bandwidth left?
+* only send multiple of maxpacket to avoid
+* unwarranted short packet.
+*/
+ if (limit < len) {
+ len = limit;
+ len -= (len % ep->ep.maxpacket);
+ }
if (len == 0)
break;
--
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 00/39] drop null test before destroy functions
> Recent commits to kernel/git/torvalds/linux.git have made the following > functions able to tolerate NULL arguments: > > kmem_cache_destroy (commit 3942d29918522) > mempool_destroy (commit 4e3ca3e033d1) > dma_pool_destroy (commit 44d7175da6ea) How do you think about to extend an other SmPL script? Related topic: scripts/coccinelle/free: Delete NULL test before freeing functions https://systeme.lip6.fr/pipermail/cocci/2015-May/001960.html https://www.mail-archive.com/cocci@systeme.lip6.fr/msg01855.html > If these changes are OK, I will address the remainder later. Would anybody like to reuse my general SmPL approach for similar source code clean-up? Regards, Markus -- 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] Revert "usb: chipidea: usbmisc_imx: delete clock information"
On Mon, Sep 14, 2015 at 1:54 AM, Peter Chen wrote: > On Fri, Sep 11, 2015 at 10:17:20AM -0300, Fabio Estevam wrote: > Sorry, a bug at former patch, it the clk_ahb has been override. > Mind to try below one? Applied the patch and still see the issue: Booting Linux on physical CPU 0x0 Linux version 4.3.0-rc1-next-20150914-dirty (fabio@fabio-Latitude-E6410) (gcc v5 CPU: ARM926EJ-S [41069264] revision 4 (ARMv5TEJ), cr=0005317f CPU: VIVT data cache, VIVT instruction cache Machine model: Freescale i.MX27 Product Development Kit Memory policy: Data cache writeback Built 1 zonelists in Zone order, mobility grouping on. Total pages: 32512 Kernel command line: noinitrd console=ttymxc0,115200 root=/dev/nfs nfsroot=192.p PID hash table entries: 512 (order: -1, 2048 bytes) Dentry cache hash table entries: 16384 (order: 4, 65536 bytes) Inode-cache hash table entries: 8192 (order: 3, 32768 bytes) Memory: 121952K/131072K available (5428K kernel code, 370K rwdata, 1636K rodata) Virtual kernel memory layout: vector : 0x - 0x1000 ( 4 kB) fixmap : 0xffc0 - 0xfff0 (3072 kB) vmalloc : 0xc880 - 0xff00 ( 872 MB) lowmem : 0xc000 - 0xc800 ( 128 MB) modules : 0xbf00 - 0xc000 ( 16 MB) .text : 0xc0008000 - 0xc06ee5f8 (7066 kB) .init : 0xc06ef000 - 0xc073a000 ( 300 kB) .data : 0xc073a000 - 0xc0796aac ( 371 kB) .bss : 0xc0796aac - 0xc07b6a18 ( 128 kB) Preemptible hierarchical RCU implementation. Build-time adjustment of leaf fanout to 32. NR_IRQS:16 nr_irqs:16 16 MXC IRQ initialized CPU identified as i.MX27, silicon rev 2.1 Switching to timer-based delay loop, resolution 75ns sched_clock: 32 bits at 13MHz, resolution 75ns, wraps every 161464936410ns clocksource: mxc_timer1: mask: 0x max_cycles: 0x, max_idle_ns: s Console: colour dummy device 80x30 Calibrating delay loop (skipped), value calculated using timer frequency.. 26.6) pid_max: default: 32768 minimum: 301 Mount-cache hash table entries: 1024 (order: 0, 4096 bytes) Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes) CPU: Testing write buffer coherency: ok Setting up static identity map for 0xa0008400 - 0xa000847c devtmpfs: initialized clocksource: jiffies: mask: 0x max_cycles: 0x, max_idle_ns: 191s pinctrl core: initialized pinctrl subsystem NET: Registered protocol family 16 DMA: preallocated 256 KiB pool for atomic coherent allocations imx27-pinctrl 10015000.iomuxc: initialized IMX pinctrl driver SCSI subsystem initialized usbcore: registered new interface driver usbfs usbcore: registered new interface driver hub usbcore: registered new device driver usb usbphy:usbphy@0 supply vcc not found, using dummy regulator Linux video capture interface: v2.00 pps_core: LinuxPPS API ver. 1 registered pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti PTP clock support registered Advanced Linux Sound Architecture Driver Initialized. clocksource: Switched to clocksource mxc_timer1 NET: Registered protocol family 2 TCP established hash table entries: 1024 (order: 0, 4096 bytes) TCP bind hash table entries: 1024 (order: 0, 4096 bytes) TCP: Hash tables configured (established 1024 bind 1024) UDP hash table entries: 256 (order: 0, 4096 bytes) UDP-Lite hash table entries: 256 (order: 0, 4096 bytes) NET: Registered protocol family 1 RPC: Registered named UNIX socket transport module. RPC: Registered udp transport module. RPC: Registered tcp transport module. RPC: Registered tcp NFSv4.1 backchannel transport module. futex hash table entries: 256 (order: -1, 3072 bytes) jffs2: version 2.2. (NAND) �© 2001-2006 Red Hat, Inc. io scheduler noop registered (default) 1000a000.serial: ttymxc0 at MMIO 0x1000a000 (irq = 36, base_baud = 831250) is aX console [ttymxc0] enabled nand: device found, Manufacturer ID: 0xec, Chip ID: 0xaa nand: Samsung NAND 256MiB 1,8V 8-bit nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64 Bad block table found at page 131008, version 0x01 Bad block table found at page 130944, version 0x01 mc13xxx spi1.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0 spi_imx 1000f000.cspi: probed 1002b000.ethernet supply phy not found, using dummy regulator libphy: fec_enet_mii_bus: probed ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver ehci-mxc: Freescale On-Chip EHCI Host driver usbcore: registered new interface driver usb-storage imx_usb 10024000.usb: three clock have enabled Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600 pgd = c0004000 [f4424600] *pgd=1452(bad) Internal error: : 8 [#1] PREEMPT ARM Modules linked in: CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0-rc1-next-20150914-dirty #56 Hardware name: Freescale i.MX27 (Device Tree Support) task: c7832b60 ti: c783e000 task.ti: c783e000 PC is at usbmisc_imx27_init+0x4c/0xbc LR is at usbmisc_imx27_init+0x40/0xbc pc : []lr : []psr: 6093 sp : c783fe00 ip :
Re: [PATCH v2 4/5] usb: gadget: dummy_hcd: fix rescan logic for transfer
Hello.
On 9/14/2015 12:45 PM, Igor Kotrasinski wrote:
transfer() schedules a rescan for transfers larger than
maxpacket, which is wrong for transfers that are multiples
of maxpacket.
Rewrite to fix and clarify packet multiple / remainder
transfer logic.
Signed-off-by: Igor Kotrasinski
---
drivers/usb/gadget/udc/dummy_hcd.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index 552c533..0ba0ab3 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
[...]
@@ -1324,12 +1325,18 @@ top:
if (len == 0)
break;
- /* use an extra pass for the final short packet */
- if (len > ep->ep.maxpacket) {
- rescan = 1;
- len -= (len % ep->ep.maxpacket);
+ /* send multiple of maxpacket first, then remainder */
+ rem = (len % ep->ep.maxpacket);
Parens not needed here (and weren't needed before your change).
[...]
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] usb: gadget: dummy_hcd: fix rescan logic for transfer
transfer() schedules a rescan for transfers larger than
maxpacket, which is wrong for transfers that are multiples
of maxpacket.
Rewrite to fix and clarify packet multiple / remainder
transfer logic.
Signed-off-by: Igor Kotrasinski
---
drivers/usb/gadget/udc/dummy_hcd.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index 552c533..0ba0ab3 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1288,6 +1288,7 @@ top:
/* if there's no request queued, the device is NAKing; return */
list_for_each_entry(req, &ep->queue, queue) {
unsignedhost_len, dev_len, len;
+ unsignedquot, rem;
int is_short, to_host;
int rescan = 0;
@@ -1324,12 +1325,18 @@ top:
if (len == 0)
break;
- /* use an extra pass for the final short packet */
- if (len > ep->ep.maxpacket) {
- rescan = 1;
- len -= (len % ep->ep.maxpacket);
+ /* send multiple of maxpacket first, then remainder */
+ rem = len % ep->ep.maxpacket;
+ quot = len - rem;
+ if (quot > 0) {
+ is_short = 0;
+ len = quot;
+ if (rem > 0)
+ rescan = 1;
+ } else { /* rem > 0 */
+ is_short = 1;
+ len = rem;
}
- is_short = (len % ep->ep.maxpacket) != 0;
len = dummy_perform_transfer(urb, req, len);
--
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 V5 1/1] usb:serial add Fintek F81532/534 driver
On Tue, Jul 21, 2015 at 09:58:19AM +0800, Peter Hung wrote: > This driver is for Fintek F81532/F81534 USB to Serial Ports IC. > > Features: > 1. F81534 is 1-to-4 & F81532 is 1-to-2 serial ports IC > 2. Support Baudrate from B50 to B150 (excluding B100). > 3. The RTS signal can be transformed their behavior with >configuration by default ioctl TIOCGRS485/TIOCSRS485 >(for RS232/RS485/RS422 with transceiver) > >If the driver setting with SER_RS485_ENABLED, the RTS signal will >high with not in TX and low with in TX. > >If the driver setting with SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND, >the RTS signal will low with not in TX and high with in TX. > > 4. There are 4x3 output-only ic pins to control transceiver mode. Are these pins only used to control the transceiver mode, and not for general purpose IO? >It's can be controlled via gpiolib. We could found the gpio >number from /sys/class/tty/ttyUSB[x]/device/gpiochap[yyy] where >x is F81532/534 serial port and yyy is gpiochip number. > >After we found chip number, we can export 3 gpios(M2/M1/M0) per >serial port by > echo yyy > /sys/class/gpio/export > echo yyy+1 > /sys/class/gpio/export > echo yyy+2 > /sys/class/gpio/export > >then we can control it with > echo [M0 value] > /sys/class/gpio/gpio[yyy]/value > echo [M1 value] > /sys/class/gpio/gpio[yyy+1]/value > echo [M2 value] > /sys/class/gpio/gpio[yyy+2]/value >which M0/M1/M2 as your desired value, value is only 0 or 1. > >When configure complete, It's a must to free all gpio by > echo yyy > /sys/class/gpio/unexport > echo yyy+1 > /sys/class/gpio/unexport > echo yyy+2 > /sys/class/gpio/unexport You don't need to document how to use the gpiolib sysfs interface here. Just mention that the driver exports these gpios. >The driver will "save" gpio configure after we release >all gpio of a serial port. > >For examples to change mode & gpio with F81532/534 >Evalaution Board. > >F81532 EVB > port0: F81437 (RS232 only) > port1: F81439 (RS232/RS485/RS422 ... etc.) >F81534 EVB > port0/1: F81437 (RS232 only) > port2/3: F81439 (RS232/RS485/RS422 ... etc.) > > 1. RS232 Mode (Default IC Mode) > 1. Set struct serial_rs485 flags "without" SER_RS485_ENABLED > (control F81532/534 RTS control) > 2. Set M2/M1/M0 as 0/0/1 > (control F81532/534 output pin to control transceiver mode) > > 2. RS485 Mode (RTS Low when TX Mode) > 1. Set struct serial_rs485 flags with SER_RS485_ENABLED > 2. Set M2/M1/M0 as 0/1/0 > > 3. RS485 Mode (RTS High when TX Mode) > 1. Set struct serial_rs485 flags with SER_RS485_ENABLED and > SER_RS485_RTS_ON_SEND > 2. Set M2/M1/M0 as 0/1/1 > > 4. RS422 Mode > 1. The RTS mode is dont care. > 2. Set M2/M1/M0 as 0/0/0 I don't think all gpios should be exported for these ports if they have special functions that the driver could control transparently (e.g. for SER_RS485_RTS_ON_SEND). We need to come up with a way to deal with rs422 mode though. >Please reference https://bitbucket.org/hpeter/fintek-general/src/ >with f81534/tools to get set_gpio.c & set_mode.c. Please use it >carefully. > > Changelog > v5 > 1. Change the configure data address F81534_CUSTOM_ADDRESS_START >from 0x4000 to 0x2f00 (for MP F/W ver:AA66) > 2. Change f81534_port_disable/enable() from H/W mode to S/W mode >It'll skip all rx data when port is not opened. > 3. Some function modifier add with static (Thanks for Paul Bolle) > 4. It's will direct return when count=0 in f81534_write() to >reduce spin_lock usage. > > v4 > 1. clearify f81534_process_read_urb() with >f81534_process_per_serial_block(). (referenced from mxuport.c) > 2. We limited f81534_write() max tx kfifo with 124Bytes. >Original subsystem is designed for auto tranmiting fifo data >if available. But we must wait for tx_empty for next tx data >(H/W design). > >With this kfifo size limit, we can use generic subsystem api with >f81534_write(). When usb_serial_generic_write_start() called after >first write URB complete, the fifo will no data. The generic >subsystem of write will go to idle state. Until we received >TX_EMPTY and release write spinlock, the fifo will fill max >124Bytes by following f81534_write(). > > v3 > 1. Migrate read, write and some routine from custom code to usbserial >subsystem callback function. > 2. Use more defines to replece magic numbers to make it meaningful > 3. Make more comments as document in source code. > > v2 > 1. v1 version submit to staging tree, but Greg KH advised me to >cleanup source code & re-submit it to correct subsystem > 2. Remove all custom ioctl c
[PATCH] usb: musb: ensure in peripheral mode when checking session
The change ensures otg is not in a A- state when checking for VBUS in
peripheral mode.
musb_start() where VBUS checking is in can be called in many situations.
One example is in babble recovery routine, in which otg is transitioning
from A-HOST to A-WAIT-BCON, but VBUS discharge takes time, so
musb->is_active could be set to 1 due to this improper checking, then it
causes musb_bus_suspend() failed which leads to warning log message
flooding.
Signed-off-by: Bin Liu
---
Fixes LCPD-4833: CONNECTIVITY: AM33XX: Issue when keyboard hub on otg
port and powered hub on host port
drivers/usb/musb/musb_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index e16451c..0fcf01f 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1041,6 +1041,7 @@ void musb_start(struct musb *musb)
* (c) peripheral initiates, using SRP
*/
if (musb->port_mode != MUSB_PORT_MODE_HOST &&
+ musb->xceiv->otg->state != OTG_STATE_A_WAIT_BCON &&
(devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS) {
musb->is_active = 1;
} else {
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/5] usb: gadget: dummy_hcd: fix unneeded else-if condition
On Mon, 14 Sep 2015, Igor Kotrasinski wrote:
> Signed-off-by: Igor Kotrasinski
> ---
> drivers/usb/gadget/udc/dummy_hcd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
> b/drivers/usb/gadget/udc/dummy_hcd.c
> index 59be03e..183e368 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -1356,7 +1356,7 @@ top:
> *status = -EOVERFLOW;
> else
> *status = 0;
> - } else if (!to_host) {
> + } else {
> *status = 0;
> if (host_len > dev_len)
> req->req.status = -EOVERFLOW;
Acked-by: 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 v2 1/5] usb: gadget: dummy_hcd: emulate sending zlp in packet logic
On Mon, 14 Sep 2015, Igor Kotrasinski wrote:
> currently, when a zlp flag is set and an urb/usb_request
> buffer is filled without a short packet, transfer() leaves
> its status at -EINPROGRESS and does not rescan for short
> packet.
>
> In a scenario where ep.maxpacket bytes are copied,
> URB_ZERO_PACKET is set, urb buffer is filled and usb_request
> buffer is not, transfer() returns with an urb with
> -EINPROGRESS status, which dummy_hcd treats as incomplete
> transfer.
>
> Check for zlp and rescan appropriately.
>
> Signed-off-by: Igor Kotrasinski
> ---
> drivers/usb/gadget/udc/dummy_hcd.c | 24
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
> b/drivers/usb/gadget/udc/dummy_hcd.c
> index 181112c..59be03e 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -1364,15 +1364,23 @@ top:
> req->req.status = 0;
> }
>
> - /* many requests terminate without a short packet */
> + /* many requests terminate without a short packet.
> + * send a zlp if demanded by flags.
> + */
The style we are using now for multi-line comments is like this:
/*
* blah, blah, blah
* blah, blah, blah
*/
I realize that's not how the existing comments look, but we try to
enforce this style for all new comments.
> } else {
> - if (req->req.length == req->req.actual
> - && !req->req.zero)
> - req->req.status = 0;
> - if (urb->transfer_buffer_length == urb->actual_length
> - && !(urb->transfer_flags
> - & URB_ZERO_PACKET))
> - *status = 0;
> + if (req->req.length == req->req.actual) {
> + if (req->req.zero && to_host)
> + rescan = 1;
> + else
> + req->req.status = 0;
> + }
> + if (urb->transfer_buffer_length == urb->actual_length) {
> + if (urb->transfer_flags & URB_ZERO_PACKET &&
> + !to_host)
> + rescan = 1;
> + else
> + *status = 0;
> + }
> }
>
> /* device side completion --> continuable */
Apart from that style issue,
Acked-by: Alan Stern
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 v2 3/5] usb: gadget: dummy_hcd: round down transfer when over frame limit
On Mon, 14 Sep 2015, Igor Kotrasinski wrote:
> When close to transfer limit for a frame, transfer() decreases
> data length to send to limit. This can cause an erroneous short
> packet transfer to be simulated.
I don't see how that could happen.
> Rework transfer logic to decrease data to tranfer to multiple of
> maxpacket.
>
> Signed-off-by: Igor Kotrasinski
> ---
> drivers/usb/gadget/udc/dummy_hcd.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
> b/drivers/usb/gadget/udc/dummy_hcd.c
> index 183e368..552c533 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -1313,10 +1313,14 @@ top:
> if (unlikely(len == 0))
> is_short = 1;
> else {
> - /* not enough bandwidth left? */
> - if (limit < ep->ep.maxpacket && limit < len)
> - break;
> - len = min_t(unsigned, len, limit);
> + /* not enough bandwidth left?
> + * only send multiple of maxpacket to avoid
> + * unwarranted short packet.
> + */
> + if (limit < len) {
> + len = limit;
> + len -= (len % ep->ep.maxpacket);
> + }
> if (len == 0)
> break;
>
I don't think this patch is needed. It looks like the new code does
exactly the same thing as the old code. In particular, if limit >= len
then both the old and new code both do nothing. If limit < len then:
Old code: If limit < ep->ep.maxpacket then break.
Otherwise set len = limit, and later on (just after the
end of this patch) len will be rounded down to a multiple
of ep->ep.maxpacket.
New code: len = limit rounded down to a multiple of
ep->ep.maxpacket. If this is 0 -- i.e.,
if limit < ep->ep.maxpacket -- then break.
Thus they end up doing the same thing.
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 v2 4/5] usb: gadget: dummy_hcd: fix rescan logic for transfer
On Mon, 14 Sep 2015, Igor Kotrasinski wrote:
> transfer() schedules a rescan for transfers larger than
> maxpacket, which is wrong for transfers that are multiples
> of maxpacket.
>
> Rewrite to fix and clarify packet multiple / remainder
> transfer logic.
>
> Signed-off-by: Igor Kotrasinski
> ---
> drivers/usb/gadget/udc/dummy_hcd.c | 17 -
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
> b/drivers/usb/gadget/udc/dummy_hcd.c
> index 552c533..0ba0ab3 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -1288,6 +1288,7 @@ top:
> /* if there's no request queued, the device is NAKing; return */
> list_for_each_entry(req, &ep->queue, queue) {
> unsignedhost_len, dev_len, len;
> + unsignedquot, rem;
> int is_short, to_host;
> int rescan = 0;
>
> @@ -1324,12 +1325,18 @@ top:
> if (len == 0)
> break;
>
> - /* use an extra pass for the final short packet */
> - if (len > ep->ep.maxpacket) {
> - rescan = 1;
> - len -= (len % ep->ep.maxpacket);
> + /* send multiple of maxpacket first, then remainder */
> + rem = (len % ep->ep.maxpacket);
> + quot = len - rem;
You shouldn't call this "quot" because it isn't a quotient. In fact,
you don't really need quot at all; you could just write "len -= rem;".
> + if (quot > 0) {
> + is_short = 0;
> + len = quot;
> + if (rem > 0)
> + rescan = 1;
> + } else { /* rem > 0 */
> + is_short = 1;
> + len = rem;
> }
> - is_short = (len % ep->ep.maxpacket) != 0;
>
> len = dummy_perform_transfer(urb, req, len);
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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hi, On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede wrote: > Hi, > > On 10-09-15 20:30, Maxime Ripard wrote: >> >> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote: >>> >>> Hi, >>> >>> On 04-09-15 08:43, Olliver Schinagl wrote: Hey Hans, On 07-08-15 10:45, Olliver Schinagl wrote: > > >> >> If you change the dr_mode to host then you _must_ also remove any >> id_det and vbus_det >> gpio settings from the usb_phy node in the dts, as the sun4i phy code >> detects >> host vs otg mode by checking for the presence of these. > > Yes, this fixes it and makes it work. Thanks. > I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset? >>> >>> >>> AFAIK you cannot unset something in dts. The only solution I >>> can comeup with is to add a dr_mode argument to the phy like >>> we already have for the otg controller itself. >>> >>> This is something which we likely need to do anyways to add >>> support for peripheral only mode, which we seem to need for >>> some "hdmi sticks". >> >> >> I haven't really followed the rest of the discussion, so sorry if you >> already talked about that, but why can't you just set the dr_mode to >> peripheral in such a case? > > > This is about the usbphy code not the musb-controller code, which are > 2 different dts nodes, atm only the musb-controller node has a > dr_mode property, and the phy code decides between host-only > and otg mode based on whether an id pin is assigned or not. > > My proposal is to get rid of the id-pin hack to determine the mode > and add a dr_mode property to the usbphy dts node. I would try to avoid adding dr_mode in the usbphy node if possible, since it is already specified in the controller node. Since the phy node is referenced in the controller node, is it possible that the phy driver looks up the controller of properties to find its dr_mode setting? I am currently doing the same thing for musb_dsps. Experienmenting...since I don't much about dts handling yet. Regards, -Bin. > > > Regards, > > Hans > -- > 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 v2 5/5] usb: gadget: dummy_hcd: in transfer(), return data sent, not limit
On Mon, 14 Sep 2015, Igor Kotrasinski wrote:
> dummy_timer uses transfer() to update transfer limit. However,
> limit passed to dummy_timer changes depending on transfer type,
> so the actual limit is overwritten.
>
> This can cause unpredictably slow / fast bulk transfers when
> coupled with control / interrupt transfers.
>
> Fix by returning actual amount of data sent in transfer() and
> substracting from total.
>
> Signed-off-by: Igor Kotrasinski
> ---
> drivers/usb/gadget/udc/dummy_hcd.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
> b/drivers/usb/gadget/udc/dummy_hcd.c
> index 0ba0ab3..38f51ff 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -1283,6 +1283,7 @@ static int transfer(struct dummy_hcd *dum_hcd, struct
> urb *urb,
> {
> struct dummy*dum = dum_hcd->dum;
> struct dummy_request*req;
> + int sent = 0;
>
> top:
> /* if there's no request queued, the device is NAKing; return */
> @@ -1345,6 +1346,7 @@ top:
> req->req.status = len;
> } else {
> limit -= len;
> + sent += len;
> urb->actual_length += len;
> req->req.actual += len;
> }
> @@ -1414,7 +1416,7 @@ top:
> if (rescan)
> goto top;
> }
> - return limit;
> + return sent;
> }
>
> static int periodic_bytes(struct dummy *dum, struct dummy_ep *ep)
> @@ -1844,7 +1846,7 @@ restart:
> default:
> treat_control_like_bulk:
> ep->last_io = jiffies;
> - total = transfer(dum_hcd, urb, ep, limit, &status);
> + total -= transfer(dum_hcd, urb, ep, limit, &status);
> break;
> }
Acked-by: 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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hi, On 14-09-15 16:44, Bin Liu wrote: Hi, On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede wrote: Hi, On 10-09-15 20:30, Maxime Ripard wrote: On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote: Hi, On 04-09-15 08:43, Olliver Schinagl wrote: Hey Hans, On 07-08-15 10:45, Olliver Schinagl wrote: If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det gpio settings from the usb_phy node in the dts, as the sun4i phy code detects host vs otg mode by checking for the presence of these. Yes, this fixes it and makes it work. Thanks. I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset? AFAIK you cannot unset something in dts. The only solution I can comeup with is to add a dr_mode argument to the phy like we already have for the otg controller itself. This is something which we likely need to do anyways to add support for peripheral only mode, which we seem to need for some "hdmi sticks". I haven't really followed the rest of the discussion, so sorry if you already talked about that, but why can't you just set the dr_mode to peripheral in such a case? This is about the usbphy code not the musb-controller code, which are 2 different dts nodes, atm only the musb-controller node has a dr_mode property, and the phy code decides between host-only and otg mode based on whether an id pin is assigned or not. My proposal is to get rid of the id-pin hack to determine the mode and add a dr_mode property to the usbphy dts node. I would try to avoid adding dr_mode in the usbphy node if possible, since it is already specified in the controller node. Since the phy node is referenced in the controller node, is it possible that the phy driver looks up the controller of properties to find its dr_mode setting? That is possible, but it very much goes against how devicetree normally works, where every node is more or less a standalone unit which contains enough info for the associated driver to do its work. Currently there is no link from the phy node to the controller node (only the other way around) and adding such a link requires more code then simple having a duplicate dr_mode. Regards, Hans -- 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: similar files: fusbh200-hcd.c and fotg210-hcd.c
On Sat, Sep 12, 2015 at 03:14:50PM +0200, Peter Senna Tschudin wrote: > >> Should these files be consolidated? And if so how? > > if you can find an easy way, that would be a very, very welcome patch. > > Is the ideal solution to consolidate both fusbh200-hcd.c and > fotg210-hcd.c in a single module? If this is the case, how to detect > at run time which version of the hw is present? Both are registered as does it matter ? If they work the same way, why does it matter which one's running? > platform devices and I could not find an obvious way to detect the > model at run time. I could successfully load fusbh200-hcd on my fedora is there a revision register ? Or you may use different platform_device names with platform_device_id table. > notebook (hp elitebook 840), and on a VM, even if neither has the hw > ($ sudo modprobe fusbh200-hcd). The module loads with the warning > "fusbh200_hcd should always be loaded before uhci_hcd and ohci_hcd, > not after". On another workstation running ubuntu, I could load both > modules at the same time, producing the same warning for each module. > Should the module load if the device is not present? > > Other solution for consolidation would be to create a common_code.c, > keeping both fusbh200-hcd.c and fotg210-hcd.c only with the code that > differ. Is this better than what is there now? > > Other ideas? just combine them :-p Use platform_device_id to differentiate. -- balbi signature.asc Description: Digital signature
[PATCH 01/16] usb: gadget: amd5536udc: introduce free_dma_pools
dma pools are being created by init_dma_pools() but there was no
function for freeing them. Introduce the function now so that we can use
it later.
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index fdacddb..384b824 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3107,6 +3107,17 @@ static void udc_remove(struct udc *dev)
udc = NULL;
}
+/* free all the dma pools */
+static void free_dma_pools(struct udc *dev)
+{
+ dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td,
+ dev->ep[UDC_EP0OUT_IX].td_phys);
+ dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td_stp,
+ dev->ep[UDC_EP0OUT_IX].td_stp_dma);
+ dma_pool_destroy(dev->stp_requests);
+ dma_pool_destroy(dev->data_requests);
+}
+
/* Reset all pci context */
static void udc_pci_remove(struct pci_dev *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 14/16] usb: gadget: amd5536udc: NULL comparison
A NULL comparison can be written as if (var) or if (!var).
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index f6d6097..3657a66 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -2189,7 +2189,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int
ep_ix)
}
/* DMA */
- } else if (!ep->cancel_transfer && req != NULL) {
+ } else if (!ep->cancel_transfer && req) {
ret_val = IRQ_HANDLED;
/* check for DMA done */
@@ -3139,7 +3139,7 @@ static void udc_pci_remove(struct pci_dev *pdev)
usb_del_gadget_udc(&udc->gadget);
/* gadget driver must not be registered */
- if (WARN_ON(dev->driver != NULL))
+ if (WARN_ON(dev->driver))
return;
/* dma pool cleanup */
@@ -3193,7 +3193,7 @@ static int init_dma_pools(struct udc *dev)
/* setup */
td_stp = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
&dev->ep[UDC_EP0OUT_IX].td_stp_dma);
- if (td_stp == NULL) {
+ if (!td_stp) {
retval = -ENOMEM;
goto err_alloc_dma;
}
@@ -3202,7 +3202,7 @@ static int init_dma_pools(struct udc *dev)
/* data: 0 packets !? */
td_data = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
&dev->ep[UDC_EP0OUT_IX].td_phys);
- if (td_data == NULL) {
+ if (!td_data) {
retval = -ENOMEM;
goto err_alloc_phys;
}
@@ -3328,7 +3328,7 @@ static int udc_pci_probe(
dev->mem_region = 1;
dev->virt_addr = ioremap_nocache(resource, len);
- if (dev->virt_addr == NULL) {
+ if (!dev->virt_addr) {
dev_dbg(&pdev->dev, "start address cannot be mapped\n");
retval = -EFAULT;
goto err_ioremap;
--
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 16/16] usb: gadget: amd5536udc: match alignment
checkpatch complains that the alignment should match with open
parenthesis.
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 156 ++--
1 file changed, 78 insertions(+), 78 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index 98b841d..9f73de8 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -226,7 +226,7 @@ module_param(use_dma_ppb, bool, S_IRUGO);
MODULE_PARM_DESC(use_dma_ppb, "true for DMA in packet per buffer mode");
module_param(use_dma_ppb_du, bool, S_IRUGO);
MODULE_PARM_DESC(use_dma_ppb_du,
- "true for DMA in packet per buffer mode with descriptor update");
+"true for DMA in packet per buffer mode with descriptor
update");
module_param(use_fullspeed, bool, S_IRUGO);
MODULE_PARM_DESC(use_fullspeed, "true for fullspeed only");
@@ -436,7 +436,7 @@ udc_ep_enable(struct usb_ep *usbep, const struct
usb_endpoint_descriptor *desc)
/* set max packet size UDC CSR */
tmp = readl(&dev->csr->ne[ep->num - UDC_CSR_EP_OUT_IX_OFS]);
tmp = AMD_ADDBITS(tmp, maxpacket,
- UDC_CSR_NE_MAX_PKT);
+ UDC_CSR_NE_MAX_PKT);
writel(tmp, &dev->csr->ne[ep->num - UDC_CSR_EP_OUT_IX_OFS]);
if (use_dma && !ep->in) {
@@ -581,7 +581,7 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
if (ep->dma) {
/* ep0 in requests are allocated from data pool here */
dma_desc = pci_pool_alloc(ep->dev->data_requests, gfp,
- &req->td_phys);
+ &req->td_phys);
if (!dma_desc) {
kfree(req);
return NULL;
@@ -622,7 +622,7 @@ static int udc_free_dma_chain(struct udc *dev, struct
udc_request *req)
for (i = 1; i < req->chain_len; i++) {
pci_pool_free(dev->data_requests, td,
- (dma_addr_t) td_last->next);
+ (dma_addr_t) td_last->next);
td_last = td;
td = phys_to_virt(td_last->next);
}
@@ -652,7 +652,7 @@ udc_free_request(struct usb_ep *usbep, struct usb_request
*usbreq)
udc_free_dma_chain(ep->dev, req);
pci_pool_free(ep->dev->data_requests, req->td_data,
- req->td_phys);
+ req->td_phys);
}
kfree(req);
}
@@ -672,7 +672,7 @@ static void udc_init_bna_dummy(struct udc_request *req)
UDC_DMA_STP_STS_BS);
#ifdef UDC_VERBOSE
pr_debug("bna desc = %p, sts = %08x\n",
- req->td_data, req->td_data->status);
+req->td_data, req->td_data->status);
#endif
}
}
@@ -723,7 +723,7 @@ udc_txfifo_write(struct udc_ep *ep, struct usb_request *req)
/* remaining bytes must be written by byte access */
for (j = 0; j < bytes % UDC_DWORD_BYTES; j++) {
writeb((u8)(*(buf + i) >> (j << UDC_BITS_PER_BYTE_SHIFT)),
- ep->txfifo);
+ ep->txfifo);
}
/* dummy write confirm */
@@ -784,7 +784,7 @@ udc_rxfifo_read(struct udc_ep *ep, struct udc_request *req)
if (bytes > buf_space) {
if ((buf_space % ep->ep.maxpacket) != 0) {
DBG(ep->dev,
- "%s: rx %d bytes, rx-buf space = %d bytesn\n",
+ "%s: rx %d bytes, rx-buf space = %d bytesn\n",
ep->ep.name, bytes, buf_space);
req->req.status = -EOVERFLOW;
}
@@ -821,7 +821,7 @@ static int udc_create_dma_chain(
unsigned len;
VDBG(ep->dev, "udc_create_dma_chain: bytes=%ld buf_len=%ld\n",
- bytes, buf_len);
+bytes, buf_len);
dma_addr = DMA_DONT_USE;
/* unset L bit in first desc for OUT */
@@ -848,7 +848,7 @@ static int udc_create_dma_chain(
if (create_new_chain) {
td = pci_pool_alloc(ep->dev->data_requests,
- gfp_flags, &dma_addr);
+ gfp_flags, &dma_addr);
if (!td)
return -ENOMEM;
@@ -889,7 +889,7 @@ static int udc_create_dma_chain(
/* first desc */
req->td_data->status =
AMD_ADDBITS(req->td_data->status,
- ep->ep.maxpacket,
+ ep-
[PATCH 13/16] usb: gadget: amd5536udc: remove forward declaration of udc_basic_init
Rearrange the udc_basic_init function to remove the forward declaration.
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 55 ++---
1 file changed, 27 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index 789441f..f6d6097 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -65,7 +65,6 @@
static void udc_tasklet_disconnect(unsigned long);
static void empty_req_queue(struct udc_ep *);
-static void udc_basic_init(struct udc *dev);
static void udc_setup_endpoints(struct udc *dev);
static void udc_soft_reset(struct udc *dev);
static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
@@ -1511,33 +1510,6 @@ static void make_ep_lists(struct udc *dev)
dev->ep[UDC_EPOUT_IX].fifo_depth = UDC_RXFIFO_SIZE;
}
-/* init registers at driver load time */
-static int startup_registers(struct udc *dev)
-{
- u32 tmp;
-
- /* init controller by soft reset */
- udc_soft_reset(dev);
-
- /* mask not needed interrupts */
- udc_mask_unused_interrupts(dev);
-
- /* put into initial config */
- udc_basic_init(dev);
- /* link up all endpoints */
- udc_setup_endpoints(dev);
-
- /* program speed */
- tmp = readl(&dev->regs->cfg);
- if (use_fullspeed)
- tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_FS, UDC_DEVCFG_SPD);
- else
- tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_HS, UDC_DEVCFG_SPD);
- writel(tmp, &dev->regs->cfg);
-
- return 0;
-}
-
/* Inits UDC context */
static void udc_basic_init(struct udc *dev)
{
@@ -1576,6 +1548,33 @@ static void udc_basic_init(struct udc *dev)
dev->data_ep_queued = 0;
}
+/* init registers at driver load time */
+static int startup_registers(struct udc *dev)
+{
+ u32 tmp;
+
+ /* init controller by soft reset */
+ udc_soft_reset(dev);
+
+ /* mask not needed interrupts */
+ udc_mask_unused_interrupts(dev);
+
+ /* put into initial config */
+ udc_basic_init(dev);
+ /* link up all endpoints */
+ udc_setup_endpoints(dev);
+
+ /* program speed */
+ tmp = readl(&dev->regs->cfg);
+ if (use_fullspeed)
+ tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_FS, UDC_DEVCFG_SPD);
+ else
+ tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_HS, UDC_DEVCFG_SPD);
+ writel(tmp, &dev->regs->cfg);
+
+ return 0;
+}
+
/* Sets initial endpoint parameters */
static void udc_setup_endpoints(struct udc *dev)
{
--
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 15/16] usb: gadget: amd5536udc: remove multiple blank lines
checkpatch complains about multiple blank lines.
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index 3657a66..98b841d 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -62,7 +62,6 @@
/* udc specific */
#include "amd5536udc.h"
-
static void udc_tasklet_disconnect(unsigned long);
static void empty_req_queue(struct udc_ep *);
static void udc_setup_endpoints(struct udc *dev);
@@ -127,7 +126,6 @@ static DECLARE_COMPLETION(on_pollstall_exit);
static DECLARE_TASKLET(disconnect_tasklet, udc_tasklet_disconnect,
(unsigned long) &udc);
-
/* endpoint names used for print */
static const char ep0_string[] = "ep0in";
static const struct {
@@ -364,7 +362,6 @@ static void UDC_QUEUE_CNAK(struct udc_ep *ep, unsigned num)
cnak_pending = cnak_pending & (~(1 << (num)));
}
-
/* Enables endpoint, is called by gadget driver */
static int
udc_ep_enable(struct usb_ep *usbep, const struct usb_endpoint_descriptor *desc)
@@ -866,7 +863,6 @@ static int udc_create_dma_chain(
td->status = 0;
}
-
if (td)
td->bufptr = req->req.dma + i; /* assign buffer */
else
@@ -1000,7 +996,6 @@ static int prep_dma(struct udc_ep *ep, struct udc_request
*req, gfp_t gfp)
UDC_DMA_STP_STS_BS_HOST_READY,
UDC_DMA_STP_STS_BS);
-
/* clear NAK by writing CNAK */
if (ep->naking) {
tmp = readl(&ep->regs->ctl);
@@ -1728,7 +1723,6 @@ static void udc_tasklet_disconnect(unsigned long par)
ep_init(dev->regs,
&dev->ep[UDC_EP0IN_IX]);
-
if (!soft_reset_occured) {
/* init controller by soft reset */
udc_soft_reset(dev);
@@ -2120,7 +2114,6 @@ static void udc_ep0_set_rde(struct udc *dev)
}
}
-
/* Interrupt handler for data OUT traffic */
static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
{
@@ -2634,7 +2627,6 @@ __acquires(dev->lock)
} else
dev->waiting_zlp_ack_ep0in = 1;
-
/* clear NAK by writing CNAK in EP0_OUT */
if (!set) {
tmp = readl(&dev->ep[UDC_EP0OUT_IX].regs->ctl);
@@ -2809,7 +2801,6 @@ static irqreturn_t udc_control_in_isr(struct udc *dev)
return ret_val;
}
-
/* Interrupt handler for global device events */
static irqreturn_t udc_dev_isr(struct udc *dev, u32 dev_irq)
__releases(dev->lock)
@@ -2846,7 +2837,6 @@ __acquires(dev->lock)
/* ep ix in UDC CSR register space */
udc_csr_epix = ep->num;
-
/* OUT ep */
} else {
/* ep ix in UDC CSR register space */
@@ -2899,7 +2889,6 @@ __acquires(dev->lock)
/* ep ix in UDC CSR register space */
udc_csr_epix = ep->num;
-
/* OUT ep */
} else {
/* ep ix in UDC CSR register space */
@@ -3080,7 +3069,6 @@ static irqreturn_t udc_irq(int irq, void *pdev)
}
-
/* check for dev irq */
reg = readl(&dev->regs->irqsts);
if (reg) {
@@ -3089,7 +3077,6 @@ static irqreturn_t udc_irq(int irq, void *pdev)
ret_val |= udc_dev_isr(dev, reg);
}
-
spin_unlock(&dev->lock);
return ret_val;
}
--
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 11/16] usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain
Rearrange udc_free_dma_chain to remove the forward declaration.
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 53 ++---
1 file changed, 26 insertions(+), 27 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index 34edb9b..778a424 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -70,7 +70,6 @@ static void udc_setup_endpoints(struct udc *dev);
static void udc_soft_reset(struct udc *dev);
static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq);
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req);
static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
static void udc_pci_remove(struct pci_dev *pdev);
@@ -611,6 +610,32 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
return &req->req;
}
+/* frees pci pool descriptors of a DMA chain */
+static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
+{
+
+ int ret_val = 0;
+ struct udc_data_dma *td;
+ struct udc_data_dma *td_last = NULL;
+ unsigned int i;
+
+ DBG(dev, "free chain req = %p\n", req);
+
+ /* do not free first desc., will be done by free for request */
+ td_last = req->td_data;
+ td = phys_to_virt(td_last->next);
+
+ for (i = 1; i < req->chain_len; i++) {
+
+ pci_pool_free(dev->data_requests, td,
+ (dma_addr_t) td_last->next);
+ td_last = td;
+ td = phys_to_virt(td_last->next);
+ }
+
+ return ret_val;
+}
+
/* Frees request packet, called by gadget driver */
static void
udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq)
@@ -1028,32 +1053,6 @@ __acquires(ep->dev->lock)
ep->halted = halted;
}
-/* frees pci pool descriptors of a DMA chain */
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
-{
-
- int ret_val = 0;
- struct udc_data_dma *td;
- struct udc_data_dma *td_last = NULL;
- unsigned int i;
-
- DBG(dev, "free chain req = %p\n", req);
-
- /* do not free first desc., will be done by free for request */
- td_last = req->td_data;
- td = phys_to_virt(td_last->next);
-
- for (i = 1; i < req->chain_len; i++) {
-
- pci_pool_free(dev->data_requests, td,
- (dma_addr_t) td_last->next);
- td_last = td;
- td = phys_to_virt(td_last->next);
- }
-
- return ret_val;
-}
-
/* Iterates to the end of a DMA chain and returns last descriptor */
static struct udc_data_dma *udc_get_last_dma_desc(struct udc_request *req)
{
--
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 09/16] usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup
Rearrange the udc_remote_wakeup function to remove the forward declaration.
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 40 ++---
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index 01a6183..8b0ed74 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -1452,6 +1452,26 @@ static int udc_get_frame(struct usb_gadget *gadget)
return -EOPNOTSUPP;
}
+/* Initiates a remote wakeup */
+static int udc_remote_wakeup(struct udc *dev)
+{
+ unsigned long flags;
+ u32 tmp;
+
+ DBG(dev, "UDC initiates remote wakeup\n");
+
+ spin_lock_irqsave(&dev->lock, flags);
+
+ tmp = readl(&dev->regs->ctl);
+ tmp |= AMD_BIT(UDC_DEVCTL_RES);
+ writel(tmp, &dev->regs->ctl);
+ tmp &= AMD_CLEAR_BIT(UDC_DEVCTL_RES);
+ writel(tmp, &dev->regs->ctl);
+
+ spin_unlock_irqrestore(&dev->lock, flags);
+ return 0;
+}
+
/* Remote wakeup gadget interface */
static int udc_wakeup(struct usb_gadget *gadget)
{
@@ -3388,26 +3408,6 @@ err_enabledev:
return retval;
}
-/* Initiates a remote wakeup */
-static int udc_remote_wakeup(struct udc *dev)
-{
- unsigned long flags;
- u32 tmp;
-
- DBG(dev, "UDC initiates remote wakeup\n");
-
- spin_lock_irqsave(&dev->lock, flags);
-
- tmp = readl(&dev->regs->ctl);
- tmp |= AMD_BIT(UDC_DEVCTL_RES);
- writel(tmp, &dev->regs->ctl);
- tmp &= AMD_CLEAR_BIT(UDC_DEVCTL_RES);
- writel(tmp, &dev->regs->ctl);
-
- spin_unlock_irqrestore(&dev->lock, flags);
- return 0;
-}
-
/* PCI device parameters */
static const struct pci_device_id pci_id[] = {
{
--
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 10/16] usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain
Rearrange udc_create_dma_chain to remove the forward declaration.
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 241 ++--
1 file changed, 119 insertions(+), 122 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index 8b0ed74..34edb9b 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -71,9 +71,6 @@ static void udc_soft_reset(struct udc *dev);
static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq);
static int udc_free_dma_chain(struct udc *dev, struct udc_request *req);
-static int udc_create_dma_chain(struct udc_ep *ep, struct udc_request *req,
- unsigned long buf_len, gfp_t gfp_flags);
-static int udc_remote_wakeup(struct udc *dev);
static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
static void udc_pci_remove(struct pci_dev *pdev);
@@ -788,6 +785,125 @@ udc_rxfifo_read(struct udc_ep *ep, struct udc_request
*req)
return finished;
}
+/* Creates or re-inits a DMA chain */
+static int udc_create_dma_chain(
+ struct udc_ep *ep,
+ struct udc_request *req,
+ unsigned long buf_len, gfp_t gfp_flags
+)
+{
+ unsigned long bytes = req->req.length;
+ unsigned int i;
+ dma_addr_t dma_addr;
+ struct udc_data_dma *td = NULL;
+ struct udc_data_dma *last = NULL;
+ unsigned long txbytes;
+ unsigned create_new_chain = 0;
+ unsigned len;
+
+ VDBG(ep->dev, "udc_create_dma_chain: bytes=%ld buf_len=%ld\n",
+ bytes, buf_len);
+ dma_addr = DMA_DONT_USE;
+
+ /* unset L bit in first desc for OUT */
+ if (!ep->in)
+ req->td_data->status &= AMD_CLEAR_BIT(UDC_DMA_IN_STS_L);
+
+ /* alloc only new desc's if not already available */
+ len = req->req.length / ep->ep.maxpacket;
+ if (req->req.length % ep->ep.maxpacket)
+ len++;
+
+ if (len > req->chain_len) {
+ /* shorter chain already allocated before */
+ if (req->chain_len > 1)
+ udc_free_dma_chain(ep->dev, req);
+ req->chain_len = len;
+ create_new_chain = 1;
+ }
+
+ td = req->td_data;
+ /* gen. required number of descriptors and buffers */
+ for (i = buf_len; i < bytes; i += buf_len) {
+ /* create or determine next desc. */
+ if (create_new_chain) {
+
+ td = pci_pool_alloc(ep->dev->data_requests,
+ gfp_flags, &dma_addr);
+ if (!td)
+ return -ENOMEM;
+
+ td->status = 0;
+ } else if (i == buf_len) {
+ /* first td */
+ td = (struct udc_data_dma *) phys_to_virt(
+ req->td_data->next);
+ td->status = 0;
+ } else {
+ td = (struct udc_data_dma *) phys_to_virt(last->next);
+ td->status = 0;
+ }
+
+
+ if (td)
+ td->bufptr = req->req.dma + i; /* assign buffer */
+ else
+ break;
+
+ /* short packet ? */
+ if ((bytes - i) >= buf_len) {
+ txbytes = buf_len;
+ } else {
+ /* short packet */
+ txbytes = bytes - i;
+ }
+
+ /* link td and assign tx bytes */
+ if (i == buf_len) {
+ if (create_new_chain)
+ req->td_data->next = dma_addr;
+ /*
+ else
+ req->td_data->next = virt_to_phys(td);
+ */
+ /* write tx bytes */
+ if (ep->in) {
+ /* first desc */
+ req->td_data->status =
+ AMD_ADDBITS(req->td_data->status,
+ ep->ep.maxpacket,
+ UDC_DMA_IN_STS_TXBYTES);
+ /* second desc */
+ td->status = AMD_ADDBITS(td->status,
+ txbytes,
+ UDC_DMA_IN_STS_TXBYTES);
+ }
+ } else {
+ if (create_new_chain)
+ last->next = dma_addr;
+ /*
+ else
+ last
[PATCH 12/16] usb: gadget: amd5536udc: remove forward declaration of udc_pci_*
Remove the forward declarations of udc_pci_probe and udc_pci_remove. Signed-off-by: Sudip Mukherjee --- drivers/usb/gadget/udc/amd5536udc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c index 778a424..789441f 100644 --- a/drivers/usb/gadget/udc/amd5536udc.c +++ b/drivers/usb/gadget/udc/amd5536udc.c @@ -70,8 +70,6 @@ static void udc_setup_endpoints(struct udc *dev); static void udc_soft_reset(struct udc *dev); static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep); static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq); -static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id); -static void udc_pci_remove(struct pci_dev *pdev); /* description */ static const char mod_desc[] = UDC_MOD_DESCRIPTION; -- 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 05/16] usb: gadget: amd5536udc: use free_dma_pools
We have the function free_dma_pools() which frees all the dma pools. Use
it instead of calling all the functions separately. The if conditions
for data_requests and stp_requests are also not required here as this is
the remove function and we are here means probe has succeeded and dma
has been successfully allocated, so they cannot be NULL here.
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index 4edcfd4..3ae0bb8 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3131,20 +3131,7 @@ static void udc_pci_remove(struct pci_dev *pdev)
return;
/* dma pool cleanup */
- if (dev->data_requests)
- pci_pool_destroy(dev->data_requests);
-
- if (dev->stp_requests) {
- /* cleanup DMA desc's for ep0in */
- pci_pool_free(dev->stp_requests,
- dev->ep[UDC_EP0OUT_IX].td_stp,
- dev->ep[UDC_EP0OUT_IX].td_stp_dma);
- pci_pool_free(dev->stp_requests,
- dev->ep[UDC_EP0OUT_IX].td,
- dev->ep[UDC_EP0OUT_IX].td_phys);
-
- pci_pool_destroy(dev->stp_requests);
- }
+ free_dma_pools(dev);
/* reset controller */
writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), &dev->regs->cfg);
--
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 03/16] usb: gadget: amd5536udc: rewrite udc_pci_probe
A rewrite of udc_pci_probe() with proper error handling. We use here
free_dma_pools() in error handling.
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 52 +++--
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index 8b700de..8646f6c 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3244,17 +3244,13 @@ static int udc_pci_probe(
/* init */
dev = kzalloc(sizeof(struct udc), GFP_KERNEL);
- if (!dev) {
- retval = -ENOMEM;
- goto finished;
- }
+ if (!dev)
+ return -ENOMEM;
/* pci setup */
if (pci_enable_device(pdev) < 0) {
- kfree(dev);
- dev = NULL;
retval = -ENODEV;
- goto finished;
+ goto err_enabledev;
}
dev->active = 1;
@@ -3264,28 +3260,22 @@ static int udc_pci_probe(
if (!request_mem_region(resource, len, name)) {
dev_dbg(&pdev->dev, "pci device used already\n");
- kfree(dev);
- dev = NULL;
retval = -EBUSY;
- goto finished;
+ goto err_requestmem;
}
dev->mem_region = 1;
dev->virt_addr = ioremap_nocache(resource, len);
if (dev->virt_addr == NULL) {
dev_dbg(&pdev->dev, "start address cannot be mapped\n");
- kfree(dev);
- dev = NULL;
retval = -EFAULT;
- goto finished;
+ goto err_ioremap;
}
if (!pdev->irq) {
dev_err(&pdev->dev, "irq not set\n");
- kfree(dev);
- dev = NULL;
retval = -ENODEV;
- goto finished;
+ goto err_irqreq;
}
spin_lock_init(&dev->lock);
@@ -3301,10 +3291,8 @@ static int udc_pci_probe(
if (request_irq(pdev->irq, udc_irq, IRQF_SHARED, name, dev) != 0) {
dev_dbg(&pdev->dev, "request_irq(%d) fail\n", pdev->irq);
- kfree(dev);
- dev = NULL;
retval = -EBUSY;
- goto finished;
+ goto err_irqreq;
}
dev->irq_registered = 1;
@@ -3320,7 +3308,7 @@ static int udc_pci_probe(
if (use_dma) {
retval = init_dma_pools(dev);
if (retval != 0)
- goto finished;
+ goto err_dma;
}
dev->phys_addr = resource;
@@ -3328,12 +3316,26 @@ static int udc_pci_probe(
dev->pdev = pdev;
/* general probing */
- if (udc_probe(dev) == 0)
- return 0;
+ if (udc_probe(dev) != 0) {
+ retval = -ENODEV;
+ goto err_probe;
+ }
+ return 0;
-finished:
- if (dev)
- udc_pci_remove(pdev);
+err_probe:
+ if (use_dma)
+ free_dma_pools(dev);
+err_dma:
+ free_irq(pdev->irq, dev);
+err_irqreq:
+ iounmap(dev->virt_addr);
+err_ioremap:
+ release_mem_region(resource, len);
+err_requestmem:
+ pci_disable_device(pdev);
+err_enabledev:
+ kfree(dev);
+ dev = NULL;
return retval;
}
--
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 08/16] usb: gadget: amd5536udc: remove forward declaration of udc_probe
Rearrange the udc_probe function to remove the forward declarations.
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 133 ++--
1 file changed, 66 insertions(+), 67 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index b7753b2..01a6183 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -65,7 +65,6 @@
static void udc_tasklet_disconnect(unsigned long);
static void empty_req_queue(struct udc_ep *);
-static int udc_probe(struct udc *dev);
static void udc_basic_init(struct udc *dev);
static void udc_setup_endpoints(struct udc *dev);
static void udc_soft_reset(struct udc *dev);
@@ -3209,6 +3208,72 @@ err_create_dma_pool:
return retval;
}
+/* general probe */
+static int udc_probe(struct udc *dev)
+{
+ chartmp[128];
+ u32 reg;
+ int retval;
+
+ /* mark timer as not initialized */
+ udc_timer.data = 0;
+ udc_pollstall_timer.data = 0;
+
+ /* device struct setup */
+ dev->gadget.ops = &udc_ops;
+
+ dev_set_name(&dev->gadget.dev, "gadget");
+ dev->gadget.name = name;
+ dev->gadget.max_speed = USB_SPEED_HIGH;
+
+ /* init registers, interrupts, ... */
+ startup_registers(dev);
+
+ dev_info(&dev->pdev->dev, "%s\n", mod_desc);
+
+ snprintf(tmp, sizeof tmp, "%d", dev->irq);
+ dev_info(&dev->pdev->dev,
+ "irq %s, pci mem %08lx, chip rev %02x(Geode5536 %s)\n",
+ tmp, dev->phys_addr, dev->chiprev,
+ (dev->chiprev == UDC_HSA0_REV) ? "A0" : "B1");
+ strcpy(tmp, UDC_DRIVER_VERSION_STRING);
+ if (dev->chiprev == UDC_HSA0_REV) {
+ dev_err(&dev->pdev->dev, "chip revision is A0; too old\n");
+ retval = -ENODEV;
+ goto finished;
+ }
+ dev_info(&dev->pdev->dev,
+ "driver version: %s(for Geode5536 B1)\n", tmp);
+ udc = dev;
+
+ retval = usb_add_gadget_udc_release(&udc->pdev->dev, &dev->gadget,
+ gadget_release);
+ if (retval)
+ goto finished;
+
+ /* timer init */
+ init_timer(&udc_timer);
+ udc_timer.function = udc_timer_function;
+ udc_timer.data = 1;
+ /* timer pollstall init */
+ init_timer(&udc_pollstall_timer);
+ udc_pollstall_timer.function = udc_pollstall_timer_function;
+ udc_pollstall_timer.data = 1;
+
+ /* set SD */
+ reg = readl(&dev->regs->ctl);
+ reg |= AMD_BIT(UDC_DEVCTL_SD);
+ writel(reg, &dev->regs->ctl);
+
+ /* print dev register info */
+ print_regs(dev);
+
+ return 0;
+
+finished:
+ return retval;
+}
+
/* Called by pci bus driver to init pci context */
static int udc_pci_probe(
struct pci_dev *pdev,
@@ -3323,72 +3388,6 @@ err_enabledev:
return retval;
}
-/* general probe */
-static int udc_probe(struct udc *dev)
-{
- chartmp[128];
- u32 reg;
- int retval;
-
- /* mark timer as not initialized */
- udc_timer.data = 0;
- udc_pollstall_timer.data = 0;
-
- /* device struct setup */
- dev->gadget.ops = &udc_ops;
-
- dev_set_name(&dev->gadget.dev, "gadget");
- dev->gadget.name = name;
- dev->gadget.max_speed = USB_SPEED_HIGH;
-
- /* init registers, interrupts, ... */
- startup_registers(dev);
-
- dev_info(&dev->pdev->dev, "%s\n", mod_desc);
-
- snprintf(tmp, sizeof tmp, "%d", dev->irq);
- dev_info(&dev->pdev->dev,
- "irq %s, pci mem %08lx, chip rev %02x(Geode5536 %s)\n",
- tmp, dev->phys_addr, dev->chiprev,
- (dev->chiprev == UDC_HSA0_REV) ? "A0" : "B1");
- strcpy(tmp, UDC_DRIVER_VERSION_STRING);
- if (dev->chiprev == UDC_HSA0_REV) {
- dev_err(&dev->pdev->dev, "chip revision is A0; too old\n");
- retval = -ENODEV;
- goto finished;
- }
- dev_info(&dev->pdev->dev,
- "driver version: %s(for Geode5536 B1)\n", tmp);
- udc = dev;
-
- retval = usb_add_gadget_udc_release(&udc->pdev->dev, &dev->gadget,
- gadget_release);
- if (retval)
- goto finished;
-
- /* timer init */
- init_timer(&udc_timer);
- udc_timer.function = udc_timer_function;
- udc_timer.data = 1;
- /* timer pollstall init */
- init_timer(&udc_pollstall_timer);
- udc_pollstall_timer.function = udc_pollstall_timer_function;
- udc_pollstall_timer.data = 1;
-
- /* set SD */
- reg = readl(&dev->regs->ctl);
- reg |= AMD_BIT(UDC_DEVCTL_SD);
- writel(reg, &dev->regs->ctl);
-
- /* print dev register info */
- print_regs(dev);
-
- return 0;
-
-finished:
- return retval;
-}
-
/* Initiates a re
[PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks
This amd5536udc was a complete mess. The major problems that i could find are: 1) if udc_pci_probe() fails in any stage then it just calls the udc_pci_remove() to handle error. And udc_pci_remove() works with struct udc *dev which we get from pci_get_drvdata(pdev). But we do the pci_set_drvdata(pdev, dev) almost at the end of probe. So basically incase of error we are handling the error by dereferencing a NULL pointer. 2) udc_pci_remove() does a BUG_ON(dev->driver != NULL) and dev->driver will be set only if probe is success. So that means if probe fails then probe will call udc_pci_remove() for error handling and udc_pci_remove() will inturn halts the kernel by calling BUG(). And apart from these numerous memory leaks and not releasing of resources. Here comes a rewrite of few of the functions in an attempt to fix these. regards sudip Sudip Mukherjee (16): usb: gadget: amd5536udc: introduce free_dma_pools usb: gadget: amd5536udc: rewrite init_dma_pools usb: gadget: amd5536udc: rewrite udc_pci_probe usb: gadget: amd5536udc: use WARN_ON usb: gadget: amd5536udc: use free_dma_pools usb: gadget: amd5536udc: remove unnecessary conditions usb: gadget: amd5536udc: unmap virt_addr usb: gadget: amd5536udc: remove forward declaration of udc_probe usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain usb: gadget: amd5536udc: remove forward declaration of udc_pci_* usb: gadget: amd5536udc: remove forward declaration of udc_basic_init usb: gadget: amd5536udc: NULL comparison usb: gadget: amd5536udc: remove multiple blank lines usb: gadget: amd5536udc: match alignment drivers/usb/gadget/udc/amd5536udc.c | 797 ++-- 1 file changed, 390 insertions(+), 407 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 06/16] usb: gadget: amd5536udc: remove unnecessary conditions
The condition checking for irq_registered, regs, mem_region and active are not required as this is the remove function. And we are in the remove means that probe was successful and they can never be NULL at this point of code. It was required in the original code as the remove function was part of the error handler of probe function. Signed-off-by: Sudip Mukherjee --- drivers/usb/gadget/udc/amd5536udc.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c index 3ae0bb8..e2f6128 100644 --- a/drivers/usb/gadget/udc/amd5536udc.c +++ b/drivers/usb/gadget/udc/amd5536udc.c @@ -3135,15 +3135,11 @@ static void udc_pci_remove(struct pci_dev *pdev) /* reset controller */ writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), &dev->regs->cfg); - if (dev->irq_registered) - free_irq(pdev->irq, dev); - if (dev->regs) - iounmap(dev->regs); - if (dev->mem_region) - release_mem_region(pci_resource_start(pdev, 0), + free_irq(pdev->irq, dev); + iounmap(dev->regs); + release_mem_region(pci_resource_start(pdev, 0), pci_resource_len(pdev, 0)); - if (dev->active) - pci_disable_device(pdev); + pci_disable_device(pdev); udc_remove(dev); } -- 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 07/16] usb: gadget: amd5536udc: unmap virt_addr
We have actually ioremapped dev->virt_addr and dev->regs is dev->virt_addr + UDC_DEVCFG_ADDR so while unmapping we should unmap dev->virt_addr. Signed-off-by: Sudip Mukherjee --- drivers/usb/gadget/udc/amd5536udc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c index e2f6128..b7753b2 100644 --- a/drivers/usb/gadget/udc/amd5536udc.c +++ b/drivers/usb/gadget/udc/amd5536udc.c @@ -3136,7 +3136,7 @@ static void udc_pci_remove(struct pci_dev *pdev) /* reset controller */ writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), &dev->regs->cfg); free_irq(pdev->irq, dev); - iounmap(dev->regs); + iounmap(dev->virt_addr); release_mem_region(pci_resource_start(pdev, 0), pci_resource_len(pdev, 0)); pci_disable_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 02/16] usb: gadget: amd5536udc: rewrite init_dma_pools
A rewrite of init_dma_pools() with proper error handling.
Signed-off-by: Sudip Mukherjee
---
drivers/usb/gadget/udc/amd5536udc.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index 384b824..8b700de 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3180,8 +3180,7 @@ static int init_dma_pools(struct udc *dev)
sizeof(struct udc_data_dma), 0, 0);
if (!dev->data_requests) {
DBG(dev, "can't get request data pool\n");
- retval = -ENOMEM;
- goto finished;
+ return -ENOMEM;
}
/* EP0 in dma regs = dev control regs */
@@ -3193,14 +3192,14 @@ static int init_dma_pools(struct udc *dev)
if (!dev->stp_requests) {
DBG(dev, "can't get stp request pool\n");
retval = -ENOMEM;
- goto finished;
+ goto err_create_dma_pool;
}
/* setup */
td_stp = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
&dev->ep[UDC_EP0OUT_IX].td_stp_dma);
if (td_stp == NULL) {
retval = -ENOMEM;
- goto finished;
+ goto err_alloc_dma;
}
dev->ep[UDC_EP0OUT_IX].td_stp = td_stp;
@@ -3209,12 +3208,20 @@ static int init_dma_pools(struct udc *dev)
&dev->ep[UDC_EP0OUT_IX].td_phys);
if (td_data == NULL) {
retval = -ENOMEM;
- goto finished;
+ goto err_alloc_phys;
}
dev->ep[UDC_EP0OUT_IX].td = td_data;
return 0;
-finished:
+err_alloc_phys:
+ dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td_stp,
+ dev->ep[UDC_EP0OUT_IX].td_stp_dma);
+err_alloc_dma:
+ dma_pool_destroy(dev->stp_requests);
+ dev->stp_requests = NULL;
+err_create_dma_pool:
+ dma_pool_destroy(dev->data_requests);
+ dev->data_requests = NULL;
return retval;
}
--
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 04/16] usb: gadget: amd5536udc: use WARN_ON
Use WARN_ON() instead of halting the kernel with BUG_ON(). Signed-off-by: Sudip Mukherjee --- drivers/usb/gadget/udc/amd5536udc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c index 8646f6c..4edcfd4 100644 --- a/drivers/usb/gadget/udc/amd5536udc.c +++ b/drivers/usb/gadget/udc/amd5536udc.c @@ -3127,7 +3127,8 @@ static void udc_pci_remove(struct pci_dev *pdev) usb_del_gadget_udc(&udc->gadget); /* gadget driver must not be registered */ - BUG_ON(dev->driver != NULL); + if (WARN_ON(dev->driver != NULL)) + return; /* dma pool cleanup */ if (dev->data_requests) -- 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/4] usb: gadget: dummy_hcd: fix rescan logic for transfer
transfer() schedules a rescan for transfers larger than
maxpacket, which is wrong for transfers that are multiples
of maxpacket.
Rewrite to fix and clarify packet multiple / remainder
transfer logic.
Signed-off-by: Igor Kotrasinski
---
drivers/usb/gadget/udc/dummy_hcd.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index da38475..3fdcbda 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1288,6 +1288,7 @@ top:
/* if there's no request queued, the device is NAKing; return */
list_for_each_entry(req, &ep->queue, queue) {
unsignedhost_len, dev_len, len;
+ unsignedrem;
int is_short, to_host;
int rescan = 0;
@@ -1320,12 +1321,17 @@ top:
if (len == 0)
break;
- /* use an extra pass for the final short packet */
- if (len > ep->ep.maxpacket) {
- rescan = 1;
- len -= (len % ep->ep.maxpacket);
+ /* send multiple of maxpacket first, then remainder */
+ rem = (len % ep->ep.maxpacket);
+ if (len - rem > 0) {
+ is_short = 0;
+ len -= rem;
+ if (rem > 0)
+ rescan = 1;
+ } else { /* rem > 0 */
+ is_short = 1;
+ len = rem;
}
- is_short = (len % ep->ep.maxpacket) != 0;
len = dummy_perform_transfer(urb, req, len);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/4] usb: gadget: dummy_hcd: emulate sending zlp in packet logic
currently, when a zlp flag is set and an urb/usb_request
buffer is filled without a short packet, transfer() leaves
its status at -EINPROGRESS and does not rescan for short
packet.
In a scenario where ep.maxpacket bytes are copied,
URB_ZERO_PACKET is set, urb buffer is filled and usb_request
buffer is not, transfer() returns with an urb with
-EINPROGRESS status, which dummy_hcd treats as incomplete
transfer.
Check for zlp and rescan appropriately.
Signed-off-by: Igor Kotrasinski
---
drivers/usb/gadget/udc/dummy_hcd.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index 181112c..df11021 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1364,15 +1364,24 @@ top:
req->req.status = 0;
}
- /* many requests terminate without a short packet */
+ /*
+* many requests terminate without a short packet.
+* send a zlp if demanded by flags.
+*/
} else {
- if (req->req.length == req->req.actual
- && !req->req.zero)
- req->req.status = 0;
- if (urb->transfer_buffer_length == urb->actual_length
- && !(urb->transfer_flags
- & URB_ZERO_PACKET))
- *status = 0;
+ if (req->req.length == req->req.actual) {
+ if (req->req.zero && to_host)
+ rescan = 1;
+ else
+ req->req.status = 0;
+ }
+ if (urb->transfer_buffer_length == urb->actual_length) {
+ if (urb->transfer_flags & URB_ZERO_PACKET &&
+ !to_host)
+ rescan = 1;
+ else
+ *status = 0;
+ }
}
/* device side completion --> continuable */
--
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/4] usb: gadget: dummy_hcd: fix unneeded else-if condition
Signed-off-by: Igor Kotrasinski
---
drivers/usb/gadget/udc/dummy_hcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index df11021..da38475 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1356,7 +1356,7 @@ top:
*status = -EOVERFLOW;
else
*status = 0;
- } else if (!to_host) {
+ } else {
*status = 0;
if (host_len > dev_len)
req->req.status = -EOVERFLOW;
--
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 4/4] usb: gadget: dummy_hcd: in transfer(), return data sent, not limit
dummy_timer uses transfer() to update transfer limit. However,
limit passed to dummy_timer changes depending on transfer type,
so the actual limit is overwritten.
This can cause unpredictably slow / fast bulk transfers when
coupled with control / interrupt transfers.
Fix by returning actual amount of data sent in transfer() and
substracting from total.
Signed-off-by: Igor Kotrasinski
---
drivers/usb/gadget/udc/dummy_hcd.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index 3fdcbda..ec22aff 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1283,6 +1283,7 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb
*urb,
{
struct dummy*dum = dum_hcd->dum;
struct dummy_request*req;
+ int sent = 0;
top:
/* if there's no request queued, the device is NAKing; return */
@@ -1340,6 +1341,7 @@ top:
req->req.status = len;
} else {
limit -= len;
+ sent += len;
urb->actual_length += len;
req->req.actual += len;
}
@@ -1410,7 +1412,7 @@ top:
if (rescan)
goto top;
}
- return limit;
+ return sent;
}
static int periodic_bytes(struct dummy *dum, struct dummy_ep *ep)
@@ -1840,7 +1842,7 @@ restart:
default:
treat_control_like_bulk:
ep->last_io = jiffies;
- total = transfer(dum_hcd, urb, ep, limit, &status);
+ total -= transfer(dum_hcd, urb, ep, limit, &status);
break;
}
--
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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hi, On Mon, Sep 14, 2015 at 9:59 AM, Hans de Goede wrote: > Hi, > > > On 14-09-15 16:44, Bin Liu wrote: >> >> Hi, >> >> On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede >> wrote: >>> >>> Hi, >>> >>> On 10-09-15 20:30, Maxime Ripard wrote: On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote: > > > Hi, > > On 04-09-15 08:43, Olliver Schinagl wrote: >> >> >> Hey Hans, >> >> On 07-08-15 10:45, Olliver Schinagl wrote: >>> >>> >>> If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det gpio settings from the usb_phy node in the dts, as the sun4i phy code detects host vs otg mode by checking for the presence of these. >>> >>> >>> Yes, this fixes it and makes it work. Thanks. >>> >> I've been going back to this and am wondering if this is something I >> can >> look into to fix properly? E.g. if the dts sets dr_mode = host, can we >> simply ignore the pins and treat them as unset? > > > > AFAIK you cannot unset something in dts. The only solution I > can comeup with is to add a dr_mode argument to the phy like > we already have for the otg controller itself. > > This is something which we likely need to do anyways to add > support for peripheral only mode, which we seem to need for > some "hdmi sticks". I haven't really followed the rest of the discussion, so sorry if you already talked about that, but why can't you just set the dr_mode to peripheral in such a case? >>> >>> >>> >>> This is about the usbphy code not the musb-controller code, which are >>> 2 different dts nodes, atm only the musb-controller node has a >>> dr_mode property, and the phy code decides between host-only >>> and otg mode based on whether an id pin is assigned or not. >>> >>> My proposal is to get rid of the id-pin hack to determine the mode >>> and add a dr_mode property to the usbphy dts node. >> >> >> I would try to avoid adding dr_mode in the usbphy node if possible, >> since it is already specified in the controller node. >> >> Since the phy node is referenced in the controller node, is it >> possible that the phy driver looks up the controller of properties to >> find its dr_mode setting? > > > That is possible, but it very much goes against how devicetree normally > works, where every node is more or less a standalone unit which > contains enough info for the associated driver to do its work. I guess you are right, but duplicating dr_mode would cause more trouble for end user. Felipe, could you please give your comments on this issue? I have to do the similar change for musb_dsps. > > Currently there is no link from the phy node to the controller node > (only the other way around) and adding such a link requires more code > then simple having a duplicate dr_mode. I guess I did not make my previous comment clearly, we don't need to add the link from the phy node to the controller node. We don't need to change the current dts, just in the phy driver to look up dr_mode of the controller node, if possible. Regards, -Bin. > > Regards, > > Hans > -- 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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hi, On 14-09-15 18:58, Bin Liu wrote: Hi, On Mon, Sep 14, 2015 at 9:59 AM, Hans de Goede wrote: Hi, On 14-09-15 16:44, Bin Liu wrote: Hi, On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede wrote: Hi, On 10-09-15 20:30, Maxime Ripard wrote: On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote: Hi, On 04-09-15 08:43, Olliver Schinagl wrote: Hey Hans, On 07-08-15 10:45, Olliver Schinagl wrote: If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det gpio settings from the usb_phy node in the dts, as the sun4i phy code detects host vs otg mode by checking for the presence of these. Yes, this fixes it and makes it work. Thanks. I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset? AFAIK you cannot unset something in dts. The only solution I can comeup with is to add a dr_mode argument to the phy like we already have for the otg controller itself. This is something which we likely need to do anyways to add support for peripheral only mode, which we seem to need for some "hdmi sticks". I haven't really followed the rest of the discussion, so sorry if you already talked about that, but why can't you just set the dr_mode to peripheral in such a case? This is about the usbphy code not the musb-controller code, which are 2 different dts nodes, atm only the musb-controller node has a dr_mode property, and the phy code decides between host-only and otg mode based on whether an id pin is assigned or not. My proposal is to get rid of the id-pin hack to determine the mode and add a dr_mode property to the usbphy dts node. I would try to avoid adding dr_mode in the usbphy node if possible, since it is already specified in the controller node. Since the phy node is referenced in the controller node, is it possible that the phy driver looks up the controller of properties to find its dr_mode setting? That is possible, but it very much goes against how devicetree normally works, where every node is more or less a standalone unit which contains enough info for the associated driver to do its work. I guess you are right, but duplicating dr_mode would cause more trouble for end user. The user already needs to set up regulators, vbus-det and id-det for otg mode in the usbphy node. Although there are 2 nodes in dts / 2 separate hardware blocks involved in reality the 2 are closely related and the user already must take care to have the settings match. Besides that writing dts files is not something which end users do, so a normal user will never see this. Felipe, could you please give your comments on this issue? I have to do the similar change for musb_dsps. Currently there is no link from the phy node to the controller node (only the other way around) and adding such a link requires more code then simple having a duplicate dr_mode. I guess I did not make my previous comment clearly, we don't need to add the link from the phy node to the controller node. We don't need to change the current dts, just in the phy driver to look up dr_mode of the controller node, if possible. And how does the phy code now where the controller node lives without having a link to it in its node? Hardcode the full path to the controller node ? That is both ugly and error prone. Regards, Hans -- 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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hi, On Mon, Sep 14, 2015 at 12:08 PM, Hans de Goede wrote: > Hi, > > > On 14-09-15 18:58, Bin Liu wrote: >> >> Hi, >> >> On Mon, Sep 14, 2015 at 9:59 AM, Hans de Goede >> wrote: >>> >>> Hi, >>> >>> >>> On 14-09-15 16:44, Bin Liu wrote: Hi, On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede wrote: > > > Hi, > > On 10-09-15 20:30, Maxime Ripard wrote: >> >> >> >> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote: >>> >>> >>> >>> Hi, >>> >>> On 04-09-15 08:43, Olliver Schinagl wrote: Hey Hans, On 07-08-15 10:45, Olliver Schinagl wrote: > > > > >> >> >> >> If you change the dr_mode to host then you _must_ also remove any >> id_det and vbus_det >> gpio settings from the usb_phy node in the dts, as the sun4i phy >> code >> detects >> host vs otg mode by checking for the presence of these. > > > > Yes, this fixes it and makes it work. Thanks. > I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset? >>> >>> >>> >>> >>> AFAIK you cannot unset something in dts. The only solution I >>> can comeup with is to add a dr_mode argument to the phy like >>> we already have for the otg controller itself. >>> >>> This is something which we likely need to do anyways to add >>> support for peripheral only mode, which we seem to need for >>> some "hdmi sticks". >> >> >> >> >> I haven't really followed the rest of the discussion, so sorry if you >> already talked about that, but why can't you just set the dr_mode to >> peripheral in such a case? > > > > > This is about the usbphy code not the musb-controller code, which are > 2 different dts nodes, atm only the musb-controller node has a > dr_mode property, and the phy code decides between host-only > and otg mode based on whether an id pin is assigned or not. > > My proposal is to get rid of the id-pin hack to determine the mode > and add a dr_mode property to the usbphy dts node. I would try to avoid adding dr_mode in the usbphy node if possible, since it is already specified in the controller node. Since the phy node is referenced in the controller node, is it possible that the phy driver looks up the controller of properties to find its dr_mode setting? >>> >>> >>> >>> That is possible, but it very much goes against how devicetree normally >>> works, where every node is more or less a standalone unit which >>> contains enough info for the associated driver to do its work. >> >> >> I guess you are right, but duplicating dr_mode would cause more >> trouble for end user. > > > The user already needs to set up regulators, vbus-det and id-det > for otg mode in the usbphy node. Although there are 2 nodes in dts / > 2 separate hardware blocks involved in reality the 2 are closely > related and the user already must take care to have the settings match. > > Besides that writing dts files is not something which end users do, > so a normal user will never see this. > >> Felipe, could you please give your comments on this issue? I have to >> do the similar change for musb_dsps. >> >>> >>> Currently there is no link from the phy node to the controller node >>> (only the other way around) and adding such a link requires more code >>> then simple having a duplicate dr_mode. >> >> >> I guess I did not make my previous comment clearly, we don't need to >> add the link from the phy node to the controller node. We don't need >> to change the current dts, just in the phy driver to look up dr_mode >> of the controller node, if possible. > > > And how does the phy code now where the controller node lives without > having a link to it in its node? Hardcode the full path to the > controller node ? That is both ugly and error prone. This is my first time looking at dts handling in drivers, so I might be completely wrong, but I am thinking that since the controller node links to the phy node, so the controller node is the parent of the phy node, so if there is an of api can look it up? Regards, -Bin. > > Regards, > > Hans -- 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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hi, On 14-09-15 19:14, Bin Liu wrote: Hi, On Mon, Sep 14, 2015 at 12:08 PM, Hans de Goede wrote: Hi, On 14-09-15 18:58, Bin Liu wrote: Hi, On Mon, Sep 14, 2015 at 9:59 AM, Hans de Goede wrote: Hi, On 14-09-15 16:44, Bin Liu wrote: Hi, On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede wrote: Hi, On 10-09-15 20:30, Maxime Ripard wrote: On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote: Hi, On 04-09-15 08:43, Olliver Schinagl wrote: Hey Hans, On 07-08-15 10:45, Olliver Schinagl wrote: If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det gpio settings from the usb_phy node in the dts, as the sun4i phy code detects host vs otg mode by checking for the presence of these. Yes, this fixes it and makes it work. Thanks. I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset? AFAIK you cannot unset something in dts. The only solution I can comeup with is to add a dr_mode argument to the phy like we already have for the otg controller itself. This is something which we likely need to do anyways to add support for peripheral only mode, which we seem to need for some "hdmi sticks". I haven't really followed the rest of the discussion, so sorry if you already talked about that, but why can't you just set the dr_mode to peripheral in such a case? This is about the usbphy code not the musb-controller code, which are 2 different dts nodes, atm only the musb-controller node has a dr_mode property, and the phy code decides between host-only and otg mode based on whether an id pin is assigned or not. My proposal is to get rid of the id-pin hack to determine the mode and add a dr_mode property to the usbphy dts node. I would try to avoid adding dr_mode in the usbphy node if possible, since it is already specified in the controller node. Since the phy node is referenced in the controller node, is it possible that the phy driver looks up the controller of properties to find its dr_mode setting? That is possible, but it very much goes against how devicetree normally works, where every node is more or less a standalone unit which contains enough info for the associated driver to do its work. I guess you are right, but duplicating dr_mode would cause more trouble for end user. The user already needs to set up regulators, vbus-det and id-det for otg mode in the usbphy node. Although there are 2 nodes in dts / 2 separate hardware blocks involved in reality the 2 are closely related and the user already must take care to have the settings match. Besides that writing dts files is not something which end users do, so a normal user will never see this. Felipe, could you please give your comments on this issue? I have to do the similar change for musb_dsps. Currently there is no link from the phy node to the controller node (only the other way around) and adding such a link requires more code then simple having a duplicate dr_mode. I guess I did not make my previous comment clearly, we don't need to add the link from the phy node to the controller node. We don't need to change the current dts, just in the phy driver to look up dr_mode of the controller node, if possible. And how does the phy code now where the controller node lives without having a link to it in its node? Hardcode the full path to the controller node ? That is both ugly and error prone. This is my first time looking at dts handling in drivers, so I might be completely wrong, but I am thinking that since the controller node links to the phy node, so the controller node is the parent of the phy node, so if there is an of api can look it up? If the phy is a child of the controller, then yes this would work, but in the case of sunxi the phy is a built-in mmio mapped peripheral just like the controller, so they sit at the same level and have no parent child relation. Regards, Hans -- 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: similar files: fusbh200-hcd.c and fotg210-hcd.c
On Mon, Sep 14, 2015 at 5:01 PM, Felipe Balbi wrote:
> On Sat, Sep 12, 2015 at 03:14:50PM +0200, Peter Senna Tschudin wrote:
>> >> Should these files be consolidated? And if so how?
>> > if you can find an easy way, that would be a very, very welcome patch.
>>
>> Is the ideal solution to consolidate both fusbh200-hcd.c and
>> fotg210-hcd.c in a single module? If this is the case, how to detect
>> at run time which version of the hw is present? Both are registered as
>
> does it matter ? If they work the same way, why does it matter which
> one's running?
I may be missing something simple, but based on a 2 page product
brief, fotg210 has more resources like memory. So even if the .c files
are _very_ similar, there are some configuration parameters that
differ, for example:
fusbh200.h:
#define BMCSR_VBUS_OFF (1<<4)
#define BMCSR_INT_POLARITY (1<<3)
fotg210.h:
#define OTGCSR_A_BUS_DROP (1 << 5)
#define OTGCSR_A_BUS_REQ (1 << 4)
which are used by {fusbh200,fotg210}_init:
fusbh200-hcd.c:
static void fusbh200_init(struct fusbh200_hcd *fusbh200)
{
u32 reg;
reg = fusbh200_readl(fusbh200, &fusbh200->regs->bmcsr);
reg |= BMCSR_INT_POLARITY;
reg &= ~BMCSR_VBUS_OFF;
fusbh200_writel(fusbh200, reg, &fusbh200->regs->bmcsr);
reg = fusbh200_readl(fusbh200, &fusbh200->regs->bmier);
fusbh200_writel(fusbh200, reg | BMIER_OVC_EN | BMIER_VBUS_ERR_EN,
&fusbh200->regs->bmier);
}
fotg210-hcd.c:
static void fotg210_init(struct fotg210_hcd *fotg210)
{
u32 value;
iowrite32(GMIR_MDEV_INT | GMIR_MOTG_INT | GMIR_INT_POLARITY,
&fotg210->regs->gmir);
value = ioread32(&fotg210->regs->otgcsr);
value &= ~OTGCSR_A_BUS_DROP;
value |= OTGCSR_A_BUS_REQ;
iowrite32(value, &fotg210->regs->otgcsr);
}
Then:
fusbh200.h:
#define BMCSR_HOST_SPD_TYP (3<<9)
static inline unsigned int
fusbh200_get_speed(struct fusbh200_hcd *fusbh200, unsigned int portsc)
{
return (readl(&fusbh200->regs->bmcsr)
& BMCSR_HOST_SPD_TYP) >> 9;
}
fotg210.h:
#define OTGCSR_HOST_SPD_TYP (3 << 22)
static inline unsigned int
fotg210_get_speed(struct fotg210_hcd *fotg210, unsigned int portsc)
{
return (readl(&fotg210->regs->otgcsr)
& OTGCSR_HOST_SPD_TYP) >> 22;
}
So my concern is to have a way to identify which is the device version
to use the right parameters. I think that the BMCSR_HOST_SPD_TYP vs
OTGCSR_HOST_SPD_TYP can be solved, but I'm not sure about the
initialization. Ideas?
>
>> platform devices and I could not find an obvious way to detect the
>> model at run time. I could successfully load fusbh200-hcd on my fedora
>
> is there a revision register ? Or you may use different platform_device
> names with platform_device_id table.
I don't know about revision registers. That would be good. I could not
find complete datasheets, only a 2 page product brief, no registry
information there.
>
>> notebook (hp elitebook 840), and on a VM, even if neither has the hw
>> ($ sudo modprobe fusbh200-hcd). The module loads with the warning
>> "fusbh200_hcd should always be loaded before uhci_hcd and ohci_hcd,
>> not after". On another workstation running ubuntu, I could load both
>> modules at the same time, producing the same warning for each module.
>> Should the module load if the device is not present?
>>
>> Other solution for consolidation would be to create a common_code.c,
>> keeping both fusbh200-hcd.c and fotg210-hcd.c only with the code that
>> differ. Is this better than what is there now?
>>
>> Other ideas?
>
> just combine them :-p Use platform_device_id to differentiate.
I'm afraid the combined version will use the correct parameters for
only one of the two. But I may be missing something simple. I did a
diff between the two files after removing white space differences, and
after replacing fusbh200 by fotg210 on the fusbh200 driver. The files
are very similar. See: http://pastebin.com/ZRY3xePv
>
> --
> balbi
--
Peter
--
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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hi, On Mon, Sep 14, 2015 at 12:25 PM, Hans de Goede wrote: > Hi, > > > On 14-09-15 19:14, Bin Liu wrote: >> >> Hi, >> >> On Mon, Sep 14, 2015 at 12:08 PM, Hans de Goede >> wrote: >>> >>> Hi, >>> >>> >>> On 14-09-15 18:58, Bin Liu wrote: Hi, On Mon, Sep 14, 2015 at 9:59 AM, Hans de Goede wrote: > > > Hi, > > > On 14-09-15 16:44, Bin Liu wrote: >> >> >> >> Hi, >> >> On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede >> wrote: >>> >>> >>> >>> Hi, >>> >>> On 10-09-15 20:30, Maxime Ripard wrote: On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote: > > > > > Hi, > > On 04-09-15 08:43, Olliver Schinagl wrote: >> >> >> >> >> Hey Hans, >> >> On 07-08-15 10:45, Olliver Schinagl wrote: >>> >>> >>> >>> >>> If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det gpio settings from the usb_phy node in the dts, as the sun4i phy code detects host vs otg mode by checking for the presence of these. >>> >>> >>> >>> >>> Yes, this fixes it and makes it work. Thanks. >>> >> I've been going back to this and am wondering if this is something >> I >> can >> look into to fix properly? E.g. if the dts sets dr_mode = host, >> can >> we >> simply ignore the pins and treat them as unset? > > > > > > AFAIK you cannot unset something in dts. The only solution I > can comeup with is to add a dr_mode argument to the phy like > we already have for the otg controller itself. > > This is something which we likely need to do anyways to add > support for peripheral only mode, which we seem to need for > some "hdmi sticks". I haven't really followed the rest of the discussion, so sorry if you already talked about that, but why can't you just set the dr_mode to peripheral in such a case? >>> >>> >>> >>> >>> >>> This is about the usbphy code not the musb-controller code, which are >>> 2 different dts nodes, atm only the musb-controller node has a >>> dr_mode property, and the phy code decides between host-only >>> and otg mode based on whether an id pin is assigned or not. >>> >>> My proposal is to get rid of the id-pin hack to determine the mode >>> and add a dr_mode property to the usbphy dts node. >> >> >> >> >> I would try to avoid adding dr_mode in the usbphy node if possible, >> since it is already specified in the controller node. >> >> Since the phy node is referenced in the controller node, is it >> possible that the phy driver looks up the controller of properties to >> find its dr_mode setting? > > > > > That is possible, but it very much goes against how devicetree normally > works, where every node is more or less a standalone unit which > contains enough info for the associated driver to do its work. I guess you are right, but duplicating dr_mode would cause more trouble for end user. >>> >>> >>> >>> The user already needs to set up regulators, vbus-det and id-det >>> for otg mode in the usbphy node. Although there are 2 nodes in dts / >>> 2 separate hardware blocks involved in reality the 2 are closely >>> related and the user already must take care to have the settings match. >>> >>> Besides that writing dts files is not something which end users do, >>> so a normal user will never see this. >>> Felipe, could you please give your comments on this issue? I have to do the similar change for musb_dsps. > > Currently there is no link from the phy node to the controller node > (only the other way around) and adding such a link requires more code > then simple having a duplicate dr_mode. I guess I did not make my previous comment clearly, we don't need to add the link from the phy node to the controller node. We don't need to change the current dts, just in the phy driver to look up dr_mode of the controller node, if possible. >>> >>> >>> >>> And how does the phy code now where the controller node lives without >>> having a link to it in its node? Hardcode the full path to the >>> controller node ? That is both ugly and error prone. >> >> >> This is my first time looking at dts handling in drivers, so I might >> be completely wrong, but I am thinking that since the controller node
Re: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hi,
On 14-09-15 19:53, Bin Liu wrote:
This is my first time looking at dts handling in drivers, so I might
be completely wrong, but I am thinking that since the controller node
links to the phy node, so the controller node is the parent of the phy
node, so if there is an of api can look it up?
If the phy is a child of the controller, then yes this would work,
but in the case of sunxi the phy is a built-in mmio mapped peripheral
just like the controller, so they sit at the same level and have no
parent child relation.
musb_dsps dts is the same.
sun8i-a33.dtsi:
soc {
usb_otg: {
phys = <&usbphy 0>;
}
usbphy: {
}
}
As in the example above, usb_otg node refers to usbphy node, so I am
wondering if there is an of api to look up the usb_otg properties in
the usbphy driver.
That would boil down to hardcoding the node name / path compatible
which is not acceptable from a devicetree pov.
Really having to duplicate the dr_mode is not that bad / such big
a deal.
Regards,
Hans
--
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/4] usb: gadget: dummy_hcd: fix rescan logic for transfer
On Mon, 14 Sep 2015, Igor Kotrasinski wrote:
> transfer() schedules a rescan for transfers larger than
> maxpacket, which is wrong for transfers that are multiples
> of maxpacket.
>
> Rewrite to fix and clarify packet multiple / remainder
> transfer logic.
>
> Signed-off-by: Igor Kotrasinski
> ---
> drivers/usb/gadget/udc/dummy_hcd.c | 16 +++-
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
> b/drivers/usb/gadget/udc/dummy_hcd.c
> index da38475..3fdcbda 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -1288,6 +1288,7 @@ top:
> /* if there's no request queued, the device is NAKing; return */
> list_for_each_entry(req, &ep->queue, queue) {
> unsignedhost_len, dev_len, len;
> + unsignedrem;
> int is_short, to_host;
> int rescan = 0;
>
> @@ -1320,12 +1321,17 @@ top:
> if (len == 0)
> break;
>
> - /* use an extra pass for the final short packet */
> - if (len > ep->ep.maxpacket) {
> - rescan = 1;
> - len -= (len % ep->ep.maxpacket);
> + /* send multiple of maxpacket first, then remainder */
> + rem = (len % ep->ep.maxpacket);
> + if (len - rem > 0) {
That looks really strange, particularly since len and rem are both
unsigned. Just say "if (len > rem)". Or even "if (len >=
ep->ep.maxpacket)", which is equivalent and is closer to the existing
code.
> + is_short = 0;
> + len -= rem;
> + if (rem > 0)
> + rescan = 1;
> + } else { /* rem > 0 */
> + is_short = 1;
> + len = rem;
This line is unnecessary. If len is not > rem then they must be equal
already.
> }
> - is_short = (len % ep->ep.maxpacket) != 0;
>
> len = dummy_perform_transfer(urb, req, len);
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: How to know when udev rules are done being applied to a new USB device
Hello, Greg. Thanks for the quick reply, and for all your contributions to Linux. On Fri, Sep 11, 2015 at 6:01 PM, Greg KH wrote: > Can't you wait for libudev to notify your application when the usb > device is attached? That should happen after the normal rules are run. > Have you tried that? I have not tried using a udev_monitor but I do believe it would work. I used it indirectly via libusb and that seemed to work. However, using a udev_monitor would impose an unwelcome constraint, which is the requirement to start monitoring for new devices before the device is plugged into the computer. This complicates the scenario I described in my first email where one program might tell a device to go into bootloader/firmware upgrade mode (which entails disconnecting from USB and re-enumerating), and then another program starts and waits for the bootloader to be available. I wrote about this in point #4 of my first email (quoted below). For future versions of Linux, I would suggest adding a sysfs attribute that changes from "0" to "1" when the rules are fully applied. In the meantime, I will think of a workaround. I'll probably open the devnode file and close it quickly as a test to see if it can be opened, and exclude devices from consideration if that test does not pass. I'll be careful to only do this for devices that I am actually interested in opening, since opening that file can have side effects. If you have better suggestions or know about some way to tell if the udev rules are fully applied, I would be happy to hear it. Thanks again! --David On Fri, Sep 11, 2015 at 4:54 PM, David Grayson wrote: > 4) After a libusb context is created, it looks like libusb detects > changes in the set of USB devices by calling > udev_monitor_receive_device(). The documentation of > udev_monitor_new_from_netlink() says: > >> The "udev" events are sent out after udev has finished >> its event processing, all rules have been processed, >> and needed device nodes are created. > > Unfortunately, using a udev_monitor (or a libusb context) will make my > code more complicated, because there will be a need to start the > monitor and keep it around while we are waiting for the new USB device > to connect. It is simpler if I can just ask the system for a list of > all the USB devices and filter out the ones that are not fully set up > yet, without needing to store any context data, or have global > variables, or have a file handle open to the monitor. > > Also, if I use the udev_monitor (or a libusb context), I will need to > guarantee that this monitor is started well before the device is > plugged in; if the device is plugged in a little bit before the > monitor is started, then I assume I wouldn't receive an event for the > new device, so I cannot be sure that it is ready to be used. This > prohibits the useful scenario where process A tells the device to > disconnect from USB (e.g. to go into bootloader mode and appear as a > new USB device), and then we start process B which actually waits for > the new device to appear and then communicates with it. By the time > process B starts, the new USB device may or may not be detected by the > system, so it is too late to start monitoring for it. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to know when udev rules are done being applied to a new USB device
On Mon, Sep 14, 2015 at 11:55:17AM -0700, David Grayson wrote: > Hello, Greg. Thanks for the quick reply, and for all your > contributions to Linux. > > On Fri, Sep 11, 2015 at 6:01 PM, Greg KH wrote: > > Can't you wait for libudev to notify your application when the usb > > device is attached? That should happen after the normal rules are run. > > Have you tried that? > > I have not tried using a udev_monitor but I do believe it would work. > I used it indirectly via libusb and that seemed to work. However, > using a udev_monitor would impose an unwelcome constraint, which is > the requirement to start monitoring for new devices before the device > is plugged into the computer. That's the only safe way to ensure you catch all USB devices, sorry. > This complicates the scenario I > described in my first email where one program might tell a device to > go into bootloader/firmware upgrade mode (which entails disconnecting > from USB and re-enumerating), and then another program starts and > waits for the bootloader to be available. I wrote about this in point > #4 of my first email (quoted below). Start your program "B" before telling the device to reset itself. > For future versions of Linux, I would suggest adding a sysfs attribute > that changes from "0" to "1" when the rules are fully applied. No, this is up to userspace to handle, don't use a kernel sysfs file as a "poor-man's IPC" please. > In the > meantime, I will think of a workaround. I'll probably open the > devnode file and close it quickly as a test to see if it can be > opened, and exclude devices from consideration if that test does not > pass. I'll be careful to only do this for devices that I am actually > interested in opening, since opening that file can have side effects. > > If you have better suggestions or know about some way to tell if the > udev rules are fully applied, I would be happy to hear it. Thanks > again! Start your program to listen and handle all devices that way through udev iterators, that way will always work (for existing and new devices). 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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hi,
On Mon, Sep 14, 2015 at 12:56 PM, Hans de Goede wrote:
> Hi,
>
> On 14-09-15 19:53, Bin Liu wrote:
>
>
>
This is my first time looking at dts handling in drivers, so I might
be completely wrong, but I am thinking that since the controller node
links to the phy node, so the controller node is the parent of the phy
node, so if there is an of api can look it up?
>>>
>>>
>>>
>>> If the phy is a child of the controller, then yes this would work,
>>> but in the case of sunxi the phy is a built-in mmio mapped peripheral
>>> just like the controller, so they sit at the same level and have no
>>> parent child relation.
>>
>>
>> musb_dsps dts is the same.
>>
>> sun8i-a33.dtsi:
>>
>> soc {
>> usb_otg: {
>> phys = <&usbphy 0>;
>> }
>> usbphy: {
>> }
>> }
>>
>> As in the example above, usb_otg node refers to usbphy node, so I am
>> wondering if there is an of api to look up the usb_otg properties in
>> the usbphy driver.
>
>
> That would boil down to hardcoding the node name / path compatible
> which is not acceptable from a devicetree pov.
Okay, I got this sorted out - querying dr_mode in the phy driver:
struct device_node *p;
struct device_node *t = NULL;
const char *dr_mode_str;
enum usb_dr_mode dr_mode;
do {
t = of_find_node_with_property(t, "phys");
if (!t)
goto end;
p = of_parse_phandle(t, "phys", 0);
if (p == pdev->dev.of_node)
break;
while (true);
of_property_read_string(t, "dr_mode", &dr_mode_str);
dr_mode = of_usb_get_dr_mode(dr_mode_str);
end:
Regards,
-Bin.
>
> Really having to duplicate the dr_mode is not that bad / such big
> a deal.
>
> Regards,
>
> Hans
--
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: xhci woes continued
On Sun, 13 Sep 2015, Hans-Peter Jansen wrote: > > > usbmon log attached. > > > > It wasn't attached. > > Hrmpf. Hopefully now... This time the trace wasn't so useful. It doesn't show any obvious reason for the disconnections. It only shows that the port's link state suddenly changes to INACTIVE along with a connect-status change. Maybe the device doesn't handle Link Power Management correctly? Mathias, is there any way to disable LPM for a SuperSpeed device? 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: How to know when udev rules are done being applied to a new USB device
On Mon, Sep 14, 2015 at 12:04 PM, Greg KH wrote: > Start your program to listen and handle all devices that way through > udev iterators, that way will always work (for existing and new > devices). If by "existing devices", you mean devices that were connected to the computer before my program started, that sounds great. I will look into that and see if I can make that work. If my understanding of what you said is correct, there would be no constraint that I have to start monitoring before the device is connected. I'll look into using udev to start a temporary udev_monitor and getting all the currently-connected USB devices from that. Hopefully all those devices will have their rules fully applied. --David -- 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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
On Thu, Sep 10, 2015 at 08:38:38PM +0200, Hans de Goede wrote: > Hi, > > On 10-09-15 20:30, Maxime Ripard wrote: > >On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote: > >>Hi, > >> > >>On 04-09-15 08:43, Olliver Schinagl wrote: > >>>Hey Hans, > >>> > >>>On 07-08-15 10:45, Olliver Schinagl wrote: > > >If you change the dr_mode to host then you _must_ also remove any id_det > >and vbus_det > >gpio settings from the usb_phy node in the dts, as the sun4i phy code > >detects > >host vs otg mode by checking for the presence of these. > Yes, this fixes it and makes it work. Thanks. > > >>>I've been going back to this and am wondering if this is something I can > >>>look into to fix properly? E.g. if the dts sets dr_mode = host, can we > >>>simply ignore the pins and treat them as unset? > >> > >>AFAIK you cannot unset something in dts. The only solution I > >>can comeup with is to add a dr_mode argument to the phy like > >>we already have for the otg controller itself. > >> > >>This is something which we likely need to do anyways to add > >>support for peripheral only mode, which we seem to need for > >>some "hdmi sticks". > > > >I haven't really followed the rest of the discussion, so sorry if you > >already talked about that, but why can't you just set the dr_mode to > >peripheral in such a case? > > This is about the usbphy code not the musb-controller code, which are > 2 different dts nodes, atm only the musb-controller node has a > dr_mode property, and the phy code decides between host-only > and otg mode based on whether an id pin is assigned or not. > > My proposal is to get rid of the id-pin hack to determine the mode > and add a dr_mode property to the usbphy dts node. I agree that we should get rid of that hack, especially since a lack of an ID pin might also be used on a peripheral-only device. However, we already have that information in the musb node, and duplicating the info seems error prone. We already have a custom function, maybe that's a case for another one, and that would allow to handle "hard" cases more easily (like CONFIG_USB_MUSB_HOST selected, with the otg node set to otg). Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M
On Sun, 13 Sep 2015, Alan Stern wrote: > On Sun, 13 Sep 2015, Roland Weber wrote: > > > I compiled a kernel 3.17(.0) with my additional debug lines... > > and this one does NOT freeze! The output is: > > > > (first unbind) > > ehci-pci :00:1d.0: remove, state 4 > > ehci-pci :00:1d.0: roothub graceful disconnect > > usb usb1: USB disconnect, device number 1 > > ehci-pci :00:1d.0: shutdown urb 880072bf4300 ep1in-intr > > usb_remove_hcd: calling stop > > ehci-pci :00:1d.0: stop > > ehci_silence_controller: entry > > ehci_halt: entry > > ehci_halt: after spin_lock_irq > > ehci_halt: about to readl prematurely > > ehci_halt: premature readl returned 10001 > > Note: 10001 instead of 1, which is what you saw in the other > kernel. That could be highly relevant. > At some point along the way, can you try adding some printk statements > to ehci_suspend() and ehci_resume() in ehci-hcd.c? I'd like to know if > they get called during the bind/unbind procedure and are responsible > for that 10001 vs. 1 value. There's one other thing I'd like to see as well. Before doing the unbind, save a copy of /sys/kernel/debug/usb/ehci/:00:1d.0/registers. Do this for both types of kernel (one that gets 1 for the readl value and one that gets 10001). There ought to be some useful information in them. Oh, and also let's see what /sys/bus/pci/devices/:00:1d.0/power/control and /sys/bus/pci/devices/:00:1d.0/power/runtime_status contain. 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: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M
Roland, Roland Weber wrote: > Assuming that this kernel does freeze, I will start to 'bisect' the > differences in the kernel configurations Thank you very much for doing this work. You are one of the many open source heroes and heroines. //Peter -- 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: How to know when udev rules are done being applied to a new USB device
Good news! I figured out that I can simply use udev_device_get_is_initialized(). It does exactly what I want. I tried using a udev_monitor but it doesn't seem to report pre-existing devices (devices that were already connected to the system). It just reports changes. Thanks for the help. --David On Mon, Sep 14, 2015 at 12:22 PM, David Grayson wrote: > On Mon, Sep 14, 2015 at 12:04 PM, Greg KH wrote: >> Start your program to listen and handle all devices that way through >> udev iterators, that way will always work (for existing and new >> devices). > > If by "existing devices", you mean devices that were connected to the > computer before my program started, that sounds great. I will look > into that and see if I can make that work. If my understanding of > what you said is correct, there would be no constraint that I have to > start monitoring before the device is connected. I'll look into using > udev to start a temporary udev_monitor and getting all the > currently-connected USB devices from that. Hopefully all those > devices will have their rules fully applied. > > --David -- 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: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M
Hi Alan, > > ehci_halt: premature readl returned 10001 > > Note: 10001 instead of 1, which is what you saw in the other > kernel. That could be highly relevant. Really? And I thought that was the least significant bit... I had a lucky guess among the 2000+ lines of diff output with which I started: CONFIG_PM_RUNTIME makes the difference. If it's on, the kernel can freeze. I'll have to add the debug output you asked for tomorrow, didn't expect to succeed so soon. Should I put the statements into the 3.17 kernel, or should I try to compile a 4.2 without CONFIG_PM_RUNTIME instead? And would you like the output of a freezing or non-freezing kernel first? I'll only be able to try out one tomorrow, the rest will follow a day or two later. cheers and thanks, Roland -- 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] Revert "usb: chipidea: usbmisc_imx: delete clock information"
On Mon, Sep 14, 2015 at 09:22:37AM -0300, Fabio Estevam wrote: > On Mon, Sep 14, 2015 at 1:54 AM, Peter Chen wrote: > > On Fri, Sep 11, 2015 at 10:17:20AM -0300, Fabio Estevam wrote: > > > Sorry, a bug at former patch, it the clk_ahb has been override. > > Mind to try below one? > > Applied the patch and still see the issue: > > Booting Linux on physical CPU 0x0 > Linux version 4.3.0-rc1-next-20150914-dirty (fabio@fabio-Latitude-E6410) (gcc > v5 > CPU: ARM926EJ-S [41069264] revision 4 (ARMv5TEJ), cr=0005317f > CPU: VIVT data cache, VIVT instruction cache > Machine model: Freescale i.MX27 Product Development Kit > Memory policy: Data cache writeback > Built 1 zonelists in Zone order, mobility grouping on. Total pages: 32512 > Kernel command line: noinitrd console=ttymxc0,115200 root=/dev/nfs > nfsroot=192.p > PID hash table entries: 512 (order: -1, 2048 bytes) > Dentry cache hash table entries: 16384 (order: 4, 65536 bytes) > Inode-cache hash table entries: 8192 (order: 3, 32768 bytes) > Memory: 121952K/131072K available (5428K kernel code, 370K rwdata, 1636K > rodata) > Virtual kernel memory layout: > vector : 0x - 0x1000 ( 4 kB) > fixmap : 0xffc0 - 0xfff0 (3072 kB) > vmalloc : 0xc880 - 0xff00 ( 872 MB) > lowmem : 0xc000 - 0xc800 ( 128 MB) > modules : 0xbf00 - 0xc000 ( 16 MB) > .text : 0xc0008000 - 0xc06ee5f8 (7066 kB) > .init : 0xc06ef000 - 0xc073a000 ( 300 kB) > .data : 0xc073a000 - 0xc0796aac ( 371 kB) >.bss : 0xc0796aac - 0xc07b6a18 ( 128 kB) > Preemptible hierarchical RCU implementation. > Build-time adjustment of leaf fanout to 32. > NR_IRQS:16 nr_irqs:16 16 > MXC IRQ initialized > CPU identified as i.MX27, silicon rev 2.1 > Switching to timer-based delay loop, resolution 75ns > sched_clock: 32 bits at 13MHz, resolution 75ns, wraps every 161464936410ns > clocksource: mxc_timer1: mask: 0x max_cycles: 0x, > max_idle_ns: s > Console: colour dummy device 80x30 > Calibrating delay loop (skipped), value calculated using timer frequency.. > 26.6) > pid_max: default: 32768 minimum: 301 > Mount-cache hash table entries: 1024 (order: 0, 4096 bytes) > Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes) > CPU: Testing write buffer coherency: ok > Setting up static identity map for 0xa0008400 - 0xa000847c > devtmpfs: initialized > clocksource: jiffies: mask: 0x max_cycles: 0x, max_idle_ns: > 191s > pinctrl core: initialized pinctrl subsystem > NET: Registered protocol family 16 > DMA: preallocated 256 KiB pool for atomic coherent allocations > imx27-pinctrl 10015000.iomuxc: initialized IMX pinctrl driver > SCSI subsystem initialized > usbcore: registered new interface driver usbfs > usbcore: registered new interface driver hub > usbcore: registered new device driver usb > usbphy:usbphy@0 supply vcc not found, using dummy regulator > Linux video capture interface: v2.00 > pps_core: LinuxPPS API ver. 1 registered > pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti > > PTP clock support registered > Advanced Linux Sound Architecture Driver Initialized. > clocksource: Switched to clocksource mxc_timer1 > NET: Registered protocol family 2 > TCP established hash table entries: 1024 (order: 0, 4096 bytes) > TCP bind hash table entries: 1024 (order: 0, 4096 bytes) > TCP: Hash tables configured (established 1024 bind 1024) > UDP hash table entries: 256 (order: 0, 4096 bytes) > UDP-Lite hash table entries: 256 (order: 0, 4096 bytes) > NET: Registered protocol family 1 > RPC: Registered named UNIX socket transport module. > RPC: Registered udp transport module. > RPC: Registered tcp transport module. > RPC: Registered tcp NFSv4.1 backchannel transport module. > futex hash table entries: 256 (order: -1, 3072 bytes) > jffs2: version 2.2. (NAND) �© 2001-2006 Red Hat, Inc. > io scheduler noop registered (default) > 1000a000.serial: ttymxc0 at MMIO 0x1000a000 (irq = 36, base_baud = 831250) is > aX > console [ttymxc0] enabled > nand: device found, Manufacturer ID: 0xec, Chip ID: 0xaa > nand: Samsung NAND 256MiB 1,8V 8-bit > nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64 > Bad block table found at page 131008, version 0x01 > Bad block table found at page 130944, version 0x01 > mc13xxx spi1.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0 > spi_imx 1000f000.cspi: probed > 1002b000.ethernet supply phy not found, using dummy regulator > libphy: fec_enet_mii_bus: probed > ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver > ehci-mxc: Freescale On-Chip EHCI Host driver > usbcore: registered new int
Re: [PATCH] Revert "usb: chipidea: usbmisc_imx: delete clock information"
On Mon, Sep 14, 2015 at 9:19 PM, Peter Chen wrote: > That's so strange, do we need to enable clk_ahb before than clk_ipg? > Would you please try below diff based on my patch? Same kernel crash with this one applied on top of the previous one. > > Meanwhile, you may need below changes > > http://marc.info/?l=linux-usb&m=143640061512265&w=2 This proposed change is not correct. It passes IMX27_CLK_USB_AHB_GATE as if it were the clock driving the ULPI PHY, which is clearly wrong. -- 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] Revert "usb: chipidea: usbmisc_imx: delete clock information"
On Mon, Sep 14, 2015 at 10:59:29PM -0300, Fabio Estevam wrote: > On Mon, Sep 14, 2015 at 9:19 PM, Peter Chen wrote: > > > That's so strange, do we need to enable clk_ahb before than clk_ipg? > > Would you please try below diff based on my patch? > > Same kernel crash with this one applied on top of the previous one. > Would you try below? diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile index 4decb12..34e9f6e 100644 --- a/drivers/usb/chipidea/Makefile +++ b/drivers/usb/chipidea/Makefile @@ -16,4 +16,4 @@ obj-$(CONFIG_USB_CHIPIDEA)+= ci_hdrc_zevio.o obj-$(CONFIG_USB_CHIPIDEA_PCI) += ci_hdrc_pci.o -obj-$(CONFIG_USB_CHIPIDEA_OF) += usbmisc_imx.o ci_hdrc_imx.o +obj-$(CONFIG_USB_CHIPIDEA_OF) += ci_hdrc_imx.o usbmisc_imx.o > > > > Meanwhile, you may need below changes > > > > http://marc.info/?l=linux-usb&m=143640061512265&w=2 > > This proposed change is not correct. It passes IMX27_CLK_USB_AHB_GATE > as if it were the clock driving the ULPI PHY, which is clearly wrong. But the error log looks similar, it seems we does not open IMX27_CLK_USB_AHB_GATE correctly. -- 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] Revert "usb: chipidea: usbmisc_imx: delete clock information"
On Mon, Sep 14, 2015 at 9:50 PM, Peter Chen wrote: > > Would you try below? > > diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile > index 4decb12..34e9f6e 100644 > --- a/drivers/usb/chipidea/Makefile > +++ b/drivers/usb/chipidea/Makefile > @@ -16,4 +16,4 @@ obj-$(CONFIG_USB_CHIPIDEA)+= ci_hdrc_zevio.o > > obj-$(CONFIG_USB_CHIPIDEA_PCI) += ci_hdrc_pci.o > > -obj-$(CONFIG_USB_CHIPIDEA_OF) += usbmisc_imx.o ci_hdrc_imx.o > +obj-$(CONFIG_USB_CHIPIDEA_OF) += ci_hdrc_imx.o usbmisc_imx.o This one gives kernel dump in loop, with a different pattern: b9c0: c073f110 c78bd800 c0743c38 c0522e3c c78bda3c b9e0: c0033b02 c78eba44 c0743c38 c78eba08 c002f2f8 c0033b00 2093 [] (__dabt_svc) from [] (kthread_data+0x4/0xc) [] (kthread_data) from [] (wq_worker_sleeping+0xc/0xbc) [] (wq_worker_sleeping) from [] (__schedule+0x2fc/0x654) [] (__schedule) from [] (schedule+0x3c/0xa8) [] (schedule) from [] (do_exit+0x7ac/0x9a0) [] (do_exit) from [] (die+0x164/0x3b0) [] (die) from [] (__do_kernel_fault.part.0+0x54/0x1e4) [] (__do_kernel_fault.part.0) from [] (do_page_fault+0x338/) [] (do_page_fault) from [] (do_DataAbort+0x34/0xb4) [] (do_DataAbort) from [] (__dabt_svc+0x3c/0x60) Exception stack(0xc78ebbe8 to 0xc78ebc30) bbe0: c78bd800 c7832b60 bc00: c78bd800 c0743c38 c0522e3c c78bda3c 0001 c78ebc74 c0743c38 c78ebc38 bc20: c002f2f8 c0033b00 2093 [] (__dabt_svc) from [] (kthread_data+0x4/0xc) [] (kthread_data) from [] (wq_worker_sleeping+0xc/0xbc) [] (wq_worker_sleeping) from [] (__schedule+0x2fc/0x654) [] (__schedule) from [] (schedule+0x3c/0xa8) [] (schedule) from [] (do_exit+0x5a4/0x9a0) [] (do_exit) from [] (die+0x164/0x3b0) [] (die) from [] (do_DataAbort+0xa4/0xb4) [] (do_DataAbort) from [] (__dabt_svc+0x3c/0x60) Exception stack(0xc78ebdc8 to 0xc78ebe10) bdc0: 0001 0001 f4424600 c7a24790 c7a8c410 bde0: 6013 0100 c055a2a4 c07943fc 0002 c78ebe18 be00: c03af778 c03af784 6093 [] (__dabt_svc) from [] (usbmisc_imx27_init+0x4c/0xbc) [] (usbmisc_imx27_init) from [] (imx_usbmisc_init+0x2c/0x38) [] (imx_usbmisc_init) from [] (ci_hdrc_imx_probe+0x22c/0x38) [] (ci_hdrc_imx_probe) from [] (platform_drv_probe+0x48/0xa) [] (platform_drv_probe) from [] (driver_probe_device+0x1e0/) [] (driver_probe_device) from [] (bus_for_each_drv+0x48/0x9) [] (bus_for_each_drv) from [] (__device_attach+0x9c/0x100) [] (__device_attach) from [] (bus_probe_device+0x88/0x90) [] (bus_probe_device) from [] (deferred_probe_work_func+0x5) [] (deferred_probe_work_func) from [] (process_one_work+0x1) [] (process_one_work) from [] (worker_thread+0x28/0x524) [] (worker_thread) from [] (kthread+0xc0/0xdc) [] (kthread) from [] (ret_from_fork+0x14/0x24) Code: c0032f64 c0621d18 c07976a0 e5903210 (e5130010) > But the error log looks similar, it seems we does not open > IMX27_CLK_USB_AHB_GATE > correctly. Yes, but we cannot pass IMX27_CLK_USB_AHB_GATE as the clock for the ULPI PHY as it is not correct. -- 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] Revert "usb: chipidea: usbmisc_imx: delete clock information"
On Mon, Sep 14, 2015 at 09:22:37AM -0300, Fabio Estevam wrote: > On Mon, Sep 14, 2015 at 1:54 AM, Peter Chen wrote: > > On Fri, Sep 11, 2015 at 10:17:20AM -0300, Fabio Estevam wrote: > > > Sorry, a bug at former patch, it the clk_ahb has been override. > > Mind to try below one? > > Applied the patch and still see the issue: > > Booting Linux on physical CPU 0x0 > Linux version 4.3.0-rc1-next-20150914-dirty (fabio@fabio-Latitude-E6410) (gcc > v5 > CPU: ARM926EJ-S [41069264] revision 4 (ARMv5TEJ), cr=0005317f > CPU: VIVT data cache, VIVT instruction cache > Machine model: Freescale i.MX27 Product Development Kit > Memory policy: Data cache writeback > Built 1 zonelists in Zone order, mobility grouping on. Total pages: 32512 > Kernel command line: noinitrd console=ttymxc0,115200 root=/dev/nfs > nfsroot=192.p > PID hash table entries: 512 (order: -1, 2048 bytes) > Dentry cache hash table entries: 16384 (order: 4, 65536 bytes) > Inode-cache hash table entries: 8192 (order: 3, 32768 bytes) > Memory: 121952K/131072K available (5428K kernel code, 370K rwdata, 1636K > rodata) > Virtual kernel memory layout: > vector : 0x - 0x1000 ( 4 kB) > fixmap : 0xffc0 - 0xfff0 (3072 kB) > vmalloc : 0xc880 - 0xff00 ( 872 MB) > lowmem : 0xc000 - 0xc800 ( 128 MB) > modules : 0xbf00 - 0xc000 ( 16 MB) > .text : 0xc0008000 - 0xc06ee5f8 (7066 kB) > .init : 0xc06ef000 - 0xc073a000 ( 300 kB) > .data : 0xc073a000 - 0xc0796aac ( 371 kB) >.bss : 0xc0796aac - 0xc07b6a18 ( 128 kB) > Preemptible hierarchical RCU implementation. > Build-time adjustment of leaf fanout to 32. > NR_IRQS:16 nr_irqs:16 16 > MXC IRQ initialized > CPU identified as i.MX27, silicon rev 2.1 > Switching to timer-based delay loop, resolution 75ns > sched_clock: 32 bits at 13MHz, resolution 75ns, wraps every 161464936410ns > clocksource: mxc_timer1: mask: 0x max_cycles: 0x, > max_idle_ns: s > Console: colour dummy device 80x30 > Calibrating delay loop (skipped), value calculated using timer frequency.. > 26.6) > pid_max: default: 32768 minimum: 301 > Mount-cache hash table entries: 1024 (order: 0, 4096 bytes) > Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes) > CPU: Testing write buffer coherency: ok > Setting up static identity map for 0xa0008400 - 0xa000847c > devtmpfs: initialized > clocksource: jiffies: mask: 0x max_cycles: 0x, max_idle_ns: > 191s > pinctrl core: initialized pinctrl subsystem > NET: Registered protocol family 16 > DMA: preallocated 256 KiB pool for atomic coherent allocations > imx27-pinctrl 10015000.iomuxc: initialized IMX pinctrl driver > SCSI subsystem initialized > usbcore: registered new interface driver usbfs > usbcore: registered new interface driver hub > usbcore: registered new device driver usb > usbphy:usbphy@0 supply vcc not found, using dummy regulator > Linux video capture interface: v2.00 > pps_core: LinuxPPS API ver. 1 registered > pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti > > PTP clock support registered > Advanced Linux Sound Architecture Driver Initialized. > clocksource: Switched to clocksource mxc_timer1 > NET: Registered protocol family 2 > TCP established hash table entries: 1024 (order: 0, 4096 bytes) > TCP bind hash table entries: 1024 (order: 0, 4096 bytes) > TCP: Hash tables configured (established 1024 bind 1024) > UDP hash table entries: 256 (order: 0, 4096 bytes) > UDP-Lite hash table entries: 256 (order: 0, 4096 bytes) > NET: Registered protocol family 1 > RPC: Registered named UNIX socket transport module. > RPC: Registered udp transport module. > RPC: Registered tcp transport module. > RPC: Registered tcp NFSv4.1 backchannel transport module. > futex hash table entries: 256 (order: -1, 3072 bytes) > jffs2: version 2.2. (NAND) �© 2001-2006 Red Hat, Inc. > io scheduler noop registered (default) > 1000a000.serial: ttymxc0 at MMIO 0x1000a000 (irq = 36, base_baud = 831250) is > aX > console [ttymxc0] enabled > nand: device found, Manufacturer ID: 0xec, Chip ID: 0xaa > nand: Samsung NAND 256MiB 1,8V 8-bit > nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64 > Bad block table found at page 131008, version 0x01 > Bad block table found at page 130944, version 0x01 > mc13xxx spi1.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0 > spi_imx 1000f000.cspi: probed > 1002b000.ethernet supply phy not found, using dummy regulator > libphy: fec_enet_mii_bus: probed > ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver > ehci-mxc: Freescale On-Chip EHCI Host driver > usbcore: registered new int
Re: [PATCH] Revert "usb: chipidea: usbmisc_imx: delete clock information"
On Mon, Sep 14, 2015 at 10:03 PM, Peter Chen wrote: > Would you try to insert udelay(100) at every clk_prepare_enable at > imx_prepare_enable_clks at ci_hdrc_imx.c? This did not help. It is getting late here, so I will be able to try more things tomorrow. Regards, Fabio Estevam -- 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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hi, On 09/14/2015 10:25 PM, Maxime Ripard wrote: On Thu, Sep 10, 2015 at 08:38:38PM +0200, Hans de Goede wrote: Hi, On 10-09-15 20:30, Maxime Ripard wrote: On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote: Hi, On 04-09-15 08:43, Olliver Schinagl wrote: Hey Hans, On 07-08-15 10:45, Olliver Schinagl wrote: If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det gpio settings from the usb_phy node in the dts, as the sun4i phy code detects host vs otg mode by checking for the presence of these. Yes, this fixes it and makes it work. Thanks. I've been going back to this and am wondering if this is something I can look into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore the pins and treat them as unset? AFAIK you cannot unset something in dts. The only solution I can comeup with is to add a dr_mode argument to the phy like we already have for the otg controller itself. This is something which we likely need to do anyways to add support for peripheral only mode, which we seem to need for some "hdmi sticks". I haven't really followed the rest of the discussion, so sorry if you already talked about that, but why can't you just set the dr_mode to peripheral in such a case? This is about the usbphy code not the musb-controller code, which are 2 different dts nodes, atm only the musb-controller node has a dr_mode property, and the phy code decides between host-only and otg mode based on whether an id pin is assigned or not. My proposal is to get rid of the id-pin hack to determine the mode and add a dr_mode property to the usbphy dts node. I agree that we should get rid of that hack, especially since a lack of an ID pin might also be used on a peripheral-only device. However, we already have that information in the musb node, and duplicating the info seems error prone. We already have a custom function, maybe that's a case for another one AFAIK the musb / phy maintainers really want to avoid adding more custom functions ... Felipe, Kishon any comments on this ? Regards, Hans -- 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] Revert "usb: chipidea: usbmisc_imx: delete clock information"
> On Mon, Sep 14, 2015 at 10:03 PM, Peter Chen > wrote: > > > Would you try to insert udelay(100) at every clk_prepare_enable at > > imx_prepare_enable_clks at ci_hdrc_imx.c? > > This did not help. > > It is getting late here, so I will be able to try more things tomorrow. > Thanks, Peter
Re: MIDI gadget allocating too much from coherent pool
On Thu, Sep 10, 2015 at 10:47:07AM +0100, Felipe Tonello wrote: > Hi, > > I have the following setup: > > DSP (read sensors and read/send MIDI) <- UART -> SOC (imx6) <- USB > MIDI GADGET -> HOST > > When the throughput from the DSP is high, thus causing the throughput > on the USB to be high as well, I get a Kernel Panic saying to increase > the coherent_pool. I've used some crazy sizes like 1M, 4M and even 8M. > Obviously when I increase, it takes longer to crash but it still > crashes. > > I am using linux-fsl 3.14.28. > > The BT as follows: > > [ 193.619701] ERROR: 1024 KiB atomic DMA coherent pool is too small! > [ 193.619701] Please increase it with coherent_pool= kernel parameter! > [ 193.632261] Unable to handle kernel NULL pointer dereference at > virtual address > [ 193.640358] pgd = 80004000 > [ 193.643070] [] *pgd= > [ 193.646670] Internal error: Oops: 817 [#1] PREEMPT SMP ARM > [ 193.652162] Modules linked in:0d 0 :s0n2d:_5s2e.q2_2m1i > snd_seq_midi_event snd_seq_dummy snd_seq snd_soc_cs4272 > snd_soc_fsl_ssi imx_pcm_dma imx_pcm_fiq snd_soc_imx_cs4272 > snd_soc_core snd_compress snd_pcm_dmaengine snd_pcm snd_te > [ 193.689447] CPU: 2 PID: 16 Comm: ksoftirqd/2 Not tainted > 3.14.28-1.0.0_ga+g91cf351 #1 > [ 193.697285] task: ea0a5100 ti: ea0d8000 task.ti: ea0d8000 > [ 193.702708] PC is at _ep_queue.isra.24+0x178/0x480 > [ 193.707508] LR is at 0xc43cee80 > [ 193.710657] pc : [<8039ea38>]lr : []psr: 600f0093 > [ 193.710657] sp : ea0d9de0 ip : 6e0ea000 fp : ea0d9e1c > [ 193.722139] r10: 5000 r9 : ea03c010 r8 : c43d05b4 > [ 193.727369] r7 : r6 : 0004 r5 : ea03c6f4 r4 : c43d0580 > [ 193.733899] r3 : c43d05bc r2 : r1 : 0001 r0 : fff4 > [ 193.740433] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM > Segment kernel > [ 193.747834] Control: 10c5387d Table: 7aca804a DAC: 0015 > [ 193.753585] Process ksoftirqd/2 (pid: 16, stack limit = 0xea0d8238) > [ 193.759857] Stack: (0xea0d9de0 to 0xea0da000) > [ 193.764225] 9de0: 0001 eaa72480 a00f0013 c43d05bc ea0d9e24 > 0003 ea03c6f4 a00f0013 > [ 193.772413] 9e00: c43d0580 ea51a200 ea51a328 ea51a328 ea0d9e3c > ea0d9e20 8039fc30 8039e8cc > [ 193.780598] 9e20: c4467000 c43d0580 ea0d9e7c > ea0d9e40 7f078754 8039fbe8 > [ 193.788784] 9e40: ea03c6f4 ea51a2ec 80009990 6f00 ea0d9e84 > ea51a330 ea51a334 > [ 193.796968] 9e60: 80718f40 ea0d8000 806d82bc ea0d9e8c > ea0d9e80 7f0787a0 7f07832c > [ 193.805153] 9e80: ea0d9ebc ea0d9e90 80031fe4 7f078798 80031f60 > 806dc080 > [ 193.826207] 9ec0: ea0d9f0c 04208040 806dc0c0 d672 > 80507358 000a 80718f40 > [ 193.834392] 9ee0: ea0d8000 806d8458 806dc080 0002 80053c8c > ea0d8000 ea0870c0 806e9cc8 > [ 193.842576] 9f00: 0001 ea0d9f34 > ea0d9f20 80032490 800321f0 > [ 193.868396] 9f40: ea087140 ea0870c0 800512e0 ea0d9fac > ea0d9f60 8004a458 800512ec > [ 193.876582] 9f60: 30b80088 0001 0002 ea0870c0 > 00030003 ea0d9f78 ea0d9f78 > [ 193.884766] 9f80: ea0d9f88 ea0d9f88 ea087140 > 8004a384 > [ 193.892952] 9fa0: ea0d9fb0 8000ea58 8004a390 > > [ 193.901137] 9fc0: > > [ 193.909322] 9fe0: 0013 > 2001a1a0 61391094 > [ 193.917501] Backtrace: > [ 193.919982] [<8039e8c0>] (_ep_queue.isra.24) from [<8039fc30>] > (ep_queue+0x54/0x88) > [ 193.927643] r10:ea51a328 r9:ea51a328 r8:ea51a200 r7:c43d0580 > r6:a00f0013 r5:ea03c6f4 > [ 193.935554] r4:0003 > [ 193.938120] [<8039fbdc>] (ep_queue) from [<7f078754>] > (f_midi_transmit+0x434/0x46c [g_midi]) > [ 193.946562] r7:c43d0580 r6:c4467000 r5: r4: > [ 193.952299] [<7f078320>] (f_midi_transmit [g_midi]) from > [<7f0787a0>] (f_midi_in_tasklet+0x14/0x18 [g_midi]) > [ 193.962129] r10:806d82bc r9:ea0d8000 r8: r7:80718f40 > r6: r5:ea51a334 > [ 193.970039] r4:ea51a330 > [ 193.972609] [<7f07878c>] (f_midi_in_tasklet [g_midi]) from > [<80031fe4>] (tasklet_hi_action+0x84/0x114) > [ 193.981928] [<80031f60>] (tasklet_hi_action) from [<8003232c>] > (__do_softirq+0x148/0x260) > [ 193.990107] r10:4000 r9:806dc080 r8:0100 r7:ea0d8000 > r6:806dc080 r5: > [ 193.998018] r4: r3:80031f60 > [ 194.001635] [<800321e4>] (__do_softirq) from [<80032490>] > (run_ksoftirqd+0x4c/0x64) > [ 194.009294] r10: r9: r8: r7:0001 > r6:806e9cc8 r5:ea0870c0 > [ 194.017203] r4:ea0d8000 > [ 194.019773] [<80032444>] (run_ksoftirqd) from [<80051468>] > (smpboot_thread_fn+0x188/0x278) > [ 194.028059] [<800512e0>] (smpboot_thread_fn) from [<8004a458>] > (kthread+0xd4/0xec) > [ 194.035632] r8: r7:800512e0 r6:ea0870c0 r5:ea0871
Re: MIDI gadget allocating too much from coherent pool
On Thu, Sep 10, 2015 at 6:47 AM, Felipe Tonello wrote: > Hi, > > I have the following setup: > > DSP (read sensors and read/send MIDI) <- UART -> SOC (imx6) <- USB > MIDI GADGET -> HOST > > When the throughput from the DSP is high, thus causing the throughput > on the USB to be high as well, I get a Kernel Panic saying to increase > the coherent_pool. I've used some crazy sizes like 1M, 4M and even 8M. > Obviously when I increase, it takes longer to crash but it still > crashes. > > I am using linux-fsl 3.14.28. Please test it with 4.2 or 4.3-rc1 kernel. -- 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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode
Hi, On Mon, Sep 14, 2015 at 9:54 PM, Hans de Goede wrote: > Hi, > > > On 09/14/2015 10:25 PM, Maxime Ripard wrote: >> >> On Thu, Sep 10, 2015 at 08:38:38PM +0200, Hans de Goede wrote: >>> >>> Hi, >>> >>> On 10-09-15 20:30, Maxime Ripard wrote: On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote: > > Hi, > > On 04-09-15 08:43, Olliver Schinagl wrote: >> >> Hey Hans, >> >> On 07-08-15 10:45, Olliver Schinagl wrote: >>> >>> If you change the dr_mode to host then you _must_ also remove any id_det and vbus_det gpio settings from the usb_phy node in the dts, as the sun4i phy code detects host vs otg mode by checking for the presence of these. >>> >>> Yes, this fixes it and makes it work. Thanks. >>> >> I've been going back to this and am wondering if this is something I >> can look into to fix properly? E.g. if the dts sets dr_mode = host, can >> we >> simply ignore the pins and treat them as unset? > > > AFAIK you cannot unset something in dts. The only solution I > can comeup with is to add a dr_mode argument to the phy like > we already have for the otg controller itself. > > This is something which we likely need to do anyways to add > support for peripheral only mode, which we seem to need for > some "hdmi sticks". I haven't really followed the rest of the discussion, so sorry if you already talked about that, but why can't you just set the dr_mode to peripheral in such a case? >>> >>> >>> This is about the usbphy code not the musb-controller code, which are >>> 2 different dts nodes, atm only the musb-controller node has a >>> dr_mode property, and the phy code decides between host-only >>> and otg mode based on whether an id pin is assigned or not. >>> >>> My proposal is to get rid of the id-pin hack to determine the mode >>> and add a dr_mode property to the usbphy dts node. >> >> >> I agree that we should get rid of that hack, especially since a lack >> of an ID pin might also be used on a peripheral-only device. >> >> However, we already have that information in the musb node, and >> duplicating the info seems error prone. We already have a custom >> function, maybe that's a case for another one > > > AFAIK the musb / phy maintainers really want to avoid adding more > custom functions ... I add a function called of_usb_get_dr_mode_by_phy() in usb/common.c, so that other phy drivers can call it to get dr_mode of the controller. I will send the patch tomorrow morning for review. Regards, -Bin. > > Felipe, Kishon any comments on this ? > > > Regards, > > Hans > -- > 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] usb: chipidea: reuse the platform_data to store the ci info
2015-09-14 15:17 GMT+08:00 Peter Chen :
> On Tue, Aug 11, 2015 at 09:43:13AM +, Barry Song wrote:
>> From: Rong Wang
>>
>> Chipidea puts ci information to drvdata, but this overwrites the drvdata
>> placed by EHCI core. EHCI core thinks drvdata is ehci_hcd. We can find this
>> from codes like ehci-sysfs.c:
>> static ssize_t show_companion(struct device *dev,
>> struct device_attribute *attr,
>> char *buf)
>> {
>> struct ehci_hcd *ehci;
>>
>> ehci = hcd_to_ehci(bus_to_hcd(dev_get_drvdata(dev)));
>> ...
>> }
>>
>> So overwritting drvdata from chipidea driver actually breaks a part of
>> functionalities of EHCI core.
>>
>> Since the platform_data would not be accessed after the device is added
>> to system after the probe process, so it's safe to move to platform_data
>> here. This fix is not elegant but currently it is the quickest fix.
>
> Hi Rong,
>
> Since we have no better solution at current stage, I accept this
> solution, rebase the latest usb-next tree and make more changes please,
> then send again, thanks.
peter, i just tried and this patch was applied to the newest usb-next,
what do you mean for more changes?
-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: chipidea: reuse the platform_data to store the ci info
On Tue, Sep 15, 2015 at 01:40:30PM +0800, Barry Song wrote:
> 2015-09-14 15:17 GMT+08:00 Peter Chen :
> > On Tue, Aug 11, 2015 at 09:43:13AM +, Barry Song wrote:
> >> From: Rong Wang
> >>
> >> Chipidea puts ci information to drvdata, but this overwrites the drvdata
> >> placed by EHCI core. EHCI core thinks drvdata is ehci_hcd. We can find this
> >> from codes like ehci-sysfs.c:
> >> static ssize_t show_companion(struct device *dev,
> >> struct device_attribute *attr,
> >> char *buf)
> >> {
> >> struct ehci_hcd *ehci;
> >>
> >> ehci = hcd_to_ehci(bus_to_hcd(dev_get_drvdata(dev)));
> >> ...
> >> }
> >>
> >> So overwritting drvdata from chipidea driver actually breaks a part of
> >> functionalities of EHCI core.
> >>
> >> Since the platform_data would not be accessed after the device is added
> >> to system after the probe process, so it's safe to move to platform_data
> >> here. This fix is not elegant but currently it is the quickest fix.
> >
> > Hi Rong,
> >
> > Since we have no better solution at current stage, I accept this
> > solution, rebase the latest usb-next tree and make more changes please,
> > then send again, thanks.
>
> peter, i just tried and this patch was applied to the newest usb-next,
> what do you mean for more changes?
>
More chipidea core code uses dev_get_drvdata after your version.
grep -nr "dev_get_drvdata" drivers/usb/chipidea/*
drivers/usb/chipidea/core.c:1006: struct ci_hdrc *ci =
dev_get_drvdata(dev);
drivers/usb/chipidea/core.c:1038: struct ci_hdrc *ci =
dev_get_drvdata(dev);
drivers/usb/chipidea/core.c:1071: struct ci_hdrc *ci =
dev_get_drvdata(dev);
drivers/usb/chipidea/core.c:1093: struct ci_hdrc *ci =
dev_get_drvdata(dev);
drivers/usb/chipidea/host.c:47: struct ci_hdrc *ci = dev_get_drvdata(dev);
drivers/usb/chipidea/host.c:83: struct ci_hdrc *ci = dev_get_drvdata(dev);
drivers/usb/chipidea/otg_fsm.c:39: struct ci_hdrc *ci =
dev_get_drvdata(dev);
drivers/usb/chipidea/otg_fsm.c:54: struct ci_hdrc *ci =
dev_get_drvdata(dev);
drivers/usb/chipidea/otg_fsm.c:83: struct ci_hdrc *ci =
dev_get_drvdata(dev);
drivers/usb/chipidea/otg_fsm.c:98: struct ci_hdrc *ci =
dev_get_drvdata(dev);
drivers/usb/chipidea/otg_fsm.c:124: struct ci_hdrc *ci =
dev_get_drvdata(dev);
drivers/usb/chipidea/otg_fsm.c:139: struct ci_hdrc *ci =
dev_get_drvdata(dev);
drivers/usb/chipidea/otg_fsm.c:161: struct ci_hdrc *ci =
dev_get_drvdata(dev);
--
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
