mbox series

[v1,00/16] Basic clock and reset support for StarFive JH8100 RISC-V SoC

Message ID 20231206115000.295825-1-jeeheng.sia@starfivetech.com
Headers show
Series Basic clock and reset support for StarFive JH8100 RISC-V SoC | expand

Message

JeeHeng Sia Dec. 6, 2023, 11:49 a.m. UTC
This patch series enabled basic clock & reset support for StarFive
JH8100 SoC.

This patch series depends on the Initial device tree support for
StarFive JH8100 SoC patch series which can be found at below link:
https://lore.kernel.org/lkml/20231201121410.95298-1-jeeheng.sia@starfivetech.com/

StarFive JH8100 shares a similar clock and reset design with JH7110.
To facilitate the reuse of the file and its functionalities, files
containing the 'jh71x0' naming convention are renamed to use the
'common' wording. Internal functions that contain the 'jh71x0'
naming convention are renamed to use 'starfive.' This is accomplished
through patches 1, 2, 3, and 4.


Patch 5 adds documentation to describe System (SYSCRG) Clock & Reset
binding.
Patch 6 adds SYSCRG clock driver.

patch 7 adds documentation to describe System-North-West (SYSCRG-NW)
Clock & Reset binding.
Patch 8 adds SYSCRG-NW clock driver.

patch 9 adds documentation to describe System-North-East (SYSCRG-NE)
Clock & Reset binding.
Patch 10 adds SYSCRG-NE clock driver.

patch 11 adds documentation to describe System-South-West (SYSCRG-SW)
Clock & Reset binding.
Patch 12 adds SYSCRG-SW clock driver.

patch 13 adds documentation to describe Always-On (AON)
Clock & Reset binding.
Patch 14 adds AON clock driver.

Patch 15 adds support for the auxiliary reset driver.

Patch 16 adds clocks and reset nodes to the JH8100 device tree.

Sia Jee Heng (16):
  reset: starfive: Rename file name "jh71x0" to "common"
  reset: starfive: Convert the word "jh71x0" to "starfive"
  clk: starfive: Rename file name "jh71x0" to "common"
  clk: starfive: Convert the word "jh71x0" to "starfive"
  dt-bindings: clock: Add StarFive JH8100 System clock and reset
    generator
  clk: starfive: Add JH8100 System clock generator driver
  dt-bindings: clock: Add StarFive JH8100 System-North-West clock and
    reset generator
  clk: starfive: Add JH8100 System-North-West clock generator driver
  dt-bindings: clock: Add StarFive JH8100 System-North-East clock and
    reset generator
  clk: starfive: Add JH8100 System-North-East clock generator driver
  dt-bindings: clock: Add StarFive JH8100 System-South-West clock and
    reset generator
  clk: starfive: Add JH8100 System-South-West clock generator driver
  dt-bindings: clock: Add StarFive JH8100 Always-On clock and reset
    generator
  clk: starfive: Add JH8100 Always-On clock generator driver
  reset: starfive: Add StarFive JH8100 reset driver
  riscv: dts: starfive: jh8100: Add clocks and resets nodes

 .../clock/starfive,jh8100-aoncrg.yaml         |  77 +++
 .../clock/starfive,jh8100-syscrg-ne.yaml      | 158 +++++
 .../clock/starfive,jh8100-syscrg-nw.yaml      | 119 ++++
 .../clock/starfive,jh8100-syscrg-sw.yaml      |  66 ++
 .../clock/starfive,jh8100-syscrg.yaml         |  66 ++
 MAINTAINERS                                   |  15 +
 arch/riscv/boot/dts/starfive/jh8100-clk.dtsi  | 180 ++++++
 arch/riscv/boot/dts/starfive/jh8100.dtsi      | 115 ++++
 drivers/clk/starfive/Kconfig                  |  49 +-
 drivers/clk/starfive/Makefile                 |   3 +-
 drivers/clk/starfive/clk-starfive-common.c    | 327 ++++++++++
 drivers/clk/starfive/clk-starfive-common.h    | 130 ++++
 .../clk/starfive/clk-starfive-jh7100-audio.c  | 127 ++--
 drivers/clk/starfive/clk-starfive-jh7100.c    | 503 ++++++++--------
 .../clk/starfive/clk-starfive-jh7110-aon.c    |  62 +-
 .../clk/starfive/clk-starfive-jh7110-isp.c    |  72 +--
 .../clk/starfive/clk-starfive-jh7110-stg.c    |  94 +--
 .../clk/starfive/clk-starfive-jh7110-sys.c    | 523 ++++++++--------
 .../clk/starfive/clk-starfive-jh7110-vout.c   |  74 +--
 drivers/clk/starfive/clk-starfive-jh7110.h    |   4 +-
 drivers/clk/starfive/clk-starfive-jh71x0.c    | 327 ----------
 drivers/clk/starfive/clk-starfive-jh71x0.h    | 123 ----
 drivers/clk/starfive/jh8100/Makefile          |   7 +
 drivers/clk/starfive/jh8100/clk-aon.c         | 275 +++++++++
 .../clk/starfive/jh8100/clk-starfive-jh8100.h |  11 +
 drivers/clk/starfive/jh8100/clk-sys-ne.c      | 566 ++++++++++++++++++
 drivers/clk/starfive/jh8100/clk-sys-nw.c      | 268 +++++++++
 drivers/clk/starfive/jh8100/clk-sys-sw.c      | 136 +++++
 drivers/clk/starfive/jh8100/clk-sys.c         | 455 ++++++++++++++
 drivers/reset/starfive/Kconfig                |  14 +-
 drivers/reset/starfive/Makefile               |   4 +-
 ...rfive-jh71x0.c => reset-starfive-common.c} |  68 +--
 .../reset/starfive/reset-starfive-common.h    |  14 +
 .../reset/starfive/reset-starfive-jh7100.c    |   4 +-
 .../reset/starfive/reset-starfive-jh7110.c    |   8 +-
 .../reset/starfive/reset-starfive-jh71x0.h    |  14 -
 .../reset/starfive/reset-starfive-jh8100.c    | 102 ++++
 .../dt-bindings/clock/starfive,jh8100-crg.h   | 430 +++++++++++++
 .../dt-bindings/reset/starfive,jh8100-crg.h   | 127 ++++
 ...rfive-jh71x0.h => reset-starfive-common.h} |  10 +-
 40 files changed, 4485 insertions(+), 1242 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-aoncrg.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-ne.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-nw.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-sw.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg.yaml
 create mode 100644 arch/riscv/boot/dts/starfive/jh8100-clk.dtsi
 create mode 100644 drivers/clk/starfive/clk-starfive-common.c
 create mode 100644 drivers/clk/starfive/clk-starfive-common.h
 delete mode 100644 drivers/clk/starfive/clk-starfive-jh71x0.c
 delete mode 100644 drivers/clk/starfive/clk-starfive-jh71x0.h
 create mode 100644 drivers/clk/starfive/jh8100/Makefile
 create mode 100644 drivers/clk/starfive/jh8100/clk-aon.c
 create mode 100644 drivers/clk/starfive/jh8100/clk-starfive-jh8100.h
 create mode 100644 drivers/clk/starfive/jh8100/clk-sys-ne.c
 create mode 100644 drivers/clk/starfive/jh8100/clk-sys-nw.c
 create mode 100644 drivers/clk/starfive/jh8100/clk-sys-sw.c
 create mode 100644 drivers/clk/starfive/jh8100/clk-sys.c
 rename drivers/reset/starfive/{reset-starfive-jh71x0.c => reset-starfive-common.c} (55%)
 create mode 100644 drivers/reset/starfive/reset-starfive-common.h
 delete mode 100644 drivers/reset/starfive/reset-starfive-jh71x0.h
 create mode 100644 drivers/reset/starfive/reset-starfive-jh8100.c
 create mode 100644 include/dt-bindings/clock/starfive,jh8100-crg.h
 create mode 100644 include/dt-bindings/reset/starfive,jh8100-crg.h
 rename include/soc/starfive/{reset-starfive-jh71x0.h => reset-starfive-common.h} (50%)

Comments

Emil Renner Berthing Dec. 8, 2023, 1:12 p.m. UTC | #1
Sia Jee Heng wrote:
> StarFive JH8100 shares a similar clock and reset design with JH7110.
> To facilitate the reuse of the file and its functionalities, files
> containing the "jh71x0" naming convention are renamed to use the
> "common" wording.
>
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

> ---
>  drivers/clk/starfive/clk-starfive-jh7110-sys.c              | 2 +-
>  drivers/reset/starfive/Kconfig                              | 6 +++---
>  drivers/reset/starfive/Makefile                             | 2 +-
>  .../{reset-starfive-jh71x0.c => reset-starfive-common.c}    | 4 ++--
>  .../{reset-starfive-jh71x0.h => reset-starfive-common.h}    | 6 +++---
>  drivers/reset/starfive/reset-starfive-jh7100.c              | 2 +-
>  drivers/reset/starfive/reset-starfive-jh7110.c              | 4 ++--
>  .../{reset-starfive-jh71x0.h => reset-starfive-common.h}    | 4 ++--
>  8 files changed, 15 insertions(+), 15 deletions(-)
>  rename drivers/reset/starfive/{reset-starfive-jh71x0.c => reset-starfive-common.c} (97%)
>  rename drivers/reset/starfive/{reset-starfive-jh71x0.h => reset-starfive-common.h} (75%)
>  rename include/soc/starfive/{reset-starfive-jh71x0.h => reset-starfive-common.h} (81%)
>
> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-sys.c b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
> index 3884eff9fe93..6e45c580c9ba 100644
> --- a/drivers/clk/starfive/clk-starfive-jh7110-sys.c
> +++ b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
> @@ -14,7 +14,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>
> -#include <soc/starfive/reset-starfive-jh71x0.h>
> +#include <soc/starfive/reset-starfive-common.h>
>
>  #include <dt-bindings/clock/starfive,jh7110-crg.h>
>
> diff --git a/drivers/reset/starfive/Kconfig b/drivers/reset/starfive/Kconfig
> index d832339f61bc..29fbcf1a7d83 100644
> --- a/drivers/reset/starfive/Kconfig
> +++ b/drivers/reset/starfive/Kconfig
> @@ -1,12 +1,12 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>
> -config RESET_STARFIVE_JH71X0
> +config RESET_STARFIVE_COMMON
>  	bool
>
>  config RESET_STARFIVE_JH7100
>  	bool "StarFive JH7100 Reset Driver"
>  	depends on ARCH_STARFIVE || COMPILE_TEST
> -	select RESET_STARFIVE_JH71X0
> +	select RESET_STARFIVE_COMMON
>  	default ARCH_STARFIVE
>  	help
>  	  This enables the reset controller driver for the StarFive JH7100 SoC.
> @@ -15,7 +15,7 @@ config RESET_STARFIVE_JH7110
>  	bool "StarFive JH7110 Reset Driver"
>  	depends on CLK_STARFIVE_JH7110_SYS
>  	select AUXILIARY_BUS
> -	select RESET_STARFIVE_JH71X0
> +	select RESET_STARFIVE_COMMON
>  	default ARCH_STARFIVE
>  	help
>  	  This enables the reset controller driver for the StarFive JH7110 SoC.
> diff --git a/drivers/reset/starfive/Makefile b/drivers/reset/starfive/Makefile
> index 7a44b66fb9d5..582e4c160bd4 100644
> --- a/drivers/reset/starfive/Makefile
> +++ b/drivers/reset/starfive/Makefile
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_RESET_STARFIVE_JH71X0)		+= reset-starfive-jh71x0.o
> +obj-$(CONFIG_RESET_STARFIVE_COMMON)		+= reset-starfive-common.o
>
>  obj-$(CONFIG_RESET_STARFIVE_JH7100)		+= reset-starfive-jh7100.o
>  obj-$(CONFIG_RESET_STARFIVE_JH7110)		+= reset-starfive-jh7110.o
> diff --git a/drivers/reset/starfive/reset-starfive-jh71x0.c b/drivers/reset/starfive/reset-starfive-common.c
> similarity index 97%
> rename from drivers/reset/starfive/reset-starfive-jh71x0.c
> rename to drivers/reset/starfive/reset-starfive-common.c
> index 55bbbd2de52c..dab454e46bbf 100644
> --- a/drivers/reset/starfive/reset-starfive-jh71x0.c
> +++ b/drivers/reset/starfive/reset-starfive-common.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - * Reset driver for the StarFive JH71X0 SoCs
> + * Reset driver for the StarFive SoCs
>   *
>   * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
>   */
> @@ -12,7 +12,7 @@
>  #include <linux/reset-controller.h>
>  #include <linux/spinlock.h>
>
> -#include "reset-starfive-jh71x0.h"
> +#include "reset-starfive-common.h"
>
>  struct jh71x0_reset {
>  	struct reset_controller_dev rcdev;
> diff --git a/drivers/reset/starfive/reset-starfive-jh71x0.h b/drivers/reset/starfive/reset-starfive-common.h
> similarity index 75%
> rename from drivers/reset/starfive/reset-starfive-jh71x0.h
> rename to drivers/reset/starfive/reset-starfive-common.h
> index db7d39a87f87..266acc4b2caf 100644
> --- a/drivers/reset/starfive/reset-starfive-jh71x0.h
> +++ b/drivers/reset/starfive/reset-starfive-common.h
> @@ -3,12 +3,12 @@
>   * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
>   */
>
> -#ifndef __RESET_STARFIVE_JH71X0_H
> -#define __RESET_STARFIVE_JH71X0_H
> +#ifndef __RESET_STARFIVE_COMMON_H
> +#define __RESET_STARFIVE_COMMON_H
>
>  int reset_starfive_jh71x0_register(struct device *dev, struct device_node *of_node,
>  				   void __iomem *assert, void __iomem *status,
>  				   const u32 *asserted, unsigned int nr_resets,
>  				   struct module *owner);
>
> -#endif /* __RESET_STARFIVE_JH71X0_H */
> +#endif /* __RESET_STARFIVE_COMMON_H */
> diff --git a/drivers/reset/starfive/reset-starfive-jh7100.c b/drivers/reset/starfive/reset-starfive-jh7100.c
> index 2a56f7fd4ba7..546dea2e5811 100644
> --- a/drivers/reset/starfive/reset-starfive-jh7100.c
> +++ b/drivers/reset/starfive/reset-starfive-jh7100.c
> @@ -8,7 +8,7 @@
>  #include <linux/mod_devicetable.h>
>  #include <linux/platform_device.h>
>
> -#include "reset-starfive-jh71x0.h"
> +#include "reset-starfive-common.h"
>
>  #include <dt-bindings/reset/starfive-jh7100.h>
>
> diff --git a/drivers/reset/starfive/reset-starfive-jh7110.c b/drivers/reset/starfive/reset-starfive-jh7110.c
> index 29a43f0f2ad6..87dba01491ae 100644
> --- a/drivers/reset/starfive/reset-starfive-jh7110.c
> +++ b/drivers/reset/starfive/reset-starfive-jh7110.c
> @@ -7,9 +7,9 @@
>
>  #include <linux/auxiliary_bus.h>
>
> -#include <soc/starfive/reset-starfive-jh71x0.h>
> +#include <soc/starfive/reset-starfive-common.h>
>
> -#include "reset-starfive-jh71x0.h"
> +#include "reset-starfive-common.h"
>
>  #include <dt-bindings/reset/starfive,jh7110-crg.h>
>
> diff --git a/include/soc/starfive/reset-starfive-jh71x0.h b/include/soc/starfive/reset-starfive-common.h
> similarity index 81%
> rename from include/soc/starfive/reset-starfive-jh71x0.h
> rename to include/soc/starfive/reset-starfive-common.h
> index 47b486ececc5..56d8f413cf18 100644
> --- a/include/soc/starfive/reset-starfive-jh71x0.h
> +++ b/include/soc/starfive/reset-starfive-common.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef __SOC_STARFIVE_RESET_JH71X0_H
> -#define __SOC_STARFIVE_RESET_JH71X0_H
> +#ifndef __SOC_STARFIVE_RESET_COMMON_H
> +#define __SOC_STARFIVE_RESET_COMMON_H
>
>  #include <linux/auxiliary_bus.h>
>  #include <linux/compiler_types.h>
> --
> 2.34.1
>
Emil Renner Berthing Dec. 8, 2023, 1:16 p.m. UTC | #2
Sia Jee Heng wrote:
> StarFive JH8100 shares a similar clock and reset design with JH7110.
> To facilitate the reuse of the file and its functionalities, files
> containing the "jh71x0" naming convention are renamed to use the
> "common" wording.
>
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

> ---
>  drivers/clk/starfive/Kconfig                              | 8 ++++----
>  drivers/clk/starfive/Makefile                             | 2 +-
>  .../{clk-starfive-jh71x0.c => clk-starfive-common.c}      | 4 ++--
>  .../{clk-starfive-jh71x0.h => clk-starfive-common.h}      | 4 ++--
>  drivers/clk/starfive/clk-starfive-jh7100-audio.c          | 2 +-
>  drivers/clk/starfive/clk-starfive-jh7100.c                | 2 +-
>  drivers/clk/starfive/clk-starfive-jh7110.h                | 2 +-
>  7 files changed, 12 insertions(+), 12 deletions(-)
>  rename drivers/clk/starfive/{clk-starfive-jh71x0.c => clk-starfive-common.c} (99%)
>  rename drivers/clk/starfive/{clk-starfive-jh71x0.h => clk-starfive-common.h} (97%)
>
> diff --git a/drivers/clk/starfive/Kconfig b/drivers/clk/starfive/Kconfig
> index bd29358ffeec..ff8eace36e64 100644
> --- a/drivers/clk/starfive/Kconfig
> +++ b/drivers/clk/starfive/Kconfig
> @@ -1,12 +1,12 @@
>  # SPDX-License-Identifier: GPL-2.0
>
> -config CLK_STARFIVE_JH71X0
> +config CLK_STARFIVE_COMMON
>  	bool
>
>  config CLK_STARFIVE_JH7100
>  	bool "StarFive JH7100 clock support"
>  	depends on ARCH_STARFIVE || COMPILE_TEST
> -	select CLK_STARFIVE_JH71X0
> +	select CLK_STARFIVE_COMMON
>  	default ARCH_STARFIVE
>  	help
>  	  Say yes here to support the clock controller on the StarFive JH7100
> @@ -15,7 +15,7 @@ config CLK_STARFIVE_JH7100
>  config CLK_STARFIVE_JH7100_AUDIO
>  	tristate "StarFive JH7100 audio clock support"
>  	depends on CLK_STARFIVE_JH7100
> -	select CLK_STARFIVE_JH71X0
> +	select CLK_STARFIVE_COMMON
>  	default m if ARCH_STARFIVE
>  	help
>  	  Say Y or M here to support the audio clocks on the StarFive JH7100
> @@ -33,7 +33,7 @@ config CLK_STARFIVE_JH7110_SYS
>  	bool "StarFive JH7110 system clock support"
>  	depends on ARCH_STARFIVE || COMPILE_TEST
>  	select AUXILIARY_BUS
> -	select CLK_STARFIVE_JH71X0
> +	select CLK_STARFIVE_COMMON
>  	select RESET_STARFIVE_JH7110 if RESET_CONTROLLER
>  	select CLK_STARFIVE_JH7110_PLL
>  	default ARCH_STARFIVE
> diff --git a/drivers/clk/starfive/Makefile b/drivers/clk/starfive/Makefile
> index 199ac0f37a2f..012f7ee83f8e 100644
> --- a/drivers/clk/starfive/Makefile
> +++ b/drivers/clk/starfive/Makefile
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_CLK_STARFIVE_JH71X0)	+= clk-starfive-jh71x0.o
> +obj-$(CONFIG_CLK_STARFIVE_COMMON)	+= clk-starfive-common.o
>
>  obj-$(CONFIG_CLK_STARFIVE_JH7100)	+= clk-starfive-jh7100.o
>  obj-$(CONFIG_CLK_STARFIVE_JH7100_AUDIO)	+= clk-starfive-jh7100-audio.o
> diff --git a/drivers/clk/starfive/clk-starfive-jh71x0.c b/drivers/clk/starfive/clk-starfive-common.c
> similarity index 99%
> rename from drivers/clk/starfive/clk-starfive-jh71x0.c
> rename to drivers/clk/starfive/clk-starfive-common.c
> index aebc99264a0b..a12490c97957 100644
> --- a/drivers/clk/starfive/clk-starfive-jh71x0.c
> +++ b/drivers/clk/starfive/clk-starfive-common.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * StarFive JH71X0 Clock Generator Driver
> + * StarFive Clock Generator Driver
>   *
>   * Copyright (C) 2021-2022 Emil Renner Berthing <kernel@esmil.dk>
>   */
> @@ -10,7 +10,7 @@
>  #include <linux/device.h>
>  #include <linux/io.h>
>
> -#include "clk-starfive-jh71x0.h"
> +#include "clk-starfive-common.h"
>
>  static struct jh71x0_clk *jh71x0_clk_from(struct clk_hw *hw)
>  {
> diff --git a/drivers/clk/starfive/clk-starfive-jh71x0.h b/drivers/clk/starfive/clk-starfive-common.h
> similarity index 97%
> rename from drivers/clk/starfive/clk-starfive-jh71x0.h
> rename to drivers/clk/starfive/clk-starfive-common.h
> index 34bb11c72eb7..1f32f7024e9f 100644
> --- a/drivers/clk/starfive/clk-starfive-jh71x0.h
> +++ b/drivers/clk/starfive/clk-starfive-common.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef __CLK_STARFIVE_JH71X0_H
> -#define __CLK_STARFIVE_JH71X0_H
> +#ifndef __CLK_STARFIVE_COMMON_H
> +#define __CLK_STARFIVE_COMMON_H
>
>  #include <linux/bits.h>
>  #include <linux/clk-provider.h>
> diff --git a/drivers/clk/starfive/clk-starfive-jh7100-audio.c b/drivers/clk/starfive/clk-starfive-jh7100-audio.c
> index ee4bda14a40e..dc4c278606d7 100644
> --- a/drivers/clk/starfive/clk-starfive-jh7100-audio.c
> +++ b/drivers/clk/starfive/clk-starfive-jh7100-audio.c
> @@ -15,7 +15,7 @@
>
>  #include <dt-bindings/clock/starfive-jh7100-audio.h>
>
> -#include "clk-starfive-jh71x0.h"
> +#include "clk-starfive-common.h"
>
>  /* external clocks */
>  #define JH7100_AUDCLK_AUDIO_SRC			(JH7100_AUDCLK_END + 0)
> diff --git a/drivers/clk/starfive/clk-starfive-jh7100.c b/drivers/clk/starfive/clk-starfive-jh7100.c
> index 69cc11ea7e33..6bb6a6af9f28 100644
> --- a/drivers/clk/starfive/clk-starfive-jh7100.c
> +++ b/drivers/clk/starfive/clk-starfive-jh7100.c
> @@ -15,7 +15,7 @@
>
>  #include <dt-bindings/clock/starfive-jh7100.h>
>
> -#include "clk-starfive-jh71x0.h"
> +#include "clk-starfive-common.h"
>
>  /* external clocks */
>  #define JH7100_CLK_OSC_SYS		(JH7100_CLK_END + 0)
> diff --git a/drivers/clk/starfive/clk-starfive-jh7110.h b/drivers/clk/starfive/clk-starfive-jh7110.h
> index 0659adae4d76..6b1bdf860f00 100644
> --- a/drivers/clk/starfive/clk-starfive-jh7110.h
> +++ b/drivers/clk/starfive/clk-starfive-jh7110.h
> @@ -2,7 +2,7 @@
>  #ifndef __CLK_STARFIVE_JH7110_H
>  #define __CLK_STARFIVE_JH7110_H
>
> -#include "clk-starfive-jh71x0.h"
> +#include "clk-starfive-common.h"
>
>  /* top clocks of ISP/VOUT domain from JH7110 SYSCRG */
>  struct jh7110_top_sysclk {
> --
> 2.34.1
>
Emil Renner Berthing Dec. 8, 2023, 4:37 p.m. UTC | #3
Sia Jee Heng wrote:
> Add bindings for the System-North-West clock and reset generator
> (SYSCRG-NW) on JH8100 SoC.
>
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
> ---
>  .../clock/starfive,jh8100-syscrg-nw.yaml      | 119 ++++++++++++++++++

The JH7110 clocks, the JH8100 system and always-on all follow the Xcrg pattern:
syscrg
aoncrg
stgcrg
ispcrg
voutcrg
etc.

Is there a reason the north-west, north-east and south-west breaks this pattern?
I'd have expected them to be called something like
nwcrg, JH8100_NWCLK_*, JH8100_NWRST_*,
necrg, JH8100_NECLK_*, JH8100_NERST_* and
swcrg, JH8100_SWCLK_*, JH8100_SWRST_*

Just like all the other Starfive drivers.

>  .../dt-bindings/clock/starfive,jh8100-crg.h   |  45 +++++++
>  .../dt-bindings/reset/starfive,jh8100-crg.h   |  15 +++
>  3 files changed, 179 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-nw.yaml
>
> diff --git a/Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-nw.yaml b/Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-nw.yaml
> new file mode 100644
> index 000000000000..b16a874828dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-nw.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/starfive,jh8100-syscrg-nw.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH8100 System-North-West Clock and Reset Generator
> +
> +maintainers:
> +  - Sia Jee Heng <jeeheng.sia@starfivetech.com>
> +
> +properties:
> +  compatible:
> +    const: starfive,jh8100-syscrg-nw
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Main Oscillator (24 MHz)
> +      - description: APB_BUS clock from SYSCRG
> +      - description: ISP_2X clock from SYSCRG
> +      - description: ISP_AXI clock from SYSCRG
> +      - description: VOUT_ROOT0 clock from SYSCRG
> +      - description: VOUT_ROOT1 clock from SYSCRG
> +      - description: VOUT_SCAN_ATS clock from SYSCRG
> +      - description: VOUT_DC_CORE clock from SYSCRG
> +      - description: VOUT_AXI clock from SYSCRG
> +      - description: AXI_400 clock from SYSCRG
> +      - description: AXI_200 clock from SYSCRG
> +      - description: Peripheral clock from SYSCRG
> +      - description: External DVP clock
> +      - description: External ISP DPHY TAP TCK clock
> +      - description: External golbal clock
> +      - description: External i2s_tscko clock
> +      - description: External VOUT MIPI DPHY TAP TCK
> +      - description: External VOUT eDP TAP TCK
> +      - description: External SPI In2 clock
> +
> +  clock-names:
> +    items:
> +      - const: clk_osc
> +      - const: sys_clk_apb_bus
> +      - const: sys_clk_isp_2x
> +      - const: sys_clk_isp_axi
> +      - const: sys_clk_vout_root0
> +      - const: sys_clk_vout_root1
> +      - const: sys_clk_vout_scan_ats
> +      - const: sys_clk_vout_dc_core
> +      - const: sys_clk_vout_axi
> +      - const: sys_clk_axi_400
> +      - const: sys_clk_axi_200
> +      - const: sys_clk_perh_root_preosc
> +      - const: clk_dvp_ext
> +      - const: clk_isp_dphy_tap_tck_ext
> +      - const: clk_glb_ext_clk
> +      - const: clk_i2s_tscko
> +      - const: clk_vout_mipi_dphy_tap_tck_ext
> +      - const: clk_vout_edp_tap_tck_ext
> +      - const: clk_spi_in2_ext
> +
> +  '#clock-cells':
> +    const: 1
> +    description:
> +      See <dt-bindings/clock/starfive,jh8100-crg.h> for valid indices.
> +
> +  '#reset-cells':
> +    const: 1
> +    description:
> +      See <dt-bindings/reset/starfive,jh8100-crg.h> for valid indices.
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/starfive,jh8100-crg.h>
> +
> +    clock-controller@123c0000 {
> +            compatible = "starfive,jh8100-syscrg-nw";
> +            reg = <0x123c0000 0x10000>;
> +            clocks = <&clk_osc>, <&syscrg SYSCRG_CLK_APB_BUS>,
> +                     <&syscrg SYSCRG_CLK_ISP_2X>,
> +                     <&syscrg SYSCRG_CLK_ISP_AXI>,
> +                     <&syscrg SYSCRG_CLK_VOUT_ROOT0>,
> +                     <&syscrg SYSCRG_CLK_VOUT_ROOT1>,
> +                     <&syscrg SYSCRG_CLK_VOUT_SCAN_ATS>,
> +                     <&syscrg SYSCRG_CLK_VOUT_DC_CORE>,
> +                     <&syscrg SYSCRG_CLK_VOUT_AXI>,
> +                     <&syscrg SYSCRG_CLK_AXI_400>,
> +                     <&syscrg SYSCRG_CLK_AXI_200>,
> +                     <&syscrg SYSCRG_CLK_PERH_ROOT_PREOSC>,
> +                     <&clk_dvp_ext>,
> +                     <&clk_isp_dphy_tap_tck_ext>,
> +                     <&clk_glb_ext_clk>,
> +                     <&clk_i2s_tscko>,
> +                     <&clk_vout_mipi_dphy_tap_tck_ext>,
> +                     <&clk_vout_edp_tap_tck_ext>,
> +                     <&clk_spi_in2_ext>;
> +            clock-names = "clk_osc", "sys_clk_apb_bus", "sys_clk_isp_2x",
> +                          "sys_clk_isp_axi", "sys_clk_vout_root0",
> +                          "sys_clk_vout_root1", "sys_clk_vout_scan_ats",
> +                          "sys_clk_vout_dc_core", "sys_clk_vout_axi",
> +                          "sys_clk_axi_400", "sys_clk_axi_200",
> +                          "sys_clk_perh_root_preosc", "clk_dvp_ext",
> +                          "clk_isp_dphy_tap_tck_ext", "clk_glb_ext_clk",
> +                          "clk_i2s_tscko", "clk_vout_mipi_dphy_tap_tck_ext",
> +                          "clk_vout_edp_tap_tck_ext", "clk_spi_in2_ext";
> +            #clock-cells = <1>;
> +            #reset-cells = <1>;
> +    };
> diff --git a/include/dt-bindings/clock/starfive,jh8100-crg.h b/include/dt-bindings/clock/starfive,jh8100-crg.h
> index e5bb588ce798..8417455c2409 100644
> --- a/include/dt-bindings/clock/starfive,jh8100-crg.h
> +++ b/include/dt-bindings/clock/starfive,jh8100-crg.h
> @@ -120,4 +120,49 @@
>  #define SYSCRG_CLK_NNE_ICG_EN						108
>
>  #define SYSCRG_CLK_END							109
> +
> +/* SYSCRG_NW_CLK */
> +#define SYSCRG_NW_CLK_PLL5_DIV2						0
> +#define SYSCRG_NW_CLK_GCLK5						1
> +#define SYSCRG_NW_CLK_GPIO_100						2
> +#define SYSCRG_NW_CLK_GPIO_50						3
> +#define SYSCRG_NW_CLK_GPIO_150						4
> +#define SYSCRG_NW_CLK_GPIO_60						5
> +#define SYSCRG_NW_CLK_IOMUX_WEST_PCLK					6
> +#define SYSCRG_NW_CLK_I2C6_APB						7
> +#define SYSCRG_NW_CLK_I2C7_APB						8
> +#define SYSCRG_NW_CLK_SPI2_APB						9
> +#define SYSCRG_NW_CLK_SPI2_CORE						10
> +#define SYSCRG_NW_CLK_SPI2_SCLK_IN					11
> +#define SYSCRG_NW_CLK_SMBUS1_APB					12
> +#define SYSCRG_NW_CLK_SMBUS1_CORE					13
> +#define SYSCRG_NW_CLK_ISP_DVP						14
> +#define SYSCRG_NW_CLK_ISP_CORE_2X					15
> +#define SYSCRG_NW_CLK_ISP_AXI						16
> +#define SYSCRG_NW_CLK_ISP_DPHY_TAP_TCK					17
> +#define SYSCRG_NW_CLK_FLEXNOC_ISPSLV					18
> +#define SYSCRG_NW_CLK_VOUT_PIX0						19
> +#define SYSCRG_NW_CLK_VOUT_PIX1						20
> +#define SYSCRG_NW_CLK_VOUT_SCAN_ATS					21
> +#define SYSCRG_NW_CLK_VOUT_DC_CORE					22
> +#define SYSCRG_NW_CLK_VOUT_APB						23
> +#define SYSCRG_NW_CLK_VOUT_DSI						24
> +#define SYSCRG_NW_CLK_VOUT_AHB						25
> +#define SYSCRG_NW_CLK_VOUT_AXI						26
> +#define SYSCRG_NW_CLK_VOUT_MIPI_DPHY_TAP_TCK				27
> +#define SYSCRG_NW_CLK_VOUT_EDP_PHY_TAP_TCK				28
> +#define SYSCRG_NW_CLK_UART5_CORE_PREOSC					29
> +#define SYSCRG_NW_CLK_UART5_APB						30
> +#define SYSCRG_NW_CLK_UART5_CORE					31
> +#define SYSCRG_NW_CLK_UART6_CORE_PREOSC					32
> +#define SYSCRG_NW_CLK_UART6_APB						33
> +#define SYSCRG_NW_CLK_UART6_CORE					34
> +#define SYSCRG_NW_CLK_SPI2_ICG_EN					35
> +#define SYSCRG_NW_CLK_SMBUS1_ICG_EN					36
> +#define SYSCRG_NW_CLK_ISP_ICG_EN					37
> +#define SYSCRG_NW_CLK_VOUT_ICG_EN					38
> +#define SYSCRG_NW_CLK_UART5_ICG_EN					39
> +#define SYSCRG_NW_CLK_UART6_ICG_EN					40
> +
> +#define SYSCRG_NW_CLK_END						41
>  #endif /* __DT_BINDINGS_CLOCK_STARFIVE_JH8100_H__ */
> diff --git a/include/dt-bindings/reset/starfive,jh8100-crg.h b/include/dt-bindings/reset/starfive,jh8100-crg.h
> index 3b7b92488e76..8c3a858bdf6a 100644
> --- a/include/dt-bindings/reset/starfive,jh8100-crg.h
> +++ b/include/dt-bindings/reset/starfive,jh8100-crg.h
> @@ -20,4 +20,19 @@
>
>  #define SYSCRG_RESET_NR_RESETS					8
>
> +/*
> + * syscrg_nw: assert0
> + */
> +#define SYSCRG_NW_RSTN_PRESETN					0
> +#define SYSCRG_NW_RSTN_SYS_IOMUX_W				1
> +#define SYSCRG_NW_RSTN_I2C6					2
> +#define SYSCRG_NW_RSTN_I2C7					3
> +#define SYSCRG_NW_RSTN_SPI2					4
> +#define SYSCRG_NW_RSTN_SMBUS1					5
> +#define SYSCRG_NW_RSTN_UART5					6
> +#define SYSCRG_NW_RSTN_UART6					7
> +#define SYSCRG_NW_RSTN_MERAK0_TVSENSOR				8
> +#define SYSCRG_NW_RSTN_MERAK1_TVSENSOR				9
> +
> +#define SYSCRG_NW_RESET_NR_RESETS				10
>  #endif /* __DT_BINDINGS_RESET_STARFIVE_JH8100_H__ */
> --
> 2.34.1
>
Emil Renner Berthing Dec. 8, 2023, 4:39 p.m. UTC | #4
Sia Jee Heng wrote:
> Add SYSCRG/SYSCRG-NE/SYSCRG-NW/SYSCRG-SW/AONCRG clock and reset
> nodes for JH8100 RISC-V SoC.
>
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
> ---
>  arch/riscv/boot/dts/starfive/jh8100-clk.dtsi | 180 +++++++++++++++++++
>  arch/riscv/boot/dts/starfive/jh8100.dtsi     | 115 ++++++++++++

Why the split here? I mean why can't the clocks just be in the jh8100.dtsi?

>  2 files changed, 295 insertions(+)
>  create mode 100644 arch/riscv/boot/dts/starfive/jh8100-clk.dtsi
>
> diff --git a/arch/riscv/boot/dts/starfive/jh8100-clk.dtsi b/arch/riscv/boot/dts/starfive/jh8100-clk.dtsi
> new file mode 100644
> index 000000000000..27ba249f523e
> --- /dev/null
> +++ b/arch/riscv/boot/dts/starfive/jh8100-clk.dtsi
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
> + */
> +
> +/ {
> +	clk_osc: clk_osc {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <24000000>;
> +	};
> +
> +	clk_i2srx_bclk_ext: clk_i2srx_bclk_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <12288000>;
> +	};
> +
> +	clk_i2srx_lrck_ext: clk_i2srx_lrck_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <192000>;
> +	};
> +
> +	clk_mclk_ext: clk_mclk_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <49152000>;
> +	};
> +	/* sys-ne */
> +	clk_usb3_tap_tck_ext: clk_usb3_tap_tck_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <100000000>;
> +	};
> +
> +	clk_glb_ext_clk: clk_glb_ext_clk {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <30000000>;
> +	};
> +
> +	clk_usb1_tap_tck_ext: clk_usb1_tap_tck_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <100000000>;
> +	};
> +
> +	clk_usb2_tap_tck_ext: clk_usb2_tap_tck_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <100000000>;
> +	};
> +
> +	clk_i2s_tscko: clk_i2s_tscko {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <12800000>;
> +	};
> +
> +	clk_typec_tap_tck_ext: clk_typec_tap_tck_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <100000000>;
> +	};
> +
> +	clk_spi_in0_ext: clk_spi_in0_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <100000000>;
> +	};
> +
> +	clk_spi_in1_ext: clk_spi_in1_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <100000000>;
> +	};
> +
> +	clk_spi_in2_ext: clk_spi_in2_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <100000000>;
> +	};
> +
> +	clk_i2stx_bclk_ext: clk_i2stx_bclk_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <12288000>;
> +	};
> +
> +	clk_i2stx_lrck_ext: clk_i2stx_lrck_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <192000>;
> +	};
> +	/* sys-nw */
> +	clk_dvp_ext: clk_dvp_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <150000000>;
> +	};
> +
> +	clk_isp_dphy_tap_tck_ext: clk_isp_dphy_tap_tck_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <100000000>;
> +	};
> +
> +	clk_vout_mipi_dphy_tap_tck_ext: clk_vout_mipi_dphy_tap_tck_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <100000000>;
> +	};
> +
> +	clk_vout_edp_tap_tck_ext: clk_vout_edp_tap_tck_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <100000000>;
> +	};
> +
> +	clk_rtc: clk_rtc {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <32768>;
> +	};
> +	/* aon */
> +	clk_gmac0_rmii_func: clk_gmac0_rmii_func {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <50000000>;
> +	};
> +
> +	clk_gmac0_rgmii_func: clk_gmac0_rgmii_func {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <125000000>;
> +	};
> +
> +	clk_aon50: clk_aon50 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <50000000>;
> +	};
> +
> +	clk_aon125: clk_aon125 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <125000000>;
> +	};
> +
> +	clk_aon2000: clk_aon2000 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <2000000000>;
> +	};
> +
> +	clk_aon200: clk_aon200 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <200000000>;
> +	};
> +
> +	clk_aon667: clk_isp_aon667 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <667000000>;
> +	};
> +
> +	clk_i3c_ext: clk_i3c_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <12500000>;
> +	};
> +
> +	clk_espi_ext: clk_espi_ext {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <60000000>;
> +	};
> +};
> diff --git a/arch/riscv/boot/dts/starfive/jh8100.dtsi b/arch/riscv/boot/dts/starfive/jh8100.dtsi
> index f26aff5c1ddf..9863c61324a0 100644
> --- a/arch/riscv/boot/dts/starfive/jh8100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh8100.dtsi
> @@ -4,6 +4,9 @@
>   */
>
>  /dts-v1/;
> +#include <dt-bindings/clock/starfive,jh8100-crg.h>
> +#include <dt-bindings/reset/starfive,jh8100-crg.h>
> +#include "jh8100-clk.dtsi"
>
>  / {
>  	compatible = "starfive,jh8100";
> @@ -357,6 +360,104 @@ uart4: serial@121a0000  {
>  			status = "disabled";
>  		};
>
> +		syscrg_ne: syscrg_ne@12320000 {
> +			compatible = "starfive,jh8100-syscrg-ne";
> +			reg = <0x0 0x12320000 0x0 0x10000>;
> +			clocks = <&clk_osc>, <&syscrg SYSCRG_CLK_AXI_400>,
> +				 <&syscrg SYSCRG_CLK_VOUT_ROOT0>,
> +				 <&syscrg SYSCRG_CLK_VOUT_ROOT1>,
> +				 <&syscrg SYSCRG_CLK_USB_WRAP_480>,
> +				 <&syscrg SYSCRG_CLK_USB_WRAP_625>,
> +				 <&syscrg SYSCRG_CLK_USB_WRAP_240>,
> +				 <&syscrg SYSCRG_CLK_USB_WRAP_60>,
> +				 <&syscrg SYSCRG_CLK_USB_WRAP_156P25>,
> +				 <&syscrg SYSCRG_CLK_USB_WRAP_312P5>,
> +				 <&syscrg SYSCRG_CLK_USB_125M>,
> +				 <&syscrg_nw SYSCRG_NW_CLK_GPIO_100>,
> +				 <&syscrg SYSCRG_CLK_PERH_ROOT>, <&syscrg SYSCRG_CLK_MCLK>,
> +				 <&syscrg SYSCRG_CLK_PERH_ROOT_PREOSC>,
> +				 <&syscrg SYSCRG_CLK_AHB0>,
> +				 <&syscrg SYSCRG_CLK_APB_BUS_PER1>,
> +				 <&syscrg SYSCRG_CLK_APB_BUS_PER2>,
> +				 <&syscrg SYSCRG_CLK_APB_BUS_PER3>,
> +				 <&syscrg SYSCRG_CLK_APB_BUS_PER5>,
> +				 <&syscrg SYSCRG_CLK_VENC_ROOT>,
> +				 <&syscrg SYSCRG_CLK_SPI_CORE_100>,
> +				 <&clk_glb_ext_clk>, <&clk_usb3_tap_tck_ext>,
> +				 <&clk_usb1_tap_tck_ext>, <&clk_usb2_tap_tck_ext>,
> +				 <&clk_typec_tap_tck_ext>, <&clk_spi_in0_ext>,
> +				 <&clk_spi_in1_ext>, <&clk_i2stx_bclk_ext>, <&clk_i2stx_lrck_ext>;
> +			clock-names = "clk_osc", "sys_clk_axi_400",
> +				      "sys_clk_vout_root0", "sys_clk_vout_root1",
> +				      "sys_clk_usb_wrap_480", "sys_clk_usb_wrap_625",
> +				      "sys_clk_usb_wrap_240", "sys_clk_usb_wrap_60",
> +				      "sys_clk_usb_wrap_156p25", "sys_clk_usb_wrap_312p5",
> +				      "sys_clk_usb_125m", "sys_nw_clk_gpio_100",
> +				      "sys_clk_perh_root", "sys_clk_mclk",
> +				      "sys_clk_perh_root_preosc", "sys_clk_ahb0",
> +				      "sys_clk_apb_bus_per1", "sys_clk_apb_bus_per2",
> +				      "sys_clk_apb_bus_per3", "sys_clk_apb_bus_per5",
> +				      "sys_clk_venc_root", "sys_clk_spi_core_100",
> +				      "clk_glb_ext_clk", "clk_usb3_tap_tck_ext",
> +				      "clk_usb1_tap_tck_ext", "clk_usb2_tap_tck_ext",
> +				      "clk_typec_tap_tck_ext", "clk_spi_in0_ext",
> +				      "clk_spi_in1_ext", "clk_i2stx_bclk_ext",
> +				      "clk_i2stx_lrck_ext";
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +		};
> +
> +		syscrg_nw: syscrg_nw@123c0000 {
> +			compatible = "starfive,jh8100-syscrg-nw";
> +			reg = <0x0 0x123c0000 0x0 0x10000>;
> +			clocks = <&clk_osc>, <&syscrg SYSCRG_CLK_APB_BUS>,
> +				 <&syscrg SYSCRG_CLK_ISP_2X>, <&syscrg SYSCRG_CLK_ISP_AXI>,
> +				 <&syscrg SYSCRG_CLK_VOUT_ROOT0>, <&syscrg SYSCRG_CLK_VOUT_ROOT1>,
> +				 <&syscrg SYSCRG_CLK_VOUT_SCAN_ATS>,
> +				 <&syscrg SYSCRG_CLK_VOUT_DC_CORE>, <&syscrg SYSCRG_CLK_VOUT_AXI>,
> +				 <&syscrg SYSCRG_CLK_AXI_400>, <&syscrg SYSCRG_CLK_AXI_200>,
> +				 <&syscrg SYSCRG_CLK_PERH_ROOT_PREOSC>,
> +				 <&clk_dvp_ext>, <&clk_isp_dphy_tap_tck_ext>,
> +				 <&clk_glb_ext_clk>, <&clk_i2s_tscko>,
> +				 <&clk_vout_mipi_dphy_tap_tck_ext>, <&clk_vout_edp_tap_tck_ext>,
> +				 <&clk_spi_in2_ext>;
> +			clock-names = "clk_osc", "sys_clk_apb_bus",
> +				      "sys_clk_isp_2x", "sys_clk_isp_axi",
> +				      "sys_clk_vout_root0", "sys_clk_vout_root1",
> +				      "sys_clk_vout_scan_ats", "sys_clk_vout_dc_core",
> +				      "sys_clk_vout_axi", "sys_clk_axi_400",
> +				      "sys_clk_axi_200", "sys_clk_perh_root_preosc", "clk_dvp_ext",
> +				      "clk_isp_dphy_tap_tck_ext", "clk_glb_ext_clk",
> +				      "clk_i2s_tscko", "clk_vout_mipi_dphy_tap_tck_ext",
> +				      "clk_vout_edp_tap_tck_ext", "clk_spi_in2_ext";
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +		};
> +
> +		syscrg: syscrg@126d0000 {
> +			compatible = "starfive,jh8100-syscrg";
> +			reg = <0x0 0x126d0000 0x0 0x10000>;
> +			clocks = <&clk_osc>, <&clk_i2srx_bclk_ext>,
> +				 <&clk_i2srx_lrck_ext>, <&clk_mclk_ext>;
> +			clock-names = "clk_osc", "clk_i2srx_bclk_ext",
> +				      "clk_i2srx_lrck_ext", "clk_mclk_ext";
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +		};
> +
> +		syscrg_sw: syscrg_sw@12720000 {
> +			compatible = "starfive,jh8100-syscrg-sw";
> +			reg = <0x0 0x12720000 0x0 0x10000>;
> +			clocks = <&syscrg SYSCRG_CLK_APB_BUS>,
> +				 <&syscrg SYSCRG_CLK_VDEC_ROOT>,
> +				 <&syscrg SYSCRG_CLK_FLEXNOC1>;
> +			clock-names = "sys_clk_apb_bus",
> +				      "sys_clk_vdec_root",
> +				      "sys_clk_flexnoc1";
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +		};
> +
>  		uart5: serial@127d0000  {
>  			compatible = "starfive,jh8100-uart", "cdns,uart-r1p8";
>  			reg = <0x0 0x127d0000 0x0 0x10000>;
> @@ -374,5 +475,19 @@ uart6: serial@127e0000  {
>  			interrupts = <73>;
>  			status = "disabled";
>  		};
> +
> +		aoncrg: aoncrg@1f310000 {
> +			compatible = "starfive,jh8100-aoncrg";
> +			reg = <0x0 0x1f310000 0x0 0x10000>;
> +			clocks = <&clk_osc>, <&clk_gmac0_rmii_func>,
> +				 <&clk_gmac0_rgmii_func>, <&clk_aon125>,
> +				 <&clk_aon2000>, <&clk_aon200>,
> +				 <&clk_aon667>, <&clk_rtc>;
> +			clock-names = "clk_osc", "clk_gmac0_rmii_func", "clk_gmac0_rgmii_func",
> +				      "clk_aon125", "clk_aon2000", "clk_aon200",
> +				      "clk_aon667", "clk_rtc";
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +		};
>  	};
>  };
> --
> 2.34.1
>
Emil Renner Berthing Dec. 8, 2023, 4:52 p.m. UTC | #5
Sia Jee Heng wrote:
> This patch series enabled basic clock & reset support for StarFive
> JH8100 SoC.
>
> This patch series depends on the Initial device tree support for
> StarFive JH8100 SoC patch series which can be found at below link:
> https://lore.kernel.org/lkml/20231201121410.95298-1-jeeheng.sia@starfivetech.com/
>
> StarFive JH8100 shares a similar clock and reset design with JH7110.
> To facilitate the reuse of the file and its functionalities, files
> containing the 'jh71x0' naming convention are renamed to use the
> 'common' wording. Internal functions that contain the 'jh71x0'
> naming convention are renamed to use 'starfive.' This is accomplished
> through patches 1, 2, 3, and 4.

I'm a little sceptical all this renaming is worth it, but if think it's likely
that future starfive SoCs can use the same clock drivers I'm ok with it. Just
know that you'll look a bit silly if your "JH9100" can't use these drivers and
you'll already need different starfive and starfive-gen2 drivers.

/Emil
>
>
> Patch 5 adds documentation to describe System (SYSCRG) Clock & Reset
> binding.
> Patch 6 adds SYSCRG clock driver.
>
> patch 7 adds documentation to describe System-North-West (SYSCRG-NW)
> Clock & Reset binding.
> Patch 8 adds SYSCRG-NW clock driver.
>
> patch 9 adds documentation to describe System-North-East (SYSCRG-NE)
> Clock & Reset binding.
> Patch 10 adds SYSCRG-NE clock driver.
>
> patch 11 adds documentation to describe System-South-West (SYSCRG-SW)
> Clock & Reset binding.
> Patch 12 adds SYSCRG-SW clock driver.
>
> patch 13 adds documentation to describe Always-On (AON)
> Clock & Reset binding.
> Patch 14 adds AON clock driver.
>
> Patch 15 adds support for the auxiliary reset driver.
>
> Patch 16 adds clocks and reset nodes to the JH8100 device tree.
>
> Sia Jee Heng (16):
>   reset: starfive: Rename file name "jh71x0" to "common"
>   reset: starfive: Convert the word "jh71x0" to "starfive"
>   clk: starfive: Rename file name "jh71x0" to "common"
>   clk: starfive: Convert the word "jh71x0" to "starfive"
>   dt-bindings: clock: Add StarFive JH8100 System clock and reset
>     generator
>   clk: starfive: Add JH8100 System clock generator driver
>   dt-bindings: clock: Add StarFive JH8100 System-North-West clock and
>     reset generator
>   clk: starfive: Add JH8100 System-North-West clock generator driver
>   dt-bindings: clock: Add StarFive JH8100 System-North-East clock and
>     reset generator
>   clk: starfive: Add JH8100 System-North-East clock generator driver
>   dt-bindings: clock: Add StarFive JH8100 System-South-West clock and
>     reset generator
>   clk: starfive: Add JH8100 System-South-West clock generator driver
>   dt-bindings: clock: Add StarFive JH8100 Always-On clock and reset
>     generator
>   clk: starfive: Add JH8100 Always-On clock generator driver
>   reset: starfive: Add StarFive JH8100 reset driver
>   riscv: dts: starfive: jh8100: Add clocks and resets nodes
>
>  .../clock/starfive,jh8100-aoncrg.yaml         |  77 +++
>  .../clock/starfive,jh8100-syscrg-ne.yaml      | 158 +++++
>  .../clock/starfive,jh8100-syscrg-nw.yaml      | 119 ++++
>  .../clock/starfive,jh8100-syscrg-sw.yaml      |  66 ++
>  .../clock/starfive,jh8100-syscrg.yaml         |  66 ++
>  MAINTAINERS                                   |  15 +
>  arch/riscv/boot/dts/starfive/jh8100-clk.dtsi  | 180 ++++++
>  arch/riscv/boot/dts/starfive/jh8100.dtsi      | 115 ++++
>  drivers/clk/starfive/Kconfig                  |  49 +-
>  drivers/clk/starfive/Makefile                 |   3 +-
>  drivers/clk/starfive/clk-starfive-common.c    | 327 ++++++++++
>  drivers/clk/starfive/clk-starfive-common.h    | 130 ++++
>  .../clk/starfive/clk-starfive-jh7100-audio.c  | 127 ++--
>  drivers/clk/starfive/clk-starfive-jh7100.c    | 503 ++++++++--------
>  .../clk/starfive/clk-starfive-jh7110-aon.c    |  62 +-
>  .../clk/starfive/clk-starfive-jh7110-isp.c    |  72 +--
>  .../clk/starfive/clk-starfive-jh7110-stg.c    |  94 +--
>  .../clk/starfive/clk-starfive-jh7110-sys.c    | 523 ++++++++--------
>  .../clk/starfive/clk-starfive-jh7110-vout.c   |  74 +--
>  drivers/clk/starfive/clk-starfive-jh7110.h    |   4 +-
>  drivers/clk/starfive/clk-starfive-jh71x0.c    | 327 ----------
>  drivers/clk/starfive/clk-starfive-jh71x0.h    | 123 ----
>  drivers/clk/starfive/jh8100/Makefile          |   7 +
>  drivers/clk/starfive/jh8100/clk-aon.c         | 275 +++++++++
>  .../clk/starfive/jh8100/clk-starfive-jh8100.h |  11 +
>  drivers/clk/starfive/jh8100/clk-sys-ne.c      | 566 ++++++++++++++++++
>  drivers/clk/starfive/jh8100/clk-sys-nw.c      | 268 +++++++++
>  drivers/clk/starfive/jh8100/clk-sys-sw.c      | 136 +++++
>  drivers/clk/starfive/jh8100/clk-sys.c         | 455 ++++++++++++++
>  drivers/reset/starfive/Kconfig                |  14 +-
>  drivers/reset/starfive/Makefile               |   4 +-
>  ...rfive-jh71x0.c => reset-starfive-common.c} |  68 +--
>  .../reset/starfive/reset-starfive-common.h    |  14 +
>  .../reset/starfive/reset-starfive-jh7100.c    |   4 +-
>  .../reset/starfive/reset-starfive-jh7110.c    |   8 +-
>  .../reset/starfive/reset-starfive-jh71x0.h    |  14 -
>  .../reset/starfive/reset-starfive-jh8100.c    | 102 ++++
>  .../dt-bindings/clock/starfive,jh8100-crg.h   | 430 +++++++++++++
>  .../dt-bindings/reset/starfive,jh8100-crg.h   | 127 ++++
>  ...rfive-jh71x0.h => reset-starfive-common.h} |  10 +-
>  40 files changed, 4485 insertions(+), 1242 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-aoncrg.yaml
>  create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-ne.yaml
>  create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-nw.yaml
>  create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-sw.yaml
>  create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg.yaml
>  create mode 100644 arch/riscv/boot/dts/starfive/jh8100-clk.dtsi
>  create mode 100644 drivers/clk/starfive/clk-starfive-common.c
>  create mode 100644 drivers/clk/starfive/clk-starfive-common.h
>  delete mode 100644 drivers/clk/starfive/clk-starfive-jh71x0.c
>  delete mode 100644 drivers/clk/starfive/clk-starfive-jh71x0.h
>  create mode 100644 drivers/clk/starfive/jh8100/Makefile
>  create mode 100644 drivers/clk/starfive/jh8100/clk-aon.c
>  create mode 100644 drivers/clk/starfive/jh8100/clk-starfive-jh8100.h
>  create mode 100644 drivers/clk/starfive/jh8100/clk-sys-ne.c
>  create mode 100644 drivers/clk/starfive/jh8100/clk-sys-nw.c
>  create mode 100644 drivers/clk/starfive/jh8100/clk-sys-sw.c
>  create mode 100644 drivers/clk/starfive/jh8100/clk-sys.c
>  rename drivers/reset/starfive/{reset-starfive-jh71x0.c => reset-starfive-common.c} (55%)
>  create mode 100644 drivers/reset/starfive/reset-starfive-common.h
>  delete mode 100644 drivers/reset/starfive/reset-starfive-jh71x0.h
>  create mode 100644 drivers/reset/starfive/reset-starfive-jh8100.c
>  create mode 100644 include/dt-bindings/clock/starfive,jh8100-crg.h
>  create mode 100644 include/dt-bindings/reset/starfive,jh8100-crg.h
>  rename include/soc/starfive/{reset-starfive-jh71x0.h => reset-starfive-common.h} (50%)
>
> --
> 2.34.1
>
Krzysztof Kozlowski Dec. 8, 2023, 5:52 p.m. UTC | #6
On 06/12/2023 12:49, Sia Jee Heng wrote:
> Add bindings for the System clocks and reset generator
> (SYSCRG) on JH8100 SoC.
> 
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
> ---

...

> +  clocks:
> +    items:
> +      - description: Main Oscillator (24 MHz)
> +      - description: External I2S Rx BCLK clock
> +      - description: External I2S Rx LRCK clock
> +      - description: External MCLK clock
> +
> +  clock-names:
> +    items:
> +      - const: clk_osc
> +      - const: clk_i2srx_bclk_ext
> +      - const: clk_i2srx_lrck_ext
> +      - const: clk_mclk_ext

Drop clk_ prefixes everywhere.

> +
> +  '#clock-cells':
> +    const: 1
> +    description:
> +      See <dt-bindings/clock/starfive,jh8100-crg.h> for valid indices.
> +
> +  '#reset-cells':
> +    const: 1
> +    description:
> +      See <dt-bindings/reset/starfive-jh8100-crg.h> for valid indices.
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/starfive,jh8100-crg.h>
> +
> +    clock-controller@126d0000 {
> +            compatible = "starfive,jh8100-syscrg";

Use 4 spaces for example indentation.

> +            reg = <0x126d0000 0x10000>;
> +            clocks = <&clk_osc>, <&clk_i2srx_bclk_ext>,
> +                     <&clk_i2srx_lrck_ext>, <&clk_mclk_ext>;
> +            clock-names = "clk_osc", "clk_i2srx_bclk_ext",
> +                          "clk_i2srx_lrck_ext", "clk_mclk_ext";
> +            #clock-cells = <1>;
> +            #reset-cells = <1>;
> +    };
> diff --git a/include/dt-bindings/clock/starfive,jh8100-crg.h b/include/dt-bindings/clock/starfive,jh8100-crg.h
> new file mode 100644
> index 000000000000..e5bb588ce798
> --- /dev/null
> +++ b/include/dt-bindings/clock/starfive,jh8100-crg.h
> @@ -0,0 +1,123 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */

How about keeping the same license as binding?

> +/*
> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
> + * Sia Jee Heng <jeeheng.sia@starfivetech.com>
> + *
> + */
> +

...

> +#define SYSCRG_CLK_NNE_ICG_EN						108
> +
> +#define SYSCRG_CLK_END							109

Drop from binding header.

> +#endif /* __DT_BINDINGS_CLOCK_STARFIVE_JH8100_H__ */

...

> + */
> +#define SYSCRG_RSTN_SYS_SYSCON					0
> +#define SYSCRG_RSTN_CLK_MOD					1
> +#define SYSCRG_RSTN_GPU						2
> +#define SYSCRG_RSTN_GPU_SPU					3
> +#define SYSCRG_RSTN_GPU_TVSENSOR				4
> +#define SYSCRG_RSTN_PPU_OP_NORET_GPU_RESET			5
> +#define SYSCRG_RSTN_NNE						6
> +#define SYSCRG_RSTN_HD_AUDIO					7
> +
> +#define SYSCRG_RESET_NR_RESETS					8

Drop from binding header.

> +
> +#endif /* __DT_BINDINGS_RESET_STARFIVE_JH8100_H__ */

Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 8, 2023, 5:53 p.m. UTC | #7
On 06/12/2023 12:49, Sia Jee Heng wrote:
> Add bindings for the System-North-West clock and reset generator
> (SYSCRG-NW) on JH8100 SoC.
> 
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
> ---
>  .../clock/starfive,jh8100-syscrg-nw.yaml      | 119 ++++++++++++++++++
>  .../dt-bindings/clock/starfive,jh8100-crg.h   |  45 +++++++
>  .../dt-bindings/reset/starfive,jh8100-crg.h   |  15 +++
>  3 files changed, 179 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-nw.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-nw.yaml b/Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-nw.yaml
> new file mode 100644
> index 000000000000..b16a874828dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/starfive,jh8100-syscrg-nw.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/starfive,jh8100-syscrg-nw.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH8100 System-North-West Clock and Reset Generator
> +
> +maintainers:
> +  - Sia Jee Heng <jeeheng.sia@starfivetech.com>
> +
> +properties:
> +  compatible:
> +    const: starfive,jh8100-syscrg-nw
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Main Oscillator (24 MHz)
> +      - description: APB_BUS clock from SYSCRG
> +      - description: ISP_2X clock from SYSCRG
> +      - description: ISP_AXI clock from SYSCRG
> +      - description: VOUT_ROOT0 clock from SYSCRG
> +      - description: VOUT_ROOT1 clock from SYSCRG
> +      - description: VOUT_SCAN_ATS clock from SYSCRG
> +      - description: VOUT_DC_CORE clock from SYSCRG
> +      - description: VOUT_AXI clock from SYSCRG
> +      - description: AXI_400 clock from SYSCRG
> +      - description: AXI_200 clock from SYSCRG
> +      - description: Peripheral clock from SYSCRG
> +      - description: External DVP clock
> +      - description: External ISP DPHY TAP TCK clock
> +      - description: External golbal clock
> +      - description: External i2s_tscko clock
> +      - description: External VOUT MIPI DPHY TAP TCK
> +      - description: External VOUT eDP TAP TCK
> +      - description: External SPI In2 clock
> +
> +  clock-names:
> +    items:
> +      - const: clk_osc
> +      - const: sys_clk_apb_bus
> +      - const: sys_clk_isp_2x
> +      - const: sys_clk_isp_axi
> +      - const: sys_clk_vout_root0
> +      - const: sys_clk_vout_root1
> +      - const: sys_clk_vout_scan_ats
> +      - const: sys_clk_vout_dc_core
> +      - const: sys_clk_vout_axi
> +      - const: sys_clk_axi_400
> +      - const: sys_clk_axi_200
> +      - const: sys_clk_perh_root_preosc
> +      - const: clk_dvp_ext
> +      - const: clk_isp_dphy_tap_tck_ext
> +      - const: clk_glb_ext_clk
> +      - const: clk_i2s_tscko
> +      - const: clk_vout_mipi_dphy_tap_tck_ext
> +      - const: clk_vout_edp_tap_tck_ext
> +      - const: clk_spi_in2_ext

Same comments as for other patch.

> +
> +  '#clock-cells':
> +    const: 1
> +    description:
> +      See <dt-bindings/clock/starfive,jh8100-crg.h> for valid indices.
> +
> +  '#reset-cells':
> +    const: 1
> +    description:
> +      See <dt-bindings/reset/starfive,jh8100-crg.h> for valid indices.
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/starfive,jh8100-crg.h>
> +
> +    clock-controller@123c0000 {
> +            compatible = "starfive,jh8100-syscrg-nw";

Same comments as for other patch.


> +            reg = <0x123c0000 0x10000>;
> +            clocks = <&clk_osc>, <&syscrg SYSCRG_CLK_APB_BUS>,
> +                     <&syscrg SYSCRG_CLK_ISP_2X>,
> +                     <&syscrg SYSCRG_CLK_ISP_AXI>,
> +                     <&syscrg SYSCRG_CLK_VOUT_ROOT0>,
> +                     <&syscrg SYSCRG_CLK_VOUT_ROOT1>,
> +                     <&syscrg SYSCRG_CLK_VOUT_SCAN_ATS>,
> +                     <&syscrg SYSCRG_CLK_VOUT_DC_CORE>,
> +                     <&syscrg SYSCRG_CLK_VOUT_AXI>,
> +                     <&syscrg SYSCRG_CLK_AXI_400>,
> +                     <&syscrg SYSCRG_CLK_AXI_200>,
> +                     <&syscrg SYSCRG_CLK_PERH_ROOT_PREOSC>,
> +                     <&clk_dvp_ext>,
> +                     <&clk_isp_dphy_tap_tck_ext>,
> +                     <&clk_glb_ext_clk>,
> +                     <&clk_i2s_tscko>,
> +                     <&clk_vout_mipi_dphy_tap_tck_ext>,
> +                     <&clk_vout_edp_tap_tck_ext>,
> +                     <&clk_spi_in2_ext>;
> +            clock-names = "clk_osc", "sys_clk_apb_bus", "sys_clk_isp_2x",
> +                          "sys_clk_isp_axi", "sys_clk_vout_root0",
> +                          "sys_clk_vout_root1", "sys_clk_vout_scan_ats",
> +                          "sys_clk_vout_dc_core", "sys_clk_vout_axi",
> +                          "sys_clk_axi_400", "sys_clk_axi_200",
> +                          "sys_clk_perh_root_preosc", "clk_dvp_ext",
> +                          "clk_isp_dphy_tap_tck_ext", "clk_glb_ext_clk",
> +                          "clk_i2s_tscko", "clk_vout_mipi_dphy_tap_tck_ext",
> +                          "clk_vout_edp_tap_tck_ext", "clk_spi_in2_ext";
> +            #clock-cells = <1>;
> +            #reset-cells = <1>;
> +    };
> diff --git a/include/dt-bindings/clock/starfive,jh8100-crg.h b/include/dt-bindings/clock/starfive,jh8100-crg.h
> index e5bb588ce798..8417455c2409 100644
> --- a/include/dt-bindings/clock/starfive,jh8100-crg.h
> +++ b/include/dt-bindings/clock/starfive,jh8100-crg.h
> @@ -120,4 +120,49 @@
>  #define SYSCRG_CLK_NNE_ICG_EN						108
>  
>  #define SYSCRG_CLK_END							109
> +
> +/* SYSCRG_NW_CLK */
> +#define SYSCRG_NW_CLK_PLL5_DIV2						0
> +#define SYSCRG_NW_CLK_GCLK5						1
> +#define SYSCRG_NW_CLK_GPIO_100						2
> +#define SYSCRG_NW_CLK_GPIO_50						3
> +#define SYSCRG_NW_CLK_GPIO_150						4
> +#define SYSCRG_NW_CLK_GPIO_60						5
> +#define SYSCRG_NW_CLK_IOMUX_WEST_PCLK					6
> +#define SYSCRG_NW_CLK_I2C6_APB						7
> +#define SYSCRG_NW_CLK_I2C7_APB						8
> +#define SYSCRG_NW_CLK_SPI2_APB						9
> +#define SYSCRG_NW_CLK_SPI2_CORE						10
> +#define SYSCRG_NW_CLK_SPI2_SCLK_IN					11
> +#define SYSCRG_NW_CLK_SMBUS1_APB					12
> +#define SYSCRG_NW_CLK_SMBUS1_CORE					13
> +#define SYSCRG_NW_CLK_ISP_DVP						14
> +#define SYSCRG_NW_CLK_ISP_CORE_2X					15
> +#define SYSCRG_NW_CLK_ISP_AXI						16
> +#define SYSCRG_NW_CLK_ISP_DPHY_TAP_TCK					17
> +#define SYSCRG_NW_CLK_FLEXNOC_ISPSLV					18
> +#define SYSCRG_NW_CLK_VOUT_PIX0						19
> +#define SYSCRG_NW_CLK_VOUT_PIX1						20
> +#define SYSCRG_NW_CLK_VOUT_SCAN_ATS					21
> +#define SYSCRG_NW_CLK_VOUT_DC_CORE					22
> +#define SYSCRG_NW_CLK_VOUT_APB						23
> +#define SYSCRG_NW_CLK_VOUT_DSI						24
> +#define SYSCRG_NW_CLK_VOUT_AHB						25
> +#define SYSCRG_NW_CLK_VOUT_AXI						26
> +#define SYSCRG_NW_CLK_VOUT_MIPI_DPHY_TAP_TCK				27
> +#define SYSCRG_NW_CLK_VOUT_EDP_PHY_TAP_TCK				28
> +#define SYSCRG_NW_CLK_UART5_CORE_PREOSC					29
> +#define SYSCRG_NW_CLK_UART5_APB						30
> +#define SYSCRG_NW_CLK_UART5_CORE					31
> +#define SYSCRG_NW_CLK_UART6_CORE_PREOSC					32
> +#define SYSCRG_NW_CLK_UART6_APB						33
> +#define SYSCRG_NW_CLK_UART6_CORE					34
> +#define SYSCRG_NW_CLK_SPI2_ICG_EN					35
> +#define SYSCRG_NW_CLK_SMBUS1_ICG_EN					36
> +#define SYSCRG_NW_CLK_ISP_ICG_EN					37
> +#define SYSCRG_NW_CLK_VOUT_ICG_EN					38
> +#define SYSCRG_NW_CLK_UART5_ICG_EN					39
> +#define SYSCRG_NW_CLK_UART6_ICG_EN					40
> +
> +#define SYSCRG_NW_CLK_END						41

Same comments as for other patch.


>  #endif /* __DT_BINDINGS_CLOCK_STARFIVE_JH8100_H__ */
> diff --git a/include/dt-bindings/reset/starfive,jh8100-crg.h b/include/dt-bindings/reset/starfive,jh8100-crg.h
> index 3b7b92488e76..8c3a858bdf6a 100644
> --- a/include/dt-bindings/reset/starfive,jh8100-crg.h
> +++ b/include/dt-bindings/reset/starfive,jh8100-crg.h
> @@ -20,4 +20,19 @@
>  
>  #define SYSCRG_RESET_NR_RESETS					8
>  
> +/*
> + * syscrg_nw: assert0
> + */
> +#define SYSCRG_NW_RSTN_PRESETN					0
> +#define SYSCRG_NW_RSTN_SYS_IOMUX_W				1
> +#define SYSCRG_NW_RSTN_I2C6					2
> +#define SYSCRG_NW_RSTN_I2C7					3
> +#define SYSCRG_NW_RSTN_SPI2					4
> +#define SYSCRG_NW_RSTN_SMBUS1					5
> +#define SYSCRG_NW_RSTN_UART5					6
> +#define SYSCRG_NW_RSTN_UART6					7
> +#define SYSCRG_NW_RSTN_MERAK0_TVSENSOR				8
> +#define SYSCRG_NW_RSTN_MERAK1_TVSENSOR				9
> +
> +#define SYSCRG_NW_RESET_NR_RESETS				10

Same comments as for other patch.



Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 8, 2023, 5:54 p.m. UTC | #8
On 06/12/2023 12:49, Sia Jee Heng wrote:
> Add bindings for the System-North-East clock and reset generator
> (SYSCRG-NE) on JH8100 SoC.
> 
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
> ---
>  .../clock/starfive,jh8100-syscrg-ne.yaml      | 158 ++++++++++++++++
>  .../dt-bindings/clock/starfive,jh8100-crg.h   | 179 ++++++++++++++++++
>  .../dt-bindings/reset/starfive,jh8100-crg.h   |  61 ++++++

All my previous comments are applicable.

Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 8, 2023, 5:54 p.m. UTC | #9
On 06/12/2023 12:49, Sia Jee Heng wrote:
> Add bindings for the System-South-West clock and reset generator
> (SYSCRG-SW) on JH8100 SoC.
> 
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
> ---

All my previous comments are applicable.

Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 8, 2023, 5:57 p.m. UTC | #10
On 06/12/2023 12:50, Sia Jee Heng wrote:
> Add SYSCRG/SYSCRG-NE/SYSCRG-NW/SYSCRG-SW/AONCRG clock and reset
> nodes for JH8100 RISC-V SoC.
> 
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>

Really? Looks automated... Care to provide any links to effects of
internal review?

> ---
>  arch/riscv/boot/dts/starfive/jh8100-clk.dtsi | 180 +++++++++++++++++++
>  arch/riscv/boot/dts/starfive/jh8100.dtsi     | 115 ++++++++++++
>  2 files changed, 295 insertions(+)
>  create mode 100644 arch/riscv/boot/dts/starfive/jh8100-clk.dtsi
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh8100-clk.dtsi b/arch/riscv/boot/dts/starfive/jh8100-clk.dtsi
> new file mode 100644
> index 000000000000..27ba249f523e
> --- /dev/null
> +++ b/arch/riscv/boot/dts/starfive/jh8100-clk.dtsi
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
> + */
> +
> +/ {
> +	clk_osc: clk_osc {

No underscores in node names.

> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <24000000>;
> +	};
> +

...

> diff --git a/arch/riscv/boot/dts/starfive/jh8100.dtsi b/arch/riscv/boot/dts/starfive/jh8100.dtsi
> index f26aff5c1ddf..9863c61324a0 100644
> --- a/arch/riscv/boot/dts/starfive/jh8100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh8100.dtsi
> @@ -4,6 +4,9 @@
>   */
>  
>  /dts-v1/;
> +#include <dt-bindings/clock/starfive,jh8100-crg.h>
> +#include <dt-bindings/reset/starfive,jh8100-crg.h>
> +#include "jh8100-clk.dtsi"
>  
>  / {
>  	compatible = "starfive,jh8100";
> @@ -357,6 +360,104 @@ uart4: serial@121a0000  {
>  			status = "disabled";
>  		};
>  
> +		syscrg_ne: syscrg_ne@12320000 {

clock-controller@

Just open your bindings and take a look how it is done there...

This applies everywhere

Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 8, 2023, 5:57 p.m. UTC | #11
On 08/12/2023 17:39, Emil Renner Berthing wrote:
> Sia Jee Heng wrote:
>> Add SYSCRG/SYSCRG-NE/SYSCRG-NW/SYSCRG-SW/AONCRG clock and reset
>> nodes for JH8100 RISC-V SoC.
>>
>> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
>> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
>> ---
>>  arch/riscv/boot/dts/starfive/jh8100-clk.dtsi | 180 +++++++++++++++++++
>>  arch/riscv/boot/dts/starfive/jh8100.dtsi     | 115 ++++++++++++
> 
> Why the split here? I mean why can't the clocks just be in the jh8100.dtsi?

There should be. What's the point? Clocks are internal part of SoC and
not really re-usable piece of hardware.

Best regards,
Krzysztof