| 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: <183930b36feb1b40.9b4fd51a8b5c6c1d.5a72bde348d1c9e9@Jude-Air.local>
Date: Thu, 24 Apr 2025 07:31:17 +0000
From: "Junhui Liu" <junhui.liu@...moral.tech>
To: "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
Hi Philipp,
Thanks for your review.
On 23/04/2025 11:43, Philipp Zabel wrote:
> On So, 2025-04-20 at 01:09 +0800, 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
>> ---
>> 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.
>> + * 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;
>> + void __iomem *base;
>> + spinlock_t lock;
>
> Add a comment, this locks register read-modify-write.
>
>> +};
>> +
>> +static const struct k230_rst_map k230_resets[] = {
>> + [RST_CPU0] = { 0x4, RST_TYPE_CPU0, BIT(12), BIT(0) },
>> + [RST_CPU1] = { 0xc, RST_TYPE_CPU1, BIT(12), BIT(0) },
>> + [RST_CPU0_FLUSH] = { 0x4, RST_TYPE_FLUSH, 0, BIT(4) },
>> + [RST_CPU1_FLUSH] = { 0xc, RST_TYPE_FLUSH, 0, BIT(4) },
>> + [RST_AI] = { 0x14, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
>> + [RST_VPU] = { 0x1c, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
>> + [RST_HS] = { 0x2c, RST_TYPE_HW_DONE, BIT(4), BIT(0) },
>> + [RST_HS_AHB] = { 0x2c, RST_TYPE_HW_DONE, BIT(5), BIT(1) },
>
> The TRM calls these HISYS. Is this shortened to HS on purpose?
>
I kept it with vendor's code, but you're right, HISYS is easier to
understand. I will update this and LS.
> [...]
>
>> +static int k230_rst_reset(struct reset_controller_dev *rcdev, unsigned long id)
>> +{
>> + struct k230_rst *rstc = to_k230_rst(rcdev);
>> + const struct k230_rst_map *rmap = &k230_resets[id];
>> + u32 reg;
>> + int ret = 0;
>> +
>> + switch (rmap->type) {
>> + case RST_TYPE_CPU0:
>> + k230_rst_clear_done(rstc, id, true);
>> + k230_rst_update(rstc, id, true, true, false);
>> + ret = k230_rst_wait_and_clear_done(rstc, id, true);
>> +
>> + /*
>> + * The time period when reset is applied and removed but the
>> + * clock is stopped for RST_TYPE_CPU0 can be set up to 7.5us.
>> + * Delay 10us to ensure proper reset timing.
>> + */
>> + udelay(10);
>> +
>> + break;
>> + case RST_TYPE_FLUSH:
>> + k230_rst_update(rstc, id, true, true, false);
>> +
>> + /* Wait flush request bit auto cleared by hardware */
>> + ret = readl_poll_timeout(rstc->base + rmap->offset, reg,
>> + !(reg & rmap->reset), 10, 1000);
>> + if (ret)
>> + dev_err(rstc->dev, "Wait for flush done timeout\n");
>> +
>> + break;
>> + case RST_TYPE_HW_DONE:
>> + k230_rst_clear_done(rstc, id, false);
>> + k230_rst_update(rstc, id, true, false, false);
>> + ret = k230_rst_wait_and_clear_done(rstc, id, false);
>> +
>> + /*
>> + * The time period when reset is applied and removed but the
>> + * clock is stopped for RST_TYPE_HW_DONE can be set up to
>> + * 127.5us. Delay 200us to ensure proper reset timing.
>> + */
>> + udelay(200);
>
> Consider using usleep_range(), or fsleep().
>
> regards
> Philipp
--
Best regards,
Junhui Liu
Powered by blists - more mailing lists