Hi Ivan,

On 08/23/2013 10:34 AM, Ivan T. Ivanov wrote:
Hi Georgi,

On Tue, 2013-08-20 at 19:44 +0300, Georgi Djakov wrote:
This platform driver adds the support of Secure Digital Host
Controller Interface compliant controller in MSM chipsets.

CC: Asutosh Das <asuto...@codeaurora.org>
CC: Venkat Gopalakrishnan <venk...@codeaurora.org>
CC: Sahitya Tummala <stumm...@codeaurora.org>
CC: Subhash Jadavani <subha...@codeaurora.org>
Signed-off-by: Georgi Djakov <gdja...@mm-sol.com>
---
Changes from v2:
- Added DT bindings for clocks
- Moved voltage regulators data to platform data
- Removed unneeded includes
- Removed obsolete and wrapper functions
- Removed error checking where unnecessary
- Removed redundant _clk suffix from clock names
- Just return instead of goto where possible
- Minor fixes


Is this version intermediate step before you address
all previous comments?


+
+static const struct of_device_id sdhci_msm_dt_match[] = {
+       {.compatible = "qcom,sdhci-msm"},

Missing termination entry

+};
+
+MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match);
+
+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 = 0, dead = 0;
+       struct pinctrl  *pinctrl;
+
+       msm_host = devm_kzalloc(&pdev->dev, sizeof(struct sdhci_msm_host),
+                               GFP_KERNEL);
+       if (!msm_host)
+               return -ENOMEM;
+
+       host = sdhci_pltfm_init(pdev, &msm_host->sdhci_msm_pdata, 0);
+       if (IS_ERR(host)) {
+               dev_err(mmc_dev(host->mmc), "sdhci_pltfm_init error\n");
+               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)

Shouldn't sdhci_pltfm_init operation be reverted?

+               return ret;
+
+       /* Extract platform data */
+       if (pdev->dev.of_node) {
+               msm_host->pdata = sdhci_msm_populate_pdata(&pdev->dev);
+               if (!msm_host->pdata) {
+                       dev_err(&pdev->dev, "DT parsing error\n");
+                       goto pltfm_free;
+               }
+       } else {
+               dev_err(&pdev->dev, "No device tree node\n");
+               goto pltfm_free;
+       }
+
+       /* Setup pins */
+       pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+       if (IS_ERR(pinctrl))
+               dev_warn(&pdev->dev, "pins are not configured by the driver\n");
+
+       /* Setup Clocks */
+
+       /* Setup SDCC bus voter clock. */
+       msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
+       if (!IS_ERR_OR_NULL(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,
+                               "Main 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, true);
+       if (ret) {
+               dev_err(&pdev->dev, "Regulator setup failed (%d)\n", ret);
+               goto clk_disable;
+       }
+
+       /* Reset the core and Enable SDHC mode */
+       core_memres = platform_get_resource_byname(pdev,
+                                                  IORESOURCE_MEM, "core_mem");
+       msm_host->core_mem = devm_ioremap(&pdev->dev, core_memres->start,
+                                         resource_size(core_memres));
+
+       if (!msm_host->core_mem) {
+               dev_err(&pdev->dev, "Failed to remap registers\n");
+               ret = -ENOMEM;
+               goto vreg_deinit;
+       }
+
+       /* Set SW_RST bit in POWER register (Offset 0x0) */
+       writel_relaxed(CORE_SW_RST, msm_host->core_mem + CORE_POWER);
+       /* 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;
+
+       /* 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_deinit;
+       }
+       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_deinit;
+       }
+
+       /* Enable pwr irq interrupts */
+       writel_relaxed(INT_MASK, (msm_host->core_mem + CORE_PWRCTL_MASK));
+
+       /* Set host capabilities */
+       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_deinit;
+       }
+
+       /* Set core clk rate */
+       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;
+       }
+
+       host->mmc->max_current_180 = host->mmc->max_current_300 =
+       host->mmc->max_current_330 = sdhci_msm_get_vdd_max_current(msm_host);
+
+       /* Successful initialization */
+       goto out;
+
+remove_host:
+       dead = (readl_relaxed(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
+       sdhci_remove_host(host, dead);
+vreg_deinit:
+       sdhci_msm_vreg_init(&pdev->dev, msm_host->pdata, false);
+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_OR_NULL(msm_host->bus_clk))
+               clk_disable_unprepare(msm_host->bus_clk);
+pltfm_free:
+       sdhci_pltfm_free(pdev);
+out:
+       return ret;
+}
+

<snip>

+
+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 = of_match_ptr(sdhci_msm_dt_match),

of_match_ptr is not really required.


Thank you for the additional comments. I'll address all issues in the next version.

BR,
Georgi

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to