Hello, > > Bind and unbind are done in connect and disconnect internally. > > Please be more precise here and mention that those operation are done > for a stub driver only and for vudc you need to bind gadget manually > using usb gadget subsystem before using connect function.
> > + int bind = 1; > > Shouldn't this parameter be exposed to cmd line among remote, busid and > device? I decided to separate bind from connect: not to bind in connect operation. Regarding both stub and vudc, I think it will be simpler and easier to understand for users. To do so, there's no need to export bind/unbind in previous patch. Thank you for your advice, Nobuo Iwata // > -----Original Message----- > From: Krzysztof Opasiak [mailto:k.opas...@samsung.com] > Sent: Thursday, January 12, 2017 9:44 PM > To: fx IWATA NOBUO <nobuo.iw...@fujixerox.co.jp>; > valentina.mane...@gmail.com; sh...@kernel.org; > gre...@linuxfoundation.org; linux-usb@vger.kernel.org > Cc: fx MICHIMURA TADAO <michimura.ta...@fujixerox.co.jp> > Subject: Re: [PATCH v14 03/10] usbip: exporting devices: new connect > operation > > > Hi, > > > On 12/26/2016 08:08 AM, Nobuo Iwata wrote: > > Implementation of new connect operation. This is linked as a part of > > usbip command. With this patch, usbip command has following > operations. > > > > bind > > unbind > > list (local/remote) > > attach > > detach > > port > > connect ... this patch > > > > In device side node, this binds a device internally, sends an export > > request and receives export response. The definition of the request and > > response are defined in original code: tools/usb/usbip/usbip_network.h > > but was not used. They are corresponding to NEW-3 in following > diagram. > > > > 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. > > Please be more precise here and mention that those operation are done > for a stub driver only and for vudc you need to bind gadget manually > using usb gadget subsystem before using connect function. > > > > > Signed-off-by: Nobuo Iwata <nobuo.iw...@fujixerox.co.jp> > > --- > > tools/usb/usbip/src/Makefile.am | 3 +- > > tools/usb/usbip/src/usbip.c | 7 + > > tools/usb/usbip/src/usbip.h | 3 + > > tools/usb/usbip/src/usbip_connect.c | 212 > ++++++++++++++++++++++++++++ > > 4 files changed, 224 insertions(+), 1 deletion(-) > > > > diff --git a/tools/usb/usbip/src/Makefile.am > b/tools/usb/usbip/src/Makefile.am > > index e81a4eb..0947476 100644 > > --- a/tools/usb/usbip/src/Makefile.am > > +++ b/tools/usb/usbip/src/Makefile.am > > @@ -6,6 +6,7 @@ sbin_PROGRAMS := usbip usbipd > > > > usbip_SOURCES := usbip.h utils.h usbip.c utils.c usbip_network.c \ > > usbip_attach.c usbip_detach.c usbip_list.c \ > > - usbip_bind.c usbip_unbind.c usbip_port.c > > + usbip_bind.c usbip_unbind.c usbip_port.c \ > > + usbip_connect.c > > > > usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c > > diff --git a/tools/usb/usbip/src/usbip.c > b/tools/usb/usbip/src/usbip.c > > index d7599d9..ad2a259 100644 > > --- a/tools/usb/usbip/src/usbip.c > > +++ b/tools/usb/usbip/src/usbip.c > > @@ -4,6 +4,7 @@ > > * > > * Copyright (C) 2011 matt mooney <m...@muteddisk.com> > > * 2005-2007 Takahiro Hirofuchi > > + * Copyright (C) 2015-2016 Nobuo Iwata > <nobuo.iw...@fujixerox.co.jp> > > * > > * This program is free software: you can redistribute it and/or modify > > * it under the terms of the GNU General Public License as published > by > > @@ -76,6 +77,12 @@ static const struct command cmds[] = { > > .usage = usbip_detach_usage > > }, > > { > > + .name = "connect", > > + .fn = usbip_connect, > > + .help = "Connect a USB device to a remote computer", > > + .usage = usbip_connect_usage > > + }, > > + { > > .name = "list", > > .fn = usbip_list, > > .help = "List exportable or local USB devices", > > diff --git a/tools/usb/usbip/src/usbip.h > b/tools/usb/usbip/src/usbip.h > > index c296910..3c22c27 100644 > > --- a/tools/usb/usbip/src/usbip.h > > +++ b/tools/usb/usbip/src/usbip.h > > @@ -1,6 +1,7 @@ > > /* > > * Copyright (C) 2011 matt mooney <m...@muteddisk.com> > > * 2005-2007 Takahiro Hirofuchi > > + * Copyright (C) 2015-2016 Nobuo Iwata > <nobuo.iw...@fujixerox.co.jp> > > * > > * This program is free software: you can redistribute it and/or modify > > * it under the terms of the GNU General Public License as published > by > > @@ -30,12 +31,14 @@ int usbip_list(int argc, char *argv[]); > > int usbip_bind(int argc, char *argv[]); > > int usbip_unbind(int argc, char *argv[]); > > int usbip_port_show(int argc, char *argv[]); > > +int usbip_connect(int argc, char *argv[]); > > > > void usbip_attach_usage(void); > > void usbip_detach_usage(void); > > void usbip_list_usage(void); > > void usbip_bind_usage(void); > > void usbip_unbind_usage(void); > > +void usbip_connect_usage(void); > > > > int usbip_bind_device(char *busid); > > int usbip_unbind_device(char *busid); > > diff --git a/tools/usb/usbip/src/usbip_connect.c > b/tools/usb/usbip/src/usbip_connect.c > > new file mode 100644 > > index 0000000..73668db2 > > --- /dev/null > > +++ b/tools/usb/usbip/src/usbip_connect.c > > @@ -0,0 +1,212 @@ > > +/* > > + * Copyright (C) 2011 matt mooney <m...@muteddisk.com> > > + * 2005-2007 Takahiro Hirofuchi > > + * Copyright (C) 2015-2016 Nobuo Iwata > <nobuo.iw...@fujixerox.co.jp> > > + * > > + * This program is free software: you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published > by > > + * the Free Software Foundation, either version 2 of the License, or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty > of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program. If not, see > <http://www.gnu.org/licenses/>. > > + */ > > + > > +#include <sys/stat.h> > > + > > +#include <limits.h> > > +#include <stdint.h> > > +#include <stdio.h> > > +#include <string.h> > > + > > +#include <getopt.h> > > +#include <unistd.h> > > + > > +#include "usbip_host_driver.h" > > +#include "usbip_host_common.h" > > +#include "usbip_device_driver.h" > > +#include "usbip_common.h" > > +#include "usbip_network.h" > > +#include "usbip.h" > > + > > +static struct usbip_host_driver *driver = &host_driver; > > + > > +static const char usbip_connect_usage_string[] = > > + "usbip connect <args>\n" > > + " -r, --remote=<host> Address of a remote computer\n" > > + " -b, --busid=<busid> Bus ID of a device to be > connected\n" > > + " -d, --device Run with an alternate driver, e.g. > vUDC\n"; > > + > > +void usbip_connect_usage(void) > > +{ > > + printf("usage: %s", usbip_connect_usage_string); > > +} > > + > > +static int send_export_device(int sockfd, struct usbip_usb_device > *udev) > > +{ > > + int rc; > > + struct op_export_request request; > > + uint16_t code = OP_REP_EXPORT; > > + > > + /* send a request */ > > + rc = usbip_net_send_op_common(sockfd, OP_REQ_EXPORT, 0); > > + if (rc < 0) { > > + err("send op_common"); > > + return -1; > > + } > > + > > + memset(&request, 0, sizeof(request)); > > + memcpy(&request.udev, udev, sizeof(request.udev)); > > + > > + PACK_OP_EXPORT_REQUEST(0, &request); > > + > > + rc = usbip_net_send(sockfd, (void *) &request, sizeof(request)); > > + if (rc < 0) { > > + err("send op_export_request"); > > + return -1; > > + } > > + > > + /* receive a reply */ > > + rc = usbip_net_recv_op_common(sockfd, &code); > > + if (rc < 0) { > > + err("recv op_common"); > > + return -1; > > + } > > + > > + return 0; > > +} > > + > > +static int export_device(char *busid, int sockfd) > > +{ > > + int rc; > > + struct usbip_exported_device *edev; > > + > > + rc = usbip_driver_open(driver); > > + if (rc) { > > + err("open driver"); > > + goto err_out; > > + } > > + > > + rc = usbip_refresh_device_list(driver); > > + if (rc < 0) { > > + err("could not refresh device list"); > > + goto err_driver_close; > > + } > > + > > + edev = usbip_get_device(driver, busid); > > + if (edev == NULL) { > > + err("find device"); > > + goto err_driver_close; > > + } > > + > > + rc = send_export_device(sockfd, &edev->udev); > > + if (rc < 0) { > > + err("send export"); > > + goto err_driver_close; > > + } > > + > > + rc = usbip_export_device(edev, sockfd); > > + if (rc < 0) { > > + err("export device"); > > + goto err_driver_close; > > + } > > + > > + usbip_driver_close(driver); > > + > > + return 0; > > + > > +err_driver_close: > > + usbip_driver_close(driver); > > +err_out: > > + return -1; > > +} > > + > > +static int connect_device(char *host, char *busid, int bind) > > +{ > > + int sockfd; > > + int rc; > > + > > + if (bind) { > > + rc = usbip_bind_device(busid); > > + if (rc) { > > + err("bind"); > > + goto err_out; > > + } > > + } > > + > > + sockfd = usbip_net_tcp_connect(host, usbip_port_string); > > + if (sockfd < 0) { > > + err("tcp connect"); > > + goto err_unbind_device; > > + } > > + > > + rc = export_device(busid, sockfd); > > + if (rc < 0) { > > + err("export"); > > + goto err_close_conn; > > + } > > + > > + close(sockfd); > > + > > + return 0; > > +err_close_conn: > > + close(sockfd); > > +err_unbind_device: > > + if (bind) > > + usbip_unbind_device(busid); > > +err_out: > > + return -1; > > +} > > + > > +int usbip_connect(int argc, char *argv[]) > > +{ > > + static const struct option opts[] = { > > + { "remote", required_argument, NULL, 'r' }, > > + { "busid", required_argument, NULL, 'b' }, > > + { "device", no_argument, NULL, 'd' }, > > + { NULL, 0, NULL, 0 } > > + }; > > + char *host = NULL; > > + char *busid = NULL; > > + int opt; > > + int bind = 1; > > Shouldn't this parameter be exposed to cmd line among remote, busid and > device? > > > + int ret = -1; > > + > > + for (;;) { > > + opt = getopt_long(argc, argv, "r:b:d", opts, NULL); > > + > > + if (opt == -1) > > + break; > > + > > + switch (opt) { > > + case 'r': > > + host = optarg; > > + break; > > + case 'b': > > + busid = optarg; > > + break; > > + case 'd': > > + driver = &device_driver; > > + bind = 0; > > + break; > > + default: > > + goto err_out; > > + } > > + } > > + > > + if (!host || !busid) > > + goto err_out; > > + > > + ret = connect_device(host, busid, bind); > > + goto out; > > + > > +err_out: > > + usbip_connect_usage(); > > +out: > > + return ret; > > +} > > > > This looks far better than previous edition. Thank you! > Just a couple of minor issues and should be ok. > > Cheers, > -- > Krzysztof Opasiak > Samsung R&D Institute Poland > Samsung Electronics -- 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
