* [PATCH 0/3] mmc: Adding support for Microchip Sparx5 SoC @ 2020-05-13 13:31 Lars Povlsen 2020-05-13 13:31 ` [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings Lars Povlsen ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Lars Povlsen @ 2020-05-13 13:31 UTC (permalink / raw) To: Ulf Hansson, Adrian Hunter, SoC Team Cc: Lars Povlsen, Microchip Linux Driver Support, linux-mmc, devicetree, linux-arm-kernel, linux-kernel This is an add-on series to the main SoC Sparx5 series (Message-ID: <20200513125532.24585-1-lars.povlsen@microchip.com>). It adds eMMC support for Sparx5, by adding a driver for the SoC SDHCI controller, DT configuration and DT binding documentation. Lars Povlsen (3): dt-bindings: mmc: Add Sparx5 SDHCI controller bindings sdhci: sparx5: Add Sparx5 SoC eMMC driver arm64: dts: sparx5: Add Sparx5 eMMC support .../mmc/microchip,dw-sparx5-sdhci.yaml | 57 +++ arch/arm64/boot/dts/microchip/sparx5.dtsi | 24 ++ .../boot/dts/microchip/sparx5_pcb125.dts | 23 ++ .../boot/dts/microchip/sparx5_pcb134_emmc.dts | 23 ++ .../boot/dts/microchip/sparx5_pcb135_emmc.dts | 23 ++ drivers/mmc/host/Kconfig | 13 + drivers/mmc/host/Makefile | 1 + drivers/mmc/host/sdhci-of-sparx5.c | 348 ++++++++++++++++++ 8 files changed, 512 insertions(+) create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c -- 2.26.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings 2020-05-13 13:31 [PATCH 0/3] mmc: Adding support for Microchip Sparx5 SoC Lars Povlsen @ 2020-05-13 13:31 ` Lars Povlsen 2020-05-14 13:03 ` Rob Herring 2020-05-13 13:31 ` [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver Lars Povlsen 2020-05-13 13:31 ` [PATCH 3/3] arm64: dts: sparx5: Add Sparx5 eMMC support Lars Povlsen 2 siblings, 1 reply; 12+ messages in thread From: Lars Povlsen @ 2020-05-13 13:31 UTC (permalink / raw) To: Ulf Hansson, Adrian Hunter, SoC Team, Rob Herring Cc: Lars Povlsen, Microchip Linux Driver Support, linux-mmc, devicetree, linux-arm-kernel, linux-kernel, Alexandre Belloni The Sparx5 SDHCI controller is based on the Designware controller IP. Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> --- .../mmc/microchip,dw-sparx5-sdhci.yaml | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml diff --git a/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml new file mode 100644 index 0000000000000..a9901c4bc25d3 --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mmc/microchip,dw-sparx5-sdhci.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip Sparx5 Mobile Storage Host Controller Binding + +allOf: + - $ref: "mmc-controller.yaml" + +maintainers: + - Lars Povlsen <lars.povlsen@microchip.com> + +# Everything else is described in the common file +properties: + compatible: + const: microchip,dw-sparx5-sdhci + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + description: + Handle to "core" clock for the sdhci controller. + + clock-names: + items: + - const: core + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/microchip,sparx5.h> + sdhci0: mmc@600800000 { + compatible = "microchip,dw-sparx5-sdhci"; + reg = <0x00800000 0x1000>; + pinctrl-0 = <&emmc_pins>; + pinctrl-names = "default"; + clocks = <&clks CLK_ID_AUX1>; + clock-names = "core"; + assigned-clocks = <&clks CLK_ID_AUX1>; + assigned-clock-rates = <800000000>; + interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>; + bus-width = <8>; + }; -- 2.26.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings 2020-05-13 13:31 ` [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings Lars Povlsen @ 2020-05-14 13:03 ` Rob Herring 2020-05-15 15:50 ` Lars Povlsen 0 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2020-05-14 13:03 UTC (permalink / raw) To: Lars Povlsen Cc: Alexandre Belloni, linux-arm-kernel, SoC Team, linux-mmc, Ulf Hansson, linux-kernel, Microchip Linux Driver Support, devicetree, Adrian Hunter, Rob Herring On Wed, 13 May 2020 15:31:20 +0200, Lars Povlsen wrote: > The Sparx5 SDHCI controller is based on the Designware controller IP. > > Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> > --- > .../mmc/microchip,dw-sparx5-sdhci.yaml | 57 +++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml > My bot found errors running 'make dt_binding_check' on your patch: Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dts:20:18: fatal error: dt-bindings/clock/microchip,sparx5.h: No such file or directory #include <dt-bindings/clock/microchip,sparx5.h> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml' failed make[1]: *** [Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs.... Makefile:1300: recipe for target 'dt_binding_check' failed make: *** [dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1289290 If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings 2020-05-14 13:03 ` Rob Herring @ 2020-05-15 15:50 ` Lars Povlsen 0 siblings, 0 replies; 12+ messages in thread From: Lars Povlsen @ 2020-05-15 15:50 UTC (permalink / raw) To: Rob Herring Cc: Lars Povlsen, Alexandre Belloni, linux-arm-kernel, SoC Team, linux-mmc, Ulf Hansson, linux-kernel, Microchip Linux Driver Support, devicetree, Adrian Hunter, Rob Herring Rob Herring writes: > On Wed, 13 May 2020 15:31:20 +0200, Lars Povlsen wrote: >> The Sparx5 SDHCI controller is based on the Designware controller IP. >> >> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> >> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> >> --- >> .../mmc/microchip,dw-sparx5-sdhci.yaml | 57 +++++++++++++++++++ >> 1 file changed, 57 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml >> > > > My bot found errors running 'make dt_binding_check' on your patch: > > Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dts:20:18: fatal error: dt-bindings/clock/microchip,sparx5.h: No such file or directory > #include <dt-bindings/clock/microchip,sparx5.h> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > compilation terminated. > scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml' failed > make[1]: *** [Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml] Error 1 > make[1]: *** Waiting for unfinished jobs.... > Makefile:1300: recipe for target 'dt_binding_check' failed > make: *** [dt_binding_check] Error 2 > > See https://patchwork.ozlabs.org/patch/1289290 > Rob, The header file is added with the "parent" SoC series for Sparx5, which was submitted separately to the SoC list. Should I rewrite the example to avoid using the (normal) header file, or can you add the header file? I have verified the YAML pass dt_binding_check with the header file. > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure dt-schema is up to date: > > pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade > > Please check and re-submit. -- Lars Povlsen, Microchip ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver 2020-05-13 13:31 [PATCH 0/3] mmc: Adding support for Microchip Sparx5 SoC Lars Povlsen 2020-05-13 13:31 ` [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings Lars Povlsen @ 2020-05-13 13:31 ` Lars Povlsen 2020-05-17 12:58 ` Adrian Hunter 2020-05-13 13:31 ` [PATCH 3/3] arm64: dts: sparx5: Add Sparx5 eMMC support Lars Povlsen 2 siblings, 1 reply; 12+ messages in thread From: Lars Povlsen @ 2020-05-13 13:31 UTC (permalink / raw) To: Ulf Hansson, Adrian Hunter, SoC Team Cc: Lars Povlsen, Microchip Linux Driver Support, linux-mmc, devicetree, linux-arm-kernel, linux-kernel, Alexandre Belloni This adds the eMMC driver for the Sparx5 SoC. It is based upon the designware IP, but requires some extra initialization and quirks. Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> --- drivers/mmc/host/Kconfig | 13 ++ drivers/mmc/host/Makefile | 1 + drivers/mmc/host/sdhci-of-sparx5.c | 348 +++++++++++++++++++++++++++++ 3 files changed, 362 insertions(+) create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index 462b5352fea75..1e8396d09df75 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -213,6 +213,19 @@ config MMC_SDHCI_OF_DWCMSHC If you have a controller with this interface, say Y or M here. If unsure, say N. +config MMC_SDHCI_OF_SPARX5 + tristate "SDHCI OF support for the MCHP Sparx5 SoC" + depends on MMC_SDHCI_PLTFM + depends on ARCH_SPARX5 + select MMC_SDHCI_IO_ACCESSORS + help + This selects the Secure Digital Host Controller Interface (SDHCI) + found in the MCHP Sparx5 SoC. + + If you have a Sparx5 SoC with this interface, say Y or M here. + + If unsure, say N. + config MMC_SDHCI_CADENCE tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller" depends on MMC_SDHCI_PLTFM diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile index b929ef9412083..9f09b7ffaaa16 100644 --- a/drivers/mmc/host/Makefile +++ b/drivers/mmc/host/Makefile @@ -89,6 +89,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ARASAN) += sdhci-of-arasan.o obj-$(CONFIG_MMC_SDHCI_OF_ASPEED) += sdhci-of-aspeed.o obj-$(CONFIG_MMC_SDHCI_OF_AT91) += sdhci-of-at91.o obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o +obj-$(CONFIG_MMC_SDHCI_OF_SPARX5) += sdhci-of-sparx5.o obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC) += sdhci-of-dwcmshc.o obj-$(CONFIG_MMC_SDHCI_BCM_KONA) += sdhci-bcm-kona.o diff --git a/drivers/mmc/host/sdhci-of-sparx5.c b/drivers/mmc/host/sdhci-of-sparx5.c new file mode 100644 index 0000000000000..8253bf80e175a --- /dev/null +++ b/drivers/mmc/host/sdhci-of-sparx5.c @@ -0,0 +1,348 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * drivers/mmc/host/sdhci-of-sparx5.c + * + * MCHP Sparx5 SoC Secure Digital Host Controller Interface. + * + * Copyright (c) 2019 Microchip Inc. + * + * Author: Lars Povlsen <lars.povlsen@microchip.com> + */ + +//#define DEBUG +//#define TRACE_REGISTER + +#include <linux/sizes.h> +#include <linux/delay.h> +#include <linux/module.h> +#include <linux/regmap.h> +#include <linux/of_device.h> +#include <linux/mfd/syscon.h> +#include <linux/dma-mapping.h> + +#include "sdhci-pltfm.h" + +#define CPU_REGS_GENERAL_CTRL (0x22 * 4) +#define MSHC_DLY_CC_MASK GENMASK(16, 13) +#define MSHC_DLY_CC_SHIFT 13 +#define MSHC_DLY_CC_MAX 15 + +#define CPU_REGS_PROC_CTRL (0x2C * 4) +#define ACP_CACHE_FORCE_ENA BIT(4) +#define ACP_AWCACHE BIT(3) +#define ACP_ARCACHE BIT(2) +#define ACP_CACHE_MASK (ACP_CACHE_FORCE_ENA|ACP_AWCACHE|ACP_ARCACHE) + +#define MSHC2_VERSION 0x500 /* Off 0x140, reg 0x0 */ +#define MSHC2_TYPE 0x504 /* Off 0x140, reg 0x1 */ +#define MSHC2_EMMC_CTRL 0x52c /* Off 0x140, reg 0xB */ +#define MSHC2_EMMC_CTRL_EMMC_RST_N BIT(2) +#define MSHC2_EMMC_CTRL_IS_EMMC BIT(0) + +struct sdhci_sparx5_data { + struct sdhci_host *host; + struct regmap *cpu_ctrl; + int delay_clock; + struct device_attribute dev_delay_clock; +}; + +#define BOUNDARY_OK(addr, len) \ + ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1))) + +#if defined(TRACE_REGISTER) +static void sdhci_sparx5_writel(struct sdhci_host *host, u32 val, int reg) +{ + pr_debug("$$$ writel(0x%08x, 0x%02x)\n", val, reg); + writel(val, host->ioaddr + reg); +} + +static void sdhci_sparx5_writew(struct sdhci_host *host, u16 val, int reg) +{ + pr_debug("$$$ writew(0x%04x, 0x%02x)\n", val, reg); + writew(val, host->ioaddr + reg); +} + +static void sdhci_sparx5_writeb(struct sdhci_host *host, u8 val, int reg) +{ + pr_debug("$$$ writeb(0x%02x, 0x%02x)\n", val, reg); + writeb(val, host->ioaddr + reg); +} +#endif + +/* + * If DMA addr spans 128MB boundary, we split the DMA transfer into two + * so that each DMA transfer doesn't exceed the boundary. + */ +static void sdhci_sparx5_adma_write_desc(struct sdhci_host *host, void **desc, + dma_addr_t addr, int len, + unsigned int cmd) +{ + int tmplen, offset; + + pr_debug("write_desc: cmd %02x: len %d, offset 0x%0llx\n", + cmd, len, addr); + + if (likely(!len || BOUNDARY_OK(addr, len))) { + sdhci_adma_write_desc(host, desc, addr, len, cmd); + return; + } + + pr_debug("write_desc: splitting dma len %d, offset 0x%0llx\n", + len, addr); + + offset = addr & (SZ_128M - 1); + tmplen = SZ_128M - offset; + sdhci_adma_write_desc(host, desc, addr, tmplen, cmd); + + addr += tmplen; + len -= tmplen; + sdhci_adma_write_desc(host, desc, addr, len, cmd); +} + +static void sparx5_set_cacheable(struct sdhci_host *host, u32 value) +{ + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host); + + pr_debug("%s: Set Cacheable = 0x%x\n", mmc_hostname(host->mmc), value); + + /* Update ACP caching attributes in HW */ + regmap_update_bits(sdhci_sparx5->cpu_ctrl, + CPU_REGS_PROC_CTRL, ACP_CACHE_MASK, value); +} + +static void sparx5_set_delay(struct sdhci_host *host, u8 value) +{ + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host); + + pr_debug("%s: Set DLY_CC = %u\n", mmc_hostname(host->mmc), value); + + /* Update DLY_CC in HW */ + regmap_update_bits(sdhci_sparx5->cpu_ctrl, + CPU_REGS_GENERAL_CTRL, + MSHC_DLY_CC_MASK, + (value << MSHC_DLY_CC_SHIFT)); +} + +static void sdhci_sparx5_set_emmc(struct sdhci_host *host) +{ + if (!mmc_card_is_removable(host->mmc)) { + u8 value; + + value = sdhci_readb(host, MSHC2_EMMC_CTRL); + if (!(value & MSHC2_EMMC_CTRL_IS_EMMC)) { + pr_debug("Get EMMC_CTRL: 0x%08x\n", value); + value |= MSHC2_EMMC_CTRL_IS_EMMC; + pr_debug("Set EMMC_CTRL: 0x%08x\n", value); + sdhci_writeb(host, value, MSHC2_EMMC_CTRL); + } + } +} + +static void sdhci_sparx5_reset_emmc(struct sdhci_host *host) +{ + u8 value; + + pr_debug("Toggle EMMC_CTRL.EMMC_RST_N\n"); + value = sdhci_readb(host, MSHC2_EMMC_CTRL) & + ~MSHC2_EMMC_CTRL_EMMC_RST_N; + sdhci_writeb(host, value, MSHC2_EMMC_CTRL); + /* For eMMC, minimum is 1us but give it 10us for good measure */ + udelay(10); + sdhci_writeb(host, value | MSHC2_EMMC_CTRL_EMMC_RST_N, + MSHC2_EMMC_CTRL); + /* For eMMC, minimum is 200us but give it 300us for good measure */ + udelay(300); +} + +static void sdhci_sparx5_reset(struct sdhci_host *host, u8 mask) +{ + pr_debug("*** RESET: mask %d\n", mask); + + sdhci_reset(host, mask); + + /* Be sure CARD_IS_EMMC stays set */ + sdhci_sparx5_set_emmc(host); +} + +static const struct sdhci_ops sdhci_sparx5_ops = { +#if defined(TRACE_REGISTER) + .write_l = sdhci_sparx5_writel, + .write_w = sdhci_sparx5_writew, + .write_b = sdhci_sparx5_writeb, +#endif + .set_clock = sdhci_set_clock, + .set_bus_width = sdhci_set_bus_width, + .set_uhs_signaling = sdhci_set_uhs_signaling, + .get_max_clock = sdhci_pltfm_clk_get_max_clock, + .reset = sdhci_sparx5_reset, + .adma_write_desc = sdhci_sparx5_adma_write_desc, +}; + +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = { + .quirks = 0, + .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */ + SDHCI_QUIRK2_NO_1_8_V, /* No sdr104, ddr50, etc */ + .ops = &sdhci_sparx5_ops, +}; + +static ssize_t sparx5_delay_clock_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct sdhci_sparx5_data *sdhci_sparx5; + + sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data, + dev_delay_clock); + return scnprintf(buf, PAGE_SIZE, "%d\n", sdhci_sparx5->delay_clock); +} + +static ssize_t sparx5_delay_clock_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + unsigned int delay_clock; + struct sdhci_sparx5_data *sdhci_sparx5; + + sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data, + dev_delay_clock); + + if (kstrtoint(buf, 10, &delay_clock) || + delay_clock > MSHC_DLY_CC_MAX) { + dev_err(dev, "sdhci-of-sparx5: wrong parameter format.\n"); + return -EINVAL; + } + + sdhci_sparx5->delay_clock = delay_clock; + sparx5_set_delay(sdhci_sparx5->host, sdhci_sparx5->delay_clock); + + return strlen(buf); +} + +int sdhci_sparx5_probe(struct platform_device *pdev) +{ + int ret; + const char *syscon = "microchip,sparx5-cpu-syscon"; + struct sdhci_host *host; + struct sdhci_pltfm_host *pltfm_host; + struct sdhci_sparx5_data *sdhci_sparx5; + struct device_node *np = pdev->dev.of_node; + u32 value; + u32 extra; + + host = sdhci_pltfm_init(pdev, &sdhci_sparx5_pdata, + sizeof(*sdhci_sparx5)); + + if (IS_ERR(host)) + return PTR_ERR(host); + + /* + * extra adma table cnt for cross 128M boundary handling. + */ + extra = DIV_ROUND_UP_ULL(dma_get_required_mask(&pdev->dev), SZ_128M); + if (extra > SDHCI_MAX_SEGS) + extra = SDHCI_MAX_SEGS; + host->adma_table_cnt += extra; + + pltfm_host = sdhci_priv(host); + sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host); + sdhci_sparx5->host = host; + + pltfm_host->clk = devm_clk_get(&pdev->dev, "core"); + if (IS_ERR(pltfm_host->clk)) { + ret = PTR_ERR(pltfm_host->clk); + dev_err(&pdev->dev, "failed to get core clk: %d\n", ret); + goto free_pltfm; + } + ret = clk_prepare_enable(pltfm_host->clk); + if (ret) + goto free_pltfm; + + if (!of_property_read_u32(np, "microchip,clock-delay", &value) && + value <= MSHC_DLY_CC_MAX) + sdhci_sparx5->delay_clock = value; + else + sdhci_sparx5->delay_clock = -1; /* Autotune */ + + /* Sysfs delay_clock interface */ + sdhci_sparx5->dev_delay_clock.show = sparx5_delay_clock_show; + sdhci_sparx5->dev_delay_clock.store = sparx5_delay_clock_store; + sysfs_attr_init(&sdhci_sparx5->dev_delay_clock.attr); + sdhci_sparx5->dev_delay_clock.attr.name = "delay_clock"; + sdhci_sparx5->dev_delay_clock.attr.mode = 0644; + ret = device_create_file(&pdev->dev, &sdhci_sparx5->dev_delay_clock); + if (ret) + dev_err(&pdev->dev, "failure creating '%s' device file", + sdhci_sparx5->dev_delay_clock.attr.name); + + sdhci_get_of_property(pdev); + + ret = mmc_of_parse(host->mmc); + if (ret) + goto err_clk; + + sdhci_sparx5->cpu_ctrl = syscon_regmap_lookup_by_compatible(syscon); + if (IS_ERR(sdhci_sparx5->cpu_ctrl)) { + dev_err(&pdev->dev, "No CPU syscon regmap !\n"); + ret = PTR_ERR(sdhci_sparx5->cpu_ctrl); + goto err_clk; + } + + if (sdhci_sparx5->delay_clock >= 0) + sparx5_set_delay(host, sdhci_sparx5->delay_clock); + + if (!mmc_card_is_removable(host->mmc)) { + /* Do a HW reset of eMMC card */ + sdhci_sparx5_reset_emmc(host); + /* Update EMMC_CTRL */ + sdhci_sparx5_set_emmc(host); + /* If eMMC, disable SD and SDIO */ + host->mmc->caps2 |= (MMC_CAP2_NO_SDIO|MMC_CAP2_NO_SD); + } + + /* Enable v4 mode */ + //sdhci_enable_v4_mode(host); + + ret = sdhci_add_host(host); + if (ret) + dev_err(&pdev->dev, "sdhci_add_host() failed (%d)\n", ret); + + /* Set AXI bus master to use un-cached access (for DMA) */ + if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA) && + IS_ENABLED(CONFIG_DMA_DECLARE_COHERENT)) + sparx5_set_cacheable(host, ACP_CACHE_FORCE_ENA); + + pr_debug("SDHC version: 0x%08x\n", sdhci_readl(host, MSHC2_VERSION)); + pr_debug("SDHC type: 0x%08x\n", sdhci_readl(host, MSHC2_TYPE)); + + return ret; + +err_clk: + clk_disable_unprepare(pltfm_host->clk); +free_pltfm: + sdhci_pltfm_free(pdev); + return ret; +} + +static const struct of_device_id sdhci_sparx5_of_match[] = { + { .compatible = "microchip,dw-sparx5-sdhci" }, + { } +}; +MODULE_DEVICE_TABLE(of, sdhci_sparx5_of_match); + +static struct platform_driver sdhci_sparx5_driver = { + .driver = { + .name = "sdhci-sparx5", + .of_match_table = sdhci_sparx5_of_match, + .pm = &sdhci_pltfm_pmops, + }, + .probe = sdhci_sparx5_probe, + .remove = sdhci_pltfm_unregister, +}; + +module_platform_driver(sdhci_sparx5_driver); + +MODULE_DESCRIPTION("Sparx5 SDHCI OF driver"); +MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>"); +MODULE_LICENSE("GPL v2"); -- 2.26.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver 2020-05-13 13:31 ` [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver Lars Povlsen @ 2020-05-17 12:58 ` Adrian Hunter 2020-05-18 8:58 ` Lars Povlsen 0 siblings, 1 reply; 12+ messages in thread From: Adrian Hunter @ 2020-05-17 12:58 UTC (permalink / raw) To: Lars Povlsen, Ulf Hansson, SoC Team Cc: Microchip Linux Driver Support, linux-mmc, devicetree, linux-arm-kernel, linux-kernel, Alexandre Belloni On 13/05/20 4:31 pm, Lars Povlsen wrote: > This adds the eMMC driver for the Sparx5 SoC. It is based upon the > designware IP, but requires some extra initialization and quirks. > > Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> > --- > drivers/mmc/host/Kconfig | 13 ++ > drivers/mmc/host/Makefile | 1 + > drivers/mmc/host/sdhci-of-sparx5.c | 348 +++++++++++++++++++++++++++++ > 3 files changed, 362 insertions(+) > create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index 462b5352fea75..1e8396d09df75 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -213,6 +213,19 @@ config MMC_SDHCI_OF_DWCMSHC > If you have a controller with this interface, say Y or M here. > If unsure, say N. > > +config MMC_SDHCI_OF_SPARX5 > + tristate "SDHCI OF support for the MCHP Sparx5 SoC" > + depends on MMC_SDHCI_PLTFM > + depends on ARCH_SPARX5 > + select MMC_SDHCI_IO_ACCESSORS > + help > + This selects the Secure Digital Host Controller Interface (SDHCI) > + found in the MCHP Sparx5 SoC. > + > + If you have a Sparx5 SoC with this interface, say Y or M here. > + > + If unsure, say N. > + > config MMC_SDHCI_CADENCE > tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller" > depends on MMC_SDHCI_PLTFM > diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile > index b929ef9412083..9f09b7ffaaa16 100644 > --- a/drivers/mmc/host/Makefile > +++ b/drivers/mmc/host/Makefile > @@ -89,6 +89,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ARASAN) += sdhci-of-arasan.o > obj-$(CONFIG_MMC_SDHCI_OF_ASPEED) += sdhci-of-aspeed.o > obj-$(CONFIG_MMC_SDHCI_OF_AT91) += sdhci-of-at91.o > obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o > +obj-$(CONFIG_MMC_SDHCI_OF_SPARX5) += sdhci-of-sparx5.o > obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o > obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC) += sdhci-of-dwcmshc.o > obj-$(CONFIG_MMC_SDHCI_BCM_KONA) += sdhci-bcm-kona.o > diff --git a/drivers/mmc/host/sdhci-of-sparx5.c b/drivers/mmc/host/sdhci-of-sparx5.c > new file mode 100644 > index 0000000000000..8253bf80e175a > --- /dev/null > +++ b/drivers/mmc/host/sdhci-of-sparx5.c > @@ -0,0 +1,348 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * drivers/mmc/host/sdhci-of-sparx5.c > + * > + * MCHP Sparx5 SoC Secure Digital Host Controller Interface. > + * > + * Copyright (c) 2019 Microchip Inc. > + * > + * Author: Lars Povlsen <lars.povlsen@microchip.com> > + */ > + > +//#define DEBUG > +//#define TRACE_REGISTER No commented out code please. > + > +#include <linux/sizes.h> > +#include <linux/delay.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/of_device.h> > +#include <linux/mfd/syscon.h> > +#include <linux/dma-mapping.h> > + > +#include "sdhci-pltfm.h" > + > +#define CPU_REGS_GENERAL_CTRL (0x22 * 4) > +#define MSHC_DLY_CC_MASK GENMASK(16, 13) > +#define MSHC_DLY_CC_SHIFT 13 > +#define MSHC_DLY_CC_MAX 15 > + > +#define CPU_REGS_PROC_CTRL (0x2C * 4) > +#define ACP_CACHE_FORCE_ENA BIT(4) > +#define ACP_AWCACHE BIT(3) > +#define ACP_ARCACHE BIT(2) > +#define ACP_CACHE_MASK (ACP_CACHE_FORCE_ENA|ACP_AWCACHE|ACP_ARCACHE) > + > +#define MSHC2_VERSION 0x500 /* Off 0x140, reg 0x0 */ > +#define MSHC2_TYPE 0x504 /* Off 0x140, reg 0x1 */ > +#define MSHC2_EMMC_CTRL 0x52c /* Off 0x140, reg 0xB */ > +#define MSHC2_EMMC_CTRL_EMMC_RST_N BIT(2) > +#define MSHC2_EMMC_CTRL_IS_EMMC BIT(0) > + > +struct sdhci_sparx5_data { > + struct sdhci_host *host; > + struct regmap *cpu_ctrl; > + int delay_clock; > + struct device_attribute dev_delay_clock; > +}; > + > +#define BOUNDARY_OK(addr, len) \ > + ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1))) > + > +#if defined(TRACE_REGISTER) If you want this then add a Kconfig entry for it > +static void sdhci_sparx5_writel(struct sdhci_host *host, u32 val, int reg) > +{ > + pr_debug("$$$ writel(0x%08x, 0x%02x)\n", val, reg); > + writel(val, host->ioaddr + reg); > +} > + > +static void sdhci_sparx5_writew(struct sdhci_host *host, u16 val, int reg) > +{ > + pr_debug("$$$ writew(0x%04x, 0x%02x)\n", val, reg); > + writew(val, host->ioaddr + reg); > +} > + > +static void sdhci_sparx5_writeb(struct sdhci_host *host, u8 val, int reg) > +{ > + pr_debug("$$$ writeb(0x%02x, 0x%02x)\n", val, reg); > + writeb(val, host->ioaddr + reg); > +} > +#endif > + > +/* > + * If DMA addr spans 128MB boundary, we split the DMA transfer into two > + * so that each DMA transfer doesn't exceed the boundary. > + */ > +static void sdhci_sparx5_adma_write_desc(struct sdhci_host *host, void **desc, > + dma_addr_t addr, int len, > + unsigned int cmd) > +{ > + int tmplen, offset; > + > + pr_debug("write_desc: cmd %02x: len %d, offset 0x%0llx\n", > + cmd, len, addr); Please prefix all kernel messages by either the mmc or device e.g. pr_debug("%s: write_desc: cmd %02x: len %d, offset 0x%0llx\n", mmc_hostname(host->mmc), cmd, len, addr); > + > + if (likely(!len || BOUNDARY_OK(addr, len))) { > + sdhci_adma_write_desc(host, desc, addr, len, cmd); > + return; > + } > + > + pr_debug("write_desc: splitting dma len %d, offset 0x%0llx\n", > + len, addr); > + > + offset = addr & (SZ_128M - 1); > + tmplen = SZ_128M - offset; > + sdhci_adma_write_desc(host, desc, addr, tmplen, cmd); > + > + addr += tmplen; > + len -= tmplen; > + sdhci_adma_write_desc(host, desc, addr, len, cmd); > +} > + > +static void sparx5_set_cacheable(struct sdhci_host *host, u32 value) > +{ > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host); > + > + pr_debug("%s: Set Cacheable = 0x%x\n", mmc_hostname(host->mmc), value); > + > + /* Update ACP caching attributes in HW */ > + regmap_update_bits(sdhci_sparx5->cpu_ctrl, > + CPU_REGS_PROC_CTRL, ACP_CACHE_MASK, value); > +} > + > +static void sparx5_set_delay(struct sdhci_host *host, u8 value) > +{ > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host); > + > + pr_debug("%s: Set DLY_CC = %u\n", mmc_hostname(host->mmc), value); > + > + /* Update DLY_CC in HW */ > + regmap_update_bits(sdhci_sparx5->cpu_ctrl, > + CPU_REGS_GENERAL_CTRL, > + MSHC_DLY_CC_MASK, > + (value << MSHC_DLY_CC_SHIFT)); > +} > + > +static void sdhci_sparx5_set_emmc(struct sdhci_host *host) > +{ > + if (!mmc_card_is_removable(host->mmc)) { > + u8 value; > + > + value = sdhci_readb(host, MSHC2_EMMC_CTRL); > + if (!(value & MSHC2_EMMC_CTRL_IS_EMMC)) { > + pr_debug("Get EMMC_CTRL: 0x%08x\n", value); > + value |= MSHC2_EMMC_CTRL_IS_EMMC; > + pr_debug("Set EMMC_CTRL: 0x%08x\n", value); > + sdhci_writeb(host, value, MSHC2_EMMC_CTRL); > + } > + } > +} > + > +static void sdhci_sparx5_reset_emmc(struct sdhci_host *host) > +{ > + u8 value; > + > + pr_debug("Toggle EMMC_CTRL.EMMC_RST_N\n"); > + value = sdhci_readb(host, MSHC2_EMMC_CTRL) & > + ~MSHC2_EMMC_CTRL_EMMC_RST_N; > + sdhci_writeb(host, value, MSHC2_EMMC_CTRL); > + /* For eMMC, minimum is 1us but give it 10us for good measure */ > + udelay(10); > + sdhci_writeb(host, value | MSHC2_EMMC_CTRL_EMMC_RST_N, > + MSHC2_EMMC_CTRL); > + /* For eMMC, minimum is 200us but give it 300us for good measure */ > + udelay(300); usleep_range() is better here > +} > + > +static void sdhci_sparx5_reset(struct sdhci_host *host, u8 mask) > +{ > + pr_debug("*** RESET: mask %d\n", mask); > + > + sdhci_reset(host, mask); > + > + /* Be sure CARD_IS_EMMC stays set */ > + sdhci_sparx5_set_emmc(host); > +} > + > +static const struct sdhci_ops sdhci_sparx5_ops = { > +#if defined(TRACE_REGISTER) > + .write_l = sdhci_sparx5_writel, > + .write_w = sdhci_sparx5_writew, > + .write_b = sdhci_sparx5_writeb, > +#endif > + .set_clock = sdhci_set_clock, > + .set_bus_width = sdhci_set_bus_width, > + .set_uhs_signaling = sdhci_set_uhs_signaling, > + .get_max_clock = sdhci_pltfm_clk_get_max_clock, > + .reset = sdhci_sparx5_reset, > + .adma_write_desc = sdhci_sparx5_adma_write_desc, > +}; > + > +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = { > + .quirks = 0, > + .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */ If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here. > + SDHCI_QUIRK2_NO_1_8_V, /* No sdr104, ddr50, etc */ > + .ops = &sdhci_sparx5_ops, > +}; > + > +static ssize_t sparx5_delay_clock_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct sdhci_sparx5_data *sdhci_sparx5; > + > + sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data, > + dev_delay_clock); > + return scnprintf(buf, PAGE_SIZE, "%d\n", sdhci_sparx5->delay_clock); > +} > + > +static ssize_t sparx5_delay_clock_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + unsigned int delay_clock; > + struct sdhci_sparx5_data *sdhci_sparx5; > + > + sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data, > + dev_delay_clock); > + > + if (kstrtoint(buf, 10, &delay_clock) || > + delay_clock > MSHC_DLY_CC_MAX) { > + dev_err(dev, "sdhci-of-sparx5: wrong parameter format.\n"); > + return -EINVAL; > + } > + > + sdhci_sparx5->delay_clock = delay_clock; > + sparx5_set_delay(sdhci_sparx5->host, sdhci_sparx5->delay_clock); > + > + return strlen(buf); > +} > + > +int sdhci_sparx5_probe(struct platform_device *pdev) > +{ > + int ret; > + const char *syscon = "microchip,sparx5-cpu-syscon"; > + struct sdhci_host *host; > + struct sdhci_pltfm_host *pltfm_host; > + struct sdhci_sparx5_data *sdhci_sparx5; > + struct device_node *np = pdev->dev.of_node; > + u32 value; > + u32 extra; > + > + host = sdhci_pltfm_init(pdev, &sdhci_sparx5_pdata, > + sizeof(*sdhci_sparx5)); > + > + if (IS_ERR(host)) > + return PTR_ERR(host); > + > + /* > + * extra adma table cnt for cross 128M boundary handling. > + */ > + extra = DIV_ROUND_UP_ULL(dma_get_required_mask(&pdev->dev), SZ_128M); > + if (extra > SDHCI_MAX_SEGS) > + extra = SDHCI_MAX_SEGS; > + host->adma_table_cnt += extra; > + > + pltfm_host = sdhci_priv(host); > + sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host); > + sdhci_sparx5->host = host; > + > + pltfm_host->clk = devm_clk_get(&pdev->dev, "core"); > + if (IS_ERR(pltfm_host->clk)) { > + ret = PTR_ERR(pltfm_host->clk); > + dev_err(&pdev->dev, "failed to get core clk: %d\n", ret); > + goto free_pltfm; > + } > + ret = clk_prepare_enable(pltfm_host->clk); > + if (ret) > + goto free_pltfm; > + > + if (!of_property_read_u32(np, "microchip,clock-delay", &value) && > + value <= MSHC_DLY_CC_MAX) > + sdhci_sparx5->delay_clock = value; > + else > + sdhci_sparx5->delay_clock = -1; /* Autotune */ > + > + /* Sysfs delay_clock interface */ > + sdhci_sparx5->dev_delay_clock.show = sparx5_delay_clock_show; > + sdhci_sparx5->dev_delay_clock.store = sparx5_delay_clock_store; > + sysfs_attr_init(&sdhci_sparx5->dev_delay_clock.attr); > + sdhci_sparx5->dev_delay_clock.attr.name = "delay_clock"; > + sdhci_sparx5->dev_delay_clock.attr.mode = 0644; > + ret = device_create_file(&pdev->dev, &sdhci_sparx5->dev_delay_clock); Why is this needed? It seems doubtful that user space knows what value to put here if neither the board information nor the driver have that information. > + if (ret) > + dev_err(&pdev->dev, "failure creating '%s' device file", > + sdhci_sparx5->dev_delay_clock.attr.name); > + > + sdhci_get_of_property(pdev); > + > + ret = mmc_of_parse(host->mmc); > + if (ret) > + goto err_clk; > + > + sdhci_sparx5->cpu_ctrl = syscon_regmap_lookup_by_compatible(syscon); > + if (IS_ERR(sdhci_sparx5->cpu_ctrl)) { > + dev_err(&pdev->dev, "No CPU syscon regmap !\n"); > + ret = PTR_ERR(sdhci_sparx5->cpu_ctrl); > + goto err_clk; > + } > + > + if (sdhci_sparx5->delay_clock >= 0) > + sparx5_set_delay(host, sdhci_sparx5->delay_clock); > + > + if (!mmc_card_is_removable(host->mmc)) { > + /* Do a HW reset of eMMC card */ > + sdhci_sparx5_reset_emmc(host); > + /* Update EMMC_CTRL */ > + sdhci_sparx5_set_emmc(host); > + /* If eMMC, disable SD and SDIO */ > + host->mmc->caps2 |= (MMC_CAP2_NO_SDIO|MMC_CAP2_NO_SD); > + } > + > + /* Enable v4 mode */ > + //sdhci_enable_v4_mode(host); No commented out code please. > + > + ret = sdhci_add_host(host); > + if (ret) > + dev_err(&pdev->dev, "sdhci_add_host() failed (%d)\n", ret); > + > + /* Set AXI bus master to use un-cached access (for DMA) */ > + if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA) && > + IS_ENABLED(CONFIG_DMA_DECLARE_COHERENT)) > + sparx5_set_cacheable(host, ACP_CACHE_FORCE_ENA); > + > + pr_debug("SDHC version: 0x%08x\n", sdhci_readl(host, MSHC2_VERSION)); > + pr_debug("SDHC type: 0x%08x\n", sdhci_readl(host, MSHC2_TYPE)); > + > + return ret; > + > +err_clk: > + clk_disable_unprepare(pltfm_host->clk); > +free_pltfm: > + sdhci_pltfm_free(pdev); > + return ret; > +} > + > +static const struct of_device_id sdhci_sparx5_of_match[] = { > + { .compatible = "microchip,dw-sparx5-sdhci" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, sdhci_sparx5_of_match); > + > +static struct platform_driver sdhci_sparx5_driver = { > + .driver = { > + .name = "sdhci-sparx5", > + .of_match_table = sdhci_sparx5_of_match, > + .pm = &sdhci_pltfm_pmops, > + }, > + .probe = sdhci_sparx5_probe, > + .remove = sdhci_pltfm_unregister, > +}; > + > +module_platform_driver(sdhci_sparx5_driver); > + > +MODULE_DESCRIPTION("Sparx5 SDHCI OF driver"); > +MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>"); > +MODULE_LICENSE("GPL v2"); > -- > 2.26.2 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver 2020-05-17 12:58 ` Adrian Hunter @ 2020-05-18 8:58 ` Lars Povlsen 2020-05-20 11:14 ` Lars Povlsen 0 siblings, 1 reply; 12+ messages in thread From: Lars Povlsen @ 2020-05-18 8:58 UTC (permalink / raw) To: Adrian Hunter Cc: Lars Povlsen, Ulf Hansson, SoC Team, Microchip Linux Driver Support, linux-mmc, devicetree, linux-arm-kernel, linux-kernel, Alexandre Belloni Adrian Hunter writes: > On 13/05/20 4:31 pm, Lars Povlsen wrote: >> This adds the eMMC driver for the Sparx5 SoC. It is based upon the >> designware IP, but requires some extra initialization and quirks. >> >> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> >> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> >> --- >> drivers/mmc/host/Kconfig | 13 ++ >> drivers/mmc/host/Makefile | 1 + >> drivers/mmc/host/sdhci-of-sparx5.c | 348 +++++++++++++++++++++++++++++ >> 3 files changed, 362 insertions(+) >> create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c >> >> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig >> index 462b5352fea75..1e8396d09df75 100644 >> --- a/drivers/mmc/host/Kconfig >> +++ b/drivers/mmc/host/Kconfig >> @@ -213,6 +213,19 @@ config MMC_SDHCI_OF_DWCMSHC >> If you have a controller with this interface, say Y or M here. >> If unsure, say N. >> >> +config MMC_SDHCI_OF_SPARX5 >> + tristate "SDHCI OF support for the MCHP Sparx5 SoC" >> + depends on MMC_SDHCI_PLTFM >> + depends on ARCH_SPARX5 >> + select MMC_SDHCI_IO_ACCESSORS >> + help >> + This selects the Secure Digital Host Controller Interface (SDHCI) >> + found in the MCHP Sparx5 SoC. >> + >> + If you have a Sparx5 SoC with this interface, say Y or M here. >> + >> + If unsure, say N. >> + >> config MMC_SDHCI_CADENCE >> tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller" >> depends on MMC_SDHCI_PLTFM >> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile >> index b929ef9412083..9f09b7ffaaa16 100644 >> --- a/drivers/mmc/host/Makefile >> +++ b/drivers/mmc/host/Makefile >> @@ -89,6 +89,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ARASAN) += sdhci-of-arasan.o >> obj-$(CONFIG_MMC_SDHCI_OF_ASPEED) += sdhci-of-aspeed.o >> obj-$(CONFIG_MMC_SDHCI_OF_AT91) += sdhci-of-at91.o >> obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o >> +obj-$(CONFIG_MMC_SDHCI_OF_SPARX5) += sdhci-of-sparx5.o >> obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o >> obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC) += sdhci-of-dwcmshc.o >> obj-$(CONFIG_MMC_SDHCI_BCM_KONA) += sdhci-bcm-kona.o >> diff --git a/drivers/mmc/host/sdhci-of-sparx5.c b/drivers/mmc/host/sdhci-of-sparx5.c >> new file mode 100644 >> index 0000000000000..8253bf80e175a >> --- /dev/null >> +++ b/drivers/mmc/host/sdhci-of-sparx5.c >> @@ -0,0 +1,348 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * drivers/mmc/host/sdhci-of-sparx5.c >> + * >> + * MCHP Sparx5 SoC Secure Digital Host Controller Interface. >> + * >> + * Copyright (c) 2019 Microchip Inc. >> + * >> + * Author: Lars Povlsen <lars.povlsen@microchip.com> >> + */ >> + >> +//#define DEBUG >> +//#define TRACE_REGISTER > > No commented out code please. Yes, that's not relevant any more, I'll remove it. > >> + >> +#include <linux/sizes.h> >> +#include <linux/delay.h> >> +#include <linux/module.h> >> +#include <linux/regmap.h> >> +#include <linux/of_device.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/dma-mapping.h> >> + >> +#include "sdhci-pltfm.h" >> + >> +#define CPU_REGS_GENERAL_CTRL (0x22 * 4) >> +#define MSHC_DLY_CC_MASK GENMASK(16, 13) >> +#define MSHC_DLY_CC_SHIFT 13 >> +#define MSHC_DLY_CC_MAX 15 >> + >> +#define CPU_REGS_PROC_CTRL (0x2C * 4) >> +#define ACP_CACHE_FORCE_ENA BIT(4) >> +#define ACP_AWCACHE BIT(3) >> +#define ACP_ARCACHE BIT(2) >> +#define ACP_CACHE_MASK (ACP_CACHE_FORCE_ENA|ACP_AWCACHE|ACP_ARCACHE) >> + >> +#define MSHC2_VERSION 0x500 /* Off 0x140, reg 0x0 */ >> +#define MSHC2_TYPE 0x504 /* Off 0x140, reg 0x1 */ >> +#define MSHC2_EMMC_CTRL 0x52c /* Off 0x140, reg 0xB */ >> +#define MSHC2_EMMC_CTRL_EMMC_RST_N BIT(2) >> +#define MSHC2_EMMC_CTRL_IS_EMMC BIT(0) >> + >> +struct sdhci_sparx5_data { >> + struct sdhci_host *host; >> + struct regmap *cpu_ctrl; >> + int delay_clock; >> + struct device_attribute dev_delay_clock; >> +}; >> + >> +#define BOUNDARY_OK(addr, len) \ >> + ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1))) >> + >> +#if defined(TRACE_REGISTER) > > If you want this then add a Kconfig entry for it > No, I'll just remove it. >> +static void sdhci_sparx5_writel(struct sdhci_host *host, u32 val, int reg) >> +{ >> + pr_debug("$$$ writel(0x%08x, 0x%02x)\n", val, reg); >> + writel(val, host->ioaddr + reg); >> +} >> + >> +static void sdhci_sparx5_writew(struct sdhci_host *host, u16 val, int reg) >> +{ >> + pr_debug("$$$ writew(0x%04x, 0x%02x)\n", val, reg); >> + writew(val, host->ioaddr + reg); >> +} >> + >> +static void sdhci_sparx5_writeb(struct sdhci_host *host, u8 val, int reg) >> +{ >> + pr_debug("$$$ writeb(0x%02x, 0x%02x)\n", val, reg); >> + writeb(val, host->ioaddr + reg); >> +} >> +#endif >> + >> +/* >> + * If DMA addr spans 128MB boundary, we split the DMA transfer into two >> + * so that each DMA transfer doesn't exceed the boundary. >> + */ >> +static void sdhci_sparx5_adma_write_desc(struct sdhci_host *host, void **desc, >> + dma_addr_t addr, int len, >> + unsigned int cmd) >> +{ >> + int tmplen, offset; >> + >> + pr_debug("write_desc: cmd %02x: len %d, offset 0x%0llx\n", >> + cmd, len, addr); > > Please prefix all kernel messages by either the mmc or device e.g. > > pr_debug("%s: write_desc: cmd %02x: len %d, offset 0x%0llx\n", > mmc_hostname(host->mmc), cmd, len, addr); Will do. >> + >> + if (likely(!len || BOUNDARY_OK(addr, len))) { >> + sdhci_adma_write_desc(host, desc, addr, len, cmd); >> + return; >> + } >> + >> + pr_debug("write_desc: splitting dma len %d, offset 0x%0llx\n", >> + len, addr); >> + >> + offset = addr & (SZ_128M - 1); >> + tmplen = SZ_128M - offset; >> + sdhci_adma_write_desc(host, desc, addr, tmplen, cmd); >> + >> + addr += tmplen; >> + len -= tmplen; >> + sdhci_adma_write_desc(host, desc, addr, len, cmd); >> +} >> + >> +static void sparx5_set_cacheable(struct sdhci_host *host, u32 value) >> +{ >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> + struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host); >> + >> + pr_debug("%s: Set Cacheable = 0x%x\n", mmc_hostname(host->mmc), value); >> + >> + /* Update ACP caching attributes in HW */ >> + regmap_update_bits(sdhci_sparx5->cpu_ctrl, >> + CPU_REGS_PROC_CTRL, ACP_CACHE_MASK, value); >> +} >> + >> +static void sparx5_set_delay(struct sdhci_host *host, u8 value) >> +{ >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> + struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host); >> + >> + pr_debug("%s: Set DLY_CC = %u\n", mmc_hostname(host->mmc), value); >> + >> + /* Update DLY_CC in HW */ >> + regmap_update_bits(sdhci_sparx5->cpu_ctrl, >> + CPU_REGS_GENERAL_CTRL, >> + MSHC_DLY_CC_MASK, >> + (value << MSHC_DLY_CC_SHIFT)); >> +} >> + >> +static void sdhci_sparx5_set_emmc(struct sdhci_host *host) >> +{ >> + if (!mmc_card_is_removable(host->mmc)) { >> + u8 value; >> + >> + value = sdhci_readb(host, MSHC2_EMMC_CTRL); >> + if (!(value & MSHC2_EMMC_CTRL_IS_EMMC)) { >> + pr_debug("Get EMMC_CTRL: 0x%08x\n", value); >> + value |= MSHC2_EMMC_CTRL_IS_EMMC; >> + pr_debug("Set EMMC_CTRL: 0x%08x\n", value); >> + sdhci_writeb(host, value, MSHC2_EMMC_CTRL); >> + } >> + } >> +} >> + >> +static void sdhci_sparx5_reset_emmc(struct sdhci_host *host) >> +{ >> + u8 value; >> + >> + pr_debug("Toggle EMMC_CTRL.EMMC_RST_N\n"); >> + value = sdhci_readb(host, MSHC2_EMMC_CTRL) & >> + ~MSHC2_EMMC_CTRL_EMMC_RST_N; >> + sdhci_writeb(host, value, MSHC2_EMMC_CTRL); >> + /* For eMMC, minimum is 1us but give it 10us for good measure */ >> + udelay(10); >> + sdhci_writeb(host, value | MSHC2_EMMC_CTRL_EMMC_RST_N, >> + MSHC2_EMMC_CTRL); >> + /* For eMMC, minimum is 200us but give it 300us for good measure */ >> + udelay(300); > > usleep_range() is better here > Got it. >> +} >> + >> +static void sdhci_sparx5_reset(struct sdhci_host *host, u8 mask) >> +{ >> + pr_debug("*** RESET: mask %d\n", mask); >> + >> + sdhci_reset(host, mask); >> + >> + /* Be sure CARD_IS_EMMC stays set */ >> + sdhci_sparx5_set_emmc(host); >> +} >> + >> +static const struct sdhci_ops sdhci_sparx5_ops = { >> +#if defined(TRACE_REGISTER) >> + .write_l = sdhci_sparx5_writel, >> + .write_w = sdhci_sparx5_writew, >> + .write_b = sdhci_sparx5_writeb, >> +#endif >> + .set_clock = sdhci_set_clock, >> + .set_bus_width = sdhci_set_bus_width, >> + .set_uhs_signaling = sdhci_set_uhs_signaling, >> + .get_max_clock = sdhci_pltfm_clk_get_max_clock, >> + .reset = sdhci_sparx5_reset, >> + .adma_write_desc = sdhci_sparx5_adma_write_desc, >> +}; >> + >> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = { >> + .quirks = 0, >> + .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */ > > If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here. Yes, its supposedly a card quirk. I'll see to use the card quirks methods in place. > >> + SDHCI_QUIRK2_NO_1_8_V, /* No sdr104, ddr50, etc */ >> + .ops = &sdhci_sparx5_ops, >> +}; >> + >> +static ssize_t sparx5_delay_clock_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct sdhci_sparx5_data *sdhci_sparx5; >> + >> + sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data, >> + dev_delay_clock); >> + return scnprintf(buf, PAGE_SIZE, "%d\n", sdhci_sparx5->delay_clock); >> +} >> + >> +static ssize_t sparx5_delay_clock_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + unsigned int delay_clock; >> + struct sdhci_sparx5_data *sdhci_sparx5; >> + >> + sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data, >> + dev_delay_clock); >> + >> + if (kstrtoint(buf, 10, &delay_clock) || >> + delay_clock > MSHC_DLY_CC_MAX) { >> + dev_err(dev, "sdhci-of-sparx5: wrong parameter format.\n"); >> + return -EINVAL; >> + } >> + >> + sdhci_sparx5->delay_clock = delay_clock; >> + sparx5_set_delay(sdhci_sparx5->host, sdhci_sparx5->delay_clock); >> + >> + return strlen(buf); >> +} >> + >> +int sdhci_sparx5_probe(struct platform_device *pdev) >> +{ >> + int ret; >> + const char *syscon = "microchip,sparx5-cpu-syscon"; >> + struct sdhci_host *host; >> + struct sdhci_pltfm_host *pltfm_host; >> + struct sdhci_sparx5_data *sdhci_sparx5; >> + struct device_node *np = pdev->dev.of_node; >> + u32 value; >> + u32 extra; >> + >> + host = sdhci_pltfm_init(pdev, &sdhci_sparx5_pdata, >> + sizeof(*sdhci_sparx5)); >> + >> + if (IS_ERR(host)) >> + return PTR_ERR(host); >> + >> + /* >> + * extra adma table cnt for cross 128M boundary handling. >> + */ >> + extra = DIV_ROUND_UP_ULL(dma_get_required_mask(&pdev->dev), SZ_128M); >> + if (extra > SDHCI_MAX_SEGS) >> + extra = SDHCI_MAX_SEGS; >> + host->adma_table_cnt += extra; >> + >> + pltfm_host = sdhci_priv(host); >> + sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host); >> + sdhci_sparx5->host = host; >> + >> + pltfm_host->clk = devm_clk_get(&pdev->dev, "core"); >> + if (IS_ERR(pltfm_host->clk)) { >> + ret = PTR_ERR(pltfm_host->clk); >> + dev_err(&pdev->dev, "failed to get core clk: %d\n", ret); >> + goto free_pltfm; >> + } >> + ret = clk_prepare_enable(pltfm_host->clk); >> + if (ret) >> + goto free_pltfm; >> + >> + if (!of_property_read_u32(np, "microchip,clock-delay", &value) && >> + value <= MSHC_DLY_CC_MAX) >> + sdhci_sparx5->delay_clock = value; >> + else >> + sdhci_sparx5->delay_clock = -1; /* Autotune */ >> + >> + /* Sysfs delay_clock interface */ >> + sdhci_sparx5->dev_delay_clock.show = sparx5_delay_clock_show; >> + sdhci_sparx5->dev_delay_clock.store = sparx5_delay_clock_store; >> + sysfs_attr_init(&sdhci_sparx5->dev_delay_clock.attr); >> + sdhci_sparx5->dev_delay_clock.attr.name = "delay_clock"; >> + sdhci_sparx5->dev_delay_clock.attr.mode = 0644; >> + ret = device_create_file(&pdev->dev, &sdhci_sparx5->dev_delay_clock); > > Why is this needed? It seems doubtful that user space knows what value to > put here if neither the board information nor the driver have that information. The interface was provided to do tuning with a scope attached. As it is not a requirement, I'll remove it. > >> + if (ret) >> + dev_err(&pdev->dev, "failure creating '%s' device file", >> + sdhci_sparx5->dev_delay_clock.attr.name); >> + >> + sdhci_get_of_property(pdev); >> + >> + ret = mmc_of_parse(host->mmc); >> + if (ret) >> + goto err_clk; >> + >> + sdhci_sparx5->cpu_ctrl = syscon_regmap_lookup_by_compatible(syscon); >> + if (IS_ERR(sdhci_sparx5->cpu_ctrl)) { >> + dev_err(&pdev->dev, "No CPU syscon regmap !\n"); >> + ret = PTR_ERR(sdhci_sparx5->cpu_ctrl); >> + goto err_clk; >> + } >> + >> + if (sdhci_sparx5->delay_clock >= 0) >> + sparx5_set_delay(host, sdhci_sparx5->delay_clock); >> + >> + if (!mmc_card_is_removable(host->mmc)) { >> + /* Do a HW reset of eMMC card */ >> + sdhci_sparx5_reset_emmc(host); >> + /* Update EMMC_CTRL */ >> + sdhci_sparx5_set_emmc(host); >> + /* If eMMC, disable SD and SDIO */ >> + host->mmc->caps2 |= (MMC_CAP2_NO_SDIO|MMC_CAP2_NO_SD); >> + } >> + >> + /* Enable v4 mode */ >> + //sdhci_enable_v4_mode(host); > > No commented out code please. I'll remove this. > >> + >> + ret = sdhci_add_host(host); >> + if (ret) >> + dev_err(&pdev->dev, "sdhci_add_host() failed (%d)\n", ret); >> + >> + /* Set AXI bus master to use un-cached access (for DMA) */ >> + if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA) && >> + IS_ENABLED(CONFIG_DMA_DECLARE_COHERENT)) >> + sparx5_set_cacheable(host, ACP_CACHE_FORCE_ENA); >> + >> + pr_debug("SDHC version: 0x%08x\n", sdhci_readl(host, MSHC2_VERSION)); >> + pr_debug("SDHC type: 0x%08x\n", sdhci_readl(host, MSHC2_TYPE)); >> + >> + return ret; >> + >> +err_clk: >> + clk_disable_unprepare(pltfm_host->clk); >> +free_pltfm: >> + sdhci_pltfm_free(pdev); >> + return ret; >> +} >> + >> +static const struct of_device_id sdhci_sparx5_of_match[] = { >> + { .compatible = "microchip,dw-sparx5-sdhci" }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, sdhci_sparx5_of_match); >> + >> +static struct platform_driver sdhci_sparx5_driver = { >> + .driver = { >> + .name = "sdhci-sparx5", >> + .of_match_table = sdhci_sparx5_of_match, >> + .pm = &sdhci_pltfm_pmops, >> + }, >> + .probe = sdhci_sparx5_probe, >> + .remove = sdhci_pltfm_unregister, >> +}; >> + >> +module_platform_driver(sdhci_sparx5_driver); >> + >> +MODULE_DESCRIPTION("Sparx5 SDHCI OF driver"); >> +MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>"); >> +MODULE_LICENSE("GPL v2"); >> -- >> 2.26.2 >> Thank you very much for your comments, I will make the suggested changes and submit an new series asap. -- Lars Povlsen, Microchip ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver 2020-05-18 8:58 ` Lars Povlsen @ 2020-05-20 11:14 ` Lars Povlsen 2020-05-24 19:26 ` Adrian Hunter 0 siblings, 1 reply; 12+ messages in thread From: Lars Povlsen @ 2020-05-20 11:14 UTC (permalink / raw) To: Adrian Hunter Cc: Ulf Hansson, SoC Team, Microchip Linux Driver Support, linux-mmc, devicetree, linux-arm-kernel, linux-kernel, Alexandre Belloni, Lars Povlsen Lars Povlsen writes: > Adrian Hunter writes: > >> On 13/05/20 4:31 pm, Lars Povlsen wrote: >>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the >>> designware IP, but requires some extra initialization and quirks. >>> >>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> >>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> >>> --- {Snip] >>> +}; >>> + >>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = { >>> + .quirks = 0, >>> + .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */ >> >> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here. > Adrian, I had a go at changing the controller quirk to a card quirk. Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue. We are using a ISSI "IS004G" device, and so I have gone through the motions of adding it to quirks.h. The comment before the list of devices using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue, which is not exactly the issue I'm seeing. I'm seeing combinations of CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is issued. I have not been able to test the controller with another eMMC device yet, but I expect its not the controller at fault. So, I'm a little bit in doubt of how to proceed - either keep the quirk as a controller quirk - or make a *new* card quirk (with SDHCI_QUIRK2_HOST_NO_CMD23 semantics)? Anybody else have had experience with ISSI eMMC devices? I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not read from the controller just (unconditionally) set in sdhci.c - so that doesn't fly either. Any suggestions? > Yes, its supposedly a card quirk. I'll see to use the card quirks > methods in place. > -- Lars Povlsen, Microchip ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver 2020-05-20 11:14 ` Lars Povlsen @ 2020-05-24 19:26 ` Adrian Hunter 2020-05-25 14:26 ` Lars Povlsen 0 siblings, 1 reply; 12+ messages in thread From: Adrian Hunter @ 2020-05-24 19:26 UTC (permalink / raw) To: Lars Povlsen Cc: Ulf Hansson, SoC Team, Microchip Linux Driver Support, linux-mmc, devicetree, linux-arm-kernel, linux-kernel, Alexandre Belloni On 20/05/20 2:14 pm, Lars Povlsen wrote: > > Lars Povlsen writes: > >> Adrian Hunter writes: >> >>> On 13/05/20 4:31 pm, Lars Povlsen wrote: >>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the >>>> designware IP, but requires some extra initialization and quirks. >>>> >>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> >>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> >>>> --- > {Snip] >>>> +}; >>>> + >>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = { >>>> + .quirks = 0, >>>> + .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */ >>> >>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here. >> > > Adrian, I had a go at changing the controller quirk to a card quirk. > > Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to > MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it > will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue. > > We are using a ISSI "IS004G" device, and so I have gone through the > motions of adding it to quirks.h. The comment before the list of devices > using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue, > which is not exactly the issue I'm seeing. I'm seeing combinations of > CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is > issued. > > I have not been able to test the controller with another eMMC device > yet, but I expect its not the controller at fault. > > So, I'm a little bit in doubt of how to proceed - either keep the quirk > as a controller quirk - or make a *new* card quirk (with > SDHCI_QUIRK2_HOST_NO_CMD23 semantics)? > > Anybody else have had experience with ISSI eMMC devices? > > I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not > read from the controller just (unconditionally) set in sdhci.c - so that > doesn't fly either. > > Any suggestions? It is up to you. In the future, you may want to distinguish devices that have this problem from ones that do not. If you are not sure it is the ISSI eMMC, and maybe not the host controller, then might it be the board? Perhaps make SDHCI_QUIRK2_HOST_NO_CMD23 conditional on the particular compatibility string? At a minimum, change the "/* Card quirk */" comment to a fuller explanation. > >> Yes, its supposedly a card quirk. I'll see to use the card quirks >> methods in place. >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver 2020-05-24 19:26 ` Adrian Hunter @ 2020-05-25 14:26 ` Lars Povlsen 2020-05-29 14:11 ` Lars Povlsen 0 siblings, 1 reply; 12+ messages in thread From: Lars Povlsen @ 2020-05-25 14:26 UTC (permalink / raw) To: Adrian Hunter Cc: Lars Povlsen, Ulf Hansson, SoC Team, Microchip Linux Driver Support, linux-mmc, devicetree, linux-arm-kernel, linux-kernel, Alexandre Belloni Adrian Hunter writes: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 20/05/20 2:14 pm, Lars Povlsen wrote: >> >> Lars Povlsen writes: >> >>> Adrian Hunter writes: >>> >>>> On 13/05/20 4:31 pm, Lars Povlsen wrote: >>>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the >>>>> designware IP, but requires some extra initialization and quirks. >>>>> >>>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> >>>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> >>>>> --- >> {Snip] >>>>> +}; >>>>> + >>>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = { >>>>> + .quirks = 0, >>>>> + .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */ >>>> >>>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here. >>> >> >> Adrian, I had a go at changing the controller quirk to a card quirk. >> >> Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to >> MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it >> will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue. >> >> We are using a ISSI "IS004G" device, and so I have gone through the >> motions of adding it to quirks.h. The comment before the list of devices >> using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue, >> which is not exactly the issue I'm seeing. I'm seeing combinations of >> CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is >> issued. >> >> I have not been able to test the controller with another eMMC device >> yet, but I expect its not the controller at fault. >> >> So, I'm a little bit in doubt of how to proceed - either keep the quirk >> as a controller quirk - or make a *new* card quirk (with >> SDHCI_QUIRK2_HOST_NO_CMD23 semantics)? >> >> Anybody else have had experience with ISSI eMMC devices? >> >> I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not >> read from the controller just (unconditionally) set in sdhci.c - so that >> doesn't fly either. >> >> Any suggestions? > > It is up to you. In the future, you may want to distinguish devices that > have this problem from ones that do not. > > If you are not sure it is the ISSI eMMC, and maybe not the host controller, > then might it be the board? Perhaps make SDHCI_QUIRK2_HOST_NO_CMD23 > conditional on the particular compatibility string? > > At a minimum, change the "/* Card quirk */" comment to a fuller explanation. > Adrian, I'm getting a board ready with another eMMC device, and we're also trying to contact ISSI for info. My hope is to at least verify whether this is a controller or a card issue one way or the other. Then, I'll choose an appropriate solution for it. Thank you for your advice so far. ---Lars >> >>> Yes, its supposedly a card quirk. I'll see to use the card quirks >>> methods in place. >>> >> -- Lars Povlsen, Microchip ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver 2020-05-25 14:26 ` Lars Povlsen @ 2020-05-29 14:11 ` Lars Povlsen 0 siblings, 0 replies; 12+ messages in thread From: Lars Povlsen @ 2020-05-29 14:11 UTC (permalink / raw) To: Adrian Hunter Cc: Ulf Hansson, SoC Team, Microchip Linux Driver Support, linux-mmc, devicetree, linux-arm-kernel, linux-kernel, Alexandre Belloni, Lars Povlsen Lars Povlsen writes: > Adrian Hunter writes: > >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On 20/05/20 2:14 pm, Lars Povlsen wrote: >>> >>> Lars Povlsen writes: >>> >>>> Adrian Hunter writes: >>>> >>>>> On 13/05/20 4:31 pm, Lars Povlsen wrote: >>>>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the >>>>>> designware IP, but requires some extra initialization and quirks. >>>>>> >>>>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> >>>>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> >>>>>> --- >>> {Snip] >>>>>> +}; >>>>>> + >>>>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = { >>>>>> + .quirks = 0, >>>>>> + .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */ >>>>> >>>>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here. >>>> >>> >>> Adrian, I had a go at changing the controller quirk to a card quirk. >>> >>> Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to >>> MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it >>> will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue. >>> >>> We are using a ISSI "IS004G" device, and so I have gone through the >>> motions of adding it to quirks.h. The comment before the list of devices >>> using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue, >>> which is not exactly the issue I'm seeing. I'm seeing combinations of >>> CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is >>> issued. >>> >>> I have not been able to test the controller with another eMMC device >>> yet, but I expect its not the controller at fault. >>> >>> So, I'm a little bit in doubt of how to proceed - either keep the quirk >>> as a controller quirk - or make a *new* card quirk (with >>> SDHCI_QUIRK2_HOST_NO_CMD23 semantics)? >>> >>> Anybody else have had experience with ISSI eMMC devices? >>> >>> I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not >>> read from the controller just (unconditionally) set in sdhci.c - so that >>> doesn't fly either. >>> >>> Any suggestions? >> >> It is up to you. In the future, you may want to distinguish devices that >> have this problem from ones that do not. >> >> If you are not sure it is the ISSI eMMC, and maybe not the host controller, >> then might it be the board? Perhaps make SDHCI_QUIRK2_HOST_NO_CMD23 >> conditional on the particular compatibility string? >> >> At a minimum, change the "/* Card quirk */" comment to a fuller explanation. >> > > Adrian, I'm getting a board ready with another eMMC device, and we're > also trying to contact ISSI for info. > > My hope is to at least verify whether this is a controller or a card > issue one way or the other. Then, I'll choose an appropriate solution > for it. > > Thank you for your advice so far. > I was able to try on a board with another eMMC card (panasonic), so that clearly casts the suspicion on the controller, and ISSI is in the clear. I reintroduced the original SDHCI_QUIRK2_HOST_NO_CMD23 quirk, with a "Controller issue" comment. I will refresh the series shortly. Cheers, > ---Lars > >>> >>>> Yes, its supposedly a card quirk. I'll see to use the card quirks >>>> methods in place. >>>> >>> -- Lars Povlsen, Microchip ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] arm64: dts: sparx5: Add Sparx5 eMMC support 2020-05-13 13:31 [PATCH 0/3] mmc: Adding support for Microchip Sparx5 SoC Lars Povlsen 2020-05-13 13:31 ` [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings Lars Povlsen 2020-05-13 13:31 ` [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver Lars Povlsen @ 2020-05-13 13:31 ` Lars Povlsen 2 siblings, 0 replies; 12+ messages in thread From: Lars Povlsen @ 2020-05-13 13:31 UTC (permalink / raw) To: Ulf Hansson, Adrian Hunter, SoC Team Cc: Lars Povlsen, Microchip Linux Driver Support, linux-mmc, devicetree, linux-arm-kernel, linux-kernel, Alexandre Belloni This adds eMMC support to the applicable Sparx5 board configuration files. Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> --- arch/arm64/boot/dts/microchip/sparx5.dtsi | 24 +++++++++++++++++++ .../boot/dts/microchip/sparx5_pcb125.dts | 23 ++++++++++++++++++ .../boot/dts/microchip/sparx5_pcb134_emmc.dts | 23 ++++++++++++++++++ .../boot/dts/microchip/sparx5_pcb135_emmc.dts | 23 ++++++++++++++++++ 4 files changed, 93 insertions(+) diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi index 3e94ac9e7dd51..f09a49c41ce19 100644 --- a/arch/arm64/boot/dts/microchip/sparx5.dtsi +++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi @@ -5,6 +5,7 @@ #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/arm-gic.h> +#include <dt-bindings/clock/microchip,sparx5.h> / { compatible = "microchip,sparx5"; @@ -151,6 +152,20 @@ timer1: timer@600105000 { interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>; }; + sdhci0: sdhci@600800000 { + compatible = "microchip,dw-sparx5-sdhci"; + status = "disabled"; + reg = <0x6 0x00800000 0x1000>; + pinctrl-0 = <&emmc_pins>; + pinctrl-names = "default"; + clocks = <&clks CLK_ID_AUX1>; + clock-names = "core"; + assigned-clocks = <&clks CLK_ID_AUX1>; + assigned-clock-rates = <800000000>; + interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>; + bus-width = <8>; + }; + gpio: pinctrl@6110101e0 { compatible = "microchip,sparx5-pinctrl"; reg = <0x6 0x110101e0 0x90>, <0x6 0x10508010 0x100>; @@ -180,6 +195,15 @@ i2c2_pins: i2c2-pins { pins = "GPIO_28", "GPIO_29"; function = "twi2"; }; + + emmc_pins: emmc-pins { + pins = "GPIO_34", "GPIO_35", "GPIO_36", + "GPIO_37", "GPIO_38", "GPIO_39", + "GPIO_40", "GPIO_41", "GPIO_42", + "GPIO_43", "GPIO_44", "GPIO_45", + "GPIO_46", "GPIO_47"; + function = "emmc"; + }; }; i2c0: i2c@600101000 { diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts index 91ee5b6cfc37a..573309fe45823 100644 --- a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts +++ b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts @@ -16,6 +16,29 @@ memory@0 { }; }; +&gpio { + emmc_pins: emmc-pins { + /* NB: No "GPIO_35", "GPIO_36", "GPIO_37" + * (N/A: CARD_nDETECT, CARD_WP, CARD_LED) + */ + pins = "GPIO_34", "GPIO_38", "GPIO_39", + "GPIO_40", "GPIO_41", "GPIO_42", + "GPIO_43", "GPIO_44", "GPIO_45", + "GPIO_46", "GPIO_47"; + drive-strength = <3>; + function = "emmc"; + }; +}; + +&sdhci0 { + status = "okay"; + bus-width = <8>; + non-removable; + pinctrl-0 = <&emmc_pins>; + max-frequency = <8000000>; + microchip,clock-delay = <10>; +}; + &i2c1 { status = "okay"; }; diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts index 10081a66961bb..bbb9852c1f151 100644 --- a/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts +++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts @@ -15,3 +15,26 @@ memory@0 { reg = <0x00000000 0x00000000 0x10000000>; }; }; + +&gpio { + emmc_pins: emmc-pins { + /* NB: No "GPIO_35", "GPIO_36", "GPIO_37" + * (N/A: CARD_nDETECT, CARD_WP, CARD_LED) + */ + pins = "GPIO_34", "GPIO_38", "GPIO_39", + "GPIO_40", "GPIO_41", "GPIO_42", + "GPIO_43", "GPIO_44", "GPIO_45", + "GPIO_46", "GPIO_47"; + drive-strength = <3>; + function = "emmc"; + }; +}; + +&sdhci0 { + status = "okay"; + pinctrl-0 = <&emmc_pins>; + non-removable; + max-frequency = <52000000>; + bus-width = <8>; + microchip,clock-delay = <10>; +}; diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts index 741f0e12260e5..f82266fe2ad49 100644 --- a/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts +++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts @@ -15,3 +15,26 @@ memory@0 { reg = <0x00000000 0x00000000 0x10000000>; }; }; + +&gpio { + emmc_pins: emmc-pins { + /* NB: No "GPIO_35", "GPIO_36", "GPIO_37" + * (N/A: CARD_nDETECT, CARD_WP, CARD_LED) + */ + pins = "GPIO_34", "GPIO_38", "GPIO_39", + "GPIO_40", "GPIO_41", "GPIO_42", + "GPIO_43", "GPIO_44", "GPIO_45", + "GPIO_46", "GPIO_47"; + drive-strength = <3>; + function = "emmc"; + }; +}; + +&sdhci0 { + status = "okay"; + pinctrl-0 = <&emmc_pins>; + non-removable; + max-frequency = <52000000>; + bus-width = <8>; + microchip,clock-delay = <10>; +}; -- 2.26.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-05-29 14:11 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-13 13:31 [PATCH 0/3] mmc: Adding support for Microchip Sparx5 SoC Lars Povlsen 2020-05-13 13:31 ` [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings Lars Povlsen 2020-05-14 13:03 ` Rob Herring 2020-05-15 15:50 ` Lars Povlsen 2020-05-13 13:31 ` [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver Lars Povlsen 2020-05-17 12:58 ` Adrian Hunter 2020-05-18 8:58 ` Lars Povlsen 2020-05-20 11:14 ` Lars Povlsen 2020-05-24 19:26 ` Adrian Hunter 2020-05-25 14:26 ` Lars Povlsen 2020-05-29 14:11 ` Lars Povlsen 2020-05-13 13:31 ` [PATCH 3/3] arm64: dts: sparx5: Add Sparx5 eMMC support Lars Povlsen
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).