lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAM4voakcd2ztCwEGHVPTXmpkK=J30YFxFe82HUHbuHr_KHw=Rg@mail.gmail.com>
Date:	Wed, 3 Dec 2014 17:56:43 +0530
From:	Abhilash Kesavan <kesavan.abhilash@...il.com>
To:	Eduardo Valentin <edubezval@...il.com>
Cc:	Zhang Rui <rui.zhang@...el.com>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
	Amit Kachhap <amit.daniel@...sung.com>,
	Lukasz Majewski <l.majewski@...sung.com>,
	최찬우 <cw00.choi@...sung.com>
Subject: Re: [PATCH v2] thermal: exynos: add special clock support

Hi Bartlomiej and Lukasz,

On Wed, Nov 26, 2014 at 9:01 PM, Abhilash Kesavan
<kesavan.abhilash@...il.com> wrote:
> Hi Eduardo,
>
> On Wed, Nov 26, 2014 at 8:44 PM, Eduardo Valentin <edubezval@...il.com> 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 <edubezval@...il.com> 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 <a.kesavan@...sung.com>
>>> >> ---
>>> >> 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)

Can you comment on the preffered approach ?

Thanks,
Abhilash
>
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ