Re: [alsa-devel] [PATCH v8] media: Add stk1160 new driver
Patch looks ok. Just a few comments:
Em 06-08-2012 10:38, Ezequiel Garcia escreveu:
This driver adds support for stk1160 usb bridge as used in some video/audio usb capture devices. It is a complete rewrite of staging/media/easycap driver and it's expected as a replacement.
Please don't add a "---" here. Everything after a --- are discarded by my scripts (and by most other kernel developer scripts).
Cc: Mauro Carvalho Chehab mchehab@redhat.com Cc: Takashi Iwai tiwai@suse.de Cc: Hans Verkuil hverkuil@xs4all.nl Cc: Sylwester Nawrocki sylvester.nawrocki@gmail.com
Hmm... weren't it reviewed already be them?
Signed-off-by: Ezequiel Garcia elezegarcia@gmail.com diff --git a/drivers/media/video/stk1160/Makefile b/drivers/media/video/stk1160/Makefile new file mode 100644 index 0000000..8f66a78 --- /dev/null +++ b/drivers/media/video/stk1160/Makefile @@ -0,0 +1,12 @@ +obj-stk1160-ac97-$(CONFIG_VIDEO_STK1160_AC97) := stk1160-ac97.o
+stk1160-y := stk1160-core.o \
stk1160-v4l.o \stk1160-video.o \stk1160-i2c.o \$(obj-stk1160-ac97-y)+obj-$(CONFIG_VIDEO_STK1160) += stk1160.o
+ccflags-y += -Wall
You shouldn't be adding the above here.
+ccflags-y += -Idrivers/media/video
Ah, please split this patch into two patches: one with the new driver addition, and another one with the removal of the driver at staging.
That will help to make the patch smaller, and avoids mixing two different things at the same place.
Thanks, Mauro
On Thu, Aug 9, 2012 at 5:25 PM, Mauro Carvalho Chehab mchehab@redhat.com wrote:
Patch looks ok. Just a few comments:
Em 06-08-2012 10:38, Ezequiel Garcia escreveu:
This driver adds support for stk1160 usb bridge as used in some video/audio usb capture devices. It is a complete rewrite of staging/media/easycap driver and it's expected as a replacement.
Please don't add a "---" here. Everything after a --- are discarded by my scripts (and by most other kernel developer scripts).
Mmm, that line was meant to separate commit message from message intended for developers/reviewers. Do you feel all the text should be part of the commit message?
Anyway, I know currently it's wrong, since the SOB should be part of commit message.
Cc: Mauro Carvalho Chehab mchehab@redhat.com Cc: Takashi Iwai tiwai@suse.de Cc: Hans Verkuil hverkuil@xs4all.nl Cc: Sylwester Nawrocki sylvester.nawrocki@gmail.com
Hmm... weren't it reviewed already be them?
Yes, Hans and Sylwester reviewed the various versions and Takashi reviewed the alsa part. I added a Cc, so they could review the changes made after their comments. Do you think I should drop it in v9?
Signed-off-by: Ezequiel Garcia elezegarcia@gmail.com diff --git a/drivers/media/video/stk1160/Makefile b/drivers/media/video/stk1160/Makefile new file mode 100644 index 0000000..8f66a78 --- /dev/null +++ b/drivers/media/video/stk1160/Makefile @@ -0,0 +1,12 @@ +obj-stk1160-ac97-$(CONFIG_VIDEO_STK1160_AC97) := stk1160-ac97.o
+stk1160-y := stk1160-core.o \
stk1160-v4l.o \stk1160-video.o \stk1160-i2c.o \$(obj-stk1160-ac97-y)+obj-$(CONFIG_VIDEO_STK1160) += stk1160.o
+ccflags-y += -Wall
You shouldn't be adding the above here.
Okey.
+ccflags-y += -Idrivers/media/video
Ah, please split this patch into two patches: one with the new driver addition, and another one with the removal of the driver at staging.
That will help to make the patch smaller, and avoids mixing two different things at the same place.
No problem. I hope the easycap removal patch passes through vger!
Thanks,
You are welcome :-) Ezequiel.
participants (2)
-
Ezequiel Garcia -
Mauro Carvalho Chehab