[riot-notifications] [RIOT-OS/RIOT] drivers/sdcard_spi: Use uint8_t instead of char for byte representation (#11465)

Frank Hessel notifications at github.com
Mon Apr 29 20:26:45 CEST 2019


### Contribution description

This PR is related to PR #11352, where I suggest to add support for Linux' `/dev/spidev` devices to the native cpu. [As mentioned in that PR](https://github.com/RIOT-OS/RIOT/pull/11352#issuecomment-486231452), enabling SPI causes the `tests/driver_sdcard_spi` to be run for the first time on the gnu / llvm toolchain. As the driver and the test use `char` for byte representation and rely on the platform's definition for `char` being unsigned, the test fails as this does not hold for Linux and these toolchains.

So in this PR, I suggest to use `uint8_t` instead of `char` for all bytes and byte arrays in the `sdcard_spi` driver and the corresponding test to be platform-independent. I checked the variables for command identifiers that I modified against the [SD Card Specs](https://www.sdcard.org/downloads/pls/), and all of them are meant to be unsigned bytes.

### Testing procedure

**Testing on hardware:**

This should assure that the changes do not break anything in platforms where the SD card can already be used. It should not be a problem, because if the code did compile without warnings before, `char` should've already been an unsigned 8 bit integer.

The `driver_sdcard_spi` or `pkg_fatfs` tests are suitable to test the modified driver. I'd suggest to wire up a board with SPI support to an SD Card slot and run the test first against the current master branch to verify the setup, and then to the modified version from this PR.

I had an ESP32 at hand and used the `esp32-wroom-32` board for testing. To do so, I added the following parameters to the test's Makefile ...

```
CFLAGS += -DSDCARD_SPI_PARAM_CS="GPIO_PIN(0,5)"
CFLAGS += -DSDCARD_SPI_PARAM_CLK="GPIO_PIN(0,18)"
CFLAGS += -DSDCARD_SPI_PARAM_MOSI="GPIO_PIN(0,23)"
CFLAGS += -DSDCARD_SPI_PARAM_MISO="GPIO_PIN(0,19)"
```

... and wired the SD card module to the corresponding pins.

Additionally, I had to change `SD_CARD_SPI_SPEED_POSTINIT` to `SPI_CLK_400KHZ` in `drivers/sdcard_spi/include/sdcard_spi_internal.h:148`, as my jumper-wire-setup failed for higher speeds. In that case you'll be able to retrieve information about the card in the slot, but not to read/write any actual data without the modification.

<details>
<summary>Example for the driver_sdcard_spi test</summary>

```
INFO # SD-card spi driver test application
INFO # insert SD-card and use 'init' command to set card to spi mode
INFO # WARNING: using 'write' or 'copy' commands WILL overwrite data on your sd-card and
INFO # almost for sure corrupt existing filesystems, partitions and contained data!
> cid
INFO #  cid
INFO # ----------------------------------------
INFO # MID: 3
INFO # OID: SD
       ...
> size
INFO #  size
INFO # Card size: 
INFO # 15931539456 bytes (14,836 GiB | 15,931 GB)
> read 0 4
INFO #  read 0 4
INFO # BLOCK 0:
INFO # fa b8 00 10 8e d0 bc 00 b0 b8 00 00 8e d8 8e c0 
INFO # fb be 00 7c bf 00 06 b9 00 02 f3 a4 ea 21 06 00
       ...
```

</details>

<details>
<summary>Example for the pkg_fatfs test</summary>

```
INFO # init sdcard_mtd 0 [OK]
> mount 0
INFO #  mount 0
INFO # mounting file system image...
INFO # [OK]
INFO # Volume name: 
INFO # 14,821 GiB of 14,821 GiB available
> ls
INFO #  ls
INFO # //LOREM.TXT
INFO # //HELLO.TXT
> read //LOREM.TXT
INFO #  read //LOREM.TXT
INFO # Lorem ipsum dolor sit amet, ...
```

</details>

**Test the changes:**

To make sure that this PR solves the problem in #11352, both branches have to be merged on top of each other (should for now work without any conflicts). Then, build `tests/driver_sdcard_spi` with gnu and/or llvm for `BOARD=native` and check that it does no longer throw any warnings, in contrast to the previous [Murdock build](https://ci.riot-os.org/RIOT-OS/RIOT/11352/fb4c52747487659a540d72eb760e0ae66d420beb/output.html). Note that `tests/pkg_fatfs` isn't suitable here as it uses the native MTD implementation.

### Issues/PRs references

Required for PR #11352

You can view, comment on, or merge this pull request online at:

  https://github.com/RIOT-OS/RIOT/pull/11465

-- Commit Summary --

  * drivers/sdcard_spi: Use uint8_t for byte values
  * tests/driver_sdcard_spi: Use uint8_t for byte buffers

-- File Changes --

    M drivers/include/sdcard_spi.h (4)
    M drivers/sdcard_spi/include/sdcard_spi_internal.h (4)
    M drivers/sdcard_spi/sdcard_spi.c (114)
    M tests/driver_sdcard_spi/main.c (14)

-- Patch Links --

https://github.com/RIOT-OS/RIOT/pull/11465.patch
https://github.com/RIOT-OS/RIOT/pull/11465.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11465
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190429/33bea50b/attachment.html>


More information about the notifications mailing list