Message-ID: <20230327192430.b2cp3yyrkzy4g4vw@penduick>
Date:   Mon, 27 Mar 2023 21:24:30 +0200
From:   Maxime Ripard <maxime@...no.tech>
To:     Aidan MacDonald <aidanmacdonald.0x0@...il.com>
Cc:     Stephen Boyd <sboyd@...nel.org>,
        Paul Cercueil <paul@...pouillou.net>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Chen-Yu Tsai <wens@...e.org>, Daniel Vetter <daniel@...ll.ch>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Thierry Reding <thierry.reding@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Shawn Guo <shawnguo@...nel.org>,
        Fabio Estevam <festevam@...il.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Claudiu Beznea <claudiu.beznea@...rochip.com>,
        Michael Turquette <mturquette@...libre.com>,
        Dinh Nguyen <dinguyen@...nel.org>,
        Chunyan Zhang <zhang.lyra@...il.com>,
        Manivannan Sadhasivam <mani@...nel.org>,
        Andreas Färber <afaerber@...e.de>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Abel Vesa <abelvesa@...nel.org>,
        Charles Keepax <ckeepax@...nsource.cirrus.com>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Peter De Schrijver <pdeschrijver@...dia.com>,
        Orson Zhai <orsonzhai@...il.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Prashant Gaikwad <pgaikwad@...dia.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Samuel Holland <samuel@...lland.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Richard Fitzgerald <rf@...nsource.cirrus.com>,
        Vinod Koul <vkoul@...nel.org>,
        NXP Linux Team <linux-imx@....com>,
        Sekhar Nori <nsekhar@...com>,
        Kishon Vijay Abraham I <kishon@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Takashi Iwai <tiwai@...e.com>,
        David Airlie <airlied@...il.com>,
        Luca Ceresoli <luca.ceresoli@...tlin.com>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        David Lechner <david@...hnology.com>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Mark Brown <broonie@...nel.org>,
        Max Filippov <jcmvbkbc@...il.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        linux-stm32@...md-mailman.stormreply.com,
        alsa-devel@...a-project.org, linux-mediatek@...ts.infradead.org,
        linux-phy@...ts.infradead.org, linux-mips@...r.kernel.org,
        linux-renesas-soc@...r.kernel.org,
        linux-actions@...ts.infradead.org, linux-clk@...r.kernel.org,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        patches@...nsource.cirrus.com, linux-tegra@...r.kernel.org,
        linux-rtc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v2 56/65] clk: ingenic: cgu: Switch to determine_rate

On Fri, Mar 24, 2023 at 08:58:48PM +0000, Aidan MacDonald wrote:
> >> My suggestion: add a per-clock bitmap to keep track of which parents
> >> are allowed. Any operation that would select a parent clock not on the
> >> whitelist should fail. Automatic reparenting should only select from
> >> clocks on the whitelist. And we need new DT bindings for controlling
> >> the whitelist, for example:
> >>
> >>     clock-parents-0 = <&clk1>, <&pll_c>;
> >>     clock-parents-1 = <&clk2>, <&pll_a>, <&pll_b>;
> >>
> >> This means that clk1 can only have pll_c as a parent, while clk2 can
> >> have pll_a or pll_b as parents. By default every clock will be able
> >> to use any parent, so a list is only needed if the machine needs a
> >> more restrictive policy.
> >>
> >> assigned-clock-parents should disable automatic reparenting, but allow
> >> explicit clk_set_parent(). This will allow clock drivers to start doing
> >> reparenting without breaking old DTs.
> >
> > I'm generally not a fan of putting all these policies in the device
> > tree. Do you have an example where it wouldn't be possible to do exactly
> > this from the driver itself?
> 
> I'm confused. What's implicit in the example is clk1 and clk2 might
> have *other* possible choices of parent clock and the device tree is
> limiting what the OS is allowed to choose.
>
> Why would you put such arbitrary limitations into the driver?

Why would we put such arbitrary limitations in the firmware? As this
entire thread can attest, people are already using the device tree to
work around the limitations of the Linux driver, or reduce the
features of Linux because they can rely on the device tree. Either
way, it's linked to the state of the Linux driver, and any other OS or
Linux version could very well implement something more dynamic.

> They would be different from machine to machine, unless the clock
> tree is so simple there is only *one* meaningful way to configure
> it.

If we look at the device trees we have in-tree, most of the users of
assigned-clocks are the same from one board to another.

> Most SoCs are complicated enough that there will be tradeoffs
> depending on what peripherals you are using (typically a single
> machine will not use *every* peripheral device provided by the SoC).

We already have APIs to lock parents or rates on a given clock from
the consumer. It's far superior (feature-wise) than what the device
tree will ever offer because it's code, and it depends on the usage
already since an unused driver won't probe.

Maxime

Powered by blists - more mailing lists