Re: [PATCH 5/5] ata/ahci_platform: Add clock framework support

On 4/20/12, Sergei Shtylyov <sshtylyov <at> mvista.com> wrote:
> On 04/20/2012 06:38 PM, Viresh Kumar wrote:
>> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
>>  <at>  <at>  -117,6 +118,20  <at>  <at>  static int __init ahci_probe(struct platform_device
>> *pdev)
>>   		return -ENOMEM;
>>   	}
>>
>> +#ifdef CONFIG_HAVE_CLK
>> +	hpriv->clk = clk_get(dev, NULL);
>> +	if (IS_ERR(hpriv->clk)) {
>> +		dev_err(dev, "Clock not found\n");
>> +		return PTR_ERR(hpriv->clk);
>> +	}
>> +
>> +	rc = clk_prepare_enable(hpriv->clk);
>> +	if (rc) {
>> +		dev_err(dev, "clock prepare enable failed");
>> +		goto free_clk;
>> +	}
>> +#endif
>> +
>>   	/*
>>   	 * Some platforms might need to prepare for mmio region access,
>>   	 * which could be done in the following init call. So, the mmio
> [...]
>>  <at>  <at>  -190,12 +205,18  <at>  <at>  static int __init ahci_probe(struct platform_device
>> *pdev)
>>   	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
>>   			&ahci_platform_sht);
>>   	if (rc)
>> -		goto err0;
>> +		goto pdata_exit;
>>
>>   	return 0;
>> -err0:
>> +pdata_exit:
>>   	if (pdata&&  pdata->exit)
>>   		pdata->exit(dev);
>> +disable_unprepare_clk:
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +free_clk:
>> +	clk_put(hpriv->clk);
>> +#endif
>>   	return rc;
>>   }
>>
>>  <at>  <at>  -204,12 +225,19  <at>  <at>  static int __devexit ahci_remove(struct
>> platform_device *pdev)
>>   	struct device *dev =&pdev->dev;
>>   	struct ahci_platform_data *pdata = dev_get_platdata(dev);
>>   	struct ata_host *host = dev_get_drvdata(dev);
>> +#ifdef CONFIG_HAVE_CLK
>> +	struct ahci_host_priv *hpriv = host->private_data;
>> +#endif
>>
>>   	ata_host_detach(host);
>>
>>   	if (pdata&&  pdata->exit)
>>   		pdata->exit(dev);
>>
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +	clk_put(hpriv->clk);
>> +#endif
>>   	return 0;
>>   }
>>
>>  <at>  <at>  -244,6 +272,11  <at>  <at>  static int ahci_suspend(struct device *dev)
>>
>>   	if (pdata&&  pdata->suspend)
>>   		return pdata->suspend(dev);
>> +
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +#endif
>> +
>>   	return 0;
>>   }
>>
>>  <at>  <at>  -253,16 +286,26  <at>  <at>  static int ahci_resume(struct device *dev)
>>   	struct ata_host *host = dev_get_drvdata(dev);
>>   	int rc;
>>
>> +#ifdef CONFIG_HAVE_CLK
>> +	struct ahci_host_priv *hpriv = host->private_data;
>> +
>> +	rc = clk_prepare_enable(hpriv->clk);
>> +	if (rc) {
>> +		dev_err(dev, "clock prepare enable failed");
>> +		return rc;
>> +	}
>> +#endif
>> +
> [...]
>>  <at>  <at>  -270,6 +313,12  <at>  <at>  static int ahci_resume(struct device *dev)
>>   	ata_host_resume(host);
>>
>>   	return 0;
>> +
>> +disable_unprepare_clk:
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +#endif
>
>     Can't we have empty inlines for !defined(CONFIG_HAVE_CLK) case in
> <linux/clk.h>. Otherwise there's too much uglye #ifdef'ery.

Isn't that simple. Lot of platforms defining these routines clk_get(),
clk_put(), etc.
So, if we create dummy routines, they will get errors for multiple declarations.

Even this CONFIG_HAVE_CLK is not the right macro i think.

 <at> Mike: Can you please suggest how to tackle this problem.

--
Viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html