Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220419081246.2546159-1-wenst@chromium.org> <3591fcc1-d34a-b40a-4e78-edcf9d2ddf08@collabora.com>
In-Reply-To: <3591fcc1-d34a-b40a-4e78-edcf9d2ddf08@collabora.com>
From:   Chen-Yu Tsai <wenst@chromium.org>
Date:   Wed, 20 Apr 2022 00:09:03 +0800
Message-ID: <CAGXv+5HhcQbdGLQFtgPnvzVbSKaQ5GQGvmjwPVOVxjrYnMh1dg@mail.gmail.com>
Subject: Re: [RFC PATCH 0/7] clk: mediatek: Move to struct clk_hw provider APIs
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
Cc:     Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Chun-Jie Chen <chun-jie.chen@mediatek.com>,
        Miles Chen <miles.chen@mediatek.com>,
        Rex-BC Chen <rex-bc.chen@mediatek.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        linux-clk@vger.kernel.org, linux-mediatek@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Apr 19, 2022 at 11:08 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 19/04/22 10:12, Chen-Yu Tsai ha scritto:
> > Hi everyone,
> >
> > This is part 2 of my proposed MediaTek clk driver cleanup work [1].
> >
>
> ..snip..
>
> >
> > The next phase of the cleanup/improvement shall be to introduce some
> > variant of `struct clk_parent_data` to describe clk relationships
> > efficiently.
> >
> > Please have a look.
> >
>
> Hello Chen-Yu,
>
> I am grateful to see this series, as the MediaTek clock drivers are getting
> a bit old, despite new platforms being implemented practically as we speak.
>
> With this, you surely get that I completely agree with the proposed cleanup
> and modernization of the entire MediaTek clocks infrastructure, but I think
> that introducing a `struct clk_parent_data` for these drivers is, at this
> point, a must, that not only fully justifies these patches, but also "makes
> the point" - as the effect of that would be a performance improvement as we
> would *at least* avoid lots of clk_cpy_name() in case of parent_hws, or in
> case or parent_data where no .fw_name is provided (which would be the case
> for most of the clocks).

You and me both. :) And yes, one of the intended results is to make the
clk driver probe faster.

> That said, my advice would be to add that conversion to declaring clocks
> with .hw.init.parent_data and/or .hw.init.parent_hws to this series as to
> really make it complete.

This series itself already touches a lot of code, even if most of it was
done by coccinelle. I'd like to send them separately to not overwhelm
people.

Also, I haven't actually started on that part yet. It is actually part 3
of my overall plan. I have a good idea of what to do, given I did similar
work for the sunxi-ng clk drivers (though half finished...).

Most of the clk references are internal to each driver, and those would
be mapped from some CLK_ID to some `struct clk_hw *` internally, but all
blocks have external parents that need to be modeled as well, and we
would likely need global clk name fallbacks for the blocks that don't
have parents declared in the device tree, which is unfortunately most
of them. Especially the central clock controllers like infracfg or pericfg
take many clk inputs, to the point that MediaTek folks were somewhat
unwilling to bloat the device tree with them.

So it does seem easier to use something like clk_parent_data with
`struct clk_hw *` replaced with an index everywhere. This structure
would get converted into clk_parent_data by the singular clk registration
helpers.

This would have to coexist with the existing helpers we have. So I think
this work would be combined with the helper API cleanup / alignment with
clk provider API.

Does that make sense to you?

> Of course, if you have concerns about old platforms that you cannot test,
> or for which this kind of conversion would require a huge amount of effort,
> then I would go for converting as many as possible as a first step and then
> leave the others for later.
>
> I would envision MT8183, 8186, 8192, 8195 to be a good amount of first
> candidates for this great effort.

I'm working with MT8183 right now, as it can readily boot mainline to a
shell. Depending on the schedule and whose on board with resources, I'd
probably handle the other ChromeOS platforms, or delegate it internally.


Regards
ChenYu