Quoting Ryan Chen (2024-08-27 23:27:40) > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 983ef4f36d8c..855b65f2d6dd 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -269,6 +269,16 @@ config COMMON_CLK_ASPEED > The G4 and G5 series, including the ast2400 and ast2500, are supported > by this driver. > > +config COMMON_CLK_AST2700 > + bool "Clock driver for AST2700 SoC" > + depends on ARCH_ASPEED || COMPILE_TEST > + select MFD_SYSCON Why is this a syscon? > + select RESET_CONTROLLER > + help > + This driver provides support for clock on AST2700 SoC. > + This driver is responsible for managing the various clocks required > + by the peripherals and cores within the AST2700. > + > config COMMON_CLK_S2MPS11 > tristate "Clock driver for S2MPS1X/S5M8767 MFD" > depends on MFD_SEC_CORE || COMPILE_TEST > diff --git a/drivers/clk/clk-ast2700.c b/drivers/clk/clk-ast2700.c > new file mode 100644 > index 000000000000..7e0466e73980 > --- /dev/null > +++ b/drivers/clk/clk-ast2700.c > @@ -0,0 +1,1198 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2024 ASPEED Technology Inc. > + * Author: Ryan Chen <ryan_chen@xxxxxxxxxxxxxx> > + */ > + > +#include <linux/bits.h> > +#include <linux/clk-provider.h> [...] > + > +struct ast2700_reset { > + void __iomem *base; > + struct ast2700_reset_signal const *signal; > + struct reset_controller_dev rcdev; > +}; Please move the reset controller to the drivers/reset directory by means of using an auxiliary device. There are some existing examples in there if you grep for auxiliary_device in drivers/reset to help guide. > + > +#define to_rc_data(p) container_of(p, struct ast2700_reset, rcdev) > + [...] > + > +static int ast2700_soc0_clk_init(struct device_node *soc0_node) > +{ > + struct clk_hw_onecell_data *clk_data; > + void __iomem *clk_base; [...] > + 0, clk_base + SCU0_CLK_STOP, > + 28, 0, &ast2700_clk_lock); > + > + of_clk_add_hw_provider(soc0_node, of_clk_hw_onecell_get, clk_data); > + > + return 0; > +}; > + > +CLK_OF_DECLARE_DRIVER(ast2700_soc0, "aspeed,ast2700-scu0", ast2700_soc0_clk_init); > +CLK_OF_DECLARE_DRIVER(ast2700_soc1, "aspeed,ast2700-scu1", ast2700_soc1_clk_init); Why can't this be a platform driver?