drivers: sdhc: Add MAX32 SDHC driver #83464

mertekren · 2024-12-31T08:09:53Z

This PR adds SDHC support for max32666fthr.

Implement SDHC driver for MAX32666fthr
Add sdhc0 instances for MAX32666 .dtsi file
Add SDHC to MAX32666fthr board driver list

tests:
.\tests\drivers\sdhc

.\samples\subsys\fs\fs_sample\

.\tests\drivers\disk\disk_performance

Dependencies:

chore(Other): Add zephyr sdhc related wrapper hal_adi#12

zephyrbot · 2025-01-03T08:25:54Z

The following west manifest projects have changed revision in this Pull Request:

Name	Old Revision	New Revision	Diff
hal_adi	zephyrproject-rtos/hal_adi@`a9ca7d1` (`main`)	zephyrproject-rtos/hal_adi#12	zephyrproject-rtos/hal_adi#12/files

⛔ DNM label due to: 1 project with PR revision

Note: This message is automatically posted and updated by the Manifest GitHub Action.

danieldegrasse · 2025-01-14T17:18:21Z

dts/bindings/sdhc/adi,max32-sdhc.yaml

+  reg:
+    required: true
+
+  sd-bus-voltage:


Does this need to be set at boot time? Typically it is configured by the SDMMC framework when the card is probed. Or is this indicating the voltage the controller actually supports?

very good note, thanks.
hal_adi side does this only in sdhc init and we thought we need to pass this voltage information in devicetree. there could be cases that sdmmc is not used, and user might want to set it via overlay etc i think?

Cases when sdmmc isn't used? Do you mean the MMC framework? SD cards are going to boot at 3.3V, so the controller should be operating at that voltage at boot time. If this indicates what the controller supports, that is fine- but currently it looks like this is setting the signalling voltage during SD init.

boards/adi/max32666fthr/doc/index.rst

drivers/sdhc/sdhc_max32.c

danieldegrasse · 2025-01-14T17:22:32Z

drivers/sdhc/sdhc_max32.c

+	return ENOTSUP;
+}
+
+static int sdhc_max32_set_io(const struct device *dev, struct sdhc_io *ios)


A few comments about this function:

can we support setting voltage in this function? Or does the hardware not support adjusting that dynamically? It likely does if the controller supports UHS cards.

very good note. i think we can support it. however, our hal doesn't have this functionality yet. i'll add this as a seperate task in future, unless it's a requirement?

Not la requirement if you only want to support slower card speeds at 3.3V. However if you want to support UHS cards with this driver, voltage setting needs to be done dynamically.

danieldegrasse · 2025-01-14T17:23:26Z

drivers/sdhc/sdhc_max32.c

+	 * remove this with a better solution in future.
+	 */
+	k_sleep(K_MSEC(1));
+	ret = MXC_SDHC_SendCommand(&sd_cmd_cfg);


Question here- is this function blocking? Or does it use interrupts within the HAL layer?

yes it's blocking. it loops in while() until register is complete/timeout. we'll try to add interrupt functionality in future.

drivers/sdhc/sdhc_max32.c

samples/subsys/fs/fs_sample/boards/max32666fthr_max32666_cpu0.overlay

tests/drivers/sdhc/boards/max32666fthr_max32666_cpu0.overlay

tests/drivers/disk/disk_performance/boards/max32666fthr_max32666_cpu0.overlay

Pull in support that adds sdhc header from hal_adi Signed-off-by: Mert Ekren <mert.ekren@analog.com>

This commit adds MAX32666 SDHC driver. Co-Authored-By: Anil Kara <Anil.Kara@analog.com> Co-Authored-By: Tahsin Mutlugun <Tahsin.Mutlugun@analog.com> Signed-off-by: Mert Ekren <mert.ekren@analog.com>

Include sdhc0 in MAX32666 devicetree and add devicetree bindings for MAX32 SDHC driver Signed-off-by: Mert Ekren <mert.ekren@analog.com>

Add SDHC to supported list for MAX32666FTHR Signed-off-by: Mert Ekren <mert.ekren@analog.com>

Add MAX32666fthr for fs_sample testing Signed-off-by: Mert Ekren <mert.ekren@analog.com>

mertekren · 2025-01-24T15:22:04Z

@mertekren I think you messed up your update to west.yml as it's doing a lot more than updating hal_adi :)

yes i rebased wrong, should be good now. thanks! :)

Add MAX32666fthr sdhc to tests/drivers/disk/disk_performance Signed-off-by: Mert Ekren <mert.ekren@analog.com>

danieldegrasse · 2025-01-27T19:33:26Z

drivers/sdhc/sdhc_max32.c

+	sdhc_data->props.host_caps.sdma_support = true;
+	sdhc_data->props.host_caps.suspend_res_support = true;
+	sdhc_data->props.host_caps.vol_330_support = true;
+	sdhc_data->props.host_caps.vol_300_support = true;


This needs to be updated- if the driver does not support reconfiguring the voltage, that should be reported to the host stack here by setting the vol_300_support and vol_180_support fields to false

danieldegrasse · 2025-01-27T19:38:17Z

drivers/sdhc/sdhc_max32.c

+		return ret;
+	}
+
+	cfg.bus_voltage = sdhc_config->bus_volt;


This cannot be configured statically. If memory serves, the SDHC spec specifies cards should start at 3.3V, and then switch to 1.8V during initialization. So there are two options here:

support dynamic voltage configuration

only support cards at 3.3V

Have you been able to initialize a card at 1.8V using this mechanism? Maybe my memory is wrong, but I thought that only eMMC could boot at 1.8V. SD cards need to switch to 1.8V at runtime

danieldegrasse · 2025-01-27T19:39:52Z

drivers/sdhc/sdhc_max32.c

+	}
+
+	/* init delay, without it applications fail. 5ms found empirically */
+	k_sleep(K_MSEC(5));


Is there some busy bit in the hardware we could poll here? Honestly, it sounds to me like what this delay is doing is giving the SD host controller time to send the 74 initialization clocks required at power on. That should not occur here though, it should occur each time the SD controller toggles power on to the card (in the sdhc_max32_set_io function)

danieldegrasse · 2025-01-27T19:41:12Z

drivers/sdhc/sdhc_max32.c

+	return ENOTSUP;
+}
+
+static int sdhc_max32_set_io(const struct device *dev, struct sdhc_io *ios)


Not la requirement if you only want to support slower card speeds at 3.3V. However if you want to support UHS cards with this driver, voltage setting needs to be done dynamically.

danieldegrasse · 2025-01-27T19:42:45Z

drivers/sdhc/sdhc_max32.c

+	enum sdhc_clock_speed speed = ios->clock;
+	unsigned int clk_div = 0;
+
+	MXC_SDHC_PowerDown();


Does this function remove power from the card? If so, you should not unconditionally toggle power to the card here- this function is called multiple times during card initialization to change clocks/IO properties, and removing power to the card will effectively reset the negotiation process.

danieldegrasse · 2025-01-27T19:43:28Z

dts/arm/adi/max32/max32666.dtsi

Commit with DT nodes and bindings should be before the commit with the driver code

danieldegrasse · 2025-01-27T19:47:13Z

dts/bindings/sdhc/adi,max32-sdhc.yaml

+  reg:
+    required: true
+
+  sd-bus-voltage:


Cases when sdmmc isn't used? Do you mean the MMC framework? SD cards are going to boot at 3.3V, so the controller should be operating at that voltage at boot time. If this indicates what the controller supports, that is fine- but currently it looks like this is setting the signalling voltage during SD init.

mertekren force-pushed the add-max32-sdhc branch from 9d6a657 to d0b3a61 Compare December 31, 2024 08:26

mertekren changed the title ~~sdhc: max32: Add sdhc for max32666fthr~~ drivers: sdhc: Add MAX32 SDHC driver Jan 3, 2025

zephyrbot added manifest manifest-hal_adi DNM This PR should not be merged (Do Not Merge) labels Jan 3, 2025

mertekren force-pushed the add-max32-sdhc branch from 137184f to e474287 Compare January 8, 2025 14:40

mertekren marked this pull request as ready for review January 9, 2025 06:46

zephyrbot added area: Disk Access platform: ADI Analog Devices, Inc. area: File System area: Samples Samples labels Jan 9, 2025

zephyrbot requested review from danieldegrasse, de-nordic, decsny, galak, jfischer-no, kartben, Laczen, MaureenHelm, microbuilder, nashif, ozersa and ttmut January 9, 2025 06:47

zephyrbot assigned danieldegrasse Jan 9, 2025

danieldegrasse reviewed Jan 14, 2025

View reviewed changes

mertekren force-pushed the add-max32-sdhc branch from 167ff00 to 5281727 Compare January 17, 2025 09:11

MaureenHelm added this to the v4.1.0 milestone Jan 20, 2025

MaureenHelm reviewed Jan 20, 2025

View reviewed changes

mertekren force-pushed the add-max32-sdhc branch from 5281727 to 3360ad1 Compare January 24, 2025 11:38

kartben removed manifest-bsim manifest-hal_microchip manifest-hal_infineon manifest-hal_ethos_u manifest-hal_espressif manifest-nrf_hw_models labels Jan 24, 2025

mertekren and others added 3 commits January 24, 2025 18:18

manifest: hal_adi: Pull sdhc header

49566f8

Pull in support that adds sdhc header from hal_adi Signed-off-by: Mert Ekren <mert.ekren@analog.com>

drivers: sdhc: Add MAX32666 SDHC driver

a9ff6b7

This commit adds MAX32666 SDHC driver. Co-Authored-By: Anil Kara <Anil.Kara@analog.com> Co-Authored-By: Tahsin Mutlugun <Tahsin.Mutlugun@analog.com> Signed-off-by: Mert Ekren <mert.ekren@analog.com>

dts: Add MAX32666 SDHC nodes and bindings

37be4a7

Include sdhc0 in MAX32666 devicetree and add devicetree bindings for MAX32 SDHC driver Signed-off-by: Mert Ekren <mert.ekren@analog.com>

kartben removed the manifest-hal_rpi_pico label Jan 24, 2025

mertekren added 2 commits January 24, 2025 18:19

boards: Add SDHC driver for MAX32666FTHR

c239c8e

Add SDHC to supported list for MAX32666FTHR Signed-off-by: Mert Ekren <mert.ekren@analog.com>

samples: drivers: sdhc: fs_sample: Add MAX32666fthr

df309df

Add MAX32666fthr for fs_sample testing Signed-off-by: Mert Ekren <mert.ekren@analog.com>

kartben removed manifest-hal_atmel manifest-nrf_wifi manifest-littlefs manifest-hostap manifest-hal_silabs manifest-libmctp manifest-hal_st manifest-hal_nxp manifest-hal_nordic manifest-lvgl manifest-hal_renesas manifest-openthread manifest-percepio labels Jan 24, 2025

mertekren force-pushed the add-max32-sdhc branch from f8746da to f846402 Compare January 24, 2025 15:19

tests: drivers: sdhc: Add MAX32666fthr to disk performance test

e890027

Add MAX32666fthr sdhc to tests/drivers/disk/disk_performance Signed-off-by: Mert Ekren <mert.ekren@analog.com>

mertekren force-pushed the add-max32-sdhc branch from f846402 to e890027 Compare January 27, 2025 06:31

danieldegrasse reviewed Jan 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: sdhc: Add MAX32 SDHC driver #83464

drivers: sdhc: Add MAX32 SDHC driver #83464

mertekren commented Dec 31, 2024 •

edited

Loading

zephyrbot commented Jan 3, 2025 •

edited

Loading

danieldegrasse Jan 14, 2025

mertekren Jan 17, 2025

danieldegrasse Jan 27, 2025

danieldegrasse Jan 14, 2025

mertekren Jan 17, 2025

danieldegrasse Jan 27, 2025

danieldegrasse Jan 14, 2025

mertekren Jan 17, 2025

mertekren commented Jan 24, 2025

danieldegrasse Jan 27, 2025

danieldegrasse Jan 27, 2025

danieldegrasse Jan 27, 2025

danieldegrasse Jan 27, 2025

danieldegrasse Jan 27, 2025

danieldegrasse Jan 27, 2025

danieldegrasse Jan 27, 2025

drivers: sdhc: Add MAX32 SDHC driver #83464

Are you sure you want to change the base?

drivers: sdhc: Add MAX32 SDHC driver #83464

Conversation

mertekren commented Dec 31, 2024 • edited Loading

zephyrbot commented Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mertekren commented Jan 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mertekren commented Dec 31, 2024 •

edited

Loading

zephyrbot commented Jan 3, 2025 •

edited

Loading