linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/10] Add Sunplus SP7021 SoC Support
@ 2021-12-03  7:34 Qin Jian
  2021-12-03  7:34 ` [PATCH v5 01/10] dt-bindings: vendor-prefixes: Add Sunplus Qin Jian
                   ` (9 more replies)
  0 siblings, 10 replies; 26+ messages in thread
From: Qin Jian @ 2021-12-03  7:34 UTC (permalink / raw)
  To: robh+dt
  Cc: mturquette, sboyd, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian

This patch series add Sunplus SP7021 SoC support.

Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates many
peripherals (ex: UART, I2C, SPI, SDIO, eMMC, USB, SD card and etc.) into a
single chip. It is designed for industrial control.

SP7021 consists of two chips (dies) in a package. One is called C-chip
(computing chip). It is a 4-core ARM Cortex A7 CPU. It adopts high-level
process (22 nm) for high performance computing. The other is called P-
chip (peripheral chip). It has many peripherals and an ARM A926 added
especially for real-time control. P-chip is made for customers. It adopts
low-level process (ex: 0.11 um) to reduce cost.

Refer to (for documentations):
https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview

Refer to (applications):
https://tibbo.com/store/plus1.html

Refer to (applications):
http://www.sinovoip.com.cn/ecp_view.asp?id=586

Changes in v5:
- reset-sunplus.c: fix strict checks
- clk/Kconfig: fix spell
- clk-sp7021.c: using bitfield ops, fix strict checks
- irqchip/Kconfig: fix spell
- irq-sp7021-intc.c: cleanup error path in probe, fix strict checks
- arm/Kconfig: fix spell & typo, remove CONFIG_SERIAL_SUNPLUS
- mach-sunplus/Kconfig: fix typo
- sp7021_defconfig: add CONFIG_SERIAL_SUNPLUS

Changes in v4:
- mach-sunplus: add initial support for SP7021
- sp7021_defconfig: add generic SP7021 defconfig
- reset-sunplus: remove Q645 support
- reset-sunplus.c: refine code based on Philipp's review
- clk-sp7021: clock defines add prefix, more clean up

Changes in v3:
- sp7021-intc: remove primary controller mode due to P-chip running Linux
  not supported any more.
- sp7021-intc.h: removed, not set ext through the DT but sp_intc_set_ext()
- sunplus,sp7021-intc.yaml: update descriptions for above changes
- irq-sp7021-intc.c: more cleanup based on Marc's review
- all driver's Kconfig removed default, it's selected by platform config

Changes in v2:
- sunplus,sp7021-intc.yaml: add descrption for "#interrupt-cells", interrupts
- sunplus,sp7021-intc.yaml: drop "ext0-mask"/"ext1-mask" from DT
- sunplus,sp7021-intc.yaml: fix example.dt too long error
- irq-sp7021-intc.c: major rewrite
- all files with dual license

Qin Jian (10):
  dt-bindings: vendor-prefixes: Add Sunplus
  dt-bindings: arm: sunplus: Add bindings for Sunplus SP7021 SoC boards
  dt-bindings: reset: Add bindings for SP7021 reset driver
  reset: Add Sunplus SP7021 reset driver
  dt-bindings: clock: Add bindings for SP7021 clock driver
  clk: Add Sunplus SP7021 clock driver
  dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt
    controller
  irqchip: Add Sunplus SP7021 interrupt controller driver
  ARM: sunplus: Add initial support for Sunplus SP7021 SoC
  ARM: sp7021_defconfig: Add Sunplus SP7021 defconfig

 .../bindings/arm/sunplus,sp7021.yaml          |  27 +
 .../bindings/clock/sunplus,sp7021-clkc.yaml   |  38 +
 .../sunplus,sp7021-intc.yaml                  |  62 ++
 .../bindings/reset/sunplus,reset.yaml         |  38 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |  17 +
 arch/arm/Kconfig                              |  18 +
 arch/arm/Makefile                             |   2 +
 arch/arm/configs/sp7021_defconfig             | 178 +++++
 arch/arm/mach-sunplus/Kconfig                 |  20 +
 arch/arm/mach-sunplus/Makefile                |   9 +
 arch/arm/mach-sunplus/Makefile.boot           |   3 +
 arch/arm/mach-sunplus/sp7021.c                |  16 +
 drivers/clk/Kconfig                           |   9 +
 drivers/clk/Makefile                          |   1 +
 drivers/clk/clk-sp7021.c                      | 738 ++++++++++++++++++
 drivers/irqchip/Kconfig                       |   9 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-sp7021-intc.c             | 284 +++++++
 drivers/reset/Kconfig                         |   9 +
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-sunplus.c                 | 132 ++++
 include/dt-bindings/clock/sp-sp7021.h         | 112 +++
 include/dt-bindings/reset/sp-sp7021.h         |  97 +++
 24 files changed, 1823 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
 create mode 100644 Documentation/devicetree/bindings/reset/sunplus,reset.yaml
 create mode 100644 arch/arm/configs/sp7021_defconfig
 create mode 100644 arch/arm/mach-sunplus/Kconfig
 create mode 100644 arch/arm/mach-sunplus/Makefile
 create mode 100644 arch/arm/mach-sunplus/Makefile.boot
 create mode 100644 arch/arm/mach-sunplus/sp7021.c
 create mode 100644 drivers/clk/clk-sp7021.c
 create mode 100644 drivers/irqchip/irq-sp7021-intc.c
 create mode 100644 drivers/reset/reset-sunplus.c
 create mode 100644 include/dt-bindings/clock/sp-sp7021.h
 create mode 100644 include/dt-bindings/reset/sp-sp7021.h

-- 
2.33.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v5 01/10] dt-bindings: vendor-prefixes: Add Sunplus
  2021-12-03  7:34 [PATCH v5 00/10] Add Sunplus SP7021 SoC Support Qin Jian
@ 2021-12-03  7:34 ` Qin Jian
  2021-12-03  7:34 ` [PATCH v5 02/10] dt-bindings: arm: sunplus: Add bindings for Sunplus SP7021 SoC boards Qin Jian
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Qin Jian @ 2021-12-03  7:34 UTC (permalink / raw)
  To: robh+dt
  Cc: mturquette, sboyd, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian, Rob Herring

Add vendor prefix for Sunplus Technology Co., Ltd. (http://www.sunplus.com)

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Qin Jian <qinjian@cqplus1.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index a867f7102..50d4ee5ac 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1131,6 +1131,8 @@ patternProperties:
     description: Summit microelectronics
   "^sunchip,.*":
     description: Shenzhen Sunchip Technology Co., Ltd
+  "^sunplus,.*":
+    description: Sunplus Technology Co., Ltd.
   "^SUNW,.*":
     description: Sun Microsystems, Inc
   "^supermicro,.*":
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 02/10] dt-bindings: arm: sunplus: Add bindings for Sunplus SP7021 SoC boards
  2021-12-03  7:34 [PATCH v5 00/10] Add Sunplus SP7021 SoC Support Qin Jian
  2021-12-03  7:34 ` [PATCH v5 01/10] dt-bindings: vendor-prefixes: Add Sunplus Qin Jian
@ 2021-12-03  7:34 ` Qin Jian
  2021-12-03  7:34 ` [PATCH v5 03/10] dt-bindings: reset: Add bindings for SP7021 reset driver Qin Jian
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Qin Jian @ 2021-12-03  7:34 UTC (permalink / raw)
  To: robh+dt
  Cc: mturquette, sboyd, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian, Rob Herring

This introduces bindings for boards based Sunplus SP7021 SoC.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Qin Jian <qinjian@cqplus1.com>
---
 .../bindings/arm/sunplus,sp7021.yaml          | 27 +++++++++++++++++++
 MAINTAINERS                                   |  7 +++++
 2 files changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml

diff --git a/Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml b/Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
new file mode 100644
index 000000000..5b9985b73
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) Sunplus Co., Ltd. 2021
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/sunplus,sp7021.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sunplus SP7021 Boards Device Tree Bindings
+
+maintainers:
+  - qinjian <qinjian@cqplus1.com>
+
+description: |
+  ARM platforms using Sunplus SP7021, an ARM Cortex A7 (4-cores) based SoC.
+  Wiki: https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
+
+properties:
+  $nodename:
+    const: '/'
+  compatible:
+    oneOf:
+      - items:
+          - const: sunplus,sp7021-achip
+
+additionalProperties: true
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index e0bca0de0..6a5422f10 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2655,6 +2655,13 @@ F:	drivers/clocksource/armv7m_systick.c
 N:	stm32
 N:	stm
 
+ARM/SUNPLUS SP7021 SOC SUPPORT
+M:	Qin Jian <qinjian@cqplus1.com>
+L:	linux-arm-kernel@lists.infradead.org (moderated for mon-subscribers)
+S:	Maintained
+W:	https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
+F:	Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
+
 ARM/Synaptics SoC support
 M:	Jisheng Zhang <Jisheng.Zhang@synaptics.com>
 M:	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 03/10] dt-bindings: reset: Add bindings for SP7021 reset driver
  2021-12-03  7:34 [PATCH v5 00/10] Add Sunplus SP7021 SoC Support Qin Jian
  2021-12-03  7:34 ` [PATCH v5 01/10] dt-bindings: vendor-prefixes: Add Sunplus Qin Jian
  2021-12-03  7:34 ` [PATCH v5 02/10] dt-bindings: arm: sunplus: Add bindings for Sunplus SP7021 SoC boards Qin Jian
@ 2021-12-03  7:34 ` Qin Jian
  2021-12-03  7:34 ` [PATCH v5 04/10] reset: Add Sunplus " Qin Jian
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Qin Jian @ 2021-12-03  7:34 UTC (permalink / raw)
  To: robh+dt
  Cc: mturquette, sboyd, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian, Rob Herring

Add documentation to describe Sunplus SP7021 reset driver bindings.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Qin Jian <qinjian@cqplus1.com>
---
 .../bindings/reset/sunplus,reset.yaml         | 38 ++++++++
 MAINTAINERS                                   |  2 +
 include/dt-bindings/reset/sp-sp7021.h         | 97 +++++++++++++++++++
 3 files changed, 137 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/sunplus,reset.yaml
 create mode 100644 include/dt-bindings/reset/sp-sp7021.h

diff --git a/Documentation/devicetree/bindings/reset/sunplus,reset.yaml b/Documentation/devicetree/bindings/reset/sunplus,reset.yaml
new file mode 100644
index 000000000..c083c821f
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/sunplus,reset.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) Sunplus Co., Ltd. 2021
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/reset/sunplus,reset.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Sunplus SoC Reset Controller
+
+maintainers:
+  - Qin Jian <qinjian@cqplus1.com>
+
+properties:
+  compatible:
+    const: sunplus,sp7021-reset
+
+  "#reset-cells":
+    const: 1
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - "#reset-cells"
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    rstc: reset@9c000054 {
+      compatible = "sunplus,sp7021-reset";
+      #reset-cells = <1>;
+      reg = <0x9c000054 0x28>;
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 6a5422f10..652f42cab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2661,6 +2661,8 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for mon-subscribers)
 S:	Maintained
 W:	https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
 F:	Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
+F:	Documentation/devicetree/bindings/reset/sunplus,reset.yaml
+F:	include/dt-bindings/reset/sp-sp7021.h
 
 ARM/Synaptics SoC support
 M:	Jisheng Zhang <Jisheng.Zhang@synaptics.com>
diff --git a/include/dt-bindings/reset/sp-sp7021.h b/include/dt-bindings/reset/sp-sp7021.h
new file mode 100644
index 000000000..c0224a960
--- /dev/null
+++ b/include/dt-bindings/reset/sp-sp7021.h
@@ -0,0 +1,97 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (C) Sunplus Technology Co., Ltd.
+ *       All rights reserved.
+ */
+#ifndef _DT_BINDINGS_RST_SUNPLUS_SP7021_H
+#define _DT_BINDINGS_RST_SUNPLUS_SP7021_H
+
+/* mo_reset0 ~ mo_reset9 */
+#define RST_SYSTEM		0x00
+#define RST_RTC			0x02
+#define RST_IOCTL		0x03
+#define RST_IOP			0x04
+#define RST_OTPRX		0x05
+#define RST_NOC			0x06
+#define RST_BR			0x07
+#define RST_RBUS_L00	0x08
+#define RST_SPIFL		0x09
+#define RST_SDCTRL0		0x0a
+#define RST_PERI0		0x0b
+#define RST_A926		0x0d
+#define RST_UMCTL2		0x0e
+#define RST_PERI1		0x0f
+
+#define RST_DDR_PHY0	0x10
+#define RST_ACHIP		0x12
+#define RST_STC0		0x14
+#define RST_STC_AV0		0x15
+#define RST_STC_AV1		0x16
+#define RST_STC_AV2		0x17
+#define RST_UA0			0x18
+#define RST_UA1			0x19
+#define RST_UA2			0x1a
+#define RST_UA3			0x1b
+#define RST_UA4			0x1c
+#define RST_HWUA		0x1d
+#define RST_DDC0		0x1e
+#define RST_UADMA		0x1f
+
+#define RST_CBDMA0		0x20
+#define RST_CBDMA1		0x21
+#define RST_SPI_COMBO_0	0x22
+#define RST_SPI_COMBO_1	0x23
+#define RST_SPI_COMBO_2	0x24
+#define RST_SPI_COMBO_3	0x25
+#define RST_AUD			0x26
+#define RST_USBC0		0x2a
+#define RST_USBC1		0x2b
+#define RST_UPHY0		0x2d
+#define RST_UPHY1		0x2e
+
+#define RST_I2CM0		0x30
+#define RST_I2CM1		0x31
+#define RST_I2CM2		0x32
+#define RST_I2CM3		0x33
+#define RST_PMC			0x3d
+#define RST_CARD_CTL0	0x3e
+#define RST_CARD_CTL1	0x3f
+
+#define RST_CARD_CTL4	0x42
+#define RST_BCH			0x44
+#define RST_DDFCH		0x4b
+#define RST_CSIIW0		0x4c
+#define RST_CSIIW1		0x4d
+#define RST_MIPICSI0	0x4e
+#define RST_MIPICSI1	0x4f
+
+#define RST_HDMI_TX		0x50
+#define RST_VPOST		0x55
+
+#define RST_TGEN		0x60
+#define RST_DMIX		0x61
+#define RST_TCON		0x6a
+#define RST_INTERRUPT	0x6f
+
+#define RST_RGST		0x70
+#define RST_GPIO		0x73
+#define RST_RBUS_TOP	0x74
+
+#define RST_MAILBOX		0x86
+#define RST_SPIND		0x8a
+#define RST_I2C2CBUS	0x8b
+#define RST_SEC			0x8d
+#define RST_DVE			0x8e
+#define RST_GPOST0		0x8f
+
+#define RST_OSD0		0x90
+#define RST_DISP_PWM	0x92
+#define RST_UADBG		0x93
+#define RST_DUMMY_MASTER	0x94
+#define RST_FIO_CTL		0x95
+#define RST_FPGA		0x96
+#define RST_L2SW		0x97
+#define RST_ICM			0x98
+#define RST_AXI_GLOBAL	0x99
+
+#endif
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 04/10] reset: Add Sunplus SP7021 reset driver
  2021-12-03  7:34 [PATCH v5 00/10] Add Sunplus SP7021 SoC Support Qin Jian
                   ` (2 preceding siblings ...)
  2021-12-03  7:34 ` [PATCH v5 03/10] dt-bindings: reset: Add bindings for SP7021 reset driver Qin Jian
@ 2021-12-03  7:34 ` Qin Jian
  2021-12-07  8:35   ` Philipp Zabel
  2021-12-03  7:34 ` [PATCH v5 05/10] dt-bindings: clock: Add bindings for SP7021 clock driver Qin Jian
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Qin Jian @ 2021-12-03  7:34 UTC (permalink / raw)
  To: robh+dt
  Cc: mturquette, sboyd, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian

Add reset driver for Sunplus SP7021 SoC.

Signed-off-by: Qin Jian <qinjian@cqplus1.com>
---
 MAINTAINERS                   |   1 +
 drivers/reset/Kconfig         |   9 +++
 drivers/reset/Makefile        |   1 +
 drivers/reset/reset-sunplus.c | 132 ++++++++++++++++++++++++++++++++++
 4 files changed, 143 insertions(+)
 create mode 100644 drivers/reset/reset-sunplus.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 652f42cab..6caffd6d0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2662,6 +2662,7 @@ S:	Maintained
 W:	https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
 F:	Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
 F:	Documentation/devicetree/bindings/reset/sunplus,reset.yaml
+F:	drivers/reset/reset-sunplus.c
 F:	include/dt-bindings/reset/sp-sp7021.h
 
 ARM/Synaptics SoC support
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index be799a5ab..5cdd022ad 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -224,6 +224,15 @@ config RESET_SOCFPGA
 	  This enables the reset driver for the SoCFPGA ARMv7 platforms. This
 	  driver gets initialized early during platform init calls.
 
+config RESET_SUNPLUS
+	bool "Sunplus SoCs Reset Driver"
+	depends on ARCH_SUNPLUS || COMPILE_TEST
+	help
+	  This enables the reset driver support for Sunplus SP7021 SoC family.
+	  Say Y if you want to control reset signals by the reset controller.
+	  Otherwise, say N.
+	  This driver is selected automatically by arm/mach-sunplus platform config.
+
 config RESET_SUNXI
 	bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
 	default ARCH_SUNXI
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 21d46d886..f03403e97 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_RESET_RZG2L_USBPHY_CTRL) += reset-rzg2l-usbphy-ctrl.o
 obj-$(CONFIG_RESET_SCMI) += reset-scmi.o
 obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
 obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
+obj-$(CONFIG_RESET_SUNPLUS) += reset-sunplus.o
 obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
 obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
diff --git a/drivers/reset/reset-sunplus.c b/drivers/reset/reset-sunplus.c
new file mode 100644
index 000000000..a1d88dbaf
--- /dev/null
+++ b/drivers/reset/reset-sunplus.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * SP7021 reset driver
+ *
+ * Copyright (C) Sunplus Technology Co., Ltd.
+ *       All rights reserved.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/reboot.h>
+
+/* HIWORD_MASK_REG BITS */
+#define BITS_PER_HWM_REG	16
+
+struct sp_reset_data {
+	struct reset_controller_dev rcdev;
+	void __iomem *membase;
+} *sp_reset;
+
+static inline struct sp_reset_data *to_sp_reset_data(struct reset_controller_dev
+						     *rcdev)
+{
+	return container_of(rcdev, struct sp_reset_data, rcdev);
+}
+
+static int sp_reset_update(struct reset_controller_dev *rcdev,
+			   unsigned long id, bool assert)
+{
+	struct sp_reset_data *data = to_sp_reset_data(rcdev);
+	int index = id / BITS_PER_HWM_REG;
+	int shift = id % BITS_PER_HWM_REG;
+	void __iomem *addr = data->membase + (index * 4);
+	u32 value;
+
+	value = (1 << (16 + shift)) | (assert << shift);
+	writel(value, addr);
+
+	return 0;
+}
+
+static int sp_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	return sp_reset_update(rcdev, id, true);
+}
+
+static int sp_reset_deassert(struct reset_controller_dev *rcdev,
+			     unsigned long id)
+{
+	return sp_reset_update(rcdev, id, false);
+}
+
+static int sp_reset_status(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct sp_reset_data *data = to_sp_reset_data(rcdev);
+	int index = id / BITS_PER_HWM_REG;
+	int shift = id % BITS_PER_HWM_REG;
+	u32 reg;
+
+	reg = readl(data->membase + (index * 4));
+
+	return !!(reg & BIT(shift));
+}
+
+static int sp_restart(struct notifier_block *this, unsigned long mode,
+		      void *cmd)
+{
+	sp_reset_assert(&sp_reset->rcdev, 0);
+	sp_reset_deassert(&sp_reset->rcdev, 0);
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block sp_restart_nb = {
+	.notifier_call = sp_restart,
+	.priority = 192,
+};
+
+static const struct reset_control_ops sp_reset_ops = {
+	.assert   = sp_reset_assert,
+	.deassert = sp_reset_deassert,
+	.status   = sp_reset_status,
+};
+
+static const struct of_device_id sp_reset_dt_ids[] = {
+	{.compatible = "sunplus,sp7021-reset",},
+	{ /* sentinel */ },
+};
+
+static int sp_reset_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	void __iomem *membase;
+	struct resource *res;
+
+	sp_reset = devm_kzalloc(&pdev->dev, sizeof(*sp_reset), GFP_KERNEL);
+	if (!sp_reset)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	membase = devm_ioremap_resource(dev, res);
+	if (IS_ERR(membase))
+		return PTR_ERR(membase);
+
+	sp_reset->membase = membase;
+	sp_reset->rcdev.owner = THIS_MODULE;
+	sp_reset->rcdev.nr_resets = resource_size(res) / 4 * 16;	/* HIWORD_MASK */
+	sp_reset->rcdev.ops = &sp_reset_ops;
+	sp_reset->rcdev.of_node = dev->of_node;
+	register_restart_handler(&sp_restart_nb);
+
+	return devm_reset_controller_register(dev, &sp_reset->rcdev);
+}
+
+static struct platform_driver sp_reset_driver = {
+	.probe = sp_reset_probe,
+	.driver = {
+		   .name = "sunplus-reset",
+		   .of_match_table = sp_reset_dt_ids,
+		   },
+};
+
+module_platform_driver(sp_reset_driver);
+
+MODULE_AUTHOR("Edwin Chiu <edwin.chiu@sunplus.com>");
+MODULE_DESCRIPTION("Sunplus Reset Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 05/10] dt-bindings: clock: Add bindings for SP7021 clock driver
  2021-12-03  7:34 [PATCH v5 00/10] Add Sunplus SP7021 SoC Support Qin Jian
                   ` (3 preceding siblings ...)
  2021-12-03  7:34 ` [PATCH v5 04/10] reset: Add Sunplus " Qin Jian
@ 2021-12-03  7:34 ` Qin Jian
  2021-12-16  6:43   ` Stephen Boyd
  2021-12-03  7:34 ` [PATCH v5 06/10] clk: Add Sunplus " Qin Jian
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Qin Jian @ 2021-12-03  7:34 UTC (permalink / raw)
  To: robh+dt
  Cc: mturquette, sboyd, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian, Rob Herring

Add documentation to describe Sunplus SP7021 clock driver bindings.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Qin Jian <qinjian@cqplus1.com>
---
 .../bindings/clock/sunplus,sp7021-clkc.yaml   |  38 ++++++
 MAINTAINERS                                   |   2 +
 include/dt-bindings/clock/sp-sp7021.h         | 112 ++++++++++++++++++
 3 files changed, 152 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
 create mode 100644 include/dt-bindings/clock/sp-sp7021.h

diff --git a/Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml b/Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
new file mode 100644
index 000000000..1ce7e41d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) Sunplus Co., Ltd. 2021
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/sunplus,sp7021-clkc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sunplus SP7021 SoC Clock Controller Binding
+
+maintainers:
+  - Qin Jian <qinjian@cqplus1.com>
+
+properties:
+  compatible:
+    const: sunplus,sp7021-clkc
+
+  "#clock-cells":
+    const: 1
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - "#clock-cells"
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    clkc: clkc@9c000000 {
+      compatible = "sunplus,sp7021-clkc";
+      #clock-cells = <1>;
+      reg = <0x9c000000 0x280>;
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 6caffd6d0..90ebb823f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2661,8 +2661,10 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for mon-subscribers)
 S:	Maintained
 W:	https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
 F:	Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
+F:	Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
 F:	Documentation/devicetree/bindings/reset/sunplus,reset.yaml
 F:	drivers/reset/reset-sunplus.c
+F:	include/dt-bindings/clock/sp-sp7021.h
 F:	include/dt-bindings/reset/sp-sp7021.h
 
 ARM/Synaptics SoC support
diff --git a/include/dt-bindings/clock/sp-sp7021.h b/include/dt-bindings/clock/sp-sp7021.h
new file mode 100644
index 000000000..778a1df1c
--- /dev/null
+++ b/include/dt-bindings/clock/sp-sp7021.h
@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (C) Sunplus Technology Co., Ltd.
+ *       All rights reserved.
+ */
+#ifndef _DT_BINDINGS_CLOCK_SUNPLUS_SP7021_H
+#define _DT_BINDINGS_CLOCK_SUNPLUS_SP7021_H
+
+#define XTAL			27000000
+
+/* plls */
+#define PLL_A			0
+#define PLL_E			1
+#define PLL_E_2P5		2
+#define PLL_E_25		3
+#define PLL_E_112P5		4
+#define PLL_F			5
+#define PLL_TV			6
+#define PLL_TV_A		7
+#define PLL_SYS			8
+
+/* gates: mo_clken0 ~ mo_clken9 */
+#define CLK_SYSTEM		0x10
+#define CLK_RTC			0x12
+#define CLK_IOCTL		0x13
+#define CLK_IOP			0x14
+#define CLK_OTPRX		0x15
+#define CLK_NOC			0x16
+#define CLK_BR			0x17
+#define CLK_RBUS_L00	0x18
+#define CLK_SPIFL		0x19
+#define CLK_SDCTRL0		0x1a
+#define CLK_PERI0		0x1b
+#define CLK_A926		0x1d
+#define CLK_UMCTL2		0x1e
+#define CLK_PERI1		0x1f
+
+#define CLK_DDR_PHY0	0x20
+#define CLK_ACHIP		0x22
+#define CLK_STC0		0x24
+#define CLK_STC_AV0		0x25
+#define CLK_STC_AV1		0x26
+#define CLK_STC_AV2		0x27
+#define CLK_UA0			0x28
+#define CLK_UA1			0x29
+#define CLK_UA2			0x2a
+#define CLK_UA3			0x2b
+#define CLK_UA4			0x2c
+#define CLK_HWUA		0x2d
+#define CLK_DDC0		0x2e
+#define CLK_UADMA		0x2f
+
+#define CLK_CBDMA0		0x30
+#define CLK_CBDMA1		0x31
+#define CLK_SPI_COMBO_0	0x32
+#define CLK_SPI_COMBO_1	0x33
+#define CLK_SPI_COMBO_2	0x34
+#define CLK_SPI_COMBO_3	0x35
+#define CLK_AUD			0x36
+#define CLK_USBC0		0x3a
+#define CLK_USBC1		0x3b
+#define CLK_UPHY0		0x3d
+#define CLK_UPHY1		0x3e
+
+#define CLK_I2CM0		0x40
+#define CLK_I2CM1		0x41
+#define CLK_I2CM2		0x42
+#define CLK_I2CM3		0x43
+#define CLK_PMC			0x4d
+#define CLK_CARD_CTL0	0x4e
+#define CLK_CARD_CTL1	0x4f
+
+#define CLK_CARD_CTL4	0x52
+#define CLK_BCH			0x54
+#define CLK_DDFCH		0x5b
+#define CLK_CSIIW0		0x5c
+#define CLK_CSIIW1		0x5d
+#define CLK_MIPICSI0	0x5e
+#define CLK_MIPICSI1	0x5f
+
+#define CLK_HDMI_TX		0x60
+#define CLK_VPOST		0x65
+
+#define CLK_TGEN		0x70
+#define CLK_DMIX		0x71
+#define CLK_TCON		0x7a
+#define CLK_INTERRUPT	0x7f
+
+#define CLK_RGST		0x80
+#define CLK_GPIO		0x83
+#define CLK_RBUS_TOP	0x84
+
+#define CLK_MAILBOX		0x96
+#define CLK_SPIND		0x9a
+#define CLK_I2C2CBUS	0x9b
+#define CLK_SEC			0x9d
+#define CLK_DVE			0x9e
+#define CLK_GPOST0		0x9f
+
+#define CLK_OSD0		0xa0
+#define CLK_DISP_PWM	0xa2
+#define CLK_UADBG		0xa3
+#define CLK_DUMMY_MASTER	0xa4
+#define CLK_FIO_CTL		0xa5
+#define CLK_FPGA		0xa6
+#define CLK_L2SW		0xa7
+#define CLK_ICM			0xa8
+#define CLK_AXI_GLOBAL	0xa9
+
+#define CLK_MAX			0xb0
+
+#endif
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 06/10] clk: Add Sunplus SP7021 clock driver
  2021-12-03  7:34 [PATCH v5 00/10] Add Sunplus SP7021 SoC Support Qin Jian
                   ` (4 preceding siblings ...)
  2021-12-03  7:34 ` [PATCH v5 05/10] dt-bindings: clock: Add bindings for SP7021 clock driver Qin Jian
@ 2021-12-03  7:34 ` Qin Jian
  2021-12-16  6:42   ` Stephen Boyd
  2021-12-03  7:34 ` [PATCH v5 07/10] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller Qin Jian
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Qin Jian @ 2021-12-03  7:34 UTC (permalink / raw)
  To: robh+dt
  Cc: mturquette, sboyd, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian

Add clock driver for Sunplus SP7021 SoC.

Signed-off-by: Qin Jian <qinjian@cqplus1.com>
---
 MAINTAINERS              |   1 +
 drivers/clk/Kconfig      |   9 +
 drivers/clk/Makefile     |   1 +
 drivers/clk/clk-sp7021.c | 738 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 749 insertions(+)
 create mode 100644 drivers/clk/clk-sp7021.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 90ebb823f..5069f552f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2663,6 +2663,7 @@ W:	https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
 F:	Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
 F:	Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
 F:	Documentation/devicetree/bindings/reset/sunplus,reset.yaml
+F:	drivers/clk/clk-sp7021.c
 F:	drivers/reset/reset-sunplus.c
 F:	include/dt-bindings/clock/sp-sp7021.h
 F:	include/dt-bindings/reset/sp-sp7021.h
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index c5b3dc973..b92a176c7 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -334,6 +334,15 @@ config COMMON_CLK_VC5
 	  This driver supports the IDT VersaClock 5 and VersaClock 6
 	  programmable clock generators.
 
+config COMMON_CLK_SP7021
+	bool "Clock driver for Sunplus SP7021 SoC"
+	help
+	  This driver supports the Sunplus SP7021 SoC clocks.
+	  It implements SP7021 PLLs/gate.
+	  Not all features of the PLL are currently supported
+	  by the driver.
+	  This driver is selected automatically by platform config.
+
 config COMMON_CLK_STM32MP157
 	def_bool COMMON_CLK && MACH_STM32MP157
 	help
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index e42312121..f15bb5070 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_COMMON_CLK_SI5351)		+= clk-si5351.o
 obj-$(CONFIG_COMMON_CLK_SI514)		+= clk-si514.o
 obj-$(CONFIG_COMMON_CLK_SI544)		+= clk-si544.o
 obj-$(CONFIG_COMMON_CLK_SI570)		+= clk-si570.o
+obj-$(CONFIG_COMMON_CLK_SP7021)		+= clk-sp7021.o
 obj-$(CONFIG_COMMON_CLK_STM32F)		+= clk-stm32f4.o
 obj-$(CONFIG_COMMON_CLK_STM32H7)	+= clk-stm32h7.o
 obj-$(CONFIG_COMMON_CLK_STM32MP157)	+= clk-stm32mp1.o
diff --git a/drivers/clk/clk-sp7021.c b/drivers/clk/clk-sp7021.c
new file mode 100644
index 000000000..040578e82
--- /dev/null
+++ b/drivers/clk/clk-sp7021.c
@@ -0,0 +1,738 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) Sunplus Technology Co., Ltd.
+ *       All rights reserved.
+ */
+//#define DEBUG
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/bitfield.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <dt-bindings/clock/sp-sp7021.h>
+
+#ifndef clk_readl
+#define clk_readl  readl
+#define clk_writel writel
+#endif
+
+#define REG(i)		(pll_regs + (i) * 4)
+
+#define PLLA_CTL	REG(7)
+#define PLLE_CTL	REG(12)
+#define PLLF_CTL	REG(13)
+#define PLLTV_CTL	REG(14)
+#define PLLSYS_CTL	REG(26)
+
+#define EXTCLK		"extclk"
+
+/* speical div_width values for PLLTV/PLLA */
+#define DIV_TV		33
+#define DIV_A		34
+
+/* PLLTV parameters */
+enum {
+	SEL_FRA,
+	SDM_MOD,
+	PH_SEL,
+	NFRA,
+	DIVR,
+	DIVN,
+	DIVM,
+	P_MAX
+};
+
+#define MASK_SEL_FRA	GENMASK(1, 1)
+#define MASK_SDM_MOD	GENMASK(2, 2)
+#define MASK_PH_SEL		GENMASK(4, 4)
+#define MASK_NFRA		GENMASK(12, 6)
+#define MASK_DIVR		GENMASK(8, 7)
+#define MASK_DIVN		GENMASK(7, 0)
+#define MASK_DIVM		GENMASK(14, 8)
+
+/* HIWORD_MASK FIELD_PREP */
+#define HWM_FIELD_PREP(mask, value) \
+({\
+	u32 m = mask; \
+	(m << 16) | FIELD_PREP(m, value); \
+})
+
+struct sp_pll {
+	struct clk_hw hw;
+	void __iomem *reg;
+	spinlock_t *lock;	/* lock for reg */
+	int div_shift;
+	int div_width;
+	int pd_bit;		/* power down bit idx */
+	int bp_bit;		/* bypass bit idx */
+	unsigned long brate;	/* base rate, TODO: replace brate with muldiv */
+	u32 p[P_MAX];	/* for hold PLLTV/PLLA parameters */
+};
+
+#define to_sp_pll(_hw)	container_of(_hw, struct sp_pll, hw)
+
+#define clk_regs	(moon_regs + 0x000) /* G0 ~ CLKEN */
+#define pll_regs	(moon_regs + 0x200) /* G4 ~ PLL */
+static void __iomem *moon_regs;
+
+#define P_EXTCLK	BIT(16)
+static const char * const parents[] = {
+	"pllsys",
+	"extclk",
+};
+
+static const u32 gates[] = {
+	CLK_SYSTEM,
+	CLK_RTC,
+	CLK_IOCTL,
+	CLK_IOP,
+	CLK_OTPRX,
+	CLK_NOC,
+	CLK_BR,
+	CLK_RBUS_L00,
+	CLK_SPIFL,
+	CLK_SDCTRL0,
+	CLK_PERI0 | P_EXTCLK,
+	CLK_A926,
+	CLK_UMCTL2,
+	CLK_PERI1 | P_EXTCLK,
+
+	CLK_DDR_PHY0,
+	CLK_ACHIP,
+	CLK_STC0,
+	CLK_STC_AV0,
+	CLK_STC_AV1,
+	CLK_STC_AV2,
+	CLK_UA0 | P_EXTCLK,
+	CLK_UA1 | P_EXTCLK,
+	CLK_UA2 | P_EXTCLK,
+	CLK_UA3 | P_EXTCLK,
+	CLK_UA4 | P_EXTCLK,
+	CLK_HWUA | P_EXTCLK,
+	CLK_DDC0,
+	CLK_UADMA | P_EXTCLK,
+
+	CLK_CBDMA0,
+	CLK_CBDMA1,
+	CLK_SPI_COMBO_0,
+	CLK_SPI_COMBO_1,
+	CLK_SPI_COMBO_2,
+	CLK_SPI_COMBO_3,
+	CLK_AUD,
+	CLK_USBC0,
+	CLK_USBC1,
+	CLK_UPHY0,
+	CLK_UPHY1,
+
+	CLK_I2CM0,
+	CLK_I2CM1,
+	CLK_I2CM2,
+	CLK_I2CM3,
+	CLK_PMC,
+	CLK_CARD_CTL0,
+	CLK_CARD_CTL1,
+
+	CLK_CARD_CTL4,
+	CLK_BCH,
+	CLK_DDFCH,
+	CLK_CSIIW0,
+	CLK_CSIIW1,
+	CLK_MIPICSI0,
+	CLK_MIPICSI1,
+
+	CLK_HDMI_TX,
+	CLK_VPOST,
+
+	CLK_TGEN,
+	CLK_DMIX,
+	CLK_TCON,
+	CLK_INTERRUPT,
+
+	CLK_RGST,
+	CLK_GPIO,
+	CLK_RBUS_TOP,
+
+	CLK_MAILBOX,
+	CLK_SPIND,
+	CLK_I2C2CBUS,
+	CLK_SEC,
+	CLK_GPOST0,
+	CLK_DVE,
+
+	CLK_OSD0,
+	CLK_DISP_PWM,
+	CLK_UADBG,
+	CLK_DUMMY_MASTER,
+	CLK_FIO_CTL,
+	CLK_FPGA,
+	CLK_L2SW,
+	CLK_ICM,
+	CLK_AXI_GLOBAL,
+};
+
+static struct clk *clks[CLK_MAX];
+static struct clk_onecell_data clk_data;
+
+static DEFINE_SPINLOCK(plla_lock);
+static DEFINE_SPINLOCK(plle_lock);
+static DEFINE_SPINLOCK(pllf_lock);
+static DEFINE_SPINLOCK(pllsys_lock);
+static DEFINE_SPINLOCK(plltv_lock);
+
+#define _M			1000000UL
+#define F_27M		(27 * _M)
+
+/******************************************** PLL_TV *******************************************/
+
+//#define PLLTV_STEP_DIR (?) /* Unit: HZ */
+
+/* TODO: set proper FVCO range */
+#define FVCO_MIN	(100 * _M)
+#define FVCO_MAX	(200 * _M)
+
+#define F_MIN		(FVCO_MIN / 8)
+#define F_MAX		(FVCO_MAX)
+
+static long plltv_integer_div(struct sp_pll *clk, unsigned long freq)
+{
+	/* valid m values: 27M must be divisible by m, 0 means end */
+	static const u32 m_table[] = {
+		1, 2, 3, 4, 5, 6, 8, 9, 10, 12, 15, 16, 18, 20, 24, 25, 27, 30, 32, 0
+	};
+	u32 m, n, r;
+#ifdef PLLTV_STEP_DIR
+	u32 step = (PLLTV_STEP_DIR > 0) ? PLLTV_STEP_DIR : -PLLTV_STEP_DIR;
+	int calc_times = 1000000 / step;
+#endif
+	unsigned long fvco, nf;
+
+	/* check freq */
+	if (freq < F_MIN) {
+		pr_warn("%s: %s freq:%lu < F_MIN:%lu, round up\n",
+			__func__, clk_hw_get_name(&clk->hw), freq, F_MIN);
+		freq = F_MIN;
+	} else if (freq > F_MAX) {
+		pr_warn("%s: %s freq:%lu > F_MAX:%lu, round down\n",
+			__func__, clk_hw_get_name(&clk->hw), freq, F_MAX);
+		freq = F_MAX;
+	}
+
+#ifdef PLLTV_STEP_DIR
+	if ((freq % step) != 0)
+		freq += step - (freq % step) + ((PLLTV_STEP_DIR > 0) ? 0 : PLLTV_STEP_DIR);
+#endif
+
+#ifdef PLLTV_STEP_DIR
+CALC:
+	if (!calc_times) {
+		pr_err("%s: %s freq:%lu out of recalc times\n",
+		       __func__, clk_hw_get_name(&clk->hw), freq);
+		return -ETIMEOUT;
+	}
+#endif
+
+	/* DIVR 0~3 */
+	for (r = 0; r <= 3; r++) {
+		fvco = freq << r;
+		if (fvco <= FVCO_MAX)
+			break;
+	}
+
+	/* DIVM */
+	for (m = 0; m_table[m]; m++) {
+		nf = fvco * m_table[m];
+		n = nf / F_27M;
+		if ((n * F_27M) == nf)
+			break;
+	}
+	m = m_table[m];
+
+	if (!m) {
+#ifdef PLLTV_STEP_DIR
+		freq += PLLTV_STEP_DIR;
+		calc_times--;
+		goto CALC;
+#else
+		pr_err("%s: %s freq:%lu not found a valid setting\n",
+		       __func__, clk_hw_get_name(&clk->hw), freq);
+		return -EINVAL;
+#endif
+	}
+
+	/* save parameters */
+	clk->p[SEL_FRA] = 0;
+	clk->p[DIVR]    = r;
+	clk->p[DIVN]    = n;
+	clk->p[DIVM]    = m;
+
+	return freq;
+}
+
+/* parameters for PLLTV fractional divider */
+static const u32 pt[][5] = {
+	/* conventional fractional */
+	{
+		1,			// factor
+		5,			// 5 * p0 (nint)
+		1,			// 1 * p0
+		F_27M,			// F_27M / p0
+		1,			// p0 / p2
+	},
+	/* phase rotation */
+	{
+		10,			// factor
+		54,			// 5.4 * p0 (nint)
+		2,			// 0.2 * p0
+		F_27M / 10,		// F_27M / p0
+		5,			// p0 / p2
+	},
+};
+
+static const u32 mods[] = { 91, 55 }; /* SDM_MOD mod values */
+
+static long plltv_fractional_div(struct sp_pll *clk, unsigned long freq)
+{
+	u32 m, r;
+	u32 nint, nfra;
+	u32 diff_min_quotient = 210000000, diff_min_remainder = 0;
+	u32 diff_min_sign = 0;
+	unsigned long fvco, nf, f, fout = 0;
+	int sdm, ph;
+
+	/* check freq */
+	if (freq < F_MIN) {
+		pr_warn("%s: %s freq:%lu < F_MIN:%lu, round up\n",
+			__func__, clk_hw_get_name(&clk->hw), freq, F_MIN);
+		freq = F_MIN;
+	} else if (freq > F_MAX) {
+		pr_warn("%s: %s freq:%lu > F_MAX:%lu, round down\n",
+			__func__, clk_hw_get_name(&clk->hw), freq, F_MAX);
+		freq = F_MAX;
+	}
+
+	/* DIVR 0~3 */
+	for (r = 0; r <= 3; r++) {
+		fvco = freq << r;
+		if (fvco <= FVCO_MAX)
+			break;
+	}
+	f = F_27M >> r;
+
+	/* PH_SEL 1/0 */
+	for (ph = 1; ph >= 0; ph--) {
+		const u32 *pp = pt[ph];
+		u32 ms = 1;
+
+		/* SDM_MOD 0/1 */
+		for (sdm = 0; sdm <= 1; sdm++) {
+			u32 mod = mods[sdm];
+
+			/* DIVM 1~32 */
+			for (m = ms; m <= 32; m++) {
+				u32 diff_freq;
+				u32 diff_freq_quotient = 0, diff_freq_remainder = 0;
+				u32 diff_freq_sign = 0; /* 0:Positive number, 1:Negative number */
+
+				nf = fvco * m;
+				nint = nf / pp[3];
+
+				if (nint < pp[1])
+					continue;
+				if (nint > pp[1])
+					break;
+
+				nfra = (((nf % pp[3]) * mod * pp[4]) + (F_27M / 2)) / F_27M;
+				if (nfra)
+					diff_freq = (f * (nint + pp[2]) / pp[0]) -
+								(f * (mod - nfra) / mod / pp[4]);
+				else
+					diff_freq = (f * (nint) / pp[0]);
+
+				diff_freq_quotient  = diff_freq / m;
+				diff_freq_remainder = ((diff_freq % m) * 1000) / m;
+
+				if (freq > diff_freq_quotient) {
+					diff_freq_quotient  = freq - diff_freq_quotient - 1;
+					diff_freq_remainder = 1000 - diff_freq_remainder;
+					diff_freq_sign = 1;
+				} else {
+					diff_freq_quotient = diff_freq_quotient - freq;
+					diff_freq_sign = 0;
+				}
+
+				if (diff_min_quotient > diff_freq_quotient ||
+				    (diff_min_quotient == diff_freq_quotient &&
+				    diff_min_remainder > diff_freq_remainder)) {
+					/* found a closer freq, save parameters */
+					clk->p[SEL_FRA] = 1;
+					clk->p[SDM_MOD] = sdm;
+					clk->p[PH_SEL]  = ph;
+					clk->p[NFRA]    = nfra;
+					clk->p[DIVR]    = r;
+					clk->p[DIVM]    = m;
+
+					fout = diff_freq / m;
+					diff_min_quotient = diff_freq_quotient;
+					diff_min_remainder = diff_freq_remainder;
+					diff_min_sign = diff_freq_sign;
+				}
+			}
+		}
+	}
+
+	if (!fout) {
+		pr_err("%s: %s freq:%lu not found a valid setting\n",
+		       __func__, clk_hw_get_name(&clk->hw), freq);
+		return -EINVAL;
+	}
+
+	return fout;
+}
+
+static long plltv_div(struct sp_pll *clk, unsigned long freq)
+{
+	if (freq % 100)
+		return plltv_fractional_div(clk, freq);
+	else
+		return plltv_integer_div(clk, freq);
+}
+
+static void plltv_set_rate(struct sp_pll *clk)
+{
+	u32 reg;
+
+	reg  = HWM_FIELD_PREP(MASK_SEL_FRA, clk->p[SEL_FRA]);
+	reg |= HWM_FIELD_PREP(MASK_SDM_MOD, clk->p[SDM_MOD]);
+	reg |= HWM_FIELD_PREP(MASK_PH_SEL, clk->p[PH_SEL]);
+	reg |= HWM_FIELD_PREP(MASK_NFRA, clk->p[NFRA]);
+	clk_writel(reg, clk->reg);
+
+	reg  = HWM_FIELD_PREP(MASK_DIVR, clk->p[DIVR]);
+	clk_writel(reg, clk->reg + 4);
+
+	reg  = HWM_FIELD_PREP(MASK_DIVN, clk->p[DIVN] - 1);
+	reg |= HWM_FIELD_PREP(MASK_DIVM, clk->p[DIVM] - 1);
+	clk_writel(reg, clk->reg + 8);
+}
+
+/******************************************** PLL_A ********************************************/
+
+/* from Q628_PLLs_REG_setting.xlsx */
+struct {
+	u32 rate;
+	u32 regs[5];
+} pa[] = {
+	{
+		.rate = 135475200,
+		.regs = {
+			0x4801,
+			0x02df,
+			0x248f,
+			0x0211,
+			0x33e9
+		}
+	},
+	{
+		.rate = 147456000,
+		.regs = {
+			0x4801,
+			0x1adf,
+			0x2490,
+			0x0349,
+			0x33e9
+		}
+	},
+	{
+		.rate = 196608000,
+		.regs = {
+			0x4801,
+			0x42ef,
+			0x2495,
+			0x01c6,
+			0x33e9
+		}
+	},
+};
+
+static void plla_set_rate(struct sp_pll *clk)
+{
+	const u32 *pp = pa[clk->p[0]].regs;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(pa->regs); i++)
+		clk_writel(0xffff0000 | pp[i], clk->reg + (i * 4));
+}
+
+static long plla_round_rate(struct sp_pll *clk, unsigned long rate)
+{
+	int i = ARRAY_SIZE(pa);
+
+	while (--i) {
+		if (rate >= pa[i].rate)
+			break;
+	}
+	clk->p[0] = i;
+	return pa[i].rate;
+}
+
+/******************************************* SP_PLL ********************************************/
+
+static long sp_pll_calc_div(struct sp_pll *clk, unsigned long rate)
+{
+	u32 fbdiv;
+	u32 max = 1 << clk->div_width;
+
+	fbdiv = DIV_ROUND_CLOSEST(rate, clk->brate);
+	if (fbdiv > max)
+		fbdiv = max;
+	return fbdiv;
+}
+
+static long sp_pll_round_rate(struct clk_hw *hw, unsigned long rate,
+			      unsigned long *prate)
+{
+	struct sp_pll *clk = to_sp_pll(hw);
+	long ret;
+
+	if (rate == *prate)
+		ret = *prate; /* bypass */
+	else if (clk->div_width == DIV_A) {
+		ret = plla_round_rate(clk, rate);
+	} else if (clk->div_width == DIV_TV) {
+		ret = plltv_div(clk, rate);
+		if (ret < 0)
+			ret = *prate;
+	} else {
+		ret = sp_pll_calc_div(clk, rate) * clk->brate;
+	}
+
+	return ret;
+}
+
+static unsigned long sp_pll_recalc_rate(struct clk_hw *hw,
+					unsigned long prate)
+{
+	struct sp_pll *clk = to_sp_pll(hw);
+	u32 reg = clk_readl(clk->reg);
+	unsigned long ret;
+
+	if (reg & BIT(clk->bp_bit)) {
+		ret = prate; /* bypass */
+	} else if (clk->div_width == DIV_A) {
+		ret = pa[clk->p[0]].rate;
+	} else if (clk->div_width == DIV_TV) {
+		u32 m, r, reg2;
+
+		r = FIELD_GET(MASK_DIVR, clk_readl(clk->reg + 4));
+		reg2 = clk_readl(clk->reg + 8);
+		m = FIELD_GET(MASK_DIVM, reg2) + 1;
+
+		if (reg & BIT(1)) { /* SEL_FRA */
+			/* fractional divider */
+			u32 sdm  = FIELD_GET(MASK_SDM_MOD, reg);
+			u32 ph   = FIELD_GET(MASK_PH_SEL, reg);
+			u32 nfra = FIELD_GET(MASK_NFRA, reg);
+			const u32 *pp = pt[ph];
+
+			ret = prate >> r;
+			ret = (ret * (pp[1] + pp[2]) / pp[0]) -
+				  (ret * (mods[sdm] - nfra) / mods[sdm] / pp[4]);
+			ret /= m;
+		} else {
+			/* integer divider */
+			u32 n = FIELD_GET(MASK_DIVN, reg2) + 1;
+
+			ret = (prate / m * n) >> r;
+		}
+	} else {
+		u32 fbdiv = (reg >> clk->div_shift) & ((1 << clk->div_width) - 1);
+
+		ret = clk->brate * fbdiv;
+	}
+
+	return ret;
+}
+
+static int sp_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+			   unsigned long prate)
+{
+	struct sp_pll *clk = to_sp_pll(hw);
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(clk->lock, flags);
+
+	reg = BIT(clk->bp_bit + 16); /* HIWORD_MASK */
+
+	if (rate == prate)
+		reg |= BIT(clk->bp_bit); /* bypass */
+	else if (clk->div_width == DIV_A)
+		plla_set_rate(clk);
+	else if (clk->div_width == DIV_TV)
+		plltv_set_rate(clk);
+	else if (clk->div_width) {
+		u32 fbdiv = sp_pll_calc_div(clk, rate);
+		u32 mask = GENMASK(clk->div_shift + clk->div_width - 1, clk->div_shift);
+
+		reg |= (mask << 16) | (((fbdiv - 1) << clk->div_shift) & mask);
+	}
+
+	clk_writel(reg, clk->reg);
+
+	spin_unlock_irqrestore(clk->lock, flags);
+
+	return 0;
+}
+
+static int sp_pll_enable(struct clk_hw *hw)
+{
+	struct sp_pll *clk = to_sp_pll(hw);
+	unsigned long flags;
+
+	spin_lock_irqsave(clk->lock, flags);
+	clk_writel(BIT(clk->pd_bit + 16) | BIT(clk->pd_bit), clk->reg); /* power up */
+	spin_unlock_irqrestore(clk->lock, flags);
+
+	return 0;
+}
+
+static void sp_pll_disable(struct clk_hw *hw)
+{
+	struct sp_pll *clk = to_sp_pll(hw);
+	unsigned long flags;
+
+	spin_lock_irqsave(clk->lock, flags);
+	clk_writel(BIT(clk->pd_bit + 16), clk->reg); /* power down */
+	spin_unlock_irqrestore(clk->lock, flags);
+}
+
+static int sp_pll_is_enabled(struct clk_hw *hw)
+{
+	struct sp_pll *clk = to_sp_pll(hw);
+
+	return clk_readl(clk->reg) & BIT(clk->pd_bit);
+}
+
+static const struct clk_ops sp_pll_ops = {
+	.enable = sp_pll_enable,
+	.disable = sp_pll_disable,
+	.is_enabled = sp_pll_is_enabled,
+	.round_rate = sp_pll_round_rate,
+	.recalc_rate = sp_pll_recalc_rate,
+	.set_rate = sp_pll_set_rate
+};
+
+static const struct clk_ops sp_pll_sub_ops = {
+	.enable = sp_pll_enable,
+	.disable = sp_pll_disable,
+	.is_enabled = sp_pll_is_enabled,
+	.recalc_rate = sp_pll_recalc_rate,
+};
+
+struct clk *clk_register_sp_pll(const char *name, const char *parent,
+				void __iomem *reg, int pd_bit, int bp_bit,
+				unsigned long brate, int shift, int width,
+				spinlock_t *lock)
+{
+	struct sp_pll *pll;
+	struct clk *clk;
+	struct clk_init_data initd = {
+		.name = name,
+		.parent_names = &parent,
+		.ops = (bp_bit >= 0) ? &sp_pll_ops : &sp_pll_sub_ops,
+		.num_parents = 1,
+		.flags = CLK_IGNORE_UNUSED
+	};
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	if (reg == PLLSYS_CTL)
+		initd.flags |= CLK_IS_CRITICAL;
+
+	pll->hw.init = &initd;
+	pll->reg = reg;
+	pll->pd_bit = pd_bit;
+	pll->bp_bit = bp_bit;
+	pll->brate = brate;
+	pll->div_shift = shift;
+	pll->div_width = width;
+	pll->lock = lock;
+
+	clk = clk_register(NULL, &pll->hw);
+	if (WARN_ON(IS_ERR(clk))) {
+		kfree(pll);
+	} else {
+		pr_info("%-20s%lu\n", name, clk_get_rate(clk));
+		clk_register_clkdev(clk, NULL, name);
+	}
+
+	return clk;
+}
+
+static void __init sp_clk_setup(struct device_node *np)
+{
+	int i, j;
+
+	moon_regs = of_iomap(np, 0);
+	if (WARN_ON(!moon_regs))
+		return; /* -ENXIO */
+
+	/* PLL_A */
+	clks[PLL_A] = clk_register_sp_pll("plla", EXTCLK,
+					  PLLA_CTL, 11, 12, 27000000, 0, DIV_A, &plla_lock);
+
+	/* PLL_E */
+	clks[PLL_E] = clk_register_sp_pll("plle", EXTCLK,
+					  PLLE_CTL, 6, 2, 50000000, 0, 0, &plle_lock);
+	clks[PLL_E_2P5] = clk_register_sp_pll("plle_2p5", "plle",
+					      PLLE_CTL, 13, -1, 2500000, 0, 0, &plle_lock);
+	clks[PLL_E_25] = clk_register_sp_pll("plle_25", "plle",
+					     PLLE_CTL, 12, -1, 25000000, 0, 0, &plle_lock);
+	clks[PLL_E_112P5] = clk_register_sp_pll("plle_112p5", "plle",
+						PLLE_CTL, 11, -1, 112500000, 0, 0, &plle_lock);
+
+	/* PLL_F */
+	clks[PLL_F] = clk_register_sp_pll("pllf", EXTCLK,
+					  PLLF_CTL, 0, 10, 13500000, 1, 4, &pllf_lock);
+
+	/* PLL_TV */
+	clks[PLL_TV] = clk_register_sp_pll("plltv", EXTCLK,
+					   PLLTV_CTL, 0, 15, 27000000, 0, DIV_TV, &plltv_lock);
+	clks[PLL_TV_A] = clk_register_divider(NULL, "plltv_a", "plltv", 0,
+					      PLLTV_CTL + 4, 5, 1,
+					      CLK_DIVIDER_POWER_OF_TWO, &plltv_lock);
+	clk_register_clkdev(clks[PLL_TV_A], NULL, "plltv_a");
+
+	/* PLL_SYS */
+	clks[PLL_SYS] = clk_register_sp_pll("pllsys", EXTCLK,
+					    PLLSYS_CTL, 10, 9, 13500000, 0, 4, &pllsys_lock);
+
+	/* gates */
+	for (i = 0; i < ARRAY_SIZE(gates); i++) {
+		char s[10];
+
+		j = gates[i] & 0xffff;
+		sprintf(s, "clken%02x", j);
+		clks[j] = clk_register_gate(NULL, s, parents[gates[i] >> 16], CLK_IGNORE_UNUSED,
+					    clk_regs + (j >> 4) * 4, j & 0x0f,
+					    CLK_GATE_HIWORD_MASK, NULL);
+	}
+
+	clk_data.clks = clks;
+	clk_data.clk_num = ARRAY_SIZE(clks);
+	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
+}
+
+CLK_OF_DECLARE(sp_clkc, "sunplus,sp7021-clkc", sp_clk_setup);
+
+MODULE_AUTHOR("Qin Jian <qinjian@cqplus1.com>");
+MODULE_DESCRIPTION("Sunplus SP7021 Clock Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 07/10] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller
  2021-12-03  7:34 [PATCH v5 00/10] Add Sunplus SP7021 SoC Support Qin Jian
                   ` (5 preceding siblings ...)
  2021-12-03  7:34 ` [PATCH v5 06/10] clk: Add Sunplus " Qin Jian
@ 2021-12-03  7:34 ` Qin Jian
  2021-12-03  7:34 ` [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver Qin Jian
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Qin Jian @ 2021-12-03  7:34 UTC (permalink / raw)
  To: robh+dt
  Cc: mturquette, sboyd, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian, Rob Herring

Add documentation to describe Sunplus SP7021 interrupt controller bindings.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Qin Jian <qinjian@cqplus1.com>
---
 .../sunplus,sp7021-intc.yaml                  | 62 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
new file mode 100644
index 000000000..5daeab63c
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) Sunplus Co., Ltd. 2021
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/sunplus,sp7021-intc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sunplus SP7021 SoC Interrupt Controller Device Tree Bindings
+
+maintainers:
+  - Qin Jian <qinjian@cqplus1.com>
+
+properties:
+  compatible:
+    items:
+      - const: sunplus,sp7021-intc
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+    description:
+      The first cell is the IRQ number, the second cell is the trigger
+      type as defined in interrupt.txt in this directory.
+
+  reg:
+    maxItems: 2
+    description:
+      Specifies base physical address(s) and size of the controller regs.
+      The 1st region include type/polarity/priority/mask regs.
+      The 2nd region include clear/masked_ext0/masked_ext1/group regs.
+
+  interrupts:
+    maxItems: 2
+    description:
+      EXT_INT0 & EXT_INT1, 2 interrupts references to primary interrupt
+      controller.
+
+required:
+  - compatible
+  - interrupt-controller
+  - "#interrupt-cells"
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    intc: interrupt-controller@9c000780 {
+        compatible = "sunplus,sp7021-intc";
+        interrupt-controller;
+        #interrupt-cells = <2>;
+        reg = <0x9c000780 0x80>, <0x9c000a80 0x80>;
+        interrupt-parent = <&gic>;
+        interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>, /* EXT_INT0 */
+                     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>; /* EXT_INT1 */
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 5069f552f..6b3bbe021 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2662,6 +2662,7 @@ S:	Maintained
 W:	https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
 F:	Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
 F:	Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
+F:	Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
 F:	Documentation/devicetree/bindings/reset/sunplus,reset.yaml
 F:	drivers/clk/clk-sp7021.c
 F:	drivers/reset/reset-sunplus.c
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver
  2021-12-03  7:34 [PATCH v5 00/10] Add Sunplus SP7021 SoC Support Qin Jian
                   ` (6 preceding siblings ...)
  2021-12-03  7:34 ` [PATCH v5 07/10] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller Qin Jian
@ 2021-12-03  7:34 ` Qin Jian
  2021-12-07  9:02   ` Marc Zyngier
  2021-12-03  7:34 ` [PATCH v5 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC Qin Jian
  2021-12-03  7:34 ` [PATCH v5 10/10] ARM: sp7021_defconfig: Add Sunplus SP7021 defconfig Qin Jian
  9 siblings, 1 reply; 26+ messages in thread
From: Qin Jian @ 2021-12-03  7:34 UTC (permalink / raw)
  To: robh+dt
  Cc: mturquette, sboyd, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian

Add interrupt controller driver for Sunplus SP7021 SoC.

This is the interrupt controller in P-chip which collects all interrupt
sources in P-chip and routes them to parent interrupt controller in C-chip.

Signed-off-by: Qin Jian <qinjian@cqplus1.com>
---
 MAINTAINERS                       |   1 +
 drivers/irqchip/Kconfig           |   9 +
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-sp7021-intc.c | 284 ++++++++++++++++++++++++++++++
 4 files changed, 295 insertions(+)
 create mode 100644 drivers/irqchip/irq-sp7021-intc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6b3bbe021..febbd97bf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2665,6 +2665,7 @@ F:	Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
 F:	Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
 F:	Documentation/devicetree/bindings/reset/sunplus,reset.yaml
 F:	drivers/clk/clk-sp7021.c
+F:	drivers/irqchip/irq-sp7021-intc.c
 F:	drivers/reset/reset-sunplus.c
 F:	include/dt-bindings/clock/sp-sp7021.h
 F:	include/dt-bindings/reset/sp-sp7021.h
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index aca7b595c..b9429b8d0 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -602,4 +602,13 @@ config APPLE_AIC
 	  Support for the Apple Interrupt Controller found on Apple Silicon SoCs,
 	  such as the M1.
 
+config SUNPLUS_SP7021_INTC
+	bool "Sunplus SP7021 interrupt controller"
+	help
+	  Support for the Sunplus SP7021 Interrupt Controller IP core.
+	  SP7021 SoC has 2 Chips: C-Chip & P-Chip. This is used as a
+	  chained controller, routing all interrupt source in P-Chip to
+	  the primary controller on C-Chip.
+	  This is selected automatically by platform config.
+
 endmenu
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index f88cbf36a..75411f654 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -116,3 +116,4 @@ obj-$(CONFIG_MACH_REALTEK_RTL)		+= irq-realtek-rtl.o
 obj-$(CONFIG_WPCM450_AIC)		+= irq-wpcm450-aic.o
 obj-$(CONFIG_IRQ_IDT3243X)		+= irq-idt3243x.o
 obj-$(CONFIG_APPLE_AIC)			+= irq-apple-aic.o
+obj-$(CONFIG_SUNPLUS_SP7021_INTC)	+= irq-sp7021-intc.o
diff --git a/drivers/irqchip/irq-sp7021-intc.c b/drivers/irqchip/irq-sp7021-intc.c
new file mode 100644
index 000000000..beabc64d5
--- /dev/null
+++ b/drivers/irqchip/irq-sp7021-intc.c
@@ -0,0 +1,284 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) Sunplus Technology Co., Ltd.
+ *       All rights reserved.
+ */
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#define SP_INTC_HWIRQ_MIN	0
+#define SP_INTC_HWIRQ_MAX	223
+
+#define SP_INTC_NR_IRQS		(SP_INTC_HWIRQ_MAX - SP_INTC_HWIRQ_MIN + 1)
+#define SP_INTC_NR_GROUPS	DIV_ROUND_UP(SP_INTC_NR_IRQS, 32)
+#define SP_INTC_REG_SIZE	(SP_INTC_NR_GROUPS * 4)
+
+/* REG_GROUP_0 regs */
+#define REG_INTR_TYPE		(sp_intc.g0)
+#define REG_INTR_POLARITY	(REG_INTR_TYPE     + SP_INTC_REG_SIZE)
+#define REG_INTR_PRIORITY	(REG_INTR_POLARITY + SP_INTC_REG_SIZE)
+#define REG_INTR_MASK		(REG_INTR_PRIORITY + SP_INTC_REG_SIZE)
+
+/* REG_GROUP_1 regs */
+#define REG_INTR_CLEAR		(sp_intc.g1)
+#define REG_MASKED_EXT1		(REG_INTR_CLEAR    + SP_INTC_REG_SIZE)
+#define REG_MASKED_EXT0		(REG_MASKED_EXT1   + SP_INTC_REG_SIZE)
+#define REG_INTR_GROUP		(REG_INTR_CLEAR    + 31 * 4)
+
+#define GROUP_MASK			(BIT(SP_INTC_NR_GROUPS) - 1)
+#define GROUP_SHIFT_EXT1	(0)
+#define GROUP_SHIFT_EXT0	(8)
+
+/*
+ * When GPIO_INT0~7 set to edge trigger, doesn't work properly.
+ * WORKAROUND: change it to level trigger, and toggle the polarity
+ * at ACK/Handler to make the HW work.
+ */
+#define GPIO_INT0_HWIRQ		120
+#define GPIO_INT7_HWIRQ		127
+#define IS_GPIO_INT(irq)	\
+({ \
+	u32 i = irq; \
+	(i >= GPIO_INT0_HWIRQ) && (i <= GPIO_INT7_HWIRQ); \
+})
+
+/* index of states */
+enum {
+	_IS_EDGE = 0,
+	_IS_LOW,
+	_IS_ACTIVE
+};
+
+#define STATE_BIT(irq, idx)			(((irq) - GPIO_INT0_HWIRQ) * 3 + (idx))
+#define ASSIGN_STATE(irq, idx, v)	assign_bit(STATE_BIT(irq, idx), sp_intc.states, v)
+#define TEST_STATE(irq, idx)		test_bit(STATE_BIT(irq, idx), sp_intc.states)
+
+static struct sp_intctl {
+	/*
+	 * REG_GROUP_0: include type/polarity/priority/mask regs.
+	 * REG_GROUP_1: include clear/masked_ext0/masked_ext1/group regs.
+	 */
+	void __iomem *g0; // REG_GROUP_0 base
+	void __iomem *g1; // REG_GROUP_1 base
+
+	struct irq_domain *domain;
+	raw_spinlock_t lock;
+
+	/*
+	 * store GPIO_INT states
+	 * each interrupt has 3 states: is_edge, is_low, is_active
+	 */
+	DECLARE_BITMAP(states, (GPIO_INT7_HWIRQ - GPIO_INT0_HWIRQ + 1) * 3);
+} sp_intc;
+
+static struct irq_chip sp_intc_chip;
+
+static void sp_intc_assign_bit(u32 hwirq, void __iomem *base, bool value)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&sp_intc.lock, flags);
+	__assign_bit(hwirq, base, value);
+	raw_spin_unlock_irqrestore(&sp_intc.lock, flags);
+}
+
+static void sp_intc_ack_irq(struct irq_data *d)
+{
+	u32 hwirq = d->hwirq;
+
+	if (unlikely(IS_GPIO_INT(hwirq) && TEST_STATE(hwirq, _IS_EDGE))) { // WORKAROUND
+		sp_intc_assign_bit(hwirq, REG_INTR_POLARITY, !TEST_STATE(hwirq, _IS_LOW));
+		ASSIGN_STATE(hwirq, _IS_ACTIVE, true);
+	}
+
+	sp_intc_assign_bit(hwirq, REG_INTR_CLEAR, 1);
+}
+
+static void sp_intc_mask_irq(struct irq_data *d)
+{
+	sp_intc_assign_bit(d->hwirq, REG_INTR_MASK, 0);
+}
+
+static void sp_intc_unmask_irq(struct irq_data *d)
+{
+	sp_intc_assign_bit(d->hwirq, REG_INTR_MASK, 1);
+}
+
+static int sp_intc_set_type(struct irq_data *d, unsigned int type)
+{
+	u32 hwirq = d->hwirq;
+	bool is_edge = !(type & IRQ_TYPE_LEVEL_MASK);
+	bool is_low = (type == IRQ_TYPE_LEVEL_LOW || type == IRQ_TYPE_EDGE_FALLING);
+
+	irq_set_handler_locked(d, is_edge ? handle_edge_irq : handle_level_irq);
+
+	if (unlikely(IS_GPIO_INT(hwirq) && is_edge)) { // WORKAROUND
+		/* store states */
+		ASSIGN_STATE(hwirq, _IS_EDGE, is_edge);
+		ASSIGN_STATE(hwirq, _IS_LOW, is_low);
+		ASSIGN_STATE(hwirq, _IS_ACTIVE, false);
+		/* change to level */
+		is_edge = false;
+	}
+
+	sp_intc_assign_bit(hwirq, REG_INTR_TYPE, is_edge);
+	sp_intc_assign_bit(hwirq, REG_INTR_POLARITY, is_low);
+
+	return 0;
+}
+
+static int sp_intc_get_ext_irq(int ext_num)
+{
+	void __iomem *base = ext_num ? REG_MASKED_EXT1 : REG_MASKED_EXT0;
+	u32 shift = ext_num ? GROUP_SHIFT_EXT1 : GROUP_SHIFT_EXT0;
+	u32 groups;
+	u32 pending_group;
+	u32 group;
+	u32 pending_irq;
+
+	groups = readl_relaxed(REG_INTR_GROUP);
+	pending_group = (groups >> shift) & GROUP_MASK;
+	if (!pending_group)
+		return -1;
+
+	group = fls(pending_group) - 1;
+	pending_irq = readl_relaxed(base + group * 4);
+	if (!pending_irq)
+		return -1;
+
+	return (group * 32) + fls(pending_irq) - 1;
+}
+
+static void sp_intc_handle_ext_cascaded(struct irq_desc *desc)
+{
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	int ext_num = (int)irq_desc_get_handler_data(desc);
+	int hwirq;
+
+	chained_irq_enter(chip, desc);
+
+	while ((hwirq = sp_intc_get_ext_irq(ext_num)) >= 0) {
+		if (unlikely(IS_GPIO_INT(hwirq) && TEST_STATE(hwirq, _IS_ACTIVE))) { // WORKAROUND
+			ASSIGN_STATE(hwirq, _IS_ACTIVE, false);
+			sp_intc_assign_bit(hwirq, REG_INTR_POLARITY, TEST_STATE(hwirq, _IS_LOW));
+		} else {
+			generic_handle_domain_irq(sp_intc.domain, hwirq);
+		}
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+#ifdef CONFIG_SMP
+static int sp_intc_set_affinity(struct irq_data *d, const struct cpumask *mask, bool force)
+{
+	return -EINVAL;
+}
+#endif
+
+static struct irq_chip sp_intc_chip = {
+	.name = "sp_intc",
+	.irq_ack = sp_intc_ack_irq,
+	.irq_mask = sp_intc_mask_irq,
+	.irq_unmask = sp_intc_unmask_irq,
+	.irq_set_type = sp_intc_set_type,
+#ifdef CONFIG_SMP
+	.irq_set_affinity = sp_intc_set_affinity,
+#endif
+};
+
+static int sp_intc_irq_domain_map(struct irq_domain *domain,
+				  unsigned int irq, irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(irq, &sp_intc_chip, handle_level_irq);
+	irq_set_chip_data(irq, &sp_intc_chip);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops sp_intc_dm_ops = {
+	.xlate = irq_domain_xlate_twocell,
+	.map = sp_intc_irq_domain_map,
+};
+
+static int sp_intc_irq_map(struct device_node *node, int i)
+{
+	unsigned int irq;
+
+	irq = irq_of_parse_and_map(node, i);
+	if (!irq)
+		return -ENOENT;
+
+	irq_set_chained_handler_and_data(irq, sp_intc_handle_ext_cascaded, (void *)i);
+
+	return 0;
+}
+
+void sp_intc_set_ext(u32 hwirq, int ext_num)
+{
+	sp_intc_assign_bit(hwirq, REG_INTR_PRIORITY, !ext_num);
+}
+EXPORT_SYMBOL_GPL(sp_intc_set_ext);
+
+int __init sp_intc_init_dt(struct device_node *node, struct device_node *parent)
+{
+	int i, ret;
+
+	sp_intc.g0 = of_iomap(node, 0);
+	if (!sp_intc.g0)
+		return -ENXIO;
+
+	sp_intc.g1 = of_iomap(node, 1);
+	if (!sp_intc.g1) {
+		ret = -ENXIO;
+		goto out_unmap0;
+	}
+
+	ret = sp_intc_irq_map(node, 0); // EXT_INT0
+	if (ret)
+		goto out_unmap1;
+
+	ret = sp_intc_irq_map(node, 1); // EXT_INT1
+	if (ret)
+		goto out_unmap1;
+
+	/* initial regs */
+	for (i = 0; i < SP_INTC_NR_GROUPS; i++) {
+		/* all mask */
+		writel_relaxed(0, REG_INTR_MASK + i * 4);
+		/* all edge */
+		writel_relaxed(~0, REG_INTR_TYPE + i * 4);
+		/* all high-active */
+		writel_relaxed(0, REG_INTR_POLARITY + i * 4);
+		/* all EXT_INT0 */
+		writel_relaxed(~0, REG_INTR_PRIORITY + i * 4);
+		/* all clear */
+		writel_relaxed(~0, REG_INTR_CLEAR + i * 4);
+	}
+
+	sp_intc.domain = irq_domain_add_linear(node, SP_INTC_NR_IRQS,
+					       &sp_intc_dm_ops, &sp_intc);
+	if (!sp_intc.domain) {
+		ret = -ENOMEM;
+		goto out_unmap1;
+	}
+
+	raw_spin_lock_init(&sp_intc.lock);
+
+	return 0;
+
+out_unmap1:
+	iounmap(sp_intc.g1);
+out_unmap0:
+	iounmap(sp_intc.g0);
+
+	return ret;
+}
+
+IRQCHIP_DECLARE(sp_intc, "sunplus,sp7021-intc", sp_intc_init_dt);
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC
  2021-12-03  7:34 [PATCH v5 00/10] Add Sunplus SP7021 SoC Support Qin Jian
                   ` (7 preceding siblings ...)
  2021-12-03  7:34 ` [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver Qin Jian
@ 2021-12-03  7:34 ` Qin Jian
  2021-12-03 12:59   ` Arnd Bergmann
  2021-12-03  7:34 ` [PATCH v5 10/10] ARM: sp7021_defconfig: Add Sunplus SP7021 defconfig Qin Jian
  9 siblings, 1 reply; 26+ messages in thread
From: Qin Jian @ 2021-12-03  7:34 UTC (permalink / raw)
  To: robh+dt
  Cc: mturquette, sboyd, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian

This patch aims to add an initial support for Sunplus SP7021 SoC.

Signed-off-by: Qin Jian <qinjian@cqplus1.com>
---
 MAINTAINERS                         |  1 +
 arch/arm/Kconfig                    | 18 ++++++++++++++++++
 arch/arm/Makefile                   |  2 ++
 arch/arm/mach-sunplus/Kconfig       | 20 ++++++++++++++++++++
 arch/arm/mach-sunplus/Makefile      |  9 +++++++++
 arch/arm/mach-sunplus/Makefile.boot |  3 +++
 arch/arm/mach-sunplus/sp7021.c      | 16 ++++++++++++++++
 7 files changed, 69 insertions(+)
 create mode 100644 arch/arm/mach-sunplus/Kconfig
 create mode 100644 arch/arm/mach-sunplus/Makefile
 create mode 100644 arch/arm/mach-sunplus/Makefile.boot
 create mode 100644 arch/arm/mach-sunplus/sp7021.c

diff --git a/MAINTAINERS b/MAINTAINERS
index febbd97bf..0ae537a41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2664,6 +2664,7 @@ F:	Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
 F:	Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
 F:	Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
 F:	Documentation/devicetree/bindings/reset/sunplus,reset.yaml
+F:	arch/arm/mach-sunplus/
 F:	drivers/clk/clk-sp7021.c
 F:	drivers/irqchip/irq-sp7021-intc.c
 F:	drivers/reset/reset-sunplus.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 59baf6c13..b128ef4ee 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -487,6 +487,22 @@ config ARCH_S3C24XX
 	  (<http://www.simtec.co.uk/products/EB110ITX/>), the IPAQ 1940 or the
 	  Samsung SMDK2410 development board (and derivatives).
 
+config ARCH_SUNPLUS
+	bool "Sunplus SoCs"
+	select CLKSRC_OF
+	select COMMON_CLK
+	select GENERIC_CLOCKEVENTS
+	select GENERIC_IRQ_CHIP
+	select GENERIC_IRQ_MULTI_HANDLER
+	select USE_OF
+	select RTC_CLASS
+	select RESET_SUNPLUS
+	help
+	  Support for Sunplus SoC family: SP7021 and succeeding SoC-based systems,
+	  such as the Banana Pi BPI-F2S development board (and derivatives).
+	  (<http://www.sinovoip.com.cn/ecp_view.asp?id=586>)
+	  (<https://tibbo.com/store/plus1.html>)
+
 config ARCH_OMAP1
 	bool "TI OMAP1"
 	depends on MMU
@@ -689,6 +705,8 @@ source "arch/arm/mach-sti/Kconfig"
 
 source "arch/arm/mach-stm32/Kconfig"
 
+source "arch/arm/mach-sunplus/Kconfig"
+
 source "arch/arm/mach-sunxi/Kconfig"
 
 source "arch/arm/mach-tegra/Kconfig"
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 847c31e7c..cac95a950 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000
 textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000
 textofs-$(CONFIG_ARCH_MESON) := 0x00208000
 textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000
+textofs-$(CONFIG_ARCH_SUNPLUS) := 0x00308000
 
 # Machine directory name.  This list is sorted alphanumerically
 # by CONFIG_* macro name.
@@ -212,6 +213,7 @@ machine-$(CONFIG_ARCH_RENESAS)	 	+= shmobile
 machine-$(CONFIG_ARCH_INTEL_SOCFPGA)	+= socfpga
 machine-$(CONFIG_ARCH_STI)		+= sti
 machine-$(CONFIG_ARCH_STM32)		+= stm32
+machine-$(CONFIG_ARCH_SUNPLUS)		+= sunplus
 machine-$(CONFIG_ARCH_SUNXI)		+= sunxi
 machine-$(CONFIG_ARCH_TEGRA)		+= tegra
 machine-$(CONFIG_ARCH_U8500)		+= ux500
diff --git a/arch/arm/mach-sunplus/Kconfig b/arch/arm/mach-sunplus/Kconfig
new file mode 100644
index 000000000..81ef4d026
--- /dev/null
+++ b/arch/arm/mach-sunplus/Kconfig
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+config SOC_SP7021
+	bool "Sunplus SP7021 SoC support"
+	default y
+	select CPU_V7
+	select ARM_GIC
+	select SUNPLUS_SP7021_INTC
+	select HAVE_SMP
+	select ARM_PSCI
+	select COMMON_CLK_SP7021
+	select PINCTRL
+	select PINCTRL_SPPCTL
+	select OF_OVERLAY
+	select GPIOLIB
+	help
+	  Support for Sunplus SP7021 SoC. It is based on ARM 4-core
+	  Cortex-A7 with various peripherals (ex: I2C, SPI, SDIO,
+	  Ethernet and etc.), FPGA interface,  chip-to-chip bus.
+	  It is designed for industrial control.
diff --git a/arch/arm/mach-sunplus/Makefile b/arch/arm/mach-sunplus/Makefile
new file mode 100644
index 000000000..c902580a7
--- /dev/null
+++ b/arch/arm/mach-sunplus/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the linux kernel.
+#
+
+# Object file lists.
+
+obj-$(CONFIG_SOC_SP7021)	+= sp7021.o
+
diff --git a/arch/arm/mach-sunplus/Makefile.boot b/arch/arm/mach-sunplus/Makefile.boot
new file mode 100644
index 000000000..401c30840
--- /dev/null
+++ b/arch/arm/mach-sunplus/Makefile.boot
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+zreladdr-$(CONFIG_ARCH_SUNPLUS) := 0x00308000
diff --git a/arch/arm/mach-sunplus/sp7021.c b/arch/arm/mach-sunplus/sp7021.c
new file mode 100644
index 000000000..774d0a5bd
--- /dev/null
+++ b/arch/arm/mach-sunplus/sp7021.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) Sunplus Technology Co., Ltd.
+ *       All rights reserved.
+ */
+#include <linux/kernel.h>
+#include <asm/mach/arch.h>
+
+static const char *sp7021_compat[] __initconst = {
+	"sunplus,sp7021",
+	NULL
+};
+
+DT_MACHINE_START(SP7021_DT, "SP7021")
+	.dt_compat	= sp7021_compat,
+MACHINE_END
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 10/10] ARM: sp7021_defconfig: Add Sunplus SP7021 defconfig
  2021-12-03  7:34 [PATCH v5 00/10] Add Sunplus SP7021 SoC Support Qin Jian
                   ` (8 preceding siblings ...)
  2021-12-03  7:34 ` [PATCH v5 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC Qin Jian
@ 2021-12-03  7:34 ` Qin Jian
  2021-12-03 12:49   ` Arnd Bergmann
  9 siblings, 1 reply; 26+ messages in thread
From: Qin Jian @ 2021-12-03  7:34 UTC (permalink / raw)
  To: robh+dt
  Cc: mturquette, sboyd, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian

Add generic Sunplus SP7021 based board defconfig

Signed-off-by: Qin Jian <qinjian@cqplus1.com>
---
 MAINTAINERS                       |   1 +
 arch/arm/configs/sp7021_defconfig | 178 ++++++++++++++++++++++++++++++
 2 files changed, 179 insertions(+)
 create mode 100644 arch/arm/configs/sp7021_defconfig

diff --git a/MAINTAINERS b/MAINTAINERS
index 0ae537a41..9340f8760 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2664,6 +2664,7 @@ F:	Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
 F:	Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
 F:	Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
 F:	Documentation/devicetree/bindings/reset/sunplus,reset.yaml
+F:	arch/arm/configs/sp7021_*defconfig
 F:	arch/arm/mach-sunplus/
 F:	drivers/clk/clk-sp7021.c
 F:	drivers/irqchip/irq-sp7021-intc.c
diff --git a/arch/arm/configs/sp7021_defconfig b/arch/arm/configs/sp7021_defconfig
new file mode 100644
index 000000000..249fe779e
--- /dev/null
+++ b/arch/arm/configs/sp7021_defconfig
@@ -0,0 +1,178 @@
+CONFIG_DEFAULT_HOSTNAME="SP7021-Ev"
+CONFIG_SYSVIPC=y
+CONFIG_USELIB=y
+CONFIG_NO_HZ_IDLE=y
+CONFIG_HIGH_RES_TIMERS=y
+CONFIG_PREEMPT=y
+CONFIG_IKCONFIG=y
+CONFIG_IKCONFIG_PROC=y
+CONFIG_LOG_BUF_SHIFT=14
+CONFIG_BLK_DEV_INITRD=y
+CONFIG_INITRAMFS_SOURCE="../rootfs/initramfs/disk/ ../rootfs/initramfs/initramfs.devnodes"
+CONFIG_INITRAMFS_ROOT_UID=-1
+CONFIG_INITRAMFS_ROOT_GID=-1
+# CONFIG_RD_GZIP is not set
+# CONFIG_RD_BZIP2 is not set
+# CONFIG_RD_LZMA is not set
+# CONFIG_RD_XZ is not set
+# CONFIG_RD_LZO is not set
+# CONFIG_RD_LZ4 is not set
+CONFIG_CC_OPTIMIZE_FOR_SIZE=y
+# CONFIG_FHANDLE is not set
+CONFIG_KALLSYMS_ALL=y
+CONFIG_EMBEDDED=y
+CONFIG_PERF_EVENTS=y
+CONFIG_SLAB=y
+CONFIG_ARCH_SUNPLUS=y
+# CONFIG_VDSO is not set
+CONFIG_SMP=y
+CONFIG_HAVE_ARM_ARCH_TIMER=y
+CONFIG_THUMB2_KERNEL=y
+CONFIG_HIGHMEM=y
+# CONFIG_HIGHPTE is not set
+CONFIG_FORCE_MAX_ZONEORDER=12
+CONFIG_ZBOOT_ROM_TEXT=0x98307000
+CONFIG_ZBOOT_ROM_BSS=0x03400000
+CONFIG_CMDLINE="root=/dev/ram rw init=/init console=ttyS0,115200 earlyprintk mem=512M@0x0"
+CONFIG_AUTO_ZRELADDR=y
+CONFIG_VFP=y
+CONFIG_NEON=y
+CONFIG_MODULES=y
+CONFIG_MODULE_UNLOAD=y
+CONFIG_MODVERSIONS=y
+# CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set
+CONFIG_NET=y
+CONFIG_PACKET=y
+CONFIG_PACKET_DIAG=y
+CONFIG_UNIX=y
+CONFIG_INET=y
+CONFIG_IP_MULTICAST=y
+CONFIG_IP_PNP=y
+CONFIG_IP_PNP_DHCP=y
+CONFIG_IP_PNP_BOOTP=y
+CONFIG_IP_PNP_RARP=y
+CONFIG_NET_IPIP=m
+CONFIG_BRIDGE=m
+CONFIG_VLAN_8021Q=m
+CONFIG_NETLINK_DIAG=y
+CONFIG_CAN=y
+CONFIG_CFG80211=m
+CONFIG_CFG80211_WEXT=y
+CONFIG_MAC80211=m
+CONFIG_CAIF=y
+CONFIG_CEPH_LIB=y
+CONFIG_CEPH_LIB_USE_DNS_RESOLVER=y
+CONFIG_UEVENT_HELPER=y
+CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
+CONFIG_DEVTMPFS=y
+CONFIG_DEVTMPFS_MOUNT=y
+CONFIG_BLK_DEV_LOOP=y
+CONFIG_NETDEVICES=y
+# CONFIG_NET_VENDOR_ALACRITECH is not set
+# CONFIG_NET_VENDOR_AMAZON is not set
+# CONFIG_NET_VENDOR_AQUANTIA is not set
+# CONFIG_NET_VENDOR_ARC is not set
+# CONFIG_NET_VENDOR_BROADCOM is not set
+# CONFIG_NET_VENDOR_CADENCE is not set
+# CONFIG_NET_VENDOR_CAVIUM is not set
+# CONFIG_NET_VENDOR_CIRRUS is not set
+# CONFIG_NET_VENDOR_CORTINA is not set
+# CONFIG_NET_VENDOR_EZCHIP is not set
+# CONFIG_NET_VENDOR_FARADAY is not set
+# CONFIG_NET_VENDOR_GOOGLE is not set
+# CONFIG_NET_VENDOR_HISILICON is not set
+# CONFIG_NET_VENDOR_HUAWEI is not set
+# CONFIG_NET_VENDOR_INTEL is not set
+# CONFIG_NET_VENDOR_MARVELL is not set
+# CONFIG_NET_VENDOR_MICREL is not set
+# CONFIG_NET_VENDOR_MICROCHIP is not set
+# CONFIG_NET_VENDOR_MICROSEMI is not set
+# CONFIG_NET_VENDOR_NATSEMI is not set
+# CONFIG_NET_VENDOR_NETRONOME is not set
+# CONFIG_NET_VENDOR_NI is not set
+# CONFIG_NET_VENDOR_PENSANDO is not set
+# CONFIG_NET_VENDOR_QUALCOMM is not set
+# CONFIG_NET_VENDOR_RENESAS is not set
+# CONFIG_NET_VENDOR_ROCKER is not set
+# CONFIG_NET_VENDOR_SAMSUNG is not set
+# CONFIG_NET_VENDOR_SEEQ is not set
+# CONFIG_NET_VENDOR_SOLARFLARE is not set
+# CONFIG_NET_VENDOR_SMSC is not set
+# CONFIG_NET_VENDOR_SOCIONEXT is not set
+# CONFIG_NET_VENDOR_STMICRO is not set
+# CONFIG_NET_VENDOR_SYNOPSYS is not set
+# CONFIG_NET_VENDOR_VIA is not set
+# CONFIG_NET_VENDOR_WIZNET is not set
+# CONFIG_NET_VENDOR_XILINX is not set
+CONFIG_NET_VENDOR_SUNPLUS=y
+CONFIG_INPUT_POLLDEV=y
+CONFIG_INPUT_SPARSEKMAP=y
+CONFIG_INPUT_MOUSEDEV=y
+CONFIG_INPUT_EVDEV=y
+# CONFIG_INPUT_KEYBOARD is not set
+# CONFIG_INPUT_MOUSE is not set
+CONFIG_INPUT_MISC=y
+CONFIG_INPUT_UINPUT=y
+CONFIG_SERIO_LIBPS2=y
+# CONFIG_LEGACY_PTYS is not set
+CONFIG_SERIAL_SUNPLUS=y
+CONFIG_SERIAL_SUNPLUS_CONSOLE=y
+# CONFIG_HW_RANDOM is not set
+# CONFIG_HWMON is not set
+CONFIG_MEDIA_SUPPORT=y
+CONFIG_MEDIA_SUPPORT_FILTER=y
+CONFIG_MEDIA_CAMERA_SUPPORT=y
+CONFIG_MEDIA_PLATFORM_SUPPORT=y
+CONFIG_MEDIA_USB_SUPPORT=y
+CONFIG_USB_VIDEO_CLASS=y
+CONFIG_FB=y
+CONFIG_FB_SP7021=y
+CONFIG_FRAMEBUFFER_CONSOLE=y
+CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER=y
+CONFIG_HIDRAW=y
+CONFIG_GEMINI_USB=y
+CONFIG_USB_USE_PLATFORM_RESOURCE=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_EHCI_HCD_PLATFORM=y
+CONFIG_USB_OHCI_HCD=y
+CONFIG_USB_OHCI_HCD_PLATFORM=y
+CONFIG_SUNPLUS_USB_PHY=y
+CONFIG_USB_SUNPLUS_OTG=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_DEVICE_LOSE_PACKET_AFTER_SET_INTERFACE_WORKAROUND=y
+CONFIG_USB_DEVICE_EP11_NOT_AUTO_SWITCH_WORKAROUND=y
+CONFIG_USB_GADGET_SUNPLUS=y
+CONFIG_USB_ZERO=y
+CONFIG_MMC=y
+CONFIG_SP_EMMC=m
+CONFIG_STAGING=y
+# CONFIG_IOMMU_SUPPORT is not set
+CONFIG_PWM=y
+CONFIG_PWM_SP7021=y
+CONFIG_RESET_CONTROLLER=y
+CONFIG_EXT2_FS=y
+# CONFIG_DNOTIFY is not set
+CONFIG_FANOTIFY=y
+CONFIG_VFAT_FS=y
+CONFIG_FAT_DEFAULT_IOCHARSET="utf8"
+CONFIG_EXFAT_FS=y
+CONFIG_TMPFS=y
+CONFIG_TMPFS_POSIX_ACL=y
+# CONFIG_MISC_FILESYSTEMS is not set
+CONFIG_NFS_FS=y
+CONFIG_NFS_V4=y
+CONFIG_ROOT_NFS=y
+CONFIG_NLS_CODEPAGE_437=y
+CONFIG_NLS_ASCII=y
+CONFIG_NLS_ISO8859_1=y
+CONFIG_NLS_UTF8=y
+CONFIG_CRC16=y
+CONFIG_PRINTK_TIME=y
+CONFIG_DYNAMIC_DEBUG=y
+CONFIG_MAGIC_SYSRQ=y
+CONFIG_DEBUG_FS=y
+# CONFIG_SCHED_DEBUG is not set
+CONFIG_SCHEDSTATS=y
+# CONFIG_DEBUG_PREEMPT is not set
+# CONFIG_FTRACE is not set
+CONFIG_DEBUG_USER=y
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 10/10] ARM: sp7021_defconfig: Add Sunplus SP7021 defconfig
  2021-12-03  7:34 ` [PATCH v5 10/10] ARM: sp7021_defconfig: Add Sunplus SP7021 defconfig Qin Jian
@ 2021-12-03 12:49   ` Arnd Bergmann
  0 siblings, 0 replies; 26+ messages in thread
From: Arnd Bergmann @ 2021-12-03 12:49 UTC (permalink / raw)
  To: Qin Jian
  Cc: Rob Herring, Michael Turquette, Stephen Boyd, Thomas Gleixner,
	Marc Zyngier, Philipp Zabel, Russell King - ARM Linux,
	Mark Brown, Arnd Bergmann, Linux ARM, DTML,
	Linux Kernel Mailing List, linux-clk, wells.lu

On Fri, Dec 3, 2021 at 8:34 AM Qin Jian <qinjian@cqplus1.com> wrote:
>
> Add generic Sunplus SP7021 based board defconfig
>
> Signed-off-by: Qin Jian <qinjian@cqplus1.com>
> ---
>  MAINTAINERS                       |   1 +
>  arch/arm/configs/sp7021_defconfig | 178 ++++++++++++++++++++++++++++++

Most platforms don't even have a custom defconfig any more. If you
think you need this one,
I would suggest naming it sunplus_defconfig so it makes more sense
when other related
SoCs are added to it.

I would also suggest enabling your platform specific drivers in
multi_v7_defconfig,
and testing that one to ensure it works.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0ae537a41..9340f8760 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2664,6 +2664,7 @@ F:        Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
>  F:     Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
>  F:     Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
>  F:     Documentation/devicetree/bindings/reset/sunplus,reset.yaml
> +F:     arch/arm/configs/sp7021_*defconfig
>  F:     arch/arm/mach-sunplus/
>  F:     drivers/clk/clk-sp7021.c
>  F:     drivers/irqchip/irq-sp7021-intc.c
> diff --git a/arch/arm/configs/sp7021_defconfig b/arch/arm/configs/sp7021_defconfig
> new file mode 100644
> index 000000000..249fe779e
> --- /dev/null
> +++ b/arch/arm/configs/sp7021_defconfig
> @@ -0,0 +1,178 @@
> +CONFIG_DEFAULT_HOSTNAME="SP7021-Ev"

I'd leave this out of the defconfig

> +CONFIG_SYSVIPC=y
> +CONFIG_USELIB=y

You almost certainly don't need CONFIG_USELIB

> +CONFIG_BLK_DEV_INITRD=y
> +CONFIG_INITRAMFS_SOURCE="../rootfs/initramfs/disk/ ../rootfs/initramfs/initramfs.devnodes"
> +CONFIG_INITRAMFS_ROOT_UID=-1

This will cause a build failure for everyone else I think

> +CONFIG_EMBEDDED=y

Probably not needed, otherwise explain what option you need to touch
that depends on this.

> +CONFIG_PERF_EVENTS=y
> +CONFIG_SLAB=y
> +CONFIG_ARCH_SUNPLUS=y
> +# CONFIG_VDSO is not set
> +CONFIG_SMP=y
> +CONFIG_HAVE_ARM_ARCH_TIMER=y
> +CONFIG_THUMB2_KERNEL=y

Ah nice, we don't have enough defconfigs using THUMB2_KERNEL. This may mean
that you get reports about thumb2 specific bugs that nobody else runs
into, but that is
better than if nobody uses that.

> +CONFIG_HIGHMEM=y
> +# CONFIG_HIGHPTE is not set

What is the maximum amount of supported RAM? Note that HIGHMEM support
will likely
go away at some point, so it may be better to use a good VMSPLIT
setting that works with
all your machines without requiring highmem.

> +CONFIG_ZBOOT_ROM_TEXT=0x98307000
> +CONFIG_ZBOOT_ROM_BSS=0x03400000

Where do these come from? I see you don't set CONFIG_ZBOOT_ROM, so they will be
ignored, but if you do rely on these, it likely means that your
machine is unable to
run a multi_v7_defconfig kernel, which is bad.

> +CONFIG_CMDLINE="root=/dev/ram rw init=/init console=ttyS0,115200 earlyprintk mem=512M@0x0"

Remove this, the configuration should come from the bootloader.

> +CONFIG_AUTO_ZRELADDR=y

This is always enabled

> +CONFIG_CAIF=y

Are you actually using CAIF? Which driver do you use?

> +CONFIG_CEPH_LIB=y
> +CONFIG_CEPH_LIB_USE_DNS_RESOLVER=y

Same here, do you use this?

> +CONFIG_INPUT_MISC=y
> +CONFIG_INPUT_UINPUT=y
> +CONFIG_SERIO_LIBPS2=y

Do you have a physical PS/2 keyboard connector

> +CONFIG_USB_VIDEO_CLASS=y
> +CONFIG_FB=y
> +CONFIG_FB_SP7021=y

CONFIG_FB is no longer recommended for new platforms, please use
CONFIG_SIMPLEDRM for the moment, until you have converted your
driver to the DRM framework.

> +CONFIG_GEMINI_USB=y

What is this?

> +CONFIG_EXT2_FS=y

If you use eMMC storage, I recommend using EXT4_FS instead of EXT2_FS


         Arnd

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC
  2021-12-03  7:34 ` [PATCH v5 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC Qin Jian
@ 2021-12-03 12:59   ` Arnd Bergmann
  2021-12-07  7:21     ` qinjian[覃健]
  0 siblings, 1 reply; 26+ messages in thread
From: Arnd Bergmann @ 2021-12-03 12:59 UTC (permalink / raw)
  To: Qin Jian
  Cc: Rob Herring, Michael Turquette, Stephen Boyd, Thomas Gleixner,
	Marc Zyngier, Philipp Zabel, Russell King - ARM Linux,
	Mark Brown, Arnd Bergmann, Linux ARM, DTML,
	Linux Kernel Mailing List, linux-clk, wells.lu

On Fri, Dec 3, 2021 at 8:34 AM Qin Jian <qinjian@cqplus1.com> wrote:

> @@ -487,6 +487,22 @@ config ARCH_S3C24XX
>           (<http://www.simtec.co.uk/products/EB110ITX/>), the IPAQ 1940 or the
>           Samsung SMDK2410 development board (and derivatives).
>
> +config ARCH_SUNPLUS
> +       bool "Sunplus SoCs"
> +       select CLKSRC_OF
> +       select COMMON_CLK
> +       select GENERIC_CLOCKEVENTS
> +       select GENERIC_IRQ_CHIP
> +       select GENERIC_IRQ_MULTI_HANDLER
> +       select USE_OF
> +       select RTC_CLASS
> +       select RESET_SUNPLUS

This is in the wrong place: move the Kconfig entry into
arch/arm/mach-sunplus/Kconfig
and make it 'depends on ARCH_MULTI_V7'.

I think you can remove all the 'select' lines as well because they are
either implied by
ARCH_MULTI_V7 or not actually necessary.

> @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000
>  textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000
>  textofs-$(CONFIG_ARCH_MESON) := 0x00208000
>  textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000
> +textofs-$(CONFIG_ARCH_SUNPLUS) := 0x00308000

What is this needed for? If it boots without this line, better avoid
adding it, because
it will increase the kernel size for everyone else (unless they also enable
AXXIA).

> +config SOC_SP7021
> +       bool "Sunplus SP7021 SoC support"
> +       default y

No 'default y' here. You can probably remove this option completely
and fold it into
ARCH_SUNPLUS.

> +       select CPU_V7
> +       select ARM_GIC
> +       select SUNPLUS_SP7021_INTC
> +       select HAVE_SMP
> +       select ARM_PSCI
> +       select COMMON_CLK_SP7021
> +       select PINCTRL
> +       select PINCTRL_SPPCTL
> +       select OF_OVERLAY
> +       select GPIOLIB
> +       help

Again, most of these should be implied by ARCH_MULTI_V7, so remove those.
For individual drivers, try to avoid the 'select' unless this is required by
the respective driver subsystem.

> diff --git a/arch/arm/mach-sunplus/Makefile.boot b/arch/arm/mach-sunplus/Makefile.boot
> new file mode 100644
> index 000000000..401c30840
> --- /dev/null
> +++ b/arch/arm/mach-sunplus/Makefile.boot
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +zreladdr-$(CONFIG_ARCH_SUNPLUS) := 0x00308000

This should not be needed any more.

       Arnd

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH v5 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC
  2021-12-03 12:59   ` Arnd Bergmann
@ 2021-12-07  7:21     ` qinjian[覃健]
  2021-12-07  9:11       ` Arnd Bergmann
  0 siblings, 1 reply; 26+ messages in thread
From: qinjian[覃健] @ 2021-12-07  7:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Rob Herring, Michael Turquette, Stephen Boyd, Thomas Gleixner,
	Marc Zyngier, Philipp Zabel, Russell King - ARM Linux,
	Mark Brown, Linux ARM, DTML, Linux Kernel Mailing List,
	linux-clk, Wells Lu 呂芳騰

> > +config ARCH_SUNPLUS
> > +       bool "Sunplus SoCs"
> > +       select CLKSRC_OF
> > +       select COMMON_CLK
> > +       select GENERIC_CLOCKEVENTS
> > +       select GENERIC_IRQ_CHIP
> > +       select GENERIC_IRQ_MULTI_HANDLER
> > +       select USE_OF
> > +       select RTC_CLASS
> > +       select RESET_SUNPLUS
> 
> This is in the wrong place: move the Kconfig entry into
> arch/arm/mach-sunplus/Kconfig
> and make it 'depends on ARCH_MULTI_V7'.
> 
> I think you can remove all the 'select' lines as well because they are
> either implied by
> ARCH_MULTI_V7 or not actually necessary.
> ......

Thanks for your review, I'll correct these at next commit.

> 
> > @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000
> >  textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000
> >  textofs-$(CONFIG_ARCH_MESON) := 0x00208000
> >  textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000
> > +textofs-$(CONFIG_ARCH_SUNPLUS) := 0x00308000
> 
> What is this needed for? If it boots without this line, better avoid
> adding it, because
> it will increase the kernel size for everyone else (unless they also enable
> AXXIA).
> 

SP7021 reserved the 1st 1MB memory for ARM926@P-Chip using,
The 2nd 1MB memory for IOP device and the 3rd 1MB memory for bootloader.
I'll add these comments at next commit.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 04/10] reset: Add Sunplus SP7021 reset driver
  2021-12-03  7:34 ` [PATCH v5 04/10] reset: Add Sunplus " Qin Jian
@ 2021-12-07  8:35   ` Philipp Zabel
  0 siblings, 0 replies; 26+ messages in thread
From: Philipp Zabel @ 2021-12-07  8:35 UTC (permalink / raw)
  To: Qin Jian, robh+dt
  Cc: mturquette, sboyd, tglx, maz, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu

On Fri, 2021-12-03 at 15:34 +0800, Qin Jian wrote:
[...]
> diff --git a/drivers/reset/reset-sunplus.c b/drivers/reset/reset-sunplus.c
> new file mode 100644
> index 000000000..a1d88dbaf
> --- /dev/null
> +++ b/drivers/reset/reset-sunplus.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/*
> + * SP7021 reset driver
> + *
> + * Copyright (C) Sunplus Technology Co., Ltd.
> + *       All rights reserved.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/reboot.h>
> +
> +/* HIWORD_MASK_REG BITS */
> +#define BITS_PER_HWM_REG	16
> +
> +struct sp_reset_data {
> +	struct reset_controller_dev rcdev;
> +	void __iomem *membase;
> +} *sp_reset;
     ^^^^^^^^^^

I'd prefer if you removed the global sp_reset pointer.

[...]
> +static int sp_restart(struct notifier_block *this, unsigned long mode,
> +		      void *cmd)
> +{

You could embed the sp_restart_nb notifier block in struct sp_reset_data
and use container_of(this, struct sp_reset_data, notifier) to get to the
rcdev here.

> +	sp_reset_assert(&sp_reset->rcdev, 0);
> +	sp_reset_deassert(&sp_reset->rcdev, 0);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block sp_restart_nb = {
> +	.notifier_call = sp_restart,
> +	.priority = 192,
> +};
> +
> +static const struct reset_control_ops sp_reset_ops = {
> +	.assert   = sp_reset_assert,
> +	.deassert = sp_reset_deassert,
> +	.status   = sp_reset_status,
> +};
> +
> +static const struct of_device_id sp_reset_dt_ids[] = {
> +	{.compatible = "sunplus,sp7021-reset",},
> +	{ /* sentinel */ },
> +};
> +
> +static int sp_reset_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	void __iomem *membase;
> +	struct resource *res;
> +
> +	sp_reset = devm_kzalloc(&pdev->dev, sizeof(*sp_reset), GFP_KERNEL);
> +	if (!sp_reset)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	membase = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(membase))
> +		return PTR_ERR(membase);
> +
> +	sp_reset->membase = membase;
> +	sp_reset->rcdev.owner = THIS_MODULE;
> +	sp_reset->rcdev.nr_resets = resource_size(res) / 4 * 16;	/* HIWORD_MASK */
> +	sp_reset->rcdev.ops = &sp_reset_ops;
> +	sp_reset->rcdev.of_node = dev->of_node;
> +	register_restart_handler(&sp_restart_nb);

Either do this after devm_reset_controller_register(), which could
theoretically fail with -ENOMEM, or call unregister_restart_handler() in
the error case below.

> +
> +	return devm_reset_controller_register(dev, &sp_reset->rcdev);
> +}
> +
> +static struct platform_driver sp_reset_driver = {
> +	.probe = sp_reset_probe,
> +	.driver = {
> +		   .name = "sunplus-reset",
> +		   .of_match_table = sp_reset_dt_ids,
		^
Please fix the indentation, two tabs here.

		.suppress_bind_attrs = true,

to stop unbinding the driver. Alternatively, add a driver remove
function that unregisters the restart handler.

> +		   },
	^
One tab here.

> +};
> +
> +module_platform_driver(sp_reset_driver);
> +
> +MODULE_AUTHOR("Edwin Chiu <edwin.chiu@sunplus.com>");
> +MODULE_DESCRIPTION("Sunplus Reset Driver");
> +MODULE_LICENSE("GPL v2");

regards
Philipp

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver
  2021-12-03  7:34 ` [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver Qin Jian
@ 2021-12-07  9:02   ` Marc Zyngier
  2021-12-08  7:15     ` qinjian[覃健]
  0 siblings, 1 reply; 26+ messages in thread
From: Marc Zyngier @ 2021-12-07  9:02 UTC (permalink / raw)
  To: Qin Jian
  Cc: robh+dt, mturquette, sboyd, tglx, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu

On Fri, 03 Dec 2021 07:34:25 +0000,
Qin Jian <qinjian@cqplus1.com> wrote:
> 
> Add interrupt controller driver for Sunplus SP7021 SoC.
> 
> This is the interrupt controller in P-chip which collects all interrupt
> sources in P-chip and routes them to parent interrupt controller in C-chip.
> 
> Signed-off-by: Qin Jian <qinjian@cqplus1.com>
> ---
>  MAINTAINERS                       |   1 +
>  drivers/irqchip/Kconfig           |   9 +
>  drivers/irqchip/Makefile          |   1 +
>  drivers/irqchip/irq-sp7021-intc.c | 284 ++++++++++++++++++++++++++++++
>  4 files changed, 295 insertions(+)
>  create mode 100644 drivers/irqchip/irq-sp7021-intc.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6b3bbe021..febbd97bf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2665,6 +2665,7 @@ F:	Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
>  F:	Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
>  F:	Documentation/devicetree/bindings/reset/sunplus,reset.yaml
>  F:	drivers/clk/clk-sp7021.c
> +F:	drivers/irqchip/irq-sp7021-intc.c
>  F:	drivers/reset/reset-sunplus.c
>  F:	include/dt-bindings/clock/sp-sp7021.h
>  F:	include/dt-bindings/reset/sp-sp7021.h
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index aca7b595c..b9429b8d0 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -602,4 +602,13 @@ config APPLE_AIC
>  	  Support for the Apple Interrupt Controller found on Apple Silicon SoCs,
>  	  such as the M1.
>  
> +config SUNPLUS_SP7021_INTC
> +	bool "Sunplus SP7021 interrupt controller"
> +	help
> +	  Support for the Sunplus SP7021 Interrupt Controller IP core.
> +	  SP7021 SoC has 2 Chips: C-Chip & P-Chip. This is used as a
> +	  chained controller, routing all interrupt source in P-Chip to
> +	  the primary controller on C-Chip.
> +	  This is selected automatically by platform config.
> +
>  endmenu
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index f88cbf36a..75411f654 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -116,3 +116,4 @@ obj-$(CONFIG_MACH_REALTEK_RTL)		+= irq-realtek-rtl.o
>  obj-$(CONFIG_WPCM450_AIC)		+= irq-wpcm450-aic.o
>  obj-$(CONFIG_IRQ_IDT3243X)		+= irq-idt3243x.o
>  obj-$(CONFIG_APPLE_AIC)			+= irq-apple-aic.o
> +obj-$(CONFIG_SUNPLUS_SP7021_INTC)	+= irq-sp7021-intc.o
> diff --git a/drivers/irqchip/irq-sp7021-intc.c b/drivers/irqchip/irq-sp7021-intc.c
> new file mode 100644
> index 000000000..beabc64d5
> --- /dev/null
> +++ b/drivers/irqchip/irq-sp7021-intc.c
> @@ -0,0 +1,284 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/*
> + * Copyright (C) Sunplus Technology Co., Ltd.
> + *       All rights reserved.
> + */
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/io.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +
> +#define SP_INTC_HWIRQ_MIN	0
> +#define SP_INTC_HWIRQ_MAX	223
> +
> +#define SP_INTC_NR_IRQS		(SP_INTC_HWIRQ_MAX - SP_INTC_HWIRQ_MIN + 1)
> +#define SP_INTC_NR_GROUPS	DIV_ROUND_UP(SP_INTC_NR_IRQS, 32)
> +#define SP_INTC_REG_SIZE	(SP_INTC_NR_GROUPS * 4)
> +
> +/* REG_GROUP_0 regs */
> +#define REG_INTR_TYPE		(sp_intc.g0)
> +#define REG_INTR_POLARITY	(REG_INTR_TYPE     + SP_INTC_REG_SIZE)
> +#define REG_INTR_PRIORITY	(REG_INTR_POLARITY + SP_INTC_REG_SIZE)
> +#define REG_INTR_MASK		(REG_INTR_PRIORITY + SP_INTC_REG_SIZE)
> +
> +/* REG_GROUP_1 regs */
> +#define REG_INTR_CLEAR		(sp_intc.g1)
> +#define REG_MASKED_EXT1		(REG_INTR_CLEAR    + SP_INTC_REG_SIZE)
> +#define REG_MASKED_EXT0		(REG_MASKED_EXT1   + SP_INTC_REG_SIZE)
> +#define REG_INTR_GROUP		(REG_INTR_CLEAR    + 31 * 4)
> +
> +#define GROUP_MASK			(BIT(SP_INTC_NR_GROUPS) - 1)
> +#define GROUP_SHIFT_EXT1	(0)
> +#define GROUP_SHIFT_EXT0	(8)
> +
> +/*
> + * When GPIO_INT0~7 set to edge trigger, doesn't work properly.
> + * WORKAROUND: change it to level trigger, and toggle the polarity
> + * at ACK/Handler to make the HW work.
> + */
> +#define GPIO_INT0_HWIRQ		120
> +#define GPIO_INT7_HWIRQ		127
> +#define IS_GPIO_INT(irq)	\
> +({ \
> +	u32 i = irq; \
> +	(i >= GPIO_INT0_HWIRQ) && (i <= GPIO_INT7_HWIRQ); \
> +})
> +
> +/* index of states */
> +enum {
> +	_IS_EDGE = 0,
> +	_IS_LOW,
> +	_IS_ACTIVE
> +};
> +
> +#define STATE_BIT(irq, idx)			(((irq) - GPIO_INT0_HWIRQ) * 3 + (idx))
> +#define ASSIGN_STATE(irq, idx, v)	assign_bit(STATE_BIT(irq, idx), sp_intc.states, v)
> +#define TEST_STATE(irq, idx)		test_bit(STATE_BIT(irq, idx), sp_intc.states)
> +
> +static struct sp_intctl {
> +	/*
> +	 * REG_GROUP_0: include type/polarity/priority/mask regs.
> +	 * REG_GROUP_1: include clear/masked_ext0/masked_ext1/group regs.
> +	 */
> +	void __iomem *g0; // REG_GROUP_0 base
> +	void __iomem *g1; // REG_GROUP_1 base
> +
> +	struct irq_domain *domain;
> +	raw_spinlock_t lock;
> +
> +	/*
> +	 * store GPIO_INT states
> +	 * each interrupt has 3 states: is_edge, is_low, is_active
> +	 */
> +	DECLARE_BITMAP(states, (GPIO_INT7_HWIRQ - GPIO_INT0_HWIRQ + 1) * 3);
> +} sp_intc;
> +
> +static struct irq_chip sp_intc_chip;
> +
> +static void sp_intc_assign_bit(u32 hwirq, void __iomem *base, bool value)
> +{
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&sp_intc.lock, flags);
> +	__assign_bit(hwirq, base, value);

__assign_bit() on an MMIO mapping? No. Why do you think we have
separate MMIO accessors?

> +	raw_spin_unlock_irqrestore(&sp_intc.lock, flags);
> +}
> +
> +static void sp_intc_ack_irq(struct irq_data *d)
> +{
> +	u32 hwirq = d->hwirq;
> +
> +	if (unlikely(IS_GPIO_INT(hwirq) && TEST_STATE(hwirq, _IS_EDGE))) { // WORKAROUND
> +		sp_intc_assign_bit(hwirq, REG_INTR_POLARITY, !TEST_STATE(hwirq, _IS_LOW));
> +		ASSIGN_STATE(hwirq, _IS_ACTIVE, true);
> +	}
> +
> +	sp_intc_assign_bit(hwirq, REG_INTR_CLEAR, 1);
> +}
> +
> +static void sp_intc_mask_irq(struct irq_data *d)
> +{
> +	sp_intc_assign_bit(d->hwirq, REG_INTR_MASK, 0);
> +}
> +
> +static void sp_intc_unmask_irq(struct irq_data *d)
> +{
> +	sp_intc_assign_bit(d->hwirq, REG_INTR_MASK, 1);
> +}
> +
> +static int sp_intc_set_type(struct irq_data *d, unsigned int type)
> +{
> +	u32 hwirq = d->hwirq;
> +	bool is_edge = !(type & IRQ_TYPE_LEVEL_MASK);
> +	bool is_low = (type == IRQ_TYPE_LEVEL_LOW || type == IRQ_TYPE_EDGE_FALLING);
> +
> +	irq_set_handler_locked(d, is_edge ? handle_edge_irq : handle_level_irq);
> +
> +	if (unlikely(IS_GPIO_INT(hwirq) && is_edge)) { // WORKAROUND
> +		/* store states */
> +		ASSIGN_STATE(hwirq, _IS_EDGE, is_edge);
> +		ASSIGN_STATE(hwirq, _IS_LOW, is_low);
> +		ASSIGN_STATE(hwirq, _IS_ACTIVE, false);
> +		/* change to level */
> +		is_edge = false;
> +	}
> +
> +	sp_intc_assign_bit(hwirq, REG_INTR_TYPE, is_edge);
> +	sp_intc_assign_bit(hwirq, REG_INTR_POLARITY, is_low);
> +
> +	return 0;
> +}
> +
> +static int sp_intc_get_ext_irq(int ext_num)
> +{
> +	void __iomem *base = ext_num ? REG_MASKED_EXT1 : REG_MASKED_EXT0;
> +	u32 shift = ext_num ? GROUP_SHIFT_EXT1 : GROUP_SHIFT_EXT0;
> +	u32 groups;
> +	u32 pending_group;
> +	u32 group;
> +	u32 pending_irq;
> +
> +	groups = readl_relaxed(REG_INTR_GROUP);
> +	pending_group = (groups >> shift) & GROUP_MASK;
> +	if (!pending_group)
> +		return -1;
> +
> +	group = fls(pending_group) - 1;
> +	pending_irq = readl_relaxed(base + group * 4);
> +	if (!pending_irq)
> +		return -1;
> +
> +	return (group * 32) + fls(pending_irq) - 1;
> +}
> +
> +static void sp_intc_handle_ext_cascaded(struct irq_desc *desc)
> +{
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	int ext_num = (int)irq_desc_get_handler_data(desc);
> +	int hwirq;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	while ((hwirq = sp_intc_get_ext_irq(ext_num)) >= 0) {
> +		if (unlikely(IS_GPIO_INT(hwirq) && TEST_STATE(hwirq, _IS_ACTIVE))) { // WORKAROUND
> +			ASSIGN_STATE(hwirq, _IS_ACTIVE, false);
> +			sp_intc_assign_bit(hwirq, REG_INTR_POLARITY, TEST_STATE(hwirq, _IS_LOW));
> +		} else {
> +			generic_handle_domain_irq(sp_intc.domain, hwirq);
> +		}
> +	}
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
> +#ifdef CONFIG_SMP
> +static int sp_intc_set_affinity(struct irq_data *d, const struct cpumask *mask, bool force)
> +{
> +	return -EINVAL;
> +}
> +#endif
> +
> +static struct irq_chip sp_intc_chip = {
> +	.name = "sp_intc",
> +	.irq_ack = sp_intc_ack_irq,
> +	.irq_mask = sp_intc_mask_irq,
> +	.irq_unmask = sp_intc_unmask_irq,
> +	.irq_set_type = sp_intc_set_type,
> +#ifdef CONFIG_SMP
> +	.irq_set_affinity = sp_intc_set_affinity,
> +#endif
> +};
> +
> +static int sp_intc_irq_domain_map(struct irq_domain *domain,
> +				  unsigned int irq, irq_hw_number_t hwirq)
> +{
> +	irq_set_chip_and_handler(irq, &sp_intc_chip, handle_level_irq);
> +	irq_set_chip_data(irq, &sp_intc_chip);
> +	irq_set_noprobe(irq);
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops sp_intc_dm_ops = {
> +	.xlate = irq_domain_xlate_twocell,
> +	.map = sp_intc_irq_domain_map,
> +};
> +
> +static int sp_intc_irq_map(struct device_node *node, int i)
> +{
> +	unsigned int irq;
> +
> +	irq = irq_of_parse_and_map(node, i);
> +	if (!irq)
> +		return -ENOENT;
> +
> +	irq_set_chained_handler_and_data(irq, sp_intc_handle_ext_cascaded, (void *)i);
> +
> +	return 0;
> +}
> +
> +void sp_intc_set_ext(u32 hwirq, int ext_num)
> +{
> +	sp_intc_assign_bit(hwirq, REG_INTR_PRIORITY, !ext_num);
> +}
> +EXPORT_SYMBOL_GPL(sp_intc_set_ext);

No way. We don't export random symbols without a good justification,
and you didn't give any.

> +
> +int __init sp_intc_init_dt(struct device_node *node, struct device_node *parent)

This should be static.

> +{
> +	int i, ret;
> +
> +	sp_intc.g0 = of_iomap(node, 0);
> +	if (!sp_intc.g0)
> +		return -ENXIO;
> +
> +	sp_intc.g1 = of_iomap(node, 1);
> +	if (!sp_intc.g1) {
> +		ret = -ENXIO;
> +		goto out_unmap0;
> +	}
> +
> +	ret = sp_intc_irq_map(node, 0); // EXT_INT0
> +	if (ret)
> +		goto out_unmap1;
> +
> +	ret = sp_intc_irq_map(node, 1); // EXT_INT1
> +	if (ret)
> +		goto out_unmap1;
> +
> +	/* initial regs */
> +	for (i = 0; i < SP_INTC_NR_GROUPS; i++) {
> +		/* all mask */
> +		writel_relaxed(0, REG_INTR_MASK + i * 4);
> +		/* all edge */
> +		writel_relaxed(~0, REG_INTR_TYPE + i * 4);
> +		/* all high-active */
> +		writel_relaxed(0, REG_INTR_POLARITY + i * 4);
> +		/* all EXT_INT0 */
> +		writel_relaxed(~0, REG_INTR_PRIORITY + i * 4);
> +		/* all clear */
> +		writel_relaxed(~0, REG_INTR_CLEAR + i * 4);
> +	}
> +
> +	sp_intc.domain = irq_domain_add_linear(node, SP_INTC_NR_IRQS,
> +					       &sp_intc_dm_ops, &sp_intc);
> +	if (!sp_intc.domain) {
> +		ret = -ENOMEM;
> +		goto out_unmap1;
> +	}
> +
> +	raw_spin_lock_init(&sp_intc.lock);
> +
> +	return 0;
> +
> +out_unmap1:
> +	iounmap(sp_intc.g1);
> +out_unmap0:
> +	iounmap(sp_intc.g0);
> +
> +	return ret;
> +}
> +
> +IRQCHIP_DECLARE(sp_intc, "sunplus,sp7021-intc", sp_intc_init_dt);

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC
  2021-12-07  7:21     ` qinjian[覃健]
@ 2021-12-07  9:11       ` Arnd Bergmann
  2021-12-09  8:49         ` qinjian[覃健]
  0 siblings, 1 reply; 26+ messages in thread
From: Arnd Bergmann @ 2021-12-07  9:11 UTC (permalink / raw)
  To: qinjian[覃健]
  Cc: Arnd Bergmann, Rob Herring, Michael Turquette, Stephen Boyd,
	Thomas Gleixner, Marc Zyngier, Philipp Zabel,
	Russell King - ARM Linux, Mark Brown, Linux ARM, DTML,
	Linux Kernel Mailing List, linux-clk,
	Wells Lu 呂芳騰

On Tue, Dec 7, 2021 at 8:21 AM qinjian[覃健] <qinjian@cqplus1.com> wrote:
> > > @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000
> > >  textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000
> > >  textofs-$(CONFIG_ARCH_MESON) := 0x00208000
> > >  textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000
> > > +textofs-$(CONFIG_ARCH_SUNPLUS) := 0x00308000
> >
> > What is this needed for? If it boots without this line, better avoid
> > adding it, because
> > it will increase the kernel size for everyone else (unless they also enable
> > AXXIA).
> >
>
> SP7021 reserved the 1st 1MB memory for ARM926@P-Chip using,
> The 2nd 1MB memory for IOP device and the 3rd 1MB memory for bootloader.
> I'll add these comments at next commit.

I think you can just remove the memory from the system memory map in the
device tree and pretend it only starts after the bootloader. It's been a while
since I looked at this though, so I could be misremembering what the minimum
boundaries are for doing this.

        Arnd

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver
  2021-12-07  9:02   ` Marc Zyngier
@ 2021-12-08  7:15     ` qinjian[覃健]
  2021-12-08  7:45       ` Marc Zyngier
  0 siblings, 1 reply; 26+ messages in thread
From: qinjian[覃健] @ 2021-12-08  7:15 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: robh+dt, mturquette, sboyd, tglx, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk,
	Wells Lu 呂芳騰

> > +void sp_intc_set_ext(u32 hwirq, int ext_num)
> > +{
> > +	sp_intc_assign_bit(hwirq, REG_INTR_PRIORITY, !ext_num);
> > +}
> > +EXPORT_SYMBOL_GPL(sp_intc_set_ext);
> 
> No way. We don't export random symbols without a good justification,
> and you didn't give any.
> 

This function called by SP7021 display driver to decide DISPLAY_IRQ
routing to which parent irq (EXT_INT0 or EXT_INT1).

In previous patches, which defined in DT, parsed & processed @ sp_intc_xlate_of()
From your comment, this is a SW decision. So I removed it from DT & intc driver, only 
export this function to access the related intc reg.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver
  2021-12-08  7:15     ` qinjian[覃健]
@ 2021-12-08  7:45       ` Marc Zyngier
  2021-12-08  9:28         ` qinjian[覃健]
  0 siblings, 1 reply; 26+ messages in thread
From: Marc Zyngier @ 2021-12-08  7:45 UTC (permalink / raw)
  To: qinjian[覃健]
  Cc: robh+dt, mturquette, sboyd, tglx, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk,
	Wells Lu 呂芳騰

On 2021-12-08 07:15, qinjian[覃健] wrote:
>> > +void sp_intc_set_ext(u32 hwirq, int ext_num)
>> > +{
>> > +	sp_intc_assign_bit(hwirq, REG_INTR_PRIORITY, !ext_num);
>> > +}
>> > +EXPORT_SYMBOL_GPL(sp_intc_set_ext);
>> 
>> No way. We don't export random symbols without a good justification,
>> and you didn't give any.
>> 
> 
> This function called by SP7021 display driver to decide DISPLAY_IRQ
> routing to which parent irq (EXT_INT0 or EXT_INT1).

Based on what? How can a display driver decide which parent is
appropriate? What improvement does this bring?

> In previous patches, which defined in DT, parsed & processed @
> sp_intc_xlate_of()
> From your comment, this is a SW decision. So I removed it from DT &
> intc driver, only
> export this function to access the related intc reg.

If a decision has to be made, it has to be done in the interrupt
controller driver, or rely on the standard API. Exporting random
low level functions to other random drivers is not acceptable.

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver
  2021-12-08  7:45       ` Marc Zyngier
@ 2021-12-08  9:28         ` qinjian[覃健]
  2021-12-08 16:02           ` Marc Zyngier
  0 siblings, 1 reply; 26+ messages in thread
From: qinjian[覃健] @ 2021-12-08  9:28 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: robh+dt, mturquette, sboyd, tglx, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk,
	Wells Lu 呂芳騰

> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: Wednesday, December 8, 2021 3:45 PM
> To: qinjian[覃健] <qinjian@cqplus1.com>
> Cc: robh+dt@kernel.org; mturquette@baylibre.com; sboyd@kernel.org; tglx@linutronix.de; p.zabel@pengutronix.de;
> linux@armlinux.org.uk; broonie@kernel.org; arnd@arndb.de; linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-clk@vger.kernel.org; Wells Lu 呂芳騰 <wells.lu@sunplus.com>
> Subject: Re: [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver
> 
> On 2021-12-08 07:15, qinjian[覃健] wrote:
> >> > +void sp_intc_set_ext(u32 hwirq, int ext_num)
> >> > +{
> >> > +	sp_intc_assign_bit(hwirq, REG_INTR_PRIORITY, !ext_num);
> >> > +}
> >> > +EXPORT_SYMBOL_GPL(sp_intc_set_ext);
> >>
> >> No way. We don't export random symbols without a good justification,
> >> and you didn't give any.
> >>
> >
> > This function called by SP7021 display driver to decide DISPLAY_IRQ
> > routing to which parent irq (EXT_INT0 or EXT_INT1).
> 
> Based on what? How can a display driver decide which parent is
> appropriate? What improvement does this bring?

In default, all IRQ routing to EXT_INT0, which processed by CPU0
Some device's IRQ need low latency, like display, so routing DISPLAY_IRQ to EXT_INT1,
which processed by CPU1 (set /proc/irq/<EXT_INT1>/smp_affinity_list)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver
  2021-12-08  9:28         ` qinjian[覃健]
@ 2021-12-08 16:02           ` Marc Zyngier
  0 siblings, 0 replies; 26+ messages in thread
From: Marc Zyngier @ 2021-12-08 16:02 UTC (permalink / raw)
  To: "qinjian[覃健]"
  Cc: robh+dt, mturquette, sboyd, tglx, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk,
	Wells Lu 呂芳騰

On Wed, 08 Dec 2021 09:28:42 +0000,
"qinjian[覃健]" <qinjian@cqplus1.com> wrote:
> 
> > -----Original Message-----
> > From: Marc Zyngier <maz@kernel.org>
> > Sent: Wednesday, December 8, 2021 3:45 PM
> > To: qinjian[覃健] <qinjian@cqplus1.com>
> > Cc: robh+dt@kernel.org; mturquette@baylibre.com; sboyd@kernel.org; tglx@linutronix.de; p.zabel@pengutronix.de;
> > linux@armlinux.org.uk; broonie@kernel.org; arnd@arndb.de; linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linux-clk@vger.kernel.org; Wells Lu 呂芳騰 <wells.lu@sunplus.com>
> > Subject: Re: [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver
> > 
> > On 2021-12-08 07:15, qinjian[覃健] wrote:
> > >> > +void sp_intc_set_ext(u32 hwirq, int ext_num)
> > >> > +{
> > >> > +	sp_intc_assign_bit(hwirq, REG_INTR_PRIORITY, !ext_num);
> > >> > +}
> > >> > +EXPORT_SYMBOL_GPL(sp_intc_set_ext);
> > >>
> > >> No way. We don't export random symbols without a good justification,
> > >> and you didn't give any.
> > >>
> > >
> > > This function called by SP7021 display driver to decide DISPLAY_IRQ
> > > routing to which parent irq (EXT_INT0 or EXT_INT1).
> > 
> > Based on what? How can a display driver decide which parent is
> > appropriate? What improvement does this bring?
> 
> In default, all IRQ routing to EXT_INT0, which processed by CPU0
> Some device's IRQ need low latency, like display, so routing
> DISPLAY_IRQ to EXT_INT1, which processed by CPU1 (set
> /proc/irq/<EXT_INT1>/smp_affinity_list)

Why would that have a lower latency? What if CPU1 is busy with
interrupts disabled most of the time? How does the display driver
finds out what is better?

And if you really wanted a lower latency, why route the interrupt via
a secondary interrupt controller, instead of attaching it directly to
the upstream GIC?

I really don't think this is an acceptable thing to do. Configure the
interrupt route statically if you want, but we're not exposing this
sort of SoC-specific API to other drivers.

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH v5 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC
  2021-12-07  9:11       ` Arnd Bergmann
@ 2021-12-09  8:49         ` qinjian[覃健]
  2021-12-09  9:58           ` Arnd Bergmann
  0 siblings, 1 reply; 26+ messages in thread
From: qinjian[覃健] @ 2021-12-09  8:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Rob Herring, Michael Turquette, Stephen Boyd, Thomas Gleixner,
	Marc Zyngier, Philipp Zabel, Russell King - ARM Linux,
	Mark Brown, Linux ARM, DTML, Linux Kernel Mailing List,
	linux-clk, Wells Lu 呂芳騰

> On Tue, Dec 7, 2021 at 8:21 AM qinjian[覃健] <qinjian@cqplus1.com> wrote:
> > > > @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000
> > > >  textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000
> > > >  textofs-$(CONFIG_ARCH_MESON) := 0x00208000
> > > >  textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000
> > > > +textofs-$(CONFIG_ARCH_SUNPLUS) := 0x00308000
> > >
> > > What is this needed for? If it boots without this line, better avoid
> > > adding it, because
> > > it will increase the kernel size for everyone else (unless they also enable
> > > AXXIA).
> > >
> >
> > SP7021 reserved the 1st 1MB memory for ARM926@P-Chip using,
> > The 2nd 1MB memory for IOP device and the 3rd 1MB memory for bootloader.
> > I'll add these comments at next commit.
> 
> I think you can just remove the memory from the system memory map in the
> device tree and pretend it only starts after the bootloader. It's been a while
> since I looked at this though, so I could be misremembering what the minimum
> boundaries are for doing this.
> 
>         Arnd

I have test following 3 methods:

1. current patch
DT:
	memory {
		reg = <0x00000000 0x20000000>; /* 512MB */
	};

	reserved-memory {
		#address-cells = <1>;
		#size-cells = <1>;
		ranges;

		iop_reserve: iop_device {
			no-map;
			reg = <0x00100000 0x00100000>; 
		};
		a926_reserve: a926_memory {
			no-map;
			reg = <0x00000000 0x00100000>;
		};
	};
arch/arm/Makefile:
	textofs-$(CONFIG_ARCH_SUNPLUS) := 0x00308000

bootlog & meminfo :
[    0.000000] Zone ranges:
[    0.000000]   Normal   [mem 0x0000000000000000-0x000000001fffffff]
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000000000000-0x00000000001fffff]
[    0.000000]   node   0: [mem 0x0000000000200000-0x000000001fffffff]
[    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x000000001fffffff]

~ # cat /proc/meminfo
MemTotal:         514008 kB
MemFree:          491960 kB
MemAvailable:     488608 kB



2. DT same as case 1, but no modify @ arch/arm/Makefile

bootlog & meminfo :
[    0.000000] OF: fdt: Reserved memory: failed to reserve memory for node 'iop_device': base 0x00100000, size 1 MiB
[    0.000000] OF: fdt: Reserved memory: failed to reserve memory for node 'a926_memory': base 0x00000000, size 1 MiB
...
[    0.000000] Zone ranges:
[    0.000000]   Normal   [mem 0x0000000000000000-0x000000001fffffff]
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000000000000-0x000000001fffffff]
[    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x000000001fffffff]

~ # cat /proc/meminfo
MemTotal:         516056 kB
MemFree:          493928 kB
MemAvailable:     490572 kB



3. DT:
	memory {
		reg = <0x00300000 0x1FD00000>; /* 512 - 3 MB */
	};
no modify @ arch/arm/Makefile

bootlog & meminfo :
[    0.000000] Zone ranges:
[    0.000000]   Normal   [mem 0x0000000000400000-0x000000001fffffff]
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000000400000-0x000000001fffffff]
[    0.000000] Initmem setup node 0 [mem 0x0000000000400000-0x000000001fffffff]

~ # cat /proc/meminfo
MemTotal:         511964 kB
MemFree:          489636 kB
MemAvailable:     486292 kB



I think method 1 should be correct (compare method 2) & better (compare method 3).


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC
  2021-12-09  8:49         ` qinjian[覃健]
@ 2021-12-09  9:58           ` Arnd Bergmann
  2021-12-09 10:12             ` Ard Biesheuvel
  0 siblings, 1 reply; 26+ messages in thread
From: Arnd Bergmann @ 2021-12-09  9:58 UTC (permalink / raw)
  To: qinjian[覃健]
  Cc: Arnd Bergmann, Rob Herring, Michael Turquette, Stephen Boyd,
	Thomas Gleixner, Marc Zyngier, Philipp Zabel,
	Russell King - ARM Linux, Mark Brown, Linux ARM, DTML,
	Linux Kernel Mailing List, linux-clk,
	Wells Lu 呂芳騰,
	Ard Biesheuvel, Linus Walleij

On Thu, Dec 9, 2021 at 9:49 AM qinjian[覃健] <qinjian@cqplus1.com> wrote:
> > On Tue, Dec 7, 2021 at 8:21 AM qinjian[覃健] <qinjian@cqplus1.com> wrote:
> > > > > @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000
> > > > >  textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000
> > > > >  textofs-$(CONFIG_ARCH_MESON) := 0x00208000
> > > > >  textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000
> > > > > +textofs-$(CONFIG_ARCH_SUNPLUS) := 0x00308000
> > > >
> > > > What is this needed for? If it boots without this line, better avoid
> > > > adding it, because
> > > > it will increase the kernel size for everyone else (unless they also enable
> > > > AXXIA).
> > > >
> > >
> > > SP7021 reserved the 1st 1MB memory for ARM926@P-Chip using,
> > > The 2nd 1MB memory for IOP device and the 3rd 1MB memory for bootloader.
> > > I'll add these comments at next commit.
> >
> > I think you can just remove the memory from the system memory map in the
> > device tree and pretend it only starts after the bootloader. It's been a while
> > since I looked at this though, so I could be misremembering what the minimum
> > boundaries are for doing this.
>
> I have test following 3 methods:

Right, I was thinking of the third method here, which has the advantage of
not requiring the same odd base address for all other platforms, this
is important to us.
I don't see what the problem is with it in your example, does that mean you
have a little less usable memory, or that something fails to work right? I don't
know what the requirements are for memreserve.

Adding a few more people to Cc, maybe they have ideas about how this
was solved elsewhere.

          Arnd

> 1. current patch
> DT:
>         memory {
>                 reg = <0x00000000 0x20000000>; /* 512MB */
>         };
>
>         reserved-memory {
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>                 ranges;
>
>                 iop_reserve: iop_device {
>                         no-map;
>                         reg = <0x00100000 0x00100000>;
>                 };
>                 a926_reserve: a926_memory {
>                         no-map;
>                         reg = <0x00000000 0x00100000>;
>                 };
>         };
> arch/arm/Makefile:
>         textofs-$(CONFIG_ARCH_SUNPLUS) := 0x00308000
>
> bootlog & meminfo :
> [    0.000000] Zone ranges:
> [    0.000000]   Normal   [mem 0x0000000000000000-0x000000001fffffff]
> [    0.000000] Movable zone start for each node
> [    0.000000] Early memory node ranges
> [    0.000000]   node   0: [mem 0x0000000000000000-0x00000000001fffff]
> [    0.000000]   node   0: [mem 0x0000000000200000-0x000000001fffffff]
> [    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x000000001fffffff]
>
> ~ # cat /proc/meminfo
> MemTotal:         514008 kB
> MemFree:          491960 kB
> MemAvailable:     488608 kB
>
>
>
> 2. DT same as case 1, but no modify @ arch/arm/Makefile
>
> bootlog & meminfo :
> [    0.000000] OF: fdt: Reserved memory: failed to reserve memory for node 'iop_device': base 0x00100000, size 1 MiB
> [    0.000000] OF: fdt: Reserved memory: failed to reserve memory for node 'a926_memory': base 0x00000000, size 1 MiB
> ...
> [    0.000000] Zone ranges:
> [    0.000000]   Normal   [mem 0x0000000000000000-0x000000001fffffff]
> [    0.000000] Movable zone start for each node
> [    0.000000] Early memory node ranges
> [    0.000000]   node   0: [mem 0x0000000000000000-0x000000001fffffff]
> [    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x000000001fffffff]
>
> ~ # cat /proc/meminfo
> MemTotal:         516056 kB
> MemFree:          493928 kB
> MemAvailable:     490572 kB
>
>
>
> 3. DT:
>         memory {
>                 reg = <0x00300000 0x1FD00000>; /* 512 - 3 MB */
>         };
> no modify @ arch/arm/Makefile
>
> bootlog & meminfo :
> [    0.000000] Zone ranges:
> [    0.000000]   Normal   [mem 0x0000000000400000-0x000000001fffffff]
> [    0.000000] Movable zone start for each node
> [    0.000000] Early memory node ranges
> [    0.000000]   node   0: [mem 0x0000000000400000-0x000000001fffffff]
> [    0.000000] Initmem setup node 0 [mem 0x0000000000400000-0x000000001fffffff]
>
> ~ # cat /proc/meminfo
> MemTotal:         511964 kB
> MemFree:          489636 kB
> MemAvailable:     486292 kB
>
>
>
> I think method 1 should be correct (compare method 2) & better (compare method 3).
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC
  2021-12-09  9:58           ` Arnd Bergmann
@ 2021-12-09 10:12             ` Ard Biesheuvel
  0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2021-12-09 10:12 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: qinjian[覃健],
	Rob Herring, Michael Turquette, Stephen Boyd, Thomas Gleixner,
	Marc Zyngier, Philipp Zabel, Russell King - ARM Linux,
	Mark Brown, Linux ARM, DTML, Linux Kernel Mailing List,
	linux-clk, Wells Lu 呂芳騰,
	Linus Walleij

On Thu, 9 Dec 2021 at 10:58, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Dec 9, 2021 at 9:49 AM qinjian[覃健] <qinjian@cqplus1.com> wrote:
> > > On Tue, Dec 7, 2021 at 8:21 AM qinjian[覃健] <qinjian@cqplus1.com> wrote:
> > > > > > @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000
> > > > > >  textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000
> > > > > >  textofs-$(CONFIG_ARCH_MESON) := 0x00208000
> > > > > >  textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000
> > > > > > +textofs-$(CONFIG_ARCH_SUNPLUS) := 0x00308000
> > > > >
> > > > > What is this needed for? If it boots without this line, better avoid
> > > > > adding it, because
> > > > > it will increase the kernel size for everyone else (unless they also enable
> > > > > AXXIA).
> > > > >
> > > >
> > > > SP7021 reserved the 1st 1MB memory for ARM926@P-Chip using,
> > > > The 2nd 1MB memory for IOP device and the 3rd 1MB memory for bootloader.
> > > > I'll add these comments at next commit.
> > >
> > > I think you can just remove the memory from the system memory map in the
> > > device tree and pretend it only starts after the bootloader. It's been a while
> > > since I looked at this though, so I could be misremembering what the minimum
> > > boundaries are for doing this.
> >
> > I have test following 3 methods:
>
> Right, I was thinking of the third method here, which has the advantage of
> not requiring the same odd base address for all other platforms, this
> is important to us.
> I don't see what the problem is with it in your example, does that mean you
> have a little less usable memory, or that something fails to work right? I don't
> know what the requirements are for memreserve.
>
> Adding a few more people to Cc, maybe they have ideas about how this
> was solved elsewhere.
>

The phys2virt patching now assumes a granularity of 2 MiB. This means
that by removing 3 MiB at the start of DRAM, you lose 1 MIB of usable
memory unless you find a way to memremap() it directly.

So I think a combination of the two approaches might work here
- remove 2 MiB from the the /memory node.
- add 1 MiB to the text offset instead of 3 MiB.

Note that this is a compromise, and still not our preferred approach.
It would be far better to move these reserved regions to the end of
DRAM instead.


>
> > 1. current patch
> > DT:
> >         memory {
> >                 reg = <0x00000000 0x20000000>; /* 512MB */
> >         };
> >
> >         reserved-memory {
> >                 #address-cells = <1>;
> >                 #size-cells = <1>;
> >                 ranges;
> >
> >                 iop_reserve: iop_device {
> >                         no-map;
> >                         reg = <0x00100000 0x00100000>;
> >                 };
> >                 a926_reserve: a926_memory {
> >                         no-map;
> >                         reg = <0x00000000 0x00100000>;
> >                 };
> >         };
> > arch/arm/Makefile:
> >         textofs-$(CONFIG_ARCH_SUNPLUS) := 0x00308000
> >
> > bootlog & meminfo :
> > [    0.000000] Zone ranges:
> > [    0.000000]   Normal   [mem 0x0000000000000000-0x000000001fffffff]
> > [    0.000000] Movable zone start for each node
> > [    0.000000] Early memory node ranges
> > [    0.000000]   node   0: [mem 0x0000000000000000-0x00000000001fffff]
> > [    0.000000]   node   0: [mem 0x0000000000200000-0x000000001fffffff]
> > [    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x000000001fffffff]
> >
> > ~ # cat /proc/meminfo
> > MemTotal:         514008 kB
> > MemFree:          491960 kB
> > MemAvailable:     488608 kB
> >
> >
> >
> > 2. DT same as case 1, but no modify @ arch/arm/Makefile
> >
> > bootlog & meminfo :
> > [    0.000000] OF: fdt: Reserved memory: failed to reserve memory for node 'iop_device': base 0x00100000, size 1 MiB
> > [    0.000000] OF: fdt: Reserved memory: failed to reserve memory for node 'a926_memory': base 0x00000000, size 1 MiB
> > ...
> > [    0.000000] Zone ranges:
> > [    0.000000]   Normal   [mem 0x0000000000000000-0x000000001fffffff]
> > [    0.000000] Movable zone start for each node
> > [    0.000000] Early memory node ranges
> > [    0.000000]   node   0: [mem 0x0000000000000000-0x000000001fffffff]
> > [    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x000000001fffffff]
> >
> > ~ # cat /proc/meminfo
> > MemTotal:         516056 kB
> > MemFree:          493928 kB
> > MemAvailable:     490572 kB
> >
> >
> >
> > 3. DT:
> >         memory {
> >                 reg = <0x00300000 0x1FD00000>; /* 512 - 3 MB */
> >         };
> > no modify @ arch/arm/Makefile
> >
> > bootlog & meminfo :
> > [    0.000000] Zone ranges:
> > [    0.000000]   Normal   [mem 0x0000000000400000-0x000000001fffffff]
> > [    0.000000] Movable zone start for each node
> > [    0.000000] Early memory node ranges
> > [    0.000000]   node   0: [mem 0x0000000000400000-0x000000001fffffff]
> > [    0.000000] Initmem setup node 0 [mem 0x0000000000400000-0x000000001fffffff]
> >
> > ~ # cat /proc/meminfo
> > MemTotal:         511964 kB
> > MemFree:          489636 kB
> > MemAvailable:     486292 kB
> >
> >
> >
> > I think method 1 should be correct (compare method 2) & better (compare method 3).
> >

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 06/10] clk: Add Sunplus SP7021 clock driver
  2021-12-03  7:34 ` [PATCH v5 06/10] clk: Add Sunplus " Qin Jian
@ 2021-12-16  6:42   ` Stephen Boyd
  0 siblings, 0 replies; 26+ messages in thread
From: Stephen Boyd @ 2021-12-16  6:42 UTC (permalink / raw)
  To: Qin Jian, robh+dt
  Cc: mturquette, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian

Quoting Qin Jian (2021-12-02 23:34:23)
> diff --git a/drivers/clk/clk-sp7021.c b/drivers/clk/clk-sp7021.c
> new file mode 100644
> index 000000000..040578e82
> --- /dev/null
> +++ b/drivers/clk/clk-sp7021.c
> @@ -0,0 +1,738 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/*
> + * Copyright (C) Sunplus Technology Co., Ltd.
> + *       All rights reserved.
> + */
> +//#define DEBUG
> +#include <linux/module.h>

It's not a module, don't include this.

> +#include <linux/clk.h>

Is this include used?

> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/bitfield.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>

Is this include used?

> +#include <dt-bindings/clock/sp-sp7021.h>
> +
> +#ifndef clk_readl
> +#define clk_readl  readl
> +#define clk_writel writel
> +#endif

Please remove these and use readl/writel directly.

> +
> +#define REG(i)         (pll_regs + (i) * 4)
> +
> +#define PLLA_CTL       REG(7)
> +#define PLLE_CTL       REG(12)
> +#define PLLF_CTL       REG(13)
> +#define PLLTV_CTL      REG(14)
> +#define PLLSYS_CTL     REG(26)
> +
> +#define EXTCLK         "extclk"
> +
> +/* speical div_width values for PLLTV/PLLA */
> +#define DIV_TV         33
> +#define DIV_A          34
> +
> +/* PLLTV parameters */
> +enum {
> +       SEL_FRA,
> +       SDM_MOD,
> +       PH_SEL,
> +       NFRA,
> +       DIVR,
> +       DIVN,
> +       DIVM,
> +       P_MAX
> +};
> +
> +#define MASK_SEL_FRA   GENMASK(1, 1)
> +#define MASK_SDM_MOD   GENMASK(2, 2)
> +#define MASK_PH_SEL            GENMASK(4, 4)
> +#define MASK_NFRA              GENMASK(12, 6)
> +#define MASK_DIVR              GENMASK(8, 7)
> +#define MASK_DIVN              GENMASK(7, 0)
> +#define MASK_DIVM              GENMASK(14, 8)
> +
> +/* HIWORD_MASK FIELD_PREP */
> +#define HWM_FIELD_PREP(mask, value) \
> +({\
> +       u32 m = mask; \
> +       (m << 16) | FIELD_PREP(m, value); \

Please tab out the \ to a single column that's the same for all.

> +})
> +
> +struct sp_pll {
> +       struct clk_hw hw;
> +       void __iomem *reg;
> +       spinlock_t *lock;       /* lock for reg */
> +       int div_shift;
> +       int div_width;
> +       int pd_bit;             /* power down bit idx */
> +       int bp_bit;             /* bypass bit idx */
> +       unsigned long brate;    /* base rate, TODO: replace brate with muldiv */
> +       u32 p[P_MAX];   /* for hold PLLTV/PLLA parameters */
> +};
> +
> +#define to_sp_pll(_hw) container_of(_hw, struct sp_pll, hw)
> +
> +#define clk_regs       (moon_regs + 0x000) /* G0 ~ CLKEN */
> +#define pll_regs       (moon_regs + 0x200) /* G4 ~ PLL */
> +static void __iomem *moon_regs;
> +
> +#define P_EXTCLK       BIT(16)
> +static const char * const parents[] = {

Can we use clk_parent_data instead?

> +       "pllsys",
> +       "extclk",
> +};
> +
> +static const u32 gates[] = {
> +       CLK_SYSTEM,
> +       CLK_RTC,
> +       CLK_IOCTL,
> +       CLK_IOP,
> +       CLK_OTPRX,
> +       CLK_NOC,
> +       CLK_BR,
> +       CLK_RBUS_L00,
> +       CLK_SPIFL,
> +       CLK_SDCTRL0,
> +       CLK_PERI0 | P_EXTCLK,
> +       CLK_A926,
> +       CLK_UMCTL2,
> +       CLK_PERI1 | P_EXTCLK,
> +
> +       CLK_DDR_PHY0,
> +       CLK_ACHIP,
> +       CLK_STC0,
> +       CLK_STC_AV0,
> +       CLK_STC_AV1,
> +       CLK_STC_AV2,
> +       CLK_UA0 | P_EXTCLK,
> +       CLK_UA1 | P_EXTCLK,
> +       CLK_UA2 | P_EXTCLK,
> +       CLK_UA3 | P_EXTCLK,
> +       CLK_UA4 | P_EXTCLK,
> +       CLK_HWUA | P_EXTCLK,
> +       CLK_DDC0,
> +       CLK_UADMA | P_EXTCLK,
> +
> +       CLK_CBDMA0,
> +       CLK_CBDMA1,
> +       CLK_SPI_COMBO_0,
> +       CLK_SPI_COMBO_1,
> +       CLK_SPI_COMBO_2,
> +       CLK_SPI_COMBO_3,
> +       CLK_AUD,
> +       CLK_USBC0,
> +       CLK_USBC1,
> +       CLK_UPHY0,
> +       CLK_UPHY1,
> +
> +       CLK_I2CM0,
> +       CLK_I2CM1,
> +       CLK_I2CM2,
> +       CLK_I2CM3,
> +       CLK_PMC,
> +       CLK_CARD_CTL0,
> +       CLK_CARD_CTL1,
> +
> +       CLK_CARD_CTL4,
> +       CLK_BCH,
> +       CLK_DDFCH,
> +       CLK_CSIIW0,
> +       CLK_CSIIW1,
> +       CLK_MIPICSI0,
> +       CLK_MIPICSI1,
> +
> +       CLK_HDMI_TX,
> +       CLK_VPOST,
> +
> +       CLK_TGEN,
> +       CLK_DMIX,
> +       CLK_TCON,
> +       CLK_INTERRUPT,
> +
> +       CLK_RGST,
> +       CLK_GPIO,
> +       CLK_RBUS_TOP,
> +
> +       CLK_MAILBOX,
> +       CLK_SPIND,
> +       CLK_I2C2CBUS,
> +       CLK_SEC,
> +       CLK_GPOST0,
> +       CLK_DVE,
> +
> +       CLK_OSD0,
> +       CLK_DISP_PWM,
> +       CLK_UADBG,
> +       CLK_DUMMY_MASTER,
> +       CLK_FIO_CTL,
> +       CLK_FPGA,
> +       CLK_L2SW,
> +       CLK_ICM,
> +       CLK_AXI_GLOBAL,
> +};
> +
> +static struct clk *clks[CLK_MAX];
> +static struct clk_onecell_data clk_data;
> +
> +static DEFINE_SPINLOCK(plla_lock);
> +static DEFINE_SPINLOCK(plle_lock);
> +static DEFINE_SPINLOCK(pllf_lock);
> +static DEFINE_SPINLOCK(pllsys_lock);
> +static DEFINE_SPINLOCK(plltv_lock);
> +
> +#define _M                     1000000UL
> +#define F_27M          (27 * _M)
> +
> +/******************************************** PLL_TV *******************************************/
> +
> +//#define PLLTV_STEP_DIR (?) /* Unit: HZ */
> +
> +/* TODO: set proper FVCO range */
> +#define FVCO_MIN       (100 * _M)
> +#define FVCO_MAX       (200 * _M)
> +
> +#define F_MIN          (FVCO_MIN / 8)
> +#define F_MAX          (FVCO_MAX)
> +
> +static long plltv_integer_div(struct sp_pll *clk, unsigned long freq)
> +{
> +       /* valid m values: 27M must be divisible by m, 0 means end */
> +       static const u32 m_table[] = {
> +               1, 2, 3, 4, 5, 6, 8, 9, 10, 12, 15, 16, 18, 20, 24, 25, 27, 30, 32, 0
> +       };
> +       u32 m, n, r;
> +#ifdef PLLTV_STEP_DIR
> +       u32 step = (PLLTV_STEP_DIR > 0) ? PLLTV_STEP_DIR : -PLLTV_STEP_DIR;
> +       int calc_times = 1000000 / step;
> +#endif
> +       unsigned long fvco, nf;
> +
> +       /* check freq */
> +       if (freq < F_MIN) {
> +               pr_warn("%s: %s freq:%lu < F_MIN:%lu, round up\n",
> +                       __func__, clk_hw_get_name(&clk->hw), freq, F_MIN);
> +               freq = F_MIN;
> +       } else if (freq > F_MAX) {
> +               pr_warn("%s: %s freq:%lu > F_MAX:%lu, round down\n",
> +                       __func__, clk_hw_get_name(&clk->hw), freq, F_MAX);
> +               freq = F_MAX;
> +       }
> +
> +#ifdef PLLTV_STEP_DIR
> +       if ((freq % step) != 0)
> +               freq += step - (freq % step) + ((PLLTV_STEP_DIR > 0) ? 0 : PLLTV_STEP_DIR);
> +#endif
> +
> +#ifdef PLLTV_STEP_DIR
> +CALC:
> +       if (!calc_times) {
> +               pr_err("%s: %s freq:%lu out of recalc times\n",
> +                      __func__, clk_hw_get_name(&clk->hw), freq);
> +               return -ETIMEOUT;
> +       }
> +#endif
> +
> +       /* DIVR 0~3 */
> +       for (r = 0; r <= 3; r++) {
> +               fvco = freq << r;
> +               if (fvco <= FVCO_MAX)
> +                       break;
> +       }
> +
> +       /* DIVM */
> +       for (m = 0; m_table[m]; m++) {
> +               nf = fvco * m_table[m];
> +               n = nf / F_27M;
> +               if ((n * F_27M) == nf)
> +                       break;
> +       }
> +       m = m_table[m];
> +
> +       if (!m) {
> +#ifdef PLLTV_STEP_DIR
> +               freq += PLLTV_STEP_DIR;
> +               calc_times--;
> +               goto CALC;
> +#else

Please remove the debugging code.

> +               pr_err("%s: %s freq:%lu not found a valid setting\n",
> +                      __func__, clk_hw_get_name(&clk->hw), freq);
> +               return -EINVAL;
> +#endif
> +       }
> +
> +       /* save parameters */
> +       clk->p[SEL_FRA] = 0;
> +       clk->p[DIVR]    = r;
> +       clk->p[DIVN]    = n;
> +       clk->p[DIVM]    = m;
> +
> +       return freq;
> +}
> +
> +/* parameters for PLLTV fractional divider */
> +static const u32 pt[][5] = {
> +       /* conventional fractional */
> +       {
> +               1,                      // factor
> +               5,                      // 5 * p0 (nint)
> +               1,                      // 1 * p0
> +               F_27M,                  // F_27M / p0
> +               1,                      // p0 / p2
> +       },
> +       /* phase rotation */
> +       {
> +               10,                     // factor
> +               54,                     // 5.4 * p0 (nint)
> +               2,                      // 0.2 * p0
> +               F_27M / 10,             // F_27M / p0
> +               5,                      // p0 / p2
> +       },
> +};
> +
> +static const u32 mods[] = { 91, 55 }; /* SDM_MOD mod values */
> +
> +static long plltv_fractional_div(struct sp_pll *clk, unsigned long freq)
> +{
> +       u32 m, r;
> +       u32 nint, nfra;
> +       u32 diff_min_quotient = 210000000, diff_min_remainder = 0;
> +       u32 diff_min_sign = 0;
> +       unsigned long fvco, nf, f, fout = 0;
> +       int sdm, ph;
> +
> +       /* check freq */
> +       if (freq < F_MIN) {
> +               pr_warn("%s: %s freq:%lu < F_MIN:%lu, round up\n",
> +                       __func__, clk_hw_get_name(&clk->hw), freq, F_MIN);
> +               freq = F_MIN;
> +       } else if (freq > F_MAX) {
> +               pr_warn("%s: %s freq:%lu > F_MAX:%lu, round down\n",
> +                       __func__, clk_hw_get_name(&clk->hw), freq, F_MAX);
> +               freq = F_MAX;
> +       }
> +
> +       /* DIVR 0~3 */
> +       for (r = 0; r <= 3; r++) {
> +               fvco = freq << r;
> +               if (fvco <= FVCO_MAX)
> +                       break;
> +       }
> +       f = F_27M >> r;
> +
> +       /* PH_SEL 1/0 */
> +       for (ph = 1; ph >= 0; ph--) {
> +               const u32 *pp = pt[ph];
> +               u32 ms = 1;
> +
> +               /* SDM_MOD 0/1 */
> +               for (sdm = 0; sdm <= 1; sdm++) {
> +                       u32 mod = mods[sdm];
> +
> +                       /* DIVM 1~32 */
> +                       for (m = ms; m <= 32; m++) {
> +                               u32 diff_freq;
> +                               u32 diff_freq_quotient = 0, diff_freq_remainder = 0;
> +                               u32 diff_freq_sign = 0; /* 0:Positive number, 1:Negative number */
> +
> +                               nf = fvco * m;
> +                               nint = nf / pp[3];
> +
> +                               if (nint < pp[1])
> +                                       continue;
> +                               if (nint > pp[1])
> +                                       break;
> +
> +                               nfra = (((nf % pp[3]) * mod * pp[4]) + (F_27M / 2)) / F_27M;
> +                               if (nfra)
> +                                       diff_freq = (f * (nint + pp[2]) / pp[0]) -
> +                                                               (f * (mod - nfra) / mod / pp[4]);
> +                               else
> +                                       diff_freq = (f * (nint) / pp[0]);
> +
> +                               diff_freq_quotient  = diff_freq / m;
> +                               diff_freq_remainder = ((diff_freq % m) * 1000) / m;
> +
> +                               if (freq > diff_freq_quotient) {
> +                                       diff_freq_quotient  = freq - diff_freq_quotient - 1;
> +                                       diff_freq_remainder = 1000 - diff_freq_remainder;
> +                                       diff_freq_sign = 1;
> +                               } else {
> +                                       diff_freq_quotient = diff_freq_quotient - freq;
> +                                       diff_freq_sign = 0;
> +                               }
> +
> +                               if (diff_min_quotient > diff_freq_quotient ||
> +                                   (diff_min_quotient == diff_freq_quotient &&
> +                                   diff_min_remainder > diff_freq_remainder)) {
> +                                       /* found a closer freq, save parameters */
> +                                       clk->p[SEL_FRA] = 1;
> +                                       clk->p[SDM_MOD] = sdm;
> +                                       clk->p[PH_SEL]  = ph;
> +                                       clk->p[NFRA]    = nfra;
> +                                       clk->p[DIVR]    = r;
> +                                       clk->p[DIVM]    = m;
> +
> +                                       fout = diff_freq / m;
> +                                       diff_min_quotient = diff_freq_quotient;
> +                                       diff_min_remainder = diff_freq_remainder;
> +                                       diff_min_sign = diff_freq_sign;
> +                               }
> +                       }
> +               }
> +       }
> +
> +       if (!fout) {
> +               pr_err("%s: %s freq:%lu not found a valid setting\n",
> +                      __func__, clk_hw_get_name(&clk->hw), freq);
> +               return -EINVAL;
> +       }
> +
> +       return fout;
> +}
> +
> +static long plltv_div(struct sp_pll *clk, unsigned long freq)
> +{
> +       if (freq % 100)
> +               return plltv_fractional_div(clk, freq);
> +       else
> +               return plltv_integer_div(clk, freq);
> +}
> +
> +static void plltv_set_rate(struct sp_pll *clk)
> +{
> +       u32 reg;
> +
> +       reg  = HWM_FIELD_PREP(MASK_SEL_FRA, clk->p[SEL_FRA]);
> +       reg |= HWM_FIELD_PREP(MASK_SDM_MOD, clk->p[SDM_MOD]);
> +       reg |= HWM_FIELD_PREP(MASK_PH_SEL, clk->p[PH_SEL]);
> +       reg |= HWM_FIELD_PREP(MASK_NFRA, clk->p[NFRA]);
> +       clk_writel(reg, clk->reg);
> +
> +       reg  = HWM_FIELD_PREP(MASK_DIVR, clk->p[DIVR]);
> +       clk_writel(reg, clk->reg + 4);
> +
> +       reg  = HWM_FIELD_PREP(MASK_DIVN, clk->p[DIVN] - 1);
> +       reg |= HWM_FIELD_PREP(MASK_DIVM, clk->p[DIVM] - 1);
> +       clk_writel(reg, clk->reg + 8);
> +}
> +
> +/******************************************** PLL_A ********************************************/
> +
> +/* from Q628_PLLs_REG_setting.xlsx */
> +struct {
> +       u32 rate;
> +       u32 regs[5];
> +} pa[] = {

Can this be const? and static?

> +       {
> +               .rate = 135475200,
> +               .regs = {
> +                       0x4801,
> +                       0x02df,
> +                       0x248f,
> +                       0x0211,
> +                       0x33e9
> +               }
> +       },
> +       {
> +               .rate = 147456000,
> +               .regs = {
> +                       0x4801,
> +                       0x1adf,
> +                       0x2490,
> +                       0x0349,
> +                       0x33e9
> +               }
> +       },
> +       {
> +               .rate = 196608000,
> +               .regs = {
> +                       0x4801,
> +                       0x42ef,
> +                       0x2495,
> +                       0x01c6,
> +                       0x33e9
> +               }
> +       },
> +};
> +
> +static void plla_set_rate(struct sp_pll *clk)
> +{
> +       const u32 *pp = pa[clk->p[0]].regs;
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(pa->regs); i++)
> +               clk_writel(0xffff0000 | pp[i], clk->reg + (i * 4));
> +}
> +
> +static long plla_round_rate(struct sp_pll *clk, unsigned long rate)
> +{
> +       int i = ARRAY_SIZE(pa);
> +
> +       while (--i) {
> +               if (rate >= pa[i].rate)
> +                       break;
> +       }
> +       clk->p[0] = i;
> +       return pa[i].rate;
> +}
> +
> +/******************************************* SP_PLL ********************************************/
> +
> +static long sp_pll_calc_div(struct sp_pll *clk, unsigned long rate)
> +{
> +       u32 fbdiv;
> +       u32 max = 1 << clk->div_width;
> +
> +       fbdiv = DIV_ROUND_CLOSEST(rate, clk->brate);
> +       if (fbdiv > max)
> +               fbdiv = max;
> +       return fbdiv;
> +}
> +
> +static long sp_pll_round_rate(struct clk_hw *hw, unsigned long rate,
> +                             unsigned long *prate)
> +{
> +       struct sp_pll *clk = to_sp_pll(hw);
> +       long ret;
> +
> +       if (rate == *prate)
> +               ret = *prate; /* bypass */
> +       else if (clk->div_width == DIV_A) {
> +               ret = plla_round_rate(clk, rate);
> +       } else if (clk->div_width == DIV_TV) {
> +               ret = plltv_div(clk, rate);
> +               if (ret < 0)
> +                       ret = *prate;
> +       } else {
> +               ret = sp_pll_calc_div(clk, rate) * clk->brate;
> +       }
> +
> +       return ret;
> +}
> +
> +static unsigned long sp_pll_recalc_rate(struct clk_hw *hw,
> +                                       unsigned long prate)
> +{
> +       struct sp_pll *clk = to_sp_pll(hw);
> +       u32 reg = clk_readl(clk->reg);
> +       unsigned long ret;
> +
> +       if (reg & BIT(clk->bp_bit)) {
> +               ret = prate; /* bypass */
> +       } else if (clk->div_width == DIV_A) {
> +               ret = pa[clk->p[0]].rate;
> +       } else if (clk->div_width == DIV_TV) {
> +               u32 m, r, reg2;
> +
> +               r = FIELD_GET(MASK_DIVR, clk_readl(clk->reg + 4));
> +               reg2 = clk_readl(clk->reg + 8);
> +               m = FIELD_GET(MASK_DIVM, reg2) + 1;
> +
> +               if (reg & BIT(1)) { /* SEL_FRA */
> +                       /* fractional divider */
> +                       u32 sdm  = FIELD_GET(MASK_SDM_MOD, reg);
> +                       u32 ph   = FIELD_GET(MASK_PH_SEL, reg);
> +                       u32 nfra = FIELD_GET(MASK_NFRA, reg);
> +                       const u32 *pp = pt[ph];
> +
> +                       ret = prate >> r;
> +                       ret = (ret * (pp[1] + pp[2]) / pp[0]) -
> +                                 (ret * (mods[sdm] - nfra) / mods[sdm] / pp[4]);
> +                       ret /= m;
> +               } else {
> +                       /* integer divider */
> +                       u32 n = FIELD_GET(MASK_DIVN, reg2) + 1;
> +
> +                       ret = (prate / m * n) >> r;
> +               }
> +       } else {
> +               u32 fbdiv = (reg >> clk->div_shift) & ((1 << clk->div_width) - 1);
> +
> +               ret = clk->brate * fbdiv;
> +       }
> +
> +       return ret;
> +}
> +
> +static int sp_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> +                          unsigned long prate)
> +{
> +       struct sp_pll *clk = to_sp_pll(hw);
> +       unsigned long flags;
> +       u32 reg;
> +
> +       spin_lock_irqsave(clk->lock, flags);
> +
> +       reg = BIT(clk->bp_bit + 16); /* HIWORD_MASK */
> +
> +       if (rate == prate)
> +               reg |= BIT(clk->bp_bit); /* bypass */
> +       else if (clk->div_width == DIV_A)
> +               plla_set_rate(clk);
> +       else if (clk->div_width == DIV_TV)
> +               plltv_set_rate(clk);
> +       else if (clk->div_width) {
> +               u32 fbdiv = sp_pll_calc_div(clk, rate);
> +               u32 mask = GENMASK(clk->div_shift + clk->div_width - 1, clk->div_shift);
> +
> +               reg |= (mask << 16) | (((fbdiv - 1) << clk->div_shift) & mask);
> +       }
> +
> +       clk_writel(reg, clk->reg);
> +
> +       spin_unlock_irqrestore(clk->lock, flags);
> +
> +       return 0;
> +}
> +
> +static int sp_pll_enable(struct clk_hw *hw)
> +{
> +       struct sp_pll *clk = to_sp_pll(hw);
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(clk->lock, flags);
> +       clk_writel(BIT(clk->pd_bit + 16) | BIT(clk->pd_bit), clk->reg); /* power up */
> +       spin_unlock_irqrestore(clk->lock, flags);
> +
> +       return 0;
> +}
> +
> +static void sp_pll_disable(struct clk_hw *hw)
> +{
> +       struct sp_pll *clk = to_sp_pll(hw);
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(clk->lock, flags);
> +       clk_writel(BIT(clk->pd_bit + 16), clk->reg); /* power down */
> +       spin_unlock_irqrestore(clk->lock, flags);
> +}
> +
> +static int sp_pll_is_enabled(struct clk_hw *hw)
> +{
> +       struct sp_pll *clk = to_sp_pll(hw);
> +
> +       return clk_readl(clk->reg) & BIT(clk->pd_bit);
> +}
> +
> +static const struct clk_ops sp_pll_ops = {
> +       .enable = sp_pll_enable,
> +       .disable = sp_pll_disable,
> +       .is_enabled = sp_pll_is_enabled,
> +       .round_rate = sp_pll_round_rate,
> +       .recalc_rate = sp_pll_recalc_rate,
> +       .set_rate = sp_pll_set_rate
> +};
> +
> +static const struct clk_ops sp_pll_sub_ops = {
> +       .enable = sp_pll_enable,
> +       .disable = sp_pll_disable,
> +       .is_enabled = sp_pll_is_enabled,
> +       .recalc_rate = sp_pll_recalc_rate,
> +};
> +
> +struct clk *clk_register_sp_pll(const char *name, const char *parent,
> +                               void __iomem *reg, int pd_bit, int bp_bit,
> +                               unsigned long brate, int shift, int width,
> +                               spinlock_t *lock)
> +{
> +       struct sp_pll *pll;
> +       struct clk *clk;
> +       struct clk_init_data initd = {
> +               .name = name,
> +               .parent_names = &parent,
> +               .ops = (bp_bit >= 0) ? &sp_pll_ops : &sp_pll_sub_ops,
> +               .num_parents = 1,
> +               .flags = CLK_IGNORE_UNUSED

Why? Preferably this flag is removed and either CLK_IS_CRITICAL is used,
with a comment indicating what is critical, or the clk is removed
entirely and driver probe just turns the clk on and forgets about it.

> +       };
> +
> +       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> +       if (!pll)
> +               return ERR_PTR(-ENOMEM);
> +
> +       if (reg == PLLSYS_CTL)
> +               initd.flags |= CLK_IS_CRITICAL;

Please comment why it is critical.

> +
> +       pll->hw.init = &initd;
> +       pll->reg = reg;
> +       pll->pd_bit = pd_bit;
> +       pll->bp_bit = bp_bit;
> +       pll->brate = brate;
> +       pll->div_shift = shift;
> +       pll->div_width = width;
> +       pll->lock = lock;
> +
> +       clk = clk_register(NULL, &pll->hw);

Please use clk_hw_register

> +       if (WARN_ON(IS_ERR(clk))) {
> +               kfree(pll);
> +       } else {
> +               pr_info("%-20s%lu\n", name, clk_get_rate(clk));
> +               clk_register_clkdev(clk, NULL, name);
> +       }
> +
> +       return clk;
> +}
> +
> +static void __init sp_clk_setup(struct device_node *np)
> +{
> +       int i, j;
> +
> +       moon_regs = of_iomap(np, 0);

Why moon_ prefix?

> +       if (WARN_ON(!moon_regs))
> +               return; /* -ENXIO */
> +
> +       /* PLL_A */
> +       clks[PLL_A] = clk_register_sp_pll("plla", EXTCLK,
> +                                         PLLA_CTL, 11, 12, 27000000, 0, DIV_A, &plla_lock);
> +
> +       /* PLL_E */
> +       clks[PLL_E] = clk_register_sp_pll("plle", EXTCLK,
> +                                         PLLE_CTL, 6, 2, 50000000, 0, 0, &plle_lock);
> +       clks[PLL_E_2P5] = clk_register_sp_pll("plle_2p5", "plle",
> +                                             PLLE_CTL, 13, -1, 2500000, 0, 0, &plle_lock);
> +       clks[PLL_E_25] = clk_register_sp_pll("plle_25", "plle",
> +                                            PLLE_CTL, 12, -1, 25000000, 0, 0, &plle_lock);
> +       clks[PLL_E_112P5] = clk_register_sp_pll("plle_112p5", "plle",
> +                                               PLLE_CTL, 11, -1, 112500000, 0, 0, &plle_lock);
> +
> +       /* PLL_F */
> +       clks[PLL_F] = clk_register_sp_pll("pllf", EXTCLK,
> +                                         PLLF_CTL, 0, 10, 13500000, 1, 4, &pllf_lock);
> +
> +       /* PLL_TV */
> +       clks[PLL_TV] = clk_register_sp_pll("plltv", EXTCLK,
> +                                          PLLTV_CTL, 0, 15, 27000000, 0, DIV_TV, &plltv_lock);
> +       clks[PLL_TV_A] = clk_register_divider(NULL, "plltv_a", "plltv", 0,
> +                                             PLLTV_CTL + 4, 5, 1,
> +                                             CLK_DIVIDER_POWER_OF_TWO, &plltv_lock);
> +       clk_register_clkdev(clks[PLL_TV_A], NULL, "plltv_a");
> +
> +       /* PLL_SYS */
> +       clks[PLL_SYS] = clk_register_sp_pll("pllsys", EXTCLK,
> +                                           PLLSYS_CTL, 10, 9, 13500000, 0, 4, &pllsys_lock);
> +
> +       /* gates */
> +       for (i = 0; i < ARRAY_SIZE(gates); i++) {
> +               char s[10];
> +
> +               j = gates[i] & 0xffff;
> +               sprintf(s, "clken%02x", j);
> +               clks[j] = clk_register_gate(NULL, s, parents[gates[i] >> 16], CLK_IGNORE_UNUSED,

Please use hw based registration.

> +                                           clk_regs + (j >> 4) * 4, j & 0x0f,
> +                                           CLK_GATE_HIWORD_MASK, NULL);
> +       }
> +
> +       clk_data.clks = clks;
> +       clk_data.clk_num = ARRAY_SIZE(clks);
> +       of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);

Please use a hw based provider instead of a clk one.

> +}
> +
> +CLK_OF_DECLARE(sp_clkc, "sunplus,sp7021-clkc", sp_clk_setup);

Why can't this be a platform driver?

> +
> +MODULE_AUTHOR("Qin Jian <qinjian@cqplus1.com>");
> +MODULE_DESCRIPTION("Sunplus SP7021 Clock Driver");
> +MODULE_LICENSE("GPL v2");

It's not a module, so these macros should be removed.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 05/10] dt-bindings: clock: Add bindings for SP7021 clock driver
  2021-12-03  7:34 ` [PATCH v5 05/10] dt-bindings: clock: Add bindings for SP7021 clock driver Qin Jian
@ 2021-12-16  6:43   ` Stephen Boyd
  0 siblings, 0 replies; 26+ messages in thread
From: Stephen Boyd @ 2021-12-16  6:43 UTC (permalink / raw)
  To: Qin Jian, robh+dt
  Cc: mturquette, tglx, maz, p.zabel, linux, broonie, arnd,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk, wells.lu,
	Qin Jian, Rob Herring

Quoting Qin Jian (2021-12-02 23:34:22)
> Add documentation to describe Sunplus SP7021 clock driver bindings.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Qin Jian <qinjian@cqplus1.com>
> ---
>  .../bindings/clock/sunplus,sp7021-clkc.yaml   |  38 ++++++
>  MAINTAINERS                                   |   2 +
>  include/dt-bindings/clock/sp-sp7021.h         | 112 ++++++++++++++++++
>  3 files changed, 152 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
>  create mode 100644 include/dt-bindings/clock/sp-sp7021.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml b/Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
> new file mode 100644
> index 000000000..1ce7e41d8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) Sunplus Co., Ltd. 2021
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/sunplus,sp7021-clkc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sunplus SP7021 SoC Clock Controller Binding
> +
> +maintainers:
> +  - Qin Jian <qinjian@cqplus1.com>
> +
> +properties:
> +  compatible:
> +    const: sunplus,sp7021-clkc
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  reg:
> +    maxItems: 1

There are some parents it seems, so please specify them as clocks and
optionally clock-names in this binding.

> +
> +required:
> +  - compatible
> +  - "#clock-cells"
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    clkc: clkc@9c000000 {

clock-controller@9c000000

> +      compatible = "sunplus,sp7021-clkc";
> +      #clock-cells = <1>;
> +      reg = <0x9c000000 0x280>;
> +    };
> +
> +...

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2021-12-16  6:43 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-03  7:34 [PATCH v5 00/10] Add Sunplus SP7021 SoC Support Qin Jian
2021-12-03  7:34 ` [PATCH v5 01/10] dt-bindings: vendor-prefixes: Add Sunplus Qin Jian
2021-12-03  7:34 ` [PATCH v5 02/10] dt-bindings: arm: sunplus: Add bindings for Sunplus SP7021 SoC boards Qin Jian
2021-12-03  7:34 ` [PATCH v5 03/10] dt-bindings: reset: Add bindings for SP7021 reset driver Qin Jian
2021-12-03  7:34 ` [PATCH v5 04/10] reset: Add Sunplus " Qin Jian
2021-12-07  8:35   ` Philipp Zabel
2021-12-03  7:34 ` [PATCH v5 05/10] dt-bindings: clock: Add bindings for SP7021 clock driver Qin Jian
2021-12-16  6:43   ` Stephen Boyd
2021-12-03  7:34 ` [PATCH v5 06/10] clk: Add Sunplus " Qin Jian
2021-12-16  6:42   ` Stephen Boyd
2021-12-03  7:34 ` [PATCH v5 07/10] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller Qin Jian
2021-12-03  7:34 ` [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver Qin Jian
2021-12-07  9:02   ` Marc Zyngier
2021-12-08  7:15     ` qinjian[覃健]
2021-12-08  7:45       ` Marc Zyngier
2021-12-08  9:28         ` qinjian[覃健]
2021-12-08 16:02           ` Marc Zyngier
2021-12-03  7:34 ` [PATCH v5 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC Qin Jian
2021-12-03 12:59   ` Arnd Bergmann
2021-12-07  7:21     ` qinjian[覃健]
2021-12-07  9:11       ` Arnd Bergmann
2021-12-09  8:49         ` qinjian[覃健]
2021-12-09  9:58           ` Arnd Bergmann
2021-12-09 10:12             ` Ard Biesheuvel
2021-12-03  7:34 ` [PATCH v5 10/10] ARM: sp7021_defconfig: Add Sunplus SP7021 defconfig Qin Jian
2021-12-03 12:49   ` Arnd Bergmann

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).