linux-kernel - Re: [PATCH 3/4] reset: spacemit: Extract common K1 reset code

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251230-dashing-black-skink-45e5a6@quoll>
Date: Tue, 30 Dec 2025 13:23:16 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Guodong Xu <guodong@...cstar.com>
Cc: 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>, 
	Alex Elder <elder@...cstar.com>, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev
Subject: Re: [PATCH 3/4] reset: spacemit: Extract common K1 reset code

On Mon, Dec 29, 2025 at 07:04:06PM +0800, Guodong Xu wrote:
> Extract the common reset controller code from the K1 driver into
> separate reset-spacemit-common.{c,h} files. This prepares for
> additional SpacemiT SoCs that share the same reset controller
> architecture.
> 
> The common code now includes handlers for reset assert
> deassert operations and probing for auxiliary bus devices.
> 
> Signed-off-by: Guodong Xu <guodong@...cstar.com>
> ---
>  drivers/reset/spacemit/Kconfig                 |  17 +++-
>  drivers/reset/spacemit/Makefile                |   2 +
>  drivers/reset/spacemit/reset-spacemit-common.c |  79 +++++++++++++++++
>  drivers/reset/spacemit/reset-spacemit-common.h |  53 ++++++++++++
>  drivers/reset/spacemit/reset-spacemit-k1.c     | 113 +++----------------------
>  5 files changed, 158 insertions(+), 106 deletions(-)
> 
> diff --git a/drivers/reset/spacemit/Kconfig b/drivers/reset/spacemit/Kconfig
> index 552884e8b72a..56a4858b30e1 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 de7e358c74fd..fecda9f211b2 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 000000000000..e4b3f0e2c59d
> --- /dev/null
> +++ b/drivers/reset/spacemit/reset-spacemit-common.c
> @@ -0,0 +1,79 @@
> +// 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);
> +}
> +
> +const struct reset_control_ops spacemit_reset_control_ops = {
> +	.assert		= spacemit_reset_assert,
> +	.deassert	= spacemit_reset_deassert,
> +};
> +EXPORT_SYMBOL_GPL(spacemit_reset_control_ops);
> +
> +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;

So who is now the owner? This module or actual driver calling and using
this?

This feels buggy, assuming this is a module and your
MODULE_DESCRIPTION() clearly suggests that.


Best regards,
Krzysztof