lkml.org 
[lkml]   [2013]   [Jun]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 2/4] mmc: atmel-mci: prepare clk before calling enable
On 28/06/2013 09:44, Nicolas Ferre wrote:
> On 25/06/2013 10:11, Boris BREZILLON :
>> Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
>> avoid common clk framework warnings.
>>
>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>
> It seems that you forgot the Acked-by line from Ludovic.
>
> One comment...
>
>> ---
>> drivers/mmc/host/atmel-mci.c | 27 ++++++++++++++++++---------
>> 1 file changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
>> index e75774f..631c4cf 100644
>> --- a/drivers/mmc/host/atmel-mci.c
>> +++ b/drivers/mmc/host/atmel-mci.c
>> @@ -380,6 +380,8 @@ static int atmci_regs_show(struct seq_file *s,
>> void *v)
>> {
>> struct atmel_mci *host = s->private;
>> u32 *buf;
>> + int ret = 0;
>> +
>>
>> buf = kmalloc(ATMCI_REGS_SIZE, GFP_KERNEL);
>> if (!buf)
>> @@ -391,9 +393,13 @@ static int atmci_regs_show(struct seq_file *s,
>> void *v)
>> * consistent.
>> */
>> spin_lock_bh(&host->lock);
>> - clk_enable(host->mck);
>> + ret = clk_prepare_enable(host->mck);
>
> Do you think that it is needed here? I do not think it can be called
> before the probe() function and I do not know if the result can change
> depending on when clk_prepare_enable() is called...
Same as for other patches: it should not fail if this function is called
after probe,
because the prepare and enable process already succeed once.

But I think this is better to keep the check wherever we can reflect the
error to the caller:
the code will be more robust to clk framework or at91 clk implementation
changes.

I know I told you this kind of check could be removed from dma driver,
so this is a bit inconsistent.

It's up to you, tell me if you want me to remove this part.


>
>> + if (ret) {
>> + spin_unlock_bh(&host->lock);
>> + goto out;
>> + }
>> memcpy_fromio(buf, host->regs, ATMCI_REGS_SIZE);
>> - clk_disable(host->mck);
>> + clk_disable_unprepare(host->mck);
>> spin_unlock_bh(&host->lock);
>>
>> seq_printf(s, "MR:\t0x%08x%s%s ",
>> @@ -444,9 +450,10 @@ static int atmci_regs_show(struct seq_file *s,
>> void *v)
>> val & ATMCI_CFG_LSYNC ? " LSYNC" : "");
>> }
>>
>> +out:
>> kfree(buf);
>>
>> - return 0;
>> + return ret;
>> }
>>
>> static int atmci_regs_open(struct inode *inode, struct file *file)
>> @@ -1281,7 +1288,7 @@ static void atmci_set_ios(struct mmc_host *mmc,
>> struct mmc_ios *ios)
>>
>> spin_lock_bh(&host->lock);
>> if (!host->mode_reg) {
>> - clk_enable(host->mck);
>> + clk_prepare_enable(host->mck);
>> atmci_writel(host, ATMCI_CR, ATMCI_CR_SWRST);
>> atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIEN);
>> if (host->caps.has_cfg_reg)
>> @@ -1361,7 +1368,7 @@ static void atmci_set_ios(struct mmc_host *mmc,
>> struct mmc_ios *ios)
>> atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIDIS);
>> if (host->mode_reg) {
>> atmci_readl(host, ATMCI_MR);
>> - clk_disable(host->mck);
>> + clk_disable_unprepare(host->mck);
>> }
>> host->mode_reg = 0;
>> }
>> @@ -2379,10 +2386,12 @@ static int __init atmci_probe(struct
>> platform_device *pdev)
>> if (!host->regs)
>> goto err_ioremap;
>>
>> - clk_enable(host->mck);
>> + ret = clk_prepare_enable(host->mck);
>> + if (ret)
>> + goto err_request_irq;
>> atmci_writel(host, ATMCI_CR, ATMCI_CR_SWRST);
>> host->bus_hz = clk_get_rate(host->mck);
>> - clk_disable(host->mck);
>> + clk_disable_unprepare(host->mck);
>>
>> host->mapbase = regs->start;
>>
>> @@ -2487,11 +2496,11 @@ static int __exit atmci_remove(struct
>> platform_device *pdev)
>> atmci_cleanup_slot(host->slot[i], i);
>> }
>>
>> - clk_enable(host->mck);
>> + clk_prepare_enable(host->mck);
>> atmci_writel(host, ATMCI_IDR, ~0UL);
>> atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIDIS);
>> atmci_readl(host, ATMCI_SR);
>> - clk_disable(host->mck);
>> + clk_disable_unprepare(host->mck);
>>
>> if (host->dma.chan)
>> dma_release_channel(host->dma.chan);
>>
>
>



\
 
 \ /
  Last update: 2013-06-28 10:21    [W:0.050 / U:0.380 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site