search

ARMmbed / mbed-os

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

LPC1768 SPI received bytes are shifted on mbed lib r147 #4847

Closed janjongboom closed 7 years ago

janjongboom commented 7 years ago

See https://developer.mbed.org/questions/78684/SPI-stopped-working-in-MBED-2-rev-147/

Maybe related to CMSIS5 or did that not get rolled out on mbed 2?

0xc0170 commented 7 years ago

There was no change in SPI for v147, neither in drivers (please look at git log to confirm that nothing has changed there) for this target that could cause this.

toyowata commented 7 years ago

I tested LPC1768 SPI with Fujitsu MB85RSxxx FRAM component and the result are not identical between mbed library Rev. 146 and 147.

I will find out further.

[Rev.146]

Fujitsu MB85RSxxx FRAM test program

read device ID = 0x4 0x7f 0x28 0x3
read status (WREN) = 0x2
read status (WRDI) = 0x0
00000000 : 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F 
00000010 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00000020 : 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F 
00000030 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00000040 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00000050 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00000060 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00000070 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 

00000000 : 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F 
00000010 : 10 11 12 13 14 15 16 17 18 19 1A 1B 1C 1D 1E 1F 
00000020 : 20 21 22 23 24 25 26 27 28 29 2A 2B 2C 2D 2E 2F 
00000030 : 30 31 32 33 34 35 36 37 38 39 3A 3B 3C 3D 3E 3F 
00000040 : 40 41 42 43 44 45 46 47 48 49 4A 4B 4C 4D 4E 4F 
00000050 : 50 51 52 53 54 55 56 57 58 59 5A 5B 5C 5D 5E 5F 
00000060 : 60 61 62 63 64 65 66 67 68 69 6A 6B 6C 6D 6E 6F 
00000070 : 70 71 72 73 74 75 76 77 78 79 7A 7B 7C 7D 7E 7F 
00000080 : 80 81 82 83 84 85 86 87 88 89 8A 8B 8C 8D 8E 8F 
00000090 : 90 91 92 93 94 95 96 97 98 99 9A 9B 9C 9D 9E 9F 
000000A0 : A0 A1 A2 A3 A4 A5 A6 A7 A8 A9 AA AB AC AD AE AF 
000000B0 : B0 B1 B2 B3 B4 B5 B6 B7 B8 B9 BA BB BC BD BE BF 
000000C0 : C0 C1 C2 C3 C4 C5 C6 C7 C8 C9 CA CB CC CD CE CF 
000000D0 : D0 D1 D2 D3 D4 D5 D6 D7 D8 D9 DA DB DC DD DE DF 
000000E0 : E0 E1 E2 E3 E4 E5 E6 E7 E8 E9 EA EB EC ED EE EF 
000000F0 : F0 F1 F2 F3 F4 F5 F6 F7 F8 F9 FA FB FC FD FE FF 

[Rev.147]

Fujitsu MB85RSxxx FRAM test program

read device ID = 0x2 0x3f 0x14 0x1
read status (WREN) = 0x1
read status (WRDI) = 0x0
00000000 : 00 00 01 01 02 02 03 03 04 04 05 05 06 06 07 07 
00000010 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00000020 : 00 00 01 01 02 02 03 03 04 04 05 05 06 06 07 07 
00000030 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00000040 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00000050 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00000060 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00000070 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 

00000000 : C0 00 01 01 02 02 03 03 04 04 05 05 06 06 07 07 
00000010 : 08 08 09 09 0A 0A 0B 0B 0C 0C 0D 0D 0E 0E 0F 0F 
00000020 : 10 10 11 11 12 12 13 13 14 14 15 15 16 16 17 17 
00000030 : 18 18 19 19 1A 1A 1B 1B 1C 1C 1D 1D 1E 1E 1F 1F 
00000040 : 20 20 21 21 22 22 23 23 24 24 25 25 26 26 27 27 
00000050 : 28 28 29 29 2A 2A 2B 2B 2C 2C 2D 2D 2E 2E 2F 2F 
00000060 : 30 30 31 31 32 32 33 33 34 34 35 35 36 36 37 37 
00000070 : 38 38 39 39 3A 3A 3B 3B 3C 3C 3D 3D 3E 3E 3F 3F 
00000080 : C0 C0 C1 C1 C2 C2 C3 C3 C4 C4 C5 C5 C6 C6 C7 C7 
00000090 : C8 C8 C9 C9 CA CA CB CB CC CC CD CD CE CE CF CF 
000000A0 : D0 D0 D1 D1 D2 D2 D3 D3 D4 D4 D5 D5 D6 D6 D7 D7 
000000B0 : D8 D8 D9 D9 DA DA DB DB DC DC DD DD DE DE DF DF 
000000C0 : E0 E0 E1 E1 E2 E2 E3 E3 E4 E4 E5 E5 E6 E6 E7 E7 
000000D0 : E8 E8 E9 E9 EA EA EB EB EC EC ED ED EE EE EF EF 
000000E0 : F0 F0 F1 F1 F2 F2 F3 F3 F4 F4 F5 F5 F6 F6 F7 F7 
000000F0 : F8 F8 F9 F9 FA FA FB FB FC FC FD FD FE FE FF FF
toyowata commented 7 years ago

The constructor of the component library is something like:

MB85RSxx_SPI::MB85RSxx_SPI(PinName mosi, PinName miso, PinName sclk, PinName cs)
    :
    _spi(mosi, miso, sclk),
    _cs(cs)
{
    uint8_t buf[4];

    _cs = 1;
    _spi.format(8, 0);
    read_device_id(buf);
    if ((buf[2] & 0x1F) > MB85_DENSITY_512K) {
        _address_bits = 24;
    } else {
        _address_bits = 16;
    }
}

I temporary added frequency() function call like below and the test program works fine as same as rev 146.

MB85RSxx_SPI::MB85RSxx_SPI(PinName mosi, PinName miso, PinName sclk, PinName cs)
    :
    _spi(mosi, miso, sclk),
    _cs(cs)
{
    uint8_t buf[4];

    _cs = 1;
    _spi.format(8, 0);
    _spi.frequency(1000000);  // *** Added this ***
    read_device_id(buf);
    if ((buf[2] & 0x1F) > MB85_DENSITY_512K) {
        _address_bits = 24;
    } else {
        _address_bits = 16;
    }
}
0xc0170 commented 7 years ago

@toyowata Can you pinpoint to the changeset that affects this? I cant locate any changes in SPI API, neither HAL for this target?

toyowata commented 7 years ago

@0xc0170 This should be an issue in HAL, not SPI API. I just quick checked to review SPI HAL code of the LPC1768 and found that spi_format() function wrongly broken control register bits for SPI serial clock rate setting. It works if SPI::frequency() is called just after the SPI::format() call to set correct serial clock rate. I will find out more detail tomorrow.

This is not only for LPC1768 SPI issue, but also other NXP LPC platforms (LPC11U68, LPC1114, LPC4337 and more?), because the LPC1768 code was reused for same SSP/SPI peripheral module.

toyowata commented 7 years ago

The control register of LPC1768 SSP/SPI is below:

lpc1768_ssp_cr0

The SCR bits are configured for SPI serial clock rate. This CR0 register is updated in the spi_format function in the spi_api.c (LPC1768 SPI HAL). It reads the current CR0 register value to tmp local variable and clears bits [8:0] to update new SPI format value. However, it wrongly clears bits [15:0] include SCR field, then serial clock rate is broken.

https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_NXP/TARGET_LPC176X/spi_api.c#L101

As far as I checked, following platform are affected this problem.

LPC1768, LPC11Uxx, LPC11U68, LPC4337, LPC1114, LPC11Cxx and LPC13xx

I will make fixes and PR for all targets above.

nenimilo commented 7 years ago

It also affects TARGET_LPC408X, and there of course both TARGET_LPC4088 and TARGET_LPC4088_DM, so LPC408x should be fixed too.

Regards Neni

0xc0170 commented 7 years ago

Thanks @toyowata for looking at this.

I still fail to understand why it is not broken in earlier mbed lib versions?

toyowata commented 7 years ago

Hi @0xc0170

I still fail to understand why it is not broken in earlier mbed lib versions?

In the Rev.146 SPI code, SPI::format() always set NULL to SPI::_owner which means calling the SPI::format() always calls both spi_format() and spi_frequency() of target HAL.

https://github.com/ARMmbed/mbed-os/commit/de89be35f8ce4421f3966089a1b9788fbb04beb8#diff-3b346da4102b78a8fc25d52adb92b458L46

In the Rev.147 (and current code), SPI::format() calls only spi_format() HAL function, because _owner variable was set by _acquire() call from the constructor when instance is created.

https://github.com/ARMmbed/mbed-os/blob/master/drivers/SPI.cpp#L40 https://github.com/ARMmbed/mbed-os/blob/master/drivers/SPI.cpp#L91 https://github.com/ARMmbed/mbed-os/blob/master/drivers/SPI.cpp#L50

That is different behavior of SPI API. Obviously, behavior of the Rev.147 is correct and unvailed latent problem in LPCs SPI HAL implimentation.