From: Davidlohr Bueso <dave.bueso@gmail.com>
To: "Michał Nazarewicz" <m.nazarewicz@samsung.com>
Cc: dbrownell@users.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH RESEND] tools/usb: Add Makefile
Date: Mon, 26 Jul 2010 10:14:50 -0400 [thread overview]
Message-ID: <1280153690.2212.10.camel@cowboy> (raw)
In-Reply-To: <op.vftzi5ku7p4s8u@pikus>
On Wed, 2010-07-14 at 13:29 +0200, Michał Nazarewicz wrote:
> You should Cc it to linux-usb@vger.kernel.org as well.
>
> On Tue, 13 Jul 2010 11:50:42 +0200, Davidlohr Bueso <dave.bueso@gmail.com> wrote:
> > Hi,
> >
> > This patch adds a Makefile for building 'testusb'.
> >
> > I am wondering if it should really still use usbdevfs (deprecated),
> > instead of usbfs?
>
> Patches welcome... ;) Personally I dunno, I just took David's code
> and changes it a bit.
>
I am modifying this a bit, will send patch when ready.
> > Thanks,
> > Davidlohr
> >
> > Signed-off-by: Davidlohr Bueso <dave@gnu.org>
>
> I'd say the above is not acceptable as a commit message. Please
> include only the text that is intended to go to the commit message
> above. All additional comments may go under the “---” marker.
>
> > ---
> > tools/usb/Makefile | 14 ++++++++++++++
> > 1 files changed, 14 insertions(+), 0 deletions(-)
> > create mode 100644 tools/usb/Makefile
> >
> > diff --git a/tools/usb/Makefile b/tools/usb/Makefile
> > new file mode 100644
> > index 0000000..45b5cab
> > --- /dev/null
> > +++ b/tools/usb/Makefile
> > @@ -0,0 +1,14 @@
> > +# Makefile for building 'usbtest'
> > +
> > +CC = $(CROSS_COMPILE)gcc
> > +PTHREAD_LIBS = -lpthread
> > +WARNINGS = -Wall
> > +WARNINGS := $(WARNINGS) -Wextra
>
> Is it necessary? Wouldn't plain
>
> WARNINGS = -Wall -Wextra
>
> suffice? I am aware “=” can be overridden by command line and
> “:=” cannot but I'd use single line anyway.
>
> > +CFLAGS = $(WARNINGS) -g $(PTHREAD_LIBS)
> > +
> > +all:
> > + $(CC) $(CFLAGS) -o testusb testusb.c
>
> IMO, you should also include a rule for ffs-test, ie:
>
> +all: testusb ffs-test
> +
> +%: %.c
> + $(CC) $(CFLAGS) -o $@ $^
>
> Not tested.
>
> > +
> > +clean:
> > + $(RM) testusb
>
> Instead:
>
> > + $(RM) testusb ffs-test
>
> > +
>
> Unnecessary empty line at EOF.
>
> Other then that, I see no reason why not to include it.
>
Sorry for the delay, I will redo the patch with the suggestions and
resend.
Thanks,
Davidlohr
prev parent reply other threads:[~2010-07-26 14:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-13 9:50 [PATCH RESEND] tools/usb: Add Makefile Davidlohr Bueso
2010-07-14 11:29 ` Michał Nazarewicz
2010-07-26 14:14 ` Davidlohr Bueso [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1280153690.2212.10.camel@cowboy \
--to=dave.bueso@gmail.com \
--cc=dbrownell@users.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=m.nazarewicz@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.