On Tue, 2016-05-24 at 08:52 -0500, Bryant G. Ly wrote: > From: bgly <bgly@xxxxxxxxxx> > > This initial commit contains WIP of the IBM VSCSI Target Fabric > Module. It currently supports read/writes, and I have tested > the ability to create a file backstore with the driver and install > RHEL VIA NIM and then boot up the partition via filio backstore > through the driver. Only trivial notes: Maybe try checkpatch with the --strict option and see if any of the additional messages are important to you. > diff --git a/MAINTAINERS b/MAINTAINERS [] > @@ -5381,6 +5381,16 @@ S: Supported > F: drivers/scsi/ibmvscsi/ibmvscsi* > F: drivers/scsi/ibmvscsi/viosrp.h > > +IBM Power Virtual SCSI Device Target Driver > +M: Bryant G. Ly <bryantly@xxxxxxxxxxxxxxxxxx> > +L: linux-scsi@xxxxxxxxxxxxxxx > +L: target-devel@xxxxxxxxxxxxxxx > +S: Supported > +F: drivers/scsi/ibmvscsi/ibmvscsis.c > +F: drivers/scsi/ibmvscsi/ibmvscsis.h > +F: drivers/scsi/libsrp.h > +F: drivers/scsi/libsrp.c Please use a tab character consistently after the : > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig [] > @@ -847,6 +847,20 @@ config SCSI_IBMVSCSI > To compile this driver as a module, choose M here: the > module will be called ibmvscsi. > > +config SCSI_IBMVSCSIS > + tristate "IBM Virtual SCSI Server support" > + depends on PPC_PSERIES && SCSI_SRP && TARGET_CORE > + help > + This is the IBM POWER Virtual SCSI Target Server > + > + The userspace component needed to initialize the driver and > + documentation can be found: here too. > + > + https://github.com/powervm/ibmvscsis > + > + To compile this driver as a module, choose M here: the > + module will be called ibmvstgt. > + [] > diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile [] > @@ -127,7 +127,9 @@ obj-$(CONFIG_SCSI_LASI700) += 53c700.o lasi700.o > obj-$(CONFIG_SCSI_SNI_53C710) += 53c700.o sni_53c710.o > obj-$(CONFIG_SCSI_NSP32) += nsp32.o > obj-$(CONFIG_SCSI_IPR) += ipr.o > +obj-$(CONFIG_SCSI_SRP) += libsrp.o > obj-$(CONFIG_SCSI_IBMVSCSI) += ibmvscsi/ > +obj-$(CONFIG_SCSI_IBMVSCSIS) += ibmvscsi/ and here > diff --git a/drivers/scsi/ibmvscsi/ibmvscsis.c b/drivers/scsi/ibmvscsi/ibmvscsis.c [] > +static int ibmvscsis_probe(struct vio_dev *vdev, > + const struct vio_device_id *id); [...] It might be nice to rearrange the code to avoid these forward function declarations. > +static ssize_t ibmvscsis_tpg_enable_store(struct config_item *item, > + const char *page, size_t count) > +{ [] > > + ret = kstrtoul(page, 0, &tmp); > + if (ret < 0) { > + pr_err("Unable to extract ibmvscsis_tpg_store_enable\n"); > + return -EINVAL; > + } It might be nicer to add: #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt before any #include to output all the logging messages with a standardized prefix. Then all the other logging with an embedded prefix can have the embedded prefix removed too. > + > + if ((tmp != 0) && (tmp != 1)) { > + pr_err("Illegal value for ibmvscsis_tpg_store_enable: %lu\n", > + tmp); > + return -EINVAL; > + } > + > + if (tmp == 1) > + tport->enabled = true; > + else > + tport->enabled = false; tport->enabled = tmp; -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
