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;
Subject: Re: [PATCH v2 7/8] clk: samsung: Add Exynos850 clock driver stub
To:     Sam Protsenko <semen.protsenko@linaro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Cc:     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>
From:   Sylwester Nawrocki <s.nawrocki@samsung.com>
Message-ID: <bb69e79d-55a8-2090-e51c-fdfea755cf99@samsung.com>
Date:   Tue, 10 Aug 2021 09:55:37 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0)
        Gecko/20100101 Thunderbird/78.12.0
MIME-Version: 1.0
In-Reply-To: <CAPLW+4=-uUcoLCjjBAC2K5NLswnXGXW1qrsTJrb_uZDgOQ5Ehw@mail.gmail.com>
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
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>
Precedence: bulk

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.  

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.

--
Regards, 
Sylwester