xc5000, rework xc_write_reg

PATCH In review Dmitri Belimov 5 participants 19 messages May 18, 2010 – Oct 26, 2010 Media (V4L/DVB)

lore

Share & tools

lore

Dmitri Belimov

author

2010-05-18 07:29 UTC #0 lore raw

PATCH 1/1 xc5000, rework xc_write_reg

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.

linux/drivers/media/common/tuners/xc5000.c +26 ~~-31~~

diff -r 8f5129efe974 linux/drivers/media/common/tuners/xc5000.c
@@ -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 <[email protected]>

With my best regards, Dmitry.

linux/drivers/media/common/tuners/xc5000.c +26 ~~-31~~

77 lines not on this page · raw message

Signed-off-by Beholder Intl. Ltd. Dmitry Belimov <[email protected]>

Devin Heitmueller

2010-05-24 02:38 UTC #1 lore raw

Quoted 6 lines · Dmitri Belimov

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

Signature · 2 lines

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

Dmitri Belimov

author

2010-05-25 01:49 UTC #2 lore raw

Hi Devin

Quoted 19 lines

Quoted 6 lines · Dmitri Belimov
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).

Quoted 4 lines

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.

Quoted 5 lines

Devin

Signature · 2 lines

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

Mauro Carvalho Chehab

2010-07-05 16:12 UTC #3 lore raw

Em 24-05-2010 22:49, Dmitri Belimov escreveu:

Quoted 21 lines

Hi Devin
Quoted 19 lines
Quoted 6 lines · Dmitri Belimov
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).

Quoted 8 lines

Quoted 4 lines
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

Devin Heitmueller

Acked-by

2010-07-05 19:07 UTC #4 lore raw

Quoted 6 lines · Mauro Carvalho Chehab

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 <[email protected]>

Thanks,

Devin

Signature · 2 lines

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

Dmitri Belimov

author

2010-10-06 05:52 UTC #5 lore raw

xc5000 and switch RF input

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.

Mauro Carvalho Chehab

2010-10-06 12:26 UTC #6 lore raw

Re: xc5000 and switch RF input

Em 06-10-2010 16:52, Dmitri Belimov escreveu:

Quoted 21 lines

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.

Quoted 2 lines

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

Mauro Carvalho Chehab

2010-10-06 12:40 UTC #7 lore raw

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

In reply to Dmitri Belimov

Em 06-10-2010 16:52, Dmitri Belimov escreveu:

Quoted 21 lines

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.

Quoted 2 lines

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

Dmitri Belimov

author

2010-10-06 23:00 UTC #8 lore raw

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

Quoted 99 lines

Em 06-10-2010 16:52, Dmitri Belimov escreveu:
Quoted 23 lines
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.
Quoted 3 lines
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

Quoted 7 lines

	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.

Quoted 2 lines

Thanks,
Mauro

Mauro Carvalho Chehab

2010-10-07 00:28 UTC #9 lore raw

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

Em 07-10-2010 10:00, Dmitri Belimov escreveu:

Quoted 104 lines

Hi
Quoted 102 lines
Em 06-10-2010 16:52, Dmitri Belimov escreveu:
Quoted 23 lines
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.
Quoted 3 lines
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);

Quoted 7 lines

Quoted 4 lines
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

Dmitri Belimov

author

2010-10-12 04:31 UTC #10 lore raw

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

Hi Mauro

Quoted 110 lines

Em 07-10-2010 10:00, Dmitri Belimov escreveu:
Quoted 109 lines
Hi
Quoted 106 lines
Em 06-10-2010 16:52, Dmitri Belimov escreveu:
Quoted 23 lines
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.
Quoted 3 lines
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?

Quoted 8 lines

Quoted 2 lines
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 :)

Quoted 2 lines

Cheers,
Mauro

With my best regards, Dmitry.

Antti Palosaari

2010-10-14 08:33 UTC #11 lore raw

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

In reply to Mauro Carvalho Chehab

Quoted 5 lines · Mauro Carvalho Chehab

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:

Quoted 7 lines

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

Signature · 1 line

http://palosaari.fi/

Devin Heitmueller

2010-10-14 08:41 UTC #12 lore raw

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

Quoted 10 lines · Antti Palosaari

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

Quoted 5 lines

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

Signature · 2 lines

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

Antti Palosaari

2010-10-15 15:34 UTC #13 lore raw

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

Quoted 17 lines · Devin Heitmueller

Quoted 10 lines · Antti Palosaari
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
Quoted 5 lines
traditional I2C send to the tuner:
TT>> B0 B1 B2 ...
demod as repeater send to the tuner:
DD>> TT B0 B1 B2 ...

Quoted 6 lines

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

Signature · 1 line

http://palosaari.fi/

Dmitri Belimov

author

2010-10-13 07:29 UTC #14 lore raw

PATCH 1/1 xc5000 and switch RF input

In reply to Devin Heitmueller

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.

linux/drivers/media/common/tuners/xc5000.c +23 -0

diff -r 1da5fed5c8b2 linux/drivers/media/common/tuners/xc5000.c
@@ -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;
 	}

linux/drivers/media/common/tuners/xc5000.h +3 -0

diff -r 1da5fed5c8b2 linux/drivers/media/common/tuners/xc5000.h
@@ -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

linux/drivers/media/video/saa7134/saa7134-cards.c +43 -2

55 lines not on this page · raw message

Signed-off-by Beholder Intl. Ltd. Dmitry Belimov <[email protected]>

Devin Heitmueller

NAK

2010-10-14 00:50 UTC #15 lore raw

Re: [PATCH] xc5000 and switch RF input

Quoted 8 lines · 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

Quoted 8 lines

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

Signature · 2 lines

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

Dmitri Belimov

author

2010-10-14 02:11 UTC #16 lore raw

Re: [PATCH] xc5000 and switch RF input

Quoted 20 lines

Quoted 10 lines · 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.
Quoted 7 lines
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.

Quoted 8 lines

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.

Quoted 10 lines

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

Ok.

With my best regards, Dmitry.

hermann pitton

2010-10-14 19:27 UTC #17 lore raw

Re: [PATCH] xc5000 and switch RF input

Am Donnerstag, den 14.10.2010, 12:12 -0400 schrieb Dmitri Belimov:

Quoted 44 lines

Hi
Quoted 20 lines
Quoted 10 lines · 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.
Quoted 7 lines
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.
Quoted 8 lines
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.
Quoted 10 lines
I understand the problem you are trying to solve, but jamming the logic into the tuner driver really is a bad idea.
NACK.
Devin
Signature · 2 lines
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Quoted 5 lines

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

Dmitri Belimov

author

2010-10-26 03:32 UTC #18 lore raw

PATCH 1/1 saa7134 behold A7 and H7

In reply to Devin Heitmueller

Fix autodetect for Behold A7 and H7 TV cards.

linux/drivers/media/video/saa7134/saa7134-cards.c +12 ~~-12~~

diff -r abd3aac6644e linux/drivers/media/video/saa7134/saa7134-cards.c
@@ -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 <[email protected]>

With my best regards, Dmitry.

linux/drivers/media/video/saa7134/saa7134-cards.c +12 ~~-12~~

diff -r abd3aac6644e linux/drivers/media/video/saa7134/saa7134-cards.c
@@ -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 <[email protected]>