RE: [PATCH v2 12/14] mmc: sdhci-pic32: Add PIC32 SDHCI host controller driver
On 12/16/2015 3:48 AM, Ulf Hansson wrote: > > [...] > > > +static int pic32_sdhci_probe(struct platform_device *pdev) { > > + struct device *dev = >dev; > > + struct sdhci_host *host; > > + struct resource *iomem; > > + struct pic32_sdhci_pdata *sdhci_pdata; > > + struct pic32_sdhci_platform_data *plat_data; > > + unsigned int clk_rate = 0; > > + int ret; > > + struct pinctrl *pinctrl; > > + > > + host = sdhci_alloc_host(dev, sizeof(*sdhci_pdata)); > > + if (IS_ERR(host)) { > > + ret = PTR_ERR(host); > > + dev_err(>dev, "cannot allocate memory for sdhci\n"); > > + goto err; > > + } > > + > > + sdhci_pdata = sdhci_priv(host); > > + sdhci_pdata->pdev = pdev; > > + platform_set_drvdata(pdev, host); > > + > > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + host->ioaddr = devm_ioremap_resource(>dev, iomem); > > + if (IS_ERR(host->ioaddr)) { > > + ret = PTR_ERR(host->ioaddr); > > + dev_err(>dev, "unable to map iomem: %d\n", ret); > > + goto err_host; > > + } > > + > > + plat_data = pdev->dev.platform_data; > > + if (plat_data && plat_data->setup_dma) { > > + ret = plat_data->setup_dma(ADMA_FIFO_RD_THSHLD, > > + ADMA_FIFO_WR_THSHLD); > > + if (ret) > > + goto err_host; > > + } > > + > > + pinctrl = devm_pinctrl_get_select_default(>dev); > > This isn't need as it's already handled by the PM core. Ack. Will remove. > > > + if (IS_ERR(pinctrl)) { > > + ret = PTR_ERR(pinctrl); > > + dev_warn(>dev, "No pinctrl provided %d\n", ret); > > + if (ret == -EPROBE_DEFER) > > + goto err_host; > > + } > > + > > + host->ops = _sdhci_ops; > > + host->irq = platform_get_irq(pdev, 0); > > + > > + sdhci_pdata->sys_clk = devm_clk_get(>dev, "sys_clk"); > > + if (IS_ERR(sdhci_pdata->sys_clk)) { > > + ret = PTR_ERR(sdhci_pdata->sys_clk); > > + dev_err(>dev, "Error getting clock\n"); > > + goto err_host; > > + } > > + > > + /* Enable clock when available! */ > > + ret = clk_prepare_enable(sdhci_pdata->sys_clk); > > + if (ret) { > > + dev_dbg(>dev, "Error enabling clock\n"); > > + goto err_host; > > + } > > + > > + /* SDH CLK enable */ > > + sdhci_pdata->base_clk = devm_clk_get(>dev, "base_clk"); > > + if (IS_ERR(sdhci_pdata->base_clk)) { > > + ret = PTR_ERR(sdhci_pdata->base_clk); > > + dev_err(>dev, "Error getting clock\n"); > > + goto err_host; > > + } > > + > > + /* Enable clock when available! */ > > + ret = clk_prepare_enable(sdhci_pdata->base_clk); > > + if (ret) { > > + dev_dbg(>dev, "Error enabling clock\n"); > > + goto err_host; > > + } > > + > > + clk_rate = clk_get_rate(sdhci_pdata->base_clk); > > + dev_dbg(>dev, "base clock at: %u\n", clk_rate); > > + clk_rate = clk_get_rate(sdhci_pdata->sys_clk); > > + dev_dbg(>dev, "sys clock at: %u\n", clk_rate); > > This looks like some leftover from a debugging task. Can you remove them? > Yes, these are not needed and will be removed. > > + > > + host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V; > > + > > + host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT; > > + > > + ret = mmc_of_parse(host->mmc); > > + if (ret) > > From this point, the error handling doesn't undo clk_prepare_enable(). > Please add that. Ack. Will do. > > > + goto err_host; > > + > > + ret = pic32_sdhci_probe_platform(pdev, sdhci_pdata); > > + if (ret) { > > + dev_err(>dev, "failed to probe platform!\n"); > > + goto err_host; > > + } > > + > > + ret = sdhci_add_host(host); > > + if (ret) { > > + dev_dbg(>dev, "error adding host\n"); > > + goto err_host; > > + } > > + > > + dev_info(>dev, "Successfully added sdhci host\n"); > > + return 0; > > + > > +err_host: > > + sdhci_free_host(host); > > +err: > > + dev_err(>dev, "pic32-sdhci probe failed: %d\n", ret); > > + return ret; > > A general comment for the ->probe() and the below ->remove() callback, is that > you should probably be able to convert to use > sdhci_pltfm_init() and sdhci_pltfm_free() in favor of > sdhci_alloc_host() and sdhci_free_host(). > > I think that could simplify the code a bit. Ok. Will do. > > > +} > > + > > +static int pic32_sdhci_remove(struct platform_device *pdev) { > > + struct sdhci_host *host = platform_get_drvdata(pdev); > > + struct pic32_sdhci_pdata *sdhci_pdata = sdhci_priv(host); > > + int dead = 0; > > +
RE: [PATCH v2 12/14] mmc: sdhci-pic32: Add PIC32 SDHCI host controller driver
On 12/16/2015 3:48 AM, Ulf Hansson wrote: > > [...] > > > +static int pic32_sdhci_probe(struct platform_device *pdev) { > > + struct device *dev = >dev; > > + struct sdhci_host *host; > > + struct resource *iomem; > > + struct pic32_sdhci_pdata *sdhci_pdata; > > + struct pic32_sdhci_platform_data *plat_data; > > + unsigned int clk_rate = 0; > > + int ret; > > + struct pinctrl *pinctrl; > > + > > + host = sdhci_alloc_host(dev, sizeof(*sdhci_pdata)); > > + if (IS_ERR(host)) { > > + ret = PTR_ERR(host); > > + dev_err(>dev, "cannot allocate memory for sdhci\n"); > > + goto err; > > + } > > + > > + sdhci_pdata = sdhci_priv(host); > > + sdhci_pdata->pdev = pdev; > > + platform_set_drvdata(pdev, host); > > + > > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + host->ioaddr = devm_ioremap_resource(>dev, iomem); > > + if (IS_ERR(host->ioaddr)) { > > + ret = PTR_ERR(host->ioaddr); > > + dev_err(>dev, "unable to map iomem: %d\n", ret); > > + goto err_host; > > + } > > + > > + plat_data = pdev->dev.platform_data; > > + if (plat_data && plat_data->setup_dma) { > > + ret = plat_data->setup_dma(ADMA_FIFO_RD_THSHLD, > > + ADMA_FIFO_WR_THSHLD); > > + if (ret) > > + goto err_host; > > + } > > + > > + pinctrl = devm_pinctrl_get_select_default(>dev); > > This isn't need as it's already handled by the PM core. Ack. Will remove. > > > + if (IS_ERR(pinctrl)) { > > + ret = PTR_ERR(pinctrl); > > + dev_warn(>dev, "No pinctrl provided %d\n", ret); > > + if (ret == -EPROBE_DEFER) > > + goto err_host; > > + } > > + > > + host->ops = _sdhci_ops; > > + host->irq = platform_get_irq(pdev, 0); > > + > > + sdhci_pdata->sys_clk = devm_clk_get(>dev, "sys_clk"); > > + if (IS_ERR(sdhci_pdata->sys_clk)) { > > + ret = PTR_ERR(sdhci_pdata->sys_clk); > > + dev_err(>dev, "Error getting clock\n"); > > + goto err_host; > > + } > > + > > + /* Enable clock when available! */ > > + ret = clk_prepare_enable(sdhci_pdata->sys_clk); > > + if (ret) { > > + dev_dbg(>dev, "Error enabling clock\n"); > > + goto err_host; > > + } > > + > > + /* SDH CLK enable */ > > + sdhci_pdata->base_clk = devm_clk_get(>dev, "base_clk"); > > + if (IS_ERR(sdhci_pdata->base_clk)) { > > + ret = PTR_ERR(sdhci_pdata->base_clk); > > + dev_err(>dev, "Error getting clock\n"); > > + goto err_host; > > + } > > + > > + /* Enable clock when available! */ > > + ret = clk_prepare_enable(sdhci_pdata->base_clk); > > + if (ret) { > > + dev_dbg(>dev, "Error enabling clock\n"); > > + goto err_host; > > + } > > + > > + clk_rate = clk_get_rate(sdhci_pdata->base_clk); > > + dev_dbg(>dev, "base clock at: %u\n", clk_rate); > > + clk_rate = clk_get_rate(sdhci_pdata->sys_clk); > > + dev_dbg(>dev, "sys clock at: %u\n", clk_rate); > > This looks like some leftover from a debugging task. Can you remove them? > Yes, these are not needed and will be removed. > > + > > + host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V; > > + > > + host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT; > > + > > + ret = mmc_of_parse(host->mmc); > > + if (ret) > > From this point, the error handling doesn't undo clk_prepare_enable(). > Please add that. Ack. Will do. > > > + goto err_host; > > + > > + ret = pic32_sdhci_probe_platform(pdev, sdhci_pdata); > > + if (ret) { > > + dev_err(>dev, "failed to probe platform!\n"); > > + goto err_host; > > + } > > + > > + ret = sdhci_add_host(host); > > + if (ret) { > > + dev_dbg(>dev, "error adding host\n"); > > + goto err_host; > > + } > > + > > + dev_info(>dev, "Successfully added sdhci host\n"); > > + return 0; > > + > > +err_host: > > + sdhci_free_host(host); > > +err: > > + dev_err(>dev, "pic32-sdhci probe failed: %d\n", ret); > > + return ret; > > A general comment for the ->probe() and the below ->remove() callback, is that > you should probably be able to convert to use > sdhci_pltfm_init() and sdhci_pltfm_free() in favor of > sdhci_alloc_host() and sdhci_free_host(). > > I think that could simplify the code a bit. Ok. Will do. > > > +} > > + > > +static int pic32_sdhci_remove(struct platform_device *pdev) { > > + struct sdhci_host *host = platform_get_drvdata(pdev); > > + struct pic32_sdhci_pdata *sdhci_pdata = sdhci_priv(host); > > + int dead = 0; > > +
RE: [PATCH v2 12/14] mmc: sdhci-pic32: Add PIC32 SDHCI host controller driver
On 12/14/2015 5:33 PM, Andy Green wrote: > Hi... looks good, just some small general comments. > > On 15 December 2015 at 06:42, Joshua Henderson > wrote: > > From: Andrei Pistirica > > > > This driver supports the SDHCI host controller found on a PIC32. > > > > Signed-off-by: Andrei Pistirica > > Signed-off-by: Joshua Henderson > > Cc: Ralf Baechle > > --- > > drivers/mmc/host/Kconfig | 11 ++ > > drivers/mmc/host/Makefile |1 + > > drivers/mmc/host/sdhci-pic32.c | 291 > > > > 3 files changed, 303 insertions(+) > > create mode 100644 drivers/mmc/host/sdhci-pic32.c > > > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index > > 1dee533..1a3a42b 100644 > > --- a/drivers/mmc/host/Kconfig > > +++ b/drivers/mmc/host/Kconfig > > @@ -785,3 +785,14 @@ config MMC_MTK > > If you have a machine with a integrated SD/MMC card reader, say Y > > or M > here. > > This is needed if support for any SD/SDIO/MMC devices is required. > > If unsure, say N. > > + > > +config MMC_SDHCI_MICROCHIP_PIC32 > > +tristate "Microchip PIC32MZDA SDHCI support" > > +depends on MMC_SDHCI && PIC32MZDA > > +help > > + This selects the Secure Digital Host Controller Interface (SDHCI) > > + for PIC32MZDA platform. > > + > > + If you have a controller with this interface, say Y or M here. > > + > > + If unsure, say N. > > diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile > > index 3595f83..af918d2 100644 > > --- a/drivers/mmc/host/Makefile > > +++ b/drivers/mmc/host/Makefile > > @@ -75,6 +75,7 @@ obj-$(CONFIG_MMC_SDHCI_BCM2835) += sdhci- > bcm2835.o > > obj-$(CONFIG_MMC_SDHCI_IPROC) += sdhci-iproc.o > > obj-$(CONFIG_MMC_SDHCI_MSM)+= sdhci-msm.o > > obj-$(CONFIG_MMC_SDHCI_ST) += sdhci-st.o > > +obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)+= sdhci-pic32.o > > > > ifeq ($(CONFIG_CB710_DEBUG),y) > > CFLAGS-cb710-mmc+= -DDEBUG > > diff --git a/drivers/mmc/host/sdhci-pic32.c > > b/drivers/mmc/host/sdhci-pic32.c new file mode 100644 index > > 000..b7d7da2 > > --- /dev/null > > +++ b/drivers/mmc/host/sdhci-pic32.c > > @@ -0,0 +1,291 @@ > > +/* > > + * Support of SDHCI platform devices for Microchip PIC32. > > + * > > + * Copyright (C) 2015 Microchip > > + * Andrei Pistirica, Paul Thacker > > + * > > + * Inspired by sdhci-pltfm.c > > + * > > + * This file is licensed under the terms of the GNU General Public > > + * License version 2. This program is licensed "as is" without any > > + * warranty of any kind, whether express or implied. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include "sdhci.h" > > +#include > > + > > +#define SDH_SHARED_BUS_CTRL0x00E0 > > +#define SDH_SHARED_BUS_NR_CLK_PINS_MASK0x7 > > +#define SDH_SHARED_BUS_NR_IRQ_PINS_MASK0x30 > > +#define SDH_SHARED_BUS_CLK_PINS0x10 > > +#define SDH_SHARED_BUS_IRQ_PINS0x14 > > +#define SDH_CAPS_SDH_SLOT_TYPE_MASK0xC000 > > +#define SDH_SLOT_TYPE_REMOVABLE0x0 > > +#define SDH_SLOT_TYPE_EMBEDDED 0x1 > > +#define SDH_SLOT_TYPE_SHARED_BUS 0x2 > > +#define SDHCI_CTRL_CDSSEL 0x80 > > +#define SDHCI_CTRL_CDTLVL 0x40 > > + > > +#define ADMA_FIFO_RD_THSHLD512 > > +#define ADMA_FIFO_WR_THSHLD512 > > + > > +#define DEV_NAME "pic32-sdhci" > > Is there any point defining this when it only has one use in the driver? Ack. Will remove. > > > +struct pic32_sdhci_pdata { > > + struct platform_device *pdev; > > + struct clk *sys_clk; > > + struct clk *base_clk; > > +}; > > + > > +static unsigned int pic32_sdhci_get_max_clock(struct sdhci_host > > +*host) { > > + struct pic32_sdhci_pdata *sdhci_pdata = sdhci_priv(host); > > + > > + return clk_get_rate(sdhci_pdata->base_clk); > > +} > > + > > +static void pic32_sdhci_set_bus_width(struct sdhci_host *host, int > > +width) { > > + u8 ctrl; > > + > > + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); > > + if (width == MMC_BUS_WIDTH_8) { > > + ctrl &= ~SDHCI_CTRL_4BITBUS; > > + if (host->version >= SDHCI_SPEC_300) > > + ctrl |= SDHCI_CTRL_8BITBUS; > > + } else { > > + if (host->version >= SDHCI_SPEC_300) > > + ctrl &= ~SDHCI_CTRL_8BITBUS; > > + if (width == MMC_BUS_WIDTH_4) > > + ctrl |= SDHCI_CTRL_4BITBUS; > > + else > > + ctrl &= ~SDHCI_CTRL_4BITBUS; > > + } > > + /* > > +* SDHCI will not work if JTAG is not Connected.As a workaround fix, > > +* set Card Detect Signal Selection
Re: [PATCH v2 12/14] mmc: sdhci-pic32: Add PIC32 SDHCI host controller driver
[...] > +static int pic32_sdhci_probe(struct platform_device *pdev) > +{ > + struct device *dev = >dev; > + struct sdhci_host *host; > + struct resource *iomem; > + struct pic32_sdhci_pdata *sdhci_pdata; > + struct pic32_sdhci_platform_data *plat_data; > + unsigned int clk_rate = 0; > + int ret; > + struct pinctrl *pinctrl; > + > + host = sdhci_alloc_host(dev, sizeof(*sdhci_pdata)); > + if (IS_ERR(host)) { > + ret = PTR_ERR(host); > + dev_err(>dev, "cannot allocate memory for sdhci\n"); > + goto err; > + } > + > + sdhci_pdata = sdhci_priv(host); > + sdhci_pdata->pdev = pdev; > + platform_set_drvdata(pdev, host); > + > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + host->ioaddr = devm_ioremap_resource(>dev, iomem); > + if (IS_ERR(host->ioaddr)) { > + ret = PTR_ERR(host->ioaddr); > + dev_err(>dev, "unable to map iomem: %d\n", ret); > + goto err_host; > + } > + > + plat_data = pdev->dev.platform_data; > + if (plat_data && plat_data->setup_dma) { > + ret = plat_data->setup_dma(ADMA_FIFO_RD_THSHLD, > + ADMA_FIFO_WR_THSHLD); > + if (ret) > + goto err_host; > + } > + > + pinctrl = devm_pinctrl_get_select_default(>dev); This isn't need as it's already handled by the PM core. > + if (IS_ERR(pinctrl)) { > + ret = PTR_ERR(pinctrl); > + dev_warn(>dev, "No pinctrl provided %d\n", ret); > + if (ret == -EPROBE_DEFER) > + goto err_host; > + } > + > + host->ops = _sdhci_ops; > + host->irq = platform_get_irq(pdev, 0); > + > + sdhci_pdata->sys_clk = devm_clk_get(>dev, "sys_clk"); > + if (IS_ERR(sdhci_pdata->sys_clk)) { > + ret = PTR_ERR(sdhci_pdata->sys_clk); > + dev_err(>dev, "Error getting clock\n"); > + goto err_host; > + } > + > + /* Enable clock when available! */ > + ret = clk_prepare_enable(sdhci_pdata->sys_clk); > + if (ret) { > + dev_dbg(>dev, "Error enabling clock\n"); > + goto err_host; > + } > + > + /* SDH CLK enable */ > + sdhci_pdata->base_clk = devm_clk_get(>dev, "base_clk"); > + if (IS_ERR(sdhci_pdata->base_clk)) { > + ret = PTR_ERR(sdhci_pdata->base_clk); > + dev_err(>dev, "Error getting clock\n"); > + goto err_host; > + } > + > + /* Enable clock when available! */ > + ret = clk_prepare_enable(sdhci_pdata->base_clk); > + if (ret) { > + dev_dbg(>dev, "Error enabling clock\n"); > + goto err_host; > + } > + > + clk_rate = clk_get_rate(sdhci_pdata->base_clk); > + dev_dbg(>dev, "base clock at: %u\n", clk_rate); > + clk_rate = clk_get_rate(sdhci_pdata->sys_clk); > + dev_dbg(>dev, "sys clock at: %u\n", clk_rate); This looks like some leftover from a debugging task. Can you remove them? > + > + host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V; > + > + host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT; > + > + ret = mmc_of_parse(host->mmc); > + if (ret) >From this point, the error handling doesn't undo clk_prepare_enable(). Please add that. > + goto err_host; > + > + ret = pic32_sdhci_probe_platform(pdev, sdhci_pdata); > + if (ret) { > + dev_err(>dev, "failed to probe platform!\n"); > + goto err_host; > + } > + > + ret = sdhci_add_host(host); > + if (ret) { > + dev_dbg(>dev, "error adding host\n"); > + goto err_host; > + } > + > + dev_info(>dev, "Successfully added sdhci host\n"); > + return 0; > + > +err_host: > + sdhci_free_host(host); > +err: > + dev_err(>dev, "pic32-sdhci probe failed: %d\n", ret); > + return ret; A general comment for the ->probe() and the below ->remove() callback, is that you should probably be able to convert to use sdhci_pltfm_init() and sdhci_pltfm_free() in favor of sdhci_alloc_host() and sdhci_free_host(). I think that could simplify the code a bit. > +} > + > +static int pic32_sdhci_remove(struct platform_device *pdev) > +{ > + struct sdhci_host *host = platform_get_drvdata(pdev); > + struct pic32_sdhci_pdata *sdhci_pdata = sdhci_priv(host); > + int dead = 0; > + u32 scratch; > + > + scratch = readl(host->ioaddr + SDHCI_INT_STATUS); > + if (scratch == (u32)-1) > + dead = 1; > + > + sdhci_remove_host(host, dead); > + clk_disable_unprepare(sdhci_pdata->base_clk); > + clk_disable_unprepare(sdhci_pdata->sys_clk); > + sdhci_free_host(host); > + > + return 0; > +} > + > +static const struct of_device_id
RE: [PATCH v2 12/14] mmc: sdhci-pic32: Add PIC32 SDHCI host controller driver
On 12/14/2015 5:33 PM, Andy Green wrote: > Hi... looks good, just some small general comments. > > On 15 December 2015 at 06:42, Joshua Henderson >wrote: > > From: Andrei Pistirica > > > > This driver supports the SDHCI host controller found on a PIC32. > > > > Signed-off-by: Andrei Pistirica > > Signed-off-by: Joshua Henderson > > Cc: Ralf Baechle > > --- > > drivers/mmc/host/Kconfig | 11 ++ > > drivers/mmc/host/Makefile |1 + > > drivers/mmc/host/sdhci-pic32.c | 291 > > > > 3 files changed, 303 insertions(+) > > create mode 100644 drivers/mmc/host/sdhci-pic32.c > > > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index > > 1dee533..1a3a42b 100644 > > --- a/drivers/mmc/host/Kconfig > > +++ b/drivers/mmc/host/Kconfig > > @@ -785,3 +785,14 @@ config MMC_MTK > > If you have a machine with a integrated SD/MMC card reader, say Y > > or M > here. > > This is needed if support for any SD/SDIO/MMC devices is required. > > If unsure, say N. > > + > > +config MMC_SDHCI_MICROCHIP_PIC32 > > +tristate "Microchip PIC32MZDA SDHCI support" > > +depends on MMC_SDHCI && PIC32MZDA > > +help > > + This selects the Secure Digital Host Controller Interface (SDHCI) > > + for PIC32MZDA platform. > > + > > + If you have a controller with this interface, say Y or M here. > > + > > + If unsure, say N. > > diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile > > index 3595f83..af918d2 100644 > > --- a/drivers/mmc/host/Makefile > > +++ b/drivers/mmc/host/Makefile > > @@ -75,6 +75,7 @@ obj-$(CONFIG_MMC_SDHCI_BCM2835) += sdhci- > bcm2835.o > > obj-$(CONFIG_MMC_SDHCI_IPROC) += sdhci-iproc.o > > obj-$(CONFIG_MMC_SDHCI_MSM)+= sdhci-msm.o > > obj-$(CONFIG_MMC_SDHCI_ST) += sdhci-st.o > > +obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)+= sdhci-pic32.o > > > > ifeq ($(CONFIG_CB710_DEBUG),y) > > CFLAGS-cb710-mmc+= -DDEBUG > > diff --git a/drivers/mmc/host/sdhci-pic32.c > > b/drivers/mmc/host/sdhci-pic32.c new file mode 100644 index > > 000..b7d7da2 > > --- /dev/null > > +++ b/drivers/mmc/host/sdhci-pic32.c > > @@ -0,0 +1,291 @@ > > +/* > > + * Support of SDHCI platform devices for Microchip PIC32. > > + * > > + * Copyright (C) 2015 Microchip > > + * Andrei Pistirica, Paul Thacker > > + * > > + * Inspired by sdhci-pltfm.c > > + * > > + * This file is licensed under the terms of the GNU General Public > > + * License version 2. This program is licensed "as is" without any > > + * warranty of any kind, whether express or implied. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include "sdhci.h" > > +#include > > + > > +#define SDH_SHARED_BUS_CTRL0x00E0 > > +#define SDH_SHARED_BUS_NR_CLK_PINS_MASK0x7 > > +#define SDH_SHARED_BUS_NR_IRQ_PINS_MASK0x30 > > +#define SDH_SHARED_BUS_CLK_PINS0x10 > > +#define SDH_SHARED_BUS_IRQ_PINS0x14 > > +#define SDH_CAPS_SDH_SLOT_TYPE_MASK0xC000 > > +#define SDH_SLOT_TYPE_REMOVABLE0x0 > > +#define SDH_SLOT_TYPE_EMBEDDED 0x1 > > +#define SDH_SLOT_TYPE_SHARED_BUS 0x2 > > +#define SDHCI_CTRL_CDSSEL 0x80 > > +#define SDHCI_CTRL_CDTLVL 0x40 > > + > > +#define ADMA_FIFO_RD_THSHLD512 > > +#define ADMA_FIFO_WR_THSHLD512 > > + > > +#define DEV_NAME "pic32-sdhci" > > Is there any point defining this when it only has one use in the driver? Ack. Will remove. > > > +struct pic32_sdhci_pdata { > > + struct platform_device *pdev; > > + struct clk *sys_clk; > > + struct clk *base_clk; > > +}; > > + > > +static unsigned int pic32_sdhci_get_max_clock(struct sdhci_host > > +*host) { > > + struct pic32_sdhci_pdata *sdhci_pdata = sdhci_priv(host); > > + > > + return clk_get_rate(sdhci_pdata->base_clk); > > +} > > + > > +static void pic32_sdhci_set_bus_width(struct sdhci_host *host, int > > +width) { > > + u8 ctrl; > > + > > + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); > > + if (width == MMC_BUS_WIDTH_8) { > > + ctrl &= ~SDHCI_CTRL_4BITBUS; > > + if (host->version >= SDHCI_SPEC_300) > > + ctrl |= SDHCI_CTRL_8BITBUS; > > + } else { > > + if (host->version >= SDHCI_SPEC_300) > > + ctrl &= ~SDHCI_CTRL_8BITBUS; > > + if (width == MMC_BUS_WIDTH_4) > > + ctrl |= SDHCI_CTRL_4BITBUS; > > + else > > + ctrl &= ~SDHCI_CTRL_4BITBUS; > > +
Re: [PATCH v2 12/14] mmc: sdhci-pic32: Add PIC32 SDHCI host controller driver
[...] > +static int pic32_sdhci_probe(struct platform_device *pdev) > +{ > + struct device *dev = >dev; > + struct sdhci_host *host; > + struct resource *iomem; > + struct pic32_sdhci_pdata *sdhci_pdata; > + struct pic32_sdhci_platform_data *plat_data; > + unsigned int clk_rate = 0; > + int ret; > + struct pinctrl *pinctrl; > + > + host = sdhci_alloc_host(dev, sizeof(*sdhci_pdata)); > + if (IS_ERR(host)) { > + ret = PTR_ERR(host); > + dev_err(>dev, "cannot allocate memory for sdhci\n"); > + goto err; > + } > + > + sdhci_pdata = sdhci_priv(host); > + sdhci_pdata->pdev = pdev; > + platform_set_drvdata(pdev, host); > + > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + host->ioaddr = devm_ioremap_resource(>dev, iomem); > + if (IS_ERR(host->ioaddr)) { > + ret = PTR_ERR(host->ioaddr); > + dev_err(>dev, "unable to map iomem: %d\n", ret); > + goto err_host; > + } > + > + plat_data = pdev->dev.platform_data; > + if (plat_data && plat_data->setup_dma) { > + ret = plat_data->setup_dma(ADMA_FIFO_RD_THSHLD, > + ADMA_FIFO_WR_THSHLD); > + if (ret) > + goto err_host; > + } > + > + pinctrl = devm_pinctrl_get_select_default(>dev); This isn't need as it's already handled by the PM core. > + if (IS_ERR(pinctrl)) { > + ret = PTR_ERR(pinctrl); > + dev_warn(>dev, "No pinctrl provided %d\n", ret); > + if (ret == -EPROBE_DEFER) > + goto err_host; > + } > + > + host->ops = _sdhci_ops; > + host->irq = platform_get_irq(pdev, 0); > + > + sdhci_pdata->sys_clk = devm_clk_get(>dev, "sys_clk"); > + if (IS_ERR(sdhci_pdata->sys_clk)) { > + ret = PTR_ERR(sdhci_pdata->sys_clk); > + dev_err(>dev, "Error getting clock\n"); > + goto err_host; > + } > + > + /* Enable clock when available! */ > + ret = clk_prepare_enable(sdhci_pdata->sys_clk); > + if (ret) { > + dev_dbg(>dev, "Error enabling clock\n"); > + goto err_host; > + } > + > + /* SDH CLK enable */ > + sdhci_pdata->base_clk = devm_clk_get(>dev, "base_clk"); > + if (IS_ERR(sdhci_pdata->base_clk)) { > + ret = PTR_ERR(sdhci_pdata->base_clk); > + dev_err(>dev, "Error getting clock\n"); > + goto err_host; > + } > + > + /* Enable clock when available! */ > + ret = clk_prepare_enable(sdhci_pdata->base_clk); > + if (ret) { > + dev_dbg(>dev, "Error enabling clock\n"); > + goto err_host; > + } > + > + clk_rate = clk_get_rate(sdhci_pdata->base_clk); > + dev_dbg(>dev, "base clock at: %u\n", clk_rate); > + clk_rate = clk_get_rate(sdhci_pdata->sys_clk); > + dev_dbg(>dev, "sys clock at: %u\n", clk_rate); This looks like some leftover from a debugging task. Can you remove them? > + > + host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V; > + > + host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT; > + > + ret = mmc_of_parse(host->mmc); > + if (ret) >From this point, the error handling doesn't undo clk_prepare_enable(). Please add that. > + goto err_host; > + > + ret = pic32_sdhci_probe_platform(pdev, sdhci_pdata); > + if (ret) { > + dev_err(>dev, "failed to probe platform!\n"); > + goto err_host; > + } > + > + ret = sdhci_add_host(host); > + if (ret) { > + dev_dbg(>dev, "error adding host\n"); > + goto err_host; > + } > + > + dev_info(>dev, "Successfully added sdhci host\n"); > + return 0; > + > +err_host: > + sdhci_free_host(host); > +err: > + dev_err(>dev, "pic32-sdhci probe failed: %d\n", ret); > + return ret; A general comment for the ->probe() and the below ->remove() callback, is that you should probably be able to convert to use sdhci_pltfm_init() and sdhci_pltfm_free() in favor of sdhci_alloc_host() and sdhci_free_host(). I think that could simplify the code a bit. > +} > + > +static int pic32_sdhci_remove(struct platform_device *pdev) > +{ > + struct sdhci_host *host = platform_get_drvdata(pdev); > + struct pic32_sdhci_pdata *sdhci_pdata = sdhci_priv(host); > + int dead = 0; > + u32 scratch; > + > + scratch = readl(host->ioaddr + SDHCI_INT_STATUS); > + if (scratch == (u32)-1) > + dead = 1; > + > + sdhci_remove_host(host, dead); > + clk_disable_unprepare(sdhci_pdata->base_clk); > + clk_disable_unprepare(sdhci_pdata->sys_clk); > + sdhci_free_host(host); > + > + return 0; > +} > + > +static const struct of_device_id
Re: [PATCH v2 12/14] mmc: sdhci-pic32: Add PIC32 SDHCI host controller driver
Hi... looks good, just some small general comments. On 15 December 2015 at 06:42, Joshua Henderson wrote: > From: Andrei Pistirica > > This driver supports the SDHCI host controller found on a PIC32. > > Signed-off-by: Andrei Pistirica > Signed-off-by: Joshua Henderson > Cc: Ralf Baechle > --- > drivers/mmc/host/Kconfig | 11 ++ > drivers/mmc/host/Makefile |1 + > drivers/mmc/host/sdhci-pic32.c | 291 > > 3 files changed, 303 insertions(+) > create mode 100644 drivers/mmc/host/sdhci-pic32.c > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index 1dee533..1a3a42b 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -785,3 +785,14 @@ config MMC_MTK > If you have a machine with a integrated SD/MMC card reader, say Y > or M here. > This is needed if support for any SD/SDIO/MMC devices is required. > If unsure, say N. > + > +config MMC_SDHCI_MICROCHIP_PIC32 > +tristate "Microchip PIC32MZDA SDHCI support" > +depends on MMC_SDHCI && PIC32MZDA > +help > + This selects the Secure Digital Host Controller Interface (SDHCI) > + for PIC32MZDA platform. > + > + If you have a controller with this interface, say Y or M here. > + > + If unsure, say N. > diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile > index 3595f83..af918d2 100644 > --- a/drivers/mmc/host/Makefile > +++ b/drivers/mmc/host/Makefile > @@ -75,6 +75,7 @@ obj-$(CONFIG_MMC_SDHCI_BCM2835) += > sdhci-bcm2835.o > obj-$(CONFIG_MMC_SDHCI_IPROC) += sdhci-iproc.o > obj-$(CONFIG_MMC_SDHCI_MSM)+= sdhci-msm.o > obj-$(CONFIG_MMC_SDHCI_ST) += sdhci-st.o > +obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)+= sdhci-pic32.o > > ifeq ($(CONFIG_CB710_DEBUG),y) > CFLAGS-cb710-mmc+= -DDEBUG > diff --git a/drivers/mmc/host/sdhci-pic32.c b/drivers/mmc/host/sdhci-pic32.c > new file mode 100644 > index 000..b7d7da2 > --- /dev/null > +++ b/drivers/mmc/host/sdhci-pic32.c > @@ -0,0 +1,291 @@ > +/* > + * Support of SDHCI platform devices for Microchip PIC32. > + * > + * Copyright (C) 2015 Microchip > + * Andrei Pistirica, Paul Thacker > + * > + * Inspired by sdhci-pltfm.c > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "sdhci.h" > +#include > + > +#define SDH_SHARED_BUS_CTRL0x00E0 > +#define SDH_SHARED_BUS_NR_CLK_PINS_MASK0x7 > +#define SDH_SHARED_BUS_NR_IRQ_PINS_MASK0x30 > +#define SDH_SHARED_BUS_CLK_PINS0x10 > +#define SDH_SHARED_BUS_IRQ_PINS0x14 > +#define SDH_CAPS_SDH_SLOT_TYPE_MASK0xC000 > +#define SDH_SLOT_TYPE_REMOVABLE0x0 > +#define SDH_SLOT_TYPE_EMBEDDED 0x1 > +#define SDH_SLOT_TYPE_SHARED_BUS 0x2 > +#define SDHCI_CTRL_CDSSEL 0x80 > +#define SDHCI_CTRL_CDTLVL 0x40 > + > +#define ADMA_FIFO_RD_THSHLD512 > +#define ADMA_FIFO_WR_THSHLD512 > + > +#define DEV_NAME "pic32-sdhci" Is there any point defining this when it only has one use in the driver? > +struct pic32_sdhci_pdata { > + struct platform_device *pdev; > + struct clk *sys_clk; > + struct clk *base_clk; > +}; > + > +static unsigned int pic32_sdhci_get_max_clock(struct sdhci_host *host) > +{ > + struct pic32_sdhci_pdata *sdhci_pdata = sdhci_priv(host); > + > + return clk_get_rate(sdhci_pdata->base_clk); > +} > + > +static void pic32_sdhci_set_bus_width(struct sdhci_host *host, int width) > +{ > + u8 ctrl; > + > + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); > + if (width == MMC_BUS_WIDTH_8) { > + ctrl &= ~SDHCI_CTRL_4BITBUS; > + if (host->version >= SDHCI_SPEC_300) > + ctrl |= SDHCI_CTRL_8BITBUS; > + } else { > + if (host->version >= SDHCI_SPEC_300) > + ctrl &= ~SDHCI_CTRL_8BITBUS; > + if (width == MMC_BUS_WIDTH_4) > + ctrl |= SDHCI_CTRL_4BITBUS; > + else > + ctrl &= ~SDHCI_CTRL_4BITBUS; > + } > + /* > +* SDHCI will not work if JTAG is not Connected.As a workaround fix, > +* set Card Detect Signal Selection bit in SDHCI Host Control > +* register and clear Card Detect Test Level bit in SDHCI Host > +* Control register. > +*/ Isn't this a clearer explanation, if I understood? "Without setting CD select and test bits now, SDHCI only works with JTAG connected." > + ctrl &= ~SDHCI_CTRL_CDTLVL; > + ctrl
[PATCH v2 12/14] mmc: sdhci-pic32: Add PIC32 SDHCI host controller driver
From: Andrei Pistirica This driver supports the SDHCI host controller found on a PIC32. Signed-off-by: Andrei Pistirica Signed-off-by: Joshua Henderson Cc: Ralf Baechle --- drivers/mmc/host/Kconfig | 11 ++ drivers/mmc/host/Makefile |1 + drivers/mmc/host/sdhci-pic32.c | 291 3 files changed, 303 insertions(+) create mode 100644 drivers/mmc/host/sdhci-pic32.c diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index 1dee533..1a3a42b 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -785,3 +785,14 @@ config MMC_MTK If you have a machine with a integrated SD/MMC card reader, say Y or M here. This is needed if support for any SD/SDIO/MMC devices is required. If unsure, say N. + +config MMC_SDHCI_MICROCHIP_PIC32 +tristate "Microchip PIC32MZDA SDHCI support" +depends on MMC_SDHCI && PIC32MZDA +help + This selects the Secure Digital Host Controller Interface (SDHCI) + for PIC32MZDA platform. + + If you have a controller with this interface, say Y or M here. + + If unsure, say N. diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile index 3595f83..af918d2 100644 --- a/drivers/mmc/host/Makefile +++ b/drivers/mmc/host/Makefile @@ -75,6 +75,7 @@ obj-$(CONFIG_MMC_SDHCI_BCM2835) += sdhci-bcm2835.o obj-$(CONFIG_MMC_SDHCI_IPROC) += sdhci-iproc.o obj-$(CONFIG_MMC_SDHCI_MSM)+= sdhci-msm.o obj-$(CONFIG_MMC_SDHCI_ST) += sdhci-st.o +obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)+= sdhci-pic32.o ifeq ($(CONFIG_CB710_DEBUG),y) CFLAGS-cb710-mmc+= -DDEBUG diff --git a/drivers/mmc/host/sdhci-pic32.c b/drivers/mmc/host/sdhci-pic32.c new file mode 100644 index 000..b7d7da2 --- /dev/null +++ b/drivers/mmc/host/sdhci-pic32.c @@ -0,0 +1,291 @@ +/* + * Support of SDHCI platform devices for Microchip PIC32. + * + * Copyright (C) 2015 Microchip + * Andrei Pistirica, Paul Thacker + * + * Inspired by sdhci-pltfm.c + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "sdhci.h" +#include + +#define SDH_SHARED_BUS_CTRL0x00E0 +#define SDH_SHARED_BUS_NR_CLK_PINS_MASK0x7 +#define SDH_SHARED_BUS_NR_IRQ_PINS_MASK0x30 +#define SDH_SHARED_BUS_CLK_PINS0x10 +#define SDH_SHARED_BUS_IRQ_PINS0x14 +#define SDH_CAPS_SDH_SLOT_TYPE_MASK0xC000 +#define SDH_SLOT_TYPE_REMOVABLE0x0 +#define SDH_SLOT_TYPE_EMBEDDED 0x1 +#define SDH_SLOT_TYPE_SHARED_BUS 0x2 +#define SDHCI_CTRL_CDSSEL 0x80 +#define SDHCI_CTRL_CDTLVL 0x40 + +#define ADMA_FIFO_RD_THSHLD512 +#define ADMA_FIFO_WR_THSHLD512 + +#define DEV_NAME "pic32-sdhci" + +struct pic32_sdhci_pdata { + struct platform_device *pdev; + struct clk *sys_clk; + struct clk *base_clk; +}; + +static unsigned int pic32_sdhci_get_max_clock(struct sdhci_host *host) +{ + struct pic32_sdhci_pdata *sdhci_pdata = sdhci_priv(host); + + return clk_get_rate(sdhci_pdata->base_clk); +} + +static void pic32_sdhci_set_bus_width(struct sdhci_host *host, int width) +{ + u8 ctrl; + + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); + if (width == MMC_BUS_WIDTH_8) { + ctrl &= ~SDHCI_CTRL_4BITBUS; + if (host->version >= SDHCI_SPEC_300) + ctrl |= SDHCI_CTRL_8BITBUS; + } else { + if (host->version >= SDHCI_SPEC_300) + ctrl &= ~SDHCI_CTRL_8BITBUS; + if (width == MMC_BUS_WIDTH_4) + ctrl |= SDHCI_CTRL_4BITBUS; + else + ctrl &= ~SDHCI_CTRL_4BITBUS; + } + /* +* SDHCI will not work if JTAG is not Connected.As a workaround fix, +* set Card Detect Signal Selection bit in SDHCI Host Control +* register and clear Card Detect Test Level bit in SDHCI Host +* Control register. +*/ + ctrl &= ~SDHCI_CTRL_CDTLVL; + ctrl |= SDHCI_CTRL_CDSSEL; + sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); +} + +static unsigned int pic32_sdhci_get_ro(struct sdhci_host *host) +{ + /* +* The SDHCI_WRITE_PROTECT bit is unstable on current hardware so we +* can't depend on its value in any way. +*/ + return 0; +} + +static const struct sdhci_ops pic32_sdhci_ops = { + .get_max_clock = pic32_sdhci_get_max_clock, + .set_clock = sdhci_set_clock, + .set_bus_width = pic32_sdhci_set_bus_width, + .reset = sdhci_reset, + .set_uhs_signaling =
Re: [PATCH v2 12/14] mmc: sdhci-pic32: Add PIC32 SDHCI host controller driver
Hi... looks good, just some small general comments. On 15 December 2015 at 06:42, Joshua Hendersonwrote: > From: Andrei Pistirica > > This driver supports the SDHCI host controller found on a PIC32. > > Signed-off-by: Andrei Pistirica > Signed-off-by: Joshua Henderson > Cc: Ralf Baechle > --- > drivers/mmc/host/Kconfig | 11 ++ > drivers/mmc/host/Makefile |1 + > drivers/mmc/host/sdhci-pic32.c | 291 > > 3 files changed, 303 insertions(+) > create mode 100644 drivers/mmc/host/sdhci-pic32.c > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index 1dee533..1a3a42b 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -785,3 +785,14 @@ config MMC_MTK > If you have a machine with a integrated SD/MMC card reader, say Y > or M here. > This is needed if support for any SD/SDIO/MMC devices is required. > If unsure, say N. > + > +config MMC_SDHCI_MICROCHIP_PIC32 > +tristate "Microchip PIC32MZDA SDHCI support" > +depends on MMC_SDHCI && PIC32MZDA > +help > + This selects the Secure Digital Host Controller Interface (SDHCI) > + for PIC32MZDA platform. > + > + If you have a controller with this interface, say Y or M here. > + > + If unsure, say N. > diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile > index 3595f83..af918d2 100644 > --- a/drivers/mmc/host/Makefile > +++ b/drivers/mmc/host/Makefile > @@ -75,6 +75,7 @@ obj-$(CONFIG_MMC_SDHCI_BCM2835) += > sdhci-bcm2835.o > obj-$(CONFIG_MMC_SDHCI_IPROC) += sdhci-iproc.o > obj-$(CONFIG_MMC_SDHCI_MSM)+= sdhci-msm.o > obj-$(CONFIG_MMC_SDHCI_ST) += sdhci-st.o > +obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)+= sdhci-pic32.o > > ifeq ($(CONFIG_CB710_DEBUG),y) > CFLAGS-cb710-mmc+= -DDEBUG > diff --git a/drivers/mmc/host/sdhci-pic32.c b/drivers/mmc/host/sdhci-pic32.c > new file mode 100644 > index 000..b7d7da2 > --- /dev/null > +++ b/drivers/mmc/host/sdhci-pic32.c > @@ -0,0 +1,291 @@ > +/* > + * Support of SDHCI platform devices for Microchip PIC32. > + * > + * Copyright (C) 2015 Microchip > + * Andrei Pistirica, Paul Thacker > + * > + * Inspired by sdhci-pltfm.c > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "sdhci.h" > +#include > + > +#define SDH_SHARED_BUS_CTRL0x00E0 > +#define SDH_SHARED_BUS_NR_CLK_PINS_MASK0x7 > +#define SDH_SHARED_BUS_NR_IRQ_PINS_MASK0x30 > +#define SDH_SHARED_BUS_CLK_PINS0x10 > +#define SDH_SHARED_BUS_IRQ_PINS0x14 > +#define SDH_CAPS_SDH_SLOT_TYPE_MASK0xC000 > +#define SDH_SLOT_TYPE_REMOVABLE0x0 > +#define SDH_SLOT_TYPE_EMBEDDED 0x1 > +#define SDH_SLOT_TYPE_SHARED_BUS 0x2 > +#define SDHCI_CTRL_CDSSEL 0x80 > +#define SDHCI_CTRL_CDTLVL 0x40 > + > +#define ADMA_FIFO_RD_THSHLD512 > +#define ADMA_FIFO_WR_THSHLD512 > + > +#define DEV_NAME "pic32-sdhci" Is there any point defining this when it only has one use in the driver? > +struct pic32_sdhci_pdata { > + struct platform_device *pdev; > + struct clk *sys_clk; > + struct clk *base_clk; > +}; > + > +static unsigned int pic32_sdhci_get_max_clock(struct sdhci_host *host) > +{ > + struct pic32_sdhci_pdata *sdhci_pdata = sdhci_priv(host); > + > + return clk_get_rate(sdhci_pdata->base_clk); > +} > + > +static void pic32_sdhci_set_bus_width(struct sdhci_host *host, int width) > +{ > + u8 ctrl; > + > + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); > + if (width == MMC_BUS_WIDTH_8) { > + ctrl &= ~SDHCI_CTRL_4BITBUS; > + if (host->version >= SDHCI_SPEC_300) > + ctrl |= SDHCI_CTRL_8BITBUS; > + } else { > + if (host->version >= SDHCI_SPEC_300) > + ctrl &= ~SDHCI_CTRL_8BITBUS; > + if (width == MMC_BUS_WIDTH_4) > + ctrl |= SDHCI_CTRL_4BITBUS; > + else > + ctrl &= ~SDHCI_CTRL_4BITBUS; > + } > + /* > +* SDHCI will not work if JTAG is not Connected.As a workaround fix, > +* set Card Detect Signal Selection bit in SDHCI Host Control > +* register and clear Card Detect Test Level bit in SDHCI Host > +* Control register. > +*/ Isn't this a clearer explanation, if I
[PATCH v2 12/14] mmc: sdhci-pic32: Add PIC32 SDHCI host controller driver
From: Andrei PistiricaThis driver supports the SDHCI host controller found on a PIC32. Signed-off-by: Andrei Pistirica Signed-off-by: Joshua Henderson Cc: Ralf Baechle --- drivers/mmc/host/Kconfig | 11 ++ drivers/mmc/host/Makefile |1 + drivers/mmc/host/sdhci-pic32.c | 291 3 files changed, 303 insertions(+) create mode 100644 drivers/mmc/host/sdhci-pic32.c diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index 1dee533..1a3a42b 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -785,3 +785,14 @@ config MMC_MTK If you have a machine with a integrated SD/MMC card reader, say Y or M here. This is needed if support for any SD/SDIO/MMC devices is required. If unsure, say N. + +config MMC_SDHCI_MICROCHIP_PIC32 +tristate "Microchip PIC32MZDA SDHCI support" +depends on MMC_SDHCI && PIC32MZDA +help + This selects the Secure Digital Host Controller Interface (SDHCI) + for PIC32MZDA platform. + + If you have a controller with this interface, say Y or M here. + + If unsure, say N. diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile index 3595f83..af918d2 100644 --- a/drivers/mmc/host/Makefile +++ b/drivers/mmc/host/Makefile @@ -75,6 +75,7 @@ obj-$(CONFIG_MMC_SDHCI_BCM2835) += sdhci-bcm2835.o obj-$(CONFIG_MMC_SDHCI_IPROC) += sdhci-iproc.o obj-$(CONFIG_MMC_SDHCI_MSM)+= sdhci-msm.o obj-$(CONFIG_MMC_SDHCI_ST) += sdhci-st.o +obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32)+= sdhci-pic32.o ifeq ($(CONFIG_CB710_DEBUG),y) CFLAGS-cb710-mmc+= -DDEBUG diff --git a/drivers/mmc/host/sdhci-pic32.c b/drivers/mmc/host/sdhci-pic32.c new file mode 100644 index 000..b7d7da2 --- /dev/null +++ b/drivers/mmc/host/sdhci-pic32.c @@ -0,0 +1,291 @@ +/* + * Support of SDHCI platform devices for Microchip PIC32. + * + * Copyright (C) 2015 Microchip + * Andrei Pistirica, Paul Thacker + * + * Inspired by sdhci-pltfm.c + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "sdhci.h" +#include + +#define SDH_SHARED_BUS_CTRL0x00E0 +#define SDH_SHARED_BUS_NR_CLK_PINS_MASK0x7 +#define SDH_SHARED_BUS_NR_IRQ_PINS_MASK0x30 +#define SDH_SHARED_BUS_CLK_PINS0x10 +#define SDH_SHARED_BUS_IRQ_PINS0x14 +#define SDH_CAPS_SDH_SLOT_TYPE_MASK0xC000 +#define SDH_SLOT_TYPE_REMOVABLE0x0 +#define SDH_SLOT_TYPE_EMBEDDED 0x1 +#define SDH_SLOT_TYPE_SHARED_BUS 0x2 +#define SDHCI_CTRL_CDSSEL 0x80 +#define SDHCI_CTRL_CDTLVL 0x40 + +#define ADMA_FIFO_RD_THSHLD512 +#define ADMA_FIFO_WR_THSHLD512 + +#define DEV_NAME "pic32-sdhci" + +struct pic32_sdhci_pdata { + struct platform_device *pdev; + struct clk *sys_clk; + struct clk *base_clk; +}; + +static unsigned int pic32_sdhci_get_max_clock(struct sdhci_host *host) +{ + struct pic32_sdhci_pdata *sdhci_pdata = sdhci_priv(host); + + return clk_get_rate(sdhci_pdata->base_clk); +} + +static void pic32_sdhci_set_bus_width(struct sdhci_host *host, int width) +{ + u8 ctrl; + + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); + if (width == MMC_BUS_WIDTH_8) { + ctrl &= ~SDHCI_CTRL_4BITBUS; + if (host->version >= SDHCI_SPEC_300) + ctrl |= SDHCI_CTRL_8BITBUS; + } else { + if (host->version >= SDHCI_SPEC_300) + ctrl &= ~SDHCI_CTRL_8BITBUS; + if (width == MMC_BUS_WIDTH_4) + ctrl |= SDHCI_CTRL_4BITBUS; + else + ctrl &= ~SDHCI_CTRL_4BITBUS; + } + /* +* SDHCI will not work if JTAG is not Connected.As a workaround fix, +* set Card Detect Signal Selection bit in SDHCI Host Control +* register and clear Card Detect Test Level bit in SDHCI Host +* Control register. +*/ + ctrl &= ~SDHCI_CTRL_CDTLVL; + ctrl |= SDHCI_CTRL_CDSSEL; + sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); +} + +static unsigned int pic32_sdhci_get_ro(struct sdhci_host *host) +{ + /* +* The SDHCI_WRITE_PROTECT bit is unstable on current hardware so we +* can't depend on its value in any way. +*/ + return 0; +} + +static const struct sdhci_ops pic32_sdhci_ops = { + .get_max_clock = pic32_sdhci_get_max_clock, + .set_clock = sdhci_set_clock, +