mbox series

[v8,00/11] add Amlogic A1 clock controller drivers

Message ID 20221201225703.6507-1-ddrokosov@sberdevices.ru
Headers show
Series add Amlogic A1 clock controller drivers | expand

Message

Dmitry Rokosov Dec. 1, 2022, 10:56 p.m. UTC
A1 SoC has four clock controllers on the board: PLL, Peripherals, CPU,
and Audio. The audio clock controller is different from others, but the
rest are very similar from a functional and regmap point of view.
This patch series add support for Amlogic A1 PLL and Peripherals clock
drivers.
It blocks all A1 peripherals mainline support and a couple of patch series,
which were already reviewed and acked, but weren't merged due to pending
clock controller drivers series, e.g.
https://lore.kernel.org/linux-amlogic/7hd09cw9oh.fsf@baylibre.com/

The previous v7 version [8] had several logic and style problems, all of
them are resolved in this version. Original Jian Hu v7 patches are not
touched, and all additional fixes are implemented in separate patches.
Patch "clk: meson: add support for A1 PLL clock ops" is removed, because
a1-pll clk driver inherits all stuff from clk-pll base driver, just
implements custom init/enable/disable/is_enabled callbacks.

TODO: CPU and Audio clock controllers are not included in this patch
series, it will be sent later. The following clks from these controllers
are not supported for now:
* Audio clks - vad, mclk_vad, mclk_d, resample_a, locker_in, mclk_b,
   pdmdclk, pdmsysclk, eqdrc, spdifin, mclk_a, audio2_toaudiotop,
   audio2_tovad, audio2_toddr_vad, audio2_tdmin_vad, audio2_pdm,
   audio2_ddr_arb, audio_audiolocker, audio_eqdrc, audio_resamplea,
   audio_spdifin, audio_toddrb, audio_toddra, audio_frddrb, audio_frddra,
   audio_tdmoutb, audio_tdmouta, audio_loopbacka, audio_tdminlb,
   audio_tdminb, audio_tdmina, audio_ddr_arb, mclk_c

* CPU clks: cpu_fixed_source_sel0, cpu_fixed_source_div0,
            cpu_fixed_source_sel1, cpu_fixed_source_div1, cpu_clk

Changes v7->v8:
    - introduced a1-clkc common driver for all A1 clock controllers
    - exported meson_clk_pll_wait_lock symbol
    - supported meson-a1-clkc common driver in the a1-pll and a1 clkc
    - inherited a1-pll from the base clk-pll driver, implemented own
      version of init/enable/disable/enabled routines; rate calculating
      logic is fully the same
    - aligned CLKID-related definitions with CLKID list from order
      perspective to remove holes and permutations
    - corrected Kconfig dependencies and types
    - provided correct MODULE_AUTHORs()
    - optimized and fixed up some clock relationships
    - removed unused register offset definitions
    - fixed up A1 PLL and Peripherals clkc dtb_check errors
    - fixed clk_summary kernel panic due to missing a1_pad_ctrl
      clk_regmap definition
    - included PLL and Peripherals clk controllers to the base a1 dts

Changes since v6 at [7]:
    - fix 'dt_binding_check' compiling error
    - add acked-by

Changes since v5 at [6]:
    - fix yaml file
    - add rst/current_en/l_detect parm detection
    - remove 'meson_eeclkc_data' in a1.c and a1-pll.c

Changes since v4 at [5]:
    - change yaml GPL
    - drop meson-eeclk.c patch, add probe function in each driver
    - add CLK_IS_CRITICAL for sys_clk clock, drop the flag for sys_a
      and sys_b
    - add new parm for pll, add protection for rst parm
    - drop flag for a1_fixed_pll
    - remove the same comment for fclk_div, add "refer to"
    - add critical flag for a1_sys_clk
    - remove rtc table
    - rename a1_dspa_en_dspa and a1_dspb_en_dspb
    - remove useless comment

Changes since v3 at [3]:
    - fix reparenting orphan failed, it depends on jerome's patch [4]
    - fix changelist in v3 about reparenting orphan
    - remove the dts patch 

Changes since v2 at [2]:
    - add probe function for A1
    - separate the clock driver into two patch
    - change some clock flags and ops
    - add support for a1 PLL ops
    - add A1 clock node
    - fix reparenting orphan clock failed, registering xtal_fixpll
      and xtal_hifipll after the provider registration, it is not
      a best way.

Changes since v1 at [1]:
    - place A1 config alphabetically
    - add actual reason for RO ops, CLK_IS_CRITICAL, CLK_IGNORE_UNUSED
    - separate the driver into two driver: peripheral and pll driver
    - delete CLK_IGNORE_UNUSED flag for pwm b/c/d/e/f clock, dsp clock
    - delete the change in Kconfig.platforms, address to Kevin alone
    - remove the useless comments
    - modify the meson pll driver to support A1 PLLs

[1] https://lkml.kernel.org/r/1569411888-98116-1-git-send-email-jian.hu@amlogic.com
[2] https://lkml.kernel.org/r/1571382865-41978-1-git-send-email-jian.hu@amlogic.com
[3] https://lkml.kernel.org/r/20191129144605.182774-1-jian.hu@amlogic.com
[4] https://lkml.kernel.org/r/20191203080805.104628-1-jbrunet@baylibre.com
[5] https://lkml.kernel.org/r/20191206074052.15557-1-jian.hu@amlogic.com
[6] https://lkml.kernel.org/r/20191227094606.143637-1-jian.hu@amlogic.com
[7] https://lkml.kernel.org/r/20200116080440.118679-1-jian.hu@amlogic.com
[8] https://lore.kernel.org/linux-amlogic/20200120034937.128600-1-jian.hu@amlogic.com/

Dmitry Rokosov (7):
  clk: meson: pll: export meson_clk_pll_wait_lock symbol
  clk: meson: introduce a1-clkc common driver for all A1 clock
    controllers
  clk: meson: a1: redesign Amlogic A1 PLL clock controller
  dt-bindings: clock: meson: fixup A1 PLL clkc dtb_check errors
  clk: meson: redesign A1 Peripherals CLK controller
  dt-bindings: clock: meson: fixup A1 peripherals clkc dtb_check errors
  arm64: dts: meson: a1: introduce PLL and Peripherals clk controllers

Jian Hu (4):
  dt-bindings: clock: meson: add A1 PLL clock controller bindings
  clk: meson: a1: add support for Amlogic A1 PLL clock driver
  dt-bindings: clock: meson: add A1 peripheral clock controller bindings
  clk: meson: a1: add support for Amlogic A1 Peripheral clock driver

 .../bindings/clock/amlogic,a1-clkc.yaml       |   73 +
 .../bindings/clock/amlogic,a1-pll-clkc.yaml   |   59 +
 MAINTAINERS                                   |    1 +
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi     |   27 +-
 drivers/clk/meson/Kconfig                     |   24 +
 drivers/clk/meson/Makefile                    |    3 +
 drivers/clk/meson/a1-pll.c                    |  451 ++++
 drivers/clk/meson/a1-pll.h                    |   59 +
 drivers/clk/meson/a1.c                        | 2222 +++++++++++++++++
 drivers/clk/meson/a1.h                        |  116 +
 drivers/clk/meson/clk-pll.c                   |    3 +-
 drivers/clk/meson/clk-pll.h                   |    2 +
 drivers/clk/meson/meson-a1-clkc.c             |   63 +
 drivers/clk/meson/meson-a1-clkc.h             |   25 +
 include/dt-bindings/clock/a1-clkc.h           |   98 +
 include/dt-bindings/clock/a1-pll-clkc.h       |   16 +
 16 files changed, 3240 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
 create mode 100644 drivers/clk/meson/a1-pll.c
 create mode 100644 drivers/clk/meson/a1-pll.h
 create mode 100644 drivers/clk/meson/a1.c
 create mode 100644 drivers/clk/meson/a1.h
 create mode 100644 drivers/clk/meson/meson-a1-clkc.c
 create mode 100644 drivers/clk/meson/meson-a1-clkc.h
 create mode 100644 include/dt-bindings/clock/a1-clkc.h
 create mode 100644 include/dt-bindings/clock/a1-pll-clkc.h

Comments

Rob Herring (Arm) Dec. 2, 2022, 4:10 a.m. UTC | #1
On Fri, 02 Dec 2022 01:56:55 +0300, Dmitry Rokosov wrote:
> From: Jian Hu <jian.hu@amlogic.com>
> 
> Add the documentation to support Amlogic A1 peripheral clock driver,
> and add A1 peripheral clock controller bindings.
> 
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  .../bindings/clock/amlogic,a1-clkc.yaml       | 65 ++++++++++++
>  include/dt-bindings/clock/a1-clkc.h           | 98 +++++++++++++++++++
>  2 files changed, 163 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
>  create mode 100644 include/dt-bindings/clock/a1-clkc.h
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/clock/amlogic,a1-clkc.yaml#
Documentation/devicetree/bindings/clock/amlogic,a1-clkc.example.dts:18.48-30.11: Warning (unit_address_vs_reg): /example-0/periphs-clock-controller: node has a reg or ranges property, but no unit name
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.example.dtb: periphs-clock-controller: reg: [[0, 2048], [0, 260]] is too long
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221201225703.6507-4-ddrokosov@sberdevices.ru

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Dmitry Rokosov Dec. 2, 2022, 9:49 a.m. UTC | #2
On Thu, Dec 01, 2022 at 10:10:04PM -0600, Rob Herring wrote:
> 
> On Fri, 02 Dec 2022 01:56:55 +0300, Dmitry Rokosov wrote:
> > From: Jian Hu <jian.hu@amlogic.com>
> > 
> > Add the documentation to support Amlogic A1 peripheral clock driver,
> > and add A1 peripheral clock controller bindings.
> > 
> > Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> > Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> > ---
> >  .../bindings/clock/amlogic,a1-clkc.yaml       | 65 ++++++++++++
> >  include/dt-bindings/clock/a1-clkc.h           | 98 +++++++++++++++++++
> >  2 files changed, 163 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
> >  create mode 100644 include/dt-bindings/clock/a1-clkc.h
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> ./Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml: $id: relative path/filename doesn't match actual path or filename
> 	expected: http://devicetree.org/schemas/clock/amlogic,a1-clkc.yaml#
> Documentation/devicetree/bindings/clock/amlogic,a1-clkc.example.dts:18.48-30.11: Warning (unit_address_vs_reg): /example-0/periphs-clock-controller: node has a reg or ranges property, but no unit name
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.example.dtb: periphs-clock-controller: reg: [[0, 2048], [0, 260]] is too long
> 	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221201225703.6507-4-ddrokosov@sberdevices.ru
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 

That's totally right warnings and errors. All of them are fixed in the
my "fixup" patch of Jian's original dt_binding schema:

https://lore.kernel.org/linux-amlogic/20221201225703.6507-11-ddrokosov@sberdevices.ru/
Krzysztof Kozlowski Dec. 2, 2022, 10:39 a.m. UTC | #3
On 02/12/2022 10:49, Dmitry Rokosov wrote:
> On Thu, Dec 01, 2022 at 10:10:04PM -0600, Rob Herring wrote:
>>
>> On Fri, 02 Dec 2022 01:56:55 +0300, Dmitry Rokosov wrote:
>>> From: Jian Hu <jian.hu@amlogic.com>
>>>
>>> Add the documentation to support Amlogic A1 peripheral clock driver,
>>> and add A1 peripheral clock controller bindings.
>>>
>>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>>> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
>>> ---
>>>  .../bindings/clock/amlogic,a1-clkc.yaml       | 65 ++++++++++++
>>>  include/dt-bindings/clock/a1-clkc.h           | 98 +++++++++++++++++++
>>>  2 files changed, 163 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
>>>  create mode 100644 include/dt-bindings/clock/a1-clkc.h
>>>
>>
>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> ./Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml: $id: relative path/filename doesn't match actual path or filename
>> 	expected: http://devicetree.org/schemas/clock/amlogic,a1-clkc.yaml#
>> Documentation/devicetree/bindings/clock/amlogic,a1-clkc.example.dts:18.48-30.11: Warning (unit_address_vs_reg): /example-0/periphs-clock-controller: node has a reg or ranges property, but no unit name
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.example.dtb: periphs-clock-controller: reg: [[0, 2048], [0, 260]] is too long
>> 	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
>>
>> doc reference errors (make refcheckdocs):
>>
>> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221201225703.6507-4-ddrokosov@sberdevices.ru
>>
>> The base for the series is generally the latest rc1. A different dependency
>> should be noted in *this* patch.
>>
>> If you already ran 'make dt_binding_check' and didn't see the above
>> error(s), then make sure 'yamllint' is installed and dt-schema is up to
>> date:
>>
>> pip3 install dtschema --upgrade
>>
>> Please check and re-submit after running the above command yourself. Note
>> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
>> your schema. However, it must be unset to test all examples with your schema.
>>
> 
> That's totally right warnings and errors. All of them are fixed in the
> my "fixup" patch of Jian's original dt_binding schema:
> 
> https://lore.kernel.org/linux-amlogic/20221201225703.6507-11-ddrokosov@sberdevices.ru/

No, this patch must be fixed. It's not correct.

Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 2, 2022, 10:40 a.m. UTC | #4
On 01/12/2022 23:57, Dmitry Rokosov wrote:
> During running dtbs_check and dt_binding_check checkers the following
> problems were found and resolved:
>     - $id is not correct, it has wrong url path
>     - CLKIDs aren't applied by names, just magic int constants there
>     - address and size cells are required for long reg definition
>     - wrong indentations
> 
> Also this patch adds new A1 clk controllers dt bindings to MAINTAINERS.
> 
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  .../bindings/clock/amlogic,a1-pll-clkc.yaml   | 27 ++++++++++++-------
>  MAINTAINERS                                   |  1 +
>  2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
> index d67250fbeece..83f98a73c04e 100644
> --- a/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
> +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>  %YAML 1.2
>  ---
> -$id: "http://devicetree.org/schemas/amlogic,a1-pll-clkc.yaml#"
> +$id: "http://devicetree.org/schemas/clock/amlogic,a1-pll-clkc.yaml#"
>  $schema: "http://devicetree.org/meta-schemas/core.yaml#"
>  
>  title: Amlogic Meson A/C serials PLL Clock Control Unit Device Tree Bindings

NAK.

This must be squashed.

Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 2, 2022, 10:40 a.m. UTC | #5
On 01/12/2022 23:57, Dmitry Rokosov wrote:
> During running dtbs_check and dt_binding_check checkers the following
> problems were found and resolved:
>     - $id is not correct, it has wrong url path
>     - no base offset in the dt node definition
>     - CLKIDs aren't applied by names, just magic int constants there
>     - address and size cells are required for long reg definition
>     - wrong indentations
> 
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  .../bindings/clock/amlogic,a1-clkc.yaml       | 36 +++++++++++--------
>  1 file changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
> index 7729850046cf..b0249ab21466 100644
> --- a/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
> +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
> @@ -1,7 +1,7 @@
> -#SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>  %YAML 1.2
>  ---
> -$id: "http://devicetree.org/schemas/amlogic,a1-clkc.yaml#"
> +$id: "http://devicetree.org/schemas/clock/amlogic,a1-clkc.yaml#"
>  $schema: "http://devicetree.org/meta-schemas/core.yaml#"
>  

NAK. This must be squashed.

Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 2, 2022, 10:43 a.m. UTC | #6
On 01/12/2022 23:56, Dmitry Rokosov wrote:
> From: Jian Hu <jian.hu@amlogic.com>
> 
> Add the documentation to support Amlogic A1 peripheral clock driver,
> and add A1 peripheral clock controller bindings.
> 
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  .../bindings/clock/amlogic,a1-clkc.yaml       | 65 ++++++++++++
>  include/dt-bindings/clock/a1-clkc.h           | 98 +++++++++++++++++++
>  2 files changed, 163 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
>  create mode 100644 include/dt-bindings/clock/a1-clkc.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
> new file mode 100644
> index 000000000000..7729850046cf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
> @@ -0,0 +1,65 @@
> +#SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/amlogic,a1-clkc.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Amlogic Meson A/C serials Peripheral Clock Control Unit Device Tree Bindings

Same comments as with previous patch. All of them.


Best regards,
Krzysztof
Dmitry Rokosov Dec. 2, 2022, 10:58 a.m. UTC | #7
Hello Krzysztof,

Thank you for quick review. Let me explain you why I sent Jian broken
patches and applied my fixup patches above.

I've found several versions of A1 clkc drivers from Jian Hu Amlogic engineer,
the last one version was a couple years ago.
I've reworked the last (v7) version and didn't know the correct way to apply
all fixes.
Jerome Brunet Dec. 2, 2022, 11:16 a.m. UTC | #8
On Fri 02 Dec 2022 at 01:56, Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:

> From: Jian Hu <jian.hu@amlogic.com>
>
> The Amlogic A1 clock includes three drivers:
> pll clocks, peripheral clocks, CPU clocks.
> sys pll and CPU clocks will be sent in next patch.
>
> Unlike the previous series, there is no EE/AO domain
> in A1 CLK controllers.
>
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  drivers/clk/meson/Kconfig  |   9 +
>  drivers/clk/meson/Makefile |   1 +
>  drivers/clk/meson/a1-pll.c | 360 +++++++++++++++++++++++++++++++++++++
>  drivers/clk/meson/a1-pll.h |  56 ++++++
>  4 files changed, 426 insertions(+)
>  create mode 100644 drivers/clk/meson/a1-pll.c
>  create mode 100644 drivers/clk/meson/a1-pll.h
>
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index fc002c155bc3..ab34662b24f0 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -99,6 +99,15 @@ config COMMON_CLK_AXG_AUDIO
>  	  Support for the audio clock controller on AmLogic A113D devices,
>  	  aka axg, Say Y if you want audio subsystem to work.
>  
> +config COMMON_CLK_A1_PLL
> +	bool

Could you add a tristate with some text please ?

> +	depends on ARCH_MESON
> +	select COMMON_CLK_MESON_REGMAP
> +	select COMMON_CLK_MESON_PLL
> +	help
> +	  Support for the PLL clock controller on Amlogic A113L device,
> +	  aka a1. Say Y if you want PLL to work.
> +
>  config COMMON_CLK_G12A
>  	tristate "G12 and SM1 SoC clock controllers support"
>  	depends on ARM64
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 6eca2a406ee3..2f17f475a48f 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
>  
>  obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
> +obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>  obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
> diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
> new file mode 100644
> index 000000000000..69c1ca07d041
> --- /dev/null
> +++ b/drivers/clk/meson/a1-pll.c
> @@ -0,0 +1,360 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + * Author: Jian Hu <jian.hu@amlogic.com>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include "a1-pll.h"
> +#include "clk-pll.h"
> +#include "clk-regmap.h"
> +
> +static struct clk_regmap a1_fixed_pll_dco = {
> +	.data = &(struct meson_clk_pll_data){
> +		.en = {
> +			.reg_off = ANACTRL_FIXPLL_CTRL0,
> +			.shift   = 28,
> +			.width   = 1,
> +		},
> +		.m = {
> +			.reg_off = ANACTRL_FIXPLL_CTRL0,
> +			.shift   = 0,
> +			.width   = 8,
> +		},
> +		.n = {
> +			.reg_off = ANACTRL_FIXPLL_CTRL0,
> +			.shift   = 10,
> +			.width   = 5,
> +		},
> +		.frac = {
> +			.reg_off = ANACTRL_FIXPLL_CTRL1,
> +			.shift   = 0,
> +			.width   = 19,
> +		},
> +		.l = {
> +			.reg_off = ANACTRL_FIXPLL_STS,
> +			.shift   = 31,
> +			.width   = 1,
> +		},
> +		.rst = {
> +			.reg_off = ANACTRL_FIXPLL_CTRL0,
> +			.shift   = 29,
> +			.width   = 1,
> +		},
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "fixed_pll_dco",
> +		.ops = &meson_clk_pll_ro_ops,
> +		.parent_data = &(const struct clk_parent_data) {
> +			.fw_name = "xtal_fixpll",
> +		},
> +		.num_parents = 1,
> +	},
> +};
> +
> +static struct clk_regmap a1_fixed_pll = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = ANACTRL_FIXPLL_CTRL0,
> +		.bit_idx = 20,
> +	},
> +	.hw.init = &(struct clk_init_data) {
> +		.name = "fixed_pll",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_fixed_pll_dco.hw
> +		},
> +		.num_parents = 1,
> +		/*
> +		 * It is enough that the fdiv leaf has critical flag,
> +		 * No critical or unused flag here.
> +		 */
> +	},
> +};
> +
> +static const struct pll_mult_range a1_hifi_pll_mult_range = {
> +	.min = 32,
> +	.max = 64,
> +};
> +
> +static const struct reg_sequence a1_hifi_init_regs[] = {
> +	{ .reg = ANACTRL_HIFIPLL_CTRL1, .def = 0x01800000 },
> +	{ .reg = ANACTRL_HIFIPLL_CTRL2, .def = 0x00001100 },
> +	{ .reg = ANACTRL_HIFIPLL_CTRL3, .def = 0x100a1100 },
> +	{ .reg = ANACTRL_HIFIPLL_CTRL4, .def = 0x00302000 },
> +	{ .reg = ANACTRL_HIFIPLL_CTRL0, .def = 0x01f18440 },
> +};
> +
> +static struct clk_regmap a1_hifi_pll = {
> +	.data = &(struct meson_clk_pll_data){
> +		.en = {
> +			.reg_off = ANACTRL_HIFIPLL_CTRL0,
> +			.shift   = 28,
> +			.width   = 1,
> +		},
> +		.m = {
> +			.reg_off = ANACTRL_HIFIPLL_CTRL0,
> +			.shift   = 0,
> +			.width   = 8,
> +		},
> +		.n = {
> +			.reg_off = ANACTRL_HIFIPLL_CTRL0,
> +			.shift   = 10,
> +			.width   = 5,
> +		},
> +		.frac = {
> +			.reg_off = ANACTRL_HIFIPLL_CTRL1,
> +			.shift   = 0,
> +			.width   = 19,
> +		},
> +		.l = {
> +			.reg_off = ANACTRL_HIFIPLL_STS,
> +			.shift   = 31,
> +			.width   = 1,
> +		},
> +		.current_en = {
> +			.reg_off = ANACTRL_HIFIPLL_CTRL0,
> +			.shift   = 26,
> +			.width   = 1,
> +		},
> +		.l_detect = {
> +			.reg_off = ANACTRL_HIFIPLL_CTRL2,
> +			.shift   = 6,
> +			.width   = 1,
> +		},
> +		.range = &a1_hifi_pll_mult_range,
> +		.init_regs = a1_hifi_init_regs,
> +		.init_count = ARRAY_SIZE(a1_hifi_init_regs),
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "hifi_pll",
> +		.ops = &meson_clk_pll_ops,
> +		.parent_data = &(const struct clk_parent_data) {
> +			.fw_name = "xtal_hifipll",
> +		},
> +		.num_parents = 1,
> +	},
> +};
> +
> +static struct clk_fixed_factor a1_fclk_div2_div = {
> +	.mult = 1,
> +	.div = 2,
> +	.hw.init = &(struct clk_init_data){
> +		.name = "fclk_div2_div",
> +		.ops = &clk_fixed_factor_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_fixed_pll.hw
> +		},
> +		.num_parents = 1,
> +	},
> +};
> +
> +static struct clk_regmap a1_fclk_div2 = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = ANACTRL_FIXPLL_CTRL0,
> +		.bit_idx = 21,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "fclk_div2",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_fclk_div2_div.hw
> +		},
> +		.num_parents = 1,
> +		/*
> +		 * This clock is used by DDR clock in BL2 firmware
> +		 * and is required by the platform to operate correctly.
> +		 * Until the following condition are met, we need this clock to
> +		 * be marked as critical:
> +		 * a) Mark the clock used by a firmware resource, if possible
> +		 * b) CCF has a clock hand-off mechanism to make the sure the
> +		 *    clock stays on until the proper driver comes along
> +		 */
> +		.flags = CLK_IS_CRITICAL,
> +	},
> +};
> +
> +static struct clk_fixed_factor a1_fclk_div3_div = {
> +	.mult = 1,
> +	.div = 3,
> +	.hw.init = &(struct clk_init_data){
> +		.name = "fclk_div3_div",
> +		.ops = &clk_fixed_factor_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_fixed_pll.hw
> +		},
> +		.num_parents = 1,
> +	},
> +};
> +
> +static struct clk_regmap a1_fclk_div3 = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = ANACTRL_FIXPLL_CTRL0,
> +		.bit_idx = 22,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "fclk_div3",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_fclk_div3_div.hw
> +		},
> +		.num_parents = 1,
> +		/*
> +		 * This clock is used by APB bus which is set in boot ROM code
> +		 * and is required by the platform to operate correctly.
> +		 * About critical, refer to a1_fclk_div2.
> +		 */
> +		.flags = CLK_IS_CRITICAL,
> +	},
> +};
> +
> +static struct clk_fixed_factor a1_fclk_div5_div = {
> +	.mult = 1,
> +	.div = 5,
> +	.hw.init = &(struct clk_init_data){
> +		.name = "fclk_div5_div",
> +		.ops = &clk_fixed_factor_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_fixed_pll.hw
> +		},
> +		.num_parents = 1,
> +	},
> +};
> +
> +static struct clk_regmap a1_fclk_div5 = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = ANACTRL_FIXPLL_CTRL0,
> +		.bit_idx = 23,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "fclk_div5",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_fclk_div5_div.hw
> +		},
> +		.num_parents = 1,
> +		/*
> +		 * This clock is used by AXI bus which setted in Romcode
> +		 * and is required by the platform to operate correctly.
> +		 * About critical, refer to a1_fclk_div2.
> +		 */
> +		.flags = CLK_IS_CRITICAL,
> +	},
> +};
> +
> +static struct clk_fixed_factor a1_fclk_div7_div = {
> +	.mult = 1,
> +	.div = 7,
> +	.hw.init = &(struct clk_init_data){
> +		.name = "fclk_div7_div",
> +		.ops = &clk_fixed_factor_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_fixed_pll.hw
> +		},
> +		.num_parents = 1,
> +	},
> +};
> +
> +static struct clk_regmap a1_fclk_div7 = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = ANACTRL_FIXPLL_CTRL0,
> +		.bit_idx = 24,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "fclk_div7",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_fclk_div7_div.hw
> +		},
> +		.num_parents = 1,
> +	},
> +};
> +
> +/* Array of all clocks provided by this provider */
> +static struct clk_hw_onecell_data a1_pll_hw_onecell_data = {
> +	.hws = {
> +		[CLKID_FIXED_PLL_DCO]		= &a1_fixed_pll_dco.hw,
> +		[CLKID_FIXED_PLL]		= &a1_fixed_pll.hw,
> +		[CLKID_HIFI_PLL]		= &a1_hifi_pll.hw,
> +		[CLKID_FCLK_DIV2]		= &a1_fclk_div2.hw,
> +		[CLKID_FCLK_DIV3]		= &a1_fclk_div3.hw,
> +		[CLKID_FCLK_DIV5]		= &a1_fclk_div5.hw,
> +		[CLKID_FCLK_DIV7]		= &a1_fclk_div7.hw,
> +		[CLKID_FCLK_DIV2_DIV]		= &a1_fclk_div2_div.hw,
> +		[CLKID_FCLK_DIV3_DIV]		= &a1_fclk_div3_div.hw,
> +		[CLKID_FCLK_DIV5_DIV]		= &a1_fclk_div5_div.hw,
> +		[CLKID_FCLK_DIV7_DIV]		= &a1_fclk_div7_div.hw,
> +		[NR_PLL_CLKS]			= NULL,
> +	},
> +	.num = NR_PLL_CLKS,
> +};
> +
> +static struct clk_regmap *const a1_pll_regmaps[] = {
> +	&a1_fixed_pll_dco,
> +	&a1_fixed_pll,
> +	&a1_hifi_pll,
> +	&a1_fclk_div2,
> +	&a1_fclk_div3,
> +	&a1_fclk_div5,
> +	&a1_fclk_div7,
> +};
> +
> +static struct regmap_config clkc_regmap_config = {
> +	.reg_bits       = 32,
> +	.val_bits       = 32,
> +	.reg_stride     = 4,
> +};
> +
> +static int meson_a1_pll_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	void __iomem *base;
> +	struct regmap *map;
> +	int ret, i;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);

devm_platform_ioremap_resource ?

> +
> +	map = devm_regmap_init_mmio(dev, base, &clkc_regmap_config);
> +	if (IS_ERR(map))
> +		return PTR_ERR(map);
> +
> +	/* Populate regmap for the regmap backed clocks */
> +	for (i = 0; i < ARRAY_SIZE(a1_pll_regmaps); i++)
> +		a1_pll_regmaps[i]->map = map;
> +
> +	for (i = 0; i < a1_pll_hw_onecell_data.num; i++) {
> +		/* array might be sparse */
> +		if (!a1_pll_hw_onecell_data.hws[i])
> +			continue;
> +
> +		ret = devm_clk_hw_register(dev, a1_pll_hw_onecell_data.hws[i]);
> +		if (ret) {
> +			dev_err(dev, "Clock registration failed\n");
> +			return ret;
> +		}
> +	}
> +
> +	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> +					   &a1_pll_hw_onecell_data);
> +}
> +
> +static const struct of_device_id clkc_match_table[] = {
> +	{ .compatible = "amlogic,a1-pll-clkc", },
> +	{}
> +};
> +
> +static struct platform_driver a1_pll_driver = {
> +	.probe		= meson_a1_pll_probe,
> +	.driver		= {
> +		.name	= "a1-pll-clkc",
> +		.of_match_table = clkc_match_table,
> +	},
> +};
> +
> +builtin_platform_driver(a1_pll_driver);
> diff --git a/drivers/clk/meson/a1-pll.h b/drivers/clk/meson/a1-pll.h
> new file mode 100644
> index 000000000000..8ded267061ad
> --- /dev/null
> +++ b/drivers/clk/meson/a1-pll.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + */
> +
> +#ifndef __A1_PLL_H
> +#define __A1_PLL_H
> +
> +/* PLL register offset */
> +#define ANACTRL_FIXPLL_CTRL0		0x0
> +#define ANACTRL_FIXPLL_CTRL1		0x4
> +#define ANACTRL_FIXPLL_CTRL2		0x8
> +#define ANACTRL_FIXPLL_CTRL3		0xc
> +#define ANACTRL_FIXPLL_CTRL4		0x10
> +#define ANACTRL_FIXPLL_STS		0x14
> +#define ANACTRL_SYSPLL_CTRL0		0x80
> +#define ANACTRL_SYSPLL_CTRL1		0x84
> +#define ANACTRL_SYSPLL_CTRL2		0x88
> +#define ANACTRL_SYSPLL_CTRL3		0x8c
> +#define ANACTRL_SYSPLL_CTRL4		0x90
> +#define ANACTRL_SYSPLL_STS		0x94
> +#define ANACTRL_HIFIPLL_CTRL0		0xc0
> +#define ANACTRL_HIFIPLL_CTRL1		0xc4
> +#define ANACTRL_HIFIPLL_CTRL2		0xc8
> +#define ANACTRL_HIFIPLL_CTRL3		0xcc
> +#define ANACTRL_HIFIPLL_CTRL4		0xd0
> +#define ANACTRL_HIFIPLL_STS		0xd4
> +#define ANACTRL_AUDDDS_CTRL0		0x100
> +#define ANACTRL_AUDDDS_CTRL1		0x104
> +#define ANACTRL_AUDDDS_CTRL2		0x108
> +#define ANACTRL_AUDDDS_CTRL3		0x10c
> +#define ANACTRL_AUDDDS_CTRL4		0x110
> +#define ANACTRL_AUDDDS_STS		0x114
> +#define ANACTRL_MISCTOP_CTRL0		0x140
> +#define ANACTRL_POR_CNTL		0x188
> +
> +/*
> + * CLKID index values
> + *
> + * These indices are entirely contrived and do not map onto the hardware.
> + * It has now been decided to expose everything by default in the DT header:
> + * include/dt-bindings/clock/a1-pll-clkc.h. Only the clocks ids we don't want
> + * to expose, such as the internal muxes and dividers of composite clocks,
> + * will remain defined here.
> + */
> +#define CLKID_FIXED_PLL_DCO		0
> +#define CLKID_FCLK_DIV2_DIV		2
> +#define CLKID_FCLK_DIV3_DIV		3
> +#define CLKID_FCLK_DIV5_DIV		4
> +#define CLKID_FCLK_DIV7_DIV		5
> +#define NR_PLL_CLKS			11
> +
> +/* include the CLKIDs that have been made part of the DT binding */
> +#include <dt-bindings/clock/a1-pll-clkc.h>
> +
> +#endif /* __A1_PLL_H */
Dmitry Rokosov Dec. 2, 2022, 11:31 a.m. UTC | #9
On Fri, Dec 02, 2022 at 12:16:12PM +0100, Jerome Brunet wrote:
> >  drivers/clk/meson/Kconfig  |   9 +
> >  drivers/clk/meson/Makefile |   1 +
> >  drivers/clk/meson/a1-pll.c | 360 +++++++++++++++++++++++++++++++++++++
> >  drivers/clk/meson/a1-pll.h |  56 ++++++
> >  4 files changed, 426 insertions(+)
> >  create mode 100644 drivers/clk/meson/a1-pll.c
> >  create mode 100644 drivers/clk/meson/a1-pll.h
> >
> > diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> > index fc002c155bc3..ab34662b24f0 100644
> > --- a/drivers/clk/meson/Kconfig
> > +++ b/drivers/clk/meson/Kconfig
> > @@ -99,6 +99,15 @@ config COMMON_CLK_AXG_AUDIO
> >  	  Support for the audio clock controller on AmLogic A113D devices,
> >  	  aka axg, Say Y if you want audio subsystem to work.
> >  
> > +config COMMON_CLK_A1_PLL
> > +	bool
> 
> Could you add a tristate with some text please ?
> 

Yep... I did it in my fixup patches :-) Looks like it's better to review
the next version with already squashed patches.
Jerome Brunet Dec. 2, 2022, 11:36 a.m. UTC | #10
On Fri 02 Dec 2022 at 01:56, Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:

> Generally, A1 SoC has four clock controllers on the board: PLL,
> Peripherals, CPU, and Audio. The audio clock controller is different
> from others, but the rest are very similar from a functional and regmap
> point of view. So a it's good idea to generalize some routines for all
> of them. Exactly, meson-a1-clkc driver contains the common probe() flow.
>
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>

I think you should leave this out for the initial submission. It makes
harder to review.

In some case, these factorizations actually the maitenance harder.

There is also the s4 that is coming up. It looks similar to the A1 as
well.

Let's wait for both them to land, see how it goes and then we'll decide
if a factorization is appropriate.

> ---
>  drivers/clk/meson/Kconfig         |  4 ++
>  drivers/clk/meson/Makefile        |  1 +
>  drivers/clk/meson/meson-a1-clkc.c | 63 +++++++++++++++++++++++++++++++
>  drivers/clk/meson/meson-a1-clkc.h | 25 ++++++++++++
>  4 files changed, 93 insertions(+)
>  create mode 100644 drivers/clk/meson/meson-a1-clkc.c
>  create mode 100644 drivers/clk/meson/meson-a1-clkc.h
>
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index bd44ba47200e..1c885541c3a9 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -43,6 +43,10 @@ config COMMON_CLK_MESON_CPU_DYNDIV
>  	tristate
>  	select COMMON_CLK_MESON_REGMAP
>  
> +config COMMON_CLK_MESON_A1_CLKC
> +	tristate
> +	select COMMON_CLK_MESON_REGMAP
> +
>  config COMMON_CLK_MESON8B
>  	bool "Meson8 SoC Clock controller support"
>  	depends on ARM
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 0e6f293c05d4..15136d861a65 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
>  obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
>  obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
>  obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
> +obj-$(CONFIG_COMMON_CLK_MESON_A1_CLKC) += meson-a1-clkc.o
>  
>  # Amlogic Clock controllers
>  
> diff --git a/drivers/clk/meson/meson-a1-clkc.c b/drivers/clk/meson/meson-a1-clkc.c
> new file mode 100644
> index 000000000000..2fe320a0e16e
> --- /dev/null
> +++ b/drivers/clk/meson/meson-a1-clkc.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Amlogic Meson-A1 Clock Controller Driver
> + *
> + * Copyright (c) 2022, SberDevices. All Rights Reserved.
> + * Author: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> + */
> +
> +#include <linux/of_device.h>
> +#include "meson-a1-clkc.h"
> +
> +static struct regmap_config clkc_regmap_config = {
> +	.reg_bits   = 32,
> +	.val_bits   = 32,
> +	.reg_stride = 4,
> +};
> +
> +int meson_a1_clkc_probe(struct platform_device *pdev)
> +{
> +	struct meson_a1_clkc_data *clkc;
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	void __iomem *base;
> +	struct regmap *map;
> +	int clkid, i, err;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return dev_err_probe(dev, -ENXIO, "can't get IO resource\n");
> +
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base))
> +		return dev_err_probe(dev, PTR_ERR(base),
> +				     "can't ioremap resource %pr\n", res);
> +
> +	map = devm_regmap_init_mmio(dev, base, &clkc_regmap_config);
> +	if (IS_ERR(map))
> +		return dev_err_probe(dev, PTR_ERR(map),
> +				     "can't init regmap mmio region\n");
> +
> +	clkc = (struct meson_a1_clkc_data *)of_device_get_match_data(dev);
> +	if (!clkc)
> +		return dev_err_probe(dev, -ENODEV,
> +				     "can't get A1 clkc driver data\n");
> +
> +	/* Populate regmap for the regmap backed clocks */
> +	for (i = 0; i < clkc->num_regs; i++)
> +		clkc->regs[i]->map = map;
> +
> +	for (clkid = 0; clkid < clkc->hw->num; clkid++) {
> +		err = devm_clk_hw_register(dev, clkc->hw->hws[clkid]);
> +		if (err)
> +			return dev_err_probe(dev, err,
> +					     "clock registration failed\n");
> +	}
> +
> +	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> +					   (void *)clkc->hw);
> +}
> +EXPORT_SYMBOL_GPL(meson_a1_clkc_probe);
> +
> +MODULE_AUTHOR("Dmitry Rokosov <ddrokosov@sberdevices.ru>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/clk/meson/meson-a1-clkc.h b/drivers/clk/meson/meson-a1-clkc.h
> new file mode 100644
> index 000000000000..503eca0f6cb5
> --- /dev/null
> +++ b/drivers/clk/meson/meson-a1-clkc.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Amlogic Meson-A1 Clock Controller driver
> + *
> + * Copyright (c) 2022, SberDevices. All Rights Reserved.
> + * Author: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> + */
> +
> +#ifndef __MESON_A1_CLKC_H__
> +#define __MESON_A1_CLKC_H__
> +
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include "clk-regmap.h"
> +
> +struct meson_a1_clkc_data {
> +	const struct clk_hw_onecell_data *hw;
> +	struct clk_regmap *const *regs;
> +	size_t num_regs;
> +};
> +
> +int meson_a1_clkc_probe(struct platform_device *pdev);
> +#endif
Dmitry Rokosov Dec. 2, 2022, 11:58 a.m. UTC | #11
On Fri, Dec 02, 2022 at 12:36:50PM +0100, Jerome Brunet wrote:
> 
> On Fri 02 Dec 2022 at 01:56, Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
> 
> > Generally, A1 SoC has four clock controllers on the board: PLL,
> > Peripherals, CPU, and Audio. The audio clock controller is different
> > from others, but the rest are very similar from a functional and regmap
> > point of view. So a it's good idea to generalize some routines for all
> > of them. Exactly, meson-a1-clkc driver contains the common probe() flow.
> >
> > Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> 
> I think you should leave this out for the initial submission. It makes
> harder to review.
> 
> In some case, these factorizations actually the maitenance harder.
> 
> There is also the s4 that is coming up. It looks similar to the A1 as
> well.
> 
> Let's wait for both them to land, see how it goes and then we'll decide
> if a factorization is appropriate.
> 

Jerome,

Sorry, let me double check a little bit that I get your point correctly.

Do you mean due to s4 development in parallel it's easier to have own
probe sequence for each driver in the first stage (until A1 and S4 clock
controllers not merged to upstream) and after think about some
"generalization", right?

...
Jerome Brunet Dec. 2, 2022, 12:01 p.m. UTC | #12
On Fri 02 Dec 2022 at 01:57, Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:

> Summary changes:
>     - fixed up clk_summary kernel panic due to missing a1_pad_ctrl
>       clk_regmap definition
>     - supported meson-a1-clkc common driver
>     - aligned CLKID-related definitions with CLKID list from order
>       perspective to remove holes and permutations
>     - corrected Kconfig dependencies and types
>     - provided correct MODULE_AUTHORs() and MODULE_LICENSE()
>     - optimized and fix up some clock relationships and parents
>       references
>     - removed unused register offset definitions

again, list in commit description a hint things are mixed up

>
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  drivers/clk/meson/Kconfig |   7 +-
>  drivers/clk/meson/a1.c    | 591 ++++++++++++++++++--------------------
>  drivers/clk/meson/a1.h    |  16 +-
>  3 files changed, 292 insertions(+), 322 deletions(-)
>
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index deb273673ec1..cabe63bf23f5 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -114,13 +114,14 @@ config COMMON_CLK_A1_PLL
>  	  aka a1. Say Y if you want PLL to work.
>  
>  config COMMON_CLK_A1
> -	bool
> -	depends on ARCH_MESON
> +	tristate "Meson A1 SoC clock controller support"
> +	depends on ARM64
>  	select COMMON_CLK_MESON_DUALDIV
>  	select COMMON_CLK_MESON_REGMAP
> +	select COMMON_CLK_MESON_A1_CLKC
>  	help
>  	  Support for the Peripheral clock controller on Amlogic A113L device,
> -	  aka a1. Say Y if you want Peripherals to work.
> +	  aka a1. Say Y if you want clock peripherals controller to work.
>  
>  config COMMON_CLK_G12A
>  	tristate "G12 and SM1 SoC clock controllers support"
> diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
> index 2cf20ae1db75..c9b7f09823f8 100644
> --- a/drivers/clk/meson/a1.c
> +++ b/drivers/clk/meson/a1.c
> @@ -2,6 +2,9 @@
>  /*
>   * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>   * Author: Jian Hu <jian.hu@amlogic.com>
> + *
> + * Copyright (c) 2022, SberDevices. All Rights Reserved.
> + * Author: Dmitry Rokosov <ddrokosov@sberdevices.ru>
>   */
>  
>  #include <linux/clk-provider.h>
> @@ -10,6 +13,7 @@
>  #include "a1.h"
>  #include "clk-dualdiv.h"
>  #include "clk-regmap.h"
> +#include "meson-a1-clkc.h"
>  
>  static struct clk_regmap a1_xtal_clktree = {
>  	.data = &(struct clk_regmap_gate_data){
> @@ -116,11 +120,128 @@ static struct clk_regmap a1_xtal_dds = {
>  	},
>  };
>  
> +static struct clk_regmap a1_rtc_32k_clkin = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = RTC_BY_OSCIN_CTRL0,
> +		.bit_idx = 31,
> +	},
> +	.hw.init = &(struct clk_init_data) {
> +		.name = "rtc_32k_clkin",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_data = &(const struct clk_parent_data) {
> +			.fw_name = "xtal",
> +		},
> +		.num_parents = 1,
> +	},
> +};
> +
> +static const struct meson_clk_dualdiv_param a1_32k_div_table[] = {
> +	{
> +		.dual		= 1,
> +		.n1		= 733,
> +		.m1		= 8,
> +		.n2		= 732,
> +		.m2		= 11,
> +	},
> +	{}
> +};
> +
> +static struct clk_regmap a1_rtc_32k_div = {
> +	.data = &(struct meson_clk_dualdiv_data){
> +		.n1 = {
> +			.reg_off = RTC_BY_OSCIN_CTRL0,
> +			.shift   = 0,
> +			.width   = 12,
> +		},
> +		.n2 = {
> +			.reg_off = RTC_BY_OSCIN_CTRL0,
> +			.shift   = 12,
> +			.width   = 12,
> +		},
> +		.m1 = {
> +			.reg_off = RTC_BY_OSCIN_CTRL1,
> +			.shift   = 0,
> +			.width   = 12,
> +		},
> +		.m2 = {
> +			.reg_off = RTC_BY_OSCIN_CTRL1,
> +			.shift   = 12,
> +			.width   = 12,
> +		},
> +		.dual = {
> +			.reg_off = RTC_BY_OSCIN_CTRL0,
> +			.shift   = 28,
> +			.width   = 1,
> +		},
> +		.table = a1_32k_div_table,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "rtc_32k_div",
> +		.ops = &meson_clk_dualdiv_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_rtc_32k_clkin.hw
> +		},
> +		.num_parents = 1,
> +	},
> +};
> +
> +static struct clk_regmap a1_rtc_32k_xtal = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = RTC_BY_OSCIN_CTRL1,
> +		.bit_idx = 24,
> +	},
> +	.hw.init = &(struct clk_init_data) {
> +		.name = "rtc_32k_xtal",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_rtc_32k_clkin.hw
> +		},
> +		.num_parents = 1,
> +	},
> +};
> +
> +static struct clk_regmap a1_rtc_32k_sel = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = RTC_CTRL,
> +		.mask = 0x3,
> +		.shift = 0,
> +		.flags = CLK_MUX_ROUND_CLOSEST,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "rtc_32k_sel",
> +		.ops = &clk_regmap_mux_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_rtc_32k_xtal.hw,
> +			&a1_rtc_32k_div.hw,
> +		},
> +		.num_parents = 2,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +struct clk_regmap a1_rtc_clk = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = RTC_BY_OSCIN_CTRL0,
> +		.bit_idx = 30,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "rtc_clk",
> +		.ops = &clk_regmap_gate_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_rtc_32k_sel.hw
> +		},
> +		.num_parents = 1,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static u32 mux_table_sys_clk[] = { 0, 1, 2, 3, 7 };
>  static const struct clk_parent_data sys_clk_parents[] = {
>  	{ .fw_name = "xtal" },
>  	{ .fw_name = "fclk_div2" },
>  	{ .fw_name = "fclk_div3" },
>  	{ .fw_name = "fclk_div5" },
> +	{ .hw = &a1_rtc_clk.hw },
>  };
>  
>  static struct clk_regmap a1_sys_b_sel = {
> @@ -128,6 +249,7 @@ static struct clk_regmap a1_sys_b_sel = {
>  		.offset = SYS_CLK_CTRL0,
>  		.mask = 0x7,
>  		.shift = 26,
> +		.table = mux_table_sys_clk,
>  	},
>  	.hw.init = &(struct clk_init_data){
>  		.name = "sys_b_sel",
> @@ -175,6 +297,7 @@ static struct clk_regmap a1_sys_a_sel = {
>  		.offset = SYS_CLK_CTRL0,
>  		.mask = 0x7,
>  		.shift = 10,
> +		.table = mux_table_sys_clk,
>  	},
>  	.hw.init = &(struct clk_init_data){
>  		.name = "sys_a_sel",
> @@ -227,7 +350,8 @@ static struct clk_regmap a1_sys_clk = {
>  		.name = "sys_clk",
>  		.ops = &clk_regmap_mux_ro_ops,
>  		.parent_hws = (const struct clk_hw *[]) {
> -			&a1_sys_a.hw, &a1_sys_b.hw,
> +			&a1_sys_a.hw,
> +			&a1_sys_b.hw,
>  		},
>  		.num_parents = 2,
>  		/*
> @@ -243,121 +367,6 @@ static struct clk_regmap a1_sys_clk = {
>  	},
>  };
>  
> -static struct clk_regmap a1_rtc_32k_clkin = {
> -	.data = &(struct clk_regmap_gate_data){
> -		.offset = RTC_BY_OSCIN_CTRL0,
> -		.bit_idx = 31,
> -	},
> -	.hw.init = &(struct clk_init_data) {
> -		.name = "rtc_32k_clkin",
> -		.ops = &clk_regmap_gate_ops,
> -		.parent_data = &(const struct clk_parent_data) {
> -			.fw_name = "xtal",
> -		},
> -		.num_parents = 1,
> -	},
> -};
> -
> -static const struct meson_clk_dualdiv_param a1_32k_div_table[] = {
> -	{
> -		.dual		= 1,
> -		.n1		= 733,
> -		.m1		= 8,
> -		.n2		= 732,
> -		.m2		= 11,
> -	},
> -	{}
> -};
> -
> -static struct clk_regmap a1_rtc_32k_div = {
> -	.data = &(struct meson_clk_dualdiv_data){
> -		.n1 = {
> -			.reg_off = RTC_BY_OSCIN_CTRL0,
> -			.shift   = 0,
> -			.width   = 12,
> -		},
> -		.n2 = {
> -			.reg_off = RTC_BY_OSCIN_CTRL0,
> -			.shift   = 12,
> -			.width   = 12,
> -		},
> -		.m1 = {
> -			.reg_off = RTC_BY_OSCIN_CTRL1,
> -			.shift   = 0,
> -			.width   = 12,
> -		},
> -		.m2 = {
> -			.reg_off = RTC_BY_OSCIN_CTRL1,
> -			.shift   = 12,
> -			.width   = 12,
> -		},
> -		.dual = {
> -			.reg_off = RTC_BY_OSCIN_CTRL0,
> -			.shift   = 28,
> -			.width   = 1,
> -		},
> -		.table = a1_32k_div_table,
> -	},
> -	.hw.init = &(struct clk_init_data){
> -		.name = "rtc_32k_div",
> -		.ops = &meson_clk_dualdiv_ops,
> -		.parent_hws = (const struct clk_hw *[]) {
> -			&a1_rtc_32k_clkin.hw
> -		},
> -		.num_parents = 1,
> -	},
> -};
> -
> -static struct clk_regmap a1_rtc_32k_xtal = {
> -	.data = &(struct clk_regmap_gate_data){
> -		.offset = RTC_BY_OSCIN_CTRL1,
> -		.bit_idx = 24,
> -	},
> -	.hw.init = &(struct clk_init_data) {
> -		.name = "rtc_32k_xtal",
> -		.ops = &clk_regmap_gate_ops,
> -		.parent_hws = (const struct clk_hw *[]) {
> -			&a1_rtc_32k_clkin.hw
> -		},
> -		.num_parents = 1,
> -	},
> -};
> -
> -static struct clk_regmap a1_rtc_32k_sel = {
> -	.data = &(struct clk_regmap_mux_data) {
> -		.offset = RTC_CTRL,
> -		.mask = 0x3,
> -		.shift = 0,
> -		.flags = CLK_MUX_ROUND_CLOSEST,
> -	},
> -	.hw.init = &(struct clk_init_data){
> -		.name = "rtc_32k_sel",
> -		.ops = &clk_regmap_mux_ops,
> -		.parent_hws = (const struct clk_hw *[]) {
> -			&a1_rtc_32k_xtal.hw,
> -			&a1_rtc_32k_div.hw,
> -		},
> -		.num_parents = 2,
> -		.flags = CLK_SET_RATE_PARENT,
> -	},
> -};
> -
> -struct clk_regmap a1_rtc_clk = {
> -	.data = &(struct clk_regmap_gate_data){
> -		.offset = RTC_BY_OSCIN_CTRL0,
> -		.bit_idx = 30,
> -	},
> -	.hw.init = &(struct clk_init_data){
> -		.name = "rtc_clk",
> -		.ops = &clk_regmap_gate_ops,
> -		.parent_hws = (const struct clk_hw *[]) {
> -			&a1_rtc_32k_sel.hw
> -		},
> -		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> -	},
> -};
> -
>  static u32 mux_table_dsp_ab[] = { 0, 1, 2, 3, 4, 7 };
>  static const struct clk_parent_data dsp_ab_clk_parent_data[] = {
>  	{ .fw_name = "xtal", },
> @@ -475,9 +484,9 @@ static struct clk_regmap a1_dspa_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "dspa_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		.parent_data = (const struct clk_parent_data []) {
> -			{ .hw = &a1_dspa_a.hw },
> -			{ .hw = &a1_dspa_b.hw },
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&a1_dspa_a.hw,
> +			&a1_dspa_b.hw,
>  		},
>  		.num_parents = 2,
>  		.flags = CLK_SET_RATE_PARENT,
> @@ -624,7 +633,8 @@ static struct clk_regmap a1_dspb_sel = {
>  		.name = "dspb_sel",
>  		.ops = &clk_regmap_mux_ops,
>  		.parent_hws = (const struct clk_hw *[]) {
> -			&a1_dspb_a.hw, &a1_dspb_b.hw,
> +			&a1_dspb_a.hw,
> +			&a1_dspb_b.hw,
>  		},
>  		.num_parents = 2,
>  		.flags = CLK_SET_RATE_PARENT,
> @@ -852,6 +862,12 @@ static struct clk_regmap a1_saradc_clk = {
>  	},
>  };
>  
> +static const struct clk_parent_data pwm_abcd_parents[] = {
> +	{ .fw_name = "xtal", },
> +	{ .hw = &a1_sys_clk.hw },
> +	{ .hw = &a1_rtc_clk.hw },
> +};
> +
>  static struct clk_regmap a1_pwm_a_sel = {
>  	.data = &(struct clk_regmap_mux_data){
>  		.offset = PWM_CLK_AB_CTRL,
> @@ -861,11 +877,8 @@ static struct clk_regmap a1_pwm_a_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "pwm_a_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		.parent_data = (const struct clk_parent_data []) {
> -			{ .fw_name = "xtal", },
> -			{ .hw = &a1_sys_clk.hw, },
> -		},
> -		.num_parents = 2,
> +		.parent_data = pwm_abcd_parents,
> +		.num_parents = ARRAY_SIZE(pwm_abcd_parents),
>  	},
>  };
>  
> @@ -918,11 +931,8 @@ static struct clk_regmap a1_pwm_b_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "pwm_b_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		.parent_data = (const struct clk_parent_data []) {
> -			{ .fw_name = "xtal", },
> -			{ .hw = &a1_sys_clk.hw, },
> -		},
> -		.num_parents = 2,
> +		.parent_data = pwm_abcd_parents,
> +		.num_parents = ARRAY_SIZE(pwm_abcd_parents),
>  	},
>  };
>  
> @@ -968,11 +978,8 @@ static struct clk_regmap a1_pwm_c_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "pwm_c_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		.parent_data = (const struct clk_parent_data []) {
> -			{ .fw_name = "xtal", },
> -			{ .hw = &a1_sys_clk.hw, },
> -		},
> -		.num_parents = 2,
> +		.parent_data = pwm_abcd_parents,
> +		.num_parents = ARRAY_SIZE(pwm_abcd_parents),
>  	},
>  };
>  
> @@ -1018,11 +1025,8 @@ static struct clk_regmap a1_pwm_d_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "pwm_d_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		.parent_data = (const struct clk_parent_data []) {
> -			{ .fw_name = "xtal", },
> -			{ .hw = &a1_sys_clk.hw, },
> -		},
> -		.num_parents = 2,
> +		.parent_data = pwm_abcd_parents,
> +		.num_parents = ARRAY_SIZE(pwm_abcd_parents),
>  	},
>  };
>  
> @@ -1059,7 +1063,7 @@ static struct clk_regmap a1_pwm_d = {
>  	},
>  };
>  
> -static const struct clk_parent_data pwm_ef_parent_data[] = {
> +static const struct clk_parent_data pwm_ef_parents[] = {
>  	{ .fw_name = "xtal", },
>  	{ .hw = &a1_sys_clk.hw },
>  	{ .fw_name = "fclk_div5", },
> @@ -1075,8 +1079,8 @@ static struct clk_regmap a1_pwm_e_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "pwm_e_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		.parent_data = pwm_ef_parent_data,
> -		.num_parents = ARRAY_SIZE(pwm_ef_parent_data),
> +		.parent_data = pwm_ef_parents,
> +		.num_parents = ARRAY_SIZE(pwm_ef_parents),
>  	},
>  };
>  
> @@ -1122,8 +1126,8 @@ static struct clk_regmap a1_pwm_f_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "pwm_f_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		.parent_data = pwm_ef_parent_data,
> -		.num_parents = ARRAY_SIZE(pwm_ef_parent_data),
> +		.parent_data = pwm_ef_parents,
> +		.num_parents = ARRAY_SIZE(pwm_ef_parents),
>  	},
>  };
>  
> @@ -1169,7 +1173,7 @@ static struct clk_regmap a1_pwm_f = {
>   *  --------------------|/
>   *                 24M
>   */
> -static const struct clk_parent_data spicc_parents[] = {
> +static const struct clk_parent_data spicc_spifc_parents[] = {
>  	{ .fw_name = "fclk_div2"},
>  	{ .fw_name = "fclk_div3"},
>  	{ .fw_name = "fclk_div5"},
> @@ -1185,8 +1189,8 @@ static struct clk_regmap a1_spicc_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "spicc_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		.parent_data = spicc_parents,
> -		.num_parents = 4,
> +		.parent_data = spicc_spifc_parents,
> +		.num_parents = ARRAY_SIZE(spicc_spifc_parents),
>  	},
>  };
>  
> @@ -1282,9 +1286,8 @@ static struct clk_regmap a1_spifc_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "spifc_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		/* the same parent with spicc */
> -		.parent_data = spicc_parents,
> -		.num_parents = 4,
> +		.parent_data = spicc_spifc_parents,
> +		.num_parents = ARRAY_SIZE(spicc_spifc_parents),
>  	},
>  };
>  
> @@ -1339,7 +1342,7 @@ static struct clk_regmap a1_spifc = {
>  	},
>  };
>  
> -static const struct clk_parent_data usb_bus_parent_data[] = {
> +static const struct clk_parent_data usb_bus_parents[] = {
>  	{ .fw_name = "xtal", },
>  	{ .hw = &a1_sys_clk.hw },
>  	{ .fw_name = "fclk_div3", },
> @@ -1355,8 +1358,8 @@ static struct clk_regmap a1_usb_bus_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "usb_bus_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		.parent_data = usb_bus_parent_data,
> -		.num_parents = ARRAY_SIZE(usb_bus_parent_data),
> +		.parent_data = usb_bus_parents,
> +		.num_parents = ARRAY_SIZE(usb_bus_parents),
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1394,7 +1397,7 @@ static struct clk_regmap a1_usb_bus = {
>  	},
>  };
>  
> -static const struct clk_parent_data sd_emmc_parents[] = {
> +static const struct clk_parent_data sd_emmc_psram_dmc_parents[] = {
>  	{ .fw_name = "fclk_div2", },
>  	{ .fw_name = "fclk_div3", },
>  	{ .fw_name = "fclk_div5", },
> @@ -1410,8 +1413,8 @@ static struct clk_regmap a1_sd_emmc_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "sd_emmc_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		.parent_data = sd_emmc_parents,
> -		.num_parents = 4,
> +		.parent_data = sd_emmc_psram_dmc_parents,
> +		.num_parents = ARRAY_SIZE(sd_emmc_psram_dmc_parents),
>  	},
>  };
>  
> @@ -1475,9 +1478,8 @@ static struct clk_regmap a1_psram_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "psram_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		/* the same parent with sd_emmc */
> -		.parent_data = sd_emmc_parents,
> -		.num_parents = 4,
> +		.parent_data = sd_emmc_psram_dmc_parents,
> +		.num_parents = ARRAY_SIZE(sd_emmc_psram_dmc_parents),
>  	},
>  };
>  
> @@ -1541,8 +1543,8 @@ static struct clk_regmap a1_dmc_sel = {
>  	.hw.init = &(struct clk_init_data){
>  		.name = "dmc_sel",
>  		.ops = &clk_regmap_mux_ops,
> -		.parent_data = sd_emmc_parents,
> -		.num_parents = 4,
> +		.parent_data = sd_emmc_psram_dmc_parents,
> +		.num_parents = ARRAY_SIZE(sd_emmc_psram_dmc_parents),
>  	},
>  };
>  
> @@ -1873,13 +1875,6 @@ static MESON_GATE(a1_prod_i2c,		AXI_CLK_EN,	12);
>  /* Array of all clocks provided by this provider */
>  static struct clk_hw_onecell_data a1_periphs_hw_onecell_data = {
>  	.hws = {
> -		[CLKID_SYS_B_SEL]		= &a1_sys_b_sel.hw,
> -		[CLKID_SYS_B_DIV]		= &a1_sys_b_div.hw,
> -		[CLKID_SYS_B]			= &a1_sys_b.hw,
> -		[CLKID_SYS_A_SEL]		= &a1_sys_a_sel.hw,
> -		[CLKID_SYS_A_DIV]		= &a1_sys_a_div.hw,
> -		[CLKID_SYS_A]			= &a1_sys_a.hw,
> -		[CLKID_SYS_CLK]			= &a1_sys_clk.hw,
>  		[CLKID_XTAL_CLKTREE]		= &a1_xtal_clktree.hw,
>  		[CLKID_XTAL_FIXPLL]		= &a1_xtal_fixpll.hw,
>  		[CLKID_XTAL_USB_PHY]		= &a1_xtal_usb_phy.hw,
> @@ -1887,6 +1882,7 @@ static struct clk_hw_onecell_data a1_periphs_hw_onecell_data = {
>  		[CLKID_XTAL_HIFIPLL]		= &a1_xtal_hifipll.hw,
>  		[CLKID_XTAL_SYSPLL]		= &a1_xtal_syspll.hw,
>  		[CLKID_XTAL_DDS]		= &a1_xtal_dds.hw,
> +		[CLKID_SYS_CLK]			= &a1_sys_clk.hw,
>  		[CLKID_CLKTREE]			= &a1_clk_tree.hw,
>  		[CLKID_RESET_CTRL]		= &a1_reset_ctrl.hw,
>  		[CLKID_ANALOG_CTRL]		= &a1_analog_ctrl.hw,
> @@ -1940,93 +1936,99 @@ static struct clk_hw_onecell_data a1_periphs_hw_onecell_data = {
>  		[CLKID_CPU_CTRL]		= &a1_cpu_ctrl.hw,
>  		[CLKID_ROM]			= &a1_rom.hw,
>  		[CLKID_PROC_I2C]		= &a1_prod_i2c.hw,
> +		[CLKID_DSPA_SEL]		= &a1_dspa_sel.hw,
> +		[CLKID_DSPB_SEL]		= &a1_dspb_sel.hw,
> +		[CLKID_DSPA_EN]			= &a1_dspa_en.hw,
> +		[CLKID_DSPA_EN_NIC]		= &a1_dspa_en_nic.hw,
> +		[CLKID_DSPB_EN]			= &a1_dspb_en.hw,
> +		[CLKID_DSPB_EN_NIC]		= &a1_dspb_en_nic.hw,
> +		[CLKID_RTC_CLK]			= &a1_rtc_clk.hw,
> +		[CLKID_CECA_32K]		= &a1_ceca_32k_clkout.hw,
> +		[CLKID_CECB_32K]		= &a1_cecb_32k_clkout.hw,
> +		[CLKID_24M]			= &a1_24m.hw,
> +		[CLKID_12M]			= &a1_12m.hw,
> +		[CLKID_FCLK_DIV2_DIVN]		= &a1_fclk_div2_divn.hw,
> +		[CLKID_GEN]			= &a1_gen.hw,
> +		[CLKID_SARADC_SEL]		= &a1_saradc_sel.hw,
> +		[CLKID_SARADC_CLK]		= &a1_saradc_clk.hw,
> +		[CLKID_PWM_A]			= &a1_pwm_a.hw,
> +		[CLKID_PWM_B]			= &a1_pwm_b.hw,
> +		[CLKID_PWM_C]			= &a1_pwm_c.hw,
> +		[CLKID_PWM_D]			= &a1_pwm_d.hw,
> +		[CLKID_PWM_E]			= &a1_pwm_e.hw,
> +		[CLKID_PWM_F]			= &a1_pwm_f.hw,
> +		[CLKID_SPICC]			= &a1_spicc.hw,
> +		[CLKID_TS]			= &a1_ts.hw,
> +		[CLKID_SPIFC]			= &a1_spifc.hw,
> +		[CLKID_USB_BUS]			= &a1_usb_bus.hw,
> +		[CLKID_SD_EMMC]			= &a1_sd_emmc.hw,
> +		[CLKID_PSRAM]			= &a1_psram.hw,
> +		[CLKID_DMC]			= &a1_dmc.hw,
> +		[CLKID_SYS_A_SEL]		= &a1_sys_a_sel.hw,
> +		[CLKID_SYS_A_DIV]		= &a1_sys_a_div.hw,
> +		[CLKID_SYS_A]			= &a1_sys_a.hw,
> +		[CLKID_SYS_B_SEL]		= &a1_sys_b_sel.hw,
> +		[CLKID_SYS_B_DIV]		= &a1_sys_b_div.hw,
> +		[CLKID_SYS_B]			= &a1_sys_b.hw,
>  		[CLKID_DSPA_A_SEL]		= &a1_dspa_a_sel.hw,
>  		[CLKID_DSPA_A_DIV]		= &a1_dspa_a_div.hw,
>  		[CLKID_DSPA_A]			= &a1_dspa_a.hw,
>  		[CLKID_DSPA_B_SEL]		= &a1_dspa_b_sel.hw,
>  		[CLKID_DSPA_B_DIV]		= &a1_dspa_b_div.hw,
>  		[CLKID_DSPA_B]			= &a1_dspa_b.hw,
> -		[CLKID_DSPA_SEL]		= &a1_dspa_sel.hw,
>  		[CLKID_DSPB_A_SEL]		= &a1_dspb_a_sel.hw,
>  		[CLKID_DSPB_A_DIV]		= &a1_dspb_a_div.hw,
>  		[CLKID_DSPB_A]			= &a1_dspb_a.hw,
>  		[CLKID_DSPB_B_SEL]		= &a1_dspb_b_sel.hw,
>  		[CLKID_DSPB_B_DIV]		= &a1_dspb_b_div.hw,
>  		[CLKID_DSPB_B]			= &a1_dspb_b.hw,
> -		[CLKID_DSPB_SEL]		= &a1_dspb_sel.hw,
> -		[CLKID_DSPA_EN]			= &a1_dspa_en.hw,
> -		[CLKID_DSPA_EN_NIC]		= &a1_dspa_en_nic.hw,
> -		[CLKID_DSPB_EN]			= &a1_dspb_en.hw,
> -		[CLKID_DSPB_EN_NIC]		= &a1_dspb_en_nic.hw,
> -		[CLKID_24M]			= &a1_24m.hw,
> -		[CLKID_24M_DIV2]		= &a1_24m_div2.hw,
> -		[CLKID_12M]			= &a1_12m.hw,
> +		[CLKID_RTC_32K_CLKIN]		= &a1_rtc_32k_clkin.hw,
> +		[CLKID_RTC_32K_DIV]		= &a1_rtc_32k_div.hw,
> +		[CLKID_RTC_32K_XTAL]		= &a1_rtc_32k_xtal.hw,
> +		[CLKID_RTC_32K_SEL]		= &a1_rtc_32k_sel.hw,
> +		[CLKID_CECB_32K_CLKIN]		= &a1_cecb_32k_clkin.hw,
> +		[CLKID_CECB_32K_DIV]		= &a1_cecb_32k_div.hw,
> +		[CLKID_CECB_32K_SEL_PRE]	= &a1_cecb_32k_sel_pre.hw,
> +		[CLKID_CECB_32K_SEL]		= &a1_cecb_32k_sel.hw,
> +		[CLKID_CECA_32K_CLKIN]		= &a1_ceca_32k_clkin.hw,
> +		[CLKID_CECA_32K_DIV]		= &a1_ceca_32k_div.hw,
> +		[CLKID_CECA_32K_SEL_PRE]	= &a1_ceca_32k_sel_pre.hw,
> +		[CLKID_CECA_32K_SEL]		= &a1_ceca_32k_sel.hw,
>  		[CLKID_DIV2_PRE]		= &a1_fclk_div2_divn_pre.hw,
> -		[CLKID_FCLK_DIV2_DIVN]		= &a1_fclk_div2_divn.hw,
> +		[CLKID_24M_DIV2]		= &a1_24m_div2.hw,
>  		[CLKID_GEN_SEL]			= &a1_gen_sel.hw,
>  		[CLKID_GEN_DIV]			= &a1_gen_div.hw,
> -		[CLKID_GEN]			= &a1_gen.hw,
> -		[CLKID_SARADC_SEL]		= &a1_saradc_sel.hw,
>  		[CLKID_SARADC_DIV]		= &a1_saradc_div.hw,
> -		[CLKID_SARADC_CLK]		= &a1_saradc_clk.hw,
>  		[CLKID_PWM_A_SEL]		= &a1_pwm_a_sel.hw,
>  		[CLKID_PWM_A_DIV]		= &a1_pwm_a_div.hw,
> -		[CLKID_PWM_A]			= &a1_pwm_a.hw,
>  		[CLKID_PWM_B_SEL]		= &a1_pwm_b_sel.hw,
>  		[CLKID_PWM_B_DIV]		= &a1_pwm_b_div.hw,
> -		[CLKID_PWM_B]			= &a1_pwm_b.hw,
>  		[CLKID_PWM_C_SEL]		= &a1_pwm_c_sel.hw,
>  		[CLKID_PWM_C_DIV]		= &a1_pwm_c_div.hw,
> -		[CLKID_PWM_C]			= &a1_pwm_c.hw,
>  		[CLKID_PWM_D_SEL]		= &a1_pwm_d_sel.hw,
>  		[CLKID_PWM_D_DIV]		= &a1_pwm_d_div.hw,
> -		[CLKID_PWM_D]			= &a1_pwm_d.hw,
>  		[CLKID_PWM_E_SEL]		= &a1_pwm_e_sel.hw,
>  		[CLKID_PWM_E_DIV]		= &a1_pwm_e_div.hw,
> -		[CLKID_PWM_E]			= &a1_pwm_e.hw,
>  		[CLKID_PWM_F_SEL]		= &a1_pwm_f_sel.hw,
>  		[CLKID_PWM_F_DIV]		= &a1_pwm_f_div.hw,
> -		[CLKID_PWM_F]			= &a1_pwm_f.hw,
>  		[CLKID_SPICC_SEL]		= &a1_spicc_sel.hw,
>  		[CLKID_SPICC_DIV]		= &a1_spicc_div.hw,
>  		[CLKID_SPICC_SEL2]		= &a1_spicc_sel2.hw,
> -		[CLKID_SPICC]			= &a1_spicc.hw,
>  		[CLKID_TS_DIV]			= &a1_ts_div.hw,
> -		[CLKID_TS]			= &a1_ts.hw,
>  		[CLKID_SPIFC_SEL]		= &a1_spifc_sel.hw,
>  		[CLKID_SPIFC_DIV]		= &a1_spifc_div.hw,
>  		[CLKID_SPIFC_SEL2]		= &a1_spifc_sel2.hw,
> -		[CLKID_SPIFC]			= &a1_spifc.hw,
>  		[CLKID_USB_BUS_SEL]		= &a1_usb_bus_sel.hw,
>  		[CLKID_USB_BUS_DIV]		= &a1_usb_bus_div.hw,
> -		[CLKID_USB_BUS]			= &a1_usb_bus.hw,
>  		[CLKID_SD_EMMC_SEL]		= &a1_sd_emmc_sel.hw,
>  		[CLKID_SD_EMMC_DIV]		= &a1_sd_emmc_div.hw,
>  		[CLKID_SD_EMMC_SEL2]		= &a1_sd_emmc_sel2.hw,
> -		[CLKID_SD_EMMC]			= &a1_sd_emmc.hw,
>  		[CLKID_PSRAM_SEL]		= &a1_psram_sel.hw,
>  		[CLKID_PSRAM_DIV]		= &a1_psram_div.hw,
>  		[CLKID_PSRAM_SEL2]		= &a1_psram_sel2.hw,
> -		[CLKID_PSRAM]			= &a1_psram.hw,
>  		[CLKID_DMC_SEL]			= &a1_dmc_sel.hw,
>  		[CLKID_DMC_DIV]			= &a1_dmc_div.hw,
>  		[CLKID_DMC_SEL2]		= &a1_dmc_sel2.hw,
> -		[CLKID_DMC]			= &a1_dmc.hw,
> -		[CLKID_RTC_32K_CLKIN]		= &a1_rtc_32k_clkin.hw,
> -		[CLKID_RTC_32K_DIV]		= &a1_rtc_32k_div.hw,
> -		[CLKID_RTC_32K_XTAL]		= &a1_rtc_32k_xtal.hw,
> -		[CLKID_RTC_32K_SEL]		= &a1_rtc_32k_sel.hw,
> -		[CLKID_RTC_CLK]			= &a1_rtc_clk.hw,
> -		[CLKID_CECA_32K_CLKIN]		= &a1_ceca_32k_clkin.hw,
> -		[CLKID_CECA_32K_DIV]		= &a1_ceca_32k_div.hw,
> -		[CLKID_CECA_32K_SEL_PRE]	= &a1_ceca_32k_sel_pre.hw,
> -		[CLKID_CECA_32K_SEL]		= &a1_ceca_32k_sel.hw,
> -		[CLKID_CECA_32K]		= &a1_ceca_32k_clkout.hw,
> -		[CLKID_CECB_32K_CLKIN]		= &a1_cecb_32k_clkin.hw,
> -		[CLKID_CECB_32K_DIV]		= &a1_cecb_32k_div.hw,
> -		[CLKID_CECB_32K_SEL_PRE]	= &a1_cecb_32k_sel_pre.hw,
> -		[CLKID_CECB_32K_SEL]		= &a1_cecb_32k_sel.hw,
> -		[CLKID_CECB_32K]		= &a1_cecb_32k_clkout.hw,
>  		[NR_CLKS]			= NULL,
>  	},

Please avoid this ordering change - It is borderline impossible to
review.

Keep the ID Order


>  	.num = NR_CLKS,
> @@ -2041,10 +2043,12 @@ static struct clk_regmap *const a1_periphs_regmaps[] = {
>  	&a1_xtal_hifipll,
>  	&a1_xtal_syspll,
>  	&a1_xtal_dds,
> +	&a1_sys_clk,
>  	&a1_clk_tree,
>  	&a1_reset_ctrl,
>  	&a1_analog_ctrl,
>  	&a1_pwr_ctrl,
> +	&a1_pad_ctrl,
>  	&a1_sys_ctrl,
>  	&a1_temp_sensor,
>  	&a1_am2axi_dev,
> @@ -2093,157 +2097,126 @@ static struct clk_regmap *const a1_periphs_regmaps[] = {
>  	&a1_cpu_ctrl,
>  	&a1_rom,
>  	&a1_prod_i2c,
> +	&a1_dspa_sel,
> +	&a1_dspb_sel,
> +	&a1_dspa_en,
> +	&a1_dspa_en_nic,
> +	&a1_dspb_en,
> +	&a1_dspb_en_nic,
> +	&a1_rtc_clk,
> +	&a1_ceca_32k_clkout,
> +	&a1_cecb_32k_clkout,
> +	&a1_24m,
> +	&a1_12m,
> +	&a1_fclk_div2_divn,
> +	&a1_gen,
> +	&a1_saradc_sel,
> +	&a1_saradc_clk,
> +	&a1_pwm_a,
> +	&a1_pwm_b,
> +	&a1_pwm_c,
> +	&a1_pwm_d,
> +	&a1_pwm_e,
> +	&a1_pwm_f,
> +	&a1_spicc,
> +	&a1_ts,
> +	&a1_spifc,
> +	&a1_usb_bus,
> +	&a1_sd_emmc,
> +	&a1_psram,
> +	&a1_dmc,
> +	&a1_sys_a_sel,
> +	&a1_sys_a_div,
> +	&a1_sys_a,
> +	&a1_sys_b_sel,
> +	&a1_sys_b_div,
> +	&a1_sys_b,
>  	&a1_dspa_a_sel,
>  	&a1_dspa_a_div,
>  	&a1_dspa_a,
>  	&a1_dspa_b_sel,
>  	&a1_dspa_b_div,
>  	&a1_dspa_b,
> -	&a1_dspa_sel,
>  	&a1_dspb_a_sel,
>  	&a1_dspb_a_div,
>  	&a1_dspb_a,
>  	&a1_dspb_b_sel,
>  	&a1_dspb_b_div,
>  	&a1_dspb_b,
> -	&a1_dspb_sel,
> -	&a1_dspa_en,
> -	&a1_dspa_en_nic,
> -	&a1_dspb_en,
> -	&a1_dspb_en_nic,
> -	&a1_24m,
> -	&a1_12m,
> +	&a1_rtc_32k_clkin,
> +	&a1_rtc_32k_div,
> +	&a1_rtc_32k_xtal,
> +	&a1_rtc_32k_sel,
> +	&a1_cecb_32k_clkin,
> +	&a1_cecb_32k_div,
> +	&a1_cecb_32k_sel_pre,
> +	&a1_cecb_32k_sel,
> +	&a1_ceca_32k_clkin,
> +	&a1_ceca_32k_div,
> +	&a1_ceca_32k_sel_pre,
> +	&a1_ceca_32k_sel,
>  	&a1_fclk_div2_divn_pre,
> -	&a1_fclk_div2_divn,
>  	&a1_gen_sel,
>  	&a1_gen_div,
> -	&a1_gen,
> -	&a1_saradc_sel,
>  	&a1_saradc_div,
> -	&a1_saradc_clk,
>  	&a1_pwm_a_sel,
>  	&a1_pwm_a_div,
> -	&a1_pwm_a,
>  	&a1_pwm_b_sel,
>  	&a1_pwm_b_div,
> -	&a1_pwm_b,
>  	&a1_pwm_c_sel,
>  	&a1_pwm_c_div,
> -	&a1_pwm_c,
>  	&a1_pwm_d_sel,
>  	&a1_pwm_d_div,
> -	&a1_pwm_d,
>  	&a1_pwm_e_sel,
>  	&a1_pwm_e_div,
> -	&a1_pwm_e,
>  	&a1_pwm_f_sel,
>  	&a1_pwm_f_div,
> -	&a1_pwm_f,
>  	&a1_spicc_sel,
>  	&a1_spicc_div,
>  	&a1_spicc_sel2,
> -	&a1_spicc,
>  	&a1_ts_div,
> -	&a1_ts,
>  	&a1_spifc_sel,
>  	&a1_spifc_div,
>  	&a1_spifc_sel2,
> -	&a1_spifc,
>  	&a1_usb_bus_sel,
>  	&a1_usb_bus_div,
> -	&a1_usb_bus,
>  	&a1_sd_emmc_sel,
>  	&a1_sd_emmc_div,
>  	&a1_sd_emmc_sel2,
> -	&a1_sd_emmc,
>  	&a1_psram_sel,
>  	&a1_psram_div,
>  	&a1_psram_sel2,
> -	&a1_psram,
>  	&a1_dmc_sel,
>  	&a1_dmc_div,
>  	&a1_dmc_sel2,
> -	&a1_dmc,
> -	&a1_sys_b_sel,
> -	&a1_sys_b_div,
> -	&a1_sys_b,
> -	&a1_sys_a_sel,
> -	&a1_sys_a_div,
> -	&a1_sys_a,
> -	&a1_sys_clk,
> -	&a1_rtc_32k_clkin,
> -	&a1_rtc_32k_div,
> -	&a1_rtc_32k_xtal,
> -	&a1_rtc_32k_sel,
> -	&a1_rtc_clk,
> -	&a1_ceca_32k_clkin,
> -	&a1_ceca_32k_div,
> -	&a1_ceca_32k_sel_pre,
> -	&a1_ceca_32k_sel,
> -	&a1_ceca_32k_clkout,
> -	&a1_cecb_32k_clkin,
> -	&a1_cecb_32k_div,
> -	&a1_cecb_32k_sel_pre,
> -	&a1_cecb_32k_sel,
> -	&a1_cecb_32k_clkout,
>  };
>  
> -static struct regmap_config clkc_regmap_config = {
> -	.reg_bits       = 32,
> -	.val_bits       = 32,
> -	.reg_stride     = 4,
> +static const struct meson_a1_clkc_data a1_periphs_clkc __maybe_unused = {
> +	.hw = &a1_periphs_hw_onecell_data,
> +	.regs = a1_periphs_regmaps,
> +	.num_regs = ARRAY_SIZE(a1_periphs_regmaps),
>  };
>  
> -static int meson_a1_periphs_probe(struct platform_device *pdev)
> -{
> -	struct device *dev = &pdev->dev;
> -	struct resource *res;
> -	void __iomem *base;
> -	struct regmap *map;
> -	int ret, i;
> -
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -
> -	base = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(base))
> -		return PTR_ERR(base);
> -
> -	map = devm_regmap_init_mmio(dev, base, &clkc_regmap_config);
> -	if (IS_ERR(map))
> -		return PTR_ERR(map);
> -
> -	/* Populate regmap for the regmap backed clocks */
> -	for (i = 0; i < ARRAY_SIZE(a1_periphs_regmaps); i++)
> -		a1_periphs_regmaps[i]->map = map;
> -
> -	for (i = 0; i < a1_periphs_hw_onecell_data.num; i++) {
> -		/* array might be sparse */
> -		if (!a1_periphs_hw_onecell_data.hws[i])
> -			continue;
> -
> -		ret = devm_clk_hw_register(dev,
> -					   a1_periphs_hw_onecell_data.hws[i]);
> -		if (ret) {
> -			dev_err(dev, "Clock registration failed\n");
> -			return ret;
> -		}
> -	}
> -
> -	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> -					   &a1_periphs_hw_onecell_data);
> -}
> -
> -static const struct of_device_id clkc_match_table[] = {
> -	{ .compatible = "amlogic,a1-periphs-clkc", },
> -	{}
> +#ifdef CONFIG_OF
> +static const struct of_device_id a1_periphs_clkc_match_table[] = {
> +	{
> +		.compatible = "amlogic,a1-periphs-clkc",
> +		.data = &a1_periphs_clkc,
> +	},
> +	{},
>  };
> +MODULE_DEVICE_TABLE(of, a1_periphs_clkc_match_table);
> +#endif /* CONFIG_OF */
>  
> -static struct platform_driver a1_periphs_driver = {
> -	.probe		= meson_a1_periphs_probe,
> -	.driver		= {
> -		.name	= "a1-periphs-clkc",
> -		.of_match_table = clkc_match_table,
> +static struct platform_driver a1_periphs_clkc_driver = {
> +	.probe = meson_a1_clkc_probe,
> +	.driver = {
> +		.name = "a1-periphs-clkc",
> +		.of_match_table = of_match_ptr(a1_periphs_clkc_match_table),
>  	},
>  };
>  
> -builtin_platform_driver(a1_periphs_driver);
> +module_platform_driver(a1_periphs_clkc_driver);
> +MODULE_AUTHOR("Jian Hu <jian.hu@amlogic.com>");
> +MODULE_AUTHOR("Dmitry Rokosov <ddrokosov@sberdevices.ru>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/clk/meson/a1.h b/drivers/clk/meson/a1.h
> index 1ae5e04848d6..94b155e33568 100644
> --- a/drivers/clk/meson/a1.h
> +++ b/drivers/clk/meson/a1.h
> @@ -1,6 +1,12 @@
>  /* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>  /*
> + * Amlogic Meson-A1 Peripheral Clock Controller internals
> + *
>   * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + * Author: Jian Hu <jian.hu@amlogic.com>
> + *
> + * Copyright (c) 2022, SberDevices. All Rights Reserved.
> + * Author: Dmitry Rokosov <ddrokosov@sberdevices.ru>
>   */
>  
>  #ifndef __A1_H
> @@ -12,7 +18,6 @@
>  #define RTC_BY_OSCIN_CTRL1		0x8
>  #define RTC_CTRL			0xc
>  #define SYS_CLK_CTRL0			0x10
> -#define AXI_CLK_CTRL0			0x14
>  #define SYS_CLK_EN0			0x1c
>  #define SYS_CLK_EN1			0x20
>  #define AXI_CLK_EN			0x24
> @@ -22,13 +27,6 @@
>  #define DSPB_CLK_CTRL0			0x34
>  #define CLK12_24_CTRL			0x38
>  #define GEN_CLK_CTRL			0x3c
> -#define TIMESTAMP_CTRL0			0x40
> -#define TIMESTAMP_CTRL1			0x44
> -#define TIMESTAMP_CTRL2			0x48
> -#define TIMESTAMP_VAL0			0x4c
> -#define TIMESTAMP_VAL1			0x50
> -#define TIMEBASE_CTRL0			0x54
> -#define TIMEBASE_CTRL1			0x58
>  #define SAR_ADC_CLK_CTRL		0xc0
>  #define PWM_CLK_AB_CTRL			0xc4
>  #define PWM_CLK_CD_CTRL			0xc8
> @@ -44,8 +42,6 @@
>  #define CECB_CLK_CTRL1			0xf0
>  #define PSRAM_CLK_CTRL			0xf4
>  #define DMC_CLK_CTRL			0xf8
> -#define FCLK_DIV1_SEL			0xfc
> -#define TST_CTRL			0x100
>  
>  #define CLKID_XTAL_CLKTREE		0
>  #define CLKID_SYS_A_SEL			89
Dmitry Rokosov Dec. 2, 2022, 12:10 p.m. UTC | #13
...

> > @@ -1873,13 +1875,6 @@ static MESON_GATE(a1_prod_i2c,		AXI_CLK_EN,	12);
> >  /* Array of all clocks provided by this provider */
> >  static struct clk_hw_onecell_data a1_periphs_hw_onecell_data = {
> >  	.hws = {
> > -		[CLKID_SYS_B_SEL]		= &a1_sys_b_sel.hw,
> > -		[CLKID_SYS_B_DIV]		= &a1_sys_b_div.hw,
> > -		[CLKID_SYS_B]			= &a1_sys_b.hw,
> > -		[CLKID_SYS_A_SEL]		= &a1_sys_a_sel.hw,
> > -		[CLKID_SYS_A_DIV]		= &a1_sys_a_div.hw,
> > -		[CLKID_SYS_A]			= &a1_sys_a.hw,
> > -		[CLKID_SYS_CLK]			= &a1_sys_clk.hw,
> >  		[CLKID_XTAL_CLKTREE]		= &a1_xtal_clktree.hw,
> >  		[CLKID_XTAL_FIXPLL]		= &a1_xtal_fixpll.hw,
> >  		[CLKID_XTAL_USB_PHY]		= &a1_xtal_usb_phy.hw,
> > @@ -1887,6 +1882,7 @@ static struct clk_hw_onecell_data a1_periphs_hw_onecell_data = {
> >  		[CLKID_XTAL_HIFIPLL]		= &a1_xtal_hifipll.hw,
> >  		[CLKID_XTAL_SYSPLL]		= &a1_xtal_syspll.hw,
> >  		[CLKID_XTAL_DDS]		= &a1_xtal_dds.hw,
> > +		[CLKID_SYS_CLK]			= &a1_sys_clk.hw,
> >  		[CLKID_CLKTREE]			= &a1_clk_tree.hw,
> >  		[CLKID_RESET_CTRL]		= &a1_reset_ctrl.hw,
> >  		[CLKID_ANALOG_CTRL]		= &a1_analog_ctrl.hw,
> > @@ -1940,93 +1936,99 @@ static struct clk_hw_onecell_data a1_periphs_hw_onecell_data = {
> >  		[CLKID_CPU_CTRL]		= &a1_cpu_ctrl.hw,
> >  		[CLKID_ROM]			= &a1_rom.hw,
> >  		[CLKID_PROC_I2C]		= &a1_prod_i2c.hw,
> > +		[CLKID_DSPA_SEL]		= &a1_dspa_sel.hw,
> > +		[CLKID_DSPB_SEL]		= &a1_dspb_sel.hw,
> > +		[CLKID_DSPA_EN]			= &a1_dspa_en.hw,
> > +		[CLKID_DSPA_EN_NIC]		= &a1_dspa_en_nic.hw,
> > +		[CLKID_DSPB_EN]			= &a1_dspb_en.hw,
> > +		[CLKID_DSPB_EN_NIC]		= &a1_dspb_en_nic.hw,
> > +		[CLKID_RTC_CLK]			= &a1_rtc_clk.hw,
> > +		[CLKID_CECA_32K]		= &a1_ceca_32k_clkout.hw,
> > +		[CLKID_CECB_32K]		= &a1_cecb_32k_clkout.hw,
> > +		[CLKID_24M]			= &a1_24m.hw,
> > +		[CLKID_12M]			= &a1_12m.hw,
> > +		[CLKID_FCLK_DIV2_DIVN]		= &a1_fclk_div2_divn.hw,
> > +		[CLKID_GEN]			= &a1_gen.hw,
> > +		[CLKID_SARADC_SEL]		= &a1_saradc_sel.hw,
> > +		[CLKID_SARADC_CLK]		= &a1_saradc_clk.hw,
> > +		[CLKID_PWM_A]			= &a1_pwm_a.hw,
> > +		[CLKID_PWM_B]			= &a1_pwm_b.hw,
> > +		[CLKID_PWM_C]			= &a1_pwm_c.hw,
> > +		[CLKID_PWM_D]			= &a1_pwm_d.hw,
> > +		[CLKID_PWM_E]			= &a1_pwm_e.hw,
> > +		[CLKID_PWM_F]			= &a1_pwm_f.hw,
> > +		[CLKID_SPICC]			= &a1_spicc.hw,
> > +		[CLKID_TS]			= &a1_ts.hw,
> > +		[CLKID_SPIFC]			= &a1_spifc.hw,
> > +		[CLKID_USB_BUS]			= &a1_usb_bus.hw,
> > +		[CLKID_SD_EMMC]			= &a1_sd_emmc.hw,
> > +		[CLKID_PSRAM]			= &a1_psram.hw,
> > +		[CLKID_DMC]			= &a1_dmc.hw,
> > +		[CLKID_SYS_A_SEL]		= &a1_sys_a_sel.hw,
> > +		[CLKID_SYS_A_DIV]		= &a1_sys_a_div.hw,
> > +		[CLKID_SYS_A]			= &a1_sys_a.hw,
> > +		[CLKID_SYS_B_SEL]		= &a1_sys_b_sel.hw,
> > +		[CLKID_SYS_B_DIV]		= &a1_sys_b_div.hw,
> > +		[CLKID_SYS_B]			= &a1_sys_b.hw,
> >  		[CLKID_DSPA_A_SEL]		= &a1_dspa_a_sel.hw,
> >  		[CLKID_DSPA_A_DIV]		= &a1_dspa_a_div.hw,
> >  		[CLKID_DSPA_A]			= &a1_dspa_a.hw,
> >  		[CLKID_DSPA_B_SEL]		= &a1_dspa_b_sel.hw,
> >  		[CLKID_DSPA_B_DIV]		= &a1_dspa_b_div.hw,
> >  		[CLKID_DSPA_B]			= &a1_dspa_b.hw,
> > -		[CLKID_DSPA_SEL]		= &a1_dspa_sel.hw,
> >  		[CLKID_DSPB_A_SEL]		= &a1_dspb_a_sel.hw,
> >  		[CLKID_DSPB_A_DIV]		= &a1_dspb_a_div.hw,
> >  		[CLKID_DSPB_A]			= &a1_dspb_a.hw,
> >  		[CLKID_DSPB_B_SEL]		= &a1_dspb_b_sel.hw,
> >  		[CLKID_DSPB_B_DIV]		= &a1_dspb_b_div.hw,
> >  		[CLKID_DSPB_B]			= &a1_dspb_b.hw,
> > -		[CLKID_DSPB_SEL]		= &a1_dspb_sel.hw,
> > -		[CLKID_DSPA_EN]			= &a1_dspa_en.hw,
> > -		[CLKID_DSPA_EN_NIC]		= &a1_dspa_en_nic.hw,
> > -		[CLKID_DSPB_EN]			= &a1_dspb_en.hw,
> > -		[CLKID_DSPB_EN_NIC]		= &a1_dspb_en_nic.hw,
> > -		[CLKID_24M]			= &a1_24m.hw,
> > -		[CLKID_24M_DIV2]		= &a1_24m_div2.hw,
> > -		[CLKID_12M]			= &a1_12m.hw,
> > +		[CLKID_RTC_32K_CLKIN]		= &a1_rtc_32k_clkin.hw,
> > +		[CLKID_RTC_32K_DIV]		= &a1_rtc_32k_div.hw,
> > +		[CLKID_RTC_32K_XTAL]		= &a1_rtc_32k_xtal.hw,
> > +		[CLKID_RTC_32K_SEL]		= &a1_rtc_32k_sel.hw,
> > +		[CLKID_CECB_32K_CLKIN]		= &a1_cecb_32k_clkin.hw,
> > +		[CLKID_CECB_32K_DIV]		= &a1_cecb_32k_div.hw,
> > +		[CLKID_CECB_32K_SEL_PRE]	= &a1_cecb_32k_sel_pre.hw,
> > +		[CLKID_CECB_32K_SEL]		= &a1_cecb_32k_sel.hw,
> > +		[CLKID_CECA_32K_CLKIN]		= &a1_ceca_32k_clkin.hw,
> > +		[CLKID_CECA_32K_DIV]		= &a1_ceca_32k_div.hw,
> > +		[CLKID_CECA_32K_SEL_PRE]	= &a1_ceca_32k_sel_pre.hw,
> > +		[CLKID_CECA_32K_SEL]		= &a1_ceca_32k_sel.hw,
> >  		[CLKID_DIV2_PRE]		= &a1_fclk_div2_divn_pre.hw,
> > -		[CLKID_FCLK_DIV2_DIVN]		= &a1_fclk_div2_divn.hw,
> > +		[CLKID_24M_DIV2]		= &a1_24m_div2.hw,
> >  		[CLKID_GEN_SEL]			= &a1_gen_sel.hw,
> >  		[CLKID_GEN_DIV]			= &a1_gen_div.hw,
> > -		[CLKID_GEN]			= &a1_gen.hw,
> > -		[CLKID_SARADC_SEL]		= &a1_saradc_sel.hw,
> >  		[CLKID_SARADC_DIV]		= &a1_saradc_div.hw,
> > -		[CLKID_SARADC_CLK]		= &a1_saradc_clk.hw,
> >  		[CLKID_PWM_A_SEL]		= &a1_pwm_a_sel.hw,
> >  		[CLKID_PWM_A_DIV]		= &a1_pwm_a_div.hw,
> > -		[CLKID_PWM_A]			= &a1_pwm_a.hw,
> >  		[CLKID_PWM_B_SEL]		= &a1_pwm_b_sel.hw,
> >  		[CLKID_PWM_B_DIV]		= &a1_pwm_b_div.hw,
> > -		[CLKID_PWM_B]			= &a1_pwm_b.hw,
> >  		[CLKID_PWM_C_SEL]		= &a1_pwm_c_sel.hw,
> >  		[CLKID_PWM_C_DIV]		= &a1_pwm_c_div.hw,
> > -		[CLKID_PWM_C]			= &a1_pwm_c.hw,
> >  		[CLKID_PWM_D_SEL]		= &a1_pwm_d_sel.hw,
> >  		[CLKID_PWM_D_DIV]		= &a1_pwm_d_div.hw,
> > -		[CLKID_PWM_D]			= &a1_pwm_d.hw,
> >  		[CLKID_PWM_E_SEL]		= &a1_pwm_e_sel.hw,
> >  		[CLKID_PWM_E_DIV]		= &a1_pwm_e_div.hw,
> > -		[CLKID_PWM_E]			= &a1_pwm_e.hw,
> >  		[CLKID_PWM_F_SEL]		= &a1_pwm_f_sel.hw,
> >  		[CLKID_PWM_F_DIV]		= &a1_pwm_f_div.hw,
> > -		[CLKID_PWM_F]			= &a1_pwm_f.hw,
> >  		[CLKID_SPICC_SEL]		= &a1_spicc_sel.hw,
> >  		[CLKID_SPICC_DIV]		= &a1_spicc_div.hw,
> >  		[CLKID_SPICC_SEL2]		= &a1_spicc_sel2.hw,
> > -		[CLKID_SPICC]			= &a1_spicc.hw,
> >  		[CLKID_TS_DIV]			= &a1_ts_div.hw,
> > -		[CLKID_TS]			= &a1_ts.hw,
> >  		[CLKID_SPIFC_SEL]		= &a1_spifc_sel.hw,
> >  		[CLKID_SPIFC_DIV]		= &a1_spifc_div.hw,
> >  		[CLKID_SPIFC_SEL2]		= &a1_spifc_sel2.hw,
> > -		[CLKID_SPIFC]			= &a1_spifc.hw,
> >  		[CLKID_USB_BUS_SEL]		= &a1_usb_bus_sel.hw,
> >  		[CLKID_USB_BUS_DIV]		= &a1_usb_bus_div.hw,
> > -		[CLKID_USB_BUS]			= &a1_usb_bus.hw,
> >  		[CLKID_SD_EMMC_SEL]		= &a1_sd_emmc_sel.hw,
> >  		[CLKID_SD_EMMC_DIV]		= &a1_sd_emmc_div.hw,
> >  		[CLKID_SD_EMMC_SEL2]		= &a1_sd_emmc_sel2.hw,
> > -		[CLKID_SD_EMMC]			= &a1_sd_emmc.hw,
> >  		[CLKID_PSRAM_SEL]		= &a1_psram_sel.hw,
> >  		[CLKID_PSRAM_DIV]		= &a1_psram_div.hw,
> >  		[CLKID_PSRAM_SEL2]		= &a1_psram_sel2.hw,
> > -		[CLKID_PSRAM]			= &a1_psram.hw,
> >  		[CLKID_DMC_SEL]			= &a1_dmc_sel.hw,
> >  		[CLKID_DMC_DIV]			= &a1_dmc_div.hw,
> >  		[CLKID_DMC_SEL2]		= &a1_dmc_sel2.hw,
> > -		[CLKID_DMC]			= &a1_dmc.hw,
> > -		[CLKID_RTC_32K_CLKIN]		= &a1_rtc_32k_clkin.hw,
> > -		[CLKID_RTC_32K_DIV]		= &a1_rtc_32k_div.hw,
> > -		[CLKID_RTC_32K_XTAL]		= &a1_rtc_32k_xtal.hw,
> > -		[CLKID_RTC_32K_SEL]		= &a1_rtc_32k_sel.hw,
> > -		[CLKID_RTC_CLK]			= &a1_rtc_clk.hw,
> > -		[CLKID_CECA_32K_CLKIN]		= &a1_ceca_32k_clkin.hw,
> > -		[CLKID_CECA_32K_DIV]		= &a1_ceca_32k_div.hw,
> > -		[CLKID_CECA_32K_SEL_PRE]	= &a1_ceca_32k_sel_pre.hw,
> > -		[CLKID_CECA_32K_SEL]		= &a1_ceca_32k_sel.hw,
> > -		[CLKID_CECA_32K]		= &a1_ceca_32k_clkout.hw,
> > -		[CLKID_CECB_32K_CLKIN]		= &a1_cecb_32k_clkin.hw,
> > -		[CLKID_CECB_32K_DIV]		= &a1_cecb_32k_div.hw,
> > -		[CLKID_CECB_32K_SEL_PRE]	= &a1_cecb_32k_sel_pre.hw,
> > -		[CLKID_CECB_32K_SEL]		= &a1_cecb_32k_sel.hw,
> > -		[CLKID_CECB_32K]		= &a1_cecb_32k_clkout.hw,
> >  		[NR_CLKS]			= NULL,
> >  	},
> 
> Please avoid this ordering change - It is borderline impossible to
> review.
> 
> Keep the ID Order

Yes, it's what I'm trying to achieve here - keeping the ID order. Jian
Hu's version mixed up CLKDID definitions. This patch resolves such
problem. Anyway, next version will not have such diff, because patches
will be squashed.

...