MIME-Version: 1.0
References: <20230208234313.23863-1-semen.protsenko@linaro.org>
 <20230208234313.23863-7-semen.protsenko@linaro.org> <CGME20230219173642eucas1p1ddff7399223df763ff9fb48c4f7fc5f9@eucas1p1.samsung.com>
 <CAPLW+4=cOV8J+Ho1t8Tkg8X_3m4npyy3FUC2zcQAYywE12uEkw@mail.gmail.com> <ea0055c7-34d4-3da0-0d3a-52020a783922@samsung.com>
In-Reply-To: <ea0055c7-34d4-3da0-0d3a-52020a783922@samsung.com>
From:   Sam Protsenko <semen.protsenko@linaro.org>
Date:   Wed, 22 Feb 2023 21:07:06 -0600
Message-ID: <CAPLW+4=pFb2HZ8bhAs9hfm_trXUUQCiS7zf8G+VBA-i0qpLJJQ@mail.gmail.com>
Subject: Re: [PATCH v2 6/6] clk: samsung: exynos5433: Extract PM support to
 common ARM64 layer
To:     Marek Szyprowski <m.szyprowski@samsung.com>
Cc:     Tomasz Figa <tomasz.figa@gmail.com>,
        Alim Akhtar <alim.akhtar@samsung.com>,
        Chanho Park <chanho61.park@samsung.com>,
        David Virag <virag.david003@gmail.com>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        linux-samsung-soc@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Sylwester Nawrocki <s.nawrocki@samsung.com>,
        Chanwoo Choi <cw00.choi@samsung.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 21 Feb 2023 at 01:22, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> On 19.02.2023 18:36, Sam Protsenko wrote:
> > On Wed, 8 Feb 2023 at 17:43, Sam Protsenko <semen.protsenko@linaro.org> wrote:
> >> Exynos5433 clock driver implements PM support internally, which might be
> >> also useful for other Exynos clock drivers. Extract all PM related code
> >> from clk-exynos5433 to common ARM64 functions.
> >>
> >> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> >> ---
> >> Changes in v2:
> >>    - Rebased on top of latest soc/for-next tree
> >>    - Included linux/slab.h for kfree (found by kernel test robot)
> >>
> >>   drivers/clk/samsung/clk-exynos-arm64.c | 171 ++++++++++++++++++++++++-
> >>   drivers/clk/samsung/clk-exynos-arm64.h |   3 +
> >>   drivers/clk/samsung/clk-exynos5433.c   | 157 +----------------------
> >>   3 files changed, 175 insertions(+), 156 deletions(-)
> >>
> >> diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> >> index 2aa3f0a5644e..7ad7fd353dda 100644
> >> --- a/drivers/clk/samsung/clk-exynos-arm64.c
> >> +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> >> @@ -10,6 +10,9 @@
> >>    */
> > Hi Marek,
> >
> > It just occurred to me that as I'm pulling your code from
> > clk-exynos5433.c here, I should've probably added you to this file's
> > author list in the header comment. Does that sound right to you? If
> > so, I can re-send v3 with fixes.
>
> Fine for me.
>
> > Also, could you please review this series, if possible? I'm working
> > right now on PM_DOMAINS support for Exynos850, so along with this
> > series that would bring the initial PM support for ARM64 Exynos chips.
>
> Well, feel free to add:
>
> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
>
> to v3 (the whole patchset).
>

Thanks, Marek!

> The only remark I have is not strictly related to this patchset, but the
> code that is already in drivers/clk/samsung/clk-exynos-arm64.c. Playing
> with GATE_ENABLE_HWACG/GATE_MANUAL is not strictly specific to all
> Exynos ARM64 SoCs. GATE_OFF_START/GATE_OFF_END and the enable register
> bits are something specific to recent Exynos SoCs (like 7885/850/auto9)
> and don't fit well into the generic arm64 clk PM code, but I don't have
> a good idea how to move it to the respective clk drivers and keep it simple.
>

Yeah, maybe having some sort of HW variants would be a better choice.
That can be implemented later. I suggest we wait a bit first (after
applying this series) and see if there going be any new users for that
PM code. Once we gather more knowledge about current platforms (or
no), the desired design decisions should become more obvious.

>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>