openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] ASPEED SD/eMMC controller clock configuration
@ 2021-09-22 10:31 Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 01/10] clk: aspeed: ast2600: Porting sdhci clock source Chin-Ting Kuo
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-22 10:31 UTC (permalink / raw)
  To: robh+dt, joel, mturquette, sboyd, adrian.hunter, linux-aspeed,
	openbmc, linux-mmc, devicetree, linux-arm-kernel, linux-kernel,
	linux-clk, andrew
  Cc: BMC-SW, steven_lee

This patch series aims to configure SD and eMMC controllers' clock
frequency and clock phase parameters. The main modification is the
clock phase calculation method which has been checked with the HW IP
designer. Also, the clock source detection method is updated for
AST2600-A2/A3. This patch series has been verified on AST2600-A3 EVB.

Chin-Ting Kuo (10):
  clk: aspeed: ast2600: Porting sdhci clock source
  sdhci: aspeed: Add SDR50 support
  dts: aspeed: ast2600: Support SDR50 for SD device
  mmc: Add invert flag for clock phase signedness
  mmc: aspeed: Adjust delay taps calculation method
  arm: dts: aspeed: Change eMMC device compatible
  arm: dts: aspeed: Adjust clock phase parameter
  arm: dts: ibm: Adjust clock phase parameter
  dt-bindings: mmc: aspeed: Add max-tap-delay property
  dt-bindings: mmc: aspeed: Add a new compatible string

 .../devicetree/bindings/mmc/aspeed,sdhci.yaml |   4 +
 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts   |   8 ++
 arch/arm/boot/dts/aspeed-ast2600-evb.dts      |  11 +-
 arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts  |   3 +-
 arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts  |   3 +-
 arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts   |   3 +-
 arch/arm/boot/dts/aspeed-g6.dtsi              |   2 +-
 drivers/clk/clk-ast2600.c                     |  69 ++++++++--
 drivers/mmc/core/host.c                       |  10 +-
 drivers/mmc/host/sdhci-of-aspeed.c            | 123 ++++++++++++++----
 include/linux/mmc/host.h                      |   2 +
 11 files changed, 193 insertions(+), 45 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 01/10] clk: aspeed: ast2600: Porting sdhci clock source
  2021-09-22 10:31 [PATCH 00/10] ASPEED SD/eMMC controller clock configuration Chin-Ting Kuo
@ 2021-09-22 10:31 ` Chin-Ting Kuo
  2021-09-23  0:02   ` Joel Stanley
  2021-09-22 10:31 ` [PATCH 02/10] sdhci: aspeed: Add SDR50 support Chin-Ting Kuo
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-22 10:31 UTC (permalink / raw)
  To: robh+dt, joel, mturquette, sboyd, adrian.hunter, linux-aspeed,
	openbmc, linux-mmc, devicetree, linux-arm-kernel, linux-kernel,
	linux-clk, andrew
  Cc: BMC-SW, steven_lee

- There are two clock sources used to generate
  SD/SDIO clock, APLL clock and HCLK (200MHz).
  User can select which clock source should be used
  by configuring SCU310[8].
- The SD/SDIO clock divider selection table SCU310[30:28]
  is different between AST2600-A1 and AST2600-A2/A3.
  For AST2600-A1, 200MHz SD/SDIO clock cannot be
  gotten by the dividers in SCU310[30:28] if APLL
  is not the multiple of 200MHz and HCLK is 200MHz.
  For AST2600-A2/A3, a new divider, "1", is added and
  200MHz SD/SDIO clock can be obtained by adopting HCLK
  as clock source and setting SCU310[30:28] to 3b'111.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 drivers/clk/clk-ast2600.c | 69 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 61 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
index bc3be5f3eae1..a6778c18274a 100644
--- a/drivers/clk/clk-ast2600.c
+++ b/drivers/clk/clk-ast2600.c
@@ -168,6 +168,30 @@ static const struct clk_div_table ast2600_div_table[] = {
 	{ 0 }
 };
 
+static const struct clk_div_table ast2600_sd_div_a1_table[] = {
+	{ 0x0, 2 },
+	{ 0x1, 4 },
+	{ 0x2, 6 },
+	{ 0x3, 8 },
+	{ 0x4, 10 },
+	{ 0x5, 12 },
+	{ 0x6, 14 },
+	{ 0x7, 16 },
+	{ 0 }
+};
+
+static const struct clk_div_table ast2600_sd_div_a2_table[] = {
+	{ 0x0, 2 },
+	{ 0x1, 4 },
+	{ 0x2, 6 },
+	{ 0x3, 8 },
+	{ 0x4, 10 },
+	{ 0x5, 12 },
+	{ 0x6, 14 },
+	{ 0x7, 1 },
+	{ 0 }
+};
+
 /* For hpll/dpll/epll/mpll */
 static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)
 {
@@ -424,6 +448,11 @@ static const char *const emmc_extclk_parent_names[] = {
 	"mpll",
 };
 
+static const char *const sd_extclk_parent_names[] = {
+	"hclk",
+	"apll",
+};
+
 static const char * const vclk_parent_names[] = {
 	"dpll",
 	"d1pll",
@@ -523,18 +552,42 @@ static int aspeed_g6_clk_probe(struct platform_device *pdev)
 		return PTR_ERR(hw);
 	aspeed_g6_clk_data->hws[ASPEED_CLK_EMMC] = hw;
 
-	/* SD/SDIO clock divider and gate */
-	hw = clk_hw_register_gate(dev, "sd_extclk_gate", "hpll", 0,
-			scu_g6_base + ASPEED_G6_CLK_SELECTION4, 31, 0,
-			&aspeed_g6_clk_lock);
+	clk_hw_register_fixed_rate(NULL, "hclk", NULL, 0, 200000000);
+
+	regmap_read(map, 0x310, &val);
+	hw = clk_hw_register_mux(dev, "sd_extclk_mux",
+				 sd_extclk_parent_names,
+				 ARRAY_SIZE(sd_extclk_parent_names), 0,
+				 scu_g6_base + ASPEED_G6_CLK_SELECTION4, 8, 1,
+				 0, &aspeed_g6_clk_lock);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
-	hw = clk_hw_register_divider_table(dev, "sd_extclk", "sd_extclk_gate",
-			0, scu_g6_base + ASPEED_G6_CLK_SELECTION4, 28, 3, 0,
-			ast2600_div_table,
-			&aspeed_g6_clk_lock);
+
+	hw = clk_hw_register_gate(dev, "sd_extclk_gate", "sd_extclk_mux",
+				  0, scu_g6_base + ASPEED_G6_CLK_SELECTION4,
+				  31, 0, &aspeed_g6_clk_lock);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
+
+	regmap_read(map, 0x14, &val);
+	/* AST2600-A2/A3 clock divisor is different from AST2600-A1 */
+	if (((val & GENMASK(23, 16)) >> 16) >= 2) {
+		/* AST2600-A2/A3 */
+		hw = clk_hw_register_divider_table(dev, "sd_extclk", "sd_extclk_gate",
+					0, scu_g6_base + ASPEED_G6_CLK_SELECTION4, 28, 3, 0,
+					ast2600_sd_div_a2_table,
+					&aspeed_g6_clk_lock);
+		if (IS_ERR(hw))
+			return PTR_ERR(hw);
+	} else {
+		/* AST2600-A1 */
+		hw = clk_hw_register_divider_table(dev, "sd_extclk", "sd_extclk_gate",
+					0, scu_g6_base + ASPEED_G6_CLK_SELECTION4, 28, 3, 0,
+					ast2600_sd_div_a1_table,
+					&aspeed_g6_clk_lock);
+		if (IS_ERR(hw))
+			return PTR_ERR(hw);
+	}
 	aspeed_g6_clk_data->hws[ASPEED_CLK_SDIO] = hw;
 
 	/* MAC1/2 RMII 50MHz RCLK */
-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 02/10] sdhci: aspeed: Add SDR50 support
  2021-09-22 10:31 [PATCH 00/10] ASPEED SD/eMMC controller clock configuration Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 01/10] clk: aspeed: ast2600: Porting sdhci clock source Chin-Ting Kuo
@ 2021-09-22 10:31 ` Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 03/10] dts: aspeed: ast2600: Support SDR50 for SD device Chin-Ting Kuo
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-22 10:31 UTC (permalink / raw)
  To: robh+dt, joel, mturquette, sboyd, adrian.hunter, linux-aspeed,
	openbmc, linux-mmc, devicetree, linux-arm-kernel, linux-kernel,
	linux-clk, andrew
  Cc: BMC-SW, steven_lee

From the analog waveform analysis result, SD/SDIO controller
of AST2600 cannot always work well with 200MHz. The upper bound
stable frequency for SD/SDIO controller is 100MHz. Thus, SDR50
supported bit, instead of SDR104, in capability 2 register
should be set in advance.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 drivers/mmc/host/sdhci-of-aspeed.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index 6e4e132903a6..c6eaeb02e3f9 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -35,6 +35,8 @@
 #define ASPEED_SDC_CAP1_1_8V           (0 * 32 + 26)
 /* SDIO{14,24} */
 #define ASPEED_SDC_CAP2_SDR104         (1 * 32 + 1)
+/* SDIO{14,24} */
+#define ASPEED_SDC_CAP2_SDR50          (1 * 32 + 0)
 
 struct aspeed_sdc {
 	struct clk *clk;
@@ -410,11 +412,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-sdr50") ||
 	    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-sdr50")) {
+		aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP2_SDR50,
+					       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);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 03/10] dts: aspeed: ast2600: Support SDR50 for SD device
  2021-09-22 10:31 [PATCH 00/10] ASPEED SD/eMMC controller clock configuration Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 01/10] clk: aspeed: ast2600: Porting sdhci clock source Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 02/10] sdhci: aspeed: Add SDR50 support Chin-Ting Kuo
@ 2021-09-22 10:31 ` Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 04/10] mmc: Add invert flag for clock phase signedness Chin-Ting Kuo
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-22 10:31 UTC (permalink / raw)
  To: robh+dt, joel, mturquette, sboyd, adrian.hunter, linux-aspeed,
	openbmc, linux-mmc, devicetree, linux-arm-kernel, linux-kernel,
	linux-clk, andrew
  Cc: BMC-SW, steven_lee

The maximum frequency for SD controller on AST2600 EVB is
100MHz. In order to achieve 100MHz, sd-uhs-sdr50 property
should be added and the driver will set the SDR50 supported
bit in capability 2 register during probing stage.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-ast2600-evb.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index b7eb552640cb..4551dba499c2 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -280,6 +280,7 @@
 &sdhci0 {
 	status = "okay";
 	bus-width = <4>;
+	sd-uhs-sdr50;
 	max-frequency = <100000000>;
 	sdhci-drive-type = /bits/ 8 <3>;
 	sdhci-caps-mask = <0x7 0x0>;
@@ -292,6 +293,7 @@
 &sdhci1 {
 	status = "okay";
 	bus-width = <4>;
+	sd-uhs-sdr50;
 	max-frequency = <100000000>;
 	sdhci-drive-type = /bits/ 8 <3>;
 	sdhci-caps-mask = <0x7 0x0>;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 04/10] mmc: Add invert flag for clock phase signedness
  2021-09-22 10:31 [PATCH 00/10] ASPEED SD/eMMC controller clock configuration Chin-Ting Kuo
                   ` (2 preceding siblings ...)
  2021-09-22 10:31 ` [PATCH 03/10] dts: aspeed: ast2600: Support SDR50 for SD device Chin-Ting Kuo
@ 2021-09-22 10:31 ` Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 05/10] mmc: aspeed: Adjust delay taps calculation method Chin-Ting Kuo
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-22 10:31 UTC (permalink / raw)
  To: robh+dt, joel, mturquette, sboyd, adrian.hunter, linux-aspeed,
	openbmc, linux-mmc, devicetree, linux-arm-kernel, linux-kernel,
	linux-clk, andrew
  Cc: BMC-SW, steven_lee

The clock phase degree may be between -360 to 360.
If the data signal is leading to the clock, the signedness
of clock phase is postive, otherwise, the signedness
is negative.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 drivers/mmc/core/host.c  | 10 ++++++----
 include/linux/mmc/host.h |  2 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index d4683b1d263f..c2de7cbc7838 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -214,14 +214,16 @@ static void mmc_retune_timer(struct timer_list *t)
 static void mmc_of_parse_timing_phase(struct device *dev, const char *prop,
 				      struct mmc_clk_phase *phase)
 {
-	int degrees[2] = {0};
+	int degrees[4] = {0};
 	int rc;
 
-	rc = device_property_read_u32_array(dev, prop, degrees, 2);
+	rc = device_property_read_u32_array(dev, prop, degrees, 4);
 	phase->valid = !rc;
 	if (phase->valid) {
-		phase->in_deg = degrees[0];
-		phase->out_deg = degrees[1];
+		phase->inv_in_deg = degrees[0] ? true : false;
+		phase->in_deg = degrees[1];
+		phase->inv_out_deg = degrees[2] ? true : false;
+		phase->out_deg = degrees[3];
 	}
 }
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 0c0c9a0fdf57..3c13010683e0 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -82,7 +82,9 @@ struct mmc_ios {
 
 struct mmc_clk_phase {
 	bool valid;
+	bool inv_in_deg;
 	u16 in_deg;
+	bool inv_out_deg;
 	u16 out_deg;
 };
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 05/10] mmc: aspeed: Adjust delay taps calculation method
  2021-09-22 10:31 [PATCH 00/10] ASPEED SD/eMMC controller clock configuration Chin-Ting Kuo
                   ` (3 preceding siblings ...)
  2021-09-22 10:31 ` [PATCH 04/10] mmc: Add invert flag for clock phase signedness Chin-Ting Kuo
@ 2021-09-22 10:31 ` Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 06/10] arm: dts: aspeed: Change eMMC device compatible Chin-Ting Kuo
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-22 10:31 UTC (permalink / raw)
  To: robh+dt, joel, mturquette, sboyd, adrian.hunter, linux-aspeed,
	openbmc, linux-mmc, devicetree, linux-arm-kernel, linux-kernel,
	linux-clk, andrew
  Cc: BMC-SW, steven_lee

- The maximum tap delay may be slightly different on
  different platforms. It may also be different due to
  different SoC processes or different manufacturers.
  Thus, the maximum tap delay should be gotten from the
  device tree through max-tap-delay property.
- The delay time for each tap is an absolute value which
  is independent of clock frequency. But, in order to combine
  this principle with "phase" concept, clock frequency is took
  into consideration during calculating delay taps.
- The delay cell of eMMC device is non-uniform.
  The time period of the first tap is two times of others.
- The clock phase degree range is from -360 to 360.
  But, if the clock phase signedness is negative, clock signal
  is output from the falling edge first by default and thus, clock
  signal is leading to data signal by 90 degrees at least.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 drivers/mmc/host/sdhci-of-aspeed.c | 115 ++++++++++++++++++++++-------
 1 file changed, 89 insertions(+), 26 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index c6eaeb02e3f9..739c9503a5ed 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -44,6 +44,7 @@ struct aspeed_sdc {
 
 	spinlock_t lock;
 	void __iomem *regs;
+	u32 max_tap_delay_ps;
 };
 
 struct aspeed_sdhci_tap_param {
@@ -63,6 +64,7 @@ struct aspeed_sdhci_tap_desc {
 struct aspeed_sdhci_phase_desc {
 	struct aspeed_sdhci_tap_desc in;
 	struct aspeed_sdhci_tap_desc out;
+	u32 nr_taps;
 };
 
 struct aspeed_sdhci_pdata {
@@ -158,43 +160,60 @@ aspeed_sdc_set_phase_taps(struct aspeed_sdc *sdc,
 }
 
 #define PICOSECONDS_PER_SECOND		1000000000000ULL
-#define ASPEED_SDHCI_NR_TAPS		15
-/* Measured value with *handwave* environmentals and static loading */
-#define ASPEED_SDHCI_MAX_TAP_DELAY_PS	1253
+#define ASPEED_SDHCI_MAX_TAPS		15
+
 static int aspeed_sdhci_phase_to_tap(struct device *dev, unsigned long rate_hz,
-				     int phase_deg)
+				     bool invert, int phase_deg, u32 nr_taps)
 {
 	u64 phase_period_ps;
 	u64 prop_delay_ps;
 	u64 clk_period_ps;
-	unsigned int tap;
-	u8 inverted;
+	u32 tap = 0;
+	struct aspeed_sdc *sdc = dev_get_drvdata(dev->parent);
 
-	phase_deg %= 360;
+	if (sdc->max_tap_delay_ps == 0)
+		return 0;
 
-	if (phase_deg >= 180) {
-		inverted = ASPEED_SDHCI_TAP_PARAM_INVERT_CLK;
-		phase_deg -= 180;
-		dev_dbg(dev,
-			"Inverting clock to reduce phase correction from %d to %d degrees\n",
-			phase_deg + 180, phase_deg);
-	} else {
-		inverted = 0;
+	prop_delay_ps = sdc->max_tap_delay_ps / nr_taps;
+	clk_period_ps = div_u64(PICOSECONDS_PER_SECOND, (u64)rate_hz);
+
+	/*
+	 * For ast2600, if clock phase degree is negative, clock signal is
+	 * output from falling edge first by default. Namely, clock signal
+	 * is leading to data signal by 90 degrees at least.
+	 */
+	if (invert) {
+		if (phase_deg >= 90)
+			phase_deg -= 90;
+		else
+			phase_deg = 0;
 	}
 
-	prop_delay_ps = ASPEED_SDHCI_MAX_TAP_DELAY_PS / ASPEED_SDHCI_NR_TAPS;
-	clk_period_ps = div_u64(PICOSECONDS_PER_SECOND, (u64)rate_hz);
 	phase_period_ps = div_u64((u64)phase_deg * clk_period_ps, 360ULL);
 
-	tap = div_u64(phase_period_ps, prop_delay_ps);
-	if (tap > ASPEED_SDHCI_NR_TAPS) {
+	/*
+	 * The delay cell is non-uniform for eMMC controller.
+	 * The time period of the first tap is two times of others.
+	 */
+	if (nr_taps == 16 && phase_period_ps > prop_delay_ps * 2) {
+		phase_period_ps -= prop_delay_ps * 2;
+		tap++;
+	}
+
+	tap += div_u64(phase_period_ps, prop_delay_ps);
+	if (tap > ASPEED_SDHCI_MAX_TAPS) {
 		dev_dbg(dev,
 			 "Requested out of range phase tap %d for %d degrees of phase compensation at %luHz, clamping to tap %d\n",
-			 tap, phase_deg, rate_hz, ASPEED_SDHCI_NR_TAPS);
-		tap = ASPEED_SDHCI_NR_TAPS;
+			 tap, phase_deg, rate_hz, ASPEED_SDHCI_MAX_TAPS);
+		tap = ASPEED_SDHCI_MAX_TAPS;
 	}
 
-	return inverted | tap;
+	if (invert) {
+		dev_info(dev, "invert the clock\n");
+		tap |= ASPEED_SDHCI_TAP_PARAM_INVERT_CLK;
+	}
+
+	return tap;
 }
 
 static void
@@ -202,13 +221,19 @@ aspeed_sdhci_phases_to_taps(struct device *dev, unsigned long rate,
 			    const struct mmc_clk_phase *phases,
 			    struct aspeed_sdhci_tap_param *taps)
 {
+	struct sdhci_host *host = dev->driver_data;
+	struct aspeed_sdhci *sdhci;
+
+	sdhci = sdhci_pltfm_priv(sdhci_priv(host));
 	taps->valid = phases->valid;
 
 	if (!phases->valid)
 		return;
 
-	taps->in = aspeed_sdhci_phase_to_tap(dev, rate, phases->in_deg);
-	taps->out = aspeed_sdhci_phase_to_tap(dev, rate, phases->out_deg);
+	taps->in = aspeed_sdhci_phase_to_tap(dev, rate, phases->inv_in_deg,
+				phases->in_deg, sdhci->phase_desc->nr_taps);
+	taps->out = aspeed_sdhci_phase_to_tap(dev, rate, phases->inv_out_deg,
+				phases->out_deg, sdhci->phase_desc->nr_taps);
 }
 
 static void
@@ -230,8 +255,8 @@ aspeed_sdhci_configure_phase(struct sdhci_host *host, unsigned long rate)
 	aspeed_sdc_set_phase_taps(sdhci->parent, sdhci->phase_desc, taps);
 	dev_dbg(dev,
 		"Using taps [%d, %d] for [%d, %d] degrees of phase correction at %luHz (%d)\n",
-		taps->in & ASPEED_SDHCI_NR_TAPS,
-		taps->out & ASPEED_SDHCI_NR_TAPS,
+		taps->in & ASPEED_SDHCI_MAX_TAPS,
+		taps->out & ASPEED_SDHCI_MAX_TAPS,
 		params->in_deg, params->out_deg, rate, host->timing);
 }
 
@@ -493,6 +518,7 @@ static const struct aspeed_sdhci_phase_desc ast2600_sdhci_phase[] = {
 			.enable_mask = ASPEED_SDC_S0_PHASE_OUT_EN,
 			.enable_value = 3,
 		},
+		.nr_taps = 15,
 	},
 	/* SDHCI/Slot 1 */
 	[1] = {
@@ -506,6 +532,31 @@ static const struct aspeed_sdhci_phase_desc ast2600_sdhci_phase[] = {
 			.enable_mask = ASPEED_SDC_S1_PHASE_OUT_EN,
 			.enable_value = 3,
 		},
+		.nr_taps = 15,
+	},
+};
+
+static const struct aspeed_sdhci_phase_desc ast2600_emmc_phase[] = {
+	/* eMMC slot 0 */
+	[0] = {
+		.in = {
+			.tap_mask = ASPEED_SDC_S0_PHASE_IN,
+			.enable_mask = ASPEED_SDC_S0_PHASE_IN_EN,
+			.enable_value = 1,
+		},
+		.out = {
+			.tap_mask = ASPEED_SDC_S0_PHASE_OUT,
+			.enable_mask = ASPEED_SDC_S0_PHASE_OUT_EN,
+			.enable_value = 3,
+		},
+
+		/*
+		 * There are 15 taps recorded in AST2600 datasheet.
+		 * But, actually, the time period of the first tap
+		 * is two times of others. Thus, 16 tap is used to
+		 * emulate this situation.
+		 */
+		.nr_taps = 16,
 	},
 };
 
@@ -515,10 +566,17 @@ static const struct aspeed_sdhci_pdata ast2600_sdhci_pdata = {
 	.nr_phase_descs = ARRAY_SIZE(ast2600_sdhci_phase),
 };
 
+static const struct aspeed_sdhci_pdata ast2600_emmc_pdata = {
+	.clk_div_start = 1,
+	.phase_desc = ast2600_emmc_phase,
+	.nr_phase_descs = ARRAY_SIZE(ast2600_emmc_phase),
+};
+
 static const struct of_device_id aspeed_sdhci_of_match[] = {
 	{ .compatible = "aspeed,ast2400-sdhci", .data = &ast2400_sdhci_pdata, },
 	{ .compatible = "aspeed,ast2500-sdhci", .data = &ast2400_sdhci_pdata, },
 	{ .compatible = "aspeed,ast2600-sdhci", .data = &ast2600_sdhci_pdata, },
+	{ .compatible = "aspeed,ast2600-emmc", .data = &ast2600_emmc_pdata, },
 	{ }
 };
 
@@ -562,6 +620,11 @@ static int aspeed_sdc_probe(struct platform_device *pdev)
 		goto err_clk;
 	}
 
+	ret = of_property_read_u32(pdev->dev.of_node, "max-tap-delay",
+			&sdc->max_tap_delay_ps);
+	if (ret)
+		sdc->max_tap_delay_ps = 0;
+
 	dev_set_drvdata(&pdev->dev, sdc);
 
 	parent = pdev->dev.of_node;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 06/10] arm: dts: aspeed: Change eMMC device compatible
  2021-09-22 10:31 [PATCH 00/10] ASPEED SD/eMMC controller clock configuration Chin-Ting Kuo
                   ` (4 preceding siblings ...)
  2021-09-22 10:31 ` [PATCH 05/10] mmc: aspeed: Adjust delay taps calculation method Chin-Ting Kuo
@ 2021-09-22 10:31 ` Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 07/10] arm: dts: aspeed: Adjust clock phase parameter Chin-Ting Kuo
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-22 10:31 UTC (permalink / raw)
  To: robh+dt, joel, mturquette, sboyd, adrian.hunter, linux-aspeed,
	openbmc, linux-mmc, devicetree, linux-arm-kernel, linux-kernel,
	linux-clk, andrew
  Cc: BMC-SW, steven_lee

Since the eMMC device's delay parameters are different from
the SD's, a new compatible should be used to distinguish
between eMMC and SD device.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-g6.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index 1b47be1704f8..d083de1d6567 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -597,7 +597,7 @@
 				status = "disabled";
 
 				emmc: sdhci@1e750100 {
-					compatible = "aspeed,ast2600-sdhci";
+					compatible = "aspeed,ast2600-emmc";
 					reg = <0x100 0x100>;
 					sdhci,auto-cmd12;
 					interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 07/10] arm: dts: aspeed: Adjust clock phase parameter
  2021-09-22 10:31 [PATCH 00/10] ASPEED SD/eMMC controller clock configuration Chin-Ting Kuo
                   ` (5 preceding siblings ...)
  2021-09-22 10:31 ` [PATCH 06/10] arm: dts: aspeed: Change eMMC device compatible Chin-Ting Kuo
@ 2021-09-22 10:31 ` Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 08/10] arm: dts: ibm: " Chin-Ting Kuo
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-22 10:31 UTC (permalink / raw)
  To: robh+dt, joel, mturquette, sboyd, adrian.hunter, linux-aspeed,
	openbmc, linux-mmc, devicetree, linux-arm-kernel, linux-kernel,
	linux-clk, andrew
  Cc: BMC-SW, steven_lee

Change clock phase degree for AST2600 EVB.
These parameter has been verified with 100MHz
clock frequency for eMMC and SD controllers.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts | 8 ++++++++
 arch/arm/boot/dts/aspeed-ast2600-evb.dts    | 9 ++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts b/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts
index dd7148060c4a..2d83617dc436 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts
@@ -13,3 +13,11 @@
 };
 
 /delete-node/ &sdc;
+
+&emmc_controller {
+	max-tap-delay = <706>;
+};
+
+&emmc {
+	clk-phase-mmc-hs200 = <0 13>, <1 103>;
+};
diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index 4551dba499c2..f728b9d9b4cf 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -143,13 +143,15 @@
 
 &emmc_controller {
 	status = "okay";
+	/* Measured value with *handwave* environmentals and static loading */
+	max-tap-delay = <736>;
 };
 
 &emmc {
 	non-removable;
 	bus-width = <4>;
 	max-frequency = <100000000>;
-	clk-phase-mmc-hs200 = <9>, <225>;
+	clk-phase-mmc-hs200 = <0 27>, <1 95>;
 };
 
 &rtc {
@@ -260,6 +262,7 @@
 
 &sdc {
 	status = "okay";
+	max-tap-delay = <9000>;
 };
 
 /*
@@ -287,7 +290,7 @@
 	sdhci,wp-inverted;
 	vmmc-supply = <&vcc_sdhci0>;
 	vqmmc-supply = <&vccq_sdhci0>;
-	clk-phase-sd-hs = <7>, <200>;
+	clk-phase-uhs-sdr50 = <0 130>, <0 238>;
 };
 
 &sdhci1 {
@@ -300,5 +303,5 @@
 	sdhci,wp-inverted;
 	vmmc-supply = <&vcc_sdhci1>;
 	vqmmc-supply = <&vccq_sdhci1>;
-	clk-phase-sd-hs = <7>, <200>;
+	clk-phase-uhs-sdr50 = <0 130>, <0 130>;
 };
-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 08/10] arm: dts: ibm: Adjust clock phase parameter
  2021-09-22 10:31 [PATCH 00/10] ASPEED SD/eMMC controller clock configuration Chin-Ting Kuo
                   ` (6 preceding siblings ...)
  2021-09-22 10:31 ` [PATCH 07/10] arm: dts: aspeed: Adjust clock phase parameter Chin-Ting Kuo
@ 2021-09-22 10:31 ` Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 09/10] dt-bindings: mmc: aspeed: Add max-tap-delay property Chin-Ting Kuo
  2021-09-22 10:31 ` [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible string Chin-Ting Kuo
  9 siblings, 0 replies; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-22 10:31 UTC (permalink / raw)
  To: robh+dt, joel, mturquette, sboyd, adrian.hunter, linux-aspeed,
	openbmc, linux-mmc, devicetree, linux-arm-kernel, linux-kernel,
	linux-clk, andrew
  Cc: BMC-SW, steven_lee

- Add max-tap-delay property for eMMC controller.
- Change clock phase degree for AST2600 on IBM platforms.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts | 3 ++-
 arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 3 ++-
 arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts  | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
index 2efd70666738..eccb4749755a 100644
--- a/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
@@ -2824,6 +2824,7 @@
 
 &emmc_controller {
 	status = "okay";
+	max-tap-delay = <1253>;
 };
 
 &pinctrl_emmc_default {
@@ -2832,7 +2833,7 @@
 
 &emmc {
 	status = "okay";
-	clk-phase-mmc-hs200 = <210>, <228>;
+	clk-phase-mmc-hs200 = <1 124>, <1 141>;
 };
 
 &fsim0 {
diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
index 6419c9762c0b..2138a8a10d6e 100644
--- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
@@ -299,6 +299,7 @@
 
 &emmc_controller {
 	status = "okay";
+	max-tap-delay = <1253>;
 };
 
 &pinctrl_emmc_default {
@@ -307,7 +308,7 @@
 
 &emmc {
 	status = "okay";
-	clk-phase-mmc-hs200 = <180>, <180>;
+	clk-phase-mmc-hs200 = <1 90>, <1 90>;
 };
 
 &fsim0 {
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts b/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
index e39f310d55eb..7427809354cc 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
@@ -182,11 +182,12 @@
 
 &emmc_controller {
 	status = "okay";
+	max-tap-delay = <1253>;
 };
 
 &emmc {
 	status = "okay";
-	clk-phase-mmc-hs200 = <36>, <270>;
+	clk-phase-mmc-hs200 = <0 40>, <1 181>;
 };
 
 &fsim0 {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 09/10] dt-bindings: mmc: aspeed: Add max-tap-delay property
  2021-09-22 10:31 [PATCH 00/10] ASPEED SD/eMMC controller clock configuration Chin-Ting Kuo
                   ` (7 preceding siblings ...)
  2021-09-22 10:31 ` [PATCH 08/10] arm: dts: ibm: " Chin-Ting Kuo
@ 2021-09-22 10:31 ` Chin-Ting Kuo
  2021-09-27 18:40   ` Rob Herring
  2021-09-22 10:31 ` [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible string Chin-Ting Kuo
  9 siblings, 1 reply; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-22 10:31 UTC (permalink / raw)
  To: robh+dt, joel, mturquette, sboyd, adrian.hunter, linux-aspeed,
	openbmc, linux-mmc, devicetree, linux-arm-kernel, linux-kernel,
	linux-clk, andrew
  Cc: BMC-SW, steven_lee

Add max-tap-delay proptery in order to record the maximum
tap delay on different platforms.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
index 987b287f3bff..5bb66849df65 100644
--- a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
@@ -37,6 +37,9 @@ properties:
   clocks:
     maxItems: 1
     description: The SD/SDIO controller clock gate
+  max-tap-delay:
+    maxItems: 1
+    description: The maximum delay in picosecond for SD/SDIO controller
 
 patternProperties:
   "^sdhci@[0-9a-f]+$":
-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible string
  2021-09-22 10:31 [PATCH 00/10] ASPEED SD/eMMC controller clock configuration Chin-Ting Kuo
                   ` (8 preceding siblings ...)
  2021-09-22 10:31 ` [PATCH 09/10] dt-bindings: mmc: aspeed: Add max-tap-delay property Chin-Ting Kuo
@ 2021-09-22 10:31 ` Chin-Ting Kuo
  2021-09-27 18:59   ` Rob Herring
  9 siblings, 1 reply; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-22 10:31 UTC (permalink / raw)
  To: robh+dt, joel, mturquette, sboyd, adrian.hunter, linux-aspeed,
	openbmc, linux-mmc, devicetree, linux-arm-kernel, linux-kernel,
	linux-clk, andrew
  Cc: BMC-SW, steven_lee

Add "aspeed,ast2600-emmc" compatible string for the sake of
distinguishing between SD and eMMC device.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
index 5bb66849df65..41105cd104c6 100644
--- a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
@@ -52,6 +52,7 @@ patternProperties:
           - aspeed,ast2400-sdhci
           - aspeed,ast2500-sdhci
           - aspeed,ast2600-sdhci
+          - aspeed,ast2600-emmc
       reg:
         maxItems: 1
         description: The SDHCI registers
-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 01/10] clk: aspeed: ast2600: Porting sdhci clock source
  2021-09-22 10:31 ` [PATCH 01/10] clk: aspeed: ast2600: Porting sdhci clock source Chin-Ting Kuo
@ 2021-09-23  0:02   ` Joel Stanley
  2021-09-23  5:31     ` Chin-Ting Kuo
  0 siblings, 1 reply; 19+ messages in thread
From: Joel Stanley @ 2021-09-23  0:02 UTC (permalink / raw)
  To: Chin-Ting Kuo
  Cc: devicetree, linux-clk, linux-aspeed, BMC-SW, Stephen Boyd,
	Steven Lee, Michael Turquette, linux-mmc, Adrian Hunter,
	Linux Kernel Mailing List, Andrew Jeffery, Rob Herring,
	OpenBMC Maillist, Linux ARM

On Wed, 22 Sept 2021 at 10:31, Chin-Ting Kuo
<chin-ting_kuo@aspeedtech.com> wrote:
>
> - There are two clock sources used to generate
>   SD/SDIO clock, APLL clock and HCLK (200MHz).
>   User can select which clock source should be used
>   by configuring SCU310[8].
> - The SD/SDIO clock divider selection table SCU310[30:28]
>   is different between AST2600-A1 and AST2600-A2/A3.
>   For AST2600-A1, 200MHz SD/SDIO clock cannot be
>   gotten by the dividers in SCU310[30:28] if APLL
>   is not the multiple of 200MHz and HCLK is 200MHz.
>   For AST2600-A2/A3, a new divider, "1", is added and
>   200MHz SD/SDIO clock can be obtained by adopting HCLK
>   as clock source and setting SCU310[30:28] to 3b'111.
>
> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> ---
>  drivers/clk/clk-ast2600.c | 69 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 61 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> index bc3be5f3eae1..a6778c18274a 100644
> --- a/drivers/clk/clk-ast2600.c
> +++ b/drivers/clk/clk-ast2600.c
> @@ -168,6 +168,30 @@ static const struct clk_div_table ast2600_div_table[] = {
>         { 0 }
>  };
>
> +static const struct clk_div_table ast2600_sd_div_a1_table[] = {

Let's put the revision next to the ast2600 like the other tables:

ast2600_a1_sd_div_table

> +       { 0x0, 2 },
> +       { 0x1, 4 },
> +       { 0x2, 6 },
> +       { 0x3, 8 },
> +       { 0x4, 10 },
> +       { 0x5, 12 },
> +       { 0x6, 14 },
> +       { 0x7, 16 },
> +       { 0 }
> +};
> +
> +static const struct clk_div_table ast2600_sd_div_a2_table[] = {

For naming; can I propose we omit the revision for the A2/A3+ case? So
this one would be called:

ast2600_sd_div_table

> +       { 0x0, 2 },
> +       { 0x1, 4 },
> +       { 0x2, 6 },
> +       { 0x3, 8 },
> +       { 0x4, 10 },
> +       { 0x5, 12 },
> +       { 0x6, 14 },
> +       { 0x7, 1 },
> +       { 0 }
> +};
> +
>  /* For hpll/dpll/epll/mpll */
>  static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)
>  {
> @@ -424,6 +448,11 @@ static const char *const emmc_extclk_parent_names[] = {
>         "mpll",
>  };
>
> +static const char *const sd_extclk_parent_names[] = {
> +       "hclk",
> +       "apll",
> +};
> +
>  static const char * const vclk_parent_names[] = {
>         "dpll",
>         "d1pll",
> @@ -523,18 +552,42 @@ static int aspeed_g6_clk_probe(struct platform_device *pdev)
>                 return PTR_ERR(hw);
>         aspeed_g6_clk_data->hws[ASPEED_CLK_EMMC] = hw;
>
> -       /* SD/SDIO clock divider and gate */
> -       hw = clk_hw_register_gate(dev, "sd_extclk_gate", "hpll", 0,
> -                       scu_g6_base + ASPEED_G6_CLK_SELECTION4, 31, 0,
> -                       &aspeed_g6_clk_lock);
> +       clk_hw_register_fixed_rate(NULL, "hclk", NULL, 0, 200000000);
> +
> +       regmap_read(map, 0x310, &val);

Use the #defines for the register number.

> +       hw = clk_hw_register_mux(dev, "sd_extclk_mux",
> +                                sd_extclk_parent_names,
> +                                ARRAY_SIZE(sd_extclk_parent_names), 0,
> +                                scu_g6_base + ASPEED_G6_CLK_SELECTION4, 8, 1,
> +                                0, &aspeed_g6_clk_lock);
>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
> -       hw = clk_hw_register_divider_table(dev, "sd_extclk", "sd_extclk_gate",
> -                       0, scu_g6_base + ASPEED_G6_CLK_SELECTION4, 28, 3, 0,
> -                       ast2600_div_table,
> -                       &aspeed_g6_clk_lock);
> +
> +       hw = clk_hw_register_gate(dev, "sd_extclk_gate", "sd_extclk_mux",
> +                                 0, scu_g6_base + ASPEED_G6_CLK_SELECTION4,
> +                                 31, 0, &aspeed_g6_clk_lock);
>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
> +
> +       regmap_read(map, 0x14, &val);
> +       /* AST2600-A2/A3 clock divisor is different from AST2600-A1 */
> +       if (((val & GENMASK(23, 16)) >> 16) >= 2) {

I've got a little patch that I recommend you base your series on (feel
free to include it in your series when posting v2 to make it
self-contained):

https://lore.kernel.org/all/20210922235449.213631-1-joel@jms.id.au/

With this one you can do:

const struct clk_div_table* table;

 if (soc_rev >= 2)
   table = ast2600_sd_div_table;
else
   table = ast2600_a1_sd_div_table;

Then you don't need to duplicate the registration for each case:

               hw = clk_hw_register_divider_table(dev, "sd_extclk",
"sd_extclk_gate",
                                       0, scu_g6_base +
ASPEED_G6_CLK_SELECTION4, 28, 3, 0,
                                       table,
                                       &aspeed_g6_clk_lock);
               if (IS_ERR(hw))
                       return PTR_ERR(hw);

> +               /* AST2600-A2/A3 */
> +               hw = clk_hw_register_divider_table(dev, "sd_extclk", "sd_extclk_gate",
> +                                       0, scu_g6_base + ASPEED_G6_CLK_SELECTION4, 28, 3, 0,
> +                                       ast2600_sd_div_a2_table,
> +                                       &aspeed_g6_clk_lock);
> +               if (IS_ERR(hw))
> +                       return PTR_ERR(hw);
> +       } else {
> +               /* AST2600-A1 */
> +               hw = clk_hw_register_divider_table(dev, "sd_extclk", "sd_extclk_gate",
> +                                       0, scu_g6_base + ASPEED_G6_CLK_SELECTION4, 28, 3, 0,
> +                                       ast2600_sd_div_a1_table,
> +                                       &aspeed_g6_clk_lock);
> +               if (IS_ERR(hw))
> +                       return PTR_ERR(hw);
> +       }
>         aspeed_g6_clk_data->hws[ASPEED_CLK_SDIO] = hw;
>
>         /* MAC1/2 RMII 50MHz RCLK */
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH 01/10] clk: aspeed: ast2600: Porting sdhci clock source
  2021-09-23  0:02   ` Joel Stanley
@ 2021-09-23  5:31     ` Chin-Ting Kuo
  0 siblings, 0 replies; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-23  5:31 UTC (permalink / raw)
  To: Joel Stanley
  Cc: devicetree, linux-clk, linux-aspeed, BMC-SW, Stephen Boyd,
	Steven Lee, Michael Turquette, linux-mmc, Adrian Hunter,
	Linux Kernel Mailing List, Andrew Jeffery, Rob Herring,
	OpenBMC Maillist, Linux ARM

Hi Joel,

Thanks for the review.

> -----Original Message-----
> From: Joel Stanley <joel@jms.id.au>
> Sent: Thursday, September 23, 2021 8:02 AM
> To: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> Subject: Re: [PATCH 01/10] clk: aspeed: ast2600: Porting sdhci clock source
> 
> On Wed, 22 Sept 2021 at 10:31, Chin-Ting Kuo
> <chin-ting_kuo@aspeedtech.com> wrote:
> >
> > - There are two clock sources used to generate
> >   SD/SDIO clock, APLL clock and HCLK (200MHz).
> >   User can select which clock source should be used
> >   by configuring SCU310[8].
> > - The SD/SDIO clock divider selection table SCU310[30:28]
> >   is different between AST2600-A1 and AST2600-A2/A3.
> >   For AST2600-A1, 200MHz SD/SDIO clock cannot be
> >   gotten by the dividers in SCU310[30:28] if APLL
> >   is not the multiple of 200MHz and HCLK is 200MHz.
> >   For AST2600-A2/A3, a new divider, "1", is added and
> >   200MHz SD/SDIO clock can be obtained by adopting HCLK
> >   as clock source and setting SCU310[30:28] to 3b'111.
> >
> > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> > ---
> >  drivers/clk/clk-ast2600.c | 69
> > ++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 61 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> > index bc3be5f3eae1..a6778c18274a 100644
> > --- a/drivers/clk/clk-ast2600.c
> > +++ b/drivers/clk/clk-ast2600.c
> > @@ -168,6 +168,30 @@ static const struct clk_div_table ast2600_div_table[]
> = {
> >         { 0 }
> >  };
> >
> > +static const struct clk_div_table ast2600_sd_div_a1_table[] = {
> 
> Let's put the revision next to the ast2600 like the other tables:
> 
> ast2600_a1_sd_div_table
> 
> > +       { 0x0, 2 },
> > +       { 0x1, 4 },
> > +       { 0x2, 6 },
> > +       { 0x3, 8 },
> > +       { 0x4, 10 },
> > +       { 0x5, 12 },
> > +       { 0x6, 14 },
> > +       { 0x7, 16 },
> > +       { 0 }
> > +};
> > +
> > +static const struct clk_div_table ast2600_sd_div_a2_table[] = {
> 

Okay, this will be updated in the next patch version.

> For naming; can I propose we omit the revision for the A2/A3+ case? So this
> one would be called:
> 
> ast2600_sd_div_table
> 

Okay, this will also be updated in the next patch version.

> > +       { 0x0, 2 },
> > +       { 0x1, 4 },
> > +       { 0x2, 6 },
> > +       { 0x3, 8 },
> > +       { 0x4, 10 },
> > +       { 0x5, 12 },
> > +       { 0x6, 14 },
> > +       { 0x7, 1 },
> > +       { 0 }
> > +};
> > +
> >  /* For hpll/dpll/epll/mpll */
> >  static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)  {
> > @@ -424,6 +448,11 @@ static const char *const
> emmc_extclk_parent_names[] = {
> >         "mpll",
> >  };
> >
> > +static const char *const sd_extclk_parent_names[] = {
> > +       "hclk",
> > +       "apll",
> > +};
> > +
> >  static const char * const vclk_parent_names[] = {
> >         "dpll",
> >         "d1pll",
> > @@ -523,18 +552,42 @@ static int aspeed_g6_clk_probe(struct
> platform_device *pdev)
> >                 return PTR_ERR(hw);
> >         aspeed_g6_clk_data->hws[ASPEED_CLK_EMMC] = hw;
> >
> > -       /* SD/SDIO clock divider and gate */
> > -       hw = clk_hw_register_gate(dev, "sd_extclk_gate", "hpll", 0,
> > -                       scu_g6_base + ASPEED_G6_CLK_SELECTION4,
> 31, 0,
> > -                       &aspeed_g6_clk_lock);
> > +       clk_hw_register_fixed_rate(NULL, "hclk", NULL, 0, 200000000);
> > +
> > +       regmap_read(map, 0x310, &val);
> 
> Use the #defines for the register number.

Okay.

> 
> > +       hw = clk_hw_register_mux(dev, "sd_extclk_mux",
> > +                                sd_extclk_parent_names,
> > +
> ARRAY_SIZE(sd_extclk_parent_names), 0,
> > +                                scu_g6_base +
> ASPEED_G6_CLK_SELECTION4, 8, 1,
> > +                                0, &aspeed_g6_clk_lock);
> >         if (IS_ERR(hw))
> >                 return PTR_ERR(hw);
> > -       hw = clk_hw_register_divider_table(dev, "sd_extclk",
> "sd_extclk_gate",
> > -                       0, scu_g6_base + ASPEED_G6_CLK_SELECTION4,
> 28, 3, 0,
> > -                       ast2600_div_table,
> > -                       &aspeed_g6_clk_lock);
> > +
> > +       hw = clk_hw_register_gate(dev, "sd_extclk_gate", "sd_extclk_mux",
> > +                                 0, scu_g6_base +
> ASPEED_G6_CLK_SELECTION4,
> > +                                 31, 0, &aspeed_g6_clk_lock);
> >         if (IS_ERR(hw))
> >                 return PTR_ERR(hw);
> > +
> > +       regmap_read(map, 0x14, &val);
> > +       /* AST2600-A2/A3 clock divisor is different from AST2600-A1 */
> > +       if (((val & GENMASK(23, 16)) >> 16) >= 2) {
> 
> I've got a little patch that I recommend you base your series on (feel free to
> include it in your series when posting v2 to make it
> self-contained):
> 
> https://lore.kernel.org/all/20210922235449.213631-1-joel@jms.id.au/
> 
> With this one you can do:
> 
> const struct clk_div_table* table;
> 
>  if (soc_rev >= 2)
>    table = ast2600_sd_div_table;
> else
>    table = ast2600_a1_sd_div_table;
> 
> Then you don't need to duplicate the registration for each case:
> 
>                hw = clk_hw_register_divider_table(dev, "sd_extclk",
> "sd_extclk_gate",
>                                        0, scu_g6_base +
> ASPEED_G6_CLK_SELECTION4, 28, 3, 0,
>                                        table,
>                                        &aspeed_g6_clk_lock);
>                if (IS_ERR(hw))
>                        return PTR_ERR(hw);
> 

Okay, I will include your patch into this patch series when posting v2.

> > +               /* AST2600-A2/A3 */
> > +               hw = clk_hw_register_divider_table(dev, "sd_extclk",
> "sd_extclk_gate",
> > +                                       0, scu_g6_base +
> ASPEED_G6_CLK_SELECTION4, 28, 3, 0,
> > +                                       ast2600_sd_div_a2_table,
> > +                                       &aspeed_g6_clk_lock);
> > +               if (IS_ERR(hw))
> > +                       return PTR_ERR(hw);
> > +       } else {
> > +               /* AST2600-A1 */
> > +               hw = clk_hw_register_divider_table(dev, "sd_extclk",
> "sd_extclk_gate",
> > +                                       0, scu_g6_base +
> ASPEED_G6_CLK_SELECTION4, 28, 3, 0,
> > +                                       ast2600_sd_div_a1_table,
> > +                                       &aspeed_g6_clk_lock);
> > +               if (IS_ERR(hw))
> > +                       return PTR_ERR(hw);
> > +       }
> >         aspeed_g6_clk_data->hws[ASPEED_CLK_SDIO] = hw;
> >
> >         /* MAC1/2 RMII 50MHz RCLK */
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 09/10] dt-bindings: mmc: aspeed: Add max-tap-delay property
  2021-09-22 10:31 ` [PATCH 09/10] dt-bindings: mmc: aspeed: Add max-tap-delay property Chin-Ting Kuo
@ 2021-09-27 18:40   ` Rob Herring
  2021-09-28  2:50     ` Chin-Ting Kuo
  0 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2021-09-27 18:40 UTC (permalink / raw)
  To: Chin-Ting Kuo
  Cc: devicetree, linux-clk, linux-aspeed, BMC-SW, sboyd, steven_lee,
	openbmc, linux-mmc, adrian.hunter, linux-kernel, andrew,
	mturquette, linux-arm-kernel

On Wed, Sep 22, 2021 at 06:31:15PM +0800, Chin-Ting Kuo wrote:
> Add max-tap-delay proptery in order to record the maximum
> tap delay on different platforms.
> 
> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> ---
>  Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> index 987b287f3bff..5bb66849df65 100644
> --- a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> @@ -37,6 +37,9 @@ properties:
>    clocks:
>      maxItems: 1
>      description: The SD/SDIO controller clock gate
> +  max-tap-delay:
> +    maxItems: 1

An array?

> +    description: The maximum delay in picosecond for SD/SDIO controller

Properties with a unit should have a standard unit suffix.

Should be common property? If not, needs a vendor prefix.

>  
>  patternProperties:
>    "^sdhci@[0-9a-f]+$":
> -- 
> 2.17.1
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible string
  2021-09-22 10:31 ` [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible string Chin-Ting Kuo
@ 2021-09-27 18:59   ` Rob Herring
  2021-09-28  2:50     ` Chin-Ting Kuo
  0 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2021-09-27 18:59 UTC (permalink / raw)
  To: Chin-Ting Kuo
  Cc: devicetree, linux-clk, linux-aspeed, BMC-SW, sboyd, steven_lee,
	openbmc, linux-mmc, adrian.hunter, linux-kernel, andrew,
	mturquette, linux-arm-kernel

On Wed, Sep 22, 2021 at 06:31:16PM +0800, Chin-Ting Kuo wrote:
> Add "aspeed,ast2600-emmc" compatible string for the sake of
> distinguishing between SD and eMMC device.

Why?

Is the h/w block different? We already have properties to handle some of 
the eMMC specifics. Also, you can have a child node for the eMMC device 
if you need that.

> 
> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> ---
>  Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> index 5bb66849df65..41105cd104c6 100644
> --- a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> @@ -52,6 +52,7 @@ patternProperties:
>            - aspeed,ast2400-sdhci
>            - aspeed,ast2500-sdhci
>            - aspeed,ast2600-sdhci
> +          - aspeed,ast2600-emmc
>        reg:
>          maxItems: 1
>          description: The SDHCI registers
> -- 
> 2.17.1
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH 09/10] dt-bindings: mmc: aspeed: Add max-tap-delay property
  2021-09-27 18:40   ` Rob Herring
@ 2021-09-28  2:50     ` Chin-Ting Kuo
  0 siblings, 0 replies; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-28  2:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-clk, linux-aspeed, BMC-SW, sboyd, Steven Lee,
	openbmc, linux-mmc, adrian.hunter, linux-kernel, andrew,
	mturquette, linux-arm-kernel

Hi Rob,

Thanks for your review.

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, September 28, 2021 2:41 AM
> To: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> Subject: Re: [PATCH 09/10] dt-bindings: mmc: aspeed: Add max-tap-delay
> property
> 
> On Wed, Sep 22, 2021 at 06:31:15PM +0800, Chin-Ting Kuo wrote:
> > Add max-tap-delay proptery in order to record the maximum tap delay on
> > different platforms.
> >
> > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> > ---
> >  Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > index 987b287f3bff..5bb66849df65 100644
> > --- a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > @@ -37,6 +37,9 @@ properties:
> >    clocks:
> >      maxItems: 1
> >      description: The SD/SDIO controller clock gate
> > +  max-tap-delay:
> > +    maxItems: 1
> 
> An array?

No, it is an "int" value and this will be fixed in the next patch version.

> > +    description: The maximum delay in picosecond for SD/SDIO
> > + controller
> 
> Properties with a unit should have a standard unit suffix.
> 
> Should be common property? If not, needs a vendor prefix.

Okay, a vender prefix and an unit suffix will be add in the next patch version.

> 
> >
> >  patternProperties:
> >    "^sdhci@[0-9a-f]+$":
> > --
> > 2.17.1
> >
> >

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible string
  2021-09-27 18:59   ` Rob Herring
@ 2021-09-28  2:50     ` Chin-Ting Kuo
  2021-09-28 22:28       ` Rob Herring
  0 siblings, 1 reply; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-28  2:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-clk, linux-aspeed, BMC-SW, sboyd, Steven Lee,
	openbmc, linux-mmc, adrian.hunter, linux-kernel, andrew,
	mturquette, linux-arm-kernel

Hi Rob,

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, September 28, 2021 2:59 AM
> To: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> Subject: Re: [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible
> string
> 
> On Wed, Sep 22, 2021 at 06:31:16PM +0800, Chin-Ting Kuo wrote:
> > Add "aspeed,ast2600-emmc" compatible string for the sake of
> > distinguishing between SD and eMMC device.
> 
> Why?
> 
> Is the h/w block different? We already have properties to handle some of the
> eMMC specifics. Also, you can have a child node for the eMMC device if you
> need that.

There are two SD/SDIO controllers in a AST2600 SoC.
One is for SD card and the other is for eMMC.
Although both of them are embedded in the same SoC, the design of delay cell and
the manufacture process are different. The delay phase is definitely different and, thus,
we need a flag, compatible, to distinguish the device, SD or eMMC.

Without "aspeed,ast2600-emmc" compatible, of course, eMMC device can work with original
sdhci driver and device tree setting. But, for ultra-speed or HS200 case, AST2600 SoC needs some
phase delay which (maximum) value is different between SD and eMMC device.

> 
> >
> > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> > ---
> >  Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > index 5bb66849df65..41105cd104c6 100644
> > --- a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > @@ -52,6 +52,7 @@ patternProperties:
> >            - aspeed,ast2400-sdhci
> >            - aspeed,ast2500-sdhci
> >            - aspeed,ast2600-sdhci
> > +          - aspeed,ast2600-emmc
> >        reg:
> >          maxItems: 1
> >          description: The SDHCI registers
> > --
> > 2.17.1
> >
> >

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible string
  2021-09-28  2:50     ` Chin-Ting Kuo
@ 2021-09-28 22:28       ` Rob Herring
  2021-09-29  3:03         ` Chin-Ting Kuo
  0 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2021-09-28 22:28 UTC (permalink / raw)
  To: Chin-Ting Kuo
  Cc: devicetree, linux-clk, linux-aspeed, BMC-SW, sboyd, Steven Lee,
	openbmc, linux-mmc, adrian.hunter, linux-kernel, andrew,
	mturquette, linux-arm-kernel

On Mon, Sep 27, 2021 at 9:51 PM Chin-Ting Kuo
<chin-ting_kuo@aspeedtech.com> wrote:
>
> Hi Rob,
>
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: Tuesday, September 28, 2021 2:59 AM
> > To: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> > Subject: Re: [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible
> > string
> >
> > On Wed, Sep 22, 2021 at 06:31:16PM +0800, Chin-Ting Kuo wrote:
> > > Add "aspeed,ast2600-emmc" compatible string for the sake of
> > > distinguishing between SD and eMMC device.
> >
> > Why?
> >
> > Is the h/w block different? We already have properties to handle some of the
> > eMMC specifics. Also, you can have a child node for the eMMC device if you
> > need that.
>
> There are two SD/SDIO controllers in a AST2600 SoC.
> One is for SD card and the other is for eMMC.
> Although both of them are embedded in the same SoC, the design of delay cell and
> the manufacture process are different. The delay phase is definitely different and, thus,
> we need a flag, compatible, to distinguish the device, SD or eMMC.
>
> Without "aspeed,ast2600-emmc" compatible, of course, eMMC device can work with original
> sdhci driver and device tree setting. But, for ultra-speed or HS200 case, AST2600 SoC needs some
> phase delay which (maximum) value is different between SD and eMMC device.

This is quite common as tweaking the timing is also need per board.
Look at what other bindings have done. A property is more appropriate
here.

Rob

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible string
  2021-09-28 22:28       ` Rob Herring
@ 2021-09-29  3:03         ` Chin-Ting Kuo
  0 siblings, 0 replies; 19+ messages in thread
From: Chin-Ting Kuo @ 2021-09-29  3:03 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-clk, linux-aspeed, BMC-SW, sboyd, Steven Lee,
	openbmc, linux-mmc, adrian.hunter, linux-kernel, andrew,
	mturquette, linux-arm-kernel

Hi Rob

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Wednesday, September 29, 2021 6:28 AM
> To: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> Subject: Re: [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible
> string
> 
> On Mon, Sep 27, 2021 at 9:51 PM Chin-Ting Kuo
> <chin-ting_kuo@aspeedtech.com> wrote:
> >
> > Hi Rob,
> >
> > > -----Original Message-----
> > > From: Rob Herring <robh@kernel.org>
> > > Sent: Tuesday, September 28, 2021 2:59 AM
> > > To: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> > > Subject: Re: [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new
> > > compatible string
> > >
> > > On Wed, Sep 22, 2021 at 06:31:16PM +0800, Chin-Ting Kuo wrote:
> > > > Add "aspeed,ast2600-emmc" compatible string for the sake of
> > > > distinguishing between SD and eMMC device.
> > >
> > > Why?
> > >
> > > Is the h/w block different? We already have properties to handle
> > > some of the eMMC specifics. Also, you can have a child node for the
> > > eMMC device if you need that.
> >
> > There are two SD/SDIO controllers in a AST2600 SoC.
> > One is for SD card and the other is for eMMC.
> > Although both of them are embedded in the same SoC, the design of
> > delay cell and the manufacture process are different. The delay phase
> > is definitely different and, thus, we need a flag, compatible, to distinguish the
> device, SD or eMMC.
> >
> > Without "aspeed,ast2600-emmc" compatible, of course, eMMC device can
> > work with original sdhci driver and device tree setting. But, for
> > ultra-speed or HS200 case, AST2600 SoC needs some phase delay which
> (maximum) value is different between SD and eMMC device.
> 
> This is quite common as tweaking the timing is also need per board.
> Look at what other bindings have done. A property is more appropriate here.

Okay, I will try to check whether there is an existing binding which can achieve this purpose.
Or, maybe, as you said, a property is better since this phase delay is a proprietary
HW design and is different between each chipset version.

> 
> Rob

Chin-Ting

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-09-29  3:05 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 10:31 [PATCH 00/10] ASPEED SD/eMMC controller clock configuration Chin-Ting Kuo
2021-09-22 10:31 ` [PATCH 01/10] clk: aspeed: ast2600: Porting sdhci clock source Chin-Ting Kuo
2021-09-23  0:02   ` Joel Stanley
2021-09-23  5:31     ` Chin-Ting Kuo
2021-09-22 10:31 ` [PATCH 02/10] sdhci: aspeed: Add SDR50 support Chin-Ting Kuo
2021-09-22 10:31 ` [PATCH 03/10] dts: aspeed: ast2600: Support SDR50 for SD device Chin-Ting Kuo
2021-09-22 10:31 ` [PATCH 04/10] mmc: Add invert flag for clock phase signedness Chin-Ting Kuo
2021-09-22 10:31 ` [PATCH 05/10] mmc: aspeed: Adjust delay taps calculation method Chin-Ting Kuo
2021-09-22 10:31 ` [PATCH 06/10] arm: dts: aspeed: Change eMMC device compatible Chin-Ting Kuo
2021-09-22 10:31 ` [PATCH 07/10] arm: dts: aspeed: Adjust clock phase parameter Chin-Ting Kuo
2021-09-22 10:31 ` [PATCH 08/10] arm: dts: ibm: " Chin-Ting Kuo
2021-09-22 10:31 ` [PATCH 09/10] dt-bindings: mmc: aspeed: Add max-tap-delay property Chin-Ting Kuo
2021-09-27 18:40   ` Rob Herring
2021-09-28  2:50     ` Chin-Ting Kuo
2021-09-22 10:31 ` [PATCH 10/10] dt-bindings: mmc: aspeed: Add a new compatible string Chin-Ting Kuo
2021-09-27 18:59   ` Rob Herring
2021-09-28  2:50     ` Chin-Ting Kuo
2021-09-28 22:28       ` Rob Herring
2021-09-29  3:03         ` Chin-Ting Kuo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).