Re: usbtmc: vendor specific i/o

[Greg Kroah-Hartman]

On Tue, Aug 02, 2016 at 12:09:12PM +0200, Ladislav Michl wrote:
> On Tue, Aug 02, 2016 at 11:25:22AM +0200, dave penkler wrote:
> >For supportability I would recommend to set the commands depending on the
> >recognised HW, not with aA  ioctl. See rigol quirk. What are the device
> >and manufacturer ids to which these vendor extensions apply?
> >/dave
> 
> This is a custom device... If I understand USBTMC spec corectly, it is not
> either "normal" message or vendor specific message, but both can happily
> coexist, so I wouldn't let it depend on quirks. Once you are going to send
> vendor specific message, you need to know how such a message looks like
> anyway, so any quirk based magic is probably not going to help.
> Also it seems both NI and IVI libs are using similar (selecting MsgID)
> approach.

But your patch was "one-way", once you switched to the other mode, the
old one could not be used :(
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Michal Hocko]

On Tue 02-08-16 15:02:44, Tejun Heo wrote:
> Hello,
> 
> On Tue, Aug 02, 2016 at 03:29:48PM +0200, Oliver Neukum wrote:
> > Apparently if that is the requirement USB will have to define its own
> > set of flags to use in such contexts. But still the calls as currently
> > executed work. Taking away WQ_MEM_RECLAIM would create danger of
> > introducing a regression. The issue with __GFP_DIRECT_RECLAIM already
> > exists and can be fixed.
> 
> Alright, let's go with WQ_MEM_RECLAIM then.

Agreed, I would just add
/*
 * TODO: make sure that no work item in the rescuer can block on an
 * allocation (so no __GF_DIRECT_RECLAIM)
 */
to all work item functions.
-- 
Michal Hocko
SUSE Labs
--
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 RFC 6/6] ARM: dts: bcm2835: Add Raspberry Pi Zero

[Stephen Warren]

On 08/02/2016 01:29 PM, Stefan Wahren wrote:

Hi Stephen,


Stephen Warren  hat am 2. August 2016 um 19:19
geschrieben:


On 07/26/2016 12:53 PM, Stefan Wahren wrote:

The Raspberry Pi Zero is a minified version of model A+. It's
notable there is no PWR LED and the ACT LED is inverted.


Patches 3-6,
Acked-by: Stephen Warren 


diff --git a/arch/arm/boot/dts/bcm2835-rpi-zero.dts
b/arch/arm/boot/dts/bcm2835-rpi-zero.dts


The following comment format is a bit strange, but I'm not sure there's
anything objectively better...


the idea behind that was to provide valid values for every dr_mode with minimum
effort. I see 2 alternative solutions:
  a) add different dr_mode examples to the dwc2 => doesn't work for all
platforms
  b) add comments to drivers/usb/dwc2/platform.c => harder to find

At the end the solution here is already obsolete. This patch series doesn't
contain an update for the bcm2835_defconfig which should set the following:

CONFIG_NOP_USB_XCEIV=y
CONFIG_USB_GADGET=y

After enabling these options another issue is revealed. Currently the dr_mode
for all the other bcm283x boards isn't defined, which means "otg" instead of
intended "host". But the dwc2 driver ignores this as long as CONFIG_USB_GADGET
is not defined. So we need to define the dr_mode for all bcm283x boards.


Do note that the existing DTs must work with any new kernel update; 
that's part of DT being an ABI. So, there should be no need to update 
any of the existing DTs. Rather, the driver must cope with missing 
properties and operate as best it can. Perhaps that means that even if 
dr_mode is unspecified and hence defaults to otg, then if properties 
that are mandatory for OTG to operate are missing, the driver falls back 
to host mode. I think that'd be completely backwards-compatible?


> In

order to avoid such massive copy & paste, we better define 3 dtsi files for each
dr_mode.


3 DT files, with an appropriate one of those included in each RPi DT, 
sounds reasonable.



The only catch about this solution is the modes "otg" and "peripheral"
wouldn't be referenced.


That's probably OK; in this patch the properties for those modes are in 
a comment too, so essentially unreferenced. Perhaps on RPi platforms 
where a choice is possible (Zero, A), there could be a comment next to 
the include of the "host" or "otg" version that states the user might 
want to edit the DT and include the other version instead. Then, those 
files would be somewhat referenced.

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

[RFC PATCH 2/4] usb: typec: Do not check if connected when setting roles

[Guenter Roeck]

The connection status can change after the connection status was
checked. Leave it up to the driver to perform the necessary checks.

Signed-off-by: Guenter Roeck 
---
 drivers/usb/typec/typec.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 0d0ebed..a0ae0d6 100644
--- a/drivers/usb/typec/typec.c
+++ b/drivers/usb/typec/typec.c
@@ -859,9 +859,6 @@ current_data_role_store(struct device *dev, struct 
device_attribute *attr,
return -EOPNOTSUPP;
}
 
-   if (!port->connected)
-   return size;
-
ret = match_string(typec_data_roles, ARRAY_SIZE(typec_data_roles), buf);
if (ret < 0)
return ret;
@@ -918,9 +915,6 @@ static ssize_t current_power_role_store(struct device *dev,
return -EIO;
}
 
-   if (!port->connected)
-   return size;
-
ret = match_string(typec_roles, ARRAY_SIZE(typec_roles), buf);
if (ret < 0)
return ret;
-- 
2.6.6

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

[RFC PATCH 0/4] Type-C Port Manager

[Guenter Roeck]

The following series of patches implements a USB Type-C Port Manager
using the pending USB Type-C class code as basis. The code is still WIP
(I am still waiting for a final decision on locking in the class code),
but I think it is important to get feedback from the community at this point.

There are four patches in the series. The first two patches remove locking from
the Type-C class code. The third patch implements the Type-C Port Manager state
machine. The forth patch is an interface between the Type-C Port Manager and a
TCPCI (Type-C Port Controller Interface) compliant USB Type-C Port Controller.

Patch 4/4 (the interface to a TCPCI compliant chip) is currently untested
since I don't have the necessary hardware available. The port manager code
was tested connecting to an Embedded Controller on a Chromebook, bypassing
the Port Manager implementation in the EC.

Both Source and Sink operation was tested with various Type-C chargers, docks,
and connectors. Alternate mode support is partially implemented (Alternate mode
support is requested from the partner), but alternate modes are actually
selected. Implementing this will require more thought, since the actual
alternate mode support has to be implemented elsewhere, such as in a dedicated
Phy driver. It should be possible to implement the interface between phy driver
and Type-C Port Controller driver using extcon, but I have not further explored
the possibilities, and other options might be possible and/or better.

Why remove locking from the Type-C class code ?

The primary problem is that Type-C state and role changes are triggered by
the Port Manager state machine and quite independent from role change
requests triggered by or requested through the class code. At the same time,
the code handling role change requests triggered from the class code has to
wait for the role change to complete before returning to the class code.
This can result in a deadlock, since the state machine also needs to report
unsolicited role (or status) changes to the class code.

Consider this situation:

- User requests a role change throuch class code.
- Class code acquires mutex, and passes role change request to port manager.
- Port manager attempts to acquire its port mutex. Port state machine is
  active, causing the code to stall.
- Port state machine executes a state change or role change request from the
  port partner. Upon completion, it attempts to inform the class that a role
  or state change occurred. It does so while the port mutex is active to
  prevent state changes while the state machine is still running.
- The callback into the class code waits for the class mutex, which is locked
  because there is also a role change request from the class code pending.

Instead of trying to handle the situation in low level drivers by trying to
avoid deadlocks, it seems better to leave all locking to low level drivers to
start with. Since low level drivers are in control of port changes and port
roles, this appears to be the simpler approach. I am open to listen to other
ideas, though.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 1/4] usb: typec: Drop locking from class code

[Guenter Roeck]

Drop locking from USB Type-C class code to avoid deadlocks between driver
and class code.

Signed-off-by: Guenter Roeck 
---
 drivers/usb/typec/typec.c | 196 ++
 1 file changed, 41 insertions(+), 155 deletions(-)

diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index bbfd6e5..0d0ebed 100644
--- a/drivers/usb/typec/typec.c
+++ b/drivers/usb/typec/typec.c
@@ -17,7 +17,6 @@
 struct typec_port {
unsigned intid;
struct device   dev;
-   struct mutexlock; /* device lock */
 
int prefer_role;
 
@@ -480,38 +479,30 @@ EXPORT_SYMBOL_GPL(typec_disconnect);
 
 void typec_set_data_role(struct typec_port *port, enum typec_data_role role)
 {
-   mutex_lock(>lock);
port->data_role = role;
sysfs_notify(>dev.kobj, NULL, "current_data_role");
-   mutex_unlock(>lock);
 }
 EXPORT_SYMBOL(typec_set_data_role);
 
 void typec_set_pwr_role(struct typec_port *port, enum typec_role role)
 {
-   mutex_lock(>lock);
port->pwr_role = role;
sysfs_notify(>dev.kobj, NULL, "current_power_role");
-   mutex_unlock(>lock);
 }
 EXPORT_SYMBOL(typec_set_pwr_role);
 
 void typec_set_vconn_role(struct typec_port *port, enum typec_role role)
 {
-   mutex_lock(>lock);
port->vconn_role = role;
sysfs_notify(>dev.kobj, NULL, "current_vconn_role");
-   mutex_unlock(>lock);
 }
 EXPORT_SYMBOL(typec_set_vconn_role);
 
 void typec_set_pwr_opmode(struct typec_port *port,
  enum typec_pwr_opmode opmode)
 {
-   mutex_lock(>lock);
port->pwr_opmode = opmode;
sysfs_notify(>dev.kobj, NULL, "power_operation_mode");
-   mutex_unlock(>lock);
 }
 EXPORT_SYMBOL(typec_set_pwr_opmode);
 
@@ -527,15 +518,12 @@ EXPORT_SYMBOL(typec_set_pwr_opmode);
 void typec_altmode_update_active(struct typec_altmode *alt, int mode,
 bool active)
 {
-   struct typec_port *port = typec_altmode2port(alt);
struct typec_mode *m = alt->modes + mode;
char dir[6];
 
-   mutex_lock(>lock);
m->active = active;
sprintf(dir, "mode%d", mode);
sysfs_notify(>dev.kobj, dir, "active");
-   mutex_unlock(>lock);
 }
 EXPORT_SYMBOL(typec_altmode_update_active);
 
@@ -573,14 +561,8 @@ typec_altmode_vdo_show(struct device *dev, struct 
device_attribute *attr,
 {
struct typec_mode *mode = container_of(attr, struct typec_mode,
   vdo_attr);
-   struct typec_port *port = typec_altmode2port(mode->alt_mode);
-   ssize_t ret;
 
-   mutex_lock(>lock);
-   ret = sprintf(buf, "0x%08x\n", mode->vdo);
-   mutex_unlock(>lock);
-
-   return ret;
+   return sprintf(buf, "0x%08x\n", mode->vdo);
 }
 
 static ssize_t
@@ -589,14 +571,8 @@ typec_altmode_desc_show(struct device *dev, struct 
device_attribute *attr,
 {
struct typec_mode *mode = container_of(attr, struct typec_mode,
   desc_attr);
-   struct typec_port *port = typec_altmode2port(mode->alt_mode);
-   ssize_t ret;
-
-   mutex_lock(>lock);
-   ret = sprintf(buf, "%s\n", mode->desc ? mode->desc : "");
-   mutex_unlock(>lock);
 
-   return ret;
+   return sprintf(buf, "%s\n", mode->desc ? mode->desc : "");
 }
 
 static ssize_t
@@ -605,14 +581,8 @@ typec_altmode_active_show(struct device *dev, struct 
device_attribute *attr,
 {
struct typec_mode *mode = container_of(attr, struct typec_mode,
   active_attr);
-   struct typec_port *port = typec_altmode2port(mode->alt_mode);
-   ssize_t ret;
-
-   mutex_lock(>lock);
-   ret = sprintf(buf, "%d\n", mode->active);
-   mutex_unlock(>lock);
 
-   return ret;
+   return sprintf(buf, "%d\n", mode->active);
 }
 
 static ssize_t
@@ -632,13 +602,11 @@ typec_altmode_active_store(struct device *dev, struct 
device_attribute *attr,
if (activate > 1)
return -EINVAL;
 
-   mutex_lock(>lock);
ret = port->cap->activate_mode(mode->alt_mode, mode->index, activate);
if (!ret) {
mode->active = activate;
ret = size;
}
-   mutex_unlock(>lock);
 
return ret;
 }
@@ -649,10 +617,8 @@ typec_altmode_roles_show(struct device *dev, struct 
device_attribute *attr,
 {
struct typec_mode *mode = container_of(attr, struct typec_mode,
   roles_attr);
-   struct typec_port *port = typec_altmode2port(mode->alt_mode);
ssize_t ret;
 
-   mutex_lock(>lock);
switch (mode->roles) {
case TYPEC_PORT_DFP:
ret =  sprintf(buf, "source\n");
@@ -665,7 +631,6 @@ typec_altmode_roles_show(struct device *dev, struct 
device_attribute *attr,
ret = sprintf(buf, "source, 

[RFC PATCH 4/4] usb: typec: Type-C Port Controller Interface driver (tcpci)

[Guenter Roeck]

The port controller interface driver interconnects the Type-C Port
Manager with a Type-C Port Controller Interface (TCPCI) compliant
port controller.

Signed-off-by: Guenter Roeck 
---
 drivers/usb/typec/Kconfig  |   9 +
 drivers/usb/typec/Makefile |   1 +
 drivers/usb/typec/tcpci.c  | 498 +
 drivers/usb/typec/tcpci.h  | 133 
 4 files changed, 641 insertions(+)
 create mode 100644 drivers/usb/typec/tcpci.c
 create mode 100644 drivers/usb/typec/tcpci.h

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 113bb1b..a92c9d1 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -25,4 +25,13 @@ config TYPEC_TCPM
  The Type-C Port Controller Manager provides a USB PD and USB Type-C
  state machine for use with Type-C Port Controllers.
 
+if TYPEC_TCPM
+
+config TYPEC_TCPCI
+   tristate "Type-C Port Controller Interface driver"
+   help
+ Type-C Port Controller driver for TCPCI-compliant controller.
+
+endif
+
 endmenu
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index bbe4572..7dbaf8c 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_TYPEC)+= typec.o
 obj-$(CONFIG_TYPEC_WCOVE)  += typec_wcove.o
 obj-$(CONFIG_TYPEC_TCPM)   += tcpm.o
+obj-$(CONFIG_TYPEC_TCPCI)  += tcpci.o
diff --git a/drivers/usb/typec/tcpci.c b/drivers/usb/typec/tcpci.c
new file mode 100644
index 000..2a98bf4
--- /dev/null
+++ b/drivers/usb/typec/tcpci.c
@@ -0,0 +1,498 @@
+/*
+ * Copyright 2015-2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * USB Type-C Port Controller Interface.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "tcpci.h"
+#include "tcpm.h"
+
+#define PD_RETRY_COUNT 3
+
+struct tcpci {
+   struct device *dev;
+   struct i2c_client *client;
+
+   struct tcpm_port *port;
+
+   struct regmap *regmap;
+
+   bool controls_vbus;
+
+   struct tcpc_dev tcpc;
+};
+
+static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc)
+{
+   return container_of(tcpc, struct tcpci, tcpc);
+}
+
+static int tcpci_read16(struct tcpci *tcpci, unsigned int reg,
+   unsigned int *val)
+{
+   return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u16));
+}
+
+static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
+{
+   return regmap_raw_write(tcpci->regmap, reg, , sizeof(u16));
+}
+
+static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
+{
+   struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+   unsigned int reg;
+   int ret;
+
+   switch (cc) {
+   case TYPEC_CC_RA:
+   reg = (TCPC_ROLE_CTRL_CC_RA << TCPC_ROLE_CTRL_CC1_SHIFT) |
+   (TCPC_ROLE_CTRL_CC_RA << TCPC_ROLE_CTRL_CC2_SHIFT);
+   break;
+   case TYPEC_CC_RD:
+   reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
+   (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
+   break;
+   case TYPEC_CC_RP_DEF:
+   reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
+   (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
+   (TCPC_ROLE_CTRL_RP_VAL_DEF <<
+TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+   break;
+   case TYPEC_CC_RP_1_5:
+   reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
+   (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
+   (TCPC_ROLE_CTRL_RP_VAL_1_5 <<
+TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+   break;
+   case TYPEC_CC_RP_3_0:
+   reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
+   (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
+   (TCPC_ROLE_CTRL_RP_VAL_1_5 <<
+TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+   break;
+   case TYPEC_CC_OPEN:
+   default:
+   reg = (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT) |
+   (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT);
+   break;
+   }
+
+   ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+   if (ret < 0)
+   return ret;
+
+   return 0;
+}
+
+static enum typec_cc_status 

[RFC PATCH 3/4] usb: typec: USB Type-C Port Manager (tcpm)

[Guenter Roeck]

This driver implements the USB Type-C Power Delivery state machine
for both source and sink ports. Alternate mode support is not
fully implemented.

The driver attaches to the USB Type-C class code implemented in
the following patches.

usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY
usb: USB Type-C connector class

This driver only implements the state machine. Lower level drivers are
responsible for
- Reporting VBUS status and activating VBUS
- Setting CC lines and providing CC line status
- Setting line polarity
- Activating and deactivating VCONN
- Setting the current limit
- Activating and deactivating PD message transfers
- Sending and receiving PD messages

The driver provides both a functional API as well as callbacks for
lower level drivers.

Signed-off-by: Guenter Roeck 
---
 drivers/usb/typec/Kconfig  |7 +
 drivers/usb/typec/Makefile |1 +
 drivers/usb/typec/tcpm.c   | 3014 
 drivers/usb/typec/tcpm.h   |  138 ++
 include/linux/usb/pd.h |  269 
 include/linux/usb/pd_bdo.h |   31 +
 include/linux/usb/pd_vdo.h |  412 ++
 7 files changed, 3872 insertions(+)
 create mode 100644 drivers/usb/typec/tcpm.c
 create mode 100644 drivers/usb/typec/tcpm.h
 create mode 100644 include/linux/usb/pd.h
 create mode 100644 include/linux/usb/pd_bdo.h
 create mode 100644 include/linux/usb/pd_vdo.h

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 7a345a4..113bb1b 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -18,4 +18,11 @@ config TYPEC_WCOVE
  To compile this driver as module, choose M here: the module will be
  called typec_wcove
 
+config TYPEC_TCPM
+   tristate "USB Type-C Port Controller Manager"
+   select TYPEC
+   help
+ The Type-C Port Controller Manager provides a USB PD and USB Type-C
+ state machine for use with Type-C Port Controllers.
+
 endmenu
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index b9cb862..bbe4572 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_TYPEC)+= typec.o
 obj-$(CONFIG_TYPEC_WCOVE)  += typec_wcove.o
+obj-$(CONFIG_TYPEC_TCPM)   += tcpm.o
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
new file mode 100644
index 000..1c0b99f
--- /dev/null
+++ b/drivers/usb/typec/tcpm.c
@@ -0,0 +1,3014 @@
+/*
+ * Copyright 2015-2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * USB Power Delivery protocol stack.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "tcpm.h"
+
+#define FOREACH_STATE(S)   \
+   S(INVALID_STATE),   \
+   S(SRC_UNATTACHED),  \
+   S(SRC_ATTACH_WAIT), \
+   S(SRC_ATTACHED),\
+   S(SRC_STARTUP), \
+   S(SRC_SEND_CAPABILITIES),   \
+   S(SRC_NEGOTIATE_CAPABILITIES),  \
+   S(SRC_TRANSITION_SUPPLY),   \
+   S(SRC_READY),   \
+   S(SRC_WAIT_NEW_CAPABILITIES),   \
+   \
+   S(SNK_UNATTACHED),  \
+   S(SNK_ATTACH_WAIT), \
+   S(SNK_DEBOUNCED),   \
+   S(SNK_ATTACHED),\
+   S(SNK_STARTUP), \
+   S(SNK_DISCOVERY),   \
+   S(SNK_DISCOVERY_DEBOUNCE),  \
+   S(SNK_DISCOVERY_DEBOUNCE_DONE), \
+   S(SNK_WAIT_CAPABILITIES),   \
+   S(SNK_NEGOTIATE_CAPABILITIES),  \
+   S(SNK_TRANSITION_SINK), \
+   S(SNK_TRANSITION_SINK_VBUS),\
+   S(SNK_READY),   \
+   \
+   S(ACC_UNATTACHED),  \
+   S(DEBUG_ACC_ATTACHED),  \
+   S(AUDIO_ACC_ATTACHED),  \
+   S(AUDIO_ACC_DEBOUNCE),  \
+   \
+   S(HARD_RESET_SEND), \
+   S(HARD_RESET_START),\
+   S(SRC_HARD_RESET_VBUS_OFF), \
+   

Re: [PATCH V2 RFC 6/6] ARM: dts: bcm2835: Add Raspberry Pi Zero

[Stefan Wahren]

Hi Stephen,

> Stephen Warren  hat am 2. August 2016 um 19:19
> geschrieben:
> 
> 
> On 07/26/2016 12:53 PM, Stefan Wahren wrote:
> > The Raspberry Pi Zero is a minified version of model A+. It's
> > notable there is no PWR LED and the ACT LED is inverted.
> 
> Patches 3-6,
> Acked-by: Stephen Warren 
> 
> > diff --git a/arch/arm/boot/dts/bcm2835-rpi-zero.dts
> > b/arch/arm/boot/dts/bcm2835-rpi-zero.dts
> 
> The following comment format is a bit strange, but I'm not sure there's 
> anything objectively better...

the idea behind that was to provide valid values for every dr_mode with minimum
effort. I see 2 alternative solutions:
  a) add different dr_mode examples to the dwc2 => doesn't work for all
platforms
  b) add comments to drivers/usb/dwc2/platform.c => harder to find

At the end the solution here is already obsolete. This patch series doesn't
contain an update for the bcm2835_defconfig which should set the following:

CONFIG_NOP_USB_XCEIV=y
CONFIG_USB_GADGET=y

After enabling these options another issue is revealed. Currently the dr_mode
for all the other bcm283x boards isn't defined, which means "otg" instead of
intended "host". But the dwc2 driver ignores this as long as CONFIG_USB_GADGET
is not defined. So we need to define the dr_mode for all bcm283x boards. In
order to avoid such massive copy & paste, we better define 3 dtsi files for each
dr_mode.  The only catch about this solution is the modes "otg" and "peripheral"
wouldn't be referenced.

I'll wait for John's feedback before sending a new version.

Stefan

> 
> > + {
> > +   dr_mode = "host";
> > +   h-rx-fifo-size = <774>;
> > +   h-np-tx-fifo-size = <256>;
> > +   h-tx-fifo-size = <512>;
> > +/*
> > + * Settings for otg
> > + *
> > +   dr_mode = "otg";
> > +   h-rx-fifo-size = <774>;
> > +   h-np-tx-fifo-size = <32>;
> > +   h-tx-fifo-size = <0>;
> > +   g-np-tx-fifo-size = <16>;
> > +   g-rx-fifo-size = <256>;
> > +   g-tx-fifo-size = <256 128 128 64 64 64 32>;
> > + *
> > + * Settings for peripheral
> > + *
> > +   dr_mode = "peripheral";
> > +   h-rx-fifo-size = <774>;
> > +   h-np-tx-fifo-size = <0>;
> > +   h-tx-fifo-size = <0>;
> > +   g-np-tx-fifo-size = <16>;
> > +   g-rx-fifo-size = <256>;
> > +   g-tx-fifo-size = <256 128 128 64 64 64 32>;
> > + */
> > +};
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc2: Remove deprecated create_singlethread_workqueue

[John Youn]

++ Felipe

On 7/28/2016 1:27 AM, Bhaktipriya Shridhar wrote:
> alloc_ordered_workqueue replaces the deprecated
> create_singlethread_workqueue.
> 
> There are multiple work items on the work queue, which require
> ordering. Hence, an ordered workqueue has been used.
> 
> The workqueue "wq_otg" is not being used on a memory reclaim path.
> Hence, WQ_MEM_RECLAIM has not been set.
> 
> Signed-off-by: Bhaktipriya Shridhar 
> ---
>  drivers/usb/dwc2/hcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 2df3d04..df5a065 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -5040,7 +5040,7 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
> 
>   /* Create new workqueue and init work */
>   retval = -ENOMEM;
> - hsotg->wq_otg = create_singlethread_workqueue("dwc2");
> + hsotg->wq_otg = alloc_ordered_workqueue("dwc2", 0);
>   if (!hsotg->wq_otg) {
>   dev_err(hsotg->dev, "Failed to create workqueue\n");
>   goto error2;
> --
> 2.1.4
> 
> 


Acked-by: John Youn 

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: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Tejun Heo]

Hello,

On Tue, Aug 02, 2016 at 03:29:48PM +0200, Oliver Neukum wrote:
> Apparently if that is the requirement USB will have to define its own
> set of flags to use in such contexts. But still the calls as currently
> executed work. Taking away WQ_MEM_RECLAIM would create danger of
> introducing a regression. The issue with __GFP_DIRECT_RECLAIM already
> exists and can be fixed.

Alright, let's go with WQ_MEM_RECLAIM then.

Thanks.

-- 
tejun
--
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] usb: dwc3: pci: runtime_resume child device

[Felipe Balbi]

During runtime_resume of dwc3-pci.c, we need to
runtime our child device (which is dwc3 proper)
otherwise nothing will happen.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/dwc3-pci.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 2eb84d6c24a6..0a32430f4c41 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -243,6 +243,13 @@ static int dwc3_pci_runtime_suspend(struct device *dev)
return -EBUSY;
 }
 
+static int dwc3_pci_runtime_resume(struct device *dev)
+{
+   struct platform_device *dwc3 = dev_get_drvdata(dev);
+
+   return pm_runtime_get(>dev);
+}
+
 static int dwc3_pci_pm_dummy(struct device *dev)
 {
/*
@@ -259,7 +266,7 @@ static int dwc3_pci_pm_dummy(struct device *dev)
 
 static struct dev_pm_ops dwc3_pci_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(dwc3_pci_pm_dummy, dwc3_pci_pm_dummy)
-   SET_RUNTIME_PM_OPS(dwc3_pci_runtime_suspend, dwc3_pci_pm_dummy,
+   SET_RUNTIME_PM_OPS(dwc3_pci_runtime_suspend, dwc3_pci_runtime_resume,
NULL)
 };
 
-- 
2.9.1

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

Re: [PATCH V2 RFC 6/6] ARM: dts: bcm2835: Add Raspberry Pi Zero

[Stephen Warren]

On 07/26/2016 12:53 PM, Stefan Wahren wrote:

The Raspberry Pi Zero is a minified version of model A+. It's
notable there is no PWR LED and the ACT LED is inverted.


Patches 3-6,
Acked-by: Stephen Warren 


diff --git a/arch/arm/boot/dts/bcm2835-rpi-zero.dts 
b/arch/arm/boot/dts/bcm2835-rpi-zero.dts


The following comment format is a bit strange, but I'm not sure there's 
anything objectively better...



+ {
+   dr_mode = "host";
+   h-rx-fifo-size = <774>;
+   h-np-tx-fifo-size = <256>;
+   h-tx-fifo-size = <512>;
+/*
+ * Settings for otg
+ *
+   dr_mode = "otg";
+   h-rx-fifo-size = <774>;
+   h-np-tx-fifo-size = <32>;
+   h-tx-fifo-size = <0>;
+   g-np-tx-fifo-size = <16>;
+   g-rx-fifo-size = <256>;
+   g-tx-fifo-size = <256 128 128 64 64 64 32>;
+ *
+ * Settings for peripheral
+ *
+   dr_mode = "peripheral";
+   h-rx-fifo-size = <774>;
+   h-np-tx-fifo-size = <0>;
+   h-tx-fifo-size = <0>;
+   g-np-tx-fifo-size = <16>;
+   g-rx-fifo-size = <256>;
+   g-tx-fifo-size = <256 128 128 64 64 64 32>;
+ */
+};


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

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Oliver Neukum]

On Tue, 2016-08-02 at 13:34 +0200, Michal Hocko wrote:
> On Tue 02-08-16 12:03:23, Oliver Neukum wrote:
> > On Tue, 2016-08-02 at 10:18 +0200, Michal Hocko wrote:
> > > On Tue 02-08-16 10:06:12, Oliver Neukum wrote:
> > > > On Mon, 2016-08-01 at 10:20 -0400, Tejun Heo wrote:
> > > > > Hello,
> > > > 
> > > > > > If any real IO depends on those devices then this is not sufficient 
> > > > > > and
> > > > > > they need some form of guarantee for progress (aka mempool).
> > > > > 
> > > > > Oliver, Alan, what do you think?  If USB itself can't operate without
> > > > > allocating memory during transactions, whatever USB storage drivers
> > > > 
> > > > It cannot. The IO must be described to the hardware with a data
> > > > structure in memory.
> > > > 
> > > > > are doing isn't all that meaningful.  Can we proceed with the
> > > > > workqueue patches?  Also, it could be that the only thing GFP_NOIO and
> > > > > GFP_ATOMIC are doing is increasing the chance of IO failures under
> > > > > memory pressure.  Maybe it'd be a good idea to reconsider the
> > > > > approach?
> > > > 
> > > > We had actual deadlocks with GFP_KERNEL. It seems to me that the SCSI
> > > > layer can deal with IO that cannot be completed due to a lack of memory
> > > > at least somewhat, but a deadlock within a driver would obviously be
> > > > deadly. So I don't think that mempools would remove the need for
> > > > GFP_NOIO as there are places in usbcore we cannot enter the page
> > > > laundering path from. They are an additional need.
> > > 
> > > OK, I guess there is some misunderstanding here. I believe that Tejun
> > > wasn't arguing to drop GFP_NOIO. It might be really needed for the dead
> > > lock avoidance. No question about that. The whole point is that
> > > WQ_RECLAIM might be completely pointless because a rescuer wouldn't help
> > > much if the work item would do GFP_NOIO and get stuck in the page
> > > allocator.
> > 
> > But that can be a problem only if the items on the work queue are
> > actually run and without WQ_MEM_RECLAIM that guarantee cannot be made.
> > We can deal with failures of memory allocation. But the requests
> > must actually terminate.
> 
> I think you have missed my point. So let me ask differently. What is the
> difference between your work item not running at all or looping
> endlessly with GFP_NOIO inside the page allocator? If that particular
> work item is necessary for the further progress then the system is
> screwed one way or another.

The key difference is that I could give the right parameters to the
kmalloc() call. If it doesn't run, I am surely screwed. Thus I conclude
that WQ_RECLAIM needs to be set.

Regards
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

Re: [PATCH 2/9] usb: gadget: align buffer size when allocating for OUT endpoint

[Felipe Ferreri Tonello]

Hi Michal,

On 27/07/16 20:59, Michal Nazarewicz wrote:
> On Tue, Jul 26 2016, Felipe F. Tonello wrote:
>> Using usb_ep_align() makes sure that the buffer size for OUT endpoints is
>> always aligned with wMaxPacketSize (512 usually). This makes sure
>> that no buffer has the wrong size, which can cause nasty bugs.
>>
>> Signed-off-by: Felipe F. Tonello 
>> ---
>>  drivers/usb/gadget/u_f.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/u_f.c b/drivers/usb/gadget/u_f.c
>> index 4bc7eea8bfc8..d1933b0b76c3 100644
>> --- a/drivers/usb/gadget/u_f.c
>> +++ b/drivers/usb/gadget/u_f.c
>> @@ -12,6 +12,7 @@
>>   */
>>  
>>  #include "u_f.h"
>> +#include 
>>  
>>  struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int 
>> default_len)
>>  {
>> @@ -20,6 +21,8 @@ struct usb_request *alloc_ep_req(struct usb_ep *ep, int 
>> len, int default_len)
>>  req = usb_ep_alloc_request(ep, GFP_ATOMIC);
>>  if (req) {
>>  req->length = len ?: default_len;
>> +if (usb_endpoint_dir_out(ep->desc))
>> +req->length = usb_ep_align(ep, req->length);
>>  req->buf = kmalloc(req->length, GFP_ATOMIC);
>>  if (!req->buf) {
>>  usb_ep_free_request(ep, req);
> 
> I’m a bit scared of this change.

I agree, it's scary. :D

> 
> Drivers which call alloc_ep_req and then ignore req->length using the
> same length they passed to the function will silently drop data.
> 
> Drivers which do not ignore req->length may end up overwriting some
> other buffer, e.g.:
> 
>   some_buffer = kmalloc(length, GFP_KERNEL);
> req = alloc_ep_req(ep, length, 0);
> … later …
> memcpy(some_buffer, req->buf, req->length);

True. The same happens if the data associated with an OUT endpoint is
smaller than wMaxPacketSize.

This patch doesn't fix all problems associated with that, but it allows
better practice to take place. It returns to the driver the actual
allocated size, like several POSIX functions.

I haven't seen any problems on all gadgets that rely on alloc_ep_req().
Maybe as we port other gadgets to this use this function instead of
usb_ep_alloc_request() we might find some issues.

Perhaps we should add better documentation to alloc_ep_req()?

-- 
Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: [PATCH] musb: omap2430: do not assume balanced enable()/disable()

[Andreas Kemnade]

On Tue, 2 Aug 2016 03:33:34 -0700
Tony Lindgren  wrote:

> * Andreas Kemnade  [160729 11:14]:
> > The code assumes that omap2430_musb_enable() and
> > omap2430_musb_disable() is called in a balanced way. The
> > That fact is broken by the fact that musb_init_controller() calls
> > musb_platform_disable() to switch from unknown state to off state.
> 
> OK, some spelling issues with the above paragraph though :)
> 
> > That means that phy_power_off() is called first so that
> > phy->power_count gets -1 and the phy is not enabled on
> > phy_power_on(). In the probably common case of using the
> > phy_twl4030, that prevents also charging the battery and so makes
> > further kernel debugging hard.
> 
> Is this with v4.7 kernel? Also, care to describe how you hit this
> and on which hardware? Just wondering..

I got this error on the Openphoenux GTA04 phone. It has a DM3730
SoC and a TPS65950 companion. Severe charging problems were already
observed with the 4.4rc1. I do not know if that already was exactly
*this* problem. I have debugged and patched the v4.7 kernel.
How I hit the problem: Just boot an that device and try to charge
via usb.
Should I resubmit the patch with an extended commit message?

Regards,
Andreas Kemnade


pgpx1rKf1rrwD.pgp
Description: OpenPGP digital signature

Re: [PATCH 7/9] usb: gadget: remove useless parameter in alloc_ep_req()

[Felipe Ferreri Tonello]

Hi Michal,

On 27/07/16 21:02, Michal Nazarewicz wrote:
> On Tue, Jul 26 2016, Felipe F. Tonello wrote:
>> This parameter was not really necessary and gadget drivers would almost 
>> always
> 
> I personally like when commit messages can be read without subject, so
> perhaps:
> 
>   The default_length parameter of alloc_ep_req was not …
> 
> But that’s just me.

Good judgment.

> 
>> create an inline function to pass the same value to len and default_len.
>>
>> So this patch also removes duplicate code from few drivers.
>>
>> Signed-off-by: Felipe F. Tonello 
> 
> Acked-by: Michal Nazarewicz 
> 
>> ---
>>  drivers/usb/gadget/function/f_hid.c| 10 ++
>>  drivers/usb/gadget/function/f_loopback.c   |  9 +
>>  drivers/usb/gadget/function/f_midi.c   | 10 ++
>>  drivers/usb/gadget/function/f_sourcesink.c | 11 ++-
>>  drivers/usb/gadget/u_f.c   |  7 +++
>>  drivers/usb/gadget/u_f.h   |  2 +-
>>  6 files changed, 11 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_hid.c 
>> b/drivers/usb/gadget/function/f_hid.c
>> index 51980c50546d..e82a7468252e 100644
>> --- a/drivers/usb/gadget/function/f_hid.c
>> +++ b/drivers/usb/gadget/function/f_hid.c
>> @@ -362,12 +362,6 @@ static int f_hidg_open(struct inode *inode, struct file 
>> *fd)
>>  
>> /*-*/
>>  /*usb_function 
>> */
>>  
>> -static inline struct usb_request *hidg_alloc_ep_req(struct usb_ep *ep,
>> -unsigned length)
>> -{
>> -return alloc_ep_req(ep, length, length);
>> -}
>> -
>>  static void hidg_set_report_complete(struct usb_ep *ep, struct usb_request 
>> *req)
>>  {
>>  struct f_hidg *hidg = (struct f_hidg *) req->context;
>> @@ -549,8 +543,8 @@ static int hidg_set_alt(struct usb_function *f, unsigned 
>> intf, unsigned alt)
>>   */
>>  for (i = 0; i < hidg->qlen && status == 0; i++) {
>>  struct usb_request *req =
>> -hidg_alloc_ep_req(hidg->out_ep,
>> -  hidg->report_length);
>> +alloc_ep_req(hidg->out_ep,
>> +hidg->report_length);
>>  if (req) {
>>  req->complete = hidg_set_report_complete;
>>  req->context  = hidg;
>> diff --git a/drivers/usb/gadget/function/f_loopback.c 
>> b/drivers/usb/gadget/function/f_loopback.c
>> index 3a9f8f9c77bd..701ee0f11c33 100644
>> --- a/drivers/usb/gadget/function/f_loopback.c
>> +++ b/drivers/usb/gadget/function/f_loopback.c
>> @@ -306,13 +306,6 @@ static void disable_loopback(struct f_loopback *loop)
>>  VDBG(cdev, "%s disabled\n", loop->function.name);
>>  }
>>  
>> -static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int 
>> len)
>> -{
>> -struct f_loopback   *loop = ep->driver_data;
>> -
>> -return alloc_ep_req(ep, len, loop->buflen);
>> -}
>> -
>>  static int alloc_requests(struct usb_composite_dev *cdev,
>>struct f_loopback *loop)
>>  {
>> @@ -333,7 +326,7 @@ static int alloc_requests(struct usb_composite_dev *cdev,
>>  if (!in_req)
>>  goto fail;
>>  
>> -out_req = lb_alloc_ep_req(loop->out_ep, 0);
>> +out_req = alloc_ep_req(loop->out_ep, loop->buflen);
>>  if (!out_req)
>>  goto fail_in;
>>  
>> diff --git a/drivers/usb/gadget/function/f_midi.c 
>> b/drivers/usb/gadget/function/f_midi.c
>> index 3a47596afcab..abf26364b46f 100644
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -208,12 +208,6 @@ static struct usb_gadget_strings *midi_strings[] = {
>>  NULL,
>>  };
>>  
>> -static inline struct usb_request *midi_alloc_ep_req(struct usb_ep *ep,
>> -unsigned length)
>> -{
>> -return alloc_ep_req(ep, length, length);
>> -}
>> -
>>  static const uint8_t f_midi_cin_length[] = {
>>  0, 0, 2, 3, 3, 1, 2, 3, 3, 3, 3, 3, 2, 2, 3, 1
>>  };
>> @@ -365,7 +359,7 @@ static int f_midi_set_alt(struct usb_function *f, 
>> unsigned intf, unsigned alt)
>>  /* pre-allocate write usb requests to use on f_midi_transmit. */
>>  while (kfifo_avail(>in_req_fifo)) {
>>  struct usb_request *req =
>> -midi_alloc_ep_req(midi->in_ep, midi->buflen);
>> +alloc_ep_req(midi->in_ep, midi->buflen);
>>  
>>  if (req == NULL)
>>  return -ENOMEM;
>> @@ -379,7 +373,7 @@ static int f_midi_set_alt(struct usb_function *f, 
>> unsigned intf, unsigned alt)
>>  /* allocate a bunch of read buffers and queue them 

[PATCH 0661/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/media/usb/zr364xx/zr364xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/zr364xx/zr364xx.c 
b/drivers/media/usb/zr364xx/zr364xx.c
index 7433ba5..896c49e 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -89,9 +89,9 @@ static int mode;
 
 
 /* Module parameters interface */
-module_param(debug, int, 0644);
+module_param(debug, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(debug, "Debug level");
-module_param(mode, int, 0644);
+module_param(mode, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(mode, "0 = 320x240, 1 = 160x120, 2 = 640x480");
 
 
-- 
2.9.2

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

Re: NEC uPD720200 xHCI Controller dies when Runtime PM enabled

[Durval Menezes]

Hello,

On Mon, Aug 1, 2016 at 1:44 PM, Durval Menezes  wrote:
> Hi Mike,
> 
> On Mon, Aug 1, 2016 at 12:05 PM, Mike Murdoch  
> wrote:
> > On 2016-08-01 13:57, Durval Menezes wrote:
> > > Hi Mathias,
> > >
> > > On Mon, Aug 1, 2016 at 8:20 AM, Mathias Nyman 
> > >  wrote:
> > >>> On 01.08.2016 13:15, Durval Menezes wrote:
> > >>> Hello Mike, Mathias, list,
> > >>>
> > >>> On 06.02.2016 19:08, Mike Murdoch wrote:
> > >>> Bug ID: 111251
> > >>>
> > >>> I have a NEC uPD720200 USB3.0 controller in a Thinkpad W520 laptop on
> > >>> kernel 4.4.1-gentoo.
> > >>>
> > >>> 0e:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host
> > >>> Controller (rev 04) (prog-if 30 [XHCI])
> > >>>  Subsystem: Lenovo uPD720200 USB 3.0 Host Controller
> > >>>  Flags: bus master, fast devsel, latency 0
> > >>>  Memory at f380 (64-bit, non-prefetchable) [size=8K]
> > >>>  Capabilities: [50] Power Management version 3
> > >>>  Capabilities: [70] MSI: Enable- Count=1/8 Maskable- 64bit+
> > >>>  Capabilities: [90] MSI-X: Enable+ Count=8 Masked-
> > >>>  Capabilities: [a0] Express Endpoint, MSI 00
> > >>>  Capabilities: [100] Advanced Error Reporting
> > >>>  Capabilities: [140] Device Serial Number ff-ff-ff-ff-ff-ff-ff-ff
> > >>>  Capabilities: [150] Latency Tolerance Reporting
> > >>>  Kernel driver in use: xhci_hcd
> > >>>  Kernel modules: xhci_pci
> > >>>
> > >>> When runtime power control for this controller is disabled
> > >>> (/sys/bus/pci/devices/:0e:00.0/power/control = on), the controller
> > >>> works fine and reaches over 120MB/s transfer rates.
> > >>>
> > >>> When runtime power control for this controller is enabled
> > >>> (/sys/bus/pci/devices/:0e:00.0/power/control = auto), two effects
> > >>> can be observed:
> > >>>
> > >>> - Transfer rates are much lower at around 30MB/s
> > >>> - During transfers, the controller dies after a couple of seconds:
> > >>>
> > >>> I found this message in the list archives, and I have the exact same
> > >>> issues on exactly the same hardware (Thinkpad W520 laptop with the same
> > >>> USB3 controller showing on lspci -v); otherwise, I'm running distro 
> > >>> kernel
> > >>> 2.6.32-573.7.1.el6.x86_64 on a Springdale Linux 6.7 (RHEL6) install.
> > >>>
> > >>> I just verified that my controller's PM was set by default to "auto":
> > >>> cat /sys/bus/pci/devices/\:0e\:00.0/power/control
> > >>> auto
> > >>> I have now set it to "on" and will test whether this will work around
> > >>> the issue (I'm waiting for my USB3.0 "heavy duty" disk docks to be
> > >>> released from another system that is using them right now).

The docks (actually a 4-disk Mediasonic Probox enclosure, and a
single-disk USpeed SATA-to-USB adapter) have returned; I've rebooted my
machine (to make sure I was starting from as clean a slate as possible),
then (before plugging anything) set the controller's PM to "on" (ie, no
power management) with the "echo" command above, and confirmed it with the
"cat" command above. Then I tried to plug first the adapter to each of
the two ports controlled by the uPD720200; the result for each attempt
(as recorded on syslog with level debug) was just:

Aug  2 11:45:16 localhost kernel: hub 3-0:1.0: unable to enumerate USB 
device on port 1

For the lower port, and 

Aug  2 11:54:19 localhost kernel: hub 3-0:1.0: unable to enumerate USB 
device on port 2

For the upper port.

To confirm that the adapter is working, I connected it to the "combo"
USB/eSATA adapter (which on the W520 is right besides the two uPD720200
ports) and it worked great (albeit limited to USB2.1, as this is this
port's type):

Aug  2 11:56:46 localhost kernel: usb 2-1.2: new high speed USB device 
number 3 using ehci_hcd
Aug  2 11:56:47 localhost kernel: usb 2-1.2: New USB device found, 
idVendor=174c, idProduct=5106
Aug  2 11:56:47 localhost kernel: usb 2-1.2: New USB device strings: 
Mfr=2, Product=3, SerialNumber=1
Aug  2 11:56:47 localhost kernel: usb 2-1.2: Product: AS2105
Aug  2 11:56:47 localhost kernel: usb 2-1.2: Manufacturer: ASMedia
Aug  2 11:56:47 localhost kernel: usb 2-1.2: SerialNumber: 
W2A87518
Aug  2 11:56:47 localhost kernel: usb 2-1.2: configuration #1 chosen 
from 1 choice
Aug  2 11:56:47 localhost kernel: Initializing USB Mass Storage 
driver...
Aug  2 11:56:47 localhost kernel: scsi6 : SCSI emulation for USB Mass 
Storage devices
Aug  2 11:56:47 localhost kernel: usb-storage: device found at 3
Aug  2 11:56:47 localhost kernel: usb-storage: waiting for device to 
settle before scanning
Aug  2 11:56:47 localhost kernel: usbcore: registered new interface 
driver usb-storage
Aug  2 11:56:47 localhost kernel: USB Mass Storage support registered.
Aug  2 11:56:48 localhost kernel: 

[PATCH 0751/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/net/usb/smsc75xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 9af9799..d384afe 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -79,7 +79,7 @@ struct usb_context {
 };
 
 static bool turbo_mode = true;
-module_param(turbo_mode, bool, 0644);
+module_param(turbo_mode, bool, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
 
 static int __must_check __smsc75xx_read_reg(struct usbnet *dev, u32 index,
-- 
2.9.2

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

[PATCH 0752/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/net/usb/smsc95xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index dc989a8..8cf6e39 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -77,7 +77,7 @@ struct smsc95xx_priv {
 };
 
 static bool turbo_mode = true;
-module_param(turbo_mode, bool, 0644);
+module_param(turbo_mode, bool, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
 
 static int __must_check __smsc95xx_read_reg(struct usbnet *dev, u32 index,
-- 
2.9.2

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

Re: [PATCH 1/9] usb: gadget: fix usb_ep_align_maybe endianness and new usb_ep_align

[Felipe Ferreri Tonello]

Hi Michal,

On 27/07/16 20:37, Michal Nazarewicz wrote:
> On Tue, Jul 26 2016, Felipe F. Tonello wrote:
>> USB spec specifies wMaxPacketSize to be little endian (as other properties),
>> so when using this variable in the driver we should convert to the current
>> CPU endianness if necessary.
>>
>> This patch also introduces usb_ep_align() which does always returns the
>> aligned buffer size for an endpoint. This is useful to be used by USB 
>> requests
>> allocator functions.
>>
>> Signed-off-by: Felipe F. Tonello 
> 
> Acked-by: Michal Nazarewicz 
> 
>> ---
>>  include/linux/usb/gadget.h | 17 ++---
>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>> index 612dbdfa388e..b8fa6901b881 100644
>> --- a/include/linux/usb/gadget.h
>> +++ b/include/linux/usb/gadget.h
>> @@ -418,8 +418,20 @@ static inline struct usb_gadget 
>> *dev_to_usb_gadget(struct device *dev)
>>  list_for_each_entry(tmp, &(gadget)->ep_list, ep_list)
>>  
>>  /**
>> + * usb_ep_align - returns @len aligned to ep's maxpacketsize.
>> + * @ep: the endpoint whose maxpacketsize is used to align @len
>> + * @len: buffer size's length to align to @ep's maxpacketsize
>> + *
>> + * This helper is used to align buffer's size to an ep's maxpacketsize.
>> + */
>> +static inline size_t usb_ep_align(struct usb_ep *ep, size_t len)
>> +{
>> +return round_up(len, (size_t)le16_to_cpu(ep->desc->wMaxPacketSize));
>> +}
>> +
>> +/**
>>   * usb_ep_align_maybe - returns @len aligned to ep's maxpacketsize if gadget
>> - *  requires quirk_ep_out_aligned_size, otherwise reguens len.
>> + *  requires quirk_ep_out_aligned_size, otherwise returns len.
>>   * @g: controller to check for quirk
>>   * @ep: the endpoint whose maxpacketsize is used to align @len
>>   * @len: buffer size's length to align to @ep's maxpacketsize
>> @@ -430,8 +442,7 @@ static inline struct usb_gadget 
>> *dev_to_usb_gadget(struct device *dev)
>>  static inline size_t
>>  usb_ep_align_maybe(struct usb_gadget *g, struct usb_ep *ep, size_t len)
>>  {
>> -return !g->quirk_ep_out_aligned_size ? len :
>> -round_up(len, (size_t)ep->desc->wMaxPacketSize);
>> +return !g->quirk_ep_out_aligned_size ? len : usb_ep_align(ep, len);
> 
> I’d drop the negation:
> 
> + return g->quirk_ep_out_aligned_size ? usb_ep_align(ep, len) : len;

Agreed.

> 
>>  }
>>  
>>  /**
>> -- 
>> 2.9.0
>>
> 

-- 
Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: [PATCH 2/2] usb: dwc3: core: allow device to runtime_suspend several times

[Alan Stern]

On Tue, 2 Aug 2016, Felipe Balbi wrote:

> After going through runtime_suspend/runtime_resume
> cycle once we would be left with an unbalanced
> pm_runtime_get() call. Fix that by making sure that
> we try to suspend right after resuming so things are
> balanced and device can runtime_suspend again.
> 
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/usb/dwc3/core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 946643157b78..35d092456bec 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1192,6 +1192,7 @@ static int dwc3_runtime_resume(struct device *dev)
>   }
>  
>   pm_runtime_mark_last_busy(dev);
> + pm_runtime_put(dev);
>  
>   return 0;
>  }

This may be correct, but it certainly looks odd.  For example, it 
wouldn't work right if you ever called pm_runtime_resume() instead of 
pm_runtime_get().

Alan Stern

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

[PATCH 0990/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/wusbcore/wusbhc.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/wusbcore/wusbhc.c b/drivers/usb/wusbcore/wusbhc.c
index 94f401a..1aaa452 100644
--- a/drivers/usb/wusbcore/wusbhc.c
+++ b/drivers/usb/wusbcore/wusbhc.c
@@ -84,7 +84,7 @@ static ssize_t wusb_trust_timeout_store(struct device *dev,
 out:
return result < 0 ? result : size;
 }
-static DEVICE_ATTR(wusb_trust_timeout, 0644, wusb_trust_timeout_show,
+static DEVICE_ATTR(wusb_trust_timeout, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
wusb_trust_timeout_show,
 wusb_trust_timeout_store);
 
 /*
@@ -145,7 +145,7 @@ static ssize_t wusb_chid_store(struct device *dev,
result = wusbhc_chid_set(wusbhc, );
return result < 0 ? result : size;
 }
-static DEVICE_ATTR(wusb_chid, 0644, wusb_chid_show, wusb_chid_store);
+static DEVICE_ATTR(wusb_chid, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
wusb_chid_show, wusb_chid_store);
 
 
 static ssize_t wusb_phy_rate_show(struct device *dev,
@@ -174,7 +174,7 @@ static ssize_t wusb_phy_rate_store(struct device *dev,
wusbhc->phy_rate = phy_rate;
return size;
 }
-static DEVICE_ATTR(wusb_phy_rate, 0644, wusb_phy_rate_show,
+static DEVICE_ATTR(wusb_phy_rate, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
wusb_phy_rate_show,
wusb_phy_rate_store);
 
 static ssize_t wusb_dnts_show(struct device *dev,
@@ -205,7 +205,7 @@ static ssize_t wusb_dnts_store(struct device *dev,
 
return size;
 }
-static DEVICE_ATTR(wusb_dnts, 0644, wusb_dnts_show, wusb_dnts_store);
+static DEVICE_ATTR(wusb_dnts, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
wusb_dnts_show, wusb_dnts_store);
 
 static ssize_t wusb_retry_count_show(struct device *dev,
  struct device_attribute *attr,
@@ -234,7 +234,7 @@ static ssize_t wusb_retry_count_store(struct device *dev,
 
return size;
 }
-static DEVICE_ATTR(wusb_retry_count, 0644, wusb_retry_count_show,
+static DEVICE_ATTR(wusb_retry_count, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
wusb_retry_count_show,
wusb_retry_count_store);
 
 /* Group all the WUSBHC attributes */
-- 
2.9.2

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

[PATCH 1/1] Add driver for smsc usb251x i2c configuration

[Fabien Lahoudere]

This driver copy the configuration of the controller EEPROM via i2c.
Configuration information is available in Documentation/usb/usb251x.txt

Signed-off-by: Fabien Lahoudere 
---
 Documentation/devicetree/bindings/usb/usb251x.txt |  27 +++
 drivers/usb/misc/Kconfig  |   9 +
 drivers/usb/misc/Makefile |   1 +
 drivers/usb/misc/usb251x.c| 265 ++
 4 files changed, 302 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/usb251x.txt
 create mode 100644 drivers/usb/misc/usb251x.c

diff --git a/Documentation/devicetree/bindings/usb/usb251x.txt 
b/Documentation/devicetree/bindings/usb/usb251x.txt
new file mode 100644
index 000..2b0863a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb251x.txt
@@ -0,0 +1,27 @@
+SMSC USB 2.0 Hi-Speed Hub Controller
+
+Required properties:
+- compatible = "smsc,usb251x";
+- reg = <0x2c>;
+
+Optional properties:
+- smsc,usb251x-cfg-data1 : u8, set configuration data 1 (byte 0x06)
+- smsc,usb251x-cfg-data2 : u8, set configuration data 2 (byte 0x07)
+- smsc,usb251x-cfg-data3 : u8, set configuration data 3 (byte 0x08)
+- smsc,usb251x-portmap12 : u8, set port mapping for ports 1 and 2 (byte 0xfb)
+- smsc,usb251x-portmap34 : u8, set port mapping for ports 3 and 4 (byte 0xfc)
+- smsc,usb251x-portmap56 : u8, set port mapping for ports 5 and 6 (byte 0xfd)
+- smsc,usb251x-portmap7 : u8, set port mapping for port 7 (byte 0xfe)
+- smsc,usb251x-status-command : u8, configure smbus behaviour (byte 0xff)
+
+Example:
+
+   usb251x: usb251x@2c {
+   compatible = "smsc,usb251x";
+   reg = <0x2c>;
+   smsc,usb251x-cfg-data3 = <0x09>;
+   smsc,usb251x-portmap12 = <0x21>;
+   smsc,usb251x-portmap12 = <0x43>;
+   smsc,usb251x-status-command = <0x1>;
+   };
+
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index eb8f8d3..89c532f 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -240,6 +240,15 @@ config USB_HSIC_USB3503
help
  This option enables support for SMSC USB3503 HSIC to USB 2.0 Driver.
 
+config USB_USB251X
+   tristate "USB251X device configurable via I2C"
+   depends on I2C
+   help
+ This option enables support for configuring SMSC USB251X USB hub.
+This driver write the hub configuration in EEPROM via i2C. Fields can 
be
+configured through device tree.
+See Documentation/devicetree/bindings/usb/usb251x.txt
+
 config USB_LINK_LAYER_TEST
tristate "USB Link Layer Test driver"
help
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 3d79faa..dac251a 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -27,5 +27,6 @@ obj-$(CONFIG_USB_HSIC_USB3503)+= usb3503.o
 obj-$(CONFIG_USB_CHAOSKEY) += chaoskey.o
 obj-$(CONFIG_UCSI) += ucsi.o
 
+obj-$(CONFIG_USB_USB251X)  += usb251x.o
 obj-$(CONFIG_USB_SISUSBVGA)+= sisusbvga/
 obj-$(CONFIG_USB_LINK_LAYER_TEST)  += lvstest.o
diff --git a/drivers/usb/misc/usb251x.c b/drivers/usb/misc/usb251x.c
new file mode 100644
index 000..b3750fc
--- /dev/null
+++ b/drivers/usb/misc/usb251x.c
@@ -0,0 +1,265 @@
+/*
+ * drivers/usb/misc/usb251x.c
+ *
+ * driver for SMSC USB251X USB Hub
+ *
+ * Authors: Rick Bronson 
+ *  Fabien Lahoudere 
+ *
+ * Register initialization is based on code examples provided by Philips
+ * Copyright (c) 2005 Koninklijke Philips Electronics N.V.
+ *
+ * This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* registers */
+#define USB251X_VENDOR_ID_LSB 0x00
+#define USB251X_VENDOR_ID_MSB 0x01
+#define USB251X_PRODUCT_ID_LSB 0x02
+#define USB251X_PRODUCT_ID_MSB 0x03
+#define USB251X_DEVICE_ID_LSB 0x04
+#define USB251X_DEVICE_ID_MSB 0x05
+#define USB251X_CONFIGURATION_DATA_BYTE_1 0x06
+#define USB251X_CONFIGURATION_DATA_BYTE_2 0x07
+#define USB251X_CONFIGURATION_DATA_BYTE_3 0x08
+#define USB251X_NON_REMOVABLE_DEVICES 0x09
+#define USB251X_PORT_DISABLE_SELF 0x0A
+#define USB251X_PORT_DISABLE_BUS 0x0B
+#define USB251X_MAX_POWER_SELF 0x0C
+#define USB251X_MAX_POWER_BUS 0x0D
+#define USB251X_HUB_CONTROLLER_MAX_CURRENT_SELF 0x0E
+#define USB251X_HUB_CONTROLLER_MAX_CURRENT_BUS 0x0F
+#define USB251X_POWER_ON_TIME 0x10
+#define USB251X_LANGUAGE_ID_HIGH 0x11
+#define USB251X_LANGUAGE_ID_LOW 0x12
+#define USB251X_MANUFACTURER_STRING_LENGTH 0x13
+#define USB251X_PRODUCT_STRING_LENGTH 0x14
+#define USB251X_SERIAL_STRING_LENGTH 0x15
+#define USB251X_MANUFACTURER_STRING 0x16
+#define USB251X_PRODUCT_STRING 

Re: [PATCH] usb: ehci: change order of register cleanup during shutdown

[Alan Stern]

On Tue, 2 Aug 2016, Marc Ohlf wrote:

> On Mon, Aug 01, 2016 at 06:07:17PM +0200, Alan Stern wrote:
> > On Mon, 1 Aug 2016, Marc Ohlf wrote:
> > 
> > > In ehci_turn_off_all_ports() all EHCI port register bits
> > > (except the PORT_RWC_BITS) are set to zero.
> > 
> > Even the PORT_WRC_BITS are set to 0.  Oddly enough, the way to set 
> > those bits to 0 is to write a 1 to them (see Table 2-16 in the EHCI 
> > spec).
> 
> Thanks for correction, i will improve that in an v2.
> 
> > 
> > > On some hardware, this can lead to an system hang,
> > > when ehci_port_power() accesses the already cleaned registers.
> > > 
> > > This patch changes the order of cleanup.
> > > First call ehci_port_power() which respects the current bits in
> > > port status registers
> > > and afterwards cleanup the hard way by setting everything else to zero.
> > > 
> > > Signed-off-by: Marc Ohlf 
> > > ---
> > >  drivers/usb/host/ehci-hcd.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> > > index a962b89..1e5f529 100644
> > > --- a/drivers/usb/host/ehci-hcd.c
> > > +++ b/drivers/usb/host/ehci-hcd.c
> > > @@ -332,11 +332,11 @@ static void ehci_turn_off_all_ports(struct ehci_hcd 
> > > *ehci)
> > >   int port = HCS_N_PORTS(ehci->hcs_params);
> > >  
> > >   while (port--) {
> > > - ehci_writel(ehci, PORT_RWC_BITS,
> > > - >regs->port_status[port]);
> > >   spin_unlock_irq(>lock);
> > >   ehci_port_power(ehci, port, false);
> > >   spin_lock_irq(>lock);
> > > + ehci_writel(ehci, PORT_RWC_BITS,
> > > + >regs->port_status[port]);
> > >   }
> > >  }
> > 
> 
> I'm new to the mailing lists, so I've some questions.
> 
> > Acked-by: Alan Stern 
> 
> I should include your Ack in my v2 patch signoff area, right?

Yes.

> > 
> > This should be marked for the -stable kernels as well.
> 
> Marking this for -stable kernels means cc it to stable mailing list, right?

No, it means adding:

CC: 

in among the Signed-off-by: and Acked-by: lines.  This tag will 
automatically cause the patch to be added to the -stable queue when it 
gets accepted.

Alan Stern

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

[PATCH 0975/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/core/devio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index e9f5043..fb3c152 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -135,7 +135,7 @@ enum snoop_when {
 
 /* Limit on the total amount of memory we can allocate for transfers */
 static unsigned usbfs_memory_mb = 16;
-module_param(usbfs_memory_mb, uint, 0644);
+module_param(usbfs_memory_mb, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(usbfs_memory_mb,
"maximum MB allowed for usbfs buffers (0 = no limit)");
 
-- 
2.9.2

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

Re: [PATCH 1/1] Add driver for smsc usb251x i2c configuration

[Mark Rutland]

Hi,

On Tue, Aug 02, 2016 at 04:31:54PM +0200, Fabien Lahoudere wrote:
> This driver copy the configuration of the controller EEPROM via i2c.
> Configuration information is available in Documentation/usb/usb251x.txt
> 
> Signed-off-by: Fabien Lahoudere 
> ---
>  Documentation/devicetree/bindings/usb/usb251x.txt |  27 +++
>  drivers/usb/misc/Kconfig  |   9 +
>  drivers/usb/misc/Makefile |   1 +
>  drivers/usb/misc/usb251x.c| 265 
> ++
>  4 files changed, 302 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/usb251x.txt
>  create mode 100644 drivers/usb/misc/usb251x.c
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb251x.txt 
> b/Documentation/devicetree/bindings/usb/usb251x.txt
> new file mode 100644
> index 000..2b0863a3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/usb251x.txt
> @@ -0,0 +1,27 @@
> +SMSC USB 2.0 Hi-Speed Hub Controller
> +
> +Required properties:
> +- compatible = "smsc,usb251x";

Please us specific compatible strings rather than wildcards.

> +- reg = <0x2c>;
> +
> +Optional properties:
> +- smsc,usb251x-cfg-data1 : u8, set configuration data 1 (byte 0x06)
> +- smsc,usb251x-cfg-data2 : u8, set configuration data 2 (byte 0x07)
> +- smsc,usb251x-cfg-data3 : u8, set configuration data 3 (byte 0x08)
> +- smsc,usb251x-portmap12 : u8, set port mapping for ports 1 and 2 (byte 0xfb)
> +- smsc,usb251x-portmap34 : u8, set port mapping for ports 3 and 4 (byte 0xfc)
> +- smsc,usb251x-portmap56 : u8, set port mapping for ports 5 and 6 (byte 0xfd)
> +- smsc,usb251x-portmap7 : u8, set port mapping for port 7 (byte 0xfe)
> +- smsc,usb251x-status-command : u8, configure smbus behaviour (byte 0xff)

For device tree bindings we generally shy away from encoding raw values
in this manner. I'm very much not keen on this as-is.

What exactly do these values represent?

Why must these be configured through DT? When should a dts author
provide them?

I have more comments on the representation below.

> +
> +Example:
> +
> + usb251x: usb251x@2c {
> + compatible = "smsc,usb251x";
> + reg = <0x2c>;
> + smsc,usb251x-cfg-data3 = <0x09>;
> + smsc,usb251x-portmap12 = <0x21>;
> + smsc,usb251x-portmap12 = <0x43>;
> + smsc,usb251x-status-command = <0x1>;
> + };

Above these were describes as u8 values, but here they're treated as u32
due to the lack of a /bits/ 8 prefix on the values. Trying to store them
as u8 saves no space whatsoever, given values are always padded to 32
bits.

[...]

> +static unsigned char default_init_table[USB251X_ADDR_SZ] = {
> + 0x24, 0x04, 0x14, 0x25, 0xa0, 0x0b, 0x9b, 0x20, /* 00 - 07 */
> + 0x02, 0x00, 0x00, 0x00, 0x01, 0x32, 0x01, 0x32, /* 08 - 0F */
> + 0x32, 0x00, 0x00, 4, 30, 0x00, 'S', 0x00,   /* 10 - 17 */
> + 'M', 0x00, 'S', 0x00, 'C', 0x00, 0x00, 0x00,/* 18 - 1F */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 20 - 27 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 28 - 2F */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 30 - 37 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 38 - 3F */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 40 - 47 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 48 - 4F */
> + 0x00, 0x00, 0x00, 0x00, 'U', 0x00, 'S', 0x00,   /* 50 - 57 */
> + 'B', 0x00, ' ', 0x00, '2', 0x00, '.', 0x00, /* 58 - 5F */
> + '0', 0x00, ' ', 0x00, 'H', 0x00, 'i', 0x00, /* 60 - 67 */
> + '-', 0x00, 'S', 0x00, 'p', 0x00, 'e', 0x00, /* 68 - 6F */
> + 'e', 0x00, 'd', 0x00, ' ', 0x00, 'H', 0x00, /* 70 - 77 */
> + 'u', 0x00, 'b', 0x00, ' ', 0x00, 'C', 0x00, /* 78 - 7F */
> + 'o', 0x00, 'n', 0x00, 't', 0x00, 'r', 0x00, /* 80 - 87 */
> + 'o', 0x00, 'l', 0x00, 'l', 0x00, 'e', 0x00, /* 88 - 8F */
> + 'r', 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  /* 90 - 97 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 98 - 9F */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* A0 - A7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* A8 - AF */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* B0 - B7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* B8 - BF */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* C0 - C7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* C8 - CF */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* D0 - D7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* D8 - DF */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* E0 - E7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* E8 - EF */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* F0 - F7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00  /* F8 - FF */
> +};

What exactly 

[PATCH 0978/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/gadget/legacy/inode.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c 
b/drivers/usb/gadget/legacy/inode.c
index aa3707b..4cedfa3 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -1895,9 +1895,9 @@ static unsigned default_uid;
 static unsigned default_gid;
 static unsigned default_perm = S_IRUSR | S_IWUSR;
 
-module_param (default_uid, uint, 0644);
-module_param (default_gid, uint, 0644);
-module_param (default_perm, uint, 0644);
+module_param (default_uid, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param (default_gid, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param (default_perm, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
 
 static struct inode *
-- 
2.9.2

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

[PATCH 0974/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/atm/ueagle-atm.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index 4333dc5..64b6616 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -573,17 +573,17 @@ static bool sync_wait[NB_MODEM];
 static char *cmv_file[NB_MODEM];
 static int annex[NB_MODEM];
 
-module_param(debug, uint, 0644);
+module_param(debug, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(debug, "module debug level (0=off,1=on,2=verbose)");
-module_param_array(altsetting, uint, NULL, 0644);
+module_param_array(altsetting, uint, NULL, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(altsetting, "alternate setting for incoming traffic: 0=bulk, "
 "1=isoc slowest, ... , 8=isoc fastest (default)");
-module_param_array(sync_wait, bool, NULL, 0644);
+module_param_array(sync_wait, bool, NULL, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(sync_wait, "wait the synchronisation before starting ATM");
-module_param_array(cmv_file, charp, NULL, 0644);
+module_param_array(cmv_file, charp, NULL, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(cmv_file,
"file name with configuration and management variables");
-module_param_array(annex, uint, NULL, 0644);
+module_param_array(annex, uint, NULL, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(annex,
"manually set annex a/b (0=auto, 1=annex a, 2=annex b)");
 
-- 
2.9.2

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

[PATCH 0977/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/gadget/legacy/g_ffs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/legacy/g_ffs.c 
b/drivers/usb/gadget/legacy/g_ffs.c
index f85639e..79f931c 100644
--- a/drivers/usb/gadget/legacy/g_ffs.c
+++ b/drivers/usb/gadget/legacy/g_ffs.c
@@ -79,11 +79,11 @@ static struct usb_device_descriptor gfs_dev_desc = {
 static char *func_names[GFS_MAX_DEVS];
 static unsigned int func_num;
 
-module_param_named(bDeviceClass,gfs_dev_desc.bDeviceClass,byte,   
0644);
+module_param_named(bDeviceClass,gfs_dev_desc.bDeviceClass,byte,   
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(bDeviceClass, "USB Device class");
-module_param_named(bDeviceSubClass, gfs_dev_desc.bDeviceSubClass, byte,   
0644);
+module_param_named(bDeviceSubClass, gfs_dev_desc.bDeviceSubClass, byte,   
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(bDeviceSubClass, "USB Device subclass");
-module_param_named(bDeviceProtocol, gfs_dev_desc.bDeviceProtocol, byte,   
0644);
+module_param_named(bDeviceProtocol, gfs_dev_desc.bDeviceProtocol, byte,   
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(bDeviceProtocol, "USB Device protocol");
 module_param_array_named(functions, func_names, charp, _num, 0);
 MODULE_PARM_DESC(functions, "USB Functions list");
-- 
2.9.2

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

[PATCH 0980/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/gadget/udc/net2280.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index c894b94..58a741f 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -133,7 +133,7 @@ static const struct {
 static ushort fifo_mode;
 
 /* "modprobe net2280 fifo_mode=1" etc */
-module_param(fifo_mode, ushort, 0644);
+module_param(fifo_mode, ushort, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
 /* enable_suspend -- When enabled, the driver will respond to
  * USB suspend requests by powering down the NET2280.  Otherwise,
@@ -143,7 +143,7 @@ module_param(fifo_mode, ushort, 0644);
 static bool enable_suspend;
 
 /* "modprobe net2280 enable_suspend=1" etc */
-module_param(enable_suspend, bool, 0444);
+module_param(enable_suspend, bool, S_IRUSR | S_IRGRP | S_IROTH);
 
 #defineDIR_STRING(bAddress) (((bAddress) & USB_DIR_IN) ? "in" : "out")
 
-- 
2.9.2

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

[PATCH 0982/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/host/fotg210-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 66efa9a..7ad395e 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -4789,7 +4789,7 @@ out_unlock:
return ret;
 }
 
-static DEVICE_ATTR(uframe_periodic_max, 0644, show_uframe_periodic_max,
+static DEVICE_ATTR(uframe_periodic_max, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
show_uframe_periodic_max,
   store_uframe_periodic_max);
 
 static inline int create_sysfs_files(struct fotg210_hcd *fotg210)
-- 
2.9.2

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

[PATCH 0979/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/gadget/udc/net2272.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/udc/net2272.c b/drivers/usb/gadget/udc/net2272.c
index 18f5ebd..74b38f3 100644
--- a/drivers/usb/gadget/udc/net2272.c
+++ b/drivers/usb/gadget/udc/net2272.c
@@ -68,7 +68,7 @@ static const char * const ep_name[] = {
  * If use_dma is disabled, pio will be used instead.
  */
 static bool use_dma = 0;
-module_param(use_dma, bool, 0644);
+module_param(use_dma, bool, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
 /*
  * dma_ep: selects the endpoint for use with dma (1=ep-a, 2=ep-b)
@@ -82,7 +82,7 @@ module_param(use_dma, bool, 0644);
  * terminate prematurely (See NET2272 Errata 630-0213-0101)
  */
 static ushort dma_ep = 1;
-module_param(dma_ep, ushort, 0644);
+module_param(dma_ep, ushort, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
 /*
  * dma_mode: net2272 dma mode setting (see LOCCTL1 definiton):
@@ -91,7 +91,7 @@ module_param(dma_ep, ushort, 0644);
  * mode 2 == Burst mode
  */
 static ushort dma_mode = 2;
-module_param(dma_mode, ushort, 0644);
+module_param(dma_mode, ushort, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 #else
 #define use_dma 0
 #define dma_ep 1
@@ -106,7 +106,7 @@ module_param(dma_mode, ushort, 0644);
  *  mode 3 == ep-a 1k, ep-b disabled, ep-c 512db
  */
 static ushort fifo_mode = 0;
-module_param(fifo_mode, ushort, 0644);
+module_param(fifo_mode, ushort, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
 /*
  * enable_suspend: When enabled, the driver will respond to
@@ -115,7 +115,7 @@ module_param(fifo_mode, ushort, 0644);
  * self-powered devices.  For bus powered devices set this to 1.
  */
 static ushort enable_suspend = 0;
-module_param(enable_suspend, ushort, 0644);
+module_param(enable_suspend, ushort, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
 static void assert_out_naking(struct net2272_ep *ep, const char *where)
 {
-- 
2.9.2

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

[PATCH 0983/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/host/isp1362-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c
index 6cf82ee..2d8c162 100644
--- a/drivers/usb/host/isp1362-hcd.c
+++ b/drivers/usb/host/isp1362-hcd.c
@@ -86,7 +86,7 @@
 
 static int dbg_level;
 #ifdef ISP1362_DEBUG
-module_param(dbg_level, int, 0644);
+module_param(dbg_level, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 #else
 module_param(dbg_level, int, 0);
 #endif
-- 
2.9.2

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

[PATCH 0976/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/core/usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 5e80697..939aee7 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -50,7 +50,7 @@ const char *usbcore_name = "usbcore";
 
 static bool nousb; /* Disable USB when built into kernel image */
 
-module_param(nousb, bool, 0444);
+module_param(nousb, bool, S_IRUSR | S_IRGRP | S_IROTH);
 
 /*
  * for external read access to 
@@ -64,7 +64,7 @@ EXPORT_SYMBOL_GPL(usb_disabled);
 #ifdef CONFIG_PM
 static int usb_autosuspend_delay = 2;  /* Default delay value,
 * in seconds */
-module_param_named(autosuspend, usb_autosuspend_delay, int, 0644);
+module_param_named(autosuspend, usb_autosuspend_delay, int, S_IRUSR | S_IWUSR 
| S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(autosuspend, "default autosuspend delay");
 
 #else
-- 
2.9.2

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

Re: [PACTH v1] cdc-wdm: Clear read pipeline in case of error

[Robert Foss]

On 2016-08-02 09:59 AM, Oliver Neukum wrote:

On Tue, 2016-08-02 at 09:54 -0400, Robert Foss wrote:


On 2016-08-02 08:23 AM, Oliver Neukum wrote:

On Thu, 2016-07-28 at 14:19 -0400, robert.f...@collabora.com wrote:

From: Prathmesh Prabhu 

Implemented queued response handling. This queue is processed every
time the
WDM_READ flag is cleared.

In case of a read error, userspace may not actually read the data,
since the
driver returns an error through wdm_poll. After this, the underlying
device may
attempt to send us more data, but the queue is not processed. While
userspace is
also blocked, because the read error is never cleared.


Could you explain why user space cannot just read more data?
That will clear the error.


Userspace certainly could read more data, but for the case when
userspace doesn't read and clear a potential an error, we still would
like to not be stuck if the device sends more data.

I hope that answers your question, if not I'll try to be more elaborate.


Clear, but why does that require the suppression of an error condition?
errors should always be delivered.


The goal is not to clear the error condition, but that is required to 
not stay stuck.


Maintaining the error condition and not staying stuck if the device 
sends more data are mutually exclusive as far as I understand it.



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

[PATCH 0985/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/musb/musb_core.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index f824336..6182097 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1642,7 +1642,7 @@ EXPORT_SYMBOL_GPL(musb_interrupt);
 static bool use_dma = 1;
 
 /* "modprobe ... use_dma=0" etc */
-module_param(use_dma, bool, 0644);
+module_param(use_dma, bool, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(use_dma, "enable/disable use of DMA");
 
 void musb_dma_completion(struct musb *musb, u8 epnum, u8 transmit)
@@ -1734,7 +1734,7 @@ musb_mode_store(struct device *dev, struct 
device_attribute *attr,
 
return (status == 0) ? n : status;
 }
-static DEVICE_ATTR(mode, 0644, musb_mode_show, musb_mode_store);
+static DEVICE_ATTR(mode, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
musb_mode_show, musb_mode_store);
 
 static ssize_t
 musb_vbus_store(struct device *dev, struct device_attribute *attr,
@@ -1786,7 +1786,7 @@ musb_vbus_show(struct device *dev, struct 
device_attribute *attr, char *buf)
return sprintf(buf, "Vbus %s, timeout %lu msec\n",
vbus ? "on" : "off", val);
 }
-static DEVICE_ATTR(vbus, 0644, musb_vbus_show, musb_vbus_store);
+static DEVICE_ATTR(vbus, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
musb_vbus_show, musb_vbus_store);
 
 /* Gadget drivers can't know that a host is connected so they might want
  * to start SRP, but users can.  This allows userspace to trigger SRP.
@@ -1809,7 +1809,7 @@ musb_srp_store(struct device *dev, struct 
device_attribute *attr,
 
return n;
 }
-static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);
+static DEVICE_ATTR(srp, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, NULL, 
musb_srp_store);
 
 static struct attribute *musb_attributes[] = {
_attr_mode.attr,
-- 
2.9.2

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

[PATCH 0981/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/host/ehci-sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-sysfs.c b/drivers/usb/host/ehci-sysfs.c
index 5216f2b..2029b9c 100644
--- a/drivers/usb/host/ehci-sysfs.c
+++ b/drivers/usb/host/ehci-sysfs.c
@@ -72,7 +72,7 @@ static ssize_t store_companion(struct device *dev,
set_owner(ehci, portnum, new_owner);
return count;
 }
-static DEVICE_ATTR(companion, 0644, show_companion, store_companion);
+static DEVICE_ATTR(companion, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
show_companion, store_companion);
 
 
 /*
@@ -156,7 +156,7 @@ out_unlock:
spin_unlock_irqrestore (>lock, flags);
return ret;
 }
-static DEVICE_ATTR(uframe_periodic_max, 0644, show_uframe_periodic_max, 
store_uframe_periodic_max);
+static DEVICE_ATTR(uframe_periodic_max, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
show_uframe_periodic_max, store_uframe_periodic_max);
 
 
 static inline int create_sysfs_files(struct ehci_hcd *ehci)
-- 
2.9.2

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

[PATCH 0984/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/misc/usbtest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 6b978f0..5e81dc3 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -15,7 +15,7 @@
 /*-*/
 
 static int override_alt = -1;
-module_param_named(alt, override_alt, int, 0644);
+module_param_named(alt, override_alt, int, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(alt, ">= 0 to override altsetting selection");
 static void complicated_callback(struct urb *urb);
 
-- 
2.9.2

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

Re: [PACTH v1] cdc-wdm: Clear read pipeline in case of error

[Oliver Neukum]

On Thu, 2016-07-28 at 14:19 -0400, robert.f...@collabora.com wrote:
> From: Prathmesh Prabhu 
> 
> Implemented queued response handling. This queue is processed every
> time the
> WDM_READ flag is cleared.
> 
> In case of a read error, userspace may not actually read the data,
> since the
> driver returns an error through wdm_poll. After this, the underlying
> device may
> attempt to send us more data, but the queue is not processed. While
> userspace is
> also blocked, because the read error is never cleared.

Could you explain why user space cannot just read more data?
That will clear the error.

Regards
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

Re: [PATCH 0987/1285] Replace numeric parameter like 0444 with macro

[Felipe Balbi]

Baole Ni  writes:

> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the 
> corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
>
> Signed-off-by: Chuansheng Liu 
> Signed-off-by: Baole Ni 
> ---
>  drivers/usb/phy/phy-twl6030-usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/phy/phy-twl6030-usb.c 
> b/drivers/usb/phy/phy-twl6030-usb.c
> index a72e8d6..4ed75c6 100644
> --- a/drivers/usb/phy/phy-twl6030-usb.c
> +++ b/drivers/usb/phy/phy-twl6030-usb.c
> @@ -208,7 +208,7 @@ static ssize_t twl6030_usb_vbus_show(struct device *dev,
>  
>   return ret;
>  }
> -static DEVICE_ATTR(vbus, 0444, twl6030_usb_vbus_show, NULL);
> +static DEVICE_ATTR(vbus, S_IRUSR | S_IRGRP | S_IROTH, twl6030_usb_vbus_show, 
> NULL);

line too long

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 0986/1285] Replace numeric parameter like 0444 with macro

[Felipe Balbi]

Baole Ni  writes:

> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the 
> corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
>
> Signed-off-by: Chuansheng Liu 
> Signed-off-by: Baole Ni 
> ---
>  drivers/usb/phy/phy-tahvo.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-tahvo.c b/drivers/usb/phy/phy-tahvo.c
> index ab5d364..a297861 100644
> --- a/drivers/usb/phy/phy-tahvo.c
> +++ b/drivers/usb/phy/phy-tahvo.c
> @@ -73,7 +73,7 @@ static ssize_t vbus_state_show(struct device *device,
>   struct tahvo_usb *tu = dev_get_drvdata(device);
>   return sprintf(buf, "%s\n", tu->vbus_state ? "on" : "off");
>  }
> -static DEVICE_ATTR(vbus, 0444, vbus_state_show, NULL);
> +static DEVICE_ATTR(vbus, S_IRUSR | S_IRGRP | S_IROTH, vbus_state_show, NULL);
>  
>  static void check_vbus_state(struct tahvo_usb *tu)
>  {
> @@ -318,7 +318,7 @@ static ssize_t otg_mode_store(struct device *device,
>  
>   return r;
>  }
> -static DEVICE_ATTR(otg_mode, 0644, otg_mode_show, otg_mode_store);
> +static DEVICE_ATTR(otg_mode, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
> otg_mode_show, otg_mode_store);

line too long

-- 
balbi


signature.asc
Description: PGP signature

[PATCH 0988/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/wusbcore/cbaf.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/wusbcore/cbaf.c b/drivers/usb/wusbcore/cbaf.c
index da1b872..d37d918 100644
--- a/drivers/usb/wusbcore/cbaf.c
+++ b/drivers/usb/wusbcore/cbaf.c
@@ -355,7 +355,7 @@ static ssize_t cbaf_wusb_chid_store(struct device *dev,
return result;
return size;
 }
-static DEVICE_ATTR(wusb_chid, 0600, cbaf_wusb_chid_show, cbaf_wusb_chid_store);
+static DEVICE_ATTR(wusb_chid, S_IRUSR | S_IWUSR, cbaf_wusb_chid_show, 
cbaf_wusb_chid_store);
 
 static ssize_t cbaf_wusb_host_name_show(struct device *dev,
struct device_attribute *attr,
@@ -381,7 +381,7 @@ static ssize_t cbaf_wusb_host_name_store(struct device *dev,
 
return size;
 }
-static DEVICE_ATTR(wusb_host_name, 0600, cbaf_wusb_host_name_show,
+static DEVICE_ATTR(wusb_host_name, S_IRUSR | S_IWUSR, cbaf_wusb_host_name_show,
 cbaf_wusb_host_name_store);
 
 static ssize_t cbaf_wusb_host_band_groups_show(struct device *dev,
@@ -412,7 +412,7 @@ static ssize_t cbaf_wusb_host_band_groups_store(struct 
device *dev,
return size;
 }
 
-static DEVICE_ATTR(wusb_host_band_groups, 0600,
+static DEVICE_ATTR(wusb_host_band_groups, S_IRUSR | S_IWUSR,
   cbaf_wusb_host_band_groups_show,
   cbaf_wusb_host_band_groups_store);
 
@@ -464,7 +464,7 @@ static ssize_t cbaf_wusb_cdid_store(struct device *dev,
 
return size;
 }
-static DEVICE_ATTR(wusb_cdid, 0600, cbaf_wusb_cdid_show, cbaf_wusb_cdid_store);
+static DEVICE_ATTR(wusb_cdid, S_IRUSR | S_IWUSR, cbaf_wusb_cdid_show, 
cbaf_wusb_cdid_store);
 
 static ssize_t cbaf_wusb_device_band_groups_show(struct device *dev,
 struct device_attribute *attr,
@@ -476,7 +476,7 @@ static ssize_t cbaf_wusb_device_band_groups_show(struct 
device *dev,
return scnprintf(buf, PAGE_SIZE, "0x%04x\n", cbaf->device_band_groups);
 }
 
-static DEVICE_ATTR(wusb_device_band_groups, 0600,
+static DEVICE_ATTR(wusb_device_band_groups, S_IRUSR | S_IWUSR,
   cbaf_wusb_device_band_groups_show,
   NULL);
 
@@ -489,7 +489,7 @@ static ssize_t cbaf_wusb_device_name_show(struct device 
*dev,
 
return scnprintf(buf, PAGE_SIZE, "%s\n", cbaf->device_name);
 }
-static DEVICE_ATTR(wusb_device_name, 0600, cbaf_wusb_device_name_show, NULL);
+static DEVICE_ATTR(wusb_device_name, S_IRUSR | S_IWUSR, 
cbaf_wusb_device_name_show, NULL);
 
 static const struct wusb_cbaf_cc_data cbaf_cc_data_defaults = {
.AssociationTypeId_hdr= WUSB_AR_AssociationTypeId,
@@ -573,7 +573,7 @@ static ssize_t cbaf_wusb_ck_store(struct device *dev,
 
return size;
 }
-static DEVICE_ATTR(wusb_ck, 0600, NULL, cbaf_wusb_ck_store);
+static DEVICE_ATTR(wusb_ck, S_IRUSR | S_IWUSR, NULL, cbaf_wusb_ck_store);
 
 static struct attribute *cbaf_dev_attrs[] = {
_attr_wusb_host_name.attr,
-- 
2.9.2

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

[PATCH 0989/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/wusbcore/dev-sysfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/wusbcore/dev-sysfs.c b/drivers/usb/wusbcore/dev-sysfs.c
index 415b140..bd15caa 100644
--- a/drivers/usb/wusbcore/dev-sysfs.c
+++ b/drivers/usb/wusbcore/dev-sysfs.c
@@ -53,7 +53,7 @@ static ssize_t wusb_disconnect_store(struct device *dev,
wusbhc_put(wusbhc);
return size;
 }
-static DEVICE_ATTR(wusb_disconnect, 0200, NULL, wusb_disconnect_store);
+static DEVICE_ATTR(wusb_disconnect, S_IWUSR, NULL, wusb_disconnect_store);
 
 static ssize_t wusb_cdid_show(struct device *dev,
  struct device_attribute *attr, char *buf)
@@ -69,7 +69,7 @@ static ssize_t wusb_cdid_show(struct device *dev,
wusb_dev_put(wusb_dev);
return result + 1;
 }
-static DEVICE_ATTR(wusb_cdid, 0444, wusb_cdid_show, NULL);
+static DEVICE_ATTR(wusb_cdid, S_IRUSR | S_IRGRP | S_IROTH, wusb_cdid_show, 
NULL);
 
 static ssize_t wusb_ck_store(struct device *dev,
 struct device_attribute *attr,
@@ -105,7 +105,7 @@ static ssize_t wusb_ck_store(struct device *dev,
wusbhc_put(wusbhc);
return result < 0 ? result : size;
 }
-static DEVICE_ATTR(wusb_ck, 0200, NULL, wusb_ck_store);
+static DEVICE_ATTR(wusb_ck, S_IWUSR, NULL, wusb_ck_store);
 
 static struct attribute *wusb_dev_attrs[] = {
_attr_wusb_disconnect.attr,
-- 
2.9.2

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

Re: [PATCH 0984/1285] Replace numeric parameter like 0444 with macro

[Marcel Holtmann]

Hi Felipe,

>> I find that the developers often just specified the numeric value
>> when calling a macro which is defined with a parameter for access permission.
>> As we know, these numeric value for access permission have had the 
>> corresponding macro,
>> and that using macro can improve the robustness and readability of the code,
>> thus, I suggest replacing the numeric parameter with the macro.
>> 
>> Signed-off-by: Chuansheng Liu 
>> Signed-off-by: Baole Ni 
>> ---
>> drivers/usb/misc/usbtest.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
>> index 6b978f0..5e81dc3 100644
>> --- a/drivers/usb/misc/usbtest.c
>> +++ b/drivers/usb/misc/usbtest.c
>> @@ -15,7 +15,7 @@
>> /*-*/
>> 
>> static int override_alt = -1;
>> -module_param_named(alt, override_alt, int, 0644);
>> +module_param_named(alt, override_alt, int, S_IRUSR | S_IWUSR | S_IRGRP | 
>> S_IROTH);
> 
> line too long. You need to run this series through scripts/checkpatch.pl

please don't give them any ideas. Next thing you know and another 1285 patch 
bomb is coming our way.

Regards

Marcel

--
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: [PACTH v1] cdc-wdm: Clear read pipeline in case of error

[Robert Foss]

On 2016-08-02 08:23 AM, Oliver Neukum wrote:

On Thu, 2016-07-28 at 14:19 -0400, robert.f...@collabora.com wrote:

From: Prathmesh Prabhu 

Implemented queued response handling. This queue is processed every
time the
WDM_READ flag is cleared.

In case of a read error, userspace may not actually read the data,
since the
driver returns an error through wdm_poll. After this, the underlying
device may
attempt to send us more data, but the queue is not processed. While
userspace is
also blocked, because the read error is never cleared.


Could you explain why user space cannot just read more data?
That will clear the error.


Userspace certainly could read more data, but for the case when 
userspace doesn't read and clear a potential an error, we still would 
like to not be stuck if the device sends more data.


I hope that answers your question, if not I'll try to be more elaborate.



Regards
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

Re: [PATCH 0984/1285] Replace numeric parameter like 0444 with macro

[Marcel Holtmann]

Hi Felipe,

>> I find that the developers often just specified the numeric value
>> when calling a macro which is defined with a parameter for access permission.
>> As we know, these numeric value for access permission have had the 
>> corresponding macro,
>> and that using macro can improve the robustness and readability of the code,
>> thus, I suggest replacing the numeric parameter with the macro.
>> 
>> Signed-off-by: Chuansheng Liu 
>> Signed-off-by: Baole Ni 
>> ---
>> drivers/usb/misc/usbtest.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
>> index 6b978f0..5e81dc3 100644
>> --- a/drivers/usb/misc/usbtest.c
>> +++ b/drivers/usb/misc/usbtest.c
>> @@ -15,7 +15,7 @@
>> /*-*/
>> 
>> static int override_alt = -1;
>> -module_param_named(alt, override_alt, int, 0644);
>> +module_param_named(alt, override_alt, int, S_IRUSR | S_IWUSR | S_IRGRP | 
>> S_IROTH);
> 
> line too long. You need to run this series through scripts/checkpatch.pl

please don't give them any ideas. Next thing you know and another 1285 patch 
bomb is coming our way.

Regards

Marcel

--
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 0984/1285] Replace numeric parameter like 0444 with macro

[Felipe Balbi]

Baole Ni  writes:

> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the 
> corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
>
> Signed-off-by: Chuansheng Liu 
> Signed-off-by: Baole Ni 
> ---
>  drivers/usb/misc/usbtest.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> index 6b978f0..5e81dc3 100644
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -15,7 +15,7 @@
>  /*-*/
>  
>  static int override_alt = -1;
> -module_param_named(alt, override_alt, int, 0644);
> +module_param_named(alt, override_alt, int, S_IRUSR | S_IWUSR | S_IRGRP | 
> S_IROTH);

line too long. You need to run this series through scripts/checkpatch.pl

-- 
balbi


signature.asc
Description: PGP signature

Re: [PACTH v1] cdc-wdm: Clear read pipeline in case of error

[Oliver Neukum]

On Tue, 2016-08-02 at 09:54 -0400, Robert Foss wrote:
> 
> On 2016-08-02 08:23 AM, Oliver Neukum wrote:
> > On Thu, 2016-07-28 at 14:19 -0400, robert.f...@collabora.com wrote:
> >> From: Prathmesh Prabhu 
> >>
> >> Implemented queued response handling. This queue is processed every
> >> time the
> >> WDM_READ flag is cleared.
> >>
> >> In case of a read error, userspace may not actually read the data,
> >> since the
> >> driver returns an error through wdm_poll. After this, the underlying
> >> device may
> >> attempt to send us more data, but the queue is not processed. While
> >> userspace is
> >> also blocked, because the read error is never cleared.
> >
> > Could you explain why user space cannot just read more data?
> > That will clear the error.
> 
> Userspace certainly could read more data, but for the case when 
> userspace doesn't read and clear a potential an error, we still would 
> like to not be stuck if the device sends more data.
> 
> I hope that answers your question, if not I'll try to be more elaborate.

Clear, but why does that require the suppression of an error condition?
errors should always be delivered.

Regards
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

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Michal Hocko]

On Tue 02-08-16 13:44:48, Oliver Neukum wrote:
> On Tue, 2016-08-02 at 13:34 +0200, Michal Hocko wrote:
> > On Tue 02-08-16 12:03:23, Oliver Neukum wrote:
> > > On Tue, 2016-08-02 at 10:18 +0200, Michal Hocko wrote:
> > > > On Tue 02-08-16 10:06:12, Oliver Neukum wrote:
> > > > > On Mon, 2016-08-01 at 10:20 -0400, Tejun Heo wrote:
> > > > > > Hello,
> > > > > 
> > > > > > > If any real IO depends on those devices then this is not 
> > > > > > > sufficient and
> > > > > > > they need some form of guarantee for progress (aka mempool).
> > > > > > 
> > > > > > Oliver, Alan, what do you think?  If USB itself can't operate 
> > > > > > without
> > > > > > allocating memory during transactions, whatever USB storage drivers
> > > > > 
> > > > > It cannot. The IO must be described to the hardware with a data
> > > > > structure in memory.
> > > > > 
> > > > > > are doing isn't all that meaningful.  Can we proceed with the
> > > > > > workqueue patches?  Also, it could be that the only thing GFP_NOIO 
> > > > > > and
> > > > > > GFP_ATOMIC are doing is increasing the chance of IO failures under
> > > > > > memory pressure.  Maybe it'd be a good idea to reconsider the
> > > > > > approach?
> > > > > 
> > > > > We had actual deadlocks with GFP_KERNEL. It seems to me that the SCSI
> > > > > layer can deal with IO that cannot be completed due to a lack of 
> > > > > memory
> > > > > at least somewhat, but a deadlock within a driver would obviously be
> > > > > deadly. So I don't think that mempools would remove the need for
> > > > > GFP_NOIO as there are places in usbcore we cannot enter the page
> > > > > laundering path from. They are an additional need.
> > > > 
> > > > OK, I guess there is some misunderstanding here. I believe that Tejun
> > > > wasn't arguing to drop GFP_NOIO. It might be really needed for the dead
> > > > lock avoidance. No question about that. The whole point is that
> > > > WQ_RECLAIM might be completely pointless because a rescuer wouldn't help
> > > > much if the work item would do GFP_NOIO and get stuck in the page
> > > > allocator.
> > > 
> > > But that can be a problem only if the items on the work queue are
> > > actually run and without WQ_MEM_RECLAIM that guarantee cannot be made.
> > > We can deal with failures of memory allocation. But the requests
> > > must actually terminate.
> > 
> > I think you have missed my point. So let me ask differently. What is the
> > difference between your work item not running at all or looping
> > endlessly with GFP_NOIO inside the page allocator? If that particular
> > work item is necessary for the further progress then the system is
> > screwed one way or another.
> 
> The key difference is that I could give the right parameters to the
> kmalloc() call. If it doesn't run, I am surely screwed. Thus I conclude
> that WQ_RECLAIM needs to be set.

Just to make sure I understand. So you will never call kmalloc with
__GFP_DIRECT_RECLAIM from the rescuer context, right?
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Oliver Neukum]

On Tue, 2016-08-02 at 14:48 +0200, Michal Hocko wrote:
> On Tue 02-08-16 13:44:48, Oliver Neukum wrote:
> > On Tue, 2016-08-02 at 13:34 +0200, Michal Hocko wrote:
> > > On Tue 02-08-16 12:03:23, Oliver Neukum wrote:
> > > > On Tue, 2016-08-02 at 10:18 +0200, Michal Hocko wrote:
> > > > > On Tue 02-08-16 10:06:12, Oliver Neukum wrote:
> > > > > > On Mon, 2016-08-01 at 10:20 -0400, Tejun Heo wrote:
> > > > > > > Hello,
> > > > > > 
> > > > > > > > If any real IO depends on those devices then this is not 
> > > > > > > > sufficient and
> > > > > > > > they need some form of guarantee for progress (aka mempool).
> > > > > > > 
> > > > > > > Oliver, Alan, what do you think?  If USB itself can't operate 
> > > > > > > without
> > > > > > > allocating memory during transactions, whatever USB storage 
> > > > > > > drivers
> > > > > > 
> > > > > > It cannot. The IO must be described to the hardware with a data
> > > > > > structure in memory.
> > > > > > 
> > > > > > > are doing isn't all that meaningful.  Can we proceed with the
> > > > > > > workqueue patches?  Also, it could be that the only thing 
> > > > > > > GFP_NOIO and
> > > > > > > GFP_ATOMIC are doing is increasing the chance of IO failures under
> > > > > > > memory pressure.  Maybe it'd be a good idea to reconsider the
> > > > > > > approach?
> > > > > > 
> > > > > > We had actual deadlocks with GFP_KERNEL. It seems to me that the 
> > > > > > SCSI
> > > > > > layer can deal with IO that cannot be completed due to a lack of 
> > > > > > memory
> > > > > > at least somewhat, but a deadlock within a driver would obviously be
> > > > > > deadly. So I don't think that mempools would remove the need for
> > > > > > GFP_NOIO as there are places in usbcore we cannot enter the page
> > > > > > laundering path from. They are an additional need.
> > > > > 
> > > > > OK, I guess there is some misunderstanding here. I believe that Tejun
> > > > > wasn't arguing to drop GFP_NOIO. It might be really needed for the 
> > > > > dead
> > > > > lock avoidance. No question about that. The whole point is that
> > > > > WQ_RECLAIM might be completely pointless because a rescuer wouldn't 
> > > > > help
> > > > > much if the work item would do GFP_NOIO and get stuck in the page
> > > > > allocator.
> > > > 
> > > > But that can be a problem only if the items on the work queue are
> > > > actually run and without WQ_MEM_RECLAIM that guarantee cannot be made.
> > > > We can deal with failures of memory allocation. But the requests
> > > > must actually terminate.
> > > 
> > > I think you have missed my point. So let me ask differently. What is the
> > > difference between your work item not running at all or looping
> > > endlessly with GFP_NOIO inside the page allocator? If that particular
> > > work item is necessary for the further progress then the system is
> > > screwed one way or another.
> > 
> > The key difference is that I could give the right parameters to the
> > kmalloc() call. If it doesn't run, I am surely screwed. Thus I conclude
> > that WQ_RECLAIM needs to be set.
> 
> Just to make sure I understand. So you will never call kmalloc with
> __GFP_DIRECT_RECLAIM from the rescuer context, right?

Apparently if that is the requirement USB will have to define its own
set of flags to use in such contexts. But still the calls as currently
executed work. Taking away WQ_MEM_RECLAIM would create danger of
introducing a regression. The issue with __GFP_DIRECT_RECLAIM already
exists and can be fixed.

Regards
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

Re: [PATCH 0982/1285] Replace numeric parameter like 0444 with macro

[Julia Lawall]

On Tue, 2 Aug 2016, Baole Ni wrote:

> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the 
> corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
>
> Signed-off-by: Chuansheng Liu 
> Signed-off-by: Baole Ni 
> ---
>  drivers/usb/host/fotg210-hcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
> index 66efa9a..7ad395e 100644
> --- a/drivers/usb/host/fotg210-hcd.c
> +++ b/drivers/usb/host/fotg210-hcd.c
> @@ -4789,7 +4789,7 @@ out_unlock:
>   return ret;
>  }
>
> -static DEVICE_ATTR(uframe_periodic_max, 0644, show_uframe_periodic_max,
> +static DEVICE_ATTR(uframe_periodic_max, S_IRUSR | S_IWUSR | S_IRGRP | 
> S_IROTH, show_uframe_periodic_max,

Over 80 chars.

julia

>  store_uframe_periodic_max);
>
>  static inline int create_sysfs_files(struct fotg210_hcd *fotg210)
> --
> 2.9.2
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0987/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/phy/phy-twl6030-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-twl6030-usb.c 
b/drivers/usb/phy/phy-twl6030-usb.c
index a72e8d6..4ed75c6 100644
--- a/drivers/usb/phy/phy-twl6030-usb.c
+++ b/drivers/usb/phy/phy-twl6030-usb.c
@@ -208,7 +208,7 @@ static ssize_t twl6030_usb_vbus_show(struct device *dev,
 
return ret;
 }
-static DEVICE_ATTR(vbus, 0444, twl6030_usb_vbus_show, NULL);
+static DEVICE_ATTR(vbus, S_IRUSR | S_IRGRP | S_IROTH, twl6030_usb_vbus_show, 
NULL);
 
 static irqreturn_t twl6030_usb_irq(int irq, void *_twl)
 {
-- 
2.9.2

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

[PATCH 0986/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/usb/phy/phy-tahvo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy-tahvo.c b/drivers/usb/phy/phy-tahvo.c
index ab5d364..a297861 100644
--- a/drivers/usb/phy/phy-tahvo.c
+++ b/drivers/usb/phy/phy-tahvo.c
@@ -73,7 +73,7 @@ static ssize_t vbus_state_show(struct device *device,
struct tahvo_usb *tu = dev_get_drvdata(device);
return sprintf(buf, "%s\n", tu->vbus_state ? "on" : "off");
 }
-static DEVICE_ATTR(vbus, 0444, vbus_state_show, NULL);
+static DEVICE_ATTR(vbus, S_IRUSR | S_IRGRP | S_IROTH, vbus_state_show, NULL);
 
 static void check_vbus_state(struct tahvo_usb *tu)
 {
@@ -318,7 +318,7 @@ static ssize_t otg_mode_store(struct device *device,
 
return r;
 }
-static DEVICE_ATTR(otg_mode, 0644, otg_mode_show, otg_mode_store);
+static DEVICE_ATTR(otg_mode, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, 
otg_mode_show, otg_mode_store);
 
 static struct attribute *tahvo_attributes[] = {
_attr_vbus.attr,
-- 
2.9.2

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

[PATCH 0696/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/mmc/host/vub300.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index 1e819f9..9abdc44 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -224,33 +224,33 @@ enum SD_RESPONSE_TYPE {
 #define REG(c) (0x01 & (c->arg>>9))
 
 static bool limit_speed_to_24_MHz;
-module_param(limit_speed_to_24_MHz, bool, 0644);
+module_param(limit_speed_to_24_MHz, bool, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(limit_speed_to_24_MHz, "Limit Max SDIO Clock Speed to 24 
MHz");
 
 static bool pad_input_to_usb_pkt;
-module_param(pad_input_to_usb_pkt, bool, 0644);
+module_param(pad_input_to_usb_pkt, bool, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(pad_input_to_usb_pkt,
 "Pad USB data input transfers to whole USB Packet");
 
 static bool disable_offload_processing;
-module_param(disable_offload_processing, bool, 0644);
+module_param(disable_offload_processing, bool, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(disable_offload_processing, "Disable Offload Processing");
 
 static bool force_1_bit_data_xfers;
-module_param(force_1_bit_data_xfers, bool, 0644);
+module_param(force_1_bit_data_xfers, bool, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(force_1_bit_data_xfers,
 "Force SDIO Data Transfers to 1-bit Mode");
 
 static bool force_polling_for_irqs;
-module_param(force_polling_for_irqs, bool, 0644);
+module_param(force_polling_for_irqs, bool, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(force_polling_for_irqs, "Force Polling for SDIO interrupts");
 
 static int firmware_irqpoll_timeout = 1024;
-module_param(firmware_irqpoll_timeout, int, 0644);
+module_param(firmware_irqpoll_timeout, int, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(firmware_irqpoll_timeout, "VUB300 firmware irqpoll timeout");
 
 static int force_max_req_size = 128;
-module_param(force_max_req_size, int, 0644);
+module_param(force_max_req_size, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(force_max_req_size, "set max request size in kBytes");
 
 #ifdef SMSC_DEVELOPMENT_BOARD
@@ -259,7 +259,7 @@ static int firmware_rom_wait_states = 0x04;
 static int firmware_rom_wait_states = 0x1C;
 #endif
 
-module_param(firmware_rom_wait_states, int, 0644);
+module_param(firmware_rom_wait_states, int, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(firmware_rom_wait_states,
 "ROM wait states byte=RRRIIEEE (Reserved Internal External)");
 
-- 
2.9.2

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

RE: USB vulnerability

[David Laight]

From: Felipe Balbi
> Sent: 29 July 2016 01:30
...
> > -   maxp = usb_endpoint_maxp(>desc) & 0x07ff;
> 
> usb_endpoint_maxp() should probably be updated to return only maximum
> packet size. Then we would need to introduce usb_endpoint_mult() or
> something along those lines to take care of the other valid bits.

Safest to change the name at the same time...

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: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Michal Hocko]

On Tue 02-08-16 12:03:23, Oliver Neukum wrote:
> On Tue, 2016-08-02 at 10:18 +0200, Michal Hocko wrote:
> > On Tue 02-08-16 10:06:12, Oliver Neukum wrote:
> > > On Mon, 2016-08-01 at 10:20 -0400, Tejun Heo wrote:
> > > > Hello,
> > > 
> > > > > If any real IO depends on those devices then this is not sufficient 
> > > > > and
> > > > > they need some form of guarantee for progress (aka mempool).
> > > > 
> > > > Oliver, Alan, what do you think?  If USB itself can't operate without
> > > > allocating memory during transactions, whatever USB storage drivers
> > > 
> > > It cannot. The IO must be described to the hardware with a data
> > > structure in memory.
> > > 
> > > > are doing isn't all that meaningful.  Can we proceed with the
> > > > workqueue patches?  Also, it could be that the only thing GFP_NOIO and
> > > > GFP_ATOMIC are doing is increasing the chance of IO failures under
> > > > memory pressure.  Maybe it'd be a good idea to reconsider the
> > > > approach?
> > > 
> > > We had actual deadlocks with GFP_KERNEL. It seems to me that the SCSI
> > > layer can deal with IO that cannot be completed due to a lack of memory
> > > at least somewhat, but a deadlock within a driver would obviously be
> > > deadly. So I don't think that mempools would remove the need for
> > > GFP_NOIO as there are places in usbcore we cannot enter the page
> > > laundering path from. They are an additional need.
> > 
> > OK, I guess there is some misunderstanding here. I believe that Tejun
> > wasn't arguing to drop GFP_NOIO. It might be really needed for the dead
> > lock avoidance. No question about that. The whole point is that
> > WQ_RECLAIM might be completely pointless because a rescuer wouldn't help
> > much if the work item would do GFP_NOIO and get stuck in the page
> > allocator.
> 
> But that can be a problem only if the items on the work queue are
> actually run and without WQ_MEM_RECLAIM that guarantee cannot be made.
> We can deal with failures of memory allocation. But the requests
> must actually terminate.

I think you have missed my point. So let me ask differently. What is the
difference between your work item not running at all or looping
endlessly with GFP_NOIO inside the page allocator? If that particular
work item is necessary for the further progress then the system is
screwed one way or another.
-- 
Michal Hocko
SUSE Labs
--
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] usb: dwc3: core: allow device to runtime_suspend several times

[Felipe Balbi]

After going through runtime_suspend/runtime_resume
cycle once we would be left with an unbalanced
pm_runtime_get() call. Fix that by making sure that
we try to suspend right after resuming so things are
balanced and device can runtime_suspend again.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 946643157b78..35d092456bec 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1192,6 +1192,7 @@ static int dwc3_runtime_resume(struct device *dev)
}
 
pm_runtime_mark_last_busy(dev);
+   pm_runtime_put(dev);
 
return 0;
 }
-- 
2.9.1

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

[PATCH 0232/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/hid/usbhid/hid-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index ae83af6..4491aa0 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -50,16 +50,16 @@
  */
 
 static unsigned int hid_mousepoll_interval;
-module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
+module_param_named(mousepoll, hid_mousepoll_interval, uint, S_IRUSR | S_IWUSR 
| S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(mousepoll, "Polling interval of mice");
 
 static unsigned int ignoreled;
-module_param_named(ignoreled, ignoreled, uint, 0644);
+module_param_named(ignoreled, ignoreled, uint, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IROTH);
 MODULE_PARM_DESC(ignoreled, "Autosuspend with active leds");
 
 /* Quirks specified at module load time */
 static char *quirks_param[MAX_USBHID_BOOT_QUIRKS];
-module_param_array_named(quirks, quirks_param, charp, NULL, 0444);
+module_param_array_named(quirks, quirks_param, charp, NULL, S_IRUSR | S_IRGRP 
| S_IROTH);
 MODULE_PARM_DESC(quirks, "Add/modify USB HID quirks by specifying "
" quirks=vendorID:productID:quirks"
" where vendorID, productID, and quirks are all in"
-- 
2.9.2

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

Re: [PATCH] musb: omap2430: do not assume balanced enable()/disable()

[Tony Lindgren]

* Andreas Kemnade  [160729 11:14]:
> The code assumes that omap2430_musb_enable() and
> omap2430_musb_disable() is called in a balanced way. The
> That fact is broken by the fact that musb_init_controller() calls
> musb_platform_disable() to switch from unknown state to off state.

OK, some spelling issues with the above paragraph though :)

> That means that phy_power_off() is called first so that
> phy->power_count gets -1 and the phy is not enabled on phy_power_on().
> In the probably common case of using the phy_twl4030, that
> prevents also charging the battery and so makes further
> kernel debugging hard.

Is this with v4.7 kernel? Also, care to describe how you hit this
and on which hardware? Just wondering..

> The patch prevents phy_power_off() from being called when
> it is already off.

OK

Regards,

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

Re: [PATCH] usb: ehci: change order of register cleanup during shutdown

[Marc Ohlf]

On Mon, Aug 01, 2016 at 06:07:17PM +0200, Alan Stern wrote:
> On Mon, 1 Aug 2016, Marc Ohlf wrote:
> 
> > In ehci_turn_off_all_ports() all EHCI port register bits
> > (except the PORT_RWC_BITS) are set to zero.
> 
> Even the PORT_WRC_BITS are set to 0.  Oddly enough, the way to set 
> those bits to 0 is to write a 1 to them (see Table 2-16 in the EHCI 
> spec).

Thanks for correction, i will improve that in an v2.

> 
> > On some hardware, this can lead to an system hang,
> > when ehci_port_power() accesses the already cleaned registers.
> > 
> > This patch changes the order of cleanup.
> > First call ehci_port_power() which respects the current bits in
> > port status registers
> > and afterwards cleanup the hard way by setting everything else to zero.
> > 
> > Signed-off-by: Marc Ohlf 
> > ---
> >  drivers/usb/host/ehci-hcd.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> > index a962b89..1e5f529 100644
> > --- a/drivers/usb/host/ehci-hcd.c
> > +++ b/drivers/usb/host/ehci-hcd.c
> > @@ -332,11 +332,11 @@ static void ehci_turn_off_all_ports(struct ehci_hcd 
> > *ehci)
> > int port = HCS_N_PORTS(ehci->hcs_params);
> >  
> > while (port--) {
> > -   ehci_writel(ehci, PORT_RWC_BITS,
> > -   >regs->port_status[port]);
> > spin_unlock_irq(>lock);
> > ehci_port_power(ehci, port, false);
> > spin_lock_irq(>lock);
> > +   ehci_writel(ehci, PORT_RWC_BITS,
> > +   >regs->port_status[port]);
> > }
> >  }
> 

I'm new to the mailing lists, so I've some questions.

> Acked-by: Alan Stern 

I should include your Ack in my v2 patch signoff area, right?

> 
> This should be marked for the -stable kernels as well.

Marking this for -stable kernels means cc it to stable mailing list, right?

> 
> Alan Stern
> 
> 
> 

Marc Ohlf
--
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: usbtmc: vendor specific i/o

[Ladislav Michl]

On Tue, Aug 02, 2016 at 11:25:22AM +0200, dave penkler wrote:
>For supportability I would recommend to set the commands depending on the
>recognised HW, not with aA  ioctl. See rigol quirk. What are the device
>and manufacturer ids to which these vendor extensions apply?
>/dave

This is a custom device... If I understand USBTMC spec corectly, it is not
either "normal" message or vendor specific message, but both can happily
coexist, so I wouldn't let it depend on quirks. Once you are going to send
vendor specific message, you need to know how such a message looks like
anyway, so any quirk based magic is probably not going to help.
Also it seems both NI and IVI libs are using similar (selecting MsgID)
approach.

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

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Oliver Neukum]

On Tue, 2016-08-02 at 10:18 +0200, Michal Hocko wrote:
> On Tue 02-08-16 10:06:12, Oliver Neukum wrote:
> > On Mon, 2016-08-01 at 10:20 -0400, Tejun Heo wrote:
> > > Hello,
> > 
> > > > If any real IO depends on those devices then this is not sufficient and
> > > > they need some form of guarantee for progress (aka mempool).
> > > 
> > > Oliver, Alan, what do you think?  If USB itself can't operate without
> > > allocating memory during transactions, whatever USB storage drivers
> > 
> > It cannot. The IO must be described to the hardware with a data
> > structure in memory.
> > 
> > > are doing isn't all that meaningful.  Can we proceed with the
> > > workqueue patches?  Also, it could be that the only thing GFP_NOIO and
> > > GFP_ATOMIC are doing is increasing the chance of IO failures under
> > > memory pressure.  Maybe it'd be a good idea to reconsider the
> > > approach?
> > 
> > We had actual deadlocks with GFP_KERNEL. It seems to me that the SCSI
> > layer can deal with IO that cannot be completed due to a lack of memory
> > at least somewhat, but a deadlock within a driver would obviously be
> > deadly. So I don't think that mempools would remove the need for
> > GFP_NOIO as there are places in usbcore we cannot enter the page
> > laundering path from. They are an additional need.
> 
> OK, I guess there is some misunderstanding here. I believe that Tejun
> wasn't arguing to drop GFP_NOIO. It might be really needed for the dead
> lock avoidance. No question about that. The whole point is that
> WQ_RECLAIM might be completely pointless because a rescuer wouldn't help
> much if the work item would do GFP_NOIO and get stuck in the page
> allocator.

But that can be a problem only if the items on the work queue are
actually run and without WQ_MEM_RECLAIM that guarantee cannot be made.
We can deal with failures of memory allocation. But the requests
must actually terminate.

Regards
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 0/1] USB: serial: option: add support for Telit LE920A4

[Daniele Palmas]

This patch adds a set of compositions for Telit LE920A4.

Compositions in short are:

0x1207: tty + tty
0x1208: tty + adb + tty + tty
0x1211: tty + adb + ecm
0x1212: tty + adb
0x1213: ecm + tty
0x1214: tty + adb + ecm + tty

telit_le922_blacklist_usbcfg3 is reused for compositions 0x1211
and 0x1214 due to the same interfaces positions.

Following lsusb output for the compositions (sorry for the long list):

Bus 003 Device 014: ID 1bc7:1207 Telit Wireless Solutions 
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass0 (Defined at Interface level)
  bDeviceSubClass 0 
  bDeviceProtocol 0 
  bMaxPacketSize064
  idVendor   0x1bc7 Telit Wireless Solutions
  idProduct  0x1207 
  bcdDevice3.18
  iManufacturer   1 Android
  iProduct2 Android
  iSerial 3 0123456789ABCDEF
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength  107
bNumInterfaces  2
bConfigurationValue 1
iConfiguration  0 
bmAttributes 0xa0
  (Bus Powered)
  Remote Wakeup
MaxPower  500mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   3
  bInterfaceClass   255 Vendor Specific Class
  bInterfaceSubClass  0 
  bInterfaceProtocol  0 
  iInterface  0 
  ** UNRECOGNIZED:  05 24 00 10 01
  ** UNRECOGNIZED:  05 24 01 00 00
  ** UNRECOGNIZED:  04 24 02 02
  ** UNRECOGNIZED:  05 24 06 00 00
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82  EP 2 IN
bmAttributes3
  Transfer TypeInterrupt
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x000a  1x 10 bytes
bInterval   9
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01  EP 1 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber1
  bAlternateSetting   0
  bNumEndpoints   3
  bInterfaceClass   255 Vendor Specific Class
  bInterfaceSubClass  0 
  bInterfaceProtocol  0 
  iInterface  0 
  ** UNRECOGNIZED:  05 24 00 10 01
  ** UNRECOGNIZED:  05 24 01 00 00
  ** UNRECOGNIZED:  04 24 02 02
  ** UNRECOGNIZED:  05 24 06 00 00
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x84  EP 4 IN
bmAttributes3
  Transfer TypeInterrupt
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x000a  1x 10 bytes
bInterval   9
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83  EP 3 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02  EP 2 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0200  1x 512 bytes
bInterval   0
Device Qualifier (for other device speed):
  bLength10
  bDescriptorType 6
  bcdUSB   2.00
  bDeviceClass0 (Defined at Interface level)
  bDeviceSubClass 0 
  bDeviceProtocol 0 
  bMaxPacketSize064
  bNumConfigurations  1
Device Status: 0x
  (Bus Powered)

***

Bus 003 Device 015: ID 1bc7:1208 Telit Wireless Solutions 
Device Descriptor:
  bLength18
  

[PATCH 1/1] USB: serial: option: add support for Telit LE920A4

[Daniele Palmas]

This patch adds a set of compositions for Telit LE920A4.

Compositions in short are:

0x1207: tty + tty
0x1208: tty + adb + tty + tty
0x1211: tty + adb + ecm
0x1212: tty + adb
0x1213: ecm + tty
0x1214: tty + adb + ecm + tty

telit_le922_blacklist_usbcfg3 is reused for compositions 0x1211
and 0x1214 due to the same interfaces positions.

Signed-off-by: Daniele Palmas 
---
 drivers/usb/serial/option.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 8e07536..7284b2b 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -274,6 +274,12 @@ static void option_instat_callback(struct urb *urb);
 #define TELIT_PRODUCT_LE9200x1200
 #define TELIT_PRODUCT_LE9100x1201
 #define TELIT_PRODUCT_LE910_USBCFG40x1206
+#define TELIT_PRODUCT_LE920A4_1207 0x1207
+#define TELIT_PRODUCT_LE920A4_1208 0x1208
+#define TELIT_PRODUCT_LE920A4_1211 0x1211
+#define TELIT_PRODUCT_LE920A4_1212 0x1212
+#define TELIT_PRODUCT_LE920A4_1213 0x1213
+#define TELIT_PRODUCT_LE920A4_1214 0x1214
 
 /* ZTE PRODUCTS */
 #define ZTE_VENDOR_ID  0x19d2
@@ -638,6 +644,11 @@ static const struct option_blacklist_info 
telit_le922_blacklist_usbcfg3 = {
.reserved = BIT(1) | BIT(2) | BIT(3),
 };
 
+static const struct option_blacklist_info telit_le920a4_blacklist_1 = {
+   .sendsetup = BIT(0),
+   .reserved = BIT(1),
+};
+
 static const struct option_blacklist_info cinterion_rmnet2_blacklist = {
.reserved = BIT(4) | BIT(5),
 };
@@ -1203,6 +1214,16 @@ static const struct usb_device_id option_ids[] = {
.driver_info = (kernel_ulong_t)_le922_blacklist_usbcfg3 },
{ USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_LE920),
.driver_info = (kernel_ulong_t)_le920_blacklist },
+   { USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_LE920A4_1207) },
+   { USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_LE920A4_1208),
+   .driver_info = (kernel_ulong_t)_le920a4_blacklist_1 },
+   { USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_LE920A4_1211),
+   .driver_info = (kernel_ulong_t)_le922_blacklist_usbcfg3 },
+   { USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_LE920A4_1212),
+   .driver_info = (kernel_ulong_t)_le920a4_blacklist_1 },
+   { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 
TELIT_PRODUCT_LE920A4_1213, 0xff) },
+   { USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_LE920A4_1214),
+   .driver_info = (kernel_ulong_t)_le922_blacklist_usbcfg3 },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MF622, 0xff, 
0xff, 0xff) }, /* ZTE WCDMA products */
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x0002, 0xff, 0xff, 
0xff),
.driver_info = (kernel_ulong_t)_intf1_blacklist },
-- 
2.7.4

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

Re: [PATCH v2 1/3] dt: bindings: add bindings for Allwinner A64 usb phy

[Hans de Goede]

Hi,

Series looks good to me and is:

Reviewed-by: Hans de Goede 

Regards,

Hans


On 01-08-16 18:34, Icenowy Zheng wrote:

Signed-off-by: Icenowy Zheng 
---
 Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt 
b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 95736d7..287150d 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -10,6 +10,7 @@ Required properties:
   * allwinner,sun8i-a23-usb-phy
   * allwinner,sun8i-a33-usb-phy
   * allwinner,sun8i-h3-usb-phy
+  * allwinner,sun50i-a64-usb-phy
 - reg : a list of offset + length pairs
 - reg-names :
   * "phy_ctrl"


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

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Michal Hocko]

On Tue 02-08-16 10:06:12, Oliver Neukum wrote:
> On Mon, 2016-08-01 at 10:20 -0400, Tejun Heo wrote:
> > Hello,
> 
> > > If any real IO depends on those devices then this is not sufficient and
> > > they need some form of guarantee for progress (aka mempool).
> > 
> > Oliver, Alan, what do you think?  If USB itself can't operate without
> > allocating memory during transactions, whatever USB storage drivers
> 
> It cannot. The IO must be described to the hardware with a data
> structure in memory.
> 
> > are doing isn't all that meaningful.  Can we proceed with the
> > workqueue patches?  Also, it could be that the only thing GFP_NOIO and
> > GFP_ATOMIC are doing is increasing the chance of IO failures under
> > memory pressure.  Maybe it'd be a good idea to reconsider the
> > approach?
> 
> We had actual deadlocks with GFP_KERNEL. It seems to me that the SCSI
> layer can deal with IO that cannot be completed due to a lack of memory
> at least somewhat, but a deadlock within a driver would obviously be
> deadly. So I don't think that mempools would remove the need for
> GFP_NOIO as there are places in usbcore we cannot enter the page
> laundering path from. They are an additional need.

OK, I guess there is some misunderstanding here. I believe that Tejun
wasn't arguing to drop GFP_NOIO. It might be really needed for the dead
lock avoidance. No question about that. The whole point is that
WQ_RECLAIM might be completely pointless because a rescuer wouldn't help
much if the work item would do GFP_NOIO and get stuck in the page
allocator.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Oliver Neukum]

On Mon, 2016-08-01 at 10:20 -0400, Tejun Heo wrote:
> Hello,

> > If any real IO depends on those devices then this is not sufficient and
> > they need some form of guarantee for progress (aka mempool).
> 
> Oliver, Alan, what do you think?  If USB itself can't operate without
> allocating memory during transactions, whatever USB storage drivers

It cannot. The IO must be described to the hardware with a data
structure in memory.

> are doing isn't all that meaningful.  Can we proceed with the
> workqueue patches?  Also, it could be that the only thing GFP_NOIO and
> GFP_ATOMIC are doing is increasing the chance of IO failures under
> memory pressure.  Maybe it'd be a good idea to reconsider the
> approach?

We had actual deadlocks with GFP_KERNEL. It seems to me that the SCSI
layer can deal with IO that cannot be completed due to a lack of memory
at least somewhat, but a deadlock within a driver would obviously be
deadly. So I don't think that mempools would remove the need for
GFP_NOIO as there are places in usbcore we cannot enter the page
laundering path from. They are an additional need.

Regards
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

Re: [PATCH] scsi: introduce a quirk for false cache reporting

[Johannes Thumshirn]

On Tue, Jul 12, 2016 at 10:24:36AM +0200, Oliver Neukum wrote:
> Some SATA to USB bridges fail to cooperate with some
> drives resulting in no cache being present being reported
> to the host. That causes the host to skip sending
> a command to synchronize caches. That causes data loss
> when the drive is powered down.
> 
> Signed-off-by: Oliver Neukum 

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
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 3/3] usb: gadget: u_ether: fix dereference after null check coverify warning

[Peter Chen]

On Fri, Jul 01, 2016 at 03:33:30PM +0800, Peter Chen wrote:
> skb is checked for null pointer at above code, so skb might be null.
> eem_wrap uses it without checking null pointer, fix it by adding null
> pointer check.
> 
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/gadget/function/u_ether.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/function/u_ether.c 
> b/drivers/usb/gadget/function/u_ether.c
> index 5f562c1..2bc8823 100644
> --- a/drivers/usb/gadget/function/u_ether.c
> +++ b/drivers/usb/gadget/function/u_ether.c
> @@ -549,7 +549,7 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
>   unsigned long   flags;
>  
>   spin_lock_irqsave(>lock, flags);
> - if (dev->port_usb)
> + if (dev->port_usb && skb)
>   skb = dev->wrap(dev->port_usb, skb);
>   spin_unlock_irqrestore(>lock, flags);
>   if (!skb) {
> -- 

Felipe, please do not queue this patch, it will break the ncm function
which the socket buffer may be NULL.

If you are ok with below changes, I will submit patch:

diff --git a/drivers/usb/gadget/function/f_eem.c 
b/drivers/usb/gadget/function/f_eem.c
index d58bfc3..5e0db68 100644
--- a/drivers/usb/gadget/function/f_eem.c
+++ b/drivers/usb/gadget/function/f_eem.c
@@ -341,11 +341,15 @@ static struct sk_buff *eem_wrap(struct gether *port, 
struct sk_buff *skb)
 {
struct sk_buff  *skb2 = NULL;
struct usb_ep   *in = port->in_ep;
-   int padlen = 0;
+   int headroom, tailroom, padlen = 0;
u16 len = skb->len;
 
-   int headroom = skb_headroom(skb);
-   int tailroom = skb_tailroom(skb);
+   if (!skb)
+   return NULL;
+
+   len = skb->len;
+   headroom = skb_headroom(skb);
+   tailroom = skb_tailroom(skb);
 
/* When (len + EEM_HLEN + ETH_FCS_LEN) % in->maxpacket) is 0,
 * stick two bytes of zero-length EEM packet on the end.
diff --git a/drivers/usb/gadget/function/f_rndis.c 
b/drivers/usb/gadget/function/f_rndis.c
index c800582..16562e4 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -374,6 +374,9 @@ static struct sk_buff *rndis_add_header(struct gether *port,
 {
struct sk_buff *skb2;
 
+   if (!skb)
+   return NULL;
+
skb2 = skb_realloc_headroom(skb, sizeof(struct rndis_packet_msg_type));
rndis_add_hdr(skb2);

-- 

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

Re: usbtmc: vendor specific i/o

[Ladislav Michl]

On Tue, Aug 02, 2016 at 06:58:11AM +0200, Greg Kroah-Hartman wrote:
> On Mon, Aug 01, 2016 at 08:40:02PM +0200, Ladislav Michl wrote:
> > Hi,
> > 
> > I need to ask usbtmc device for vendor specific data. What about adding 
> > ioctl
> > which switches MsgIDs to be vendor specific? Or are there other, better,
> > options? Patch bellow ilustrates changes to allow reading and writing vendor
> > specific messages.
> 
> I don't understand exactly what you are changing here, it seems like you
> are just changing the messages being sent to the device?  Doesn't that
> break existing users?

Sorry for being too brief. For sure I not want to break any existing user.
Normally MsgID=DEV_DEP_MSG_OUT is used for command message,
REQUEST_DEV_DEP_MSG_IN to request response message and DEV_DEP_MSG_IN
is MsgID of response message (values are 1 and 2 respectively).
However usbtmc class defines also vendor specific messages, which uses
the same scheme with values 126, resp. 127. Currently there's no way
to send vendor specific messages.
My question is whenever it is okay to add ioctl which will alter between
normal and vendor specific messages or if there's another prefered way,
something like this pseudo-patch (showing only one way switch):

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 917a55c..4c90e50 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -549,7 +549,7 @@ static int send_request_dev_dep_msg_in(struct 
usbtmc_device_data *data, size_t t
/* Setup IO buffer for REQUEST_DEV_DEP_MSG_IN message
 * Refer to class specs for details
 */
-   buffer[0] = 2;
+   buffer[0] = data->msgid_in;
buffer[1] = data->bTag;
buffer[2] = ~data->bTag;
buffer[3] = 0; /* Reserved */
@@ -677,8 +677,8 @@ static ssize_t usbtmc_read(struct file *filp, char __user 
*buf,
goto exit;
}
 
-   if (buffer[0] != 2) {
-   dev_err(dev, "Device sent reply with wrong 
MsgID: %u != 2\n", buffer[0]);
+   if (buffer[0] != data->msgid_in) {
+   dev_err(dev, "Device sent reply with wrong 
MsgID: %u != %u\n", buffer[0], data->msgid_out);
if (data->auto_abort)
usbtmc_ioctl_abort_bulk_in(data);
goto exit;
@@ -807,7 +807,7 @@ static ssize_t usbtmc_write(struct file *filp, const char 
__user *buf,
}
 
/* Setup IO buffer for DEV_DEP_MSG_OUT message */
-   buffer[0] = 1;
+   buffer[0] = data->msgid_out;
buffer[1] = data->bTag;
buffer[2] = ~data->bTag;
buffer[3] = 0; /* Reserved */
@@ -1227,6 +1227,12 @@ static long usbtmc_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
retval = usbtmc_ioctl_abort_bulk_in(data);
break;
 
+   case USBTMC_IOCTL_VENDOR_SPECIFIC:
+   data->msgid_out = 126;
+   data->msgid_in = 127;
+   retval = 0;
+   break;
+
case USBTMC488_IOCTL_GET_CAPS:
retval = copy_to_user((void __user *)arg,
>usb488_caps,
@@ -1414,6 +1420,8 @@ static int usbtmc_probe(struct usb_interface *intf,
data->TermChar = '\n';
/*  2 <= bTag <= 127   USBTMC-USB488 subclass specification 4.3.1 */
data->iin_bTag = 2;
+   data->msgid_out = 1;
+   data->msgid_in = 2;
 
/* USBTMC devices have only one setting, so use that */
iface_desc = data->intf->cur_altsetting;
--
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