Message ID | 1432711182-17530-10-git-send-email-nmusini@cisco.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 05/27/2015 10:19 AM, Narsimhulu Musini wrote: > Kconfig for kbuild > Makefile to build snic module > > Updated MAINTAINERS file > > Signed-off-by: Narsimhulu Musini <nmusini@cisco.com> > Signed-off-by: Sesidhar Baddela <sebaddel@cisco.com> > --- > * v3 > - Added additional config section (CONFIG_SNIC_DEBUG_FS) for enabling debugging > functionality. > > * v2 > - Added compile time flags for debugfs dependent functionality. > > MAINTAINERS | 7 +++++++ > drivers/scsi/Kconfig | 17 +++++++++++++++++ > drivers/scsi/Makefile | 1 + > drivers/scsi/snic/Makefile | 21 +++++++++++++++++++++ > 4 files changed, 46 insertions(+) > create mode 100644 drivers/scsi/snic/Makefile > > diff --git a/MAINTAINERS b/MAINTAINERS > index 2a97e05..368fb76 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2536,6 +2536,13 @@ L: linux-scsi@vger.kernel.org > S: Supported > F: drivers/scsi/fnic/ > > +CISCO SCSI HBA DRIVER > +M: Narsimhulu Musini <nmusini@cisco.com> > +M: Sesidhar Baddela <sebaddel@cisco.com> > +L: linux-scsi@vger.kernel.org > +S: Supported > +F: drivers/scsi/snic/ > + > CMPC ACPI DRIVER > M: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com> > M: Daniel Oliveira Nascimento <don@syst.com.br> > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig > index 9c92f41..8baab3f 100644 > --- a/drivers/scsi/Kconfig > +++ b/drivers/scsi/Kconfig > @@ -634,6 +634,23 @@ config FCOE_FNIC > <file:Documentation/scsi/scsi.txt>. > The module will be called fnic. > > +config SCSI_SNIC > + tristate "Cisco SNIC Driver" > + depends on PCI && SCSI && X86_64 > + help > + This is support for the Cisco PCI-Express SCSI HBA. > + > + To compile this driver as a module, choose M here and read > + <file:Documentation/scsi/scsi.txt>. > + The module will be called snic. > + > +config SCSI_SNIC_DEBUG_FS > + bool "Cisco SNIC Driver Debugfs Support" > + depends on SCSI_SNIC && DEBUG_FS > + help > + This enables to list debugging information from SNIC Driver > + available via debugfs file system > + > config SCSI_DMX3191D > tristate "DMX3191D SCSI support" > depends on PCI && SCSI > diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile > index 58158f1..f643942 100644 > --- a/drivers/scsi/Makefile > +++ b/drivers/scsi/Makefile > @@ -39,6 +39,7 @@ obj-$(CONFIG_LIBFC) += libfc/ > obj-$(CONFIG_LIBFCOE) += fcoe/ > obj-$(CONFIG_FCOE) += fcoe/ > obj-$(CONFIG_FCOE_FNIC) += fnic/ > +obj-$(CONFIG_SCSI_SNIC) += snic/ > obj-$(CONFIG_SCSI_BNX2X_FCOE) += libfc/ fcoe/ bnx2fc/ > obj-$(CONFIG_ISCSI_TCP) += libiscsi.o libiscsi_tcp.o iscsi_tcp.o > obj-$(CONFIG_INFINIBAND_ISER) += libiscsi.o > diff --git a/drivers/scsi/snic/Makefile b/drivers/scsi/snic/Makefile > new file mode 100644 > index 0000000..572102a > --- /dev/null > +++ b/drivers/scsi/snic/Makefile > @@ -0,0 +1,21 @@ > +obj-$(CONFIG_SCSI_SNIC) += snic.o > + > +snic-y := \ > + snic_attrs.o \ > + snic_main.o \ > + snic_res.o \ > + snic_isr.o \ > + snic_ctl.o \ > + snic_io.o \ > + snic_scsi.o \ > + snic_disc.o \ > + vnic_cq.o \ > + vnic_intr.o \ > + vnic_dev.o \ > + vnic_wq.o > + > +ifeq ($(CONFIG_SCSI_SNIC_DEBUG_FS), y) > +ccflags-y += -DSNIC_DEBUG_FS Why do you need an extra define here just use CONFIG_SCSI_SNIC_DEBUG_FS in source code directly > +snic-y += snic_debugfs.o \ > + snic_trc.o > +endif > snic-$(CONFIG_SCSI_SNIC_DEBUG_FS) += snic_debugfs.o You do not the ifeq () thing at all Cheers Boaz -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Boaz, Thank you for reviewing patch. Please find responses inline. On 27/05/15 2:47 pm, "Boaz Harrosh" <boaz@plexistor.com> wrote: >On 05/27/2015 10:19 AM, Narsimhulu Musini wrote: >> Kconfig for kbuild >> Makefile to build snic module >> >> Updated MAINTAINERS file >> >> Signed-off-by: Narsimhulu Musini <nmusini@cisco.com> >> Signed-off-by: Sesidhar Baddela <sebaddel@cisco.com> >> --- >> * v3 >> - Added additional config section (CONFIG_SNIC_DEBUG_FS) for enabling >>debugging >> functionality. >> >> * v2 >> - Added compile time flags for debugfs dependent functionality. >> >> MAINTAINERS | 7 +++++++ >> drivers/scsi/Kconfig | 17 +++++++++++++++++ >> drivers/scsi/Makefile | 1 + >> drivers/scsi/snic/Makefile | 21 +++++++++++++++++++++ >> 4 files changed, 46 insertions(+) >> create mode 100644 drivers/scsi/snic/Makefile >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 2a97e05..368fb76 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -2536,6 +2536,13 @@ L: linux-scsi@vger.kernel.org >> S: Supported >> F: drivers/scsi/fnic/ >> >> +CISCO SCSI HBA DRIVER >> +M: Narsimhulu Musini <nmusini@cisco.com> >> +M: Sesidhar Baddela <sebaddel@cisco.com> >> +L: linux-scsi@vger.kernel.org >> +S: Supported >> +F: drivers/scsi/snic/ >> + >> CMPC ACPI DRIVER >> M: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com> >> M: Daniel Oliveira Nascimento <don@syst.com.br> >> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig >> index 9c92f41..8baab3f 100644 >> --- a/drivers/scsi/Kconfig >> +++ b/drivers/scsi/Kconfig >> @@ -634,6 +634,23 @@ config FCOE_FNIC >> <file:Documentation/scsi/scsi.txt>. >> The module will be called fnic. >> >> +config SCSI_SNIC >> + tristate "Cisco SNIC Driver" >> + depends on PCI && SCSI && X86_64 >> + help >> + This is support for the Cisco PCI-Express SCSI HBA. >> + >> + To compile this driver as a module, choose M here and read >> + <file:Documentation/scsi/scsi.txt>. >> + The module will be called snic. >> + >> +config SCSI_SNIC_DEBUG_FS >> + bool "Cisco SNIC Driver Debugfs Support" >> + depends on SCSI_SNIC && DEBUG_FS >> + help >> + This enables to list debugging information from SNIC Driver >> + available via debugfs file system >> + >> config SCSI_DMX3191D >> tristate "DMX3191D SCSI support" >> depends on PCI && SCSI >> diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile >> index 58158f1..f643942 100644 >> --- a/drivers/scsi/Makefile >> +++ b/drivers/scsi/Makefile >> @@ -39,6 +39,7 @@ obj-$(CONFIG_LIBFC) += libfc/ >> obj-$(CONFIG_LIBFCOE) += fcoe/ >> obj-$(CONFIG_FCOE) += fcoe/ >> obj-$(CONFIG_FCOE_FNIC) += fnic/ >> +obj-$(CONFIG_SCSI_SNIC) += snic/ >> obj-$(CONFIG_SCSI_BNX2X_FCOE) += libfc/ fcoe/ bnx2fc/ >> obj-$(CONFIG_ISCSI_TCP) += libiscsi.o libiscsi_tcp.o iscsi_tcp.o >> obj-$(CONFIG_INFINIBAND_ISER) += libiscsi.o >> diff --git a/drivers/scsi/snic/Makefile b/drivers/scsi/snic/Makefile >> new file mode 100644 >> index 0000000..572102a >> --- /dev/null >> +++ b/drivers/scsi/snic/Makefile >> @@ -0,0 +1,21 @@ >> +obj-$(CONFIG_SCSI_SNIC) += snic.o >> + >> +snic-y := \ >> + snic_attrs.o \ >> + snic_main.o \ >> + snic_res.o \ >> + snic_isr.o \ >> + snic_ctl.o \ >> + snic_io.o \ >> + snic_scsi.o \ >> + snic_disc.o \ >> + vnic_cq.o \ >> + vnic_intr.o \ >> + vnic_dev.o \ >> + vnic_wq.o >> + >> +ifeq ($(CONFIG_SCSI_SNIC_DEBUG_FS), y) >> +ccflags-y += -DSNIC_DEBUG_FS > >Why do you need an extra define here just use >CONFIG_SCSI_SNIC_DEBUG_FS in source code directly Agree, I just want to use a shorter macro in the source. > >> +snic-y += snic_debugfs.o \ >> + snic_trc.o >> +endif >> > >snic-$(CONFIG_SCSI_SNIC_DEBUG_FS) += snic_debugfs.o If CONFIG_SCSI_SNIC_DEBUGFS is not set, then it leaves a build variable ³snic-" in build system. ifeq() avoids such thing. > >You do not the ifeq () thing at all > >Cheers >Boaz > Thanks simha > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/27/2015 01:02 PM, Narsimhulu Musini (nmusini) wrote: <> >>> +ifeq ($(CONFIG_SCSI_SNIC_DEBUG_FS), y) >>> +ccflags-y += -DSNIC_DEBUG_FS >> >> Why do you need an extra define here just use >> CONFIG_SCSI_SNIC_DEBUG_FS in source code directly > Agree, I just want to use a shorter macro in the source. Don't do this. It is a convention that if a programmer sees a CONFIG_XXX he knows that this is settable by a Kconfig. By making it shorter the programmer will think that this is dead code because it is not set anywhere. >> >>> +snic-y += snic_debugfs.o \ >>> + snic_trc.o >>> +endif >>> >> >> snic-$(CONFIG_SCSI_SNIC_DEBUG_FS) += snic_debugfs.o > If CONFIG_SCSI_SNIC_DEBUGFS is not set, then it leaves a build variable > ³snic-" in build system. ifeq() avoids such thing. That is fine. This is done all over the Kernel Makefile. There are two tons of these "do nothing make variables" It the way we do it in the Kernel. (I actually like it) >> >> You do not the ifeq () thing at all >> >> Cheers >> Boaz >> > Thanks > simha >> > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
SGkgQm9heiwgDQoNCiAgICBTdXJlLCBJIHdpbGwgaW5jb3Jwb3JhdGUgdGhlIGNvbW1lbnRzIGFu ZCBzdWdnZXN0aW9ucyBpbiBuZXh0IHBhdGNoDQpzdWJtaXR0YWwuDQoNClRoYW5rcw0KTmFyc2lt aHVsdQ0KDQpPbiAyNy8wNS8xNSA0OjU1IHBtLCAiQm9heiBIYXJyb3NoIiA8Ym9hekBwbGV4aXN0 b3IuY29tPiB3cm90ZToNCg0KPk9uIDA1LzI3LzIwMTUgMDE6MDIgUE0sIE5hcnNpbWh1bHUgTXVz aW5pIChubXVzaW5pKSB3cm90ZToNCj48Pg0KPj4+PiAraWZlcSAoJChDT05GSUdfU0NTSV9TTklD X0RFQlVHX0ZTKSwgeSkNCj4+Pj4gK2NjZmxhZ3MteSArPSAtRFNOSUNfREVCVUdfRlMNCj4+Pg0K Pj4+IFdoeSBkbyB5b3UgbmVlZCBhbiBleHRyYSBkZWZpbmUgaGVyZSBqdXN0IHVzZQ0KPj4+IENP TkZJR19TQ1NJX1NOSUNfREVCVUdfRlMgaW4gc291cmNlIGNvZGUgZGlyZWN0bHkNCj4+IEFncmVl LCBJIGp1c3Qgd2FudCB0byB1c2UgYSBzaG9ydGVyIG1hY3JvIGluIHRoZSBzb3VyY2UuDQo+DQo+ RG9uJ3QgZG8gdGhpcy4gSXQgaXMgYSBjb252ZW50aW9uIHRoYXQgaWYgYSBwcm9ncmFtbWVyDQo+ c2VlcyBhIENPTkZJR19YWFggaGUga25vd3MgdGhhdCB0aGlzIGlzIHNldHRhYmxlIGJ5IGENCj5L Y29uZmlnLiBCeSBtYWtpbmcgaXQgc2hvcnRlciB0aGUgcHJvZ3JhbW1lciB3aWxsIHRoaW5rIHRo YXQNCj50aGlzIGlzIGRlYWQgY29kZSBiZWNhdXNlIGl0IGlzIG5vdCBzZXQgYW55d2hlcmUuDQo+ DQo+Pj4NCj4+Pj4gK3NuaWMteSArPSBzbmljX2RlYnVnZnMubyBcDQo+Pj4+ICsJCXNuaWNfdHJj Lm8NCj4+Pj4gK2VuZGlmDQo+Pj4+DQo+Pj4NCj4+PiBzbmljLSQoQ09ORklHX1NDU0lfU05JQ19E RUJVR19GUykgKz0gc25pY19kZWJ1Z2ZzLm8NCj4+IElmIENPTkZJR19TQ1NJX1NOSUNfREVCVUdG UyBpcyBub3Qgc2V0LCB0aGVuIGl0IGxlYXZlcyBhIGJ1aWxkIHZhcmlhYmxlDQo+PiCp+HNuaWMt IiBpbiBidWlsZCBzeXN0ZW0uIGlmZXEoKSBhdm9pZHMgc3VjaCB0aGluZy4NCj4NCj5UaGF0IGlz IGZpbmUuIFRoaXMgaXMgZG9uZSBhbGwgb3ZlciB0aGUgS2VybmVsIE1ha2VmaWxlLg0KPlRoZXJl IGFyZSB0d28gdG9ucyBvZiB0aGVzZSAiZG8gbm90aGluZyBtYWtlIHZhcmlhYmxlcyINCj4NCj5J dCB0aGUgd2F5IHdlIGRvIGl0IGluIHRoZSBLZXJuZWwuIChJIGFjdHVhbGx5IGxpa2UgaXQpDQo+ DQo+Pj4NCj4+PiBZb3UgZG8gbm90IHRoZSAgaWZlcSAoKSB0aGluZyBhdCBhbGwNCj4+Pg0KPj4+ IENoZWVycw0KPj4+IEJvYXoNCj4+Pg0KPj4gVGhhbmtzDQo+PiBzaW1oYQ0KPj4+DQo+PiANCj4N Cg0K -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 28, 2015 at 09:49:08AM +0000, Narsimhulu Musini (nmusini) wrote: > Hi Boaz, > > Sure, I will incorporate the comments and suggestions in next patch > submittal. > > Thanks > Narsimhulu As you're going to re-submit anyway, I have found several occurences of: [Insert appropriate license here when releasing outside of Cisco] in the patches. Doesn't really look intentional. Johannes -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
In general, patch is much better: it passes all the static checker tests for byte width and endianness issues. However On Wed, 2015-05-27 at 00:19 -0700, Narsimhulu Musini wrote: > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig > index 9c92f41..8baab3f 100644 > --- a/drivers/scsi/Kconfig > +++ b/drivers/scsi/Kconfig > @@ -634,6 +634,23 @@ config FCOE_FNIC > <file:Documentation/scsi/scsi.txt>. > The module will be called fnic. > > +config SCSI_SNIC > + tristate "Cisco SNIC Driver" > + depends on PCI && SCSI && X86_64 Please don't restrict the compilation architecture in the Kconfig because that restricts the amount of build testing the driver gets. Instead you can put a runtime warning in the driver if you insist. Something like this http://thread.gmane.org/gmane.linux.scsi/101846 Where you add a runtime taint is much better. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Johannes, Thank you for reviewing the patches. I will incorporate your comments in next patch submittal. On 28/05/15 7:12 pm, "Johannes Thumshirn" <jthumshirn@suse.de> wrote: >On Thu, May 28, 2015 at 09:49:08AM +0000, Narsimhulu Musini (nmusini) >wrote: >> Hi Boaz, >> >> Sure, I will incorporate the comments and suggestions in next patch >> submittal. >> >> Thanks >> Narsimhulu > >As you're going to re-submit anyway, I have found several occurences of: >[Insert appropriate license here when releasing outside of Cisco] >in the patches. Thanks for pointing it. > >Doesn't really look intentional. > >Johannes Thanks Narsimhulu -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi James, Thank you for reviewing the patches. Sure, I will incorporate your comments and suggestions in next patch submittal. On 28/05/15 7:25 pm, "James Bottomley" <James.Bottomley@HansenPartnership.com> wrote: >In general, patch is much better: it passes all the static checker tests >for byte width and endianness issues. However > >On Wed, 2015-05-27 at 00:19 -0700, Narsimhulu Musini wrote: >> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig >> index 9c92f41..8baab3f 100644 >> --- a/drivers/scsi/Kconfig >> +++ b/drivers/scsi/Kconfig >> @@ -634,6 +634,23 @@ config FCOE_FNIC >> <file:Documentation/scsi/scsi.txt>. >> The module will be called fnic. >> >> +config SCSI_SNIC >> + tristate "Cisco SNIC Driver" >> + depends on PCI && SCSI && X86_64 > >Please don't restrict the compilation architecture in the Kconfig >because that restricts the amount of build testing the driver gets. >Instead you can put a runtime warning in the driver if you insist. >Something like this > >http://thread.gmane.org/gmane.linux.scsi/101846 > >Where you add a runtime taint is much better. Sure, I will add runtime taint. Thanks for suggesting it. > >James > > Thanks Narsimhulu > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/MAINTAINERS b/MAINTAINERS index 2a97e05..368fb76 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2536,6 +2536,13 @@ L: linux-scsi@vger.kernel.org S: Supported F: drivers/scsi/fnic/ +CISCO SCSI HBA DRIVER +M: Narsimhulu Musini <nmusini@cisco.com> +M: Sesidhar Baddela <sebaddel@cisco.com> +L: linux-scsi@vger.kernel.org +S: Supported +F: drivers/scsi/snic/ + CMPC ACPI DRIVER M: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com> M: Daniel Oliveira Nascimento <don@syst.com.br> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index 9c92f41..8baab3f 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -634,6 +634,23 @@ config FCOE_FNIC <file:Documentation/scsi/scsi.txt>. The module will be called fnic. +config SCSI_SNIC + tristate "Cisco SNIC Driver" + depends on PCI && SCSI && X86_64 + help + This is support for the Cisco PCI-Express SCSI HBA. + + To compile this driver as a module, choose M here and read + <file:Documentation/scsi/scsi.txt>. + The module will be called snic. + +config SCSI_SNIC_DEBUG_FS + bool "Cisco SNIC Driver Debugfs Support" + depends on SCSI_SNIC && DEBUG_FS + help + This enables to list debugging information from SNIC Driver + available via debugfs file system + config SCSI_DMX3191D tristate "DMX3191D SCSI support" depends on PCI && SCSI diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile index 58158f1..f643942 100644 --- a/drivers/scsi/Makefile +++ b/drivers/scsi/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_LIBFC) += libfc/ obj-$(CONFIG_LIBFCOE) += fcoe/ obj-$(CONFIG_FCOE) += fcoe/ obj-$(CONFIG_FCOE_FNIC) += fnic/ +obj-$(CONFIG_SCSI_SNIC) += snic/ obj-$(CONFIG_SCSI_BNX2X_FCOE) += libfc/ fcoe/ bnx2fc/ obj-$(CONFIG_ISCSI_TCP) += libiscsi.o libiscsi_tcp.o iscsi_tcp.o obj-$(CONFIG_INFINIBAND_ISER) += libiscsi.o diff --git a/drivers/scsi/snic/Makefile b/drivers/scsi/snic/Makefile new file mode 100644 index 0000000..572102a --- /dev/null +++ b/drivers/scsi/snic/Makefile @@ -0,0 +1,21 @@ +obj-$(CONFIG_SCSI_SNIC) += snic.o + +snic-y := \ + snic_attrs.o \ + snic_main.o \ + snic_res.o \ + snic_isr.o \ + snic_ctl.o \ + snic_io.o \ + snic_scsi.o \ + snic_disc.o \ + vnic_cq.o \ + vnic_intr.o \ + vnic_dev.o \ + vnic_wq.o + +ifeq ($(CONFIG_SCSI_SNIC_DEBUG_FS), y) +ccflags-y += -DSNIC_DEBUG_FS +snic-y += snic_debugfs.o \ + snic_trc.o +endif