drivers/flash: it51xxx: Add the M1K flash driver #92495
Conversation
To prevent FSPI pins from floating, which may cause internal leakage and increase SoC power consumption, tri-state is enabled by default. Signed-off-by: Tim Lin <tim2.lin@ite.corp-partner.google.com>
github-actions
bot
added
platform: ITE
github-actions
bot
requested review from
carlocaione,
de-nordic,
Dino-Li,
edersondisouza,
fabiobaltieri,
fkokosinski,
jackrosenthal,
katsuster,
keith-zephyr,
kgugala,
mgielda,
npitre,
rghaddab,
RuibinChang and
tgorochowik
| .m1k_base = DT_INST_REG_ADDR_BY_IDX(0, 0), | ||
| .smfi_base = DT_INST_REG_ADDR_BY_IDX(0, 1), |
There was a problem hiding this comment.
since this is a single driver instance (and I'd imagine it'll stay that way), I think you'd be better off defining these at the top of the file and the using them to hardcode the offset on the register definitions themselves, the current driver has a lot of
cfg->m1k_base + SMFI_M1K_READ_LBA2
just get the offset once and use it as part of SMFI_M1K_READ_LBA2 itself, that should make the driver a bit easier to read.
| flash_set_m1k_dlm_ba(dev, (uint32_t)dst_data); | ||
|
|
||
| /* M1k-READ size (Maximum 1024 bytes) */ | ||
| len = ((len - 1) & M1K_READ_BCNT_MASK); |
There was a problem hiding this comment.
no need for the outer parenthesis
| int ret; | ||
| uint8_t m1kflhctrl5_cmd; | ||
|
|
||
| LOG_INF("%s: offset=%lx, data addr=%p, len=%u", __func__, offset, (uint8_t *)dst_data, len); |
| int ret; | ||
| uint8_t m1kflhctrl7_cmd; | ||
|
|
||
| LOG_INF("%s: offset=%lx, data addr=%p, len=%u", __func__, offset, (const uint8_t *)src_data, |
| if (!is_valid_range(offset, len) || (len > FLASH_WRITE_MAX_SZ)) { | ||
| LOG_ERR("Write range exceeds the flash boundaries. FLASH_SIZE=%#x " | ||
| "or the len exceeds maximun write size=%u", | ||
| FLASH_SIZE, FLASH_WRITE_MAX_SZ); | ||
| return -EINVAL; | ||
| } |
There was a problem hiding this comment.
keep it compact, these messages eat up flash no need to be super verbose "out of boundaries: FLASH_SIZE=%#x, size=%u", also log len as well
| flash_set_m1k_dlm_ba(dev, (uint32_t)src_data); | ||
|
|
||
| /* M1k-PROG size (Maximum 1024 bytes) */ | ||
| len = ((len - 1) & M1K_PROG_BCNT_MASK); |
There was a problem hiding this comment.
drop the outer parenthesis
| int ret; | ||
| uint8_t m1kflhctrl7_cmd; | ||
|
|
||
| LOG_INF("%s: offset=%lx, len=%u", __func__, offset, len); |
GTLin08
force-pushed
the
it51xxx_m1k_flash_driver
branch
from
55bab94 to
34f5e7c
Compare
| struct flash_it51xxx_dev_data *data = dev->data; | ||
| int ret; | ||
| uint8_t m1kflhctrl5_cmd; | ||
|
|
There was a problem hiding this comment.
Missing len check to avoid any work, and return success, when len is 0.
|
|
||
| LOG_DBG("%s: offset=%lx, data addr=%p, len=%u", __func__, offset, (const uint8_t *)src_data, | ||
| len); | ||
|
|
There was a problem hiding this comment.
Missing len check to avoid any work, and return success, when len is 0.
| uint8_t m1kflhctrl7_cmd; | ||
|
|
||
| LOG_DBG("%s: offset=%lx, len=%u", __func__, offset, len); | ||
|
|
There was a problem hiding this comment.
Missing len check to avoid any work, and return success, when len is 0.
| /* M1k-READ size (Maximum 1024 bytes) */ | ||
| len = (len - 1) & M1K_READ_BCNT_MASK; |
There was a problem hiding this comment.
I do not get this lines. So, I request len bytes to be read, but I will get at most 1023 bytes? I mean 1023 not 1024, because the mask will mask is from bit 0 to 9? And actually I always get one less byte?
There was a problem hiding this comment.
According to the hardware specification, this field is a 10-bit 0-based counter, which means the value written represents (N - 1) bytes to transfer. As a result, the maximum value 0x3FF (1023) corresponds to a full 1024-byte (1 KB) transfer.
|
|
||
| LOG_DBG("%s: offset=%lx, data addr=%p, len=%u", __func__, offset, (uint8_t *)dst_data, len); | ||
|
|
||
| if (!is_valid_range(offset, len) || (len > FLASH_READ_MAX_SZ)) { |
There was a problem hiding this comment.
If this thing can read at most 1024 bytes at once, shouldn't here be a loop? There is no limit, from the point of Flash API, on how much bytes you can run at once.
The flash M1K driver supports read (up to 1K), write (1K), and erase (4K) operations, which can be accessed via DLM. Accessible flash regions include internal e-Flash or external SPI flash via FSCE# or FSCE1#. Signed-off-by: Tim Lin <tim2.lin@ite.corp-partner.google.com>
GTLin08
force-pushed
the
it51xxx_m1k_flash_driver
branch
from
34f5e7c to
20ebd62
Compare
|
The size restrictions in the read, write, and erase functions have been removed in the latest update. |
|
@de-nordic please revisit. |
This PR includes two commits:
Enable FSPI tri-state by default:
To prevent FSPI pins from floating, which may cause internal leakage and increase SoC power consumption, tri-state is enabled by default.
Add flash M1K driver support:
The flash M1K driver supports read (up to 1K), write (1K), and erase (4K) operations, which can be accessed via DLM.
Accessible flash regions include internal e-Flash or external SPI flash via FSCE# or FSCE1#.