diff mbox series

[v1,1/5] mmc: sdhci_am654: Add tuning algorithm for delay chain

Message ID 20240131215044.3163469-2-jm@ti.com (mailing list archive)
State New, archived
Headers show
Series Add tuning algorithm for delay chain | expand

Commit Message

Judith Mendez Jan. 31, 2024, 9:50 p.m. UTC 
Currently the sdhci_am654 driver only supports one tuning
algorithm which should be used only when DLL is enabled. The
ITAPDLY is selected from the largest passing window and the
buffer is viewed as a circular buffer.

The new algorithm should be used when the delay chain
is enabled. The ITAPDLY is selected from the largest passing
window and the buffer is not viewed as a circular buffer.

This implementation is based off of the following paper: [1].

Also add support for multiple failing windows.

[1] https://www.ti.com/lit/an/spract9/spract9.pdf

Fixes: 13ebeae68ac9 ("mmc: sdhci_am654: Add support for software tuning")
Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/mmc/host/sdhci_am654.c | 128 +++++++++++++++++++++++++++------
 1 file changed, 108 insertions(+), 20 deletions(-)

Comments

Andrew Davis Feb. 1, 2024, 7:24 p.m. UTC | #1 
On 1/31/24 3:50 PM, Judith Mendez wrote:
> Currently the sdhci_am654 driver only supports one tuning
> algorithm which should be used only when DLL is enabled. The
> ITAPDLY is selected from the largest passing window and the
> buffer is viewed as a circular buffer.
> 
> The new algorithm should be used when the delay chain
> is enabled. The ITAPDLY is selected from the largest passing
> window and the buffer is not viewed as a circular buffer.
> 
> This implementation is based off of the following paper: [1].
> 
> Also add support for multiple failing windows.
> 
> [1] https://www.ti.com/lit/an/spract9/spract9.pdf
> 
> Fixes: 13ebeae68ac9 ("mmc: sdhci_am654: Add support for software tuning")
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>   drivers/mmc/host/sdhci_am654.c | 128 +++++++++++++++++++++++++++------
>   1 file changed, 108 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index d659c59422e1..a3798c9912f6 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -149,10 +149,17 @@ struct sdhci_am654_data {
>   	int strb_sel;
>   	u32 flags;
>   	u32 quirks;
> +	bool dll_enable;
>   
>   #define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
>   };
>   
> +struct window {
> +	u8 start;
> +	u8 end;
> +	u8 length;
> +};
> +
>   struct sdhci_am654_driver_data {
>   	const struct sdhci_pltfm_data *pdata;
>   	u32 flags;
> @@ -290,10 +297,13 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
>   
>   	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
>   
> -	if (timing > MMC_TIMING_UHS_SDR25 && clock >= CLOCK_TOO_SLOW_HZ)
> +	if (timing > MMC_TIMING_UHS_SDR25 && clock >= CLOCK_TOO_SLOW_HZ) {
>   		sdhci_am654_setup_dll(host, clock);
> -	else
> +		sdhci_am654->dll_enable = true;
> +	} else {
>   		sdhci_am654_setup_delay_chain(sdhci_am654, timing);
> +		sdhci_am654->dll_enable = false;
> +	}
>   
>   	regmap_update_bits(sdhci_am654->base, PHY_CTRL5, CLKBUFSEL_MASK,
>   			   sdhci_am654->clkbuf_sel);
> @@ -408,39 +418,117 @@ static u32 sdhci_am654_cqhci_irq(struct sdhci_host *host, u32 intmask)
>   	return 0;
>   }
>   
> -#define ITAP_MAX	32
> +#define ITAPDLY_LENGTH 32
> +#define ITAPDLY_LAST_INDEX 31
> +static u32 sdhci_am654_calculate_itap(struct sdhci_host *host, struct window
> +			  *fail_window, u8 num_fails, bool circular_buffer)
> +{
> +	struct device *dev = mmc_dev(host->mmc);
> +	struct window pass_window, first_fail, last_fail;

struct window pass_window = {}, ..

Then you can drop the memset()s below.

> +	u8 itap = 0, start_fail = 0, end_fail = 0, pass_length = 0;
> +	int prev_end_fail = -1;
> +	u8 i;
> +
> +	memset(&pass_window, 0, sizeof(pass_window));
> +	memset(&first_fail, 0, sizeof(first_fail));
> +	memset(&last_fail, 0, sizeof(last_fail));
> +
> +	if (!num_fails) {
> +		itap = ITAPDLY_LAST_INDEX >> 1;

return ITAPDLY_LAST_INDEX >> 1;

> +	} else if (fail_window->length == ITAPDLY_LENGTH) {
> +		dev_err(dev, "No passing ITAPDLY, return 0\n");
> +		itap = 0;

return 0;

> +	} else {

If you shortcut return directly in the above to branches, then
this all below doesn't need to be in the else {} and you won't
have to indent it all out so far.

> +		for (i = 0; i < num_fails; i++) {
> +			start_fail = fail_window[i].start;
> +			end_fail = fail_window[i].end;
> +
> +			if (i == 0) {

Move this first case to before the loop, we already know what
first_fail will be filled with. No need to check i == 0 every iteration
of the loop. Same for last_fail, just move to after the loop.

> +				first_fail.start = start_fail;
> +				first_fail.end = end_fail;
> +				first_fail.length = fail_window[0].length;
> +			}
> +
> +			if (i == num_fails - 1) {
> +				last_fail.start = start_fail;
> +				last_fail.end = end_fail;
> +				last_fail.length = fail_window[i].length;
> +			}
> +
> +			pass_length = start_fail - (prev_end_fail + 1);
> +			if (pass_length > pass_window.length) {
> +				pass_window.start = prev_end_fail + 1;
> +				pass_window.length = pass_length;
> +			}
> +			prev_end_fail = end_fail;
> +		}
> +
> +		if (!circular_buffer) {
> +			if (ITAPDLY_LAST_INDEX - end_fail > pass_window.length) {
> +				pass_window.start = end_fail + 1;
> +				pass_window.length = ITAPDLY_LAST_INDEX - end_fail;
> +			}
> +		} else {
> +			pass_length = ITAPDLY_LAST_INDEX - end_fail + first_fail.start;
> +			if (pass_length > pass_window.length) {
> +				pass_window.start = last_fail.end + 1;
> +				pass_window.length = pass_length;
> +			}
> +		}
> +
> +		if (!circular_buffer)
> +			itap = pass_window.start + (pass_window.length >> 1);
> +		else
> +			itap = (pass_window.start + (pass_window.length >> 1)) % ITAPDLY_LENGTH;
> +
> +		if (itap < 0 || itap > ITAPDLY_LAST_INDEX)
> +			itap = 0;
> +	}
> +
> +	return itap;
> +}
> +
>   static int sdhci_am654_platform_execute_tuning(struct sdhci_host *host,
>   					       u32 opcode)
>   {
>   	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>   	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> -	int cur_val, prev_val = 1, fail_len = 0, pass_window = 0, pass_len;
> -	u32 itap;
> +	struct window fail_window[ITAPDLY_LENGTH];
> +	u8 prev_pass = 1;
> +	u8 fail_index = 0;
> +	u8 curr_pass, itap;
> +
> +	memset(fail_window, 0, sizeof(fail_window[0]) * ITAPDLY_LENGTH);
>   
>   	/* Enable ITAPDLY */
>   	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPDLYENA_MASK,
>   			   1 << ITAPDLYENA_SHIFT);
>   
> -	for (itap = 0; itap < ITAP_MAX; itap++) {
> +	for (itap = 0; itap < ITAPDLY_LENGTH; itap++) {
>   		sdhci_am654_write_itapdly(sdhci_am654, itap);
>   
> -		cur_val = !mmc_send_tuning(host->mmc, opcode, NULL);
> -		if (cur_val && !prev_val)
> -			pass_window = itap;
> +		curr_pass = !mmc_send_tuning(host->mmc, opcode, NULL);
>   
> -		if (!cur_val)
> -			fail_len++;
> +		if (!curr_pass && prev_pass)
> +			fail_window[fail_index].start = itap;
>   
> -		prev_val = cur_val;
> +		if (!curr_pass) {
> +			fail_window[fail_index].end = itap;
> +			fail_window[fail_index].length++;
> +		}
> +
> +		if (curr_pass && !prev_pass)
> +			fail_index++;
> +
> +		prev_pass = curr_pass;
>   	}
> -	/*
> -	 * Having determined the length of the failing window and start of
> -	 * the passing window calculate the length of the passing window and
> -	 * set the final value halfway through it considering the range as a
> -	 * circular buffer
> -	 */
> -	pass_len = ITAP_MAX - fail_len;
> -	itap = (pass_window + (pass_len >> 1)) % ITAP_MAX;
> +
> +	if (fail_window[fail_index].length != 0)
> +		fail_index++;
> +
> +	itap = sdhci_am654_calculate_itap(host, fail_window, fail_index,
> +					  (sdhci_am654->dll_enable ? true : false));

dll_enable is already a bool, the line:

(sdhci_am654->dll_enable ? true : false)

has no effect, just use sdhci_am654->dll_enable directly.

Andrew

> +
>   	sdhci_am654_write_itapdly(sdhci_am654, itap);
>   
>   	return 0;
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index d659c59422e1..a3798c9912f6 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -149,10 +149,17 @@  struct sdhci_am654_data {
 	int strb_sel;
 	u32 flags;
 	u32 quirks;
+	bool dll_enable;
 
 #define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
 };
 
+struct window {
+	u8 start;
+	u8 end;
+	u8 length;
+};
+
 struct sdhci_am654_driver_data {
 	const struct sdhci_pltfm_data *pdata;
 	u32 flags;
@@ -290,10 +297,13 @@  static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
 
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
 
-	if (timing > MMC_TIMING_UHS_SDR25 && clock >= CLOCK_TOO_SLOW_HZ)
+	if (timing > MMC_TIMING_UHS_SDR25 && clock >= CLOCK_TOO_SLOW_HZ) {
 		sdhci_am654_setup_dll(host, clock);
-	else
+		sdhci_am654->dll_enable = true;
+	} else {
 		sdhci_am654_setup_delay_chain(sdhci_am654, timing);
+		sdhci_am654->dll_enable = false;
+	}
 
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL5, CLKBUFSEL_MASK,
 			   sdhci_am654->clkbuf_sel);
@@ -408,39 +418,117 @@  static u32 sdhci_am654_cqhci_irq(struct sdhci_host *host, u32 intmask)
 	return 0;
 }
 
-#define ITAP_MAX	32
+#define ITAPDLY_LENGTH 32
+#define ITAPDLY_LAST_INDEX 31
+static u32 sdhci_am654_calculate_itap(struct sdhci_host *host, struct window
+			  *fail_window, u8 num_fails, bool circular_buffer)
+{
+	struct device *dev = mmc_dev(host->mmc);
+	struct window pass_window, first_fail, last_fail;
+	u8 itap = 0, start_fail = 0, end_fail = 0, pass_length = 0;
+	int prev_end_fail = -1;
+	u8 i;
+
+	memset(&pass_window, 0, sizeof(pass_window));
+	memset(&first_fail, 0, sizeof(first_fail));
+	memset(&last_fail, 0, sizeof(last_fail));
+
+	if (!num_fails) {
+		itap = ITAPDLY_LAST_INDEX >> 1;
+	} else if (fail_window->length == ITAPDLY_LENGTH) {
+		dev_err(dev, "No passing ITAPDLY, return 0\n");
+		itap = 0;
+	} else {
+		for (i = 0; i < num_fails; i++) {
+			start_fail = fail_window[i].start;
+			end_fail = fail_window[i].end;
+
+			if (i == 0) {
+				first_fail.start = start_fail;
+				first_fail.end = end_fail;
+				first_fail.length = fail_window[0].length;
+			}
+
+			if (i == num_fails - 1) {
+				last_fail.start = start_fail;
+				last_fail.end = end_fail;
+				last_fail.length = fail_window[i].length;
+			}
+
+			pass_length = start_fail - (prev_end_fail + 1);
+			if (pass_length > pass_window.length) {
+				pass_window.start = prev_end_fail + 1;
+				pass_window.length = pass_length;
+			}
+			prev_end_fail = end_fail;
+		}
+
+		if (!circular_buffer) {
+			if (ITAPDLY_LAST_INDEX - end_fail > pass_window.length) {
+				pass_window.start = end_fail + 1;
+				pass_window.length = ITAPDLY_LAST_INDEX - end_fail;
+			}
+		} else {
+			pass_length = ITAPDLY_LAST_INDEX - end_fail + first_fail.start;
+			if (pass_length > pass_window.length) {
+				pass_window.start = last_fail.end + 1;
+				pass_window.length = pass_length;
+			}
+		}
+
+		if (!circular_buffer)
+			itap = pass_window.start + (pass_window.length >> 1);
+		else
+			itap = (pass_window.start + (pass_window.length >> 1)) % ITAPDLY_LENGTH;
+
+		if (itap < 0 || itap > ITAPDLY_LAST_INDEX)
+			itap = 0;
+	}
+
+	return itap;
+}
+
 static int sdhci_am654_platform_execute_tuning(struct sdhci_host *host,
 					       u32 opcode)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
-	int cur_val, prev_val = 1, fail_len = 0, pass_window = 0, pass_len;
-	u32 itap;
+	struct window fail_window[ITAPDLY_LENGTH];
+	u8 prev_pass = 1;
+	u8 fail_index = 0;
+	u8 curr_pass, itap;
+
+	memset(fail_window, 0, sizeof(fail_window[0]) * ITAPDLY_LENGTH);
 
 	/* Enable ITAPDLY */
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPDLYENA_MASK,
 			   1 << ITAPDLYENA_SHIFT);
 
-	for (itap = 0; itap < ITAP_MAX; itap++) {
+	for (itap = 0; itap < ITAPDLY_LENGTH; itap++) {
 		sdhci_am654_write_itapdly(sdhci_am654, itap);
 
-		cur_val = !mmc_send_tuning(host->mmc, opcode, NULL);
-		if (cur_val && !prev_val)
-			pass_window = itap;
+		curr_pass = !mmc_send_tuning(host->mmc, opcode, NULL);
 
-		if (!cur_val)
-			fail_len++;
+		if (!curr_pass && prev_pass)
+			fail_window[fail_index].start = itap;
 
-		prev_val = cur_val;
+		if (!curr_pass) {
+			fail_window[fail_index].end = itap;
+			fail_window[fail_index].length++;
+		}
+
+		if (curr_pass && !prev_pass)
+			fail_index++;
+
+		prev_pass = curr_pass;
 	}
-	/*
-	 * Having determined the length of the failing window and start of
-	 * the passing window calculate the length of the passing window and
-	 * set the final value halfway through it considering the range as a
-	 * circular buffer
-	 */
-	pass_len = ITAP_MAX - fail_len;
-	itap = (pass_window + (pass_len >> 1)) % ITAP_MAX;
+
+	if (fail_window[fail_index].length != 0)
+		fail_index++;
+
+	itap = sdhci_am654_calculate_itap(host, fail_window, fail_index,
+					  (sdhci_am654->dll_enable ? true : false));
+
 	sdhci_am654_write_itapdly(sdhci_am654, itap);
 
 	return 0;