* [PATCH 2/2] mmc: sdhci-pci: Add support for HS200 tuning mode on AMD, eMMC-4.5.1
@ 2016-11-10 12:51 Shyam Sundar S K
2016-11-11 12:41 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Shyam Sundar S K @ 2016-11-10 12:51 UTC (permalink / raw)
To: Adrian Hunter, Ulf Hansson
Cc: linux-mmc, Sen, Pankaj, Agrawal, Nitesh-kumar,
Shah, Nehal-bakulchandra
This patch adds support for HS200 tuning mode on AMD eMMC-4.5.1
Reviewed-by: Sen, Pankaj <Pankaj.Sen@amd.com>
Reviewed-by: Shah, Nehal-bakulchandra <Nehal-bakulchandra.Shah@amd.com>
Signed-off-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
---
drivers/mmc/host/sdhci-pci-core.c | 178 +++++++++++++++++++++++++++++++++++++-
1 file changed, 177 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 782c8d2..9576f82 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -817,6 +817,171 @@ enum amd_chipset_gen {
AMD_CHIPSET_UNKNOWN,
};
+static const struct sdhci_ops amd_sdhci_pci_ops;
+
+struct amd_tuning_descriptor {
+ u8 tune_around;
+ bool this_tune_ok;
+ bool last_tune_ok;
+ u8 valid_front;
+ u8 valid_window_max;
+ u8 tune_low_max;
+ u8 tune_low;
+ u8 valid_window;
+ u8 tune_result;
+};
+
+static int amd_tuning_reset(struct sdhci_host *host)
+{
+ unsigned int val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+ val |= SDHCI_CTRL_PRESET_VAL_ENABLE | SDHCI_CTRL_EXEC_TUNING;
+ sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
+
+ val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+ val &= ~SDHCI_CTRL_EXEC_TUNING;
+ sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
+
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ return 0;
+}
+
+static int amd_config_tuning_phase(struct sdhci_host *host, u8 phase)
+{
+ struct sdhci_pci_slot *slot = sdhci_priv(host);
+ struct pci_dev *pdev = slot->chip->pdev;
+ unsigned int val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ pci_read_config_dword(pdev, 0xb8, &val);
+ val &= ~0x1f;
+ val |= (0x10800 | (phase << 1));
+ pci_write_config_dword(pdev, 0xb8, val);
+
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ return 0;
+}
+
+static int amd_find_good_phase(struct sdhci_host *host)
+{
+ struct sdhci_pci_slot *slot = sdhci_priv(host);
+ struct pci_dev *pdev = slot->chip->pdev;
+ struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
+
+ unsigned int val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ if (td->this_tune_ok)
+ td->valid_front += 1;
+ if ((!td->this_tune_ok && td->last_tune_ok) ||
+ (td->tune_around == 11)) {
+ if (td->valid_window > td->valid_window_max) {
+ td->valid_window_max = td->valid_window;
+ td->tune_low_max = td->tune_low;
+ }
+ }
+ if (td->this_tune_ok && (!td->last_tune_ok))
+ td->tune_low = td->tune_around;
+ if (!td->this_tune_ok && td->last_tune_ok)
+ td->valid_window = 0;
+ else if (td->this_tune_ok)
+ td->valid_window += 1;
+
+ td->last_tune_ok = td->this_tune_ok;
+
+ if (td->tune_around == 11) {
+ if ((td->valid_front + td->valid_window) >
+ td->valid_window_max) {
+ if (td->valid_front > td->valid_window)
+ td->tune_result = ((td->valid_front -
+ td->valid_window) >> 1);
+ else
+ td->tune_result = td->tune_low +
+ ((td->valid_window + td->valid_front) >> 1);
+ } else {
+ td->tune_result = td->tune_low_max +
+ (td->valid_window_max >> 1);
+ }
+
+ if (td->tune_result > 0x0b)
+ td->tune_result = 0x0b;
+
+ pci_read_config_dword(pdev, 0xb8, &val);
+ val &= ~0x1f;
+ val |= (0x10800 | (td->tune_result << 1));
+ pci_write_config_dword(pdev, 0xb8, val);
+ }
+
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ return 0;
+}
+
+static int amd_enable_manual_tuning(struct sdhci_pci_slot *slot)
+{
+ struct pci_dev *pdev = slot->chip->pdev;
+ unsigned int val;
+
+ pci_read_config_dword(pdev, 0xd0, &val);
+ val &= 0xffffffcf;
+ val |= 0x30;
+ pci_write_config_dword(pdev, 0xd0, val);
+
+ return 0;
+}
+
+static int amd_execute_tuning(struct sdhci_host *host, u32 opcode)
+{
+ struct sdhci_pci_slot *slot = sdhci_priv(host);
+ struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
+ u8 ctrl;
+
+ amd_tuning_reset(host);
+ memset(td, 0, sizeof(struct amd_tuning_descriptor));
+
+ /*********************************************************************/
+ /* Enabling Software Tuning */
+ /********************************************************************/
+ /* 1. First switch the eMMC to HS200 Mode
+ * 2. Prepare the registers by using the sampling clock select
+ * 3. Send the CMD21 12 times with block length of 64 bytes
+ * 4. Everytime change the clk phase and check for CRC error
+ * (CMD and DATA),if error, soft reset the CMD and DATA line
+ * 5. Calculate the window and set the clock phase.
+ */
+
+ for (td->tune_around = 0; td->tune_around < 12; td->tune_around++) {
+ amd_config_tuning_phase(host, td->tune_around);
+
+ if (mmc_send_tuning(host->mmc, opcode, NULL)) {
+ td->this_tune_ok = false;
+ host->mmc->need_retune = 0;
+ mdelay(4);
+ ctrl = SDHCI_RESET_CMD | SDHCI_RESET_DATA;
+ sdhci_writeb(host, ctrl, SDHCI_SOFTWARE_RESET);
+ } else {
+ td->this_tune_ok = true;
+ }
+
+ amd_find_good_phase(host);
+ }
+
+ host->mmc->retune_period = 0;
+
+ amd_enable_manual_tuning(slot);
+ return 0;
+}
+
static int amd_probe(struct sdhci_pci_chip *chip)
{
struct pci_dev *smbus_dev;
@@ -841,7 +1006,6 @@ static int amd_probe(struct sdhci_pci_chip *chip)
if ((gen == AMD_CHIPSET_BEFORE_ML) || (gen == AMD_CHIPSET_CZ)) {
chip->quirks2 |= SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD;
- chip->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
}
return 0;
@@ -849,6 +1013,7 @@ static int amd_probe(struct sdhci_pci_chip *chip)
static const struct sdhci_pci_fixes sdhci_amd = {
.probe = amd_probe,
+ .ops = &amd_sdhci_pci_ops,
};
static const struct pci_device_id pci_ids[] = {
@@ -1469,6 +1634,17 @@ static const struct sdhci_ops sdhci_pci_ops = {
.select_drive_strength = sdhci_pci_select_drive_strength,
};
+static const struct sdhci_ops amd_sdhci_pci_ops = {
+ .set_clock = sdhci_set_clock,
+ .enable_dma = sdhci_pci_enable_dma,
+ .set_bus_width = sdhci_pci_set_bus_width,
+ .reset = sdhci_reset,
+ .set_uhs_signaling = sdhci_set_uhs_signaling,
+ .hw_reset = sdhci_pci_hw_reset,
+ .select_drive_strength = sdhci_pci_select_drive_strength,
+ .platform_execute_tuning = amd_execute_tuning,
+};
+
/*****************************************************************************\
* *
* Suspend/resume *
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci-pci: Add support for HS200 tuning mode on AMD, eMMC-4.5.1
2016-11-10 12:51 [PATCH 2/2] mmc: sdhci-pci: Add support for HS200 tuning mode on AMD, eMMC-4.5.1 Shyam Sundar S K
@ 2016-11-11 12:41 ` Adrian Hunter
2016-11-14 6:38 ` Shyam Sundar S K
0 siblings, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2016-11-11 12:41 UTC (permalink / raw)
To: Shyam Sundar S K, Ulf Hansson
Cc: linux-mmc, Sen, Pankaj, Agrawal, Nitesh-kumar,
Shah, Nehal-bakulchandra
On 10/11/16 14:51, Shyam Sundar S K wrote:
> This patch adds support for HS200 tuning mode on AMD eMMC-4.5.1
>
> Reviewed-by: Sen, Pankaj <Pankaj.Sen@amd.com>
> Reviewed-by: Shah, Nehal-bakulchandra <Nehal-bakulchandra.Shah@amd.com>
> Signed-off-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
> ---
> drivers/mmc/host/sdhci-pci-core.c | 178 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 177 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 782c8d2..9576f82 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -817,6 +817,171 @@ enum amd_chipset_gen {
> AMD_CHIPSET_UNKNOWN,
> };
>
> +static const struct sdhci_ops amd_sdhci_pci_ops;
> +
> +struct amd_tuning_descriptor {
> + u8 tune_around;
> + bool this_tune_ok;
> + bool last_tune_ok;
> + u8 valid_front;
> + u8 valid_window_max;
> + u8 tune_low_max;
> + u8 tune_low;
> + u8 valid_window;
> + u8 tune_result;
> +};
> +
> +static int amd_tuning_reset(struct sdhci_host *host)
> +{
> + unsigned int val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + val |= SDHCI_CTRL_PRESET_VAL_ENABLE | SDHCI_CTRL_EXEC_TUNING;
> + sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
> +
> + val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + val &= ~SDHCI_CTRL_EXEC_TUNING;
> + sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + return 0;
> +}
> +
> +static int amd_config_tuning_phase(struct sdhci_host *host, u8 phase)
> +{
> + struct sdhci_pci_slot *slot = sdhci_priv(host);
> + struct pci_dev *pdev = slot->chip->pdev;
> + unsigned int val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + pci_read_config_dword(pdev, 0xb8, &val);
> + val &= ~0x1f;
> + val |= (0x10800 | (phase << 1));
> + pci_write_config_dword(pdev, 0xb8, val);
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + return 0;
> +}
> +
> +static int amd_find_good_phase(struct sdhci_host *host)
> +{
> + struct sdhci_pci_slot *slot = sdhci_priv(host);
> + struct pci_dev *pdev = slot->chip->pdev;
> + struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
> +
> + unsigned int val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + if (td->this_tune_ok)
> + td->valid_front += 1;
> + if ((!td->this_tune_ok && td->last_tune_ok) ||
> + (td->tune_around == 11)) {
> + if (td->valid_window > td->valid_window_max) {
> + td->valid_window_max = td->valid_window;
> + td->tune_low_max = td->tune_low;
> + }
> + }
> + if (td->this_tune_ok && (!td->last_tune_ok))
> + td->tune_low = td->tune_around;
> + if (!td->this_tune_ok && td->last_tune_ok)
> + td->valid_window = 0;
> + else if (td->this_tune_ok)
> + td->valid_window += 1;
> +
> + td->last_tune_ok = td->this_tune_ok;
> +
> + if (td->tune_around == 11) {
> + if ((td->valid_front + td->valid_window) >
> + td->valid_window_max) {
> + if (td->valid_front > td->valid_window)
> + td->tune_result = ((td->valid_front -
> + td->valid_window) >> 1);
> + else
> + td->tune_result = td->tune_low +
> + ((td->valid_window + td->valid_front) >> 1);
> + } else {
> + td->tune_result = td->tune_low_max +
> + (td->valid_window_max >> 1);
> + }
> +
> + if (td->tune_result > 0x0b)
> + td->tune_result = 0x0b;
> +
> + pci_read_config_dword(pdev, 0xb8, &val);
> + val &= ~0x1f;
> + val |= (0x10800 | (td->tune_result << 1));
> + pci_write_config_dword(pdev, 0xb8, val);
> + }
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + return 0;
> +}
> +
> +static int amd_enable_manual_tuning(struct sdhci_pci_slot *slot)
> +{
> + struct pci_dev *pdev = slot->chip->pdev;
> + unsigned int val;
> +
> + pci_read_config_dword(pdev, 0xd0, &val);
> + val &= 0xffffffcf;
> + val |= 0x30;
> + pci_write_config_dword(pdev, 0xd0, val);
> +
> + return 0;
> +}
> +
> +static int amd_execute_tuning(struct sdhci_host *host, u32 opcode)
> +{
> + struct sdhci_pci_slot *slot = sdhci_priv(host);
> + struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
> + u8 ctrl;
> +
> + amd_tuning_reset(host);
> + memset(td, 0, sizeof(struct amd_tuning_descriptor));
I didn't notice last time that you are always clearing all the values in the
tuning descriptor, so it doesn't need to be kept between calls to
amd_execute_tuning(). So it should be a local variable that you pass to the
functions that need it. Then you don't need the patch I sent at all.
Does that make sense?
> +
> + /*********************************************************************/
> + /* Enabling Software Tuning */
> + /********************************************************************/
> + /* 1. First switch the eMMC to HS200 Mode
> + * 2. Prepare the registers by using the sampling clock select
> + * 3. Send the CMD21 12 times with block length of 64 bytes
> + * 4. Everytime change the clk phase and check for CRC error
> + * (CMD and DATA),if error, soft reset the CMD and DATA line
> + * 5. Calculate the window and set the clock phase.
> + */
> +
> + for (td->tune_around = 0; td->tune_around < 12; td->tune_around++) {
> + amd_config_tuning_phase(host, td->tune_around);
> +
> + if (mmc_send_tuning(host->mmc, opcode, NULL)) {
> + td->this_tune_ok = false;
> + host->mmc->need_retune = 0;
> + mdelay(4);
> + ctrl = SDHCI_RESET_CMD | SDHCI_RESET_DATA;
> + sdhci_writeb(host, ctrl, SDHCI_SOFTWARE_RESET);
> + } else {
> + td->this_tune_ok = true;
> + }
> +
> + amd_find_good_phase(host);
> + }
> +
> + host->mmc->retune_period = 0;
> +
> + amd_enable_manual_tuning(slot);
> + return 0;
> +}
> +
> static int amd_probe(struct sdhci_pci_chip *chip)
> {
> struct pci_dev *smbus_dev;
> @@ -841,7 +1006,6 @@ static int amd_probe(struct sdhci_pci_chip *chip)
>
> if ((gen == AMD_CHIPSET_BEFORE_ML) || (gen == AMD_CHIPSET_CZ)) {
> chip->quirks2 |= SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD;
> - chip->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
> }
>
> return 0;
> @@ -849,6 +1013,7 @@ static int amd_probe(struct sdhci_pci_chip *chip)
>
> static const struct sdhci_pci_fixes sdhci_amd = {
> .probe = amd_probe,
> + .ops = &amd_sdhci_pci_ops,
> };
>
> static const struct pci_device_id pci_ids[] = {
> @@ -1469,6 +1634,17 @@ static const struct sdhci_ops sdhci_pci_ops = {
> .select_drive_strength = sdhci_pci_select_drive_strength,
> };
>
> +static const struct sdhci_ops amd_sdhci_pci_ops = {
> + .set_clock = sdhci_set_clock,
> + .enable_dma = sdhci_pci_enable_dma,
> + .set_bus_width = sdhci_pci_set_bus_width,
> + .reset = sdhci_reset,
> + .set_uhs_signaling = sdhci_set_uhs_signaling,
> + .hw_reset = sdhci_pci_hw_reset,
> + .select_drive_strength = sdhci_pci_select_drive_strength,
> + .platform_execute_tuning = amd_execute_tuning,
> +};
> +
> /*****************************************************************************\
> * *
> * Suspend/resume *
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci-pci: Add support for HS200 tuning mode on AMD, eMMC-4.5.1
2016-11-11 12:41 ` Adrian Hunter
@ 2016-11-14 6:38 ` Shyam Sundar S K
2016-11-14 13:37 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Shyam Sundar S K @ 2016-11-14 6:38 UTC (permalink / raw)
To: Adrian Hunter, Ulf Hansson
Cc: linux-mmc, Sen, Pankaj, Agrawal, Nitesh-kumar,
Shah, Nehal-bakulchandra
On 11/11/2016 6:11 PM, Adrian Hunter wrote:
> On 10/11/16 14:51, Shyam Sundar S K wrote:
>> This patch adds support for HS200 tuning mode on AMD eMMC-4.5.1
>>
>> Reviewed-by: Sen, Pankaj <Pankaj.Sen@amd.com>
>> Reviewed-by: Shah, Nehal-bakulchandra <Nehal-bakulchandra.Shah@amd.com>
>> Signed-off-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
>> ---
>> drivers/mmc/host/sdhci-pci-core.c | 178 +++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 177 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
>> index 782c8d2..9576f82 100644
>> --- a/drivers/mmc/host/sdhci-pci-core.c
>> +++ b/drivers/mmc/host/sdhci-pci-core.c
>> @@ -817,6 +817,171 @@ enum amd_chipset_gen {
>> AMD_CHIPSET_UNKNOWN,
>> };
>>
>> +static const struct sdhci_ops amd_sdhci_pci_ops;
>> +
>> +struct amd_tuning_descriptor {
>> + u8 tune_around;
>> + bool this_tune_ok;
>> + bool last_tune_ok;
>> + u8 valid_front;
>> + u8 valid_window_max;
>> + u8 tune_low_max;
>> + u8 tune_low;
>> + u8 valid_window;
>> + u8 tune_result;
>> +};
>> +
>> +static int amd_tuning_reset(struct sdhci_host *host)
>> +{
>> + unsigned int val;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&host->lock, flags);
>> +
>> + val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> + val |= SDHCI_CTRL_PRESET_VAL_ENABLE | SDHCI_CTRL_EXEC_TUNING;
>> + sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
>> +
>> + val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> + val &= ~SDHCI_CTRL_EXEC_TUNING;
>> + sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
>> +
>> + spin_unlock_irqrestore(&host->lock, flags);
>> +
>> + return 0;
>> +}
>> +
>> +static int amd_config_tuning_phase(struct sdhci_host *host, u8 phase)
>> +{
>> + struct sdhci_pci_slot *slot = sdhci_priv(host);
>> + struct pci_dev *pdev = slot->chip->pdev;
>> + unsigned int val;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&host->lock, flags);
>> +
>> + pci_read_config_dword(pdev, 0xb8, &val);
>> + val &= ~0x1f;
>> + val |= (0x10800 | (phase << 1));
>> + pci_write_config_dword(pdev, 0xb8, val);
>> +
>> + spin_unlock_irqrestore(&host->lock, flags);
>> +
>> + return 0;
>> +}
>> +
>> +static int amd_find_good_phase(struct sdhci_host *host)
>> +{
>> + struct sdhci_pci_slot *slot = sdhci_priv(host);
>> + struct pci_dev *pdev = slot->chip->pdev;
>> + struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
>> +
>> + unsigned int val;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&host->lock, flags);
>> +
>> + if (td->this_tune_ok)
>> + td->valid_front += 1;
>> + if ((!td->this_tune_ok && td->last_tune_ok) ||
>> + (td->tune_around == 11)) {
>> + if (td->valid_window > td->valid_window_max) {
>> + td->valid_window_max = td->valid_window;
>> + td->tune_low_max = td->tune_low;
>> + }
>> + }
>> + if (td->this_tune_ok && (!td->last_tune_ok))
>> + td->tune_low = td->tune_around;
>> + if (!td->this_tune_ok && td->last_tune_ok)
>> + td->valid_window = 0;
>> + else if (td->this_tune_ok)
>> + td->valid_window += 1;
>> +
>> + td->last_tune_ok = td->this_tune_ok;
>> +
>> + if (td->tune_around == 11) {
>> + if ((td->valid_front + td->valid_window) >
>> + td->valid_window_max) {
>> + if (td->valid_front > td->valid_window)
>> + td->tune_result = ((td->valid_front -
>> + td->valid_window) >> 1);
>> + else
>> + td->tune_result = td->tune_low +
>> + ((td->valid_window + td->valid_front) >> 1);
>> + } else {
>> + td->tune_result = td->tune_low_max +
>> + (td->valid_window_max >> 1);
>> + }
>> +
>> + if (td->tune_result > 0x0b)
>> + td->tune_result = 0x0b;
>> +
>> + pci_read_config_dword(pdev, 0xb8, &val);
>> + val &= ~0x1f;
>> + val |= (0x10800 | (td->tune_result << 1));
>> + pci_write_config_dword(pdev, 0xb8, val);
>> + }
>> +
>> + spin_unlock_irqrestore(&host->lock, flags);
>> +
>> + return 0;
>> +}
>> +
>> +static int amd_enable_manual_tuning(struct sdhci_pci_slot *slot)
>> +{
>> + struct pci_dev *pdev = slot->chip->pdev;
>> + unsigned int val;
>> +
>> + pci_read_config_dword(pdev, 0xd0, &val);
>> + val &= 0xffffffcf;
>> + val |= 0x30;
>> + pci_write_config_dword(pdev, 0xd0, val);
>> +
>> + return 0;
>> +}
>> +
>> +static int amd_execute_tuning(struct sdhci_host *host, u32 opcode)
>> +{
>> + struct sdhci_pci_slot *slot = sdhci_priv(host);
>> + struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
>> + u8 ctrl;
>> +
>> + amd_tuning_reset(host);
>> + memset(td, 0, sizeof(struct amd_tuning_descriptor));
>
> I didn't notice last time that you are always clearing all the values in the
> tuning descriptor, so it doesn't need to be kept between calls to
> amd_execute_tuning(). So it should be a local variable that you pass to the
> functions that need it. Then you don't need the patch I sent at all.
>
> Does that make sense?
>
Hi Adrian,
Thanks for pointing this out. Your patch will help, so will remove the memset()
and redo the submission. Is this OK ?
Thanks,
Shyam
>> +
>> + /*********************************************************************/
>> + /* Enabling Software Tuning */
>> + /********************************************************************/
>> + /* 1. First switch the eMMC to HS200 Mode
>> + * 2. Prepare the registers by using the sampling clock select
>> + * 3. Send the CMD21 12 times with block length of 64 bytes
>> + * 4. Everytime change the clk phase and check for CRC error
>> + * (CMD and DATA),if error, soft reset the CMD and DATA line
>> + * 5. Calculate the window and set the clock phase.
>> + */
>> +
>> + for (td->tune_around = 0; td->tune_around < 12; td->tune_around++) {
>> + amd_config_tuning_phase(host, td->tune_around);
>> +
>> + if (mmc_send_tuning(host->mmc, opcode, NULL)) {
>> + td->this_tune_ok = false;
>> + host->mmc->need_retune = 0;
>> + mdelay(4);
>> + ctrl = SDHCI_RESET_CMD | SDHCI_RESET_DATA;
>> + sdhci_writeb(host, ctrl, SDHCI_SOFTWARE_RESET);
>> + } else {
>> + td->this_tune_ok = true;
>> + }
>> +
>> + amd_find_good_phase(host);
>> + }
>> +
>> + host->mmc->retune_period = 0;
>> +
>> + amd_enable_manual_tuning(slot);
>> + return 0;
>> +}
>> +
>> static int amd_probe(struct sdhci_pci_chip *chip)
>> {
>> struct pci_dev *smbus_dev;
>> @@ -841,7 +1006,6 @@ static int amd_probe(struct sdhci_pci_chip *chip)
>>
>> if ((gen == AMD_CHIPSET_BEFORE_ML) || (gen == AMD_CHIPSET_CZ)) {
>> chip->quirks2 |= SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD;
>> - chip->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
>> }
>>
>> return 0;
>> @@ -849,6 +1013,7 @@ static int amd_probe(struct sdhci_pci_chip *chip)
>>
>> static const struct sdhci_pci_fixes sdhci_amd = {
>> .probe = amd_probe,
>> + .ops = &amd_sdhci_pci_ops,
>> };
>>
>> static const struct pci_device_id pci_ids[] = {
>> @@ -1469,6 +1634,17 @@ static const struct sdhci_ops sdhci_pci_ops = {
>> .select_drive_strength = sdhci_pci_select_drive_strength,
>> };
>>
>> +static const struct sdhci_ops amd_sdhci_pci_ops = {
>> + .set_clock = sdhci_set_clock,
>> + .enable_dma = sdhci_pci_enable_dma,
>> + .set_bus_width = sdhci_pci_set_bus_width,
>> + .reset = sdhci_reset,
>> + .set_uhs_signaling = sdhci_set_uhs_signaling,
>> + .hw_reset = sdhci_pci_hw_reset,
>> + .select_drive_strength = sdhci_pci_select_drive_strength,
>> + .platform_execute_tuning = amd_execute_tuning,
>> +};
>> +
>> /*****************************************************************************\
>> * *
>> * Suspend/resume *
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci-pci: Add support for HS200 tuning mode on AMD, eMMC-4.5.1
2016-11-14 6:38 ` Shyam Sundar S K
@ 2016-11-14 13:37 ` Adrian Hunter
0 siblings, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2016-11-14 13:37 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Ulf Hansson, linux-mmc, Sen, Pankaj, Agrawal, Nitesh-kumar,
Shah, Nehal-bakulchandra
On 14/11/16 08:38, Shyam Sundar S K wrote:
>
>
> On 11/11/2016 6:11 PM, Adrian Hunter wrote:
>> On 10/11/16 14:51, Shyam Sundar S K wrote:
>>> This patch adds support for HS200 tuning mode on AMD eMMC-4.5.1
>>>
>>> Reviewed-by: Sen, Pankaj <Pankaj.Sen@amd.com>
>>> Reviewed-by: Shah, Nehal-bakulchandra <Nehal-bakulchandra.Shah@amd.com>
>>> Signed-off-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
>>> ---
>>> drivers/mmc/host/sdhci-pci-core.c | 178 +++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 177 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
>>> index 782c8d2..9576f82 100644
>>> --- a/drivers/mmc/host/sdhci-pci-core.c
>>> +++ b/drivers/mmc/host/sdhci-pci-core.c
>>> @@ -817,6 +817,171 @@ enum amd_chipset_gen {
>>> AMD_CHIPSET_UNKNOWN,
>>> };
>>>
>>> +static const struct sdhci_ops amd_sdhci_pci_ops;
>>> +
>>> +struct amd_tuning_descriptor {
>>> + u8 tune_around;
>>> + bool this_tune_ok;
>>> + bool last_tune_ok;
>>> + u8 valid_front;
>>> + u8 valid_window_max;
>>> + u8 tune_low_max;
>>> + u8 tune_low;
>>> + u8 valid_window;
>>> + u8 tune_result;
>>> +};
>>> +
>>> +static int amd_tuning_reset(struct sdhci_host *host)
>>> +{
>>> + unsigned int val;
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&host->lock, flags);
>>> +
>>> + val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>> + val |= SDHCI_CTRL_PRESET_VAL_ENABLE | SDHCI_CTRL_EXEC_TUNING;
>>> + sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
>>> +
>>> + val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>> + val &= ~SDHCI_CTRL_EXEC_TUNING;
>>> + sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
>>> +
>>> + spin_unlock_irqrestore(&host->lock, flags);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int amd_config_tuning_phase(struct sdhci_host *host, u8 phase)
>>> +{
>>> + struct sdhci_pci_slot *slot = sdhci_priv(host);
>>> + struct pci_dev *pdev = slot->chip->pdev;
>>> + unsigned int val;
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&host->lock, flags);
>>> +
>>> + pci_read_config_dword(pdev, 0xb8, &val);
>>> + val &= ~0x1f;
>>> + val |= (0x10800 | (phase << 1));
>>> + pci_write_config_dword(pdev, 0xb8, val);
>>> +
>>> + spin_unlock_irqrestore(&host->lock, flags);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int amd_find_good_phase(struct sdhci_host *host)
>>> +{
>>> + struct sdhci_pci_slot *slot = sdhci_priv(host);
>>> + struct pci_dev *pdev = slot->chip->pdev;
>>> + struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
>>> +
>>> + unsigned int val;
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&host->lock, flags);
>>> +
>>> + if (td->this_tune_ok)
>>> + td->valid_front += 1;
>>> + if ((!td->this_tune_ok && td->last_tune_ok) ||
>>> + (td->tune_around == 11)) {
>>> + if (td->valid_window > td->valid_window_max) {
>>> + td->valid_window_max = td->valid_window;
>>> + td->tune_low_max = td->tune_low;
>>> + }
>>> + }
>>> + if (td->this_tune_ok && (!td->last_tune_ok))
>>> + td->tune_low = td->tune_around;
>>> + if (!td->this_tune_ok && td->last_tune_ok)
>>> + td->valid_window = 0;
>>> + else if (td->this_tune_ok)
>>> + td->valid_window += 1;
>>> +
>>> + td->last_tune_ok = td->this_tune_ok;
>>> +
>>> + if (td->tune_around == 11) {
>>> + if ((td->valid_front + td->valid_window) >
>>> + td->valid_window_max) {
>>> + if (td->valid_front > td->valid_window)
>>> + td->tune_result = ((td->valid_front -
>>> + td->valid_window) >> 1);
>>> + else
>>> + td->tune_result = td->tune_low +
>>> + ((td->valid_window + td->valid_front) >> 1);
>>> + } else {
>>> + td->tune_result = td->tune_low_max +
>>> + (td->valid_window_max >> 1);
>>> + }
>>> +
>>> + if (td->tune_result > 0x0b)
>>> + td->tune_result = 0x0b;
>>> +
>>> + pci_read_config_dword(pdev, 0xb8, &val);
>>> + val &= ~0x1f;
>>> + val |= (0x10800 | (td->tune_result << 1));
>>> + pci_write_config_dword(pdev, 0xb8, val);
>>> + }
>>> +
>>> + spin_unlock_irqrestore(&host->lock, flags);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int amd_enable_manual_tuning(struct sdhci_pci_slot *slot)
>>> +{
>>> + struct pci_dev *pdev = slot->chip->pdev;
>>> + unsigned int val;
>>> +
>>> + pci_read_config_dword(pdev, 0xd0, &val);
>>> + val &= 0xffffffcf;
>>> + val |= 0x30;
>>> + pci_write_config_dword(pdev, 0xd0, val);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int amd_execute_tuning(struct sdhci_host *host, u32 opcode)
>>> +{
>>> + struct sdhci_pci_slot *slot = sdhci_priv(host);
>>> + struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
>>> + u8 ctrl;
>>> +
>>> + amd_tuning_reset(host);
>>> + memset(td, 0, sizeof(struct amd_tuning_descriptor));
>>
>> I didn't notice last time that you are always clearing all the values in the
>> tuning descriptor, so it doesn't need to be kept between calls to
>> amd_execute_tuning(). So it should be a local variable that you pass to the
>> functions that need it. Then you don't need the patch I sent at all.
>>
>> Does that make sense?
>>
>
> Hi Adrian,
>
> Thanks for pointing this out. Your patch will help, so will remove the memset()
> and redo the submission. Is this OK ?
Sure
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] mmc: sdhci-pci: Add support for HS200 tuning mode on AMD eMMC-4.5.1
@ 2016-11-15 6:57 Shyam Sundar S K
2016-11-16 12:01 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Shyam Sundar S K @ 2016-11-15 6:57 UTC (permalink / raw)
To: Adrian Hunter, Ulf Hansson, linux-mmc, Sen, Pankaj,
Shah, Nehal-bakulchandra, Agrawal, Nitesh-kumar
This patch adds support for HS200 tuning mode on AMD eMMC-4.5.1
Reviewed-by: Sen, Pankaj <Pankaj.Sen@amd.com>
Reviewed-by: Shah, Nehal-bakulchandra <Nehal-bakulchandra.Shah@amd.com>
Signed-off-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
---
drivers/mmc/host/sdhci-pci-core.c | 177 +++++++++++++++++++++++++++++++++++++-
1 file changed, 176 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 782c8d2..1a31bdf 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -817,6 +817,170 @@ enum amd_chipset_gen {
AMD_CHIPSET_UNKNOWN,
};
+static const struct sdhci_ops amd_sdhci_pci_ops;
+
+struct amd_tuning_descriptor {
+ u8 tune_around;
+ bool this_tune_ok;
+ bool last_tune_ok;
+ u8 valid_front;
+ u8 valid_window_max;
+ u8 tune_low_max;
+ u8 tune_low;
+ u8 valid_window;
+ u8 tune_result;
+};
+
+static int amd_tuning_reset(struct sdhci_host *host)
+{
+ unsigned int val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+ val |= SDHCI_CTRL_PRESET_VAL_ENABLE | SDHCI_CTRL_EXEC_TUNING;
+ sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
+
+ val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+ val &= ~SDHCI_CTRL_EXEC_TUNING;
+ sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
+
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ return 0;
+}
+
+static int amd_config_tuning_phase(struct sdhci_host *host, u8 phase)
+{
+ struct sdhci_pci_slot *slot = sdhci_priv(host);
+ struct pci_dev *pdev = slot->chip->pdev;
+ unsigned int val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ pci_read_config_dword(pdev, 0xb8, &val);
+ val &= ~0x1f;
+ val |= (0x10800 | (phase << 1));
+ pci_write_config_dword(pdev, 0xb8, val);
+
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ return 0;
+}
+
+static int amd_find_good_phase(struct sdhci_host *host)
+{
+ struct sdhci_pci_slot *slot = sdhci_priv(host);
+ struct pci_dev *pdev = slot->chip->pdev;
+ struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
+
+ unsigned int val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&host->lock, flags);
+
+ if (td->this_tune_ok)
+ td->valid_front += 1;
+ if ((!td->this_tune_ok && td->last_tune_ok) ||
+ (td->tune_around == 11)) {
+ if (td->valid_window > td->valid_window_max) {
+ td->valid_window_max = td->valid_window;
+ td->tune_low_max = td->tune_low;
+ }
+ }
+ if (td->this_tune_ok && (!td->last_tune_ok))
+ td->tune_low = td->tune_around;
+ if (!td->this_tune_ok && td->last_tune_ok)
+ td->valid_window = 0;
+ else if (td->this_tune_ok)
+ td->valid_window += 1;
+
+ td->last_tune_ok = td->this_tune_ok;
+
+ if (td->tune_around == 11) {
+ if ((td->valid_front + td->valid_window) >
+ td->valid_window_max) {
+ if (td->valid_front > td->valid_window)
+ td->tune_result = ((td->valid_front -
+ td->valid_window) >> 1);
+ else
+ td->tune_result = td->tune_low +
+ ((td->valid_window + td->valid_front) >> 1);
+ } else {
+ td->tune_result = td->tune_low_max +
+ (td->valid_window_max >> 1);
+ }
+
+ if (td->tune_result > 0x0b)
+ td->tune_result = 0x0b;
+
+ pci_read_config_dword(pdev, 0xb8, &val);
+ val &= ~0x1f;
+ val |= (0x10800 | (td->tune_result << 1));
+ pci_write_config_dword(pdev, 0xb8, val);
+ }
+
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ return 0;
+}
+
+static int amd_enable_manual_tuning(struct sdhci_pci_slot *slot)
+{
+ struct pci_dev *pdev = slot->chip->pdev;
+ unsigned int val;
+
+ pci_read_config_dword(pdev, 0xd0, &val);
+ val &= 0xffffffcf;
+ val |= 0x30;
+ pci_write_config_dword(pdev, 0xd0, val);
+
+ return 0;
+}
+
+static int amd_execute_tuning(struct sdhci_host *host, u32 opcode)
+{
+ struct sdhci_pci_slot *slot = sdhci_priv(host);
+ struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
+ u8 ctrl;
+
+ amd_tuning_reset(host);
+
+ /*********************************************************************/
+ /* Enabling Software Tuning */
+ /********************************************************************/
+ /* 1. First switch the eMMC to HS200 Mode
+ * 2. Prepare the registers by using the sampling clock select
+ * 3. Send the CMD21 12 times with block length of 64 bytes
+ * 4. Everytime change the clk phase and check for CRC error
+ * (CMD and DATA),if error, soft reset the CMD and DATA line
+ * 5. Calculate the window and set the clock phase.
+ */
+
+ for (td->tune_around = 0; td->tune_around < 12; td->tune_around++) {
+ amd_config_tuning_phase(host, td->tune_around);
+
+ if (mmc_send_tuning(host->mmc, opcode, NULL)) {
+ td->this_tune_ok = false;
+ host->mmc->need_retune = 0;
+ mdelay(4);
+ ctrl = SDHCI_RESET_CMD | SDHCI_RESET_DATA;
+ sdhci_writeb(host, ctrl, SDHCI_SOFTWARE_RESET);
+ } else {
+ td->this_tune_ok = true;
+ }
+
+ amd_find_good_phase(host);
+ }
+
+ host->mmc->retune_period = 0;
+
+ amd_enable_manual_tuning(slot);
+ return 0;
+}
+
static int amd_probe(struct sdhci_pci_chip *chip)
{
struct pci_dev *smbus_dev;
@@ -841,7 +1005,6 @@ static int amd_probe(struct sdhci_pci_chip *chip)
if ((gen == AMD_CHIPSET_BEFORE_ML) || (gen == AMD_CHIPSET_CZ)) {
chip->quirks2 |= SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD;
- chip->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
}
return 0;
@@ -849,6 +1012,7 @@ static int amd_probe(struct sdhci_pci_chip *chip)
static const struct sdhci_pci_fixes sdhci_amd = {
.probe = amd_probe,
+ .ops = &amd_sdhci_pci_ops,
};
static const struct pci_device_id pci_ids[] = {
@@ -1469,6 +1633,17 @@ static const struct sdhci_ops sdhci_pci_ops = {
.select_drive_strength = sdhci_pci_select_drive_strength,
};
+static const struct sdhci_ops amd_sdhci_pci_ops = {
+ .set_clock = sdhci_set_clock,
+ .enable_dma = sdhci_pci_enable_dma,
+ .set_bus_width = sdhci_pci_set_bus_width,
+ .reset = sdhci_reset,
+ .set_uhs_signaling = sdhci_set_uhs_signaling,
+ .hw_reset = sdhci_pci_hw_reset,
+ .select_drive_strength = sdhci_pci_select_drive_strength,
+ .platform_execute_tuning = amd_execute_tuning,
+};
+
/*****************************************************************************\
* *
* Suspend/resume *
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci-pci: Add support for HS200 tuning mode on AMD eMMC-4.5.1
2016-11-15 6:57 [PATCH 2/2] mmc: sdhci-pci: Add support for HS200 tuning mode on AMD eMMC-4.5.1 Shyam Sundar S K
@ 2016-11-16 12:01 ` Adrian Hunter
0 siblings, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2016-11-16 12:01 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Ulf Hansson, linux-mmc, Sen, Pankaj, Shah, Nehal-bakulchandra,
Agrawal, Nitesh-kumar
On 15/11/16 08:57, Shyam Sundar S K wrote:
> This patch adds support for HS200 tuning mode on AMD eMMC-4.5.1
>
> Reviewed-by: Sen, Pankaj <Pankaj.Sen@amd.com>
> Reviewed-by: Shah, Nehal-bakulchandra <Nehal-bakulchandra.Shah@amd.com>
> Signed-off-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
> ---
> drivers/mmc/host/sdhci-pci-core.c | 177 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 176 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 782c8d2..1a31bdf 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -817,6 +817,170 @@ enum amd_chipset_gen {
> AMD_CHIPSET_UNKNOWN,
> };
>
> +static const struct sdhci_ops amd_sdhci_pci_ops;
You shouldn't need this. Just put amd_sdhci_pci_ops in the right place.
> +
> +struct amd_tuning_descriptor {
> + u8 tune_around;
> + bool this_tune_ok;
> + bool last_tune_ok;
> + u8 valid_front;
> + u8 valid_window_max;
> + u8 tune_low_max;
> + u8 tune_low;
> + u8 valid_window;
> + u8 tune_result;
> +};
> +
> +static int amd_tuning_reset(struct sdhci_host *host)
> +{
> + unsigned int val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + val |= SDHCI_CTRL_PRESET_VAL_ENABLE | SDHCI_CTRL_EXEC_TUNING;
> + sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
> +
> + val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + val &= ~SDHCI_CTRL_EXEC_TUNING;
> + sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + return 0;
> +}
> +
> +static int amd_config_tuning_phase(struct sdhci_host *host, u8 phase)
> +{
> + struct sdhci_pci_slot *slot = sdhci_priv(host);
> + struct pci_dev *pdev = slot->chip->pdev;
> + unsigned int val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + pci_read_config_dword(pdev, 0xb8, &val);
> + val &= ~0x1f;
> + val |= (0x10800 | (phase << 1));
> + pci_write_config_dword(pdev, 0xb8, val);
It wouldn't hurt to #define registers and bit fields.
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + return 0;
> +}
> +
> +static int amd_find_good_phase(struct sdhci_host *host)
> +{
> + struct sdhci_pci_slot *slot = sdhci_priv(host);
> + struct pci_dev *pdev = slot->chip->pdev;
> + struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
> +
> + unsigned int val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + if (td->this_tune_ok)
> + td->valid_front += 1;
So now valid_front is never reset ?
> + if ((!td->this_tune_ok && td->last_tune_ok) ||
> + (td->tune_around == 11)) {
> + if (td->valid_window > td->valid_window_max) {
> + td->valid_window_max = td->valid_window;
> + td->tune_low_max = td->tune_low;
> + }
> + }
> + if (td->this_tune_ok && (!td->last_tune_ok))
> + td->tune_low = td->tune_around;
> + if (!td->this_tune_ok && td->last_tune_ok)
> + td->valid_window = 0;
> + else if (td->this_tune_ok)
> + td->valid_window += 1;
> +
> + td->last_tune_ok = td->this_tune_ok;
> +
> + if (td->tune_around == 11) {
> + if ((td->valid_front + td->valid_window) >
> + td->valid_window_max) {
> + if (td->valid_front > td->valid_window)
> + td->tune_result = ((td->valid_front -
> + td->valid_window) >> 1);
> + else
> + td->tune_result = td->tune_low +
> + ((td->valid_window + td->valid_front) >> 1);
> + } else {
> + td->tune_result = td->tune_low_max +
> + (td->valid_window_max >> 1);
> + }
So tune_result doesn't need to be in amd_tuning_descriptor.
It is not at all obvious why you need to retain amd_tuning_descriptor
between calls to amd_execute_tuning. Please explain.
> +
> + if (td->tune_result > 0x0b)
> + td->tune_result = 0x0b;
> +
> + pci_read_config_dword(pdev, 0xb8, &val);
> + val &= ~0x1f;
> + val |= (0x10800 | (td->tune_result << 1));
> + pci_write_config_dword(pdev, 0xb8, val);
> + }
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + return 0;
> +}
> +
> +static int amd_enable_manual_tuning(struct sdhci_pci_slot *slot)
> +{
> + struct pci_dev *pdev = slot->chip->pdev;
> + unsigned int val;
> +
> + pci_read_config_dword(pdev, 0xd0, &val);
> + val &= 0xffffffcf;
No need to set the bits to 0 if you are then going to OR them with 1.
i.e. (val & 0xffffffcf) | 0x30 == val | 0x30
> + val |= 0x30;
> + pci_write_config_dword(pdev, 0xd0, val);
> +
> + return 0;
> +}
> +
> +static int amd_execute_tuning(struct sdhci_host *host, u32 opcode)
> +{
> + struct sdhci_pci_slot *slot = sdhci_priv(host);
> + struct amd_tuning_descriptor *td = sdhci_pci_priv(slot);
> + u8 ctrl;
> +
> + amd_tuning_reset(host);
> +
> + /*********************************************************************/
> + /* Enabling Software Tuning */
> + /********************************************************************/
> + /* 1. First switch the eMMC to HS200 Mode
> + * 2. Prepare the registers by using the sampling clock select
> + * 3. Send the CMD21 12 times with block length of 64 bytes
> + * 4. Everytime change the clk phase and check for CRC error
> + * (CMD and DATA),if error, soft reset the CMD and DATA line
> + * 5. Calculate the window and set the clock phase.
> + */
> +
> + for (td->tune_around = 0; td->tune_around < 12; td->tune_around++) {
tune_around does not need to be in amd_tuning_descriptor
> + amd_config_tuning_phase(host, td->tune_around);
> +
> + if (mmc_send_tuning(host->mmc, opcode, NULL)) {
> + td->this_tune_ok = false;
> + host->mmc->need_retune = 0;
> + mdelay(4);
Use msleep not mdelay
> + ctrl = SDHCI_RESET_CMD | SDHCI_RESET_DATA;
> + sdhci_writeb(host, ctrl, SDHCI_SOFTWARE_RESET);
> + } else {
> + td->this_tune_ok = true;
this_tune_ok does not need to be in amd_tuning_descriptor
> + }
> +
> + amd_find_good_phase(host);
Pass tune_around and this_tune_ok to amd_find_good_phase
> + }
> +
> + host->mmc->retune_period = 0;
> +
> + amd_enable_manual_tuning(slot);
> + return 0;
> +}
> +
> static int amd_probe(struct sdhci_pci_chip *chip)
> {
> struct pci_dev *smbus_dev;
> @@ -841,7 +1005,6 @@ static int amd_probe(struct sdhci_pci_chip *chip)
>
> if ((gen == AMD_CHIPSET_BEFORE_ML) || (gen == AMD_CHIPSET_CZ)) {
> chip->quirks2 |= SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD;
> - chip->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
> }
>
> return 0;
> @@ -849,6 +1012,7 @@ static int amd_probe(struct sdhci_pci_chip *chip)
>
Put amd_sdhci_pci_ops here.
> static const struct sdhci_pci_fixes sdhci_amd = {
> .probe = amd_probe,
> + .ops = &amd_sdhci_pci_ops,
Need to define the private size i.e.
.priv_size = sizeof(struct amd_tuning_descriptor),
> };
>
> static const struct pci_device_id pci_ids[] = {
> @@ -1469,6 +1633,17 @@ static const struct sdhci_ops sdhci_pci_ops = {
> .select_drive_strength = sdhci_pci_select_drive_strength,
> };
>
> +static const struct sdhci_ops amd_sdhci_pci_ops = {
> + .set_clock = sdhci_set_clock,
> + .enable_dma = sdhci_pci_enable_dma,
> + .set_bus_width = sdhci_pci_set_bus_width,
> + .reset = sdhci_reset,
> + .set_uhs_signaling = sdhci_set_uhs_signaling,
> + .hw_reset = sdhci_pci_hw_reset,
> + .select_drive_strength = sdhci_pci_select_drive_strength,
> + .platform_execute_tuning = amd_execute_tuning,
> +};
Please try to line up the '='
> +
> /*****************************************************************************\
> * *
> * Suspend/resume *
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-16 12:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-15 6:57 [PATCH 2/2] mmc: sdhci-pci: Add support for HS200 tuning mode on AMD eMMC-4.5.1 Shyam Sundar S K
2016-11-16 12:01 ` Adrian Hunter
-- strict thread matches above, loose matches on Subject: below --
2016-11-10 12:51 [PATCH 2/2] mmc: sdhci-pci: Add support for HS200 tuning mode on AMD, eMMC-4.5.1 Shyam Sundar S K
2016-11-11 12:41 ` Adrian Hunter
2016-11-14 6:38 ` Shyam Sundar S K
2016-11-14 13:37 ` Adrian Hunter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).