From: Andrew Morton <akpm@linux-foundation•org>
To: "Jeff Haran" <jharan@Brocade•COM>
Cc: Doug, linux-kernel@vger•kernel.org,
Dave Jiang <djiang@mvista•com>,
linuxppc-dev@ozlabs•org, Thompson <dougthompson@xmission•com>,
Kumar Gala <galak@gate•crashing.org>
Subject: Re: bug in drivers/edac/mpc85xx_edac.c:mpc85xx_mc_check()
Date: Wed, 29 Apr 2009 00:37:04 -0700 [thread overview]
Message-ID: <20090429003704.1c37d6e1.akpm@linux-foundation.org> (raw)
In-Reply-To: <57AC2FA1761300418C7AB8F3EA493C9702E319DB@HQ-EXCH-5.corp.brocade.com>
Let's cc the suitable people.
On Tue, 28 Apr 2009 18:23:42 -0700 "Jeff Haran" <jharan@Brocade•COM> wrote:
> Hi,
>
> Recent versions of this function contain the following snippets:
>
> if (err_detect & DDR_EDE_SBE)
> edac_mc_handle_ce(mci, pfn, err_addr & PAGE_MASK,
> syndrome, row_index, 0, mci->ctl_name);
>
> if (err_detect & DDR_EDE_MBE)
> edac_mc_handle_ue(mci, pfn, err_addr & PAGE_MASK,
> row_index, mci->ctl_name);
>
> I am pretty sure the references to PAGE_MASK should be proceeded by a
> tilda, as in:
>
> if (err_detect & DDR_EDE_SBE)
> edac_mc_handle_ce(mci, pfn, err_addr & ~PAGE_MASK,
> syndrome, row_index, 0, mci->ctl_name);
>
> if (err_detect & DDR_EDE_MBE)
> edac_mc_handle_ue(mci, pfn, err_addr & ~PAGE_MASK,
> row_index, mci->ctl_name);
>
Could well be. PAGE_MASK is very easy to get wrong. I've _never_
trusted my own memory of it and I always have to go back to the
definition when reviewing code :(
> Much as I would like to submit a tested patch like the rest of the
> world, I find myself in the situation where the only Freescale target
> system I have to test on is running a 3 year old kernel (2.6.14), which
> preceeds the introduction of EDAC driver support, at least for
> Freescale. So the best I can do is borrow from the new EDAC driver and
> backport it to the old kernel.
>
> But I have learned a few things in this process and can thus share what
> I've learned as it may be of help to the EDAC driver developers:
>
> 1) Before you read the Freescale 8548 CAPTURE_ADDRESS register, you want
> to read CAPTURE_ATTRIBUTES first and make sure the VLD bit (least
> significant bit in the register) is set or else the data in
> CAPTURE_ADDRESS may not be yet valid.
>
> 2) When you are done scrubbing the memory with the single bit error, you
> want to write 0 to CAPTURE_ATTRIBUTES so as to clear VLD and thus setup
> the ECC capture logic to capture the next single bit error.
>
next parent reply other threads:[~2009-04-29 7:41 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <57AC2FA1761300418C7AB8F3EA493C9702E319DB@HQ-EXCH-5.corp.brocade.com>
2009-04-29 7:37 ` Andrew Morton [this message]
2009-04-29 12:46 ` bug in drivers/edac/mpc85xx_edac.c:mpc85xx_mc_check() Kumar Gala
[not found] <57AC2FA1761300418C7AB8F3EA493C9702C5F200@HQ-EXCH-5.corp.brocade.com>
2009-04-10 21:47 ` bug in drivers/edac/mpc85xx_edac.c:mpc85xx_mc_check() ? Andrew Morton
2009-04-13 17:10 ` Dave Jiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090429003704.1c37d6e1.akpm@linux-foundation.org \
--to=akpm@linux-foundation$(echo .)org \
--cc=djiang@mvista$(echo .)com \
--cc=dougthompson@xmission$(echo .)com \
--cc=galak@gate$(echo .)crashing.org \
--cc=jharan@Brocade$(echo .)COM \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox