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 ba...@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 ba...@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-omapm=129828637120270w=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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH resend] video: omap24xxcam: Fix compilation
On Mon, Feb 21, 2011 at 9:36 AM, Felipe Balbi ba...@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-omapm=129828637120270w=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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 ba...@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-omapm=129828637120270w=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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH resend] video: omap24xxcam: Fix compilation
Hi Sakari and Felipe, On Tue, Feb 15, 2011 at 2:17 PM, Sakari Ailus sakari.ai...@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.ai...@maxwell.research.nokia.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH resend] video: omap24xxcam: Fix compilation
On Sat, Feb 19, 2011 at 5:00 PM, Felipe Balbi ba...@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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 we...@corscience.de Thanks, Thomas! Acked-by: Sakari Ailus sakari.ai...@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.ai...@maxwell.research.nokia.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 we...@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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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 we...@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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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 sakari.ai...@maxwell.research.nokia.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 sakari.ai...@maxwell.research.nokia.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[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 we...@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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 we...@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 *** -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
