Hello,
This series exposes some devicetree properties for tuning phase delay in the
Aspeed SD/eMMC controllers. The relevant register was introduced on the AST2600
and is present for both the SD/MMC controller and the dedicated eMMC
controller.
v2 addresses comments from Joel.
v1 can be found here:
https://lore.kernel.org/linux-arm-kernel/[email protected]/T/
Please review!
Cheers,
Andrew
Andrew Jeffery (3):
dt: bindings: mmc: Add phase control properties for the Aspeed SDHCI
mmc: sdhci-of-aspeed: Expose data sample phase delay tuning
ARM: dts: tacoma: Add data sample phase delay for eMMC
.../devicetree/bindings/mmc/aspeed,sdhci.yaml | 8 ++
arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts | 2 +
drivers/mmc/host/sdhci-of-aspeed.c | 126 +++++++++++++++++-
3 files changed, 131 insertions(+), 5 deletions(-)
--
2.25.1
Adjust the phase delay to avoid data timeout splats like the following:
[ 731.368601] mmc0: Timeout waiting for hardware interrupt.
[ 731.374644] mmc0: sdhci: ============ SDHCI REGISTER DUMP ===========
[ 731.381828] mmc0: sdhci: Sys addr: 0x00000020 | Version: 0x00000002
[ 731.389012] mmc0: sdhci: Blk size: 0x00007200 | Blk cnt: 0x00000020
[ 731.396194] mmc0: sdhci: Argument: 0x00462a18 | Trn mode: 0x0000002b
[ 731.403377] mmc0: sdhci: Present: 0x01f70106 | Host ctl: 0x00000017
[ 731.410559] mmc0: sdhci: Power: 0x0000000f | Blk gap: 0x00000000
[ 731.417733] mmc0: sdhci: Wake-up: 0x00000000 | Clock: 0x00000107
[ 731.424915] mmc0: sdhci: Timeout: 0x0000000e | Int stat: 0x00000000
[ 731.432098] mmc0: sdhci: Int enab: 0x03ff008b | Sig enab: 0x03ff008b
[ 731.439282] mmc0: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
[ 731.446464] mmc0: sdhci: Caps: 0x01f80080 | Caps_1: 0x00000007
[ 731.453647] mmc0: sdhci: Cmd: 0x0000193a | Max curr: 0x001f0f08
[ 731.460829] mmc0: sdhci: Resp[0]: 0x00000900 | Resp[1]: 0xffffffff
[ 731.468013] mmc0: sdhci: Resp[2]: 0x320f5913 | Resp[3]: 0x00000900
[ 731.475195] mmc0: sdhci: Host ctl2: 0x0000008b
[ 731.480139] mmc0: sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0xbe040200
[ 731.487321] mmc0: sdhci: ============================================
Signed-off-by: Andrew Jeffery <[email protected]>
Acked-by: Joel Stanley <[email protected]>
---
arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts b/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
index 5f4ee67ac787..94ec301ceb73 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
@@ -179,6 +179,8 @@ &emmc_controller {
&emmc {
status = "okay";
+ aspeed,input-phase = <0x7>;
+ aspeed,output-phase = <0x1f>;
};
&fsim0 {
--
2.25.1
The Aspeed SD/eMMC controllers feature up to two SDHCIs alongside a
a set of "global" configuration registers. The global configuration
registers house controller-specific settings that aren't exposed by the
SDHCI, one example being a register for phase tuning.
The phase tuning feature is new in the AST2600 design. It's exposed as a
single register in the global register set and controls both the input
and output phase adjustment for each slot. As the settings are
slot-specific, the values to program are extracted from properties in
the SDHCI devicetree nodes.
Signed-off-by: Andrew Jeffery <[email protected]>
---
In v2:
* Rework devicetree parsing to minimise state disruption
* Switch some log statements from dev_info() to dev_dbg()
---
drivers/mmc/host/sdhci-of-aspeed.c | 126 +++++++++++++++++++++++++++--
1 file changed, 121 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index 4f008ba3280e..c61eb42e1ebb 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -16,9 +16,19 @@
#include "sdhci-pltfm.h"
-#define ASPEED_SDC_INFO 0x00
-#define ASPEED_SDC_S1MMC8 BIT(25)
-#define ASPEED_SDC_S0MMC8 BIT(24)
+#define ASPEED_SDC_INFO 0x00
+#define ASPEED_SDC_S1_MMC8 BIT(25)
+#define ASPEED_SDC_S0_MMC8 BIT(24)
+#define ASPEED_SDC_PHASE 0xf4
+#define ASPEED_SDC_S1_PHASE_IN GENMASK(25, 21)
+#define ASPEED_SDC_S0_PHASE_IN GENMASK(20, 16)
+#define ASPEED_SDC_S1_PHASE_OUT GENMASK(15, 11)
+#define ASPEED_SDC_S1_PHASE_IN_EN BIT(10)
+#define ASPEED_SDC_S1_PHASE_OUT_EN GENMASK(9, 8)
+#define ASPEED_SDC_S0_PHASE_OUT GENMASK(7, 3)
+#define ASPEED_SDC_S0_PHASE_IN_EN BIT(2)
+#define ASPEED_SDC_S0_PHASE_OUT_EN GENMASK(1, 0)
+#define ASPEED_SDC_PHASE_MAX 31
struct aspeed_sdc {
struct clk *clk;
@@ -28,9 +38,21 @@ struct aspeed_sdc {
void __iomem *regs;
};
+struct aspeed_sdhci_phase_desc {
+ u32 value_mask;
+ u32 enable_mask;
+ u8 enable_value;
+};
+
+struct aspeed_sdhci_phase {
+ struct aspeed_sdhci_phase_desc in;
+ struct aspeed_sdhci_phase_desc out;
+};
+
struct aspeed_sdhci {
struct aspeed_sdc *parent;
u32 width_mask;
+ const struct aspeed_sdhci_phase *phase;
};
static void aspeed_sdc_configure_8bit_mode(struct aspeed_sdc *sdc,
@@ -50,6 +72,22 @@ static void aspeed_sdc_configure_8bit_mode(struct aspeed_sdc *sdc,
spin_unlock(&sdc->lock);
}
+static void
+aspeed_sdc_configure_phase(struct aspeed_sdc *sdc,
+ const struct aspeed_sdhci_phase_desc *phase,
+ uint8_t value)
+{
+ u32 reg;
+
+ spin_lock(&sdc->lock);
+ reg = readl(sdc->regs + ASPEED_SDC_PHASE);
+ reg &= ~(phase->enable_mask | phase->value_mask);
+ reg |= value << __ffs(phase->value_mask);
+ reg |= phase->enable_value << __ffs(phase->enable_mask);
+ writel(reg, sdc->regs + ASPEED_SDC_PHASE);
+ spin_unlock(&sdc->lock);
+}
+
static void aspeed_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
{
struct sdhci_pltfm_host *pltfm_host;
@@ -155,8 +193,49 @@ static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev,
return (delta / 0x100) - 1;
}
+static int aspeed_sdhci_configure_of(struct platform_device *pdev,
+ struct aspeed_sdhci *sdhci)
+{
+ struct device_node *np;
+ struct device *dev;
+ u32 phase;
+
+ if (!sdhci->phase)
+ return 0;
+
+ dev = &pdev->dev;
+ np = dev->of_node;
+
+ if (!of_property_read_u32(np, "aspeed,input-phase", &phase)) {
+ if (phase <= ASPEED_SDC_PHASE_MAX) {
+ aspeed_sdc_configure_phase(sdhci->parent,
+ &sdhci->phase->in,
+ phase);
+ dev_dbg(dev, "Input phase adjustment: %u", phase);
+ } else {
+ dev_err(dev, "Invalid input phase value: %u", phase);
+ return -EINVAL;
+ }
+ }
+
+ if (!of_property_read_u32(np, "aspeed,output-phase", &phase)) {
+ if (phase <= ASPEED_SDC_PHASE_MAX) {
+ aspeed_sdc_configure_phase(sdhci->parent,
+ &sdhci->phase->out,
+ phase);
+ dev_dbg(dev, "Output phase adjustment: %u", phase);
+ } else {
+ dev_err(dev, "Invalid output phase value: %u", phase);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
static int aspeed_sdhci_probe(struct platform_device *pdev)
{
+ const struct aspeed_sdhci_phase *phase;
struct sdhci_pltfm_host *pltfm_host;
struct aspeed_sdhci *dev;
struct sdhci_host *host;
@@ -181,7 +260,10 @@ static int aspeed_sdhci_probe(struct platform_device *pdev)
return -EINVAL;
dev_info(&pdev->dev, "Configuring for slot %d\n", slot);
- dev->width_mask = !slot ? ASPEED_SDC_S0MMC8 : ASPEED_SDC_S1MMC8;
+ dev->width_mask = !slot ? ASPEED_SDC_S0_MMC8 : ASPEED_SDC_S1_MMC8;
+ phase = of_device_get_match_data(&pdev->dev);
+ if (phase)
+ dev->phase = &phase[slot];
sdhci_get_of_property(pdev);
@@ -195,6 +277,10 @@ static int aspeed_sdhci_probe(struct platform_device *pdev)
goto err_pltfm_free;
}
+ ret = aspeed_sdhci_configure_of(pdev, dev);
+ if (ret)
+ goto err_sdhci_add;
+
ret = mmc_of_parse(host->mmc);
if (ret)
goto err_sdhci_add;
@@ -230,10 +316,40 @@ static int aspeed_sdhci_remove(struct platform_device *pdev)
return 0;
}
+static const struct aspeed_sdhci_phase ast2600_sdhci_phase[] = {
+ /* SDHCI/Slot 0 */
+ [0] = {
+ .in = {
+ .value_mask = ASPEED_SDC_S0_PHASE_IN,
+ .enable_mask = ASPEED_SDC_S0_PHASE_IN_EN,
+ .enable_value = 1,
+ },
+ .out = {
+ .value_mask = ASPEED_SDC_S0_PHASE_OUT,
+ .enable_mask = ASPEED_SDC_S0_PHASE_OUT_EN,
+ .enable_value = 3,
+ },
+ },
+ /* SDHCI/Slot 1 */
+ [1] = {
+ .in = {
+ .value_mask = ASPEED_SDC_S1_PHASE_IN,
+ .enable_mask = ASPEED_SDC_S1_PHASE_IN_EN,
+ .enable_value = 1,
+ },
+ .out = {
+ .value_mask = ASPEED_SDC_S1_PHASE_OUT,
+ .enable_mask = ASPEED_SDC_S1_PHASE_OUT_EN,
+ .enable_value = 3,
+ },
+ },
+};
+
+/* If supported, phase adjustment fields are stored in the data pointer */
static const struct of_device_id aspeed_sdhci_of_match[] = {
{ .compatible = "aspeed,ast2400-sdhci", },
{ .compatible = "aspeed,ast2500-sdhci", },
- { .compatible = "aspeed,ast2600-sdhci", },
+ { .compatible = "aspeed,ast2600-sdhci", .data = ast2600_sdhci_phase },
{ }
};
--
2.25.1
Add properties to control the phase delay for input and output data.
Signed-off-by: Andrew Jeffery <[email protected]>
---
Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
index 987b287f3bff..d7b605968c8a 100644
--- a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
@@ -61,6 +61,14 @@ patternProperties:
sdhci,auto-cmd12:
type: boolean
description: Specifies that controller should use auto CMD12
+ "aspeed,input-phase":
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description:
+ The input phase delay value.
+ "aspeed,output-phase":
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description:
+ The output phase delay value.
required:
- compatible
- reg
--
2.25.1