| 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: <d2614363-1dab-4ea3-b79a-5d3c02c4bc17@riscstar.com>
Date: Thu, 8 May 2025 06:55:17 -0500
From: Alex Elder <elder@...cstar.com>
To: Haylen Chu <heylenay@....org>, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, mturquette@...libre.com, sboyd@...nel.org,
p.zabel@...gutronix.de, paul.walmsley@...ive.com, palmer@...belt.com,
aou@...s.berkeley.edu, alex@...ti.fr, dlan@...too.org
Cc: inochiama@...look.com, guodong@...cstar.com, devicetree@...r.kernel.org,
linux-clk@...r.kernel.org, spacemit@...ts.linux.dev,
linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 4/6] reset: spacemit: add support for SpacemiT CCU
resets
On 5/8/25 12:38 AM, Haylen Chu wrote:
> 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@...cstar.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,
Wow, I really should have made this v1 of a new series. You point
out several problems that came out of this using the auxiliary
device framework, which I should have tested more thoroughly.
Yes I will update this to select that, and will test it before
my next version.
>
> 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.
I'm not sure I understand this statement, at least in this
context. This pattern is used to define a single module
"reset_spacemit.o" out of several source files.
But I think you're saying that RESET_SPACEMIT and
RESET_SPACEMIT_K1 should both be bool, or both be
tristate. I will resolve that question before I
send the next version.
>> +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,
OK, thank you.
-Alex
>
> 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
Powered by blists - more mailing lists