Re: [PATCH V2 0/3] Add S4 SoC clock controller driver

Hi JB,
	We suspect that you are not looking carefully. You only see two base 
addresses for controllers, so you think you should use two clock 
controllers. But you're missing their direct connection.

On 2022/7/28 16:06, Yu Tu wrote:

Hi JB，

On 2022/7/28 15:08, Jerome Brunet wrote:

quoted

[ EXTERNAL EMAIL ]


On Thu 28 Jul 2022 at 13:41, Yu Tu [off-list ref] wrote:

quoted

1. Add clock controller driver for S4 SOC.

Yu Tu (3):
   dt-bindings: clk: meson: add S4 SoC clock controller bindings
   arm64: dts: meson: add S4 Soc clock controller in DT
   clk: meson: s4: add s4 SoC clock controller driver

V1 -> V2: Change format as discussed in the email.

Link:https://lore.kernel.org/linux-amlogic/20220708062757.3662-1-yu.tu@amlogic.com/ (local) 


  .../bindings/clock/amlogic,gxbb-clkc.txt      |    1 +
  MAINTAINERS                                   |    1 +
  arch/arm64/boot/dts/amlogic/meson-s4.dtsi     |   11 +
  drivers/clk/meson/Kconfig                     |   15 +
  drivers/clk/meson/Makefile                    |    1 +
  drivers/clk/meson/s4.c                        | 4732 +++++++++++++++++
  drivers/clk/meson/s4.h                        |  296 ++
  include/dt-bindings/clock/s4-clkc.h           |  146 +
  8 files changed, 5203 insertions(+)
  create mode 100644 drivers/clk/meson/s4.c
  create mode 100644 drivers/clk/meson/s4.h
  create mode 100644 include/dt-bindings/clock/s4-clkc.h


base-commit: b293bc9286ee21824e93f0fcfed3b78fdfee01e6

Please don't post until you have addressed *ALL* the comments from the
previous version.

The last email asked you to adopt A1 method, but you did not reply?

quoted

At first glance, I can see that this is still a single driver for
what is obviously 2 controllers with 2 register spaces. Simple comments
like the "<< 2" in the register declaration have not been addressed 
either.

I understand that this should be a controller, just two address 
descriptions. One is the various PLL registers and one is the clock for 
the peripherals. And PLL is to provide a clock source for various 
peripheral clocks. So a clock controller is reasonable. I think you got 
it wrong.

Ok, if you insist on using two clock controllers,, please provide your 
the reason and example code?

quoted

Seeing that, I have not reviewed this version further.
I won't until all the comments from v1 are either addressed or answer

Regards
Jerome

.