Thread (1 message) 1 message, 1 author, 2015-06-11

Re: [PATCH v5 2/7] mmc: mediatek: Add Mediatek MMC driver

From: Chaotian Jing <hidden>
Date: 2015-06-11 01:36:32
Also in: linux-arm-kernel, linux-devicetree, linux-gpio, linux-mmc

Possibly related (same subject, not in this thread)

Dear Ulf,

Thanks, please see my comment:

On Wed, 2015-06-10 at 13:53 +0200, Ulf Hansson wrote:
[...]
quoted
+static int msdc_drv_probe(struct platform_device *pdev)
+{
+       struct mmc_host *mmc;
+       struct msdc_host *host;
+       struct resource *res;
+       int ret;
+
+       if (!pdev->dev.of_node) {
+               dev_err(&pdev->dev, "No DT found\n");
+               return -EINVAL;
+       }
+       /* Allocate MMC host for this device */
+       mmc = mmc_alloc_host(sizeof(struct msdc_host), &pdev->dev);
+       if (!mmc)
+               return -ENOMEM;
+
+       host = mmc_priv(mmc);
+       ret = mmc_of_parse(mmc);
+       if (ret)
+               goto host_free;
+
+       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       host->base = devm_ioremap_resource(&pdev->dev, res);
+       if (IS_ERR(host->base)) {
+               ret = PTR_ERR(host->base);
+               goto host_free;
+       }
+
+       ret = mmc_regulator_get_supply(mmc);
+       if (ret == -EPROBE_DEFER)
+               goto host_free;
+
+       host->src_clk = devm_clk_get(&pdev->dev, "source");
+       if (IS_ERR(host->src_clk)) {
+               ret = PTR_ERR(host->src_clk);
+               goto host_free;
+       }
+
+       host->h_clk = devm_clk_get(&pdev->dev, "hclk");
+       if (IS_ERR(host->h_clk)) {
+               /* host->h_clk is optional, Only for MSDC0/3 at MT8173 */
+               dev_dbg(&pdev->dev,
+                               "Invalid hclk from the device tree!\n");
+       }
+
+       host->irq = platform_get_irq(pdev, 0);
+       if (host->irq < 0) {
+               ret = -EINVAL;
+               goto host_free;
+       }
+
+       host->pinctrl = devm_pinctrl_get(&pdev->dev);
+       if (IS_ERR(host->pinctrl)) {
+               ret = PTR_ERR(host->pinctrl);
+               dev_err(&pdev->dev, "Cannot find pinctrl!\n");
+               goto host_free;
+       }
+
+       host->pins_default = pinctrl_lookup_state(host->pinctrl, "default");
+       if (IS_ERR(host->pins_default)) {
+               ret = PTR_ERR(host->pins_default);
+               dev_err(&pdev->dev, "Cannot find pinctrl default!\n");
+               goto host_free;
+       }
+
+       host->pins_uhs = pinctrl_lookup_state(host->pinctrl, "state_uhs");
+       if (IS_ERR(host->pins_uhs)) {
+               ret = PTR_ERR(host->pins_uhs);
+               dev_err(&pdev->dev, "Cannot find pinctrl uhs!\n");
+               goto host_free;
+       }
+
+       host->dev = &pdev->dev;
+       host->mmc = mmc;
+       host->hclk = clk_get_rate(host->src_clk);
This looks odd. The clock rate for the source clock is assigned as the
speed of hclk!? Why?
Yes, the name is odd, here the host->hclk is indicate the source clock frequency, but not the HCLK frequency.
I will modify the name to host->src_clk_freq at next revision.
quoted
+       /* Set host parameters to mmc */
+       mmc->ops = &mt_msdc_ops;
+       mmc->f_min = host->hclk / (4 * 255);
+
+       mmc->caps |= MMC_CAP_ERASE | MMC_CAP_CMD23;
+       /* MMC core transfer sizes tunable parameters */
+       mmc->max_segs = MAX_BD_NUM;
+       mmc->max_seg_size = BDMA_DESC_BUFLEN;
+       mmc->max_blk_size = 2048;
+       mmc->max_req_size = 512 * 1024;
+       mmc->max_blk_count = mmc->max_req_size / 512;
+       host->dma_mask = DMA_BIT_MASK(32);
+       mmc_dev(mmc)->dma_mask = &host->dma_mask;
+
+       host->timeout_clks = 3 * 1048576;
+       host->dma.gpd = dma_alloc_coherent(&pdev->dev,
+                               sizeof(struct mt_gpdma_desc),
+                               &host->dma.gpd_addr, GFP_KERNEL);
+       host->dma.bd = dma_alloc_coherent(&pdev->dev,
+                               MAX_BD_NUM * sizeof(struct mt_bdma_desc),
+                               &host->dma.bd_addr, GFP_KERNEL);
+       if (!host->dma.gpd || !host->dma.bd) {
+               ret = -ENOMEM;
+               goto release_mem;
+       }
+       msdc_init_gpd_bd(host, &host->dma);
+       INIT_DELAYED_WORK(&host->req_timeout, msdc_request_timeout);
+       spin_lock_init(&host->lock);
+
+       platform_set_drvdata(pdev, mmc);
+       msdc_ungate_clock(host);
+       msdc_init_hw(host);
+
+       ret = devm_request_irq(&pdev->dev, host->irq, msdc_irq,
+               IRQF_TRIGGER_LOW | IRQF_ONESHOT, pdev->name, host);
+       if (ret)
+               goto release;
+
+       ret = mmc_add_host(mmc);
+       if (ret)
+               goto release;
+
+       return 0;
+
+release:
+       platform_set_drvdata(pdev, NULL);
+       msdc_deinit_hw(host);
+       msdc_gate_clock(host);
+release_mem:
+       if (host->dma.gpd)
+               dma_free_coherent(&pdev->dev,
+                       sizeof(struct mt_gpdma_desc),
+                       host->dma.gpd, host->dma.gpd_addr);
+       if (host->dma.bd)
+               dma_free_coherent(&pdev->dev,
+                       MAX_BD_NUM * sizeof(struct mt_bdma_desc),
+                       host->dma.bd, host->dma.bd_addr);
+host_free:
+       mmc_free_host(mmc);
+
+       return ret;
+}
[...]

Kind regards
Uffe
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help