On 10/01/17 12:09, 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@xxxxxxx> > Reviewed-by: Shah, Nehal-bakulchandra <Nehal-bakulchandra.Shah@xxxxxxx> > Reviewed-by: Agrawal, Nitesh-kumar <Nitesh-kumar.Agrawal@xxxxxxx> > Signed-off-by: S-k, Shyam-sundar <Shyam-sundar.S-k@xxxxxxx> There are just a few minor details left below. > --- > > Changes since v1: (https://patchwork.kernel.org/patch/9429071/) > * Reworked on review comments received which has the following: > -> Removed the amd_tuning_descriptor struct and amd_find_good_phase() > and handled the tuning changes in amd_execute_tuning(). > -> Fix some alignment problems. > -> Defined macros for registers and bit fields used. > -> removed mdelay and used msleep > > Changes since v2: (https://patchwork.kernel.org/patch/9469247/) > * Resubmitted the v1 patch by removing unused variables. > > Changes since v3: (https://patchwork.kernel.org/patch/9469571/) > * Adding a changelog description and resubmitting the changes made in v2 > > Changes since v4: (https://patchwork.kernel.org/patch/9470185/) > * Changed the amd_execute_tuning logic as per Adrian suggestions and > addressed the comments received > > drivers/mmc/host/sdhci-pci-core.c | 103 ++++++++++++++++++++++++++++++++++++-- > 1 file changed, 99 insertions(+), 4 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c > index 1a72d32..b9899d0 100644 > --- a/drivers/mmc/host/sdhci-pci-core.c > +++ b/drivers/mmc/host/sdhci-pci-core.c > @@ -866,11 +866,98 @@ enum amd_chipset_gen { > AMD_CHIPSET_UNKNOWN, > }; > > +/* AMD registers */ > +#define AMD_SD_AUTO_PATTERN 0xB8 > +#define AMD_MSLEEP_DURATION 4 > +#define AMD_SD_MISC_CONTROL 0xD0 > +#define AMD_MAX_TUNE_VALUE 0x0B > +#define AMD_AUTO_TUNE_SEL 0x10800 > +#define AMD_FIFO_PTR 0x30 > +#define AMD_BIT_MASK 0x1F > + > +static int amd_tuning_reset(struct sdhci_host *host) Should return void > +{ > + unsigned int val; > + > + 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); > + > + return 0; > +} > + > +static int amd_config_tuning_phase(struct sdhci_host *host, u8 phase) Should return void and take pdev as a parameter i.e. static void amd_config_tuning_phase(struct pci_dev *pdev, u8 phase) > +{ > + struct sdhci_pci_slot *slot = sdhci_priv(host); > + struct pci_dev *pdev = slot->chip->pdev; > + unsigned int val; > + > + pci_read_config_dword(pdev, AMD_SD_AUTO_PATTERN, &val); > + val &= ~AMD_BIT_MASK; > + val |= (AMD_AUTO_TUNE_SEL | (phase << 1)); > + pci_write_config_dword(pdev, AMD_SD_AUTO_PATTERN, val); > + > + return 0; > +} > + > +static int amd_enable_manual_tuning(struct sdhci_pci_slot *slot) Should return void and take pdev as a parameter i.e. static void amd_enable_manual_tuning(struct pci_dev *pdev) > +{ > + struct pci_dev *pdev = slot->chip->pdev; > + unsigned int val; > + > + pci_read_config_dword(pdev, AMD_SD_MISC_CONTROL, &val); > + val |= AMD_FIFO_PTR; > + pci_write_config_dword(pdev, AMD_SD_MISC_CONTROL, val); > + > + return 0; > +} > + > +static int amd_execute_tuning(struct sdhci_host *host, u32 opcode) > +{ > + struct sdhci_pci_slot *slot = sdhci_priv(host); Get pdev here and pass it to the functions that use it. struct pci_dev *pdev = slot->chip->pdev; > + u8 valid_win = 0; > + u8 valid_win_max = 0; > + u8 valid_win_end = 0; > + u8 ctrl, tune_around; > + > + amd_tuning_reset(host); > + > + for (tune_around = 0; tune_around < 12; tune_around++) { > + amd_config_tuning_phase(host, tune_around); > + > + if (mmc_send_tuning(host->mmc, opcode, NULL)) { > + valid_win = 0; > + msleep(AMD_MSLEEP_DURATION); > + ctrl = SDHCI_RESET_CMD | SDHCI_RESET_DATA; > + sdhci_writeb(host, ctrl, SDHCI_SOFTWARE_RESET); > + } else if (++valid_win > valid_win_max) { > + valid_win_max = valid_win; > + valid_win_end = tune_around; > + } > + } > + > + if (!valid_win_max) { > + dev_err(&slot->chip->pdev->dev, "no tuning point found\n"); Then this can be dev_err(&pdev->dev, "no tuning point found\n"); > + return -EIO; > + } > + > + amd_config_tuning_phase(host, valid_win_end - valid_win_max / 2); > + > + amd_enable_manual_tuning(slot); > + > + host->mmc->retune_period = 0; > + > + return 0; > +} > + > static int amd_probe(struct sdhci_pci_chip *chip) > { > struct pci_dev *smbus_dev; > enum amd_chipset_gen gen; > - Please don't remove this blank line. > smbus_dev = pci_get_device(PCI_VENDOR_ID_AMD, > PCI_DEVICE_ID_AMD_HUDSON2_SMBUS, NULL); > if (smbus_dev) { > @@ -888,16 +975,24 @@ static int amd_probe(struct sdhci_pci_chip *chip) > } > } > > - if ((gen == AMD_CHIPSET_BEFORE_ML) || (gen == AMD_CHIPSET_CZ)) { > + 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; > } > > +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, > + .platform_execute_tuning = amd_execute_tuning, > +}; > + > static const struct sdhci_pci_fixes sdhci_amd = { > .probe = amd_probe, > + .ops = &amd_sdhci_pci_ops, > }; > > static const struct pci_device_id pci_ids[] = { > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
