| Message ID | 1393586675-14628-3-git-send-email-gdjakov@mm-sol.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 28, 2014 at 3:24 AM, Georgi Djakov <gdjakov@mm-sol.com> wrote: > This platform driver adds the initial support of Secure > Digital Host Controller Interface compliant controller > found in Qualcomm chipsets. > Hi Georgi, Sorry for reposting this, I have no idea how I managed to send this as an answer to patch 1/3... When testing this I was confused by the warnings from sdhci not finding vmmc and vqmmc. Is the power irq something Qualcomm specific or is there any other reason why the sdhci provided regulator functionality can't be used? Regarding the usage of the regulator api here, I think you should call regulator_set_voltage() with your default voltage when you acquire the regulator handles; then your power enable/disable functions will be simpler and you should be able to clean up the power irq function further. > [...] > +/* This structure keeps information per regulator */ > +struct sdhci_msm_reg_data { > + struct regulator *reg; > + const char *name; > + /* Voltage level values */ > + u32 low_vol_level; > + u32 high_vol_level; Is there a reason why these should be different? In your example and the other cases I've seen they are always 2.95V and 1.8V. > [...] > + > +static int sdhci_msm_vreg_enable(struct device *dev, > + struct sdhci_msm_reg_data *vreg) > +{ > + int ret = 0; > + > + if (!regulator_is_enabled(vreg->reg)) { > + /* Set voltage level */ > + ret = regulator_set_voltage(vreg->reg, vreg->high_vol_level, > + vreg->high_vol_level); > + if (ret) > + return ret; So when you enable voltage in the irq handler or in probe, you will go to "high voltage", then you might lower this directly again. > + } > + > + ret = regulator_enable(vreg->reg); > + if (ret) { > + dev_err(dev, "Failed to enable regulator %s (%d)\n", > + vreg->name, ret); > + } > + > + return ret; > +} > + > +static int sdhci_msm_vreg_disable(struct device *dev, > + struct sdhci_msm_reg_data *vreg) > +{ > + int ret = 0; > + > + if (!regulator_is_enabled(vreg->reg)) > + return ret; > + > + /* Set min. voltage to 0 */ > + ret = regulator_set_voltage(vreg->reg, 0, vreg->high_vol_level); > + if (ret) > + return ret; Why do you set the voltage to 0 here? > + > + ret = regulator_disable(vreg->reg); > + if (ret) { > + dev_err(dev, "Failed to disable regulator %s (%d)\n", > + vreg->name, ret); > + } > + > + return ret; > +} > + > +static int sdhci_msm_setup_vreg(struct sdhci_msm_host *msm_host, bool enable) > +{ Instead of having a function with one big if statement of which path you came from you should have two functions for this. > + int ret, i; > + struct sdhci_msm_reg_data *vreg_table[2]; > + > + vreg_table[0] = &msm_host->pdata.vdd; > + vreg_table[1] = &msm_host->pdata.vdd_io; > + > + for (i = 0; i < ARRAY_SIZE(vreg_table); i++) { > + if (enable) > + ret = sdhci_msm_vreg_enable(&msm_host->pdev->dev, > + vreg_table[i]); > + else > + ret = sdhci_msm_vreg_disable(&msm_host->pdev->dev, > + vreg_table[i]); > + if (ret) > + return ret; > + } This seems to a complicated way of saying: if (enable) { sdhci_msm_vreg_enable(vdd) sdhci_msm_vreg_enable(vdd_io) } else { sdhci_msm_vreg_disable(vdd) sdhci_msm_vreg_disable(vdd_io) } Do you plan to add more regulators here? > + > + return 0; > +} > + > +static int sdhci_msm_vreg_init(struct device *dev, > + struct sdhci_msm_pltfm_data *pdata) > +{ > + struct sdhci_msm_reg_data *vdd_reg = &pdata->vdd; > + struct sdhci_msm_reg_data *vdd_io_reg = &pdata->vdd_io; > + > + vdd_reg->reg = devm_regulator_get(dev, vdd_reg->name); > + if (IS_ERR(vdd_reg->reg)) > + return PTR_ERR(vdd_reg->reg); > + > + vdd_io_reg->reg = devm_regulator_get(dev, vdd_io_reg->name); > + if (IS_ERR(vdd_io_reg->reg)) > + return PTR_ERR(vdd_io_reg->reg); > + > + return 0; > +} > + > +static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data) > +{ > + struct sdhci_host *host = (struct sdhci_host *)data; > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_msm_host *msm_host = pltfm_host->priv; > + u8 irq_status; > + u8 irq_ack = 0; > + int ret = 0; > + > + irq_status = readb_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS); > + dev_dbg(mmc_dev(msm_host->mmc), "%s: Received IRQ(%d), status=0x%x\n", > + mmc_hostname(msm_host->mmc), irq, irq_status); > + > + /* Clear the interrupt */ > + writeb_relaxed(irq_status, (msm_host->core_mem + CORE_PWRCTL_CLEAR)); > + /* > + * SDHC has core_mem and hc_mem device memory and these memory > + * addresses do not fall within 1KB region. Hence, any update to > + * core_mem address space would require an mb() to ensure this gets > + * completed before its next update to registers within hc_mem. > + */ This is the standard Qualcomm disclaimer regarding memory barriers, but what part of the system does actually touch hc_mem? As far as I can see this driver does not go outside that 1K and if the core sdhci core does, it seems to all be using the non-relaxed write*; so there would be an implicit sync there. Is it so that we make sure to clear the interrupt here and now? > + mb(); > + > + /* Handle BUS ON/OFF */ > + if (irq_status & CORE_PWRCTL_BUS_ON) { > + ret = sdhci_msm_setup_vreg(msm_host, true); > + if (!ret) > + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, > + msm_host->pdata. > + vdd_io.high_vol_level, > + msm_host->pdata. > + vdd_io.high_vol_level); If sdhci_msm_setup_vreg succeeds, you've already set a voltage to vdd_io and enabled it, why do this one more time? > + if (ret) > + irq_ack |= CORE_PWRCTL_BUS_FAIL; > + else > + irq_ack |= CORE_PWRCTL_BUS_SUCCESS; > + } > + > + if (irq_status & CORE_PWRCTL_BUS_OFF) { > + ret = sdhci_msm_setup_vreg(msm_host, false); > + if (!ret) > + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, > + msm_host->pdata. > + vdd_io.low_vol_level, > + msm_host->pdata. > + vdd_io.low_vol_level); Same here. > + if (ret) > + irq_ack |= CORE_PWRCTL_BUS_FAIL; > + else > + irq_ack |= CORE_PWRCTL_BUS_SUCCESS; > + } > + > + /* Handle IO LOW/HIGH */ > + if (irq_status & CORE_PWRCTL_IO_LOW) { > + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, > + msm_host->pdata. > + vdd_io.low_vol_level, > + msm_host->pdata. > + vdd_io.low_vol_level); I assume that LOW is xor HIGH here, or you sould set it low then high. May I suggest that you restructure this to first figuring out what new voltage (if any) you're aiming for and then call regulator_set_voltage(vdd_io) once and based on that update the IO_{SUCCESS,FAIL} bits of irq_ack. > + if (ret) > + irq_ack |= CORE_PWRCTL_IO_FAIL; > + else > + irq_ack |= CORE_PWRCTL_IO_SUCCESS; > + } > + > + if (irq_status & CORE_PWRCTL_IO_HIGH) { > + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, > + msm_host->pdata. > + vdd_io.high_vol_level, > + msm_host->pdata. > + vdd_io.high_vol_level); > + if (ret) > + irq_ack |= CORE_PWRCTL_IO_FAIL; > + else > + irq_ack |= CORE_PWRCTL_IO_SUCCESS; > + } > + > + /* ACK status to the core */ > + writeb_relaxed(irq_ack, (msm_host->core_mem + CORE_PWRCTL_CTL)); > + /* > + * SDHC has core_mem and hc_mem device memory and these memory > + * addresses do not fall within 1KB region. Hence, any update to > + * core_mem address space would require an mb() to ensure this gets > + * completed before its next update to registers within hc_mem. > + */ Like above, is this mb() to guard for re-ordering or to commit the write? > + mb(); > + > + dev_dbg(mmc_dev(msm_host->mmc), "%s: Handled IRQ(%d), ret=%d, ack=0x%x\n", > + mmc_hostname(msm_host->mmc), irq, ret, irq_ack); > + return IRQ_HANDLED; > +} > + > +static const struct of_device_id sdhci_msm_dt_match[] = { > + { .compatible = "qcom,sdhci-msm-v4" }, > + {}, > +}; > + > +MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match); > + > +static struct sdhci_ops sdhci_msm_ops = { > + .platform_execute_tuning = sdhci_msm_execute_tuning, > +}; > + > +static int sdhci_msm_probe(struct platform_device *pdev) > +{ > + struct sdhci_host *host; > + struct sdhci_pltfm_host *pltfm_host; > + struct sdhci_msm_host *msm_host; > + struct resource *core_memres = NULL; No need to initialize, as first reference is an assignment. > + int ret, dead; > + u16 host_version; > + > + if (!pdev->dev.of_node) { > + dev_err(&pdev->dev, "No device tree data\n"); > + return -ENOENT; > + } > + > + msm_host = devm_kzalloc(&pdev->dev, sizeof(*msm_host), GFP_KERNEL); > + if (!msm_host) > + return -ENOMEM; > + > + msm_host->sdhci_msm_pdata.ops = &sdhci_msm_ops; > + host = sdhci_pltfm_init(pdev, &msm_host->sdhci_msm_pdata, 0); > + if (IS_ERR(host)) > + return PTR_ERR(host); > + > + pltfm_host = sdhci_priv(host); > + pltfm_host->priv = msm_host; > + msm_host->mmc = host->mmc; > + msm_host->pdev = pdev; > + > + ret = mmc_of_parse(host->mmc); > + if (ret) { > + dev_err(&pdev->dev, "Failed parsing mmc device tree\n"); > + goto pltfm_free; > + } > + > + sdhci_get_of_property(pdev); > + > + ret = sdhci_msm_populate_pdata(&pdev->dev, &msm_host->pdata); > + if (ret) { > + dev_err(&pdev->dev, "DT parsing error\n"); > + goto pltfm_free; > + } > + > + /* Setup SDCC bus voter clock. */ > + msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus"); > + if (!IS_ERR(msm_host->bus_clk)) { > + /* Vote for max. clk rate for max. performance */ > + ret = clk_set_rate(msm_host->bus_clk, INT_MAX); > + if (ret) > + goto pltfm_free; > + ret = clk_prepare_enable(msm_host->bus_clk); > + if (ret) > + goto pltfm_free; > + } > + > + /* Setup main peripheral bus clock */ > + msm_host->pclk = devm_clk_get(&pdev->dev, "iface"); > + if (!IS_ERR(msm_host->pclk)) { iface clock is marked required in the binding documentation, so you probably don't want to fall through here on error. > + ret = clk_prepare_enable(msm_host->pclk); > + if (ret) { > + dev_err(&pdev->dev, > + "Peripheral clock setup failed (%d)\n", ret); > + goto bus_clk_disable; > + } > + } > + > + /* Setup SDC MMC clock */ > + msm_host->clk = devm_clk_get(&pdev->dev, "core"); > + if (IS_ERR(msm_host->clk)) { > + ret = PTR_ERR(msm_host->clk); > + dev_err(&pdev->dev, "SDC MMC clock setup failed (%d)\n", ret); > + goto pclk_disable; > + } > + > + ret = clk_prepare_enable(msm_host->clk); > + if (ret) > + goto pclk_disable; > + > + /* Setup regulators */ > + ret = sdhci_msm_vreg_init(&pdev->dev, &msm_host->pdata); > + if (ret) { > + if (ret != -EPROBE_DEFER) > + dev_err(&pdev->dev, > + "Regulator setup failed (%d)\n", ret); > + goto clk_disable; > + } > + > + core_memres = platform_get_resource_byname(pdev, > + IORESOURCE_MEM, "core_mem"); > + msm_host->core_mem = devm_ioremap_resource(&pdev->dev, core_memres); > + > + if (IS_ERR(msm_host->core_mem)) { > + dev_err(&pdev->dev, "Failed to remap registers\n"); > + ret = PTR_ERR(msm_host->core_mem); > + goto vreg_disable; > + } > + > + /* Reset the core and Enable SDHC mode */ > + writel_relaxed(readl_relaxed(msm_host->core_mem + CORE_POWER) | > + CORE_SW_RST, msm_host->core_mem + CORE_POWER); > + > + /* SW reset can take upto 10HCLK + 15MCLK cycles. (min 40us) */ > + usleep_range(1000, 5000); > + if (readl(msm_host->core_mem + CORE_POWER) & CORE_SW_RST) { > + dev_err(&pdev->dev, "Stuck in reset\n"); > + ret = -ETIMEDOUT; > + goto vreg_disable; At this point you have only acquired a handle to the vregs, you have not enabled them. So you don't need to disable them. > + } > + > + /* Set HC_MODE_EN bit in HC_MODE register */ > + writel_relaxed(HC_MODE_EN, (msm_host->core_mem + CORE_HC_MODE)); > + > + /* > + * Following are the deviations from SDHC spec v3.0 - > + * 1. Card detection is handled using separate GPIO. > + * 2. Bus power control is handled by interacting with PMIC. > + */ > + host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION; > + host->quirks |= SDHCI_QUIRK_SINGLE_POWER_WRITE; > + > + host_version = readw_relaxed((host->ioaddr + SDHCI_HOST_VERSION)); > + dev_dbg(&pdev->dev, "Host Version: 0x%x Vendor Version 0x%x\n", > + host_version, ((host_version & SDHCI_VENDOR_VER_MASK) >> > + SDHCI_VENDOR_VER_SHIFT)); > + > + /* Setup PWRCTL irq */ > + msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq"); > + if (msm_host->pwr_irq < 0) { > + dev_err(&pdev->dev, "Failed to get pwr_irq by name (%d)\n", > + msm_host->pwr_irq); > + goto vreg_disable; At this point you have only acquired a handle to the vregs, you have not enabled them. So you don't need to disable them. > + } > + ret = devm_request_threaded_irq(&pdev->dev, msm_host->pwr_irq, NULL, > + sdhci_msm_pwr_irq, IRQF_ONESHOT, > + dev_name(&pdev->dev), host); > + if (ret) { > + dev_err(&pdev->dev, "Request threaded irq(%d) failed (%d)\n", > + msm_host->pwr_irq, ret); > + goto vreg_disable; If this fails you haven't enabled the regulators, so no need to disable them again. > + } > + > + /* Enable pwr irq interrupts */ > + writel_relaxed(INT_MASK, (msm_host->core_mem + CORE_PWRCTL_MASK)); > + > + msm_host->mmc->caps |= msm_host->pdata.caps; > + msm_host->mmc->caps2 |= msm_host->pdata.caps2; > + > + ret = sdhci_add_host(host); > + if (ret) { > + dev_err(&pdev->dev, "Add host failed (%d)\n", ret); > + goto vreg_disable; > + } > + > + ret = clk_set_rate(msm_host->clk, host->max_clk); > + if (ret) { > + dev_err(&pdev->dev, "MClk rate set failed (%d)\n", ret); > + goto remove_host; > + } Why do you enable the clk further up in this function but wait with setting the rate until the last thing in this function? > + > + return 0; > + > +remove_host: > + dead = (readl_relaxed(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff); > + sdhci_remove_host(host, dead); > +vreg_disable: > + if (!IS_ERR(msm_host->pdata.vdd.reg)) If IS_ERR(vdd) or IS_ERR(vdd_io) then you would end up in clk_disable. > + sdhci_msm_vreg_disable(&pdev->dev, &msm_host->pdata.vdd); > + if (!IS_ERR(msm_host->pdata.vdd_io.reg)) > + sdhci_msm_vreg_disable(&pdev->dev, &msm_host->pdata.vdd_io); > +clk_disable: > + if (!IS_ERR(msm_host->clk)) If IS_ERR(clk) then you would end up in pclk_disable. > + clk_disable_unprepare(msm_host->clk); > +pclk_disable: > + if (!IS_ERR(msm_host->pclk)) Based on the assumption that the check for errors on pclk above is incorrect, then you would end up in bus_clk_disable if IS_ERR(pclk). > + clk_disable_unprepare(msm_host->pclk); > +bus_clk_disable: > + if (!IS_ERR(msm_host->bus_clk)) bus_clk might be IS_ERR() as it's optional, so this makes sense. > + clk_disable_unprepare(msm_host->bus_clk); > +pltfm_free: > + sdhci_pltfm_free(pdev); > + return ret; > +} > + > +static int sdhci_msm_remove(struct platform_device *pdev) > +{ > + struct sdhci_host *host = platform_get_drvdata(pdev); > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_msm_host *msm_host = pltfm_host->priv; > + int dead = (readl_relaxed(host->ioaddr + SDHCI_INT_STATUS) == > + 0xffffffff); > + You should probably start with disabling the pwr_irq here, to make sure that it doesn't kick in after you starting to free resources. > + sdhci_remove_host(host, dead); > + sdhci_pltfm_free(pdev); > + sdhci_msm_vreg_disable(&pdev->dev, &msm_host->pdata.vdd); > + sdhci_msm_vreg_disable(&pdev->dev, &msm_host->pdata.vdd_io); > + clk_disable_unprepare(msm_host->clk); > + clk_disable_unprepare(msm_host->pclk); > + if (!IS_ERR(msm_host->bus_clk)) > + clk_disable_unprepare(msm_host->bus_clk); > + return 0; > +} > + Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/04/2014 05:15 AM, Bjorn Andersson wrote: > On Fri, Feb 28, 2014 at 3:24 AM, Georgi Djakov <gdjakov@mm-sol.com> wrote: >> This platform driver adds the initial support of Secure >> Digital Host Controller Interface compliant controller >> found in Qualcomm chipsets. >> > > Hi Georgi, > > Sorry for reposting this, I have no idea how I managed to send this as an answer > to patch 1/3... > > > When testing this I was confused by the warnings from sdhci not finding vmmc > and vqmmc. Is the power irq something Qualcomm specific or is there any other > reason why the sdhci provided regulator functionality can't be used? > > Regarding the usage of the regulator api here, I think you should call > regulator_set_voltage() with your default voltage when you acquire the > regulator handles; then your power enable/disable functions will be simpler and > you should be able to clean up the power irq function further. > Hi Bjorn, Yes it is Qualcomm specific - voltage control is done not via the standard SDHCI control registers. Writing to the registers will trigger a separate IRQ and the handler configures the PMIC voltages. >> > [...] >> +/* This structure keeps information per regulator */ >> +struct sdhci_msm_reg_data { >> + struct regulator *reg; >> + const char *name; >> + /* Voltage level values */ >> + u32 low_vol_level; >> + u32 high_vol_level; > > Is there a reason why these should be different? In your example and the other > cases I've seen they are always 2.95V and 1.8V. > The host can support also 1.2V for DDR modes. Now I'll do it with 2.95/1.8V as you suggest and later i can expand it with optional properties like mmc-hs200-1_2v or mmc-highspeed-ddr-1_2v. >> > [...] >> + >> +static int sdhci_msm_vreg_enable(struct device *dev, >> + struct sdhci_msm_reg_data *vreg) >> +{ >> + int ret = 0; >> + >> + if (!regulator_is_enabled(vreg->reg)) { >> + /* Set voltage level */ >> + ret = regulator_set_voltage(vreg->reg, vreg->high_vol_level, >> + vreg->high_vol_level); >> + if (ret) >> + return ret; > > So when you enable voltage in the irq handler or in probe, you will go to "high > voltage", then you might lower this directly again. > Yes, but I will clean-up the irq handler. >> + } >> + >> + ret = regulator_enable(vreg->reg); >> + if (ret) { >> + dev_err(dev, "Failed to enable regulator %s (%d)\n", >> + vreg->name, ret); >> + } >> + >> + return ret; >> +} >> + >> +static int sdhci_msm_vreg_disable(struct device *dev, >> + struct sdhci_msm_reg_data *vreg) >> +{ >> + int ret = 0; >> + >> + if (!regulator_is_enabled(vreg->reg)) >> + return ret; >> + >> + /* Set min. voltage to 0 */ >> + ret = regulator_set_voltage(vreg->reg, 0, vreg->high_vol_level); >> + if (ret) >> + return ret; > > Why do you set the voltage to 0 here? > The regulators can be shared with other peripherals, so when we are not using them, we vote for 0 as minimum acceptable voltage. >> + >> + ret = regulator_disable(vreg->reg); >> + if (ret) { >> + dev_err(dev, "Failed to disable regulator %s (%d)\n", >> + vreg->name, ret); >> + } >> + >> + return ret; >> +} >> + >> +static int sdhci_msm_setup_vreg(struct sdhci_msm_host *msm_host, bool enable) >> +{ > > Instead of having a function with one big if statement of which path you came > from you should have two functions for this. > Oh sure! Will fix! Thanks! >> + int ret, i; >> + struct sdhci_msm_reg_data *vreg_table[2]; >> + >> + vreg_table[0] = &msm_host->pdata.vdd; >> + vreg_table[1] = &msm_host->pdata.vdd_io; >> + >> + for (i = 0; i < ARRAY_SIZE(vreg_table); i++) { >> + if (enable) >> + ret = sdhci_msm_vreg_enable(&msm_host->pdev->dev, >> + vreg_table[i]); >> + else >> + ret = sdhci_msm_vreg_disable(&msm_host->pdev->dev, >> + vreg_table[i]); >> + if (ret) >> + return ret; >> + } > > This seems to a complicated way of saying: > > if (enable) { > sdhci_msm_vreg_enable(vdd) > sdhci_msm_vreg_enable(vdd_io) > } else { > sdhci_msm_vreg_disable(vdd) > sdhci_msm_vreg_disable(vdd_io) > } > > Do you plan to add more regulators here? > No. >> + >> + return 0; >> +} >> + >> +static int sdhci_msm_vreg_init(struct device *dev, >> + struct sdhci_msm_pltfm_data *pdata) >> +{ >> + struct sdhci_msm_reg_data *vdd_reg = &pdata->vdd; >> + struct sdhci_msm_reg_data *vdd_io_reg = &pdata->vdd_io; >> + >> + vdd_reg->reg = devm_regulator_get(dev, vdd_reg->name); >> + if (IS_ERR(vdd_reg->reg)) >> + return PTR_ERR(vdd_reg->reg); >> + >> + vdd_io_reg->reg = devm_regulator_get(dev, vdd_io_reg->name); >> + if (IS_ERR(vdd_io_reg->reg)) >> + return PTR_ERR(vdd_io_reg->reg); >> + >> + return 0; >> +} >> + >> +static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data) >> +{ >> + struct sdhci_host *host = (struct sdhci_host *)data; >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> + struct sdhci_msm_host *msm_host = pltfm_host->priv; >> + u8 irq_status; >> + u8 irq_ack = 0; >> + int ret = 0; >> + >> + irq_status = readb_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS); >> + dev_dbg(mmc_dev(msm_host->mmc), "%s: Received IRQ(%d), status=0x%x\n", >> + mmc_hostname(msm_host->mmc), irq, irq_status); >> + >> + /* Clear the interrupt */ >> + writeb_relaxed(irq_status, (msm_host->core_mem + CORE_PWRCTL_CLEAR)); >> + /* >> + * SDHC has core_mem and hc_mem device memory and these memory >> + * addresses do not fall within 1KB region. Hence, any update to >> + * core_mem address space would require an mb() to ensure this gets >> + * completed before its next update to registers within hc_mem. >> + */ > > This is the standard Qualcomm disclaimer regarding memory barriers, but what > part of the system does actually touch hc_mem? As far as I can see this driver > does not go outside that 1K and if the core sdhci core does, it seems to all be > using the non-relaxed write*; so there would be an implicit sync there. > > Is it so that we make sure to clear the interrupt here and now? > Perhaps the comment is not entirely correct. We must make sure that the interrupt is cleared first and code is not reordered. >> + mb(); >> + >> + /* Handle BUS ON/OFF */ >> + if (irq_status & CORE_PWRCTL_BUS_ON) { >> + ret = sdhci_msm_setup_vreg(msm_host, true); >> + if (!ret) >> + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, >> + msm_host->pdata. >> + vdd_io.high_vol_level, >> + msm_host->pdata. >> + vdd_io.high_vol_level); > > If sdhci_msm_setup_vreg succeeds, you've already set a voltage to vdd_io and > enabled it, why do this one more time? > Oops! Thanks! >> + if (ret) >> + irq_ack |= CORE_PWRCTL_BUS_FAIL; >> + else >> + irq_ack |= CORE_PWRCTL_BUS_SUCCESS; >> + } >> + >> + if (irq_status & CORE_PWRCTL_BUS_OFF) { >> + ret = sdhci_msm_setup_vreg(msm_host, false); >> + if (!ret) >> + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, >> + msm_host->pdata. >> + vdd_io.low_vol_level, >> + msm_host->pdata. >> + vdd_io.low_vol_level); > > Same here. > Thanks! >> + if (ret) >> + irq_ack |= CORE_PWRCTL_BUS_FAIL; >> + else >> + irq_ack |= CORE_PWRCTL_BUS_SUCCESS; >> + } >> + >> + /* Handle IO LOW/HIGH */ >> + if (irq_status & CORE_PWRCTL_IO_LOW) { >> + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, >> + msm_host->pdata. >> + vdd_io.low_vol_level, >> + msm_host->pdata. >> + vdd_io.low_vol_level); > > I assume that LOW is xor HIGH here, or you sould set it low then high. > > May I suggest that you restructure this to first figuring out what new voltage > (if any) you're aiming for and then call regulator_set_voltage(vdd_io) once and > based on that update the IO_{SUCCESS,FAIL} bits of irq_ack. > Ok. I'll restructure it. Thanks! >> + if (ret) >> + irq_ack |= CORE_PWRCTL_IO_FAIL; >> + else >> + irq_ack |= CORE_PWRCTL_IO_SUCCESS; >> + } >> + >> + if (irq_status & CORE_PWRCTL_IO_HIGH) { >> + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, >> + msm_host->pdata. >> + vdd_io.high_vol_level, >> + msm_host->pdata. >> + vdd_io.high_vol_level); >> + if (ret) >> + irq_ack |= CORE_PWRCTL_IO_FAIL; >> + else >> + irq_ack |= CORE_PWRCTL_IO_SUCCESS; >> + } >> + >> + /* ACK status to the core */ >> + writeb_relaxed(irq_ack, (msm_host->core_mem + CORE_PWRCTL_CTL)); >> + /* >> + * SDHC has core_mem and hc_mem device memory and these memory >> + * addresses do not fall within 1KB region. Hence, any update to >> + * core_mem address space would require an mb() to ensure this gets >> + * completed before its next update to registers within hc_mem. >> + */ > > Like above, is this mb() to guard for re-ordering or to commit the write? > Here we want to commit the write. >> + mb(); >> + >> + dev_dbg(mmc_dev(msm_host->mmc), "%s: Handled IRQ(%d), ret=%d, ack=0x%x\n", >> + mmc_hostname(msm_host->mmc), irq, ret, irq_ack); >> + return IRQ_HANDLED; >> +} >> + >> +static const struct of_device_id sdhci_msm_dt_match[] = { >> + { .compatible = "qcom,sdhci-msm-v4" }, >> + {}, >> +}; >> + >> +MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match); >> + >> +static struct sdhci_ops sdhci_msm_ops = { >> + .platform_execute_tuning = sdhci_msm_execute_tuning, >> +}; >> + >> +static int sdhci_msm_probe(struct platform_device *pdev) >> +{ >> + struct sdhci_host *host; >> + struct sdhci_pltfm_host *pltfm_host; >> + struct sdhci_msm_host *msm_host; >> + struct resource *core_memres = NULL; > > No need to initialize, as first reference is an assignment. > Agree! Thank you! >> + int ret, dead; >> + u16 host_version; >> + >> + if (!pdev->dev.of_node) { >> + dev_err(&pdev->dev, "No device tree data\n"); >> + return -ENOENT; >> + } >> + >> + msm_host = devm_kzalloc(&pdev->dev, sizeof(*msm_host), GFP_KERNEL); >> + if (!msm_host) >> + return -ENOMEM; >> + >> + msm_host->sdhci_msm_pdata.ops = &sdhci_msm_ops; >> + host = sdhci_pltfm_init(pdev, &msm_host->sdhci_msm_pdata, 0); >> + if (IS_ERR(host)) >> + return PTR_ERR(host); >> + >> + pltfm_host = sdhci_priv(host); >> + pltfm_host->priv = msm_host; >> + msm_host->mmc = host->mmc; >> + msm_host->pdev = pdev; >> + >> + ret = mmc_of_parse(host->mmc); >> + if (ret) { >> + dev_err(&pdev->dev, "Failed parsing mmc device tree\n"); >> + goto pltfm_free; >> + } >> + >> + sdhci_get_of_property(pdev); >> + >> + ret = sdhci_msm_populate_pdata(&pdev->dev, &msm_host->pdata); >> + if (ret) { >> + dev_err(&pdev->dev, "DT parsing error\n"); >> + goto pltfm_free; >> + } >> + >> + /* Setup SDCC bus voter clock. */ >> + msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus"); >> + if (!IS_ERR(msm_host->bus_clk)) { >> + /* Vote for max. clk rate for max. performance */ >> + ret = clk_set_rate(msm_host->bus_clk, INT_MAX); >> + if (ret) >> + goto pltfm_free; >> + ret = clk_prepare_enable(msm_host->bus_clk); >> + if (ret) >> + goto pltfm_free; >> + } >> + >> + /* Setup main peripheral bus clock */ >> + msm_host->pclk = devm_clk_get(&pdev->dev, "iface"); >> + if (!IS_ERR(msm_host->pclk)) { > > iface clock is marked required in the binding documentation, so you probably > don't want to fall through here on error. > I'll fix it. This code is from a few months ago when there was no GCC support and i was using some power-on default clocks for testing. Thanks! >> + ret = clk_prepare_enable(msm_host->pclk); >> + if (ret) { >> + dev_err(&pdev->dev, >> + "Peripheral clock setup failed (%d)\n", ret); >> + goto bus_clk_disable; >> + } >> + } >> + >> + /* Setup SDC MMC clock */ >> + msm_host->clk = devm_clk_get(&pdev->dev, "core"); >> + if (IS_ERR(msm_host->clk)) { >> + ret = PTR_ERR(msm_host->clk); >> + dev_err(&pdev->dev, "SDC MMC clock setup failed (%d)\n", ret); >> + goto pclk_disable; >> + } >> + >> + ret = clk_prepare_enable(msm_host->clk); >> + if (ret) >> + goto pclk_disable; >> + >> + /* Setup regulators */ >> + ret = sdhci_msm_vreg_init(&pdev->dev, &msm_host->pdata); >> + if (ret) { >> + if (ret != -EPROBE_DEFER) >> + dev_err(&pdev->dev, >> + "Regulator setup failed (%d)\n", ret); >> + goto clk_disable; >> + } >> + >> + core_memres = platform_get_resource_byname(pdev, >> + IORESOURCE_MEM, "core_mem"); >> + msm_host->core_mem = devm_ioremap_resource(&pdev->dev, core_memres); >> + >> + if (IS_ERR(msm_host->core_mem)) { >> + dev_err(&pdev->dev, "Failed to remap registers\n"); >> + ret = PTR_ERR(msm_host->core_mem); >> + goto vreg_disable; >> + } >> + >> + /* Reset the core and Enable SDHC mode */ >> + writel_relaxed(readl_relaxed(msm_host->core_mem + CORE_POWER) | >> + CORE_SW_RST, msm_host->core_mem + CORE_POWER); >> + >> + /* SW reset can take upto 10HCLK + 15MCLK cycles. (min 40us) */ >> + usleep_range(1000, 5000); >> + if (readl(msm_host->core_mem + CORE_POWER) & CORE_SW_RST) { >> + dev_err(&pdev->dev, "Stuck in reset\n"); >> + ret = -ETIMEDOUT; >> + goto vreg_disable; > > At this point you have only acquired a handle to the vregs, you have not > enabled them. So you don't need to disable them. Agree! Thanks! > >> + } >> + >> + /* Set HC_MODE_EN bit in HC_MODE register */ >> + writel_relaxed(HC_MODE_EN, (msm_host->core_mem + CORE_HC_MODE)); >> + >> + /* >> + * Following are the deviations from SDHC spec v3.0 - >> + * 1. Card detection is handled using separate GPIO. >> + * 2. Bus power control is handled by interacting with PMIC. >> + */ >> + host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION; >> + host->quirks |= SDHCI_QUIRK_SINGLE_POWER_WRITE; >> + >> + host_version = readw_relaxed((host->ioaddr + SDHCI_HOST_VERSION)); >> + dev_dbg(&pdev->dev, "Host Version: 0x%x Vendor Version 0x%x\n", >> + host_version, ((host_version & SDHCI_VENDOR_VER_MASK) >> >> + SDHCI_VENDOR_VER_SHIFT)); >> + >> + /* Setup PWRCTL irq */ >> + msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq"); >> + if (msm_host->pwr_irq < 0) { >> + dev_err(&pdev->dev, "Failed to get pwr_irq by name (%d)\n", >> + msm_host->pwr_irq); >> + goto vreg_disable; > > At this point you have only acquired a handle to the vregs, you have not > enabled them. So you don't need to disable them. Thanks! > >> + } >> + ret = devm_request_threaded_irq(&pdev->dev, msm_host->pwr_irq, NULL, >> + sdhci_msm_pwr_irq, IRQF_ONESHOT, >> + dev_name(&pdev->dev), host); >> + if (ret) { >> + dev_err(&pdev->dev, "Request threaded irq(%d) failed (%d)\n", >> + msm_host->pwr_irq, ret); >> + goto vreg_disable; > > If this fails you haven't enabled the regulators, so no need to disable them > again. > Thanks! >> + } >> + >> + /* Enable pwr irq interrupts */ >> + writel_relaxed(INT_MASK, (msm_host->core_mem + CORE_PWRCTL_MASK)); >> + >> + msm_host->mmc->caps |= msm_host->pdata.caps; >> + msm_host->mmc->caps2 |= msm_host->pdata.caps2; >> + >> + ret = sdhci_add_host(host); >> + if (ret) { >> + dev_err(&pdev->dev, "Add host failed (%d)\n", ret); >> + goto vreg_disable; >> + } >> + >> + ret = clk_set_rate(msm_host->clk, host->max_clk); >> + if (ret) { >> + dev_err(&pdev->dev, "MClk rate set failed (%d)\n", ret); >> + goto remove_host; >> + } > > Why do you enable the clk further up in this function but wait with setting the > rate until the last thing in this function? > This serves mostly as a workaround as the hardware requires a custom set_clock() implementation and the clocks will fail in sdhci_add_host(). I prefer to keep it this way until i submit a patch that implements the clock control - clock scaling and clock gating. >> + >> + return 0; >> + >> +remove_host: >> + dead = (readl_relaxed(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff); >> + sdhci_remove_host(host, dead); >> +vreg_disable: >> + if (!IS_ERR(msm_host->pdata.vdd.reg)) > > If IS_ERR(vdd) or IS_ERR(vdd_io) then you would end up in clk_disable. > >> + sdhci_msm_vreg_disable(&pdev->dev, &msm_host->pdata.vdd); >> + if (!IS_ERR(msm_host->pdata.vdd_io.reg)) >> + sdhci_msm_vreg_disable(&pdev->dev, &msm_host->pdata.vdd_io); >> +clk_disable: >> + if (!IS_ERR(msm_host->clk)) > > If IS_ERR(clk) then you would end up in pclk_disable. > >> + clk_disable_unprepare(msm_host->clk); >> +pclk_disable: >> + if (!IS_ERR(msm_host->pclk)) > > Based on the assumption that the check for errors on pclk above is incorrect, > then you would end up in bus_clk_disable if IS_ERR(pclk). > Yes, de-init in reverse order and free resources. It seems that i can drop the IS_ERR. >> + clk_disable_unprepare(msm_host->pclk); >> +bus_clk_disable: >> + if (!IS_ERR(msm_host->bus_clk)) > > bus_clk might be IS_ERR() as it's optional, so this makes sense. > >> + clk_disable_unprepare(msm_host->bus_clk); >> +pltfm_free: >> + sdhci_pltfm_free(pdev); >> + return ret; >> +} >> + >> +static int sdhci_msm_remove(struct platform_device *pdev) >> +{ >> + struct sdhci_host *host = platform_get_drvdata(pdev); >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> + struct sdhci_msm_host *msm_host = pltfm_host->priv; >> + int dead = (readl_relaxed(host->ioaddr + SDHCI_INT_STATUS) == >> + 0xffffffff); >> + > > You should probably start with disabling the pwr_irq here, to make sure that it > doesn't kick in after you starting to free resources. > Oops, i will fix it. Thanks for the detailed review and all the comments! BR, Georgi -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index 82cc34d..66ef8b9 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -334,6 +334,19 @@ config MMC_ATMELMCI If unsure, say N. +config MMC_SDHCI_MSM + tristate "Qualcomm SDHCI Controller Support" + depends on ARCH_QCOM + depends on MMC_SDHCI_PLTFM + help + This selects the Secure Digital Host Controller Interface (SDHCI) + support present in Qualcomm SOCs. The controller supports + SD/MMC/SDIO devices. + + If you have a controller with this interface, say Y or M here. + + If unsure, say N. + config MMC_MSM tristate "Qualcomm SDCC Controller Support" depends on MMC && (ARCH_MSM7X00A || ARCH_MSM7X30 || ARCH_QSD8X50) diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile index f162f87a0..0c8aa5e 100644 --- a/drivers/mmc/host/Makefile +++ b/drivers/mmc/host/Makefile @@ -63,6 +63,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o obj-$(CONFIG_MMC_SDHCI_BCM_KONA) += sdhci-bcm-kona.o obj-$(CONFIG_MMC_SDHCI_BCM2835) += sdhci-bcm2835.o +obj-$(CONFIG_MMC_SDHCI_MSM) += sdhci-msm.o ifeq ($(CONFIG_CB710_DEBUG),y) CFLAGS-cb710-mmc += -DDEBUG diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c new file mode 100644 index 0000000..b4490a2 --- /dev/null +++ b/drivers/mmc/host/sdhci-msm.c @@ -0,0 +1,528 @@ +/* + * drivers/mmc/host/sdhci-msm.c - Qualcomm SDHCI Platform driver + * + * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/regulator/consumer.h> +#include <linux/delay.h> + +#include "sdhci-pltfm.h" + +#define CORE_HC_MODE 0x78 +#define HC_MODE_EN 0x1 + +#define CORE_POWER 0x0 +#define CORE_SW_RST BIT(7) + +#define CORE_PWRCTL_STATUS 0xdc +#define CORE_PWRCTL_MASK 0xe0 +#define CORE_PWRCTL_CLEAR 0xe4 +#define CORE_PWRCTL_CTL 0xe8 + +#define CORE_PWRCTL_BUS_OFF BIT(0) +#define CORE_PWRCTL_BUS_ON BIT(1) +#define CORE_PWRCTL_IO_LOW BIT(2) +#define CORE_PWRCTL_IO_HIGH BIT(3) + +#define CORE_PWRCTL_BUS_SUCCESS BIT(0) +#define CORE_PWRCTL_BUS_FAIL BIT(1) +#define CORE_PWRCTL_IO_SUCCESS BIT(2) +#define CORE_PWRCTL_IO_FAIL BIT(3) + +#define INT_MASK 0xf + + +/* This structure keeps information per regulator */ +struct sdhci_msm_reg_data { + struct regulator *reg; + const char *name; + /* Voltage level values */ + u32 low_vol_level; + u32 high_vol_level; +}; + +struct sdhci_msm_pltfm_data { + u32 caps; /* Supported UHS-I Modes */ + u32 caps2; /* More capabilities */ + struct sdhci_msm_reg_data vdd; /* VDD/VCC regulator info */ + struct sdhci_msm_reg_data vdd_io; /* VDD IO regulator info */ +}; + +struct sdhci_msm_host { + struct platform_device *pdev; + void __iomem *core_mem; /* MSM SDCC mapped address */ + int pwr_irq; /* power irq */ + struct clk *clk; /* main SD/MMC bus clock */ + struct clk *pclk; /* SDHC peripheral bus clock */ + struct clk *bus_clk; /* SDHC bus voter clock */ + struct sdhci_msm_pltfm_data pdata; + struct mmc_host *mmc; + struct sdhci_pltfm_data sdhci_msm_pdata; +}; + +/* MSM platform specific tuning */ +int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode) +{ + /* + * Tuning is required for SDR104, HS200 and HS400 cards and if the clock + * frequency greater than 100MHz in those modes. The standard tuning + * procedure should not be executed, but a custom implementation will be + * added here instead. + */ + return 0; +} + +#define MAX_PROP_SIZE 32 +static int sdhci_msm_dt_parse_vreg_info(struct device *dev, + struct sdhci_msm_reg_data *vreg, const char *vreg_name) +{ + char prop_name[MAX_PROP_SIZE]; + struct device_node *np = dev->of_node; + + vreg->name = vreg_name; + + snprintf(prop_name, MAX_PROP_SIZE, "qcom,%s-voltage-min", vreg_name); + of_property_read_u32(np, prop_name, &vreg->low_vol_level); + snprintf(prop_name, MAX_PROP_SIZE, "qcom,%s-voltage-max", vreg_name); + of_property_read_u32(np, prop_name, &vreg->high_vol_level); + + /* Sanity check */ + if (vreg->low_vol_level > vreg->high_vol_level) { + dev_err(dev, "%s Invalid constraints specified\n", vreg->name); + return -EINVAL; + } + + return 0; +} + +/* Parse devicetree data */ +static int sdhci_msm_populate_pdata(struct device *dev, + struct sdhci_msm_pltfm_data *pdata) +{ + if (sdhci_msm_dt_parse_vreg_info(dev, &pdata->vdd, "vdd")) { + dev_err(dev, "Failed parsing vdd data\n"); + return -EINVAL; + } + + if (sdhci_msm_dt_parse_vreg_info(dev, &pdata->vdd_io, "vdd-io")) { + dev_err(dev, "Failed parsing vdd-io data\n"); + return -EINVAL; + } + + return 0; +} + +static int sdhci_msm_vreg_enable(struct device *dev, + struct sdhci_msm_reg_data *vreg) +{ + int ret = 0; + + if (!regulator_is_enabled(vreg->reg)) { + /* Set voltage level */ + ret = regulator_set_voltage(vreg->reg, vreg->high_vol_level, + vreg->high_vol_level); + if (ret) + return ret; + } + + ret = regulator_enable(vreg->reg); + if (ret) { + dev_err(dev, "Failed to enable regulator %s (%d)\n", + vreg->name, ret); + } + + return ret; +} + +static int sdhci_msm_vreg_disable(struct device *dev, + struct sdhci_msm_reg_data *vreg) +{ + int ret = 0; + + if (!regulator_is_enabled(vreg->reg)) + return ret; + + /* Set min. voltage to 0 */ + ret = regulator_set_voltage(vreg->reg, 0, vreg->high_vol_level); + if (ret) + return ret; + + ret = regulator_disable(vreg->reg); + if (ret) { + dev_err(dev, "Failed to disable regulator %s (%d)\n", + vreg->name, ret); + } + + return ret; +} + +static int sdhci_msm_setup_vreg(struct sdhci_msm_host *msm_host, bool enable) +{ + int ret, i; + struct sdhci_msm_reg_data *vreg_table[2]; + + vreg_table[0] = &msm_host->pdata.vdd; + vreg_table[1] = &msm_host->pdata.vdd_io; + + for (i = 0; i < ARRAY_SIZE(vreg_table); i++) { + if (enable) + ret = sdhci_msm_vreg_enable(&msm_host->pdev->dev, + vreg_table[i]); + else + ret = sdhci_msm_vreg_disable(&msm_host->pdev->dev, + vreg_table[i]); + if (ret) + return ret; + } + + return 0; +} + +static int sdhci_msm_vreg_init(struct device *dev, + struct sdhci_msm_pltfm_data *pdata) +{ + struct sdhci_msm_reg_data *vdd_reg = &pdata->vdd; + struct sdhci_msm_reg_data *vdd_io_reg = &pdata->vdd_io; + + vdd_reg->reg = devm_regulator_get(dev, vdd_reg->name); + if (IS_ERR(vdd_reg->reg)) + return PTR_ERR(vdd_reg->reg); + + vdd_io_reg->reg = devm_regulator_get(dev, vdd_io_reg->name); + if (IS_ERR(vdd_io_reg->reg)) + return PTR_ERR(vdd_io_reg->reg); + + return 0; +} + +static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data) +{ + struct sdhci_host *host = (struct sdhci_host *)data; + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_msm_host *msm_host = pltfm_host->priv; + u8 irq_status; + u8 irq_ack = 0; + int ret = 0; + + irq_status = readb_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS); + dev_dbg(mmc_dev(msm_host->mmc), "%s: Received IRQ(%d), status=0x%x\n", + mmc_hostname(msm_host->mmc), irq, irq_status); + + /* Clear the interrupt */ + writeb_relaxed(irq_status, (msm_host->core_mem + CORE_PWRCTL_CLEAR)); + /* + * SDHC has core_mem and hc_mem device memory and these memory + * addresses do not fall within 1KB region. Hence, any update to + * core_mem address space would require an mb() to ensure this gets + * completed before its next update to registers within hc_mem. + */ + mb(); + + /* Handle BUS ON/OFF */ + if (irq_status & CORE_PWRCTL_BUS_ON) { + ret = sdhci_msm_setup_vreg(msm_host, true); + if (!ret) + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, + msm_host->pdata. + vdd_io.high_vol_level, + msm_host->pdata. + vdd_io.high_vol_level); + if (ret) + irq_ack |= CORE_PWRCTL_BUS_FAIL; + else + irq_ack |= CORE_PWRCTL_BUS_SUCCESS; + } + + if (irq_status & CORE_PWRCTL_BUS_OFF) { + ret = sdhci_msm_setup_vreg(msm_host, false); + if (!ret) + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, + msm_host->pdata. + vdd_io.low_vol_level, + msm_host->pdata. + vdd_io.low_vol_level); + if (ret) + irq_ack |= CORE_PWRCTL_BUS_FAIL; + else + irq_ack |= CORE_PWRCTL_BUS_SUCCESS; + } + + /* Handle IO LOW/HIGH */ + if (irq_status & CORE_PWRCTL_IO_LOW) { + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, + msm_host->pdata. + vdd_io.low_vol_level, + msm_host->pdata. + vdd_io.low_vol_level); + if (ret) + irq_ack |= CORE_PWRCTL_IO_FAIL; + else + irq_ack |= CORE_PWRCTL_IO_SUCCESS; + } + + if (irq_status & CORE_PWRCTL_IO_HIGH) { + ret = regulator_set_voltage(msm_host->pdata.vdd_io.reg, + msm_host->pdata. + vdd_io.high_vol_level, + msm_host->pdata. + vdd_io.high_vol_level); + if (ret) + irq_ack |= CORE_PWRCTL_IO_FAIL; + else + irq_ack |= CORE_PWRCTL_IO_SUCCESS; + } + + /* ACK status to the core */ + writeb_relaxed(irq_ack, (msm_host->core_mem + CORE_PWRCTL_CTL)); + /* + * SDHC has core_mem and hc_mem device memory and these memory + * addresses do not fall within 1KB region. Hence, any update to + * core_mem address space would require an mb() to ensure this gets + * completed before its next update to registers within hc_mem. + */ + mb(); + + dev_dbg(mmc_dev(msm_host->mmc), "%s: Handled IRQ(%d), ret=%d, ack=0x%x\n", + mmc_hostname(msm_host->mmc), irq, ret, irq_ack); + return IRQ_HANDLED; +} + +static const struct of_device_id sdhci_msm_dt_match[] = { + { .compatible = "qcom,sdhci-msm-v4" }, + {}, +}; + +MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match); + +static struct sdhci_ops sdhci_msm_ops = { + .platform_execute_tuning = sdhci_msm_execute_tuning, +}; + +static int sdhci_msm_probe(struct platform_device *pdev) +{ + struct sdhci_host *host; + struct sdhci_pltfm_host *pltfm_host; + struct sdhci_msm_host *msm_host; + struct resource *core_memres = NULL; + int ret, dead; + u16 host_version; + + if (!pdev->dev.of_node) { + dev_err(&pdev->dev, "No device tree data\n"); + return -ENOENT; + } + + msm_host = devm_kzalloc(&pdev->dev, sizeof(*msm_host), GFP_KERNEL); + if (!msm_host) + return -ENOMEM; + + msm_host->sdhci_msm_pdata.ops = &sdhci_msm_ops; + host = sdhci_pltfm_init(pdev, &msm_host->sdhci_msm_pdata, 0); + if (IS_ERR(host)) + return PTR_ERR(host); + + pltfm_host = sdhci_priv(host); + pltfm_host->priv = msm_host; + msm_host->mmc = host->mmc; + msm_host->pdev = pdev; + + ret = mmc_of_parse(host->mmc); + if (ret) { + dev_err(&pdev->dev, "Failed parsing mmc device tree\n"); + goto pltfm_free; + } + + sdhci_get_of_property(pdev); + + ret = sdhci_msm_populate_pdata(&pdev->dev, &msm_host->pdata); + if (ret) { + dev_err(&pdev->dev, "DT parsing error\n"); + goto pltfm_free; + } + + /* Setup SDCC bus voter clock. */ + msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus"); + if (!IS_ERR(msm_host->bus_clk)) { + /* Vote for max. clk rate for max. performance */ + ret = clk_set_rate(msm_host->bus_clk, INT_MAX); + if (ret) + goto pltfm_free; + ret = clk_prepare_enable(msm_host->bus_clk); + if (ret) + goto pltfm_free; + } + + /* Setup main peripheral bus clock */ + msm_host->pclk = devm_clk_get(&pdev->dev, "iface"); + if (!IS_ERR(msm_host->pclk)) { + ret = clk_prepare_enable(msm_host->pclk); + if (ret) { + dev_err(&pdev->dev, + "Peripheral clock setup failed (%d)\n", ret); + goto bus_clk_disable; + } + } + + /* Setup SDC MMC clock */ + msm_host->clk = devm_clk_get(&pdev->dev, "core"); + if (IS_ERR(msm_host->clk)) { + ret = PTR_ERR(msm_host->clk); + dev_err(&pdev->dev, "SDC MMC clock setup failed (%d)\n", ret); + goto pclk_disable; + } + + ret = clk_prepare_enable(msm_host->clk); + if (ret) + goto pclk_disable; + + /* Setup regulators */ + ret = sdhci_msm_vreg_init(&pdev->dev, &msm_host->pdata); + if (ret) { + if (ret != -EPROBE_DEFER) + dev_err(&pdev->dev, + "Regulator setup failed (%d)\n", ret); + goto clk_disable; + } + + core_memres = platform_get_resource_byname(pdev, + IORESOURCE_MEM, "core_mem"); + msm_host->core_mem = devm_ioremap_resource(&pdev->dev, core_memres); + + if (IS_ERR(msm_host->core_mem)) { + dev_err(&pdev->dev, "Failed to remap registers\n"); + ret = PTR_ERR(msm_host->core_mem); + goto vreg_disable; + } + + /* Reset the core and Enable SDHC mode */ + writel_relaxed(readl_relaxed(msm_host->core_mem + CORE_POWER) | + CORE_SW_RST, msm_host->core_mem + CORE_POWER); + + /* SW reset can take upto 10HCLK + 15MCLK cycles. (min 40us) */ + usleep_range(1000, 5000); + if (readl(msm_host->core_mem + CORE_POWER) & CORE_SW_RST) { + dev_err(&pdev->dev, "Stuck in reset\n"); + ret = -ETIMEDOUT; + goto vreg_disable; + } + + /* Set HC_MODE_EN bit in HC_MODE register */ + writel_relaxed(HC_MODE_EN, (msm_host->core_mem + CORE_HC_MODE)); + + /* + * Following are the deviations from SDHC spec v3.0 - + * 1. Card detection is handled using separate GPIO. + * 2. Bus power control is handled by interacting with PMIC. + */ + host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION; + host->quirks |= SDHCI_QUIRK_SINGLE_POWER_WRITE; + + host_version = readw_relaxed((host->ioaddr + SDHCI_HOST_VERSION)); + dev_dbg(&pdev->dev, "Host Version: 0x%x Vendor Version 0x%x\n", + host_version, ((host_version & SDHCI_VENDOR_VER_MASK) >> + SDHCI_VENDOR_VER_SHIFT)); + + /* Setup PWRCTL irq */ + msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq"); + if (msm_host->pwr_irq < 0) { + dev_err(&pdev->dev, "Failed to get pwr_irq by name (%d)\n", + msm_host->pwr_irq); + goto vreg_disable; + } + ret = devm_request_threaded_irq(&pdev->dev, msm_host->pwr_irq, NULL, + sdhci_msm_pwr_irq, IRQF_ONESHOT, + dev_name(&pdev->dev), host); + if (ret) { + dev_err(&pdev->dev, "Request threaded irq(%d) failed (%d)\n", + msm_host->pwr_irq, ret); + goto vreg_disable; + } + + /* Enable pwr irq interrupts */ + writel_relaxed(INT_MASK, (msm_host->core_mem + CORE_PWRCTL_MASK)); + + msm_host->mmc->caps |= msm_host->pdata.caps; + msm_host->mmc->caps2 |= msm_host->pdata.caps2; + + ret = sdhci_add_host(host); + if (ret) { + dev_err(&pdev->dev, "Add host failed (%d)\n", ret); + goto vreg_disable; + } + + ret = clk_set_rate(msm_host->clk, host->max_clk); + if (ret) { + dev_err(&pdev->dev, "MClk rate set failed (%d)\n", ret); + goto remove_host; + } + + return 0; + +remove_host: + dead = (readl_relaxed(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff); + sdhci_remove_host(host, dead); +vreg_disable: + if (!IS_ERR(msm_host->pdata.vdd.reg)) + sdhci_msm_vreg_disable(&pdev->dev, &msm_host->pdata.vdd); + if (!IS_ERR(msm_host->pdata.vdd_io.reg)) + sdhci_msm_vreg_disable(&pdev->dev, &msm_host->pdata.vdd_io); +clk_disable: + if (!IS_ERR(msm_host->clk)) + clk_disable_unprepare(msm_host->clk); +pclk_disable: + if (!IS_ERR(msm_host->pclk)) + clk_disable_unprepare(msm_host->pclk); +bus_clk_disable: + if (!IS_ERR(msm_host->bus_clk)) + clk_disable_unprepare(msm_host->bus_clk); +pltfm_free: + sdhci_pltfm_free(pdev); + return ret; +} + +static int sdhci_msm_remove(struct platform_device *pdev) +{ + struct sdhci_host *host = platform_get_drvdata(pdev); + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_msm_host *msm_host = pltfm_host->priv; + int dead = (readl_relaxed(host->ioaddr + SDHCI_INT_STATUS) == + 0xffffffff); + + sdhci_remove_host(host, dead); + sdhci_pltfm_free(pdev); + sdhci_msm_vreg_disable(&pdev->dev, &msm_host->pdata.vdd); + sdhci_msm_vreg_disable(&pdev->dev, &msm_host->pdata.vdd_io); + clk_disable_unprepare(msm_host->clk); + clk_disable_unprepare(msm_host->pclk); + if (!IS_ERR(msm_host->bus_clk)) + clk_disable_unprepare(msm_host->bus_clk); + return 0; +} + +static struct platform_driver sdhci_msm_driver = { + .probe = sdhci_msm_probe, + .remove = sdhci_msm_remove, + .driver = { + .name = "sdhci_msm", + .owner = THIS_MODULE, + .of_match_table = sdhci_msm_dt_match, + }, +}; + +module_platform_driver(sdhci_msm_driver); + +MODULE_DESCRIPTION("Qualcomm Secure Digital Host Controller Interface driver"); +MODULE_LICENSE("GPL v2");