Bug 177101

Created attachment 241211 [details]
dmesg output

Created attachment 241221 [details]
DSDT

does the problem still exist if you set CONFIG_PINCTRL_CHERRYVIEW=y?

(In reply to Zhang Rui from comment #3)
> does the problem still exist if you set CONFIG_PINCTRL_CHERRYVIEW=y?

No, with CONFIG_PINCTRL_CHERRYVIEW=y card detect works just fine! Thanks a lot!

So this is a problem that the GPIO dependency, which exists in _CRS, is not properly described via _DEP method, and results in a racing issue.
If we make GPIO controller driver load before the GPIO consumer driver (force built-in in this case), then problem is gone.

I'm thinking if we should propose a patch to handle the dependency described in _CRS.

Created attachment 246211 [details]
dsdt with GPO3 described in _DEP

another to confirm this is to compile and apply the new dsdt attached, which has GPIO controller dependency described in _DEP method.
Please check if the problem still exists, even with CONFIG_PINTRL_CHERRYTRAIL=m.

Build and tested with:
CONFIG_PINCTRL_CHERRYVIEW=m
and used your attached dsdt. Card detect doesn't work with that combination.

Created attachment 246581 [details]
patch to add dependency checking for sdhci-acpi driver

oops, I forgot one thing.
You need to applied this patch as well as the customized DSDT.
If this works, then I will work out another two patches to do the hack in kernel, instead of customized DSDT.

With this patch applied, the eMMC is not detected, and boot ends up in initramfs' shell.
I insmod'ed the pinctrl-cherryview module from an usbdrive, but nothing happen. I rmmod'ed the sdhci, sdhci-acpi and mmc_core.
Then I insmod'ed these modules again. But from dmesg's point of view nothing happened.

I don't get you added a check for mailbox device:
        !strcmp(info->hardware_id.string, "INT33BD"));
I was expecting INT33BB - sd controller.

So I patched your patch changing INT33BD to INT33BB, and tested. But nothing changed.

        Device (SHC1)
        {
            Name (_HID, "80860F14" /* Intel Baytrail SDIO/MMC Host Controller */)  // _HID: Hardware ID
            Name (_CID, "PNP0D40" /* SDA Standard Compliant SD Host Controller */)  // _CID: Compatible ID
            Name (_DDN, "Intel(R) SD Card Controller - 80862296")  // _DDN: DOS Device Name
            Name (_UID, 0x03)  // _UID: Unique ID
            Name (_HRV, One)  // _HRV: Hardware Revision
            Name (_DEP, Package (0x02)  // _DEP: Dependencies
            {
                MBID,
                I2C7
            })
            ...
         }


This is the description of sdhci-acpi 80860F14:03.
what I'm doing the customized dsdt is to add GPO3 into the _DEP package.
what I'm doing in the patch is to add check for _DEP when SHC1 driver is probed.

With both applied, it should work in this way
1.During acpi initialization, we know that SHC1 has 3 dependencies (MBID, I2C7 and GPO3), and increase the dep_unmet count. The hacks I do in drivers/acpi/scan.c is to ignore the dependency of MBID device, which does not have a driver in Linux.
2.during I2C7 and GPO3 driver probing, it will decrease the dep_unmet count
3.for SHC1, it is probed only if dep_unmet is decreased to 0, which means all the dependencies are solved.

> I don't get you added a check for mailbox device:
>         !strcmp(info->hardware_id.string, "INT33BD"));
> I was expecting INT33BB - sd controller.
> 
> So I patched your patch changing INT33BD to INT33BB, and tested. But nothing
> changed.

int33bd is the hid for MBID device.

I think I know what the problem this.
My patches enables dependency checking, and the dependency for SHC1 is resolved.
But, at the same time, dependency checking is enabled for the other 80860F14 devices as well, which has unmet dependency.
PEPD (INT33A4) and PMIC (INT33FD) is referenced in _DEP for other 80860F14 devices.

to make a quick check, can you please modify the patch I attached a little,
in drivers/acpi/scan.c, ignore INT33A4 and INT33FD as well as INT33BD?

if it works, please check if ignoring INT33A4 and INT33BD is sufficient.

Thanks for explaining!

Ignoring INT33A4 and INT33BD is sufficient. The eMMC is present when needed,
the sd-card reader and wifi (over sdio) works once pinctrl-cherryview
is modprobed. Card detect works fine.

hmmm, can you please try some kernel later than 4.9-rc3?
The below commit seems to fix the order issue, in a different way, please verify if it works for you or not, when CONFIG_PINCTRL_CHERRYVIEW=m.

commit 67bf5156edc4f58241fd7c119ae145c552adddd6
Author: David Arcari <darcari@redhat.com>
Date:   Wed Oct 12 18:40:30 2016 +0200

    gpio / ACPI: fix returned error from acpi_dev_gpio_irq_get()
    
    acpi_dev_gpio_irq_get() currently ignores the error returned
    by acpi_get_gpiod_by_index() and overwrites it with -ENOENT.
    
    Problem is this error can be -EPROBE_DEFER, which just blows
    up some drivers when the module ordering is not correct.
    
    Cc: stable@vger.kernel.org
    Signed-off-by: David Arcari <darcari@redhat.com>
    Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
    Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Sorry for that I've not mentioned, but I've been testing on 4.9-rc7.

But I'll retest on rc8 without any of you patches, but with PINCTRL_CHERRYVIEW=m

Retested with vanilla 4.9-rc8 + PINCTRL_CHERRYVIEW=m + my dsdt and then with yours. Neither did work.

here is the code flow for requiring an ACPI GPIO resource in sdhci-acpi driver
sdhci_acpi_probe->mmc_gpiod_request_cd->devm_gpiod_get_index->gpiod_get_index->acpi_find_gpio->acpi_get_gpiod_by_index->acpi_gpio_resource_lookup->acpi_populate_gpio_lookup->acpi_get_gpiod

acpi_get_gpiod returns -EPROBE_DEFER when the GPIO chip is not registered yet.

But, the problem here is that, in sdhci_acpi_probe(), the return value of mmc_gpiod_request_cd() is ignored. Hard to say it is right or not, but if we change the code to something like
        if (sdhci_acpi_flag(c, SDHCI_ACPI_SD_CD)) {
                bool v = sdhci_acpi_flag(c, SDHCI_ACPI_SD_CD_OVERRIDE_LEVEL);

                err = mmc_gpiod_request_cd(host->mmc, NULL, 0, v, 0, NULL);
                if (err) {
                        if (err = -EPROBE_DEFER)
                                 cleanup and return err;
                        dev_warn(dev, "failed to setup card detect gpio\n");
                        c->use_runtime_pm = false;
                }
        }
then nothing needs to be changed in ACPI code, the dependency problem is resolved even if PINCTRL_CHERRYVIEW=m.

Anyway, I'm not sure this is the right behaviour and what cleanup needs to be done before return, let me raise this problem to Adrian Hunter, the sdhci-acpi expert.

Created attachment 247061 [details]
patch to support deferred probe if GPIO not ready in sdhci-acpi driver

please try this patch on top of 4.9-rc8 kernel, with your dsdt file, without any other patch and see if the problem is gone.

Nope, that patch on 4.9-rc8 with my dtds didn't work :-(

Might be completely unrelated, but it is in the same area. I filed a bug on that on this computer you have to do changes to the dsdt in order to get the sd card reader and wifi (over sdio) to work at all:

https://bugzilla.kernel.org/show_bug.cgi?id=189801

Maybe that DSDT workaround/fix has something to do with this issue.

(In reply to Jonas Aaberg from comment #0)
> # dmesg |grep sdh
> [    1.293450] sdhci: Secure Digital Host Controller Interface driver
> [    1.295946] sdhci: Copyright(c) Pierre Ossman
> [    2.370378] sdhci-acpi 80860F14:03: GPIO lookup for consumer 80860F14:03
> cd
> [    2.370390] sdhci-acpi 80860F14:03: using ACPI for GPIO lookup
> [    2.371059] sdhci-acpi 80860F14:03: lookup for GPIO 80860F14:03 cd failed
> [    2.371070] sdhci-acpi 80860F14:03: failed to setup card detect gpio

with the patch in comment #18, the symptom is exactly the same as comment #0, except we lose this line "sdhci-acpi 80860F14:03: failed to setup card detect gpio", right?

On the contrary, with your patch on rc8 I get loads of "failed to setup cards detect gpio:"

dmesg |grep sdh

[    2.096840] sdhci: Secure Digital Host Controller Interface driver
[    2.099692] sdhci: Copyright(c) Pierre Ossman
[    3.194645] sdhci-acpi 80860F14:03: GPIO lookup for consumer 80860F14:03 cd
[    3.194657] sdhci-acpi 80860F14:03: using ACPI for GPIO lookup
[    3.195303] sdhci-acpi 80860F14:03: lookup for GPIO 80860F14:03 cd failed
[    3.250903] sdhci-acpi 80860F14:03: GPIO lookup for consumer 80860F14:03 cd
[    3.250908] sdhci-acpi 80860F14:03: using ACPI for GPIO lookup
[    3.251131] sdhci-acpi 80860F14:03: lookup for GPIO 80860F14:03 cd failed
[    3.268194] sdhci-acpi 80860F14:03: GPIO lookup for consumer 80860F14:03 cd
[    3.268202] sdhci-acpi 80860F14:03: using ACPI for GPIO lookup
[    3.268795] sdhci-acpi 80860F14:03: lookup for GPIO 80860F14:03 cd failed
[    3.791110] sdhci-acpi 80860F14:03: GPIO lookup for consumer 80860F14:03 cd
[    3.791115] sdhci-acpi 80860F14:03: using ACPI for GPIO lookup
[    3.791413] sdhci-acpi 80860F14:03: lookup for GPIO 80860F14:03 cd failed
[    4.203745] sdhci-acpi 80860F14:03: GPIO lookup for consumer 80860F14:03 cd
[    4.203785] sdhci-acpi 80860F14:03: using ACPI for GPIO lookup
[    4.203952] sdhci-acpi 80860F14:03: lookup for GPIO 80860F14:03 cd failed
[    4.414246] sdhci-acpi 80860F14:03: GPIO lookup for consumer 80860F14:03 cd
[    4.414250] sdhci-acpi 80860F14:03: using ACPI for GPIO lookup
[    4.414401] sdhci-acpi 80860F14:03: lookup for GPIO 80860F14:03 cd failed
[    4.433935] sdhci-acpi 80860F14:03: GPIO lookup for consumer 80860F14:03 cd
[    4.433938] sdhci-acpi 80860F14:03: using ACPI for GPIO lookup
[    5.930499] sdhci: Timeout waiting for Buffer Read Ready interrupt during tuning procedure, falling back to fixed sampling clock

Otherwise the behavior is as in comment #0

(In reply to Jonas Aaberg from comment #21)
> On the contrary, with your patch on rc8 I get loads of "failed to setup
> cards detect gpio:"
> 
> dmesg |grep sdh
> 
> [    2.096840] sdhci: Secure Digital Host Controller Interface driver
> [    2.099692] sdhci: Copyright(c) Pierre Ossman
> [    3.194645] sdhci-acpi 80860F14:03: GPIO lookup for consumer 80860F14:03
> cd
> [    3.194657] sdhci-acpi 80860F14:03: using ACPI for GPIO lookup
> [    3.195303] sdhci-acpi 80860F14:03: lookup for GPIO 80860F14:03 cd failed

sdhci-acpi driver probe is deferred here.

> [    3.250903] sdhci-acpi 80860F14:03: GPIO lookup for consumer 80860F14:03
> cd
> [    3.250908] sdhci-acpi 80860F14:03: using ACPI for GPIO lookup
> [    3.251131] sdhci-acpi 80860F14:03: lookup for GPIO 80860F14:03 cd failed

sdhci-acpi driver is rescheduled and re-deferred here.
and same below.

Looks like the patch works, sdhci-acpi driver is deferred because GPIO is not ready. But then the problem is that, why the pinctrl-cherrytrail driver is not loaded in the end? It's built as module, and it should be loaded automatically by udev.

can you please reboot with boot option "initcall_debug" and then attach the full dmesg output after boot?
please attach the output of "ls -l /sys/bus/platform/devices/INT33FF*/", and "cat /sys/bus/platform/devices/INT33FF*/*" after boot.

When I added initcall_debug, I noticed that 4.9rc8+your patch from comment #18
+ my dsdt did work! I rebuilt to make sure I test on the right build and tested again without initcall debug, it does still work!

A difference from the working case, is that during the none working case I got this print:

--[    5.930499] sdhci: Timeout waiting for Buffer Read Ready interrupt during tuning procedure, falling back to fixed sampling clock
--
Maybe that did stop the card detect from working? unfortunately, I don't have the kernel log left from the none-working cases.

I will try to reproduce the none-working case again.

(In reply to Jonas Aaberg from comment #24)
> When I added initcall_debug, I noticed that 4.9rc8+your patch from comment
> #18
> + my dsdt did work! I rebuilt to make sure I test on the right build and
> tested again without initcall debug, it does still work!
> 
Good news to me. :)

> A difference from the working case, is that during the none working case I
> got this print:
> 
> --[    5.930499] sdhci: Timeout waiting for Buffer Read Ready interrupt
> during tuning procedure, falling back to fixed sampling clock
> --
> Maybe that did stop the card detect from working? unfortunately, I don't
> have the kernel log left from the none-working cases.
> 
> I will try to reproduce the none-working case again.

yes, please. And if it can be reproduced, it indicates the patch in comment #18 is not sufficient, maybe we need some help from the sdhci experts.

ping...
if you can not reproduce the problem any more after applying the patch in comment #18, I will send this patch out and close this bug report.

Sorry for my silence. No, I've not been able to reproduce the sdhci time out bug.
However, in any case it should be reported as a new bug, since it is probably unrelated.

Please go ahead and close this bug. Patch in comment #18 is a working fix. Please add me as tested by, if you want to.

Thanks!

patch sent at https://patchwork.kernel.org/patch/9488397/