Re: [PATCH RFC 1/4] phy: Add new Exynos5 USB 3.0 PHY driver
Hi Kishon, On Tue, Dec 24, 2013 at 11:15 PM, Kishon Vijay Abraham I wrote: > Hi, > > > On Thursday 05 December 2013 01:44 PM, Vivek Gautam wrote: >> >> Hi Kishon, >> >> >> On Wed, Dec 4, 2013 at 7:58 PM, Kishon Vijay Abraham I >> wrote: >>> >>> Hi Vivek, >>> >>> On Wednesday 20 November 2013 09:14 PM, Kishon Vijay Abraham I wrote: Hi, On Wednesday 20 November 2013 03:02 PM, Vivek Gautam wrote: > > On Wed, Nov 20, 2013 at 2:34 PM, Kishon Vijay Abraham I > wrote: >> >> On Wednesday 20 November 2013 02:27 PM, Vivek Gautam wrote: >>> >>> Hi Kishon, >>> >>> >>> On Mon, Nov 11, 2013 at 4:41 PM, Kishon Vijay Abraham I >>> wrote: Hi, >>> >>> sorry for the delayed response. >>> On Wednesday 06 November 2013 05:37 AM, Jingoo Han wrote: > > On Wednesday, November 06, 2013 2:58 AM, Vivek Gautam wrote: >> >> On Tue, Nov 5, 2013 at 5:33 PM, Jingoo Han >> wrote: > > > [.] > >>> USB3.0 PHY consists of two blocks such as 3.0 block and 2.0 >>> block. >>> This USB3.0 PHY can support UTMI+ and PIPE3 interface for 3.0 >>> block >>> and 2.0 block, respectively. >>> >>> Conclusion: >>> >>> 1) USB2.0 PHY: USB2.0 HOST, USB2.0 Device >>> Base address: 0x1213 >>> >>> 2) USB3.0 PHY: USB3.0 DRD (3.0 HOST & 3.0 Device) >>> Base address: 0x1210 >>> 2.0 block(UTMI+) & 3.0 block(PIPE3) >> >> >> And this is of course the PHY used by DWC3 controller, which works >> at >> both High speed as well as Super Speed. >> Right ? > > > Right. > > While 3.0 block(PIPE3) can be used for Super Speed, 2.0 > block(UTMI+) > can be used for High speed. It should then come under *single IP muliple PHY* category similar to what Sylwester has done. >>> >>> >>> Do you mean that i should be including PHY IDs for UTMI+ phy and >>> PIPE3 >>> phy present in this PHY block ? >>> AFAICS the two phys (UTMI+ and PIPE3) do not really have separate >>> registers to program, and that's the reason >>> we program the entire PHY in a shot. >> >> >> you mean you program the same set of bits for UTMI+ and PIPE3? > > > No, looking closely into PHY datasheet as well as Exynos5250 manual, i > can see that UTMI+ and PIPE3 > phys have separate bit settings. So i think we should be able to > segregate the two PHYs (UTMI+ and PIPE3). > Pardon me for my earlier observations. no problem.. > > Let me clarify more with our h/w team also on this and then i will > confirm with this. >>> >>> >>> Did you get more information on this? >> >> >> Yes, i have been in contact with our hardware team. >> The functionality of setting up UTMI+ and PIPE3 phys separately, and >> thereby using only one functionality of the two >> at some point of time (either high speed or super speed) hasn't been >> tested so far. > > > Irrespective of whether we are able to test the functionality separately or > not, we should model it as multiple PHYs since you have separate bit > settings for UTMI+ and PIPE3. > > (I'll review your next patch version shortly). Thanks Kishon, i know i am disturbing you in the holiday season. :-) But there's one concern, on Exynos5 platforms we have only one bit to power control the entire PHY (irrespective of the two PHYs present in the USB 3.0 PHY controller). So anyways we won't be able to save anything on the power front even if we program only one PHY (UTMI/PIPE3). Although there are PHY settings register bits which seem separate for the two phys. What do you suggest in that case ? May be i am not able to understand you properly on the front of multiple PHYs > > Cheers > Kishon > > >> So i will be looking into this and try to find out proper init >> sequences for the two available PHYs >> separately and as soon as i get a working solution for this, i will >> update. >> >>> >>> Thanks >>> Kishon >> >> >> >> > -- Best Regards Vivek Gautam Samsung R&D Institute, Bangalore India -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: at91_udc: fix build warning
Hi,
It does not solve the problem. When I wrote my patch (usb: gadget: add
"maxpacket_limit" field to struct usb_ep) I didn't noticed that original
code sets maxpacket values to field of struct at91_ep, not struct
usb_ep. Function usb_ep_set_maxpacket_limit() should not be used in this
place. Initialization of maxpacket field of struct at91_ep instances in
this place is necessary.
Reverting changes in at91udc_probe() function will be best solution,
because usb_ep_set_maxpacket_limit() function is called in udc_reinit(),
and this in enough to initialize maxpacket_limit properly.
I will prepare patch fixing this.
Best regards
Robert Baldyga
Samsung R&D Institute Poland
On 12/24/2013 02:30 AM, Felipe Balbi wrote:
> commit e117e742 (usb: gadget: add "maxpacket_limit"
> field to struct usb_ep) added a build warning to
> at91_udc when it passed the wrong argument to
> usb_ep_set_maxpacket_limit().
>
> Fix this by passing correct argument.
>
> Signed-off-by: Felipe Balbi
> ---
> drivers/usb/gadget/at91_udc.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
> index 0353b64..c3f49fd 100644
> --- a/drivers/usb/gadget/at91_udc.c
> +++ b/drivers/usb/gadget/at91_udc.c
> @@ -1759,15 +1759,15 @@ static int at91udc_probe(struct platform_device *pdev)
>
> /* newer chips have more FIFO memory than rm9200 */
> if (cpu_is_at91sam9260() || cpu_is_at91sam9g20()) {
> - usb_ep_set_maxpacket_limit(&udc->ep[0], 64);
> - usb_ep_set_maxpacket_limit(&udc->ep[3], 64);
> - usb_ep_set_maxpacket_limit(&udc->ep[4], 512);
> - usb_ep_set_maxpacket_limit(&udc->ep[5], 512);
> + usb_ep_set_maxpacket_limit(&udc->ep[0].ep, 64);
> + usb_ep_set_maxpacket_limit(&udc->ep[3].ep, 64);
> + usb_ep_set_maxpacket_limit(&udc->ep[4].ep, 512);
> + usb_ep_set_maxpacket_limit(&udc->ep[5].ep, 512);
> } else if (cpu_is_at91sam9261() || cpu_is_at91sam9g10()) {
> - usb_ep_set_maxpacket_limit(&udc->ep[3], 64);
> + usb_ep_set_maxpacket_limit(&udc->ep[3].ep, 64);
> } else if (cpu_is_at91sam9263()) {
> - usb_ep_set_maxpacket_limit(&udc->ep[0], 64);
> - usb_ep_set_maxpacket_limit(&udc->ep[3], 64);
> + usb_ep_set_maxpacket_limit(&udc->ep[0].ep, 64);
> + usb_ep_set_maxpacket_limit(&udc->ep[3].ep, 64);
> }
>
> udc->udp_baseaddr = ioremap(res->start, resource_size(res));
>
--
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 issues: WD MyBook 1230 - reset SuperSpeed USB device
Hi Andrej, Same problem here. HP elitebook 8560w. Running on Arch linux, kernel: 3.12.5. Attaching WD Elements 1TB.no Thanks for confirming that the issue also occurs with HP EliteBook 8560w. Do you believe you could also create the usbmon capture trace plus the outputs of dmesg, lspci and lsusb and post them here? I am quite sure the developers will need to have a look at those. In the meantime, is there any information I can add to my original posting to help identifying the cause of this issue? Best regards, 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: [PATCH v5 4/9] usb: ehci-s5p: Change to use phy provided by the generic phy framework
Hi Vivek,
> From: Vivek Gautam [mailto:gautamvivek1...@gmail.com]
> Sent: Thursday, December 26, 2013 11:14 AM
>
> Hi Kamil,
>
>
> On Fri, Dec 20, 2013 at 6:54 PM, Kamil Debski
> wrote:
> > Change the phy provider used from the old one using the USB phy
> > framework to a new one using the Generic phy framework.
> >
> > Signed-off-by: Kamil Debski
> > Signed-off-by: Kyungmin Park
>
> Commit title:
> s/ehci-s5p/ehci-exynos
Thank you for spotting this.
>
> > ---
> > Documentation/devicetree/bindings/usb/usb-ehci.txt | 35 +++
> > drivers/usb/host/ehci-exynos.c | 97
> +---
> > 2 files changed, 98 insertions(+), 34 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt
> > b/Documentation/devicetree/bindings/usb/usb-ehci.txt
> > index fa18612..413f7cd 100644
> > --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
> > +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
> > @@ -14,6 +14,10 @@ If controller implementation operates with big
> > endian descriptors, If both big endian registers and descriptors are
> > used by the controller implementation, "big-endian" property can be
> > specified instead of having both "big-endian-regs" and "big-endian-
> desc".
> > + - port: if in the SoC there are EHCI phys, they should be listed
> here.
> > +One phy per port. Each port should have its reg entry with a
> > +consecutive number. Also it should contain phys and phy-names
> entries
> > +specifying the phy used by the port.
> >
> > Example (Sequoia 440EPx):
> > ehci@e300 {
> > @@ -23,3 +27,34 @@ Example (Sequoia 440EPx):
> >reg = <0 e300 90 0 e390 70>;
> >big-endian;
> > };
> > +
> > +Example (Exynos 4212):
> > +ehci@1258 {
> > +compatible = "samsung,exynos4210-ehci";
> > +reg = <0x1258 0x2>;
> > +interrupts = <0 70 0>;
> > +clocks = <&clock 304>, <&clock 305>;
> > +clock-names = "usbhost", "otg";
> > +status = "disabled";
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +port@0 {
> > +reg = <0>;
> > +phys = <&usb2phy 1>;
> > +phy-names = "host";
> > +status = "disabled";
> > +};
> > +port@1 {
> > +reg = <1>;
> > +phys = <&usb2phy 2>;
> > +phy-names = "hsic0";
> > +status = "disabled";
> > +};
> > +port@2 {
> > +reg = <2>;
> > +phys = <&usb2phy 3>;
> > +phy-names = "hsic1";
> > +status = "disabled";
> > +};
> > +};
>
> Should we place above documentation in
> "Documentation/devicetree/bindings/usb/exynos-usb.txt" ?
> or is it something that i am missing.
Indeed, this should go to exynos-usb.txt instead of usb-ehci.txt.
Thanks!
> > +
> > diff --git a/drivers/usb/host/ehci-exynos.c
> > b/drivers/usb/host/ehci-exynos.c index d1d8c47..7c35501 100644
> > --- a/drivers/usb/host/ehci-exynos.c
> > +++ b/drivers/usb/host/ehci-exynos.c
> > @@ -19,12 +19,12 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include #include
> > #include -#include
> >
> > #include "ehci.h"
> >
> > @@ -42,10 +42,10 @@
> > static const char hcd_name[] = "ehci-exynos"; static struct
> > hc_driver __read_mostly exynos_ehci_hc_driver;
> >
> > +#define PHY_NUMBER 3
> > struct exynos_ehci_hcd {
> > struct clk *clk;
> > - struct usb_phy *phy;
> > - struct usb_otg *otg;
> > + struct phy *phy[PHY_NUMBER];
> > };
> >
> > #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd
> > *)(hcd_to_ehci(hcd)->priv) @@ -69,13 +69,43 @@ static void
> exynos_setup_vbus_gpio(struct platform_device *pdev)
> > dev_err(dev, "can't request ehci vbus gpio %d",
> gpio);
> > }
> >
> > +static int exynos_phys_on(struct phy *p[]) {
> > + int i;
> > + int ret = 0;
> > +
> > + for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
> > + if (p[i])
> > + ret = phy_power_on(p[i]);
> > + if (ret)
> > + for (i--; i > 0; i--)
> > + if (p[i])
> > + phy_power_off(p[i]);
>
> So we are turning off, say, port0 phy in case port1 phy power_on fails;
> can't we still leave a usb2.0 phy(a normal host phy) 'on' in case the
> HSIC phy fails ?
Currently all phys are can be either switched on or off. So if powering on
one phy fails (and exynos_phy_on returns an error code), I would expect
that no phy is switched on. I think that doing otherwise could leave
the phys in strange state - some phys are on, some are off and the power_on
call returned an error.
> > +
> > + return ret;
> > +}
> > +
> > +static int exynos_phys_off(struct phy *p[]) {
> > + int i;
> > + int ret = 0;
> > +
> > + for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
> > +
[PATCH] USB: iowarrior: fix spelling mistake in comment
Signed-off-by: Rahul Bedarkar
---
drivers/usb/misc/iowarrior.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c
index d36f34e..367725c 100644
--- a/drivers/usb/misc/iowarrior.c
+++ b/drivers/usb/misc/iowarrior.c
@@ -300,7 +300,7 @@ static ssize_t iowarrior_read(struct file *file, char
__user *buffer,
do {
atomic_set(&dev->overflow_flag, 0);
if ((read_idx = read_index(dev)) == -1) {
- /* queue emty */
+ /* queue empty */
if (file->f_flags & O_NONBLOCK)
return -EAGAIN;
else {
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 9/9] dts: Add usb2phy to Exynos 5250
Hi,
> From: Vivek Gautam [mailto:gautamvivek1...@gmail.com]
> Sent: Thursday, December 26, 2013 11:32 AM
>
> Hi Kamil,
>
>
> On Fri, Dec 20, 2013 at 6:54 PM, Kamil Debski
> wrote:
> > Add support to PHY of USB2 of the Exynos 5250 SoC.
> >
> > Signed-off-by: Kamil Debski
> > ---
> > arch/arm/boot/dts/exynos5250.dtsi | 33 ---
> > drivers/phy/phy-exynos5250-usb2.c | 64
> +
> > 2 files changed, 78 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> > b/arch/arm/boot/dts/exynos5250.dtsi
> > index 2f264ad..922e0ed 100644
> > --- a/arch/arm/boot/dts/exynos5250.dtsi
> > +++ b/arch/arm/boot/dts/exynos5250.dtsi
> > @@ -163,6 +163,11 @@
> > interrupts = <0 47 0>;
> > };
> >
> > + sys_syscon: syscon@1004 {
> > + compatible = "samsung,exynos5250-sys", "syscon";
> > + reg = <0x1005 0x5000>;
> > + };
> > +
> > pmu_syscon: syscon@1004 {
> > compatible = "samsung,exynos5250-pmu", "syscon";
> > reg = <0x1004 0x5000>; @@ -505,6 +510,14 @@
> >
> > clocks = <&clock 285>;
> > clock-names = "usbhost";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + port@0 {
> > + reg = <0>;
> > + phys = <&usb2_phy 1>;
> > + phy-names = "host";
> > + status = "ok";
> > + };
> > };
> >
> > usb@1212 {
> > @@ -516,19 +529,15 @@
> > clock-names = "usbhost";
> > };
> >
> > - usb2_phy: usbphy@1213 {
> > - compatible = "samsung,exynos5250-usb2phy";
> > + usb2_phy: phy@1213 {
> > + compatible = "samsung,exynos5250-usb2-phy";
> > reg = <0x1213 0x100>;
> > - clocks = <&clock 1>, <&clock 285>;
> > - clock-names = "ext_xtal", "usbhost";
> > - #address-cells = <1>;
> > - #size-cells = <1>;
> > - ranges;
> > -
> > - usbphy-sys {
> > - reg = <0x10040704 0x8>,
> > - <0x10050230 0x4>;
> > - };
> > + clocks = <&clock 285>, <&clock 1>, <&clock 1>,
> <&clock 1>,
> > +
> <&clock 1>;
> > + clock-names = "phy", "device", "host", "hsic0",
> "hsic1";
> > + #phy-cells = <1>;
> > + samsung,sysreg-phandle = <&sys_syscon>;
> > + samsung,pmureg-phandle = <&pmu_syscon>;
> > };
> >
> > amba {
> > diff --git a/drivers/phy/phy-exynos5250-usb2.c
> > b/drivers/phy/phy-exynos5250-usb2.c
> > index b9b3b98..337bf82 100644
> > --- a/drivers/phy/phy-exynos5250-usb2.c
> > +++ b/drivers/phy/phy-exynos5250-usb2.c
>
> Separate patches for dt and driver ?
> I think you wanted to move these changes to :
> [PATCH v5 7/9] phy: Add Exynos 5250 support to the Exynos USB 2.0 PHY
> driver
Good point. I am planning to reorganise this patchset to prevent breaking
git bisect. I wanted to wait for more comments to this version, so I could
address any issues that may be reported.
>
> > @@ -58,7 +58,13 @@
> > #define EXYNOS_5250_HOSTPHYCTRL2 0x20
>
> Shouldn't we leave the naming as EXYNOS_5250_HSICPHYCTRL2 instead of
> EXYNOS_5250_HOSTPHYCTRL2 ? That will go in sync with the user-manual
> too.
> and similar for EXYNOS_5250_HOSTPHYCTRL1 and below bit definitions too
> ?
Thank you for pointing this out.
> >
> > #define EXYNOS_5250_HOSTPHYCTRLX_REFCLKSEL_MASK(0x3
> << 23)
> > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKSEL_DEFAULT (0x2 << 23)
> > #define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_MASK(0x7f
> << 16)
> > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_12 (0x24 << 16)
> > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_15 (0x1c << 16)
> > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_16 (0x1a << 16)
> > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_19_2(0x15
> << 16)
> > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_20 (0x14 << 16)
> > #define EXYNOS_5250_HOSTPHYCTRLX_SIDDQ BIT(6)
> > #define EXYNOS_5250_HOSTPHYCTRLX_FORCESLEEPBIT(5)
> > #define EXYNOS_5250_HOSTPHYCTRLX_FORCESUSPEND BIT(4)
> > @@ -191,13 +197,14 @@ static void exynos5250_isol(struct
> samsung_usb2_phy_instance *inst, bool on)
> > regmap_update_bits(drv->reg_pmu, offset, mask, on ? 0 :
> mask);
> > }
> >
> > -static void exynos5250_phy_pwr(struct samsung_usb2_phy_instance
> > *inst, bool on)
> > +static int exynos5250_power_on(struct samsung_usb2_phy_instance
> > +*inst)
>
> void ? we really don't have much to return in this function.
Initially the idea was to enable the return of an error code. However,
I see that for all currently supported SoC
[PATCH] USB: yurex: fix spelling mistake in comment
fix spelling mistake in comment
Signed-off-by: Rahul Bedarkar
---
drivers/usb/misc/yurex.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/yurex.c b/drivers/usb/misc/yurex.c
index b6ab515..0609114 100644
--- a/drivers/usb/misc/yurex.c
+++ b/drivers/usb/misc/yurex.c
@@ -464,7 +464,7 @@ static ssize_t yurex_write(struct file *file, const char
*user_buffer, size_t co
goto error;
mutex_lock(&dev->io_mutex);
- if (!dev->interface) { /* alreaday disconnected */
+ if (!dev->interface) { /* already disconnected */
mutex_unlock(&dev->io_mutex);
retval = -ENODEV;
goto error;
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.13 mainline, USB 2.0 support is broken on intel z77 chipset's motherboard
On Mon, 30 Dec 2013, Vi0L0 wrote: > Motherboard: MSI Z77A-GD65 > BIOS: 10.11 > CPU: intel i7 3770K (ivy) > > GNU/Linux: Arch Linux x86_64 (with systemd) > > Linux: mainline 3.13 rc <= 5, I will test rc6 later today > > > Problem: > Connecting most of the devices to USB 2.0 port is causing instabillity > of the whole system. > > > It's easiest to notice when connecting mouse and starting DM/X. > Keyborad seems to work fine so I'm able to login to tty and do > anything there, but whenever I start DM/X and trying to move mouse' > cursour the X freeze. Establishing the new ssh connection then is > impossible (so there's possibility of kernel panic in the background, > after getting back home I will recompile kernel with some tracers to > be sure). Fortunatelly ssh session established before DM/X start is > alive so I can trace whatever you need, still it's impossible to kill > X process and perform system' reboot. > > I've tested 2 mouses with same results: A4tech X750 and Razor DeathAdder > Also checked out 2 different DMs: KDM and GDM. > > Connecting mouse to USB 3.0 port is solving the mouse' problem, but > then other devices connected to USB 2.0 ports are causing problems - > ie. using Logitech webcam for some short time is making KDE 4.12.0 > freeze. > > There were no such problems with kernels < 3.13, fresh 3.12.6 works > perfectly fine. > > I'm sending some basic informations (with my kernel config) in the > attachments, if you need more just let me know. > > There are some warns/errors related to fglrx but feel free to skip it, > i've tested oss radeon and get same results. In fact, none of the error messages in the log seem to be related to USB. Since you know that the 3.12 kernel works okay, maybe you can use "git bisect" to find the change that introduced this problem. 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
order of rx/tx sequence
Hi, I have a case in which the class layer has tx and rx urbs in sequence, and the class layer expects the rx complete() should be called after the tx complete() is called. But due to scheduling, I guess, sometimes the tx complete() is called after rx_complete(), which confuses the class layer. So normal case is - 1. submit tx/OUT urb; - 2. submit rx/IN urb; - 3. the device received the tx/OUT packet; - 4. the device sent rx/IN packet; - 5. tx complete() is called; - 6. rx complete() is called; But in failure case, step 5 & 6 are swapped. Is this considered as a bug in the host controller driver, which should guarantee the order of the sequence? Or it is a bug in the class layer, which should not rely on the behavior in core and host controller drivers? Thanks, -Bin. -- 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: order of rx/tx sequence
On Mon, Dec 30, 2013 at 11:46:13AM -0600, Bin Liu wrote: > Hi, > > I have a case in which the class layer has tx and rx urbs in sequence, What "class layer"? > and the class layer expects the rx complete() should be called after > the tx complete() is called. What driver expects this? Sounds like a bug in the driver. > But due to scheduling, I guess, sometimes the tx complete() is called > after rx_complete(), which confuses the class layer. > > So normal case is > - 1. submit tx/OUT urb; > - 2. submit rx/IN urb; > - 3. the device received the tx/OUT packet; > - 4. the device sent rx/IN packet; > - 5. tx complete() is called; > - 6. rx complete() is called; > > But in failure case, step 5 & 6 are swapped. That's not a "failure", it's a "bug in the driver to assume something like this would happen". > Is this considered as a bug in the host controller driver, which > should guarantee the order of the sequence? Or it is a bug in the > class layer, which should not rely on the behavior in core and host > controller drivers? Again, what do you mean by "class layer"? 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: order of rx/tx sequence
Hi Greg, On Mon, Dec 30, 2013 at 12:04 PM, Greg KH wrote: > On Mon, Dec 30, 2013 at 11:46:13AM -0600, Bin Liu wrote: >> Hi, >> >> I have a case in which the class layer has tx and rx urbs in sequence, > > What "class layer"? This is a third party proprietary wifi driver sits on top of the usb stack, so I called it class (application) layer. Sorry for the confusion. BTY, I don't have the driver source code. > >> and the class layer expects the rx complete() should be called after >> the tx complete() is called. > > What driver expects this? Sounds like a bug in the driver. I was told the wifi driver is designed like that. And it works on other platforms (I don't know which specifically) including x86, but failed on MUSB+CPPI41. > >> But due to scheduling, I guess, sometimes the tx complete() is called >> after rx_complete(), which confuses the class layer. >> >> So normal case is >> - 1. submit tx/OUT urb; >> - 2. submit rx/IN urb; >> - 3. the device received the tx/OUT packet; >> - 4. the device sent rx/IN packet; >> - 5. tx complete() is called; >> - 6. rx complete() is called; >> >> But in failure case, step 5 & 6 are swapped. > > That's not a "failure", it's a "bug in the driver to assume something > like this would happen". By failure, I meant the wifi driver stopped working due the swapping above. Thanks, -Bin. > >> Is this considered as a bug in the host controller driver, which >> should guarantee the order of the sequence? Or it is a bug in the >> class layer, which should not rely on the behavior in core and host >> controller drivers? > > Again, what do you mean by "class layer"? > > 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: order of rx/tx sequence
On Mon, Dec 30, 2013 at 12:39:11PM -0600, Bin Liu wrote: > Hi Greg, > > On Mon, Dec 30, 2013 at 12:04 PM, Greg KH wrote: > > On Mon, Dec 30, 2013 at 11:46:13AM -0600, Bin Liu wrote: > >> Hi, > >> > >> I have a case in which the class layer has tx and rx urbs in sequence, > > > > What "class layer"? > > This is a third party proprietary wifi driver sits on top of the usb > stack, so I called it class (application) layer. Sorry for the > confusion. BTY, I don't have the driver source code. USB drivers are not allowed to be "closed source", so there's nothing we can do here to help you out, sorry. Please contact the authors of the driver and work with them on this. Best of luck, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: order of rx/tx sequence
Hi Greg, On Mon, Dec 30, 2013 at 12:51 PM, Greg KH wrote: > On Mon, Dec 30, 2013 at 12:39:11PM -0600, Bin Liu wrote: >> Hi Greg, >> >> On Mon, Dec 30, 2013 at 12:04 PM, Greg KH wrote: >> > On Mon, Dec 30, 2013 at 11:46:13AM -0600, Bin Liu wrote: >> >> Hi, >> >> >> >> I have a case in which the class layer has tx and rx urbs in sequence, >> > >> > What "class layer"? >> >> This is a third party proprietary wifi driver sits on top of the usb >> stack, so I called it class (application) layer. Sorry for the >> confusion. BTY, I don't have the driver source code. > > USB drivers are not allowed to be "closed source", so there's nothing we > can do here to help you out, sorry. Please contact the authors of the > driver and work with them on this. I am fully aware of this, and not asking the community to help me to solve the particular issue. My question was that is it controller driver bug which does not keep the order or wifi driver bug which relies on the non-guaranteed order. I believe you have already answered me. Thanks. Regards, -Bin. > > Best of luck, > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/12] libusbg: Add error handling to gadget_read_string.
On 12/19/2013 06:29 AM, Krzysztof Opasiak wrote:
Add error handling when gadget_read_buf return NULL.
If read of string fails, the string should be set as empty.
It's typical to put () at the end of function names in log messages.
Also s/return/returns/
"Add error handling when gadget_read_buf() returns NULL.
If read of string fails, the string should be set as empty."
Signed-off-by: Krzysztof Opasiak
---
Changes since v1:
- Remove additional check of p variable
src/gadget.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/gadget.c b/src/gadget.c
index f613c3e..d225af0 100644
--- a/src/gadget.c
+++ b/src/gadget.c
@@ -121,11 +121,18 @@ static int gadget_read_int(char *path, char *name, char
*file, int base)
static void gadget_read_string(char *path, char *name, char *file, char *buf)
{
- char *p;
+ char *p = NULL;
Initializer isn't necessary.
+
+ p = gadget_read_buf(path, name, file, buf);
+ //check whether read was successful
Make /* */ (assuming Matt wants kernel style).
+ if (p != NULL) {
+ if ((p = strchr(buf, '\n')) != NULL)
+ *p = '\0';
+ } else {
+ //Set this as empty string
/* */
+ *buf = '\0';
+ }
- gadget_read_buf(path, name, file, buf);
- if ((p = strchr(buf, '\n')) != NULL)
- *p = '\0';
}
static void gadget_write_buf(char *path, char *name, char *file, char *buf)
@@ -185,10 +192,14 @@ static void gadget_parse_function_attrs(struct function
*f)
case F_RNDIS:
gadget_read_string(f->path, f->name, "dev_addr", str_addr);
addr = ether_aton(str_addr);
maybe ether_aton_r() here.
- memcpy(&f->attr.net.dev_addr, addr, 6);
+ if (addr)
+ memcpy(&f->attr.net.dev_addr, addr, sizeof(struct
ether_addr));
+
gadget_read_string(f->path, f->name, "host_addr", str_addr);
addr = ether_aton(str_addr);
ether_aton_r()
- memcpy(&f->attr.net.host_addr, addr, 6);
+ if(addr)
+ memcpy(&f->attr.net.host_addr, addr, sizeof(struct
ether_addr));
+
gadget_read_string(f->path, f->name, "ifname",
f->attr.net.ifname);
f->attr.net.qmult = gadget_read_dec(f->path, f->name, "qmult");
break;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: gadget: at91_udc: fix usb_ep_set_maxpacket_limit() usage
In commit e117e742 (usb: gadget: add "maxpacket_limit" field to
struct usb_ep) usb_ep_set_maxpacket_limit() function was used incorrectly
in at91udc_probe() function, when maxpacket value should be set to
instances of struct at91_ep, not struct usb_ep.
This patch fix this by reverting original code.
Signed-off-by: Robert Baldyga
Signed-off-by: Kyungmin Park
---
Hello,
This patch is reply for problem discussed here:
http://www.spinics.net/lists/linux-usb/msg100241.html
Best regards
Robert Baldyga
Samsung R&D Institute Poland
drivers/usb/gadget/at91_udc.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
index 0353b64..dc7a7c2 100644
--- a/drivers/usb/gadget/at91_udc.c
+++ b/drivers/usb/gadget/at91_udc.c
@@ -1759,15 +1759,15 @@ static int at91udc_probe(struct platform_device *pdev)
/* newer chips have more FIFO memory than rm9200 */
if (cpu_is_at91sam9260() || cpu_is_at91sam9g20()) {
- usb_ep_set_maxpacket_limit(&udc->ep[0], 64);
- usb_ep_set_maxpacket_limit(&udc->ep[3], 64);
- usb_ep_set_maxpacket_limit(&udc->ep[4], 512);
- usb_ep_set_maxpacket_limit(&udc->ep[5], 512);
+ udc->ep[0].maxpacket = 64;
+ udc->ep[3].maxpacket = 64;
+ udc->ep[4].maxpacket = 512;
+ udc->ep[5].maxpacket = 512;
} else if (cpu_is_at91sam9261() || cpu_is_at91sam9g10()) {
- usb_ep_set_maxpacket_limit(&udc->ep[3], 64);
+ udc->ep[3].maxpacket = 64;
} else if (cpu_is_at91sam9263()) {
- usb_ep_set_maxpacket_limit(&udc->ep[0], 64);
- usb_ep_set_maxpacket_limit(&udc->ep[3], 64);
+ udc->ep[0].maxpacket = 64;
+ udc->ep[3].maxpacket = 64;
}
udc->udp_baseaddr = ioremap(res->start, resource_size(res));
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
