* [Buildroot] [PATCH V3] Adding support for NVME utils @ 2016-01-12 5:52 Mamatha 2016-01-12 20:27 ` Thomas Petazzoni 0 siblings, 1 reply; 6+ messages in thread From: Mamatha @ 2016-01-12 5:52 UTC (permalink / raw) To: buildroot Add support for building NVME utility - a utility for interacting with standard NVM Express (optimized PCI Express SSD interface) devices. Signed-off-by: Mamatha <mamatha4@linux.vnet.ibm.com> --- package/Config.in | 1 + package/nvme/Config.in | 6 ++++++ package/nvme/nvme.mk | 23 +++++++++++++++++++++++ 3 files changed, 30 insertions(+) create mode 100755 package/nvme/Config.in create mode 100755 package/nvme/nvme.mk diff --git a/package/Config.in b/package/Config.in index e0c2e2a..014debd 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1399,6 +1399,7 @@ endif source "package/openvmtools/Config.in" source "package/polkit/Config.in" source "package/powerpc-utils/Config.in" + source "package/nvme/Config.in" if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS source "package/procps-ng/Config.in" source "package/psmisc/Config.in" diff --git a/package/nvme/Config.in b/package/nvme/Config.in new file mode 100755 index 0000000..a3c8ada --- /dev/null +++ b/package/nvme/Config.in @@ -0,0 +1,6 @@ +config BR2_PACKAGE_NVME + bool "nvme" + help + System utilities for standard NVME Express devices + + https://github.com/linux-nvme/nvme-cli diff --git a/package/nvme/nvme.mk b/package/nvme/nvme.mk new file mode 100755 index 0000000..43bec67 --- /dev/null +++ b/package/nvme/nvme.mk @@ -0,0 +1,23 @@ +################################################################################ +# +# nvme +# +################################################################################ + +NVME_VERSION = 0.1 +NVME_SITE = https://github.com/linux-nvme/nvme-cli.git +NVME_SITE_METHOD = git +NVME_LICENSE = GPLv2 +NVME_LICENSE_FILES = COPYING + +define NVME_BUILD_CMDS + $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) \ + INCLUDEDIR="-I." all +endef + +define NVME_INSTALL_TARGET_CMDS + $(INSTALL) -D -m 0755 $(@D)/nvme $(TARGET_DIR)/sbin/nvme +endef + +$(eval $(generic-package)) + ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH V3] Adding support for NVME utils 2016-01-12 5:52 [Buildroot] [PATCH V3] Adding support for NVME utils Mamatha @ 2016-01-12 20:27 ` Thomas Petazzoni 2016-01-13 11:05 ` Mamatha Inamdar 0 siblings, 1 reply; 6+ messages in thread From: Thomas Petazzoni @ 2016-01-12 20:27 UTC (permalink / raw) To: buildroot Dear Mamatha, Thanks for your contribution! On Tue, 12 Jan 2016 11:22:06 +0530, Mamatha wrote: > Add support for building NVME utility - a utility for interacting with > standard NVM Express (optimized PCI Express SSD interface) devices. For all patches on packages, we prefer to have the title of the commit comply with the following format: <package>: <description> So in your case, it should be: nvme: new package > Signed-off-by: Mamatha <mamatha4@linux.vnet.ibm.com> Sorry if I'm not familiar with naming practices in your part of the world, but we require a full name (i.e first name + last name). Is Mamatha your full name ? > --- > package/Config.in | 1 + > package/nvme/Config.in | 6 ++++++ > package/nvme/nvme.mk | 23 +++++++++++++++++++++++ > 3 files changed, 30 insertions(+) > create mode 100755 package/nvme/Config.in > create mode 100755 package/nvme/nvme.mk > > diff --git a/package/Config.in b/package/Config.in > index e0c2e2a..014debd 100644 > --- a/package/Config.in > +++ b/package/Config.in > @@ -1399,6 +1399,7 @@ endif > source "package/openvmtools/Config.in" > source "package/polkit/Config.in" > source "package/powerpc-utils/Config.in" > + source "package/nvme/Config.in" You should respect alphabetic ordering here when including nvme/Config.in. Also I believe this package better belongs to the "Hardware handling" menu rather than "System tools". > if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS > source "package/procps-ng/Config.in" > source "package/psmisc/Config.in" > diff --git a/package/nvme/Config.in b/package/nvme/Config.in > new file mode 100755 > index 0000000..a3c8ada > --- /dev/null > +++ b/package/nvme/Config.in > @@ -0,0 +1,6 @@ > +config BR2_PACKAGE_NVME > + bool "nvme" > + help > + System utilities for standard NVME Express devices > + > + https://github.com/linux-nvme/nvme-cli > diff --git a/package/nvme/nvme.mk b/package/nvme/nvme.mk > new file mode 100755 > index 0000000..43bec67 > --- /dev/null > +++ b/package/nvme/nvme.mk > @@ -0,0 +1,23 @@ > +################################################################################ > +# > +# nvme > +# > +################################################################################ > + > +NVME_VERSION = 0.1 > +NVME_SITE = https://github.com/linux-nvme/nvme-cli.git > +NVME_SITE_METHOD = git You should use the github helper. See also https://buildroot.org/downloads/manual/manual.html#github-download-url. > +NVME_LICENSE = GPLv2 The license is GPLv2+ > +NVME_LICENSE_FILES = COPYING > + > +define NVME_BUILD_CMDS > + $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) \ > + INCLUDEDIR="-I." all Why do you explicitly specify "all" here ? It is going to build the documentation, while if you leave it out, it will only build the first target, which is named "default" in nvme's Makefile, and it will only build the nvme binary. Also, when calling $(MAKE), please pass $(TARGET_MAKE_ENV) in the environment: $(TARGET_MAKE_ENV) $(MAKE) ... > +endef > + > +define NVME_INSTALL_TARGET_CMDS > + $(INSTALL) -D -m 0755 $(@D)/nvme $(TARGET_DIR)/sbin/nvme Why you don't you use the install-bin target ? Like this: $(TARGET_MAKE_ENV) $(MAKE) -C (@D) DESTDIR=$(TARGET_DIR) install-bin Also, could you add a hash file, as explained in https://buildroot.org/downloads/manual/manual.html#adding-packages-hash ? Could you rework your patch to take into account those comments and send an updated version ? Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH V3] Adding support for NVME utils 2016-01-12 20:27 ` Thomas Petazzoni @ 2016-01-13 11:05 ` Mamatha Inamdar 2016-01-13 11:38 ` Thomas Petazzoni 0 siblings, 1 reply; 6+ messages in thread From: Mamatha Inamdar @ 2016-01-13 11:05 UTC (permalink / raw) To: buildroot Hi Thomas, Thanks for reviewing the patch. I have worked on all your comments,facing some issue while building nvme. Will check and send new version of patch soon. On 01/13/2016 01:57 AM, Thomas Petazzoni wrote: > Dear Mamatha, > > Thanks for your contribution! > > On Tue, 12 Jan 2016 11:22:06 +0530, Mamatha wrote: >> Add support for building NVME utility - a utility for interacting with >> standard NVM Express (optimized PCI Express SSD interface) devices. > For all patches on packages, we prefer to have the title of the commit > comply with the following format: > > <package>: <description> > > So in your case, it should be: > > nvme: new package > >> Signed-off-by: Mamatha <mamatha4@linux.vnet.ibm.com> > Sorry if I'm not familiar with naming practices in your part of the > world, but we require a full name (i.e first name + last name). Is > Mamatha your full name ? > >> --- >> package/Config.in | 1 + >> package/nvme/Config.in | 6 ++++++ >> package/nvme/nvme.mk | 23 +++++++++++++++++++++++ >> 3 files changed, 30 insertions(+) >> create mode 100755 package/nvme/Config.in >> create mode 100755 package/nvme/nvme.mk >> >> diff --git a/package/Config.in b/package/Config.in >> index e0c2e2a..014debd 100644 >> --- a/package/Config.in >> +++ b/package/Config.in >> @@ -1399,6 +1399,7 @@ endif >> source "package/openvmtools/Config.in" >> source "package/polkit/Config.in" >> source "package/powerpc-utils/Config.in" >> + source "package/nvme/Config.in" > You should respect alphabetic ordering here when including > nvme/Config.in. Also I believe this package better belongs to the > "Hardware handling" menu rather than "System tools". > >> if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS >> source "package/procps-ng/Config.in" >> source "package/psmisc/Config.in" >> diff --git a/package/nvme/Config.in b/package/nvme/Config.in >> new file mode 100755 >> index 0000000..a3c8ada >> --- /dev/null >> +++ b/package/nvme/Config.in >> @@ -0,0 +1,6 @@ >> +config BR2_PACKAGE_NVME >> + bool "nvme" >> + help >> + System utilities for standard NVME Express devices >> + >> + https://github.com/linux-nvme/nvme-cli >> diff --git a/package/nvme/nvme.mk b/package/nvme/nvme.mk >> new file mode 100755 >> index 0000000..43bec67 >> --- /dev/null >> +++ b/package/nvme/nvme.mk >> @@ -0,0 +1,23 @@ >> +################################################################################ >> +# >> +# nvme >> +# >> +################################################################################ >> + >> +NVME_VERSION = 0.1 >> +NVME_SITE = https://github.com/linux-nvme/nvme-cli.git >> +NVME_SITE_METHOD = git > You should use the github helper. See also > https://buildroot.org/downloads/manual/manual.html#github-download-url. > >> +NVME_LICENSE = GPLv2 > The license is GPLv2+ > >> +NVME_LICENSE_FILES = COPYING >> + >> +define NVME_BUILD_CMDS >> + $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) \ >> + INCLUDEDIR="-I." all > Why do you explicitly specify "all" here ? It is going to build the > documentation, while if you leave it out, it will only build the first > target, which is named "default" in nvme's Makefile, and it will only > build the nvme binary. > > Also, when calling $(MAKE), please pass $(TARGET_MAKE_ENV) in the > environment: > > $(TARGET_MAKE_ENV) $(MAKE) ... > >> +endef >> + >> +define NVME_INSTALL_TARGET_CMDS >> + $(INSTALL) -D -m 0755 $(@D)/nvme $(TARGET_DIR)/sbin/nvme > Why you don't you use the install-bin target ? Like this: > > $(TARGET_MAKE_ENV) $(MAKE) -C (@D) DESTDIR=$(TARGET_DIR) install-bin > > Also, could you add a hash file, as explained in > https://buildroot.org/downloads/manual/manual.html#adding-packages-hash ? > > Could you rework your patch to take into account those comments and > send an updated version ? > > Thanks! > > Thomas ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH V3] Adding support for NVME utils 2016-01-13 11:05 ` Mamatha Inamdar @ 2016-01-13 11:38 ` Thomas Petazzoni [not found] ` <569720FF.6020807@linux.vnet.ibm.com> 0 siblings, 1 reply; 6+ messages in thread From: Thomas Petazzoni @ 2016-01-13 11:38 UTC (permalink / raw) To: buildroot Mamatha, On Wed, 13 Jan 2016 16:35:22 +0530, Mamatha Inamdar wrote: > Thanks for reviewing the patch. You're welcome, thanks for your contribution in the first place. > I have worked on all your comments,facing some issue while building nvme. Which issues? > Will check and send new version of patch soon. Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <569720FF.6020807@linux.vnet.ibm.com>]
* [Buildroot] [PATCH V3] Adding support for NVME utils [not found] ` <569720FF.6020807@linux.vnet.ibm.com> @ 2016-01-14 9:04 ` Thomas Petazzoni 2016-01-14 11:15 ` Mamatha Inamdar 0 siblings, 1 reply; 6+ messages in thread From: Thomas Petazzoni @ 2016-01-14 9:04 UTC (permalink / raw) To: buildroot Hello, Please don't reply to me directly, always keep the list in Cc. On Thu, 14 Jan 2016 09:45:59 +0530, Mamatha Inamdar wrote: > I am facing two issues here. > > 1) NVME_VERSION = 0.1 > If I update version as 0.1 and compile the build, It's unable to > build and following errors occur You need to do: NVME_VERSION = v0.2 NVME_SITE = $(call github,linux-nvme,nvme-cli,$(NVME_VERSION)) (and not define the NVME_SOURCE variable) Note that I've changed the version from 0.1 to 0.2, since 0.2 is available, so you should use this one. > 2) Second issue is...If I use the install-bin target Like this: > > $(TARGET_MAKE_ENV) $(MAKE) -C (@D) DESTDIR=$(TARGET_DIR) install-bin > > I don't see nvme tool installed in sbin/bin, but it works for the > following install command Because you also need to set PREFIX=/usr otherwise it gets installed in /usr/local. A few other things: *) You need to patch the Makefile to remove -m64 and -Werror otherwise it won't build *) You need to pass $(TARGET_CONFIGURE_OPTS) on the left hand side of $(MAKE) rather than the right hand side so that the CFLAGS from $(TARGET_CONFIGURE_OPTS) don't override the ones defined in the Makefile. Without this, -std=gnu99 is no longer passed in the CFLAGS, and the package fails to build. *) INCLUDEDIR="-I." becomes useless. *) You need to pass LIBUDEV=1 to disable using the udev library. If you don't do this, the Makefile looks on the host machine if libudev is installed to determine whether it should enable udev support or not, which is completely wrong since we're cross-compiling. Alternatively, you could optionally add libudev support if you want, but that's not mandatory. *) Since the upstream project is named nvme-cli, I think the Buildroot package should also be named nvme-cli. All in all, I had a .mk file that looks like this: NVME_VERSION = v0.2 NVME_SITE = $(call github,linux-nvme,nvme-cli,$(NVME_VERSION)) NVME_LICENSE = GPLv2 NVME_LICENSE_FILES = COPYING # LIBUDEV=1 indicates that we don't want udev support define NVME_BUILD_CMDS $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) \ LIBUDEV=1 -C $(@D) endef define NVME_INSTALL_TARGET_CMDS $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) \ PREFIX=/usr install-bin endef $(eval $(generic-package)) Hope this helps, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH V3] Adding support for NVME utils 2016-01-14 9:04 ` Thomas Petazzoni @ 2016-01-14 11:15 ` Mamatha Inamdar 0 siblings, 0 replies; 6+ messages in thread From: Mamatha Inamdar @ 2016-01-14 11:15 UTC (permalink / raw) To: buildroot Thanks for the detailed explanation, updated your comments and sent new version of Patch. Thanks Mamatha On 01/14/2016 02:34 PM, Thomas Petazzoni wrote: > Hello, > > Please don't reply to me directly, always keep the list in Cc. > > On Thu, 14 Jan 2016 09:45:59 +0530, Mamatha Inamdar wrote: > >> I am facing two issues here. >> >> 1) NVME_VERSION = 0.1 >> If I update version as 0.1 and compile the build, It's unable to >> build and following errors occur > You need to do: > > NVME_VERSION = v0.2 > NVME_SITE = $(call github,linux-nvme,nvme-cli,$(NVME_VERSION)) > > (and not define the NVME_SOURCE variable) > > Note that I've changed the version from 0.1 to 0.2, since 0.2 is > available, so you should use this one. > > >> 2) Second issue is...If I use the install-bin target Like this: >> >> $(TARGET_MAKE_ENV) $(MAKE) -C (@D) DESTDIR=$(TARGET_DIR) install-bin >> >> I don't see nvme tool installed in sbin/bin, but it works for the >> following install command > Because you also need to set PREFIX=/usr otherwise it gets installed > in /usr/local. > > A few other things: > > *) You need to patch the Makefile to remove -m64 and -Werror otherwise > it won't build > > *) You need to pass $(TARGET_CONFIGURE_OPTS) on the left hand side of > $(MAKE) rather than the right hand side so that the CFLAGS from > $(TARGET_CONFIGURE_OPTS) don't override the ones defined in the > Makefile. Without this, -std=gnu99 is no longer passed in the CFLAGS, > and the package fails to build. > > *) INCLUDEDIR="-I." becomes useless. > > *) You need to pass LIBUDEV=1 to disable using the udev library. If > you don't do this, the Makefile looks on the host machine if libudev > is installed to determine whether it should enable udev support or > not, which is completely wrong since we're cross-compiling. > Alternatively, you could optionally add libudev support if you > want, but that's not mandatory. > > *) Since the upstream project is named nvme-cli, I think the Buildroot > package should also be named nvme-cli. > > All in all, I had a .mk file that looks like this: > > NVME_VERSION = v0.2 > NVME_SITE = $(call github,linux-nvme,nvme-cli,$(NVME_VERSION)) > NVME_LICENSE = GPLv2 > NVME_LICENSE_FILES = COPYING > > # LIBUDEV=1 indicates that we don't want udev support > define NVME_BUILD_CMDS > $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) \ > LIBUDEV=1 -C $(@D) > endef > > define NVME_INSTALL_TARGET_CMDS > $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) \ > PREFIX=/usr install-bin > endef > > $(eval $(generic-package)) > > Hope this helps, > > Thomas ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-14 11:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-12 5:52 [Buildroot] [PATCH V3] Adding support for NVME utils Mamatha
2016-01-12 20:27 ` Thomas Petazzoni
2016-01-13 11:05 ` Mamatha Inamdar
2016-01-13 11:38 ` Thomas Petazzoni
[not found] ` <569720FF.6020807@linux.vnet.ibm.com>
2016-01-14 9:04 ` Thomas Petazzoni
2016-01-14 11:15 ` Mamatha Inamdar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox