From e68aeb0c8e54838c8206e6b9ac9a4bb12d5edf27 Mon Sep 17 00:00:00 2001 From: Alessio Morale Date: Wed, 27 Mar 2013 00:26:47 +0100 Subject: [PATCH 1/3] OP-881 Make PIOS_Flash_Jedec_Init able to configure the flash chip based on Jedec Id returned using a catalog of known flash chips. +review OPReview --- flight/PiOS/Common/pios_flash_jedec.c | 25 +++++-- flight/PiOS/inc/pios_flash_jedec_catalog.h | 71 +++++++++++++++++++ flight/PiOS/inc/pios_flash_jedec_priv.h | 2 +- .../targets/CopterControl/System/pios_board.c | 4 +- flight/targets/RevoMini/System/pios_board.c | 9 ++- flight/targets/Revolution/System/pios_board.c | 4 +- .../coptercontrol/board_hw_defs.c | 16 +---- .../board_hw_defs/revolution/board_hw_defs.c | 8 --- .../board_hw_defs/revomini/board_hw_defs.c | 8 --- 9 files changed, 102 insertions(+), 45 deletions(-) create mode 100644 flight/PiOS/inc/pios_flash_jedec_catalog.h diff --git a/flight/PiOS/Common/pios_flash_jedec.c b/flight/PiOS/Common/pios_flash_jedec.c index 6d382a1c1..e9cb32bb9 100644 --- a/flight/PiOS/Common/pios_flash_jedec.c +++ b/flight/PiOS/Common/pios_flash_jedec.c @@ -30,6 +30,7 @@ */ #include "pios.h" #include "pios_flash_jedec_priv.h" +#include "pios_flash_jedec_catalog.h" #define JEDEC_WRITE_ENABLE 0x06 #define JEDEC_WRITE_DISABLE 0x04 @@ -115,7 +116,7 @@ static int32_t PIOS_Flash_Jedec_Validate(struct jedec_flash_dev * flash_dev) { /** * @brief Initialize the flash device and enable write access */ -int32_t PIOS_Flash_Jedec_Init(uintptr_t * flash_id, uint32_t spi_id, uint32_t slave_num, const struct pios_flash_jedec_cfg * cfg) +int32_t PIOS_Flash_Jedec_Init(uintptr_t * flash_id, uint32_t spi_id, uint32_t slave_num) { struct jedec_flash_dev * flash_dev = PIOS_Flash_Jedec_alloc(); if (flash_dev == NULL) @@ -123,16 +124,26 @@ int32_t PIOS_Flash_Jedec_Init(uintptr_t * flash_id, uint32_t spi_id, uint32_t sl flash_dev->spi_id = spi_id; flash_dev->slave_num = slave_num; - flash_dev->cfg = cfg; + flash_dev->cfg = 0; (void) PIOS_Flash_Jedec_ReadID(flash_dev); - if ((flash_dev->manufacturer != flash_dev->cfg->expect_manufacturer) || - (flash_dev->memorytype != flash_dev->cfg->expect_memorytype) || - (flash_dev->capacity != flash_dev->cfg->expect_capacity)) { - /* Mismatched device has been discovered */ - return -1; + uint32_t i = 0; + while(i < pios_flash_jedec_catalog_size) + { + const struct pios_flash_jedec_cfg flash_jedec_entry = pios_flash_jedec_catalog[i]; + + if ((flash_dev->manufacturer == flash_jedec_entry.expect_manufacturer) && + (flash_dev->memorytype == flash_jedec_entry.expect_memorytype) && + (flash_dev->capacity == flash_jedec_entry.expect_capacity)) + { + flash_dev->cfg = &pios_flash_jedec_catalog[i]; + } + i++; } + if(flash_dev->cfg == 0) + return -1; + /* Give back a handle to this flash device */ *flash_id = (uintptr_t) flash_dev; diff --git a/flight/PiOS/inc/pios_flash_jedec_catalog.h b/flight/PiOS/inc/pios_flash_jedec_catalog.h new file mode 100644 index 000000000..4c6349be0 --- /dev/null +++ b/flight/PiOS/inc/pios_flash_jedec_catalog.h @@ -0,0 +1,71 @@ +/** + ****************************************************************************** + * + * @addtogroup PIOS PIOS Core hardware abstraction layer + * @{ + * @addtogroup PIOS_FLASH JEDEC devices catalog + * @{ + * + * @file pios_flash_jedec_catalog.h + * @author The OpenPilot Team, http://www.openpilot.org Copyright (C) 2013. + * @brief Driver for talking to most JEDEC flash chips + * @see The GNU Public License (GPL) Version 3 + * + *****************************************************************************/ +/* + * 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; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + + +#ifndef PIOS_FLASH_JEDEC_CATALOG_H_ +#define PIOS_FLASH_JEDEC_CATALOG_H_ +#include + +// this structure contains the catalog of all "known" flash chip used +const struct pios_flash_jedec_cfg pios_flash_jedec_catalog [] = +{ + { // m25p16 + .expect_manufacturer = JEDEC_MANUFACTURER_ST, + .expect_memorytype = 0x20, + .expect_capacity = 0x15, + .sector_erase = 0xD8, + .chip_erase = 0xC7, + }, + { // m25px16 + .expect_manufacturer = JEDEC_MANUFACTURER_ST, + .expect_memorytype = 0x71, + .expect_capacity = 0x15, + .sector_erase = 0xD8, + .chip_erase = 0xC7, + }, + { //w25x + .expect_manufacturer = JEDEC_MANUFACTURER_WINBOND, + .expect_memorytype = 0x30, + .expect_capacity = 0x13, + .sector_erase = 0x20, + .chip_erase = 0x60 + }, + { //25q16 + .expect_manufacturer = JEDEC_MANUFACTURER_WINBOND, + .expect_memorytype = 0x40, + .expect_capacity = 0x15, + .sector_erase = 0x20, + .chip_erase = 0x60 + } +}; +const uint32_t pios_flash_jedec_catalog_size = NELEMENTS(pios_flash_jedec_catalog); + + +#endif /* PIOS_FLASH_JEDEC_CATALOG_H_ */ diff --git a/flight/PiOS/inc/pios_flash_jedec_priv.h b/flight/PiOS/inc/pios_flash_jedec_priv.h index 8b772c0b9..c6a5c79a7 100644 --- a/flight/PiOS/inc/pios_flash_jedec_priv.h +++ b/flight/PiOS/inc/pios_flash_jedec_priv.h @@ -47,6 +47,6 @@ struct pios_flash_jedec_cfg { uint32_t chip_erase; }; -int32_t PIOS_Flash_Jedec_Init(uintptr_t * flash_id, uint32_t spi_id, uint32_t slave_num, const struct pios_flash_jedec_cfg * cfg); +int32_t PIOS_Flash_Jedec_Init(uintptr_t * flash_id, uint32_t spi_id, uint32_t slave_num); #endif /* PIOS_FLASH_JEDEC_H_ */ diff --git a/flight/targets/CopterControl/System/pios_board.c b/flight/targets/CopterControl/System/pios_board.c index 2656bfab1..c7ae0e494 100644 --- a/flight/targets/CopterControl/System/pios_board.c +++ b/flight/targets/CopterControl/System/pios_board.c @@ -169,11 +169,11 @@ void PIOS_Board_Init(void) { uintptr_t fs_id; switch(bdinfo->board_rev) { case BOARD_REVISION_CC: - PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_accel_id, 1, &flash_w25x_cfg); + PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_accel_id, 1); PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_w25x_cfg, &pios_jedec_flash_driver, flash_id); break; case BOARD_REVISION_CC3D: - PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_accel_id, 0, &flash_m25p_cfg); + PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_accel_id, 0); PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_m25p_cfg, &pios_jedec_flash_driver, flash_id); break; default: diff --git a/flight/targets/RevoMini/System/pios_board.c b/flight/targets/RevoMini/System/pios_board.c index be334adad..c15b459b6 100644 --- a/flight/targets/RevoMini/System/pios_board.c +++ b/flight/targets/RevoMini/System/pios_board.c @@ -336,9 +336,14 @@ void PIOS_Board_Init(void) { #if defined(PIOS_INCLUDE_FLASH) /* Connect flash to the appropriate interface and configure it */ uintptr_t flash_id; - PIOS_Flash_Jedec_Init(&flash_id, pios_spi_telem_flash_id, 1, &flash_m25p_cfg); + if (PIOS_Flash_Jedec_Init(&flash_id, pios_spi_telem_flash_id, 1)) + PIOS_DEBUG_Assert(0); + uintptr_t fs_id; - PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_m25p_cfg, &pios_jedec_flash_driver, flash_id); + + if (PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_m25p_cfg, &pios_jedec_flash_driver, flash_id)) + PIOS_DEBUG_Assert(0); + #endif /* Initialize UAVObject libraries */ diff --git a/flight/targets/Revolution/System/pios_board.c b/flight/targets/Revolution/System/pios_board.c index 317919ef6..3d1bae3c2 100644 --- a/flight/targets/Revolution/System/pios_board.c +++ b/flight/targets/Revolution/System/pios_board.c @@ -386,11 +386,11 @@ void PIOS_Board_Init(void) { } /* Connect flash to the appropriate interface and configure it */ uintptr_t flash_id; - PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_id, 0, &flash_m25p_cfg); + PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_id, 0); #else /* Connect flash to the appropriate interface and configure it */ uintptr_t flash_id; - PIOS_Flash_Jedec_Init(&flash_id, pios_spi_accel_id, 1, &flash_m25p_cfg); + PIOS_Flash_Jedec_Init(&flash_id, pios_spi_accel_id, 1); #endif uintptr_t fs_id; PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_m25p_cfg, &pios_jedec_flash_driver, flash_id); diff --git a/flight/targets/board_hw_defs/coptercontrol/board_hw_defs.c b/flight/targets/board_hw_defs/coptercontrol/board_hw_defs.c index 19408858c..368084a5c 100644 --- a/flight/targets/board_hw_defs/coptercontrol/board_hw_defs.c +++ b/flight/targets/board_hw_defs/coptercontrol/board_hw_defs.c @@ -414,13 +414,7 @@ static const struct flashfs_logfs_cfg flashfs_w25x_cfg = { .page_size = 0x00000100, /* 256 bytes */ }; -static const struct pios_flash_jedec_cfg flash_w25x_cfg = { - .expect_manufacturer = JEDEC_MANUFACTURER_WINBOND, - .expect_memorytype = 0x30, - .expect_capacity = 0x13, - .sector_erase = 0x20, - .chip_erase = 0x60 -}; + static const struct flashfs_logfs_cfg flashfs_m25p_cfg = { .fs_magic = 0x99abceef, @@ -433,14 +427,6 @@ static const struct flashfs_logfs_cfg flashfs_m25p_cfg = { .page_size = 0x00000100, /* 256 bytes */ }; -static const struct pios_flash_jedec_cfg flash_m25p_cfg = { - .expect_manufacturer = JEDEC_MANUFACTURER_ST, - .expect_memorytype = 0x20, - .expect_capacity = 0x15, - .sector_erase = 0xD8, - .chip_erase = 0xC7, -}; - #include "pios_flash.h" #endif /* PIOS_INCLUDE_FLASH */ diff --git a/flight/targets/board_hw_defs/revolution/board_hw_defs.c b/flight/targets/board_hw_defs/revolution/board_hw_defs.c index 75128549f..642f06052 100644 --- a/flight/targets/board_hw_defs/revolution/board_hw_defs.c +++ b/flight/targets/board_hw_defs/revolution/board_hw_defs.c @@ -462,14 +462,6 @@ static const struct flashfs_logfs_cfg flashfs_m25p_cfg = { .page_size = 0x00000100, /* 256 bytes */ }; -static const struct pios_flash_jedec_cfg flash_m25p_cfg = { - .expect_manufacturer = JEDEC_MANUFACTURER_ST, - .expect_memorytype = 0x20, - .expect_capacity = 0x15, - .sector_erase = 0xD8, - .chip_erase = 0xC7, -}; - #endif /* PIOS_INCLUDE_FLASH */ #if defined(PIOS_OVERO_SPI) diff --git a/flight/targets/board_hw_defs/revomini/board_hw_defs.c b/flight/targets/board_hw_defs/revomini/board_hw_defs.c index a19e904e2..d58d1a01d 100644 --- a/flight/targets/board_hw_defs/revomini/board_hw_defs.c +++ b/flight/targets/board_hw_defs/revomini/board_hw_defs.c @@ -567,14 +567,6 @@ static const struct flashfs_logfs_cfg flashfs_m25p_cfg = { .page_size = 0x00000100, /* 256 bytes */ }; -static const struct pios_flash_jedec_cfg flash_m25p_cfg = { - .expect_manufacturer = JEDEC_MANUFACTURER_ST, - .expect_memorytype = 0x20, - .expect_capacity = 0x15, - .sector_erase = 0xD8, - .chip_erase = 0xC7, -}; - #endif /* PIOS_INCLUDE_FLASH */ #include From 46e9e60e76ebc1727457c6bff1821415434a304f Mon Sep 17 00:00:00 2001 From: Alessio Morale Date: Fri, 29 Mar 2013 00:30:28 +0100 Subject: [PATCH 2/3] OP-881 fixed for stile compliance, added test and assertions on flash init for revolution and CC targets +review OPReview-423 --- flight/PiOS/Common/pios_flash_jedec.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/flight/PiOS/Common/pios_flash_jedec.c b/flight/PiOS/Common/pios_flash_jedec.c index e9cb32bb9..864560dfa 100644 --- a/flight/PiOS/Common/pios_flash_jedec.c +++ b/flight/PiOS/Common/pios_flash_jedec.c @@ -124,11 +124,11 @@ int32_t PIOS_Flash_Jedec_Init(uintptr_t * flash_id, uint32_t spi_id, uint32_t sl flash_dev->spi_id = spi_id; flash_dev->slave_num = slave_num; - flash_dev->cfg = 0; + flash_dev->cfg = NULL; (void) PIOS_Flash_Jedec_ReadID(flash_dev); uint32_t i = 0; - while(i < pios_flash_jedec_catalog_size) + while ((i < pios_flash_jedec_catalog_size) && !flash_dev->cfg) { const struct pios_flash_jedec_cfg flash_jedec_entry = pios_flash_jedec_catalog[i]; @@ -141,8 +141,10 @@ int32_t PIOS_Flash_Jedec_Init(uintptr_t * flash_id, uint32_t spi_id, uint32_t sl i++; } - if(flash_dev->cfg == 0) + if(!flash_dev->cfg) + { return -1; + } /* Give back a handle to this flash device */ *flash_id = (uintptr_t) flash_dev; From ca0910370f9e64fa70d5d15a8b5105d24d62fc47 Mon Sep 17 00:00:00 2001 From: Oleg Semyonov Date: Fri, 29 Mar 2013 11:26:56 +0200 Subject: [PATCH 3/3] OP-881: flashfs: coding style fixes according to OPReview-423 +review OPReview-423 --- flight/PiOS/Common/pios_flash_jedec.c | 20 +++++++++---------- .../targets/CopterControl/System/pios_board.c | 16 +++++++++++---- flight/targets/RevoMini/System/pios_board.c | 7 ++++--- flight/targets/Revolution/System/pios_board.c | 12 ++++++++--- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/flight/PiOS/Common/pios_flash_jedec.c b/flight/PiOS/Common/pios_flash_jedec.c index 864560dfa..06731f383 100644 --- a/flight/PiOS/Common/pios_flash_jedec.c +++ b/flight/PiOS/Common/pios_flash_jedec.c @@ -119,30 +119,28 @@ static int32_t PIOS_Flash_Jedec_Validate(struct jedec_flash_dev * flash_dev) { int32_t PIOS_Flash_Jedec_Init(uintptr_t * flash_id, uint32_t spi_id, uint32_t slave_num) { struct jedec_flash_dev * flash_dev = PIOS_Flash_Jedec_alloc(); - if (flash_dev == NULL) + if (!flash_dev) { return -1; + } flash_dev->spi_id = spi_id; flash_dev->slave_num = slave_num; flash_dev->cfg = NULL; (void) PIOS_Flash_Jedec_ReadID(flash_dev); - uint32_t i = 0; - while ((i < pios_flash_jedec_catalog_size) && !flash_dev->cfg) - { + + for (uint32_t i = 0; i < pios_flash_jedec_catalog_size; ++i) { const struct pios_flash_jedec_cfg flash_jedec_entry = pios_flash_jedec_catalog[i]; - if ((flash_dev->manufacturer == flash_jedec_entry.expect_manufacturer) && - (flash_dev->memorytype == flash_jedec_entry.expect_memorytype) && - (flash_dev->capacity == flash_jedec_entry.expect_capacity)) - { + if ((flash_dev->manufacturer == flash_jedec_entry.expect_manufacturer) + && (flash_dev->memorytype == flash_jedec_entry.expect_memorytype) + && (flash_dev->capacity == flash_jedec_entry.expect_capacity)) { flash_dev->cfg = &pios_flash_jedec_catalog[i]; + break; } - i++; } - if(!flash_dev->cfg) - { + if (!flash_dev->cfg) { return -1; } diff --git a/flight/targets/CopterControl/System/pios_board.c b/flight/targets/CopterControl/System/pios_board.c index c7ae0e494..86ee24309 100644 --- a/flight/targets/CopterControl/System/pios_board.c +++ b/flight/targets/CopterControl/System/pios_board.c @@ -169,12 +169,20 @@ void PIOS_Board_Init(void) { uintptr_t fs_id; switch(bdinfo->board_rev) { case BOARD_REVISION_CC: - PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_accel_id, 1); - PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_w25x_cfg, &pios_jedec_flash_driver, flash_id); + if (PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_accel_id, 1)) { + PIOS_DEBUG_Assert(0); + } + if (PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_w25x_cfg, &pios_jedec_flash_driver, flash_id)) { + PIOS_DEBUG_Assert(0); + } break; case BOARD_REVISION_CC3D: - PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_accel_id, 0); - PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_m25p_cfg, &pios_jedec_flash_driver, flash_id); + if (PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_accel_id, 0)) { + PIOS_DEBUG_Assert(0); + } + if (PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_m25p_cfg, &pios_jedec_flash_driver, flash_id)) { + PIOS_DEBUG_Assert(0); + } break; default: PIOS_DEBUG_Assert(0); diff --git a/flight/targets/RevoMini/System/pios_board.c b/flight/targets/RevoMini/System/pios_board.c index c15b459b6..43a8813f0 100644 --- a/flight/targets/RevoMini/System/pios_board.c +++ b/flight/targets/RevoMini/System/pios_board.c @@ -336,13 +336,14 @@ void PIOS_Board_Init(void) { #if defined(PIOS_INCLUDE_FLASH) /* Connect flash to the appropriate interface and configure it */ uintptr_t flash_id; - if (PIOS_Flash_Jedec_Init(&flash_id, pios_spi_telem_flash_id, 1)) + if (PIOS_Flash_Jedec_Init(&flash_id, pios_spi_telem_flash_id, 1)) { PIOS_DEBUG_Assert(0); + } uintptr_t fs_id; - - if (PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_m25p_cfg, &pios_jedec_flash_driver, flash_id)) + if (PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_m25p_cfg, &pios_jedec_flash_driver, flash_id)) { PIOS_DEBUG_Assert(0); + } #endif diff --git a/flight/targets/Revolution/System/pios_board.c b/flight/targets/Revolution/System/pios_board.c index 3d1bae3c2..f68118801 100644 --- a/flight/targets/Revolution/System/pios_board.c +++ b/flight/targets/Revolution/System/pios_board.c @@ -386,14 +386,20 @@ void PIOS_Board_Init(void) { } /* Connect flash to the appropriate interface and configure it */ uintptr_t flash_id; - PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_id, 0); + if (PIOS_Flash_Jedec_Init(&flash_id, pios_spi_flash_id, 0)) { + PIOS_DEBUG_Assert(0); + } #else /* Connect flash to the appropriate interface and configure it */ uintptr_t flash_id; - PIOS_Flash_Jedec_Init(&flash_id, pios_spi_accel_id, 1); + if (PIOS_Flash_Jedec_Init(&flash_id, pios_spi_accel_id, 1)) { + PIOS_DEBUG_Assert(0); + } #endif uintptr_t fs_id; - PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_m25p_cfg, &pios_jedec_flash_driver, flash_id); + if (PIOS_FLASHFS_Logfs_Init(&fs_id, &flashfs_m25p_cfg, &pios_jedec_flash_driver, flash_id)) { + PIOS_DEBUG_Assert(0); + } /* Initialize UAVObject libraries */ EventDispatcherInitialize();