| Message ID | 1349820063-21955-1-git-send-email-elezegarcia@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Tue, 9 Oct 2012 19:01:03 -0300 Ezequiel Garcia <elezegarcia@gmail.com> escreveu: > In order to fully replace easycap driver with stk1160, > it's also necessary to add S-Video support. > > A similar patch backported for v3.2 kernel has been > tested by three different users. > > Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com> > --- > Hi Mauro, > > I'm sending this for inclusion in v3.7 second media pull request. > I realize it's very late, so I understand if you don't > want to pick it. > > drivers/media/usb/stk1160/stk1160-core.c | 15 +++++++++++---- > drivers/media/usb/stk1160/stk1160-v4l.c | 7 ++++++- > drivers/media/usb/stk1160/stk1160.h | 3 ++- > 3 files changed, 19 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/usb/stk1160/stk1160-core.c b/drivers/media/usb/stk1160/stk1160-core.c > index b627408..34a26e0 100644 > --- a/drivers/media/usb/stk1160/stk1160-core.c > +++ b/drivers/media/usb/stk1160/stk1160-core.c > @@ -100,12 +100,21 @@ int stk1160_write_reg(struct stk1160 *dev, u16 reg, u16 value) > > void stk1160_select_input(struct stk1160 *dev) > { > + int route; > static const u8 gctrl[] = { > - 0x98, 0x90, 0x88, 0x80 > + 0x98, 0x90, 0x88, 0x80, 0x98 > }; > > - if (dev->ctl_input < ARRAY_SIZE(gctrl)) > + if (dev->ctl_input == STK1160_SVIDEO_INPUT) > + route = SAA7115_SVIDEO3; > + else > + route = SAA7115_COMPOSITE0; > + > + if (dev->ctl_input < ARRAY_SIZE(gctrl)) { > + v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing, > + route, 0, 0); > stk1160_write_reg(dev, STK1160_GCTRL, gctrl[dev->ctl_input]); > + } > } > > /* TODO: We should break this into pieces */ > @@ -351,8 +360,6 @@ static int stk1160_probe(struct usb_interface *interface, > > /* i2c reset saa711x */ > v4l2_device_call_all(&dev->v4l2_dev, 0, core, reset, 0); > - v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing, > - 0, 0, 0); > v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0); > > /* reset stk1160 to default values */ > diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c > index fe6e857..6694f9e 100644 > --- a/drivers/media/usb/stk1160/stk1160-v4l.c > +++ b/drivers/media/usb/stk1160/stk1160-v4l.c > @@ -419,7 +419,12 @@ static int vidioc_enum_input(struct file *file, void *priv, > if (i->index > STK1160_MAX_INPUT) > return -EINVAL; > > - sprintf(i->name, "Composite%d", i->index); > + /* S-Video special handling */ > + if (i->index == STK1160_SVIDEO_INPUT) > + sprintf(i->name, "S-Video"); > + else > + sprintf(i->name, "Composite%d", i->index); > + Had you ever test this patch with the v4l2-compliance tool? It seems broken to me: this driver has just two inputs. So, it should return -EINVAL for all inputs after that, or otherwise userspace applications that query the inputs will loop forever! Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 9, 2012 at 7:25 PM, Mauro Carvalho Chehab <mchehab@redhat.com> wrote: > Em Tue, 9 Oct 2012 19:01:03 -0300 > Ezequiel Garcia <elezegarcia@gmail.com> escreveu: > >> In order to fully replace easycap driver with stk1160, >> it's also necessary to add S-Video support. >> >> A similar patch backported for v3.2 kernel has been >> tested by three different users. >> >> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com> >> --- >> Hi Mauro, >> >> I'm sending this for inclusion in v3.7 second media pull request. >> I realize it's very late, so I understand if you don't >> want to pick it. >> >> drivers/media/usb/stk1160/stk1160-core.c | 15 +++++++++++---- >> drivers/media/usb/stk1160/stk1160-v4l.c | 7 ++++++- >> drivers/media/usb/stk1160/stk1160.h | 3 ++- >> 3 files changed, 19 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/media/usb/stk1160/stk1160-core.c b/drivers/media/usb/stk1160/stk1160-core.c >> index b627408..34a26e0 100644 >> --- a/drivers/media/usb/stk1160/stk1160-core.c >> +++ b/drivers/media/usb/stk1160/stk1160-core.c >> @@ -100,12 +100,21 @@ int stk1160_write_reg(struct stk1160 *dev, u16 reg, u16 value) >> >> void stk1160_select_input(struct stk1160 *dev) >> { >> + int route; >> static const u8 gctrl[] = { >> - 0x98, 0x90, 0x88, 0x80 >> + 0x98, 0x90, 0x88, 0x80, 0x98 >> }; >> >> - if (dev->ctl_input < ARRAY_SIZE(gctrl)) >> + if (dev->ctl_input == STK1160_SVIDEO_INPUT) >> + route = SAA7115_SVIDEO3; >> + else >> + route = SAA7115_COMPOSITE0; >> + >> + if (dev->ctl_input < ARRAY_SIZE(gctrl)) { >> + v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing, >> + route, 0, 0); >> stk1160_write_reg(dev, STK1160_GCTRL, gctrl[dev->ctl_input]); >> + } >> } >> >> /* TODO: We should break this into pieces */ >> @@ -351,8 +360,6 @@ static int stk1160_probe(struct usb_interface *interface, >> >> /* i2c reset saa711x */ >> v4l2_device_call_all(&dev->v4l2_dev, 0, core, reset, 0); >> - v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing, >> - 0, 0, 0); >> v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0); >> >> /* reset stk1160 to default values */ >> diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c >> index fe6e857..6694f9e 100644 >> --- a/drivers/media/usb/stk1160/stk1160-v4l.c >> +++ b/drivers/media/usb/stk1160/stk1160-v4l.c >> @@ -419,7 +419,12 @@ static int vidioc_enum_input(struct file *file, void *priv, >> if (i->index > STK1160_MAX_INPUT) >> return -EINVAL; >> Look here... I'm returning EINVAL after STK1160_MAX_INPUT. (see below) >> - sprintf(i->name, "Composite%d", i->index); >> + /* S-Video special handling */ >> + if (i->index == STK1160_SVIDEO_INPUT) >> + sprintf(i->name, "S-Video"); >> + else >> + sprintf(i->name, "Composite%d", i->index); >> + > > Had you ever test this patch with the v4l2-compliance tool? > > It seems broken to me: this driver has just two inputs. So, it should return > -EINVAL for all inputs after that, or otherwise userspace applications that > query the inputs will loop forever! > Actually the driver has five inputs, since there are two kinds of devices: one with four composites, and another with one composite and one s-video. So, I simply support all of them, since there's no way to distinguish. I just tested this patch with v4l2-compliance and with qv4l2 and there are no regressions. Unless I'm missing something, I think the patch is OK. Let me know if you want me to change something. Ezequiel. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Tue, 9 Oct 2012 20:52:06 -0300 Ezequiel Garcia <elezegarcia@gmail.com> escreveu: > On Tue, Oct 9, 2012 at 7:25 PM, Mauro Carvalho Chehab > <mchehab@redhat.com> wrote: > > Em Tue, 9 Oct 2012 19:01:03 -0300 > > Ezequiel Garcia <elezegarcia@gmail.com> escreveu: > > > >> In order to fully replace easycap driver with stk1160, > >> it's also necessary to add S-Video support. > >> > >> A similar patch backported for v3.2 kernel has been > >> tested by three different users. > >> > >> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com> > >> --- > >> Hi Mauro, > >> > >> I'm sending this for inclusion in v3.7 second media pull request. > >> I realize it's very late, so I understand if you don't > >> want to pick it. > >> > >> drivers/media/usb/stk1160/stk1160-core.c | 15 +++++++++++---- > >> drivers/media/usb/stk1160/stk1160-v4l.c | 7 ++++++- > >> drivers/media/usb/stk1160/stk1160.h | 3 ++- > >> 3 files changed, 19 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/media/usb/stk1160/stk1160-core.c b/drivers/media/usb/stk1160/stk1160-core.c > >> index b627408..34a26e0 100644 > >> --- a/drivers/media/usb/stk1160/stk1160-core.c > >> +++ b/drivers/media/usb/stk1160/stk1160-core.c > >> @@ -100,12 +100,21 @@ int stk1160_write_reg(struct stk1160 *dev, u16 reg, u16 value) > >> > >> void stk1160_select_input(struct stk1160 *dev) > >> { > >> + int route; > >> static const u8 gctrl[] = { > >> - 0x98, 0x90, 0x88, 0x80 > >> + 0x98, 0x90, 0x88, 0x80, 0x98 > >> }; > >> > >> - if (dev->ctl_input < ARRAY_SIZE(gctrl)) > >> + if (dev->ctl_input == STK1160_SVIDEO_INPUT) > >> + route = SAA7115_SVIDEO3; > >> + else > >> + route = SAA7115_COMPOSITE0; > >> + > >> + if (dev->ctl_input < ARRAY_SIZE(gctrl)) { > >> + v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing, > >> + route, 0, 0); > >> stk1160_write_reg(dev, STK1160_GCTRL, gctrl[dev->ctl_input]); > >> + } > >> } > >> > >> /* TODO: We should break this into pieces */ > >> @@ -351,8 +360,6 @@ static int stk1160_probe(struct usb_interface *interface, > >> > >> /* i2c reset saa711x */ > >> v4l2_device_call_all(&dev->v4l2_dev, 0, core, reset, 0); > >> - v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing, > >> - 0, 0, 0); > >> v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0); > >> > >> /* reset stk1160 to default values */ > >> diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c > >> index fe6e857..6694f9e 100644 > >> --- a/drivers/media/usb/stk1160/stk1160-v4l.c > >> +++ b/drivers/media/usb/stk1160/stk1160-v4l.c > >> @@ -419,7 +419,12 @@ static int vidioc_enum_input(struct file *file, void *priv, > >> if (i->index > STK1160_MAX_INPUT) > >> return -EINVAL; > >> > > Look here... I'm returning EINVAL after STK1160_MAX_INPUT. > (see below) Sorry, my bad. I was, in fact, tricked by the first hunk, where you was routing to either saa7115 video3 or composite0. Now, I noticed that this device has an extra video switch, controlled via STK1160_GCTRL register. Tricky. > > >> - sprintf(i->name, "Composite%d", i->index); > >> + /* S-Video special handling */ > >> + if (i->index == STK1160_SVIDEO_INPUT) > >> + sprintf(i->name, "S-Video"); > >> + else > >> + sprintf(i->name, "Composite%d", i->index); > >> + > > > > Had you ever test this patch with the v4l2-compliance tool? > > > > It seems broken to me: this driver has just two inputs. So, it should return > > -EINVAL for all inputs after that, or otherwise userspace applications that > > query the inputs will loop forever! > > > > Actually the driver has five inputs, since there are two kinds of devices: > one with four composites, and another with one composite and one s-video. > So, I simply support all of them, since there's no way to distinguish. Ok then. > > I just tested this patch with v4l2-compliance and with qv4l2 and there > are no regressions. > > Unless I'm missing something, I think the patch is OK. > Let me know if you want me to change something. With the light of your comments, the patch looks fine on my eyes. > > Ezequiel.
Em Tue, 9 Oct 2012 21:04:46 -0300 Mauro Carvalho Chehab <mchehab@redhat.com> escreveu: > Em Tue, 9 Oct 2012 20:52:06 -0300 > Ezequiel Garcia <elezegarcia@gmail.com> escreveu: > > > On Tue, Oct 9, 2012 at 7:25 PM, Mauro Carvalho Chehab > > <mchehab@redhat.com> wrote: > > > Em Tue, 9 Oct 2012 19:01:03 -0300 > > > Ezequiel Garcia <elezegarcia@gmail.com> escreveu: > > > > > Let me know if you want me to change something. Hmm... patchwork didn't get it. The MIME type for the attachment is wrong... it is "text/plain". That may explain why patchwork didn't get it. Could you please re-submit it with the patch inlined, or if you really can't do it, using the proper MIME types for patch? Thanks! Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Tue, 9 Oct 2012 21:11:04 -0300 Mauro Carvalho Chehab <mchehab@redhat.com> escreveu: > Em Tue, 9 Oct 2012 21:04:46 -0300 > Mauro Carvalho Chehab <mchehab@redhat.com> escreveu: > > > Em Tue, 9 Oct 2012 20:52:06 -0300 > > Ezequiel Garcia <elezegarcia@gmail.com> escreveu: > > > > > On Tue, Oct 9, 2012 at 7:25 PM, Mauro Carvalho Chehab > > > <mchehab@redhat.com> wrote: > > > > Em Tue, 9 Oct 2012 19:01:03 -0300 > > > > Ezequiel Garcia <elezegarcia@gmail.com> escreveu: > > > > > > > > Let me know if you want me to change something. > > Hmm... patchwork didn't get it. The MIME type for the attachment > is wrong... it is "text/plain". That may explain why patchwork > didn't get it. Could you please re-submit it with the patch > inlined, or if you really can't do it, using the proper MIME > types for patch? Nevermind. the patch looks correct. I'll double-check what's wrong at patchwork side. > > Thanks! > Mauro
On Tue, Oct 9, 2012 at 9:13 PM, Mauro Carvalho Chehab <mchehab@redhat.com> wrote: > Em Tue, 9 Oct 2012 21:11:04 -0300 > Mauro Carvalho Chehab <mchehab@redhat.com> escreveu: > >> Em Tue, 9 Oct 2012 21:04:46 -0300 >> Mauro Carvalho Chehab <mchehab@redhat.com> escreveu: >> >> > Em Tue, 9 Oct 2012 20:52:06 -0300 >> > Ezequiel Garcia <elezegarcia@gmail.com> escreveu: >> > >> > > On Tue, Oct 9, 2012 at 7:25 PM, Mauro Carvalho Chehab >> > > <mchehab@redhat.com> wrote: >> > > > Em Tue, 9 Oct 2012 19:01:03 -0300 >> > > > Ezequiel Garcia <elezegarcia@gmail.com> escreveu: >> > > > >> >> > > Let me know if you want me to change something. >> >> Hmm... patchwork didn't get it. The MIME type for the attachment >> is wrong... it is "text/plain". That may explain why patchwork >> didn't get it. Could you please re-submit it with the patch >> inlined, or if you really can't do it, using the proper MIME >> types for patch? > > Nevermind. the patch looks correct. I'll double-check what's wrong > at patchwork side. Great! Thanks! Ezequiel -- To unsubscribe from this list: send the line "unsubscribe linux-media" 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/drivers/media/usb/stk1160/stk1160-core.c b/drivers/media/usb/stk1160/stk1160-core.c index b627408..34a26e0 100644 --- a/drivers/media/usb/stk1160/stk1160-core.c +++ b/drivers/media/usb/stk1160/stk1160-core.c @@ -100,12 +100,21 @@ int stk1160_write_reg(struct stk1160 *dev, u16 reg, u16 value) void stk1160_select_input(struct stk1160 *dev) { + int route; static const u8 gctrl[] = { - 0x98, 0x90, 0x88, 0x80 + 0x98, 0x90, 0x88, 0x80, 0x98 }; - if (dev->ctl_input < ARRAY_SIZE(gctrl)) + if (dev->ctl_input == STK1160_SVIDEO_INPUT) + route = SAA7115_SVIDEO3; + else + route = SAA7115_COMPOSITE0; + + if (dev->ctl_input < ARRAY_SIZE(gctrl)) { + v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing, + route, 0, 0); stk1160_write_reg(dev, STK1160_GCTRL, gctrl[dev->ctl_input]); + } } /* TODO: We should break this into pieces */ @@ -351,8 +360,6 @@ static int stk1160_probe(struct usb_interface *interface, /* i2c reset saa711x */ v4l2_device_call_all(&dev->v4l2_dev, 0, core, reset, 0); - v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing, - 0, 0, 0); v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0); /* reset stk1160 to default values */ diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c index fe6e857..6694f9e 100644 --- a/drivers/media/usb/stk1160/stk1160-v4l.c +++ b/drivers/media/usb/stk1160/stk1160-v4l.c @@ -419,7 +419,12 @@ static int vidioc_enum_input(struct file *file, void *priv, if (i->index > STK1160_MAX_INPUT) return -EINVAL; - sprintf(i->name, "Composite%d", i->index); + /* S-Video special handling */ + if (i->index == STK1160_SVIDEO_INPUT) + sprintf(i->name, "S-Video"); + else + sprintf(i->name, "Composite%d", i->index); + i->type = V4L2_INPUT_TYPE_CAMERA; i->std = dev->vdev.tvnorms; return 0; diff --git a/drivers/media/usb/stk1160/stk1160.h b/drivers/media/usb/stk1160/stk1160.h index 3feba00..68c8707 100644 --- a/drivers/media/usb/stk1160/stk1160.h +++ b/drivers/media/usb/stk1160/stk1160.h @@ -46,7 +46,8 @@ #define STK1160_MIN_PKT_SIZE 3072 -#define STK1160_MAX_INPUT 3 +#define STK1160_MAX_INPUT 4 +#define STK1160_SVIDEO_INPUT 4 #define STK1160_I2C_TIMEOUT 100
In order to fully replace easycap driver with stk1160, it's also necessary to add S-Video support. A similar patch backported for v3.2 kernel has been tested by three different users. Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com> --- Hi Mauro, I'm sending this for inclusion in v3.7 second media pull request. I realize it's very late, so I understand if you don't want to pick it. drivers/media/usb/stk1160/stk1160-core.c | 15 +++++++++++---- drivers/media/usb/stk1160/stk1160-v4l.c | 7 ++++++- drivers/media/usb/stk1160/stk1160.h | 3 ++- 3 files changed, 19 insertions(+), 6 deletions(-)