Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Jerome Brunet]

On Wed 02 Oct 2024 at 13:20, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:

> On 02/10/2024 12:48, Conor Dooley wrote:
>> From: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
>> I like this one better than qualcomms and wish to use it for the
>> PolarFire SoC clock drivers.
>> Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
>> ---
>>   drivers/clk/Kconfig                           |  4 ++
>>   drivers/clk/Makefile                          |  1 +
>>   drivers/clk/{meson => }/clk-regmap.c          |  2 +-
>>   drivers/clk/meson/Kconfig                     | 46 +++++++++----------
>>   drivers/clk/meson/Makefile                    |  1 -
>>   drivers/clk/meson/a1-peripherals.c            |  2 +-
>>   drivers/clk/meson/a1-pll.c                    |  2 +-
>>   drivers/clk/meson/axg-aoclk.c                 |  2 +-
>>   drivers/clk/meson/axg-audio.c                 |  2 +-
>>   drivers/clk/meson/axg.c                       |  2 +-
>>   drivers/clk/meson/c3-peripherals.c            |  2 +-
>>   drivers/clk/meson/c3-pll.c                    |  2 +-
>>   drivers/clk/meson/clk-cpu-dyndiv.c            |  2 +-
>>   drivers/clk/meson/clk-dualdiv.c               |  2 +-
>>   drivers/clk/meson/clk-mpll.c                  |  2 +-
>>   drivers/clk/meson/clk-phase.c                 |  2 +-
>>   drivers/clk/meson/clk-pll.c                   |  2 +-
>>   drivers/clk/meson/g12a-aoclk.c                |  2 +-
>>   drivers/clk/meson/g12a.c                      |  2 +-
>>   drivers/clk/meson/gxbb-aoclk.c                |  2 +-
>>   drivers/clk/meson/gxbb.c                      |  2 +-
>>   drivers/clk/meson/meson-aoclk.h               |  2 +-
>>   drivers/clk/meson/meson-eeclk.c               |  2 +-
>>   drivers/clk/meson/meson-eeclk.h               |  2 +-
>>   drivers/clk/meson/meson8-ddr.c                |  2 +-
>>   drivers/clk/meson/meson8b.c                   |  2 +-
>>   drivers/clk/meson/s4-peripherals.c            |  2 +-
>>   drivers/clk/meson/s4-pll.c                    |  2 +-
>>   drivers/clk/meson/sclk-div.c                  |  2 +-
>>   drivers/clk/meson/vclk.h                      |  2 +-
>>   drivers/clk/meson/vid-pll-div.c               |  2 +-
>>   .../meson => include/linux/clk}/clk-regmap.h  |  0
>>   32 files changed, 53 insertions(+), 53 deletions(-)
>>   rename drivers/clk/{meson => }/clk-regmap.c (99%)
>>   rename {drivers/clk/meson => include/linux/clk}/clk-regmap.h (100%)
>> 
> <snip>
>
> I don't have objections, but I think Stephen didn't like the idea
> a few years ago, but perhaps it has changed...
>
> Anyway, take my:
> Acked-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>

We had a similar discussion 3y ago indeed:
https://lore.kernel.org/linux-clk/162734682512.2368309.12015873010777083014@xxxxxxxxxxxxxxxxxxxxxxxxxx/

There are needs for a common regmap backed clocks indeed, but allowing
meson flavored regmap clocks to spread in the kernel was not really the
prefered way to do it. 

IIRC, Stephen's idea was more the bring regmap support in clk-gate.c,
clk-mux, etc ... I'm not quite sure how make iomem and regmap co-exist
in a manageable/maintainable way within those drivers (without adding yet
another level of abstraction I mean) ? Silently creating a regmap maybe
? but that's probably a bit heavy. I did not really had time to dig more
on this, I guess no one did.

I don't really have a preference one way or the other but if it is going
to be exposed in 'include/linux', we need to be sure that's how we want
to do it. With clocks poping in many driver subsystems, it will
difficult to change afterward. 

>
> Thanks,
> Neil

-- 
Jerome