From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nishanth Menon Subject: Re: [PATCHv2] DSPBRIDGE: Include missing info in MMU Fault debugging trace Date: Thu, 13 May 2010 10:49:01 -0500 Message-ID: <4BEC1F6D.5020408@ti.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from comal.ext.ti.com ([198.47.26.152]:58058 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758971Ab0EMPtJ (ORCPT ); Thu, 13 May 2010 11:49:09 -0400 In-Reply-To: Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: "Ramos Falcon, Ernesto" Cc: "linux-omap@vger.kernel.org" , Ameya Palande , "Contreras Felipe (Nokia-D/Helsinki)" Ramos Falcon, Ernesto had written, on 05/13/2010 10:40 AM, the following: > Resending this patch rebased to the last changes included in dspbridge. > --- > > Some execution info is missing at the beginning of the > trace buffer printout. This fix is to include this information > according to the buffer contents description and > also couple of cosmetic changes to make the traces more > readable. > > Signed-off-by: Ernesto Ramos > --- > drivers/dsp/bridge/core/io_sm.c | 44 +++++++++++++++++++++++++++++++------- > 1 files changed, 36 insertions(+), 8 deletions(-) > > diff --git a/drivers/dsp/bridge/core/io_sm.c b/drivers/dsp/bridge/core/io_sm.c > index d6c1a98..d11bd9f 100644 > --- a/drivers/dsp/bridge/core/io_sm.c > +++ b/drivers/dsp/bridge/core/io_sm.c > @@ -1949,9 +1949,9 @@ dsp_status print_dsp_trace_buffer(struct bridge_dev_context *hbridge_context) > if (DSP_FAILED(status)) > goto func_end; > /* Pack and do newline conversion */ > - pr_info("%s: DSP Trace Buffer Begin:\n" > + pr_info("DSP Trace Buffer Begin:\n" > "=======================\n%s\n", > - __func__, psz_buf); > + psz_buf); > > > /* convert to offset */ > @@ -2058,6 +2058,7 @@ dsp_status dump_dsp_stack(struct bridge_dev_context *bridge_context) > u32 size; > } mmu_fault_dbg_info; > u32 *buffer; > + u32 *buffer_beg; > u32 *buffer_end; > u32 exc_type; > u32 i; > @@ -2067,6 +2068,7 @@ dsp_status dump_dsp_stack(struct bridge_dev_context *bridge_context) > const char *dsp_regs[] = {"EFR", "IERR", "ITSR", "NTSR", > "IRP", "NRP", "AMR", "SSR", > "ILC", "RILC", "IER", "CSR"}; > + const char *exec_ctxt[] = {"Task", "SWI", "HWI", "Unknown"}; > struct bridge_drv_interface *intf_fxns; > struct dev_object *dev_object = bridge_context->hdev_obj; > > @@ -2136,6 +2138,7 @@ dsp_status dump_dsp_stack(struct bridge_dev_context *bridge_context) > total_size = MAX_MMU_DBGBUFF; > > buffer = kzalloc(total_size, GFP_ATOMIC); > + buffer_beg = buffer; > buffer_end = buffer + total_size / 4; > > if (!buffer) { > @@ -2155,7 +2158,7 @@ dsp_status dump_dsp_stack(struct bridge_dev_context *bridge_context) > goto func_end; > } > > - pr_err("Aproximate Crash Position:\n"); > + pr_err("\nAproximate Crash Position:\n"); > pr_err("--------------------------\n"); > > exc_type = buffer[3]; > @@ -2171,7 +2174,27 @@ dsp_status dump_dsp_stack(struct bridge_dev_context *bridge_context) > else > pr_err("0x%-8x [Unable to match to a symbol.]\n", i); > > - pr_err("Execution Info:\n"); > + buffer += 4; > + > + pr_err("\nExecution Info:\n"); > + pr_err("---------------\n"); > + > + if (*buffer < ARRAY_SIZE(exec_ctxt)) { > + pr_err("Execution context \t%s\n", > + exec_ctxt[*buffer++]); > + } else { > + pr_err("Execution context corrupt\n"); > + kfree(buffer_beg); > + return -EFAULT; > + } > + pr_err("Task Handle\t\t0x%x\n", *buffer++); > + pr_err("Stack Pointer\t\t0x%x\n", *buffer++); > + pr_err("Stack Top\t\t0x%x\n", *buffer++); > + pr_err("Stack Bottom\t\t0x%x\n", *buffer++); > + pr_err("Stack Size\t\t0x%x\n", *buffer++); > + pr_err("Stack Size In Use\t0x%x\n", *buffer++); > + How about casting buffer to a structure and using it? the use of buffer++ etc is a nightmare as can be seen here.. > + pr_err("\nCPU Registers\n"); > pr_err("---------------\n"); btw, linux allows you to use string in a single pr_err.. e.g. pr_err("\n CPU Registers\n" "----------------\n"); it is a bit easier on the system without going thru two function call overhead > > for (i = 0; i < 32; i++) { > @@ -2205,16 +2228,21 @@ dsp_status dump_dsp_stack(struct bridge_dev_context *bridge_context) > if (i == 4 || i == 6 || i == 8) > pr_err("B%d 0x%-8x [Function Argument %d]\n", > i, *buffer++, i-2); > - else if (i == 15) > + else if (i == 14) > pr_err("B14 0x%-8x [Data Page Pointer]\n", > *buffer++); > else > pr_err("B%d 0x%x\n", i, *buffer++); > } > > + pr_err("\n"); > + > for (i = 0; i < ARRAY_SIZE(dsp_regs); i++) > pr_err("%s 0x%x\n", dsp_regs[i], *buffer++); > > + pr_err("\nStack:\n"); > + pr_err("------\n"); > + > for (i = 0; buffer < buffer_end; i++, buffer++) { > if ((*buffer > 0x01000000) && (node_find_addr(node_mgr, > *buffer , 0x600, &offset_output, name) == > @@ -2225,7 +2253,7 @@ dsp_status dump_dsp_stack(struct bridge_dev_context *bridge_context) > else > pr_err("[%d] 0x%x\n", i, *buffer); > } > - kfree(buffer - total_size / 4); > + kfree(buffer_beg); > } > func_end: > return status; > @@ -2290,8 +2318,8 @@ void dump_dl_modules(struct bridge_dev_context *bridge_context) > pr_debug("%s: dll_module_header 0x%x %d\n", __func__, module_dsp_addr, > module_size); > > - pr_err("%s: \nDynamically Loaded Modules:\n" > - "---------------------------\n", __func__); > + pr_err("\nDynamically Loaded Modules:\n" > + "---------------------------\n"); > > /* For each dll_module structure in the list... */ > while (module_size) { -- Regards, Nishanth Menon