Re: [PATCH] DSPBRIDGE: MMU-Fault debugging enhancements
On Wed, May 5, 2010 at 2:58 PM, Felipe Contreras wrote: > Hey Ernesto, > > On Fri, Apr 30, 2010 at 09:24:15PM +0200, ext Ramos Falcon, Ernesto wrote: >> >This patch seems to be doing a lot of things. Couldn't it have been >> >split? >> > >> >Also, from the commit message it seems to implement a new feature, >> >however, I heard it's supposed to fix memory corruption too. Is that >> >true? If that's the case the code that fixes that would have to be >> >separate. >> > >> >I understand this patch was already pushed to dspbridge branch, but I >> >think such important changes should be properly recorded in the >> >history. >> >> I agree important changes must be properly recorded but in this case >> the patch introduces only one new feature and because of the way that >> it is implemented using gpt8 overflow interrupt instead of mailbox to >> inform about the MMU Fault, the problem of the memory corruption was >> fixed indirectly, however these changes are part of the feature design >> itself and I don't see the need to split this new feature. > > In general, logically independent changes should end up as separate > patches. Many times it looks like a patch cannot be split further, but > with a little bit of creativity it usually can. > > I can think of one patch that switches to gpt8 overflow, and another one > that actually shows the extra information. > > Anyway, I ask because we found some issues with the latest commits of > dspbridge, and I would like to isolate the memory corruption fix just to > be safe. Disregard that. This patch doesn't actually fix the corruption. I sent one proposal for a patch that does. > Also, from that description it looks like there might be still a problem > in the mailbox that is hidded by this patch... so it's more like a > workaround, not really a fix. I've extracted the GPT8 related changes, which are good in themselves, and I sent a separate patch for reference. I also cleaned the patch a bit. Now I wonder if there are some situations where we might not want to use GPT8, but the mailbox interrupt. Maybe some baseimages are not using GPT8, or maybe the system doesn't have OMAP_DM_TIMER. Also, all this MMU fault backtracing and so on seems to consume quite a bit of code. Production systems most likely would want to have this off (specially when constrained by size), so it should be a configuration option. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] DSPBRIDGE: MMU-Fault debugging enhancements
Hey Ernesto, On Fri, Apr 30, 2010 at 09:24:15PM +0200, ext Ramos Falcon, Ernesto wrote: > >This patch seems to be doing a lot of things. Couldn't it have been > >split? > > > >Also, from the commit message it seems to implement a new feature, > >however, I heard it's supposed to fix memory corruption too. Is that > >true? If that's the case the code that fixes that would have to be > >separate. > > > >I understand this patch was already pushed to dspbridge branch, but I > >think such important changes should be properly recorded in the > >history. > > I agree important changes must be properly recorded but in this case > the patch introduces only one new feature and because of the way that > it is implemented using gpt8 overflow interrupt instead of mailbox to > inform about the MMU Fault, the problem of the memory corruption was > fixed indirectly, however these changes are part of the feature design > itself and I don't see the need to split this new feature. In general, logically independent changes should end up as separate patches. Many times it looks like a patch cannot be split further, but with a little bit of creativity it usually can. I can think of one patch that switches to gpt8 overflow, and another one that actually shows the extra information. Anyway, I ask because we found some issues with the latest commits of dspbridge, and I would like to isolate the memory corruption fix just to be safe. Also, from that description it looks like there might be still a problem in the mailbox that is hidded by this patch... so it's more like a workaround, not really a fix. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] DSPBRIDGE: MMU-Fault debugging enhancements
Hi, >-Original Message- >From: Felipe Contreras [mailto:felipe.contre...@gmail.com] >Sent: Friday, April 30, 2010 10:21 AM >To: Guzman Lugo, Fernando; Ramos Falcon, Ernesto >Cc: linux-omap@vger.kernel.org; Hiroshi DOYU; Ameya Palande; >felipe.contre...@nokia.com >Subject: Re: [PATCH] DSPBRIDGE: MMU-Fault debugging enhancements > >On Fri, Apr 9, 2010 at 3:15 AM, Guzman Lugo, Fernando >wrote: >> From db3d76a2e89a1c227322a2732ddf7ebf5cd4b4cf Mon Sep 17 00:00:00 2001 >> From: Ernesto Ramos >> Date: Wed, 24 Mar 2010 11:12:05 -0600 >> Subject: [PATCH] DSPBRIDGE: MMU-Fault debugging enhancements >> >> These changes allow for DSP task information to be printed >> by the MPU dspbridge when DSP MMU fault ocurrs. >> >> Signed-off-by: Cris Jansson >> [change to open source coding style] >> Signed-off-by: Ernesto Ramos > >This patch seems to be doing a lot of things. Couldn't it have been split? > >Also, from the commit message it seems to implement a new feature, >however, I heard it's supposed to fix memory corruption too. Is that >true? If that's the case the code that fixes that would have to be >separate. > >I understand this patch was already pushed to dspbridge branch, but I >think such important changes should be properly recorded in the >history. > >Cheers. > Hi Felipe, I agree important changes must be properly recorded but in this case the patch introduces only one new feature and because of the way that it is implemented using gpt8 overflow interrupt instead of mailbox to inform about the MMU Fault, the problem of the memory corruption was fixed indirectly, however these changes are part of the feature design itself and I don't see the need to split this new feature. Saludos, Ernesto >-- >Felipe Contreras-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] DSPBRIDGE: MMU-Fault debugging enhancements
On Fri, Apr 9, 2010 at 3:15 AM, Guzman Lugo, Fernando wrote: > From db3d76a2e89a1c227322a2732ddf7ebf5cd4b4cf Mon Sep 17 00:00:00 2001 > From: Ernesto Ramos > Date: Wed, 24 Mar 2010 11:12:05 -0600 > Subject: [PATCH] DSPBRIDGE: MMU-Fault debugging enhancements > > These changes allow for DSP task information to be printed > by the MPU dspbridge when DSP MMU fault ocurrs. > > Signed-off-by: Cris Jansson > [change to open source coding style] > Signed-off-by: Ernesto Ramos This patch seems to be doing a lot of things. Couldn't it have been split? Also, from the commit message it seems to implement a new feature, however, I heard it's supposed to fix memory corruption too. Is that true? If that's the case the code that fixes that would have to be separate. I understand this patch was already pushed to dspbridge branch, but I think such important changes should be properly recorded in the history. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] DSPBRIDGE: MMU-Fault debugging enhancements
> >From db3d76a2e89a1c227322a2732ddf7ebf5cd4b4cf Mon Sep 17 00:00:00 2001 >From: Ernesto Ramos >Date: Wed, 24 Mar 2010 11:12:05 -0600 >Subject: [PATCH] DSPBRIDGE: MMU-Fault debugging enhancements > >These changes allow for DSP task information to be printed >by the MPU dspbridge when DSP MMU fault ocurrs. > >Signed-off-by: Cris Jansson >[change to open source coding style] >Signed-off-by: Ernesto Ramos >--- Pushed to dspbridge. - omar -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH] DSPBRIDGE: MMU-Fault debugging enhancements
Hi Ernesto, On Fri, 2010-03-05 at 01:39 +0100, ext Ramos Falcon, Ernesto wrote: > From 1d7f5d634176be558d5a60e26ee2bdeddffd7d3b Mon Sep 17 00:00:00 2001 > From: Cris Jansson > Date: Thu, 25 Feb 2010 11:38:23 -0600 > Subject: [PATCH] DSPBRIDGE: MMU-Fault debugging enhancements > > These changes allow for DSP task information to be printed > by the MPU dspbridge when DSP MMU fault ocurrs. > > Signed-off-by: Cris Jansson > [change to open soutce coding style] source > Signed-off-by: Ernesto Ramos > --- > arch/arm/plat-omap/include/dspbridge/dbll.h |3 + > arch/arm/plat-omap/include/dspbridge/gh.h|3 + > arch/arm/plat-omap/include/dspbridge/io_sm.h |5 + > arch/arm/plat-omap/include/dspbridge/nldr.h |3 + > arch/arm/plat-omap/include/dspbridge/node.h | 12 + > drivers/dsp/bridge/Makefile |1 + > drivers/dsp/bridge/gen/gh.c | 22 ++ > drivers/dsp/bridge/pmgr/dbll.c | 62 +++ > drivers/dsp/bridge/rmgr/nldr.c | 71 > drivers/dsp/bridge/rmgr/node.c | 31 ++ > drivers/dsp/bridge/wmd/io_sm.c | 515 > -- > drivers/dsp/bridge/wmd/ue_deh.c | 67 +++- > 12 files changed, 680 insertions(+), 115 deletions(-) > + > +/* > + * node_find_addr > + * Purpose: > + * Find the closest symbol to the given address. > + * Parameters: > + * > + */ Since you are adhering to open source coding style why not use linux kernel commenting style at all places in patch ;) Cheers, Ameya. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
