[PATCH] tty/serial/pmac_zilog: Fix "nobody cared" IRQ message
Following commit a79dd5a titled "tty/serial/pmac_zilog: Fix suspend & resume",
my Powerbook G4 Titanium showed the following stack dump:
[ 36.878225] irq 23: nobody cared (try booting with the "irqpoll" option)
[ 36.878251] Call Trace:
[ 36.878291] [dfff3f00] [c000984c] show_stack+0x7c/0x194 (unreliable)
[ 36.878322] [dfff3f40] [c00a6868] __report_bad_irq+0x44/0xf4
[ 36.878339] [dfff3f60] [c00a6b04] note_interrupt+0x1ec/0x2ac
[ 36.878356] [dfff3f80] [c00a48d0] handle_irq_event_percpu+0x250/0x2b8
[ 36.878372] [dfff3fd0] [c00a496c] handle_irq_event+0x34/0x54
[ 36.878389] [dfff3fe0] [c00a753c] handle_fasteoi_irq+0xb4/0x124
[ 36.878412] [dfff3ff0] [c000f5bc] call_handle_irq+0x18/0x28
[ 36.878428] [deef1f10] [c000719c] do_IRQ+0x114/0x1cc
[ 36.878446] [deef1f40] [c0015868] ret_from_except+0x0/0x1c
[ 36.878484] --- Exception: 501 at 0xf497610
[ 36.878489] LR = 0xfdc3dd0
[ 36.878497] handlers:
[ 36.878510] [] pmz_interrupt
[ 36.878520] Disabling IRQ #23
From an E-mail exchange about this problem, Andreas Schwab noticed a typo
that resulted in the wrong condition being tested.
The patch also corrects 2 typos that incorrectly report why an error branch
is being taken.
Signed-off-by: Larry Finger
---
Ben,
Any changes you wish to make in the commit message are OK with me.
Larry
---
Index: wireless-testing/drivers/tty/serial/pmac_zilog.c
===
--- wireless-testing.orig/drivers/tty/serial/pmac_zilog.c 2012-04-28
15:51:38.843723074 -0500
+++ wireless-testing/drivers/tty/serial/pmac_zilog.c 2012-04-28
18:34:34.053900600 -0500
@@ -469,7 +469,7 @@
tty = NULL;
if (r3 & (CHAEXT | CHATxIP | CHARxIP)) {
if (!ZS_IS_OPEN(uap_a)) {
- pmz_debug("ChanA interrupt while open !\n");
+ pmz_debug("ChanA interrupt while not open !\n");
goto skip_a;
}
write_zsreg(uap_a, R0, RES_H_IUS);
@@ -493,8 +493,8 @@
spin_lock(&uap_b->port.lock);
tty = NULL;
if (r3 & (CHBEXT | CHBTxIP | CHBRxIP)) {
- if (!ZS_IS_OPEN(uap_a)) {
- pmz_debug("ChanB interrupt while open !\n");
+ if (!ZS_IS_OPEN(uap_b)) {
+ pmz_debug("ChanB interrupt while not open !\n");
goto skip_b;
}
write_zsreg(uap_b, R0, RES_H_IUS);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Regression in 32-bit ppc kernel
On 04/28/2012 06:23 PM, Benjamin Herrenschmidt wrote: On Sat, 2012-04-28 at 18:17 -0500, Larry Finger wrote: Yes, good catch by Andreas. That change does fix the problem. Ben - Do you want to fix the typos for open/not open with the same patch? Sure, if you're going to do a proper patch, by all means please fix those too :-) Does it fix all the occurrences of the problem for you ? Yes. After the patch is applied, no more "nobody cared" IRQ messages. I will prepare the patch and send it to you. Larry ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Regression in 32-bit ppc kernel
On Sat, 2012-04-28 at 18:17 -0500, Larry Finger wrote: > Yes, good catch by Andreas. That change does fix the problem. > > Ben - Do you want to fix the typos for open/not open with the same > patch? > Sure, if you're going to do a proper patch, by all means please fix those too :-) Does it fix all the occurrences of the problem for you ? Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Regression in 32-bit ppc kernel
On 04/28/2012 05:48 PM, Benjamin Herrenschmidt wrote:
On Sat, 2012-04-28 at 20:23 +0200, Andreas Schwab wrote:
Larry Finger writes:
I have done a little more debugging. The problem is definitely coming from
drivers/tty/serial/pmac_zilog.c. I am getting ChanB interrupts while open,
which causes the following code segment to return IRQ_NONE:
if (r3& (CHBEXT | CHBTxIP | CHBRxIP)) {
if (!ZS_IS_OPEN(uap_a)) {
s/uap_a/uap_b/?
Good catch... Let's see if that fixes it for Larry...
Yes, good catch by Andreas. That change does fix the problem.
Ben - Do you want to fix the typos for open/not open with the same patch?
Larry
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Regression in 32-bit ppc kernel
On Sun, 2012-04-29 at 08:41 +1000, Benjamin Herrenschmidt wrote:
> On Sat, 2012-04-28 at 13:09 -0500, Larry Finger wrote:
> > I have done a little more debugging. The problem is definitely coming
> > from
> > drivers/tty/serial/pmac_zilog.c. I am getting ChanB interrupts while
> > open, which
> > causes the following code segment to return IRQ_NONE:
> >
> > if (r3 & (CHBEXT | CHBTxIP | CHBRxIP)) {
> > if (!ZS_IS_OPEN(uap_a)) {
> > pmz_debug("ChanB interrupt while open !\n");
> > goto skip_b;
> > }
> > write_zsreg(uap_b, R0, RES_H_IUS);
> > zssync(uap_b);
> > if (r3 & CHBEXT)
> >
> > When this section is entered, r3 == 0x2 (CHBTxIP).
> >
> >
> Ok. The debug code was meant to spell "while not open" btw :-)
>
> I have some ideas what's going on. I think the irda stuff can trigger
> interrupts during the open/close sequence before ZS_IS_OPEN is true.
>
> I'll send a fix.
Hrm, actually, Andreas also found an actual bug here, as we aren't
testing uap_b but uap_a ... oops. I think when I tested chan b I always
had chan a open :-) That will be easy to fix.
Can you try turning the uap_a to uap_b test above and see if that fixes
some of it for you ?
Cheers,
Ben.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Regression in 32-bit ppc kernel
On Sat, 2012-04-28 at 20:23 +0200, Andreas Schwab wrote:
> Larry Finger writes:
>
> > I have done a little more debugging. The problem is definitely coming from
> > drivers/tty/serial/pmac_zilog.c. I am getting ChanB interrupts while open,
> > which causes the following code segment to return IRQ_NONE:
> >
> >if (r3 & (CHBEXT | CHBTxIP | CHBRxIP)) {
> >if (!ZS_IS_OPEN(uap_a)) {
>
> s/uap_a/uap_b/?
Good catch... Let's see if that fixes it for Larry...
Cheers,
Ben.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Regression in 32-bit ppc kernel
On Sat, 2012-04-28 at 13:09 -0500, Larry Finger wrote:
> I have done a little more debugging. The problem is definitely coming
> from
> drivers/tty/serial/pmac_zilog.c. I am getting ChanB interrupts while
> open, which
> causes the following code segment to return IRQ_NONE:
>
> if (r3 & (CHBEXT | CHBTxIP | CHBRxIP)) {
> if (!ZS_IS_OPEN(uap_a)) {
> pmz_debug("ChanB interrupt while open !\n");
> goto skip_b;
> }
> write_zsreg(uap_b, R0, RES_H_IUS);
> zssync(uap_b);
> if (r3 & CHBEXT)
>
> When this section is entered, r3 == 0x2 (CHBTxIP).
>
>
Ok. The debug code was meant to spell "while not open" btw :-)
I have some ideas what's going on. I think the irda stuff can trigger
interrupts during the open/close sequence before ZS_IS_OPEN is true.
I'll send a fix.
Cheers,
Ben.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers
On Sat, 2012-04-28 at 10:52 +0200, Borislav Petkov wrote: > On Fri, Apr 27, 2012 at 01:07:38PM -0300, Mauro Carvalho Chehab wrote: > > Yes. This is a common issue at the EDAC core: on several places, it calls > > the > > edac debug macros (DEBUGF0...DEBUGF4) passing a __func__ as an argument, > > while > > the debug macros already handles that. I suspect that, in the past, the > > __func__ > > were not at the macros, but some patch added it there, and forgot to fix the > > occurrences of its call. > > The patch that added it is d357cbb445208 and you reviewed it. > > > This is something that needs to be reviewed at the entire EDAC core (and > > likely > > at the drivers). > > Looks like a job for a newbie to get her/his feet wet with kernel work. Looks to me more like a lazy maintainer/developer who doesn't want to bother with a few minutes work. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Regression in 32-bit ppc kernel
Larry Finger writes:
> I have done a little more debugging. The problem is definitely coming from
> drivers/tty/serial/pmac_zilog.c. I am getting ChanB interrupts while open,
> which causes the following code segment to return IRQ_NONE:
>
>if (r3 & (CHBEXT | CHBTxIP | CHBRxIP)) {
>if (!ZS_IS_OPEN(uap_a)) {
s/uap_a/uap_b/?
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Regression in 32-bit ppc kernel
On 04/27/2012 07:42 PM, Benjamin Herrenschmidt wrote:
Ok, so you do have a serial port, probably two even :-) One of them is
connected to the infra red transceiver and the other one is probably
connected to the internal modem.
(The modem itself might not use it, some of these machines use an
i2s/i2c modem, some use a usb modem, but the serial port is wired to the
connector regardless).
I have done a little more debugging. The problem is definitely coming from
drivers/tty/serial/pmac_zilog.c. I am getting ChanB interrupts while open, which
causes the following code segment to return IRQ_NONE:
if (r3 & (CHBEXT | CHBTxIP | CHBRxIP)) {
if (!ZS_IS_OPEN(uap_a)) {
pmz_debug("ChanB interrupt while open !\n");
goto skip_b;
}
write_zsreg(uap_b, R0, RES_H_IUS);
zssync(uap_b);
if (r3 & CHBEXT)
When this section is entered, r3 == 0x2 (CHBTxIP).
Larry
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers
On Sat, 2012-04-28 at 11:16 +0200, Borislav Petkov wrote: > On Fri, Apr 27, 2012 at 02:52:35PM -0300, Mauro Carvalho Chehab wrote: > > > All those local variables should be sorted in a reverse christmas tree > > > order: > > > > > > u32 this_is_the_longest_array_name[LENGTH]; > > > void *shorter_named_variable; > > > unsigned long size; > > > int i; > > > > > > ... > > > > Why? There's nothing at the CodingStyle saying about how the vars should > > be ordered. If you want to enforce some particular order, please do it > > yourself, but apply it consistently among the entire subsystem. > > First of all, this way it is more readable. Not in my opinion, and blindly using "reverse christmas tree" can separate variables that should be declared together. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers
On Fri, Apr 27, 2012 at 02:52:35PM -0300, Mauro Carvalho Chehab wrote:
[..]
> >> +struct mem_ctl_info *new_edac_mc_alloc(unsigned edac_index,
> >> + unsigned n_layers,
> >> + struct edac_mc_layer *layers,
> >> + bool rev_order,
> >> + unsigned sz_pvt)
> >
> > strange function argument vertical alignment
> >
> >> {
> >>void *ptr = NULL;
> >>struct mem_ctl_info *mci;
> >> - struct csrow_info *csi, *csrow;
> >> + struct edac_mc_layer *lay;
> >
> > As before, call this "layers" pls.
> >
> >> + struct csrow_info *csi, *csr;
> >>struct rank_info *chi, *chp, *chan;
> >>struct dimm_info *dimm;
> >> + u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
> >>void *pvt;
> >> - unsigned size;
> >> - int row, chn;
> >> + unsigned size, tot_dimms, count, pos[EDAC_MAX_LAYERS];
> >> + unsigned tot_csrows, tot_cschannels;
> >
> > No need to call this "tot_cschannels" - "tot_channels" should be enough.
> >
> >> + int i, j;
> >>int err;
> >> + int row, chn;
> >
> > All those local variables should be sorted in a reverse christmas tree
> > order:
> >
> > u32 this_is_the_longest_array_name[LENGTH];
> > void *shorter_named_variable;
> > unsigned long size;
> > int i;
> >
> > ...
>
> Why? There's nothing at the CodingStyle saying about how the vars should
> be ordered. If you want to enforce some particular order, please do it
> yourself, but apply it consistently among the entire subsystem.
First of all, this way it is more readable. Second of all, maybe we
should hold it down in CodingStyle. Third of all, you touch this code so you
could fix it up to be more readable while you're at it.
> >> +
> >> + BUG_ON(n_layers > EDAC_MAX_LAYERS);
> >
> >
> > Push this BUG_ON up into edac_mc_alloc as the first thing this function
> > does.
>
> It is already the first thing at the function.
Ah, that happens later in the patchset where you rename it back to
edac_mc_alloc, ok.
> > Also, is it valid to have n_layers == 0? The memcpy call below
> > will do nothing.
>
> Changed to:
> BUG_ON(n_layers > EDAC_MAX_LAYERS || n_layers == 0);
Really? Look below.
>
> >> + /*
> >> + * Calculate the total amount of dimms and csrows/cschannels while
> >> + * in the old API emulation mode
> >> + */
> >> + tot_dimms = 1;
> >> + tot_cschannels = 1;
> >> + tot_csrows = 1;
> >
> > Those initializations can be done above at variable declaration time.
>
> Yes, but the compiled code will be the same anyway, as gcc will optimize
Hey, are you looking at compiled code or at source code? Because I'm
looking at source code, and it is a pretty safe bet the majority of the
people here do that too.
> it, either by using registers for those vars or by moving the initialization
> to the top of the function.
>
> This function is too complex, so it is better to initialize those vars
> just before the loops that are calculating those totals.
Simply initialize those variables at declaration time and that's it.
Initializing them before the loop doesn't make the function less complex
- splitting it and sanitizing it does.
> >
> >> + for (i = 0; i < n_layers; i++) {
> >> + tot_dimms *= layers[i].size;
> >> + if (layers[i].is_virt_csrow)
> >> + tot_csrows *= layers[i].size;
> >> + else
> >> + tot_cschannels *= layers[i].size;
> >> + }
[..]
> -struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
> - unsigned nr_chans, int edac_index)
> +struct mem_ctl_info *new_edac_mc_alloc(unsigned edac_index,
> +unsigned n_layers,
> +struct edac_mc_layer *layers,
> +bool rev_order,
> +unsigned sz_pvt)
> {
> void *ptr = NULL;
> struct mem_ctl_info *mci;
> - struct csrow_info *csi, *csrow;
> + struct edac_mc_layer *layer;
> + struct csrow_info *csi, *csr;
> struct rank_info *chi, *chp, *chan;
> struct dimm_info *dimm;
> + u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
> void *pvt;
> - unsigned size;
> - int row, chn;
> + unsigned size, tot_dimms, count, pos[EDAC_MAX_LAYERS];
> + unsigned tot_csrows, tot_channels, tot_errcount = 0;
> + int i, j;
> int err;
> + int row, chn;
> + bool per_rank = false;
> +
> + BUG_ON(n_layers > EDAC_MAX_LAYERS);
^^
--
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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers
On Fri, Apr 27, 2012 at 12:36:12PM -0300, Mauro Carvalho Chehab wrote: > The fix for it were in another patch[1], as calling them as "rank" is > needed also at the sysfs API. No, this doesn't fix it either: [ 10.486440] EDAC MC: DCT0 chip selects: [ 10.486443] EDAC amd64: MC: 0: 2048MB 1: 2048MB [ 10.486445] EDAC amd64: MC: 2: 2048MB 3: 2048MB [ 10.486448] EDAC amd64: MC: 4: 0MB 5: 0MB [ 10.486450] EDAC amd64: MC: 6: 0MB 7: 0MB [ 10.486453] EDAC DEBUG: amd64_debug_display_dimm_sizes: F2x180 (DRAM Bank Address Mapping): 0x0088 [ 10.486455] EDAC MC: DCT1 chip selects: [ 10.486458] EDAC amd64: MC: 0: 2048MB 1: 2048MB [ 10.486460] EDAC amd64: MC: 2: 2048MB 3: 2048MB [ 10.486463] EDAC amd64: MC: 4: 0MB 5: 0MB [ 10.486465] EDAC amd64: MC: 6: 0MB 7: 0MB [ 10.486467] EDAC amd64: using x8 syndromes. [ 10.486469] EDAC DEBUG: amd64_dump_dramcfg_low: F2x190 (DRAM Cfg Low): 0x00083100 [ 10.486472] EDAC DEBUG: amd64_dump_dramcfg_low: DIMM type: buffered; all DIMMs support ECC: yes [ 10.486475] EDAC DEBUG: amd64_dump_dramcfg_low: PAR/ERR parity: enabled [ 10.486478] EDAC DEBUG: amd64_dump_dramcfg_low: DCT 128bit mode width: 64b [ 10.486481] EDAC DEBUG: amd64_dump_dramcfg_low: x4 logical DIMMs present: L0: yes L1: yes L2: no L3: no [ 10.486485] EDAC DEBUG: f1x_early_channel_count: Data width is not 128 bits - need more decoding [ 10.486488] EDAC amd64: MCT channel count: 2 [ 10.486493] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc(): allocating 3692 bytes for mci data (16 ranks, 16 csrows/channels) [ 10.486501] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 0: rank0 (0:0:0): row 0, chan 0 [ 10.486506] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 1: rank1 (0:1:0): row 0, chan 1 [ 10.486510] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 2: rank2 (1:0:0): row 1, chan 0 [ 10.486514] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 3: rank3 (1:1:0): row 1, chan 1 [ 10.486518] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 4: rank4 (2:0:0): row 2, chan 0 [ 10.486522] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 5: rank5 (2:1:0): row 2, chan 1 [ 10.486526] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 6: rank6 (3:0:0): row 3, chan 0 [ 10.486530] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 7: rank7 (3:1:0): row 3, chan 1 [ 10.486534] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 8: rank8 (4:0:0): row 4, chan 0 [ 10.486538] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 9: rank9 (4:1:0): row 4, chan 1 [ 10.486542] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 10: rank10 (5:0:0): row 5, chan 0 [ 10.486546] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 11: rank11 (5:1:0): row 5, chan 1 [ 10.486550] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 12: rank12 (6:0:0): row 6, chan 0 [ 10.486554] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 13: rank13 (6:1:0): row 6, chan 1 [ 10.486558] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 14: rank14 (7:0:0): row 7, chan 0 [ 10.486562] EDAC DEBUG: new_edac_mc_alloc: new_edac_mc_alloc: 15: rank15 (7:1:0): row 7, chan 1 DCT0 has 4 ranks + DCT1 also 4 ranks = 8 ranks total. Now your change is showing 16 ranks. Still b0rked. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers
On Fri, Apr 27, 2012 at 04:24:28PM -0300, Mauro Carvalho Chehab wrote:
> Em 27-04-2012 15:11, Luck, Tony escreveu:
> +for (i = 0; i < dimm->mci->n_layers; i++) {
> +printk(KERN_CONT "%d", dimm->location[i]);
> +if (i < dimm->mci->n_layers - 1)
> +printk(KERN_CONT ".");
> +}
> +printk(KERN_CONT "\n");
> >>>
> >>> This looks hacky but I don't have a good suggestion what to do instead
> >>> here. Maybe snprintf into a complete string which you can issue with
> >>> debugf4()...
> >>
> >> This is not hacky. There are several places at the Kernel doing loops like
> >> that. Look, for example, at lib/hexdump.c (without KERN_CONT, as this
> >> macro was added later - probably to avoid checkpatch.pl complains).
> >
> > There is some benefit to "one printk == one output line" ... it means
> > that console output will not be (as) jumbled if multiple cpus are
> > printk'ing at the same time.
>
> Ok, but this message only appears when all the conditions below are met:
> - the driver is compiled with EDAC_DEBUG;
> - the edac_core is modprobed with edac_debug_level=4;
> - during the driver modprobe, when the EDAC driver is being registered.
That means nothing.
> Even on several-core machines, those messages won't mangle, in practice.
>
> Let's not over-design a simple debug message.
No, let's design a simple debug message correctly, regardless of when it
appears.
>
> Regards,
> Mauro
>
--
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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers
On Fri, Apr 27, 2012 at 01:07:38PM -0300, Mauro Carvalho Chehab wrote:
> Yes. This is a common issue at the EDAC core: on several places, it calls the
> edac debug macros (DEBUGF0...DEBUGF4) passing a __func__ as an argument, while
> the debug macros already handles that. I suspect that, in the past, the
> __func__
> were not at the macros, but some patch added it there, and forgot to fix the
> occurrences of its call.
The patch that added it is d357cbb445208 and you reviewed it.
> This is something that needs to be reviewed at the entire EDAC core (and
> likely
> at the drivers).
Looks like a job for a newbie to get her/his feet wet with kernel work.
> I opted to not touch on this at the existing debug logic, as I think that the
> better is to address all those issues on one separate patch, after fixing the
> EDAC core bugs.
No,
you simply need to remove the __func__ argument in your newly added debug call:
debugf2("%s: %d: dimm%zd (%d:%d:%d): row %d, chan %d\n",
__func__,
i, (dimm - mci->dimms),
pos[0], pos[1], pos[2], row, chn);
And while you're at it, remove the rest of the __func__ arguments from
your newly added debugfX calls.
--
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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
