Messages in this thread | | | Date | Mon, 30 Apr 2012 09:47:17 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers |
| |
On Sun, Apr 29, 2012 at 02:39:04PM -0300, Mauro Carvalho Chehab wrote: > Em 29-04-2012 13:43, Joe Perches escreveu: > > On Sun, 2012-04-29 at 13:20 -0300, Mauro Carvalho Chehab wrote: > >> The script below is even better. After that, only 113 occurrences of __func__ > >> is now found at drivers/edac, and some of them are not related to debugf[1-9], > >> so they shouldn't be cover on a patch like that. > >> I'll do some manual cleanup on it. > > > > Hi Mauro. > > > > Another thing you could do would be to > > separate the level from the multiple macros, > > use a single macro, and convert the uses. > > > > #define debugf(level, fmt, ...) > > and change the uses to > > debugf([0-n], "some format", args...) > > > > I believe that's the more predominate > > kernel style for debugging macros with > > a tested level or mask. > > Agreed. > > > Perhaps also add !CONFIG_EDAC_DEBUG > > format/args checking to the debug statements. > > Most/all debug-only stuff are already checking for CONFIG_EDAC_DEBUG. > There are a few static debug-only data/functions that aren't testing for > it, but the compiler should remove the dead code anyway, so this shouldn't > cause any harm. > > > Lastly, indenting the messages 2 tabs isn't > > really useful, one or two spaces is probably > > enough. > > agreed. > > > > > I did this a bit ago so it may not apply > > after your changes: > > Believe or not, it applied without troubles ;) > > I've added at the end of my experimental series, at: > > > git://git.infradead.org/users/mchehab/edac.git experimental > > be careful if you use this branch, as I'm rebasing it every time I need > to change something on this series. > > I'm keeping a non-rebased version, with one branch per review, at: > > git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-edac.git > > The current review is at hw_events_v17. Patches were already pushed there. > they should be there after the usual kernel.org master/mirror replication > delay.
Now wait a minute,
you guys are so trigger-happy to apply humongous, cleanup patches but let me ask this: can anyone of you really test those changes with each driver? Do you have all the hardware that those patches touch?
I know, I know, it builds fine and it looks correct but subtle bugs tend to sneak in in exactly such situations.
-- Regards/Gruss, Boris.
Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551
| |