Re: [PATCH net 1/4] pegasus: Use heap buffers for all register access

[Petko Manolov]

On 17-02-07 10:32:16, Steve Calfee wrote:
> On Mon, Feb 6, 2017 at 4:51 AM, Petko Manolov  wrote:
> > On 17-02-06 09:28:22, Greg KH wrote:
> >> On Mon, Feb 06, 2017 at 10:14:44AM +0200, Petko Manolov wrote:
> >> > On 17-02-05 01:30:39, Greg KH wrote:
> >> > > On Sat, Feb 04, 2017 at 04:56:03PM +, Ben Hutchings wrote:
> >> > > > Allocating USB buffers on the stack is not portable, and no longer 
> >> > > > works
> >> > > > on x86_64 (with VMAP_STACK enabled as per default).
> >> > >
> >> > > It's never worked on other platforms, so these should go to the stable
> >> > > releases please.
> >> >
> >> > As far as i know both drivers works fine on other platforms, though I 
> >> > only
> >> > tested it on arm and mipsel. ;)
> >>
> >> It all depends on the arm and mips platforms, the ones that can not DMA 
> >> from stack memory are the ones that would always fail here (since the 2.2 
> >> kernel days).
> >
> > Seems like most modern SOCs have decent DMA controllers.
> >
> 
> The real problem is not DMA exactly, it is cache coherency.
> 
> X86 has a coherent cache and all the cpu cores watch DMA transfers and keep 
> the cpu caches up to date.

Yep, these cpus are more user friendly.

> Most ARMs and MIPS processors have incoherent cache, so DMA can change memory 
> without the CPU cache updates. CPU cache view of what is in memory can be 
> different from what was DMAed in, this makes failures very hard to detect, 
> reproduce and racy.

Except for a very few purposes (like framebuffer memory) one should be crazy to 
use anything but kseg1 for accessing the peripherals.  One of the real problems 
here is the 512MB limit of the uncached segment.  While the DMA controller can 
typically access all available RAM you'll run into the issues that you describe 
with more than that amount of memory.

MIPS like doing things small and simple.  Everybody knows that software can 
compensate for hardware omissions. ;)

> So all DMA buffers should always be separate allocations from the stack AND 
> not be embedded in structs. Memory allocations are always at least cache line 
> aligned, so coherency is not a problem.

I do agree.


cheers,
Petko
--
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 2/2] drivers: usb: gadget: udc: remove logically dead code

[Gustavo A. R. Silva]

Remove unnecesary code because zlt never evaluates to zero.

Addresses-Coverity-ID: 1226747
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/usb/gadget/udc/mv_udc_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/mv_udc_core.c 
b/drivers/usb/gadget/udc/mv_udc_core.c
index 56b3574..9ca6d92 100644
--- a/drivers/usb/gadget/udc/mv_udc_core.c
+++ b/drivers/usb/gadget/udc/mv_udc_core.c
@@ -509,7 +509,7 @@ static int mv_ep_enable(struct usb_ep *_ep,
dqh = ep->dqh;
dqh->max_packet_length = (max << EP_QUEUE_HEAD_MAX_PKT_LEN_POS)
| (mult << EP_QUEUE_HEAD_MULT_POS)
-   | (zlt ? EP_QUEUE_HEAD_ZLT_SEL : 0)
+   | EP_QUEUE_HEAD_ZLT_SEL
| (ios ? EP_QUEUE_HEAD_IOS : 0);
dqh->next_dtd_ptr = 1;
dqh->size_ioc_int_sts = 0;
-- 
2.5.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

[PATCH 1/2] drivers: usb: gadget: udc: add missing break in switch

[Gustavo A. R. Silva]

Add missing break in switch.

Addresses-Coverity-ID: 201385
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/usb/gadget/udc/mv_udc_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/udc/mv_udc_core.c 
b/drivers/usb/gadget/udc/mv_udc_core.c
index 27ebb0d..56b3574 100644
--- a/drivers/usb/gadget/udc/mv_udc_core.c
+++ b/drivers/usb/gadget/udc/mv_udc_core.c
@@ -489,6 +489,7 @@ static int mv_ep_enable(struct usb_ep *_ep,
break;
case USB_ENDPOINT_XFER_CONTROL:
ios = 1;
+   break;
case USB_ENDPOINT_XFER_INT:
mult = 0;
break;
-- 
2.5.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

[PATCH] drivers: usb: early: remove unused code

[Gustavo A. R. Silva]

Remove this line of code because devnum is overwritten before it can be used.
This could happen if line of code 609 (goto try_again;) is executed. Otherwise,
devnum is never used again.

Addresses-Coverity-ID: 1226870
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/usb/early/ehci-dbgp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
index ea73afb..e265444 100644
--- a/drivers/usb/early/ehci-dbgp.c
+++ b/drivers/usb/early/ehci-dbgp.c
@@ -580,7 +580,6 @@ static int _dbgp_external_startup(void)
USB_DEBUG_DEVNUM);
goto err;
}
-   devnum = USB_DEBUG_DEVNUM;
dbgp_printk("debug device renamed to 127\n");
}
 
-- 
2.5.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

How to resolve "Waited 2000ms for CONNECT" in system resuming?

[Yoshihiro Shimoda]

Hi,

In my environment, it causes the following message during system resume if 
debug messages are enabled:

usb 2-1: Waited 2000ms for CONNECT

< My environment >
 - EHCI/OHCI controllers on R-Car H3 
(arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts)
 - Greg's usb.git / next branch (c95a9f83711bf53faeb4ed9bbb63a3f065613dfb) + 
some dts patches for R-Car H3
 - A USB 1.1 (full speed) device (A USB1.1 hub is easy to reproduce)

< Details >
- I investigated this issue and I found the issue is related the workqueue of 
drivers/usb/core/hub.c.
  If I modified the flags from WQ_FREEZABLE to "0", the issue disappeared.

/*
 * The workqueue needs to be freezable to avoid interfering with
 * USB-PERSIST port handover. Otherwise it might see that a full-speed
 * device was gone before the EHCI controller had handed its port
 * over to the companion full-speed controller.
 */
hub_wq = alloc_workqueue("usb_hub_wq", WQ_FREEZABLE, 0);

- IIUC, an EHCI connects a full speed device first. After bus reset, an OHCI 
can connect the device.
  However, if WQ_FREEZABLE is set, the hub driver cannot issue bus reset while 
system resuming,
  and then the issue happened.

- I also found another option about "persist" feature on sysfs. If a USB1.1 
device (exclude a hub) is connected,
  we can disable the feature via sysfs and then the issue also disappeared.

< Question >
How to resolve the issue?
 - Can we modified the flags of the hub's workqueue?
 - Should we disable the persist feature if we need to avoid the wait in system 
resume?
 - Or, other ideas?

FYI, a kernel log is copied in the end of this email when the issue happened.

Best regards,
Yoshihiro Shimoda

---
NOTICE:  BL2: R-Car Gen3 Initial Program Loader(CA57) Rev.1.0.12
NOTICE:  BL2: PRR is R-Car H3 ES1.1
NOTICE:  BL2: Boot device is HyperFlash(80MHz)
NOTICE:  BL2: LCM state is CM
NOTICE:  BL2: AVS setting succeeded. DVFS_SetVID=0x52
NOTICE:  BL2: DDR1600(rev.0.20)[COLD_BOOT]..0
NOTICE:  BL2: DRAM Split is 4ch
NOTICE:  BL2: QoS is default setting(rev.0.33)
NOTICE:  BL2: v1.3(release):c040be5
NOTICE:  BL2: Built : 16:03:29, Jan 30 2017
NOTICE:  BL2: Normal boot
NOTICE:  BL2: dst=0xe6317190 src=0x818 len=512(0x200)
NOTICE:  BL2: dst=0x43f0 src=0x8180400 len=6144(0x1800)
NOTICE:  BL2: dst=0x4400 src=0x81c len=65536(0x1)
NOTICE:  BL2: dst=0x4410 src=0x820 len=524288(0x8)
NOTICE:  BL2: dst=0x5000 src=0x864 len=1048576(0x10)


U-Boot 2015.04 (Jan 30 2017 - 16:03:32)

CPU: Renesas Electronics R8A7795 rev 1.1
Board: Salvator-X
I2C:   ready
DRAM:  3.9 GiB
MMC:   sh-sdhi: 0, sh-sdhi: 1, sh-sdhi: 2
In:serial
Out:   serial
Err:   serial
Net:   ravb
Hit any key to stop autoboot:  3 2 1 0 
ravb Waiting for PHY auto negotiation to complete.. done
ravb: 100Base/Full
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 192.168.44.124 (1314 ms)
Using ravb device
TFTP from server 192.168.44.74; our IP address is 192.168.44.124
Filename '/salvator-x/r8a7795-salvator-x.dtb'.
Load address: 0x4800
Loading: * ###
 1.9 MiB/s
done
Bytes transferred = 35769 (8bb9 hex)
ravb:0 is connected to ravb.  Reconnecting to ravb
ravb Waiting for PHY auto negotiation to complete.. done
ravb: 100Base/Full
Using ravb device
TFTP from server 192.168.44.74; our IP address is 192.168.44.124
Filename '/salvator-x/Image'.
Load address: 0x4808
Loading: * #
 #
 #
 #
 #
 #
 #
 #
 #
 #
 #
 #
 ###
 1.1 MiB/s
done
Bytes transferred = 12311040 (bbda00 hex)
## Flattened Device Tree blob at 4800
   Booting using the fdt blob at 0x4800
   Using Device Tree in place at 4800, end 4800bbb8

Starting kernel ...

[0.00] Booting Linux on physical CPU 0x0
[0.00] Linux version 4.9.0-rc5-00059-g9f5d196-dirty 
(shimoda@shimoda-i7-pc) (gcc version 5.2.1 20151005 (Linaro GCC 5.2-2015.11-1) 
) #45 SMP PREEMPT Wed Feb 8 14:23:47 JST 2017
[0.00] Boot CPU: AArch64 Processor [411fd073]
[0.00] 

[PATCH] drivers: usb-misc: sisusbvga: remove dead code

[Gustavo A. R. Silva]

The condition modex % 16 cannot be true when modex value is equal to 640
The condition du & 0xff cannot be true when du value is equal to 0x1400

Addresses-Coverity-Id: 101163
Addresses-Coverity-Id: 744373
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/usb/misc/sisusbvga/sisusb.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/usb/misc/sisusbvga/sisusb.c 
b/drivers/usb/misc/sisusbvga/sisusb.c
index 05bd39d..440d7fe 100644
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -1831,16 +1831,10 @@ static int sisusb_set_default_mode(struct 
sisusb_usb_data *sisusb,
SETIREGANDOR(SISCR, 0x09, 0x5f, ((crtcdata[16] & 0x01) << 5));
SETIREG(SISCR, 0x14, 0x4f);
du = (modex / 16) * (bpp * 2);  /* offset/pitch */
-   if (modex % 16)
-   du += bpp;
-
SETIREGANDOR(SISSR, 0x0e, 0xf0, ((du >> 8) & 0x0f));
SETIREG(SISCR, 0x13, (du & 0xff));
du <<= 5;
tmp8 = du >> 8;
-   if (du & 0xff)
-   tmp8++;
-
SETIREG(SISSR, 0x10, tmp8);
SETIREG(SISSR, 0x31, 0x00); /* VCLK */
SETIREG(SISSR, 0x2b, 0x1b);
-- 
2.5.0

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

Re: [PATCH net-next v2 08/12] iscsi: fix build errors when linux/phy*.h is removed from net/dsa.h

[Nicholas A. Bellinger]

Hi Florian,

On Tue, 2017-02-07 at 15:03 -0800, Florian Fainelli wrote:
> From: Russell King 
> 
> drivers/target/iscsi/iscsi_target_login.c:1135:7: error: implicit declaration 
> of function 'try_module_get' [-Werror=implicit-function-declaration]
> 
> Add linux/module.h to iscsi_target_login.c.
> 
> Signed-off-by: Russell King 
> Reviewed-by: Bart Van Assche 
> ---

Acked-by: Nicholas Bellinger 

--
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/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications

[kbuild test robot]

Hi Stefan,

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.10-rc7]
[cannot apply to next-20170207]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Stefan-Br-ns/sierra_net-Add-support-for-IPv6-and-Dual-Stack-Link-Sense-Indications/20170207-105111
config: x86_64-randconfig-b0-02081035 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/net/usb/sierra_net.c: In function 'sierra_net_bind':
>> drivers/net/usb/sierra_net.c:687: warning: unused variable 'eth'

vim +/eth +687 drivers/net/usb/sierra_net.c

f7385ec9 Ming Lei  2012-10-24  671  if (result < 0)
eb4fd8cd Elina Pasheva 2010-04-27  672  return -EIO;
eb4fd8cd Elina Pasheva 2010-04-27  673  
f7385ec9 Ming Lei  2012-10-24  674  *datap = le16_to_cpu(attrdata);
eb4fd8cd Elina Pasheva 2010-04-27  675  return result;
eb4fd8cd Elina Pasheva 2010-04-27  676  }
eb4fd8cd Elina Pasheva 2010-04-27  677  
eb4fd8cd Elina Pasheva 2010-04-27  678  /*
eb4fd8cd Elina Pasheva 2010-04-27  679   * collects the bulk endpoints, the 
status endpoint.
eb4fd8cd Elina Pasheva 2010-04-27  680   */
eb4fd8cd Elina Pasheva 2010-04-27  681  static int sierra_net_bind(struct 
usbnet *dev, struct usb_interface *intf)
eb4fd8cd Elina Pasheva 2010-04-27  682  {
eb4fd8cd Elina Pasheva 2010-04-27  683  u8  ifacenum;
eb4fd8cd Elina Pasheva 2010-04-27  684  u8  numendpoints;
eb4fd8cd Elina Pasheva 2010-04-27  685  u16 fwattr = 0;
eb4fd8cd Elina Pasheva 2010-04-27  686  int status;
eb4fd8cd Elina Pasheva 2010-04-27 @687  struct ethhdr *eth;
eb4fd8cd Elina Pasheva 2010-04-27  688  struct sierra_net_data *priv;
eb4fd8cd Elina Pasheva 2010-04-27  689  static const u8 
sync_tmplate[sizeof(priv->sync_msg)] = {
eb4fd8cd Elina Pasheva 2010-04-27  690  0x00, 0x00, 
SIERRA_NET_HIP_MSYNC_ID, 0x00};
eb4fd8cd Elina Pasheva 2010-04-27  691  static const u8 
shdwn_tmplate[sizeof(priv->shdwn_msg)] = {
eb4fd8cd Elina Pasheva 2010-04-27  692  0x00, 0x00, 
SIERRA_NET_HIP_SHUTD_ID, 0x00};
eb4fd8cd Elina Pasheva 2010-04-27  693  
eb4fd8cd Elina Pasheva 2010-04-27  694  dev_dbg(>udev->dev, "%s", 
__func__);
eb4fd8cd Elina Pasheva 2010-04-27  695  

:: The code at line 687 was first introduced by commit
:: eb4fd8cd355c8ec425a12ec6cbdac614e8a4819d net/usb: add sierra_net.c driver

:: TO: Elina Pasheva <epash...@sierrawireless.com>
:: CC: David S. Miller <da...@davemloft.net>

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH v3 1/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications

[Stefan Brüns]

If a context is configured as dualstack ("IPv4v6"), the modem indicates
the context activation with a slightly different indication message.
The dual-stack indication omits the link_type (IPv4/v6) and adds
additional address fields.
IPv6 LSIs are identical to IPv4 LSIs, but have a different link type.

Signed-off-by: Stefan Brüns 
---
v2: Do not overwrite protocol field in rx_fixup
v3: Remove leftover struct ethhdr *eth declaration

Example LSI LINK UP indication:

   00 ed 78 00 04 01 00 e9 0a 14 00 54 00 65 00 6c  ..xT.e.l
0010   00 65 00 6b 00 6f 00 6d 00 2e 00 64 00 65 48 03  .e.k.o.m...d.eH.
0020   c8 be d1 00 62 00 00 00 2c 80 f0 01 00 00 00 00  b...,...
0030   30 cb 04 4c 49 4e 4b 20 55 50 00 00 00 00 00 00  0..LINK UP..
0040   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
0050   00 00 00 00 04 0a 23 38 db 10 2a 01 05 98 88 c0  ..#8..*.
0060   1f da 00 01 00 01 91 23 a8 f9 00 00 00 00 00 00  ...#
0070   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
0080   00 04 0a 4a d2 d2 10 2a 01 05 98 07 ff 00 00 00  ...J...*
0090   10 00 74 02 10 02 10 04 0a 4a d2 d3 10 2a 01 05  ..t..J...*..
00a0   98 07 ff 00 00 00 10 00 74 02 10 02 11 00 00 00  t...
00b0   00 00 00 00 00 00 00 00 00 00 00 00 00 ff ff 00  
00c0   00 00 00 00 00 c3 50 04 00 00 00 00 10 fe 80 00  ..P.
00d0   00 00 00 00 00 00 00 00 00 00 00 00 05 00 00 00  
00e0   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
00f0   00
---
 drivers/net/usb/sierra_net.c | 101 ---
 1 file changed, 66 insertions(+), 35 deletions(-)

diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index a251588762ec..6300a4454ae5 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -73,8 +73,6 @@ staticatomic_t iface_counter = ATOMIC_INIT(0);
 /* Private data structure */
 struct sierra_net_data {
 
-   u8 ethr_hdr_tmpl[ETH_HLEN]; /* ethernet header template for rx'd pkts */
-
u16 link_up;/* air link up or down */
u8 tx_hdr_template[4];  /* part of HIP hdr for tx'd packets */
 
@@ -122,6 +120,7 @@ struct param {
 
 /* LSI Protocol types */
 #define SIERRA_NET_PROTOCOL_UMTS  0x01
+#define SIERRA_NET_PROTOCOL_UMTS_DS   0x04
 /* LSI Coverage */
 #define SIERRA_NET_COVERAGE_NONE  0x00
 #define SIERRA_NET_COVERAGE_NOPACKET  0x01
@@ -129,7 +128,8 @@ struct param {
 /* LSI Session */
 #define SIERRA_NET_SESSION_IDLE   0x00
 /* LSI Link types */
-#define SIERRA_NET_AS_LINK_TYPE_IPv4  0x00
+#define SIERRA_NET_AS_LINK_TYPE_IPV4  0x00
+#define SIERRA_NET_AS_LINK_TYPE_IPV6  0x02
 
 struct lsi_umts {
u8 protocol;
@@ -137,9 +137,14 @@ struct lsi_umts {
__be16 length;
/* eventually use a union for the rest - assume umts for now */
u8 coverage;
-   u8 unused2[41];
+   u8 network_len; /* network name len */
+   u8 network[40]; /* network name (UCS2, bigendian) */
u8 session_state;
u8 unused3[33];
+} __packed;
+
+struct lsi_umts_single {
+   struct lsi_umts lsi;
u8 link_type;
u8 pdp_addr_len; /* NW-supplied PDP address len */
u8 pdp_addr[16]; /* NW-supplied PDP address (bigendian)) */
@@ -158,10 +163,31 @@ struct lsi_umts {
u8 reserved[8];
 } __packed;
 
+struct lsi_umts_dual {
+   struct lsi_umts lsi;
+   u8 pdp_addr4_len; /* NW-supplied PDP IPv4 address len */
+   u8 pdp_addr4[4];  /* NW-supplied PDP IPv4 address (bigendian)) */
+   u8 pdp_addr6_len; /* NW-supplied PDP IPv6 address len */
+   u8 pdp_addr6[16]; /* NW-supplied PDP IPv6 address (bigendian)) */
+   u8 unused4[23];
+   u8 dns1_addr4_len; /* NW-supplied 1st DNS v4 address len (bigendian) */
+   u8 dns1_addr4[4];  /* NW-supplied 1st DNS v4 address */
+   u8 dns1_addr6_len; /* NW-supplied 1st DNS v6 address len */
+   u8 dns1_addr6[16]; /* NW-supplied 1st DNS v6 address (bigendian)*/
+   u8 dns2_addr4_len; /* NW-supplied 2nd DNS v4 address len (bigendian) */
+   u8 dns2_addr4[4];  /* NW-supplied 2nd DNS v4 address */
+   u8 dns2_addr6_len; /* NW-supplied 2nd DNS v6 address len */
+   u8 dns2_addr6[16]; /* NW-supplied 2nd DNS v6 address (bigendian)*/
+   u8 unused5[68];
+} __packed;
+
 #define SIERRA_NET_LSI_COMMON_LEN  4
-#define SIERRA_NET_LSI_UMTS_LEN(sizeof(struct lsi_umts))
+#define SIERRA_NET_LSI_UMTS_LEN(sizeof(struct lsi_umts_single))
 #define SIERRA_NET_LSI_UMTS_STATUS_LEN \
(SIERRA_NET_LSI_UMTS_LEN - SIERRA_NET_LSI_COMMON_LEN)
+#define SIERRA_NET_LSI_UMTS_DS_LEN (sizeof(struct lsi_umts_dual))
+#define SIERRA_NET_LSI_UMTS_DS_STATUS_LEN \
+   (SIERRA_NET_LSI_UMTS_DS_LEN - SIERRA_NET_LSI_COMMON_LEN)
 
 /* Forward definitions */
 static void sierra_sync_timer(unsigned long syncdata);
@@ -191,10 +217,11 @@ 

[PATCH v3 2/2] sierra_net: Skip validating irrelevant fields for IDLE LSIs

[Stefan Brüns]

When the context is deactivated, the link_type is set to 0xff, which
triggers a warning message, and results in a wrong link status, as
the LSI is ignored.

Signed-off-by: Stefan Brüns 
---
 drivers/net/usb/sierra_net.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index 6300a4454ae5..0b5a84c9022c 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -385,6 +385,13 @@ static int sierra_net_parse_lsi(struct usbnet *dev, char 
*data, int datalen)
return -1;
}
 
+   /* Validate the session state */
+   if (lsi->session_state == SIERRA_NET_SESSION_IDLE) {
+   netdev_err(dev->net, "Session idle, 0x%02x\n",
+  lsi->session_state);
+   return 0;
+   }
+
/* Validate the protocol  - only support UMTS for now */
if (lsi->protocol == SIERRA_NET_PROTOCOL_UMTS) {
struct lsi_umts_single *single = (struct lsi_umts_single *)lsi;
@@ -418,13 +425,6 @@ static int sierra_net_parse_lsi(struct usbnet *dev, char 
*data, int datalen)
return 0;
}
 
-   /* Validate the session state */
-   if (lsi->session_state == SIERRA_NET_SESSION_IDLE) {
-   netdev_err(dev->net, "Session idle, 0x%02x\n",
-   lsi->session_state);
-   return 0;
-   }
-
/* Set link_sense true */
return 1;
 }
-- 
2.11.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

[PATCH v3 0/2] Fixes for sierra_net driver

[Stefan Brüns]

When trying to initiate a dual-stack (ipv4v6) connection, a MC7710, FW
version SWI9200X_03.05.24.00ap answers with an unsupported LSI. Add support
for this LSI.
Also the link_type should be ignored when going idle, otherwise the modem
is stuck in a bad link state.
Tested on MC7710, T-Mobile DE, APN internet.telekom, IPv4v6 PDP type. Both
IPv4 and IPv6 connections work.

v2: Do not overwrite protocol field in rx_fixup
v3: Remove leftover struct ethhdr *eth declaration

Stefan Brüns (2):
  sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications
  sierra_net: Skip validating irrelevant fields for IDLE LSIs

 drivers/net/usb/sierra_net.c | 111 +++
 1 file changed, 71 insertions(+), 40 deletions(-)

-- 
2.11.0

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

RE: [PATCH resend v5 0/4] xhci fixes for usb-linus

[Peter Chen]

 
>
>> On Thu, Jan 19, 2017 at 02:21:26PM +0200, Mathias Nyman wrote:
>> > Hi Greg
>> >
>> > This series by Arnd Bergmann was originally six patches, but last
>> > two of them were already taken to 4.10. Without the rest of them
>> > there will be a regression in 4.10.
>> >
>> > Original cover letter says:
>> >
>> > For xhci-hcd platform device, all the DMA parameters are not
>> > configured properly, notably dma ops for dwc3 devices.
>> >
>> > The idea here is that you pass in the parent of_node along with the
>> > child device pointer, so it would behave exactly like the parent
>> > already does. The difference is that it also handles all the other
>> > attributes besides the mask.
>> >
>> > -Mathias
>> >
>> > Arnd Bergmann (4):
>> >   usb: separate out sysdev pointer from usb_bus
>> >   usb: chipidea: use bus->sysdev for DMA configuration
>> >   usb: ehci: fsl: use bus->sysdev for DMA configuration
>> >   usb: xhci: use bus->sysdev for DMA configuration
>>
>> The ehci part needs to similar change, otherwise, it will fail too.
>> I post a patch for it [1]. Alan, would you please help to review it?
>> Then, Greg can pick up the five patches together.
>>
>> [1] http://www.spinics.net/lists/linux-usb/msg153263.html
>
>That patch contains a lot of unrelated whitespace changes.  Can you redo it 
>with only
>the necessary changes?
>

Alan, the ./scripts/checkpatch.pl complaints it, so I made changes, and I think 
the
whitespace changes are not needed to have an additional patch, so I add these 
whitespace
changes to this patch, is it ok I just add the comments to mention that?

Below is console output for the patch without whitespace changes:

b29397@b29397-desktop:~/work/projects/usb$ ./scripts/checkpatch.pl 
0001-usb-ehci-use-bus-sysdev-for-DMA-configuration.patch
WARNING: space prohibited between function name and open parenthesis '('
#25: FILE: drivers/usb/host/ehci-mem.c:141:
+   dma_free_coherent (ehci_to_hcd(ehci)->self.sysdev,

WARNING: space prohibited between function name and open parenthesis '('
#70: FILE: drivers/usb/host/ehci-mem.c:202:
+   dma_alloc_coherent (ehci_to_hcd(ehci)->self.sysdev,

total: 0 errors, 2 warnings, 48 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
  mechanically convert to the typical style using --fix or --fix-inplace.

0001-usb-ehci-use-bus-sysdev-for-DMA-configuration.patch has style problems, 
please review.


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

Re: USB SUBSYSTEM - drivers/usb/core/config.c

[Oliver Neukum]

Am Dienstag, den 07.02.2017, 19:01 +0100 schrieb John Skelton:
> I'm open to any suggestions.  As I see it the above code is probably
> wrong
> in trying to use a low speed Bulk endpoint.  The low performance of
> cheap
> Arduinos and digistump etc (with no hardware USB) is not going to
> allow
> anything but low speed (via software), however.

The usual work around for this issue is to treat them as interrupt
endpoints on the host side. That won work for XHCI though.

HTH
Oliver

--
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 v2 02/12] net: cgroups: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

net/core/netprio_cgroup.c:303:16: error: expected declaration specifiers or 
'...' before string constant
MODULE_LICENSE("GPL v2");
   ^~~~

Add linux/module.h to fix this.

Signed-off-by: Russell King 
---
 net/core/netprio_cgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index 2ec86fc552df..756637dc7a57 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -13,6 +13,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.9.3

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

[PATCH net-next v2 00/12] net: dsa: remove unnecessary phy.h include

[Florian Fainelli]

Hi all,

Including phy.h and phy_fixed.h into net/dsa.h causes phy*.h to be an
unnecessary dependency for quite a large amount of the kernel.  There's
very little which actually requires definitions from phy.h in net/dsa.h
- the include itself only wants the declaration of a couple of
structures and IFNAMSIZ.

Add linux/if.h for IFNAMSIZ, declarations for the structures, phy.h to
mv88e6xxx.h as it needs it for phy_interface_t, and remove both phy.h
and phy_fixed.h from net/dsa.h.

This patch reduces from around 800 files rebuilt to around 40 - even
with ccache, the time difference is noticable.

In order to make this change, several drivers need to be updated to
include necessary headers that they were picking up through this
include.  This has resulted in a much larger patch series.

I'm assuming the 0-day builder has had 24 hours with this series, and
hasn't reported any further issues with it - the last issue was two
weeks ago (before I became ill) which I fixed over the last weekend.

I'm hoping this doesn't conflict with what's already in net-next...

David, this should probably go via your tree considering the diffstat.

Changes in v2:

- took Russell's patch series
- removed Qualcomm EMAC patch
- rebased against net-next/master

Russell King (12):
  net: sunrpc: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: cgroups: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: macb: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: lan78xx: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: bgmac: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: fman: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: mvneta: fix build errors when linux/phy*.h is removed from
net/dsa.h
  iscsi: fix build errors when linux/phy*.h is removed from net/dsa.h
  MIPS: Octeon: Remove unnecessary MODULE_*()
  net: liquidio: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: ath5k: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: dsa: remove unnecessary phy*.h includes

 arch/mips/cavium-octeon/octeon-platform.c | 4 
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 +
 drivers/net/ethernet/broadcom/bgmac.c | 2 ++
 drivers/net/ethernet/cadence/macb.h   | 2 ++
 drivers/net/ethernet/cavium/liquidio/lio_main.c   | 1 +
 drivers/net/ethernet/cavium/liquidio/lio_vf_main.c| 1 +
 drivers/net/ethernet/cavium/liquidio/octeon_console.c | 1 +
 drivers/net/ethernet/freescale/fman/fman_memac.c  | 1 +
 drivers/net/ethernet/marvell/mvneta.c | 1 +
 drivers/net/usb/lan78xx.c | 1 +
 drivers/net/wireless/ath/ath5k/ahb.c  | 2 +-
 drivers/target/iscsi/iscsi_target_login.c | 1 +
 include/net/dsa.h | 5 +++--
 net/core/netprio_cgroup.c | 1 +
 net/sunrpc/xprtrdma/svc_rdma_backchannel.c| 1 +
 15 files changed, 18 insertions(+), 7 deletions(-)

-- 
2.9.3

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

[PATCH net-next v2 03/12] net: macb: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/ethernet/cadence/macb.h:862:33: sparse: expected ; at end of 
declaration
drivers/net/ethernet/cadence/macb.h:862:33: sparse: Expected } at end of 
struct-union-enum-specifier
drivers/net/ethernet/cadence/macb.h:862:33: sparse: got phy_interface
drivers/net/ethernet/cadence/macb.h:877:1: sparse: Expected ; at the end of 
type declaration
drivers/net/ethernet/cadence/macb.h:877:1: sparse: got }
In file included from drivers/net/ethernet/cadence/macb_pci.c:29:0:
drivers/net/ethernet/cadence/macb.h:862:2: error: unknown type name 
'phy_interface_t'
 phy_interface_t  phy_interface;
 ^~~

Add linux/phy.h to macb.h

Signed-off-by: Russell King 
Acked-by: Nicolas Ferre 
---
 drivers/net/ethernet/cadence/macb.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index a2cf91223003..234a49eaccfd 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -10,6 +10,8 @@
 #ifndef _MACB_H
 #define _MACB_H
 
+#include 
+
 #define MACB_GREGS_NBR 16
 #define MACB_GREGS_VERSION 2
 #define MACB_MAX_QUEUES 8
-- 
2.9.3

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

[PATCH net-next v2 04/12] net: lan78xx: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/usb/lan78xx.c:394:33: sparse: expected ; at end of declaration
drivers/net/usb/lan78xx.c:394:33: sparse: Expected } at end of 
struct-union-enum-specifier
drivers/net/usb/lan78xx.c:394:33: sparse: got interface
drivers/net/usb/lan78xx.c:403:1: sparse: Expected ; at the end of type 
declaration
drivers/net/usb/lan78xx.c:403:1: sparse: got }

Add linux/phy.h to lan78xx.c

Signed-off-by: Russell King 
---
 drivers/net/usb/lan78xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 08f8703e4d54..9889a70ff4f6 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "lan78xx.h"
 
 #define DRIVER_AUTHOR  "WOOJUNG HUH "
-- 
2.9.3

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

[PATCH net-next v2 01/12] net: sunrpc: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

Removing linux/phy.h from net/dsa.h reveals a build error in the sunrpc
code:

net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_rdma_bc_put':
net/sunrpc/xprtrdma/svc_rdma_backchannel.c:277:2: error: implicit declaration 
of function 'module_put' [-Werror=implicit-function-declaration]
net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_setup_rdma_bc':
net/sunrpc/xprtrdma/svc_rdma_backchannel.c:348:7: error: implicit declaration 
of function 'try_module_get' [-Werror=implicit-function-declaration]

Fix this by adding linux/module.h to svc_rdma_backchannel.c

Signed-off-by: Russell King 
Acked-by: Anna Schumaker 
---
 net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c 
b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
index 288e35c2d8f4..cb1e48e54eb1 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
@@ -4,6 +4,7 @@
  * Support for backward direction RPCs on RPC/RDMA (server-side).
  */
 
+#include 
 #include 
 #include "xprt_rdma.h"
 
-- 
2.9.3

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

[PATCH net-next v2 05/12] net: bgmac: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/ethernet/broadcom/bgmac.c:1015:17: error: dereferencing pointer to 
incomplete type 'struct mii_bus'
drivers/net/ethernet/broadcom/bgmac.c:1185:2: error: implicit declaration of 
function 'phy_start' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1198:2: error: implicit declaration of 
function 'phy_stop' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1239:9: error: implicit declaration of 
function 'phy_mii_ioctl' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1389:28: error: 
'phy_ethtool_get_link_ksettings' undeclared here (not in a function)
drivers/net/ethernet/broadcom/bgmac.c:1390:28: error: 
'phy_ethtool_set_link_ksettings' undeclared here (not in a function)
drivers/net/ethernet/broadcom/bgmac.c:1403:13: error: dereferencing pointer to 
incomplete type 'struct phy_device'
drivers/net/ethernet/broadcom/bgmac.c:1417:3: error: implicit declaration of 
function 'phy_print_status' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1424:26: error: storage size of 
'fphy_status' isn't known
drivers/net/ethernet/broadcom/bgmac.c:1424:9: error: variable 'fphy_status' has 
initializer but incomplete type
drivers/net/ethernet/broadcom/bgmac.c:1425:11: warning: excess elements in 
struct initializer
drivers/net/ethernet/broadcom/bgmac.c:1425:3: error: unknown field 'link' 
specified in initializer
drivers/net/ethernet/broadcom/bgmac.c:1426:12: note: in expansion of macro 
'SPEED_1000'
drivers/net/ethernet/broadcom/bgmac.c:1426:3: error: unknown field 'speed' 
specified in initializer
drivers/net/ethernet/broadcom/bgmac.c:1427:13: note: in expansion of macro 
'DUPLEX_FULL'
drivers/net/ethernet/broadcom/bgmac.c:1427:3: error: unknown field 'duplex' 
specified in initializer
drivers/net/ethernet/broadcom/bgmac.c:1432:12: error: implicit declaration of 
function 'fixed_phy_register' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1432:31: error: 'PHY_POLL' undeclared 
(first use in this function)
drivers/net/ethernet/broadcom/bgmac.c:1438:8: error: implicit declaration of 
function 'phy_connect_direct' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1439:6: error: 'PHY_INTERFACE_MODE_MII' 
undeclared (first use in this function)
drivers/net/ethernet/broadcom/bgmac.c:1521:2: error: implicit declaration of 
function 'phy_disconnect' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1541:15: error: expected declaration 
specifiers or '...' before string constant

Add linux/phy.h to bgmac.c

Signed-off-by: Russell King 
Acked-by: Rafał Miłecki 
---
 drivers/net/ethernet/broadcom/bgmac.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index fe88126b1e0c..20fe2520da42 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -12,6 +12,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "bgmac.h"
 
 static bool bgmac_wait_value(struct bgmac *bgmac, u16 reg, u32 mask,
-- 
2.9.3

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

[PATCH net-next v2 09/12] MIPS: Octeon: Remove unnecessary MODULE_*()

[Florian Fainelli]

From: Russell King 

octeon-platform.c can not be built as a module for two reasons:

(a) the Makefile doesn't allow it:
obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o

(b) the multiple *_initcall() statements, each of which are translated
to a module_init() call when attempting a module build, become
aliases to init_module().  Having more than one alias will cause a
build error.

Hence, rather than adding a linux/module.h include, remove the redundant
MODULE_*() from this file.

Acked-by: David Daney 
Signed-off-by: Russell King 
---
 arch/mips/cavium-octeon/octeon-platform.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-platform.c 
b/arch/mips/cavium-octeon/octeon-platform.c
index 37a932d9148c..8297ce714c5e 100644
--- a/arch/mips/cavium-octeon/octeon-platform.c
+++ b/arch/mips/cavium-octeon/octeon-platform.c
@@ -1060,7 +1060,3 @@ static int __init octeon_publish_devices(void)
return of_platform_bus_probe(NULL, octeon_ids, NULL);
 }
 arch_initcall(octeon_publish_devices);
-
-MODULE_AUTHOR("David Daney ");
-MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Platform driver for Octeon SOC");
-- 
2.9.3

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

[PATCH net-next v2 12/12] net: dsa: remove unnecessary phy*.h includes

[Florian Fainelli]

From: Russell King 

Including phy.h and phy_fixed.h into net/dsa.h causes phy*.h to be an
unnecessary dependency for quite a large amount of the kernel.  There's
very little which actually requires definitions from phy.h in net/dsa.h
- the include itself only wants the declaration of a couple of
structures and IFNAMSIZ.

Add linux/if.h for IFNAMSIZ, declarations for the structures, phy.h to
mv88e6xxx.h as it needs it for phy_interface_t, and remove both phy.h
and phy_fixed.h from net/dsa.h.

This patch reduces from around 800 files rebuilt to around 40 - even
with ccache, the time difference is noticable.

Tested-by: Vivien Didelot 
Reviewed-by: Florian Fainelli 
Signed-off-by: Russell King 
---
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 +
 include/net/dsa.h | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h 
b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 8a21800374f3..91c4dd25c2d3 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifndef UINT64_MAX
 #define UINT64_MAX (u64)(~((u64)0))
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b49b2004891e..4e13e695f025 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -11,17 +11,18 @@
 #ifndef __LINUX_NET_DSA_H
 #define __LINUX_NET_DSA_H
 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
 
 struct tc_action;
+struct phy_device;
+struct fixed_phy_status;
 
 enum dsa_tag_protocol {
DSA_TAG_PROTO_NONE = 0,
-- 
2.9.3

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

[PATCH net-next v2 10/12] net: liquidio: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: error: expected 
declaration specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: error: type defaults to 
'int' in declaration of 'MODULE_AUTHOR'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: error: function 
declaration isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: error: expected 
declaration specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: error: type defaults to 
'int' in declaration of 'MODULE_DESCRIPTION'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: error: function 
declaration isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: error: expected 
declaration specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: error: type defaults to 
'int' in declaration of 'MODULE_LICENSE'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: error: function 
declaration isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: error: expected 
declaration specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: error: type defaults to 
'int' in declaration of 'MODULE_VERSION'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: error: function 
declaration isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:36: error: expected ')' 
before 'int'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:37: error: expected ')' 
before string constant
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:325: warning: data 
definition has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:325: error: type defaults to 
'int' in declaration of 'MODULE_DEVICE_TABLE'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:325: warning: parameter 
names (without types) in function declaration
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3250: warning: data 
definition has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3250: error: type defaults 
to 'int' in declaration of 'module_init'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3250: warning: parameter 
names (without types) in function declaration
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3251: warning: data 
definition has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3251: error: type defaults 
to 'int' in declaration of 'module_exit'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3251: warning: parameter 
names (without types) in function declaration
drivers/net/ethernet/cavium/liquidio/lio_main.c:36: error: expected declaration 
specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:36: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:36: error: type defaults to 
'int' in declaration of 'MODULE_AUTHOR'
drivers/net/ethernet/cavium/liquidio/lio_main.c:36: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:37: error: expected declaration 
specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:37: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:37: error: type defaults to 
'int' in declaration of 'MODULE_DESCRIPTION'
drivers/net/ethernet/cavium/liquidio/lio_main.c:37: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:38: error: expected declaration 
specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:38: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:38: error: type defaults to 
'int' in declaration of 'MODULE_LICENSE'
drivers/net/ethernet/cavium/liquidio/lio_main.c:38: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:39: error: expected declaration 
specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:39: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:39: error: type defaults to 
'int' in declaration of 'MODULE_VERSION'
drivers/net/ethernet/cavium/liquidio/lio_main.c:39: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:40: 

[PATCH net-next v2 08/12] iscsi: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/target/iscsi/iscsi_target_login.c:1135:7: error: implicit declaration 
of function 'try_module_get' [-Werror=implicit-function-declaration]

Add linux/module.h to iscsi_target_login.c.

Signed-off-by: Russell King 
Reviewed-by: Bart Van Assche 
---
 drivers/target/iscsi/iscsi_target_login.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/target/iscsi/iscsi_target_login.c 
b/drivers/target/iscsi/iscsi_target_login.c
index 450f51deb2a2..eab274d17b5c 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -17,6 +17,7 @@
  
**/
 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.9.3

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

[PATCH net-next v2 06/12] net: fman: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/ethernet/freescale/fman/fman_memac.c:519:21: error: dereferencing 
pointer to incomplete type 'struct fixed_phy_status'

Add linux/phy_fixed.h to fman_memac.c

Signed-off-by: Russell King 
---
 drivers/net/ethernet/freescale/fman/fman_memac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c 
b/drivers/net/ethernet/freescale/fman/fman_memac.c
index 71a5ded9d1de..cd6a53eaf161 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* PCS registers */
-- 
2.9.3

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

[PATCH net-next v2 07/12] net: mvneta: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/ethernet/marvell/mvneta.c:2694:26: error: storage size of 'status' 
isn't known
drivers/net/ethernet/marvell/mvneta.c:2695:26: error: storage size of 'changed' 
isn't known
drivers/net/ethernet/marvell/mvneta.c:2695:9: error: variable 'changed' has 
initializer but incomplete type
drivers/net/ethernet/marvell/mvneta.c:2709:2: error: implicit declaration of 
function 'fixed_phy_update_state' [-Werror=implicit-function-declaration]

Add linux/phy_fixed.h to mvneta.c

Signed-off-by: Russell King 
Acked-by: Thomas Petazzoni 
---
 drivers/net/ethernet/marvell/mvneta.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/marvell/mvneta.c 
b/drivers/net/ethernet/marvell/mvneta.c
index 0f4d1697be46..fdf71720e707 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.9.3

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

[PATCH net-next v2 11/12] net: ath5k: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

Fix these errors reported by the 0-day builder by replacing the
linux/export.h include with linux/module.h.

In file included from include/linux/platform_device.h:14:0,
 from drivers/net/wireless/ath/ath5k/ahb.c:20:
include/linux/device.h:1463:1: warning: data definition has no type or storage 
class
 module_init(__driver##_init); \
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
include/linux/device.h:1463:1: error: type defaults to 'int' in declaration of 
'module_init' [-Werror=implicit-int]
 module_init(__driver##_init); \
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: warning: parameter names (without 
types) in function declaration
In file included from include/linux/platform_device.h:14:0,
 from drivers/net/wireless/ath/ath5k/ahb.c:20:
include/linux/device.h:1468:1: warning: data definition has no type or storage 
class
 module_exit(__driver##_exit);
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
include/linux/device.h:1468:1: error: type defaults to 'int' in declaration of 
'module_exit' [-Werror=implicit-int]
 module_exit(__driver##_exit);
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: warning: parameter names (without 
types) in function declaration
In file included from include/linux/platform_device.h:14:0,
 from drivers/net/wireless/ath/ath5k/ahb.c:20:
drivers/net/wireless/ath/ath5k/ahb.c:233:24: warning: 'ath_ahb_driver_exit' 
defined but not used [-Wunused-function]
 module_platform_driver(ath_ahb_driver);
^
include/linux/device.h:1464:20: note: in definition of macro 'module_driver'
 static void __exit __driver##_exit(void) \
^~~~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
drivers/net/wireless/ath/ath5k/ahb.c:233:24: warning: 'ath_ahb_driver_init' 
defined but not used [-Wunused-function]
 module_platform_driver(ath_ahb_driver);
^
include/linux/device.h:1459:19: note: in definition of macro 'module_driver'
 static int __init __driver##_init(void) \
   ^~~~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~

Signed-off-by: Russell King 
Acked-by: Kalle Valo 
---
 drivers/net/wireless/ath/ath5k/ahb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath5k/ahb.c 
b/drivers/net/wireless/ath/ath5k/ahb.c
index 2ca88b593e4c..c0794f5988b3 100644
--- a/drivers/net/wireless/ath/ath5k/ahb.c
+++ b/drivers/net/wireless/ath/ath5k/ahb.c
@@ -16,10 +16,10 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include "ath5k.h"
 #include "debug.h"
-- 
2.9.3

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

Re: [PATCH V2 1/2] dt-bindings: leds: document new led-triggers property

[Rob Herring]

On Wed, Feb 1, 2017 at 3:55 PM, Rafał Miłecki  wrote:
> On 02/01/2017 10:26 PM, Jacek Anaszewski wrote:
>>
>> On 02/01/2017 04:56 PM, Rafał Miłecki wrote:
>>>
>>> On 01/31/2017 10:34 PM, Jacek Anaszewski wrote:

 On 01/31/2017 05:11 PM, Rafał Miłecki wrote:
>
> Thanks a lot Jacek for this explanation (and sorry but I needed a bit
> of
> time to
> think about this). I can finally see your point, I think we're on the
> same page
> now.
>
> I think that for all this time I was thinking from the pure DT
> perspective. If
> you ignore fact that Linux has multiple LED trigger drivers, then
> "led-triggers"
> may not sound that bad.
>
> After reading your description however I can see this property can be
> misleading
> as Linux people may think "drivers" when seeing "triggers".
>
> I still think this is a useful property and I hope we can still find a
> way to
> name it in a sane way: to be nice from DT PoV and march Linux LEDs
> subsystem.


 I also see the need for this property.

> I think we should still work on some generic property (without linux,
> prefix) as
> this seems to be something generic, not really specific to Linux
> implementation.
> Specifying relation between LED and devices is something than AFAICS
> can be
> reused by other systems as well.
>
> So my suggestion is to try some new name & leave linux,default-trigger
> to allow
> specifying one single trigger driver as required by current Linux
> implementation.
>
> What do you think about this?
>
> There are few suggestions to came to my mind:
> "trigger-sources"
> "trigger-devices"
> "led-trigger-devices"
> "led-related-devices"
> "led-event-devices"


 trigger-devices would work in this specific use case,
 where we can give phandles to usb ports. Nonetheless, we
 have also other kernel based events serving as trigger
 sources - e.g. timer.

 In this case I'd go for trigger-sources, but we'd need
 to have some generic way of defining the source like timer.

+1 for the name.

>>>
>>>
>>> I'd leave timer for later discussion but I'm glad you brought this
>>> problem.
>>> Maybe we could discuss this on another example that is more supported
>>> like
>>> GPIOs?
>>>
>>> We know this property allows specifying USB ports and we want it to
>>> support
>>> mixing various sources. So a very simple sample case seems to be using
>>> it for
>>> specifying GPIO as trigger source.
>>>
>>> Is this possible to mix various entries in a list assigned to single
>>> property?
>>> Let's say:
>>> trigger-sources =
>>> <_port1>,
>>> <_port1>,
>>> < 1 GPIO_ACTIVE_HIGH>;
>>
>>
>> According to my knowledge all elements in the list are elements
>> of one array, no matter if they are comma separated or space separated
>> in "<>" brackets. DT maintainer would have to confirm that though.
>
>
> This matches my knowledge as well.

It is a bit problematic, but this could work as long as you know the
possible arg types and a given node only has 1 match (nodes with
interrupt and gpio being likely). You look up the phandle, iterate
thru possible "#*-cells" properties for that phandle, then parse the
cells. You could define rules of what is the required cell type (e.g.
must use gpio cells if a phandle has both gpio and interrupt cells
defined.).

Another, simpler approach would be defining #trigger-cells in each
source. This is probably the better option as all the infrastructure
is there and could handle unforeseen cases.

>>> Is this possible to support this? If so, which function I can use to
>>> detect
>>> what is pointed by a phandle?
>>> I'm not that familiar with DT and I'm not sure how to handle this.
>>
>>
>> I wonder if this is correct way to go. Maybe we should think of
>> creating entirely new trigger mechanism that would allow for having
>> multiple active triggers at a time.
>
>
> I'm OK with reworking Linux's triggers if it need be. I think we should
> agree
> on binding(s) first though.

Yes, they are separate problems.

Rob
--
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/2] dt-bindings: leds: document new led-triggers property

[Jacek Anaszewski]

On 02/03/2017 12:00 AM, Rafał Miłecki wrote:
> On 02/02/2017 09:44 PM, Jacek Anaszewski wrote:
>> On 02/01/2017 10:55 PM, Rafał Miłecki wrote:
>>> On 02/01/2017 10:26 PM, Jacek Anaszewski wrote:
 On 02/01/2017 04:56 PM, Rafał Miłecki wrote:
> On 01/31/2017 10:34 PM, Jacek Anaszewski wrote:
>> On 01/31/2017 05:11 PM, Rafał Miłecki wrote:
>>> Thanks a lot Jacek for this explanation (and sorry but I needed a
>>> bit of
>>> time to
>>> think about this). I can finally see your point, I think we're on
>>> the
>>> same page
>>> now.
>>>
>>> I think that for all this time I was thinking from the pure DT
>>> perspective. If
>>> you ignore fact that Linux has multiple LED trigger drivers, then
>>> "led-triggers"
>>> may not sound that bad.
>>>
>>> After reading your description however I can see this property
>>> can be
>>> misleading
>>> as Linux people may think "drivers" when seeing "triggers".
>>>
>>> I still think this is a useful property and I hope we can still
>>> find a
>>> way to
>>> name it in a sane way: to be nice from DT PoV and march Linux LEDs
>>> subsystem.
>>
>> I also see the need for this property.
>>
>>> I think we should still work on some generic property (without
>>> linux,
>>> prefix) as
>>> this seems to be something generic, not really specific to Linux
>>> implementation.
>>> Specifying relation between LED and devices is something than AFAICS
>>> can be
>>> reused by other systems as well.
>>>
>>> So my suggestion is to try some new name & leave
>>> linux,default-trigger
>>> to allow
>>> specifying one single trigger driver as required by current Linux
>>> implementation.
>>>
>>> What do you think about this?
>>>
>>> There are few suggestions to came to my mind:
>>> "trigger-sources"
>>> "trigger-devices"
>>> "led-trigger-devices"
>>> "led-related-devices"
>>> "led-event-devices"
>>
>> trigger-devices would work in this specific use case,
>> where we can give phandles to usb ports. Nonetheless, we
>> have also other kernel based events serving as trigger
>> sources - e.g. timer.
>>
>> In this case I'd go for trigger-sources, but we'd need
>> to have some generic way of defining the source like timer.
>
> I'd leave timer for later discussion but I'm glad you brought this
> problem.
> Maybe we could discuss this on another example that is more supported
> like
> GPIOs?
>
> We know this property allows specifying USB ports and we want it to
> support
> mixing various sources. So a very simple sample case seems to be using
> it for
> specifying GPIO as trigger source.
>
> Is this possible to mix various entries in a list assigned to single
> property?
> Let's say:
> trigger-sources =
> <_port1>,
> <_port1>,
> < 1 GPIO_ACTIVE_HIGH>;

 According to my knowledge all elements in the list are elements
 of one array, no matter if they are comma separated or space separated
 in "<>" brackets. DT maintainer would have to confirm that though.
>>>
>>> This matches my knowledge as well.
>>
>> Having that, we would be limited to a list of fixed size
>> tuples IMHO.
> 
> That sounds OK. Now I spent some time thinking about this I think it can
> work.
> First of all we may need something like #sources-cells to extend our
> property
> in the future.
> Secondly it should be possible to detect type of phandle node by trying
> things
> one by one. We should be e.g. able to check is phandle is for GPIO by
> trying
> of_find_gpiochip_by_xlate.
> 
> 
> Is this possible to support this? If so, which function I can use to
> detect
> what is pointed by a phandle?
> I'm not that familiar with DT and I'm not sure how to handle this.

 I wonder if this is correct way to go. Maybe we should think of
 creating entirely new trigger mechanism that would allow for having
 multiple active triggers at a time.
>>>
>>> I'm OK with reworking Linux's triggers if it need be. I think we should
>>> agree
>>> on binding(s) first though.
>>
>> I was thinking of creating entirely new mechanism (new sysfs file),
>> as we can't break existing users.
> 
> Agree.
> 
> 
 Of course trigger-sources would be still useful for defining
 a list of devices of the same type like in case of usbport trigger.
 Nonetheless, I have a problem with this property being a part of
 LED class device DT bindings. Triggers are not tightly coupled with
 LED class devices, but trigger-sources being a list of usb ports
 would be applicable only to usbport trigger. What if there was
 another combo trigger e.g. for reporting activity on mulitple GPIOs?
>>>
>>> That was exactly my point with above trigger-sources example including 2
>>> USB
>>> ports & GPIO.

Re: USB SUBSYSTEM - drivers/usb/core/config.c

[Alan Stern]

On Tue, 7 Feb 2017, John Skelton wrote:

> > What happens after that?  Does the cdc-acm driver refuse to bind?  Any
> > other types of error/warning messages?
> 
> Along the lines of:
> usb 2-1.2: >new low-speed USB device number 95 using ehci_hcd
> usb 2-1.2: >device descriptor read/64, error -32
> usb 2-1.2: >config 1 interface 1 altsetting 0 endpoint 0x1 is Bulk; changing 
> to Interrupt
> usb 2-1.2: >config 1 interface 1 altsetting 0 endpoint 0x81 is Bulk; changing 
> to Interrupt
> usb 2-1.2: >New USB device found, idVendor=16d0, idProduct=087e
> usb 2-1.2: >New USB device strings: Mfr=1, Product=2, SerialNumber=0
> usb 2-1.2: >Product: Digispark Serial
> usb 2-1.2: >Manufacturer: digistump.com
> cdc_acm 2-1.2:1.0: >ttyACM0: USB ACM device
> 
> But screen /dev/ttyACM0 (with or without a "baud" rate) just hangs.

Have you tried using minicom instead of screen?

Do any other messages show up in the kernel log?

What does /sys/kernel/debug/usb/devices say?

Can you collect a usbmon trace?

In theory, this should work.  Especially since the port you're
connecting to is hooked up to an EHCI controller and not an xHCI
controller.

> > Doing this type of thing really isn't a good idea for the obvious reason
> > that hubs might just die a horrible death.  Who knows what a USB 3
> > controller would do with a low speed device like this as well (hubs are
> > crazy complex, and low speed makes them really sad.)
> 
> Ah.  Maybe it's my laptop's internal hub that hangs.  Though the other USB
> ports carry on working.

That is unlikely.  Treating a bulk interrupt as an interrupt endpoint
makes very little difference (mostly it just affects the throughput,
but a low-speed connection wouldn't be expected to have high throughput
anyway).

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: USB SUBSYSTEM - drivers/usb/core/config.c

[Greg KH]

On Tue, Feb 07, 2017 at 07:28:40PM +0100, John Skelton wrote:
> > From: "Greg KH" 
> > On Tue, Feb 07, 2017 at 06:56:20PM +0100, John Skelton wrote:
> > > > From: "Felipe Balbi" 
> > > > > I'm particularly interested in drivers/usb/core/config.c
> > > > > which appears to enforce the USB specification by refusing to allow a
> > > > > low speed CDC ACM.  (Comment "Some buggy low-speed devices ...", at
> > > > > about line 300.)
> > > > >
> > > > > However, such devices exist and some are potentially quite useful 
> > > > > (such
> > > > > as Arduinos & digistump).  Various people have posted about not being
> > > > > able to use them with Linux and I think the above file is the reason
> > > > > (another well known OS family allows them).
> > > >
> > > > Here's the comment:
> > > > 
> > > > /* Some buggy low-speed devices have Bulk endpoints, which is
> > > >  * explicitly forbidden by the USB spec.  In an attempt to make
> > > >  * them usable, we will try treating them as Interrupt 
> > > > endpoints.
> > > >  */
> > > > 
> > > > The code isn't forbidding CDC ACM low speed devices, it's forbidding
> > > > Bulk endpoints on Low speed, which don't exist :-) Are you saying there
> > > > are Bulk endpoints on low speed Arduinos?
> > > 
> > > Yes - or I may have misunderstood the situation.
> > > 
> > > It is code which is derived from V-USB
> > > https://www.obdev.at/products/vusb/index.html
> > > and is described at
> > > http://www.recursion.jp/prose/avrcdc/cdc-232.html
> > > 
> > > If it worked with Linux it would be a handy way to provide output (such as
> > > debug data) from low-cost Arduinos and such like via USB.
> > > 
> > > To be clear: I am not saying the Linux USB code is wrong.  But it would
> > > be good to have a way to have the above kind of code work, such that
> > > screen / minicom etc can be used with it.
> > > 
> > 
> > That page says that any kernel newer than 2.6.31 should work just fine,
> > have you tried this out and had it fail?
> 
> It does say that but it does not work.  I get the dev_warn message that it
> has changed the Bulk endpoint to Interrupt but that's it.

What happens after that?  Does the cdc-acm driver refuse to bind?  Any
other types of error/warning messages?

Doing this type of thing really isn't a good idea for the obvious reason
that hubs might just die a horrible death.  Who knows what a USB 3
controller would do with a low speed device like this as well (hubs are
crazy complex, and low speed makes them really sad.)

You would be better off spending a few cents and getting a better USB
device controller.  It will save everyone here, and yourself, a lot of
time and money...

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 v2 2/2] sierra_net: Skip validating irrelevant fields for IDLE LSIs

[David Miller]

From: Stefan Brüns 
Date: Tue, 7 Feb 2017 03:33:17 +0100

> When the context is deactivated, the link_type is set to 0xff, which
> triggers a warning message, and results in a wrong link status, as
> the LSI is ignored.
> 
> Signed-off-by: Stefan Brüns 

Applied.
--
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/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications

[David Miller]

From: Stefan Brüns 
Date: Tue, 7 Feb 2017 03:33:16 +0100

> If a context is configured as dualstack ("IPv4v6"), the modem indicates
> the context activation with a slightly different indication message.
> The dual-stack indication omits the link_type (IPv4/v6) and adds
> additional address fields.
> IPv6 LSIs are identical to IPv4 LSIs, but have a different link type.
> 
> Signed-off-by: Stefan Brüns 

Applied.

Ignore my earlier comments about the UMTS comment, it is still
accurate of course. :-)
--
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 SUBSYSTEM - drivers/usb/core/config.c

[John Skelton]

> From: "Greg KH" 
> To: "John Skelton" 
[snip]
> > > > It is code which is derived from V-USB
> > > > https://www.obdev.at/products/vusb/index.html
> > > > and is described at
> > > > http://www.recursion.jp/prose/avrcdc/cdc-232.html
> > > > 
> > > > If it worked with Linux it would be a handy way to provide output (such 
> > > > as
> > > > debug data) from low-cost Arduinos and such like via USB.
> > > > 
> > > > To be clear: I am not saying the Linux USB code is wrong.  But it would
> > > > be good to have a way to have the above kind of code work, such that
> > > > screen / minicom etc can be used with it.
> > > > 
> > > 
> > > That page says that any kernel newer than 2.6.31 should work just fine,
> > > have you tried this out and had it fail?
> > 
> > It does say that but it does not work.  I get the dev_warn message that it
> > has changed the Bulk endpoint to Interrupt but that's it.
> 
> What happens after that?  Does the cdc-acm driver refuse to bind?  Any
> other types of error/warning messages?

Along the lines of:
usb 2-1.2: >new low-speed USB device number 95 using ehci_hcd
usb 2-1.2: >device descriptor read/64, error -32
usb 2-1.2: >config 1 interface 1 altsetting 0 endpoint 0x1 is Bulk; changing to 
Interrupt
usb 2-1.2: >config 1 interface 1 altsetting 0 endpoint 0x81 is Bulk; changing 
to Interrupt
usb 2-1.2: >New USB device found, idVendor=16d0, idProduct=087e
usb 2-1.2: >New USB device strings: Mfr=1, Product=2, SerialNumber=0
usb 2-1.2: >Product: Digispark Serial
usb 2-1.2: >Manufacturer: digistump.com
cdc_acm 2-1.2:1.0: >ttyACM0: USB ACM device

But screen /dev/ttyACM0 (with or without a "baud" rate) just hangs.

> Doing this type of thing really isn't a good idea for the obvious reason
> that hubs might just die a horrible death.  Who knows what a USB 3
> controller would do with a low speed device like this as well (hubs are
> crazy complex, and low speed makes them really sad.)

Ah.  Maybe it's my laptop's internal hub that hangs.  Though the other USB
ports carry on working.

> You would be better off spending a few cents and getting a better USB
> device controller.  It will save everyone here, and yourself, a lot of
> time and money...

True :(

Consider it closed if you'd rather.  I'd rather not take up the ML's useful time
if there's no reasonable workaround.  People can debug via flashing LEDs etc...

John
--
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/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications

[David Miller]

From: David Miller 
Date: Tue, 07 Feb 2017 13:53:12 -0500 (EST)

> From: Stefan Brüns 
> Date: Tue, 7 Feb 2017 03:33:16 +0100
> 
>> If a context is configured as dualstack ("IPv4v6"), the modem indicates
>> the context activation with a slightly different indication message.
>> The dual-stack indication omits the link_type (IPv4/v6) and adds
>> additional address fields.
>> IPv6 LSIs are identical to IPv4 LSIs, but have a different link type.
>> 
>> Signed-off-by: Stefan Brüns 
> 
> Applied.
> 
> Ignore my earlier comments about the UMTS comment, it is still
> accurate of course. :-)

Acutally, this still need work.  I had to revert.

You removed the code using the 'eth' variable from sierra_net_bind() but
left the unused variable declared.

Please respin this series with it removed so we don't get the warning:

drivers/net/usb/sierra_net.c: In function ‘sierra_net_bind’:
drivers/net/usb/sierra_net.c:687:17: warning: unused variable ‘eth’ 
[-Wunused-variable]
  struct ethhdr *eth;

Re: [PATCH net 1/4] pegasus: Use heap buffers for all register access

[Steve Calfee]

On Mon, Feb 6, 2017 at 4:51 AM, Petko Manolov  wrote:
> On 17-02-06 09:28:22, Greg KH wrote:
>> On Mon, Feb 06, 2017 at 10:14:44AM +0200, Petko Manolov wrote:
>> > On 17-02-05 01:30:39, Greg KH wrote:
>> > > On Sat, Feb 04, 2017 at 04:56:03PM +, Ben Hutchings wrote:
>> > > > Allocating USB buffers on the stack is not portable, and no longer 
>> > > > works
>> > > > on x86_64 (with VMAP_STACK enabled as per default).
>> > >
>> > > It's never worked on other platforms, so these should go to the stable
>> > > releases please.
>> >
>> > As far as i know both drivers works fine on other platforms, though I only
>> > tested it on arm and mipsel. ;)
>>
>> It all depends on the arm and mips platforms, the ones that can not DMA from
>> stack memory are the ones that would always fail here (since the 2.2 kernel
>> days).
>
> Seems like most modern SOCs have decent DMA controllers.
>

The real problem is not DMA exactly, it is cache coherency.

X86 has a coherent cache and all the cpu cores watch DMA transfers and
keep the cpu caches up to date.

Most ARMs and MIPS processors have incoherent cache, so DMA can change
memory without the CPU cache updates. CPU cache view of what is in
memory can be different from what was DMAed in, this makes failures
very hard to detect, reproduce and racy.

So all DMA buffers should always be separate allocations from the
stack AND not be embedded in structs. Memory allocations are always at
least cache line aligned, so coherency is not a problem.

Regards, Steve
--
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 SUBSYSTEM - drivers/usb/core/config.c

[John Skelton]

> From: "Greg KH" 
> On Tue, Feb 07, 2017 at 06:56:20PM +0100, John Skelton wrote:
> > > From: "Felipe Balbi" 
> > > > I'm particularly interested in drivers/usb/core/config.c
> > > > which appears to enforce the USB specification by refusing to allow a
> > > > low speed CDC ACM.  (Comment "Some buggy low-speed devices ...", at
> > > > about line 300.)
> > > >
> > > > However, such devices exist and some are potentially quite useful (such
> > > > as Arduinos & digistump).  Various people have posted about not being
> > > > able to use them with Linux and I think the above file is the reason
> > > > (another well known OS family allows them).
> > >
> > > Here's the comment:
> > > 
> > >   /* Some buggy low-speed devices have Bulk endpoints, which is
> > >* explicitly forbidden by the USB spec.  In an attempt to make
> > >* them usable, we will try treating them as Interrupt endpoints.
> > >*/
> > > 
> > > The code isn't forbidding CDC ACM low speed devices, it's forbidding
> > > Bulk endpoints on Low speed, which don't exist :-) Are you saying there
> > > are Bulk endpoints on low speed Arduinos?
> > 
> > Yes - or I may have misunderstood the situation.
> > 
> > It is code which is derived from V-USB
> > https://www.obdev.at/products/vusb/index.html
> > and is described at
> > http://www.recursion.jp/prose/avrcdc/cdc-232.html
> > 
> > If it worked with Linux it would be a handy way to provide output (such as
> > debug data) from low-cost Arduinos and such like via USB.
> > 
> > To be clear: I am not saying the Linux USB code is wrong.  But it would
> > be good to have a way to have the above kind of code work, such that
> > screen / minicom etc can be used with it.
> > 
> 
> That page says that any kernel newer than 2.6.31 should work just fine,
> have you tried this out and had it fail?

It does say that but it does not work.  I get the dev_warn message that it
has changed the Bulk endpoint to Interrupt but that's it.

> For Windows, you note that you have to patch it.  Remember, this is a
> spec violation, you really shouldn't be doing this :)

I'm not wanting to use Windows.  Never again, if I can help it!

John
--
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 SUBSYSTEM - drivers/usb/core/config.c

[Greg KH]

On Tue, Feb 07, 2017 at 06:56:20PM +0100, John Skelton wrote:
> > From: "Felipe Balbi" 
> > > I'm particularly interested in drivers/usb/core/config.c
> > > which appears to enforce the USB specification by refusing to allow a
> > > low speed CDC ACM.  (Comment "Some buggy low-speed devices ...", at
> > > about line 300.)
> > >
> > > However, such devices exist and some are potentially quite useful (such
> > > as Arduinos & digistump).  Various people have posted about not being
> > > able to use them with Linux and I think the above file is the reason
> > > (another well known OS family allows them).
> >
> > Here's the comment:
> > 
> > /* Some buggy low-speed devices have Bulk endpoints, which is
> >  * explicitly forbidden by the USB spec.  In an attempt to make
> >  * them usable, we will try treating them as Interrupt endpoints.
> >  */
> > 
> > The code isn't forbidding CDC ACM low speed devices, it's forbidding
> > Bulk endpoints on Low speed, which don't exist :-) Are you saying there
> > are Bulk endpoints on low speed Arduinos?
> 
> Yes - or I may have misunderstood the situation.
> 
> It is code which is derived from V-USB
> https://www.obdev.at/products/vusb/index.html
> and is described at
> http://www.recursion.jp/prose/avrcdc/cdc-232.html
> 
> If it worked with Linux it would be a handy way to provide output (such as
> debug data) from low-cost Arduinos and such like via USB.
> 
> To be clear: I am not saying the Linux USB code is wrong.  But it would
> be good to have a way to have the above kind of code work, such that
> screen / minicom etc can be used with it.
> 

That page says that any kernel newer than 2.6.31 should work just fine,
have you tried this out and had it fail?

For Windows, you note that you have to patch it.  Remember, this is a
spec violation, you really shouldn't be doing this :)

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: USB SUBSYSTEM - drivers/usb/core/config.c

[John Skelton]

> From: "Greg KH" 
> On Tue, Feb 07, 2017 at 05:51:05PM +0100, John Skelton wrote:
> > Apologies if this ought to have gone to the USB maintainer (but his
> > auto-reply implies not).
> 
> This is the correct place.

Thanks.

> > I'm particularly interested in drivers/usb/core/config.c
> > which appears to enforce the USB specification by refusing to allow a
> > low speed CDC ACM.  (Comment "Some buggy low-speed devices ...", at
> > about line 300.)
> > 
> > However, such devices exist and some are potentially quite useful (such
> > as Arduinos & digistump).  Various people have posted about not being
> > able to use them with Linux and I think the above file is the reason
> > (another well known OS family allows them).
> As Felipe points out, this is to try to help some obviously broken
> devices to work properly.  We could take that code out and even more
> devices would stop working, but I think people would be upset at that :)
> 
> What device out there that is so obviously incorrect that still doesn't
> work even with the workaround that that the kernel is trying to do?

The code I stumbled on (trying to find a useful debug mechanism, really)
is at http://www.recursion.jp/prose/avrcdc/cdc-232.html

I'm open to any suggestions.  As I see it the above code is probably wrong
in trying to use a low speed Bulk endpoint.  The low performance of cheap
Arduinos and digistump etc (with no hardware USB) is not going to allow
anything but low speed (via software), however.

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

Re: [PATCH 1/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications

[David Miller]

From: Stefan Brüns 
Date: Mon, 6 Feb 2017 21:20:33 +0100

>  
>   /* Validate the protocol  - only support UMTS for now */
> - if (lsi->protocol != SIERRA_NET_PROTOCOL_UMTS) {
> + if (lsi->protocol == SIERRA_NET_PROTOCOL_UMTS) {
 ...
> + } else if (lsi->protocol == SIERRA_NET_PROTOCOL_UMTS_DS) {
> + expected_length = SIERRA_NET_LSI_UMTS_DS_STATUS_LEN;

If you are now supporting more protocols, that comment is not correct
any longer.  You therefore need to update it or remove it.

Thanks.
--
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 SUBSYSTEM - drivers/usb/core/config.c

[John Skelton]

> From: "Felipe Balbi" 
> > I'm particularly interested in drivers/usb/core/config.c
> > which appears to enforce the USB specification by refusing to allow a
> > low speed CDC ACM.  (Comment "Some buggy low-speed devices ...", at
> > about line 300.)
> >
> > However, such devices exist and some are potentially quite useful (such
> > as Arduinos & digistump).  Various people have posted about not being
> > able to use them with Linux and I think the above file is the reason
> > (another well known OS family allows them).
>
> Here's the comment:
> 
>   /* Some buggy low-speed devices have Bulk endpoints, which is
>* explicitly forbidden by the USB spec.  In an attempt to make
>* them usable, we will try treating them as Interrupt endpoints.
>*/
> 
> The code isn't forbidding CDC ACM low speed devices, it's forbidding
> Bulk endpoints on Low speed, which don't exist :-) Are you saying there
> are Bulk endpoints on low speed Arduinos?

Yes - or I may have misunderstood the situation.

It is code which is derived from V-USB
https://www.obdev.at/products/vusb/index.html
and is described at
http://www.recursion.jp/prose/avrcdc/cdc-232.html

If it worked with Linux it would be a handy way to provide output (such as
debug data) from low-cost Arduinos and such like via USB.

To be clear: I am not saying the Linux USB code is wrong.  But it would
be good to have a way to have the above kind of code work, such that
screen / minicom etc can be used with it.

John
--
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 SUBSYSTEM - drivers/usb/core/config.c

[Greg KH]

On Tue, Feb 07, 2017 at 05:51:05PM +0100, John Skelton wrote:
> Apologies if this ought to have gone to the USB maintainer (but his
> auto-reply implies not).

This is the correct place.

> I'm particularly interested in drivers/usb/core/config.c
> which appears to enforce the USB specification by refusing to allow a
> low speed CDC ACM.  (Comment "Some buggy low-speed devices ...", at
> about line 300.)
> 
> However, such devices exist and some are potentially quite useful (such
> as Arduinos & digistump).  Various people have posted about not being
> able to use them with Linux and I think the above file is the reason
> (another well known OS family allows them).
> 
> I'd like to ask if there's a sane (Linux) way to allow them or are you
> open to one being created?
> 
> If so, as I don't know the best way then if you have any ideas I'd
> certainly be pleased to hear them.  Some sort of quirk?
> 
> To make it easy for non-expert/novice Linux users to connect
> non-conforming USB devices and have them "just work" please consider
> a way that is either the default or easy, i.e. so they don't have to
> config/build a kernel.
> 
> I am a bit surprised that the code does what it currently does, as it
> basically makes such devices unusable so far as I can tell and may as
> well just reject them entirely.

As Felipe points out, this is to try to help some obviously broken
devices to work properly.  We could take that code out and even more
devices would stop working, but I think people would be upset at that :)

What device out there that is so obviously incorrect that still doesn't
work even with the workaround that that the kernel is trying to do?

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: USB SUBSYSTEM - drivers/usb/core/config.c

[Felipe Balbi]

Hi,

John Skelton  writes:
> Apologies if this ought to have gone to the USB maintainer (but his
> auto-reply implies not).
>
> I'm particularly interested in drivers/usb/core/config.c
> which appears to enforce the USB specification by refusing to allow a
> low speed CDC ACM.  (Comment "Some buggy low-speed devices ...", at
> about line 300.)
>
> However, such devices exist and some are potentially quite useful (such
> as Arduinos & digistump).  Various people have posted about not being
> able to use them with Linux and I think the above file is the reason
> (another well known OS family allows them).
>
> I'd like to ask if there's a sane (Linux) way to allow them or are you
> open to one being created?
>
> If so, as I don't know the best way then if you have any ideas I'd
> certainly be pleased to hear them.  Some sort of quirk?
>
> To make it easy for non-expert/novice Linux users to connect
> non-conforming USB devices and have them "just work" please consider
> a way that is either the default or easy, i.e. so they don't have to
> config/build a kernel.
>
> I am a bit surprised that the code does what it currently does, as it
> basically makes such devices unusable so far as I can tell and may as
> well just reject them entirely.

Here's the comment:

/* Some buggy low-speed devices have Bulk endpoints, which is
 * explicitly forbidden by the USB spec.  In an attempt to make
 * them usable, we will try treating them as Interrupt endpoints.
 */

The code isn't forbidding CDC ACM low speed devices, it's forbidding
Bulk endpoints on Low speed, which don't exist :-) Are you saying there
are Bulk endpoints on low speed Arduinos?

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

USB SUBSYSTEM - drivers/usb/core/config.c

[John Skelton]

Apologies if this ought to have gone to the USB maintainer (but his
auto-reply implies not).

I'm particularly interested in drivers/usb/core/config.c
which appears to enforce the USB specification by refusing to allow a
low speed CDC ACM.  (Comment "Some buggy low-speed devices ...", at
about line 300.)

However, such devices exist and some are potentially quite useful (such
as Arduinos & digistump).  Various people have posted about not being
able to use them with Linux and I think the above file is the reason
(another well known OS family allows them).

I'd like to ask if there's a sane (Linux) way to allow them or are you
open to one being created?

If so, as I don't know the best way then if you have any ideas I'd
certainly be pleased to hear them.  Some sort of quirk?

To make it easy for non-expert/novice Linux users to connect
non-conforming USB devices and have them "just work" please consider
a way that is either the default or easy, i.e. so they don't have to
config/build a kernel.

I am a bit surprised that the code does what it currently does, as it
basically makes such devices unusable so far as I can tell and may as
well just reject them entirely.

Thanks,

John
--
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 -next] usb: dwc2: Fix the error handling of dwc2_pci_probe()

[Wei Yongjun]

From: Wei Yongjun 

Fix the error handling of dwc2_pci_probe() to avoid resources leak.

Signed-off-by: Wei Yongjun 
---
 drivers/usb/dwc2/pci.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index fdeb8c7..58fc1bf 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -124,7 +124,7 @@ static int dwc2_pci_probe(struct pci_dev *pci,
ret = platform_device_add_resources(dwc2, res, ARRAY_SIZE(res));
if (ret) {
dev_err(dev, "couldn't add resources to dwc2 device\n");
-   return ret;
+   goto err_device_put;
}
 
dwc2->dev.parent = dev;
@@ -133,30 +133,37 @@ static int dwc2_pci_probe(struct pci_dev *pci,
if (IS_ERR(phy)) {
dev_err(dev, "error registering generic PHY (%ld)\n",
PTR_ERR(phy));
-   return PTR_ERR(phy);
+   ret = PTR_ERR(phy);
+   goto err_device_put;
}
 
ret = dwc2_pci_quirks(pci, dwc2);
if (ret)
-   goto err;
+   goto err_unregister;
 
ret = platform_device_add(dwc2);
if (ret) {
dev_err(dev, "failed to register dwc2 device\n");
-   goto err;
+   goto err_unregister;
}
 
glue = kzalloc(sizeof(*glue), GFP_KERNEL);
-   if (!glue)
-   return -ENOMEM;
+   if (!glue) {
+   ret = -ENOMEM;
+   goto err_device_del;
+   }
 
glue->phy = phy;
glue->dwc2 = dwc2;
pci_set_drvdata(pci, glue);
 
return 0;
-err:
+
+err_device_del:
+   platform_device_del(dwc2);
+err_unregister:
usb_phy_generic_unregister(phy);
+err_device_put:
platform_device_put(dwc2);
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

Re: Typo in "drivers/usb/host/ohci-hub.c"

[Alan Stern]

On Tue, 7 Feb 2017, Jelle Martijn Kok wrote:

> I actually stumbled on the next (quite unimportant) typo.
> 
> In "ohci-hub.c" there is "dbg_port" macro which uses the "outside" 
> parameter (="temp") instead of the parameters (="value") given in the macro.
> 
> This actually poses no problem as it is a macro and not a function.
> 
> Below is the diff.

This is a valid correction.  Please resubmit it, following the
procedure described in Documentation/SubmittingPatches, and be sure to
CC: Greg KH  and the USB list
. You can add

Acked-by: Alan Stern 

to the submission.

Alan Stern

> Best Regards,
> 
> Jelle Martijn Kok
> 
> $ git diff drivers/usb/host/ohci-hub.c
> diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
> index ed678c17..248eb77 100644
> --- a/drivers/usb/host/ohci-hub.c
> +++ b/drivers/usb/host/ohci-hub.c
> @@ -17,21 +17,21 @@
>  ohci_dbg (hc, \
>  "%s roothub.portstatus [%d] " \
>  "= 0x%08x%s%s%s%s%s%s%s%s%s%s%s%s\n", \
> -   label, num, temp, \
> -   (temp & RH_PS_PRSC) ? " PRSC" : "", \
> -   (temp & RH_PS_OCIC) ? " OCIC" : "", \
> -   (temp & RH_PS_PSSC) ? " PSSC" : "", \
> -   (temp & RH_PS_PESC) ? " PESC" : "", \
> -   (temp & RH_PS_CSC) ? " CSC" : "", \
> +   label, num, value, \
> +   (value & RH_PS_PRSC) ? " PRSC" : "", \
> +   (value & RH_PS_OCIC) ? " OCIC" : "", \
> +   (value & RH_PS_PSSC) ? " PSSC" : "", \
> +   (value & RH_PS_PESC) ? " PESC" : "", \
> +   (value & RH_PS_CSC) ? " CSC" : "", \
>  \
> -   (temp & RH_PS_LSDA) ? " LSDA" : "", \
> -   (temp & RH_PS_PPS) ? " PPS" : "", \
> -   (temp & RH_PS_PRS) ? " PRS" : "", \
> -   (temp & RH_PS_POCI) ? " POCI" : "", \
> -   (temp & RH_PS_PSS) ? " PSS" : "", \
> +   (value & RH_PS_LSDA) ? " LSDA" : "", \
> +   (value & RH_PS_PPS) ? " PPS" : "", \
> +   (value & RH_PS_PRS) ? " PRS" : "", \
> +   (value & RH_PS_POCI) ? " POCI" : "", \
> +   (value & RH_PS_PSS) ? " PSS" : "", \
>  \
> -   (temp & RH_PS_PES) ? " PES" : "", \
> -   (temp & RH_PS_CCS) ? " CCS" : "" \
> +   (value & RH_PS_PES) ? " PES" : "", \
> +   (value & RH_PS_CCS) ? " CCS" : "" \
>  );
> 
>   
> /*-*/
> 
> 
> 

--
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 resend v5 0/4] xhci fixes for usb-linus

[Alan Stern]

On Tue, 7 Feb 2017, Peter Chen wrote:

> On Thu, Jan 19, 2017 at 02:21:26PM +0200, Mathias Nyman wrote:
> > Hi Greg
> > 
> > This series by Arnd Bergmann was originally six patches, but last two of
> > them were already taken to 4.10. Without the rest of them there will
> > be a regression in 4.10.
> > 
> > Original cover letter says:
> > 
> > For xhci-hcd platform device, all the DMA parameters are not
> > configured properly, notably dma ops for dwc3 devices.
> > 
> > The idea here is that you pass in the parent of_node along
> > with the child device pointer, so it would behave exactly
> > like the parent already does. The difference is that it also
> > handles all the other attributes besides the mask.
> > 
> > -Mathias
> > 
> > Arnd Bergmann (4):
> >   usb: separate out sysdev pointer from usb_bus
> >   usb: chipidea: use bus->sysdev for DMA configuration
> >   usb: ehci: fsl: use bus->sysdev for DMA configuration
> >   usb: xhci: use bus->sysdev for DMA configuration
> 
> The ehci part needs to similar change, otherwise, it will fail too.
> I post a patch for it [1]. Alan, would you please help to review it?
> Then, Greg can pick up the five patches together.
> 
> [1] http://www.spinics.net/lists/linux-usb/msg153263.html

That patch contains a lot of unrelated whitespace changes.  Can you 
redo it with only the necessary changes?

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 net 0/4] Fix on-stack USB buffers

[David Miller]

From: Ben Hutchings 
Date: Sat, 4 Feb 2017 16:54:51 +

> Allocating USB buffers on the stack is not portable, and no longer
> works on x86_64 (with VMAP_STACK enabled as per default).  This
> series fixes all the instances I could find where USB networking
> drivers do that.

Series applied and queued up for -stable, thanks Ben.
--
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 2/4] rtl8150: Use heap buffers for all register access

[Petko Manolov]

On 17-02-07 14:14:30, David Laight wrote:
> From: Petko Manolov
> > Sent: 07 February 2017 13:21
> ...
> > > > Would you consider what David proposed (usb_control_msg_with_malloc()) 
> > > > for 4.11,
> > > > for example?  I for one will use something like that in all my drivers.
> > >
> > > Sure, but you might want to make it a bit smaller of a function name :)
> > 
> > Whatever i say may be taken as volunteering. :D
> > 
> > Second - i've got really bad taste when naming things.  asdfgl() is a short 
> > name
> > although not very descriptive. ;)
> ...
> 
> Actually, for short control transfers a field in the urb itself could be used 
> (unless another 8 bytes makes the structure larger).
> 
> IIRC for xhci, the 8 byte pointer field can be marked as containing the 
> actual 
> data - saving the hardware from doing another dma as well.

Unless set up for automatic triggering, 8 byte DMA transfer is an overkill (by 
means of having to program a bunch of IO registers), especially on a 64bit 
machine.

However, not knowing the USB host controllers specifics i'll shut up.


cheers,
Petko
--
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 2/4] rtl8150: Use heap buffers for all register access

[David Laight]

From: Petko Manolov
> Sent: 07 February 2017 13:21
...
> > > Would you consider what David proposed (usb_control_msg_with_malloc()) 
> > > for 4.11,
> > > for example?  I for one will use something like that in all my drivers.
> >
> > Sure, but you might want to make it a bit smaller of a function name :)
> 
> Whatever i say may be taken as volunteering. :D
> 
> Second - i've got really bad taste when naming things.  asdfgl() is a short 
> name
> although not very descriptive. ;)
...

Actually, for short control transfers a field in the urb itself could be used
(unless another 8 bytes makes the structure larger).

IIRC for xhci, the 8 byte pointer field can be marked as containing the
actual data - saving the hardware from doing another dma as well.

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 net 2/4] rtl8150: Use heap buffers for all register access

[Petko Manolov]

On 17-02-07 14:01:02, Greg KH wrote:
> On Tue, Feb 07, 2017 at 02:53:24PM +0200, Petko Manolov wrote:
> > On 17-02-07 11:51:31, Greg KH wrote:
> > > On Tue, Feb 07, 2017 at 12:34:52PM +0200, Petko Manolov wrote:
> > > > On 17-02-06 16:25:20, Ben Hutchings wrote:
> > > > > On Mon, Feb 06, 2017 at 04:09:18PM +, David Laight wrote:
> > > > > > From: Ben Hutchings
> > > > > [...]
> > > > > > > + ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > > > > > > +   RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
> > > > > > > +   indx, 0, buf, size, 500);
> > > > > > > + if (ret > 0 && ret <= size)
> > > > > > > + memcpy(data, buf, ret);
> > > > > > 
> > > > > > If ret > size something is horridly wrong.
> > > > > > Silently not updating the callers buffer at all cannot be right.
> > > > > 
> > > > > Yes, it seems strange to check this.  I originally wrote this as ret >
> > > > > 0, but then I checked the usbnet core and found __usbnet_read_cmd()
> > > > > has the second comparison as well.
> > > > > 
> > > > > > > + kfree(buf);
> > > > > > > + return ret;
> > > > 
> > > > Since we return what usb_control_msg() told us to return i assume the 
> > > > error code 
> > > > will be available to anybody who cares.
> > > > 
> > > > > > I can't help feeling that it would be better to add a wrapper to
> > > > > > usb_control_msg() that does the kmalloc() and memcpy()s and
> > > > > > drop that into all the call sites.
> > > > > 
> > > > > It might be.  Right now I'm trying to patch up a bunch of regressions 
> > > > > rather 
> > > > > than argue over an API change.
> > > > 
> > > > Right, first thing first.
> > > > 
> > > > I am in favor of changing the API, but this should not happen in the 
> > > > stable 
> > > > releases.  I hope Greg will make up his mind and let us know.
> > > 
> > > make up my mind about what?  These are bugs, they should be fixed, I'm 
> > > not 
> > > taking a total api change at this point in time, sorry.
> > 
> > Exactly what i said above: " ... shoud not happen in the stable releases".  
> > 
> > Would you consider what David proposed (usb_control_msg_with_malloc()) for 
> > 4.11, 
> > for example?  I for one will use something like that in all my drivers.
> 
> Sure, but you might want to make it a bit smaller of a function name :)

Whatever i say may be taken as volunteering. :D

Second - i've got really bad taste when naming things.  asdfgl() is a short 
name 
although not very descriptive. ;)


Seriously, if nobody else step up i'll do my best to come up with a patch in 
the 
next few days.



cheers,
Petko
--
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 2/4] rtl8150: Use heap buffers for all register access

[Greg KH]

On Tue, Feb 07, 2017 at 02:53:24PM +0200, Petko Manolov wrote:
> On 17-02-07 11:51:31, Greg KH wrote:
> > On Tue, Feb 07, 2017 at 12:34:52PM +0200, Petko Manolov wrote:
> > > On 17-02-06 16:25:20, Ben Hutchings wrote:
> > > > On Mon, Feb 06, 2017 at 04:09:18PM +, David Laight wrote:
> > > > > From: Ben Hutchings
> > > > [...]
> > > > > > +   ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > > > > > + RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
> > > > > > + indx, 0, buf, size, 500);
> > > > > > +   if (ret > 0 && ret <= size)
> > > > > > +   memcpy(data, buf, ret);
> > > > > 
> > > > > If ret > size something is horridly wrong.
> > > > > Silently not updating the callers buffer at all cannot be right.
> > > > 
> > > > Yes, it seems strange to check this.  I originally wrote this as ret >
> > > > 0, but then I checked the usbnet core and found __usbnet_read_cmd()
> > > > has the second comparison as well.
> > > > 
> > > > > > +   kfree(buf);
> > > > > > +   return ret;
> > > 
> > > Since we return what usb_control_msg() told us to return i assume the 
> > > error code 
> > > will be available to anybody who cares.
> > > 
> > > > > I can't help feeling that it would be better to add a wrapper to
> > > > > usb_control_msg() that does the kmalloc() and memcpy()s and
> > > > > drop that into all the call sites.
> > > > 
> > > > It might be.  Right now I'm trying to patch up a bunch of regressions 
> > > > rather 
> > > > than argue over an API change.
> > > 
> > > Right, first thing first.
> > > 
> > > I am in favor of changing the API, but this should not happen in the 
> > > stable 
> > > releases.  I hope Greg will make up his mind and let us know.
> > 
> > make up my mind about what?  These are bugs, they should be fixed, I'm not 
> > taking a total api change at this point in time, sorry.
> 
> Exactly what i said above: " ... shoud not happen in the stable releases".  
> 
> Would you consider what David proposed (usb_control_msg_with_malloc()) for 
> 4.11, 
> for example?  I for one will use something like that in all my drivers.

Sure, but you might want to make it a bit smaller of a function name :)
--
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 2/4] rtl8150: Use heap buffers for all register access

[Petko Manolov]

On 17-02-07 11:51:31, Greg KH wrote:
> On Tue, Feb 07, 2017 at 12:34:52PM +0200, Petko Manolov wrote:
> > On 17-02-06 16:25:20, Ben Hutchings wrote:
> > > On Mon, Feb 06, 2017 at 04:09:18PM +, David Laight wrote:
> > > > From: Ben Hutchings
> > > [...]
> > > > > + ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > > > > +   RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
> > > > > +   indx, 0, buf, size, 500);
> > > > > + if (ret > 0 && ret <= size)
> > > > > + memcpy(data, buf, ret);
> > > > 
> > > > If ret > size something is horridly wrong.
> > > > Silently not updating the callers buffer at all cannot be right.
> > > 
> > > Yes, it seems strange to check this.  I originally wrote this as ret >
> > > 0, but then I checked the usbnet core and found __usbnet_read_cmd()
> > > has the second comparison as well.
> > > 
> > > > > + kfree(buf);
> > > > > + return ret;
> > 
> > Since we return what usb_control_msg() told us to return i assume the error 
> > code 
> > will be available to anybody who cares.
> > 
> > > > I can't help feeling that it would be better to add a wrapper to
> > > > usb_control_msg() that does the kmalloc() and memcpy()s and
> > > > drop that into all the call sites.
> > > 
> > > It might be.  Right now I'm trying to patch up a bunch of regressions 
> > > rather 
> > > than argue over an API change.
> > 
> > Right, first thing first.
> > 
> > I am in favor of changing the API, but this should not happen in the stable 
> > releases.  I hope Greg will make up his mind and let us know.
> 
> make up my mind about what?  These are bugs, they should be fixed, I'm not 
> taking a total api change at this point in time, sorry.

Exactly what i said above: " ... shoud not happen in the stable releases".  

Would you consider what David proposed (usb_control_msg_with_malloc()) for 
4.11, 
for example?  I for one will use something like that in all my drivers.


cheers,
Petko
--
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 1/4] pegasus: Use heap buffers for all register access

[Petko Manolov]

On 17-02-07 11:45:06, Greg KH wrote:
> On Tue, Feb 07, 2017 at 12:24:12PM +0200, Petko Manolov wrote:
> > On 17-02-06 14:46:21, Johan Hovold wrote:
> > > On Mon, Feb 06, 2017 at 02:32:23PM +0100, Johan Hovold wrote:
> > > > On Mon, Feb 06, 2017 at 02:21:24PM +0100, Johan Hovold wrote:
> > > > > On Mon, Feb 06, 2017 at 02:51:09PM +0200, Petko Manolov wrote:
> > > > > > On 17-02-06 09:28:22, Greg KH wrote:
> > > > > > > On Mon, Feb 06, 2017 at 10:14:44AM +0200, Petko Manolov wrote:
> > > > > 
> > > > > > > > Random thought: isn't it better to add the alloc/free code in 
> > > > > > > > usb_control_msg() and avoid code duplication all over the 
> > > > > > > > driver space?
> > > > > > > 
> > > > > > > A very long time ago we considered it, but realized that the 
> > > > > > > majority of 
> > > > > > > drivers already had the memory dynamically allocated, so we just 
> > > > > > > went with 
> > > > > > > this.  Perhaps we could revisit that if it turns out we were 
> > > > > > > wrong, and would 
> > > > > > > simplify things.
> > > > > > 
> > > > > > A quick glance at usb_control_msg() users (looked only at 
> > > > > > .../net/usb and 
> > > > > > .../usb/serial) shows that most of them have the buffer allocated 
> > > > > > in stack.  
> > > > > 
> > > > > That's simply not true at all for usb-serial. If you find an instance
> > > > > were a stack allocated buffer is used for DMA that hasn't yet been
> > > > > fixed in USB serial, then please point it out so we can fix it up (I
> > > > > doubt you'll find one, though).
> > > > 
> > > > Heh, I just found one myself (in ark3116) that I'll fix up. Please let
> > > > me know if you find any more.
> > > 
> > > False alarm, the ark3116 driver is fine too.
> > 
> > You may want to check pl2303_vendor_read() in drivers/usb/serial/pl2303.c
> > It seems to me that 'buf' is allocated in the stack. ;)
> 
> Really?  Doesn't look like it to me, it's passed in from the caller and it's 
> allocated in that caller.  What am I missing here?

Sorry, got confused by the weird pointer definition of the last argument:

static int pl2303_vendor_read(struct usb_serial *serial, u16 value, unsigned 
char buf[1])


cheers,
Petko
--
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 2/4] rtl8150: Use heap buffers for all register access

['Greg KH']

On Tue, Feb 07, 2017 at 11:56:51AM +, David Laight wrote:
> From: Greg KH
> > Sent: 07 February 2017 10:52
> > To: Petko Manolov
> > Cc: Ben Hutchings; David Laight; net...@vger.kernel.org; 
> > linux-usb@vger.kernel.org
> > Subject: Re: [PATCH net 2/4] rtl8150: Use heap buffers for all register 
> > access
> > 
> > On Tue, Feb 07, 2017 at 12:34:52PM +0200, Petko Manolov wrote:
> > > On 17-02-06 16:25:20, Ben Hutchings wrote:
> > > > On Mon, Feb 06, 2017 at 04:09:18PM +, David Laight wrote:
> > > > > From: Ben Hutchings
> > > > [...]
> > > > > > +   ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > > > > > + RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
> > > > > > + indx, 0, buf, size, 500);
> > > > > > +   if (ret > 0 && ret <= size)
> > > > > > +   memcpy(data, buf, ret);
> > > > >
> > > > > If ret > size something is horridly wrong.
> > > > > Silently not updating the callers buffer at all cannot be right.
> > > >
> > > > Yes, it seems strange to check this.  I originally wrote this as ret >
> > > > 0, but then I checked the usbnet core and found __usbnet_read_cmd()
> > > > has the second comparison as well.
> > > >
> > > > > > +   kfree(buf);
> > > > > > +   return ret;
> > >
> > > Since we return what usb_control_msg() told us to return i assume the 
> > > error code
> > > will be available to anybody who cares.
> > >
> > > > > I can't help feeling that it would be better to add a wrapper to
> > > > > usb_control_msg() that does the kmalloc() and memcpy()s and
> > > > > drop that into all the call sites.
> > > >
> > > > It might be.  Right now I'm trying to patch up a bunch of regressions 
> > > > rather
> > > > than argue over an API change.
> > >
> > > Right, first thing first.
> > >
> > > I am in favor of changing the API, but this should not happen in the 
> > > stable
> > > releases.  I hope Greg will make up his mind and let us know.
> > 
> > make up my mind about what?  These are bugs, they should be fixed, I'm
> > not taking a total api change at this point in time, sorry.
> 
> Adding a usb_control_msg_with_malloc() wrapper is a smaller change than those
> proposed. The smaller churn probably makes back porting other changes easier.
> 
> Given the nature of this problem, and how common it seems to be,
> it is almost worth renaming usb_control_msg() itself so that all the
> callers are properly audited.

As this is something that we have been auditing for a decade now, I
don't think you will find all that many instances :)

But for now, fixes like this are fine, if someone wants to tackle the
larger issue here, with a new api function, that would be great.

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 net 2/4] rtl8150: Use heap buffers for all register access

[David Laight]

From: Greg KH
> Sent: 07 February 2017 10:52
> To: Petko Manolov
> Cc: Ben Hutchings; David Laight; net...@vger.kernel.org; 
> linux-usb@vger.kernel.org
> Subject: Re: [PATCH net 2/4] rtl8150: Use heap buffers for all register access
> 
> On Tue, Feb 07, 2017 at 12:34:52PM +0200, Petko Manolov wrote:
> > On 17-02-06 16:25:20, Ben Hutchings wrote:
> > > On Mon, Feb 06, 2017 at 04:09:18PM +, David Laight wrote:
> > > > From: Ben Hutchings
> > > [...]
> > > > > + ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > > > > +   RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
> > > > > +   indx, 0, buf, size, 500);
> > > > > + if (ret > 0 && ret <= size)
> > > > > + memcpy(data, buf, ret);
> > > >
> > > > If ret > size something is horridly wrong.
> > > > Silently not updating the callers buffer at all cannot be right.
> > >
> > > Yes, it seems strange to check this.  I originally wrote this as ret >
> > > 0, but then I checked the usbnet core and found __usbnet_read_cmd()
> > > has the second comparison as well.
> > >
> > > > > + kfree(buf);
> > > > > + return ret;
> >
> > Since we return what usb_control_msg() told us to return i assume the error 
> > code
> > will be available to anybody who cares.
> >
> > > > I can't help feeling that it would be better to add a wrapper to
> > > > usb_control_msg() that does the kmalloc() and memcpy()s and
> > > > drop that into all the call sites.
> > >
> > > It might be.  Right now I'm trying to patch up a bunch of regressions 
> > > rather
> > > than argue over an API change.
> >
> > Right, first thing first.
> >
> > I am in favor of changing the API, but this should not happen in the stable
> > releases.  I hope Greg will make up his mind and let us know.
> 
> make up my mind about what?  These are bugs, they should be fixed, I'm
> not taking a total api change at this point in time, sorry.

Adding a usb_control_msg_with_malloc() wrapper is a smaller change than those
proposed. The smaller churn probably makes back porting other changes easier.

Given the nature of this problem, and how common it seems to be,
it is almost worth renaming usb_control_msg() itself so that all the
callers are properly audited.

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 net 2/4] rtl8150: Use heap buffers for all register access

[Greg KH]

On Tue, Feb 07, 2017 at 12:34:52PM +0200, Petko Manolov wrote:
> On 17-02-06 16:25:20, Ben Hutchings wrote:
> > On Mon, Feb 06, 2017 at 04:09:18PM +, David Laight wrote:
> > > From: Ben Hutchings
> > [...]
> > > > +   ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > > > + RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
> > > > + indx, 0, buf, size, 500);
> > > > +   if (ret > 0 && ret <= size)
> > > > +   memcpy(data, buf, ret);
> > > 
> > > If ret > size something is horridly wrong.
> > > Silently not updating the callers buffer at all cannot be right.
> > 
> > Yes, it seems strange to check this.  I originally wrote this as ret >
> > 0, but then I checked the usbnet core and found __usbnet_read_cmd()
> > has the second comparison as well.
> > 
> > > > +   kfree(buf);
> > > > +   return ret;
> 
> Since we return what usb_control_msg() told us to return i assume the error 
> code 
> will be available to anybody who cares.
> 
> > > I can't help feeling that it would be better to add a wrapper to
> > > usb_control_msg() that does the kmalloc() and memcpy()s and
> > > drop that into all the call sites.
> > 
> > It might be.  Right now I'm trying to patch up a bunch of regressions 
> > rather 
> > than argue over an API change.
> 
> Right, first thing first.
> 
> I am in favor of changing the API, but this should not happen in the stable 
> releases.  I hope Greg will make up his mind and let us know.

make up my mind about what?  These are bugs, they should be fixed, I'm
not taking a total api change at this point in time, sorry.

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 net 1/4] pegasus: Use heap buffers for all register access

[Greg KH]

On Tue, Feb 07, 2017 at 12:24:12PM +0200, Petko Manolov wrote:
> On 17-02-06 14:46:21, Johan Hovold wrote:
> > On Mon, Feb 06, 2017 at 02:32:23PM +0100, Johan Hovold wrote:
> > > On Mon, Feb 06, 2017 at 02:21:24PM +0100, Johan Hovold wrote:
> > > > On Mon, Feb 06, 2017 at 02:51:09PM +0200, Petko Manolov wrote:
> > > > > On 17-02-06 09:28:22, Greg KH wrote:
> > > > > > On Mon, Feb 06, 2017 at 10:14:44AM +0200, Petko Manolov wrote:
> > > > 
> > > > > > > Random thought: isn't it better to add the alloc/free code in 
> > > > > > > usb_control_msg() and avoid code duplication all over the driver 
> > > > > > > space?
> > > > > > 
> > > > > > A very long time ago we considered it, but realized that the 
> > > > > > majority of 
> > > > > > drivers already had the memory dynamically allocated, so we just 
> > > > > > went with 
> > > > > > this.  Perhaps we could revisit that if it turns out we were wrong, 
> > > > > > and would 
> > > > > > simplify things.
> > > > > 
> > > > > A quick glance at usb_control_msg() users (looked only at .../net/usb 
> > > > > and 
> > > > > .../usb/serial) shows that most of them have the buffer allocated in 
> > > > > stack.  
> > > > 
> > > > That's simply not true at all for usb-serial. If you find an instance
> > > > were a stack allocated buffer is used for DMA that hasn't yet been
> > > > fixed in USB serial, then please point it out so we can fix it up (I
> > > > doubt you'll find one, though).
> > > 
> > > Heh, I just found one myself (in ark3116) that I'll fix up. Please let
> > > me know if you find any more.
> > 
> > False alarm, the ark3116 driver is fine too.
> 
> You may want to check pl2303_vendor_read() in drivers/usb/serial/pl2303.c
> It seems to me that 'buf' is allocated in the stack. ;)

Really?  Doesn't look like it to me, it's passed in from the caller and
it's allocated in that caller.  What am I missing here?

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 net 2/4] rtl8150: Use heap buffers for all register access

[Petko Manolov]

On 17-02-06 16:25:20, Ben Hutchings wrote:
> On Mon, Feb 06, 2017 at 04:09:18PM +, David Laight wrote:
> > From: Ben Hutchings
> [...]
> > > + ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > > +   RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
> > > +   indx, 0, buf, size, 500);
> > > + if (ret > 0 && ret <= size)
> > > + memcpy(data, buf, ret);
> > 
> > If ret > size something is horridly wrong.
> > Silently not updating the callers buffer at all cannot be right.
> 
> Yes, it seems strange to check this.  I originally wrote this as ret >
> 0, but then I checked the usbnet core and found __usbnet_read_cmd()
> has the second comparison as well.
> 
> > > + kfree(buf);
> > > + return ret;

Since we return what usb_control_msg() told us to return i assume the error 
code 
will be available to anybody who cares.

> > I can't help feeling that it would be better to add a wrapper to
> > usb_control_msg() that does the kmalloc() and memcpy()s and
> > drop that into all the call sites.
> 
> It might be.  Right now I'm trying to patch up a bunch of regressions rather 
> than argue over an API change.

Right, first thing first.

I am in favor of changing the API, but this should not happen in the stable 
releases.  I hope Greg will make up his mind and let us know.


cheers,
Petko
--
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 1/4] pegasus: Use heap buffers for all register access

[Petko Manolov]

On 17-02-06 14:46:21, Johan Hovold wrote:
> On Mon, Feb 06, 2017 at 02:32:23PM +0100, Johan Hovold wrote:
> > On Mon, Feb 06, 2017 at 02:21:24PM +0100, Johan Hovold wrote:
> > > On Mon, Feb 06, 2017 at 02:51:09PM +0200, Petko Manolov wrote:
> > > > On 17-02-06 09:28:22, Greg KH wrote:
> > > > > On Mon, Feb 06, 2017 at 10:14:44AM +0200, Petko Manolov wrote:
> > > 
> > > > > > Random thought: isn't it better to add the alloc/free code in 
> > > > > > usb_control_msg() and avoid code duplication all over the driver 
> > > > > > space?
> > > > > 
> > > > > A very long time ago we considered it, but realized that the majority 
> > > > > of 
> > > > > drivers already had the memory dynamically allocated, so we just went 
> > > > > with 
> > > > > this.  Perhaps we could revisit that if it turns out we were wrong, 
> > > > > and would 
> > > > > simplify things.
> > > > 
> > > > A quick glance at usb_control_msg() users (looked only at .../net/usb 
> > > > and 
> > > > .../usb/serial) shows that most of them have the buffer allocated in 
> > > > stack.  
> > > 
> > > That's simply not true at all for usb-serial. If you find an instance
> > > were a stack allocated buffer is used for DMA that hasn't yet been
> > > fixed in USB serial, then please point it out so we can fix it up (I
> > > doubt you'll find one, though).
> > 
> > Heh, I just found one myself (in ark3116) that I'll fix up. Please let
> > me know if you find any more.
> 
> False alarm, the ark3116 driver is fine too.

You may want to check pl2303_vendor_read() in drivers/usb/serial/pl2303.c
It seems to me that 'buf' is allocated in the stack. ;)


Petko
--
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: [RESEND PATCH 1/1] usb: dwc2: add multiple clock handling

[John Youn]

On 2/6/2017 7:18 PM, Frank Wang wrote:
> Hi Heiko, John and Greg,
>
> On 2017/2/7 8:06, Heiko Stuebner wrote:
>> Hi Frank,
>>
>> Am Sonntag, 5. Februar 2017, 10:51:01 CET schrieb Frank Wang:
>>> Originally, dwc2 just handle one clock named otg, however, it may have
>>> two or more clock need to manage for some new SoCs, so this adds
>>> change clk to clk's array of dwc2_hsotg to handle more clocks operation.
>>>
>>> Signed-off-by: Frank Wang 
>>> ---
>>>   drivers/usb/dwc2/core.h |  5 -
>>>   drivers/usb/dwc2/platform.c | 39 ++-
>>>   2 files changed, 30 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>>> index 1a7e830..d10a466 100644
>>> --- a/drivers/usb/dwc2/core.h
>>> +++ b/drivers/usb/dwc2/core.h
>>> @@ -121,6 +121,9 @@ static inline void dwc2_writel(u32 value, void __iomem
>>> *addr) /* Maximum number of Endpoints/HostChannels */
>>>   #define MAX_EPS_CHANNELS  16
>>>
>>> +/* Maximum number of dwc2 clocks */
>>> +#define DWC2_MAX_CLKS 3
>> why 3 clocks?
>
> Showing from the chapter 2.4 of dwc otg databook v3.10, it seems there
> should be five clocks, am I right?
> hclk/pmu_hclk/utmi_clk/ulpi_clk/utmifs_clk48
>
>> I.e. the binding currently only specifies the "otg" clock, so you should
>> definitly amend it to also specifiy this "pmu" clock - in a separate patch
>> before this one of course :-) .
>> "pmu" also looks like a good name for that clock-binding and these new clocks
>> of course should be optional in the binding.
>
> No problem, I will amend the binding when the implementation scheme is
> clear.
>
>> And ideally also just specify this mysterious third clock as well while 
>> you're
>> at it.
>>
>>> +
>>>   /* dwc2-hsotg declarations */
>>>   static const char * const dwc2_hsotg_supply_names[] = {
>>> "vusb_d",   /* digital USB supply, 1.2V */
>>> @@ -913,7 +916,7 @@ struct dwc2_hsotg {
>>> spinlock_t lock;
>>> void *priv;
>>> int irq;
>>> -   struct clk *clk;
>>> +   struct clk *clks[DWC2_MAX_CLKS];
>>> struct reset_control *reset;
>>>
>>> unsigned int queuing_high_bandwidth:1;
>> [...]
>>
>>> @@ -123,17 +123,20 @@ static int dwc2_get_dr_mode(struct dwc2_hsotg *hsotg)
>>>   static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
>>>   {
>>> struct platform_device *pdev = to_platform_device(hsotg->dev);
>>> -   int ret;
>>> +   int clk, ret;
>>>
>>> ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
>>> hsotg->supplies);
>>> if (ret)
>>> return ret;
>>>
>>> -   if (hsotg->clk) {
>>> -   ret = clk_prepare_enable(hsotg->clk);
>>> -   if (ret)
>>> +   for (clk = 0; clk < DWC2_MAX_CLKS && hsotg->clks[clk]; clk++) {
>>> +   ret = clk_prepare_enable(hsotg->clks[clk]);
>>> +   if (ret) {
>>> +   while (--clk >= 0)
>>> +   clk_disable_unprepare(hsotg->clks[clk]);
>>> return ret;
>>> +   }
>>> }
>>>
>>> if (hsotg->uphy) {
>>> @@ -168,7 +171,7 @@ int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
>>>   static int __dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg)
>>>   {
>>> struct platform_device *pdev = to_platform_device(hsotg->dev);
>>> -   int ret = 0;
>>> +   int clk, ret = 0;
>>>
>>> if (hsotg->uphy) {
>>> usb_phy_shutdown(hsotg->uphy);
>>> @@ -182,8 +185,9 @@ static int __dwc2_lowlevel_hw_disable(struct dwc2_hsotg
>>> *hsotg) if (ret)
>>> return ret;
>>>
>>> -   if (hsotg->clk)
>>> -   clk_disable_unprepare(hsotg->clk);
>>> +   for (clk = DWC2_MAX_CLKS - 1; clk >= 0; clk--)
>>> +   if (hsotg->clks[clk])
>>> +   clk_disable_unprepare(hsotg->clks[clk]);
>>> ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
>>>  hsotg->supplies);
>>> @@ -209,7 +213,7 @@ int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg)
>>>
>>>   static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
>>>   {
>>> -   int i, ret;
>>> +   int i, clk, ret;
>>>
>>> hsotg->reset = devm_reset_control_get_optional(hsotg->dev, "dwc2");
>>> if (IS_ERR(hsotg->reset)) {
>>> @@ -282,11 +286,20 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg
>>> *hsotg) hsotg->phyif = GUSBCFG_PHYIF8;
>>> }
>>>
>>> -   /* Clock */
>>> -   hsotg->clk = devm_clk_get(hsotg->dev, "otg");
>>> -   if (IS_ERR(hsotg->clk)) {
>>> -   hsotg->clk = NULL;
>>> -   dev_dbg(hsotg->dev, "cannot get otg clock\n");
>>> +   /* Clocks */
>>> +   for (clk = 0; clk < DWC2_MAX_CLKS; clk++) {
>>> +   hsotg->clks[clk] = of_clk_get(hsotg->dev->of_node, clk);
>>> +   if (IS_ERR(hsotg->clks[clk])) {
>>> +   ret = PTR_ERR(hsotg->clks[clk]);
>>> +   if (ret == -EPROBE_DEFER) {
>>> +   while (--clk >= 0)
>>> +

Re: [PATCH] ALSA: line6: Always setup isochronous transfer properties

[Takashi Iwai]

On Mon, 06 Feb 2017 22:54:59 +0100,
Andrej Kruták wrote:
> 
> On Mon, Feb 6, 2017 at 10:19 PM, Takashi Iwai  wrote:
> > On Mon, 06 Feb 2017 20:34:58 +0100,
> > Andrej Krutak wrote:
> >>
> >> While not all line6 devices currently support PCM, it causes no
> >> harm to 'have it prepared'.
> >>
> >> This also fixes toneport, which only has PCM - in which case
> >> we previously skipped the USB transfer properties detection completely.
> >>
> >> Signed-off-by: Andrej Krutak 
> >
> > Well, this looks too complex and vague as a regression fix.  If it
> > were 4.10-rc1, I could take it.  But now it's already rc7 and the next
> > might be final, so I prefer a shorter and easier solution.
> >
> 
> In the end, the patch is just "careful", so that the endpoint
> structures are not read if the devices is not "marked" as having them.

Yes, but it's still not very clear what actually "fixes".

> > That said, we can revert the commit as a clear fix at first for 4.10
> > (with Cc to stable), then take this on top as an enhancement for
> > 4.11.
> >
> > Still it's important to know whether this works on Igor's system, so
> > Igor, please give this one try.
> >
> 
> Thanks to zero-init of toneport_properties_table, the proposed simple
> revert should be enough for toneport, and shouldn't break other
> devices (perhaps HD300-HD500 may start showing the dev_err from
> line6_get_interval()).
> 
> So do as you like - e.g. simple revert for 4.10 and apply this patch
> for 4.11. But I don't mind if you won't :-)

Yes, I now queued the revert patch to for-linus branch, and yours to
for-next branch on top of it.


thanks,

Takashi
--
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 linux-next] fixup! usb: gadget: udc: atmel: Update endpoint allocation scheme

[Greg KH]

On Mon, Feb 06, 2017 at 10:02:50PM +0200, cristian.bir...@microchip.com wrote:
> From: Cristian Birsan 
> 
> Signed-off-by: Cristian Birsan 

I can't take a patch with no changelog comments at all, sorry.

And what is with the "fixup!" in the subject line?  I don't see any
other kernel patches having that in it, do you?

Please "fix up" and resend :)

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