| 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:
<MA0P287MB226255035355E9F97F0A9486FEBA2@MA0P287MB2262.INDP287.PROD.OUTLOOK.COM>
Date: Wed, 23 Apr 2025 13:55:12 +0800
From: Chen Wang <unicorn_wang@...look.com>
To: Junhui Liu <junhui.liu@...moral.tech>,
Philipp Zabel <p.zabel@...gutronix.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v2 2/2] reset: canaan: add reset driver for Kendryte K230
On 2025/4/23 13:36, Junhui Liu wrote:
> Hi Chen,
>
> Thanks for your review.
>
> On 23/04/2025 11:21, Chen Wang wrote:
>> Hi, Junhui,
>>
>>
>> On 2025/4/20 1:09, Junhui Liu wrote:
>>> Add support for the resets on Canaan Kendryte K230 SoC. The driver
>>> support CPU0, CPU1, L2 cache flush, hardware auto clear and software
>>> clear resets.
>>>
>>> Signed-off-by: Junhui Liu <junhui.liu@...moral.tech>
>>>
>>> ---
>>> The reset management module in the K230 SoC also provides reset time
>>> control registers. For RST_TYPE_CPU0, RST_TYPE_CPU1 and RST_TYPE_SW_DONE,
>>> the time period when reset is applyed/removed but the clock is stopped
>>> can be set up to 15*0.25 = 3.75 us. For some RST_TYPE_HW_DONE cases, the
>>> time period can be set up to 255*0.25 = 63.75 us. For RST_TYPE_FLUSH,
>>> the reset bit will automatically cleared by hardware when flush done.
>>>
>>> Although the current reset driver does not support configuration of
>>> reset time registers, delay has been added to the assert, deassert and
>>> reset functions to accommodate the longest reset time.
>>>
>>> Besides, although some reset types have done bits, the reference manual
>>> does not explicitly indicate whether the hardware removes reset or the
>>> clock stop time period has passed when done bits toggle. Therefore, I
>>> think it is a safer way to keep delay for reset types with done bits.
>>>
>>> link: https://kendryte-download.canaan-creative.com/developer/k230/HDK/K230%E7%A1%AC%E4%BB%B6%E6%96%87%E6%A1%A3/K230_Technical_Reference_Manual_V0.3.1_20241118.pdf
>> Regards these notes, please move it to the beginning of source code, see
>> example: drivers/pwm/pwm-sophgo-sg2042.c
>>> ---
>>> drivers/reset/Kconfig | 9 ++
>>> drivers/reset/Makefile | 1 +
>>> drivers/reset/reset-k230.c | 355 +++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 365 insertions(+)
>>>
>>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>>> index 99f6f9784e6865faddf8621ccfca095778c4dc47..248138ffba3bfbf859c74ba1aed7ba2f72819f7a 100644
>>> --- a/drivers/reset/Kconfig
>>> +++ b/drivers/reset/Kconfig
>>> @@ -140,6 +140,15 @@ config RESET_K210
>>> Say Y if you want to control reset signals provided by this
>>> controller.
>>>
>>> +config RESET_K230
>>> + tristate "Reset controller driver for Canaan Kendryte K230 SoC"
>>> + depends on ARCH_CANAAN || COMPILE_TEST
>>> + depends on OF
>>> + help
>>> + Support for the Canaan Kendryte K230 RISC-V SoC reset controller.
>>> + Say Y if you want to control reset signals provided by this
>>> + controller.
>>> +
>>> config RESET_LANTIQ
>>> bool "Lantiq XWAY Reset Driver" if COMPILE_TEST
>>> default SOC_TYPE_XWAY
>>> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
>>> index 31f9904d13f9c3a107fc1ee1ec9f9baba016d101..13fe94531bea1eb91268b1804e1321b167815a4b 100644
>>> --- a/drivers/reset/Makefile
>>> +++ b/drivers/reset/Makefile
>>> @@ -20,6 +20,7 @@ obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
>>> obj-$(CONFIG_RESET_IMX8MP_AUDIOMIX) += reset-imx8mp-audiomix.o
>>> obj-$(CONFIG_RESET_INTEL_GW) += reset-intel-gw.o
>>> obj-$(CONFIG_RESET_K210) += reset-k210.o
>>> +obj-$(CONFIG_RESET_K230) += reset-k230.o
>>> obj-$(CONFIG_RESET_LANTIQ) += reset-lantiq.o
>>> obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
>>> obj-$(CONFIG_RESET_MCHP_SPARX5) += reset-microchip-sparx5.o
>>> diff --git a/drivers/reset/reset-k230.c b/drivers/reset/reset-k230.c
>>> new file mode 100644
>>> index 0000000000000000000000000000000000000000..492d2274893675b0ff1967426c8fa9e75aed1791
>>> --- /dev/null
>>> +++ b/drivers/reset/reset-k230.c
>>> @@ -0,0 +1,355 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright (C) 2016-2017 Linaro Ltd.
>> Linaro? Are you sure?
> Actually, I kept this from vendor's code.
>
> link: https://github.com/kendryte/k230_sdk/blob/v1.9/src/little/linux/drivers/reset/canaan/reset-k230.c#L19
No reason to keep this, I suggest dropping this line.
>
>>> + * Copyright (C) 2022-2024 Canaan Bright Sight Co., Ltd
>>> + * Copyright (C) 2024-2025 Junhui Liu <junhui.liu@...moral.tech>
>>> + */
>>> +
>>> +#include <linux/cleanup.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/io.h>
>>> +#include <linux/iopoll.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/reset-controller.h>
>>> +#include <linux/spinlock.h>
>>> +
>>> +#include <dt-bindings/reset/canaan,k230-rst.h>
>>> +
>>> +/**
>>> + * enum k230_rst_type - K230 reset types
>>> + * @RST_TYPE_CPU0: Reset type for CPU0
>>> + * Automatically clears, has write enable and done bit, active high
>>> + * @RST_TYPE_CPU1: Reset type for CPU1
>>> + * Manually clears, has write enable and done bit, active high
>>> + * @RST_TYPE_FLUSH: Reset type for CPU L2 cache flush
>>> + * Automatically clears, has write enable, no done bit, active high
>>> + * @RST_TYPE_HW_DONE: Reset type for hardware auto clear
>>> + * Automatically clears, no write enable, has done bit, active high
>>> + * @RST_TYPE_SW_DONE: Reset type for software manual clear
>>> + * Manually clears, no write enable and done bit,
>>> + * active high if ID is RST_SPI2AXI, otherwise active low
>>> + */
>>> +enum k230_rst_type {
>>> + RST_TYPE_CPU0,
>>> + RST_TYPE_CPU1,
>>> + RST_TYPE_FLUSH,
>>> + RST_TYPE_HW_DONE,
>>> + RST_TYPE_SW_DONE,
>>> +};
>>> +
>>> +struct k230_rst_map {
>>> + u32 offset;
>>> + enum k230_rst_type type;
>>> + u32 done;
>>> + u32 reset;
>>> +};
>>> +
>>> +struct k230_rst {
>>> + struct reset_controller_dev rcdev;
>>> + struct device *dev;
>> Why don't usercdev.dev to store the dev, so we can save a pointer.
> Thanks for your reminder, I will store it in rcdev.dev.
>
>>> + void __iomem *base;
>>> + spinlock_t lock;
>>> +};
>> [......]
>>
>> Thanks,
>>
>> Chen
Powered by blists - more mailing lists