Skip to content
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.

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

stm32 spi driver adds the frame format #40860

Merged
merged 2 commits into from
Jan 17, 2022

Conversation

FRASTM
Copy link
Collaborator

@FRASTM FRASTM commented Dec 2, 2021

this PR includes the support of the Format (TI vs MOTOROLA) of the SPI frame
The spi frame format is a property added to the spi device through an overlay for the led_strip sample on ws2812
$ west build -p auto -b nucleo_l476rg samples/drivers/led_ws2812/

After the new #39990

fixes #38683

Signed-off-by: Francois Ramu [email protected]

@erwango erwango requested review from erwango and tbursztyka December 3, 2021 08:21
@FRASTM FRASTM force-pushed the stm32_spi_frame_format branch from 9fb7037 to 77db82a Compare December 3, 2021 09:43
@github-actions github-actions bot added the area: Samples Samples label Dec 3, 2021
@FRASTM FRASTM marked this pull request as ready for review December 3, 2021 09:46
@FRASTM FRASTM requested a review from nashif as a code owner December 3, 2021 09:46
@FRASTM FRASTM force-pushed the stm32_spi_frame_format branch 2 times, most recently from c10aeab to 61c4705 Compare December 3, 2021 14:45
@FRASTM
Copy link
Collaborator Author

FRASTM commented Dec 3, 2021

stm32F1xx and some of the stm32L1 MCUs do not support the frame format.

@zephyrbot zephyrbot added the area: LED Label to identify LED subsystem label Dec 3, 2021
@gmarull gmarull removed their request for review December 3, 2021 15:01
@FRASTM FRASTM force-pushed the stm32_spi_frame_format branch from 61c4705 to 16d9296 Compare December 6, 2021 09:12
@FRASTM
Copy link
Collaborator Author

FRASTM commented Dec 6, 2021

includes overlay for nucleo_g071rb board

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

Comment on lines 941 to 942
#if !defined(CONFIG_SOC_SERIES_STM32F1X) \
&& (!defined(CONFIG_SOC_SERIES_STM32L1X) || defined(SPI_CR2_FRF))
Copy link
Member

Choose a reason for hiding this comment

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

Rather than silently converting property to 0 for these cases, it should rather be notified at binding level,
and then generate an error if an unsupported case is configured

Copy link
Collaborator Author

@FRASTM FRASTM Dec 6, 2021

Choose a reason for hiding this comment

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

#else
--> raising a compilation error 

drivers/spi/spi_ll_stm32.c Outdated Show resolved Hide resolved
@FRASTM FRASTM force-pushed the stm32_spi_frame_format branch 2 times, most recently from 68e4ccb to 0fd24f8 Compare December 7, 2021 16:04
Copy link
Collaborator

@ABOSTM ABOSTM left a comment

Choose a reason for hiding this comment

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

LGTM

@FRASTM FRASTM force-pushed the stm32_spi_frame_format branch 2 times, most recently from ab14969 to 99361dd Compare December 10, 2021 13:28
drivers/spi/spi_ll_stm32.c Outdated Show resolved Hide resolved
drivers/spi/spi_ll_stm32.c Outdated Show resolved Hide resolved
@FRASTM FRASTM force-pushed the stm32_spi_frame_format branch from 99361dd to 42ca28c Compare December 15, 2021 13:05
@FRASTM FRASTM requested a review from carlescufi December 15, 2021 13:09
@FRASTM
Copy link
Collaborator Author

FRASTM commented Dec 15, 2021

@tbursztyka
Since the PR #39990, the frame_format property is missing in the spi_config structure as follows:
.frame_format = DT_PROP(node_id, frame_format),
However there is a bit [15] in the .operation bitfield
* format [ 15 ] - Motorola or TI frame format (optional).
but it seems not immediately compatible with the frame-format as described in the dts/bindings/spi/spi-device.yaml

@FRASTM
Copy link
Collaborator Author

FRASTM commented Jan 6, 2022

rebase on f882d43

Some stm32L1x mcus (cat 1 or 2) do not have the LL function to set the frame format because they do not have the SPI TI mode (their SPI Control reg 2 bit FRF remains at reset value)

@FRASTM FRASTM force-pushed the stm32_spi_frame_format branch from fd0fafe to b74d16e Compare January 6, 2022 10:52
@tbursztyka
Copy link
Collaborator

tbursztyka commented Jan 7, 2022

@tbursztyka Since the PR #39990, the frame_format property is missing in the spi_config structure as follows: .frame_format = DT_PROP(node_id, frame_format), However there is a bit [15] in the .operation bitfield * format [ 15 ] - Motorola or TI frame format (optional). but it seems not immediately compatible with the frame-format as described in the dts/bindings/spi/spi-device.yaml

there isn't a property match between dts and spi_config. in dts it's frame_format, and in in spi_config.operation it's a bit there. Where is the issue?

Let me check the bit values, they must be the same (15th bit set or not)
[edit] they seem to match for me. spi_config.operation |= DT_PROP(node_id, frame_format) should be the same as spi_config.operation |= <SPI_FRAME_FORMAT_MOTOROLA/SPI_FRAME_FORMAT_TI>

@tbursztyka
Copy link
Collaborator

@FRASTM looks like I forgot to update SPI_CONFIG_DT() macro. see pr #41641

@FRASTM FRASTM force-pushed the stm32_spi_frame_format branch from b74d16e to 1ff8efe Compare January 7, 2022 11:00
@FRASTM FRASTM requested a review from galak as a code owner January 7, 2022 11:00
@github-actions github-actions bot added the area: Devicetree Binding PR modifies or adds a Device Tree binding label Jan 7, 2022
@FRASTM FRASTM force-pushed the stm32_spi_frame_format branch from 1ff8efe to 0d170d6 Compare January 7, 2022 11:04
@FRASTM
Copy link
Collaborator Author

FRASTM commented Jan 7, 2022

rebase
get the PR #41641
--> the stm32 drivers takes the frame_format from the DTS property : operation bitfield

drivers/spi/spi_ll_stm32.c Outdated Show resolved Hide resolved
@FRASTM
Copy link
Collaborator Author

FRASTM commented Jan 11, 2022

rebase

  • changed to have else flagged by #if defined(LL_SPI_PROTOCOL_MOTOROLA) && defined(SPI_CR2_FRF)

drivers/spi/spi_ll_stm32.c Outdated Show resolved Hide resolved
@FRASTM FRASTM force-pushed the stm32_spi_frame_format branch from fe6d76a to 2f0f267 Compare January 11, 2022 14:52
@erwango erwango added this to the v3.0.0 milestone Jan 17, 2022
the stm32 spi drivers now takes the DTS frame_format property
from the include/ drivers/spi.h
It will be possible to select the Motorola (default)
or TI from the DTS entry of the device,
when soc supports it, else a run time error is raised.

Signed-off-by: Francois Ramu <[email protected]>
The spi node of the led_ws2812 now includes the frame format
as defined by the dts bindings spi-device.yaml
SPI_FRAME_FORMAT_TI is selected with overlay for target boards.

Signed-off-by: Francois Ramu <[email protected]>
@FRASTM FRASTM force-pushed the stm32_spi_frame_format branch from 2f0f267 to b6efb3f Compare January 17, 2022 14:24
@FRASTM
Copy link
Collaborator Author

FRASTM commented Jan 17, 2022

rebase on bb00120

@erwango erwango requested a review from MaureenHelm January 17, 2022 15:00
@codecov-commenter
Copy link

Codecov Report

Merging #40860 (bb00120) into main (bb00120) will not change coverage.
The diff coverage is n/a.

❗ Current head bb00120 differs from pull request most recent head b6efb3f. Consider uploading reports for the commit b6efb3f to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main   #40860   +/-   ##
=======================================
  Coverage   51.45%   51.45%           
=======================================
  Files         616      616           
  Lines       72357    72357           
  Branches    16651    16651           
=======================================
  Hits        37234    37234           
  Misses      29003    29003           
  Partials     6120     6120           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb00120...b6efb3f. Read the comment docs.

@nashif nashif merged commit ff34965 into zephyrproject-rtos:main Jan 17, 2022
@FRASTM FRASTM deleted the stm32_spi_frame_format branch August 16, 2022 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Boards area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: LED Label to identify LED subsystem area: Samples Samples area: SPI SPI bus platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stm32g0b1: spi: MOSI signal goes high before and after transmitting data
7 participants