diff mbox series

[2/2] reset: canaan: Add reset driver for Kendryte K230

Message ID 20240924-k230-reset-v1-2-d0cdc11989eb@outlook.com (mailing list archive)
State Handled Elsewhere
Headers show
Series Add initial support for Canaan Kendryte K230 reset controller | expand

Checks

Context Check Description
conchuod/vmtest-for-next-PR success PR summary
conchuod/patch-2-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh took 134.16s
conchuod/patch-2-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh took 1376.07s
conchuod/patch-2-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh took 1622.38s
conchuod/patch-2-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh took 20.28s
conchuod/patch-2-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh took 22.38s
conchuod/patch-2-test-6 warning .github/scripts/patches/tests/checkpatch.sh took 1.37s
conchuod/patch-2-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh took 42.60s
conchuod/patch-2-test-8 success .github/scripts/patches/tests/header_inline.sh took 0.00s
conchuod/patch-2-test-9 success .github/scripts/patches/tests/kdoc.sh took 0.51s
conchuod/patch-2-test-10 success .github/scripts/patches/tests/module_param.sh took 0.01s
conchuod/patch-2-test-11 success .github/scripts/patches/tests/verify_fixes.sh took 0.00s
conchuod/patch-2-test-12 success .github/scripts/patches/tests/verify_signedoff.sh took 0.03s

Commit Message

Junhui Liu Sept. 24, 2024, 6 a.m. UTC
From: Junhui Liu <liujh2818@outlook.com>

Add support for the resets on Canaan Kendryte K230 SoC.

Signed-off-by: Junhui Liu <liujh2818@outlook.com>
---
 drivers/reset/Kconfig      |   8 ++
 drivers/reset/Makefile     |   1 +
 drivers/reset/reset-k230.c | 321 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 330 insertions(+)

Comments

Philipp Zabel Sept. 24, 2024, 9:17 a.m. UTC | #1
On Di, 2024-09-24 at 14:00 +0800, Junhui Liu wrote:
> From: Junhui Liu <liujh2818@outlook.com>
> 
> Add support for the resets on Canaan Kendryte K230 SoC.
> 
> Signed-off-by: Junhui Liu <liujh2818@outlook.com>
> ---
>  drivers/reset/Kconfig      |   8 ++
>  drivers/reset/Makefile     |   1 +
>  drivers/reset/reset-k230.c | 321 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 330 insertions(+)
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 5484a65f66b95374e25bac31f539a2dd92ae007e..716c676e9b934dd3d2b1ee28f2c43ef38daf7dba 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -133,6 +133,14 @@ config RESET_K210
>  	  Say Y if you want to control reset signals provided by this
>  	  controller.
>  
> +config RESET_K230
> +	bool "Reset controller driver for Canaan Kendryte K230 SoC"
> +	depends on (ARCH_CANAAN || COMPILE_TEST) && 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 4411a2a124d7de29808fcf36d0829393fc79af72..f02c35607ba88947e868d33ead70e9ec91a85636 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -18,6 +18,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..9c693e1cee35dd725bebb6916002f846e8b4003b
> --- /dev/null
> +++ b/drivers/reset/reset-k230.c
> @@ -0,0 +1,321 @@
> +// 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 Junhui Liu <liujh2818@outlook.com>
> + */
> +
> +#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 <linux/delay.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 = 0,
> +	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;
> +};
> +
> +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) },
> +	[RST_SDIO0]		= { 0x34, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
> +	[RST_SDIO1]		= { 0x34, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
> +	[RST_SDIO_AXI]		= { 0x34, RST_TYPE_HW_DONE, BIT(30), BIT(2) },
> +	[RST_USB0]		= { 0x3c, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
> +	[RST_USB1]		= { 0x3c, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
> +	[RST_USB0_AHB]		= { 0x3c, RST_TYPE_HW_DONE, BIT(30), BIT(0) },
> +	[RST_USB1_AHB]		= { 0x3c, RST_TYPE_HW_DONE, BIT(31), BIT(1) },
> +	[RST_SPI0]		= { 0x44, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
> +	[RST_SPI1]		= { 0x44, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
> +	[RST_SPI2]		= { 0x44, RST_TYPE_HW_DONE, BIT(30), BIT(2) },
> +	[RST_SEC]		= { 0x4c, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
> +	[RST_PDMA]		= { 0x54, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
> +	[RST_SDMA]		= { 0x54, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
> +	[RST_DECOMPRESS]	= { 0x5c, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
> +	[RST_SRAM]		= { 0x64, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
> +	[RST_SHRM_AXIM]		= { 0x64, RST_TYPE_HW_DONE, BIT(30), BIT(2) },
> +	[RST_SHRM_AXIS]		= { 0x64, RST_TYPE_HW_DONE, BIT(31), BIT(3) },
> +	[RST_NONAI2D]		= { 0x6c, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
> +	[RST_MCTL]		= { 0x74, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
> +	[RST_ISP]		= { 0x80, RST_TYPE_HW_DONE, BIT(29), BIT(6) },
> +	[RST_ISP_DW]		= { 0x80, RST_TYPE_HW_DONE, BIT(28), BIT(5) },
> +	[RST_DPU]		= { 0x88, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
> +	[RST_DISP]		= { 0x90, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
> +	[RST_GPU]		= { 0x98, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
> +	[RST_AUDIO]		= { 0xa4, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
> +	[RST_TIMER0]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(0) },
> +	[RST_TIMER1]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(1) },
> +	[RST_TIMER2]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(2) },
> +	[RST_TIMER3]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(3) },
> +	[RST_TIMER4]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(4) },
> +	[RST_TIMER5]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(5) },
> +	[RST_TIMER_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(6) },
> +	[RST_HDI]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(7) },
> +	[RST_WDT0]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(12) },
> +	[RST_WDT1]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(13) },
> +	[RST_WDT0_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(14) },
> +	[RST_WDT1_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(15) },
> +	[RST_TS_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(16) },
> +	[RST_MAILBOX]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(17) },
> +	[RST_STC]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(18) },
> +	[RST_PMU]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(19) },
> +	[RST_LS_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(0) },
> +	[RST_UART0]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(1) },
> +	[RST_UART1]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(2) },
> +	[RST_UART2]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(3) },
> +	[RST_UART3]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(4) },
> +	[RST_UART4]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(5) },
> +	[RST_I2C0]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(6) },
> +	[RST_I2C1]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(7) },
> +	[RST_I2C2]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(8) },
> +	[RST_I2C3]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(9) },
> +	[RST_I2C4]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(10) },
> +	[RST_JAMLINK0_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(11) },
> +	[RST_JAMLINK1_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(12) },
> +	[RST_JAMLINK2_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(13) },
> +	[RST_JAMLINK3_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(14) },
> +	[RST_CODEC_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(17) },
> +	[RST_GPIO_DB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(18) },
> +	[RST_GPIO_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(19) },
> +	[RST_ADC]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(20) },
> +	[RST_ADC_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(21) },
> +	[RST_PWM_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(22) },
> +	[RST_SHRM_APB]		= { 0x64, RST_TYPE_SW_DONE, 0,       BIT(1) },
> +	[RST_CSI0]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(0) },
> +	[RST_CSI1]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(1) },
> +	[RST_CSI2]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(2) },
> +	[RST_CSI_DPHY]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(3) },
> +	[RST_ISP_AHB]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(4) },
> +	[RST_M0]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(7) },
> +	[RST_M1]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(8) },
> +	[RST_M2]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(9) },
> +	[RST_SPI2AXI]		= { 0xa8, RST_TYPE_SW_DONE, 0,       BIT(0) }
> +};
> +
> +#define to_k230_rst(p) container_of((p), struct k230_rst, rcdev)

Consider turning this into an inline function.

> +static void k230_rst_clear_done(struct k230_rst *rstc, unsigned long id,
> +				bool write_en)
> +{
> +	const struct k230_rst_map *rmap = &k230_resets[id];
> +	unsigned long flags;
> +	u32 reg;
> +
> +	spin_lock_irqsave(&rstc->lock, flags);

You could use guard(spinlock_irqsave)(&rstc->lock) to save a few lines.

> +	reg = readl(rstc->base + rmap->offset);
> +
> +	/* write 1 to clear */
> +	reg |= rmap->done;
> +	if (write_en)
> +		reg |= rmap->done << 16;
> +
> +	writel(reg, rstc->base + rmap->offset);
> +
> +	spin_unlock_irqrestore(&rstc->lock, flags);
> +}
> +
> +static int k230_rst_wait_and_clear_done(struct k230_rst *rstc, unsigned long id,
> +					bool write_en)
> +{
> +	const struct k230_rst_map *rmap = &k230_resets[id];
> +	u32 reg;
> +	int ret;
> +
> +	ret = readl_poll_timeout(rstc->base + rmap->offset, reg,
> +				 reg & rmap->done, 10, 1000);
> +	if (ret) {
> +		dev_err(rstc->dev, "Wait for reset done timeout\n");
> +		return ret;
> +	}
> +
> +	k230_rst_clear_done(rstc, id, write_en);
> +
> +	return 0;
> +}
> +
> +static void k230_rst_update(struct k230_rst *rstc, unsigned long id,
> +			    bool assert, bool write_en, bool active_low)
> +{
> +	const struct k230_rst_map *rmap = &k230_resets[id];
> +	unsigned long flags;
> +	u32 reg;
> +
> +	spin_lock_irqsave(&rstc->lock, flags);

Same as above, maybe use guard(spinlock_irqsave)(&rstc->lock).

> +
> +	reg = readl(rstc->base + rmap->offset);
> +
> +	if (assert ^ active_low)
> +		reg |= rmap->reset;
> +	else
> +		reg &= ~rmap->reset;
> +
> +	if (write_en)
> +		reg |= rmap->reset << 16;
> +
> +	writel(reg, rstc->base + rmap->offset);
> +
> +	spin_unlock_irqrestore(&rstc->lock, flags);
> +}
> +
> +static int k230_rst_assert(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];
> +	int ret;
> +
> +	switch (rmap->type) {
> +	case RST_TYPE_CPU0:

I'd expect this and the other self-clearing resets to return -ENOTSUPP,
as reset_control_assert() most likely won't return with the reset line
still asserted.

> +		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);
> +		break;

This should be implemented in k230_rst_reset().

> +	case RST_TYPE_CPU1:
> +	case RST_TYPE_FLUSH:
> +		k230_rst_update(rstc, id, true, true, false);
> +		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);

Same for RST_TYPE_FLUSH and RST_TYPE_HW_DONE.

> +		break;
> +	case RST_TYPE_SW_DONE:
> +		k230_rst_update(rstc, id, true, false,
> +				id == RST_SPI2AXI ? false : true);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int k230_rst_deassert(struct reset_controller_dev *rcdev,
> +			     unsigned long id)
> +{
> +	struct k230_rst *rstc = to_k230_rst(rcdev);
> +	int ret;

Here ret should be initialized to 0.

> +
> +	switch (k230_resets[id].type) {
> +	case RST_TYPE_CPU0:
> +		break;
> +	case RST_TYPE_CPU1:
> +		k230_rst_update(rstc, id, false, true, false);
> +		ret = k230_rst_wait_and_clear_done(rstc, id, true);

This looks odd, but maybe that's how the hardware works. To be sure,
you are waiting for the done bit *after* manually deasserting the
reset, on purpose?

> +		break;
> +	case RST_TYPE_FLUSH:
> +	case RST_TYPE_HW_DONE:
> +		break;
> +	case RST_TYPE_SW_DONE:
> +		k230_rst_update(rstc, id, false, false,
> +				id == RST_SPI2AXI ? false : true);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret;

Currently this returns an uninitialized value for the self-clearing
resets (RST_TYPE_CPU0, RST_TYPE_FLUSH, and RST_TYPE_HW_DONE).

> +}
> +
> +static int k230_rst_reset(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	int ret;
> +
> +	ret = k230_rst_assert(rcdev, id);
> +	if (ret)
> +		return ret;
> +
> +	udelay(10);

Is this delay enough for all consumers?
Is this delay needed for the resets that wait for the done bit after
assertion (RST_TYPE_CPU0 and RST_TYPE_HW_DONE)?

> +
> +	return k230_rst_deassert(rcdev, id);
> +}
> +
> +static const struct reset_control_ops k230_rst_ops = {
> +	.reset		= k230_rst_reset,
> +	.assert		= k230_rst_assert,
> +	.deassert	= k230_rst_deassert,
> +};
> +
> +static int k230_rst_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct k230_rst *rstc;
> +
> +	rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
> +	if (!rstc)
> +		return -ENOMEM;
> +
> +	rstc->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(rstc->base))
> +		return PTR_ERR(rstc->base);
> +
> +	spin_lock_init(&rstc->lock);
> +
> +	rstc->dev		= dev;
> +	rstc->rcdev.owner	= THIS_MODULE;
> +	rstc->rcdev.ops		= &k230_rst_ops;
> +	rstc->rcdev.nr_resets	= ARRAY_SIZE(k230_resets);
> +	rstc->rcdev.of_node	= dev->of_node;
> +
> +	return devm_reset_controller_register(dev, &rstc->rcdev);
> +}
> +
> +static const struct of_device_id k230_rst_match[] = {
> +	{ .compatible = "canaan,k230-rst", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, k230_rst_match);
> +
> +static struct platform_driver k230_rst_driver = {
> +	.probe = k230_rst_probe,
> +	.driver = {
> +		.name = "k230-rst",
> +		.of_match_table = k230_rst_match,
> +	}
> +};
> +module_platform_driver(k230_rst_driver);
> +
> +MODULE_AUTHOR("Junhui Liu <liujh2818@outlook.com>");
> +MODULE_DESCRIPTION("Canaan K230 reset driver");
> +MODULE_LICENSE("GPL v2");

regards
Philipp
kernel test robot Sept. 24, 2024, 6:32 p.m. UTC | #2
Hi Junhui,

kernel test robot noticed the following build warnings:

[auto build test WARNING on abf2050f51fdca0fd146388f83cddd95a57a008d]

url:    https://github.com/intel-lab-lkp/linux/commits/Junhui-Liu/dt-bindings-reset-Add-support-for-canaan-k230-rst/20240924-140732
base:   abf2050f51fdca0fd146388f83cddd95a57a008d
patch link:    https://lore.kernel.org/r/20240924-k230-reset-v1-2-d0cdc11989eb%40outlook.com
patch subject: [PATCH 2/2] reset: canaan: Add reset driver for Kendryte K230
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240925/202409250201.ZlZsYfH8-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 8663a75fa2f31299ab8d1d90288d9df92aadee88)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240925/202409250201.ZlZsYfH8-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409250201.ZlZsYfH8-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/reset/reset-k230.c:8:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/reset/reset-k230.c:8:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/reset/reset-k230.c:8:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> drivers/reset/reset-k230.c:223:7: warning: variable 'ret' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
     223 |         case RST_TYPE_SW_DONE:
         |              ^~~~~~~~~~~~~~~~
   drivers/reset/reset-k230.c:231:9: note: uninitialized use occurs here
     231 |         return ret;
         |                ^~~
   drivers/reset/reset-k230.c:214:7: warning: variable 'ret' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
     214 |         case RST_TYPE_CPU1:
         |              ^~~~~~~~~~~~~
   drivers/reset/reset-k230.c:231:9: note: uninitialized use occurs here
     231 |         return ret;
         |                ^~~
   drivers/reset/reset-k230.c:215:7: warning: variable 'ret' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
     215 |         case RST_TYPE_FLUSH:
         |              ^~~~~~~~~~~~~~
   drivers/reset/reset-k230.c:231:9: note: uninitialized use occurs here
     231 |         return ret;
         |                ^~~
   drivers/reset/reset-k230.c:206:9: note: initialize the variable 'ret' to silence this warning
     206 |         int ret;
         |                ^
         |                 = 0
   drivers/reset/reset-k230.c:250:7: warning: variable 'ret' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
     250 |         case RST_TYPE_SW_DONE:
         |              ^~~~~~~~~~~~~~~~
   drivers/reset/reset-k230.c:258:9: note: uninitialized use occurs here
     258 |         return ret;
         |                ^~~
   drivers/reset/reset-k230.c:247:7: warning: variable 'ret' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
     247 |         case RST_TYPE_FLUSH:
         |              ^~~~~~~~~~~~~~
   drivers/reset/reset-k230.c:258:9: note: uninitialized use occurs here
     258 |         return ret;
         |                ^~~
   drivers/reset/reset-k230.c:248:7: warning: variable 'ret' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
     248 |         case RST_TYPE_HW_DONE:
         |              ^~~~~~~~~~~~~~~~
   drivers/reset/reset-k230.c:258:9: note: uninitialized use occurs here
     258 |         return ret;
         |                ^~~
   drivers/reset/reset-k230.c:241:7: warning: variable 'ret' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
     241 |         case RST_TYPE_CPU0:
         |              ^~~~~~~~~~~~~
   drivers/reset/reset-k230.c:258:9: note: uninitialized use occurs here
     258 |         return ret;
         |                ^~~
   drivers/reset/reset-k230.c:238:9: note: initialize the variable 'ret' to silence this warning
     238 |         int ret;
         |                ^
         |                 = 0
   13 warnings generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [m]:
   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]


vim +/ret +223 drivers/reset/reset-k230.c

   201	
   202	static int k230_rst_assert(struct reset_controller_dev *rcdev, unsigned long id)
   203	{
   204		struct k230_rst *rstc = to_k230_rst(rcdev);
   205		const struct k230_rst_map *rmap = &k230_resets[id];
   206		int ret;
   207	
   208		switch (rmap->type) {
   209		case RST_TYPE_CPU0:
   210			k230_rst_clear_done(rstc, id, true);
   211			k230_rst_update(rstc, id, true, true, false);
   212			ret = k230_rst_wait_and_clear_done(rstc, id, true);
   213			break;
   214		case RST_TYPE_CPU1:
   215		case RST_TYPE_FLUSH:
   216			k230_rst_update(rstc, id, true, true, false);
   217			break;
   218		case RST_TYPE_HW_DONE:
   219			k230_rst_clear_done(rstc, id, false);
   220			k230_rst_update(rstc, id, true, false, false);
   221			ret = k230_rst_wait_and_clear_done(rstc, id, false);
   222			break;
 > 223		case RST_TYPE_SW_DONE:
   224			k230_rst_update(rstc, id, true, false,
   225					id == RST_SPI2AXI ? false : true);
   226			break;
   227		default:
   228			return -EINVAL;
   229		}
   230	
   231		return ret;
   232	}
   233
Junhui Liu Oct. 4, 2024, 5:12 a.m. UTC | #3
Hi, Philipp:
Thanks for your review!

On 2024/9/24 17:17, Philipp Zabel wrote:
> On Di, 2024-09-24 at 14:00 +0800, Junhui Liu wrote:
>> From: Junhui Liu<liujh2818@outlook.com>
>>
>> Add support for the resets on Canaan Kendryte K230 SoC.
>>
>> Signed-off-by: Junhui Liu<liujh2818@outlook.com>
>> ---
>>   drivers/reset/Kconfig      |   8 ++
>>   drivers/reset/Makefile     |   1 +
>>   drivers/reset/reset-k230.c | 321 +++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 330 insertions(+)
>>
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index 5484a65f66b95374e25bac31f539a2dd92ae007e..716c676e9b934dd3d2b1ee28f2c43ef38daf7dba 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -133,6 +133,14 @@ config RESET_K210
>>   	  Say Y if you want to control reset signals provided by this
>>   	  controller.
>>   
>> +config RESET_K230
>> +	bool "Reset controller driver for Canaan Kendryte K230 SoC"
>> +	depends on (ARCH_CANAAN || COMPILE_TEST) && 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 4411a2a124d7de29808fcf36d0829393fc79af72..f02c35607ba88947e868d33ead70e9ec91a85636 100644
>> --- a/drivers/reset/Makefile
>> +++ b/drivers/reset/Makefile
>> @@ -18,6 +18,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..9c693e1cee35dd725bebb6916002f846e8b4003b
>> --- /dev/null
>> +++ b/drivers/reset/reset-k230.c
>> @@ -0,0 +1,321 @@
>> +// 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 Junhui Liu<liujh2818@outlook.com>
>> + */
>> +
>> +#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 <linux/delay.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 = 0,
>> +	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;
>> +};
>> +
>> +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) },
>> +	[RST_SDIO0]		= { 0x34, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
>> +	[RST_SDIO1]		= { 0x34, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
>> +	[RST_SDIO_AXI]		= { 0x34, RST_TYPE_HW_DONE, BIT(30), BIT(2) },
>> +	[RST_USB0]		= { 0x3c, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
>> +	[RST_USB1]		= { 0x3c, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
>> +	[RST_USB0_AHB]		= { 0x3c, RST_TYPE_HW_DONE, BIT(30), BIT(0) },
>> +	[RST_USB1_AHB]		= { 0x3c, RST_TYPE_HW_DONE, BIT(31), BIT(1) },
>> +	[RST_SPI0]		= { 0x44, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
>> +	[RST_SPI1]		= { 0x44, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
>> +	[RST_SPI2]		= { 0x44, RST_TYPE_HW_DONE, BIT(30), BIT(2) },
>> +	[RST_SEC]		= { 0x4c, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
>> +	[RST_PDMA]		= { 0x54, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
>> +	[RST_SDMA]		= { 0x54, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
>> +	[RST_DECOMPRESS]	= { 0x5c, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
>> +	[RST_SRAM]		= { 0x64, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
>> +	[RST_SHRM_AXIM]		= { 0x64, RST_TYPE_HW_DONE, BIT(30), BIT(2) },
>> +	[RST_SHRM_AXIS]		= { 0x64, RST_TYPE_HW_DONE, BIT(31), BIT(3) },
>> +	[RST_NONAI2D]		= { 0x6c, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
>> +	[RST_MCTL]		= { 0x74, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
>> +	[RST_ISP]		= { 0x80, RST_TYPE_HW_DONE, BIT(29), BIT(6) },
>> +	[RST_ISP_DW]		= { 0x80, RST_TYPE_HW_DONE, BIT(28), BIT(5) },
>> +	[RST_DPU]		= { 0x88, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
>> +	[RST_DISP]		= { 0x90, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
>> +	[RST_GPU]		= { 0x98, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
>> +	[RST_AUDIO]		= { 0xa4, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
>> +	[RST_TIMER0]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(0) },
>> +	[RST_TIMER1]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(1) },
>> +	[RST_TIMER2]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(2) },
>> +	[RST_TIMER3]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(3) },
>> +	[RST_TIMER4]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(4) },
>> +	[RST_TIMER5]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(5) },
>> +	[RST_TIMER_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(6) },
>> +	[RST_HDI]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(7) },
>> +	[RST_WDT0]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(12) },
>> +	[RST_WDT1]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(13) },
>> +	[RST_WDT0_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(14) },
>> +	[RST_WDT1_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(15) },
>> +	[RST_TS_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(16) },
>> +	[RST_MAILBOX]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(17) },
>> +	[RST_STC]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(18) },
>> +	[RST_PMU]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(19) },
>> +	[RST_LS_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(0) },
>> +	[RST_UART0]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(1) },
>> +	[RST_UART1]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(2) },
>> +	[RST_UART2]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(3) },
>> +	[RST_UART3]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(4) },
>> +	[RST_UART4]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(5) },
>> +	[RST_I2C0]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(6) },
>> +	[RST_I2C1]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(7) },
>> +	[RST_I2C2]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(8) },
>> +	[RST_I2C3]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(9) },
>> +	[RST_I2C4]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(10) },
>> +	[RST_JAMLINK0_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(11) },
>> +	[RST_JAMLINK1_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(12) },
>> +	[RST_JAMLINK2_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(13) },
>> +	[RST_JAMLINK3_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(14) },
>> +	[RST_CODEC_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(17) },
>> +	[RST_GPIO_DB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(18) },
>> +	[RST_GPIO_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(19) },
>> +	[RST_ADC]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(20) },
>> +	[RST_ADC_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(21) },
>> +	[RST_PWM_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(22) },
>> +	[RST_SHRM_APB]		= { 0x64, RST_TYPE_SW_DONE, 0,       BIT(1) },
>> +	[RST_CSI0]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(0) },
>> +	[RST_CSI1]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(1) },
>> +	[RST_CSI2]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(2) },
>> +	[RST_CSI_DPHY]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(3) },
>> +	[RST_ISP_AHB]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(4) },
>> +	[RST_M0]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(7) },
>> +	[RST_M1]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(8) },
>> +	[RST_M2]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(9) },
>> +	[RST_SPI2AXI]		= { 0xa8, RST_TYPE_SW_DONE, 0,       BIT(0) }
>> +};
>> +
>> +#define to_k230_rst(p) container_of((p), struct k230_rst, rcdev)
> Consider turning this into an inline function.
>
>> +static void k230_rst_clear_done(struct k230_rst *rstc, unsigned long id,
>> +				bool write_en)
>> +{
>> +	const struct k230_rst_map *rmap = &k230_resets[id];
>> +	unsigned long flags;
>> +	u32 reg;
>> +
>> +	spin_lock_irqsave(&rstc->lock, flags);
> You could use guard(spinlock_irqsave)(&rstc->lock) to save a few lines.

Thanks for letting me know about this method, I will try to add it.

>> +	reg = readl(rstc->base + rmap->offset);
>> +
>> +	/* write 1 to clear */
>> +	reg |= rmap->done;
>> +	if (write_en)
>> +		reg |= rmap->done << 16;
>> +
>> +	writel(reg, rstc->base + rmap->offset);
>> +
>> +	spin_unlock_irqrestore(&rstc->lock, flags);
>> +}
>> +
>> +static int k230_rst_wait_and_clear_done(struct k230_rst *rstc, unsigned long id,
>> +					bool write_en)
>> +{
>> +	const struct k230_rst_map *rmap = &k230_resets[id];
>> +	u32 reg;
>> +	int ret;
>> +
>> +	ret = readl_poll_timeout(rstc->base + rmap->offset, reg,
>> +				 reg & rmap->done, 10, 1000);
>> +	if (ret) {
>> +		dev_err(rstc->dev, "Wait for reset done timeout\n");
>> +		return ret;
>> +	}
>> +
>> +	k230_rst_clear_done(rstc, id, write_en);
>> +
>> +	return 0;
>> +}
>> +
>> +static void k230_rst_update(struct k230_rst *rstc, unsigned long id,
>> +			    bool assert, bool write_en, bool active_low)
>> +{
>> +	const struct k230_rst_map *rmap = &k230_resets[id];
>> +	unsigned long flags;
>> +	u32 reg;
>> +
>> +	spin_lock_irqsave(&rstc->lock, flags);
> Same as above, maybe use guard(spinlock_irqsave)(&rstc->lock).
>
>> +
>> +	reg = readl(rstc->base + rmap->offset);
>> +
>> +	if (assert ^ active_low)
>> +		reg |= rmap->reset;
>> +	else
>> +		reg &= ~rmap->reset;
>> +
>> +	if (write_en)
>> +		reg |= rmap->reset << 16;
>> +
>> +	writel(reg, rstc->base + rmap->offset);
>> +
>> +	spin_unlock_irqrestore(&rstc->lock, flags);
>> +}
>> +
>> +static int k230_rst_assert(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];
>> +	int ret;
>> +
>> +	switch (rmap->type) {
>> +	case RST_TYPE_CPU0:
> I'd expect this and the other self-clearing resets to return -ENOTSUPP,
> as reset_control_assert() most likely won't return with the reset line
> still asserted.
>
>

Okay, I will move the self-clearing resets into k230_rst_reset(), and
let them return -ENOTSUPP in assert() and deassert().

>> +		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);
>> +		break;
> This should be implemented in k230_rst_reset().
>
>> +	case RST_TYPE_CPU1:
>> +	case RST_TYPE_FLUSH:
>> +		k230_rst_update(rstc, id, true, true, false);
>> +		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);
> Same for RST_TYPE_FLUSH and RST_TYPE_HW_DONE.
>
>> +		break;
>> +	case RST_TYPE_SW_DONE:
>> +		k230_rst_update(rstc, id, true, false,
>> +				id == RST_SPI2AXI ? false : true);
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int k230_rst_deassert(struct reset_controller_dev *rcdev,
>> +			     unsigned long id)
>> +{
>> +	struct k230_rst *rstc = to_k230_rst(rcdev);
>> +	int ret;
> Here ret should be initialized to 0.
>
>> +
>> +	switch (k230_resets[id].type) {
>> +	case RST_TYPE_CPU0:
>> +		break;
>> +	case RST_TYPE_CPU1:
>> +		k230_rst_update(rstc, id, false, true, false);
>> +		ret = k230_rst_wait_and_clear_done(rstc, id, true);
> This looks odd, but maybe that's how the hardware works. To be sure,
> you are waiting for the done bit *after* manually deasserting the
> reset, on purpose?


Even though this is a manual clear reset, it also has the done bit to
indicate the completion of the reset process (in this case for the
deassert operation). If it's odd here, I will move it to k230_rst_reset().

>> +		break;
>> +	case RST_TYPE_FLUSH:
>> +	case RST_TYPE_HW_DONE:
>> +		break;
>> +	case RST_TYPE_SW_DONE:
>> +		k230_rst_update(rstc, id, false, false,
>> +				id == RST_SPI2AXI ? false : true);
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return ret;
> Currently this returns an uninitialized value for the self-clearing
> resets (RST_TYPE_CPU0, RST_TYPE_FLUSH, and RST_TYPE_HW_DONE).
>
>> +}
>> +
>> +static int k230_rst_reset(struct reset_controller_dev *rcdev, unsigned long id)
>> +{
>> +	int ret;
>> +
>> +	ret = k230_rst_assert(rcdev, id);
>> +	if (ret)
>> +		return ret;
>> +
>> +	udelay(10);
> Is this delay enough for all consumers?

Thanks for your reminder. I am still confirming this issue
with the vendor and I will update it in the next version.

> Is this delay needed for the resets that wait for the done bit after
> assertion (RST_TYPE_CPU0 and RST_TYPE_HW_DONE)?
>
I will remove the delay for self-clearing reset.

>> +
>> +	return k230_rst_deassert(rcdev, id);
>> +}
>> +
>> +static const struct reset_control_ops k230_rst_ops = {
>> +	.reset		= k230_rst_reset,
>> +	.assert		= k230_rst_assert,
>> +	.deassert	= k230_rst_deassert,
>> +};
>> +
>> +static int k230_rst_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct k230_rst *rstc;
>> +
>> +	rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
>> +	if (!rstc)
>> +		return -ENOMEM;
>> +
>> +	rstc->base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(rstc->base))
>> +		return PTR_ERR(rstc->base);
>> +
>> +	spin_lock_init(&rstc->lock);
>> +
>> +	rstc->dev		= dev;
>> +	rstc->rcdev.owner	= THIS_MODULE;
>> +	rstc->rcdev.ops		= &k230_rst_ops;
>> +	rstc->rcdev.nr_resets	= ARRAY_SIZE(k230_resets);
>> +	rstc->rcdev.of_node	= dev->of_node;
>> +
>> +	return devm_reset_controller_register(dev, &rstc->rcdev);
>> +}
>> +
>> +static const struct of_device_id k230_rst_match[] = {
>> +	{ .compatible = "canaan,k230-rst", },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, k230_rst_match);
>> +
>> +static struct platform_driver k230_rst_driver = {
>> +	.probe = k230_rst_probe,
>> +	.driver = {
>> +		.name = "k230-rst",
>> +		.of_match_table = k230_rst_match,
>> +	}
>> +};
>> +module_platform_driver(k230_rst_driver);
>> +
>> +MODULE_AUTHOR("Junhui Liu<liujh2818@outlook.com>");
>> +MODULE_DESCRIPTION("Canaan K230 reset driver");
>> +MODULE_LICENSE("GPL v2");
> regards
> Philipp

BR,
Junhui
diff mbox series

Patch

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 5484a65f66b95374e25bac31f539a2dd92ae007e..716c676e9b934dd3d2b1ee28f2c43ef38daf7dba 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -133,6 +133,14 @@  config RESET_K210
 	  Say Y if you want to control reset signals provided by this
 	  controller.
 
+config RESET_K230
+	bool "Reset controller driver for Canaan Kendryte K230 SoC"
+	depends on (ARCH_CANAAN || COMPILE_TEST) && 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 4411a2a124d7de29808fcf36d0829393fc79af72..f02c35607ba88947e868d33ead70e9ec91a85636 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -18,6 +18,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..9c693e1cee35dd725bebb6916002f846e8b4003b
--- /dev/null
+++ b/drivers/reset/reset-k230.c
@@ -0,0 +1,321 @@ 
+// 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 Junhui Liu <liujh2818@outlook.com>
+ */
+
+#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 <linux/delay.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 = 0,
+	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;
+};
+
+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) },
+	[RST_SDIO0]		= { 0x34, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
+	[RST_SDIO1]		= { 0x34, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
+	[RST_SDIO_AXI]		= { 0x34, RST_TYPE_HW_DONE, BIT(30), BIT(2) },
+	[RST_USB0]		= { 0x3c, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
+	[RST_USB1]		= { 0x3c, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
+	[RST_USB0_AHB]		= { 0x3c, RST_TYPE_HW_DONE, BIT(30), BIT(0) },
+	[RST_USB1_AHB]		= { 0x3c, RST_TYPE_HW_DONE, BIT(31), BIT(1) },
+	[RST_SPI0]		= { 0x44, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
+	[RST_SPI1]		= { 0x44, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
+	[RST_SPI2]		= { 0x44, RST_TYPE_HW_DONE, BIT(30), BIT(2) },
+	[RST_SEC]		= { 0x4c, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
+	[RST_PDMA]		= { 0x54, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
+	[RST_SDMA]		= { 0x54, RST_TYPE_HW_DONE, BIT(29), BIT(1) },
+	[RST_DECOMPRESS]	= { 0x5c, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
+	[RST_SRAM]		= { 0x64, RST_TYPE_HW_DONE, BIT(28), BIT(0) },
+	[RST_SHRM_AXIM]		= { 0x64, RST_TYPE_HW_DONE, BIT(30), BIT(2) },
+	[RST_SHRM_AXIS]		= { 0x64, RST_TYPE_HW_DONE, BIT(31), BIT(3) },
+	[RST_NONAI2D]		= { 0x6c, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
+	[RST_MCTL]		= { 0x74, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
+	[RST_ISP]		= { 0x80, RST_TYPE_HW_DONE, BIT(29), BIT(6) },
+	[RST_ISP_DW]		= { 0x80, RST_TYPE_HW_DONE, BIT(28), BIT(5) },
+	[RST_DPU]		= { 0x88, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
+	[RST_DISP]		= { 0x90, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
+	[RST_GPU]		= { 0x98, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
+	[RST_AUDIO]		= { 0xa4, RST_TYPE_HW_DONE, BIT(31), BIT(0) },
+	[RST_TIMER0]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(0) },
+	[RST_TIMER1]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(1) },
+	[RST_TIMER2]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(2) },
+	[RST_TIMER3]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(3) },
+	[RST_TIMER4]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(4) },
+	[RST_TIMER5]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(5) },
+	[RST_TIMER_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(6) },
+	[RST_HDI]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(7) },
+	[RST_WDT0]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(12) },
+	[RST_WDT1]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(13) },
+	[RST_WDT0_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(14) },
+	[RST_WDT1_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(15) },
+	[RST_TS_APB]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(16) },
+	[RST_MAILBOX]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(17) },
+	[RST_STC]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(18) },
+	[RST_PMU]		= { 0x20, RST_TYPE_SW_DONE, 0,       BIT(19) },
+	[RST_LS_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(0) },
+	[RST_UART0]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(1) },
+	[RST_UART1]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(2) },
+	[RST_UART2]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(3) },
+	[RST_UART3]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(4) },
+	[RST_UART4]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(5) },
+	[RST_I2C0]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(6) },
+	[RST_I2C1]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(7) },
+	[RST_I2C2]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(8) },
+	[RST_I2C3]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(9) },
+	[RST_I2C4]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(10) },
+	[RST_JAMLINK0_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(11) },
+	[RST_JAMLINK1_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(12) },
+	[RST_JAMLINK2_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(13) },
+	[RST_JAMLINK3_APB]	= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(14) },
+	[RST_CODEC_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(17) },
+	[RST_GPIO_DB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(18) },
+	[RST_GPIO_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(19) },
+	[RST_ADC]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(20) },
+	[RST_ADC_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(21) },
+	[RST_PWM_APB]		= { 0x24, RST_TYPE_SW_DONE, 0,       BIT(22) },
+	[RST_SHRM_APB]		= { 0x64, RST_TYPE_SW_DONE, 0,       BIT(1) },
+	[RST_CSI0]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(0) },
+	[RST_CSI1]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(1) },
+	[RST_CSI2]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(2) },
+	[RST_CSI_DPHY]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(3) },
+	[RST_ISP_AHB]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(4) },
+	[RST_M0]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(7) },
+	[RST_M1]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(8) },
+	[RST_M2]		= { 0x80, RST_TYPE_SW_DONE, 0,       BIT(9) },
+	[RST_SPI2AXI]		= { 0xa8, RST_TYPE_SW_DONE, 0,       BIT(0) }
+};
+
+#define to_k230_rst(p) container_of((p), struct k230_rst, rcdev)
+
+static void k230_rst_clear_done(struct k230_rst *rstc, unsigned long id,
+				bool write_en)
+{
+	const struct k230_rst_map *rmap = &k230_resets[id];
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(&rstc->lock, flags);
+
+	reg = readl(rstc->base + rmap->offset);
+
+	/* write 1 to clear */
+	reg |= rmap->done;
+	if (write_en)
+		reg |= rmap->done << 16;
+
+	writel(reg, rstc->base + rmap->offset);
+
+	spin_unlock_irqrestore(&rstc->lock, flags);
+}
+
+static int k230_rst_wait_and_clear_done(struct k230_rst *rstc, unsigned long id,
+					bool write_en)
+{
+	const struct k230_rst_map *rmap = &k230_resets[id];
+	u32 reg;
+	int ret;
+
+	ret = readl_poll_timeout(rstc->base + rmap->offset, reg,
+				 reg & rmap->done, 10, 1000);
+	if (ret) {
+		dev_err(rstc->dev, "Wait for reset done timeout\n");
+		return ret;
+	}
+
+	k230_rst_clear_done(rstc, id, write_en);
+
+	return 0;
+}
+
+static void k230_rst_update(struct k230_rst *rstc, unsigned long id,
+			    bool assert, bool write_en, bool active_low)
+{
+	const struct k230_rst_map *rmap = &k230_resets[id];
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(&rstc->lock, flags);
+
+	reg = readl(rstc->base + rmap->offset);
+
+	if (assert ^ active_low)
+		reg |= rmap->reset;
+	else
+		reg &= ~rmap->reset;
+
+	if (write_en)
+		reg |= rmap->reset << 16;
+
+	writel(reg, rstc->base + rmap->offset);
+
+	spin_unlock_irqrestore(&rstc->lock, flags);
+}
+
+static int k230_rst_assert(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];
+	int ret;
+
+	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);
+		break;
+	case RST_TYPE_CPU1:
+	case RST_TYPE_FLUSH:
+		k230_rst_update(rstc, id, true, true, false);
+		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);
+		break;
+	case RST_TYPE_SW_DONE:
+		k230_rst_update(rstc, id, true, false,
+				id == RST_SPI2AXI ? false : true);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int k230_rst_deassert(struct reset_controller_dev *rcdev,
+			     unsigned long id)
+{
+	struct k230_rst *rstc = to_k230_rst(rcdev);
+	int ret;
+
+	switch (k230_resets[id].type) {
+	case RST_TYPE_CPU0:
+		break;
+	case RST_TYPE_CPU1:
+		k230_rst_update(rstc, id, false, true, false);
+		ret = k230_rst_wait_and_clear_done(rstc, id, true);
+		break;
+	case RST_TYPE_FLUSH:
+	case RST_TYPE_HW_DONE:
+		break;
+	case RST_TYPE_SW_DONE:
+		k230_rst_update(rstc, id, false, false,
+				id == RST_SPI2AXI ? false : true);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int k230_rst_reset(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	int ret;
+
+	ret = k230_rst_assert(rcdev, id);
+	if (ret)
+		return ret;
+
+	udelay(10);
+
+	return k230_rst_deassert(rcdev, id);
+}
+
+static const struct reset_control_ops k230_rst_ops = {
+	.reset		= k230_rst_reset,
+	.assert		= k230_rst_assert,
+	.deassert	= k230_rst_deassert,
+};
+
+static int k230_rst_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct k230_rst *rstc;
+
+	rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
+	if (!rstc)
+		return -ENOMEM;
+
+	rstc->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(rstc->base))
+		return PTR_ERR(rstc->base);
+
+	spin_lock_init(&rstc->lock);
+
+	rstc->dev		= dev;
+	rstc->rcdev.owner	= THIS_MODULE;
+	rstc->rcdev.ops		= &k230_rst_ops;
+	rstc->rcdev.nr_resets	= ARRAY_SIZE(k230_resets);
+	rstc->rcdev.of_node	= dev->of_node;
+
+	return devm_reset_controller_register(dev, &rstc->rcdev);
+}
+
+static const struct of_device_id k230_rst_match[] = {
+	{ .compatible = "canaan,k230-rst", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, k230_rst_match);
+
+static struct platform_driver k230_rst_driver = {
+	.probe = k230_rst_probe,
+	.driver = {
+		.name = "k230-rst",
+		.of_match_table = k230_rst_match,
+	}
+};
+module_platform_driver(k230_rst_driver);
+
+MODULE_AUTHOR("Junhui Liu <liujh2818@outlook.com>");
+MODULE_DESCRIPTION("Canaan K230 reset driver");
+MODULE_LICENSE("GPL v2");