Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753850AbaKZPbH (ORCPT ); Wed, 26 Nov 2014 10:31:07 -0500 Received: from mail-qc0-f180.google.com ([209.85.216.180]:37708 "EHLO mail-qc0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750892AbaKZPbC (ORCPT ); Wed, 26 Nov 2014 10:31:02 -0500 MIME-Version: 1.0 In-Reply-To: <20141126151435.GB3925@developer> References: <1416963070-5806-1-git-send-email-a.kesavan@samsung.com> <20141126144504.GA3808@developer> <20141126151435.GB3925@developer> Date: Wed, 26 Nov 2014 21:01:01 +0530 Message-ID: Subject: Re: [PATCH v2] thermal: exynos: add special clock support From: Abhilash Kesavan To: Eduardo Valentin Cc: Zhang Rui , "linux-pm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Bartlomiej Zolnierkiewicz , Amit Kachhap , Lukasz Majewski , =?UTF-8?B?7LWc7LCs7Jqw?= Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Eduardo, On Wed, Nov 26, 2014 at 8:44 PM, Eduardo Valentin wrote: > Abhilash, > > On Wed, Nov 26, 2014 at 08:34:19PM +0530, Abhilash Kesavan wrote: >> Hi Eduardo, >> >> On Wed, Nov 26, 2014 at 8:15 PM, Eduardo Valentin wrote: >> > >> > Abhilash, >> > >> > On Wed, Nov 26, 2014 at 06:21:10AM +0530, Abhilash Kesavan 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 clock and update the binding documentation. >> >> >> >> Signed-off-by: Abhilash Kesavan >> >> --- >> >> Changes since v1: >> >> - Added a per-soc flag to indicate the presence of special clock >> >> - Changed the name of special clock from "tmu_sclk" to "sclk" >> >> - Fixed the error handling for sclk >> >> >> >> Tested on 5420 and 5800 based chromebooks, no change in existing behavior. >> >> >> >> .../devicetree/bindings/thermal/exynos-thermal.txt | 3 ++ >> >> drivers/thermal/samsung/exynos_tmu.c | 31 ++++++++++++++++---- >> >> 2 files changed, 29 insertions(+), 5 deletions(-) >> >> >> >> diff --git a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt >> >> index ae738f5..acf4705 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 >> >> + -- "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..8ed8409 100644 >> >> --- a/drivers/thermal/samsung/exynos_tmu.c >> >> +++ b/drivers/thermal/samsung/exynos_tmu.c >> >> @@ -123,11 +123,14 @@ >> >> * @base: base address of the single instance of the TMU controller. >> >> * @base_second: base address of the common registers of the TMU controller. >> >> * @irq: irq number of the TMU controller. >> >> + * @needs_sclk: SoC specific flag indicating that sclk is required for >> >> + functional operation of the TMU controller. >> >> * @soc: id of the SOC type. >> >> * @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. >> >> + * @sclk: pointer to the clock structure for accessing the tmu special clock. >> >> * @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. >> >> @@ -144,10 +147,11 @@ struct exynos_tmu_data { >> >> void __iomem *base; >> >> void __iomem *base_second; >> >> int irq; >> >> + bool needs_sclk; >> >> 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 +887,24 @@ static int exynos_tmu_probe(struct platform_device *pdev) >> >> goto err_clk_sec; >> >> } >> >> >> >> + if (data->needs_sclk) { >> > >> > Based on the trend we see from Bartlomiej's and Lukasz' works, you >> > should be asking for SoC version, not adding a flag. Can you please >> > crosscheck with them? >> >> I planned to populate this flag as true for Exynos7 in >> exynos_map_dt_data() where other soc specific methods are being setup. > > Well, that I wouldn't expect different, otherwise how your code is > supposed to work, right? yes :) > >> >> Bartlomiej/Lukasz can you kindly comment on if you would prefer the >> check to be based on the SoC version. >> > > > Have a look on how Bartz has removed the flag based code in my -linus > branch. Here is an example: > https://git.kernel.org/cgit/linux/kernel/git/evalenti/linux-soc-thermal.git/commit/?h=linus&id=ef3f80fc7f79c32a1b015afcbffce2a2630011a4 OK, I see what you mean. I'll wait for Bartlomiej and Lukasz to weigh in as well. If they are in favor of using the SoC version then I will fold this into the patch adding support for Exynos7 (to be posted after all of Lukasz' patches are in) Thanks, Abhilash > >> Regards, >> Abhilash >> > >> > Cheers, >> > >> > Eduardo Valentin >> > >> >> + data->sclk = devm_clk_get(&pdev->dev, "sclk"); >> >> + if (IS_ERR(data->sclk)) { >> >> + dev_err(&pdev->dev, "Failed to get sclk\n"); >> >> + goto err_clk; >> >> + } else { >> >> + ret = clk_prepare_enable(data->sclk); >> >> + if (ret) { >> >> + dev_err(&pdev->dev, "Failed to enable sclk\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 +914,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 +946,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 +954,12 @@ 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: >> >> + clk_disable_unprepare(data->sclk); >> >> err_clk: >> >> clk_unprepare(data->clk); >> >> err_clk_sec: >> >> @@ -956,6 +976,7 @@ static int exynos_tmu_remove(struct platform_device *pdev) >> >> >> >> exynos_tmu_control(pdev, false); >> >> >> >> + clk_disable_unprepare(data->sclk); >> >> clk_unprepare(data->clk); >> >> if (!IS_ERR(data->clk_sec)) >> >> clk_unprepare(data->clk_sec); >> >> -- >> >> 1.7.9.5 >> >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/