Re: [linux-dvb] Leadtek WinFast DTV-1800H support

From: Steven Toth <stoth@linuxtv.org>
To: sustmi <sustmidown@centrum.cz>
Cc: linux-dvb@linuxtv.org
Subject: Re: [linux-dvb] Leadtek WinFast DTV-1800H support
Date: Mon, 20 Oct 2008 14:45:48 -0400	[thread overview]
Message-ID: <48FCD1DC.3010701@linuxtv.org> (raw)
In-Reply-To: <200810200141.629@centrum.cz>

sustmi wrote:
> Hi,
> 
> I was working on support for Leadtek WinFast DTV-1800H.
> First I made it work with Markus Rechberger's v4l repository (there was
> better XC3028-tunner code).
> 
> Few weeks ago Markus Rechberger announced that he is no going to
> support non-ex28xx cards anymore and that I have to focus on the linuxtv
> linux-dvb repository.
> 
> So I (cooperating with Paul Chubb) made a patch against linuxtv linux-dvb.
> (attached file: leadtek_winfast_dtv1800h.patch)
> 
> It enables support for DVB-T, analogue TV, radio and IR remote control.
> 
> I have tried to use existing parts of code as much as possible.
> The creation of 'cx88_xc3028_winfast1800h_callback' function was
> necessary. Function 'cx88_xc3028_geniatech_tuner_callback' is similar,
> but the extra GPIO code makes IR remote control not work.
> 
> It will be great if patch is merged into the repository.
> 
> There is one question I want to ask you in terms of this message:
> http://www.linuxtv.org/pipermail/linux-dvb/2008-August/028117.html
> Why is using of cx_write() risky?

 From my orig email: "Don't call cx_write() inside the gpio card setup, 
you're potentially destroying the other bits, it's risky."

cx_write destroys the content of the GPIO direction-enablement and 
values bits. That's a bad thing, and can lead to unexpected behaviors if 
used generally in drivers.

It's better to have a driver read the previous register value, and/or in 
the appropriate bit and write the value back to the gpio registers. This 
is what cx_set/clear do. That way parts of the driver can toggle GPIO's 
for important pieces, without having to understand GPIO usage in other 
disconnected/unrelated parts of the driver.

Your patch uses set/clear, which is good.

Incidently, you patch cannot be merged as-is, it has C99 style comments 
'//' are not allowed, change to /* */ etc.

- Steve

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