Update SPI Device HAL to use new autogen interface#387
Conversation
ziuziakowska
force-pushed
the
hal-autogen-rework-spi-device
branch
from
1755b42 to
2cfbaa7
Compare
ziuziakowska
force-pushed
the
hal-autogen-rework-spi-device
branch
2 times, most recently
from
f45de2f to
8b85c97
Compare
ziuziakowska
changed the title
ziuziakowska
force-pushed
the
hal-autogen-rework-spi-device
branch
from
8b85c97 to
98fbdda
Compare
engdoreis
left a comment
engdoreis
left a comment
There was a problem hiding this comment.
Thanks for the changes, I had a quick look and letf some comments.
I'm happy to give a more detailed review when it pass CI.
| return true; | ||
| default: | ||
| uprintf(ctx->console, "\nUnsupported command: 0x%0x", cmd.opcode); | ||
| uprintf(ctx->console, "Unsupported command: 0x%x\n", cmd.opcode); |
There was a problem hiding this comment.
Why remove the line break?
| #include <stdint.h> | ||
|
|
||
| bool spi_device_interrupt_is_pending(spi_device_t spi_device, uint8_t intr_id) | ||
| enum : size_t { |
There was a problem hiding this comment.
These structs should not be inside the spi hal, you should create a flash header somewhere, then the app should call the hal to configure the spi.
There was a problem hiding this comment.
I just tried to translate the file into the new autogen interface and make some changes, but I do think this should stay here - the SPI Device is largely special purpose for emulating a SPI Flash device.
then the app should call the hal to configure the spi.
Programs still use the same HAL to configure the SPI Device, these enums are private to the C file as the SFDP table can be considered implementation details.
ziuziakowska
force-pushed
the
hal-autogen-rework-spi-device
branch
3 times, most recently
from
d54f157 to
aa95279
Compare
ziuziakowska
force-pushed
the
hal-autogen-rework-spi-device
branch
2 times, most recently
from
372ea62 to
320c623
Compare
ziuziakowska
force-pushed
the
hal-autogen-rework-spi-device
branch
from
320c623 to
e90d51e
Compare
ziuziakowska
force-pushed
the
hal-autogen-rework-spi-device
branch
2 times, most recently
from
d9d839f to
68b17b9
Compare
Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
ziuziakowska
force-pushed
the
hal-autogen-rework-spi-device
branch
from
68b17b9 to
81451b2
Compare
| bool spi_device_flash_payload_buffer_read_byte(spi_device_t spi_device, size_t byte_index, | ||
| uint8_t *byte) | ||
| { | ||
| size_t word_index = byte_index >> 4u; |
There was a problem hiding this comment.
I think you want:
size_t word_index = byte_index / 4u;or:
size_t word_index = byte_index >> 2u;
| { | ||
| // Wait for software-handled command | ||
| while (!spi_device_interrupt_is_pending(spi_device, SPI_DEVICE_INTR_UPLOAD_CMDFIFO_NOT_EMPTY)) { | ||
| size_t word_index = byte_index >> 4u; |
There was a problem hiding this comment.
| size_t word_index = byte_index >> 4u; | |
| size_t word_index = byte_index >> 2u; |
| if (cmd.status != 0) { | ||
| spi_device_software_command command; | ||
| while (true) { | ||
| bool success = spi_device_software_command_get(spi_device, &command); |
There was a problem hiding this comment.
This function returns a enum, right?
2 participants
Split off from #299.
Depends on #579.Merged.