On 11/21/2016 11:48 PM, Nobuo Iwata wrote:
> usbip_get_device() method in usbip_host_driver_ops was not used. It is 
> modified as a function to find an exported device for new operations 
> 'connect' and 'disconnect'.
> 
> bind and unbind function are exported to reuse by new connect and 
> disconnect operation.

This doesn't look like a simple change to rename and reuse an unused
function. This patch does lot more and is changing the user interface.
Looks like instead of taking an integer value for device lookup, you
are changing it to char *.

Any reason why you have to change the user interface to go to char *busid?

I would like to see a good explanation why this user interface change is
necessary.

thanks,
-- Shuah

> 
> Here, connect and disconnect is NEW-3 and NEW-4 respactively in diagram 
> below.
> 
> EXISTING) - invites devices from application(vhci)-side
>          +------+                                 +------------------+
>  device--+ STUB |                                 | application/VHCI |
>          +------+                                 +------------------+
>  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(stb)-side
>          +------+                                 +------------------+
>  device--+ STUB |                                 | application/VHCI |
>          +------+                                 +------------------+
>                                               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.iw...@fujixerox.co.jp>
> ---
>  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))
>                       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;
> 


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America(Silicon Valley)
shuah...@samsung.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to