Hi,
drivers/media/dvb/b2c2/flexcop-dma.c uses the PCI DMA API, but DVB_B2C2_FLEXCOP
doesn't depend on PCI, causing the following problem on PCI-less systems:
| linux/drivers/media/dvb/b2c2/flexcop-dma.c:20: warning: implicit declaration of function 'pci_alloc_consistent'
| linux/drivers/media/dvb/b2c2/flexcop-dma.c:20: warning: implicit declaration of function 'pci_alloc_consistent'
Apparently this is the flexcop DMA core, which is used by both
DVB_B2C2_FLEXCOP_PCI and DVB_B2C2_FLEXCOP_USB.
DVB_B2C2_FLEXCOP_PCI depends on PCI.
DVB_B2C2_FLEXCOP_USB depends on USB.
Perhaps the flexcop DMA core should use the generic DMA API instead?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
Em Qua, 2007-06-06 às 21:11 +0200, Geert Uytterhoeven escreveu:
> Hi,
>
> drivers/media/dvb/b2c2/flexcop-dma.c uses the PCI DMA API, but DVB_B2C2_FLEXCOP
> doesn't depend on PCI, causing the following problem on PCI-less systems:
>
> | linux/drivers/media/dvb/b2c2/flexcop-dma.c:20: warning: implicit declaration of function 'pci_alloc_consistent'
> | linux/drivers/media/dvb/b2c2/flexcop-dma.c:20: warning: implicit declaration of function 'pci_alloc_consistent'
>
> Apparently this is the flexcop DMA core, which is used by both
> DVB_B2C2_FLEXCOP_PCI and DVB_B2C2_FLEXCOP_USB.
>
> DVB_B2C2_FLEXCOP_PCI depends on PCI.
> DVB_B2C2_FLEXCOP_USB depends on USB.
Thanks for pointing us about this issue. While the usage of the generic
dma is the better way, a simple fix can be applied by simply moving
flexcop-dma to b2c2-flexcop-pci (currently, only the last uses the DMA
stuff). I've committed such patch. It is available at:
http://linuxtv.org/hg/v4l-dvb/rev/c314ec17335a
Cheers,
Mauro
Mauro Carvalho Chehab wrote:
> Hi Geert,
>
> Em Qua, 2007-06-06 às 21:11 +0200, Geert Uytterhoeven escreveu:
>
>> Hi,
>>
>> drivers/media/dvb/b2c2/flexcop-dma.c uses the PCI DMA API, but DVB_B2C2_FLEXCOP
>> doesn't depend on PCI, causing the following problem on PCI-less systems:
>>
>> | linux/drivers/media/dvb/b2c2/flexcop-dma.c:20: warning: implicit declaration of function 'pci_alloc_consistent'
>> | linux/drivers/media/dvb/b2c2/flexcop-dma.c:20: warning: implicit declaration of function 'pci_alloc_consistent'
>>
>> Apparently this is the flexcop DMA core, which is used by both
>> DVB_B2C2_FLEXCOP_PCI and DVB_B2C2_FLEXCOP_USB.
>>
>> DVB_B2C2_FLEXCOP_PCI depends on PCI.
>> DVB_B2C2_FLEXCOP_USB depends on USB.
>>
>
> Thanks for pointing us about this issue. While the usage of the generic
> dma is the better way, a simple fix can be applied by simply moving
> flexcop-dma to b2c2-flexcop-pci (currently, only the last uses the DMA
> stuff). I've committed such patch. It is available at:
>
> http://linuxtv.org/hg/v4l-dvb/rev/c314ec17335a
>
Mauro,
It appears that your change has caused the following build warning:
WARNING: "b2c2_flexcop_debug"
[/home/mk/v4l-dvb-master/v4l/b2c2-flexcop-pci.ko] undefined!
Regards,
Mike
> Mauro,
>
> It appears that your change has caused the following build warning:
>
> WARNING: "b2c2_flexcop_debug"
> [/home/mk/v4l-dvb-master/v4l/b2c2-flexcop-pci.ko] undefined!
Weird, this error didn't appeared on my tests here.
Ok, I'll work on fixing this.
I have another alternative working only on Kconfig stuff, but this seems
to be the cleaner one.
--
Cheers,
Mauro
On Thu, 7 Jun 2007, Mauro Carvalho Chehab wrote:
> > Mauro,
> >
> > It appears that your change has caused the following build warning:
> >
> > WARNING: "b2c2_flexcop_debug"
> > [/home/mk/v4l-dvb-master/v4l/b2c2-flexcop-pci.ko] undefined!
>
> Weird, this error didn't appeared on my tests here.
>
> Ok, I'll work on fixing this.
>
> I have another alternative working only on Kconfig stuff, but this seems
> to be the cleaner one.
You probably had CONFIG_DVB_B2C2_FLEXCOP_DEBUG off, in which case the
debugging stuff isn't used.
The problem is that when flexcop-dma.c was part of b2c2-flexcop.ko, it used a
non-exported global called b2c2_flexcop_debug.
In flexcop-pci.c, there is a static global called debug, which is slightly
different. They are both a set of bit flags but the flags are different.
Since the functions in flexcop-dma are only used by flexcop-pci, it makes much
more sense to make them part of the pci module instead of the common module.
It will effectively reduce the size of the flexcop-usb driver, since the
pci-only dma functions won't be loaded.
I see three ways to fix the debug symbol:
A) Export b2c2_flexcop_debug in flexcop.c, so that flexcop-dma.c can use it.
The debug messages flexcop-dma writes will be controlled the same way they
were before.
But, this means some debug messages from b2c2-flexcop-pci.ko will be
controlled by the debug parameter of b2c2-flexcop-pci.ko (those from
flexcop-pci.c), and some (those from flexcop-dma.c) will be controlled by
the debug parameter of b2c2-flexcop.ko. IMHO, that's messed up.
B) Change flexcop-dma.c to use the debug parameter of flexcop-pci.c. This
means that parameter will need to change from static global to non-exported
non-static global.
The debug messages from flexcop-dma will now be controlled by the
b2c2-flexcop-pci.ko debug parameter, since it's part of that module.
C) Do away with the separate debug parameter in flexcop-pci, and just export
b2c2_flexcop_debug. Both flexcop-pci and flexcop-usb debugging messages
will be controlled by this parameter. This makes the parameter and the
code the simplest, but it's more limited. Maybe someone just wants
messages from the pci or usb module? It also makes sense to me that
messags printing by module X should be controlled by a parameter to module
X.
Here is a patch that does just option B. I have a couple other
patches too.
--------------------------------------------------------------------------
flexcop: Have flexcop-dma use the flexcop-pci debug parameter
From: Trent Piepho <[email protected]>
Another patch moved flexcop-dma.c from the flexcop common module
(b2c2-flexcop.ko) to the flexcop pci module (b2c2-flexcop-pci.ko), since it's
PCI specific.
Debug printing was controlled by b2c2-flexcop.ko's debug parameter, but now
that it's in b2c2-flexcop-pci.ko, it makes sense for debug printing to be
controlled by b2c2-flexcop-pci.ko's debug parameter.
Signed-off-by: Trent Piepho <[email protected]>
diff --git a/linux/drivers/media/dvb/b2c2/flexcop-dma.c b/linux/drivers/media/dvb/b2c2/flexcop-dma.c
--- a/linux/drivers/media/dvb/b2c2/flexcop-dma.c
+++ b/linux/drivers/media/dvb/b2c2/flexcop-dma.c
@@ -5,7 +5,22 @@
*
* see flexcop.c for copyright information.
*/
-#include "flexcop.h"
+#define FC_LOG_PREFIX "flexcop-pci"
+#include "flexcop-common.h"
+
+#ifdef CONFIG_DVB_B2C2_FLEXCOP_DEBUG
+#define dprintk(level,args...) \
+ do { if ((b2c2_flexcop_pci_debug & level)) printk(args); } while (0)
+#define DEBSTATUS ""
+#else
+#define dprintk(level,args...)
+#define DEBSTATUS " (debugging is not enabled)"
+#endif
+
+#define deb_info(args...) dprintk(0x01,args)
+#define deb_reg(args...) dprintk(0x02,args)
+
+extern int b2c2_flexcop_pci_debug;
int flexcop_dma_allocate(struct pci_dev *pdev, struct flexcop_dma *dma, u32 size)
{
@@ -88,8 +103,8 @@ int flexcop_dma_xfer_control(struct flex
v0x0 = fc->read_ibi_reg(fc,r0x0);
v0xc = fc->read_ibi_reg(fc,r0xc);
- deb_rdump("reg: %03x: %x\n",r0x0,v0x0.raw);
- deb_rdump("reg: %03x: %x\n",r0xc,v0xc.raw);
+ deb_reg("reg: %03x: %x\n",r0x0,v0x0.raw);
+ deb_reg("reg: %03x: %x\n",r0xc,v0xc.raw);
if (index & FC_DMA_SUBADDR_0)
v0x0.dma_0x0.dma_0start = onoff;
@@ -100,8 +115,8 @@ int flexcop_dma_xfer_control(struct flex
fc->write_ibi_reg(fc,r0x0,v0x0);
fc->write_ibi_reg(fc,r0xc,v0xc);
- deb_rdump("reg: %03x: %x\n",r0x0,v0x0.raw);
- deb_rdump("reg: %03x: %x\n",r0xc,v0xc.raw);
+ deb_reg("reg: %03x: %x\n",r0x0,v0x0.raw);
+ deb_reg("reg: %03x: %x\n",r0xc,v0xc.raw);
return 0;
}
EXPORT_SYMBOL(flexcop_dma_xfer_control);
@@ -178,7 +193,7 @@ int flexcop_dma_control_packet_irq(struc
{
flexcop_ibi_value v = fc->read_ibi_reg(fc,ctrl_208);
- deb_rdump("reg: %03x: %x\n",ctrl_208,v.raw);
+ deb_reg("reg: %03x: %x\n",ctrl_208,v.raw);
if (no & FC_DMA_1)
v.ctrl_208.DMA1_Size_IRQ_Enable_sig = onoff;
@@ -186,7 +201,7 @@ int flexcop_dma_control_packet_irq(struc
v.ctrl_208.DMA2_Size_IRQ_Enable_sig = onoff;
fc->write_ibi_reg(fc,ctrl_208,v);
- deb_rdump("reg: %03x: %x\n",ctrl_208,v.raw);
+ deb_reg("reg: %03x: %x\n",ctrl_208,v.raw);
return 0;
}
diff --git a/linux/drivers/media/dvb/b2c2/flexcop-pci.c b/linux/drivers/media/dvb/b2c2/flexcop-pci.c
--- a/linux/drivers/media/dvb/b2c2/flexcop-pci.c
+++ b/linux/drivers/media/dvb/b2c2/flexcop-pci.c
@@ -19,7 +19,7 @@ MODULE_PARM_DESC(irq_chk_intv, "set the
#ifdef CONFIG_DVB_B2C2_FLEXCOP_DEBUG
#define dprintk(level,args...) \
- do { if ((debug & level)) printk(args); } while (0)
+ do { if ((b2c2_flexcop_pci_debug & level)) printk(args); } while (0)
#define DEBSTATUS ""
#else
#define dprintk(level,args...)
@@ -32,8 +32,8 @@ MODULE_PARM_DESC(irq_chk_intv, "set the
#define deb_irq(args...) dprintk(0x08,args)
#define deb_chk(args...) dprintk(0x10,args)
-static int debug = 0;
-module_param(debug, int, 0644);
+int b2c2_flexcop_pci_debug = 0;
+module_param_named(debug, b2c2_flexcop_pci_debug, int, 0644);
MODULE_PARM_DESC(debug, "set debug level (1=info,2=regs,4=TS,8=irqdma (|-able))." DEBSTATUS);
#define DRIVER_VERSION "0.1"
Hi Trent,
> Here is a patch that does just option B. I have a couple other
> patches too.
Tracking the flexcop, I've arrived on a similar patch to yours, but this
approach will duplicate some debug macros. This is somewhat ugly. Since
we need this fix for a late -rc, I think that the better for now is to
use an optional code on Makefile that will add flexcop-dma to
b2c2-flexcop only if flexcop pci support is selected. The end result is
very simple, and will solve the issue.
The final patch is this one:
http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=ee9a55464446146acbe12556d6de12cad0ac18c7
Cheers,
Mauro
On Fri, 8 Jun 2007, Mauro Carvalho Chehab wrote:
> Hi Trent,
>
> > Here is a patch that does just option B. I have a couple other
> > patches too.
>
> Tracking the flexcop, I've arrived on a similar patch to yours, but this
> approach will duplicate some debug macros. This is somewhat ugly. Since
> we need this fix for a late -rc, I think that the better for now is to
> use an optional code on Makefile that will add flexcop-dma to
> b2c2-flexcop only if flexcop pci support is selected. The end result is
> very simple, and will solve the issue.
That is a simple way to solve the issue. My patch was just the simplest
one that made flexcop-dma work inside the flexcop-pci module, which is more
complex, but a better way overall I think. I had another patch that
removed the duplicated macros. The patch series is here:
http://linuxtv.org/hg/~tap/flexcop
Maybe it would be ok for 2.6.23? I'll post the two new patches inline so
they're easier to look at.
-----------------------------------------------------------------
flexcop: remove unnecessary exports
From: Trent Piepho <[email protected]>
flexcop-dma.c used to be part of b2c2-flexcop.ko, and all its exported
functions were only used by b2c2-flexcop-pci.ko. Now it's part of
b2c2-flexcop-pci, so none of its functions need to be exported anymore.
Signed-off-by: Trent Piepho <[email protected]>
diff --git a/linux/drivers/media/dvb/b2c2/flexcop-dma.c b/linux/drivers/media/dvb/b2c2/flexcop-dma.c
--- a/linux/drivers/media/dvb/b2c2/flexcop-dma.c
+++ b/linux/drivers/media/dvb/b2c2/flexcop-dma.c
@@ -43,14 +43,12 @@ int flexcop_dma_allocate(struct pci_dev
}
return -ENOMEM;
}
-EXPORT_SYMBOL(flexcop_dma_allocate);
void flexcop_dma_free(struct flexcop_dma *dma)
{
pci_free_consistent(dma->pdev, dma->size*2,dma->cpu_addr0, dma->dma_addr0);
memset(dma,0,sizeof(struct flexcop_dma));
}
-EXPORT_SYMBOL(flexcop_dma_free);
int flexcop_dma_config(struct flexcop_device *fc,
struct flexcop_dma *dma,
@@ -78,7 +76,6 @@ int flexcop_dma_config(struct flexcop_de
return 0;
}
-EXPORT_SYMBOL(flexcop_dma_config);
/* start the DMA transfers, but not the DMA IRQs */
int flexcop_dma_xfer_control(struct flexcop_device *fc,
@@ -119,7 +116,6 @@ int flexcop_dma_xfer_control(struct flex
deb_reg("reg: %03x: %x\n",r0xc,v0xc.raw);
return 0;
}
-EXPORT_SYMBOL(flexcop_dma_xfer_control);
static int flexcop_dma_remap(struct flexcop_device *fc,
flexcop_dma_index_t dma_idx,
@@ -148,7 +144,6 @@ int flexcop_dma_control_size_irq(struct
fc->write_ibi_reg(fc,ctrl_208,v);
return 0;
}
-EXPORT_SYMBOL(flexcop_dma_control_size_irq);
int flexcop_dma_control_timer_irq(struct flexcop_device *fc,
flexcop_dma_index_t no,
@@ -165,7 +160,6 @@ int flexcop_dma_control_timer_irq(struct
fc->write_ibi_reg(fc,ctrl_208,v);
return 0;
}
-EXPORT_SYMBOL(flexcop_dma_control_timer_irq);
/* 1 cycles = 1.97 msec */
int flexcop_dma_config_timer(struct flexcop_device *fc,
@@ -182,7 +176,6 @@ int flexcop_dma_config_timer(struct flex
fc->write_ibi_reg(fc,r,v);
return 0;
}
-EXPORT_SYMBOL(flexcop_dma_config_timer);
#if 0
@@ -205,7 +198,6 @@ int flexcop_dma_control_packet_irq(struc
return 0;
}
-EXPORT_SYMBOL(flexcop_dma_control_packet_irq);
int flexcop_dma_config_packet_count(struct flexcop_device *fc,
flexcop_dma_index_t dma_idx,
@@ -220,6 +212,5 @@ int flexcop_dma_config_packet_count(stru
fc->write_ibi_reg(fc,r,v);
return 0;
}
-EXPORT_SYMBOL(flexcop_dma_config_packet_count);
#endif /* 0 */
----------------------------------------------------------------------------
flexcop: clean up debug printing functions
From: Trent Piepho <[email protected]>
Common debug macro definitions from flexcop-pci.c and flexcop-dma.c are moved
into a new header named flexcop-pci.h. This header has the flexcop-pci
version of the code that is in flexcop.h.
The debug macro in both flexcop-pci.h and flexcop.h is changed so that it adds
a message level (KERN_DEBUG) to the messages as well as the driver prefix. In
effect this:
printk("i2c success\n")
is changed to this:
printk(KERN_DEBUG "b2c2-flexcop: " "i2c success\n")
The flexcop.h file was checking "__FLEXCOP_H__" but defining "__FLEXCOP_H___",
with an extra underscore.
Signed-off-by: Trent Piepho <[email protected]>
diff --git a/linux/drivers/media/dvb/b2c2/flexcop-dma.c b/linux/drivers/media/dvb/b2c2/flexcop-dma.c
--- a/linux/drivers/media/dvb/b2c2/flexcop-dma.c
+++ b/linux/drivers/media/dvb/b2c2/flexcop-dma.c
@@ -5,22 +5,7 @@
*
* see flexcop.c for copyright information.
*/
-#define FC_LOG_PREFIX "flexcop-pci"
-#include "flexcop-common.h"
-
-#ifdef CONFIG_DVB_B2C2_FLEXCOP_DEBUG
-#define dprintk(level,args...) \
- do { if ((b2c2_flexcop_pci_debug & level)) printk(args); } while (0)
-#define DEBSTATUS ""
-#else
-#define dprintk(level,args...)
-#define DEBSTATUS " (debugging is not enabled)"
-#endif
-
-#define deb_info(args...) dprintk(0x01,args)
-#define deb_reg(args...) dprintk(0x02,args)
-
-extern int b2c2_flexcop_pci_debug;
+#include "flexcop-pci.h"
int flexcop_dma_allocate(struct pci_dev *pdev, struct flexcop_dma *dma, u32 size)
{
diff --git a/linux/drivers/media/dvb/b2c2/flexcop-pci.c b/linux/drivers/media/dvb/b2c2/flexcop-pci.c
--- a/linux/drivers/media/dvb/b2c2/flexcop-pci.c
+++ b/linux/drivers/media/dvb/b2c2/flexcop-pci.c
@@ -6,8 +6,7 @@
* see flexcop.c for copyright information.
*/
-#define FC_LOG_PREFIX "flexcop-pci"
-#include "flexcop-common.h"
+#include "flexcop-pci.h"
static int enable_pid_filtering = 1;
module_param(enable_pid_filtering, int, 0444);
@@ -16,21 +15,6 @@ static int irq_chk_intv;
static int irq_chk_intv;
module_param(irq_chk_intv, int, 0644);
MODULE_PARM_DESC(irq_chk_intv, "set the interval for IRQ watchdog (currently just debugging).");
-
-#ifdef CONFIG_DVB_B2C2_FLEXCOP_DEBUG
-#define dprintk(level,args...) \
- do { if ((b2c2_flexcop_pci_debug & level)) printk(args); } while (0)
-#define DEBSTATUS ""
-#else
-#define dprintk(level,args...)
-#define DEBSTATUS " (debugging is not enabled)"
-#endif
-
-#define deb_info(args...) dprintk(0x01,args)
-#define deb_reg(args...) dprintk(0x02,args)
-#define deb_ts(args...) dprintk(0x04,args)
-#define deb_irq(args...) dprintk(0x08,args)
-#define deb_chk(args...) dprintk(0x10,args)
int b2c2_flexcop_pci_debug = 0;
module_param_named(debug, b2c2_flexcop_pci_debug, int, 0644);
diff --git a/linux/drivers/media/dvb/b2c2/flexcop-pci.h b/linux/drivers/media/dvb/b2c2/flexcop-pci.h
new file mode 100644
--- /dev/null
+++ b/linux/drivers/media/dvb/b2c2/flexcop-pci.h
@@ -0,0 +1,33 @@
+/*
+ * This file is part of linux driver the digital TV devices equipped with B2C2 FlexcopII(b)/III
+ *
+ * flexcop-pci.h - private header file for all flexcop-pci source files.
+ *
+ * see flexcop.c for copyright information.
+ */
+#ifndef __FLEXCOP_PCI_H__
+#define __FLEXCOP_PCI_H__
+
+#define FC_LOG_PREFIX "flexcop-pci"
+#include "flexcop-common.h"
+
+extern int b2c2_flexcop_pci_debug;
+
+#ifdef CONFIG_DVB_B2C2_FLEXCOP_DEBUG
+#define dprintk(level,fmt,args...) \
+ do { if ((b2c2_flexcop_pci_debug & level)) \
+ printk(KERN_DEBUG FC_LOG_PREFIX ": " fmt, ## args); \
+ } while (0)
+#define DEBSTATUS ""
+#else
+#define dprintk(level,fmt,args...)
+#define DEBSTATUS " (debugging is not enabled)"
+#endif
+
+#define deb_info(args...) dprintk(0x01,args)
+#define deb_reg(args...) dprintk(0x02,args)
+#define deb_ts(args...) dprintk(0x04,args)
+#define deb_irq(args...) dprintk(0x08,args)
+#define deb_chk(args...) dprintk(0x10,args)
+
+#endif
diff --git a/linux/drivers/media/dvb/b2c2/flexcop.h b/linux/drivers/media/dvb/b2c2/flexcop.h
--- a/linux/drivers/media/dvb/b2c2/flexcop.h
+++ b/linux/drivers/media/dvb/b2c2/flexcop.h
@@ -6,7 +6,7 @@
* see flexcop.c for copyright information.
*/
#ifndef __FLEXCOP_H__
-#define __FLEXCOP_H___
+#define __FLEXCOP_H__
#define FC_LOG_PREFIX "b2c2-flexcop"
#include "flexcop-common.h"
@@ -15,10 +15,12 @@ extern int b2c2_flexcop_debug;
/* debug */
#ifdef CONFIG_DVB_B2C2_FLEXCOP_DEBUG
-#define dprintk(level,args...) \
- do { if ((b2c2_flexcop_debug & level)) printk(args); } while (0)
+#define dprintk(level,fmt,args...) \
+ do { if ((b2c2_flexcop_debug & level)) \
+ printk(KERN_DEBUG FC_LOG_PREFIX ": " fmt, ## args); \
+ } while (0)
#else
-#define dprintk(level,args...)
+#define dprintk(level,fmt,args...)
#endif
#define deb_info(args...) dprintk(0x01,args)