On 21/05/2022 11:51, Anand Moon wrote:

Hi Krzysztof,

On Wed, 18 May 2022 at 12:58, Krzysztof Kozlowski
[off-list ref] wrote:

quoted

On 17/05/2022 20:43, Anand Moon wrote:

quoted

Hi Krzysztof,

On Sun, 15 May 2022 at 15:11, Krzysztof Kozlowski
[off-list ref] wrote:

quoted

On 15/05/2022 08:41, Anand Moon wrote:

quoted

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".

tmu_triminfo_apbif is not a core driver to all the Exynos SOC board
it is only used by the Exynos542x SOC family

If we look into the original code its place in between
the devm_clk_get(data->clk) and clk_prepare(data->clk)
after this change, the code is in the correct order of initialization
of the clock.

What was wrong with original order? You still did not explain it.

quoted

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.

quoted

quoted

quoted

Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Signed-off-by: Anand Moon <redacted>
---
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?

After cleanly applying the patches I see the below changes.
if you want me to remove the else part below and keep
the original code I am ok.

        data->clk = devm_clk_get(&pdev->dev, "tmu_apbif");
        if (IS_ERR(data->clk)) {
                dev_err(&pdev->dev, "Failed to get clock\n");
                ret = PTR_ERR(data->clk);
                goto err_sensor;
        } else {
                ret = clk_prepare_enable(data->clk);

Which is wrong and does not make any sense. This is third question - why
the main clock is prepared within 'else' branch?

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel