[RFC] TDA8290 / TDA827X with LNA: testers wanted

[RFC] TDA8290 / TDA827X with LNA: testers wanted

[Hartmut Hackmann]

Hi, Folks

Currently, the LNA support code for TDA8275a is broken, it may even cause a kernel oops.
The bugs were introduced during tuner refactoring.
In my personal repository at
  http://linuxtv.org/hg/~hhackmann/v4l-dvb/
these bugs hopefully are fixed. But i can test only 3 cases. So i am looking for owners
of the cards
Pinnacle 310i,
Happauge hvr1110
ASUSTeK P7131 with LNA
MSI TV@NYWHERE AD11
KWORLD DVBT 210
to check whether things are right again. This holds for both, analog as well as DVB-T.

Michael, may i ask you to check whether my changes contradict with things you are doing?
Mauro, what's your opinion on this? As far as i know, the broken code is in the upcoming
kernel release. The patch is big, is there a chance to commit it to the kernel?

Best regards
  Hartmut

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Michael Krufky]

On Tue, Mar 18, 2008 at 8:40 PM, Hartmut Hackmann
<hartmut.hackmann@t-online.de> wrote:
> Hi, Folks
>
>  Currently, the LNA support code for TDA8275a is broken, it may even cause a kernel oops.
>  The bugs were introduced during tuner refactoring.
>  In my personal repository at
>   http://linuxtv.org/hg/~hhackmann/v4l-dvb/
>  these bugs hopefully are fixed. But i can test only 3 cases. So i am looking for owners
>  of the cards
>  Pinnacle 310i,
>  Happauge hvr1110
>  ASUSTeK P7131 with LNA
>  MSI TV@NYWHERE AD11
>  KWORLD DVBT 210
>  to check whether things are right again. This holds for both, analog as well as DVB-T.
>
>  Michael, may i ask you to check whether my changes contradict with things you are doing?
>  Mauro, what's your opinion on this? As far as i know, the broken code is in the upcoming
>  kernel release. The patch is big, is there a chance to commit it to the kernel?

Hartmut,

I've already checked over your changes (I noticed the tree pushed a
few days ago)  -- I do not see any reason why they would cause any
problems on the tda8295 nor the tda18271.  These changes will not
contradict anything that I am currently working on, and it would be
nice to have them merged sooner than later.

There is only one detail that I would like to point out.  You made the
following change:

--- a/linux/drivers/media/video/tda8290.h	Mon Mar 03 22:55:05 2008 +0100
+++ b/linux/drivers/media/video/tda8290.h	Sun Mar 16 23:49:43 2008 +0100
@@ -21,7 +21,7 @@
 #include "dvb_frontend.h"

 struct tda829x_config {
-	unsigned int *lna_cfg;
+	unsigned int lna_cfg;
 	int (*tuner_callback) (void *dev, int command, int arg);

 	unsigned int probe_tuner:1;
--- a/linux/drivers/media/video/tuner-core.c	Mon Mar 03 22:55:05 2008 +0100
+++ b/linux/drivers/media/video/tuner-core.c	Sun Mar 16 23:49:43 2008 +0100
@@ -349,7 +349,7 @@ static void attach_tda829x(struct tuner
 static void attach_tda829x(struct tuner *t)
 {
 	struct tda829x_config cfg = {
-		.lna_cfg        = &t->config,
+		.lna_cfg        = t->config,
 		.tuner_callback = t->tuner_callback,
 	};
 	tda829x_attach(&t->fe, t->i2c->adapter, t->i2c->addr, &cfg);



...The above change means that the lna setting is set at tuner driver
attach time, and if somebody wants to use TUNER_SET_CONFIG (or some
other method) to enable / disable the LNA on-the-fly, it will not be
possible.    Meanwhile, I'm not even sure it that was possible to
begin with.  Just some food for thought -- you should decide what is
best, here.

I have an HVR1110, and I have a QAM64 generator that I use to test it.
 Obviously, it is a hot signal.  Is it possible for me to test the LNA
under these circumstances?  ...or do we need somebody "out in the
field" to do that sort of test?  (I live in ATSC-land ;-) )

You mentioned a possible kernel OOPS.  Have you actually experienced
an OOPS with the current tree?  I apologize if this feature being
broken is the result of my tuner refactoring.  I appreciate your
taking the time to fix it.

Regards,

Mike

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [linux-dvb] [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Hartmut Hackmann]

Hi, Mike

Michael Krufky schrieb:
> On Tue, Mar 18, 2008 at 8:40 PM, Hartmut Hackmann
> <hartmut.hackmann@t-online.de> wrote:
>> Hi, Folks
>>
>>  Currently, the LNA support code for TDA8275a is broken, it may even cause a kernel oops.
>>  The bugs were introduced during tuner refactoring.
>>  In my personal repository at
>>   http://linuxtv.org/hg/~hhackmann/v4l-dvb/
>>  these bugs hopefully are fixed. But i can test only 3 cases. So i am looking for owners
>>  of the cards
>>  Pinnacle 310i,
>>  Happauge hvr1110
>>  ASUSTeK P7131 with LNA
>>  MSI TV@NYWHERE AD11
>>  KWORLD DVBT 210
>>  to check whether things are right again. This holds for both, analog as well as DVB-T.
>>
>>  Michael, may i ask you to check whether my changes contradict with things you are doing?
>>  Mauro, what's your opinion on this? As far as i know, the broken code is in the upcoming
>>  kernel release. The patch is big, is there a chance to commit it to the kernel?
> 
> Hartmut,
> 
> I've already checked over your changes (I noticed the tree pushed a
> few days ago)  -- I do not see any reason why they would cause any
> problems on the tda8295 nor the tda18271.  These changes will not
> contradict anything that I am currently working on, and it would be
> nice to have them merged sooner than later.
> 
> There is only one detail that I would like to point out.  You made the
> following change:
> 
> --- a/linux/drivers/media/video/tda8290.h	Mon Mar 03 22:55:05 2008 +0100
> +++ b/linux/drivers/media/video/tda8290.h	Sun Mar 16 23:49:43 2008 +0100
> @@ -21,7 +21,7 @@
>  #include "dvb_frontend.h"
> 
>  struct tda829x_config {
> -	unsigned int *lna_cfg;
> +	unsigned int lna_cfg;
>  	int (*tuner_callback) (void *dev, int command, int arg);
> 
>  	unsigned int probe_tuner:1;
> --- a/linux/drivers/media/video/tuner-core.c	Mon Mar 03 22:55:05 2008 +0100
> +++ b/linux/drivers/media/video/tuner-core.c	Sun Mar 16 23:49:43 2008 +0100
> @@ -349,7 +349,7 @@ static void attach_tda829x(struct tuner
>  static void attach_tda829x(struct tuner *t)
>  {
>  	struct tda829x_config cfg = {
> -		.lna_cfg        = &t->config,
> +		.lna_cfg        = t->config,
>  		.tuner_callback = t->tuner_callback,
>  	};
>  	tda829x_attach(&t->fe, t->i2c->adapter, t->i2c->addr, &cfg);
> 
> 
> 
> ...The above change means that the lna setting is set at tuner driver
> attach time, and if somebody wants to use TUNER_SET_CONFIG (or some
> other method) to enable / disable the LNA on-the-fly, it will not be
> possible.    Meanwhile, I'm not even sure it that was possible to
> begin with.  Just some food for thought -- you should decide what is
> best, here.
> 
Ah, That's your intention.. I changed this because in the case of DVB
there is nothing that creates a copy of the tda827x config structure -
so no legal way to change the data. And imho it is not necessary to change
this dynamically.

> I have an HVR1110, and I have a QAM64 generator that I use to test it.
>  Obviously, it is a hot signal.  Is it possible for me to test the LNA
> under these circumstances?  ...or do we need somebody "out in the
> field" to do that sort of test?  (I live in ATSC-land ;-) )
>
You should be able to. You need to have the debug option for the tuner on
and you need to be aware that the decicion LNA on / off is taken only once
while tuning. When you modify the RF level you should notice that increasing
the amplitude results in a lower AGC2 value. When it reaches the value 2, the
driver should report that it turns the LNA to low gain. You will also need to
monitor the AGC value of the channel decoder to see the effect.

> You mentioned a possible kernel OOPS.  Have you actually experienced
> an OOPS with the current tree?  I apologize if this feature being
> broken is the result of my tuner refactoring.  I appreciate your
> taking the time to fix it.
> 
Yes. The first parameter to the tuner callback was wrong and cause a reference
to a NULL pointer.

Best regards
 Hartmut

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [linux-dvb] [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Michael Krufky]

Hello Hartmut,

Hartmut Hackmann wrote:
> Michael Krufky schrieb:
>> I have an HVR1110, and I have a QAM64 generator that I use to test it.
>>  Obviously, it is a hot signal.  Is it possible for me to test the LNA
>> under these circumstances?  ...or do we need somebody "out in the
>> field" to do that sort of test?  (I live in ATSC-land ;-) )
>>
> You should be able to. You need to have the debug option for the tuner on
> and you need to be aware that the decicion LNA on / off is taken only once
> while tuning. When you modify the RF level you should notice that increasing
> the amplitude results in a lower AGC2 value. When it reaches the value 2, the
> driver should report that it turns the LNA to low gain. You will also need to
> monitor the AGC value of the channel decoder to see the effect.

I'll give this a try when I have some time, and send you the logs.

>> You mentioned a possible kernel OOPS.  Have you actually experienced
>> an OOPS with the current tree?  I apologize if this feature being
>> broken is the result of my tuner refactoring.  I appreciate your
>> taking the time to fix it.
>>
> Yes. The first parameter to the tuner callback was wrong and cause a reference
> to a NULL pointer.

Ah, I believe this may have been caused by a very recent changeset:

http://linuxtv.org/hg/v4l-dvb/rev/ad6fb7fe6240

I tested all of my changes thoroughly, and had received positive test results from other users with various hardware.
The regression was not part of my tuner refactoring changes, and I do not think that this is upstream in 2.6.25-rc.

Can you confirm whether or not the above is the problem changeset?

Regards,

Mike

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [linux-dvb] [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Hartmut Hackmann]

Hi, Michael

Michael Krufky schrieb:
> Hello Hartmut,
> 
> Hartmut Hackmann wrote:
>> Michael Krufky schrieb:
>>> I have an HVR1110, and I have a QAM64 generator that I use to test it.
>>>  Obviously, it is a hot signal.  Is it possible for me to test the LNA
>>> under these circumstances?  ...or do we need somebody "out in the
>>> field" to do that sort of test?  (I live in ATSC-land ;-) )
>>>
>> You should be able to. You need to have the debug option for the tuner on
>> and you need to be aware that the decicion LNA on / off is taken only once
>> while tuning. When you modify the RF level you should notice that increasing
>> the amplitude results in a lower AGC2 value. When it reaches the value 2, the
>> driver should report that it turns the LNA to low gain. You will also need to
>> monitor the AGC value of the channel decoder to see the effect.
> 
> I'll give this a try when I have some time, and send you the logs.
> 
By the way: I once heard that Hauppauge has cards with LNA, but uses a different
configuration. Did you ever notice "strange" effects in analog mode like
a small moving horizontal bar while locking?

>>> You mentioned a possible kernel OOPS.  Have you actually experienced
>>> an OOPS with the current tree?  I apologize if this feature being
>>> broken is the result of my tuner refactoring.  I appreciate your
>>> taking the time to fix it.
>>>
>> Yes. The first parameter to the tuner callback was wrong and cause a reference
>> to a NULL pointer.
> 
> Ah, I believe this may have been caused by a very recent changeset:
> 
> http://linuxtv.org/hg/v4l-dvb/rev/ad6fb7fe6240
> 
> I tested all of my changes thoroughly, and had received positive test results from other users with various hardware.
> The regression was not part of my tuner refactoring changes, and I do not think that this is upstream in 2.6.25-rc.
> 
> Can you confirm whether or not the above is the problem changeset?
> 
> Regards,
> 
> Mike
> 
Yes, that one caused the problem (from static analysis). This patch has some
inconsistencies that were grinded out later but this was the starting point.

If this code is not in 2.6.25-rc: good!! It leaves us time for testing and
discussion.
Do you think we need further discussion on my tuner config pointer change?

Best regards
  Hartmut

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [linux-dvb] [RFC] TDA8290 / TDA827X with LNA: testers wanted

[mkrufky]

Hartmut Hackmann wrote:
> Hi, Michael
>
> Michael Krufky schrieb:
>   
>> Hello Hartmut,
>>
>> Hartmut Hackmann wrote:
>>     
>>> Michael Krufky schrieb:
>>>       
>>>> I have an HVR1110, and I have a QAM64 generator that I use to test it.
>>>>  Obviously, it is a hot signal.  Is it possible for me to test the LNA
>>>> under these circumstances?  ...or do we need somebody "out in the
>>>> field" to do that sort of test?  (I live in ATSC-land ;-) )
>>>>
>>>>         
>>> You should be able to. You need to have the debug option for the tuner
on
>>> and you need to be aware that the decicion LNA on / off is taken only
once
>>> while tuning. When you modify the RF level you should notice that
increasing
>>> the amplitude results in a lower AGC2 value. When it reaches the value
2, the
>>> driver should report that it turns the LNA to low gain. You will also
need to
>>> monitor the AGC value of the channel decoder to see the effect.
>>>       
>> I'll give this a try when I have some time, and send you the logs.
>>
>>     
> By the way: I once heard that Hauppauge has cards with LNA, but uses a
different
> configuration. Did you ever notice "strange" effects in analog mode like
> a small moving horizontal bar while locking?
>
>   
>>>> You mentioned a possible kernel OOPS.  Have you actually experienced
>>>> an OOPS with the current tree?  I apologize if this feature being
>>>> broken is the result of my tuner refactoring.  I appreciate your
>>>> taking the time to fix it.
>>>>
>>>>         
>>> Yes. The first parameter to the tuner callback was wrong and cause a
reference
>>> to a NULL pointer.
>>>       
>> Ah, I believe this may have been caused by a very recent changeset:
>>
>> http://linuxtv.org/hg/v4l-dvb/rev/ad6fb7fe6240
>>
>> I tested all of my changes thoroughly, and had received positive test
results from other users with various hardware.
>> The regression was not part of my tuner refactoring changes, and I do not
think that this is upstream in 2.6.25-rc.
>>
>> Can you confirm whether or not the above is the problem changeset?
>>
>> Regards,
>>
>> Mike
>>
>>     
> Yes, that one caused the problem (from static analysis). This patch has
some
> inconsistencies that were grinded out later but this was the starting
point.
>
> If this code is not in 2.6.25-rc: good!! It leaves us time for testing and
> discussion.
> Do you think we need further discussion on my tuner config pointer change?
>
> Best regards
>   Hartmut
>   
Hartmut,

I don't think we need further discussion -- When I did the refactoring, 
I was only trying to preserve your functionality while adding new 
functionality for the new silicon.

I trust you as the authority on how the LNA should work, and I think 
that we should merge your changes into the master branch as soon as 
possible.

I'm glad to hear you confirm that Mauro's recent patch is the one that 
broke the driver.  This patch of his is not in 2.6.25-rc, but is planned 
for 2.6.26 -- for this reason, we should merge in your changes now and 
establish a known point of functionality.

As far as the HVR1110, I do not see such issues in the analog video.  I 
will not have time to conduct the LNA test until next week, the earliest.

Please ask Mauro to merge it asap.  Let him know that this is *not* 
urgent for 2.6.25, because the offending changeset is not yet upstream, 
but we do need this in master now for the sake of testing and good 
housekeeping.

Regards,

Mike

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [linux-dvb] [RFC] TDA8290 / TDA827X with LNA: testers wanted

[mkrufky]

Hartmut Hackmann wrote:
> Hi, Michael
>
> Michael Krufky schrieb:
>   
>> Hello Hartmut,
>>
>> Hartmut Hackmann wrote:
>>     
>>> Michael Krufky schrieb:
>>>       
>>>> I have an HVR1110, and I have a QAM64 generator that I use to test it.
>>>>  Obviously, it is a hot signal.  Is it possible for me to test the LNA
>>>> under these circumstances?  ...or do we need somebody "out in the
>>>> field" to do that sort of test?  (I live in ATSC-land ;-) )
>>>>
>>>>         
>>> You should be able to. You need to have the debug option for the tuner
on
>>> and you need to be aware that the decicion LNA on / off is taken only
once
>>> while tuning. When you modify the RF level you should notice that
increasing
>>> the amplitude results in a lower AGC2 value. When it reaches the value
2, the
>>> driver should report that it turns the LNA to low gain. You will also
need to
>>> monitor the AGC value of the channel decoder to see the effect.
>>>       
>> I'll give this a try when I have some time, and send you the logs.
>>
>>     
> By the way: I once heard that Hauppauge has cards with LNA, but uses a
different
> configuration. Did you ever notice "strange" effects in analog mode like
> a small moving horizontal bar while locking?
>
>   
>>>> You mentioned a possible kernel OOPS.  Have you actually experienced
>>>> an OOPS with the current tree?  I apologize if this feature being
>>>> broken is the result of my tuner refactoring.  I appreciate your
>>>> taking the time to fix it.
>>>>
>>>>         
>>> Yes. The first parameter to the tuner callback was wrong and cause a
reference
>>> to a NULL pointer.
>>>       
>> Ah, I believe this may have been caused by a very recent changeset:
>>
>> http://linuxtv.org/hg/v4l-dvb/rev/ad6fb7fe6240
>>
>> I tested all of my changes thoroughly, and had received positive test
results from other users with various hardware.
>> The regression was not part of my tuner refactoring changes, and I do not
think that this is upstream in 2.6.25-rc.
>>
>> Can you confirm whether or not the above is the problem changeset?
>>
>> Regards,
>>
>> Mike
>>
>>     
> Yes, that one caused the problem (from static analysis). This patch has
some
> inconsistencies that were grinded out later but this was the starting
point.
>
> If this code is not in 2.6.25-rc: good!! It leaves us time for testing and
> discussion.
> Do you think we need further discussion on my tuner config pointer change?
>
> Best regards
>   Hartmut
>   
Hartmut,

I don't think we need further discussion -- When I did the refactoring, 
I was only trying to preserve your functionality while adding new 
functionality for the new silicon.

I trust you as the authority on how the LNA should work, and I think 
that we should merge your changes into the master branch as soon as 
possible.

I'm glad to hear you confirm that Mauro's recent patch is the one that 
broke the driver.  This patch of his is not in 2.6.25-rc, but is planned 
for 2.6.26 -- for this reason, we should merge in your changes now and 
establish a known point of functionality.

As far as the HVR1110, I do not see such issues in the analog video.  I 
will not have time to conduct the LNA test until next week, the earliest.

Please ask Mauro to merge it asap.  Let him know that this is *not* 
urgent for 2.6.25, because the offending changeset is not yet upstream, 
but we do need this in master now for the sake of testing and good 
housekeeping.

As for your changeset:

Reviewed-by: Michael Krufky <mkrufky@linuxtv.org>

Regards,

Mike

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Mauro Carvalho Chehab]

On Wed, 19 Mar 2008, Hartmut Hackmann wrote:

> Mauro, what's your opinion on this? As far as i know, the broken code is in the upcoming
> kernel release. The patch is big, is there a chance to commit it to the kernel?

While some fixes are cosmetic (like __func__ change), and others are just 
function reordering, I suspect that the real changes are still too big for 
-rc6. It will probably be nacked.

Yet, it may be worthy to try.

I still need to send a patchset to Linus, after testing compilation 
(unfortunately, I had to postpone, since I need first to free some 
hundreds of Mb on my HD on my /home, to allow kernel compilation). 
Hopefully, I'll have some time tomorrow for doing a "housekeeping".

Cheers,
Mauro

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Hartmut Hackmann]

Hi, Mauro

Mauro Carvalho Chehab schrieb:
> On Wed, 19 Mar 2008, Hartmut Hackmann wrote:
> 
>> Mauro, what's your opinion on this? As far as i know, the broken code
>> is in the upcoming
>> kernel release. The patch is big, is there a chance to commit it to
>> the kernel?
> 
> While some fixes are cosmetic (like __func__ change), and others are
> just function reordering, I suspect that the real changes are still too
> big for -rc6. It will probably be nacked.
> 
> Yet, it may be worthy to try.

This was my opinion as well.
Did you notice Michaels reply on this issue? He pointed out that the problem
was introduced by this changeset:
http://linuxtv.org/hg/v4l-dvb/rev/ad6fb7fe6240 : Add support for xc3028-based boards

If this did not go to Linus yet, we don't have a problem. This also explains
why we don't have bug reports on this.

> 
> I still need to send a patchset to Linus, after testing compilation
> (unfortunately, I had to postpone, since I need first to free some
> hundreds of Mb on my HD on my /home, to allow kernel compilation).
> Hopefully, I'll have some time tomorrow for doing a "housekeeping".
> 
Unfortunately, i deleted you mails describing what went to linux and i don't
have the RC source here :-(

Best regards
  Hartmut

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Mauro Carvalho Chehab]

On Wed, 19 Mar 2008 23:16:47 +0100
Hartmut Hackmann <hartmut.hackmann@t-online.de> wrote:

> Hi, Mauro
> 
> Mauro Carvalho Chehab schrieb:
> > On Wed, 19 Mar 2008, Hartmut Hackmann wrote:
> > 
> >> Mauro, what's your opinion on this? As far as i know, the broken code
> >> is in the upcoming
> >> kernel release. The patch is big, is there a chance to commit it to
> >> the kernel?
> > 
> > While some fixes are cosmetic (like __func__ change), and others are
> > just function reordering, I suspect that the real changes are still too
> > big for -rc6. It will probably be nacked.
> > 
> > Yet, it may be worthy to try.
> 
> This was my opinion as well.
> Did you notice Michaels reply on this issue? He pointed out that the problem
> was introduced by this changeset:
> http://linuxtv.org/hg/v4l-dvb/rev/ad6fb7fe6240 : Add support for xc3028-based boards
> 
> If this did not go to Linus yet, we don't have a problem. This also explains
> why we don't have bug reports on this.

It didn't reach mainstream yet. About the bug report, there's a related bug, on
a thread about Avermedia A16D. The issue is that "dev" is NULL but this
shouldn't happen (otherwise, all callbacks will fail).

On your patch, you're just returning, if dev=NULL, at saa7134 callback function. IMO, the correct would be to
print an error message and return. Also, we should discover why dev is being
null there (I'll try to identify here - the reason - yet, I can't really test,
since the saa7134 boards I have don't need any callback.
> 
> > 
> > I still need to send a patchset to Linus, after testing compilation
> > (unfortunately, I had to postpone, since I need first to free some
> > hundreds of Mb on my HD on my /home, to allow kernel compilation).
> > Hopefully, I'll have some time tomorrow for doing a "housekeeping".
> > 
> Unfortunately, i deleted you mails describing what went to linux and i don't
> have the RC source here :-(

You may take a look on master branch on my git tree. I'm about to forward him a
series of patches. Hopefully, 2GB free space will be enough for a full kernel
compilation. I'll discover soon...


Cheers,
Mauro

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Hartmut Hackmann]

Mauro Carvalho Chehab schrieb:
> On Wed, 19 Mar 2008 23:16:47 +0100
> Hartmut Hackmann <hartmut.hackmann@t-online.de> wrote:
> 
>> Hi, Mauro
>>
>> Mauro Carvalho Chehab schrieb:
>>> On Wed, 19 Mar 2008, Hartmut Hackmann wrote:
>>>
>>>> Mauro, what's your opinion on this? As far as i know, the broken code
>>>> is in the upcoming
>>>> kernel release. The patch is big, is there a chance to commit it to
>>>> the kernel?
>>> While some fixes are cosmetic (like __func__ change), and others are
>>> just function reordering, I suspect that the real changes are still too
>>> big for -rc6. It will probably be nacked.
>>>
>>> Yet, it may be worthy to try.
>> This was my opinion as well.
>> Did you notice Michaels reply on this issue? He pointed out that the problem
>> was introduced by this changeset:
>> http://linuxtv.org/hg/v4l-dvb/rev/ad6fb7fe6240 : Add support for xc3028-based boards
>>
>> If this did not go to Linus yet, we don't have a problem. This also explains
>> why we don't have bug reports on this.
> 
> It didn't reach mainstream yet. About the bug report, there's a related bug, on
> a thread about Avermedia A16D. The issue is that "dev" is NULL but this
> shouldn't happen (otherwise, all callbacks will fail).
> 
> On your patch, you're just returning, if dev=NULL, at saa7134 callback function. IMO, the correct would be to
> print an error message and return. Also, we should discover why dev is being
> null there (I'll try to identify here - the reason - yet, I can't really test,
> since the saa7134 boards I have don't need any callback.

That's not the point. In the call in tda827x.c tda827xa_lna_gain(), the argument
did not point to the saa7134_dev structure as the function expected. I added
the check for NULL because only at the very first call, the pointer is still
not valid. I did not check this carefully but i guess this is a matter of the
initilization sequence of the data structures. IMHO yes, we should understand this
sometime but this does not have priority because i am sure that the NULL pointer
occurs only during initialization.

>>> I still need to send a patchset to Linus, after testing compilation
>>> (unfortunately, I had to postpone, since I need first to free some
>>> hundreds of Mb on my HD on my /home, to allow kernel compilation).
>>> Hopefully, I'll have some time tomorrow for doing a "housekeeping".
>>>
>> Unfortunately, i deleted you mails describing what went to linux and i don't
>> have the RC source here :-(
> 
> You may take a look on master branch on my git tree. I'm about to forward him a
> series of patches. Hopefully, 2GB free space will be enough for a full kernel
> compilation. I'll discover soon...
> 
Jep. Meanwhile Michael confirmed that the problem is not in mainstream,
so there is no reason to hurry.
But we should have a bigger audience for my latest changes, so i will send
you a pull request in a minute.


Best regards
  Hartmut

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Mauro Carvalho Chehab]

On Thu, 20 Mar 2008 02:23:59 +0100
Hartmut Hackmann <hartmut.hackmann@t-online.de> wrote:

> > On your patch, you're just returning, if dev=NULL, at saa7134 callback function. IMO, the correct would be to
> > print an error message and return. Also, we should discover why dev is being
> > null there (I'll try to identify here - the reason - yet, I can't really test,
> > since the saa7134 boards I have don't need any callback.
> 
> That's not the point. In the call in tda827x.c tda827xa_lna_gain(), the argument
> did not point to the saa7134_dev structure as the function expected. I added
> the check for NULL because only at the very first call, the pointer is still
> not valid. I did not check this carefully but i guess this is a matter of the
> initilization sequence of the data structures. IMHO yes, we should understand this
> sometime but this does not have priority because i am sure that the NULL pointer
> occurs only during initialization.

This is caused by a patch conflict between hybrid redesign and the merge of
xc3028 support. The enclosed experimental patch fixes the tuner_callback
argument, on linux/drivers/media/dvb/frontends/tda827x.c. 
It should also fix the priv argument on saa7134_tuner_callback(). I can't test
the saa7134 part here, due to the lack of a saa7134 hardware that needs a
callback.

The patch also intends to make xc3028 easier to use. That part is still not
fully working. I should finish this patch tomorrow.

> >>> I still need to send a patchset to Linus, after testing compilation
> >>> (unfortunately, I had to postpone, since I need first to free some
> >>> hundreds of Mb on my HD on my /home, to allow kernel compilation).
> >>> Hopefully, I'll have some time tomorrow for doing a "housekeeping".
> >>>
> >> Unfortunately, i deleted you mails describing what went to linux and i don't
> >> have the RC source here :-(
> > 
> > You may take a look on master branch on my git tree. I'm about to forward him a
> > series of patches. Hopefully, 2GB free space will be enough for a full kernel
> > compilation. I'll discover soon...
> > 
> Jep. Meanwhile Michael confirmed that the problem is not in mainstream,
> so there is no reason to hurry.

Yes.

> But we should have a bigger audience for my latest changes, so i will send
> you a pull request in a minute.

Could you please test my patch first? Having the same arguments for all
callback functions avoid future mistakes.

---
[RFC] Fix tuner_callback for tda827x

Signed-off-by Mauro Carvalho Chehab <mchehab@infradead.org>

---

diff -r f24051885fe9 linux/drivers/media/dvb/dvb-usb/cxusb.c
--- a/linux/drivers/media/dvb/dvb-usb/cxusb.c	Tue Mar 18 18:10:06 2008 -0300
+++ b/linux/drivers/media/dvb/dvb-usb/cxusb.c	Wed Mar 19 22:23:54 2008 -0300
@@ -509,7 +509,6 @@ static int cxusb_dvico_xc3028_tuner_atta
 	struct xc2028_config	  cfg = {
 		.i2c_adap  = &adap->dev->i2c_adap,
 		.i2c_addr  = 0x61,
-		.video_dev = adap->dev,
 		.callback  = dvico_bluebird_xc2028_callback,
 	};
 	static struct xc2028_ctrl ctl = {
diff -r f24051885fe9 linux/drivers/media/dvb/frontends/tda827x.c
--- a/linux/drivers/media/dvb/frontends/tda827x.c	Tue Mar 18 18:10:06 2008 -0300
+++ b/linux/drivers/media/dvb/frontends/tda827x.c	Wed Mar 19 22:23:54 2008 -0300
@@ -579,7 +579,8 @@ static void tda827xa_lna_gain(struct dvb
 		else
 			arg = 0;
 		if (priv->cfg->tuner_callback)
-			priv->cfg->tuner_callback(priv, 1, arg);
+			priv->cfg->tuner_callback(priv->i2c_adap->algo_data,
+						  1, arg);
 		buf[1] = high ? 0 : 1;
 		if (*priv->cfg->config == 2)
 			buf[1] = high ? 1 : 0;
@@ -587,7 +588,8 @@ static void tda827xa_lna_gain(struct dvb
 		break;
 	case 3: /* switch with GPIO of saa713x */
 		if (priv->cfg->tuner_callback)
-			priv->cfg->tuner_callback(priv, 0, high);
+			priv->cfg->tuner_callback(priv->i2c_adap->algo_data,
+						  0, high);
 		break;
 	}
 }
diff -r f24051885fe9 linux/drivers/media/video/cx23885/cx23885-dvb.c
--- a/linux/drivers/media/video/cx23885/cx23885-dvb.c	Tue Mar 18 18:10:06 2008 -0300
+++ b/linux/drivers/media/video/cx23885/cx23885-dvb.c	Wed Mar 19 22:23:54 2008 -0300
@@ -298,7 +298,6 @@ static int dvb_register(struct cx23885_t
 			struct xc2028_config cfg = {
 				.i2c_adap  = &i2c_bus->i2c_adap,
 				.i2c_addr  = 0x61,
-				.video_dev = port,
 				.callback  = cx23885_hvr1500_xc3028_callback,
 			};
 			static struct xc2028_ctrl ctl = {
diff -r f24051885fe9 linux/drivers/media/video/cx88/cx88-cards.c
--- a/linux/drivers/media/video/cx88/cx88-cards.c	Tue Mar 18 18:10:06 2008 -0300
+++ b/linux/drivers/media/video/cx88/cx88-cards.c	Wed Mar 19 22:23:54 2008 -0300
@@ -2140,11 +2140,9 @@ static void gdi_eeprom(struct cx88_core 
 
 /* ------------------------------------------------------------------- */
 /* some Divco specific stuff                                           */
-static int cx88_dvico_xc2028_callback(void *priv, int command, int arg)
+static int cx88_dvico_xc2028_callback(struct cx88_core *core,
+				      int command, int arg)
 {
-	struct i2c_algo_bit_data *i2c_algo = priv;
-	struct cx88_core *core = i2c_algo->data;
-
 	switch (command) {
 	case XC2028_TUNER_RESET:
 		cx_write(MO_GP0_IO, 0x101000);
@@ -2162,11 +2160,9 @@ static int cx88_dvico_xc2028_callback(vo
 /* ----------------------------------------------------------------------- */
 /* some Geniatech specific stuff                                           */
 
-static int cx88_xc3028_geniatech_tuner_callback(void *priv, int command, int mode)
+static int cx88_xc3028_geniatech_tuner_callback(struct cx88_core *core,
+						int command, int mode)
 {
-	struct i2c_algo_bit_data *i2c_algo = priv;
-	struct cx88_core *core = i2c_algo->data;
-
 	switch (command) {
 	case XC2028_TUNER_RESET:
 		switch (INPUT(core->input).type) {
@@ -2193,11 +2189,9 @@ static int cx88_xc3028_geniatech_tuner_c
 
 /* ------------------------------------------------------------------- */
 /* some Divco specific stuff                                           */
-static int cx88_pv_8000gt_callback(void *priv, int command, int arg)
+static int cx88_pv_8000gt_callback(struct cx88_core *core,
+				   int command, int arg)
 {
-	struct i2c_algo_bit_data *i2c_algo = priv;
-	struct cx88_core *core = i2c_algo->data;
-
 	switch (command) {
 	case XC2028_TUNER_RESET:
 		cx_write(MO_GP2_IO, 0xcf7);
@@ -2248,21 +2242,19 @@ static void dvico_fusionhdtv_hybrid_init
 	}
 }
 
-static int cx88_xc2028_tuner_callback(void *priv, int command, int arg)
+static int cx88_xc2028_tuner_callback(struct cx88_core *core, int command, int arg)
 {
-	struct i2c_algo_bit_data *i2c_algo = priv;
-	struct cx88_core *core = i2c_algo->data;
-
 	/* Board-specific callbacks */
 	switch (core->boardnr) {
 	case CX88_BOARD_WINFAST_TV2000_XP_GLOBAL:
 	case CX88_BOARD_POWERCOLOR_REAL_ANGEL:
 	case CX88_BOARD_GENIATECH_X8000_MT:
-		return cx88_xc3028_geniatech_tuner_callback(priv, command, arg);
+		return cx88_xc3028_geniatech_tuner_callback(core,
+							command, arg);
 	case CX88_BOARD_PROLINK_PV_8000GT:
-		return cx88_pv_8000gt_callback(priv, command, arg);
+		return cx88_pv_8000gt_callback(core, command, arg);
 	case CX88_BOARD_DVICO_FUSIONHDTV_DVB_T_PRO:
-		return cx88_dvico_xc2028_callback(priv, command, arg);
+		return cx88_dvico_xc2028_callback(core, command, arg);
 	}
 
 	switch (command) {
@@ -2296,11 +2288,9 @@ static int cx88_xc2028_tuner_callback(vo
  * PCTV HD 800i with an xc5000 sillicon tuner. This is used for both	   *
  * analog tuner attach (tuner-core.c) and dvb tuner attach (cx88-dvb.c)    */
 
-static int cx88_xc5000_tuner_callback(void *priv, int command, int arg)
+static int cx88_xc5000_tuner_callback(struct cx88_core *core,
+				      int command, int arg)
 {
-	struct i2c_algo_bit_data *i2c_algo = priv;
-	struct cx88_core *core = i2c_algo->data;
-
 	switch (core->boardnr) {
 	case CX88_BOARD_PINNACLE_PCTV_HD_800i:
 		if (command == 0) { /* This is the reset command from xc5000 */
@@ -2334,15 +2324,27 @@ int cx88_tuner_callback(void *priv, int 
 int cx88_tuner_callback(void *priv, int command, int arg)
 {
 	struct i2c_algo_bit_data *i2c_algo = priv;
-	struct cx88_core *core = i2c_algo->data;
+	struct cx88_core *core;
+
+	if (!i2c_algo) {
+		printk(KERN_ERR "cx88: Error - i2c_algo not defined.\n");
+		return -EINVAL;
+	}
+
+	core = i2c_algo->data;
+
+	if (!core) {
+		printk(KERN_ERR "cx88: Error - i2c_algo->data not defined. priv=%p\n", priv);
+		return -EINVAL;
+	}
 
 	switch (core->board.tuner_type) {
 		case TUNER_XC2028:
 			info_printk(core, "Calling XC2028/3028 callback\n");
-			return cx88_xc2028_tuner_callback(priv, command, arg);
+			return cx88_xc2028_tuner_callback(core, command, arg);
 		case TUNER_XC5000:
 			info_printk(core, "Calling XC5000 callback\n");
-			return cx88_xc5000_tuner_callback(priv, command, arg);
+			return cx88_xc5000_tuner_callback(core, command, arg);
 	}
 	err_printk(core, "Error: Calling callback for tuner %d\n",
 		   core->board.tuner_type);
diff -r f24051885fe9 linux/drivers/media/video/cx88/cx88-dvb.c
--- a/linux/drivers/media/video/cx88/cx88-dvb.c	Tue Mar 18 18:10:06 2008 -0300
+++ b/linux/drivers/media/video/cx88/cx88-dvb.c	Wed Mar 19 22:23:54 2008 -0300
@@ -465,7 +465,6 @@ static int attach_xc3028(u8 addr, struct
 	struct xc2028_config cfg = {
 		.i2c_adap  = &dev->core->i2c_adap,
 		.i2c_addr  = addr,
-		.video_dev = dev->core->i2c_adap.algo_data,
 	};
 
 	if (!dev->dvb.frontend) {
@@ -787,7 +786,6 @@ static int dvb_register(struct cx8802_de
 			struct xc2028_config cfg = {
 				.i2c_adap  = &dev->core->i2c_adap,
 				.i2c_addr  = 0x61,
-				.video_dev = dev->core,
 				.callback  = cx88_pci_nano_callback,
 			};
 			static struct xc2028_ctrl ctl = {
diff -r f24051885fe9 linux/drivers/media/video/saa7134/saa7134-cards.c
--- a/linux/drivers/media/video/saa7134/saa7134-cards.c	Tue Mar 18 18:10:06 2008 -0300
+++ b/linux/drivers/media/video/saa7134/saa7134-cards.c	Wed Mar 19 22:23:54 2008 -0300
@@ -5353,10 +5353,10 @@ static int saa7134_tda8290_callback(stru
 	return 0;
 }
 
+/* priv retuns algo_data - on saa7134, it is equal to dev */
 int saa7134_tuner_callback(void *priv, int command, int arg)
 {
-	struct i2c_algo_bit_data *i2c_algo = priv;
-	struct saa7134_dev *dev = i2c_algo->data;
+	struct saa7134_dev *dev = priv;
 
 	switch (dev->tuner_type) {
 	case TUNER_PHILIPS_TDA8290:
diff -r f24051885fe9 linux/drivers/media/video/saa7134/saa7134-dvb.c
--- a/linux/drivers/media/video/saa7134/saa7134-dvb.c	Tue Mar 18 18:10:06 2008 -0300
+++ b/linux/drivers/media/video/saa7134/saa7134-dvb.c	Wed Mar 19 22:23:54 2008 -0300
@@ -1173,7 +1173,6 @@ static int dvb_init(struct saa7134_dev *
 		struct xc2028_config cfg = {
 			.i2c_adap  = &dev->i2c_adap,
 			.i2c_addr  = 0x61,
-			.video_dev = dev->i2c_adap.algo_data,
 		};
 		fe = dvb_attach(xc2028_attach, dev->dvb.frontend, &cfg);
 		if (!fe) {
diff -r f24051885fe9 linux/drivers/media/video/tuner-core.c
--- a/linux/drivers/media/video/tuner-core.c	Tue Mar 18 18:10:06 2008 -0300
+++ b/linux/drivers/media/video/tuner-core.c	Wed Mar 19 22:23:54 2008 -0300
@@ -448,7 +448,6 @@ static void set_type(struct i2c_client *
 		struct xc2028_config cfg = {
 			.i2c_adap  = t->i2c->adapter,
 			.i2c_addr  = t->i2c->addr,
-			.video_dev = c->adapter->algo_data,
 			.callback  = t->tuner_callback,
 		};
 		if (!xc2028_attach(&t->fe, &cfg)) {
diff -r f24051885fe9 linux/drivers/media/video/tuner-xc2028.c
--- a/linux/drivers/media/video/tuner-xc2028.c	Tue Mar 18 18:10:06 2008 -0300
+++ b/linux/drivers/media/video/tuner-xc2028.c	Wed Mar 19 22:23:54 2008 -0300
@@ -1174,7 +1174,7 @@ struct dvb_frontend *xc2028_attach(struc
 	if (debug)
 		printk(KERN_DEBUG "xc2028: Xcv2028/3028 init called!\n");
 
-	if (NULL == cfg || NULL == cfg->video_dev)
+	if (NULL == cfg)
 		return NULL;
 
 	if (!fe) {
@@ -1182,7 +1182,7 @@ struct dvb_frontend *xc2028_attach(struc
 		return NULL;
 	}
 
-	video_dev = cfg->video_dev;
+	video_dev = cfg->i2c_adap->algo_data;
 
 	mutex_lock(&xc2028_list_mutex);
 





Cheers,
Mauro

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Hartmut Hackmann]

HI, Mauro

Mauro Carvalho Chehab schrieb:
> On Thu, 20 Mar 2008 02:23:59 +0100
> Hartmut Hackmann <hartmut.hackmann@t-online.de> wrote:
> 
>>> On your patch, you're just returning, if dev=NULL, at saa7134 callback function. IMO, the correct would be to
>>> print an error message and return. Also, we should discover why dev is being
>>> null there (I'll try to identify here - the reason - yet, I can't really test,
>>> since the saa7134 boards I have don't need any callback.
>> That's not the point. In the call in tda827x.c tda827xa_lna_gain(), the argument
>> did not point to the saa7134_dev structure as the function expected. I added
>> the check for NULL because only at the very first call, the pointer is still
>> not valid. I did not check this carefully but i guess this is a matter of the
>> initilization sequence of the data structures. IMHO yes, we should understand this
>> sometime but this does not have priority because i am sure that the NULL pointer
>> occurs only during initialization.
> 
> This is caused by a patch conflict between hybrid redesign and the merge of
> xc3028 support. The enclosed experimental patch fixes the tuner_callback
> argument, on linux/drivers/media/dvb/frontends/tda827x.c. 
> It should also fix the priv argument on saa7134_tuner_callback(). I can't test
> the saa7134 part here, due to the lack of a saa7134 hardware that needs a
> callback.
> 
> The patch also intends to make xc3028 easier to use. That part is still not
> fully working. I should finish this patch tomorrow.
> 
>>>>> I still need to send a patchset to Linus, after testing compilation
>>>>> (unfortunately, I had to postpone, since I need first to free some
>>>>> hundreds of Mb on my HD on my /home, to allow kernel compilation).
>>>>> Hopefully, I'll have some time tomorrow for doing a "housekeeping".
>>>>>
>>>> Unfortunately, i deleted you mails describing what went to linux and i don't
>>>> have the RC source here :-(
>>> You may take a look on master branch on my git tree. I'm about to forward him a
>>> series of patches. Hopefully, 2GB free space will be enough for a full kernel
>>> compilation. I'll discover soon...
>>>
>> Jep. Meanwhile Michael confirmed that the problem is not in mainstream,
>> so there is no reason to hurry.
> 
> Yes.
> 
>> But we should have a bigger audience for my latest changes, so i will send
>> you a pull request in a minute.
> 
> Could you please test my patch first? Having the same arguments for all
> callback functions avoid future mistakes.
> 
> ---
> [RFC] Fix tuner_callback for tda827x
> 
> Signed-off-by Mauro Carvalho Chehab <mchehab@infradead.org>
> 
<snip>

Your patch does not completely apply for me, it fails in cx88-dvb.c
I had a close look and found that we are going in the same direction.
- The change in tda827x is the same as i did.
- In saa7134-cards.c your patch is right. My version just worked by accident.
  I corrected this in my repository.
By the way: the dev pointer is NULL during initialization is gone.
I tested again and things work for me.

I would recommend the following:
- You pull from my repository (sent you the request yesterday)
- You apply the patch *except* the changes in tda827x.c, saa7134-cards.c
  and saa7134-dvb.c. Afterwards we should be fine.

My other changes to tda827x and saa7134-dvb.c are not only cosmetic. It merged
the _lna_gain functions for analog and dvb and adapt the data structures.
What do you think?

I will be out from friday to monday.

Best regards
   Hartmut

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Mauro Carvalho Chehab]

On Thu, 20 Mar 2008 21:41:19 +0100
Hartmut Hackmann <hartmut.hackmann@t-online.de> wrote:

> HI, Mauro
> 
> Mauro Carvalho Chehab schrieb:
> > On Thu, 20 Mar 2008 02:23:59 +0100
> > Hartmut Hackmann <hartmut.hackmann@t-online.de> wrote:
> > 
> >>> On your patch, you're just returning, if dev=NULL, at saa7134 callback function. IMO, the correct would be to
> >>> print an error message and return. Also, we should discover why dev is being
> >>> null there (I'll try to identify here - the reason - yet, I can't really test,
> >>> since the saa7134 boards I have don't need any callback.
> >> That's not the point. In the call in tda827x.c tda827xa_lna_gain(), the argument
> >> did not point to the saa7134_dev structure as the function expected. I added
> >> the check for NULL because only at the very first call, the pointer is still
> >> not valid. I did not check this carefully but i guess this is a matter of the
> >> initilization sequence of the data structures. IMHO yes, we should understand this
> >> sometime but this does not have priority because i am sure that the NULL pointer
> >> occurs only during initialization.
> > 
> > This is caused by a patch conflict between hybrid redesign and the merge of
> > xc3028 support. The enclosed experimental patch fixes the tuner_callback
> > argument, on linux/drivers/media/dvb/frontends/tda827x.c. 
> > It should also fix the priv argument on saa7134_tuner_callback(). I can't test
> > the saa7134 part here, due to the lack of a saa7134 hardware that needs a
> > callback.
> > 
> > The patch also intends to make xc3028 easier to use. That part is still not
> > fully working. I should finish this patch tomorrow.
> > 
> >>>>> I still need to send a patchset to Linus, after testing compilation
> >>>>> (unfortunately, I had to postpone, since I need first to free some
> >>>>> hundreds of Mb on my HD on my /home, to allow kernel compilation).
> >>>>> Hopefully, I'll have some time tomorrow for doing a "housekeeping".
> >>>>>
> >>>> Unfortunately, i deleted you mails describing what went to linux and i don't
> >>>> have the RC source here :-(
> >>> You may take a look on master branch on my git tree. I'm about to forward him a
> >>> series of patches. Hopefully, 2GB free space will be enough for a full kernel
> >>> compilation. I'll discover soon...
> >>>
> >> Jep. Meanwhile Michael confirmed that the problem is not in mainstream,
> >> so there is no reason to hurry.
> > 
> > Yes.
> > 
> >> But we should have a bigger audience for my latest changes, so i will send
> >> you a pull request in a minute.
> > 
> > Could you please test my patch first? Having the same arguments for all
> > callback functions avoid future mistakes.
> > 
> > ---
> > [RFC] Fix tuner_callback for tda827x
> > 
> > Signed-off-by Mauro Carvalho Chehab <mchehab@infradead.org>
> > 
> <snip>
> 
> Your patch does not completely apply for me, it fails in cx88-dvb.c

This is due to some changes that happened later on the tree, for cx88.

I also had some a trivial conflict on saa7134-cards, when merging from your tree ;)

The better is to do an "hg pull -u", before asking me to pull.

Anyway, the conflict were easily solved.

> I had a close look and found that we are going in the same direction.
> - The change in tda827x is the same as i did.
> - In saa7134-cards.c your patch is right. My version just worked by accident.
>   I corrected this in my repository.
> By the way: the dev pointer is NULL during initialization is gone.
> I tested again and things work for me.

Great news.

> I would recommend the following:
> - You pull from my repository (sent you the request yesterday)
> - You apply the patch *except* the changes in tda827x.c, saa7134-cards.c
>   and saa7134-dvb.c. Afterwards we should be fine.

Ok, I did this. 

Patches applied on a order that won't generate bissect compilation issues.

I still had to patch saa7134-dvb, probably due to the conflict
I've mentioned. I'll double check when importing the changes to -git, since
I'll need to manually fix there, instead of trusting on kdiff3.

I also added a printk, if dev=NULL. This won't work with the current code, but
it helps to track future issues, if we have this kind of troubles later with
other callbacks.

> My other changes to tda827x and saa7134-dvb.c are not only cosmetic. It merged
> the _lna_gain functions for analog and dvb and adapt the data structures.
> What do you think?

Seems fine to me.

Just one comment about the config var (this applies also to the previous code): 
I'd prefer to have an enum, instead of config=0,1,2,3. Something like:

enum {
	TDA827x_NO_LNA,
	TDA827x_LNA_VIA8290_LOW,
	TDA827x_LNA_VIA8290_HIGH,
	TDA827x_LNA_VIA_HOST,
} config;

This helps people to better understand the LNA config code.

> I will be out from friday to monday.

I should also take a break during those days ;)


Cheers,
Mauro

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Hartmut Hackmann]

Hi, Mauro

Mauro Carvalho Chehab schrieb:
> On Thu, 20 Mar 2008 21:41:19 +0100
> Hartmut Hackmann <hartmut.hackmann@t-online.de> wrote:
> 
>> HI, Mauro
>>
>> Mauro Carvalho Chehab schrieb:
>>> On Thu, 20 Mar 2008 02:23:59 +0100
>>> Hartmut Hackmann <hartmut.hackmann@t-online.de> wrote:
>>>
>>>>> On your patch, you're just returning, if dev=NULL, at saa7134 callback function. IMO, the correct would be to
>>>>> print an error message and return. Also, we should discover why dev is being
>>>>> null there (I'll try to identify here - the reason - yet, I can't really test,
>>>>> since the saa7134 boards I have don't need any callback.
>>>> That's not the point. In the call in tda827x.c tda827xa_lna_gain(), the argument
>>>> did not point to the saa7134_dev structure as the function expected. I added
>>>> the check for NULL because only at the very first call, the pointer is still
>>>> not valid. I did not check this carefully but i guess this is a matter of the
>>>> initilization sequence of the data structures. IMHO yes, we should understand this
>>>> sometime but this does not have priority because i am sure that the NULL pointer
>>>> occurs only during initialization.
>>> This is caused by a patch conflict between hybrid redesign and the merge of
>>> xc3028 support. The enclosed experimental patch fixes the tuner_callback
>>> argument, on linux/drivers/media/dvb/frontends/tda827x.c. 
>>> It should also fix the priv argument on saa7134_tuner_callback(). I can't test
>>> the saa7134 part here, due to the lack of a saa7134 hardware that needs a
>>> callback.
>>>
>>> The patch also intends to make xc3028 easier to use. That part is still not
>>> fully working. I should finish this patch tomorrow.
>>>
>>>>>>> I still need to send a patchset to Linus, after testing compilation
>>>>>>> (unfortunately, I had to postpone, since I need first to free some
>>>>>>> hundreds of Mb on my HD on my /home, to allow kernel compilation).
>>>>>>> Hopefully, I'll have some time tomorrow for doing a "housekeeping".
>>>>>>>
>>>>>> Unfortunately, i deleted you mails describing what went to linux and i don't
>>>>>> have the RC source here :-(
>>>>> You may take a look on master branch on my git tree. I'm about to forward him a
>>>>> series of patches. Hopefully, 2GB free space will be enough for a full kernel
>>>>> compilation. I'll discover soon...
>>>>>
>>>> Jep. Meanwhile Michael confirmed that the problem is not in mainstream,
>>>> so there is no reason to hurry.
>>> Yes.
>>>
>>>> But we should have a bigger audience for my latest changes, so i will send
>>>> you a pull request in a minute.
>>> Could you please test my patch first? Having the same arguments for all
>>> callback functions avoid future mistakes.
>>>
>>> ---
>>> [RFC] Fix tuner_callback for tda827x
>>>
>>> Signed-off-by Mauro Carvalho Chehab <mchehab@infradead.org>
>>>
>> <snip>
>>
>> Your patch does not completely apply for me, it fails in cx88-dvb.c
> 
> This is due to some changes that happened later on the tree, for cx88.
> 
> I also had some a trivial conflict on saa7134-cards, when merging from your tree ;)
> 
> The better is to do an "hg pull -u", before asking me to pull.
> 
Basically you are right of corse. I try to avoid this after i once saw a merge
causing an unnecessary huge change log. But this was an old hg version.

> Anyway, the conflict were easily solved.
> 
>> I had a close look and found that we are going in the same direction.
>> - The change in tda827x is the same as i did.
>> - In saa7134-cards.c your patch is right. My version just worked by accident.
>>   I corrected this in my repository.
>> By the way: the dev pointer is NULL during initialization is gone.
>> I tested again and things work for me.
> 
> Great news.
> 
>> I would recommend the following:
>> - You pull from my repository (sent you the request yesterday)
>> - You apply the patch *except* the changes in tda827x.c, saa7134-cards.c
>>   and saa7134-dvb.c. Afterwards we should be fine.
> 
> Ok, I did this. 
> 
> Patches applied on a order that won't generate bissect compilation issues.
> 
> I still had to patch saa7134-dvb, probably due to the conflict
> I've mentioned. I'll double check when importing the changes to -git, since
> I'll need to manually fix there, instead of trusting on kdiff3.
> 
> I also added a printk, if dev=NULL. This won't work with the current code, but
> it helps to track future issues, if we have this kind of troubles later with
> other callbacks.
> 
That's fine.

>> My other changes to tda827x and saa7134-dvb.c are not only cosmetic. It merged
>> the _lna_gain functions for analog and dvb and adapt the data structures.
>> What do you think?
> 
> Seems fine to me.
> 
> Just one comment about the config var (this applies also to the previous code): 
> I'd prefer to have an enum, instead of config=0,1,2,3. Something like:
> 
> enum {
> 	TDA827x_NO_LNA,
> 	TDA827x_LNA_VIA8290_LOW,
> 	TDA827x_LNA_VIA8290_HIGH,
> 	TDA827x_LNA_VIA_HOST,
> } config;
> 
> This helps people to better understand the LNA config code.
> 
Hm, i did similar things in other places...
I did not do this here because i wanted to be able to use this variable
with other tuners as well -> use #defines instead?

>> I will be out from friday to monday.
> 
> I should also take a break during those days ;)
> 
> 
> Cheers,
> Mauro
> 
Best regards
  Hartmut

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Mauro Carvalho Chehab]

On Thu, 27 Mar 2008, Hartmut Hackmann wrote:

>> The better is to do an "hg pull -u", before asking me to pull.
>>
> Basically you are right of corse. I try to avoid this after i once saw a merge
> causing an unnecessary huge change log. But this was an old hg version.

hg merge logs sucks, IMO. Git do a much better job. I didn't tested yet hg 
version 1.0 (finally, they lauched version 1 ;) ).

>> Just one comment about the config var (this applies also to the previous code):
>> I'd prefer to have an enum, instead of config=0,1,2,3. Something like:
>>
>> enum {
>> 	TDA827x_NO_LNA,
>> 	TDA827x_LNA_VIA8290_LOW,
>> 	TDA827x_LNA_VIA8290_HIGH,
>> 	TDA827x_LNA_VIA_HOST,
>> } config;
>>
>> This helps people to better understand the LNA config code.
>>
> Hm, i did similar things in other places...
> I did not do this here because i wanted to be able to use this variable
> with other tuners as well -> use #defines instead?

Could be #define or enum. Yet, we'll need to have those defines/enum 
declared into a .h, and included on the driver that uses it. We are using 
this approach also for other drivers that need callback, like 
tuner-xc2028.h.

Cheers,
Mauro Carvalho Chehab
http://linuxtv.org
mchehab@infradead.org

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [linux-dvb] [RFC] TDA8290 / TDA827X with LNA: testers wanted

[Ian Haywood]

On Wednesday 19 March 2008 11:40:11 Hartmut Hackmann wrote:

> KWORLD DVBT 210
I have a Tevion/KWorld 220RF. This seems to include the chips
you are referring to. Anyway, with your latest code on kernel 2.6.24.3:

[   66.794518] saa7133[0]: found at 0000:00:0a.0, rev: 208, irq: 19, latency: 32, mmio: 0xe60080
00
[   66.794592] saa7133[0]: subsystem: 10d4:0201, board: UNKNOWN/GENERIC [card=0,autodetected]
[   66.794667] saa7133[0]: board init: gpio is 100
[   66.974166] saa7133[0]: i2c eeprom 00: 12 14 00 10 06 83 fa ff f7 58 1a 8d e0 00 00 51
[   66.974899] saa7133[0]: i2c eeprom 10: d1 d4 b8 13 04 96 bc 8c c0 27 10 50 36 e7 d1 00
[   66.975627] saa7133[0]: i2c eeprom 20: 01 50 20 23 00 20 d1 7c 37 08 02 5b 2b 58 1b 5b
[   66.976356] saa7133[0]: i2c eeprom 30: e0 85 ff ff ff 87 ff 00 01 5e 00 20 00 01 00 71
[   66.977084] saa7133[0]: i2c eeprom 40: d1 8c b2 50 46 23 01 20 cb 50 10 20 d1 d4 b8 15
[   66.977812] saa7133[0]: i2c eeprom 50: 06 83 fa ff f7 58 1a 8d e0 00 00 41 81 02 78 d4
[   66.978548] saa7133[0]: i2c eeprom 60: b8 12 03 96 bc 79 ab cc 7f 50 26 87 c0 ff 00 87
[   66.979276] saa7133[0]: i2c eeprom 70: b8 9f c4 9f fc 8d c0 7f ff 50 46 5e 00 7f ff 20
[   66.980004] saa7133[0]: i2c eeprom 80: ab 50 a0 9f c4 9f fc 8d c0 80 00 50 44 5e 00 80
[   66.980731] saa7133[0]: i2c eeprom 90: 00 20 ab 50 10 3c ab d4 b8 15 04 83 fa ff fc 58
[   66.982020] saa7133[0]: i2c eeprom a0: 1a 8d e0 00 02 51 82 71 78 cc 05 52 56 5e 1a 3e
[   66.982750] saa7133[0]: i2c eeprom b0: 7f 86 b9 83 e8 f0 00 8c c0 7c fe 50 d3 d4 b8 13
[   66.983478] saa7133[0]: i2c eeprom c0: 04 77 60 96 bc 8c c0 01 f4 50 16 5b 20 d4 b8 12
[   66.984206] saa7133[0]: i2c eeprom d0: 00 c2 fd 80 bb 51 30 71 78 cc 05 50 d6 5e 1a 7d
[   66.984932] saa7133[0]: i2c eeprom e0: 00 86 b9 5e 1b fa 00 83 e8 f0 00 83 e9 f0 00 8c
[   66.985660] saa7133[0]: i2c eeprom f0: b9 50 76 50 10 20 ab d4 b8 12 00 c0 02 d4 bc 12
[   67.074658] saa7133[0]: registered device video0 [v4l2]
[   67.076845] saa7133[0]: registered device vbi0

The tda8290 module doesn't seem to get loaded at all (with stock kernel
code it does, but doesn't tune as I reported earlier)

Ian


_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Re: [linux-dvb] [RFC] TDA8290 / TDA827X with LNA: testers wanted

[hermann pitton]

Hi Ian,

Am Mittwoch, den 19.03.2008, 20:50 +1100 schrieb Ian Haywood:
> On Wednesday 19 March 2008 11:40:11 Hartmut Hackmann wrote:
> 
> > KWORLD DVBT 210
> I have a Tevion/KWorld 220RF. This seems to include the chips
> you are referring to. Anyway, with your latest code on kernel 2.6.24.3:
> 
> [   66.794518] saa7133[0]: found at 0000:00:0a.0, rev: 208, irq: 19, latency: 32, mmio: 0xe60080
> 00
> [   66.794592] saa7133[0]: subsystem: 10d4:0201, board: UNKNOWN/GENERIC [card=0,autodetected]
> [   66.794667] saa7133[0]: board init: gpio is 100
> [   66.974166] saa7133[0]: i2c eeprom 00: 12 14 00 10 06 83 fa ff f7 58 1a 8d e0 00 00 51
> [   66.974899] saa7133[0]: i2c eeprom 10: d1 d4 b8 13 04 96 bc 8c c0 27 10 50 36 e7 d1 00
> [   66.975627] saa7133[0]: i2c eeprom 20: 01 50 20 23 00 20 d1 7c 37 08 02 5b 2b 58 1b 5b
> [   66.976356] saa7133[0]: i2c eeprom 30: e0 85 ff ff ff 87 ff 00 01 5e 00 20 00 01 00 71
> [   66.977084] saa7133[0]: i2c eeprom 40: d1 8c b2 50 46 23 01 20 cb 50 10 20 d1 d4 b8 15
> [   66.977812] saa7133[0]: i2c eeprom 50: 06 83 fa ff f7 58 1a 8d e0 00 00 41 81 02 78 d4
> [   66.978548] saa7133[0]: i2c eeprom 60: b8 12 03 96 bc 79 ab cc 7f 50 26 87 c0 ff 00 87
> [   66.979276] saa7133[0]: i2c eeprom 70: b8 9f c4 9f fc 8d c0 7f ff 50 46 5e 00 7f ff 20
> [   66.980004] saa7133[0]: i2c eeprom 80: ab 50 a0 9f c4 9f fc 8d c0 80 00 50 44 5e 00 80
> [   66.980731] saa7133[0]: i2c eeprom 90: 00 20 ab 50 10 3c ab d4 b8 15 04 83 fa ff fc 58
> [   66.982020] saa7133[0]: i2c eeprom a0: 1a 8d e0 00 02 51 82 71 78 cc 05 52 56 5e 1a 3e
> [   66.982750] saa7133[0]: i2c eeprom b0: 7f 86 b9 83 e8 f0 00 8c c0 7c fe 50 d3 d4 b8 13
> [   66.983478] saa7133[0]: i2c eeprom c0: 04 77 60 96 bc 8c c0 01 f4 50 16 5b 20 d4 b8 12
> [   66.984206] saa7133[0]: i2c eeprom d0: 00 c2 fd 80 bb 51 30 71 78 cc 05 50 d6 5e 1a 7d
> [   66.984932] saa7133[0]: i2c eeprom e0: 00 86 b9 5e 1b fa 00 83 e8 f0 00 83 e9 f0 00 8c
> [   66.985660] saa7133[0]: i2c eeprom f0: b9 50 76 50 10 20 ab d4 b8 12 00 c0 02 d4 bc 12
> [   67.074658] saa7133[0]: registered device video0 [v4l2]
> [   67.076845] saa7133[0]: registered device vbi0
> 
> The tda8290 module doesn't seem to get loaded at all (with stock kernel
> code it does, but doesn't tune as I reported earlier)

the i2c goes mad currently on your card and that results in a total mess
for the eeprom readout, that goes also for the subsystem there, which is
corrupted and hence you get card=0 without tuner and DVB loaded.

In your previous posting from Friday it was at least OK.

- quote ---------
I am using kernel 2.6.24.3,
The relevant dmesg entries are:
[   46.632313] saa7133[0]: found at 0000:00:0a.0, rev: 208, irq: 20,
latency: 32, mmio: 0xe6008000
[   46.632946] saa7133[0]: subsystem: 17de:7201, board: Tevion/KWorld
DVB-T 220RF [card=88,autodetected]
[   46.633026] saa7133[0]: board init: gpio is 100
[   46.770494] saa7133[0]: i2c eeprom 00: de 17 01 72 ff ff ff ff ff ff
ff ff ff ff ff ff
[   46.771227] saa7133[0]: i2c eeprom 10: ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff
[   46.771956] saa7133[0]: i2c eeprom 20: ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff
[   46.772693] saa7133[0]: i2c eeprom 30: ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff
[   46.773440] saa7133[0]: i2c eeprom 40: ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff
[   46.774167] saa7133[0]: i2c eeprom 50: ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff
[   46.774894] saa7133[0]: i2c eeprom 60: ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff
[   46.775623] saa7133[0]: i2c eeprom 70: ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff
[   47.009197] tuner 0-004b: chip found @ 0x96 (saa7133[0])
[   47.057166] tda8290 0-004b: setting tuner address to 61
[   47.161150] tuner 0-004b: type set to tda8290+75a
[   47.209142] tda8290 0-004b: setting tuner address to 61
[   47.313131] tuner 0-004b: type set to tda8290+75a
[   47.315906] saa7133[0]: registered device video0 [v4l2]
[   47.315984] saa7133[0]: registered device vbi0
[   47.316060] saa7133[0]: registered device radio0
......
[   47.611627] DVB: registering new adapter (saa7133[0])
[   47.611690] DVB: registering frontend 0 (Philips TDA10046H DVB-T)...
[   47.681091] tda1004x: setting up plls for 48MHz sampling clock
.......
[   49.614319] tda1004x: found firmware revision 23 -- ok
-------------------------------------

This looks all fine. The card has no LNA.
If a firmware eeprom is on the card, firmware is always loaded from
there, regardless what you put in /lib/firmware.

Also your previous experimentation with the tuner address was pointless
that way. The analog tuner is correctly detected at 0x61, the second
tuner used for DVB-T is at 0x60 and also did not throw an error?

Last time I saw such eeprom read corruption was with three saa7134
cards, where one, a CTX918_DVB-T, usually needs a special multi bus
master capable PCI slot. The read out of one more card was also
corrupted, but the eeprom content is not altered and I was able to still
use the card by forcing card and tuner number. 

Replacing that CTX918, which has additionally a modem on the board and
is else known to work without problems nevertheless, by "normal" PCI
cards like CTX917, CTX946 or a Asus Tiger revision 1.0, the trouble was
gone.

At first I thought we might be in timing issues, maybe caused by the
recently extended eeprom readout to support the Hauppauge eeprom, but
since I could not confirm that on another machine with four saa7134 (two
Quad, triple CTX948 and the CTX946 moved there) just only mention it
now.

To get the card back, power down and remove all voltage. Check all cards
for good contact and being well seated in the slots, maybe try another
slot for the Kworld, take care that it does not overheat and ask
yourself how much you can trust your PSU. Try to get an unshared IRQ for
it at the next boot, your card should be detected again.

Use recent scan files from linuxtv.org mercurial dvb-apps, for example
"Seven" recently changed, maybe we can discover something then, for sure
other people have the card working, not long back we fixed some analog
inputs and the radio and DVB-T was still OK.

Cheers,
Hermann




_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

[linux-dvb] Kworld 220RF not tuning

[Ian Haywood]

On Thursday 20 March 2008 08:34:25 hermann pitton wrote:
> Hi Ian,

> the i2c goes mad currently on your card and that results in a total mess
> for the eeprom readout, that goes also for the subsystem there, which is
> corrupted and hence you get card=0 without tuner and DVB loaded.
reboot fixes this problem.

> Use recent scan files from linuxtv.org mercurial dvb-apps, for example
> "Seven" recently changed, maybe we can discover something then, for sure
> other people have the card working, not long back we fixed some analog
> inputs and the radio and DVB-T was still OK.
Ok, got these, and compiled the "scan" tool from them. 
No change: all channels fail with ">>> tuning status == 0x00" 
At the risk of asking the obvious, what does this error actually
mean? I've assumed it means problem with the driver, could
it be a hardware or aerial problem?
> Also your previous experimentation with the tuner address was pointless
> that way. The analog tuner is correctly detected at 0x61, the second
> tuner used for DVB-T is at 0x60 and also did not throw an error?
Maybe stupid question: is the second tuner for DVB detected? 
There's no mention of it in my dmesg, and that would explain the 
problem (analog fine, DVB doesn't tune)

Thanks for your help.

Ian




_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb