mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: "barebox@lists.infradead.org" <barebox@lists.infradead.org>
Subject: Re: [PATCH 10/10] ARM: pbl: Add an option to validate DRAM
Date: Tue, 26 May 2015 08:57:56 +0200	[thread overview]
Message-ID: <20150526065756.GZ6325@pengutronix.de> (raw)
In-Reply-To: <CAHQ1cqECGkwNQp4tgBrj1g6NR01EQHjsbf_omeiN+GPHpaeuhA@mail.gmail.com>

On Sat, May 23, 2015 at 11:48:41AM -0700, Andrey Smirnov wrote:
> On Tue, May 19, 2015 at 12:06 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > Hi Andrey,
> >
> > On Wed, May 13, 2015 at 07:54:27PM -0700, Andrey Smirnov wrote:
> >> Add an option to perform DRAM region validation before using it. The
> >> framework allows individual boards to set up a memory validaion hook
> >> that would be called prior to Barebox using that region of memory.
> >>
> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> >
> > What usecase do you have for this patch?
> 
> The usecase is to be able to have a hardware verification build of the
> boot-loader which can be used to test the boards in manufacturing.
> 
> > Is it debugging or something  you always want to enable on your hardware?
> 
> I need it to be always enabled only in a special build of the BL.
> 
> > Why must the validate_memory_range function be board specific?
> 
> Because the choice of a memory validation algorithm depends on many factors:
> - Speed vs. how extensive you want your tests to be
> - Chosen memory fault model (DDR vs. SRAM would be different)
> - etc.
> 
> Also various memory controllers also might various degree of low level
> control so some might allow the developer to flip more switches and
> test more corner cases.
> 
> >
> > I see that you call validate_memory_range on potentially large areas of
> > memory, so I wonder if you can't call validate_memory_range from your
> > board setup code with the complete memory instead.
> 
> Even with the current algorithm implemented in mem_test() (which ,
> having read a number of academic papers on memory testing, I don't
> believe is comprehensible enough) testing any significant of memory
> takes a very noticeable amount of time. I wanted to spend as little
> amount of time without having access to extended Barebox functionality
> to communicate with the rest of the world(like networking, proper
> serial) as possible so I set up the algorithm the way it is and
> configured Barebox to have a small(3MB) heap.
> 
> Also, testing all of the memory in PBL code brings up the question of
> what is the point of 'memtest' command? If the only comprehensive way
> of testing memory is in PBL code than, IMHO, that function is not very
> useful.
> 
> > I am not very fond of overly using get_runtime_offset to calculate
> > pointers. Setting callback functions from early code which does not run
> > on its link address is something I really want to avoid.
> 
> I agree with you on this point. I don't like that code very much either.

How about testing only a small fragment of DRAM, say 8MB, in your
lowlevel board code and calling barebox_arm_entry() with the
membase/memsize you previously tested? This way you can make sure that
barebox only uses tested memory without having to test all memory before
calling barebox_arm_entry() and without having to call back into some
testing function.

The newest TLSF implementation also supports pools, so we could add the
full amount of memory later if we want to.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  parent reply	other threads:[~2015-05-26  6:58 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-14  2:54 [PATCH 01/10] common/memtest.c: Fix incorrect array boundary check Andrey Smirnov
2015-05-14  2:54 ` [PATCH 02/10] common/memtest.c: Do not omit offset of 0 from tests Andrey Smirnov
2015-05-14  2:54 ` [PATCH 03/10] common/memtest.c: Refactor mem_test() into three surbroutines Andrey Smirnov
2015-05-14  2:54 ` [PATCH 04/10] common/memtest.c: Distil common error reporting code Andrey Smirnov
2015-05-14  2:54 ` [PATCH 05/10] serial: i.MX: Write settings to a correct register Andrey Smirnov
2015-05-14  2:54 ` [PATCH 06/10] common: pbl: Allow boards to override hang() Andrey Smirnov
2015-05-15  5:25   ` Sascha Hauer
2015-05-23 18:13     ` Andrey Smirnov
2015-05-14  2:54 ` [PATCH 07/10] debug_ll: i.MX: Add support for input to DEBUG_LL Andrey Smirnov
2015-05-14  2:54 ` [PATCH 08/10] i.MX51: babbage: Add UART RXD pin configuration Andrey Smirnov
2015-05-14  2:54 ` [PATCH 09/10] pbl: Implement ctrlc() using getc_ll() Andrey Smirnov
2015-05-14  2:54 ` [PATCH 10/10] ARM: pbl: Add an option to validate DRAM Andrey Smirnov
2015-05-19  7:06   ` Sascha Hauer
2015-05-23 18:48     ` Andrey Smirnov
2015-05-23 20:44       ` Alexander Aring
2015-05-24 18:39         ` Andrey Smirnov
2015-05-26  6:57       ` Sascha Hauer [this message]
2015-06-01 13:09         ` Andrey Smirnov
2015-06-03  8:10           ` Sascha Hauer
2015-05-15  5:33 ` [PATCH 01/10] common/memtest.c: Fix incorrect array boundary check Sascha Hauer
2015-05-23 18:20   ` Andrey Smirnov

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=20150526065756.GZ6325@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=andrew.smirnov@gmail.com \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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