ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.66k stars 2.97k forks source link

Ambiguity in allocation SPI peripheral of SPIFBlockDevice #13508

Closed lefebvresam closed 4 years ago

lefebvresam commented 4 years ago

Description of defect

When using SPIFBlockDevice you have to define the pins for SPI connection in the form:

SPIFBlockDevice spif(PB_5, PB_4, PB_3, PB_10); // mosi,miso,clk,cs,freq(opt)

The problem is that pins PB_3/4/5 are shared for SPI1 and 3. (See datasheet STM32F474RE p75). When I use another SPI device like the display:

// mosi,miso,sclk,csel,reset,sda,scl,irq,name
RA8875 lcd(PA_7, PA_6, PA_5, PB_6, NC, PB_9, PB_8, PC_7, "tft");

PA5/6/7 als also using SPI1.

The result is that accessing the SPI flash memory is not longer possible after configuration of the LCD. They probably use both the SPI1 hardware and come in conflict. The purpose should be that the one uses SPI1 and the other SPI3. Or maybe having an option to select the hardware peripheral to not be confused by a multi slave config.

Target(s) affected by this defect ?

NUCLEO_G474RE

Toolchain(s) (name and version) displaying this defect ?

GCC_ARM

What version of Mbed-os are you using (tag or sha) ?

mbed-os-6.2.0

a2ada74770f043aff3e61e29d164a8e78274fcd4

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed-cli 1.10.4

How is this defect reproduced ?

Setup a project:

// mosi,miso,sclk,csel,reset,sda,scl,irq,name
RA8875 lcd(PA_7, PA_6, PA_5, PB_6, NC, PB_9, PB_8, PC_7, "tft");
SPIFBlockDevice spif(PB_5, PB_4, PB_3, PB_10); // mosi,miso,clk,cs,freq(opt)

int main()
{
    lcd.Reset();
    printf("spif type: %s\n", spif.get_type());
}
LMESTM commented 4 years ago

the available pin mapping options are defined here https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/TARGET_STM32G4/TARGET_STM32G474xx/TARGET_NUCLEO_G474RE/PeripheralPins.c#L309

For the kind of case you refer to, there is no easy way to guess whether user want to use SPI3. So by default PB3/4/5 are mapped to SPI1 In your paticular case, you can instead refer to PB_4_ALT0, PB_5_ALT0, PB_6_ALT0 and they will be mapped to SPI3 as you can see in the table.

SPIFBlockDevice spif(PB_5_ALT0, PB_4_ALT0, PB_3_ALT0, PB_10); // mosi,miso,clk,cs,freq(opt)

should do the job

lefebvresam commented 4 years ago

Thank you. I tried to solve this issue by rewiring the hardware pins to avoid this ambiguity. I'm still working on a development board. But however because it can happen unexpectedly elsewhere it can be a suggestion to make a note of it on the documentation site.

LMESTM commented 4 years ago

Thank you. I tried to solve this issue by rewiring the hardware pins to avoid this ambiguity. I'm still working on a development board. But however because it can happen unexpectedly elsewhere it can be a suggestion to make a note of it on the documentation site.

This method might be specific to ST targets I am not sure .... Where would you recommend to put the information for users to easily find it ?

lefebvresam commented 4 years ago

Thank you. I tried to solve this issue by rewiring the hardware pins to avoid this ambiguity. I'm still working on a development board. But however because it can happen unexpectedly elsewhere it can be a suggestion to make a note of it on the documentation site.

This method might be specific to ST targets I am not sure .... Where would you recommend to put the information for users to easily find it ?

It's maybe an issue that can happen with all peripherals that use SPI. Maybe it can be recommended to make a separate page for it and put a link to from SPIFBlockDevice.

lefebvresam commented 4 years ago

It is info that belongs to the explanation of the constructor. Or maybe a better idea is to put this info on the SPI page and link to it.

LMESTM commented 4 years ago

This method does not apply only to SPI, the same is valid for PWM, ADC, ETH ... I understand the need, but finding the proper place is not simple (@jeromecoutant maybe this is already documented in ST pages ?)

lefebvresam commented 4 years ago

In my opinion all devices that can be 'hardware switched' must have a small link to this explanation from the constructor info doc, because everybody who use it can encounter this problem and they not always know how to deal with it.

jeromecoutant commented 4 years ago

Hi I will try to add info in https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/README.md

ciarmcom commented 4 years ago

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers. Internal Jira reference: https://jira.arm.com/browse/MBOTRIAGE-2806

jeromecoutant commented 4 years ago

FYI: #13513

Maybe we could close this issue ? Thx

0xc0170 commented 4 years ago

I'll close as resolved via #13513 (it's readme update that will be merged soon).

@lefebvresam if not yet resolved, reopen with an update what we have missed.

0xc0170 commented 4 years ago

Thanks for reporting!

lefebvresam commented 4 years ago

OK, if the documentation is updated, this issue can be closed. My solution was to swap the pins. Thx