On 03/05/2014 06:41 AM, Ulf Hansson wrote:
On 4 March 2014 20:27, Georgi Djakov <gdjakov@xxxxxxxxxx> wrote:
[..]
+
+struct sdhci_msm_pltfm_data {
+ u32 caps; /* Supported UHS-I Modes */
+ u32 caps2; /* More capabilities */
Why do you need these caps, there are already a part of the mmc host.
Thanks! Will remove.
+ struct regulator *vdd; /* VDD/VCC regulator */
+ struct regulator *vdd_io; /* VDD IO regulator */
Why do you need to duplicate the regulators for sdhci_msm? sdhci core
is already taking care of regulators, I suppose you should adopt to
that!?
Ok, I'll try to adopt this.
+};
+
+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;
+};
+
[..]
+static int sdhci_msm_vreg_init(struct device *dev,
+ struct sdhci_msm_pltfm_data *pdata)
+{
+ pdata->vdd = devm_regulator_get(dev, "vdd-supply");
+ if (IS_ERR(pdata->vdd))
+ return PTR_ERR(pdata->vdd);
+
+ pdata->vdd_io = devm_regulator_get(dev, "vdd-io-supply");
+ if (IS_ERR(pdata->vdd_io))
+ return PTR_ERR(pdata->vdd_io);
+
+ return 0;
+}
+
The hole regulator handling seems like it's being duplicated from the
sdhci core. If the regulator handling from the sdhci core don't suite
your need I guess you should extend that instead - to prevent the
duplication of code and structs.
Moreover I think it's time for sdhci core to move on to use the
mmc_regulator_get_supply() API. Additionally it should be able to use
mmc_regulator_set_ocr() API to change voltage. I guess that's not
related to this patch though, but just wanted to provide you my view
on it.
Ok, I see. Thanks for the hints!
[..]
+ 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);
Don't you need to set the rate before adding the host?
I will just make use of the sdhci core for clock setup too. Thanks for
reviewing!
BR,
Georgi
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
