date:20131101

Re: CSW endpoint status returned STALL after BOT MSC Reset

2013-11-01 Thread Pratyush Anand

On Thu, Oct 31, 2013 at 12:32:29AM +0800, Alan Stern wrote:
 On Wed, 30 Oct 2013, Alan Stern wrote:
 
  I think you have found a bug in the dwc3 driver.
 
  At this point, because the IGNORE_BULK_OUT bit is set, g_mass_storage
  issues a usb_ep_clear_halt() call for the bulk-in (CSW) endpoint.  
  This tells the dwc3 driver to change the endpoint's status back to 0:
  
  if (test_and_clear_bit(IGNORE_BULK_OUT,
 common-fsg-atomic_bitflags))
  usb_ep_clear_halt(common-fsg-bulk_in);
  
   INFO  Retrieving status on CBW endpoint
   INFO  CBW endpoint status = 0x0
   INFO  Retrieving status on CSW endpoint
   INFO  CSW endpoint status = 0x1
  
  But the status is still set to 1.  Clearly this is a bug in dwc3.
 
 And indeed it is.  The dwc3 driver does not implement the wedge method 
 correctly.  This patch should fix it.  Let me know how it works.
 
 Alan Stern
 
 
 
 Index: usb-3.12/drivers/usb/dwc3/ep0.c
 ===
 --- usb-3.12.orig/drivers/usb/dwc3/ep0.c
 +++ usb-3.12/drivers/usb/dwc3/ep0.c
 @@ -459,6 +459,8 @@ static int dwc3_ep0_handle_feature(struc
   dep = dwc3_wIndex_to_dep(dwc, wIndex);
   if (!dep)
   return -EINVAL;
 + if (set == 0  (dep-flags  DWC3_EP_WEDGE))
 + break;
   ret = __dwc3_gadget_ep_set_halt(dep, set);
   if (ret)
   return -EINVAL;
 Index: usb-3.12/drivers/usb/dwc3/gadget.c
 ===
 --- usb-3.12.orig/drivers/usb/dwc3/gadget.c
 +++ usb-3.12/drivers/usb/dwc3/gadget.c
 @@ -1200,9 +1200,6 @@ int __dwc3_gadget_ep_set_halt(struct dwc
   else
   dep-flags |= DWC3_EP_STALL;
   } else {
 - if (dep-flags  DWC3_EP_WEDGE)
 - return 0;
 -
   ret = dwc3_send_gadget_ep_cmd(dwc, dep-number,
   DWC3_DEPCMD_CLEARSTALL, params);
   if (ret)
 @@ -1210,7 +1207,7 @@ int __dwc3_gadget_ep_set_halt(struct dwc
   value ? set : clear,
   dep-name);
   else
 - dep-flags = ~DWC3_EP_STALL;
 + dep-flags = ~(DWC3_EP_STALL | DWC3_EP_WEDGE);
   }
  
   return ret;

Tested-by: Pratyush Anand pratyush.an...@st.com 

However, I noticed that the same error recovery test fails even after
above patch if dwc3 dbg/vdbg messages are enabled. However, reason is
not in dwc3 driver, rather in mass storage driver.

Any specific reason for returning DELAYED_STATUS and not 
USB_GADGET_DELAYED_STATUS
while handling bulk reset request.? It seems a bug to me in mass
storage driver.


Test passes with dwc3 debug messages enabled if following patch is
used.

diff --git a/drivers/usb/gadget/f_mass_storage.c 
b/drivers/usb/gadget/f_mass_storage.c
index 1b443e3..915024b 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -634,7 +634,7 @@ static int fsg_setup(struct usb_function *f,
 */
DBG(fsg, bulk reset request\n);
raise_exception(fsg-common, FSG_STATE_RESET);
-   return DELAYED_STATUS;
+   return USB_GADGET_DELAYED_STATUS;
 
case USB_BULK_GET_MAX_LUN_REQUEST:
if (ctrl-bRequestType !=

Regards
Pratyush
--
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: gadget: should usb_ep_enable() clear EP STALL?

2013-11-01 Thread Johannes Stezenbach

Hi,

On Thu, Oct 31, 2013 at 10:21:28AM -0500, Felipe Balbi wrote:
 On Thu, Oct 31, 2013 at 04:02:20PM +0100, Johannes Stezenbach wrote:
  On Wed, Oct 30, 2013 at 10:44:30PM +0100, Johannes Stezenbach wrote:
   On Wed, Oct 30, 2013 at 12:54:15PM -0500, Felipe Balbi wrote:

Do you have any patches on f_sourcesink which might have caused this
bug ?
   
   No patches, but maybe out of date code.  However, I'm looking
   at current git master, I see you clear dep-flags but I don't
   see any dwc3_send_gadget_ep_cmd(DWC3_DEPCMD_CLEARSTALL)?
  
  I just tried to add __dwc3_gadget_ep_set_halt(dep, false);
  right before the call to __dwc3_gadget_ep_disable()
  in dwc3_gadget_ep_disable(), it seems to fix the issue.
  
  It is easily reproducible using the attached Python script, it
  should print:
  
  $ ./dwc3test.py
  [Errno 32] Pipe error (expected)
  OK
  
  but instead prints:
  
  $ ./dwc3test.py
  [Errno 32] Pipe error (expected)
  [Errno 32] Pipe error

 can you send a patch to dwc3 so we can review and possibly apply ?

I could create a patch, but I can't test it with the current upstream
version because I'm stuck with an old 3.4.x kernel and dwc3 version
(not under my control). So my idea was to make it
reproducible for you so you can create the patch and test it.
If you want you can add Reported-by: or Suggested-by:.


Thanks,
Johannes
--
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: choice =y selection becomes lost after having multiple entries =m with depends on

2013-11-01 Thread Dirk Gouders

Yann E. MORIN yann.morin.1...@free.fr writes:

 Dirk, All,

 On 2013-10-30 15:26 +0100, Dirk Gouders spake thusly:
 Dirk Gouders d...@gouders.net writes:
 [--SNIP--]
 below is a patch that prevents choice_values to appear in the list if
 they depend on 'm' symbols and the choice symbol is set to 'y'.  I would
 be glad if you could have a look at it.

 Next time, could you use 'git send-email' to send your patches, it makes
 reviewing and commenting much simpler.

 Also, it does not mangle the patch commit, and thus makes it easier to
 apply with 'git am'.

 Yann, in this patch I wrote  if (choice_sym != NULL... -- I guess that
 is one point that you will probably remark in the previous patch.

 When we check that a pointer is valid, there's no reason to check it
 against NULL, just:
 if (choice_sym  choice_sym-...)

 Another point is that all the conditionals in both patches could be

 I am not sure I understand what issue this patch(es) are supposed to
 fix.

 What bothers me is that they touch two different places in the code, yet
 have the same subject, so it seems both are tryiong to fix the same
 issue.

 Do you intend that both patches should be applied, or that this second
 one supersedes the previous one?

 limited to only those choice_values that are of type tristate but I
 think that makes the code even harder to read...

 Yes, and it does not matter since non-trisates are necessarily booleans,
 and those are covered since they will never be '== mod', so their
 visibility was, and still is, properly handled.

 From 677f5830588749cbb0bdb0568cbdaba271937c8d Mon Sep 17 00:00:00 2001
 From: Dirk Gouders d...@gouders.net
 Date: Wed, 30 Oct 2013 15:06:05 +0100
 Subject: [PATCH 2/2] kconfig/symbol.c: handle visibility of choice_values 
 that
  depend on 'm' symbols
 
 If choice_values depend on symbols that are set to 'm' then these
 choice_values should not be visible in choice lists if the choice
 symbol is set to 'y'.  See USB Gadget Drivers, for example.

 I liked your previous commit message better, because it had a test-case
 that did exhibit the problem.

 Can you please:
   - confirm which patch/es is/are needed
   - update the commit log/s back with the test-case/s
   - resend needed patch/es

Thanks for the comments, Yann, and apologies for the confusion.
Patch v3 comes in a minute and hopefully in a satisfactory format.

You are right, both patches that I sent fix the same (reported) problem
but v2 seems to be more sensible to me, because it causes non-selectable
choices to not even appear in choice lists.  However, it uses the
side-effect or natural consequence that a tristate choice_value's
value is set to no in sym_calc_value() if it's visibility is no.  So,
this seems to be a bit subtle and I tried to address it in the new commit
message.

I probably noticed another problem with those tristate choices: if
a non-optional tristate choice is set to 'm', then the default value is
not selected if no other is and I think that is not correct, but I'd
prefer to hear if others also think that this is a problem that should
be fixed.

Dirk



--
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] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

2013-11-01 Thread Dirk Gouders

If choices consist of choice_values that depend on symbols set to 'm',
those choice_values are not set to 'n' if the choice is changed from
'm' to 'y' (in which case only one active choice_value is allowed).
Those values are also written to the config file causing modules to be
built when they should not.

The following config can be used to reproduce and examine the problem:

config modules
boolean modules
default y
option modules

config dependency
tristate Dependency
default m

choice
prompt Tristate Choice
default choice0

config choice0
tristate Choice 0

config choice1
tristate Choice 1
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set
to 'm' to 'n' if the corresponding choice is set to 'y'.  This makes
them disappear from the choice list and will also cause the
choice_values' value set to 'n' in sym_calc_value() and as a result
they are written as not set to the resulting .config file.

Reported-by: Sebastian Andrzej Siewior bige...@linutronix.de
Signed-off-by: Dirk Gouders d...@gouders.net
---
 scripts/kconfig/symbol.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 22a3c40..32bbaa3 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -188,12 +188,23 @@ static void sym_validate_range(struct symbol *sym)
 static void sym_calc_visibility(struct symbol *sym)
 {
struct property *prop;
+   struct symbol *choice_sym = NULL;
tristate tri;
 
/* any prompt visible? */
tri = no;
+
+   if (sym_is_choice_value(sym))
+   choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
+
for_all_prompts(sym, prop) {
prop-visible.tri = expr_calc_value(prop-visible.expr);
+   /*
+* choice_values with visibility 'mod' are not visible if the
+* corresponding choice's value is 'yes'.
+*/
+   if (prop-visible.tri == mod  (choice_sym  
choice_sym-curr.tri == yes))
+   prop-visible.tri = no;
tri = EXPR_OR(tri, prop-visible.tri);
}
if (tri == mod  (sym-type != S_TRISTATE || modules_val == no))
-- 
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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Frank Schäfer

Am 31.10.2013 21:56, schrieb Johan Hovold:
 On Thu, Oct 31, 2013 at 09:26:06PM +0100, Johan Hovold wrote:
 On Thu, Oct 31, 2013 at 7:45 PM, Frank Schäfer wrote:
 Am 31.10.2013 13:30, schrieb Mika Westerberg:
 On Thu, Oct 31, 2013 at 01:02:56PM +0100, Frank Schäfer wrote:
 2) comment out the following line at the end of
 pl2303_baudrate_encode_divisor_HXD():

 baud = 1200 * 32 / ((1  buf[1]) * buf[0]);
 This seems to fix the problem :)

 Once the line is commented out, the serial console works fine with the
 speeds I've been testing (115200, 230400 and 460800).
 Urgh... so it seems getty gets confused if the actually set baud rate is
 reported back.
 The kernel still reports the standard baud rate (e.g. B230400), it only
 sets the exact value in the c_ispeed and c_ospeed fields of the termios
 struct.
 So even if getty doesn't support non-standard baud rates, everything
 should be fine.
 I'll have to take a closer look at this issue later.
 I think I know what's going on. The pl2303 is known to loose bytes when
 changing the termios settings (see commit bf5e5834b (pl2303: Fix mode
 switching regression)).

 Your commit 57ce61aad (usb: pl2303: fix+improve the divisor based baud
 rate encoding method) introduced a regression when it started reporting
 back the baudrates determined by the divisor algorithm.

 Since that commit, tty_termios_hw_change(tty-termios, old_termios)
 will always return true -- even when userspace isn't requesting a
 different baudrate.
Hmm... so there is a bug in tty_termios_hw_change() ?

 Ok, so now let's see if the fixed/improved divisor based method also
 works for the HXD chip if we don't report the actually set baud rate.
 Try commenting out the line

 baud = 1200 * 32 / ((1  A) * B);

 at the end of pl2303_baudrate_encode_divisor() of a clean 3.12-rc kernel.
 It's unlikely that this makes baud rates  115200 working, but testing
 both cases doesn't harm. ;)
 I can trigger the problem here with my HXD/EA/RA/SA-device as well, and
 I've verified that not returning the calculated baudrate makes this
 particular issue *seem* to go away.

 This obviously has to be fixed or reverted quickly.
Indeed.

 Greg, what do you say about this? Is reverting for 3.12 the correct way
 to deal with this, and make sure these fairly invasive patches get some
 more testing before being reapplied?

 Reverting would mean reverting a whole bunch of commits though, as they
 appear to depend quite heavily on each other:

 7d26a78 USB: pl2303: distinguish between original and cloned HX chips
 034d152 pl2303: improve the chip type detection/distinction
 a77a8c2 pl2303: improve the chip type information output on startup
 73b583a pl2303: simplify the else-if contruct for type_1 chips in 
 pl2303_startup()
 c23bda3 usb: pl2303: add two comments concerning the supported baud rates 
 with HX chips
 61fa8d6 usb: pl2303: also use the divisor based baud rate encoding method for 
 baud rates  115200 with HX chips
 b5c16c6 usb: pl2303: increase the allowed baud rate range for the divisor 
 based encoding method
 e917ba0 usb: pl2303: move the two baud rate encoding methods to separate 
 functions
 b9208c7 usb: pl2303: remove 50 baud from the list of standard baud rates
 75417d9 usb: pl2303: do not round to the next nearest standard baud rate for 
 the divisor based baud rate encoding method
 57ce61a usb: pl2303: fix+improve the divsor based baud rate encoding method
 b8bdad6 USB: pl2303: restrict the divisor based baud rate encoding method to 
 the HX chip type
No need to revert all these patches, removing the calculation of the
resulting baud rateat the end of fcn pl2303_baudrate_encode_divisor()
should fix this issue.
The remaining question is, does the HXD work with the improved divisor
method or do we have to reintroduce the old method ?

Regards,
Frank

 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

[PATCH net-next 14/24] net: cdc_ncm: remove ethtool ops

2013-11-01 Thread Bjørn Mork

No need to keep this code duplicated from usbnet.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |   26 --
 1 file changed, 26 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index c90d843..8fc1a06 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -53,8 +53,6 @@
 #include linux/usb/cdc.h
 #include linux/usb/cdc_ncm.h
 
-#defineDRIVER_VERSION  14-Mar-2012
-
 #if IS_ENABLED(CONFIG_USB_NET_CDC_MBIM)
 static bool prefer_mbim = true;
 #else
@@ -68,18 +66,6 @@ static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx 
*ctx);
 static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
 static struct usb_driver cdc_ncm_driver;
 
-static void
-cdc_ncm_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
-{
-   struct usbnet *dev = netdev_priv(net);
-
-   strlcpy(info-driver, dev-driver_name, sizeof(info-driver));
-   strlcpy(info-version, DRIVER_VERSION, sizeof(info-version));
-   strlcpy(info-fw_version, dev-driver_info-description,
-   sizeof(info-fw_version));
-   usb_make_path(dev-udev, info-bus_info, sizeof(info-bus_info));
-}
-
 static u8 cdc_ncm_setup(struct usbnet *dev)
 {
struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0];
@@ -360,16 +346,6 @@ static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
kfree(ctx);
 }
 
-static const struct ethtool_ops cdc_ncm_ethtool_ops = {
-   .get_drvinfo = cdc_ncm_get_drvinfo,
-   .get_link = usbnet_get_link,
-   .get_msglevel = usbnet_get_msglevel,
-   .set_msglevel = usbnet_set_msglevel,
-   .get_settings = usbnet_get_settings,
-   .set_settings = usbnet_set_settings,
-   .nway_reset = usbnet_nway_reset,
-};
-
 int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 
data_altsetting)
 {
const struct usb_cdc_union_desc *union_desc = NULL;
@@ -499,8 +475,6 @@ advance:
if (cdc_ncm_setup(dev))
goto error2;
 
-   dev-net-ethtool_ops = cdc_ncm_ethtool_ops;
-
usb_set_intfdata(ctx-data, dev);
usb_set_intfdata(ctx-control, dev);
 
-- 
1.7.10.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

[PATCH net-next 05/24] net: cdc_ncm: remove redundant netdev field

2013-11-01 Thread Bjørn Mork

Too many pointers back and forth are likely to confuse developers,
creating subtle bugs whenever we forget to syncronize them all.

As a usbnet driver, we should stick with the standard struct
usbnet fields as much as possible.  The netdevice is one such
field.

Cc: Greg Suarez gsua...@smithmicro.com
Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_mbim.c  |2 +-
 drivers/net/usb/cdc_ncm.c   |   73 ++-
 include/linux/usb/cdc_ncm.h |3 +-
 3 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 253472a..af76aaf 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -175,7 +175,7 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet 
*dev, struct sk_buff *skb
}
 
spin_lock_bh(ctx-mtx);
-   skb_out = cdc_ncm_fill_tx_frame(ctx, skb, sign);
+   skb_out = cdc_ncm_fill_tx_frame(dev, skb, sign);
spin_unlock_bh(ctx-mtx);
return skb_out;
 
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index a989bd5..e39e767 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -80,8 +80,9 @@ cdc_ncm_get_drvinfo(struct net_device *net, struct 
ethtool_drvinfo *info)
usb_make_path(dev-udev, info-bus_info, sizeof(info-bus_info));
 }
 
-static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
+static u8 cdc_ncm_setup(struct usbnet *dev)
 {
+   struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0];
u32 val;
u8 flags;
u8 iface_no;
@@ -90,7 +91,6 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
u16 ntb_fmt_supported;
u32 min_dgram_size;
u32 min_hdr_size;
-   struct usbnet *dev = netdev_priv(ctx-netdev);
 
iface_no = ctx-control-cur_altsetting-desc.bInterfaceNumber;
 
@@ -285,8 +285,8 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
}
 
 max_dgram_err:
-   if (ctx-netdev-mtu != (ctx-max_datagram_size - eth_hlen))
-   ctx-netdev-mtu = ctx-max_datagram_size - eth_hlen;
+   if (dev-net-mtu != (ctx-max_datagram_size - eth_hlen))
+   dev-net-mtu = ctx-max_datagram_size - eth_hlen;
 
return 0;
 }
@@ -375,11 +375,10 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
 
hrtimer_init(ctx-tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
ctx-tx_timer.function = cdc_ncm_tx_timer_cb;
-   ctx-bh.data = (unsigned long)ctx;
+   ctx-bh.data = (unsigned long)dev;
ctx-bh.func = cdc_ncm_txpath_bh;
atomic_set(ctx-stop, 0);
spin_lock_init(ctx-mtx);
-   ctx-netdev = dev-net;
 
/* store ctx pointer in device data field */
dev-data[0] = (unsigned long)ctx;
@@ -477,7 +476,7 @@ advance:
goto error2;
 
/* initialize data interface */
-   if (cdc_ncm_setup(ctx))
+   if (cdc_ncm_setup(dev))
goto error2;
 
/* configure data interface */
@@ -669,9 +668,9 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct 
cdc_ncm_ctx *ctx, struct sk_
 }
 
 struct sk_buff *
-cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 
sign)
+cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
 {
-   struct usbnet *dev = netdev_priv(ctx-netdev);
+   struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0];
struct usb_cdc_ncm_nth16 *nth16;
struct usb_cdc_ncm_ndp16 *ndp16;
struct sk_buff *skb_out;
@@ -695,7 +694,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct 
sk_buff *skb, __le32 sign)
if (skb_out == NULL) {
if (skb != NULL) {
dev_kfree_skb_any(skb);
-   ctx-netdev-stats.tx_dropped++;
+   dev-net-stats.tx_dropped++;
}
goto exit_no_skb;
}
@@ -733,12 +732,12 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct 
sk_buff *skb, __le32 sign)
/* won't fit, MTU problem? */
dev_kfree_skb_any(skb);
skb = NULL;
-   ctx-netdev-stats.tx_dropped++;
+   dev-net-stats.tx_dropped++;
} else {
/* no room for skb - store for later */
if (ctx-tx_rem_skb != NULL) {
dev_kfree_skb_any(ctx-tx_rem_skb);
-   ctx-netdev-stats.tx_dropped++;
+   dev-net-stats.tx_dropped++;
}
ctx-tx_rem_skb = skb;
ctx-tx_rem_sign = sign;
@@ -771,7 +770,7 @@

[PATCH net-next 12/24] net: cdc_ncm: no point in filling up the NTBs if we send ZLPs

2013-11-01 Thread Bjørn Mork

Padding NTBs to max size is part of the support for devices
optimizing their DMA transfers. This optimization depends on
max sized NTBs not being ZLP terminated. So we are much better
off dropping the padding if we are going to send a ZLP anyway.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 5aa3e60..42c8620 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -800,8 +800,12 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff 
*skb, __le32 sign)
 * would be more efficient for USB HS mobile device with DMA
 * engine to receive a full size NTB, than canceling DMA
 * transfer and receiving a short packet.
+*
+* This optimization support is pointless if we end up sending
+* a ZLP after full sized NTBs.
 */
-   if (skb_out-len  CDC_NCM_MIN_TX_PKT)
+   if (!(dev-driver_info-flags  FLAG_SEND_ZLP) 
+   skb_out-len  CDC_NCM_MIN_TX_PKT)
memset(skb_put(skb_out, ctx-tx_max - skb_out-len), 0,
   ctx-tx_max - skb_out-len);
else if ((skb_out-len % dev-maxpacket) == 0)
-- 
1.7.10.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

[PATCH net-next 20/24] net: cdc_ncm: drop extern from header declarations

2013-11-01 Thread Bjørn Mork

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 include/linux/usb/cdc_ncm.h |   12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index cad54ad..2300f74 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -119,11 +119,11 @@ struct cdc_ncm_ctx {
u16 connected;
 };
 
-extern u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface 
*intf);
-extern int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, 
u8 data_altsetting);
-extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
-extern struct sk_buff *cdc_ncm_fill_tx_frame(struct usbnet *dev, struct 
sk_buff *skb, __le32 sign);
-extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff 
*skb_in);
-extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
+u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf);
+int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 
data_altsetting);
+void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
+struct sk_buff *cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, 
__le32 sign);
+int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
+int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
 
 #endif /* __LINUX_USB_CDC_NCM_H */
-- 
1.7.10.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

[PATCH net-next 06/24] net: cdc_ncm: remove unused udev field

2013-11-01 Thread Bjørn Mork

We already use the usbnet udev field everywhere this could have
been used.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c   |2 --
 include/linux/usb/cdc_ncm.h |1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index e39e767..9cdd762 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -388,8 +388,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
buf = intf-cur_altsetting-extra;
len = intf-cur_altsetting-extralen;
 
-   ctx-udev = dev-udev;
-
/* parse through descriptors associated with control interface */
while ((len  0)  (buf[0]  2)  (buf[0] = len)) {
 
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 5c47bd9..059dcc9 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -98,7 +98,6 @@ struct cdc_ncm_ctx {
const struct usb_cdc_union_desc *union_desc;
const struct usb_cdc_ether_desc *ether_desc;
 
-   struct usb_device *udev;
struct usb_interface *control;
struct usb_interface *data;
 
-- 
1.7.10.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

[PATCH net-next 18/24] net: cdc_ncm: log signatures in hex

2013-11-01 Thread Bjørn Mork

These signatures are well known bit patterns, mostly made up
of ascii characters.  Mentally parsing works best if they
are printed in hex.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 73faf05..d49e4c5 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -883,8 +883,9 @@ int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct 
sk_buff *skb_in)
nth16 = (struct usb_cdc_ncm_nth16 *)skb_in-data;
 
if (le32_to_cpu(nth16-dwSignature) != USB_CDC_NCM_NTH16_SIGN) {
-   pr_debug(invalid NTH16 signature %u\n,
-   le32_to_cpu(nth16-dwSignature));
+   netif_dbg(dev, rx_err, dev-net,
+ invalid NTH16 signature %#010x\n,
+ le32_to_cpu(nth16-dwSignature));
goto error;
}
 
@@ -972,8 +973,9 @@ next_ndp:
ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in-data + ndpoffset);
 
if (le32_to_cpu(ndp16-dwSignature) != USB_CDC_NCM_NDP16_NOCRC_SIGN) {
-   pr_debug(invalid DPT16 signature %u\n,
-le32_to_cpu(ndp16-dwSignature));
+   netif_dbg(dev, rx_err, dev-net,
+ invalid DPT16 signature %#010x\n,
+ le32_to_cpu(ndp16-dwSignature));
goto err_ndp;
}
dpe16 = ndp16-dpe16;
-- 
1.7.10.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

[PATCH net-next 24/24] net: cdc_ncm: no not set tx_max higher than the device supports

2013-11-01 Thread Bjørn Mork

There are MBIM devices out there reporting

  dwNtbInMaxSize=2048 dwNtbOutMaxSize=2048

and since the spec require a datagram max size of at least
2048, this means that a full sized datagram will never fit.

Still, sending larger NTBs than the device supports is not
going to help.  We do not have any other options than either
 a) refusing to bindi, or
 b) respect the insanely low value.

Alternative b will at least make these devices work, so go
for it.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4531f38..11c7033 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -159,8 +159,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
}
 
/* verify maximum size of transmitted NTB in bytes */
-   if ((ctx-tx_max  (CDC_NCM_MIN_HDR_SIZE + ctx-max_datagram_size)) ||
-   (ctx-tx_max  CDC_NCM_NTB_MAX_SIZE_TX)) {
+   if (ctx-tx_max  CDC_NCM_NTB_MAX_SIZE_TX) {
dev_dbg(dev-intf-dev, Using default maximum transmit 
length=%d\n,
CDC_NCM_NTB_MAX_SIZE_TX);
ctx-tx_max = CDC_NCM_NTB_MAX_SIZE_TX;
-- 
1.7.10.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

[PATCH net-next 09/24] net: cdc_ncm: fix SET_MAX_DATAGRAM_SIZE

2013-11-01 Thread Bjørn Mork

We need to inform the device about the *new* value, not the
old one.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4de3a542..b8de8dd 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -280,6 +280,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
/* if value changed, update device */
if (ctx-max_datagram_size !=
le16_to_cpu(max_datagram_size)) {
+   max_datagram_size = 
cpu_to_le16(ctx-max_datagram_size);
err = usbnet_write_cmd(dev,
USB_CDC_SET_MAX_DATAGRAM_SIZE,
USB_TYPE_CLASS | USB_DIR_OUT
-- 
1.7.10.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

[PATCH net-next 02/24] net: cdc_ncm: add include protection to cdc_ncm.h

2013-11-01 Thread Bjørn Mork

This makes it a lot easier to test modified versions

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 include/linux/usb/cdc_ncm.h |5 +
 1 file changed, 5 insertions(+)

diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index cc25b70..89f0bbc 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -36,6 +36,9 @@
  * SUCH DAMAGE.
  */
 
+#ifndef __LINUX_USB_CDC_NCM_H
+#define __LINUX_USB_CDC_NCM_H
+
 #define CDC_NCM_COMM_ALTSETTING_NCM0
 #define CDC_NCM_COMM_ALTSETTING_MBIM   1
 
@@ -133,3 +136,5 @@ extern void cdc_ncm_unbind(struct usbnet *dev, struct 
usb_interface *intf);
 extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct 
sk_buff *skb, __le32 sign);
 extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff 
*skb_in);
 extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
+
+#endif /* __LINUX_USB_CDC_NCM_H */
-- 
1.7.10.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

[PATCH net-next 23/24] net: cdc_ncm: improve bind error debug messages

2013-11-01 Thread Bjørn Mork

Make it a bit easier for users to figure out what goes
wrong when bind fails.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |   36 ++--
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index f168bc8..4531f38 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -371,8 +371,10 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
union_desc = (const struct usb_cdc_union_desc *)buf;
/* the master must be the interface we are probing */
if (intf-cur_altsetting-desc.bInterfaceNumber !=
-   union_desc-bMasterInterface0)
+   union_desc-bMasterInterface0) {
+   dev_dbg(intf-dev, bogus CDC Union\n);
goto error;
+   }
ctx-data = usb_ifnum_to_if(dev-udev,

union_desc-bSlaveInterface0);
break;
@@ -416,45 +418,59 @@ advance:
}
 
/* check if we got everything */
-   if (!ctx-data || (!ctx-mbim_desc  !ctx-ether_desc))
+   if (!ctx-data || (!ctx-mbim_desc  !ctx-ether_desc)) {
+   dev_dbg(intf-dev, CDC descriptors missing\n);
goto error;
+   }
 
/* claim data interface, if different from control */
if (ctx-data != ctx-control) {
temp = usb_driver_claim_interface(driver, ctx-data, dev);
-   if (temp)
+   if (temp) {
+   dev_dbg(intf-dev, failed to claim data intf\n);
goto error;
+   }
}
 
iface_no = ctx-data-cur_altsetting-desc.bInterfaceNumber;
 
/* reset data interface */
temp = usb_set_interface(dev-udev, iface_no, 0);
-   if (temp)
+   if (temp) {
+   dev_dbg(intf-dev, set interface failed\n);
goto error2;
+   }
 
/* configure data interface */
temp = usb_set_interface(dev-udev, iface_no, data_altsetting);
-   if (temp)
+   if (temp) {
+   dev_dbg(intf-dev, set interface failed\n);
goto error2;
+   }
 
cdc_ncm_find_endpoints(dev, ctx-data);
cdc_ncm_find_endpoints(dev, ctx-control);
-   if (!dev-in || !dev-out || !dev-status)
+   if (!dev-in || !dev-out || !dev-status) {
+   dev_dbg(intf-dev, failed to collect endpoints\n);
goto error2;
+   }
 
/* initialize data interface */
-   if (cdc_ncm_setup(dev))
+   if (cdc_ncm_setup(dev)) {
+   dev_dbg(intf-dev, cdc_ncm_setup() failed\n);
goto error2;
+   }
 
usb_set_intfdata(ctx-data, dev);
usb_set_intfdata(ctx-control, dev);
 
if (ctx-ether_desc) {
temp = usbnet_get_ethernet_addr(dev, 
ctx-ether_desc-iMACAddress);
-   if (temp)
+   if (temp) {
+   dev_dbg(intf-dev, failed to get mac address\n);
goto error2;
-   dev_info(dev-udev-dev, MAC-Address: %pM\n, 
dev-net-dev_addr);
+   }
+   dev_info(intf-dev, MAC-Address: %pM\n, dev-net-dev_addr);
}
 
/* usbnet use these values for sizing tx/rx queues */
@@ -471,7 +487,7 @@ error2:
 error:
cdc_ncm_free((struct cdc_ncm_ctx *)dev-data[0]);
dev-data[0] = 0;
-   dev_info(dev-udev-dev, bind() failure\n);
+   dev_info(intf-dev, bind() failure\n);
return -ENODEV;
 }
 EXPORT_SYMBOL_GPL(cdc_ncm_bind_common);
-- 
1.7.10.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

[PATCH net-next 16/24] net: cdc_ncm: log the length we warn about

2013-11-01 Thread Bjørn Mork

Fix cut'n'paste typo.  Log the bogus length and not the
irrelevant signature.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index c40f742..b5fdf8f 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -923,7 +923,7 @@ int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int 
ndpoffset)
 
if (le16_to_cpu(ndp16-wLength)  USB_CDC_NCM_NDP16_LENGTH_MIN) {
pr_debug(invalid DPT16 length %u\n,
-   le32_to_cpu(ndp16-dwSignature));
+le16_to_cpu(ndp16-wLength));
goto error;
}
 
-- 
1.7.10.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

[PATCH net-next 22/24] net: cdc_ncm: return proper error if setup fails

2013-11-01 Thread Bjørn Mork

Most setup errors are ignored to ensure maximum firmware
compatibilty.  But GET_NTB_PARAMETERS and the functional
descriptors are required.  Use proper error codes and
log level if these fail.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 62dcb2e..f168bc8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -86,8 +86,8 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
  0, iface_no, ncm_parm,
  sizeof(ncm_parm));
if (err  0) {
-   dev_dbg(dev-intf-dev, failed GET_NTB_PARAMETERS\n);
-   return 1;
+   dev_err(dev-intf-dev, failed GET_NTB_PARAMETERS\n);
+   return err; /* GET_NTB_PARAMETERS is required */
}
 
/* read correct set of parameters according to device mode */
-- 
1.7.10.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

[PATCH net-next 07/24] net: cdc_ncm: remove tx_speed and rx_speed fields

2013-11-01 Thread Bjørn Mork

These fields are only used to prevent printing the same speeds
multiple times if we receive multiple identical speed notifications.

The value of these printk's is questionable, and even more so when
we filter out some of the notifications sent us by the firmware. If
we are going to print any of these, then we should print them all.

Removing little used fields is a bonus.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c   |   37 ++---
 include/linux/usb/cdc_ncm.h |2 --
 2 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 9cdd762..fc36a99 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -510,7 +510,6 @@ advance:
ctx-tx_max % usb_maxpacket(dev-udev, dev-out, 1) == 0)
ctx-tx_max++;
 
-   ctx-tx_speed = ctx-rx_speed = 0;
return 0;
 
 error2:
@@ -1048,7 +1047,6 @@ static void
 cdc_ncm_speed_change(struct usbnet *dev,
 struct usb_cdc_speed_change *data)
 {
-   struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0];
uint32_t rx_speed = le32_to_cpu(data-DLBitRRate);
uint32_t tx_speed = le32_to_cpu(data-ULBitRate);
 
@@ -1056,25 +1054,20 @@ cdc_ncm_speed_change(struct usbnet *dev,
 * Currently the USB-NET API does not support reporting the actual
 * device speed. Do print it instead.
 */
-   if ((tx_speed != ctx-tx_speed) || (rx_speed != ctx-rx_speed)) {
-   ctx-tx_speed = tx_speed;
-   ctx-rx_speed = rx_speed;
-
-   if ((tx_speed  100)  (rx_speed  100)) {
-   printk(KERN_INFO KBUILD_MODNAME
-  : %s: %u mbit/s downlink 
-  %u mbit/s uplink\n,
-  dev-net-name,
-  (unsigned int)(rx_speed / 100U),
-  (unsigned int)(tx_speed / 100U));
-   } else {
-   printk(KERN_INFO KBUILD_MODNAME
-  : %s: %u kbit/s downlink 
-  %u kbit/s uplink\n,
-  dev-net-name,
-  (unsigned int)(rx_speed / 1000U),
-  (unsigned int)(tx_speed / 1000U));
-   }
+   if ((tx_speed  100)  (rx_speed  100)) {
+   printk(KERN_INFO KBUILD_MODNAME
+  : %s: %u mbit/s downlink 
+  %u mbit/s uplink\n,
+  dev-net-name,
+  (unsigned int)(rx_speed / 100U),
+  (unsigned int)(tx_speed / 100U));
+   } else {
+   printk(KERN_INFO KBUILD_MODNAME
+  : %s: %u kbit/s downlink 
+  %u kbit/s uplink\n,
+  dev-net-name,
+  (unsigned int)(rx_speed / 1000U),
+  (unsigned int)(tx_speed / 1000U));
}
 }
 
@@ -,8 +1104,6 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb 
*urb)
   dev-net-name, ctx-connected ?  : dis);
 
usbnet_link_change(dev, ctx-connected, 0);
-   if (!ctx-connected)
-   ctx-tx_speed = ctx-rx_speed = 0;
break;
 
case USB_CDC_NOTIFY_SPEED_CHANGE:
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 059dcc9..f14af3d 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -110,8 +110,6 @@ struct cdc_ncm_ctx {
 
u32 tx_timer_pending;
u32 tx_curr_frame_num;
-   u32 rx_speed;
-   u32 tx_speed;
u32 rx_max;
u32 tx_max;
u32 max_datagram_size;
-- 
1.7.10.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

[PATCH net-next 10/24] net: cdc_ncm: remove descriptor pointers

2013-11-01 Thread Bjørn Mork

header_desc was completely unused and union_desc was never used
outside cdc_ncm_bind_common.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c   |   12 ++--
 include/linux/usb/cdc_ncm.h |4 +---
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index b8de8dd..435fcc7 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -372,6 +372,7 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
 
 int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 
data_altsetting)
 {
+   const struct usb_cdc_union_desc *union_desc = NULL;
struct cdc_ncm_ctx *ctx;
struct usb_driver *driver;
u8 *buf;
@@ -406,16 +407,15 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
 
switch (buf[2]) {
case USB_CDC_UNION_TYPE:
-   if (buf[0]  sizeof(*(ctx-union_desc)))
+   if (buf[0]  sizeof(*union_desc))
break;
 
-   ctx-union_desc =
-   (const struct usb_cdc_union_desc *)buf;
+   union_desc = (const struct usb_cdc_union_desc *)buf;
 
ctx-control = usb_ifnum_to_if(dev-udev,
-   ctx-union_desc-bMasterInterface0);
+  
union_desc-bMasterInterface0);
ctx-data = usb_ifnum_to_if(dev-udev,
-   ctx-union_desc-bSlaveInterface0);
+   
union_desc-bSlaveInterface0);
break;
 
case USB_CDC_ETHERNET_TYPE:
@@ -458,7 +458,7 @@ advance:
}
 
/* some buggy devices have an IAD but no CDC Union */
-   if (!ctx-union_desc  intf-intf_assoc  
intf-intf_assoc-bInterfaceCount == 2) {
+   if (!union_desc  intf-intf_assoc  
intf-intf_assoc-bInterfaceCount == 2) {
ctx-control = intf;
ctx-data = usb_ifnum_to_if(dev-udev, 
intf-cur_altsetting-desc.bInterfaceNumber + 1);
dev_dbg(intf-dev, CDC Union missing - got slave from IAD\n);
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 89b52a0..cad54ad 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -92,9 +92,7 @@ struct cdc_ncm_ctx {
struct tasklet_struct bh;
 
const struct usb_cdc_ncm_desc *func_desc;
-   const struct usb_cdc_mbim_desc   *mbim_desc;
-   const struct usb_cdc_header_desc *header_desc;
-   const struct usb_cdc_union_desc *union_desc;
+   const struct usb_cdc_mbim_desc *mbim_desc;
const struct usb_cdc_ether_desc *ether_desc;
 
struct usb_interface *control;
-- 
1.7.10.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

[PATCH net-next 17/24] net: cdc_ncm: use netif_* and dev_* instead of pr_*

2013-11-01 Thread Bjørn Mork

Take advantage of standard device name prefixing and
netdevice msglvl control where possible.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |   98 ++---
 1 file changed, 48 insertions(+), 50 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index b5fdf8f..73faf05 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -87,7 +87,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
  0, iface_no, ncm_parm,
  sizeof(ncm_parm));
if (err  0) {
-   pr_debug(failed GET_NTB_PARAMETERS\n);
+   dev_dbg(dev-intf-dev, failed GET_NTB_PARAMETERS\n);
return 1;
}
 
@@ -115,11 +115,10 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
flags = 0;
}
 
-   pr_debug(dwNtbInMaxSize=%u dwNtbOutMaxSize=%u 
-wNdpOutPayloadRemainder=%u wNdpOutDivisor=%u 
-wNdpOutAlignment=%u wNtbOutMaxDatagrams=%u flags=0x%x\n,
-ctx-rx_max, ctx-tx_max, ctx-tx_remainder, ctx-tx_modulus,
-ctx-tx_ndp_modulus, ctx-tx_max_datagrams, flags);
+   dev_dbg(dev-intf-dev,
+   dwNtbInMaxSize=%u dwNtbOutMaxSize=%u 
wNdpOutPayloadRemainder=%u wNdpOutDivisor=%u wNdpOutAlignment=%u 
wNtbOutMaxDatagrams=%u flags=0x%x\n,
+   ctx-rx_max, ctx-tx_max, ctx-tx_remainder, ctx-tx_modulus,
+   ctx-tx_ndp_modulus, ctx-tx_max_datagrams, flags);
 
/* max count of tx datagrams */
if ((ctx-tx_max_datagrams == 0) ||
@@ -128,14 +127,14 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 
/* verify maximum size of received NTB in bytes */
if (ctx-rx_max  USB_CDC_NCM_NTB_MIN_IN_SIZE) {
-   pr_debug(Using min receive length=%d\n,
-   USB_CDC_NCM_NTB_MIN_IN_SIZE);
+   dev_dbg(dev-intf-dev, Using min receive length=%d\n,
+   USB_CDC_NCM_NTB_MIN_IN_SIZE);
ctx-rx_max = USB_CDC_NCM_NTB_MIN_IN_SIZE;
}
 
if (ctx-rx_max  CDC_NCM_NTB_MAX_SIZE_RX) {
-   pr_debug(Using default maximum receive length=%d\n,
-   CDC_NCM_NTB_MAX_SIZE_RX);
+   dev_dbg(dev-intf-dev, Using default maximum receive 
length=%d\n,
+   CDC_NCM_NTB_MAX_SIZE_RX);
ctx-rx_max = CDC_NCM_NTB_MAX_SIZE_RX;
}
 
@@ -148,15 +147,15 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
   | USB_RECIP_INTERFACE,
   0, iface_no, dwNtbInMaxSize, 4);
if (err  0)
-   pr_debug(Setting NTB Input Size failed\n);
+   dev_dbg(dev-intf-dev, Setting NTB Input Size 
failed\n);
}
 
/* verify maximum size of transmitted NTB in bytes */
if ((ctx-tx_max 
(min_hdr_size + min_dgram_size)) ||
(ctx-tx_max  CDC_NCM_NTB_MAX_SIZE_TX)) {
-   pr_debug(Using default maximum transmit length=%d\n,
-   CDC_NCM_NTB_MAX_SIZE_TX);
+   dev_dbg(dev-intf-dev, Using default maximum transmit 
length=%d\n,
+   CDC_NCM_NTB_MAX_SIZE_TX);
ctx-tx_max = CDC_NCM_NTB_MAX_SIZE_TX;
 
/* Adding a pad byte here simplifies the handling in
@@ -178,7 +177,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 
if ((val  USB_CDC_NCM_NDP_ALIGN_MIN_SIZE) ||
(val != ((-val)  val)) || (val = ctx-tx_max)) {
-   pr_debug(Using default alignment: 4 bytes\n);
+   dev_dbg(dev-intf-dev, Using default alignment: 4 bytes\n);
ctx-tx_ndp_modulus = USB_CDC_NCM_NDP_ALIGN_MIN_SIZE;
}
 
@@ -192,13 +191,13 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 
if ((val  USB_CDC_NCM_NDP_ALIGN_MIN_SIZE) ||
(val != ((-val)  val)) || (val = ctx-tx_max)) {
-   pr_debug(Using default transmit modulus: 4 bytes\n);
+   dev_dbg(dev-intf-dev, Using default transmit modulus: 4 
bytes\n);
ctx-tx_modulus = USB_CDC_NCM_NDP_ALIGN_MIN_SIZE;
}
 
/* verify the payload remainder */
if (ctx-tx_remainder = ctx-tx_modulus) {
-   pr_debug(Using default transmit remainder: 0 bytes\n);
+   dev_dbg(dev-intf-dev, Using default transmit remainder: 0 
bytes\n);
ctx-tx_remainder = 0;
}
 
@@ -216,7 +215,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
   USB_CDC_NCM_CRC_NOT_APPENDED,
   iface_no, NULL, 0);
if (err  0)
-   pr_debug(Setting CRC mode off failed\n);
+   dev_dbg(dev-intf-dev, Setting CRC

[PATCH net-next 13/24] net: cdc_ncm: remove probe and disconnect wrappers

2013-11-01 Thread Bjørn Mork

These functions were merely wrappers around the usbnet
variants.  Remove them.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |   20 ++--
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 42c8620..c90d843 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1141,22 +1141,6 @@ static int cdc_ncm_check_connect(struct usbnet *dev)
return !ctx-connected;
 }
 
-static int
-cdc_ncm_probe(struct usb_interface *udev, const struct usb_device_id *prod)
-{
-   return usbnet_probe(udev, prod);
-}
-
-static void cdc_ncm_disconnect(struct usb_interface *intf)
-{
-   struct usbnet *dev = usb_get_intfdata(intf);
-
-   if (dev == NULL)
-   return; /* already disconnected */
-
-   usbnet_disconnect(intf);
-}
-
 static const struct driver_info cdc_ncm_info = {
.description = CDC NCM,
.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
@@ -1267,8 +1251,8 @@ MODULE_DEVICE_TABLE(usb, cdc_devs);
 static struct usb_driver cdc_ncm_driver = {
.name = cdc_ncm,
.id_table = cdc_devs,
-   .probe = cdc_ncm_probe,
-   .disconnect = cdc_ncm_disconnect,
+   .probe = usbnet_probe,
+   .disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
.reset_resume = usbnet_resume,
-- 
1.7.10.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

[PATCH net-next 21/24] net: cdc_ncm: refactoring cdc_ncm_setup

2013-11-01 Thread Bjørn Mork

Rewriting the set max datagram part of dc_ncm_setup to
separate the selection and validatation of the size from
the code which optionally informs the device of this
value.  This ensures that we use the correct value
regardless of device support for the get and set commands.

Removing some of the many indent levels while doing this
to make the code more readable.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |  108 ++---
 1 file changed, 44 insertions(+), 64 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 83e6d5b..62dcb2e 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -76,8 +76,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
int err;
int eth_hlen;
u16 ntb_fmt_supported;
-   u32 min_dgram_size;
-   u32 min_hdr_size;
+   __le16 max_datagram_size;
 
iface_no = ctx-control-cur_altsetting-desc.bInterfaceNumber;
 
@@ -101,20 +100,29 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
ctx-tx_max_datagrams = le16_to_cpu(ncm_parm.wNtbOutMaxDatagrams);
ntb_fmt_supported = le16_to_cpu(ncm_parm.bmNtbFormatsSupported);
 
-   eth_hlen = ETH_HLEN;
-   min_dgram_size = CDC_NCM_MIN_DATAGRAM_SIZE;
-   min_hdr_size = CDC_NCM_MIN_HDR_SIZE;
-   if (ctx-mbim_desc != NULL) {
-   flags = ctx-mbim_desc-bmNetworkCapabilities;
+   /* there are some minor differences in NCM and MBIM defaults */
+   if (cdc_ncm_comm_intf_is_mbim(ctx-control-cur_altsetting)) {
+   if (!ctx-mbim_desc)
+   return -EINVAL;
eth_hlen = 0;
-   min_dgram_size = CDC_MBIM_MIN_DATAGRAM_SIZE;
-   min_hdr_size = 0;
-   } else if (ctx-func_desc != NULL) {
-   flags = ctx-func_desc-bmNetworkCapabilities;
+   flags = ctx-mbim_desc-bmNetworkCapabilities;
+   ctx-max_datagram_size = 
le16_to_cpu(ctx-mbim_desc-wMaxSegmentSize);
+   if (ctx-max_datagram_size  CDC_MBIM_MIN_DATAGRAM_SIZE)
+   ctx-max_datagram_size = CDC_MBIM_MIN_DATAGRAM_SIZE;
} else {
-   flags = 0;
+   if (!ctx-func_desc)
+   return -EINVAL;
+   eth_hlen = ETH_HLEN;
+   flags = ctx-func_desc-bmNetworkCapabilities;
+   ctx-max_datagram_size = 
le16_to_cpu(ctx-ether_desc-wMaxSegmentSize);
+   if (ctx-max_datagram_size  CDC_NCM_MIN_DATAGRAM_SIZE)
+   ctx-max_datagram_size = CDC_NCM_MIN_DATAGRAM_SIZE;
}
 
+   /* common absolute max for NCM and MBIM */
+   if (ctx-max_datagram_size  CDC_NCM_MAX_DATAGRAM_SIZE)
+   ctx-max_datagram_size = CDC_NCM_MAX_DATAGRAM_SIZE;
+
dev_dbg(dev-intf-dev,
dwNtbInMaxSize=%u dwNtbOutMaxSize=%u 
wNdpOutPayloadRemainder=%u wNdpOutDivisor=%u wNdpOutAlignment=%u 
wNtbOutMaxDatagrams=%u flags=0x%x\n,
ctx-rx_max, ctx-tx_max, ctx-tx_remainder, ctx-tx_modulus,
@@ -151,8 +159,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
}
 
/* verify maximum size of transmitted NTB in bytes */
-   if ((ctx-tx_max 
-   (min_hdr_size + min_dgram_size)) ||
+   if ((ctx-tx_max  (CDC_NCM_MIN_HDR_SIZE + ctx-max_datagram_size)) ||
(ctx-tx_max  CDC_NCM_NTB_MAX_SIZE_TX)) {
dev_dbg(dev-intf-dev, Using default maximum transmit 
length=%d\n,
CDC_NCM_NTB_MAX_SIZE_TX);
@@ -229,60 +236,33 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
dev_dbg(dev-intf-dev, Setting NTB format to 16-bit 
failed\n);
}
 
-   ctx-max_datagram_size = min_dgram_size;
+   /* inform the device about the selected Max Datagram Size */
+   if (!(flags  USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE))
+   goto out;
 
-   /* set Max Datagram Size (MTU) */
-   if (flags  USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE) {
-   __le16 max_datagram_size;
-   u16 eth_max_sz;
-   if (ctx-ether_desc != NULL)
-   eth_max_sz = 
le16_to_cpu(ctx-ether_desc-wMaxSegmentSize);
-   else if (ctx-mbim_desc != NULL)
-   eth_max_sz = 
le16_to_cpu(ctx-mbim_desc-wMaxSegmentSize);
-   else
-   goto max_dgram_err;
-
-   err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE,
- USB_TYPE_CLASS | USB_DIR_IN
- | USB_RECIP_INTERFACE,
- 0, iface_no, max_datagram_size, 2);
-   if (err  0) {
-   dev_dbg(dev-intf-dev, GET_MAX_DATAGRAM_SIZE failed, 
use size=%u\n,
-   min_dgram_size);
-   } else {
-   ctx-max_datagram_size =
-

[PATCH net-next 01/24] net: cdc_ncm: simplify and optimize frame padding

2013-11-01 Thread Bjørn Mork

We can avoid the costly division for the common case where
we pad the frame to tx_max size as long as we ensure that
tx_max is either the device specified dwNtbOutMaxSize or not
a multiplum of wMaxPacketSize.

Using the preconverted 'maxpacket' field avoids converting
wMaxPacketSize to CPU endianness for every transmitted frame

And since we only will hit the one byte padding rule for short
frames, we can drop testing the skb for tailroom.

The change means that tx_max now represents the real maximum
skb size, enabling us to allocate the correct size instead of
always making room for one extra byte.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |   31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..38566bf 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -506,6 +506,15 @@ advance:
dev-status = ctx-status_ep;
dev-rx_urb_size = ctx-rx_max;
 
+   /* cdc_ncm_setup will override dwNtbOutMaxSize if it is
+* outside the sane range. Adding a pad byte here if necessary
+* simplifies the handling in cdc_ncm_fill_tx_frame, making
+* tx_max always represent the real skb max size.
+*/
+   if (ctx-tx_max != le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize) 
+   ctx-tx_max % usb_maxpacket(dev-udev, dev-out, 1) == 0)
+   ctx-tx_max++;
+
ctx-tx_speed = ctx-rx_speed = 0;
return 0;
 
@@ -664,6 +673,7 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct 
cdc_ncm_ctx *ctx, struct sk_
 struct sk_buff *
 cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 
sign)
 {
+   struct usbnet *dev = netdev_priv(ctx-netdev);
struct usb_cdc_ncm_nth16 *nth16;
struct usb_cdc_ncm_ndp16 *ndp16;
struct sk_buff *skb_out;
@@ -683,7 +693,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct 
sk_buff *skb, __le32 sign)
 
/* allocate a new OUT skb */
if (!skb_out) {
-   skb_out = alloc_skb((ctx-tx_max + 1), GFP_ATOMIC);
+   skb_out = alloc_skb(ctx-tx_max, GFP_ATOMIC);
if (skb_out == NULL) {
if (skb != NULL) {
dev_kfree_skb_any(skb);
@@ -788,19 +798,16 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct 
sk_buff *skb, __le32 sign)
/* variables will be reset at next call */
}
 
-   /*
-* If collected data size is less or equal CDC_NCM_MIN_TX_PKT bytes,
-* we send buffers as it is. If we get more data, it would be more
-* efficient for USB HS mobile device with DMA engine to receive a full
-* size NTB, than canceling DMA transfer and receiving a short packet.
+   /* If collected data size is less or equal CDC_NCM_MIN_TX_PKT
+* bytes, we send buffers as it is. If we get more data, it
+* would be more efficient for USB HS mobile device with DMA
+* engine to receive a full size NTB, than canceling DMA
+* transfer and receiving a short packet.
 */
if (skb_out-len  CDC_NCM_MIN_TX_PKT)
-   /* final zero padding */
-   memset(skb_put(skb_out, ctx-tx_max - skb_out-len), 0, 
ctx-tx_max - skb_out-len);
-
-   /* do we need to prevent a ZLP? */
-   if (((skb_out-len % le16_to_cpu(ctx-out_ep-desc.wMaxPacketSize)) == 
0) 
-   (skb_out-len  le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize))  
skb_tailroom(skb_out))
+   memset(skb_put(skb_out, ctx-tx_max - skb_out-len), 0,
+  ctx-tx_max - skb_out-len);
+   else if ((skb_out-len % dev-maxpacket) == 0)
*skb_put(skb_out, 1) = 0;   /* force short packet */
 
/* set final frame length */
-- 
1.7.10.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

[PATCH net-next 00/24] cdc_ncm: many small and mostly trivial fixes

2013-11-01 Thread Bjørn Mork

This series ended up longer than expected, and it is still not
complete. There is more to come when time allows...

Most changes are trivial. Notable non-trivial changes are
 - removed filtering of identical speed notifications
 - tx_max calulation is changed to count the pad byte if
   necessary, and respect the device limit as an absolute
   upper limit even if it is too low according to the spec
 - remove the bug preventing SET_MAX_DATAGRAM_SIZE from having
   any effect
 - drop the pad-to-max if ZLPs are enabled
 - the driver specific VERSION is dropped
 - dev-hard_mtu is set to tx_max instead of max_datagram_size
   causing usbnet to calculate the qlen based on the real max
   size of tx skbs

This series has been tested, along with the previously posted
cdc_mbim series, on the NCM and MBIM devices I have:
 - Ericsson F5521gw (NCM)
 - Huawei E367 (MBIM)
 - D-Link DWM-156 A7 (MBIM w/ too low dwNtb{In,Out}MaxSize bug)
 - Sierra Wireless MC7710 (MBIM w/ ZLP and CDC Union bugs)

Apart from the D-Link modem dropping a lot less oversized
frames with the fix dedicated to it, there are no end user
noticable functional changes as a result of this series.  But
all the non-trivial changes I listed above are of course
detectable by users looking at that specific area (except maybe
the removed speed notification, which requires a device sending
duplicates to be noticable - I don't have any such device).


Bjørn Mork (24):
  net: cdc_ncm: simplify and optimize frame padding
  net: cdc_ncm: add include protection to cdc_ncm.h
  net: cdc_ncm: remove redundant intf field
  net: cdc_ncm: remove redundant endpoint pointers
  net: cdc_ncm: remove redundant netdev field
  net: cdc_ncm: remove unused udev field
  net: cdc_ncm: remove tx_speed and rx_speed fields
  net: cdc_ncm: remove ncm_parm field
  net: cdc_ncm: fix SET_MAX_DATAGRAM_SIZE
  net: cdc_ncm: remove descriptor pointers
  net: cdc_ncm: only the control intf can be probed
  net: cdc_ncm: no point in filling up the NTBs if we send ZLPs
  net: cdc_ncm: remove probe and disconnect wrappers
  net: cdc_ncm: remove ethtool ops
  net: cdc_ncm: set correct dev-hard_mtu
  net: cdc_ncm: log the length we warn about
  net: cdc_ncm: use netif_* and dev_* instead of pr_*
  net: cdc_ncm: log signatures in hex
  net: cdc_ncm: endian convert constants instead of variables
  net: cdc_ncm: drop extern from header declarations
  net: cdc_ncm: refactoring cdc_ncm_setup
  net: cdc_ncm: return proper error if setup fails
  net: cdc_ncm: improve bind error debug messages
  net: cdc_ncm: no not set tx_max higher than the device supports

 drivers/net/usb/cdc_mbim.c  |2 +-
 drivers/net/usb/cdc_ncm.c   |  490 +++
 include/linux/usb/cdc_ncm.h |   30 ++-
 3 files changed, 233 insertions(+), 289 deletions(-)

-- 
1.7.10.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

[PATCH net-next 03/24] net: cdc_ncm: remove redundant intf field

2013-11-01 Thread Bjørn Mork

This is always a duplicate of the control field. It causes
confusion wrt intf_data updates and cleanups.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c   |4 +---
 include/linux/usb/cdc_ncm.h |1 -
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 38566bf..bfab879 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -382,7 +382,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
len = intf-cur_altsetting-extralen;
 
ctx-udev = dev-udev;
-   ctx-intf = intf;
 
/* parse through descriptors associated with control interface */
while ((len  0)  (buf[0]  2)  (buf[0] = len)) {
@@ -489,7 +488,6 @@ advance:
 
usb_set_intfdata(ctx-data, dev);
usb_set_intfdata(ctx-control, dev);
-   usb_set_intfdata(ctx-intf, dev);
 
if (ctx-ether_desc) {
temp = usbnet_get_ethernet_addr(dev, 
ctx-ether_desc-iMACAddress);
@@ -562,7 +560,7 @@ void cdc_ncm_unbind(struct usbnet *dev, struct 
usb_interface *intf)
ctx-control = NULL;
}
 
-   usb_set_intfdata(ctx-intf, NULL);
+   usb_set_intfdata(intf, NULL);
cdc_ncm_free(ctx);
 }
 EXPORT_SYMBOL_GPL(cdc_ncm_unbind);
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 89f0bbc..c14e00f 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -103,7 +103,6 @@ struct cdc_ncm_ctx {
struct usb_host_endpoint *in_ep;
struct usb_host_endpoint *out_ep;
struct usb_host_endpoint *status_ep;
-   struct usb_interface *intf;
struct usb_interface *control;
struct usb_interface *data;
 
-- 
1.7.10.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

[PATCH net-next 04/24] net: cdc_ncm: remove redundant endpoint pointers

2013-11-01 Thread Bjørn Mork

No need to duplicate stuff already in the common usbnet
struct.  We still need to keep our special find_endpoints
function because we need explicit control over the selected
altsetting.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c   |   38 +++---
 include/linux/usb/cdc_ncm.h |3 ---
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index bfab879..a989bd5 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -292,9 +292,9 @@ max_dgram_err:
 }
 
 static void
-cdc_ncm_find_endpoints(struct cdc_ncm_ctx *ctx, struct usb_interface *intf)
+cdc_ncm_find_endpoints(struct usbnet *dev, struct usb_interface *intf)
 {
-   struct usb_host_endpoint *e;
+   struct usb_host_endpoint *e, *in = NULL, *out = NULL;
u8 ep;
 
for (ep = 0; ep  intf-cur_altsetting-desc.bNumEndpoints; ep++) {
@@ -303,18 +303,18 @@ cdc_ncm_find_endpoints(struct cdc_ncm_ctx *ctx, struct 
usb_interface *intf)
switch (e-desc.bmAttributes  USB_ENDPOINT_XFERTYPE_MASK) {
case USB_ENDPOINT_XFER_INT:
if (usb_endpoint_dir_in(e-desc)) {
-   if (ctx-status_ep == NULL)
-   ctx-status_ep = e;
+   if (!dev-status)
+   dev-status = e;
}
break;
 
case USB_ENDPOINT_XFER_BULK:
if (usb_endpoint_dir_in(e-desc)) {
-   if (ctx-in_ep == NULL)
-   ctx-in_ep = e;
+   if (!in)
+   in = e;
} else {
-   if (ctx-out_ep == NULL)
-   ctx-out_ep = e;
+   if (!out)
+   out = e;
}
break;
 
@@ -322,6 +322,14 @@ cdc_ncm_find_endpoints(struct cdc_ncm_ctx *ctx, struct 
usb_interface *intf)
break;
}
}
+   if (in  !dev-in)
+   dev-in = usb_rcvbulkpipe(dev-udev,
+ in-desc.bEndpointAddress 
+ USB_ENDPOINT_NUMBER_MASK);
+   if (out  !dev-out)
+   dev-out = usb_sndbulkpipe(dev-udev,
+  out-desc.bEndpointAddress 
+  USB_ENDPOINT_NUMBER_MASK);
 }
 
 static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
@@ -477,11 +485,9 @@ advance:
if (temp)
goto error2;
 
-   cdc_ncm_find_endpoints(ctx, ctx-data);
-   cdc_ncm_find_endpoints(ctx, ctx-control);
-
-   if ((ctx-in_ep == NULL) || (ctx-out_ep == NULL) ||
-   (ctx-status_ep == NULL))
+   cdc_ncm_find_endpoints(dev, ctx-data);
+   cdc_ncm_find_endpoints(dev, ctx-control);
+   if (!dev-in || !dev-out || !dev-status)
goto error2;
 
dev-net-ethtool_ops = cdc_ncm_ethtool_ops;
@@ -496,12 +502,6 @@ advance:
dev_info(dev-udev-dev, MAC-Address: %pM\n, 
dev-net-dev_addr);
}
 
-
-   dev-in = usb_rcvbulkpipe(dev-udev,
-   ctx-in_ep-desc.bEndpointAddress  USB_ENDPOINT_NUMBER_MASK);
-   dev-out = usb_sndbulkpipe(dev-udev,
-   ctx-out_ep-desc.bEndpointAddress  USB_ENDPOINT_NUMBER_MASK);
-   dev-status = ctx-status_ep;
dev-rx_urb_size = ctx-rx_max;
 
/* cdc_ncm_setup will override dwNtbOutMaxSize if it is
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index c14e00f..36e1e15 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -100,9 +100,6 @@ struct cdc_ncm_ctx {
 
struct net_device *netdev;
struct usb_device *udev;
-   struct usb_host_endpoint *in_ep;
-   struct usb_host_endpoint *out_ep;
-   struct usb_host_endpoint *status_ep;
struct usb_interface *control;
struct usb_interface *data;
 
-- 
1.7.10.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

[PATCH net-next 19/24] net: cdc_ncm: endian convert constants instead of variables

2013-11-01 Thread Bjørn Mork

Converting the constants used in these comparisons at build
time instead of converting the variables for every received
frame at run time.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index d49e4c5..83e6d5b 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -882,7 +882,7 @@ int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct 
sk_buff *skb_in)
 
nth16 = (struct usb_cdc_ncm_nth16 *)skb_in-data;
 
-   if (le32_to_cpu(nth16-dwSignature) != USB_CDC_NCM_NTH16_SIGN) {
+   if (nth16-dwSignature != cpu_to_le32(USB_CDC_NCM_NTH16_SIGN)) {
netif_dbg(dev, rx_err, dev-net,
  invalid NTH16 signature %#010x\n,
  le32_to_cpu(nth16-dwSignature));
@@ -972,7 +972,7 @@ next_ndp:
 
ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in-data + ndpoffset);
 
-   if (le32_to_cpu(ndp16-dwSignature) != USB_CDC_NCM_NDP16_NOCRC_SIGN) {
+   if (ndp16-dwSignature != cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN)) {
netif_dbg(dev, rx_err, dev-net,
  invalid DPT16 signature %#010x\n,
  le32_to_cpu(ndp16-dwSignature));
-- 
1.7.10.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

[PATCH net-next 15/24] net: cdc_ncm: set correct dev-hard_mtu

2013-11-01 Thread Bjørn Mork

usbnet use the hard_mtu value for sizing the tx queue and nothing
else.  We will be transmitting buffers of up to tx_max size, so
that's the proper value to give usbnet.

The individual datagram size is completely irrelevant here.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 8fc1a06..c40f742 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -404,13 +404,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
 
ctx-ether_desc =
(const struct usb_cdc_ether_desc *)buf;
-   dev-hard_mtu =
-   le16_to_cpu(ctx-ether_desc-wMaxSegmentSize);
-
-   if (dev-hard_mtu  CDC_NCM_MIN_DATAGRAM_SIZE)
-   dev-hard_mtu = CDC_NCM_MIN_DATAGRAM_SIZE;
-   else if (dev-hard_mtu  CDC_NCM_MAX_DATAGRAM_SIZE)
-   dev-hard_mtu = CDC_NCM_MAX_DATAGRAM_SIZE;
break;
 
case USB_CDC_NCM_TYPE:
@@ -485,6 +478,8 @@ advance:
dev_info(dev-udev-dev, MAC-Address: %pM\n, 
dev-net-dev_addr);
}
 
+   /* usbnet use these values for sizing tx/rx queues */
+   dev-hard_mtu = ctx-tx_max;
dev-rx_urb_size = ctx-rx_max;
 
return 0;
-- 
1.7.10.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

[PATCH net-next 08/24] net: cdc_ncm: remove ncm_parm field

2013-11-01 Thread Bjørn Mork

Moving the call to cdc_ncm_setup() after the endpoint
setup removes the last remaining reference to ncm_parm
outside cdc_ncm_setup.

Collecting all the ncm_parm based calculations in
cdc_ncm_setup improves readability.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c   |   46 +--
 include/linux/usb/cdc_ncm.h |1 -
 2 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index fc36a99..4de3a542 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -83,6 +83,7 @@ cdc_ncm_get_drvinfo(struct net_device *net, struct 
ethtool_drvinfo *info)
 static u8 cdc_ncm_setup(struct usbnet *dev)
 {
struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0];
+   struct usb_cdc_ncm_ntb_parameters ncm_parm;
u32 val;
u8 flags;
u8 iface_no;
@@ -97,22 +98,22 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
err = usbnet_read_cmd(dev, USB_CDC_GET_NTB_PARAMETERS,
  USB_TYPE_CLASS | USB_DIR_IN
  |USB_RECIP_INTERFACE,
- 0, iface_no, ctx-ncm_parm,
- sizeof(ctx-ncm_parm));
+ 0, iface_no, ncm_parm,
+ sizeof(ncm_parm));
if (err  0) {
pr_debug(failed GET_NTB_PARAMETERS\n);
return 1;
}
 
/* read correct set of parameters according to device mode */
-   ctx-rx_max = le32_to_cpu(ctx-ncm_parm.dwNtbInMaxSize);
-   ctx-tx_max = le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize);
-   ctx-tx_remainder = le16_to_cpu(ctx-ncm_parm.wNdpOutPayloadRemainder);
-   ctx-tx_modulus = le16_to_cpu(ctx-ncm_parm.wNdpOutDivisor);
-   ctx-tx_ndp_modulus = le16_to_cpu(ctx-ncm_parm.wNdpOutAlignment);
+   ctx-rx_max = le32_to_cpu(ncm_parm.dwNtbInMaxSize);
+   ctx-tx_max = le32_to_cpu(ncm_parm.dwNtbOutMaxSize);
+   ctx-tx_remainder = le16_to_cpu(ncm_parm.wNdpOutPayloadRemainder);
+   ctx-tx_modulus = le16_to_cpu(ncm_parm.wNdpOutDivisor);
+   ctx-tx_ndp_modulus = le16_to_cpu(ncm_parm.wNdpOutAlignment);
/* devices prior to NCM Errata shall set this field to zero */
-   ctx-tx_max_datagrams = le16_to_cpu(ctx-ncm_parm.wNtbOutMaxDatagrams);
-   ntb_fmt_supported = le16_to_cpu(ctx-ncm_parm.bmNtbFormatsSupported);
+   ctx-tx_max_datagrams = le16_to_cpu(ncm_parm.wNtbOutMaxDatagrams);
+   ntb_fmt_supported = le16_to_cpu(ncm_parm.bmNtbFormatsSupported);
 
eth_hlen = ETH_HLEN;
min_dgram_size = CDC_NCM_MIN_DATAGRAM_SIZE;
@@ -153,7 +154,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
}
 
/* inform device about NTB input size changes */
-   if (ctx-rx_max != le32_to_cpu(ctx-ncm_parm.dwNtbInMaxSize)) {
+   if (ctx-rx_max != le32_to_cpu(ncm_parm.dwNtbInMaxSize)) {
__le32 dwNtbInMaxSize = cpu_to_le32(ctx-rx_max);
 
err = usbnet_write_cmd(dev, USB_CDC_SET_NTB_INPUT_SIZE,
@@ -171,6 +172,14 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
pr_debug(Using default maximum transmit length=%d\n,
CDC_NCM_NTB_MAX_SIZE_TX);
ctx-tx_max = CDC_NCM_NTB_MAX_SIZE_TX;
+
+   /* Adding a pad byte here simplifies the handling in
+* cdc_ncm_fill_tx_frame, by making tx_max always
+* represent the real skb max size.
+*/
+   if (ctx-tx_max % usb_maxpacket(dev-udev, dev-out, 1) == 0)
+   ctx-tx_max++;
+
}
 
/*
@@ -473,10 +482,6 @@ advance:
if (temp)
goto error2;
 
-   /* initialize data interface */
-   if (cdc_ncm_setup(dev))
-   goto error2;
-
/* configure data interface */
temp = usb_set_interface(dev-udev, iface_no, data_altsetting);
if (temp)
@@ -487,6 +492,10 @@ advance:
if (!dev-in || !dev-out || !dev-status)
goto error2;
 
+   /* initialize data interface */
+   if (cdc_ncm_setup(dev))
+   goto error2;
+
dev-net-ethtool_ops = cdc_ncm_ethtool_ops;
 
usb_set_intfdata(ctx-data, dev);
@@ -501,15 +510,6 @@ advance:
 
dev-rx_urb_size = ctx-rx_max;
 
-   /* cdc_ncm_setup will override dwNtbOutMaxSize if it is
-* outside the sane range. Adding a pad byte here if necessary
-* simplifies the handling in cdc_ncm_fill_tx_frame, making
-* tx_max always represent the real skb max size.
-*/
-   if (ctx-tx_max != le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize) 
-   ctx-tx_max % usb_maxpacket(dev-udev, dev-out, 1) == 0)
-   ctx-tx_max++;
-
return 0;
 
 error2:
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index f14af3d..89b52a0 100644
---

[PATCH net-next 11/24] net: cdc_ncm: only the control intf can be probed

2013-11-01 Thread Bjørn Mork

The probed interface must be the master/control interface of the
function.  Make this explicit and simplify redundant tests.

Cc: Alexey Orishko alexey.oris...@gmail.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_ncm.c |   14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 435fcc7..5aa3e60 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -394,6 +394,9 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
/* store ctx pointer in device data field */
dev-data[0] = (unsigned long)ctx;
 
+   /* only the control interface can be successfully probed */
+   ctx-control = intf;
+
/* get some pointers */
driver = driver_of(intf);
buf = intf-cur_altsetting-extra;
@@ -411,9 +414,10 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
break;
 
union_desc = (const struct usb_cdc_union_desc *)buf;
-
-   ctx-control = usb_ifnum_to_if(dev-udev,
-  
union_desc-bMasterInterface0);
+   /* the master must be the interface we are probing */
+   if (intf-cur_altsetting-desc.bInterfaceNumber !=
+   union_desc-bMasterInterface0)
+   goto error;
ctx-data = usb_ifnum_to_if(dev-udev,

union_desc-bSlaveInterface0);
break;
@@ -459,14 +463,12 @@ advance:
 
/* some buggy devices have an IAD but no CDC Union */
if (!union_desc  intf-intf_assoc  
intf-intf_assoc-bInterfaceCount == 2) {
-   ctx-control = intf;
ctx-data = usb_ifnum_to_if(dev-udev, 
intf-cur_altsetting-desc.bInterfaceNumber + 1);
dev_dbg(intf-dev, CDC Union missing - got slave from IAD\n);
}
 
/* check if we got everything */
-   if ((ctx-control == NULL) || (ctx-data == NULL) ||
-   ((!ctx-mbim_desc)  ((ctx-ether_desc == NULL) || (ctx-control 
!= intf
+   if (!ctx-data || (!ctx-mbim_desc  !ctx-ether_desc))
goto error;
 
/* claim data interface, if different from control */
-- 
1.7.10.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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Frank Schäfer

Am 01.11.2013 11:06, schrieb Frank Schäfer:
 Am 31.10.2013 21:56, schrieb Johan Hovold:
 On Thu, Oct 31, 2013 at 09:26:06PM +0100, Johan Hovold wrote:
 On Thu, Oct 31, 2013 at 7:45 PM, Frank Schäfer wrote:
 Am 31.10.2013 13:30, schrieb Mika Westerberg:
 On Thu, Oct 31, 2013 at 01:02:56PM +0100, Frank Schäfer wrote:
 2) comment out the following line at the end of
 pl2303_baudrate_encode_divisor_HXD():

 baud = 1200 * 32 / ((1  buf[1]) * buf[0]);
 This seems to fix the problem :)

 Once the line is commented out, the serial console works fine with the
 speeds I've been testing (115200, 230400 and 460800).
 Urgh... so it seems getty gets confused if the actually set baud rate is
 reported back.
 The kernel still reports the standard baud rate (e.g. B230400), it only
 sets the exact value in the c_ispeed and c_ospeed fields of the termios
 struct.
 So even if getty doesn't support non-standard baud rates, everything
 should be fine.
 I'll have to take a closer look at this issue later.
 I think I know what's going on. The pl2303 is known to loose bytes when
 changing the termios settings (see commit bf5e5834b (pl2303: Fix mode
 switching regression)).

 Your commit 57ce61aad (usb: pl2303: fix+improve the divisor based baud
 rate encoding method) introduced a regression when it started reporting
 back the baudrates determined by the divisor algorithm.

 Since that commit, tty_termios_hw_change(tty-termios, old_termios)
 will always return true -- even when userspace isn't requesting a
 different baudrate.
 Hmm... so there is a bug in tty_termios_hw_change() ?

Ahh... no... I see what's going on.
I assume we have to compare the resulting encoded baud rates to check if
the settings changed.
But that's something for the next kernel...

Frank


 Ok, so now let's see if the fixed/improved divisor based method also
 works for the HXD chip if we don't report the actually set baud rate.
 Try commenting out the line

 baud = 1200 * 32 / ((1  A) * B);

 at the end of pl2303_baudrate_encode_divisor() of a clean 3.12-rc kernel.
 It's unlikely that this makes baud rates  115200 working, but testing
 both cases doesn't harm. ;)
 I can trigger the problem here with my HXD/EA/RA/SA-device as well, and
 I've verified that not returning the calculated baudrate makes this
 particular issue *seem* to go away.

 This obviously has to be fixed or reverted quickly.
 Indeed.

 Greg, what do you say about this? Is reverting for 3.12 the correct way
 to deal with this, and make sure these fairly invasive patches get some
 more testing before being reapplied?

 Reverting would mean reverting a whole bunch of commits though, as they
 appear to depend quite heavily on each other:

 7d26a78 USB: pl2303: distinguish between original and cloned HX chips
 034d152 pl2303: improve the chip type detection/distinction
 a77a8c2 pl2303: improve the chip type information output on startup
 73b583a pl2303: simplify the else-if contruct for type_1 chips in 
 pl2303_startup()
 c23bda3 usb: pl2303: add two comments concerning the supported baud rates 
 with HX chips
 61fa8d6 usb: pl2303: also use the divisor based baud rate encoding method 
 for baud rates  115200 with HX chips
 b5c16c6 usb: pl2303: increase the allowed baud rate range for the divisor 
 based encoding method
 e917ba0 usb: pl2303: move the two baud rate encoding methods to separate 
 functions
 b9208c7 usb: pl2303: remove 50 baud from the list of standard baud rates
 75417d9 usb: pl2303: do not round to the next nearest standard baud rate for 
 the divisor based baud rate encoding method
 57ce61a usb: pl2303: fix+improve the divsor based baud rate encoding method
 b8bdad6 USB: pl2303: restrict the divisor based baud rate encoding method to 
 the HX chip type
 No need to revert all these patches, removing the calculation of the
 resulting baud rateat the end of fcn pl2303_baudrate_encode_divisor()
 should fix this issue.
 The remaining question is, does the HXD work with the improved divisor
 method or do we have to reintroduce the old method ?

 Regards,
 Frank

 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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Johan Hovold

On Fri, Nov 01, 2013 at 11:06:40AM +0100, Frank Schäfer wrote:
 Am 31.10.2013 21:56, schrieb Johan Hovold:
  On Thu, Oct 31, 2013 at 09:26:06PM +0100, Johan Hovold wrote:
  On Thu, Oct 31, 2013 at 7:45 PM, Frank Schäfer wrote:
  Am 31.10.2013 13:30, schrieb Mika Westerberg:
  On Thu, Oct 31, 2013 at 01:02:56PM +0100, Frank Schäfer wrote:
  2) comment out the following line at the end of
  pl2303_baudrate_encode_divisor_HXD():
 
  baud = 1200 * 32 / ((1  buf[1]) * buf[0]);
  This seems to fix the problem :)
 
  Once the line is commented out, the serial console works fine with the
  speeds I've been testing (115200, 230400 and 460800).
  Urgh... so it seems getty gets confused if the actually set baud rate is
  reported back.
  The kernel still reports the standard baud rate (e.g. B230400), it only
  sets the exact value in the c_ispeed and c_ospeed fields of the termios
  struct.
  So even if getty doesn't support non-standard baud rates, everything
  should be fine.
  I'll have to take a closer look at this issue later.
  I think I know what's going on. The pl2303 is known to loose bytes when
  changing the termios settings (see commit bf5e5834b (pl2303: Fix mode
  switching regression)).
 
  Your commit 57ce61aad (usb: pl2303: fix+improve the divisor based baud
  rate encoding method) introduced a regression when it started reporting
  back the baudrates determined by the divisor algorithm.
 
  Since that commit, tty_termios_hw_change(tty-termios, old_termios)
  will always return true -- even when userspace isn't requesting a
  different baudrate.
 Hmm... so there is a bug in tty_termios_hw_change() ?

Not necessarily.

  Ok, so now let's see if the fixed/improved divisor based method also
  works for the HXD chip if we don't report the actually set baud rate.
  Try commenting out the line
 
  baud = 1200 * 32 / ((1  A) * B);
 
  at the end of pl2303_baudrate_encode_divisor() of a clean 3.12-rc kernel.
  It's unlikely that this makes baud rates  115200 working, but testing
  both cases doesn't harm. ;)
  I can trigger the problem here with my HXD/EA/RA/SA-device as well, and
  I've verified that not returning the calculated baudrate makes this
  particular issue *seem* to go away.
 
  This obviously has to be fixed or reverted quickly.
 Indeed.
 
  Greg, what do you say about this? Is reverting for 3.12 the correct way
  to deal with this, and make sure these fairly invasive patches get some
  more testing before being reapplied?
 
  Reverting would mean reverting a whole bunch of commits though, as they
  appear to depend quite heavily on each other:
 
  7d26a78 USB: pl2303: distinguish between original and cloned HX chips
  034d152 pl2303: improve the chip type detection/distinction
  a77a8c2 pl2303: improve the chip type information output on startup
  73b583a pl2303: simplify the else-if contruct for type_1 chips in 
  pl2303_startup()
  c23bda3 usb: pl2303: add two comments concerning the supported baud rates 
  with HX chips
  61fa8d6 usb: pl2303: also use the divisor based baud rate encoding method 
  for baud rates  115200 with HX chips
  b5c16c6 usb: pl2303: increase the allowed baud rate range for the divisor 
  based encoding method
  e917ba0 usb: pl2303: move the two baud rate encoding methods to separate 
  functions
  b9208c7 usb: pl2303: remove 50 baud from the list of standard baud rates
  75417d9 usb: pl2303: do not round to the next nearest standard baud rate 
  for the divisor based baud rate encoding method
  57ce61a usb: pl2303: fix+improve the divsor based baud rate encoding method
  b8bdad6 USB: pl2303: restrict the divisor based baud rate encoding method 
  to the HX chip type
 No need to revert all these patches, removing the calculation of the
 resulting baud rateat the end of fcn pl2303_baudrate_encode_divisor()
 should fix this issue.

It could, if the problem I identified above is the only issue here.
However, we have a clear regression and 3.12 is coming out very, very
soon.

 The remaining question is, does the HXD work with the improved divisor
 method or do we have to reintroduce the old method ?

This is exactly the kind of issues we can discuss and investigate after
reverting the changes. Let's not break any systems meanwhile.

I think that adding support for odd baud rates (and the improved device
detection) can be implemented in a cleaner way and want to have a chance
to discuss that with you.

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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Mika Westerberg

On Fri, Nov 01, 2013 at 11:06:40AM +0100, Frank Schäfer wrote:
 No need to revert all these patches, removing the calculation of the
 resulting baud rateat the end of fcn pl2303_baudrate_encode_divisor()
 should fix this issue.
 The remaining question is, does the HXD work with the improved divisor
 method or do we have to reintroduce the old method ?

It works, with only removing the baud rate calculation.
--
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: chipidea: host: add vbus regulator control

2013-11-01 Thread Dan Carpenter

Hello Peter Chen,

This is a semi-automatic email about new static checker warnings.

The patch 40ed51a4b858: usb: chipidea: host: add vbus regulator 
control from Aug 14, 2013, leads to the following Smatch complaint:

drivers/usb/chipidea/host.c:91 host_start()
 error: we previously assumed 'ci-platdata-reg_vbus' could be null 
(see line 69)

drivers/usb/chipidea/host.c
68  
69  if (ci-platdata-reg_vbus) {
^^
Patch introduces a new NULL check.

70  ret = regulator_enable(ci-platdata-reg_vbus);
71  if (ret) {
72  dev_err(ci-dev,
73  Failed to enable vbus regulator, 
ret=%d\n,
74  ret);
75  goto put_hcd;
76  }
77  }
78  
79  ret = usb_add_hcd(hcd, 0, 0);
80  if (ret)
81  goto disable_reg;
82  else
83  ci-hcd = hcd;
84  
85  if (ci-platdata-flags  CI_HDRC_DISABLE_STREAMING)
86  hw_write(ci, OP_USBMODE, USBMODE_CI_SDIS, 
USBMODE_CI_SDIS);
87  
88  return ret;
89  
90  disable_reg:
91  regulator_disable(ci-platdata-reg_vbus);
  ^^
Patch introduces a new unchecked NULL dereference.

92  
93  put_hcd:

regards,
dan carpenter
--
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 net-next 17/24] net: cdc_ncm: use netif_* and dev_* instead of pr_*

2013-11-01 Thread Joe Perches

On Fri, 2013-11-01 at 11:16 +0100, Bjørn Mork wrote:
 Take advantage of standard device name prefixing and
 netdevice msglvl control where possible.

Nice, thanks.

You did most all the multi-line statement
alignment perfectly but missed a couple.

Maybe in a follow-on patch.

 diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
[]
 @@ -1031,17 +1035,13 @@ cdc_ncm_speed_change(struct usbnet *dev,
* device speed. Do print it instead.
*/
   if ((tx_speed  100)  (rx_speed  100)) {
 - printk(KERN_INFO KBUILD_MODNAME
 -: %s: %u mbit/s downlink 
 -%u mbit/s uplink\n,
 -dev-net-name,
 + netif_info(dev, link, dev-net,
 +%u mbit/s downlink %u mbit/s uplink\n,
  (unsigned int)(rx_speed / 100U),
  (unsigned int)(tx_speed / 100U));
   } else {
 - printk(KERN_INFO KBUILD_MODNAME
 -: %s: %u kbit/s downlink 
 -%u kbit/s uplink\n,
 -dev-net-name,
 + netif_info(dev, link, dev-net,
 +%u kbit/s downlink %u kbit/s uplink\n,
  (unsigned int)(rx_speed / 1000U),
  (unsigned int)(tx_speed / 1000U));
   }


--
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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Frank Schäfer

Am 01.11.2013 11:22, schrieb Johan Hovold:
 Greg, what do you say about this? Is reverting for 3.12 the correct way
 to deal with this, and make sure these fairly invasive patches get some
 more testing before being reapplied?

 Reverting would mean reverting a whole bunch of commits though, as they
 appear to depend quite heavily on each other:

 7d26a78 USB: pl2303: distinguish between original and cloned HX chips
 034d152 pl2303: improve the chip type detection/distinction
 a77a8c2 pl2303: improve the chip type information output on startup
 73b583a pl2303: simplify the else-if contruct for type_1 chips in 
 pl2303_startup()
 c23bda3 usb: pl2303: add two comments concerning the supported baud rates 
 with HX chips
 61fa8d6 usb: pl2303: also use the divisor based baud rate encoding method 
 for baud rates  115200 with HX chips
 b5c16c6 usb: pl2303: increase the allowed baud rate range for the divisor 
 based encoding method
 e917ba0 usb: pl2303: move the two baud rate encoding methods to separate 
 functions
 b9208c7 usb: pl2303: remove 50 baud from the list of standard baud rates
 75417d9 usb: pl2303: do not round to the next nearest standard baud rate 
 for the divisor based baud rate encoding method
 57ce61a usb: pl2303: fix+improve the divsor based baud rate encoding method
 b8bdad6 USB: pl2303: restrict the divisor based baud rate encoding method 
 to the HX chip type
 No need to revert all these patches, removing the calculation of the
 resulting baud rateat the end of fcn pl2303_baudrate_encode_divisor()
 should fix this issue.
 It could, if the problem I identified above is the only issue here.
Well, unless you don't find another regression, I see no reason to
revert. ;)

 However, we have a clear regression and 3.12 is coming out very, very
 soon.
Indeed, but fortunately we are also close to a solution/decision.


 The remaining question is, does the HXD work with the improved divisor
 method or do we have to reintroduce the old method ?
 This is exactly the kind of issues we can discuss and investigate after
 reverting the changes. Let's not break any systems meanwhile.
Well, Mika and you have have such a device, so you can test it.
Alternatively, we can apply the patch I've sent that restores the old
behavior (of course without the calculation of the resulting baud rate).

 I think that adding support for odd baud rates (and the improved device
 detection) can be implemented in a cleaner way and want to have a chance
 to discuss that with you.
Ok, between the lines I can read that you just don't trust this patch
series. ;)
I'm looking forward to your suggestions for a cleaner way to implement
this. :)

Regards,
Frank


 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: xhci: Less verbose tracing of short receives

2013-11-01 Thread David Laight

 Subject: Re: [PATCH] usb: xhci: Less verbose tracing of short receives
 
 On Thu, 31 Oct 2013, David Laight wrote:
 
  Only receive TD can have a transfer length less than the transfer size,
 
 Without commenting on the patch itself, let me point out that this
 statement is wrong.  An OUT transfer can terminate early for several
 reasons.  They are all error conditions, however, whereas an IN
 transfer can terminate early without any error.

That is a bit of a nit-pick.
The TRB completion code of 'Short packet' (13) only applies to
IN transfers, if all the data of a OUT transfer isn't sent then a
different error code is returned.

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: [PATCH] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Frank Schäfer

Am 01.11.2013 11:39, schrieb Mika Westerberg:
 On Fri, Nov 01, 2013 at 11:06:40AM +0100, Frank Schäfer wrote:
 No need to revert all these patches, removing the calculation of the
 resulting baud rateat the end of fcn pl2303_baudrate_encode_divisor()
 should fix this issue.
 The remaining question is, does the HXD work with the improved divisor
 method or do we have to reintroduce the old method ?
 It works, with only removing the baud rate calculation.
For both baud rates = 115200 AND  115200 ?

Frank
--
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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Mika Westerberg

On Fri, Nov 01, 2013 at 11:45:32AM +0100, Frank Schäfer wrote:
 Am 01.11.2013 11:39, schrieb Mika Westerberg:
  On Fri, Nov 01, 2013 at 11:06:40AM +0100, Frank Schäfer wrote:
  No need to revert all these patches, removing the calculation of the
  resulting baud rateat the end of fcn pl2303_baudrate_encode_divisor()
  should fix this issue.
  The remaining question is, does the HXD work with the improved divisor
  method or do we have to reintroduce the old method ?
  It works, with only removing the baud rate calculation.
 For both baud rates = 115200 AND  115200 ?

Yes.
--
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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Johan Hovold

On Fri, Nov 01, 2013 at 11:44:33AM +0100, Frank Schäfer wrote:
 Am 01.11.2013 11:22, schrieb Johan Hovold:
  Greg, what do you say about this? Is reverting for 3.12 the correct way
  to deal with this, and make sure these fairly invasive patches get some
  more testing before being reapplied?
 
  Reverting would mean reverting a whole bunch of commits though, as they
  appear to depend quite heavily on each other:
 
  7d26a78 USB: pl2303: distinguish between original and cloned HX chips
  034d152 pl2303: improve the chip type detection/distinction
  a77a8c2 pl2303: improve the chip type information output on startup
  73b583a pl2303: simplify the else-if contruct for type_1 chips in 
  pl2303_startup()
  c23bda3 usb: pl2303: add two comments concerning the supported baud rates 
  with HX chips
  61fa8d6 usb: pl2303: also use the divisor based baud rate encoding method 
  for baud rates  115200 with HX chips
  b5c16c6 usb: pl2303: increase the allowed baud rate range for the divisor 
  based encoding method
  e917ba0 usb: pl2303: move the two baud rate encoding methods to separate 
  functions
  b9208c7 usb: pl2303: remove 50 baud from the list of standard baud 
  rates
  75417d9 usb: pl2303: do not round to the next nearest standard baud rate 
  for the divisor based baud rate encoding method
  57ce61a usb: pl2303: fix+improve the divsor based baud rate encoding 
  method
  b8bdad6 USB: pl2303: restrict the divisor based baud rate encoding method 
  to the HX chip type
  No need to revert all these patches, removing the calculation of the
  resulting baud rateat the end of fcn pl2303_baudrate_encode_divisor()
  should fix this issue.
  It could, if the problem I identified above is the only issue here.
 Well, unless you don't find another regression, I see no reason to
 revert. ;)
 
  However, we have a clear regression and 3.12 is coming out very, very
  soon.
 Indeed, but fortunately we are also close to a solution/decision.
 
  The remaining question is, does the HXD work with the improved divisor
  method or do we have to reintroduce the old method ?
  This is exactly the kind of issues we can discuss and investigate after
  reverting the changes. Let's not break any systems meanwhile.
 Well, Mika and you have have such a device, so you can test it.

I'm afraid I can't just drop everything else I'm working with to do that
testing at the moment.

 Alternatively, we can apply the patch I've sent that restores the old
 behavior (of course without the calculation of the resulting baud rate).

But that's another change. Late change.

  I think that adding support for odd baud rates (and the improved device
  detection) can be implemented in a cleaner way and want to have a chance
  to discuss that with you.
 Ok, between the lines I can read that you just don't trust this patch
 series. ;)
 I'm looking forward to your suggestions for a cleaner way to implement
 this. :)

You have done a great job here in reverse-engineering the chip and
documenting the quirks. It's not an easy job at all with all the
different chips out there, including pirate ones.

I just think we should take the safe way here and hold off the changes
another cycle. Get some more people to do more testing (which chip types
do we have among us at the moment?). This would also allow us to come up
with a series which abstracts the differences in a cleaner way rather
than spreading this information all over the driver. I'm aware that it
has grown to that state incrementally, but at this point I think it's
clear that some abstraction is needed.

I also believe using the direct encoding method also for (known,
supported) baudrates  115200, as you suggested somewhere, should be
considered (and only fall back to divisor method for other rates if the
chip supports it).

Johan
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V5 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver

2013-11-01 Thread Bjørn Mork

Oliver Neukum oneu...@suse.de writes:
 On Mon, 2013-09-30 at 04:50 +, Enrico Mioso wrote:

 +static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
 +{
 +struct huawei_cdc_ncm_state *drvstate = (void *)usbnet_dev-data;
 +int rv = 0;
 +
 +if ((on  atomic_add_return(1, drvstate-pmcount) == 1) ||
 +(!on  atomic_dec_and_test(drvstate-pmcount))) {
 +rv = usb_autopm_get_interface(usbnet_dev-intf);
 +if (rv  0)
 +goto err;

 The error case corrupts drvstate-pmcount

 +usbnet_dev-intf-needs_remote_wakeup = on;
 +usb_autopm_put_interface(usbnet_dev-intf);
 +}
 +err:
 +return rv;
 +}

Hello Oliver,

I finally got around to looking closer at this and you are of course
correct.  This is all my fault when I initially wrapped the
needs_remote_wakeup update in usb_autopm_{get,put}_interface,

And the problem is not only here in this new driver, but *everywhere* I
did this, including in cdc-wdm.  That driver doesn't have the counter to
corrupt, but failing to update intf-needs_remote_wakeup if
usb_autopm_get_interface fails is still wrong. The get/put were only
added to make the change take effect immediately.  Things sort of worked
fine before that, and ignoring a get error would only revert to that
older behaviour.

So I believe we should do the update unconditionally, and but skip
usb_autopm_put_interface if the get failed.  Accordingly, these
functions should always return 0 (not that there is anything currently
checking the return anyway).

I'll prepare patches for cdc-wdm, qmi_wwan and cdc_mbim.


Bjørn
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/RFT PATCH] pl2303: avoid data corruption with some chip types

2013-11-01 Thread Frank Schäfer

Some PL2303 chips are reported to lose bytes if the serial settings are set to
the same values as before.
At least HXD chips are affected, HX chips seem to be ok.
The current code tries to avoid this by calling tty_termios_hw_change(), but
this check isn't sufficient because different requested baud rates can result
in identic encoded baud rates.
The solution is to save and compare the encoded settings instead.

Signed-off-by: Frank Schäfer fschaefer@googlemail.com
Cc: sta...@kernel.org
---
 drivers/usb/serial/pl2303.c |   22 ++
 1 Datei geändert, 14 Zeilen hinzugefügt(+), 8 Zeilen entfernt(-)

diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index bedf8e4..27a2691 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -155,6 +155,7 @@ struct pl2303_private {
spinlock_t lock;
u8 line_control;
u8 line_status;
+   u8 line_settings_enc[7]; /* baud rate, data bits, stop bits, parity */
 };
 
 static int pl2303_vendor_read(__u16 value, __u16 index,
@@ -500,14 +501,6 @@ static void pl2303_set_termios(struct tty_struct *tty,
int i;
u8 control;
 
-   /*
-* The PL2303 is reported to lose bytes if you change serial settings
-* even to the same values as before. Thus we actually need to filter
-* in this specific case.
-*/
-   if (old_termios  !tty_termios_hw_change(tty-termios, old_termios))
-   return;
-
buf = kzalloc(7, GFP_KERNEL);
if (!buf) {
dev_err(port-dev, %s - out of memory.\n, __func__);
@@ -591,10 +584,23 @@ static void pl2303_set_termios(struct tty_struct *tty,
dev_dbg(port-dev, parity = none\n);
}
 
+   /*
+* Some PL2303 chips are reported to lose bytes if you change the
+* serial settings even to the same values as before.
+* At least HXD chips are affected, HX chips seem to be ok.
+* Thus we actually need to filter in this specific case.
+* NOTE: a tty_termios_hw_change() check isn't sufficient, because
+* different baud rates can result in identic encoded values.
+*/
+   if (!memcmp(priv-line_settings_enc, buf, 7))
+   return;
+
i = usb_control_msg(serial-dev, usb_sndctrlpipe(serial-dev, 0),
SET_LINE_REQUEST, SET_LINE_REQUEST_TYPE,
0, 0, buf, 7, 100);
dev_dbg(port-dev, 0x21:0x20:0:0  %d\n, i);
+   if (i == 7)
+   memcpy(buf, priv-line_settings_enc, 7);
 
/* change control lines if we are switching to or from B0 */
spin_lock_irqsave(priv-lock, flags);
-- 
1.7.10.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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Frank Schäfer

Am 01.11.2013 11:59, schrieb Mika Westerberg:
 On Fri, Nov 01, 2013 at 11:45:32AM +0100, Frank Schäfer wrote:
 Am 01.11.2013 11:39, schrieb Mika Westerberg:
 On Fri, Nov 01, 2013 at 11:06:40AM +0100, Frank Schäfer wrote:
 No need to revert all these patches, removing the calculation of the
 resulting baud rateat the end of fcn pl2303_baudrate_encode_divisor()
 should fix this issue.
 The remaining question is, does the HXD work with the improved divisor
 method or do we have to reintroduce the old method ?
 It works, with only removing the baud rate calculation.
 For both baud rates = 115200 AND  115200 ?
 Yes.
Great ! Everything else would have been surprising.
I've sent a better patch that should fix this issue 3 minutes ago. Can
you test it ?

Regards
Frank
--
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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Frank Schäfer

Am 01.11.2013 12:15, schrieb Johan Hovold:
 On Fri, Nov 01, 2013 at 11:44:33AM +0100, Frank Schäfer wrote:
 Am 01.11.2013 11:22, schrieb Johan Hovold:
 Greg, what do you say about this? Is reverting for 3.12 the correct way
 to deal with this, and make sure these fairly invasive patches get some
 more testing before being reapplied?

 Reverting would mean reverting a whole bunch of commits though, as they
 appear to depend quite heavily on each other:

 7d26a78 USB: pl2303: distinguish between original and cloned HX chips
 034d152 pl2303: improve the chip type detection/distinction
 a77a8c2 pl2303: improve the chip type information output on startup
 73b583a pl2303: simplify the else-if contruct for type_1 chips in 
 pl2303_startup()
 c23bda3 usb: pl2303: add two comments concerning the supported baud rates 
 with HX chips
 61fa8d6 usb: pl2303: also use the divisor based baud rate encoding method 
 for baud rates  115200 with HX chips
 b5c16c6 usb: pl2303: increase the allowed baud rate range for the divisor 
 based encoding method
 e917ba0 usb: pl2303: move the two baud rate encoding methods to separate 
 functions
 b9208c7 usb: pl2303: remove 50 baud from the list of standard baud 
 rates
 75417d9 usb: pl2303: do not round to the next nearest standard baud rate 
 for the divisor based baud rate encoding method
 57ce61a usb: pl2303: fix+improve the divsor based baud rate encoding 
 method
 b8bdad6 USB: pl2303: restrict the divisor based baud rate encoding method 
 to the HX chip type
 No need to revert all these patches, removing the calculation of the
 resulting baud rateat the end of fcn pl2303_baudrate_encode_divisor()
 should fix this issue.
 It could, if the problem I identified above is the only issue here.
 Well, unless you don't find another regression, I see no reason to
 revert. ;)

 However, we have a clear regression and 3.12 is coming out very, very
 soon.
 Indeed, but fortunately we are also close to a solution/decision.

 The remaining question is, does the HXD work with the improved divisor
 method or do we have to reintroduce the old method ?
 This is exactly the kind of issues we can discuss and investigate after
 reverting the changes. Let's not break any systems meanwhile.
 Well, Mika and you have have such a device, so you can test it.
 I'm afraid I can't just drop everything else I'm working with to do that
 testing at the moment.
No problem. :)
In the meantime, Mika has tested it and the improved divisor method
works fine.


 Alternatively, we can apply the patch I've sent that restores the old
 behavior (of course without the calculation of the resulting baud rate).
 But that's another change. Late change.
[superseeded]


 I think that adding support for odd baud rates (and the improved device
 detection) can be implemented in a cleaner way and want to have a chance
 to discuss that with you.
 Ok, between the lines I can read that you just don't trust this patch
 series. ;)
 I'm looking forward to your suggestions for a cleaner way to implement
 this. :)
 You have done a great job here in reverse-engineering the chip and
 documenting the quirks. It's not an easy job at all with all the
 different chips out there, including pirate ones.

 I just think we should take the safe way here and hold off the changes
 another cycle. Get some more people to do more testing (which chip types
 do we have among us at the moment?). This would also allow us to come up
 with a series which abstracts the differences in a cleaner way rather
 than spreading this information all over the driver. I'm aware that it
 has grown to that state incrementally, but at this point I think it's
 clear that some abstraction is needed.
You're the maintainer, the decission is of course up to you.
I have generally no problem with reverting patches. If something isn't
ready or plain crap, it should of course be reverted.
But AFAICS, the problem here isn't the patch series itself. It's just
that this series unveils another long standing bug.

Please also consider that the patch I've sent 5 minutes ago should also
needs to be applied to stable.
We have the same issue there and it's not restricted to the divisor
based baud rate encoding method.
The same can happen with the direct encoding method...

So IMHO, going a step forward and fixing this bug is the better solution.

Regards,
Frank

 I also believe using the direct encoding method also for (known,
 supported) baudrates  115200, as you suggested somewhere, should be
 considered (and only fall back to divisor method for other rates if the
 chip supports it).

 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: [RFC/RFT PATCH] pl2303: avoid data corruption with some chip types

2013-11-01 Thread Mika Westerberg

On Fri, Nov 01, 2013 at 12:49:22PM +0100, Frank Schäfer wrote:
 Some PL2303 chips are reported to lose bytes if the serial settings are set to
 the same values as before.
 At least HXD chips are affected, HX chips seem to be ok.
 The current code tries to avoid this by calling tty_termios_hw_change(), but
 this check isn't sufficient because different requested baud rates can result
 in identic encoded baud rates.
 The solution is to save and compare the encoded settings instead.

I tried this on top of rc7 and I can still see data get corrupted (both
115200 and 460800 bps).
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] Revert sierra_net: keep status interrupt URB active

2013-11-01 Thread Bjørn Mork

This reverts commit 7b0c5f21f348a66de495868b8df0284e8dfd6bbf.

It's not easy to create a driver for all the various firmware
bugs out there.

This change caused regressions for a number of devices, which
started to fail link detection and therefore became completely
non-functional. The exact reason is yet unknown, it looks like
the affected firmwares might actually need all or some of the
additional SYNC messages the patch got rid of.

Reverting is not optimal, as it will re-introduce the original
problem, but it is currently the only alternative known to fix
this issue.

Cc: Dan Williams d...@redhat.com
Link: https://bugzilla.redhat.com/show_bug.cgi?id=998342
Reported-by: John Henderson j...@talk21.com
Signed-off-by: Bjørn Mork bj...@mork.no
---
Hello Dan,

I don't know if you've noticed the bug report referenced above
and the discussions taking place down under.  E.g.:
http://forums.whirlpool.net.au/forum-replies.cfm?t=2135188p=15#r300

It seems that a number of devices just won't work at all with
the fix for the SYNC problem.  Unfortunately, the only DirectIP
device I've got (MC7710) doesn't have any of these firmware problems,
so I have been unable to come up with any better solution than
reverting.  Which has been confirmed to fix the problem.

My only real alternative proposal, delaying the first SYNC message
to give the firmware some slack, did not work:
http://forums.whirlpool.net.au/forum-replies.cfm?t=2135188p=16#r312

So unless you or someone else has a better idea, I don't really
see any other option than reverting the patch.  I am posting
this as an RFC initially as I believe we need to discuss the
options.


Bjørn

 drivers/net/usb/sierra_net.c |   38 +++---
 1 file changed, 7 insertions(+), 31 deletions(-)

diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index a79e9d3..a923d61 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -426,13 +426,6 @@ static void sierra_net_dosync(struct usbnet *dev)
 
dev_dbg(dev-udev-dev, %s, __func__);
 
-   /* The SIERRA_NET_HIP_MSYNC_ID command appears to request that the
-* firmware restart itself.  After restarting, the modem will respond
-* with the SIERRA_NET_HIP_RESTART_ID indication.  The driver continues
-* sending MSYNC commands every few seconds until it receives the
-* RESTART event from the firmware
-*/
-
/* tell modem we are ready */
status = sierra_net_send_sync(dev);
if (status  0)
@@ -711,9 +704,6 @@ static int sierra_net_bind(struct usbnet *dev, struct 
usb_interface *intf)
/* set context index initially to 0 - prepares tx hdr template */
sierra_net_set_ctx_index(priv, 0);
 
-   /* prepare sync message template */
-   memcpy(priv-sync_msg, sync_tmplate, sizeof(priv-sync_msg));
-
/* decrease the rx_urb_size and max_tx_size to 4k on USB 1.1 */
dev-rx_urb_size  = SIERRA_NET_RX_URB_SIZE;
if (dev-udev-speed != USB_SPEED_HIGH)
@@ -749,6 +739,11 @@ static int sierra_net_bind(struct usbnet *dev, struct 
usb_interface *intf)
kfree(priv);
return -ENODEV;
}
+   /* prepare sync message from template */
+   memcpy(priv-sync_msg, sync_tmplate, sizeof(priv-sync_msg));
+
+   /* initiate the sync sequence */
+   sierra_net_dosync(dev);
 
return 0;
 }
@@ -771,9 +766,8 @@ static void sierra_net_unbind(struct usbnet *dev, struct 
usb_interface *intf)
netdev_err(dev-net,
usb_control_msg failed, status %d\n, status);
 
-   usbnet_status_stop(dev);
-
sierra_net_set_private(dev, NULL);
+
kfree(priv);
 }
 
@@ -914,24 +908,6 @@ static const struct driver_info sierra_net_info_direct_ip 
= {
.tx_fixup = sierra_net_tx_fixup,
 };
 
-static int
-sierra_net_probe(struct usb_interface *udev, const struct usb_device_id *prod)
-{
-   int ret;
-
-   ret = usbnet_probe(udev, prod);
-   if (ret == 0) {
-   struct usbnet *dev = usb_get_intfdata(udev);
-
-   ret = usbnet_status_start(dev, GFP_KERNEL);
-   if (ret == 0) {
-   /* Interrupt URB now set up; initiate sync sequence */
-   sierra_net_dosync(dev);
-   }
-   }
-   return ret;
-}
-
 #define DIRECT_IP_DEVICE(vend, prod) \
{USB_DEVICE_INTERFACE_NUMBER(vend, prod, 7), \
.driver_info = (unsigned long)sierra_net_info_direct_ip}, \
@@ -954,7 +930,7 @@ MODULE_DEVICE_TABLE(usb, products);
 static struct usb_driver sierra_net_driver = {
.name = sierra_net,
.id_table = products,
-   .probe = sierra_net_probe,
+   .probe = usbnet_probe,
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to

Re: [RFC/RFT PATCH] pl2303: avoid data corruption with some chip types

2013-11-01 Thread Frank Schäfer

Am 01.11.2013 13:19, schrieb Mika Westerberg:
 On Fri, Nov 01, 2013 at 12:49:22PM +0100, Frank Schäfer wrote:
 Some PL2303 chips are reported to lose bytes if the serial settings are set 
 to
 the same values as before.
 At least HXD chips are affected, HX chips seem to be ok.
 The current code tries to avoid this by calling tty_termios_hw_change(), but
 this check isn't sufficient because different requested baud rates can result
 in identic encoded baud rates.
 The solution is to save and compare the encoded settings instead.
 I tried this on top of rc7 and I can still see data get corrupted (both
 115200 and 460800 bps).
Ok... Ready for the next round ? ;)
Attached are two further experimental patches.

Regards,
Frank
From 95a3f0e0751dad4ae39fdd7a915a2fa22997a540 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Frank=20Sch=C3=A4fer?= fschaefer@googlemail.com
Date: Fri, 1 Nov 2013 13:56:53 +0100
Subject: [PATCH] EXPERIMENTAL PATCH 1
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Frank SchÃ¤fer fschaefer@googlemail.com
---
 drivers/usb/serial/pl2303.c |   30 ++
 1 Datei geÃ¤ndert, 18 Zeilen hinzugefÃ¼gt(+), 12 Zeilen entfernt(-)

diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index bedf8e4..8496eef 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -496,20 +496,13 @@ static void pl2303_set_termios(struct tty_struct *tty,
 	struct pl2303_serial_private *spriv = usb_get_serial_data(serial);
 	struct pl2303_private *priv = usb_get_serial_port_data(port);
 	unsigned long flags;
-	unsigned char *buf;
+	unsigned char *buf, *buf_old;
 	int i;
 	u8 control;
 
-	/*
-	 * The PL2303 is reported to lose bytes if you change serial settings
-	 * even to the same values as before. Thus we actually need to filter
-	 * in this specific case.
-	 */
-	if (old_termios  !tty_termios_hw_change(tty-termios, old_termios))
-		return;
-
 	buf = kzalloc(7, GFP_KERNEL);
-	if (!buf) {
+	buf_old = kzalloc(7, GFP_KERNEL);
+	if (!buf || buf_old) {
 		dev_err(port-dev, %s - out of memory.\n, __func__);
 		/* Report back no change occurred */
 		if (old_termios)
@@ -519,8 +512,8 @@ static void pl2303_set_termios(struct tty_struct *tty,
 
 	i = usb_control_msg(serial-dev, usb_rcvctrlpipe(serial-dev, 0),
 			GET_LINE_REQUEST, GET_LINE_REQUEST_TYPE,
-			0, 0, buf, 7, 100);
-	dev_dbg(port-dev, 0xa1:0x21:0:0  %d - %7ph\n, i, buf);
+			0, 0, buf_old, 7, 100);
+	dev_dbg(port-dev, 0xa1:0x21:0:0  %d - %7ph\n, i, buf_old);
 
 	if (C_CSIZE(tty)) {
 		switch (C_CSIZE(tty)) {
@@ -591,6 +584,17 @@ static void pl2303_set_termios(struct tty_struct *tty,
 		dev_dbg(port-dev, parity = none\n);
 	}
 
+	/*
+	 * Some PL2303 chips are reported to lose bytes if you change the
+	 * serial settings even to the same values as before.
+	 * At least HXD chips are affected, HX chips seem to be ok.
+	 * Thus we actually need to filter in this specific case.
+	 * NOTE: a tty_termios_hw_change() check isn't sufficient, because
+	 * different baud rates can result in identic encoded values.
+	 */
+	if (!memcmp(buf, buf_old, 7))
+		goto out;
+
 	i = usb_control_msg(serial-dev, usb_sndctrlpipe(serial-dev, 0),
 			SET_LINE_REQUEST, SET_LINE_REQUEST_TYPE,
 			0, 0, buf, 7, 100);
@@ -626,7 +630,9 @@ static void pl2303_set_termios(struct tty_struct *tty,
 		pl2303_vendor_write(0x0, 0x0, serial);
 	}
 
+out:
 	kfree(buf);
+	kfree(buf_old);
 }
 
 static void pl2303_dtr_rts(struct usb_serial_port *port, int on)
-- 
1.7.10.4

From d303ff67ab1b817bf75b32eaa276369039ee4af2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Frank=20Sch=C3=A4fer?= fschaefer@googlemail.com
Date: Fri, 1 Nov 2013 13:59:58 +0100
Subject: [PATCH] EXPERIMENTAL PATCH 2
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Frank SchÃ¤fer fschaefer@googlemail.com
---
 drivers/usb/serial/pl2303.c |   29 -
 1 Datei geÃ¤ndert, 16 Zeilen hinzugefÃ¼gt(+), 13 Zeilen entfernt(-)

diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index bedf8e4..b0d3d66 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -155,6 +155,7 @@ struct pl2303_private {
 	spinlock_t lock;
 	u8 line_control;
 	u8 line_status;
+	u8 line_settings_enc[7]; /* baud rate, data bits, stop bits, parity */
 };
 
 static int pl2303_vendor_read(__u16 value, __u16 index,
@@ -500,14 +501,6 @@ static void pl2303_set_termios(struct tty_struct *tty,
 	int i;
 	u8 control;
 
-	/*
-	 * The PL2303 is reported to lose bytes if you change serial settings
-	 * even to the same values as before. Thus we actually need to filter
-	 * in this specific case.
-	 */
-	if (old_termios  !tty_termios_hw_change(tty-termios, old_termios))
-		return;
-
 	buf = kzalloc(7, GFP_KERNEL);
 	if (!buf) {
 		dev_err(port-dev, %s - out of memory.\n, __func__);
@@ -517,11 +510,6 @@ static void

Re: [RFC/RFT PATCH] pl2303: avoid data corruption with some chip types

2013-11-01 Thread Johan Hovold

On Fri, Nov 01, 2013 at 12:49:22PM +0100, Frank Schäfer wrote:
 Some PL2303 chips are reported to lose bytes if the serial settings are set to
 the same values as before.
 At least HXD chips are affected, HX chips seem to be ok.
 The current code tries to avoid this by calling tty_termios_hw_change(), but
 this check isn't sufficient because different requested baud rates can result
 in identic encoded baud rates.
 The solution is to save and compare the encoded settings instead.

We're gonna need something along these lines, but this is just a hack
that not even correct.

 Signed-off-by: Frank Schäfer fschaefer@googlemail.com
 Cc: sta...@kernel.org
 ---
  drivers/usb/serial/pl2303.c |   22 ++
  1 Datei geändert, 14 Zeilen hinzugefügt(+), 8 Zeilen entfernt(-)
 
 diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
 index bedf8e4..27a2691 100644
 --- a/drivers/usb/serial/pl2303.c
 +++ b/drivers/usb/serial/pl2303.c
 @@ -155,6 +155,7 @@ struct pl2303_private {
   spinlock_t lock;
   u8 line_control;
   u8 line_status;
 + u8 line_settings_enc[7]; /* baud rate, data bits, stop bits, parity */
  };
  
  static int pl2303_vendor_read(__u16 value, __u16 index,
 @@ -500,14 +501,6 @@ static void pl2303_set_termios(struct tty_struct *tty,
   int i;
   u8 control;
  
 - /*
 -  * The PL2303 is reported to lose bytes if you change serial settings
 -  * even to the same values as before. Thus we actually need to filter
 -  * in this specific case.
 -  */
 - if (old_termios  !tty_termios_hw_change(tty-termios, old_termios))
 - return;
 -

   buf = kzalloc(7, GFP_KERNEL);
   if (!buf) {
   dev_err(port-dev, %s - out of memory.\n, __func__);

Here another call to get the line settings is made. Have you verified
that that isn't related to the lost bytes?


 @@ -591,10 +584,23 @@ static void pl2303_set_termios(struct tty_struct *tty,
   dev_dbg(port-dev, parity = none\n);
   }
  
 + /*
 +  * Some PL2303 chips are reported to lose bytes if you change the
 +  * serial settings even to the same values as before.
 +  * At least HXD chips are affected, HX chips seem to be ok.
 +  * Thus we actually need to filter in this specific case.
 +  * NOTE: a tty_termios_hw_change() check isn't sufficient, because
 +  * different baud rates can result in identic encoded values.
 +  */
 + if (!memcmp(priv-line_settings_enc, buf, 7))
 + return;
 +

Here you're returning if there no baud rate change, which means that
flow control is never handled a bit further down in set_termios.

   i = usb_control_msg(serial-dev, usb_sndctrlpipe(serial-dev, 0),
   SET_LINE_REQUEST, SET_LINE_REQUEST_TYPE,
   0, 0, buf, 7, 100);
   dev_dbg(port-dev, 0x21:0x20:0:0  %d\n, i);
 + if (i == 7)
 + memcpy(buf, priv-line_settings_enc, 7);
  
   /* change control lines if we are switching to or from B0 */
   spin_lock_irqsave(priv-lock, flags);
 -- 
 1.7.10.4

Again, this too much of a hack. There is a long-standing bug that needs
to be addressed. Let's think some more of how best to handle that (and
get to the bottom with exactly what requests are causing it, etc).

The important thing is to fix the regression. We can fix this bug during
the 3.13-cycle if the fix is not too invasive.

Sorry,
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

[PATCH net-next 0/5] cdc_mbim + qmi_wwan trivial fixes

2013-11-01 Thread Bjørn Mork

This series fixes three problems Oliver pointed out during the
review of the new huawei_cdc_ncm driver:
http://patchwork.ozlabs.org/patch/278903/

That innocent driver only used cdc_mbim as a blueprint, and
all the blame should really have gone to me

I do have a similar fix for the manage_power issue in the
cdc-wdm USB class driver as well.  It will be submitted to
linux-usb as soon as Greg opens up his mailbox again :-)


Bjørn Mork (5):
  net: cdc_mbim: manage_power should always set needs_remote_wakeup
  net: qmi_wwan: manage_power should always set needs_remote_wakeup
  net: qmi_wwan: no need to check for resume if suspend exists
  net: cdc_mbim: no need to check for resume if suspend exists
  net: cdc_mbim: fixup error return value

 drivers/net/usb/cdc_mbim.c |   16 ++--
 drivers/net/usb/qmi_wwan.c |   12 +---
 2 files changed, 11 insertions(+), 17 deletions(-)

-- 
1.7.10.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

[PATCH net-next 2/5] net: qmi_wwan: manage_power should always set needs_remote_wakeup

2013-11-01 Thread Bjørn Mork

Reported-by: Oliver Neukum oneu...@suse.de
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/qmi_wwan.c |   10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index e0a4a2b..5dd0d85 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -149,7 +149,7 @@ static const struct net_device_ops qmi_wwan_netdev_ops = {
 static int qmi_wwan_manage_power(struct usbnet *dev, int on)
 {
struct qmi_wwan_state *info = (void *)dev-data;
-   int rv = 0;
+   int rv;
 
dev_dbg(dev-intf-dev, %s() pmcount=%d, on=%d\n, __func__,
atomic_read(info-pmcount), on);
@@ -160,13 +160,11 @@ static int qmi_wwan_manage_power(struct usbnet *dev, int 
on)
 * the new value
 */
rv = usb_autopm_get_interface(dev-intf);
-   if (rv  0)
-   goto err;
dev-intf-needs_remote_wakeup = on;
-   usb_autopm_put_interface(dev-intf);
+   if (!rv)
+   usb_autopm_put_interface(dev-intf);
}
-err:
-   return rv;
+   return 0;
 }
 
 static int qmi_wwan_cdc_wdm_manage_power(struct usb_interface *intf, int on)
-- 
1.7.10.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

[PATCH net-next 1/5] net: cdc_mbim: manage_power should always set needs_remote_wakeup

2013-11-01 Thread Bjørn Mork

Reported-by: Oliver Neukum oneu...@suse.de
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_mbim.c |8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 25ba7ec..172059e 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -42,13 +42,11 @@ static int cdc_mbim_manage_power(struct usbnet *dev, int on)
if ((on  atomic_add_return(1, info-pmcount) == 1) || (!on  
atomic_dec_and_test(info-pmcount))) {
/* need autopm_get/put here to ensure the usbcore sees the new 
value */
rv = usb_autopm_get_interface(dev-intf);
-   if (rv  0)
-   goto err;
dev-intf-needs_remote_wakeup = on;
-   usb_autopm_put_interface(dev-intf);
+   if (!rv)
+   usb_autopm_put_interface(dev-intf);
}
-err:
-   return rv;
+   return 0;
 }
 
 static int cdc_mbim_wdm_manage_power(struct usb_interface *intf, int status)
-- 
1.7.10.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

[PATCH net-next 5/5] net: cdc_mbim: fixup error return value

2013-11-01 Thread Bjørn Mork

Reported-by: Oliver Neukum oneu...@suse.de
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_mbim.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 862d137..166e195 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -311,15 +311,13 @@ error:
 
 static int cdc_mbim_suspend(struct usb_interface *intf, pm_message_t message)
 {
-   int ret = 0;
+   int ret = -ENODEV;
struct usbnet *dev = usb_get_intfdata(intf);
struct cdc_mbim_state *info = (void *)dev-data;
struct cdc_ncm_ctx *ctx = info-ctx;
 
-   if (ctx == NULL) {
-   ret = -1;
+   if (!ctx)
goto error;
-   }
 
/*
 * Both usbnet_suspend() and subdriver-suspend() MUST return 0
-- 
1.7.10.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

[PATCH net-next 3/5] net: qmi_wwan: no need to check for resume if suspend exists

2013-11-01 Thread Bjørn Mork

Reported-by: Oliver Neukum oneu...@suse.de
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/qmi_wwan.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 5dd0d85..23bdd5b 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -416,7 +416,7 @@ static int qmi_wwan_resume(struct usb_interface *intf)
if (ret  0)
goto err;
ret = usbnet_resume(intf);
-   if (ret  0  callsub  info-subdriver-suspend)
+   if (ret  0  callsub)
info-subdriver-suspend(intf, PMSG_SUSPEND);
 err:
return ret;
-- 
1.7.10.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

[PATCH net-next 4/5] net: cdc_mbim: no need to check for resume if suspend exists

2013-11-01 Thread Bjørn Mork

Reported-by: Oliver Neukum oneu...@suse.de
Signed-off-by: Bjørn Mork bj...@mork.no
---
 drivers/net/usb/cdc_mbim.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 172059e..862d137 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -352,7 +352,7 @@ static int cdc_mbim_resume(struct usb_interface *intf)
if (ret  0)
goto err;
ret = usbnet_resume(intf);
-   if (ret  0  callsub  info-subdriver-suspend)
+   if (ret  0  callsub)
info-subdriver-suspend(intf, PMSG_SUSPEND);
 err:
return ret;
-- 
1.7.10.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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Frank Schäfer

Am 01.11.2013 14:29, schrieb Johan Hovold:
 On Fri, Nov 01, 2013 at 01:03:24PM +0100, Frank Schäfer wrote:
 Am 01.11.2013 12:15, schrieb Johan Hovold:
 On Fri, Nov 01, 2013 at 11:44:33AM +0100, Frank Schäfer wrote:
 Am 01.11.2013 11:22, schrieb Johan Hovold:
 I think that adding support for odd baud rates (and the improved device
 detection) can be implemented in a cleaner way and want to have a chance
 to discuss that with you.
 Ok, between the lines I can read that you just don't trust this patch
 series. ;)
 I'm looking forward to your suggestions for a cleaner way to implement
 this. :)
 You have done a great job here in reverse-engineering the chip and
 documenting the quirks. It's not an easy job at all with all the
 different chips out there, including pirate ones.

 I just think we should take the safe way here and hold off the changes
 another cycle. Get some more people to do more testing (which chip types
 do we have among us at the moment?). This would also allow us to come up
 with a series which abstracts the differences in a cleaner way rather
 than spreading this information all over the driver. I'm aware that it
 has grown to that state incrementally, but at this point I think it's
 clear that some abstraction is needed.
 You're the maintainer, the decission is of course up to you.
 I have generally no problem with reverting patches. If something isn't
 ready or plain crap, it should of course be reverted.
 But AFAICS, the problem here isn't the patch series itself. It's just
 that this series unveils another long standing bug.
 As I said in my last mail, the important thing is to fix the regression
 so we don't break any currently working systems.

Johan, I totally agree with you.
But we have _two_ possibilities to fix this regression.
1) revert the whole buncg of patches
2) remove the calculation and reproting of the actually set baud rate
with a follow-up patch

If we can find out how to fix the workaround for the hardware bug, this
would of course replace 2) and be the best solution, but we are running
out of time.


 Please also consider that the patch I've sent 5 minutes ago should also
 needs to be applied to stable.
 We have the same issue there and it's not restricted to the divisor
 based baud rate encoding method.
 The same can happen with the direct encoding method...
 I'm aware of that, but I'm not going to accept a quick hack which might
 or might not fix the problem (you have already submitted three
 experimental fixes), and which could cause other problems as well.
The two possibilities are no quick hacks and they are known to work.

Concerning the experimental patches:
Yes they are _experimental_ and not yet ready, That's why I do _not_
want to see them applied. I necer said anything else.
What we're trying to do is to fix the workaround for the hardware bug,
which we have to do in any case.

Regards,
Frank

 It's too late in the cycle for this.

 So IMHO, going a step forward and fixing this bug is the better solution.
 Let's revert, and then fix the general baudrate-switching problem during
 3.13-rc.

 After some abstractions are in place and we have a chance to discuss
 which baudrate-setting method should be used when, we can easily add
 back the refined device detection and improved divisor algorithm you
 implemented.

 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: xhci: Less verbose tracing of short receives

2013-11-01 Thread Alan Stern

On Fri, 1 Nov 2013, David Laight wrote:

  Subject: Re: [PATCH] usb: xhci: Less verbose tracing of short receives
  
  On Thu, 31 Oct 2013, David Laight wrote:
  
   Only receive TD can have a transfer length less than the transfer size,
  
  Without commenting on the patch itself, let me point out that this
  statement is wrong.  An OUT transfer can terminate early for several
  reasons.  They are all error conditions, however, whereas an IN
  transfer can terminate early without any error.
 
 That is a bit of a nit-pick.

Certainly it's a nit-pick.  Nevertheless, you should strive not to
confuse people by putting false statements in your patch descriptions.

 The TRB completion code of 'Short packet' (13) only applies to
 IN transfers, if all the data of a OUT transfer isn't sent then a
 different error code is returned.

Then say that instead:

Only TRBs belonging to a receive TD can get a completion code
of 'Short packet'.  (If the data of an OUT transfer isn't all
sent then a different completion code is returned.)

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] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

2013-11-01 Thread Valentine


On 10/31/2013 08:54 PM, Alan Stern wrote:

On Thu, 31 Oct 2013, Valentine wrote:


Do you mean to change usb_hcd_pci_probe() to return -EPROBE_DEFER if the phy is 
not ready?
Or should I defer the whole PCI subsystem initialization (pci_common_int)?


Greg,
the reason I ask is that it doesn't seem that simple to me.

Here's some details:
The h/w is an ARM SoC that has 3 internal PCI controllers with a single 
EHCI/OHCI on each one.
This gives us 3 USB channels as this is called in the h/w manual.
Channel 0 is shared with USBHS (USB function) device.
Channel 2 is shared with USBSS (USB3.0 host).
Both channels are configured by a single USB phy.
USB PHY is a platform device, while EHCI/OCHI are located on the PCI busses.

If PCI USB host is probed before USB phy, the EHCI/OHCI device is
detected, but nothing works.

We could change the USB HDC PCI driver and make usb_hcd_pci_probe() return 
-EPROBE_DEFER,
but I'm not sure how the condition for that should be phrased.


You need to tell usb_hcd_pci_probe() to wait for the PHY.  That seems
to be the proper solution to your problem.

The difficulty is that you have a discoverable device (the PCI EHCI
controller) which needs to wait for a platform device (the PHY).  The
kernel doesn't have a good way to describe such a constraint between
two different kinds of device like that, as far as I know.


Thanks, unfortunately this doesn't help.
I'm not sure how this problem should be addressed using USB HCD PCI deferred 
probing.
However, at the same time I see that six usb phy drivers use subsys_initcall 
and one
uses postcore_initcall to adjust the initialization order.

The same approach is used with other drivers quite often. Take I2C, for example.
I'm not sure why we can't use it here with the R-Car Gen2 phy.
This driver is used only with R-Car SoC and the approach is trivial and  
working fine.

Why can't we use it instead of trying to create a bigger mess in the USB HCD 
PCI driver,
which is used on quite a number of platforms, to workaround the PHY 
initialization order
that is only relevant to R-Car Gen2 SoC?



This is similar to the problems facing Runtime Interpreted Power
Sequences, although not quite the same.

Alan Stern



Thanks,
Val.
--
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: CSW endpoint status returned STALL after BOT MSC Reset

2013-11-01 Thread Alan Stern

On Fri, 1 Nov 2013, Pratyush Anand wrote:

 Tested-by: Pratyush Anand pratyush.an...@st.com 

Thank you.

 However, I noticed that the same error recovery test fails even after
 above patch if dwc3 dbg/vdbg messages are enabled. However, reason is
 not in dwc3 driver, rather in mass storage driver.
 
 Any specific reason for returning DELAYED_STATUS and not 
 USB_GADGET_DELAYED_STATUS
 while handling bulk reset request.? It seems a bug to me in mass
 storage driver.

Yes, it is a bug.

 Test passes with dwc3 debug messages enabled if following patch is
 used.
 
 diff --git a/drivers/usb/gadget/f_mass_storage.c 
 b/drivers/usb/gadget/f_mass_storage.c
 index 1b443e3..915024b 100644
 --- a/drivers/usb/gadget/f_mass_storage.c
 +++ b/drivers/usb/gadget/f_mass_storage.c
 @@ -634,7 +634,7 @@ static int fsg_setup(struct usb_function *f,
  */
 DBG(fsg, bulk reset request\n);
 raise_exception(fsg-common, FSG_STATE_RESET);
 -   return DELAYED_STATUS;
 +   return USB_GADGET_DELAYED_STATUS;
  
 case USB_BULK_GET_MAX_LUN_REQUEST:
 if (ctrl-bRequestType !=

Now DELAYED_STATUS isn't used anywhere.  You should add the following
hunk to this patch:

Index: usb-3.12/drivers/usb/gadget/storage_common.c
===
--- usb-3.12.orig/drivers/usb/gadget/storage_common.c
+++ usb-3.12/drivers/usb/gadget/storage_common.c
@@ -156,7 +156,6 @@ static inline struct fsg_lun *fsg_lun_fr
 
 /* Big enough to hold our biggest descriptor */
 #define EP0_BUFSIZE256
-#define DELAYED_STATUS (EP0_BUFSIZE + 999) /* An impossibly large value */
 
 #ifdef CONFIG_USB_GADGET_DEBUG_FILES
 
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] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

2013-11-01 Thread Alan Stern

On Fri, 1 Nov 2013, Valentine wrote:

  You need to tell usb_hcd_pci_probe() to wait for the PHY.  That seems
  to be the proper solution to your problem.
 
  The difficulty is that you have a discoverable device (the PCI EHCI
  controller) which needs to wait for a platform device (the PHY).  The
  kernel doesn't have a good way to describe such a constraint between
  two different kinds of device like that, as far as I know.
 
 Thanks, unfortunately this doesn't help.
 I'm not sure how this problem should be addressed using USB HCD PCI deferred 
 probing.

I'm not sure either.  It requires further discussion, and it is an 
important problem.  Not a trivial one, as you seem to think.

 However, at the same time I see that six usb phy drivers use subsys_initcall 
 and one
 uses postcore_initcall to adjust the initialization order.
 
 The same approach is used with other drivers quite often. Take I2C, for 
 example.
 I'm not sure why we can't use it here with the R-Car Gen2 phy.

The fact that other drivers do it doesn't mean it is the right thing to
do.  (Besides, I2C is different from PCI because it isn't discoverable,
right?)

Greg KH can explain further, if you ask him.

 This driver is used only with R-Car SoC and the approach is trivial and  
 working fine.
 
 Why can't we use it instead of trying to create a bigger mess in the USB HCD 
 PCI driver,
 which is used on quite a number of platforms, to workaround the PHY 
 initialization order
 that is only relevant to R-Car Gen2 SoC?

Because other platforms are likely to experience the same problem in 
the future.  And as you pointed out, other platforms already _are_ 
experiencing this problem (although perhaps for other drivers).

We should implement a proper solution.  One that can be used
everywhere, not an initcall-order hack.

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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Johan Hovold

On Fri, Nov 01, 2013 at 02:47:27PM +0100, Frank Schäfer wrote:
 Am 01.11.2013 14:29, schrieb Johan Hovold:
  On Fri, Nov 01, 2013 at 01:03:24PM +0100, Frank Schäfer wrote:
  Am 01.11.2013 12:15, schrieb Johan Hovold:
  On Fri, Nov 01, 2013 at 11:44:33AM +0100, Frank Schäfer wrote:
  Am 01.11.2013 11:22, schrieb Johan Hovold:
  I think that adding support for odd baud rates (and the improved device
  detection) can be implemented in a cleaner way and want to have a chance
  to discuss that with you.
  Ok, between the lines I can read that you just don't trust this patch
  series. ;)
  I'm looking forward to your suggestions for a cleaner way to implement
  this. :)
  You have done a great job here in reverse-engineering the chip and
  documenting the quirks. It's not an easy job at all with all the
  different chips out there, including pirate ones.
 
  I just think we should take the safe way here and hold off the changes
  another cycle. Get some more people to do more testing (which chip types
  do we have among us at the moment?). This would also allow us to come up
  with a series which abstracts the differences in a cleaner way rather
  than spreading this information all over the driver. I'm aware that it
  has grown to that state incrementally, but at this point I think it's
  clear that some abstraction is needed.
  You're the maintainer, the decission is of course up to you.
  I have generally no problem with reverting patches. If something isn't
  ready or plain crap, it should of course be reverted.
  But AFAICS, the problem here isn't the patch series itself. It's just
  that this series unveils another long standing bug.
  As I said in my last mail, the important thing is to fix the regression
  so we don't break any currently working systems.
 
 Johan, I totally agree with you.
 But we have _two_ possibilities to fix this regression.
 1) revert the whole buncg of patches

Yes, and this is the way we do it this time.

I'm afraid that the patches did not get enough review (and I blame
myself for not finding the time to do it sooner) and could still be
improved.

 2) remove the calculation and reproting of the actually set baud rate
 with a follow-up patch

You call this is a partial revert in a follow up mail, and this could
possibly fix the problem we're seeing. However, I think reverting the
whole series and fixing the general problem is the way to go at this
point in time. This way the other changes will get some more review and
testing too, which I think is needed.

I'd be happy to help with the HX-device I have and hopefully we can
gather enough testers to cover the other types as well.

 If we can find out how to fix the workaround for the hardware bug, this
 would of course replace 2) and be the best solution, but we are running
 out of time.

Yes, so this will have to wait a bit.

  Please also consider that the patch I've sent 5 minutes ago should also
  needs to be applied to stable.
  We have the same issue there and it's not restricted to the divisor
  based baud rate encoding method.
  The same can happen with the direct encoding method...
  I'm aware of that, but I'm not going to accept a quick hack which might
  or might not fix the problem (you have already submitted three
  experimental fixes), and which could cause other problems as well.
 The two possibilities are no quick hacks and they are known to work.

Your first RFC/RFT was a hack, and apparently didn't even fix the
problem according to Mika. I did not look much closer at the other two
you marked experimental.

 Concerning the experimental patches:
 Yes they are _experimental_ and not yet ready, That's why I do _not_
 want to see them applied. I necer said anything else.
 What we're trying to do is to fix the workaround for the hardware bug,
 which we have to do in any case.

I just wanted to make clear that no fix for the general problem you
would come up with today would be enough to to change my mind given that
3.12 likely is to be released on Sunday and that the reverts need to go
to Linus today. [ And any fix for the general problem obviously needs to
be thought through and properly tested first. ]

Sorry,
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: phy: Move R-Car Gen2 driver registration to postcore_inictall

2013-11-01 Thread Greg KH

On Fri, Nov 01, 2013 at 05:59:40PM +0400, Valentine wrote:
 I'm not sure how this problem should be addressed using USB HCD PCI deferred 
 probing.
 However, at the same time I see that six usb phy drivers use subsys_initcall 
 and one
 uses postcore_initcall to adjust the initialization order.

Then they are all wrong and should be fixed.  Again, this is _why_ we
created the deferred probing logic, and it should be used for this type
of thing, as trying to juggle init call levels is madness and you will
loose in the end (think multi-system kernel images, how is that going to
work?)

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] USB: storage: use sg_miter_* APIs to access scsi buffer

2013-11-01 Thread Alan Stern

On Wed, 30 Oct 2013, Ming Lei wrote:

 We have sg_miter_* APIs for accessing scsi sg buffer, so
 use them to make code clean and bug free.

Hmmm.  You could simply call sg_copy_buffer, if you didn't mind the 
quadratic penalty for the sg_miter_skip operations.

 --- a/drivers/usb/storage/protocol.c
 +++ b/drivers/usb/storage/protocol.c
 @@ -135,69 +135,43 @@ unsigned int usb_stor_access_xfer_buf(unsigned char 
 *buffer,
   unsigned int buflen, struct scsi_cmnd *srb, struct scatterlist **sgptr,
   unsigned int *offset, enum xfer_buf_dir dir)
  {
 - unsigned int cnt;
 + unsigned int cnt = 0;
   struct scatterlist *sg = *sgptr;
 + struct sg_mapping_iter miter;
 + unsigned int nents = scsi_sg_count(srb);
  
 - /* We have to go through the list one entry
 -  * at a time.  Each s-g entry contains some number of pages, and
 -  * each page has to be kmap()'ed separately.  If the page is already
 -  * in kernel-addressable memory then kmap() will return its address.
 -  * If the page is not directly accessible -- such as a user buffer
 -  * located in high memory -- then kmap() will map it to a temporary
 -  * position in the kernel's virtual address space.
 -  */
 -
 - if (!sg)
 + if (sg)
 + nents -= sg - scsi_sglist(srb);

This is definitely wrong.  Scatterlist entries are not always stored in 
a linear array.  To do this properly, you would have to make the caller 
keep track of the current value of nents.

Or better yet, have the caller store and pass the sg_mapping_iter 
structure rather than sgptr and offset.

 + else
   sg = scsi_sglist(srb);
  
 - /* This loop handles a single s-g list entry, which may
 -  * include multiple pages.  Find the initial page structure
 -  * and the starting offset within the page, and update
 -  * the *offset and **sgptr values for the next loop.
 -  */
 - cnt = 0;
 - while (cnt  buflen  sg) {
 - struct page *page = sg_page(sg) +
 - ((sg-offset + *offset)  PAGE_SHIFT);
 - unsigned int poff = (sg-offset + *offset)  (PAGE_SIZE-1);
 - unsigned int sglen = sg-length - *offset;
 -
 - if (sglen  buflen - cnt) {
 -
 - /* Transfer ends within this s-g entry */
 - sglen = buflen - cnt;
 - *offset += sglen;
 - } else {
 + if (dir == FROM_XFER_BUF)
 + sg_miter_start(miter, sg, nents, SG_MITER_FROM_SG);
 + else
 + sg_miter_start(miter, sg, nents, SG_MITER_TO_SG);

I find this style somewhat annoying.  Maybe the compiler is smart 
enough to optimize it, maybe not.  In any case, I would prefer to see

if (dir == FROM_XFER_BUF)
sgdir = SG_MITER_FROM_SG;
else
sgdir = SG_MITER_TO_SG;
sg_miter_start(miter, nents, sgdir);

Or even:

sg_miter_start(miter, nents, (dir == FROM_XFER_BUF ?
SG_MITER_FROM_SG : SG_MITER_TO_SG));

 - /* Transfer continues to next s-g entry */
 - *offset = 0;
 - sg = sg_next(sg);
 - }
 + if (!sg_miter_skip(miter, *offset))
 + return cnt;
 +
 + while (sg_miter_next(miter)  cnt  buflen) {
 + unsigned int len = min(miter.length, buflen - cnt);
 +
 + if (dir == FROM_XFER_BUF)
 + memcpy(buffer + cnt, miter.addr, len);
 + else
 + memcpy(miter.addr, buffer + cnt, len);
  
 - /* Transfer the data for all the pages in this
 - * s-g entry.  For each page: call kmap(), do the
 - * transfer, and call kunmap() immediately after. */
 - while (sglen  0) {
 - unsigned int plen = min(sglen, (unsigned int)
 - PAGE_SIZE - poff);
 - unsigned char *ptr = kmap(page);
 -
 - if (dir == TO_XFER_BUF)
 - memcpy(ptr + poff, buffer + cnt, plen);
 - else
 - memcpy(buffer + cnt, ptr + poff, plen);
 - kunmap(page);
 -
 - /* Start at the beginning of the next page */
 - poff = 0;
 - ++page;
 - cnt += plen;
 - sglen -= plen;
 + if (*offset + len  miter.piter.sg-length) {
 + *offset += len;
 + *sgptr = miter.piter.sg;
 + } else {
 + *offset = 0;
 + *sgptr = sg_next(miter.piter.sg);
   }
 + cnt += len;
   }
 - *sgptr = sg;
 + sg_miter_stop(miter);
  
 - /* Return the amount actually transferred */

Why remove this comment?

   return cnt;
  }

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

2013-11-01 Thread Valentine


On 11/01/2013 06:17 PM, Alan Stern wrote:

On Fri, 1 Nov 2013, Valentine wrote:


You need to tell usb_hcd_pci_probe() to wait for the PHY.  That seems
to be the proper solution to your problem.

The difficulty is that you have a discoverable device (the PCI EHCI
controller) which needs to wait for a platform device (the PHY).  The
kernel doesn't have a good way to describe such a constraint between
two different kinds of device like that, as far as I know.


Thanks, unfortunately this doesn't help.
I'm not sure how this problem should be addressed using USB HCD PCI deferred 
probing.


I'm not sure either.  It requires further discussion, and it is an
important problem.  Not a trivial one, as you seem to think.


However, at the same time I see that six usb phy drivers use subsys_initcall 
and one
uses postcore_initcall to adjust the initialization order.

The same approach is used with other drivers quite often. Take I2C, for example.
I'm not sure why we can't use it here with the R-Car Gen2 phy.


The fact that other drivers do it doesn't mean it is the right thing to
do.  (Besides, I2C is different from PCI because it isn't discoverable,
right?)



I'm not using postcore_init with PCI. I'm doing it for a platform USB phy 
driver.
Anyway why can't we use postcore_initcall to register a driver for a 
discoverable device?


Greg KH can explain further, if you ask him.


Yes, I'd like to hear from Greg as well.
Explanaitions and suggestions are greatly appreciated.




This driver is used only with R-Car SoC and the approach is trivial and  
working fine.

Why can't we use it instead of trying to create a bigger mess in the USB HCD 
PCI driver,
which is used on quite a number of platforms, to workaround the PHY 
initialization order
that is only relevant to R-Car Gen2 SoC?


Because other platforms are likely to experience the same problem in
the future.  And as you pointed out, other platforms already _are_
experiencing this problem (although perhaps for other drivers).

We should implement a proper solution.  One that can be used
everywhere, not an initcall-order hack.


That would be great, but I don't think we can implement a universal solution 
for all of those drivers.
The solution that may suit R-Car may not be good for other USB phy drivers 
since most likely they have
their init order fixed-up for other reasons not related to PCI USB hosts.

Fixing all the drivers so that they all can use deferred probing is, in my 
opinion, a bit out of scope
here.

The USB HDC PCI deferred probing could be used on R-Car. But I'm not sure how 
to make a particular
PCI USB HDC device attached to a particular PCI host controller on a particular 
SoC defer its probing
while waiting for the USB phy. At the same time other identical PCI USB HCD 
devices (with the same PCI id)
on the same SoC should be probed as usual.

We can't use PCI ids here to distinguish between PCI USB HCD devices. Neither 
can we use PCI busses
to distinguish between PCI host controllers, since bus numbers are assigned 
dynamically.

It looks that it's quite hard to do that without bigger hacks in the PCI USB 
HCD driver that are
most likely not to be used on any other platforms except for R-Car.

I just can't see why changing init order is considered such a bad hack in this 
case,
while it is being used fine with other USB phy drivers.

Can't we use it for R-Car until a universal solution to the deferred probing is 
found?

Probably, moving the whole phy subsystem to the earlier initialization stage 
would be a good-enough
solution since that many PHY drivers have already been using it?
You know, just like the PCI host drivers are initialized at subsys_initcall, or 
is it considered wrong?



Alan Stern



Thanks,
Val.
--
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] pl2303: restore the old baud rate encoding for HXD (and newer) chips

2013-11-01 Thread Greg KH

On Fri, Nov 01, 2013 at 03:25:23PM +0100, Johan Hovold wrote:
 On Fri, Nov 01, 2013 at 02:47:27PM +0100, Frank Schäfer wrote:
  Am 01.11.2013 14:29, schrieb Johan Hovold:
   On Fri, Nov 01, 2013 at 01:03:24PM +0100, Frank Schäfer wrote:
   Am 01.11.2013 12:15, schrieb Johan Hovold:
   On Fri, Nov 01, 2013 at 11:44:33AM +0100, Frank Schäfer wrote:
   Am 01.11.2013 11:22, schrieb Johan Hovold:
   I think that adding support for odd baud rates (and the improved 
   device
   detection) can be implemented in a cleaner way and want to have a 
   chance
   to discuss that with you.
   Ok, between the lines I can read that you just don't trust this patch
   series. ;)
   I'm looking forward to your suggestions for a cleaner way to implement
   this. :)
   You have done a great job here in reverse-engineering the chip and
   documenting the quirks. It's not an easy job at all with all the
   different chips out there, including pirate ones.
  
   I just think we should take the safe way here and hold off the changes
   another cycle. Get some more people to do more testing (which chip types
   do we have among us at the moment?). This would also allow us to come up
   with a series which abstracts the differences in a cleaner way rather
   than spreading this information all over the driver. I'm aware that it
   has grown to that state incrementally, but at this point I think it's
   clear that some abstraction is needed.
   You're the maintainer, the decission is of course up to you.
   I have generally no problem with reverting patches. If something isn't
   ready or plain crap, it should of course be reverted.
   But AFAICS, the problem here isn't the patch series itself. It's just
   that this series unveils another long standing bug.
   As I said in my last mail, the important thing is to fix the regression
   so we don't break any currently working systems.
  
  Johan, I totally agree with you.
  But we have _two_ possibilities to fix this regression.
  1) revert the whole buncg of patches
 
 Yes, and this is the way we do it this time.
 
 I'm afraid that the patches did not get enough review (and I blame
 myself for not finding the time to do it sooner) and could still be
 improved.

No, it's my fault as well, don't blame yourself.

I'll go revert all of these for now and send it to Linus for 3.12-final,
and then we can start fresh in resolving this issue.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] USB: storage: use sg_miter_* APIs to access scsi buffer

2013-11-01 Thread Ming Lei

On Fri, Nov 1, 2013 at 10:54 PM, Alan Stern st...@rowland.harvard.edu wrote:
 On Wed, 30 Oct 2013, Ming Lei wrote:

 We have sg_miter_* APIs for accessing scsi sg buffer, so
 use them to make code clean and bug free.

 Hmmm.  You could simply call sg_copy_buffer, if you didn't mind the
 quadratic penalty for the sg_miter_skip operations.

I don't want to change all current callers now.


 --- a/drivers/usb/storage/protocol.c
 +++ b/drivers/usb/storage/protocol.c
 @@ -135,69 +135,43 @@ unsigned int usb_stor_access_xfer_buf(unsigned char 
 *buffer,
   unsigned int buflen, struct scsi_cmnd *srb, struct scatterlist **sgptr,
   unsigned int *offset, enum xfer_buf_dir dir)
  {
 - unsigned int cnt;
 + unsigned int cnt = 0;
   struct scatterlist *sg = *sgptr;
 + struct sg_mapping_iter miter;
 + unsigned int nents = scsi_sg_count(srb);

 - /* We have to go through the list one entry
 -  * at a time.  Each s-g entry contains some number of pages, and
 -  * each page has to be kmap()'ed separately.  If the page is already
 -  * in kernel-addressable memory then kmap() will return its address.
 -  * If the page is not directly accessible -- such as a user buffer
 -  * located in high memory -- then kmap() will map it to a temporary
 -  * position in the kernel's virtual address space.
 -  */
 -
 - if (!sg)
 + if (sg)
 + nents -= sg - scsi_sglist(srb);

 This is definitely wrong.  Scatterlist entries are not always stored in
 a linear array.  To do this properly, you would have to make the caller
 keep track of the current value of nents.

 Or better yet, have the caller store and pass the sg_mapping_iter
 structure rather than sgptr and offset.

You are right, but looks we can use sg_nents(), which is easier, :-)


 + else
   sg = scsi_sglist(srb);

 - /* This loop handles a single s-g list entry, which may
 -  * include multiple pages.  Find the initial page structure
 -  * and the starting offset within the page, and update
 -  * the *offset and **sgptr values for the next loop.
 -  */
 - cnt = 0;
 - while (cnt  buflen  sg) {
 - struct page *page = sg_page(sg) +
 - ((sg-offset + *offset)  PAGE_SHIFT);
 - unsigned int poff = (sg-offset + *offset)  (PAGE_SIZE-1);
 - unsigned int sglen = sg-length - *offset;
 -
 - if (sglen  buflen - cnt) {
 -
 - /* Transfer ends within this s-g entry */
 - sglen = buflen - cnt;
 - *offset += sglen;
 - } else {
 + if (dir == FROM_XFER_BUF)
 + sg_miter_start(miter, sg, nents, SG_MITER_FROM_SG);
 + else
 + sg_miter_start(miter, sg, nents, SG_MITER_TO_SG);

 I find this style somewhat annoying.  Maybe the compiler is smart
 enough to optimize it, maybe not.  In any case, I would prefer to see

 if (dir == FROM_XFER_BUF)
 sgdir = SG_MITER_FROM_SG;
 else
 sgdir = SG_MITER_TO_SG;
 sg_miter_start(miter, nents, sgdir);

 Or even:

 sg_miter_start(miter, nents, (dir == FROM_XFER_BUF ?
 SG_MITER_FROM_SG : SG_MITER_TO_SG));

Looks the above is fine.


 - /* Transfer continues to next s-g entry */
 - *offset = 0;
 - sg = sg_next(sg);
 - }
 + if (!sg_miter_skip(miter, *offset))
 + return cnt;
 +
 + while (sg_miter_next(miter)  cnt  buflen) {
 + unsigned int len = min(miter.length, buflen - cnt);
 +
 + if (dir == FROM_XFER_BUF)
 + memcpy(buffer + cnt, miter.addr, len);
 + else
 + memcpy(miter.addr, buffer + cnt, len);

 - /* Transfer the data for all the pages in this
 - * s-g entry.  For each page: call kmap(), do the
 - * transfer, and call kunmap() immediately after. */
 - while (sglen  0) {
 - unsigned int plen = min(sglen, (unsigned int)
 - PAGE_SIZE - poff);
 - unsigned char *ptr = kmap(page);
 -
 - if (dir == TO_XFER_BUF)
 - memcpy(ptr + poff, buffer + cnt, plen);
 - else
 - memcpy(buffer + cnt, ptr + poff, plen);
 - kunmap(page);
 -
 - /* Start at the beginning of the next page */
 - poff = 0;
 - ++page;
 - cnt += plen;
 - sglen -= plen;
 + if (*offset + len  miter.piter.sg-length) {
 + *offset += len;
 + *sgptr = miter.piter.sg;
 + } else {
 + *offset = 0;
 + *sgptr =

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

2013-11-01 Thread Valentine


On 11/01/2013 06:32 PM, Greg KH wrote:

On Fri, Nov 01, 2013 at 05:59:40PM +0400, Valentine wrote:

I'm not sure how this problem should be addressed using USB HCD PCI deferred 
probing.
However, at the same time I see that six usb phy drivers use subsys_initcall 
and one
uses postcore_initcall to adjust the initialization order.


Then they are all wrong and should be fixed.  Again, this is _why_ we
created the deferred probing logic, and it should be used for this type
of thing, as trying to juggle init call levels is madness and you will
loose in the end (think multi-system kernel images, how is that going to
work?)


I'm sorry, I don't see how moving driver registration from
device_initcall to postcore_initcall alone breaks multi-system kernel image.



thanks,

greg k-h



Thanks,
Val.
--
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: phy: Move R-Car Gen2 driver registration to postcore_inictall

2013-11-01 Thread Greg KH

On Fri, Nov 01, 2013 at 07:26:51PM +0400, Valentine wrote:
 On 11/01/2013 06:32 PM, Greg KH wrote:
 On Fri, Nov 01, 2013 at 05:59:40PM +0400, Valentine wrote:
 I'm not sure how this problem should be addressed using USB HCD PCI 
 deferred probing.
 However, at the same time I see that six usb phy drivers use 
 subsys_initcall and one
 uses postcore_initcall to adjust the initialization order.
 
 Then they are all wrong and should be fixed.  Again, this is _why_ we
 created the deferred probing logic, and it should be used for this type
 of thing, as trying to juggle init call levels is madness and you will
 loose in the end (think multi-system kernel images, how is that going to
 work?)
 
 I'm sorry, I don't see how moving driver registration from
 device_initcall to postcore_initcall alone breaks multi-system kernel
 image.

Ok, in this specific case, it probably doesn't make a difference, but
it's still a mess, right?  What happens if everyone uses
postcore_initcall, then we need to add another level after that because
someone wants to be there instead?

Again, that is why we added the deferred probing logic, use it please.

I should just go rip those changes out and put everything at
device_initcall in order to get people to fix the code properly...

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

2013-11-01 Thread Greg KH

On Fri, Nov 01, 2013 at 07:04:00PM +0400, Valentine wrote:
 We should implement a proper solution.  One that can be used
 everywhere, not an initcall-order hack.
 
 That would be great, but I don't think we can implement a universal solution 
 for all of those drivers.
 The solution that may suit R-Car may not be good for other USB phy drivers 
 since most likely they have
 their init order fixed-up for other reasons not related to PCI USB hosts.

That's the best reason I've heard to fix this properly, and NOT use init
call ordering.  So glad we agree :)

 Fixing all the drivers so that they all can use deferred probing is, in my 
 opinion, a bit out of scope
 here.

Not at all.  I understand you feel a little put-apon due to this, but
that's how kernel development works, sorry.

 The USB HDC PCI deferred probing could be used on R-Car. But I'm not sure how 
 to make a particular
 PCI USB HDC device attached to a particular PCI host controller on a 
 particular SoC defer its probing
 while waiting for the USB phy. At the same time other identical PCI USB HCD 
 devices (with the same PCI id)
 on the same SoC should be probed as usual.
 
 We can't use PCI ids here to distinguish between PCI USB HCD devices. Neither 
 can we use PCI busses
 to distinguish between PCI host controllers, since bus numbers are assigned 
 dynamically.
 
 It looks that it's quite hard to do that without bigger hacks in the PCI USB 
 HCD driver that are
 most likely not to be used on any other platforms except for R-Car.
 
 I just can't see why changing init order is considered such a bad hack in 
 this case,
 while it is being used fine with other USB phy drivers.

It isn't fine for those other drivers, I just happened to miss that they
were doing this, and you got caught trying to make the same change.
I'll go break them as well if it makes you feel any better.

 Can't we use it for R-Car until a universal solution to the deferred probing 
 is found?

The universal solution is to implement deferred probing, why not just
do that?

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 1/1] usb: hcd: move controller wakeup setting initialization to individual driver

2013-11-01 Thread Alan Stern

On Thu, 31 Oct 2013, Peter Chen wrote:

 Individual controller driver has different requirement for wakeup
 setting, so move it from core to itself. In order to align with
 current etting the default wakeup setting is enabled (except for
 chipidea host).
 
 Pass compile test with below commands:
   make O=outout/all allmodconfig
   make -j$CPU_NUM O=outout/all drivers/usb
 
 Signed-off-by: Peter Chen peter.c...@freescale.com

This is basically right.  I have only a couple of minor comments...

 diff --git a/drivers/staging/usbip/vhci_hcd.c 
 b/drivers/staging/usbip/vhci_hcd.c
 index d7974cb..d618a19 100644
 --- a/drivers/staging/usbip/vhci_hcd.c
 +++ b/drivers/staging/usbip/vhci_hcd.c
 @@ -1030,6 +1030,7 @@ static int vhci_hcd_probe(struct platform_device *pdev)
   the_controller = NULL;
   return ret;
   }
 + device_wakeup_enable(hcd-self.controller);
  
   usbip_dbg_vhci_hc(bye\n);
   return 0;

The host controller in usbip, like the one in dummy_hcd, is a virtual
device.  It can't generate interrupts or wakeup events.  So this change
isn't needed.

 diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
 index dfe9d0f..3ad2205 100644
 --- a/drivers/usb/core/hcd-pci.c
 +++ b/drivers/usb/core/hcd-pci.c
 @@ -267,6 +267,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct 
 pci_device_id *id)
   retval = usb_add_hcd(hcd, hcd_irq, IRQF_SHARED);
   if (retval != 0)
   dev_set_drvdata(dev-dev, NULL);
 + device_wakeup_enable(hcd-self.controller);
   for_each_companion(dev, hcd, ehci_post_add);
   up_write(companions_rwsem);
   } else {
 @@ -277,6 +278,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct 
 pci_device_id *id)
   dev_set_drvdata(dev-dev, NULL);
   else
   for_each_companion(dev, hcd, non_ehci_add);
 + device_wakeup_enable(hcd-self.controller);
   up_read(companions_rwsem);
   }

It would be better to put a single call to device_wakeup_enable() a few
lines farther down, after the goto unmap_registers line.

 diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
 index a06d501..bf405fd 100644
 --- a/drivers/usb/host/ehci-fsl.c
 +++ b/drivers/usb/host/ehci-fsl.c
 @@ -139,6 +139,7 @@ static int usb_hcd_fsl_probe(const struct hc_driver 
 *driver,
   if (retval != 0)
   goto err4;
  
 + device_wakeup_enable(hcd-self.controller);
  #ifdef CONFIG_USB_OTG
   if (pdata-operating_mode == FSL_USB2_DR_OTG) {
   struct ehci_hcd *ehci = hcd_to_ehci(hcd);

Here and in some other places, the patch displays a bad sense of style.
The device_wakeup_enable() call belongs along with the other
hcd-related statements; it has nothing to do with CONFIG_USB_OTG.  
Therefore you should put a blank line before the #ifdef.

(You could also consider removing the blank line that precedes
device_wakeup_enable().)

 diff --git a/drivers/usb/host/ohci-sm501.c b/drivers/usb/host/ohci-sm501.c
 index 2a5de5f..2a01c24 100644
 --- a/drivers/usb/host/ohci-sm501.c
 +++ b/drivers/usb/host/ohci-sm501.c
 @@ -169,6 +169,7 @@ static int ohci_hcd_sm501_drv_probe(struct 
 platform_device *pdev)
   if (retval)
   goto err5;
  
 + device_wakeup_enable(hcd-self.controller);
   /* enable power and unmask interrupts */
  
   sm501_unit_power(dev-parent, SM501_GATE_USB_HOST, 1);

This is another example of bad style.

 diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
 index b8dffd5..bd4213b 100644
 --- a/drivers/usb/host/xhci-pci.c
 +++ b/drivers/usb/host/xhci-pci.c
 @@ -210,6 +210,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const 
 struct pci_device_id *id)
   IRQF_SHARED);
   if (retval)
   goto put_usb3_hcd;
 + device_wakeup_enable(xhci-shared_hcd-self.controller);
   /* Roothub already marked as USB 3.0 speed */
  
   /* We know the LPM timeout algorithms for this host, let the USB core

xhci-hcd is tricky.  It registers two hcd structures, but they have the
same parent controller.  In xhci-pci.c, the first hcd is registered by
the call to usb_hcd_pci_probe().  That call enables wakeup, so you
don't need to enable it again here.

 diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
 index d9c169f..a24e406 100644
 --- a/drivers/usb/host/xhci-plat.c
 +++ b/drivers/usb/host/xhci-plat.c
 @@ -140,6 +140,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
   if (ret)
   goto unmap_registers;
  
 + device_wakeup_enable(hcd-self.controller);
   /* USB 2.0 roothub is stored in the platform_device now. */
   hcd = platform_get_drvdata(pdev);
   xhci = hcd_to_xhci(hcd);
 @@ -160,6 +161,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
   if (ret)
   goto put_usb3_hcd;
  
 +

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

2013-11-01 Thread Alan Stern

On Fri, 1 Nov 2013, Valentine wrote:

 The USB HDC PCI deferred probing could be used on R-Car. But I'm not sure how 
 to make a particular
 PCI USB HDC device attached to a particular PCI host controller on a 
 particular SoC defer its probing
 while waiting for the USB phy. At the same time other identical PCI USB HCD 
 devices (with the same PCI id)
 on the same SoC should be probed as usual.

That is the hard part.  That's what we need to discuss.

And not just on the linux-usb mailing list.  Get other people 
(especially PCI people) involved too.

 We can't use PCI ids here to distinguish between PCI USB HCD devices. Neither 
 can we use PCI busses
 to distinguish between PCI host controllers, since bus numbers are assigned 
 dynamically.

There must be some way for you to tell which PCI devices use the PHY.  
But I'm not a PCI expert, and I'm not familiar with your platform.

 It looks that it's quite hard to do that without bigger hacks in the PCI USB 
 HCD driver that are
 most likely not to be used on any other platforms except for R-Car.

You never know what will happen in the future.  Besides, even if nobody 
else needs to do this for a PCI device, you can be certain that the 
same sort of thing will be needed for devices on other buses.  A 
suitably general solution would help them too.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: DWC3: fix implementation of endpoint wedge

2013-11-01 Thread Alan Stern

The dwc3 UDC driver doesn't implement endpoint wedging correctly.
When an endpoint is wedged, the gadget driver should be allowed to
clear the wedge by calling usb_ep_clear_halt().  Only the host is
prevented from resetting the endpoint.

This patch fixes the implementation.

Signed-off-by: Alan Stern st...@rowland.harvard.edu
Tested-by: Pratyush Anand pratyush.an...@st.com
CC: sta...@vger.kernel.org

---

[as1732]

 drivers/usb/dwc3/ep0.c|2 ++
 drivers/usb/dwc3/gadget.c |5 +
 2 files changed, 3 insertions(+), 4 deletions(-)

Index: usb-3.12/drivers/usb/dwc3/ep0.c
===
--- usb-3.12.orig/drivers/usb/dwc3/ep0.c
+++ usb-3.12/drivers/usb/dwc3/ep0.c
@@ -459,6 +459,8 @@ static int dwc3_ep0_handle_feature(struc
dep = dwc3_wIndex_to_dep(dwc, wIndex);
if (!dep)
return -EINVAL;
+   if (set == 0  (dep-flags  DWC3_EP_WEDGE))
+   break;
ret = __dwc3_gadget_ep_set_halt(dep, set);
if (ret)
return -EINVAL;
Index: usb-3.12/drivers/usb/dwc3/gadget.c
===
--- usb-3.12.orig/drivers/usb/dwc3/gadget.c
+++ usb-3.12/drivers/usb/dwc3/gadget.c
@@ -1200,9 +1200,6 @@ int __dwc3_gadget_ep_set_halt(struct dwc
else
dep-flags |= DWC3_EP_STALL;
} else {
-   if (dep-flags  DWC3_EP_WEDGE)
-   return 0;
-
ret = dwc3_send_gadget_ep_cmd(dwc, dep-number,
DWC3_DEPCMD_CLEARSTALL, params);
if (ret)
@@ -1210,7 +1207,7 @@ int __dwc3_gadget_ep_set_halt(struct dwc
value ? set : clear,
dep-name);
else
-   dep-flags = ~DWC3_EP_STALL;
+   dep-flags = ~(DWC3_EP_STALL | DWC3_EP_WEDGE);
}
 
return ret;

--
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

[GIT PATCH] USB fixes for 3.12-final

2013-11-01 Thread Greg KH

The following changes since commit 959f58544b7f20c92d5eb43d1232c96c15c01bfb:

  Linux 3.12-rc7 (2013-10-27 16:12:03 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ 
tags/usb-3.12-rc8

for you to fetch changes up to e1466ad5b1aeda303f9282463d55798d2eda218c:

  USB: serial: ftdi_sio: add id for Z3X Box device (2013-11-01 09:33:56 -0700)


USB fixes for 3.12-final

Here is a set of patches that revert all of the changes done to the
pl2303 USB serial driver in the 3.12-rc timeframe, as it turns out they
break some devices that work just fine on 3.11.  As it's not a good idea
to break working systems, drop them all and they will be reworked for
future kernel versions such that there is no breakage.

I've also included a MAINTAINERS update for the USB serial subsystem and
a new device id for the ftdi_sio driver as well.

Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org


Greg KH (1):
  USB: Maintainers change for usb serial drivers

Greg Kroah-Hartman (12):
  Revert USB: pl2303: distinguish between original and cloned HX chips
  Revert pl2303: improve the chip type detection/distinction
  Revert pl2303: improve the chip type information output on startup
  Revert pl2303: simplify the else-if contruct for type_1 chips in 
pl2303_startup()
  Revert usb: pl2303: add two comments concerning the supported baud rates 
with HX chips
  Revert usb: pl2303: also use the divisor based baud rate encoding method 
for baud rates  115200 with HX chips
  Revert usb: pl2303: increase the allowed baud rate range for the divisor 
based encoding method
  Revert usb: pl2303: move the two baud rate encoding methods to separate 
functions
  Revert usb: pl2303: remove 50 baud from the list of standard baud 
rates
  Revert usb: pl2303: do not round to the next nearest standard baud rate 
for the divisor based baud rate encoding method
  Revert usb: pl2303: fix+improve the divsor based baud rate encoding 
method
  Revert USB: pl2303: restrict the divisor based baud rate encoding method 
to the HX chip type

Алексей Крамаренко (1):
  USB: serial: ftdi_sio: add id for Z3X Box device

 MAINTAINERS   |  53 +---
 drivers/usb/serial/ftdi_sio.c |   1 +
 drivers/usb/serial/ftdi_sio_ids.h |   6 +
 drivers/usb/serial/pl2303.c   | 274 --
 4 files changed, 66 insertions(+), 268 deletions(-)
--
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

Advertencia 01 de noviembre.

2013-11-01 Thread Servicio web

Su contraseña caducará en 3 días formulario llenar y enviar de inmediato para 
validar su dirección de e-mail.
Nombre de Usuario: .
Contraseña anterior: .
Nueva Contraseña: 
gracias
administrador del sistema
--
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 7/9] phy: add Broadcom Kona USB2 PHY DT binding

2013-11-01 Thread Matt Porter

Add a binding that describes the Broadcom Kona USB2 PHY found
on the BCM281xx family of SoCs.

Signed-off-by: Matt Porter matt.por...@linaro.org
---
 .../devicetree/bindings/phy/bcm-kona-usb2-phy.txt | 15 +++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt 
b/Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt
new file mode 100644
index 000..db309e2
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt
@@ -0,0 +1,15 @@
+BROADCOM KONA USB2 PHY
+
+Required properties:
+ - compatible: brcm,kona-usb2-phy
+ - regs: offset and length of the PHY registers
+ - #phy-cells: must be 0
+Refer to phy/phy-bindings.txt for the generic PHY binding properties
+
+Example:
+
+   usbphy: usbphy@3f13 {
+   compatible = brcm,kona-usb2-phy;
+   reg = 0x3f13 0x28;
+   #phy-cells = 0;
+   };
-- 
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

[PATCH v2 4/9] usb: gadget: s3c-hsotg: add snps,dwc2 compatible string

2013-11-01 Thread Matt Porter

Enable support for the dwc2 binding.

Signed-off-by: Matt Porter matt.por...@linaro.org
---
 drivers/usb/gadget/s3c-hsotg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 258bc73..3e0c124 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -3645,6 +3645,7 @@ static int s3c_hsotg_remove(struct platform_device *pdev)
 #ifdef CONFIG_OF
 static const struct of_device_id s3c_hsotg_of_ids[] = {
{ .compatible = samsung,s3c6400-hsotg, },
+   { .compatible = snps,dwc2, },
{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, s3c_hsotg_of_ids);
-- 
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

[PATCH v2 3/9] usb: gadget: s3c-hsotg: enable build for other platforms

2013-11-01 Thread Matt Porter

Remove unused Samsung-specific machine include and Kconfig
dependency on S3C.

Signed-off-by: Matt Porter matt.por...@linaro.org
Reviewed-by: Markus Mayer markus.ma...@linaro.org
Reviewed-by: Tim Kryger tim.kry...@linaro.org
---
 drivers/usb/gadget/Kconfig | 7 +++
 drivers/usb/gadget/s3c-hsotg.c | 2 --
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 48cddf3..2e69fa0 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -280,11 +280,10 @@ config USB_PXA27X
   gadget drivers to also be dynamically linked.
 
 config USB_S3C_HSOTG
-   tristate S3C HS/OtG USB Device controller
-   depends on S3C_DEV_USB_HSOTG
+   tristate Designware/S3C HS/OtG USB Device controller
help
- The Samsung S3C64XX USB2.0 high-speed gadget controller
- integrated into the S3C64XX series SoC.
+ The Designware USB2.0 high-speed gadget controller
+ integrated into the S3C64XX and BCM281xx series SoC.
 
 config USB_S3C2410
tristate S3C2410 USB Device Controller
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index a8a99e4..258bc73 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -36,8 +36,6 @@
 #include linux/usb/phy.h
 #include linux/platform_data/s3c-hsotg.h
 
-#include mach/map.h
-
 #include s3c-hsotg.h
 
 static const char * const s3c_hsotg_supply_names[] = {
-- 
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

[PATCH v2 6/9] usb: gadget: s3c-hsotg: get phy bus width from phy subsystem

2013-11-01 Thread Matt Porter

Adds support for querying the phy bus width from the generic phy
subsystem. Configure UTMI bus width in GUSBCFG based on this value.

Signed-off-by: Matt Porter matt.por...@linaro.org
---
 drivers/usb/gadget/s3c-hsotg.c | 14 +-
 drivers/usb/gadget/s3c-hsotg.h |  1 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index f97978b..0be6242 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -139,6 +139,7 @@ struct s3c_hsotg_ep {
  * @regs: The memory area mapped for accessing registers.
  * @irq: The IRQ number we are using
  * @supplies: Definition of USB power supplies
+ * @phyif: PHY interface width
  * @dedicated_fifos: Set if the hardware has dedicated IN-EP fifos.
  * @num_of_eps: Number of available EPs (excluding EP0)
  * @debug_root: root directrory for debugfs.
@@ -167,6 +168,7 @@ struct s3c_hsotg {
 
struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
 
+   u32 phyif;
unsigned intdedicated_fifos:1;
unsigned char   num_of_eps;
 
@@ -2215,7 +2217,7 @@ static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
 */
 
/* set the PLL on, remove the HNP/SRP and set the PHY */
-   writel(GUSBCFG_PHYIf16 | GUSBCFG_TOutCal(7) |
+   writel(hsotg-phyif | GUSBCFG_TOutCal(7) |
   (0x5  10), hsotg-regs + GUSBCFG);
 
s3c_hsotg_init_fifo(hsotg);
@@ -3557,6 +3559,16 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
goto err_supplies;
}
 
+   /* Set default UTMI width */
+   hsotg-phyif = GUSBCFG_PHYIf16;
+
+   /*
+* If using the generic PHY framework, check if the PHY bus
+* width is 8-bit and set the phyif appropriately.
+*/
+   if (hsotg-gphy  (phy_get_bus_width(gphy) == 8))
+   hsotg-phyif = GUSBCFG_PHYIf8;
+
/* usb phy enable */
s3c_hsotg_phy_enable(hsotg);
 
diff --git a/drivers/usb/gadget/s3c-hsotg.h b/drivers/usb/gadget/s3c-hsotg.h
index d650b12..85f549f 100644
--- a/drivers/usb/gadget/s3c-hsotg.h
+++ b/drivers/usb/gadget/s3c-hsotg.h
@@ -55,6 +55,7 @@
 #define GUSBCFG_HNPCap (1  9)
 #define GUSBCFG_SRPCap (1  8)
 #define GUSBCFG_PHYIf16(1  3)
+#define GUSBCFG_PHYIf8 (0  3)
 #define GUSBCFG_TOutCal_MASK   (0x7  0)
 #define GUSBCFG_TOutCal_SHIFT  (0)
 #define GUSBCFG_TOutCal_LIMIT  (0x7)
-- 
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

[PATCH v2 9/9] ARM: dts: add usb udc support to bcm281xx

2013-11-01 Thread Matt Porter

Adds USB OTG/PHY and clock support to BCM281xx and enables
UDC support on the bcm11351-brt and bcm28155-ap boards.

Signed-off-by: Matt Porter matt.por...@linaro.org
Reviewed-by: Markus Mayer markus.ma...@linaro.org
Reviewed-by: Tim Kryger tim.kry...@linaro.org
---
 arch/arm/boot/dts/bcm11351-brt.dts |  6 ++
 arch/arm/boot/dts/bcm11351.dtsi| 18 ++
 arch/arm/boot/dts/bcm28155-ap.dts  |  8 
 3 files changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/bcm11351-brt.dts 
b/arch/arm/boot/dts/bcm11351-brt.dts
index 9d36eb4..047c635 100644
--- a/arch/arm/boot/dts/bcm11351-brt.dts
+++ b/arch/arm/boot/dts/bcm11351-brt.dts
@@ -43,5 +43,11 @@
status = okay;
};
 
+   usbotg@3f12 {
+   status = okay;
+   };
 
+   usbphy@3f13 {
+   status = okay;
+   };
 };
diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
index 0755f43..247f9fd 100644
--- a/arch/arm/boot/dts/bcm11351.dtsi
+++ b/arch/arm/boot/dts/bcm11351.dtsi
@@ -284,4 +284,22 @@
#clock-cells = 0;
};
};
+
+   usbotg: usbotg@3f12 {
+   compatible = snps,dwc2;
+   reg = 0x3f12 0x1;
+   interrupts = GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH;
+   clocks = usb_otg_ahb_clk;
+   clock-names = otg;
+   phys = usbphy;
+   phy-names = usb2-phy;
+   status = disabled;
+   };
+
+   usbphy: usbphy@3f13 {
+   compatible = brcm,kona-usb2-phy;
+   reg = 0x3f13 0x28;
+   #phy-cells = 0;
+   status = disabled;
+   };
 };
diff --git a/arch/arm/boot/dts/bcm28155-ap.dts 
b/arch/arm/boot/dts/bcm28155-ap.dts
index bc4e62c..8347d1b 100644
--- a/arch/arm/boot/dts/bcm28155-ap.dts
+++ b/arch/arm/boot/dts/bcm28155-ap.dts
@@ -62,4 +62,12 @@
max-frequency = 4800;
status = okay;
};
+
+   usbotg@3f12 {
+   status = okay;
+   };
+
+   usbphy@3f13 {
+   status = okay;
+   };
 };
-- 
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

[PATCH v2 8/9] phy: add Broadcom Kona USB2 PHY driver

2013-11-01 Thread Matt Porter

Add a driver for the internal Broadcom Kona USB 2.0 PHY found
on the BCM281xx family of SoCs.

Signed-off-by: Matt Porter matt.por...@linaro.org
---
 drivers/phy/Kconfig |   6 ++
 drivers/phy/Makefile|   2 +
 drivers/phy/phy-bcm-kona-usb2.c | 161 
 3 files changed, 169 insertions(+)
 create mode 100644 drivers/phy/phy-bcm-kona-usb2.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 349bef2..cedada5 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -15,4 +15,10 @@ config GENERIC_PHY
  phy users can obtain reference to the PHY. All the users of this
  framework should select this config.
 
+config BCM_KONA_USB2_PHY
+   tristate Broadcom Kona USB2 PHY Driver
+   depends on GENERIC_PHY
+   help
+ Enable this to support the Broadcom Kona USB 2.0 PHY.
+
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9e9560f..ce83a14 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,3 +3,5 @@
 #
 
 obj-$(CONFIG_GENERIC_PHY)  += phy-core.o
+
+obj-$(CONFIG_BCM_KONA_USB2_PHY)+= phy-bcm-kona-usb2.o
diff --git a/drivers/phy/phy-bcm-kona-usb2.c b/drivers/phy/phy-bcm-kona-usb2.c
new file mode 100644
index 000..1beea7f
--- /dev/null
+++ b/drivers/phy/phy-bcm-kona-usb2.c
@@ -0,0 +1,161 @@
+/*
+ * phy-bcm-kona-usb2.c - Broadcom Kona USB2 Phy Driver
+ *
+ * Copyright (C) 2013 Linaro Limited
+ * Matt Porter matt.por...@linaro.org
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/module.h
+#include linux/of.h
+#include linux/delay.h
+#include linux/platform_device.h
+#include linux/err.h
+#include linux/io.h
+#include linux/clk.h
+#include linux/phy/phy.h
+
+#define OTGCTL_OTGSTAT2(1  31)
+#define OTGCTL_OTGSTAT1(1  30)
+#define OTGCTL_PRST_N_SW   (1  11)
+#define OTGCTL_HRESET_N(1  10)
+#define OTGCTL_UTMI_LINE_STATE1(1  9)
+#define OTGCTL_UTMI_LINE_STATE0(1  8)
+
+#define P1CTL_SOFT_RESET   (1  1)
+#define P1CTL_NON_DRIVING  (1  0)
+
+struct bcm_kona_usb_phy_regs {
+   u32 ctrl;
+   u32 cfg;
+   u32 p1ctl;
+   u32 status;
+   u32 bc_cfg;
+   u32 tp_in;
+   u32 tp_out;
+   u32 phy_ctrl;
+   u32 usbreg;
+   u32 usbproben;
+};
+
+struct bcm_kona_usb {
+   struct bcm_kona_usb_phy_regs *regs;
+};
+
+static void bcm_kona_usb_phy_power(struct bcm_kona_usb *phy, int on)
+{
+   u32 val;
+
+   val = readl(phy-regs-ctrl);
+   if (on) {
+   /* Configure and power PHY */
+   val = ~(OTGCTL_OTGSTAT2 | OTGCTL_OTGSTAT1 |
+OTGCTL_UTMI_LINE_STATE1 | OTGCTL_UTMI_LINE_STATE0);
+   val |= OTGCTL_PRST_N_SW | OTGCTL_HRESET_N;
+   writel(val, phy-regs-ctrl);
+
+   /* Soft reset PHY */
+   val = readl(phy-regs-p1ctl);
+   val = ~P1CTL_NON_DRIVING;
+   val |= P1CTL_SOFT_RESET;
+   writel(val, phy-regs-p1ctl);
+   writel(val  ~P1CTL_SOFT_RESET, phy-regs-p1ctl);
+   /* Reset needs to be asserted for 2ms */
+   mdelay(2);
+   writel(val | P1CTL_SOFT_RESET, phy-regs-p1ctl);
+   } else {
+   val = ~(OTGCTL_PRST_N_SW | OTGCTL_HRESET_N);
+   writel(val, phy-regs-ctrl);
+   }
+}
+
+static int bcm_kona_usb_phy_power_on(struct phy *gphy)
+{
+   struct bcm_kona_usb *phy = phy_get_drvdata(gphy);
+
+   bcm_kona_usb_phy_power(phy, 1);
+
+   return 0;
+}
+
+static int bcm_kona_usb_phy_power_off(struct phy *gphy)
+{
+   struct bcm_kona_usb *phy = phy_get_drvdata(gphy);
+
+   bcm_kona_usb_phy_power(phy, 0);
+
+   return 0;
+}
+
+static struct phy_ops ops = {
+   .power_on   = bcm_kona_usb_phy_power_on,
+   .power_off  = bcm_kona_usb_phy_power_off,
+   .owner = THIS_MODULE,
+};
+
+static int bcm_kona_usb2_probe(struct platform_device *pdev)
+{
+   struct device *dev = pdev-dev;
+   struct bcm_kona_usb *phy;
+   struct resource *res;
+   struct phy *gphy;
+   struct phy_provider *phy_provider;
+
+   phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+   if (!phy)
+   return -ENOMEM;
+
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   phy-regs = devm_ioremap_resource(pdev-dev, res);
+   if (IS_ERR(phy-regs))
+   return PTR_ERR(phy-regs);
+
+   platform_set_drvdata(pdev, phy);
+
+   phy_provider =

[PATCH v2 5/9] usb: gadget: s3c-hsotg: enable generic phy support

2013-11-01 Thread Matt Porter

Adds support for the generic PHY subsystem. Generic PHY
support is probed and then the driver falls back to checking
for an old style USB PHY and pdata if not found.

Signed-off-by: Matt Porter matt.por...@linaro.org
---
 drivers/usb/gadget/s3c-hsotg.c | 54 --
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 3e0c124..f97978b 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -34,6 +34,7 @@
 #include linux/usb/ch9.h
 #include linux/usb/gadget.h
 #include linux/usb/phy.h
+#include linux/phy/phy.h
 #include linux/platform_data/s3c-hsotg.h
 
 #include s3c-hsotg.h
@@ -131,7 +132,8 @@ struct s3c_hsotg_ep {
  * struct s3c_hsotg - driver state.
  * @dev: The parent device supplied to the probe function
  * @driver: USB gadget driver
- * @phy: The otg phy transceiver structure for phy control.
+ * @phy: The otg phy transceiver structure for old USB phy control.
+ * @gphy: The otg phy transceiver structure for generic phy control.
  * @plat: The platform specific configuration data. This can be removed once
  * all SoCs support usb transceiver.
  * @regs: The memory area mapped for accessing registers.
@@ -154,6 +156,7 @@ struct s3c_hsotg {
struct device*dev;
struct usb_gadget_driver *driver;
struct usb_phy  *phy;
+   struct phy  *gphy;
struct s3c_hsotg_plat*plat;
 
spinlock_t  lock;
@@ -2820,9 +2823,12 @@ static void s3c_hsotg_phy_enable(struct s3c_hsotg *hsotg)
 
dev_dbg(hsotg-dev, pdev 0x%p\n, pdev);
 
-   if (hsotg-phy)
+   if (hsotg-gphy) {
+   phy_init(hsotg-gphy);
+   phy_power_on(hsotg-gphy);
+   } else if (hsotg-phy) {
usb_phy_init(hsotg-phy);
-   else if (hsotg-plat-phy_init)
+   } else if (hsotg-plat-phy_init)
hsotg-plat-phy_init(pdev, hsotg-plat-phy_type);
 }
 
@@ -2837,9 +2843,12 @@ static void s3c_hsotg_phy_disable(struct s3c_hsotg 
*hsotg)
 {
struct platform_device *pdev = to_platform_device(hsotg-dev);
 
-   if (hsotg-phy)
+   if (hsotg-gphy) {
+   phy_power_off(hsotg-gphy);
+   phy_exit(hsotg-gphy);
+   } else if (hsotg-phy) {
usb_phy_shutdown(hsotg-phy);
-   else if (hsotg-plat-phy_exit)
+   } else if (hsotg-plat-phy_exit)
hsotg-plat-phy_exit(pdev, hsotg-plat-phy_type);
 }
 
@@ -3446,6 +3455,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
 {
struct s3c_hsotg_plat *plat = dev_get_platdata(pdev-dev);
struct usb_phy *phy;
+   struct phy *gphy;
struct device *dev = pdev-dev;
struct s3c_hsotg_ep *eps;
struct s3c_hsotg *hsotg;
@@ -3460,19 +3470,27 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
return -ENOMEM;
}
 
-   phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
-   if (IS_ERR(phy)) {
-   /* Fallback for pdata */
-   plat = dev_get_platdata(pdev-dev);
-   if (!plat) {
-   dev_err(pdev-dev, no platform data or transceiver 
defined\n);
-   return -EPROBE_DEFER;
-   } else {
-   hsotg-plat = plat;
-   }
-   } else {
-   hsotg-phy = phy;
-   }
+   /*
+* Attempt to find a generic PHY, then look for an old style
+*  USB PHY, finally fall back to pdata
+*/
+   gphy = devm_phy_get(dev, usb2-phy);
+   if (IS_ERR(gphy)) {
+   phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+   if (IS_ERR(phy)) {
+   /* Fallback for pdata */
+   plat = dev_get_platdata(pdev-dev);
+   if (!plat) {
+   dev_err(pdev-dev,
+   no platform data or transceiver 
defined\n);
+   return -EPROBE_DEFER;
+   } else {
+   hsotg-plat = plat;
+   }
+   } else
+   hsotg-phy = phy;
+   } else
+   hsotg-gphy = gphy;
 
hsotg-dev = dev;
 
-- 
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

[PATCH v2 2/9] staging: dwc2: update DT binding to add generic clock/phy properties

2013-11-01 Thread Matt Porter

dwc2/s3c-hsotg require a single clock to be specified and optionally
a generic phy. On the s3c-hsotg driver old style USB phy support is
present as a fallback so the generic phy properties are optional.

Signed-off-by: Matt Porter matt.por...@linaro.org
---
 Documentation/devicetree/bindings/staging/dwc2.txt | 12 
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt 
b/Documentation/devicetree/bindings/staging/dwc2.txt
index 1a1b7cf..f60477e 100644
--- a/Documentation/devicetree/bindings/staging/dwc2.txt
+++ b/Documentation/devicetree/bindings/staging/dwc2.txt
@@ -5,6 +5,14 @@ Required properties:
 - compatible : snps,dwc2
 - reg : Should contain 1 register range (address and length)
 - interrupts : Should contain 1 interrupt
+- clocks: clock provider specifier
+- clock-names: shall be otg
+Refer to clk/clock-bindings.txt for generic clock consumer properties
+
+Optional properties:
+- phys: phy provider specifier
+- phy-names: shall be usb2-phy
+Refer to phy/phy-bindings.txt for generic phy consumer properties
 
 Example:
 
@@ -12,4 +20,8 @@ Example:
 compatible = ralink,rt3050-usb, snps,dwc2;
 reg = 0x101c 4;
 interrupts = 18;
+   clocks = usb_otg_ahb_clk;
+   clock-names = otg;
+   phys = usbphy;
+   phy-names = usb2-phy;
 };
-- 
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

[PATCH v2 0/9] USB Device Controller support for BCM281xx

2013-11-01 Thread Matt Porter

Changes since v1:
- Convert USB phy driver to generic phy subsystem
- Add phy bus width apis
- Drop dwc2 phy bus width DT property in favor of querying the
  phy provider for bus width
- Add generic phy/clock properties to dwc2 DT binding
- Add generic phy subsystem support to s3c-hsotg with the
  existing usb phy and pdata phy methods as a fallback
- Split bindings out to separate patches to match the latest
  DT binding review guidelines

This series adds USB Device Controller support for the Broadcom
BCM281xx family of parts. BCM281xx contains a DWC2 OTG block and
s3c-hsotg is used to support UDC operation.

Part 1 adds phy bus width support to the generic phy subsystem

Parts 2-6 allows s3c-hsotg to build on non-Samsung platforms, supports
the dwc2 binding, and enables generic phy support to support fetching
of the phy bus width.

Part 7-8 add a generic phy binding and driver for the BCM Kona USB PHY.

Part 9 adds the DT nodes to enable UDC support on both BCM281xx boards
in the kernel.

This series depends on the Update Kona drivers to use clocks series
(https://lkml.org/lkml/2013/10/3/645). The dependencies noted for that
series are already queued for 3.13. It also depends on the generic PHY
subsystem which has also been queued for 3.13.

Matt Porter (9):
  phy: add phy_get_bus_width()/phy_set_bus_width() calls
  staging: dwc2: update DT binding to add generic clock/phy properties
  usb: gadget: s3c-hsotg: enable build for other platforms
  usb: gadget: s3c-hsotg: add snps,dwc2 compatible string
  usb: gadget: s3c-hsotg: enable generic phy support
  usb: gadget: s3c-hsotg: get phy bus width from phy subsystem
  phy: add Broadcom Kona USB2 PHY DT binding
  phy: add Broadcom Kona USB2 PHY driver
  ARM: dts: add usb udc support to bcm281xx

 .../devicetree/bindings/phy/bcm-kona-usb2-phy.txt  |  15 ++
 Documentation/devicetree/bindings/staging/dwc2.txt |  12 ++
 arch/arm/boot/dts/bcm11351-brt.dts |   6 +
 arch/arm/boot/dts/bcm11351.dtsi|  18 +++
 arch/arm/boot/dts/bcm28155-ap.dts  |   8 +
 drivers/phy/Kconfig|   6 +
 drivers/phy/Makefile   |   2 +
 drivers/phy/phy-bcm-kona-usb2.c| 161 +
 drivers/usb/gadget/Kconfig |   7 +-
 drivers/usb/gadget/s3c-hsotg.c |  71 ++---
 drivers/usb/gadget/s3c-hsotg.h |   1 +
 include/linux/phy/phy.h|  16 ++
 12 files changed, 298 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt
 create mode 100644 drivers/phy/phy-bcm-kona-usb2.c

-- 
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

[PATCH v2 1/9] phy: add phy_get_bus_width()/phy_set_bus_width() calls

2013-11-01 Thread Matt Porter

This adds a pair of APIs that allows the generic PHY subsystem to
provide information on the PHY bus width. The PHY provider driver may
use phy_set_bus_width() to set the bus width that the PHY supports.
The controller driver may then use phy_get_bus_width() to fetch the
PHY bus width in order to properly configure the controller.

Signed-off-by: Matt Porter matt.por...@linaro.org
---
 include/linux/phy/phy.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..e858ce1 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -46,6 +46,7 @@ struct phy_ops {
  * @mutex: mutex to protect phy_ops
  * @init_count: used to protect when the PHY is used by multiple consumers
  * @power_count: used to protect when the PHY is used by multiple consumers
+ * @bus_width: used to specify data width of the PHY bus
  */
 struct phy {
struct device   dev;
@@ -55,6 +56,7 @@ struct phy {
struct mutexmutex;
int init_count;
int power_count;
+   int bus_width;
 };
 
 /**
@@ -127,6 +129,12 @@ int phy_init(struct phy *phy);
 int phy_exit(struct phy *phy);
 int phy_power_on(struct phy *phy);
 int phy_power_off(struct phy *phy);
+static inline int phy_get_bus_width(struct phy *phy) {
+   return phy-bus_width;
+};
+static inline void phy_set_bus_width(struct phy *phy, int bus_width) {
+   phy-bus_width = bus_width;
+};
 struct phy *phy_get(struct device *dev, const char *string);
 struct phy *devm_phy_get(struct device *dev, const char *string);
 void phy_put(struct phy *phy);
@@ -199,6 +207,14 @@ static inline int phy_power_off(struct phy *phy)
return -ENOSYS;
 }
 
+static inline int phy_get_bus_width(struct phy *phy) {
+   return -ENOSYS;
+};
+
+static inline void phy_set_bus_width(struct phy *phy, bus_width) {
+   return;
+};
+
 static inline struct phy *phy_get(struct device *dev, const char *string)
 {
return ERR_PTR(-ENOSYS);
-- 
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 9/9] ARM: dts: add usb udc support to bcm281xx

2013-11-01 Thread Sergei Shtylyov


Hello.

On 11/01/2013 10:45 PM, Matt Porter wrote:


Adds USB OTG/PHY and clock support to BCM281xx and enables
UDC support on the bcm11351-brt and bcm28155-ap boards.



Signed-off-by: Matt Porter matt.por...@linaro.org
Reviewed-by: Markus Mayer markus.ma...@linaro.org
Reviewed-by: Tim Kryger tim.kry...@linaro.org
---
  arch/arm/boot/dts/bcm11351-brt.dts |  6 ++
  arch/arm/boot/dts/bcm11351.dtsi| 18 ++
  arch/arm/boot/dts/bcm28155-ap.dts  |  8 
  3 files changed, 32 insertions(+)


[...]

diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
index 0755f43..247f9fd 100644
--- a/arch/arm/boot/dts/bcm11351.dtsi
+++ b/arch/arm/boot/dts/bcm11351.dtsi
@@ -284,4 +284,22 @@
#clock-cells = 0;
};
};
+
+   usbotg: usbotg@3f12 {


   According to ePAPR spec [1], the node name should be usb@3f12.



+   compatible = snps,dwc2;
+   reg = 0x3f12 0x1;
+   interrupts = GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH;
+   clocks = usb_otg_ahb_clk;
+   clock-names = otg;
+   phys = usbphy;
+   phy-names = usb2-phy;
+   status = disabled;
+   };
+
+   usbphy: usbphy@3f13 {


   This one should probably be named usb-phy@3f13, just like 
ethernet-phy from the ePAPR spec.



+   compatible = brcm,kona-usb2-phy;
+   reg = 0x3f13 0x28;
+   #phy-cells = 0;
+   status = disabled;
+   };
  };


[1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf

WBR, 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

RE: CSW endpoint status returned STALL after BOT MSC Reset

2013-11-01 Thread Paul Zimmerman

 From: Alan Stern
 Sent: Friday, November 01, 2013 7:09 AM
 
 Now DELAYED_STATUS isn't used anywhere.  You should add the following
 hunk to this patch:
 
 Index: usb-3.12/drivers/usb/gadget/storage_common.c
 ===
 --- usb-3.12.orig/drivers/usb/gadget/storage_common.c
 +++ usb-3.12/drivers/usb/gadget/storage_common.c
 @@ -156,7 +156,6 @@ static inline struct fsg_lun *fsg_lun_fr
 
  /* Big enough to hold our biggest descriptor */
  #define EP0_BUFSIZE  256
 -#define DELAYED_STATUS   (EP0_BUFSIZE + 999) /* An impossibly large 
 value */
 
  #ifdef CONFIG_USB_GADGET_DEBUG_FILES

And after that, EP0_BUFSIZE isn't used anywhere that I can see. So I
think that should be:

--- usb-3.12.orig/drivers/usb/gadget/storage_common.c
+++ usb-3.12/drivers/usb/gadget/storage_common.c
@@ -156,7 +156,3 @@ static inline struct fsg_lun *fsg_lun_fr
 
-/* Big enough to hold our biggest descriptor */
-#define EP0_BUFSIZE256
-#define DELAYED_STATUS (EP0_BUFSIZE + 999) /* An impossibly large value */
-
 #ifdef CONFIG_USB_GADGET_DEBUG_FILES
 
-- 
Paul

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Large USB HID transfers

2013-11-01 Thread Cliff Brake

On Thu, Oct 31, 2013 at 2:25 PM, Alan Stern st...@rowland.harvard.edu wrote:

  What host controller driver are you using?

 Its the USB EHCI host in the TI DM3730, so it uses the
 drivers/usb/host/ehci-omap.c driver.

 You can find out exactly what part of the kernel is responsible for
 interrupt delays by using the irqsoff tracer.  See
 Documentation/trace/ftrace.txt for details about how to use it.

I captured a trace here: http://bec-systems.com/usb-trace.txt

Still working to decode it -- any suggestions are welcome.

Enabling irqsoff significantly slows down the system.

Thanks,
Cliff
--
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 7/9] phy: add Broadcom Kona USB2 PHY DT binding

2013-11-01 Thread Arend van Spriel


On 11/01/2013 08:45 PM, Matt Porter wrote:

Add a binding that describes the Broadcom Kona USB2 PHY found
on the BCM281xx family of SoCs.

Signed-off-by: Matt Porter matt.por...@linaro.org
---
  .../devicetree/bindings/phy/bcm-kona-usb2-phy.txt | 15 +++
  1 file changed, 15 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt 
b/Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt
new file mode 100644
index 000..db309e2
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt
@@ -0,0 +1,15 @@
+BROADCOM KONA USB2 PHY
+
+Required properties:
+ - compatible: brcm,kona-usb2-phy
+ - regs: offset and length of the PHY registers
+ - #phy-cells: must be 0
+Refer to phy/phy-bindings.txt for the generic PHY binding properties
+
+Example:
+
+   usbphy: usbphy@3f13 {
+   compatible = brcm,kona-usb2-phy;
+   reg = 0x3f13 0x28;


I expect 'regs' iso 'reg' in this example.

Regards,
Arend


+   #phy-cells = 0;
+   };




--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Large USB HID transfers

2013-11-01 Thread Alan Stern

On Fri, 1 Nov 2013, Cliff Brake wrote:

 On Thu, Oct 31, 2013 at 2:25 PM, Alan Stern st...@rowland.harvard.edu wrote:
 
   What host controller driver are you using?
 
  Its the USB EHCI host in the TI DM3730, so it uses the
  drivers/usb/host/ehci-omap.c driver.
 
  You can find out exactly what part of the kernel is responsible for
  interrupt delays by using the irqsoff tracer.  See
  Documentation/trace/ftrace.txt for details about how to use it.
 
 I captured a trace here: http://bec-systems.com/usb-trace.txt
 
 Still working to decode it -- any suggestions are welcome.

As far as I can tell, the big blockage occurs when the machine has to 
analyze an input report from which it extracts 767 fields and ends up 
calling hid_process_event about 2300 times.  Since each call takes 
around 60-80 microseconds, you end up with a very large latency.

Why on earth are you using such enormous HID reports?

 Enabling irqsoff significantly slows down the system.

That's okay, since it's meant only for testing.

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 9/9] ARM: dts: add usb udc support to bcm281xx

2013-11-01 Thread Matt Porter

On Fri, Nov 01, 2013 at 11:56:33PM +0300, Sergei Shtylyov wrote:
 Hello.
 
 On 11/01/2013 10:45 PM, Matt Porter wrote:
 
 Adds USB OTG/PHY and clock support to BCM281xx and enables
 UDC support on the bcm11351-brt and bcm28155-ap boards.
 
 Signed-off-by: Matt Porter matt.por...@linaro.org
 Reviewed-by: Markus Mayer markus.ma...@linaro.org
 Reviewed-by: Tim Kryger tim.kry...@linaro.org
 ---
   arch/arm/boot/dts/bcm11351-brt.dts |  6 ++
   arch/arm/boot/dts/bcm11351.dtsi| 18 ++
   arch/arm/boot/dts/bcm28155-ap.dts  |  8 
   3 files changed, 32 insertions(+)
 
 [...]
 diff --git a/arch/arm/boot/dts/bcm11351.dtsi 
 b/arch/arm/boot/dts/bcm11351.dtsi
 index 0755f43..247f9fd 100644
 --- a/arch/arm/boot/dts/bcm11351.dtsi
 +++ b/arch/arm/boot/dts/bcm11351.dtsi
 @@ -284,4 +284,22 @@
  #clock-cells = 0;
  };
  };
 +
 +usbotg: usbotg@3f12 {
 
According to ePAPR spec [1], the node name should be usb@3f12.

Will address in v3.

 +compatible = snps,dwc2;
 +reg = 0x3f12 0x1;
 +interrupts = GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH;
 +clocks = usb_otg_ahb_clk;
 +clock-names = otg;
 +phys = usbphy;
 +phy-names = usb2-phy;
 +status = disabled;
 +};
 +
 +usbphy: usbphy@3f13 {
 
This one should probably be named usb-phy@3f13, just like
 ethernet-phy from the ePAPR spec.

Yes, agreed that will follow the same naming convention. I'll roll this
in v3. Thanks for the review.

-Matt

 +compatible = brcm,kona-usb2-phy;
 +reg = 0x3f13 0x28;
 +#phy-cells = 0;
 +status = disabled;
 +};
   };
 
 [1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf
 
 WBR, 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

Re: [PATCH v2 7/9] phy: add Broadcom Kona USB2 PHY DT binding

2013-11-01 Thread Matt Porter

On Fri, Nov 01, 2013 at 09:54:10PM +0100, Arend van Spriel wrote:
 On 11/01/2013 08:45 PM, Matt Porter wrote:
 Add a binding that describes the Broadcom Kona USB2 PHY found
 on the BCM281xx family of SoCs.
 
 Signed-off-by: Matt Porter matt.por...@linaro.org
 ---
   .../devicetree/bindings/phy/bcm-kona-usb2-phy.txt | 15 
  +++
   1 file changed, 15 insertions(+)
   create mode 100644 
  Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt
 
 diff --git a/Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt 
 b/Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt
 new file mode 100644
 index 000..db309e2
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/phy/bcm-kona-usb2-phy.txt
 @@ -0,0 +1,15 @@
 +BROADCOM KONA USB2 PHY
 +
 +Required properties:
 + - compatible: brcm,kona-usb2-phy
 + - regs: offset and length of the PHY registers
 + - #phy-cells: must be 0
 +Refer to phy/phy-bindings.txt for the generic PHY binding properties
 +
 +Example:
 +
 +usbphy: usbphy@3f13 {
 +compatible = brcm,kona-usb2-phy;
 +reg = 0x3f13 0x28;
 
 I expect 'regs' iso 'reg' in this example.

Yes, will fix the typo in that property for v3.

Thanks,
Matt

 +#phy-cells = 0;
 +};
 
 
 
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

90 matches

Mail list logo