2005-12-06 19:53:04

by Prakash Punnoor

[permalink] [raw]
Subject: [PATCH] b2c2: make front-ends selectable and include noob option

Hi,

this patch probably needs some touch-up but mainly I am sking the dvb guys why
don't they do something like this. Current situation:

For my Skystar2 card I am forced to compile in half a dozen front-ends + fw
loader - but I only need one front-end driver and no fw loader. Furthermore
this crap in the log goes away by throwing unneccessary stuff out:

before:
b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver chip loaded
successfully
flexcop-pci: will use the HW PID filter.
flexcop-pci: card revision 1
ACPI: PCI Interrupt Link [APC1] enabled at IRQ 16
ACPI: PCI Interrupt 0000:01:08.0[A] -> Link [APC1] -> GSI 16 (level, high) ->
IRQ 17
DVB: registering new adapter (FlexCop Digital TV device).
b2c2-flexcop: MAC address = 00:d0:d7:03:73:66
b2c2-flexcop: i2c master_xfer failed
b2c2-flexcop: i2c master_xfer failed
b2c2-flexcop: i2c master_xfer failed
mt352_read_register: readreg error (reg=127, ret==-121)
b2c2-flexcop: i2c master_xfer failed
i2c_readbytes: i2c read error (addr 0a, err == -121)
b2c2-flexcop: i2c master_xfer failed
lgdt330x: i2c_read_demod_bytes: addr 0x59 select 0x02 error (ret == -121)
b2c2-flexcop: i2c master_xfer failed
b2c2-flexcop: i2c master_xfer failed
stv0297_readreg: readreg error (reg == 0x80, ret == -22)
b2c2-flexcop: found the vp310 (aka mt312) at i2c address: 0x0e
DVB: registering frontend 0 (Zarlink VP310 DVB-S)...
b2c2-flexcop: initialization of 'Sky2PC/SkyStar 2 DVB-S (old version)' at the
'PCI' bus controlled by a 'FlexCopII' complete

after:
b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver chip loaded
successfully
flexcop-pci: will use the HW PID filter.
flexcop-pci: card revision 1
ACPI: PCI Interrupt Link [APC1] enabled at IRQ 16
ACPI: PCI Interrupt 0000:01:08.0[A] -> Link [APC1] -> GSI 16 (level, high) ->
IRQ 17
DVB: registering new adapter (FlexCop Digital TV device).
b2c2-flexcop: MAC address = 00:d0:d7:03:73:66
b2c2-flexcop: found the vp310 (aka mt312) at i2c address: 0x0e
DVB: registering frontend 0 (Zarlink VP310 DVB-S)...
b2c2-flexcop: initialization of 'Sky2PC/SkyStar 2 DVB-S (old version)' at the
'PCI' bus controlled by a 'FlexCopII' complete

Needless to say that my kernel size is a bit smaller and my card still works.

I made the patch trying to follow: http://www.linuxjournal.com/article/5780
by Greg Kroah-Hartman, esp the paragraph "No ifdefs in .c Code". Unfortunately
there this is the problem with request_firmware(...) which I needed to ifdef
out. Probably someone has a better idea.

I also added an *option* to include every supported front-end, so that noobs
don't have to guess around and I guess this was the intention by the dvb devs
by selection all frontends by default. I only dislike the missing possibility
to deselect unwanted ones. (Is it possible to select this option by default?
I don't know much about Kbuild.)

Anyway I asked the dvb guys why the included everything unselectable by
default, but I got no response, thus I submit this patch hoping to get more
response now.

Of course, this patch only addresses the b2c2 chips. So here it goes, hoping I
haven't missed anything:

signed-off-by: Prakash Punnoor <[email protected]>


diff -urd dvb.old/b2c2/flexcop-fe-tuner.c dvb/b2c2/flexcop-fe-tuner.c
--- dvb.old/b2c2/flexcop-fe-tuner.c 2005-12-06 19:55:39.060449080 +0100
+++ dvb/b2c2/flexcop-fe-tuner.c 2005-12-06 20:29:55.144876736 +0100
@@ -293,8 +293,12 @@

static int flexcop_fe_request_firmware(struct dvb_frontend* fe, const struct
firmware **fw, char* name)
{
+#ifdef CONFIG_FW_LOADER
struct flexcop_device *fc = fe->dvb->priv;
return request_firmware(fw, name, fc->dev);
+#else
+ return -EINVAL;
+#endif
}

static int lgdt3303_pll_set(struct dvb_frontend* fe,
diff -urd dvb.old/b2c2/Kconfig dvb/b2c2/Kconfig
--- dvb.old/b2c2/Kconfig 2005-12-06 19:55:39.060449080 +0100
+++ dvb/b2c2/Kconfig 2005-12-06 20:36:36.751823200 +0100
@@ -1,6 +1,15 @@
config DVB_B2C2_FLEXCOP
tristate "Technisat/B2C2 FlexCopII(b) and FlexCopIII adapters"
depends on DVB_CORE
+ help
+ Support for the digital TV receiver chip made by B2C2 Inc. included in
+ Technisats PCI cards and USB boxes.
+
+ Say Y if you own such a device and want to use it.
+
+config DVB_B2C2_FLEXCOP_ALL_FRONTENDS
+ bool "Select all supported front-ends"
+ depends on DVB_B2C2_FLEXCOP
select DVB_STV0299
select DVB_MT352
select DVB_MT312
@@ -9,10 +18,12 @@
select DVB_BCM3510
select DVB_LGDT330X
help
- Support for the digital TV receiver chip made by B2C2 Inc. included in
- Technisats PCI cards and USB boxes.
+ Selects all supported front-ends by Technisat/B2C2 driver.
+ If you don't select this option, you must select the correct
+ front-end by hand.
+
+ Say Y if you don't know your front-end.

- Say Y if you own such a device and want to use it.

config DVB_B2C2_FLEXCOP_PCI
tristate "Technisat/B2C2 Air/Sky/Cable2PC PCI"
diff -urd dvb.old/frontends/bcm3510.h dvb/frontends/bcm3510.h
--- dvb.old/frontends/bcm3510.h 2005-12-06 19:55:39.445390560 +0100
+++ dvb/frontends/bcm3510.h 2005-12-06 20:09:37.878929288 +0100
@@ -34,7 +34,15 @@
int (*request_firmware)(struct dvb_frontend* fe, const struct firmware **fw,
char* name);
};

+#ifdef CONFIG_DVB_BCM3510
extern struct dvb_frontend* bcm3510_attach(const struct bcm3510_config*
config,
struct i2c_adapter* i2c);
+#else
+static inline struct dvb_frontend* bcm3510_attach(const struct
bcm3510_config* config,
+ struct i2c_adapter* i2c)
+{
+ return NULL;
+}
+#endif

#endif
diff -urd dvb.old/frontends/lgdt330x.h dvb/frontends/lgdt330x.h
--- dvb.old/frontends/lgdt330x.h 2005-12-06 19:55:39.645360160 +0100
+++ dvb/frontends/lgdt330x.h 2005-12-06 20:00:10.309213000 +0100
@@ -53,8 +53,16 @@
int clock_polarity_flip;
};

+#ifdef CONFIG_DVB_LGDT330X
extern struct dvb_frontend* lgdt330x_attach(const struct lgdt330x_config*
config,
struct i2c_adapter* i2c);
+#else
+static inline struct dvb_frontend* lgdt330x_attach(const struct
lgdt330x_config* config,
+ struct i2c_adapter* i2c)
+{
+ return NULL;
+}
+#endif

#endif /* LGDT330X_H */

diff -urd dvb.old/frontends/mt312.h dvb/frontends/mt312.h
--- dvb.old/frontends/mt312.h 2005-12-06 19:55:39.503381744 +0100
+++ dvb/frontends/mt312.h 2005-12-06 20:04:07.191201464 +0100
@@ -38,10 +38,24 @@
int (*pll_set)(struct dvb_frontend* fe, struct dvb_frontend_parameters*
params);
};

+#ifdef CONFIG_DVB_MT312
extern struct dvb_frontend* mt312_attach(const struct mt312_config* config,
struct i2c_adapter* i2c);

extern struct dvb_frontend* vp310_attach(const struct mt312_config* config,
struct i2c_adapter* i2c);
+#else
+static inline struct dvb_frontend* mt312_attach(const struct mt312_config*
config,
+ struct i2c_adapter* i2c)
+{
+ return NULL;
+}
+
+static inline struct dvb_frontend* vp310_attach(const struct mt312_config*
config,
+ struct i2c_adapter* i2c)
+{
+ return NULL;
+}
+#endif

#endif // MT312_H
diff -urd dvb.old/frontends/mt352.h dvb/frontends/mt352.h
--- dvb.old/frontends/mt352.h 2005-12-06 19:55:39.540376120 +0100
+++ dvb/frontends/mt352.h 2005-12-06 20:04:21.881968128 +0100
@@ -57,9 +57,22 @@
int (*pll_set)(struct dvb_frontend* fe, struct dvb_frontend_parameters*
params, u8* pllbuf);
};

+#ifdef CONFIG_DVB_MT352
extern struct dvb_frontend* mt352_attach(const struct mt352_config* config,
struct i2c_adapter* i2c);

extern int mt352_write(struct dvb_frontend* fe, u8* ibuf, int ilen);
+#else
+static inline struct dvb_frontend* mt352_attach(const struct mt352_config*
config,
+ struct i2c_adapter* i2c)
+{
+ return NULL;
+}
+
+static inline int mt352_write(struct dvb_frontend* fe, u8* ibuf, int ilen)
+{
+ return 0;
+}
+#endif

#endif // MT352_H
diff -urd dvb.old/frontends/nxt2002.h dvb/frontends/nxt2002.h
--- dvb.old/frontends/nxt2002.h 2005-12-06 19:55:39.540376120 +0100
+++ dvb/frontends/nxt2002.h 2005-12-06 20:08:39.722770368 +0100
@@ -17,7 +17,15 @@
int (*request_firmware)(struct dvb_frontend* fe, const struct firmware **fw,
char* name);
};

+#ifdef CONFIG_DVB_NXT2002
extern struct dvb_frontend* nxt2002_attach(const struct nxt2002_config*
config,
struct i2c_adapter* i2c);
+#else
+static inline struct dvb_frontend* nxt2002_attach(const struct
nxt2002_config* config,
+ struct i2c_adapter* i2c)
+{
+ return NULL;
+}
+#endif

#endif // NXT2002_H
diff -urd dvb.old/frontends/stv0297.h dvb/frontends/stv0297.h
--- dvb.old/frontends/stv0297.h 2005-12-06 19:55:39.577370496 +0100
+++ dvb/frontends/stv0297.h 2005-12-06 20:06:34.959737232 +0100
@@ -43,8 +43,21 @@
int (*pll_set)(struct dvb_frontend* fe, struct dvb_frontend_parameters*
params);
};

+#ifdef CONFIG_DVB_STV0297
extern struct dvb_frontend* stv0297_attach(const struct stv0297_config*
config,
struct i2c_adapter* i2c);
extern int stv0297_enable_plli2c(struct dvb_frontend* fe);
+#else
+static inline struct dvb_frontend* stv0297_attach(const struct
stv0297_config* config,
+ struct i2c_adapter* i2c)
+{
+ return NULL;
+}
+
+static inline int stv0297_enable_plli2c(struct dvb_frontend* fe)
+{
+ return 0;
+}
+#endif

#endif // STV0297_H
diff -urd dvb.old/frontends/stv0299.h dvb/frontends/stv0299.h
--- dvb.old/frontends/stv0299.h 2005-12-06 19:55:39.681354688 +0100
+++ dvb/frontends/stv0299.h 2005-12-06 19:59:45.158036560 +0100
@@ -93,9 +93,22 @@
int (*pll_set)(struct dvb_frontend *fe, struct i2c_adapter *i2c, struct
dvb_frontend_parameters *params);
};

+#ifdef CONFIG_DVB_STV0299
extern int stv0299_writereg (struct dvb_frontend* fe, u8 reg, u8 data);

extern struct dvb_frontend* stv0299_attach(const struct stv0299_config*
config,
struct i2c_adapter* i2c);
+#else
+static inline int stv0299_writereg (struct dvb_frontend* fe, u8 reg, u8 data)
+{
+ return 0;
+}
+
+static inline struct dvb_frontend* stv0299_attach(const struct
stv0299_config* config,
+ struct i2c_adapter* i2c)
+{
+ return NULL;
+}
+#endif

#endif // STV0299_H




--
(?= =?)
//\ Prakash Punnoor /\\
V_/ \_V


Attachments:
(No filename) (0.00 B)
(No filename) (189.00 B)
Download all attachments

2005-12-06 20:28:41

by Michael Krufky

[permalink] [raw]
Subject: Re: [PATCH] b2c2: make front-ends selectable and include noob option

I apologize if you've received this twice. My first message got
bounced from LKML... Trying again.

On 12/6/05, Prakash Punnoor <[email protected]> wrote:
> Hi,
>
> this patch probably needs some touch-up but mainly I am sking the dvb guys why
> don't they do something like this. Current situation:
>
> For my Skystar2 card I am forced to compile in half a dozen front-ends + fw
> loader - but I only need one front-end driver and no fw loader. Furthermore
> this crap in the log goes away by throwing unneccessary stuff out:
>
> before:
> b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver chip loaded
> successfully
> flexcop-pci: will use the HW PID filter.
> flexcop-pci: card revision 1
> ACPI: PCI Interrupt Link [APC1] enabled at IRQ 16
> ACPI: PCI Interrupt 0000:01:08.0[A] -> Link [APC1] -> GSI 16 (level, high) ->
> IRQ 17
> DVB: registering new adapter (FlexCop Digital TV device).
> b2c2-flexcop: MAC address = 00:d0:d7:03:73:66
> b2c2-flexcop: i2c master_xfer failed
> b2c2-flexcop: i2c master_xfer failed
> b2c2-flexcop: i2c master_xfer failed
> mt352_read_register: readreg error (reg=127, ret==-121)
> b2c2-flexcop: i2c master_xfer failed
> i2c_readbytes: i2c read error (addr 0a, err == -121)
> b2c2-flexcop: i2c master_xfer failed
> lgdt330x: i2c_read_demod_bytes: addr 0x59 select 0x02 error (ret == -121)
> b2c2-flexcop: i2c master_xfer failed
> b2c2-flexcop: i2c master_xfer failed
> stv0297_readreg: readreg error (reg == 0x80, ret == -22)
> b2c2-flexcop: found the vp310 (aka mt312) at i2c address: 0x0e
> DVB: registering frontend 0 (Zarlink VP310 DVB-S)...
> b2c2-flexcop: initialization of 'Sky2PC/SkyStar 2 DVB-S (old version)' at the
> 'PCI' bus controlled by a 'FlexCopII' complete
>
> after:
> b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver chip loaded
> successfully
> flexcop-pci: will use the HW PID filter.
> flexcop-pci: card revision 1
> ACPI: PCI Interrupt Link [APC1] enabled at IRQ 16
> ACPI: PCI Interrupt 0000:01:08.0[A] -> Link [APC1] -> GSI 16 (level, high) ->
> IRQ 17
> DVB: registering new adapter (FlexCop Digital TV device).
> b2c2-flexcop: MAC address = 00:d0:d7:03:73:66
> b2c2-flexcop: found the vp310 (aka mt312) at i2c address: 0x0e
> DVB: registering frontend 0 (Zarlink VP310 DVB-S)...
> b2c2-flexcop: initialization of 'Sky2PC/SkyStar 2 DVB-S (old version)' at the
> 'PCI' bus controlled by a 'FlexCopII' complete
>
> Needless to say that my kernel size is a bit smaller and my card still works.
>
> I made the patch trying to follow: http://www.linuxjournal.com/article/5780
> by Greg Kroah-Hartman, esp the paragraph "No ifdefs in .c Code". Unfortunately
> there this is the problem with request_firmware(...) which I needed to ifdef
> out. Probably someone has a better idea.
>
> I also added an *option* to include every supported front-end, so that noobs
> don't have to guess around and I guess this was the intention by the dvb devs
> by selection all frontends by default. I only dislike the missing possibility
> to deselect unwanted ones. (Is it possible to select this option by default?
> I don't know much about Kbuild.)
>
> Anyway I asked the dvb guys why the included everything unselectable by
> default, but I got no response, thus I submit this patch hoping to get more
> response now.
>
> Of course, this patch only addresses the b2c2 chips. So here it goes, hoping I
> haven't missed anything:
>
> signed-off-by: Prakash Punnoor <[email protected]>
>
>
> diff -urd dvb.old/b2c2/flexcop-fe-tuner.c dvb/b2c2/flexcop-fe-tuner.c
> --- dvb.old/b2c2/flexcop-fe-tuner.c 2005-12-06 19:55:39.060449080 +0100
> +++ dvb/b2c2/flexcop-fe-tuner.c 2005-12-06 20:29:55.144876736 +0100
> @@ -293,8 +293,12 @@
>
> static int flexcop_fe_request_firmware(struct dvb_frontend* fe, const struct
> firmware **fw, char* name)
> {
> +#ifdef CONFIG_FW_LOADER
> struct flexcop_device *fc = fe->dvb->priv;
> return request_firmware(fw, name, fc->dev);
> +#else
> + return -EINVAL;
> +#endif
> }
>
> static int lgdt3303_pll_set(struct dvb_frontend* fe,
> diff -urd dvb.old/b2c2/Kconfig dvb/b2c2/Kconfig
> --- dvb.old/b2c2/Kconfig 2005-12-06 19:55:39.060449080 +0100
> +++ dvb/b2c2/Kconfig 2005-12-06 20:36:36.751823200 +0100
> @@ -1,6 +1,15 @@
> config DVB_B2C2_FLEXCOP
> tristate "Technisat/B2C2 FlexCopII(b) and FlexCopIII adapters"
> depends on DVB_CORE
> + help
> + Support for the digital TV receiver chip made by B2C2 Inc. included in
> + Technisats PCI cards and USB boxes.
> +
> + Say Y if you own such a device and want to use it.
> +
> +config DVB_B2C2_FLEXCOP_ALL_FRONTENDS
> + bool "Select all supported front-ends"
> + depends on DVB_B2C2_FLEXCOP
> select DVB_STV0299
> select DVB_MT352
> select DVB_MT312
> @@ -9,10 +18,12 @@
> select DVB_BCM3510
> select DVB_LGDT330X
> help
> - Support for the digital TV receiver chip made by B2C2 Inc. included in
> - Technisats PCI cards and USB boxes.
> + Selects all supported front-ends by Technisat/B2C2 driver.
> + If you don't select this option, you must select the correct
> + front-end by hand.
> +
> + Say Y if you don't know your front-end.
>
> - Say Y if you own such a device and want to use it.
>
> config DVB_B2C2_FLEXCOP_PCI
> tristate "Technisat/B2C2 Air/Sky/Cable2PC PCI"
> diff -urd dvb.old/frontends/bcm3510.h dvb/frontends/bcm3510.h
> --- dvb.old/frontends/bcm3510.h 2005-12-06 19:55:39.445390560 +0100
> +++ dvb/frontends/bcm3510.h 2005-12-06 20:09:37.878929288 +0100
> @@ -34,7 +34,15 @@
> int (*request_firmware)(struct dvb_frontend* fe, const struct firmware **fw,
> char* name);
> };
>
> +#ifdef CONFIG_DVB_BCM3510
> extern struct dvb_frontend* bcm3510_attach(const struct bcm3510_config*
> config,
> struct i2c_adapter* i2c);
> +#else
> +static inline struct dvb_frontend* bcm3510_attach(const struct
> bcm3510_config* config,
> + struct i2c_adapter* i2c)
> +{
> + return NULL;
> +}
> +#endif
>
> #endif
> diff -urd dvb.old/frontends/lgdt330x.h dvb/frontends/lgdt330x.h
> --- dvb.old/frontends/lgdt330x.h 2005-12-06 19:55:39.645360160 +0100
> +++ dvb/frontends/lgdt330x.h 2005-12-06 20:00:10.309213000 +0100
> @@ -53,8 +53,16 @@
> int clock_polarity_flip;
> };
>
> +#ifdef CONFIG_DVB_LGDT330X
> extern struct dvb_frontend* lgdt330x_attach(const struct lgdt330x_config*
> config,
> struct i2c_adapter* i2c);
> +#else
> +static inline struct dvb_frontend* lgdt330x_attach(const struct
> lgdt330x_config* config,
> + struct i2c_adapter* i2c)
> +{
> + return NULL;
> +}
> +#endif
>
> #endif /* LGDT330X_H */
>
> diff -urd dvb.old/frontends/mt312.h dvb/frontends/mt312.h
> --- dvb.old/frontends/mt312.h 2005-12-06 19:55:39.503381744 +0100
> +++ dvb/frontends/mt312.h 2005-12-06 20:04:07.191201464 +0100
> @@ -38,10 +38,24 @@
> int (*pll_set)(struct dvb_frontend* fe, struct dvb_frontend_parameters*
> params);
> };
>
> +#ifdef CONFIG_DVB_MT312
> extern struct dvb_frontend* mt312_attach(const struct mt312_config* config,
> struct i2c_adapter* i2c);
>
> extern struct dvb_frontend* vp310_attach(const struct mt312_config* config,
> struct i2c_adapter* i2c);
> +#else
> +static inline struct dvb_frontend* mt312_attach(const struct mt312_config*
> config,
> + struct i2c_adapter* i2c)
> +{
> + return NULL;
> +}
> +
> +static inline struct dvb_frontend* vp310_attach(const struct mt312_config*
> config,
> + struct i2c_adapter* i2c)
> +{
> + return NULL;
> +}
> +#endif
>
> #endif // MT312_H
> diff -urd dvb.old/frontends/mt352.h dvb/frontends/mt352.h
> --- dvb.old/frontends/mt352.h 2005-12-06 19:55:39.540376120 +0100
> +++ dvb/frontends/mt352.h 2005-12-06 20:04:21.881968128 +0100
> @@ -57,9 +57,22 @@
> int (*pll_set)(struct dvb_frontend* fe, struct dvb_frontend_parameters*
> params, u8* pllbuf);
> };
>
> +#ifdef CONFIG_DVB_MT352
> extern struct dvb_frontend* mt352_attach(const struct mt352_config* config,
> struct i2c_adapter* i2c);
>
> extern int mt352_write(struct dvb_frontend* fe, u8* ibuf, int ilen);
> +#else
> +static inline struct dvb_frontend* mt352_attach(const struct mt352_config*
> config,
> + struct i2c_adapter* i2c)
> +{
> + return NULL;
> +}
> +
> +static inline int mt352_write(struct dvb_frontend* fe, u8* ibuf, int ilen)
> +{
> + return 0;
> +}
> +#endif
>
> #endif // MT352_H
> diff -urd dvb.old/frontends/nxt2002.h dvb/frontends/nxt2002.h
> --- dvb.old/frontends/nxt2002.h 2005-12-06 19:55:39.540376120 +0100
> +++ dvb/frontends/nxt2002.h 2005-12-06 20:08:39.722770368 +0100
> @@ -17,7 +17,15 @@
> int (*request_firmware)(struct dvb_frontend* fe, const struct firmware **fw,
> char* name);
> };
>
> +#ifdef CONFIG_DVB_NXT2002
> extern struct dvb_frontend* nxt2002_attach(const struct nxt2002_config*
> config,
> struct i2c_adapter* i2c);
> +#else
> +static inline struct dvb_frontend* nxt2002_attach(const struct
> nxt2002_config* config,
> + struct i2c_adapter* i2c)
> +{
> + return NULL;
> +}
> +#endif
>
> #endif // NXT2002_H
> diff -urd dvb.old/frontends/stv0297.h dvb/frontends/stv0297.h
> --- dvb.old/frontends/stv0297.h 2005-12-06 19:55:39.577370496 +0100
> +++ dvb/frontends/stv0297.h 2005-12-06 20:06:34.959737232 +0100
> @@ -43,8 +43,21 @@
> int (*pll_set)(struct dvb_frontend* fe, struct dvb_frontend_parameters*
> params);
> };
>
> +#ifdef CONFIG_DVB_STV0297
> extern struct dvb_frontend* stv0297_attach(const struct stv0297_config*
> config,
> struct i2c_adapter* i2c);
> extern int stv0297_enable_plli2c(struct dvb_frontend* fe);
> +#else
> +static inline struct dvb_frontend* stv0297_attach(const struct
> stv0297_config* config,
> + struct i2c_adapter* i2c)
> +{
> + return NULL;
> +}
> +
> +static inline int stv0297_enable_plli2c(struct dvb_frontend* fe)
> +{
> + return 0;
> +}
> +#endif
>
> #endif // STV0297_H
> diff -urd dvb.old/frontends/stv0299.h dvb/frontends/stv0299.h
> --- dvb.old/frontends/stv0299.h 2005-12-06 19:55:39.681354688 +0100
> +++ dvb/frontends/stv0299.h 2005-12-06 19:59:45.158036560 +0100
> @@ -93,9 +93,22 @@
> int (*pll_set)(struct dvb_frontend *fe, struct i2c_adapter *i2c, struct
> dvb_frontend_parameters *params);
> };
>
> +#ifdef CONFIG_DVB_STV0299
> extern int stv0299_writereg (struct dvb_frontend* fe, u8 reg, u8 data);
>
> extern struct dvb_frontend* stv0299_attach(const struct stv0299_config*
> config,
> struct i2c_adapter* i2c);
> +#else
> +static inline int stv0299_writereg (struct dvb_frontend* fe, u8 reg, u8 data)
> +{
> + return 0;
> +}
> +
> +static inline struct dvb_frontend* stv0299_attach(const struct
> stv0299_config* config,
> + struct i2c_adapter* i2c)
> +{
> + return NULL;
> +}
> +#endif
>
> #endif // STV0299_H
>
>
>
>
> --
> (?= =?)
> //\ Prakash Punnoor /\\
> V_/ \_V
>
>
>


NACK.

You are going to run into some problems with this patch... For
instance, What if the user chooses to compile the b2c2-flexcop driver
in-kernel, but only compiles the frontend drivers as modules... Then,
the frontend support will be built into the flexcop driver, but the
module will not yet be available at the time when the kernel is
looking for it...

Take a look at what I did to the following files.... You may want to
apply the same method to the flexcop build, although I am beginning to
think that this is more trouble that it's worth...

I am considering reverting the frontend selection patch for cx88 and
saa7134 from 2.6.15 , if I hear one more complaint about it...

I will say it publicly -- Johannes originally didn't like this idea,
and now I see why, although my solution seems to be working well, but
I had only intended on sending this to -mm ..... It got merged into
2.6.15 too quickly, but luckily it seems to be working well.

Take a look at what I have done to the following files to enable
advanced frontend selection for cx88-dvb and saa7134-dvb :

drivers/media/video/cx88
/Kconfig
drivers/media/video/cx88/Makefile
drivers/media/video/saa7134/Kconfig
drivers/media/video/saa7134/Makefile

If you see above, select all frontends is selected by default,
because this is the desired behavior, and the previous bahevior that
everyone is used to.

When that option is disabled, the user is presented with a list of
frontends to choose from... The frontends are selected with the same
tristate value of the [cx88,saa7134]-dvb driver ..... ie:

cx88-dvb is M , frontends will be M
cx88-dvb is Y, frontends will be Y

This MUST happen in this exact manner, otherwise you will get
unresolved symbols, and a useless installation.

I'll tell you the truth, I think Johannes is going to ask that I
revert this frontend selection thing.... If he does, I will respect
his wishes. Now that v4l and dvb development has merged into a single
cvs repository, I believe that we will find new and better ways to
accomplish this "frontend selection" goal........

Regards,

Michael Krufky

P.S.: To anyone interested, my regular email service is down right
now.... You can reach me this gmail address. Please cc me on replies
to this message, as my gmail account is not subscriber to the
linux-dvb-maintainer list. Thank you.

2005-12-06 20:39:24

by Prakash Punnoor

[permalink] [raw]
Subject: Re: [PATCH] b2c2: make front-ends selectable and include noob option

Am Dienstag Dezember 6 2005 21:20 schrieb Michael Krufky:
> On 12/6/05, Prakash Punnoor <[email protected]> wrote:
> > Hi,
> >
> > this patch probably needs some touch-up but mainly I am sking the dvb
> > guys why
> > don't they do something like this. Current situation:
> >
>
> NACK.
>
> You are going to run into some problems with this patch... For instance,
> What if the user chooses to compile the b2c2-flexcop driver in-kernel, but
> only compiles the frontend drivers as modules... Then, the frontend
> support will be built into the flexcop driver, but the module will not yet
> be available at the time when the kernel is looking for it...

Well, I said it needed touch up. ;-) After all I didn't seriously believe it
gets merged in current state (and yes, I didn't think about the module issue,
but you're right , of course). But it simply didn't seem like dvb guys are
caring about the problem. I once (probably half a year ago already) mailed to
linux-dvb and got zero response. That told me everything.

Personally I won't invest more time in perfecting the patch. I just wanted to
get some attention to this problem and will use the patch privately for my
own happiness...

Thanks for your input.

--
(°= =°)
//\ Prakash Punnoor /\\
V_/ \_V


Attachments:
(No filename) (1.27 kB)
(No filename) (189.00 B)
Download all attachments

2005-12-06 20:42:26

by Michael Krufky

[permalink] [raw]
Subject: Re: [PATCH] b2c2: make front-ends selectable and include noob option

On 12/6/05, Prakash Punnoor <[email protected]> wrote:
> Am Dienstag Dezember 6 2005 21:20 schrieb Michael Krufky:
> > On 12/6/05, Prakash Punnoor <[email protected]> wrote:
> > > Hi,
> > >
> > > this patch probably needs some touch-up but mainly I am sking the dvb
> > > guys why
> > > don't they do something like this. Current situation:
> > >
> >
> > NACK.
> >
> > You are going to run into some problems with this patch... For instance,
> > What if the user chooses to compile the b2c2-flexcop driver in-kernel, but
> > only compiles the frontend drivers as modules... Then, the frontend
> > support will be built into the flexcop driver, but the module will not yet
> > be available at the time when the kernel is looking for it...
>
> Well, I said it needed touch up. ;-) After all I didn't seriously believe it
> gets merged in current state (and yes, I didn't think about the module issue,
> but you're right , of course). But it simply didn't seem like dvb guys are
> caring about the problem. I once (probably half a year ago already) mailed to
> linux-dvb and got zero response. That told me everything.
>
> Personally I won't invest more time in perfecting the patch. I just wanted to
> get some attention to this problem and will use the patch privately for my
> own happiness...
>
> Thanks for your input.

Okay, that sounds reasonable... I will make it a point to discuss this
issue with Johannes, and either

a) revert my frontend selection patch

or

b) apply the same logic that I employed to [cx88,saa7134]-dvb to the
other drivers.

Thanks,

Michael

2005-12-06 21:55:59

by Johannes Stezenbach

[permalink] [raw]
Subject: Re: [linux-dvb-maintainer] Re: [PATCH] b2c2: make front-ends selectable and include noob option

On Tue, Dec 06, 2005, Prakash Punnoor wrote:
> Am Dienstag Dezember 6 2005 21:20 schrieb Michael Krufky:
> > On 12/6/05, Prakash Punnoor <[email protected]> wrote:
> > > this patch probably needs some touch-up but mainly I am sking the dvb
> > > guys why
> > > don't they do something like this. Current situation:
> >
> > NACK.
> >
> > You are going to run into some problems with this patch... For instance,
> > What if the user chooses to compile the b2c2-flexcop driver in-kernel, but
> > only compiles the frontend drivers as modules... Then, the frontend
> > support will be built into the flexcop driver, but the module will not yet
> > be available at the time when the kernel is looking for it...
>
> Well, I said it needed touch up. ;-) After all I didn't seriously believe it
> gets merged in current state (and yes, I didn't think about the module issue,
> but you're right , of course). But it simply didn't seem like dvb guys are
> caring about the problem. I once (probably half a year ago already) mailed to
> linux-dvb and got zero response. That told me everything.

I make it a point to ignore postings which ignore
the recent mailing list history ;-)

This had been discussed on linux-dvb and the consensus was that
no one wants to invest time to maintain an #ifdef mess
just so that people can save a few KB in their kernel.

Also, most users don't know and don't care what demodulator
their card has, the dependency on all of them, plus the
implied auto probing saves them some headaches and us a lot of
newbie questions.

> Personally I won't invest more time in perfecting the patch. I just wanted to
> get some attention to this problem and will use the patch privately for my
> own happiness...

The b2c2-flexcop-pci driver could certainly use some fixing. Your
patch just hides the driver problems by deselecting functionality
that _you_ don't need.


Johannes

2005-12-06 22:20:06

by Michael Krufky

[permalink] [raw]
Subject: Re: [linux-dvb-maintainer] Re: [PATCH] b2c2: make front-ends selectable and include noob option

On 12/6/05, Johannes Stezenbach <[email protected]> wrote:
> On Tue, Dec 06, 2005, Prakash Punnoor wrote:
> > Am Dienstag Dezember 6 2005 21:20 schrieb Michael Krufky:
> > > On 12/6/05, Prakash Punnoor <[email protected]> wrote:
> > > > this patch probably needs some touch-up but mainly I am sking the dvb
> > > > guys why
> > > > don't they do something like this. Current situation:
> > >
> > > NACK.
> > >
> > > You are going to run into some problems with this patch... For instance,
> > > What if the user chooses to compile the b2c2-flexcop driver in-kernel, but
> > > only compiles the frontend drivers as modules... Then, the frontend
> > > support will be built into the flexcop driver, but the module will not yet
> > > be available at the time when the kernel is looking for it...
> >
> > Well, I said it needed touch up. ;-) After all I didn't seriously believe it
> > gets merged in current state (and yes, I didn't think about the module issue,
> > but you're right , of course). But it simply didn't seem like dvb guys are
> > caring about the problem. I once (probably half a year ago already) mailed to
> > linux-dvb and got zero response. That told me everything.
>
> I make it a point to ignore postings which ignore
> the recent mailing list history ;-)
>
> This had been discussed on linux-dvb and the consensus was that
> no one wants to invest time to maintain an #ifdef mess
> just so that people can save a few KB in their kernel.
>
> Also, most users don't know and don't care what demodulator
> their card has, the dependency on all of them, plus the
> implied auto probing saves them some headaches and us a lot of
> newbie questions.
>
> > Personally I won't invest more time in perfecting the patch. I just wanted to
> > get some attention to this problem and will use the patch privately for my
> > own happiness...
>
> The b2c2-flexcop-pci driver could certainly use some fixing. Your
> patch just hides the driver problems by deselecting functionality
> that _you_ don't need.
>

Johannes -

If you approve of the method that I used to implement compile-time
frontend selection in cx88-dvb and saa7134-dvb, then I would be happy
to implement this into the flexcop driver as well. I understand why
developers might not want to invest the time into this, but I see the
benefits to it, and I am willing to work on it. Of course, this
wouldnt be sent to the kernel until 2.6.16 (or maybe 2.6.17)

I have the bcm3510-based board in my possesion right now, and both the
nxt2002 and lgdt3303 versions are in the mail to me as I type this
email. (I plan to use the nxt2002 version to test and fix the nxt200x
driver for this use.)

OTOH, if there are other reasons to stay away from this idea, say the
word, and I'll send a revert patch to Andrew for the cx88 and saa7134
stuff... It is working well -- Gene reported a bug and I fixed it,
already in Linus' tree.

What do you think?

-Michael

2005-12-07 00:29:08

by Johannes Stezenbach

[permalink] [raw]
Subject: Re: [linux-dvb-maintainer] Re: [PATCH] b2c2: make front-ends selectable and include noob option

On Tue, Dec 06, 2005, Michael Krufky wrote:
> On 12/6/05, Johannes Stezenbach <[email protected]> wrote:
> > On Tue, Dec 06, 2005, Prakash Punnoor wrote:
> > > Well, I said it needed touch up. ;-) After all I didn't seriously believe it
> > > gets merged in current state (and yes, I didn't think about the module issue,
> > > but you're right , of course). But it simply didn't seem like dvb guys are
> > > caring about the problem. I once (probably half a year ago already) mailed to
> > > linux-dvb and got zero response. That told me everything.
> >
> > I make it a point to ignore postings which ignore
> > the recent mailing list history ;-)
> >
> > This had been discussed on linux-dvb and the consensus was that
> > no one wants to invest time to maintain an #ifdef mess
> > just so that people can save a few KB in their kernel.
> >
> > Also, most users don't know and don't care what demodulator
> > their card has, the dependency on all of them, plus the
> > implied auto probing saves them some headaches and us a lot of
> > newbie questions.
> >
> > > Personally I won't invest more time in perfecting the patch. I just wanted to
> > > get some attention to this problem and will use the patch privately for my
> > > own happiness...
> >
> > The b2c2-flexcop-pci driver could certainly use some fixing. Your
> > patch just hides the driver problems by deselecting functionality
> > that _you_ don't need.
>
> If you approve of the method that I used to implement compile-time
> frontend selection in cx88-dvb and saa7134-dvb, then I would be happy
> to implement this into the flexcop driver as well. I understand why
> developers might not want to invest the time into this, but I see the
> benefits to it, and I am willing to work on it. Of course, this
> wouldnt be sent to the kernel until 2.6.16 (or maybe 2.6.17)
>
> I have the bcm3510-based board in my possesion right now, and both the
> nxt2002 and lgdt3303 versions are in the mail to me as I type this
> email. (I plan to use the nxt2002 version to test and fix the nxt200x
> driver for this use.)
>
> OTOH, if there are other reasons to stay away from this idea, say the
> word, and I'll send a revert patch to Andrew for the cx88 and saa7134
> stuff... It is working well -- Gene reported a bug and I fixed it,
> already in Linus' tree.
>
> What do you think?

I think b2c2-flexcop-pci uses a 240K dma buffer, whether you
save a few K in demodulator code doesn't mean much.
The saved memory will be similarly unnoticable to the user as
if you would go and scatter #ifdefs all over tuner-simple.c.

But I'm neither the author nor the maintainer of the b2c2-flexcop
driver, you better ask Patrick if he likes it.


Johannes

2005-12-07 07:29:26

by Prakash Punnoor

[permalink] [raw]
Subject: Re: [linux-dvb-maintainer] Re: [PATCH] b2c2: make front-ends selectable and include noob option

Am Dienstag Dezember 6 2005 22:56 schrieb Johannes Stezenbach:
> On Tue, Dec 06, 2005, Prakash Punnoor wrote:
> > Well, I said it needed touch up. ;-) After all I didn't seriously believe
> > it gets merged in current state (and yes, I didn't think about the module
> > issue, but you're right , of course). But it simply didn't seem like dvb
> > guys are caring about the problem. I once (probably half a year ago
> > already) mailed to linux-dvb and got zero response. That told me
> > everything.
>
> I make it a point to ignore postings which ignore
> the recent mailing list history ;-)
>
> This had been discussed on linux-dvb and the consensus was that
> no one wants to invest time to maintain an #ifdef mess
> just so that people can save a few KB in their kernel.

A short "search the list" note would have been enough, then. I think I
followed the list, but probably skipped the messages, as the subject probably
seemed like something else.


> Also, most users don't know and don't care what demodulator
> their card has, the dependency on all of them, plus the
> implied auto probing saves them some headaches and us a lot of
> newbie questions.


Well, could you be so nice and at least turn that xfer transfer errors and
similar off? I guess that should be debugging output, but only printed when
you actually want debugging and shouldn't be default behaviour. Does't this
confuse users? It at least always draws my attention...

bye,
--
(?= =?)
//\ Prakash Punnoor /\\
V_/ \_V


Attachments:
(No filename) (1.50 kB)
(No filename) (189.00 B)
Download all attachments

2005-12-07 07:57:21

by Patrick Boettcher

[permalink] [raw]
Subject: Re: [linux-dvb-maintainer] Re: [PATCH] b2c2: make front-ends selectable and include noob option

Hi,

On Wed, 7 Dec 2005, Johannes Stezenbach wrote:
> I think b2c2-flexcop-pci uses a 240K dma buffer, whether you
> save a few K in demodulator code doesn't mean much.
> The saved memory will be similarly unnoticable to the user as
> if you would go and scatter #ifdefs all over tuner-simple.c.
>
> But I'm neither the author nor the maintainer of the b2c2-flexcop
> driver, you better ask Patrick if he likes it.

There will be at least two new devices in the future, which again will
need (at least) two new demod(and maybe tuner)-modules.

Prakash, if it is just the i2c_xfer failed, that could be easily turned
into a debug-message, but I rather have these as errors, because they are.

I don't see the need for the ifdef - on the contrary: the number of people
asking for which demod they shall load dropped significantly after
FE_REFACTORING starting one year ago - now reintroducing that again is not
a good idea, IMO.

best regards,
Patrick.

--
Mail: [email protected]
WWW: http://www.wi-bw.tfh-wildau.de/~pboettch/

2005-12-07 08:25:21

by Prakash Punnoor

[permalink] [raw]
Subject: Re: [linux-dvb-maintainer] Re: [PATCH] b2c2: make front-ends selectable and include noob option

Am Mittwoch Dezember 7 2005 08:56 schrieb Patrick Boettcher:
> Hi,
>
> On Wed, 7 Dec 2005, Johannes Stezenbach wrote:
> > I think b2c2-flexcop-pci uses a 240K dma buffer, whether you
> > save a few K in demodulator code doesn't mean much.
> > The saved memory will be similarly unnoticable to the user as
> > if you would go and scatter #ifdefs all over tuner-simple.c.
> >
> > But I'm neither the author nor the maintainer of the b2c2-flexcop
> > driver, you better ask Patrick if he likes it.
>
> There will be at least two new devices in the future, which again will
> need (at least) two new demod(and maybe tuner)-modules.
>
> Prakash, if it is just the i2c_xfer failed, that could be easily turned
> into a debug-message, but I rather have these as errors, because they are.

Well aren't these expected errors? You want to attach an non-existent
front-end and the driver finds out it isn't there? IMHO it doesn't look very
professional if some strange errors are scattered in the log. I think if you
don't find *any* front-end there should be perhaps an error or note to the
user, but not because half a dozen of front-ends drivers can't find the hw.
At least please make the errors verbose and not cryptical, if you really want
them.

If these errors are not expected, they have been here since months and
probably should have been fixed. But I think I remember someone saying that
they are expected once I reported them...

> I don't see the need for the ifdef - on the contrary: the number of people
> asking for which demod they shall load dropped significantly after
> FE_REFACTORING starting one year ago - now reintroducing that again is not
> a good idea, IMO.

Well, I got twice annoyed by missing reverse dependecies introduced by new
front-ends. Once the fw loader and once the new lgt330x(?) front-end. If new
front-ends wouldn't go in automatically as rev dependency this wouldn't break
for me (and everyone alse using b2c2 in general and not only the new
frontends), which is another reason, if you don't care about the bit of code
saving.

I really don't see the big problem of the ifdefs as the are in the *header*
files as it is just checking whether something is in or not - as the user
selected thus emulating that a probe for a front-end failed... If the .c file
is coded properly there is no chance of breaking - otherwise you'd have the
same problem with the unpatched source (leaving the module problem out for
now). (Yes, I know ifdefs in the code as such are bad, but I noted that in
conjunction with the fw loader, which was the only "dirty" ifdef.)

And as I said I added an *option* for unsure users to include all supported
front-ends, which is another one of your arguments. Selecting it by default
(I don't know how to do this) would be what you want then. Wouldn't this make
all happy (if Michael additionally fixes the module stuff)?

bye,

--
(?= =?)
//\ Prakash Punnoor /\\
V_/ \_V


Attachments:
(No filename) (2.91 kB)
(No filename) (189.00 B)
Download all attachments