Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210806152146.16107-1-semen.protsenko@linaro.org>
 <20210806152146.16107-8-semen.protsenko@linaro.org> <3add6f87-7293-e1ae-8f9e-c69e9de18cf5@canonical.com>
 <CGME20210809194915eucas1p19999295aef3127f95ffe4c75a8acc63f@eucas1p1.samsung.com>
 <CAPLW+4=-uUcoLCjjBAC2K5NLswnXGXW1qrsTJrb_uZDgOQ5Ehw@mail.gmail.com> <bb69e79d-55a8-2090-e51c-fdfea755cf99@samsung.com>
In-Reply-To: <bb69e79d-55a8-2090-e51c-fdfea755cf99@samsung.com>
From:   Sam Protsenko <semen.protsenko@linaro.org>
Date:   Wed, 11 Aug 2021 14:20:02 +0300
Message-ID: <CAPLW+4mPrPSeukiJY6DmpEz0V=bjEYbjzLTnJ28T01yqoQY=mg@mail.gmail.com>
Subject: Re: [PATCH v2 7/8] clk: samsung: Add Exynos850 clock driver stub
To:     Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Tomasz Figa <tomasz.figa@gmail.com>,
        =?UTF-8?Q?Pawe=C5=82_Chmiel?= <pawel.mikolaj.chmiel@gmail.com>,
        Marc Zyngier <maz@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Jiri Slaby <jirislaby@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Charles Keepax <ckeepax@opensource.wolfsonmicro.com>,
        Ryu Euiyoul <ryu.real@samsung.com>,
        Tom Gall <tom.gall@linaro.org>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        John Stultz <john.stultz@linaro.org>,
        Amit Pundir <amit.pundir@linaro.org>,
        devicetree <devicetree@vger.kernel.org>,
        linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>,
        linux-clk <linux-clk@vger.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux Samsung SOC <linux-samsung-soc@vger.kernel.org>,
        "open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 10 Aug 2021 at 10:55, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>
> On 09.08.2021 21:48, Sam Protsenko wrote:
> >>> +/* Will be extracted to bindings header once proper clk driver is implemented */
> >>> +#define OSCCLK               1
> >>> +#define DOUT_UART    2
> >>> +#define CLK_NR_CLKS  3
> >>> +
> >>> +/* Fixed rate clocks generated outside the SoC */
> >>> +static struct samsung_fixed_rate_clock exynos850_fixed_rate_ext_clks[] __initdata = {
> >>> +     FRATE(OSCCLK, "fin_pll", NULL, 0, 26000000),
> >>> +};
> >>> +
> >>> +/*
> >>> + * Model the UART clock as a fixed-rate clock for now, to make serial driver
> >>> + * work. This clock is already configured in the bootloader.
> >>> + */
> >>> +static const struct samsung_fixed_rate_clock exynos850_peri_clks[] __initconst = {
> >>> +     FRATE(DOUT_UART, "DOUT_UART", NULL, 0, 200000000),
> >>> +};
> >>> +
> >>> +static const struct of_device_id ext_clk_match[] __initconst = {
> >>> +     { .compatible = "samsung,exynos850-oscclk" },
> >>
> >> One more thing - I am not sure anymore if this is correct. AFAIR, we
> >> wanted to drop compatibles for external clocks.
> >>
> > I'll remove oscclk from the clock driver and device tree. It's not
> > needed right now anyway, as that driver is just a stub.
> >
> > But I'd still like to know the proper way to define external clocks. I
> > can see that in exynos7.dtsi and exynos5433.dtsi there is just regular
> > fixed clock defined for "oscclk" (or "fin_pll"), and then that clock
> > is referenced in corresponding clock driver by its
> > 'clock-output-names' property. I guess that approach is the
> > recommended one?
>
> Yes, we should use generic "fixed-clock" in DT to model the external
> root clock. Registering the external clock from within the CMU driver
> is a legacy method that predates generic "fixed-clock" and should be
> avoided.
>

Thanks for confirming this. I'll go with generic fixed clock in my
clock patch series then.

> That said I think this temporary stub driver is not needed at all,
> you could well define a fixed clock in DT and reference it in the UART
> node, as Krzysztof suggested.
>

Ok, I'll remove the stub clock driver in v3. Using fixed clock in
device tree for serial seems to work fine.

> --
> Regards,
> Sylwester