2021-05-20 19:02:59

by Steven Lee

[permalink] [raw]
Subject: [PATCH v4 0/3] mmc: sdhci-of-aspeed: Support toggling SD bus signal


AST2600-A2 EVB has the reference design for enabling SD bus
power and toggling SD bus signal voltage between 3.3v and 1.8v by
GPIO regulators.
This patch series adds sdhci node and gpio regulators in a new dts file
for AST2600-A2 EVB.
The description of the reference design of AST2600-A2 EVB is added
in the new dts file.

This patch also include a helper for updating AST2600 sdhci capability
registers.

Changes from v3:
* Remove the example of gpio regulator from dt-bindings.
* Add sdhci node and gpio regulators to a new dts file.
* Move the comment of the reference design to the new
dts file.
* Modify commit message of sdhci-of-aspeed.c.
* Fix coding style issues of sdhci-of-aspeed.c.
* Remove the implementation of eMMC resetc since it has no relevance to
the goal that this patch series want to achieve and it may needs further
discussion about the design of reset behavior.

Changes from v2:
* Move the comment of the reference design from dt-bindings to device tree.
* Add clk-phase binding for eMMC controller.
* Reimplement aspeed_sdc_set_slot_capability().
* Separate the implementation of eMMC reset to another patch file.
* Fix yaml document error per the report of dt_binding_check and
dtbs_check.

Changes from v1:
* Add the device tree example for AST2600 A2 EVB in dt-bindings
document
* Add timing-phase for eMMC controller.
* Remove power-gpio and power-switch-gpio from sdhci driver, they should
be handled by regulator.
* Add a helper to update capability registers in the driver.
* Sync sdhci settings from device tree to SoC capability registers.
* Sync timing-phase from device tree to SoC Clock Phase Control
register

Please help to review.

Regards,
Steven

Steven Lee (3):
ARM: dts: aspeed: ast2600evb: Add sdhci node and gpio regulator for A2
evb.
ARM: dts: aspeed: ast2600evb: Add phase correction for emmc
controller.
mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the
devicetree.

arch/arm/boot/dts/aspeed-ast2600-evb-a2.dts | 98 +++++++++++++++++++++
arch/arm/boot/dts/aspeed-ast2600-evb.dts | 3 +-
drivers/mmc/host/sdhci-of-aspeed.c | 48 ++++++++++
3 files changed, 148 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/boot/dts/aspeed-ast2600-evb-a2.dts

--
2.17.1


2021-05-20 19:03:00

by Steven Lee

[permalink] [raw]
Subject: [PATCH v4 3/3] mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the devicetree.

The hardware provides capability configuration registers for each SDHCI
in the global configuration space for the SD controller. Writes to the
global capability registers are mirrored to the capability registers in
the associated SDHCI. Configuration of the capabilities must be written
through the mirror registers prior to initialisation of the SDHCI.

Signed-off-by: Steven Lee <[email protected]>
---
drivers/mmc/host/sdhci-of-aspeed.c | 48 ++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index d001c51074a0..65b5685f6c15 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -31,6 +31,11 @@
#define ASPEED_SDC_S0_PHASE_OUT_EN GENMASK(1, 0)
#define ASPEED_SDC_PHASE_MAX 31

+/* SDIO{10,20} */
+#define ASPEED_SDC_CAP1_1_8V (0 * 32 + 26)
+/* SDIO{14,24} */
+#define ASPEED_SDC_CAP2_SDR104 (1 * 32 + 1)
+
struct aspeed_sdc {
struct clk *clk;
struct resource *res;
@@ -72,6 +77,37 @@ struct aspeed_sdhci {
const struct aspeed_sdhci_phase_desc *phase_desc;
};

+/*
+ * The function sets the mirror register for updating
+ * capbilities of the current slot.
+ *
+ * slot | capability | caps_reg | mirror_reg
+ * -----|-------------|----------|------------
+ * 0 | CAP1_1_8V | SDIO140 | SDIO10
+ * 0 | CAP2_SDR104 | SDIO144 | SDIO14
+ * 1 | CAP1_1_8V | SDIO240 | SDIO20
+ * 1 | CAP2_SDR104 | SDIO244 | SDIO24
+ */
+static void aspeed_sdc_set_slot_capability(struct sdhci_host *host, struct aspeed_sdc *sdc,
+ int capability, bool enable, u8 slot)
+{
+ u32 mirror_reg_offset;
+ u32 cap_val;
+ u8 cap_reg;
+
+ if (slot > 1)
+ return;
+
+ cap_reg = capability / 32;
+ cap_val = sdhci_readl(host, 0x40 + (cap_reg * 4));
+ if (enable)
+ cap_val |= BIT(capability % 32);
+ else
+ cap_val &= ~BIT(capability % 32);
+ mirror_reg_offset = ((slot + 1) * 0x10) + (cap_reg * 4);
+ writel(cap_val, sdc->regs + mirror_reg_offset);
+}
+
static void aspeed_sdc_configure_8bit_mode(struct aspeed_sdc *sdc,
struct aspeed_sdhci *sdhci,
bool bus8)
@@ -328,6 +364,7 @@ static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev,
static int aspeed_sdhci_probe(struct platform_device *pdev)
{
const struct aspeed_sdhci_pdata *aspeed_pdata;
+ struct device_node *np = pdev->dev.of_node;
struct sdhci_pltfm_host *pltfm_host;
struct aspeed_sdhci *dev;
struct sdhci_host *host;
@@ -372,6 +409,17 @@ static int aspeed_sdhci_probe(struct platform_device *pdev)

sdhci_get_of_property(pdev);

+ if (of_property_read_bool(np, "mmc-hs200-1_8v") ||
+ of_property_read_bool(np, "sd-uhs-sdr104")) {
+ aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP1_1_8V,
+ true, slot);
+ }
+
+ if (of_property_read_bool(np, "sd-uhs-sdr104")) {
+ aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP2_SDR104,
+ true, slot);
+ }
+
pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(pltfm_host->clk))
return PTR_ERR(pltfm_host->clk);
--
2.17.1

2021-05-20 19:03:03

by Steven Lee

[permalink] [raw]
Subject: [PATCH v4 2/3] ARM: dts: aspeed: ast2600evb: Add phase correction for emmc controller.

Set MMC timing-phase register by adding the phase correction binding in the
device tree.

Signed-off-by: Steven Lee <[email protected]>
Acked-by: Andrew Jeffery <[email protected]>
---
arch/arm/boot/dts/aspeed-ast2600-evb.dts | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index 2772796e215e..2b8634513fe6 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -107,7 +107,8 @@
&emmc {
non-removable;
bus-width = <4>;
- max-frequency = <52000000>;
+ max-frequency = <100000000>;
+ clk-phase-mmc-hs200 = <9>, <225>;
};

&rtc {
--
2.17.1

2021-05-21 08:59:04

by Joel Stanley

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] ARM: dts: aspeed: ast2600evb: Add phase correction for emmc controller.

On Thu, 20 May 2021 at 10:16, Steven Lee <[email protected]> wrote:
>
> Set MMC timing-phase register by adding the phase correction binding in the
> device tree.
>
> Signed-off-by: Steven Lee <[email protected]>
> Acked-by: Andrew Jeffery <[email protected]>

As the aspeed maintainer:

Reviewed-by: Joel Stanley <[email protected]>

I don't mind if this gets applied directly by the mmc maintainers (I
do not anticpiate any conflicts).

Cheers,

Joel

2021-05-21 09:00:43

by Andrew Jeffery

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the devicetree.



On Thu, 20 May 2021, at 19:43, Steven Lee wrote:
> The hardware provides capability configuration registers for each SDHCI
> in the global configuration space for the SD controller. Writes to the
> global capability registers are mirrored to the capability registers in
> the associated SDHCI. Configuration of the capabilities must be written
> through the mirror registers prior to initialisation of the SDHCI.
>
> Signed-off-by: Steven Lee <[email protected]>

Reviewed-by: Andrew Jeffery <[email protected]>

2021-05-21 09:04:08

by Joel Stanley

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the devicetree.

On Thu, 20 May 2021 at 10:16, Steven Lee <[email protected]> wrote:
>
> The hardware provides capability configuration registers for each SDHCI
> in the global configuration space for the SD controller. Writes to the
> global capability registers are mirrored to the capability registers in
> the associated SDHCI. Configuration of the capabilities must be written
> through the mirror registers prior to initialisation of the SDHCI.
>
> Signed-off-by: Steven Lee <[email protected]>

Reviewed-by: Joel Stanley <[email protected]>

MMC maintainers, please ignore my other mail about taking the dts
patch through your tree. I didn't realise there were two dts patches;
I'll take them both through the aspeed tree once they are ready.
Please don't let that review stop you from applying this patch.

Cheers,

Joel

> ---
> drivers/mmc/host/sdhci-of-aspeed.c | 48 ++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
> index d001c51074a0..65b5685f6c15 100644
> --- a/drivers/mmc/host/sdhci-of-aspeed.c
> +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> @@ -31,6 +31,11 @@
> #define ASPEED_SDC_S0_PHASE_OUT_EN GENMASK(1, 0)
> #define ASPEED_SDC_PHASE_MAX 31
>
> +/* SDIO{10,20} */
> +#define ASPEED_SDC_CAP1_1_8V (0 * 32 + 26)
> +/* SDIO{14,24} */
> +#define ASPEED_SDC_CAP2_SDR104 (1 * 32 + 1)
> +
> struct aspeed_sdc {
> struct clk *clk;
> struct resource *res;
> @@ -72,6 +77,37 @@ struct aspeed_sdhci {
> const struct aspeed_sdhci_phase_desc *phase_desc;
> };
>
> +/*
> + * The function sets the mirror register for updating
> + * capbilities of the current slot.
> + *
> + * slot | capability | caps_reg | mirror_reg
> + * -----|-------------|----------|------------
> + * 0 | CAP1_1_8V | SDIO140 | SDIO10
> + * 0 | CAP2_SDR104 | SDIO144 | SDIO14
> + * 1 | CAP1_1_8V | SDIO240 | SDIO20
> + * 1 | CAP2_SDR104 | SDIO244 | SDIO24
> + */
> +static void aspeed_sdc_set_slot_capability(struct sdhci_host *host, struct aspeed_sdc *sdc,
> + int capability, bool enable, u8 slot)
> +{
> + u32 mirror_reg_offset;
> + u32 cap_val;
> + u8 cap_reg;
> +
> + if (slot > 1)
> + return;
> +
> + cap_reg = capability / 32;
> + cap_val = sdhci_readl(host, 0x40 + (cap_reg * 4));
> + if (enable)
> + cap_val |= BIT(capability % 32);
> + else
> + cap_val &= ~BIT(capability % 32);
> + mirror_reg_offset = ((slot + 1) * 0x10) + (cap_reg * 4);
> + writel(cap_val, sdc->regs + mirror_reg_offset);
> +}
> +
> static void aspeed_sdc_configure_8bit_mode(struct aspeed_sdc *sdc,
> struct aspeed_sdhci *sdhci,
> bool bus8)
> @@ -328,6 +364,7 @@ static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev,
> static int aspeed_sdhci_probe(struct platform_device *pdev)
> {
> const struct aspeed_sdhci_pdata *aspeed_pdata;
> + struct device_node *np = pdev->dev.of_node;
> struct sdhci_pltfm_host *pltfm_host;
> struct aspeed_sdhci *dev;
> struct sdhci_host *host;
> @@ -372,6 +409,17 @@ static int aspeed_sdhci_probe(struct platform_device *pdev)
>
> sdhci_get_of_property(pdev);
>
> + if (of_property_read_bool(np, "mmc-hs200-1_8v") ||
> + of_property_read_bool(np, "sd-uhs-sdr104")) {
> + aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP1_1_8V,
> + true, slot);
> + }
> +
> + if (of_property_read_bool(np, "sd-uhs-sdr104")) {
> + aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP2_SDR104,
> + true, slot);
> + }
> +
> pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
> if (IS_ERR(pltfm_host->clk))
> return PTR_ERR(pltfm_host->clk);
> --
> 2.17.1
>