[RFC PATCH 00/13] Add tuning algorithm for delay chain

[RFC PATCH 00/13] Add tuning algorithm for delay chain

[Judith Mendez]

This patch series introduces a new tuning algorithm for
mmc. The new algorithm should be used when delay chain is
enabled. The ITAPDLY is selected from the largest passing
window and the buffer is not viewed as a circular buffer.
The new tuning algorithm is implemented as per the paper
published here [0] and has been tested on the following
platforms: AM62x SK, AM62A SK, AM62p SK, AM64x SK, and AM64x
EVM.

The series also includes a few fixes in the sdhci_am654
driver on OTAPDLYEN/ITAPDLYEN and ITAPDELSEL. There are
also device tree node fixes for missing mmc nodes,
modifying DLL properties, and fixes for OTAP/ITAP delay
values. 

MMC0/MMC2 nodes are introduced for AM62ax in this series.

This series is sent as a RFC mostly to get some feedback
and/or comments on the new tuning algorithm implementation.

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

Judith Mendez (11):
  drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
  drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
  drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
  drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
    sdhci_j721e_4bit_set_clock
  drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing
  arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
  arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
  arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
  arm64: dts: ti: k3-am62p: Add missing properties for MMC
  arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
  arm64: dts: ti: k3-am6*: Reorganize MMC properties

Nitin Yadav (2):
  arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
  arm64: dts: ti: k3-am62a7-sk: Enable eMMC support

 arch/arm64/boot/dts/ti/k3-am62-main.dtsi      |  57 +++--
 .../arm64/boot/dts/ti/k3-am625-beagleplay.dts |   5 -
 arch/arm64/boot/dts/ti/k3-am62a-main.dtsi     |  45 +++-
 arch/arm64/boot/dts/ti/k3-am62a7-sk.dts       |  27 ++-
 arch/arm64/boot/dts/ti/k3-am62p-main.dtsi     |  44 +++-
 arch/arm64/boot/dts/ti/k3-am62p5-sk.dts       |   7 +-
 .../arm64/boot/dts/ti/k3-am62x-sk-common.dtsi |   4 +-
 arch/arm64/boot/dts/ti/k3-am64-main.dtsi      |  17 +-
 arch/arm64/boot/dts/ti/k3-am642-evm.dts       |   4 +-
 arch/arm64/boot/dts/ti/k3-am642-sk.dts        |   2 -
 drivers/mmc/host/sdhci_am654.c                | 215 ++++++++++++++----
 11 files changed, 321 insertions(+), 106 deletions(-)

-- 
2.34.1

[RFC PATCH 01/13] drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain

[Judith Mendez]

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

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..59d205511312 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 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;
+
+	memset(&pass_window, 0, sizeof(pass_window));
+	memset(&first_fail, 0, sizeof(first_fail));
+	memset(&last_fail, 0, sizeof(last_fail));
+
+	if (!num_fails) {
+		return ITAPDLY_LAST_INDEX >> 1;
+	} else if (fail_window->length == ITAPDLY_LENGTH) {
+		dev_warn(dev, "No passing ITAPDLY, return 0\n");
+		return 0;
+	} else {
+		for (int 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, i;
+
+	for (i = 0; i < ITAPDLY_LENGTH; i++)
+		memset(&fail_window[i], 0, sizeof(fail_window[0]));
 
 	/* 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 = calculate_itap(host, &fail_window[0], fail_index,
+			      (sdhci_am654->dll_enable ? true : false));
+
 	sdhci_am654_write_itapdly(sdhci_am654, itap);
 
 	return 0;
-- 
2.34.1

Re: [RFC PATCH 01/13] drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain

[Vignesh Raghavendra]

Hi Judith,

On 31/01/24 06:07, 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
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---


$subject prefix should be

 mmc: sdhci_am654:

See git log --oneline drivers/mmc/host/sdhci_am654.c for precedence.

>  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..59d205511312 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 calculate_itap(struct sdhci_host *host, struct window *fail_window,
> +			  u8 num_fails, bool circular_buffer)


s/calculate_itap/sdhci_am654_calculate_itap/ to keep function naming
consistent within the file

> +{
> +	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;
> +
> +	memset(&pass_window, 0, sizeof(pass_window));
> +	memset(&first_fail, 0, sizeof(first_fail));
> +	memset(&last_fail, 0, sizeof(last_fail));
> +
> +	if (!num_fails) {
> +		return ITAPDLY_LAST_INDEX >> 1;
> +	} else if (fail_window->length == ITAPDLY_LENGTH) {
> +		dev_warn(dev, "No passing ITAPDLY, return 0\n");

Drop return 0 in dev_warn()

> +		return 0;

Should this be an error?

> +	} else {
> +		for (int i = 0; i < num_fails; i++) {


please avoid inline declaration to align with rest of the file.

> +			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, i;
> +
> +	for (i = 0; i < ITAPDLY_LENGTH; i++)
> +		memset(&fail_window[i], 0, sizeof(fail_window[0]));

Don't need for() loop, just memset entire array at once. Something like:

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++;

These is some tab vs space mangling here.

> +
> +	itap = calculate_itap(host, &fail_window[0], fail_index,
> +			      (sdhci_am654->dll_enable ? true : false));
> +
>  	sdhci_am654_write_itapdly(sdhci_am654, itap);
>  
>  	return 0;

-- 
Regards
Vignesh

Re: [RFC PATCH 01/13] drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain

[Judith Mendez]

On 1/31/24 5:04 AM, Vignesh Raghavendra wrote:
> Hi Judith,
> 
> On 31/01/24 06:07, 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
>>
>> Signed-off-by: Judith Mendez <jm@ti.com>
>> ---
> 
> 
> $subject prefix should be
> 
>   mmc: sdhci_am654:
> 
> See git log --oneline drivers/mmc/host/sdhci_am654.c for precedence.

Thanks, will add in v1.

> 
>>   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..59d205511312 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 calculate_itap(struct sdhci_host *host, struct window *fail_window,
>> +			  u8 num_fails, bool circular_buffer)
> 
> 
> s/calculate_itap/sdhci_am654_calculate_itap/ to keep function naming
> consistent within the file

Will add.

> 
>> +{
>> +	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;
>> +
>> +	memset(&pass_window, 0, sizeof(pass_window));
>> +	memset(&first_fail, 0, sizeof(first_fail));
>> +	memset(&last_fail, 0, sizeof(last_fail));
>> +
>> +	if (!num_fails) {
>> +		return ITAPDLY_LAST_INDEX >> 1;
>> +	} else if (fail_window->length == ITAPDLY_LENGTH) {
>> +		dev_warn(dev, "No passing ITAPDLY, return 0\n");
> 
> Drop return 0 in dev_warn()

Good idea, will update itap value here then.

> 
>> +		return 0;
> 
> Should this be an error?

It is an error, but I thought it was a good idea to return 0 in case all 
ITAPDLY values fail and report this via dev_warn or dev_error. For now I 
can use dev_error.

> 
>> +	} else {
>> +		for (int i = 0; i < num_fails; i++) {
> 
> 
> please avoid inline declaration to align with rest of the file.

Will do.
> 
>> +			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, i;
>> +
>> +	for (i = 0; i < ITAPDLY_LENGTH; i++)
>> +		memset(&fail_window[i], 0, sizeof(fail_window[0]));
> 
> Don't need for() loop, just memset entire array at once. Something like:
> 
> memset(fail_window, 0, sizeof(fail_window[0]) * ITAPDLY_LENGTH);

Great, will add this, thanks.

> 
>>   
>>   	/* 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++;
> 
> These is some tab vs space mangling here.

I will fix this. (:

> 
>> +
>> +	itap = calculate_itap(host, &fail_window[0], fail_index,
>> +			      (sdhci_am654->dll_enable ? true : false));
>> +
>>   	sdhci_am654_write_itapdly(sdhci_am654, itap);
>>   
>>   	return 0;
> 

Re: [RFC PATCH 01/13] drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain

[Roger Quadros]

On 31/01/2024 02:37, Judith Mendez wrote:
> Currently the sdhci_am654 driver only supports one tuning
> algorithm which should be used only when DLL is enabled. The

What does DLL stand for?

> 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
> 
> 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..59d205511312 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 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;
> +
> +	memset(&pass_window, 0, sizeof(pass_window));
> +	memset(&first_fail, 0, sizeof(first_fail));
> +	memset(&last_fail, 0, sizeof(last_fail));
> +
> +	if (!num_fails) {
> +		return ITAPDLY_LAST_INDEX >> 1;
> +	} else if (fail_window->length == ITAPDLY_LENGTH) {
> +		dev_warn(dev, "No passing ITAPDLY, return 0\n");

This is a helper function, instead of printing a warning here,
you should return an error value, and allow the caller to decide
what to print and if this is serious enough to bail out.

> +		return 0;
> +	} else {
> +		for (int 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, i;
> +
> +	for (i = 0; i < ITAPDLY_LENGTH; i++)
> +		memset(&fail_window[i], 0, sizeof(fail_window[0]));
>  
>  	/* 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 = calculate_itap(host, &fail_window[0], fail_index,
> +			      (sdhci_am654->dll_enable ? true : false));
> +

If calculate_itap fails, you should return error
so caller can decide what to do.

>  	sdhci_am654_write_itapdly(sdhci_am654, itap);
>  
>  	return 0;

-- 
cheers,
-roger

[RFC PATCH 02/13] drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing

[Judith Mendez]

For DDR52 timing, DLL is enabled but tuning is not carried
out, therefore the ITAPDLY value in PHY CTRL 4 register is
not correct. Fix this by writing ITAPDLY after enabling DLL.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/mmc/host/sdhci_am654.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index 59d205511312..0a1ed2ae2eef 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -170,7 +170,19 @@ struct sdhci_am654_driver_data {
 #define DLL_CALIB	(1 << 4)
 };
 
-static void sdhci_am654_setup_dll(struct sdhci_host *host, unsigned int clock)
+static void sdhci_am654_write_itapdly(struct sdhci_am654_data *sdhci_am654,
+				      u32 itapdly)
+{
+	/* Set ITAPCHGWIN before writing to ITAPDLY */
+	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK,
+			   0x1 << ITAPCHGWIN_SHIFT);
+	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPDLYSEL_MASK,
+			   itapdly << ITAPDLYSEL_SHIFT);
+	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, 0);
+}
+
+static void sdhci_am654_setup_dll(struct sdhci_host *host, unsigned int clock,
+				  unsigned char timing)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
@@ -236,17 +248,8 @@ static void sdhci_am654_setup_dll(struct sdhci_host *host, unsigned int clock)
 		dev_err(mmc_dev(host->mmc), "DLL failed to relock\n");
 		return;
 	}
-}
 
-static void sdhci_am654_write_itapdly(struct sdhci_am654_data *sdhci_am654,
-				      u32 itapdly)
-{
-	/* Set ITAPCHGWIN before writing to ITAPDLY */
-	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK,
-			   1 << ITAPCHGWIN_SHIFT);
-	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPDLYSEL_MASK,
-			   itapdly << ITAPDLYSEL_SHIFT);
-	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, 0);
+	sdhci_am654_write_itapdly(sdhci_am654, sdhci_am654->itap_del_sel[timing]);
 }
 
 static void sdhci_am654_setup_delay_chain(struct sdhci_am654_data *sdhci_am654,
@@ -298,7 +301,7 @@ 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) {
-		sdhci_am654_setup_dll(host, clock);
+		sdhci_am654_setup_dll(host, clock, timing);
 		sdhci_am654->dll_enable = true;
 	} else {
 		sdhci_am654_setup_delay_chain(sdhci_am654, timing);
-- 
2.34.1

[RFC PATCH 03/13] drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable

[Judith Mendez]

Currently the OTAP/ITAP delay enable functionality is missing in
the am654_set_clock function which is used for MMC0 on AM62p
and AM64x devices. The OTAP delay is not enabled when timing <
SDR25 bus speed mode. The ITAP delay is not enabled for all bus
speed modes.

Add this OTAP/ITAP delay functionality according to the datasheet
[1] OTAPDLYENA and ITAPDLYENA for MMC0.

[1] https://www.ti.com/lit/ds/symlink/am62p.pdf

Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/mmc/host/sdhci_am654.c | 48 +++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index 0a1ed2ae2eef..35e02f4128a7 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -143,6 +143,7 @@ struct sdhci_am654_data {
 	struct regmap *base;
 	int otap_del_sel[ARRAY_SIZE(td)];
 	int itap_del_sel[ARRAY_SIZE(td)];
+	u8 itap_del_ena[ARRAY_SIZE(td)];
 	int clkbuf_sel;
 	int trm_icp;
 	int drv_strength;
@@ -171,11 +172,13 @@ struct sdhci_am654_driver_data {
 };
 
 static void sdhci_am654_write_itapdly(struct sdhci_am654_data *sdhci_am654,
-				      u32 itapdly)
+				      u32 itapdly, u32 enable)
 {
 	/* Set ITAPCHGWIN before writing to ITAPDLY */
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK,
 			   0x1 << ITAPCHGWIN_SHIFT);
+	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPDLYENA_MASK,
+			   enable << ITAPDLYENA_SHIFT);
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPDLYSEL_MASK,
 			   itapdly << ITAPDLYSEL_SHIFT);
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, 0);
@@ -249,7 +252,8 @@ static void sdhci_am654_setup_dll(struct sdhci_host *host, unsigned int clock,
 		return;
 	}
 
-	sdhci_am654_write_itapdly(sdhci_am654, sdhci_am654->itap_del_sel[timing]);
+	sdhci_am654_write_itapdly(sdhci_am654, sdhci_am654->itap_del_sel[timing],
+				  sdhci_am654->itap_del_ena[timing]);
 }
 
 static void sdhci_am654_setup_delay_chain(struct sdhci_am654_data *sdhci_am654,
@@ -263,8 +267,8 @@ static void sdhci_am654_setup_delay_chain(struct sdhci_am654_data *sdhci_am654,
 	mask = SELDLYTXCLK_MASK | SELDLYRXCLK_MASK;
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL5, mask, val);
 
-	sdhci_am654_write_itapdly(sdhci_am654,
-				  sdhci_am654->itap_del_sel[timing]);
+	sdhci_am654_write_itapdly(sdhci_am654, sdhci_am654->itap_del_sel[timing],
+				  sdhci_am654->itap_del_ena[timing]);
 }
 
 static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
@@ -273,20 +277,17 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
 	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
 	unsigned char timing = host->mmc->ios.timing;
 	u32 otap_del_sel;
-	u32 otap_del_ena;
 	u32 mask, val;
 
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK, 0);
 
 	sdhci_set_clock(host, clock);
 
-	/* Setup DLL Output TAP delay */
+	/* Setup Output TAP delay */
 	otap_del_sel = sdhci_am654->otap_del_sel[timing];
-	otap_del_ena = (timing > MMC_TIMING_UHS_SDR25) ? 1 : 0;
 
 	mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
-	val = (otap_del_ena << OTAPDLYENA_SHIFT) |
-	      (otap_del_sel << OTAPDLYSEL_SHIFT);
+	val = (0x1 << OTAPDLYENA_SHIFT) | (otap_del_sel << OTAPDLYSEL_SHIFT);
 
 	/* Write to STRBSEL for HS400 speed mode */
 	if (timing == MMC_TIMING_MMC_HS400) {
@@ -319,14 +320,20 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
 	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
 	unsigned char timing = host->mmc->ios.timing;
 	u32 otap_del_sel;
+	u32 itap_del_ena;
 	u32 mask, val;
 
-	/* Setup DLL Output TAP delay */
+	/* Setup Output TAP delay */
 	otap_del_sel = sdhci_am654->otap_del_sel[timing];
 
 	mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
-	val = (0x1 << OTAPDLYENA_SHIFT) |
-	      (otap_del_sel << OTAPDLYSEL_SHIFT);
+	val = (0x1 << OTAPDLYENA_SHIFT) | (otap_del_sel << OTAPDLYSEL_SHIFT);
+
+	itap_del_ena = sdhci_am654->itap_del_ena[timing];
+
+	mask |= ITAPDLYENA_MASK;
+	val |= (itap_del_ena << ITAPDLYENA_SHIFT);
+
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
 
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL5, CLKBUFSEL_MASK,
@@ -503,12 +510,8 @@ static int sdhci_am654_platform_execute_tuning(struct sdhci_host *host,
 	for (i = 0; i < ITAPDLY_LENGTH; i++)
 		memset(&fail_window[i], 0, sizeof(fail_window[0]));
 
-	/* Enable ITAPDLY */
-	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPDLYENA_MASK,
-			   1 << ITAPDLYENA_SHIFT);
-
 	for (itap = 0; itap < ITAPDLY_LENGTH; itap++) {
-		sdhci_am654_write_itapdly(sdhci_am654, itap);
+		sdhci_am654_write_itapdly(sdhci_am654, itap, 1);
 
 		curr_pass = !mmc_send_tuning(host->mmc, opcode, NULL);
 
@@ -532,7 +535,7 @@ static int sdhci_am654_platform_execute_tuning(struct sdhci_host *host,
 	itap = calculate_itap(host, &fail_window[0], fail_index,
 			      (sdhci_am654->dll_enable ? true : false));
 
-	sdhci_am654_write_itapdly(sdhci_am654, itap);
+	sdhci_am654_write_itapdly(sdhci_am654, itap, 1);
 
 	return 0;
 }
@@ -681,9 +684,12 @@ static int sdhci_am654_get_otap_delay(struct sdhci_host *host,
 				host->mmc->caps2 &= ~td[i].capability;
 		}
 
-		if (td[i].itap_binding)
-			device_property_read_u32(dev, td[i].itap_binding,
-						 &sdhci_am654->itap_del_sel[i]);
+		if (td[i].itap_binding) {
+			ret = device_property_read_u32(dev, td[i].itap_binding,
+						       &sdhci_am654->itap_del_sel[i]);
+				if (!ret)
+					sdhci_am654->itap_del_ena[i] = 0x1;
+		}
 	}
 
 	return 0;
-- 
2.34.1

[RFC PATCH 04/13] drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in sdhci_j721e_4bit_set_clock

[Judith Mendez]

Add ITAPDLYSEL to sdhci_j721e_4bit_set_clock function.
This allows to set the correct ITAPDLY for timings that
do not carry out tuning.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/mmc/host/sdhci_am654.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index 35e02f4128a7..61f95aad3f80 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -321,6 +321,7 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
 	unsigned char timing = host->mmc->ios.timing;
 	u32 otap_del_sel;
 	u32 itap_del_ena;
+	u32 itap_del_sel;
 	u32 mask, val;
 
 	/* Setup Output TAP delay */
@@ -329,12 +330,17 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
 	mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
 	val = (0x1 << OTAPDLYENA_SHIFT) | (otap_del_sel << OTAPDLYSEL_SHIFT);
 
+	/* Setup Input TAP delay */
 	itap_del_ena = sdhci_am654->itap_del_ena[timing];
+	itap_del_sel = sdhci_am654->itap_del_sel[timing];
 
-	mask |= ITAPDLYENA_MASK;
-	val |= (itap_del_ena << ITAPDLYENA_SHIFT);
+	mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK;
+	val |= (itap_del_ena << ITAPDLYENA_SHIFT) | (itap_del_sel << ITAPDLYSEL_SHIFT);
 
+	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK,
+			   1 << ITAPCHGWIN_SHIFT);
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
+	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, 0);
 
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL5, CLKBUFSEL_MASK,
 			   sdhci_am654->clkbuf_sel);
-- 
2.34.1

[RFC PATCH 05/13] drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing

[Judith Mendez]

While STRB is currently used for DATA and CRC responses, the CMD
responses from the device to the host still require ITAPDLY for
HS400 timing.

Currently what is stored for HS400 is the ITAPDLY from High Speed
mode which is incorrect. The ITAPDLY for HS400 speed mode should
be the same as ITAPDLY as HS200 timing after tuning is executed.
Add the functionality to save ITAPDLY from HS200 tuning and save
as HS400 ITAPDLY.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/mmc/host/sdhci_am654.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index 61f95aad3f80..0f0178936a6d 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -151,6 +151,7 @@ struct sdhci_am654_data {
 	u32 flags;
 	u32 quirks;
 	bool dll_enable;
+	bool hs200_tunning;
 
 #define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
 };
@@ -252,6 +253,10 @@ static void sdhci_am654_setup_dll(struct sdhci_host *host, unsigned int clock,
 		return;
 	}
 
+	/* HS400 ITAPDLY should be the same as HS200 ITAPDLY*/
+	if (timing == MMC_TIMING_MMC_HS400)
+		sdhci_am654->itap_del_sel[timing] = sdhci_am654->itap_del_sel[timing - 1];
+
 	sdhci_am654_write_itapdly(sdhci_am654, sdhci_am654->itap_del_sel[timing],
 				  sdhci_am654->itap_del_ena[timing]);
 }
@@ -311,6 +316,9 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
 
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL5, CLKBUFSEL_MASK,
 			   sdhci_am654->clkbuf_sel);
+
+	if (timing == MMC_TIMING_MMC_HS200 && sdhci_am654->dll_enable)
+		sdhci_am654->hs200_tunning = true;
 }
 
 static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
@@ -543,6 +551,10 @@ static int sdhci_am654_platform_execute_tuning(struct sdhci_host *host,
 
 	sdhci_am654_write_itapdly(sdhci_am654, itap, 1);
 
+	/* Save ITAPDLY for HS200 */
+	if (sdhci_am654->hs200_tunning)
+		sdhci_am654->itap_del_sel[MMC_TIMING_MMC_HS200] = itap;
+
 	return 0;
 }
 
-- 
2.34.1

[RFC PATCH 06/13] arm64: dts: ti: k3-am62a-main: Add sdhci0 instance

[Judith Mendez]

From: Nitin Yadav <n-yadav@ti.com>

Add sdhci0 DT node in k3-am62a-main for eMMC support. Add otap/itap
values according to the datasheet[0], Refer to Table 7-79.

[0] https://www.ti.com/lit/ds/symlink/am62a3.pdf

Signed-off-by: Judith Mendez <jm@ti.com>
Signed-off-by: Nitin Yadav <n-yadav@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62a-main.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
index f0b8c9ab1459..523dee78123a 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
@@ -536,6 +536,24 @@ main_gpio1: gpio@601000 {
 		status = "disabled";
 	};
 
+	sdhci0: mmc@fa10000 {
+		compatible = "ti,am62-sdhci";
+		reg = <0x00 0xfa10000 0x00 0x260>, <0x00 0xfa18000 0x00 0x134>;
+		interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
+		power-domains = <&k3_pds 57 TI_SCI_PD_EXCLUSIVE>;
+		clocks = <&k3_clks 57 5>, <&k3_clks 57 6>;
+		clock-names = "clk_ahb", "clk_xin";
+		assigned-clocks = <&k3_clks 57 6>;
+		assigned-clock-parents = <&k3_clks 57 8>;
+		bus-width = <8>;
+		mmc-hs200-1_8v;
+		ti,clkbuf-sel = <0x7>;
+		ti,otap-del-sel-legacy = <0x0>;
+		ti,otap-del-sel-mmc-hs = <0x0>;
+		ti,otap-del-sel-hs200 = <0x6>;
+		status = "disabled";
+	};
+
 	sdhci1: mmc@fa00000 {
 		compatible = "ti,am62-sdhci";
 		reg = <0x00 0xfa00000 0x00 0x260>, <0x00 0xfa08000 0x00 0x134>;
-- 
2.34.1

Re: [RFC PATCH 06/13] arm64: dts: ti: k3-am62a-main: Add sdhci0 instance

[Nishanth Menon]

On 18:37-20240130, Judith Mendez wrote:
> From: Nitin Yadav <n-yadav@ti.com>
> 
> Add sdhci0 DT node in k3-am62a-main for eMMC support. Add otap/itap
> values according to the datasheet[0], Refer to Table 7-79.
> 
> [0] https://www.ti.com/lit/ds/symlink/am62a3.pdf
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> Signed-off-by: Nitin Yadav <n-yadav@ti.com>
> ---

Side note: will appreciate if the dt patches come via the SoC dt tree
for TI K3 and not via mmc tree.

>  arch/arm64/boot/dts/ti/k3-am62a-main.dtsi | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
> index f0b8c9ab1459..523dee78123a 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
> @@ -536,6 +536,24 @@ main_gpio1: gpio@601000 {
>  		status = "disabled";
>  	};
>  
> +	sdhci0: mmc@fa10000 {
> +		compatible = "ti,am62-sdhci";
> +		reg = <0x00 0xfa10000 0x00 0x260>, <0x00 0xfa18000 0x00 0x134>;
> +		interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
> +		power-domains = <&k3_pds 57 TI_SCI_PD_EXCLUSIVE>;
> +		clocks = <&k3_clks 57 5>, <&k3_clks 57 6>;
> +		clock-names = "clk_ahb", "clk_xin";
> +		assigned-clocks = <&k3_clks 57 6>;
> +		assigned-clock-parents = <&k3_clks 57 8>;
> +		bus-width = <8>;
> +		mmc-hs200-1_8v;
> +		ti,clkbuf-sel = <0x7>;
> +		ti,otap-del-sel-legacy = <0x0>;
> +		ti,otap-del-sel-mmc-hs = <0x0>;
> +		ti,otap-del-sel-hs200 = <0x6>;
> +		status = "disabled";
> +	};
> +
>  	sdhci1: mmc@fa00000 {
>  		compatible = "ti,am62-sdhci";
>  		reg = <0x00 0xfa00000 0x00 0x260>, <0x00 0xfa08000 0x00 0x134>;
> -- 
> 2.34.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [RFC PATCH 06/13] arm64: dts: ti: k3-am62a-main: Add sdhci0 instance

[Judith Mendez]

On 1/31/24 1:17 PM, Nishanth Menon wrote:
> On 18:37-20240130, Judith Mendez wrote:
>> From: Nitin Yadav <n-yadav@ti.com>
>>
>> Add sdhci0 DT node in k3-am62a-main for eMMC support. Add otap/itap
>> values according to the datasheet[0], Refer to Table 7-79.
>>
>> [0] https://www.ti.com/lit/ds/symlink/am62a3.pdf
>>
>> Signed-off-by: Judith Mendez <jm@ti.com>
>> Signed-off-by: Nitin Yadav <n-yadav@ti.com>
>> ---
> 
> Side note: will appreciate if the dt patches come via the SoC dt tree
> for TI K3 and not via mmc tree.

Will use DO NOT MERGE for v1, thanks.

> 
>>   arch/arm64/boot/dts/ti/k3-am62a-main.dtsi | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
>> index f0b8c9ab1459..523dee78123a 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
>> @@ -536,6 +536,24 @@ main_gpio1: gpio@601000 {
>>   		status = "disabled";
>>   	};
>>   
>> +	sdhci0: mmc@fa10000 {
>> +		compatible = "ti,am62-sdhci";
>> +		reg = <0x00 0xfa10000 0x00 0x260>, <0x00 0xfa18000 0x00 0x134>;
>> +		interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
>> +		power-domains = <&k3_pds 57 TI_SCI_PD_EXCLUSIVE>;
>> +		clocks = <&k3_clks 57 5>, <&k3_clks 57 6>;
>> +		clock-names = "clk_ahb", "clk_xin";
>> +		assigned-clocks = <&k3_clks 57 6>;
>> +		assigned-clock-parents = <&k3_clks 57 8>;
>> +		bus-width = <8>;
>> +		mmc-hs200-1_8v;
>> +		ti,clkbuf-sel = <0x7>;
>> +		ti,otap-del-sel-legacy = <0x0>;
>> +		ti,otap-del-sel-mmc-hs = <0x0>;
>> +		ti,otap-del-sel-hs200 = <0x6>;
>> +		status = "disabled";
>> +	};
>> +
>>   	sdhci1: mmc@fa00000 {
>>   		compatible = "ti,am62-sdhci";
>>   		reg = <0x00 0xfa00000 0x00 0x260>, <0x00 0xfa08000 0x00 0x134>;
>> -- 
>> 2.34.1
>>
> 

[RFC PATCH 07/13] arm64: dts: ti: k3-am62a7-sk: Enable eMMC support

[Judith Mendez]

From: Nitin Yadav <n-yadav@ti.com>

Add support for 32GB eMMC card on AM62A7 SK. Includes adding mmc0
pins settings. Add mmc0 alias for sdhci0 in k3-am62a7-sk.dts.

Signed-off-by: Judith Mendez <jm@ti.com>
Signed-off-by: Nitin Yadav <n-yadav@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 26 +++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
index 7b7142586295..2497acc0dd60 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
@@ -20,6 +20,7 @@ aliases {
 		serial0 = &wkup_uart0;
 		serial2 = &main_uart0;
 		serial3 = &main_uart1;
+		mmc0 = &sdhci0;
 		mmc1 = &sdhci1;
 	};
 
@@ -218,6 +219,22 @@ AM62AX_IOPAD(0x0b4, PIN_INPUT_PULLUP, 1) /* (K24) GPMC0_CSn3.I2C2_SDA */
 		>;
 	};
 
+	main_mmc0_pins_default: main-mmc0-default-pins {
+		pinctrl-single,pins = <
+			AM62AX_IOPAD(0x220, PIN_INPUT, 0) /* (Y3) MMC0_CMD */
+			AM62AX_IOPAD(0x218, PIN_INPUT, 0) /* (AB1) MMC0_CLKLB */
+			AM62AX_IOPAD(0x21c, PIN_INPUT, 0) /* (AB1) MMC0_CLK */
+			AM62AX_IOPAD(0x214, PIN_INPUT, 0) /* (AA2) MMC0_DAT0 */
+			AM62AX_IOPAD(0x210, PIN_INPUT_PULLUP, 0) /* (AA1) MMC0_DAT1 */
+			AM62AX_IOPAD(0x20c, PIN_INPUT_PULLUP, 0) /* (AA3) MMC0_DAT2 */
+			AM62AX_IOPAD(0x208, PIN_INPUT_PULLUP, 0) /* (Y4) MMC0_DAT3 */
+			AM62AX_IOPAD(0x204, PIN_INPUT_PULLUP, 0) /* (AB2) MMC0_DAT4 */
+			AM62AX_IOPAD(0x200, PIN_INPUT_PULLUP, 0) /* (AC1) MMC0_DAT5 */
+			AM62AX_IOPAD(0x1fc, PIN_INPUT_PULLUP, 0) /* (AD2) MMC0_DAT6 */
+			AM62AX_IOPAD(0x1f8, PIN_INPUT_PULLUP, 0) /* (AC2) MMC0_DAT7 */
+		>;
+	};
+
 	main_mmc1_pins_default: main-mmc1-default-pins {
 		pinctrl-single,pins = <
 			AM62AX_IOPAD(0x23c, PIN_INPUT, 0) /* (A21) MMC1_CMD */
@@ -475,6 +492,15 @@ &main_i2c2 {
 	clock-frequency = <400000>;
 };
 
+&sdhci0 {
+	/* eMMC */
+	status = "okay";
+	non-removable;
+	pinctrl-names = "default";
+	pinctrl-0 = <&main_mmc0_pins_default>;
+	disable-wp;
+};
+
 &sdhci1 {
 	/* SD/MMC */
 	status = "okay";
-- 
2.34.1

[RFC PATCH 08/13] arm64: dts: ti: k3-am62a-main: Add sdhci2 instance

[Judith Mendez]

Add sdhci2 DT node in k3-am62a-main for mmc2. Add otap/itap
values according to the datasheet[0], Refer to Table 7-97.

[0] https://www.ti.com/lit/ds/symlink/am62a3.pdf

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62a-main.dtsi | 24 +++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
index 523dee78123a..227e7fb3e5bb 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
@@ -579,6 +579,30 @@ sdhci1: mmc@fa00000 {
 		status = "disabled";
 	};
 
+	sdhci2: mmc@fa00000 {
+		compatible = "ti,am62-sdhci";
+		reg = <0x00 0xfa20000 0x00 0x260>, <0x00 0xfa28000 0x00 0x134>;
+		interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+		power-domains = <&k3_pds 184 TI_SCI_PD_EXCLUSIVE>;
+		clocks = <&k3_clks 184 5>, <&k3_clks 184 6>;
+		clock-names = "clk_ahb", "clk_xin";
+		bus-width = <4>;
+		ti,clkbuf-sel = <0x7>;
+		ti,otap-del-sel-legacy = <0x0>;
+		ti,otap-del-sel-sd-hs = <0x0>;
+		ti,otap-del-sel-sdr12 = <0xf>;
+		ti,otap-del-sel-sdr25 = <0xf>;
+		ti,otap-del-sel-sdr50 = <0xc>;
+		ti,otap-del-sel-sdr104 = <0x6>;
+		ti,otap-del-sel-ddr50 = <0x9>;
+		ti,itap-del-sel-legacy = <0x0>;
+		ti,itap-del-sel-sd-hs = <0x0>;
+		ti,itap-del-sel-sdr12 = <0x0>;
+		ti,itap-del-sel-sdr25 = <0x0>;
+		no-1-8-v;
+		status = "disabled";
+	};
+
 	usbss0: dwc3-usb@f900000 {
 		compatible = "ti,am62-usb";
 		reg = <0x00 0x0f900000 0x00 0x800>;
-- 
2.34.1

[RFC PATCH 09/13] arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC

[Judith Mendez]

Update MMC0/MMC1 OTAP/ITAP values according to the datasheet
[0], refer to Table 7-68 for MMC0 and Table 7-77 for MMC1.

[0] https://www.ti.com/lit/ds/symlink/am6442.pdf

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am64-main.dtsi | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
index e348114f42e0..b355d0a715a9 100644
--- a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
@@ -633,6 +633,9 @@ sdhci0: mmc@fa10000 {
 		ti,otap-del-sel-mmc-hs = <0x0>;
 		ti,otap-del-sel-ddr52 = <0x6>;
 		ti,otap-del-sel-hs200 = <0x7>;
+		ti,itap-del-sel-legacy = <0x10>;
+		ti,itap-del-sel-mmc-hs = <0xa>;
+		ti,itap-del-sel-ddr52 = <0x3>;
 		status = "disabled";
 	};
 
@@ -645,13 +648,17 @@ sdhci1: mmc@fa00000 {
 		clock-names = "clk_ahb", "clk_xin";
 		ti,trm-icp = <0x2>;
 		ti,otap-del-sel-legacy = <0x0>;
-		ti,otap-del-sel-sd-hs = <0xf>;
+		ti,otap-del-sel-sd-hs = <0x0>;
 		ti,otap-del-sel-sdr12 = <0xf>;
 		ti,otap-del-sel-sdr25 = <0xf>;
 		ti,otap-del-sel-sdr50 = <0xc>;
 		ti,otap-del-sel-sdr104 = <0x6>;
 		ti,otap-del-sel-ddr50 = <0x9>;
 		ti,clkbuf-sel = <0x7>;
+		ti,itap-del-sel-legacy = <0x0>;
+		ti,itap-del-sel-sd-hs = <0x0>;
+		ti,itap-del-sel-sdr12 = <0x0>;
+		ti,itap-del-sel-sdr25 = <0x0>;
 		status = "disabled";
 	};
 
-- 
2.34.1

[RFC PATCH 10/13] arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC

[Judith Mendez]

Update MMC0/MMC1 OTAP/ITAP values according to the datasheet
[0], refer to Table 7-79 for MMC0 and Table 7-97 for MMC1.

[0] https://www.ti.com/lit/ds/symlink/am625.pdf

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 49 +++++++++++-------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
index 464b7565d085..d10a02ede06c 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
@@ -566,10 +566,7 @@ sdhci0: mmc@fa10000 {
 		ti,clkbuf-sel = <0x7>;
 		ti,otap-del-sel-legacy = <0x0>;
 		ti,otap-del-sel-mmc-hs = <0x0>;
-		ti,otap-del-sel-ddr52 = <0x5>;
-		ti,otap-del-sel-hs200 = <0x5>;
-		ti,itap-del-sel-legacy = <0xa>;
-		ti,itap-del-sel-mmc-hs = <0x1>;
+		ti,otap-del-sel-hs200 = <0x6>;
 		status = "disabled";
 	};
 
@@ -581,19 +578,19 @@ sdhci1: mmc@fa00000 {
 		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
 		clock-names = "clk_ahb", "clk_xin";
 		ti,trm-icp = <0x2>;
-		ti,otap-del-sel-legacy = <0x8>;
-		ti,otap-del-sel-sd-hs = <0x0>;
-		ti,otap-del-sel-sdr12 = <0x0>;
-		ti,otap-del-sel-sdr25 = <0x0>;
-		ti,otap-del-sel-sdr50 = <0x8>;
-		ti,otap-del-sel-sdr104 = <0x7>;
-		ti,otap-del-sel-ddr50 = <0x4>;
-		ti,itap-del-sel-legacy = <0xa>;
-		ti,itap-del-sel-sd-hs = <0x1>;
-		ti,itap-del-sel-sdr12 = <0xa>;
-		ti,itap-del-sel-sdr25 = <0x1>;
 		ti,clkbuf-sel = <0x7>;
 		bus-width = <4>;
+		ti,otap-del-sel-legacy = <0x0>;
+		ti,otap-del-sel-sd-hs = <0x0>;
+		ti,otap-del-sel-sdr12 = <0xf>;
+		ti,otap-del-sel-sdr25 = <0xf>;
+		ti,otap-del-sel-sdr50 = <0xc>;
+		ti,otap-del-sel-sdr104 = <0x6>;
+		ti,otap-del-sel-ddr50 = <0x9>;
+		ti,itap-del-sel-legacy = <0x0>;
+		ti,itap-del-sel-sd-hs = <0x0>;
+		ti,itap-del-sel-sdr12 = <0x0>;
+		ti,itap-del-sel-sdr25 = <0x0>;
 		status = "disabled";
 	};
 
@@ -605,18 +602,18 @@ sdhci2: mmc@fa20000 {
 		clocks = <&k3_clks 184 5>, <&k3_clks 184 6>;
 		clock-names = "clk_ahb", "clk_xin";
 		ti,trm-icp = <0x2>;
-		ti,otap-del-sel-legacy = <0x8>;
-		ti,otap-del-sel-sd-hs = <0x0>;
-		ti,otap-del-sel-sdr12 = <0x0>;
-		ti,otap-del-sel-sdr25 = <0x0>;
-		ti,otap-del-sel-sdr50 = <0x8>;
-		ti,otap-del-sel-sdr104 = <0x7>;
-		ti,otap-del-sel-ddr50 = <0x8>;
-		ti,itap-del-sel-legacy = <0xa>;
-		ti,itap-del-sel-sd-hs = <0xa>;
-		ti,itap-del-sel-sdr12 = <0xa>;
-		ti,itap-del-sel-sdr25 = <0x1>;
 		ti,clkbuf-sel = <0x7>;
+		ti,otap-del-sel-legacy = <0x0>;
+		ti,otap-del-sel-sd-hs = <0x0>;
+		ti,otap-del-sel-sdr12 = <0xf>;
+		ti,otap-del-sel-sdr25 = <0xf>;
+		ti,otap-del-sel-sdr50 = <0xc>;
+		ti,otap-del-sel-sdr104 = <0x6>;
+		ti,otap-del-sel-ddr50 = <0x9>;
+		ti,itap-del-sel-legacy = <0x0>;
+		ti,itap-del-sel-sd-hs = <0x0>;
+		ti,itap-del-sel-sdr12 = <0x0>;
+		ti,itap-del-sel-sdr25 = <0x0>;
 		status = "disabled";
 	};
 
-- 
2.34.1

[RFC PATCH 11/13] arm64: dts: ti: k3-am62p: Add missing properties for MMC

[Judith Mendez]

Add missing properties to enable HS200 timing for MMC0 and
SDR104 timing for MMC1 according to the datasheet [0] for
AM62p device, refer to Table 7-79 for MMC0 and Table 7-97
for MMC1/MMC2.

[0] https://www.ti.com/lit/ds/symlink/am625.pdf

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62p-main.dtsi | 44 +++++++++++++++++++++--
 arch/arm64/boot/dts/ti/k3-am62p5-sk.dts   |  6 ++--
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
index 4c51bae06b57..f743700dd5bd 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
@@ -534,7 +534,21 @@ sdhci0: mmc@fa10000 {
 		clock-names = "clk_ahb", "clk_xin";
 		assigned-clocks = <&k3_clks 57 2>;
 		assigned-clock-parents = <&k3_clks 57 4>;
-		ti,otap-del-sel-legacy = <0x0>;
+		bus-width = <8>;
+		mmc-ddr-1_8v;
+		mmc-hs200-1_8v;
+		mmc-hs400-1_8v;
+		ti,clkbuf-sel = <0x7>;
+		ti,strobe-sel = <0x77>;
+		ti,trm-icp = <0x8>;
+		ti,otap-del-sel-legacy = <0x1>;
+		ti,otap-del-sel-mmc-hs = <0x1>;
+		ti,otap-del-sel-ddr52 = <0x6>;
+		ti,otap-del-sel-hs200 = <0x8>;
+		ti,otap-del-sel-hs400 = <0x5>;
+		ti,itap-del-sel-legacy = <0x10>;
+		ti,itap-del-sel-mmc-hs = <0xa>;
+		ti,itap-del-sel-ddr52 = <0x3>;
 		status = "disabled";
 	};
 
@@ -545,7 +559,19 @@ sdhci1: mmc@fa00000 {
 		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
 		clock-names = "clk_ahb", "clk_xin";
-		ti,otap-del-sel-legacy = <0x8>;
+		bus-width = <4>;
+		ti,clkbuf-sel = <0x7>;
+		ti,otap-del-sel-legacy = <0x0>;
+		ti,otap-del-sel-sd-hs = <0x0>;
+		ti,otap-del-sel-sdr12 = <0xf>;
+		ti,otap-del-sel-sdr25 = <0xf>;
+		ti,otap-del-sel-sdr50 = <0xc>;
+		ti,otap-del-sel-ddr50 = <0x9>;
+		ti,otap-del-sel-sdr104 = <0x6>;
+		ti,itap-del-sel-legacy = <0x0>;
+		ti,itap-del-sel-sd-hs = <0x0>;
+		ti,itap-del-sel-sdr12 = <0x0>;
+		ti,itap-del-sel-sdr25 = <0x0>;
 		status = "disabled";
 	};
 
@@ -556,7 +582,19 @@ sdhci2: mmc@fa20000 {
 		power-domains = <&k3_pds 184 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 184 5>, <&k3_clks 184 6>;
 		clock-names = "clk_ahb", "clk_xin";
-		ti,otap-del-sel-legacy = <0x8>;
+		bus-width = <4>;
+		ti,clkbuf-sel = <0x7>;
+		ti,otap-del-sel-legacy = <0x0>;
+		ti,otap-del-sel-sd-hs = <0x0>;
+		ti,otap-del-sel-sdr12 = <0xf>;
+		ti,otap-del-sel-sdr25 = <0xf>;
+		ti,otap-del-sel-sdr50 = <0xc>;
+		ti,otap-del-sel-ddr50 = <0x9>;
+		ti,otap-del-sel-sdr104 = <0x6>;
+		ti,itap-del-sel-legacy = <0x0>;
+		ti,itap-del-sel-sd-hs = <0x0>;
+		ti,itap-del-sel-sdr12 = <0x0>;
+		ti,itap-del-sel-sdr25 = <0x0>;
 		status = "disabled";
 	};
 
diff --git a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
index 1773c05f752c..10156a04a92c 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
@@ -410,13 +410,17 @@ &main_i2c2 {
 };
 
 &sdhci0 {
+	/* eMMC */
+	bootph-all;
 	status = "okay";
+	non-removable;
 	ti,driver-strength-ohm = <50>;
 	disable-wp;
 };
 
 &sdhci1 {
 	/* SD/MMC */
+	bootph-all;
 	status = "okay";
 	vmmc-supply = <&vdd_mmc1>;
 	vqmmc-supply = <&vddshv_sdio>;
@@ -424,8 +428,6 @@ &sdhci1 {
 	pinctrl-0 = <&main_mmc1_pins_default>;
 	ti,driver-strength-ohm = <50>;
 	disable-wp;
-	no-1-8-v;
-	bootph-all;
 };
 
 &cpsw3g {
-- 
2.34.1

Re: [RFC PATCH 11/13] arm64: dts: ti: k3-am62p: Add missing properties for MMC

[Roger Quadros]

On 31/01/2024 02:37, Judith Mendez wrote:
> Add missing properties to enable HS200 timing for MMC0 and
> SDR104 timing for MMC1 according to the datasheet [0] for
> AM62p device, refer to Table 7-79 for MMC0 and Table 7-97
> for MMC1/MMC2.
> 
> [0] https://www.ti.com/lit/ds/symlink/am625.pdf
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>  arch/arm64/boot/dts/ti/k3-am62p-main.dtsi | 44 +++++++++++++++++++++--
>  arch/arm64/boot/dts/ti/k3-am62p5-sk.dts   |  6 ++--
>  2 files changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> index 4c51bae06b57..f743700dd5bd 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> @@ -534,7 +534,21 @@ sdhci0: mmc@fa10000 {
>  		clock-names = "clk_ahb", "clk_xin";
>  		assigned-clocks = <&k3_clks 57 2>;
>  		assigned-clock-parents = <&k3_clks 57 4>;
> -		ti,otap-del-sel-legacy = <0x0>;
> +		bus-width = <8>;
> +		mmc-ddr-1_8v;
> +		mmc-hs200-1_8v;
> +		mmc-hs400-1_8v;
> +		ti,clkbuf-sel = <0x7>;
> +		ti,strobe-sel = <0x77>;
> +		ti,trm-icp = <0x8>;
> +		ti,otap-del-sel-legacy = <0x1>;
> +		ti,otap-del-sel-mmc-hs = <0x1>;
> +		ti,otap-del-sel-ddr52 = <0x6>;
> +		ti,otap-del-sel-hs200 = <0x8>;
> +		ti,otap-del-sel-hs400 = <0x5>;
> +		ti,itap-del-sel-legacy = <0x10>;
> +		ti,itap-del-sel-mmc-hs = <0xa>;
> +		ti,itap-del-sel-ddr52 = <0x3>;
>  		status = "disabled";
>  	};
>  
> @@ -545,7 +559,19 @@ sdhci1: mmc@fa00000 {
>  		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
>  		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
>  		clock-names = "clk_ahb", "clk_xin";
> -		ti,otap-del-sel-legacy = <0x8>;
> +		bus-width = <4>;
> +		ti,clkbuf-sel = <0x7>;
> +		ti,otap-del-sel-legacy = <0x0>;
> +		ti,otap-del-sel-sd-hs = <0x0>;
> +		ti,otap-del-sel-sdr12 = <0xf>;
> +		ti,otap-del-sel-sdr25 = <0xf>;
> +		ti,otap-del-sel-sdr50 = <0xc>;
> +		ti,otap-del-sel-ddr50 = <0x9>;
> +		ti,otap-del-sel-sdr104 = <0x6>;
> +		ti,itap-del-sel-legacy = <0x0>;
> +		ti,itap-del-sel-sd-hs = <0x0>;
> +		ti,itap-del-sel-sdr12 = <0x0>;
> +		ti,itap-del-sel-sdr25 = <0x0>;
>  		status = "disabled";
>  	};
>  
> @@ -556,7 +582,19 @@ sdhci2: mmc@fa20000 {
>  		power-domains = <&k3_pds 184 TI_SCI_PD_EXCLUSIVE>;
>  		clocks = <&k3_clks 184 5>, <&k3_clks 184 6>;
>  		clock-names = "clk_ahb", "clk_xin";
> -		ti,otap-del-sel-legacy = <0x8>;
> +		bus-width = <4>;
> +		ti,clkbuf-sel = <0x7>;
> +		ti,otap-del-sel-legacy = <0x0>;
> +		ti,otap-del-sel-sd-hs = <0x0>;
> +		ti,otap-del-sel-sdr12 = <0xf>;
> +		ti,otap-del-sel-sdr25 = <0xf>;
> +		ti,otap-del-sel-sdr50 = <0xc>;
> +		ti,otap-del-sel-ddr50 = <0x9>;
> +		ti,otap-del-sel-sdr104 = <0x6>;
> +		ti,itap-del-sel-legacy = <0x0>;
> +		ti,itap-del-sel-sd-hs = <0x0>;
> +		ti,itap-del-sel-sdr12 = <0x0>;
> +		ti,itap-del-sel-sdr25 = <0x0>;
>  		status = "disabled";
>  	};
>  
> diff --git a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
> index 1773c05f752c..10156a04a92c 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
> @@ -410,13 +410,17 @@ &main_i2c2 {
>  };
>  
>  &sdhci0 {
> +	/* eMMC */
> +	bootph-all;
>  	status = "okay";
> +	non-removable;
>  	ti,driver-strength-ohm = <50>;
>  	disable-wp;
>  };
>  
>  &sdhci1 {
>  	/* SD/MMC */
> +	bootph-all;
>  	status = "okay";
>  	vmmc-supply = <&vdd_mmc1>;
>  	vqmmc-supply = <&vddshv_sdio>;
> @@ -424,8 +428,6 @@ &sdhci1 {
>  	pinctrl-0 = <&main_mmc1_pins_default>;
>  	ti,driver-strength-ohm = <50>;
>  	disable-wp;
> -	no-1-8-v;

Why was 'no-1-8-v' removed?

> -	bootph-all;
>  };
>  
>  &cpsw3g {

-- 
cheers,
-roger

Re: [RFC PATCH 11/13] arm64: dts: ti: k3-am62p: Add missing properties for MMC

[Judith Mendez]

Hi Roger,

On 2/2/24 3:50 AM, Roger Quadros wrote:
> 
> 
> On 31/01/2024 02:37, Judith Mendez wrote:
>> Add missing properties to enable HS200 timing for MMC0 and
>> SDR104 timing for MMC1 according to the datasheet [0] for
>> AM62p device, refer to Table 7-79 for MMC0 and Table 7-97
>> for MMC1/MMC2.
>>
>> [0] https://www.ti.com/lit/ds/symlink/am625.pdf
>>
>> Signed-off-by: Judith Mendez <jm@ti.com>
>> ---
>>   arch/arm64/boot/dts/ti/k3-am62p-main.dtsi | 44 +++++++++++++++++++++--
>>   arch/arm64/boot/dts/ti/k3-am62p5-sk.dts   |  6 ++--
>>   2 files changed, 45 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
>> index 4c51bae06b57..f743700dd5bd 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
>> @@ -534,7 +534,21 @@ sdhci0: mmc@fa10000 {
>>   		clock-names = "clk_ahb", "clk_xin";
>>   		assigned-clocks = <&k3_clks 57 2>;
>>   		assigned-clock-parents = <&k3_clks 57 4>;
>> -		ti,otap-del-sel-legacy = <0x0>;
>> +		bus-width = <8>;
>> +		mmc-ddr-1_8v;
>> +		mmc-hs200-1_8v;
>> +		mmc-hs400-1_8v;
>> +		ti,clkbuf-sel = <0x7>;
>> +		ti,strobe-sel = <0x77>;
>> +		ti,trm-icp = <0x8>;
>> +		ti,otap-del-sel-legacy = <0x1>;
>> +		ti,otap-del-sel-mmc-hs = <0x1>;
>> +		ti,otap-del-sel-ddr52 = <0x6>;
>> +		ti,otap-del-sel-hs200 = <0x8>;
>> +		ti,otap-del-sel-hs400 = <0x5>;
>> +		ti,itap-del-sel-legacy = <0x10>;
>> +		ti,itap-del-sel-mmc-hs = <0xa>;
>> +		ti,itap-del-sel-ddr52 = <0x3>;
>>   		status = "disabled";
>>   	};
>>   
>> @@ -545,7 +559,19 @@ sdhci1: mmc@fa00000 {
>>   		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
>>   		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
>>   		clock-names = "clk_ahb", "clk_xin";
>> -		ti,otap-del-sel-legacy = <0x8>;
>> +		bus-width = <4>;
>> +		ti,clkbuf-sel = <0x7>;
>> +		ti,otap-del-sel-legacy = <0x0>;
>> +		ti,otap-del-sel-sd-hs = <0x0>;
>> +		ti,otap-del-sel-sdr12 = <0xf>;
>> +		ti,otap-del-sel-sdr25 = <0xf>;
>> +		ti,otap-del-sel-sdr50 = <0xc>;
>> +		ti,otap-del-sel-ddr50 = <0x9>;
>> +		ti,otap-del-sel-sdr104 = <0x6>;
>> +		ti,itap-del-sel-legacy = <0x0>;
>> +		ti,itap-del-sel-sd-hs = <0x0>;
>> +		ti,itap-del-sel-sdr12 = <0x0>;
>> +		ti,itap-del-sel-sdr25 = <0x0>;
>>   		status = "disabled";
>>   	};
>>   
>> @@ -556,7 +582,19 @@ sdhci2: mmc@fa20000 {
>>   		power-domains = <&k3_pds 184 TI_SCI_PD_EXCLUSIVE>;
>>   		clocks = <&k3_clks 184 5>, <&k3_clks 184 6>;
>>   		clock-names = "clk_ahb", "clk_xin";
>> -		ti,otap-del-sel-legacy = <0x8>;
>> +		bus-width = <4>;
>> +		ti,clkbuf-sel = <0x7>;
>> +		ti,otap-del-sel-legacy = <0x0>;
>> +		ti,otap-del-sel-sd-hs = <0x0>;
>> +		ti,otap-del-sel-sdr12 = <0xf>;
>> +		ti,otap-del-sel-sdr25 = <0xf>;
>> +		ti,otap-del-sel-sdr50 = <0xc>;
>> +		ti,otap-del-sel-ddr50 = <0x9>;
>> +		ti,otap-del-sel-sdr104 = <0x6>;
>> +		ti,itap-del-sel-legacy = <0x0>;
>> +		ti,itap-del-sel-sd-hs = <0x0>;
>> +		ti,itap-del-sel-sdr12 = <0x0>;
>> +		ti,itap-del-sel-sdr25 = <0x0>;
>>   		status = "disabled";
>>   	};
>>   
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
>> index 1773c05f752c..10156a04a92c 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
>> @@ -410,13 +410,17 @@ &main_i2c2 {
>>   };
>>   
>>   &sdhci0 {
>> +	/* eMMC */
>> +	bootph-all;
>>   	status = "okay";
>> +	non-removable;
>>   	ti,driver-strength-ohm = <50>;
>>   	disable-wp;
>>   };
>>   
>>   &sdhci1 {
>>   	/* SD/MMC */
>> +	bootph-all;
>>   	status = "okay";
>>   	vmmc-supply = <&vdd_mmc1>;
>>   	vqmmc-supply = <&vddshv_sdio>;
>> @@ -424,8 +428,6 @@ &sdhci1 {
>>   	pinctrl-0 = <&main_mmc1_pins_default>;
>>   	ti,driver-strength-ohm = <50>;
>>   	disable-wp;
>> -	no-1-8-v;
> 
> Why was 'no-1-8-v' removed?

To enable HS400 timing, the no-1-8-v property should
be removed. I add this comment in patch description for
v1.

Thanks for reviewing!

~ Judith

> 
>> -	bootph-all;
>>   };
>>   
>>   &cpsw3g {
> 

[RFC PATCH 12/13] arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys

[Judith Mendez]

Remove DLL properties which are not applicable for soft phys
since they do not have a DLL to enable.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62-main.dtsi       | 3 ---
 arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 3 ---
 arch/arm64/boot/dts/ti/k3-am62a-main.dtsi      | 1 -
 arch/arm64/boot/dts/ti/k3-am62a7-sk.dts        | 1 -
 arch/arm64/boot/dts/ti/k3-am62p5-sk.dts        | 1 -
 arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi | 2 --
 arch/arm64/boot/dts/ti/k3-am64-main.dtsi       | 1 -
 arch/arm64/boot/dts/ti/k3-am642-evm.dts        | 1 -
 arch/arm64/boot/dts/ti/k3-am642-sk.dts         | 1 -
 9 files changed, 14 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
index d10a02ede06c..ca825088970f 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
@@ -561,7 +561,6 @@ sdhci0: mmc@fa10000 {
 		assigned-clock-parents = <&k3_clks 57 8>;
 		mmc-ddr-1_8v;
 		mmc-hs200-1_8v;
-		ti,trm-icp = <0x2>;
 		bus-width = <8>;
 		ti,clkbuf-sel = <0x7>;
 		ti,otap-del-sel-legacy = <0x0>;
@@ -577,7 +576,6 @@ sdhci1: mmc@fa00000 {
 		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
 		clock-names = "clk_ahb", "clk_xin";
-		ti,trm-icp = <0x2>;
 		ti,clkbuf-sel = <0x7>;
 		bus-width = <4>;
 		ti,otap-del-sel-legacy = <0x0>;
@@ -601,7 +599,6 @@ sdhci2: mmc@fa20000 {
 		power-domains = <&k3_pds 184 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 184 5>, <&k3_clks 184 6>;
 		clock-names = "clk_ahb", "clk_xin";
-		ti,trm-icp = <0x2>;
 		ti,clkbuf-sel = <0x7>;
 		ti,otap-del-sel-legacy = <0x0>;
 		ti,otap-del-sel-sd-hs = <0x0>;
diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
index eadbdd9ffe37..f69dbf9b8406 100644
--- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
+++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
@@ -827,7 +827,6 @@ &sdhci0 {
 	bootph-all;
 	pinctrl-names = "default";
 	pinctrl-0 = <&emmc_pins_default>;
-	ti,driver-strength-ohm = <50>;
 	disable-wp;
 	status = "okay";
 };
@@ -840,7 +839,6 @@ &sdhci1 {
 
 	vmmc-supply = <&vdd_3v3_sd>;
 	vqmmc-supply = <&vdd_sd_dv>;
-	ti,driver-strength-ohm = <50>;
 	disable-wp;
 	cd-gpios = <&main_gpio1 48 GPIO_ACTIVE_LOW>;
 	cd-debounce-delay-ms = <100>;
@@ -857,7 +855,6 @@ &sdhci2 {
 	ti,fails-without-test-cd;
 	cap-power-off-card;
 	keep-power-in-suspend;
-	ti,driver-strength-ohm = <50>;
 	assigned-clocks = <&k3_clks 157 158>;
 	assigned-clock-parents = <&k3_clks 157 160>;
 	#address-cells = <1>;
diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
index 227e7fb3e5bb..db5a7746c82e 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
@@ -561,7 +561,6 @@ sdhci1: mmc@fa00000 {
 		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
 		clock-names = "clk_ahb", "clk_xin";
-		ti,trm-icp = <0x2>;
 		ti,otap-del-sel-legacy = <0x0>;
 		ti,otap-del-sel-sd-hs = <0x0>;
 		ti,otap-del-sel-sdr12 = <0xf>;
diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
index 2497acc0dd60..74816e08a15e 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
@@ -507,7 +507,6 @@ &sdhci1 {
 	vmmc-supply = <&vdd_mmc1>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&main_mmc1_pins_default>;
-	ti,driver-strength-ohm = <50>;
 	disable-wp;
 };
 
diff --git a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
index 10156a04a92c..9e37e54f141a 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
@@ -426,7 +426,6 @@ &sdhci1 {
 	vqmmc-supply = <&vddshv_sdio>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&main_mmc1_pins_default>;
-	ti,driver-strength-ohm = <50>;
 	disable-wp;
 };
 
diff --git a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
index 33768c02d8eb..6dd48c826f74 100644
--- a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
@@ -411,7 +411,6 @@ &sdhci0 {
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&main_mmc0_pins_default>;
-	ti,driver-strength-ohm = <50>;
 	disable-wp;
 };
 
@@ -421,7 +420,6 @@ &sdhci1 {
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&main_mmc1_pins_default>;
-	ti,driver-strength-ohm = <50>;
 	disable-wp;
 };
 
diff --git a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
index b355d0a715a9..1842f05ac351 100644
--- a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
@@ -646,7 +646,6 @@ sdhci1: mmc@fa00000 {
 		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 58 3>, <&k3_clks 58 4>;
 		clock-names = "clk_ahb", "clk_xin";
-		ti,trm-icp = <0x2>;
 		ti,otap-del-sel-legacy = <0x0>;
 		ti,otap-del-sel-sd-hs = <0x0>;
 		ti,otap-del-sel-sdr12 = <0xf>;
diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
index 8c5651d2cf5d..0583ec3a9b52 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
@@ -508,7 +508,6 @@ &sdhci1 {
 	pinctrl-names = "default";
 	bus-width = <4>;
 	pinctrl-0 = <&main_mmc1_pins_default>;
-	ti,driver-strength-ohm = <50>;
 	disable-wp;
 };
 
diff --git a/arch/arm64/boot/dts/ti/k3-am642-sk.dts b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
index 1dddd6fc1a0d..c3a77f6282cb 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
@@ -471,7 +471,6 @@ &sdhci1 {
 	pinctrl-names = "default";
 	bus-width = <4>;
 	pinctrl-0 = <&main_mmc1_pins_default>;
-	ti,driver-strength-ohm = <50>;
 	disable-wp;
 };
 
-- 
2.34.1

[RFC PATCH 13/13] arm64: dts: ti: k3-am6*: Reorganize MMC properties

[Judith Mendez]

Reorganize various MMC properties for MMC nodes to be
more uniform across devices.

Add ti,clkbuf-sel to MMC nodes that are missing this property.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62-main.dtsi       | 5 +++--
 arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 2 --
 arch/arm64/boot/dts/ti/k3-am62a-main.dtsi      | 4 ++--
 arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi | 2 ++
 arch/arm64/boot/dts/ti/k3-am64-main.dtsi       | 7 +++++--
 arch/arm64/boot/dts/ti/k3-am642-evm.dts        | 3 +--
 arch/arm64/boot/dts/ti/k3-am642-sk.dts         | 1 -
 7 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
index ca825088970f..32a8a68f1311 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
@@ -559,9 +559,9 @@ sdhci0: mmc@fa10000 {
 		clock-names = "clk_ahb", "clk_xin";
 		assigned-clocks = <&k3_clks 57 6>;
 		assigned-clock-parents = <&k3_clks 57 8>;
+		bus-width = <8>;
 		mmc-ddr-1_8v;
 		mmc-hs200-1_8v;
-		bus-width = <8>;
 		ti,clkbuf-sel = <0x7>;
 		ti,otap-del-sel-legacy = <0x0>;
 		ti,otap-del-sel-mmc-hs = <0x0>;
@@ -576,8 +576,8 @@ sdhci1: mmc@fa00000 {
 		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
 		clock-names = "clk_ahb", "clk_xin";
-		ti,clkbuf-sel = <0x7>;
 		bus-width = <4>;
+		ti,clkbuf-sel = <0x7>;
 		ti,otap-del-sel-legacy = <0x0>;
 		ti,otap-del-sel-sd-hs = <0x0>;
 		ti,otap-del-sel-sdr12 = <0xf>;
@@ -599,6 +599,7 @@ sdhci2: mmc@fa20000 {
 		power-domains = <&k3_pds 184 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 184 5>, <&k3_clks 184 6>;
 		clock-names = "clk_ahb", "clk_xin";
+		bus-width = <4>;
 		ti,clkbuf-sel = <0x7>;
 		ti,otap-del-sel-legacy = <0x0>;
 		ti,otap-del-sel-sd-hs = <0x0>;
diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
index f69dbf9b8406..0422615e4d98 100644
--- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
+++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
@@ -836,7 +836,6 @@ &sdhci1 {
 	bootph-all;
 	pinctrl-names = "default";
 	pinctrl-0 = <&sd_pins_default>;
-
 	vmmc-supply = <&vdd_3v3_sd>;
 	vqmmc-supply = <&vdd_sd_dv>;
 	disable-wp;
@@ -850,7 +849,6 @@ &sdhci2 {
 	vmmc-supply = <&wlan_en>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&wifi_pins_default>, <&wifi_32k_clk>;
-	bus-width = <4>;
 	non-removable;
 	ti,fails-without-test-cd;
 	cap-power-off-card;
diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
index db5a7746c82e..88b112e657c8 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
@@ -561,6 +561,8 @@ sdhci1: mmc@fa00000 {
 		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
 		clock-names = "clk_ahb", "clk_xin";
+		bus-width = <4>;
+		ti,clkbuf-sel = <0x7>;
 		ti,otap-del-sel-legacy = <0x0>;
 		ti,otap-del-sel-sd-hs = <0x0>;
 		ti,otap-del-sel-sdr12 = <0xf>;
@@ -572,8 +574,6 @@ sdhci1: mmc@fa00000 {
 		ti,itap-del-sel-sd-hs = <0x0>;
 		ti,itap-del-sel-sdr12 = <0x0>;
 		ti,itap-del-sel-sdr25 = <0x0>;
-		ti,clkbuf-sel = <0x7>;
-		bus-width = <4>;
 		no-1-8-v;
 		status = "disabled";
 	};
diff --git a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
index 6dd48c826f74..2b4c10b35db1 100644
--- a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
@@ -407,10 +407,12 @@ &main_i2c2 {
 };
 
 &sdhci0 {
+	/* eMMC */
 	bootph-all;
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&main_mmc0_pins_default>;
+	non-removable;
 	disable-wp;
 };
 
diff --git a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
index 1842f05ac351..34706ab9f5fb 100644
--- a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
@@ -626,9 +626,11 @@ sdhci0: mmc@fa10000 {
 		power-domains = <&k3_pds 57 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 57 0>, <&k3_clks 57 1>;
 		clock-names = "clk_ahb", "clk_xin";
+		bus-width = <8>;
 		mmc-ddr-1_8v;
 		mmc-hs200-1_8v;
-		ti,trm-icp = <0x2>;
+		ti,clkbuf-sel = <0x7>;
+		ti,trm-icp = <0x8>;
 		ti,otap-del-sel-legacy = <0x0>;
 		ti,otap-del-sel-mmc-hs = <0x0>;
 		ti,otap-del-sel-ddr52 = <0x6>;
@@ -646,6 +648,8 @@ sdhci1: mmc@fa00000 {
 		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 58 3>, <&k3_clks 58 4>;
 		clock-names = "clk_ahb", "clk_xin";
+		bus-width = <4>;
+		ti,clkbuf-sel = <0x7>;
 		ti,otap-del-sel-legacy = <0x0>;
 		ti,otap-del-sel-sd-hs = <0x0>;
 		ti,otap-del-sel-sdr12 = <0xf>;
@@ -653,7 +657,6 @@ sdhci1: mmc@fa00000 {
 		ti,otap-del-sel-sdr50 = <0xc>;
 		ti,otap-del-sel-sdr104 = <0x6>;
 		ti,otap-del-sel-ddr50 = <0x9>;
-		ti,clkbuf-sel = <0x7>;
 		ti,itap-del-sel-legacy = <0x0>;
 		ti,itap-del-sel-sd-hs = <0x0>;
 		ti,itap-del-sel-sdr12 = <0x0>;
diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
index 0583ec3a9b52..572b98a217a6 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
@@ -493,8 +493,8 @@ eeprom@0 {
 
 /* eMMC */
 &sdhci0 {
+	bootph-all;
 	status = "okay";
-	bus-width = <8>;
 	non-removable;
 	ti,driver-strength-ohm = <50>;
 	disable-wp;
@@ -506,7 +506,6 @@ &sdhci1 {
 	status = "okay";
 	vmmc-supply = <&vdd_mmc1>;
 	pinctrl-names = "default";
-	bus-width = <4>;
 	pinctrl-0 = <&main_mmc1_pins_default>;
 	disable-wp;
 };
diff --git a/arch/arm64/boot/dts/ti/k3-am642-sk.dts b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
index c3a77f6282cb..600056105874 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
@@ -469,7 +469,6 @@ &sdhci1 {
 	status = "okay";
 	vmmc-supply = <&vdd_mmc1>;
 	pinctrl-names = "default";
-	bus-width = <4>;
 	pinctrl-0 = <&main_mmc1_pins_default>;
 	disable-wp;
 };
-- 
2.34.1

Re: [RFC PATCH 13/13] arm64: dts: ti: k3-am6*: Reorganize MMC properties

[Roger Quadros]

On 31/01/2024 02:37, Judith Mendez wrote:
> Reorganize various MMC properties for MMC nodes to be
> more uniform across devices.
> 
> Add ti,clkbuf-sel to MMC nodes that are missing this property.
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi       | 5 +++--
>  arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 2 --
>  arch/arm64/boot/dts/ti/k3-am62a-main.dtsi      | 4 ++--
>  arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi | 2 ++
>  arch/arm64/boot/dts/ti/k3-am64-main.dtsi       | 7 +++++--
>  arch/arm64/boot/dts/ti/k3-am642-evm.dts        | 3 +--
>  arch/arm64/boot/dts/ti/k3-am642-sk.dts         | 1 -
>  7 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> index ca825088970f..32a8a68f1311 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> @@ -559,9 +559,9 @@ sdhci0: mmc@fa10000 {
>  		clock-names = "clk_ahb", "clk_xin";
>  		assigned-clocks = <&k3_clks 57 6>;
>  		assigned-clock-parents = <&k3_clks 57 8>;
> +		bus-width = <8>;

Is bus-width fix for this instance? If not then we don't really know here what
bus-width is used by the board implementation. And it should come in the
board DTS file.

>  		mmc-ddr-1_8v;
>  		mmc-hs200-1_8v;
> -		bus-width = <8>;
>  		ti,clkbuf-sel = <0x7>;
>  		ti,otap-del-sel-legacy = <0x0>;
>  		ti,otap-del-sel-mmc-hs = <0x0>;
> @@ -576,8 +576,8 @@ sdhci1: mmc@fa00000 {
>  		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
>  		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
>  		clock-names = "clk_ahb", "clk_xin";
> -		ti,clkbuf-sel = <0x7>;
>  		bus-width = <4>;
> +		ti,clkbuf-sel = <0x7>;
>  		ti,otap-del-sel-legacy = <0x0>;
>  		ti,otap-del-sel-sd-hs = <0x0>;
>  		ti,otap-del-sel-sdr12 = <0xf>;
> @@ -599,6 +599,7 @@ sdhci2: mmc@fa20000 {
>  		power-domains = <&k3_pds 184 TI_SCI_PD_EXCLUSIVE>;
>  		clocks = <&k3_clks 184 5>, <&k3_clks 184 6>;
>  		clock-names = "clk_ahb", "clk_xin";
> +		bus-width = <4>;
>  		ti,clkbuf-sel = <0x7>;
>  		ti,otap-del-sel-legacy = <0x0>;
>  		ti,otap-del-sel-sd-hs = <0x0>;
> diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
> index f69dbf9b8406..0422615e4d98 100644
> --- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
> @@ -836,7 +836,6 @@ &sdhci1 {
>  	bootph-all;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&sd_pins_default>;
> -
>  	vmmc-supply = <&vdd_3v3_sd>;
>  	vqmmc-supply = <&vdd_sd_dv>;
>  	disable-wp;
> @@ -850,7 +849,6 @@ &sdhci2 {
>  	vmmc-supply = <&wlan_en>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&wifi_pins_default>, <&wifi_32k_clk>;
> -	bus-width = <4>;

I wouldn't remove this from here if bus-width is variable for this
instance of MMC controller.

>  	non-removable;
>  	ti,fails-without-test-cd;
>  	cap-power-off-card;
> diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
> index db5a7746c82e..88b112e657c8 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
> @@ -561,6 +561,8 @@ sdhci1: mmc@fa00000 {
>  		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
>  		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
>  		clock-names = "clk_ahb", "clk_xin";
> +		bus-width = <4>;
> +		ti,clkbuf-sel = <0x7>;
>  		ti,otap-del-sel-legacy = <0x0>;
>  		ti,otap-del-sel-sd-hs = <0x0>;
>  		ti,otap-del-sel-sdr12 = <0xf>;
> @@ -572,8 +574,6 @@ sdhci1: mmc@fa00000 {
>  		ti,itap-del-sel-sd-hs = <0x0>;
>  		ti,itap-del-sel-sdr12 = <0x0>;
>  		ti,itap-del-sel-sdr25 = <0x0>;
> -		ti,clkbuf-sel = <0x7>;
> -		bus-width = <4>;
>  		no-1-8-v;
>  		status = "disabled";
>  	};
> diff --git a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
> index 6dd48c826f74..2b4c10b35db1 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
> @@ -407,10 +407,12 @@ &main_i2c2 {
>  };
>  
>  &sdhci0 {
> +	/* eMMC */
>  	bootph-all;
>  	status = "okay";
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&main_mmc0_pins_default>;
> +	non-removable;
>  	disable-wp;
>  };
>  
> diff --git a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
> index 1842f05ac351..34706ab9f5fb 100644
> --- a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
> @@ -626,9 +626,11 @@ sdhci0: mmc@fa10000 {
>  		power-domains = <&k3_pds 57 TI_SCI_PD_EXCLUSIVE>;
>  		clocks = <&k3_clks 57 0>, <&k3_clks 57 1>;
>  		clock-names = "clk_ahb", "clk_xin";
> +		bus-width = <8>;
>  		mmc-ddr-1_8v;
>  		mmc-hs200-1_8v;
> -		ti,trm-icp = <0x2>;
> +		ti,clkbuf-sel = <0x7>;
> +		ti,trm-icp = <0x8>;
>  		ti,otap-del-sel-legacy = <0x0>;
>  		ti,otap-del-sel-mmc-hs = <0x0>;
>  		ti,otap-del-sel-ddr52 = <0x6>;
> @@ -646,6 +648,8 @@ sdhci1: mmc@fa00000 {
>  		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
>  		clocks = <&k3_clks 58 3>, <&k3_clks 58 4>;
>  		clock-names = "clk_ahb", "clk_xin";
> +		bus-width = <4>;
> +		ti,clkbuf-sel = <0x7>;
>  		ti,otap-del-sel-legacy = <0x0>;
>  		ti,otap-del-sel-sd-hs = <0x0>;
>  		ti,otap-del-sel-sdr12 = <0xf>;
> @@ -653,7 +657,6 @@ sdhci1: mmc@fa00000 {
>  		ti,otap-del-sel-sdr50 = <0xc>;
>  		ti,otap-del-sel-sdr104 = <0x6>;
>  		ti,otap-del-sel-ddr50 = <0x9>;
> -		ti,clkbuf-sel = <0x7>;
>  		ti,itap-del-sel-legacy = <0x0>;
>  		ti,itap-del-sel-sd-hs = <0x0>;
>  		ti,itap-del-sel-sdr12 = <0x0>;
> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> index 0583ec3a9b52..572b98a217a6 100644
> --- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> @@ -493,8 +493,8 @@ eeprom@0 {
>  
>  /* eMMC */
>  &sdhci0 {
> +	bootph-all;
>  	status = "okay";
> -	bus-width = <8>;
>  	non-removable;
>  	ti,driver-strength-ohm = <50>;
>  	disable-wp;
> @@ -506,7 +506,6 @@ &sdhci1 {
>  	status = "okay";
>  	vmmc-supply = <&vdd_mmc1>;
>  	pinctrl-names = "default";
> -	bus-width = <4>;
>  	pinctrl-0 = <&main_mmc1_pins_default>;
>  	disable-wp;
>  };
> diff --git a/arch/arm64/boot/dts/ti/k3-am642-sk.dts b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
> index c3a77f6282cb..600056105874 100644
> --- a/arch/arm64/boot/dts/ti/k3-am642-sk.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
> @@ -469,7 +469,6 @@ &sdhci1 {
>  	status = "okay";
>  	vmmc-supply = <&vdd_mmc1>;
>  	pinctrl-names = "default";
> -	bus-width = <4>;
>  	pinctrl-0 = <&main_mmc1_pins_default>;
>  	disable-wp;
>  };

-- 
cheers,
-roger

Re: [RFC PATCH 13/13] arm64: dts: ti: k3-am6*: Reorganize MMC properties

[Judith Mendez]

Hi Roger,

On 2/2/24 3:54 AM, Roger Quadros wrote:
> 
> 
> On 31/01/2024 02:37, Judith Mendez wrote:
>> Reorganize various MMC properties for MMC nodes to be
>> more uniform across devices.
>>
>> Add ti,clkbuf-sel to MMC nodes that are missing this property.
>>
>> Signed-off-by: Judith Mendez <jm@ti.com>
>> ---
>>   arch/arm64/boot/dts/ti/k3-am62-main.dtsi       | 5 +++--
>>   arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 2 --
>>   arch/arm64/boot/dts/ti/k3-am62a-main.dtsi      | 4 ++--
>>   arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi | 2 ++
>>   arch/arm64/boot/dts/ti/k3-am64-main.dtsi       | 7 +++++--
>>   arch/arm64/boot/dts/ti/k3-am642-evm.dts        | 3 +--
>>   arch/arm64/boot/dts/ti/k3-am642-sk.dts         | 1 -
>>   7 files changed, 13 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
>> index ca825088970f..32a8a68f1311 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
>> @@ -559,9 +559,9 @@ sdhci0: mmc@fa10000 {
>>   		clock-names = "clk_ahb", "clk_xin";
>>   		assigned-clocks = <&k3_clks 57 6>;
>>   		assigned-clock-parents = <&k3_clks 57 8>;
>> +		bus-width = <8>;
> 
> Is bus-width fix for this instance? If not then we don't really know here what
> bus-width is used by the board implementation. And it should come in the
> board DTS file.

I move bus-width changes to a different patch in v1. Why should it come
in the board DTS file?

> 
>>   		mmc-ddr-1_8v;
>>   		mmc-hs200-1_8v;
>> -		bus-width = <8>;
>>   		ti,clkbuf-sel = <0x7>;
>>   		ti,otap-del-sel-legacy = <0x0>;
>>   		ti,otap-del-sel-mmc-hs = <0x0>;
>> @@ -576,8 +576,8 @@ sdhci1: mmc@fa00000 {
>>   		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
>>   		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
>>   		clock-names = "clk_ahb", "clk_xin";
>> -		ti,clkbuf-sel = <0x7>;
>>   		bus-width = <4>;
>> +		ti,clkbuf-sel = <0x7>;
>>   		ti,otap-del-sel-legacy = <0x0>;
>>   		ti,otap-del-sel-sd-hs = <0x0>;
>>   		ti,otap-del-sel-sdr12 = <0xf>;
>> @@ -599,6 +599,7 @@ sdhci2: mmc@fa20000 {
>>   		power-domains = <&k3_pds 184 TI_SCI_PD_EXCLUSIVE>;
>>   		clocks = <&k3_clks 184 5>, <&k3_clks 184 6>;
>>   		clock-names = "clk_ahb", "clk_xin";
>> +		bus-width = <4>;
>>   		ti,clkbuf-sel = <0x7>;
>>   		ti,otap-del-sel-legacy = <0x0>;
>>   		ti,otap-del-sel-sd-hs = <0x0>;
>> diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>> index f69dbf9b8406..0422615e4d98 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>> @@ -836,7 +836,6 @@ &sdhci1 {
>>   	bootph-all;
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&sd_pins_default>;
>> -
>>   	vmmc-supply = <&vdd_3v3_sd>;
>>   	vqmmc-supply = <&vdd_sd_dv>;
>>   	disable-wp;
>> @@ -850,7 +849,6 @@ &sdhci2 {
>>   	vmmc-supply = <&wlan_en>;
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&wifi_pins_default>, <&wifi_32k_clk>;
>> -	bus-width = <4>;
> 
> I wouldn't remove this from here if bus-width is variable for this
> instance of MMC controller.

It is moved to the .dtsi file. Sorry for the confusion.

> 
>>   	non-removable;
>>   	ti,fails-without-test-cd;
>>   	cap-power-off-card;
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
>> index db5a7746c82e..88b112e657c8 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
>> @@ -561,6 +561,8 @@ sdhci1: mmc@fa00000 {
>>   		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
>>   		clocks = <&k3_clks 58 5>, <&k3_clks 58 6>;
>>   		clock-names = "clk_ahb", "clk_xin";
>> +		bus-width = <4>;
>> +		ti,clkbuf-sel = <0x7>;
>>   		ti,otap-del-sel-legacy = <0x0>;
>>   		ti,otap-del-sel-sd-hs = <0x0>;
>>   		ti,otap-del-sel-sdr12 = <0xf>;
>> @@ -572,8 +574,6 @@ sdhci1: mmc@fa00000 {
>>   		ti,itap-del-sel-sd-hs = <0x0>;
>>   		ti,itap-del-sel-sdr12 = <0x0>;
>>   		ti,itap-del-sel-sdr25 = <0x0>;
>> -		ti,clkbuf-sel = <0x7>;
>> -		bus-width = <4>;
>>   		no-1-8-v;
>>   		status = "disabled";
>>   	};
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
>> index 6dd48c826f74..2b4c10b35db1 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
>> @@ -407,10 +407,12 @@ &main_i2c2 {
>>   };
>>   
>>   &sdhci0 {
>> +	/* eMMC */
>>   	bootph-all;
>>   	status = "okay";
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&main_mmc0_pins_default>;
>> +	non-removable;
>>   	disable-wp;
>>   };
>>   
>> diff --git a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
>> index 1842f05ac351..34706ab9f5fb 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
>> @@ -626,9 +626,11 @@ sdhci0: mmc@fa10000 {
>>   		power-domains = <&k3_pds 57 TI_SCI_PD_EXCLUSIVE>;
>>   		clocks = <&k3_clks 57 0>, <&k3_clks 57 1>;
>>   		clock-names = "clk_ahb", "clk_xin";
>> +		bus-width = <8>;
>>   		mmc-ddr-1_8v;
>>   		mmc-hs200-1_8v;
>> -		ti,trm-icp = <0x2>;
>> +		ti,clkbuf-sel = <0x7>;
>> +		ti,trm-icp = <0x8>;
>>   		ti,otap-del-sel-legacy = <0x0>;
>>   		ti,otap-del-sel-mmc-hs = <0x0>;
>>   		ti,otap-del-sel-ddr52 = <0x6>;
>> @@ -646,6 +648,8 @@ sdhci1: mmc@fa00000 {
>>   		power-domains = <&k3_pds 58 TI_SCI_PD_EXCLUSIVE>;
>>   		clocks = <&k3_clks 58 3>, <&k3_clks 58 4>;
>>   		clock-names = "clk_ahb", "clk_xin";
>> +		bus-width = <4>;
>> +		ti,clkbuf-sel = <0x7>;
>>   		ti,otap-del-sel-legacy = <0x0>;
>>   		ti,otap-del-sel-sd-hs = <0x0>;
>>   		ti,otap-del-sel-sdr12 = <0xf>;
>> @@ -653,7 +657,6 @@ sdhci1: mmc@fa00000 {
>>   		ti,otap-del-sel-sdr50 = <0xc>;
>>   		ti,otap-del-sel-sdr104 = <0x6>;
>>   		ti,otap-del-sel-ddr50 = <0x9>;
>> -		ti,clkbuf-sel = <0x7>;
>>   		ti,itap-del-sel-legacy = <0x0>;
>>   		ti,itap-del-sel-sd-hs = <0x0>;
>>   		ti,itap-del-sel-sdr12 = <0x0>;
>> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>> index 0583ec3a9b52..572b98a217a6 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>> @@ -493,8 +493,8 @@ eeprom@0 {
>>   
>>   /* eMMC */
>>   &sdhci0 {
>> +	bootph-all;
>>   	status = "okay";
>> -	bus-width = <8>;
>>   	non-removable;
>>   	ti,driver-strength-ohm = <50>;
>>   	disable-wp;
>> @@ -506,7 +506,6 @@ &sdhci1 {
>>   	status = "okay";
>>   	vmmc-supply = <&vdd_mmc1>;
>>   	pinctrl-names = "default";
>> -	bus-width = <4>;
>>   	pinctrl-0 = <&main_mmc1_pins_default>;
>>   	disable-wp;
>>   };
>> diff --git a/arch/arm64/boot/dts/ti/k3-am642-sk.dts b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
>> index c3a77f6282cb..600056105874 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am642-sk.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
>> @@ -469,7 +469,6 @@ &sdhci1 {
>>   	status = "okay";
>>   	vmmc-supply = <&vdd_mmc1>;
>>   	pinctrl-names = "default";
>> -	bus-width = <4>;
>>   	pinctrl-0 = <&main_mmc1_pins_default>;
>>   	disable-wp;
>>   };
> 

~ Judith

Re: [RFC PATCH 00/13] Add tuning algorithm for delay chain

[Raghavendra, Vignesh]

Hi,

On 1/31/2024 6:07 AM, Judith Mendez wrote:
> This patch series introduces a new tuning algorithm for
> mmc. The new algorithm should be used when delay chain is
> enabled. The ITAPDLY is selected from the largest passing
> window and the buffer is not viewed as a circular buffer.
> The new tuning algorithm is implemented as per the paper
> published here [0] and has been tested on the following
> platforms: AM62x SK, AM62A SK, AM62p SK, AM64x SK, and AM64x
> EVM.
> 
> The series also includes a few fixes in the sdhci_am654
> driver on OTAPDLYEN/ITAPDLYEN and ITAPDELSEL. There are
> also device tree node fixes for missing mmc nodes,
> modifying DLL properties, and fixes for OTAP/ITAP delay
> values. 
> 
> MMC0/MMC2 nodes are introduced for AM62ax in this series.
> 
> This series is sent as a RFC mostly to get some feedback
> and/or comments on the new tuning algorithm implementation.
> 
> [0] https://www.ti.com/lit/an/spract9/spract9.pdf
> 


> Judith Mendez (11):
>   drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
>   drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
>   drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
>   drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
>     sdhci_j721e_4bit_set_clock
>   drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing

These patches needs to have Fixes: tag as they are bug fixes IMO.

>   arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
>   arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
>   arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
>   arm64: dts: ti: k3-am62p: Add missing properties for MMC
>   arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
>   arm64: dts: ti: k3-am6*: Reorganize MMC properties
> 
> Nitin Yadav (2):
>   arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
>   arm64: dts: ti: k3-am62a7-sk: Enable eMMC support
> 

Can the driver changes be merged independent of DT changes? Or are they
meant to go together? Latter would be problematic as it creates cross
tree dependencies.

>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi      |  57 +++--
>  .../arm64/boot/dts/ti/k3-am625-beagleplay.dts |   5 -
>  arch/arm64/boot/dts/ti/k3-am62a-main.dtsi     |  45 +++-
>  arch/arm64/boot/dts/ti/k3-am62a7-sk.dts       |  27 ++-
>  arch/arm64/boot/dts/ti/k3-am62p-main.dtsi     |  44 +++-
>  arch/arm64/boot/dts/ti/k3-am62p5-sk.dts       |   7 +-
>  .../arm64/boot/dts/ti/k3-am62x-sk-common.dtsi |   4 +-
>  arch/arm64/boot/dts/ti/k3-am64-main.dtsi      |  17 +-
>  arch/arm64/boot/dts/ti/k3-am642-evm.dts       |   4 +-
>  arch/arm64/boot/dts/ti/k3-am642-sk.dts        |   2 -
>  drivers/mmc/host/sdhci_am654.c                | 215 ++++++++++++++----
>  11 files changed, 321 insertions(+), 106 deletions(-)
> 

Re: [RFC PATCH 00/13] Add tuning algorithm for delay chain

[Krzysztof Kozlowski]

On 31/01/2024 14:35, Raghavendra, Vignesh wrote:
>> Judith Mendez (11):
>>   drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
>>   drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
>>   drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
>>   drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
>>     sdhci_j721e_4bit_set_clock
>>   drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing
> 
> These patches needs to have Fixes: tag as they are bug fixes IMO.
> 
>>   arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
>>   arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
>>   arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
>>   arm64: dts: ti: k3-am62p: Add missing properties for MMC
>>   arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
>>   arm64: dts: ti: k3-am6*: Reorganize MMC properties
>>
>> Nitin Yadav (2):
>>   arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
>>   arm64: dts: ti: k3-am62a7-sk: Enable eMMC support
>>
> 
> Can the driver changes be merged independent of DT changes? Or are they
> meant to go together? Latter would be problematic as it creates cross
> tree dependencies.

DTS cannot depend on driver changes, because that would mean hardware
description is not really hardware but OS. So the answer to your
question must be "yes, can be merged independently".

Best regards,
Krzysztof

Re: [RFC PATCH 00/13] Add tuning algorithm for delay chain

[Raghavendra, Vignesh]

On 1/31/2024 7:11 PM, Krzysztof Kozlowski wrote:
> On 31/01/2024 14:35, Raghavendra, Vignesh wrote:
>>> Judith Mendez (11):
>>>   drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
>>>   drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
>>>   drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
>>>   drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
>>>     sdhci_j721e_4bit_set_clock
>>>   drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing
>>
>> These patches needs to have Fixes: tag as they are bug fixes IMO.
>>
>>>   arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
>>>   arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
>>>   arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
>>>   arm64: dts: ti: k3-am62p: Add missing properties for MMC
>>>   arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
>>>   arm64: dts: ti: k3-am6*: Reorganize MMC properties
>>>
>>> Nitin Yadav (2):
>>>   arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
>>>   arm64: dts: ti: k3-am62a7-sk: Enable eMMC support
>>>
>>
>> Can the driver changes be merged independent of DT changes? Or are they
>> meant to go together? Latter would be problematic as it creates cross
>> tree dependencies.
> 
> DTS cannot depend on driver changes, because that would mean hardware
> description is not really hardware but OS. So the answer to your
> question must be "yes, can be merged independently".
> 

Normally yes, but here I see update to tuning algorithm and timing
paramaters to the algorithm. DT updates seem to be nature of bug fixes
where in parameters have been refined due to more HW char
data/understanding being available.

I get nervous when anyone posts both driver and DT changes in a single
series, because they may not have tested driver changes standalone and
may have broken the DT backward compatibility

> Best regards,
> Krzysztof
> 

Regards
Vignesh

Re: [RFC PATCH 00/13] Add tuning algorithm for delay chain

[Krzysztof Kozlowski]

On 31/01/2024 14:53, Raghavendra, Vignesh wrote:
> 
> 
> On 1/31/2024 7:11 PM, Krzysztof Kozlowski wrote:
>> On 31/01/2024 14:35, Raghavendra, Vignesh wrote:
>>>> Judith Mendez (11):
>>>>   drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
>>>>   drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
>>>>   drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
>>>>   drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
>>>>     sdhci_j721e_4bit_set_clock
>>>>   drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing
>>>
>>> These patches needs to have Fixes: tag as they are bug fixes IMO.
>>>
>>>>   arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
>>>>   arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
>>>>   arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
>>>>   arm64: dts: ti: k3-am62p: Add missing properties for MMC
>>>>   arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
>>>>   arm64: dts: ti: k3-am6*: Reorganize MMC properties
>>>>
>>>> Nitin Yadav (2):
>>>>   arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
>>>>   arm64: dts: ti: k3-am62a7-sk: Enable eMMC support
>>>>
>>>
>>> Can the driver changes be merged independent of DT changes? Or are they
>>> meant to go together? Latter would be problematic as it creates cross
>>> tree dependencies.
>>
>> DTS cannot depend on driver changes, because that would mean hardware
>> description is not really hardware but OS. So the answer to your
>> question must be "yes, can be merged independently".
>>
> 
> Normally yes, but here I see update to tuning algorithm and timing
> paramaters to the algorithm. DT updates seem to be nature of bug fixes
> where in parameters have been refined due to more HW char
> data/understanding being available.

Then the patchset should be probably split into fixes and new features,
so it would be clear that new DTS features do not depend on driver code.


Best regards,
Krzysztof

Re: [RFC PATCH 00/13] Add tuning algorithm for delay chain

[Judith Mendez]

On 1/31/24 7:35 AM, Raghavendra, Vignesh wrote:
> Hi,
> 
> On 1/31/2024 6:07 AM, Judith Mendez wrote:
>> This patch series introduces a new tuning algorithm for
>> mmc. The new algorithm should be used when delay chain is
>> enabled. The ITAPDLY is selected from the largest passing
>> window and the buffer is not viewed as a circular buffer.
>> The new tuning algorithm is implemented as per the paper
>> published here [0] and has been tested on the following
>> platforms: AM62x SK, AM62A SK, AM62p SK, AM64x SK, and AM64x
>> EVM.
>>
>> The series also includes a few fixes in the sdhci_am654
>> driver on OTAPDLYEN/ITAPDLYEN and ITAPDELSEL. There are
>> also device tree node fixes for missing mmc nodes,
>> modifying DLL properties, and fixes for OTAP/ITAP delay
>> values.
>>
>> MMC0/MMC2 nodes are introduced for AM62ax in this series.
>>
>> This series is sent as a RFC mostly to get some feedback
>> and/or comments on the new tuning algorithm implementation.
>>
>> [0] https://www.ti.com/lit/an/spract9/spract9.pdf
>>
> 
> 
>> Judith Mendez (11):
>>    drivers: mmc: host: sdhci_am654: Add tuning algorithm for delay chain
>>    drivers: mmc: host: sdhci_am654: Write ITAPDLY for DDR52 timing
>>    drivers: mmc: host: sdhci_am654: Add missing OTAP/ITAP enable
>>    drivers: mmc: host: sdhci_am654: Add ITAPDLYSEL in
>>      sdhci_j721e_4bit_set_clock
>>    drivers: mmc: host: sdhci_am654: Fix ITAPDLY for HS400 timing
> 
> These patches needs to have Fixes: tag as they are bug fixes IMO.

Understood, will add.

> 
>>    arm64: dts: ti: k3-am62a-main: Add sdhci2 instance
>>    arm64: dts: ti: k3-am64-main: Update ITAP/OTAP values for MMC
>>    arm64: dts: ti: k3-am62-main: Update ITAP/OTAP values for MMC
>>    arm64: dts: ti: k3-am62p: Add missing properties for MMC
>>    arm64: dts: ti: k3-am6*: Remove DLL properties for soft phys
>>    arm64: dts: ti: k3-am6*: Reorganize MMC properties
>>
>> Nitin Yadav (2):
>>    arm64: dts: ti: k3-am62a-main: Add sdhci0 instance
>>    arm64: dts: ti: k3-am62a7-sk: Enable eMMC support
>>
> 
> Can the driver changes be merged independent of DT changes? Or are they
> meant to go together? Latter would be problematic as it creates cross
> tree dependencies.

The driver changes can be merged independently.

> 
>>   arch/arm64/boot/dts/ti/k3-am62-main.dtsi      |  57 +++--
>>   .../arm64/boot/dts/ti/k3-am625-beagleplay.dts |   5 -
>>   arch/arm64/boot/dts/ti/k3-am62a-main.dtsi     |  45 +++-
>>   arch/arm64/boot/dts/ti/k3-am62a7-sk.dts       |  27 ++-
>>   arch/arm64/boot/dts/ti/k3-am62p-main.dtsi     |  44 +++-
>>   arch/arm64/boot/dts/ti/k3-am62p5-sk.dts       |   7 +-
>>   .../arm64/boot/dts/ti/k3-am62x-sk-common.dtsi |   4 +-
>>   arch/arm64/boot/dts/ti/k3-am64-main.dtsi      |  17 +-
>>   arch/arm64/boot/dts/ti/k3-am642-evm.dts       |   4 +-
>>   arch/arm64/boot/dts/ti/k3-am642-sk.dts        |   2 -
>>   drivers/mmc/host/sdhci_am654.c                | 215 ++++++++++++++----
>>   11 files changed, 321 insertions(+), 106 deletions(-)
>>