Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Prabu Thangamuthu <Prabu.T@synopsys.com>
To:     Adrian Hunter <adrian.hunter@intel.com>,
        "ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>
CC:     Manjunath M Bettegowda <Manjunath.MB@synopsys.com>
Subject: Re: [PATCH 1/1] mmc: sdhci-pci-dwc-mshc: synopsys dwc mshc support
Thread-Topic: [PATCH 1/1] mmc: sdhci-pci-dwc-mshc: synopsys dwc mshc support
Thread-Index: AdPxmBIFOmCk2CVqRi+baz/mIdVd7Q==
Date:   Thu, 24 May 2018 11:28:41 +0000
Message-ID: <705D14B1C7978B40A723277C067CEDE2010A9B5491@IN01WEMBXB.internal.synopsys.com>
References: <705D14B1C7978B40A723277C067CEDE2010A9B43CC@IN01WEMBXB.internal.synopsys.com>
 <abe62da4-3ef9-5660-94e1-a3f919df06cd@intel.com>
Accept-Language: en-US, en-IN
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Adrian,=0A=
=0A=
On 5/24/2018 2:06 PM, Adrian Hunter wrote:=0A=
> Hi=0A=
>=0A=
> This patch is mangled.=0A=
We will check it.=0A=
>=0A=
> On 22/05/18 09:42, Prabu Thangamuthu wrote:=0A=
>> To enable Synopsys DWC MSHC controller on HPAS-DX platform connected usi=
ng=0A=
>> PCIe interface. As Clock generation logic is implemented in MMCM module =
of=0A=
>> HAPS-DX platform, we have separate functions to control the MMCM to=0A=
>> generate required clocks with respect to speed mode. Also we have platfo=
rm=0A=
>> specific set_power function to support different VDD of eMMC devices.=0A=
>>=0A=
>> Signed-off-by: Prabu Thangamuthu <prabu.t@synopsys.com>=0A=
>> ---=0A=
>>  MAINTAINERS                           |   7 ++=0A=
>>  drivers/mmc/host/Makefile             |   3 +-=0A=
>>  drivers/mmc/host/sdhci-pci-core.c     |   1 +=0A=
>>  drivers/mmc/host/sdhci-pci-dwc-mshc.c | 146=0A=
>> ++++++++++++++++++++++++++++++++++=0A=
>>  drivers/mmc/host/sdhci-pci-dwc-mshc.h |  37 +++++++++=0A=
>>  drivers/mmc/host/sdhci-pci.h          |   3 +=0A=
>>  6 files changed, 196 insertions(+), 1 deletion(-)=0A=
>>  create mode 100644 drivers/mmc/host/sdhci-pci-dwc-mshc.c=0A=
>>  create mode 100644 drivers/mmc/host/sdhci-pci-dwc-mshc.h=0A=
>>=0A=
>> diff --git a/MAINTAINERS b/MAINTAINERS=0A=
>> index 9051a9c..f1749c4 100644=0A=
>> --- a/MAINTAINERS=0A=
>> +++ b/MAINTAINERS=0A=
>> @@ -12643,6 +12643,13 @@ S:    Maintained=0A=
>>  F:    drivers/mmc/host/sdhci*=0A=
>>  F:    include/linux/mmc/sdhci*=0A=
>>  =0A=
>> +SYNOPSYS SDHCI COMPLIANT DWC MSHC DRIVER=0A=
>> +M:    Prabu Thangamuthu <prabu.t@synopsys.com>=0A=
>> +M:    Manjunath M B <manjumb@synopsys.com>=0A=
>> +L:    linux-mmc@vger.kernel.org=0A=
>> +S:    Maintained=0A=
>> +F:    drivers/mmc/host/sdhci-pci-dwc-mshc*=0A=
>> +=0A=
>>  SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) SAMSUNG DRIVER=0A=
>>  M:    Ben Dooks <ben-linux@fluff.org>=0A=
>>  M:    Jaehoon Chung <jh80.chung@samsung.com>=0A=
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile=0A=
>> index 6aead24..6c0d3fb 100644=0A=
>> --- a/drivers/mmc/host/Makefile=0A=
>> +++ b/drivers/mmc/host/Makefile=0A=
>> @@ -11,7 +11,8 @@ obj-$(CONFIG_MMC_MXC)        +=3D mxcmmc.o=0A=
>>  obj-$(CONFIG_MMC_MXS)        +=3D mxs-mmc.o=0A=
>>  obj-$(CONFIG_MMC_SDHCI)        +=3D sdhci.o=0A=
>>  obj-$(CONFIG_MMC_SDHCI_PCI)    +=3D sdhci-pci.o=0A=
>> -sdhci-pci-y            +=3D sdhci-pci-core.o sdhci-pci-o2micro.o=0A=
>> sdhci-pci-arasan.o=0A=
>> +sdhci-pci-y            +=3D sdhci-pci-core.o sdhci-pci-o2micro.o=0A=
>> sdhci-pci-arasan.o \=0A=
>> +                   sdhci-pci-dwc-mshc.o=0A=
>>  obj-$(subst m,y,$(CONFIG_MMC_SDHCI_PCI))    +=3D sdhci-pci-data.o=0A=
>>  obj-$(CONFIG_MMC_SDHCI_ACPI)    +=3D sdhci-acpi.o=0A=
>>  obj-$(CONFIG_MMC_SDHCI_PXAV3)    +=3D sdhci-pxav3.o=0A=
>> diff --git a/drivers/mmc/host/sdhci-pci-core.c=0A=
>> b/drivers/mmc/host/sdhci-pci-core.c=0A=
>> index 77dd352..96b6963 100644=0A=
>> --- a/drivers/mmc/host/sdhci-pci-core.c=0A=
>> +++ b/drivers/mmc/host/sdhci-pci-core.c=0A=
>> @@ -1511,6 +1511,7 @@ static int amd_probe(struct sdhci_pci_chip *chip)=
=0A=
>>      SDHCI_PCI_DEVICE(O2, SEABIRD0, o2),=0A=
>>      SDHCI_PCI_DEVICE(O2, SEABIRD1, o2),=0A=
>>      SDHCI_PCI_DEVICE(ARASAN, PHY_EMMC, arasan),=0A=
>> +    SDHCI_PCI_DEVICE(SYNOPSYS, DWC_MSHC,  snps),=0A=
>>      SDHCI_PCI_DEVICE_CLASS(AMD, SYSTEM_SDHCI, PCI_CLASS_MASK, amd),=0A=
>>      /* Generic SD host controller */=0A=
>>      {PCI_DEVICE_CLASS(SYSTEM_SDHCI, PCI_CLASS_MASK)},=0A=
>> diff --git a/drivers/mmc/host/sdhci-pci-dwc-mshc.c=0A=
>> b/drivers/mmc/host/sdhci-pci-dwc-mshc.c=0A=
>> new file mode 100644=0A=
>> index 0000000..bca3db4=0A=
>> --- /dev/null=0A=
>> +++ b/drivers/mmc/host/sdhci-pci-dwc-mshc.c=0A=
>> @@ -0,0 +1,146 @@=0A=
>> +// SPDX-License-Identifier: GPL-2.0=0A=
>> +/*=0A=
>> + * SDHCI driver for Synopsys DWC_MSHC controller=0A=
>> + *=0A=
>> + * Copyright (C) 2018 Synopsys, Inc. (www.synopsys.com)=0A=
>> + *=0A=
>> + * Authors:=0A=
>> + *    Prabu Thangamuthu <prabu.t@synopsys.com>=0A=
>> + *    Manjunath M B <manjumb@synopsys.com>=0A=
>> + *=0A=
>> + */=0A=
>> +=0A=
>> +#include <linux/pci.h>=0A=
>> +#include <linux/delay.h>=0A=
>> +#include <linux/module.h>=0A=
>> +#include <linux/moduleparam.h>=0A=
>> +=0A=
>> +#include "sdhci.h"=0A=
>> +#include "sdhci-pci.h"=0A=
>> +#include "sdhci-pci-dwc-mshc.h"=0A=
>> +=0A=
>> +/* Default emmc vdd is set to 3.3V */=0A=
>> +static unsigned int emmc_vdd =3D SDHC_EMMC_VDD_330V;=0A=
>> +module_param(emmc_vdd, int, 0444);=0A=
> Why a module parameter?=0A=
Our platform has slots for eMMC devices which can supports both 1.8 V=0A=
and 3.3 V=0A=
operating modes. We want this module parameter to control the operating=0A=
voltage=0A=
of eMMC devices.=0A=
>> +=0A=
>> +static void sdhci_snps_set_clock(struct sdhci_host *host, unsigned int=
=0A=
>> clock)=0A=
>> +{=0A=
>> +    u16 clk =3D 0;=0A=
>> +    u32 reg =3D 0;=0A=
>> +    u32 vendor_ptr =3D 0;=0A=
>> +=0A=
>> +    vendor_ptr =3D sdhci_readw(host, SDHCI_VENDOR_PTR_R);=0A=
>> +=0A=
>> +    /* Disable Software managed rx tuning */=0A=
>> +    reg =3D sdhci_readl(host, (SDHC_AT_CTRL_R + vendor_ptr));=0A=
>> +    reg &=3D ~SDHC_SW_TUNE_EN;=0A=
>> +    sdhci_writel(host, reg, (SDHC_AT_CTRL_R + vendor_ptr));=0A=
>> +=0A=
>> +    if (clock <=3D 52000000) {=0A=
>> +        sdhci_set_clock(host, clock);=0A=
>> +    } else {=0A=
>> +        /* Assert reset to MMCM */=0A=
>> +        reg =3D sdhci_readl(host, (SDHC_GPIO_OUT + vendor_ptr));=0A=
>> +        reg |=3D SDHC_CCLK_MMCM_RST;=0A=
>> +        sdhci_writel(host, reg, (SDHC_GPIO_OUT + vendor_ptr));=0A=
>> +=0A=
>> +        /* Configure MMCM*/=0A=
> Space between MMCM and *=0A=
Okay.=0A=
>=0A=
>> +        sdhci_writel(host, DIV_REG_100_MHZ, SDHC_MMCM_DIV_REG);=0A=
>> +        sdhci_writel(host, CLKFBOUT_100_MHZ,=0A=
>> +                SDHC_MMCM_CLKFBOUT);=0A=
>> +=0A=
>> +        /* De-assert reset to MMCM*/=0A=
> Space between MMCM and *=0A=
Okay.=0A=
>=0A=
>=0A=
>> +        reg =3D sdhci_readl(host, (SDHC_GPIO_OUT + vendor_ptr));=0A=
>> +        reg &=3D ~SDHC_CCLK_MMCM_RST;=0A=
>> +        sdhci_writel(host, reg, (SDHC_GPIO_OUT + vendor_ptr));=0A=
>> +=0A=
>> +        /* Enable clock */=0A=
>> +        clk =3D SDHCI_PROG_CLOCK_MODE | SDHCI_CLOCK_INT_EN |=0A=
>> +            SDHCI_CLOCK_CARD_EN;=0A=
>> +        sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);=0A=
>> +    }=0A=
>> +}=0A=
>> +=0A=
>> +static void sdhci_snps_set_power(struct sdhci_host *host, unsigned char=
=0A=
>> mode,=0A=
>> +             unsigned short vdd)=0A=
>> +{=0A=
>> +    u8 pwr =3D 0;=0A=
>> +    u16 ctrl;=0A=
>> +=0A=
>> +    if (mode !=3D MMC_POWER_OFF) {=0A=
>> +        switch (1 << vdd) {=0A=
>> +        case MMC_VDD_165_195:=0A=
>> +            pwr =3D SDHCI_POWER_180;=0A=
>> +            break;=0A=
>> +        case MMC_VDD_29_30:=0A=
>> +        case MMC_VDD_30_31:=0A=
>> +            pwr =3D SDHCI_POWER_300;=0A=
>> +            break;=0A=
>> +        case MMC_VDD_32_33:=0A=
>> +        case MMC_VDD_33_34:=0A=
>> +            pwr =3D SDHCI_POWER_330;=0A=
>> +            break;=0A=
>> +        default:=0A=
>> +            WARN(1, "%s: Invalid vdd %#x\n",=0A=
>> +                 mmc_hostname(host->mmc), vdd);=0A=
>> +            break;=0A=
>> +        }=0A=
>> +    }=0A=
>> +=0A=
>> +    if (host->pwr =3D=3D pwr)=0A=
>> +        return;=0A=
>> +=0A=
>> +    host->pwr =3D pwr;=0A=
>> +=0A=
>> +    if (pwr =3D=3D 0) {=0A=
>> +        sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);=0A=
>> +    } else {=0A=
>> +        sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);=0A=
>> +=0A=
>> +        /*=0A=
>> +         * Enable it for eMMC phy cfg1 test with 1.8V mode=0A=
>> +         */=0A=
>> +        if (emmc_vdd =3D=3D SDHC_EMMC_VDD_180V) {=0A=
>> +            pwr =3D SDHCI_POWER_180;=0A=
>> +            sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);=0A=
>> +=0A=
>> +            ctrl =3D sdhci_readw(host, SDHCI_HOST_CONTROL2);=0A=
>> +            /*=0A=
>> +             * Enable 1.8V Signal Enable in the Host Control2=0A=
>> +             * register=0A=
>> +             */=0A=
>> +            ctrl |=3D SDHCI_CTRL_VDD_180;=0A=
>> +            sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);=0A=
>> +        }=0A=
>> +        pwr |=3D SDHCI_POWER_ON;=0A=
>> +=0A=
>> +        sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);=0A=
>> +    }=0A=
>> +}=0A=
>> +=0A=
>> +static int sdhci_snps_pci_probe_slot(struct sdhci_pci_slot *slot)=0A=
>> +{=0A=
>> +    struct sdhci_host *host =3D slot->host;=0A=
>> +=0A=
>> +    host->caps =3D sdhci_readl(host, SDHCI_CAPABILITIES);=0A=
>> +    host->caps1 =3D sdhci_readl(host, SDHCI_CAPABILITIES_1);=0A=
> Why read caps here?=0A=
We had it for logging purpose. we will remove it.=0A=
>=0A=
>> +=0A=
>> +    return 0;=0A=
>> +}=0A=
>> +=0A=
>> +/* Synopsys DWC MSHC Controller based on SDHCI-PCI */=0A=
>> +static const struct sdhci_ops sdhci_snps_ops =3D {=0A=
>> +    .set_clock    =3D sdhci_snps_set_clock,=0A=
>> +    .set_power    =3D sdhci_snps_set_power,=0A=
>> +    .enable_dma    =3D sdhci_pci_enable_dma,=0A=
>> +    .set_bus_width    =3D sdhci_set_bus_width,=0A=
>> +    .reset        =3D sdhci_reset,=0A=
>> +    .set_uhs_signaling =3D sdhci_set_uhs_signaling,=0A=
>> +};=0A=
>> +=0A=
>> +const struct sdhci_pci_fixes sdhci_snps =3D {=0A=
>> +    .probe_slot    =3D sdhci_snps_pci_probe_slot,=0A=
>> +    .ops        =3D &sdhci_snps_ops,=0A=
>> +};=0A=
>> +=0A=
>> +MODULE_PARM_DESC(emmc_vdd, "VDD to configure eMMC device supply voltage=
");=0A=
>> diff --git a/drivers/mmc/host/sdhci-pci-dwc-mshc.h=0A=
>> b/drivers/mmc/host/sdhci-pci-dwc-mshc.h=0A=
>> new file mode 100644=0A=
>> index 0000000..352bbfd=0A=
>> --- /dev/null=0A=
>> +++ b/drivers/mmc/host/sdhci-pci-dwc-mshc.h=0A=
> Please put the definitions into sdhci-pci-dwc-mshc.c and then this file i=
s=0A=
> not needed.=0A=
Okay.=0A=
=0A=
Thanks for review comments.=0A=
=0A=
Regards,=0A=
Prabu.=0A=