On Thu, 20 Mar 2008 21:41:19 +0100
Hartmut Hackmann <hartmut.hackmann@xxxxxxxxxxx> wrote:
> HI, Mauro
>
> Mauro Carvalho Chehab schrieb:
> > On Thu, 20 Mar 2008 02:23:59 +0100
> > Hartmut Hackmann <hartmut.hackmann@xxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> >
> <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
_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
