[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a197ed7f-6115-4407-6931-f37b719587be@linaro.org>
Date: Wed, 18 May 2022 09:28:17 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Anand Moon <linux.amoon@...il.com>,
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Amit Kucheria <amitk@...nel.org>,
Zhang Rui <rui.zhang@...el.com>,
Alim Akhtar <alim.akhtar@...sung.com>,
Linux PM list <linux-pm@...r.kernel.org>,
linux-samsung-soc@...r.kernel.org,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2 2/6] thermal: exynos: Reorder the gpu clock
initialization for exynos5420 SoC
On 17/05/2022 20:43, Anand Moon wrote:
> Hi Krzysztof,
>
> On Sun, 15 May 2022 at 15:11, Krzysztof Kozlowski
> <krzysztof.kozlowski@...aro.org> wrote:
>>
>> On 15/05/2022 08:41, Anand Moon wrote:
>>> Reorder the tmu_gpu clock initialization for exynos5422 SoC.
>>
>> Why?
> It just code reorder
I know what it is. I asked why. The answer in English to question "Why"
is starting with "Because".
You repeated again the argument what are you doing to my question "Why
are you doing it".
It was the same before, many, many times. It's a waste of reviewers
time, because you receive a review and you do not implement the feedback.
>>
>>>
>>> Cc: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
>>> Signed-off-by: Anand Moon <linux.amoon@...il.com>
>>> ---
>>> v1: split the changes and improve the commit messages
>>> ---
>>> drivers/thermal/samsung/exynos_tmu.c | 43 ++++++++++++++--------------
>>> 1 file changed, 21 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>>> index 75b3afadb5be..1ef90dc52c08 100644
>>> --- a/drivers/thermal/samsung/exynos_tmu.c
>>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>>> @@ -1044,42 +1044,41 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>> dev_err(&pdev->dev, "Failed to get clock\n");
>>> ret = PTR_ERR(data->clk);
>>> goto err_sensor;
>>> - }
>>> -
>>> - data->clk_sec = devm_clk_get(&pdev->dev, "tmu_triminfo_apbif");
>>> - if (IS_ERR(data->clk_sec)) {
>>> - if (data->soc == SOC_ARCH_EXYNOS5420_TRIMINFO) {
>>> - dev_err(&pdev->dev, "Failed to get triminfo clock\n");
>>> - ret = PTR_ERR(data->clk_sec);
>>> - goto err_sensor;
>>> - }
>>> } else {
>>> - ret = clk_prepare_enable(data->clk_sec);
>>> + ret = clk_prepare_enable(data->clk);
>>
>> This looks a bit odd. The clock was before taken unconditionally, not
>> within "else" branch...
>
> The whole *clk_sec* ie tmu_triminfo_apbif clock enable is being moved
> down to the switch case.
> tmu_triminfo_apbif clock is not used by Exynos4412 and Exynos5433 and
> Exynos7 SoC.
This is not the answer. Why are you preparing data->clk within else{}
branch?
Best regards,
Krzysztof
Powered by blists - more mailing lists