Thread (9 messages) 9 messages, 4 authors, 2025-02-20

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

From: Conor Dooley <conor@kernel.org>
Date: 2024-10-03 11:33:46
Also in: linux-amlogic, linux-clk, linux-devicetree, linux-riscv, lkml

On Wed, Oct 02, 2024 at 03:21:16PM +0200, Jerome Brunet wrote:
On Wed 02 Oct 2024 at 13:20, Neil Armstrong [off-list ref] wrote:
quoted
On 02/10/2024 12:48, Conor Dooley wrote:
quoted
From: Conor Dooley <conor.dooley@microchip.com>
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@microchip.com>
---
  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@linaro.org>
We had a similar discussion 3y ago indeed:
https://lore.kernel.org/linux-clk/162734682512.2368309.12015873010777083014@swboyd.mtv.corp.google.com/ (local)

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. 
Cool, thanks for that link.
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 guess I have some motivation to looking into it at the moment. I had
my reservations about the Meson approach too, liking it more than
Qualcomm's didn't mean I completely liked it.
It was already my intention to implement point b of your mail, had the
general idea here been acceptable, cos that's a divergence from how the
generic clock types (that the driver in question currently uses) work.
And on that note, I just noticed I left the mild-annoyance variable name
"sigh" in the submitted driver changes, which I had used for the
clk_regmap struct that your point b in the link relates to.
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. 
Yeah, I agree. I didn't expect this to go in right away, and I also
didn't want to surge ahead on some rework of the clock types, were
people to hate even the reuse.

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help