Discussion:
[PATCH 03/11] clk: sunxi-ng: a83t: Support new timing mode for mmc2 clock
(too old to reply)
Chen-Yu Tsai
2017-07-14 06:42:54 UTC
Permalink
Raw Message
The MMC2 clock supports a new timing mode. When the new mode is active,
the output clock rate is halved.

This patch sets the feature flag for the new timing mode, and adds
a pre-divider based on the mode bit.

Signed-off-by: Chen-Yu Tsai <***@csie.org>
---
drivers/clk/sunxi-ng/ccu-sun8i-a83t.c | 38 +++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
index 947f9f6e05d2..ee6688e9b361 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
@@ -418,14 +418,36 @@ static SUNXI_CCU_PHASE(mmc1_sample_clk, "mmc1-sample", "mmc1",
static SUNXI_CCU_PHASE(mmc1_output_clk, "mmc1-output", "mmc1",
0x08c, 8, 3, 0);

-/* TODO Support MMC2 clock's new timing mode. */
-static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mod0_default_parents,
- 0x090,
- 0, 4, /* M */
- 16, 2, /* P */
- 24, 2, /* mux */
- BIT(31), /* gate */
- 0);
+/*
+ * MMC2 supports both old and new timing modes. When the new timing
+ * mode is active, the output clock rate is halved by two. Here we
+ * treat it as a variable pre-divider. Note that the pre-divider is
+ * _not_ included in the possible factors during a set clock rate
+ * operation. It is only read out.
+ */
+static const struct ccu_mux_var_prediv mmc2_new_timing_predivs[] = {
+ { .index = 0, .shift = 30, .width = 1 },
+ { .index = 1, .shift = 30, .width = 1 },
+};
+static struct ccu_mp mmc2_clk = {
+ .enable = BIT(31),
+ .m = _SUNXI_CCU_DIV(0, 4),
+ .p = _SUNXI_CCU_DIV(16, 2),
+ .mux = {
+ .shift = 24,
+ .width = 2,
+ .var_predivs = mmc2_new_timing_predivs,
+ .n_var_predivs = ARRAY_SIZE(mmc2_new_timing_predivs),
+ },
+ .common = {
+ .reg = 0x090,
+ .features = CCU_FEATURE_MMC_TIMING_SWITCH,
+ .hw.init = CLK_HW_INIT_PARENTS("mmc2",
+ mod0_default_parents,
+ &ccu_mp_ops,
+ CLK_GET_RATE_NOCACHE),
+ },
+};

static SUNXI_CCU_PHASE(mmc2_sample_clk, "mmc2-sample", "mmc2",
0x090, 20, 3, 0);
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 06:42:52 UTC
Permalink
Raw Message
Now that the CCU device tree binding headers have been merged, we can
use the properly named macros in the device tree, instead of raw
numbers.

Signed-off-by: Chen-Yu Tsai <***@csie.org>
---

This patch is included as it is a pre-requisite to the other device
tree changes in this series. It is however independent, and should
be applied as a fix for 4.13-rc, not for -next, and preferably sooner
than later.

---
arch/arm/boot/dts/sun8i-a83t.dtsi | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 21439d0aa55d..beed05e10a3b 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -44,7 +44,9 @@

#include <dt-bindings/interrupt-controller/arm-gic.h>

+#include <dt-bindings/clock/sun8i-a83t-ccu.h>
#include <dt-bindings/clock/sun8i-r-ccu.h>
+#include <dt-bindings/reset/sun8i-a83t-ccu.h>

/ {
interrupt-parent = <&gic>;
@@ -175,8 +177,8 @@
compatible = "allwinner,sun8i-a83t-dma";
reg = <0x01c02000 0x1000>;
interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&ccu 21>;
- resets = <&ccu 7>;
+ clocks = <&ccu CLK_BUS_DMA>;
+ resets = <&ccu RST_BUS_DMA>;
#dma-cells = <1>;
};

@@ -195,7 +197,7 @@
<GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
reg = <0x01c20800 0x400>;
- clocks = <&ccu 45>, <&osc24M>, <&osc16Md512>;
+ clocks = <&ccu CLK_BUS_PIO>, <&osc24M>, <&osc16Md512>;
clock-names = "apb", "hosc", "losc";
gpio-controller;
interrupt-controller;
@@ -247,8 +249,8 @@
"allwinner,sun8i-h3-spdif";
reg = <0x01c21000 0x400>;
interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&ccu 44>, <&ccu 76>;
- resets = <&ccu 32>;
+ clocks = <&ccu CLK_BUS_SPDIF>, <&ccu CLK_SPDIF>;
+ resets = <&ccu RST_BUS_SPDIF>;
clock-names = "apb", "spdif";
dmas = <&dma 2>;
dma-names = "tx";
@@ -263,8 +265,8 @@
interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
reg-shift = <2>;
reg-io-width = <4>;
- clocks = <&ccu 53>;
- resets = <&ccu 40>;
+ clocks = <&ccu CLK_BUS_UART0>;
+ resets = <&ccu RST_BUS_UART0>;
status = "disabled";
};
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Maxime Ripard
2017-07-17 09:06:51 UTC
Permalink
Raw Message
Post by Chen-Yu Tsai
Now that the CCU device tree binding headers have been merged, we can
use the properly named macros in the device tree, instead of raw
numbers.
---
This patch is included as it is a pre-requisite to the other device
tree changes in this series. It is however independent, and should
be applied as a fix for 4.13-rc, not for -next, and preferably sooner
than later.
Applied as a fix for 4.13, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 06:42:55 UTC
Permalink
Raw Message
The register for the "new timing mode" also has bit fields for setting
output and sample timing phases. According to comments in Allwinner's
BSP kernel, the default values are good enough.

Keep the default values already in the hardware when setting new timing
mode, instead of overwriting the whole register.

Fixes: 9a37e53e451e ("mmc: sunxi: Enable the new timings for the A64 MMC
controllers")
Signed-off-by: Chen-Yu Tsai <***@csie.org>
---
drivers/mmc/host/sunxi-mmc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index d6fa2214aaae..0fb4e4c119e1 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -793,8 +793,12 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
}
mmc_writel(host, REG_CLKCR, rval);

- if (host->cfg->needs_new_timings)
- mmc_writel(host, REG_SD_NTSR, SDXC_2X_TIMING_MODE);
+ if (host->cfg->needs_new_timings) {
+ /* Don't touch the delay bits */
+ rval = mmc_readl(host, REG_SD_NTSR);
+ rval |= SDXC_2X_TIMING_MODE;
+ mmc_writel(host, REG_SD_NTSR, rval);
+ }

ret = sunxi_mmc_clk_set_phase(host, ios, rate);
if (ret)
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Ulf Hansson
2017-07-14 09:16:41 UTC
Permalink
Raw Message
Post by Chen-Yu Tsai
The register for the "new timing mode" also has bit fields for setting
output and sample timing phases. According to comments in Allwinner's
BSP kernel, the default values are good enough.
Keep the default values already in the hardware when setting new timing
mode, instead of overwriting the whole register.
Fixes: 9a37e53e451e ("mmc: sunxi: Enable the new timings for the A64 MMC
controllers")
It looks like this change doesn't depend on anything else? Do you want
me to pick it up for fixes and adding stable tag?

Kind regards
Uffe
Post by Chen-Yu Tsai
---
drivers/mmc/host/sunxi-mmc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index d6fa2214aaae..0fb4e4c119e1 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -793,8 +793,12 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
}
mmc_writel(host, REG_CLKCR, rval);
- if (host->cfg->needs_new_timings)
- mmc_writel(host, REG_SD_NTSR, SDXC_2X_TIMING_MODE);
+ if (host->cfg->needs_new_timings) {
+ /* Don't touch the delay bits */
+ rval = mmc_readl(host, REG_SD_NTSR);
+ rval |= SDXC_2X_TIMING_MODE;
+ mmc_writel(host, REG_SD_NTSR, rval);
+ }
ret = sunxi_mmc_clk_set_phase(host, ios, rate);
if (ret)
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 09:44:40 UTC
Permalink
Raw Message
Post by Ulf Hansson
Post by Chen-Yu Tsai
The register for the "new timing mode" also has bit fields for setting
output and sample timing phases. According to comments in Allwinner's
BSP kernel, the default values are good enough.
Keep the default values already in the hardware when setting new timing
mode, instead of overwriting the whole register.
Fixes: 9a37e53e451e ("mmc: sunxi: Enable the new timings for the A64 MMC
controllers")
It looks like this change doesn't depend on anything else? Do you want
me to pick it up for fixes and adding stable tag?
Yes, please.

ChenYu
Post by Ulf Hansson
Kind regards
Uffe
Post by Chen-Yu Tsai
---
drivers/mmc/host/sunxi-mmc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index d6fa2214aaae..0fb4e4c119e1 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -793,8 +793,12 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
}
mmc_writel(host, REG_CLKCR, rval);
- if (host->cfg->needs_new_timings)
- mmc_writel(host, REG_SD_NTSR, SDXC_2X_TIMING_MODE);
+ if (host->cfg->needs_new_timings) {
+ /* Don't touch the delay bits */
+ rval = mmc_readl(host, REG_SD_NTSR);
+ rval |= SDXC_2X_TIMING_MODE;
+ mmc_writel(host, REG_SD_NTSR, rval);
+ }
ret = sunxi_mmc_clk_set_phase(host, ios, rate);
if (ret)
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Maxime Ripard
2017-07-17 09:14:44 UTC
Permalink
Raw Message
Post by Chen-Yu Tsai
The register for the "new timing mode" also has bit fields for setting
output and sample timing phases. According to comments in Allwinner's
BSP kernel, the default values are good enough.
Keep the default values already in the hardware when setting new timing
mode, instead of overwriting the whole register.
Fixes: 9a37e53e451e ("mmc: sunxi: Enable the new timings for the A64 MMC
controllers")
Acked-by: Maxime Ripard <***@free-electrons.com>

Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Ulf Hansson
2017-07-17 10:37:41 UTC
Permalink
Raw Message
+stable
Post by Chen-Yu Tsai
The register for the "new timing mode" also has bit fields for setting
output and sample timing phases. According to comments in Allwinner's
BSP kernel, the default values are good enough.
Keep the default values already in the hardware when setting new timing
mode, instead of overwriting the whole register.
Fixes: 9a37e53e451e ("mmc: sunxi: Enable the new timings for the A64 MMC
controllers")
Thanks, applied for fixes and added a stable tag.

Kind regards
Uffe
Post by Chen-Yu Tsai
---
drivers/mmc/host/sunxi-mmc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index d6fa2214aaae..0fb4e4c119e1 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -793,8 +793,12 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
}
mmc_writel(host, REG_CLKCR, rval);
- if (host->cfg->needs_new_timings)
- mmc_writel(host, REG_SD_NTSR, SDXC_2X_TIMING_MODE);
+ if (host->cfg->needs_new_timings) {
+ /* Don't touch the delay bits */
+ rval = mmc_readl(host, REG_SD_NTSR);
+ rval |= SDXC_2X_TIMING_MODE;
+ mmc_writel(host, REG_SD_NTSR, rval);
+ }
ret = sunxi_mmc_clk_set_phase(host, ios, rate);
if (ret)
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 06:42:53 UTC
Permalink
Raw Message
Starting with the A83T SoC, Allwinner introduced a new timing mode for
its MMC clocks. The new mode changes how the MMC controller sample and
output clocks are delayed to match chip and board specifics. There are
two controls for this, one on the CCU side controlling how the clocks
behave, and one in the MMC controller controlling what inputs to take
and how to route them.

In the old mode, the MMC clock had 2 child clocks providing the output
and sample clocks, which could be delayed by a number of clock cycles
measured from the MMC clock's parent.

With the new mode, the 2 delay clocks are no longer active. Instead,
the delays and associated controls are moved into the MMC controller.
The output of the MMC clock is also halved.

The difference in how things are wired between the modes means that the
clock controls and the MMC controls must match. To achieve this in a
clear, explicit way, we introduce two functions for the MMC driver to
use: one queries the hardware for the current mode set, and the other
allows the MMC driver to request a mode.

With newer SoCs such as the A64, the old mode is all but removed. Hence
we support two variations, one where the mode can be toggled, and the
other where the clock is fixed in the new mode.

Signed-off-by: Chen-Yu Tsai <***@csie.org>
---
drivers/clk/sunxi-ng/Makefile | 1 +
drivers/clk/sunxi-ng/ccu_common.h | 2 +
drivers/clk/sunxi-ng/ccu_mmc_timing.c | 73 +++++++++++++++++++++++++++++++++++
include/linux/clk/sunxi-ng.h | 20 ++++++++++
4 files changed, 96 insertions(+)
create mode 100644 drivers/clk/sunxi-ng/ccu_mmc_timing.c
create mode 100644 include/linux/clk/sunxi-ng.h

diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
index 0c45fa50283d..45a5910379a5 100644
--- a/drivers/clk/sunxi-ng/Makefile
+++ b/drivers/clk/sunxi-ng/Makefile
@@ -1,5 +1,6 @@
# Common objects
lib-$(CONFIG_SUNXI_CCU) += ccu_common.o
+lib-$(CONFIG_SUNXI_CCU) += ccu_mmc_timing.o
lib-$(CONFIG_SUNXI_CCU) += ccu_reset.o

# Base clock types
diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-ng/ccu_common.h
index d6fdd7a789aa..88981e7fd978 100644
--- a/drivers/clk/sunxi-ng/ccu_common.h
+++ b/drivers/clk/sunxi-ng/ccu_common.h
@@ -23,6 +23,8 @@
#define CCU_FEATURE_FIXED_POSTDIV BIT(3)
#define CCU_FEATURE_ALL_PREDIV BIT(4)
#define CCU_FEATURE_LOCK_REG BIT(5)
+#define CCU_FEATURE_MMC_TIMING_SWITCH BIT(6)
+#define CCU_FEATURE_MMC_ALWAYS_NEW BIT(7)

struct device_node;

diff --git a/drivers/clk/sunxi-ng/ccu_mmc_timing.c b/drivers/clk/sunxi-ng/ccu_mmc_timing.c
new file mode 100644
index 000000000000..f236a56abf48
--- /dev/null
+++ b/drivers/clk/sunxi-ng/ccu_mmc_timing.c
@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2017 Chen-Yu Tsai. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clk/sunxi-ng.h>
+
+#include "ccu_common.h"
+
+#define CCU_MMC_NEW_TIMING_MODE BIT(30)
+
+/**
+ * sunxi_ccu_set_mmc_timing_mode: Configure the MMC clock timing mode
+ * @clk: clock to be configured
+ * @new_mode: true for new timing mode introduced in A83T and later
+ *
+ * Returns 0 on success, -ENOTSUPP if the clock does not support
+ * switching modes.
+ */
+int sunxi_ccu_set_mmc_timing_mode(struct clk *clk, bool new_mode)
+{
+ struct clk_hw *hw = __clk_get_hw(clk);
+ struct ccu_common *cm = hw_to_ccu_common(hw);
+ unsigned long flags;
+ u32 val;
+
+ if (!(cm->features & CCU_FEATURE_MMC_TIMING_SWITCH))
+ return -ENOTSUPP;
+
+ spin_lock_irqsave(cm->lock, flags);
+
+ val = readl(cm->base + cm->reg);
+ if (new_mode)
+ val |= CCU_MMC_NEW_TIMING_MODE;
+ else
+ val &= ~CCU_MMC_NEW_TIMING_MODE;
+ writel(val, cm->base + cm->reg);
+
+ spin_unlock_irqrestore(cm->lock, flags);
+
+ return 0;
+}
+
+/**
+ * sunxi_ccu_set_mmc_timing_mode: Get the current MMC clock timing mode
+ * @clk: clock to query
+ *
+ * Returns 0 if the clock is in old timing mode, > 0 if it is in
+ * new timing mode, and -ENOTSUPP if the clock does not support
+ * this function.
+ */
+int sunxi_ccu_get_mmc_timing_mode(struct clk *clk)
+{
+ struct clk_hw *hw = __clk_get_hw(clk);
+ struct ccu_common *cm = hw_to_ccu_common(hw);
+
+ if (cm->features & CCU_FEATURE_MMC_ALWAYS_NEW)
+ return 1;
+
+ if (!(cm->features & CCU_FEATURE_MMC_TIMING_SWITCH))
+ return -ENOTSUPP;
+
+ return !!(readl(cm->base + cm->reg) & CCU_MMC_NEW_TIMING_MODE);
+}
diff --git a/include/linux/clk/sunxi-ng.h b/include/linux/clk/sunxi-ng.h
new file mode 100644
index 000000000000..f1bd321fc264
--- /dev/null
+++ b/include/linux/clk/sunxi-ng.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright (c) 2017 Chen-Yu Tsai. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _LINUX_CLK_SUNXI_NG_H_
+#define _LINUX_CLK_SUNXI_NG_H_
+
+int sunxi_ccu_set_mmc_timing_mode(struct clk *clk, bool new_mode);
+int sunxi_ccu_get_mmc_timing_mode(struct clk *clk);
+
+#endif
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Maxime Ripard
2017-07-17 09:09:45 UTC
Permalink
Raw Message
Hi,
Post by Chen-Yu Tsai
Starting with the A83T SoC, Allwinner introduced a new timing mode for
its MMC clocks. The new mode changes how the MMC controller sample and
output clocks are delayed to match chip and board specifics. There are
two controls for this, one on the CCU side controlling how the clocks
behave, and one in the MMC controller controlling what inputs to take
and how to route them.
In the old mode, the MMC clock had 2 child clocks providing the output
and sample clocks, which could be delayed by a number of clock cycles
measured from the MMC clock's parent.
With the new mode, the 2 delay clocks are no longer active. Instead,
the delays and associated controls are moved into the MMC controller.
The output of the MMC clock is also halved.
The difference in how things are wired between the modes means that the
clock controls and the MMC controls must match. To achieve this in a
clear, explicit way, we introduce two functions for the MMC driver to
use: one queries the hardware for the current mode set, and the other
allows the MMC driver to request a mode.
With newer SoCs such as the A64, the old mode is all but removed. Hence
we support two variations, one where the mode can be toggled, and the
other where the clock is fixed in the new mode.
---
drivers/clk/sunxi-ng/Makefile | 1 +
drivers/clk/sunxi-ng/ccu_common.h | 2 +
drivers/clk/sunxi-ng/ccu_mmc_timing.c | 73 +++++++++++++++++++++++++++++++++++
include/linux/clk/sunxi-ng.h | 20 ++++++++++
4 files changed, 96 insertions(+)
create mode 100644 drivers/clk/sunxi-ng/ccu_mmc_timing.c
create mode 100644 include/linux/clk/sunxi-ng.h
diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
index 0c45fa50283d..45a5910379a5 100644
--- a/drivers/clk/sunxi-ng/Makefile
+++ b/drivers/clk/sunxi-ng/Makefile
@@ -1,5 +1,6 @@
# Common objects
lib-$(CONFIG_SUNXI_CCU) += ccu_common.o
+lib-$(CONFIG_SUNXI_CCU) += ccu_mmc_timing.o
lib-$(CONFIG_SUNXI_CCU) += ccu_reset.o
# Base clock types
diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-ng/ccu_common.h
index d6fdd7a789aa..88981e7fd978 100644
--- a/drivers/clk/sunxi-ng/ccu_common.h
+++ b/drivers/clk/sunxi-ng/ccu_common.h
@@ -23,6 +23,8 @@
#define CCU_FEATURE_FIXED_POSTDIV BIT(3)
#define CCU_FEATURE_ALL_PREDIV BIT(4)
#define CCU_FEATURE_LOCK_REG BIT(5)
+#define CCU_FEATURE_MMC_TIMING_SWITCH BIT(6)
+#define CCU_FEATURE_MMC_ALWAYS_NEW BIT(7)
I'm not really sure we need the ALWAYS_NEW bit here. In the case where
the clocks cannot operate in the old mode any more, we won't even
query the clocks, since we know that it's not needed at all.

Pretty much just like what we're doing for old-mode-only clocks at the
moment.

I guess the only thing we should indentify is whether the clock can
switch between the two, or not, and the MMC_TIMING_SWITCH bit is
already perfect for that.

Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 06:42:57 UTC
Permalink
Raw Message
The MMC controller can support DDR52 transfers under the new timing
mode. According to the BSP kernel, the module clock has to be double
the card clock, regardless of the bus width. The default timings in
the hardware can be used.

This also reworks the code setting the internal divider, getting rid
of a extra conditional.

Signed-off-by: Chen-Yu Tsai <***@csie.org>
---
drivers/mmc/host/sunxi-mmc.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 56e45c65b52d..7b6f5f49620e 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -751,7 +751,7 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
{
struct mmc_host *mmc = host->mmc;
long rate;
- u32 rval, clock = ios->clock;
+ u32 rval, clock = ios->clock, div = 1;
int ret;

ret = sunxi_mmc_oclk_onoff(host, 0);
@@ -764,10 +764,21 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
if (!ios->clock)
return 0;

- /* 8 bit DDR requires a higher module clock */
+ /*
+ * Under the old timing mode, 8 bit DDR requires the module
+ * clock to be double the card clock. Under the new timing
+ * mode, all DDR modes require a doubled module clock.
+ *
+ * We currently only support the standard MMC DDR52 mode.
+ * This block should be updated once support for other DDR
+ * modes is added.
+ */
if (ios->timing == MMC_TIMING_MMC_DDR52 &&
- ios->bus_width == MMC_BUS_WIDTH_8)
+ (host->use_new_timings ||
+ ios->bus_width == MMC_BUS_WIDTH_8)) {
+ div = 2;
clock <<= 1;
+ }

if (host->use_new_timings) {
ret = sunxi_ccu_set_mmc_timing_mode(host->clk_mmc, true);
@@ -795,15 +806,10 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
return ret;
}

- /* clear internal divider */
+ /* set internal divider */
rval = mmc_readl(host, REG_CLKCR);
rval &= ~0xff;
- /* set internal divider for 8 bit eMMC DDR, so card clock is right */
- if (ios->timing == MMC_TIMING_MMC_DDR52 &&
- ios->bus_width == MMC_BUS_WIDTH_8) {
- rval |= 1;
- rate >>= 1;
- }
+ rval |= div - 1;
mmc_writel(host, REG_CLKCR, rval);

if (host->use_new_timings) {
@@ -834,7 +840,7 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
return ret;

/* And we just enabled our clock back */
- mmc->actual_clock = rate;
+ mmc->actual_clock = rate / div;

return 0;
}
@@ -1300,7 +1306,7 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
MMC_CAP_ERASE | MMC_CAP_SDIO_IRQ;

- if (host->cfg->clk_delays)
+ if (host->cfg->clk_delays || host->use_new_timings)
mmc->caps |= MMC_CAP_1_8V_DDR;

ret = mmc_of_parse(mmc);
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 06:43:00 UTC
Permalink
Raw Message
mmc2 can support 8-bit eMMC chips, with a dedicated reset line.

Signed-off-by: Chen-Yu Tsai <***@csie.org>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 085312d0c521..b1198d80873e 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -269,6 +269,15 @@
bias-pull-up;
};

+ mmc2_8bit_emmc_pins: mmc2-8bit-emmc-pins {
+ pins = "PC5", "PC6", "PC8", "PC9",
+ "PC10", "PC11", "PC12", "PC13",
+ "PC14", "PC15", "PC16";
+ function = "mmc2";
+ drive-strength = <30>;
+ bias-pull-up;
+ };
+
spdif_tx_pin: spdif-tx-pin {
pins = "PE18";
function = "spdif";
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 06:42:58 UTC
Permalink
Raw Message
The third MMC controller (MMC2) on the Allwinner A83T SoC is slightly
different. It supports a wider 8-bit bus, has a dedicated controllable
reset pin for eMMC, and a "new timing mode" which is supposed to deliver
better signals and thus better performance.

Add a compatible for this one to use the new timing mode not found in the
other controllers.

Signed-off-by: Chen-Yu Tsai <***@csie.org>
---
Documentation/devicetree/bindings/mmc/sunxi-mmc.txt | 1 +
drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
2 files changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/sunxi-mmc.txt b/Documentation/devicetree/bindings/mmc/sunxi-mmc.txt
index 7d53a799f140..63b57e2a10fb 100644
--- a/Documentation/devicetree/bindings/mmc/sunxi-mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/sunxi-mmc.txt
@@ -12,6 +12,7 @@ Required properties:
* "allwinner,sun4i-a10-mmc"
* "allwinner,sun5i-a13-mmc"
* "allwinner,sun7i-a20-mmc"
+ * "allwinner,sun8i-a83t-emmc"
* "allwinner,sun9i-a80-mmc"
* "allwinner,sun50i-a64-emmc"
* "allwinner,sun50i-a64-mmc"
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 7b6f5f49620e..e70065dbd50e 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1113,6 +1113,13 @@ static const struct sunxi_mmc_cfg sun7i_a20_cfg = {
.can_calibrate = false,
};

+static const struct sunxi_mmc_cfg sun8i_a83t_emmc_cfg = {
+ .idma_des_size_bits = 16,
+ .clk_delays = sunxi_mmc_clk_delays,
+ .can_calibrate = false,
+ .has_new_timings = true,
+};
+
static const struct sunxi_mmc_cfg sun9i_a80_cfg = {
.idma_des_size_bits = 16,
.clk_delays = sun9i_mmc_clk_delays,
@@ -1137,6 +1144,7 @@ static const struct of_device_id sunxi_mmc_of_match[] = {
{ .compatible = "allwinner,sun4i-a10-mmc", .data = &sun4i_a10_cfg },
{ .compatible = "allwinner,sun5i-a13-mmc", .data = &sun5i_a13_cfg },
{ .compatible = "allwinner,sun7i-a20-mmc", .data = &sun7i_a20_cfg },
+ { .compatible = "allwinner,sun8i-a83t-emmc", .data = &sun8i_a83t_emmc_cfg },
{ .compatible = "allwinner,sun9i-a80-mmc", .data = &sun9i_a80_cfg },
{ .compatible = "allwinner,sun50i-a64-mmc", .data = &sun50i_a64_cfg },
{ .compatible = "allwinner,sun50i-a64-emmc", .data = &sun50i_a64_emmc_cfg },
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Rob Herring
2017-07-17 18:51:26 UTC
Permalink
Raw Message
Post by Chen-Yu Tsai
The third MMC controller (MMC2) on the Allwinner A83T SoC is slightly
different. It supports a wider 8-bit bus, has a dedicated controllable
reset pin for eMMC, and a "new timing mode" which is supposed to deliver
better signals and thus better performance.
Add a compatible for this one to use the new timing mode not found in the
other controllers.
---
Documentation/devicetree/bindings/mmc/sunxi-mmc.txt | 1 +
drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
2 files changed, 9 insertions(+)
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 06:42:56 UTC
Permalink
Raw Message
On the SoCs that introduced the new timing mode for MMC controllers,
both the old (where the clock delays are set in the CCU) and new
(where the clock delays are set in the MMC controller) timing modes
are available, and we have to support them both. However there are
two bits that control which mode is active. One is in the CCU, the
other is in the MMC controller. The settings on both sides must be
the same, or nothing will work.

The CCU's get/set_phase callbacks return -ENOTSUPP when the new
timing mode is active. This provides a way to know which mode is
active on that side, and we can set the bit on the MMC controller
side accordingly.

Signed-off-by: Chen-Yu Tsai <***@csie.org>
---
drivers/mmc/host/sunxi-mmc.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 0fb4e4c119e1..56e45c65b52d 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -22,6 +22,7 @@
#include <linux/err.h>

#include <linux/clk.h>
+#include <linux/clk/sunxi-ng.h>
#include <linux/gpio.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
@@ -259,7 +260,7 @@ struct sunxi_mmc_cfg {
/* Does DATA0 needs to be masked while the clock is updated */
bool mask_data0;

- bool needs_new_timings;
+ bool has_new_timings;
};

struct sunxi_mmc_host {
@@ -293,6 +294,9 @@ struct sunxi_mmc_host {

/* vqmmc */
bool vqmmc_enabled;
+
+ /* timings */
+ bool use_new_timings;
};

static int sunxi_mmc_reset_host(struct sunxi_mmc_host *host)
@@ -714,7 +718,7 @@ static int sunxi_mmc_clk_set_phase(struct sunxi_mmc_host *host,
{
int index;

- if (!host->cfg->clk_delays)
+ if (host->use_new_timings)
return 0;

/* determine delays */
@@ -765,6 +769,15 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
ios->bus_width == MMC_BUS_WIDTH_8)
clock <<= 1;

+ if (host->use_new_timings) {
+ ret = sunxi_ccu_set_mmc_timing_mode(host->clk_mmc, true);
+ if (ret) {
+ dev_err(mmc_dev(mmc),
+ "error setting new timing mode\n");
+ return ret;
+ }
+ }
+
rate = clk_round_rate(host->clk_mmc, clock);
if (rate < 0) {
dev_err(mmc_dev(mmc), "error rounding clk to %d: %ld\n",
@@ -793,7 +806,7 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
}
mmc_writel(host, REG_CLKCR, rval);

- if (host->cfg->needs_new_timings) {
+ if (host->use_new_timings) {
/* Don't touch the delay bits */
rval = mmc_readl(host, REG_SD_NTSR);
rval |= SDXC_2X_TIMING_MODE;
@@ -1105,7 +1118,7 @@ static const struct sunxi_mmc_cfg sun50i_a64_cfg = {
.clk_delays = NULL,
.can_calibrate = true,
.mask_data0 = true,
- .needs_new_timings = true,
+ .has_new_timings = true,
};

static const struct sunxi_mmc_cfg sun50i_a64_emmc_cfg = {
@@ -1262,6 +1275,19 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
goto error_free_host;
}

+ if (host->cfg->clk_delays && host->cfg->has_new_timings) {
+ /*
+ * Supports both old and new timing modes.
+ * Try setting the clk to new timing mode.
+ */
+ ret = sunxi_ccu_set_mmc_timing_mode(host->clk_mmc, true);
+ if (!ret)
+ host->use_new_timings = true;
+ } else if (host->cfg->has_new_timings) {
+ /* Supports new timing mode only */
+ host->use_new_timings = true;
+ }
+
mmc->ops = &sunxi_mmc_ops;
mmc->max_blk_count = 8192;
mmc->max_blk_size = 4096;
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Ulf Hansson
2017-07-14 09:26:32 UTC
Permalink
Raw Message
Post by Chen-Yu Tsai
On the SoCs that introduced the new timing mode for MMC controllers,
both the old (where the clock delays are set in the CCU) and new
(where the clock delays are set in the MMC controller) timing modes
are available, and we have to support them both. However there are
two bits that control which mode is active. One is in the CCU, the
other is in the MMC controller. The settings on both sides must be
the same, or nothing will work.
The CCU's get/set_phase callbacks return -ENOTSUPP when the new
timing mode is active. This provides a way to know which mode is
active on that side, and we can set the bit on the MMC controller
side accordingly.
---
drivers/mmc/host/sunxi-mmc.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 0fb4e4c119e1..56e45c65b52d 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -22,6 +22,7 @@
#include <linux/err.h>
#include <linux/clk.h>
+#include <linux/clk/sunxi-ng.h>
I don't like this. This looks like an SoC specific hack.
Post by Chen-Yu Tsai
#include <linux/gpio.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
@@ -259,7 +260,7 @@ struct sunxi_mmc_cfg {
/* Does DATA0 needs to be masked while the clock is updated */
bool mask_data0;
- bool needs_new_timings;
+ bool has_new_timings;
};
struct sunxi_mmc_host {
@@ -293,6 +294,9 @@ struct sunxi_mmc_host {
/* vqmmc */
bool vqmmc_enabled;
+
+ /* timings */
+ bool use_new_timings;
};
static int sunxi_mmc_reset_host(struct sunxi_mmc_host *host)
@@ -714,7 +718,7 @@ static int sunxi_mmc_clk_set_phase(struct sunxi_mmc_host *host,
{
int index;
- if (!host->cfg->clk_delays)
+ if (host->use_new_timings)
return 0;
/* determine delays */
@@ -765,6 +769,15 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
ios->bus_width == MMC_BUS_WIDTH_8)
clock <<= 1;
+ if (host->use_new_timings) {
+ ret = sunxi_ccu_set_mmc_timing_mode(host->clk_mmc, true);
Can't this be solved through some other generic API/interface?
Post by Chen-Yu Tsai
+ if (ret) {
+ dev_err(mmc_dev(mmc),
+ "error setting new timing mode\n");
+ return ret;
+ }
+ }
+
[...]

Kind regards
Uffe
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 09:40:34 UTC
Permalink
Raw Message
Post by Ulf Hansson
Post by Chen-Yu Tsai
On the SoCs that introduced the new timing mode for MMC controllers,
both the old (where the clock delays are set in the CCU) and new
(where the clock delays are set in the MMC controller) timing modes
are available, and we have to support them both. However there are
two bits that control which mode is active. One is in the CCU, the
other is in the MMC controller. The settings on both sides must be
the same, or nothing will work.
The CCU's get/set_phase callbacks return -ENOTSUPP when the new
timing mode is active. This provides a way to know which mode is
active on that side, and we can set the bit on the MMC controller
side accordingly.
Argh... I forgot to update the commit log... :(
Post by Ulf Hansson
Post by Chen-Yu Tsai
---
drivers/mmc/host/sunxi-mmc.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 0fb4e4c119e1..56e45c65b52d 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -22,6 +22,7 @@
#include <linux/err.h>
#include <linux/clk.h>
+#include <linux/clk/sunxi-ng.h>
I don't like this. This looks like an SoC specific hack.
Post by Chen-Yu Tsai
#include <linux/gpio.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
@@ -259,7 +260,7 @@ struct sunxi_mmc_cfg {
/* Does DATA0 needs to be masked while the clock is updated */
bool mask_data0;
- bool needs_new_timings;
+ bool has_new_timings;
};
struct sunxi_mmc_host {
@@ -293,6 +294,9 @@ struct sunxi_mmc_host {
/* vqmmc */
bool vqmmc_enabled;
+
+ /* timings */
+ bool use_new_timings;
};
static int sunxi_mmc_reset_host(struct sunxi_mmc_host *host)
@@ -714,7 +718,7 @@ static int sunxi_mmc_clk_set_phase(struct sunxi_mmc_host *host,
{
int index;
- if (!host->cfg->clk_delays)
+ if (host->use_new_timings)
return 0;
/* determine delays */
@@ -765,6 +769,15 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
ios->bus_width == MMC_BUS_WIDTH_8)
clock <<= 1;
+ if (host->use_new_timings) {
+ ret = sunxi_ccu_set_mmc_timing_mode(host->clk_mmc, true);
Can't this be solved through some other generic API/interface?
The old discussion is here: https://lkml.org/lkml/2017/5/5/77

It is possible to piggy back on existing API, but as Maxime mentioned
back in the discussion, it is confusing.

IIRC Mike said (via Maxime) an SoC specific call was the easy way
to handle this. I don't think there's anything generic about this.
Even if you could have a _set_mode callback for the clks, the modes
would be SoC specific anyway.

ChenYu
Post by Ulf Hansson
Post by Chen-Yu Tsai
+ if (ret) {
+ dev_err(mmc_dev(mmc),
+ "error setting new timing mode\n");
+ return ret;
+ }
+ }
+
[...]
Kind regards
Uffe
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Ulf Hansson
2017-07-14 09:57:35 UTC
Permalink
Raw Message
Post by Chen-Yu Tsai
Post by Ulf Hansson
Post by Chen-Yu Tsai
On the SoCs that introduced the new timing mode for MMC controllers,
both the old (where the clock delays are set in the CCU) and new
(where the clock delays are set in the MMC controller) timing modes
are available, and we have to support them both. However there are
two bits that control which mode is active. One is in the CCU, the
other is in the MMC controller. The settings on both sides must be
the same, or nothing will work.
The CCU's get/set_phase callbacks return -ENOTSUPP when the new
timing mode is active. This provides a way to know which mode is
active on that side, and we can set the bit on the MMC controller
side accordingly.
Argh... I forgot to update the commit log... :(
Post by Ulf Hansson
Post by Chen-Yu Tsai
---
drivers/mmc/host/sunxi-mmc.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 0fb4e4c119e1..56e45c65b52d 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -22,6 +22,7 @@
#include <linux/err.h>
#include <linux/clk.h>
+#include <linux/clk/sunxi-ng.h>
I don't like this. This looks like an SoC specific hack.
Post by Chen-Yu Tsai
#include <linux/gpio.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
@@ -259,7 +260,7 @@ struct sunxi_mmc_cfg {
/* Does DATA0 needs to be masked while the clock is updated */
bool mask_data0;
- bool needs_new_timings;
+ bool has_new_timings;
};
struct sunxi_mmc_host {
@@ -293,6 +294,9 @@ struct sunxi_mmc_host {
/* vqmmc */
bool vqmmc_enabled;
+
+ /* timings */
+ bool use_new_timings;
};
static int sunxi_mmc_reset_host(struct sunxi_mmc_host *host)
@@ -714,7 +718,7 @@ static int sunxi_mmc_clk_set_phase(struct sunxi_mmc_host *host,
{
int index;
- if (!host->cfg->clk_delays)
+ if (host->use_new_timings)
return 0;
/* determine delays */
@@ -765,6 +769,15 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
ios->bus_width == MMC_BUS_WIDTH_8)
clock <<= 1;
+ if (host->use_new_timings) {
+ ret = sunxi_ccu_set_mmc_timing_mode(host->clk_mmc, true);
Can't this be solved through some other generic API/interface?
The old discussion is here: https://lkml.org/lkml/2017/5/5/77
It is possible to piggy back on existing API, but as Maxime mentioned
back in the discussion, it is confusing.
IIRC Mike said (via Maxime) an SoC specific call was the easy way
to handle this. I don't think there's anything generic about this.
Even if you could have a _set_mode callback for the clks, the modes
would be SoC specific anyway.
Right. But it would benefit that we can keep drivers generic, as they
are using generic APIs/interfaces. I prefer that.

Anyway, let me try to dig up the earlier discussion.

Kind regards
Uffe
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Maxime Ripard
2017-07-17 09:20:45 UTC
Permalink
Raw Message
Post by Ulf Hansson
Post by Chen-Yu Tsai
Post by Ulf Hansson
Post by Chen-Yu Tsai
+ if (host->use_new_timings) {
+ ret = sunxi_ccu_set_mmc_timing_mode(host->clk_mmc, true);
Can't this be solved through some other generic API/interface?
The old discussion is here: https://lkml.org/lkml/2017/5/5/77
It is possible to piggy back on existing API, but as Maxime mentioned
back in the discussion, it is confusing.
IIRC Mike said (via Maxime) an SoC specific call was the easy way
to handle this. I don't think there's anything generic about this.
Even if you could have a _set_mode callback for the clks, the modes
would be SoC specific anyway.
Right. But it would benefit that we can keep drivers generic, as they
are using generic APIs/interfaces. I prefer that.
Anyway, let me try to dig up the earlier discussion.
There's really not any generic way to support that. Even if we reuse
some other function (clk_set_phase/clk_get_phase was suggested), and
use error codes and / or values to differentiate between two modes,
this will be very much implementation-specific as well, and any other
SoC that in theory would be using that will very likely to not
implement the same behaviour for its clocks.

And this driver is only used on one SoC family, so it's not really a
big deal anyway.

Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Maxime Ripard
2017-07-17 09:17:47 UTC
Permalink
Raw Message
Hi,
Post by Chen-Yu Tsai
On the SoCs that introduced the new timing mode for MMC controllers,
both the old (where the clock delays are set in the CCU) and new
(where the clock delays are set in the MMC controller) timing modes
are available, and we have to support them both. However there are
two bits that control which mode is active. One is in the CCU, the
other is in the MMC controller. The settings on both sides must be
the same, or nothing will work.
The CCU's get/set_phase callbacks return -ENOTSUPP when the new
timing mode is active. This provides a way to know which mode is
active on that side, and we can set the bit on the MMC controller
side accordingly.
---
drivers/mmc/host/sunxi-mmc.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 0fb4e4c119e1..56e45c65b52d 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -22,6 +22,7 @@
#include <linux/err.h>
#include <linux/clk.h>
+#include <linux/clk/sunxi-ng.h>
#include <linux/gpio.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
@@ -259,7 +260,7 @@ struct sunxi_mmc_cfg {
/* Does DATA0 needs to be masked while the clock is updated */
bool mask_data0;
- bool needs_new_timings;
+ bool has_new_timings;
I think we should have both, it's a bit different. Newer SoCs like the
A64 can only operate using new timings, while the older ones can
operate in both modes.

In one case, we're forced to use it, in the other one it's a
policy. We should differentiate both cases.

Looks good otherwise, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-19 08:59:23 UTC
Permalink
Raw Message
On Mon, Jul 17, 2017 at 5:17 PM, Maxime Ripard
Post by Maxime Ripard
Hi,
Post by Chen-Yu Tsai
On the SoCs that introduced the new timing mode for MMC controllers,
both the old (where the clock delays are set in the CCU) and new
(where the clock delays are set in the MMC controller) timing modes
are available, and we have to support them both. However there are
two bits that control which mode is active. One is in the CCU, the
other is in the MMC controller. The settings on both sides must be
the same, or nothing will work.
The CCU's get/set_phase callbacks return -ENOTSUPP when the new
timing mode is active. This provides a way to know which mode is
active on that side, and we can set the bit on the MMC controller
side accordingly.
---
drivers/mmc/host/sunxi-mmc.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 0fb4e4c119e1..56e45c65b52d 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -22,6 +22,7 @@
#include <linux/err.h>
#include <linux/clk.h>
+#include <linux/clk/sunxi-ng.h>
#include <linux/gpio.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
@@ -259,7 +260,7 @@ struct sunxi_mmc_cfg {
/* Does DATA0 needs to be masked while the clock is updated */
bool mask_data0;
- bool needs_new_timings;
+ bool has_new_timings;
I think we should have both, it's a bit different. Newer SoCs like the
A64 can only operate using new timings, while the older ones can
operate in both modes.
In one case, we're forced to use it, in the other one it's a
policy. We should differentiate both cases.
For the A64's case, the limit is implied by not having any clk_delays.

But yes, I'll keep "needs_new_timings", and rename the new option to
"has_timing_switch" to make things clearer.

ChenYu
Post by Maxime Ripard
Looks good otherwise, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Maxime Ripard
2017-07-19 11:28:56 UTC
Permalink
Raw Message
Post by Chen-Yu Tsai
On Mon, Jul 17, 2017 at 5:17 PM, Maxime Ripard
Post by Maxime Ripard
Hi,
Post by Chen-Yu Tsai
On the SoCs that introduced the new timing mode for MMC controllers,
both the old (where the clock delays are set in the CCU) and new
(where the clock delays are set in the MMC controller) timing modes
are available, and we have to support them both. However there are
two bits that control which mode is active. One is in the CCU, the
other is in the MMC controller. The settings on both sides must be
the same, or nothing will work.
The CCU's get/set_phase callbacks return -ENOTSUPP when the new
timing mode is active. This provides a way to know which mode is
active on that side, and we can set the bit on the MMC controller
side accordingly.
---
drivers/mmc/host/sunxi-mmc.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 0fb4e4c119e1..56e45c65b52d 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -22,6 +22,7 @@
#include <linux/err.h>
#include <linux/clk.h>
+#include <linux/clk/sunxi-ng.h>
#include <linux/gpio.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
@@ -259,7 +260,7 @@ struct sunxi_mmc_cfg {
/* Does DATA0 needs to be masked while the clock is updated */
bool mask_data0;
- bool needs_new_timings;
+ bool has_new_timings;
I think we should have both, it's a bit different. Newer SoCs like the
A64 can only operate using new timings, while the older ones can
operate in both modes.
In one case, we're forced to use it, in the other one it's a
policy. We should differentiate both cases.
For the A64's case, the limit is implied by not having any clk_delays.
FWIW, I'm really not a big fan of that either :)

Explicit is better than implicit.©
Post by Chen-Yu Tsai
But yes, I'll keep "needs_new_timings", and rename the new option to
"has_timing_switch" to make things clearer.
Great, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 06:43:01 UTC
Permalink
Raw Message
Now that we support the MMC controllers on the A83T SoC, we can enable
them on some boards.

Signed-off-by: Chen-Yu Tsai <***@csie.org>
---
arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts | 27 ++++++++++++++++++++++++
1 file changed, 27 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts b/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
index cff33454fc24..163ddf8868b5 100644
--- a/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts
@@ -83,6 +83,13 @@
};
};

+ reg_vcc3v3: vcc3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc3v3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
sound {
compatible = "simple-audio-card";
simple-audio-card,name = "On-board SPDIF";
@@ -102,6 +109,26 @@
};
};

+&mmc0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc0_pins>;
+ vmmc-supply = <&reg_vcc3v3>;
+ bus-width = <4>;
+ cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
+ cd-inverted;
+ status = "okay";
+};
+
+&mmc2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc2_8bit_emmc_pins>;
+ vmmc-supply = <&reg_vcc3v3>;
+ bus-width = <8>;
+ non-removable;
+ cap-mmc-hw-reset;
+ status = "okay";
+};
+
&spdif {
status = "okay";
};
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 06:42:59 UTC
Permalink
Raw Message
The A83T has 3 MMC controllers. The third one is a bit special, as it
supports a wider 8-bit bus, and a "new timing mode".

Signed-off-by: Chen-Yu Tsai <***@csie.org>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 57 +++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index beed05e10a3b..085312d0c521 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -182,6 +182,63 @@
#dma-cells = <1>;
};

+ mmc0: ***@1c0f000 {
+ compatible = "allwinner,sun7i-a20-mmc";
+ reg = <0x01c0f000 0x1000>;
+ clocks = <&ccu CLK_BUS_MMC0>,
+ <&ccu CLK_MMC0>,
+ <&ccu CLK_MMC0_OUTPUT>,
+ <&ccu CLK_MMC0_SAMPLE>;
+ clock-names = "ahb",
+ "mmc",
+ "output",
+ "sample";
+ resets = <&ccu RST_BUS_MMC0>;
+ reset-names = "ahb";
+ interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ mmc1: ***@1c10000 {
+ compatible = "allwinner,sun7i-a20-mmc";
+ reg = <0x01c10000 0x1000>;
+ clocks = <&ccu CLK_BUS_MMC1>,
+ <&ccu CLK_MMC1>,
+ <&ccu CLK_MMC1_OUTPUT>,
+ <&ccu CLK_MMC1_SAMPLE>;
+ clock-names = "ahb",
+ "mmc",
+ "output",
+ "sample";
+ resets = <&ccu RST_BUS_MMC1>;
+ reset-names = "ahb";
+ interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ mmc2: ***@1c11000 {
+ compatible = "allwinner,sun8i-a83t-emmc";
+ reg = <0x01c11000 0x1000>;
+ clocks = <&ccu CLK_BUS_MMC2>,
+ <&ccu CLK_MMC2>,
+ <&ccu CLK_MMC2_OUTPUT>,
+ <&ccu CLK_MMC2_SAMPLE>;
+ clock-names = "ahb",
+ "mmc",
+ "output",
+ "sample";
+ resets = <&ccu RST_BUS_MMC2>;
+ reset-names = "ahb";
+ interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
ccu: ***@1c20000 {
compatible = "allwinner,sun8i-a83t-ccu";
reg = <0x01c20000 0x400>;
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Maxime Ripard
2017-07-17 09:22:09 UTC
Permalink
Raw Message
Hi,
Post by Chen-Yu Tsai
The A83T has 3 MMC controllers. The third one is a bit special, as it
supports a wider 8-bit bus, and a "new timing mode".
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 57 +++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index beed05e10a3b..085312d0c521 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -182,6 +182,63 @@
#dma-cells = <1>;
};
+ compatible = "allwinner,sun7i-a20-mmc";
Ideally, we should have an A83T compatible here too, just in case.

Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-14 06:43:02 UTC
Permalink
Raw Message
The H8 homlet has a micro-SD card slot connected to mmc0,
and onboard eMMC from FORESEE, connected to mmc2.

Signed-off-by: Chen-Yu Tsai <***@csie.org>
---
.../boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts b/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
index aecdeeb368ed..7afbaa4eea8d 100644
--- a/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-allwinner-h8homlet-v2.dts
@@ -43,6 +43,7 @@

/dts-v1/;
#include "sun8i-a83t.dtsi"
+#include "sunxi-common-regulators.dtsi"

/ {
model = "Allwinner A83T H8Homlet Proto Dev Board v2.0";
@@ -57,6 +58,26 @@
};
};

+&mmc0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc0_pins>;
+ vmmc-supply = <&reg_vcc3v0>;
+ cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
+ bus-width = <4>;
+ cd-inverted;
+ status = "okay";
+};
+
+&mmc2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc2_8bit_emmc_pins>;
+ vmmc-supply = <&reg_vcc3v0>;
+ bus-width = <8>;
+ non-removable;
+ cap-mmc-hw-reset;
+ status = "okay";
+};
+
&uart0 {
pinctrl-names = "default";
pinctrl-0 = <&uart0_pb_pins>;
--
2.13.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Maxime Ripard
2017-07-17 09:14:02 UTC
Permalink
Raw Message
Hi,
Post by Chen-Yu Tsai
The MMC2 clock supports a new timing mode. When the new mode is active,
the output clock rate is halved.
This patch sets the feature flag for the new timing mode, and adds
a pre-divider based on the mode bit.
---
drivers/clk/sunxi-ng/ccu-sun8i-a83t.c | 38 +++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
index 947f9f6e05d2..ee6688e9b361 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
@@ -418,14 +418,36 @@ static SUNXI_CCU_PHASE(mmc1_sample_clk, "mmc1-sample", "mmc1",
static SUNXI_CCU_PHASE(mmc1_output_clk, "mmc1-output", "mmc1",
0x08c, 8, 3, 0);
-/* TODO Support MMC2 clock's new timing mode. */
-static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mod0_default_parents,
- 0x090,
- 0, 4, /* M */
- 16, 2, /* P */
- 24, 2, /* mux */
- BIT(31), /* gate */
- 0);
+/*
+ * MMC2 supports both old and new timing modes. When the new timing
+ * mode is active, the output clock rate is halved by two. Here we
+ * treat it as a variable pre-divider. Note that the pre-divider is
+ * _not_ included in the possible factors during a set clock rate
+ * operation. It is only read out.
+ */
+static const struct ccu_mux_var_prediv mmc2_new_timing_predivs[] = {
+ { .index = 0, .shift = 30, .width = 1 },
+ { .index = 1, .shift = 30, .width = 1 },
+};
+static struct ccu_mp mmc2_clk = {
+ .enable = BIT(31),
+ .m = _SUNXI_CCU_DIV(0, 4),
+ .p = _SUNXI_CCU_DIV(16, 2),
+ .mux = {
+ .shift = 24,
+ .width = 2,
+ .var_predivs = mmc2_new_timing_predivs,
+ .n_var_predivs = ARRAY_SIZE(mmc2_new_timing_predivs),
+ },
+ .common = {
+ .reg = 0x090,
+ .features = CCU_FEATURE_MMC_TIMING_SWITCH,
+ .hw.init = CLK_HW_INIT_PARENTS("mmc2",
+ mod0_default_parents,
+ &ccu_mp_ops,
+ CLK_GET_RATE_NOCACHE),
+ },
+};
Treating the new bit seems a bit of a hack to me. It only works
because we're not evaluating the various pre-dividers during a
determine_rate (and set_rate), but it might change in the future, and
we will break all our eMMC controllers then.

Since they're quite special, I was thinking about creating a new MMC
clock type? We're going to use it on a number of SoCs, and we'll be
able to model it properly, without crippling the regular and generic
MP clocks.

Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai
2017-07-17 10:12:35 UTC
Permalink
Raw Message
On Mon, Jul 17, 2017 at 5:14 PM, Maxime Ripard
Post by Maxime Ripard
Hi,
Post by Chen-Yu Tsai
The MMC2 clock supports a new timing mode. When the new mode is active,
the output clock rate is halved.
This patch sets the feature flag for the new timing mode, and adds
a pre-divider based on the mode bit.
---
drivers/clk/sunxi-ng/ccu-sun8i-a83t.c | 38 +++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
index 947f9f6e05d2..ee6688e9b361 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
@@ -418,14 +418,36 @@ static SUNXI_CCU_PHASE(mmc1_sample_clk, "mmc1-sample", "mmc1",
static SUNXI_CCU_PHASE(mmc1_output_clk, "mmc1-output", "mmc1",
0x08c, 8, 3, 0);
-/* TODO Support MMC2 clock's new timing mode. */
-static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mod0_default_parents,
- 0x090,
- 0, 4, /* M */
- 16, 2, /* P */
- 24, 2, /* mux */
- BIT(31), /* gate */
- 0);
+/*
+ * MMC2 supports both old and new timing modes. When the new timing
+ * mode is active, the output clock rate is halved by two. Here we
+ * treat it as a variable pre-divider. Note that the pre-divider is
+ * _not_ included in the possible factors during a set clock rate
+ * operation. It is only read out.
+ */
+static const struct ccu_mux_var_prediv mmc2_new_timing_predivs[] = {
+ { .index = 0, .shift = 30, .width = 1 },
+ { .index = 1, .shift = 30, .width = 1 },
+};
+static struct ccu_mp mmc2_clk = {
+ .enable = BIT(31),
+ .m = _SUNXI_CCU_DIV(0, 4),
+ .p = _SUNXI_CCU_DIV(16, 2),
+ .mux = {
+ .shift = 24,
+ .width = 2,
+ .var_predivs = mmc2_new_timing_predivs,
+ .n_var_predivs = ARRAY_SIZE(mmc2_new_timing_predivs),
+ },
+ .common = {
+ .reg = 0x090,
+ .features = CCU_FEATURE_MMC_TIMING_SWITCH,
+ .hw.init = CLK_HW_INIT_PARENTS("mmc2",
+ mod0_default_parents,
+ &ccu_mp_ops,
+ CLK_GET_RATE_NOCACHE),
+ },
+};
Treating the new bit seems a bit of a hack to me. It only works
because we're not evaluating the various pre-dividers during a
determine_rate (and set_rate), but it might change in the future, and
we will break all our eMMC controllers then.
Since they're quite special, I was thinking about creating a new MMC
clock type? We're going to use it on a number of SoCs, and we'll be
able to model it properly, without crippling the regular and generic
MP clocks.
Yes that should be doable. I could put them in the same file and
reuse all the existing MP clocks stuff by wrapping them in new
functions that check the timing mode bit.

Would that work for you?

ChenYu
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Maxime Ripard
2017-07-18 14:47:58 UTC
Permalink
Raw Message
Post by Chen-Yu Tsai
On Mon, Jul 17, 2017 at 5:14 PM, Maxime Ripard
Post by Maxime Ripard
Hi,
Post by Chen-Yu Tsai
The MMC2 clock supports a new timing mode. When the new mode is active,
the output clock rate is halved.
This patch sets the feature flag for the new timing mode, and adds
a pre-divider based on the mode bit.
---
drivers/clk/sunxi-ng/ccu-sun8i-a83t.c | 38 +++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
index 947f9f6e05d2..ee6688e9b361 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
@@ -418,14 +418,36 @@ static SUNXI_CCU_PHASE(mmc1_sample_clk, "mmc1-sample", "mmc1",
static SUNXI_CCU_PHASE(mmc1_output_clk, "mmc1-output", "mmc1",
0x08c, 8, 3, 0);
-/* TODO Support MMC2 clock's new timing mode. */
-static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mod0_default_parents,
- 0x090,
- 0, 4, /* M */
- 16, 2, /* P */
- 24, 2, /* mux */
- BIT(31), /* gate */
- 0);
+/*
+ * MMC2 supports both old and new timing modes. When the new timing
+ * mode is active, the output clock rate is halved by two. Here we
+ * treat it as a variable pre-divider. Note that the pre-divider is
+ * _not_ included in the possible factors during a set clock rate
+ * operation. It is only read out.
+ */
+static const struct ccu_mux_var_prediv mmc2_new_timing_predivs[] = {
+ { .index = 0, .shift = 30, .width = 1 },
+ { .index = 1, .shift = 30, .width = 1 },
+};
+static struct ccu_mp mmc2_clk = {
+ .enable = BIT(31),
+ .m = _SUNXI_CCU_DIV(0, 4),
+ .p = _SUNXI_CCU_DIV(16, 2),
+ .mux = {
+ .shift = 24,
+ .width = 2,
+ .var_predivs = mmc2_new_timing_predivs,
+ .n_var_predivs = ARRAY_SIZE(mmc2_new_timing_predivs),
+ },
+ .common = {
+ .reg = 0x090,
+ .features = CCU_FEATURE_MMC_TIMING_SWITCH,
+ .hw.init = CLK_HW_INIT_PARENTS("mmc2",
+ mod0_default_parents,
+ &ccu_mp_ops,
+ CLK_GET_RATE_NOCACHE),
+ },
+};
Treating the new bit seems a bit of a hack to me. It only works
because we're not evaluating the various pre-dividers during a
determine_rate (and set_rate), but it might change in the future, and
we will break all our eMMC controllers then.
Since they're quite special, I was thinking about creating a new MMC
clock type? We're going to use it on a number of SoCs, and we'll be
able to model it properly, without crippling the regular and generic
MP clocks.
Yes that should be doable. I could put them in the same file and
reuse all the existing MP clocks stuff by wrapping them in new
functions that check the timing mode bit.
Would that work for you?
Yep, it does.

Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
kbuild test robot
2017-07-17 13:10:47 UTC
Permalink
Raw Message
Hi Chen-Yu,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.13-rc1]
[cannot apply to robh/for-next clk/clk-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Chen-Yu-Tsai/ARM-sun8i-a83t-Add-support-for-MMC-controllers/20170715-071008
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
ERROR: "sunxi_ccu_set_mmc_timing_mode" [drivers/mmc/host/sunxi-mmc.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+***@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Loading...