staging: usbip: miscellaneous code cleanup
Subsystems (2)
cover staging: usbip: miscellaneous code cleanup
Hi Greg,
I am interested in hearing your thoughts on the userspace interface?
Even as is, it definitely needs some work. For one thing I was thinking of removing the debug flag from userspace. The userspace utilites are not using it anyway, and it is not 100% working yet.
Thanks,
matt
matt mooney (12):
staging: usbip: change debug configuration option
staging: usbip: use single version for all modules
staging: usbip: fix header includes
staging: usbip: replace usbip_u{dbg,err,info} and printk with pr_
equivalent
staging: usbip: remove unnecessary lines and extra return statements
staging: usbip: stub_main.c: code cleanup
staging: usbip: stub_main.c: change __init/__exit prefix and use
KMEM_CACHE
staging: usbip: usbip_common.c: fix misspelled function name
staging: usbip: edit Kconfig and rename CONFIG options
staging: usbip: stub.h: reorganize
staging: usbip: vhci.h: reorganize
staging: usbip: usbip_common.h: reorganize and document request
headers
drivers/staging/Makefile | 2 +- drivers/staging/usbip/Kconfig | 52 +++--- drivers/staging/usbip/Makefile | 11 +- drivers/staging/usbip/stub.h | 29 ++-- drivers/staging/usbip/stub_dev.c | 30 ++-- drivers/staging/usbip/stub_main.c | 225 ++++++++++++-------------- drivers/staging/usbip/stub_rx.c | 29 ++-- drivers/staging/usbip/stub_tx.c | 31 ++-- drivers/staging/usbip/usbip_common.c | 262 ++++++++++++++---------------- drivers/staging/usbip/usbip_common.h | 302 ++++++++++++++++------------------ drivers/staging/usbip/usbip_event.c | 4 +- drivers/staging/usbip/vhci.h | 16 ++- drivers/staging/usbip/vhci_hcd.c | 107 +++++------- drivers/staging/usbip/vhci_rx.c | 37 ++--- drivers/staging/usbip/vhci_sysfs.c | 19 +- drivers/staging/usbip/vhci_tx.c | 10 +- 16 files changed, 541 insertions(+), 625 deletions(-)
Signature · 1 line
1.7.5.1
PATCH 1/12 staging: usbip: change debug configuration option
Change CONFIG_USB_IP_DEBUG_ENABLE to CONFIG_USB_IP_DEBUG, and move ccflags- to the head of the makefile.
--- drivers/staging/usbip/Kconfig | 4 ++-- drivers/staging/usbip/Makefile | 5 ++--- drivers/staging/usbip/usbip_common.c | 2 +- drivers/staging/usbip/usbip_common.h | 8 ++++---- 4 files changed, 9 insertions(+), 10 deletions(-)
drivers/staging/usbip/Kconfig +2 -2
@@ -35,8 +35,8 @@ config USB_IP_HOST To compile this driver as a module, choose M here: the module will be called usbip. -config USB_IP_DEBUG_ENABLE - bool "USB-IP Debug Enable" +config USB_IP_DEBUG + bool "USB/IP debug messages" depends on USB_IP_COMMON default N ---help---
drivers/staging/usbip/Makefile +2 -3
@@ -1,3 +1,5 @@ +ccflags-$(CONFIG_USB_IP_DEBUG) := -DDEBUG + obj-$(CONFIG_USB_IP_COMMON) += usbip_common_mod.o usbip_common_mod-y := usbip_common.o usbip_event.o
@@ -6,6 +8,3 @@ vhci-hcd-y := vhci_sysfs.o vhci_tx.o vhci_rx.o vhci_hcd.o obj-$(CONFIG_USB_IP_HOST) += usbip.o usbip-y := stub_dev.o stub_main.o stub_rx.o stub_tx.o - -ccflags-$(CONFIG_USB_IP_DEBUG_ENABLE) := -DDEBUG -
drivers/staging/usbip/usbip_common.c +1 -1
@@ -30,7 +30,7 @@#define DRIVER_AUTHOR "Takahiro Hirofuchi <[email protected]>"#define DRIVER_DESC "USB/IP Common Driver" -#ifdef CONFIG_USB_IP_DEBUG_ENABLE +#ifdef CONFIG_USB_IP_DEBUG unsigned long usbip_debug_flag =0xffffffff ;#else unsigned long usbip_debug_flag;
drivers/staging/usbip/usbip_common.h +4 -4
@@ -27,11 +27,11 @@#include <net/sock.h> /** -* usbip_udbg - print debug messages if CONFIG_USB_IP_DEBUG_ENABLE is defined +* usbip_udbg - print debug messages if CONFIG_USB_IP_DEBUG is defined * @fmt: * @args: */ -#ifdef CONFIG_USB_IP_DEBUG_ENABLE +#ifdef CONFIG_USB_IP_DEBUG #define usbip_udbg(fmt, args...) \ do { \
@@ -40,11 +40,11 @@ __FILE__, __LINE__, __func__, ##args); \ } while (0) -#else /* CONFIG_USB_IP_DEBUG_ENABLE */ +#else #define usbip_udbg(fmt, args...) do { } while (0) -#endif /* CONFIG_USB_IP_DEBUG_ENABLE */ +#endif /* CONFIG_USB_IP_DEBUG */ enum { usbip_debug_xmit = (1 <<0 ),
Signature · 1 line
1.7.5.1
PATCH 2/12 staging: usbip: use single version for all modules
In reply to matt mooneyHaving separate versions in this situation makes little sense, so USBIP_VERSION will be used by all modules.
--- drivers/staging/usbip/stub_main.c | 5 ++--- drivers/staging/usbip/usbip_common.c | 5 ++--- drivers/staging/usbip/usbip_common.h | 2 ++ drivers/staging/usbip/vhci_hcd.c | 5 ++--- 4 files changed, 8 insertions(+), 9 deletions(-)
drivers/staging/usbip/stub_main.c +2 -3
@@ -22,7 +22,6 @@#include "usbip_common.h" #include "stub.h" -#define DRIVER_VERSION "1.0"#define DRIVER_AUTHOR "Takahiro Hirofuchi"#define DRIVER_DESC "Stub Driver for USB/IP"
@@ -278,7 +277,7 @@ static int __init usb_stub_init(void) goto error_usb_register; } - printk(KERN_INFO KBUILD_MODNAME ": " DRIVER_DESC " " DRIVER_VERSION + printk(KERN_INFO KBUILD_MODNAME ": " DRIVER_DESC " " USBIP_VERSION "\n");init_busid_table ();
@@ -319,4 +318,4 @@ module_exit(usb_stub_exit);MODULE_AUTHOR (DRIVER_AUTHOR );MODULE_DESCRIPTION (DRIVER_DESC );MODULE_LICENSE ("GPL" ); -MODULE_VERSION (DRIVER_VERSION ); +MODULE_VERSION (USBIP_VERSION );
drivers/staging/usbip/usbip_common.c +2 -3
@@ -26,7 +26,6 @@#include "usbip_common.h" -#define DRIVER_VERSION "1.0"#define DRIVER_AUTHOR "Takahiro Hirofuchi <[email protected]>"#define DRIVER_DESC "USB/IP Common Driver"
@@ -861,7 +860,7 @@ EXPORT_SYMBOL_GPL(usbip_recv_xbuff);static int __init usbip_common_init (void ) { - printk(KERN_INFO KBUILD_MODNAME ": " DRIVER_DESC " " DRIVER_VERSION + printk(KERN_INFO KBUILD_MODNAME ": " DRIVER_DESC " " USBIP_VERSION "\n");return 0 ; }
@@ -877,4 +876,4 @@ module_exit(usbip_common_exit);MODULE_AUTHOR (DRIVER_AUTHOR );MODULE_DESCRIPTION (DRIVER_DESC );MODULE_LICENSE ("GPL" ); -MODULE_VERSION (DRIVER_VERSION ); +MODULE_VERSION (USBIP_VERSION );
drivers/staging/usbip/usbip_common.h +2 -0
@@ -26,6 +26,8 @@#include <asm/byteorder.h> #include <net/sock.h> +#define USBIP_VERSION "1.0.0" +/** * usbip_udbg - print debug messages if CONFIG_USB_IP_DEBUG is defined * @fmt:
drivers/staging/usbip/vhci_hcd.c +2 -3
@@ -23,7 +23,6 @@#include "usbip_common.h" #include "vhci.h" -#define DRIVER_VERSION "1.0"#define DRIVER_AUTHOR "Takahiro Hirofuchi"#define DRIVER_DESC "Virtual Host Controller Interface Driver for USB/IP"
@@ -1231,7 +1230,7 @@ static int __init vhci_init(void)return -ENODEV ; printk(KERN_INFO KBUILD_MODNAME ": %s, %s\n", driver_name, - DRIVER_VERSION); + USBIP_VERSION); ret =platform_driver_register (&vhci_driver);if (ret <0 )
@@ -1268,4 +1267,4 @@ module_exit(vhci_cleanup);MODULE_AUTHOR (DRIVER_AUTHOR );MODULE_DESCRIPTION (DRIVER_DESC );MODULE_LICENSE ("GPL" ); -MODULE_VERSION (DRIVER_VERSION ); +MODULE_VERSION (USBIP_VERSION );
Signature · 1 line
1.7.5.1
PATCH 3/12 staging: usbip: fix header includes
In reply to matt mooneyModify header directives to include what is needed by each file and not already included in its own header.
--- drivers/staging/usbip/stub.h | 9 ++++----- drivers/staging/usbip/stub_dev.c | 2 +- drivers/staging/usbip/stub_main.c | 2 +- drivers/staging/usbip/stub_rx.c | 5 +++-- drivers/staging/usbip/stub_tx.c | 2 +- drivers/staging/usbip/usbip_common.c | 8 ++++---- drivers/staging/usbip/usbip_common.h | 16 ++++++++++------ drivers/staging/usbip/usbip_event.c | 1 + drivers/staging/usbip/vhci.h | 8 +++++++- drivers/staging/usbip/vhci_hcd.c | 6 +++++- drivers/staging/usbip/vhci_rx.c | 2 +- drivers/staging/usbip/vhci_sysfs.c | 5 ++--- drivers/staging/usbip/vhci_tx.c | 2 +- 13 files changed, 41 insertions(+), 27 deletions(-)
drivers/staging/usbip/stub.h +4 -5
@@ -17,13 +17,12 @@* USA. */ -#include <linux/kernel.h> #include <linux/list.h> -#include <linux/spinlock.h> #include <linux/slab.h> -#include <linux/string.h> -#include <linux/module.h> -#include <linux/net.h> +#include <linux/spinlock.h> +#include <linux/types.h> +#include <linux/usb.h> +#include <linux/wait.h> #define STUB_BUSID_OTHER 0#define STUB_BUSID_REMOV 1
drivers/staging/usbip/stub_dev.c +1 -1
@@ -17,7 +17,7 @@* USA. */ -#include <linux/slab.h> +#include <linux/device.h> #include <linux/kthread.h> #include "usbip_common.h"
drivers/staging/usbip/stub_main.c +1 -1
@@ -17,7 +17,7 @@* USA. */ -#include <linux/slab.h> +#include <linux/string.h> #include "usbip_common.h" #include "stub.h"
drivers/staging/usbip/stub_rx.c +3 -2
@@ -17,12 +17,13 @@* USA. */ -#include <linux/slab.h> +#include <asm/byteorder.h> #include <linux/kthread.h> +#include <linux/usb.h> +#include <linux/usb/hcd.h> #include "usbip_common.h" #include "stub.h" -#include <linux/usb/hcd.h> static int is_clear_halt_cmd (struct urb *urb) {
drivers/staging/usbip/stub_tx.c +1 -1
@@ -17,8 +17,8 @@* USA. */ -#include <linux/slab.h> #include <linux/kthread.h> +#include <linux/socket.h> #include "usbip_common.h" #include "stub.h"
drivers/staging/usbip/usbip_common.c +4 -4
@@ -17,12 +17,12 @@* USA. */ -#include <linux/kernel.h> +#include <asm/byteorder.h> #include <linux/file.h> -#include <linux/tcp.h> -#include <linux/in.h> -#include <linux/kthread.h> +#include <linux/fs.h> +#include <linux/kernel.h> #include <linux/slab.h> +#include <net/sock.h> #include "usbip_common.h"
drivers/staging/usbip/usbip_common.h +10 -6
@@ -17,14 +17,18 @@* USA. */ -#ifndef __VHCI_COMMON_H -#define __VHCI_COMMON_H +#ifndef __USBIP_COMMON_H +#define __USBIP_COMMON_H -#include <linux/version.h> #include <linux/compiler.h> +#include <linux/device.h> +#include <linux/interrupt.h> +#include <linux/net.h> +#include <linux/printk.h> +#include <linux/spinlock.h> +#include <linux/types.h> #include <linux/usb.h> -#include <asm/byteorder.h> -#include <net/sock.h> +#include <linux/wait.h> #define USBIP_VERSION "1.0.0"
@@ -366,4 +370,4 @@ void usbip_stop_eh(struct usbip_device *ud);void usbip_event_add (struct usbip_device *ud,unsigned long event);int usbip_event_happened (struct usbip_device *ud); -#endif +#endif /* __USBIP_COMMON_H */
drivers/staging/usbip/usbip_event.c +1 -0
@@ -18,6 +18,7 @@*/ #include <linux/kthread.h> +#include "usbip_common.h" static int event_handler (struct usbip_device *ud)
drivers/staging/usbip/vhci.h +7 -1
@@ -17,8 +17,14 @@* USA. */ -#include <linux/platform_device.h> +#include <linux/device.h> +#include <linux/list.h> +#include <linux/spinlock.h> +#include <linux/sysfs.h> +#include <linux/types.h> +#include <linux/usb.h> #include <linux/usb/hcd.h> +#include <linux/wait.h> struct vhci_device {struct usb_device *udev;
drivers/staging/usbip/vhci_hcd.c +5 -1
@@ -17,8 +17,12 @@* USA. */ -#include <linux/slab.h> +#include <linux/init.h> +#include <linux/kernel.h> #include <linux/kthread.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/slab.h> #include "usbip_common.h" #include "vhci.h"
drivers/staging/usbip/vhci_rx.c +1 -1
@@ -17,8 +17,8 @@* USA. */ -#include <linux/slab.h> #include <linux/kthread.h> +#include <linux/slab.h> #include "usbip_common.h" #include "vhci.h"
drivers/staging/usbip/vhci_sysfs.c +2 -3
@@ -17,12 +17,11 @@* USA. */ +#include <linux/kthread.h> +#include "usbip_common.h" #include "vhci.h" -#include <linux/in.h> -#include <linux/kthread.h> -/* TODO: refine locking ?*/ /* Sysfs entry to show port status */
drivers/staging/usbip/vhci_tx.c +1 -1
@@ -17,8 +17,8 @@* USA. */ -#include <linux/slab.h> #include <linux/kthread.h> +#include <linux/slab.h> #include "usbip_common.h" #include "vhci.h"
Signature · 1 line
1.7.5.1
Re: [PATCH 03/12] staging: usbip: fix header includes
Quoted 2 lines · matt mooney
Modify header directives to include what is needed by each file and not already included in its own header.
This patch didn't apply properly:
patching file drivers/staging/usbip/stub.h
patching file drivers/staging/usbip/stub_dev.c
patching file drivers/staging/usbip/stub_main.c
patching file drivers/staging/usbip/stub_rx.c
patching file drivers/staging/usbip/stub_tx.c
patching file drivers/staging/usbip/usbip_common.c
patching file drivers/staging/usbip/usbip_common.h
patching file drivers/staging/usbip/usbip_event.c
patching file drivers/staging/usbip/vhci.h
patching file drivers/staging/usbip/vhci_hcd.c
patching file drivers/staging/usbip/vhci_rx.c
patching file drivers/staging/usbip/vhci_sysfs.c
Hunk #1 FAILED at 17.
1 out of 1 hunk FAILED -- saving rejects to file drivers/staging/usbip/vhci_sysfs.c.rej patching file drivers/staging/usbip/vhci_tx.c
What tree did you make it against?
thanks,
greg k-h
Re: [PATCH 03/12] staging: usbip: fix header includes
Quoted 21 lines · Greg KH
Quoted 2 lines · matt mooney
Modify header directives to include what is needed by each file and not already included in its own header.
This patch didn't apply properly:
patching filedrivers/staging/usbip/stub.h
patching filedrivers/staging/usbip/stub_dev.c
patching filedrivers/staging/usbip/stub_main.c
patching filedrivers/staging/usbip/stub_rx.c
patching filedrivers/staging/usbip/stub_tx.c
patching filedrivers/staging/usbip/usbip_common.c
patching filedrivers/staging/usbip/usbip_common.h
patching filedrivers/staging/usbip/usbip_event.c
patching filedrivers/staging/usbip/vhci.h
patching filedrivers/staging/usbip/vhci_hcd.c
patching filedrivers/staging/usbip/vhci_rx.c
patching filedrivers/staging/usbip/vhci_sysfs.c
Hunk #1 FAILED at 17.
1 out of 1 hunk FAILED -- saving rejects to filedrivers/staging/usbip/vhci_sysfs.c.rejpatching filedrivers/staging/usbip/vhci_tx.c
Quoted 1 line
What tree did you make it against?
Hmm, this was all done on linux-next-20110510. I will rebase it onto your staging-next tree and resend it.
Signature · 5 lines
GPG-Key: 9AFE00EA -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 4/12 staging: usbip: replace usbip_u{dbg,err,info} and printk with pr_ equivalent
In reply to matt mooneyThis switches all of the non dev_<level> print statements to use the pr_<level> macros. And a few debug statements are removed.
--- drivers/staging/usbip/stub_dev.c | 22 ++-- drivers/staging/usbip/stub_main.c | 19 ++-- drivers/staging/usbip/stub_rx.c | 24 ++-- drivers/staging/usbip/stub_tx.c | 16 ++-- drivers/staging/usbip/usbip_common.c | 228 +++++++++++++++------------------ drivers/staging/usbip/usbip_common.h | 46 +------ drivers/staging/usbip/usbip_event.c | 3 +- drivers/staging/usbip/vhci_hcd.c | 87 +++++-------- drivers/staging/usbip/vhci_rx.c | 33 +++--- drivers/staging/usbip/vhci_sysfs.c | 14 +- drivers/staging/usbip/vhci_tx.c | 8 +- 11 files changed, 209 insertions(+), 291 deletions(-)
drivers/staging/usbip/stub_dev.c +10 -12
@@ -196,7 +196,7 @@ static void stub_shutdown_connection(struct usbip_device *ud)* step 1? */ if (ud->tcp_socket) { -usbip_udbg ("shutdown tcp_socket %p\n" , ud->tcp_socket); +pr_debug ("shutdown tcp_socket %p\n" , ud->tcp_socket);kernel_sock_shutdown (ud->tcp_socket,SHUT_RDWR ); }
@@ -244,7 +244,7 @@ static void stub_device_reset(struct usbip_device *ud)struct usb_device *udev = sdev->udev;int ret; -usbip_udbg ("device reset" ); +pr_debug ("device reset" ); ret =usb_lock_device_for_reset (udev, sdev->interface);if (ret <0 ) {
@@ -336,7 +336,7 @@ static struct stub_device *stub_device_alloc(struct usb_device *udev,usbip_start_eh (&sdev->ud); -usbip_udbg ("register new interface\n" ); +pr_debug ("register new interface\n" );return sdev; }
@@ -346,7 +346,7 @@ static int stub_device_free(struct stub_device *sdev)return -EINVAL ;kfree (sdev); -usbip_udbg ("kfree udev ok\n" ); +pr_debug ("kfree udev ok\n" );return 0 ; }
@@ -389,14 +389,14 @@ static int stub_probe(struct usb_interface *interface, }if (udev->descriptor.bDeviceClass =USB_CLASS_HUB ) { -usbip_udbg ("this device %s is a usb hub device. skip!\n" , - udev_busid); +pr_debug ("this device %s is a usb hub device. skip!\n" , + udev_busid);return -ENODEV ; }if (!strcmp (udev->bus->bus_name,"vhci_hcd" )) { -usbip_udbg ("this device %s is attached on vhci_hcd. skip!\n" , - udev_busid); +pr_debug ("this device %s is attached on vhci_hcd. skip!\n" , + udev_busid);return -ENODEV ; }
@@ -486,10 +486,9 @@ static void stub_disconnect(struct usb_interface *interface)const char *udev_busid =dev_name (interface->dev.parent);struct bus_id_priv *busid_priv; - busid_priv =get_busid_priv (udev_busid); - -usbip_udbg ("Enter\n" ); +pr_debug ("Enter\n" ); + busid_priv =get_busid_priv (udev_busid);if (!busid_priv) {BUG ();return ;
@@ -543,5 +542,4 @@ static void stub_disconnect(struct usb_interface *interface) busid_priv->status =STUB_BUSID_OTHER ;del_match_busid ((char *)udev_busid); } -usbip_udbg ("bye\n" ); }
drivers/staging/usbip/stub_main.c +8 -11
@@ -178,14 +178,14 @@ static ssize_t store_match_busid(struct device_driver *dev, const char *buf,if (add_match_busid (busid) <0 )return -ENOMEM ;else { -usbip_udbg ("add busid %s\n" , busid); +pr_debug ("add busid %s\n" , busid);return count; } }else if (!strncmp (buf,"del " ,4 )) {if (del_match_busid (busid) <0 )return -ENODEV ;else { -usbip_udbg ("del busid %s\n" , busid); +pr_debug ("del busid %s\n" , busid);return count; } }else
@@ -239,12 +239,12 @@ void stub_device_cleanup_urbs(struct stub_device *sdev) {struct stub_priv *priv; -usbip_udbg ("free sdev %p\n" , sdev); +pr_debug ("free sdev %p\n" , sdev);while ((priv =stub_priv_pop (sdev))) {struct urb *urb = priv->urb; -usbip_udbg (" free urb %p\n" , urb); +pr_debug (" free urb %p\n" , urb);usb_kill_urb (urb);kmem_cache_free (stub_priv_cache, priv);
@@ -265,20 +265,17 @@ static int __init usb_stub_init(void) SLAB_HWCACHE_ALIGN, NULL);if (!stub_priv_cache) { - printk(KERN_ERR KBUILD_MODNAME - ": create stub_priv_cache error\n"); +pr_err ("create stub_priv_cache error\n" );return -ENOMEM ; } ret =usb_register (&stub_driver);if (ret) { - printk(KERN_ERR KBUILD_MODNAME ": usb_register failed %d\n", - ret); +pr_err ("usb_register failed %d\n" , ret); goto error_usb_register; } - printk(KERN_INFO KBUILD_MODNAME ": " DRIVER_DESC " " USBIP_VERSION - "\n"); +pr_info (DRIVER_DESC " " USBIP_VERSION "\n" );init_busid_table ();
@@ -286,7 +283,7 @@ static int __init usb_stub_init(void) &driver_attr_match_busid);if (ret) { - printk(KERN_ERR KBUILD_MODNAME ": create driver sysfs\n"); +pr_err ("create driver sysfs\n" ); goto error_create_file; }
drivers/staging/usbip/stub_rx.c +12 -12
@@ -102,11 +102,11 @@ static int tweak_clear_halt_cmd(struct urb *urb) ret =usb_clear_halt (urb->dev, target_pipe);if (ret <0 ) -usbip_uinfo ("clear_halt error: devnum %d endp %d, %d\n" , - urb->dev->devnum, target_endp, ret); +pr_err ("clear_halt error: devnum %d endp %d, %d\n" , + urb->dev->devnum, target_endp, ret);else -usbip_uinfo ("clear_halt done: devnum %d endp %d\n" , - urb->dev->devnum, target_endp); +pr_info ("clear_halt done: devnum %d endp %d\n" , + urb->dev->devnum, target_endp);return ret; }
@@ -127,11 +127,11 @@ static int tweak_set_interface_cmd(struct urb *urb) ret =usb_set_interface (urb->dev, interface, alternate);if (ret <0 ) -usbip_uinfo ("set_interface error: inf %u alt %u, %d\n" , - interface, alternate, ret); +pr_err ("set_interface error: inf %u alt %u, %d\n" , + interface, alternate, ret);else -usbip_uinfo ("set_interface done: inf %u alt %u\n" , - interface, alternate); +pr_info ("set_interface done: inf %u alt %u\n" , + interface, alternate);return ret; }
@@ -160,9 +160,9 @@ static int tweak_set_configuration_cmd(struct urb *urb)* A user may need to set a special configuration value before * exporting the device. */ -usbip_uinfo ("set_configuration %d to %s\n" , - config,dev_name (&urb->dev->dev)); -usbip_uinfo ("but, skip!\n" ); +pr_info ("set_configuration %d to %s\n" , + config,dev_name (&urb->dev->dev)); +pr_info ("but, skip!\n" );return 0 ;/* return usb_driver_set_configuration(urb->dev, config); */
@@ -173,7 +173,7 @@ static int tweak_reset_device_cmd(struct urb *urb)struct stub_priv *priv = (struct stub_priv *) urb->context;struct stub_device *sdev = priv->sdev; -usbip_uinfo ("reset_device %s\n" ,dev_name (&urb->dev->dev)); +pr_info ("reset_device %s\n" ,dev_name (&urb->dev->dev));/* * usb_lock_device_for_reset caused a deadlock: it causes the driver
drivers/staging/usbip/stub_tx.c +8 -8
@@ -75,22 +75,22 @@ void stub_complete(struct urb *urb)/* OK */ break ;case -ENOENT : -usbip_uinfo ("stopped by a call of usb_kill_urb() because of" -"cleaning up a virtual connection\n" ); +pr_info ("stopped by a call of usb_kill_urb() because of" +"cleaning up a virtual connection\n" );return ;case -ECONNRESET : -usbip_uinfo ("unlinked by a call of usb_unlink_urb()\n" ); +pr_info ("unlinked by a call of usb_unlink_urb()\n" );break ;case -EPIPE : -usbip_uinfo ("endpoint %d is stalled\n" , -usb_pipeendpoint (urb->pipe)); +pr_info ("endpoint %d is stalled\n" , +usb_pipeendpoint (urb->pipe));break ;case -ESHUTDOWN : -usbip_uinfo ("device removed?\n" ); +pr_info ("device removed?\n" );break ;default : -usbip_uinfo ("urb completion with non-zero status %d\n" , - urb->status); +pr_info ("urb completion with non-zero status %d\n" , + urb->status);break ; }
drivers/staging/usbip/usbip_common.c +104 -124
430 lines not on this page · raw message
drivers/staging/usbip/usbip_common.h +6 -40
66 lines not on this page · raw message
drivers/staging/usbip/usbip_event.c +1 -2
@@ -84,8 +84,7 @@ int usbip_start_eh(struct usbip_device *ud) ud->eh =kthread_run (event_handler_loop, ud,"usbip_eh" );if (IS_ERR (ud->eh)) { -printk (KERN_WARNING -"Unable to start control thread\n" ); +pr_warning ("Unable to start control thread\n" );return PTR_ERR (ud->eh); }
drivers/staging/usbip/vhci_hcd.c +6 -9
@@ -96,12 +96,12 @@ static void dump_port_status(u32 status) {int i =0 ; -printk (KERN_DEBUG "status %08x:" , status); +pr_debug ("status %08x:" , status);for (i =0 ; i <32 ; i++) {if (status & (1 << i)) -printk (KERN_DEBUG " %s" , bit_desc[i]); +pr_debug (" %s" , bit_desc[i]); } -printk (KERN_DEBUG "\n" ); +pr_debug ("\n" ); }void rh_port_connect (int rhport,enum usb_device_speed speed)
@@ -216,7 +216,7 @@ static int vhci_hub_status(struct usb_hcd *hcd, char *buf) } } -usbip_uinfo ("changed %d\n" , changed); +pr_info ("changed %d\n" , changed);if (hcd->state =HC_STATE_SUSPENDED )usb_hcd_resume_root_hub (hcd);
@@ -264,8 +264,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,usbip_dbg_vhci_rh ("typeReq %x wValue %x wIndex %x\n" , typeReq, wValue, wIndex);if (wIndex >VHCI_NPORTS ) -printk (KERN_ERR "%s: invalid port number %d\n" , __func__, - wIndex); +pr_err ("invalid port number %d\n" , wIndex); rhport = ((__u8)(wIndex &0x00ff )) -1 ; dum =hcd_to_vhci (hcd);
@@ -334,8 +333,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,case GetPortStatus:usbip_dbg_vhci_rh (" GetPortStatus port %x\n" , wIndex);if (wIndex >VHCI_NPORTS || wIndex <1 ) { -printk (KERN_ERR "%s: invalid port number %d\n" , - __func__, wIndex); +pr_err ("invalid port number %d\n" , wIndex); retval = -EPIPE ; }
@@ -345,7 +343,6 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,* complete it!! * */ if (dum->resuming &&time_after (jiffies, dum->re_timeout)) { -printk (KERN_ERR "%s: not yet\n" , __func__); dum->port_status[rhport] | (1 <<USB_PORT_FEAT_C_SUSPEND ); dum->port_status[rhport] &@@ -415,7 +412,6 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, case USB_PORT_FEAT_SUSPEND:
usbip_dbg_vhci_rh(" SetPortFeature: "
"USB_PORT_FEAT_SUSPEND\n");
- printk(KERN_ERR "%s: not yet\n", __func__);
#if 0
dum->port_status[rhport] | (1 << USB_PORT_FEAT_SUSPEND);
@@ -457,7 +453,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
break;
default:
- printk(KERN_ERR "%s: default: no such request\n", __func__);
+ pr_err("default: no such request\n");
/* dev_dbg (hardware,
* "hub control req%04x v%04x i%04x l%d\n",
* typeReq, wValue, wIndex, wLength); */
@@ -467,7 +463,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
}
if (usbip_dbg_flag_vhci_rh) {
- printk(KERN_DEBUG "port %d\n", rhport);
+ pr_debug("port %d\n", rhport);
dump_port_status(prev_port_status[rhport]); dump_port_status(dum->port_status[rhport]);
}
@@ -499,7 +495,7 @@ static void vhci_tx_urb(struct urb *urb)
unsigned long flag;
if (!vdev) {
- err("could not get virtual device");
+ pr_err("could not get virtual device");
/* BUG(); */
return;
}
@@ -517,7 +513,7 @@ static void vhci_tx_urb(struct urb *urb)
priv->seqnum = atomic_inc_return(&the_controller->seqnum); if (priv->seqnum = 0xffff)
- usbip_uinfo("seqnum max\n");
+ pr_info("seqnum max\n");
priv->vdev = vdev;
priv->urb = urb;
@@ -558,7 +554,7 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
spin_lock(&vdev->ud.lock);
if (vdev->ud.status = VDEV_ST_NULL ||
vdev->ud.status = VDEV_ST_ERROR) {
- usbip_uerr("enqueue for inactive port %d\n", vdev->rhport);
+ pr_err("enqueue for inactive port %d\n", vdev->rhport);
spin_unlock(&vdev->ud.lock); spin_unlock_irqrestore(&the_controller->lock, flags);
return -ENODEV;
@@ -701,7 +697,7 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
struct vhci_priv *priv;
struct vhci_device *vdev;
- usbip_uinfo("vhci_hcd: dequeue a urb %p\n", urb);
+ pr_info("dequeue a urb %p\n", urb);
spin_lock_irqsave(&the_controller->lock, flags);
@@ -731,8 +727,7 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
spin_lock_irqsave(&vdev->priv_lock, flags2);
- usbip_uinfo("vhci_hcd: device %p seems to be disconnected\n",
- vdev);
+ pr_info("device %p seems to be disconnected\n", vdev);
list_del(&priv->list);
kfree(priv);
urb->hcpriv = NULL;
@@ -744,8 +739,7 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
* vhci_rx will receive RET_UNLINK and give back the URB.
* Otherwise, we give back it here.
*/
- usbip_uinfo("vhci_hcd: vhci_urb_dequeue() gives back urb %p\n",
- urb);
+ pr_info("gives back urb %p\n", urb);
usb_hcd_unlink_urb_from_ep(hcd, urb);
@@ -764,7 +758,7 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
/* setup CMD_UNLINK pdu */
unlink = kzalloc(sizeof(struct vhci_unlink), GFP_ATOMIC);
if (!unlink) {
- usbip_uerr("malloc vhci_unlink\n");
+ pr_err("malloc vhci_unlink\n");
spin_unlock_irqrestore(&vdev->priv_lock, flags2); spin_unlock_irqrestore(&the_controller->lock, flags); usbip_event_add(&vdev->ud, VDEV_EVENT_ERROR_MALLOC);
@@ -773,12 +767,11 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
unlink->seqnum = atomic_inc_return(&the_controller->seqnum); if (unlink->seqnum = 0xffff)
- usbip_uinfo("seqnum max\n");
+ pr_info("seqnum max\n");
unlink->unlink_seqnum = priv->seqnum;
- usbip_uinfo("vhci_hcd: device %p seems to be still connected\n",
- vdev);
+ pr_info("device %p seems to be still connected\n", vdev);
/* send cmd_unlink and try to cancel the pending URB in the
* peer */
@@ -801,7 +794,7 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
spin_lock(&vdev->priv_lock);
list_for_each_entry_safe(unlink, tmp, &vdev->unlink_tx, list) {
- usbip_uinfo("unlink cleanup tx %lu\n", unlink->unlink_seqnum);
+ pr_info("unlink cleanup tx %lu\n", unlink->unlink_seqnum);
list_del(&unlink->list);
kfree(unlink);
}
@@ -810,12 +803,12 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
struct urb *urb;
/* give back URB of unanswered unlink request */
- usbip_uinfo("unlink cleanup rx %lu\n", unlink->unlink_seqnum);
+ pr_info("unlink cleanup rx %lu\n", unlink->unlink_seqnum);
urb = pickup_urb_and_free_priv(vdev, unlink->unlink_seqnum); if (!urb) {
- usbip_uinfo("the urb (seqnum %lu) was already given "
- "back\n", unlink->unlink_seqnum);
+ pr_info("the urb (seqnum %lu) was already given back\n",
+ unlink->unlink_seqnum);
list_del(&unlink->list);
kfree(unlink);
continue;
@@ -848,7 +841,7 @@ static void vhci_shutdown_connection(struct usbip_device *ud)
/* need this? see stub_dev.c */
if (ud->tcp_socket) {
- usbip_udbg("shutdown tcp_socket %p\n", ud->tcp_socket);
+ pr_debug("shutdown tcp_socket %p\n", ud->tcp_socket);
kernel_sock_shutdown(ud->tcp_socket, SHUT_RDWR);
}
@@ -858,14 +851,14 @@ static void vhci_shutdown_connection(struct usbip_device *ud)
if (vdev->ud.tcp_tx)
kthread_stop(vdev->ud.tcp_tx);
- usbip_uinfo("stop threads\n");
+ pr_info("stop threads\n");
/* active connection is closed */ if (vdev->ud.tcp_socket != NULL) {
sock_release(vdev->ud.tcp_socket); vdev->ud.tcp_socket = NULL;
}
- usbip_uinfo("release socket\n");
+ pr_info("release socket\n");
vhci_device_unlink_cleanup(vdev);
@@ -891,7 +884,7 @@ static void vhci_shutdown_connection(struct usbip_device *ud)
*/
rh_port_disconnect(vdev->rhport);
- usbip_uinfo("disconnect device\n");
+ pr_info("disconnect device\n");
}
@@ -972,7 +965,7 @@ static int vhci_start(struct usb_hcd *hcd)
/* vhci_hcd is now ready to be controlled through sysfs */ err = sysfs_create_group(&vhci_dev(vhci)->kobj, &dev_attr_group); if (err) {
- usbip_uerr("create sysfs files\n");
+ pr_err("create sysfs files\n");
return err;
}
@@ -996,13 +989,11 @@ static void vhci_stop(struct usb_hcd *hcd)
usbip_event_add(&vdev->ud, VDEV_EVENT_REMOVED); usbip_stop_eh(&vdev->ud);
}
-
- usbip_uinfo("vhci_stop done\n");
}
static int vhci_get_frame_number(struct usb_hcd *hcd) {
- usbip_uerr("Not yet implemented\n");
+ pr_err("Not yet implemented\n");
return 0;
}
@@ -1079,8 +1070,6 @@ static int vhci_hcd_probe(struct platform_device *pdev)
struct usb_hcd *hcd;
int ret;
- usbip_uinfo("proving...\n");
-
usbip_dbg_vhci_hc("name %s id %d\n", pdev->name, pdev->id);
/* will be removed */
@@ -1095,7 +1084,7 @@ static int vhci_hcd_probe(struct platform_device *pdev)
*/
hcd = usb_create_hcd(&vhci_hc_driver, &pdev->dev, dev_name(&pdev->dev));
if (!hcd) {
- usbip_uerr("create hcd failed\n");
+ pr_err("create hcd failed\n");
return -ENOMEM;
}
hcd->has_tt = 1;
@@ -1109,7 +1098,7 @@ static int vhci_hcd_probe(struct platform_device *pdev)
*/
ret = usb_add_hcd(hcd, 0, 0);
if (ret != 0) {
- usbip_uerr("usb_add_hcd failed %d\n", ret);
+ pr_err("usb_add_hcd failed %d\n", ret);
usb_put_hcd(hcd);
the_controller = NULL;
return ret;
@@ -1163,11 +1152,11 @@ static int vhci_hcd_suspend(struct platform_device *pdev, pm_message_t state)
spin_unlock(&the_controller->lock);
if (connected > 0) {
- usbip_uinfo("We have %d active connection%s. Do not suspend.\n",
- connected, (connected = 1 ? "" : "s"));
+ pr_info("We have %d active connection%s. Do not suspend.\n",
+ connected, (connected = 1 ? "" : "s"));
ret = -EBUSY;
} else {
- usbip_uinfo("suspend vhci_hcd");
+ pr_info("suspend vhci_hcd");
clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
}
@@ -1229,13 +1218,9 @@ static int __init vhci_init(void)
{
int ret;
- usbip_dbg_vhci_hc("enter\n");
if (usb_disabled())
return -ENODEV;
- printk(KERN_INFO KBUILD_MODNAME ": %s, %s\n", driver_name,
- USBIP_VERSION);
-
ret = platform_driver_register(&vhci_driver); if (ret < 0)
goto err_driver_register;
@@ -1244,25 +1229,19 @@ static int __init vhci_init(void)
if (ret < 0)
goto err_platform_device_register;
- usbip_dbg_vhci_hc("bye\n");
+ pr_info(DRIVER_DESC " v" USBIP_VERSION "\n");
return ret;
- /* error occurred */
err_platform_device_register:
platform_driver_unregister(&vhci_driver);
err_driver_register:
- usbip_dbg_vhci_hc("bye\n");
return ret;
}
static void __exit vhci_cleanup(void)
{
- usbip_dbg_vhci_hc("enter\n");
-
platform_device_unregister(&the_pdev);
platform_driver_unregister(&vhci_driver);
-
- usbip_dbg_vhci_hc("bye\n");
}
module_init(vhci_init);
drivers/staging/usbip/vhci_rx.c +16 -17
@@ -74,10 +74,9 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,spin_unlock (&vdev->priv_lock);if (!urb) { -usbip_uerr ("cannot find a urb of seqnum %u\n" , - pdu->base.seqnum); -usbip_uinfo ("max seqnum %d\n" , -atomic_read (&the_controller->seqnum)); +pr_err ("cannot find a urb of seqnum %u\n" , pdu->base.seqnum); +pr_info ("max seqnum %d\n" , +atomic_read (&the_controller->seqnum));usbip_event_add (ud,VDEV_EVENT_ERROR_TCP );return ; }
@@ -121,7 +120,7 @@ static struct vhci_unlink *dequeue_pending_unlink(struct vhci_device *vdev,spin_lock (&vdev->priv_lock);list_for_each_entry_safe (unlink, tmp, &vdev->unlink_rx, list) { -usbip_uinfo ("unlink->seqnum %lu\n" , unlink->seqnum); +pr_info ("unlink->seqnum %lu\n" , unlink->seqnum);if (unlink->seqnum = pdu->base.seqnum) {usbip_dbg_vhci_rx ("found pending unlink, %lu\n" , unlink->seqnum);
@@ -147,8 +146,8 @@ static void vhci_recv_ret_unlink(struct vhci_device *vdev, unlink =dequeue_pending_unlink (vdev, pdu);if (!unlink) { -usbip_uinfo ("cannot find the pending unlink %u\n" , - pdu->base.seqnum); +pr_info ("cannot find the pending unlink %u\n" , + pdu->base.seqnum);return ; }
@@ -162,14 +161,14 @@ static void vhci_recv_ret_unlink(struct vhci_device *vdev,* already received the result of its submit result and gave * back the URB. */ -usbip_uinfo ("the urb (seqnum %d) was already given backed\n" , - pdu->base.seqnum); +pr_info ("the urb (seqnum %d) was already given backed\n" , + pdu->base.seqnum); }else {usbip_dbg_vhci_rx ("now giveback urb %p\n" , urb);/* If unlink is succeed, status is -ECONNRESET */ urb->status = pdu->u.ret_unlink.status; -usbip_uinfo ("%d\n" , urb->status); +pr_info ("urb->status %d\n" , urb->status);spin_lock (&the_controller->lock);usb_hcd_unlink_urb_from_ep (vhci_to_hcd (the_controller), urb);
@@ -210,26 +209,26 @@ static void vhci_rx_pdu(struct usbip_device *ud) ret =usbip_xmit (0 , ud->tcp_socket, (char *) &pdu,sizeof (pdu),0 );if (ret <0 ) {if (ret = -ECONNRESET ) -usbip_uinfo ("connection reset by peer\n" ); +pr_info ("connection reset by peer\n" );else if (ret = -EAGAIN ) {/* ignore if connection was idle */ if (vhci_priv_tx_empty (vdev))return ; -usbip_uinfo ("connection timed out with pending urbs\n" ); +pr_info ("connection timed out with pending urbs\n" ); }else if (ret != -ERESTARTSYS ) -usbip_uinfo ("xmit failed %d\n" , ret); +pr_info ("xmit failed %d\n" , ret);usbip_event_add (ud,VDEV_EVENT_ERROR_TCP );return ; }if (ret =0 ) { -usbip_uinfo ("connection closed" ); +pr_info ("connection closed" );usbip_event_add (ud,VDEV_EVENT_DOWN );return ; }if (ret !=sizeof (pdu)) { -usbip_uerr ("received pdu size is %d, should be %d\n" , - ret, (unsigned int )sizeof (pdu)); +pr_err ("received pdu size is %d, should be %d\n" , ret, + (unsigned int )sizeof (pdu));usbip_event_add (ud,VDEV_EVENT_ERROR_TCP );return ; }
@@ -248,7 +247,7 @@ static void vhci_rx_pdu(struct usbip_device *ud)break ;default :/* NOT REACHED */ -usbip_uerr ("unknown pdu %u\n" , pdu.base.command); +pr_err ("unknown pdu %u\n" , pdu.base.command);usbip_dump_header (&pdu);usbip_event_add (ud,VDEV_EVENT_ERROR_TCP );break ;
drivers/staging/usbip/vhci_sysfs.c +7 -7
@@ -88,7 +88,7 @@ static int vhci_port_disconnect(__u32 rhport)spin_lock (&vdev->ud.lock);if (vdev->ud.status =VDEV_ST_NULL ) { -usbip_uerr ("not connected %d\n" , vdev->ud.status); +pr_err ("not connected %d\n" , vdev->ud.status);/* unlock */ spin_unlock (&vdev->ud.lock);
@@ -116,7 +116,7 @@ static ssize_t store_detach(struct device *dev, struct device_attribute *attr,/* check rhport */ if (rhport >=VHCI_NPORTS ) { -usbip_uerr ("invalid port %u\n" , rhport); +pr_err ("invalid port %u\n" , rhport);return -EINVAL ; }
@@ -135,7 +135,7 @@ static int valid_args(__u32 rhport, enum usb_device_speed speed) {/* check rhport */ if ((rhport <0 ) || (rhport >=VHCI_NPORTS )) { -usbip_uerr ("port %u\n" , rhport); +pr_err ("port %u\n" , rhport);return -EINVAL ; }
@@ -147,7 +147,7 @@ static int valid_args(__u32 rhport, enum usb_device_speed speed)case USB_SPEED_WIRELESS :break ;default : -usbip_uerr ("speed %d\n" , speed); +pr_err ("speed %d\n" , speed);return -EINVAL ; }
@@ -205,12 +205,12 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,spin_unlock (&vdev->ud.lock);spin_unlock (&the_controller->lock); -usbip_uerr ("port %d already used\n" , rhport); +pr_err ("port %d already used\n" , rhport);return -EINVAL ; } -usbip_uinfo ("rhport(%u) sockfd(%d) devid(%u) speed(%u)\n" , - rhport, sockfd, devid, speed); +pr_info ("rhport(%u) sockfd(%d) devid(%u) speed(%u)\n" , + rhport, sockfd, devid, speed); vdev->devid = devid; vdev->speed = speed;
drivers/staging/usbip/vhci_tx.c +4 -4
@@ -118,8 +118,8 @@ static int vhci_send_cmd_submit(struct vhci_device *vdev) ret =kernel_sendmsg (vdev->ud.tcp_socket, &msg, iov,3 , txsize);if (ret != txsize) { -usbip_uerr ("sendmsg failed!, retval %d for %zd\n" , ret, - txsize); +pr_err ("sendmsg failed!, ret=%d for %zd\n" , ret, + txsize);kfree (iso_buffer);usbip_event_add (&vdev->ud,VDEV_EVENT_ERROR_TCP );return -1 ;
@@ -188,8 +188,8 @@ static int vhci_send_cmd_unlink(struct vhci_device *vdev) ret =kernel_sendmsg (vdev->ud.tcp_socket, &msg, iov,1 , txsize);if (ret != txsize) { -usbip_uerr ("sendmsg failed!, retval %d for %zd\n" , ret, - txsize); +pr_err ("sendmsg failed!, ret=%d for %zd\n" , ret, + txsize);usbip_event_add (&vdev->ud,VDEV_EVENT_ERROR_TCP );return -1 ; }
Signature · 1 line
1.7.5.1
Re: [PATCH 04/12] staging: usbip: replace usbip_u{dbg,err,info} and printk with pr_ equivalent
Quoted 2 lines · matt mooney
This switches all of the non dev_<level> print statements to use the pr_<level> macros. And a few debug statements are removed.
Can't most of these be moved to use the dev_* calls instead?
Quoted 21 lines
Signed-off-by matt mooney <[email protected]>--- drivers/staging/usbip/stub_dev.c | 22 ++-- drivers/staging/usbip/stub_main.c | 19 ++-- drivers/staging/usbip/stub_rx.c | 24 ++-- drivers/staging/usbip/stub_tx.c | 16 ++-- drivers/staging/usbip/usbip_common.c | 228 +++++++++++++++------------------ drivers/staging/usbip/usbip_common.h | 46 +------ drivers/staging/usbip/usbip_event.c | 3 +- drivers/staging/usbip/vhci_hcd.c | 87 +++++-------- drivers/staging/usbip/vhci_rx.c | 33 +++--- drivers/staging/usbip/vhci_sysfs.c | 14 +- drivers/staging/usbip/vhci_tx.c | 8 +- 11 files changed, 209 insertions(+), 291 deletions(-)drivers/staging/usbip/stub_dev.c +0
-0
drivers/staging/usbip/stub_dev.c static void stub_shutdown_connection(struct usbip_device *ud) · line 199
Quoted 5 lines
drivers/staging/usbip/stub_dev.c +1
-1@@ -196,7 +196,7 @@ static void stub_shutdown_connection(struct usbip_device *ud)* step 1? */ if (ud->tcp_socket) { -usbip_udbg ("shutdown tcp_socket %p\n" , ud->tcp_socket); +pr_debug ("shutdown tcp_socket %p\n" , ud->tcp_socket);
Like here, we have a valid struct device pointer from what I can tell, so we should use dev_dbg() instead of pr_debug().
I think a lot of these conversions could use those calls here, right?
So care to split this into 2 patches, one that converts to valid usages of the dev_* calls, and the other a patch that converts the rest to pr_* calls?
thanks,
greg k-h
Re: [PATCH 04/12] staging: usbip: replace usbip_u{dbg,err,info} and printk with pr_ equivalent
Quoted 5 lines · Greg KH
Quoted 2 lines · matt mooney
This switches all of the non dev_<level> print statements to use the pr_<level> macros. And a few debug statements are removed.
Can't most of these be moved to use the dev_* calls instead?
I believe so and had planned to do this is in a second pass.
Quoted 30 lines
Quoted 21 lines
Signed-off-by matt mooney <[email protected]>---
drivers/staging/usbip/stub_dev.c | 22 ++--drivers/staging/usbip/stub_main.c| 19 ++--drivers/staging/usbip/stub_rx.c| 24 ++--drivers/staging/usbip/stub_tx.c| 16 ++--drivers/staging/usbip/usbip_common.c| 228 +++++++++++++++------------------drivers/staging/usbip/usbip_common.h| 46 +------drivers/staging/usbip/usbip_event.c| 3 +-drivers/staging/usbip/vhci_hcd.c| 87 +++++--------drivers/staging/usbip/vhci_rx.c| 33 +++---drivers/staging/usbip/vhci_sysfs.c| 14 +-drivers/staging/usbip/vhci_tx.c| 8 +-11 files changed, 209 insertions(+), 291 deletions(-)drivers/staging/usbip/stub_dev.c +0
-0drivers/staging/usbip/stub_dev.c static void stub_shutdown_connection(struct usbip_device *ud) · line 199
Quoted 5 lines
drivers/staging/usbip/stub_dev.c +1
-1@@ -196,7 +196,7 @@ static void stub_shutdown_connection(struct usbip_device *ud)* step 1? */ if (ud->tcp_socket) { - usbip_udbg("shutdown tcp_socket %p\n", ud->tcp_socket); + pr_debug("shutdown tcp_socket %p\n", ud->tcp_socket);Like here, we have a valid struct device pointer from what I can tell, so we should use
dev_dbg()instead ofpr_debug().
Quoted 5 lines
I think a lot of these conversions could use those calls here, right?
So care to split this into 2 patches, one that converts to valid usages of the dev_* calls, and the other a patch that converts the rest to pr_* calls?
Do you mind just taking this as is for now? That way I can do the dev_dbg() conversion on a case by case basis as I become more familiar with the code, which was kind of my original plan. This patch still provides a fair amount of cleanup ;)
Signature · 5 lines
GPG-Key: 9AFE00EA -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/12] staging: usbip: replace usbip_u{dbg,err,info} and printk with pr_ equivalent
Quoted 8 lines · matt mooney
Quoted 5 lines · Greg KH
Quoted 2 lines · matt mooney
This switches all of the non dev_<level> print statements to use the pr_<level> macros. And a few debug statements are removed.
Can't most of these be moved to use the dev_* calls instead?
I believe so and had planned to do this is in a second pass.
Ah, you didn't say that :)
Quoted 31 lines
Quoted 31 lines
Quoted 20 lines
Signed-off-by matt mooney <[email protected]>---
drivers/staging/usbip/stub_dev.c | 22 ++--drivers/staging/usbip/stub_main.c| 19 ++--drivers/staging/usbip/stub_rx.c| 24 ++--drivers/staging/usbip/stub_tx.c| 16 ++--drivers/staging/usbip/usbip_common.c| 228 +++++++++++++++------------------drivers/staging/usbip/usbip_common.h| 46 +------drivers/staging/usbip/usbip_event.c| 3 +-drivers/staging/usbip/vhci_hcd.c| 87 +++++--------drivers/staging/usbip/vhci_rx.c| 33 +++---drivers/staging/usbip/vhci_sysfs.c| 14 +-drivers/staging/usbip/vhci_tx.c| 8 +-11 files changed, 209 insertions(+), 291 deletions(-)drivers/staging/usbip/stub_dev.c +0
-0drivers/staging/usbip/stub_dev.c static void stub_shutdown_connection(struct usbip_device *ud) · line 199
Quoted 5 lines
drivers/staging/usbip/stub_dev.c +1
-1@@ -196,7 +196,7 @@ static void stub_shutdown_connection(struct usbip_device *ud)* step 1? */ if (ud->tcp_socket) { - usbip_udbg("shutdown tcp_socket %p\n", ud->tcp_socket); + pr_debug("shutdown tcp_socket %p\n", ud->tcp_socket);Like here, we have a valid struct device pointer from what I can tell, so we should use
dev_dbg()instead ofpr_debug().I think a lot of these conversions could use those calls here, right?
Quoted 8 lines
Quoted 3 lines
So care to split this into 2 patches, one that converts to valid usages of the dev_* calls, and the other a patch that converts the rest to pr_* calls?
Do you mind just taking this as is for now? That way I can do the
dev_dbg()conversion on a case by case basis as I become more familiar with the code, which was kind of my original plan. This patch still provides a fair amount of cleanup ;)
Yes it does, but I would prefer if you did it right the first time. It's neater that way and makes more sense.
So, can you please redo this and resend it?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 5/12 staging: usbip: remove unnecessary lines and extra return statements
In reply to matt mooneyAlso, fix a few alignment issues that were originally missed.
--- drivers/staging/usbip/stub.h | 1 + drivers/staging/usbip/stub_dev.c | 6 ------ drivers/staging/usbip/stub_main.c | 13 ------------- drivers/staging/usbip/stub_tx.c | 13 ++++++------- drivers/staging/usbip/usbip_common.c | 13 ++++++------- drivers/staging/usbip/usbip_common.h | 2 +- drivers/staging/usbip/vhci_hcd.c | 9 +++------ drivers/staging/usbip/vhci_rx.c | 2 -- 8 files changed, 17 insertions(+), 42 deletions(-)
drivers/staging/usbip/stub.h +1 -0
@@ -77,6 +77,7 @@ struct stub_unlink { };#define BUSID_SIZE 20 +struct bus_id_priv {char name[BUSID_SIZE ];char status;
drivers/staging/usbip/stub_dev.c +0 -6
@@ -249,7 +249,6 @@ static void stub_device_reset(struct usbip_device *ud) ret =usb_lock_device_for_reset (udev, sdev->interface);if (ret <0 ) {dev_err (&udev->dev,"lock for reset\n" ); -spin_lock (&ud->lock); ud->status =SDEV_ST_ERROR ;spin_unlock (&ud->lock);
@@ -258,22 +257,17 @@ static void stub_device_reset(struct usbip_device *ud)/* try to reset the device */ ret =usb_reset_device (udev); -usb_unlock_device (udev);spin_lock (&ud->lock);if (ret) {dev_err (&udev->dev,"device reset\n" ); ud->status =SDEV_ST_ERROR ; - }else {dev_info (&udev->dev,"device reset\n" ); ud->status =SDEV_ST_AVAILABLE ; - }spin_unlock (&ud->lock); - -return ; }static void stub_device_unusable (struct usbip_device *ud)
drivers/staging/usbip/stub_main.c +0 -13
@@ -25,9 +25,7 @@#define DRIVER_AUTHOR "Takahiro Hirofuchi"#define DRIVER_DESC "Stub Driver for USB/IP" -/* stub_priv is allocated from stub_priv_cache */ struct kmem_cache *stub_priv_cache; -/* * busid_tables defines matching busids that usbip can grab. A user can change * dynamically what device is locally used and what device is exported to a
@@ -42,7 +40,6 @@ int match_busid(const char *busid)int i;spin_lock (&busid_table_lock); -for (i =0 ; i <MAX_BUSID ; i++)if (busid_table[i].name[0 ])if (!strncmp (busid_table[i].name, busid,BUSID_SIZE )) {
@@ -50,7 +47,6 @@ int match_busid(const char *busid)spin_unlock (&busid_table_lock);return 0 ; } -spin_unlock (&busid_table_lock);return 1 ;
@@ -61,7 +57,6 @@ struct bus_id_priv *get_busid_priv(const char *busid)int i;spin_lock (&busid_table_lock); -for (i =0 ; i <MAX_BUSID ; i++)if (busid_table[i].name[0 ])if (!strncmp (busid_table[i].name, busid,BUSID_SIZE )) {
@@ -69,7 +64,6 @@ struct bus_id_priv *get_busid_priv(const char *busid)spin_unlock (&busid_table_lock);return &(busid_table[i]); } -spin_unlock (&busid_table_lock);return NULL ;
@@ -81,15 +75,12 @@ static ssize_t show_match_busid(struct device_driver *drv, char *buf)char *out = buf;spin_lock (&busid_table_lock); -for (i =0 ; i <MAX_BUSID ; i++)if (busid_table[i].name[0 ]) out +=sprintf (out,"%s " , busid_table[i].name); -spin_unlock (&busid_table_lock); out +=sprintf (out,"\n" ); -return out - buf; }
@@ -101,7 +92,6 @@ static int add_match_busid(char *busid)return 0 ;spin_lock (&busid_table_lock); -for (i =0 ; i <MAX_BUSID ; i++)if (!busid_table[i].name[0 ]) {strncpy (busid_table[i].name, busid,BUSID_SIZE );
@@ -111,7 +101,6 @@ static int add_match_busid(char *busid)spin_unlock (&busid_table_lock);return 0 ; } -spin_unlock (&busid_table_lock);return -1 ;
@@ -122,7 +111,6 @@ int del_match_busid(char *busid)int i;spin_lock (&busid_table_lock); -for (i =0 ; i <MAX_BUSID ; i++)if (!strncmp (busid_table[i].name, busid,BUSID_SIZE )) {/* found */
@@ -135,7 +123,6 @@ int del_match_busid(char *busid)spin_unlock (&busid_table_lock);return 0 ; } -spin_unlock (&busid_table_lock);return -1 ;
drivers/staging/usbip/stub_tx.c +6 -7
@@ -96,13 +96,12 @@ void stub_complete(struct urb *urb)/* link a urb to the queue of tx. */ spin_lock_irqsave (&sdev->priv_lock, flags); -if (priv->unlinking) {stub_enqueue_ret_unlink (sdev, priv->seqnum, urb->status);stub_free_priv_and_urb (priv); - }else + }else {list_move_tail (&priv->list, &sdev->priv_tx); - + }spin_unlock_irqrestore (&sdev->priv_lock, flags);/* wake up tx_thread */
@@ -112,10 +111,10 @@ void stub_complete(struct urb *urb)static inline void setup_base_pdu (struct usbip_header_basic *base,__u32 command,__u32 seqnum) { - base->command = command; - base->seqnum = seqnum; - base->devid =0 ; - base->ep =0 ; + base->command = command; + base->seqnum = seqnum; + base->devid =0 ; + base->ep =0 ; base->direction =0 ; }
drivers/staging/usbip/usbip_common.c +6 -7
@@ -63,9 +63,9 @@ static void usbip_dump_buffer(char *buff, int bufflen)static void usbip_dump_pipe (unsigned int p) {unsigned char type =usb_pipetype (p); -unsigned char ep =usb_pipeendpoint (p); -unsigned char dev =usb_pipedevice (p); -unsigned char dir =usb_pipein (p); +unsigned char ep =usb_pipeendpoint (p); +unsigned char dev =usb_pipedevice (p); +unsigned char dir =usb_pipein (p);pr_debug ("dev(%d) ep(%d) [%s] " , dev, ep, dir ?"IN" :"OUT" );
@@ -334,8 +334,8 @@ void usbip_dump_header(struct usbip_header *pdu)EXPORT_SYMBOL_GPL (usbip_dump_header);/* Send/receive messages over TCP/IP. I refer drivers/block/nbd.c */ -int usbip_xmit (int send,struct socket *sock,char *buf, -int size,int msg_flags) +int usbip_xmit (int send,struct socket *sock,char *buf,int size, +int msg_flags) {int result;struct msghdr msg;
@@ -628,8 +628,7 @@ void usbip_header_correct_endian(struct usbip_header *pdu, int send)EXPORT_SYMBOL_GPL (usbip_header_correct_endian);static void usbip_iso_pakcet_correct_endian ( -struct usbip_iso_packet_descriptor *iso, -int send) +struct usbip_iso_packet_descriptor *iso,int send) {/* does not need all members. but copy all simply. */ if (send) {
drivers/staging/usbip/usbip_common.h +1 -1
@@ -65,7 +65,7 @@ enum {#define usbip_dbg_flag_vhci_tx (usbip_debug_flag & usbip_debug_vhci_tx)#define usbip_dbg_flag_stub_rx (usbip_debug_flag & usbip_debug_stub_rx)#define usbip_dbg_flag_stub_tx (usbip_debug_flag & usbip_debug_stub_tx) -#define usbip_dbg_flag_vhci_sysfs (usbip_debug_flag & usbip_debug_vhci_sysfs) +#define usbip_dbg_flag_vhci_sysfs (usbip_debug_flag & usbip_debug_vhci_sysfs)extern unsigned long usbip_debug_flag;extern struct device_attribute dev_attr_usbip_debug;
drivers/staging/usbip/vhci_hcd.c +3 -4
@@ -344,9 +344,9 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,* */ if (dum->resuming &&time_after (jiffies, dum->re_timeout)) { dum->port_status[rhport] |- (1 <<USB_PORT_FEAT_C_SUSPEND ); + (1 <<USB_PORT_FEAT_C_SUSPEND ); dum->port_status[rhport] &- ~(1 <<USB_PORT_FEAT_SUSPEND ); + ~(1 <<USB_PORT_FEAT_SUSPEND ); dum->resuming =0 ; dum->re_timeout =0 ;/* if (dum->driver && dum->driver->resume) {
@@ -639,9 +639,7 @@ no_need_xmit:usb_hcd_unlink_urb_from_ep (hcd, urb); no_need_unlink:spin_unlock_irqrestore (&the_controller->lock, flags); -usb_hcd_giveback_urb (vhci_to_hcd (the_controller), urb, urb->status); -return ret; }
@@ -1033,9 +1031,8 @@ static int vhci_bus_resume(struct usb_hcd *hcd) hcd->state = HC_STATE_RUNNING; }spin_unlock_irq (&vhci->lock); -return rc; -return 0 ; +return rc; }#else
drivers/staging/usbip/vhci_rx.c +0 -2
@@ -179,8 +179,6 @@ static void vhci_recv_ret_unlink(struct vhci_device *vdev, }kfree (unlink); - -return ; }static int vhci_priv_tx_empty (struct vhci_device *vdev)
Signature · 1 line
1.7.5.1
PATCH 6/12 staging: usbip: stub_main.c: code cleanup
In reply to matt mooneyReorder functions; remove match_find() and replace with get_busid_idx(); change other functions to use get_busid_idx(); and code cleanup in the other functions.
---
I apologize for this one. I realize that I should have split it into two with the reorder being separate.
Also, it seems as if there is a synchronization problem carried over from the original code in get_busid_priv(). An address into the busid_table is returned and then the element is accessed and modified elsewhere. Right? Or am I missing something.
drivers/staging/usbip/stub_main.c | 191 +++++++++++++++++++------------------ 1 files changed, 96 insertions(+), 95 deletions(-)
drivers/staging/usbip/stub_main.c +96 -95
@@ -35,112 +35,121 @@ struct kmem_cache *stub_priv_cache;static struct bus_id_priv busid_table[MAX_BUSID ];static spinlock_t busid_table_lock; -int match_busid (const char *busid) +static void init_busid_table (void ) {int i; -spin_lock (&busid_table_lock); -for (i =0 ; i <MAX_BUSID ; i++) -if (busid_table[i].name[0 ]) -if (!strncmp (busid_table[i].name, busid,BUSID_SIZE )) { -/* already registerd */ -spin_unlock (&busid_table_lock); -return 0 ; - } -spin_unlock (&busid_table_lock); +for (i =0 ; i <MAX_BUSID ; i++) { +memset (busid_table[i].name,0 ,BUSID_SIZE ); + busid_table[i].status =STUB_BUSID_OTHER ; + busid_table[i].interf_count =0 ; + busid_table[i].sdev =NULL ; + busid_table[i].shutdown_busid =0 ; + } -return 1 ; +spin_lock_init (&busid_table_lock); } -struct bus_id_priv *get_busid_priv (const char *busid) +/* +* Find the index of the busid by name. +* Must be called with busid_table_lock held. +*/ +static int get_busid_idx (const char *busid) {int i; +int idx =-1 ; -spin_lock (&busid_table_lock);for (i =0 ; i <MAX_BUSID ; i++)if (busid_table[i].name[0 ])if (!strncmp (busid_table[i].name, busid,BUSID_SIZE )) { -/* already registerd */ -spin_unlock (&busid_table_lock); -return &(busid_table[i]); + idx = i; +break ; } -spin_unlock (&busid_table_lock); - -return NULL ; +return idx; } -static ssize_t show_match_busid (struct device_driver *drv,char *buf) +struct bus_id_priv *get_busid_priv (const char *busid) { -int i; -char *out = buf; +int idx; +struct bus_id_priv *bid =NULL ;spin_lock (&busid_table_lock); -for (i =0 ; i <MAX_BUSID ; i++) -if (busid_table[i].name[0 ]) - out +=sprintf (out,"%s " , busid_table[i].name); + idx =get_busid_idx (busid); +if (idx >=0 ) + bid = &(busid_table[idx]);spin_unlock (&busid_table_lock); - out +=sprintf (out,"\n" ); -return out - buf; +return bid; }static int add_match_busid (char *busid) {int i; - -if (!match_busid (busid)) -return 0 ; +int ret =-1 ;spin_lock (&busid_table_lock); +/* already registered? */ +if (get_busid_idx (busid) >=0 ) { + ret =0 ; + goto out; + } +for (i =0 ; i <MAX_BUSID ; i++)if (!busid_table[i].name[0 ]) {strncpy (busid_table[i].name, busid,BUSID_SIZE );if ((busid_table[i].status !=STUB_BUSID_ALLOC ) && (busid_table[i].status !=STUB_BUSID_REMOV )) busid_table[i].status =STUB_BUSID_ADDED ; -spin_unlock (&busid_table_lock); -return 0 ; + ret =0 ; +break ; } + +out:spin_unlock (&busid_table_lock); -return -1 ; +return ret; }int del_match_busid (char *busid) { -int i; +int idx; +int ret =-1 ;spin_lock (&busid_table_lock); -for (i =0 ; i <MAX_BUSID ; i++) -if (!strncmp (busid_table[i].name, busid,BUSID_SIZE )) { -/* found */ -if (busid_table[i].status =STUB_BUSID_OTHER ) -memset (busid_table[i].name,0 ,BUSID_SIZE ); -if ((busid_table[i].status !=STUB_BUSID_OTHER ) && - (busid_table[i].status !=STUB_BUSID_ADDED )) { - busid_table[i].status =STUB_BUSID_REMOV ; - } -spin_unlock (&busid_table_lock); -return 0 ; - } + idx =get_busid_idx (busid); +if (idx <0 ) + goto out; + +/* found */ + ret =0 ; + +if (busid_table[idx].status =STUB_BUSID_OTHER ) +memset (busid_table[idx].name,0 ,BUSID_SIZE ); + +if ((busid_table[idx].status !=STUB_BUSID_OTHER ) && + (busid_table[idx].status !=STUB_BUSID_ADDED )) + busid_table[idx].status =STUB_BUSID_REMOV ; + +out:spin_unlock (&busid_table_lock); -return -1 ; +return ret; } -static void init_busid_table (void ) +static ssize_t show_match_busid (struct device_driver *drv,char *buf) {int i; +char *out = buf; -for (i =0 ; i <MAX_BUSID ; i++) { -memset (busid_table[i].name,0 ,BUSID_SIZE ); - busid_table[i].status =STUB_BUSID_OTHER ; - busid_table[i].interf_count =0 ; - busid_table[i].sdev =NULL ; - busid_table[i].shutdown_busid =0 ; - } +spin_lock (&busid_table_lock); +for (i =0 ; i <MAX_BUSID ; i++) +if (busid_table[i].name[0 ]) + out +=sprintf (out,"%s " , busid_table[i].name); +spin_unlock (&busid_table_lock); -spin_lock_init (&busid_table_lock); + out +=sprintf (out,"\n" ); + +return out - buf; } static ssize_t store_match_busid(struct device_driver *dev, const char *buf,
@@ -162,23 +171,24 @@ static ssize_t store_match_busid(struct device_driver *dev, const char *buf,strncpy (busid, buf +4 ,BUSID_SIZE );if (!strncmp (buf,"add " ,4 )) { -if (add_match_busid (busid) <0 ) +if (add_match_busid (busid) <0 ) {return -ENOMEM ; -else { + }else {pr_debug ("add busid %s\n" , busid);return count; } }else if (!strncmp (buf,"del " ,4 )) { -if (del_match_busid (busid) <0 ) +if (del_match_busid (busid) <0 ) {return -ENODEV ; -else { + }else {pr_debug ("del busid %s\n" , busid);return count; } - }else + }else {return -EINVAL ; + } } -static DRIVER_ATTR(match_busid, S_IRUSR|S_IWUSR, show_match_busid, +static DRIVER_ATTR(match_busid, S_IRUSR | S_IWUSR, show_match_busid, store_match_busid);static struct stub_priv *stub_priv_pop_from_listhead (struct list_head *listhead)
@@ -201,84 +211,75 @@ static struct stub_priv *stub_priv_pop(struct stub_device *sdev)spin_lock_irqsave (&sdev->priv_lock, flags); priv =stub_priv_pop_from_listhead (&sdev->priv_init); -if (priv) { -spin_unlock_irqrestore (&sdev->priv_lock, flags); -return priv; - } +if (priv) + goto done; priv =stub_priv_pop_from_listhead (&sdev->priv_tx); -if (priv) { -spin_unlock_irqrestore (&sdev->priv_lock, flags); -return priv; - } +if (priv) + goto done; priv =stub_priv_pop_from_listhead (&sdev->priv_free); -if (priv) { -spin_unlock_irqrestore (&sdev->priv_lock, flags); -return priv; - } +done:spin_unlock_irqrestore (&sdev->priv_lock, flags); -return NULL ; + +return priv; }void stub_device_cleanup_urbs (struct stub_device *sdev) {struct stub_priv *priv; +struct urb *urb;pr_debug ("free sdev %p\n" , sdev);while ((priv =stub_priv_pop (sdev))) { -struct urb *urb = priv->urb; - -pr_debug (" free urb %p\n" , urb); + urb = priv->urb; +pr_debug ("free urb %p\n" , urb);usb_kill_urb (urb);kmem_cache_free (stub_priv_cache, priv);kfree (urb->transfer_buffer);kfree (urb->setup_packet); -usb_free_urb (urb); } }static int __init usb_stub_init (void ) { -int ret; +int ret =0 ; stub_priv_cache =kmem_cache_create ("stub_priv" ,sizeof (struct stub_priv ),0 ,SLAB_HWCACHE_ALIGN ,NULL ); -if (!stub_priv_cache) { -pr_err ("create stub_priv_cache error\n" ); +pr_err ("kmem_cache_create failed\n" );return -ENOMEM ; } ret =usb_register (&stub_driver); -if (ret) { +if (ret <0 ) {pr_err ("usb_register failed %d\n" , ret); - goto error_usb_register; + goto err_usb_register; } -pr_info (DRIVER_DESC " " USBIP_VERSION "\n" ); - -init_busid_table (); - ret =driver_create_file (&stub_driver.drvwrap.driver, &driver_attr_match_busid); - -if (ret) { -pr_err ("create driver sysfs\n" ); - goto error_create_file; +if (ret <0 ) { +pr_err ("driver_create_file failed\n" ); + goto err_create_file; } -return ret; -error_create_file: +init_busid_table (); +pr_info (DRIVER_DESC " v" USBIP_VERSION "\n" ); + goto out; + +err_create_file:usb_deregister (&stub_driver); -error_usb_register: +err_usb_register:kmem_cache_destroy (stub_priv_cache); +out:return ret; }
Signature · 1 line
1.7.5.1
Re: [PATCH 06/12] staging: usbip: stub_main.c: code cleanup
Quoted 8 lines · matt mooney
Reorder functions; remove
match_find()and replace withget_busid_idx(); change other functions to useget_busid_idx(); and code cleanup in the other functions.Signed-off-by matt mooney <[email protected]>---
I apologize for this one. I realize that I should have split it into two with the reorder being separate.
Yes, please split it up.
Quoted 4 lines
Also, it seems as if there is a synchronization problem carried over from the original code in
get_busid_priv(). An address into thebusid_tableis returned and then the element is accessed and modified elsewhere. Right? Or am I missing something.
You might be correct, I don't really know.
thanks,
greg k-h
Re: [PATCH 06/12] staging: usbip: stub_main.c: code cleanup
Quoted 11 lines · Greg KH
Quoted 8 lines · matt mooney
Reorder functions; remove
match_find()and replace withget_busid_idx(); change other functions to useget_busid_idx(); and code cleanup in the other functions.Signed-off-by matt mooney <[email protected]>---
I apologize for this one. I realize that I should have split it into two with the reorder being separate.Yes, please split it up.
Okay, I have to try and figure out a sane way of doing this but I will split it.
Quoted 7 lines
Quoted 4 lines
Also, it seems as if there is a synchronization problem carried over from the original code in
get_busid_priv(). An address into thebusid_tableis returned and then the element is accessed and modified elsewhere. Right? Or am I missing something.You might be correct, I don't really know.
Yeah I will have to figure that out as I go.
Signature · 1 line
GPG-Key: 9AFE00EA
PATCH 7/12 staging: usbip: stub_main.c: change __init/__exit prefix and use KMEM_CACHE
In reply to matt mooneyChange prefix to usbip and use the KMEM_CACHE macro for cache creation.
--- drivers/staging/usbip/stub_main.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-)
drivers/staging/usbip/stub_main.c +6 -7
@@ -246,13 +246,12 @@ void stub_device_cleanup_urbs(struct stub_device *sdev) } } -static int __init usb_stub_init (void ) +static int __init usbip_stub_init (void ) {int ret =0 ; - stub_priv_cache =kmem_cache_create ("stub_priv" , -sizeof (struct stub_priv ),0 , -SLAB_HWCACHE_ALIGN ,NULL ); + stub_priv_cache =KMEM_CACHE (stub_priv,SLAB_HWCACHE_ALIGN ); +if (!stub_priv_cache) {pr_err ("kmem_cache_create failed\n" );return -ENOMEM ;
@@ -283,7 +282,7 @@ out:return ret; } -static void __exit usb_stub_exit(void) +static void __exit usbip_stub_exit(void) {driver_remove_file (&stub_driver.drvwrap.driver, &driver_attr_match_busid);
@@ -297,8 +296,8 @@ static void __exit usb_stub_exit(void)kmem_cache_destroy (stub_priv_cache); } -module_init (usb_stub_init); -module_exit (usb_stub_exit); +module_init (usbip_stub_init); +module_exit (usbip_stub_exit);MODULE_AUTHOR (DRIVER_AUTHOR );MODULE_DESCRIPTION (DRIVER_DESC );
Signature · 1 line
1.7.5.1
PATCH 8/12 staging: usbip: usbip_common.c: fix misspelled function name
In reply to matt mooneyChange pakcet to packet in usbip_iso_packet_correct_endian().
--- drivers/staging/usbip/usbip_common.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/staging/usbip/usbip_common.c +3 -3
@@ -627,7 +627,7 @@ void usbip_header_correct_endian(struct usbip_header *pdu, int send) }EXPORT_SYMBOL_GPL (usbip_header_correct_endian); -static void usbip_iso_pakcet_correct_endian ( +static void usbip_iso_packet_correct_endian (struct usbip_iso_packet_descriptor *iso,int send) {/* does not need all members. but copy all simply. */
@@ -677,7 +677,7 @@ void *usbip_alloc_iso_desc_pdu(struct urb *urb, ssize_t *bufflen) iso = buff + (i *sizeof (*iso));usbip_pack_iso (iso, &urb->iso_frame_desc[i],1 ); -usbip_iso_pakcet_correct_endian (iso,1 ); +usbip_iso_packet_correct_endian (iso,1 ); } *bufflen = size;
@@ -728,7 +728,7 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)for (i =0 ; i < np; i++) { iso = buff + (i *sizeof (*iso)); -usbip_iso_pakcet_correct_endian (iso,0 ); +usbip_iso_packet_correct_endian (iso,0 );usbip_pack_iso (iso, &urb->iso_frame_desc[i],0 ); total_length += urb->iso_frame_desc[i].actual_length; }
Signature · 1 line
1.7.5.1
PATCH 9/12 staging: usbip: edit Kconfig and rename CONFIG options
In reply to matt mooneyEdit configuration option help text; rename CONFIG options to use a USBIP prefix and change COMMON to CORE in both the Kconfig and Makefile; edit the menu entries; and edit the driver descriptions.
---
I was thinking that renaming the modules would be a good idea. This patch is a start in that direction without changing anything in userspace yet. My idea is to change usbip_common_mod to usbip-core and usbip to usbip-host. What do you think?
drivers/staging/Makefile | 2 +- drivers/staging/usbip/Kconfig | 52 +++++++++++++++++---------------- drivers/staging/usbip/Makefile | 8 ++-- drivers/staging/usbip/stub_main.c | 2 +- drivers/staging/usbip/usbip_common.c | 4 +- drivers/staging/usbip/vhci_hcd.c | 2 +- 6 files changed, 36 insertions(+), 34 deletions(-)
drivers/staging/Makefile +1 -1
@@ -12,7 +12,7 @@ obj-$(CONFIG_VIDEO_CX25821) += cx25821/ obj-$(CONFIG_VIDEO_TM6000) += tm6000/ obj-$(CONFIG_DVB_CXD2099) += cxd2099/ obj-$(CONFIG_LIRC_STAGING) += lirc/ -obj-$(CONFIG_USB_IP_COMMON) += usbip/ +obj-$(CONFIG_USBIP_CORE) += usbip/ obj-$(CONFIG_W35UND) += winbond/ obj-$(CONFIG_PRISM2_USB) += wlan-ng/ obj-$(CONFIG_ECHO) += echo/
drivers/staging/usbip/Kconfig +27 -25
70 lines not on this page · raw message
drivers/staging/usbip/Makefile +4 -4
@@ -1,10 +1,10 @@ -ccflags-$(CONFIG_USB_IP_DEBUG) := -DDEBUG +ccflags-$(CONFIG_USBIP_DEBUG) := -DDEBUG -obj-$(CONFIG_USB_IP_COMMON) += usbip_common_mod.o +obj-$(CONFIG_USBIP_CORE) += usbip_common_mod.o usbip_common_mod-y := usbip_common.o usbip_event.o -obj-$(CONFIG_USB_IP_VHCI_HCD) += vhci-hcd.o +obj-$(CONFIG_USBIP_VHCI_HCD) += vhci-hcd.o vhci-hcd-y := vhci_sysfs.o vhci_tx.o vhci_rx.o vhci_hcd.o -obj-$(CONFIG_USB_IP_HOST) += usbip.o +obj-$(CONFIG_USBIP_HOST) += usbip.o usbip-y := stub_dev.o stub_main.o stub_rx.o stub_tx.o
drivers/staging/usbip/stub_main.c +1 -1
@@ -23,7 +23,7 @@#include "stub.h" #define DRIVER_AUTHOR "Takahiro Hirofuchi" -#define DRIVER_DESC "Stub Driver for USB/IP" +#define DRIVER_DESC "USB/IP Host Driver"struct kmem_cache *stub_priv_cache;/*
drivers/staging/usbip/usbip_common.c +2 -2
@@ -27,9 +27,9 @@#include "usbip_common.h" #define DRIVER_AUTHOR "Takahiro Hirofuchi <[email protected]>" -#define DRIVER_DESC "USB/IP Common Driver" +#define DRIVER_DESC "USB/IP Core" -#ifdef CONFIG_USB_IP_DEBUG +#ifdef CONFIG_USBIP_DEBUG unsigned long usbip_debug_flag =0xffffffff ;#else unsigned long usbip_debug_flag;
drivers/staging/usbip/vhci_hcd.c +1 -1
@@ -28,7 +28,7 @@#include "vhci.h" #define DRIVER_AUTHOR "Takahiro Hirofuchi" -#define DRIVER_DESC "Virtual Host Controller Interface Driver for USB/IP" +#define DRIVER_DESC "USB/IP 'Virtual' Host Controller (VHCI) Driver"/* * TODO
Signature · 1 line
1.7.5.1
Re: [PATCH 09/12] staging: usbip: edit Kconfig and rename CONFIG options
Quoted 10 lines · matt mooney
Edit configuration option help text; rename CONFIG options to use a USBIP prefix and change COMMON to CORE in both the
KconfigandMakefile; edit the menu entries; and edit the driver descriptions.Signed-off-by matt mooney <[email protected]>---
I was thinking that renaming the modules would be a good idea. This patch is a start in that direction without changing anything in userspace yet. My idea is to changeusbip_common_modto usbip-core and usbip to usbip-host. What do you think?
That makes more sense, yes, please do so.
thanks,
greg k-h
PATCH 10/12 staging: usbip: stub.h: reorganize
In reply to matt mooneyMove a few functions around for better organization and add missing parameter names in function declarations.
--- drivers/staging/usbip/stub.h | 19 ++++++++++--------- 1 files changed, 10 insertions(+), 9 deletions(-)
drivers/staging/usbip/stub.h +10 -9
@@ -86,21 +86,22 @@ struct bus_id_priv {char shutdown_busid; }; +/* stub_priv is allocated from stub_priv_cache */ extern struct kmem_cache *stub_priv_cache; -/* stub_tx.c */ -void stub_complete (struct urb *); -int stub_tx_loop (void *data); -/* stub_dev.c */ extern struct usb_driver stub_driver; -/* stub_rx.c */ -int stub_rx_loop (void *data); -void stub_enqueue_ret_unlink (struct stub_device *,__u32 ,__u32 ); -/* stub_main.c */ struct bus_id_priv *get_busid_priv (const char *busid);int del_match_busid (char *busid); -void stub_device_cleanup_urbs (struct stub_device *sdev); + +/* stub_rx.c */ +int stub_rx_loop (void *data); + +/* stub_tx.c */ +void stub_enqueue_ret_unlink (struct stub_device *sdev,__u32 seqnum, +__u32 status); +void stub_complete (struct urb *urb); +int stub_tx_loop (void *data);
Signature · 1 line
1.7.5.1
PATCH 11/12 staging: usbip: vhci.h: reorganize
In reply to matt mooneySplit function declarations by source file.
--- drivers/staging/usbip/vhci.h | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
drivers/staging/usbip/vhci.h +5 -3
@@ -106,16 +106,18 @@ struct vhci_hcd {extern struct vhci_hcd *the_controller;extern struct attribute_group dev_attr_group; +#define hardware (&the_controller->pdev.dev)/* vhci_hcd.c */ void rh_port_connect (int rhport,enum usb_device_speed speed);void rh_port_disconnect (int rhport); -int vhci_rx_loop (void *data); -int vhci_tx_loop (void *data); +/* vhci_rx.c */ struct urb *pickup_urb_and_free_priv (struct vhci_device *vdev,__u32 seqnum); +int vhci_rx_loop (void *data); -#define hardware (&the_controller->pdev.dev) +/* vhci_tx.c */ +int vhci_tx_loop (void *data);static inline struct vhci_device *port_to_vdev (__u32 port) {
Signature · 1 line
1.7.5.1
PATCH 12/12 staging: usbip: usbip_common.h: reorganize and document request headers
In reply to matt mooneyDocument the request header structures; move #defines out of the structures; organize function declarations by source file; and move inline functions to the end of file.
--- drivers/staging/usbip/usbip_common.h | 236 +++++++++++++++++----------------- 1 files changed, 121 insertions(+), 115 deletions(-)
drivers/staging/usbip/usbip_common.h +121 -115
@@ -104,111 +104,110 @@ extern struct device_attribute dev_attr_usbip_debug; usbip_dbg_with_flag(usbip_debug_stub_tx, fmt , ##args)/* -* USB/IP request headers. -* Currently, we define 4 request types: +* USB/IP request headers * -* - CMD_SUBMIT transfers a USB request, corresponding to usb_submit_urb(). -* (client to server) -* - RET_RETURN transfers the result of CMD_SUBMIT. -* (server to client) -* - CMD_UNLINK transfers an unlink request of a pending USB request. +* Each request is transferred across the network to its counterpart, which +* facilitates the normal USB communication. The values contained in the headers +* are basically the same as in a URB. Currently, four request types are +* defined: +* +* - USBIP_CMD_SUBMIT: a USB request block, corresponds to usb_submit_urb() * (client to server) -* - RET_UNLINK transfers the result of CMD_UNLINK. +* +* - USBIP_RET_SUBMIT: the result of USBIP_CMD_SUBMIT * (server to client) * -* Note: The below request formats are based on the USB subsystem of Linux. Its -* details will be defined when other implementations come. +* - USBIP_CMD_UNLINK: an unlink request of a pending USBIP_CMD_SUBMIT, +* corresponds to usb_unlink_urb() +* (client to server) * +* - USBIP_RET_UNLINK: the result of USBIP_CMD_UNLINK +* (server to client) * */ +#define USBIP_CMD_SUBMIT 0x0001 +#define USBIP_RET_SUBMIT 0x0002 +#define USBIP_CMD_UNLINK 0x0003 +#define USBIP_RET_UNLINK 0x0004 -/* -* A basic header followed by other additional headers. +#define USBIP_DIR_IN 0x00 +#define USBIP_DIR_OUT 0x01 + +/** +* struct usbip_header_basic - data pertinent to every request +* @command: the usbip request type +* @seqnum: sequential number that identifies requests; incremented per +* connection +* @devid: specifies a remote USB device uniquely instead of busnum and devnum; +* in the stub driver, this value is ((busnum << 16) | devnum) +* @direction: direction of the transfer +* @ep: endpoint number */ struct usbip_header_basic { -#define USBIP_CMD_SUBMIT 0x0001 -#define USBIP_CMD_UNLINK 0x0002 -#define USBIP_RET_SUBMIT 0x0003 -#define USBIP_RET_UNLINK 0x0004__u32 command; - -/* sequential number which identifies requests. -* incremented per connections */ __u32 seqnum; - -/* devid is used to specify a remote USB device uniquely instead -* of busnum and devnum in Linux. In the case of Linux stub_driver, -* this value is ((busnum << 16) | devnum) */ __u32 devid; - -#define USBIP_DIR_OUT 0 -#define USBIP_DIR_IN 1__u32 direction; -__u32 ep;/* endpoint number */ +__u32 ep; } __packed; -/* -* An additional header for a CMD_SUBMIT packet. +/** +* struct usbip_header_cmd_submit - USBIP_CMD_SUBMIT packet header +* @transfer_flags: URB flags +* @transfer_buffer_length: the data size for (in) or (out) transfer +* @start_frame: initial frame for isochronous or interrupt transfers +* @number_of_packets: number of isochronous packets +* @interval: maximum time for the request on the server-side host controller +* @setup: setup data for a control request */ struct usbip_header_cmd_submit { -/* these values are basically the same as in a URB. */ - -/* the same in a URB. */ __u32 transfer_flags; - -/* set the following data size (out), -* or expected reading data size (in) */ __s32 transfer_buffer_length;/* it is difficult for usbip to sync frames (reserved only?) */ __s32 start_frame; - -/* the number of iso descriptors that follows this header */ __s32 number_of_packets; - -/* the maximum time within which this request works in a host -* controller of a server side */ __s32 interval; -/* set setup packet data for a CTRL request */ unsigned char setup[8 ]; } __packed; -/* -* An additional header for a RET_SUBMIT packet. +/** +* struct usbip_header_ret_submit - USBIP_RET_SUBMIT packet header +* @status: return status of a non-iso request +* @actual_length: number of bytes transferred +* @start_frame: initial frame for isochronous or interrupt transfers +* @number_of_packets: number of isochronous packets +* @error_count: number of errors for isochronous transfers */ struct usbip_header_ret_submit {__s32 status; -__s32 actual_length;/* returned data length */ -__s32 start_frame;/* ISO and INT */ -__s32 number_of_packets;/* ISO only */ -__s32 error_count;/* ISO only */ +__s32 actual_length; +__s32 start_frame; +__s32 number_of_packets; +__s32 error_count; } __packed; -/* -* An additional header for a CMD_UNLINK packet. +/** +* struct usbip_header_cmd_unlink - USBIP_CMD_UNLINK packet header +* @seqnum: the URB seqnum to unlink */ struct usbip_header_cmd_unlink { -__u32 seqnum;/* URB's seqnum that will be unlinked */ +__u32 seqnum; } __packed; -/* -* An additional header for a RET_UNLINK packet. +/** +* struct usbip_header_ret_unlink - USBIP_RET_UNLINK packet header +* @status: return status of the request */ struct usbip_header_ret_unlink {__s32 status; } __packed; -/* the same as usb_iso_packet_descriptor but packed for pdu */ -struct usbip_iso_packet_descriptor { -__u32 offset; -__u32 length;/* expected length */ -__u32 actual_length; -__u32 status; -} __packed; - -/* -* All usbip packets use a common header to keep code simple. +/** +* struct usbip_header - common header for all usbip packets +* @base: the basic header +* @u: packet type dependent header */ struct usbip_header {struct usbip_header_basic base;
@@ -221,40 +220,15 @@ struct usbip_header { } u; } __packed; -int usbip_xmit (int ,struct socket *,char *,int ,int ); -int usbip_sendmsg (struct socket *,struct msghdr *,int ); - -static inline int interface_to_busnum (struct usb_interface *interface) -{ -struct usb_device *udev =interface_to_usbdev (interface); -return udev->bus->busnum; -} - -static inline int interface_to_devnum (struct usb_interface *interface) -{ -struct usb_device *udev =interface_to_usbdev (interface); -return udev->devnum; -} - -static inline int interface_to_infnum (struct usb_interface *interface) -{ -return interface->cur_altsetting->desc.bInterfaceNumber; -} - -#if 0 -int setnodelay (struct socket *); -int setquickack (struct socket *); -int setkeepalive (struct socket *socket); -void setreuse (struct socket *); -#endif - -struct socket *sockfd_to_socket (unsigned int ); -int set_sockaddr (struct socket *socket,struct sockaddr_storage *ss); - -void usbip_dump_urb (struct urb *purb); -void usbip_dump_header (struct usbip_header *pdu); - -struct usbip_device ; +/* +* This is the same as usb_iso_packet_descriptor but packed for pdu. +*/ +struct usbip_iso_packet_descriptor { +__u32 offset; +__u32 length;/* expected length */ +__u32 actual_length; +__u32 status; +} __packed;enum usbip_side {USBIP_VHCI ,
@@ -277,20 +251,7 @@ enum usbip_status { VDEV_ST_ERROR }; -/* a common structure for stub_device and vhci_device */ -struct usbip_device { -enum usbip_side side; -enum usbip_status status; - -/* lock for status */ -spinlock_t lock; - -struct socket *tcp_socket; - -struct task_struct *tcp_rx; -struct task_struct *tcp_tx; - -/* event handler */ +/* event handler */ #define USBIP_EH_SHUTDOWN (1 << 0)#define USBIP_EH_BYE (1 << 1)#define USBIP_EH_RESET (1 << 2)
@@ -307,6 +268,19 @@ struct usbip_device {#define VDEV_EVENT_ERROR_TCP (USBIP_EH_SHUTDOWN | USBIP_EH_RESET)#define VDEV_EVENT_ERROR_MALLOC (USBIP_EH_SHUTDOWN | USBIP_EH_UNUSABLE) +/* a common structure for stub_device and vhci_device */ +struct usbip_device { +enum usbip_side side; +enum usbip_status status; + +/* lock for status */ +spinlock_t lock; + +struct socket *tcp_socket; + +struct task_struct *tcp_rx; +struct task_struct *tcp_tx; +unsigned long event;struct task_struct *eh;wait_queue_head_t eh_waitq;
@@ -318,17 +292,32 @@ struct usbip_device { } eh_ops; }; +#if 0 +int usbip_sendmsg (struct socket *,struct msghdr *,int ); +int set_sockaddr (struct socket *socket,struct sockaddr_storage *ss); +int setnodelay (struct socket *); +int setquickack (struct socket *); +int setkeepalive (struct socket *socket); +void setreuse (struct socket *); +#endif + +/* usbip_common.c */ +void usbip_dump_urb (struct urb *purb); +void usbip_dump_header (struct usbip_header *pdu); + +int usbip_xmit (int send,struct socket *sock,char *buf,int size, +int msg_flags); +struct socket *sockfd_to_socket (unsigned int sockfd); +void usbip_pack_pdu (struct usbip_header *pdu,struct urb *urb,int cmd,int pack); -void usbip_header_correct_endian (struct usbip_header *pdu,int send); -/* some members of urb must be substituted before. */ -int usbip_recv_xbuff (struct usbip_device *ud,struct urb *urb); + +void *usbip_alloc_iso_desc_pdu (struct urb *urb,ssize_t *bufflen);/* some members of urb must be substituted before. */ int usbip_recv_iso (struct usbip_device *ud,struct urb *urb); -/* some members of urb must be substituted before. */ int usbip_pad_iso (struct usbip_device *ud,struct urb *urb); -void *usbip_alloc_iso_desc_pdu (struct urb *urb,ssize_t *bufflen); +int usbip_recv_xbuff (struct usbip_device *ud,struct urb *urb);/* usbip_event.c */ int usbip_start_eh (struct usbip_device *ud);
@@ -336,4 +325,21 @@ void usbip_stop_eh(struct usbip_device *ud);void usbip_event_add (struct usbip_device *ud,unsigned long event);int usbip_event_happened (struct usbip_device *ud); +static inline int interface_to_busnum (struct usb_interface *interface) +{ +struct usb_device *udev =interface_to_usbdev (interface); +return udev->bus->busnum; +} + +static inline int interface_to_devnum (struct usb_interface *interface) +{ +struct usb_device *udev =interface_to_usbdev (interface); +return udev->devnum; +} + +static inline int interface_to_infnum (struct usb_interface *interface) +{ +return interface->cur_altsetting->desc.bInterfaceNumber; +} +#endif /* __USBIP_COMMON_H */
Signature · 1 line
1.7.5.1
Greg KH
AppliedQuoted 3 lines · matt mooney
Hi Greg,
I am interested in hearing your thoughts on the userspace interface?
Which one? The user/kernel interface?
Quoted 3 lines
Even as is, it definitely needs some work. For one thing I was thinking of removing the debug flag from userspace. The userspace utilites are not using it anyway, and it is not 100% working yet.
I haven't looked at the interface much yet, what is your issue with the debug flag?
To make things easier, and allow you to be able to change the userspace interface, I suggest moving the userspace code into the kernel tree, and building it here. That would allow you to move forward with changing things easier, doing it all in one place allows you to keep stuff in sync, and provides people an easier way to actually get the needed tools to drive the code.
What do you think about doing that?
I've applied a number of these patches, care to respin the others and resend them based on my latest tree?
thanks,
greg k-h
Quoted 6 lines · Greg KH
Quoted 3 lines · matt mooney
Hi Greg,
I am interested in hearing your thoughts on the userspace interface?
Which one? The user/kernel interface?
Yes.
Quoted 7 lines
Quoted 4 lines
Even as is, it definitely needs some work. For one thing I was thinking of removing the debug flag from userspace. The userspace utilites are not using it anyway, and it is not 100% working yet.
I haven't looked at the interface much yet, what is your issue with the debug flag?
I guess it just added a lot of code (most was actually eliminated in the pr_*() change), is not complete, and is inconsistently used. But I see that it would be helpful especially due to the amount of debug messages printed by default. So ignore that complaint.
Quoted 8 lines
To make things easier, and allow you to be able to change the userspace interface, I suggest moving the userspace code into the kernel tree, and building it here. That would allow you to move forward with changing things easier, doing it all in one place allows you to keep stuff in sync, and provides people an easier way to actually get the needed tools to drive the code.
What do you think about doing that?
Sure, I didn't even know that was an option. How do you propose that is done though?
Quoted 2 lines
I've applied a number of these patches, care to respin the others and resend them based on my latest tree?
Okay, I will do that. I noticed some were missed but that may be because they depended on a few of the unapplied ones anyway, so I will also resend those as well.
Signature · 5 lines
GPG-Key: 9AFE00EA -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 15 lines · matt mooney
Quoted 6 lines · Greg KH
Quoted 3 lines · matt mooney
Hi Greg,
I am interested in hearing your thoughts on the userspace interface?
Which one? The user/kernel interface?
Yes.
Quoted 5 lines
Quoted 4 lines
Even as is, it definitely needs some work. For one thing I was thinking of removing the debug flag from userspace. The userspace utilites are not using it anyway, and it is not 100% working yet.
Quoted 7 lines
Quoted 2 lines
I haven't looked at the interface much yet, what is your issue with the debug flag?
I guess it just added a lot of code (most was actually eliminated in the pr_*() change), is not complete, and is inconsistently used. But I see that it would be helpful especially due to the amount of debug messages printed by default. So ignore that complaint.
Ok.
Quoted 11 lines
Quoted 8 lines
To make things easier, and allow you to be able to change the userspace interface, I suggest moving the userspace code into the kernel tree, and building it here. That would allow you to move forward with changing things easier, doing it all in one place allows you to keep stuff in sync, and provides people an easier way to actually get the needed tools to drive the code.
What do you think about doing that?
Sure, I didn't even know that was an option. How do you propose that is done though?
We add it to drivers/staging/usbip/userspace/ and build it like the perf code is built.
Quoted 6 lines
Quoted 2 lines
I've applied a number of these patches, care to respin the others and resend them based on my latest tree?
Okay, I will do that. I noticed some were missed but that may be because they depended on a few of the unapplied ones anyway, so I will also resend those as well.
Yes, they couldn't be applied so I didn't.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 33 lines · Greg KH
Quoted 15 lines · matt mooney
Quoted 6 lines · Greg KH
Quoted 3 lines · matt mooney
Hi Greg,
I am interested in hearing your thoughts on the userspace interface?
Which one? The user/kernel interface?
Yes.
Quoted 5 lines
Quoted 4 lines
Even as is, it definitely needs some work. For one thing I was thinking of removing the debug flag from userspace. The userspace utilites are not using it anyway, and it is not 100% working yet.
Quoted 7 lines
Quoted 2 lines
I haven't looked at the interface much yet, what is your issue with the debug flag?
I guess it just added a lot of code (most was actually eliminated in the pr_*() change), is not complete, and is inconsistently used. But I see that it would be helpful especially due to the amount of debug messages printed by default. So ignore that complaint.
Ok.
Quoted 7 lines
Quoted 7 lines
To make things easier, and allow you to be able to change the userspace interface, I suggest moving the userspace code into the kernel tree, and building it here. That would allow you to move forward with changing things easier, doing it all in one place allows you to keep stuff in sync, and provides people an easier way to actually get the needed tools to drive the code.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
What do you think about doing that?
Sure, I didn't even know that was an option. How do you propose that is done though?
We add it to
drivers/staging/usbip/userspace/and build it like the perf code is built.
Okay, I will add the code as is to userspace/ except for the module name changes of course. Now I am suppose to add this to scripts/packages/Makefile or is there another way for staging?
Thanks,
matt
Quoted 13 lines
Quoted 6 lines
Quoted 2 lines
I've applied a number of these patches, care to respin the others and resend them based on my latest tree?
Okay, I will do that. I noticed some were missed but that may be because they depended on a few of the unapplied ones anyway, so I will also resend those as well.
Yes, they couldn't be applied so I didn't.
thanks,
greg k-h
Signature · 5 lines
GPG-Key: 9AFE00EA -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 6 lines · matt mooney
Quoted 2 lines
We add it to
drivers/staging/usbip/userspace/and build it like the perf code is built.Okay, I will add the code as is to userspace/ except for the module name changes of course. Now I am suppose to add this to
scripts/packages/Makefileor is there another way for staging?
I really don't know as I haven't looked at that part of the build system in years. If you have problems, I will be glad to help out, but give it a try first :)
thanks,
greg k-h
Quoted 11 lines · Greg KH
Quoted 6 lines · matt mooney
Quoted 2 lines
We add it to
drivers/staging/usbip/userspace/and build it like the perf code is built.Okay, I will add the code as is to userspace/ except for the module name changes of course. Now I am suppose to add this to
scripts/packages/Makefileor is there another way for staging?I really don't know as I haven't looked at that part of the build system in years. If you have problems, I will be glad to help out, but give it a try first :)
Not a problem; I just wanted to make sure. I actually sent in a patch to fix a bug with building perf earlier today so hopefully Michal picks it up quickly.
I resent two patches that don't rely on my pr_*() commit in a new thread. The other patches will be resent once I get the dev_*() changes in. But I will send in the userspace utility patch asap although I might have created a conflict with the perf patch I sent in earlier.
Thanks,
matt
Signature · 5 lines
GPG-Key: 9AFE00EA -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 5 lines · matt mooney
I resent two patches that don't rely on my pr_*() commit in a new thread. The other patches will be resent once I get the dev_*() changes in. But I will send in the userspace utility patch asap although I might have created a conflict with the perf patch I sent in earlier.
I've applied these two patches.
thanks,
greg k-h
PATCH v2 3/12 staging: usbip: fix header includes
In reply to matt mooneyFrom: matt mooney <[email protected]>
Modify header directives to include what is needed by each file and not already included in its own header.
--- drivers/staging/usbip/stub.h | 9 ++++----- drivers/staging/usbip/stub_dev.c | 2 +- drivers/staging/usbip/stub_main.c | 2 +- drivers/staging/usbip/stub_rx.c | 5 +++-- drivers/staging/usbip/stub_tx.c | 2 +- drivers/staging/usbip/usbip_common.c | 8 ++++---- drivers/staging/usbip/usbip_common.h | 16 ++++++++++------ drivers/staging/usbip/usbip_event.c | 1 + drivers/staging/usbip/vhci.h | 8 +++++++- drivers/staging/usbip/vhci_hcd.c | 6 +++++- drivers/staging/usbip/vhci_rx.c | 2 +- drivers/staging/usbip/vhci_sysfs.c | 5 +++-- drivers/staging/usbip/vhci_tx.c | 2 +- 13 files changed, 42 insertions(+), 26 deletions(-)
drivers/staging/usbip/stub.h +4 -5
@@ -17,13 +17,12 @@* USA. */ -#include <linux/kernel.h> #include <linux/list.h> -#include <linux/spinlock.h> #include <linux/slab.h> -#include <linux/string.h> -#include <linux/module.h> -#include <linux/net.h> +#include <linux/spinlock.h> +#include <linux/types.h> +#include <linux/usb.h> +#include <linux/wait.h> #define STUB_BUSID_OTHER 0#define STUB_BUSID_REMOV 1
drivers/staging/usbip/stub_dev.c +1 -1
@@ -17,7 +17,7 @@* USA. */ -#include <linux/slab.h> +#include <linux/device.h> #include <linux/kthread.h> #include "usbip_common.h"
drivers/staging/usbip/stub_main.c +1 -1
@@ -17,7 +17,7 @@* USA. */ -#include <linux/slab.h> +#include <linux/string.h> #include "usbip_common.h" #include "stub.h"
drivers/staging/usbip/stub_rx.c +3 -2
@@ -17,12 +17,13 @@* USA. */ -#include <linux/slab.h> +#include <asm/byteorder.h> #include <linux/kthread.h> +#include <linux/usb.h> +#include <linux/usb/hcd.h> #include "usbip_common.h" #include "stub.h" -#include <linux/usb/hcd.h> static int is_clear_halt_cmd (struct urb *urb) {
drivers/staging/usbip/stub_tx.c +1 -1
@@ -17,8 +17,8 @@* USA. */ -#include <linux/slab.h> #include <linux/kthread.h> +#include <linux/socket.h> #include "usbip_common.h" #include "stub.h"
drivers/staging/usbip/usbip_common.c +4 -4
@@ -17,12 +17,12 @@* USA. */ -#include <linux/kernel.h> +#include <asm/byteorder.h> #include <linux/file.h> -#include <linux/tcp.h> -#include <linux/in.h> -#include <linux/kthread.h> +#include <linux/fs.h> +#include <linux/kernel.h> #include <linux/slab.h> +#include <net/sock.h> #include "usbip_common.h"
drivers/staging/usbip/usbip_common.h +10 -6
@@ -17,14 +17,18 @@* USA. */ -#ifndef __VHCI_COMMON_H -#define __VHCI_COMMON_H +#ifndef __USBIP_COMMON_H +#define __USBIP_COMMON_H -#include <linux/version.h> #include <linux/compiler.h> +#include <linux/device.h> +#include <linux/interrupt.h> +#include <linux/net.h> +#include <linux/printk.h> +#include <linux/spinlock.h> +#include <linux/types.h> #include <linux/usb.h> -#include <asm/byteorder.h> -#include <net/sock.h> +#include <linux/wait.h> #define USBIP_VERSION "1.0.0"
@@ -366,4 +370,4 @@ void usbip_stop_eh(struct usbip_device *ud);void usbip_event_add (struct usbip_device *ud,unsigned long event);int usbip_event_happened (struct usbip_device *ud); -#endif +#endif /* __USBIP_COMMON_H */
drivers/staging/usbip/usbip_event.c +1 -0
@@ -18,6 +18,7 @@*/ #include <linux/kthread.h> +#include "usbip_common.h" static int event_handler (struct usbip_device *ud)
drivers/staging/usbip/vhci.h +7 -1
@@ -17,8 +17,14 @@* USA. */ -#include <linux/platform_device.h> +#include <linux/device.h> +#include <linux/list.h> +#include <linux/spinlock.h> +#include <linux/sysfs.h> +#include <linux/types.h> +#include <linux/usb.h> #include <linux/usb/hcd.h> +#include <linux/wait.h> struct vhci_device {struct usb_device *udev;
drivers/staging/usbip/vhci_hcd.c +5 -1
@@ -17,8 +17,12 @@* USA. */ -#include <linux/slab.h> +#include <linux/init.h> +#include <linux/kernel.h> #include <linux/kthread.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/slab.h> #include "usbip_common.h" #include "vhci.h"
drivers/staging/usbip/vhci_rx.c +1 -1
@@ -17,8 +17,8 @@* USA. */ -#include <linux/slab.h> #include <linux/kthread.h> +#include <linux/slab.h> #include "usbip_common.h" #include "vhci.h"
drivers/staging/usbip/vhci_sysfs.c +3 -2
@@ -17,11 +17,12 @@* USA. */ +#include <linux/kthread.h> +#include <linux/net.h> +#include "usbip_common.h" #include "vhci.h" -#include <linux/in.h> -/* TODO: refine locking ?*/ /* Sysfs entry to show port status */
drivers/staging/usbip/vhci_tx.c +1 -1
@@ -17,8 +17,8 @@* USA. */ -#include <linux/slab.h> #include <linux/kthread.h> +#include <linux/slab.h> #include "usbip_common.h" #include "vhci.h"
Signature · 1 line
1.7.5.1
PATCH v2 9/12 staging: usbip: edit Kconfig and rename CONFIG options
In reply to matt mooneyFrom: matt mooney <[email protected]>
Edit configuration option help text; rename CONFIG options to use a USBIP prefix and change COMMON to CORE in both the Kconfig and Makefile; edit the menu entries; and edit the driver descriptions.
--- drivers/staging/Makefile | 2 +- drivers/staging/usbip/Kconfig | 52 +++++++++++++++++---------------- drivers/staging/usbip/Makefile | 12 ++++---- drivers/staging/usbip/stub_main.c | 2 +- drivers/staging/usbip/usbip_common.c | 4 +- drivers/staging/usbip/vhci_hcd.c | 2 +- 6 files changed, 38 insertions(+), 36 deletions(-)
drivers/staging/Makefile +1 -1
@@ -12,7 +12,7 @@ obj-$(CONFIG_VIDEO_CX25821) += cx25821/ obj-$(CONFIG_VIDEO_TM6000) += tm6000/ obj-$(CONFIG_DVB_CXD2099) += cxd2099/ obj-$(CONFIG_LIRC_STAGING) += lirc/ -obj-$(CONFIG_USB_IP_COMMON) += usbip/ +obj-$(CONFIG_USBIP_CORE) += usbip/ obj-$(CONFIG_W35UND) += winbond/ obj-$(CONFIG_PRISM2_USB) += wlan-ng/ obj-$(CONFIG_ECHO) += echo/
drivers/staging/usbip/Kconfig +27 -25
70 lines not on this page · raw message
drivers/staging/usbip/Makefile +6 -6
@@ -1,10 +1,10 @@ -ccflags-$(CONFIG_USB_IP_DEBUG) := -DDEBUG +ccflags-$(CONFIG_USBIP_DEBUG) := -DDEBUG -obj-$(CONFIG_USB_IP_COMMON) += usbip_common_mod.o -usbip_common_mod-y := usbip_common.o usbip_event.o +obj-$(CONFIG_USBIP_CORE) += usbip-core.o +usbip-core-y := usbip_common.o usbip_event.o -obj-$(CONFIG_USB_IP_VHCI_HCD) += vhci-hcd.o +obj-$(CONFIG_USBIP_VHCI_HCD) += vhci-hcd.o vhci-hcd-y := vhci_sysfs.o vhci_tx.o vhci_rx.o vhci_hcd.o -obj-$(CONFIG_USB_IP_HOST) += usbip.o -usbip-y := stub_dev.o stub_main.o stub_rx.o stub_tx.o +obj-$(CONFIG_USBIP_HOST) += usbip-host.o +usbip-host-y := stub_dev.o stub_main.o stub_rx.o stub_tx.o
drivers/staging/usbip/stub_main.c +1 -1
@@ -23,7 +23,7 @@#include "stub.h" #define DRIVER_AUTHOR "Takahiro Hirofuchi" -#define DRIVER_DESC "Stub Driver for USB/IP" +#define DRIVER_DESC "USB/IP Host Driver"/* stub_priv is allocated from stub_priv_cache */ struct kmem_cache *stub_priv_cache;
drivers/staging/usbip/usbip_common.c +2 -2
@@ -27,9 +27,9 @@#include "usbip_common.h" #define DRIVER_AUTHOR "Takahiro Hirofuchi <[email protected]>" -#define DRIVER_DESC "USB/IP Common Driver" +#define DRIVER_DESC "USB/IP Core" -#ifdef CONFIG_USB_IP_DEBUG +#ifdef CONFIG_USBIP_DEBUG unsigned long usbip_debug_flag =0xffffffff ;#else unsigned long usbip_debug_flag;
drivers/staging/usbip/vhci_hcd.c +1 -1
@@ -28,7 +28,7 @@#include "vhci.h" #define DRIVER_AUTHOR "Takahiro Hirofuchi" -#define DRIVER_DESC "Virtual Host Controller Interface Driver for USB/IP" +#define DRIVER_DESC "USB/IP 'Virtual' Host Controller (VHCI) Driver"/* * TODO
Signature · 1 line
1.7.5.1
PATCH v3 9/12 staging: usbip: edit Kconfig and rename CONFIG options
In reply to matt mooneyFrom: matt mooney <[email protected]>
Change modules usbip_common_mod to usbip-core and usbip to usbip-host; edit configuration option help text; rename CONFIG options to use a USBIP prefix and change COMMON to CORE in both the Kconfig and Makefiles; edit the menu entries and the driver descriptions.
---
I actually change to module names in this patch now, and I had forgot to add that into the commit message.
drivers/staging/Makefile | 2 +- drivers/staging/usbip/Kconfig | 52 +++++++++++++++++---------------- drivers/staging/usbip/Makefile | 12 ++++---- drivers/staging/usbip/stub_main.c | 2 +- drivers/staging/usbip/usbip_common.c | 4 +- drivers/staging/usbip/vhci_hcd.c | 2 +- 6 files changed, 38 insertions(+), 36 deletions(-)
drivers/staging/Makefile +1 -1
@@ -12,7 +12,7 @@ obj-$(CONFIG_VIDEO_CX25821) += cx25821/ obj-$(CONFIG_VIDEO_TM6000) += tm6000/ obj-$(CONFIG_DVB_CXD2099) += cxd2099/ obj-$(CONFIG_LIRC_STAGING) += lirc/ -obj-$(CONFIG_USB_IP_COMMON) += usbip/ +obj-$(CONFIG_USBIP_CORE) += usbip/ obj-$(CONFIG_W35UND) += winbond/ obj-$(CONFIG_PRISM2_USB) += wlan-ng/ obj-$(CONFIG_ECHO) += echo/
drivers/staging/usbip/Kconfig +27 -25
70 lines not on this page · raw message
drivers/staging/usbip/Makefile +6 -6
@@ -1,10 +1,10 @@ -ccflags-$(CONFIG_USB_IP_DEBUG) := -DDEBUG +ccflags-$(CONFIG_USBIP_DEBUG) := -DDEBUG -obj-$(CONFIG_USB_IP_COMMON) += usbip_common_mod.o -usbip_common_mod-y := usbip_common.o usbip_event.o +obj-$(CONFIG_USBIP_CORE) += usbip-core.o +usbip-core-y := usbip_common.o usbip_event.o -obj-$(CONFIG_USB_IP_VHCI_HCD) += vhci-hcd.o +obj-$(CONFIG_USBIP_VHCI_HCD) += vhci-hcd.o vhci-hcd-y := vhci_sysfs.o vhci_tx.o vhci_rx.o vhci_hcd.o -obj-$(CONFIG_USB_IP_HOST) += usbip.o -usbip-y := stub_dev.o stub_main.o stub_rx.o stub_tx.o +obj-$(CONFIG_USBIP_HOST) += usbip-host.o +usbip-host-y := stub_dev.o stub_main.o stub_rx.o stub_tx.o
drivers/staging/usbip/stub_main.c +1 -1
@@ -23,7 +23,7 @@#include "stub.h" #define DRIVER_AUTHOR "Takahiro Hirofuchi" -#define DRIVER_DESC "Stub Driver for USB/IP" +#define DRIVER_DESC "USB/IP Host Driver"/* stub_priv is allocated from stub_priv_cache */ struct kmem_cache *stub_priv_cache;
drivers/staging/usbip/usbip_common.c +2 -2
@@ -27,9 +27,9 @@#include "usbip_common.h" #define DRIVER_AUTHOR "Takahiro Hirofuchi <[email protected]>" -#define DRIVER_DESC "USB/IP Common Driver" +#define DRIVER_DESC "USB/IP Core" -#ifdef CONFIG_USB_IP_DEBUG +#ifdef CONFIG_USBIP_DEBUG unsigned long usbip_debug_flag =0xffffffff ;#else unsigned long usbip_debug_flag;
drivers/staging/usbip/vhci_hcd.c +1 -1
@@ -28,7 +28,7 @@#include "vhci.h" #define DRIVER_AUTHOR "Takahiro Hirofuchi" -#define DRIVER_DESC "Virtual Host Controller Interface Driver for USB/IP" +#define DRIVER_DESC "USB/IP 'Virtual' Host Controller (VHCI) Driver"/* * TODO
Signature · 1 line
1.7.5.1
Re: [PATCH 09/12 v3] staging: usbip: edit Kconfig and rename CONFIG options
Quoted 11 lines · On Wed, May 11, 2011 at 10:17:08PM -0700, [email protected] wrote:
From: matt mooney <[email protected]>
Change modules
usbip_common_modto usbip-core and usbip to usbip-host; edit configuration option help text; rename CONFIG options to use a USBIP prefix and change COMMON to CORE in both theKconfigand Makefiles; edit the menu entries and the driver descriptions.Signed-off-by matt mooney <[email protected]>---
I actually change to module names in this patch now, and I had forgot to add that into the commit message.
I'm confused, there really isn't going to be 12 patches here in this new series, right?
Care to start numbering them over and send them in a new thread so I can figure out what to apply in what order?
thanks,
greg k-h
Re: [PATCH 09/12 v3] staging: usbip: edit Kconfig and rename CONFIG options
Quoted 16 lines · Greg KH
Quoted 11 lines · On Wed, May 11, 2011 at 10:17:08PM -0700, [email protected] wrote:
From: matt mooney <[email protected]>
Change modules
usbip_common_modto usbip-core and usbip to usbip-host; edit configuration option help text; rename CONFIG options to use a USBIP prefix and change COMMON to CORE in both theKconfigand Makefiles; edit the menu entries and the driver descriptions.Signed-off-by matt mooney <[email protected]>---
I actually change to module names in this patch now, and I had forgot to add that into the commit message.I'm confused, there really isn't going to be 12 patches here in this new series, right?
Quoted 7 lines
Care to start numbering them over and send them in a new thread so I can figure out what to apply in what order?
thanks,
greg k-h
Heh, sorry I didn't know what was easier for you.
Signature · 1 line
GPG-Key: 9AFE00EA
Quoted 3 lines · matt mooney
Hi Greg,
I am interested in hearing your thoughts on the userspace interface?
Hey Matt, if you're looking at usbip, could I interest you in fixing the protocol that it speaks? See the thread here:
http://marc.info/?l=linux-kernel&m2001863119269&w=2
I ran out of time to work on it, but I think the problems still remain.
Signature · 4 lines
Matthew Wilcox Intel Open Source Technology Centre "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Quoted 11 lines · Matthew Wilcox
Quoted 3 lines · matt mooney
Hi Greg,
I am interested in hearing your thoughts on the userspace interface?
Hey Matt, if you're looking at usbip, could I interest you in fixing the protocol that it speaks? See the thread here:
http://marc.info/?l=linux-kernel&m2001863119269&w=2
I ran out of time to work on it, but I think the problems still remain.
Hi Matthew,
I do plan on improving it where I can, so I will certainly take a look at that thread.
Thanks,
matt
Signature · 5 lines
GPG-Key: 9AFE00EA -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html