| Message ID | 20211008154352.19519-1-semen.protsenko@linaro.org |
|---|---|
| Headers | show |
| Series | clk: samsung: Introduce Exynos850 SoC clock driver | expand |
On 08.10.2021 17:43, Sam Protsenko wrote: > pll0822x PLL is used in Exynos850 SoC for top-level integer PLLs. The > code was derived from very similar pll35xx type, with next differences: > > 1. Lock time for pll0822x is 150*P_DIV, when for pll35xx it's 270*P_DIV > 2. It's not suggested in Exynos850 TRM that S_DIV change doesn't require > performing PLL lock procedure (which is done in pll35xx > implementation) > > When defining pll0822x type, CON3 register offset should be provided as > a "con" parameter of PLL() macro, like this: > > PLL(pll_0822x, 0, "fout_shared0_pll", "oscclk", > PLL_LOCKTIME_PLL_SHARED0, PLL_CON3_PLL_SHARED0, > exynos850_shared0_pll_rates), > > To define PLL rates table, one can use PLL_35XX_RATE() macro, e.g.: > > PLL_35XX_RATE(26 * MHZ, 1600 * MHZ, 800, 13, 0) > > as it's completely appropriate for pl0822x type and there is no sense in > duplicating that. > > If bit #1 (MANUAL_PLL_CTRL) is not set in CON1 register, it won't be > possible to set new rate, with next error showing in kernel log: > > Could not lock PLL fout_shared1_pll > > That can happen for example if bootloader clears that bit beforehand. > PLL driver doesn't account for that, so if MANUAL_PLL_CTRL bit was > cleared, it's assumed it was done for a reason and it shouldn't be > possible to change that PLL's rate at all. > > Signed-off-by: Sam Protsenko<semen.protsenko@linaro.org> > Reviewed-by: Krzysztof Kozlowski<krzysztof.kozlowski@canonical.com> > Acked-by: Chanwoo Choi<cw00.choi@samsung.com> Applied, thanks.
On 08.10.2021 17:43, Sam Protsenko wrote: > Clock controller driver is designed to have separate instances for each > particular CMU. So clock IDs in this bindings header also start from 1 > for each CMU. > > Signed-off-by: Sam Protsenko<semen.protsenko@linaro.org> > Reviewed-by: Krzysztof Kozlowski<krzysztof.kozlowski@canonical.com> > Acked-by: Rob Herring<robh@kernel.org> Applied, thanks.
On Sat, 9 Oct 2021 at 23:41, Sylwester Nawrocki <snawrocki@kernel.org> wrote: > > On 08.10.2021 17:43, Sam Protsenko wrote: > > Provide dt-schema documentation for Exynos850 SoC clock controller. > > > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > > Acked-by: Chanwoo Choi <cw00.choi@samsung.com> > > > diff --git a/Documentation/devicetree/bindings/clock/samsung,exynos850-clock.yaml b/Documentation/devicetree/bindings/clock/samsung,exynos850-clock.yaml > > new file mode 100644 > > index 000000000000..79202e6e6402 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/samsung,exynos850-clock.yaml > > @@ -0,0 +1,185 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/clock/samsung,exynos850-clock.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Samsung Exynos850 SoC clock controller > > + > > +maintainers: > > + - Sam Protsenko <semen.protsenko@linaro.org> > > + - Chanwoo Choi <cw00.choi@samsung.com> > > + - Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > > + - Sylwester Nawrocki <s.nawrocki@samsung.com> > > + - Tomasz Figa <tomasz.figa@gmail.com> > > + > > +description: | > > + Exynos850 clock controller is comprised of several CMU units, generating > > + clocks for different domains. Those CMU units are modeled as separate device > > + tree nodes, and might depend on each other. Root clocks in that clock tree are > > + two external clocks:: OSCCLK (26 MHz) and RTCCLK (32768 Hz). Those external > > + clocks must be defined as fixed-rate clocks in dts. > > + > > + CMU_TOP is a top-level CMU, where all base clocks are prepared using PLLs and > > + dividers; all other leaf clocks (other CMUs) are usually derived from CMU_TOP. > > + > > + Each clock is assigned an identifier and client nodes can use this identifier > > + to specify the clock which they consume. All clocks that available for usage > > s/All clocks that available/All clocks available ? > No need to resend, I can amend it when applying. > Yeah, not a native speaker, I tend to do such mistakes sometimes :) Please fix when applying. Btw, I can see that you already applied 3 out of 5 patches from this patch series here: [1]. Can you please also apply the rest, or is there any outstanding comments that I missed? [1] https://git.kernel.org/pub/scm/linux/kernel/git/snawrocki/clk.git/log/?h=for-v5.16/next
On 11.10.2021 12:13, Sam Protsenko wrote: > On Sat, 9 Oct 2021 at 23:41, Sylwester Nawrocki <snawrocki@kernel.org> wrote: >> >> On 08.10.2021 17:43, Sam Protsenko wrote: >>> Provide dt-schema documentation for Exynos850 SoC clock controller. >>> >>> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >>> Acked-by: Chanwoo Choi <cw00.choi@samsung.com> [...] >>> +++ b/Documentation/devicetree/bindings/clock/samsung,exynos850-clock.yaml >>> @@ -0,0 +1,185 @@ [...] >>> + >>> +title: Samsung Exynos850 SoC clock controller >>> + >>> +maintainers: >>> + - Sam Protsenko <semen.protsenko@linaro.org> >>> + - Chanwoo Choi <cw00.choi@samsung.com> >>> + - Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >>> + - Sylwester Nawrocki <s.nawrocki@samsung.com> >>> + - Tomasz Figa <tomasz.figa@gmail.com> >>> + >>> +description: | >>> + Exynos850 clock controller is comprised of several CMU units, generating >>> + clocks for different domains. Those CMU units are modeled as separate device >>> + tree nodes, and might depend on each other. Root clocks in that clock tree are >>> + two external clocks:: OSCCLK (26 MHz) and RTCCLK (32768 Hz). Those external >>> + clocks must be defined as fixed-rate clocks in dts. >>> + >>> + CMU_TOP is a top-level CMU, where all base clocks are prepared using PLLs and >>> + dividers; all other leaf clocks (other CMUs) are usually derived from CMU_TOP. >>> + >>> + Each clock is assigned an identifier and client nodes can use this identifier >>> + to specify the clock which they consume. All clocks that available for usage >> >> s/All clocks that available/All clocks available ? >> No need to resend, I can amend it when applying. >> > > Yeah, not a native speaker, I tend to do such mistakes sometimes :) > Please fix when applying. > > Btw, I can see that you already applied 3 out of 5 patches from this > patch series here: [1]. Can you please also apply the rest, or is > there any outstanding comments that I missed? The patches look good to me, I just wanted to allow some for Rob to have a look and provide an Ack. Regards, -- Sylwester Nawrocki Samsung R&D Institute Poland