| Message ID | a3d82e5a28fe53f1f61621d37d1695b0cd7655d2.1713036964.git.andrea.porta@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for BCM2712 SD card controller | expand |
Le 14/04/2024 à 00:14, Andrea della Porta a écrit : > Broadcom BCM2712 SDHCI controller is SD Express capable. Add support > for sde capability where the implementation is based on downstream driver, > diverging from it in the way that init_sd_express callback is invoked: > in downstream the sdhci_ops structure has been augmented with a new > function ptr 'init_sd_express' that just propagate the call to the > driver specific callback so that the callstack from a structure > standpoint is mmc_host_ops -> sdhci_ops. The drawback here is in the > added level of indirection (the newly added init_sd_express is > redundant) and the sdhci_ops structure declaration has to be changed. > To overcome this the presented approach consist in patching the mmc_host_ops > init_sd_express callback to point directly to the custom function defined in > this driver (see struct brcmstb_match_priv). > > Signed-off-by: Andrea della Porta <andrea.porta-IBi9RG/b67k@public.gmane.org> > --- > drivers/mmc/host/Kconfig | 1 + > drivers/mmc/host/sdhci-brcmstb.c | 147 ++++++++++++++++++++++++++++++- > 2 files changed, 147 insertions(+), 1 deletion(-) ... > + if (brcmstb_priv->sde_pcie) { > + struct of_changeset changeset; > + static struct property okay_property = { > + .name = "status", > + .value = "okay", > + .length = 5, > + }; > + > + /* Enable the pcie controller */ > + of_changeset_init(&changeset); > + ret = of_changeset_update_property(&changeset, > + brcmstb_priv->sde_pcie, > + &okay_property); > + if (ret) { > + dev_err(dev, "%s: failed to update property - %d\n", __func__, > + ret); > + return -ENODEV; > + } > + ret = of_changeset_apply(&changeset); > + } > + > + dev_dbg(dev, "%s -> %d\n", __func__, ret); Is this really useful? > + return ret; > +} > + ... > @@ -468,6 +581,24 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) > if (res) > goto err; > > + priv->sde_1v8 = devm_regulator_get_optional(&pdev->dev, "sde-1v8"); > + if (IS_ERR(priv->sde_1v8)) > + priv->flags &= ~BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS; > + > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 2); > + if (iomem) { > + priv->sde_ioaddr = devm_ioremap_resource(&pdev->dev, iomem); > + if (IS_ERR(priv->sde_ioaddr)) > + priv->sde_ioaddr = NULL; > + } > + > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 3); > + if (iomem) { > + priv->sde_ioaddr2 = devm_ioremap_resource(&pdev->dev, iomem); > + if (IS_ERR(priv->sde_ioaddr2)) > + priv->sde_ioaddr = NULL; sde_ioaddr2 ? > + } > + > priv->pinctrl = devm_pinctrl_get(&pdev->dev); > if (IS_ERR(priv->pinctrl)) { > no_pinctrl = true; > @@ -478,8 +609,16 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) > no_pinctrl = true; > } > > - if (no_pinctrl ) > + priv->pins_sdex = pinctrl_lookup_state(priv->pinctrl, "sd-express"); > + if (IS_ERR(priv->pins_sdex)) { > + dev_dbg(&pdev->dev, "No pinctrl sd-express state\n"); > + no_pinctrl = true; Indentation looks too large. > + } > + > + if (no_pinctrl || !priv->sde_ioaddr || !priv->sde_ioaddr2) { > priv->pinctrl = NULL; > + priv->flags &= ~BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS; > + } > > /* > * Automatic clock gating does not work for SD cards that may ...
On 4/13/2024 3:14 PM, Andrea della Porta wrote: > Broadcom BCM2712 SDHCI controller is SD Express capable. Add support > for sde capability where the implementation is based on downstream driver, > diverging from it in the way that init_sd_express callback is invoked: > in downstream the sdhci_ops structure has been augmented with a new > function ptr 'init_sd_express' that just propagate the call to the > driver specific callback so that the callstack from a structure > standpoint is mmc_host_ops -> sdhci_ops. The drawback here is in the > added level of indirection (the newly added init_sd_express is > redundant) and the sdhci_ops structure declaration has to be changed. > To overcome this the presented approach consist in patching the mmc_host_ops > init_sd_express callback to point directly to the custom function defined in > this driver (see struct brcmstb_match_priv). > > Signed-off-by: Andrea della Porta <andrea.porta@suse.com> > --- > drivers/mmc/host/Kconfig | 1 + > drivers/mmc/host/sdhci-brcmstb.c | 147 ++++++++++++++++++++++++++++++- > 2 files changed, 147 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index aebc587f77a7..343ccac1a4e4 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -1018,6 +1018,7 @@ config MMC_SDHCI_BRCMSTB > depends on ARCH_BRCMSTB || BMIPS_GENERIC || COMPILE_TEST > depends on MMC_SDHCI_PLTFM > select MMC_CQHCI > + select OF_DYNAMIC > default ARCH_BRCMSTB || BMIPS_GENERIC > help > This selects support for the SDIO/SD/MMC Host Controller on > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c > index 907a4947abe5..56fb34a75ec2 100644 > --- a/drivers/mmc/host/sdhci-brcmstb.c > +++ b/drivers/mmc/host/sdhci-brcmstb.c > @@ -29,6 +29,7 @@ > > #define BRCMSTB_PRIV_FLAGS_HAS_CQE BIT(0) > #define BRCMSTB_PRIV_FLAGS_GATE_CLOCK BIT(1) > +#define BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS BIT(2) > > #define SDHCI_ARASAN_CQE_BASE_ADDR 0x200 > > @@ -50,13 +51,19 @@ struct sdhci_brcmstb_priv { > unsigned int flags; > struct clk *base_clk; > u32 base_freq_hz; > + struct regulator *sde_1v8; > + struct device_node *sde_pcie; > + void *__iomem sde_ioaddr; > + void *__iomem sde_ioaddr2; > struct pinctrl *pinctrl; > struct pinctrl_state *pins_default; > + struct pinctrl_state *pins_sdex; > }; > > struct brcmstb_match_priv { > void (*hs400es)(struct mmc_host *mmc, struct mmc_ios *ios); > void (*cfginit)(struct sdhci_host *host); > + int (*init_sd_express)(struct mmc_host *mmc, struct mmc_ios *ios); > struct sdhci_ops *ops; > const unsigned int flags; > }; > @@ -263,6 +270,105 @@ static void sdhci_brcmstb_cfginit_2712(struct sdhci_host *host) > } > } > > +static int bcm2712_init_sd_express(struct mmc_host *mmc, struct mmc_ios *ios) > +{ > + struct sdhci_host *host = mmc_priv(mmc); > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_brcmstb_priv *brcmstb_priv = sdhci_pltfm_priv(pltfm_host); > + struct device *dev = host->mmc->parent; > + u32 ctrl_val; > + u32 present_state; > + int ret; > + > + if (!brcmstb_priv->sde_ioaddr || !brcmstb_priv->sde_ioaddr2) > + return -EINVAL; > + > + if (!brcmstb_priv->pinctrl) > + return -EINVAL; > + > + /* Turn off the SD clock first */ > + sdhci_set_clock(host, 0); > + > + /* Disable SD DAT0-3 pulls */ > + pinctrl_select_state(brcmstb_priv->pinctrl, brcmstb_priv->pins_sdex); > + > + ctrl_val = readl(brcmstb_priv->sde_ioaddr); > + dev_dbg(dev, "ctrl_val 1 %08x\n", ctrl_val); > + > + /* Tri-state the SD pins */ > + ctrl_val |= 0x1ff8; No magic values please. > + writel(ctrl_val, brcmstb_priv->sde_ioaddr); > + dev_dbg(dev, "ctrl_val 1->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr)); > + /* Let voltages settle */ > + udelay(100); Why not usleep_range()? > + > + /* Enable the PCIe sideband pins */ > + ctrl_val &= ~0x6000; No magic values please. > + writel(ctrl_val, brcmstb_priv->sde_ioaddr); > + dev_dbg(dev, "ctrl_val 1->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr)); > + /* Let voltages settle */ > + udelay(100); Likewise. > + > + /* Turn on the 1v8 VDD2 regulator */ > + ret = regulator_enable(brcmstb_priv->sde_1v8); > + if (ret) > + return ret; > + > + /* Wait for Tpvcrl */ > + msleep(1); > + > + /* Sample DAT2 (CLKREQ#) - if low, card is in PCIe mode */ > + present_state = sdhci_readl(host, SDHCI_PRESENT_STATE); > + present_state = (present_state & SDHCI_DATA_LVL_MASK) >> SDHCI_DATA_LVL_SHIFT; > + dev_dbg(dev, "state = 0x%08x\n", present_state); > + > + if (present_state & BIT(2)) { Likewise, replace with constant. > + dev_err(dev, "DAT2 still high, abandoning SDex switch\n"); > + return -ENODEV; > + } > + > + /* Turn on the LCPLL PTEST mux */ > + ctrl_val = readl(brcmstb_priv->sde_ioaddr2 + 20); // misc5 > + ctrl_val &= ~(0x7 << 7); > + ctrl_val |= 3 << 7; > + writel(ctrl_val, brcmstb_priv->sde_ioaddr2 + 20); > + dev_dbg(dev, "misc 5->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr2 + 20)); > + > + /* PTEST diff driver enable */ > + ctrl_val = readl(brcmstb_priv->sde_ioaddr2); > + ctrl_val |= BIT(21); > + writel(ctrl_val, brcmstb_priv->sde_ioaddr2); > + > + dev_dbg(dev, "misc 0->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr2)); > + > + /* Wait for more than the minimum Tpvpgl time */ > + msleep(100); > + > + if (brcmstb_priv->sde_pcie) { > + struct of_changeset changeset; > + static struct property okay_property = { > + .name = "status", > + .value = "okay", > + .length = 5, > + }; > + > + /* Enable the pcie controller */ > + of_changeset_init(&changeset); > + ret = of_changeset_update_property(&changeset, > + brcmstb_priv->sde_pcie, > + &okay_property); > + if (ret) { > + dev_err(dev, "%s: failed to update property - %d\n", __func__, > + ret); > + return -ENODEV; > + } > + ret = of_changeset_apply(&changeset); > + } Why are you doing this? Cannot the firmware enable/disable the node according to the boot mode or something else? This is not going to fly for upstream, sorry.
On 09:34 Sun 14 Apr , Christophe JAILLET wrote: > Le 14/04/2024 à 00:14, Andrea della Porta a écrit : > > Broadcom BCM2712 SDHCI controller is SD Express capable. Add support > > for sde capability where the implementation is based on downstream driver, > > diverging from it in the way that init_sd_express callback is invoked: > > in downstream the sdhci_ops structure has been augmented with a new > > function ptr 'init_sd_express' that just propagate the call to the > > driver specific callback so that the callstack from a structure > > standpoint is mmc_host_ops -> sdhci_ops. The drawback here is in the > > added level of indirection (the newly added init_sd_express is > > redundant) and the sdhci_ops structure declaration has to be changed. > > To overcome this the presented approach consist in patching the mmc_host_ops > > init_sd_express callback to point directly to the custom function defined in > > this driver (see struct brcmstb_match_priv). > > > > Signed-off-by: Andrea della Porta <andrea.porta-IBi9RG/b67k@public.gmane.org> > > --- > > drivers/mmc/host/Kconfig | 1 + > > drivers/mmc/host/sdhci-brcmstb.c | 147 ++++++++++++++++++++++++++++++- > > 2 files changed, 147 insertions(+), 1 deletion(-) > > ... > > > + if (brcmstb_priv->sde_pcie) { > > + struct of_changeset changeset; > > + static struct property okay_property = { > > + .name = "status", > > + .value = "okay", > > + .length = 5, > > + }; > > + > > + /* Enable the pcie controller */ > > + of_changeset_init(&changeset); > > + ret = of_changeset_update_property(&changeset, > > + brcmstb_priv->sde_pcie, > > + &okay_property); > > + if (ret) { > > + dev_err(dev, "%s: failed to update property - %d\n", __func__, > > + ret); > > + return -ENODEV; > > + } > > + ret = of_changeset_apply(&changeset); > > + } > > + > > + dev_dbg(dev, "%s -> %d\n", __func__, ret); > > Is this really useful? Not really. Removed. > > > + return ret; > > +} > > + > > ... > > > @@ -468,6 +581,24 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) > > if (res) > > goto err; > > + priv->sde_1v8 = devm_regulator_get_optional(&pdev->dev, "sde-1v8"); > > + if (IS_ERR(priv->sde_1v8)) > > + priv->flags &= ~BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS; > > + > > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 2); > > + if (iomem) { > > + priv->sde_ioaddr = devm_ioremap_resource(&pdev->dev, iomem); > > + if (IS_ERR(priv->sde_ioaddr)) > > + priv->sde_ioaddr = NULL; > > + } > > + > > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 3); > > + if (iomem) { > > + priv->sde_ioaddr2 = devm_ioremap_resource(&pdev->dev, iomem); > > + if (IS_ERR(priv->sde_ioaddr2)) > > + priv->sde_ioaddr = NULL; > > sde_ioaddr2 ? > > > + } > > + > > priv->pinctrl = devm_pinctrl_get(&pdev->dev); > > if (IS_ERR(priv->pinctrl)) { > > no_pinctrl = true; > > @@ -478,8 +609,16 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) > > no_pinctrl = true; > > } > > - if (no_pinctrl ) > > + priv->pins_sdex = pinctrl_lookup_state(priv->pinctrl, "sd-express"); > > + if (IS_ERR(priv->pins_sdex)) { > > + dev_dbg(&pdev->dev, "No pinctrl sd-express state\n"); > > + no_pinctrl = true; > > Indentation looks too large. Ack. > > > + } > > + > > + if (no_pinctrl || !priv->sde_ioaddr || !priv->sde_ioaddr2) { > > priv->pinctrl = NULL; > > + priv->flags &= ~BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS; > > + } > > /* > > * Automatic clock gating does not work for SD cards that may > > ... > In general I'll drop SD express patch for now, it will be re-introduced in a future patch. Best regards, Andrea
On 08:55 Sun 14 Apr , Florian Fainelli wrote: > > > On 4/13/2024 3:14 PM, Andrea della Porta wrote: > > Broadcom BCM2712 SDHCI controller is SD Express capable. Add support > > for sde capability where the implementation is based on downstream driver, > > diverging from it in the way that init_sd_express callback is invoked: > > in downstream the sdhci_ops structure has been augmented with a new > > function ptr 'init_sd_express' that just propagate the call to the > > driver specific callback so that the callstack from a structure > > standpoint is mmc_host_ops -> sdhci_ops. The drawback here is in the > > added level of indirection (the newly added init_sd_express is > > redundant) and the sdhci_ops structure declaration has to be changed. > > To overcome this the presented approach consist in patching the mmc_host_ops > > init_sd_express callback to point directly to the custom function defined in > > this driver (see struct brcmstb_match_priv). > > > > Signed-off-by: Andrea della Porta <andrea.porta@suse.com> > > --- > > drivers/mmc/host/Kconfig | 1 + > > drivers/mmc/host/sdhci-brcmstb.c | 147 ++++++++++++++++++++++++++++++- > > 2 files changed, 147 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > > index aebc587f77a7..343ccac1a4e4 100644 > > --- a/drivers/mmc/host/Kconfig > > +++ b/drivers/mmc/host/Kconfig > > @@ -1018,6 +1018,7 @@ config MMC_SDHCI_BRCMSTB > > depends on ARCH_BRCMSTB || BMIPS_GENERIC || COMPILE_TEST > > depends on MMC_SDHCI_PLTFM > > select MMC_CQHCI > > + select OF_DYNAMIC > > default ARCH_BRCMSTB || BMIPS_GENERIC > > help > > This selects support for the SDIO/SD/MMC Host Controller on > > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c > > index 907a4947abe5..56fb34a75ec2 100644 > > --- a/drivers/mmc/host/sdhci-brcmstb.c > > +++ b/drivers/mmc/host/sdhci-brcmstb.c > > @@ -29,6 +29,7 @@ > > #define BRCMSTB_PRIV_FLAGS_HAS_CQE BIT(0) > > #define BRCMSTB_PRIV_FLAGS_GATE_CLOCK BIT(1) > > +#define BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS BIT(2) > > #define SDHCI_ARASAN_CQE_BASE_ADDR 0x200 > > @@ -50,13 +51,19 @@ struct sdhci_brcmstb_priv { > > unsigned int flags; > > struct clk *base_clk; > > u32 base_freq_hz; > > + struct regulator *sde_1v8; > > + struct device_node *sde_pcie; > > + void *__iomem sde_ioaddr; > > + void *__iomem sde_ioaddr2; > > struct pinctrl *pinctrl; > > struct pinctrl_state *pins_default; > > + struct pinctrl_state *pins_sdex; > > }; > > struct brcmstb_match_priv { > > void (*hs400es)(struct mmc_host *mmc, struct mmc_ios *ios); > > void (*cfginit)(struct sdhci_host *host); > > + int (*init_sd_express)(struct mmc_host *mmc, struct mmc_ios *ios); > > struct sdhci_ops *ops; > > const unsigned int flags; > > }; > > @@ -263,6 +270,105 @@ static void sdhci_brcmstb_cfginit_2712(struct sdhci_host *host) > > } > > } > > +static int bcm2712_init_sd_express(struct mmc_host *mmc, struct mmc_ios *ios) > > +{ > > + struct sdhci_host *host = mmc_priv(mmc); > > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > > + struct sdhci_brcmstb_priv *brcmstb_priv = sdhci_pltfm_priv(pltfm_host); > > + struct device *dev = host->mmc->parent; > > + u32 ctrl_val; > > + u32 present_state; > > + int ret; > > + > > + if (!brcmstb_priv->sde_ioaddr || !brcmstb_priv->sde_ioaddr2) > > + return -EINVAL; > > + > > + if (!brcmstb_priv->pinctrl) > > + return -EINVAL; > > + > > + /* Turn off the SD clock first */ > > + sdhci_set_clock(host, 0); > > + > > + /* Disable SD DAT0-3 pulls */ > > + pinctrl_select_state(brcmstb_priv->pinctrl, brcmstb_priv->pins_sdex); > > + > > + ctrl_val = readl(brcmstb_priv->sde_ioaddr); > > + dev_dbg(dev, "ctrl_val 1 %08x\n", ctrl_val); > > + > > + /* Tri-state the SD pins */ > > + ctrl_val |= 0x1ff8; > > No magic values please. Ack. > > > + writel(ctrl_val, brcmstb_priv->sde_ioaddr); > > + dev_dbg(dev, "ctrl_val 1->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr)); > > + /* Let voltages settle */ > > + udelay(100); > > Why not usleep_range()? No real reason. I assume only the lower boundary is critical so I can use usleep_range instead. Will be fixed in a future patch, the SD express support will be drpped in V2 since nto strictly necessary. > > > + > > + /* Enable the PCIe sideband pins */ > > + ctrl_val &= ~0x6000; > > No magic values please. > > > + writel(ctrl_val, brcmstb_priv->sde_ioaddr); > > + dev_dbg(dev, "ctrl_val 1->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr)); > > + /* Let voltages settle */ > > + udelay(100); > > Likewise. Ditto. > > > + > > + /* Turn on the 1v8 VDD2 regulator */ > > + ret = regulator_enable(brcmstb_priv->sde_1v8); > > + if (ret) > > + return ret; > > + > > + /* Wait for Tpvcrl */ > > + msleep(1); > > + > > + /* Sample DAT2 (CLKREQ#) - if low, card is in PCIe mode */ > > + present_state = sdhci_readl(host, SDHCI_PRESENT_STATE); > > + present_state = (present_state & SDHCI_DATA_LVL_MASK) >> SDHCI_DATA_LVL_SHIFT; > > + dev_dbg(dev, "state = 0x%08x\n", present_state); > > + > > + if (present_state & BIT(2)) { > > Likewise, replace with constant. Ack. > > > + dev_err(dev, "DAT2 still high, abandoning SDex switch\n"); > > + return -ENODEV; > > + } > > + > > + /* Turn on the LCPLL PTEST mux */ > > + ctrl_val = readl(brcmstb_priv->sde_ioaddr2 + 20); // misc5 > > + ctrl_val &= ~(0x7 << 7); > > + ctrl_val |= 3 << 7; > > + writel(ctrl_val, brcmstb_priv->sde_ioaddr2 + 20); > > + dev_dbg(dev, "misc 5->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr2 + 20)); > > + > > + /* PTEST diff driver enable */ > > + ctrl_val = readl(brcmstb_priv->sde_ioaddr2); > > + ctrl_val |= BIT(21); > > + writel(ctrl_val, brcmstb_priv->sde_ioaddr2); > > + > > + dev_dbg(dev, "misc 0->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr2)); > > + > > + /* Wait for more than the minimum Tpvpgl time */ > > + msleep(100); > > + > > + if (brcmstb_priv->sde_pcie) { > > + struct of_changeset changeset; > > + static struct property okay_property = { > > + .name = "status", > > + .value = "okay", > > + .length = 5, > > + }; > > + > > + /* Enable the pcie controller */ > > + of_changeset_init(&changeset); > > + ret = of_changeset_update_property(&changeset, > > + brcmstb_priv->sde_pcie, > > + &okay_property); > > + if (ret) { > > + dev_err(dev, "%s: failed to update property - %d\n", __func__, > > + ret); > > + return -ENODEV; > > + } > > + ret = of_changeset_apply(&changeset); > > + } > > Why are you doing this? Cannot the firmware enable/disable the node > according to the boot mode or something else? > > This is not going to fly for upstream, sorry. > -- > Florian
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index aebc587f77a7..343ccac1a4e4 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -1018,6 +1018,7 @@ config MMC_SDHCI_BRCMSTB depends on ARCH_BRCMSTB || BMIPS_GENERIC || COMPILE_TEST depends on MMC_SDHCI_PLTFM select MMC_CQHCI + select OF_DYNAMIC default ARCH_BRCMSTB || BMIPS_GENERIC help This selects support for the SDIO/SD/MMC Host Controller on diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c index 907a4947abe5..56fb34a75ec2 100644 --- a/drivers/mmc/host/sdhci-brcmstb.c +++ b/drivers/mmc/host/sdhci-brcmstb.c @@ -29,6 +29,7 @@ #define BRCMSTB_PRIV_FLAGS_HAS_CQE BIT(0) #define BRCMSTB_PRIV_FLAGS_GATE_CLOCK BIT(1) +#define BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS BIT(2) #define SDHCI_ARASAN_CQE_BASE_ADDR 0x200 @@ -50,13 +51,19 @@ struct sdhci_brcmstb_priv { unsigned int flags; struct clk *base_clk; u32 base_freq_hz; + struct regulator *sde_1v8; + struct device_node *sde_pcie; + void *__iomem sde_ioaddr; + void *__iomem sde_ioaddr2; struct pinctrl *pinctrl; struct pinctrl_state *pins_default; + struct pinctrl_state *pins_sdex; }; struct brcmstb_match_priv { void (*hs400es)(struct mmc_host *mmc, struct mmc_ios *ios); void (*cfginit)(struct sdhci_host *host); + int (*init_sd_express)(struct mmc_host *mmc, struct mmc_ios *ios); struct sdhci_ops *ops; const unsigned int flags; }; @@ -263,6 +270,105 @@ static void sdhci_brcmstb_cfginit_2712(struct sdhci_host *host) } } +static int bcm2712_init_sd_express(struct mmc_host *mmc, struct mmc_ios *ios) +{ + struct sdhci_host *host = mmc_priv(mmc); + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_brcmstb_priv *brcmstb_priv = sdhci_pltfm_priv(pltfm_host); + struct device *dev = host->mmc->parent; + u32 ctrl_val; + u32 present_state; + int ret; + + if (!brcmstb_priv->sde_ioaddr || !brcmstb_priv->sde_ioaddr2) + return -EINVAL; + + if (!brcmstb_priv->pinctrl) + return -EINVAL; + + /* Turn off the SD clock first */ + sdhci_set_clock(host, 0); + + /* Disable SD DAT0-3 pulls */ + pinctrl_select_state(brcmstb_priv->pinctrl, brcmstb_priv->pins_sdex); + + ctrl_val = readl(brcmstb_priv->sde_ioaddr); + dev_dbg(dev, "ctrl_val 1 %08x\n", ctrl_val); + + /* Tri-state the SD pins */ + ctrl_val |= 0x1ff8; + writel(ctrl_val, brcmstb_priv->sde_ioaddr); + dev_dbg(dev, "ctrl_val 1->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr)); + /* Let voltages settle */ + udelay(100); + + /* Enable the PCIe sideband pins */ + ctrl_val &= ~0x6000; + writel(ctrl_val, brcmstb_priv->sde_ioaddr); + dev_dbg(dev, "ctrl_val 1->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr)); + /* Let voltages settle */ + udelay(100); + + /* Turn on the 1v8 VDD2 regulator */ + ret = regulator_enable(brcmstb_priv->sde_1v8); + if (ret) + return ret; + + /* Wait for Tpvcrl */ + msleep(1); + + /* Sample DAT2 (CLKREQ#) - if low, card is in PCIe mode */ + present_state = sdhci_readl(host, SDHCI_PRESENT_STATE); + present_state = (present_state & SDHCI_DATA_LVL_MASK) >> SDHCI_DATA_LVL_SHIFT; + dev_dbg(dev, "state = 0x%08x\n", present_state); + + if (present_state & BIT(2)) { + dev_err(dev, "DAT2 still high, abandoning SDex switch\n"); + return -ENODEV; + } + + /* Turn on the LCPLL PTEST mux */ + ctrl_val = readl(brcmstb_priv->sde_ioaddr2 + 20); // misc5 + ctrl_val &= ~(0x7 << 7); + ctrl_val |= 3 << 7; + writel(ctrl_val, brcmstb_priv->sde_ioaddr2 + 20); + dev_dbg(dev, "misc 5->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr2 + 20)); + + /* PTEST diff driver enable */ + ctrl_val = readl(brcmstb_priv->sde_ioaddr2); + ctrl_val |= BIT(21); + writel(ctrl_val, brcmstb_priv->sde_ioaddr2); + + dev_dbg(dev, "misc 0->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr2)); + + /* Wait for more than the minimum Tpvpgl time */ + msleep(100); + + if (brcmstb_priv->sde_pcie) { + struct of_changeset changeset; + static struct property okay_property = { + .name = "status", + .value = "okay", + .length = 5, + }; + + /* Enable the pcie controller */ + of_changeset_init(&changeset); + ret = of_changeset_update_property(&changeset, + brcmstb_priv->sde_pcie, + &okay_property); + if (ret) { + dev_err(dev, "%s: failed to update property - %d\n", __func__, + ret); + return -ENODEV; + } + ret = of_changeset_apply(&changeset); + } + + dev_dbg(dev, "%s -> %d\n", __func__, ret); + return ret; +} + static void sdhci_brcmstb_dumpregs(struct mmc_host *mmc) { sdhci_dumpregs(mmc_priv(mmc)); @@ -342,6 +448,7 @@ static struct brcmstb_match_priv match_priv_74165b0 = { static const struct brcmstb_match_priv match_priv_2712 = { .cfginit = sdhci_brcmstb_cfginit_2712, + .init_sd_express = bcm2712_init_sd_express, .ops = &sdhci_brcmstb_ops_2712, }; @@ -423,6 +530,7 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) struct sdhci_brcmstb_priv *priv; u32 actual_clock_mhz; struct sdhci_host *host; + struct resource *iomem; bool no_pinctrl = false; struct clk *clk; struct clk *base_clk = NULL; @@ -456,6 +564,11 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) match_priv->ops->irq = sdhci_brcmstb_cqhci_irq; } + priv->sde_pcie = of_parse_phandle(pdev->dev.of_node, + "sde-pcie", 0); + if (priv->sde_pcie) + priv->flags |= BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS; + /* Map in the non-standard CFG registers */ priv->cfg_regs = devm_platform_get_and_ioremap_resource(pdev, 1, NULL); if (IS_ERR(priv->cfg_regs)) { @@ -468,6 +581,24 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) if (res) goto err; + priv->sde_1v8 = devm_regulator_get_optional(&pdev->dev, "sde-1v8"); + if (IS_ERR(priv->sde_1v8)) + priv->flags &= ~BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS; + + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 2); + if (iomem) { + priv->sde_ioaddr = devm_ioremap_resource(&pdev->dev, iomem); + if (IS_ERR(priv->sde_ioaddr)) + priv->sde_ioaddr = NULL; + } + + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 3); + if (iomem) { + priv->sde_ioaddr2 = devm_ioremap_resource(&pdev->dev, iomem); + if (IS_ERR(priv->sde_ioaddr2)) + priv->sde_ioaddr = NULL; + } + priv->pinctrl = devm_pinctrl_get(&pdev->dev); if (IS_ERR(priv->pinctrl)) { no_pinctrl = true; @@ -478,8 +609,16 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) no_pinctrl = true; } - if (no_pinctrl ) + priv->pins_sdex = pinctrl_lookup_state(priv->pinctrl, "sd-express"); + if (IS_ERR(priv->pins_sdex)) { + dev_dbg(&pdev->dev, "No pinctrl sd-express state\n"); + no_pinctrl = true; + } + + if (no_pinctrl || !priv->sde_ioaddr || !priv->sde_ioaddr2) { priv->pinctrl = NULL; + priv->flags &= ~BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS; + } /* * Automatic clock gating does not work for SD cards that may @@ -497,6 +636,12 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) (host->mmc->caps2 & MMC_CAP2_HS400_ES)) host->mmc_host_ops.hs400_enhanced_strobe = match_priv->hs400es; + if (match_priv->init_sd_express && + (priv->flags & BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS)) { + host->mmc->caps2 |= MMC_CAP2_SD_EXP; + host->mmc_host_ops.init_sd_express = match_priv->init_sd_express; + } + if(match_priv->cfginit) match_priv->cfginit(host);
Broadcom BCM2712 SDHCI controller is SD Express capable. Add support for sde capability where the implementation is based on downstream driver, diverging from it in the way that init_sd_express callback is invoked: in downstream the sdhci_ops structure has been augmented with a new function ptr 'init_sd_express' that just propagate the call to the driver specific callback so that the callstack from a structure standpoint is mmc_host_ops -> sdhci_ops. The drawback here is in the added level of indirection (the newly added init_sd_express is redundant) and the sdhci_ops structure declaration has to be changed. To overcome this the presented approach consist in patching the mmc_host_ops init_sd_express callback to point directly to the custom function defined in this driver (see struct brcmstb_match_priv). Signed-off-by: Andrea della Porta <andrea.porta@suse.com> --- drivers/mmc/host/Kconfig | 1 + drivers/mmc/host/sdhci-brcmstb.c | 147 ++++++++++++++++++++++++++++++- 2 files changed, 147 insertions(+), 1 deletion(-)