June 2014 - coreboot-gerrit - mail.coreboot.org

Patch set updated for coreboot: 7c9e511 Remove unnecessary mappings in cbfs_media
by Naman Govil June 30, 2014

June 30, 2014

Naman Govil (namangov(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6169 -gerrit commit 7c9e511163197db9e57c874212a1c9574f2a1f47 Author: Naman Govil <namangov(a)gmail.com> Date: Mon Jun 30 22:57:11 2014 +0530 Remove unnecessary mappings in cbfs_media Reducing the sram usage while booting by removing unwanted map()s by creating a structure to replace it with appropriate read()s Change-Id: If503368553308a2fdd3ecc1a312a11f14c7a707f Signed-off-by: Naman Govil <namangov(a)gmail.com> --- src/include/cbfs_core.h | 19 ++++- src/lib/cbfs.c | 72 +++++++++++++------ src/lib/cbfs_core.c | 182 +++++++++++++++++++++++++++++++----------------- 3 files changed, 188 insertions(+), 85 deletions(-) diff --git a/src/include/cbfs_core.h b/src/include/cbfs_core.h index a1d8127..6e0193a 100644 --- a/src/include/cbfs_core.h +++ b/src/include/cbfs_core.h @@ -4,6 +4,7 @@ * Copyright (C) 2008 Jordan Crouse <jordan(a)cosmicpenguin.net> * Copyright (C) 2012 Google, Inc. * Copyright (C) 2013 The Chromium OS Authors. All rights reserved. + * Copyright (C) 2014 Naman Govil <namangov(a)gmail.com> * * This file is dual-licensed. You can choose between: * - The GNU GPL, version 2, as published by the Free Software Foundation @@ -189,6 +190,13 @@ struct cbfs_optionrom { #define CBFS_MEDIA_INVALID_MAP_ADDRESS ((void*)(0xffffffff)) #define CBFS_DEFAULT_MEDIA ((void*)(0x0)) +struct cbfs_file_handler { + uint32_t found; + uint32_t data_offset; + uint32_t data_len; + struct cbfs_file file; +}; + /* Media for CBFS to load files. */ struct cbfs_media { @@ -220,9 +228,6 @@ struct cbfs_media { /* returns pointer to a file entry inside CBFS or NULL */ struct cbfs_file *cbfs_get_file(struct cbfs_media *media, const char *name); -/* returns pointer to file content inside CBFS after if type is correct */ -void *cbfs_get_file_content(struct cbfs_media *media, const char *name, - int type, size_t *sz); /* returns decompressed size on success, 0 on failure */ int cbfs_decompress(int algo, void *src, void *dst, int len); @@ -231,6 +236,14 @@ int cbfs_decompress(int algo, void *src, void *dst, int len); * on failure */ const struct cbfs_header *cbfs_get_header(struct cbfs_media *media); +/*Returns success (0) on finding the file requested by verifying name/type; -1 if file not found + The absolute data_offset to the file is stored*/ +int cbfs_find_file(struct cbfs_media *media, struct cbfs_file_handler *f, const char *name, int type); + + +/* returns pointer to file content inside CBFS after verifying if type is correct */ +void *cbfs_get_file_content(struct cbfs_media *media, const char *name, int type, size_t *sz); + #endif /* __ROMCC__ */ #endif diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index dc08937..d349383 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -3,7 +3,7 @@ * * Copyright (C) 2008, Jordan Crouse <jordan(a)cosmicpenguin.net> * Copyright (C) 2013 The Chromium OS Authors. All rights reserved. - * + * Copyright (C) 2014, Naman Govil <namangov(a)gmail.com> * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; version 2 of the License. @@ -119,39 +119,71 @@ void *cbfs_load_optionrom(struct cbfs_media *media, uint16_t vendor, void * cbfs_load_stage(struct cbfs_media *media, const char *name) { - struct cbfs_stage *stage = (struct cbfs_stage *) - cbfs_get_file_content(media, name, CBFS_TYPE_STAGE, NULL); + struct cbfs_file_handler f; + struct cbfs_stage stage; + int c; + ssize_t value_read; + void * data; + ssize_t v_read; + struct cbfs_media default_media; + + if (media == CBFS_DEFAULT_MEDIA) { + media = &default_media; + if (init_default_cbfs_media(media) != 0) { + ERROR("Failed to initialize default media.\n"); + return NULL; + } + } + + c = cbfs_find_file(media, &f, name, CBFS_TYPE_STAGE); + if (c == 0) { + value_read = media->read(media, &stage, f.data_offset, sizeof(stage)); /* this is a mess. There is no ntohll. */ /* for now, assume compatible byte order until we solve this. */ uint32_t entry; uint32_t final_size; - if (stage == NULL) + DEBUG("Read Complete @offset = 0x%x and length = %d\n", f.data_offset, value_read); + if (value_read != sizeof(stage)) return (void *) -1; LOG("loading stage %s @ 0x%x (%d bytes), entry @ 0x%llx\n", name, - (uint32_t) stage->load, stage->memlen, - stage->entry); - - final_size = cbfs_decompress(stage->compression, - ((unsigned char *) stage) + - sizeof(struct cbfs_stage), - (void *) (uint32_t) stage->load, - stage->len); - if (!final_size) - return (void *) -1; + (uint32_t) stage.load, stage.memlen, + stage.entry); - /* Stages rely the below clearing so that the bss is initialized. */ - memset((void *)((uintptr_t)stage->load + final_size), 0, - stage->memlen - final_size); - DEBUG("stage loaded.\n"); + if(stage.compression == CBFS_COMPRESS_NONE) //i.e no compression + { + //No compression; hence we can directly read + DEBUG("Read Done\n"); + v_read = media->read(media, (void *) (uintptr_t) stage.load, f.data_offset + sizeof(stage), f.data_len); + final_size = f.data_len; + } + else + { + data = media->map(media, f.data_offset + sizeof(stage), f.data_len); + DEBUG("Map Done\n"); + final_size = cbfs_decompress(stage.compression, data, + (void *) (uint32_t) stage.load, + stage.len); + + if (!final_size) + return (void *) -1; + } - entry = stage->entry; - // entry = ntohll(stage->entry); + /* Stages rely the below clearing so that the bss is initialized. */ + memset((void *)((uintptr_t)stage.load + final_size), 0, + stage.memlen - final_size); + DEBUG("stage loaded.\n"); + entry = stage.entry; return (void *) entry; + + } else { + ERROR("Stage not loaded\n"); + return (void *)-1; + } } /* Simple buffer */ diff --git a/src/lib/cbfs_core.c b/src/lib/cbfs_core.c index 50c037e..10e23f1 100644 --- a/src/lib/cbfs_core.c +++ b/src/lib/cbfs_core.c @@ -3,6 +3,7 @@ * * Copyright (C) 2011 secunet Security Networks AG * Copyright (C) 2013 The Chromium OS Authors. All rights reserved. + * Copyright (C) 2014 Naman Govil <namangov(a)gmail.com> * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -56,8 +57,10 @@ * on failure */ const struct cbfs_header *cbfs_get_header(struct cbfs_media *media) { - const struct cbfs_header *header; + struct cbfs_header header; + const struct cbfs_header *h; struct cbfs_media default_media; + ssize_t header_size; if (media == CBFS_DEFAULT_MEDIA) { media = &default_media; @@ -70,133 +73,188 @@ const struct cbfs_header *cbfs_get_header(struct cbfs_media *media) media->open(media); DEBUG("CBFS_HEADER_ROM_ADDRESS: 0x%x/0x%x\n", CBFS_HEADER_ROM_ADDRESS, CONFIG_ROM_SIZE); - header = media->map(media, CBFS_HEADER_ROM_ADDRESS, sizeof(*header)); + header_size = media->read(media, &header, CBFS_HEADER_ROM_ADDRESS, sizeof(header)); + //size of mapping (now read) is 32 bytes media->close(media); - if (header == CBFS_MEDIA_INVALID_MAP_ADDRESS) { + if (header_size == sizeof(header)) { ERROR("Failed to load CBFS header from 0x%x\n", CBFS_HEADER_ROM_ADDRESS); return CBFS_HEADER_INVALID_ADDRESS; } - if (CBFS_HEADER_MAGIC != ntohl(header->magic)) { + if (CBFS_HEADER_MAGIC != ntohl(header.magic)) { ERROR("Could not find valid CBFS master header at %x: " "%x vs %x.\n", CBFS_HEADER_ROM_ADDRESS, CBFS_HEADER_MAGIC, - ntohl(header->magic)); - if (header->magic == 0xffffffff) { + ntohl(header.magic)); + if (header.magic == 0xffffffff) { ERROR("Maybe ROM is not mapped properly?\n"); } return CBFS_HEADER_INVALID_ADDRESS; } - return header; + + h = &header; + return h; } /* public API starts here*/ -struct cbfs_file *cbfs_get_file(struct cbfs_media *media, const char *name) +/* This functions finds the absolute data_offset of a file searched for by name/type + Returns 0 on success and -1 on failure + */ +int cbfs_find_file(struct cbfs_media *media, struct cbfs_file_handler *f, const char *name, int type) { - const char *file_name; - uint32_t offset, align, romsize, name_len; - const struct cbfs_header *header; - struct cbfs_file file, *file_ptr; + uint32_t offset, align, romsize,name_len; struct cbfs_media default_media; + const struct cbfs_header *header; + ssize_t value_read; + const char *file_name; if (media == CBFS_DEFAULT_MEDIA) { media = &default_media; if (init_default_cbfs_media(media) != 0) { ERROR("Failed to initialize default media.\n"); - return NULL; + return -1; } } if (CBFS_HEADER_INVALID_ADDRESS == (header = cbfs_get_header(media))) - return NULL; + return -1; // error // Logical offset (for source media) of first file. + // Now we know where the file lives offset = ntohl(header->offset); align = ntohl(header->align); romsize = ntohl(header->romsize); - // TODO Add a "size" in CBFS header for a platform independent way to - // determine the end of CBFS data. #if defined(CONFIG_ARCH_X86) && CONFIG_ARCH_X86 romsize -= htonl(header->bootblocksize); #endif + int catch; DEBUG("CBFS location: 0x%x~0x%x, align: %d\n", offset, romsize, align); - DEBUG("Looking for '%s' starting from 0x%x.\n", name, offset); media->open(media); - while (offset < romsize && - media->read(media, &file, offset, sizeof(file)) == sizeof(file)) { - if (memcmp(CBFS_FILE_MAGIC, file.magic, - sizeof(file.magic)) != 0) { + while (offset < romsize) + { + value_read = media->read(media, &f->file, offset, sizeof(f->file)); + if(value_read != sizeof(f->file)){ + return -1; //error: since read not successful + break; + } + //make all ntohl() at once place to avoid any gibberish later + f->file.len = ntohl(f->file.len); + f->file.offset = ntohl(f->file.offset); + f->file.type = ntohl(f->file.type); + + if (memcmp(CBFS_FILE_MAGIC, f->file.magic, + sizeof(f->file.magic)) != 0) { uint32_t new_align = align; if (offset % align) new_align += align - (offset % align); - ERROR("ERROR: No file header found at 0x%x - " - "try next aligned address: 0x%x.\n", offset, - offset + new_align); + ERROR("ERROR: No file header found at 0x%x - try next aligned address: 0x%x.\n", offset,offset+new_align); offset += new_align; continue; + //continuing with new offset } - name_len = ntohl(file.offset) - sizeof(file); - DEBUG(" - load entry 0x%x file name (%d bytes)...\n", offset, - name_len); - - // load file name (arbitrary length). - file_name = (const char *)media->map( - media, offset + sizeof(file), name_len); - if (file_name == CBFS_MEDIA_INVALID_MAP_ADDRESS) { - ERROR("ERROR: Failed to get filename: 0x%x.\n", offset); - } else if (strcmp(file_name, name) == 0) { - int file_offset = ntohl(file.offset), - file_len = ntohl(file.len); - DEBUG("Found file (offset=0x%x, len=%d).\n", - offset + file_offset, file_len); - media->unmap(media, file_name); - file_ptr = media->map(media, offset, - file_offset + file_len); - media->close(media); - return file_ptr; - } else { - DEBUG(" (unmatched file @0x%x: %s)\n", offset, - file_name); - media->unmap(media, file_name); - } + f->found = -1; + if(f->file.type == type){ + + name_len = f->file.offset - sizeof(f->file); + DEBUG(" - load entry 0x%x file name (%d bytes)...\n", offset,name_len); + // load file name (arbitrary length). + file_name = (const char *)media->map(media, offset + sizeof(f->file), name_len); + //this mapping done to verify name of file + if (file_name == CBFS_MEDIA_INVALID_MAP_ADDRESS) { + ERROR("ERROR: Failed to get filename: 0x%x.\n", offset); + } else if (strcmp(file_name, name) == 0) { + f->found = 0; + f->data_offset = offset + f->file.offset; + f->data_len = f->file.len; + media->unmap(media, file_name); + DEBUG("Found file:offset = 0x%x, len=%d\n", f->data_offset, f->data_len); + catch = f->found; + return catch; + } else { + DEBUG("unmatched file offset = 0x%x : %s\n", offset, file_name); + media->unmap(media,file_name); + } + } // Move to next file. - offset += ntohl(file.len) + ntohl(file.offset); + offset += f->file.len + f->file.offset; if (offset % align) offset += align - (offset % align); + DEBUG("Going for next offset\n"); } media->close(media); - LOG("WARNING: '%s' not found.\n", name); - return NULL; + LOG("Warning: '%s' not found\n",name); + return -1; } -void *cbfs_get_file_content(struct cbfs_media *media, const char *name, - int type, size_t *sz) + +/*Returns pointer to file content inside CBFS after verifying type + */ +void *cbfs_get_file_content(struct cbfs_media *media, const char *name, int type, size_t *sz) { - struct cbfs_file *file = cbfs_get_file(media, name); + struct cbfs_file_handler f; + int c; + struct cbfs_media default_media; + if (media == CBFS_DEFAULT_MEDIA) { + media = &default_media; + if (init_default_cbfs_media(media) != 0) { + ERROR("Failed to initialize default media.\n"); + return NULL; + } + } + c = cbfs_find_file(media, &f, name, type); + if (c == 0){ + DEBUG("Found file. Will be mapping it now!\n"); if (sz) *sz = 0; - if (file == NULL) { - ERROR("Could not find file '%s'.\n", name); + if (f.file.type != type) { + ERROR("File '%s' is of type %x, but we requested %x.\n", name, f.file.type, type); return NULL; } - if (ntohl(file->type) != type) { - ERROR("File '%s' is of type %x, but we requested %x.\n", name, - ntohl(file->type), type); + if (sz) + *sz = f.data_len; + return media->map(media, f.data_offset, f.data_len + f.file.offset); + } + else { + DEBUG("condition not successful\n"); + ERROR("File Not Found\n"); return NULL; } +} - if (sz) - *sz = ntohl(file->len); +/*void cbfs_get_data(struct cbfs_media *media, const char *name, int type) +{ + struct cbfs_file_handler f; + ssize_t value_read; + int c; + c = cbfs_find_file(media, &f, name, type); + if (c == 0){ + DEBUG("File has been found and can be read\n"); + uint32_t loop_offset = f.data_offset + f.file.offset; + uint32_t loop_align = f.align; + + while (f.data_len > 0){ + + struct cbfs_file file; + value_read = media->read(media, &file, loop_offset , sizeof(file)); + loop_offset += file.len + file.offset; + if (loop_offset % loop_align) + loop_offset += loop_align = (loop_offset % loop_align); + f.data_len -= sizeof(file); + } + } + else { + ERROR("File not found\n"); + } - return (void *)CBFS_SUBHEADER(file); } +*/ int cbfs_decompress(int algo, void *src, void *dst, int len) {

1 0

New patch to review for coreboot: fb0ab0e Remove unnecessary mappings in cbfs_media
by Naman Govil June 30, 2014

June 30, 2014

Naman Govil (namangov(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6169 -gerrit commit fb0ab0e43fa665eec28fc4b286c27adff80b7e5b Author: Naman Govil <namangov(a)gmail.com> Date: Mon Jun 30 22:57:11 2014 +0530 Remove unnecessary mappings in cbfs_media Reducing the sram usage while booting by removing unwanted map()s by creating a structure to replace it with appropriate read()s Change-Id: If503368553308a2fdd3ecc1a312a11f14c7a707f Signed-off-by: Naman Govil <namangov(a)gmail.com> --- src/include/cbfs_core.h | 19 +++++- src/lib/cbfs.c | 72 ++++++++++++++------ src/lib/cbfs_core.c | 178 +++++++++++++++++++++++++++++++----------------- 3 files changed, 185 insertions(+), 84 deletions(-) diff --git a/src/include/cbfs_core.h b/src/include/cbfs_core.h index a1d8127..6e0193a 100644 --- a/src/include/cbfs_core.h +++ b/src/include/cbfs_core.h @@ -4,6 +4,7 @@ * Copyright (C) 2008 Jordan Crouse <jordan(a)cosmicpenguin.net> * Copyright (C) 2012 Google, Inc. * Copyright (C) 2013 The Chromium OS Authors. All rights reserved. + * Copyright (C) 2014 Naman Govil <namangov(a)gmail.com> * * This file is dual-licensed. You can choose between: * - The GNU GPL, version 2, as published by the Free Software Foundation @@ -189,6 +190,13 @@ struct cbfs_optionrom { #define CBFS_MEDIA_INVALID_MAP_ADDRESS ((void*)(0xffffffff)) #define CBFS_DEFAULT_MEDIA ((void*)(0x0)) +struct cbfs_file_handler { + uint32_t found; + uint32_t data_offset; + uint32_t data_len; + struct cbfs_file file; +}; + /* Media for CBFS to load files. */ struct cbfs_media { @@ -220,9 +228,6 @@ struct cbfs_media { /* returns pointer to a file entry inside CBFS or NULL */ struct cbfs_file *cbfs_get_file(struct cbfs_media *media, const char *name); -/* returns pointer to file content inside CBFS after if type is correct */ -void *cbfs_get_file_content(struct cbfs_media *media, const char *name, - int type, size_t *sz); /* returns decompressed size on success, 0 on failure */ int cbfs_decompress(int algo, void *src, void *dst, int len); @@ -231,6 +236,14 @@ int cbfs_decompress(int algo, void *src, void *dst, int len); * on failure */ const struct cbfs_header *cbfs_get_header(struct cbfs_media *media); +/*Returns success (0) on finding the file requested by verifying name/type; -1 if file not found + The absolute data_offset to the file is stored*/ +int cbfs_find_file(struct cbfs_media *media, struct cbfs_file_handler *f, const char *name, int type); + + +/* returns pointer to file content inside CBFS after verifying if type is correct */ +void *cbfs_get_file_content(struct cbfs_media *media, const char *name, int type, size_t *sz); + #endif /* __ROMCC__ */ #endif diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index dc08937..d349383 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -3,7 +3,7 @@ * * Copyright (C) 2008, Jordan Crouse <jordan(a)cosmicpenguin.net> * Copyright (C) 2013 The Chromium OS Authors. All rights reserved. - * + * Copyright (C) 2014, Naman Govil <namangov(a)gmail.com> * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; version 2 of the License. @@ -119,39 +119,71 @@ void *cbfs_load_optionrom(struct cbfs_media *media, uint16_t vendor, void * cbfs_load_stage(struct cbfs_media *media, const char *name) { - struct cbfs_stage *stage = (struct cbfs_stage *) - cbfs_get_file_content(media, name, CBFS_TYPE_STAGE, NULL); + struct cbfs_file_handler f; + struct cbfs_stage stage; + int c; + ssize_t value_read; + void * data; + ssize_t v_read; + struct cbfs_media default_media; + + if (media == CBFS_DEFAULT_MEDIA) { + media = &default_media; + if (init_default_cbfs_media(media) != 0) { + ERROR("Failed to initialize default media.\n"); + return NULL; + } + } + + c = cbfs_find_file(media, &f, name, CBFS_TYPE_STAGE); + if (c == 0) { + value_read = media->read(media, &stage, f.data_offset, sizeof(stage)); /* this is a mess. There is no ntohll. */ /* for now, assume compatible byte order until we solve this. */ uint32_t entry; uint32_t final_size; - if (stage == NULL) + DEBUG("Read Complete @offset = 0x%x and length = %d\n", f.data_offset, value_read); + if (value_read != sizeof(stage)) return (void *) -1; LOG("loading stage %s @ 0x%x (%d bytes), entry @ 0x%llx\n", name, - (uint32_t) stage->load, stage->memlen, - stage->entry); - - final_size = cbfs_decompress(stage->compression, - ((unsigned char *) stage) + - sizeof(struct cbfs_stage), - (void *) (uint32_t) stage->load, - stage->len); - if (!final_size) - return (void *) -1; + (uint32_t) stage.load, stage.memlen, + stage.entry); - /* Stages rely the below clearing so that the bss is initialized. */ - memset((void *)((uintptr_t)stage->load + final_size), 0, - stage->memlen - final_size); - DEBUG("stage loaded.\n"); + if(stage.compression == CBFS_COMPRESS_NONE) //i.e no compression + { + //No compression; hence we can directly read + DEBUG("Read Done\n"); + v_read = media->read(media, (void *) (uintptr_t) stage.load, f.data_offset + sizeof(stage), f.data_len); + final_size = f.data_len; + } + else + { + data = media->map(media, f.data_offset + sizeof(stage), f.data_len); + DEBUG("Map Done\n"); + final_size = cbfs_decompress(stage.compression, data, + (void *) (uint32_t) stage.load, + stage.len); + + if (!final_size) + return (void *) -1; + } - entry = stage->entry; - // entry = ntohll(stage->entry); + /* Stages rely the below clearing so that the bss is initialized. */ + memset((void *)((uintptr_t)stage.load + final_size), 0, + stage.memlen - final_size); + DEBUG("stage loaded.\n"); + entry = stage.entry; return (void *) entry; + + } else { + ERROR("Stage not loaded\n"); + return (void *)-1; + } } /* Simple buffer */ diff --git a/src/lib/cbfs_core.c b/src/lib/cbfs_core.c index 50c037e..ae36080 100644 --- a/src/lib/cbfs_core.c +++ b/src/lib/cbfs_core.c @@ -3,6 +3,7 @@ * * Copyright (C) 2011 secunet Security Networks AG * Copyright (C) 2013 The Chromium OS Authors. All rights reserved. + * Copyright (C) 2014 Naman Govil <namangov(a)gmail.com> * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -56,8 +57,10 @@ * on failure */ const struct cbfs_header *cbfs_get_header(struct cbfs_media *media) { - const struct cbfs_header *header; + struct cbfs_header header; + const struct cbfs_header *h; struct cbfs_media default_media; + ssize_t header_size; if (media == CBFS_DEFAULT_MEDIA) { media = &default_media; @@ -70,20 +73,21 @@ const struct cbfs_header *cbfs_get_header(struct cbfs_media *media) media->open(media); DEBUG("CBFS_HEADER_ROM_ADDRESS: 0x%x/0x%x\n", CBFS_HEADER_ROM_ADDRESS, CONFIG_ROM_SIZE); - header = media->map(media, CBFS_HEADER_ROM_ADDRESS, sizeof(*header)); + header_size = media->read(media, &header, CBFS_HEADER_ROM_ADDRESS, sizeof(header)); + //size of mapping (now read) is 32 bytes media->close(media); - if (header == CBFS_MEDIA_INVALID_MAP_ADDRESS) { + if (header_size == sizeof(header)) { ERROR("Failed to load CBFS header from 0x%x\n", CBFS_HEADER_ROM_ADDRESS); return CBFS_HEADER_INVALID_ADDRESS; } - if (CBFS_HEADER_MAGIC != ntohl(header->magic)) { + if (CBFS_HEADER_MAGIC != ntohl(header.magic)) { ERROR("Could not find valid CBFS master header at %x: " "%x vs %x.\n", CBFS_HEADER_ROM_ADDRESS, CBFS_HEADER_MAGIC, - ntohl(header->magic)); - if (header->magic == 0xffffffff) { + ntohl(header.magic)); + if (header.magic == 0xffffffff) { ERROR("Maybe ROM is not mapped properly?\n"); } return CBFS_HEADER_INVALID_ADDRESS; @@ -92,111 +96,163 @@ const struct cbfs_header *cbfs_get_header(struct cbfs_media *media) } /* public API starts here*/ -struct cbfs_file *cbfs_get_file(struct cbfs_media *media, const char *name) +/* This functions finds the absolute data_offset of a file searched for by name/type + Returns 0 on success and -1 on failure + */ +int cbfs_find_file(struct cbfs_media *media, struct cbfs_file_handler *f, const char *name, int type) { - const char *file_name; - uint32_t offset, align, romsize, name_len; - const struct cbfs_header *header; - struct cbfs_file file, *file_ptr; + uint32_t offset, align, romsize,name_len; struct cbfs_media default_media; + const struct cbfs_header *header; + ssize_t value_read; + const char *file_name; if (media == CBFS_DEFAULT_MEDIA) { media = &default_media; if (init_default_cbfs_media(media) != 0) { ERROR("Failed to initialize default media.\n"); - return NULL; + return -1; } } if (CBFS_HEADER_INVALID_ADDRESS == (header = cbfs_get_header(media))) - return NULL; + return -1; // error // Logical offset (for source media) of first file. + // Now we know where the file lives offset = ntohl(header->offset); align = ntohl(header->align); romsize = ntohl(header->romsize); - // TODO Add a "size" in CBFS header for a platform independent way to - // determine the end of CBFS data. #if defined(CONFIG_ARCH_X86) && CONFIG_ARCH_X86 romsize -= htonl(header->bootblocksize); #endif + int catch; DEBUG("CBFS location: 0x%x~0x%x, align: %d\n", offset, romsize, align); - DEBUG("Looking for '%s' starting from 0x%x.\n", name, offset); media->open(media); - while (offset < romsize && - media->read(media, &file, offset, sizeof(file)) == sizeof(file)) { - if (memcmp(CBFS_FILE_MAGIC, file.magic, - sizeof(file.magic)) != 0) { + while (offset < romsize) + { + value_read = media->read(media, &f->file, offset, sizeof(f->file)); + if(value_read != sizeof(f->file)){ + return -1; //error: since read not successful + break; + } + //make all ntohl() at once place to avoid any gibberish later + f->file.len = ntohl(f->file.len); + f->file.offset = ntohl(f->file.offset); + f->file.type = ntohl(f->file.type); + + if (memcmp(CBFS_FILE_MAGIC, f->file.magic, + sizeof(f->file.magic)) != 0) { uint32_t new_align = align; if (offset % align) new_align += align - (offset % align); - ERROR("ERROR: No file header found at 0x%x - " - "try next aligned address: 0x%x.\n", offset, - offset + new_align); + ERROR("ERROR: No file header found at 0x%x - try next aligned address: 0x%x.\n", offset,offset+new_align); offset += new_align; continue; - } - name_len = ntohl(file.offset) - sizeof(file); - DEBUG(" - load entry 0x%x file name (%d bytes)...\n", offset, - name_len); - - // load file name (arbitrary length). - file_name = (const char *)media->map( - media, offset + sizeof(file), name_len); - if (file_name == CBFS_MEDIA_INVALID_MAP_ADDRESS) { - ERROR("ERROR: Failed to get filename: 0x%x.\n", offset); - } else if (strcmp(file_name, name) == 0) { - int file_offset = ntohl(file.offset), - file_len = ntohl(file.len); - DEBUG("Found file (offset=0x%x, len=%d).\n", - offset + file_offset, file_len); - media->unmap(media, file_name); - file_ptr = media->map(media, offset, - file_offset + file_len); - media->close(media); - return file_ptr; - } else { - DEBUG(" (unmatched file @0x%x: %s)\n", offset, - file_name); - media->unmap(media, file_name); + //continuing with new offset } + f->found = -1; + if(f->file.type == type){ + + name_len = f->file.offset - sizeof(f->file); + DEBUG(" - load entry 0x%x file name (%d bytes)...\n", offset,name_len); + // load file name (arbitrary length). + file_name = (const char *)media->map(media, offset + sizeof(f->file), name_len); + //this mapping done to verify name of file + if (file_name == CBFS_MEDIA_INVALID_MAP_ADDRESS) { + ERROR("ERROR: Failed to get filename: 0x%x.\n", offset); + } else if (strcmp(file_name, name) == 0) { + f->found = 0; + f->data_offset = offset + f->file.offset; + f->data_len = f->file.len; + media->unmap(media, file_name); + DEBUG("Found file:offset = 0x%x, len=%d\n", f->data_offset, f->data_len); + catch = f->found; + return catch; + } else { + DEBUG("unmatched file offset = 0x%x : %s\n", offset, file_name); + media->unmap(media,file_name); + } + } // Move to next file. - offset += ntohl(file.len) + ntohl(file.offset); + offset += f->file.len + f->file.offset; if (offset % align) offset += align - (offset % align); + DEBUG("Going for next offset\n"); } media->close(media); - LOG("WARNING: '%s' not found.\n", name); - return NULL; + LOG("Warning: '%s' not found\n",name); + return -1; } -void *cbfs_get_file_content(struct cbfs_media *media, const char *name, - int type, size_t *sz) + +/*Returns pointer to file content inside CBFS after verifying type + */ +void *cbfs_get_file_content(struct cbfs_media *media, const char *name, int type, size_t *sz) { - struct cbfs_file *file = cbfs_get_file(media, name); + struct cbfs_file_handler f; + int c; + struct cbfs_media default_media; + if (media == CBFS_DEFAULT_MEDIA) { + media = &default_media; + if (init_default_cbfs_media(media) != 0) { + ERROR("Failed to initialize default media.\n"); + return NULL; + } + } + c = cbfs_find_file(media, &f, name, type); + if (c == 0){ + DEBUG("Found file. Will be mapping it now!\n"); if (sz) *sz = 0; - if (file == NULL) { - ERROR("Could not find file '%s'.\n", name); + if (f.file.type != type) { + ERROR("File '%s' is of type %x, but we requested %x.\n", name, f.file.type, type); return NULL; } - if (ntohl(file->type) != type) { - ERROR("File '%s' is of type %x, but we requested %x.\n", name, - ntohl(file->type), type); + if (sz) + *sz = f.data_len; + return media->map(media, f.data_offset, f.data_len + f.file.offset); + } + else { + DEBUG("condition not successful\n"); + ERROR("File Not Found\n"); return NULL; } +} - if (sz) - *sz = ntohl(file->len); +/*void cbfs_get_data(struct cbfs_media *media, const char *name, int type) +{ + struct cbfs_file_handler f; + ssize_t value_read; + int c; + c = cbfs_find_file(media, &f, name, type); + if (c == 0){ + DEBUG("File has been found and can be read\n"); + uint32_t loop_offset = f.data_offset + f.file.offset; + uint32_t loop_align = f.align; + + while (f.data_len > 0){ + + struct cbfs_file file; + value_read = media->read(media, &file, loop_offset , sizeof(file)); + loop_offset += file.len + file.offset; + if (loop_offset % loop_align) + loop_offset += loop_align = (loop_offset % loop_align); + f.data_len -= sizeof(file); + } + } + else { + ERROR("File not found\n"); + } - return (void *)CBFS_SUBHEADER(file); } +*/ int cbfs_decompress(int algo, void *src, void *dst, int len) {

1 0

Patch merged into coreboot/master: 42b716f northbridge/intel/nehalem/raminit.c: Extraneous parenthese
by gerrit＠coreboot.org June 30, 2014

June 30, 2014

the following patch was just integrated into master: commit 42b716f119ea994880364a7d423841ec97d99ac7 Author: Edward O'Callaghan <eocallaghan(a)alterapraxis.com> Date: Thu Jun 26 21:38:52 2014 +1000 northbridge/intel/nehalem/raminit.c: Extraneous parenthese Equality comparison with extraneous parenthese, spotted by Clang. Change-Id: I8d532392a0365753583ed441958e06d5da784587 Signed-off-by: Edward O'Callaghan <eocallaghan(a)alterapraxis.com> Reviewed-on: http://review.coreboot.org/6124 Tested-by: build bot (Jenkins) Reviewed-by: Patrick Georgi <patrick(a)georgi-clan.de> See http://review.coreboot.org/6124 for details. -gerrit

1 0

Patch merged into coreboot/master: df3629b northbridge/amd/{gx2, lx}: Qualify pointer with `volatile`
by gerrit＠coreboot.org June 30, 2014

June 30, 2014

the following patch was just integrated into master: commit df3629b63cc840f089cfd645fc5cf992bbbffe16 Author: Edward O'Callaghan <eocallaghan(a)alterapraxis.com> Date: Sat Jun 28 17:31:58 2014 +1000 northbridge/amd/{gx2,lx}: Qualify pointer with `volatile` There is no guarantee reading a dereferenced null pointer will not be optimised away. Qualify the integer storage type with volatile. Clang enforces this explicitness. Change-Id: I31524141d70632cade0490c820936a3a8b570346 Signed-off-by: Edward O'Callaghan <eocallaghan(a)alterapraxis.com> Reviewed-on: http://review.coreboot.org/6148 Tested-by: build bot (Jenkins) Reviewed-by: Patrick Georgi <patrick(a)georgi-clan.de> See http://review.coreboot.org/6148 for details. -gerrit

1 0

Patch merged into coreboot/master: 38a8fb0 x86 MTRR: Drop unused return value
by gerrit＠coreboot.org June 30, 2014

June 30, 2014

the following patch was just integrated into master: commit 38a8fb0c1891bbe3b53965e04c1c7ebcc00654da Author: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Date: Mon Jun 30 13:48:18 2014 +0300 x86 MTRR: Drop unused return value It was never well-defined what value this function should return. Change-Id: If84aff86e0b556591d7ad557842910a2dfcd3b46 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Reviewed-on: http://review.coreboot.org/6166 Tested-by: build bot (Jenkins) Reviewed-by: Patrick Georgi <patrick(a)georgi-clan.de> Reviewed-by: Edward O'Callaghan <eocallaghan(a)alterapraxis.com> See http://review.coreboot.org/6166 for details. -gerrit

1 0

Patch merged into coreboot/master: 599cda8 Use MTRR defines
by gerrit＠coreboot.org June 30, 2014

June 30, 2014

the following patch was just integrated into master: commit 599cda82283294944883cb17e8c2a43d96f9d2e9 Author: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Date: Wed Jun 25 01:22:17 2014 +0300 Use MTRR defines Change-Id: I60ae6dcb8c3b280fe74f27f4d61de70cc1ba190b Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Reviewed-on: http://review.coreboot.org/6123 Tested-by: build bot (Jenkins) Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net> Reviewed-by: Patrick Georgi <patrick(a)georgi-clan.de> Reviewed-by: Edward O'Callaghan <eocallaghan(a)alterapraxis.com> See http://review.coreboot.org/6123 for details. -gerrit

1 0

New patch to review for coreboot: 544d1c1 x86 MTRR: Drop unused return value
by Kyösti Mälkki June 30, 2014

June 30, 2014

Kyösti Mälkki (kyosti.malkki(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6166 -gerrit commit 544d1c1b6315e350ddf21601f51674926e6c95b5 Author: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Date: Mon Jun 30 13:48:18 2014 +0300 x86 MTRR: Drop unused return value It was never well-defined what value this function should return. Change-Id: If84aff86e0b556591d7ad557842910a2dfcd3b46 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- src/cpu/x86/mtrr/mtrr.c | 3 +-- src/include/cpu/x86/mtrr.h | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cpu/x86/mtrr/mtrr.c b/src/cpu/x86/mtrr/mtrr.c index 8826535..b19b853 100644 --- a/src/cpu/x86/mtrr/mtrr.c +++ b/src/cpu/x86/mtrr/mtrr.c @@ -772,7 +772,7 @@ void x86_setup_mtrrs(void) x86_setup_var_mtrrs(address_size, 1); } -int x86_mtrr_check(void) +void x86_mtrr_check(void) { /* Only Pentium Pro and later have MTRR */ msr_t msr; @@ -795,5 +795,4 @@ int x86_mtrr_check(void) printk(BIOS_DEBUG, "\n"); post_code(0x93); - return ((int) msr.lo); } diff --git a/src/include/cpu/x86/mtrr.h b/src/include/cpu/x86/mtrr.h index 2a44072..95cabf1 100644 --- a/src/include/cpu/x86/mtrr.h +++ b/src/include/cpu/x86/mtrr.h @@ -66,7 +66,7 @@ void enable_fixed_mtrr(void); void x86_setup_fixed_mtrrs(void); /* Set up fixed MTRRs but do not enable them. */ void x86_setup_fixed_mtrrs_no_enable(void); -int x86_mtrr_check(void); +void x86_mtrr_check(void); #endif #if !defined(__ASSEMBLER__) && defined(__PRE_RAM__) && !defined(__ROMCC__)

1 0

New patch to review for coreboot: 6004684 ROMCC: Fix collision with token name max
by Kyösti Mälkki June 30, 2014

June 30, 2014

Kyösti Mälkki (kyosti.malkki(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6165 -gerrit commit 6004684558cd5db85ee421538bdaf48c51aad407 Author: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Date: Mon Jun 30 13:15:42 2014 +0300 ROMCC: Fix collision with token name max Even with !defined(__ROMCC__) in the file, romcc chokes on these parameter names after we declare common max() macro in stdlib.h. Change-Id: Id4f2aa61d9c5b19f428452cd475b1b2ed9a70f52 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- src/include/device/device.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/include/device/device.h b/src/include/device/device.h index ced2786..ec17adf 100644 --- a/src/include/device/device.h +++ b/src/include/device/device.h @@ -45,7 +45,7 @@ struct device_operations { void (*enable_resources)(device_t dev); void (*init)(device_t dev); void (*final)(device_t dev); - unsigned int (*scan_bus)(device_t bus, unsigned int max); + unsigned int (*scan_bus)(device_t bus, unsigned int _max); void (*enable)(device_t dev); void (*disable)(device_t dev); void (*set_link)(device_t dev, unsigned int link); @@ -58,6 +58,7 @@ struct device_operations { const struct pci_bus_operations * (*ops_pci_bus)(device_t dev); const struct pnp_mode_ops *ops_pnp_mode; }; + #endif /* ! __SIMPLE_DEVICE__ */ @@ -157,7 +158,7 @@ void dev_finalize_chips(void); /* Generic device helper functions */ int reset_bus(struct bus *bus); -unsigned int scan_bus(struct device *bus, unsigned int max); +unsigned int scan_bus(struct device *bus, unsigned int _max); void assign_resources(struct bus *bus); void enumerate_static_device(void); void enumerate_static_devices(void); @@ -211,8 +212,8 @@ void show_all_devs_resources(int debug_level, const char* msg); extern struct device_operations default_dev_ops_root; void pci_domain_read_resources(struct device *dev); -unsigned int pci_domain_scan_bus(struct device *dev, unsigned int max); -unsigned int scan_static_bus(device_t bus, unsigned int max); +unsigned int pci_domain_scan_bus(struct device *dev, unsigned int _max); +unsigned int scan_static_bus(device_t bus, unsigned int _max); void fixed_mem_resource(device_t dev, unsigned long index, unsigned long basek, unsigned long sizek, unsigned long type);

1 0

Patch set updated for coreboot: f9a55c8 SPI: Split writes using spi_crop_chunk()
by Kyösti Mälkki June 30, 2014

June 30, 2014

Kyösti Mälkki (kyosti.malkki(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6163 -gerrit commit f9a55c8382370d650e8850307ccea5ab95e57063 Author: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Date: Sun Jun 29 16:17:33 2014 +0300 SPI: Split writes using spi_crop_chunk() SPI controllers in Intel and AMD bridges have a slightly different restriction on how long transactions they can handle. Change-Id: I3d149d4b7e7e9633482a153d5e380a86c553d871 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- src/drivers/spi/adesto.c | 4 ++-- src/drivers/spi/amic.c | 4 ++-- src/drivers/spi/gigadevice.c | 4 ++-- src/drivers/spi/macronix.c | 5 ++--- src/drivers/spi/stmicro.c | 4 ++-- src/drivers/spi/winbond.c | 4 ++-- src/include/spi-generic.h | 2 ++ src/include/spi_flash.h | 8 -------- src/soc/intel/baytrail/spi.c | 7 ++++++- src/soc/intel/fsp_baytrail/spi.c | 7 ++++++- src/southbridge/amd/agesa/hudson/spi.c | 9 +++++++++ src/southbridge/amd/cimx/sb800/spi.c | 10 ++++++++++ src/southbridge/intel/common/spi.c | 7 ++++++- 13 files changed, 51 insertions(+), 24 deletions(-) diff --git a/src/drivers/spi/adesto.c b/src/drivers/spi/adesto.c index 9dc8e14..50e5883 100644 --- a/src/drivers/spi/adesto.c +++ b/src/drivers/spi/adesto.c @@ -100,7 +100,7 @@ static int adesto_write(struct spi_flash *flash, for (actual = 0; actual < len; actual += chunk_len) { chunk_len = min(len - actual, page_size - byte_addr); - chunk_len = min(chunk_len, CONTROLLER_PAGE_LIMIT); + chunk_len = spi_crop_chunk(sizeof(cmd), chunk_len); cmd[0] = CMD_AT25DF_PP; cmd[1] = (offset >> 16) & 0xff; @@ -118,7 +118,7 @@ static int adesto_write(struct spi_flash *flash, goto out; } - ret = spi_flash_cmd_write(flash->spi, cmd, 4, + ret = spi_flash_cmd_write(flash->spi, cmd, sizeof(cmd), buf + actual, chunk_len); if (ret < 0) { printk(BIOS_WARNING, "SF: adesto Page Program failed\n"); diff --git a/src/drivers/spi/amic.c b/src/drivers/spi/amic.c index 4cccc71..63a68a5 100644 --- a/src/drivers/spi/amic.c +++ b/src/drivers/spi/amic.c @@ -82,7 +82,7 @@ static int amic_write(struct spi_flash *flash, for (actual = 0; actual < len; actual += chunk_len) { chunk_len = min(len - actual, page_size - byte_addr); - chunk_len = min(chunk_len, CONTROLLER_PAGE_LIMIT); + chunk_len = spi_crop_chunk(sizeof(cmd), chunk_len); cmd[0] = CMD_A25_PP; cmd[1] = (offset >> 16) & 0xff; @@ -100,7 +100,7 @@ static int amic_write(struct spi_flash *flash, goto out; } - ret = spi_flash_cmd_write(flash->spi, cmd, 4, + ret = spi_flash_cmd_write(flash->spi, cmd, sizeof(cmd), buf + actual, chunk_len); if (ret < 0) { printk(BIOS_WARNING, "SF: AMIC Page Program failed\n"); diff --git a/src/drivers/spi/gigadevice.c b/src/drivers/spi/gigadevice.c index 3fb89c7..783d321 100644 --- a/src/drivers/spi/gigadevice.c +++ b/src/drivers/spi/gigadevice.c @@ -141,7 +141,7 @@ static int gigadevice_write(struct spi_flash *flash, u32 offset, for (actual = 0; actual < len; actual += chunk_len) { chunk_len = min(len - actual, page_size - byte_addr); - chunk_len = min(chunk_len, CONTROLLER_PAGE_LIMIT); + chunk_len = spi_crop_chunk(sizeof(cmd), chunk_len); ret = spi_flash_cmd(flash->spi, CMD_GD25_WREN, NULL, 0); if (ret < 0) { @@ -161,7 +161,7 @@ static int gigadevice_write(struct spi_flash *flash, u32 offset, cmd[0], cmd[1], cmd[2], cmd[3], chunk_len); #endif - ret = spi_flash_cmd_write(flash->spi, cmd, 4, + ret = spi_flash_cmd_write(flash->spi, cmd, sizeof(cmd), buf + actual, chunk_len); if (ret < 0) { printk(BIOS_WARNING, diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c index 3611599..6e910f6 100644 --- a/src/drivers/spi/macronix.c +++ b/src/drivers/spi/macronix.c @@ -144,13 +144,12 @@ static int macronix_write(struct spi_flash *flash, ret = 0; for (actual = 0; actual < len; actual += chunk_len) { chunk_len = min(len - actual, page_size - byte_addr); - chunk_len = min(chunk_len, CONTROLLER_PAGE_LIMIT); + chunk_len = spi_crop_chunk(sizeof(cmd), chunk_len); cmd[0] = CMD_MX25XX_PP; cmd[1] = (offset >> 16) & 0xff; cmd[2] = (offset >> 8) & 0xff; cmd[3] = offset & 0xff; - #if CONFIG_DEBUG_SPI_FLASH printk(BIOS_SPEW, "PP: 0x%p => cmd = { 0x%02x 0x%02x%02x%02x }" " chunk_len = %zu\n", @@ -163,7 +162,7 @@ static int macronix_write(struct spi_flash *flash, break; } - ret = spi_flash_cmd_write(flash->spi, cmd, 4, + ret = spi_flash_cmd_write(flash->spi, cmd, sizeof(cmd), buf + actual, chunk_len); if (ret < 0) { printk(BIOS_WARNING, "SF: Macronix Page Program failed\n"); diff --git a/src/drivers/spi/stmicro.c b/src/drivers/spi/stmicro.c index c825bd0..bc7969d 100644 --- a/src/drivers/spi/stmicro.c +++ b/src/drivers/spi/stmicro.c @@ -155,7 +155,7 @@ static int stmicro_write(struct spi_flash *flash, for (actual = 0; actual < len; actual += chunk_len) { chunk_len = min(len - actual, page_size - byte_addr); - chunk_len = min(chunk_len, CONTROLLER_PAGE_LIMIT); + chunk_len = spi_crop_chunk(sizeof(cmd), chunk_len); cmd[0] = CMD_M25PXX_PP; cmd[1] = (offset >> 16) & 0xff; @@ -173,7 +173,7 @@ static int stmicro_write(struct spi_flash *flash, goto out; } - ret = spi_flash_cmd_write(flash->spi, cmd, 4, + ret = spi_flash_cmd_write(flash->spi, cmd, sizeof(cmd), buf + actual, chunk_len); if (ret < 0) { printk(BIOS_WARNING, "SF: STMicro Page Program failed\n"); diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index eb0868e..7ddd312 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -134,7 +134,7 @@ static int winbond_write(struct spi_flash *flash, for (actual = 0; actual < len; actual += chunk_len) { chunk_len = min(len - actual, page_size - byte_addr); - chunk_len = min(chunk_len, CONTROLLER_PAGE_LIMIT); + chunk_len = spi_crop_chunk(sizeof(cmd), chunk_len); cmd[0] = CMD_W25_PP; cmd[1] = (offset >> 16) & 0xff; @@ -152,7 +152,7 @@ static int winbond_write(struct spi_flash *flash, goto out; } - ret = spi_flash_cmd_write(flash->spi, cmd, 4, + ret = spi_flash_cmd_write(flash->spi, cmd, sizeof(cmd), buf + actual, chunk_len); if (ret < 0) { printk(BIOS_WARNING, "SF: Winbond Page Program failed\n"); diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h index 6cdb87a..e92e56c 100644 --- a/src/include/spi-generic.h +++ b/src/include/spi-generic.h @@ -181,6 +181,8 @@ void spi_cs_deactivate(struct spi_slave *slave); */ void spi_set_speed(struct spi_slave *slave, uint32_t hz); +unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len); + /*----------------------------------------------------------------------- * Write 8 bits, then read 8 bits. * slave: The SPI slave we're communicating with diff --git a/src/include/spi_flash.h b/src/include/spi_flash.h index 8e51471..b424b4b 100644 --- a/src/include/spi_flash.h +++ b/src/include/spi_flash.h @@ -39,14 +39,6 @@ const typeof( ((type *)0)->member ) *__mptr = (ptr); \ (type *)( (char *)__mptr - offsetof(type,member) );}) -#define CONFIG_ICH_SPI -#ifdef CONFIG_ICH_SPI -#define CONTROLLER_PAGE_LIMIT 64 -#else -/* any number larger than 4K would do, actually */ -#define CONTROLLER_PAGE_LIMIT ((int)(~0U>>1)) -#endif - struct spi_flash { struct spi_slave *spi; diff --git a/src/soc/intel/baytrail/spi.c b/src/soc/intel/baytrail/spi.c index 5ac42c2..b66f6dc 100644 --- a/src/soc/intel/baytrail/spi.c +++ b/src/soc/intel/baytrail/spi.c @@ -500,6 +500,11 @@ static int ich_status_poll(u16 bitmask, int wait_til_set) return -1; } +unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len) +{ + return min(cntlr.databytes, buf_len); +} + int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int bitsout, void *din, unsigned int bitsin) { @@ -594,7 +599,7 @@ int spi_xfer(struct spi_slave *slave, const void *dout, */ if (trans.bytesout > cntlr.databytes) { printk(BIOS_DEBUG, "ICH SPI: Too much to write. Does your SPI chip driver use" - " CONTROLLER_PAGE_LIMIT?\n"); + " spi_crop_chunk()?\n"); return -1; } diff --git a/src/soc/intel/fsp_baytrail/spi.c b/src/soc/intel/fsp_baytrail/spi.c index ddec2e0..c12dd52 100644 --- a/src/soc/intel/fsp_baytrail/spi.c +++ b/src/soc/intel/fsp_baytrail/spi.c @@ -498,6 +498,11 @@ static int ich_status_poll(u16 bitmask, int wait_til_set) return -1; } +unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len) +{ + return min(cntlr.databytes, buf_len); +} + int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int bitsout, void *din, unsigned int bitsin) { @@ -591,7 +596,7 @@ int spi_xfer(struct spi_slave *slave, const void *dout, */ if (trans.bytesout > cntlr.databytes) { printk(BIOS_DEBUG, "ICH SPI: Too much to write. Does your SPI chip driver use" - " CONTROLLER_PAGE_LIMIT?\n"); + " spi_crop_chunk()?\n"); return -1; } diff --git a/src/southbridge/amd/agesa/hudson/spi.c b/src/southbridge/amd/agesa/hudson/spi.c index 573dd3d..de42699 100644 --- a/src/southbridge/amd/agesa/hudson/spi.c +++ b/src/southbridge/amd/agesa/hudson/spi.c @@ -42,6 +42,7 @@ static int bus_claimed = 0; #define SPI_REG_CNTRL11 0xd #define CNTRL11_FIFOPTR_MASK 0x07 +#define AMD_SB_SPI_TX_LEN 64 static u32 spibar; @@ -86,6 +87,14 @@ void spi_init(void) spibar = pci_read_config32(dev, 0xA0) & ~0x1F; } +unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len) +{ + /* Command byte does not consume buffer space. */ + if (cmd_len) + cmd_len--; + return min(AMD_SB_SPI_TX_LEN - cmd_len, buf_len); +} + int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int bitsout, void *din, unsigned int bitsin) { diff --git a/src/southbridge/amd/cimx/sb800/spi.c b/src/southbridge/amd/cimx/sb800/spi.c index a1cdf55..52e5d0c 100644 --- a/src/southbridge/amd/cimx/sb800/spi.c +++ b/src/southbridge/amd/cimx/sb800/spi.c @@ -32,6 +32,8 @@ static int bus_claimed = 0; #endif +#define AMD_SB_SPI_TX_LEN 8 + static u32 spibar; static void reset_internal_fifo_pointer(void) @@ -56,6 +58,14 @@ void spi_init() spibar = pci_read_config32(dev, 0xA0) & ~0x1F; } +unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len) +{ + /* Command byte does not consume buffer space. */ + if (cmd_len) + cmd_len--; + return min(AMD_SB_SPI_TX_LEN - cmd_len, buf_len); +} + int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int bitsout, void *din, unsigned int bitsin) { diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c index 51586cc..8db134a 100644 --- a/src/southbridge/intel/common/spi.c +++ b/src/southbridge/intel/common/spi.c @@ -551,6 +551,11 @@ static int spi_is_multichip (void) return !!((cntlr.flmap0 >> 8) & 3); } +unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len) +{ + return min(cntlr.databytes, buf_len); +} + int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int bitsout, void *din, unsigned int bitsin) { @@ -645,7 +650,7 @@ int spi_xfer(struct spi_slave *slave, const void *dout, */ if (trans.bytesout > cntlr.databytes) { printk(BIOS_DEBUG, "ICH SPI: Too much to write. Does your SPI chip driver use" - " CONTROLLER_PAGE_LIMIT?\n"); + " spi_crop_chunk()?\n"); return -1; }

1 0

Patch set updated for coreboot: 406c350 AMD SPI: Optimise for longer writes
by Kyösti Mälkki June 30, 2014

June 30, 2014

Kyösti Mälkki (kyosti.malkki(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6164 -gerrit commit 406c350d0b7bc766cb3ff11e05a0b7f435377c06 Author: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Date: Mon Jun 30 07:34:36 2014 +0300 AMD SPI: Optimise for longer writes Leave it to the implementation of flash->write() to split the writes to match SPI controller and SPI flash part restrictions. This allows for some optimisation for auto-address-increment (AAI) commands. Kconfig AMD_SB_SPI_TX_LEN can be kept as local. Change-Id: I4a8bc55ab7eb0eeda8f25003a8f5ff2a643ab7a7 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- src/cpu/amd/agesa/spi.c | 7 +------ src/southbridge/amd/Kconfig | 9 --------- src/southbridge/amd/agesa/hudson/spi.c | 14 ++++++++++++++ src/southbridge/amd/cimx/sb800/spi.c | 14 ++++++++++++++ 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/cpu/amd/agesa/spi.c b/src/cpu/amd/agesa/spi.c index f4ffa62..78ddf5e 100644 --- a/src/cpu/amd/agesa/spi.c +++ b/src/cpu/amd/agesa/spi.c @@ -40,12 +40,7 @@ void spi_SaveS3info(u32 pos, u32 size, u8 *buf, u32 len) flash->erase(flash, pos, size); flash->write(flash, pos, sizeof(len), &len); - - u32 nvram_pos; - for (nvram_pos = 0; nvram_pos < len - CONFIG_AMD_SB_SPI_TX_LEN; nvram_pos += CONFIG_AMD_SB_SPI_TX_LEN) { - flash->write(flash, nvram_pos + pos + 4, CONFIG_AMD_SB_SPI_TX_LEN, (u8 *)(buf + nvram_pos)); - } - flash->write(flash, nvram_pos + pos + 4, len % CONFIG_AMD_SB_SPI_TX_LEN, (u8 *)(buf + nvram_pos)); + flash->write(flash, pos + sizeof(len), len, buf); flash->spi->rw = SPI_WRITE_FLAG; spi_release_bus(flash->spi); diff --git a/src/southbridge/amd/Kconfig b/src/southbridge/amd/Kconfig index 39aeb09..867afca 100644 --- a/src/southbridge/amd/Kconfig +++ b/src/southbridge/amd/Kconfig @@ -14,12 +14,3 @@ source src/southbridge/amd/sb800/Kconfig source src/southbridge/amd/cimx/Kconfig source src/southbridge/amd/agesa/Kconfig source src/southbridge/amd/sr5650/Kconfig - -if CPU_AMD_AGESA - -config AMD_SB_SPI_TX_LEN - int - default 4 - depends on SPI_FLASH - -endif diff --git a/src/southbridge/amd/agesa/hudson/spi.c b/src/southbridge/amd/agesa/hudson/spi.c index de42699..5492140 100644 --- a/src/southbridge/amd/agesa/hudson/spi.c +++ b/src/southbridge/amd/agesa/hudson/spi.c @@ -20,6 +20,7 @@ #include <stdlib.h> #include <string.h> #include <arch/io.h> +#include <console/console.h> #include <spi-generic.h> #include <device/device.h> #include <device/pci.h> @@ -111,6 +112,19 @@ int spi_xfer(struct spi_slave *slave, const void *dout, bytesout = bitsout / 8; bytesin = bitsin / 8; + /* + * Check if this is a write command attempting to transfer more bytes + * than the controller can handle. Iterations for writes are not + * supported here because each SPI write command needs to be preceded + * and followed by other SPI commands, and this sequence is controlled + * by the SPI chip driver. + */ + if (bytesout > AMD_SB_SPI_TX_LEN) { + printk(BIOS_DEBUG, "FCH SPI: Too much to write. Does your SPI chip driver use" + " spi_crop_chunk()?\n"); + return -1; + } + readoffby1 = bytesout ? 0 : 1; #if CONFIG_SOUTHBRIDGE_AMD_AGESA_YANGTZE diff --git a/src/southbridge/amd/cimx/sb800/spi.c b/src/southbridge/amd/cimx/sb800/spi.c index 52e5d0c..512c26d 100644 --- a/src/southbridge/amd/cimx/sb800/spi.c +++ b/src/southbridge/amd/cimx/sb800/spi.c @@ -20,6 +20,7 @@ #include <stdlib.h> #include <string.h> #include <arch/io.h> +#include <console/console.h> #include <spi-generic.h> #include <device/device.h> #include <device/pci.h> @@ -80,6 +81,19 @@ int spi_xfer(struct spi_slave *slave, const void *dout, bytesout = bitsout / 8; bytesin = bitsin / 8; + /* + * Check if this is a write command attempting to transfer more bytes + * than the controller can handle. Iterations for writes are not + * supported here because each SPI write command needs to be preceded + * and followed by other SPI commands, and this sequence is controlled + * by the SPI chip driver. + */ + if (bytesout > AMD_SB_SPI_TX_LEN) { + printk(BIOS_DEBUG, "FCH SPI: Too much to write. Does your SPI chip driver use" + " spi_crop_chunk()?\n"); + return -1; + } + readoffby1 = bytesout ? 0 : 1; readwrite = (bytesin + readoffby1) << 4 | bytesout;

1 0