Dev spi asynch stm #2888
Dev spi asynch stm #2888
Conversation
|
updated tests results |
|
@LMESTM can you resolve the conflict? |
|
Just minor things, but having one spi for any STM device is a big 👍 |
when you mention minor things: are their comments I need to take into account ? |
yes ! minor should be taken into account. Please fix the style. Let's be consistent within this code base. |
|
@0xc0170 I'm fine with correcting any comments, I just can't seen them right now ... |
| SPI_BAUDRATEPRESCALER_128, | ||
| SPI_BAUDRATEPRESCALER_256}; | ||
|
|
||
| void spi_frequency(spi_t *obj, int hz) { |
There was a problem hiding this comment.
Can you fix formatting in this function ? Indentation is off for almost all lines below
There was a problem hiding this comment.
ok - indeed there were tabs around :-(
|
|
||
| obj->spi.transfer_type = transfer_type; | ||
|
|
||
| if (is16bit) words = length / 2; |
| NVIC_EnableIRQ(irq_n); | ||
|
|
||
| // enable the right hal transfer | ||
| //static uint16_t sink; |
|
|
||
| if (event) DEBUG_PRINTF("SPI: Event: 0x%x\n", event); | ||
|
|
||
| return (event & (obj->spi.event | SPI_EVENT_INTERNAL_TRANSFER_COMPLETE)); |
There was a problem hiding this comment.
Is this correct here | SPI_EVENT_INTERNAL_TRANSFER_COMPLETE ? It's set above if we are done or event error?
There was a problem hiding this comment.
I think it is correct.
we're filtering the current possible local events (complete, error, ...) with the registered events from application (can be all, or only complete ...). The appl registered events are stored in spi.event in spi _master_transfer function.
If we don't add internal event to the mask, the internal event will not be sent back, even might be though needed internally.
| struct spi_s *spiobj = SPI_S(obj); | ||
| SPI_HandleTypeDef *handle = &(spiobj->handle); | ||
| while (!ssp_writeable(obj)); | ||
| //spi->DR = (uint16_t)value; |
|
@LMESTM Sorry, I did not send those for review. I corrected it now, should be visible |
|
@0xc0170 no pb - I started wondering what I was missing .. I'll update in line with your comments ! |
LMESTM
force-pushed
the
dev_spi_asynch_stm
branch
from
2cb8b30 to
3e55ff8
Compare
|
@LMESTM a failure in travis CI https://travis-ci.org/ARMmbed/mbed-os/builds/166817016#L1402 |
|
@sg- corrected now. I don't know how this wasn't seen before ... |
|
@LMESTM can you resolve the targets.json conflict? |
the SPI_ASYNCH feature has been already activated for STM32F4. This patchset makes it supported on all STM32 families by: - moving spi_s structure at family level instead of board level - using the F4 spi_api.c reference implementation and making it a common stm_spi_api.c file which makes maintenance a lot easier. - the only part that needs to be implemented for each family is the computation of the clock frequency input to the spi peripheral which is not the same accross families. So this is what remains in the spi_api.c of each family. Because of the introduction of the common file, all the above modifications needs to be done at once.
ASYNCH SPI transfer support has been added based on STM HAL services. To have both ASYNCH and SYNCH work together, we're also moving the write API to STM HAL instead of direct registers access.
Now that SPI_ASYNCH is supported on all STM32 based boards, let's activate the feature. Using a default SPI QUEUE of size 2 - this can be later modified if this prives to be to low. (or too high)
Let's swap default PA_4 pin mapping to SPI_3 otherwise SPI3 cannot be used
Fix indentation issues, remove useless comments, correct if/else format
STM32F303ZE was introduced in parallel to the changes which consist in having family wide definitions like device.h file and a common objects definition. This target is updated accordingly now to benefit of SPI definitions.
LMESTM
force-pushed
the
dev_spi_asynch_stm
branch
from
68ec05d to
aeac255
Compare
|
@sg- rebased again |
|
I know, its annoying. Will have a look into maknig this better |
as I mentioned, I think that moving each feature of device_has line into a single separate line (and same for macros) would avoid most of the conflicts. maybe worth a try ? |
|
Yeah and I'd also like to have a think about single target.json for each target but needs a bit more thinking around inheritance etc... |
Notes:
Description
SPI ASYNCH was already supported on STM32F4 targets.
This PR makes it generic for all STM32 based targets.
While doing this, the spi_api.c files that were more or less duplicated over several targets have now been moved into a single stm_spi_api.c file, except for the SPI peripheral frequency which is target depandant.
The ASYNCH has then been been enabled on all STM32 except L0 and L1 because there are limiations on those targets when SPI frequency gets @ 1MHz or higher while this works ok at lower frequencies like 500kHz (as asynch mode requires a high interrupt rate). This may be enabled in a later stage if we can get rid of the limitations.
Status
READY
Migrations
no change of API
Related PRs
No known dependency
Todos
Test using SPI MOSI/MISO LOOP and doing
spi.write loops then checking results
spi.transfer loops then checking results (in case of SPI_ASYNCH support)
spi_write loops again, then checking results
tested at 3 different frequencies on 1 board per family.
Already tests before mbed directory structure have changed (in #2878). Tests restarted now, will update the PR with results, but I want to send it now to start review and hopefully minimize number of rebase to be done ...
documented in commits and PR description
Deploy notes
Notes regarding the deployment of this PR. These should note any
required changes in the build environment, tools, compilers, etc.
Steps to test or reproduce
Outline the steps to test or reproduce the PR here.