xc5000, rework xc_write_reg

Message ID 20100518173011.5d9c7f2c@glory.loctelecom.ru (mailing list archive)
State Superseded, archived
Headers

Commit Message

Dmitri Belimov May 18, 2010, 7:30 a.m. UTC
  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.


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


With my best regards, Dmitry.
  

Comments

Devin Heitmueller May 24, 2010, 2:38 a.m. UTC | #1
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
  
Dmitri Belimov May 25, 2010, 1:49 a.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Mauro Carvalho Chehab July 5, 2010, 4:11 p.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Devin Heitmueller July 5, 2010, 7:07 p.m. UTC | #4
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
  
Mauro Carvalho Chehab Oct. 6, 2010, 12:25 p.m. UTC | #5
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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Mauro Carvalho Chehab Oct. 6, 2010, 12:40 p.m. UTC | #6
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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Dmitri Belimov Oct. 6, 2010, 7:52 p.m. UTC | #7
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Mauro Carvalho Chehab Oct. 7, 2010, 12:28 a.m. UTC | #8
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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Dmitri Belimov Oct. 7, 2010, 1 p.m. UTC | #9
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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Dmitri Belimov Oct. 12, 2010, 6:32 p.m. UTC | #10
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Antti Palosaari Oct. 14, 2010, 8:33 a.m. UTC | #11
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
  
Devin Heitmueller Oct. 14, 2010, 8:41 a.m. UTC | #12
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
  
Antti Palosaari Oct. 15, 2010, 3:33 p.m. UTC | #13
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
  

Patch

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>