LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH resend] video: omap24xxcam: Fix compilation
@ 2011-02-07  8:49 Thomas Weber
  2011-02-07 16:15 ` Randy Dunlap
  2011-02-15 11:28 ` Sakari Ailus
  0 siblings, 2 replies; 18+ messages in thread
From: Thomas Weber @ 2011-02-07  8:49 UTC (permalink / raw)
  To: linux-omap
  Cc: Thomas Weber, Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo,
	Sakari Ailus, linux-media, linux-kernel

Add linux/sched.h because of missing declaration of TASK_NORMAL.

This patch fixes the following error:

drivers/media/video/omap24xxcam.c: In function
'omap24xxcam_vbq_complete':
drivers/media/video/omap24xxcam.c:415: error: 'TASK_NORMAL' undeclared
(first use in this function)
drivers/media/video/omap24xxcam.c:415: error: (Each undeclared
identifier is reported only once
drivers/media/video/omap24xxcam.c:415: error: for each function it
appears in.)

Signed-off-by: Thomas Weber <weber@corscience.de>
---
 drivers/media/video/omap24xxcam.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/omap24xxcam.c b/drivers/media/video/omap24xxcam.c
index 0175527..f6626e8 100644
--- a/drivers/media/video/omap24xxcam.c
+++ b/drivers/media/video/omap24xxcam.c
@@ -36,6 +36,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/slab.h>
+#include <linux/sched.h>
 
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
-- 
1.7.4.rc3


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-07  8:49 [PATCH resend] video: omap24xxcam: Fix compilation Thomas Weber
@ 2011-02-07 16:15 ` Randy Dunlap
  2011-02-15 11:28 ` Sakari Ailus
  1 sibling, 0 replies; 18+ messages in thread
From: Randy Dunlap @ 2011-02-07 16:15 UTC (permalink / raw)
  To: Thomas Weber
  Cc: linux-omap, Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo,
	Sakari Ailus, linux-media, linux-kernel

On Mon,  7 Feb 2011 09:49:07 +0100 Thomas Weber wrote:

> Add linux/sched.h because of missing declaration of TASK_NORMAL.
> 
> This patch fixes the following error:
> 
> drivers/media/video/omap24xxcam.c: In function
> 'omap24xxcam_vbq_complete':
> drivers/media/video/omap24xxcam.c:415: error: 'TASK_NORMAL' undeclared
> (first use in this function)
> drivers/media/video/omap24xxcam.c:415: error: (Each undeclared
> identifier is reported only once
> drivers/media/video/omap24xxcam.c:415: error: for each function it
> appears in.)
> 
> Signed-off-by: Thomas Weber <weber@corscience.de>
> ---
>  drivers/media/video/omap24xxcam.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

Hi,

Please use media: or multimedia: or media/video: in the subject line,
not just video:.  video: traditionally is used for drivers/video/,
not drivers/media/video.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-07  8:49 [PATCH resend] video: omap24xxcam: Fix compilation Thomas Weber
  2011-02-07 16:15 ` Randy Dunlap
@ 2011-02-15 11:28 ` Sakari Ailus
  2011-02-15 11:37   ` Felipe Balbi
  1 sibling, 1 reply; 18+ messages in thread
From: Sakari Ailus @ 2011-02-15 11:28 UTC (permalink / raw)
  To: Thomas Weber
  Cc: linux-omap, Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo,
	linux-media, linux-kernel

Thomas Weber wrote:
> Add linux/sched.h because of missing declaration of TASK_NORMAL.
> 
> This patch fixes the following error:
> 
> drivers/media/video/omap24xxcam.c: In function
> 'omap24xxcam_vbq_complete':
> drivers/media/video/omap24xxcam.c:415: error: 'TASK_NORMAL' undeclared
> (first use in this function)
> drivers/media/video/omap24xxcam.c:415: error: (Each undeclared
> identifier is reported only once
> drivers/media/video/omap24xxcam.c:415: error: for each function it
> appears in.)
> 
> Signed-off-by: Thomas Weber <weber@corscience.de>

Thanks, Thomas!

Acked-by: Sakari Ailus <sakari.ailus@iki.fi>

> ---
>  drivers/media/video/omap24xxcam.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/omap24xxcam.c b/drivers/media/video/omap24xxcam.c
> index 0175527..f6626e8 100644
> --- a/drivers/media/video/omap24xxcam.c
> +++ b/drivers/media/video/omap24xxcam.c
> @@ -36,6 +36,7 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/slab.h>
> +#include <linux/sched.h>
>  
>  #include <media/v4l2-common.h>
>  #include <media/v4l2-ioctl.h>


-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-15 11:28 ` Sakari Ailus
@ 2011-02-15 11:37   ` Felipe Balbi
  2011-02-15 11:44     ` Sylwester Nawrocki
  0 siblings, 1 reply; 18+ messages in thread
From: Felipe Balbi @ 2011-02-15 11:37 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Thomas Weber, linux-omap, Mauro Carvalho Chehab, Hans Verkuil,
	Tejun Heo, linux-media, linux-kernel

On Tue, Feb 15, 2011 at 01:28:19PM +0200, Sakari Ailus wrote:
> Thomas Weber wrote:
> > Add linux/sched.h because of missing declaration of TASK_NORMAL.
> > 
> > This patch fixes the following error:
> > 
> > drivers/media/video/omap24xxcam.c: In function
> > 'omap24xxcam_vbq_complete':
> > drivers/media/video/omap24xxcam.c:415: error: 'TASK_NORMAL' undeclared
> > (first use in this function)
> > drivers/media/video/omap24xxcam.c:415: error: (Each undeclared
> > identifier is reported only once
> > drivers/media/video/omap24xxcam.c:415: error: for each function it
> > appears in.)
> > 
> > Signed-off-by: Thomas Weber <weber@corscience.de>
> 
> Thanks, Thomas!

Are we using the same tree ? I don't see anything related to TASK_* on
that function on today's mainline, here's a copy of the function:

 387 static void omap24xxcam_vbq_complete(struct omap24xxcam_sgdma *sgdma,
 388                                      u32 csr, void *arg)
 389 {
 390         struct omap24xxcam_device *cam =
 391                 container_of(sgdma, struct omap24xxcam_device, sgdma);
 392         struct omap24xxcam_fh *fh = cam->streaming->private_data;
 393         struct videobuf_buffer *vb = (struct videobuf_buffer *)arg;
 394         const u32 csr_error = CAMDMA_CSR_MISALIGNED_ERR
 395                 | CAMDMA_CSR_SUPERVISOR_ERR | CAMDMA_CSR_SECURE_ERR
 396                 | CAMDMA_CSR_TRANS_ERR | CAMDMA_CSR_DROP;
 397         unsigned long flags;
 398 
 399         spin_lock_irqsave(&cam->core_enable_disable_lock, flags);
 400         if (--cam->sgdma_in_queue == 0)
 401                 omap24xxcam_core_disable(cam);
 402         spin_unlock_irqrestore(&cam->core_enable_disable_lock, flags);
 403 
 404         do_gettimeofday(&vb->ts);
 405         vb->field_count = atomic_add_return(2, &fh->field_count);
 406         if (csr & csr_error) {
 407                 vb->state = VIDEOBUF_ERROR;
 408                 if (!atomic_read(&fh->cam->in_reset)) {
 409                         dev_dbg(cam->dev, "resetting camera, csr 0x%x\n", csr);
 410                         omap24xxcam_reset(cam);
 411                 }
 412         } else
 413                 vb->state = VIDEOBUF_DONE;
 414         wake_up(&vb->done);
 415 }

see that line 415 is where the function ends. My head is
795abaf1e4e188c4171e3cd3dbb11a9fcacaf505

-- 
balbi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-15 11:37   ` Felipe Balbi
@ 2011-02-15 11:44     ` Sylwester Nawrocki
  2011-02-15 11:47       ` Felipe Balbi
  2011-02-15 11:50       ` Thomas Weber
  0 siblings, 2 replies; 18+ messages in thread
From: Sylwester Nawrocki @ 2011-02-15 11:44 UTC (permalink / raw)
  To: balbi
  Cc: Sakari Ailus, Thomas Weber, linux-omap, Mauro Carvalho Chehab,
	Hans Verkuil, Tejun Heo, linux-media, linux-kernel

Hi Felipe,

On 02/15/2011 12:37 PM, Felipe Balbi wrote:
> On Tue, Feb 15, 2011 at 01:28:19PM +0200, Sakari Ailus wrote:
>> Thomas Weber wrote:
>>> Add linux/sched.h because of missing declaration of TASK_NORMAL.
>>>
>>> This patch fixes the following error:
>>>
>>> drivers/media/video/omap24xxcam.c: In function
>>> 'omap24xxcam_vbq_complete':
>>> drivers/media/video/omap24xxcam.c:415: error: 'TASK_NORMAL' undeclared
>>> (first use in this function)
>>> drivers/media/video/omap24xxcam.c:415: error: (Each undeclared
>>> identifier is reported only once
>>> drivers/media/video/omap24xxcam.c:415: error: for each function it
>>> appears in.)
>>>
>>> Signed-off-by: Thomas Weber <weber@corscience.de>
>>
>> Thanks, Thomas!
> 
> Are we using the same tree ? I don't see anything related to TASK_* on

Please have a look at definition of macro wake_up. This where those
TASK_* flags are used.

> that function on today's mainline, here's a copy of the function:
> 
>  387 static void omap24xxcam_vbq_complete(struct omap24xxcam_sgdma *sgdma,
>  388                                      u32 csr, void *arg)
>  389 {
>  390         struct omap24xxcam_device *cam =
>  391                 container_of(sgdma, struct omap24xxcam_device, sgdma);
>  392         struct omap24xxcam_fh *fh = cam->streaming->private_data;
>  393         struct videobuf_buffer *vb = (struct videobuf_buffer *)arg;
>  394         const u32 csr_error = CAMDMA_CSR_MISALIGNED_ERR
>  395                 | CAMDMA_CSR_SUPERVISOR_ERR | CAMDMA_CSR_SECURE_ERR
>  396                 | CAMDMA_CSR_TRANS_ERR | CAMDMA_CSR_DROP;
>  397         unsigned long flags;
>  398 
>  399         spin_lock_irqsave(&cam->core_enable_disable_lock, flags);
>  400         if (--cam->sgdma_in_queue == 0)
>  401                 omap24xxcam_core_disable(cam);
>  402         spin_unlock_irqrestore(&cam->core_enable_disable_lock, flags);
>  403 
>  404         do_gettimeofday(&vb->ts);
>  405         vb->field_count = atomic_add_return(2, &fh->field_count);
>  406         if (csr & csr_error) {
>  407                 vb->state = VIDEOBUF_ERROR;
>  408                 if (!atomic_read(&fh->cam->in_reset)) {
>  409                         dev_dbg(cam->dev, "resetting camera, csr 0x%x\n", csr);
>  410                         omap24xxcam_reset(cam);
>  411                 }
>  412         } else
>  413                 vb->state = VIDEOBUF_DONE;
>  414         wake_up(&vb->done);
>  415 }
> 
> see that line 415 is where the function ends. My head is
> 795abaf1e4e188c4171e3cd3dbb11a9fcacaf505
> 

Cheers,
Sylwester Nawrocki

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-15 11:44     ` Sylwester Nawrocki
@ 2011-02-15 11:47       ` Felipe Balbi
  2011-02-15 11:49         ` Sakari Ailus
  2011-02-15 11:50       ` Thomas Weber
  1 sibling, 1 reply; 18+ messages in thread
From: Felipe Balbi @ 2011-02-15 11:47 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: balbi, Sakari Ailus, Thomas Weber, linux-omap,
	Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo, linux-media,
	linux-kernel

hi,

On Tue, Feb 15, 2011 at 12:44:42PM +0100, Sylwester Nawrocki wrote:
> > Are we using the same tree ? I don't see anything related to TASK_* on
> 
> Please have a look at definition of macro wake_up. This where those
> TASK_* flags are used.

$ git grep -e TASK_ drivers/media/video/omap*.[ch]
$ 

???

-- 
balbi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-15 11:47       ` Felipe Balbi
@ 2011-02-15 11:49         ` Sakari Ailus
  0 siblings, 0 replies; 18+ messages in thread
From: Sakari Ailus @ 2011-02-15 11:49 UTC (permalink / raw)
  To: balbi
  Cc: Sylwester Nawrocki, Thomas Weber, linux-omap,
	Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo, linux-media,
	linux-kernel

Felipe Balbi wrote:
> hi,
> 
> On Tue, Feb 15, 2011 at 12:44:42PM +0100, Sylwester Nawrocki wrote:
>>> Are we using the same tree ? I don't see anything related to TASK_* on
>>
>> Please have a look at definition of macro wake_up. This where those
>> TASK_* flags are used.
> 
> $ git grep -e TASK_ drivers/media/video/omap*.[ch]
> $ 
> 
> ???
> 

It's wake_up() on line 414.

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-15 11:44     ` Sylwester Nawrocki
  2011-02-15 11:47       ` Felipe Balbi
@ 2011-02-15 11:50       ` Thomas Weber
  2011-02-15 11:53         ` Felipe Balbi
  1 sibling, 1 reply; 18+ messages in thread
From: Thomas Weber @ 2011-02-15 11:50 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: balbi, Sakari Ailus, linux-omap, Mauro Carvalho Chehab,
	Hans Verkuil, Tejun Heo, linux-media, linux-kernel

Am 15.02.2011 12:44, schrieb Sylwester Nawrocki:
> Hi Felipe,
>
> On 02/15/2011 12:37 PM, Felipe Balbi wrote:
>> On Tue, Feb 15, 2011 at 01:28:19PM +0200, Sakari Ailus wrote:
>>> Thomas Weber wrote:
>>>> Add linux/sched.h because of missing declaration of TASK_NORMAL.
>>>>
>>>> This patch fixes the following error:
>>>>
>>>> drivers/media/video/omap24xxcam.c: In function
>>>> 'omap24xxcam_vbq_complete':
>>>> drivers/media/video/omap24xxcam.c:415: error: 'TASK_NORMAL' undeclared
>>>> (first use in this function)
>>>> drivers/media/video/omap24xxcam.c:415: error: (Each undeclared
>>>> identifier is reported only once
>>>> drivers/media/video/omap24xxcam.c:415: error: for each function it
>>>> appears in.)
>>>>
>>>> Signed-off-by: Thomas Weber <weber@corscience.de>
>>> Thanks, Thomas!
>> Are we using the same tree ? I don't see anything related to TASK_* on
> Please have a look at definition of macro wake_up. This where those
> TASK_* flags are used.
>
>> that function on today's mainline, here's a copy of the function:
>>
>>  387 static void omap24xxcam_vbq_complete(struct omap24xxcam_sgdma *sgdma,
>>  388                                      u32 csr, void *arg)
>>  389 {
>>  390         struct omap24xxcam_device *cam =
>>  391                 container_of(sgdma, struct omap24xxcam_device, sgdma);
>>  392         struct omap24xxcam_fh *fh = cam->streaming->private_data;
>>  393         struct videobuf_buffer *vb = (struct videobuf_buffer *)arg;
>>  394         const u32 csr_error = CAMDMA_CSR_MISALIGNED_ERR
>>  395                 | CAMDMA_CSR_SUPERVISOR_ERR | CAMDMA_CSR_SECURE_ERR
>>  396                 | CAMDMA_CSR_TRANS_ERR | CAMDMA_CSR_DROP;
>>  397         unsigned long flags;
>>  398 
>>  399         spin_lock_irqsave(&cam->core_enable_disable_lock, flags);
>>  400         if (--cam->sgdma_in_queue == 0)
>>  401                 omap24xxcam_core_disable(cam);
>>  402         spin_unlock_irqrestore(&cam->core_enable_disable_lock, flags);
>>  403 
>>  404         do_gettimeofday(&vb->ts);
>>  405         vb->field_count = atomic_add_return(2, &fh->field_count);
>>  406         if (csr & csr_error) {
>>  407                 vb->state = VIDEOBUF_ERROR;
>>  408                 if (!atomic_read(&fh->cam->in_reset)) {
>>  409                         dev_dbg(cam->dev, "resetting camera, csr 0x%x\n", csr);
>>  410                         omap24xxcam_reset(cam);
>>  411                 }
>>  412         } else
>>  413                 vb->state = VIDEOBUF_DONE;
>>  414         wake_up(&vb->done);
>>  415 }
>>
>> see that line 415 is where the function ends. My head is
>> 795abaf1e4e188c4171e3cd3dbb11a9fcacaf505
>>
> Cheers,
> Sylwester Nawrocki
> --

Hello Felipe,

in include/linux/wait.h

#define wake_up(x)            __wake_up(x, TASK_NORMAL, 1, NULL)

Regards,
Thomas

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-15 11:50       ` Thomas Weber
@ 2011-02-15 11:53         ` Felipe Balbi
  2011-02-15 12:17           ` Sakari Ailus
  0 siblings, 1 reply; 18+ messages in thread
From: Felipe Balbi @ 2011-02-15 11:53 UTC (permalink / raw)
  To: Thomas Weber
  Cc: Sylwester Nawrocki, balbi, Sakari Ailus, linux-omap,
	Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo, linux-media,
	linux-kernel

Hi,

On Tue, Feb 15, 2011 at 12:50:12PM +0100, Thomas Weber wrote:
> Hello Felipe,
> 
> in include/linux/wait.h
> 
> #define wake_up(x)            __wake_up(x, TASK_NORMAL, 1, NULL)

aha, now I get it, so shouldn't the real fix be including <linux/sched.h>
on <linux/wait.h>, I mean, it's <linuux/wait.h> who uses a symbol
defined in <linux/sched.h>, right ?

-- 
balbi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-15 11:53         ` Felipe Balbi
@ 2011-02-15 12:17           ` Sakari Ailus
  2011-02-19 11:35             ` David Cohen
  0 siblings, 1 reply; 18+ messages in thread
From: Sakari Ailus @ 2011-02-15 12:17 UTC (permalink / raw)
  To: balbi
  Cc: Thomas Weber, Sylwester Nawrocki, linux-omap,
	Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo, linux-media,
	linux-kernel

Felipe Balbi wrote:
> Hi,
> 
> On Tue, Feb 15, 2011 at 12:50:12PM +0100, Thomas Weber wrote:
>> Hello Felipe,
>>
>> in include/linux/wait.h
>>
>> #define wake_up(x)            __wake_up(x, TASK_NORMAL, 1, NULL)
> 
> aha, now I get it, so shouldn't the real fix be including <linux/sched.h>
> on <linux/wait.h>, I mean, it's <linuux/wait.h> who uses a symbol
> defined in <linux/sched.h>, right ?

Surprisingly many other files still don't seem to be affected. But this
is actually a better solution (to include sched.h in wait.h).

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-15 12:17           ` Sakari Ailus
@ 2011-02-19 11:35             ` David Cohen
  2011-02-19 15:00               ` Felipe Balbi
  0 siblings, 1 reply; 18+ messages in thread
From: David Cohen @ 2011-02-19 11:35 UTC (permalink / raw)
  To: Sakari Ailus, balbi
  Cc: Thomas Weber, Sylwester Nawrocki, linux-omap,
	Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo, linux-media,
	linux-kernel

Hi Sakari and Felipe,

On Tue, Feb 15, 2011 at 2:17 PM, Sakari Ailus
<sakari.ailus@maxwell.research.nokia.com> wrote:
> Felipe Balbi wrote:
>> Hi,
>>
>> On Tue, Feb 15, 2011 at 12:50:12PM +0100, Thomas Weber wrote:
>>> Hello Felipe,
>>>
>>> in include/linux/wait.h
>>>
>>> #define wake_up(x)            __wake_up(x, TASK_NORMAL, 1, NULL)
>>
>> aha, now I get it, so shouldn't the real fix be including <linux/sched.h>
>> on <linux/wait.h>, I mean, it's <linuux/wait.h> who uses a symbol
>> defined in <linux/sched.h>, right ?

That's a tricky situation. linux/sched.h includes indirectly
linux/completion.h which includes linux/wait.h.
By including sched.h in wait.h, the side effect is completion.h will
then include a blank wait.h file and trigger a compilation error every
time wait.h is included by any file.

>
> Surprisingly many other files still don't seem to be affected. But this
> is actually a better solution (to include sched.h in wait.h).

It does not affect all files include wait.h because TASK_* macros are
used with #define statements only. So it has no effect unless some
file tries to use a macro which used TASK_*. It seems the usual on
kernel is to include both wait.h and sched.h when necessary.
IMO your patch is fine.

Br,

David

>
> --
> Sakari Ailus
> sakari.ailus@maxwell.research.nokia.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-19 11:35             ` David Cohen
@ 2011-02-19 15:00               ` Felipe Balbi
  2011-02-19 16:04                 ` David Cohen
  0 siblings, 1 reply; 18+ messages in thread
From: Felipe Balbi @ 2011-02-19 15:00 UTC (permalink / raw)
  To: David Cohen
  Cc: Sakari Ailus, balbi, Thomas Weber, Sylwester Nawrocki,
	linux-omap, Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo,
	linux-media, linux-kernel

Hi,

On Sat, Feb 19, 2011 at 01:35:09PM +0200, David Cohen wrote:
> >> aha, now I get it, so shouldn't the real fix be including <linux/sched.h>
> >> on <linux/wait.h>, I mean, it's <linuux/wait.h> who uses a symbol
> >> defined in <linux/sched.h>, right ?
> 
> That's a tricky situation. linux/sched.h includes indirectly
> linux/completion.h which includes linux/wait.h.

Ok, so the real problem is that there is circular dependency between
<linux/sched.h> and <linux/wait.h>

> By including sched.h in wait.h, the side effect is completion.h will
> then include a blank wait.h file and trigger a compilation error every
> time wait.h is included by any file.

true, but the real problem is the circular dependency between those
files.

> > Surprisingly many other files still don't seem to be affected. But this
> > is actually a better solution (to include sched.h in wait.h).
> 
> It does not affect all files include wait.h because TASK_* macros are
> used with #define statements only. So it has no effect unless some
> file tries to use a macro which used TASK_*. It seems the usual on
> kernel is to include both wait.h and sched.h when necessary.
> IMO your patch is fine.

I have to disagree. The fundamental problem is the circular dependency
between those two files:

sched.h uses wait_queue_head_t defined in wait.h
wait.h uses TASK_* defined in sched.h

So, IMO the real fix would be clear out the circular dependency. Maybe
introducing <linux/task.h> to define those TASK_* symbols and include
that on sched.h and wait.h

Just dig a quick and dirty to try it out and works like a charm

-- 
balbi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-19 15:00               ` Felipe Balbi
@ 2011-02-19 16:04                 ` David Cohen
  2011-02-21  7:36                   ` Felipe Balbi
  0 siblings, 1 reply; 18+ messages in thread
From: David Cohen @ 2011-02-19 16:04 UTC (permalink / raw)
  To: balbi
  Cc: Sakari Ailus, Thomas Weber, Sylwester Nawrocki, linux-omap,
	Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo, linux-media,
	linux-kernel

On Sat, Feb 19, 2011 at 5:00 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Sat, Feb 19, 2011 at 01:35:09PM +0200, David Cohen wrote:
>> >> aha, now I get it, so shouldn't the real fix be including <linux/sched.h>
>> >> on <linux/wait.h>, I mean, it's <linuux/wait.h> who uses a symbol
>> >> defined in <linux/sched.h>, right ?
>>
>> That's a tricky situation. linux/sched.h includes indirectly
>> linux/completion.h which includes linux/wait.h.
>
> Ok, so the real problem is that there is circular dependency between
> <linux/sched.h> and <linux/wait.h>
>
>> By including sched.h in wait.h, the side effect is completion.h will
>> then include a blank wait.h file and trigger a compilation error every
>> time wait.h is included by any file.
>
> true, but the real problem is the circular dependency between those
> files.
>
>> > Surprisingly many other files still don't seem to be affected. But this
>> > is actually a better solution (to include sched.h in wait.h).
>>
>> It does not affect all files include wait.h because TASK_* macros are
>> used with #define statements only. So it has no effect unless some
>> file tries to use a macro which used TASK_*. It seems the usual on
>> kernel is to include both wait.h and sched.h when necessary.
>> IMO your patch is fine.
>
> I have to disagree. The fundamental problem is the circular dependency
> between those two files:
>
> sched.h uses wait_queue_head_t defined in wait.h
> wait.h uses TASK_* defined in sched.h
>
> So, IMO the real fix would be clear out the circular dependency. Maybe
> introducing <linux/task.h> to define those TASK_* symbols and include
> that on sched.h and wait.h
>
> Just dig a quick and dirty to try it out and works like a charm

We have 2 problems:
 - omap24xxcam compilation broken
 - circular dependency between sched.h and wait.h

To fix the broken compilation we can do what the rest of the kernel is
doing, which is to include sched.h.
Then, the circular dependency is fixed by some different approach
which would probably change *all* current usage of TASK_*.

IMO, there's no need to create a dependency between those issues.

Br,

David

>
> --
> balbi
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-19 16:04                 ` David Cohen
@ 2011-02-21  7:36                   ` Felipe Balbi
  2011-02-21 12:09                     ` David Cohen
  0 siblings, 1 reply; 18+ messages in thread
From: Felipe Balbi @ 2011-02-21  7:36 UTC (permalink / raw)
  To: David Cohen
  Cc: balbi, Sakari Ailus, Thomas Weber, Sylwester Nawrocki,
	linux-omap, Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo,
	linux-media, linux-kernel

Hi,

On Sat, Feb 19, 2011 at 06:04:58PM +0200, David Cohen wrote:
> > I have to disagree. The fundamental problem is the circular dependency
> > between those two files:
> >
> > sched.h uses wait_queue_head_t defined in wait.h
> > wait.h uses TASK_* defined in sched.h
> >
> > So, IMO the real fix would be clear out the circular dependency. Maybe
> > introducing <linux/task.h> to define those TASK_* symbols and include
> > that on sched.h and wait.h
> >
> > Just dig a quick and dirty to try it out and works like a charm
> 
> We have 2 problems:
>  - omap24xxcam compilation broken
>  - circular dependency between sched.h and wait.h
> 
> To fix the broken compilation we can do what the rest of the kernel is
> doing, which is to include sched.h.
> Then, the circular dependency is fixed by some different approach
> which would probably change *all* current usage of TASK_*.

considering that 1 is caused by 2 I would fix 2.

> IMO, there's no need to create a dependency between those issues.

There's no dependency between them, it's just that the root cause for
this problem is a circular dependency between wait.h and sched.h

-- 
balbi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-21  7:36                   ` Felipe Balbi
@ 2011-02-21 12:09                     ` David Cohen
  2011-02-21 12:21                       ` Felipe Balbi
  0 siblings, 1 reply; 18+ messages in thread
From: David Cohen @ 2011-02-21 12:09 UTC (permalink / raw)
  To: balbi
  Cc: Sakari Ailus, Thomas Weber, Sylwester Nawrocki, linux-omap,
	Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo, linux-media,
	linux-kernel

On Mon, Feb 21, 2011 at 9:36 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Sat, Feb 19, 2011 at 06:04:58PM +0200, David Cohen wrote:
>> > I have to disagree. The fundamental problem is the circular dependency
>> > between those two files:
>> >
>> > sched.h uses wait_queue_head_t defined in wait.h
>> > wait.h uses TASK_* defined in sched.h
>> >
>> > So, IMO the real fix would be clear out the circular dependency. Maybe
>> > introducing <linux/task.h> to define those TASK_* symbols and include
>> > that on sched.h and wait.h
>> >
>> > Just dig a quick and dirty to try it out and works like a charm
>>
>> We have 2 problems:
>>  - omap24xxcam compilation broken
>>  - circular dependency between sched.h and wait.h
>>
>> To fix the broken compilation we can do what the rest of the kernel is
>> doing, which is to include sched.h.
>> Then, the circular dependency is fixed by some different approach
>> which would probably change *all* current usage of TASK_*.
>
> considering that 1 is caused by 2 I would fix 2.
>
>> IMO, there's no need to create a dependency between those issues.
>
> There's no dependency between them, it's just that the root cause for
> this problem is a circular dependency between wait.h and sched.h

I did a try to fix this circular dependency and the comment I got was
to include sched.h in omap24xxcam.c file:
http://marc.info/?l=linux-omap&m=129828637120270&w=2

I'm working to remove v4l2 internal device interface from omap24xxcam
and then I need this driver's compilation fixed.
The whole kernel is including sched.h when wake_up*() macro is used,
so this should be our first solution IMO.
As I said earlier, no need to make this compilation fix be dependent
of wait.h fix (if it's really going to be changed).

I think we should proceed with this patch.

Br,

David

>
> --
> balbi
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-21 12:09                     ` David Cohen
@ 2011-02-21 12:21                       ` Felipe Balbi
  2011-02-24 23:36                         ` David Cohen
  0 siblings, 1 reply; 18+ messages in thread
From: Felipe Balbi @ 2011-02-21 12:21 UTC (permalink / raw)
  To: David Cohen
  Cc: balbi, Sakari Ailus, Thomas Weber, Sylwester Nawrocki,
	linux-omap, Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo,
	linux-media, linux-kernel

On Mon, Feb 21, 2011 at 02:09:07PM +0200, David Cohen wrote:
> On Mon, Feb 21, 2011 at 9:36 AM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Sat, Feb 19, 2011 at 06:04:58PM +0200, David Cohen wrote:
> >> > I have to disagree. The fundamental problem is the circular dependency
> >> > between those two files:
> >> >
> >> > sched.h uses wait_queue_head_t defined in wait.h
> >> > wait.h uses TASK_* defined in sched.h
> >> >
> >> > So, IMO the real fix would be clear out the circular dependency. Maybe
> >> > introducing <linux/task.h> to define those TASK_* symbols and include
> >> > that on sched.h and wait.h
> >> >
> >> > Just dig a quick and dirty to try it out and works like a charm
> >>
> >> We have 2 problems:
> >>  - omap24xxcam compilation broken
> >>  - circular dependency between sched.h and wait.h
> >>
> >> To fix the broken compilation we can do what the rest of the kernel is
> >> doing, which is to include sched.h.
> >> Then, the circular dependency is fixed by some different approach
> >> which would probably change *all* current usage of TASK_*.
> >
> > considering that 1 is caused by 2 I would fix 2.
> >
> >> IMO, there's no need to create a dependency between those issues.
> >
> > There's no dependency between them, it's just that the root cause for
> > this problem is a circular dependency between wait.h and sched.h
> 
> I did a try to fix this circular dependency and the comment I got was
> to include sched.h in omap24xxcam.c file:
> http://marc.info/?l=linux-omap&m=129828637120270&w=2
> 
> I'm working to remove v4l2 internal device interface from omap24xxcam
> and then I need this driver's compilation fixed.
> The whole kernel is including sched.h when wake_up*() macro is used,
> so this should be our first solution IMO.
> As I said earlier, no need to make this compilation fix be dependent
> of wait.h fix (if it's really going to be changed).
> 
> I think we should proceed with this patch.

I would wait to hear from Ingo or Peter who are the maintainers for that
part, but fine by me.

-- 
balbi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-21 12:21                       ` Felipe Balbi
@ 2011-02-24 23:36                         ` David Cohen
  2011-02-25  6:59                           ` Thomas Weber
  0 siblings, 1 reply; 18+ messages in thread
From: David Cohen @ 2011-02-24 23:36 UTC (permalink / raw)
  To: balbi
  Cc: Sakari Ailus, Thomas Weber, Sylwester Nawrocki, linux-omap,
	Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo, linux-media,
	linux-kernel

On Mon, Feb 21, 2011 at 2:21 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Mon, Feb 21, 2011 at 02:09:07PM +0200, David Cohen wrote:
>> On Mon, Feb 21, 2011 at 9:36 AM, Felipe Balbi <balbi@ti.com> wrote:
>> > Hi,
>> >
>> > On Sat, Feb 19, 2011 at 06:04:58PM +0200, David Cohen wrote:
>> >> > I have to disagree. The fundamental problem is the circular dependency
>> >> > between those two files:
>> >> >
>> >> > sched.h uses wait_queue_head_t defined in wait.h
>> >> > wait.h uses TASK_* defined in sched.h
>> >> >
>> >> > So, IMO the real fix would be clear out the circular dependency. Maybe
>> >> > introducing <linux/task.h> to define those TASK_* symbols and include
>> >> > that on sched.h and wait.h
>> >> >
>> >> > Just dig a quick and dirty to try it out and works like a charm
>> >>
>> >> We have 2 problems:
>> >>  - omap24xxcam compilation broken
>> >>  - circular dependency between sched.h and wait.h
>> >>
>> >> To fix the broken compilation we can do what the rest of the kernel is
>> >> doing, which is to include sched.h.
>> >> Then, the circular dependency is fixed by some different approach
>> >> which would probably change *all* current usage of TASK_*.
>> >
>> > considering that 1 is caused by 2 I would fix 2.
>> >
>> >> IMO, there's no need to create a dependency between those issues.
>> >
>> > There's no dependency between them, it's just that the root cause for
>> > this problem is a circular dependency between wait.h and sched.h
>>
>> I did a try to fix this circular dependency and the comment I got was
>> to include sched.h in omap24xxcam.c file:
>> http://marc.info/?l=linux-omap&m=129828637120270&w=2
>>
>> I'm working to remove v4l2 internal device interface from omap24xxcam
>> and then I need this driver's compilation fixed.
>> The whole kernel is including sched.h when wake_up*() macro is used,
>> so this should be our first solution IMO.
>> As I said earlier, no need to make this compilation fix be dependent
>> of wait.h fix (if it's really going to be changed).
>>
>> I think we should proceed with this patch.
>
> I would wait to hear from Ingo or Peter who are the maintainers for that
> part, but fine by me.

How about to proceed with this patch?

Regards,

David

>
> --
> balbi
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH resend] video: omap24xxcam: Fix compilation
  2011-02-24 23:36                         ` David Cohen
@ 2011-02-25  6:59                           ` Thomas Weber
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Weber @ 2011-02-25  6:59 UTC (permalink / raw)
  To: David Cohen
  Cc: balbi, Sakari Ailus, Sylwester Nawrocki, linux-omap,
	Mauro Carvalho Chehab, Hans Verkuil, Tejun Heo, linux-media,
	linux-kernel

Hallo David,

Am 25.02.2011 00:36, schrieb David Cohen:
> On Mon, Feb 21, 2011 at 2:21 PM, Felipe Balbi <balbi@ti.com> wrote:
>> On Mon, Feb 21, 2011 at 02:09:07PM +0200, David Cohen wrote:
>>> On Mon, Feb 21, 2011 at 9:36 AM, Felipe Balbi <balbi@ti.com> wrote:
>>>> Hi,
>>>>
>>>> On Sat, Feb 19, 2011 at 06:04:58PM +0200, David Cohen wrote:
>>>>>> I have to disagree. The fundamental problem is the circular dependency
>>>>>> between those two files:
>>>>>>
>>>>>> sched.h uses wait_queue_head_t defined in wait.h
>>>>>> wait.h uses TASK_* defined in sched.h
>>>>>>
>>>>>> So, IMO the real fix would be clear out the circular dependency. Maybe
>>>>>> introducing <linux/task.h> to define those TASK_* symbols and include
>>>>>> that on sched.h and wait.h
>>>>>>
>>>>>> Just dig a quick and dirty to try it out and works like a charm
>>>>> We have 2 problems:
>>>>>  - omap24xxcam compilation broken
>>>>>  - circular dependency between sched.h and wait.h
>>>>>
>>>>> To fix the broken compilation we can do what the rest of the kernel is
>>>>> doing, which is to include sched.h.
>>>>> Then, the circular dependency is fixed by some different approach
>>>>> which would probably change *all* current usage of TASK_*.
>>>> considering that 1 is caused by 2 I would fix 2.
>>>>
>>>>> IMO, there's no need to create a dependency between those issues.
>>>> There's no dependency between them, it's just that the root cause for
>>>> this problem is a circular dependency between wait.h and sched.h
>>> I did a try to fix this circular dependency and the comment I got was
>>> to include sched.h in omap24xxcam.c file:
>>> http://marc.info/?l=linux-omap&m=129828637120270&w=2
>>>
>>> I'm working to remove v4l2 internal device interface from omap24xxcam
>>> and then I need this driver's compilation fixed.
>>> The whole kernel is including sched.h when wake_up*() macro is used,
>>> so this should be our first solution IMO.
>>> As I said earlier, no need to make this compilation fix be dependent
>>> of wait.h fix (if it's really going to be changed).
>>>
>>> I think we should proceed with this patch.
>> I would wait to hear from Ingo or Peter who are the maintainers for that
>> part, but fine by me.
> How about to proceed with this patch?
>
> Regards,
>
> David
>

I got a message that the patch is queued at

http://git.linuxtv.org/media_tree.git for_v2.6.39


Thanks Mauro.


Thomas





^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2011-02-25  6:59 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-07  8:49 [PATCH resend] video: omap24xxcam: Fix compilation Thomas Weber
2011-02-07 16:15 ` Randy Dunlap
2011-02-15 11:28 ` Sakari Ailus
2011-02-15 11:37   ` Felipe Balbi
2011-02-15 11:44     ` Sylwester Nawrocki
2011-02-15 11:47       ` Felipe Balbi
2011-02-15 11:49         ` Sakari Ailus
2011-02-15 11:50       ` Thomas Weber
2011-02-15 11:53         ` Felipe Balbi
2011-02-15 12:17           ` Sakari Ailus
2011-02-19 11:35             ` David Cohen
2011-02-19 15:00               ` Felipe Balbi
2011-02-19 16:04                 ` David Cohen
2011-02-21  7:36                   ` Felipe Balbi
2011-02-21 12:09                     ` David Cohen
2011-02-21 12:21                       ` Felipe Balbi
2011-02-24 23:36                         ` David Cohen
2011-02-25  6:59                           ` Thomas Weber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).