Message-ID: <1jzgvnfdng.fsf@starbuckisacylon.baylibre.com>
Date:   Fri, 18 Jun 2021 16:30:59 +0200
From:   Jerome Brunet <jbrunet@...libre.com>
To:     Marc Kleine-Budde <mkl@...gutronix.de>,
        Stephen Boyd <sboyd@...nel.org>,
        "Peng Fan (OSS)" <peng.fan@....nxp.com>, mturquette@...libre.com
Cc:     Peng Fan <peng.fan@....com>, linux-kernel@...r.kernel.org,
        kernel@...gutronix.de, linux-clk@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 0/3] clk: support regmap


On Wed 02 Jun 2021 at 11:32, Jerome Brunet <jbrunet@...libre.com> wrote:

> On Wed 02 Jun 2021 at 10:21, Marc Kleine-Budde <mkl@...gutronix.de> wrote:
>
>> On 6/2/21 10:18 AM, Stephen Boyd wrote:
>>> Quoting Peng Fan (OSS) (2021-05-28 04:34:00)
>>>> From: Peng Fan <peng.fan@....com>
>>>>
>>>> To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and peripheral
>>>> reset functionality, so we need make sure the access to the PCC register
>>>> be protected to avoid concurrent access from clk and reset subsystem.
>>>>
>>>> So let's use regmap here.
>>>>
>>>> The i.MX specific code will be posted if this patchset is ok for you.
>>> 
>>> We have a couple regmap clk drivers in the tree. Either combine the
>>> different regmap clk drivers or duplicate it into the imx directory. I'd
>>> prefer we combine them but last time I couldn't agree on the approach
>>> when Jerome wanted to do it. Maybe now is the time to combine them all
>>> into one common piece of code.
>>
>> IMHO for the basic drivers, such as gate, divider, mux, mux-div, etc... it makes
>> no sense to have them in an arch specific subdir, like meson.
>
> Indeed, those basic types were not meant to remain platform
> specific. Some framework (ASoC for ex) make heavy use of regmap and
> could welcome regmap based basic clock types.
>
> At the time, Stephen (qcom) and I (meson) had slightly different
> approaches. Before having those types spread through the kernel, I think
> testing things out was a good thing ... this is why these are platform
> specific ATM.
>
> It's been 3 years now ... and it has not been a total disaster :)
>
> In the end things are not so different. Let's compare:
> a. Both have a generic "clk_regmap" type common to all regmap based
>   types. This is very useful to easily fix the regmap pointer in static
>   clocks (which are heavily used by both platform)
>
> b. Meson uses a generic pointer to store the type specific info.
>   Qcom embeds the generic clk_regmap into the specific type one.
>   => In the end, I don't see any advantage to the meson
>   approach. Switching to the qcom method would be quite a big bang
>   for meson but it is doable (nothing difficult, just a huge line count)
>   
> c. Qcom basic regmap type deviates a bit from the regular basic ones
>   when it comes to the actual data. The qcom "clk_regmap" also provides
>   the gate, mux have the qcom "parent_map". In meson, I tried to keep as
>   close as possible to regular basic types ... at least what they were 3
>   years ago. Only the register poking part should be different actually.
>   => I'd be in favor of keeping close to meson here.
>
> A possible plan could be:
> 1. Rework meson as explained in [b] above.
> 2. reword types in qcom where necessary to avoid name clashes (add
>    "_qcom" extension for ex)
> 3. Move the clk_regmap implementation out of meson to drivers/clk
> 4. Things are yours to play with ...
>
> I can take care of 1. and 3. I would welcome help for 2. especially since
> I won't be able to test it.
>

Hi Stephen,

What do you think of the proposition above ?
There rework is going to take some time to do. I'll start only if this
OK with you.

Cheers
Jerome

>>
>> regards,
>> Marc

Powered by blists - more mailing lists