search

embassy-rs / stm32-data

66 stars 94 forks source link

added I2SCFGR register to spi_v3.yaml #482

Closed liarokapisv closed 1 month ago

liarokapisv commented 2 months ago

The I2SCFGR register was missing from the spi_v3.yaml description. I double-checked that the register is the same between the spi2s2_v1_0 and spi2s2_v1_1 variants by diffing the svd files. Also fixed some svd typos in the process

embassy-ci[bot] commented 2 months ago

diff: https://ci.embassy.dev/jobs/115555e7d479/artifacts/diff.html

embassy-ci[bot] commented 2 months ago

diff: https://ci.embassy.dev/jobs/603d525a849a/artifacts/diff.html

liarokapisv commented 2 months ago

I will try to fix some naming incompatibilities with the other spi versions, specifically the enums, so that we have fewer conflicts in the common drivers.

embassy-ci[bot] commented 2 months ago

diff: https://ci.embassy.dev/jobs/6e7626069ee1/artifacts/diff.html

liarokapisv commented 2 months ago

I think this is mostly it, I have a few reservations about changing the I2SCFGR enum itself, any feedback is welcome.

Dirbaio commented 1 month ago

Have you checked that all SPIv3 chips have I2SCFGR? iirc newer chips with SPIv3 that have SAI have dropped the I2S stuff.

liarokapisv commented 1 month ago

Two H7s (H743 & H745) that I have used, definitely have SAI, but also have I2S on their SPI because it is simpler (and can be used with SPI pins). In stm32-data-gen/src/chips, spi_v3 matches the spi2s2_v1_0 and spi2s2_v1_1 spi versions. I checked a subset of SVD files with those SPI versions, and the I2SCFGR register is definitely present in those. I also double-checked with reference manuals but again only for a subset with those versions. It is possible that some chips are missing it, in which case we may need another versioning scheme.

EDIT:

I grepped the cubedb folder for the spi2s2_v1_0 and spi2s2_v1_1 versions, and for each matching chip, I checked the relevant header in the sources/headers folder for the SPI_I2SCFGR register and it was present in all of them. I did not check the stm32mp lines due to missing headers.