| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <80161b42-c665-4fe8-bc7c-845ca44bc723@riscstar.com>
Date: Thu, 15 Jan 2026 11:27:28 -0600
From: Alex Elder <elder@...cstar.com>
To: Guodong Xu <guodong@...cstar.com>, Philipp Zabel
<p.zabel@...gutronix.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Yixun Lan <dlan@...too.org>,
Haylen Chu <heylenay@....org>
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev
Subject: Re: [PATCH v2 3/4] reset: spacemit: Extract common K1 reset code
On 1/8/26 8:22 AM, Guodong Xu wrote:
> Extract the common reset controller code from the K1 driver into
> separate reset-spacemit-common.{c,h} files to prepare for additional
> SpacemiT SoCs that share the same reset controller architecture.
>
> The common code includes handlers for reset assert and deassert
> operations and probing for auxiliary bus devices.
>
> Changes during extraction:
> - Module ownership: Use dev->driver->owner instead of THIS_MODULE in
> spacemit_reset_controller_register() to correctly reference the
> calling driver's module.
> - Rename spacemit_reset_ids to spacemit_k1_reset_ids.
> - Define new namespace "RESET_SPACEMIT" for the exported common
> functions (spacemit_reset_probe) and update K1 driver to import it.
>
> This prepares for additional SpacemiT SoCs (K3) that share the same reset
> controller architecture.
>
> Signed-off-by: Guodong Xu <guodong@...cstar.com>
This looks good to me.
Reviewed-by: Alex Elder <elder@...cstar.com>
> ---
> v2: Use dev->driver->owner for the reset controller owner instead of
> THIS_MODULE to fix the module reference counting issue pointed out
> by Krzysztof Kozlowski.
> ---
> drivers/reset/spacemit/Kconfig | 17 +++-
> drivers/reset/spacemit/Makefile | 2 +
> drivers/reset/spacemit/reset-spacemit-common.c | 77 ++++++++++++++++++
> drivers/reset/spacemit/reset-spacemit-common.h | 42 ++++++++++
> drivers/reset/spacemit/reset-spacemit-k1.c | 107 +++----------------------
> 5 files changed, 144 insertions(+), 101 deletions(-)
>
> diff --git a/drivers/reset/spacemit/Kconfig b/drivers/reset/spacemit/Kconfig
> index 552884e8b72afeb05cdb9b6565ad8e7fd32f990b..56a4858b30e136296e1f9c85b7b13ccee91fe7b4 100644
> --- a/drivers/reset/spacemit/Kconfig
> +++ b/drivers/reset/spacemit/Kconfig
> @@ -1,10 +1,20 @@
> # SPDX-License-Identifier: GPL-2.0-only
>
> -config RESET_SPACEMIT_K1
> - tristate "SpacemiT K1 reset driver"
> +menu "Reset support for SpacemiT platforms"
> depends on ARCH_SPACEMIT || COMPILE_TEST
> - depends on SPACEMIT_K1_CCU
> +
> +config RESET_SPACEMIT_COMMON
> + tristate
> select AUXILIARY_BUS
> + help
> + Common reset controller infrastructure for SpacemiT SoCs.
> + This provides shared code and helper functions used by
> + reset drivers for various SpacemiT SoC families.
> +
> +config RESET_SPACEMIT_K1
> + tristate "Support for SpacemiT K1 SoC"
> + depends on SPACEMIT_K1_CCU
> + select RESET_SPACEMIT_COMMON
> default SPACEMIT_K1_CCU
> help
> Support for reset controller in SpacemiT K1 SoC.
> @@ -12,3 +22,4 @@ config RESET_SPACEMIT_K1
> unit (CCU) driver to provide reset control functionality
> for various peripherals and subsystems in the SoC.
>
> +endmenu
> diff --git a/drivers/reset/spacemit/Makefile b/drivers/reset/spacemit/Makefile
> index de7e358c74fd7b0fac3ec2c18d985331af64fcbb..fecda9f211b24a54707b3f425b9325be1f2f7738 100644
> --- a/drivers/reset/spacemit/Makefile
> +++ b/drivers/reset/spacemit/Makefile
> @@ -1,3 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_RESET_SPACEMIT_COMMON) += reset-spacemit-common.o
> +
> obj-$(CONFIG_RESET_SPACEMIT_K1) += reset-spacemit-k1.o
>
> diff --git a/drivers/reset/spacemit/reset-spacemit-common.c b/drivers/reset/spacemit/reset-spacemit-common.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..0626633a5e7db6e31be4ed36505b15291eb186b1
> --- /dev/null
> +++ b/drivers/reset/spacemit/reset-spacemit-common.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/* SpacemiT reset controller driver - common implementation */
> +
> +#include <linux/container_of.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +
> +#include <soc/spacemit/ccu.h>
> +
> +#include "reset-spacemit-common.h"
> +
> +static int spacemit_reset_update(struct reset_controller_dev *rcdev,
> + unsigned long id, bool assert)
> +{
> + struct ccu_reset_controller *controller;
> + const struct ccu_reset_data *data;
> + u32 mask;
> + u32 val;
> +
> + controller = container_of(rcdev, struct ccu_reset_controller, rcdev);
> + data = &controller->data->reset_data[id];
> + mask = data->assert_mask | data->deassert_mask;
> + val = assert ? data->assert_mask : data->deassert_mask;
> +
> + return regmap_update_bits(controller->regmap, data->offset, mask, val);
> +}
> +
> +static int spacemit_reset_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return spacemit_reset_update(rcdev, id, true);
> +}
> +
> +static int spacemit_reset_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return spacemit_reset_update(rcdev, id, false);
> +}
> +
> +static const struct reset_control_ops spacemit_reset_control_ops = {
> + .assert = spacemit_reset_assert,
> + .deassert = spacemit_reset_deassert,
> +};
> +
> +static int spacemit_reset_controller_register(struct device *dev,
> + struct ccu_reset_controller *controller)
> +{
> + struct reset_controller_dev *rcdev = &controller->rcdev;
> +
> + rcdev->ops = &spacemit_reset_control_ops;
> + rcdev->owner = dev->driver->owner;
> + rcdev->of_node = dev->of_node;
> + rcdev->nr_resets = controller->data->count;
> +
> + return devm_reset_controller_register(dev, &controller->rcdev);
> +}
> +
> +int spacemit_reset_probe(struct auxiliary_device *adev,
> + const struct auxiliary_device_id *id)
> +{
> + struct spacemit_ccu_adev *rdev = to_spacemit_ccu_adev(adev);
> + struct ccu_reset_controller *controller;
> + struct device *dev = &adev->dev;
> +
> + controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
> + if (!controller)
> + return -ENOMEM;
> + controller->data = (const struct ccu_reset_controller_data *)id->driver_data;
> + controller->regmap = rdev->regmap;
> +
> + return spacemit_reset_controller_register(dev, controller);
> +}
> +EXPORT_SYMBOL_NS_GPL(spacemit_reset_probe, "RESET_SPACEMIT");
> +
> +MODULE_DESCRIPTION("SpacemiT reset controller driver - common code");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/reset/spacemit/reset-spacemit-common.h b/drivers/reset/spacemit/reset-spacemit-common.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..ffaf2f86eb39df72b079095b3f5da3622e132c8d
> --- /dev/null
> +++ b/drivers/reset/spacemit/reset-spacemit-common.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * SpacemiT reset controller driver - common definitions
> + */
> +
> +#ifndef _RESET_SPACEMIT_COMMON_H_
> +#define _RESET_SPACEMIT_COMMON_H_
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +#include <linux/types.h>
> +
> +struct ccu_reset_data {
> + u32 offset;
> + u32 assert_mask;
> + u32 deassert_mask;
> +};
> +
> +struct ccu_reset_controller_data {
> + const struct ccu_reset_data *reset_data; /* array */
> + size_t count;
> +};
> +
> +struct ccu_reset_controller {
> + struct reset_controller_dev rcdev;
> + const struct ccu_reset_controller_data *data;
> + struct regmap *regmap;
> +};
> +
> +#define RESET_DATA(_offset, _assert_mask, _deassert_mask) \
> + { \
> + .offset = (_offset), \
> + .assert_mask = (_assert_mask), \
> + .deassert_mask = (_deassert_mask), \
> + }
> +
> +/* Common probe function */
> +int spacemit_reset_probe(struct auxiliary_device *adev,
> + const struct auxiliary_device_id *id);
> +
> +#endif /* _RESET_SPACEMIT_COMMON_H_ */
> diff --git a/drivers/reset/spacemit/reset-spacemit-k1.c b/drivers/reset/spacemit/reset-spacemit-k1.c
> index cc7fd1f8750d422f3d8f96367ae259f38418c44b..8f3b5329ea5f627ab454e15bf485b155c9f48ca5 100644
> --- a/drivers/reset/spacemit/reset-spacemit-k1.c
> +++ b/drivers/reset/spacemit/reset-spacemit-k1.c
> @@ -1,41 +1,13 @@
> // SPDX-License-Identifier: GPL-2.0-only
>
> -/* SpacemiT reset controller driver */
> +/* SpacemiT K1 reset controller driver */
>
> -#include <linux/auxiliary_bus.h>
> -#include <linux/container_of.h>
> -#include <linux/device.h>
> #include <linux/module.h>
> -#include <linux/regmap.h>
> -#include <linux/reset-controller.h>
> -#include <linux/types.h>
>
> -#include <soc/spacemit/k1-syscon.h>
> #include <dt-bindings/clock/spacemit,k1-syscon.h>
> +#include <soc/spacemit/k1-syscon.h>
>
> -struct ccu_reset_data {
> - u32 offset;
> - u32 assert_mask;
> - u32 deassert_mask;
> -};
> -
> -struct ccu_reset_controller_data {
> - const struct ccu_reset_data *reset_data; /* array */
> - size_t count;
> -};
> -
> -struct ccu_reset_controller {
> - struct reset_controller_dev rcdev;
> - const struct ccu_reset_controller_data *data;
> - struct regmap *regmap;
> -};
> -
> -#define RESET_DATA(_offset, _assert_mask, _deassert_mask) \
> - { \
> - .offset = (_offset), \
> - .assert_mask = (_assert_mask), \
> - .deassert_mask = (_deassert_mask), \
> - }
> +#include "reset-spacemit-common.h"
>
> static const struct ccu_reset_data k1_mpmu_resets[] = {
> [RESET_WDT] = RESET_DATA(MPMU_WDTPCR, BIT(2), 0),
> @@ -214,91 +186,30 @@ static const struct ccu_reset_controller_data k1_apbc2_reset_data = {
> .count = ARRAY_SIZE(k1_apbc2_resets),
> };
>
> -static int spacemit_reset_update(struct reset_controller_dev *rcdev,
> - unsigned long id, bool assert)
> -{
> - struct ccu_reset_controller *controller;
> - const struct ccu_reset_data *data;
> - u32 mask;
> - u32 val;
> -
> - controller = container_of(rcdev, struct ccu_reset_controller, rcdev);
> - data = &controller->data->reset_data[id];
> - mask = data->assert_mask | data->deassert_mask;
> - val = assert ? data->assert_mask : data->deassert_mask;
> -
> - return regmap_update_bits(controller->regmap, data->offset, mask, val);
> -}
> -
> -static int spacemit_reset_assert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - return spacemit_reset_update(rcdev, id, true);
> -}
> -
> -static int spacemit_reset_deassert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - return spacemit_reset_update(rcdev, id, false);
> -}
> -
> -static const struct reset_control_ops spacemit_reset_control_ops = {
> - .assert = spacemit_reset_assert,
> - .deassert = spacemit_reset_deassert,
> -};
> -
> -static int spacemit_reset_controller_register(struct device *dev,
> - struct ccu_reset_controller *controller)
> -{
> - struct reset_controller_dev *rcdev = &controller->rcdev;
> -
> - rcdev->ops = &spacemit_reset_control_ops;
> - rcdev->owner = THIS_MODULE;
> - rcdev->of_node = dev->of_node;
> - rcdev->nr_resets = controller->data->count;
> -
> - return devm_reset_controller_register(dev, &controller->rcdev);
> -}
> -
> -static int spacemit_reset_probe(struct auxiliary_device *adev,
> - const struct auxiliary_device_id *id)
> -{
> - struct spacemit_ccu_adev *rdev = to_spacemit_ccu_adev(adev);
> - struct ccu_reset_controller *controller;
> - struct device *dev = &adev->dev;
> -
> - controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
> - if (!controller)
> - return -ENOMEM;
> - controller->data = (const struct ccu_reset_controller_data *)id->driver_data;
> - controller->regmap = rdev->regmap;
> -
> - return spacemit_reset_controller_register(dev, controller);
> -}
> -
> #define K1_AUX_DEV_ID(_unit) \
> { \
> .name = "spacemit_ccu.k1-" #_unit "-reset", \
> .driver_data = (kernel_ulong_t)&k1_ ## _unit ## _reset_data, \
> }
>
> -static const struct auxiliary_device_id spacemit_reset_ids[] = {
> +static const struct auxiliary_device_id spacemit_k1_reset_ids[] = {
> K1_AUX_DEV_ID(mpmu),
> K1_AUX_DEV_ID(apbc),
> K1_AUX_DEV_ID(apmu),
> K1_AUX_DEV_ID(rcpu),
> K1_AUX_DEV_ID(rcpu2),
> K1_AUX_DEV_ID(apbc2),
> - { },
> + { /* sentinel */ }
> };
> -MODULE_DEVICE_TABLE(auxiliary, spacemit_reset_ids);
> +MODULE_DEVICE_TABLE(auxiliary, spacemit_k1_reset_ids);
>
> static struct auxiliary_driver spacemit_k1_reset_driver = {
> .probe = spacemit_reset_probe,
> - .id_table = spacemit_reset_ids,
> + .id_table = spacemit_k1_reset_ids,
> };
> module_auxiliary_driver(spacemit_k1_reset_driver);
>
> +MODULE_IMPORT_NS("RESET_SPACEMIT");
> MODULE_AUTHOR("Alex Elder <elder@...nel.org>");
> -MODULE_DESCRIPTION("SpacemiT reset controller driver");
> +MODULE_DESCRIPTION("SpacemiT K1 reset controller driver");
> MODULE_LICENSE("GPL");
>
Powered by blists - more mailing lists