Re: [PATCH v2 2/3] Documentation: bindings: add DT documentation for u2phy and u2phy grf
Am Freitag, 3. März 2017, 00:21:56 CET schrieb Rob Herring: > On Thu, Mar 02, 2017 at 03:49:04PM +0800, Meng Dongyang wrote: > > Due to the u2phy registers are separated from general grf, we need to > > add u2phy grf node and place u2phy node in it. So this patch add u2phy > > grf node. > > Similar comment on the subject. > > > Changes in v2: > > - add u2phy grf specification > > > > Signed-off-by: Meng Dongyang > > --- > > > > Documentation/devicetree/bindings/soc/rockchip/grf.txt | 15 > > +++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/soc/rockchip/grf.txt > > b/Documentation/devicetree/bindings/soc/rockchip/grf.txt index > > 013e71a..6ac23f9 100644 > > --- a/Documentation/devicetree/bindings/soc/rockchip/grf.txt > > +++ b/Documentation/devicetree/bindings/soc/rockchip/grf.txt > > @@ -7,6 +7,9 @@ From RK3368 SoCs, the GRF is divided into two sections, > > > > - GRF, used for general non-secure system, > > - PMUGRF, used for always on system > > > > +On RK3328 SoCs, the GRF is divided into three sections, > > +except GRF and PMUGRF, add a section for USB2PHYGRF, > > + > > > > Required Properties: > > > > - compatible: GRF should be one of the followings > > > > @@ -19,6 +22,8 @@ Required Properties: > > - compatible: PMUGRF should be one of the followings > > > > - "rockchip,rk3368-pmugrf", "syscon": for rk3368 > > - "rockchip,rk3399-pmugrf", "syscon": for rk3399 > > > > +- compatible: USB2GRF should be one of the followings > > + - "rockchip,rk3328-usb2phy-grf", "syscon": for rk3328 > > > > - reg: physical base address of the controller and length of memory > > mapped > > > >region. > > > > @@ -33,3 +38,13 @@ Example: GRF and PMUGRF of RK3399 SoCs > > > > compatible = "rockchip,rk3399-grf", "syscon"; > > reg = <0x0 0xff77 0x0 0x1>; > > > > }; > > > > + > > +Example: USB2GRF of RK3328 SoCs > > + > > + usb2phy_grf: syscon@ff45 { > > + compatible = "rockchip,rk3328-usb2phy-grf", "syscon", > > +"simple-mfd"; > > simple-mfd is not documented. Do you have child nodes? Yes, this will have child nodes and I guess we don't really need the additional example, that only differs in the compatible. > What makes this a syscon? It has non-PHY registers? It is still a "General Register Files" block in all of the socs manual and yes, that special area also contains registers not used by the phy driver. Heiko -- To unsubscribe from this list: send the line "unsubscribe 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 v17 2/3] usb: USB Type-C connector class
On 2017-03-03 04:13, Guenter Roeck wrote: On 03/02/2017 07:22 AM, Mats Karrman wrote: Looking forward, one thing I have run into is how to connect the typec driver with a driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in the Attention message. These signals are needed by the DisplayPort driver to know when to start negotiation etc. Have you got any thoughts on how to standardize such interfaces? That really depends on the lower level driver. For Chromebooks, where the Type-C Protocol Manager runs on the EC, we have an extcon driver which reports the pin states to the graphics drivers and connects to the Type-C class code using the Type-C class API. I still need to update, re-test, and publish that code. The published code in https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch chromeos-4.4, shows how it can be done, though that code currently still uses the Android Type-C infrastructure. OK, thanks! My system is a bit different. It's an i.MX6 SoC with the typec phy and DP controller connected directly to the SoC and it's using DTB/OF. Using extcon I would have a driver that is both typec class and extcon driver at the same time since I can't share the access to the typec phy. Is this done elsewhere in the kernel? I don't know much about the wcove PMIC and what alternate modes it might support but I guess that driver would end up in the same place. Do we need to further standardize attributes under (each) specific alternate mode to include things such as HPD for the DP mode? BR // Mats -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected
2017-03-02 23:13 GMT+09:00 Benjamin Tissoires : > On Mar 02 2017 or thereabouts, Jaejoong Kim wrote: >> The hid-core announces kernel message which driver is loaded if there is >> a hiddev, but hiddev's minor number is always zero even though it's not >> zero. >> >> So, we need to store the minor number asked from usb core and do some >> refactoring work(move from hiddev.c to hiddev.h) to access hiddev in >> hid-core. >> >> Signed-off-by: Jaejoong Kim >> --- >> drivers/hid/hid-core.c | 2 +- >> drivers/hid/usbhid/hiddev.c | 26 +++--- >> include/linux/hiddev.h | 24 >> 3 files changed, 28 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c >> index e9e87d3..1a0b910 100644 >> --- a/drivers/hid/hid-core.c >> +++ b/drivers/hid/hid-core.c >> @@ -1695,7 +1695,7 @@ int hid_connect(struct hid_device *hdev, unsigned int >> connect_mask) >> len += sprintf(buf + len, "input"); >> if (hdev->claimed & HID_CLAIMED_HIDDEV) >> len += sprintf(buf + len, "%shiddev%d", len ? "," : "", >> - hdev->minor); >> + ((struct hiddev *)hdev->hiddev)->minor); >> if (hdev->claimed & HID_CLAIMED_HIDRAW) >> len += sprintf(buf + len, "%shidraw%d", len ? "," : "", >> ((struct hidraw *)hdev->hidraw)->minor); >> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c >> index 5c2c489..ef83d68 100644 >> --- a/drivers/hid/usbhid/hiddev.c >> +++ b/drivers/hid/usbhid/hiddev.c >> @@ -44,29 +44,6 @@ >> #define HIDDEV_MINOR_BASE96 >> #define HIDDEV_MINORS16 >> #endif >> -#define HIDDEV_BUFFER_SIZE 2048 >> - >> -struct hiddev { >> - int minor; >> - int exist; >> - int open; >> - struct mutex existancelock; >> - wait_queue_head_t wait; >> - struct hid_device *hid; >> - struct list_head list; >> - spinlock_t list_lock; >> -}; >> - >> -struct hiddev_list { >> - struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE]; >> - int head; >> - int tail; >> - unsigned flags; >> - struct fasync_struct *fasync; >> - struct hiddev *hiddev; >> - struct list_head node; >> - struct mutex thread_lock; >> -}; >> >> /* >> * Find a report, given the report's type and ID. The ID can be specified >> @@ -911,6 +888,9 @@ int hiddev_connect(struct hid_device *hid, unsigned int >> force) >> kfree(hiddev); >> return -1; >> } >> + >> + hiddev->minor = usbhid->intf->minor; >> + >> return 0; >> } >> >> diff --git a/include/linux/hiddev.h b/include/linux/hiddev.h >> index a5dd814..ff3701b 100644 >> --- a/include/linux/hiddev.h >> +++ b/include/linux/hiddev.h >> @@ -32,6 +32,30 @@ >> * In-kernel definitions. >> */ >> >> +#define HIDDEV_BUFFER_SIZE 2048 >> + >> +struct hiddev { >> + int minor; >> + int exist; >> + int open; >> + struct mutex existancelock; >> + wait_queue_head_t wait; >> + struct hid_device *hid; >> + struct list_head list; >> + spinlock_t list_lock; >> +}; >> + >> +struct hiddev_list { >> + struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE]; >> + int head; >> + int tail; >> + unsigned flags; >> + struct fasync_struct *fasync; >> + struct hiddev *hiddev; >> + struct list_head node; >> + struct mutex thread_lock; >> +}; > > Why do you need to also export struct hiddev_list? Unless I am missing > something we don't need it elsewhere but in hiddev.c, and so there is no > point exporting this struct to the world. You're right. I will export only struct hiddev. > > With this change amended, the end result looks good and the series > should be ready to be integrated IMO. > I will resend v2 patchset your said. Thanks, jaejoong > Cheers, > Benjamin > >> + >> struct hid_device; >> struct hid_usage; >> struct hid_field; >> -- >> 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 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev
2017-03-02 23:10 GMT+09:00 Benjamin Tissoires : > On Mar 02 2017 or thereabouts, Jaejoong Kim wrote: >> We need to store the minor number each drivers. In case of hidraw, it's >> minor number stores in struct hidraw. But hiddev's minor is located in >> struct hid_device. >> >> So reallocates for hiddev's minor number. >> > > There is not a real need to have this one in a separate patch. Also, it > depends on the patch "[PATCH] HID: cp2112: use proper hidraw name with > minor number", so better include this cp2112 in this series (as I > mentioned in the cp2112 patch). > > I'd say simply squash this patch with 2/2 and have the cp2112 as 1/2. Ok. I will resend v2 patchset. Thanks. jaejoong > > Cheers, > Benjamin > >> Signed-off-by: Jaejoong Kim >> --- >> drivers/hid/usbhid/hiddev.c | 1 + >> include/linux/hid.h | 1 - >> 2 files changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c >> index 700145b..5c2c489 100644 >> --- a/drivers/hid/usbhid/hiddev.c >> +++ b/drivers/hid/usbhid/hiddev.c >> @@ -47,6 +47,7 @@ >> #define HIDDEV_BUFFER_SIZE 2048 >> >> struct hiddev { >> + int minor; >> int exist; >> int open; >> struct mutex existancelock; >> diff --git a/include/linux/hid.h b/include/linux/hid.h >> index 28f38e2b8..643c017 100644 >> --- a/include/linux/hid.h >> +++ b/include/linux/hid.h >> @@ -541,7 +541,6 @@ struct hid_device { >> /* device report descriptor */ >> struct list_head inputs;/* The >> list of inputs */ >> void *hiddev; /* The >> hiddev structure */ >> void *hidraw; >> - int minor; /* >> Hiddev minor number */ >> >> int open; /* is >> the device open by anyone? */ >> char name[128]; /* >> Device name */ >> -- >> 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] Documentation: bindings: add assign clock property in u2phy node
On Thu, Mar 02, 2017 at 03:49:03PM +0800, Meng Dongyang wrote: > On some platform such as RK3328, the 480m clock may need to assign > clock parent in dts in stead of clock driver. So this patch add > property of assigned-clocks and assigned-clock-parents to assign > parent for 480m clock. > > Changes in v2: > - move u2phy grf specification to grf.txt > > Signed-off-by: Meng Dongyang > --- > Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt | 6 ++ > 1 file changed, 6 insertions(+) If you respin, use "dt-bindings: phy: ..." for the subject. Acked-by: Rob Herring -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] Documentation: bindings: add DT documentation for u2phy and u2phy grf
On Thu, Mar 02, 2017 at 03:49:04PM +0800, Meng Dongyang wrote: > Due to the u2phy registers are separated from general grf, we need to > add u2phy grf node and place u2phy node in it. So this patch add u2phy > grf node. Similar comment on the subject. > > Changes in v2: > - add u2phy grf specification > > Signed-off-by: Meng Dongyang > --- > Documentation/devicetree/bindings/soc/rockchip/grf.txt | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/rockchip/grf.txt > b/Documentation/devicetree/bindings/soc/rockchip/grf.txt > index 013e71a..6ac23f9 100644 > --- a/Documentation/devicetree/bindings/soc/rockchip/grf.txt > +++ b/Documentation/devicetree/bindings/soc/rockchip/grf.txt > @@ -7,6 +7,9 @@ From RK3368 SoCs, the GRF is divided into two sections, > - GRF, used for general non-secure system, > - PMUGRF, used for always on system > > +On RK3328 SoCs, the GRF is divided into three sections, > +except GRF and PMUGRF, add a section for USB2PHYGRF, > + > Required Properties: > > - compatible: GRF should be one of the followings > @@ -19,6 +22,8 @@ Required Properties: > - compatible: PMUGRF should be one of the followings > - "rockchip,rk3368-pmugrf", "syscon": for rk3368 > - "rockchip,rk3399-pmugrf", "syscon": for rk3399 > +- compatible: USB2GRF should be one of the followings > + - "rockchip,rk3328-usb2phy-grf", "syscon": for rk3328 > - reg: physical base address of the controller and length of memory mapped >region. > > @@ -33,3 +38,13 @@ Example: GRF and PMUGRF of RK3399 SoCs > compatible = "rockchip,rk3399-grf", "syscon"; > reg = <0x0 0xff77 0x0 0x1>; > }; > + > +Example: USB2GRF of RK3328 SoCs > + > + usb2phy_grf: syscon@ff45 { > + compatible = "rockchip,rk3328-usb2phy-grf", "syscon", > + "simple-mfd"; simple-mfd is not documented. Do you have child nodes? What makes this a syscon? It has non-PHY registers? > + reg = <0x0 0xff45 0x0 0x1>; > + #address-cells = <1>; > + #size-cells = <1>; > + }; > -- > 1.9.1 > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] Documentation: bindings: add DT documentation for u2phy and u2phy grf
On 2017/3/3 14:21, Rob Herring wrote: On Thu, Mar 02, 2017 at 03:49:04PM +0800, Meng Dongyang wrote: Due to the u2phy registers are separated from general grf, we need to add u2phy grf node and place u2phy node in it. So this patch add u2phy grf node. Similar comment on the subject. Changes in v2: - add u2phy grf specification Signed-off-by: Meng Dongyang --- Documentation/devicetree/bindings/soc/rockchip/grf.txt | 15 +++ 1 file changed, 15 insertions(+) diff --git a/Documentation/devicetree/bindings/soc/rockchip/grf.txt b/Documentation/devicetree/bindings/soc/rockchip/grf.txt index 013e71a..6ac23f9 100644 --- a/Documentation/devicetree/bindings/soc/rockchip/grf.txt +++ b/Documentation/devicetree/bindings/soc/rockchip/grf.txt @@ -7,6 +7,9 @@ From RK3368 SoCs, the GRF is divided into two sections, - GRF, used for general non-secure system, - PMUGRF, used for always on system +On RK3328 SoCs, the GRF is divided into three sections, +except GRF and PMUGRF, add a section for USB2PHYGRF, + Required Properties: - compatible: GRF should be one of the followings @@ -19,6 +22,8 @@ Required Properties: - compatible: PMUGRF should be one of the followings - "rockchip,rk3368-pmugrf", "syscon": for rk3368 - "rockchip,rk3399-pmugrf", "syscon": for rk3399 +- compatible: USB2GRF should be one of the followings + - "rockchip,rk3328-usb2phy-grf", "syscon": for rk3328 - reg: physical base address of the controller and length of memory mapped region. @@ -33,3 +38,13 @@ Example: GRF and PMUGRF of RK3399 SoCs compatible = "rockchip,rk3399-grf", "syscon"; reg = <0x0 0xff77 0x0 0x1>; }; + +Example: USB2GRF of RK3328 SoCs + + usb2phy_grf: syscon@ff45 { + compatible = "rockchip,rk3328-usb2phy-grf", "syscon", +"simple-mfd"; simple-mfd is not documented. Do you have child nodes? do you mean we need to add a child node of phy here? What makes this a syscon? It has non-PHY registers? rk3328 devides grf into three parts and we put all u2phy registers into the part of u2phy grf, so here we add a simple usb2phy_grf. + reg = <0x0 0xff45 0x0 0x1>; + #address-cells = <1>; + #size-cells = <1>; + }; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/21] USB: serial: digi_acceleport: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Note that this driver uses an additional bulk-endpoint pair as an out-of-band port. Signed-off-by: Johan Hovold --- drivers/usb/serial/digi_acceleport.c | 23 --- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c index eb433922598c..dc44c38525e1 100644 --- a/drivers/usb/serial/digi_acceleport.c +++ b/drivers/usb/serial/digi_acceleport.c @@ -272,6 +272,8 @@ static struct usb_serial_driver digi_acceleport_2_device = { .description = "Digi 2 port USB adapter", .id_table = id_table_2, .num_ports =3, + .num_bulk_in = 4, + .num_bulk_out = 4, .open = digi_open, .close =digi_close, .dtr_rts = digi_dtr_rts, @@ -301,6 +303,8 @@ static struct usb_serial_driver digi_acceleport_4_device = { .description = "Digi 4 port USB adapter", .id_table = id_table_4, .num_ports =4, + .num_bulk_in = 5, + .num_bulk_out = 5, .open = digi_open, .close =digi_close, .write =digi_write, @@ -1250,27 +1254,8 @@ static int digi_port_init(struct usb_serial_port *port, unsigned port_num) static int digi_startup(struct usb_serial *serial) { - struct device *dev = &serial->interface->dev; struct digi_serial *serial_priv; int ret; - int i; - - /* check whether the device has the expected number of endpoints */ - if (serial->num_port_pointers < serial->type->num_ports + 1) { - dev_err(dev, "OOB endpoints missing\n"); - return -ENODEV; - } - - for (i = 0; i < serial->type->num_ports + 1 ; i++) { - if (!serial->port[i]->read_urb) { - dev_err(dev, "bulk-in endpoint missing\n"); - return -ENODEV; - } - if (!serial->port[i]->write_urb) { - dev_err(dev, "bulk-out endpoint missing\n"); - return -ENODEV; - } - } serial_priv = kzalloc(sizeof(*serial_priv), GFP_KERNEL); if (!serial_priv) -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: misc: ldusb: Fixed coding style issues
On Fri, Mar 03, 2017 at 12:55:46AM +0100, Milian Reichardt wrote: > Fixed multiple coding style issues. Please be specific as to exactly what issues you are fixing up. And don't do multiple things in the same patch, each different thing needs to be broken up into an individual patch. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v17 2/3] usb: USB Type-C connector class
On 03/02/2017 07:35 PM, Peter Chen wrote: On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote: +/* --- */ +/* Driver callbacks to report role updates */ + +/** + * typec_set_data_role - Report data role change + * @port: The USB Type-C Port where the role was changed + * @role: The new data role + * + * This routine is used by the port drivers to report data role changes. + */ +void typec_set_data_role(struct typec_port *port, enum typec_data_role role) +{ + if (port->data_role == role) + return; + + port->data_role = role; + sysfs_notify(&port->dev.kobj, NULL, "data_role"); + kobject_uevent(&port->dev.kobj, KOBJ_CHANGE); +} +EXPORT_SYMBOL_GPL(typec_set_data_role); + Hi Keikki, Have you tested this interface with real dual-role controller/board? If it helps, my primary test system is a HP Chromebook 13 G1. What interface you use when you receive this event to handle dual-role switch? I am wonder if a common dual-role class is needed, then we can have a common user utility. I don't really understand "What interface you use when you receive this event". Can you explain ? Eg, if "data_role" has changed, the udev can echo "data_role" to /sys/class/usb-dual-role/role That sounds like a kernel event delivered to user space via udev or sysfs notification and returned back into the kernel through a sysfs attribute. Do I understand that correctly ? Thanks, Guenter Maybe we can enhance Roger's drd framework [1] to fulfill that. [1] https://lwn.net/Articles/682531/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bug ID: 193901 Summary: dvb-usb: bulk message failed on Linux 4.9 - on Linux 4.8 no issues
Hallo! I have the same problems with my DVB-T2 USB-Stick. August DVB-T210 (V 2.0) is detected by kernel as Mygica T230 DVB-T/T2/C which is the same as Geniatech 230. Tested: Kernel 4.10.1.: "no frontend was attached" Latest working kernel for me is Kernel 4.4.X. with firmware from openELEC.(dvb-demod-si2168-b40-01.fw (V 11) and dvb-tuner-si2158-a20-01.fw). W.Pelser -- To unsubscribe from this list: send the line "unsubscribe 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 v17 2/3] usb: USB Type-C connector class
On Thu, Mar 02, 2017 at 08:29:07PM -0800, Guenter Roeck wrote: > On 03/02/2017 07:35 PM, Peter Chen wrote: > >On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote: > >>+/* --- */ > >>+/* Driver callbacks to report role updates */ > >>+ > >>+/** > >>+ * typec_set_data_role - Report data role change > >>+ * @port: The USB Type-C Port where the role was changed > >>+ * @role: The new data role > >>+ * > >>+ * This routine is used by the port drivers to report data role changes. > >>+ */ > >>+void typec_set_data_role(struct typec_port *port, enum typec_data_role > >>role) > >>+{ > >>+ if (port->data_role == role) > >>+ return; > >>+ > >>+ port->data_role = role; > >>+ sysfs_notify(&port->dev.kobj, NULL, "data_role"); > >>+ kobject_uevent(&port->dev.kobj, KOBJ_CHANGE); > >>+} > >>+EXPORT_SYMBOL_GPL(typec_set_data_role); > >>+ > > > >Hi Keikki, > > > >Have you tested this interface with real dual-role controller/board? > > If it helps, my primary test system is a HP Chromebook 13 G1. > > >What interface you use when you receive this event to handle > >dual-role switch? I am wonder if a common dual-role class is > >needed, then we can have a common user utility. > > I don't really understand "What interface you use when you receive > this event". Can you explain ? > I mean "How to trigger kernel USB controller driver do role switch?" > > > >Eg, if "data_role" has changed, the udev can echo "data_role" to > >/sys/class/usb-dual-role/role > > > That sounds like a kernel event delivered to user space via udev or > sysfs notification and returned back into the kernel through a sysfs > attribute. Do I understand that correctly ? > Yes. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v17 2/3] usb: USB Type-C connector class
On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote: > +/* --- */ > +/* Driver callbacks to report role updates */ > + > +/** > + * typec_set_data_role - Report data role change > + * @port: The USB Type-C Port where the role was changed > + * @role: The new data role > + * > + * This routine is used by the port drivers to report data role changes. > + */ > +void typec_set_data_role(struct typec_port *port, enum typec_data_role role) > +{ > + if (port->data_role == role) > + return; > + > + port->data_role = role; > + sysfs_notify(&port->dev.kobj, NULL, "data_role"); > + kobject_uevent(&port->dev.kobj, KOBJ_CHANGE); > +} > +EXPORT_SYMBOL_GPL(typec_set_data_role); > + Hi Keikki, Have you tested this interface with real dual-role controller/board? What interface you use when you receive this event to handle dual-role switch? I am wonder if a common dual-role class is needed, then we can have a common user utility. Eg, if "data_role" has changed, the udev can echo "data_role" to /sys/class/usb-dual-role/role Maybe we can enhance Roger's drd framework [1] to fulfill that. [1] https://lwn.net/Articles/682531/ -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev
On Mar 02 2017 or thereabouts, Jaejoong Kim wrote: > We need to store the minor number each drivers. In case of hidraw, it's > minor number stores in struct hidraw. But hiddev's minor is located in > struct hid_device. > > So reallocates for hiddev's minor number. > There is not a real need to have this one in a separate patch. Also, it depends on the patch "[PATCH] HID: cp2112: use proper hidraw name with minor number", so better include this cp2112 in this series (as I mentioned in the cp2112 patch). I'd say simply squash this patch with 2/2 and have the cp2112 as 1/2. Cheers, Benjamin > Signed-off-by: Jaejoong Kim > --- > drivers/hid/usbhid/hiddev.c | 1 + > include/linux/hid.h | 1 - > 2 files changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c > index 700145b..5c2c489 100644 > --- a/drivers/hid/usbhid/hiddev.c > +++ b/drivers/hid/usbhid/hiddev.c > @@ -47,6 +47,7 @@ > #define HIDDEV_BUFFER_SIZE 2048 > > struct hiddev { > + int minor; > int exist; > int open; > struct mutex existancelock; > diff --git a/include/linux/hid.h b/include/linux/hid.h > index 28f38e2b8..643c017 100644 > --- a/include/linux/hid.h > +++ b/include/linux/hid.h > @@ -541,7 +541,6 @@ struct hid_device { > /* device report descriptor */ > struct list_head inputs;/* The > list of inputs */ > void *hiddev; /* The > hiddev structure */ > void *hidraw; > - int minor; /* > Hiddev minor number */ > > int open; /* is > the device open by anyone? */ > char name[128]; /* > Device name */ > -- > 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 v17 2/3] usb: USB Type-C connector class
On 03/02/2017 07:22 AM, Mats Karrman wrote: Hi Heikki, Good to see things are happening with Type-C! On 2017-02-21 15:24, Heikki Krogerus wrote: ... +When connected, the partner will be presented also as its own device under +/sys/class/typec/. The parent of the partner device will always be the port it +is attached to. The partner attached to port "port0" will be named +"port0-partner". Full path to the device would be +/sys/class/typec/port0/port0-partner/. A "/port0" too much? + +The cable and the two plugs on it may also be optionally presented as their own +devices under /sys/class/typec/. The cable attached to the port "port0" port +will be named port0-cable and the plug on the SOP Prime end (see USB Power +Delivery Specification ch. 2.4) will be named "port0-plug0" and on the SOP +Double Prime end "port0-plug1". The parent of a cable will always be the port, +and the parent of the cable plugs will always be the cable. + +If the port, partner or cable plug support Alternate Modes, every supported +Alternate Mode SVID will have their own device describing them. The Alternate +Modes will not be attached to the typec class. The parent of an alternate mode +will be the device that supports it, so for example an alternate mode of +port0-partner will bees presented under /sys/class/typec/port0-partner/. Every bees? +mode that is supported will have its own group under the Alternate Mode device +named "mode", for example /sys/class/typec/port0//mode1/. +The requests for entering/exiting a mode can be done with "active" attribute +file in that group. + ... I'm hoping to find time to upgrade the kernel and try these patches in my system. Looking forward, one thing I have run into is how to connect the typec driver with a driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in the Attention message. These signals are needed by the DisplayPort driver to know when to start negotiation etc. Have you got any thoughts on how to standardize such interfaces? That really depends on the lower level driver. For Chromebooks, where the Type-C Protocol Manager runs on the EC, we have an extcon driver which reports the pin states to the graphics drivers and connects to the Type-C class code using the Type-C class API. I still need to update, re-test, and publish that code. The published code in https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch chromeos-4.4, shows how it can be done, though that code currently still uses the Android Type-C infrastructure. Guenter -- To unsubscribe from this list: send the line "unsubscribe 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 v19 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
On Mon, Feb 20 2017, Baolin Wang wrote: > Currently the Linux kernel does not provide any standard integration of this > feature that integrates the USB subsystem with the system power regulation > provided by PMICs meaning that either vendors must add this in their kernels > or USB gadget devices based on Linux (such as mobile phones) may not behave > as they should. Thus provide a standard framework for doing this in kernel. > > Now introduce one user with wm831x_power to support and test the usb charger. > Another user introduced to support charger detection by Jun Li: > https://www.spinics.net/lists/linux-usb/msg139425.html > Moreover there may be other potential users will use it in future. > > 1. Before v19 patchset we've fixed below issues in extcon subsystem and usb > phy driver, now all were merged. (Thanks for Neil's suggestion) > (1) Have fixed the inconsistencies with USB connector types in extcon > subsystem > by following links: > https://lkml.org/lkml/2016/12/21/13 > https://lkml.org/lkml/2016/12/21/15 > https://lkml.org/lkml/2016/12/21/79 > https://lkml.org/lkml/2017/1/3/13 > > (2) Instead of using 'set_power' callback in phy drivers, we will introduce > USB charger to set PMIC current drawn from USB configuration, moreover some > 'set_power' callbacks did not implement anything to set PMIC current, thus > remove them by following links: > https://lkml.org/lkml/2017/1/18/436 > https://lkml.org/lkml/2017/1/18/439 > https://lkml.org/lkml/2017/1/18/438 > Now only two phy drivers (phy-isp1301-omap.c and phy-gpio-vbus-usb.c) still > used 'set_power' callback to set current, we can remove them in future. (I > have no platform with enabling these two phy drivers, so I can not test them > if I converted 'set_power' callback to USB charger.) > > 2. Some issues pointed by Neil Brown were sill kept in this v19 patchset, and > I expalined each issue and may be need discuss again: > (1) Change all usb phys to register an extcon and to send appropriate > notifications. > Firstly, now only 3 USB phy drivers (phy-qcom-8x16-usb.c, phy-omap-otg.c and > phy-msm-usb.c) had registered an extcon, mostly did not. I can not change all > usb phys to register an extcon, since there are no extcon device to register > for these different phy drivers. You don't have to change every driver. You just need to make it easy and obvious how to change drivers in a consistent coherent way. For a start you would add a 'struct extcon_dev' to 'struct usb_phy', and possibly add or extend some 'static inline's in linux/usb/phy.h to send notification on that extcon (if it is non-NULL). e.g. usb_phy_vbus_on() could send an extcon notification. Then any phy driver which adds support for setting phy->extcon_dev appropriately, immediately gets the relevant notifications sent. > Secondly, I also agreed with Peter's comments: Not only USB PHY to register > an extcon, but also for the drivers which can detect USB charger type, it may > be USB controller driver, USB type-c driver, pmic driver, and these drivers > may not have an extcon device since the internal part can finish the vbus > detect. Whichever part can detect vbus, the driver for that part must be able to find the extcon and trigger a notification. Maybe one part can detect VBUS, another can measure the resistance on ID and a third can work through the state machine to determine if D+ and D- are shorted together. Somehow these three need to work together to determine what is plugged in to the external connection port. Somewhere there much an 'extcon' device which represents that port and which the three devices can find and can interact with. I think it makes sense for the usb_phy to be the connection point. Each of the devices can get to the phy, and the phy can get to the extcon. It doesn't matter very much if the usb phy driver creates the extcon, or if something else creates the extcon and the phy driver performs a lookup to find it (e.g. based on devicetree info). The point is that there is obviously an external physical connection, and so there should be an 'extcon' device that represents it. > > (2) Change the notifier of usb_phy to be used consistently. > Now only 3 phy drivers (phy-generic.c, phy-ab8500-usb.c and > phy-gpio-vbus-usb.c) > used the notifier of usb_phy. phy-generic.c and phy-gpio-vbus-usb.c were used > to > send out the connect events, and phy-ab8500-usb.c also was used to send out > the > MUSB connect events. There are no phy drivers will notify 'vbus_draw' > information > by the notifier of usb_phy, which was used consistently now. > Moreover it is difficult to change the notifier of usb_phy to be used only to > communicate the 'vbus_draw' information, since we need to refactor and test > these > related phy drivers, power drivers or some mfd drivers, which is a > huge workload. You missed drivers/usb/musb/omap2430.c in you list, but that hardly matters. phy-ab8500-usb.c appears to send vbus_draw information. I understand your reluc
[PATCH] usb: dwc2: Make sure we disconnect the gadget state
I had seen some odd behavior with HiKey's usb-gadget interface that I finally seemed to have chased down. Basically every other time I pluged in the OTG port, the gadget interface would properly initialize. The other times, I'd get a big WARN_ON in dwc2_hsotg_init_fifo() about the fifo_map not being clear. Ends up if we don't disconnect the gadget state, the fifo-map doesn't get cleared properly, which causes WARN_ON messages and also results in the device not properly being setup as a gadget every other time the OTG port is connected. So this patch adds a call to dwc2_hsotg_disconnect() in the reset path so the state is properly cleared. With it, the gadget interface initializes properly on every plug in. Cc: Wei Xu Cc: Guodong Xu Cc: Amit Pundir Cc: Rob Herring Cc: John Youn Cc: Douglas Anderson Cc: Chen Yu Cc: Felipe Balbi Cc: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org Acked-by: John Youn Signed-off-by: John Stultz --- drivers/usb/dwc2/hcd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index a73722e..91ed5b6 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -3264,6 +3264,7 @@ static void dwc2_conn_id_status_change(struct work_struct *work) dwc2_core_init(hsotg, false); dwc2_enable_global_interrupts(hsotg); spin_lock_irqsave(&hsotg->lock, flags); + dwc2_hsotg_disconnect(hsotg); dwc2_hsotg_core_init_disconnected(hsotg, false); spin_unlock_irqrestore(&hsotg->lock, flags); dwc2_hsotg_core_connect(hsotg); -- 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
[PATCH 02/21] USB: serial: clean up endpoint and port-counter types
Use unsigned-char type for the endpoint and port counters. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb-serial.c | 14 +++--- include/linux/usb/serial.h | 11 ++- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index c20d90ed1ef2..e5b859ad15c6 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -728,12 +728,12 @@ static int usb_serial_probe(struct usb_interface *interface, int buffer_size; int i; int j; - int num_interrupt_in = 0; - int num_interrupt_out = 0; - int num_bulk_in = 0; - int num_bulk_out = 0; + unsigned char num_interrupt_in = 0; + unsigned char num_interrupt_out = 0; + unsigned char num_bulk_in = 0; + unsigned char num_bulk_out = 0; int num_ports = 0; - int max_endpoints; + unsigned char max_endpoints; mutex_lock(&table_lock); type = search_serial_device(interface); @@ -879,7 +879,7 @@ static int usb_serial_probe(struct usb_interface *interface, num_ports = MAX_NUM_PORTS; } - serial->num_ports = num_ports; + serial->num_ports = (unsigned char)num_ports; serial->num_bulk_in = num_bulk_in; serial->num_bulk_out = num_bulk_out; serial->num_interrupt_in = num_interrupt_in; @@ -894,7 +894,7 @@ static int usb_serial_probe(struct usb_interface *interface, max_endpoints = max(num_bulk_in, num_bulk_out); max_endpoints = max(max_endpoints, num_interrupt_in); max_endpoints = max(max_endpoints, num_interrupt_out); - max_endpoints = max(max_endpoints, (int)serial->num_ports); + max_endpoints = max(max_endpoints, serial->num_ports); serial->num_port_pointers = max_endpoints; dev_dbg(ddev, "setting up %d port structure(s)\n", max_endpoints); diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h index 704a1ab8240c..85b475933848 100644 --- a/include/linux/usb/serial.h +++ b/include/linux/usb/serial.h @@ -159,10 +159,10 @@ struct usb_serial { unsigned char minors_reserved:1; unsigned char num_ports; unsigned char num_port_pointers; - charnum_interrupt_in; - charnum_interrupt_out; - charnum_bulk_in; - charnum_bulk_out; + unsigned char num_interrupt_in; + unsigned char num_interrupt_out; + unsigned char num_bulk_in; + unsigned char num_bulk_out; struct usb_serial_port *port[MAX_NUM_PORTS]; struct kref kref; struct mutexdisc_mutex; @@ -227,13 +227,14 @@ static inline void usb_set_serial_data(struct usb_serial *serial, void *data) struct usb_serial_driver { const char *description; const struct usb_device_id *id_table; - charnum_ports; struct list_headdriver_list; struct device_driverdriver; struct usb_driver *usb_driver; struct usb_dynids dynids; + unsigned char num_ports; + size_t bulk_in_size; size_t bulk_out_size; -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/21] USB: serial: opticon: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Signed-off-by: Johan Hovold --- drivers/usb/serial/opticon.c | 12 +--- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/drivers/usb/serial/opticon.c b/drivers/usb/serial/opticon.c index 3937b9c3cc69..58657d64678b 100644 --- a/drivers/usb/serial/opticon.c +++ b/drivers/usb/serial/opticon.c @@ -367,16 +367,6 @@ static int opticon_ioctl(struct tty_struct *tty, return -ENOIOCTLCMD; } -static int opticon_startup(struct usb_serial *serial) -{ - if (!serial->num_bulk_in) { - dev_err(&serial->dev->dev, "no bulk in endpoint\n"); - return -ENODEV; - } - - return 0; -} - static int opticon_port_probe(struct usb_serial_port *port) { struct opticon_private *priv; @@ -408,8 +398,8 @@ static struct usb_serial_driver opticon_device = { }, .id_table = id_table, .num_ports =1, + .num_bulk_in = 1, .bulk_in_size = 256, - .attach = opticon_startup, .port_probe = opticon_port_probe, .port_remove = opticon_port_remove, .open = opticon_open, -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] USB: serial: ftdi_sio: detect BM chip with iSerialNumber bug
On Tue, Feb 28, 2017 at 12:51:24PM +, Ian Abbott wrote: > If a BM type chip has iSerialNumber set to 0 in its EEPROM, an incorrect > value is read from the bcdDevice field of the USB descriptor, making it > look like an AM type chip. Attempt to correct this in > ftdi_determine_type() by attempting to read the latency timer for an AM > type chip. If that succeeds, assume it is a BM type chip. > > Currently, read_latency_timer() bails out without reading the latency > timer for an AM type chip, so factor out the guts of > read_latency_timer() into a new function _read_latency_timer() that > attempts to read the latency timer regardless of chip type, and returns > either the latency timer value or a negative error number. > > Signed-off-by: Ian Abbott > --- > drivers/usb/serial/ftdi_sio.c | 38 ++ > 1 file changed, 30 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c > index 72314734dfd0..a1b90f4184a7 100644 > --- a/drivers/usb/serial/ftdi_sio.c > +++ b/drivers/usb/serial/ftdi_sio.c > @@ -1609,9 +1623,17 @@ static void ftdi_determine_type(struct usb_serial_port > *port) > priv->baud_base = 1200 / 16; > } else if (version < 0x400) { > /* Assume it's an FT8U232AM (or FT8U245AM) */ > - /* (It might be a BM because of the iSerialNumber bug, > - * but it will still work as an AM device.) */ > priv->chip_type = FT8U232AM; > + /* > + * It might be a BM type because of the iSerialNumber bug. > + * If the latency timer is readable, assume it is BM type. > + */ > + if (_read_latency_timer(port) >= 0) { Would it be possible to only try to read the latency timer when iSerialNumber is 0? > + dev_dbg(&port->dev, > + "%s: has latency timer so not an AM type\n", > + __func__); > + priv->chip_type = FT232BM; > + } > } else if (version < 0x600) { > /* Assume it's an FT232BM (or FT245BM) */ > priv->chip_type = FT232BM; Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 2/2] phy: rockchip-inno-usb2: add support of u2phy for rk3328
Hi Meng, [auto build test ERROR on robh/for-next] [also build test ERROR on v4.10 next-20170302] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Meng-Dongyang/Documentation-bindings-add-DT-documentation-for-u2phy-and-u2phy-grf/20170302-223541 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: xtensa-allmodconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 4.9.0 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=xtensa All errors (new ones prefixed by >>): >> drivers/phy/phy-rockchip-inno-usb2.c:1143:3: error: unknown field >> 'phy_tuning' specified in initializer .phy_tuning = rk3328_usb2phy_tuning, ^ >> drivers/phy/phy-rockchip-inno-usb2.c:1143:17: error: 'rk3328_usb2phy_tuning' >> undeclared here (not in a function) .phy_tuning = rk3328_usb2phy_tuning, ^ vim +/phy_tuning +1143 drivers/phy/phy-rockchip-inno-usb2.c 1137 } 1138 1139 static const struct rockchip_usb2phy_cfg rk3328_phy_cfgs[] = { 1140 { 1141 .reg = 0x100, 1142 .num_ports = 2, > 1143 .phy_tuning = rk3328_usb2phy_tuning, 1144 .clkout_ctl = { 0x108, 4, 4, 1, 0 }, 1145 .port_cfgs = { 1146 [USB2PHY_PORT_HOST] = { --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v17 2/3] usb: USB Type-C connector class
Hi Heikki, Good to see things are happening with Type-C! On 2017-02-21 15:24, Heikki Krogerus wrote: ... +When connected, the partner will be presented also as its own device under +/sys/class/typec/. The parent of the partner device will always be the port it +is attached to. The partner attached to port "port0" will be named +"port0-partner". Full path to the device would be +/sys/class/typec/port0/port0-partner/. A "/port0" too much? + +The cable and the two plugs on it may also be optionally presented as their own +devices under /sys/class/typec/. The cable attached to the port "port0" port +will be named port0-cable and the plug on the SOP Prime end (see USB Power +Delivery Specification ch. 2.4) will be named "port0-plug0" and on the SOP +Double Prime end "port0-plug1". The parent of a cable will always be the port, +and the parent of the cable plugs will always be the cable. + +If the port, partner or cable plug support Alternate Modes, every supported +Alternate Mode SVID will have their own device describing them. The Alternate +Modes will not be attached to the typec class. The parent of an alternate mode +will be the device that supports it, so for example an alternate mode of +port0-partner will bees presented under /sys/class/typec/port0-partner/. Every bees? +mode that is supported will have its own group under the Alternate Mode device +named "mode", for example /sys/class/typec/port0//mode1/. +The requests for entering/exiting a mode can be done with "active" attribute +file in that group. + ... I'm hoping to find time to upgrade the kernel and try these patches in my system. Looking forward, one thing I have run into is how to connect the typec driver with a driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in the Attention message. These signals are needed by the DisplayPort driver to know when to start negotiation etc. Have you got any thoughts on how to standardize such interfaces? BR // Mats -- To unsubscribe from this list: send the line "unsubscribe 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 4/6] USB: serial: ftdi_sio: only allow valid event_char values
On Tue, Feb 28, 2017 at 12:51:26PM +, Ian Abbott wrote: > The "event_char" device attribute value, when written, is interpreted as > an enable bit in bit 8, and an "event character" in bits 7 to 0. Return > an error for out-of-range values. > > Signed-off-by: Ian Abbott > --- > drivers/usb/serial/ftdi_sio.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c > index 2da99875cecb..2662fc3b49c5 100644 > --- a/drivers/usb/serial/ftdi_sio.c > +++ b/drivers/usb/serial/ftdi_sio.c > @@ -1738,6 +1738,9 @@ static ssize_t store_event_char(struct device *dev, > int v = simple_strtoul(valbuf, NULL, 10); > int rv; > > + if (v < 0 || v >= 0x200) > + return -EINVAL; > + v < 0 is always false here due to the unsigned simple_strtoul above, which continues to accept negative values after this change. It may be better to combine this with the kstrtouint conversion. > dev_dbg(&port->dev, "%s: setting event char = %i\n", __func__, v); > > rv = usb_control_msg(udev, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] HID: hiddev: move hiddev's minor number and refactoring
Hi all, I found hiddev's minor number is always zero in struct hid_device. So, store the minor number asked from usb core in struct hid_device. This is my first approach. But after reviewed from Bendjamin, he suggested that it would make sense to store a minor number in struct hiddev like hidraw if it neeeded. So, I move the minor number from hid_device to hiddev and do some refactoring to access struct hiddev in hid-core Jaejoong Kim (2): HID: hiddev: move hiddev's minor number from struct hid_device to hiddev HID: hiddev: store hiddev's minor number when hiddev is connected drivers/hid/hid-core.c | 2 +- drivers/hid/usbhid/hiddev.c | 25 +++-- include/linux/hid.h | 1 - include/linux/hiddev.h | 24 4 files changed, 28 insertions(+), 24 deletions(-) -- 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 3/6] USB: serial: ftdi_sio: only allow valid latency timer values
On Tue, Feb 28, 2017 at 12:51:25PM +, Ian Abbott wrote: > Valid latency timer values are between 1 ms and 255 ms in 1 ms steps. > The store function for the "latency_timer" device attribute currently > allows any value, although only the lower 8-bits will be written to the > latency timer. Return an error for out-of-range values. And in fact, 0 is currently used (and accepted by the device) if a negative value is provided. > Signed-off-by: Ian Abbott > --- > drivers/usb/serial/ftdi_sio.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c > index a1b90f4184a7..2da99875cecb 100644 > --- a/drivers/usb/serial/ftdi_sio.c > +++ b/drivers/usb/serial/ftdi_sio.c > @@ -1716,6 +1716,9 @@ static ssize_t latency_timer_store(struct device *dev, > int v = simple_strtoul(valbuf, NULL, 10); > int rv; > > + if (v < 1 || v > 255) > + return -EINVAL; > + We probably still need to accept 0 here, which seems to give a timer of 1 ms, as someone may be relying on that behaviour already. > priv->latency = v; > rv = write_latency_timer(port); > if (rv < 0) Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected
On Mar 02 2017 or thereabouts, Jaejoong Kim wrote: > The hid-core announces kernel message which driver is loaded if there is > a hiddev, but hiddev's minor number is always zero even though it's not > zero. > > So, we need to store the minor number asked from usb core and do some > refactoring work(move from hiddev.c to hiddev.h) to access hiddev in > hid-core. > > Signed-off-by: Jaejoong Kim > --- > drivers/hid/hid-core.c | 2 +- > drivers/hid/usbhid/hiddev.c | 26 +++--- > include/linux/hiddev.h | 24 > 3 files changed, 28 insertions(+), 24 deletions(-) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index e9e87d3..1a0b910 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1695,7 +1695,7 @@ int hid_connect(struct hid_device *hdev, unsigned int > connect_mask) > len += sprintf(buf + len, "input"); > if (hdev->claimed & HID_CLAIMED_HIDDEV) > len += sprintf(buf + len, "%shiddev%d", len ? "," : "", > - hdev->minor); > + ((struct hiddev *)hdev->hiddev)->minor); > if (hdev->claimed & HID_CLAIMED_HIDRAW) > len += sprintf(buf + len, "%shidraw%d", len ? "," : "", > ((struct hidraw *)hdev->hidraw)->minor); > diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c > index 5c2c489..ef83d68 100644 > --- a/drivers/hid/usbhid/hiddev.c > +++ b/drivers/hid/usbhid/hiddev.c > @@ -44,29 +44,6 @@ > #define HIDDEV_MINOR_BASE96 > #define HIDDEV_MINORS16 > #endif > -#define HIDDEV_BUFFER_SIZE 2048 > - > -struct hiddev { > - int minor; > - int exist; > - int open; > - struct mutex existancelock; > - wait_queue_head_t wait; > - struct hid_device *hid; > - struct list_head list; > - spinlock_t list_lock; > -}; > - > -struct hiddev_list { > - struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE]; > - int head; > - int tail; > - unsigned flags; > - struct fasync_struct *fasync; > - struct hiddev *hiddev; > - struct list_head node; > - struct mutex thread_lock; > -}; > > /* > * Find a report, given the report's type and ID. The ID can be specified > @@ -911,6 +888,9 @@ int hiddev_connect(struct hid_device *hid, unsigned int > force) > kfree(hiddev); > return -1; > } > + > + hiddev->minor = usbhid->intf->minor; > + > return 0; > } > > diff --git a/include/linux/hiddev.h b/include/linux/hiddev.h > index a5dd814..ff3701b 100644 > --- a/include/linux/hiddev.h > +++ b/include/linux/hiddev.h > @@ -32,6 +32,30 @@ > * In-kernel definitions. > */ > > +#define HIDDEV_BUFFER_SIZE 2048 > + > +struct hiddev { > + int minor; > + int exist; > + int open; > + struct mutex existancelock; > + wait_queue_head_t wait; > + struct hid_device *hid; > + struct list_head list; > + spinlock_t list_lock; > +}; > + > +struct hiddev_list { > + struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE]; > + int head; > + int tail; > + unsigned flags; > + struct fasync_struct *fasync; > + struct hiddev *hiddev; > + struct list_head node; > + struct mutex thread_lock; > +}; Why do you need to also export struct hiddev_list? Unless I am missing something we don't need it elsewhere but in hiddev.c, and so there is no point exporting this struct to the world. With this change amended, the end result looks good and the series should be ready to be integrated IMO. Cheers, Benjamin > + > struct hid_device; > struct hid_usage; > struct hid_field; > -- > 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
[PATCH 14/21] USB: serial: mos7720: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Note that the driver expects two bulk-endpoint pairs also for mcs7715 devices for which only one serial port is registered. Signed-off-by: Johan Hovold --- drivers/usb/serial/mos7720.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c index f075121c6e32..df45ebad5f6f 100644 --- a/drivers/usb/serial/mos7720.c +++ b/drivers/usb/serial/mos7720.c @@ -1900,11 +1900,6 @@ static int mos7720_startup(struct usb_serial *serial) u16 product; int ret_val; - if (serial->num_bulk_in < 2 || serial->num_bulk_out < 2) { - dev_err(&serial->interface->dev, "missing bulk endpoints\n"); - return -ENODEV; - } - product = le16_to_cpu(serial->dev->descriptor.idProduct); dev = serial->dev; @@ -2039,6 +2034,8 @@ static struct usb_serial_driver moschip7720_2port_driver = { }, .description= "Moschip 2 port adapter", .id_table = id_table, + .num_bulk_in= 2, + .num_bulk_out = 2, .calc_num_ports = mos77xx_calc_num_ports, .open = mos7720_open, .close = mos7720_close, -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] USB: serial: ftdi_sio: allow other bases for "event_char"
On Tue, Feb 28, 2017 at 12:51:28PM +, Ian Abbott wrote: > The 'store' function for the "event_char" device attribute currently > expects a base 10 value. The value is composed of an enable bit in bit > 8 and an 8-bit "event character" code in bits 7 to 0. It seems > reasonable to allow hexadecimal and octal numbers to be written to the > device attribute in addition to decimal. Make it so. > > Signed-off-by: Ian Abbott > --- > drivers/usb/serial/ftdi_sio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c > index e6785d84280b..a007ec8238eb 100644 > --- a/drivers/usb/serial/ftdi_sio.c > +++ b/drivers/usb/serial/ftdi_sio.c > @@ -1742,7 +1742,7 @@ static ssize_t store_event_char(struct device *dev, > unsigned int v; > int rv; > > - rv = kstrtouint(valbuf, 10, &v); > + rv = kstrtouint(valbuf, 0, &v); > if (rv) > return rv; How about changing the debug printk just below to use "0x%02x" as well? Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/21] USB: serial: kobil_sct: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Signed-off-by: Johan Hovold --- drivers/usb/serial/kobil_sct.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c index 813035f51fe7..3024b9b25360 100644 --- a/drivers/usb/serial/kobil_sct.c +++ b/drivers/usb/serial/kobil_sct.c @@ -51,7 +51,6 @@ /* Function prototypes */ -static int kobil_attach(struct usb_serial *serial); static int kobil_port_probe(struct usb_serial_port *probe); static int kobil_port_remove(struct usb_serial_port *probe); static int kobil_open(struct tty_struct *tty, struct usb_serial_port *port); @@ -87,7 +86,7 @@ static struct usb_serial_driver kobil_device = { .description = "KOBIL USB smart card terminal", .id_table = id_table, .num_ports =1, - .attach = kobil_attach, + .num_interrupt_out =1, .port_probe = kobil_port_probe, .port_remove = kobil_port_remove, .ioctl =kobil_ioctl, @@ -115,16 +114,6 @@ struct kobil_private { }; -static int kobil_attach(struct usb_serial *serial) -{ - if (serial->num_interrupt_out < serial->num_ports) { - dev_err(&serial->interface->dev, "missing interrupt-out endpoint\n"); - return -ENODEV; - } - - return 0; -} - static int kobil_port_probe(struct usb_serial_port *port) { struct usb_serial *serial = port->serial; -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev
We need to store the minor number each drivers. In case of hidraw, it's minor number stores in struct hidraw. But hiddev's minor is located in struct hid_device. So reallocates for hiddev's minor number. Signed-off-by: Jaejoong Kim --- drivers/hid/usbhid/hiddev.c | 1 + include/linux/hid.h | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c index 700145b..5c2c489 100644 --- a/drivers/hid/usbhid/hiddev.c +++ b/drivers/hid/usbhid/hiddev.c @@ -47,6 +47,7 @@ #define HIDDEV_BUFFER_SIZE 2048 struct hiddev { + int minor; int exist; int open; struct mutex existancelock; diff --git a/include/linux/hid.h b/include/linux/hid.h index 28f38e2b8..643c017 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -541,7 +541,6 @@ struct hid_device { /* device report descriptor */ struct list_head inputs;/* The list of inputs */ void *hiddev; /* The hiddev structure */ void *hidraw; - int minor; /* Hiddev minor number */ int open; /* is the device open by anyone? */ char name[128]; /* Device name */ -- 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
[PATCH 18/21] USB: serial: pl2303: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Signed-off-by: Johan Hovold --- drivers/usb/serial/pl2303.c | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c index ca69eb42071b..60840004568a 100644 --- a/drivers/usb/serial/pl2303.c +++ b/drivers/usb/serial/pl2303.c @@ -221,17 +221,9 @@ static int pl2303_probe(struct usb_serial *serial, static int pl2303_startup(struct usb_serial *serial) { struct pl2303_serial_private *spriv; - unsigned char num_ports = serial->num_ports; enum pl2303_type type = TYPE_01; unsigned char *buf; - if (serial->num_bulk_in < num_ports || - serial->num_bulk_out < num_ports || - serial->num_interrupt_in < num_ports) { - dev_err(&serial->interface->dev, "missing endpoints\n"); - return -ENODEV; - } - spriv = kzalloc(sizeof(*spriv), GFP_KERNEL); if (!spriv) return -ENOMEM; @@ -939,6 +931,9 @@ static struct usb_serial_driver pl2303_device = { }, .id_table = id_table, .num_ports =1, + .num_bulk_in = 1, + .num_bulk_out = 1, + .num_interrupt_in = 1, .bulk_in_size = 256, .bulk_out_size =256, .open = pl2303_open, -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/21] USB: serial: replace runtime overflow check
Since commit 0a8fd1346254 ("USB: fix problems with duplicate endpoint addresses") USB core guarantees that there are no more than 15 endpoint descriptors per type (and altsetting) so the corresponding overflow checks can now be replaced with a compile-time check on the array sizes (and indirectly the maximum number of ports). Signed-off-by: Johan Hovold --- drivers/usb/serial/usb-serial.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index 68af8054..3d8a6c7d34c1 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -729,29 +729,26 @@ static void find_endpoints(struct usb_serial *serial, struct usb_endpoint_descriptor *epd; unsigned int i; + BUILD_BUG_ON(ARRAY_SIZE(epds->bulk_in) < USB_MAXENDPOINTS / 2); + BUILD_BUG_ON(ARRAY_SIZE(epds->bulk_out) < USB_MAXENDPOINTS / 2); + BUILD_BUG_ON(ARRAY_SIZE(epds->interrupt_in) < USB_MAXENDPOINTS / 2); + BUILD_BUG_ON(ARRAY_SIZE(epds->interrupt_out) < USB_MAXENDPOINTS / 2); + iface_desc = serial->interface->cur_altsetting; for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { epd = &iface_desc->endpoint[i].desc; if (usb_endpoint_is_bulk_in(epd)) { dev_dbg(dev, "found bulk in on endpoint %u\n", i); - if (epds->num_bulk_in == MAX_NUM_PORTS) - continue; epds->bulk_in[epds->num_bulk_in++] = epd; } else if (usb_endpoint_is_bulk_out(epd)) { dev_dbg(dev, "found bulk out on endpoint %u\n", i); - if (epds->num_bulk_out == MAX_NUM_PORTS) - continue; epds->bulk_out[epds->num_bulk_out++] = epd; } else if (usb_endpoint_is_int_in(epd)) { dev_dbg(dev, "found interrupt in on endpoint %u\n", i); - if (epds->num_interrupt_in == MAX_NUM_PORTS) - continue; epds->interrupt_in[epds->num_interrupt_in++] = epd; } else if (usb_endpoint_is_int_out(epd)) { dev_dbg(dev, "found interrupt out on endpoint %u\n", i); - if (epds->num_interrupt_out == MAX_NUM_PORTS) - continue; epds->interrupt_out[epds->num_interrupt_out++] = epd; } } -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected
The hid-core announces kernel message which driver is loaded if there is a hiddev, but hiddev's minor number is always zero even though it's not zero. So, we need to store the minor number asked from usb core and do some refactoring work(move from hiddev.c to hiddev.h) to access hiddev in hid-core. Signed-off-by: Jaejoong Kim --- drivers/hid/hid-core.c | 2 +- drivers/hid/usbhid/hiddev.c | 26 +++--- include/linux/hiddev.h | 24 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index e9e87d3..1a0b910 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1695,7 +1695,7 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask) len += sprintf(buf + len, "input"); if (hdev->claimed & HID_CLAIMED_HIDDEV) len += sprintf(buf + len, "%shiddev%d", len ? "," : "", - hdev->minor); + ((struct hiddev *)hdev->hiddev)->minor); if (hdev->claimed & HID_CLAIMED_HIDRAW) len += sprintf(buf + len, "%shidraw%d", len ? "," : "", ((struct hidraw *)hdev->hidraw)->minor); diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c index 5c2c489..ef83d68 100644 --- a/drivers/hid/usbhid/hiddev.c +++ b/drivers/hid/usbhid/hiddev.c @@ -44,29 +44,6 @@ #define HIDDEV_MINOR_BASE 96 #define HIDDEV_MINORS 16 #endif -#define HIDDEV_BUFFER_SIZE 2048 - -struct hiddev { - int minor; - int exist; - int open; - struct mutex existancelock; - wait_queue_head_t wait; - struct hid_device *hid; - struct list_head list; - spinlock_t list_lock; -}; - -struct hiddev_list { - struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE]; - int head; - int tail; - unsigned flags; - struct fasync_struct *fasync; - struct hiddev *hiddev; - struct list_head node; - struct mutex thread_lock; -}; /* * Find a report, given the report's type and ID. The ID can be specified @@ -911,6 +888,9 @@ int hiddev_connect(struct hid_device *hid, unsigned int force) kfree(hiddev); return -1; } + + hiddev->minor = usbhid->intf->minor; + return 0; } diff --git a/include/linux/hiddev.h b/include/linux/hiddev.h index a5dd814..ff3701b 100644 --- a/include/linux/hiddev.h +++ b/include/linux/hiddev.h @@ -32,6 +32,30 @@ * In-kernel definitions. */ +#define HIDDEV_BUFFER_SIZE 2048 + +struct hiddev { + int minor; + int exist; + int open; + struct mutex existancelock; + wait_queue_head_t wait; + struct hid_device *hid; + struct list_head list; + spinlock_t list_lock; +}; + +struct hiddev_list { + struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE]; + int head; + int tail; + unsigned flags; + struct fasync_struct *fasync; + struct hiddev *hiddev; + struct list_head node; + struct mutex thread_lock; +}; + struct hid_device; struct hid_usage; struct hid_field; -- 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
[PATCH 01/21] USB: serial: clean up probe error paths
Clean up the probe error paths by adding a couple of new error labels. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb-serial.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index 4a037b4a79cf..c20d90ed1ef2 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -752,8 +752,8 @@ static int usb_serial_probe(struct usb_interface *interface, serial = create_serial(dev, interface, type); if (!serial) { - module_put(type->driver.owner); - return -ENOMEM; + retval = -ENOMEM; + goto err_put_module; } /* if this device type has a probe function, call it */ @@ -765,9 +765,7 @@ static int usb_serial_probe(struct usb_interface *interface, if (retval) { dev_dbg(ddev, "sub driver rejected device\n"); - usb_serial_put(serial); - module_put(type->driver.owner); - return retval; + goto err_put_serial; } } @@ -849,9 +847,8 @@ static int usb_serial_probe(struct usb_interface *interface, */ if (num_bulk_in == 0 || num_bulk_out == 0) { dev_info(ddev, "PL-2303 hack: descriptors matched but endpoints did not\n"); - usb_serial_put(serial); - module_put(type->driver.owner); - return -ENODEV; + retval = -ENODEV; + goto err_put_serial; } } /* END HORRIBLE HACK FOR PL2303 */ @@ -862,9 +859,8 @@ static int usb_serial_probe(struct usb_interface *interface, num_ports = num_bulk_out; if (num_ports == 0) { dev_err(ddev, "Generic device with no bulk out, not allowed.\n"); - usb_serial_put(serial); - module_put(type->driver.owner); - return -EIO; + retval = -EIO; + goto err_put_serial; } dev_info(ddev, "The \"generic\" usb-serial driver is only for testing and one-off prototypes.\n"); dev_info(ddev, "Tell linux-usb@vger.kernel.org to add your device to a proper driver.\n"); @@ -1085,9 +1081,13 @@ static int usb_serial_probe(struct usb_interface *interface, return 0; probe_error: + retval = -EIO; +err_put_serial: usb_serial_put(serial); +err_put_module: module_put(type->driver.owner); - return -EIO; + + return retval; } static void usb_serial_disconnect(struct usb_interface *interface) -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 21/21] USB: serial: whiteheat: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Note that the driver registers four ports but uses five bulk-endpoint pairs. Signed-off-by: Johan Hovold --- drivers/usb/serial/whiteheat.c | 32 ++-- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/drivers/usb/serial/whiteheat.c b/drivers/usb/serial/whiteheat.c index 5ab65eb1dacc..55cebc1e6fec 100644 --- a/drivers/usb/serial/whiteheat.c +++ b/drivers/usb/serial/whiteheat.c @@ -80,8 +80,6 @@ static int whiteheat_firmware_download(struct usb_serial *serial, static int whiteheat_firmware_attach(struct usb_serial *serial); /* function prototypes for the Connect Tech WhiteHEAT serial converter */ -static int whiteheat_probe(struct usb_serial *serial, - const struct usb_device_id *id); static int whiteheat_attach(struct usb_serial *serial); static void whiteheat_release(struct usb_serial *serial); static int whiteheat_port_probe(struct usb_serial_port *port); @@ -118,7 +116,8 @@ static struct usb_serial_driver whiteheat_device = { .description = "Connect Tech - WhiteHEAT", .id_table = id_table_std, .num_ports =4, - .probe =whiteheat_probe, + .num_bulk_in = 5, + .num_bulk_out = 5, .attach = whiteheat_attach, .release = whiteheat_release, .port_probe = whiteheat_port_probe, @@ -221,33 +220,6 @@ static int whiteheat_firmware_attach(struct usb_serial *serial) * Connect Tech's White Heat serial driver functions */ -static int whiteheat_probe(struct usb_serial *serial, - const struct usb_device_id *id) -{ - struct usb_host_interface *iface_desc; - struct usb_endpoint_descriptor *endpoint; - size_t num_bulk_in = 0; - size_t num_bulk_out = 0; - size_t min_num_bulk; - unsigned int i; - - iface_desc = serial->interface->cur_altsetting; - - for (i = 0; i < iface_desc->desc.bNumEndpoints; i++) { - endpoint = &iface_desc->endpoint[i].desc; - if (usb_endpoint_is_bulk_in(endpoint)) - ++num_bulk_in; - if (usb_endpoint_is_bulk_out(endpoint)) - ++num_bulk_out; - } - - min_num_bulk = COMMAND_PORT + 1; - if (num_bulk_in < min_num_bulk || num_bulk_out < min_num_bulk) - return -ENODEV; - - return 0; -} - static int whiteheat_attach(struct usb_serial *serial) { struct usb_serial_port *command_port; -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 19/21] USB: serial: spcp8x5: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Signed-off-by: Johan Hovold --- drivers/usb/serial/spcp8x5.c | 16 ++-- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/drivers/usb/serial/spcp8x5.c b/drivers/usb/serial/spcp8x5.c index ddfd787c461c..5167b6564c8b 100644 --- a/drivers/usb/serial/spcp8x5.c +++ b/drivers/usb/serial/spcp8x5.c @@ -154,19 +154,6 @@ static int spcp8x5_probe(struct usb_serial *serial, return 0; } -static int spcp8x5_attach(struct usb_serial *serial) -{ - unsigned char num_ports = serial->num_ports; - - if (serial->num_bulk_in < num_ports || - serial->num_bulk_out < num_ports) { - dev_err(&serial->interface->dev, "missing endpoints\n"); - return -ENODEV; - } - - return 0; -} - static int spcp8x5_port_probe(struct usb_serial_port *port) { const struct usb_device_id *id = usb_get_serial_data(port->serial); @@ -488,6 +475,8 @@ static struct usb_serial_driver spcp8x5_device = { }, .id_table = id_table, .num_ports = 1, + .num_bulk_in= 1, + .num_bulk_out = 1, .open = spcp8x5_open, .dtr_rts= spcp8x5_dtr_rts, .carrier_raised = spcp8x5_carrier_raised, @@ -496,7 +485,6 @@ static struct usb_serial_driver spcp8x5_device = { .tiocmget = spcp8x5_tiocmget, .tiocmset = spcp8x5_tiocmset, .probe = spcp8x5_probe, - .attach = spcp8x5_attach, .port_probe = spcp8x5_port_probe, .port_remove= spcp8x5_port_remove, }; -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/21] USB: serial: refactor endpoint sanity checks
This series refactors the endpoint sanity checks by allowing subdrivers to specify a minimum number of endpoints required per type and letting core verify this during probe. Note that the checks are minimum (i.e. we allow further unused endpoints) and are verified only after the subdriver probe callbacks returns where the altsetting may have been changed or an interface may have been rejected based on some other property (e.g. interface number). This series also moves the endpoint descriptor arrays used to determine the port-endpoint mapping of the stack and increases the maximum number of ports a device can register to 16, something which specifically enables the upper eight ports of some Moxa devices. Note that a follow-on series will pass the endpoint descriptors to the subdrivers, something which allows for clean ups of more elaborate sanity checks as well as the port-endpoint mapping to be modified (and some related hacks to be removed). Johan Johan Hovold (21): USB: serial: clean up probe error paths USB: serial: clean up endpoint and port-counter types USB: serial: refactor and clean up endpoint handling USB: serial: allow up to 16 ports per device USB: serial: replace runtime overflow check USB: serial: add endpoint sanity check to core USB: serial: ark3116: simplify endpoint sanity check USB: serial: cyberjack: simplify endpoint check USB: serial: digi_acceleport: simplify endpoint check USB: serial: io_edgeport: simplify and tighten endpoint check USB: serial: iuu_phoenix: simplify endpoint check USB: serial: keyspan_pda: simplify endpoint check USB: serial: kobil_sct: simplify endpoint check USB: serial: mos7720: simplify endpoint check USB: serial: omninet: simplify endpoint check USB: serial: opticon: simplify endpoint check USB: serial: oti6858: simplify endpoint check USB: serial: pl2303: simplify endpoint check USB: serial: spcp8x5: simplify endpoint check USB: serial: symbolserial: simplify endpoint check USB: serial: whiteheat: simplify endpoint check drivers/usb/serial/ark3116.c | 17 +--- drivers/usb/serial/cyberjack.c | 11 +-- drivers/usb/serial/digi_acceleport.c | 23 + drivers/usb/serial/io_edgeport.c | 17 +++- drivers/usb/serial/iuu_phoenix.c | 13 +-- drivers/usb/serial/keyspan_pda.c | 16 +-- drivers/usb/serial/kobil_sct.c | 13 +-- drivers/usb/serial/mos7720.c | 7 +- drivers/usb/serial/omninet.c | 14 +-- drivers/usb/serial/opticon.c | 12 +-- drivers/usb/serial/oti6858.c | 19 +--- drivers/usb/serial/pl2303.c | 11 +-- drivers/usb/serial/spcp8x5.c | 16 +-- drivers/usb/serial/symbolserial.c| 12 +-- drivers/usb/serial/usb-serial.c | 187 ++- drivers/usb/serial/whiteheat.c | 32 +- include/linux/usb/serial.h | 22 +++-- 17 files changed, 154 insertions(+), 288 deletions(-) -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/21] USB: serial: refactor and clean up endpoint handling
Refactor and clean up endpoint handling. This specifically moves the endpoint-descriptor arrays of the stack. Signed-off-by: Johan Hovold --- drivers/usb/serial/usb-serial.c | 155 +--- 1 file changed, 80 insertions(+), 75 deletions(-) diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index e5b859ad15c6..68af8054 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -710,6 +710,53 @@ static const struct tty_port_operations serial_port_ops = { .shutdown = serial_port_shutdown, }; +struct usb_serial_endpoints { + unsigned char num_bulk_in; + unsigned char num_bulk_out; + unsigned char num_interrupt_in; + unsigned char num_interrupt_out; + struct usb_endpoint_descriptor *bulk_in[MAX_NUM_PORTS]; + struct usb_endpoint_descriptor *bulk_out[MAX_NUM_PORTS]; + struct usb_endpoint_descriptor *interrupt_in[MAX_NUM_PORTS]; + struct usb_endpoint_descriptor *interrupt_out[MAX_NUM_PORTS]; +}; + +static void find_endpoints(struct usb_serial *serial, + struct usb_serial_endpoints *epds) +{ + struct device *dev = &serial->interface->dev; + struct usb_host_interface *iface_desc; + struct usb_endpoint_descriptor *epd; + unsigned int i; + + iface_desc = serial->interface->cur_altsetting; + for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { + epd = &iface_desc->endpoint[i].desc; + + if (usb_endpoint_is_bulk_in(epd)) { + dev_dbg(dev, "found bulk in on endpoint %u\n", i); + if (epds->num_bulk_in == MAX_NUM_PORTS) + continue; + epds->bulk_in[epds->num_bulk_in++] = epd; + } else if (usb_endpoint_is_bulk_out(epd)) { + dev_dbg(dev, "found bulk out on endpoint %u\n", i); + if (epds->num_bulk_out == MAX_NUM_PORTS) + continue; + epds->bulk_out[epds->num_bulk_out++] = epd; + } else if (usb_endpoint_is_int_in(epd)) { + dev_dbg(dev, "found interrupt in on endpoint %u\n", i); + if (epds->num_interrupt_in == MAX_NUM_PORTS) + continue; + epds->interrupt_in[epds->num_interrupt_in++] = epd; + } else if (usb_endpoint_is_int_out(epd)) { + dev_dbg(dev, "found interrupt out on endpoint %u\n", i); + if (epds->num_interrupt_out == MAX_NUM_PORTS) + continue; + epds->interrupt_out[epds->num_interrupt_out++] = epd; + } + } +} + static int usb_serial_probe(struct usb_interface *interface, const struct usb_device_id *id) { @@ -719,19 +766,12 @@ static int usb_serial_probe(struct usb_interface *interface, struct usb_serial_port *port; struct usb_host_interface *iface_desc; struct usb_endpoint_descriptor *endpoint; - struct usb_endpoint_descriptor *interrupt_in_endpoint[MAX_NUM_PORTS]; - struct usb_endpoint_descriptor *interrupt_out_endpoint[MAX_NUM_PORTS]; - struct usb_endpoint_descriptor *bulk_in_endpoint[MAX_NUM_PORTS]; - struct usb_endpoint_descriptor *bulk_out_endpoint[MAX_NUM_PORTS]; + struct usb_serial_endpoints *epds; struct usb_serial_driver *type = NULL; int retval; int buffer_size; int i; int j; - unsigned char num_interrupt_in = 0; - unsigned char num_interrupt_out = 0; - unsigned char num_bulk_in = 0; - unsigned char num_bulk_out = 0; int num_ports = 0; unsigned char max_endpoints; @@ -770,50 +810,14 @@ static int usb_serial_probe(struct usb_interface *interface, } /* descriptor matches, let's find the endpoints needed */ - /* check out the endpoints */ - iface_desc = interface->cur_altsetting; - for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { - endpoint = &iface_desc->endpoint[i].desc; - - if (usb_endpoint_is_bulk_in(endpoint)) { - /* we found a bulk in endpoint */ - dev_dbg(ddev, "found bulk in on endpoint %d\n", i); - if (num_bulk_in < MAX_NUM_PORTS) { - bulk_in_endpoint[num_bulk_in] = endpoint; - ++num_bulk_in; - } - } - - if (usb_endpoint_is_bulk_out(endpoint)) { - /* we found a bulk out endpoint */ - dev_dbg(ddev, "found bulk out on endpoint %d\n", i); - if (num_bulk_out < MAX_NUM_PORTS) { - bulk_out_endpoint[num_bulk
Re: [PATCH] usb: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase
Hi, Alan Stern writes: >> Alan Stern writes: >> >> So I am not sure how the Gadget driver can figure out that it needs to >> >> usb_ep_queue() another request for status stage when handling the >> >> no-data control? >> > >> > Gadget drivers already queue status-stage requests for no-data >> > control-OUT requests. The difficulty comes when you want to handle an >> > IN request or an OUT request with a data stage. >> >> I don't see a difficulty there. Gadget driver will see wLength and >> notice it needs both data and status stages, then it does: >> >> usb_ep_queue(ep0, data_req, GFP_KERNEL); >> usb_ep_queue(ep0, status_req, GFP_KERNEL); > > The main difficulty is that all the gadget/function drivers will have > to be audited to add the status requests. yeah, that's a given and was also mentioned in this thread somewhere. >> Just needs to prepare both requests and queue them both ahead of >> time. UDC drivers should hold both requests in their own private list >> and process one at a time. > > Or the gadget driver should queue the status request after the > data stage has been fully processed, in the case of an OUT transfer. right, we could use ->complete() for that. > There is still a possible race. The host might send another SETUP > packet before the status request has been queued, or after it has been we should also have code for this race since it would happen with USB_GADGET_DELAYED_STATUS. > queued but before the UDC driver has completed it. (Of course, that's > already true now for the data request...) right -- balbi signature.asc Description: PGP signature
[PATCH 04/21] USB: serial: allow up to 16 ports per device
Raise the arbitrary limit of how many ports a single device can claim from eight to 16. This specifically enables the upper eight ports of some mxuport devices. Signed-off-by: Johan Hovold --- include/linux/usb/serial.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h index 85b475933848..ee4394d8932f 100644 --- a/include/linux/usb/serial.h +++ b/include/linux/usb/serial.h @@ -20,7 +20,7 @@ #include /* The maximum number of ports one device can grab at once */ -#define MAX_NUM_PORTS 8 +#define MAX_NUM_PORTS 16 /* parity check flag */ #define RELEVANT_IFLAG(iflag) (iflag & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK)) -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/21] USB: serial: add endpoint sanity check to core
Allow drivers to specify a minimum number of endpoints per type, which USB serial core will verify after subdriver probe has returned (where the current alternate setting may have been changed). Signed-off-by: Johan Hovold --- drivers/usb/serial/usb-serial.c | 9 - include/linux/usb/serial.h | 9 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index 3d8a6c7d34c1..63429b23335a 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -852,7 +852,14 @@ static int usb_serial_probe(struct usb_interface *interface, } /* END HORRIBLE HACK FOR PL2303 */ #endif - + if (epds->num_bulk_in < type->num_bulk_in || + epds->num_bulk_out < type->num_bulk_out || + epds->num_interrupt_in < type->num_interrupt_in || + epds->num_interrupt_out < type->num_interrupt_out) { + dev_err(ddev, "required endpoints missing\n"); + retval = -ENODEV; + goto err_free_epds; + } #ifdef CONFIG_USB_SERIAL_GENERIC if (type == &usb_serial_generic_device) { num_ports = epds->num_bulk_out; diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h index ee4394d8932f..f1b8a8493762 100644 --- a/include/linux/usb/serial.h +++ b/include/linux/usb/serial.h @@ -188,6 +188,10 @@ static inline void usb_set_serial_data(struct usb_serial *serial, void *data) * @id_table: pointer to a list of usb_device_id structures that define all * of the devices this structure can support. * @num_ports: the number of different ports this device will have. + * @num_bulk_in: minimum number of bulk-in endpoints + * @num_bulk_out: minimum number of bulk-out endpoints + * @num_interrupt_in: minimum number of interrupt-in endpoints + * @num_interrupt_out: minimum number of interrupt-out endpoints * @bulk_in_size: minimum number of bytes to allocate for bulk-in buffer * (0 = end-point size) * @bulk_out_size: bytes to allocate for bulk-out buffer (0 = end-point size) @@ -235,6 +239,11 @@ struct usb_serial_driver { unsigned char num_ports; + unsigned char num_bulk_in; + unsigned char num_bulk_out; + unsigned char num_interrupt_in; + unsigned char num_interrupt_out; + size_t bulk_in_size; size_t bulk_out_size; -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/21] USB: serial: io_edgeport: simplify and tighten endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Also require the presence of a bulk-out endpoint, something which prevents the driver from trying to send bulk messages over the control pipe should a bulk-out endpoint be missing. Signed-off-by: Johan Hovold --- drivers/usb/serial/io_edgeport.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c index bb7673e80a57..751e7454c37b 100644 --- a/drivers/usb/serial/io_edgeport.c +++ b/drivers/usb/serial/io_edgeport.c @@ -2848,11 +2848,6 @@ static int edge_startup(struct usb_serial *serial) EDGE_COMPATIBILITY_MASK1, EDGE_COMPATIBILITY_MASK2 }; - if (serial->num_bulk_in < 1 || serial->num_interrupt_in < 1) { - dev_err(&serial->interface->dev, "missing endpoints\n"); - return -ENODEV; - } - dev = serial->dev; /* create our private serial structure */ @@ -3120,6 +3115,9 @@ static struct usb_serial_driver edgeport_2port_device = { .description= "Edgeport 2 port adapter", .id_table = edgeport_2port_id_table, .num_ports = 2, + .num_bulk_in= 1, + .num_bulk_out = 1, + .num_interrupt_in = 1, .open = edge_open, .close = edge_close, .throttle = edge_throttle, @@ -3152,6 +3150,9 @@ static struct usb_serial_driver edgeport_4port_device = { .description= "Edgeport 4 port adapter", .id_table = edgeport_4port_id_table, .num_ports = 4, + .num_bulk_in= 1, + .num_bulk_out = 1, + .num_interrupt_in = 1, .open = edge_open, .close = edge_close, .throttle = edge_throttle, @@ -3184,6 +3185,9 @@ static struct usb_serial_driver edgeport_8port_device = { .description= "Edgeport 8 port adapter", .id_table = edgeport_8port_id_table, .num_ports = 8, + .num_bulk_in= 1, + .num_bulk_out = 1, + .num_interrupt_in = 1, .open = edge_open, .close = edge_close, .throttle = edge_throttle, @@ -3216,6 +3220,9 @@ static struct usb_serial_driver epic_device = { .description= "EPiC device", .id_table = Epic_port_id_table, .num_ports = 1, + .num_bulk_in= 1, + .num_bulk_out = 1, + .num_interrupt_in = 1, .open = edge_open, .close = edge_close, .throttle = edge_throttle, -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/21] USB: serial: ark3116: simplify endpoint sanity check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Signed-off-by: Johan Hovold --- drivers/usb/serial/ark3116.c | 17 +++-- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c index 2779e59c30f1..0adbd38b4eea 100644 --- a/drivers/usb/serial/ark3116.c +++ b/drivers/usb/serial/ark3116.c @@ -122,19 +122,6 @@ static inline int calc_divisor(int bps) return (1200 + 2*bps) / (4*bps); } -static int ark3116_attach(struct usb_serial *serial) -{ - /* make sure we have our end-points */ - if (serial->num_bulk_in == 0 || - serial->num_bulk_out == 0 || - serial->num_interrupt_in == 0) { - dev_err(&serial->interface->dev, "missing endpoint\n"); - return -ENODEV; - } - - return 0; -} - static int ark3116_port_probe(struct usb_serial_port *port) { struct usb_serial *serial = port->serial; @@ -671,7 +658,9 @@ static struct usb_serial_driver ark3116_device = { }, .id_table = id_table, .num_ports =1, - .attach = ark3116_attach, + .num_bulk_in = 1, + .num_bulk_out = 1, + .num_interrupt_in = 1, .port_probe = ark3116_port_probe, .port_remove = ark3116_port_remove, .set_termios = ark3116_set_termios, -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 15/21] USB: serial: omninet: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Note that the driver uses the second bulk-out endpoint for writing. Signed-off-by: Johan Hovold --- drivers/usb/serial/omninet.c | 14 +- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/drivers/usb/serial/omninet.c b/drivers/usb/serial/omninet.c index a180b17d2432..b1685c2ef544 100644 --- a/drivers/usb/serial/omninet.c +++ b/drivers/usb/serial/omninet.c @@ -38,7 +38,6 @@ static int omninet_write(struct tty_struct *tty, struct usb_serial_port *port, const unsigned char *buf, int count); static int omninet_write_room(struct tty_struct *tty); static void omninet_disconnect(struct usb_serial *serial); -static int omninet_attach(struct usb_serial *serial); static int omninet_port_probe(struct usb_serial_port *port); static int omninet_port_remove(struct usb_serial_port *port); @@ -57,7 +56,7 @@ static struct usb_serial_driver zyxel_omninet_device = { .description = "ZyXEL - omni.net lcd plus usb", .id_table = id_table, .num_ports =1, - .attach = omninet_attach, + .num_bulk_out = 2, .port_probe = omninet_port_probe, .port_remove = omninet_port_remove, .open = omninet_open, @@ -106,17 +105,6 @@ struct omninet_data { __u8od_outseq; /* Sequence number for bulk_out URBs */ }; -static int omninet_attach(struct usb_serial *serial) -{ - /* The second bulk-out endpoint is used for writing. */ - if (serial->num_bulk_out < 2) { - dev_err(&serial->interface->dev, "missing endpoints\n"); - return -ENODEV; - } - - return 0; -} - static int omninet_port_probe(struct usb_serial_port *port) { struct omninet_data *od; -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/21] USB: serial: iuu_phoenix: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Signed-off-by: Johan Hovold --- drivers/usb/serial/iuu_phoenix.c | 13 ++--- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/usb/serial/iuu_phoenix.c b/drivers/usb/serial/iuu_phoenix.c index 030390f37b0a..7dd1601e4a02 100644 --- a/drivers/usb/serial/iuu_phoenix.c +++ b/drivers/usb/serial/iuu_phoenix.c @@ -68,16 +68,6 @@ struct iuu_private { u32 clk; }; -static int iuu_attach(struct usb_serial *serial) -{ - unsigned char num_ports = serial->num_ports; - - if (serial->num_bulk_in < num_ports || serial->num_bulk_out < num_ports) - return -ENODEV; - - return 0; -} - static int iuu_port_probe(struct usb_serial_port *port) { struct iuu_private *priv; @@ -1183,6 +1173,8 @@ static struct usb_serial_driver iuu_device = { }, .id_table = id_table, .num_ports = 1, + .num_bulk_in = 1, + .num_bulk_out = 1, .bulk_in_size = 512, .bulk_out_size = 512, .open = iuu_open, @@ -1193,7 +1185,6 @@ static struct usb_serial_driver iuu_device = { .tiocmset = iuu_tiocmset, .set_termios = iuu_set_termios, .init_termios = iuu_init_termios, - .attach = iuu_attach, .port_probe = iuu_port_probe, .port_remove = iuu_port_remove, }; -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/21] USB: serial: cyberjack: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Signed-off-by: Johan Hovold --- drivers/usb/serial/cyberjack.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/usb/serial/cyberjack.c b/drivers/usb/serial/cyberjack.c index 80260b08398b..47fbd9f0c0c7 100644 --- a/drivers/usb/serial/cyberjack.c +++ b/drivers/usb/serial/cyberjack.c @@ -50,7 +50,6 @@ #define CYBERJACK_PRODUCT_ID 0x0100 /* Function prototypes */ -static int cyberjack_attach(struct usb_serial *serial); static int cyberjack_port_probe(struct usb_serial_port *port); static int cyberjack_port_remove(struct usb_serial_port *port); static int cyberjack_open(struct tty_struct *tty, @@ -78,7 +77,7 @@ static struct usb_serial_driver cyberjack_device = { .description = "Reiner SCT Cyberjack USB card reader", .id_table = id_table, .num_ports =1, - .attach = cyberjack_attach, + .num_bulk_out = 1, .port_probe = cyberjack_port_probe, .port_remove = cyberjack_port_remove, .open = cyberjack_open, @@ -102,14 +101,6 @@ struct cyberjack_private { short wrsent; /* Data already sent */ }; -static int cyberjack_attach(struct usb_serial *serial) -{ - if (serial->num_bulk_out < serial->num_ports) - return -ENODEV; - - return 0; -} - static int cyberjack_port_probe(struct usb_serial_port *port) { struct cyberjack_private *priv; -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 20/21] USB: serial: symbolserial: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Signed-off-by: Johan Hovold --- drivers/usb/serial/symbolserial.c | 12 +--- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/drivers/usb/serial/symbolserial.c b/drivers/usb/serial/symbolserial.c index 37f3ad15ed06..0d1727232d0c 100644 --- a/drivers/usb/serial/symbolserial.c +++ b/drivers/usb/serial/symbolserial.c @@ -147,16 +147,6 @@ static void symbol_unthrottle(struct tty_struct *tty) } } -static int symbol_startup(struct usb_serial *serial) -{ - if (!serial->num_interrupt_in) { - dev_err(&serial->dev->dev, "no interrupt-in endpoint\n"); - return -ENODEV; - } - - return 0; -} - static int symbol_port_probe(struct usb_serial_port *port) { struct symbol_private *priv; @@ -188,7 +178,7 @@ static struct usb_serial_driver symbol_device = { }, .id_table = id_table, .num_ports =1, - .attach = symbol_startup, + .num_interrupt_in = 1, .port_probe = symbol_port_probe, .port_remove = symbol_port_remove, .open = symbol_open, -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 17/21] USB: serial: oti6858: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Signed-off-by: Johan Hovold --- drivers/usb/serial/oti6858.c | 19 +++ 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/drivers/usb/serial/oti6858.c b/drivers/usb/serial/oti6858.c index b8bf52bf7a94..b11eead469ee 100644 --- a/drivers/usb/serial/oti6858.c +++ b/drivers/usb/serial/oti6858.c @@ -134,7 +134,6 @@ static int oti6858_chars_in_buffer(struct tty_struct *tty); static int oti6858_tiocmget(struct tty_struct *tty); static int oti6858_tiocmset(struct tty_struct *tty, unsigned int set, unsigned int clear); -static int oti6858_attach(struct usb_serial *serial); static int oti6858_port_probe(struct usb_serial_port *port); static int oti6858_port_remove(struct usb_serial_port *port); @@ -146,6 +145,9 @@ static struct usb_serial_driver oti6858_device = { }, .id_table = id_table, .num_ports =1, + .num_bulk_in = 1, + .num_bulk_out = 1, + .num_interrupt_in = 1, .open = oti6858_open, .close =oti6858_close, .write =oti6858_write, @@ -159,7 +161,6 @@ static struct usb_serial_driver oti6858_device = { .write_bulk_callback = oti6858_write_bulk_callback, .write_room = oti6858_write_room, .chars_in_buffer = oti6858_chars_in_buffer, - .attach = oti6858_attach, .port_probe = oti6858_port_probe, .port_remove = oti6858_port_remove, }; @@ -326,20 +327,6 @@ static void send_data(struct work_struct *work) usb_serial_port_softint(port); } -static int oti6858_attach(struct usb_serial *serial) -{ - unsigned char num_ports = serial->num_ports; - - if (serial->num_bulk_in < num_ports || - serial->num_bulk_out < num_ports || - serial->num_interrupt_in < num_ports) { - dev_err(&serial->interface->dev, "missing endpoints\n"); - return -ENODEV; - } - - return 0; -} - static int oti6858_port_probe(struct usb_serial_port *port) { struct oti6858_private *priv; -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/21] USB: serial: keyspan_pda: simplify endpoint check
Simplify the endpoint sanity check by letting core verify that the required endpoints are present. Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 16 ++-- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index d2dab2a341b8..196908dd25a1 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -708,19 +708,6 @@ MODULE_FIRMWARE("keyspan_pda/keyspan_pda.fw"); MODULE_FIRMWARE("keyspan_pda/xircom_pgs.fw"); #endif -static int keyspan_pda_attach(struct usb_serial *serial) -{ - unsigned char num_ports = serial->num_ports; - - if (serial->num_bulk_out < num_ports || - serial->num_interrupt_in < num_ports) { - dev_err(&serial->interface->dev, "missing endpoints\n"); - return -ENODEV; - } - - return 0; -} - static int keyspan_pda_port_probe(struct usb_serial_port *port) { @@ -784,6 +771,8 @@ static struct usb_serial_driver keyspan_pda_device = { .description = "Keyspan PDA", .id_table = id_table_std, .num_ports =1, + .num_bulk_out = 1, + .num_interrupt_in = 1, .dtr_rts = keyspan_pda_dtr_rts, .open = keyspan_pda_open, .close =keyspan_pda_close, @@ -798,7 +787,6 @@ static struct usb_serial_driver keyspan_pda_device = { .break_ctl =keyspan_pda_break_ctl, .tiocmget = keyspan_pda_tiocmget, .tiocmset = keyspan_pda_tiocmset, - .attach = keyspan_pda_attach, .port_probe = keyspan_pda_port_probe, .port_remove = keyspan_pda_port_remove, }; -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: cppi41: don't check early-TX-interrupt for Isoch transfer
On 03/02/2017 03:52 AM, Bin Liu wrote: > The CPPI41 driver polls register to workaround the pre-mature TX > interrupt issue, but it causes audio playback underrun when triggered in > Isoch transfers. > > Isoch doesn't do back-to-back transfers, the TX should be done by the > time the next transfer is scheduled. So skip this polling workaround for > Isoch transfer. > > Fixes: a655f481d83d6 ("usb: musb: musb_cppi41: handle pre-mature TX complete > interrupt") > Cc: #4.1+ > Reported-by: Alexandre Bailon > Signed-off-by: Bin Liu > --- > drivers/usb/musb/musb_cppi41.c | 23 +-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c > index 00e272bfee39..355655f8a3fb 100644 > --- a/drivers/usb/musb/musb_cppi41.c > +++ b/drivers/usb/musb/musb_cppi41.c > @@ -238,8 +238,27 @@ static void cppi41_dma_callback(void *private_data, > transferred < cppi41_channel->packet_sz) > cppi41_channel->prog_len = 0; > > - if (cppi41_channel->is_tx) > - empty = musb_is_tx_fifo_empty(hw_ep); > + if (cppi41_channel->is_tx) { > + u8 type; > + > + if (is_host_active(musb)) > + type = hw_ep->out_qh->type; > + else > + type = hw_ep->ep_in.type; > + > + if (type == USB_ENDPOINT_XFER_ISOC) > + /* > + * Don't use the early-TX-interrupt workaround below > + * for Isoch transfter. Since Isoch are periodic > + * transfer, by the time the next transfer is > + * scheduled, the current one should be done already. > + * > + * This avoids audio playback underrun issue. > + */ > + empty = true; > + else > + empty = musb_is_tx_fifo_empty(hw_ep); > + } > > if (!cppi41_channel->is_tx || empty) { > cppi41_trans_done(cppi41_channel); > Tested on omapl138-lcdk and it significantly decreases the cpu load and then fixes the underruns. Tested-by: Alexandre Bailon -- To unsubscribe from this list: send the line "unsubscribe 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: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase
Hi, Baolin Wang writes: >>> > Baolin Wang writes: >>> (One possible approach would be to have the setup routine return >>> different values for explicit and implicit status stages -- for >>> example, return 1 if it wants to submit an explicit status request. >>> That wouldn't be very different from the current >>> USB_GADGET_DELAYED_STATUS approach.) >>> >>> >>> >>> not really, no. The idea was for composite.c and/or functions to support >>> >>> both methods (temporarily) and use "gadget->wants_explicit_stages" to >>> >>> explicitly queue DATA and STATUS. That would mean that f_mass_storage >>> >>> wouldn't have to return DELAYED_STATUS if >>> >>> (gadget->wants_explicit_stages). >>> >>> >>> >>> After all UDCs are converted over and set wants_explicit_stages (which >>> >>> should all be done in a single series), then we get rid of the flag and >>> >>> the older method of DELAYED_STATUS. >>> >> >>> >> (Sorry for late reply due to my holiday) >>> >> I also met the problem pointed by Alan, from my test, I still want to >>> >> need one return value to indicate if it wants to submit an explicit >>> >> status request. Think about the Control-IN with a data stage, we can >>> >> not get the STATUS phase request from usb_ep_queue() call, and we need >>> > >>> > why not? wLength tells you that this is a 3-stage transfer. Gadget >>> > driver should be able to figure out that it needs to usb_ep_queue() >>> > another request for status stage. >>> >>> I tried again, but still can not work. Suppose the no-data control: >>> (1) SET_ADDRESS request: function driver will not queue one request >>> for status phase by usb_ep_queue() call. >> >> Function drivers do not handle Set-Address requests at all. The UDC >> driver handles these requests without telling the gadget driver about >> them. > > Correct. What I mean is it will not queue one request for status phase > by usb_ep_queue() call, UDC driver will do that. how the UDC driver handles this case, is up to the UDC driver. In DWC3 I chose to rely on the same ep_queue mechanism; but that's an arbitrary choice. >>> (2) SET_CONFIGURATION request: function driver will queue one 0-length >>> request for status phase by usb_ep_queue() call, especially for >>> mass_storage driver, it will queue one request for status phase >>> later. >>> >>> So I am not sure how the Gadget driver can figure out that it needs to >>> usb_ep_queue() another request for status stage when handling the >>> no-data control? >> >> Gadget drivers already queue status-stage requests for no-data >> control-OUT requests. The difficulty comes when you want to handle an >> IN request or an OUT request with a data stage. >> > > I try to explain that explicitly, In dwc3 driver, we can handle the > STATUS phase request in 2 places: (1) in usb_ep_queue(), (2) in > dwc3_ep0_xfernotready() this is the very detail that what I proposed will change. After what I proposed is implemented, status stage will *always* be done in response to a usb_ep_queue(). > For no-data control-OUT requests: > (1) SET_ADDRESS request: no request queued for status phase by > usb_ep_queue(), dwc3 driver need handle the STATUS phase request when > one not-ready-event comes in dwc3_ep0_xfernotready() function. or we change dwc3 to prepare an internal request and queue it to its own enpdoint. > (2) SET_CONFIGURATION request: function driver will queue one 0-length > request for status phase by usb_ep_queue(), but we can handle this > request in usb_ep_queue() or dwc3_ep0_xfernotready(). When the for DWC3, status stage *must* be done after XFER_NOT_READY event. That's required by the databook. What you're claiming is not correct. The only situation where we start status stage from usb_ep_queue() is for the case when XFER_NOT_READY already triggered and we set PENDING_REQUEST flag for the endpoint. > function driver queued one 0-length request for status phase before > the not-ready-event comes, we need handle this request in > dwc3_ep0_xfernotready() when the not-ready-event comes. When the > function driver queued one 0-length request for status phase after the > not-ready-event comes, we can handle this request in usb_ep_queue(). already implemented. Nothing will change for this case. > So in dwc3_ep0_xfernotready(), we need to check if the request for > status phase has been queued into pending request list, but if the > pending request list is NULL, which means the function driver have not > queued one 0-length request until now (then we can handle it in > usb_ep_queue()), or situation (1) (no request queued for status > phase), then I can not identify this 2 situations to determine where I > can handle the status request. Hope I make it clear. this is already implemented. There's nothing new coming to this case. -- balbi signature.asc Description: PGP signature
Re: [PATCH] Revert "usb: gadget: uvc: Add missing call for additional setup data"
Hi Roger, Thank you for the patch. On Thursday 02 Mar 2017 10:44:58 Roger Quadros wrote: > This reverts commit 4fbac5206afd01b717d4bdc58793d471f3391b4b. > > This commit breaks g_webcam when used with uvc-gadget [1]. > > The user space application (e.g. uvc-gadget) is responsible for > sending response to UVC class specific requests on control endpoint > in uvc_send_response() in uvc_v4l2.c. > > The bad commit was causing a duplicate response to be sent with > incorrect response data thus causing UVC probe to fail at the host > and broken control transfer endpoint at the gadget. > > [1] - git://git.ideasonboard.org/uvc-gadget.git > > Cc: # v4.9+ > Signed-off-by: Roger Quadros Acked-by: Laurent Pinchart > --- > drivers/usb/gadget/function/f_uvc.c | 7 --- > 1 file changed, 7 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_uvc.c > b/drivers/usb/gadget/function/f_uvc.c index 27ed51b..29b41b5 100644 > --- a/drivers/usb/gadget/function/f_uvc.c > +++ b/drivers/usb/gadget/function/f_uvc.c > @@ -258,13 +258,6 @@ uvc_function_setup(struct usb_function *f, const struct > usb_ctrlrequest *ctrl) memcpy(&uvc_event->req, ctrl, > sizeof(uvc_event->req)); > v4l2_event_queue(&uvc->vdev, &v4l2_event); > > - /* Pass additional setup data to userspace */ > - if (uvc->event_setup_out && uvc->event_length) { > - uvc->control_req->length = uvc->event_length; > - return usb_ep_queue(uvc->func.config->cdev->gadget->ep0, > - uvc->control_req, GFP_ATOMIC); > - } > - > return 0; > } -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe 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: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase
Hi, On 28 February 2017 at 06:11, Alan Stern wrote: > On Tue, 21 Feb 2017, Baolin Wang wrote: > >> On 17 February 2017 at 16:04, Felipe Balbi wrote: >> > >> > Hi, >> > >> > Baolin Wang writes: >> (One possible approach would be to have the setup routine return >> different values for explicit and implicit status stages -- for >> example, return 1 if it wants to submit an explicit status request. >> That wouldn't be very different from the current >> USB_GADGET_DELAYED_STATUS approach.) >> >>> >> >>> not really, no. The idea was for composite.c and/or functions to support >> >>> both methods (temporarily) and use "gadget->wants_explicit_stages" to >> >>> explicitly queue DATA and STATUS. That would mean that f_mass_storage >> >>> wouldn't have to return DELAYED_STATUS if >> >>> (gadget->wants_explicit_stages). >> >>> >> >>> After all UDCs are converted over and set wants_explicit_stages (which >> >>> should all be done in a single series), then we get rid of the flag and >> >>> the older method of DELAYED_STATUS. >> >> >> >> (Sorry for late reply due to my holiday) >> >> I also met the problem pointed by Alan, from my test, I still want to >> >> need one return value to indicate if it wants to submit an explicit >> >> status request. Think about the Control-IN with a data stage, we can >> >> not get the STATUS phase request from usb_ep_queue() call, and we need >> > >> > why not? wLength tells you that this is a 3-stage transfer. Gadget >> > driver should be able to figure out that it needs to usb_ep_queue() >> > another request for status stage. >> >> I tried again, but still can not work. Suppose the no-data control: >> (1) SET_ADDRESS request: function driver will not queue one request >> for status phase by usb_ep_queue() call. > > Function drivers do not handle Set-Address requests at all. The UDC > driver handles these requests without telling the gadget driver about > them. Correct. What I mean is it will not queue one request for status phase by usb_ep_queue() call, UDC driver will do that. > >> (2) SET_CONFIGURATION request: function driver will queue one 0-length >> request for status phase by usb_ep_queue() call, especially for >> mass_storage driver, it will queue one request for status phase >> later. >> >> So I am not sure how the Gadget driver can figure out that it needs to >> usb_ep_queue() another request for status stage when handling the >> no-data control? > > Gadget drivers already queue status-stage requests for no-data > control-OUT requests. The difficulty comes when you want to handle an > IN request or an OUT request with a data stage. > I try to explain that explicitly, In dwc3 driver, we can handle the STATUS phase request in 2 places: (1) in usb_ep_queue(), (2) in dwc3_ep0_xfernotready() For no-data control-OUT requests: (1) SET_ADDRESS request: no request queued for status phase by usb_ep_queue(), dwc3 driver need handle the STATUS phase request when one not-ready-event comes in dwc3_ep0_xfernotready() function. (2) SET_CONFIGURATION request: function driver will queue one 0-length request for status phase by usb_ep_queue(), but we can handle this request in usb_ep_queue() or dwc3_ep0_xfernotready(). When the function driver queued one 0-length request for status phase before the not-ready-event comes, we need handle this request in dwc3_ep0_xfernotready() when the not-ready-event comes. When the function driver queued one 0-length request for status phase after the not-ready-event comes, we can handle this request in usb_ep_queue(). So in dwc3_ep0_xfernotready(), we need to check if the request for status phase has been queued into pending request list, but if the pending request list is NULL, which means the function driver have not queued one 0-length request until now (then we can handle it in usb_ep_queue()), or situation (1) (no request queued for status phase), then I can not identify this 2 situations to determine where I can handle the status request. Hope I make it clear. Your concern about an IN request or an OUT request with a data stage, I agree with Felipe and we can identify. Thanks. -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe 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: musb: cppi41: don't check early-TX-interrupt for Isoch transfer
Hello! On 3/2/2017 5:52 AM, Bin Liu wrote: The CPPI41 driver polls register to workaround the pre-mature TX It's CPPI 4.1. And "premature". interrupt issue, but it causes audio playback underrun when triggered in Isoch transfers. Isoch doesn't do back-to-back transfers, the TX should be done by the time the next transfer is scheduled. So skip this polling workaround for Isoch transfer. Fixes: a655f481d83d6 ("usb: musb: musb_cppi41: handle pre-mature TX complete interrupt") Cc: #4.1+ Reported-by: Alexandre Bailon Signed-off-by: Bin Liu [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 11/19] scsi: megaraid: Replace PCI pool old API
>-Original Message- >From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- >ow...@vger.kernel.org] On Behalf Of Romain Perier >Sent: Wednesday, March 01, 2017 9:25 PM >To: Dan Williams; Doug Ledford; Sean Hefty; Hal Rosenstock; >jeffrey.t.kirs...@intel.com; David S. Miller; stas.yakov...@gmail.com; James E.J. >Bottomley; Martin K. Petersen; Felipe Balbi; Greg Kroah-Hartman >Cc: linux-r...@vger.kernel.org; net...@vger.kernel.org; linux- >u...@vger.kernel.org; linux-s...@vger.kernel.org; linux-ker...@vger.kernel.org; >Romain Perier; Peter Senna Tschudin >Subject: [PATCH v4 11/19] scsi: megaraid: Replace PCI pool old API > >The PCI pool API is deprecated. This commits replaces the PCI pool old API by the >appropriated function with the DMA pool API. > >Signed-off-by: Romain Perier >Reviewed-by: Peter Senna Tschudin >--- > drivers/scsi/megaraid/megaraid_mbox.c | 33 +++ > drivers/scsi/megaraid/megaraid_mm.c | 32 +++--- > drivers/scsi/megaraid/megaraid_sas_base.c | 29 +++-- > drivers/scsi/megaraid/megaraid_sas_fusion.c | 66 + > 4 files changed, 77 insertions(+), 83 deletions(-) > >diff --git a/drivers/scsi/megaraid/megaraid_mbox.c >b/drivers/scsi/megaraid/megaraid_mbox.c >index f0987f2..7dfc2e2 100644 >--- a/drivers/scsi/megaraid/megaraid_mbox.c >+++ b/drivers/scsi/megaraid/megaraid_mbox.c >@@ -1153,8 +1153,8 @@ megaraid_mbox_setup_dma_pools(adapter_t >*adapter) > > > // Allocate memory for 16-bytes aligned mailboxes >- raid_dev->mbox_pool_handle = pci_pool_create("megaraid mbox pool", >- adapter->pdev, >+ raid_dev->mbox_pool_handle = dma_pool_create("megaraid mbox >pool", >+ &adapter->pdev->dev, > sizeof(mbox64_t) + 16, > 16, 0); > >@@ -1164,7 +1164,7 @@ megaraid_mbox_setup_dma_pools(adapter_t >*adapter) > > mbox_pci_blk = raid_dev->mbox_pool; > for (i = 0; i < MBOX_MAX_SCSI_CMDS; i++) { >- mbox_pci_blk[i].vaddr = pci_pool_alloc( >+ mbox_pci_blk[i].vaddr = dma_pool_alloc( > raid_dev->mbox_pool_handle, > GFP_KERNEL, > &mbox_pci_blk[i].dma_addr); >@@ -1181,8 +1181,8 @@ megaraid_mbox_setup_dma_pools(adapter_t >*adapter) >* share common memory pool. Passthru structures piggyback on >memory >* allocted to extended passthru since passthru is smaller of the two >*/ >- raid_dev->epthru_pool_handle = pci_pool_create("megaraid mbox >pthru", >- adapter->pdev, sizeof(mraid_epassthru_t), 128, 0); >+ raid_dev->epthru_pool_handle = dma_pool_create("megaraid mbox >pthru", >+ &adapter->pdev->dev, sizeof(mraid_epassthru_t), 128, >0); > > if (raid_dev->epthru_pool_handle == NULL) { > goto fail_setup_dma_pool; >@@ -1190,7 +1190,7 @@ megaraid_mbox_setup_dma_pools(adapter_t >*adapter) > > epthru_pci_blk = raid_dev->epthru_pool; > for (i = 0; i < MBOX_MAX_SCSI_CMDS; i++) { >- epthru_pci_blk[i].vaddr = pci_pool_alloc( >+ epthru_pci_blk[i].vaddr = dma_pool_alloc( > raid_dev->epthru_pool_handle, > GFP_KERNEL, > &epthru_pci_blk[i].dma_addr); >@@ -1202,8 +1202,8 @@ megaraid_mbox_setup_dma_pools(adapter_t >*adapter) > > // Allocate memory for each scatter-gather list. Request for 512 bytes > // alignment for each sg list >- raid_dev->sg_pool_handle = pci_pool_create("megaraid mbox sg", >- adapter->pdev, >+ raid_dev->sg_pool_handle = dma_pool_create("megaraid mbox sg", >+ &adapter->pdev->dev, > sizeof(mbox_sgl64) * >MBOX_MAX_SG_SIZE, > 512, 0); > >@@ -1213,7 +1213,7 @@ megaraid_mbox_setup_dma_pools(adapter_t >*adapter) > > sg_pci_blk = raid_dev->sg_pool; > for (i = 0; i < MBOX_MAX_SCSI_CMDS; i++) { >- sg_pci_blk[i].vaddr = pci_pool_alloc( >+ sg_pci_blk[i].vaddr = dma_pool_alloc( > raid_dev->sg_pool_handle, > GFP_KERNEL, > &sg_pci_blk[i].dma_addr); >@@ -1249,29 +1249,26 @@ megaraid_mbox_teardown_dma_pools(adapter_t >*adapter) > > sg_pci_blk = raid_dev->sg_pool; > for (i = 0; i < MBOX_MAX_SCSI_CMDS && sg_pci_blk[i].vaddr; i++) { >- pci_pool_free(raid_dev->sg_pool_handle, sg_pci_blk[i].vaddr, >+ dma_pool_free(raid_dev->sg_pool_handle, sg_pci_blk[i].vaddr, > sg_pci_blk[i].dma_addr); > } >- if (raid_dev->sg_pool_handle) >- pci_pool_destroy(raid_dev->sg_pool_handle); >+ dma_pool_
[PATCH] Revert "usb: gadget: uvc: Add missing call for additional setup data"
This reverts commit 4fbac5206afd01b717d4bdc58793d471f3391b4b. This commit breaks g_webcam when used with uvc-gadget [1]. The user space application (e.g. uvc-gadget) is responsible for sending response to UVC class specific requests on control endpoint in uvc_send_response() in uvc_v4l2.c. The bad commit was causing a duplicate response to be sent with incorrect response data thus causing UVC probe to fail at the host and broken control transfer endpoint at the gadget. [1] - git://git.ideasonboard.org/uvc-gadget.git Cc: # v4.9+ Signed-off-by: Roger Quadros --- drivers/usb/gadget/function/f_uvc.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 27ed51b..29b41b5 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -258,13 +258,6 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl) memcpy(&uvc_event->req, ctrl, sizeof(uvc_event->req)); v4l2_event_queue(&uvc->vdev, &v4l2_event); - /* Pass additional setup data to userspace */ - if (uvc->event_setup_out && uvc->event_length) { - uvc->control_req->length = uvc->event_length; - return usb_ep_queue(uvc->func.config->cdev->gadget->ep0, - uvc->control_req, GFP_ATOMIC); - } - return 0; } -- 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] usb: musb: cppi41: don't check early-TX-interrupt for Isoch transfer
On 2017-03-01 20:40:04 [-0600], Bin Liu wrote: > The CPPI41 driver polls register to workaround the pre-mature TX > interrupt issue, but it causes audio playback underrun when triggered in > Isoch transfers. > > Isoch doesn't do back-to-back transfers, the TX should be done by the > time the next transfer is scheduled. So skip this polling workaround for > Isoch transfer. > > Fixes: a655f481d83d6 ("usb: musb: musb_cppi41: handle pre-mature TX complete > interrupt") > Cc: #4.1+ > Reported-by: Alexandre Bailon > Signed-off-by: Bin Liu Looks reasonable. Acked-by: Sebastian Andrzej Siewior Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
FW: RE: Re: FW: RE: Re: Subject: [PATCH v3] USB:Core: BugFix: Proper handling of Race Condition when two USB class drivers try to call init_usb_class simultaneously
> On Wed, 1 Mar 2017, Alan Stern wrote: >> On Wed, 1 Mar 2017, Ajay Kaher wrote: >>> On Mon, 22 Feb 2017, Ajay Kaher wrote: >>> > Alan, as per my understanding I have shifted the lock from > release_usb_class() to destroy_usb_class() in patch v3. > If it is not right, please explain in detail which race condition > I have missed and also share your suggestions. > Have you considered what would happen if destroy_usb_class() ran, but some other CPU was still holding a reference to usb_class? And what if the last reference gets dropped later on, while init_usb_class() is running? >>> >>> Access of usb_class->kref is only from either init_usb_class() >>> or destroy_usb_class(), and both these functions are now protected >>> with Mutex Locking in patch v3, so there is no chance of race condition >>> as per above scenarios. >>> Maybe that's not possible here, but it is possible in general for refcounted objects. So yes, this code is probably okay, but it isn't good form. >>> >>> As per my understanding, I found to be one of the best possible solution >>> for this problem and this solutiuon don't have any side effect. >> >> Alan, I had shared modified Patch v3 as per your inputs to prevent >> the race condition during simultaneously calling of init_usb_class(). >> If you think there is scope to improve the patch, please share your inputs. > > Under the circumstances, your patch is acceptable. > > If you really want to make the point crystal clear, you could replace > usb_class->kref with an ordinary integer counter. Then it would be > obvious that there are no references other than the ones taken by > init_usb_class() and released by destroy_usb_class(). usb_class->kref is not accessible outside the file.c as usb_class is _static_ inside the file.c and pointer of usb_class->kref is not passed anywhere. Hence as you wanted, there are no references of usb_class->kref other than taken by init_usb_class() and released by destroy_usb_class(). thanks, ajay kaher Signed-off-by: Ajay Kaher --- drivers/usb/core/file.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/core/file.c b/drivers/usb/core/file.c index 822ced9..a12d184 100644 --- a/drivers/usb/core/file.c +++ b/drivers/usb/core/file.c @@ -27,6 +27,7 @@ #define MAX_USB_MINORS 256 static const struct file_operations *usb_minors[MAX_USB_MINORS]; static DECLARE_RWSEM(minor_rwsem); +static DEFINE_MUTEX(init_usb_class_mutex); static int usb_open(struct inode *inode, struct file *file) { @@ -109,8 +110,10 @@ static void release_usb_class(struct kref *kref) static void destroy_usb_class(void) { + mutex_lock(&init_usb_class_mutex); if (usb_class) kref_put(&usb_class->kref, release_usb_class); + mutex_unlock(&init_usb_class_mutex); } int usb_major_init(void) @@ -171,7 +174,10 @@ int usb_register_dev(struct usb_interface *intf, if (intf->minor >= 0) return -EADDRINUSE; + mutex_lock(&init_usb_class_mutex); retval = init_usb_class(); + mutex_unlock(&init_usb_class_mutex); + if (retval) return retval; Thanks and Regards, Ajay Kaher