Re: XC4000: added card_type

[istva...@mailbox.hu]

On 06/03/2011 04:00 PM, Mauro Carvalho Chehab wrote:

> While the xc4000 is not merged upstream, we may have such hack, but
> before merging, this issue should be solved.
> 
> However, it seems better to just do the right thing since the beginning:
> 
> just add a patch for cx88 adding the xc4000 boards there and filling
> the config stuff inside cx88 driver.

I do intend to remove the card_type code later, and only send cx88
patches once the interface is cleaned up. It is only included
for now to have a working code without conflicting patches, and I
do not know exactly what the config structure should eventually
contain, that is, some of the current board-specific 'if' statements
may actually turn out to be unnecessary and can be made the same for
all cards.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: XC4000: added card_type

[Mauro Carvalho Chehab]

Em 03-06-2011 10:16, istva...@mailbox.hu escreveu:
> On 06/03/2011 02:46 PM, Devin Heitmueller wrote:
> 
>> I understand what you're trying to do here, but this is not a good
>> approach.  We do not want to be littering tuner drivers with
>> card-specific if() statements.  Also, this is inconsistent with the
>> way all other tuner drivers work.
>>
>> The approach you are attempting may seem easier at first, but it gets
>> very difficult to manage over time as the number of boards that use
>> the driver increases.
>>
>> You should have the bridge driver be setting up the cfg structure and
>> passing it to the xc4000 driver, just like the xc5000 and xc3028 do.
> 
> Well, for now, I just create patches that reproduce all the changes
> I have made to the driver. Of course, these may not always be the
> best or most elegant possible solutions, and I expect many will not be
> accepted, or further changes/cleanup will be needed later.
> 
> I do not think the number of boards that use this tuner is likely to
> increase much in the future, though, since the XC4000 seems to be a
> discontinued product (at least it no longer appears anywhere on the
> Xceive web site).

While the xc4000 is not merged upstream, we may have such hack, but
before merging, this issue should be solved.

However, it seems better to just do the right thing since the beginning:

just add a patch for cx88 adding the xc4000 boards there and filling
the config stuff inside cx88 driver.

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: XC4000: added card_type

[Mauro Carvalho Chehab]

Em 03-06-2011 09:38, istva...@mailbox.hu escreveu:
> This patch adds support for selecting a card type in struct
> xc4000_config, to allow for implementing some card specific code
> in the driver.

Hi Istvan,

Please send your patches to linux-media@vger.kernel.org. The linux-dvb
ML is obsolete. I didn't remove it from the server just to avoid loosing
the mail history.

With respect to this specific patch, as Devin pointed, the proper way is to
set the configurable data via the boards entries, and not inside xc4000.

So, feel free to send us patches to cx88 and other bridge drivers whose
boards require different configs, in order to work with xc4000.

Thanks,
Mauro

> 
> Signed-off-by: Istvan Varga 
> 
> 
> xc4000_card_type.patch
> 
> 
> diff -uNr xc4000_orig/drivers/media/common/tuners/xc4000.c 
> xc4000/drivers/media/common/tuners/xc4000.c
> --- xc4000_orig/drivers/media/common/tuners/xc4000.c  2011-06-03 
> 11:54:19.0 +0200
> +++ xc4000/drivers/media/common/tuners/xc4000.c   2011-06-03 
> 14:32:59.0 +0200
> @@ -85,6 +85,7 @@
>   u32 bandwidth;
>   u8  video_standard;
>   u8  rf_mode;
> + u8  card_type;
>   u8  ignore_i2c_write_errors;
>   /*  struct xc2028_ctrl  ctrl; */
>   struct firmware_properties cur_fw;
> @@ -1426,6 +1427,16 @@
>   int instance;
>   u16 id = 0;
>  
> + if (cfg->card_type != XC4000_CARD_GENERIC) {
> + if (cfg->card_type == XC4000_CARD_WINFAST_CX88) {
> + cfg->i2c_address = 0x61;
> + cfg->if_khz = 4560;
> + } else {/* default to PCTV 340E */
> + cfg->i2c_address = 0x61;
> + cfg->if_khz = 5400;
> + }
> + }
> +
>   dprintk(1, "%s(%d-%04x)\n", __func__,
>   i2c ? i2c_adapter_id(i2c) : -1,
>   cfg ? cfg->i2c_address : -1);
> @@ -1435,6 +1446,8 @@
>   instance = hybrid_tuner_request_state(struct xc4000_priv, priv,
> hybrid_tuner_instance_list,
> i2c, cfg->i2c_address, "xc4000");
> + if (cfg->card_type != XC4000_CARD_GENERIC)
> + priv->card_type = cfg->card_type;
>   switch (instance) {
>   case 0:
>   goto fail;
> @@ -1450,7 +1463,7 @@
>   break;
>   }
>  
> - if (priv->if_khz == 0) {
> + if (cfg->if_khz != 0) {
>   /* If the IF hasn't been set yet, use the value provided by
>  the caller (occurs in hybrid devices where the analog
>  call to xc4000_attach occurs before the digital side) */
> diff -uNr xc4000_orig/drivers/media/common/tuners/xc4000.h 
> xc4000/drivers/media/common/tuners/xc4000.h
> --- xc4000_orig/drivers/media/common/tuners/xc4000.h  2011-06-03 
> 11:54:19.0 +0200
> +++ xc4000/drivers/media/common/tuners/xc4000.h   2011-06-03 
> 14:29:32.0 +0200
> @@ -27,8 +27,13 @@
>  struct dvb_frontend;
>  struct i2c_adapter;
>  
> +#define XC4000_CARD_GENERIC  0
> +#define XC4000_CARD_PCTV_340E1
> +#define XC4000_CARD_WINFAST_CX88 2
> +
>  struct xc4000_config {
> - u8  i2c_address;
> + u8  card_type;  /* if card type is not generic, all other */
> + u8  i2c_address;/* parameters are automatically set */
>   u32 if_khz;
>  };
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html