[PATCH resend] video: omap24xxcam: Fix compilation

[PATCH resend] video: omap24xxcam: Fix compilation

[Thomas Weber]

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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Randy Dunlap]

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 ***

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Sakari Ailus]

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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Felipe Balbi]

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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Felipe Balbi]

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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Sakari Ailus]

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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Thomas Weber]

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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Felipe Balbi]

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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Sakari Ailus]

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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[David Cohen]

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/
>

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Felipe Balbi]

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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[David Cohen]

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
>

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Felipe Balbi]

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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[David Cohen]

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
>

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Felipe Balbi]

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

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[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

>
> --
> balbi
>

Re: [PATCH resend] video: omap24xxcam: Fix compilation

[Thomas Weber]

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