[PATCH] thermal: exynos: handle gate clock for misplaced TRIMINFO register

[PATCH] thermal: exynos: handle gate clock for misplaced TRIMINFO register

[Naveen Krishna Chatradhi]

On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from
the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second
should be acompanied by enabling the respective clock.

This patch which allow for a "clk_sec" clock to be specified in the
device-tree which will be ungated when accessing the TRIMINFO register.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c |   24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index b54825a..33edd1a 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -47,6 +47,7 @@
  * @irq_work: pointer to the irq work structure.
  * @lock: lock to implement synchronization.
  * @clk: pointer to the clock structure.
+ * @clk_sec: pointer to the clock structure for accessing the base_second.
  * @temp_error1: fused value of the first point trim.
  * @temp_error2: fused value of the second point trim.
  * @regulator: pointer to the TMU regulator structure.
@@ -61,7 +62,7 @@ struct exynos_tmu_data {
 	enum soc_type soc;
 	struct work_struct irq_work;
 	struct mutex lock;
-	struct clk *clk;
+	struct clk *clk, *clk_sec;
 	u8 temp_error1, temp_error2;
 	struct regulator *regulator;
 	struct thermal_sensor_conf *reg_conf;
@@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 
 	mutex_lock(&data->lock);
 	clk_enable(data->clk);
+	if (!IS_ERR(data->clk_sec))
+		clk_enable(data->clk_sec);
 
 	if (TMU_SUPPORTS(pdata, READY_STATUS)) {
 		status = readb(data->base + reg->tmu_status);
@@ -306,6 +309,8 @@ skip_calib_data:
 out:
 	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
+	if (!IS_ERR(data->clk_sec))
+		clk_disable(data->clk_sec);
 
 	return ret;
 }
@@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work)
 	const struct exynos_tmu_registers *reg = pdata->registers;
 	unsigned int val_irq, val_type;
 
+	if (!IS_ERR(data->clk_sec))
+		clk_enable(data->clk_sec);
 	/* Find which sensor generated this interrupt */
 	if (reg->tmu_irqstatus) {
 		val_type = readl(data->base_second + reg->tmu_irqstatus);
 		if (!((val_type >> data->id) & 0x1))
 			goto out;
 	}
+	if (!IS_ERR(data->clk_sec))
+		clk_disable(data->clk_sec);
 
 	exynos_report_trigger(data->reg_conf);
 	mutex_lock(&data->lock);
@@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec");
+	if (!IS_ERR(data->clk_sec)) {
+		ret = clk_prepare(data->clk_sec);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to get clock\n");
+			return  PTR_ERR(data->clk_sec);
+		}
+	}
+
 	if (pdata->type == SOC_ARCH_EXYNOS4210 ||
 	    pdata->type == SOC_ARCH_EXYNOS4412 ||
 	    pdata->type == SOC_ARCH_EXYNOS5250 ||
@@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 	return 0;
 err_clk:
 	clk_unprepare(data->clk);
+	if (!IS_ERR(data->clk_sec))
+		clk_unprepare(data->clk_sec);
 	return ret;
 }
 
@@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)
 	exynos_unregister_thermal(data->reg_conf);
 
 	clk_unprepare(data->clk);
+	if (!IS_ERR(data->clk_sec))
+		clk_unprepare(data->clk_sec);
 
 	if (!IS_ERR(data->regulator))
 		regulator_disable(data->regulator);
-- 
1.7.10.4

Re: [PATCH] thermal: exynos: handle gate clock for misplaced TRIMINFO register

[Tomasz Figa]

Hi Naveen,

On Thursday 07 of November 2013 18:12:34 Naveen Krishna Chatradhi wrote:
> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from
> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second
> should be acompanied by enabling the respective clock.
> 
> This patch which allow for a "clk_sec" clock to be specified in the
> device-tree which will be ungated when accessing the TRIMINFO register.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c |   24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)

This patch modifies the device binding, but fails to mention anything
about the modification in respective documentation. Please do not do that.

In addition, since the series adding support for Exynos 5420 has not been
merged yet, I would rather make this patch a part of that series.

Also please see my comment below.

> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index b54825a..33edd1a 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
[snip]
> @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec");
> +	if (!IS_ERR(data->clk_sec)) {

This code does not check if the clock was specified for TMU channels that
require it, i.e. the driver will not fail if you don't specify this clock.

Instead, it would be better create a separate compatible value for such
channels, like "samsung,exynos5420-tmu-broken-triminfo", for which this
clock would be mandatory and for which, if this clock is not specified,
the driver would fail.

Best regards,
Tomasz

Re: [PATCH] thermal: exynos: handle gate clock for misplaced TRIMINFO register

[Naveen Krishna Ch]

On 7 November 2013 19:48, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Naveen,
>
> On Thursday 07 of November 2013 18:12:34 Naveen Krishna Chatradhi wrote:
>> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from
>> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second
>> should be acompanied by enabling the respective clock.
>>
>> This patch which allow for a "clk_sec" clock to be specified in the
>> device-tree which will be ungated when accessing the TRIMINFO register.
>>
>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>> ---
>>  drivers/thermal/samsung/exynos_tmu.c |   24 +++++++++++++++++++++++-
>>  1 file changed, 23 insertions(+), 1 deletion(-)
>
> This patch modifies the device binding, but fails to mention anything
> about the modification in respective documentation. Please do not do that.
Will make a note to update Documentation from now on.

>
> In addition, since the series adding support for Exynos 5420 has not been
> merged yet, I would rather make this patch a part of that series.
Will merge and upload the whole set, first thing tomorrow.
>
> Also please see my comment below.
>
>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>> index b54825a..33edd1a 100644
>> --- a/drivers/thermal/samsung/exynos_tmu.c
>> +++ b/drivers/thermal/samsung/exynos_tmu.c
> [snip]
>> @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>       if (ret)
>>               return ret;
>>
>> +     data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec");
>> +     if (!IS_ERR(data->clk_sec)) {
>
> This code does not check if the clock was specified for TMU channels that
> require it, i.e. the driver will not fail if you don't specify this clock.
I thought of making it mandatory, And TMU on Exynos5440 may not need
second clock for accessing the second base. I Will confirm with the Exynso5440
specs and update accordingly.
>
> Instead, it would be better create a separate compatible value for such
> channels, like "samsung,exynos5420-tmu-broken-triminfo", for which this
> clock would be mandatory and for which, if this clock is not specified,
> the driver would fail.
Right

Sure Tomasz,
Creating a new compatible makes handling this case really simple

Thanks for reviewing.
>
> Best regards,
> Tomasz
>



-- 
Shine bright,
(: Nav :)

Re: [PATCH] thermal: exynos: handle gate clock for misplaced TRIMINFO register

[Zhang Rui]

On Thu, 2013-11-07 at 18:12 +0530, Naveen Krishna Chatradhi wrote:
> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from
> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second
> should be acompanied by enabling the respective clock.
> 
> This patch which allow for a "clk_sec" clock to be specified in the
> device-tree which will be ungated when accessing the TRIMINFO register.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>

Eduardo,

what do you think of this patch?

thanks,
rui
> ---
>  drivers/thermal/samsung/exynos_tmu.c |   24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index b54825a..33edd1a 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -47,6 +47,7 @@
>   * @irq_work: pointer to the irq work structure.
>   * @lock: lock to implement synchronization.
>   * @clk: pointer to the clock structure.
> + * @clk_sec: pointer to the clock structure for accessing the base_second.
>   * @temp_error1: fused value of the first point trim.
>   * @temp_error2: fused value of the second point trim.
>   * @regulator: pointer to the TMU regulator structure.
> @@ -61,7 +62,7 @@ struct exynos_tmu_data {
>  	enum soc_type soc;
>  	struct work_struct irq_work;
>  	struct mutex lock;
> -	struct clk *clk;
> +	struct clk *clk, *clk_sec;
>  	u8 temp_error1, temp_error2;
>  	struct regulator *regulator;
>  	struct thermal_sensor_conf *reg_conf;
> @@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  
>  	mutex_lock(&data->lock);
>  	clk_enable(data->clk);
> +	if (!IS_ERR(data->clk_sec))
> +		clk_enable(data->clk_sec);
>  
>  	if (TMU_SUPPORTS(pdata, READY_STATUS)) {
>  		status = readb(data->base + reg->tmu_status);
> @@ -306,6 +309,8 @@ skip_calib_data:
>  out:
>  	clk_disable(data->clk);
>  	mutex_unlock(&data->lock);
> +	if (!IS_ERR(data->clk_sec))
> +		clk_disable(data->clk_sec);
>  
>  	return ret;
>  }
> @@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work)
>  	const struct exynos_tmu_registers *reg = pdata->registers;
>  	unsigned int val_irq, val_type;
>  
> +	if (!IS_ERR(data->clk_sec))
> +		clk_enable(data->clk_sec);
>  	/* Find which sensor generated this interrupt */
>  	if (reg->tmu_irqstatus) {
>  		val_type = readl(data->base_second + reg->tmu_irqstatus);
>  		if (!((val_type >> data->id) & 0x1))
>  			goto out;
>  	}
> +	if (!IS_ERR(data->clk_sec))
> +		clk_disable(data->clk_sec);
>  
>  	exynos_report_trigger(data->reg_conf);
>  	mutex_lock(&data->lock);
> @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec");
> +	if (!IS_ERR(data->clk_sec)) {
> +		ret = clk_prepare(data->clk_sec);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Failed to get clock\n");
> +			return  PTR_ERR(data->clk_sec);
> +		}
> +	}
> +
>  	if (pdata->type == SOC_ARCH_EXYNOS4210 ||
>  	    pdata->type == SOC_ARCH_EXYNOS4412 ||
>  	    pdata->type == SOC_ARCH_EXYNOS5250 ||
> @@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>  	return 0;
>  err_clk:
>  	clk_unprepare(data->clk);
> +	if (!IS_ERR(data->clk_sec))
> +		clk_unprepare(data->clk_sec);
>  	return ret;
>  }
>  
> @@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)
>  	exynos_unregister_thermal(data->reg_conf);
>  
>  	clk_unprepare(data->clk);
> +	if (!IS_ERR(data->clk_sec))
> +		clk_unprepare(data->clk_sec);
>  
>  	if (!IS_ERR(data->regulator))
>  		regulator_disable(data->regulator);

Re: [PATCH] thermal: exynos: handle gate clock for misplaced TRIMINFO register

[Naveen Krishna Ch]

Hello All,

On 2 January 2014 11:37, Zhang Rui <rui.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, 2013-11-07 at 18:12 +0530, Naveen Krishna Chatradhi wrote:
>> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from
>> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second
>> should be acompanied by enabling the respective clock.
>>
>> This patch which allow for a "clk_sec" clock to be specified in the
>> device-tree which will be ungated when accessing the TRIMINFO register.
>>
>> Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>
> Eduardo,
>
> what do you think of this patch?
>
> thanks,
> rui
>> ---
>>  drivers/thermal/samsung/exynos_tmu.c |   24 +++++++++++++++++++++++-
>>  1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>> index b54825a..33edd1a 100644
>> --- a/drivers/thermal/samsung/exynos_tmu.c
>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>> @@ -47,6 +47,7 @@
>>   * @irq_work: pointer to the irq work structure.
>>   * @lock: lock to implement synchronization.
>>   * @clk: pointer to the clock structure.
>> + * @clk_sec: pointer to the clock structure for accessing the base_second.
>>   * @temp_error1: fused value of the first point trim.
>>   * @temp_error2: fused value of the second point trim.
>>   * @regulator: pointer to the TMU regulator structure.
>> @@ -61,7 +62,7 @@ struct exynos_tmu_data {
>>       enum soc_type soc;
>>       struct work_struct irq_work;
>>       struct mutex lock;
>> -     struct clk *clk;
>> +     struct clk *clk, *clk_sec;
>>       u8 temp_error1, temp_error2;
>>       struct regulator *regulator;
>>       struct thermal_sensor_conf *reg_conf;
>> @@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>>
>>       mutex_lock(&data->lock);
>>       clk_enable(data->clk);
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_enable(data->clk_sec);
>>
>>       if (TMU_SUPPORTS(pdata, READY_STATUS)) {
>>               status = readb(data->base + reg->tmu_status);
>> @@ -306,6 +309,8 @@ skip_calib_data:
>>  out:
>>       clk_disable(data->clk);
>>       mutex_unlock(&data->lock);
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_disable(data->clk_sec);
>>
>>       return ret;
>>  }
>> @@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work)
>>       const struct exynos_tmu_registers *reg = pdata->registers;
>>       unsigned int val_irq, val_type;
>>
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_enable(data->clk_sec);
>>       /* Find which sensor generated this interrupt */
>>       if (reg->tmu_irqstatus) {
>>               val_type = readl(data->base_second + reg->tmu_irqstatus);
>>               if (!((val_type >> data->id) & 0x1))
>>                       goto out;
>>       }
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_disable(data->clk_sec);
>>
>>       exynos_report_trigger(data->reg_conf);
>>       mutex_lock(&data->lock);
>> @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>       if (ret)
>>               return ret;
>>
>> +     data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec");
>> +     if (!IS_ERR(data->clk_sec)) {
>> +             ret = clk_prepare(data->clk_sec);
>> +             if (ret) {
>> +                     dev_err(&pdev->dev, "Failed to get clock\n");
>> +                     return  PTR_ERR(data->clk_sec);
>> +             }
>> +     }
>> +
>>       if (pdata->type == SOC_ARCH_EXYNOS4210 ||
>>           pdata->type == SOC_ARCH_EXYNOS4412 ||
>>           pdata->type == SOC_ARCH_EXYNOS5250 ||
>> @@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>       return 0;
>>  err_clk:
>>       clk_unprepare(data->clk);
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_unprepare(data->clk_sec);
>>       return ret;
>>  }
>>
>> @@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)
>>       exynos_unregister_thermal(data->reg_conf);
>>
>>       clk_unprepare(data->clk);
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_unprepare(data->clk_sec);
>>
>>       if (!IS_ERR(data->regulator))
>>               regulator_disable(data->regulator);
>
>
Ping.



-- 
Shine bright,
(: Nav :)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] thermal: exynos: handle gate clock for misplaced TRIMINFO register

[Mark Rutland]

On Thu, Nov 07, 2013 at 12:42:34PM +0000, Naveen Krishna Chatradhi wrote:
> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from
> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second
> should be acompanied by enabling the respective clock.
> 
> This patch which allow for a "clk_sec" clock to be specified in the
> device-tree which will be ungated when accessing the TRIMINFO register.

Missing binding document update? Or was "clk_sec" originally in the
binding but unused? 

The code seems to expect "tmu_apbif_sec" as the clock name in the DT,
but this isn't mentioned in the commit message. 

I grepped Documentation/devicetree in mainline, but found no reference
of either.

Thanks,
Mark.

> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c |   24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index b54825a..33edd1a 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -47,6 +47,7 @@
>   * @irq_work: pointer to the irq work structure.
>   * @lock: lock to implement synchronization.
>   * @clk: pointer to the clock structure.
> + * @clk_sec: pointer to the clock structure for accessing the base_second.
>   * @temp_error1: fused value of the first point trim.
>   * @temp_error2: fused value of the second point trim.
>   * @regulator: pointer to the TMU regulator structure.
> @@ -61,7 +62,7 @@ struct exynos_tmu_data {
>  	enum soc_type soc;
>  	struct work_struct irq_work;
>  	struct mutex lock;
> -	struct clk *clk;
> +	struct clk *clk, *clk_sec;
>  	u8 temp_error1, temp_error2;
>  	struct regulator *regulator;
>  	struct thermal_sensor_conf *reg_conf;
> @@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  
>  	mutex_lock(&data->lock);
>  	clk_enable(data->clk);
> +	if (!IS_ERR(data->clk_sec))
> +		clk_enable(data->clk_sec);
>  
>  	if (TMU_SUPPORTS(pdata, READY_STATUS)) {
>  		status = readb(data->base + reg->tmu_status);
> @@ -306,6 +309,8 @@ skip_calib_data:
>  out:
>  	clk_disable(data->clk);
>  	mutex_unlock(&data->lock);
> +	if (!IS_ERR(data->clk_sec))
> +		clk_disable(data->clk_sec);
>  
>  	return ret;
>  }
> @@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work)
>  	const struct exynos_tmu_registers *reg = pdata->registers;
>  	unsigned int val_irq, val_type;
>  
> +	if (!IS_ERR(data->clk_sec))
> +		clk_enable(data->clk_sec);
>  	/* Find which sensor generated this interrupt */
>  	if (reg->tmu_irqstatus) {
>  		val_type = readl(data->base_second + reg->tmu_irqstatus);
>  		if (!((val_type >> data->id) & 0x1))
>  			goto out;
>  	}
> +	if (!IS_ERR(data->clk_sec))
> +		clk_disable(data->clk_sec);
>  
>  	exynos_report_trigger(data->reg_conf);
>  	mutex_lock(&data->lock);
> @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec");
> +	if (!IS_ERR(data->clk_sec)) {
> +		ret = clk_prepare(data->clk_sec);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Failed to get clock\n");
> +			return  PTR_ERR(data->clk_sec);
> +		}
> +	}
> +
>  	if (pdata->type == SOC_ARCH_EXYNOS4210 ||
>  	    pdata->type == SOC_ARCH_EXYNOS4412 ||
>  	    pdata->type == SOC_ARCH_EXYNOS5250 ||
> @@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>  	return 0;
>  err_clk:
>  	clk_unprepare(data->clk);
> +	if (!IS_ERR(data->clk_sec))
> +		clk_unprepare(data->clk_sec);
>  	return ret;
>  }
>  
> @@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)
>  	exynos_unregister_thermal(data->reg_conf);
>  
>  	clk_unprepare(data->clk);
> +	if (!IS_ERR(data->clk_sec))
> +		clk_unprepare(data->clk_sec);
>  
>  	if (!IS_ERR(data->regulator))
>  		regulator_disable(data->regulator);
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] thermal: exynos: handle gate clock for misplaced TRIMINFO register

[Naveen Krishna Ch]

Hello Mark,

On 10 February 2014 16:03, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Nov 07, 2013 at 12:42:34PM +0000, Naveen Krishna Chatradhi wrote:
>> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from
>> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second
>> should be acompanied by enabling the respective clock.
>>
>> This patch which allow for a "clk_sec" clock to be specified in the
>> device-tree which will be ungated when accessing the TRIMINFO register.
>
> Missing binding document update? Or was "clk_sec" originally in the
> binding but unused?
>
> The code seems to expect "tmu_apbif_sec" as the clock name in the DT,
> but this isn't mentioned in the commit message.
>
> I grepped Documentation/devicetree in mainline, but found no reference
> of either.
>
> Thanks,
> Mark.
This CL is to be abandoned.

As mentioned in the previous replies to this patch.
The changes in this patched were merged with
http://www.spinics.net/lists/devicetree/msg15165.html

The latest patch set can be found at.
1. http://www.spinics.net/lists/devicetree/msg15163.html
2. http://www.spinics.net/lists/devicetree/msg15164.html
3. http://www.spinics.net/lists/devicetree/msg15165.html
4. http://www.spinics.net/lists/devicetree/msg15165.html

Thanks for your time.
>
>>
>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>> ---
>>  drivers/thermal/samsung/exynos_tmu.c |   24 +++++++++++++++++++++++-
>>  1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>> index b54825a..33edd1a 100644
>> --- a/drivers/thermal/samsung/exynos_tmu.c
>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>> @@ -47,6 +47,7 @@
>>   * @irq_work: pointer to the irq work structure.
>>   * @lock: lock to implement synchronization.
>>   * @clk: pointer to the clock structure.
>> + * @clk_sec: pointer to the clock structure for accessing the base_second.
>>   * @temp_error1: fused value of the first point trim.
>>   * @temp_error2: fused value of the second point trim.
>>   * @regulator: pointer to the TMU regulator structure.
>> @@ -61,7 +62,7 @@ struct exynos_tmu_data {
>>       enum soc_type soc;
>>       struct work_struct irq_work;
>>       struct mutex lock;
>> -     struct clk *clk;
>> +     struct clk *clk, *clk_sec;
>>       u8 temp_error1, temp_error2;
>>       struct regulator *regulator;
>>       struct thermal_sensor_conf *reg_conf;
>> @@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>>
>>       mutex_lock(&data->lock);
>>       clk_enable(data->clk);
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_enable(data->clk_sec);
>>
>>       if (TMU_SUPPORTS(pdata, READY_STATUS)) {
>>               status = readb(data->base + reg->tmu_status);
>> @@ -306,6 +309,8 @@ skip_calib_data:
>>  out:
>>       clk_disable(data->clk);
>>       mutex_unlock(&data->lock);
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_disable(data->clk_sec);
>>
>>       return ret;
>>  }
>> @@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work)
>>       const struct exynos_tmu_registers *reg = pdata->registers;
>>       unsigned int val_irq, val_type;
>>
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_enable(data->clk_sec);
>>       /* Find which sensor generated this interrupt */
>>       if (reg->tmu_irqstatus) {
>>               val_type = readl(data->base_second + reg->tmu_irqstatus);
>>               if (!((val_type >> data->id) & 0x1))
>>                       goto out;
>>       }
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_disable(data->clk_sec);
>>
>>       exynos_report_trigger(data->reg_conf);
>>       mutex_lock(&data->lock);
>> @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>       if (ret)
>>               return ret;
>>
>> +     data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec");
>> +     if (!IS_ERR(data->clk_sec)) {
>> +             ret = clk_prepare(data->clk_sec);
>> +             if (ret) {
>> +                     dev_err(&pdev->dev, "Failed to get clock\n");
>> +                     return  PTR_ERR(data->clk_sec);
>> +             }
>> +     }
>> +
>>       if (pdata->type == SOC_ARCH_EXYNOS4210 ||
>>           pdata->type == SOC_ARCH_EXYNOS4412 ||
>>           pdata->type == SOC_ARCH_EXYNOS5250 ||
>> @@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>       return 0;
>>  err_clk:
>>       clk_unprepare(data->clk);
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_unprepare(data->clk_sec);
>>       return ret;
>>  }
>>
>> @@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)
>>       exynos_unregister_thermal(data->reg_conf);
>>
>>       clk_unprepare(data->clk);
>> +     if (!IS_ERR(data->clk_sec))
>> +             clk_unprepare(data->clk_sec);
>>
>>       if (!IS_ERR(data->regulator))
>>               regulator_disable(data->regulator);
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>



-- 
Shine bright,
(: Nav :)

Re: [PATCH] thermal: exynos: handle gate clock for misplaced TRIMINFO register

[Mark Rutland]

On Mon, Feb 10, 2014 at 10:50:01AM +0000, Naveen Krishna Ch wrote:
> Hello Mark,
> 
> On 10 February 2014 16:03, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Nov 07, 2013 at 12:42:34PM +0000, Naveen Krishna Chatradhi wrote:
> >> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from
> >> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second
> >> should be acompanied by enabling the respective clock.
> >>
> >> This patch which allow for a "clk_sec" clock to be specified in the
> >> device-tree which will be ungated when accessing the TRIMINFO register.
> >
> > Missing binding document update? Or was "clk_sec" originally in the
> > binding but unused?
> >
> > The code seems to expect "tmu_apbif_sec" as the clock name in the DT,
> > but this isn't mentioned in the commit message.
> >
> > I grepped Documentation/devicetree in mainline, but found no reference
> > of either.
> >
> > Thanks,
> > Mark.
> This CL is to be abandoned.

Ok.

> 
> As mentioned in the previous replies to this patch.
> The changes in this patched were merged with
> http://www.spinics.net/lists/devicetree/msg15165.html
> 
> The latest patch set can be found at.
> 1. http://www.spinics.net/lists/devicetree/msg15163.html
> 2. http://www.spinics.net/lists/devicetree/msg15164.html
> 3. http://www.spinics.net/lists/devicetree/msg15165.html
> 4. http://www.spinics.net/lists/devicetree/msg15165.html

I responded here because of your ping message on 2014-02-07. The latest
patches seem to have been posted before that. Was the ping misplaced or
have I misunderstood?

Thanks,
Mark

Re: [PATCH] thermal: exynos: handle gate clock for misplaced TRIMINFO register

[Naveen Krishna Ch]

Hello Mark,

On 10 February 2014 16:37, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> On Mon, Feb 10, 2014 at 10:50:01AM +0000, Naveen Krishna Ch wrote:
>> Hello Mark,
>>
>> On 10 February 2014 16:03, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
>> > On Thu, Nov 07, 2013 at 12:42:34PM +0000, Naveen Krishna Chatradhi wrote:
>> >> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from
>> >> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second
>> >> should be acompanied by enabling the respective clock.
>> >>
>> >> This patch which allow for a "clk_sec" clock to be specified in the
>> >> device-tree which will be ungated when accessing the TRIMINFO register.
>> >
>> > Missing binding document update? Or was "clk_sec" originally in the
>> > binding but unused?
>> >
>> > The code seems to expect "tmu_apbif_sec" as the clock name in the DT,
>> > but this isn't mentioned in the commit message.
>> >
>> > I grepped Documentation/devicetree in mainline, but found no reference
>> > of either.
>> >
>> > Thanks,
>> > Mark.
>> This CL is to be abandoned.
>
> Ok.
>
>>
>> As mentioned in the previous replies to this patch.
>> The changes in this patched were merged with
>> http://www.spinics.net/lists/devicetree/msg15165.html
>>
>> The latest patch set can be found at.
>> 1. http://www.spinics.net/lists/devicetree/msg15163.html
>> 2. http://www.spinics.net/lists/devicetree/msg15164.html
>> 3. http://www.spinics.net/lists/devicetree/msg15165.html
>> 4. http://www.spinics.net/lists/devicetree/msg15165.html
>
> I responded here because of your ping message on 2014-02-07. The latest
> patches seem to have been posted before that. Was the ping misplaced or
> have I misunderstood?
It was my bad, I replied to the patches in that series and i myself forgot that
it was abandoned patch. Sorry.
>
> Thanks,
> Mark



-- 
Shine bright,
(: Nav :)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html