Re: [PATCH 1/7] mmc: dw_mmc: lookup for optional biu and ciu clocks

[Thomas Abraham]

On 2 May 2012 14:47, Will Newton <will.newton@xxxxxxxxx> wrote:
> On Wed, May 2, 2012 at 6:07 AM, Thomas Abraham
> <thomas.abraham@xxxxxxxxxx> wrote:
>> Some platforms allow for clock gating and control of bus interface unit clock
>> and card interface unit clock. Add support for clock lookup of optional biu
>> and ciu clocks for clock gating and clock speed determination.
>>
>> Signed-off-by: Abhilash Kesavan <a.kesavan@xxxxxxxxxxx>
>> Signed-off-by: Thomas Abraham <thomas.abraham@xxxxxxxxxx>
>> ---
>>  drivers/mmc/host/dw_mmc.c  |   35 +++++++++++++++++++++++++++++++----
>>  include/linux/mmc/dw_mmc.h |    4 ++++
>>  2 files changed, 35 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 1532357..036846f 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1938,19 +1938,35 @@ int dw_mci_probe(struct dw_mci *host)
>>                return -ENODEV;
>>        }
>>
>> -       if (!host->pdata->bus_hz) {
>> +       host->biu_clk = clk_get(&host->dev, "biu");
>> +       if (IS_ERR(host->biu_clk))
>> +               dev_info(&host->dev, "biu clock not available\n");
>> +       else
>> +               clk_enable(host->biu_clk);
>> +
>> +       host->ciu_clk = clk_get(&host->dev, "ciu");
>> +       if (IS_ERR(host->ciu_clk))
>> +               dev_info(&host->dev, "ciu clock not available\n");
>
> Should these printks be at debug level? I'm not 100% sure either way
> but it could be a little noisy on systems that do not use these
> clocks.

Right. I will change dev_info to dev_dbg.

>
>> +       else
>> +               clk_enable(host->ciu_clk);
>> +
>> +       if (IS_ERR(host->ciu_clk))
>> +               host->bus_hz = host->pdata->bus_hz;
>> +       else
>> +               host->bus_hz = clk_get_rate(host->ciu_clk);
>> +
>> +       if (!host->bus_hz) {
>>                dev_err(&host->dev,
>>                        "Platform data must supply bus speed\n");
>> -               return -ENODEV;
>> +               ret = -ENODEV;
>> +               goto err_clk;
>>        }
>>
>> -       host->bus_hz = host->pdata->bus_hz;
>>        host->quirks = host->pdata->quirks;
>>
>>        spin_lock_init(&host->lock);
>>        INIT_LIST_HEAD(&host->queue);
>>
>> -
>>        host->dma_ops = host->pdata->dma_ops;
>>        dw_mci_init_dma(host);
>>
>> @@ -2095,6 +2111,13 @@ err_dmaunmap:
>>                regulator_disable(host->vmmc);
>>                regulator_put(host->vmmc);
>>        }
>> +       kfree(host);
>> +
>> +err_clk:
>> +       clk_disable(host->ciu_clk);
>> +       clk_disable(host->biu_clk);
>> +       clk_put(host->ciu_clk);
>> +       clk_put(host->biu_clk);
>
> Are these calls safe, or do we need to check IS_ERR as above?

The clk_disable is safe on Samsung platforms but it cannot be assumed
for other platforms. So, the IS_ERR check will be added before
clk_disable. clk_put will not need the IS_ERR check. Thanks for
pointing this out.

>
>>        return ret;
>>  }
>>  EXPORT_SYMBOL(dw_mci_probe);
>> @@ -2128,6 +2151,10 @@ void dw_mci_remove(struct dw_mci *host)
>>                regulator_put(host->vmmc);
>>        }
>>
>> +       clk_disable(host->ciu_clk);
>> +       clk_disable(host->biu_clk);
>> +       clk_put(host->ciu_clk);
>> +       clk_put(host->biu_clk);
>
> Likewise.

Yes, will be fixed.

Thanks,
Thomas.

[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/