[PATCH] thermal: exynos: add optional sclk support
Abhilash Kesavan
kesavan.abhilash at gmail.com
Mon Nov 24 08:10:38 PST 2014
Hi Lukasz,
On Mon, Nov 24, 2014 at 4:18 PM, Lukasz Majewski <l.majewski at samsung.com> wrote:
> Hi Abhilash,
>
>> Hi Chanwoo,
>>
>> On Sun, Nov 23, 2014 at 10:21 AM, Chanwoo Choi <cwchoi00 at gmail.com>
>> wrote:
>> > Hi Abhilash,
>> >
>> > On Sat, Nov 22, 2014 at 4:45 PM, Abhilash Kesavan
>> > <a.kesavan at samsung.com> wrote:
>> >> 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
>>
>> Thanks for the link.
>>
>> >
>> > So, I suggest you that Exynos would use the similar method to
>> > support special clock
>> > on all of IPs for Exynos SoC.
>> >
>> >> Signed-off-by: Abhilash Kesavan <a.kesavan at samsung.com>
>> >> ---
>> >> 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.txt
>> >> b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>> >> index 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.
>> >> diff --git a/drivers/thermal/samsung/exynos_tmu.c
>> >> b/drivers/thermal/samsung/exynos_tmu.c index d44d91d..6627937
>> >> 100644 --- a/drivers/thermal/samsung/exynos_tmu.c
>> >> +++ b/drivers/thermal/samsung/exynos_tmu.c
>> >> @@ -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");
>> >
>> > 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.
>
> 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
>
>>
>> >
>> > 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'.
>>
>> >
>> > [2]
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/273557.html
>> >
>> >> + } 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);
>> >
>> > I think IS_ERROR don't be necessary because
>> > clk_disalbe_unprepare check the NULL pointer of clock pointer.
>> >
>> >> 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);
>> >
>> > ditto.
>>
>> Will fix.
>>
>> Thanks for the review.
>>
>> Regards,
>> Abhilash
>> >
>> > Best Regards,
>> > Chanwoo Choi
>> >
>> >> clk_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
More information about the linux-arm-kernel
mailing list