A second easycap driver implementation

A second easycap driver implementation

[Ezequiel García]

Hello,

After some research on v4l2 and videbuf2, and considering that easycap
driver is pretty
outdated I've decided to start writing a new driver from scratch.

I am using the excellent vivi driver and some usb video capture drivers as
a starting point. And of course, I'm using the current easycap implementation
as a reference (it works pretty well).

I have a couple of doubts regarding the development itself (how to
trace properly,
where to allocate urbs, and such) but perhaps the maintainers prefer
to take a look
at the code.

However, currently the driver is just a skeleton: it does all v4l2 and
videobuf2 intialization
but it doesn't actually stream video or submit urbs.

So,
1. Should I try to have something more finished before submit or can I
submit as it is?
2. In any case, how should I submit it? (Considering there is already
a working driver).

Thanks,
Ezequiel.

A second easycap driver implementation

[Ezequiel García]

Hello,

After some research on v4l2 and videbuf2, and considering that easycap
driver is pretty
outdated I've decided to start writing a new driver from scratch.

I am using the excellent vivi driver and some usb video capture drivers as
a starting point. And of course, I'm using the current easycap implementation
as a reference (it works pretty well).

I have a couple of doubts regarding the development itself (how to
trace properly,
where to allocate urbs, and such) but perhaps the maintainers prefer
to take a look
at the code.

However, currently the driver is just a skeleton: it does all v4l2 and
videobuf2 intialization
but it doesn't actually stream video or submit urbs.

So,
1. Should I try to have something more finished before submit or can I
submit as it is?
2. In any case, how should I submit it? (Considering there is already
a working driver).

Thanks,
Ezequiel.

Re: A second easycap driver implementation

[Hans de Goede]

Hi,

On 03/06/2012 10:04 PM, Ezequiel García wrote:
> Hello,
>
> After some research on v4l2 and videbuf2, and considering that easycap
> driver is pretty
> outdated I've decided to start writing a new driver from scratch.
>
> I am using the excellent vivi driver and some usb video capture drivers as
> a starting point. And of course, I'm using the current easycap implementation
> as a reference (it works pretty well).
>
> I have a couple of doubts regarding the development itself (how to
> trace properly,
> where to allocate urbs, and such) but perhaps the maintainers prefer
> to take a look
> at the code.
>
> However, currently the driver is just a skeleton: it does all v4l2 and
> videobuf2 intialization
> but it doesn't actually stream video or submit urbs.
>
> So,
> 1. Should I try to have something more finished before submit or can I
> submit as it is?
> 2. In any case, how should I submit it? (Considering there is already
> a working driver).

Have you considered instead slowly moving the existing easycap driver
over to all the new infrastructure we have now. For starters replace
its buffer management with videobuf2, then in another patch replace
some other bits, etc. ?  See what I've done to the pwc driver :)

OTOH if you already have a new more modern driver ready, then I say
go ahead and submit it. I would suggest to add it to staging too,
and make the 2 kconfig options conflict, so enabling one would allow
the user to no longer select the other (with a note about this in
the help text). And then hopefully soon we will see a follow up
patch removing the old driver, and then moving the new one out
of staging.

Which ever path you choose: Thanks for working on this!

Regards,

Hans

Re: A second easycap driver implementation

[Ezequiel García]

Hi,

>
> Have you considered instead slowly moving the existing easycap driver
> over to all the new infrastructure we have now. For starters replace
> its buffer management with videobuf2, then in another patch replace
> some other bits, etc. ?  See what I've done to the pwc driver :)

Yes. And that was what I was doing until now.
Yet, after some work it seemed much easier
to simply start over from scratch.

Besides, it's being a great learning experience :)

So, since the driver is not yet working I guess there
is no point in submitting anything.

Instead, anyone the wants to help I can send what I have now
or we can start working through github.
If someone owns this device, it would be a *huge* help
with testing.

However, as soon as this is capturing video I would like
to put it on staging, so everyone can help.
Is this possible?

Thanks,
Ezequiel.

Re: A second easycap driver implementation

[gregkh]

On Wed, Mar 07, 2012 at 11:32:23AM -0300, Ezequiel García wrote:
> Hi,
> 
> >
> > Have you considered instead slowly moving the existing easycap driver
> > over to all the new infrastructure we have now. For starters replace
> > its buffer management with videobuf2, then in another patch replace
> > some other bits, etc. ?  See what I've done to the pwc driver :)
> 
> Yes. And that was what I was doing until now.
> Yet, after some work it seemed much easier
> to simply start over from scratch.
> 
> Besides, it's being a great learning experience :)
> 
> So, since the driver is not yet working I guess there
> is no point in submitting anything.
> 
> Instead, anyone the wants to help I can send what I have now
> or we can start working through github.
> If someone owns this device, it would be a *huge* help
> with testing.
> 
> However, as soon as this is capturing video I would like
> to put it on staging, so everyone can help.
> Is this possible?

Yes it is, just send the patches to the correct people (note I don't
control the drivers/staging/media subdirectory.)

good luck,

greg k-h

Re: A second easycap driver implementation

[Mauro Carvalho Chehab]

Em 07-03-2012 12:43, gregkh escreveu:
> On Wed, Mar 07, 2012 at 11:32:23AM -0300, Ezequiel García wrote:
>> Hi,
>>
>>>
>>> Have you considered instead slowly moving the existing easycap driver
>>> over to all the new infrastructure we have now. For starters replace
>>> its buffer management with videobuf2, then in another patch replace
>>> some other bits, etc. ?  See what I've done to the pwc driver :)
>>
>> Yes. And that was what I was doing until now.
>> Yet, after some work it seemed much easier
>> to simply start over from scratch.

Yes, the driver is weird, as it encapsulates the demod code
inside it , instead of using the saa7115 driver, that covers most
of saa711x devices, including saa7113.

Btw, is this driver really needed? The em28xx driver has support
for the Easy Cap Capture DC-60 model (I had access to one of those
in the past, and I know that the driver works properly).

What's the chipset using on your Easycap device?

If it is not an Empiatech em28xx USB bridge, then it makes sense
to have a separate driver for it. Otherwise, it is just easier
and better to add support for your device there.

>>
>> Besides, it's being a great learning experience :)
>>
>> So, since the driver is not yet working I guess there
>> is no point in submitting anything.
>>
>> Instead, anyone the wants to help I can send what I have now
>> or we can start working through github.
>> If someone owns this device, it would be a *huge* help
>> with testing.
>>
>> However, as soon as this is capturing video I would like
>> to put it on staging, so everyone can help.
>> Is this possible?
> 
> Yes it is, just send the patches to the correct people (note I don't
> control the drivers/staging/media subdirectory.)
> 
> good luck,
> 
> greg k-h

Re: A second easycap driver implementation

[Ezequiel García]

On Wed, Mar 7, 2012 at 1:35 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
>
> Yes, the driver is weird, as it encapsulates the demod code
> inside it , instead of using the saa7115 driver, that covers most
> of saa711x devices, including saa7113.
>
> Btw, is this driver really needed? The em28xx driver has support
> for the Easy Cap Capture DC-60 model (I had access to one of those
> in the past, and I know that the driver works properly).
>
> What's the chipset using on your Easycap device?

Chipset is STK116. I'm not entirely sure but is seems that
there are to models: DC60 and DC60+.

Apparently, the former would be using STK1160.

>
> If it is not an Empiatech em28xx USB bridge, then it makes sense
> to have a separate driver for it. Otherwise, it is just easier
> and better to add support for your device there.

Ok, I'll take a look at the em28xx driver and also at the saa711x
to see how would it be possible to add support for this device.
Perhaps, we'll end up with a separate driver but based on
some common code.

Thanks,
Ezequiel.

Re: A second easycap driver implementation

[Mauro Carvalho Chehab]

Em 07-03-2012 13:45, Ezequiel García escreveu:
> On Wed, Mar 7, 2012 at 1:35 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>>
>> Yes, the driver is weird, as it encapsulates the demod code
>> inside it , instead of using the saa7115 driver, that covers most
>> of saa711x devices, including saa7113.
>>
>> Btw, is this driver really needed? The em28xx driver has support
>> for the Easy Cap Capture DC-60 model (I had access to one of those
>> in the past, and I know that the driver works properly).
>>
>> What's the chipset using on your Easycap device?
> 
> Chipset is STK116. I'm not entirely sure but is seems that
> there are to models: DC60 and DC60+.

Hmmm... so there are two different chipsets for DC60.
> 
> Apparently, the former would be using STK1160.
> 
>>
>> If it is not an Empiatech em28xx USB bridge, then it makes sense
>> to have a separate driver for it. Otherwise, it is just easier
>> and better to add support for your device there.
> 
> Ok, I'll take a look at the em28xx driver and also at the saa711x
> to see how would it be possible to add support for this device.

em28xx is a good reference.

The usage of saa711x is simple. All you need to do is to implement
an I2C bus at your easycap driver, load the module, and then, redirect
any demod ioctl call to the I2C bus, like:

static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm)
{
	struct em28xx_fh   *fh  = priv;
	struct em28xx      *dev = fh->dev;
	int                rc;

	rc = check_dev(dev);
	if (rc < 0)
		return rc;

	v4l2_device_call_all(&dev->v4l2_dev, 0, video, querystd, norm);

	return 0;
}


An I2C device has an address that needs to be send through the I2C
bus.

The saa711x devices use one of the I2C addresses below:

static unsigned short saa711x_addrs[] = {
	0x4a >> 1, 0x48 >> 1,   /* SAA7111, SAA7111A and SAA7113 */
	0x42 >> 1, 0x40 >> 1,   /* SAA7114, SAA7115 and SAA7118 */
	I2C_CLIENT_END };

It is not clear, from the easycap code, where the I2C address
is stored:

int write_saa(struct usb_device *p, u16 reg0, u16 set0)
{
	if (!p)
		return -ENODEV;
	SET(p, 0x200, 0x00);
	SET(p, 0x204, reg0);
	SET(p, 0x205, set0);
	SET(p, 0x200, 0x01);
	return wait_i2c(p);
}

int read_saa(struct usb_device *p, u16 reg0)
{
	u8 igot;

	if (!p)
		return -ENODEV;
	SET(p, 0x208, reg0);
	SET(p, 0x200, 0x20);
	if (0 != wait_i2c(p))
		return -1;
	igot = 0;
	GET(p, 0x0209, &igot);
	return igot;
}

Maybe they're stored on some other register, or maybe this device
has the I2C hardcoded (with would be very ugly and unlikely, but
I won't doubt that some vendor might be doing that).

Anyway, if you take a look at cx231xx and tm6000, you'll see that
some I2C bad implementations require a per-address type of logic.

> Perhaps, we'll end up with a separate driver but based on
> some common code.

As it is not an em28xx, a separate implementation for the stk chipset
is required. You should not need to re-invent the wheel for saa711x.
> 
> Thanks,
> Ezequiel.

Re: A second easycap driver implementation

[Ezequiel García]

Hi Mauro,

>
> em28xx is a good reference.
>

I'm looking at it.
In fact, I have a first question: why there is a limit to the number of devices
the driver support? I found the same idea in easycap original
implementation, but I
can't understand why do we have to limit in such a way.

        /* Check to see next free device and mark as used */
        do {
                nr = find_first_zero_bit(&em28xx_devused, EM28XX_MAXBOARDS);
                if (nr >= EM28XX_MAXBOARDS) {
                        /* No free device slots */
                        printk(DRIVER_NAME ": Supports only %i em28xx
boards.\n",
                                        EM28XX_MAXBOARDS);
                        retval = -ENOMEM;
                        goto err_no_slot;
                }
        } while (test_and_set_bit(nr, &em28xx_devused));


>
> It is not clear, from the easycap code, where the I2C address
> is stored:
>
> int write_saa(struct usb_device *p, u16 reg0, u16 set0)
> {
>        if (!p)
>                return -ENODEV;
>        SET(p, 0x200, 0x00);
>        SET(p, 0x204, reg0);
>        SET(p, 0x205, set0);
>        SET(p, 0x200, 0x01);
>        return wait_i2c(p);
> }

I think i2c_address it is near registers 0x200/0x204, which gets
initialised at setup_stk().
I'll have a closer look.

Thanks for your comments,
Ezequiel.

Re: A second easycap driver implementation

[Mauro Carvalho Chehab]

Em 07-03-2012 17:39, Ezequiel García escreveu:
> Hi Mauro,
> 
>>
>> em28xx is a good reference.
>>
> 
> I'm looking at it.
> In fact, I have a first question: why there is a limit to the number of devices
> the driver support? I found the same idea in easycap original
> implementation, but I
> can't understand why do we have to limit in such a way.
> 
>         /* Check to see next free device and mark as used */
>         do {
>                 nr = find_first_zero_bit(&em28xx_devused, EM28XX_MAXBOARDS);
>                 if (nr >= EM28XX_MAXBOARDS) {
>                         /* No free device slots */
>                         printk(DRIVER_NAME ": Supports only %i em28xx
> boards.\n",
>                                         EM28XX_MAXBOARDS);
>                         retval = -ENOMEM;
>                         goto err_no_slot;
>                 }
>         } while (test_and_set_bit(nr, &em28xx_devused));

There are several reasons for that, including a few historical ones. One of them 
is that the number of reserved v4l char devices is limited (well, V4L core
now supports dynamic allocation). Another one is a few modprobe arrays for em28xx.

Anyway, an USB 2.0 bus only supports one  uncompressed video stream.

> 
> 
>>
>> It is not clear, from the easycap code, where the I2C address
>> is stored:
>>
>> int write_saa(struct usb_device *p, u16 reg0, u16 set0)
>> {
>>        if (!p)
>>                return -ENODEV;
>>        SET(p, 0x200, 0x00);
>>        SET(p, 0x204, reg0);
>>        SET(p, 0x205, set0);
>>        SET(p, 0x200, 0x01);
>>        return wait_i2c(p);
>> }
> 
> I think i2c_address it is near registers 0x200/0x204, which gets
> initialised at setup_stk().
> I'll have a closer look.

Ok.

> 
> Thanks for your comments,
> Ezequiel.

Re: A second easycap driver implementation

[Ezequiel García]

Hi Mauro,

On 3/7/12, Mauro Carvalho Chehab <mchehab@redhat.com> wrote:
>
> The usage of saa711x is simple. All you need to do is to implement
> an I2C bus at your easycap driver, load the module, and then, redirect
> any demod ioctl call to the I2C bus, like:
>
> static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm)
> {
> 	struct em28xx_fh   *fh  = priv;
> 	struct em28xx      *dev = fh->dev;
> 	int                rc;
>
> 	rc = check_dev(dev);
> 	if (rc < 0)
> 		return rc;
>
> 	v4l2_device_call_all(&dev->v4l2_dev, 0, video, querystd, norm);
>
> 	return 0;
> }
>
>
> An I2C device has an address that needs to be send through the I2C
> bus.
>
> The saa711x devices use one of the I2C addresses below:
>
> static unsigned short saa711x_addrs[] = {
> 	0x4a >> 1, 0x48 >> 1,   /* SAA7111, SAA7111A and SAA7113 */
> 	0x42 >> 1, 0x40 >> 1,   /* SAA7114, SAA7115 and SAA7118 */
> 	I2C_CLIENT_END };
>

I made my easycap driver use saa7115 driver to detect its saa7113 chip.
It wasn't so hard after some head scratching.

The problem is now modprobe is taking too long, mainly because saa7115 does
some probing.
I was thinking (since we already discussed deferring stuff to a workqueue):

Would it be problematic (in any fashion) to do add the i2c sub device

  v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap,
                        "saa7115_auto", 0, saa711x_addrs);

in a workqueue, (in the same way modules are loaded in workqueues)?

I think not, since we won't call i2c directly, but rather through
v4l2_device_call_all(), right?

Thanks,
Ezequiel.

Re: A second easycap driver implementation

[Mauro Carvalho Chehab]

Em 19-03-2012 19:05, Ezequiel García escreveu:
> Hi Mauro,
> 
> On 3/7/12, Mauro Carvalho Chehab <mchehab@redhat.com> wrote:
>>
>> The usage of saa711x is simple. All you need to do is to implement
>> an I2C bus at your easycap driver, load the module, and then, redirect
>> any demod ioctl call to the I2C bus, like:
>>
>> static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm)
>> {
>> 	struct em28xx_fh   *fh  = priv;
>> 	struct em28xx      *dev = fh->dev;
>> 	int                rc;
>>
>> 	rc = check_dev(dev);
>> 	if (rc < 0)
>> 		return rc;
>>
>> 	v4l2_device_call_all(&dev->v4l2_dev, 0, video, querystd, norm);
>>
>> 	return 0;
>> }
>>
>>
>> An I2C device has an address that needs to be send through the I2C
>> bus.
>>
>> The saa711x devices use one of the I2C addresses below:
>>
>> static unsigned short saa711x_addrs[] = {
>> 	0x4a >> 1, 0x48 >> 1,   /* SAA7111, SAA7111A and SAA7113 */
>> 	0x42 >> 1, 0x40 >> 1,   /* SAA7114, SAA7115 and SAA7118 */
>> 	I2C_CLIENT_END };
>>
> 
> I made my easycap driver use saa7115 driver to detect its saa7113 chip.
> It wasn't so hard after some head scratching.

Good!

> The problem is now modprobe is taking too long, mainly because saa7115 does
> some probing.
> I was thinking (since we already discussed deferring stuff to a workqueue):
> 
> Would it be problematic (in any fashion) to do add the i2c sub device
> 
>   v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap,
>                         "saa7115_auto", 0, saa711x_addrs);
> 
> in a workqueue, (in the same way modules are loaded in workqueues)?
> 
> I think not, since we won't call i2c directly, but rather through
> v4l2_device_call_all(), right?

It would likely work. I would add some locking maybe at open or at ioctl
level, to prevent calling the device while the init doesn't finish.

Regards,
Mauro
> 
> Thanks,
> Ezequiel.