thermal: exynos: add optional sclk support
Subsystems (4)
PATCH 1/1 thermal: exynos: add optional sclk support
Exynos7 has a special clock required for the functional operation of the TMU that is not present in earlier SoCs. Add support for this optional clock and update the binding documentation.
---
This patch was earlier part of the series adding TMU support for Exynos7 [1]. It has been split out as it does not impact the on-going consolidation in the exynos tmu driver and can be considered independently.
.../devicetree/bindings/thermal/exynos-thermal.txt | 3 +++ drivers/thermal/samsung/exynos_tmu.c | 27 ++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/thermal/exynos-thermal.txt +3 -0
@@ -32,10 +32,13 @@ - clocks : The main clocks for TMU device -- 1. operational clock for TMU channel -- 2. optional clock to access the shared registers of TMU channel + -- 3. optional special clock for functional operation - clock-names : Thermal system clock name -- "tmu_apbif" operational clock for current TMU channel -- "tmu_triminfo_apbif" clock to access the shared triminfo register for current TMU channel + -- "tmu_sclk" clock for functional operation of the current TMU + channel - vtmu-supply: This entry is optional and provides the regulator node supplying voltage to TMU. If needed this entry can be placed inside board/platform specific dts file.
drivers/thermal/samsung/exynos_tmu.c +22 -5
@@ -128,6 +128,7 @@* @lock: lock to implement synchronization. * @clk: pointer to the clock structure. * @clk_sec: pointer to the clock structure for accessing the base_second. +* @sclk: pointer to the clock structure for accessing the tmu special clk. * @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.
@@ -147,7 +148,7 @@ struct exynos_tmu_data {enum soc_type soc;struct work_struct irq_work;struct mutex lock; -struct clk *clk, *clk_sec; +struct clk *clk, *clk_sec, *sclk;u8 temp_error1, temp_error2;struct regulator *regulator;struct thermal_sensor_conf *reg_conf;
@@ -883,10 +884,21 @@ static int exynos_tmu_probe(struct platform_device *pdev) goto err_clk_sec; } + data->sclk =devm_clk_get (&pdev->dev,"tmu_sclk" ); +if (IS_ERR (data->sclk)) { +dev_err (&pdev->dev,"Failed to get optional special clock\n" ); + }else { + ret =clk_prepare_enable (data->sclk); +if (ret) { +dev_err (&pdev->dev,"Failed to enable special clock\n" ); + goto err_clk; + } + } + ret =exynos_tmu_initialize (pdev);if (ret) {dev_err (&pdev->dev,"Failed to initialize TMU\n" ); - goto err_clk; + goto err_sclk; }exynos_tmu_control (pdev, true);
@@ -896,7 +908,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) sizeof(struct thermal_sensor_conf), GFP_KERNEL);if (!sensor_conf) { ret = -ENOMEM ; - goto err_clk; + goto err_sclk; }sprintf (sensor_conf->name,"therm_zone%d" , data->id); sensor_conf->read_temperature = (int (*)(void *))exynos_tmu_read;
@@ -928,7 +940,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) ret =exynos_register_thermal (sensor_conf);if (ret) {dev_err (&pdev->dev,"Failed to register thermal interface\n" ); - goto err_clk; + goto err_sclk; } data->reg_conf = sensor_conf;
@@ -936,10 +948,13 @@ static int exynos_tmu_probe(struct platform_device *pdev) IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data);if (ret) {dev_err (&pdev->dev,"Failed to request irq: %d\n" , data->irq); - goto err_clk; + goto err_sclk; }return 0 ; +err_sclk: +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk); err_clk:clk_unprepare (data->clk); err_clk_sec:
@@ -956,6 +971,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)exynos_tmu_control (pdev, false); +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);clk_unprepare (data->clk);if (!IS_ERR (data->clk_sec))clk_unprepare (data->clk_sec);
Signature · 1 line
1.7.9.5
PATCH 1/1 thermal: exynos: add optional sclk support
Hi Abhilash,
Quoted 4 lines · Abhilash Kesavan
Exynos7 has a special clock required for the functional operation of the TMU that is not present in earlier SoCs. Add support for this optional clock and update the binding documentation.
Latest Exynos SoC needs the special clocks. It is different part from previous Exynos SoC.
Exynos3250 must need the special clock for ADC IP like this patch. So, I sent the smiliar patch[1] to support SCLK as following and merged it.
[1] https://lkml.org/lkml/2014/7/21/734
So, I suggest you that Exynos would use the similar method to support special clock
on all of IPs for Exynos SoC.
Quoted 51 lines
Signed-off-by Abhilash Kesavan <[email protected]>---
This patch was earlier part of the series adding TMU support for Exynos7 [1]. It has been split out as it does not impact the on-going consolidation in the exynos tmu driver and can be considered independently..../devicetree/bindings/thermal/exynos-thermal.txt | 3 +++ drivers/thermal/samsung/exynos_tmu.c | 27 ++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-)Documentation/devicetree/bindings/thermal/exynos-thermal.txt +3
-0@@ -32,10 +32,13 @@ - clocks : The main clocks for TMU device -- 1. operational clock for TMU channel -- 2. optional clock to access the shared registers of TMU channel + -- 3. optional special clock for functional operation - clock-names : Thermal system clock name -- "tmu_apbif" operational clock for current TMU channel -- "tmu_triminfo_apbif" clock to access the shared triminfo register for current TMU channel + -- "tmu_sclk" clock for functional operation of the current TMU + channel - vtmu-supply: This entry is optional and provides the regulator node supplying voltage to TMU. If needed this entry can be placed inside board/platform specific dts file.drivers/thermal/samsung/exynos_tmu.c +2
-1@@ -128,6 +128,7 @@* @lock: lock to implement synchronization. * @clk: pointer to the clock structure. * @clk_sec: pointer to the clock structure for accessing the base_second. +* @sclk: pointer to the clock structure for accessing the tmu special clk. * @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. @@ -147,7 +148,7 @@ struct exynos_tmu_data {enum soc_type soc;struct work_struct irq_work;struct mutex lock; -struct clk *clk, *clk_sec; +struct clk *clk, *clk_sec, *sclk;u8 temp_error1, temp_error2;struct regulator *regulator;struct thermal_sensor_conf *reg_conf;
drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct platform_device *pdev) · line 889
Quoted 6 lines
drivers/thermal/samsung/exynos_tmu.c +3
-0@@ -883,10 +884,21 @@ static int exynos_tmu_probe(struct platform_device *pdev) goto err_clk_sec; } + data->sclk =devm_clk_get (&pdev->dev,"tmu_sclk" ); +if (IS_ERR (data->sclk)) { +dev_err (&pdev->dev,"Failed to get optional special clock\n" );
Exynos4, Exynos3250 etc may show error message always. I think It is not proper. I recommend that you use additional 'needs_sclk' field. If 'needs_sclk' is true, tmu driver will get the special clock of tmu without error message.
Also, How about just 'sclk' instead of 'tmu_sclk'?
I discussed the name of 'sclk' with Arnd Bergmann on following patch[2]
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/273557.html
Quoted 36 lines
drivers/thermal/samsung/exynos_tmu.c +11
-3+ }else { + ret =clk_prepare_enable (data->sclk); +if (ret) { +dev_err (&pdev->dev,"Failed to enable special clock\n" ); + goto err_clk; + } + } + ret =exynos_tmu_initialize (pdev);if (ret) {dev_err (&pdev->dev,"Failed to initialize TMU\n" ); - goto err_clk; + goto err_sclk; }exynos_tmu_control (pdev, true);@@ -896,7 +908,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) sizeof(struct thermal_sensor_conf), GFP_KERNEL);if (!sensor_conf) { ret = -ENOMEM ; - goto err_clk; + goto err_sclk; }sprintf (sensor_conf->name,"therm_zone%d" , data->id); sensor_conf->read_temperature = (int (*)(void *))exynos_tmu_read;@@ -928,7 +940,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) ret =exynos_register_thermal (sensor_conf);if (ret) {dev_err (&pdev->dev,"Failed to register thermal interface\n" ); - goto err_clk; + goto err_sclk; } data->reg_conf = sensor_conf;@@ -936,10 +948,13 @@ static int exynos_tmu_probe(struct platform_device *pdev) IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data);
drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct platform_device *pdev) · line 957
Quoted 10 lines
drivers/thermal/samsung/exynos_tmu.c +4
-1if (ret) {dev_err (&pdev->dev,"Failed to request irq: %d\n" , data->irq); - goto err_clk; + goto err_sclk; }return 0 ; +err_sclk: +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);
I think IS_ERROR don't be necessary because clk_disalbe_unprepare check the NULL pointer of clock pointer.
drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_remove(struct platform_device *pdev) · line 975
Quoted 9 lines
drivers/thermal/samsung/exynos_tmu.c +2
-0err_clk:clk_unprepare (data->clk); err_clk_sec:@@ -956,6 +971,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)exynos_tmu_control (pdev, false); +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);
ditto.
Best Regards,
Chanwoo Choi
Quoted 11 lines
drivers/thermal/samsung/exynos_tmu.c +0
-1clk_unprepare (data->clk);if (!IS_ERR (data->clk_sec))clk_unprepare (data->clk_sec); --1.7.9.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 thermal: exynos: add optional sclk support
Hi Chanwoo,
Quoted 8 lines · Chanwoo Choi
Hi Abhilash,
Quoted 4 lines · Abhilash Kesavan
Exynos7 has a special clock required for the functional operation of the TMU that is not present in earlier SoCs. Add support for this optional clock and update the binding documentation.
Quoted 6 lines
Latest Exynos SoC needs the special clocks. It is different part from previous Exynos SoC.
Exynos3250 must need the special clock for ADC IP like this patch. So, I sent the smiliar patch[1] to support SCLK as following and merged it.
Thanks for the link.
Quoted 59 lines
So, I suggest you that Exynos would use the similar method to support special clock
on all of IPs for Exynos SoC.Quoted 54 lines
Signed-off-by Abhilash Kesavan <[email protected]>---
This patch was earlier part of the series adding TMU support for Exynos7 [1]. It has been split out as it does not impact the on-going consolidation in the exynos tmu driver and can be considered independently..../devicetree/bindings/thermal/exynos-thermal.txt | 3 +++ drivers/thermal/samsung/exynos_tmu.c | 27 ++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-)Documentation/devicetree/bindings/thermal/exynos-thermal.txt +3
-0@@ -32,10 +32,13 @@ - clocks : The main clocks for TMU device -- 1. operational clock for TMU channel -- 2. optional clock to access the shared registers of TMU channel + -- 3. optional special clock for functional operation - clock-names : Thermal system clock name -- "tmu_apbif" operational clock for current TMU channel -- "tmu_triminfo_apbif" clock to access the shared triminfo register for current TMU channel + -- "tmu_sclk" clock for functional operation of the current TMU + channel - vtmu-supply: This entry is optional and provides the regulator node supplying voltage to TMU. If needed this entry can be placed inside board/platform specific dts file.drivers/thermal/samsung/exynos_tmu.c +2
-1@@ -128,6 +128,7 @@* @lock: lock to implement synchronization. * @clk: pointer to the clock structure. * @clk_sec: pointer to the clock structure for accessing the base_second. +* @sclk: pointer to the clock structure for accessing the tmu special clk. * @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. @@ -147,7 +148,7 @@ struct exynos_tmu_data {enum soc_type soc;struct work_struct irq_work;struct mutex lock; -struct clk *clk, *clk_sec; +struct clk *clk, *clk_sec, *sclk;u8 temp_error1, temp_error2;struct regulator *regulator;struct thermal_sensor_conf *reg_conf;@@ -883,10 +884,21 @@ static int exynos_tmu_probe(struct platform_device *pdev) goto err_clk_sec; }
Quoted 7 lines
drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct platform_device *pdev) · line 889
Quoted 3 lines
drivers/thermal/samsung/exynos_tmu.c +3
-0+ data->sclk =devm_clk_get (&pdev->dev,"tmu_sclk" ); +if (IS_ERR (data->sclk)) { +dev_err (&pdev->dev,"Failed to get optional special clock\n" );Exynos4, Exynos3250 etc may show error message always. I think It is not proper. I recommend that you use additional '
needs_sclk' field. If 'needs_sclk' is true, tmu driver will get the special clock of tmu without error message.
OK, I'll add a per-soc field to check for sclk availability.
Quoted 3 lines
Also, How about just 'sclk' instead of '
tmu_sclk'?
I discussed the name of 'sclk' with Arnd Bergmann on following patch[2]
Sure, I'll use 'sclk' instead of 'tmu_sclk'.
Quoted 58 lines
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/273557.html
Quoted 36 lines
drivers/thermal/samsung/exynos_tmu.c +11
-3+ }else { + ret =clk_prepare_enable (data->sclk); +if (ret) { +dev_err (&pdev->dev,"Failed to enable special clock\n" ); + goto err_clk; + } + } + ret =exynos_tmu_initialize (pdev);if (ret) {dev_err (&pdev->dev,"Failed to initialize TMU\n" ); - goto err_clk; + goto err_sclk; }exynos_tmu_control (pdev, true);@@ -896,7 +908,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) sizeof(struct thermal_sensor_conf), GFP_KERNEL);if (!sensor_conf) { ret = -ENOMEM ; - goto err_clk; + goto err_sclk; }sprintf (sensor_conf->name,"therm_zone%d" , data->id); sensor_conf->read_temperature = (int (*)(void *))exynos_tmu_read;@@ -928,7 +940,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) ret =exynos_register_thermal (sensor_conf);if (ret) {dev_err (&pdev->dev,"Failed to register thermal interface\n" ); - goto err_clk; + goto err_sclk; } data->reg_conf = sensor_conf;@@ -936,10 +948,13 @@ static int exynos_tmu_probe(struct platform_device *pdev) IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data);drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct platform_device *pdev) · line 957
Quoted 10 lines
drivers/thermal/samsung/exynos_tmu.c +4
-1if (ret) {dev_err (&pdev->dev,"Failed to request irq: %d\n" , data->irq); - goto err_clk; + goto err_sclk; }return 0 ; +err_sclk: +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);I think
IS_ERRORdon't be necessary becauseclk_disalbe_unpreparecheck the NULL pointer of clock pointer.Quoted 5 lines
drivers/thermal/samsung/exynos_tmu.c +0
-0err_clk:clk_unprepare (data->clk); err_clk_sec:@@ -956,6 +971,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)
Quoted 6 lines
drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_remove(struct platform_device *pdev) · line 975
Quoted 4 lines
drivers/thermal/samsung/exynos_tmu.c +2
-0exynos_tmu_control (pdev, false); +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);ditto.
Will fix.
Thanks for the review.
Regards,
Abhilash
Quoted 15 lines
Best Regards,
Chanwoo ChoiQuoted 11 lines
drivers/thermal/samsung/exynos_tmu.c +0
-1clk_unprepare (data->clk);if (!IS_ERR (data->clk_sec))clk_unprepare (data->clk_sec); --1.7.9.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 thermal: exynos: add optional sclk support
Hi Abhilash,
Quoted 99 lines
Hi Chanwoo,
Quoted 9 lines · Chanwoo Choi
Hi Abhilash,
Quoted 4 lines · Abhilash Kesavan
Exynos7 has a special clock required for the functional operation of the TMU that is not present in earlier SoCs. Add support for this optional clock and update the binding documentation.
Quoted 7 lines
Latest Exynos SoC needs the special clocks. It is different part from previous Exynos SoC.
Exynos3250 must need the special clock for ADC IP like this patch. So, I sent the smiliar patch[1] to support SCLK as following and merged it.Thanks for the link.
Quoted 66 lines
So, I suggest you that Exynos would use the similar method to support special clock
on all of IPs for Exynos SoC.Quoted 61 lines
Signed-off-by Abhilash Kesavan <[email protected]>---
This patch was earlier part of the series adding TMU support for Exynos7 [1]. It has been split out as it does not impact the on-going consolidation in the exynos tmu driver and can be considered independently..../devicetree/bindings/thermal/exynos-thermal.txt | 3 +++ drivers/thermal/samsung/exynos_tmu.c | 27++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-)
diff --git
a/Documentation/devicetree/bindings/thermal/exynos-thermal.txtb/Documentation/devicetree/bindings/thermal/exynos-thermal.txtindex ae738f5..2393eac 100644 ---a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt+++b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt@@ -32,10 +32,13 @@
- clocks : The main clocks for TMU device
-- 1. operational clock for TMU channel
-- 2. optional clock to access the shared registers of TMU
channel
+ -- 3. optional special clock for functional operation
- clock-names : Thermal system clock name
-- "tmu_apbif" operational clock for current TMU channel -- "tmu_triminfo_apbif" clock to access the shared
triminfo register for current TMU channel
+ -- "tmu_sclk" clock for functional operation of the
current TMU
+ channel
- vtmu-supply: This entry is optional and provides the regulator
node supplying voltage to TMU. If needed this entry can be placed inside board/platform specific dts file.(quoted diff) +0
-0
b/drivers/thermal/samsung/exynos_tmu.cindex d44d91d..6627937 100644 ---a/drivers/thermal/samsung/exynos_tmu.c+++b/drivers/thermal/samsung/exynos_tmu.cdrivers/thermal/samsung/exynos_tmu.c +0
-0@@ -128,6 +128,7 @@* @lock: lock to implement synchronization. * @clk: pointer to the clock structure. * @clk_sec: pointer to the clock structure for accessing the
base_second.
+ * @sclk: pointer to the clock structure for accessing the tmu special clk.
* @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.drivers/thermal/samsung/exynos_tmu.c +1
-1@@ -147,7 +148,7 @@ struct exynos_tmu_data {enum soc_type soc;struct work_struct irq_work;struct mutex lock; -struct clk *clk, *clk_sec; +struct clk *clk, *clk_sec, *sclk;u8 temp_error1, temp_error2;struct regulator *regulator;struct thermal_sensor_conf *reg_conf;@@ -883,10 +884,21 @@ static int exynos_tmu_probe(struct platform_device *pdev) goto err_clk_sec; }Quoted 9 lines
drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct platform_device *pdev)
Quoted 4 lines
drivers/thermal/samsung/exynos_tmu.c +3
-0+ data->sclk =devm_clk_get (&pdev->dev,"tmu_sclk" ); + if (IS_ERR(data->sclk)) { + dev_err(&pdev->dev, "Failed to get optional special clock\n");Exynos4, Exynos3250 etc may show error message always. I think It is not proper. I recommend that you use additional '
needs_sclk' field. If 'needs_sclk' is true, tmu driver will get the special clock of tmu without error message.
Quoted 1 line
OK, I'll add a per-soc field to check for sclk availability.
I think that it would be enough to change dev_err() -> dev_dbg().
Quoted 99 lines
Quoted 5 lines
Also, How about just 'sclk' instead of '
tmu_sclk'?
I discussed the name of 'sclk' with Arnd Bergmann on following patch[2]Sure, I'll use 'sclk' instead of '
tmu_sclk'.Quoted 63 lines
[2]
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/273557.htmlQuoted 39 lines
drivers/thermal/samsung/exynos_tmu.c +9
-1+ }else { + ret =clk_prepare_enable (data->sclk); +if (ret) { + dev_err(&pdev->dev, "Failed to enable special clock\n"); + goto err_clk; + } + } + ret =exynos_tmu_initialize (pdev);if (ret) {dev_err (&pdev->dev,"Failed to initialize TMU\n" ); - goto err_clk; + goto err_sclk; }exynos_tmu_control(pdev, true);drivers/thermal/samsung/exynos_tmu.c +1
-1@@ -896,7 +908,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) sizeof(struct thermal_sensor_conf), GFP_KERNEL); if (!sensor_conf) { ret = -ENOMEM ; - goto err_clk; + goto err_sclk; }sprintf (sensor_conf->name,"therm_zone%d" , data->id); sensor_conf->read_temperature = (int (*)(void*))exynos_tmu_read; @@ -928,7 +940,7 @@ static int
exynos_tmu_probe(struct platform_device *pdev)ret =exynos_register_thermal(sensor_conf); if (ret) { dev_err(&pdev->dev, "Failed to register thermalinterface\n");
drivers/thermal/samsung/exynos_tmu.c +1
-1- goto err_clk; + goto err_sclk; } data->reg_conf = sensor_conf;@@ -936,10 +948,13 @@ static int exynos_tmu_probe(struct platform_device *pdev) IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data); if (ret) {drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct · line 958
Quoted 10 lines
drivers/thermal/samsung/exynos_tmu.c +4
-1dev_err (&pdev->dev,"Failed to request irq: %d\n" , data->irq); - goto err_clk; + goto err_sclk; }return 0 ; +err_sclk: +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);I think
IS_ERRORdon't be necessary becauseclk_disalbe_unpreparecheck the NULL pointer of clock pointer.Quoted 6 lines
drivers/thermal/samsung/exynos_tmu.c +0
-0err_clk:clk_unprepare (data->clk);
err_clk_sec:(quoted diff) +0
-0@@ -956,6 +971,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)Quoted 6 lines
drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_remove(struct platform_device *pdev) · line 975
Quoted 4 lines
drivers/thermal/samsung/exynos_tmu.c +2
-0exynos_tmu_control (pdev, false); +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);ditto.
Will fix.
Thanks for the review.
Regards,
AbhilashQuoted 15 lines
Best Regards,
Chanwoo ChoiQuoted 11 lines
drivers/thermal/samsung/exynos_tmu.c +0
-0clk_unprepare (data->clk);if (!IS_ERR (data->clk_sec))clk_unprepare(data->clk_sec);--
1.7.9.5_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 5 lines
Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
PATCH 1/1 thermal: exynos: add optional sclk support
Hi Lukasz,
Quoted 101 lines · Lukasz Majewski
Hi Abhilash,
Quoted 99 lines
Hi Chanwoo,
Quoted 9 lines · Chanwoo Choi
Hi Abhilash,
Quoted 4 lines · Abhilash Kesavan
Exynos7 has a special clock required for the functional operation of the TMU that is not present in earlier SoCs. Add support for this optional clock and update the binding documentation.
Quoted 7 lines
Latest Exynos SoC needs the special clocks. It is different part from previous Exynos SoC.
Exynos3250 must need the special clock for ADC IP like this patch. So, I sent the smiliar patch[1] to support SCLK as following and merged it.Thanks for the link.
Quoted 75 lines
So, I suggest you that Exynos would use the similar method to support special clock
on all of IPs for Exynos SoC.Quoted 58 lines
Signed-off-by Abhilash Kesavan <[email protected]>---
This patch was earlier part of the series adding TMU support for Exynos7 [1]. It has been split out as it does not impact the on-going consolidation in the exynos tmu driver and can be considered independently..../devicetree/bindings/thermal/exynos-thermal.txt | 3 +++ drivers/thermal/samsung/exynos_tmu.c | 27++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-)
diff --git
a/Documentation/devicetree/bindings/thermal/exynos-thermal.txtb/Documentation/devicetree/bindings/thermal/exynos-thermal.txtindex ae738f5..2393eac 100644 ---a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt+++b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt@@ -32,10 +32,13 @@
- clocks : The main clocks for TMU device
-- 1. operational clock for TMU channel
-- 2. optional clock to access the shared registers of TMU
channel
+ -- 3. optional special clock for functional operation
- clock-names : Thermal system clock name
-- "tmu_apbif" operational clock for current TMU channel -- "tmu_triminfo_apbif" clock to access the shared
triminfo register for current TMU channel
+ -- "tmu_sclk" clock for functional operation of the
current TMU
+ channel
- vtmu-supply: This entry is optional and provides the regulator
node supplying voltage to TMU. If needed this entry can be placed inside board/platform specific dts file.(quoted diff) +0
-0
b/drivers/thermal/samsung/exynos_tmu.cindex d44d91d..6627937 100644 ---a/drivers/thermal/samsung/exynos_tmu.c+++b/drivers/thermal/samsung/exynos_tmu.cdrivers/thermal/samsung/exynos_tmu.c +0
-0@@ -128,6 +128,7 @@* @lock: lock to implement synchronization. * @clk: pointer to the clock structure. * @clk_sec: pointer to the clock structure for accessing the
base_second.
+ * @sclk: pointer to the clock structure for accessing the tmu special clk.
* @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.drivers/thermal/samsung/exynos_tmu.c +1
-1@@ -147,7 +148,7 @@ struct exynos_tmu_data {enum soc_type soc;struct work_struct irq_work;struct mutex lock; -struct clk *clk, *clk_sec; +struct clk *clk, *clk_sec, *sclk;u8 temp_error1, temp_error2;struct regulator *regulator;struct thermal_sensor_conf *reg_conf;drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct · line 890
Quoted 7 lines
drivers/thermal/samsung/exynos_tmu.c +3
-0@@ -883,10 +884,21 @@ static int exynos_tmu_probe(struct platform_device *pdev) goto err_clk_sec; } + data->sclk =devm_clk_get (&pdev->dev,"tmu_sclk" ); + if (IS_ERR(data->sclk)) { + dev_err(&pdev->dev, "Failed to get optional special clock\n");Exynos4, Exynos3250 etc may show error message always. I think It is not proper. I recommend that you use additional '
needs_sclk' field. If 'needs_sclk' is true, tmu driver will get the special clock of tmu without error message.
Quoted 3 lines
Quoted 1 line
OK, I'll add a per-soc field to check for sclk availability.
I think that it would be enough to change
dev_err()->dev_dbg().
Lukasz, thanks for the review.
Chanwoo, are you OK with this ? If so, I will post a v2 with this and the other changes you suggested.
Abhilash
Quoted 109 lines
Quoted 79 lines
Quoted 6 lines
Also, How about just 'sclk' instead of '
tmu_sclk'?
I discussed the name of 'sclk' with Arnd Bergmann on following patch[2]Sure, I'll use 'sclk' instead of '
tmu_sclk'.Quoted 63 lines
[2]
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/273557.htmlQuoted 39 lines
drivers/thermal/samsung/exynos_tmu.c +9
-1+ }else { + ret =clk_prepare_enable (data->sclk); +if (ret) { + dev_err(&pdev->dev, "Failed to enable special clock\n"); + goto err_clk; + } + } + ret =exynos_tmu_initialize (pdev);if (ret) {dev_err (&pdev->dev,"Failed to initialize TMU\n" ); - goto err_clk; + goto err_sclk; }exynos_tmu_control(pdev, true);drivers/thermal/samsung/exynos_tmu.c +1
-1@@ -896,7 +908,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) sizeof(struct thermal_sensor_conf), GFP_KERNEL); if (!sensor_conf) { ret = -ENOMEM ; - goto err_clk; + goto err_sclk; }sprintf (sensor_conf->name,"therm_zone%d" , data->id); sensor_conf->read_temperature = (int (*)(void*))exynos_tmu_read; @@ -928,7 +940,7 @@ static int
exynos_tmu_probe(struct platform_device *pdev)ret =exynos_register_thermal(sensor_conf); if (ret) { dev_err(&pdev->dev, "Failed to register thermalinterface\n");
drivers/thermal/samsung/exynos_tmu.c +1
-1- goto err_clk; + goto err_sclk; } data->reg_conf = sensor_conf;@@ -936,10 +948,13 @@ static int exynos_tmu_probe(struct platform_device *pdev) IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data); if (ret) {drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct · line 958
Quoted 10 lines
drivers/thermal/samsung/exynos_tmu.c +4
-1dev_err (&pdev->dev,"Failed to request irq: %d\n" , data->irq); - goto err_clk; + goto err_sclk; }return 0 ; +err_sclk: +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);I think
IS_ERRORdon't be necessary becauseclk_disalbe_unpreparecheck the NULL pointer of clock pointer.Quoted 6 lines
drivers/thermal/samsung/exynos_tmu.c +0
-0err_clk:clk_unprepare (data->clk);
err_clk_sec:(quoted diff) +0
-0@@ -956,6 +971,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)Quoted 6 lines
drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_remove(struct platform_device *pdev) · line 975
Quoted 4 lines
drivers/thermal/samsung/exynos_tmu.c +2
-0exynos_tmu_control (pdev, false); +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);ditto.
Quoted 21 lines
Will fix.
Thanks for the review.
Regards,
AbhilashQuoted 15 lines
Best Regards,
Chanwoo ChoiQuoted 11 lines
drivers/thermal/samsung/exynos_tmu.c +0
-0clk_unprepare (data->clk);if (!IS_ERR (data->clk_sec))clk_unprepare(data->clk_sec);--
1.7.9.5_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel--
Best regards,Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
PATCH 1/1 thermal: exynos: add optional sclk support
Hello Folks,
Quoted 104 lines · Abhilash Kesavan
Hi Lukasz,
Quoted 101 lines · Lukasz Majewski
Hi Abhilash,
Quoted 99 lines
Hi Chanwoo,
Quoted 9 lines · Chanwoo Choi
Hi Abhilash,
Quoted 4 lines · Abhilash Kesavan
Exynos7 has a special clock required for the functional operation of the TMU that is not present in earlier SoCs. Add support for this optional clock and update the binding documentation.
Quoted 7 lines
Latest Exynos SoC needs the special clocks. It is different part from previous Exynos SoC.
Exynos3250 must need the special clock for ADC IP like this patch. So, I sent the smiliar patch[1] to support SCLK as following and merged it.Thanks for the link.
Quoted 75 lines
So, I suggest you that Exynos would use the similar method to support special clock
on all of IPs for Exynos SoC.Quoted 58 lines
Signed-off-by Abhilash Kesavan <[email protected]>---
This patch was earlier part of the series adding TMU support for Exynos7 [1]. It has been split out as it does not impact the on-going consolidation in the exynos tmu driver and can be considered independently..../devicetree/bindings/thermal/exynos-thermal.txt | 3 +++ drivers/thermal/samsung/exynos_tmu.c | 27++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-)
diff --git
a/Documentation/devicetree/bindings/thermal/exynos-thermal.txtb/Documentation/devicetree/bindings/thermal/exynos-thermal.txtindex ae738f5..2393eac 100644 ---a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt+++b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt@@ -32,10 +32,13 @@
- clocks : The main clocks for TMU device
-- 1. operational clock for TMU channel
-- 2. optional clock to access the shared registers of TMU
channel
+ -- 3. optional special clock for functional operation
- clock-names : Thermal system clock name
-- "tmu_apbif" operational clock for current TMU channel -- "tmu_triminfo_apbif" clock to access the shared
triminfo register for current TMU channel
+ -- "tmu_sclk" clock for functional operation of the
current TMU
+ channel
- vtmu-supply: This entry is optional and provides the regulator
node supplying voltage to TMU. If needed this entry can be placed inside board/platform specific dts file.(quoted diff) +0
-0
b/drivers/thermal/samsung/exynos_tmu.cindex d44d91d..6627937 100644 ---a/drivers/thermal/samsung/exynos_tmu.c+++b/drivers/thermal/samsung/exynos_tmu.cdrivers/thermal/samsung/exynos_tmu.c +0
-0@@ -128,6 +128,7 @@* @lock: lock to implement synchronization. * @clk: pointer to the clock structure. * @clk_sec: pointer to the clock structure for accessing the
base_second.
+ * @sclk: pointer to the clock structure for accessing the tmu special clk.
* @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.drivers/thermal/samsung/exynos_tmu.c +1
-1@@ -147,7 +148,7 @@ struct exynos_tmu_data {enum soc_type soc;struct work_struct irq_work;struct mutex lock; -struct clk *clk, *clk_sec; +struct clk *clk, *clk_sec, *sclk;u8 temp_error1, temp_error2;struct regulator *regulator;struct thermal_sensor_conf *reg_conf;drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct · line 890
Quoted 7 lines
drivers/thermal/samsung/exynos_tmu.c +3
-0@@ -883,10 +884,21 @@ static int exynos_tmu_probe(struct platform_device *pdev) goto err_clk_sec; } + data->sclk =devm_clk_get (&pdev->dev,"tmu_sclk" ); + if (IS_ERR(data->sclk)) { + dev_err(&pdev->dev, "Failed to get optional special clock\n");Exynos4, Exynos3250 etc may show error message always. I think It is not proper. I recommend that you use additional '
needs_sclk' field. If 'needs_sclk' is true, tmu driver will get the special clock of tmu without error message.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
OK, I'll add a per-soc field to check for sclk availability.
I think that it would be enough to change
dev_err()->dev_dbg().Lukasz, thanks for the review.
Chanwoo, are you OK with this ? If so, I will post a v2 with this and the other changes you suggested.
Is this a mandatory or a optional clk? What happens if the chip has the clock, but you fail to get it in this code?
Think the rule of thumb is to at least warn the user that you are bailing to the defaults, because you failed to get the clock, when you were supposed to.
Besides, warning / logging (even if it is in debugging mode) the user that you failed to get a clock, when the board / platform does not support it, sounds at least bogus for them.
my two cents,
Quoted 111 lines
Abhilash
Quoted 109 lines
Quoted 79 lines
Quoted 6 lines
Also, How about just 'sclk' instead of '
tmu_sclk'?
I discussed the name of 'sclk' with Arnd Bergmann on following patch[2]Sure, I'll use 'sclk' instead of '
tmu_sclk'.Quoted 63 lines
[2]
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/273557.htmlQuoted 39 lines
drivers/thermal/samsung/exynos_tmu.c +9
-1+ }else { + ret =clk_prepare_enable (data->sclk); +if (ret) { + dev_err(&pdev->dev, "Failed to enable special clock\n"); + goto err_clk; + } + } + ret =exynos_tmu_initialize (pdev);if (ret) {dev_err (&pdev->dev,"Failed to initialize TMU\n" ); - goto err_clk; + goto err_sclk; }exynos_tmu_control(pdev, true);drivers/thermal/samsung/exynos_tmu.c +1
-1@@ -896,7 +908,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) sizeof(struct thermal_sensor_conf), GFP_KERNEL); if (!sensor_conf) { ret = -ENOMEM ; - goto err_clk; + goto err_sclk; }sprintf (sensor_conf->name,"therm_zone%d" , data->id); sensor_conf->read_temperature = (int (*)(void*))exynos_tmu_read; @@ -928,7 +940,7 @@ static int
exynos_tmu_probe(struct platform_device *pdev)ret =exynos_register_thermal(sensor_conf); if (ret) { dev_err(&pdev->dev, "Failed to register thermalinterface\n");
drivers/thermal/samsung/exynos_tmu.c +1
-1- goto err_clk; + goto err_sclk; } data->reg_conf = sensor_conf;@@ -936,10 +948,13 @@ static int exynos_tmu_probe(struct platform_device *pdev) IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data); if (ret) {drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct · line 958
Quoted 10 lines
drivers/thermal/samsung/exynos_tmu.c +4
-1dev_err (&pdev->dev,"Failed to request irq: %d\n" , data->irq); - goto err_clk; + goto err_sclk; }return 0 ; +err_sclk: +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);I think
IS_ERRORdon't be necessary becauseclk_disalbe_unpreparecheck the NULL pointer of clock pointer.Quoted 6 lines
drivers/thermal/samsung/exynos_tmu.c +0
-0err_clk:clk_unprepare (data->clk);
err_clk_sec:(quoted diff) +0
-0@@ -956,6 +971,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)Quoted 6 lines
drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_remove(struct platform_device *pdev) · line 975
Quoted 4 lines
drivers/thermal/samsung/exynos_tmu.c +2
-0exynos_tmu_control (pdev, false); +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);ditto.
Quoted 21 lines
Will fix.
Thanks for the review.
Regards,
AbhilashQuoted 15 lines
Best Regards,
Chanwoo ChoiQuoted 11 lines
drivers/thermal/samsung/exynos_tmu.c +0
-0clk_unprepare (data->clk);if (!IS_ERR (data->clk_sec))clk_unprepare(data->clk_sec);--
1.7.9.5_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel--
Best regards,Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141124/14448735/attachment.sig>
PATCH 1/1 thermal: exynos: add optional sclk support
Hi Eduardo,
Quoted 111 lines · Eduardo Valentin
Hello Folks,
Quoted 108 lines · Abhilash Kesavan
Hi Lukasz,
Quoted 104 lines · Lukasz Majewski
Hi Abhilash,
Quoted 99 lines
Hi Chanwoo,
Quoted 9 lines · Chanwoo Choi
Hi Abhilash,
Quoted 4 lines · Abhilash Kesavan
Exynos7 has a special clock required for the functional operation of the TMU that is not present in earlier SoCs. Add support for this optional clock and update the binding documentation.
Quoted 7 lines
Latest Exynos SoC needs the special clocks. It is different part from previous Exynos SoC.
Exynos3250 must need the special clock for ADC IP like this patch. So, I sent the smiliar patch[1] to support SCLK as following and merged it.Thanks for the link.
Quoted 66 lines
So, I suggest you that Exynos would use the similar method to support special clock
on all of IPs for Exynos SoC.Quoted 61 lines
Signed-off-by Abhilash Kesavan <[email protected]>---
This patch was earlier part of the series adding TMU support for Exynos7 [1]. It has been split out as it does not impact the on-going consolidation in the exynos tmu driver and can be considered independently..../devicetree/bindings/thermal/exynos-thermal.txt | 3 +++ drivers/thermal/samsung/exynos_tmu.c | 27++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-)
diff --git
a/Documentation/devicetree/bindings/thermal/exynos-thermal.txtb/Documentation/devicetree/bindings/thermal/exynos-thermal.txtindex ae738f5..2393eac 100644 ---a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt+++b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt@@ -32,10 +32,13 @@
- clocks : The main clocks for TMU device
-- 1. operational clock for TMU channel
-- 2. optional clock to access the shared registers of TMU
channel
+ -- 3. optional special clock for functional operation
- clock-names : Thermal system clock name
-- "tmu_apbif" operational clock for current TMU channel -- "tmu_triminfo_apbif" clock to access the shared
triminfo register for current TMU channel
+ -- "tmu_sclk" clock for functional operation of the
current TMU
+ channel
- vtmu-supply: This entry is optional and provides the regulator
node supplying voltage to TMU. If needed this entry can be placed inside board/platform specific dts file.(quoted diff) +0
-0
b/drivers/thermal/samsung/exynos_tmu.cindex d44d91d..6627937 100644 ---a/drivers/thermal/samsung/exynos_tmu.c+++b/drivers/thermal/samsung/exynos_tmu.cdrivers/thermal/samsung/exynos_tmu.c +0
-0@@ -128,6 +128,7 @@* @lock: lock to implement synchronization. * @clk: pointer to the clock structure. * @clk_sec: pointer to the clock structure for accessing the
base_second.
+ * @sclk: pointer to the clock structure for accessing the tmu special clk.
* @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.drivers/thermal/samsung/exynos_tmu.c +1
-1@@ -147,7 +148,7 @@ struct exynos_tmu_data {enum soc_type soc;struct work_struct irq_work;struct mutex lock; -struct clk *clk, *clk_sec; +struct clk *clk, *clk_sec, *sclk;u8 temp_error1, temp_error2;struct regulator *regulator;struct thermal_sensor_conf *reg_conf;@@ -883,10 +884,21 @@ static int exynos_tmu_probe(struct platform_device *pdev) goto err_clk_sec; }Quoted 9 lines
drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct platform_device *pdev)
Quoted 4 lines
drivers/thermal/samsung/exynos_tmu.c +3
-0+ data->sclk =devm_clk_get (&pdev->dev,"tmu_sclk" ); + if (IS_ERR(data->sclk)) { + dev_err(&pdev->dev, "Failed to get optional special clock\n");Exynos4, Exynos3250 etc may show error message always. I think It is not proper. I recommend that you use additional '
needs_sclk' field. If 'needs_sclk' is true, tmu driver will get the special clock of tmu without error message.Quoted 1 line
OK, I'll add a per-soc field to check for sclk availability.
I think that it would be enough to change
dev_err()->dev_dbg().
Quoted 7 lines
Quoted 4 lines
Lukasz, thanks for the review.
Chanwoo, are you OK with this ? If so, I will post a v2 with this and the other changes you suggested.
Is this a mandatory or a optional clk? What happens if the chip has the clock, but you fail to get it in this code?
This is mandatory on Exynos7 but not on the older SoCs. On Exynos7 if this clock is disabled then the TMU will not function.
Quoted 8 lines
Think the rule of thumb is to at least warn the user that you are bailing to the defaults, because you failed to get the clock, when you were supposed to.
Besides, warning / logging (even if it is in debugging mode) the user that you failed to get a clock, when the board / platform does not support it, sounds at least bogus for them.
So, you would rather that I go with Chanwoo's approach of adding a per-soc flag indicating the presence of this clock ?
Regards,
Abhilash
Quoted 115 lines
my two cents,
Quoted 111 lines
Abhilash
Quoted 109 lines
Quoted 79 lines
Quoted 6 lines
Also, How about just 'sclk' instead of '
tmu_sclk'?
I discussed the name of 'sclk' with Arnd Bergmann on following patch[2]Sure, I'll use 'sclk' instead of '
tmu_sclk'.Quoted 63 lines
[2]
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/273557.htmlQuoted 39 lines
drivers/thermal/samsung/exynos_tmu.c +9
-1+ }else { + ret =clk_prepare_enable (data->sclk); +if (ret) { + dev_err(&pdev->dev, "Failed to enable special clock\n"); + goto err_clk; + } + } + ret =exynos_tmu_initialize (pdev);if (ret) {dev_err (&pdev->dev,"Failed to initialize TMU\n" ); - goto err_clk; + goto err_sclk; }exynos_tmu_control(pdev, true);drivers/thermal/samsung/exynos_tmu.c +1
-1@@ -896,7 +908,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) sizeof(struct thermal_sensor_conf), GFP_KERNEL); if (!sensor_conf) { ret = -ENOMEM ; - goto err_clk; + goto err_sclk; }sprintf (sensor_conf->name,"therm_zone%d" , data->id); sensor_conf->read_temperature = (int (*)(void*))exynos_tmu_read; @@ -928,7 +940,7 @@ static int
exynos_tmu_probe(struct platform_device *pdev)ret =exynos_register_thermal(sensor_conf); if (ret) { dev_err(&pdev->dev, "Failed to register thermalinterface\n");
drivers/thermal/samsung/exynos_tmu.c +1
-1- goto err_clk; + goto err_sclk; } data->reg_conf = sensor_conf;@@ -936,10 +948,13 @@ static int exynos_tmu_probe(struct platform_device *pdev) IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data); if (ret) {drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_probe(struct · line 958
Quoted 10 lines
drivers/thermal/samsung/exynos_tmu.c +4
-1dev_err (&pdev->dev,"Failed to request irq: %d\n" , data->irq); - goto err_clk; + goto err_sclk; }return 0 ; +err_sclk: +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);I think
IS_ERRORdon't be necessary becauseclk_disalbe_unpreparecheck the NULL pointer of clock pointer.Quoted 6 lines
drivers/thermal/samsung/exynos_tmu.c +0
-0err_clk:clk_unprepare (data->clk);
err_clk_sec:(quoted diff) +0
-0@@ -956,6 +971,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)Quoted 6 lines
drivers/thermal/samsung/exynos_tmu.c static int exynos_tmu_remove(struct platform_device *pdev) · line 975
Quoted 4 lines
drivers/thermal/samsung/exynos_tmu.c +2
-0exynos_tmu_control (pdev, false); +if (!IS_ERR (data->sclk)) +clk_disable_unprepare (data->sclk);ditto.
Quoted 21 lines
Will fix.
Thanks for the review.
Regards,
AbhilashQuoted 15 lines
Best Regards,
Chanwoo ChoiQuoted 11 lines
drivers/thermal/samsung/exynos_tmu.c +0
-0clk_unprepare (data->clk);if (!IS_ERR (data->clk_sec))clk_unprepare(data->clk_sec);--
1.7.9.5_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel--
Best regards,Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
PATCH 1/1 thermal: exynos: add optional sclk support
Abhilash,
Hi,
On Tue, Nov 25, 2014 at 08:00:50AM +0530, Abhilash Kesavan wrote:
<big cut>
Quoted 12 lines
Quoted 8 lines
Quoted 5 lines
Lukasz, thanks for the review.
Chanwoo, are you OK with this ? If so, I will post a v2 with this and the other changes you suggested.
Is this a mandatory or a optional clk? What happens if the chip has the clock, but you fail to get it in this code?
This is mandatory on Exynos7 but not on the older SoCs. On Exynos7 if this clock is disabled then the TMU will not function.
It means, that only on Exynos7, outputing a dev_err is the right thing to do. On other SoCs versions, it does not make sense at all requesting this clock.
Quoted 5 lines
Quoted 5 lines
Think the rule of thumb is to at least warn the user that you are bailing to the defaults, because you failed to get the clock, when you were supposed to.
Quoted 7 lines
Quoted 3 lines
Besides, warning / logging (even if it is in debugging mode) the user that you failed to get a clock, when the board / platform does not support it, sounds at least bogus for them.
So, you would rather that I go with Chanwoo's approach of adding a per-soc flag indicating the presence of this clock ?
Yes.
BR,
Eduardo Valentin
Quoted 2 lines
Regards,
Abhilash
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141125/975c460d/attachment.sig>
PATCH 1/1 thermal: exynos: add optional sclk support
Hi Eduardo,
Quoted 16 lines · Eduardo Valentin
Abhilash,
Hi,
On Tue, Nov 25, 2014 at 08:00:50AM +0530, Abhilash Kesavan wrote:
<big cut>Quoted 9 lines
Quoted 8 lines
Quoted 5 lines
Lukasz, thanks for the review.
Chanwoo, are you OK with this ? If so, I will post a v2 with this and the other changes you suggested.
Is this a mandatory or a optional clk? What happens if the chip has the clock, but you fail to get it in this code?
Quoted 8 lines
Quoted 3 lines
This is mandatory on Exynos7 but not on the older SoCs. On Exynos7 if this clock is disabled then the TMU will not function.
It means, that only on Exynos7, outputing a
dev_erris the right thing to do. On other SoCs versions, it does not make sense at all requesting this clock.
OK
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
Think the rule of thumb is to at least warn the user that you are bailing to the defaults, because you failed to get the clock, when you were supposed to.
Besides, warning / logging (even if it is in debugging mode) the user that you failed to get a clock, when the board / platform does not support it, sounds at least bogus for them.
Quoted 5 lines
Quoted 3 lines
So, you would rather that I go with Chanwoo's approach of adding a per-soc flag indicating the presence of this clock ?
Yes.
Thanks for the clarification, will post v2 tomorrow.
Abhilash
Quoted 7 lines
BR,
Eduardo Valentin
Quoted 2 lines
Regards,
Abhilash