Re: [PATCH v6 4/6] reset: spacemit: add support for SpacemiT CCU resets

Haylen Chu
2025-05-08T05:38:54Z

On Tue, May 06, 2025 at 04:06:35PM -0500, Alex Elder wrote:
> Implement reset support for SpacemiT CCUs. The code is structured to
> handle SpacemiT resets generically, while defining the set of specific
> reset controllers and their resets in an SoC-specific source file. A
> SpacemiT reset controller device is an auxiliary device associated with
> a clock controller (CCU).
>
> This initial patch defines the reset controllers for the MPMU, APBC, and
> MPMU CCUs, which already defined clock controllers.
>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
> drivers/reset/Kconfig | 1 +
> drivers/reset/Makefile | 1 +
> drivers/reset/spacemit/Kconfig | 12 +++
> drivers/reset/spacemit/Makefile | 7 ++
> drivers/reset/spacemit/core.c | 61 +++++++++++
> drivers/reset/spacemit/core.h | 39 +++++++
> drivers/reset/spacemit/k1.c | 177 ++++++++++++++++++++++++++++++++
> 7 files changed, 298 insertions(+)
> create mode 100644 drivers/reset/spacemit/Kconfig
> create mode 100644 drivers/reset/spacemit/Makefile
> create mode 100644 drivers/reset/spacemit/core.c
> create mode 100644 drivers/reset/spacemit/core.h
> create mode 100644 drivers/reset/spacemit/k1.c
>

...

> diff --git a/drivers/reset/spacemit/Kconfig b/drivers/reset/spacemit/Kconfig
> new file mode 100644
> index 0000000000000..4ff3487a99eff
> --- /dev/null
> +++ b/drivers/reset/spacemit/Kconfig
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +config RESET_SPACEMIT
> + bool
> +
> +config RESET_SPACEMIT_K1
> + tristate "SpacemiT K1 reset driver"
> + depends on ARCH_SPACEMIT || COMPILE_TEST
> + select RESET_SPACEMIT
> + default ARCH_SPACEMIT
> + help
> + This enables the reset controller driver for the SpacemiT K1 SoC.

With auxiliary bus introduced, Kconfig entries for both the reset and
clock should select AUXILIARY_BUS, or building defconfig will fail with
undefined references,

riscv64-unknown-linux-musl-ld: drivers/clk/spacemit/ccu-k1.o: in function `k1_ccu_probe':
ccu-k1.c:(.text+0x19c): undefined reference to `auxiliary_device_init'
riscv64-unknown-linux-musl-ld: ccu-k1.c:(.text+0x226): undefined reference to `__auxiliary_device_add'
riscv64-unknown-linux-musl-ld: drivers/reset/spacemit/k1.o: in function `spacemit_k1_reset_driver_init':
k1.c:(.init.text+0x1a): undefined reference to `__auxiliary_driver_register'
riscv64-unknown-linux-musl-ld: drivers/reset/spacemit/k1.o: in function `spacemit_k1_reset_driver_exit':
k1.c:(.exit.text+0x10): undefined reference to `auxiliary_driver_unregister'

> diff --git a/drivers/reset/spacemit/Makefile b/drivers/reset/spacemit/Makefile
> new file mode 100644
> index 0000000000000..3a064e9d5d6b4
> --- /dev/null
> +++ b/drivers/reset/spacemit/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_RESET_SPACEMIT) += reset_spacemit.o

As RESET_SPACEMIT is defined as bool, the reset driver will never be
compiled as a module... so either the RESET_SPACEMIT_K1 should be
limited to bool as well or you could take an approach similar to the
clock driver.

> +reset_spacemit-y := core.o
> +
> +reset_spacemit-$(CONFIG_RESET_SPACEMIT_K1) += k1.o

...

> new file mode 100644
> index 0000000000000..19a34f151b214
> --- /dev/null
> +++ b/drivers/reset/spacemit/k1.c

...

> +MODULE_DEVICE_TABLE(auxiliary, spacemit_k1_reset_ids);
> +
> +#undef K1_AUX_DEV_ID
> +
> +static struct auxiliary_driver spacemit_k1_reset_driver = {
> + .probe = spacemit_k1_reset_probe,
> + .id_table = spacemit_k1_reset_ids,
> +};
> +module_auxiliary_driver(spacemit_k1_reset_driver);
> --
> 2.45.2

If you're willing to make the reset driver buildable as a module, please
add MODULE_{LICENSE,DESCRIPTION} statements and possibly also
MODULE_AUTHOR(), or modpost will complain,

ERROR: modpost: missing MODULE_LICENSE() in drivers/reset/spacemit/reset_spacemit.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/reset/spacemit/reset_spacemit.o

Best regards,
Haylen Chu