Skip to content

drivers/flash: Add flash_get_parameters call to flash API #25947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 22, 2020
Merged

drivers/flash: Add flash_get_parameters call to flash API #25947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aad5528
drivers/flash: Add API call returning flash parameters
de-nordic Jun 3, 2020
4e9e14d
drivers/flash: Add support for flash_get_parameters to drivers
de-nordic Jun 3, 2020
c273621
subsys/fs/nvs: Use flash_get_parameters() to get erase value
de-nordic Jun 3, 2020
763a9ec
drivers/flash: Move write_block_size into flash_parameters
de-nordic Jun 3, 2020
a946de6
subsys/fs/nvs: Move write_block_size to flash_parameters
de-nordic Jun 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions boards/posix/native_posix/native_posix.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#address-cells = <1>;
#size-cells = <1>;
erase-value = <0xff>;

label = "flash_ctrl";

Expand Down
1 change: 1 addition & 0 deletions boards/x86/qemu_x86/qemu_x86.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@

#address-cells = <1>;
#size-cells = <1>;
erase-value = <0xff>;

flash_sim0: flash_sim@0 {
compatible = "soc-nv-flash";
Expand Down
16 changes: 15 additions & 1 deletion drivers/flash/flash_gecko.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ struct flash_gecko_data {
struct k_sem mutex;
};


static const struct flash_parameters flash_gecko_parameters = {
.write_block_size = DT_PROP(SOC_NV_FLASH_NODE, write_block_size),
.erase_value = 0xff,
};

#define DEV_NAME(dev) ((dev)->name)
#define DEV_DATA(dev) \
((struct flash_gecko_data *const)(dev)->driver_data)
Expand Down Expand Up @@ -178,6 +184,14 @@ void flash_gecko_page_layout(struct device *dev,
}
#endif /* CONFIG_FLASH_PAGE_LAYOUT */

static const struct flash_parameters *
flash_gecko_get_parameters(const struct device *dev)
{
ARG_UNUSED(dev);

return &flash_gecko_parameters;
}

static int flash_gecko_init(struct device *dev)
{
struct flash_gecko_data *const dev_data = DEV_DATA(dev);
Expand All @@ -199,10 +213,10 @@ static const struct flash_driver_api flash_gecko_driver_api = {
.write = flash_gecko_write,
.erase = flash_gecko_erase,
.write_protection = flash_gecko_write_protection,
.get_parameters = flash_gecko_get_parameters,
#ifdef CONFIG_FLASH_PAGE_LAYOUT
.page_layout = flash_gecko_page_layout,
#endif
.write_block_size = DT_PROP(SOC_NV_FLASH_NODE, write_block_size),
};

static struct flash_gecko_data flash_gecko_0_data;
Expand Down
15 changes: 14 additions & 1 deletion drivers/flash/flash_sam.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ struct flash_sam_dev_data {
struct k_sem sem;
};

static const struct flash_parameters flash_sam_parameters = {
.write_block_size = FLASH_WRITE_BLK_SZ,
.erase_value = 0xff,
};

#define DEV_CFG(dev) \
((const struct flash_sam_dev_cfg *const)(dev)->config_info)

Expand Down Expand Up @@ -330,6 +335,14 @@ void flash_sam_page_layout(struct device *dev,
}
#endif

static const struct flash_parameters *
flash_sam_get_parameters(const struct device *dev)
{
ARG_UNUSED(dev);

return &flash_sam_parameters;
}

static int flash_sam_init(struct device *dev)
{
struct flash_sam_dev_data *const data = DEV_DATA(dev);
Expand All @@ -344,10 +357,10 @@ static const struct flash_driver_api flash_sam_api = {
.erase = flash_sam_erase,
.write = flash_sam_write,
.read = flash_sam_read,
.get_parameters = flash_sim_get_parameters,
#ifdef CONFIG_FLASH_PAGE_LAYOUT
.page_layout = flash_sam_page_layout,
#endif
.write_block_size = FLASH_WRITE_BLK_SZ,
};

static const struct flash_sam_dev_cfg flash_sam_cfg = {
Expand Down
23 changes: 18 additions & 5 deletions drivers/flash/flash_sam0.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ static const struct flash_pages_layout flash_sam0_pages_layout = {
};
#endif

static const struct flash_parameters flash_sam0_parameters = {
#if CONFIG_SOC_FLASH_SAM0_EMULATE_BYTE_PAGES
.write_block_size = 1,
#else
.write_block_size = FLASH_PAGE_SIZE,
#endif
.erase_value = 0xff,
};

static inline void flash_sam0_sem_take(struct device *dev)
{
struct flash_sam0_data *ctx = dev->driver_data;
Expand Down Expand Up @@ -388,6 +397,14 @@ void flash_sam0_page_layout(struct device *dev,
}
#endif

static const struct flash_parameters *
flash_sam0_get_parameters(const struct device *dev)
{
ARG_UNUSED(dev);

return &flash_sam0_parameters;
}

static int flash_sam0_init(struct device *dev)
{
struct flash_sam0_data *ctx = dev->driver_data;
Expand All @@ -414,14 +431,10 @@ static const struct flash_driver_api flash_sam0_api = {
.erase = flash_sam0_erase,
.write = flash_sam0_write,
.read = flash_sam0_read,
.get_parameters = flash_sam0_get_parameters,
#ifdef CONFIG_FLASH_PAGE_LAYOUT
.page_layout = flash_sam0_page_layout,
#endif
#if CONFIG_SOC_FLASH_SAM0_EMULATE_BYTE_PAGES
.write_block_size = 1,
#else
.write_block_size = FLASH_PAGE_SIZE,
#endif
};

static struct flash_sam0_data flash_sam0_data_0;
Expand Down
25 changes: 20 additions & 5 deletions drivers/flash/flash_simulator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
#define FLASH_SIMULATOR_FLASH_SIZE DT_REG_SIZE(SOC_NV_FLASH_NODE)

#define FLASH_SIMULATOR_DEV_NAME DT_INST_LABEL(0)
#define FLASH_SIMULATOR_ERASE_VALUE \
DT_PROP(DT_PARENT(SOC_NV_FLASH_NODE), erase_value)

#define FLASH_SIMULATOR_PAGE_COUNT (FLASH_SIMULATOR_FLASH_SIZE / \
FLASH_SIMULATOR_ERASE_UNIT)
Expand Down Expand Up @@ -139,6 +141,11 @@ static bool write_protection;

static const struct flash_driver_api flash_sim_api;

static const struct flash_parameters flash_sim_parameters = {
.write_block_size = FLASH_SIMULATOR_PROG_UNIT,
.erase_value = FLASH_SIMULATOR_ERASE_VALUE
};

static int flash_range_is_valid(struct device *dev, off_t offset, size_t len)
{
ARG_UNUSED(dev);
Expand Down Expand Up @@ -219,7 +226,7 @@ static int flash_sim_write(struct device *dev, const off_t offset,

uint8_t buf[FLASH_SIMULATOR_PROG_UNIT];

memset(buf, 0xFF, sizeof(buf));
memset(buf, FLASH_SIMULATOR_ERASE_VALUE, sizeof(buf));
if (memcmp(buf, FLASH(offset + i), sizeof(buf))) {
STATS_INC(flash_sim_stats, double_writes);
#if !CONFIG_FLASH_SIMULATOR_DOUBLE_WRITES
Expand Down Expand Up @@ -273,9 +280,9 @@ static void unit_erase(const uint32_t unit)
(unit * FLASH_SIMULATOR_ERASE_UNIT);

/* byte pattern to fill the flash with */
uint8_t byte_pattern = 0xFF;
uint8_t byte_pattern = FLASH_SIMULATOR_ERASE_VALUE;

/* erase the memory unit by pulling all bits to one */
/* erase the memory unit by setting it to erase value */
memset(FLASH(unit_addr), byte_pattern,
FLASH_SIMULATOR_ERASE_UNIT);
}
Expand Down Expand Up @@ -343,12 +350,20 @@ static void flash_sim_page_layout(struct device *dev,
}
#endif

static const struct flash_parameters *
flash_sim_get_parameters(const struct device *dev)
{
ARG_UNUSED(dev);

return &flash_sim_parameters;
}

static const struct flash_driver_api flash_sim_api = {
.read = flash_sim_read,
.write = flash_sim_write,
.erase = flash_sim_erase,
.write_protection = flash_wp_set,
.write_block_size = FLASH_SIMULATOR_PROG_UNIT,
.get_parameters = flash_sim_get_parameters,
#ifdef CONFIG_FLASH_PAGE_LAYOUT
.page_layout = flash_sim_page_layout,
#endif
Expand Down Expand Up @@ -409,7 +424,7 @@ static int flash_mock_init(struct device *dev)

static int flash_mock_init(struct device *dev)
{
memset(mock_flash, 0xFF, ARRAY_SIZE(mock_flash));
memset(mock_flash, FLASH_SIMULATOR_ERASE_VALUE, ARRAY_SIZE(mock_flash));
return 0;
}

Expand Down
30 changes: 22 additions & 8 deletions drivers/flash/flash_stm32.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,18 @@ LOG_MODULE_REGISTER(flash_stm32, CONFIG_FLASH_LOG_LEVEL);

#define CFG_HW_FLASH_SEMID 2

static const struct flash_parameters flash_stm32_parameters = {
#if DT_PROP(DT_INST(0, soc_nv_flash), write_block_size)
.write_block_size = DT_PROP(DT_INST(0, soc_nv_flash), write_block_size),
#else
#error Flash write block size not available
/* Flash Write block size is extracted from device tree */
/* as flash node property 'write-block-size' */
#endif
/* WARNING: This value may be not valid for L0/L1 chips */
.erase_value = 0xff,
Copy link
Member

@erwango erwango Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't apply to all series.
On a handfull, this should be 0x00.
I'll get back to you with the references

Copy link
Contributor Author

@de-nordic de-nordic Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks for looking into this.

Copy link
Contributor Author

@de-nordic de-nordic Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@utzig has mentioned same thing here: #25947 (comment) though I do not know how does it apply to our stm32 drivers.

Copy link
Contributor Author

@de-nordic de-nordic Jun 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erwango Can you advise me on the stm32 characteristic values for erase_value?

Copy link
Member

@erwango erwango Jun 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@de-nordic After check, only L0/L1 use 0x00 as erase value (I guess reason is they have an EEPROM and this is easier to use same erase value for both).
Since L0/L1 have no flash driver yet, proposed code is fine. Maybe adding a warning comment in case of L0/L1 future addition would be a nice thing though.

Copy link
Contributor Author

@de-nordic de-nordic Jun 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erwango Thanks for the information. Will add warning information.

Copy link
Contributor

@andysan andysan Jun 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a PR for an L0 driver (#25245) that will hit this issue. The easiest way to distinguish between the EEPROM and FLASH implementations is probably to check for the FLASH_CR_LOCK (erase value 0xff) or FLASH_PECR_PELOCK (erase value 0x00). It's not exactly pretty, but it avoids hard-coding the SOC family.

Copy link
Contributor

@Laczen Laczen Jun 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a PR for an L0 driver (#25245) that will hit this issue. The easiest way to distinguish between the EEPROM and FLASH implementations is probably to check for the FLASH_CR_LOCK (erase value 0xff) or FLASH_PECR_PELOCK (erase value 0x00). It's not exactly pretty, but it avoids hard-coding the SOC family.

@andysan, does this mean that also for L0 the flash erase value is 0xff?

};

#if defined(CONFIG_MULTITHREADING)
/*
* This is named flash_stm32_sem_take instead of flash_stm32_lock (and
Expand Down Expand Up @@ -281,6 +293,14 @@ static int flash_stm32_write_protection(struct device *dev, bool enable)
return rc;
}

static const struct flash_parameters *
flash_stm32_get_parameters(const struct device *dev)
{
ARG_UNUSED(dev);

return &flash_stm32_parameters;
}

static struct flash_stm32_priv flash_data = {
.regs = (FLASH_TypeDef *) DT_INST_REG_ADDR(0),
#if defined(CONFIG_SOC_SERIES_STM32L4X) || \
Expand All @@ -299,16 +319,10 @@ static const struct flash_driver_api flash_stm32_api = {
.erase = flash_stm32_erase,
.write = flash_stm32_write,
.read = flash_stm32_read,
.get_parameters = flash_stm32_get_parameters,
#ifdef CONFIG_FLASH_PAGE_LAYOUT
.page_layout = flash_stm32_page_layout,
#endif
#if DT_PROP(DT_INST(0, soc_nv_flash), write_block_size)
.write_block_size = DT_PROP(DT_INST(0, soc_nv_flash), write_block_size),
#else
#error Flash write block size not available
/* Flash Write block size is extracted from device tree */
/* as flash node property 'write-block-size' */
#endif
};

static int stm32_flash_init(struct device *dev)
Expand Down Expand Up @@ -348,7 +362,7 @@ static int stm32_flash_init(struct device *dev)
flash_stm32_sem_init(dev);

LOG_DBG("Flash initialized. BS: %zu",
flash_stm32_api.write_block_size);
flash_stm32_parameters.write_block_size);

#if ((CONFIG_FLASH_LOG_LEVEL >= LOG_LEVEL_DBG) && CONFIG_FLASH_PAGE_LAYOUT)
const struct flash_pages_layout *layout;
Expand Down
15 changes: 14 additions & 1 deletion drivers/flash/nrf_qspi_nor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@

LOG_MODULE_REGISTER(qspi_nor, CONFIG_FLASH_LOG_LEVEL);

static const struct flash_parameters qspi_flash_parameters = {
.write_block_size = 1,
.erase_value = 0xff,
};

/**
* @brief QSPI buffer structure
* Structure used both for TX and RX purposes.
Expand Down Expand Up @@ -757,6 +762,14 @@ static const struct flash_pages_layout dev_layout = {
};
#undef LAYOUT_PAGES_COUNT

static const struct flash_parameters *
qspi_flash_get_parameters(const struct device *dev)
{
ARG_UNUSED(dev);

return &qspi_flash_parameters;
}

static void qspi_nor_pages_layout(struct device *dev,
const struct flash_pages_layout **layout,
size_t *layout_size)
Expand All @@ -771,10 +784,10 @@ static const struct flash_driver_api qspi_nor_api = {
.write = qspi_nor_write,
.erase = qspi_nor_erase,
.write_protection = qspi_nor_write_protection_set,
.get_parameters = qspi_flash_get_parameters,
#if defined(CONFIG_FLASH_PAGE_LAYOUT)
.page_layout = qspi_nor_pages_layout,
#endif
.write_block_size = 1,
};


Expand Down
27 changes: 20 additions & 7 deletions drivers/flash/soc_flash_mcux.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ struct flash_priv {
uint32_t pflash_block_base;
};

static const struct flash_parameters flash_mcux_parameters = {
#if DT_NODE_HAS_PROP(SOC_NV_FLASH_NODE, write_block_size)
.write_block_size = DT_PROP(SOC_NV_FLASH_NODE, write_block_size),
#else
.write_block_size = FSL_FEATURE_FLASH_PFLASH_BLOCK_WRITE_UNIT_SIZE,
#endif
.erase_value = 0xff,
};

/*
* Interrupt vectors could be executed from flash hence the need for locking.
* The underlying MCUX driver takes care of copying the functions to SRAM.
Expand Down Expand Up @@ -137,29 +146,33 @@ static const struct flash_pages_layout dev_layout = {
};

static void flash_mcux_pages_layout(struct device *dev,
const struct flash_pages_layout **layout,
size_t *layout_size)
const struct flash_pages_layout **layout,
size_t *layout_size)
{
*layout = &dev_layout;
*layout_size = 1;
}
#endif /* CONFIG_FLASH_PAGE_LAYOUT */

static const struct flash_parameters *
flash_mcux_get_parameters(const struct device *dev)
{
ARG_UNUSED(dev);

return &flash_mcux_parameters;
}

static struct flash_priv flash_data;

static const struct flash_driver_api flash_mcux_api = {
.write_protection = flash_mcux_write_protection,
.erase = flash_mcux_erase,
.write = flash_mcux_write,
.read = flash_mcux_read,
.get_parameters = flash_mcux_get_parameters,
#if defined(CONFIG_FLASH_PAGE_LAYOUT)
.page_layout = flash_mcux_pages_layout,
#endif
#if DT_NODE_HAS_PROP(SOC_NV_FLASH_NODE, write_block_size)
.write_block_size = DT_PROP(SOC_NV_FLASH_NODE, write_block_size),
#else
.write_block_size = FSL_FEATURE_FLASH_PFLASH_BLOCK_WRITE_UNIT_SIZE,
#endif
};

static int flash_mcux_init(struct device *dev)
Expand Down
Loading