* [PATCH 0/2] Add initial support for Canaan Kendryte K230 reset controller
@ 2024-09-24 6:00 Junhui Liu
2024-09-24 6:00 ` [PATCH 1/2] dt-bindings: reset: Add support for canaan,k230-rst Junhui Liu
2024-09-24 6:00 ` [PATCH 2/2] reset: canaan: Add reset driver for Kendryte K230 Junhui Liu
0 siblings, 2 replies; 7+ messages in thread
From: Junhui Liu @ 2024-09-24 6:00 UTC (permalink / raw)
To: Philipp Zabel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Junhui Liu
Cc: devicetree, linux-kernel, linux-riscv
This patch series add reset controller support for the Canaan Kendryte
K230 SoC.
Tested on CanMV-K230-V1.1 board.
---
Junhui Liu (2):
dt-bindings: reset: Add support for canaan,k230-rst
reset: canaan: Add reset driver for Kendryte K230
.../devicetree/bindings/reset/canaan,k230-rst.yaml | 39 +++
drivers/reset/Kconfig | 8 +
drivers/reset/Makefile | 1 +
drivers/reset/reset-k230.c | 321 +++++++++++++++++++++
include/dt-bindings/reset/canaan,k230-rst.h | 90 ++++++
5 files changed, 459 insertions(+)
---
base-commit: abf2050f51fdca0fd146388f83cddd95a57a008d
change-id: 20240917-k230-reset-e8c58d4b9c28
Best regards,
--
Junhui Liu <liujh2818@outlook.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] dt-bindings: reset: Add support for canaan,k230-rst
2024-09-24 6:00 [PATCH 0/2] Add initial support for Canaan Kendryte K230 reset controller Junhui Liu
@ 2024-09-24 6:00 ` Junhui Liu
2024-09-24 16:32 ` Conor Dooley
2024-09-24 6:00 ` [PATCH 2/2] reset: canaan: Add reset driver for Kendryte K230 Junhui Liu
1 sibling, 1 reply; 7+ messages in thread
From: Junhui Liu @ 2024-09-24 6:00 UTC (permalink / raw)
To: Philipp Zabel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Junhui Liu
Cc: devicetree, linux-kernel, linux-riscv
From: Junhui Liu <liujh2818@outlook.com>
Introduces a reset controller driver for the Kendryte K230 SoC,
resposible for managing the reset functionality of the CPUs and
various sub-modules.
Signed-off-by: Junhui Liu <liujh2818@outlook.com>
---
.../devicetree/bindings/reset/canaan,k230-rst.yaml | 39 ++++++++++
include/dt-bindings/reset/canaan,k230-rst.h | 90 ++++++++++++++++++++++
2 files changed, 129 insertions(+)
diff --git a/Documentation/devicetree/bindings/reset/canaan,k230-rst.yaml b/Documentation/devicetree/bindings/reset/canaan,k230-rst.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..ede02dd01ee53a6e20e3387dec51a34e685aa0e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/canaan,k230-rst.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/canaan,k230-rst.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Canaan Kendryte K230 Reset Controller
+
+maintainers:
+ - Junhui Liu <liujh2818@outlook.com>
+
+description:
+ The Canaan Kendryte K230 reset controller is part of the SoC's system
+ controller and controls the reset registers for CPUs and various peripherals.
+
+properties:
+ compatible:
+ const: canaan,k230-rst
+
+ reg:
+ maxItems: 1
+
+ '#reset-cells':
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ reset-controller@91101000 {
+ compatible = "canaan,k230-rst";
+ reg = <0x91101000 0x1000>;
+ #reset-cells = <1>;
+ };
diff --git a/include/dt-bindings/reset/canaan,k230-rst.h b/include/dt-bindings/reset/canaan,k230-rst.h
new file mode 100644
index 0000000000000000000000000000000000000000..5960705636fbab70859624437862bf06d7fb65cd
--- /dev/null
+++ b/include/dt-bindings/reset/canaan,k230-rst.h
@@ -0,0 +1,90 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023-2024 Canaan Bright Sight Co., Ltd
+ * Copyright (C) 2024 Junhui Liu <liujh2818@outlook.com>
+ */
+#ifndef _DT_BINDINGS_CANAAN_K230_RST_H_
+#define _DT_BINDINGS_CANAAN_K230_RST_H_
+
+#define RST_CPU0 0
+#define RST_CPU1 1
+#define RST_CPU0_FLUSH 2
+#define RST_CPU1_FLUSH 3
+#define RST_AI 4
+#define RST_VPU 5
+#define RST_HS 6
+#define RST_HS_AHB 7
+#define RST_SDIO0 8
+#define RST_SDIO1 9
+#define RST_SDIO_AXI 10
+#define RST_USB0 11
+#define RST_USB1 12
+#define RST_USB0_AHB 13
+#define RST_USB1_AHB 14
+#define RST_SPI0 15
+#define RST_SPI1 16
+#define RST_SPI2 17
+#define RST_SEC 18
+#define RST_PDMA 19
+#define RST_SDMA 20
+#define RST_DECOMPRESS 21
+#define RST_SRAM 22
+#define RST_SHRM_AXIM 23
+#define RST_SHRM_AXIS 24
+#define RST_NONAI2D 25
+#define RST_MCTL 26
+#define RST_ISP 27
+#define RST_ISP_DW 28
+#define RST_DPU 29
+#define RST_DISP 30
+#define RST_GPU 31
+#define RST_AUDIO 32
+#define RST_TIMER0 33
+#define RST_TIMER1 34
+#define RST_TIMER2 35
+#define RST_TIMER3 36
+#define RST_TIMER4 37
+#define RST_TIMER5 38
+#define RST_TIMER_APB 39
+#define RST_HDI 40
+#define RST_WDT0 41
+#define RST_WDT1 42
+#define RST_WDT0_APB 43
+#define RST_WDT1_APB 44
+#define RST_TS_APB 45
+#define RST_MAILBOX 46
+#define RST_STC 47
+#define RST_PMU 48
+#define RST_LS_APB 49
+#define RST_UART0 50
+#define RST_UART1 51
+#define RST_UART2 52
+#define RST_UART3 53
+#define RST_UART4 54
+#define RST_I2C0 55
+#define RST_I2C1 56
+#define RST_I2C2 57
+#define RST_I2C3 58
+#define RST_I2C4 59
+#define RST_JAMLINK0_APB 60
+#define RST_JAMLINK1_APB 61
+#define RST_JAMLINK2_APB 62
+#define RST_JAMLINK3_APB 63
+#define RST_CODEC_APB 64
+#define RST_GPIO_DB 65
+#define RST_GPIO_APB 66
+#define RST_ADC 67
+#define RST_ADC_APB 68
+#define RST_PWM_APB 69
+#define RST_SHRM_APB 70
+#define RST_CSI0 71
+#define RST_CSI1 72
+#define RST_CSI2 73
+#define RST_CSI_DPHY 74
+#define RST_ISP_AHB 75
+#define RST_M0 76
+#define RST_M1 77
+#define RST_M2 78
+#define RST_SPI2AXI 79
+
+#endif
--
2.46.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] reset: canaan: Add reset driver for Kendryte K230
2024-09-24 6:00 [PATCH 0/2] Add initial support for Canaan Kendryte K230 reset controller Junhui Liu
2024-09-24 6:00 ` [PATCH 1/2] dt-bindings: reset: Add support for canaan,k230-rst Junhui Liu
@ 2024-09-24 6:00 ` Junhui Liu
2024-09-24 9:17 ` Philipp Zabel
2024-09-24 18:32 ` kernel test robot
1 sibling, 2 replies; 7+ messages in thread
From: Junhui Liu @ 2024-09-24 6:00 UTC (permalink / raw)
To: Philipp Zabel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Junhui Liu
Cc: devicetree, linux-kernel, linux-riscv
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)
+
+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");
--
2.46.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] reset: canaan: Add reset driver for Kendryte K230
2024-09-24 6:00 ` [PATCH 2/2] reset: canaan: Add reset driver for Kendryte K230 Junhui Liu
@ 2024-09-24 9:17 ` Philipp Zabel
2024-10-04 5:12 ` Junhui Liu
2024-09-24 18:32 ` kernel test robot
1 sibling, 1 reply; 7+ messages in thread
From: Philipp Zabel @ 2024-09-24 9:17 UTC (permalink / raw)
To: Junhui Liu, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Junhui Liu
Cc: devicetree, linux-kernel, linux-riscv
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: reset: Add support for canaan,k230-rst
2024-09-24 6:00 ` [PATCH 1/2] dt-bindings: reset: Add support for canaan,k230-rst Junhui Liu
@ 2024-09-24 16:32 ` Conor Dooley
0 siblings, 0 replies; 7+ messages in thread
From: Conor Dooley @ 2024-09-24 16:32 UTC (permalink / raw)
To: Junhui Liu
Cc: Philipp Zabel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Junhui Liu, devicetree, linux-kernel, linux-riscv
[-- Attachment #1: Type: text/plain, Size: 382 bytes --]
On Tue, Sep 24, 2024 at 02:00:40PM +0800, Junhui Liu wrote:
> From: Junhui Liu <liujh2818@outlook.com>
>
> Introduces a reset controller driver for the Kendryte K230 SoC,
> resposible for managing the reset functionality of the CPUs and
> various sub-modules.
>
> Signed-off-by: Junhui Liu <liujh2818@outlook.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] reset: canaan: Add reset driver for Kendryte K230
2024-09-24 6:00 ` [PATCH 2/2] reset: canaan: Add reset driver for Kendryte K230 Junhui Liu
2024-09-24 9:17 ` Philipp Zabel
@ 2024-09-24 18:32 ` kernel test robot
1 sibling, 0 replies; 7+ messages in thread
From: kernel test robot @ 2024-09-24 18:32 UTC (permalink / raw)
To: Junhui Liu, Philipp Zabel, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Junhui Liu
Cc: llvm, oe-kbuild-all, devicetree, linux-kernel, linux-riscv
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] reset: canaan: Add reset driver for Kendryte K230
2024-09-24 9:17 ` Philipp Zabel
@ 2024-10-04 5:12 ` Junhui Liu
0 siblings, 0 replies; 7+ messages in thread
From: Junhui Liu @ 2024-10-04 5:12 UTC (permalink / raw)
To: Philipp Zabel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Junhui Liu
Cc: devicetree, linux-kernel, linux-riscv
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-10-04 5:13 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-24 6:00 [PATCH 0/2] Add initial support for Canaan Kendryte K230 reset controller Junhui Liu
2024-09-24 6:00 ` [PATCH 1/2] dt-bindings: reset: Add support for canaan,k230-rst Junhui Liu
2024-09-24 16:32 ` Conor Dooley
2024-09-24 6:00 ` [PATCH 2/2] reset: canaan: Add reset driver for Kendryte K230 Junhui Liu
2024-09-24 9:17 ` Philipp Zabel
2024-10-04 5:12 ` Junhui Liu
2024-09-24 18:32 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).