issue with CONFIG_MMC_VOLUME_NAME

[KlimRadostev]

Describe the bug observed issue with CONFIG_MMC_VOLUME_NAME, in https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/disk/mmc_subsys.c#L134 could cause issues in a situation where 2 mmc subsystems need to be initialized as it will marry the driver only to 1 label.

would the correct way not be to leverage the device trees Label otherwise we will get collisions when attempting to use disk_access APIs?

[github-actions[bot]]

Hi @KlimRadostev! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

[henrikbrixandersen]

Please use our bug template when reporting bugs. You need to edit this issue to include the information requested in https://github.com/zephyrproject-rtos/zephyr/blob/main/.github/ISSUE_TEMPLATE/001_bug_report.md

[danieldegrasse]

would the correct way not be to leverage the device trees Label otherwise we will get collisions when attempting to use disk_access APIs?

Yes, I think it would be better to use device tree labels here. We could probably add support for using a devicetree property here. The ramdisk driver does this: https://github.com/zephyrproject-rtos/zephyr/blob/a6e672f9b2368e6d011da189afd462fec599fd0d/drivers/disk/ramdisk.c#L164. This is a case where we'd be doing software configuration with devicetree though. @decsny, any thoughts on a way to solve this without devicetree?

[decsny]

would the correct way not be to leverage the device trees Label otherwise we will get collisions when attempting to use disk_access APIs?

Yes, I think it would be better to use device tree labels here. We could probably add support for using a devicetree property here. The ramdisk driver does this:

https://github.com/zephyrproject-rtos/zephyr/blob/a6e672f9b2368e6d011da189afd462fec599fd0d/drivers/disk/ramdisk.c#L164 . This is a case where we'd be doing software configuration with devicetree though. @decsny, any thoughts on a way to solve this without devicetree?

personally think the DT label property is exactly for this purpose, but clearly our documentation and recent historical efforts have been hostile to using this property, although I'm actually not 100% clear why. I know it was used improperly all over the place in zephyr in the past but I think deprecating it and banning it completely might have gone too far.

@galak

This is from the devicetree v0.4 spec. It seems like this is the exact situation where it should be used. Do you disagree?

[decsny]

@danieldegrasse I am assuming that this "name" of a disk is intended to be human readable, otherwise this is a moot discussion. Can you confirm the reason for this. Right now I only see it used in LOG_DBG statements.

[jfischer-no]

would the correct way not be to leverage the device trees Label otherwise we will get collisions when attempting to use disk_access APIs?

Yes, I think it would be better to use device tree labels here. We could probably add support for using a devicetree property here. The ramdisk driver does this:

https://github.com/zephyrproject-rtos/zephyr/blob/a6e672f9b2368e6d011da189afd462fec599fd0d/drivers/disk/ramdisk.c#L164

disk_name property refers to disk_info.name, which is used as disk identifier. IMO disk_name is less confusing for users as label property. Only specific names can be used if the filesystem is ELM FATFS, see https://docs.zephyrproject.org/latest/services/file_system/index.html#c.fs_mount. Why cannot zephyr,mmc-disk.yaml aligned with zephyr,ram-disk.yam? Also, it should be move do dts/bindings/disk. (Btw, I described the problem to you in Prague. Same applies to SDMMC)

[decsny]

would the correct way not be to leverage the device trees Label otherwise we will get collisions when attempting to use disk_access APIs?

Yes, I think it would be better to use device tree labels here. We could probably add support for using a devicetree property here. The ramdisk driver does this: https://github.com/zephyrproject-rtos/zephyr/blob/a6e672f9b2368e6d011da189afd462fec599fd0d/drivers/disk/ramdisk.c#L164

disk_name property refers to disk_info.name, which is used as disk identifier. IMO disk_name is less confusing for users as label property. Only specific names can be used if the filesystem is ELM FATFS, see https://docs.zephyrproject.org/latest/services/file_system/index.html#c.fs_mount. Why cannot zephyr,mmc-disk.yaml aligned with zephyr,ram-disk.yam? Also, it should be move do dts/bindings/disk. (Btw, I described the problem to you in Prague. Same applies to SDMMC)

As in the screenshot I posted, the label meaning should be specified by the binding. So there should not be any confusion. But I am still not clear if this identifier really needs to be a human readable string, I don't think LOG_DBG is enough justification for that, but maybe I am not seeing something.

[danieldegrasse]

As in the screenshot I posted, the label meaning should be specified by the binding. So there should not be any confusion. But I am still not clear if this identifier really needs to be a human readable string, I don't think LOG_DBG is enough justification for that, but maybe I am not seeing something.

I think it does need to be human readable, or at least able to be set in a way that is deterministic between builds (so we can't just add the instance number or something similar to make it unique)

The reason being that the user's application would access the disk by the disk name if there was a need to make direct I/O calls to the disk itself, without a file system layer