On 12/06/2011 08:40 AM, Nicholas A. Bellinger wrote:
Hi Sebastian,
Hi Nicholas,
So this looks like a reasonable first step to get a control plane up for /sys/kernel/config/target/uasp/. I have not at the USB parts beyond the initial patch so I can't really comment there, but thanks for using tcm_mod_builder.py to generate your initial skeleton.
Thanks for providing the skeleton building script :) I don't have more yet. Storage on USB should provide two configurations: BOT and UASP. BOT is currently served by drivers/usb /gadget/mass_storage.c. The configuration (modprobe parameters) and protocol handling is done there. Since UASP brings its own configuration I think that it will be too complicated to merge both code basis (not to mention the way both handling things internally). So I decided first to integrate UASP with target and then worry about BOT.
Just a minor few comments below on the target-related pieces below, and a few comments to keep in mind for he main I/O path with struct uasp_cmd->se_cmd descriptor usage.diff --git a/drivers/target/Kconfig b/drivers/target/Kconfig index e66fcc7..64d3204 100644 --- a/drivers/target/Kconfig +++ b/drivers/target/Kconfig @@ -50,3 +50,4 @@ source "drivers/target/tcm_qla2xxx/Kconfig" source "drivers/target/tcm_vhost/Kconfig" endif +source "drivers/target/uasp/Kconfig" diff --git a/drivers/target/Makefile b/drivers/target/Makefile index 1945dba..b1135d5 100644 --- a/drivers/target/Makefile +++ b/drivers/target/Makefile @@ -28,3 +28,4 @@ obj-$(CONFIG_TCM_FC) += tcm_fc/ obj-$(CONFIG_ISCSI_TARGET) += iscsi/ obj-$(CONFIG_TCM_QLA2XXX) += tcm_qla2xxx/ obj-$(CONFIG_TCM_VHOST) += tcm_vhost/ +obj-$(CONFIG_TARGET_UASP) += uasp/ diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index f7cb64f..127496a 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -162,6 +162,13 @@ static struct config_group *target_core_register_fabric( " tcm_loop.ko: %d\n", ret); return ERR_PTR(-EINVAL); } + } else if (!strncmp(name, "uasp", 4)) { + ret = request_module("tcm_uasp"); + if (ret< 0) { + pr_err("request_module() failed for" + " tcm_loop.ko: %d\n", ret); + return ERR_PTR(-EINVAL); + } } tf = target_core_get_fabric(name);So making the request_module() calls was an original feature of v3.x target core code, but this is not strictly required for normal usage after loading the module, or with rtslib /var/target/spec/uasp.spec usage.
I see.
As we are trying to avoid extra request_module() usage in target-core, please go ahead and drop this part for now..
Okay.
diff --git a/drivers/target/uasp/uasp_configfs.c b/drivers/target/uasp/uasp_configfs.c new file mode 100644 index 0000000..7f6b280 --- /dev/null +++ b/drivers/target/uasp/uasp_configfs.c @@ -0,0 +1,333 @@ +#include<linux/module.h> +#include<linux/moduleparam.h> +#include<linux/version.h> +#include<generated/utsrelease.h> +#include<linux/utsname.h> +#include<linux/init.h> +#include<linux/slab.h> +#include<linux/kthread.h> +#include<linux/types.h> +#include<linux/string.h> +#include<linux/configfs.h> +#include<linux/ctype.h> +#include<asm/unaligned.h> + +#include<target/target_core_base.h> +#include<target/target_core_fabric.h> +#include<target/target_core_fabric_configfs.h> +#include<target/target_core_configfs.h> +#include<target/configfs_macros.h> + +#include "uasp_base.h" +#include "uasp_fabric.h" +#include "gadget_ops.h" + +/* Local pointer to allocated TCM configfs fabric module */ +struct target_fabric_configfs *uasp_fabric_configfs; + +static const char *uasp_check_wwn(const char *name) +{ + const char *n; + unsigned int len; + + n = strstr(name, "naa."); + if (!n) + return NULL; + n += 4; + len = strlen(n); + if (len == 0 || len> UASP_NAMELEN - 1) + return NULL; + return n; +} +Note this is currently using naa. prefixed targetnames for UASP endpoints, as loopback does with SAS proto_id port emulation..
Not sure if this is the right thing to do. I've seen it in loop and vhost that once "naa." has been found, the protocol was set to SAS. I think I have to use SAS so I am using the same prefix here.
+int uasp_register_configfs(void) +{ + struct target_fabric_configfs *fabric; + int ret; + + printk(KERN_INFO "UASP fabric module %s on %s/%s" + " on "UTS_RELEASE"\n",UASP_VERSION, utsname()->sysname, + utsname()->machine); + /* + * Register the top level struct config_item_type with TCM core + */ + fabric = target_fabric_configfs_init(THIS_MODULE, "uasp"); + if (!(fabric)) { + printk(KERN_ERR "target_fabric_configfs_init() failed\n"); + return -ENOMEM; + }This should be checking using IS_ERR() with fabric, which is a current bug in tcm_mod_builder.py
Okay, will fix in both.
<SNIP> So for the primary incoming I/O path, you'll want to be using the new target_submit_cmd() call that we've been working on in lio-core.git/qla_tgt-3.3 recently, as this will simplify incoming I/O + exception handling for struct uasp_cmd from processing context usage.
Okay, will look at that.
Also, do you expect this new driver to pass pre-populated scatterlists, or to leave se_cmd->t_data_sg allocation up to target-core..? Depending on the expected usage this will look slightly different from the current usage in tcm_loop/tcm_vhost that both pass pre-populated scatterlists from Linux/SCSI.
USB has IN and OUT packets. OUT packets are sent by the HOST (received by the UDC or device and IN packets are received by the host and ..... OUT and IN is always seen / described from HOST's point of view. That means USB-OUT packet for the device is "receive-data".
I have to queue an USB-OUT packet for the endpoint that receives the command. For that I have to allocate memory. That means cdb + sense can be part of that. For both I don't know the exact size. For cdb I allocate maxpacket size (that is 512 bytes for high speed and 1024 bytes for super speed) memory and I receive the size, the host sent to me. For DATA-IN/OUT transfers I think it would be easier to let the core allocate the memory. After all I don't know the exact size of the transfer. We don't have scatterlist support on the gadget framework yet but Felipe is looking into. So sgl is perfect, will provide a buffer fallback and people may udpate their drivers if it is too slow :)
Aside from that, let me know if you have questions while filling in the write_pending and response (queue_data_in + queue_status) fabric callbacks.
Thanks for that. Once I have something I try to come up with a sane question :)
Thanks! --nab
Sebastian -- 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
