Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754170AbbLVJwg (ORCPT ); Tue, 22 Dec 2015 04:52:36 -0500 Received: from mail-yk0-f169.google.com ([209.85.160.169]:34569 "EHLO mail-yk0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753564AbbLVJwe (ORCPT ); Tue, 22 Dec 2015 04:52:34 -0500 MIME-Version: 1.0 In-Reply-To: <1450802408-16354-1-git-send-email-vincent.wan@amd.com> References: <1450802408-16354-1-git-send-email-vincent.wan@amd.com> Date: Tue, 22 Dec 2015 11:52:33 +0200 Message-ID: Subject: Re: [PATCH 1/3] mmc: sdhci-pci: Add AMD HS200 mode tuning function From: Andy Shevchenko To: Wan Zongshun Cc: Ulf Hansson , "linux-mmc@vger.kernel.org" , Wan Zongshun , Borislav Petkov , "linux-kernel@vger.kernel.org" , Huang Rui Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6159 Lines: 200 On Tue, Dec 22, 2015 at 6:40 PM, Wan Zongshun wrote: > From: Wan Zongshun > > This patch is to add software tuning functions for AMD hs200 > mode. > > Signed-off-by: Wan Zongshun > --- > drivers/mmc/host/sdhci-pci-core.c | 146 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 146 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c > index 08f4a9f..01c5723 100644 > --- a/drivers/mmc/host/sdhci-pci-core.c > +++ b/drivers/mmc/host/sdhci-pci-core.c > @@ -729,6 +729,152 @@ enum amd_chipset_gen { > AMD_CHIPSET_UNKNOWN, > }; > > +struct tuning_descriptor { > + unsigned char tune_around; > + bool this_tune_ok; > + bool last_tune_ok; > + bool valid_front_end; > + unsigned char valid_front; > + unsigned char valid_window_max; > + unsigned char tune_low_max; > + unsigned char tune_low; > + unsigned char valid_window; > + unsigned char tune_result; > +}; > + > +#define AMD_EMMC_TUNE_REG 0xb8 > +static struct tuning_descriptor tdescriptor; Global variable?! > + > +static int tuning_reset(struct sdhci_host *host) Better prefixes? > +{ > + 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 config_tuning_phase(struct sdhci_host *host, unsigned char 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, AMD_EMMC_TUNE_REG, &val); > + val &= ~0xf; > + val |= (0x10800 | phase); Magic. > + pci_write_config_dword(pdev, AMD_EMMC_TUNE_REG, val); > + > + spin_unlock_irqrestore(&host->lock, flags); > + > + return 0; > +} > + > +static int find_good_phase(struct sdhci_host *host) > +{ > + struct tuning_descriptor *td = &tdescriptor; > + 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); > + > + if (td->this_tune_ok == false) > + td->valid_front_end = 1; > + > + if (td->valid_front_end) > + td->valid_front = td->valid_front; > + else if (td->this_tune_ok) > + td->valid_front = td->valid_front + 1; > + > + if ((!td->this_tune_ok && td->last_tune_ok) || > + (td->tune_around == 11)) { Magic. > + 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) { > + if (!td->last_tune_ok) > + td->tune_low = td->tune_around; > + td->valid_window = td->valid_window + 1; > + } else { > + if (td->last_tune_ok) > + td->valid_window = 0x0; > + } > + > + 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, AMD_EMMC_TUNE_REG, &val); > + val &= ~0xf; > + val |= (0x10800 | td->tune_result); Magic. > + pci_write_config_dword(pdev, AMD_EMMC_TUNE_REG, val); > + } > + > + spin_unlock_irqrestore(&host->lock, flags); > + > + return 0; > +} > + > +static int amd_execute_tuning(struct sdhci_host *host, u32 opcode) > +{ > + struct tuning_descriptor *td = &tdescriptor; > + > + tuning_reset(host); > + > + for (td->tune_around = 0; td->tune_around < 12; td->tune_around++) { Magic. Why loop is done with non-local variable? > + > + 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); > + } else { > + td->this_tune_ok = true; > + } > + > + find_good_phase(host); > + } > + > + host->mmc->retune_period = 0; > + > + return 0; > +} > + > static int amd_probe(struct sdhci_pci_chip *chip) > { > struct pci_dev *smbus_dev; No users for such code. I don't think it makes sense to push it separately. -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/