2011-02-07 08:50:17

by Thomas Weber

[permalink] [raw]
Subject: [PATCH resend] video: omap24xxcam: Fix compilation

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 <[email protected]>
---
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


2011-02-07 16:15:25

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

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 <[email protected]>
> ---
> 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 ***

2011-02-15 11:29:58

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

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 <[email protected]>

Thanks, Thomas!

Acked-by: Sakari Ailus <[email protected]>

> ---
> 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
[email protected]

2011-02-15 11:37:30

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

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 <[email protected]>
>
> 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

2011-02-15 11:44:50

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

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 <[email protected]>
>>
>> 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

2011-02-15 11:47:20

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

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

2011-02-15 11:51:00

by Thomas Weber

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

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 <[email protected]>
>>> 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

2011-02-15 11:51:05

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

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
[email protected]

2011-02-15 11:54:00

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

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

2011-02-15 12:19:46

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

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
[email protected]

2011-02-19 11:35:15

by David Cohen

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

Hi Sakari and Felipe,

On Tue, Feb 15, 2011 at 2:17 PM, Sakari Ailus
<[email protected]> 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
> [email protected]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

2011-02-19 15:00:33

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

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

2011-02-19 16:05:01

by David Cohen

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

On Sat, Feb 19, 2011 at 5:00 PM, Felipe Balbi <[email protected]> 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
>

2011-02-21 07:36:48

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

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

2011-02-21 12:09:11

by David Cohen

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

On Mon, Feb 21, 2011 at 9:36 AM, Felipe Balbi <[email protected]> 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
>

2011-02-21 12:21:18

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

On Mon, Feb 21, 2011 at 02:09:07PM +0200, David Cohen wrote:
> On Mon, Feb 21, 2011 at 9:36 AM, Felipe Balbi <[email protected]> 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

2011-02-24 23:36:47

by David Cohen

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

On Mon, Feb 21, 2011 at 2:21 PM, Felipe Balbi <[email protected]> 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 <[email protected]> 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
>

2011-02-25 06:59:49

by Thomas Weber

[permalink] [raw]
Subject: Re: [PATCH resend] video: omap24xxcam: Fix compilation

Hallo David,

Am 25.02.2011 00:36, schrieb David Cohen:
> On Mon, Feb 21, 2011 at 2:21 PM, Felipe Balbi <[email protected]> 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 <[email protected]> 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