Closed humanoid2050 closed 2 years ago
andy-shev
commented
2 years ago As it appears in another custom design (see @Joel2B's page) the polarity is Active Low. So, that's what we need to support at minimum, but I will look if we can modify to catch on both edges reliably.
htot
commented
2 years ago @humanoid2050: @andy-shev has added a series of patches to his repo here: https://github.com/andy-shev/linux/commits/eds-acpi 2 patches should be needed https://github.com/andy-shev/linux/commit/eca1440d2029d5ebea5091778086a047b85f3fcd and https://github.com/andy-shev/linux/commit/b7f992c1f181df38365b4771a7ab170a682e02b2 to fix this issue.
Would you care to test on your hardware?
humanoid2050
commented
2 years ago @htot I generated a patch based on the two commits from @andy-shev and applied it to the gatesgarth branch of https://github.com/htot/meta-intel-edison.git. Looks like it worked! The only part I wasn't sure about was in drivers/mmc/host/sdhci-pci-core.c where I removed slot->host->mmc->caps |= MMC_CAP_CD_WAKE;. That line was not present in Andy's diffs (before or after patch).
andy-shev
commented
2 years ago @humanoid2050, thanks for testing!
@htot I generated a patch based on the two commits from @andy-shev and applied it to the gatesgarth branch of https://github.com/htot/meta-intel-edison.git. Looks like it worked!
Would you be able to mention this in the mailing list (informally, maybe from some burned email)?
https://lore.kernel.org/linux-mmc/20211005102430.63716-1-andriy.shevchenko@linux.intel.com/T/#u
The only part I wasn't sure about was in
drivers/mmc/host/sdhci-pci-core.cwhere I removedslot->host->mmc->caps |= MMC_CAP_CD_WAKE;. That line was not present in Andy's diffs (before or after patch).
If it’s needed it should be tested and implemented in a separate change anyway.
humanoid2050
commented
2 years ago @andy-shev Just for my own edification, what is MMC_CAP_CD_WAKE? It's added to the code as part of the patch meta-intel-edison/meta-intel-edison-bsp/recipes-kernel/linux/files/0001-mmc-sdhci-pci-Read-card-detect-from-ACPI-for-Intel-M.patch
From eb3e11ccaa2f15e9b7b63fd7ce98c14886646d27 Mon Sep 17 00:00:00 2001
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Fri, 10 Sep 2021 00:14:06 +0300
Subject: [PATCH 1/4] mmc: sdhci-pci: Read card detect from ACPI for Intel
Merrifield
Intel Merrifield platform had been converted to use ACPI enumeration.
However, the driver missed an update to retrieve card detect GPIO.
Fix it here.
Fixes: 4590d98f5a4f ("sfi: Remove framework for deprecated firmware")
BugLink: https://github.com/edison-fw/meta-intel-edison/issues/135
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/mmc/host/sdhci-pci-core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index be19785227fe..5aac2b1edf03 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -1341,6 +1341,9 @@ static int intel_mrfld_mmc_probe_slot(struct sdhci_pci_slot *slot)
MMC_CAP_1_8V_DDR;
break;
case INTEL_MRFLD_SD:
+ slot->cd_idx = 0;
+ slot->cd_override_level = true;
+ slot->host->mmc->caps |= MMC_CAP_CD_WAKE;
slot->host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
break;
case INTEL_MRFLD_SDIO:
--
2.30.2@andy-shev Just for my own edification, what is MMC_CAP_CD_WAKE?
It's a feature to mate CD signal a wake-source. It means if system is in sleep mode the insertion or removal of SD card wakes the system up.
It's added to the code as part of the patch
meta-intel-edison/meta-intel-edison-bsp/recipes-kernel/linux/files/0001-mmc-sdhci-pci-Read-card-detect-from-ACPI-for-Intel-M.patch
It shouldn't be there anymore. @htot?
htot
commented
2 years ago Yes the 4 mmc patches here need to be removed while testing these 2 new ones.
Saturday I will have time to remove the 4 and add the 6 eca1440d...6f87ed00 from https://github.com/andy-shev/linux/commits/eds-acpi
htot
commented
2 years ago I just force pushed to the gatesgarth branch for testing here. I backported the quirk patches to the 5.10.63 and 5.10.59-rt52 kernels as well and tested 5.14 and 5.10.63 on Edison-Arduino board. @humanoid2050 can you test on your platform as well? If this is working I want to release Gatesgarth and move on to Hardknott. Thanks!
humanoid2050
commented
2 years ago @htot I built and tested 5.10.63 and 5.14.0 kernels. Success on both. I still can't see the card from uboot though. Is that a separate set of patching and tests?
htot
commented
2 years ago Unless I missed something here U-Boot should have a working SD card on Intel Edison. Not sure if it should work with inverted CD line. @andy-shev ?
humanoid2050
commented
2 years ago I'm tinkering. Do the ACPI tables apply to uBoot? Alterations I make to the ACPI patches only seem to impact the kernel, not uboot itself.
htot
commented
2 years ago No, U-Boot provides the tables to the kernel but does not use them. I believe the patches to arch/x86/dts/edison.dts are for U-Boot itself.
humanoid2050
commented
2 years ago Ok, that's kind of what I was coming up with. I'm looking at the 0004-x86-edison-Enable-SD-card-detect-on-the-SDHCI-2.patch file. I'm not sure why, but altering the GPIO flags does not actually alter the behavior of U-Boot. Even removing the patch completely does not alter the behavior of U-Boot on my system. It looks like the kernel is behaving properly now. Should this U-Boot issue be opened as its own item against Hardknott?
htot
commented
2 years ago This patch actually enables the SD card in U-Boot for Edison-Arduino, i.e. allows U-Boot to boot from SD card. I would expect that changing GPIO_ACTIVE_LOW to GPIO_ACTIVE_HIGH would work for you. Maybe @andy-shev knows.
humanoid2050
commented
2 years ago @htot That's what I assumed too.
htot
commented
2 years ago Ok, that's kind of what I was coming up with. I'm looking at the
0004-x86-edison-Enable-SD-card-detect-on-the-SDHCI-2.patchfile. I'm not sure why, but altering the GPIO flags does not actually alter the behavior of U-Boot. Even removing the patch completely does not alter the behavior of U-Boot on my system. It looks like the kernel is behaving properly now. Should this U-Boot issue be opened as its own item against Hardknott?
According to @andy-shev this patch 0004-x86-edison-Enable-SD-card-detect-on-the-SDHCI-2.patch and 0003-x86-edison-Enable-GPIO-controller.patch are experimental and should not be needed to make SD card inside U-Boot. In fact on your hardware they might even prevent the card from working.
When you say:
For some reason, U-Boot doesn't detect the SD card either. Never has.
how do you know? When you boot with the SD card inserted and inside U-Boot do ls mmc 1 (iirc) what do you get?
andy-shev
commented
2 years ago The kernel patch is in upstream now, pending for v5.15-rc6.
I'm closing this now as patches to fix the regression are either in upstream (U-Boot) or pending (Linux kernel).
The SD card issue in U-Boot requires a separate bug report to be created.
@humanoid2050, thanks for your report and testing!
humanoid2050
commented
2 years ago @htot @andy-shev Thanks for helping sort out these issues. I definitely would not have gotten it on my own.
I will make a new issue for the U-Boot SD card problem.
htot
commented
2 years ago I tested pending V3 by applying to 5.14.0 1..5.
Also tested by backporting V3 1..2 to 5.10.63.
Patch 3..5 do apply but cause a build error:
kernel-source/arch/x86/platform/intel-mid/device_libs/platform_mrfld_sd.c:12:10: fatal error: linux/mmc/sdhci-pci-data.h: No such file or directory
| 12 | #include <linux/mmc/sdhci-pci-data.h>
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~Instead of fixing I just dropped as these 3 only remove dead code.
After building and flashing dunfell, the SD card does not work. I tried btrfs/non-btrfs, and debian/non-debian variants. In all cases, the SD card (attached via a Sparkfun microSD block) does not appear in the output of
lsblkorls /dev. The output of dmesg indicates that the kernel is detecting the hardware on some level, but not the card itself.Reverting to the zeus branch and rebuilding causes the sd card to appear without hardware changes.
Rebuilding dunfell brings back the issue.