Page MenuHomeFreeBSD

sdhci_fsl_fdt: Provide more accurate clk calculation
ClosedPublic

Authored by ar_semihalf.com on Oct 28 2021, 1:37 PM.

Details

Summary

SDHCI controllers found in the QorIQ SoCs offer improved accuracy of
the clock frequency selection, compared to the SDHCI standard. Frequency
selection is performed using two divider registers, named prescaler and
divisor, according to the following formula:
frequency = base clock / (prescaler * divisor), where prescaler can be
bypassed (set to 1) and divisor permitted to take odd values.

Rather than depend on clock division precalculated by sdhci core, make
use of this property of the divider registers and achieve frequencies
closer to the ones requested.

Submitted by: Artur Rojek <ar@semihalf.com>
Obtained from: Semihalf
Sponsored by: Alstom Group

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision was not accepted when it landed; it landed in state Needs Review.Nov 5 2021, 9:21 AM
This revision was automatically updated to reflect the committed changes.

Unfortunately this does not work at all and should be reverted.
eSDHC implements two versions of the clock divider, which are selected by the ESDHCCTL[CRS] bit.

A zero (default) selects the SDHCI v2.0 compatible divider with an additional 4-bit divider. Note that the SDCLKFS field does not contain a *divider value*; only one one is allowed in this field, whose position selects one of the predefined dividers.

The one in the CRS field selects the standard 10-bit SDHCI v3.0 divider.

IMHO, the whole problem is that we are not emulating the SHDCI capability register correctly. For optimal performance we should set ESDHCCTL[CRS] to 1 and signal SDHCI 3.0 in the capability register.

We should also translate SDHCI SDHCI_CLOCK_INT_EN and SDHCI_CLOCK_INT_STABLE when emulating the SDHCI_CLOCK_CONTROL register.

There are still some conditions under which this patch is valid and necessary. On ls1028a the base clock is 600MHz, therefore to obtain 200MHz frequency for HS200 we need divider 3. As SDHCI standard does not support odd dividers, this patch allows to use them. With this, we can use 200MHz instead of 150MHz for HS200 for faster transfers.
As it comes to the SDHCI_CLOCK_CONTROL register, it is captured properly in sdhci_fsl_fdt_read_2 function - we read controller register responsible for clock stability indicator and set flag if matched. I can't see any other invalid usages of aforementioned.

Regarding your note this does not work at all - have you encountered any problems with it causing the driver to fail?`

First of all, I apologize for being too harsh.
The problem is that this patch breaks both SD and eMMC on Honeycomb (LX2160). the eMMC is not detected at all, abd HSSSD card fires out with tons of CRC errors.
Unfortunately however this patch break SD and eMMC on Honezcomb (LX2160).
After further digging, I thing that fsl_sdhc_fdt_set_clock() is broken from day zero.
fsl_sdhc_fdt_set_clock() was supposed to be paired with fsl_sdhc_fdt_get_clock() to perform a conversion of differences between SDHCI and eSDHCl clock register format.

Problem is that original sdhci code still uses sdhci_init_slot() and each write to SDHCI_CLOCK_CONTROL register are directed do fsl_sdhc_fdt_set_clock().

About ls1028a: SDHCI 3.0 defines a "Programmable Clock Mode" which is standard and suits your requirements. Why do you prefer a custom solution for one case over a trivial to implement standard solution with potential benefit for many other cases?

We still need to apply a limited clock division errata. Basically in HS400 mode and that mode only the controller can only use a limited set of divisors. I will try to modify the generic code to support Programmable Clock Mode and test it against ls1028 whether HS400 mode works correctly with or without errata application. Then we could possibly get rid of the problematic sdhci_fsl_fdt_set_clock function, and possibly apply the errata in some other, less invasive way.

The driver doesn't use Programmable Clock Mode because it does not converges to the SDHCI standard here.
In case of ls1028 the generic code (even with the added support) won't switch to PCM considering it unsupported... Obviously, we can switch to PCM on our own, but all that logic must be performed in additional function like fsl_sdhc_fdt_set_clock. IMHO that is not the case to rewrite the function to use PCM.
Divided Clock Mode can be used without any additional helper functions, but then divider 3 is unavailable - we won't get 600/3=200MHz for HS200. DCM is inapplicable.