drivers/mtd: use XFA for pointers to defined MTDs #19465

gschorcht · 2023-04-11T16:57:06Z

Contribution description

This PR provides the support to hold pointers to defined MTDs within a XFA. The XFA allows

to access MTDs of different types (mtd_flashpage, mtd_sdcard, mtd_emulated, ...) by an index
to determine the number of MTDs defined in the system.

Testing procedure

To be defined once PR #19443 is merged because emulated MTDs will allow to test this PR on arbitrary boards.

Porting Guide

For external boards:

remove the MTD_NUMOF definition from board.h
add MTD_XFA_ADD(<mtd_dev>, <idx>); to the definition of <mtd_dev>.
MTD_0, MTD_1, … defines are no longer needed. Use mtd_dev_get(0), mtd_dev_get(1), … instead.

Issues/PRs references

Related to PR #19443

gschorcht · 2023-04-12T04:38:03Z

Since sys/usb/usbus/msc.c uses dynamic memory allocation anyway, the LUN descriptor array is allocated at runtime with malloc so that XFA_LEN can be used to determine the number of MTDs defined and thus the number of LUN descriptors required.

Alternatively, we could also define a XFA for the LUN descriptor array and an additional macro to add MTDs that has to be used to add MTDs to this XFA to make LUNs exposable at all.

riot-ci · 2023-04-12T04:52:36Z

Murdock results

✔️ PASSED

9da0cc5 drivers/mtd_sdmmc: use XFA with MTD pointers

Success	Failures	Total	Runtime
7937	0	7937	14m:04s

Artifacts

Documentation preview

dylad · 2023-04-12T13:21:58Z

I think a few boards are missing in the update, they manually declare mtd0 in their board.h

sensebox_samd21 (mtd_sdcard_spi)
remote-revb (mtd_sdcard_spi)
waspmote-pro (mtd_sdcard_spi)

There are also a bunch of extern mtd_dev_t *mtd0; present in the boards folder, are they still needed ?

Also I am now wondering if the module MTD_SDCARD_DEFAULT is still relevant with this patch ?

gschorcht · 2023-04-12T13:46:19Z

I think a few boards are missing in the update, they manually declare mtd0 in their board.h

sensebox_samd21 (mtd_sdcard_spi)

remote-revb (mtd_sdcard_spi)

waspmote-pro (mtd_sdcard_spi)

These board only declare mtd0 and define MTD_0 for their SD card interface but they don't define the MTDs. The MTDs are defined by mtd_sdcard_default as mtd0 instead.

Also I am now wondering if the module MTD_SDCARD_DEFAULT is still relevant with this patch ?

Yes, because mtd_sdcard_default is also defining the default SD card parameters and VFS mount points. It was introduced by PR #19216 to avoid that each board with SD card interface has to define them again and again.

There are also a bunch of extern mtd_dev_t *mtd0; present in the boards folder, are they still needed ?

I din't touch these declarations for the case that they are used in use code.

benpicco

That's a nice addition and as far as I can see the only brekaing change is that boards now have to define the MTD device as an XFA member - but that's an easy change.

Please squash.

drivers/include/mtd_emulated.h

19914: boards: complete SD Card MTD definition for several bords r=benpicco a=gschorcht ### Contribution description This PR completes the MTD definition for the following boards: - `seeedstudio-gd32` - `sipeed-longan-nano` including `sipeed-longan-nano-tft` - `waveshare-nrf52840-eval-kit` - ESP32x boards that have an SPI SD Card interface and use `mtd_sdcard_default` ### Testing procedure Green CI ### Issues/PRs references#19465 Prerequisite for PR #19465 Co-authored-by: Gunar Schorcht <[email protected]>

benpicco

looks good to me, feel free to squash

benpicco · 2023-09-15T16:07:03Z

boards/adafruit-itsybitsy-m4/board.c

@@ -54,7 +54,7 @@ static mtd_spi_nor_t samd51_nor_dev = {
 .params = &_samd51_nor_params,
 };

-mtd_dev_t *mtd0 = (mtd_dev_t *)&samd51_nor_dev;
+XFA(mtd_dev_xfa, 0) mtd_dev_t *mtd0 = (mtd_dev_t *)&samd51_nor_dev;


This is kind of ugly to have in each board - how about a macro

#define MTD_XFA_ADD(dev, idx) XFA(mtd_dev_xfa, (idx)) mtd_dev_t *mtd # (idx) = (mtd_dev_t *)&(dev);

Then this would be

Suggested change

XFA(mtd_dev_xfa, 0) mtd_dev_t *mtd0 = (mtd_dev_t *)&samd51_nor_dev;

MTD_XFA_ADD(samd51_nor_dev, 0);

The 0 in XFA(mtd_dev_xfa, 0) is not the index but the priority. But defining the macro as

#define MTD_XFA_ADD(dev, idx) XFA(mtd_dev_xfa, 0) mtd_dev_t *mtd # (idx) = (mtd_dev_t *)&(dev);

seems to be reasonable.

@benpicco I have added the macro according to your suggestion. Parameter idx is used now as priority so that the index corresponds to the position in the XFA provided that the indices are unique, start from 0 and are consecutive.

19914: boards: complete SD Card MTD definition for several bords r=benpicco a=gschorcht ### Contribution description This PR completes the MTD definition for the following boards: - `seeedstudio-gd32` - `sipeed-longan-nano` including `sipeed-longan-nano-tft` - `waveshare-nrf52840-eval-kit` - ESP32x boards that have an SPI SD Card interface and use `mtd_sdcard_default` ### Testing procedure Green CI ### Issues/PRs references#19465 Prerequisite for PR #19465 19915: drivers/lcd: support MCU 8080 8-bit parallel mode r=benpicco a=gschorcht ### Contribution description LCD driver ICs usually support - SPI serial mode, - MCU 8080 8-bit parallel mode and - MCU 8080 16-bit parallel mode. This PR extends the LCD display driver API to support the MCU 8080 8-/16-bit parallel modes and implements a GPIO-driven MCU 8080 8-bit parallel mode. The following features are already working locally and will be provided as follow-on PRs for which this PR is a prerequisite. - GPIO-driven bit-banging implementation of the 16-bit mode of the MCU 8080 parallel interface - Enabling the display on `stm32f723e-disco` and `stm32l496g-disco` using the feature above - Definition of a low-level API for the parallel modes using the LCD controller of the MCU - Using FMC for the display on `stm32f723e-disco` and `stm32l496g-disco` - Using LCD controller for the display of `esp32-wt32-sc01-plus` (PR #19917) ### Testing procedure The PR can be tested with PR #19917 on top of this PR. ``` BOARD=esp32s3-wt32-sc01-plus make -j8 -C tests/drivers/st77xx flash ``` The following video shows the test. **Please note** The test is pretty slow because the display has 480 x 320 pixels and the MCU 8080 8-bit parallel interface is realized by a GPIO-driven bit-banging implementation where each GPIO of the data bus is set separately. A follow-up PR will use the ESP32-S3 LCD controller and DMA for this board. This PR just defines the extension of the driver by the parallel interface and provides the bit-banging implementation for MCUs that don't have a LCD controller on chip. https://github.com/RIOT-OS/RIOT/assets/31932013/c1e3e3d7-05d9-4ca5-8fff-9a5eaca50fba ### Issues/PRs references 19919: drivers/st77xx: introduce rotation defines r=benpicco a=gschorcht ### Contribution description The PR introduces counterclockwise rotations for the definition of parameter `ST77XX_PARAM_ROTATION`. It is more intuitive and universal to use `ST77XX_ROTATION_{0,90,180,270}` instead of `ST77XX_ROTATION_{ST77XX_ROTATION_{VERT,VERT_FLIP,HORZ,HORZ_FLIP}`, especially because the orientation of the display may vary with respect to the orientation of the board. ### Testing procedure `tests/drivers/st77xx` should still work, for example: ``` BOARD=adafruit-pybadge make -C tests/drivers/st77xx flash ``` ``` BOARD=esp32s3-usb-otg make -j8 -C tests/drivers/st77xx flash ``` ### Issues/PRs references 19931: boards: fix documentation for GD32V boards and doxygen 1.9.4 r=benpicco a=gschorcht ### Contribution description This PR fixes some small problems in documentation of `sipeed-longan-nano`, `sipeed-longan-nano-tft` and `seeedstudio-gd32` for doxygen 1.9.4 that is used on `doc.riot-os.org`. Doxygen version 1.9.4 doesn't like anymore - single double quotes as symbol for the inches unit in the text - line breaks in `[]()` to avoid exhausting the 100 characters per line. See https://doc.riot-os.org/group__boards__sipeed__longan__nano.html for example. Doxygen 1.9.1 which is part of `riot-docker` container didn't have theses problems 😟 ### Testing procedure Documentation should be fixed. ### Issues/PRs references 19935: boards/nucleo64: fix SPI Arduino mapping for most boards r=benpicco a=maribu ### Contribution description Before, the Arduino SPI mapping for all Nucleo-64 boards was incorrect. With this, the situation improves to the following: - [x] nucleo-f030r8 - [ ] nucleo-f070rb - No SPI buses provided. - [x] nucleo-f072rb - [x] nucleo-f091rc - [x] nucleo-f103rb - [ ] nucleo-f302r8 - No SPI bus at D11, D12, D13 provided - [x] nucleo-f303re - [x] nucleo-f334r8 - [x] nucleo-f401re - [x] nucleo-f410rb - [x] nucleo-f411re - [x] nucleo-f446re - [x] nucleo-g070rb - [x] nucleo-g071rb - [x] nucleo-g431rb - [x] nucleo-g474re - [x] nucleo-l053r8 - [x] nucleo-l073rz - [x] nucleo-l152re - No SPI bus at D11, D12, D13 provided - [x] nucleo-l452re - [x] nucleo-l476rg - [x] nucleo-wl55jc The remaining offenders still need to be fixed, but that is better done one PR at a time. ### Testing procedure - Check if the SPI device provided in the given `boards/<BOARD_NAME>/incude/periph_conf.h` is indeed `SPI_DEV(0)`, or in `periph_conf.h` the correct SPI dev is found - this should be fine for all boards above, except for the unchecked ones or: - run #19932: The SPI test should pass now ### Issues/PRs references Bug found in #19932 (comment) Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: Marian Buschsieweke <[email protected]>

19914: boards: complete SD Card MTD definition for several bords r=benpicco a=gschorcht ### Contribution description This PR completes the MTD definition for the following boards: - `seeedstudio-gd32` - `sipeed-longan-nano` including `sipeed-longan-nano-tft` - `waveshare-nrf52840-eval-kit` - ESP32x boards that have an SPI SD Card interface and use `mtd_sdcard_default` ### Testing procedure Green CI ### Issues/PRs references#19465 Prerequisite for PR #19465 19915: drivers/lcd: support MCU 8080 8-bit parallel mode r=benpicco a=gschorcht ### Contribution description LCD driver ICs usually support - SPI serial mode, - MCU 8080 8-bit parallel mode and - MCU 8080 16-bit parallel mode. This PR extends the LCD display driver API to support the MCU 8080 8-/16-bit parallel modes and implements a GPIO-driven MCU 8080 8-bit parallel mode. The following features are already working locally and will be provided as follow-on PRs for which this PR is a prerequisite. - GPIO-driven bit-banging implementation of the 16-bit mode of the MCU 8080 parallel interface - Enabling the display on `stm32f723e-disco` and `stm32l496g-disco` using the feature above - Definition of a low-level API for the parallel modes using the LCD controller of the MCU - Using FMC for the display on `stm32f723e-disco` and `stm32l496g-disco` - Using LCD controller for the display of `esp32-wt32-sc01-plus` (PR #19917) ### Testing procedure The PR can be tested with PR #19917 on top of this PR. ``` BOARD=esp32s3-wt32-sc01-plus make -j8 -C tests/drivers/st77xx flash ``` The following video shows the test. **Please note** The test is pretty slow because the display has 480 x 320 pixels and the MCU 8080 8-bit parallel interface is realized by a GPIO-driven bit-banging implementation where each GPIO of the data bus is set separately. A follow-up PR will use the ESP32-S3 LCD controller and DMA for this board. This PR just defines the extension of the driver by the parallel interface and provides the bit-banging implementation for MCUs that don't have a LCD controller on chip. https://github.com/RIOT-OS/RIOT/assets/31932013/c1e3e3d7-05d9-4ca5-8fff-9a5eaca50fba ### Issues/PRs references Co-authored-by: Gunar Schorcht <[email protected]>

benpicco

Please squash!

gschorcht · 2023-10-05T16:34:25Z

@benpicco Can we merge it?

gschorcht · 2023-10-18T06:23:45Z

@benpicco Unfortunatly, we just forgot to merge it before the hard freeze 🙈

benpicco · 2023-10-18T10:19:19Z

Sorry I wanted to check how this deals with external boards first, but it gives a clear

error: "MTD_NUMOF" redefined

error and no silent failure. So I added a short porting guide to the PR description that can be added to the release notes.

benpicco · 2023-10-19T14:42:30Z

bors merge

bors · 2023-10-20T00:40:01Z

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

gschorcht requested review from bergzand, dylad, aabadie, benpicco, haukepetersen and basilfx as code owners April 11, 2023 16:57

dylad added the State: waiting for other PR State: The PR requires another PR to be merged first label Apr 11, 2023

benpicco requested a review from kaspar030 April 11, 2023 17:04

gschorcht added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 12, 2023

gschorcht requested review from smlng, leandrolanzieri, MichelRottleuthner and fjmolinas as code owners April 12, 2023 06:04

github-actions bot added the Area: tests Area: tests and testing framework label Apr 12, 2023

gschorcht force-pushed the drivers/mtd_xfa branch from 49d1cb3 to a132a8e Compare April 12, 2023 12:19

gschorcht removed the State: waiting for other PR State: The PR requires another PR to be merged first label Apr 12, 2023

benpicco added the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label Apr 28, 2023

benpicco reviewed Apr 28, 2023

View reviewed changes

drivers/include/mtd_emulated.h Outdated Show resolved Hide resolved

gschorcht force-pushed the drivers/mtd_xfa branch from 29a01ce to 20936d6 Compare September 7, 2023 17:01

gschorcht added this to the Release 2023.10 milestone Sep 15, 2023

benpicco reviewed Sep 15, 2023

View reviewed changes

gschorcht force-pushed the drivers/mtd_xfa branch from 7f65c67 to f98cd31 Compare October 2, 2023 05:14

benpicco approved these changes Oct 2, 2023

View reviewed changes

gschorcht added 9 commits October 2, 2023 12:27

drivers/mtd: store MTD pointers as XFA

bd67236

drivers/mtd_default: use XFA with MTD pointers

739cb53

drivers/mtd_sdcard: use XFA with MTD pointers

e5c541b

boards: use XFA with MTD pointers for defined MTDs

46040a4

cpu/esp_common: use XFA with MTD pointers for Flash MTD

d535277

sys/usbus/msc: use MTD pointers in XFA for LUN handling

a783a15

tests/mtd_raw: fix compilation XFA with MTD pointers

61c2259

drivers/mtd_emulated: use XFA with MTD pointers

e70e6d7

drivers/mtd_sdmmc: use XFA with MTD pointers

9da0cc5

gschorcht force-pushed the drivers/mtd_xfa branch from 836ef12 to 9da0cc5 Compare October 2, 2023 10:30

gschorcht added the CI: ready for merge train 🚃 PR is ready to be merged and awaiting the next merge train label Oct 3, 2023

bors bot merged commit 03d3874 into RIOT-OS:master Oct 20, 2023
26 checks passed

benpicco mentioned this pull request Nov 10, 2023

drivers/mtd: rename mtd_default_get_dev() -> mtd_dev_get() #20078

Merged

gschorcht mentioned this pull request Nov 27, 2023

drivers/mtd: fix the order of entries in the MTD pointer XFA mtd_dev_xfa #20113

Merged

MrKevinWeiss modified the milestones: Release 2023.10, Release 2024.01 Feb 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers/mtd: use XFA for pointers to defined MTDs #19465

drivers/mtd: use XFA for pointers to defined MTDs #19465

gschorcht commented Apr 11, 2023 •

edited by benpicco

Loading

gschorcht commented Apr 12, 2023

riot-ci commented Apr 12, 2023 •

edited

Loading

dylad commented Apr 12, 2023 •

edited

Loading

gschorcht commented Apr 12, 2023

benpicco left a comment

benpicco left a comment

benpicco Sep 15, 2023

gschorcht Sep 17, 2023 •

edited

Loading

gschorcht Oct 1, 2023

benpicco left a comment

gschorcht commented Oct 5, 2023

gschorcht commented Oct 18, 2023

benpicco commented Oct 18, 2023

benpicco commented Oct 19, 2023

bors bot commented Oct 20, 2023

	XFA(mtd_dev_xfa, 0) mtd_dev_t mtd0 = (mtd_dev_t )&samd51_nor_dev;
	MTD_XFA_ADD(samd51_nor_dev, 0);

drivers/mtd: use XFA for pointers to defined MTDs #19465

drivers/mtd: use XFA for pointers to defined MTDs #19465

Conversation

gschorcht commented Apr 11, 2023 • edited by benpicco Loading

Contribution description

Testing procedure

Porting Guide

Issues/PRs references

gschorcht commented Apr 12, 2023

riot-ci commented Apr 12, 2023 • edited Loading

Murdock results

Artifacts

dylad commented Apr 12, 2023 • edited Loading

gschorcht commented Apr 12, 2023

benpicco left a comment

Choose a reason for hiding this comment

benpicco left a comment

Choose a reason for hiding this comment

benpicco Sep 15, 2023

Choose a reason for hiding this comment

gschorcht Sep 17, 2023 • edited Loading

Choose a reason for hiding this comment

gschorcht Oct 1, 2023

Choose a reason for hiding this comment

benpicco left a comment

Choose a reason for hiding this comment

gschorcht commented Oct 5, 2023

gschorcht commented Oct 18, 2023

benpicco commented Oct 18, 2023

benpicco commented Oct 19, 2023

bors bot commented Oct 20, 2023

gschorcht commented Apr 11, 2023 •

edited by benpicco

Loading

riot-ci commented Apr 12, 2023 •

edited

Loading

dylad commented Apr 12, 2023 •

edited

Loading

gschorcht Sep 17, 2023 •

edited

Loading