Giter VIP home page Giter VIP logo

Comments (10)

dlbeer avatar dlbeer commented on July 17, 2024

Yes, what you've described is correct. You should ensure that the NAND OOB data contains at least one non-0xff byte in order for this to work reliably. You could do this either using an ECC scheme that produces some zero ECC bits for an all-ones page, or by including an explicit reserved non-0xff byte.

from dhara.

mnijweide avatar mnijweide commented on July 17, 2024

I see. I'd hoped there was some way to avoid OOB data, the NAND chip I'm using already has built-in ECC so I don't need that.

from dhara.

dlbeer avatar dlbeer commented on July 17, 2024

Sorry, it's been a while since I worked on this, and I'd forgotten something: the pages that are checked will, if they've been programmed, contain non-FF bytes. They're checkpoint pages, which have a specific format -- so it is actually fine to test for the page being all-FF.

from dhara.

mnijweide avatar mnijweide commented on July 17, 2024

I tried that, but it didn't work. So I 'improved' the dhara_nand_is_free function so that it would only mark a page as free if all following pages are also FF - this vastly improved the stability, but is inefficient at boot time.

But the following still fails:

const int sector_size = 512;

struct dhara_map map;
struct dhara_nand flash_memory;
dhara_error_t err;
uint8_t* page_buf = malloc(sector_size);
uint8_t* sector_buf = malloc(sector_size);

flash_memory.log2_page_size = 9; // 2^9 = 512, the sector size
flash_memory.log2_ppb = 9; // and 2^9 = 512 sectors fit in the 256 kB flash erase block
flash_memory.num_blocks = 4 * 5; // 4 erase blocks = 1 MB, and we have a 5 MB region

dhara_map_init(&map, &flash_memory, page_buf, 64);
dhara_map_resume(&map, &err);
dhara_map_clear(&map);

dhara_sector_t cap = dhara_map_capacity(&map);
cap = cap * 4 / 5; /* Use 80% otherwise this test takes way too long */

for( int run = 0; run < 7; run ++) {
    const int set_a[7] = { 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0x00 };
    const int set_b[7] = { 0xff, 0xff, 0x00, 0xff, 0x00, 0x00, 0xff };

    printf("Run: %d - ", run);

    memset( sector_buf, set_a[run], sector_size );

    for (dhara_sector_t i = 0; i < cap - 1; i++) {
        if (i == cap / 2) {
            memset(sector_buf, set_b[run], sector_size );

            /* Clear memory to simulate reboot */
            memset(page_buf, 0xFF, sector_size);
            memset(&map, 0xFF, sizeof(map));

            dhara_map_init(&map, &flash_memory, page_buf, 64);
            dhara_map_resume(&map, &err);
        }
        dhara_map_write(&map, i, sector_buf, &err);
    }

    /* Clear memory to simulate reboot */
    memset(page_buf, 0xFF, sector_size);
    memset(&map, 0xFF, sizeof(map));

    dhara_map_init(&map, &flash_memory, page_buf, 64);
    dhara_map_resume(&map, &err);

    for (dhara_sector_t i = 0; i < cap - 1; i++) {
        dhara_map_read(&map, i, sector_buf, &err);
        uint8_t cmp = i < cap/2 ? set_a[run] : set_b[run];
        for (int j = 0; j < sector_size; j++) {
            if (sector_buf[j] != cmp) {
                printf("Error: %d; sector %d contains %d should be %d\n", run, i, sector_buf[j], cmp);
                break;
            }
        }
    }
}

(the code was copied from test code and I had to remove some parts, so it may not work)

from dhara.

dlbeer avatar dlbeer commented on July 17, 2024

The simulator used in the test suite keeps a "next page" counter for each block and uses that for testing whether a page is free. I just modified it check the page's contents instead, and I find that everything works as expected.

I'm sure you've checked already, but are you absolutely sure there's nothing wrong with your NAND flash access layer? If so, is there a minimal test case you can think of that I'd be able to use to reproduce the problem (and fix it)?

from dhara.

mnijweide avatar mnijweide commented on July 17, 2024

I'm very sure there is nothing wrong with my NAND flash access layer. I've created a small test case that writes logical sector 0 with FF then reboots and writes sector 1 with 00. Inserted printf() in the nand access layer and this is what I get:

*** boot
dhara_nand_is_free( ..., page = 5884 ) : 0
dhara_nand_is_free( ..., page = 5888 ) : 0
dhara_nand_is_free( ..., page = 6012 ) : 1
dhara_nand_is_free( ..., page = 5948 ) : 1
dhara_nand_is_free( ..., page = 5916 ) : 1
dhara_nand_is_free( ..., page = 5900 ) : 1
dhara_nand_is_free( ..., page = 5892 ) : 0
dhara_nand_is_free( ..., page = 5896 ) : 1
dhara_nand_is_free( ..., page = 5893 ) : 0
dhara_nand_is_free( ..., page = 5894 ) : 0
dhara_nand_is_free( ..., page = 5896 ) : 1             // dhara_map_clear is here 
*** write sector 0 with 0xff
dhara_nand_prog( ..., page = 5896, data[0] = 255, ... )   // data
dhara_nand_prog( ..., page = 5899, data[0] = 68, ... )     // checkpoint page?
*** reboot
dhara_nand_is_free( ..., page = 5884 ) : 0
dhara_nand_is_free( ..., page = 5888 ) : 0
dhara_nand_is_free( ..., page = 6012 ) : 1
dhara_nand_is_free( ..., page = 5948 ) : 1
dhara_nand_is_free( ..., page = 5916 ) : 1
dhara_nand_is_free( ..., page = 5900 ) : 1
dhara_nand_is_free( ..., page = 5892 ) : 0
dhara_nand_is_free( ..., page = 5896 ) : 1
dhara_nand_is_free( ..., page = 5893 ) : 0
dhara_nand_is_free( ..., page = 5894 ) : 0
dhara_nand_is_free( ..., page = 5896 ) : 1            // but this is a user page
*** write sector 1 with 0x 0
dhara_nand_prog( ..., page = 5896, data[0] = 0, ... )     // and this is writing to the user page
dhara_nand_prog( ..., page = 5899, data[0] = 68, ... )
*** reboot
dhara_nand_is_free( ..., page = 5884 ) : 0
dhara_nand_is_free( ..., page = 5888 ) : 0
dhara_nand_is_free( ..., page = 6012 ) : 1
dhara_nand_is_free( ..., page = 5948 ) : 1
dhara_nand_is_free( ..., page = 5916 ) : 1
dhara_nand_is_free( ..., page = 5900 ) : 1
dhara_nand_is_free( ..., page = 5892 ) : 0
dhara_nand_is_free( ..., page = 5896 ) : 0
dhara_nand_is_free( ..., page = 5896 ) : 0
dhara_nand_is_free( ..., page = 5900 ) : 1
dhara_nand_is_free( ..., page = 5897 ) : 0
dhara_nand_is_free( ..., page = 5898 ) : 0
dhara_nand_is_free( ..., page = 5900 ) : 1
sector 0 contains 0 should be 255
sector 1 contains 252 should be 0

And fixing this.... Is see that dhara_nand_is_free is invoked for several user pages. find_last_group() hits only on user pages and find_head() even explicitly iterates through user pages and invokes dhara_nand_free() on them. I think it would be easy to change find_last_group so it hits on checkpoint pages, but as for find_head()?

test case code:

static void test_simple()
{
    const int sector_size = 512;
    const int fill_byte[] = { 0xff, 0x00 };

    struct dhara_nand flash_memory;
    flash_memory.region = REG1; /* Test region */
    flash_memory.log2_page_size = 9; // 2^9 = 512, the sector size
    flash_memory.log2_ppb = 9; // and 2^9 = 512 sectors fit in the 256 kB flash erase block
    flash_memory.num_blocks = 4 * 4; // 4 erase blocks = 1 MB, and we have a 4 MB region

    uint8_t* page_buf = malloc(sector_size);
    uint8_t* sector_buf = malloc(sector_size);

    int r;
    struct dhara_map map;
    dhara_error_t err;

    /* "boot" */
    printf("*** boot\n");
    dhara_map_init(&map, &flash_memory, page_buf, 64);
    r = dhara_map_resume(&map, &err);
    if (r == 0) {
        // there is recognizable data, make sure we start with empty memory
        dhara_map_clear(&map);
    }

    /* cycle */
    for (dhara_sector_t s = 0; s < sizeof(fill_byte) / sizeof(fill_byte[0]); s++) {
        printf("*** write sector %d with 0x%2x\n",s, fill_byte[s]);
        memset( sector_buf, fill_byte[s], sector_size );
        dhara_map_write(&map, s, sector_buf, &err);
        dhara_map_sync(&map, &err);

        /* "reboot" */
        printf("*** reboot\n");
        memset(page_buf, 0xFF, sector_size);
        memset(&map, 0xFF, sizeof(map));
        dhara_map_init(&map, &flash_memory, page_buf, 64);
        dhara_map_resume(&map, &err);
    }

    /* Verify correct data was written */
    for (dhara_sector_t s = 0; s < sizeof(fill_byte) / sizeof(fill_byte[0]); s++) {
        r = dhara_map_read(&map, s, sector_buf, &err);
        for (int i = 0; i < sector_size; i++) {
            if (sector_buf[i] != fill_byte[s]) {
                printf("sector %d contains %d should be %d\n", s, sector_buf[i], fill_byte[s]);
                break;
            }
        }
    }

    free(sector_buf);
    free(page_buf);
}

from dhara.

dlbeer avatar dlbeer commented on July 17, 2024

Ah, I see. Sorry, it's been a while since I worked on this -- I think I was expecting that users would be implementing their own OOB scheme and would therefore be able to determine whether a page was programmed by looking at something other than its contents (that's why nand_is_free exists at all).

If you really aren't able to do anything with OOB or query the chip, what I'd suggest in your case is a small change to dhara/journal.c. Add this function:

static int group_is_free(struct dhara_journal *j, dhara_page_t up0)
{
       const unsigned int n = 1 << j->log2_ppc;
       unsigned int i;

       for (i = 0; i < n; i++)
               if (!dhara_nand_is_free(j->nand, up0 + i))
                       return 0;

       return 1;
}

...and then change calls elsewhere to dhara_nand_is_free(j->nand, ...) to group_is_free(j, ...).

I haven't checked that thoroughly, so be careful with it.

from dhara.

mnijweide avatar mnijweide commented on July 17, 2024

That works. The only thing I'm concerned about that if reboot occurs after a user page with all FF is written but before the checkpoint page was written, that initialization of the journal will use a page that is already in use. But that is an academical case in my situation.

Anyways, thanks for your time.

from dhara.

dlbeer avatar dlbeer commented on July 17, 2024

No problem. It might possibly not be an issue -- I've found on most chips that if you program a page to be all 0xff (including the ECC bytes), it's essentially a no-op and you can freely write the page (i.e. you can go from 1 to 0 at any time, just never from 0 to 1 without an erase).

When I have a bit of spare time I'll look at the fix I've suggested in a bit more detail and perhaps integrate it into the repository properly. In the case where the user is able to implement an ECC-based is_free(), it shouldn't impose much extra cost.

In the meantime, let me know if you do run into any problems with it.

from dhara.

dlbeer avatar dlbeer commented on July 17, 2024

After further thought and a bit of testing, this seems to be fine. I've added commit f4a0733.

from dhara.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.