diff mbox

[2/6] thermal:exynos4: TMU Common clock framework support for TMU (Thermal Monitoring Unit)

Message ID 1366389493-8239-3-git-send-email-l.majewski@samsung.com (mailing list archive)
State Accepted, archived
Delegated to: Zhang Rui
Headers show

Commit Message

Lukasz Majewski April 19, 2013, 4:38 p.m. UTC
This patch modifies exynos_thermal.c file to use clk_disable_unprepare()
and clk_prepare_enable() instead of clk_{enable|disable}.

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/thermal/exynos_thermal.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Eduardo Valentin April 19, 2013, 5:26 p.m. UTC | #1
On 19-04-2013 12:38, Lukasz Majewski wrote:
> This patch modifies exynos_thermal.c file to use clk_disable_unprepare()
> and clk_prepare_enable() instead of clk_{enable|disable}.
>
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>   drivers/thermal/exynos_thermal.c |   18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
> index 46568c0..ba6094c 100644
> --- a/drivers/thermal/exynos_thermal.c
> +++ b/drivers/thermal/exynos_thermal.c
> @@ -584,7 +584,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>   	int ret = 0, threshold_code, i, trigger_levs = 0;
>
>   	mutex_lock(&data->lock);
> -	clk_enable(data->clk);
> +	clk_prepare_enable(data->clk);
>
>   	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
>   	if (!status) {
> @@ -655,7 +655,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>   				data->base + EXYNOS_TMU_REG_INTCLEAR);
>   	}
>   out:
> -	clk_disable(data->clk);
> +	clk_disable_unprepare(data->clk);
>   	mutex_unlock(&data->lock);
>
>   	return ret;
> @@ -668,7 +668,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
>   	unsigned int con, interrupt_en;
>
>   	mutex_lock(&data->lock);
> -	clk_enable(data->clk);
> +	clk_prepare_enable(data->clk);
>
>   	con = pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT |
>   		pdata->gain << EXYNOS_TMU_GAIN_SHIFT;
> @@ -693,7 +693,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
>   	writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN);
>   	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
>
> -	clk_disable(data->clk);
> +	clk_disable_unprepare(data->clk);
>   	mutex_unlock(&data->lock);
>   }
>
> @@ -703,12 +703,12 @@ static int exynos_tmu_read(struct exynos_tmu_data *data)
>   	int temp;
>
>   	mutex_lock(&data->lock);
> -	clk_enable(data->clk);
> +	clk_prepare_enable(data->clk);
>
>   	temp_code = readb(data->base + EXYNOS_TMU_REG_CURRENT_TEMP);
>   	temp = code_to_temp(data, temp_code);
>
> -	clk_disable(data->clk);
> +	clk_disable_unprepare(data->clk);
>   	mutex_unlock(&data->lock);
>
>   	return temp;
> @@ -721,7 +721,7 @@ static void exynos_tmu_work(struct work_struct *work)
>
>   	exynos_report_trigger();
>   	mutex_lock(&data->lock);
> -	clk_enable(data->clk);
> +	clk_prepare_enable(data->clk);
>   	if (data->soc == SOC_ARCH_EXYNOS)
>   		writel(EXYNOS_TMU_CLEAR_RISE_INT |
>   				EXYNOS_TMU_CLEAR_FALL_INT,
> @@ -729,7 +729,7 @@ static void exynos_tmu_work(struct work_struct *work)
>   	else
>   		writel(EXYNOS4210_TMU_INTCLEAR_VAL,
>   				data->base + EXYNOS_TMU_REG_INTCLEAR);
> -	clk_disable(data->clk);
> +	clk_disable_unprepare(data->clk);
>   	mutex_unlock(&data->lock);
>
>   	enable_irq(data->irq);
> @@ -985,7 +985,7 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>   		return ret;
>   	}
>
> -	data->clk = clk_get(NULL, "tmu_apbif");
> +	data->clk = clk_get(&pdev->dev, "tmu_apbif");

This change does not seam to be part of the patch orginal purpose, at 
least not as described in your patch description.

>   	if (IS_ERR(data->clk)) {
>   		dev_err(&pdev->dev, "Failed to get clock\n");
>   		return  PTR_ERR(data->clk);
>

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sachin Kamat April 19, 2013, 6:08 p.m. UTC | #2
On 19 April 2013 22:08, Lukasz Majewski <l.majewski@samsung.com> wrote:
> This patch modifies exynos_thermal.c file to use clk_disable_unprepare()
> and clk_prepare_enable() instead of clk_{enable|disable}.
>
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---

I have already submitted a similar patch for this:
http://permalink.gmane.org/gmane.linux.power-management.general/33310
Lukasz Majewski April 23, 2013, 6:17 a.m. UTC | #3
Hi Sachin,

> On 19 April 2013 22:08, Lukasz Majewski <l.majewski@samsung.com>
> wrote:
> > This patch modifies exynos_thermal.c file to use
> > clk_disable_unprepare() and clk_prepare_enable() instead of
> > clk_{enable|disable}.
> >
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> 
> I have already submitted a similar patch for this:
> http://permalink.gmane.org/gmane.linux.power-management.general/33310
> 
> 

Thanks for pointing to the correct patch.

However, I've got a question:

The patch only changes clock names at exynos_tmu_{probe|remove}.

Correct me if I'm wrong, but shouldn't we also change clock_enable to
clk_prepare_enable at exynos_tmu_read()? (Are we guaranteed, that we
will NOT sleep there?)
Sachin Kamat April 23, 2013, 7:15 a.m. UTC | #4
Hi Lukasz,

On 23 April 2013 11:47, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Hi Sachin,
>
>> On 19 April 2013 22:08, Lukasz Majewski <l.majewski@samsung.com>
>> wrote:
>> > This patch modifies exynos_thermal.c file to use
>> > clk_disable_unprepare() and clk_prepare_enable() instead of
>> > clk_{enable|disable}.
>> >
>> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
>> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> > ---
>>
>> I have already submitted a similar patch for this:
>> http://permalink.gmane.org/gmane.linux.power-management.general/33310
>>
>>
>
> Thanks for pointing to the correct patch.
>
> However, I've got a question:
>
> The patch only changes clock names at exynos_tmu_{probe|remove}.
>
> Correct me if I'm wrong, but shouldn't we also change clock_enable to
> clk_prepare_enable at exynos_tmu_read()? (Are we guaranteed, that we
> will NOT sleep there?)

Since clk_prepare does not do anything, i thought it was sufficient to
have it once in probe and then unprepare in remove.
Do you see a real problem in this implementation. If so I can update
it to use clk_prepare_enable at other places as well.
Lukasz Majewski April 23, 2013, 8:06 a.m. UTC | #5
Hi Sachin,

> Hi Lukasz,
> 
> On 23 April 2013 11:47, Lukasz Majewski <l.majewski@samsung.com>
> wrote:
> > Hi Sachin,
> >
> >> On 19 April 2013 22:08, Lukasz Majewski <l.majewski@samsung.com>
> >> wrote:
> >> > This patch modifies exynos_thermal.c file to use
> >> > clk_disable_unprepare() and clk_prepare_enable() instead of
> >> > clk_{enable|disable}.
> >> >
> >> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> >> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >> > ---
> >>
> >> I have already submitted a similar patch for this:
> >> http://permalink.gmane.org/gmane.linux.power-management.general/33310
> >>
> >>
> >
> > Thanks for pointing to the correct patch.
> >
> > However, I've got a question:
> >
> > The patch only changes clock names at exynos_tmu_{probe|remove}.
> >
> > Correct me if I'm wrong, but shouldn't we also change clock_enable
> > to clk_prepare_enable at exynos_tmu_read()? (Are we guaranteed,
> > that we will NOT sleep there?)
> 
> Since clk_prepare does not do anything, i thought it was sufficient to
> have it once in probe and then unprepare in remove.
> Do you see a real problem in this implementation. If so I can update
> it to use clk_prepare_enable at other places as well.
> 
I just wanted to stick to the common clock new API. It seems to me that
exynos_tmu_read() might sleep, but I'm quite novice at TMU :-).

If yours patches work, then we shall apply them.

I will do my best to test yours three patches ASAP on my setup.
Sachin Kamat April 23, 2013, 8:42 a.m. UTC | #6
Hi Lukasz,

On 23 April 2013 13:36, Lukasz Majewski <l.majewski@samsung.com> wrote:

>>
> I just wanted to stick to the common clock new API. It seems to me that
> exynos_tmu_read() might sleep, but I'm quite novice at TMU :-).
>
> If yours patches work, then we shall apply them.
>
> I will do my best to test yours three patches ASAP on my setup.

Thanks. Please let me know if it works for you.
diff mbox

Patch

diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
index 46568c0..ba6094c 100644
--- a/drivers/thermal/exynos_thermal.c
+++ b/drivers/thermal/exynos_thermal.c
@@ -584,7 +584,7 @@  static int exynos_tmu_initialize(struct platform_device *pdev)
 	int ret = 0, threshold_code, i, trigger_levs = 0;
 
 	mutex_lock(&data->lock);
-	clk_enable(data->clk);
+	clk_prepare_enable(data->clk);
 
 	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
 	if (!status) {
@@ -655,7 +655,7 @@  static int exynos_tmu_initialize(struct platform_device *pdev)
 				data->base + EXYNOS_TMU_REG_INTCLEAR);
 	}
 out:
-	clk_disable(data->clk);
+	clk_disable_unprepare(data->clk);
 	mutex_unlock(&data->lock);
 
 	return ret;
@@ -668,7 +668,7 @@  static void exynos_tmu_control(struct platform_device *pdev, bool on)
 	unsigned int con, interrupt_en;
 
 	mutex_lock(&data->lock);
-	clk_enable(data->clk);
+	clk_prepare_enable(data->clk);
 
 	con = pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT |
 		pdata->gain << EXYNOS_TMU_GAIN_SHIFT;
@@ -693,7 +693,7 @@  static void exynos_tmu_control(struct platform_device *pdev, bool on)
 	writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN);
 	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
 
-	clk_disable(data->clk);
+	clk_disable_unprepare(data->clk);
 	mutex_unlock(&data->lock);
 }
 
@@ -703,12 +703,12 @@  static int exynos_tmu_read(struct exynos_tmu_data *data)
 	int temp;
 
 	mutex_lock(&data->lock);
-	clk_enable(data->clk);
+	clk_prepare_enable(data->clk);
 
 	temp_code = readb(data->base + EXYNOS_TMU_REG_CURRENT_TEMP);
 	temp = code_to_temp(data, temp_code);
 
-	clk_disable(data->clk);
+	clk_disable_unprepare(data->clk);
 	mutex_unlock(&data->lock);
 
 	return temp;
@@ -721,7 +721,7 @@  static void exynos_tmu_work(struct work_struct *work)
 
 	exynos_report_trigger();
 	mutex_lock(&data->lock);
-	clk_enable(data->clk);
+	clk_prepare_enable(data->clk);
 	if (data->soc == SOC_ARCH_EXYNOS)
 		writel(EXYNOS_TMU_CLEAR_RISE_INT |
 				EXYNOS_TMU_CLEAR_FALL_INT,
@@ -729,7 +729,7 @@  static void exynos_tmu_work(struct work_struct *work)
 	else
 		writel(EXYNOS4210_TMU_INTCLEAR_VAL,
 				data->base + EXYNOS_TMU_REG_INTCLEAR);
-	clk_disable(data->clk);
+	clk_disable_unprepare(data->clk);
 	mutex_unlock(&data->lock);
 
 	enable_irq(data->irq);
@@ -985,7 +985,7 @@  static int exynos_tmu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	data->clk = clk_get(NULL, "tmu_apbif");
+	data->clk = clk_get(&pdev->dev, "tmu_apbif");
 	if (IS_ERR(data->clk)) {
 		dev_err(&pdev->dev, "Failed to get clock\n");
 		return  PTR_ERR(data->clk);