[PATCH] xc5000, rework xc_write_reg

[PATCH] xc5000, rework xc_write_reg

[Dmitri Belimov]

[-- Attachment #1: Type: text/plain, Size: 2524 bytes --]

Hi

Rework xc_write_reg function for correct read register of the xc5000.
It is very useful for tm6000.

Tested for tm6000 and for saa7134 works well.

diff -r 8f5129efe974 linux/drivers/media/common/tuners/xc5000.c
--- a/linux/drivers/media/common/tuners/xc5000.c	Sun May 16 18:48:01 2010 -0300
+++ b/linux/drivers/media/common/tuners/xc5000.c	Tue May 18 11:14:55 2010 +1000
@@ -232,6 +232,26 @@
 	return 0;
 }
 
+static int xc5000_readreg(struct xc5000_priv *priv, u16 reg, u16 *val)
+{
+	u8 buf[2] = { reg >> 8, reg & 0xff };
+	u8 bval[2] = { 0, 0 };
+	struct i2c_msg msg[2] = {
+		{ .addr = priv->i2c_props.addr,
+			.flags = 0, .buf = &buf[0], .len = 2 },
+		{ .addr = priv->i2c_props.addr,
+			.flags = I2C_M_RD, .buf = &bval[0], .len = 2 },
+	};
+
+	if (i2c_transfer(priv->i2c_props.adap, msg, 2) != 2) {
+		printk(KERN_WARNING "xc5000: I2C read failed\n");
+		return -EREMOTEIO;
+	}
+
+	*val = (bval[0] << 8) | bval[1];
+	return XC_RESULT_SUCCESS;
+}
+
 static void xc_wait(int wait_ms)
 {
 	msleep(wait_ms);
@@ -275,20 +295,14 @@
 	if (result == XC_RESULT_SUCCESS) {
 		/* wait for busy flag to clear */
 		while ((WatchDogTimer > 0) && (result == XC_RESULT_SUCCESS)) {
-			buf[0] = 0;
-			buf[1] = XREG_BUSY;
-
-			result = xc_send_i2c_data(priv, buf, 2);
+			result = xc5000_readreg(priv, XREG_BUSY, buf);
 			if (result == XC_RESULT_SUCCESS) {
-				result = xc_read_i2c_data(priv, buf, 2);
-				if (result == XC_RESULT_SUCCESS) {
-					if ((buf[0] == 0) && (buf[1] == 0)) {
-						/* busy flag cleared */
+				if ((buf[0] == 0) && (buf[1] == 0)) {
+					/* busy flag cleared */
 					break;
-					} else {
-						xc_wait(5); /* wait 5 ms */
-						WatchDogTimer--;
-					}
+				} else {
+					xc_wait(5); /* wait 5 ms */
+					WatchDogTimer--;
 				}
 			}
 		}
@@ -534,25 +548,6 @@
 	return found;
 }
 
-static int xc5000_readreg(struct xc5000_priv *priv, u16 reg, u16 *val)
-{
-	u8 buf[2] = { reg >> 8, reg & 0xff };
-	u8 bval[2] = { 0, 0 };
-	struct i2c_msg msg[2] = {
-		{ .addr = priv->i2c_props.addr,
-			.flags = 0, .buf = &buf[0], .len = 2 },
-		{ .addr = priv->i2c_props.addr,
-			.flags = I2C_M_RD, .buf = &bval[0], .len = 2 },
-	};
-
-	if (i2c_transfer(priv->i2c_props.adap, msg, 2) != 2) {
-		printk(KERN_WARNING "xc5000: I2C read failed\n");
-		return -EREMOTEIO;
-	}
-
-	*val = (bval[0] << 8) | bval[1];
-	return XC_RESULT_SUCCESS;
-}
 
 static int xc5000_fwupload(struct dvb_frontend *fe)
 {

Signed-off-by: Beholder Intl. Ltd. Dmitry Belimov <d.belimov@gmail.com>


With my best regards, Dmitry.

[-- Attachment #2: xc5000_write_read.patch --]
[-- Type: text/x-patch, Size: 2340 bytes --]

diff -r 8f5129efe974 linux/drivers/media/common/tuners/xc5000.c
--- a/linux/drivers/media/common/tuners/xc5000.c	Sun May 16 18:48:01 2010 -0300
+++ b/linux/drivers/media/common/tuners/xc5000.c	Tue May 18 11:14:55 2010 +1000
@@ -232,6 +232,26 @@
 	return 0;
 }
 
+static int xc5000_readreg(struct xc5000_priv *priv, u16 reg, u16 *val)
+{
+	u8 buf[2] = { reg >> 8, reg & 0xff };
+	u8 bval[2] = { 0, 0 };
+	struct i2c_msg msg[2] = {
+		{ .addr = priv->i2c_props.addr,
+			.flags = 0, .buf = &buf[0], .len = 2 },
+		{ .addr = priv->i2c_props.addr,
+			.flags = I2C_M_RD, .buf = &bval[0], .len = 2 },
+	};
+
+	if (i2c_transfer(priv->i2c_props.adap, msg, 2) != 2) {
+		printk(KERN_WARNING "xc5000: I2C read failed\n");
+		return -EREMOTEIO;
+	}
+
+	*val = (bval[0] << 8) | bval[1];
+	return XC_RESULT_SUCCESS;
+}
+
 static void xc_wait(int wait_ms)
 {
 	msleep(wait_ms);
@@ -275,20 +295,14 @@
 	if (result == XC_RESULT_SUCCESS) {
 		/* wait for busy flag to clear */
 		while ((WatchDogTimer > 0) && (result == XC_RESULT_SUCCESS)) {
-			buf[0] = 0;
-			buf[1] = XREG_BUSY;
-
-			result = xc_send_i2c_data(priv, buf, 2);
+			result = xc5000_readreg(priv, XREG_BUSY, buf);
 			if (result == XC_RESULT_SUCCESS) {
-				result = xc_read_i2c_data(priv, buf, 2);
-				if (result == XC_RESULT_SUCCESS) {
-					if ((buf[0] == 0) && (buf[1] == 0)) {
-						/* busy flag cleared */
+				if ((buf[0] == 0) && (buf[1] == 0)) {
+					/* busy flag cleared */
 					break;
-					} else {
-						xc_wait(5); /* wait 5 ms */
-						WatchDogTimer--;
-					}
+				} else {
+					xc_wait(5); /* wait 5 ms */
+					WatchDogTimer--;
 				}
 			}
 		}
@@ -534,25 +548,6 @@
 	return found;
 }
 
-static int xc5000_readreg(struct xc5000_priv *priv, u16 reg, u16 *val)
-{
-	u8 buf[2] = { reg >> 8, reg & 0xff };
-	u8 bval[2] = { 0, 0 };
-	struct i2c_msg msg[2] = {
-		{ .addr = priv->i2c_props.addr,
-			.flags = 0, .buf = &buf[0], .len = 2 },
-		{ .addr = priv->i2c_props.addr,
-			.flags = I2C_M_RD, .buf = &bval[0], .len = 2 },
-	};
-
-	if (i2c_transfer(priv->i2c_props.adap, msg, 2) != 2) {
-		printk(KERN_WARNING "xc5000: I2C read failed\n");
-		return -EREMOTEIO;
-	}
-
-	*val = (bval[0] << 8) | bval[1];
-	return XC_RESULT_SUCCESS;
-}
 
 static int xc5000_fwupload(struct dvb_frontend *fe)
 {

Signed-off-by: Beholder Intl. Ltd. Dmitry Belimov <d.belimov@gmail.com>

Re: [PATCH] xc5000, rework xc_write_reg

[Devin Heitmueller]

On Tue, May 18, 2010 at 3:30 AM, Dmitri Belimov <d.belimov@gmail.com> wrote:
> Hi
>
> Rework xc_write_reg function for correct read register of the xc5000.
> It is very useful for tm6000.
>
> Tested for tm6000 and for saa7134 works well.

Hi Dmitri,

I've put this on my list of patches to review.  My concern is that the
xc_wait logic is pretty nasty since it's related to timing of the bus
(it took several weeks as well as a dozen emails with the people at
Xceive), and hence I am loathed to change it since it took quite a bit
of time to test against all the different cards that use xc5000 (and
in some cases there were bugs exposed in various bridge's i2c
implementations).

That said, I think I actually did attempt to implement a patch
comparable to what you did here, but I backed it out for some reason.
I will need to review my trees and my notes to see what the rationale
was for doing such.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] xc5000, rework xc_write_reg

[Dmitri Belimov]

Hi Devin

> On Tue, May 18, 2010 at 3:30 AM, Dmitri Belimov <d.belimov@gmail.com>
> wrote:
> > Hi
> >
> > Rework xc_write_reg function for correct read register of the
> > xc5000. It is very useful for tm6000.
> >
> > Tested for tm6000 and for saa7134 works well.
> 
> Hi Dmitri,
> 
> I've put this on my list of patches to review.  My concern is that the
> xc_wait logic is pretty nasty since it's related to timing of the bus
> (it took several weeks as well as a dozen emails with the people at
> Xceive), and hence I am loathed to change it since it took quite a bit
> of time to test against all the different cards that use xc5000 (and
> in some cases there were bugs exposed in various bridge's i2c
> implementations).
> 
> That said, I think I actually did attempt to implement a patch
> comparable to what you did here, but I backed it out for some reason.
> I will need to review my trees and my notes to see what the rationale
> was for doing such.

Ok. I can test your solution on our hardware.
XC5000+SAA7134
XC5000+TM6010

With my best regards, Dmitry.

> Devin
> 
> -- 
> Devin J. Heitmueller - Kernel Labs
> http://www.kernellabs.com

Re: [PATCH] xc5000, rework xc_write_reg

[Mauro Carvalho Chehab]

Em 24-05-2010 22:49, Dmitri Belimov escreveu:
> Hi Devin
> 
>> On Tue, May 18, 2010 at 3:30 AM, Dmitri Belimov <d.belimov@gmail.com>
>> wrote:
>>> Hi
>>>
>>> Rework xc_write_reg function for correct read register of the
>>> xc5000. It is very useful for tm6000.
>>>
>>> Tested for tm6000 and for saa7134 works well.
>>
>> Hi Dmitri,
>>
>> I've put this on my list of patches to review.  My concern is that the
>> xc_wait logic is pretty nasty since it's related to timing of the bus
>> (it took several weeks as well as a dozen emails with the people at
>> Xceive), and hence I am loathed to change it since it took quite a bit
>> of time to test against all the different cards that use xc5000 (and
>> in some cases there were bugs exposed in various bridge's i2c
>> implementations).
>>
>> That said, I think I actually did attempt to implement a patch
>> comparable to what you did here, but I backed it out for some reason.
>> I will need to review my trees and my notes to see what the rationale
>> was for doing such.
> 
> Ok. I can test your solution on our hardware.
> XC5000+SAA7134
> XC5000+TM6010

Devin/Dmitri,

Any progress about this patch?

Cheers,
Mauro

Re: [PATCH] xc5000, rework xc_write_reg

[Devin Heitmueller]

On Mon, Jul 5, 2010 at 12:11 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Devin/Dmitri,
>
> Any progress about this patch?
>
> Cheers,
> Mauro

Sorry for the delay.

I did some testing with it today, and it looks fine.

Acked-by: Devin Heitmueller <dheitmueller@kernellabs.com>

Thanks,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

xc5000 and switch RF input

[Dmitri Belimov]

Hi

Our TV card Behold X7 has two different RF input. This RF inputs can switch between
different RF sources. 

ANT 1 for analog and digital TV
ANT 2 for FM radio

The switch controlled by zl10353.

How to I can control this switch?

I found 2 way

1. 
Use tuner callback to saa1734. add some params like XC5000_TUNER_SET/XC5000_TUNER_SET_TV to the xc5000.h
call tuner callback from xc5000_set_analog_params with new params about switching.
In this case inside saa7134 need know about zl10353 and configuration. I don't understand how to do.
The structure saa7134_dev hasn't pointer to the structure dvb_frontend. 
Or use hardcoded i2c_addr and params.

2.
Direct call switch the switcher from the tuner code. In this case need know TV card type. I think it is not so good idea.

With my best regards, Dmitry.

Re: xc5000 and switch RF input

[Mauro Carvalho Chehab]

Em 06-10-2010 16:52, Dmitri Belimov escreveu:
> Hi
> 
> Our TV card Behold X7 has two different RF input. This RF inputs can switch between
> different RF sources. 
> 
> ANT 1 for analog and digital TV
> ANT 2 for FM radio
> 
> The switch controlled by zl10353.
> 
> How to I can control this switch?
> 
> I found 2 way
> 
> 1. 
> Use tuner callback to saa1734. add some params like XC5000_TUNER_SET/XC5000_TUNER_SET_TV to the xc5000.h
> call tuner callback from xc5000_set_analog_params with new params about switching.
> In this case inside saa7134 need know about zl10353 and configuration. I don't understand how to do.
> The structure saa7134_dev hasn't pointer to the structure dvb_frontend. 
> Or use hardcoded i2c_addr and params.
> 
> 2.
> Direct call switch the switcher from the tuner code. In this case need know TV card type. I think it is not so good idea.


Alternative 2 is not a good idea.

There's another alternative: just call some code at saa7134-video, when changing from TV or from video, 
for example, at s_freq. It may be hard to track when this happens, as the user may be using a program 
like kradio, that may keep the device open even while not using it.

This problem looks like one I'm currently having where both DVB and analog parts are trying to access
tda18271 tuner and mb86a20s demod at the same time. The same problem also happened on em28xx and cx231xx.
The solution there were to (ab)use of the device lock, but it is not perfect.

There's a better alternative that requires adding more glue at frontend.

I'll post soon an email with a proposal for that. It would also solve other problems that we're currently
experiencing.

Cheers,
Mauro

[RFC] Resource reservation for frontend - Was: Re: xc5000 and switch RF input

[Mauro Carvalho Chehab]

Em 06-10-2010 16:52, Dmitri Belimov escreveu:
> Hi
> 
> Our TV card Behold X7 has two different RF input. This RF inputs can switch between
> different RF sources. 
> 
> ANT 1 for analog and digital TV
> ANT 2 for FM radio
> 
> The switch controlled by zl10353.
> 
> How to I can control this switch?
> 
> I found 2 way
> 
> 1. 
> Use tuner callback to saa1734. add some params like XC5000_TUNER_SET/XC5000_TUNER_SET_TV to the xc5000.h
> call tuner callback from xc5000_set_analog_params with new params about switching.
> In this case inside saa7134 need know about zl10353 and configuration. I don't understand how to do.
> The structure saa7134_dev hasn't pointer to the structure dvb_frontend. 
> Or use hardcoded i2c_addr and params.
> 
> 2.
> Direct call switch the switcher from the tuner code. In this case need know TV card type. I think it is not so good idea.

The issues between FM and TV mode is only a small part of a big problem that we're currently
facing: drivers that support multiple types of streams, like radio, analog TV and digital TV
need a way to avoid conflicts between different parts of the driver, and between a DVB and a
V4L call.

The long-term solution seems to implement a tuner/frontend resource reservation routine. This 
will solve other problems as well, like having both analog and digital parts of the driver 
trying to access the same resource at the same time.

While implementing both analog and ISDB-T support for a saa7134-based board, I got one issue
where analog tuner were trying to do RF callibration while the DVB demod were initialized.
As the access to the demod requires one I2C gate setup and the access to the tuner requires
another setup, both operations failed.

We had similar problems in em28xx and cx231xx. Both were partially solved with a lock, but still
if the user tries to open both DVB and V4L interfaces, it will likely have problems.

So, we really need to implement some type of resource locking that will properly setup I2C gates,
RF gates, etc, depending on the type of resource currently in use.

Basically, the idea would be to implement something like:

enum frontend_resource {
	ANALOG_TV_TUNER,
	DIGITAL_TV_TUNER,
	FM_TUNER,
	ANALOG_DEMOD,
	DIGITAL_DEMOD,
};

And add a new callback at struct dvb_frontend_ops():

	int (*set_resource)(struct dvb_frontend* fe, enum frontend_resource, bool reserve);

Those callbacks may replace i2c_gate_ctrl().

With those changes, when a driver needs to access, for example, a tuner at a dvb part of the driver,
it would do:

fe->ops.set_resource(fe, DIGITAL_TV_TUNER, true);
/* All i2c transactions and other operations needed at tuner */
fe->ops.set_resource(fe, DIGITAL_TV_TUNER, false);

In other words, it will basically replace the current occurrences of i2c_gate_ctrl(), providing
a clearer indication that the I2C change needed are to enable the access to the tuner.

The fun begins if other parts of the driver try to do different things on resources that
may be shared. They can now say that they want to access the demod with:

fe->ops.set_resource(fe, DIGITAL_DEMOD, true);

So, demods operations will be protected by:

fe->ops.set_resource(fe, DIGITAL_DEMOD, true);
	/* All i2c transactions and other operations applied on demod */
fe->ops.set_resource(fe, DIGITAL_DEMOD, false);

It is up to driver callback to handle this call. If both DIGITAL_DEMOD and DIGITAL_TV_TUNER are
at the same i2c bus (e.g. there's no i2c gate), and if there's no risk for one I2C access to affect
the other, the callback can just return 0. Otherwise, it may return -EBUSY and let the caller wait
for the resource to be freed with:
	wake_up(fe->ops.set_resource(fe, DIGITAL_DEMOD, true) == 0);
or
	wake_up_interruptible(fe->ops.set_resource(fe, DIGITAL_DEMOD, true) == 0);

This way, when the resource is freed, the digital demod logic may happen.

Comments?

Thanks,
Mauro

Re: [RFC] Resource reservation for frontend - Was: Re: xc5000 and switch RF input

[Dmitri Belimov]

Hi

> Em 06-10-2010 16:52, Dmitri Belimov escreveu:
> > Hi
> > 
> > Our TV card Behold X7 has two different RF input. This RF inputs
> > can switch between different RF sources. 
> > 
> > ANT 1 for analog and digital TV
> > ANT 2 for FM radio
> > 
> > The switch controlled by zl10353.
> > 
> > How to I can control this switch?
> > 
> > I found 2 way
> > 
> > 1. 
> > Use tuner callback to saa1734. add some params like
> > XC5000_TUNER_SET/XC5000_TUNER_SET_TV to the xc5000.h call tuner
> > callback from xc5000_set_analog_params with new params about
> > switching. In this case inside saa7134 need know about zl10353 and
> > configuration. I don't understand how to do. The structure
> > saa7134_dev hasn't pointer to the structure dvb_frontend. Or use
> > hardcoded i2c_addr and params.
> > 
> > 2.
> > Direct call switch the switcher from the tuner code. In this case
> > need know TV card type. I think it is not so good idea.
> 
> The issues between FM and TV mode is only a small part of a big
> problem that we're currently facing: drivers that support multiple
> types of streams, like radio, analog TV and digital TV need a way to
> avoid conflicts between different parts of the driver, and between a
> DVB and a V4L call.
> 
> The long-term solution seems to implement a tuner/frontend resource
> reservation routine. This will solve other problems as well, like
> having both analog and digital parts of the driver trying to access
> the same resource at the same time.
> 
> While implementing both analog and ISDB-T support for a saa7134-based
> board, I got one issue where analog tuner were trying to do RF
> callibration while the DVB demod were initialized. As the access to
> the demod requires one I2C gate setup and the access to the tuner
> requires another setup, both operations failed.
> 
> We had similar problems in em28xx and cx231xx. Both were partially
> solved with a lock, but still if the user tries to open both DVB and
> V4L interfaces, it will likely have problems.
> 
> So, we really need to implement some type of resource locking that
> will properly setup I2C gates, RF gates, etc, depending on the type
> of resource currently in use.
> 
> Basically, the idea would be to implement something like:
> 
> enum frontend_resource {
> 	ANALOG_TV_TUNER,
> 	DIGITAL_TV_TUNER,
> 	FM_TUNER,
> 	ANALOG_DEMOD,
> 	DIGITAL_DEMOD,
> };
> 
> And add a new callback at struct dvb_frontend_ops():
> 
> 	int (*set_resource)(struct dvb_frontend* fe, enum
> frontend_resource, bool reserve);
> 
> Those callbacks may replace i2c_gate_ctrl().
> 
> With those changes, when a driver needs to access, for example, a
> tuner at a dvb part of the driver, it would do:
> 
> fe->ops.set_resource(fe, DIGITAL_TV_TUNER, true);
> /* All i2c transactions and other operations needed at tuner */
> fe->ops.set_resource(fe, DIGITAL_TV_TUNER, false);
> 
> In other words, it will basically replace the current occurrences of
> i2c_gate_ctrl(), providing a clearer indication that the I2C change
> needed are to enable the access to the tuner.
> 
> The fun begins if other parts of the driver try to do different
> things on resources that may be shared. They can now say that they
> want to access the demod with:
> 
> fe->ops.set_resource(fe, DIGITAL_DEMOD, true);
> 
> So, demods operations will be protected by:
> 
> fe->ops.set_resource(fe, DIGITAL_DEMOD, true);
> 	/* All i2c transactions and other operations applied on demod
> */ fe->ops.set_resource(fe, DIGITAL_DEMOD, false);
> 
> It is up to driver callback to handle this call. If both
> DIGITAL_DEMOD and DIGITAL_TV_TUNER are at the same i2c bus (e.g.
> there's no i2c gate), and if there's no risk for one I2C access to
> affect the other, the callback can just return 0. Otherwise, it may
> return -EBUSY and let the caller wait for the resource to be freed
> with: wake_up(fe->ops.set_resource(fe, DIGITAL_DEMOD, true) == 0); or
> 	wake_up_interruptible(fe->ops.set_resource(fe, DIGITAL_DEMOD,
> true) == 0);
> 
> This way, when the resource is freed, the digital demod logic may
> happen.
> 
> Comments?

What about hardware encoders? May be need switch between some TV and encoders.
Switch input source, output bus and other.

With my best regards, Dmitry.

> Thanks,
> Mauro

Re: [RFC] Resource reservation for frontend - Was: Re: xc5000 and switch RF input

[Mauro Carvalho Chehab]

Em 07-10-2010 10:00, Dmitri Belimov escreveu:
> Hi
> 
>> Em 06-10-2010 16:52, Dmitri Belimov escreveu:
>>> Hi
>>>
>>> Our TV card Behold X7 has two different RF input. This RF inputs
>>> can switch between different RF sources. 
>>>
>>> ANT 1 for analog and digital TV
>>> ANT 2 for FM radio
>>>
>>> The switch controlled by zl10353.
>>>
>>> How to I can control this switch?
>>>
>>> I found 2 way
>>>
>>> 1. 
>>> Use tuner callback to saa1734. add some params like
>>> XC5000_TUNER_SET/XC5000_TUNER_SET_TV to the xc5000.h call tuner
>>> callback from xc5000_set_analog_params with new params about
>>> switching. In this case inside saa7134 need know about zl10353 and
>>> configuration. I don't understand how to do. The structure
>>> saa7134_dev hasn't pointer to the structure dvb_frontend. Or use
>>> hardcoded i2c_addr and params.
>>>
>>> 2.
>>> Direct call switch the switcher from the tuner code. In this case
>>> need know TV card type. I think it is not so good idea.
>>
>> The issues between FM and TV mode is only a small part of a big
>> problem that we're currently facing: drivers that support multiple
>> types of streams, like radio, analog TV and digital TV need a way to
>> avoid conflicts between different parts of the driver, and between a
>> DVB and a V4L call.
>>
>> The long-term solution seems to implement a tuner/frontend resource
>> reservation routine. This will solve other problems as well, like
>> having both analog and digital parts of the driver trying to access
>> the same resource at the same time.
>>
>> While implementing both analog and ISDB-T support for a saa7134-based
>> board, I got one issue where analog tuner were trying to do RF
>> callibration while the DVB demod were initialized. As the access to
>> the demod requires one I2C gate setup and the access to the tuner
>> requires another setup, both operations failed.
>>
>> We had similar problems in em28xx and cx231xx. Both were partially
>> solved with a lock, but still if the user tries to open both DVB and
>> V4L interfaces, it will likely have problems.
>>
>> So, we really need to implement some type of resource locking that
>> will properly setup I2C gates, RF gates, etc, depending on the type
>> of resource currently in use.
>>
>> Basically, the idea would be to implement something like:
>>
>> enum frontend_resource {
>> 	ANALOG_TV_TUNER,
>> 	DIGITAL_TV_TUNER,
>> 	FM_TUNER,
>> 	ANALOG_DEMOD,
>> 	DIGITAL_DEMOD,
>> };
>>
>> And add a new callback at struct dvb_frontend_ops():
>>
>> 	int (*set_resource)(struct dvb_frontend* fe, enum
>> frontend_resource, bool reserve);
>>
>> Those callbacks may replace i2c_gate_ctrl().
>>
>> With those changes, when a driver needs to access, for example, a
>> tuner at a dvb part of the driver, it would do:
>>
>> fe->ops.set_resource(fe, DIGITAL_TV_TUNER, true);
>> /* All i2c transactions and other operations needed at tuner */
>> fe->ops.set_resource(fe, DIGITAL_TV_TUNER, false);
>>
>> In other words, it will basically replace the current occurrences of
>> i2c_gate_ctrl(), providing a clearer indication that the I2C change
>> needed are to enable the access to the tuner.
>>
>> The fun begins if other parts of the driver try to do different
>> things on resources that may be shared. They can now say that they
>> want to access the demod with:
>>
>> fe->ops.set_resource(fe, DIGITAL_DEMOD, true);
>>
>> So, demods operations will be protected by:
>>
>> fe->ops.set_resource(fe, DIGITAL_DEMOD, true);
>> 	/* All i2c transactions and other operations applied on demod
>> */ fe->ops.set_resource(fe, DIGITAL_DEMOD, false);
>>
>> It is up to driver callback to handle this call. If both
>> DIGITAL_DEMOD and DIGITAL_TV_TUNER are at the same i2c bus (e.g.
>> there's no i2c gate), and if there's no risk for one I2C access to
>> affect the other, the callback can just return 0. Otherwise, it may
>> return -EBUSY and let the caller wait for the resource to be freed
>> with: wake_up(fe->ops.set_resource(fe, DIGITAL_DEMOD, true) == 0); or
>> 	wake_up_interruptible(fe->ops.set_resource(fe, DIGITAL_DEMOD,
>> true) == 0);
>>
>> This way, when the resource is freed, the digital demod logic may
>> happen.
>>
>> Comments?
> 
> What about hardware encoders? May be need switch between some TV and encoders.
> Switch input source, output bus and other.

Yeah, we may need to add encoders here, and other stuff like IR and CA modules.

an approach like that is easy to extend, as new issues that may require
resource reservation is added. 

Cheers,
Mauro

Re: [RFC] Resource reservation for frontend - Was: Re: xc5000 and switch RF input

[Dmitri Belimov]

Hi Mauro

> Em 07-10-2010 10:00, Dmitri Belimov escreveu:
> > Hi
> > 
> >> Em 06-10-2010 16:52, Dmitri Belimov escreveu:
> >>> Hi
> >>>
> >>> Our TV card Behold X7 has two different RF input. This RF inputs
> >>> can switch between different RF sources. 
> >>>
> >>> ANT 1 for analog and digital TV
> >>> ANT 2 for FM radio
> >>>
> >>> The switch controlled by zl10353.
> >>>
> >>> How to I can control this switch?
> >>>
> >>> I found 2 way
> >>>
> >>> 1. 
> >>> Use tuner callback to saa1734. add some params like
> >>> XC5000_TUNER_SET/XC5000_TUNER_SET_TV to the xc5000.h call tuner
> >>> callback from xc5000_set_analog_params with new params about
> >>> switching. In this case inside saa7134 need know about zl10353 and
> >>> configuration. I don't understand how to do. The structure
> >>> saa7134_dev hasn't pointer to the structure dvb_frontend. Or use
> >>> hardcoded i2c_addr and params.
> >>>
> >>> 2.
> >>> Direct call switch the switcher from the tuner code. In this case
> >>> need know TV card type. I think it is not so good idea.
> >>
> >> The issues between FM and TV mode is only a small part of a big
> >> problem that we're currently facing: drivers that support multiple
> >> types of streams, like radio, analog TV and digital TV need a way
> >> to avoid conflicts between different parts of the driver, and
> >> between a DVB and a V4L call.
> >>
> >> The long-term solution seems to implement a tuner/frontend resource
> >> reservation routine. This will solve other problems as well, like
> >> having both analog and digital parts of the driver trying to access
> >> the same resource at the same time.
> >>
> >> While implementing both analog and ISDB-T support for a
> >> saa7134-based board, I got one issue where analog tuner were
> >> trying to do RF callibration while the DVB demod were initialized.
> >> As the access to the demod requires one I2C gate setup and the
> >> access to the tuner requires another setup, both operations failed.
> >>
> >> We had similar problems in em28xx and cx231xx. Both were partially
> >> solved with a lock, but still if the user tries to open both DVB
> >> and V4L interfaces, it will likely have problems.
> >>
> >> So, we really need to implement some type of resource locking that
> >> will properly setup I2C gates, RF gates, etc, depending on the type
> >> of resource currently in use.
> >>
> >> Basically, the idea would be to implement something like:
> >>
> >> enum frontend_resource {
> >> 	ANALOG_TV_TUNER,
> >> 	DIGITAL_TV_TUNER,
> >> 	FM_TUNER,
> >> 	ANALOG_DEMOD,
> >> 	DIGITAL_DEMOD,
> >> };
> >>
> >> And add a new callback at struct dvb_frontend_ops():
> >>
> >> 	int (*set_resource)(struct dvb_frontend* fe, enum
> >> frontend_resource, bool reserve);
> >>
> >> Those callbacks may replace i2c_gate_ctrl().
> >>
> >> With those changes, when a driver needs to access, for example, a
> >> tuner at a dvb part of the driver, it would do:
> >>
> >> fe->ops.set_resource(fe, DIGITAL_TV_TUNER, true);
> >> /* All i2c transactions and other operations needed at tuner */
> >> fe->ops.set_resource(fe, DIGITAL_TV_TUNER, false);
> >>
> >> In other words, it will basically replace the current occurrences
> >> of i2c_gate_ctrl(), providing a clearer indication that the I2C
> >> change needed are to enable the access to the tuner.
> >>
> >> The fun begins if other parts of the driver try to do different
> >> things on resources that may be shared. They can now say that they
> >> want to access the demod with:
> >>
> >> fe->ops.set_resource(fe, DIGITAL_DEMOD, true);
> >>
> >> So, demods operations will be protected by:
> >>
> >> fe->ops.set_resource(fe, DIGITAL_DEMOD, true);
> >> 	/* All i2c transactions and other operations applied on
> >> demod */ fe->ops.set_resource(fe, DIGITAL_DEMOD, false);
> >>
> >> It is up to driver callback to handle this call. If both
> >> DIGITAL_DEMOD and DIGITAL_TV_TUNER are at the same i2c bus (e.g.
> >> there's no i2c gate), and if there's no risk for one I2C access to
> >> affect the other, the callback can just return 0. Otherwise, it may
> >> return -EBUSY and let the caller wait for the resource to be freed
> >> with: wake_up(fe->ops.set_resource(fe, DIGITAL_DEMOD, true) == 0);
> >> or wake_up_interruptible(fe->ops.set_resource(fe, DIGITAL_DEMOD,
> >> true) == 0);
> >>
> >> This way, when the resource is freed, the digital demod logic may
> >> happen.
> >>
> >> Comments?
> > 
> > What about hardware encoders? May be need switch between some TV
> > and encoders. Switch input source, output bus and other.
> 
> Yeah, we may need to add encoders here, and other stuff like IR and
> CA modules.
> 
> an approach like that is easy to extend, as new issues that may
> require resource reservation is added. 

Are you wrote this feature already? I'll try it on my boards.
Some description need :)

> Cheers,
> Mauro

With my best regards, Dmitry.

Re: [RFC] Resource reservation for frontend - Was: Re: xc5000 and switch RF input

[Antti Palosaari]

On 10/06/2010 03:40 PM, Mauro Carvalho Chehab wrote:
> So, we really need to implement some type of resource locking that will properly setup I2C gates,
> RF gates, etc, depending on the type of resource currently in use.
>
> Basically, the idea would be to implement something like:
>
> enum frontend_resource {
> 	ANALOG_TV_TUNER,
> 	DIGITAL_TV_TUNER,
> 	FM_TUNER,
> 	ANALOG_DEMOD,
> 	DIGITAL_DEMOD,
> };

I haven't examined this yet enough, but for the background information I 
can say I have one device which needs this. There is tuner behind 
demodulator, but instead of normal I2C-gate switch, it is rather much 
likely repeater. All tuner commands are send to the demod which then 
writes those to the tuner.

DD = demod I2C addr
TT = tuner I2C addr
Bn = payload data

traditional I2C send to the tuner:
TT >> B0 B1 B2 ...

demod as repeater send to the tuner:
DD >> TT B0 B1 B2 ...

Antti
-- 
http://palosaari.fi/

Re: [RFC] Resource reservation for frontend - Was: Re: xc5000 and switch RF input

[Devin Heitmueller]

On Thu, Oct 14, 2010 at 4:33 AM, Antti Palosaari <crope@iki.fi> wrote:
> I haven't examined this yet enough, but for the background information I can
> say I have one device which needs this. There is tuner behind demodulator,
> but instead of normal I2C-gate switch, it is rather much likely repeater.
> All tuner commands are send to the demod which then writes those to the
> tuner.
>
> DD = demod I2C addr
> TT = tuner I2C addr
> Bn = payload data
>
> traditional I2C send to the tuner:
> TT >> B0 B1 B2 ...
>
> demod as repeater send to the tuner:
> DD >> TT B0 B1 B2 ...

You can accomplish this by having the demod create an i2c adapter
instance, which generates i2c commands to the bridge.  Then when
instantiating the tuner subdev, pass a pointer to the demod's i2c
adapter instead of the i2c adapter provided by the bridge.

No changes required to the core framework.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [RFC] Resource reservation for frontend - Was: Re: xc5000 and switch RF input

[Antti Palosaari]

On 10/14/2010 11:41 AM, Devin Heitmueller wrote:
> On Thu, Oct 14, 2010 at 4:33 AM, Antti Palosaari<crope@iki.fi>  wrote:
>> I haven't examined this yet enough, but for the background information I can
>> say I have one device which needs this. There is tuner behind demodulator,
>> but instead of normal I2C-gate switch, it is rather much likely repeater.
>> All tuner commands are send to the demod which then writes those to the
>> tuner.
>>
>> DD = demod I2C addr
>> TT = tuner I2C addr
>> Bn = payload data
>>
>> traditional I2C send to the tuner:
>> TT>>  B0 B1 B2 ...
>>
>> demod as repeater send to the tuner:
>> DD>>  TT B0 B1 B2 ...
>
> You can accomplish this by having the demod create an i2c adapter
> instance, which generates i2c commands to the bridge.  Then when
> instantiating the tuner subdev, pass a pointer to the demod's i2c
> adapter instead of the i2c adapter provided by the bridge.
>
> No changes required to the core framework.

Thank you Devin. I didn't realized that earlier. It worked just fine.

Antti
-- 
http://palosaari.fi/

[PATCH] xc5000 and switch RF input

[Dmitri Belimov]

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

Hi

Our TV card Behold X7 has two different RF input. This RF inputs can switch between
different RF sources. 

ANT 1 for analog and digital TV
ANT 2 for FM radio

The switch controlled by zl10353.

I add some defines for the tuner xc5000 and use tuner callback to saa7134 part.
All works well. But my patch can touch other TV cards with xc5000.

Devin can you check my changes on the other TV cards.

With my best regards, Dmitry.

[-- Attachment #2: behold_switch_ant.patch --]
[-- Type: text/x-patch, Size: 3459 bytes --]

diff -r 1da5fed5c8b2 linux/drivers/media/common/tuners/xc5000.c
--- a/linux/drivers/media/common/tuners/xc5000.c	Sun Sep 19 02:23:09 2010 -0300
+++ b/linux/drivers/media/common/tuners/xc5000.c	Wed Oct 13 10:49:45 2010 +1000
@@ -903,10 +903,33 @@
 	switch (params->mode) {
 	case V4L2_TUNER_RADIO:
 		ret = xc5000_set_radio_freq(fe, params);
+		if (fe->callback) {
+			fe->callback(((fe->dvb) && (fe->dvb->priv)) ?
+					   fe->dvb->priv :
+					   priv->i2c_props.adap->algo_data,
+					   DVB_FRONTEND_COMPONENT_TUNER,
+					   XC5000_TUNER_SET_RADIO, 0);
+		}
 		break;
 	case V4L2_TUNER_ANALOG_TV:
+		ret = xc5000_set_tv_freq(fe, params);
+		if (fe->callback) {
+			fe->callback(((fe->dvb) && (fe->dvb->priv)) ?
+					   fe->dvb->priv :
+					   priv->i2c_props.adap->algo_data,
+					   DVB_FRONTEND_COMPONENT_TUNER,
+					   XC5000_TUNER_SET_ANALOG_TV, 0);
+		}
+		break;
 	case V4L2_TUNER_DIGITAL_TV:
 		ret = xc5000_set_tv_freq(fe, params);
+		if (fe->callback) {
+			fe->callback(((fe->dvb) && (fe->dvb->priv)) ?
+					   fe->dvb->priv :
+					   priv->i2c_props.adap->algo_data,
+					   DVB_FRONTEND_COMPONENT_TUNER,
+					   XC5000_TUNER_SET_DIGITAL_TV, 0);
+		}
 		break;
 	}
 
diff -r 1da5fed5c8b2 linux/drivers/media/common/tuners/xc5000.h
--- a/linux/drivers/media/common/tuners/xc5000.h	Sun Sep 19 02:23:09 2010 -0300
+++ b/linux/drivers/media/common/tuners/xc5000.h	Wed Oct 13 10:49:45 2010 +1000
@@ -35,6 +35,9 @@
 
 /* xc5000 callback command */
 #define XC5000_TUNER_RESET		0
+#define XC5000_TUNER_SET_RADIO		1
+#define XC5000_TUNER_SET_ANALOG_TV	2
+#define XC5000_TUNER_SET_DIGITAL_TV	3
 
 /* Possible Radio inputs */
 #define XC5000_RADIO_NOT_CONFIGURED		0
diff -r 1da5fed5c8b2 linux/drivers/media/video/saa7134/saa7134-cards.c
--- a/linux/drivers/media/video/saa7134/saa7134-cards.c	Sun Sep 19 02:23:09 2010 -0300
+++ b/linux/drivers/media/video/saa7134/saa7134-cards.c	Wed Oct 13 10:49:45 2010 +1000
@@ -6842,12 +6842,53 @@
 	case SAA7134_BOARD_BEHOLD_X7:
 	case SAA7134_BOARD_BEHOLD_H7:
 	case SAA7134_BOARD_BEHOLD_A7:
-		if (command == XC5000_TUNER_RESET) {
-		/* Down and UP pheripherial RESET pin for reset all chips */
+		switch (command) {
+		case XC5000_TUNER_RESET:
+			/* Down/UP pheripherial RESET pin for reset all chips */
 			saa_writeb(SAA7134_SPECIAL_MODE, 0x00);
 			msleep(10);
 			saa_writeb(SAA7134_SPECIAL_MODE, 0x01);
 			msleep(10);
+			break;
+		case XC5000_TUNER_SET_RADIO:
+			{
+			static u8 zl10353_ant2_enable[]  = { 0x63, 0x40 };
+			struct i2c_msg zl10353_msg = {.addr = 0x1e >> 1,
+					.flags = 0, .len = 2};
+
+			zl10353_msg.buf = zl10353_ant2_enable;
+
+			/* Switch RF to the ANT2 source */
+			if (i2c_transfer(&dev->i2c_adap, &zl10353_msg, 1) != 1) {
+				printk(KERN_INFO "could not access RF
+					 source control\n");
+				return -EIO;
+				}
+
+			msleep(10);
+			}
+			break;
+		case XC5000_TUNER_SET_ANALOG_TV:
+		case XC5000_TUNER_SET_DIGITAL_TV:
+			{
+			static u8 zl10353_ant1_enable[]  = { 0x63, 0x00 };
+			struct i2c_msg zl10353_msg = {.addr = 0x1e >> 1,
+					.flags = 0, .len = 2};
+
+			zl10353_msg.buf = zl10353_ant1_enable;
+
+			/* Switch RF to the ANT1 source */
+			if (i2c_transfer(&dev->i2c_adap, &zl10353_msg, 1) != 1) {
+				printk(KERN_INFO "could not access RF
+					 source control\n");
+				return -EIO;
+				}
+
+			msleep(10);
+			}
+			break;
+		default:
+			break;
 		}
 		break;
 	default:

Signed-off-by: Beholder Intl. Ltd. Dmitry Belimov <d.belimov@gmail.com>

Re: [PATCH] xc5000 and switch RF input

[Devin Heitmueller]

On Wed, Oct 13, 2010 at 5:30 PM, Dmitri Belimov <d.belimov@gmail.com> wrote:
> Hi
>
> Our TV card Behold X7 has two different RF input. This RF inputs can switch between
> different RF sources.
>
> ANT 1 for analog and digital TV
> ANT 2 for FM radio
>
> The switch controlled by zl10353.
>
> I add some defines for the tuner xc5000 and use tuner callback to saa7134 part.
> All works well. But my patch can touch other TV cards with xc5000.
>
> Devin can you check my changes on the other TV cards.
>
> With my best regards, Dmitry.

Hello Dmitri,

I've looked at the patch.  I really don't think this is the right
approach.  The tuner driver should not have any of this logic - it
should be in the bridge driver.  You can also look at Michael Krufky's
frontend override patches, which allow the bridge to intervene when
DVB frontend commands are made (for example, to toggle the antenna
before the tune is performed).

I understand the problem you are trying to solve, but jamming the
logic into the tuner driver really is a bad idea.

NACK.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] xc5000 and switch RF input

[Dmitri Belimov]

Hi

> On Wed, Oct 13, 2010 at 5:30 PM, Dmitri Belimov <d.belimov@gmail.com>
> wrote:
> > Hi
> >
> > Our TV card Behold X7 has two different RF input. This RF inputs
> > can switch between different RF sources.
> >
> > ANT 1 for analog and digital TV
> > ANT 2 for FM radio
> >
> > The switch controlled by zl10353.
> >
> > I add some defines for the tuner xc5000 and use tuner callback to
> > saa7134 part. All works well. But my patch can touch other TV cards
> > with xc5000.
> >
> > Devin can you check my changes on the other TV cards.
> >
> > With my best regards, Dmitry.
> 
> Hello Dmitri,
> 
> I've looked at the patch.  I really don't think this is the right
> approach.  The tuner driver should not have any of this logic - it
> should be in the bridge driver.  You can also look at Michael Krufky's
> frontend override patches, which allow the bridge to intervene when
> DVB frontend commands are made (for example, to toggle the antenna
> before the tune is performed).

Ok.

> I understand the problem you are trying to solve, but jamming the
> logic into the tuner driver really is a bad idea.
> 
> NACK.
> 
> Devin
> 
> -- 
> Devin J. Heitmueller - Kernel Labs
> http://www.kernellabs.com

Ok.

With my best regards, Dmitry.

Re: [PATCH] xc5000 and switch RF input

[hermann pitton]

Am Donnerstag, den 14.10.2010, 12:12 -0400 schrieb Dmitri Belimov:
> Hi
> 
> > On Wed, Oct 13, 2010 at 5:30 PM, Dmitri Belimov <d.belimov@gmail.com>
> > wrote:
> > > Hi
> > >
> > > Our TV card Behold X7 has two different RF input. This RF inputs
> > > can switch between different RF sources.
> > >
> > > ANT 1 for analog and digital TV
> > > ANT 2 for FM radio
> > >
> > > The switch controlled by zl10353.
> > >
> > > I add some defines for the tuner xc5000 and use tuner callback to
> > > saa7134 part. All works well. But my patch can touch other TV cards
> > > with xc5000.
> > >
> > > Devin can you check my changes on the other TV cards.
> > >
> > > With my best regards, Dmitry.
> > 
> > Hello Dmitri,
> > 
> > I've looked at the patch.  I really don't think this is the right
> > approach.  The tuner driver should not have any of this logic - it
> > should be in the bridge driver.  You can also look at Michael Krufky's
> > frontend override patches, which allow the bridge to intervene when
> > DVB frontend commands are made (for example, to toggle the antenna
> > before the tune is performed).
> 
> Ok.
> 
> > I understand the problem you are trying to solve, but jamming the
> > logic into the tuner driver really is a bad idea.
> > 
> > NACK.
> > 
> > Devin
> > 
> > -- 
> > Devin J. Heitmueller - Kernel Labs
> > http://www.kernellabs.com
> 
> Ok.
> 
> With my best regards, Dmitry.
> 
> --

Dmitry,

please adjust your timezone somehow better.

I do read the stuff only in backlash mode currently,

but it is annoying to have you always in the future and real time
relations are broken within all the other stuff coming in.

Cheers,
Hermann

[PATCH] saa7134 behold A7 and H7

[Dmitri Belimov]

[-- Attachment #1: Type: text/plain, Size: 1721 bytes --]

Hi

Fix autodetect for Behold A7 and H7 TV cards.

diff -r abd3aac6644e linux/drivers/media/video/saa7134/saa7134-cards.c
--- a/linux/drivers/media/video/saa7134/saa7134-cards.c	Fri Jul 02 00:38:54 2010 -0300
+++ b/linux/drivers/media/video/saa7134/saa7134-cards.c	Mon Oct 25 09:16:24 2010 +1000
@@ -6700,6 +6700,18 @@
 		.subdevice    = 0x2804,
 		.driver_data  = SAA7134_BOARD_TECHNOTREND_BUDGET_T3000,
 	}, {
+		.vendor       = PCI_VENDOR_ID_PHILIPS,
+		.device       = PCI_DEVICE_ID_PHILIPS_SAA7133,
+		.subvendor    = 0x5ace, /* Beholder Intl. Ltd. */
+		.subdevice    = 0x7190,
+		.driver_data  = SAA7134_BOARD_BEHOLD_H7,
+	}, {
+		.vendor       = PCI_VENDOR_ID_PHILIPS,
+		.device       = PCI_DEVICE_ID_PHILIPS_SAA7133,
+		.subvendor    = 0x5ace, /* Beholder Intl. Ltd. */
+		.subdevice    = 0x7090,
+		.driver_data  = SAA7134_BOARD_BEHOLD_A7,
+	}, {
 		/* --- boards without eeprom + subsystem ID --- */
 		.vendor       = PCI_VENDOR_ID_PHILIPS,
 		.device       = PCI_DEVICE_ID_PHILIPS_SAA7134,
@@ -6737,18 +6749,6 @@
 		.subvendor    = PCI_ANY_ID,
 		.subdevice    = PCI_ANY_ID,
 		.driver_data  = SAA7134_BOARD_UNKNOWN,
-	}, {
-		.vendor       = PCI_VENDOR_ID_PHILIPS,
-		.device       = PCI_DEVICE_ID_PHILIPS_SAA7133,
-		.subvendor    = 0x5ace, /* Beholder Intl. Ltd. */
-		.subdevice    = 0x7190,
-		.driver_data  = SAA7134_BOARD_BEHOLD_H7,
-	}, {
-		.vendor       = PCI_VENDOR_ID_PHILIPS,
-		.device       = PCI_DEVICE_ID_PHILIPS_SAA7133,
-		.subvendor    = 0x5ace, /* Beholder Intl. Ltd. */
-		.subdevice    = 0x7090,
-		.driver_data  = SAA7134_BOARD_BEHOLD_A7,
 	},{
 		/* --- end of list --- */
 	}

Signed-off-by: Beholder Intl. Ltd. Dmitry Belimov <d.belimov@gmail.com>


With my best regards, Dmitry.

[-- Attachment #2: behold_a7_h7_fix.patch --]
[-- Type: text/x-patch, Size: 1638 bytes --]

diff -r abd3aac6644e linux/drivers/media/video/saa7134/saa7134-cards.c
--- a/linux/drivers/media/video/saa7134/saa7134-cards.c	Fri Jul 02 00:38:54 2010 -0300
+++ b/linux/drivers/media/video/saa7134/saa7134-cards.c	Mon Oct 25 09:16:24 2010 +1000
@@ -6700,6 +6700,18 @@
 		.subdevice    = 0x2804,
 		.driver_data  = SAA7134_BOARD_TECHNOTREND_BUDGET_T3000,
 	}, {
+		.vendor       = PCI_VENDOR_ID_PHILIPS,
+		.device       = PCI_DEVICE_ID_PHILIPS_SAA7133,
+		.subvendor    = 0x5ace, /* Beholder Intl. Ltd. */
+		.subdevice    = 0x7190,
+		.driver_data  = SAA7134_BOARD_BEHOLD_H7,
+	}, {
+		.vendor       = PCI_VENDOR_ID_PHILIPS,
+		.device       = PCI_DEVICE_ID_PHILIPS_SAA7133,
+		.subvendor    = 0x5ace, /* Beholder Intl. Ltd. */
+		.subdevice    = 0x7090,
+		.driver_data  = SAA7134_BOARD_BEHOLD_A7,
+	}, {
 		/* --- boards without eeprom + subsystem ID --- */
 		.vendor       = PCI_VENDOR_ID_PHILIPS,
 		.device       = PCI_DEVICE_ID_PHILIPS_SAA7134,
@@ -6737,18 +6749,6 @@
 		.subvendor    = PCI_ANY_ID,
 		.subdevice    = PCI_ANY_ID,
 		.driver_data  = SAA7134_BOARD_UNKNOWN,
-	}, {
-		.vendor       = PCI_VENDOR_ID_PHILIPS,
-		.device       = PCI_DEVICE_ID_PHILIPS_SAA7133,
-		.subvendor    = 0x5ace, /* Beholder Intl. Ltd. */
-		.subdevice    = 0x7190,
-		.driver_data  = SAA7134_BOARD_BEHOLD_H7,
-	}, {
-		.vendor       = PCI_VENDOR_ID_PHILIPS,
-		.device       = PCI_DEVICE_ID_PHILIPS_SAA7133,
-		.subvendor    = 0x5ace, /* Beholder Intl. Ltd. */
-		.subdevice    = 0x7090,
-		.driver_data  = SAA7134_BOARD_BEHOLD_A7,
 	},{
 		/* --- end of list --- */
 	}

Signed-off-by: Beholder Intl. Ltd. Dmitry Belimov <d.belimov@gmail.com>