| 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: <CAH1PCMYG6DEj-oq_S8GNddF0m=tt+h313i9HXjFi7DkeRc7DiA@mail.gmail.com>
Date: Wed, 31 Dec 2025 17:36:35 +0800
From: Guodong Xu <guodong@...cstar.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
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
Hi Krzysztof,
On Tue, Dec 30, 2025 at 8:23 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> 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?
You're absolutely right, good catch & thank you!
I'll fix it in the v2 version with:
rcdev->owner = dev->driver->owner;
Best regards,
Guodong Xu
>
> This feels buggy, assuming this is a module and your
> MODULE_DESCRIPTION() clearly suggests that.
>
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists