Hello Chevron,
Ignoring the errors at the probe stage will get the driver working as
well as it did before kernel version 5.12, however, it is still incorrect.
Because, if the read errors are ignored, the values returned in the
"scratch_32" variable will be invalid.
Those invalid values are then used to compute a mask that written back
to the chip's O2_SD_PLL_SETTING register.
Would it be possible to power the chip at the start of the probe call,
so its registers could be properly read?
Alternatively, could one delay the configuration of these registers
until after the chip had been properly powered?
(Could one turn on the power in probe, delaying the rest of the
initialization until after power had stabilized?)
Note that even when this driver functioned, it required
debug_quirks2=4
to disable ultra high-speed support.
Also, note that the PCI_DEVICE_ID_O2_SDS0, SDS1 and FUJIN2
cases of the probe function in version 5.10 ignored the results form reading O2_SD_FUNC_REG0 and O2_SD_FUNC_REG4.
I don't have those chip types to test, but I must assume the results, in these cases, were ignored for similar reasons.
- brent
Chevron Li (WH) wrote:
Hi, Ulf/Adrian,
Sorry for response late.
After checked some documents internal, we think it's necessary to regression "mmc: sdhci-pci-o2micro: Add missing checks in sdhci_pci_o2_probe" in sdhci-pci-o2micro.c .
Below is the reason:
Some chip of this hardware(O2_SEABIRD0/O2_SEABIRD1) main power may be not ready when access it at driver probe stage.
So, the pci_read_config_dword() calls errors should be ignored at probe stage.
BR,
Chevron
-----Original Message-----
From: Chevron Li (WH)
Sent: Thursday, January 18, 2024 16:53
To: 'Ulf Hansson' <ulf.hansson@xxxxxxxxxx>; 'Brent Roman' <brent@xxxxxxxxx>; Fred Ai(WH) <fred.ai@xxxxxxxxxxxxxx>
Cc: 'linux-mmc@xxxxxxxxxxxxxxx' <linux-mmc@xxxxxxxxxxxxxxx>; 'Adrian Hunter' <adrian.hunter@xxxxxxxxx>
Subject: RE: Regression in sdhci-pci-o2micro.c
Hi, Ulf/Adrian,
Please ignore former email.
I confused it with another issue.
I agree Regression "mmc: sdhci-pci-o2micro: Add missing checks in sdhci_pci_o2_probe" in sdhci-pci-o2micro.c
For the issue Brent reported, we will check it internal first.
BR,
Chevron
-----Original Message-----
From: Chevron Li (WH)
Sent: Thursday, January 18, 2024 11:53
To: Ulf Hansson <ulf.hansson@xxxxxxxxxx>; Brent Roman <brent@xxxxxxxxx>; Fred Ai(WH) <fred.ai@xxxxxxxxxxxxxx>
Cc: linux-mmc@xxxxxxxxxxxxxxx; Adrian Hunter <adrian.hunter@xxxxxxxxx>
Subject: RE: Regression in sdhci-pci-o2micro.c
Hi, Ulf/Adrian,
We tried to implement the "remove_slot" at "sdhci_pci_fixes" and recover some changed PCR configure value for next reboot at BIOS stage.
But I'm not sure that the added patch in "remove_slot" will be called when OS reboot.
If there are callback for o2micro host driver when OS reboot.
Attachment is a sample code which implemented the "remove_slot" at "sdhci_pci_fixes".
Any suggestions will be appreciated.
BR,
Chevron
-----Original Message-----
From: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
Sent: Wednesday, January 17, 2024 20:18
To: Brent Roman <brent@xxxxxxxxx>; Chevron Li (WH) <chevron.li@xxxxxxxxxxxxxx>; Fred Ai(WH) <fred.ai@xxxxxxxxxxxxxx>
Cc: linux-mmc@xxxxxxxxxxxxxxx; Adrian Hunter <adrian.hunter@xxxxxxxxx>
Subject: Re: Regression in sdhci-pci-o2micro.c
+ Fred, Chevron Li, Adrian
On Tue, 16 Jan 2024 at 23:18, Brent Roman <brent@xxxxxxxxx> wrote:
Hi,
I have an Intel Hades Canyon NUC (NUC8i7HVK) whose O2 Micro based
SD-Card reader is no longer identified when its Linux kernel is
updated past 5.12
I "fixed" this by reverting a change from 5/9/21 (git
efc58a96adcd29cc37487a60582d9d08b34f6640)
that inserted proper error checking after all the PCI config space
reads in the device probe.
This would be code removed enclosed in #if 0 below:
case PCI_DEVICE_ID_O2_SEABIRD0:
case PCI_DEVICE_ID_O2_SEABIRD1:
/* UnLock WP */
ret = pci_read_config_byte(chip->pdev,
O2_SD_LOCK_WP, &scratch);
if (ret)
return ret;
scratch &= 0x7f;
pci_write_config_byte(chip->pdev, O2_SD_LOCK_WP, scratch);
ret = pci_read_config_dword(chip->pdev,
O2_SD_PLL_SETTING, &scratch_32); #if 0
if (ret)
return ret;
#endif
if ((scratch_32 & 0xff000000) == 0x01000000) {
scratch_32 &= 0x0000FFFF;
scratch_32 |= 0x1F340000;
pci_write_config_dword(chip->pdev,
O2_SD_PLL_SETTING, scratch_32);
} else {
scratch_32 &= 0x0000FFFF;
scratch_32 |= 0x25100000;
pci_write_config_dword(chip->pdev,
O2_SD_PLL_SETTING, scratch_32);
ret = pci_read_config_dword(chip->pdev,
O2_SD_FUNC_REG4,
&scratch_32); #if 0
if (ret)
return ret;
#endif
scratch_32 |= (1 << 22);
pci_write_config_dword(chip->pdev,
O2_SD_FUNC_REG4, scratch_32);
}
Both those pci_read_config_dword() calls return -EINVAL on my machine.
The driver had been working earlier precisely because it was ignoring
these error returns from pci_read_config_dword.
Have you seen this behavior before on any other hardware?
The SDcard reader identifies as:
03:00.0 SD Host controller: O2 Micro, Inc. SD/MMC Card Reader
Controller (rev 01)
03:00.0 0805: 1217:8621 (rev 01)
I've been unable to find /any/ information on this chip.
Do you have any you could share? A datasheet would be ideal :-)
Also:
I've always had to operate this driver with debug_quirks2=4 to disable
ultra high-speed support.
Is this a known issue?
Or, could it be a symptom of the failing pci_read_configs curing probe?
Thanks for your attention,
Looks like the offending commit efc58a96adcd ("mmc: sdhci-pci-o2micro:
Add missing checks in sdhci_pci_o2_probe"), may not have been thoroughly tested. Perhaps a revert is needed.
Let's see if Fred/Chevron Li has some thoughts around this.
Kind regards
Uffe
--
Brent Roman MBARI
Software Engineer Tel: (831) 775-1808
mailto:brent@xxxxxxxxx http://www.mbari.org/~brent
