Page MenuHomeFreeBSD

sdhci_fsl_fdt: Add support for HS200/HS400 modes
ClosedPublic

Authored by hum_semihalf.com on Dec 7 2021, 12:21 PM.
Tags
None
Referenced Files
F102820339: D33320.id.diff
Sun, Nov 17, 2:56 PM
Unknown Object (File)
Sun, Nov 17, 12:46 AM
Unknown Object (File)
Fri, Nov 8, 9:16 AM
Unknown Object (File)
Oct 17 2024, 7:15 AM
Unknown Object (File)
Oct 7 2024, 7:37 PM
Unknown Object (File)
Oct 4 2024, 6:13 PM
Unknown Object (File)
Oct 4 2024, 4:12 PM
Unknown Object (File)
Oct 2 2024, 2:05 AM

Details

Summary

The controller requires some custom logic to perform MMC tuning
and to later switch to HS400 mode. Implement it supplying mmcbr_tune
and sdhci_set_uhs_timing devmethods respectivly. Since the latter
is called unconditionally when the ios is updated we need to keep
track of the tuning state in sc and execute the HS400 switch logic
only when required.

Two HS200/HS400 related errata were implemented.

  1. In HS400 modes the clock divisors are limited to 4, 8, 12. Apply it by falling back to the closes, higher divider when needed.
  2. Hardware tuning procedure can sometimes fails. If that is the case fallback to the software tuning.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I haven't given this patch a thorough look yet, but here are some details that I spotted at a first glance.

sys/dev/sdhci/sdhci_fsl_fdt.c
1091

There's a lot of those repeating do-while loops in the code where you are polling on a particular register bit change. Why don't you create a helper function instead? And shouldn't it be while-do instead? E.g. you shouldn't incur delay penalty in case the bit in question is already off to begin with.

1130

Doesn't seem like it's ever used.

1362

No DELAY(10) here?

1482

Again, unused.

sys/dev/sdhci/sdhci_fsl_fdt.c
153

s/bo/be

sys/dev/sdhci/sdhci_fsl_fdt.c
507

nit: s/successfull/successful

sys/dev/sdhci/sdhci_fsl_fdt.c
1126

Since this is the same type as above, why not list it at the same line as the remaining declarations?
Same goes for similar lines in the rest of this patch.

1157

These two lines above can be concatenated into a single expression, as there is no need to be so explicit about these flags:
reg |= SDHCI_FSL_AUTOCERR_EXTN | SDHCI_FSL_AUTOCERR_SMPCLKSEL;

1252

Again, no need to be explicit about these flags, we can just do:

	reg &= ~SDHCI_FSL_TBCTL_TB_MODE_MASK;
	reg |= SDHCI_FSL_TBCTL_TBEN | SDHCI_FSL_TBCTL_MODE_3;
1313

For else if (hs400) path, error will always evaluate to 0, so let's rewrite that section a bit:

	if (error != 0) {
		sdhci_fsl_switch_tuning_block(bus, false);
		return (error);
	}

	if (hs400) {
		...
	}

	return (0);

Update commit log. Add helper function for polling registers. Fix typos and remove redundant variables declaration.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 8 2021, 10:23 AM
This revision was automatically updated to reflect the committed changes.

This breaks mmc on FDT based Honeycomb with following log:

mmc0: Probing bus
sdhci_fsl_fdt0-slot0: Divider 896 for freq 390625 (base 700000000)
sdhci_fsl_fdt0-slot0: Divider 896 for freq 390625 (base 700000000)
mmc0: SD 2.0 interface conditions: OK
mmc0: SD probe: OK (OCR: 0x00ff8000)
sdhci_fsl_fdt0-slot0: Divider 896 for freq 390625 (base 700000000)
sdhci_fsl_fdt0-slot0: Divider 896 for freq 390625 (base 700000000)
mmc0: Current OCR: 0x00ff8000
mmc0: Probing cards
mmc0: New card detected (CID 4134325344313647303b700758012200)
mmc0: New card detected (CSD 400e00325b59000073677f800a400000)
mmc0: Card at relative address 0x0007 added:
mmc0:  card: SDHC SD16G 3.0 SN 3B700758 MFG 02/2018 by 65 42
mmc0:  quirks: 0
mmc0:  bus: 4bit, 50MHz (high speed timing)
mmc0:  memory: 30253056 blocks, erase sector 8192 blocks
sdhci_fsl_fdt0-slot0: Divider 896 for freq 390625 (base 700000000)
mmc0: setting transfer rate to 50.000MHz (high speed timing)
sdhci_fsl_fdt0-slot0: Divider 896 for freq 390625 (base 700000000)
sdhci_fsl_fdt0-slot0: Divider 7 for freq 50000000 (base 700000000)
sdhci_fsl_fdt0-slot0: Divider 7 for freq 50000000 (base 700000000)
sdhci_fsl_fdt0: Timeout while waiting for hardware to finish tuning.
sdhci_fsl_fdt0: Timeout while waiting for hardware to finish tuning.
sdhci_fsl_fdt0: Timeout while waiting for hardware to finish tuning.
sdhci_fsl_fdt0: Timeout while waiting for hardware to finish tuning.
sdhci_fsl_fdt0: Timeout while waiting for hardware to finish tuning.
sdhci_fsl_fdt0: Timeout while waiting for hardware to finish tuning.
sdhci_fsl_fdt0: Timeout while waiting for hardware to finish tuning.
sdhci_fsl_fdt0: Timeout while waiting for hardware to finish tuning.
sdhci_fsl_fdt0: Timeout while waiting for hardware to finish tuning.
sdhci_fsl_fdt0: Timeout while waiting for hardware to finish tuning.
sdhci_fsl_fdt0: Timeout while waiting for hardware to finish tuning.
sdhci_fsl_fdt0: Failed to execute HW tuning.
sdhci_fsl_fdt0: Hardware tuning failed. Turning off tuning block
mmc0: Card at relative address 7 failed to execute initial tuning
...
sdhci_fsl_fdt0-slot0: Controller timeout
sdhci_fsl_fdt0-slot0: ============== REGISTER DUMP ==============
sdhci_fsl_fdt0-slot0: Sys addr: 0x82900040 | Version:  0x00002202
sdhci_fsl_fdt0-slot0: Blk size: 0x00000040 | Blk cnt:  0x00000001
sdhci_fsl_fdt0-slot0: Argument: 0x00000000 | Trn mode: 0x00000000
sdhci_fsl_fdt0-slot0: Present:  0x01fd0000 | Host ctl: 0x00000001
sdhci_fsl_fdt0-slot0: Power:    0x0000000d | Blk gap:  0x00000000
sdhci_fsl_fdt0-slot0: Wake-up:  0x00000000 | Clock:    0x00000707
sdhci_fsl_fdt0-slot0: Timeout:  0x0000000e | Int stat: 0x00000000
sdhci_fsl_fdt0-slot0: Int enab: 0x017f00f3 | Sig enab: 0x017f00f3
sdhci_fsl_fdt0-slot0: AC12 err: 0x00000000 | Host ctl2:0x00000046
sdhci_fsl_fdt0-slot0: Caps:     0x35f20000 | Caps2:    0x0000af07
sdhci_fsl_fdt0-slot0: Max curr: 0x00000000 | ADMA err: 0x00000000
sdhci_fsl_fdt0-slot0: ADMA addr:0x00000000 | Slot int: 0x00000000
sdhci_fsl_fdt0-slot0: ===========================================
....

while log with this reverted looks like:

sdhci_fsl_fdt0: Powering up sd/mmc
sdhci_fsl_fdt0-slot0: Divider 896 for freq 390625 (base 700000000)
mmc0: Probing bus
mmc0: SD 2.0 interface conditions: OK
mmc0: SD probe: OK (OCR: 0x00ff8000)
mmc0: Current OCR: 0x00ff8000
mmc0: Probing cards
mmc0: New card detected (CID 4134325344313647303b700758012200)
mmc0: New card detected (CSD 400e00325b59000073677f800a400000)
mmc0: Card at relative address 0x0007 added:
mmc0:  card: SDHC SD16G 3.0 SN 3B700758 MFG 02/2018 by 65 42
mmc0:  quirks: 0
mmc0:  bus: 4bit, 50MHz (high speed timing)
mmc0:  memory: 30253056 blocks, erase sector 8192 blocks
mmc0: setting transfer rate to 50.000MHz (high speed timing)
sdhci_fsl_fdt0-slot0: Divider 7 for freq 50000000 (base 700000000)
mmcsd0: 15GB <SDHC SD16G 3.0 SN 3B700758 MFG 02/2018 by 65 42> at mmc0 50.0MHz/4bit/1024-block
mmc0: setting bus width to 4 bits high speed timing

Apparently LX2160A requires window pointer errata enabled for tuning to succeed. (SDHCI_FSL_TUNING_ERRATUM_TYPE2)
Unfortunately on this SoC we have a generic compat string for SDHCI driver, so there is no easy way to detect and apply it.
For now, I will prepare a patch that disables HS200/HS400 modes for all boards that use the generic compat.

ofw_bus_node_is_compatible(OF_finddevice("/"), "fsl,lx2160a")

does the trick (assuming that OF_finddevice("/") cannot fail)...