Re: [PATCH v14 02/10] usbip: exporting devices: modifications to host side libraries

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/26/2016 12:08 AM, Nobuo Iwata wrote:
> Modifications to host driver wrapper and its related operations (i.e. 
> bind/unbind).

Way too many changes in this one patch.

> 
> usbip_get_device() method was not used. The implementation of the 
> method searches a bound devices list by list index. The position in the 
> list is meaningless for any operations so it is assumed not to be used 
> in future.
> 
> For this patch set, a method to find a bound device in the bound 
> devices list by bus-id is needed. usbip_get_device() is re-implemented 
> as a function to find a bound device by bus-id.

This is not an accurate description. You are really changing look ups using
bus-id instead bus-num and whether usbip_get_device() is used or not is
irrelevant.

Please make changes to find bound device by bus-id in a separate patch.
You will have to change usbip_get_device() anyway because you are changing
.get_device() interface. Include exporting usbip_get_debice() in a separate
patch.

> 
> bind and unbind function are exported to reuse by new connect and 
> disconnect operation. Here, connect and disconnect is NEW-3 and NEW-4 
> respactively in diagram below.

Please don't include exporting bind_device() and unbind_device() and
changing their names in this patch. Send a separate patch for that.

It makes sense to move the functions you are exporting bind_device(),
unbind_device(), and usbip_get_device() to libsrc - usbip_common.c
might be a good choice.

The following isn't part of this patch, hence shouldn't be included in
the change log.

> 
> EXISTING) - invites devices from application(vhci)-side
>          +------+                               +------------------+
>  device--+ STUB |                               | application/VHCI |
>          +------+                               +------------------+
>          (server)                               (client)
>  1) # usbipd ... start daemon
>  = = =
>  2) # usbip list --local
>  3) # usbip bind
>                   <--- list bound devices ---  4) # usbip list --remote
>                   <--- import a device ------  5) # usbip attach
>  = = =
>                      X disconnected            6) # usbip detach
>  7) usbip unbind
> 
> NEW) - dedicates devices from device(stub)-side
>          +------+                               +------------------+
>  device--+ STUB |                               | application/VHCI |
>          +------+                               +------------------+
>          (client)                               (server)
>                                             1) # usbipa ... start daemon
>  = = =
>  2) # usbip list --local
>  3) # usbip connect    --- export a device ------>
>  = = =
>  4) # usbip disconnect --- un-export a device --->
> 
>  Bind and unbind are done in connect and disconnect internally.
> 
> Signed-off-by: Nobuo Iwata <nobuo.iwata@xxxxxxxxxxxxxxx>
> ---
>  tools/usb/usbip/libsrc/usbip_host_common.c | 6 ++----
>  tools/usb/usbip/libsrc/usbip_host_common.h | 8 ++++----
>  tools/usb/usbip/src/usbip.h                | 3 +++
>  tools/usb/usbip/src/usbip_bind.c           | 4 ++--
>  tools/usb/usbip/src/usbip_unbind.c         | 4 ++--
>  5 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c
> index 9d41522..6a98d6c 100644
> --- a/tools/usb/usbip/libsrc/usbip_host_common.c
> +++ b/tools/usb/usbip/libsrc/usbip_host_common.c
> @@ -256,17 +256,15 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd)
>  }
>  
>  struct usbip_exported_device *usbip_generic_get_device(
> -		struct usbip_host_driver *hdriver, int num)
> +		struct usbip_host_driver *hdriver, char *busid)
>  {
>  	struct list_head *i;
>  	struct usbip_exported_device *edev;
> -	int cnt = 0;
>  
>  	list_for_each(i, &hdriver->edev_list) {
>  		edev = list_entry(i, struct usbip_exported_device, node);
> -		if (num == cnt)
> +		if (!strncmp(busid, edev->udev.busid, SYSFS_BUS_ID_SIZE))

Use strlen(edev->udev.busid) instead of SYSFS_BUS_ID_SIZE

>  			return edev;
> -		cnt++;
>  	}
>  
>  	return NULL;
> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.h b/tools/usb/usbip/libsrc/usbip_host_common.h
> index a64b803..f9a9def 100644
> --- a/tools/usb/usbip/libsrc/usbip_host_common.h
> +++ b/tools/usb/usbip/libsrc/usbip_host_common.h
> @@ -38,7 +38,7 @@ struct usbip_host_driver_ops {
>  	void (*close)(struct usbip_host_driver *hdriver);
>  	int (*refresh_device_list)(struct usbip_host_driver *hdriver);
>  	struct usbip_exported_device * (*get_device)(
> -		struct usbip_host_driver *hdriver, int num);
> +		struct usbip_host_driver *hdriver, char *busid);
>  
>  	int (*read_device)(struct udev_device *sdev,
>  			   struct usbip_usb_device *dev);
> @@ -86,11 +86,11 @@ static inline int usbip_refresh_device_list(struct usbip_host_driver *hdriver)
>  }
>  
>  static inline struct usbip_exported_device *
> -usbip_get_device(struct usbip_host_driver *hdriver, int num)
> +usbip_get_device(struct usbip_host_driver *hdriver, char *busid)
>  {
>  	if (!hdriver->ops.get_device)
>  		return NULL;
> -	return hdriver->ops.get_device(hdriver, num);
> +	return hdriver->ops.get_device(hdriver, busid);
>  }
>  
>  /* Helper functions for implementing driver backend */
> @@ -99,6 +99,6 @@ void usbip_generic_driver_close(struct usbip_host_driver *hdriver);
>  int usbip_generic_refresh_device_list(struct usbip_host_driver *hdriver);
>  int usbip_export_device(struct usbip_exported_device *edev, int sockfd);
>  struct usbip_exported_device *usbip_generic_get_device(
> -		struct usbip_host_driver *hdriver, int num);
> +		struct usbip_host_driver *hdriver, char *busid);
>  
>  #endif /* __USBIP_HOST_COMMON_H */
> diff --git a/tools/usb/usbip/src/usbip.h b/tools/usb/usbip/src/usbip.h
> index 84fe66a..c296910 100644
> --- a/tools/usb/usbip/src/usbip.h
> +++ b/tools/usb/usbip/src/usbip.h
> @@ -37,4 +37,7 @@ void usbip_list_usage(void);
>  void usbip_bind_usage(void);
>  void usbip_unbind_usage(void);
>  
> +int usbip_bind_device(char *busid);
> +int usbip_unbind_device(char *busid);
> +
>  #endif /* __USBIP_H */
> diff --git a/tools/usb/usbip/src/usbip_bind.c b/tools/usb/usbip/src/usbip_bind.c
> index fa46141..2401745 100644
> --- a/tools/usb/usbip/src/usbip_bind.c
> +++ b/tools/usb/usbip/src/usbip_bind.c
> @@ -139,7 +139,7 @@ static int unbind_other(char *busid)
>  	return status;
>  }
>  
> -static int bind_device(char *busid)
> +int usbip_bind_device(char *busid)
>  {
>  	int rc;
>  	struct udev *udev;
> @@ -200,7 +200,7 @@ int usbip_bind(int argc, char *argv[])
>  
>  		switch (opt) {
>  		case 'b':
> -			ret = bind_device(optarg);
> +			ret = usbip_bind_device(optarg);
>  			goto out;
>  		default:
>  			goto err_out;
> diff --git a/tools/usb/usbip/src/usbip_unbind.c b/tools/usb/usbip/src/usbip_unbind.c
> index a4a496c..324d986 100644
> --- a/tools/usb/usbip/src/usbip_unbind.c
> +++ b/tools/usb/usbip/src/usbip_unbind.c
> @@ -39,7 +39,7 @@ void usbip_unbind_usage(void)
>  	printf("usage: %s", usbip_unbind_usage_string);
>  }
>  
> -static int unbind_device(char *busid)
> +int usbip_unbind_device(char *busid)
>  {
>  	char bus_type[] = "usb";
>  	int rc, ret = -1;
> @@ -127,7 +127,7 @@ int usbip_unbind(int argc, char *argv[])
>  
>  		switch (opt) {
>  		case 'b':
> -			ret = unbind_device(optarg);
> +			ret = usbip_unbind_device(optarg);
>  			goto out;
>  		default:
>  			goto err_out;
> 

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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux