Hello Ulf,

Thanks a lot for your feedback.

On 01/29/2015 02:05 PM, Ulf Hansson wrote:

quoted

 struct mmc_pwrseq_simple {
        struct mmc_pwrseq pwrseq;
+       struct clk *ext_clk;

You need to add a bool, maybe call it clk_enabled;  See why below.

Ok
 

quoted

        int nr_gpios;
        struct gpio_desc *reset_gpios[0];
 };

@@ -39,6 +41,9 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
        struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
                                        struct mmc_pwrseq_simple, pwrseq);

+       if (!IS_ERR(pwrseq->ext_clk))
+               clk_prepare_enable(pwrseq->ext_clk);
+

There are no guarantee that the ->mmc_pwrseq_simple_pre_power_on()
will be invoked prior ->mmc_pwrseq_simple_power_off().

Got it, I didn't know that mmc_pwrseq_simple_power_off() could be invoked.
without mmc_pwrseq_simple_pre_power_on() not being called before.

That means you need to keep track of if you have gated/ungated the
clock. In other words check pwrseq->clk_enabled. That will prevent
potential clk unbalance issues.

Yes, I'll change to check for the boolean in _simple_power_off() and
_post_power_on() then.

quoted

@@ -85,6 +104,14 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
        if (!pwrseq)
                return -ENOMEM;

+       pwrseq->ext_clk = clk_get(dev, "ext_clock");
+       if (IS_ERR(pwrseq->ext_clk) &&
+           PTR_ERR(pwrseq->ext_clk) != -ENOENT &&
+           PTR_ERR(pwrseq->ext_clk) != -ENOSYS) {

I don't think you can get -ENOSYS.

You are right, I'll remove that.

Best regards,
Javier