[PATCH] staging: tidspbridge: enable watchdog by default

[PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

From: Felipe Contreras <felipe.contreras@nokia.com>

The public images have it enabled, it's safer, and we get rid of this
warning:

WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c()
In-band Error seen by IVA_SS  at address 0
Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether
[<c0012848>] (unwind_backtrace+0x0/0xec) from [<c002e760>] (warn_slowpath_common+0x4c/0x64)
[<c002e760>] (warn_slowpath_common+0x4c/0x64) from [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c)
[<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c)
[<c00217c8>] (omap3_l3_app_irq+0x114/0x15c) from [<c0059844>] (handle_irq_event_percpu+0x28/0x174)
[<c0059844>] (handle_irq_event_percpu+0x28/0x174) from [<c00599b8>] (handle_irq_event+0x28/0x38)
[<c00599b8>] (handle_irq_event+0x28/0x38) from [<c005b8d0>] (handle_level_irq+0xb8/0xe0)
[<c005b8d0>] (handle_level_irq+0xb8/0xe0) from [<c0059598>] (generic_handle_irq+0x28/0x30)
[<c0059598>] (generic_handle_irq+0x28/0x30) from [<c000e62c>] (handle_IRQ+0x60/0x84)
[<c000e62c>] (handle_IRQ+0x60/0x84) from [<c000d334>] (__irq_svc+0x34/0x80)
[<c000d334>] (__irq_svc+0x34/0x80) from [<c0015804>] (v7_dma_inv_range+0x34/0x48)
[<c0015804>] (v7_dma_inv_range+0x34/0x48) from [<c00129ec>] (dma_cache_maint_page+0x2c/0x34)
[<c00129ec>] (dma_cache_maint_page+0x2c/0x34) from [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68)
[<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68) from [<c0012b50>] (dma_unmap_sg+0x40/0x64)
[<c0012b50>] (dma_unmap_sg+0x40/0x64) from [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48)
[<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48) from [<c01932f4>] (mmc_post_req+0x18/0x1c)
[<c01932f4>] (mmc_post_req+0x18/0x1c) from [<c019535c>] (mmc_start_req+0xd4/0xec)
[<c019535c>] (mmc_start_req+0xd4/0xec) from [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244)
[<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244) from [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc)
[<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc) from [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8)
[<c019e1c0>] (mmc_queue_thread+0x8c/0xf8) from [<c0044c10>] (kthread+0x84/0x8c)
[<c0044c10>] (kthread+0x84/0x8c) from [<c000e694>] (kernel_thread_exit+0x0/0x8)
---[ end trace 5dec1c8d7857375d ]---

Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com>
---
 drivers/staging/tidspbridge/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
index 21a559e..614e974 100644
--- a/drivers/staging/tidspbridge/Kconfig
+++ b/drivers/staging/tidspbridge/Kconfig
@@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK
 config TIDSPBRIDGE_WDT3
 	bool "Enable watchdog timer"
 	depends on TIDSPBRIDGE
+	default y
 	help
 	  WTD3 is managed by DSP and once it is enabled, DSP side bridge is in
 	  charge of refreshing the timer before overflow, if the DSP hangs MPU
-- 
1.7.9

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Greg KH]

On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote:
> From: Felipe Contreras <felipe.contreras@nokia.com>
> 
> The public images have it enabled, it's safer, and we get rid of this
> warning:
> 
> WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c()
> In-band Error seen by IVA_SS  at address 0
> Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether
> [<c0012848>] (unwind_backtrace+0x0/0xec) from [<c002e760>] (warn_slowpath_common+0x4c/0x64)
> [<c002e760>] (warn_slowpath_common+0x4c/0x64) from [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c)
> [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c)
> [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c) from [<c0059844>] (handle_irq_event_percpu+0x28/0x174)
> [<c0059844>] (handle_irq_event_percpu+0x28/0x174) from [<c00599b8>] (handle_irq_event+0x28/0x38)
> [<c00599b8>] (handle_irq_event+0x28/0x38) from [<c005b8d0>] (handle_level_irq+0xb8/0xe0)
> [<c005b8d0>] (handle_level_irq+0xb8/0xe0) from [<c0059598>] (generic_handle_irq+0x28/0x30)
> [<c0059598>] (generic_handle_irq+0x28/0x30) from [<c000e62c>] (handle_IRQ+0x60/0x84)
> [<c000e62c>] (handle_IRQ+0x60/0x84) from [<c000d334>] (__irq_svc+0x34/0x80)
> [<c000d334>] (__irq_svc+0x34/0x80) from [<c0015804>] (v7_dma_inv_range+0x34/0x48)
> [<c0015804>] (v7_dma_inv_range+0x34/0x48) from [<c00129ec>] (dma_cache_maint_page+0x2c/0x34)
> [<c00129ec>] (dma_cache_maint_page+0x2c/0x34) from [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68)
> [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68) from [<c0012b50>] (dma_unmap_sg+0x40/0x64)
> [<c0012b50>] (dma_unmap_sg+0x40/0x64) from [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48)
> [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48) from [<c01932f4>] (mmc_post_req+0x18/0x1c)
> [<c01932f4>] (mmc_post_req+0x18/0x1c) from [<c019535c>] (mmc_start_req+0xd4/0xec)
> [<c019535c>] (mmc_start_req+0xd4/0xec) from [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244)
> [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244) from [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc)
> [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc) from [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8)
> [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8) from [<c0044c10>] (kthread+0x84/0x8c)
> [<c0044c10>] (kthread+0x84/0x8c) from [<c000e694>] (kernel_thread_exit+0x0/0x8)
> ---[ end trace 5dec1c8d7857375d ]---
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com>
> ---
>  drivers/staging/tidspbridge/Kconfig |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
> index 21a559e..614e974 100644
> --- a/drivers/staging/tidspbridge/Kconfig
> +++ b/drivers/staging/tidspbridge/Kconfig
> @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK
>  config TIDSPBRIDGE_WDT3
>  	bool "Enable watchdog timer"
>  	depends on TIDSPBRIDGE
> +	default y

Nothing is ever default y, sorry, I can't accept this.

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Wed, Feb 1, 2012 at 3:12 AM, Greg KH <gregkh@suse.de> wrote:
> On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote:
>> From: Felipe Contreras <felipe.contreras@nokia.com>
>>
>> The public images have it enabled, it's safer, and we get rid of this
>> warning:
>>
>> WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c()
>> In-band Error seen by IVA_SS  at address 0
>> Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether
>> [<c0012848>] (unwind_backtrace+0x0/0xec) from [<c002e760>] (warn_slowpath_common+0x4c/0x64)
>> [<c002e760>] (warn_slowpath_common+0x4c/0x64) from [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c)
>> [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c)
>> [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c) from [<c0059844>] (handle_irq_event_percpu+0x28/0x174)
>> [<c0059844>] (handle_irq_event_percpu+0x28/0x174) from [<c00599b8>] (handle_irq_event+0x28/0x38)
>> [<c00599b8>] (handle_irq_event+0x28/0x38) from [<c005b8d0>] (handle_level_irq+0xb8/0xe0)
>> [<c005b8d0>] (handle_level_irq+0xb8/0xe0) from [<c0059598>] (generic_handle_irq+0x28/0x30)
>> [<c0059598>] (generic_handle_irq+0x28/0x30) from [<c000e62c>] (handle_IRQ+0x60/0x84)
>> [<c000e62c>] (handle_IRQ+0x60/0x84) from [<c000d334>] (__irq_svc+0x34/0x80)
>> [<c000d334>] (__irq_svc+0x34/0x80) from [<c0015804>] (v7_dma_inv_range+0x34/0x48)
>> [<c0015804>] (v7_dma_inv_range+0x34/0x48) from [<c00129ec>] (dma_cache_maint_page+0x2c/0x34)
>> [<c00129ec>] (dma_cache_maint_page+0x2c/0x34) from [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68)
>> [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68) from [<c0012b50>] (dma_unmap_sg+0x40/0x64)
>> [<c0012b50>] (dma_unmap_sg+0x40/0x64) from [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48)
>> [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48) from [<c01932f4>] (mmc_post_req+0x18/0x1c)
>> [<c01932f4>] (mmc_post_req+0x18/0x1c) from [<c019535c>] (mmc_start_req+0xd4/0xec)
>> [<c019535c>] (mmc_start_req+0xd4/0xec) from [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244)
>> [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244) from [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc)
>> [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc) from [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8)
>> [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8) from [<c0044c10>] (kthread+0x84/0x8c)
>> [<c0044c10>] (kthread+0x84/0x8c) from [<c000e694>] (kernel_thread_exit+0x0/0x8)
>> ---[ end trace 5dec1c8d7857375d ]---
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com>
>> ---
>>  drivers/staging/tidspbridge/Kconfig |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
>> index 21a559e..614e974 100644
>> --- a/drivers/staging/tidspbridge/Kconfig
>> +++ b/drivers/staging/tidspbridge/Kconfig
>> @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK
>>  config TIDSPBRIDGE_WDT3
>>       bool "Enable watchdog timer"
>>       depends on TIDSPBRIDGE
>> +     default y
>
> Nothing is ever default y, sorry, I can't accept this.

% git grep 'default y' -- 'drivers/*/Kconfig' | wc -l
493

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Justin P. Mattock]

On 01/31/2012 05:37 PM, Felipe Contreras wrote:
> On Wed, Feb 1, 2012 at 3:12 AM, Greg KH<gregkh@suse.de>  wrote:
>> On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote:
>>> From: Felipe Contreras<felipe.contreras@nokia.com>
>>>
>>> The public images have it enabled, it's safer, and we get rid of this
>>> warning:
>>>
>>> WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c()
>>> In-band Error seen by IVA_SS  at address 0
>>> Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether
>>> [<c0012848>] (unwind_backtrace+0x0/0xec) from [<c002e760>] (warn_slowpath_common+0x4c/0x64)
>>> [<c002e760>] (warn_slowpath_common+0x4c/0x64) from [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c)
>>> [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c)
>>> [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c) from [<c0059844>] (handle_irq_event_percpu+0x28/0x174)
>>> [<c0059844>] (handle_irq_event_percpu+0x28/0x174) from [<c00599b8>] (handle_irq_event+0x28/0x38)
>>> [<c00599b8>] (handle_irq_event+0x28/0x38) from [<c005b8d0>] (handle_level_irq+0xb8/0xe0)
>>> [<c005b8d0>] (handle_level_irq+0xb8/0xe0) from [<c0059598>] (generic_handle_irq+0x28/0x30)
>>> [<c0059598>] (generic_handle_irq+0x28/0x30) from [<c000e62c>] (handle_IRQ+0x60/0x84)
>>> [<c000e62c>] (handle_IRQ+0x60/0x84) from [<c000d334>] (__irq_svc+0x34/0x80)
>>> [<c000d334>] (__irq_svc+0x34/0x80) from [<c0015804>] (v7_dma_inv_range+0x34/0x48)
>>> [<c0015804>] (v7_dma_inv_range+0x34/0x48) from [<c00129ec>] (dma_cache_maint_page+0x2c/0x34)
>>> [<c00129ec>] (dma_cache_maint_page+0x2c/0x34) from [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68)
>>> [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68) from [<c0012b50>] (dma_unmap_sg+0x40/0x64)
>>> [<c0012b50>] (dma_unmap_sg+0x40/0x64) from [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48)
>>> [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48) from [<c01932f4>] (mmc_post_req+0x18/0x1c)
>>> [<c01932f4>] (mmc_post_req+0x18/0x1c) from [<c019535c>] (mmc_start_req+0xd4/0xec)
>>> [<c019535c>] (mmc_start_req+0xd4/0xec) from [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244)
>>> [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244) from [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc)
>>> [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc) from [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8)
>>> [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8) from [<c0044c10>] (kthread+0x84/0x8c)
>>> [<c0044c10>] (kthread+0x84/0x8c) from [<c000e694>] (kernel_thread_exit+0x0/0x8)
>>> ---[ end trace 5dec1c8d7857375d ]---
>>>
>>> Signed-off-by: Felipe Contreras<felipe.contreras@nokia.com>
>>> ---
>>>   drivers/staging/tidspbridge/Kconfig |    1 +
>>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
>>> index 21a559e..614e974 100644
>>> --- a/drivers/staging/tidspbridge/Kconfig
>>> +++ b/drivers/staging/tidspbridge/Kconfig
>>> @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK
>>>   config TIDSPBRIDGE_WDT3
>>>        bool "Enable watchdog timer"
>>>        depends on TIDSPBRIDGE
>>> +     default y
>>
>> Nothing is ever default y, sorry, I can't accept this.
>
> % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l
> 493
>

not sure if I see this warning or not on my machine, but is there a fix 
for the warning? I would rather see that, than hide it with setting: 
default y (if that's what its doing!)

Justin P. Mattock

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Greg KH]

On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote:
> On Wed, Feb 1, 2012 at 3:12 AM, Greg KH <gregkh@suse.de> wrote:
> > On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote:
> >> From: Felipe Contreras <felipe.contreras@nokia.com>
> >>
> >> The public images have it enabled, it's safer, and we get rid of this
> >> warning:
> >>
> >> WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c()
> >> In-band Error seen by IVA_SS  at address 0
> >> Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether
> >> [<c0012848>] (unwind_backtrace+0x0/0xec) from [<c002e760>] (warn_slowpath_common+0x4c/0x64)
> >> [<c002e760>] (warn_slowpath_common+0x4c/0x64) from [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c)
> >> [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c)
> >> [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c) from [<c0059844>] (handle_irq_event_percpu+0x28/0x174)
> >> [<c0059844>] (handle_irq_event_percpu+0x28/0x174) from [<c00599b8>] (handle_irq_event+0x28/0x38)
> >> [<c00599b8>] (handle_irq_event+0x28/0x38) from [<c005b8d0>] (handle_level_irq+0xb8/0xe0)
> >> [<c005b8d0>] (handle_level_irq+0xb8/0xe0) from [<c0059598>] (generic_handle_irq+0x28/0x30)
> >> [<c0059598>] (generic_handle_irq+0x28/0x30) from [<c000e62c>] (handle_IRQ+0x60/0x84)
> >> [<c000e62c>] (handle_IRQ+0x60/0x84) from [<c000d334>] (__irq_svc+0x34/0x80)
> >> [<c000d334>] (__irq_svc+0x34/0x80) from [<c0015804>] (v7_dma_inv_range+0x34/0x48)
> >> [<c0015804>] (v7_dma_inv_range+0x34/0x48) from [<c00129ec>] (dma_cache_maint_page+0x2c/0x34)
> >> [<c00129ec>] (dma_cache_maint_page+0x2c/0x34) from [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68)
> >> [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68) from [<c0012b50>] (dma_unmap_sg+0x40/0x64)
> >> [<c0012b50>] (dma_unmap_sg+0x40/0x64) from [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48)
> >> [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48) from [<c01932f4>] (mmc_post_req+0x18/0x1c)
> >> [<c01932f4>] (mmc_post_req+0x18/0x1c) from [<c019535c>] (mmc_start_req+0xd4/0xec)
> >> [<c019535c>] (mmc_start_req+0xd4/0xec) from [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244)
> >> [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244) from [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc)
> >> [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc) from [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8)
> >> [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8) from [<c0044c10>] (kthread+0x84/0x8c)
> >> [<c0044c10>] (kthread+0x84/0x8c) from [<c000e694>] (kernel_thread_exit+0x0/0x8)
> >> ---[ end trace 5dec1c8d7857375d ]---
> >>
> >> Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com>
> >> ---
> >>  drivers/staging/tidspbridge/Kconfig |    1 +
> >>  1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
> >> index 21a559e..614e974 100644
> >> --- a/drivers/staging/tidspbridge/Kconfig
> >> +++ b/drivers/staging/tidspbridge/Kconfig
> >> @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK
> >>  config TIDSPBRIDGE_WDT3
> >>       bool "Enable watchdog timer"
> >>       depends on TIDSPBRIDGE
> >> +     default y
> >
> > Nothing is ever default y, sorry, I can't accept this.
> 
> % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l
> 493

That's nice, but proves nothing, no new entries should be y.

If it should be y, then it should just not be an option and always
enabled, right?  Why not do that?

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Wed, Feb 1, 2012 at 5:44 AM, Greg KH <greg@kroah.com> wrote:
> On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote:
>> On Wed, Feb 1, 2012 at 3:12 AM, Greg KH <gregkh@suse.de> wrote:
>> > On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote:
>> >> From: Felipe Contreras <felipe.contreras@nokia.com>
>> >>
>> >> The public images have it enabled, it's safer, and we get rid of this
>> >> warning:
>> >>
>> >> WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c()
>> >> In-band Error seen by IVA_SS  at address 0
>> >> Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether
>> >> [<c0012848>] (unwind_backtrace+0x0/0xec) from [<c002e760>] (warn_slowpath_common+0x4c/0x64)
>> >> [<c002e760>] (warn_slowpath_common+0x4c/0x64) from [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c)
>> >> [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c)
>> >> [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c) from [<c0059844>] (handle_irq_event_percpu+0x28/0x174)
>> >> [<c0059844>] (handle_irq_event_percpu+0x28/0x174) from [<c00599b8>] (handle_irq_event+0x28/0x38)
>> >> [<c00599b8>] (handle_irq_event+0x28/0x38) from [<c005b8d0>] (handle_level_irq+0xb8/0xe0)
>> >> [<c005b8d0>] (handle_level_irq+0xb8/0xe0) from [<c0059598>] (generic_handle_irq+0x28/0x30)
>> >> [<c0059598>] (generic_handle_irq+0x28/0x30) from [<c000e62c>] (handle_IRQ+0x60/0x84)
>> >> [<c000e62c>] (handle_IRQ+0x60/0x84) from [<c000d334>] (__irq_svc+0x34/0x80)
>> >> [<c000d334>] (__irq_svc+0x34/0x80) from [<c0015804>] (v7_dma_inv_range+0x34/0x48)
>> >> [<c0015804>] (v7_dma_inv_range+0x34/0x48) from [<c00129ec>] (dma_cache_maint_page+0x2c/0x34)
>> >> [<c00129ec>] (dma_cache_maint_page+0x2c/0x34) from [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68)
>> >> [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68) from [<c0012b50>] (dma_unmap_sg+0x40/0x64)
>> >> [<c0012b50>] (dma_unmap_sg+0x40/0x64) from [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48)
>> >> [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48) from [<c01932f4>] (mmc_post_req+0x18/0x1c)
>> >> [<c01932f4>] (mmc_post_req+0x18/0x1c) from [<c019535c>] (mmc_start_req+0xd4/0xec)
>> >> [<c019535c>] (mmc_start_req+0xd4/0xec) from [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244)
>> >> [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244) from [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc)
>> >> [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc) from [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8)
>> >> [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8) from [<c0044c10>] (kthread+0x84/0x8c)
>> >> [<c0044c10>] (kthread+0x84/0x8c) from [<c000e694>] (kernel_thread_exit+0x0/0x8)
>> >> ---[ end trace 5dec1c8d7857375d ]---
>> >>
>> >> Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com>
>> >> ---
>> >>  drivers/staging/tidspbridge/Kconfig |    1 +
>> >>  1 files changed, 1 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
>> >> index 21a559e..614e974 100644
>> >> --- a/drivers/staging/tidspbridge/Kconfig
>> >> +++ b/drivers/staging/tidspbridge/Kconfig
>> >> @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK
>> >>  config TIDSPBRIDGE_WDT3
>> >>       bool "Enable watchdog timer"
>> >>       depends on TIDSPBRIDGE
>> >> +     default y
>> >
>> > Nothing is ever default y, sorry, I can't accept this.
>>
>> % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l
>> 493
>
> That's nice, but proves nothing, no new entries should be y.
>
> If it should be y, then it should just not be an option and always
> enabled, right?  Why not do that?

What if I compiled a DSP baseimage that doesn't have this watchdog?

And using that logic, we should start removing all configurations in
the kernel that have 'default y'.

No. There's a reason they are *configurations*, and there's a reason
they are 'default y'.

You might have noticed Linus' rant about ARM defconfigs (LWN
article[1]), and the solution to that was simplified configs (make
savedefconfig), and the ideal would be to have no defconfigs at all.
But in order to achieve that ideal goal (or at least move in that
direction), the Kconfig files need to have good defaults--this was
also agreed in the discussion.

If you look at arch/arm/configs/, there are still quite a lot of
configs there, and many not small at all. If Linus goes forward with
his threat to remove ARM defconfigs, we would be in a bit of a
situation, specially since there's still a lot of Kconfigs that have
not been fixed.

There's plenty of other reasons why 'default y' is good.

This change is good, it helps everyone, and it doesn't hurt anyone.
But if you like, I can put forward a case and take it with the penguin
chief.

[1] http://lwn.net/Articles/391372/

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Wed, Feb 1, 2012 at 5:22 AM, Justin P. Mattock
<justinmattock@gmail.com> wrote:
> not sure if I see this warning or not on my machine, but is there a fix for
> the warning? I would rather see that, than hide it with setting: default y
> (if that's what its doing!)

No, there's no fix. IIRC Omar said it would not be easy.

-- 
Felipe Contreras

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Ramirez Luna, Omar]

On Wed, Feb 1, 2012 at 2:11 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Wed, Feb 1, 2012 at 5:22 AM, Justin P. Mattock
> <justinmattock@gmail.com> wrote:
>> not sure if I see this warning or not on my machine, but is there a fix for
>> the warning? I would rather see that, than hide it with setting: default y
>> (if that's what its doing!)
>
> No, there's no fix. IIRC Omar said it would not be easy.

Indeed, the ideal would be the ability to turn off the config option
but on the dsp side (when compiling the baseimage), however the
release package only contains binaries given that the source is not
open.

I have been exploring introducing a flag in the shared memory area
which would require a new baseimage release.

Regards,

Omar

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Greg KH]

On Wed, Feb 01, 2012 at 09:26:16AM +0200, Felipe Contreras wrote:
> On Wed, Feb 1, 2012 at 5:44 AM, Greg KH <greg@kroah.com> wrote:
> > On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote:
> >> On Wed, Feb 1, 2012 at 3:12 AM, Greg KH <gregkh@suse.de> wrote:
> >> > On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote:
> >> >> From: Felipe Contreras <felipe.contreras@nokia.com>
> >> >>
> >> >> The public images have it enabled, it's safer, and we get rid of this
> >> >> warning:
> >> >>
> >> >> WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c()
> >> >> In-band Error seen by IVA_SS  at address 0
> >> >> Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether
> >> >> [<c0012848>] (unwind_backtrace+0x0/0xec) from [<c002e760>] (warn_slowpath_common+0x4c/0x64)
> >> >> [<c002e760>] (warn_slowpath_common+0x4c/0x64) from [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c)
> >> >> [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c)
> >> >> [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c) from [<c0059844>] (handle_irq_event_percpu+0x28/0x174)
> >> >> [<c0059844>] (handle_irq_event_percpu+0x28/0x174) from [<c00599b8>] (handle_irq_event+0x28/0x38)
> >> >> [<c00599b8>] (handle_irq_event+0x28/0x38) from [<c005b8d0>] (handle_level_irq+0xb8/0xe0)
> >> >> [<c005b8d0>] (handle_level_irq+0xb8/0xe0) from [<c0059598>] (generic_handle_irq+0x28/0x30)
> >> >> [<c0059598>] (generic_handle_irq+0x28/0x30) from [<c000e62c>] (handle_IRQ+0x60/0x84)
> >> >> [<c000e62c>] (handle_IRQ+0x60/0x84) from [<c000d334>] (__irq_svc+0x34/0x80)
> >> >> [<c000d334>] (__irq_svc+0x34/0x80) from [<c0015804>] (v7_dma_inv_range+0x34/0x48)
> >> >> [<c0015804>] (v7_dma_inv_range+0x34/0x48) from [<c00129ec>] (dma_cache_maint_page+0x2c/0x34)
> >> >> [<c00129ec>] (dma_cache_maint_page+0x2c/0x34) from [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68)
> >> >> [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68) from [<c0012b50>] (dma_unmap_sg+0x40/0x64)
> >> >> [<c0012b50>] (dma_unmap_sg+0x40/0x64) from [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48)
> >> >> [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48) from [<c01932f4>] (mmc_post_req+0x18/0x1c)
> >> >> [<c01932f4>] (mmc_post_req+0x18/0x1c) from [<c019535c>] (mmc_start_req+0xd4/0xec)
> >> >> [<c019535c>] (mmc_start_req+0xd4/0xec) from [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244)
> >> >> [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244) from [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc)
> >> >> [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc) from [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8)
> >> >> [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8) from [<c0044c10>] (kthread+0x84/0x8c)
> >> >> [<c0044c10>] (kthread+0x84/0x8c) from [<c000e694>] (kernel_thread_exit+0x0/0x8)
> >> >> ---[ end trace 5dec1c8d7857375d ]---
> >> >>
> >> >> Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com>
> >> >> ---
> >> >>  drivers/staging/tidspbridge/Kconfig |    1 +
> >> >>  1 files changed, 1 insertions(+), 0 deletions(-)
> >> >>
> >> >> diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
> >> >> index 21a559e..614e974 100644
> >> >> --- a/drivers/staging/tidspbridge/Kconfig
> >> >> +++ b/drivers/staging/tidspbridge/Kconfig
> >> >> @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK
> >> >>  config TIDSPBRIDGE_WDT3
> >> >>       bool "Enable watchdog timer"
> >> >>       depends on TIDSPBRIDGE
> >> >> +     default y
> >> >
> >> > Nothing is ever default y, sorry, I can't accept this.
> >>
> >> % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l
> >> 493
> >
> > That's nice, but proves nothing, no new entries should be y.
> >
> > If it should be y, then it should just not be an option and always
> > enabled, right?  Why not do that?
> 
> What if I compiled a DSP baseimage that doesn't have this watchdog?

I don't know, you tell me.

> And using that logic, we should start removing all configurations in
> the kernel that have 'default y'.

If your machine needs it to boot, then no, do not.

> No. There's a reason they are *configurations*, and there's a reason
> they are 'default y'.

Again, no NEW config options should be default y unless you really have
a good reason.  And I'll take that a step further and say, if you want
this to be default y, then why even make it a config option at all and
not just build it in whenever the dependancy is detected?

So, care to just send a patch removing this config option instead?

Or, if bad things happen if the option is turned off, then fix those,
don't paper over it by trying to enable the option, that's not a real
fix.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Thu, Feb 9, 2012 at 7:35 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Wed, Feb 01, 2012 at 09:26:16AM +0200, Felipe Contreras wrote:
>> On Wed, Feb 1, 2012 at 5:44 AM, Greg KH <greg@kroah.com> wrote:
>> > On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote:
>> >> On Wed, Feb 1, 2012 at 3:12 AM, Greg KH <gregkh@suse.de> wrote:
>> >> > On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote:
>> >> >> From: Felipe Contreras <felipe.contreras@nokia.com>
>> >> >>
>> >> >> The public images have it enabled, it's safer, and we get rid of this
>> >> >> warning:
>> >> >>
>> >> >> WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c()
>> >> >> In-band Error seen by IVA_SS  at address 0
>> >> >> Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether
>> >> >> [<c0012848>] (unwind_backtrace+0x0/0xec) from [<c002e760>] (warn_slowpath_common+0x4c/0x64)
>> >> >> [<c002e760>] (warn_slowpath_common+0x4c/0x64) from [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c)
>> >> >> [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c)
>> >> >> [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c) from [<c0059844>] (handle_irq_event_percpu+0x28/0x174)
>> >> >> [<c0059844>] (handle_irq_event_percpu+0x28/0x174) from [<c00599b8>] (handle_irq_event+0x28/0x38)
>> >> >> [<c00599b8>] (handle_irq_event+0x28/0x38) from [<c005b8d0>] (handle_level_irq+0xb8/0xe0)
>> >> >> [<c005b8d0>] (handle_level_irq+0xb8/0xe0) from [<c0059598>] (generic_handle_irq+0x28/0x30)
>> >> >> [<c0059598>] (generic_handle_irq+0x28/0x30) from [<c000e62c>] (handle_IRQ+0x60/0x84)
>> >> >> [<c000e62c>] (handle_IRQ+0x60/0x84) from [<c000d334>] (__irq_svc+0x34/0x80)
>> >> >> [<c000d334>] (__irq_svc+0x34/0x80) from [<c0015804>] (v7_dma_inv_range+0x34/0x48)
>> >> >> [<c0015804>] (v7_dma_inv_range+0x34/0x48) from [<c00129ec>] (dma_cache_maint_page+0x2c/0x34)
>> >> >> [<c00129ec>] (dma_cache_maint_page+0x2c/0x34) from [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68)
>> >> >> [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68) from [<c0012b50>] (dma_unmap_sg+0x40/0x64)
>> >> >> [<c0012b50>] (dma_unmap_sg+0x40/0x64) from [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48)
>> >> >> [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48) from [<c01932f4>] (mmc_post_req+0x18/0x1c)
>> >> >> [<c01932f4>] (mmc_post_req+0x18/0x1c) from [<c019535c>] (mmc_start_req+0xd4/0xec)
>> >> >> [<c019535c>] (mmc_start_req+0xd4/0xec) from [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244)
>> >> >> [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244) from [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc)
>> >> >> [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc) from [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8)
>> >> >> [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8) from [<c0044c10>] (kthread+0x84/0x8c)
>> >> >> [<c0044c10>] (kthread+0x84/0x8c) from [<c000e694>] (kernel_thread_exit+0x0/0x8)
>> >> >> ---[ end trace 5dec1c8d7857375d ]---
>> >> >>
>> >> >> Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com>
>> >> >> ---
>> >> >>  drivers/staging/tidspbridge/Kconfig |    1 +
>> >> >>  1 files changed, 1 insertions(+), 0 deletions(-)
>> >> >>
>> >> >> diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
>> >> >> index 21a559e..614e974 100644
>> >> >> --- a/drivers/staging/tidspbridge/Kconfig
>> >> >> +++ b/drivers/staging/tidspbridge/Kconfig
>> >> >> @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK
>> >> >>  config TIDSPBRIDGE_WDT3
>> >> >>       bool "Enable watchdog timer"
>> >> >>       depends on TIDSPBRIDGE
>> >> >> +     default y
>> >> >
>> >> > Nothing is ever default y, sorry, I can't accept this.
>> >>
>> >> % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l
>> >> 493
>> >
>> > That's nice, but proves nothing, no new entries should be y.
>> >
>> > If it should be y, then it should just not be an option and always
>> > enabled, right?  Why not do that?
>>
>> What if I compiled a DSP baseimage that doesn't have this watchdog?
>
> I don't know, you tell me.

Then you want this option disabled.

>> And using that logic, we should start removing all configurations in
>> the kernel that have 'default y'.
>
> If your machine needs it to boot, then no, do not.

There's many configurations that are not needed for booting, yet they
are 'default y'.

>> No. There's a reason they are *configurations*, and there's a reason
>> they are 'default y'.
>
> Again, no NEW config options should be default y unless you really have
> a good reason.

I already gave you a good reason, and this came from Linus Torvalds.

http://article.gmane.org/gmane.linux.kernel/995419

Notice how he agreed that setting defaults was a good idea. I can't
point to the mails of other people that also agreed this was the
direction to go.

You keep saying no options should be 'default y', but you don't say
where that policy comes from.

> And I'll take that a step further and say, if you want
> this to be default y, then why even make it a config option at all and
> not just build it in whenever the dependancy is detected?

The dependency can't be detected.

> So, care to just send a patch removing this config option instead?

No. That's not what anybody wants.

> Or, if bad things happen if the option is turned off, then fix those,
> don't paper over it by trying to enable the option, that's not a real
> fix.

The real fix is not coming any time soon (if possible at all). So, do
you want the warning fixed or not?

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Thu, Feb 9, 2012 at 8:41 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> I already gave you a good reason, and this came from Linus Torvalds.
>
> http://article.gmane.org/gmane.linux.kernel/995419
>
> Notice how he agreed that setting defaults was a good idea. I can't
> point to the mails of other people that also agreed this was the
> direction to go.

Er, I can, if you want.

-- 
Felipe Contreras

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Greg KH]

On Thu, Feb 09, 2012 at 08:41:55PM +0200, Felipe Contreras wrote:
> On Thu, Feb 9, 2012 at 7:35 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Wed, Feb 01, 2012 at 09:26:16AM +0200, Felipe Contreras wrote:
> >> On Wed, Feb 1, 2012 at 5:44 AM, Greg KH <greg@kroah.com> wrote:
> >> > On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote:
> >> >> On Wed, Feb 1, 2012 at 3:12 AM, Greg KH <gregkh@suse.de> wrote:
> >> >> > On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote:
> >> >> >> From: Felipe Contreras <felipe.contreras@nokia.com>
> >> >> >>
> >> >> >> The public images have it enabled, it's safer, and we get rid of this
> >> >> >> warning:
> >> >> >>
> >> >> >> WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c()
> >> >> >> In-band Error seen by IVA_SS  at address 0
> >> >> >> Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether
> >> >> >> [<c0012848>] (unwind_backtrace+0x0/0xec) from [<c002e760>] (warn_slowpath_common+0x4c/0x64)
> >> >> >> [<c002e760>] (warn_slowpath_common+0x4c/0x64) from [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c)
> >> >> >> [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c)
> >> >> >> [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c) from [<c0059844>] (handle_irq_event_percpu+0x28/0x174)
> >> >> >> [<c0059844>] (handle_irq_event_percpu+0x28/0x174) from [<c00599b8>] (handle_irq_event+0x28/0x38)
> >> >> >> [<c00599b8>] (handle_irq_event+0x28/0x38) from [<c005b8d0>] (handle_level_irq+0xb8/0xe0)
> >> >> >> [<c005b8d0>] (handle_level_irq+0xb8/0xe0) from [<c0059598>] (generic_handle_irq+0x28/0x30)
> >> >> >> [<c0059598>] (generic_handle_irq+0x28/0x30) from [<c000e62c>] (handle_IRQ+0x60/0x84)
> >> >> >> [<c000e62c>] (handle_IRQ+0x60/0x84) from [<c000d334>] (__irq_svc+0x34/0x80)
> >> >> >> [<c000d334>] (__irq_svc+0x34/0x80) from [<c0015804>] (v7_dma_inv_range+0x34/0x48)
> >> >> >> [<c0015804>] (v7_dma_inv_range+0x34/0x48) from [<c00129ec>] (dma_cache_maint_page+0x2c/0x34)
> >> >> >> [<c00129ec>] (dma_cache_maint_page+0x2c/0x34) from [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68)
> >> >> >> [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68) from [<c0012b50>] (dma_unmap_sg+0x40/0x64)
> >> >> >> [<c0012b50>] (dma_unmap_sg+0x40/0x64) from [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48)
> >> >> >> [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48) from [<c01932f4>] (mmc_post_req+0x18/0x1c)
> >> >> >> [<c01932f4>] (mmc_post_req+0x18/0x1c) from [<c019535c>] (mmc_start_req+0xd4/0xec)
> >> >> >> [<c019535c>] (mmc_start_req+0xd4/0xec) from [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244)
> >> >> >> [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244) from [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc)
> >> >> >> [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc) from [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8)
> >> >> >> [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8) from [<c0044c10>] (kthread+0x84/0x8c)
> >> >> >> [<c0044c10>] (kthread+0x84/0x8c) from [<c000e694>] (kernel_thread_exit+0x0/0x8)
> >> >> >> ---[ end trace 5dec1c8d7857375d ]---
> >> >> >>
> >> >> >> Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com>
> >> >> >> ---
> >> >> >>  drivers/staging/tidspbridge/Kconfig |    1 +
> >> >> >>  1 files changed, 1 insertions(+), 0 deletions(-)
> >> >> >>
> >> >> >> diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
> >> >> >> index 21a559e..614e974 100644
> >> >> >> --- a/drivers/staging/tidspbridge/Kconfig
> >> >> >> +++ b/drivers/staging/tidspbridge/Kconfig
> >> >> >> @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK
> >> >> >>  config TIDSPBRIDGE_WDT3
> >> >> >>       bool "Enable watchdog timer"
> >> >> >>       depends on TIDSPBRIDGE
> >> >> >> +     default y
> >> >> >
> >> >> > Nothing is ever default y, sorry, I can't accept this.
> >> >>
> >> >> % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l
> >> >> 493
> >> >
> >> > That's nice, but proves nothing, no new entries should be y.
> >> >
> >> > If it should be y, then it should just not be an option and always
> >> > enabled, right?  Why not do that?
> >>
> >> What if I compiled a DSP baseimage that doesn't have this watchdog?
> >
> > I don't know, you tell me.
> 
> Then you want this option disabled.
> 
> >> And using that logic, we should start removing all configurations in
> >> the kernel that have 'default y'.
> >
> > If your machine needs it to boot, then no, do not.
> 
> There's many configurations that are not needed for booting, yet they
> are 'default y'.

Again, for historical reasons, yes, but do you see that happening for
"new" code these days?

And if it does get set to 'y' for new code, you have seen the yelling,
right?

Again, I'm totally confused as to _WHY_ this needs to be y.  What is
causing this oops without it?  If an oops is happening, then shouldn't
this be a strict dependancy?  Why allow it to be disabled at all if it
can break your box if you don't enable it?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Thu, Feb 9, 2012 at 8:59 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Feb 09, 2012 at 08:41:55PM +0200, Felipe Contreras wrote:
>> On Thu, Feb 9, 2012 at 7:35 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Wed, Feb 01, 2012 at 09:26:16AM +0200, Felipe Contreras wrote:
>> >> On Wed, Feb 1, 2012 at 5:44 AM, Greg KH <greg@kroah.com> wrote:
>> >> > On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote:
>> >> >> On Wed, Feb 1, 2012 at 3:12 AM, Greg KH <gregkh@suse.de> wrote:
>> >> >> > On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote:
>> >> >> >> From: Felipe Contreras <felipe.contreras@nokia.com>
>> >> >> >>
>> >> >> >> The public images have it enabled, it's safer, and we get rid of this
>> >> >> >> warning:
>> >> >> >>
>> >> >> >> WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c()
>> >> >> >> In-band Error seen by IVA_SS  at address 0
>> >> >> >> Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether
>> >> >> >> [<c0012848>] (unwind_backtrace+0x0/0xec) from [<c002e760>] (warn_slowpath_common+0x4c/0x64)
>> >> >> >> [<c002e760>] (warn_slowpath_common+0x4c/0x64) from [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c)
>> >> >> >> [<c002e7f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c)
>> >> >> >> [<c00217c8>] (omap3_l3_app_irq+0x114/0x15c) from [<c0059844>] (handle_irq_event_percpu+0x28/0x174)
>> >> >> >> [<c0059844>] (handle_irq_event_percpu+0x28/0x174) from [<c00599b8>] (handle_irq_event+0x28/0x38)
>> >> >> >> [<c00599b8>] (handle_irq_event+0x28/0x38) from [<c005b8d0>] (handle_level_irq+0xb8/0xe0)
>> >> >> >> [<c005b8d0>] (handle_level_irq+0xb8/0xe0) from [<c0059598>] (generic_handle_irq+0x28/0x30)
>> >> >> >> [<c0059598>] (generic_handle_irq+0x28/0x30) from [<c000e62c>] (handle_IRQ+0x60/0x84)
>> >> >> >> [<c000e62c>] (handle_IRQ+0x60/0x84) from [<c000d334>] (__irq_svc+0x34/0x80)
>> >> >> >> [<c000d334>] (__irq_svc+0x34/0x80) from [<c0015804>] (v7_dma_inv_range+0x34/0x48)
>> >> >> >> [<c0015804>] (v7_dma_inv_range+0x34/0x48) from [<c00129ec>] (dma_cache_maint_page+0x2c/0x34)
>> >> >> >> [<c00129ec>] (dma_cache_maint_page+0x2c/0x34) from [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68)
>> >> >> >> [<c0012a34>] (___dma_page_dev_to_cpu+0x24/0x68) from [<c0012b50>] (dma_unmap_sg+0x40/0x64)
>> >> >> >> [<c0012b50>] (dma_unmap_sg+0x40/0x64) from [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48)
>> >> >> >> [<c019fba4>] (omap_hsmmc_post_req+0x3c/0x48) from [<c01932f4>] (mmc_post_req+0x18/0x1c)
>> >> >> >> [<c01932f4>] (mmc_post_req+0x18/0x1c) from [<c019535c>] (mmc_start_req+0xd4/0xec)
>> >> >> >> [<c019535c>] (mmc_start_req+0xd4/0xec) from [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244)
>> >> >> >> [<c019d330>] (mmc_blk_issue_rw_rq+0x64/0x244) from [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc)
>> >> >> >> [<c019d7bc>] (mmc_blk_issue_rq+0x2ac/0x2cc) from [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8)
>> >> >> >> [<c019e1c0>] (mmc_queue_thread+0x8c/0xf8) from [<c0044c10>] (kthread+0x84/0x8c)
>> >> >> >> [<c0044c10>] (kthread+0x84/0x8c) from [<c000e694>] (kernel_thread_exit+0x0/0x8)
>> >> >> >> ---[ end trace 5dec1c8d7857375d ]---
>> >> >> >>
>> >> >> >> Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com>
>> >> >> >> ---
>> >> >> >>  drivers/staging/tidspbridge/Kconfig |    1 +
>> >> >> >>  1 files changed, 1 insertions(+), 0 deletions(-)
>> >> >> >>
>> >> >> >> diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
>> >> >> >> index 21a559e..614e974 100644
>> >> >> >> --- a/drivers/staging/tidspbridge/Kconfig
>> >> >> >> +++ b/drivers/staging/tidspbridge/Kconfig
>> >> >> >> @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK
>> >> >> >>  config TIDSPBRIDGE_WDT3
>> >> >> >>       bool "Enable watchdog timer"
>> >> >> >>       depends on TIDSPBRIDGE
>> >> >> >> +     default y
>> >> >> >
>> >> >> > Nothing is ever default y, sorry, I can't accept this.
>> >> >>
>> >> >> % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l
>> >> >> 493
>> >> >
>> >> > That's nice, but proves nothing, no new entries should be y.
>> >> >
>> >> > If it should be y, then it should just not be an option and always
>> >> > enabled, right?  Why not do that?
>> >>
>> >> What if I compiled a DSP baseimage that doesn't have this watchdog?
>> >
>> > I don't know, you tell me.
>>
>> Then you want this option disabled.
>>
>> >> And using that logic, we should start removing all configurations in
>> >> the kernel that have 'default y'.
>> >
>> > If your machine needs it to boot, then no, do not.
>>
>> There's many configurations that are not needed for booting, yet they
>> are 'default y'.
>
> Again, for historical reasons, yes, but do you see that happening for
> "new" code these days?

Yes, I already explained why, and I already pointed to a link where
Linus says it's the right thing to do. Here it is again:

http://article.gmane.org/gmane.linux.kernel/995419

It is needed to simplify defconfigs.

> And if it does get set to 'y' for new code, you have seen the yelling,
> right?

I didn't parse that.

> Again, I'm totally confused as to _WHY_ this needs to be y.  What is
> causing this oops without it?  If an oops is happening, then shouldn't
> this be a strict dependancy?  Why allow it to be disabled at all if it
> can break your box if you don't enable it?

It's not an oops, it's a warning, and again, it depends on the
firmware being used. We don't have control over that, and we have no
way to detect if this feature is there. It's up to the user.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Ramirez Luna, Omar]

Hi,

On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>> Again, I'm totally confused as to _WHY_ this needs to be y.  What is
>> causing this oops without it?  If an oops is happening, then shouldn't
>> this be a strict dependancy?  Why allow it to be disabled at all if it
>> can break your box if you don't enable it?
>
> It's not an oops, it's a warning, and again, it depends on the
> firmware being used. We don't have control over that, and we have no
> way to detect if this feature is there. It's up to the user.

I have been thinking more into it, how about looking for a WDT symbol
inside the baseimage to decide whether to turn ON/OFF WDT3, this would
mean that the code is always compiled in, but the decision to turn it
on/off is made at runtime.

Regards,

Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Greg KH]

On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote:
> Hi,
> 
> On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >> Again, I'm totally confused as to _WHY_ this needs to be y.  What is
> >> causing this oops without it?  If an oops is happening, then shouldn't
> >> this be a strict dependancy?  Why allow it to be disabled at all if it
> >> can break your box if you don't enable it?
> >
> > It's not an oops, it's a warning, and again, it depends on the
> > firmware being used. We don't have control over that, and we have no
> > way to detect if this feature is there. It's up to the user.
> 
> I have been thinking more into it, how about looking for a WDT symbol
> inside the baseimage to decide whether to turn ON/OFF WDT3, this would
> mean that the code is always compiled in, but the decision to turn it
> on/off is made at runtime.

I totally don't understand, why not just silence the warning properly
then?

I fail to understand why this warning happens, why it depends on the
firmware, and why you can't detect it at runtime to not do it.  And how
it all ties into a kconfig option...

confused,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Justin P. Mattock]

On Thu, 9 Feb 2012 21:18:49 -0800
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote:
> > Hi,
> > 
> > On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
> > >> Again, I'm totally confused as to _WHY_ this needs to be y.  What is
> > >> causing this oops without it?  If an oops is happening, then shouldn't
> > >> this be a strict dependancy?  Why allow it to be disabled at all if it
> > >> can break your box if you don't enable it?
> > >
> > > It's not an oops, it's a warning, and again, it depends on the
> > > firmware being used. We don't have control over that, and we have no
> > > way to detect if this feature is there. It's up to the user.
> > 
> > I have been thinking more into it, how about looking for a WDT symbol
> > inside the baseimage to decide whether to turn ON/OFF WDT3, this would
> > mean that the code is always compiled in, but the decision to turn it
> > on/off is made at runtime.
> 
> I totally don't understand, why not just silence the warning properly
> then?
> 
> I fail to understand why this warning happens, why it depends on the
> firmware, and why you can't detect it at runtime to not do it.  And how
> it all ties into a kconfig option...
> 
> confused,
> 
> greg k-h

so there are _two_ issues that need to be fixed here:

1) the warning fix.  

2) the whole default y thing that Mr. Torvalds is talking about.
(the first fix(in my mind)would need to go first before anything else)

Now reading through, greg had mentioned something about dependency, so if this is just dependency then add the proper
Kconfig option, but if this is more than just a _dependency_ then somebody(who knows this code)is going to 
have to supply the proper fix for the warning(my C skills only take me so far!)

below is my go at sending a _dependency_ fix for this, but could be totally wrong..

From 5c7ad6c00d051d5444474007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001
From: "Justin P. Mattock" <justinmattock@gmail.com>
Date: Fri, 10 Feb 2012 07:19:45 -0800
Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE.

This would add the missing _dependency_ to tidsbridge to prevent a warning from happening.

Note: my Kconfig skills are not the greatest so the below may or may not work.
I can't test this because I dont have the hardware. 

Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

---
 drivers/staging/tidspbridge/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
index 21a559e..7b385e0 100644
--- a/drivers/staging/tidspbridge/Kconfig
+++ b/drivers/staging/tidspbridge/Kconfig
@@ -6,6 +6,7 @@ menuconfig TIDSPBRIDGE
 	tristate "DSP Bridge driver"
 	depends on ARCH_OMAP3
 	select OMAP_MBOX_FWK
+	select TIDSPBRIDGE_WDT3
 	help
 	  DSP/BIOS Bridge is designed for platforms that contain a GPP and
 	  one or more attached DSPs.  The GPP is considered the master or
-- 
1.7.9


-- 
Justin P. Mattock <justinmattock@gmail.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Fri, Feb 10, 2012 at 5:35 PM, Justin P. Mattock
<justinmattock@gmail.com> wrote:
> On Thu, 9 Feb 2012 21:18:49 -0800
> Greg KH <gregkh@linuxfoundation.org> wrote:
>
>> On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote:
>> > Hi,
>> >
>> > On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras
>> > <felipe.contreras@gmail.com> wrote:
>> > >> Again, I'm totally confused as to _WHY_ this needs to be y.  What is
>> > >> causing this oops without it?  If an oops is happening, then shouldn't
>> > >> this be a strict dependancy?  Why allow it to be disabled at all if it
>> > >> can break your box if you don't enable it?
>> > >
>> > > It's not an oops, it's a warning, and again, it depends on the
>> > > firmware being used. We don't have control over that, and we have no
>> > > way to detect if this feature is there. It's up to the user.
>> >
>> > I have been thinking more into it, how about looking for a WDT symbol
>> > inside the baseimage to decide whether to turn ON/OFF WDT3, this would
>> > mean that the code is always compiled in, but the decision to turn it
>> > on/off is made at runtime.
>>
>> I totally don't understand, why not just silence the warning properly
>> then?
>>
>> I fail to understand why this warning happens, why it depends on the
>> firmware, and why you can't detect it at runtime to not do it.  And how
>> it all ties into a kconfig option...
>>
>> confused,
>>
>> greg k-h
>
> so there are _two_ issues that need to be fixed here:
>
> 1) the warning fix.
>
> 2) the whole default y thing that Mr. Torvalds is talking about.
> (the first fix(in my mind)would need to go first before anything else)
>
> Now reading through, greg had mentioned something about dependency, so if this is just dependency then add the proper
> Kconfig option, but if this is more than just a _dependency_ then somebody(who knows this code)is going to
> have to supply the proper fix for the warning(my C skills only take me so far!)
>
> below is my go at sending a _dependency_ fix for this, but could be totally wrong..
>
> From 5c7ad6c00d051d5444474007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001
> From: "Justin P. Mattock" <justinmattock@gmail.com>
> Date: Fri, 10 Feb 2012 07:19:45 -0800
> Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE.
>
> This would add the missing _dependency_ to tidsbridge to prevent a warning from happening.
>
> Note: my Kconfig skills are not the greatest so the below may or may not work.
> I can't test this because I dont have the hardware.

Your patch *always* turns on TIDSPBRIDGE_WDT3, which is not what we
want. Depending on the firmware, some people might want it off.

Basically, right now on the typical firmware, people have to either
manually turn TIDSPBRIDGE_WDT3 on, or they will see the warning.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Justin P. Mattock]

On Fri, 10 Feb 2012 18:05:59 +0200
Felipe Contreras <felipe.contreras@gmail.com> wrote:

> On Fri, Feb 10, 2012 at 5:35 PM, Justin P. Mattock
> <justinmattock@gmail.com> wrote:
> > On Thu, 9 Feb 2012 21:18:49 -0800
> > Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> >> On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote:
> >> > Hi,
> >> >
> >> > On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras
> >> > <felipe.contreras@gmail.com> wrote:
> >> > >> Again, I'm totally confused as to _WHY_ this needs to be y.  What is
> >> > >> causing this oops without it?  If an oops is happening, then shouldn't
> >> > >> this be a strict dependancy?  Why allow it to be disabled at all if it
> >> > >> can break your box if you don't enable it?
> >> > >
> >> > > It's not an oops, it's a warning, and again, it depends on the
> >> > > firmware being used. We don't have control over that, and we have no
> >> > > way to detect if this feature is there. It's up to the user.
> >> >
> >> > I have been thinking more into it, how about looking for a WDT symbol
> >> > inside the baseimage to decide whether to turn ON/OFF WDT3, this would
> >> > mean that the code is always compiled in, but the decision to turn it
> >> > on/off is made at runtime.
> >>
> >> I totally don't understand, why not just silence the warning properly
> >> then?
> >>
> >> I fail to understand why this warning happens, why it depends on the
> >> firmware, and why you can't detect it at runtime to not do it.  And how
> >> it all ties into a kconfig option...
> >>
> >> confused,
> >>
> >> greg k-h
> >
> > so there are _two_ issues that need to be fixed here:
> >
> > 1) the warning fix.
> >
> > 2) the whole default y thing that Mr. Torvalds is talking about.
> > (the first fix(in my mind)would need to go first before anything else)
> >
> > Now reading through, greg had mentioned something about dependency, so if this is just dependency then add the proper
> > Kconfig option, but if this is more than just a _dependency_ then somebody(who knows this code)is going to
> > have to supply the proper fix for the warning(my C skills only take me so far!)
> >
> > below is my go at sending a _dependency_ fix for this, but could be totally wrong..
> >
> > From 5c7ad6c00d051d5444474007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001
> > From: "Justin P. Mattock" <justinmattock@gmail.com>
> > Date: Fri, 10 Feb 2012 07:19:45 -0800
> > Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE.
> >
> > This would add the missing _dependency_ to tidsbridge to prevent a warning from happening.
> >
> > Note: my Kconfig skills are not the greatest so the below may or may not work.
> > I can't test this because I dont have the hardware.
> 
> Your patch *always* turns on TIDSPBRIDGE_WDT3, which is not what we
> want. Depending on the firmware, some people might want it off.
> 
> Basically, right now on the typical firmware, people have to either
> manually turn TIDSPBRIDGE_WDT3 on, or they will see the warning.
> 
> Cheers.
> 
> -- 
> Felipe Contreras

ahh!! I see.. 
then maybe it needs be something like what omar had suggested, or maybe even some 
dmi_system_id(but the kernel has to many of those).

-- 
Justin P. Mattock <justinmattock@gmail.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Fri, Feb 10, 2012 at 7:18 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote:
>> Hi,
>>
>> On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>> >> Again, I'm totally confused as to _WHY_ this needs to be y.  What is
>> >> causing this oops without it?  If an oops is happening, then shouldn't
>> >> this be a strict dependancy?  Why allow it to be disabled at all if it
>> >> can break your box if you don't enable it?
>> >
>> > It's not an oops, it's a warning, and again, it depends on the
>> > firmware being used. We don't have control over that, and we have no
>> > way to detect if this feature is there. It's up to the user.
>>
>> I have been thinking more into it, how about looking for a WDT symbol
>> inside the baseimage to decide whether to turn ON/OFF WDT3, this would
>> mean that the code is always compiled in, but the decision to turn it
>> on/off is made at runtime.
>
> I totally don't understand, why not just silence the warning properly
> then?

The warning doesn't come from the driver, and AFAIK it's a valid
warning. Now, I haven't explored the code, so I don't know what's
going on, I just know Omar said this fixes the warning, and that
there's no easy way around it.

> I fail to understand why this warning happens,

I don't know either, I just know enabling TIDSPBRIDGE_WDT3 fixes it.

> why it depends on the firmware,

Because it just does.

Does your firmware have WDT? Yes => TIDSPBRIDGE_WDT3 on, No =>
TIDSPBRIDGE_WDT3 off.

What is so complicated about this?

> and why you can't detect it at runtime to not do it.

The DSP "firmwmare" is an operating system. You even run Linux on it.

We just don't know if has WDT support or not. Omar is familiar with
the code regarding the WDT, and he said it was not easy to detect if
enabling WDT failed or not.

> And how it all ties into a kconfig option...

Some people want it, some people don't.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Greg KH]

On Fri, Feb 10, 2012 at 06:05:59PM +0200, Felipe Contreras wrote:
> > From 5c7ad6c00d051d5444474007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001
> > From: "Justin P. Mattock" <justinmattock@gmail.com>
> > Date: Fri, 10 Feb 2012 07:19:45 -0800
> > Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE.
> >
> > This would add the missing _dependency_ to tidsbridge to prevent a warning from happening.
> >
> > Note: my Kconfig skills are not the greatest so the below may or may not work.
> > I can't test this because I dont have the hardware.
> 
> Your patch *always* turns on TIDSPBRIDGE_WDT3, which is not what we
> want. Depending on the firmware, some people might want it off.

What firmware?  Why not document this properly somewhere in the help
entries?  Why not detect this automatically in the kernel based on the
firmware version?

> Basically, right now on the typical firmware, people have to either
> manually turn TIDSPBRIDGE_WDT3 on, or they will see the warning.

So, for the "typical" firmware, you do want this on, so the patch makes
sense.

How about the code be fixed so that it doesn't generate this type of
warning when using the "typical" firmware, instead of having to rely on
confusing Kconfig entries.

Still confused,

greg k-h

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Fri, Feb 10, 2012 at 6:16 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Feb 10, 2012 at 06:05:59PM +0200, Felipe Contreras wrote:
>> > From 5c7ad6c00d051d5444474007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001
>> > From: "Justin P. Mattock" <justinmattock@gmail.com>
>> > Date: Fri, 10 Feb 2012 07:19:45 -0800
>> > Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE.
>> >
>> > This would add the missing _dependency_ to tidsbridge to prevent a warning from happening.
>> >
>> > Note: my Kconfig skills are not the greatest so the below may or may not work.
>> > I can't test this because I dont have the hardware.
>>
>> Your patch *always* turns on TIDSPBRIDGE_WDT3, which is not what we
>> want. Depending on the firmware, some people might want it off.
>
> What firmware?

What do you think is the purpose of tidspbridge?

> Why not document this properly somewhere in the help
> entries?

It is documented (kinda). If you are using a non-standard baseimage,
you should know what is the watchdog timer, and whether or not your
baseimage supports it.

Fortunately, the vast majority of users don't need to care about it.
They can just use the default.

> Why not detect this automatically in the kernel based on the
> firmware version?

There is no firmware version. The "firmware", or rather baseimage, is
an operating system.

You can't detect if Linux has USB support based on the Linux version, can you?

>> Basically, right now on the typical firmware, people have to either
>> manually turn TIDSPBRIDGE_WDT3 on, or they will see the warning.
>
> So, for the "typical" firmware, you do want this on, so the patch makes
> sense.

Yes.

> How about the code be fixed so that it doesn't generate this type of
> warning when using the "typical" firmware, instead of having to rely on
> confusing Kconfig entries.

Patches welcome.

I for one don't know a way, and I'd rather concentrate on one of the
myriad other tasks that need to be done for this driver. I just
thought it would be nice to make it easier for most users to avoid
this warning.

Cheers.

-- 
Felipe Contreras

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Greg KH]

On Fri, Feb 10, 2012 at 06:16:19PM +0200, Felipe Contreras wrote:
> On Fri, Feb 10, 2012 at 7:18 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote:
> >> Hi,
> >>
> >> On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras
> >> <felipe.contreras@gmail.com> wrote:
> >> >> Again, I'm totally confused as to _WHY_ this needs to be y.  What is
> >> >> causing this oops without it?  If an oops is happening, then shouldn't
> >> >> this be a strict dependancy?  Why allow it to be disabled at all if it
> >> >> can break your box if you don't enable it?
> >> >
> >> > It's not an oops, it's a warning, and again, it depends on the
> >> > firmware being used. We don't have control over that, and we have no
> >> > way to detect if this feature is there. It's up to the user.
> >>
> >> I have been thinking more into it, how about looking for a WDT symbol
> >> inside the baseimage to decide whether to turn ON/OFF WDT3, this would
> >> mean that the code is always compiled in, but the decision to turn it
> >> on/off is made at runtime.
> >
> > I totally don't understand, why not just silence the warning properly
> > then?
> 
> The warning doesn't come from the driver, and AFAIK it's a valid
> warning. Now, I haven't explored the code, so I don't know what's
> going on, I just know Omar said this fixes the warning, and that
> there's no easy way around it.
> 
> > I fail to understand why this warning happens,
> 
> I don't know either, I just know enabling TIDSPBRIDGE_WDT3 fixes it.

Ok, I suggest someone get to the root cause of this before we change
anything, although some better Kconfig text would be nice to have to
document the existing issues.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote:
> It's not an oops, it's a warning, and again, it depends on the
> firmware being used. We don't have control over that, and we have no
> way to detect if this feature is there. It's up to the user.

Perhaps just remove the warning message and handle the condition
instead of printing a stack dump?  The user should be triggering
stack dumps.  What on earth?

regards,
dan carpenter

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote:
>> It's not an oops, it's a warning, and again, it depends on the
>> firmware being used. We don't have control over that, and we have no
>> way to detect if this feature is there. It's up to the user.
>
> Perhaps just remove the warning message and handle the condition
> instead of printing a stack dump?  The user should be triggering
> stack dumps.  What on earth?

The warning doesn't come from the driver.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote:
> On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote:
> >> It's not an oops, it's a warning, and again, it depends on the
> >> firmware being used. We don't have control over that, and we have no
> >> way to detect if this feature is there. It's up to the user.
> >
> > Perhaps just remove the warning message and handle the condition
> > instead of printing a stack dump?  The user should be triggering
> > stack dumps.  What on earth?
> 
> The warning doesn't come from the driver.

I'm not sure I understand.  Are you saying that because it comes
from the arch/ directory, it can't be fixed?  I have good news for
you my friend.  :)  It's all open source!  \o/

Anyway, I saw in another email that Omar is working on a fix so
probably we can just wait for his patch, yes?

regards,
dan carpenter

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Fri, Feb 10, 2012 at 10:35 PM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote:
>> On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>> > On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote:
>> >> It's not an oops, it's a warning, and again, it depends on the
>> >> firmware being used. We don't have control over that, and we have no
>> >> way to detect if this feature is there. It's up to the user.
>> >
>> > Perhaps just remove the warning message and handle the condition
>> > instead of printing a stack dump?  The user should be triggering
>> > stack dumps.  What on earth?
>>
>> The warning doesn't come from the driver.
>
> I'm not sure I understand.  Are you saying that because it comes
> from the arch/ directory, it can't be fixed?  I have good news for
> you my friend.  :)  It's all open source!  \o/

The fact that you _can_ remove the warning doesn't mean you should. To
me it sounds like a proper warning.

> Anyway, I saw in another email that Omar is working on a fix so
> probably we can just wait for his patch, yes?

He only proposed a solution, I doubt he is working on. And to me, that
sounded like a hack rather than a proper fix.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Sat, Feb 11, 2012 at 9:19 PM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
>
> On Feb 10, 2012 12:44 PM, "Felipe Contreras" <felipe.contreras@gmail.com>
> wrote:
>>
>> On Fri, Feb 10, 2012 at 10:35 PM, Dan Carpenter
>> <dan.carpenter@oracle.com> wrote:
>> > On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote:
>> >> On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter
>> >> <dan.carpenter@oracle.com> wrote:
>> >> > On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote:
>> >> >> It's not an oops, it's a warning, and again, it depends on the
>> >> >> firmware being used. We don't have control over that, and we have no
>> >> >> way to detect if this feature is there. It's up to the user.
>> >> >
>> >> > Perhaps just remove the warning message and handle the condition
>> >> > instead of printing a stack dump?  The user should be triggering
>> >> > stack dumps.  What on earth?
>> >>
>> >> The warning doesn't come from the driver.
>> >
>> > I'm not sure I understand.  Are you saying that because it comes
>> > from the arch/ directory, it can't be fixed?  I have good news for
>> > you my friend.  :)  It's all open source!  \o/
>>
>> The fact that you _can_ remove the warning doesn't mean you should. To
>> me it sounds like a proper warning.
>>
>> > Anyway, I saw in another email that Omar is working on a fix so
>> > probably we can just wait for his patch, yes?
>>
>> He only proposed a solution, I doubt he is working on. And to me, that
>> sounded like a hack rather than a proper fix.
>
> I'm out on travel but will be able to look at it on Monday.
>
> Well,  I think it is the right way, you look on the firmware if it has WDT
> you use it if it doesn't then you don't. Rather than guessing if you have
> the feature. It would be like reading a config option in the firmware.

Yeah, but it's not really firmware, it's an operating system image. I
can be running Linux there, and I might have implemented WDT. How is
that code going to find that out?

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Ramirez Luna, Omar]

On Feb 11, 2012 3:03 PM, "Felipe Contreras" <felipe.contreras@gmail.com> wrote:
>
> On Sat, Feb 11, 2012 at 9:19 PM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
> >
> > On Feb 10, 2012 12:44 PM, "Felipe Contreras" <felipe.contreras@gmail.com>
> > wrote:
> >>
> >> On Fri, Feb 10, 2012 at 10:35 PM, Dan Carpenter
> >> <dan.carpenter@oracle.com> wrote:
> >> > On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote:
> >> >> On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter
> >> >> <dan.carpenter@oracle.com> wrote:
> >> >> > On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote:
> >> >> >> It's not an oops, it's a warning, and again, it depends on the
> >> >> >> firmware being used. We don't have control over that, and we have no
> >> >> >> way to detect if this feature is there. It's up to the user.
> >> >> >
> >> >> > Perhaps just remove the warning message and handle the condition
> >> >> > instead of printing a stack dump?  The user should be triggering
> >> >> > stack dumps.  What on earth?
> >> >>
> >> >> The warning doesn't come from the driver.
> >> >
> >> > I'm not sure I understand.  Are you saying that because it comes
> >> > from the arch/ directory, it can't be fixed?  I have good news for
> >> > you my friend.  :)  It's all open source!  \o/
> >>
> >> The fact that you _can_ remove the warning doesn't mean you should. To
> >> me it sounds like a proper warning.
> >>
> >> > Anyway, I saw in another email that Omar is working on a fix so
> >> > probably we can just wait for his patch, yes?
> >>
> >> He only proposed a solution, I doubt he is working on. And to me, that
> >> sounded like a hack rather than a proper fix.
> >
> > I'm out on travel but will be able to look at it on Monday.
> >
> > Well,  I think it is the right way, you look on the firmware if it has WDT
> > you use it if it doesn't then you don't. Rather than guessing if you have
> > the feature. It would be like reading a config option in the firmware.
>
> Yeah, but it's not really firmware, it's an operating system image. I
> can be running Linux there, and I might have implemented WDT. How is
> that code going to find that out?

Tidspbridge loader doesn't support that use case, baseimage and let's
say a dsp-linuximg won't have the same layout, hence it won't be able
to parse and load the latter.

When that case is applicable, we should first modify the loader code
or prepare the baseimages to be common so we can get rid of specific
loaders and just dump them into memory.

WDT could be detected by prepending common symbols into the baseimages
or just making the new images to treat all peripherals as resources,
that is, the new baseimg should actually request the wdt3 as any other
clock.

Regards,

Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Ramirez Luna, Omar]

On Thu, Feb 9, 2012 at 9:18 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote:
>> Hi,
>>
>> On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>> >> Again, I'm totally confused as to _WHY_ this needs to be y.  What is
>> >> causing this oops without it?  If an oops is happening, then shouldn't
>> >> this be a strict dependancy?  Why allow it to be disabled at all if it
>> >> can break your box if you don't enable it?
>> >
>> > It's not an oops, it's a warning, and again, it depends on the
>> > firmware being used. We don't have control over that, and we have no
>> > way to detect if this feature is there. It's up to the user.
>>
>> I have been thinking more into it, how about looking for a WDT symbol
>> inside the baseimage to decide whether to turn ON/OFF WDT3, this would
>> mean that the code is always compiled in, but the decision to turn it
>> on/off is made at runtime.
>
> I totally don't understand, why not just silence the warning properly
> then?
>
> I fail to understand why this warning happens, why it depends on the
> firmware, and why you can't detect it at runtime to not do it.  And how
> it all ties into a kconfig option...

Just FTR, the warning comes from an interconnect driver that reports errors.

This specific warning occurs because the dsp tries to access wdt3
registers without a clock, hence turning ON the wdt3 (or default y for
wdt3 Kconfig) will make the warning to disappear when first loading a
base image into the dsp, because now there will be a clock for the
registers.

It depends on the firmware because the accesses are made from the dsp itself.

Regards,

Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

On Tue, Feb 14, 2012 at 3:06 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
> On Feb 11, 2012 3:03 PM, "Felipe Contreras" <felipe.contreras@gmail.com> wrote:
>>
>> On Sat, Feb 11, 2012 at 9:19 PM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
>> >
>> > On Feb 10, 2012 12:44 PM, "Felipe Contreras" <felipe.contreras@gmail.com>
>> > wrote:
>> >>
>> >> On Fri, Feb 10, 2012 at 10:35 PM, Dan Carpenter
>> >> <dan.carpenter@oracle.com> wrote:
>> >> > On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote:
>> >> >> On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter
>> >> >> <dan.carpenter@oracle.com> wrote:
>> >> >> > On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote:
>> >> >> >> It's not an oops, it's a warning, and again, it depends on the
>> >> >> >> firmware being used. We don't have control over that, and we have no
>> >> >> >> way to detect if this feature is there. It's up to the user.
>> >> >> >
>> >> >> > Perhaps just remove the warning message and handle the condition
>> >> >> > instead of printing a stack dump?  The user should be triggering
>> >> >> > stack dumps.  What on earth?
>> >> >>
>> >> >> The warning doesn't come from the driver.
>> >> >
>> >> > I'm not sure I understand.  Are you saying that because it comes
>> >> > from the arch/ directory, it can't be fixed?  I have good news for
>> >> > you my friend.  :)  It's all open source!  \o/
>> >>
>> >> The fact that you _can_ remove the warning doesn't mean you should. To
>> >> me it sounds like a proper warning.
>> >>
>> >> > Anyway, I saw in another email that Omar is working on a fix so
>> >> > probably we can just wait for his patch, yes?
>> >>
>> >> He only proposed a solution, I doubt he is working on. And to me, that
>> >> sounded like a hack rather than a proper fix.
>> >
>> > I'm out on travel but will be able to look at it on Monday.
>> >
>> > Well,  I think it is the right way, you look on the firmware if it has WDT
>> > you use it if it doesn't then you don't. Rather than guessing if you have
>> > the feature. It would be like reading a config option in the firmware.
>>
>> Yeah, but it's not really firmware, it's an operating system image. I
>> can be running Linux there, and I might have implemented WDT. How is
>> that code going to find that out?
>
> Tidspbridge loader doesn't support that use case, baseimage and let's
> say a dsp-linuximg won't have the same layout, hence it won't be able
> to parse and load the latter.
>
> When that case is applicable, we should first modify the loader code
> or prepare the baseimages to be common so we can get rid of specific
> loaders and just dump them into memory.

I'd say the less workarounds, the better.

> WDT could be detected by prepending common symbols into the baseimages
> or just making the new images to treat all peripherals as resources,
> that is, the new baseimg should actually request the wdt3 as any other
> clock.

I see, but if wdt3 is requested as another clock, the Linux ARM side
would still need to know how to threat the WDT.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Ramirez Luna, Omar]

On Tue, Feb 14, 2012 at 10:23 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>> When that case is applicable, we should first modify the loader code
>> or prepare the baseimages to be common so we can get rid of specific
>> loaders and just dump them into memory.
>
> I'd say the less workarounds, the better.

If there are ever more base images compatible with the dsp, I would
say that unifying them into a common format to be dumped in memory
isn't a workaround, and in that process we can get rid of the custom
loader code.

>> WDT could be detected by prepending common symbols into the baseimages
>> or just making the new images to treat all peripherals as resources,
>> that is, the new baseimg should actually request the wdt3 as any other
>> clock.
>
> I see, but if wdt3 is requested as another clock, the Linux ARM side
> would still need to know how to threat the WDT.

Tidspbridge does know how to treat other clock request from dsp (gpt,
mcbsp), it would be a matter of adding the logic in the arm side for
any dsp image that is able to do it, however this doesn't apply to the
current (latest) base image since it assumes the wdt3 is always
controlled by tidspbridge.

Regards,

Omar

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Víctor M. Jáquez L.]

On Tue, Feb 14, 2012 at 08:56:11PM -0600, Ramirez Luna, Omar wrote:
> On Tue, Feb 14, 2012 at 10:23 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >> When that case is applicable, we should first modify the loader code
> >> or prepare the baseimages to be common so we can get rid of specific
> >> loaders and just dump them into memory.
> >
> > I'd say the less workarounds, the better.
> 
> If there are ever more base images compatible with the dsp, I would
> say that unifying them into a common format to be dumped in memory
> isn't a workaround, and in that process we can get rid of the custom
> loader code.

Yes! please! and use Ohad's rproc thingy.

What would be the steps to unify that common format? I guess we will depend on
TI for that... Do we?

vmjl

> 
> >> WDT could be detected by prepending common symbols into the baseimages
> >> or just making the new images to treat all peripherals as resources,
> >> that is, the new baseimg should actually request the wdt3 as any other
> >> clock.
> >
> > I see, but if wdt3 is requested as another clock, the Linux ARM side
> > would still need to know how to threat the WDT.
> 
> Tidspbridge does know how to treat other clock request from dsp (gpt,
> mcbsp), it would be a matter of adding the logic in the arm side for
> any dsp image that is able to do it, however this doesn't apply to the
> current (latest) base image since it assumes the wdt3 is always
> controlled by tidspbridge.
> 
> Regards,
> 
> Omar
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Felipe Contreras]

2012/2/15 Víctor M. Jáquez L. <vjaquez@igalia.com>:
> On Tue, Feb 14, 2012 at 08:56:11PM -0600, Ramirez Luna, Omar wrote:
>> On Tue, Feb 14, 2012 at 10:23 AM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>> >> When that case is applicable, we should first modify the loader code
>> >> or prepare the baseimages to be common so we can get rid of specific
>> >> loaders and just dump them into memory.
>> >
>> > I'd say the less workarounds, the better.
>>
>> If there are ever more base images compatible with the dsp, I would
>> say that unifying them into a common format to be dumped in memory
>> isn't a workaround, and in that process we can get rid of the custom
>> loader code.
>
> Yes! please! and use Ohad's rproc thingy.

I thought rproc is tied to elf for now.

> What would be the steps to unify that common format? I guess we will depend on
> TI for that... Do we?

But this "common" format would be specific for tidspbridge, I don't
think it makes sense for these images to have that constraint.
Certainly rproc doesn't have it, and that one is not on staging.

In any case, the proposed patch looks good. We can deal about these
futuristic situations later on.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Víctor M. Jáquez L.]

On Wed, Feb 15, 2012 at 01:41:19PM +0200, Felipe Contreras wrote:
> 2012/2/15 Víctor M. Jáquez L. <vjaquez@igalia.com>:
> > On Tue, Feb 14, 2012 at 08:56:11PM -0600, Ramirez Luna, Omar wrote:
> >> On Tue, Feb 14, 2012 at 10:23 AM, Felipe Contreras
> >> <felipe.contreras@gmail.com> wrote:
> >> >> When that case is applicable, we should first modify the loader code
> >> >> or prepare the baseimages to be common so we can get rid of specific
> >> >> loaders and just dump them into memory.
> >> >
> >> > I'd say the less workarounds, the better.
> >>
> >> If there are ever more base images compatible with the dsp, I would
> >> say that unifying them into a common format to be dumped in memory
> >> isn't a workaround, and in that process we can get rid of the custom
> >> loader code.
> >
> > Yes! please! and use Ohad's rproc thingy.
> 
> I thought rproc is tied to elf for now.

It is.

> 
> > What would be the steps to unify that common format? I guess we will depend on
> > TI for that... Do we?
> 
> But this "common" format would be specific for tidspbridge, I don't
> think it makes sense for these images to have that constraint.
> Certainly rproc doesn't have it, and that one is not on staging.
> 
> In any case, the proposed patch looks good. We can deal about these
> futuristic situations later on.

I do agree.

vmjl
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: tidspbridge: enable watchdog by default

[Omar Ramirez Luna]

2012/2/15 Víctor M. Jáquez L. <vjaquez@igalia.com>:
> On Tue, Feb 14, 2012 at 08:56:11PM -0600, Ramirez Luna, Omar wrote:
>> On Tue, Feb 14, 2012 at 10:23 AM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>> >> When that case is applicable, we should first modify the loader code
>> >> or prepare the baseimages to be common so we can get rid of specific
>> >> loaders and just dump them into memory.
>> >
>> > I'd say the less workarounds, the better.
>>
>> If there are ever more base images compatible with the dsp, I would
>> say that unifying them into a common format to be dumped in memory
>> isn't a workaround, and in that process we can get rid of the custom
>> loader code.
>
> Yes! please! and use Ohad's rproc thingy.

Yes it would be nice, in theory we could replace some parts with
remoteproc maybe not the loader yet, not sure how much rpmsg will fit.

> What would be the steps to unify that common format? I guess we will depend on
> TI for that... Do we?

I don't know the stance of TI on this.

But basically we have a loader in kernel space that prepares the
baseimage to be dumped in memory, that can be done in user space or
better to distribute a baseimage that just needs to be dumped in
memory, however there might be some constraints that need to be solved
like the tight coupling between the driver and the loader, the current
format of the base image, the size and how it expands to fit in the
6MB of shm, and so on, not to mention the ability to load dynamic
code.

I guess it would be better to dive in the code and see.

Regards,

Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html